From 1bc7f7e6b03cff557efff30d381d8a049c056d5a Mon Sep 17 00:00:00 2001 From: sim Date: Wed, 12 Aug 2026 00:51:07 -0400 Subject: [PATCH 1/5] =?UTF-8?q?test(#3339):=20fold=20the=20state/phase/dis?= =?UTF-8?q?patch=20&=20model-profile=20issue-*=20cluster=20=E2=80=94=20Wav?= =?UTF-8?q?e=207?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds 9 legacy issue-*.test.cjs regression files (140 test() blocks) into their module's main suite, per H3 (#3315) of the test-hygiene epic (#3053). LAST of 4 issue-* waves — closes out the 74-file fix-*/issue-* backlog (pending BUG_FILE_RE extension, held for a follow-up commit until Wave 6 is confirmed merged, per the epic's own zero-backlog precondition). - issue-2828-flat-roadmap-total-phases.test.cjs (1) + issue-3204-state- writer-phase-count.test.cjs (21): both target state-document.cjs buildStateFrontmatter via different CLI entrypoints — merged jointly into state-document.test.cjs, 0 dropped. - issue-2945-phase-complete-checkbox-rollback.test.cjs (4) + issue-2949- phase-complete-stage3-sentinel.test.cjs (4): both target phase.cts cmdPhaseComplete; issue explicitly warned of overlap — verified disjoint fixtures/assertions, 0 dropped, merged into phase.test.cjs. - issue-2927-reviewer-lane-overlay-invocation.test.cjs (10) merged into review-lane-descriptor.test.cjs. - issue-2939-dispatch-flatten-maxdepth.test.cjs (9) merged into host-integration.test.cjs, 2 dropped as verified exact duplicates. - issue-2977-frontmatter-bom.test.cjs (5) merged into frontmatter.test.cjs. - issue-2045-third-party-skills-surface.test.cjs (6) merged into capability-loader.test.cjs. - issue-2517-runtime-aware-profiles.test.cjs (80, the largest single fold in the epic) merged into model-resolver.test.cjs, 1 dropped as a verified true duplicate (checked against src/model-resolver.cts logic, not just title similarity). Fixed a genuine eslint irregular-whitespace finding: a literal BOM character embedded in a doc comment (pre-existing content from the original #2977 source, illustrating what a BOM looks like) — replaced with a readable U+FEFF notation. 3 stale doc references found and fixed (docs/adr/2313, 3180, 443). Zero net test-coverage loss. No production code changed. --- docs/adr/2313-codex-passive-model-posture.md | 5 +- ...80-planning-semantic-model-single-owner.md | 2 +- ...48-unified-effort-and-fast-mode-routing.md | 3 +- tests/capability-loader.test.cjs | 274 ++++++ tests/frontmatter.test.cjs | 86 ++ tests/host-integration.test.cjs | 100 ++ ...e-2045-third-party-skills-surface.test.cjs | 265 ------ ...issue-2517-runtime-aware-profiles.test.cjs | 885 ----------------- ...ue-2828-flat-roadmap-total-phases.test.cjs | 94 -- ...-reviewer-lane-overlay-invocation.test.cjs | 322 ------- ...ue-2939-dispatch-flatten-maxdepth.test.cjs | 135 --- ...-phase-complete-checkbox-rollback.test.cjs | 140 --- ...49-phase-complete-stage3-sentinel.test.cjs | 205 ---- tests/issue-2977-frontmatter-bom.test.cjs | 78 -- ...sue-3204-state-writer-phase-count.test.cjs | 784 --------------- tests/model-resolver.test.cjs | 885 +++++++++++++++++ tests/phase.test.cjs | 363 +++++++ tests/review-lane-descriptor.test.cjs | 331 +++++++ tests/state-document.test.cjs | 896 ++++++++++++++++++ 19 files changed, 2941 insertions(+), 2912 deletions(-) delete mode 100644 tests/issue-2045-third-party-skills-surface.test.cjs delete mode 100644 tests/issue-2517-runtime-aware-profiles.test.cjs delete mode 100644 tests/issue-2828-flat-roadmap-total-phases.test.cjs delete mode 100644 tests/issue-2927-reviewer-lane-overlay-invocation.test.cjs delete mode 100644 tests/issue-2939-dispatch-flatten-maxdepth.test.cjs delete mode 100644 tests/issue-2945-phase-complete-checkbox-rollback.test.cjs delete mode 100644 tests/issue-2949-phase-complete-stage3-sentinel.test.cjs delete mode 100644 tests/issue-2977-frontmatter-bom.test.cjs delete mode 100644 tests/issue-3204-state-writer-phase-count.test.cjs diff --git a/docs/adr/2313-codex-passive-model-posture.md b/docs/adr/2313-codex-passive-model-posture.md index 0e56fdd27..c16c2d862 100644 --- a/docs/adr/2313-codex-passive-model-posture.md +++ b/docs/adr/2313-codex-passive-model-posture.md @@ -191,8 +191,9 @@ ADR-2719 §7 deliberately keeps and `npm run gen:install-tree` regenerates. exist in the tree. ADR-1239's Codex-binding section still names the retired fixture; it is recorded here so a Phase-1 implementer does not go looking for a gate that was removed a release ago. -`tests/codex-config.test.cjs` and `tests/issue-2517-runtime-aware-profiles.test.cjs` assert the -embedding today and flip to assert omission in the same phase. +`tests/codex-config.test.cjs` and `tests/model-resolver.test.cjs` (folds former +`issue-2517-runtime-aware-profiles`) assert the embedding today and flip to assert omission in the +same phase. ## Alternatives considered diff --git a/docs/adr/3180-planning-semantic-model-single-owner.md b/docs/adr/3180-planning-semantic-model-single-owner.md index 191ab6755..26e3d19ca 100644 --- a/docs/adr/3180-planning-semantic-model-single-owner.md +++ b/docs/adr/3180-planning-semantic-model-single-owner.md @@ -613,7 +613,7 @@ are right, because they ask different questions: | Contract | Question | Verdict on `0.x` | |---|---|---| | #2554 (`roadmap-parser.test.cjs`) | is this directory part of the current milestone's phase SET? | **count it** — a `00.1-` dir declared as `### Phase 00.1:` is a real phase | -| #2949 (`issue-2949-phase-complete-stage3-sentinel.test.cjs`) | must this phase COMPLETE before the milestone can close? | **sentinel** — a `0.x` must not block `is_last_phase` | +| #2949 (`phase.test.cjs`, folded:issue-2949-phase-complete-stage3-sentinel) | must this phase COMPLETE before the milestone can close? | **sentinel** — a `0.x` must not block `is_last_phase` | No single global predicate answers both. The resolution is layered, not unified: `isSentinelPhaseId` keeps its semantics (`0.x` IS a sentinel, satisfying #2949), and the milestone-WINDOW layer keeps a diff --git a/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md b/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md index c95cae159..aa96b534b 100644 --- a/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md +++ b/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md @@ -118,7 +118,8 @@ Common core: `low`, `medium`, `high`, `xhigh`. ## References - Tracking issue: #443 -- Prior art (inert reasoning_effort): #2517; `tests/issue-2517-runtime-aware-profiles.test.cjs` +- Prior art (inert reasoning_effort): #2517; `tests/model-resolver.test.cjs` (folds former + `issue-2517-runtime-aware-profiles`) - dynamic_routing escalation: #3024; `tests/model-profiles.test.cjs` (folds former `feat-3024-dynamic-routing`, consolidation epic #1969) - phase-type tiers: #3023 - Anthropic effort API: `output_config.effort` (`low`/`medium`/`high`/`xhigh`/`max`); fast mode: `speed` (`standard`/`fast`) diff --git a/tests/capability-loader.test.cjs b/tests/capability-loader.test.cjs index a4a9e6db9..4dfcfc024 100644 --- a/tests/capability-loader.test.cjs +++ b/tests/capability-loader.test.cjs @@ -1353,3 +1353,277 @@ describe('loadRegistry — #1461 finding 1: a throwing per-candidate validator s 'a null gate records no blockedGates entry'); }); }); + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2045-third-party-skills-surface.test.cjs — test-hygiene sweep #3339 (H3 Wave 7) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2045-third-party-skills-surface', () => { +'use strict'; + +/** + * issue-2045-third-party-skills-surface.test.cjs — regression for bug #2045. + * + * A skills-only `role: feature` third-party capability installs "active" but its + * skills never surface, `capability enable`/`set` reject it as "unknown", and + * `capability list` disagrees with `capability state`. Three defects, fix shape + * "1b" (teach resolveSurface, no on-disk linking): + * + * D1 (materialization): resolveSurface built the `skills` Set only from the + * on-disk skill manifest; third-party cap skills live at + * ~/.gsd/capabilities//skills/ and never entered the Set → surfaced:false. + * FIX: union registry.capabilityClusters values into the Set. + * D2 (enable/set unknown): setCapabilityState validated the capId against the + * STATIC first-party registry instead of the composed overlay-aware registry. + * FIX: validate against loadRegistry({ includeInstalled }). + * D3 (list vs state): `capability list` derived `status` purely from ledger + * existence, never surface composition. FIX: add a `surfaced` field so list + * reflects the same surface state `capability state` reports. + * + * Acceptance criteria (each is a release blocker, per the issue's "I'd expect" + * table): + * AC1 [D1]: resolveSurface includes a third-party cap's skill stems. + * AC2: capability state reports the cap present with surfaced:true. + * AC3 [D2]: capability enable/set on an installed third-party cap does NOT + * error "unknown capability". + * AC4 [D3]: capability list reflects surface state (list/state agree). + * AC5: first-party caps unaffected; writer still rejects truly-unknown ids. + */ + +const { describe, test, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const { runGsdTools, cleanup } = require('./helpers.cjs'); + +const { resolveCapabilityRuntimeState } = require('../gsd-core/bin/lib/capability-state.cjs'); +const { setCapabilityState } = require('../gsd-core/bin/lib/capability-writer.cjs'); +const { resolveSurface } = require('../gsd-core/bin/lib/surface.cjs'); +const { loadRegistry } = require('../gsd-core/bin/lib/capability-loader.cjs'); + +// ─── Fixture helpers ───────────────────────────────────────────────────────── + +const CAP_ID = 'demo-2045-cap'; +const CAP_SKILLS = ['demo-2045-alpha', 'demo-2045-beta']; +const HOST_RANGE = '>=1.0.0'; + +const tmps = []; +function tmpDir(prefix) { + const d = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tmps.push(d); + return d; +} +after(() => { for (const d of tmps) cleanup(d); }); + +/** A conformant skills-only feature capability manifest (the reporter's shape). */ +function skillsOnlyCap(id, skills) { + return { + id, + role: 'feature', + version: '1.0.0', + title: id, + description: 'skills-only third-party capability (issue #2045 fixture)', + tier: 'full', + requires: [], + engines: { gsd: HOST_RANGE }, + runtimeCompat: { supported: ['*'], unsupported: [] }, + skills, + agents: [], + hooks: [], + config: {}, + steps: [], + contributions: [], + gates: [], + }; +} + +/** Write a global-scope overlay bundle at /.gsd/capabilities//. */ +function writeGlobalBundle(home, id, skills) { + const dir = path.join(home, '.gsd', 'capabilities', id); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'capability.json'), JSON.stringify(skillsOnlyCap(id, skills)), 'utf8'); + // Materialize each declared skill so the bundle mirrors a real install. + for (const stem of skills) { + const skillDir = path.join(dir, 'skills', stem); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), `---\ndescription: ${stem}\n---\n# ${stem}\n`, 'utf8'); + } + return dir; +} + +/** A temp runtime config dir with NO .gsd-profile marker → default 'full' profile. */ +function makeRcd() { + return tmpDir('issue2045-rcd-'); +} + +/** A temp cwd with .planning/config.json so project-root resolution is hermetic. */ +function makeCwd() { + const cwd = tmpDir('issue2045-cwd-'); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'config.json'), '{}'); + return cwd; +} + +/** GSD_HOME-sandboxed env that also neutralizes ambient GSD_ vars (hermeticity). */ +function scopeEnv(home) { + return { GSD_HOME: home, GSD_WORKSTREAM: '', GSD_PROJECT: '' }; +} + +const savedGsdHome = process.env.GSD_HOME; + +// ─── AC1 [D1]: resolveSurface unions registry.capabilityClusters ───────────── + +describe('issue #2045 AC1 — resolveSurface includes third-party cap skills [D1]', () => { + test('a composed registry surfaces third-party cap skills without on-disk linking', () => { + const home = tmpDir('issue2045-home-'); + writeGlobalBundle(home, CAP_ID, CAP_SKILLS); + const rcd = makeRcd(); + const cwd = makeCwd(); + process.env.GSD_HOME = home; + try { + // Composed overlay-aware registry (the loader composes ACTIVE overlay caps). + const registry = loadRegistry({ includeInstalled: true, cwd, gsdHome: home }); + const clusters = (registry && registry.capabilityClusters) || {}; + assert.ok(Array.isArray(clusters[CAP_ID]), `fixture cap "${CAP_ID}" must own skills in capabilityClusters (loader did not accept the overlay — check engines/validation)`); + + // Empty manifest + composed registry: in the 'full' profile the skills Set + // is materialized from the manifest (empty here) THEN, after the fix, unioned + // with capabilityClusters values. Third-party skills are NOT on disk in rcd, + // so they can only appear via the registry union. + const surface = resolveSurface(rcd, new Map(), undefined, registry); + assert.ok(surface.skills instanceof Set, 'resolveSurface returns a skills Set'); + for (const stem of CAP_SKILLS) { + assert.ok( + surface.skills.has(stem), + `AC1: third-party skill "${stem}" must be in the surfaced Set (no on-disk linking) — got [${[...surface.skills].join(', ')}]`, + ); + } + } finally { + process.env.GSD_HOME = savedGsdHome; + } + }); +}); + +// ─── AC2: capability state reports present + surfaced:true ─────────────────── + +describe('issue #2045 AC2 — capability state reports surfaced:true', () => { + test('resolveCapabilityRuntimeState surfaces an installed third-party skills cap', () => { + const home = tmpDir('issue2045-home-'); + writeGlobalBundle(home, CAP_ID, CAP_SKILLS); + const rcd = makeRcd(); + const cwd = makeCwd(); + process.env.GSD_HOME = home; + try { + const state = resolveCapabilityRuntimeState(cwd, rcd); + const cap = state.capabilities.find((c) => c.id === CAP_ID); + assert.ok(cap, `AC2: capability state must list "${CAP_ID}" as present`); + assert.equal(cap.surfaced, true, 'AC2: surfaced must be true'); + assert.equal(cap.installed, true, 'AC2: installed must be true (default full profile)'); + // No activationKey on the fixture → active === enabled. + assert.equal(cap.active, true, 'AC2: active must be true (no config gate)'); + } finally { + process.env.GSD_HOME = savedGsdHome; + } + }); +}); + +// ─── AC3 [D2]: enable/set does NOT error "unknown capability" ──────────────── + +describe('issue #2045 AC3 — enable/set accepts an installed third-party cap [D2]', () => { + test('setCapabilityState({enabled:true}) on an installed third-party cap is not "unknown"', () => { + const home = tmpDir('issue2045-home-'); + writeGlobalBundle(home, CAP_ID, CAP_SKILLS); + const rcd = makeRcd(); + const cwd = makeCwd(); + process.env.GSD_HOME = home; + try { + const result = setCapabilityState(cwd, rcd, [{ id: CAP_ID, enabled: true }]); + const unknownErr = (result.errors || []).find((e) => /unknown capability/i.test(String(e))); + assert.ok(!unknownErr, `AC3: must not error "unknown capability" for installed third-party cap — got errors: ${JSON.stringify(result.errors)}`); + const cap = (result.capabilities || []).find((c) => c.id === CAP_ID); + assert.ok(cap, 'AC3: result must include the third-party cap'); + } finally { + process.env.GSD_HOME = savedGsdHome; + } + }); + + test('AC5 (regression): a truly-unknown id is STILL rejected as "unknown capability"', () => { + const rcd = makeRcd(); + const cwd = makeCwd(); + const home = tmpDir('issue2045-home-empty-'); + process.env.GSD_HOME = home; + try { + const result = setCapabilityState(cwd, rcd, [{ id: 'nonexistent-cap-2045', enabled: true }]); + const unknownErr = (result.errors || []).find((e) => /unknown capability/i.test(String(e))); + assert.ok(unknownErr, 'AC5: truly-unknown id must still be rejected (writer validates against composed registry, not "accept all")'); + } finally { + process.env.GSD_HOME = savedGsdHome; + } + }); + + test('AC5 (regression): a first-party cap still resolves and surfaces', () => { + const rcd = makeRcd(); + const cwd = makeCwd(); + const home = tmpDir('issue2045-home-empty-'); + process.env.GSD_HOME = home; + try { + const state = resolveCapabilityRuntimeState(cwd, rcd); + // 'ui' is a first-party skill-owning capability; it must still be present. + const ui = state.capabilities.find((c) => c.id === 'ui'); + assert.ok(ui, 'AC5: first-party "ui" capability still present'); + } finally { + process.env.GSD_HOME = savedGsdHome; + } + }); +}); + +// ─── AC4 [D3]: capability list reflects surface state (list/state agree) ───── + +describe('issue #2045 AC4 — capability list reflects surface state [D3]', () => { + test('an installed skills cap: list `surfaced` agrees with `capability state`', () => { + const home = tmpDir('issue2045-home-'); + const cwd = makeCwd(); + + // Build a local source dir that declares skills AND materializes them, then + // install it globally — the real end-to-end path the reporter used. + const src = tmpDir('issue2045-src-'); + const cap = skillsOnlyCap(CAP_ID, CAP_SKILLS); + fs.writeFileSync(path.join(src, 'capability.json'), JSON.stringify(cap), 'utf8'); + for (const stem of CAP_SKILLS) { + const skillDir = path.join(src, 'skills', stem); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), `---\ndescription: ${stem}\n---\n# ${stem}\n`, 'utf8'); + } + + const installRes = runGsdTools(['capability', 'install', src, '--scope', 'global', '--raw'], cwd, scopeEnv(home)); + assert.equal(installRes.success, true, `install failed: ${installRes.error || installRes.output}`); + + // capability list --json must include a `surfaced` field for the overlay row. + const listRes = runGsdTools(['capability', 'list', '--json'], cwd, scopeEnv(home)); + assert.equal(listRes.success, true, `list failed: ${listRes.error || listRes.output}`); + const listRows = JSON.parse(listRes.output); + const listRow = listRows.find((r) => r.id === CAP_ID); + assert.ok(listRow, 'AC4: installed cap present in list'); + assert.ok( + Object.prototype.hasOwnProperty.call(listRow, 'surfaced'), + `AC4: list row must carry a 'surfaced' field reflecting surface composition — got keys: ${Object.keys(listRow).join(', ')}`, + ); + assert.equal(listRow.surfaced, true, 'AC4: list surfaced === true (skills resolved via registry union)'); + + // capability state --json must agree. + const stateRes = runGsdTools(['capability', 'state', CAP_ID, '--json'], cwd, scopeEnv(home)); + assert.equal(stateRes.success, true, `state failed: ${stateRes.error || stateRes.output}`); + const stateObj = JSON.parse(stateRes.output); + const stateCap = (stateObj.capabilities || []).find((c) => c.id === CAP_ID); + assert.ok(stateCap, 'AC4: capability state must list the cap'); + assert.equal(stateCap.surfaced, true, 'AC4: state surfaced === true'); + + // The agreement the reporter asked for: list.surfaced === state.surfaced. + assert.equal(listRow.surfaced, stateCap.surfaced, 'AC4: list and state must AGREE on surfaced'); + }); +}); + }); +} diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index a0e09ace9..9c744af32 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -2243,3 +2243,89 @@ describe('#2847: schema name consistency between gsd-planner.md and src/frontmat }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2977-frontmatter-bom.test.cjs — test-hygiene sweep #3339 (H3 Wave 7) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2977-frontmatter-bom', () => { +'use strict'; + +/** + * Regression test for #2977 — `extractFrontmatter` returns {} for any file whose + * frontmatter fence is preceded by a UTF-8 BOM (Windows PowerShell `>`/`Out-File`, + * several editors). The `startsWith('---')` byte-0 check fails on any leading byte, + * so every frontmatter field silently disappears with no error. + * + * The fix strips a leading UTF-8 BOM (U+FEFF) before the fence check. Scope: BOM only + * (acceptance criteria 1-3). The generalized "arbitrary content before the fence" fork + * (tolerate vs diagnose) is a product-intent decision, surfaced in the PR — out of scope. + * + * Matrix: .gsd/bug/fix/2977-frontmatter-bom-tolerance/50-test-matrix.md + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const { extractFrontmatter } = require('../gsd-core/bin/lib/frontmatter.cjs'); + +const BOM = '\uFEFF'; + +describe('extractFrontmatter BOM tolerance (#2977)', () => { + test('bomPrefixedFrontmatterParses', () => { + // Row 1 (failing-first regression): a BOM-prefixed frontmatter document parses + // identically to the same document without the BOM. + const clean = '---\ntitle: T\nphase: "01"\nstatus: passed\n---\n\n# Body\n'; + const bommed = BOM + clean; + const expected = extractFrontmatter(clean, 'a.md'); + const actual = extractFrontmatter(bommed, 'a.md'); + assert.deepEqual(actual, expected, 'BOM-prefixed frontmatter must parse identically to no-BOM'); + assert.strictEqual(actual.title, 'T', 'title field recovered'); + assert.strictEqual(actual.phase, '01', 'phase field recovered'); + assert.strictEqual(actual.status, 'passed', 'status field recovered'); + }); + + test('bomWithCrlfParses', () => { + // Row 2 (acceptance #2): BOM + CRLF line endings together still parse correctly. + const clean = '---\r\ntitle: T\r\nphase: "01"\r\n---\r\n\r\n# Body\r\n'; + const bommed = BOM + clean; + const actual = extractFrontmatter(bommed, 'a.md'); + assert.strictEqual(actual.title, 'T', 'title recovered (BOM + CRLF)'); + assert.strictEqual(actual.phase, '01', 'phase recovered (BOM + CRLF)'); + }); + + test('bomWithNoFrontmatterStaysEmpty', () => { + // Row 3 (acceptance #3): a BOM prefixing a document with no frontmatter (or genuinely + // empty frontmatter) returns {} with no false diagnostic — same as no-BOM. + assert.deepEqual(extractFrontmatter(BOM + 'just plain text', 'a.md'), {}, 'BOM + no frontmatter -> {}'); + assert.deepEqual(extractFrontmatter(BOM + '', 'a.md'), {}, 'BOM + empty -> {}'); + // A thematic-break-first-line Markdown doc (--- then prose) must stay {} — protected by + // the existing false-positive threshold; the BOM strip must not lower that bar. + assert.deepEqual(extractFrontmatter(BOM + '---\n\nA horizontal rule, not frontmatter.\n', 'a.md'), {}, + 'BOM + thematic-break Markdown -> {} (no false diagnostic)'); + }); + + test('bomAcrossArtifactTypes', () => { + // Row 4 (acceptance #1 across artifact types): each frontmatter-bearing artifact shape + // recovers its fields when BOM-prefixed. + const cases = [ + { name: 'STATE.md', body: '---\ncurrent_phase: "01"\nstatus: "In progress"\n---\n\n# State\n', expect: { current_phase: '01', status: 'In progress' } }, + { name: 'PLAN.md', body: '---\nphase: "01"\nplan: "01-01"\nstatus: "done"\n---\n\n# Plan\n', expect: { phase: '01', plan: '01-01', status: 'done' } }, + { name: 'SUMMARY.md', body: '---\none-liner: "shipped the thing"\n---\n\n# Summary\n', expect: { 'one-liner': 'shipped the thing' } }, + { name: 'UAT.md', body: '---\nphase: "02"\nverdict: "pass"\n---\n\n# UAT\n', expect: { phase: '02', verdict: 'pass' } }, + ]; + for (const c of cases) { + const actual = extractFrontmatter(BOM + c.body, c.name); + assert.deepEqual(actual, c.expect, `${c.name}: BOM-prefixed frontmatter must recover fields`); + } + }); + + test('controlNoBom', () => { + // Row 5 (no regression): no BOM, valid frontmatter still parses correctly (unchanged). + const actual = extractFrontmatter('---\ntitle: T\nphase: "01"\n---\n\n# Body\n', 'a.md'); + assert.strictEqual(actual.title, 'T'); + assert.strictEqual(actual.phase, '01'); + }); +}); + }); +} diff --git a/tests/host-integration.test.cjs b/tests/host-integration.test.cjs index d4cd594de..31778e9a4 100644 --- a/tests/host-integration.test.cjs +++ b/tests/host-integration.test.cjs @@ -2502,3 +2502,103 @@ describe('#2728 B1 — isolation degrades re-record through the single write pat ); }); }); + +// --------------------------------------------------------------------------- +// Folded from tests/issue-2939-dispatch-flatten-maxdepth.test.cjs (H3 Wave 7, +// issue #3339). Two of the original nine cases (maxDepth:1 → true and +// maxDepth:2 → false, both with the full codex-like descriptor) were exact +// duplicates of the "Phase B: shouldFlattenDispatch — contract pin" describe +// above (lines ~1014-1029) and were dropped; the remaining seven exercise +// input shapes (maxDepth:-1 unbounded, nested:false, non-full toolkit, +// background:false/backgroundDispatch:false with maxDepth:5, maxDepth:0, +// and maxDepth missing/non-number) not covered elsewhere in this file. +// --------------------------------------------------------------------------- +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2939-dispatch-flatten-maxdepth', () => { + /** The live Codex-shaped descriptor (the #2939 bug input), with per-test depth overrides. */ + function codexLike(overrides = {}) { + return { + namedDispatch: true, + nested: true, + maxDepth: 1, + background: true, + subagentToolkit: 'full', + backgroundDispatch: true, + ...overrides, + }; + } + + test('maxDepthUnboundedBackgroundsUnchanged', () => { + // Row 3: maxDepth:-1 (unbounded) → background permitted, unchanged. + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ maxDepth: -1 })), + false, + 'maxDepth:-1 (unbounded) → background permitted (unchanged)', + ); + }); + + test('nestedFalseFlattensRegardlessOfDepth', () => { + // Row 4 / acceptance #4: nested:false cannot host a nesting orchestrator → + // flatten regardless of maxDepth. + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ nested: false, maxDepth: 5 })), + true, + 'nested:false → flatten regardless of maxDepth', + ); + }); + + test('nonFullToolkitFlattens', () => { + // Row 5 / acceptance #4: a non-full toolkit cannot delegate → flatten + // regardless of maxDepth. + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ subagentToolkit: 'read-only', maxDepth: 5 })), + true, + 'subagentToolkit!=="full" → flatten regardless of maxDepth', + ); + }); + + test('backgroundFalseStillFlattens', () => { + // Row 6 / negative-space: background:false → flatten (the existing + // background-boolean fail-closed path is unchanged). + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ background: false, maxDepth: 5 })), + true, + 'background:false → flatten (unchanged)', + ); + }); + + test('backgroundDispatchFalseStillFlattens', () => { + // Row 7 / negative-space: backgroundDispatch:false → flatten (unchanged). + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ backgroundDispatch: false, maxDepth: 5 })), + true, + 'backgroundDispatch:false → flatten (unchanged)', + ); + }); + + test('maxDepth0Flattens', () => { + // Row 9: maxDepth:0 (zero depth budget) → flatten. + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ maxDepth: 0 })), + true, + 'maxDepth:0 → flatten (zero depth budget)', + ); + }); + + test('maxDepthMissingFlattens', () => { + // Row 10: maxDepth missing/non-number → flatten (fail-closed on absent + // budget, mirrors degradationFor treating non-finite as 0). + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ maxDepth: undefined })), + true, + 'maxDepth missing → flatten (fail-closed)', + ); + assert.strictEqual( + hi.shouldFlattenDispatch(codexLike({ maxDepth: 'deep' })), + true, + 'maxDepth non-number → flatten (fail-closed)', + ); + }); + }); +} diff --git a/tests/issue-2045-third-party-skills-surface.test.cjs b/tests/issue-2045-third-party-skills-surface.test.cjs deleted file mode 100644 index 3e5ccd1cb..000000000 --- a/tests/issue-2045-third-party-skills-surface.test.cjs +++ /dev/null @@ -1,265 +0,0 @@ -'use strict'; - -/** - * issue-2045-third-party-skills-surface.test.cjs — regression for bug #2045. - * - * A skills-only `role: feature` third-party capability installs "active" but its - * skills never surface, `capability enable`/`set` reject it as "unknown", and - * `capability list` disagrees with `capability state`. Three defects, fix shape - * "1b" (teach resolveSurface, no on-disk linking): - * - * D1 (materialization): resolveSurface built the `skills` Set only from the - * on-disk skill manifest; third-party cap skills live at - * ~/.gsd/capabilities//skills/ and never entered the Set → surfaced:false. - * FIX: union registry.capabilityClusters values into the Set. - * D2 (enable/set unknown): setCapabilityState validated the capId against the - * STATIC first-party registry instead of the composed overlay-aware registry. - * FIX: validate against loadRegistry({ includeInstalled }). - * D3 (list vs state): `capability list` derived `status` purely from ledger - * existence, never surface composition. FIX: add a `surfaced` field so list - * reflects the same surface state `capability state` reports. - * - * Acceptance criteria (each is a release blocker, per the issue's "I'd expect" - * table): - * AC1 [D1]: resolveSurface includes a third-party cap's skill stems. - * AC2: capability state reports the cap present with surfaced:true. - * AC3 [D2]: capability enable/set on an installed third-party cap does NOT - * error "unknown capability". - * AC4 [D3]: capability list reflects surface state (list/state agree). - * AC5: first-party caps unaffected; writer still rejects truly-unknown ids. - */ - -const { describe, test, after } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const os = require('node:os'); -const path = require('node:path'); - -const { runGsdTools, cleanup } = require('./helpers.cjs'); - -const { resolveCapabilityRuntimeState } = require('../gsd-core/bin/lib/capability-state.cjs'); -const { setCapabilityState } = require('../gsd-core/bin/lib/capability-writer.cjs'); -const { resolveSurface } = require('../gsd-core/bin/lib/surface.cjs'); -const { loadRegistry } = require('../gsd-core/bin/lib/capability-loader.cjs'); - -// ─── Fixture helpers ───────────────────────────────────────────────────────── - -const CAP_ID = 'demo-2045-cap'; -const CAP_SKILLS = ['demo-2045-alpha', 'demo-2045-beta']; -const HOST_RANGE = '>=1.0.0'; - -const tmps = []; -function tmpDir(prefix) { - const d = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); - tmps.push(d); - return d; -} -after(() => { for (const d of tmps) cleanup(d); }); - -/** A conformant skills-only feature capability manifest (the reporter's shape). */ -function skillsOnlyCap(id, skills) { - return { - id, - role: 'feature', - version: '1.0.0', - title: id, - description: 'skills-only third-party capability (issue #2045 fixture)', - tier: 'full', - requires: [], - engines: { gsd: HOST_RANGE }, - runtimeCompat: { supported: ['*'], unsupported: [] }, - skills, - agents: [], - hooks: [], - config: {}, - steps: [], - contributions: [], - gates: [], - }; -} - -/** Write a global-scope overlay bundle at /.gsd/capabilities//. */ -function writeGlobalBundle(home, id, skills) { - const dir = path.join(home, '.gsd', 'capabilities', id); - fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, 'capability.json'), JSON.stringify(skillsOnlyCap(id, skills)), 'utf8'); - // Materialize each declared skill so the bundle mirrors a real install. - for (const stem of skills) { - const skillDir = path.join(dir, 'skills', stem); - fs.mkdirSync(skillDir, { recursive: true }); - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), `---\ndescription: ${stem}\n---\n# ${stem}\n`, 'utf8'); - } - return dir; -} - -/** A temp runtime config dir with NO .gsd-profile marker → default 'full' profile. */ -function makeRcd() { - return tmpDir('issue2045-rcd-'); -} - -/** A temp cwd with .planning/config.json so project-root resolution is hermetic. */ -function makeCwd() { - const cwd = tmpDir('issue2045-cwd-'); - fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); - fs.writeFileSync(path.join(cwd, '.planning', 'config.json'), '{}'); - return cwd; -} - -/** GSD_HOME-sandboxed env that also neutralizes ambient GSD_ vars (hermeticity). */ -function scopeEnv(home) { - return { GSD_HOME: home, GSD_WORKSTREAM: '', GSD_PROJECT: '' }; -} - -const savedGsdHome = process.env.GSD_HOME; - -// ─── AC1 [D1]: resolveSurface unions registry.capabilityClusters ───────────── - -describe('issue #2045 AC1 — resolveSurface includes third-party cap skills [D1]', () => { - test('a composed registry surfaces third-party cap skills without on-disk linking', () => { - const home = tmpDir('issue2045-home-'); - writeGlobalBundle(home, CAP_ID, CAP_SKILLS); - const rcd = makeRcd(); - const cwd = makeCwd(); - process.env.GSD_HOME = home; - try { - // Composed overlay-aware registry (the loader composes ACTIVE overlay caps). - const registry = loadRegistry({ includeInstalled: true, cwd, gsdHome: home }); - const clusters = (registry && registry.capabilityClusters) || {}; - assert.ok(Array.isArray(clusters[CAP_ID]), `fixture cap "${CAP_ID}" must own skills in capabilityClusters (loader did not accept the overlay — check engines/validation)`); - - // Empty manifest + composed registry: in the 'full' profile the skills Set - // is materialized from the manifest (empty here) THEN, after the fix, unioned - // with capabilityClusters values. Third-party skills are NOT on disk in rcd, - // so they can only appear via the registry union. - const surface = resolveSurface(rcd, new Map(), undefined, registry); - assert.ok(surface.skills instanceof Set, 'resolveSurface returns a skills Set'); - for (const stem of CAP_SKILLS) { - assert.ok( - surface.skills.has(stem), - `AC1: third-party skill "${stem}" must be in the surfaced Set (no on-disk linking) — got [${[...surface.skills].join(', ')}]`, - ); - } - } finally { - process.env.GSD_HOME = savedGsdHome; - } - }); -}); - -// ─── AC2: capability state reports present + surfaced:true ─────────────────── - -describe('issue #2045 AC2 — capability state reports surfaced:true', () => { - test('resolveCapabilityRuntimeState surfaces an installed third-party skills cap', () => { - const home = tmpDir('issue2045-home-'); - writeGlobalBundle(home, CAP_ID, CAP_SKILLS); - const rcd = makeRcd(); - const cwd = makeCwd(); - process.env.GSD_HOME = home; - try { - const state = resolveCapabilityRuntimeState(cwd, rcd); - const cap = state.capabilities.find((c) => c.id === CAP_ID); - assert.ok(cap, `AC2: capability state must list "${CAP_ID}" as present`); - assert.equal(cap.surfaced, true, 'AC2: surfaced must be true'); - assert.equal(cap.installed, true, 'AC2: installed must be true (default full profile)'); - // No activationKey on the fixture → active === enabled. - assert.equal(cap.active, true, 'AC2: active must be true (no config gate)'); - } finally { - process.env.GSD_HOME = savedGsdHome; - } - }); -}); - -// ─── AC3 [D2]: enable/set does NOT error "unknown capability" ──────────────── - -describe('issue #2045 AC3 — enable/set accepts an installed third-party cap [D2]', () => { - test('setCapabilityState({enabled:true}) on an installed third-party cap is not "unknown"', () => { - const home = tmpDir('issue2045-home-'); - writeGlobalBundle(home, CAP_ID, CAP_SKILLS); - const rcd = makeRcd(); - const cwd = makeCwd(); - process.env.GSD_HOME = home; - try { - const result = setCapabilityState(cwd, rcd, [{ id: CAP_ID, enabled: true }]); - const unknownErr = (result.errors || []).find((e) => /unknown capability/i.test(String(e))); - assert.ok(!unknownErr, `AC3: must not error "unknown capability" for installed third-party cap — got errors: ${JSON.stringify(result.errors)}`); - const cap = (result.capabilities || []).find((c) => c.id === CAP_ID); - assert.ok(cap, 'AC3: result must include the third-party cap'); - } finally { - process.env.GSD_HOME = savedGsdHome; - } - }); - - test('AC5 (regression): a truly-unknown id is STILL rejected as "unknown capability"', () => { - const rcd = makeRcd(); - const cwd = makeCwd(); - const home = tmpDir('issue2045-home-empty-'); - process.env.GSD_HOME = home; - try { - const result = setCapabilityState(cwd, rcd, [{ id: 'nonexistent-cap-2045', enabled: true }]); - const unknownErr = (result.errors || []).find((e) => /unknown capability/i.test(String(e))); - assert.ok(unknownErr, 'AC5: truly-unknown id must still be rejected (writer validates against composed registry, not "accept all")'); - } finally { - process.env.GSD_HOME = savedGsdHome; - } - }); - - test('AC5 (regression): a first-party cap still resolves and surfaces', () => { - const rcd = makeRcd(); - const cwd = makeCwd(); - const home = tmpDir('issue2045-home-empty-'); - process.env.GSD_HOME = home; - try { - const state = resolveCapabilityRuntimeState(cwd, rcd); - // 'ui' is a first-party skill-owning capability; it must still be present. - const ui = state.capabilities.find((c) => c.id === 'ui'); - assert.ok(ui, 'AC5: first-party "ui" capability still present'); - } finally { - process.env.GSD_HOME = savedGsdHome; - } - }); -}); - -// ─── AC4 [D3]: capability list reflects surface state (list/state agree) ───── - -describe('issue #2045 AC4 — capability list reflects surface state [D3]', () => { - test('an installed skills cap: list `surfaced` agrees with `capability state`', () => { - const home = tmpDir('issue2045-home-'); - const cwd = makeCwd(); - - // Build a local source dir that declares skills AND materializes them, then - // install it globally — the real end-to-end path the reporter used. - const src = tmpDir('issue2045-src-'); - const cap = skillsOnlyCap(CAP_ID, CAP_SKILLS); - fs.writeFileSync(path.join(src, 'capability.json'), JSON.stringify(cap), 'utf8'); - for (const stem of CAP_SKILLS) { - const skillDir = path.join(src, 'skills', stem); - fs.mkdirSync(skillDir, { recursive: true }); - fs.writeFileSync(path.join(skillDir, 'SKILL.md'), `---\ndescription: ${stem}\n---\n# ${stem}\n`, 'utf8'); - } - - const installRes = runGsdTools(['capability', 'install', src, '--scope', 'global', '--raw'], cwd, scopeEnv(home)); - assert.equal(installRes.success, true, `install failed: ${installRes.error || installRes.output}`); - - // capability list --json must include a `surfaced` field for the overlay row. - const listRes = runGsdTools(['capability', 'list', '--json'], cwd, scopeEnv(home)); - assert.equal(listRes.success, true, `list failed: ${listRes.error || listRes.output}`); - const listRows = JSON.parse(listRes.output); - const listRow = listRows.find((r) => r.id === CAP_ID); - assert.ok(listRow, 'AC4: installed cap present in list'); - assert.ok( - Object.prototype.hasOwnProperty.call(listRow, 'surfaced'), - `AC4: list row must carry a 'surfaced' field reflecting surface composition — got keys: ${Object.keys(listRow).join(', ')}`, - ); - assert.equal(listRow.surfaced, true, 'AC4: list surfaced === true (skills resolved via registry union)'); - - // capability state --json must agree. - const stateRes = runGsdTools(['capability', 'state', CAP_ID, '--json'], cwd, scopeEnv(home)); - assert.equal(stateRes.success, true, `state failed: ${stateRes.error || stateRes.output}`); - const stateObj = JSON.parse(stateRes.output); - const stateCap = (stateObj.capabilities || []).find((c) => c.id === CAP_ID); - assert.ok(stateCap, 'AC4: capability state must list the cap'); - assert.equal(stateCap.surfaced, true, 'AC4: state surfaced === true'); - - // The agreement the reporter asked for: list.surfaced === state.surfaced. - assert.equal(listRow.surfaced, stateCap.surfaced, 'AC4: list and state must AGREE on surfaced'); - }); -}); diff --git a/tests/issue-2517-runtime-aware-profiles.test.cjs b/tests/issue-2517-runtime-aware-profiles.test.cjs deleted file mode 100644 index 17c0a5c0f..000000000 --- a/tests/issue-2517-runtime-aware-profiles.test.cjs +++ /dev/null @@ -1,885 +0,0 @@ -/** - * Issue #2517 — runtime-aware model profile resolution. - * - * Today, profile tiers (opus/sonnet/haiku) only resolve to Claude IDs. On Codex / - * other runtimes, users must use `inherit` or write large `model_overrides` blocks. - * - * This adds a `runtime` config key + `model_profile_overrides[runtime][tier]` map. - * When `runtime` is set to a non-Claude value, profile tiers resolve to runtime- - * native model IDs. - * - * Codex: opus -> gpt-5.6-sol (xhigh), sonnet -> gpt-5.6-terra (medium), haiku -> gpt-5.6-luna (medium) - * - * `runtime: "claude"` is the implicit default and is treated as a no-op for - * resolution — it does not override `resolve_model_ids: "omit"` or any other - * Claude-native semantics (review finding #4). - * - * `inherit` keeps current behavior. Unknown runtimes fall back safely (do NOT emit - * provider-specific IDs the runtime can't accept) and trigger a one-shot stderr - * warning so typos like `runtime: "codx"` surface immediately (review finding #13). - * - * HOME isolation: every test sets `process.env.HOME` to a per-suite tmpdir so the - * developer's real `~/.gsd/defaults.json` cannot bleed into assertions - * (review finding #8 / pattern from CodeRabbit on PRs #2603, #2604). - */ - -'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 { createTempProject, cleanup, resetRuntimeWarningCaches } = require('./helpers.cjs'); - -const { - resolveModelInternal, - resolveEffortInternal, - resolveTierEntry, -} = require('../gsd-core/bin/lib/model-resolver.cjs'); -const { - RUNTIME_PROFILE_MAP, - KNOWN_RUNTIMES, -} = require('../gsd-core/bin/lib/model-catalog.cjs'); -const { renderEffortForRuntime } = require('../gsd-core/bin/lib/model-catalog.cjs'); -const { isValidConfigKey } = require('../gsd-core/bin/lib/config-schema.cjs'); - -function writeConfig(tmpDir, obj) { - fs.writeFileSync( - path.join(tmpDir, '.planning', 'config.json'), - JSON.stringify(obj, null, 2) - ); -} - -// ─── Shared HOME isolation (#2517 review finding #8) ──────────────────────── -// Without this, a developer's real `~/.gsd/defaults.json` (e.g. one with -// `runtime: codex` set) silently overrides test assertions about back-compat -// behavior. Capture HOME, point it at an isolated tmpdir for the duration of -// each test, restore on teardown. -let _origHome; -let _origUserProfile; -let _origGsdHome; -let _isolatedHome; -function isolateHome() { - _origHome = process.env.HOME; - _origUserProfile = process.env.USERPROFILE; - _origGsdHome = process.env.GSD_HOME; - _isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-home-iso-')); - process.env.HOME = _isolatedHome; - process.env.USERPROFILE = _isolatedHome; - process.env.GSD_HOME = _isolatedHome; -} -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; - cleanup(_isolatedHome); - _isolatedHome = null; -} - -// ─── Backwards compatibility — no `runtime` set ───────────────────────────── -describe('issue #2517: backwards compat — no runtime key set', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('balanced profile returns Claude alias when runtime absent', () => { - writeConfig(tmpDir, { model_profile: 'balanced' }); - // gsd-planner balanced -> opus - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); - }); - - test('inherit profile still returns "inherit" with no runtime', () => { - writeConfig(tmpDir, { model_profile: 'inherit' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'inherit'); - }); - - test('resolve_model_ids:true still maps alias -> full Claude ID with no runtime', () => { - writeConfig(tmpDir, { model_profile: 'balanced', resolve_model_ids: true }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-opus-4-8'); - }); - - test('resolve_model_ids:"omit" still returns "" with no runtime', () => { - writeConfig(tmpDir, { model_profile: 'balanced', resolve_model_ids: 'omit' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), ''); - }); - - test('effort resolves universally but render param is null when runtime absent', () => { - writeConfig(tmpDir, { model_profile: 'balanced' }); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - // Effort always resolves (universal); rendering without a runtime yields no wire param. - const rendered = renderEffortForRuntime(undefined, eff); - assert.strictEqual(rendered.param, null); - }); - - test('adaptive profile still works without runtime (#1713/#1806)', () => { - writeConfig(tmpDir, { model_profile: 'adaptive' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'haiku'); - }); -}); - -// ─── runtime: "claude" — no-op (preserves Claude-native semantics) ────────── -describe('issue #2517: runtime "claude" is a no-op for resolution (finding #4)', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('runtime:"claude" + balanced returns the alias, not the resolved Claude ID', () => { - // `runtime: "claude"` is the implicit default — it must not silently flip - // resolve_model_ids on. The alias passes through identically to the unset case. - writeConfig(tmpDir, { runtime: 'claude', model_profile: 'balanced' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); - }); - - test('runtime:"claude" + resolve_model_ids:"omit" returns "" (finding #4 regression)', () => { - // The pre-fix bug: runtime:"claude" hijacked the resolution chain and - // returned the resolved Claude ID even when the user explicitly asked for the - // omit semantics. - writeConfig(tmpDir, { - runtime: 'claude', - model_profile: 'quality', - resolve_model_ids: 'omit', - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), ''); - }); - - test('runtime:"claude" + resolve_model_ids:true maps alias -> full Claude ID', () => { - writeConfig(tmpDir, { - runtime: 'claude', - model_profile: 'quality', - resolve_model_ids: true, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-opus-4-8'); - }); - - test('effort is first-class on Claude (emits output_config.effort)', () => { - writeConfig(tmpDir, { runtime: 'claude', model_profile: 'quality' }); - // Under unification, Claude effort is first-class — rendered via output_config.effort. - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('claude', eff); - assert.strictEqual(rendered.param, 'output_config.effort'); - // gsd-planner is heavy tier → default effort 'xhigh' - assert.strictEqual(rendered.value, 'xhigh'); - }); -}); - -// ─── runtime: "codex" — resolves tiers to Codex IDs + reasoning_effort ────── -describe('issue #2517: runtime "codex" — Codex tier resolution', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('opus tier -> gpt-5.6-sol model; heavy-tier agent -> xhigh effort on codex', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); - // gsd-planner quality -> opus -> gpt-5.6-sol (model unchanged) - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); - // gsd-planner is heavy routing tier → effort 'xhigh' → rendered model_reasoning_effort - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'xhigh'); - }); - - test('sonnet tier -> gpt-5.6-terra model; heavy-tier agent -> xhigh effort on codex', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'balanced' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'gpt-5.6-terra'); - // gsd-roadmapper is heavy routing tier → effort 'xhigh' (not catalog medium) - const eff = resolveEffortInternal(tmpDir, 'gsd-roadmapper'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'xhigh'); - }); - - test('haiku tier -> gpt-5.6-luna model; light-tier agent -> low effort on codex', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'budget' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'gpt-5.6-luna'); - // gsd-codebase-mapper is light routing tier → effort 'low' (not catalog medium) - const eff = resolveEffortInternal(tmpDir, 'gsd-codebase-mapper'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'low'); - }); - - test('adaptive profile resolves on Codex (no #1713/#1806 regression)', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'adaptive' }); - // gsd-planner adaptive -> opus -> gpt-5.6-sol - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); - // gsd-codebase-mapper adaptive -> haiku -> gpt-5.6-luna - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'gpt-5.6-luna'); - }); - - test('inherit profile still returns "inherit" on Codex; effort still resolves universally', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'inherit' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'inherit'); - // Unified effort is config-driven (routing_tier_defaults), independent of model_profile. - // gsd-planner (heavy tier) → 'xhigh'; rendered to codex param. - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'xhigh'); - }); - - test('runtime:"codex" beats resolve_model_ids:"omit" (explicit non-Claude opt-in wins)', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - resolve_model_ids: 'omit', - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); - }); -}); - -// ─── Precedence chain ─────────────────────────────────────────────────────── -describe('issue #2517: precedence chain', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('per-agent model_overrides wins over runtime tier resolution', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_overrides: { 'gsd-planner': 'gpt-5.6-luna' }, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-luna'); - }); - - test('model_profile_overrides[runtime][tier] beats built-in defaults', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_profile_overrides: { - codex: { opus: 'gpt-5-pro' }, - }, - }); - // gsd-planner quality -> opus -> overridden to gpt-5-pro - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5-pro'); - // gsd-codebase-mapper quality -> sonnet -> gpt-5.6-terra - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'gpt-5.6-terra'); - }); - - test('partial profile_overrides — only opus overridden, sonnet uses default', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'balanced', - model_profile_overrides: { - codex: { opus: 'gpt-5-pro' }, // only opus overridden - }, - }); - // gsd-planner balanced -> opus -> overridden to gpt-5-pro - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5-pro'); - // gsd-roadmapper balanced -> sonnet -> spec default - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'gpt-5.6-terra'); - }); - - test('per-agent override beats profile override beats default', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_profile_overrides: { codex: { opus: 'gpt-5-pro' } }, - model_overrides: { 'gsd-planner': 'custom-model' }, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'custom-model'); - }); -}); - -// ─── Field-merge semantics — review findings #2 ───────────────────────────── -describe('issue #2517: field-merge of overrides with built-in defaults (finding #2)', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('string-shorthand override: model is overridden; unified effort derives from routing tier', () => { - // `{ codex: { opus: "gpt-5-pro" } }` is the documented shorthand. - // Model is overridden to gpt-5-pro; effort now derives from the universal - // config-driven path (gsd-planner heavy tier → 'xhigh'), not from the catalog. - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_profile_overrides: { codex: { opus: 'gpt-5-pro' } }, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5-pro'); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'xhigh'); - }); - - test('partial-object override (no model) keeps model from built-in; unified effort from routing tier', () => { - // `{ codex: { opus: { reasoning_effort: "low" } } }` preserves the built-in model. - // Under unification, the catalog reasoning_effort field is not read for effort resolution; - // effort comes from routing_tier_defaults (gsd-planner heavy → 'xhigh'). - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_profile_overrides: { codex: { opus: { reasoning_effort: 'low' } } }, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'xhigh'); - }); - - test('full-object override: model replaced; unified effort from routing tier (not catalog field)', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_profile_overrides: { - codex: { opus: { model: 'custom-model', reasoning_effort: 'minimal' } }, - }, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'custom-model'); - // Effort comes from routing_tier_defaults, not the catalog 'minimal' field. - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('codex', eff); - assert.strictEqual(rendered.param, 'model_reasoning_effort'); - assert.strictEqual(rendered.value, 'xhigh'); - }); - - test('resolveTierEntry helper: shorthand merge', () => { - // Direct unit-test of the shared helper used by core + install.js. - const entry = resolveTierEntry({ - runtime: 'codex', - tier: 'opus', - overrides: { codex: { opus: 'gpt-5-pro' } }, - }); - assert.deepStrictEqual(entry, { model: 'gpt-5-pro', reasoning_effort: 'xhigh' }); - }); - - test('resolveTierEntry helper: partial-object merge keeps built-in model', () => { - const entry = resolveTierEntry({ - runtime: 'codex', - tier: 'opus', - overrides: { codex: { opus: { reasoning_effort: 'low' } } }, - }); - assert.deepStrictEqual(entry, { model: 'gpt-5.6-sol', reasoning_effort: 'low' }); - }); - - test('resolveTierEntry helper: unknown runtime + no overrides -> null', () => { - const entry = resolveTierEntry({ - runtime: 'mystery', - tier: 'opus', - overrides: null, - }); - assert.strictEqual(entry, null); - }); -}); - -// ─── Unknown runtime render safety (finding #3 spirit) ────────────────────── -describe('issue #2517: unknown runtime render param is null (effort does not leak to install path)', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('unknown runtime: model resolves via override; render param is null (no wire param leaked)', () => { - // Under unification, effort always resolves (universal), but renderEffortForRuntime - // returns param=null for unknown runtimes — no effort leaks to the install path. - writeConfig(tmpDir, { - runtime: 'mystery', - model_profile: 'quality', - model_profile_overrides: { - mystery: { opus: { model: 'mystery-opus', reasoning_effort: 'xhigh' } }, - }, - }); - // Model still resolves (overrides are honored). - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'mystery-opus'); - // Effort resolves universally but the unknown runtime has no wire param. - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('mystery', eff); - assert.strictEqual(rendered.param, null); - }); - - test('typo runtime "codx": render param is null (no leak into install path)', () => { - writeConfig(tmpDir, { - runtime: 'codx', - model_profile: 'quality', - model_profile_overrides: { codx: { opus: { model: 'gpt-5.6-terra', reasoning_effort: 'xhigh' } } }, - }); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - const rendered = renderEffortForRuntime('codx', eff); - assert.strictEqual(rendered.param, null); - }); -}); - -// ─── Unknown runtime / unknown tier ───────────────────────────────────────── -describe('issue #2517: unknown runtime + safe fallback', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('unknown runtime falls back to Claude-alias safe default (no Codex IDs leaked)', () => { - writeConfig(tmpDir, { runtime: 'mystery-runtime', model_profile: 'quality' }); - // Should NOT emit gpt-5.6-sol — should fall back to Claude alias - const resolved = resolveModelInternal(tmpDir, 'gsd-planner'); - assert.notStrictEqual(resolved, 'gpt-5.6-sol'); - assert.strictEqual(resolved, 'opus'); - }); - - test('unknown runtime + user-provided overrides for that runtime — uses overrides', () => { - writeConfig(tmpDir, { - runtime: 'mystery-runtime', - model_profile: 'quality', - model_profile_overrides: { - 'mystery-runtime': { opus: 'mystery-opus' }, - }, - }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'mystery-opus'); - }); - - test('runtime:"codex" but missing model_profile_overrides[codex] uses spec defaults', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); - // No model_profile_overrides at all — built-in Codex defaults take over - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); - }); -}); - -// ─── Schema validation (config-set time + load time) ──────────────────────── -describe('issue #2517: VALID_CONFIG_KEYS schema', () => { - test('"runtime" is a valid config key', () => { - assert.strictEqual(isValidConfigKey('runtime'), true); - }); - - test('model_profile_overrides.codex.opus is valid', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.opus'), true); - }); - - test('model_profile_overrides.codex.sonnet is valid', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.sonnet'), true); - }); - - test('model_profile_overrides.codex.haiku is valid', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.haiku'), true); - }); - - test('model_profile_overrides.claude.opus is valid', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.claude.opus'), true); - }); - - test('model_profile_overrides with unknown runtime is valid (free-string runtime)', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.acme.opus'), true); - }); - - test('model_profile_overrides with bogus tier is rejected', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.banana'), false); - }); - - test('model_profile_overrides without tier is rejected', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides.codex'), false); - }); - - test('model_profile_overrides root key alone is rejected (must include runtime+tier)', () => { - assert.strictEqual(isValidConfigKey('model_profile_overrides'), false); - }); -}); - -// ─── loadConfig validation warnings (review findings #10, #13) ────────────── -describe('issue #2517: loadConfig warns on unknown runtime/tier (findings #10, #13)', () => { - const { loadConfig } = require('../gsd-core/bin/lib/config-loader.cjs'); - let tmpDir; - let origWrite; - let captured; - beforeEach(() => { - isolateHome(); - tmpDir = createTempProject(); - resetRuntimeWarningCaches(); - captured = []; - origWrite = process.stderr.write.bind(process.stderr); - process.stderr.write = (chunk) => { captured.push(String(chunk)); return true; }; - }); - afterEach(() => { process.stderr.write = origWrite; cleanup(tmpDir); restoreHome(); }); - - test('unknown runtime triggers a stderr warning', () => { - writeConfig(tmpDir, { runtime: 'codx', model_profile: 'quality' }); - loadConfig(tmpDir); - const joined = captured.join(''); - assert.match(joined, /unknown value "codx"/); - }); - - test('known runtime does NOT trigger a runtime warning', () => { - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); - loadConfig(tmpDir); - const joined = captured.join(''); - assert.doesNotMatch(joined, /unknown value/); - }); - - test('unknown tier in overrides triggers a stderr warning', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile_overrides: { codex: { banana: 'whatever' } }, - }); - loadConfig(tmpDir); - const joined = captured.join(''); - assert.match(joined, /unknown tier "banana"/); - }); - - test('unknown runtime in overrides triggers a stderr warning', () => { - writeConfig(tmpDir, { - runtime: 'codex', - model_profile_overrides: { mystery: { opus: 'whatever' } }, - }); - loadConfig(tmpDir); - const joined = captured.join(''); - assert.match(joined, /model_profile_overrides\.mystery\.\* uses unknown runtime/); - }); - - test('every name in KNOWN_RUNTIMES survives the warning gate', () => { - // Smoke check: `KNOWN_RUNTIMES` must list every runtime `bin/install.js` - // emits for, otherwise legitimate users get spammed at every loadConfig. - for (const r of KNOWN_RUNTIMES) { - assert.ok(typeof r === 'string' && r.length > 0); - } - }); -}); - -// ─── End-to-end: per-project config -> Codex TOML emit (finding #1) ───────── -describe('issue #2517: install end-to-end — per-project config reaches Codex TOML (finding #1)', () => { - // Load install.js in test-mode so its module exports are populated. - const prevTestMode = process.env.GSD_TEST_MODE; - process.env.GSD_TEST_MODE = '1'; - const installMod = require('../bin/install.js'); - if (prevTestMode === undefined) delete process.env.GSD_TEST_MODE; - else process.env.GSD_TEST_MODE = prevTestMode; - const { readGsdRuntimeProfileResolver, generateCodexAgentToml } = installMod; - - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('readGsdRuntimeProfileResolver picks up runtime from .planning/config.json', () => { - // No ~/.gsd/defaults.json (HOME is isolated tmpdir). Per-project config alone - // must drive the resolver — pre-fix, it returned null. - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); - const resolver = readGsdRuntimeProfileResolver(tmpDir); - assert.ok(resolver, 'expected a resolver from per-project config'); - assert.strictEqual(resolver.runtime, 'codex'); - const entry = resolver.resolve('gsd-planner'); - assert.deepStrictEqual(entry, { model: 'gpt-5.6-sol', reasoning_effort: 'xhigh' }); - }); - - test('per-project config wins over global ~/.gsd/defaults.json', () => { - fs.mkdirSync(path.join(_isolatedHome, '.gsd'), { recursive: true }); - fs.writeFileSync( - path.join(_isolatedHome, '.gsd', 'defaults.json'), - JSON.stringify({ runtime: 'claude', model_profile: 'budget' }) - ); - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); - const resolver = readGsdRuntimeProfileResolver(tmpDir); - assert.strictEqual(resolver.runtime, 'codex'); - const entry = resolver.resolve('gsd-planner'); - assert.strictEqual(entry.model, 'gpt-5.6-sol'); - }); - - test('generated Codex TOML omits model = and model_reasoning_effort = lines when only the resolver would have supplied them (#3241)', () => { - // #3241 flips this: the runtime-resolver auto-embed (D1) was removed, so a - // resolver alone with no explicit model_overrides no longer pins a model, - // and #838's coupling means the reasoning-effort line is omitted too. - writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); - const resolver = readGsdRuntimeProfileResolver(tmpDir); - const toml = generateCodexAgentToml( - 'gsd-planner', - '---\nname: gsd-planner\ndescription: Planner agent\n---\nBody.\n', - null, - resolver - ); - assert.doesNotMatch(toml, /^model = "gpt-5\.6-sol"$/m); - assert.doesNotMatch(toml, /^model_reasoning_effort = "xhigh"$/m); - }); - - test('generated TOML always includes model_reasoning_effort even when model_profile_overrides sets reasoning_effort to empty (#443 unified) (#3241: model now pinned via explicit model_overrides, not the resolver alone)', () => { - // Under the unified effort design (#443), model_reasoning_effort in the Codex TOML - // is driven by the unified effort resolver (resolveInstallTimeEffort / effortCfg), - // NOT by model_profile_overrides.reasoning_effort. Setting reasoning_effort: '' in - // model_profile_overrides does NOT suppress the unified effort when a model IS - // pinned — the TOML carries a valid model_reasoning_effort drawn from the agent's - // routing tier. - // #3241: the resolver alone no longer pins a model (D1), so this test now supplies - // an explicit model_overrides pin ('custom', a real-looking Codex id — row 4, - // "unchanged") to keep exercising the unrelated property under test: that - // model_profile_overrides.reasoning_effort is ignored by the unified resolver. - // gsd-planner is a heavy-tier agent → unified default resolves to "xhigh". - writeConfig(tmpDir, { - runtime: 'codex', - model_profile: 'quality', - model_profile_overrides: { codex: { opus: { model: 'custom', reasoning_effort: '' } } }, - }); - const resolver = readGsdRuntimeProfileResolver(tmpDir); - const toml = generateCodexAgentToml( - 'gsd-planner', - '---\nname: gsd-planner\n---\nBody.\n', - { 'gsd-planner': 'custom' }, - resolver - ); - // Explicit model_overrides pin is respected (#3241 row 4 — unchanged). - assert.match(toml, /^model = "custom"$/m); - // Unified effort always fires when a model is pinned — model_reasoning_effort is - // present and valid, ignoring model_profile_overrides.reasoning_effort. - assert.match(toml, /^model_reasoning_effort = "(minimal|low|medium|high|xhigh)"$/m); - // gsd-planner is heavy-tier, so with no effortCfg the manifest tier default applies → xhigh. - assert.match(toml, /^model_reasoning_effort = "xhigh"$/m); - }); - - test('resolver returns null with no global, no per-project config', () => { - // Sanity: nothing configured -> nothing emitted. Pre-existing back-compat. - const resolver = readGsdRuntimeProfileResolver(tmpDir); - assert.strictEqual(resolver, null); - }); - - test('inline require paths resolve relative to install.js __dirname (finding #6)', () => { - // Defensive: assert the lib files install.js requires actually exist at - // resolver-construction time. Catches accidental relative-path drift in CI. - const installDir = path.dirname(require.resolve('../bin/install.js')); - const libDir = path.join(installDir, '..', 'gsd-core', 'bin', 'lib'); - assert.ok(fs.existsSync(path.join(libDir, 'model-catalog.cjs'))); - assert.ok(fs.existsSync(path.join(libDir, 'model-profiles.cjs'))); - }); -}); - -// ─── RUNTIME_PROFILE_MAP single source of truth (finding #16) ─────────────── -describe('issue #2517: RUNTIME_PROFILE_MAP single source of truth (finding #16)', () => { - test('install.js consumes the same map as model-catalog.cjs', () => { - // `bin/install.js` must NOT carry its own duplicate copy of the map. - // The shared resolver imported in install.js exposes `runtime` and the - // entries through `resolveTierEntry`, so any future drift between the two - // files would surface as a test failure here rather than a silent bug. - const codexOpus = RUNTIME_PROFILE_MAP.codex?.opus; - assert.deepStrictEqual(codexOpus, { model: 'gpt-5.6-sol', reasoning_effort: 'xhigh' }); - const claudeOpus = RUNTIME_PROFILE_MAP.claude?.opus; - assert.deepStrictEqual(claudeOpus, { model: 'claude-opus-4-8' }); - }); -}); - -// #1928: the "gemini" runtime tier-resolution suite was removed with the -// sunset Gemini CLI runtime. The gemini-3.x models remain in the catalog for -// Antigravity (which runs on the Gemini backend and carries its own -// runtimeTierDefaults); Antigravity's tier resolution is covered elsewhere. - -// ─── Issue #2612: qwen runtime tier resolution ─────────────────────────────── -describe('issue #2612: runtime "qwen" — Qwen tier resolution', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('opus tier -> qwen3-max-2026-01-23', () => { - writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'quality' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'qwen3-max-2026-01-23'); - }); - - test('sonnet tier -> qwen3-coder-plus', () => { - writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'balanced' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'qwen3-coder-plus'); - }); - - test('haiku tier -> qwen3-coder-next', () => { - writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'budget' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'qwen3-coder-next'); - }); - - test('qwen: effort resolves universally but render param is null (no wire param)', () => { - writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'quality' }); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - assert.strictEqual(renderEffortForRuntime('qwen', eff).param, null); - }); -}); - -// ─── Issue #2612: opencode runtime tier resolution ─────────────────────────── -describe('issue #2612: runtime "opencode" — OpenCode tier resolution', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('opus tier -> anthropic/claude-opus-4-8', () => { - writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'quality' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); - }); - - test('sonnet tier -> anthropic/claude-sonnet-5', () => { - writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'balanced' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-5'); - }); - - test('haiku tier -> anthropic/claude-haiku-4-5', () => { - writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'budget' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); - }); - - test('opencode: effort resolves universally but render param is null (no wire param)', () => { - writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'quality' }); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - assert.strictEqual(renderEffortForRuntime('opencode', eff).param, null); - }); -}); - -// ─── Issue #2093: kilo runtime tier resolution ─────────────────────────────── -// Kilo is an OpenCode fork and shares the IDENTICAL built-in tier IDs (UPGRADE 2 -// / ADR-1239). Kilo moved from Group B (no built-in defaults) to Group A here. -describe('issue #2093: runtime "kilo" — Kilo tier resolution', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('opus tier -> anthropic/claude-opus-4-8', () => { - writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'quality' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); - }); - - test('sonnet tier -> anthropic/claude-sonnet-5', () => { - writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'balanced' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-5'); - }); - - test('haiku tier -> anthropic/claude-haiku-4-5', () => { - writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'budget' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); - }); - - test('kilo: effort resolves universally but render param is null (no wire param)', () => { - writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'quality' }); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - assert.strictEqual(renderEffortForRuntime('kilo', eff).param, null); - }); -}); - -// ─── Issue #2612: copilot runtime tier resolution ──────────────────────────── -describe('issue #2612: runtime "copilot" — Copilot tier resolution', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('opus tier -> claude-opus-4-8', () => { - writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'quality' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-opus-4-8'); - }); - - test('sonnet tier -> claude-sonnet-5', () => { - writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'balanced' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'claude-sonnet-5'); - }); - - test('haiku tier -> claude-haiku-4-5', () => { - writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'budget' }); - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'claude-haiku-4-5'); - }); - - test('copilot: effort resolves universally but render param is null (no wire param)', () => { - writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'quality' }); - const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); - assert.strictEqual(renderEffortForRuntime('copilot', eff).param, null); - }); -}); - -// ─── Issue #2612: Group B runtimes fall through (no built-in map) ──────────── -describe('issue #2612: Group B runtimes — no built-in map, use unknown-runtime fallback', () => { - test('cursor is not in RUNTIME_PROFILE_MAP (uses unknown-runtime fallback)', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.cursor, undefined); - }); - - test('windsurf is not in RUNTIME_PROFILE_MAP', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.windsurf, undefined); - }); - - test('cline is not in RUNTIME_PROFILE_MAP', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.cline, undefined); - }); - - test('augment is not in RUNTIME_PROFILE_MAP', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.augment, undefined); - }); - - test('trae is not in RUNTIME_PROFILE_MAP', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.trae, undefined); - }); - - test('codebuddy is not in RUNTIME_PROFILE_MAP', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.codebuddy, undefined); - }); - - test('antigravity is not in RUNTIME_PROFILE_MAP', () => { - assert.strictEqual(RUNTIME_PROFILE_MAP.antigravity, undefined); - }); - - test('cursor runtime falls back to Claude alias (not a Gemini/Qwen/etc ID)', () => { - const { createTempProject, cleanup } = require('./helpers.cjs'); - isolateHome(); - const tmpDir = createTempProject(); - resetRuntimeWarningCaches(); - try { - writeConfig(tmpDir, { runtime: 'cursor', model_profile: 'quality' }); - // Should fall back to Claude alias, not emit a provider-specific ID - const resolved = resolveModelInternal(tmpDir, 'gsd-planner'); - assert.strictEqual(resolved, 'opus'); - } finally { - cleanup(tmpDir); - restoreHome(); - } - }); -}); - -// ─── Issue #2612: Partial override merge for new runtimes ──────────────────── -describe('issue #2612: partial override merge for new Group A runtimes', () => { - let tmpDir; - beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); - afterEach(() => { cleanup(tmpDir); restoreHome(); }); - - test('qwen.opus override wins; sonnet and haiku use built-in defaults', () => { - writeConfig(tmpDir, { - runtime: 'qwen', - model_profile: 'quality', - model_profile_overrides: { - qwen: { opus: 'qwen3-max-custom' }, - }, - }); - // opus is overridden - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'qwen3-max-custom'); - // sonnet not overridden — quality -> sonnet for gsd-codebase-mapper - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'qwen3-coder-plus'); - }); - - test('opencode.sonnet override wins; opus and haiku still use built-in defaults', () => { - writeConfig(tmpDir, { - runtime: 'opencode', - model_profile: 'balanced', - model_profile_overrides: { - opencode: { sonnet: 'anthropic/claude-sonnet-4-7' }, - }, - }); - // gsd-planner balanced -> opus -> built-in default - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); - // gsd-roadmapper balanced -> sonnet -> overridden - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-4-7'); - // gsd-codebase-mapper balanced -> haiku -> built-in default (haiku not overridden) - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); - }); - - test('copilot.haiku override wins; opus and sonnet still use built-in defaults', () => { - writeConfig(tmpDir, { - runtime: 'copilot', - model_profile: 'budget', - model_profile_overrides: { - copilot: { haiku: 'claude-haiku-4-6' }, - }, - }); - // gsd-codebase-mapper budget -> haiku -> overridden - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'claude-haiku-4-6'); - // gsd-planner budget -> sonnet -> built-in default - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-sonnet-5'); - }); - - // #2093: kilo just moved into Group A — same partial-merge coverage as opencode. - test('kilo.sonnet override wins; opus and haiku still use built-in defaults', () => { - writeConfig(tmpDir, { - runtime: 'kilo', - model_profile: 'balanced', - model_profile_overrides: { - kilo: { sonnet: 'anthropic/claude-sonnet-4-7' }, - }, - }); - // gsd-planner balanced -> opus -> built-in default - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); - // gsd-roadmapper balanced -> sonnet -> overridden - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-4-7'); - // gsd-codebase-mapper balanced -> haiku -> built-in default (haiku not overridden) - assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); - }); -}); diff --git a/tests/issue-2828-flat-roadmap-total-phases.test.cjs b/tests/issue-2828-flat-roadmap-total-phases.test.cjs deleted file mode 100644 index e89de566d..000000000 --- a/tests/issue-2828-flat-roadmap-total-phases.test.cjs +++ /dev/null @@ -1,94 +0,0 @@ -'use strict'; - -// Regression guard for #2828: on a flat unmilestoned roadmap (no versioned milestone -// heading), `state-snapshot`/`state record-session` reported progress.total_phases as -// the on-disk phase-dir count (1) instead of the authoritative roadmap count (6). The -// read-path disk-scan cache fell back to phaseDirs.length when milestoneBounded was -// false, even though roadmapPhaseCount (6) was correct for a flat roadmap (no sibling -// milestones to conflate). Fix: use roadmapPhaseCount as the floor when > 0. - -const { test, describe, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); - -describe('#2828 — total_phases uses the roadmap count on a flat unmilestoned roadmap', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject('gsd-2828-'); - const planningDir = path.join(tmpDir, '.planning'); - // Flat unmilestoned roadmap with 6 phases (no versioned milestone heading). - fs.writeFileSync( - path.join(planningDir, 'ROADMAP.md'), - [ - '# Roadmap', - '', - '### Phase 1: Foundation', - '### Phase 2: Core API', - '### Phase 3: UI Layer', - '### Phase 4: Integration', - '### Phase 5: Polish', - '### Phase 6: Release', - '', - ].join('\n'), - ); - // Only phase 1 has been discussed → 1 phase dir on disk. - const phaseDir = path.join(planningDir, 'phases', '01-foundation'); - fs.mkdirSync(phaseDir, { recursive: true }); - fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), '# Phase 1 Context\n'); - // Minimal STATE.md with a milestone set (so milestoneBounded is computed) but no - // versioned heading to bound it to → the flat-roadmap unbounded case. - fs.writeFileSync( - path.join(planningDir, 'STATE.md'), - [ - '---', - 'status: executing', - 'milestone: v1.0', - 'milestone_name: milestone', - '---', - '', - '# Project State', - '', - '**Current Phase:** 01', - '**Status:** In progress', - '', - ].join('\n'), - ); - }); - - afterEach(() => cleanup(tmpDir)); - - test('state sync writes progress.total_phases === 6 (roadmap count), not 1 (phase-dir count) (#2828)', () => { - // `state sync` derives progress.total_phases from the disk-scan cache (the read path - // #2828 fixes) and writes it to STATE.md frontmatter. Pre-fix this wrote 1. - const result = runGsdTools(['state', 'sync'], tmpDir); - assert.ok(result.success, `state sync failed: ${result.error}`); - - const stateMd = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8'); - // Parse the `progress:` YAML block line-by-line (ReDoS-safe: avoids a nested-quantifier - // regex over the whole block). Find total_phases among the block's indented children. - const lines = stateMd.split(/\r?\n/); - let inProgress = false; - let totalPhases = null; - for (const line of lines) { - if (/^progress:\s*$/.test(line)) { inProgress = true; continue; } - if (inProgress) { - // A new top-level (column-0) key ends the progress block. - if (/^\S/.test(line)) { inProgress = false; continue; } - const tp = line.match(/^\s+total_phases:\s*(\d+)/); - if (tp) { totalPhases = Number(tp[1]); break; } - } - } - assert.ok( - totalPhases !== null, - `progress.total_phases must be written by state sync. STATE.md:\n${stateMd}`, - ); - assert.strictEqual( - totalPhases, - 6, - `progress.total_phases must be the roadmap count (6) for a flat unmilestoned roadmap, not the on-disk phase-dir count (1). Got: ${totalPhases}`, - ); - }); -}); diff --git a/tests/issue-2927-reviewer-lane-overlay-invocation.test.cjs b/tests/issue-2927-reviewer-lane-overlay-invocation.test.cjs deleted file mode 100644 index 55a422dbc..000000000 --- a/tests/issue-2927-reviewer-lane-overlay-invocation.test.cjs +++ /dev/null @@ -1,322 +0,0 @@ -'use strict'; -process.env.GSD_TEST_MODE = '1'; - -/** - * Regression test for #2927 — third-party reviewer lane installs and is - * roster-visible, but `review-lane sections|flags|plan|invoke` cannot select, - * plan, or invoke it. - * - * Root cause: `routeReviewLane` (gsd-core/bin/gsd-tools.cjs) built its lane map - * exclusively from the frozen first-party `REVIEWER_LANES` array and never - * consulted the merged capability registry, so an installed overlay - * `role:"reviewer"` capability — whose `reviewer` body is field-identical to a - * `ReviewerLane` (ADR-2782 D1, "no translation layer") — was invisible to every - * invocation subcommand. - * - * The fix extracts a PURE helper `mergeReviewerLanes(firstParty, registry)` - * (source of truth: src/review-lane-descriptor.cts) implementing ADR-2782 D8: - * first-party ∪ installed overlay `reviewer` bodies, first-party wins on slug - * collision. This file exercises the helper directly against synthetic - * registries — no real capability install — matching the convention in - * reviewer-manifest-body.test.cjs / review-lane-invocation.test.cjs. - * - * Matrix: .gsd/bug/fix/2927-reviewer-lane-overlay-invocation/50-test-matrix.md - */ - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); - -const { - REVIEWER_LANES, - mergeReviewerLanes, - LANE_SLUG_RE, -} = require('../gsd-core/bin/lib/review-lane-descriptor.cjs'); - -/** A first-party lane set small enough to read at a glance, but real-shaped. */ -const FP = REVIEWER_LANES.slice(0, 2); // gemini, claude -const FP_SLUGS = FP.map((l) => l.slug); - -/** A valid overlay `reviewer` body, field-identical to a SpawnLane (ADR-2782 D1). */ -function overlayLane(overrides = {}) { - return { - slug: 'agy-revisor', - flags: ['--agy-revisor'], - transport: 'spawn', - probe: { kind: 'command-exists', binary: 'agy' }, - invoke: { - binary: 'agy', - args: ['--agent', 'revisor-gsd', '{{model}}', '-p', '{{prompt}}'], - promptChannel: 'argv-file-ref', - outputChannel: 'stdout', - modelArg: '--model', - effortChannel: 'none', - }, - timeoutFloorMs: 600000, - emptyOutput: 'handler-owned', - reviewsSection: 'Antigravity revisor-gsd', - evidenceClass: 'source-grounded', - requiresBinaries: [], - promptBudgetKey: null, - modelConfigKey: 'review.models.agy-revisor', - handler: 'antigravity', - ...overrides, - }; -} - -/** A `role:"reviewer"` capability envelope carrying a reviewer body. */ -function reviewerCap(body) { - return { id: body && typeof body === 'object' && body.slug ? body.slug : 'x', role: 'reviewer', reviewer: body }; -} - -/** Build a synthetic registry shape ({ capabilities: { id: cap } }). */ -function registry(...caps) { - const capabilities = {}; - for (const c of caps) capabilities[c.id] = c; - return { capabilities }; -} - -describe('mergeReviewerLanes (#2927)', () => { - test('overlayAbsentReturnsFirstPartyUnchanged', () => { - // Row 1: no overlay reviewer caps → merged set is first-party exactly. - const merged = mergeReviewerLanes(FP, registry()); - assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS); - assert.equal(merged.length, FP.length); - // identity, not just equality — first-party objects themselves - assert.equal(merged[0], FP[0]); - assert.equal(merged[1], FP[1]); - }); - - test('overlayLaneIncludedInMerge', () => { - // Row 2 (failing-first regression): one valid non-colliding overlay lane is present. - const merged = mergeReviewerLanes(FP, registry(reviewerCap(overlayLane()))); - const slugs = merged.map((l) => l.slug); - assert.ok(slugs.includes('agy-revisor'), 'overlay slug admitted into merged set'); - assert.ok(slugs.includes('gemini'), 'first-party lanes preserved'); - // the overlay body itself is the merged entry (no translation layer) - const overlay = merged.find((l) => l.slug === 'agy-revisor'); - assert.equal(overlay.reviewsSection, 'Antigravity revisor-gsd'); - assert.deepEqual(overlay.flags, ['--agy-revisor']); - }); - - test('firstPartyWinsOnSlugCollision', () => { - // Row 3 / D8: an overlay declaring a first-party slug is superseded by first-party. - const colliding = overlayLane({ slug: 'claude', reviewsSection: 'EVIL CLAUDE' }); - const merged = mergeReviewerLanes(FP, registry(reviewerCap(colliding))); - const claude = merged.find((l) => l.slug === 'claude'); - assert.equal(claude, FP.find((l) => l.slug === 'claude'), 'first-party identity wins'); - assert.notEqual(claude.reviewsSection, 'EVIL CLAUDE', 'overlay did not leak through'); - assert.equal(merged.length, FP.length, 'collision added no extra entry'); - }); - - test('runtimeCapWithoutReviewerBodyAddsNoLane', () => { - // Row 4: a role:"runtime" cap with only the legacy reviewerCli alias has no lane descriptor. - const runtimeCap = { id: 'some-runtime', role: 'runtime', runtime: { hostBehaviors: { reviewerCli: true } } }; - const merged = mergeReviewerLanes(FP, registry(runtimeCap)); - assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS, 'runtime alias contributed no lane'); - }); - - test('emptySlugOverlaySkippedNotThrown', () => { - // Row 5: an overlay body whose slug is empty/whitespace is skipped, never throws. - const empty = reviewerCap(overlayLane({ slug: ' ' })); - const missing = reviewerCap(overlayLane({ slug: '' })); - assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(empty))); - assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(missing))); - const merged = mergeReviewerLanes(FP, registry(empty, missing)); - assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS, 'empty-slug overlays admitted no lane'); - }); - - test('invalidGrammarSlugSkipped', () => { - // Row 6 / security: a slug outside LANE_SLUG_RE (path-traversal class) is skipped at the merge. - const evil = reviewerCap(overlayLane({ slug: '../evil' })); - assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(evil))); - const merged = mergeReviewerLanes(FP, registry(evil)); - assert.ok(!merged.map((l) => l.slug).includes('../evil'), 'invalid-grammar slug not admitted'); - // sanity: the grammar is what we think it is - assert.ok(!LANE_SLUG_RE.test('../evil')); - assert.ok(LANE_SLUG_RE.test('agy-revisor')); - }); - - test('twoOverlaysBothIncluded', () => { - // Row 7: two distinct non-colliding overlays both present; count == fp + 2. - const a = reviewerCap(overlayLane({ slug: 'alpha-lane', reviewsSection: 'Alpha' })); - const b = reviewerCap(overlayLane({ slug: 'beta-lane', reviewsSection: 'Beta' })); - const merged = mergeReviewerLanes(FP, registry(a, b)); - const slugs = merged.map((l) => l.slug); - assert.ok(slugs.includes('alpha-lane')); - assert.ok(slugs.includes('beta-lane')); - assert.equal(merged.length, FP.length + 2); - }); - - test('malformedReviewerBodySkipped', () => { - // Row 8: reviewer body that is null / array / string is skipped, no throw. - const nullBody = { id: 'n', role: 'reviewer', reviewer: null }; - const arrBody = { id: 'a', role: 'reviewer', reviewer: [] }; - const strBody = { id: 's', role: 'reviewer', reviewer: 'not-an-object' }; - assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(nullBody, arrBody, strBody))); - const merged = mergeReviewerLanes(FP, registry(nullBody, arrBody, strBody)); - assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS, 'malformed bodies admitted no lane'); - }); -}); - -// --------------------------------------------------------------------------- -// Rows 9–10: the WIRING defect this PR exists to close. The eight rows above -// guard the pure helper, but the actual bug was that `routeReviewLane` never -// CALLED any merge — so a revert of the one-line wiring change would leave every -// helper test green. These rows exercise the real CLI end-to-end: install a -// global-scope `role:"reviewer"` overlay (global scope is trusted without a -// consent record, CONTEXT.md capability-loader predicate), then assert -// `review-lane sections|flags|plan` actually see it through loadRegistry → -// mergeReviewerLanes → the lane map. This is acceptance criteria #1–#3. -// --------------------------------------------------------------------------- - -const fs = require('node:fs'); -const os = require('node:os'); -const nodePath = require('node:path'); -const { runGsdTools, cleanup } = require('./helpers.cjs'); - -const cliTmps = []; -function cliTmpDir(prefix) { - const d = fs.mkdtempSync(nodePath.join(os.tmpdir(), prefix)); - cliTmps.push(d); - return d; -} -test.after(() => { for (const d of cliTmps) cleanup(d); }); - -/** A GSD_HOME-sandboxed env that neutralizes ambient GSD_ vars (hermeticity). */ -function scopeEnv(home) { - return { GSD_HOME: home, GSD_WORKSTREAM: '', GSD_PROJECT: '' }; -} - -/** A cwd with a .planning/ root so findProjectRoot resolves cleanly. */ -function makeCwd() { - const cwd = cliTmpDir('rev2927-cwd-'); - fs.mkdirSync(nodePath.join(cwd, '.planning'), { recursive: true }); - fs.writeFileSync(nodePath.join(cwd, '.planning', 'config.json'), '{}'); - return cwd; -} - -/** - * Write a conformant `role:"reviewer"` capability source dir whose `reviewer` - * body is a valid SpawnLane (ADR-2782 D1 shape). Returns the source path, - * usable as a `capability install ` argument. - */ -function writeReviewerCapSource(id, bodyOverrides = {}) { - const src = cliTmpDir(`rev2927-src-${id}-`); - // A `role:"reviewer"` manifest carries ONLY id/role/version/title/description/ - // tier/requires/engines/reviewer (+ optional config) — skills/agents/steps/ - // contributions/gates/hooks/runtimeCompat are feature-only fields the validator - // rejects for a reviewer (mirrors the shipped `capabilities/lm-studio` shape). - const cap = { - id, - role: 'reviewer', - version: '1.0.0', - title: `${id} test lane`, - description: 'test third-party reviewer lane for #2927', - tier: 'standard', - requires: [], - engines: { gsd: '>=1.9.0' }, - reviewer: { - slug: id, - flags: [`--${id}`], - transport: 'spawn', - probe: { kind: 'command-exists', binary: id }, - invoke: { - binary: id, - args: ['{{model}}', '-p', '{{prompt}}'], - promptChannel: 'stdin', - outputChannel: 'stdout', - modelArg: '--model', - effortChannel: 'none', - }, - timeoutFloorMs: 600000, - emptyOutput: 'stub-with-stderr', - reviewsSection: `${id} review`, - evidenceClass: 'source-grounded', - requiresBinaries: [], - promptBudgetKey: null, - modelConfigKey: `review.models.${id}`, - handler: null, - ...bodyOverrides, - }, - }; - fs.writeFileSync(nodePath.join(src, 'capability.json'), JSON.stringify(cap, null, 2)); - return src; -} - -describe('review-lane CLI overlay invocation (#2927, rows 9–10)', () => { - test('cliSectionsAndPlanSeeOverlayLane', () => { - // Acceptance #1 + #3: an installed overlay lane appears in `sections` and - // `plan --selected ` returns ok:true with a usable plan. - const home = cliTmpDir('rev2927-home-'); - const cwd = makeCwd(); - const src = writeReviewerCapSource('rev2927lane'); - // Global scope is trusted without a consent record; --yes acknowledges the - // executable reviewer surface; --raw emits JSON. - const install = runGsdTools( - ['capability', 'install', src, '--scope', 'global', '--yes', '--raw'], - cwd, - scopeEnv(home), - ); - assert.equal(install.success, true, `install failed: ${install.error || install.output}`); - const installOut = JSON.parse(install.output); - assert.equal(installOut.status, 'installed', `install did not report installed: ${install.output}`); - - // Row 9 / acceptance #1: sections includes the overlay slug + reviewsSection. - const sections = runGsdTools(['review-lane', 'sections'], cwd, scopeEnv(home)); - assert.equal(sections.success, true, `sections failed: ${sections.error || sections.output}`); - const sectionRows = sections.output.split('\n').filter(Boolean); - const overlayRow = sectionRows.find((r) => r.startsWith('rev2927lane\t')); - assert.ok(overlayRow, `overlay lane missing from sections output:\n${sections.output}`); - assert.equal(overlayRow, 'rev2927lane\trev2927lane review'); - - // Row 9 / acceptance #3: plan --selected resolves ok (NOT - // malformed_lane / no such declared lane — the pre-fix failure). The `plan` - // subcommand renders an ARRAY of {slug, ok, section, transport, ...} (it strips - // the nested invocation `plan` object before output), so find the overlay entry. - const plan = runGsdTools( - ['review-lane', 'plan', '--selected', 'rev2927lane', '--run-dir', cwd, '--repo-root', cwd], - cwd, - scopeEnv(home), - ); - assert.equal(plan.success, true, `plan failed: ${plan.error || plan.output}`); - const planOut = JSON.parse(plan.output); - assert.ok(Array.isArray(planOut), `plan output is not an array:\n${plan.output}`); - const overlayPlan = planOut.find((p) => p.slug === 'rev2927lane'); - assert.ok(overlayPlan, `overlay plan entry missing:\n${plan.output}`); - assert.equal(overlayPlan.ok, true, `overlay plan did not resolve ok:\n${plan.output}`); - assert.equal(overlayPlan.section, 'rev2927lane review'); - assert.equal(overlayPlan.transport, 'spawn'); - }); - - test('cliFlagsIncludeOverlayFlag', () => { - // Acceptance #2: the overlay's declared --flag appears in `flags` output. - // - // NOTE on the negative-space "malformed flag filtered" case: the capability - // validator enforces the /^--[a-z0-9][a-z0-9-]*$/ flag grammar AT INSTALL TIME - // (capability-validator rejects a reviewer.flags entry that fails it), so a lane - // carrying a malformed flag (e.g. `--bad flag`, `*.js`) can never be installed - // and therefore never reaches the `flags` shape filter. That filter is - // defense-in-depth over an input class the validator already excludes; it is - // not independently reachable through a validated install, so it is not asserted - // here. A lane declaring two well-formed flags (mirroring antigravity's - // --antigravity/--agy) proves the per-lane flag array is preserved, not flattened. - const home = cliTmpDir('rev2927-home-'); - const cwd = makeCwd(); - const src = writeReviewerCapSource('rev2927flag', { - flags: ['--rev2927flag', '--rev2927alt'], - }); - const install = runGsdTools( - ['capability', 'install', src, '--scope', 'global', '--yes', '--raw'], - cwd, - scopeEnv(home), - ); - assert.equal(install.success, true, `install failed: ${install.error || install.output}`); - assert.equal(JSON.parse(install.output).status, 'installed', `install did not report installed: ${install.output}`); - - const flags = runGsdTools(['review-lane', 'flags'], cwd, scopeEnv(home)); - assert.equal(flags.success, true, `flags failed: ${flags.error || flags.output}`); - const flagLines = flags.output.split('\n').filter(Boolean); - assert.ok(flagLines.includes('--rev2927flag'), `overlay flag missing from flags output:\n${flags.output}`); - assert.ok(flagLines.includes('--rev2927alt'), 'second well-formed overlay flag missing (flag array flattened?)'); - }); -}); diff --git a/tests/issue-2939-dispatch-flatten-maxdepth.test.cjs b/tests/issue-2939-dispatch-flatten-maxdepth.test.cjs deleted file mode 100644 index 7d19fe1b3..000000000 --- a/tests/issue-2939-dispatch-flatten-maxdepth.test.cjs +++ /dev/null @@ -1,135 +0,0 @@ -'use strict'; -process.env.GSD_TEST_MODE = '1'; - -/** - * Regression test for #2939 — `shouldFlattenDispatch` ignores the declared - * depth budget, so a runtime advertising `maxDepth:1` (no room for a background - * orchestrator plus a delegated leaf) is still told it may background. - * - * Root cause: `shouldFlattenDispatch` (src/host-integration.cts) checked ONLY - * `dispatch.background` and `dispatch.backgroundDispatch`, never `nested`, - * `subagentToolkit`, or `maxDepth`. With the live Codex descriptor - * (background:true, backgroundDispatch:true, nested:true, subagentToolkit:"full", - * maxDepth:1) it returned `shouldFlatten:false`, which then permitted a depth-2 - * orchestration tree the declared contract cannot support. - * - * The fix reconciles `shouldFlattenDispatch` with the depth-budget convention - * already used in the same file (`degradationFor`: nested && depth>=2 is - * full-depth; maxDepth===1 is flat) and in `bin/install.js` - * (`_normalizeDispatchCallSpan`: subagentToolkit==='full' && (maxDepth===-1 || - * maxDepth>1)). A host may background only if it can background AND has a depth - * budget sufficient for a backgrounded orchestrator plus a delegated leaf. - * - * Matrix: .gsd/bug/fix/2939-dispatch-flatten-maxdepth/50-test-matrix.md - */ - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); - -const { shouldFlattenDispatch } = require('../gsd-core/bin/lib/host-integration.cjs'); - -/** The live Codex-shaped descriptor (the bug input), with per-test depth overrides. */ -function codexLike(overrides = {}) { - return { - namedDispatch: true, - nested: true, - maxDepth: 1, - background: true, - subagentToolkit: 'full', - backgroundDispatch: true, - ...overrides, - }; -} - -describe('shouldFlattenDispatch depth budget (#2939)', () => { - test('codexLikeMaxDepth1Flattens', () => { - // Row 1 (failing-first regression): maxDepth:1 cannot host a bg orchestrator - // (depth 1) AND a delegated leaf (depth 2) → must flatten (inline). - assert.strictEqual( - shouldFlattenDispatch(codexLike({ maxDepth: 1 })), - true, - 'maxDepth:1 is insufficient for a backgrounded orchestrator plus a leaf → flatten', - ); - }); - - test('maxDepth2BackgroundsUnchanged', () => { - // Row 2: maxDepth:2 leaves room → background permitted, unchanged from today. - assert.strictEqual( - shouldFlattenDispatch(codexLike({ maxDepth: 2 })), - false, - 'maxDepth:2 is sufficient → background permitted (unchanged)', - ); - }); - - test('maxDepthUnboundedBackgroundsUnchanged', () => { - // Row 3: maxDepth:-1 (unbounded) → background permitted, unchanged. - assert.strictEqual( - shouldFlattenDispatch(codexLike({ maxDepth: -1 })), - false, - 'maxDepth:-1 (unbounded) → background permitted (unchanged)', - ); - }); - - test('nestedFalseFlattensRegardlessOfDepth', () => { - // Row 4 / acceptance #4: nested:false cannot host a nesting orchestrator → - // flatten regardless of maxDepth. - assert.strictEqual( - shouldFlattenDispatch(codexLike({ nested: false, maxDepth: 5 })), - true, - 'nested:false → flatten regardless of maxDepth', - ); - }); - - test('nonFullToolkitFlattens', () => { - // Row 5 / acceptance #4: a non-full toolkit cannot delegate → flatten - // regardless of maxDepth. - assert.strictEqual( - shouldFlattenDispatch(codexLike({ subagentToolkit: 'read-only', maxDepth: 5 })), - true, - 'subagentToolkit!=="full" → flatten regardless of maxDepth', - ); - }); - - test('backgroundFalseStillFlattens', () => { - // Row 6 / negative-space: background:false → flatten (the existing - // background-boolean fail-closed path is unchanged). - assert.strictEqual( - shouldFlattenDispatch(codexLike({ background: false, maxDepth: 5 })), - true, - 'background:false → flatten (unchanged)', - ); - }); - - test('backgroundDispatchFalseStillFlattens', () => { - // Row 7 / negative-space: backgroundDispatch:false → flatten (unchanged). - assert.strictEqual( - shouldFlattenDispatch(codexLike({ backgroundDispatch: false, maxDepth: 5 })), - true, - 'backgroundDispatch:false → flatten (unchanged)', - ); - }); - - test('maxDepth0Flattens', () => { - // Row 9: maxDepth:0 (zero depth budget) → flatten. - assert.strictEqual( - shouldFlattenDispatch(codexLike({ maxDepth: 0 })), - true, - 'maxDepth:0 → flatten (zero depth budget)', - ); - }); - - test('maxDepthMissingFlattens', () => { - // Row 10: maxDepth missing/non-number → flatten (fail-closed on absent - // budget, mirrors degradationFor treating non-finite as 0). - assert.strictEqual( - shouldFlattenDispatch(codexLike({ maxDepth: undefined })), - true, - 'maxDepth missing → flatten (fail-closed)', - ); - assert.strictEqual( - shouldFlattenDispatch(codexLike({ maxDepth: 'deep' })), - true, - 'maxDepth non-number → flatten (fail-closed)', - ); - }); -}); diff --git a/tests/issue-2945-phase-complete-checkbox-rollback.test.cjs b/tests/issue-2945-phase-complete-checkbox-rollback.test.cjs deleted file mode 100644 index 309e1a443..000000000 --- a/tests/issue-2945-phase-complete-checkbox-rollback.test.cjs +++ /dev/null @@ -1,140 +0,0 @@ -'use strict'; -process.env.GSD_TEST_MODE = '1'; - -/** - * Regression test for #2945 — `phase complete`'s inline REQUIREMENTS.md checkbox - * flip is traceability-blind: it flips `- [ ] **REQ-ID**` → `- [x]` unconditionally, - * then attempts the traceability-row write, and KEEPS the flip when the row exists - * but rejects the write (Out/Deferred/Blocked). The sibling `requirements.mark-complete` - * got the #2788 defect-2 rollback; `cmdPhaseComplete`'s inline copy did not. - * - * The fix ports the rollback from `cmdRequirementsMarkComplete` (src/milestone.cts): - * capture beforeCheckbox, track whether the row write actually changed (tableHit), and - * restore beforeCheckbox when the row EXISTS but rejects the write. - * - * Matrix: .gsd/bug/fix/2945-phase-complete-checkbox-rollback/50-test-matrix.md - */ - -const { test, describe, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); - -/** - * Write a passed-VERIFICATION marker for the phase, then run `phase complete N`. - * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` - * with `status: passed` frontmatter. Requires the phase directory to exist. - */ -function runVerifiedPhaseComplete(args, tmpDir) { - const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); - const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); - const phase = argv[completeIdx + 1]; - const phasesDir = path.join(tmpDir, '.planning', 'phases'); - const wanted = parseInt(String(phase).replace(/^0+/, ''), 10); - const phaseDirName = fs.readdirSync(phasesDir).find((name) => { - const m = name.match(/^(\d+)/); - return m && parseInt(m[1], 10) === wanted; - }); - if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); - fs.writeFileSync( - path.join(phasesDir, phaseDirName, `${phase}-VERIFICATION.md`), - ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), - ); - return runGsdTools(args, tmpDir); -} - -describe('phase complete checkbox rollback (#2945)', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject('gsd-2945-'); - }); - afterEach(() => { - cleanup(tmpDir); - }); - - /** - * Scaffold a single-phase project whose ROADMAP cites the given REQ-IDs, with a - * REQUIREMENTS.md whose traceability table rows carry the given statuses, then run - * `phase complete 1`. Returns the REQUIREMENTS.md content after completion. - */ - function completeWithRows(reqIds, rowStatuses) { - const reqList = reqIds.join(', '); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap\n\n- [ ] Phase 1: The phase\n\n### Phase 1: The phase\n**Goal:** do it\n**Requirements:** ${reqList}\n**Plans:** 1 plans\n`, - ); - const reqLines = reqIds.map((id) => `- [ ] **${id}**: a requirement`).join('\n'); - const tableRows = reqIds.map((id, i) => `| ${id} | Phase 1 | ${rowStatuses[i]} |`).join('\n'); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), - `# Requirements\n\n## v1 Requirements\n\n${reqLines}\n\n## Traceability\n\n| Requirement | Phase | Status |\n|-------------|-------|--------|\n${tableRows}\n`, - ); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# State\n\n**Current Phase:** 01\n**Current Phase Name:** The phase\n**Status:** In progress\n**Current Plan:** 01-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n`, - ); - const p1 = path.join(tmpDir, '.planning', 'phases', '01-the-phase'); - fs.mkdirSync(p1, { recursive: true }); - fs.writeFileSync(path.join(p1, '01-01-PLAN.md'), '# Plan'); - fs.writeFileSync(path.join(p1, '01-01-SUMMARY.md'), '# Summary'); - - const result = runVerifiedPhaseComplete('phase complete 1', tmpDir); - assert.ok(result.success, `phase complete failed: ${result.error || result.output}`); - return fs.readFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), 'utf-8'); - } - - test('deferredRowRollsBackCheckbox', () => { - // Row 1 (failing-first regression): a Deferred row rejects the Status write, so the - // checkbox must NOT flip (it stays [ ]), and the row must stay Deferred. - const req = completeWithRows(['DEF-01'], ['Deferred']); - assert.ok(req.includes('- [ ] **DEF-01**'), 'checkbox must stay [ ] when the row is Deferred (no silent divergence)'); - assert.ok(/^\| DEF-01 \| Phase 1 \| Deferred \|/m.test(req), 'row must stay Deferred'); - assert.ok(!req.includes('- [x] **DEF-01**'), 'checkbox must NOT have flipped to [x]'); - }); - - test('blockedRowRollsBackCheckbox', () => { - // Row 2: an Out/Blocked row likewise rejects the write → checkbox stays [ ]. - const req = completeWithRows(['BLK-01'], ['Blocked']); - assert.ok(req.includes('- [ ] **BLK-01**'), 'checkbox must stay [ ] when the row is Blocked'); - assert.ok(/^\| BLK-01 \| Phase 1 \| Blocked \|/m.test(req), 'row must stay Blocked'); - assert.ok(!req.includes('- [x] **BLK-01**'), 'checkbox must NOT have flipped to [x]'); - }); - - test('pendingRowStillFlipsAndAdvances', () => { - // Row 3 (negative-space / unchanged forward behavior): a Pending row accepts the write, - // so the checkbox MUST flip to [x] and the row MUST advance to Complete. An over-broad - // rollback would break this. - const req = completeWithRows(['FWD-01'], ['Pending']); - assert.ok(req.includes('- [x] **FWD-01**'), 'checkbox MUST flip to [x] for a Pending (forward) row'); - assert.ok(/^\| FWD-01 \| Phase 1 \| Complete \|/m.test(req), 'row MUST advance to Complete'); - }); - - test('noRowStillFlipsCheckbox', () => { - // Row 4 (acceptance #3): a cited REQ-ID with NO traceability row → checkbox still flips - // (nothing to disagree with). The rollback only fires when a row EXISTS and rejects. - fs.writeFileSync( - path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap\n\n- [ ] Phase 1: The phase\n\n### Phase 1: The phase\n**Goal:** do it\n**Requirements:** NOROW-01\n**Plans:** 1 plans\n`, - ); - // REQUIREMENTS.md with the checkbox but NO traceability table. - fs.writeFileSync( - path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), - `# Requirements\n\n## v1 Requirements\n\n- [ ] **NOROW-01**: a requirement with no traceability row\n`, - ); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# State\n\n**Current Phase:** 01\n**Current Phase Name:** The phase\n**Status:** In progress\n**Current Plan:** 01-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n`, - ); - const p1 = path.join(tmpDir, '.planning', 'phases', '01-the-phase'); - fs.mkdirSync(p1, { recursive: true }); - fs.writeFileSync(path.join(p1, '01-01-PLAN.md'), '# Plan'); - fs.writeFileSync(path.join(p1, '01-01-SUMMARY.md'), '# Summary'); - - const result = runVerifiedPhaseComplete('phase complete 1', tmpDir); - assert.ok(result.success, `phase complete failed: ${result.error || result.output}`); - const req = fs.readFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), 'utf-8'); - assert.ok(req.includes('- [x] **NOROW-01**'), 'checkbox MUST flip to [x] when no traceability row exists'); - }); -}); diff --git a/tests/issue-2949-phase-complete-stage3-sentinel.test.cjs b/tests/issue-2949-phase-complete-stage3-sentinel.test.cjs deleted file mode 100644 index a6b0a711b..000000000 --- a/tests/issue-2949-phase-complete-stage3-sentinel.test.cjs +++ /dev/null @@ -1,205 +0,0 @@ -'use strict'; -process.env.GSD_TEST_MODE = '1'; - -/** - * Regression test for #2949 — `phase complete`'s stage-3 lowest-outstanding-override - * loop admits unchecked `0.x` backlog sentinel rows as `next_phase`, corrupting STATE.md - * and desyncing `current_phase` from `current_phase_name`. - * - * Root cause: `src/phase.cts` stage-3 condition `!isChecked && comparePhaseNum(cbm[2], phaseNum) < 0` - * has no sentinel filter, so `comparePhaseNum("0.1","12") === -12` admits the `0.x` backlog row. - * The fix adds `&& !isSentinelPhaseId(cbm[2])` (reusing the existing zero-caller predicate), which - * excludes both sentinel ranges (0 and 999). Stage-3 only here — PR #2815 (in-flight) covers - * stages 1-2 for #2786. - * - * Matrix: .gsd/bug/fix/2949-phase-complete-stage3-sentinel-filter/50-test-matrix.md - */ - -const { test, describe, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); - -/** - * Write a passed-verification marker for a phase, then run `phase complete N`. - * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` - * with `status: passed` frontmatter, plus a SUMMARY for each plan (the completion gate - * requires executed plans). Requires the phase directory to already exist. - */ -function runVerifiedPhaseComplete(args, tmpDir) { - const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); - const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); - const phase = argv[completeIdx + 1]; - const phasesDir = path.join(tmpDir, '.planning', 'phases'); - // Find the phase directory whose leading token matches the requested phase number. - const wantedPadded = String(phase).replace(/^0+/, ''); - const phaseDirName = fs.readdirSync(phasesDir).find((name) => { - const m = name.match(/^(\d+)/); - return m && parseInt(m[1], 10) === parseInt(wantedPadded, 10); - }); - if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); - const phaseDir = path.join(phasesDir, phaseDirName); - fs.writeFileSync( - path.join(phaseDir, `${phase}-VERIFICATION.md`), - ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), - ); - return runGsdTools(args, tmpDir); -} - -describe('phase complete stage-3 sentinel filter (#2949)', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject('gsd-2949-'); - }); - - afterEach(() => { - cleanup(tmpDir); - }); - - /** Scaffold a phase dir with one executed plan (PLAN + SUMMARY). */ - function scaffoldPhase(slug, planNum) { - const dir = path.join(tmpDir, '.planning', 'phases', slug); - fs.mkdirSync(dir, { recursive: true }); - const padded = String(planNum).padStart(2, '0'); - fs.writeFileSync(path.join(dir, `${padded}-01-PLAN.md`), '# Plan'); - fs.writeFileSync(path.join(dir, `${padded}-01-SUMMARY.md`), '# Summary'); - } - - test('zeroXSentinelDoesNotBecomeNextPhase', () => { - // Row 1 (failing-first regression): completing the last real phase with an unchecked - // 0.x backlog sentinel row present must NOT select the sentinel as next_phase. - fs.writeFileSync( - path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap - -- [ ] **Phase 0.1: Backlog sentinel item** — deferred work -- [x] **Phase 11: First phase** (completed 2025-01-01) -- [ ] **Phase 12: Last phase** - -### Phase 11: First phase -**Goal:** first -**Plans:** 1 plans - -### Phase 12: Last phase -**Goal:** last -**Plans:** 1 plans -`, - ); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# State\n\n**Current Phase:** 12\n**Current Phase Name:** Last phase\n**Status:** In progress\n**Current Plan:** 12-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 12\n`, - ); - scaffoldPhase('11-first-phase', 11); - scaffoldPhase('12-last-phase', 12); - - const result = runVerifiedPhaseComplete('phase complete 12', tmpDir); - assert.ok(result.success, `Command failed: ${result.error || result.output}`); - const output = JSON.parse(result.output); - - assert.strictEqual(output.completed_phase, '12'); - assert.strictEqual(output.is_last_phase, true, '0.x sentinel must not prevent milestone completion (is_last_phase=true)'); - assert.strictEqual(output.next_phase, null, '0.x sentinel must not be selected as next_phase'); - - // STATE.md current_phase must stay on the completed phase (12), not advance to 0.1. - const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); - assert.ok(!/\*\*Current Phase:\*\*\s*0\.1/i.test(state), 'STATE.md current_phase must NOT have advanced to the 0.x sentinel'); - }); - - test('realLowerOutstandingPhaseStillSelected', () => { - // Row 2 (#2028 non-regression): a REAL lower-numbered unchecked phase must STILL be - // selected as next_phase. The sentinel filter must not over-broaden to real phases. - fs.writeFileSync( - path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap - -- [ ] **Phase 9: Skipped-then-resumed phase** -- [ ] **Phase 10: Current phase** - -### Phase 9: Skipped-then-resumed phase -**Goal:** nine -**Plans:** 1 plans - -### Phase 10: Current phase -**Goal:** ten -**Plans:** 1 plans -`, - ); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# State\n\n**Current Phase:** 10\n**Current Phase Name:** Current phase\n**Status:** In progress\n**Current Plan:** 10-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 10\n`, - ); - scaffoldPhase('09-skipped-then-resumed-phase', 9); - scaffoldPhase('10-current-phase', 10); - - const result = runVerifiedPhaseComplete('phase complete 10', tmpDir); - assert.ok(result.success, `Command failed: ${result.error || result.output}`); - const output = JSON.parse(result.output); - - // A real lower phase (9) IS selected — the #2028 out-of-order behavior is preserved. - // next_phase may be padded ("09") or unpadded ("9"); compare numerically. - assert.strictEqual(output.is_last_phase, false, 'a real lower outstanding phase must keep is_last_phase=false'); - const nextNum = parseInt(String(output.next_phase), 10); - assert.strictEqual(nextNum, 9, `real lower phase 9 must be selected as next_phase (got ${output.next_phase})`); - }); - - test('zeroXSentinelNoCurrentPhaseDesync', () => { - // Row 3 (acceptance #3/#4): current_phase and current_phase_name must not desync when - // a 0.x sentinel is present and the milestone completes. - fs.writeFileSync( - path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap - -- [ ] **Phase 0.1: Backlog** -- [ ] **Phase 5: Only phase** - -### Phase 5: Only phase -**Goal:** five -**Plans:** 1 plans -`, - ); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# State\n\n**Current Phase:** 5\n**Current Phase Name:** Only phase\n**Status:** In progress\n**Current Plan:** 05-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 5\n`, - ); - scaffoldPhase('05-only-phase', 5); - - const result = runVerifiedPhaseComplete('phase complete 5', tmpDir); - assert.ok(result.success, `Command failed: ${result.error || result.output}`); - const output = JSON.parse(result.output); - assert.strictEqual(output.is_last_phase, true, 'milestone completes despite the 0.x sentinel'); - - const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); - // current_phase must NOT have advanced to 0.1 (no desync into the sentinel). - assert.ok(!/\*\*Current Phase:\*\*\s*0\.1/i.test(state), 'no desync: current_phase did not advance to 0.1'); - }); - - test('checkedZeroXSentinelIrrelevant', () => { - // Row 4 (boundary): a CHECKED 0.x sentinel is irrelevant — completing the last real phase - // still completes the milestone. - fs.writeFileSync( - path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap - -- [x] **Phase 0.1: Already-done backlog item** -- [ ] **Phase 3: Last phase** - -### Phase 3: Last phase -**Goal:** three -**Plans:** 1 plans -`, - ); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - `# State\n\n**Current Phase:** 3\n**Current Phase Name:** Last phase\n**Status:** In progress\n**Current Plan:** 03-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 3\n`, - ); - scaffoldPhase('03-last-phase', 3); - - const result = runVerifiedPhaseComplete('phase complete 3', tmpDir); - assert.ok(result.success, `Command failed: ${result.error || result.output}`); - const output = JSON.parse(result.output); - assert.strictEqual(output.is_last_phase, true, 'checked sentinel is irrelevant; milestone completes'); - assert.strictEqual(output.next_phase, null); - }); -}); diff --git a/tests/issue-2977-frontmatter-bom.test.cjs b/tests/issue-2977-frontmatter-bom.test.cjs deleted file mode 100644 index 89000f677..000000000 --- a/tests/issue-2977-frontmatter-bom.test.cjs +++ /dev/null @@ -1,78 +0,0 @@ -'use strict'; -process.env.GSD_TEST_MODE = '1'; - -/** - * Regression test for #2977 — `extractFrontmatter` returns {} for any file whose - * frontmatter fence is preceded by a UTF-8 BOM (Windows PowerShell `>`/`Out-File`, - * several editors). The `startsWith('---')` byte-0 check fails on any leading byte, - * so every frontmatter field silently disappears with no error. - * - * The fix strips a leading UTF-8 BOM (\uFEFF) before the fence check. Scope: BOM only - * (acceptance criteria 1-3). The generalized "arbitrary content before the fence" fork - * (tolerate vs diagnose) is a product-intent decision, surfaced in the PR — out of scope. - * - * Matrix: .gsd/bug/fix/2977-frontmatter-bom-tolerance/50-test-matrix.md - */ - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const { extractFrontmatter } = require('../gsd-core/bin/lib/frontmatter.cjs'); - -const BOM = '\uFEFF'; - -describe('extractFrontmatter BOM tolerance (#2977)', () => { - test('bomPrefixedFrontmatterParses', () => { - // Row 1 (failing-first regression): a BOM-prefixed frontmatter document parses - // identically to the same document without the BOM. - const clean = '---\ntitle: T\nphase: "01"\nstatus: passed\n---\n\n# Body\n'; - const bommed = BOM + clean; - const expected = extractFrontmatter(clean, 'a.md'); - const actual = extractFrontmatter(bommed, 'a.md'); - assert.deepEqual(actual, expected, 'BOM-prefixed frontmatter must parse identically to no-BOM'); - assert.strictEqual(actual.title, 'T', 'title field recovered'); - assert.strictEqual(actual.phase, '01', 'phase field recovered'); - assert.strictEqual(actual.status, 'passed', 'status field recovered'); - }); - - test('bomWithCrlfParses', () => { - // Row 2 (acceptance #2): BOM + CRLF line endings together still parse correctly. - const clean = '---\r\ntitle: T\r\nphase: "01"\r\n---\r\n\r\n# Body\r\n'; - const bommed = BOM + clean; - const actual = extractFrontmatter(bommed, 'a.md'); - assert.strictEqual(actual.title, 'T', 'title recovered (BOM + CRLF)'); - assert.strictEqual(actual.phase, '01', 'phase recovered (BOM + CRLF)'); - }); - - test('bomWithNoFrontmatterStaysEmpty', () => { - // Row 3 (acceptance #3): a BOM prefixing a document with no frontmatter (or genuinely - // empty frontmatter) returns {} with no false diagnostic — same as no-BOM. - assert.deepEqual(extractFrontmatter(BOM + 'just plain text', 'a.md'), {}, 'BOM + no frontmatter -> {}'); - assert.deepEqual(extractFrontmatter(BOM + '', 'a.md'), {}, 'BOM + empty -> {}'); - // A thematic-break-first-line Markdown doc (--- then prose) must stay {} — protected by - // the existing false-positive threshold; the BOM strip must not lower that bar. - assert.deepEqual(extractFrontmatter(BOM + '---\n\nA horizontal rule, not frontmatter.\n', 'a.md'), {}, - 'BOM + thematic-break Markdown -> {} (no false diagnostic)'); - }); - - test('bomAcrossArtifactTypes', () => { - // Row 4 (acceptance #1 across artifact types): each frontmatter-bearing artifact shape - // recovers its fields when BOM-prefixed. - const cases = [ - { name: 'STATE.md', body: '---\ncurrent_phase: "01"\nstatus: "In progress"\n---\n\n# State\n', expect: { current_phase: '01', status: 'In progress' } }, - { name: 'PLAN.md', body: '---\nphase: "01"\nplan: "01-01"\nstatus: "done"\n---\n\n# Plan\n', expect: { phase: '01', plan: '01-01', status: 'done' } }, - { name: 'SUMMARY.md', body: '---\none-liner: "shipped the thing"\n---\n\n# Summary\n', expect: { 'one-liner': 'shipped the thing' } }, - { name: 'UAT.md', body: '---\nphase: "02"\nverdict: "pass"\n---\n\n# UAT\n', expect: { phase: '02', verdict: 'pass' } }, - ]; - for (const c of cases) { - const actual = extractFrontmatter(BOM + c.body, c.name); - assert.deepEqual(actual, c.expect, `${c.name}: BOM-prefixed frontmatter must recover fields`); - } - }); - - test('controlNoBom', () => { - // Row 5 (no regression): no BOM, valid frontmatter still parses correctly (unchanged). - const actual = extractFrontmatter('---\ntitle: T\nphase: "01"\n---\n\n# Body\n', 'a.md'); - assert.strictEqual(actual.title, 'T'); - assert.strictEqual(actual.phase, '01'); - }); -}); diff --git a/tests/issue-3204-state-writer-phase-count.test.cjs b/tests/issue-3204-state-writer-phase-count.test.cjs deleted file mode 100644 index abcdb542a..000000000 --- a/tests/issue-3204-state-writer-phase-count.test.cjs +++ /dev/null @@ -1,784 +0,0 @@ -// allow-test-rule: source-text-is-the-product, see #3204 -// Reads STATE.md/ROADMAP.md fixture files whose deployed text IS what the -// runtime loads — testing text content tests the deployed contract. - -/** - * #3204 / #3185 — failing-first regression suite for `buildStateFrontmatter`'s - * `total_phases` selection (`src/state.cts:1620`, guard at `:1795-1805`). - * - * `hasMilestoneSectioning` (`src/roadmap-parser.cts:195`) is - * /^#{2,3}\s+(?!Phase\s+\S)/mi - * — true for ANY non-Phase level-2/3 heading, so a FLAT roadmap carrying an - * ordinary structural heading (`## Progress`, `## Overview`, ...) is - * misclassified as milestone-sectioned. `safeToUseRoadmapCount` then goes - * false and the on-disk phase-directory count silently clobbers the - * ROADMAP-declared count — a regression of #2828, reported in #3204 as - * "roadmap declares 6 phases, 4 directories exist, state.record-session - * writes total_phases: 4". - * - * DO NOT fix src/state.cts or src/roadmap-parser.cts from this file. Rows 2, - * 3, and 14 below assert the CORRECT (post-fix) value and currently FAIL — - * that is the point of a failing-first suite. Every other row asserts - * behavior verified to already hold today (see phase-log for the manual CLI - * probes that established each expected value before this file was written). - * - * Rows and naming follow `.gsd/phase/fix-3185-state-writer-phase-count/50-test-matrix.md` - * verbatim (row numbers refer to that matrix, not the 8-row table in - * `40-design.md`). - * - * Driven via `state record-session` (the shape #3204's own report used), - * then read back with `state json --raw` — the product's own frontmatter - * parser — so `progress.total_phases` is asserted as a NUMBER, never a - * regex over rendered STATE.md text. `tests/helpers.cjs`'s `parseFrontmatter` - * only reads flat top-level keys (it does not descend into the nested - * `progress:` block), so `state json --raw` is the correct structured seam - * for a nested field — it is what `tests/state.test.cjs`'s own '#1761 - * read-path' and 'milestone-scoped phase counting' suites already use for - * this exact assertion shape. - */ - -const { test, describe, beforeEach, afterEach } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); - -// ───────────────────────────────────────────────────────────────────────────── -// Fixture builders -// ───────────────────────────────────────────────────────────────────────────── - -/** - * Seed `.planning/phases/-phase-` for each phase number in `nums`, - * each with a single PLAN.md so the directory is a recognizable phase dir. - */ -function seedPhaseDirs(tmpDir, nums) { - for (const n of nums) { - const padded = String(n).padStart(2, '0'); - const dir = path.join(tmpDir, '.planning', 'phases', `${padded}-phase-${n}`); - fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, `${padded}-01-PLAN.md`), '# Plan\n'); - } -} - -/** Seed one arbitrarily-named phase directory (sentinel / dup / pre-milestone cases). */ -function seedNamedPhaseDir(tmpDir, dirName, planBase) { - const dir = path.join(tmpDir, '.planning', 'phases', dirName); - fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, `${planBase}-01-PLAN.md`), '# Plan\n'); -} - -/** - * Build STATE.md frontmatter + minimal body. `milestone` is always set (the - * #3204 fixture needs it truthy — `getMilestoneInfo` defaults an absent - * `milestone:` field to 'v1.0' anyway, so this pins the same value - * explicitly for every row rather than relying on that fallback). - * Lines are joined with the caller-supplied `eol` (default '\n') — row 14 - * reuses this to build the CRLF variant without a second copy. - */ -function buildStateMd({ milestone = 'v1.0', milestoneName = 'Test', totalPhases, currentPhase = '01', eol = '\n' }) { - const lines = [ - '---', - 'gsd_state_version: 1.0', - `milestone: ${milestone}`, - `milestone_name: ${milestoneName}`, - `current_phase: "${currentPhase}"`, - 'status: executing', - 'progress:', - ` total_phases: ${totalPhases}`, - ' completed_phases: 0', - ' total_plans: 0', - ' completed_plans: 0', - ' percent: 0', - '---', - '', - '# GSD State', - '', - '## Current Position', - '', - `**Current Phase:** ${currentPhase}`, - '**Status:** Executing', - '', - ]; - return lines.join(eol); -} - -/** Invoke `state record-session` (the #3204 entry point) then read back `state json --raw`. */ -function recordSessionAndReadTotalPhases(tmpDir) { - const recordResult = runGsdTools( - ['state', 'record-session', '--stopped-at', 'Phase 1, Plan 1', '--resume-file', 'none'], - tmpDir, - ); - assert.ok(recordResult.success, `state record-session failed: ${recordResult.error}`); - - const jsonResult = runGsdTools(['state', 'json', '--raw'], tmpDir); - assert.ok(jsonResult.success, `state json --raw failed: ${jsonResult.error}`); - return JSON.parse(jsonResult.output); -} - -// ───────────────────────────────────────────────────────────────────────────── -// Rows 1-3, 14 — #3204 regression: flat roadmap + a structural heading -// ───────────────────────────────────────────────────────────────────────────── - -describe('#3204 buildStateFrontmatter total_phases — flat roadmap misclassified as milestone-sectioned', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject(); - }); - - afterEach(() => { - cleanup(tmpDir); - }); - - test('flat roadmap with no structural headings keeps the roadmap count', () => { - // Row 1 (happy path / control) — no non-Phase heading anywhere, so - // hasMilestoneSectioning is false today and this already passes. Guards - // against a fix that overcorrects and breaks the trivial flat case. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual(Number(out.progress.total_phases), 6, `expected roadmap count 6, got ${out.progress && out.progress.total_phases}`); - }); - - test('#3204 flat roadmap with a Progress heading is not treated as milestone-sectioned', () => { - // Row 2 — the crux repro, transcribed from #3204's own report: 6 - // declared phases, 4 directories, a flat '## Progress' heading. FAILS - // TODAY: hasMilestoneSectioning misclassifies '## Progress' as - // sectioning, safeToUseRoadmapCount goes false, and the write clobbers - // total_phases down to the disk count (4) instead of 6. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - '## Progress', - '', - 'Some progress notes.', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 6, - `#3204: total_phases must stay 6 (roadmap-declared), not clobber to the disk count of 4. Got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('#3204 multiple structural headings still count as flat', () => { - // Row 3 — same shape as row 2 with TWO structural headings ('## Overview', - // '## Phase Details'); '## Phase Details' is correctly excluded by the - // heading's own '(?!Phase\s+\S)' lookahead, but '## Overview' still trips - // the misclassification. FAILS TODAY for the same reason as row 2. - const roadmap = [ - '# Roadmap', - '', - '## Overview', - '', - 'Some overview text.', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - '## Phase Details', - '', - 'More detail prose.', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 6, - `#3204: multiple structural headings must still count as flat (6), got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('#3204 repro under CRLF', () => { - // Row 14 — row 2's exact repro, all fixture content authored with CRLF - // line endings, proving the bug (and required fix) is not an artifact of - // LF-only fixtures. FAILS TODAY for the same reason as row 2. - const roadmapLines = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - '## Progress', - '', - 'Some progress notes.', - '', - ]; - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmapLines.join('\r\n')); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ totalPhases: 6, eol: '\r\n' }), - ); - seedPhaseDirs(tmpDir, [1, 2, 3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 6, - `#3204 under CRLF: total_phases must stay 6, got ${out.progress && out.progress.total_phases}`, - ); - }); -}); - -// ───────────────────────────────────────────────────────────────────────────── -// Rows 4-11 — negative space and boundaries (must hold both before and after the fix) -// ───────────────────────────────────────────────────────────────────────────── - -describe('#3204 buildStateFrontmatter total_phases — negative space / boundaries (must not regress)', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject(); - }); - - afterEach(() => { - cleanup(tmpDir); - }); - - test('bounded milestone uses its own section count', () => { - // Row 4 — versioned roadmap with two sibling milestone sections ('## v1.0' - // owning phases 1-2, '## v2.0' owning phases 3-5). The asserted milestone - // ('v2.0') IS bound to its own heading, so `sliceMilestoneWindow`/ - // `extractCurrentMilestoneScoped` narrow to that section and - // roadmapPhaseCount is the SECTION's count (3), not the whole-document - // count (5) and not the disk count (2 dirs seeded). - const roadmap = [ - '# Roadmap', - '', - '## v1.0', - '## Phase 1: One', - '## Phase 2: Two', - '', - '## v2.0', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ milestone: 'v2.0', milestoneName: 'Second', totalPhases: 3 }), - ); - seedPhaseDirs(tmpDir, [3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 3, - `a bounded milestone must use its own section's phase count (3), not the whole document (5) or the disk count (2). Got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('a single milestone section cannot conflate siblings', () => { - // Row 6 — exactly ONE '## v2.0' section owning phases, with the asserted - // milestone ('v9.9') absent from the roadmap entirely. One milestone - // heading can never satisfy hasMilestoneSectioning's >=2 threshold, so - // this is NOT sectioned and the roadmap-declared count is still used. - const roadmap = [ - '# Roadmap', - '', - '## v2.0', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ milestone: 'v9.9', milestoneName: 'Absent', totalPhases: 4 }), - ); - seedPhaseDirs(tmpDir, [1, 2]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 4, - `a single milestone section cannot conflate siblings; expected the roadmap count (4), got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('#1761 sibling milestone sections still fall back to the disk count', () => { - // Row 5 — TWO sibling (unversioned) milestone sections, asserted - // milestone ('v3.0') absent from either. This is genuinely - // milestone-sectioned (2 phase-bearing sections would conflate if - // whole-doc counted), so total_phases must stay the disk count. Passes - // today; a fix that touches hasMilestoneSectioning must not break it. - const roadmap = [ - '# Roadmap', - '', - '## Milestone 1: First Milestone', - '### Phase 1: a', - '### Phase 2: b', - '### Phase 3: c', - '### Phase 4: d', - '', - '## Milestone 2: Second Milestone', - '### Phase 5: e', - '### Phase 6: f', - '### Phase 7: g', - '### Phase 8: h', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ milestone: 'v3.0', milestoneName: 'Third', totalPhases: 8 }), - ); - seedPhaseDirs(tmpDir, [1, 2, 3]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 3, - `#1761: unbounded sibling milestones must fall back to the disk count (3), got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('zero phase directories keeps the declared count', () => { - // Row 7 — boundary limit-1: 0 dirs vs 6 declared. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); - // No phase dirs seeded. - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual(Number(out.progress.total_phases), 6, `expected 6, got ${out.progress && out.progress.total_phases}`); - }); - - test('equal counts agree', () => { - // Row 8 — boundary limit: 6 dirs vs 6 declared. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4, 5, 6]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual(Number(out.progress.total_phases), 6, `expected 6, got ${out.progress && out.progress.total_phases}`); - }); - - test('extra directories win via max()', () => { - // Row 9 — boundary limit+1. 6 heading-declared phases + a 7th phase - // declared only via the bullet-entry syntax ('- [ ] **Phase 7 — Extra**', - // #2199 bullet house style), which the directory-membership filter - // counts but the heading-only roadmapPhaseCount scan does not — so disk - // (7, all pass the membership filter) legitimately exceeds the - // heading-only roadmap count (6), and max() must pick 7. - // - // NOTE: a naive "N heading-declared phases + N+1 plain directories" does - // NOT exercise this path — the directory-membership filter - // (getMilestonePhaseFilter, roadmap-parser.cts) excludes any directory - // whose phase number has no matching roadmap entry at all, so an - // out-of-roadmap directory number is silently dropped from the disk - // count rather than inflating it. Verified against the running CLI - // before authoring this fixture. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - '- [ ] **Phase 7 — Extra**', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4, 5, 6, 7]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 7, - `expected max(7 dirs, 6 heading-declared) = 7, got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('absent roadmap falls back to disk', () => { - // Row 10 — no ROADMAP.md at all. - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual(Number(out.progress.total_phases), 4, `expected disk count 4, got ${out.progress && out.progress.total_phases}`); - }); - - test('roadmap with no phase headings falls back to disk', () => { - // Row 11 — ROADMAP.md present but zero Phase headings anywhere. - const roadmap = ['# Roadmap', '', '## Notes', '', 'No phases declared yet.', ''].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); - seedPhaseDirs(tmpDir, [1, 2, 3, 4]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual(Number(out.progress.total_phases), 4, `expected disk count 4, got ${out.progress && out.progress.total_phases}`); - }); - - test('a phase heading carrying a version token is not a milestone heading', () => { - // Row 12 (hostile, negative space) — '### Phase 3: Ship v2.0 gaps' carries - // a version token in its own text, but hasMilestoneSectioning's - // isPhaseHeading check excludes any heading matching '^Phase\s+\S' before - // the vocabulary signal is ever tested, so this must NOT count as a - // milestone heading. Otherwise-flat roadmap, so the roadmap-declared - // count must be used, not the disk count. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '### Phase 3: Ship v2.0 gaps', - '## Phase 4: Four', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); - seedPhaseDirs(tmpDir, [1, 2]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 4, - `a version token borne by a Phase heading must not trigger milestone sectioning; expected roadmap count 4, got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('a version heading inside a fence is not sectioning', () => { - // Row 13 (hostile, negative space) — '## v2.0' appears only inside a - // fenced code block (a documentation example of the heading syntax) on an - // otherwise flat roadmap. hasMilestoneSectioning is routed through - // tokenizeHeadings (fence-aware), so a fenced heading is never tokenised - // and must NOT count as sectioning. The roadmap-declared count must be - // used, not the disk count. - const roadmap = [ - '# Roadmap', - '', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Phase 4: Four', - '', - 'Example heading syntax:', - '', - '```', - '## v2.0', - '```', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); - seedPhaseDirs(tmpDir, [1, 2]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 4, - `a version heading inside a fence must not trigger milestone sectioning; expected roadmap count 4, got ${out.progress && out.progress.total_phases}`, - ); - }); -}); - -// ───────────────────────────────────────────────────────────────────────────── -// #3185 adversarial review — hasMilestoneSectioning ownership-model shapes -// missed by the original suite (BLOCKER + MAJOR findings against the #3184 -// "strictly-deeper nesting" rewrite). -// ───────────────────────────────────────────────────────────────────────────── - -describe('#3185 review — hasMilestoneSectioning shapes the original suite missed', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject(); - }); - - afterEach(() => { - cleanup(tmpDir); - }); - - test('BLOCKER: same-level sibling milestones fall back to the disk count', () => { - // Adversarial review BLOCKER (#1761 regression): the #3184 rewrite - // required a candidate milestone heading's owned Phase heading to be - // STRICTLY DEEPER (next.level > candidate.level). Real sibling - // milestones are frequently at the SAME level as their own Phase - // headings ('## v1.0' / '## Phase 1:' / '## v2.0' / '## Phase 3:'), so - // that predicate answered false and the whole-document count conflated - // both milestones. The asserted milestone ('v3.0') is unbound (matches - // neither v1.0 nor v2.0), so this is genuinely sectioned and must fall - // back to the disk count. - const roadmap = [ - '# Roadmap', - '', - '## v1.0', - '## Phase 1: One', - '## Phase 2: Two', - '', - '## v2.0', - '## Phase 3: Three', - '## Phase 4: Four', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ milestone: 'v3.0', milestoneName: 'Third', totalPhases: 4 }), - ); - seedPhaseDirs(tmpDir, [1, 2]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 2, - `same-level sibling milestones must fall back to the disk count (2), got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('#3185 repro: structural headings interleaved among flat phases keep the roadmap count', () => { - // #3185's own reproduction of the adjacency model this suite's - // predecessor shipped: '## Overview' sits immediately before - // '## Phase 1:' and '## Notes' sits immediately before '## Phase 4:', - // giving an adjacency-based predicate 2 "owning" candidates even though - // neither heading carries any milestone vocabulary (no version token, no - // status marker, no "Milestone" word) and the roadmap is genuinely flat. - const roadmap = [ - '# Roadmap', - '', - '## Overview', - '## Phase 1: One', - '## Phase 2: Two', - '## Phase 3: Three', - '## Notes', - '## Phase 4: Four', - '## Phase 5: Five', - '## Phase 6: Six', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ milestone: 'v9.9', milestoneName: 'Test', totalPhases: 6 }), - ); - seedPhaseDirs(tmpDir, [1, 2]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 6, - `#3185: structural headings adjacent to phase headings must not be treated as milestone sectioning; expected roadmap count 6, got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('MAJOR: wrapper + single nested milestone keeps the roadmap count', () => { - // Adversarial review MAJOR (#3204 reintroduction): every ancestor in a - // nesting chain was counted as its own candidate section under the - // #3184 rewrite, so a generic wrapper heading with only ONE real - // milestone nested under it was misclassified as sectioned. Mirrors - // this repo's own bundled template shape (gsd-core/templates/roadmap.md: - // '## Phases' -> '### 🚧 v1.1 [Name] (In Progress)' -> '#### Phase N:'). - // The asserted milestone ('v9.9') is deliberately unbound so the - // assertion exercises hasMilestoneSectioning itself, not - // isMilestoneBoundedInRoadmap. - const roadmap = [ - '# Roadmap', - '', - '## Phases', - '', - '### 🚧 v1.1 [Name] (In Progress)', - '', - '#### Phase 1: One', - '#### Phase 2: Two', - '#### Phase 3: Three', - '#### Phase 4: Four', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync( - path.join(tmpDir, '.planning', 'STATE.md'), - buildStateMd({ milestone: 'v9.9', milestoneName: 'Unbound', totalPhases: 2 }), - ); - seedPhaseDirs(tmpDir, [1, 2]); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 4, - `wrapper + single nested milestone must keep the roadmap-declared count (4), not clobber to the disk count of 2. Got ${out.progress && out.progress.total_phases}`, - ); - }); -}); - -// ───────────────────────────────────────────────────────────────────────────── -// Rows 15-18 — #3185 consolidation independence checks -// -// These exercise the directory-enumeration owner (listMilestonePhaseDirs / -// getMilestonePhaseFilter), not hasMilestoneSectioning. Verified PASSING -// against the current build (manual CLI probe) before being added here — -// included per the dispatch brief's "include only if they pass today" -// condition. If a future change to the #3204 fix regresses one of these, -// that is a SEPARATE finding from the #3204 repro above, not folded into it. -// ───────────────────────────────────────────────────────────────────────────── - -describe('#3185 buildStateFrontmatter total_phases — directory-enumeration independence (currently passing)', () => { - let tmpDir; - - beforeEach(() => { - tmpDir = createTempProject(); - }); - - afterEach(() => { - cleanup(tmpDir); - }); - - test('sentinel directories are excluded by the canonical enumeration', () => { - // Row 15 — a 999.x backlog directory alongside 3 real phase directories - // must not inflate total_phases. - const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 3 })); - seedPhaseDirs(tmpDir, [1, 2, 3]); - seedNamedPhaseDir(tmpDir, '999.1-backlog-idea', '999.1'); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 3, - `sentinel 999.x directory must be excluded, expected 3, got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('pre-milestone directories are excluded', () => { - // Row 16 — a '0-*' pre-milestone directory must not be counted. - const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 3 })); - seedPhaseDirs(tmpDir, [1, 2, 3]); - seedNamedPhaseDir(tmpDir, '0-premilestone', '0'); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 3, - `pre-milestone '0-*' directory must be excluded, expected 3, got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('duplicate phase-number directories count once', () => { - // Row 17 — two directories both keyed to phase number 2 must dedup to a - // single count. - const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 3 })); - seedPhaseDirs(tmpDir, [1, 2, 3]); - seedNamedPhaseDir(tmpDir, '02-phase-2-dup', '02'); - - const out = recordSessionAndReadTotalPhases(tmpDir); - assert.strictEqual( - Number(out.progress.total_phases), - 3, - `duplicate phase-2 directories must dedup to a single count (3), got ${out.progress && out.progress.total_phases}`, - ); - }); - - test('re-running record-session does not move total_phases', () => { - // Row 18 — idempotence: a second record-session call over an unchanged - // tree, with the clock pinned so 'Last session' does not itself vary, - // must produce a byte-identical STATE.md. - const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); - const sessionState = [ - '# GSD State', - '', - '## Session', - '', - '**Last session:** 2024-01-01T00:00:00.000Z', - '**Stopped at:** None', - '**Resume file:** None', - '', - '## Current Position', - '', - '**Current Phase:** 01', - '**Status:** Executing', - '', - ].join('\n'); - fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), sessionState); - seedPhaseDirs(tmpDir, [1, 2, 3]); - - const statePath = path.join(tmpDir, '.planning', 'STATE.md'); - const pinnedEnv = { GSD_TEST_MODE: '1', GSD_NOW_MS: '1600000000000' }; - const args = ['state', 'record-session', '--stopped-at', 'Phase 1, Plan 1', '--resume-file', 'none']; - - const first = runGsdTools(args, tmpDir, pinnedEnv); - assert.ok(first.success, `first record-session failed: ${first.error}`); - const afterFirst = fs.readFileSync(statePath, 'utf8'); - - const second = runGsdTools(args, tmpDir, pinnedEnv); - assert.ok(second.success, `second record-session failed: ${second.error}`); - const afterSecond = fs.readFileSync(statePath, 'utf8'); - - assert.strictEqual( - afterSecond, - afterFirst, - 're-running record-session on an unchanged tree with a pinned clock must produce a byte-identical STATE.md', - ); - }); -}); diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 7c815614f..9c56419cf 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -4779,3 +4779,888 @@ describe('#2297: install-marker precedence rung (GSD_RUNTIME and config.runtime }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2517-runtime-aware-profiles.test.cjs +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2517-runtime-aware-profiles', () => { +/** + * Issue #2517 — runtime-aware model profile resolution. + * + * Today, profile tiers (opus/sonnet/haiku) only resolve to Claude IDs. On Codex / + * other runtimes, users must use `inherit` or write large `model_overrides` blocks. + * + * This adds a `runtime` config key + `model_profile_overrides[runtime][tier]` map. + * When `runtime` is set to a non-Claude value, profile tiers resolve to runtime- + * native model IDs. + * + * Codex: opus -> gpt-5.6-sol (xhigh), sonnet -> gpt-5.6-terra (medium), haiku -> gpt-5.6-luna (medium) + * + * `runtime: "claude"` is the implicit default and is treated as a no-op for + * resolution — it does not override `resolve_model_ids: "omit"` or any other + * Claude-native semantics (review finding #4). + * + * `inherit` keeps current behavior. Unknown runtimes fall back safely (do NOT emit + * provider-specific IDs the runtime can't accept) and trigger a one-shot stderr + * warning so typos like `runtime: "codx"` surface immediately (review finding #13). + * + * HOME isolation: every test sets `process.env.HOME` to a per-suite tmpdir so the + * developer's real `~/.gsd/defaults.json` cannot bleed into assertions + * (review finding #8 / pattern from CodeRabbit on PRs #2603, #2604). + */ + +'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 { createTempProject, cleanup, resetRuntimeWarningCaches } = require('./helpers.cjs'); + +const { + resolveModelInternal, + resolveEffortInternal, + resolveTierEntry, +} = require('../gsd-core/bin/lib/model-resolver.cjs'); +const { + RUNTIME_PROFILE_MAP, + KNOWN_RUNTIMES, +} = require('../gsd-core/bin/lib/model-catalog.cjs'); +const { renderEffortForRuntime } = require('../gsd-core/bin/lib/model-catalog.cjs'); +const { isValidConfigKey } = require('../gsd-core/bin/lib/config-schema.cjs'); + +function writeConfig(tmpDir, obj) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify(obj, null, 2) + ); +} + +// ─── Shared HOME isolation (#2517 review finding #8) ──────────────────────── +// Without this, a developer's real `~/.gsd/defaults.json` (e.g. one with +// `runtime: codex` set) silently overrides test assertions about back-compat +// behavior. Capture HOME, point it at an isolated tmpdir for the duration of +// each test, restore on teardown. +let _origHome; +let _origUserProfile; +let _origGsdHome; +let _isolatedHome; +function isolateHome() { + _origHome = process.env.HOME; + _origUserProfile = process.env.USERPROFILE; + _origGsdHome = process.env.GSD_HOME; + _isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-home-iso-')); + process.env.HOME = _isolatedHome; + process.env.USERPROFILE = _isolatedHome; + process.env.GSD_HOME = _isolatedHome; +} +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; + cleanup(_isolatedHome); + _isolatedHome = null; +} + +// ─── Backwards compatibility — no `runtime` set ───────────────────────────── +describe('issue #2517: backwards compat — no runtime key set', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('balanced profile returns Claude alias when runtime absent', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + // gsd-planner balanced -> opus + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); + }); + + test('inherit profile still returns "inherit" with no runtime', () => { + writeConfig(tmpDir, { model_profile: 'inherit' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'inherit'); + }); + + test('resolve_model_ids:true still maps alias -> full Claude ID with no runtime', () => { + writeConfig(tmpDir, { model_profile: 'balanced', resolve_model_ids: true }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-opus-4-8'); + }); + + test('resolve_model_ids:"omit" still returns "" with no runtime', () => { + writeConfig(tmpDir, { model_profile: 'balanced', resolve_model_ids: 'omit' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), ''); + }); + + test('effort resolves universally but render param is null when runtime absent', () => { + writeConfig(tmpDir, { model_profile: 'balanced' }); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + // Effort always resolves (universal); rendering without a runtime yields no wire param. + const rendered = renderEffortForRuntime(undefined, eff); + assert.strictEqual(rendered.param, null); + }); + + test('adaptive profile still works without runtime (#1713/#1806)', () => { + writeConfig(tmpDir, { model_profile: 'adaptive' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'haiku'); + }); +}); + +// ─── runtime: "claude" — no-op (preserves Claude-native semantics) ────────── +describe('issue #2517: runtime "claude" is a no-op for resolution (finding #4)', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('runtime:"claude" + balanced returns the alias, not the resolved Claude ID', () => { + // `runtime: "claude"` is the implicit default — it must not silently flip + // resolve_model_ids on. The alias passes through identically to the unset case. + writeConfig(tmpDir, { runtime: 'claude', model_profile: 'balanced' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); + }); + + test('runtime:"claude" + resolve_model_ids:"omit" returns "" (finding #4 regression)', () => { + // The pre-fix bug: runtime:"claude" hijacked the resolution chain and + // returned the resolved Claude ID even when the user explicitly asked for the + // omit semantics. + writeConfig(tmpDir, { + runtime: 'claude', + model_profile: 'quality', + resolve_model_ids: 'omit', + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), ''); + }); + + test('runtime:"claude" + resolve_model_ids:true maps alias -> full Claude ID', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_profile: 'quality', + resolve_model_ids: true, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-opus-4-8'); + }); + + test('effort is first-class on Claude (emits output_config.effort)', () => { + writeConfig(tmpDir, { runtime: 'claude', model_profile: 'quality' }); + // Under unification, Claude effort is first-class — rendered via output_config.effort. + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('claude', eff); + assert.strictEqual(rendered.param, 'output_config.effort'); + // gsd-planner is heavy tier → default effort 'xhigh' + assert.strictEqual(rendered.value, 'xhigh'); + }); +}); + +// ─── runtime: "codex" — resolves tiers to Codex IDs + reasoning_effort ────── +describe('issue #2517: runtime "codex" — Codex tier resolution', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('opus tier -> gpt-5.6-sol model; heavy-tier agent -> xhigh effort on codex', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); + // gsd-planner quality -> opus -> gpt-5.6-sol (model unchanged) + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); + // gsd-planner is heavy routing tier → effort 'xhigh' → rendered model_reasoning_effort + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'xhigh'); + }); + + test('sonnet tier -> gpt-5.6-terra model; heavy-tier agent -> xhigh effort on codex', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'balanced' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'gpt-5.6-terra'); + // gsd-roadmapper is heavy routing tier → effort 'xhigh' (not catalog medium) + const eff = resolveEffortInternal(tmpDir, 'gsd-roadmapper'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'xhigh'); + }); + + test('haiku tier -> gpt-5.6-luna model; light-tier agent -> low effort on codex', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'budget' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'gpt-5.6-luna'); + // gsd-codebase-mapper is light routing tier → effort 'low' (not catalog medium) + const eff = resolveEffortInternal(tmpDir, 'gsd-codebase-mapper'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'low'); + }); + + test('adaptive profile resolves on Codex (no #1713/#1806 regression)', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'adaptive' }); + // gsd-planner adaptive -> opus -> gpt-5.6-sol + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); + // gsd-codebase-mapper adaptive -> haiku -> gpt-5.6-luna + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'gpt-5.6-luna'); + }); + + test('inherit profile still returns "inherit" on Codex; effort still resolves universally', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'inherit' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'inherit'); + // Unified effort is config-driven (routing_tier_defaults), independent of model_profile. + // gsd-planner (heavy tier) → 'xhigh'; rendered to codex param. + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'xhigh'); + }); + + test('runtime:"codex" beats resolve_model_ids:"omit" (explicit non-Claude opt-in wins)', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + resolve_model_ids: 'omit', + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); + }); +}); + +// ─── Precedence chain ─────────────────────────────────────────────────────── +describe('issue #2517: precedence chain', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('per-agent model_overrides wins over runtime tier resolution', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_overrides: { 'gsd-planner': 'gpt-5.6-luna' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-luna'); + }); + + test('model_profile_overrides[runtime][tier] beats built-in defaults', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_profile_overrides: { + codex: { opus: 'gpt-5-pro' }, + }, + }); + // gsd-planner quality -> opus -> overridden to gpt-5-pro + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5-pro'); + // gsd-codebase-mapper quality -> sonnet -> gpt-5.6-terra + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'gpt-5.6-terra'); + }); + + test('partial profile_overrides — only opus overridden, sonnet uses default', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'balanced', + model_profile_overrides: { + codex: { opus: 'gpt-5-pro' }, // only opus overridden + }, + }); + // gsd-planner balanced -> opus -> overridden to gpt-5-pro + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5-pro'); + // gsd-roadmapper balanced -> sonnet -> spec default + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'gpt-5.6-terra'); + }); + + test('per-agent override beats profile override beats default', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_profile_overrides: { codex: { opus: 'gpt-5-pro' } }, + model_overrides: { 'gsd-planner': 'custom-model' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'custom-model'); + }); +}); + +// ─── Field-merge semantics — review findings #2 ───────────────────────────── +describe('issue #2517: field-merge of overrides with built-in defaults (finding #2)', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('string-shorthand override: model is overridden; unified effort derives from routing tier', () => { + // `{ codex: { opus: "gpt-5-pro" } }` is the documented shorthand. + // Model is overridden to gpt-5-pro; effort now derives from the universal + // config-driven path (gsd-planner heavy tier → 'xhigh'), not from the catalog. + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_profile_overrides: { codex: { opus: 'gpt-5-pro' } }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5-pro'); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'xhigh'); + }); + + test('partial-object override (no model) keeps model from built-in; unified effort from routing tier', () => { + // `{ codex: { opus: { reasoning_effort: "low" } } }` preserves the built-in model. + // Under unification, the catalog reasoning_effort field is not read for effort resolution; + // effort comes from routing_tier_defaults (gsd-planner heavy → 'xhigh'). + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_profile_overrides: { codex: { opus: { reasoning_effort: 'low' } } }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'xhigh'); + }); + + test('full-object override: model replaced; unified effort from routing tier (not catalog field)', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_profile_overrides: { + codex: { opus: { model: 'custom-model', reasoning_effort: 'minimal' } }, + }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'custom-model'); + // Effort comes from routing_tier_defaults, not the catalog 'minimal' field. + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('codex', eff); + assert.strictEqual(rendered.param, 'model_reasoning_effort'); + assert.strictEqual(rendered.value, 'xhigh'); + }); + + test('resolveTierEntry helper: shorthand merge', () => { + // Direct unit-test of the shared helper used by core + install.js. + const entry = resolveTierEntry({ + runtime: 'codex', + tier: 'opus', + overrides: { codex: { opus: 'gpt-5-pro' } }, + }); + assert.deepStrictEqual(entry, { model: 'gpt-5-pro', reasoning_effort: 'xhigh' }); + }); + + test('resolveTierEntry helper: partial-object merge keeps built-in model', () => { + const entry = resolveTierEntry({ + runtime: 'codex', + tier: 'opus', + overrides: { codex: { opus: { reasoning_effort: 'low' } } }, + }); + assert.deepStrictEqual(entry, { model: 'gpt-5.6-sol', reasoning_effort: 'low' }); + }); +}); + +// ─── Unknown runtime render safety (finding #3 spirit) ────────────────────── +describe('issue #2517: unknown runtime render param is null (effort does not leak to install path)', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('unknown runtime: model resolves via override; render param is null (no wire param leaked)', () => { + // Under unification, effort always resolves (universal), but renderEffortForRuntime + // returns param=null for unknown runtimes — no effort leaks to the install path. + writeConfig(tmpDir, { + runtime: 'mystery', + model_profile: 'quality', + model_profile_overrides: { + mystery: { opus: { model: 'mystery-opus', reasoning_effort: 'xhigh' } }, + }, + }); + // Model still resolves (overrides are honored). + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'mystery-opus'); + // Effort resolves universally but the unknown runtime has no wire param. + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('mystery', eff); + assert.strictEqual(rendered.param, null); + }); + + test('typo runtime "codx": render param is null (no leak into install path)', () => { + writeConfig(tmpDir, { + runtime: 'codx', + model_profile: 'quality', + model_profile_overrides: { codx: { opus: { model: 'gpt-5.6-terra', reasoning_effort: 'xhigh' } } }, + }); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + const rendered = renderEffortForRuntime('codx', eff); + assert.strictEqual(rendered.param, null); + }); +}); + +// ─── Unknown runtime / unknown tier ───────────────────────────────────────── +describe('issue #2517: unknown runtime + safe fallback', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('unknown runtime falls back to Claude-alias safe default (no Codex IDs leaked)', () => { + writeConfig(tmpDir, { runtime: 'mystery-runtime', model_profile: 'quality' }); + // Should NOT emit gpt-5.6-sol — should fall back to Claude alias + const resolved = resolveModelInternal(tmpDir, 'gsd-planner'); + assert.notStrictEqual(resolved, 'gpt-5.6-sol'); + assert.strictEqual(resolved, 'opus'); + }); + + test('unknown runtime + user-provided overrides for that runtime — uses overrides', () => { + writeConfig(tmpDir, { + runtime: 'mystery-runtime', + model_profile: 'quality', + model_profile_overrides: { + 'mystery-runtime': { opus: 'mystery-opus' }, + }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'mystery-opus'); + }); + + test('runtime:"codex" but missing model_profile_overrides[codex] uses spec defaults', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); + // No model_profile_overrides at all — built-in Codex defaults take over + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'gpt-5.6-sol'); + }); +}); + +// ─── Schema validation (config-set time + load time) ──────────────────────── +describe('issue #2517: VALID_CONFIG_KEYS schema', () => { + test('"runtime" is a valid config key', () => { + assert.strictEqual(isValidConfigKey('runtime'), true); + }); + + test('model_profile_overrides.codex.opus is valid', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.opus'), true); + }); + + test('model_profile_overrides.codex.sonnet is valid', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.sonnet'), true); + }); + + test('model_profile_overrides.codex.haiku is valid', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.haiku'), true); + }); + + test('model_profile_overrides.claude.opus is valid', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.claude.opus'), true); + }); + + test('model_profile_overrides with unknown runtime is valid (free-string runtime)', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.acme.opus'), true); + }); + + test('model_profile_overrides with bogus tier is rejected', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.codex.banana'), false); + }); + + test('model_profile_overrides without tier is rejected', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides.codex'), false); + }); + + test('model_profile_overrides root key alone is rejected (must include runtime+tier)', () => { + assert.strictEqual(isValidConfigKey('model_profile_overrides'), false); + }); +}); + +// ─── loadConfig validation warnings (review findings #10, #13) ────────────── +describe('issue #2517: loadConfig warns on unknown runtime/tier (findings #10, #13)', () => { + const { loadConfig } = require('../gsd-core/bin/lib/config-loader.cjs'); + let tmpDir; + let origWrite; + let captured; + beforeEach(() => { + isolateHome(); + tmpDir = createTempProject(); + resetRuntimeWarningCaches(); + captured = []; + origWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk) => { captured.push(String(chunk)); return true; }; + }); + afterEach(() => { process.stderr.write = origWrite; cleanup(tmpDir); restoreHome(); }); + + test('unknown runtime triggers a stderr warning', () => { + writeConfig(tmpDir, { runtime: 'codx', model_profile: 'quality' }); + loadConfig(tmpDir); + const joined = captured.join(''); + assert.match(joined, /unknown value "codx"/); + }); + + test('known runtime does NOT trigger a runtime warning', () => { + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); + loadConfig(tmpDir); + const joined = captured.join(''); + assert.doesNotMatch(joined, /unknown value/); + }); + + test('unknown tier in overrides triggers a stderr warning', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile_overrides: { codex: { banana: 'whatever' } }, + }); + loadConfig(tmpDir); + const joined = captured.join(''); + assert.match(joined, /unknown tier "banana"/); + }); + + test('unknown runtime in overrides triggers a stderr warning', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_profile_overrides: { mystery: { opus: 'whatever' } }, + }); + loadConfig(tmpDir); + const joined = captured.join(''); + assert.match(joined, /model_profile_overrides\.mystery\.\* uses unknown runtime/); + }); + + test('every name in KNOWN_RUNTIMES survives the warning gate', () => { + // Smoke check: `KNOWN_RUNTIMES` must list every runtime `bin/install.js` + // emits for, otherwise legitimate users get spammed at every loadConfig. + for (const r of KNOWN_RUNTIMES) { + assert.ok(typeof r === 'string' && r.length > 0); + } + }); +}); + +// ─── End-to-end: per-project config -> Codex TOML emit (finding #1) ───────── +describe('issue #2517: install end-to-end — per-project config reaches Codex TOML (finding #1)', () => { + // Load install.js in test-mode so its module exports are populated. + const prevTestMode = process.env.GSD_TEST_MODE; + process.env.GSD_TEST_MODE = '1'; + const installMod = require('../bin/install.js'); + if (prevTestMode === undefined) delete process.env.GSD_TEST_MODE; + else process.env.GSD_TEST_MODE = prevTestMode; + const { readGsdRuntimeProfileResolver, generateCodexAgentToml } = installMod; + + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('readGsdRuntimeProfileResolver picks up runtime from .planning/config.json', () => { + // No ~/.gsd/defaults.json (HOME is isolated tmpdir). Per-project config alone + // must drive the resolver — pre-fix, it returned null. + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); + const resolver = readGsdRuntimeProfileResolver(tmpDir); + assert.ok(resolver, 'expected a resolver from per-project config'); + assert.strictEqual(resolver.runtime, 'codex'); + const entry = resolver.resolve('gsd-planner'); + assert.deepStrictEqual(entry, { model: 'gpt-5.6-sol', reasoning_effort: 'xhigh' }); + }); + + test('per-project config wins over global ~/.gsd/defaults.json', () => { + fs.mkdirSync(path.join(_isolatedHome, '.gsd'), { recursive: true }); + fs.writeFileSync( + path.join(_isolatedHome, '.gsd', 'defaults.json'), + JSON.stringify({ runtime: 'claude', model_profile: 'budget' }) + ); + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); + const resolver = readGsdRuntimeProfileResolver(tmpDir); + assert.strictEqual(resolver.runtime, 'codex'); + const entry = resolver.resolve('gsd-planner'); + assert.strictEqual(entry.model, 'gpt-5.6-sol'); + }); + + test('generated Codex TOML omits model = and model_reasoning_effort = lines when only the resolver would have supplied them (#3241)', () => { + // #3241 flips this: the runtime-resolver auto-embed (D1) was removed, so a + // resolver alone with no explicit model_overrides no longer pins a model, + // and #838's coupling means the reasoning-effort line is omitted too. + writeConfig(tmpDir, { runtime: 'codex', model_profile: 'quality' }); + const resolver = readGsdRuntimeProfileResolver(tmpDir); + const toml = generateCodexAgentToml( + 'gsd-planner', + '---\nname: gsd-planner\ndescription: Planner agent\n---\nBody.\n', + null, + resolver + ); + assert.doesNotMatch(toml, /^model = "gpt-5\.6-sol"$/m); + assert.doesNotMatch(toml, /^model_reasoning_effort = "xhigh"$/m); + }); + + test('generated TOML always includes model_reasoning_effort even when model_profile_overrides sets reasoning_effort to empty (#443 unified) (#3241: model now pinned via explicit model_overrides, not the resolver alone)', () => { + // Under the unified effort design (#443), model_reasoning_effort in the Codex TOML + // is driven by the unified effort resolver (resolveInstallTimeEffort / effortCfg), + // NOT by model_profile_overrides.reasoning_effort. Setting reasoning_effort: '' in + // model_profile_overrides does NOT suppress the unified effort when a model IS + // pinned — the TOML carries a valid model_reasoning_effort drawn from the agent's + // routing tier. + // #3241: the resolver alone no longer pins a model (D1), so this test now supplies + // an explicit model_overrides pin ('custom', a real-looking Codex id — row 4, + // "unchanged") to keep exercising the unrelated property under test: that + // model_profile_overrides.reasoning_effort is ignored by the unified resolver. + // gsd-planner is a heavy-tier agent → unified default resolves to "xhigh". + writeConfig(tmpDir, { + runtime: 'codex', + model_profile: 'quality', + model_profile_overrides: { codex: { opus: { model: 'custom', reasoning_effort: '' } } }, + }); + const resolver = readGsdRuntimeProfileResolver(tmpDir); + const toml = generateCodexAgentToml( + 'gsd-planner', + '---\nname: gsd-planner\n---\nBody.\n', + { 'gsd-planner': 'custom' }, + resolver + ); + // Explicit model_overrides pin is respected (#3241 row 4 — unchanged). + assert.match(toml, /^model = "custom"$/m); + // Unified effort always fires when a model is pinned — model_reasoning_effort is + // present and valid, ignoring model_profile_overrides.reasoning_effort. + assert.match(toml, /^model_reasoning_effort = "(minimal|low|medium|high|xhigh)"$/m); + // gsd-planner is heavy-tier, so with no effortCfg the manifest tier default applies → xhigh. + assert.match(toml, /^model_reasoning_effort = "xhigh"$/m); + }); + + test('resolver returns null with no global, no per-project config', () => { + // Sanity: nothing configured -> nothing emitted. Pre-existing back-compat. + const resolver = readGsdRuntimeProfileResolver(tmpDir); + assert.strictEqual(resolver, null); + }); + + test('inline require paths resolve relative to install.js __dirname (finding #6)', () => { + // Defensive: assert the lib files install.js requires actually exist at + // resolver-construction time. Catches accidental relative-path drift in CI. + const installDir = path.dirname(require.resolve('../bin/install.js')); + const libDir = path.join(installDir, '..', 'gsd-core', 'bin', 'lib'); + assert.ok(fs.existsSync(path.join(libDir, 'model-catalog.cjs'))); + assert.ok(fs.existsSync(path.join(libDir, 'model-profiles.cjs'))); + }); +}); + +// ─── RUNTIME_PROFILE_MAP single source of truth (finding #16) ─────────────── +describe('issue #2517: RUNTIME_PROFILE_MAP single source of truth (finding #16)', () => { + test('install.js consumes the same map as model-catalog.cjs', () => { + // `bin/install.js` must NOT carry its own duplicate copy of the map. + // The shared resolver imported in install.js exposes `runtime` and the + // entries through `resolveTierEntry`, so any future drift between the two + // files would surface as a test failure here rather than a silent bug. + const codexOpus = RUNTIME_PROFILE_MAP.codex?.opus; + assert.deepStrictEqual(codexOpus, { model: 'gpt-5.6-sol', reasoning_effort: 'xhigh' }); + const claudeOpus = RUNTIME_PROFILE_MAP.claude?.opus; + assert.deepStrictEqual(claudeOpus, { model: 'claude-opus-4-8' }); + }); +}); + +// #1928: the "gemini" runtime tier-resolution suite was removed with the +// sunset Gemini CLI runtime. The gemini-3.x models remain in the catalog for +// Antigravity (which runs on the Gemini backend and carries its own +// runtimeTierDefaults); Antigravity's tier resolution is covered elsewhere. + +// ─── Issue #2612: qwen runtime tier resolution ─────────────────────────────── +describe('issue #2612: runtime "qwen" — Qwen tier resolution', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('opus tier -> qwen3-max-2026-01-23', () => { + writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'quality' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'qwen3-max-2026-01-23'); + }); + + test('sonnet tier -> qwen3-coder-plus', () => { + writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'balanced' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'qwen3-coder-plus'); + }); + + test('haiku tier -> qwen3-coder-next', () => { + writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'budget' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'qwen3-coder-next'); + }); + + test('qwen: effort resolves universally but render param is null (no wire param)', () => { + writeConfig(tmpDir, { runtime: 'qwen', model_profile: 'quality' }); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + assert.strictEqual(renderEffortForRuntime('qwen', eff).param, null); + }); +}); + +// ─── Issue #2612: opencode runtime tier resolution ─────────────────────────── +describe('issue #2612: runtime "opencode" — OpenCode tier resolution', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('opus tier -> anthropic/claude-opus-4-8', () => { + writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'quality' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); + }); + + test('sonnet tier -> anthropic/claude-sonnet-5', () => { + writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'balanced' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-5'); + }); + + test('haiku tier -> anthropic/claude-haiku-4-5', () => { + writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'budget' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); + }); + + test('opencode: effort resolves universally but render param is null (no wire param)', () => { + writeConfig(tmpDir, { runtime: 'opencode', model_profile: 'quality' }); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + assert.strictEqual(renderEffortForRuntime('opencode', eff).param, null); + }); +}); + +// ─── Issue #2093: kilo runtime tier resolution ─────────────────────────────── +// Kilo is an OpenCode fork and shares the IDENTICAL built-in tier IDs (UPGRADE 2 +// / ADR-1239). Kilo moved from Group B (no built-in defaults) to Group A here. +describe('issue #2093: runtime "kilo" — Kilo tier resolution', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('opus tier -> anthropic/claude-opus-4-8', () => { + writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'quality' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); + }); + + test('sonnet tier -> anthropic/claude-sonnet-5', () => { + writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'balanced' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-5'); + }); + + test('haiku tier -> anthropic/claude-haiku-4-5', () => { + writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'budget' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); + }); + + test('kilo: effort resolves universally but render param is null (no wire param)', () => { + writeConfig(tmpDir, { runtime: 'kilo', model_profile: 'quality' }); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + assert.strictEqual(renderEffortForRuntime('kilo', eff).param, null); + }); +}); + +// ─── Issue #2612: copilot runtime tier resolution ──────────────────────────── +describe('issue #2612: runtime "copilot" — Copilot tier resolution', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('opus tier -> claude-opus-4-8', () => { + writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'quality' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-opus-4-8'); + }); + + test('sonnet tier -> claude-sonnet-5', () => { + writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'balanced' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'claude-sonnet-5'); + }); + + test('haiku tier -> claude-haiku-4-5', () => { + writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'budget' }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'claude-haiku-4-5'); + }); + + test('copilot: effort resolves universally but render param is null (no wire param)', () => { + writeConfig(tmpDir, { runtime: 'copilot', model_profile: 'quality' }); + const eff = resolveEffortInternal(tmpDir, 'gsd-planner'); + assert.strictEqual(renderEffortForRuntime('copilot', eff).param, null); + }); +}); + +// ─── Issue #2612: Group B runtimes fall through (no built-in map) ──────────── +describe('issue #2612: Group B runtimes — no built-in map, use unknown-runtime fallback', () => { + test('cursor is not in RUNTIME_PROFILE_MAP (uses unknown-runtime fallback)', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.cursor, undefined); + }); + + test('windsurf is not in RUNTIME_PROFILE_MAP', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.windsurf, undefined); + }); + + test('cline is not in RUNTIME_PROFILE_MAP', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.cline, undefined); + }); + + test('augment is not in RUNTIME_PROFILE_MAP', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.augment, undefined); + }); + + test('trae is not in RUNTIME_PROFILE_MAP', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.trae, undefined); + }); + + test('codebuddy is not in RUNTIME_PROFILE_MAP', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.codebuddy, undefined); + }); + + test('antigravity is not in RUNTIME_PROFILE_MAP', () => { + assert.strictEqual(RUNTIME_PROFILE_MAP.antigravity, undefined); + }); + + test('cursor runtime falls back to Claude alias (not a Gemini/Qwen/etc ID)', () => { + const { createTempProject, cleanup } = require('./helpers.cjs'); + isolateHome(); + const tmpDir = createTempProject(); + resetRuntimeWarningCaches(); + try { + writeConfig(tmpDir, { runtime: 'cursor', model_profile: 'quality' }); + // Should fall back to Claude alias, not emit a provider-specific ID + const resolved = resolveModelInternal(tmpDir, 'gsd-planner'); + assert.strictEqual(resolved, 'opus'); + } finally { + cleanup(tmpDir); + restoreHome(); + } + }); +}); + +// ─── Issue #2612: Partial override merge for new runtimes ──────────────────── +describe('issue #2612: partial override merge for new Group A runtimes', () => { + let tmpDir; + beforeEach(() => { isolateHome(); tmpDir = createTempProject(); resetRuntimeWarningCaches(); }); + afterEach(() => { cleanup(tmpDir); restoreHome(); }); + + test('qwen.opus override wins; sonnet and haiku use built-in defaults', () => { + writeConfig(tmpDir, { + runtime: 'qwen', + model_profile: 'quality', + model_profile_overrides: { + qwen: { opus: 'qwen3-max-custom' }, + }, + }); + // opus is overridden + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'qwen3-max-custom'); + // sonnet not overridden — quality -> sonnet for gsd-codebase-mapper + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'qwen3-coder-plus'); + }); + + test('opencode.sonnet override wins; opus and haiku still use built-in defaults', () => { + writeConfig(tmpDir, { + runtime: 'opencode', + model_profile: 'balanced', + model_profile_overrides: { + opencode: { sonnet: 'anthropic/claude-sonnet-4-7' }, + }, + }); + // gsd-planner balanced -> opus -> built-in default + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); + // gsd-roadmapper balanced -> sonnet -> overridden + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-4-7'); + // gsd-codebase-mapper balanced -> haiku -> built-in default (haiku not overridden) + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); + }); + + test('copilot.haiku override wins; opus and sonnet still use built-in defaults', () => { + writeConfig(tmpDir, { + runtime: 'copilot', + model_profile: 'budget', + model_profile_overrides: { + copilot: { haiku: 'claude-haiku-4-6' }, + }, + }); + // gsd-codebase-mapper budget -> haiku -> overridden + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'claude-haiku-4-6'); + // gsd-planner budget -> sonnet -> built-in default + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'claude-sonnet-5'); + }); + + // #2093: kilo just moved into Group A — same partial-merge coverage as opencode. + test('kilo.sonnet override wins; opus and haiku still use built-in defaults', () => { + writeConfig(tmpDir, { + runtime: 'kilo', + model_profile: 'balanced', + model_profile_overrides: { + kilo: { sonnet: 'anthropic/claude-sonnet-4-7' }, + }, + }); + // gsd-planner balanced -> opus -> built-in default + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'anthropic/claude-opus-4-8'); + // gsd-roadmapper balanced -> sonnet -> overridden + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-roadmapper'), 'anthropic/claude-sonnet-4-7'); + // gsd-codebase-mapper balanced -> haiku -> built-in default (haiku not overridden) + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'anthropic/claude-haiku-4-5'); + }); +}); + }); +} diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index dbaa88600..ca4175277 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -10491,3 +10491,366 @@ describe('#2572: phase complete warns when a SUMMARY claims files that never lan } }); }); + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2945-phase-complete-checkbox-rollback.test.cjs — H3 Wave 7 test-hygiene sweep (#3339) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2945-phase-complete-checkbox-rollback (#3339)', () => { +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * Regression test for #2945 — `phase complete`'s inline REQUIREMENTS.md checkbox + * flip is traceability-blind: it flips `- [ ] **REQ-ID**` → `- [x]` unconditionally, + * then attempts the traceability-row write, and KEEPS the flip when the row exists + * but rejects the write (Out/Deferred/Blocked). The sibling `requirements.mark-complete` + * got the #2788 defect-2 rollback; `cmdPhaseComplete`'s inline copy did not. + * + * The fix ports the rollback from `cmdRequirementsMarkComplete` (src/milestone.cts): + * capture beforeCheckbox, track whether the row write actually changed (tableHit), and + * restore beforeCheckbox when the row EXISTS but rejects the write. + * + * Matrix: .gsd/bug/fix/2945-phase-complete-checkbox-rollback/50-test-matrix.md + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +/** + * Write a passed-VERIFICATION marker for the phase, then run `phase complete N`. + * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` + * with `status: passed` frontmatter. Requires the phase directory to exist. + */ +function runVerifiedPhaseComplete(args, tmpDir) { + const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); + const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); + const phase = argv[completeIdx + 1]; + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + const wanted = parseInt(String(phase).replace(/^0+/, ''), 10); + const phaseDirName = fs.readdirSync(phasesDir).find((name) => { + const m = name.match(/^(\d+)/); + return m && parseInt(m[1], 10) === wanted; + }); + if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); + fs.writeFileSync( + path.join(phasesDir, phaseDirName, `${phase}-VERIFICATION.md`), + ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), + ); + return runGsdTools(args, tmpDir); +} + +describe('phase complete checkbox rollback (#2945)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-2945-'); + }); + afterEach(() => { + cleanup(tmpDir); + }); + + /** + * Scaffold a single-phase project whose ROADMAP cites the given REQ-IDs, with a + * REQUIREMENTS.md whose traceability table rows carry the given statuses, then run + * `phase complete 1`. Returns the REQUIREMENTS.md content after completion. + */ + function completeWithRows(reqIds, rowStatuses) { + const reqList = reqIds.join(', '); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n- [ ] Phase 1: The phase\n\n### Phase 1: The phase\n**Goal:** do it\n**Requirements:** ${reqList}\n**Plans:** 1 plans\n`, + ); + const reqLines = reqIds.map((id) => `- [ ] **${id}**: a requirement`).join('\n'); + const tableRows = reqIds.map((id, i) => `| ${id} | Phase 1 | ${rowStatuses[i]} |`).join('\n'); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), + `# Requirements\n\n## v1 Requirements\n\n${reqLines}\n\n## Traceability\n\n| Requirement | Phase | Status |\n|-------------|-------|--------|\n${tableRows}\n`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 01\n**Current Phase Name:** The phase\n**Status:** In progress\n**Current Plan:** 01-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n`, + ); + const p1 = path.join(tmpDir, '.planning', 'phases', '01-the-phase'); + fs.mkdirSync(p1, { recursive: true }); + fs.writeFileSync(path.join(p1, '01-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(p1, '01-01-SUMMARY.md'), '# Summary'); + + const result = runVerifiedPhaseComplete('phase complete 1', tmpDir); + assert.ok(result.success, `phase complete failed: ${result.error || result.output}`); + return fs.readFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), 'utf-8'); + } + + test('deferredRowRollsBackCheckbox', () => { + // Row 1 (failing-first regression): a Deferred row rejects the Status write, so the + // checkbox must NOT flip (it stays [ ]), and the row must stay Deferred. + const req = completeWithRows(['DEF-01'], ['Deferred']); + assert.ok(req.includes('- [ ] **DEF-01**'), 'checkbox must stay [ ] when the row is Deferred (no silent divergence)'); + assert.ok(/^\| DEF-01 \| Phase 1 \| Deferred \|/m.test(req), 'row must stay Deferred'); + assert.ok(!req.includes('- [x] **DEF-01**'), 'checkbox must NOT have flipped to [x]'); + }); + + test('blockedRowRollsBackCheckbox', () => { + // Row 2: an Out/Blocked row likewise rejects the write → checkbox stays [ ]. + const req = completeWithRows(['BLK-01'], ['Blocked']); + assert.ok(req.includes('- [ ] **BLK-01**'), 'checkbox must stay [ ] when the row is Blocked'); + assert.ok(/^\| BLK-01 \| Phase 1 \| Blocked \|/m.test(req), 'row must stay Blocked'); + assert.ok(!req.includes('- [x] **BLK-01**'), 'checkbox must NOT have flipped to [x]'); + }); + + test('pendingRowStillFlipsAndAdvances', () => { + // Row 3 (negative-space / unchanged forward behavior): a Pending row accepts the write, + // so the checkbox MUST flip to [x] and the row MUST advance to Complete. An over-broad + // rollback would break this. + const req = completeWithRows(['FWD-01'], ['Pending']); + assert.ok(req.includes('- [x] **FWD-01**'), 'checkbox MUST flip to [x] for a Pending (forward) row'); + assert.ok(/^\| FWD-01 \| Phase 1 \| Complete \|/m.test(req), 'row MUST advance to Complete'); + }); + + test('noRowStillFlipsCheckbox', () => { + // Row 4 (acceptance #3): a cited REQ-ID with NO traceability row → checkbox still flips + // (nothing to disagree with). The rollback only fires when a row EXISTS and rejects. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n- [ ] Phase 1: The phase\n\n### Phase 1: The phase\n**Goal:** do it\n**Requirements:** NOROW-01\n**Plans:** 1 plans\n`, + ); + // REQUIREMENTS.md with the checkbox but NO traceability table. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), + `# Requirements\n\n## v1 Requirements\n\n- [ ] **NOROW-01**: a requirement with no traceability row\n`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 01\n**Current Phase Name:** The phase\n**Status:** In progress\n**Current Plan:** 01-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n`, + ); + const p1 = path.join(tmpDir, '.planning', 'phases', '01-the-phase'); + fs.mkdirSync(p1, { recursive: true }); + fs.writeFileSync(path.join(p1, '01-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(p1, '01-01-SUMMARY.md'), '# Summary'); + + const result = runVerifiedPhaseComplete('phase complete 1', tmpDir); + assert.ok(result.success, `phase complete failed: ${result.error || result.output}`); + const req = fs.readFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), 'utf-8'); + assert.ok(req.includes('- [x] **NOROW-01**'), 'checkbox MUST flip to [x] when no traceability row exists'); + }); +}); + }); +} + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2949-phase-complete-stage3-sentinel.test.cjs — H3 Wave 7 test-hygiene sweep (#3339) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2949-phase-complete-stage3-sentinel (#3339)', () => { +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * Regression test for #2949 — `phase complete`'s stage-3 lowest-outstanding-override + * loop admits unchecked `0.x` backlog sentinel rows as `next_phase`, corrupting STATE.md + * and desyncing `current_phase` from `current_phase_name`. + * + * Root cause: `src/phase.cts` stage-3 condition `!isChecked && comparePhaseNum(cbm[2], phaseNum) < 0` + * has no sentinel filter, so `comparePhaseNum("0.1","12") === -12` admits the `0.x` backlog row. + * The fix adds `&& !isSentinelPhaseId(cbm[2])` (reusing the existing zero-caller predicate), which + * excludes both sentinel ranges (0 and 999). Stage-3 only here — PR #2815 (in-flight) covers + * stages 1-2 for #2786. + * + * Matrix: .gsd/bug/fix/2949-phase-complete-stage3-sentinel-filter/50-test-matrix.md + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +/** + * Write a passed-verification marker for a phase, then run `phase complete N`. + * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` + * with `status: passed` frontmatter, plus a SUMMARY for each plan (the completion gate + * requires executed plans). Requires the phase directory to already exist. + */ +function runVerifiedPhaseComplete(args, tmpDir) { + const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); + const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); + const phase = argv[completeIdx + 1]; + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + // Find the phase directory whose leading token matches the requested phase number. + const wantedPadded = String(phase).replace(/^0+/, ''); + const phaseDirName = fs.readdirSync(phasesDir).find((name) => { + const m = name.match(/^(\d+)/); + return m && parseInt(m[1], 10) === parseInt(wantedPadded, 10); + }); + if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); + const phaseDir = path.join(phasesDir, phaseDirName); + fs.writeFileSync( + path.join(phaseDir, `${phase}-VERIFICATION.md`), + ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), + ); + return runGsdTools(args, tmpDir); +} + +describe('phase complete stage-3 sentinel filter (#2949)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-2949-'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + /** Scaffold a phase dir with one executed plan (PLAN + SUMMARY). */ + function scaffoldPhase(slug, planNum) { + const dir = path.join(tmpDir, '.planning', 'phases', slug); + fs.mkdirSync(dir, { recursive: true }); + const padded = String(planNum).padStart(2, '0'); + fs.writeFileSync(path.join(dir, `${padded}-01-PLAN.md`), '# Plan'); + fs.writeFileSync(path.join(dir, `${padded}-01-SUMMARY.md`), '# Summary'); + } + + test('zeroXSentinelDoesNotBecomeNextPhase', () => { + // Row 1 (failing-first regression): completing the last real phase with an unchecked + // 0.x backlog sentinel row present must NOT select the sentinel as next_phase. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +- [ ] **Phase 0.1: Backlog sentinel item** — deferred work +- [x] **Phase 11: First phase** (completed 2025-01-01) +- [ ] **Phase 12: Last phase** + +### Phase 11: First phase +**Goal:** first +**Plans:** 1 plans + +### Phase 12: Last phase +**Goal:** last +**Plans:** 1 plans +`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 12\n**Current Phase Name:** Last phase\n**Status:** In progress\n**Current Plan:** 12-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 12\n`, + ); + scaffoldPhase('11-first-phase', 11); + scaffoldPhase('12-last-phase', 12); + + const result = runVerifiedPhaseComplete('phase complete 12', tmpDir); + assert.ok(result.success, `Command failed: ${result.error || result.output}`); + const output = JSON.parse(result.output); + + assert.strictEqual(output.completed_phase, '12'); + assert.strictEqual(output.is_last_phase, true, '0.x sentinel must not prevent milestone completion (is_last_phase=true)'); + assert.strictEqual(output.next_phase, null, '0.x sentinel must not be selected as next_phase'); + + // STATE.md current_phase must stay on the completed phase (12), not advance to 0.1. + const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok(!/\*\*Current Phase:\*\*\s*0\.1/i.test(state), 'STATE.md current_phase must NOT have advanced to the 0.x sentinel'); + }); + + test('realLowerOutstandingPhaseStillSelected', () => { + // Row 2 (#2028 non-regression): a REAL lower-numbered unchecked phase must STILL be + // selected as next_phase. The sentinel filter must not over-broaden to real phases. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +- [ ] **Phase 9: Skipped-then-resumed phase** +- [ ] **Phase 10: Current phase** + +### Phase 9: Skipped-then-resumed phase +**Goal:** nine +**Plans:** 1 plans + +### Phase 10: Current phase +**Goal:** ten +**Plans:** 1 plans +`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 10\n**Current Phase Name:** Current phase\n**Status:** In progress\n**Current Plan:** 10-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 10\n`, + ); + scaffoldPhase('09-skipped-then-resumed-phase', 9); + scaffoldPhase('10-current-phase', 10); + + const result = runVerifiedPhaseComplete('phase complete 10', tmpDir); + assert.ok(result.success, `Command failed: ${result.error || result.output}`); + const output = JSON.parse(result.output); + + // A real lower phase (9) IS selected — the #2028 out-of-order behavior is preserved. + // next_phase may be padded ("09") or unpadded ("9"); compare numerically. + assert.strictEqual(output.is_last_phase, false, 'a real lower outstanding phase must keep is_last_phase=false'); + const nextNum = parseInt(String(output.next_phase), 10); + assert.strictEqual(nextNum, 9, `real lower phase 9 must be selected as next_phase (got ${output.next_phase})`); + }); + + test('zeroXSentinelNoCurrentPhaseDesync', () => { + // Row 3 (acceptance #3/#4): current_phase and current_phase_name must not desync when + // a 0.x sentinel is present and the milestone completes. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +- [ ] **Phase 0.1: Backlog** +- [ ] **Phase 5: Only phase** + +### Phase 5: Only phase +**Goal:** five +**Plans:** 1 plans +`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 5\n**Current Phase Name:** Only phase\n**Status:** In progress\n**Current Plan:** 05-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 5\n`, + ); + scaffoldPhase('05-only-phase', 5); + + const result = runVerifiedPhaseComplete('phase complete 5', tmpDir); + assert.ok(result.success, `Command failed: ${result.error || result.output}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.is_last_phase, true, 'milestone completes despite the 0.x sentinel'); + + const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + // current_phase must NOT have advanced to 0.1 (no desync into the sentinel). + assert.ok(!/\*\*Current Phase:\*\*\s*0\.1/i.test(state), 'no desync: current_phase did not advance to 0.1'); + }); + + test('checkedZeroXSentinelIrrelevant', () => { + // Row 4 (boundary): a CHECKED 0.x sentinel is irrelevant — completing the last real phase + // still completes the milestone. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +- [x] **Phase 0.1: Already-done backlog item** +- [ ] **Phase 3: Last phase** + +### Phase 3: Last phase +**Goal:** three +**Plans:** 1 plans +`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# State\n\n**Current Phase:** 3\n**Current Phase Name:** Last phase\n**Status:** In progress\n**Current Plan:** 03-01\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working on phase 3\n`, + ); + scaffoldPhase('03-last-phase', 3); + + const result = runVerifiedPhaseComplete('phase complete 3', tmpDir); + assert.ok(result.success, `Command failed: ${result.error || result.output}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.is_last_phase, true, 'checked sentinel is irrelevant; milestone completes'); + assert.strictEqual(output.next_phase, null); + }); +}); + }); +} diff --git a/tests/review-lane-descriptor.test.cjs b/tests/review-lane-descriptor.test.cjs index 556546d0e..3f393b211 100644 --- a/tests/review-lane-descriptor.test.cjs +++ b/tests/review-lane-descriptor.test.cjs @@ -712,3 +712,334 @@ describe('reviewer lane descriptor — declared shape (ADR-2782 D1/D2/D6/D7)', ( assert.ok(Object.isFrozen(PARITY_VIOLATION)); }); }); + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2927-reviewer-lane-overlay-invocation.test.cjs — H3 wave-fold (#3339) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe("folded:issue-2927-reviewer-lane-overlay-invocation", () => { +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * Regression test for #2927 — third-party reviewer lane installs and is + * roster-visible, but `review-lane sections|flags|plan|invoke` cannot select, + * plan, or invoke it. + * + * Root cause: `routeReviewLane` (gsd-core/bin/gsd-tools.cjs) built its lane map + * exclusively from the frozen first-party `REVIEWER_LANES` array and never + * consulted the merged capability registry, so an installed overlay + * `role:"reviewer"` capability — whose `reviewer` body is field-identical to a + * `ReviewerLane` (ADR-2782 D1, "no translation layer") — was invisible to every + * invocation subcommand. + * + * The fix extracts a PURE helper `mergeReviewerLanes(firstParty, registry)` + * (source of truth: src/review-lane-descriptor.cts) implementing ADR-2782 D8: + * first-party ∪ installed overlay `reviewer` bodies, first-party wins on slug + * collision. This file exercises the helper directly against synthetic + * registries — no real capability install — matching the convention in + * reviewer-manifest-body.test.cjs / review-lane-invocation.test.cjs. + * + * Matrix: .gsd/bug/fix/2927-reviewer-lane-overlay-invocation/50-test-matrix.md + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + REVIEWER_LANES, + mergeReviewerLanes, + LANE_SLUG_RE, +} = require('../gsd-core/bin/lib/review-lane-descriptor.cjs'); + +/** A first-party lane set small enough to read at a glance, but real-shaped. */ +const FP = REVIEWER_LANES.slice(0, 2); // gemini, claude +const FP_SLUGS = FP.map((l) => l.slug); + +/** A valid overlay `reviewer` body, field-identical to a SpawnLane (ADR-2782 D1). */ +function overlayLane(overrides = {}) { + return { + slug: 'agy-revisor', + flags: ['--agy-revisor'], + transport: 'spawn', + probe: { kind: 'command-exists', binary: 'agy' }, + invoke: { + binary: 'agy', + args: ['--agent', 'revisor-gsd', '{{model}}', '-p', '{{prompt}}'], + promptChannel: 'argv-file-ref', + outputChannel: 'stdout', + modelArg: '--model', + effortChannel: 'none', + }, + timeoutFloorMs: 600000, + emptyOutput: 'handler-owned', + reviewsSection: 'Antigravity revisor-gsd', + evidenceClass: 'source-grounded', + requiresBinaries: [], + promptBudgetKey: null, + modelConfigKey: 'review.models.agy-revisor', + handler: 'antigravity', + ...overrides, + }; +} + +/** A `role:"reviewer"` capability envelope carrying a reviewer body. */ +function reviewerCap(body) { + return { id: body && typeof body === 'object' && body.slug ? body.slug : 'x', role: 'reviewer', reviewer: body }; +} + +/** Build a synthetic registry shape ({ capabilities: { id: cap } }). */ +function registry(...caps) { + const capabilities = {}; + for (const c of caps) capabilities[c.id] = c; + return { capabilities }; +} + +describe('mergeReviewerLanes (#2927)', () => { + test('overlayAbsentReturnsFirstPartyUnchanged', () => { + // Row 1: no overlay reviewer caps → merged set is first-party exactly. + const merged = mergeReviewerLanes(FP, registry()); + assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS); + assert.equal(merged.length, FP.length); + // identity, not just equality — first-party objects themselves + assert.equal(merged[0], FP[0]); + assert.equal(merged[1], FP[1]); + }); + + test('overlayLaneIncludedInMerge', () => { + // Row 2 (failing-first regression): one valid non-colliding overlay lane is present. + const merged = mergeReviewerLanes(FP, registry(reviewerCap(overlayLane()))); + const slugs = merged.map((l) => l.slug); + assert.ok(slugs.includes('agy-revisor'), 'overlay slug admitted into merged set'); + assert.ok(slugs.includes('gemini'), 'first-party lanes preserved'); + // the overlay body itself is the merged entry (no translation layer) + const overlay = merged.find((l) => l.slug === 'agy-revisor'); + assert.equal(overlay.reviewsSection, 'Antigravity revisor-gsd'); + assert.deepEqual(overlay.flags, ['--agy-revisor']); + }); + + test('firstPartyWinsOnSlugCollision', () => { + // Row 3 / D8: an overlay declaring a first-party slug is superseded by first-party. + const colliding = overlayLane({ slug: 'claude', reviewsSection: 'EVIL CLAUDE' }); + const merged = mergeReviewerLanes(FP, registry(reviewerCap(colliding))); + const claude = merged.find((l) => l.slug === 'claude'); + assert.equal(claude, FP.find((l) => l.slug === 'claude'), 'first-party identity wins'); + assert.notEqual(claude.reviewsSection, 'EVIL CLAUDE', 'overlay did not leak through'); + assert.equal(merged.length, FP.length, 'collision added no extra entry'); + }); + + test('runtimeCapWithoutReviewerBodyAddsNoLane', () => { + // Row 4: a role:"runtime" cap with only the legacy reviewerCli alias has no lane descriptor. + const runtimeCap = { id: 'some-runtime', role: 'runtime', runtime: { hostBehaviors: { reviewerCli: true } } }; + const merged = mergeReviewerLanes(FP, registry(runtimeCap)); + assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS, 'runtime alias contributed no lane'); + }); + + test('emptySlugOverlaySkippedNotThrown', () => { + // Row 5: an overlay body whose slug is empty/whitespace is skipped, never throws. + const empty = reviewerCap(overlayLane({ slug: ' ' })); + const missing = reviewerCap(overlayLane({ slug: '' })); + assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(empty))); + assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(missing))); + const merged = mergeReviewerLanes(FP, registry(empty, missing)); + assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS, 'empty-slug overlays admitted no lane'); + }); + + test('invalidGrammarSlugSkipped', () => { + // Row 6 / security: a slug outside LANE_SLUG_RE (path-traversal class) is skipped at the merge. + const evil = reviewerCap(overlayLane({ slug: '../evil' })); + assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(evil))); + const merged = mergeReviewerLanes(FP, registry(evil)); + assert.ok(!merged.map((l) => l.slug).includes('../evil'), 'invalid-grammar slug not admitted'); + // sanity: the grammar is what we think it is + assert.ok(!LANE_SLUG_RE.test('../evil')); + assert.ok(LANE_SLUG_RE.test('agy-revisor')); + }); + + test('twoOverlaysBothIncluded', () => { + // Row 7: two distinct non-colliding overlays both present; count == fp + 2. + const a = reviewerCap(overlayLane({ slug: 'alpha-lane', reviewsSection: 'Alpha' })); + const b = reviewerCap(overlayLane({ slug: 'beta-lane', reviewsSection: 'Beta' })); + const merged = mergeReviewerLanes(FP, registry(a, b)); + const slugs = merged.map((l) => l.slug); + assert.ok(slugs.includes('alpha-lane')); + assert.ok(slugs.includes('beta-lane')); + assert.equal(merged.length, FP.length + 2); + }); + + test('malformedReviewerBodySkipped', () => { + // Row 8: reviewer body that is null / array / string is skipped, no throw. + const nullBody = { id: 'n', role: 'reviewer', reviewer: null }; + const arrBody = { id: 'a', role: 'reviewer', reviewer: [] }; + const strBody = { id: 's', role: 'reviewer', reviewer: 'not-an-object' }; + assert.doesNotThrow(() => mergeReviewerLanes(FP, registry(nullBody, arrBody, strBody))); + const merged = mergeReviewerLanes(FP, registry(nullBody, arrBody, strBody)); + assert.deepEqual(merged.map((l) => l.slug), FP_SLUGS, 'malformed bodies admitted no lane'); + }); +}); + +// --------------------------------------------------------------------------- +// Rows 9–10: the WIRING defect this PR exists to close. The eight rows above +// guard the pure helper, but the actual bug was that `routeReviewLane` never +// CALLED any merge — so a revert of the one-line wiring change would leave every +// helper test green. These rows exercise the real CLI end-to-end: install a +// global-scope `role:"reviewer"` overlay (global scope is trusted without a +// consent record, CONTEXT.md capability-loader predicate), then assert +// `review-lane sections|flags|plan` actually see it through loadRegistry → +// mergeReviewerLanes → the lane map. This is acceptance criteria #1–#3. +// --------------------------------------------------------------------------- + +const fs = require('node:fs'); +const os = require('node:os'); +const nodePath = require('node:path'); +const { runGsdTools, cleanup } = require('./helpers.cjs'); + +const cliTmps = []; +function cliTmpDir(prefix) { + const d = fs.mkdtempSync(nodePath.join(os.tmpdir(), prefix)); + cliTmps.push(d); + return d; +} +test.after(() => { for (const d of cliTmps) cleanup(d); }); + +/** A GSD_HOME-sandboxed env that neutralizes ambient GSD_ vars (hermeticity). */ +function scopeEnv(home) { + return { GSD_HOME: home, GSD_WORKSTREAM: '', GSD_PROJECT: '' }; +} + +/** A cwd with a .planning/ root so findProjectRoot resolves cleanly. */ +function makeCwd() { + const cwd = cliTmpDir('rev2927-cwd-'); + fs.mkdirSync(nodePath.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(nodePath.join(cwd, '.planning', 'config.json'), '{}'); + return cwd; +} + +/** + * Write a conformant `role:"reviewer"` capability source dir whose `reviewer` + * body is a valid SpawnLane (ADR-2782 D1 shape). Returns the source path, + * usable as a `capability install ` argument. + */ +function writeReviewerCapSource(id, bodyOverrides = {}) { + const src = cliTmpDir(`rev2927-src-${id}-`); + // A `role:"reviewer"` manifest carries ONLY id/role/version/title/description/ + // tier/requires/engines/reviewer (+ optional config) — skills/agents/steps/ + // contributions/gates/hooks/runtimeCompat are feature-only fields the validator + // rejects for a reviewer (mirrors the shipped `capabilities/lm-studio` shape). + const cap = { + id, + role: 'reviewer', + version: '1.0.0', + title: `${id} test lane`, + description: 'test third-party reviewer lane for #2927', + tier: 'standard', + requires: [], + engines: { gsd: '>=1.9.0' }, + reviewer: { + slug: id, + flags: [`--${id}`], + transport: 'spawn', + probe: { kind: 'command-exists', binary: id }, + invoke: { + binary: id, + args: ['{{model}}', '-p', '{{prompt}}'], + promptChannel: 'stdin', + outputChannel: 'stdout', + modelArg: '--model', + effortChannel: 'none', + }, + timeoutFloorMs: 600000, + emptyOutput: 'stub-with-stderr', + reviewsSection: `${id} review`, + evidenceClass: 'source-grounded', + requiresBinaries: [], + promptBudgetKey: null, + modelConfigKey: `review.models.${id}`, + handler: null, + ...bodyOverrides, + }, + }; + fs.writeFileSync(nodePath.join(src, 'capability.json'), JSON.stringify(cap, null, 2)); + return src; +} + +describe('review-lane CLI overlay invocation (#2927, rows 9–10)', () => { + test('cliSectionsAndPlanSeeOverlayLane', () => { + // Acceptance #1 + #3: an installed overlay lane appears in `sections` and + // `plan --selected ` returns ok:true with a usable plan. + const home = cliTmpDir('rev2927-home-'); + const cwd = makeCwd(); + const src = writeReviewerCapSource('rev2927lane'); + // Global scope is trusted without a consent record; --yes acknowledges the + // executable reviewer surface; --raw emits JSON. + const install = runGsdTools( + ['capability', 'install', src, '--scope', 'global', '--yes', '--raw'], + cwd, + scopeEnv(home), + ); + assert.equal(install.success, true, `install failed: ${install.error || install.output}`); + const installOut = JSON.parse(install.output); + assert.equal(installOut.status, 'installed', `install did not report installed: ${install.output}`); + + // Row 9 / acceptance #1: sections includes the overlay slug + reviewsSection. + const sections = runGsdTools(['review-lane', 'sections'], cwd, scopeEnv(home)); + assert.equal(sections.success, true, `sections failed: ${sections.error || sections.output}`); + const sectionRows = sections.output.split('\n').filter(Boolean); + const overlayRow = sectionRows.find((r) => r.startsWith('rev2927lane\t')); + assert.ok(overlayRow, `overlay lane missing from sections output:\n${sections.output}`); + assert.equal(overlayRow, 'rev2927lane\trev2927lane review'); + + // Row 9 / acceptance #3: plan --selected resolves ok (NOT + // malformed_lane / no such declared lane — the pre-fix failure). The `plan` + // subcommand renders an ARRAY of {slug, ok, section, transport, ...} (it strips + // the nested invocation `plan` object before output), so find the overlay entry. + const plan = runGsdTools( + ['review-lane', 'plan', '--selected', 'rev2927lane', '--run-dir', cwd, '--repo-root', cwd], + cwd, + scopeEnv(home), + ); + assert.equal(plan.success, true, `plan failed: ${plan.error || plan.output}`); + const planOut = JSON.parse(plan.output); + assert.ok(Array.isArray(planOut), `plan output is not an array:\n${plan.output}`); + const overlayPlan = planOut.find((p) => p.slug === 'rev2927lane'); + assert.ok(overlayPlan, `overlay plan entry missing:\n${plan.output}`); + assert.equal(overlayPlan.ok, true, `overlay plan did not resolve ok:\n${plan.output}`); + assert.equal(overlayPlan.section, 'rev2927lane review'); + assert.equal(overlayPlan.transport, 'spawn'); + }); + + test('cliFlagsIncludeOverlayFlag', () => { + // Acceptance #2: the overlay's declared --flag appears in `flags` output. + // + // NOTE on the negative-space "malformed flag filtered" case: the capability + // validator enforces the /^--[a-z0-9][a-z0-9-]*$/ flag grammar AT INSTALL TIME + // (capability-validator rejects a reviewer.flags entry that fails it), so a lane + // carrying a malformed flag (e.g. `--bad flag`, `*.js`) can never be installed + // and therefore never reaches the `flags` shape filter. That filter is + // defense-in-depth over an input class the validator already excludes; it is + // not independently reachable through a validated install, so it is not asserted + // here. A lane declaring two well-formed flags (mirroring antigravity's + // --antigravity/--agy) proves the per-lane flag array is preserved, not flattened. + const home = cliTmpDir('rev2927-home-'); + const cwd = makeCwd(); + const src = writeReviewerCapSource('rev2927flag', { + flags: ['--rev2927flag', '--rev2927alt'], + }); + const install = runGsdTools( + ['capability', 'install', src, '--scope', 'global', '--yes', '--raw'], + cwd, + scopeEnv(home), + ); + assert.equal(install.success, true, `install failed: ${install.error || install.output}`); + assert.equal(JSON.parse(install.output).status, 'installed', `install did not report installed: ${install.output}`); + + const flags = runGsdTools(['review-lane', 'flags'], cwd, scopeEnv(home)); + assert.equal(flags.success, true, `flags failed: ${flags.error || flags.output}`); + const flagLines = flags.output.split('\n').filter(Boolean); + assert.ok(flagLines.includes('--rev2927flag'), `overlay flag missing from flags output:\n${flags.output}`); + assert.ok(flagLines.includes('--rev2927alt'), 'second well-formed overlay flag missing (flag array flattened?)'); + }); +}); + }); +} diff --git a/tests/state-document.test.cjs b/tests/state-document.test.cjs index 3cb553bf1..a51c31586 100644 --- a/tests/state-document.test.cjs +++ b/tests/state-document.test.cjs @@ -309,3 +309,899 @@ describe('property: fieldNameMatchesRawCell case-fold semantics, via stateExtrac ); }); }); + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-2828-flat-roadmap-total-phases.test.cjs (H3 Wave 7, #3339) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-2828-flat-roadmap-total-phases', () => { +'use strict'; + +// Regression guard for #2828: on a flat unmilestoned roadmap (no versioned milestone +// heading), `state-snapshot`/`state record-session` reported progress.total_phases as +// the on-disk phase-dir count (1) instead of the authoritative roadmap count (6). The +// read-path disk-scan cache fell back to phaseDirs.length when milestoneBounded was +// false, even though roadmapPhaseCount (6) was correct for a flat roadmap (no sibling +// milestones to conflate). Fix: use roadmapPhaseCount as the floor when > 0. + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +describe('#2828 — total_phases uses the roadmap count on a flat unmilestoned roadmap', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-2828-'); + const planningDir = path.join(tmpDir, '.planning'); + // Flat unmilestoned roadmap with 6 phases (no versioned milestone heading). + fs.writeFileSync( + path.join(planningDir, 'ROADMAP.md'), + [ + '# Roadmap', + '', + '### Phase 1: Foundation', + '### Phase 2: Core API', + '### Phase 3: UI Layer', + '### Phase 4: Integration', + '### Phase 5: Polish', + '### Phase 6: Release', + '', + ].join('\n'), + ); + // Only phase 1 has been discussed → 1 phase dir on disk. + const phaseDir = path.join(planningDir, 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), '# Phase 1 Context\n'); + // Minimal STATE.md with a milestone set (so milestoneBounded is computed) but no + // versioned heading to bound it to → the flat-roadmap unbounded case. + fs.writeFileSync( + path.join(planningDir, 'STATE.md'), + [ + '---', + 'status: executing', + 'milestone: v1.0', + 'milestone_name: milestone', + '---', + '', + '# Project State', + '', + '**Current Phase:** 01', + '**Status:** In progress', + '', + ].join('\n'), + ); + }); + + afterEach(() => cleanup(tmpDir)); + + test('state sync writes progress.total_phases === 6 (roadmap count), not 1 (phase-dir count) (#2828)', () => { + // `state sync` derives progress.total_phases from the disk-scan cache (the read path + // #2828 fixes) and writes it to STATE.md frontmatter. Pre-fix this wrote 1. + const result = runGsdTools(['state', 'sync'], tmpDir); + assert.ok(result.success, `state sync failed: ${result.error}`); + + const stateMd = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8'); + // Parse the `progress:` YAML block line-by-line (ReDoS-safe: avoids a nested-quantifier + // regex over the whole block). Find total_phases among the block's indented children. + const lines = stateMd.split(/\r?\n/); + let inProgress = false; + let totalPhases = null; + for (const line of lines) { + if (/^progress:\s*$/.test(line)) { inProgress = true; continue; } + if (inProgress) { + // A new top-level (column-0) key ends the progress block. + if (/^\S/.test(line)) { inProgress = false; continue; } + const tp = line.match(/^\s+total_phases:\s*(\d+)/); + if (tp) { totalPhases = Number(tp[1]); break; } + } + } + assert.ok( + totalPhases !== null, + `progress.total_phases must be written by state sync. STATE.md:\n${stateMd}`, + ); + assert.strictEqual( + totalPhases, + 6, + `progress.total_phases must be the roadmap count (6) for a flat unmilestoned roadmap, not the on-disk phase-dir count (1). Got: ${totalPhases}`, + ); + }); +}); + }); +} + +// ──────────────────────────────────────────────────────────────────────── +// Folded from tests/issue-3204-state-writer-phase-count.test.cjs (H3 Wave 7, #3339) +// ──────────────────────────────────────────────────────────────────────── +{ + const { describe: __foldDescribe } = require('node:test'); + __foldDescribe('folded:issue-3204-state-writer-phase-count', () => { +// allow-test-rule: source-text-is-the-product, see #3204 +// Reads STATE.md/ROADMAP.md fixture files whose deployed text IS what the +// runtime loads — testing text content tests the deployed contract. + +/** + * #3204 / #3185 — failing-first regression suite for `buildStateFrontmatter`'s + * `total_phases` selection (`src/state.cts:1620`, guard at `:1795-1805`). + * + * `hasMilestoneSectioning` (`src/roadmap-parser.cts:195`) is + * /^#{2,3}\s+(?!Phase\s+\S)/mi + * — true for ANY non-Phase level-2/3 heading, so a FLAT roadmap carrying an + * ordinary structural heading (`## Progress`, `## Overview`, ...) is + * misclassified as milestone-sectioned. `safeToUseRoadmapCount` then goes + * false and the on-disk phase-directory count silently clobbers the + * ROADMAP-declared count — a regression of #2828, reported in #3204 as + * "roadmap declares 6 phases, 4 directories exist, state.record-session + * writes total_phases: 4". + * + * DO NOT fix src/state.cts or src/roadmap-parser.cts from this file. Rows 2, + * 3, and 14 below assert the CORRECT (post-fix) value and currently FAIL — + * that is the point of a failing-first suite. Every other row asserts + * behavior verified to already hold today (see phase-log for the manual CLI + * probes that established each expected value before this file was written). + * + * Rows and naming follow `.gsd/phase/fix-3185-state-writer-phase-count/50-test-matrix.md` + * verbatim (row numbers refer to that matrix, not the 8-row table in + * `40-design.md`). + * + * Driven via `state record-session` (the shape #3204's own report used), + * then read back with `state json --raw` — the product's own frontmatter + * parser — so `progress.total_phases` is asserted as a NUMBER, never a + * regex over rendered STATE.md text. `tests/helpers.cjs`'s `parseFrontmatter` + * only reads flat top-level keys (it does not descend into the nested + * `progress:` block), so `state json --raw` is the correct structured seam + * for a nested field — it is what `tests/state.test.cjs`'s own '#1761 + * read-path' and 'milestone-scoped phase counting' suites already use for + * this exact assertion shape. + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +// ───────────────────────────────────────────────────────────────────────────── +// Fixture builders +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Seed `.planning/phases/-phase-` for each phase number in `nums`, + * each with a single PLAN.md so the directory is a recognizable phase dir. + */ +function seedPhaseDirs(tmpDir, nums) { + for (const n of nums) { + const padded = String(n).padStart(2, '0'); + const dir = path.join(tmpDir, '.planning', 'phases', `${padded}-phase-${n}`); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, `${padded}-01-PLAN.md`), '# Plan\n'); + } +} + +/** Seed one arbitrarily-named phase directory (sentinel / dup / pre-milestone cases). */ +function seedNamedPhaseDir(tmpDir, dirName, planBase) { + const dir = path.join(tmpDir, '.planning', 'phases', dirName); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, `${planBase}-01-PLAN.md`), '# Plan\n'); +} + +/** + * Build STATE.md frontmatter + minimal body. `milestone` is always set (the + * #3204 fixture needs it truthy — `getMilestoneInfo` defaults an absent + * `milestone:` field to 'v1.0' anyway, so this pins the same value + * explicitly for every row rather than relying on that fallback). + * Lines are joined with the caller-supplied `eol` (default '\n') — row 14 + * reuses this to build the CRLF variant without a second copy. + */ +function buildStateMd({ milestone = 'v1.0', milestoneName = 'Test', totalPhases, currentPhase = '01', eol = '\n' }) { + const lines = [ + '---', + 'gsd_state_version: 1.0', + `milestone: ${milestone}`, + `milestone_name: ${milestoneName}`, + `current_phase: "${currentPhase}"`, + 'status: executing', + 'progress:', + ` total_phases: ${totalPhases}`, + ' completed_phases: 0', + ' total_plans: 0', + ' completed_plans: 0', + ' percent: 0', + '---', + '', + '# GSD State', + '', + '## Current Position', + '', + `**Current Phase:** ${currentPhase}`, + '**Status:** Executing', + '', + ]; + return lines.join(eol); +} + +/** Invoke `state record-session` (the #3204 entry point) then read back `state json --raw`. */ +function recordSessionAndReadTotalPhases(tmpDir) { + const recordResult = runGsdTools( + ['state', 'record-session', '--stopped-at', 'Phase 1, Plan 1', '--resume-file', 'none'], + tmpDir, + ); + assert.ok(recordResult.success, `state record-session failed: ${recordResult.error}`); + + const jsonResult = runGsdTools(['state', 'json', '--raw'], tmpDir); + assert.ok(jsonResult.success, `state json --raw failed: ${jsonResult.error}`); + return JSON.parse(jsonResult.output); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Rows 1-3, 14 — #3204 regression: flat roadmap + a structural heading +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3204 buildStateFrontmatter total_phases — flat roadmap misclassified as milestone-sectioned', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('flat roadmap with no structural headings keeps the roadmap count', () => { + // Row 1 (happy path / control) — no non-Phase heading anywhere, so + // hasMilestoneSectioning is false today and this already passes. Guards + // against a fix that overcorrects and breaks the trivial flat case. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual(Number(out.progress.total_phases), 6, `expected roadmap count 6, got ${out.progress && out.progress.total_phases}`); + }); + + test('#3204 flat roadmap with a Progress heading is not treated as milestone-sectioned', () => { + // Row 2 — the crux repro, transcribed from #3204's own report: 6 + // declared phases, 4 directories, a flat '## Progress' heading. FAILS + // TODAY: hasMilestoneSectioning misclassifies '## Progress' as + // sectioning, safeToUseRoadmapCount goes false, and the write clobbers + // total_phases down to the disk count (4) instead of 6. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + '## Progress', + '', + 'Some progress notes.', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 6, + `#3204: total_phases must stay 6 (roadmap-declared), not clobber to the disk count of 4. Got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('#3204 multiple structural headings still count as flat', () => { + // Row 3 — same shape as row 2 with TWO structural headings ('## Overview', + // '## Phase Details'); '## Phase Details' is correctly excluded by the + // heading's own '(?!Phase\s+\S)' lookahead, but '## Overview' still trips + // the misclassification. FAILS TODAY for the same reason as row 2. + const roadmap = [ + '# Roadmap', + '', + '## Overview', + '', + 'Some overview text.', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + '## Phase Details', + '', + 'More detail prose.', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 6, + `#3204: multiple structural headings must still count as flat (6), got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('#3204 repro under CRLF', () => { + // Row 14 — row 2's exact repro, all fixture content authored with CRLF + // line endings, proving the bug (and required fix) is not an artifact of + // LF-only fixtures. FAILS TODAY for the same reason as row 2. + const roadmapLines = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + '## Progress', + '', + 'Some progress notes.', + '', + ]; + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmapLines.join('\r\n')); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ totalPhases: 6, eol: '\r\n' }), + ); + seedPhaseDirs(tmpDir, [1, 2, 3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 6, + `#3204 under CRLF: total_phases must stay 6, got ${out.progress && out.progress.total_phases}`, + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Rows 4-11 — negative space and boundaries (must hold both before and after the fix) +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3204 buildStateFrontmatter total_phases — negative space / boundaries (must not regress)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('bounded milestone uses its own section count', () => { + // Row 4 — versioned roadmap with two sibling milestone sections ('## v1.0' + // owning phases 1-2, '## v2.0' owning phases 3-5). The asserted milestone + // ('v2.0') IS bound to its own heading, so `sliceMilestoneWindow`/ + // `extractCurrentMilestoneScoped` narrow to that section and + // roadmapPhaseCount is the SECTION's count (3), not the whole-document + // count (5) and not the disk count (2 dirs seeded). + const roadmap = [ + '# Roadmap', + '', + '## v1.0', + '## Phase 1: One', + '## Phase 2: Two', + '', + '## v2.0', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ milestone: 'v2.0', milestoneName: 'Second', totalPhases: 3 }), + ); + seedPhaseDirs(tmpDir, [3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 3, + `a bounded milestone must use its own section's phase count (3), not the whole document (5) or the disk count (2). Got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('a single milestone section cannot conflate siblings', () => { + // Row 6 — exactly ONE '## v2.0' section owning phases, with the asserted + // milestone ('v9.9') absent from the roadmap entirely. One milestone + // heading can never satisfy hasMilestoneSectioning's >=2 threshold, so + // this is NOT sectioned and the roadmap-declared count is still used. + const roadmap = [ + '# Roadmap', + '', + '## v2.0', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ milestone: 'v9.9', milestoneName: 'Absent', totalPhases: 4 }), + ); + seedPhaseDirs(tmpDir, [1, 2]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 4, + `a single milestone section cannot conflate siblings; expected the roadmap count (4), got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('#1761 sibling milestone sections still fall back to the disk count', () => { + // Row 5 — TWO sibling (unversioned) milestone sections, asserted + // milestone ('v3.0') absent from either. This is genuinely + // milestone-sectioned (2 phase-bearing sections would conflate if + // whole-doc counted), so total_phases must stay the disk count. Passes + // today; a fix that touches hasMilestoneSectioning must not break it. + const roadmap = [ + '# Roadmap', + '', + '## Milestone 1: First Milestone', + '### Phase 1: a', + '### Phase 2: b', + '### Phase 3: c', + '### Phase 4: d', + '', + '## Milestone 2: Second Milestone', + '### Phase 5: e', + '### Phase 6: f', + '### Phase 7: g', + '### Phase 8: h', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ milestone: 'v3.0', milestoneName: 'Third', totalPhases: 8 }), + ); + seedPhaseDirs(tmpDir, [1, 2, 3]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 3, + `#1761: unbounded sibling milestones must fall back to the disk count (3), got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('zero phase directories keeps the declared count', () => { + // Row 7 — boundary limit-1: 0 dirs vs 6 declared. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); + // No phase dirs seeded. + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual(Number(out.progress.total_phases), 6, `expected 6, got ${out.progress && out.progress.total_phases}`); + }); + + test('equal counts agree', () => { + // Row 8 — boundary limit: 6 dirs vs 6 declared. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4, 5, 6]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual(Number(out.progress.total_phases), 6, `expected 6, got ${out.progress && out.progress.total_phases}`); + }); + + test('extra directories win via max()', () => { + // Row 9 — boundary limit+1. 6 heading-declared phases + a 7th phase + // declared only via the bullet-entry syntax ('- [ ] **Phase 7 — Extra**', + // #2199 bullet house style), which the directory-membership filter + // counts but the heading-only roadmapPhaseCount scan does not — so disk + // (7, all pass the membership filter) legitimately exceeds the + // heading-only roadmap count (6), and max() must pick 7. + // + // NOTE: a naive "N heading-declared phases + N+1 plain directories" does + // NOT exercise this path — the directory-membership filter + // (getMilestonePhaseFilter, roadmap-parser.cts) excludes any directory + // whose phase number has no matching roadmap entry at all, so an + // out-of-roadmap directory number is silently dropped from the disk + // count rather than inflating it. Verified against the running CLI + // before authoring this fixture. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + '- [ ] **Phase 7 — Extra**', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 6 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4, 5, 6, 7]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 7, + `expected max(7 dirs, 6 heading-declared) = 7, got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('absent roadmap falls back to disk', () => { + // Row 10 — no ROADMAP.md at all. + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual(Number(out.progress.total_phases), 4, `expected disk count 4, got ${out.progress && out.progress.total_phases}`); + }); + + test('roadmap with no phase headings falls back to disk', () => { + // Row 11 — ROADMAP.md present but zero Phase headings anywhere. + const roadmap = ['# Roadmap', '', '## Notes', '', 'No phases declared yet.', ''].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); + seedPhaseDirs(tmpDir, [1, 2, 3, 4]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual(Number(out.progress.total_phases), 4, `expected disk count 4, got ${out.progress && out.progress.total_phases}`); + }); + + test('a phase heading carrying a version token is not a milestone heading', () => { + // Row 12 (hostile, negative space) — '### Phase 3: Ship v2.0 gaps' carries + // a version token in its own text, but hasMilestoneSectioning's + // isPhaseHeading check excludes any heading matching '^Phase\s+\S' before + // the vocabulary signal is ever tested, so this must NOT count as a + // milestone heading. Otherwise-flat roadmap, so the roadmap-declared + // count must be used, not the disk count. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '### Phase 3: Ship v2.0 gaps', + '## Phase 4: Four', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); + seedPhaseDirs(tmpDir, [1, 2]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 4, + `a version token borne by a Phase heading must not trigger milestone sectioning; expected roadmap count 4, got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('a version heading inside a fence is not sectioning', () => { + // Row 13 (hostile, negative space) — '## v2.0' appears only inside a + // fenced code block (a documentation example of the heading syntax) on an + // otherwise flat roadmap. hasMilestoneSectioning is routed through + // tokenizeHeadings (fence-aware), so a fenced heading is never tokenised + // and must NOT count as sectioning. The roadmap-declared count must be + // used, not the disk count. + const roadmap = [ + '# Roadmap', + '', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Phase 4: Four', + '', + 'Example heading syntax:', + '', + '```', + '## v2.0', + '```', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 4 })); + seedPhaseDirs(tmpDir, [1, 2]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 4, + `a version heading inside a fence must not trigger milestone sectioning; expected roadmap count 4, got ${out.progress && out.progress.total_phases}`, + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// #3185 adversarial review — hasMilestoneSectioning ownership-model shapes +// missed by the original suite (BLOCKER + MAJOR findings against the #3184 +// "strictly-deeper nesting" rewrite). +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3185 review — hasMilestoneSectioning shapes the original suite missed', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('BLOCKER: same-level sibling milestones fall back to the disk count', () => { + // Adversarial review BLOCKER (#1761 regression): the #3184 rewrite + // required a candidate milestone heading's owned Phase heading to be + // STRICTLY DEEPER (next.level > candidate.level). Real sibling + // milestones are frequently at the SAME level as their own Phase + // headings ('## v1.0' / '## Phase 1:' / '## v2.0' / '## Phase 3:'), so + // that predicate answered false and the whole-document count conflated + // both milestones. The asserted milestone ('v3.0') is unbound (matches + // neither v1.0 nor v2.0), so this is genuinely sectioned and must fall + // back to the disk count. + const roadmap = [ + '# Roadmap', + '', + '## v1.0', + '## Phase 1: One', + '## Phase 2: Two', + '', + '## v2.0', + '## Phase 3: Three', + '## Phase 4: Four', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ milestone: 'v3.0', milestoneName: 'Third', totalPhases: 4 }), + ); + seedPhaseDirs(tmpDir, [1, 2]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 2, + `same-level sibling milestones must fall back to the disk count (2), got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('#3185 repro: structural headings interleaved among flat phases keep the roadmap count', () => { + // #3185's own reproduction of the adjacency model this suite's + // predecessor shipped: '## Overview' sits immediately before + // '## Phase 1:' and '## Notes' sits immediately before '## Phase 4:', + // giving an adjacency-based predicate 2 "owning" candidates even though + // neither heading carries any milestone vocabulary (no version token, no + // status marker, no "Milestone" word) and the roadmap is genuinely flat. + const roadmap = [ + '# Roadmap', + '', + '## Overview', + '## Phase 1: One', + '## Phase 2: Two', + '## Phase 3: Three', + '## Notes', + '## Phase 4: Four', + '## Phase 5: Five', + '## Phase 6: Six', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ milestone: 'v9.9', milestoneName: 'Test', totalPhases: 6 }), + ); + seedPhaseDirs(tmpDir, [1, 2]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 6, + `#3185: structural headings adjacent to phase headings must not be treated as milestone sectioning; expected roadmap count 6, got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('MAJOR: wrapper + single nested milestone keeps the roadmap count', () => { + // Adversarial review MAJOR (#3204 reintroduction): every ancestor in a + // nesting chain was counted as its own candidate section under the + // #3184 rewrite, so a generic wrapper heading with only ONE real + // milestone nested under it was misclassified as sectioned. Mirrors + // this repo's own bundled template shape (gsd-core/templates/roadmap.md: + // '## Phases' -> '### 🚧 v1.1 [Name] (In Progress)' -> '#### Phase N:'). + // The asserted milestone ('v9.9') is deliberately unbound so the + // assertion exercises hasMilestoneSectioning itself, not + // isMilestoneBoundedInRoadmap. + const roadmap = [ + '# Roadmap', + '', + '## Phases', + '', + '### 🚧 v1.1 [Name] (In Progress)', + '', + '#### Phase 1: One', + '#### Phase 2: Two', + '#### Phase 3: Three', + '#### Phase 4: Four', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + buildStateMd({ milestone: 'v9.9', milestoneName: 'Unbound', totalPhases: 2 }), + ); + seedPhaseDirs(tmpDir, [1, 2]); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 4, + `wrapper + single nested milestone must keep the roadmap-declared count (4), not clobber to the disk count of 2. Got ${out.progress && out.progress.total_phases}`, + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Rows 15-18 — #3185 consolidation independence checks +// +// These exercise the directory-enumeration owner (listMilestonePhaseDirs / +// getMilestonePhaseFilter), not hasMilestoneSectioning. Verified PASSING +// against the current build (manual CLI probe) before being added here — +// included per the dispatch brief's "include only if they pass today" +// condition. If a future change to the #3204 fix regresses one of these, +// that is a SEPARATE finding from the #3204 repro above, not folded into it. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3185 buildStateFrontmatter total_phases — directory-enumeration independence (currently passing)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('sentinel directories are excluded by the canonical enumeration', () => { + // Row 15 — a 999.x backlog directory alongside 3 real phase directories + // must not inflate total_phases. + const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 3 })); + seedPhaseDirs(tmpDir, [1, 2, 3]); + seedNamedPhaseDir(tmpDir, '999.1-backlog-idea', '999.1'); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 3, + `sentinel 999.x directory must be excluded, expected 3, got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('pre-milestone directories are excluded', () => { + // Row 16 — a '0-*' pre-milestone directory must not be counted. + const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 3 })); + seedPhaseDirs(tmpDir, [1, 2, 3]); + seedNamedPhaseDir(tmpDir, '0-premilestone', '0'); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 3, + `pre-milestone '0-*' directory must be excluded, expected 3, got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('duplicate phase-number directories count once', () => { + // Row 17 — two directories both keyed to phase number 2 must dedup to a + // single count. + const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), buildStateMd({ totalPhases: 3 })); + seedPhaseDirs(tmpDir, [1, 2, 3]); + seedNamedPhaseDir(tmpDir, '02-phase-2-dup', '02'); + + const out = recordSessionAndReadTotalPhases(tmpDir); + assert.strictEqual( + Number(out.progress.total_phases), + 3, + `duplicate phase-2 directories must dedup to a single count (3), got ${out.progress && out.progress.total_phases}`, + ); + }); + + test('re-running record-session does not move total_phases', () => { + // Row 18 — idempotence: a second record-session call over an unchanged + // tree, with the clock pinned so 'Last session' does not itself vary, + // must produce a byte-identical STATE.md. + const roadmap = ['# Roadmap', '', '## Phase 1: One', '## Phase 2: Two', '## Phase 3: Three', ''].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + const sessionState = [ + '# GSD State', + '', + '## Session', + '', + '**Last session:** 2024-01-01T00:00:00.000Z', + '**Stopped at:** None', + '**Resume file:** None', + '', + '## Current Position', + '', + '**Current Phase:** 01', + '**Status:** Executing', + '', + ].join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), sessionState); + seedPhaseDirs(tmpDir, [1, 2, 3]); + + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + const pinnedEnv = { GSD_TEST_MODE: '1', GSD_NOW_MS: '1600000000000' }; + const args = ['state', 'record-session', '--stopped-at', 'Phase 1, Plan 1', '--resume-file', 'none']; + + const first = runGsdTools(args, tmpDir, pinnedEnv); + assert.ok(first.success, `first record-session failed: ${first.error}`); + const afterFirst = fs.readFileSync(statePath, 'utf8'); + + const second = runGsdTools(args, tmpDir, pinnedEnv); + assert.ok(second.success, `second record-session failed: ${second.error}`); + const afterSecond = fs.readFileSync(statePath, 'utf8'); + + assert.strictEqual( + afterSecond, + afterFirst, + 're-running record-session on an unchanged tree with a pinned clock must produce a byte-identical STATE.md', + ); + }); +}); + }); +} From c444051bef8ca147bd3de59411cb98a933c7e8bb Mon Sep 17 00:00:00 2001 From: sim Date: Wed, 12 Aug 2026 01:09:40 -0400 Subject: [PATCH 2/5] =?UTF-8?q?test(#3339):=20fix=20orthogonal-review=20fi?= =?UTF-8?q?ndings=20=E2=80=94=20Wave=207=20fold?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Standards/Spec-axis review + Memtrace graph pass found real issues in the just-folded suites, all fixed here: - tests/phase.test.cjs: the issue explicitly asked to dedupe overlapping fixtures between issue-2945/issue-2949 — the fold preserved both test sets correctly (genuinely distinct code paths, 0 tests dropped, already verified correct) but left each fold block with its own near-identical copy of a runVerifiedPhaseComplete(args, tmpDir) helper, the fixture- level dedup the issue actually asked for. Consolidated into one shared definition without disturbing the file's other, unrelated same-named helper at module scope (would have collided if hoisted directly). Removed two now-unused local runGsdTools destructurings left behind by the consolidation (both blocks already close over the module-scope import at line 26). - tests/review-lane-descriptor.test.cjs: two .find() results dereferenced without a presence guard (same defect class Wave 3/#3335 already found and fixed once in this epic) — added assert.ok() guards matching the repo's established style. - tests/model-resolver.test.cjs: documented the 1-of-80 dropped duplicate test with an inline comment, matching this same wave's host-integration fold's convention of citing drops by exact reference instead of leaving a reviewer to reconstruct the justification via git archaeology. No test() count changed in any file. No production code touched. --- tests/model-resolver.test.cjs | 7 ++- tests/phase.test.cjs | 91 +++++++++++++-------------- tests/review-lane-descriptor.test.cjs | 2 + 3 files changed, 50 insertions(+), 50 deletions(-) diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 9c56419cf..d56ef896a 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -4781,7 +4781,12 @@ describe('#2297: install-marker precedence rung (GSD_RUNTIME and config.runtime } // ──────────────────────────────────────────────────────────────────────── -// Folded from tests/issue-2517-runtime-aware-profiles.test.cjs +// Folded from tests/issue-2517-runtime-aware-profiles.test.cjs (H3 Wave 7, +// issue #3339). 1 of 80 source test blocks ('resolveTierEntry helper: unknown +// runtime + no overrides -> null', runtime:'mystery') was dropped as a verified +// duplicate of the pre-existing test at line 525 ('unknown runtime + unknown +// tier, no overrides -> null') — same resolveTierEntry null-return assertion +// for an unknown runtime with no overrides. // ──────────────────────────────────────────────────────────────────────── { const { describe: __foldDescribe } = require('node:test'); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index ca4175277..b596966a1 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -10492,6 +10492,39 @@ describe('#2572: phase complete warns when a SUMMARY claims files that never lan }); }); +// Outer scope shared by the two fold blocks below (deliberately NOT module-scope: +// this file already declares a distinct, 3-arg `runVerifiedPhaseComplete(args, tmpDir, env)` +// at line 54 used throughout the rest of the file; nesting here avoids shadowing it). +{ +/** + * Write a passed-VERIFICATION marker for the phase, then run `phase complete N`. + * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` + * with `status: passed` frontmatter. Requires the phase directory to exist. + * + * Shared by the folded:issue-2945-phase-complete-checkbox-rollback and + * folded:issue-2949-phase-complete-stage3-sentinel blocks below — both fold sources + * defined this same helper independently; consolidated to one definition (PR #3339 + * review, Fowler-baseline duplication finding) since both bodies were functionally + * identical modulo variable naming. + */ +function runVerifiedPhaseComplete(args, tmpDir) { + const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); + const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); + const phase = argv[completeIdx + 1]; + const phasesDir = path.join(tmpDir, '.planning', 'phases'); + const wanted = parseInt(String(phase).replace(/^0+/, ''), 10); + const phaseDirName = fs.readdirSync(phasesDir).find((name) => { + const m = name.match(/^(\d+)/); + return m && parseInt(m[1], 10) === wanted; + }); + if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); + fs.writeFileSync( + path.join(phasesDir, phaseDirName, `${phase}-VERIFICATION.md`), + ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), + ); + return runGsdTools(args, tmpDir); +} + // ──────────────────────────────────────────────────────────────────────── // Folded from tests/issue-2945-phase-complete-checkbox-rollback.test.cjs — H3 Wave 7 test-hygiene sweep (#3339) // ──────────────────────────────────────────────────────────────────────── @@ -10519,30 +10552,11 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { createTempProject, cleanup } = require('./helpers.cjs'); -/** - * Write a passed-VERIFICATION marker for the phase, then run `phase complete N`. - * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` - * with `status: passed` frontmatter. Requires the phase directory to exist. - */ -function runVerifiedPhaseComplete(args, tmpDir) { - const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); - const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); - const phase = argv[completeIdx + 1]; - const phasesDir = path.join(tmpDir, '.planning', 'phases'); - const wanted = parseInt(String(phase).replace(/^0+/, ''), 10); - const phaseDirName = fs.readdirSync(phasesDir).find((name) => { - const m = name.match(/^(\d+)/); - return m && parseInt(m[1], 10) === wanted; - }); - if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); - fs.writeFileSync( - path.join(phasesDir, phaseDirName, `${phase}-VERIFICATION.md`), - ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), - ); - return runGsdTools(args, tmpDir); -} +// runVerifiedPhaseComplete is defined once, hoisted above both fold blocks (see +// the shared helper preceding the "Folded from tests/issue-2945-..." banner); +// this block closes over that module-scope definition. describe('phase complete checkbox rollback (#2945)', () => { let tmpDir; @@ -10668,33 +10682,11 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { createTempProject, cleanup } = require('./helpers.cjs'); -/** - * Write a passed-verification marker for a phase, then run `phase complete N`. - * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` - * with `status: passed` frontmatter, plus a SUMMARY for each plan (the completion gate - * requires executed plans). Requires the phase directory to already exist. - */ -function runVerifiedPhaseComplete(args, tmpDir) { - const argv = Array.isArray(args) ? args : args.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g); - const completeIdx = argv.findIndex((t, i) => t === 'complete' && argv[i - 1] === 'phase'); - const phase = argv[completeIdx + 1]; - const phasesDir = path.join(tmpDir, '.planning', 'phases'); - // Find the phase directory whose leading token matches the requested phase number. - const wantedPadded = String(phase).replace(/^0+/, ''); - const phaseDirName = fs.readdirSync(phasesDir).find((name) => { - const m = name.match(/^(\d+)/); - return m && parseInt(m[1], 10) === parseInt(wantedPadded, 10); - }); - if (!phaseDirName) throw new Error(`no phase directory for phase ${phase}`); - const phaseDir = path.join(phasesDir, phaseDirName); - fs.writeFileSync( - path.join(phaseDir, `${phase}-VERIFICATION.md`), - ['---', 'status: passed', '---', '', '# Verification', ''].join('\n'), - ); - return runGsdTools(args, tmpDir); -} +// runVerifiedPhaseComplete is defined once, in the outer scope shared by this block +// and the folded:issue-2945-phase-complete-checkbox-rollback block above (see the +// shared helper preceding that block's banner comment); this block closes over it. describe('phase complete stage-3 sentinel filter (#2949)', () => { let tmpDir; @@ -10854,3 +10846,4 @@ describe('phase complete stage-3 sentinel filter (#2949)', () => { }); }); } +} diff --git a/tests/review-lane-descriptor.test.cjs b/tests/review-lane-descriptor.test.cjs index 3f393b211..594555b31 100644 --- a/tests/review-lane-descriptor.test.cjs +++ b/tests/review-lane-descriptor.test.cjs @@ -815,6 +815,7 @@ describe('mergeReviewerLanes (#2927)', () => { assert.ok(slugs.includes('gemini'), 'first-party lanes preserved'); // the overlay body itself is the merged entry (no translation layer) const overlay = merged.find((l) => l.slug === 'agy-revisor'); + assert.ok(overlay, 'agy-revisor overlay lane should be present in merged set'); assert.equal(overlay.reviewsSection, 'Antigravity revisor-gsd'); assert.deepEqual(overlay.flags, ['--agy-revisor']); }); @@ -824,6 +825,7 @@ describe('mergeReviewerLanes (#2927)', () => { const colliding = overlayLane({ slug: 'claude', reviewsSection: 'EVIL CLAUDE' }); const merged = mergeReviewerLanes(FP, registry(reviewerCap(colliding))); const claude = merged.find((l) => l.slug === 'claude'); + assert.ok(claude, 'claude first-party lane should be present in merged set'); assert.equal(claude, FP.find((l) => l.slug === 'claude'), 'first-party identity wins'); assert.notEqual(claude.reviewsSection, 'EVIL CLAUDE', 'overlay did not leak through'); assert.equal(merged.length, FP.length, 'collision added no extra entry'); From 4036470ca0899cfd35c2946384be44e842b74350 Mon Sep 17 00:00:00 2001 From: sim Date: Wed, 12 Aug 2026 07:40:34 -0400 Subject: [PATCH 3/5] fix(#3339): consolidated helper silently overwrote an unrelated module-scope function The runVerifiedPhaseComplete(args, tmpDir) consolidation in the prior commit hoisted a `function` declaration into a bare (sloppy-mode) block. Annex B legacy hoisting semantics mean a block-scoped function declaration in sloppy mode also reassigns any enclosing var of the same name the instant the block executes -- and this file already had an unrelated module-scope runVerifiedPhaseComplete(args, tmpDir, env) at line 54, used by ~50 other call sites throughout the file. The block ran before any test() body did, so every one of those 50 call sites was silently pointed at the consolidated helper's different phase-matching logic (parseInt-based, loses dotted decimal sub-phase segments) instead of the real one (normalizePhaseToken/phaseTokenFromDirName-based), breaking multi-level decimal phases like 03.2.1. Fixed by wrapping the consolidated helper in a strict-mode IIFE, which disables Annex B hoisting and restores real block scoping. Caught by a genuine gsd-test failure (2 unique failures, both throw- class, both in phase-completion verification-gate behavior) -- root- caused via diff against the pre-consolidation commit and a scoped local node --test run confirming the fix, not asserted from a hunch. No test() count changed. No production code touched. --- tests/phase.test.cjs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index b596966a1..9f639b8a5 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -10495,7 +10495,18 @@ describe('#2572: phase complete warns when a SUMMARY claims files that never lan // Outer scope shared by the two fold blocks below (deliberately NOT module-scope: // this file already declares a distinct, 3-arg `runVerifiedPhaseComplete(args, tmpDir, env)` // at line 54 used throughout the rest of the file; nesting here avoids shadowing it). -{ +// +// IIFE, not a bare `{ }` block: this file has no top-level 'use strict', so a plain +// block is sloppy-mode and Annex B function-hoisting semantics apply — a `function` +// declared directly inside a bare block still leaks out and REASSIGNS the enclosing +// (module-scope) `runVerifiedPhaseComplete` var the moment this block runs, clobbering +// the real one at line 54 for every call site in the file (test() bodies are deferred +// and all run after this synchronous top-level code, so every caller ends up hitting +// this one). Wrapping in a strict-mode function expression suppresses Annex B leakage, +// matching how the two original un-consolidated copies were each scoped inside a +// strict-mode `describe(() => { 'use strict'; ... })` arrow function body. +(function () { +'use strict'; /** * Write a passed-VERIFICATION marker for the phase, then run `phase complete N`. * Mirrors phase.test.cjs's writePassedVerificationForPhase: a `-VERIFICATION.md` @@ -10846,4 +10857,4 @@ describe('phase complete stage-3 sentinel filter (#2949)', () => { }); }); } -} +})(); From c70e736c85a46dda9c948113d586b04b4a77267b Mon Sep 17 00:00:00 2001 From: sim Date: Wed, 12 Aug 2026 08:16:57 -0400 Subject: [PATCH 4/5] fix: eliminate resource-collision race in ship-notes-wedged-pr jq() mock Unrelated defect surfaced by a real gsd-test failure while verifying #3339 (test/3339-fold-state-model-profile diffs zero files this fix touches -- confirmed via `git diff origin/next -- ...` returning empty for this file, ship.md, and gsd-core/bin/lib/*.cjs). Fixed inline per this repo's no-defer policy for defects found while building, rather than deferred. The test's jq() mock shelled out to a full new Node.js process for every mocked `jq -r .field` call inside the extracted track_shipping bash script. The "exhausts the polling bound" test hits up to 24 cold Node spawns in one spawnSync call (6 polling iterations x 4 jq calls), racing a hardcoded 10s timeout -- under CI contention this tipped over on the linux-node22 lane specifically while linux-node24 had headroom, producing spawnSync's signal-killed `status: null` instead of the process's real exit code, asserted against the expected 0. Replaced the Node-subprocess jq() mock with a pure-shell sed -nE field extractor that never forks a process, eliminating the variable-cost operation rather than just widening the timeout. Verified extraction correctness against sample JSON (head/status/checks/review, including empty-string and non-matching-field cases). Locally verified (scoped per session policy): all 9 tests in the file pass; the previously- marginal test dropped from ~617ms to ~198ms. The underlying track_shipping polling logic in ship.md was confirmed correct and untouched -- this was a test-fixture race, not a product bug. --- tests/ship-notes-wedged-pr.test.cjs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/ship-notes-wedged-pr.test.cjs b/tests/ship-notes-wedged-pr.test.cjs index 17e35a5d6..915031b0a 100644 --- a/tests/ship-notes-wedged-pr.test.cjs +++ b/tests/ship-notes-wedged-pr.test.cjs @@ -62,7 +62,11 @@ function runTrackShipping(responses) { ' sed -n "${_call}p" "$GH_RESPONSES"', '}', 'jq() {', - ' "$NODE_BIN" -e "let d=\'\';process.stdin.on(\'data\',c=>d+=c);process.stdin.on(\'end\',()=>{const o=JSON.parse(d);process.stdout.write(String(o[process.argv[1].slice(1)] ?? \'\'));});" "$2"', + ' _field="${2#.}"', + ' _raw=$(sed -nE \'s/.*"\'"$_field"\'":("[^"]*"|[^,}]*).*/\\1/p\')', + ' _raw="${_raw%\\"}"', + ' _raw="${_raw#\\"}"', + ' printf \'%s\' "$_raw"', '}', ].join('\n'); @@ -77,7 +81,6 @@ function runTrackShipping(responses) { GH_CALLS: ghCallsPath, GH_RESPONSES: responsesPath, GIT_CALLS: gitCallsPath, - NODE_BIN: process.execPath, PHASE_NUMBER: '1', PR_NUMBER: '123', padded_phase: '01', From 23e26a9de80a73696b09b8cd9712c6c1dae1f8dd Mon Sep 17 00:00:00 2001 From: sim Date: Wed, 12 Aug 2026 08:28:17 -0400 Subject: [PATCH 5/5] chore(#3339): extend BUG_FILE_RE to fix-/issue- prefixes, close H3 (#3315) Widens the identity ratchet in scripts/lint-regression-test-names.cjs from banning only new bug-NNNN-*.test.cjs files to also banning fix-NNNN-*.test.cjs and issue-NNNN-*.test.cjs, per epic #3053's own recorded decision: extend only after the 74-file fix-*/issue-* backlog fully drains, never before. That precondition is now real, not assumed: this branch is rebased onto origin/next with Waves 1-6 (#3341, #3342, #3373, #3376, #3378, #3383) all merged, and a repo-wide check confirms zero tests/fix-*.test.cjs or tests/issue--*.test.cjs files remain -- only the two known false matches (issue-dedupe.test.cjs, issue-version-gate.test.cjs, feature-named suites with no digits after the prefix) survive the glob, and the widened regex correctly excludes both. Allowlist snapshotted at zero (scripts/lint-regression-test-names.allowlist.json was already [] and stays [] -- confirmed via --update reporting "already in sync"). Updated docs/TESTING-SUITES.md's Regression tests section to name fix-/issue- alongside bug- as banned new-file prefixes. This is the last commit of H3 (#3315)'s 7-wave decomposition. --- docs/TESTING-SUITES.md | 13 +++++++----- scripts/lint-regression-test-names.cjs | 28 ++++++++++++++------------ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index d4fce6d09..d11fa580f 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -33,7 +33,8 @@ The suite-suffix convention was chosen over a directory layout (`tests/security/ ## Regression tests -**Do not create new top-level `tests/bug-NNNN-*.test.cjs` files.** Add the +**Do not create new top-level `tests/bug-NNNN-*.test.cjs`, +`tests/fix-NNNN-*.test.cjs`, or `tests/issue-NNNN-*.test.cjs` files.** Add the regression case to the owning module's main test file instead (e.g. a `describe('regressions')` block in `tests/.test.cjs`). @@ -42,14 +43,16 @@ count — is the unit of CI overhead, and it is worst on Windows lanes where every spawn is Defender-scanned. The 2026-06 CI audit found 244 one-off `bug-*` files (~38% of the suite). That population is grandfathered in `scripts/lint-regression-test-names.allowlist.json` and enforced by an -identity ratchet (`npm run lint:regression-names`, part of `npm run lint:ci`): +identity ratchet (`npm run lint:regression-names`, part of `npm run lint:ci`), +which also bans `fix-*` and `issue-*` NNNN-prefixed files the same way: -- A **new** `bug-*` file fails CI — fold it into the owning module's file. +- A **new** `bug-*`, `fix-*`, or `issue-*` NNNN-prefixed file fails CI — fold + it into the owning module's file. - **Deleting/consolidating** a grandfathered file requires pruning its allowlist entry, so the baseline only ever shrinks. - **Inherited drift** (the failure names files your PR didn't add — e.g. the - base branch merged `bug-*` files without feeding the allowlist, or you - rebased and carried a pre-rebase allowlist): run + base branch merged `bug-*`/`fix-*`/`issue-*` files without feeding the + allowlist, or you rebased and carried a pre-rebase allowlist): run `node scripts/lint-regression-test-names.cjs --update` and commit the regenerated allowlist. Snapshot artifacts like this allowlist (and `docs/INVENTORY.md`) must be regenerated **after** rebasing, never carried diff --git a/scripts/lint-regression-test-names.cjs b/scripts/lint-regression-test-names.cjs index f75293f99..715ebd69d 100644 --- a/scripts/lint-regression-test-names.cjs +++ b/scripts/lint-regression-test-names.cjs @@ -2,7 +2,7 @@ 'use strict'; /** - * lint-regression-test-names.cjs — ban NEW top-level bug-NNNN test files. + * lint-regression-test-names.cjs — ban NEW top-level bug-NNNN/fix-NNNN/issue-NNNN test files. * * ## Why * @@ -17,20 +17,22 @@ * ## What this enforces * * Identity ratchet (scripts/lib/allowlist-ratchet.cjs) over basenames matching - * /^bug-\d+.*\.test\.cjs$/ in tests/: - * - A NEW bug-* file (not in the allowlist) fails: fold the regression into - * the owning module's test file instead. - * - A REMOVED bug-* file with a stale allowlist entry also fails: prune the - * entry from scripts/lint-regression-test-names.allowlist.json so the - * baseline only ever shrinks. + * /^(?:bug|fix|issue)-\d+.*\.test\.cjs$/ in tests/: + * - A NEW bug-*, fix-*, or issue-* file (not in the allowlist) fails: fold + * the regression into the owning module's test file instead. + * - A REMOVED bug-*, fix-*, or issue-* file with a stale allowlist entry + * also fails: prune the entry from + * scripts/lint-regression-test-names.allowlist.json so the baseline only + * ever shrinks. * * ## --update (allowlist drift repair) * * `node scripts/lint-regression-test-names.cjs --update` regenerates the * allowlist from the files currently in tests/ and reports what changed. * Use it when the failure is INHERITED drift, not your own new file — e.g. - * the base branch merged bug-* files without feeding the allowlist (the - * #947/#948/#950 race after the ratchet landed), or after a rebase. The + * the base branch merged bug-*, fix-*, or issue-* files without feeding the + * allowlist (the #947/#948/#950 race after the ratchet landed), or after a + * rebase. The * allowlist is a snapshot artifact: regenerate it AFTER rebasing, never * carry a pre-rebase copy through. * @@ -49,7 +51,7 @@ const ALLOWLIST_PATH = process.env.GSD_LINT_REGRESSION_ALLOWLIST || path.join(__dirname, 'lint-regression-test-names.allowlist.json'); -const BUG_FILE_RE = /^bug-\d+.*\.test\.cjs$/; +const BUG_FILE_RE = /^(?:bug|fix|issue)-\d+.*\.test\.cjs$/; function main() { const args = process.argv.slice(2); @@ -97,8 +99,8 @@ function main() { for (const msg of failures) console.error(msg); if (novel.length > 0) { console.error( - '\nIf this PR added the file(s) above: new bug-NNNN test files are not ' + - "accepted — add the regression case to the owning module's test file " + + '\nIf this PR added the file(s) above: new bug-NNNN/fix-NNNN/issue-NNNN ' + + "test files are not accepted — add the regression case to the owning module's test file " + "(e.g. a describe('regressions') block in tests/.test.cjs) instead.\n" + 'If the file(s) came from the base branch (inherited allowlist drift, ' + 'e.g. after a rebase): run ' + @@ -110,7 +112,7 @@ function main() { } console.log( - `ok lint-regression-test-names: ${current.length} grandfathered bug-* file(s), no novel offenders` + `ok lint-regression-test-names: ${current.length} grandfathered bug-*/fix-*/issue-* file(s), no novel offenders` ); }