diff --git a/tests/fix-2337-add-todo-severity.test.cjs b/tests/add-todo.test.cjs similarity index 100% rename from tests/fix-2337-add-todo-severity.test.cjs rename to tests/add-todo.test.cjs diff --git a/tests/fix-2136-clock-local-today.test.cjs b/tests/clock.test.cjs similarity index 100% rename from tests/fix-2136-clock-local-today.test.cjs rename to tests/clock.test.cjs diff --git a/tests/fix-1941-quick-worktree-stale-base.test.cjs b/tests/fix-1941-quick-worktree-stale-base.test.cjs deleted file mode 100644 index 614711b1b..000000000 --- a/tests/fix-1941-quick-worktree-stale-base.test.cjs +++ /dev/null @@ -1,91 +0,0 @@ -// allow-test-rule: source-text-is-the-product #1941 -// Workflow .md files are the installed AI instructions — their text IS what the runtime -// loads. Testing text content tests the deployed contract. Per CONTRIBUTING.md exception matrix. - -/** - * Regression tests for bug #1941: /gsd-quick worktree executor forks from a stale base — - * up to many commits behind, not just the one-commit gap #1265 already covers. - * - * Root cause: Claude Code's isolation="worktree" forks new worktrees from origin/HEAD, not - * the live local HEAD. When prior local commits (e.g. earlier quick tasks in the same - * session, or this task's own Step 5.6 pre-dispatch plan commit) advance local HEAD without - * an intervening `git push`, origin/HEAD stays pinned to a stale ancestor and the executor's - * worktree_branch_check guard halts with a base-mismatch fatal. The fix ports the - * worktree.base-check auto-degrade pattern already used by execute-phase (#683/#1369) into - * quick.md's single-dispatch path. - */ - -const { test, describe } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -const WORKFLOW_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'quick.md'); - -describe('quick: pre-dispatch worktree base re-check (#1941)', () => { - test('workflow file exists', () => { - assert.ok(fs.existsSync(WORKFLOW_PATH), 'workflows/quick.md should exist'); - }); - - test('Step 6 runs worktree.base-check before capturing EXPECTED_BASE', () => { - const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); - const step6Idx = content.indexOf('**Step 6: Spawn executor**'); - const baseCheckIdx = content.indexOf('worktree.base-check', step6Idx); - const expectedBaseIdx = content.indexOf('EXPECTED_BASE=$(git rev-parse HEAD)', step6Idx); - assert.ok(step6Idx !== -1, '"Step 6: Spawn executor" must exist in quick.md'); - assert.ok(baseCheckIdx !== -1, 'worktree.base-check must be invoked within Step 6'); - assert.ok(expectedBaseIdx !== -1, 'EXPECTED_BASE capture must exist within Step 6'); - assert.ok( - baseCheckIdx < expectedBaseIdx, - 'worktree.base-check must run BEFORE EXPECTED_BASE is captured so the degrade decision reflects the most current local HEAD' - ); - }); - - test('degrade check references #1941 for traceability', () => { - const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); - assert.ok(content.includes('#1941'), 'quick.md must reference #1941'); - }); - - test('degrade check clears BOTH USE_WORKTREES and ISOLATION when shouldDegrade is true', () => { - const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); - const baseCheckIdx = content.indexOf('worktree.base-check'); - const block = content.slice(baseCheckIdx, baseCheckIdx + 900); - assert.ok(block.includes('shouldDegrade'), 'degrade check must branch on shouldDegrade'); - // Both must move together (#2652). Dispatch keys on ISOLATION while the prompt - // guard and worktree manifest key on USE_WORKTREES; clearing only one dispatches - // an isolated executor with no base guard and no manifest, then blocks in cleanup - // looking for a manifest that was never initialized. - assert.ok(block.includes('USE_WORKTREES=false'), 'degrade must set USE_WORKTREES=false'); - assert.ok( - block.includes('ISOLATION=none'), - 'degrade must ALSO set ISOLATION=none — dispatch reads ISOLATION, so clearing only ' + - 'USE_WORKTREES still passes the harness isolation flag (#2652)' - ); - }); - - // #2652: this assertion previously required `RUNTIME = "claude"`, encoding the - // pre-#2584 premise that worktree isolation is Claude-specific. #2584 replaced - // that with the negotiated dispatch.isolation capability, so the guard now keys - // on the capability — Cursor also declares harness-worktree. - test('degrade check guards on the negotiated capability, not a runtime id', () => { - const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); - const baseCheckIdx = content.indexOf('worktree.base-check'); - const block = content.slice(Math.max(0, baseCheckIdx - 300), baseCheckIdx + 200); - assert.ok( - block.includes('ISOLATION') && block.includes('harness-worktree'), - 'degrade check must guard on ISOLATION = harness-worktree' - ); - assert.ok( - !/\[\s*"\$RUNTIME"\s*=/.test(block), - 'degrade check must NOT branch on a RUNTIME literal (#2584/#2652)' - ); - }); - - test('degrade check names origin/HEAD as the stale fork base', () => { - const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); - const step6Idx = content.indexOf('**Step 6: Spawn executor**'); - const nextSection = content.indexOf('\n---', step6Idx); - const section = content.slice(step6Idx, nextSection === -1 ? undefined : nextSection); - assert.ok(section.includes('origin/HEAD'), 'Step 6 must name origin/HEAD as the stale fork base'); - }); -}); diff --git a/tests/fix-2297-resolve-model-ids-runtime-scoping.test.cjs b/tests/fix-2297-resolve-model-ids-runtime-scoping.test.cjs deleted file mode 100644 index b50fd6e5a..000000000 --- a/tests/fix-2297-resolve-model-ids-runtime-scoping.test.cjs +++ /dev/null @@ -1,411 +0,0 @@ -/** - * Bug #2297 — `resolve_model_ids:"omit"` must be scoped to the ACTIVE runtime, - * not applied blindly whenever it appears anywhere in the merged config. - * - * Root cause (pre-fix): the installer writes `resolve_model_ids:"omit"` into the - * SHARED `~/.gsd/defaults.json` for every runtime that lacks native model - * aliases (#1156). Because that file is machine-wide, installing a non-Claude - * runtime (e.g. codex) on a box that also runs Claude poisoned Claude's - * no-project resolution: Claude would see `resolve_model_ids:"omit"` in the - * merged global defaults and return `''` instead of its tier aliases - * (opus/sonnet/haiku), silently defeating Claude's adaptive tier distinction. - * - * Fix (`src/model-resolver.cts` `resolveModelInternal`): the `"omit"` branch - * now returns `''` ONLY when either - * (a) the PROJECT's own `.planning/config.json` explicitly sets - * `resolve_model_ids:"omit"` (user intent — #2517 finding #4, unchanged), OR - * (b) the ACTIVE runtime genuinely lacks native model aliases. - * A native-alias runtime (currently only `claude`) IGNORES an `"omit"` that - * came solely from the shared global defaults and falls through to its tier - * aliases. Active-runtime precedence: `process.env.GSD_RUNTIME` -> `config.runtime` - * -> per-install `.gsd-runtime` marker (absent in this dev/test tree, so the - * chain always bottoms out at `'claude'`) -> `'claude'` (all canonicalized). - * - * IMPORTANT (empirically verified — see dispatch report): the global-defaults - * merge path in `config-loader.cjs` (branch D: "no .planning/ at all") is ONLY - * exercised when the project directory has NO `.planning/` directory whatsoever. - * The moment a `.planning/` directory exists — even with an empty or absent - * `config.json` inside it — the loader takes a different branch that does NOT - * merge `~/.gsd/defaults.json` for these fields at all. So Group A below - * (which specifically exercises the global-defaults poisoning fix) uses BARE - * `fs.mkdtempSync` project dirs with no `.planning/` subdirectory. Group A #4, - * Group B, and Group C all need a real per-project config, so those DO create - * `.planning/config.json`. - */ - -'use strict'; - -const { describe, test, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const os = require('os'); -const path = require('path'); - -const { - resolveModelInternal, - _setInstallRuntimeMarkerForTests, - _resetInstallRuntimeMarkerCacheForTests, -} = require('../gsd-core/bin/lib/model-resolver.cjs'); - -// ─── HOME / GSD_HOME / GSD_RUNTIME isolation ──────────────────────────────── -// config-loader.cjs reads global defaults from -// path.join(process.env.GSD_HOME || os.homedir(), '.gsd', 'defaults.json'). -// Isolate both HOME and GSD_HOME to a fresh tmpdir per test (so a developer's -// real ~/.gsd/defaults.json never bleeds into assertions), and save/restore -// GSD_RUNTIME since several tests set it directly to drive the active-runtime -// chain (#2297's second precedence rung). Also save/restore GSD_WORKSTREAM and -// GSD_PROJECT (#2297 correctness-review hermeticity gap): planningDir() reads -// both directly from process.env when its ws/project params are omitted, so an -// ambient GSD_WORKSTREAM/GSD_PROJECT in a developer's shell could silently -// redirect projectExplicitlySetsOmit()'s config-file reads to the wrong layer. -let _origHome; -let _origUserProfile; -let _origGsdHome; -let _origGsdRuntime; -let _origGsdWorkstream; -let _origGsdProject; -let _isolatedHome; - -function isolateHome() { - _origHome = process.env.HOME; - _origUserProfile = process.env.USERPROFILE; - _origGsdHome = process.env.GSD_HOME; - _origGsdRuntime = process.env.GSD_RUNTIME; - _origGsdWorkstream = process.env.GSD_WORKSTREAM; - _origGsdProject = process.env.GSD_PROJECT; - _isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-home-')); - process.env.HOME = _isolatedHome; - // Windows resolves the home dir from USERPROFILE, not HOME — set both so the - // isolation holds cross-platform (local/require-userprofile-with-home). - process.env.USERPROFILE = _isolatedHome; - process.env.GSD_HOME = _isolatedHome; - delete process.env.GSD_RUNTIME; - delete process.env.GSD_WORKSTREAM; - delete process.env.GSD_PROJECT; -} - -function restoreHome() { - if (_origHome === undefined) delete process.env.HOME; else process.env.HOME = _origHome; - if (_origUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = _origUserProfile; - if (_origGsdHome === undefined) delete process.env.GSD_HOME; else process.env.GSD_HOME = _origGsdHome; - if (_origGsdRuntime === undefined) delete process.env.GSD_RUNTIME; else process.env.GSD_RUNTIME = _origGsdRuntime; - if (_origGsdWorkstream === undefined) delete process.env.GSD_WORKSTREAM; else process.env.GSD_WORKSTREAM = _origGsdWorkstream; - if (_origGsdProject === undefined) delete process.env.GSD_PROJECT; else process.env.GSD_PROJECT = _origGsdProject; - rmDir(_isolatedHome); - _isolatedHome = null; -} - -function rmDir(dir) { - if (typeof dir !== 'string' || dir.length === 0) return; - // eslint-disable-next-line local/no-raw-rmsync-in-tests -- carries the same maxRetries/retryDelay budget as helpers.cleanup; used for both the isolated-home and bare project temp dirs - fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); -} - -function writeGlobalDefaults(obj) { - fs.mkdirSync(path.join(_isolatedHome, '.gsd'), { recursive: true }); - fs.writeFileSync(path.join(_isolatedHome, '.gsd', 'defaults.json'), JSON.stringify(obj, null, 2)); -} - -// Bare project dir with NO .planning/ subdirectory — needed to exercise the -// config-loader's global-defaults merge branch (see file header). -function mkProjNoPlanning() { - return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-proj-noplan-')); -} - -// Project dir WITH a .planning/config.json — the normal "inside a project" path. -function mkProjWithConfig(obj) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-proj-')); - fs.mkdirSync(path.join(dir, '.planning'), { recursive: true }); - fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify(obj, null, 2)); - return dir; -} - -// ─── Group A: GLOBAL-defaults "omit" is runtime-scoped (the #2297 fix) ───── -describe('#2297: global-defaults resolve_model_ids:"omit" is scoped to the active runtime', () => { - let projDir; - beforeEach(() => { isolateHome(); projDir = null; }); - afterEach(() => { rmDir(projDir); restoreHome(); }); - - test('no runtime signal defaults to claude: executor and planner get distinct non-empty tier aliases (acceptance #3)', () => { - // Global defaults poison the shared file with "omit" (simulating a - // non-Claude runtime having been installed on this machine). With no - // .planning/config.json (no project) and no GSD_RUNTIME, the active - // runtime falls back to 'claude', which has native aliases and must - // ignore the poisoned global "omit" — the adaptive tier distinction - // between executor (sonnet) and planner (opus) must survive. - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - - const executor = resolveModelInternal(projDir, 'gsd-executor'); - const planner = resolveModelInternal(projDir, 'gsd-planner'); - - assert.strictEqual(executor, 'sonnet'); - assert.strictEqual(planner, 'opus'); - assert.notStrictEqual(executor, ''); - assert.notStrictEqual(planner, ''); - assert.notStrictEqual(executor, planner); - }); - - test('GSD_RUNTIME="claude" explicitly: executor still resolves to "sonnet" (claude ignores global omit)', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - process.env.GSD_RUNTIME = 'claude'; - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); - }); - - test('GSD_RUNTIME="codex": a non-alias runtime still honors the global omit (acceptance #4)', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - process.env.GSD_RUNTIME = 'codex'; - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), ''); - }); - - // #2297 correctness-review BLOCKER: resolveActiveRuntime() must canonicalize - // its candidates via resolveRuntimeNameFromCandidates before checking - // RUNTIMES_WITH_NATIVE_ALIASES, or an alias/case variant of "claude" would - // fail the Set('claude').has() check and wrongly fall through to honoring the - // poisoned global omit. These would FAIL against a non-canonicalizing resolver. - test('GSD_RUNTIME="claude-code" (alias, not canonical "claude"): executor and planner still ignore the global omit', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - process.env.GSD_RUNTIME = 'claude-code'; - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); - assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), 'opus'); - }); - - test('GSD_RUNTIME="Claude" (case variant): executor still resolves to "sonnet" (canonicalization is case-insensitive)', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - process.env.GSD_RUNTIME = 'Claude'; - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); - }); - - test('project config.runtime="codex" (no resolve_model_ids in project) takes precedence over GSD_RUNTIME/marker in the active-runtime chain', () => { - // config.runtime is checked before GSD_RUNTIME / the install marker. This - // scenario uses a REAL project (.planning/config.json present), so the - // config-loader does NOT merge ~/.gsd/defaults.json for resolve_model_ids - // at all here (see file header) — resolution instead reaches the #2517 - // runtime-tier path (step 3 in resolveModelInternal, which fires before - // the omit gate) and returns codex's native sonnet-tier model id directly, - // rather than the omit gate's ''. Verified empirically: the built resolver - // returns 'gpt-5.6-terra', not ''. Assert it is non-empty and NOT a claude - // alias, which is the property this test actually needs to guarantee - // (config.runtime, not GSD_RUNTIME/env, drove the resolution). - projDir = mkProjWithConfig({ runtime: 'codex' }); - writeGlobalDefaults({ resolve_model_ids: 'omit' }); // irrelevant: not merged when .planning/ exists - - const result = resolveModelInternal(projDir, 'gsd-executor'); - assert.notStrictEqual(result, ''); - assert.ok( - !['sonnet', 'opus', 'haiku'].includes(result), - `expected a non-claude-alias result for config.runtime="codex", got ${JSON.stringify(result)}` - ); - }); - - test('install-order independence (acceptance #1/#2): a global omit poisoned by a prior non-Claude install does not affect Claude resolution, and Claude retains its adaptive tier distinction', () => { - // Resolution depends on the RESOLVING runtime (active runtime at call - // time), not on install order — installing codex (or any non-alias - // runtime) before/after Claude must never change what Claude itself - // resolves to. Global omit present, no project, no runtime signal -> - // default 'claude' -> tier aliases survive. Distinct from the first Group A - // test above: this asserts install-order independence AND, specifically, - // that executor/planner remain DIFFERENT tiers under the poisoned global - // omit — i.e. install order never collapses Claude's adaptive tier - // distinction into a single omitted value. - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - - const executor = resolveModelInternal(projDir, 'gsd-executor'); - const planner = resolveModelInternal(projDir, 'gsd-planner'); - - assert.strictEqual(executor, 'sonnet'); - assert.strictEqual(planner, 'opus'); - assert.notStrictEqual(executor, planner, 'install-order poisoning must not collapse the adaptive tier distinction'); - }); -}); - -// ─── Group B: explicit PROJECT "omit" is still honored for EVERY runtime ─── -// (#2517 finding #4 — preserved, NOT changed by #2297.) -describe('#2297: explicit project-level resolve_model_ids:"omit" is honored regardless of runtime', () => { - let projDir; - beforeEach(() => { isolateHome(); projDir = null; }); - afterEach(() => { rmDir(projDir); restoreHome(); }); - - test('no runtime set, explicit project omit -> "" even though the default runtime is claude', () => { - projDir = mkProjWithConfig({ resolve_model_ids: 'omit' }); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), ''); - }); - - test('runtime:"claude" + explicit project omit -> "" (mirrors #2517 finding #4)', () => { - projDir = mkProjWithConfig({ runtime: 'claude', resolve_model_ids: 'omit' }); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), ''); - }); -}); - -// ─── Group B2: projectExplicitlySetsOmit is workstream-scope aware (#2297) ── -// The root .planning/config.json does NOT set resolve_model_ids, but the -// ACTIVE workstream's own config.json does — projectExplicitlySetsOmit() -// resolves via planningDir(cwd) (workstream layer wins over root, mirroring -// loadConfig's precedence), so the workstream's explicit "omit" must still be -// honored even though no global default and the default runtime (claude) would -// otherwise have returned a tier alias. -describe('#2297: explicit project-level "omit" is honored at the active-workstream config layer', () => { - let projDir; - let _origGsdWorkstreamForBlock; - beforeEach(() => { - isolateHome(); // clears GSD_WORKSTREAM/GSD_PROJECT as part of hermeticity - projDir = null; - _origGsdWorkstreamForBlock = process.env.GSD_WORKSTREAM; - }); - afterEach(() => { - if (_origGsdWorkstreamForBlock === undefined) delete process.env.GSD_WORKSTREAM; - else process.env.GSD_WORKSTREAM = _origGsdWorkstreamForBlock; - rmDir(projDir); - restoreHome(); - }); - - test('root config has no resolve_model_ids, but the active workstream config sets "omit" -> "" despite default runtime claude', () => { - const ws = 'ws-alpha'; - projDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-proj-ws-')); - fs.mkdirSync(path.join(projDir, '.planning'), { recursive: true }); - // Root config exists but does NOT set resolve_model_ids at all. - fs.writeFileSync( - path.join(projDir, '.planning', 'config.json'), - JSON.stringify({ model_profile: 'balanced' }, null, 2) - ); - // The active workstream's own config explicitly sets "omit". - const wsConfigDir = path.join(projDir, '.planning', 'workstreams', ws); - fs.mkdirSync(wsConfigDir, { recursive: true }); - fs.writeFileSync( - path.join(wsConfigDir, 'config.json'), - JSON.stringify({ resolve_model_ids: 'omit' }, null, 2) - ); - process.env.GSD_WORKSTREAM = ws; - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), ''); - }); -}); - -// ─── Group C: explicit `true` still materializes full model ids (acceptance #5) ── -describe('#2297: resolve_model_ids:true still materializes full Claude model ids', () => { - let projDir; - beforeEach(() => { isolateHome(); projDir = null; }); - afterEach(() => { rmDir(projDir); restoreHome(); }); - - test('resolve_model_ids:true + balanced profile -> full materialized claude-opus-4-8 id', () => { - projDir = mkProjWithConfig({ resolve_model_ids: true, model_profile: 'balanced' }); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), 'claude-opus-4-8'); - }); -}); - -// ─── Group D: registry parity guard ───────────────────────────────────────── -describe('#2297: capability-registry nativeModelAliases parity guard', () => { - test('exactly the runtimes with hostBehaviors.nativeModelAliases:true match RUNTIMES_WITH_NATIVE_ALIASES ([\'claude\'])', () => { - // The model-resolver hardcodes RUNTIMES_WITH_NATIVE_ALIASES = new Set(['claude']) - // rather than reading the registry at runtime. This test keeps that - // hardcoded set honest against the generated registry's actual contract: - // registry.runtimes[id].runtime.hostBehaviors.nativeModelAliases. - // If a future runtime gains nativeModelAliases:true, this fails loudly so - // RUNTIMES_WITH_NATIVE_ALIASES in model-resolver.cts is updated in lockstep. - const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); - - const nativeAliasRuntimes = Object.keys(registry.runtimes) - .filter((id) => registry.runtimes[id]?.runtime?.hostBehaviors?.nativeModelAliases === true) - .sort(); - - assert.deepStrictEqual(nativeAliasRuntimes, ['claude']); - }); -}); - -// ─── Group E: installer writes the per-install .gsd-runtime marker ───────── -describe('#2297: installer emits the gsd-core/.gsd-runtime marker (fixture parity)', () => { - test('claude and codex install-tree fixtures both list gsd-core/.gsd-runtime', () => { - // These fixtures are flat JSON arrays of install-relative paths, generated - // by running the real installer (tests/fixtures/install-tree/*.json). Their - // presence here proves the installer actually emits the per-install marker - // that resolveActiveRuntime()'s precedence chain falls back to. - const claudeFixturePath = path.join(__dirname, 'fixtures', 'install-tree', 'claude.json'); - const codexFixturePath = path.join(__dirname, 'fixtures', 'install-tree', 'codex.json'); - - const claudeFixture = JSON.parse(fs.readFileSync(claudeFixturePath, 'utf8')); - const codexFixture = JSON.parse(fs.readFileSync(codexFixturePath, 'utf8')); - - assert.ok(Array.isArray(claudeFixture), 'expected claude.json fixture to be a flat array of paths'); - assert.ok(Array.isArray(codexFixture), 'expected codex.json fixture to be a flat array of paths'); - - assert.ok( - claudeFixture.includes('gsd-core/.gsd-runtime'), - 'expected claude.json install-tree fixture to include gsd-core/.gsd-runtime' - ); - assert.ok( - codexFixture.includes('gsd-core/.gsd-runtime'), - 'expected codex.json install-tree fixture to include gsd-core/.gsd-runtime' - ); - }); -}); - -// ─── Group F: the install-marker precedence rung, driven directly (#2297) ── -// Previously untested: with no GSD_RUNTIME and no project config.runtime, the -// active runtime falls all the way through to the per-install .gsd-runtime -// marker (third precedence rung). The dev/source tree has no real marker file, -// so these tests drive that rung directly via the _setInstallRuntimeMarkerForTests -// / _resetInstallRuntimeMarkerCacheForTests seams exported specifically for this -// purpose (#2297 correctness-review gap). -describe('#2297: install-marker precedence rung (GSD_RUNTIME and config.runtime both absent)', () => { - let projDir; - beforeEach(() => { - isolateHome(); // also deletes GSD_RUNTIME - projDir = null; - // Belt-and-suspenders: the marker rung is only reached when GSD_RUNTIME and - // config.runtime are both absent; isolateHome() already deletes GSD_RUNTIME. - delete process.env.GSD_RUNTIME; - }); - afterEach(() => { - rmDir(projDir); - restoreHome(); - // CRITICAL: reset the module-level marker cache after every case in this - // block so a set value never leaks into a later case here, or into any - // OTHER describe block in this file (readInstallRuntimeMarker() otherwise - // memoizes the first value it sees for the lifetime of the process). - _resetInstallRuntimeMarkerCacheForTests(); - }); - - test('marker="codex" (non-alias runtime): honors the poisoned global omit -> ""', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - _setInstallRuntimeMarkerForTests('codex'); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), ''); - }); - - test('marker="claude": ignores the poisoned global omit -> "sonnet"', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - _setInstallRuntimeMarkerForTests('claude'); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); - }); - - test('marker="claude-code" (alias): canonicalized to "claude" and still ignores the poisoned global omit -> "sonnet"', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - _setInstallRuntimeMarkerForTests('claude-code'); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); - }); - - test('marker unset (null): falls through to the "claude" default and ignores the poisoned global omit -> "sonnet"', () => { - writeGlobalDefaults({ resolve_model_ids: 'omit' }); - projDir = mkProjNoPlanning(); - _setInstallRuntimeMarkerForTests(null); - - assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); - }); -}); diff --git a/tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs b/tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs deleted file mode 100644 index b7ef01f3e..000000000 --- a/tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs +++ /dev/null @@ -1,120 +0,0 @@ -// allow-test-rule: source-text-is-the-product #2649 -// Workflow .md files are the installed AI instructions — their text IS what the runtime -// loads. Testing text content tests the deployed contract. Per CONTRIBUTING.md exception -// matrix. Mirrors tests/fix-1941-quick-worktree-stale-base.test.cjs. - -/** - * Regression tests for bug #2649: /gsd-verify-work's UAT-gap diagnosis step - * (workflows/diagnose-issues.md) and execute-phase's single-plan interactive - * dispatch (workflows/execute-plan.md Pattern A) spawn worktree-isolated - * subagents without first checking whether the harness's worktree fork base has - * diverged from live local HEAD — unlike every other worktree-dispatch site. - * - * Root cause: Claude Code's isolation="worktree" forks new worktrees from - * origin/HEAD, not the live local HEAD. When local commits advance HEAD without - * an intervening `git push` (the documented GSD steady state), origin/HEAD is - * pinned to a stale ancestor and the subagent's worktree_branch_check guard - * halts with a base-mismatch fatal mid-investigation, with no auto-degrade. The - * fix ports the worktree.base-check auto-degrade pattern (execute-phase #683/ - * #1369, quick #1941) into these two not-yet-covered dispatch sites. - * - * The triage for #2649 found execute-plan.md's Pattern A has the identical gap; - * per the bug's acceptance criterion 5 it is fixed in the same change (same bug - * class, same one-line gate) rather than filed as a separate follow-up. - */ - -const { test, describe } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -const DIAGNOSE_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'diagnose-issues.md'); -const EXECUTE_PLAN_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-plan.md'); - -describe('diagnose-issues: pre-dispatch worktree base-check (#2649)', () => { - test('workflow file exists', () => { - assert.ok(fs.existsSync(DIAGNOSE_PATH), 'workflows/diagnose-issues.md should exist'); - }); - - test('spawn_agents step runs worktree.base-check before the Agent() dispatch', () => { - const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); - const spawnIdx = content.indexOf(''); - assert.ok(spawnIdx !== -1, '"spawn_agents" step must exist in diagnose-issues.md'); - const baseCheckIdx = content.indexOf('worktree.base-check', spawnIdx); - assert.ok(baseCheckIdx !== -1, 'worktree.base-check must be invoked within the spawn_agents step'); - // The load-bearing invariant is "base-check BEFORE the Agent() dispatch" so the - // degrade decision can drop isolation from the spawn. (Where EXPECTED_BASE is - // captured relative to the check is cosmetic — the check only reads HEAD, never - // mutates it — so assert the real invariant, not a loose disjunction.) - const agentIdx = content.indexOf('Agent(', spawnIdx); - assert.ok(agentIdx !== -1, 'spawn_agents must contain an Agent() dispatch'); - assert.ok( - baseCheckIdx < agentIdx, - 'worktree.base-check must run before the Agent() dispatch so the degrade decision can drop isolation from the spawn', - ); - }); - - test('verify-only worktree_branch_check backstop remains embedded in the Agent() prompt', () => { - // Acceptance criterion #4: the base-check is a PRE-DISPATCH degrade; the - // guard is a POST-FORK fail-closed backstop. Both - // layers must survive — a future edit that dropped the backstop embedding - // would re-open the silent-stale-base class. Guard its continued presence. - const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); - const spawnIdx = content.indexOf(''); - assert.ok(spawnIdx !== -1, '"spawn_agents" step must exist'); - assert.ok( - content.indexOf('worktree-branch-check.md', spawnIdx) !== -1, - 'spawn_agents must still materialize the backstop after the base-check gate (#2649 acceptance criterion 4)', - ); - }); - - test('degrade check sets USE_WORKTREES=false when shouldDegrade is true', () => { - const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); - const baseCheckIdx = content.indexOf('worktree.base-check'); - const block = content.slice(baseCheckIdx, baseCheckIdx + 600); - assert.ok( - block.includes('shouldDegrade') && block.includes('USE_WORKTREES=false'), - 'degrade check must override USE_WORKTREES=false when shouldDegrade is true', - ); - }); - - test('degrade check references #2649 for traceability', () => { - const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); - assert.ok(content.includes('#2649'), 'diagnose-issues.md must reference #2649'); - }); -}); - -describe('execute-plan Pattern A: pre-dispatch worktree base-check (#2649)', () => { - test('workflow file exists', () => { - assert.ok(fs.existsSync(EXECUTE_PLAN_PATH), 'workflows/execute-plan.md should exist'); - }); - - test('Pattern A runs the worktree base-check before spawning the executor', () => { - const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); - const patternAIdx = content.indexOf('**Pattern A:**'); - assert.ok(patternAIdx !== -1, '"Pattern A:" must exist in execute-plan.md'); - // The base-check instruction must appear within the Pattern A description, - // before the isolation="worktree" embedding instruction. - const patternAEnd = content.indexOf('**Pattern B:**', patternAIdx); - const patternA = content.slice(patternAIdx, patternAEnd === -1 ? undefined : patternAEnd); - assert.ok( - patternA.includes('#2649') && /worktree\.base-check|base-check/.test(patternA), - 'Pattern A must run the #2649 worktree base-check before dispatching the executor', - ); - assert.ok( - patternA.includes('shouldDegrade'), - 'Pattern A base-check must consult shouldDegrade', - ); - }); - - test('Pattern A documents the auto-degrade (drop isolation on shouldDegrade)', () => { - const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); - const patternAIdx = content.indexOf('**Pattern A:**'); - const patternAEnd = content.indexOf('**Pattern B:**', patternAIdx); - const patternA = content.slice(patternAIdx, patternAEnd === -1 ? undefined : patternAEnd); - assert.ok( - /degrad|sequential/i.test(patternA), - 'Pattern A must document auto-degrading to sequential mode when shouldDegrade is true', - ); - }); -}); diff --git a/tests/fix-2830-halted-plan-dependents.test.cjs b/tests/fix-2830-halted-plan-dependents.test.cjs deleted file mode 100644 index ae2c35454..000000000 --- a/tests/fix-2830-halted-plan-dependents.test.cjs +++ /dev/null @@ -1,764 +0,0 @@ -/** - * Regression tests for #2830: a halted plan leaves its dependents on the - * runnable work list. - * - * Two independent "which plans are incomplete" readers exist: - * - phase.cts's cmdPhasePlanIndex (`gsd-tools phase-plan-index`) — parses - * depends_on for wave assignment, but (pre-fix) never propagates a halt. - * - phase-locator.cts's searchPhaseInDir/findPhaseInternal — the - * phase-location primitive consumed by ~50 symbols across 5 command - * routers; (pre-fix) never parsed depends_on at all. - * - * Both must now report a direct or transitive dependent of a halted plan as - * blocked — never offered as ordinary runnable work — while leaving the - * pre-existing `incomplete`/`incomplete_plans` fields byte-identical. - * - * Every assertion below is behavioral (structured JSON from the real CLI / - * the real compiled module) — no source-grep or raw-text matching, so no - * `allow-test-rule` exemption is needed anywhere in this file. - */ - -'use strict'; - -const { test, describe, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); -const fc = require('./helpers/fast-check-setup.cjs'); - -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); - -const phaseLocator = require('../gsd-core/bin/lib/phase-locator.cjs'); -const planDependencyGraph = require('../gsd-core/bin/lib/plan-dependency-graph.cjs'); - -// ─── Fixture builder ────────────────────────────────────────────────────── -// -// Builds a phase directory with: -// 01-01 — halted spike (SUMMARY status: halted) -// 01-02 — depends_on 01-01 (direct dependent) -// 01-03 — depends_on 01-02 (transitive dependent, 2 hops) -// 01-04 — decoupled (no depends_on) — the negative case -// Callers can pass extra plan/summary writers for diamond/boundary variants. - -function writePlan(phaseDir, filename, frontmatterLines, taskLine = 'Work') { - fs.writeFileSync( - path.join(phaseDir, filename), - [ - '---', - ...frontmatterLines, - '---', - '', - `# ${filename}`, - '', - `${filename}`, - '', - taskLine, - ].join('\n'), - ); -} - -function writeSummary(phaseDir, filename, status = 'complete') { - fs.writeFileSync( - path.join(phaseDir, filename), - ['---', 'phase: 01-alpha', 'plan: 01', `status: ${status}`, 'completed: 2026-08-02', '---', '', '# Summary', ''].join('\n'), - ); -} - -function buildBaseFixture(tmpDir) { - const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '01-01-PLAN.md', ['wave: 1', 'objective: Halted spike', 'autonomous: true']); - writeSummary(phaseDir, '01-01-SUMMARY.md', 'halted'); - - writePlan(phaseDir, '01-02-PLAN.md', [ - 'wave: 2', 'objective: Direct dependent', 'autonomous: true', 'depends_on:', ' - 01-01', - ]); - - writePlan(phaseDir, '01-03-PLAN.md', [ - 'wave: 3', 'objective: Transitive dependent', 'autonomous: true', 'depends_on:', ' - 01-02', - ]); - - writePlan(phaseDir, '01-04-PLAN.md', ['wave: 1', 'objective: Decoupled plan', 'autonomous: true']); - - return phaseDir; -} - -// ─── phase-plan-index (cmdPhasePlanIndex, src/phase.cts) ────────────────── - -describe('phase-plan-index: halt propagation (#2830)', () => { - let tmpDir; - afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); - - test('direct dependent of a halted plan is blocked, not runnable', () => { - tmpDir = createTempProject('gsd-2830-'); - buildBaseFixture(tmpDir); - - const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); - assert.ok(result.success, `phase-plan-index should succeed: ${result.error}`); - const data = JSON.parse(result.output); - - const p02 = data.plans.find((p) => p.id === '01-02'); - assert.ok(p02, '01-02 should be present'); - assert.deepEqual(p02.blocked_by, ['01-01'], '01-02 should be blocked by the halted 01-01'); - assert.strictEqual(p02.has_summary, false); - assert.ok(!data.runnable.includes('01-02'), '01-02 must NOT be in the runnable view'); - }); - - test('transitive dependent (2 hops) is blocked via chain', () => { - tmpDir = createTempProject('gsd-2830-'); - buildBaseFixture(tmpDir); - - const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); - const data = JSON.parse(result.output); - - const p03 = data.plans.find((p) => p.id === '01-03'); - assert.ok(p03, '01-03 should be present'); - assert.deepEqual(p03.blocked_by, ['01-01'], '01-03 should be transitively blocked by 01-01'); - assert.ok(!data.runnable.includes('01-03'), '01-03 must NOT be in the runnable view'); - }); - - test('transitive dependent at 3 hops stays blocked', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = buildBaseFixture(tmpDir); - writePlan(phaseDir, '01-05-PLAN.md', [ - 'wave: 4', 'objective: 3-hop dependent', 'autonomous: true', 'depends_on:', ' - 01-03', - ]); - - const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); - const data = JSON.parse(result.output); - - const p05 = data.plans.find((p) => p.id === '01-05'); - assert.ok(p05, '01-05 should be present'); - assert.deepEqual(p05.blocked_by, ['01-01'], '01-05 should stay blocked at 3 hops'); - assert.ok(!data.runnable.includes('01-05')); - }); - - test('diamond dependency is blocked by both halted ancestors', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-diamond'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '02-01-PLAN.md', ['wave: 1', 'objective: Halted A', 'autonomous: true']); - writeSummary(phaseDir, '02-01-SUMMARY.md', 'halted'); - writePlan(phaseDir, '02-02-PLAN.md', ['wave: 1', 'objective: Halted B', 'autonomous: true']); - writeSummary(phaseDir, '02-02-SUMMARY.md', 'halted'); - writePlan(phaseDir, '02-03-PLAN.md', [ - 'wave: 2', 'objective: Diamond join', 'autonomous: true', - 'depends_on:', ' - 02-01', ' - 02-02', - ]); - - const result = runGsdTools(['phase-plan-index', '2', '--raw'], tmpDir); - assert.ok(result.success, `phase-plan-index should succeed: ${result.error}`); - const data = JSON.parse(result.output); - - const p03 = data.plans.find((p) => p.id === '02-03'); - assert.ok(p03, '02-03 should be present'); - assert.deepEqual( - [...p03.blocked_by].sort(), - ['02-01', '02-02'], - '02-03 should be blocked by BOTH halted ancestors, deduplicated', - ); - }); - - test('unrelated decoupled plan stays runnable', () => { - tmpDir = createTempProject('gsd-2830-'); - buildBaseFixture(tmpDir); - - const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); - const data = JSON.parse(result.output); - - const p04 = data.plans.find((p) => p.id === '01-04'); - assert.ok(p04, '01-04 should be present'); - assert.deepEqual(p04.blocked_by, [], '01-04 has no depends_on, so it must not be blocked'); - assert.ok(data.runnable.includes('01-04'), '01-04 (decoupled) must stay in the runnable view'); - }); - - test('incomplete field stays byte-identical when blocked plans are present', () => { - tmpDir = createTempProject('gsd-2830-'); - buildBaseFixture(tmpDir); - - const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); - const data = JSON.parse(result.output); - - // Pre-#2830 semantics: incomplete = every plan without a matching SUMMARY, - // blocked or not. 01-01 has a SUMMARY (halted, but still a SUMMARY) so it - // is excluded; 01-02/01-03/01-04 have none, so all three are included — - // exactly as they would be with no halt-awareness at all. - assert.deepEqual( - [...data.incomplete].sort(), - ['01-02', '01-03', '01-04'], - 'incomplete must list every no-SUMMARY plan regardless of blocked status', - ); - }); - - test('dependency on an ordinary incomplete (non-halted) plan is not "blocked"', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-ordinary'); - fs.mkdirSync(phaseDir, { recursive: true }); - - // 03-01 has NO summary at all (ordinary incomplete, not halted). - writePlan(phaseDir, '03-01-PLAN.md', ['wave: 1', 'objective: Ordinary unfinished plan', 'autonomous: true']); - writePlan(phaseDir, '03-02-PLAN.md', [ - 'wave: 2', 'objective: Depends on ordinary incomplete plan', 'autonomous: true', - 'depends_on:', ' - 03-01', - ]); - - const result = runGsdTools(['phase-plan-index', '3', '--raw'], tmpDir); - const data = JSON.parse(result.output); - - const p01 = data.plans.find((p) => p.id === '03-01'); - const p02 = data.plans.find((p) => p.id === '03-02'); - assert.deepEqual(p01.blocked_by, [], '03-01 (no summary) is not itself halted or blocked'); - assert.deepEqual(p02.blocked_by, [], '03-02 must NOT be "blocked" by an ordinary (non-halted) dependency'); - assert.ok(data.runnable.includes('03-02'), '03-02 stays runnable — only a halted upstream blocks'); - }); - - test('unresolved depends_on id is ignored, not blocked', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '04-unresolved'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '04-01-PLAN.md', [ - 'wave: 1', 'objective: References a nonexistent plan', 'autonomous: true', - 'depends_on:', ' - 99-99', - ]); - - const result = runGsdTools(['phase-plan-index', '4', '--raw'], tmpDir); - assert.ok(result.success, `phase-plan-index should not throw on an unresolved dependency: ${result.error}`); - const data = JSON.parse(result.output); - - const p01 = data.plans.find((p) => p.id === '04-01'); - assert.deepEqual(p01.blocked_by, [], 'an unresolved depends_on id must not produce a spurious block'); - assert.ok(data.runnable.includes('04-01')); - }); - - test('malformed (unterminated) SUMMARY frontmatter fails open to not-halted', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '05-malformed'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '05-01-PLAN.md', ['wave: 1', 'objective: Has a malformed summary', 'autonomous: true']); - writeSummary(phaseDir, '05-01-SUMMARY.md', 'halted'); - writePlan(phaseDir, '05-02-PLAN.md', [ - 'wave: 2', 'objective: Depends on 05-01', 'autonomous: true', 'depends_on:', ' - 05-01', - ]); - - // extractFrontmatter must fail safe on an unterminated frontmatter block - // (no closing '---'): isSummaryHalted's try/catch wraps BOTH the - // fs.readFileSync call and the extractFrontmatter call, so this exercises - // the identical fail-open catch site a genuine fs read error would hit — - // see the separate real-fs-fault-injection test below for the read-error - // half of that same catch block. - fs.writeFileSync( - path.join(phaseDir, '05-01-SUMMARY.md'), - '---\nphase: 05-malformed\nplan: 01\nstatus: halted\n', // no closing '---' - ); - - const result = runGsdTools(['phase-plan-index', '5', '--raw'], tmpDir); - assert.ok(result.success, `phase-plan-index must not throw on malformed frontmatter: ${result.error}`); - const data = JSON.parse(result.output); - - const p01 = data.plans.find((p) => p.id === '05-01'); - const p02 = data.plans.find((p) => p.id === '05-02'); - assert.strictEqual(p01.halted, false, 'unterminated frontmatter must fail open to not-halted'); - assert.deepEqual(p02.blocked_by, [], 'dependent of a fail-open-not-halted plan must not be blocked'); - }); - - test('CRLF SUMMARY frontmatter still detects status: halted', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '06-crlf'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '06-01-PLAN.md', ['wave: 1', 'objective: Halted with CRLF summary', 'autonomous: true']); - fs.writeFileSync( - path.join(phaseDir, '06-01-SUMMARY.md'), - ['---', 'phase: 06-crlf', 'plan: 01', 'status: halted', 'completed: 2026-08-02', '---', '', '# Summary', ''].join('\r\n'), - ); - writePlan(phaseDir, '06-02-PLAN.md', [ - 'wave: 2', 'objective: Depends on CRLF-summarized halt', 'autonomous: true', 'depends_on:', ' - 06-01', - ]); - - const result = runGsdTools(['phase-plan-index', '6', '--raw'], tmpDir); - assert.ok(result.success, `phase-plan-index should succeed on CRLF frontmatter: ${result.error}`); - const data = JSON.parse(result.output); - - const p01 = data.plans.find((p) => p.id === '06-01'); - const p02 = data.plans.find((p) => p.id === '06-02'); - assert.strictEqual(p01.halted, true, 'CRLF SUMMARY frontmatter must still parse status: halted'); - assert.deepEqual(p02.blocked_by, ['06-01'], 'dependent must be blocked even when the halt was recorded with CRLF newlines'); - }); - - test('status complete does not block dependents', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '07-complete'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '07-01-PLAN.md', ['wave: 1', 'objective: Ordinary completion', 'autonomous: true']); - writeSummary(phaseDir, '07-01-SUMMARY.md', 'complete'); - writePlan(phaseDir, '07-02-PLAN.md', [ - 'wave: 2', 'objective: Depends on completed plan', 'autonomous: true', 'depends_on:', ' - 07-01', - ]); - - const result = runGsdTools(['phase-plan-index', '7', '--raw'], tmpDir); - const data = JSON.parse(result.output); - - const p01 = data.plans.find((p) => p.id === '07-01'); - const p02 = data.plans.find((p) => p.id === '07-02'); - assert.strictEqual(p01.halted, false); - assert.deepEqual(p02.blocked_by, []); - assert.ok(data.runnable.includes('07-02')); - }); - - test('dependency cycle detection is unaffected by halt propagation', () => { - tmpDir = createTempProject('gsd-2830-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '08-cycle'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '08-01-PLAN.md', [ - 'wave: 1', 'objective: Cycle A', 'autonomous: true', 'depends_on:', ' - 08-02', - ]); - writePlan(phaseDir, '08-02-PLAN.md', [ - 'wave: 1', 'objective: Cycle B', 'autonomous: true', 'depends_on:', ' - 08-01', - ]); - - const result = runGsdTools(['phase-plan-index', '8', '--raw'], tmpDir); - // CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs" bans - // regex-matching a child process's human-readable stderr/reason prose — - // assert on the typed failure signal (`result.success`) instead. To keep - // this test specific to the CYCLE (not "failed for any reason"), pair it - // with a differential fixture: the identical dependency shape with the - // cycle edge removed must succeed, isolating the cycle as the cause of - // the failure above without parsing error prose. - assert.strictEqual(result.success, false, 'a dependency cycle must still fail the command'); - - const acyclicDir = path.join(tmpDir, '.planning', 'phases', '09-nocycle'); - fs.mkdirSync(acyclicDir, { recursive: true }); - writePlan(acyclicDir, '09-01-PLAN.md', ['wave: 1', 'objective: No cycle A', 'autonomous: true']); - writePlan(acyclicDir, '09-02-PLAN.md', [ - 'wave: 1', 'objective: No cycle B', 'autonomous: true', 'depends_on:', ' - 09-01', - ]); - const acyclicResult = runGsdTools(['phase-plan-index', '9', '--raw'], tmpDir); - assert.strictEqual( - acyclicResult.success, - true, - `the identical dependency shape without the cycle edge must succeed, isolating the cycle as the cause of the failure above: ${acyclicResult.error}`, - ); - }); -}); - -// ─── findPhaseInternal / searchPhaseInDir (src/phase-locator.cts) ───────── - -describe('findPhaseInternal: halt propagation (#2830)', () => { - let tmpDir; - afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); - - test('direct dependent of a halted plan is blocked', () => { - tmpDir = createTempProject('gsd-2830-pl-'); - buildBaseFixture(tmpDir); - - const result = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.ok(result, 'expected a result'); - assert.deepEqual(result.blocked_by['01-02-PLAN.md'], ['01-01'], '01-02 should be blocked by halted 01-01'); - assert.ok(!result.runnable_plans.includes('01-02-PLAN.md')); - }); - - test('transitive dependent is blocked via chain', () => { - tmpDir = createTempProject('gsd-2830-pl-'); - buildBaseFixture(tmpDir); - - const result = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.deepEqual(result.blocked_by['01-03-PLAN.md'], ['01-01'], '01-03 should be transitively blocked'); - assert.ok(!result.runnable_plans.includes('01-03-PLAN.md')); - }); - - test('diamond dependency blocked by both halted ancestors', () => { - tmpDir = createTempProject('gsd-2830-pl-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-diamond'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '02-01-PLAN.md', ['wave: 1', 'objective: Halted A', 'autonomous: true']); - writeSummary(phaseDir, '02-01-SUMMARY.md', 'halted'); - writePlan(phaseDir, '02-02-PLAN.md', ['wave: 1', 'objective: Halted B', 'autonomous: true']); - writeSummary(phaseDir, '02-02-SUMMARY.md', 'halted'); - writePlan(phaseDir, '02-03-PLAN.md', [ - 'wave: 2', 'objective: Diamond join', 'autonomous: true', - 'depends_on:', ' - 02-01', ' - 02-02', - ]); - - const result = phaseLocator.findPhaseInternal(tmpDir, '2'); - assert.ok(result, 'expected a result'); - assert.deepEqual( - [...result.blocked_by['02-03-PLAN.md']].sort(), - ['02-01', '02-02'], - 'diamond join should be blocked by both halted ancestors, deduplicated', - ); - }); - - test('unrelated decoupled plan stays runnable', () => { - tmpDir = createTempProject('gsd-2830-pl-'); - buildBaseFixture(tmpDir); - - const result = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.ok(result.runnable_plans.includes('01-04-PLAN.md'), '01-04 (decoupled) must stay runnable'); - assert.strictEqual(result.blocked_by['01-04-PLAN.md'], undefined); - }); - - test('incomplete_plans stays byte-identical when blocked plans are present', () => { - tmpDir = createTempProject('gsd-2830-pl-'); - buildBaseFixture(tmpDir); - - const result = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.deepEqual( - [...result.incomplete_plans].sort(), - ['01-02-PLAN.md', '01-03-PLAN.md', '01-04-PLAN.md'], - 'incomplete_plans must list every no-SUMMARY plan regardless of blocked status', - ); - }); - - test('halted_plans reports the halted plan itself by filename', () => { - tmpDir = createTempProject('gsd-2830-pl-'); - buildBaseFixture(tmpDir); - - const result = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.deepEqual(result.halted_plans, ['01-01-PLAN.md']); - }); -}); - -// ─── Parity: the two implementations must agree ─────────────────────────── - -describe('parity: phase-plan-index and findPhaseInternal agree on blocking (#2830)', () => { - let tmpDir; - afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); - - test('same fixture yields the same blocked-plan set and cause set from both readers', () => { - tmpDir = createTempProject('gsd-2830-parity-'); - buildBaseFixture(tmpDir); - - const cliResult = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); - assert.ok(cliResult.success, `phase-plan-index should succeed: ${cliResult.error}`); - const cliData = JSON.parse(cliResult.output); - - const locatorResult = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.ok(locatorResult, 'findPhaseInternal should return a result'); - - // Build { planId -> sorted cause list } from each reader and require them - // to be structurally identical (id-mapped, since one reader keys by bare - // id and the other by filename). - const cliBlocked = {}; - for (const plan of cliData.plans) { - if (plan.blocked_by.length > 0) cliBlocked[plan.id] = [...plan.blocked_by].sort(); - } - - const locatorBlocked = {}; - for (const [filename, causes] of Object.entries(locatorResult.blocked_by)) { - const planId = filename.replace(/-PLAN\.md$/i, '').replace(/^PLAN\.md$/i, ''); - locatorBlocked[planId] = [...causes].sort(); - } - - assert.deepEqual( - locatorBlocked, - cliBlocked, - 'phase-plan-index and findPhaseInternal must report the exact same blocked-plan-id -> cause-set mapping', - ); - }); -}); - -// ─── computeHaltPropagation — direct module tests + fast-check property ─── - -describe('computeHaltPropagation: graph invariants (#2830)', () => { - test('a node with no depends_on and not halted is never blocked', () => { - const { blockedBy } = planDependencyGraph.computeHaltPropagation([ - { id: 'A', resolvedDependsOn: [], halted: false }, - ]); - assert.strictEqual(blockedBy.get('A'), undefined); - }); - - test('a halted node is never blocked by itself', () => { - const { blockedBy } = planDependencyGraph.computeHaltPropagation([ - { id: 'A', resolvedDependsOn: [], halted: true }, - ]); - assert.strictEqual(blockedBy.get('A'), undefined, 'a halted plan is halted, not "blocked"'); - }); - - test('precomputedOrder (the phase.cts call shape) yields the same result as self-derived order', () => { - // phase.cts's cmdPhasePlanIndex passes computeDependencyLevels's own - // topological order in as `precomputedOrder` so this function does not - // re-run Kahn's algorithm. Assert both call shapes agree. - const nodes = [ - { id: 'A', resolvedDependsOn: [], halted: true }, - { id: 'B', resolvedDependsOn: ['A'], halted: false }, - { id: 'C', resolvedDependsOn: ['B'], halted: false }, - ]; - const selfDerived = planDependencyGraph.computeHaltPropagation(nodes); - const withPrecomputed = planDependencyGraph.computeHaltPropagation(nodes, ['A', 'B', 'C']); - assert.deepEqual([...withPrecomputed.blockedBy.entries()], [...selfDerived.blockedBy.entries()]); - assert.deepEqual(withPrecomputed.order, ['A', 'B', 'C']); - assert.strictEqual(withPrecomputed.visited, 3); - }); - - // Generate a random DAG over N nodes: edges only point from a lower index - // to a higher index (guarantees acyclicity by construction, independent of - // the code under test), with a random halted flag per node. - // - // Acyclic BY CONSTRUCTION: `to` is always drawn strictly above `from`, so - // no `.filter()` is involved. A filter here is not merely slower — with - // n === 1 the predicate `from < to` is unsatisfiable and fast-check retries - // generation forever, which hung the whole suite (the runner sets - // --test-timeout=0, so it never dies). Hoisted to describe scope so the - // regression test below can sample the identical arbitrary. - const dagArb = fc.integer({ min: 1, max: 12 }).chain((n) => { - const ids = Array.from({ length: n }, (_, i) => `N${i}`); - const edgeArb = n < 2 - ? fc.constant([]) - : fc.array( - fc.integer({ min: 0, max: n - 2 }).chain((from) => - fc.integer({ min: from + 1, max: n - 1 }).map((to) => ({ from, to }))), - { maxLength: n * 2 }, - ); - const haltedArb = fc.array(fc.boolean(), { minLength: n, maxLength: n }); - return fc.record({ ids: fc.constant(ids), edges: edgeArb, halted: haltedArb }); - }); - - test('fast-check — blocked set matches reachability from halted nodes', () => { - fc.assert( - fc.property(dagArb, ({ ids, edges, halted }) => { - const dependsOn = new Map(ids.map((id) => [id, []])); - for (const { from, to } of edges) { - dependsOn.get(ids[from]).push(ids[to]); - } - const nodes = ids.map((id, i) => ({ - id, - resolvedDependsOn: dependsOn.get(id), - halted: halted[i], - })); - - const { blockedBy } = planDependencyGraph.computeHaltPropagation(nodes); - - // Reference model: reachability via depends_on edges from a halted node. - const haltedSet = new Set(nodes.filter((n) => n.halted).map((n) => n.id)); - const dependsOnMap = new Map(nodes.map((n) => [n.id, n.resolvedDependsOn])); - function reachableHaltedCauses(id, seen = new Set()) { - const causes = new Set(); - for (const dep of dependsOnMap.get(id) ?? []) { - if (seen.has(dep)) continue; - seen.add(dep); - if (haltedSet.has(dep)) causes.add(dep); - for (const c of reachableHaltedCauses(dep, seen)) causes.add(c); - } - return causes; - } - - for (const n of nodes) { - const expected = [...reachableHaltedCauses(n.id)].sort(); - const actual = [...(blockedBy.get(n.id) ?? [])].sort(); - assert.deepEqual( - actual, - expected, - `node ${n.id}: computeHaltPropagation blockedBy must equal halted-reachability`, - ); - } - }), - { numRuns: 50 }, - ); - }); - - test('the DAG generator terminates on the degenerate single-node case (regression: unsatisfiable filter hung the suite)', () => { - // Bounded, non-hanging sample: if edgeArb regresses to a `.filter(from < to)` - // over a forced-equal {from, to} pair (n === 1), fast-check would retry - // generation forever and this assertion would never run. A small, - // explicit numRuns/seed keeps the check itself deterministic and fast. - const samples = fc.sample(dagArb, { numRuns: 20, seed: 7 }); - assert.ok(samples.length === 20, 'fc.sample must return the requested number of samples without hanging'); - const singleNodeSamples = samples.filter(({ ids }) => ids.length === 1); - assert.ok(singleNodeSamples.length > 0, 'the sample must include at least one degenerate single-node case'); - for (const { edges } of singleNodeSamples) { - assert.deepEqual(edges, [], 'the single-node case must yield an empty edge list, not an unsatisfiable filter'); - } - }); -}); - -// ─── init execute-phase (cmdInitExecutePhase, src/init.cts) ─────────────── -// -// #2830 names `init execute-phase` as the exact regressed consumer: the -// locator already computed halted_plans/blocked_by/runnable_plans, but the -// command built its output by explicit field enumeration, silently dropping -// all three. This drives the real CLI end to end (not the locator directly) -// to prove the passthrough, modeled on the "init execute-phase JSON output" -// fixture shape in tests/tdd-mode.test.cjs (ROADMAP.md + a phase directory -// resolvable by number). - -describe('init execute-phase: halt propagation passthrough (#2830)', () => { - let tmpDir; - afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); - - test('halted_plans, blocked_by, runnable_plans are forwarded; incomplete fields stay byte-identical', () => { - tmpDir = createTempProject('gsd-2830-init-'); - buildBaseFixture(tmpDir); - - const result = runGsdTools(['init', 'execute-phase', '1', '--raw'], tmpDir); - assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); - const json = JSON.parse(result.output); - - // Guard: if the phase was not resolved, every assertion below is vacuous. - assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); - - assert.ok( - json.halted_plans.includes('01-01-PLAN.md'), - 'halted_plans must include the halted plan', - ); - - assert.ok( - Object.prototype.hasOwnProperty.call(json.blocked_by, '01-02-PLAN.md'), - 'blocked_by must have an entry for the direct dependent', - ); - assert.deepEqual( - json.blocked_by['01-02-PLAN.md'], - ['01-01'], - "blocked_by['01-02-PLAN.md'] must name 01-01 as the blocking cause", - ); - - assert.ok( - !json.runnable_plans.includes('01-02-PLAN.md'), - 'the dependent must NOT be offered as runnable work', - ); - assert.ok( - json.runnable_plans.includes('01-04-PLAN.md'), - 'the decoupled plan (no dependency on the halted plan) must stay runnable', - ); - - // Back-compat: incomplete_plans/incomplete_count must be exactly what they - // were pre-#2830 — every no-SUMMARY plan, blocked or not, still counted. - assert.deepEqual( - [...json.incomplete_plans].sort(), - ['01-02-PLAN.md', '01-03-PLAN.md', '01-04-PLAN.md'], - 'incomplete_plans must be unchanged by halt-awareness', - ); - assert.strictEqual( - json.incomplete_count, - 3, - 'incomplete_count must be unchanged by halt-awareness', - ); - }); -}); - -// ─── #2830 review findings ───────────────────────────────────────────────── -// -// Adversarial review of the #2830 fix found two confirmed defects: -// 1. (BLOCKER) computeHaltPropagation's Kahn pass silently drops cycle -// participants (and anything downstream of them) from BOTH `order` and -// `blockedBy` — phase.cts hard-fails on a cycle before this ever -// matters, but phase-locator.cts (consumed by `init execute-phase`) -// does not pre-check, so a plan directly depends_on-ing a halted plan -// inside a cycle was reported as ordinary runnable. -// 2. (MAJOR) the summary templates presented `status: halted` as a -// trailing `#`-comment on the value line, but `extractFrontmatter` -// does not strip trailing YAML comments — an executor mimicking the -// template's own presentation wrote a halt that silently read back as -// not-halted. - -describe('#2830 review findings', () => { - let tmpDir; - afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); - - test('defect 1: cycle repro — 08-02 depends_on [halted 08-01, 08-03]; 08-03 depends_on [08-02]', () => { - tmpDir = createTempProject('gsd-2830-review-cycle-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '08-cyclehalt'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '08-01-PLAN.md', ['wave: 1', 'objective: Halted upstream', 'autonomous: true']); - writeSummary(phaseDir, '08-01-SUMMARY.md', 'halted'); - writePlan(phaseDir, '08-02-PLAN.md', [ - 'wave: 2', 'objective: Depends on halted + cyclic peer', 'autonomous: true', - 'depends_on:', ' - 08-01', ' - 08-03', - ]); - writePlan(phaseDir, '08-03-PLAN.md', [ - 'wave: 2', 'objective: Cyclic peer of 08-02', 'autonomous: true', 'depends_on:', ' - 08-02', - ]); - - const result = runGsdTools(['init', 'execute-phase', '8', '--raw'], tmpDir); - assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); - const json = JSON.parse(result.output); - assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); - - assert.ok( - !json.runnable_plans.includes('08-02-PLAN.md'), - 'a plan that directly depends_on a halted plan must never be offered as runnable, cycle or not', - ); - assert.ok( - Object.prototype.hasOwnProperty.call(json.blocked_by, '08-02-PLAN.md'), - 'a plan must never silently vanish from both runnable_plans and blocked_by', - ); - assert.ok( - Array.isArray(json.blocked_by['08-02-PLAN.md']) && json.blocked_by['08-02-PLAN.md'].length > 0, - 'blocked_by entry must be non-empty, not a vacuous placeholder', - ); - }); - - test('defect 1: self-dependency (A depends_on A, nothing halted) must not be silently runnable', () => { - tmpDir = createTempProject('gsd-2830-review-self-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '09-selfdep'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '09-01-PLAN.md', [ - 'wave: 1', 'objective: Depends on itself', 'autonomous: true', 'depends_on:', ' - 09-01', - ]); - - const result = runGsdTools(['init', 'execute-phase', '9', '--raw'], tmpDir); - assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); - const json = JSON.parse(result.output); - assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); - - assert.ok( - !json.runnable_plans.includes('09-01-PLAN.md'), - 'a self-dependent plan must not be silently offered as runnable', - ); - assert.ok( - Object.prototype.hasOwnProperty.call(json.blocked_by, '09-01-PLAN.md'), - 'a self-dependent plan must never silently vanish from both runnable_plans and blocked_by', - ); - }); - - test('defect 2: SUMMARY status with an inline YAML comment still blocks the dependent', () => { - tmpDir = createTempProject('gsd-2830-review-comment-'); - const phaseDir = path.join(tmpDir, '.planning', 'phases', '10-inlinecomment'); - fs.mkdirSync(phaseDir, { recursive: true }); - - writePlan(phaseDir, '10-01-PLAN.md', ['wave: 1', 'objective: Halted, recorded with an inline comment', 'autonomous: true']); - writeSummary(phaseDir, '10-01-SUMMARY.md', 'halted # designed stop'); - writePlan(phaseDir, '10-02-PLAN.md', [ - 'wave: 2', 'objective: Depends on the inline-commented halt', 'autonomous: true', 'depends_on:', ' - 10-01', - ]); - - const result = runGsdTools(['init', 'execute-phase', '10', '--raw'], tmpDir); - assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); - const json = JSON.parse(result.output); - assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); - - assert.ok( - json.halted_plans.includes('10-01-PLAN.md'), - 'status: halted # designed stop must still be recognized as halted', - ); - assert.ok( - Object.prototype.hasOwnProperty.call(json.blocked_by, '10-02-PLAN.md'), - 'the dependent of an inline-commented halt must be blocked', - ); - assert.ok( - !json.runnable_plans.includes('10-02-PLAN.md'), - 'the dependent of an inline-commented halt must not be offered as runnable', - ); - }); - - test('defect 2: isHaltedStatus strips an unquoted trailing YAML comment before comparing', () => { - const { isHaltedStatus } = planDependencyGraph; - assert.strictEqual(isHaltedStatus('halted'), true); - assert.strictEqual(isHaltedStatus('Halted'), true); - assert.strictEqual(isHaltedStatus('halted '), true); - assert.strictEqual(isHaltedStatus('halted # designed stop'), true); - assert.strictEqual(isHaltedStatus('halted#nospace'), false, 'a `#` with no preceding whitespace is not a YAML comment'); - assert.strictEqual(isHaltedStatus('complete'), false); - assert.strictEqual(isHaltedStatus('complete # done'), false); - assert.strictEqual(isHaltedStatus(''), false); - assert.strictEqual(isHaltedStatus(undefined), false); - }); -}); diff --git a/tests/fix-2847-gap-closure-frontmatter.test.cjs b/tests/fix-2847-gap-closure-frontmatter.test.cjs deleted file mode 100644 index 1db5fe83a..000000000 --- a/tests/fix-2847-gap-closure-frontmatter.test.cjs +++ /dev/null @@ -1,220 +0,0 @@ -'use strict'; - -// allow-test-rule: source-text-is-the-product (see #2847) -// agents/gsd-planner.md is the deployed runtime prompt contract — the planner -// agent literally executes this markdown. Testing its text content tests the -// deployed contract, per the CONTRIBUTING.md exception matrix and the existing -// precedent in tests/plan-phase-drift-guard.test.cjs and -// tests/edge-probe-planner-contract.test.cjs. - -/** - * Regression tests for #2847 - * - * "--gaps does not load planner-gap-closure.md, so generated gap plans may - * miss gap_closure metadata" - * - * Root cause: the planner's only machine-checked validation gate - * (`gsd_run query frontmatter.validate "$PLAN_PATH" --schema plan`) never - * required `gap_closure`. The only place `gap_closure: true` was actually - * documented as required was prose in a conditionally-loaded reference file - * (gsd-core/references/planner-gap-closure.md) plus an unvalidated checklist - * item — neither backed by a deterministic gate. - * - * Fix: - * - src/frontmatter.cts: new `plan-gap-closure` FRONTMATTER_SCHEMAS entry - * (covered behaviorally in tests/frontmatter-cli.test.cjs and - * tests/frontmatter.unit.test.cjs — this file covers the prompt-level wiring - * that selects it). - * - agents/gsd-planner.md ``: the bash invocation - * now reads `--schema "$SCHEMA"` — a real shell-variable reference, bound in - * the same style as the file's existing `"$PLAN_PATH"` convention — instead - * of a hardcoded literal. An earlier revision left the bash line unconditional - * (`--schema plan)`, a copy-executable no-op) while only the prose sentence - * above it mentioned the conditional; that revision satisfied every - * substring-presence check but never actually selected plan-gap-closure at - * runtime. Caught by review, not by tests — see the describe block below for - * the executable-content assertions written specifically to catch it. - * - * Deliberately NOT touched: gsd-core/workflows/plan-phase.md's - * `` block. An earlier draft of this fix added a - * gap_closure mention there too (mirroring plan-phase.md's existing - * `` mode-scoped-block pattern for reviews - * mode), but plan-phase.md sits only 36 bytes under the hard ADR-857 - * PRE_PHASE6 ceiling (tests/phase6-capstone-conformance.test.cjs, - * `PRE_PHASE6['plan-phase.md'] = 94519`) and cannot absorb the ~330-byte - * addition. The `` fix in gsd-planner.md is the - * actual call site and is sufficient on its own: the planner already tracks - * gap_closure mode internally (its own `` switches - * to gap_closure_mode on `--gaps`), so the schema selection does not depend on - * plan-phase.md's prose at all. See .gsd/bug/fix-2847-gap-closure-frontmatter/10-diagnosis.md. - */ - -const { test, describe } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); - -const PLANNER_AGENT_PATH = path.join(__dirname, '..', 'agents', 'gsd-planner.md'); - -function readFile(p) { - return fs.readFileSync(p, 'utf-8'); -} - -function extractStep(content, stepName) { - const marker = ``; - const start = content.indexOf(marker); - if (start === -1) return null; - const end = content.indexOf('', start); - if (end === -1) return null; - return content.slice(start, end + ''.length); -} - -/** - * Extract the FIRST ```bash ... ``` fenced block from a step's text. Returns null - * if no fenced bash block is found. - */ -function extractFirstBashBlock(stepText) { - const m = /```bash\r?\n([\s\S]*?)```/.exec(stepText); - return m ? m[1] : null; -} - -/** - * Find the literal line, within a bash block, that invokes `frontmatter.validate`. - * Returns null if not found. - */ -function findValidateInvocationLine(bashBlock) { - if (!bashBlock) return null; - return bashBlock.split('\n').find((l) => l.includes('frontmatter.validate')) || null; -} - -// ─── agents/gsd-planner.md: validate_plan step BINDS schema to mode (#2847) ── -// -// This describe block asserts on the EXECUTABLE content of the step — the literal -// argument passed to `--schema` in the fenced bash block the agent actually runs — -// not on whether explanatory words appear anywhere in the step's prose. A prose -// sentence like "use plan-gap-closure in gap_closure mode, else plan" sitting next -// to an UNCONDITIONAL `--schema plan)` line satisfies every substring-presence -// check imaginable while the agent still only ever executes `--schema plan`. That -// exact shape shipped in an earlier revision of this fix and was caught by review, -// not by tests — these tests are written specifically to catch it mechanically: -// verified RED against that revision (`--schema plan)` hardcoded in the bash -// block, `--schema plan-gap-closure` only in the prose sentence above it) before -// the bash block was changed to `--schema "$SCHEMA"`. - -describe('#2847: gsd-planner.md validate_plan step BINDS --schema to gap_closure mode (executable content, not prose)', () => { - const plannerContent = readFile(PLANNER_AGENT_PATH); - const validateStep = extractStep(plannerContent, 'validate_plan'); - const bashBlock = extractFirstBashBlock(validateStep || ''); - const invocationLine = findValidateInvocationLine(bashBlock); - - test('validate_plan step exists and has a fenced bash block invoking frontmatter.validate', () => { - assert.ok(validateStep, ' must exist in agents/gsd-planner.md'); - assert.ok(bashBlock, 'validate_plan step must have a ```bash fenced block'); - assert.ok(invocationLine, 'validate_plan step bash block must invoke frontmatter.validate'); - }); - - test('the --schema argument in the bash invocation is NOT a hardcoded literal', () => { - // Precondition: both regex checks below use `.test(invocationLine)`, and - // RegExp#test coerces a null/undefined argument to the STRING "null"/ - // "undefined" rather than throwing — neither hardcoded-literal pattern - // matches that string, so both negative assertions would pass vacuously - // (reporting "not hardcoded") even if the step or its bash block were - // deleted entirely. Fail loudly on that precondition first so a deleted - // step is reported as exactly that, not as a false "fix confirmed". - assert.ok(invocationLine, 'precondition: invocationLine must be found (see the first test in this block)'); - - // This is the exact regression: a prior revision had this line read - // `--schema plan)` verbatim — a plain, hardcoded, always-the-same-value - // literal that an agent executes as-is regardless of mode. Reject BOTH - // possible hardcoded literals explicitly, not just one, so a fix that - // flips the hardcoded default to plan-gap-closure (breaking standard mode - // instead of gap_closure mode) is caught too. - assert.ok( - !/--schema\s+plan\)/.test(invocationLine), - `bash invocation must not hardcode --schema plan — found: ${invocationLine}` - ); - assert.ok( - !/--schema\s+plan-gap-closure\)/.test(invocationLine), - `bash invocation must not hardcode --schema plan-gap-closure — found: ${invocationLine}` - ); - }); - - test('the --schema argument in the bash invocation IS a shell variable reference', () => { - // A variable reference means the value is resolved at execution time from - // whatever the agent has bound it to, not printed once in the template and - // copy-executed unchanged. Matches --schema "$SCHEMA", --schema $SCHEMA, - // or --schema "${SCHEMA}". - const varMatch = /--schema\s+"?\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?"?\)/.exec(invocationLine); - assert.ok( - varMatch, - `bash invocation's --schema argument must be a shell variable (e.g. --schema "$SCHEMA"), not a literal — found: ${invocationLine}` - ); - }); - - test('the bound variable is actually conditioned on gap_closure mode in the step prose, and both target schema names are named', () => { - const varMatch = /--schema\s+"?\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?"?\)/.exec(invocationLine); - assert.ok(varMatch, 'precondition: --schema must reference a variable (see previous test)'); - const varName = varMatch[1]; - - // The SAME variable name the bash block reads must appear in the step's prose - // (outside the bash block) — otherwise the "binding" is a variable nothing - // ever explains how to set, which is not meaningfully better than a literal. - const proseOutsideBash = validateStep.replace(/```bash\r?\n[\s\S]*?```/, ''); - assert.ok( - proseOutsideBash.includes(`$${varName}`) || proseOutsideBash.includes(`\`$${varName}\``), - `step prose must explain how $${varName} is set — the bash block references it but nothing binds it` - ); - - // Both concrete schema names this variable can resolve to must be named - // somewhere in the step, and gap_closure mode must be the stated condition - // for choosing between them. Match the plain `plan` schema as a standalone - // backtick-quoted token (`` `plan` ``), not the bare substring "plan" — - // a bare-substring check is satisfied incidentally by "verify.plan-structure" - // a few lines below even if the plain-plan branch were deleted entirely from - // the prose, which would make this assertion unable to ever fail. - assert.ok(validateStep.includes('plan-gap-closure'), 'step must name the plan-gap-closure schema'); - assert.ok( - /`plan`/.test(validateStep), - 'step must name the plain plan schema, as a standalone `plan` token, as the other branch' - ); - assert.ok(/gap_closure mode/i.test(validateStep), 'step must condition the choice on gap_closure mode by name'); - }); - - test('the plan-structure validation call below (unrelated step) is unaffected', () => { - // Regression guard for the fix itself: confirm the edit did not touch the - // sibling verify.plan-structure invocation in the same step. - assert.ok( - validateStep.includes('verify.plan-structure "$PLAN_PATH"'), - 'validate_plan step must still invoke verify.plan-structure unchanged' - ); - }); -}); - -// ─── Cross-file consistency: schema name used by both files matches (#2847) ── - -describe('#2847: schema name consistency between gsd-planner.md and src/frontmatter.cts', () => { - test('gsd-planner.md references the exact schema name "plan-gap-closure"', () => { - const plannerContent = readFile(PLANNER_AGENT_PATH); - assert.ok( - plannerContent.includes('plan-gap-closure'), - 'agents/gsd-planner.md must reference the literal schema name "plan-gap-closure" ' + - '(the exact key registered in FRONTMATTER_SCHEMAS in src/frontmatter.cts) — a ' + - 'mismatched name would fail at runtime with "Unknown schema"' - ); - }); -}); - -// #2847 review: two describe blocks previously lived here — -// "planner-gap-closure.md reference is untouched" and "plan-phase.md is -// deliberately unmodified by this fix" — both deleted. Neither file is -// touched by this fix, so both assertions were already GREEN at the RED -// commit (5e5897cd2f17ebf2fc55757bae651bbbeb236289): they pinned untouched -// files rather than providing regression coverage for anything this change -// altered. The plan-phase.md one was worse than merely unhelpful — it -// permanently forbade any FUTURE legitimate `gap_closure` mention in -// plan-phase.md, a trap for whoever eventually frees up that file's byte -// budget and has a real reason to add one. The design decision itself (why -// plan-phase.md is untouched — the ADR-857 PRE_PHASE6 byte ceiling) remains -// documented in the file-level comment above and in -// .gsd/bug/fix-2847-gap-closure-frontmatter/10-diagnosis.md; it just isn't -// asserted as a permanent negative here. diff --git a/tests/fix-2855-phase-locator-workstream-archive-scope.test.cjs b/tests/fix-2855-phase-locator-workstream-archive-scope.test.cjs deleted file mode 100644 index 8a4b4c58c..000000000 --- a/tests/fix-2855-phase-locator-workstream-archive-scope.test.cjs +++ /dev/null @@ -1,246 +0,0 @@ -/** - * Regression tests for #2855: the phase-locator's archived-milestone fallback - * hardcoded the project-root `.planning/milestones/` tree instead of routing - * through the workstream-aware `planningDir(cwd)` helper. A pending phase in - * workstream A, whose own `phases/` directory does not exist yet, would - * silently resolve to a same-numbered phase archived under the ROOT tree - * (an unrelated workstream's history, or a flat-mode project's archive) — - * complete with stale plan/summary counts and an "archived" status for a - * phase that is actually brand new. - * - * Root cause: src/phase-locator.cts:139 (`findPhaseInternal`) and - * src/phase-locator.cts:167 (`getArchivedPhaseDirs`) both used - * `path.join(cwd, '.planning', 'milestones')` instead of - * `path.join(planningDir(cwd), 'milestones')` — the same seam the - * active-phase search (line 132) and the archive-write path - * (`archivePhaseDirectories`, src/milestone.cts) already use. - * - * Ambient-env hermeticity: GSD_WORKSTREAM/GSD_PROJECT are read directly from - * process.env by planningDir() when omitted, so every test here explicitly - * saves and restores both (pattern from - * tests/fix-2297-resolve-model-ids-runtime-scoping.test.cjs) to avoid leaking - * state across tests or picking up a developer's ambient shell env. - */ - -'use strict'; - -const { test, describe, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); - -const phaseLocator = require('../gsd-core/bin/lib/phase-locator.cjs'); -const { createTempProject, cleanup } = require('./helpers.cjs'); - -let _origWorkstream; -let _origProject; - -function isolateWorkstreamEnv() { - _origWorkstream = process.env.GSD_WORKSTREAM; - _origProject = process.env.GSD_PROJECT; - delete process.env.GSD_WORKSTREAM; - delete process.env.GSD_PROJECT; -} - -function restoreWorkstreamEnv() { - if (_origWorkstream === undefined) delete process.env.GSD_WORKSTREAM; - else process.env.GSD_WORKSTREAM = _origWorkstream; - if (_origProject === undefined) delete process.env.GSD_PROJECT; - else process.env.GSD_PROJECT = _origProject; -} - -describe('#2855: findPhaseInternal does not leak cross-workstream archived phases', () => { - let tmpDir; - beforeEach(() => { isolateWorkstreamEnv(); }); - afterEach(() => { - restoreWorkstreamEnv(); - if (tmpDir) { cleanup(tmpDir); tmpDir = null; } - }); - - test('does not leak root-tree archived phase into an unrelated workstream', () => { - tmpDir = createTempProject('gsd-2855-'); - // Root archive holds phase 03 (unrelated workstream's / flat-mode history). - const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-legacy'); - fs.mkdirSync(rootArchive, { recursive: true }); - fs.writeFileSync(path.join(rootArchive, 'SOME-SUMMARY.md'), '# stale'); - - // Workstream "beta" exists with an empty phases/ dir — phase 03 is pending. - fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'beta', 'phases'), { recursive: true }); - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.findPhaseInternal(tmpDir, '3'); - assert.strictEqual(result, null, 'pending workstream phase must not resolve to the root archive'); - }); - - test('does not leak root archive when workstream phases dir is entirely absent', () => { - tmpDir = createTempProject('gsd-2855-'); - const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v2.0-phases', '01-legacy'); - fs.mkdirSync(rootArchive, { recursive: true }); - - // Brand-new workstream: no .planning/workstreams/gamma/ directory at all yet. - process.env.GSD_WORKSTREAM = 'gamma'; - - const result = phaseLocator.findPhaseInternal(tmpDir, '1'); - assert.strictEqual(result, null, 'a workstream with no directory yet must not resolve to the root archive'); - }); - - // Issue #2855 AC1: "...regardless of whether workstream A's roadmap already - // lists the phase and when it doesn't yet." findPhaseInternal never reads - // ROADMAP.md (it is a pure filesystem lookup), so this dimension cannot - // change its behavior — demonstrated directly rather than left as an - // inference from reading the source. - for (const roadmapHasEntry of [true, false]) { - test(`does not leak root archive whether or not the workstream's ROADMAP.md already lists the phase (roadmapHasEntry=${roadmapHasEntry})`, () => { - tmpDir = createTempProject('gsd-2855-'); - const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-legacy'); - fs.mkdirSync(rootArchive, { recursive: true }); - - const wsDir = path.join(tmpDir, '.planning', 'workstreams', 'beta'); - fs.mkdirSync(path.join(wsDir, 'phases'), { recursive: true }); - if (roadmapHasEntry) { - fs.writeFileSync( - path.join(wsDir, 'ROADMAP.md'), - ['# Roadmap', '', '### Phase 03: Pending Work', ''].join('\n'), - ); - } - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.findPhaseInternal(tmpDir, '3'); - assert.strictEqual(result, null, 'ROADMAP.md presence/absence must not affect the archive-leak guard'); - }); - } - - test('still finds a phase genuinely archived under the active workstream\'s own tree', () => { - tmpDir = createTempProject('gsd-2855-'); - const ownArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '03-real'); - fs.mkdirSync(ownArchive, { recursive: true }); - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.findPhaseInternal(tmpDir, '3'); - assert.ok(result !== null, 'workstream\'s own archived phase must still resolve'); - assert.strictEqual(result.found, true); - assert.strictEqual(result.archived, 'v1.0'); - assert.strictEqual( - result.directory, - '.planning/workstreams/beta/milestones/v1.0-phases/03-real', - ); - }); - - test('flat/non-workstream project archive resolution is unchanged', () => { - tmpDir = createTempProject('gsd-2855-'); - const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-flat'); - fs.mkdirSync(rootArchive, { recursive: true }); - // No GSD_WORKSTREAM / GSD_PROJECT set — flat mode. - - const result = phaseLocator.findPhaseInternal(tmpDir, '3'); - assert.ok(result !== null, 'flat-mode archive lookup must be unaffected by the fix'); - assert.strictEqual(result.found, true); - assert.strictEqual(result.archived, 'v1.0'); - assert.strictEqual(result.directory, '.planning/milestones/v1.0-phases/03-flat'); - }); - - test('two workstreams with same-numbered archived phases never cross-resolve', () => { - tmpDir = createTempProject('gsd-2855-'); - const alphaArchive = path.join(tmpDir, '.planning', 'workstreams', 'alpha', 'milestones', 'v1.0-phases', '03-alpha-work'); - const betaArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '03-beta-work'); - fs.mkdirSync(alphaArchive, { recursive: true }); - fs.mkdirSync(betaArchive, { recursive: true }); - - process.env.GSD_WORKSTREAM = 'alpha'; - const alphaResult = phaseLocator.findPhaseInternal(tmpDir, '3'); - assert.ok(alphaResult !== null); - assert.strictEqual(alphaResult.phase_name, 'alpha-work'); - - process.env.GSD_WORKSTREAM = 'beta'; - const betaResult = phaseLocator.findPhaseInternal(tmpDir, '3'); - assert.ok(betaResult !== null); - assert.strictEqual(betaResult.phase_name, 'beta-work'); - }); - - test('project+workstream combination scopes the archive fallback', () => { - tmpDir = createTempProject('gsd-2855-'); - const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '05-root-legacy'); - fs.mkdirSync(rootArchive, { recursive: true }); - - const scopedArchive = path.join(tmpDir, '.planning', 'proj-x', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '05-scoped'); - fs.mkdirSync(scopedArchive, { recursive: true }); - - process.env.GSD_PROJECT = 'proj-x'; - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.findPhaseInternal(tmpDir, '5'); - assert.ok(result !== null, 'project+workstream scoped archive must resolve'); - assert.strictEqual(result.phase_name, 'scoped'); - assert.strictEqual( - result.directory, - '.planning/proj-x/workstreams/beta/milestones/v1.0-phases/05-scoped', - ); - }); - - test('workstream-scoped archived directory is posix-style', () => { - tmpDir = createTempProject('gsd-2855-'); - const ownArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '07-posix'); - fs.mkdirSync(ownArchive, { recursive: true }); - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.findPhaseInternal(tmpDir, '7'); - assert.ok(result !== null); - assert.ok(!result.directory.includes('\\'), 'directory must use forward slashes on every platform'); - }); -}); - -describe('#2855: getArchivedPhaseDirs does not leak cross-workstream archived phases', () => { - let tmpDir; - beforeEach(() => { isolateWorkstreamEnv(); }); - afterEach(() => { - restoreWorkstreamEnv(); - if (tmpDir) { cleanup(tmpDir); tmpDir = null; } - }); - - test('does not leak root-tree archive entries under an active workstream', () => { - tmpDir = createTempProject('gsd-2855-'); - const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-legacy'); - fs.mkdirSync(rootArchive, { recursive: true }); - - fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'beta', 'phases'), { recursive: true }); - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.getArchivedPhaseDirs(tmpDir); - assert.deepEqual(result, [], 'getArchivedPhaseDirs must not surface the root archive for a workstream'); - }); - - test('still finds phases genuinely archived under the active workstream\'s own tree', () => { - tmpDir = createTempProject('gsd-2855-'); - const ownArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v2.0-phases', '04-own'); - fs.mkdirSync(ownArchive, { recursive: true }); - process.env.GSD_WORKSTREAM = 'beta'; - - const result = phaseLocator.getArchivedPhaseDirs(tmpDir); - assert.strictEqual(result.length, 1); - assert.strictEqual(result[0].name, '04-own'); - assert.strictEqual(result[0].milestone, 'v2.0'); - // basePath is posix-normalized (toPosixPath) — a forward-slash literal is - // the correct cross-platform expectation, not path.join. - assert.strictEqual( - result[0].basePath, - '.planning/workstreams/beta/milestones/v2.0-phases', - ); - }); - - test('flat/non-workstream project resolution is unchanged', () => { - tmpDir = createTempProject('gsd-2855-'); - const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v2.1.0-phases'); - fs.mkdirSync(path.join(archiveDir, '03-auth'), { recursive: true }); - // No GSD_WORKSTREAM / GSD_PROJECT set — flat mode. - - const result = phaseLocator.getArchivedPhaseDirs(tmpDir); - assert.strictEqual(result.length, 1); - const entry = result[0]; - assert.strictEqual(entry.name, '03-auth'); - assert.strictEqual(entry.milestone, 'v2.1.0'); - // basePath is posix-normalized (toPosixPath) — a forward-slash literal is - // the correct cross-platform expectation, not path.join. - assert.strictEqual(entry.basePath, '.planning/milestones/v2.1.0-phases'); - assert.strictEqual(entry.fullPath, path.join(archiveDir, '03-auth')); - }); -}); diff --git a/tests/fix-3174-quick-verification-status-read.test.cjs b/tests/fix-3174-quick-verification-status-read.test.cjs deleted file mode 100644 index 080116a7e..000000000 --- a/tests/fix-3174-quick-verification-status-read.test.cjs +++ /dev/null @@ -1,128 +0,0 @@ -// allow-test-rule: source-text-is-the-product see #3174 -// Workflow .md / agent .md / command .md / reference .md files — their text -// IS what the runtime loads. Testing text content tests the deployed contract. -// Per CONTRIBUTING.md exception matrix. -'use strict'; - -/** - * quick verification-status read contract (#3174) - * - * quick's verification step used to read the verifier's result with a raw - * `grep "^status:" F | cut -d: -f2 | tr -d ' '` and route it through arms - * passed / human_needed / gaps_found only. That read failed two ways, - * both measured against the old pipeline. - * - * Matched NO arm: a missing report; most off-schema values; a `status:` line - * in BOTH the frontmatter and the prose (two lines); and — on a CRLF - * checkout — a perfectly valid `passed`, which arrives as `passed\r`. - * - * Matched the SUCCESS arm when it should not have: a stale report still - * reading `passed` (staleness was never evaluated); a report whose only - * `status:` line sits in its prose; and an off-schema value carrying a colon - * (`passed:bogus`), which `cut -d: -f2` splits at that colon, leaving the - * pipeline to yield `passed` once `tr -d ' '` strips the leading space. - * - * The unanchored match is the DEFECT.FRONTMATTER-SCALAR-BROAD-GREP class the - * code side already fixed by name. - * - * These tests pin the five properties that keep the replacement honest. - */ - -const { test, describe } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); - -const QUICK_VERIFICATION = path.join( - __dirname, '..', 'gsd-core', 'workflows', 'quick', 'steps', 'quick-verification.md', -); -// The canonical launcher preamble. scripts/sync-runtime-launcher.cjs rewrites -// every workflow's bootstrap from this file, so THIS is the authority — not -// whichever sibling step file happens to carry a copy today. -const LAUNCHER_SNIPPET = path.join( - __dirname, '..', 'gsd-core', 'workflows', '_runtime-launcher.snippet.sh', -); - -const SHIM_ANCHOR = '_GSD_SHIM_NAME="gsd-tools.cjs"'; - -describe('quick verification status read (#3174)', () => { - test('status is read through the canonical query, not a raw frontmatter grep', () => { - const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); - const queryIdx = content.indexOf('gsd_run query verification.status "${QUICK_DIR}"'); - - assert.ok(queryIdx !== -1, 'quick-verification.md must read status via the verification.status query'); - assert.ok( - !content.includes('grep "^status:"'), - 'the raw frontmatter-scalar grep must not return — it matches body lines too (DEFECT.FRONTMATTER-SCALAR-BROAD-GREP)', - ); - }); - - test('the query call is preceded by the runtime shim bootstrap in this step file', () => { - // Step files are read and executed as their own units, so quick.md's - // bootstrap does not reach here. Without this the call resolves to - // nothing, 2>/dev/null swallows it, and the default arm is taken forever. - const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); - const shimIdx = content.indexOf(SHIM_ANCHOR); - const queryIdx = content.indexOf('gsd_run query verification.status'); - - assert.ok(shimIdx !== -1, 'the step file must carry its own runtime shim bootstrap'); - assert.ok(queryIdx > shimIdx, 'the shim bootstrap must precede the gsd_run call'); - }); - - test('the shim bootstrap is the canonical launcher preamble, not a fork of it', () => { - // Anchored on _runtime-launcher.snippet.sh rather than on a sibling step - // file: sync-runtime-launcher.cjs regenerates every workflow from the - // snippet, so a synchronized launcher update keeps this green (correct), - // and a sibling that legitimately stops calling gsd_run cannot fail us. - const lineWithShim = (file) => fs.readFileSync(file, 'utf-8') - .split(/\r?\n/) - .find((line) => line.startsWith(SHIM_ANCHOR)); - - const mine = lineWithShim(QUICK_VERIFICATION); - const canonical = lineWithShim(LAUNCHER_SNIPPET); - - assert.ok(canonical, '_runtime-launcher.snippet.sh must carry the canonical preamble'); - assert.equal(mine, canonical, 'the bootstrap must match the canonical launcher snippet verbatim'); - }); - - test('status extraction does not depend on jq', () => { - // #2589: a `| jq -r '.field'` pipe yields an empty variable with no - // diagnostic wherever jq is absent (the Windows/Git-Bash default), which - // would route a passing verification into the recovery arm. - // - // Scoped to the executable fence on purpose: the surrounding prose cites - // the jq form in order to explain why it is not used, and an assertion - // over the whole file would fire on its own rationale. - const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); - const fences = content.match(/```bash\r?\n[\s\S]*?```/g) || []; - const statusFence = fences.find((f) => f.includes('gsd_run query verification.status')); - - assert.ok(statusFence, 'the status read must live in a bash fence'); - assert.ok( - statusFence.includes('--pick status'), - 'the bare status must be picked by the query itself', - ); - assert.ok(!/\|\s*jq\b/.test(statusFence), 'the status-read fence must not pipe through jq'); - }); - - test('the routing table carries a terminal arm for missing / unknown / stale', () => { - const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); - const gapsIdx = content.indexOf('| `gaps_found` |'); - const fallbackIdx = content.indexOf('| anything else'); - - assert.ok(gapsIdx !== -1, 'the three verifier-status arms must remain'); - assert.ok(fallbackIdx > gapsIdx, 'a terminal arm must follow the verifier-status arms'); - - const fallbackRow = content.slice(fallbackIdx, content.indexOf('\n', fallbackIdx)); - for (const sentinel of ['missing', 'unknown', 'stale']) { - assert.ok( - fallbackRow.includes(sentinel), - `the terminal arm must name the ${sentinel} sentinel the query can return`, - ); - } - assert.ok( - fallbackRow.includes('VERIFICATION_STATUS'), - 'the terminal arm must set the display string consumed by the quick index row and banner', - ); - }); -}); diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index 9ddf70c59..a0e09ace9 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -2053,3 +2053,193 @@ test('extractFrontmatter handles large frontmatter blocks without body bleed', ( }); }); } + + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/fix-2847-gap-closure-frontmatter.test.cjs — test-hygiene sweep #3335 (H3 Wave 3) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe("folded:fix-2847-gap-closure-frontmatter (test-hygiene sweep #3335 H3 Wave 3)", () => { +'use strict'; + +// allow-test-rule: source-text-is-the-product (see #2847) +// agents/gsd-planner.md is the deployed runtime prompt contract — testing its +// text content tests the deployed contract (CONTRIBUTING.md exception matrix). +// +// Regression tests for #2847: "--gaps does not load planner-gap-closure.md, +// so generated gap plans may miss gap_closure metadata". Root cause: the +// planner's only machine-checked gate (`frontmatter.validate ... --schema +// plan`) never required gap_closure — the requirement lived only in +// conditionally-loaded prose. Fix: src/frontmatter.cts gained a +// `plan-gap-closure` schema (covered behaviorally in frontmatter-cli.test.cjs +// / frontmatter.unit.test.cjs); this block covers the prompt-level wiring in +// agents/gsd-planner.md's that selects it via a +// real `--schema "$SCHEMA"` shell-variable reference instead of a hardcoded +// literal. An earlier revision left the bash line unconditional +// (`--schema plan)`) while only prose mentioned the conditional — caught by +// review, not tests; the assertions below target executable content +// specifically to catch that shape. See +// .gsd/bug/fix-2847-gap-closure-frontmatter/10-diagnosis.md. +// +// Deliberately NOT touched: gsd-core/workflows/plan-phase.md sits under the +// ADR-857 PRE_PHASE6 byte ceiling and cannot absorb a gap_closure mention; +// the gsd-planner.md validate_plan step is the actual call site and is +// sufficient on its own. + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const PLANNER_AGENT_PATH = path.join(__dirname, '..', 'agents', 'gsd-planner.md'); + +function readFile(p) { + return fs.readFileSync(p, 'utf-8'); +} + +function extractStep(content, stepName) { + const marker = ``; + const start = content.indexOf(marker); + if (start === -1) return null; + const end = content.indexOf('', start); + if (end === -1) return null; + return content.slice(start, end + ''.length); +} + +/** + * Extract the FIRST ```bash ... ``` fenced block from a step's text. Returns null + * if no fenced bash block is found. + */ +function extractFirstBashBlock(stepText) { + const m = /```bash\r?\n([\s\S]*?)```/.exec(stepText); + return m ? m[1] : null; +} + +/** + * Find the literal line, within a bash block, that invokes `frontmatter.validate`. + * Returns null if not found. + */ +function findValidateInvocationLine(bashBlock) { + if (!bashBlock) return null; + return bashBlock.split('\n').find((l) => l.includes('frontmatter.validate')) || null; +} + +// ─── agents/gsd-planner.md: validate_plan step BINDS schema to mode (#2847) ── +// +// This describe block asserts on the EXECUTABLE content of the step — the literal +// argument passed to `--schema` in the fenced bash block the agent actually runs — +// not on whether explanatory words appear anywhere in the step's prose. A prose +// sentence like "use plan-gap-closure in gap_closure mode, else plan" sitting next +// to an UNCONDITIONAL `--schema plan)` line satisfies every substring-presence +// check imaginable while the agent still only ever executes `--schema plan`. That +// exact shape shipped in an earlier revision of this fix and was caught by review, +// not by tests — these tests are written specifically to catch it mechanically: +// verified RED against that revision (`--schema plan)` hardcoded in the bash +// block, `--schema plan-gap-closure` only in the prose sentence above it) before +// the bash block was changed to `--schema "$SCHEMA"`. + +describe('#2847: gsd-planner.md validate_plan step BINDS --schema to gap_closure mode (executable content, not prose)', () => { + const plannerContent = readFile(PLANNER_AGENT_PATH); + const validateStep = extractStep(plannerContent, 'validate_plan'); + const bashBlock = extractFirstBashBlock(validateStep || ''); + const invocationLine = findValidateInvocationLine(bashBlock); + + test('validate_plan step exists and has a fenced bash block invoking frontmatter.validate', () => { + assert.ok(validateStep, ' must exist in agents/gsd-planner.md'); + assert.ok(bashBlock, 'validate_plan step must have a ```bash fenced block'); + assert.ok(invocationLine, 'validate_plan step bash block must invoke frontmatter.validate'); + }); + + test('the --schema argument in the bash invocation is NOT a hardcoded literal', () => { + // Precondition: both regex checks below use `.test(invocationLine)`, and + // RegExp#test coerces a null/undefined argument to the STRING "null"/ + // "undefined" rather than throwing — neither hardcoded-literal pattern + // matches that string, so both negative assertions would pass vacuously + // (reporting "not hardcoded") even if the step or its bash block were + // deleted entirely. Fail loudly on that precondition first so a deleted + // step is reported as exactly that, not as a false "fix confirmed". + assert.ok(invocationLine, 'precondition: invocationLine must be found (see the first test in this block)'); + + // This is the exact regression: a prior revision had this line read + // `--schema plan)` verbatim — a plain, hardcoded, always-the-same-value + // literal that an agent executes as-is regardless of mode. Reject BOTH + // possible hardcoded literals explicitly, not just one, so a fix that + // flips the hardcoded default to plan-gap-closure (breaking standard mode + // instead of gap_closure mode) is caught too. + assert.ok( + !/--schema\s+plan\)/.test(invocationLine), + `bash invocation must not hardcode --schema plan — found: ${invocationLine}` + ); + assert.ok( + !/--schema\s+plan-gap-closure\)/.test(invocationLine), + `bash invocation must not hardcode --schema plan-gap-closure — found: ${invocationLine}` + ); + }); + + test('the --schema argument in the bash invocation IS a shell variable reference', () => { + // A variable reference means the value is resolved at execution time from + // whatever the agent has bound it to, not printed once in the template and + // copy-executed unchanged. Matches --schema "$SCHEMA", --schema $SCHEMA, + // or --schema "${SCHEMA}". + const varMatch = /--schema\s+"?\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?"?\)/.exec(invocationLine); + assert.ok( + varMatch, + `bash invocation's --schema argument must be a shell variable (e.g. --schema "$SCHEMA"), not a literal — found: ${invocationLine}` + ); + }); + + test('the bound variable is actually conditioned on gap_closure mode in the step prose, and both target schema names are named', () => { + const varMatch = /--schema\s+"?\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?"?\)/.exec(invocationLine); + assert.ok(varMatch, 'precondition: --schema must reference a variable (see previous test)'); + const varName = varMatch[1]; + + // The SAME variable name the bash block reads must appear in the step's prose + // (outside the bash block) — otherwise the "binding" is a variable nothing + // ever explains how to set, which is not meaningfully better than a literal. + const proseOutsideBash = validateStep.replace(/```bash\r?\n[\s\S]*?```/, ''); + assert.ok( + proseOutsideBash.includes(`$${varName}`) || proseOutsideBash.includes(`\`$${varName}\``), + `step prose must explain how $${varName} is set — the bash block references it but nothing binds it` + ); + + // Both concrete schema names this variable can resolve to must be named + // somewhere in the step, and gap_closure mode must be the stated condition + // for choosing between them. Match the plain `plan` schema as a standalone + // backtick-quoted token (`` `plan` ``), not the bare substring "plan" — + // a bare-substring check is satisfied incidentally by "verify.plan-structure" + // a few lines below even if the plain-plan branch were deleted entirely from + // the prose, which would make this assertion unable to ever fail. + assert.ok(validateStep.includes('plan-gap-closure'), 'step must name the plan-gap-closure schema'); + assert.ok( + /`plan`/.test(validateStep), + 'step must name the plain plan schema, as a standalone `plan` token, as the other branch' + ); + assert.ok(/gap_closure mode/i.test(validateStep), 'step must condition the choice on gap_closure mode by name'); + }); + + test('the plan-structure validation call below (unrelated step) is unaffected', () => { + // Regression guard for the fix itself: confirm the edit did not touch the + // sibling verify.plan-structure invocation in the same step. + assert.ok( + validateStep.includes('verify.plan-structure "$PLAN_PATH"'), + 'validate_plan step must still invoke verify.plan-structure unchanged' + ); + }); +}); + +// ─── Cross-file consistency: schema name used by both files matches (#2847) ── + +describe('#2847: schema name consistency between gsd-planner.md and src/frontmatter.cts', () => { + test('gsd-planner.md references the exact schema name "plan-gap-closure"', () => { + const plannerContent = readFile(PLANNER_AGENT_PATH); + assert.ok( + plannerContent.includes('plan-gap-closure'), + 'agents/gsd-planner.md must reference the literal schema name "plan-gap-closure" ' + + '(the exact key registered in FRONTMATTER_SCHEMAS in src/frontmatter.cts) — a ' + + 'mismatched name would fail at runtime with "Unknown schema"' + ); + }); +}); + }); +} diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 8c9917c59..7ac17f530 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -921,6 +921,39 @@ function clearSessionEnv() { for (const k of SESSION_ENV_KEYS) delete process.env[k]; } +/** + * Save + clear GSD_WORKSTREAM and GSD_PROJECT on process.env, paired with + * restoreWorkstreamEnv(). planningDir() reads both directly from + * process.env when its params are omitted, so a test asserting + * workstream/project-scoped behavior must isolate them from ambient shell + * state (and from whatever an earlier test in the same process left behind). + * + * Previously duplicated as a local isolateWorkstreamEnv()/restoreWorkstreamEnv() + * pair in tests/phase-locator.test.cjs, and as the GSD_WORKSTREAM/GSD_PROJECT + * slice of tests/model-resolver.test.cjs's broader isolateHome()/restoreHome() + * (which still isolates HOME/USERPROFILE/GSD_HOME/GSD_RUNTIME locally — that + * part is genuinely specific to model-resolver's tests and stays there). + * + * Module-level save slot (not a returned snapshot) to match the exact + * no-arg isolate()/restore() call shape both prior local copies used. + */ +let _origGsdWorkstream; +let _origGsdProject; + +function isolateWorkstreamEnv() { + _origGsdWorkstream = process.env.GSD_WORKSTREAM; + _origGsdProject = process.env.GSD_PROJECT; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; +} + +function restoreWorkstreamEnv() { + if (_origGsdWorkstream === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = _origGsdWorkstream; + if (_origGsdProject === undefined) delete process.env.GSD_PROJECT; + else process.env.GSD_PROJECT = _origGsdProject; +} + /** * #3156: env for a RAW installer spawn — one that bypasses runGsdTools and so * never receives TEST_ENV_BASE on its own. @@ -965,7 +998,7 @@ function installSpawnEnv(overrides = {}) { return { ...process.env, ...testEnvBase(), HOME: home, USERPROFILE: home, ...overrides }; } -module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, tmpRootCandidates, readFileNormalized, readWorkflowCombined, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, absPlanningPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, SESSION_ENV_KEYS, saveSessionEnv, restoreSessionEnv, clearSessionEnv, TOOLS_PATH, SESSION_IDENTITY_ENV_KEYS, scrubConfigLocationEnv, installSpawnEnv, installSpawnHome }; +module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, tmpRootCandidates, readFileNormalized, readWorkflowCombined, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, absPlanningPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, SESSION_ENV_KEYS, saveSessionEnv, restoreSessionEnv, clearSessionEnv, isolateWorkstreamEnv, restoreWorkstreamEnv, TOOLS_PATH, SESSION_IDENTITY_ENV_KEYS, scrubConfigLocationEnv, installSpawnEnv, installSpawnHome }; // Lazy, for the reason builtLib() is lazy: reading either of these is what // forces the built-lib require, so a test file that needs neither can still diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 279d74414..7c815614f 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -4372,3 +4372,410 @@ describe('#2229 PROPERTY: resolveTierFromConfig never throws and always returns }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/fix-2297-resolve-model-ids-runtime-scoping.test.cjs +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:fix-2297-resolve-model-ids-runtime-scoping', () => { +/** + * Bug #2297 — `resolve_model_ids:"omit"` must be scoped to the ACTIVE runtime, + * not applied blindly whenever it appears anywhere in the merged config. + * + * Root cause (pre-fix): the installer writes `resolve_model_ids:"omit"` into + * the SHARED `~/.gsd/defaults.json` for every runtime that lacks native model + * aliases (#1156). Because that file is machine-wide, installing a non-Claude + * runtime (e.g. codex) on a box that also runs Claude poisoned Claude's + * no-project resolution: Claude would see `resolve_model_ids:"omit"` in the + * merged global defaults and return `''` instead of its tier aliases + * (opus/sonnet/haiku), silently defeating Claude's adaptive tier distinction. + * + * Fix (`resolveModelInternal`): the `"omit"` branch now returns `''` ONLY + * when either (a) the PROJECT's own `.planning/config.json` explicitly sets + * `resolve_model_ids:"omit"` (user intent — #2517 finding #4, unchanged), or + * (b) the ACTIVE runtime genuinely lacks native model aliases. A native-alias + * runtime (currently only `claude`) ignores an `"omit"` that came solely from + * the shared global defaults and falls through to its tier aliases. + * Active-runtime precedence: `process.env.GSD_RUNTIME` -> `config.runtime` -> + * per-install `.gsd-runtime` marker -> `'claude'` (all canonicalized). + * + * NOTE: the global-defaults merge path in config-loader.cjs (branch D: "no + * .planning/ at all") only fires when the project dir has NO `.planning/` + * whatsoever — the moment `.planning/` exists, `~/.gsd/defaults.json` is not + * merged for these fields at all. Group A below therefore uses bare + * `fs.mkdtempSync` project dirs with no `.planning/` subdir; Group A #4, + * Group B, and Group C need a real per-project config and create + * `.planning/config.json`. + */ + +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { + resolveModelInternal, + _setInstallRuntimeMarkerForTests, + _resetInstallRuntimeMarkerCacheForTests, +} = require('../gsd-core/bin/lib/model-resolver.cjs'); + +const { isolateWorkstreamEnv, restoreWorkstreamEnv } = require('./helpers.cjs'); + +// HOME / GSD_HOME / GSD_RUNTIME isolation — config-loader.cjs reads global +// defaults from path.join(process.env.GSD_HOME || os.homedir(), '.gsd', 'defaults.json'). +// Isolate HOME and GSD_HOME to a fresh tmpdir per test, and save/restore +// GSD_RUNTIME (several tests set it directly to drive the active-runtime +// chain) plus GSD_WORKSTREAM/GSD_PROJECT via the shared helpers.cjs +// isolateWorkstreamEnv()/restoreWorkstreamEnv() pair (planningDir() reads both +// directly from process.env when its params are omitted, so an ambient value +// in a developer's shell could redirect projectExplicitlySetsOmit()'s reads). +let _origHome; +let _origUserProfile; +let _origGsdHome; +let _origGsdRuntime; +let _isolatedHome; + +function isolateHome() { + _origHome = process.env.HOME; + _origUserProfile = process.env.USERPROFILE; + _origGsdHome = process.env.GSD_HOME; + _origGsdRuntime = process.env.GSD_RUNTIME; + isolateWorkstreamEnv(); + _isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-home-')); + process.env.HOME = _isolatedHome; + // Windows resolves the home dir from USERPROFILE, not HOME. + process.env.USERPROFILE = _isolatedHome; + process.env.GSD_HOME = _isolatedHome; + delete process.env.GSD_RUNTIME; +} + +function restoreHome() { + if (_origHome === undefined) delete process.env.HOME; else process.env.HOME = _origHome; + if (_origUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = _origUserProfile; + if (_origGsdHome === undefined) delete process.env.GSD_HOME; else process.env.GSD_HOME = _origGsdHome; + if (_origGsdRuntime === undefined) delete process.env.GSD_RUNTIME; else process.env.GSD_RUNTIME = _origGsdRuntime; + restoreWorkstreamEnv(); + rmDir(_isolatedHome); + _isolatedHome = null; +} + +function rmDir(dir) { + if (typeof dir !== 'string' || dir.length === 0) return; + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- carries the same maxRetries/retryDelay budget as helpers.cleanup; used for both the isolated-home and bare project temp dirs + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); +} + +function writeGlobalDefaults(obj) { + fs.mkdirSync(path.join(_isolatedHome, '.gsd'), { recursive: true }); + fs.writeFileSync(path.join(_isolatedHome, '.gsd', 'defaults.json'), JSON.stringify(obj, null, 2)); +} + +// Bare project dir with NO .planning/ subdirectory — needed to exercise the +// config-loader's global-defaults merge branch (see header comment above). +function mkProjNoPlanning() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-proj-noplan-')); +} + +// Project dir WITH a .planning/config.json — the normal "inside a project" path. +function mkProjWithConfig(obj) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-proj-')); + fs.mkdirSync(path.join(dir, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify(obj, null, 2)); + return dir; +} + +// ─── Group A: GLOBAL-defaults "omit" is runtime-scoped (the #2297 fix) ───── +describe('#2297: global-defaults resolve_model_ids:"omit" is scoped to the active runtime', () => { + let projDir; + beforeEach(() => { isolateHome(); projDir = null; }); + afterEach(() => { rmDir(projDir); restoreHome(); }); + + test('no runtime signal defaults to claude: executor and planner get distinct non-empty tier aliases (acceptance #3)', () => { + // Global defaults poison the shared file with "omit" (simulating a + // non-Claude runtime having been installed on this machine). With no + // .planning/config.json (no project) and no GSD_RUNTIME, the active + // runtime falls back to 'claude', which has native aliases and must + // ignore the poisoned global "omit" — the adaptive tier distinction + // between executor (sonnet) and planner (opus) must survive. + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + + const executor = resolveModelInternal(projDir, 'gsd-executor'); + const planner = resolveModelInternal(projDir, 'gsd-planner'); + + assert.strictEqual(executor, 'sonnet'); + assert.strictEqual(planner, 'opus'); + assert.notStrictEqual(executor, ''); + assert.notStrictEqual(planner, ''); + assert.notStrictEqual(executor, planner); + }); + + test('GSD_RUNTIME="claude" explicitly: executor still resolves to "sonnet" (claude ignores global omit)', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + process.env.GSD_RUNTIME = 'claude'; + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); + }); + + test('GSD_RUNTIME="codex": a non-alias runtime still honors the global omit (acceptance #4)', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + process.env.GSD_RUNTIME = 'codex'; + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), ''); + }); + + // #2297 correctness-review BLOCKER: resolveActiveRuntime() must canonicalize + // its candidates via resolveRuntimeNameFromCandidates before checking + // RUNTIMES_WITH_NATIVE_ALIASES, or an alias/case variant of "claude" would + // fail the Set('claude').has() check and wrongly fall through to honoring the + // poisoned global omit. These would FAIL against a non-canonicalizing resolver. + test('GSD_RUNTIME="claude-code" (alias, not canonical "claude"): executor and planner still ignore the global omit', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + process.env.GSD_RUNTIME = 'claude-code'; + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); + assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), 'opus'); + }); + + test('GSD_RUNTIME="Claude" (case variant): executor still resolves to "sonnet" (canonicalization is case-insensitive)', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + process.env.GSD_RUNTIME = 'Claude'; + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); + }); + + test('project config.runtime="codex" (no resolve_model_ids in project) takes precedence over GSD_RUNTIME/marker in the active-runtime chain', () => { + // config.runtime is checked before GSD_RUNTIME / the install marker. This + // scenario uses a REAL project (.planning/config.json present), so the + // config-loader does NOT merge ~/.gsd/defaults.json for resolve_model_ids + // at all here (see header comment above) — resolution instead reaches the + // #2517 runtime-tier path (step 3 in resolveModelInternal, which fires + // before the omit gate) and returns codex's native sonnet-tier model id + // directly, rather than the omit gate's ''. Verified empirically: the + // built resolver returns 'gpt-5.6-terra', not ''. Assert it is non-empty + // and NOT a claude alias, which is the property this test actually needs + // to guarantee (config.runtime, not GSD_RUNTIME/env, drove the resolution). + projDir = mkProjWithConfig({ runtime: 'codex' }); + writeGlobalDefaults({ resolve_model_ids: 'omit' }); // irrelevant: not merged when .planning/ exists + + const result = resolveModelInternal(projDir, 'gsd-executor'); + assert.notStrictEqual(result, ''); + assert.ok( + !['sonnet', 'opus', 'haiku'].includes(result), + `expected a non-claude-alias result for config.runtime="codex", got ${JSON.stringify(result)}` + ); + }); + + test('install-order independence (acceptance #1/#2): a global omit poisoned by a prior non-Claude install does not affect Claude resolution, and Claude retains its adaptive tier distinction', () => { + // Resolution depends on the RESOLVING runtime (active runtime at call + // time), not on install order — installing codex (or any non-alias + // runtime) before/after Claude must never change what Claude itself + // resolves to. Global omit present, no project, no runtime signal -> + // default 'claude' -> tier aliases survive. Distinct from the first Group A + // test above: this asserts install-order independence AND, specifically, + // that executor/planner remain DIFFERENT tiers under the poisoned global + // omit — i.e. install order never collapses Claude's adaptive tier + // distinction into a single omitted value. + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + + const executor = resolveModelInternal(projDir, 'gsd-executor'); + const planner = resolveModelInternal(projDir, 'gsd-planner'); + + assert.strictEqual(executor, 'sonnet'); + assert.strictEqual(planner, 'opus'); + assert.notStrictEqual(executor, planner, 'install-order poisoning must not collapse the adaptive tier distinction'); + }); +}); + +// ─── Group B: explicit PROJECT "omit" is still honored for EVERY runtime ─── +// (#2517 finding #4 — preserved, NOT changed by #2297.) +describe('#2297: explicit project-level resolve_model_ids:"omit" is honored regardless of runtime', () => { + let projDir; + beforeEach(() => { isolateHome(); projDir = null; }); + afterEach(() => { rmDir(projDir); restoreHome(); }); + + test('no runtime set, explicit project omit -> "" even though the default runtime is claude', () => { + projDir = mkProjWithConfig({ resolve_model_ids: 'omit' }); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), ''); + }); + + test('runtime:"claude" + explicit project omit -> "" (mirrors #2517 finding #4)', () => { + projDir = mkProjWithConfig({ runtime: 'claude', resolve_model_ids: 'omit' }); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), ''); + }); +}); + +// ─── Group B2: projectExplicitlySetsOmit is workstream-scope aware (#2297) ── +// The root .planning/config.json does NOT set resolve_model_ids, but the +// ACTIVE workstream's own config.json does — projectExplicitlySetsOmit() +// resolves via planningDir(cwd) (workstream layer wins over root, mirroring +// loadConfig's precedence), so the workstream's explicit "omit" must still be +// honored even though no global default and the default runtime (claude) would +// otherwise have returned a tier alias. +describe('#2297: explicit project-level "omit" is honored at the active-workstream config layer', () => { + let projDir; + let _origGsdWorkstreamForBlock; + beforeEach(() => { + isolateHome(); // clears GSD_WORKSTREAM/GSD_PROJECT as part of hermeticity + projDir = null; + _origGsdWorkstreamForBlock = process.env.GSD_WORKSTREAM; + }); + afterEach(() => { + if (_origGsdWorkstreamForBlock === undefined) delete process.env.GSD_WORKSTREAM; + else process.env.GSD_WORKSTREAM = _origGsdWorkstreamForBlock; + rmDir(projDir); + restoreHome(); + }); + + test('root config has no resolve_model_ids, but the active workstream config sets "omit" -> "" despite default runtime claude', () => { + const ws = 'ws-alpha'; + projDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2297-proj-ws-')); + fs.mkdirSync(path.join(projDir, '.planning'), { recursive: true }); + // Root config exists but does NOT set resolve_model_ids at all. + fs.writeFileSync( + path.join(projDir, '.planning', 'config.json'), + JSON.stringify({ model_profile: 'balanced' }, null, 2) + ); + // The active workstream's own config explicitly sets "omit". + const wsConfigDir = path.join(projDir, '.planning', 'workstreams', ws); + fs.mkdirSync(wsConfigDir, { recursive: true }); + fs.writeFileSync( + path.join(wsConfigDir, 'config.json'), + JSON.stringify({ resolve_model_ids: 'omit' }, null, 2) + ); + process.env.GSD_WORKSTREAM = ws; + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), ''); + }); +}); + +// ─── Group C: explicit `true` still materializes full model ids (acceptance #5) ── +describe('#2297: resolve_model_ids:true still materializes full Claude model ids', () => { + let projDir; + beforeEach(() => { isolateHome(); projDir = null; }); + afterEach(() => { rmDir(projDir); restoreHome(); }); + + test('resolve_model_ids:true + balanced profile -> full materialized claude-opus-4-8 id', () => { + projDir = mkProjWithConfig({ resolve_model_ids: true, model_profile: 'balanced' }); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-planner'), 'claude-opus-4-8'); + }); +}); + +// ─── Group D: registry parity guard ───────────────────────────────────────── +describe('#2297: capability-registry nativeModelAliases parity guard', () => { + test('exactly the runtimes with hostBehaviors.nativeModelAliases:true match RUNTIMES_WITH_NATIVE_ALIASES ([\'claude\'])', () => { + // The model-resolver hardcodes RUNTIMES_WITH_NATIVE_ALIASES = new Set(['claude']) + // rather than reading the registry at runtime. This test keeps that + // hardcoded set honest against the generated registry's actual contract: + // registry.runtimes[id].runtime.hostBehaviors.nativeModelAliases. + // If a future runtime gains nativeModelAliases:true, this fails loudly so + // RUNTIMES_WITH_NATIVE_ALIASES in model-resolver.cts is updated in lockstep. + const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); + + const nativeAliasRuntimes = Object.keys(registry.runtimes) + .filter((id) => registry.runtimes[id]?.runtime?.hostBehaviors?.nativeModelAliases === true) + .sort(); + + assert.deepStrictEqual(nativeAliasRuntimes, ['claude']); + }); +}); + +// ─── Group E: installer writes the per-install .gsd-runtime marker ───────── +describe('#2297: installer emits the gsd-core/.gsd-runtime marker (fixture parity)', () => { + test('claude and codex install-tree fixtures both list gsd-core/.gsd-runtime', () => { + // These fixtures are flat JSON arrays of install-relative paths, generated + // by running the real installer (tests/fixtures/install-tree/*.json). Their + // presence here proves the installer actually emits the per-install marker + // that resolveActiveRuntime()'s precedence chain falls back to. + const claudeFixturePath = path.join(__dirname, 'fixtures', 'install-tree', 'claude.json'); + const codexFixturePath = path.join(__dirname, 'fixtures', 'install-tree', 'codex.json'); + + const claudeFixture = JSON.parse(fs.readFileSync(claudeFixturePath, 'utf8')); + const codexFixture = JSON.parse(fs.readFileSync(codexFixturePath, 'utf8')); + + assert.ok(Array.isArray(claudeFixture), 'expected claude.json fixture to be a flat array of paths'); + assert.ok(Array.isArray(codexFixture), 'expected codex.json fixture to be a flat array of paths'); + + assert.ok( + claudeFixture.includes('gsd-core/.gsd-runtime'), + 'expected claude.json install-tree fixture to include gsd-core/.gsd-runtime' + ); + assert.ok( + codexFixture.includes('gsd-core/.gsd-runtime'), + 'expected codex.json install-tree fixture to include gsd-core/.gsd-runtime' + ); + }); +}); + +// ─── Group F: the install-marker precedence rung, driven directly (#2297) ── +// Previously untested: with no GSD_RUNTIME and no project config.runtime, the +// active runtime falls all the way through to the per-install .gsd-runtime +// marker (third precedence rung). The dev/source tree has no real marker file, +// so these tests drive that rung directly via the _setInstallRuntimeMarkerForTests +// / _resetInstallRuntimeMarkerCacheForTests seams exported specifically for this +// purpose (#2297 correctness-review gap). +describe('#2297: install-marker precedence rung (GSD_RUNTIME and config.runtime both absent)', () => { + let projDir; + beforeEach(() => { + isolateHome(); // also deletes GSD_RUNTIME + projDir = null; + // Belt-and-suspenders: the marker rung is only reached when GSD_RUNTIME and + // config.runtime are both absent; isolateHome() already deletes GSD_RUNTIME. + delete process.env.GSD_RUNTIME; + }); + afterEach(() => { + rmDir(projDir); + restoreHome(); + // CRITICAL: reset the module-level marker cache after every case in this + // block so a set value never leaks into a later case here, or into any + // OTHER describe block in this file (readInstallRuntimeMarker() otherwise + // memoizes the first value it sees for the lifetime of the process). + _resetInstallRuntimeMarkerCacheForTests(); + }); + + test('marker="codex" (non-alias runtime): honors the poisoned global omit -> ""', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + _setInstallRuntimeMarkerForTests('codex'); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), ''); + }); + + test('marker="claude": ignores the poisoned global omit -> "sonnet"', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + _setInstallRuntimeMarkerForTests('claude'); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); + }); + + test('marker="claude-code" (alias): canonicalized to "claude" and still ignores the poisoned global omit -> "sonnet"', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + _setInstallRuntimeMarkerForTests('claude-code'); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); + }); + + test('marker unset (null): falls through to the "claude" default and ignores the poisoned global omit -> "sonnet"', () => { + writeGlobalDefaults({ resolve_model_ids: 'omit' }); + projDir = mkProjNoPlanning(); + _setInstallRuntimeMarkerForTests(null); + + assert.strictEqual(resolveModelInternal(projDir, 'gsd-executor'), 'sonnet'); + }); +}); + }); +} diff --git a/tests/phase-locator.test.cjs b/tests/phase-locator.test.cjs index 5fcf2966f..4dc42dec0 100644 --- a/tests/phase-locator.test.cjs +++ b/tests/phase-locator.test.cjs @@ -18,13 +18,17 @@ 'use strict'; -const { test, describe, afterEach } = require('node:test'); +const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const fc = require('./helpers/fast-check-setup.cjs'); const phaseLocator = require('../gsd-core/bin/lib/phase-locator.cjs'); -const { createTempProject, cleanup } = require('./helpers.cjs'); +const planDependencyGraph = require('../gsd-core/bin/lib/plan-dependency-graph.cjs'); +const { + runGsdTools, createTempProject, cleanup, isolateWorkstreamEnv, restoreWorkstreamEnv, +} = require('./helpers.cjs'); // ─── findPhaseInternal — basic active-phase lookup ──────────────────────────── @@ -457,3 +461,958 @@ describe('#2237: ambiguous phase-directory collision', () => { assert.ok(!result.ambiguous_matches, 'single match must not set ambiguous_matches'); }); }); + +// ═══════════════════════════════════════════════════════════════════════ +// Folded from tests/fix-2830-halted-plan-dependents.test.cjs (#3335 H3 fold). +// +// #2830: a halted plan must leave its direct/transitive dependents blocked, +// never offered as ordinary runnable work. Two independent "which plans are +// incomplete" readers exist — phase.cts's cmdPhasePlanIndex (parses +// depends_on for wave assignment) and phase-locator.cts's +// searchPhaseInDir/findPhaseInternal (the phase-location primitive) — both +// must report a halted dependent as blocked while leaving the pre-existing +// incomplete/incomplete_plans fields byte-identical. computeHaltPropagation +// (plan-dependency-graph.cts) tests stay in this file: no dedicated +// plan-dependency-graph test file exists yet, and #3335 names this file as +// the fold target. +// ═══════════════════════════════════════════════════════════════════════ +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:fix-2830-halted-plan-dependents', () => { + +// Fixture: 01-01 halted spike (SUMMARY status: halted), 01-02 depends_on +// 01-01 (direct dependent), 01-03 depends_on 01-02 (transitive, 2 hops), +// 01-04 decoupled (no depends_on — the negative case). + +function writePlan(phaseDir, filename, frontmatterLines, taskLine = 'Work') { + fs.writeFileSync( + path.join(phaseDir, filename), + [ + '---', + ...frontmatterLines, + '---', + '', + `# ${filename}`, + '', + `${filename}`, + '', + taskLine, + ].join('\n'), + ); +} + +function writeSummary(phaseDir, filename, status = 'complete') { + fs.writeFileSync( + path.join(phaseDir, filename), + ['---', 'phase: 01-alpha', 'plan: 01', `status: ${status}`, 'completed: 2026-08-02', '---', '', '# Summary', ''].join('\n'), + ); +} + +function buildBaseFixture(tmpDir) { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '01-01-PLAN.md', ['wave: 1', 'objective: Halted spike', 'autonomous: true']); + writeSummary(phaseDir, '01-01-SUMMARY.md', 'halted'); + + writePlan(phaseDir, '01-02-PLAN.md', [ + 'wave: 2', 'objective: Direct dependent', 'autonomous: true', 'depends_on:', ' - 01-01', + ]); + + writePlan(phaseDir, '01-03-PLAN.md', [ + 'wave: 3', 'objective: Transitive dependent', 'autonomous: true', 'depends_on:', ' - 01-02', + ]); + + writePlan(phaseDir, '01-04-PLAN.md', ['wave: 1', 'objective: Decoupled plan', 'autonomous: true']); + + return phaseDir; +} + +describe('phase-plan-index: halt propagation (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('direct dependent of a halted plan is blocked, not runnable', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should succeed: ${result.error}`); + const data = JSON.parse(result.output); + + const p02 = data.plans.find((p) => p.id === '01-02'); + assert.ok(p02, '01-02 should be present'); + assert.deepEqual(p02.blocked_by, ['01-01'], '01-02 should be blocked by the halted 01-01'); + assert.strictEqual(p02.has_summary, false); + assert.ok(!data.runnable.includes('01-02'), '01-02 must NOT be in the runnable view'); + }); + + test('transitive dependent (2 hops) is blocked via chain', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p03 = data.plans.find((p) => p.id === '01-03'); + assert.ok(p03, '01-03 should be present'); + assert.deepEqual(p03.blocked_by, ['01-01'], '01-03 should be transitively blocked by 01-01'); + assert.ok(!data.runnable.includes('01-03'), '01-03 must NOT be in the runnable view'); + }); + + test('transitive dependent at 3 hops stays blocked', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = buildBaseFixture(tmpDir); + writePlan(phaseDir, '01-05-PLAN.md', [ + 'wave: 4', 'objective: 3-hop dependent', 'autonomous: true', 'depends_on:', ' - 01-03', + ]); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p05 = data.plans.find((p) => p.id === '01-05'); + assert.ok(p05, '01-05 should be present'); + assert.deepEqual(p05.blocked_by, ['01-01'], '01-05 should stay blocked at 3 hops'); + assert.ok(!data.runnable.includes('01-05')); + }); + + test('diamond dependency is blocked by both halted ancestors', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-diamond'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '02-01-PLAN.md', ['wave: 1', 'objective: Halted A', 'autonomous: true']); + writeSummary(phaseDir, '02-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-02-PLAN.md', ['wave: 1', 'objective: Halted B', 'autonomous: true']); + writeSummary(phaseDir, '02-02-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-03-PLAN.md', [ + 'wave: 2', 'objective: Diamond join', 'autonomous: true', + 'depends_on:', ' - 02-01', ' - 02-02', + ]); + + const result = runGsdTools(['phase-plan-index', '2', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should succeed: ${result.error}`); + const data = JSON.parse(result.output); + + const p03 = data.plans.find((p) => p.id === '02-03'); + assert.ok(p03, '02-03 should be present'); + assert.deepEqual( + [...p03.blocked_by].sort(), + ['02-01', '02-02'], + '02-03 should be blocked by BOTH halted ancestors, deduplicated', + ); + }); + + test('unrelated decoupled plan stays runnable', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p04 = data.plans.find((p) => p.id === '01-04'); + assert.ok(p04, '01-04 should be present'); + assert.deepEqual(p04.blocked_by, [], '01-04 has no depends_on, so it must not be blocked'); + assert.ok(data.runnable.includes('01-04'), '01-04 (decoupled) must stay in the runnable view'); + }); + + test('incomplete field stays byte-identical when blocked plans are present', () => { + tmpDir = createTempProject('gsd-2830-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + // Pre-#2830 semantics: incomplete = every plan without a matching SUMMARY, + // blocked or not. 01-01 has a SUMMARY (halted, but still a SUMMARY) so it + // is excluded; 01-02/01-03/01-04 have none, so all three are included — + // exactly as they would be with no halt-awareness at all. + assert.deepEqual( + [...data.incomplete].sort(), + ['01-02', '01-03', '01-04'], + 'incomplete must list every no-SUMMARY plan regardless of blocked status', + ); + }); + + test('dependency on an ordinary incomplete (non-halted) plan is not "blocked"', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-ordinary'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // 03-01 has NO summary at all (ordinary incomplete, not halted). + writePlan(phaseDir, '03-01-PLAN.md', ['wave: 1', 'objective: Ordinary unfinished plan', 'autonomous: true']); + writePlan(phaseDir, '03-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on ordinary incomplete plan', 'autonomous: true', + 'depends_on:', ' - 03-01', + ]); + + const result = runGsdTools(['phase-plan-index', '3', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '03-01'); + assert.ok(p01, '03-01 should be present'); + const p02 = data.plans.find((p) => p.id === '03-02'); + assert.ok(p02, '03-02 should be present'); + assert.deepEqual(p01.blocked_by, [], '03-01 (no summary) is not itself halted or blocked'); + assert.deepEqual(p02.blocked_by, [], '03-02 must NOT be "blocked" by an ordinary (non-halted) dependency'); + assert.ok(data.runnable.includes('03-02'), '03-02 stays runnable — only a halted upstream blocks'); + }); + + test('unresolved depends_on id is ignored, not blocked', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '04-unresolved'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '04-01-PLAN.md', [ + 'wave: 1', 'objective: References a nonexistent plan', 'autonomous: true', + 'depends_on:', ' - 99-99', + ]); + + const result = runGsdTools(['phase-plan-index', '4', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should not throw on an unresolved dependency: ${result.error}`); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '04-01'); + assert.ok(p01, '04-01 should be present'); + assert.deepEqual(p01.blocked_by, [], 'an unresolved depends_on id must not produce a spurious block'); + assert.ok(data.runnable.includes('04-01')); + }); + + test('malformed (unterminated) SUMMARY frontmatter fails open to not-halted', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '05-malformed'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '05-01-PLAN.md', ['wave: 1', 'objective: Has a malformed summary', 'autonomous: true']); + writeSummary(phaseDir, '05-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '05-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on 05-01', 'autonomous: true', 'depends_on:', ' - 05-01', + ]); + + // extractFrontmatter must fail safe on an unterminated frontmatter block + // (no closing '---'): isSummaryHalted's try/catch wraps BOTH the + // fs.readFileSync call and the extractFrontmatter call, so this exercises + // the identical fail-open catch site a genuine fs read error would hit. + fs.writeFileSync( + path.join(phaseDir, '05-01-SUMMARY.md'), + '---\nphase: 05-malformed\nplan: 01\nstatus: halted\n', // no closing '---' + ); + + const result = runGsdTools(['phase-plan-index', '5', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index must not throw on malformed frontmatter: ${result.error}`); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '05-01'); + assert.ok(p01, '05-01 should be present'); + const p02 = data.plans.find((p) => p.id === '05-02'); + assert.ok(p02, '05-02 should be present'); + assert.strictEqual(p01.halted, false, 'unterminated frontmatter must fail open to not-halted'); + assert.deepEqual(p02.blocked_by, [], 'dependent of a fail-open-not-halted plan must not be blocked'); + }); + + test('CRLF SUMMARY frontmatter still detects status: halted', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '06-crlf'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '06-01-PLAN.md', ['wave: 1', 'objective: Halted with CRLF summary', 'autonomous: true']); + fs.writeFileSync( + path.join(phaseDir, '06-01-SUMMARY.md'), + ['---', 'phase: 06-crlf', 'plan: 01', 'status: halted', 'completed: 2026-08-02', '---', '', '# Summary', ''].join('\r\n'), + ); + writePlan(phaseDir, '06-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on CRLF-summarized halt', 'autonomous: true', 'depends_on:', ' - 06-01', + ]); + + const result = runGsdTools(['phase-plan-index', '6', '--raw'], tmpDir); + assert.ok(result.success, `phase-plan-index should succeed on CRLF frontmatter: ${result.error}`); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '06-01'); + assert.ok(p01, '06-01 should be present'); + const p02 = data.plans.find((p) => p.id === '06-02'); + assert.ok(p02, '06-02 should be present'); + assert.strictEqual(p01.halted, true, 'CRLF SUMMARY frontmatter must still parse status: halted'); + assert.deepEqual(p02.blocked_by, ['06-01'], 'dependent must be blocked even when the halt was recorded with CRLF newlines'); + }); + + test('status complete does not block dependents', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '07-complete'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '07-01-PLAN.md', ['wave: 1', 'objective: Ordinary completion', 'autonomous: true']); + writeSummary(phaseDir, '07-01-SUMMARY.md', 'complete'); + writePlan(phaseDir, '07-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on completed plan', 'autonomous: true', 'depends_on:', ' - 07-01', + ]); + + const result = runGsdTools(['phase-plan-index', '7', '--raw'], tmpDir); + const data = JSON.parse(result.output); + + const p01 = data.plans.find((p) => p.id === '07-01'); + assert.ok(p01, '07-01 should be present'); + const p02 = data.plans.find((p) => p.id === '07-02'); + assert.ok(p02, '07-02 should be present'); + assert.strictEqual(p01.halted, false); + assert.deepEqual(p02.blocked_by, []); + assert.ok(data.runnable.includes('07-02')); + }); + + test('dependency cycle detection is unaffected by halt propagation', () => { + tmpDir = createTempProject('gsd-2830-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '08-cycle'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '08-01-PLAN.md', [ + 'wave: 1', 'objective: Cycle A', 'autonomous: true', 'depends_on:', ' - 08-02', + ]); + writePlan(phaseDir, '08-02-PLAN.md', [ + 'wave: 1', 'objective: Cycle B', 'autonomous: true', 'depends_on:', ' - 08-01', + ]); + + const result = runGsdTools(['phase-plan-index', '8', '--raw'], tmpDir); + // CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs" bans + // regex-matching a child process's human-readable stderr/reason prose — + // assert on the typed failure signal (`result.success`) instead. Pair it + // with a differential fixture: the identical dependency shape with the + // cycle edge removed must succeed, isolating the cycle as the cause. + assert.strictEqual(result.success, false, 'a dependency cycle must still fail the command'); + + const acyclicDir = path.join(tmpDir, '.planning', 'phases', '09-nocycle'); + fs.mkdirSync(acyclicDir, { recursive: true }); + writePlan(acyclicDir, '09-01-PLAN.md', ['wave: 1', 'objective: No cycle A', 'autonomous: true']); + writePlan(acyclicDir, '09-02-PLAN.md', [ + 'wave: 1', 'objective: No cycle B', 'autonomous: true', 'depends_on:', ' - 09-01', + ]); + const acyclicResult = runGsdTools(['phase-plan-index', '9', '--raw'], tmpDir); + assert.strictEqual( + acyclicResult.success, + true, + `the identical dependency shape without the cycle edge must succeed, isolating the cycle as the cause of the failure above: ${acyclicResult.error}`, + ); + }); +}); + +describe('findPhaseInternal: halt propagation (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('direct dependent of a halted plan is blocked', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.ok(result, 'expected a result'); + assert.deepEqual(result.blocked_by['01-02-PLAN.md'], ['01-01'], '01-02 should be blocked by halted 01-01'); + assert.ok(!result.runnable_plans.includes('01-02-PLAN.md')); + }); + + test('transitive dependent is blocked via chain', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.deepEqual(result.blocked_by['01-03-PLAN.md'], ['01-01'], '01-03 should be transitively blocked'); + assert.ok(!result.runnable_plans.includes('01-03-PLAN.md')); + }); + + test('diamond dependency blocked by both halted ancestors', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '02-diamond'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '02-01-PLAN.md', ['wave: 1', 'objective: Halted A', 'autonomous: true']); + writeSummary(phaseDir, '02-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-02-PLAN.md', ['wave: 1', 'objective: Halted B', 'autonomous: true']); + writeSummary(phaseDir, '02-02-SUMMARY.md', 'halted'); + writePlan(phaseDir, '02-03-PLAN.md', [ + 'wave: 2', 'objective: Diamond join', 'autonomous: true', + 'depends_on:', ' - 02-01', ' - 02-02', + ]); + + const result = phaseLocator.findPhaseInternal(tmpDir, '2'); + assert.ok(result, 'expected a result'); + assert.deepEqual( + [...result.blocked_by['02-03-PLAN.md']].sort(), + ['02-01', '02-02'], + 'diamond join should be blocked by both halted ancestors, deduplicated', + ); + }); + + test('unrelated decoupled plan stays runnable', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.ok(result.runnable_plans.includes('01-04-PLAN.md'), '01-04 (decoupled) must stay runnable'); + assert.strictEqual(result.blocked_by['01-04-PLAN.md'], undefined); + }); + + test('incomplete_plans stays byte-identical when blocked plans are present', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.deepEqual( + [...result.incomplete_plans].sort(), + ['01-02-PLAN.md', '01-03-PLAN.md', '01-04-PLAN.md'], + 'incomplete_plans must list every no-SUMMARY plan regardless of blocked status', + ); + }); + + test('halted_plans reports the halted plan itself by filename', () => { + tmpDir = createTempProject('gsd-2830-pl-'); + buildBaseFixture(tmpDir); + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.deepEqual(result.halted_plans, ['01-01-PLAN.md']); + }); +}); + +describe('parity: phase-plan-index and findPhaseInternal agree on blocking (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('same fixture yields the same blocked-plan set and cause set from both readers', () => { + tmpDir = createTempProject('gsd-2830-parity-'); + buildBaseFixture(tmpDir); + + const cliResult = runGsdTools(['phase-plan-index', '1', '--raw'], tmpDir); + assert.ok(cliResult.success, `phase-plan-index should succeed: ${cliResult.error}`); + const cliData = JSON.parse(cliResult.output); + + const locatorResult = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.ok(locatorResult, 'findPhaseInternal should return a result'); + + // Build { planId -> sorted cause list } from each reader and require them + // to be structurally identical (id-mapped, since one reader keys by bare + // id and the other by filename). + const cliBlocked = {}; + for (const plan of cliData.plans) { + if (plan.blocked_by.length > 0) cliBlocked[plan.id] = [...plan.blocked_by].sort(); + } + + const locatorBlocked = {}; + for (const [filename, causes] of Object.entries(locatorResult.blocked_by)) { + const planId = filename.replace(/-PLAN\.md$/i, '').replace(/^PLAN\.md$/i, ''); + locatorBlocked[planId] = [...causes].sort(); + } + + assert.deepEqual( + locatorBlocked, + cliBlocked, + 'phase-plan-index and findPhaseInternal must report the exact same blocked-plan-id -> cause-set mapping', + ); + }); +}); + +describe('computeHaltPropagation: graph invariants (#2830)', () => { + test('a node with no depends_on and not halted is never blocked', () => { + const { blockedBy } = planDependencyGraph.computeHaltPropagation([ + { id: 'A', resolvedDependsOn: [], halted: false }, + ]); + assert.strictEqual(blockedBy.get('A'), undefined); + }); + + test('a halted node is never blocked by itself', () => { + const { blockedBy } = planDependencyGraph.computeHaltPropagation([ + { id: 'A', resolvedDependsOn: [], halted: true }, + ]); + assert.strictEqual(blockedBy.get('A'), undefined, 'a halted plan is halted, not "blocked"'); + }); + + test('precomputedOrder (the phase.cts call shape) yields the same result as self-derived order', () => { + // phase.cts's cmdPhasePlanIndex passes computeDependencyLevels's own + // topological order in as `precomputedOrder` so this function does not + // re-run Kahn's algorithm. Assert both call shapes agree. + const nodes = [ + { id: 'A', resolvedDependsOn: [], halted: true }, + { id: 'B', resolvedDependsOn: ['A'], halted: false }, + { id: 'C', resolvedDependsOn: ['B'], halted: false }, + ]; + const selfDerived = planDependencyGraph.computeHaltPropagation(nodes); + const withPrecomputed = planDependencyGraph.computeHaltPropagation(nodes, ['A', 'B', 'C']); + assert.deepEqual([...withPrecomputed.blockedBy.entries()], [...selfDerived.blockedBy.entries()]); + assert.deepEqual(withPrecomputed.order, ['A', 'B', 'C']); + assert.strictEqual(withPrecomputed.visited, 3); + }); + + // Random DAG over N nodes: edges only point from a lower index to a higher + // index (guarantees acyclicity by construction, independent of the code + // under test), with a random halted flag per node. + // + // Acyclic BY CONSTRUCTION: `to` is always drawn strictly above `from`, so + // no `.filter()` is involved. A filter here is not merely slower — with + // n === 1 the predicate `from < to` is unsatisfiable and fast-check retries + // generation forever, which hung the whole suite (the runner sets + // --test-timeout=0, so it never dies). Hoisted to describe scope so the + // regression test below can sample the identical arbitrary. + const dagArb = fc.integer({ min: 1, max: 12 }).chain((n) => { + const ids = Array.from({ length: n }, (_, i) => `N${i}`); + const edgeArb = n < 2 + ? fc.constant([]) + : fc.array( + fc.integer({ min: 0, max: n - 2 }).chain((from) => + fc.integer({ min: from + 1, max: n - 1 }).map((to) => ({ from, to }))), + { maxLength: n * 2 }, + ); + const haltedArb = fc.array(fc.boolean(), { minLength: n, maxLength: n }); + return fc.record({ ids: fc.constant(ids), edges: edgeArb, halted: haltedArb }); + }); + + test('fast-check — blocked set matches reachability from halted nodes', () => { + fc.assert( + fc.property(dagArb, ({ ids, edges, halted }) => { + const dependsOn = new Map(ids.map((id) => [id, []])); + for (const { from, to } of edges) { + dependsOn.get(ids[from]).push(ids[to]); + } + const nodes = ids.map((id, i) => ({ + id, + resolvedDependsOn: dependsOn.get(id), + halted: halted[i], + })); + + const { blockedBy } = planDependencyGraph.computeHaltPropagation(nodes); + + // Reference model: reachability via depends_on edges from a halted node. + const haltedSet = new Set(nodes.filter((n) => n.halted).map((n) => n.id)); + const dependsOnMap = new Map(nodes.map((n) => [n.id, n.resolvedDependsOn])); + function reachableHaltedCauses(id, seen = new Set()) { + const causes = new Set(); + for (const dep of dependsOnMap.get(id) ?? []) { + if (seen.has(dep)) continue; + seen.add(dep); + if (haltedSet.has(dep)) causes.add(dep); + for (const c of reachableHaltedCauses(dep, seen)) causes.add(c); + } + return causes; + } + + for (const n of nodes) { + const expected = [...reachableHaltedCauses(n.id)].sort(); + const actual = [...(blockedBy.get(n.id) ?? [])].sort(); + assert.deepEqual( + actual, + expected, + `node ${n.id}: computeHaltPropagation blockedBy must equal halted-reachability`, + ); + } + }), + { numRuns: 50, seed: 7 }, + ); + }); + + test('the DAG generator terminates on the degenerate single-node case (regression: unsatisfiable filter hung the suite)', () => { + // Bounded, non-hanging sample: if edgeArb regresses to a `.filter(from < to)` + // over a forced-equal {from, to} pair (n === 1), fast-check would retry + // generation forever and this assertion would never run. + const samples = fc.sample(dagArb, { numRuns: 20, seed: 7 }); + assert.ok(samples.length === 20, 'fc.sample must return the requested number of samples without hanging'); + const singleNodeSamples = samples.filter(({ ids }) => ids.length === 1); + assert.ok(singleNodeSamples.length > 0, 'the sample must include at least one degenerate single-node case'); + for (const { edges } of singleNodeSamples) { + assert.deepEqual(edges, [], 'the single-node case must yield an empty edge list, not an unsatisfiable filter'); + } + }); +}); + +// init execute-phase (cmdInitExecutePhase, src/init.cts): the locator already +// computed halted_plans/blocked_by/runnable_plans, but the command built its +// output by explicit field enumeration, silently dropping all three. This +// drives the real CLI end to end (not the locator directly) to prove the +// passthrough. +describe('init execute-phase: halt propagation passthrough (#2830)', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('halted_plans, blocked_by, runnable_plans are forwarded; incomplete fields stay byte-identical', () => { + tmpDir = createTempProject('gsd-2830-init-'); + buildBaseFixture(tmpDir); + + const result = runGsdTools(['init', 'execute-phase', '1', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + + // Guard: if the phase was not resolved, every assertion below is vacuous. + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + json.halted_plans.includes('01-01-PLAN.md'), + 'halted_plans must include the halted plan', + ); + + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '01-02-PLAN.md'), + 'blocked_by must have an entry for the direct dependent', + ); + assert.deepEqual( + json.blocked_by['01-02-PLAN.md'], + ['01-01'], + "blocked_by['01-02-PLAN.md'] must name 01-01 as the blocking cause", + ); + + assert.ok( + !json.runnable_plans.includes('01-02-PLAN.md'), + 'the dependent must NOT be offered as runnable work', + ); + assert.ok( + json.runnable_plans.includes('01-04-PLAN.md'), + 'the decoupled plan (no dependency on the halted plan) must stay runnable', + ); + + // Back-compat: incomplete_plans/incomplete_count must be exactly what they + // were pre-#2830 — every no-SUMMARY plan, blocked or not, still counted. + assert.deepEqual( + [...json.incomplete_plans].sort(), + ['01-02-PLAN.md', '01-03-PLAN.md', '01-04-PLAN.md'], + 'incomplete_plans must be unchanged by halt-awareness', + ); + assert.strictEqual( + json.incomplete_count, + 3, + 'incomplete_count must be unchanged by halt-awareness', + ); + }); +}); + +// Adversarial review of the #2830 fix found two confirmed defects: +// 1. (BLOCKER) computeHaltPropagation's Kahn pass silently drops cycle +// participants (and anything downstream of them) from BOTH `order` and +// `blockedBy` — phase.cts hard-fails on a cycle before this ever +// matters, but phase-locator.cts (consumed by `init execute-phase`) +// does not pre-check, so a plan directly depends_on-ing a halted plan +// inside a cycle was reported as ordinary runnable. +// 2. (MAJOR) the summary templates presented `status: halted` as a +// trailing `#`-comment on the value line, but `extractFrontmatter` +// does not strip trailing YAML comments — an executor mimicking the +// template's own presentation wrote a halt that silently read back as +// not-halted. +describe('#2830 review findings', () => { + let tmpDir; + afterEach(() => { if (tmpDir) { cleanup(tmpDir); tmpDir = null; } }); + + test('defect 1: cycle repro — 08-02 depends_on [halted 08-01, 08-03]; 08-03 depends_on [08-02]', () => { + tmpDir = createTempProject('gsd-2830-review-cycle-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '08-cyclehalt'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '08-01-PLAN.md', ['wave: 1', 'objective: Halted upstream', 'autonomous: true']); + writeSummary(phaseDir, '08-01-SUMMARY.md', 'halted'); + writePlan(phaseDir, '08-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on halted + cyclic peer', 'autonomous: true', + 'depends_on:', ' - 08-01', ' - 08-03', + ]); + writePlan(phaseDir, '08-03-PLAN.md', [ + 'wave: 2', 'objective: Cyclic peer of 08-02', 'autonomous: true', 'depends_on:', ' - 08-02', + ]); + + const result = runGsdTools(['init', 'execute-phase', '8', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + !json.runnable_plans.includes('08-02-PLAN.md'), + 'a plan that directly depends_on a halted plan must never be offered as runnable, cycle or not', + ); + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '08-02-PLAN.md'), + 'a plan must never silently vanish from both runnable_plans and blocked_by', + ); + assert.ok( + Array.isArray(json.blocked_by['08-02-PLAN.md']) && json.blocked_by['08-02-PLAN.md'].length > 0, + 'blocked_by entry must be non-empty, not a vacuous placeholder', + ); + }); + + test('defect 1: self-dependency (A depends_on A, nothing halted) must not be silently runnable', () => { + tmpDir = createTempProject('gsd-2830-review-self-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '09-selfdep'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '09-01-PLAN.md', [ + 'wave: 1', 'objective: Depends on itself', 'autonomous: true', 'depends_on:', ' - 09-01', + ]); + + const result = runGsdTools(['init', 'execute-phase', '9', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + !json.runnable_plans.includes('09-01-PLAN.md'), + 'a self-dependent plan must not be silently offered as runnable', + ); + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '09-01-PLAN.md'), + 'a self-dependent plan must never silently vanish from both runnable_plans and blocked_by', + ); + }); + + test('defect 2: SUMMARY status with an inline YAML comment still blocks the dependent', () => { + tmpDir = createTempProject('gsd-2830-review-comment-'); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '10-inlinecomment'); + fs.mkdirSync(phaseDir, { recursive: true }); + + writePlan(phaseDir, '10-01-PLAN.md', ['wave: 1', 'objective: Halted, recorded with an inline comment', 'autonomous: true']); + writeSummary(phaseDir, '10-01-SUMMARY.md', 'halted # designed stop'); + writePlan(phaseDir, '10-02-PLAN.md', [ + 'wave: 2', 'objective: Depends on the inline-commented halt', 'autonomous: true', 'depends_on:', ' - 10-01', + ]); + + const result = runGsdTools(['init', 'execute-phase', '10', '--raw'], tmpDir); + assert.ok(result.success, `init execute-phase should succeed: ${result.error}`); + const json = JSON.parse(result.output); + assert.strictEqual(json.phase_found, true, 'phase must be found for the rest of this test to be meaningful'); + + assert.ok( + json.halted_plans.includes('10-01-PLAN.md'), + 'status: halted # designed stop must still be recognized as halted', + ); + assert.ok( + Object.prototype.hasOwnProperty.call(json.blocked_by, '10-02-PLAN.md'), + 'the dependent of an inline-commented halt must be blocked', + ); + assert.ok( + !json.runnable_plans.includes('10-02-PLAN.md'), + 'the dependent of an inline-commented halt must not be offered as runnable', + ); + }); + + test('defect 2: isHaltedStatus strips an unquoted trailing YAML comment before comparing', () => { + const { isHaltedStatus } = planDependencyGraph; + assert.strictEqual(isHaltedStatus('halted'), true); + assert.strictEqual(isHaltedStatus('Halted'), true); + assert.strictEqual(isHaltedStatus('halted '), true); + assert.strictEqual(isHaltedStatus('halted # designed stop'), true); + assert.strictEqual(isHaltedStatus('halted#nospace'), false, 'a `#` with no preceding whitespace is not a YAML comment'); + assert.strictEqual(isHaltedStatus('complete'), false); + assert.strictEqual(isHaltedStatus('complete # done'), false); + assert.strictEqual(isHaltedStatus(''), false); + assert.strictEqual(isHaltedStatus(undefined), false); + }); +}); + + }); +} + +// ═══════════════════════════════════════════════════════════════════════ +// Folded from tests/fix-2855-phase-locator-workstream-archive-scope.test.cjs +// (#3335 H3 fold). +// +// #2855: the archived-milestone fallback hardcoded the project-root +// .planning/milestones/ tree instead of routing through the +// workstream-aware planningDir(cwd) helper (the same seam the active-phase +// search and the archive-write path already use). A pending phase in one +// workstream, whose own phases/ dir does not exist yet, would silently +// resolve to a same-numbered phase archived under the ROOT tree (an +// unrelated workstream's history, or a flat-mode project's archive). +// +// GSD_WORKSTREAM/GSD_PROJECT are read directly from process.env by +// planningDir() when omitted, so every test here explicitly saves/restores +// both to avoid leaking state across tests or picking up ambient shell env. +// ═══════════════════════════════════════════════════════════════════════ +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:fix-2855-phase-locator-workstream-archive-scope', () => { + +describe('#2855: findPhaseInternal does not leak cross-workstream archived phases', () => { + let tmpDir; + beforeEach(() => { isolateWorkstreamEnv(); }); + afterEach(() => { + restoreWorkstreamEnv(); + if (tmpDir) { cleanup(tmpDir); tmpDir = null; } + }); + + test('does not leak root-tree archived phase into an unrelated workstream', () => { + tmpDir = createTempProject('gsd-2855-'); + // Root archive holds phase 03 (unrelated workstream's / flat-mode history). + const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-legacy'); + fs.mkdirSync(rootArchive, { recursive: true }); + fs.writeFileSync(path.join(rootArchive, 'SOME-SUMMARY.md'), '# stale'); + + // Workstream "beta" exists with an empty phases/ dir — phase 03 is pending. + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'beta', 'phases'), { recursive: true }); + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.findPhaseInternal(tmpDir, '3'); + assert.strictEqual(result, null, 'pending workstream phase must not resolve to the root archive'); + }); + + test('does not leak root archive when workstream phases dir is entirely absent', () => { + tmpDir = createTempProject('gsd-2855-'); + const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v2.0-phases', '01-legacy'); + fs.mkdirSync(rootArchive, { recursive: true }); + + // Brand-new workstream: no .planning/workstreams/gamma/ directory at all yet. + process.env.GSD_WORKSTREAM = 'gamma'; + + const result = phaseLocator.findPhaseInternal(tmpDir, '1'); + assert.strictEqual(result, null, 'a workstream with no directory yet must not resolve to the root archive'); + }); + + // Issue #2855 AC1: "...regardless of whether workstream A's roadmap already + // lists the phase and when it doesn't yet." findPhaseInternal never reads + // ROADMAP.md (it is a pure filesystem lookup), so this dimension cannot + // change its behavior — demonstrated directly rather than left as an + // inference from reading the source. + for (const roadmapHasEntry of [true, false]) { + test(`does not leak root archive whether or not the workstream's ROADMAP.md already lists the phase (roadmapHasEntry=${roadmapHasEntry})`, () => { + tmpDir = createTempProject('gsd-2855-'); + const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-legacy'); + fs.mkdirSync(rootArchive, { recursive: true }); + + const wsDir = path.join(tmpDir, '.planning', 'workstreams', 'beta'); + fs.mkdirSync(path.join(wsDir, 'phases'), { recursive: true }); + if (roadmapHasEntry) { + fs.writeFileSync( + path.join(wsDir, 'ROADMAP.md'), + ['# Roadmap', '', '### Phase 03: Pending Work', ''].join('\n'), + ); + } + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.findPhaseInternal(tmpDir, '3'); + assert.strictEqual(result, null, 'ROADMAP.md presence/absence must not affect the archive-leak guard'); + }); + } + + test('still finds a phase genuinely archived under the active workstream\'s own tree', () => { + tmpDir = createTempProject('gsd-2855-'); + const ownArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '03-real'); + fs.mkdirSync(ownArchive, { recursive: true }); + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.findPhaseInternal(tmpDir, '3'); + assert.ok(result !== null, 'workstream\'s own archived phase must still resolve'); + assert.strictEqual(result.found, true); + assert.strictEqual(result.archived, 'v1.0'); + assert.strictEqual( + result.directory, + '.planning/workstreams/beta/milestones/v1.0-phases/03-real', + ); + }); + + test('flat/non-workstream project archive resolution is unchanged', () => { + tmpDir = createTempProject('gsd-2855-'); + const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-flat'); + fs.mkdirSync(rootArchive, { recursive: true }); + // No GSD_WORKSTREAM / GSD_PROJECT set — flat mode. + + const result = phaseLocator.findPhaseInternal(tmpDir, '3'); + assert.ok(result !== null, 'flat-mode archive lookup must be unaffected by the fix'); + assert.strictEqual(result.found, true); + assert.strictEqual(result.archived, 'v1.0'); + assert.strictEqual(result.directory, '.planning/milestones/v1.0-phases/03-flat'); + }); + + test('two workstreams with same-numbered archived phases never cross-resolve', () => { + tmpDir = createTempProject('gsd-2855-'); + const alphaArchive = path.join(tmpDir, '.planning', 'workstreams', 'alpha', 'milestones', 'v1.0-phases', '03-alpha-work'); + const betaArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '03-beta-work'); + fs.mkdirSync(alphaArchive, { recursive: true }); + fs.mkdirSync(betaArchive, { recursive: true }); + + process.env.GSD_WORKSTREAM = 'alpha'; + const alphaResult = phaseLocator.findPhaseInternal(tmpDir, '3'); + assert.ok(alphaResult !== null); + assert.strictEqual(alphaResult.phase_name, 'alpha-work'); + + process.env.GSD_WORKSTREAM = 'beta'; + const betaResult = phaseLocator.findPhaseInternal(tmpDir, '3'); + assert.ok(betaResult !== null); + assert.strictEqual(betaResult.phase_name, 'beta-work'); + }); + + test('project+workstream combination scopes the archive fallback', () => { + tmpDir = createTempProject('gsd-2855-'); + const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '05-root-legacy'); + fs.mkdirSync(rootArchive, { recursive: true }); + + const scopedArchive = path.join(tmpDir, '.planning', 'proj-x', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '05-scoped'); + fs.mkdirSync(scopedArchive, { recursive: true }); + + process.env.GSD_PROJECT = 'proj-x'; + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.findPhaseInternal(tmpDir, '5'); + assert.ok(result !== null, 'project+workstream scoped archive must resolve'); + assert.strictEqual(result.phase_name, 'scoped'); + assert.strictEqual( + result.directory, + '.planning/proj-x/workstreams/beta/milestones/v1.0-phases/05-scoped', + ); + }); + + test('workstream-scoped archived directory is posix-style', () => { + tmpDir = createTempProject('gsd-2855-'); + const ownArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v1.0-phases', '07-posix'); + fs.mkdirSync(ownArchive, { recursive: true }); + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.findPhaseInternal(tmpDir, '7'); + assert.ok(result !== null); + assert.ok(!result.directory.includes('\\'), 'directory must use forward slashes on every platform'); + }); +}); + +describe('#2855: getArchivedPhaseDirs does not leak cross-workstream archived phases', () => { + let tmpDir; + beforeEach(() => { isolateWorkstreamEnv(); }); + afterEach(() => { + restoreWorkstreamEnv(); + if (tmpDir) { cleanup(tmpDir); tmpDir = null; } + }); + + test('does not leak root-tree archive entries under an active workstream', () => { + tmpDir = createTempProject('gsd-2855-'); + const rootArchive = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '03-legacy'); + fs.mkdirSync(rootArchive, { recursive: true }); + + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'beta', 'phases'), { recursive: true }); + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.getArchivedPhaseDirs(tmpDir); + assert.deepEqual(result, [], 'getArchivedPhaseDirs must not surface the root archive for a workstream'); + }); + + test('still finds phases genuinely archived under the active workstream\'s own tree', () => { + tmpDir = createTempProject('gsd-2855-'); + const ownArchive = path.join(tmpDir, '.planning', 'workstreams', 'beta', 'milestones', 'v2.0-phases', '04-own'); + fs.mkdirSync(ownArchive, { recursive: true }); + process.env.GSD_WORKSTREAM = 'beta'; + + const result = phaseLocator.getArchivedPhaseDirs(tmpDir); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].name, '04-own'); + assert.strictEqual(result[0].milestone, 'v2.0'); + // basePath is posix-normalized (toPosixPath) — a forward-slash literal is + // the correct cross-platform expectation, not path.join. + assert.strictEqual( + result[0].basePath, + '.planning/workstreams/beta/milestones/v2.0-phases', + ); + }); + + test('flat/non-workstream project resolution is unchanged', () => { + tmpDir = createTempProject('gsd-2855-'); + const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v2.1.0-phases'); + fs.mkdirSync(path.join(archiveDir, '03-auth'), { recursive: true }); + // No GSD_WORKSTREAM / GSD_PROJECT set — flat mode. + + const result = phaseLocator.getArchivedPhaseDirs(tmpDir); + assert.strictEqual(result.length, 1); + const entry = result[0]; + assert.strictEqual(entry.name, '03-auth'); + assert.strictEqual(entry.milestone, 'v2.1.0'); + // basePath is posix-normalized (toPosixPath) — a forward-slash literal is + // the correct cross-platform expectation, not path.join. + assert.strictEqual(entry.basePath, '.planning/milestones/v2.1.0-phases'); + assert.strictEqual(entry.fullPath, path.join(archiveDir, '03-auth')); + }); +}); + + }); +} diff --git a/tests/fix-2068-resolve-execution-dynamic-routing.test.cjs b/tests/resolve-execution-dynamic-routing.test.cjs similarity index 100% rename from tests/fix-2068-resolve-execution-dynamic-routing.test.cjs rename to tests/resolve-execution-dynamic-routing.test.cjs diff --git a/tests/fix-2138-ship-note-lost-on-merge.test.cjs b/tests/ship-note.test.cjs similarity index 100% rename from tests/fix-2138-ship-note-lost-on-merge.test.cjs rename to tests/ship-note.test.cjs diff --git a/tests/fix-1700-spike-manifest-idea-scoping.test.cjs b/tests/spike-manifest-scoping.test.cjs similarity index 100% rename from tests/fix-1700-spike-manifest-idea-scoping.test.cjs rename to tests/spike-manifest-scoping.test.cjs diff --git a/tests/verification-status.test.cjs b/tests/verification-status.test.cjs index 5c122825c..68784ffa4 100644 --- a/tests/verification-status.test.cjs +++ b/tests/verification-status.test.cjs @@ -1201,3 +1201,120 @@ describe('#2868: verification status CLI drives the execute-phase stranded-phase } }); }); + +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:fix-3174-quick-verification-status-read', () => { + // allow-test-rule: source-text-is-the-product see #3174 + // Workflow .md / agent .md / command .md / reference .md files — their text + // IS what the runtime loads. Testing text content tests the deployed contract. + // Per CONTRIBUTING.md exception matrix. + // + // #3174: quick's verification step used to read the verifier's result with a + // raw `grep "^status:" F | cut -d: -f2 | tr -d ' '` and route it through arms + // passed / human_needed / gaps_found only. That read failed two ways, both + // measured against the old pipeline: it matched NO arm on a missing report, + // most off-schema values, a `status:` line in both frontmatter and prose, or + // (on a CRLF checkout) a valid `passed` arriving as `passed\r`; and it + // matched the SUCCESS arm when it should not have on a stale `passed` + // report (staleness was never evaluated), a report whose only `status:` line + // sits in its prose, or an off-schema value carrying a colon + // (`passed:bogus`), which `cut -d: -f2` splits at that colon. The unanchored + // match is the DEFECT.FRONTMATTER-SCALAR-BROAD-GREP class the code side + // already fixed by name. These tests pin the five properties that keep the + // replacement honest. + describe('quick verification status read (#3174)', () => { + const QUICK_VERIFICATION = path.join( + __dirname, '..', 'gsd-core', 'workflows', 'quick', 'steps', 'quick-verification.md', + ); + // The canonical launcher preamble. scripts/sync-runtime-launcher.cjs rewrites + // every workflow's bootstrap from this file, so THIS is the authority — not + // whichever sibling step file happens to carry a copy today. + const LAUNCHER_SNIPPET = path.join( + __dirname, '..', 'gsd-core', 'workflows', '_runtime-launcher.snippet.sh', + ); + + const SHIM_ANCHOR = '_GSD_SHIM_NAME="gsd-tools.cjs"'; + + test('status is read through the canonical query, not a raw frontmatter grep', () => { + const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); + const queryIdx = content.indexOf('gsd_run query verification.status "${QUICK_DIR}"'); + + assert.ok(queryIdx !== -1, 'quick-verification.md must read status via the verification.status query'); + assert.ok( + !content.includes('grep "^status:"'), + 'the raw frontmatter-scalar grep must not return — it matches body lines too (DEFECT.FRONTMATTER-SCALAR-BROAD-GREP)', + ); + }); + + test('the query call is preceded by the runtime shim bootstrap in this step file', () => { + // Step files are read and executed as their own units, so quick.md's + // bootstrap does not reach here. Without this the call resolves to + // nothing, 2>/dev/null swallows it, and the default arm is taken forever. + const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); + const shimIdx = content.indexOf(SHIM_ANCHOR); + const queryIdx = content.indexOf('gsd_run query verification.status'); + + assert.ok(shimIdx !== -1, 'the step file must carry its own runtime shim bootstrap'); + assert.ok(queryIdx > shimIdx, 'the shim bootstrap must precede the gsd_run call'); + }); + + test('the shim bootstrap is the canonical launcher preamble, not a fork of it', () => { + // Anchored on _runtime-launcher.snippet.sh rather than on a sibling step + // file: sync-runtime-launcher.cjs regenerates every workflow from the + // snippet, so a synchronized launcher update keeps this green (correct), + // and a sibling that legitimately stops calling gsd_run cannot fail us. + const lineWithShim = (file) => fs.readFileSync(file, 'utf-8') + .split(/\r?\n/) + .find((line) => line.startsWith(SHIM_ANCHOR)); + + const mine = lineWithShim(QUICK_VERIFICATION); + const canonical = lineWithShim(LAUNCHER_SNIPPET); + + assert.ok(canonical, '_runtime-launcher.snippet.sh must carry the canonical preamble'); + assert.equal(mine, canonical, 'the bootstrap must match the canonical launcher snippet verbatim'); + }); + + test('status extraction does not depend on jq', () => { + // #2589: a `| jq -r '.field'` pipe yields an empty variable with no + // diagnostic wherever jq is absent (the Windows/Git-Bash default), which + // would route a passing verification into the recovery arm. + // + // Scoped to the executable fence on purpose: the surrounding prose cites + // the jq form in order to explain why it is not used, and an assertion + // over the whole file would fire on its own rationale. + const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); + const fences = content.match(/```bash\r?\n[\s\S]*?```/g) || []; + const statusFence = fences.find((f) => f.includes('gsd_run query verification.status')); + + assert.ok(statusFence, 'the status read must live in a bash fence'); + assert.ok( + statusFence.includes('--pick status'), + 'the bare status must be picked by the query itself', + ); + assert.ok(!/\|\s*jq\b/.test(statusFence), 'the status-read fence must not pipe through jq'); + }); + + test('the routing table carries a terminal arm for missing / unknown / stale', () => { + const content = fs.readFileSync(QUICK_VERIFICATION, 'utf-8'); + const gapsIdx = content.indexOf('| `gaps_found` |'); + const fallbackIdx = content.indexOf('| anything else'); + + assert.ok(gapsIdx !== -1, 'the three verifier-status arms must remain'); + assert.ok(fallbackIdx > gapsIdx, 'a terminal arm must follow the verifier-status arms'); + + const fallbackRow = content.slice(fallbackIdx, content.indexOf('\n', fallbackIdx)); + for (const sentinel of ['missing', 'unknown', 'stale']) { + assert.ok( + fallbackRow.includes(sentinel), + `the terminal arm must name the ${sentinel} sentinel the query can return`, + ); + } + assert.ok( + fallbackRow.includes('VERIFICATION_STATUS'), + 'the terminal arm must set the display string consumed by the quick index row and banner', + ); + }); + }); + }); +} diff --git a/tests/fix-2589-workflow-jq-dependency.test.cjs b/tests/workflow-jq-dependency.test.cjs similarity index 100% rename from tests/fix-2589-workflow-jq-dependency.test.cjs rename to tests/workflow-jq-dependency.test.cjs diff --git a/tests/worktree-base-ref.test.cjs b/tests/worktree-base-ref.test.cjs index 9044767b4..ceffdb348 100644 --- a/tests/worktree-base-ref.test.cjs +++ b/tests/worktree-base-ref.test.cjs @@ -14,6 +14,7 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('node:fs'); const path = require('node:path'); const { makeFaultyGit } = require('./helpers/faulty-deps.cjs'); @@ -1037,3 +1038,199 @@ describe('cmdWorktreeBaseCheck — user/global cascade (#1013)', () => { assert.strictEqual(result.reason, 'fork-ref-unknown'); }); }); + +// ─── workflow dispatch-site coverage: worktree.base-check gates (folded from +// fix-1941-quick-worktree-stale-base.test.cjs and +// fix-2649-diagnose-issues-worktree-stale-base.test.cjs, #3335) ────────────── +// +// allow-test-rule: source-text-is-the-product #1941 #2649 +// Workflow .md files are the installed AI instructions — their text IS what the +// runtime loads. Testing text content tests the deployed contract. Per +// CONTRIBUTING.md exception matrix. +// +// Root cause shared by #1941 and #2649: Claude Code's isolation="worktree" +// forks new worktrees from origin/HEAD, not the live local HEAD. When prior +// local commits advance local HEAD without an intervening `git push`, +// origin/HEAD stays pinned to a stale ancestor and the executor's +// worktree_branch_check guard halts with a base-mismatch fatal. The fix ports +// the worktree.base-check auto-degrade pattern (originally execute-phase +// #683/#1369) into each not-yet-covered dispatch site: quick.md's +// single-dispatch path (#1941), and diagnose-issues.md's spawn_agents step +// plus execute-plan.md's Pattern A single-plan dispatch (#2649, fixed +// together per that bug's acceptance criterion 5 — same bug class, same +// one-line gate). + +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:fix-1941-quick-worktree-stale-base', () => { + +const QUICK_WORKFLOW_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'quick.md'); + +describe('quick: pre-dispatch worktree base re-check (#1941)', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(QUICK_WORKFLOW_PATH), 'workflows/quick.md should exist'); + }); + + test('Step 6 runs worktree.base-check before capturing EXPECTED_BASE', () => { + const content = fs.readFileSync(QUICK_WORKFLOW_PATH, 'utf-8'); + const step6Idx = content.indexOf('**Step 6: Spawn executor**'); + const baseCheckIdx = content.indexOf('worktree.base-check', step6Idx); + const expectedBaseIdx = content.indexOf('EXPECTED_BASE=$(git rev-parse HEAD)', step6Idx); + assert.ok(step6Idx !== -1, '"Step 6: Spawn executor" must exist in quick.md'); + assert.ok(baseCheckIdx !== -1, 'worktree.base-check must be invoked within Step 6'); + assert.ok(expectedBaseIdx !== -1, 'EXPECTED_BASE capture must exist within Step 6'); + assert.ok( + baseCheckIdx < expectedBaseIdx, + 'worktree.base-check must run BEFORE EXPECTED_BASE is captured so the degrade decision reflects the most current local HEAD' + ); + }); + + test('degrade check references #1941 for traceability', () => { + const content = fs.readFileSync(QUICK_WORKFLOW_PATH, 'utf-8'); + assert.ok(content.includes('#1941'), 'quick.md must reference #1941'); + }); + + test('degrade check clears BOTH USE_WORKTREES and ISOLATION when shouldDegrade is true', () => { + const content = fs.readFileSync(QUICK_WORKFLOW_PATH, 'utf-8'); + const baseCheckIdx = content.indexOf('worktree.base-check'); + const block = content.slice(baseCheckIdx, baseCheckIdx + 900); + assert.ok(block.includes('shouldDegrade'), 'degrade check must branch on shouldDegrade'); + // Both must move together (#2652). Dispatch keys on ISOLATION while the prompt + // guard and worktree manifest key on USE_WORKTREES; clearing only one dispatches + // an isolated executor with no base guard and no manifest, then blocks in cleanup + // looking for a manifest that was never initialized. + assert.ok(block.includes('USE_WORKTREES=false'), 'degrade must set USE_WORKTREES=false'); + assert.ok( + block.includes('ISOLATION=none'), + 'degrade must ALSO set ISOLATION=none — dispatch reads ISOLATION, so clearing only ' + + 'USE_WORKTREES still passes the harness isolation flag (#2652)' + ); + }); + + // #2652: this assertion previously required `RUNTIME = "claude"`, encoding the + // pre-#2584 premise that worktree isolation is Claude-specific. #2584 replaced + // that with the negotiated dispatch.isolation capability, so the guard now keys + // on the capability — Cursor also declares harness-worktree. + test('degrade check guards on the negotiated capability, not a runtime id', () => { + const content = fs.readFileSync(QUICK_WORKFLOW_PATH, 'utf-8'); + const baseCheckIdx = content.indexOf('worktree.base-check'); + const block = content.slice(Math.max(0, baseCheckIdx - 300), baseCheckIdx + 200); + assert.ok( + block.includes('ISOLATION') && block.includes('harness-worktree'), + 'degrade check must guard on ISOLATION = harness-worktree' + ); + assert.ok( + !/\[\s*"\$RUNTIME"\s*=/.test(block), + 'degrade check must NOT branch on a RUNTIME literal (#2584/#2652)' + ); + }); + + test('degrade check names origin/HEAD as the stale fork base', () => { + const content = fs.readFileSync(QUICK_WORKFLOW_PATH, 'utf-8'); + const step6Idx = content.indexOf('**Step 6: Spawn executor**'); + const nextSection = content.indexOf('\n---', step6Idx); + const section = content.slice(step6Idx, nextSection === -1 ? undefined : nextSection); + assert.ok(section.includes('origin/HEAD'), 'Step 6 must name origin/HEAD as the stale fork base'); + }); +}); + + }); +} + +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:fix-2649-diagnose-issues-worktree-stale-base', () => { + +const DIAGNOSE_ISSUES_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'diagnose-issues.md'); +const EXECUTE_PLAN_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-plan.md'); + +describe('diagnose-issues: pre-dispatch worktree base-check (#2649)', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(DIAGNOSE_ISSUES_PATH), 'workflows/diagnose-issues.md should exist'); + }); + + test('spawn_agents step runs worktree.base-check before the Agent() dispatch', () => { + const content = fs.readFileSync(DIAGNOSE_ISSUES_PATH, 'utf-8'); + const spawnIdx = content.indexOf(''); + assert.ok(spawnIdx !== -1, '"spawn_agents" step must exist in diagnose-issues.md'); + const baseCheckIdx = content.indexOf('worktree.base-check', spawnIdx); + assert.ok(baseCheckIdx !== -1, 'worktree.base-check must be invoked within the spawn_agents step'); + // The load-bearing invariant is "base-check BEFORE the Agent() dispatch" so the + // degrade decision can drop isolation from the spawn. (Where EXPECTED_BASE is + // captured relative to the check is cosmetic — the check only reads HEAD, never + // mutates it — so assert the real invariant, not a loose disjunction.) + const agentIdx = content.indexOf('Agent(', spawnIdx); + assert.ok(agentIdx !== -1, 'spawn_agents must contain an Agent() dispatch'); + assert.ok( + baseCheckIdx < agentIdx, + 'worktree.base-check must run before the Agent() dispatch so the degrade decision can drop isolation from the spawn', + ); + }); + + test('verify-only worktree_branch_check backstop remains embedded in the Agent() prompt', () => { + // Acceptance criterion #4: the base-check is a PRE-DISPATCH degrade; the + // guard is a POST-FORK fail-closed backstop. Both + // layers must survive — a future edit that dropped the backstop embedding + // would re-open the silent-stale-base class. Guard its continued presence. + const content = fs.readFileSync(DIAGNOSE_ISSUES_PATH, 'utf-8'); + const spawnIdx = content.indexOf(''); + assert.ok(spawnIdx !== -1, '"spawn_agents" step must exist'); + assert.ok( + content.indexOf('worktree-branch-check.md', spawnIdx) !== -1, + 'spawn_agents must still materialize the backstop after the base-check gate (#2649 acceptance criterion 4)', + ); + }); + + test('degrade check sets USE_WORKTREES=false when shouldDegrade is true', () => { + const content = fs.readFileSync(DIAGNOSE_ISSUES_PATH, 'utf-8'); + const baseCheckIdx = content.indexOf('worktree.base-check'); + const block = content.slice(baseCheckIdx, baseCheckIdx + 600); + assert.ok( + block.includes('shouldDegrade') && block.includes('USE_WORKTREES=false'), + 'degrade check must override USE_WORKTREES=false when shouldDegrade is true', + ); + }); + + test('degrade check references #2649 for traceability', () => { + const content = fs.readFileSync(DIAGNOSE_ISSUES_PATH, 'utf-8'); + assert.ok(content.includes('#2649'), 'diagnose-issues.md must reference #2649'); + }); +}); + +describe('execute-plan Pattern A: pre-dispatch worktree base-check (#2649)', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(EXECUTE_PLAN_PATH), 'workflows/execute-plan.md should exist'); + }); + + test('Pattern A runs the worktree base-check before spawning the executor', () => { + const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); + const patternAIdx = content.indexOf('**Pattern A:**'); + assert.ok(patternAIdx !== -1, '"Pattern A:" must exist in execute-plan.md'); + // The base-check instruction must appear within the Pattern A description, + // before the isolation="worktree" embedding instruction. + const patternAEnd = content.indexOf('**Pattern B:**', patternAIdx); + const patternA = content.slice(patternAIdx, patternAEnd === -1 ? undefined : patternAEnd); + assert.ok( + patternA.includes('#2649') && /worktree\.base-check|base-check/.test(patternA), + 'Pattern A must run the #2649 worktree base-check before dispatching the executor', + ); + assert.ok( + patternA.includes('shouldDegrade'), + 'Pattern A base-check must consult shouldDegrade', + ); + }); + + test('Pattern A documents the auto-degrade (drop isolation on shouldDegrade)', () => { + const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); + const patternAIdx = content.indexOf('**Pattern A:**'); + const patternAEnd = content.indexOf('**Pattern B:**', patternAIdx); + const patternA = content.slice(patternAIdx, patternAEnd === -1 ? undefined : patternAEnd); + assert.ok( + /degrad|sequential/i.test(patternA), + 'Pattern A must document auto-degrading to sequential mode when shouldDegrade is true', + ); + }); +}); + + }); +}