Merge pull request #940 from open-gsd/hotfix/1.4.3
chore: merge release v1.4.3 to main
This commit is contained in:
Tom Boucher
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "gsd-core",
|
||||
"displayName": "GSD Core",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.",
|
||||
"author": {
|
||||
"name": "open-gsd",
|
||||
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -113,6 +113,7 @@ build/
|
||||
/gsd-core/bin/lib/model-profiles.cjs
|
||||
/gsd-core/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs
|
||||
/gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs
|
||||
/gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs
|
||||
/gsd-core/bin/lib/observability/logger.cjs
|
||||
/gsd-core/bin/lib/active-workstream-store.cjs
|
||||
/gsd-core/bin/lib/adr-parser.cjs
|
||||
|
||||
12
CHANGELOG.md
12
CHANGELOG.md
@@ -6,6 +6,18 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [1.4.3] - 2026-06-09
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fix `--reapply` verifier false-positives on post-#604-rename installs caused by two gaps in pristine-baseline handling:
|
||||
|
||||
**Gap 1** (`verify-reapply-patches.cjs`): when `backup-meta.json` records a `pristine_hash` for a file but `gsd-pristine/` has no corresponding snapshot on disk, the verifier fell to over-broad mode (every upstream-changed line treated as a user-added requirement) and produced `FAIL_USER_LINES_MISSING` false positives. Fix: return advisory `OK_NO_BASELINE` reason (non-blocking, exit 0) when a recorded hash is present but the pristine file is absent — the verifier cannot reason correctly without a baseline and must not block.
|
||||
|
||||
**Gap 2** (new migration `004-prune-stale-pristine-snapshots`): migration 003 removed legacy `get-shit-done/` runtime files but left `gsd-pristine/get-shit-done/` orphan snapshots in place. Those stale snapshots referenced `get-shit-done/...` key paths that no longer match the active `gsd-core/...` layout, contributing to `FAIL_INSTALLED_MISSING` false reports. Fix: add a new migration (not editing 003, to preserve its checksum) that removes all files under `gsd-pristine/get-shit-done/`. (#934) (#937)
|
||||
- **`/gsd-update` changelog preview no longer silently fails** — the installer now copies `scripts/changeset/` and `scripts/lib/` into the runtime config dir so `$GSD_DIR/scripts/changeset/cli.cjs` resolves at runtime; `update.md` was updated to use the correct installed path and to surface an explicit error if the CLI is missing rather than swallowing it. (#938)
|
||||
- **`plan-review-convergence` now runs `gsd-plan-phase` inline instead of inside `Agent()`** — both sites that previously wrapped `gsd-plan-phase` in `Agent()` (initial planning + replan loop) have been changed to bare `Skill()` calls at depth 0. On Claude Code, a depth-1 Agent has no Agent tool, so a wrapped `plan-phase` could never spawn `gsd-planner` or `gsd-plan-checker` — the replan loop silently failed to produce a revised plan whenever HIGH concerns were found. Running plan-phase inline from the depth-0 orchestrator (which retains the Agent tool) restores the full planner→checker sub-agent chain. A new structural guard test (`bug-936-no-nested-spawner-wrap.test.cjs`) statically scans all workflow files and fails if any workflow wraps a spawner orchestrator in `Agent()` without a `RUNTIME != claude` carve-out, preventing regression. (#936) (#939)
|
||||
|
||||
## [1.4.2] - 2026-06-09
|
||||
|
||||
### Fixed
|
||||
|
||||
122
bin/install.js
122
bin/install.js
@@ -8496,6 +8496,52 @@ function uninstall(isGlobal, runtime = 'claude') {
|
||||
}
|
||||
}
|
||||
|
||||
// 4a. Remove scripts/changeset/ and scripts/lib/ (#935)
|
||||
// GSD-managed files only: enumerate the exact set the installer writes.
|
||||
// Any file NOT in this set is user-owned and must survive uninstall.
|
||||
// After removing GSD files, attempt to rmdir — if the directory is still
|
||||
// non-empty (user has custom helpers) it stays; otherwise it goes cleanly.
|
||||
const GSD_CHANGESET_FILES = [
|
||||
'cli.cjs', 'parse.cjs', 'render.cjs', 'serialize.cjs',
|
||||
'github-release-notes.cjs', 'lint.cjs', 'new.cjs',
|
||||
'README.md', // documentation only — not user-authored
|
||||
];
|
||||
const GSD_SCRIPTS_LIB_FILES = ['cli-exit.cjs', 'allowlist-ratchet.cjs'];
|
||||
|
||||
const changesetUninstallDir = path.join(targetDir, 'scripts', 'changeset');
|
||||
if (fs.existsSync(changesetUninstallDir)) {
|
||||
let removedChangeset = 0;
|
||||
for (const file of GSD_CHANGESET_FILES) {
|
||||
const fp = path.join(changesetUninstallDir, file);
|
||||
try { fs.unlinkSync(fp); removedChangeset++; } catch (_) { /* best-effort */ }
|
||||
}
|
||||
// Remove directory if empty after our cleanup
|
||||
try { fs.rmdirSync(changesetUninstallDir); } catch (_) { /* Not empty — user content present */ }
|
||||
if (removedChangeset > 0) {
|
||||
removedCount++;
|
||||
console.log(` ${green}✓${reset} Removed scripts/changeset/ GSD files`);
|
||||
}
|
||||
}
|
||||
const scriptsLibUninstallDir = path.join(targetDir, 'scripts', 'lib');
|
||||
if (fs.existsSync(scriptsLibUninstallDir)) {
|
||||
let removedScriptsLib = 0;
|
||||
for (const file of GSD_SCRIPTS_LIB_FILES) {
|
||||
const fp = path.join(scriptsLibUninstallDir, file);
|
||||
try { fs.unlinkSync(fp); removedScriptsLib++; } catch (_) { /* best-effort */ }
|
||||
}
|
||||
// Remove directory if empty after our cleanup
|
||||
try { fs.rmdirSync(scriptsLibUninstallDir); } catch (_) { /* Not empty — user content present */ }
|
||||
if (removedScriptsLib > 0) {
|
||||
removedCount++;
|
||||
console.log(` ${green}✓${reset} Removed scripts/lib/ GSD files`);
|
||||
}
|
||||
}
|
||||
// If scripts/ dir is now empty, remove it too
|
||||
const scriptsUninstallDir = path.join(targetDir, 'scripts');
|
||||
if (fs.existsSync(scriptsUninstallDir)) {
|
||||
try { fs.rmdirSync(scriptsUninstallDir); } catch (_) { /* Not empty — leave it */ }
|
||||
}
|
||||
|
||||
// 5. Remove GSD package.json (CommonJS mode marker)
|
||||
const pkgJsonPath = path.join(targetDir, 'package.json');
|
||||
if (fs.existsSync(pkgJsonPath)) {
|
||||
@@ -9169,6 +9215,24 @@ function writeManifest(configDir, runtime = 'claude', options = {}) {
|
||||
}
|
||||
}
|
||||
|
||||
// Track scripts/changeset/ and scripts/lib/ so saveLocalPatches() can detect drift
|
||||
const changesetInstallDir = path.join(configDir, 'scripts', 'changeset');
|
||||
if (fs.existsSync(changesetInstallDir)) {
|
||||
for (const file of fs.readdirSync(changesetInstallDir)) {
|
||||
if (file.endsWith('.cjs')) {
|
||||
manifest.files['scripts/changeset/' + file] = fileHash(path.join(changesetInstallDir, file));
|
||||
}
|
||||
}
|
||||
}
|
||||
const scriptsLibInstallDir = path.join(configDir, 'scripts', 'lib');
|
||||
if (fs.existsSync(scriptsLibInstallDir)) {
|
||||
for (const file of fs.readdirSync(scriptsLibInstallDir)) {
|
||||
if (file.endsWith('.cjs')) {
|
||||
manifest.files['scripts/lib/' + file] = fileHash(path.join(scriptsLibInstallDir, file));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fs.writeFileSync(path.join(configDir, MANIFEST_NAME), JSON.stringify(manifest, null, 2));
|
||||
return manifest;
|
||||
}
|
||||
@@ -10446,6 +10510,64 @@ function install(isGlobal, runtime = 'claude', options = {}) {
|
||||
console.log(` ${green}✓${reset} Installed hooks/lib/ helpers (git-cmd, graphify-rebuild, ...)`);
|
||||
}
|
||||
|
||||
// Install scripts/changeset/ and scripts/lib/ into <configDir>/scripts/
|
||||
// so that `node "$GSD_DIR/scripts/changeset/cli.cjs"` resolves at runtime.
|
||||
//
|
||||
// The changeset CLI (scripts/changeset/cli.cjs) is invoked by the update
|
||||
// workflow (gsd-core/workflows/update.md) to extract changelog ranges for
|
||||
// the /gsd-update preview step. It was previously only present in the npm
|
||||
// tarball root but never copied to the runtime config dir, causing the
|
||||
// preview to always silently fail (#935).
|
||||
//
|
||||
// cli.cjs requires:
|
||||
// - sibling files in scripts/changeset/ (parse/render/serialize/github-release-notes)
|
||||
// - ../lib/cli-exit.cjs → scripts/lib/cli-exit.cjs
|
||||
// - ../../gsd-core/bin/lib/semver-compare.cjs (already installed under gsd-core/)
|
||||
// - ../../gsd-core/bin/lib/package-identity.cjs (already installed under gsd-core/)
|
||||
//
|
||||
// All runtimes that use the update workflow need this, so we copy unconditionally
|
||||
// (same scope as gsd-core/ itself — every runtime that installs workflows gets it).
|
||||
const changesetSrc = path.join(src, 'scripts', 'changeset');
|
||||
const scriptsLibSrc = path.join(src, 'scripts', 'lib');
|
||||
if (!fs.existsSync(changesetSrc)) {
|
||||
// The changeset CLI source is missing from the package — mark as a hard failure
|
||||
// so the user knows the changelog preview will not work rather than silently degrading.
|
||||
failures.push('scripts/changeset/ (source missing from package — reinstall from npm)');
|
||||
} else {
|
||||
const changesetDest = path.join(targetDir, 'scripts', 'changeset');
|
||||
const scriptsLibDest = path.join(targetDir, 'scripts', 'lib');
|
||||
fs.mkdirSync(changesetDest, { recursive: true });
|
||||
fs.mkdirSync(scriptsLibDest, { recursive: true });
|
||||
// Copy scripts/changeset/ — all .cjs and .md files
|
||||
for (const entry of fs.readdirSync(changesetSrc)) {
|
||||
const srcFile = path.join(changesetSrc, entry);
|
||||
if (fs.statSync(srcFile).isFile()) {
|
||||
fs.copyFileSync(srcFile, path.join(changesetDest, entry));
|
||||
}
|
||||
}
|
||||
// Copy scripts/lib/ — cli-exit.cjs (required by cli.cjs) and any future lib helpers.
|
||||
// Hard-fail if missing: without cli-exit.cjs the installed CLI throws MODULE_NOT_FOUND.
|
||||
if (!fs.existsSync(scriptsLibSrc)) {
|
||||
failures.push('scripts/lib/ (source missing from package — reinstall from npm)');
|
||||
} else {
|
||||
for (const entry of fs.readdirSync(scriptsLibSrc)) {
|
||||
const srcFile = path.join(scriptsLibSrc, entry);
|
||||
if (fs.statSync(srcFile).isFile()) {
|
||||
fs.copyFileSync(srcFile, path.join(scriptsLibDest, entry));
|
||||
}
|
||||
}
|
||||
// Verify the critical dep cli-exit.cjs landed
|
||||
if (!verifyFileInstalled(path.join(scriptsLibDest, 'cli-exit.cjs'), 'scripts/lib/cli-exit.cjs')) {
|
||||
failures.push('scripts/lib/cli-exit.cjs');
|
||||
}
|
||||
}
|
||||
if (verifyFileInstalled(path.join(changesetDest, 'cli.cjs'), 'scripts/changeset/cli.cjs')) {
|
||||
console.log(` ${green}✓${reset} Installed scripts/changeset/ (changelog preview CLI)`);
|
||||
} else {
|
||||
failures.push('scripts/changeset/cli.cjs');
|
||||
}
|
||||
}
|
||||
|
||||
// Remove legacy get-shit-done-cc artifacts and stale update caches (#607).
|
||||
// cleanupLegacyGsdCc handles both the legacy shared cache and the per-package
|
||||
// cache (formerly an inline unlinkSync here). A cleanup failure must never
|
||||
|
||||
@@ -17,11 +17,11 @@ requires: [phase, review]
|
||||
Cross-AI plan convergence loop — an outer revision gate around gsd-review and gsd-planner.
|
||||
Repeatedly: review plans with external AI CLIs → if HIGH concerns found → replan with --reviews feedback → re-review. Stops when no HIGH concerns remain or max cycles reached.
|
||||
|
||||
**Flow:** Agent→Skill("gsd-plan-phase") → Agent→Skill("gsd-review") → check HIGHs → Agent→Skill("gsd-plan-phase --reviews") → Agent→Skill("gsd-review") → ... → Converge or escalate
|
||||
**Flow:** Skill("gsd-plan-phase") → Agent→Skill("gsd-review") → check HIGHs → Skill("gsd-plan-phase --reviews") → Agent→Skill("gsd-review") → ... → Converge or escalate
|
||||
|
||||
Replaces gsd-plan-phase's internal gsd-plan-checker with external AI reviewers (codex, gemini, etc.). Each step runs inside an isolated Agent that calls the corresponding existing Skill — orchestrator only does loop control.
|
||||
Replaces gsd-plan-phase's internal gsd-plan-checker with external AI reviewers (codex, gemini, etc.). Plan-phase runs **inline** (bare Skill at depth 0) so it can spawn gsd-planner/gsd-plan-checker at depth 1. Review runs inside an isolated Agent (gsd-review is a Bash leaf — no sub-agents needed). Orchestrator only does loop control.
|
||||
|
||||
**Orchestrator role:** Parse arguments, validate phase, spawn Agents for existing Skills, check HIGHs, stall detection, escalation gate.
|
||||
**Orchestrator role:** Parse arguments, validate phase, run plan-phase inline (Skill at depth 0), spawn an Agent for gsd-review, check HIGHs, stall detection, escalation gate.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
|
||||
@@ -75,6 +75,7 @@ export default tseslint.config(
|
||||
'gsd-core/bin/lib/model-profiles.cjs',
|
||||
'gsd-core/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs',
|
||||
'gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs',
|
||||
'gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs',
|
||||
'gsd-core/bin/lib/observability/logger.cjs',
|
||||
'gsd-core/bin/lib/active-workstream-store.cjs',
|
||||
'gsd-core/bin/lib/adr-parser.cjs',
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gsd-core",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"description": "GSD Core — a meta-prompting, context engineering, and spec-driven development system for AI coding agents. Loads gsd's operating context into every Gemini CLI session.",
|
||||
"contextFileName": "GEMINI.md"
|
||||
}
|
||||
|
||||
@@ -165,6 +165,19 @@ const REASON = Object.freeze({
|
||||
// resolve this file; the guard here ensures the gate does not report spurious
|
||||
// failures in the meantime.
|
||||
OK_PRISTINE_DRIFT_DETECTED: 'ok_pristine_drift_detected',
|
||||
// Bug #934: backup-meta.json records a pristine_hash for this file but the
|
||||
// gsd-pristine/ file is absent from disk. This happens on post-#604-rename
|
||||
// installs where saveLocalPatches discarded the only pristine candidate
|
||||
// because its hash did not match the old-release hash (the file changed
|
||||
// upstream between releases). Without a baseline the verifier cannot
|
||||
// distinguish user-added lines from upstream-changed lines, so falling to
|
||||
// over-broad mode would produce FAIL_USER_LINES_MISSING false positives for
|
||||
// every upstream-removed line. The correct posture is advisory/non-blocking:
|
||||
// report OK_NO_BASELINE so the caller can log a warning without halting the
|
||||
// gate on a spurious failure. This is a bounded "cannot reason → do not
|
||||
// block" rather than "ignore everything" — it only applies when the hash was
|
||||
// recorded (modern installer) but the file is absent (specific gap).
|
||||
OK_NO_BASELINE: 'ok_no_baseline',
|
||||
FAIL_INSTALLED_MISSING: 'fail_installed_missing',
|
||||
FAIL_INSTALLED_NOT_REGULAR_FILE: 'fail_installed_not_regular_file',
|
||||
FAIL_READ_ERROR: 'fail_read_error',
|
||||
@@ -208,11 +221,25 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe
|
||||
return result;
|
||||
}
|
||||
|
||||
// Normalize to forward slashes so the key lookup matches on Windows
|
||||
// where path.join produces backslash-separated relPath values but
|
||||
// backup-meta.json stores keys written with forward slashes.
|
||||
const hashKey = relPath.replace(/\\/g, '/');
|
||||
const recordedHash = pristineHashes && pristineHashes[hashKey];
|
||||
|
||||
let pristineContent = null;
|
||||
if (pristineDir) {
|
||||
const pristinePath = path.join(pristineDir, relPath);
|
||||
// Bug #934: track whether the pristine path EXISTS on disk (stat did not
|
||||
// throw ENOENT). A regular file that fails to read, or a non-file path
|
||||
// (e.g. a directory accidentally placed at the pristine path), is treated
|
||||
// as "present but unusable" — we fall to over-broad mode (safe side).
|
||||
// OK_NO_BASELINE is reserved for the strictly absent case: stat throws,
|
||||
// meaning the file was never written (the gap the bug describes).
|
||||
let pristinePathExists = false;
|
||||
try {
|
||||
const stat = fs.statSync(pristinePath);
|
||||
pristinePathExists = true; // path exists (any type)
|
||||
if (stat.isFile()) {
|
||||
const candidate = fs.readFileSync(pristinePath, 'utf8');
|
||||
// Bug #3657: if backup-meta.json recorded a pristine_hash for this
|
||||
@@ -226,11 +253,6 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe
|
||||
// Over-broad mode never false-fails for a different reason because all
|
||||
// backup lines that are genuinely user-added will still be present in a
|
||||
// correctly merged install.
|
||||
// Normalize to forward slashes so the key lookup matches on Windows
|
||||
// where path.join produces backslash-separated relPath values but
|
||||
// backup-meta.json stores keys written with forward slashes.
|
||||
const hashKey = relPath.replace(/\\/g, '/');
|
||||
const recordedHash = pristineHashes && pristineHashes[hashKey];
|
||||
if (recordedHash) {
|
||||
if (sha256(candidate) === recordedHash) {
|
||||
// Hash matches: the on-disk pristine is the correct baseline.
|
||||
@@ -251,8 +273,28 @@ function verifyFile({ relPath, patchesDir, configDir, pristineDir, pristineHashe
|
||||
pristineContent = candidate;
|
||||
}
|
||||
}
|
||||
// Non-file at pristinePath (e.g. a directory): stat succeeded so
|
||||
// pristinePathExists is true; we fall through to over-broad mode below,
|
||||
// which is safe and conservative.
|
||||
} catch {
|
||||
// Pristine missing or unreadable — fall through to over-broad mode.
|
||||
// Pristine stat threw — path is absent (ENOENT) or inaccessible.
|
||||
// pristinePathExists stays false.
|
||||
}
|
||||
|
||||
// Bug #934: recordedHash is present (modern installer) but the pristine
|
||||
// path does not exist on disk at all (stat threw above). This means
|
||||
// saveLocalPatches recorded a hash but could not write the corresponding
|
||||
// gsd-pristine/ file (the only candidate was discarded because it was from
|
||||
// a newer release). Falling to over-broad mode here would treat every
|
||||
// upstream-changed line as a "user-added line that must survive", producing
|
||||
// false FAIL_USER_LINES_MISSING for each upstream removal. Since we
|
||||
// cannot reason correctly without a baseline, the safe answer is advisory/
|
||||
// non-blocking: return OK_NO_BASELINE and let the caller decide.
|
||||
// NOTE: this guard fires ONLY when stat threw (path absent), not when the
|
||||
// path is present but non-file — in that case over-broad mode is safer.
|
||||
if (!pristinePathExists && recordedHash) {
|
||||
result.reason = REASON.OK_NO_BASELINE;
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -316,9 +358,17 @@ function main() {
|
||||
const drifted = driftedResults.length;
|
||||
const drifted_files = driftedResults.map((r) => r.file);
|
||||
|
||||
// Bug #934: aggregate no-baseline files into top-level report fields so the
|
||||
// workflow can log a warning about files that could not be verified. Like
|
||||
// drift, this is NOT a failure (exit code stays 0) but gives the caller
|
||||
// structured data to surface the advisory condition.
|
||||
const noBaselineResults = results.filter((r) => r.reason === REASON.OK_NO_BASELINE);
|
||||
const no_baseline = noBaselineResults.length;
|
||||
const no_baseline_files = noBaselineResults.map((r) => r.file);
|
||||
|
||||
if (opts.json) {
|
||||
process.stdout.write(
|
||||
JSON.stringify({ checked: results.length, failures: failures.length, drifted, drifted_files, results }, null, 2) + '\n',
|
||||
JSON.stringify({ checked: results.length, failures: failures.length, drifted, drifted_files, no_baseline, no_baseline_files, results }, null, 2) + '\n',
|
||||
);
|
||||
} else {
|
||||
process.stdout.write(`# Hunk Verification Gate (#2969)\n\n`);
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
<purpose>
|
||||
Cross-AI plan convergence loop — automates the manual chain:
|
||||
gsd-plan-phase N → gsd-review N --codex → gsd-plan-phase N --reviews → gsd-review N --codex → ...
|
||||
Each step runs inside an isolated Agent that calls the corresponding Skill.
|
||||
Plan-phase runs inline (bare Skill at depth 0) so it can spawn gsd-planner/gsd-plan-checker at depth 1.
|
||||
Review runs inside an isolated Agent (leaf skill — Bash only, no sub-agents needed).
|
||||
Orchestrator only does: init, loop control, parse CYCLE_SUMMARY for HIGH count, stall detection, escalation.
|
||||
</purpose>
|
||||
|
||||
@@ -98,21 +99,15 @@ Display startup banner:
|
||||
|
||||
**If `has_plans` is false:**
|
||||
|
||||
Display: `◆ No plans found — spawning initial planning agent... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)`
|
||||
Display: `◆ No plans found — running initial planning inline... (plan-phase runs here in the orchestrator — no output until planning is complete, ~1–5 min; expected, not a freeze)`
|
||||
|
||||
```text
|
||||
Agent(
|
||||
description="Initial planning Phase {PHASE}",
|
||||
prompt="Run /gsd:plan-phase for Phase {PHASE}.
|
||||
|
||||
Execute: Skill(skill='gsd-plan-phase', args='{PHASE} {GSD_WS}')
|
||||
|
||||
Complete the full planning workflow. Do NOT return until planning is complete and PLAN.md files are committed.",
|
||||
mode="auto"
|
||||
)
|
||||
Skill(skill="gsd-plan-phase", args="{PHASE} {GSD_WS}")
|
||||
```
|
||||
|
||||
After agent returns, verify plans were created:
|
||||
Run plan-phase **inline** (do NOT wrap it in Agent()). The convergence orchestrator runs at depth 0 with Agent available, so inline plan-phase can spawn gsd-planner and gsd-plan-checker at depth 1 — the one level of nesting that works on Claude Code. Wrapping plan-phase in Agent() would push it to depth 1 where the Agent tool is absent, preventing it from spawning any sub-agents. Wait until plan-phase completes and PLAN.md files are committed before continuing.
|
||||
|
||||
After plan-phase completes, verify plans were created:
|
||||
```bash
|
||||
PLAN_COUNT=$(ls ${phase_dir}/${padded_phase}-*-PLAN.md 2>/dev/null | wc -l)
|
||||
```
|
||||
@@ -302,44 +297,35 @@ To restart loop: /gsd:plan-review-convergence {PHASE} {REVIEWER_FLAGS}
|
||||
```
|
||||
Exit workflow.
|
||||
|
||||
### 5d. Replan (Spawn Agent)
|
||||
### 5d. Replan (Inline)
|
||||
|
||||
**If under max cycles:**
|
||||
|
||||
Update `prev_high_count = HIGH_COUNT`.
|
||||
|
||||
Display: `◆ Spawning replan agent with review feedback... (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)`
|
||||
Display: `◆ Replanning inline with review feedback... (plan-phase runs here in the orchestrator — no output until replanning is complete, ~1–5 min; expected, not a freeze)`
|
||||
|
||||
```text
|
||||
Agent(
|
||||
description="Replan Phase {PHASE} with review feedback cycle {cycle}",
|
||||
prompt="Run /gsd:plan-phase with --reviews for Phase {PHASE}.
|
||||
|
||||
Execute: Skill(skill='gsd-plan-phase', args='{PHASE} --reviews --skip-research {GSD_WS}')
|
||||
|
||||
This will replan incorporating cross-AI review feedback from REVIEWS.md.
|
||||
Do NOT return until replanning is complete and updated PLAN.md files are committed.
|
||||
|
||||
IMPORTANT: When gsd-plan-phase outputs '## PLANNING COMPLETE', that means replanning is done. Return at that point.",
|
||||
mode="auto"
|
||||
)
|
||||
Skill(skill="gsd-plan-phase", args="{PHASE} --reviews --skip-research {GSD_WS}")
|
||||
```
|
||||
|
||||
After agent returns → go back to **step 5a** (review again).
|
||||
Run plan-phase **inline** (do NOT wrap it in Agent()). Same rationale as step 4: the convergence orchestrator runs at depth 0 with Agent available, so inline plan-phase can spawn gsd-planner and gsd-plan-checker at depth 1. Wrapping in Agent() pushes plan-phase to depth 1 where the Agent tool is absent — the replan loop can never produce a revised plan when HIGHs are found. This is the root cause of bug #936. Wait until plan-phase completes (outputs '## PLANNING COMPLETE') and updated PLAN.md files are committed before continuing.
|
||||
|
||||
After plan-phase completes → go back to **step 5a** (review again).
|
||||
|
||||
</process>
|
||||
|
||||
<success_criteria>
|
||||
- [ ] Config gate checked before running — exits with enable instructions if workflow.plan_review_convergence is false
|
||||
- [ ] Initial planning via Agent → Skill("gsd-plan-phase") if no plans exist
|
||||
- [ ] Review via Agent → Skill("gsd-review") — isolated, not inline; {GSD_WS} forwarded
|
||||
- [ ] Replan via Agent → Skill("gsd-plan-phase --reviews") — isolated, not inline
|
||||
- [ ] Initial planning via inline Skill("gsd-plan-phase") if no plans exist — NOT wrapped in Agent() (bug #936: depth-1 Agent has no Agent tool)
|
||||
- [ ] Review via Agent → Skill("gsd-review") — isolated Agent is correct; gsd-review is a Bash leaf with no sub-agent spawns; {GSD_WS} forwarded
|
||||
- [ ] Replan via inline Skill("gsd-plan-phase --reviews") — NOT wrapped in Agent(); inline lets plan-phase spawn gsd-planner/gsd-plan-checker at depth 1
|
||||
- [ ] Orchestrator only does: init, config gate, loop control, parse CYCLE_SUMMARY for HIGH count, stall detection, escalation
|
||||
- [ ] HIGH count extracted from review agent's CYCLE_SUMMARY return message (not by grepping REVIEWS.md)
|
||||
- [ ] Review agent prompt defines CYCLE_SUMMARY: current_high=<N> contract with PARTIALLY/FULLY RESOLVED definitions
|
||||
- [ ] Abort with clear error if CYCLE_SUMMARY is absent; distinguish malformed from absent
|
||||
- [ ] Warn if HIGH_COUNT > 0 but ## Current HIGH Concerns section is absent from return message
|
||||
- [ ] Each Agent fully completes its Skill before returning
|
||||
- [ ] The review Agent fully completes gsd-review before returning (plan-phase runs inline — no Agent wrap)
|
||||
- [ ] Loop exits on: no HIGH concerns (converged) OR max cycles (escalation)
|
||||
- [ ] Stall detection reported when HIGH count not decreasing
|
||||
- [ ] STATE.md updated on convergence completion
|
||||
|
||||
@@ -299,11 +299,28 @@ VERIFY_OUTPUT="$(node "${GSD_HOME}/gsd-core/bin/verify-reapply-patches.cjs" "${V
|
||||
VERIFY_STATUS=$?
|
||||
```
|
||||
|
||||
**Step 5a: drift check** — even when `VERIFY_STATUS` is 0, the report may signal that one or more files were skipped due to pristine-snapshot drift (Bug #3657). Parse the JSON and check:
|
||||
**Step 5a: drift check** — even when `VERIFY_STATUS` is 0, the report may signal that one or more files were skipped due to pristine-snapshot drift (Bug #3657) or a missing baseline (Bug #934). Parse the JSON and check:
|
||||
|
||||
```bash
|
||||
DRIFTED_COUNT="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));process.stdout.write(String(d.drifted||0))")"
|
||||
DRIFTED_FILES="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));(d.drifted_files||[]).forEach(f=>process.stdout.write(f+'\n'))")"
|
||||
NO_BASELINE_COUNT="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));process.stdout.write(String(d.no_baseline||0))")"
|
||||
NO_BASELINE_FILES="$(echo "$VERIFY_OUTPUT" | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8'));(d.no_baseline_files||[]).forEach(f=>process.stdout.write(f+'\n'))")"
|
||||
```
|
||||
|
||||
**If `NO_BASELINE_COUNT` is greater than 0**, emit an advisory warning (non-blocking — the gate still exits 0 for these files). Do NOT halt:
|
||||
|
||||
```text
|
||||
ADVISORY: {NO_BASELINE_COUNT} file(s) could not be diff-verified because no pristine
|
||||
baseline exists on disk despite a hash being recorded in backup-meta.json (Bug #934:
|
||||
the installer discarded the only pristine candidate because it was from a newer release).
|
||||
These files were skipped rather than false-failed; their user customisations may or
|
||||
may not have survived the merge.
|
||||
|
||||
Unverified files:
|
||||
{each path in NO_BASELINE_FILES, one per line, indented two spaces}
|
||||
|
||||
Recommended: manually inspect each file above and confirm your customisations survived.
|
||||
```
|
||||
|
||||
**If `DRIFTED_COUNT` is greater than 0**, STOP and report to the user, then set `DRIFT_DETECTED=true` and halt — do not proceed to 5b or cleanup:
|
||||
|
||||
@@ -195,24 +195,29 @@ CHANGELOG_TMP="/tmp/gsd-changelog-$$.md"
|
||||
curl -fsSL "https://raw.githubusercontent.com/open-gsd/gsd-core/main/CHANGELOG.md" -o "$CHANGELOG_TMP" 2>/dev/null \
|
||||
|| wget -qO "$CHANGELOG_TMP" "https://raw.githubusercontent.com/open-gsd/gsd-core/main/CHANGELOG.md" 2>/dev/null
|
||||
|
||||
EXTRACT_JSON=$(node "$GSD_DIR/gsd-core/scripts/changeset/cli.cjs" extract \
|
||||
--from "$INSTALLED_VERSION" \
|
||||
--to "$LATEST_VERSION" \
|
||||
--changelog "$CHANGELOG_TMP" \
|
||||
--json 2>/dev/null)
|
||||
EXTRACT_EXIT=$?
|
||||
|
||||
if [ "$EXTRACT_EXIT" -eq 2 ]; then
|
||||
# Exit 2 = no releases in range (e.g. versions are equal or changelog is sparse)
|
||||
CHANGELOG_PREVIEW="No changelog updates between v${INSTALLED_VERSION} and v${LATEST_VERSION}."
|
||||
elif [ "$EXTRACT_EXIT" -ne 0 ] || [ -z "$EXTRACT_JSON" ]; then
|
||||
CHANGELOG_PREVIEW="(Could not extract changelog — update will still proceed)"
|
||||
GSD_CHANGESET_CLI="$GSD_DIR/scripts/changeset/cli.cjs"
|
||||
if [ ! -f "$GSD_CHANGESET_CLI" ]; then
|
||||
CHANGELOG_PREVIEW="(Changelog CLI not found at $GSD_CHANGESET_CLI — reinstall GSD to restore preview. Update will still proceed.)"
|
||||
else
|
||||
# Re-run without --json to get the human-readable markdown for display
|
||||
CHANGELOG_PREVIEW=$(node "$GSD_DIR/gsd-core/scripts/changeset/cli.cjs" extract \
|
||||
EXTRACT_JSON=$(node "$GSD_CHANGESET_CLI" extract \
|
||||
--from "$INSTALLED_VERSION" \
|
||||
--to "$LATEST_VERSION" \
|
||||
--changelog "$CHANGELOG_TMP" 2>/dev/null || echo "(changelog unavailable)")
|
||||
--changelog "$CHANGELOG_TMP" \
|
||||
--json 2>&1)
|
||||
EXTRACT_EXIT=$?
|
||||
|
||||
if [ "$EXTRACT_EXIT" -eq 2 ]; then
|
||||
# Exit 2 = no releases in range (e.g. versions are equal or changelog is sparse)
|
||||
CHANGELOG_PREVIEW="No changelog updates between v${INSTALLED_VERSION} and v${LATEST_VERSION}."
|
||||
elif [ "$EXTRACT_EXIT" -ne 0 ] || [ -z "$EXTRACT_JSON" ]; then
|
||||
CHANGELOG_PREVIEW="(Could not extract changelog — update will still proceed)"
|
||||
else
|
||||
# Re-run without --json to get the human-readable markdown for display
|
||||
CHANGELOG_PREVIEW=$(node "$GSD_CHANGESET_CLI" extract \
|
||||
--from "$INSTALLED_VERSION" \
|
||||
--to "$LATEST_VERSION" \
|
||||
--changelog "$CHANGELOG_TMP" 2>/dev/null || echo "(changelog unavailable)")
|
||||
fi
|
||||
fi
|
||||
# Clean up temp changelog now that both extract runs are done
|
||||
rm -f "$CHANGELOG_TMP"
|
||||
|
||||
4
package-lock.json
generated
4
package-lock.json
generated
@@ -1,12 +1,12 @@
|
||||
{
|
||||
"name": "@opengsd/gsd-core",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "@opengsd/gsd-core",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@anthropic-ai/claude-agent-sdk": "^0.2.84",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@opengsd/gsd-core",
|
||||
"version": "1.4.2",
|
||||
"version": "1.4.3",
|
||||
"description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.",
|
||||
"bin": {
|
||||
"gsd-core": "bin/install.js",
|
||||
|
||||
@@ -324,7 +324,7 @@ function resolveChangelogPath(opts) {
|
||||
* 1 — I/O error or missing required flags.
|
||||
*
|
||||
* Fix for #3496: provides a deterministic range-aware helper so the
|
||||
* `/gsd:update` show_changes_and_confirm step no longer relies on
|
||||
* `/gsd-update` show_changes_and_confirm step no longer relies on
|
||||
* vague/manual extraction that can silently skip intermediate versions.
|
||||
*/
|
||||
function cmdExtract(opts) {
|
||||
|
||||
145
src/installer-migrations/004-prune-stale-pristine-snapshots.cts
Normal file
145
src/installer-migrations/004-prune-stale-pristine-snapshots.cts
Normal file
@@ -0,0 +1,145 @@
|
||||
/**
|
||||
* Installer migration 004: remove stale gsd-pristine/get-shit-done/ snapshot // gsd-allow-legacy-name
|
||||
* files after the get-shit-done → gsd-core rename (#604, #934). // gsd-allow-legacy-name
|
||||
*
|
||||
* Background: migration 003 removed legacy runtime files from
|
||||
* get-shit-done/ but did not touch gsd-pristine/get-shit-done/, the // gsd-allow-legacy-name
|
||||
* parallel directory that holds pristine snapshots captured before the rename.
|
||||
* These snapshot files are GSD-managed (written by the installer, never by the
|
||||
* user) and reference stale get-shit-done/... key paths that no longer exist // gsd-allow-legacy-name
|
||||
* in the active layout. When verify-reapply-patches.cjs looks up a backup entry
|
||||
* keyed under gsd-core/... it finds no matching gsd-pristine/ snapshot, falls
|
||||
* to over-broad mode, and reports false FAIL_INSTALLED_MISSING / // gsd-allow-legacy-name
|
||||
* FAIL_USER_LINES_MISSING for every backed-up pre-rename file (#934).
|
||||
*
|
||||
* Fix: walk gsd-pristine/get-shit-done/ and emit remove-managed for each file. // gsd-allow-legacy-name
|
||||
* These files are always GSD-written snapshots — users never place their own
|
||||
* files inside gsd-pristine/ — so the classification override
|
||||
* (managed-pristine) is safe: there is no user content to protect.
|
||||
*
|
||||
* Checksum safety: migration 003's body is left untouched. Adding this
|
||||
* separate migration avoids modifying 003's checksum, which would break
|
||||
* upgrade state for any user who already applied 003 (root cause of #670).
|
||||
*
|
||||
* Per-file approach: the migration framework has no recursive directory-removal
|
||||
* primitive — all actions operate on individual files. Empty directory shells
|
||||
* left after removal can be cleaned up manually; this is the intentional ADR-0008
|
||||
* limitation.
|
||||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
interface MigrationAction {
|
||||
type: string;
|
||||
relPath: string;
|
||||
reason: string;
|
||||
ownershipEvidence: string;
|
||||
classification?: string;
|
||||
}
|
||||
|
||||
interface MigrationPlanContext {
|
||||
configDir: string;
|
||||
classifyArtifact(relPath: string): { classification: string; [key: string]: unknown };
|
||||
}
|
||||
|
||||
interface InstallerMigration {
|
||||
id: string;
|
||||
title: string;
|
||||
description: string;
|
||||
introducedIn: string;
|
||||
scopes: string[];
|
||||
destructive: boolean;
|
||||
plan(ctx: MigrationPlanContext): MigrationAction[];
|
||||
}
|
||||
|
||||
function walkPristineFiles(root: string, relDir: string, baseResolved: string, results: string[]): void {
|
||||
const dir = path.join(root, relDir);
|
||||
let entries;
|
||||
try {
|
||||
entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return; // directory absent or unreadable — nothing to do
|
||||
}
|
||||
for (const entry of entries) {
|
||||
// Do not follow symlinks — skip to avoid out-of-tree traversal.
|
||||
if (entry.isSymbolicLink()) continue;
|
||||
const relPath = path.posix.join(relDir, entry.name);
|
||||
// Bounds check: ensure the resolved path stays under configDir.
|
||||
const resolved = path.resolve(root, relPath);
|
||||
if (resolved !== baseResolved && !resolved.startsWith(baseResolved + path.sep)) continue;
|
||||
if (entry.isDirectory()) {
|
||||
walkPristineFiles(root, relPath, baseResolved, results);
|
||||
} else if (entry.isFile()) {
|
||||
results.push(relPath);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const REASON = 'stale pristine snapshot from legacy get-shit-done/ dir, orphaned by rename migration 003 (#604, #934)'; // gsd-allow-legacy-name
|
||||
|
||||
const migration: InstallerMigration = {
|
||||
id: '2026-06-09-prune-stale-pristine-get-shit-done', // gsd-allow-legacy-name
|
||||
title: 'Remove stale gsd-pristine/get-shit-done/ snapshot files (#934)', // gsd-allow-legacy-name
|
||||
description:
|
||||
'Migration 003 removed runtime files from get-shit-done/ but left the matching pristine snapshot ' + // gsd-allow-legacy-name
|
||||
'directory gsd-pristine/get-shit-done/ intact. Those snapshots reference stale key paths and cause ' + // gsd-allow-legacy-name
|
||||
'verify-reapply-patches false positives (#934). Remove all files under gsd-pristine/get-shit-done/ ' + // gsd-allow-legacy-name
|
||||
'as they are GSD-managed snapshots, never user content.',
|
||||
introducedIn: '1.4.3',
|
||||
scopes: ['global', 'local'],
|
||||
destructive: true,
|
||||
plan(ctx: MigrationPlanContext): MigrationAction[] {
|
||||
const pristineGsdRoot = path.join(ctx.configDir, 'gsd-pristine', 'get-shit-done'); // gsd-allow-legacy-name
|
||||
|
||||
// Idempotency: if the stale pristine subdir doesn't exist, nothing to do.
|
||||
if (!fs.existsSync(pristineGsdRoot)) return [];
|
||||
|
||||
// Safety: reject symlinks in ANY ancestor component of the path we will walk
|
||||
// to prevent following a symlink out of configDir. Check both gsd-pristine/
|
||||
// and gsd-pristine/get-shit-done/ — either being a symlink could redirect // gsd-allow-legacy-name
|
||||
// the walk to an out-of-tree location.
|
||||
const pristineParent = path.join(ctx.configDir, 'gsd-pristine');
|
||||
try {
|
||||
if (fs.lstatSync(pristineParent).isSymbolicLink()) return [];
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
try {
|
||||
if (fs.lstatSync(pristineGsdRoot).isSymbolicLink()) return []; // gsd-allow-legacy-name
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
|
||||
const baseResolved = path.resolve(ctx.configDir);
|
||||
const relPaths: string[] = [];
|
||||
walkPristineFiles(ctx.configDir, path.posix.join('gsd-pristine', 'get-shit-done'), baseResolved, relPaths); // gsd-allow-legacy-name
|
||||
|
||||
const actions: MigrationAction[] = [];
|
||||
for (const relPath of relPaths) {
|
||||
// Bounds-check each relPath before emitting any action.
|
||||
const resolved = path.resolve(ctx.configDir, relPath);
|
||||
if (resolved !== baseResolved && !resolved.startsWith(baseResolved + path.sep)) continue;
|
||||
|
||||
// These files are GSD-managed pristine snapshots — the installer writes
|
||||
// them during install/upgrade; users never place personal files inside
|
||||
// gsd-pristine/. Pass classification: 'managed-pristine' explicitly so
|
||||
// the framework does not downgrade remove-managed to preserve-user when
|
||||
// the manifest has no entry (these paths were never in the manifest since
|
||||
// they live under gsd-pristine/, not the tracked runtime dir).
|
||||
actions.push({
|
||||
type: 'remove-managed',
|
||||
relPath,
|
||||
reason: REASON,
|
||||
ownershipEvidence:
|
||||
'GSD-written pristine snapshot under gsd-pristine/get-shit-done/; ' + // gsd-allow-legacy-name
|
||||
'installer is the sole author of gsd-pristine/ contents; no user content lives here',
|
||||
classification: 'managed-pristine',
|
||||
});
|
||||
}
|
||||
|
||||
return actions;
|
||||
},
|
||||
};
|
||||
|
||||
export = migration;
|
||||
@@ -88,6 +88,7 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => {
|
||||
// Locks the public diagnostic surface — adding a code requires updating
|
||||
// this assertion, removing one breaks consumers that switch on the enum.
|
||||
// Bug #3657 added OK_PRISTINE_DRIFT_DETECTED.
|
||||
// Bug #934 added OK_NO_BASELINE.
|
||||
assert.deepEqual(
|
||||
Object.keys(REASON).sort(),
|
||||
[
|
||||
@@ -95,6 +96,7 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => {
|
||||
'FAIL_INSTALLED_NOT_REGULAR_FILE',
|
||||
'FAIL_READ_ERROR',
|
||||
'FAIL_USER_LINES_MISSING',
|
||||
'OK_NO_BASELINE',
|
||||
'OK_NO_SIGNIFICANT_BACKUP_LINES',
|
||||
'OK_NO_USER_LINES_VS_PRISTINE',
|
||||
'OK_PRISTINE_DRIFT_DETECTED',
|
||||
@@ -178,7 +180,8 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => {
|
||||
assert.equal(status, 1);
|
||||
// Bug #3657 (Finding 1): drifted + drifted_files are additive fields added to surface
|
||||
// pristine-drift skips distinctly from failures. Shape-lock updated to include them.
|
||||
assert.deepEqual(Object.keys(report).sort(), ['checked', 'drifted', 'drifted_files', 'failures', 'results']);
|
||||
// Bug #934: no_baseline + no_baseline_files are additive fields for missing-pristine advisory.
|
||||
assert.deepEqual(Object.keys(report).sort(), ['checked', 'drifted', 'drifted_files', 'failures', 'no_baseline', 'no_baseline_files', 'results']);
|
||||
const r0 = report.results[0];
|
||||
assert.deepEqual(Object.keys(r0).sort(), ['file', 'missing', 'reason', 'status']);
|
||||
assert.equal(typeof r0.file, 'string');
|
||||
|
||||
@@ -332,6 +332,7 @@ describe('Bug #3657: pristine-drift does not produce false FAIL_USER_LINES_MISSI
|
||||
'FAIL_INSTALLED_NOT_REGULAR_FILE',
|
||||
'FAIL_READ_ERROR',
|
||||
'FAIL_USER_LINES_MISSING',
|
||||
'OK_NO_BASELINE',
|
||||
'OK_NO_SIGNIFICANT_BACKUP_LINES',
|
||||
'OK_NO_USER_LINES_VS_PRISTINE',
|
||||
'OK_PRISTINE_DRIFT_DETECTED',
|
||||
@@ -525,3 +526,156 @@ describe('Bug #3657: pristine-drift does not produce false FAIL_USER_LINES_MISSI
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Bug #934: OK_NO_BASELINE — pristine dir provided, hash recorded, but file absent
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Bug #934: OK_NO_BASELINE when recordedHash present but pristine file absent', () => {
|
||||
|
||||
/**
|
||||
* Core regression: backup-meta.json has a pristine_hash for the file but
|
||||
* the gsd-pristine/ snapshot is absent from disk (the installer's
|
||||
* saveLocalPatches discarded the only candidate because its hash did not
|
||||
* match the old-release hash — the file changed upstream between releases).
|
||||
* Without the fix the verifier falls to over-broad mode and treats every
|
||||
* upstream-removed line as a "user-added line that must survive", producing
|
||||
* FAIL_USER_LINES_MISSING false positives.
|
||||
* With the fix the verifier returns OK_NO_BASELINE (non-blocking, advisory).
|
||||
*/
|
||||
test('exits 0 with reason=OK_NO_BASELINE when recordedHash present but pristine absent', () => {
|
||||
resetFixture();
|
||||
|
||||
const FILE = 'gsd-core/workflows/execute-phase.md';
|
||||
|
||||
// The backup contains both the old upstream content and the user's line.
|
||||
const backupContent =
|
||||
'upstream line that was present in 1.4.0 but removed in 1.4.2 release\n' +
|
||||
'another upstream line removed upstream between gsd-core releases here\n' +
|
||||
'model: sonnet in frontmatter — this is the real user customisation line\n';
|
||||
|
||||
// The installed file has the new upstream content + the user's real line.
|
||||
const installedContent =
|
||||
'brand-new upstream line that replaced the old content in gsd-core 1.4.2\n' +
|
||||
'model: sonnet in frontmatter — this is the real user customisation line\n';
|
||||
|
||||
// backup-meta.json records a hash (modern installer) but gsd-pristine/ is absent.
|
||||
writeBackupMeta({ pristine_hashes: { [FILE]: 'sha256:deadbeef00000000000000000000000000000000000000000000000000000001' } });
|
||||
writeFile(path.join(patchesDir, FILE), backupContent);
|
||||
writeFile(path.join(configDir, FILE), installedContent);
|
||||
// Deliberately do NOT write a pristine file — this is the gap-1 scenario.
|
||||
|
||||
const { status, report } = runVerifier();
|
||||
|
||||
// Must exit 0: cannot reason without baseline → non-blocking advisory.
|
||||
assert.equal(status, 0, `expected exit 0; got ${status}; report=${JSON.stringify(report)}`);
|
||||
assert.equal(report.failures, 0, `expected 0 failures; got ${report.failures}`);
|
||||
const r0 = report.results[0];
|
||||
assert.equal(r0.status, 'ok', `expected status ok; got ${r0.status}`);
|
||||
assert.equal(r0.reason, REASON.OK_NO_BASELINE,
|
||||
`expected OK_NO_BASELINE; got ${r0.reason}`);
|
||||
assert.deepEqual(r0.missing, []);
|
||||
});
|
||||
|
||||
/**
|
||||
* Counter-test: when pristine is absent but NO recordedHash is present
|
||||
* (pre-fix installer that never wrote backup-meta.json), the verifier must
|
||||
* still fall to over-broad mode — the old behaviour for untracked backups.
|
||||
* OK_NO_BASELINE must NOT fire in this case.
|
||||
*/
|
||||
test('falls through to over-broad mode when pristine absent AND no recordedHash', () => {
|
||||
resetFixture();
|
||||
|
||||
const FILE = 'gsd-core/workflows/plan-phase.md';
|
||||
const droppedLine = 'user-added instruction that was dropped from the install output';
|
||||
const backupContent =
|
||||
'stock upstream line long enough to be significant in the file\n' +
|
||||
droppedLine + '\n';
|
||||
const installedContent = 'stock upstream line long enough to be significant in the file\n';
|
||||
|
||||
// No backup-meta.json — simulates pre-fix installer with no hash records.
|
||||
writeFile(path.join(patchesDir, FILE), backupContent);
|
||||
writeFile(path.join(configDir, FILE), installedContent);
|
||||
// No pristine file.
|
||||
|
||||
const { status, report } = runVerifier();
|
||||
|
||||
// Over-broad mode catches the genuinely dropped user line.
|
||||
assert.equal(status, 1, 'over-broad mode should catch the dropped user line');
|
||||
assert.equal(report.failures, 1);
|
||||
const r0 = report.results[0];
|
||||
assert.equal(r0.status, 'fail');
|
||||
assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING);
|
||||
assert.ok(r0.missing.includes(droppedLine),
|
||||
`dropped line must appear in .missing[]; got ${JSON.stringify(r0.missing)}`);
|
||||
// Must NOT be OK_NO_BASELINE — that only fires when a hash WAS recorded.
|
||||
assert.notEqual(r0.reason, REASON.OK_NO_BASELINE);
|
||||
});
|
||||
|
||||
/**
|
||||
* Presence check: when pristine IS present AND hash matches, the normal
|
||||
* flow must proceed (not short-circuit to OK_NO_BASELINE).
|
||||
* A real dropped user line must still be caught.
|
||||
*/
|
||||
test('does not short-circuit to OK_NO_BASELINE when pristine exists and hash matches', () => {
|
||||
resetFixture();
|
||||
|
||||
const FILE = 'gsd-core/workflows/plan-phase.md';
|
||||
const pristineContent = 'stock upstream line long enough to be significant content\n';
|
||||
const droppedLine = 'user customisation that was genuinely dropped from the merged output';
|
||||
const backupContent = pristineContent + droppedLine + '\n';
|
||||
const installedContent = pristineContent; // user line dropped — real failure
|
||||
|
||||
writeBackupMeta({ pristine_hashes: { [FILE]: sha256(pristineContent) } });
|
||||
writeFile(path.join(patchesDir, FILE), backupContent);
|
||||
writeFile(path.join(configDir, FILE), installedContent);
|
||||
writeFile(path.join(pristineDir, FILE), pristineContent);
|
||||
|
||||
const { status, report } = runVerifier();
|
||||
|
||||
assert.equal(status, 1, 'real dropped user line must be caught');
|
||||
assert.equal(report.failures, 1);
|
||||
const r0 = report.results[0];
|
||||
assert.equal(r0.status, 'fail');
|
||||
assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING);
|
||||
assert.notEqual(r0.reason, REASON.OK_NO_BASELINE);
|
||||
assert.ok(r0.missing.includes(droppedLine));
|
||||
});
|
||||
|
||||
/**
|
||||
* When --pristine-dir is NOT provided at all (old CLI invocation without the
|
||||
* flag), the OK_NO_BASELINE path must never fire — there is no pristine dir
|
||||
* context to consult and the old over-broad behaviour must be preserved.
|
||||
*/
|
||||
test('does not return OK_NO_BASELINE when --pristine-dir is not provided', () => {
|
||||
resetFixture();
|
||||
|
||||
const FILE = 'gsd-core/workflows/execute-phase.md';
|
||||
const backupContent =
|
||||
'upstream line removed in newer version but present in backup\n' +
|
||||
'model: sonnet — user customisation line in the backup file\n';
|
||||
const installedContent =
|
||||
'replacement upstream line in the newer release version\n' +
|
||||
'model: sonnet — user customisation line in the backup file\n';
|
||||
|
||||
// Record a hash — but no pristine dir will be passed to the verifier.
|
||||
writeBackupMeta({ pristine_hashes: { [FILE]: 'sha256:deadbeef00000000000000000000000000000000000000000000000000000001' } });
|
||||
writeFile(path.join(patchesDir, FILE), backupContent);
|
||||
writeFile(path.join(configDir, FILE), installedContent);
|
||||
|
||||
// Run without --pristine-dir flag.
|
||||
const { status, report } = runVerifier({ pristine: false });
|
||||
|
||||
// Over-broad mode: every significant backup line is required.
|
||||
// "upstream line removed in newer version but present in backup" is NOT in
|
||||
// the installed content → over-broad mode FAILS this file (exit 1).
|
||||
// OK_NO_BASELINE must NOT fire — there was no pristine dir to consult.
|
||||
assert.equal(status, 1, `over-broad mode should fail (upstream-removed line absent); got ${status}`);
|
||||
const r0 = report.results[0];
|
||||
assert.equal(r0.status, 'fail', `expected fail status; got ${r0.status}`);
|
||||
assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING,
|
||||
`expected FAIL_USER_LINES_MISSING from over-broad mode; got ${r0.reason}`);
|
||||
assert.notEqual(r0.reason, REASON.OK_NO_BASELINE,
|
||||
`OK_NO_BASELINE must not fire when --pristine-dir is not provided`);
|
||||
});
|
||||
});
|
||||
|
||||
207
tests/bug-936-no-nested-spawner-wrap.test.cjs
Normal file
207
tests/bug-936-no-nested-spawner-wrap.test.cjs
Normal file
@@ -0,0 +1,207 @@
|
||||
'use strict';
|
||||
/**
|
||||
* Structural guard — bug(#936): plan-review-convergence wrapped gsd-plan-phase
|
||||
* in Agent() at TWO sites (initial planning + replan). On Claude Code, a depth-1
|
||||
* Agent has no Agent tool, so plan-phase cannot spawn gsd-planner / gsd-plan-checker
|
||||
* → the replan loop never works when HIGHs are found.
|
||||
*
|
||||
* Fix: run plan-phase INLINE (bare Skill()) from the convergence orchestrator,
|
||||
* which runs at depth 0 and has Agent available — exactly how autonomous.md,
|
||||
* manager.md, and discuss-phase-assumptions.md already chain plan-phase.
|
||||
*
|
||||
* This guard dynamically derives the set of "spawner" workflows (those containing
|
||||
* `subagent_type=`) and asserts that NO workflow wraps a spawner inside Agent()
|
||||
* UNLESS the wrapping block includes a RUNTIME != claude carve-out (the #853
|
||||
* pattern already applied to autonomous.md / manager.md).
|
||||
*/
|
||||
|
||||
// allow-test-rule: source-text-is-the-product
|
||||
// The workflow markdown IS the runtime instruction — static guards over
|
||||
// workflow text are the canonical regression-test mechanism (per CONTRIBUTING
|
||||
// exception matrix and tests/bug-853-bg-dispatch-runtime-gating.test.cjs).
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows');
|
||||
|
||||
// ── 1. Derive spawner skill names dynamically ──────────────────────────────
|
||||
// A "spawner" workflow is one that contains `subagent_type=` — it NEEDS the
|
||||
// Agent tool to run and therefore cannot safely be wrapped in another Agent()
|
||||
// on Claude Code (where depth-1 agents have no Agent tool).
|
||||
|
||||
// Recursively collect all *.md files under WORKFLOWS_DIR (covers nested fragments
|
||||
// like discuss-phase/modes/*.md and execute-phase/steps/*.md).
|
||||
function collectWorkflowFiles(dir) {
|
||||
const entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
const results = [];
|
||||
for (const e of entries) {
|
||||
const fullPath = path.join(dir, e.name);
|
||||
if (e.isDirectory()) {
|
||||
results.push(...collectWorkflowFiles(fullPath));
|
||||
} else if (e.name.endsWith('.md')) {
|
||||
results.push({
|
||||
name: path.relative(WORKFLOWS_DIR, fullPath),
|
||||
path: fullPath,
|
||||
content: fs.readFileSync(fullPath, 'utf8'),
|
||||
});
|
||||
}
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
const allWorkflowFiles = collectWorkflowFiles(WORKFLOWS_DIR);
|
||||
|
||||
// Map: base-slug → workflow filename (e.g. "plan-phase" → "plan-phase.md")
|
||||
// Skill() calls use the "gsd-<slug>" convention in all workflow files.
|
||||
// We build BOTH the bare slug set and the gsd-prefixed skill-name set.
|
||||
const SPAWNER_BASE_SLUGS = new Set(
|
||||
allWorkflowFiles
|
||||
.filter((w) => w.content.includes('subagent_type='))
|
||||
.map((w) => w.name.replace(/\.md$/, ''))
|
||||
);
|
||||
|
||||
// Skill invocations use "gsd-<slug>" (e.g. gsd-plan-phase, gsd-execute-phase).
|
||||
// Build the regex from the prefixed names so it actually matches what workflows write.
|
||||
const SPAWNER_GSD_NAMES = new Set([...SPAWNER_BASE_SLUGS].map((s) => `gsd-${s}`));
|
||||
|
||||
// Build a regex that matches Skill(skill='gsd-<spawner>') or Skill(skill="gsd-<spawner>")
|
||||
const spawnerPattern = new RegExp(
|
||||
`Skill\\(\\s*skill=['"](?:${[...SPAWNER_GSD_NAMES].join('|')})['"]`,
|
||||
's'
|
||||
);
|
||||
|
||||
// ── 2. Helper: extract Agent() blocks from a workflow ─────────────────────
|
||||
// Each block starts at "Agent(" and ends at the balancing ")". We collect
|
||||
// the text of each such block together with the surrounding context (a 400
|
||||
// char window before the block) so we can check for RUNTIME carve-outs.
|
||||
|
||||
function extractAgentBlocks(content) {
|
||||
const blocks = [];
|
||||
let pos = 0;
|
||||
while (pos < content.length) {
|
||||
const start = content.indexOf('Agent(', pos);
|
||||
if (start === -1) break;
|
||||
// Walk forward to find the balancing closing paren
|
||||
let depth = 0;
|
||||
let i = start + 'Agent('.length - 1; // at the '('
|
||||
for (; i < content.length; i++) {
|
||||
if (content[i] === '(') depth++;
|
||||
else if (content[i] === ')') {
|
||||
depth--;
|
||||
if (depth === 0) break;
|
||||
}
|
||||
}
|
||||
const end = i + 1;
|
||||
const blockText = content.slice(start, end);
|
||||
// Capture context: 400 chars before the block (for RUNTIME gate detection)
|
||||
const contextBefore = content.slice(Math.max(0, start - 400), start);
|
||||
blocks.push({ start, end, blockText, contextBefore });
|
||||
pos = end;
|
||||
}
|
||||
return blocks;
|
||||
}
|
||||
|
||||
// ── 3. Helper: does a block have a RUNTIME != claude carve-out nearby? ────
|
||||
// The #853 pattern looks like: "RUNTIME is `claude`" in a preceding condition
|
||||
// that switches to inline Skill() instead of the Agent() block. A block is
|
||||
// considered guarded when the 400-char context window before it (or the block
|
||||
// body itself for block-internal guards) contains any of these markers.
|
||||
|
||||
function hasRuntimeCarveout(block) {
|
||||
const haystack = block.contextBefore + block.blockText;
|
||||
return (
|
||||
/RUNTIME[^`\n]{0,30}(?:!=|≠|is not|!==)\s*[`'"]?claude/i.test(haystack) ||
|
||||
/RUNTIME[^`\n]{0,30}claude[^`\n]{0,30}(?:inline|not.*Agent|do NOT)/i.test(haystack) ||
|
||||
/If `RUNTIME` is `claude`/i.test(haystack) ||
|
||||
/On Claude Code.*inline/is.test(haystack)
|
||||
);
|
||||
}
|
||||
|
||||
// ── 4. The guard: scan every workflow for unguarded Agent→spawner wraps ───
|
||||
|
||||
describe('bug-936 — no workflow wraps a spawner skill inside Agent() without a RUNTIME carve-out', () => {
|
||||
test('spawner set is non-empty (self-check: subagent_type= grep must find files)', () => {
|
||||
assert.ok(SPAWNER_BASE_SLUGS.size > 0, `No spawner workflows found in ${WORKFLOWS_DIR} — SPAWNER_BASE_SLUGS derivation is broken`);
|
||||
// plan-phase must be a spawner (base slug)
|
||||
assert.ok(SPAWNER_BASE_SLUGS.has('plan-phase'), 'plan-phase.md must be in the spawner set (contains subagent_type=)');
|
||||
// gsd-plan-phase must be in the prefixed set used by the regex
|
||||
assert.ok(SPAWNER_GSD_NAMES.has('gsd-plan-phase'), 'gsd-plan-phase must be in SPAWNER_GSD_NAMES — the prefixed form used in Skill() calls');
|
||||
});
|
||||
|
||||
for (const wf of allWorkflowFiles) {
|
||||
// Only scan files that have at least one Agent( call
|
||||
if (!wf.content.includes('Agent(')) continue;
|
||||
|
||||
test(`${wf.name}: no Agent() block wraps a spawner Skill without a RUNTIME carve-out`, () => {
|
||||
const blocks = extractAgentBlocks(wf.content);
|
||||
const violations = blocks.filter((b) => {
|
||||
const wrapsSpawner = spawnerPattern.test(b.blockText);
|
||||
if (!wrapsSpawner) return false;
|
||||
return !hasRuntimeCarveout(b);
|
||||
});
|
||||
|
||||
assert.deepStrictEqual(
|
||||
violations.map((v) => v.blockText.slice(0, 120).replace(/\n/g, '\\n')),
|
||||
[],
|
||||
`${wf.name} wraps a spawner Skill inside Agent() without a RUNTIME != claude carve-out.\n` +
|
||||
`Fix: run the spawner Skill inline (bare Skill() call at depth 0) OR add a RUNTIME gate.\n` +
|
||||
`See: bug #936, tests/bug-853-bg-dispatch-runtime-gating.test.cjs for the guarded pattern.`
|
||||
);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ── 5. Focused regression: plan-review-convergence never wraps plan-phase ─
|
||||
|
||||
describe('bug-936 — plan-review-convergence runs plan-phase inline, not inside Agent()', () => {
|
||||
const CONVERGENCE = fs.readFileSync(
|
||||
path.join(WORKFLOWS_DIR, 'plan-review-convergence.md'),
|
||||
'utf8'
|
||||
);
|
||||
|
||||
test('plan-review-convergence does NOT wrap gsd-plan-phase inside Agent()', () => {
|
||||
// The anti-pattern: Agent( block whose body contains Skill(skill='gsd-plan-phase')
|
||||
const blocks = extractAgentBlocks(CONVERGENCE);
|
||||
const wrapping = blocks.filter((b) =>
|
||||
/Skill\(\s*skill=['"]gsd-plan-phase['"]/.test(b.blockText) &&
|
||||
!hasRuntimeCarveout(b)
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
wrapping.map((v) => v.blockText.slice(0, 120).replace(/\n/g, '\\n')),
|
||||
[],
|
||||
'plan-review-convergence must NOT wrap gsd-plan-phase inside Agent(). ' +
|
||||
'Run it inline (bare Skill() at depth 0) so it can spawn gsd-planner/gsd-plan-checker. ' +
|
||||
'See: bug #936'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-review-convergence calls gsd-plan-phase inline (bare Skill call outside Agent block)', () => {
|
||||
// After the fix: at least one bare Skill(skill="gsd-plan-phase") must appear
|
||||
// outside any Agent( block — that is the inline call from the depth-0 orchestrator.
|
||||
const blocks = extractAgentBlocks(CONVERGENCE);
|
||||
// Remove all Agent block ranges from the text
|
||||
let masked = CONVERGENCE;
|
||||
// Work from end to start so offsets stay valid
|
||||
const sorted = [...blocks].sort((a, b) => b.start - a.start);
|
||||
for (const b of sorted) {
|
||||
masked = masked.slice(0, b.start) + ' '.repeat(b.end - b.start) + masked.slice(b.end);
|
||||
}
|
||||
const hasInlineCall = /Skill\(\s*skill=["']gsd-plan-phase["']/.test(masked);
|
||||
assert.ok(
|
||||
hasInlineCall,
|
||||
'plan-review-convergence must contain at least one bare Skill(skill="gsd-plan-phase") ' +
|
||||
'outside any Agent() block — this is the inline call that lets plan-phase spawn its sub-agents. ' +
|
||||
'See: bug #936'
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-review-convergence still wraps gsd-review inside Agent() (leaf — isolation is correct)', () => {
|
||||
// gsd-review is a leaf (shells out via Bash, no subagent_type) so the Agent wrap is fine and intentional.
|
||||
const blocks = extractAgentBlocks(CONVERGENCE);
|
||||
const reviewWrap = blocks.some((b) => /Skill\(\s*skill=['"]gsd-review['"]/.test(b.blockText));
|
||||
assert.ok(reviewWrap, 'gsd-review must still be wrapped in Agent() — it is a Bash leaf and isolation is intentional');
|
||||
});
|
||||
});
|
||||
@@ -332,10 +332,13 @@ describe('changeset cli extract: version-range changelog extraction (#3496)', ()
|
||||
test('F1: workflows/update.md contains concrete extract subcommand invocation', (_t) => {
|
||||
const workflowPath = path.join(ROOT, 'gsd-core', 'workflows', 'update.md');
|
||||
const workflowText = fs.readFileSync(workflowPath, 'utf8');
|
||||
// The invocation is: node "$GSD_DIR/gsd-core/scripts/changeset/cli.cjs" extract
|
||||
// so the literal substring is 'cli.cjs" extract' (quote between script path and subcommand)
|
||||
// The invocation uses either a direct path or an intermediate variable:
|
||||
// node "$GSD_DIR/scripts/changeset/cli.cjs" extract
|
||||
// node "$GSD_CHANGESET_CLI" extract
|
||||
// Accept either form so future refactors don't immediately trip this anchor.
|
||||
assert.ok(
|
||||
workflowText.includes('cli.cjs" extract') || workflowText.includes('cli.cjs extract'),
|
||||
workflowText.includes('cli.cjs" extract') || workflowText.includes('cli.cjs extract') ||
|
||||
(workflowText.includes('GSD_CHANGESET_CLI') && workflowText.includes('" extract')),
|
||||
'update.md must invoke cli.cjs extract (fix for #3496 BLOCKER 1)',
|
||||
);
|
||||
assert.ok(
|
||||
@@ -351,6 +354,40 @@ describe('changeset cli extract: version-range changelog extraction (#3496)', ()
|
||||
'update.md must capture exit code or JSON output from extract',
|
||||
);
|
||||
});
|
||||
|
||||
// F2: update.md must use the INSTALLED path ($GSD_DIR/scripts/changeset/cli.cjs),
|
||||
// NOT the old broken path ($GSD_DIR/gsd-core/scripts/changeset/cli.cjs).
|
||||
// The installer copies scripts/changeset/ into <configDir>/scripts/changeset/,
|
||||
// so the runtime path is $GSD_DIR/scripts/changeset/cli.cjs (#935).
|
||||
// allow-test-rule: reads a product workflow .md file (not CJS source) to verify
|
||||
// the runtime install path contract; there is no behavioural runtime to invoke.
|
||||
test('F2: update.md CLI path is $GSD_DIR/scripts/changeset/cli.cjs (not gsd-core/scripts/…) (#935)', (_t) => {
|
||||
const workflowPath = path.join(ROOT, 'gsd-core', 'workflows', 'update.md');
|
||||
const workflowText = fs.readFileSync(workflowPath, 'utf8');
|
||||
// The correct installed path must appear somewhere in the update workflow
|
||||
assert.ok(
|
||||
workflowText.includes('scripts/changeset/cli.cjs'),
|
||||
'update.md must reference scripts/changeset/cli.cjs',
|
||||
);
|
||||
// The old broken path ($GSD_DIR/gsd-core/scripts/changeset/cli.cjs) must not appear
|
||||
assert.ok(
|
||||
!workflowText.includes('gsd-core/scripts/changeset/cli.cjs'),
|
||||
'update.md must NOT reference the old gsd-core/scripts/changeset/cli.cjs path (fix for #935)',
|
||||
);
|
||||
});
|
||||
|
||||
// F3: update.md must guard against the CLI being missing (not pure silent-swallow)
|
||||
// allow-test-rule: reads a product workflow .md file (not CJS source) to verify
|
||||
// the guard is present; there is no behavioural runtime to invoke.
|
||||
test('F3: update.md has an explicit guard when changeset CLI is missing (#935)', (_t) => {
|
||||
const workflowPath = path.join(ROOT, 'gsd-core', 'workflows', 'update.md');
|
||||
const workflowText = fs.readFileSync(workflowPath, 'utf8');
|
||||
// The workflow must check for CLI existence before invoking it
|
||||
assert.ok(
|
||||
workflowText.includes('GSD_CHANGESET_CLI') && workflowText.includes('! -f'),
|
||||
'update.md must guard against a missing changeset CLI with [ ! -f "$GSD_CHANGESET_CLI" ] (#935)',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('changeset cli render: file-I/O wrapper (#2975)', () => {
|
||||
|
||||
@@ -684,3 +684,126 @@ describe('Kilo source integration assertions', () => {
|
||||
assert.ok(updateContextSrc.includes('KILO_CONFIG'));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Section N: changeset CLI install regression (#935) ──────────────────────
|
||||
|
||||
describe('install — changeset CLI lands at scripts/changeset/cli.cjs (#935)', () => {
|
||||
// Regression guard: the changeset CLI must be copied into the runtime config dir
|
||||
// by the installer so $GSD_DIR/scripts/changeset/cli.cjs resolves at runtime.
|
||||
// Before this fix, scripts/ was never copied and /gsd-update changelog preview
|
||||
// silently failed on every real install.
|
||||
let tmpDir;
|
||||
let previousCwd;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempDir('gsd-changeset-install-');
|
||||
previousCwd = process.cwd();
|
||||
process.chdir(tmpDir);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.chdir(previousCwd);
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('install() copies scripts/changeset/cli.cjs to <configDir>/scripts/changeset/cli.cjs', () => {
|
||||
install(false, 'claude');
|
||||
const claudeDir = path.join(tmpDir, '.claude');
|
||||
const cliPath = path.join(claudeDir, 'scripts', 'changeset', 'cli.cjs');
|
||||
assert.ok(
|
||||
fs.existsSync(cliPath),
|
||||
`scripts/changeset/cli.cjs must exist at ${path.relative(tmpDir, cliPath)} after install (#935)`,
|
||||
);
|
||||
});
|
||||
|
||||
test('install() copies scripts/lib/cli-exit.cjs to <configDir>/scripts/lib/cli-exit.cjs', () => {
|
||||
install(false, 'claude');
|
||||
const claudeDir = path.join(tmpDir, '.claude');
|
||||
const cliExitPath = path.join(claudeDir, 'scripts', 'lib', 'cli-exit.cjs');
|
||||
assert.ok(
|
||||
fs.existsSync(cliExitPath),
|
||||
`scripts/lib/cli-exit.cjs must exist at ${path.relative(tmpDir, cliExitPath)} after install (#935)`,
|
||||
);
|
||||
});
|
||||
|
||||
test('installed cli.cjs executes without module-resolution errors', () => {
|
||||
// Smoke test: node can load the installed changeset CLI without crashing.
|
||||
// This catches path mismatches in require('../lib/cli-exit.cjs') etc.
|
||||
install(false, 'claude');
|
||||
const claudeDir = path.join(tmpDir, '.claude');
|
||||
const cliPath = path.join(claudeDir, 'scripts', 'changeset', 'cli.cjs');
|
||||
const { spawnSync } = require('node:child_process');
|
||||
const result = spawnSync(process.execPath, [cliPath, '--help'], { encoding: 'utf8' });
|
||||
// --help exits with code 1 (usage shown), but must NOT throw a MODULE_NOT_FOUND error
|
||||
assert.ok(
|
||||
!result.stderr.includes('MODULE_NOT_FOUND'),
|
||||
`cli.cjs must not produce MODULE_NOT_FOUND errors; stderr=${result.stderr}`,
|
||||
);
|
||||
assert.ok(
|
||||
!result.stderr.includes('Cannot find module'),
|
||||
`cli.cjs must resolve all modules; stderr=${result.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('installed cli.cjs can run extract subcommand end-to-end (#935)', () => {
|
||||
// Integration smoke test: the installed CLI's extract path (invoked by update.md)
|
||||
// must actually work — this catches require() path issues that --help wouldn't surface.
|
||||
install(false, 'claude');
|
||||
const claudeDir = path.join(tmpDir, '.claude');
|
||||
const cliPath = path.join(claudeDir, 'scripts', 'changeset', 'cli.cjs');
|
||||
// Use the CHANGELOG.md that was installed into gsd-core/ (installed by the installer)
|
||||
const changelogPath = path.join(claudeDir, 'gsd-core', 'CHANGELOG.md');
|
||||
assert.ok(fs.existsSync(changelogPath), 'CHANGELOG.md must be installed under gsd-core/');
|
||||
const { spawnSync } = require('node:child_process');
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[cliPath, 'extract', '--from', '0.0.0', '--to', '9999.0.0', '--changelog', changelogPath, '--json'],
|
||||
{ encoding: 'utf8' },
|
||||
);
|
||||
// extract must NOT throw a MODULE_NOT_FOUND or Cannot find module error
|
||||
assert.ok(
|
||||
!result.stderr.includes('MODULE_NOT_FOUND') && !result.stderr.includes('Cannot find module'),
|
||||
`installed cli.cjs extract must resolve all modules; stderr=${result.stderr}`,
|
||||
);
|
||||
// extract exit code 0 (found entries) or 2 (no entries in range) are both valid;
|
||||
// any other exit code is an error
|
||||
assert.ok(
|
||||
result.status === 0 || result.status === 2,
|
||||
`installed cli.cjs extract must exit 0 or 2; got ${result.status}; stderr=${result.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('writeManifest() tracks scripts/changeset/ and scripts/lib/ files', () => {
|
||||
install(false, 'claude');
|
||||
const claudeDir = path.join(tmpDir, '.claude');
|
||||
const manifest = writeManifest(claudeDir, 'claude');
|
||||
const changesetKeys = Object.keys(manifest.files).filter(k => k.startsWith('scripts/changeset/'));
|
||||
const libKeys = Object.keys(manifest.files).filter(k => k.startsWith('scripts/lib/'));
|
||||
assert.ok(changesetKeys.length > 0, 'manifest must track scripts/changeset/ files');
|
||||
assert.ok(libKeys.length > 0, 'manifest must track scripts/lib/ files');
|
||||
assert.ok(
|
||||
changesetKeys.includes('scripts/changeset/cli.cjs'),
|
||||
'manifest must include scripts/changeset/cli.cjs',
|
||||
);
|
||||
assert.ok(
|
||||
libKeys.includes('scripts/lib/cli-exit.cjs'),
|
||||
'manifest must include scripts/lib/cli-exit.cjs',
|
||||
);
|
||||
});
|
||||
|
||||
test('uninstall() removes scripts/changeset/ and scripts/lib/', () => {
|
||||
install(false, 'claude');
|
||||
const claudeDir = path.join(tmpDir, '.claude');
|
||||
assert.ok(fs.existsSync(path.join(claudeDir, 'scripts', 'changeset', 'cli.cjs')),
|
||||
'pre-condition: cli.cjs must be installed before uninstall');
|
||||
uninstall(false, 'claude');
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(claudeDir, 'scripts', 'changeset')),
|
||||
'scripts/changeset/ must be removed on uninstall',
|
||||
);
|
||||
assert.ok(
|
||||
!fs.existsSync(path.join(claudeDir, 'scripts', 'lib')),
|
||||
'scripts/lib/ must be removed on uninstall',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
280
tests/installer-migration-prune-stale-pristine.test.cjs
Normal file
280
tests/installer-migration-prune-stale-pristine.test.cjs
Normal file
@@ -0,0 +1,280 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* TDD tests for installer migration 004:
|
||||
* 2026-06-09-prune-stale-pristine-get-shit-done // gsd-allow-legacy-name
|
||||
*
|
||||
* Verifies plan() logic for:
|
||||
* 1. Stale pristine subdir absent -> empty plan (idempotency)
|
||||
* 2. Stale pristine subdir present -> remove-managed actions emitted for each file
|
||||
* 3. Stale pristine root is a symlink -> empty plan (symlink safety)
|
||||
* 4. Symlinked entry inside stale pristine dir is NOT emitted
|
||||
* 5. Mixed files: all get remove-managed (no user-file classification needed)
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
|
||||
// Load compiled module (build:lib compiles src/*.cts -> gsd-core/bin/lib/*.cjs)
|
||||
const migration = require('../gsd-core/bin/lib/installer-migrations/004-prune-stale-pristine-snapshots.cjs');
|
||||
|
||||
function createTempDir() {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-migration-004-test-'));
|
||||
}
|
||||
|
||||
function cleanup(dir) {
|
||||
// eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup in migration test; no helpers import available
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
function writeFile(root, relPath, content) {
|
||||
const fullPath = path.join(root, relPath);
|
||||
fs.mkdirSync(path.dirname(fullPath), { recursive: true });
|
||||
fs.writeFileSync(fullPath, content, 'utf8');
|
||||
}
|
||||
|
||||
function writeManifest(root, files) {
|
||||
fs.writeFileSync(
|
||||
path.join(root, 'gsd-file-manifest.json'),
|
||||
JSON.stringify({
|
||||
version: '1.3.0',
|
||||
timestamp: '2026-06-01T00:00:00.000Z',
|
||||
mode: 'full',
|
||||
files,
|
||||
}, null, 2),
|
||||
'utf8'
|
||||
);
|
||||
}
|
||||
|
||||
// Build a plan context using the real installer-migrations classifyArtifact.
|
||||
const {
|
||||
classifyArtifact: realClassifyArtifact,
|
||||
readInstallManifest,
|
||||
} = require('../gsd-core/bin/lib/installer-migrations.cjs');
|
||||
|
||||
function makePlanCtx(configDir) {
|
||||
const manifest = readInstallManifest(configDir);
|
||||
return {
|
||||
configDir,
|
||||
classifyArtifact: (relPath) => realClassifyArtifact(configDir, relPath, manifest),
|
||||
};
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Metadata
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('migration 004 metadata', () => {
|
||||
test('exports a single migration object with required fields', () => {
|
||||
assert.equal(typeof migration, 'object');
|
||||
assert.equal(typeof migration.id, 'string');
|
||||
assert.ok(migration.id.length > 0, 'id must be non-empty');
|
||||
assert.equal(typeof migration.title, 'string');
|
||||
assert.equal(typeof migration.description, 'string');
|
||||
assert.equal(typeof migration.introducedIn, 'string');
|
||||
assert.ok(Array.isArray(migration.scopes), 'scopes must be an array');
|
||||
assert.ok(migration.scopes.includes('global'), 'scopes must include global');
|
||||
assert.ok(migration.scopes.includes('local'), 'scopes must include local');
|
||||
assert.strictEqual(migration.destructive, true);
|
||||
assert.equal(typeof migration.plan, 'function');
|
||||
});
|
||||
|
||||
test('id contains expected date prefix', () => {
|
||||
assert.ok(migration.id.startsWith('2026-06-09-'), `id should start with date prefix, got: ${migration.id}`);
|
||||
});
|
||||
|
||||
test('id references prune-stale-pristine', () => {
|
||||
assert.ok(
|
||||
migration.id.includes('prune-stale-pristine') || migration.id.includes('pristine'),
|
||||
`id should reference pristine pruning, got: ${migration.id}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Case 1: stale pristine subdir absent -> empty plan
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('plan() — stale pristine subdir absent', () => {
|
||||
test('returns empty array when gsd-pristine/get-shit-done/ does not exist', () => { // gsd-allow-legacy-name
|
||||
const configDir = createTempDir();
|
||||
try {
|
||||
// Only gsd-pristine/gsd-core/ exists — no legacy subdir.
|
||||
writeFile(configDir, 'gsd-pristine/gsd-core/workflows/plan.md', 'pristine snapshot\n');
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
assert.deepEqual(actions, []);
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('returns empty array when gsd-pristine/ does not exist at all', () => {
|
||||
const configDir = createTempDir();
|
||||
try {
|
||||
writeManifest(configDir, {});
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
assert.deepEqual(actions, []);
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Case 2: stale pristine subdir present -> remove-managed for each file
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('plan() — stale pristine files present', () => {
|
||||
test('emits remove-managed for each file under gsd-pristine/get-shit-done/', () => { // gsd-allow-legacy-name
|
||||
const configDir = createTempDir();
|
||||
try {
|
||||
writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', 'old pristine\n'); // gsd-allow-legacy-name
|
||||
writeFile(configDir, 'gsd-pristine/get-shit-done/skills/gsd-foo/SKILL.md', 'old skill\n'); // gsd-allow-legacy-name
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
assert.equal(actions.length, 2, `expected 2 actions, got ${actions.length}`);
|
||||
for (const action of actions) {
|
||||
assert.equal(action.type, 'remove-managed', `expected remove-managed, got ${action.type}`);
|
||||
assert.ok(
|
||||
action.relPath.replace(/\\/g, '/').startsWith('gsd-pristine/get-shit-done/'), // gsd-allow-legacy-name
|
||||
`relPath should start with gsd-pristine/get-shit-done/, got: ${action.relPath}`, // gsd-allow-legacy-name
|
||||
);
|
||||
assert.equal(typeof action.reason, 'string');
|
||||
assert.ok(action.reason.length > 0, 'reason must not be empty');
|
||||
assert.equal(typeof action.ownershipEvidence, 'string');
|
||||
assert.ok(action.ownershipEvidence.length > 0, 'ownershipEvidence must not be empty');
|
||||
}
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('emits exactly one remove-managed per file (correct relPaths)', () => {
|
||||
const configDir = createTempDir();
|
||||
try {
|
||||
writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/execute-phase.md', 'pristine\n'); // gsd-allow-legacy-name
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
assert.equal(actions.length, 1);
|
||||
const relPathNorm = actions[0].relPath.replace(/\\/g, '/');
|
||||
assert.equal(relPathNorm, 'gsd-pristine/get-shit-done/workflows/execute-phase.md'); // gsd-allow-legacy-name
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('actions include classification override to managed-pristine', () => {
|
||||
const configDir = createTempDir();
|
||||
try {
|
||||
writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', 'pristine snapshot\n'); // gsd-allow-legacy-name
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
assert.equal(actions.length, 1);
|
||||
// The action must carry classification:'managed-pristine' so the framework
|
||||
// does not downgrade remove-managed to preserve-user (the file is not in
|
||||
// the manifest so classify() would return 'unknown').
|
||||
assert.equal(actions[0].classification, 'managed-pristine',
|
||||
'action must carry classification:managed-pristine override');
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Case 3: stale pristine root is a symlink -> plan returns [] (symlink safety)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('plan() — stale pristine root is a symlink', () => {
|
||||
test('returns empty array when gsd-pristine/get-shit-done/ is a symlink', () => { // gsd-allow-legacy-name
|
||||
const configDir = createTempDir();
|
||||
const externalDir = createTempDir();
|
||||
try {
|
||||
writeFile(externalDir, 'workflows/plan.md', 'pristine content\n');
|
||||
// Create gsd-pristine/ as a real dir but make get-shit-done/ a symlink. // gsd-allow-legacy-name
|
||||
fs.mkdirSync(path.join(configDir, 'gsd-pristine'), { recursive: true });
|
||||
const legacyLink = path.join(configDir, 'gsd-pristine', 'get-shit-done'); // gsd-allow-legacy-name
|
||||
fs.symlinkSync(externalDir, legacyLink);
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
assert.deepEqual(actions, [], 'plan() must return [] when stale pristine root is a symlink');
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
cleanup(externalDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Case 4: symlinked entry inside stale pristine dir is skipped
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('plan() — symlinked entry inside stale pristine dir is skipped', () => {
|
||||
test('symlinked file inside stale pristine dir is not included in plan actions', () => {
|
||||
const configDir = createTempDir();
|
||||
const externalTarget = createTempDir();
|
||||
try {
|
||||
// A real file inside gsd-pristine/get-shit-done/ // gsd-allow-legacy-name
|
||||
writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', 'real pristine\n'); // gsd-allow-legacy-name
|
||||
|
||||
// A symlink inside the same dir pointing to external target.
|
||||
const externalFile = path.join(externalTarget, 'external.md');
|
||||
fs.writeFileSync(externalFile, 'external content\n', 'utf8');
|
||||
const symlinkPath = path.join(configDir, 'gsd-pristine', 'get-shit-done', 'workflows', 'symlinked.md'); // gsd-allow-legacy-name
|
||||
fs.symlinkSync(externalFile, symlinkPath);
|
||||
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const actions = migration.plan(makePlanCtx(configDir));
|
||||
|
||||
// Only the real file should appear; the symlinked entry must be skipped.
|
||||
assert.equal(actions.length, 1, `expected 1 action (real file only), got ${actions.length}`);
|
||||
const hasSymlinked = actions.some((a) => a.relPath.includes('symlinked'));
|
||||
assert.equal(hasSymlinked, false, 'symlinked entry must not appear in plan actions');
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
cleanup(externalTarget);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Integration: plan goes through planInstallerMigrations + applyInstallerMigrationPlan
|
||||
// Files are actually removed from disk.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('plan() — integration: stale pristine files are removed', () => {
|
||||
test('stale gsd-pristine/get-shit-done/ file is removed after apply', () => { // gsd-allow-legacy-name
|
||||
const configDir = createTempDir();
|
||||
try {
|
||||
const fileContent = 'old pristine snapshot content written by gsd installer\n';
|
||||
writeFile(configDir, 'gsd-pristine/get-shit-done/workflows/plan.md', fileContent); // gsd-allow-legacy-name
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const { planInstallerMigrations, applyInstallerMigrationPlan } = require('../gsd-core/bin/lib/installer-migrations.cjs');
|
||||
const plan = planInstallerMigrations({
|
||||
configDir,
|
||||
migrations: [migration],
|
||||
scope: 'global',
|
||||
});
|
||||
|
||||
assert.equal(plan.blocked.length, 0, `expected no blocked actions; got ${JSON.stringify(plan.blocked)}`);
|
||||
applyInstallerMigrationPlan({ configDir, plan });
|
||||
|
||||
// File must be gone after apply.
|
||||
const stillThere = fs.existsSync(path.join(configDir, 'gsd-pristine', 'get-shit-done', 'workflows', 'plan.md')); // gsd-allow-legacy-name
|
||||
assert.equal(stillThere, false, 'stale pristine file must be removed after apply');
|
||||
} finally {
|
||||
cleanup(configDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1465,6 +1465,9 @@ test('shipped installer-migration checksums are locked to a committed baseline (
|
||||
'sha256:5ce55294aa02f25758f604a569c899a6d2d060299189f5f447f68d8033157058',
|
||||
'2026-06-02-rename-get-shit-done-to-gsd-core':
|
||||
'sha256:3a9f1d97f64097fb313203d19c6d93a187a38df61dd299afa5eef73e16124e95',
|
||||
// Migration 004: prune stale gsd-pristine/get-shit-done/ snapshots (#934) // gsd-allow-legacy-name
|
||||
'2026-06-09-prune-stale-pristine-get-shit-done': // gsd-allow-legacy-name
|
||||
'sha256:6555dd044659276fbc204e81793cd92c5315d54e7316bcdd82d2c98d15a7e9e8',
|
||||
};
|
||||
|
||||
const { DEFAULT_MIGRATIONS_DIR, migrationChecksum: computeChecksum } = require('../gsd-core/bin/lib/installer-migrations.cjs');
|
||||
|
||||
@@ -186,10 +186,10 @@ describe('plan-review-convergence workflow: initial planning gate (#2306)', () =
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow spawns isolated planning agent when no plans exist', () => {
|
||||
test('workflow runs gsd-plan-phase when no plans exist', () => {
|
||||
assert.ok(
|
||||
workflow.includes('gsd-plan-phase'),
|
||||
'workflow must spawn Agent → gsd-plan-phase when no plans exist'
|
||||
'workflow must invoke gsd-plan-phase when no plans exist'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -651,3 +651,95 @@ describe('plan-review-convergence local model CONFIGURATION.md documentation (#2
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Bug #936: plan-phase must run inline, not inside Agent() ─────────────
|
||||
//
|
||||
// Regression guard: inverted from the pre-#936 behavior that locked in the bug.
|
||||
// On Claude Code a depth-1 Agent has no Agent tool, so gsd-plan-phase wrapped in
|
||||
// Agent() cannot spawn gsd-planner / gsd-plan-checker → the replan loop breaks.
|
||||
// Fix: run plan-phase inline (bare Skill()) from the depth-0 convergence orchestrator.
|
||||
//
|
||||
// These tests FAIL on pre-fix code and PASS after the fix.
|
||||
|
||||
describe('plan-review-convergence workflow: inline plan-phase dispatch (#936)', () => {
|
||||
const workflow = fs.readFileSync(WORKFLOW_PATH, 'utf8');
|
||||
|
||||
// Helper: extract Agent() block bodies from workflow text
|
||||
function extractAgentBlocks(content) {
|
||||
const blocks = [];
|
||||
let pos = 0;
|
||||
while (pos < content.length) {
|
||||
const start = content.indexOf('Agent(', pos);
|
||||
if (start === -1) break;
|
||||
let depth = 0;
|
||||
let i = start + 'Agent('.length - 1;
|
||||
for (; i < content.length; i++) {
|
||||
if (content[i] === '(') depth++;
|
||||
else if (content[i] === ')') { depth--; if (depth === 0) break; }
|
||||
}
|
||||
blocks.push({ start, end: i + 1, blockText: content.slice(start, i + 1) });
|
||||
pos = i + 1;
|
||||
}
|
||||
return blocks;
|
||||
}
|
||||
|
||||
test('initial planning does NOT wrap gsd-plan-phase inside Agent() (#936 fix)', () => {
|
||||
// Pre-fix: Agent( ... Skill('gsd-plan-phase') ... ) in step 4
|
||||
// Post-fix: bare Skill(skill="gsd-plan-phase") at orchestrator level
|
||||
const blocks = extractAgentBlocks(workflow);
|
||||
const wrapping = blocks.filter((b) =>
|
||||
/Skill\(\s*skill=['"]gsd-plan-phase['"]/.test(b.blockText)
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
wrapping.map((b) => b.blockText.slice(0, 80).replace(/\n/g, '\\n')),
|
||||
[],
|
||||
'Initial planning must NOT wrap gsd-plan-phase inside Agent() — run it inline so ' +
|
||||
'it can spawn gsd-planner/gsd-plan-checker at depth 1. See: bug #936'
|
||||
);
|
||||
});
|
||||
|
||||
test('replan step does NOT wrap gsd-plan-phase inside Agent() (#936 fix)', () => {
|
||||
// Same check as above; explicitly named for the replan site (step 5d)
|
||||
const blocks = extractAgentBlocks(workflow);
|
||||
const wrapping = blocks.filter((b) =>
|
||||
/Skill\(\s*skill=['"]gsd-plan-phase['"]/.test(b.blockText) &&
|
||||
/--reviews/.test(b.blockText)
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
wrapping.map((b) => b.blockText.slice(0, 80).replace(/\n/g, '\\n')),
|
||||
[],
|
||||
'Replan step must NOT wrap gsd-plan-phase inside Agent() — the replan loop can ' +
|
||||
'never produce a plan on Claude Code when plan-phase is at depth 1. See: bug #936'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow calls gsd-plan-phase inline (bare Skill outside Agent block) (#936 fix)', () => {
|
||||
// After the fix there must be at least one bare Skill(skill="gsd-plan-phase")
|
||||
// OUTSIDE any Agent() block.
|
||||
const blocks = extractAgentBlocks(workflow);
|
||||
let masked = workflow;
|
||||
const sorted = [...blocks].sort((a, b) => b.start - a.start);
|
||||
for (const b of sorted) {
|
||||
masked = masked.slice(0, b.start) + ' '.repeat(b.end - b.start) + masked.slice(b.end);
|
||||
}
|
||||
assert.ok(
|
||||
/Skill\(\s*skill=["']gsd-plan-phase["']/.test(masked),
|
||||
'plan-review-convergence must contain at least one bare Skill(skill="gsd-plan-phase") ' +
|
||||
'outside any Agent() block — the inline call that preserves depth-0 Agent availability. See: bug #936'
|
||||
);
|
||||
});
|
||||
|
||||
test('success_criteria describes inline plan-phase, not Agent → Skill (#936 fix)', () => {
|
||||
const successBlock = workflow.slice(workflow.lastIndexOf('<success_criteria>'));
|
||||
// The broken criterion said "Initial planning via Agent → Skill"
|
||||
assert.ok(
|
||||
!successBlock.includes('via Agent → Skill("gsd-plan-phase")'),
|
||||
'success_criteria must NOT describe plan-phase as Agent → Skill — that was the broken pattern. See: bug #936'
|
||||
);
|
||||
// The broken criterion said "isolated, not inline" for the replan
|
||||
assert.ok(
|
||||
!successBlock.includes('isolated, not inline'),
|
||||
'success_criteria must NOT say "isolated, not inline" for plan-phase — the fix makes it inline. See: bug #936'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user