diff --git a/.changeset/deferred-items-reach-milestone-close.md b/.changeset/deferred-items-reach-milestone-close.md new file mode 100644 index 000000000..0edc0d7b7 --- /dev/null +++ b/.changeset/deferred-items-reach-milestone-close.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 2983 +--- +**Unresolved `deferred-items.md` entries now reach the milestone-close audit.** `auditOpenArtifacts` gains `deferred_items` as a ninth scanned category, so an out-of-scope discovery a phase agent correctly recorded rather than fixed surfaces in `/gsd-complete-milestone`'s pre-close report alongside the other eight, and the existing `[R] Resolve / [A] Acknowledge / [C] Cancel` prompt applies to it. #2287 made the file readable at the phase boundary (`audit-uat`, `/gsd-progress` check 7); one boundary up it was still invisible, and phase directories archive to `milestones/vX.Y-phases/` by default (#1871), so an unresolved entry left the live tree without ever being triaged. The resolved/unresolved predicate is not reimplemented — the scanner lazily requires `uat.cjs`'s exported `parseDeferredItems`, so both boundaries agree by construction about what "open" means. **Behaviour change worth noting before you upgrade:** a project carrying unresolved deferred items will now see the `[R]/[A]/[C]` prompt at milestone close where close previously proceeded silently. That is the intended correction, but it surfaces pre-existing debt on the first run. (#2646) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 26d7ec734..075e0b52f 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -454,6 +454,24 @@ Archive milestone, tag release. /gsd-complete-milestone ``` +**Pre-close artifact audit.** Before archiving, the workflow runs `gsd-tools audit-open` and reports every unresolved item across nine categories: + +| Category | Source | Open when | +|----------|--------|-----------| +| Debug sessions | `.planning/debug/` | status not `resolved` / `complete` | +| Quick tasks | `.planning/quick/` | SUMMARY missing or not `complete` | +| Threads | `.planning/threads/` | status not terminal | +| Pending todos | `.planning/todos/pending/` | present | +| Seeds | `.planning/seeds/` | not yet implemented | +| UAT gaps | `*-UAT.md` | scenarios still pending | +| Verification gaps | `*-VERIFICATION.md` | verdict `gaps_found` / `human_needed` | +| CONTEXT questions | `*-CONTEXT.md` | questions left open | +| **Deferred items** | `deferred-items.md` | entry lacks `status: resolved` | + +If any category is non-empty you are prompted with `[R] Resolve` / `[A] Acknowledge all` / `[C] Cancel`. `[A]` records the items to `STATE.md` under its own `## Deferred Items` heading and closes as `override_closeout`; an all-clear closes as `verified_closeout`. + +> **Note:** the `deferred-items.md` category is the per-phase SCOPE BOUNDARY log a phase agent writes when it finds a defect it should not fix. It is a different artifact from the `## Deferred Items` section `[A]` writes into `STATE.md`, which records what you acknowledged at close. + --- ### `/gsd-milestone-summary` diff --git a/src/audit.cts b/src/audit.cts index 6f0f2a539..c1007d292 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -92,6 +92,22 @@ interface ContextQuestionItem { scan_error?: boolean; } +interface DeferredItem { + phase: string; + file: string; + text: string; + scan_error?: boolean; +} + +/** + * Minimal structural view of `uat.cjs` — only the export `scanDeferredItems` + * lazily requires. Mirrors the local-interface convention in + * `audit-command-router.cts`, which types its lazy requires the same way. + */ +interface UatDeferredModule { + parseDeferredItems(content: string): Array<{ name: string }>; +} + interface AuditCounts { debug_sessions: number; quick_tasks: number; @@ -101,6 +117,7 @@ interface AuditCounts { uat_gaps: number; verification_gaps: number; context_questions: number; + deferred_items: number; total: number; } @@ -117,9 +134,14 @@ interface AuditResult { uat_gaps: UatGapItem[]; verification_gaps: VerificationGapItem[]; context_questions: ContextQuestionItem[]; + deferred_items: DeferredItem[]; }; } +// The SCOPE BOUNDARY convention's filename (`agents/gsd-executor.md`), shared +// verbatim with the #2287 phase-boundary reader in `uat.cts`. +const DEFERRED_ITEMS_FILENAME = 'deferred-items.md'; + // Terminal UAT states: `complete` (legacy) and `resolved` (post-gap-closure // per workflows/execute-phase.md). Hoisted outside scanUatGaps so the Set is // not recreated on each loop iteration. @@ -680,6 +702,78 @@ function scanContextQuestions(planDir: string): ContextQuestionItem[] { return results; } +// ─── scanDeferredItems ──────────────────────────────────────────────────────── + +/** + * Scan phase directories for UNRESOLVED entries in `deferred-items.md` (#2646). + * + * The SCOPE BOUNDARY convention (`agents/gsd-executor.md`) has a phase agent + * log an out-of-scope discovery here rather than fix it. #2287 made that file + * readable at the PHASE boundary (`/gsd-progress` check 7, `audit-uat`); this + * scanner closes the remaining reader gap one boundary up, so an entry still + * unresolved at MILESTONE close surfaces in the pre-close audit alongside the + * other eight categories and the existing `[R]/[A]/[C]` prompt applies to it. + * Without this, phase directories archive to `milestones/vX.Y-phases/` (#1871) + * and the entry leaves the live tree having never been triaged. + * + * The resolved/unresolved predicate is NOT reimplemented here: `uat.cjs` + * already exports `parseDeferredItems`, which owns the parsing rule (entries + * under a `## Deferred Items` level-2 heading, else the whole file fail-safe; + * RESOLVED only on an explicit case-insensitive `status: resolved` field). + * Duplicating that inequality is how two readers of the same file drift into + * disagreeing about what "open" means. The require is deliberately LAZY, + * inside the scan, to preserve `audit-command-router.cts`'s property that a + * route never loads the module it does not need. + */ +function scanDeferredItems(planDir: string): DeferredItem[] { + const phasesDir = path.join(planDir, 'phases'); + if (!fs.existsSync(phasesDir)) return []; + + let dirs: string[]; + try { + dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) + .filter(e => e.isDirectory()) + .map(e => e.name) + .sort(); + } catch { + return [{ scan_error: true, phase: '', file: '', text: '' }]; + } + + // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment + const uat: UatDeferredModule = require('./uat.cjs'); + + const results: DeferredItem[] = []; + + for (const dir of dirs) { + const phaseDir = path.join(phasesDir, dir); + const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); + const phaseNum = phaseMatch ? phaseMatch[1] : dir; + + const filePath = path.join(phaseDir, DEFERRED_ITEMS_FILENAME); + if (!fs.existsSync(filePath)) continue; + + let safeFilePath: string; + try { + safeFilePath = requireSafePath(filePath, planDir, 'deferred items file', { allowAbsolute: true }); + } catch { + continue; + } + + const content = platformReadSync(safeFilePath); + if (content === null) continue; + + for (const item of uat.parseDeferredItems(content)) { + results.push({ + phase: sanitizeForDisplay(phaseNum), + file: DEFERRED_ITEMS_FILENAME, + text: sanitizeForDisplay(item.name), + }); + } + } + + return results; +} + // ─── auditOpenArtifacts ─────────────────────────────────────────────────────── /** @@ -723,6 +817,10 @@ function auditOpenArtifacts(cwd: string): AuditResult { try { return scanContextQuestions(planDir); } catch { return [{ scan_error: true, phase: '', file: '', question_count: 0, questions: [] }]; } })(); + const deferredItems = (() => { + try { return scanDeferredItems(planDir); } catch { return [{ scan_error: true, phase: '', file: '', text: '' }]; } + })(); + // Count real items (not scan_error sentinels) const countReal = (arr: Array<{ scan_error?: boolean; _remainder_count?: number }>) => arr.filter(i => !i.scan_error && !i._remainder_count).length; @@ -736,9 +834,10 @@ function auditOpenArtifacts(cwd: string): AuditResult { uat_gaps: countReal(uatGaps), verification_gaps: countReal(verificationGaps), context_questions: countReal(contextQuestions), + deferred_items: countReal(deferredItems), total: 0, }; - counts.total = counts.debug_sessions + counts.quick_tasks + counts.threads + counts.todos + counts.seeds + counts.uat_gaps + counts.verification_gaps + counts.context_questions; + counts.total = counts.debug_sessions + counts.quick_tasks + counts.threads + counts.todos + counts.seeds + counts.uat_gaps + counts.verification_gaps + counts.context_questions + counts.deferred_items; return { scanned_at: new Date().toISOString(), @@ -753,6 +852,7 @@ function auditOpenArtifacts(cwd: string): AuditResult { uat_gaps: uatGaps, verification_gaps: verificationGaps, context_questions: contextQuestions, + deferred_items: deferredItems, }, }; } @@ -869,6 +969,16 @@ function formatAuditReport(auditResult: AuditResult): string { } } + // Deferred items (deferred decisions — blue). Out-of-scope discoveries a + // phase agent recorded rather than fixed, still unresolved at close (#2646). + if (counts.deferred_items > 0) { + lines.push(''); + lines.push(`🔵 Deferred Items (${counts.deferred_items} unresolved)`); + for (const item of items.deferred_items.filter(i => !i.scan_error)) { + lines.push(` • Phase ${item.phase}: ${item.text}`); + } + } + lines.push(''); lines.push(hr); lines.push(` ${counts.total} item${counts.total !== 1 ? 's' : ''} require decisions before close.`); diff --git a/tests/audit-command-cutover.test.cjs b/tests/audit-command-cutover.test.cjs index ff130d0c9..755d338a7 100644 --- a/tests/audit-command-cutover.test.cjs +++ b/tests/audit-command-cutover.test.cjs @@ -569,7 +569,8 @@ describe('audit-open — output shape (#2911)', () => { const expectedCountKeys = [ 'debug_sessions', 'quick_tasks', 'threads', 'todos', - 'seeds', 'uat_gaps', 'verification_gaps', 'context_questions', 'total', + 'seeds', 'uat_gaps', 'verification_gaps', 'context_questions', + 'deferred_items', 'total', ]; for (const key of expectedCountKeys) { assert.equal( @@ -581,6 +582,7 @@ describe('audit-open — output shape (#2911)', () => { const expectedItemKeys = [ 'debug_sessions', 'quick_tasks', 'threads', 'todos', 'seeds', 'uat_gaps', 'verification_gaps', 'context_questions', + 'deferred_items', ]; for (const key of expectedItemKeys) { assert.ok( diff --git a/tests/feat-2646-deferred-items-audit-scanner.test.cjs b/tests/feat-2646-deferred-items-audit-scanner.test.cjs new file mode 100644 index 000000000..c1095ac97 --- /dev/null +++ b/tests/feat-2646-deferred-items-audit-scanner.test.cjs @@ -0,0 +1,186 @@ +/** + * #2646 (c) — `deferred-items.md` has no reader at MILESTONE close. + * + * `auditOpenArtifacts` (src/audit.cts) is the pre-close gate behind + * `/gsd-complete-milestone`. It scanned eight categories — debug sessions, + * quick tasks, threads, todos, seeds, UAT gaps, verification gaps, context + * questions — and `deferred-items.md` was not among them. + * + * #2287 made that file readable at the PHASE boundary (`audit-uat`, + * `/gsd-progress` check 7). One boundary up it was still invisible, and phase + * directories archive to `milestones/vX.Y-phases/` by default (#1871) — so an + * out-of-scope discovery a phase agent correctly recorded rather than fixed + * left the live tree at milestone close without ever reaching the existing + * `[R] Resolve / [A] Acknowledge / [C] Cancel` prompt. + * + * This adds `deferred_items` as a ninth scanner, its count, its `items` entry + * and its report section. The resolved/unresolved predicate is NOT + * reimplemented — the scanner lazily requires `uat.cjs`'s exported + * `parseDeferredItems`, so both boundaries agree by construction about what + * "open" means. + */ + +'use strict'; + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +/** Write a phase-directory deferred-items.md and return its phase dir. */ +function writeDeferred(tmpDir, phaseDirName, lines) { + const phaseDir = path.join(tmpDir, '.planning', 'phases', phaseDirName); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, 'deferred-items.md'), lines.join('\n') + '\n'); + return phaseDir; +} + +function auditJson(tmpDir) { + const result = runGsdTools('audit-open --json', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + return JSON.parse(result.output); +} + +describe('#2646 auditOpenArtifacts: deferred-items.md is a scanned category', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('the category exists in the contract even when empty', () => { + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 0); + assert.deepStrictEqual(out.items.deferred_items, []); + }); + + test('no deferred-items.md in a phase dir → no false positive', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '.gitkeep'), ''); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 0); + assert.strictEqual(out.has_open_items, false); + }); + + test('an unresolved entry is surfaced with its phase and counted', () => { + writeDeferred(tmpDir, '01-foundation', [ + '## Deferred Items', + '', + '- getUserById() has a pre-existing N+1 in the members join.', + ]); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 1); + assert.strictEqual(out.items.deferred_items.length, 1); + + const [item] = out.items.deferred_items; + assert.strictEqual(item.phase, '01'); + assert.strictEqual(item.file, 'deferred-items.md'); + assert.match(item.text, /pre-existing N\+1/); + }); + + test('an entry marked `status: resolved` is NOT surfaced', () => { + writeDeferred(tmpDir, '01-foundation', [ + '## Deferred Items', + '', + '- Already handled unrelated lint warning.', + ' status: resolved', + ]); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 0); + }); + + test('resolved and unresolved entries in one file → only the unresolved surfaces', () => { + writeDeferred(tmpDir, '02-api', [ + '## Deferred Items', + '', + '- Fixed already.', + ' status: resolved', + '- Still open: config loader swallows a parse error.', + ]); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 1); + assert.match(out.items.deferred_items[0].text, /swallows a parse error/); + }); + + test('a headless deferred-items.md still surfaces (fail-safe, inherited from #2287)', () => { + writeDeferred(tmpDir, '03-ui', [ + '- No level-2 heading here, but this is still a real discovery.', + ]); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 1); + }); + + test('entries across multiple phases are all surfaced', () => { + writeDeferred(tmpDir, '01-foundation', ['## Deferred Items', '', '- One.']); + writeDeferred(tmpDir, '02-api', ['## Deferred Items', '', '- Two.', '- Three.']); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.counts.deferred_items, 3); + assert.deepStrictEqual( + [...new Set(out.items.deferred_items.map(i => i.phase))].sort(), + ['01', '02'], + ); + }); + + test('deferred items contribute to counts.total and flip has_open_items', () => { + writeDeferred(tmpDir, '01-foundation', ['## Deferred Items', '', '- A real discovery.']); + + const out = auditJson(tmpDir); + + assert.strictEqual(out.has_open_items, true); + assert.strictEqual(out.counts.total, 1); + }); +}); + +describe('#2646 formatAuditReport: deferred items render in the report', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('the section and its entry appear in the human-readable report', () => { + writeDeferred(tmpDir, '01-foundation', [ + '## Deferred Items', + '', + '- getUserById() has a pre-existing N+1 in the members join.', + ]); + + const result = runGsdTools('audit-open', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + assert.match(result.output, /Deferred Items \(1 unresolved\)/); + assert.match(result.output, /Phase 01: getUserById\(\) has a pre-existing N\+1/); + assert.match(result.output, /1 item requires? decisions? before close\./); + }); + + test('a clean tree still reports all-clear (no empty section emitted)', () => { + const result = runGsdTools('audit-open', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + assert.doesNotMatch(result.output, /Deferred Items/); + assert.match(result.output, /All artifact types clear/); + }); +});