From 74b81379cf77b0d6593d552990376396ff1d20cc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 29 Apr 2026 08:00:17 -0400 Subject: [PATCH] fix(#2836): audit-open quick SUMMARY filename + UAT terminal-status drift (#2847) * fix(#2836): audit-open quick SUMMARY filename + UAT terminal-status drift Fixes two convention drifts in bin/lib/audit.cjs that produced false-positive "open" items at every milestone close: 1. scanQuickTasks: looked for bare `SUMMARY.md`, but workflows/quick.md mandates `${quick_id}-SUMMARY.md`. Now matches either filename so quick tasks created via the documented workflow are recognized. 2. scanUatGaps: only treated `status: complete` as terminal, but workflows/execute-phase.md uses `status: resolved` post-gap-closure. Now treats both `complete` and `resolved` as terminal, with `result: all_pass` as a fallback when status is absent. Also reconciles workflows/help.md one-liner that referenced bare `SUMMARY.md` so docs match the authoritative quick.md workflow. Adds tests/bug-2836-audit-open-summary-uat-drift.test.cjs with 6 structural regression tests covering both fixes plus no-regression cases. * refactor(#2836): hoist TERMINAL_UAT_STATUSES outside scanUatGaps loop Address CodeRabbit nitpick: the Set was being recreated on each UAT file iteration. Hoist to module scope so it is constructed once. Co-Authored-By: Claude Opus 4.7 --------- Co-authored-by: Claude Opus 4.7 --- get-shit-done/bin/lib/audit.cjs | 30 ++- get-shit-done/workflows/help.md | 2 +- ...2836-audit-open-summary-uat-drift.test.cjs | 183 ++++++++++++++++++ 3 files changed, 211 insertions(+), 4 deletions(-) create mode 100644 tests/bug-2836-audit-open-summary-uat-drift.test.cjs diff --git a/get-shit-done/bin/lib/audit.cjs b/get-shit-done/bin/lib/audit.cjs index 1ef751eae..746028993 100644 --- a/get-shit-done/bin/lib/audit.cjs +++ b/get-shit-done/bin/lib/audit.cjs @@ -105,12 +105,27 @@ function scanQuickTasks(planDir) { continue; } - const summaryPath = path.join(safeTaskDir, 'SUMMARY.md'); + // workflows/quick.md mandates `${quick_id}-SUMMARY.md`; older flows used + // bare `SUMMARY.md`. Accept either to avoid false-positive "missing". + let summaryPath = null; + try { + const summaryFiles = fs.readdirSync(safeTaskDir, { withFileTypes: true }) + .filter(e => e.isFile() && (e.name === 'SUMMARY.md' || e.name.endsWith('-SUMMARY.md'))); + if (summaryFiles.length > 0) { + // Prefer the per-task `${quick_id}-SUMMARY.md` form when present. + const preferred = summaryFiles.find(e => e.name === `${dirName}-SUMMARY.md`) + || summaryFiles.find(e => e.name.endsWith('-SUMMARY.md')) + || summaryFiles[0]; + summaryPath = path.join(safeTaskDir, preferred.name); + } + } catch { + // fall through with summaryPath = null → status: missing + } let status = 'missing'; let description = ''; - if (fs.existsSync(summaryPath)) { + if (summaryPath && fs.existsSync(summaryPath)) { let safeSum; try { safeSum = requireSafePath(summaryPath, planDir, 'quick task summary', { allowAbsolute: true }); @@ -344,6 +359,11 @@ function scanSeeds(planDir) { return results; } +// 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. +const TERMINAL_UAT_STATUSES = new Set(['complete', 'resolved']); + /** * Scan .planning/phases for UAT gaps (UAT files with status != 'complete'). */ @@ -394,8 +414,12 @@ function scanUatGaps(planDir) { const fm = extractFrontmatter(content); const status = (fm.status || 'unknown').toLowerCase(); + const result = (fm.result || '').toString().toLowerCase(); - if (status === 'complete') continue; + // Also accept `result: all_pass` as a fallback when status is absent + // — covers UATs that omit `status:`. + if (TERMINAL_UAT_STATUSES.has(status)) continue; + if (status === 'unknown' && result === 'all_pass') continue; // Count open scenarios const pendingMatches = (content.match(/result:\s*(?:pending|\[pending\])/gi) || []).length; diff --git a/get-shit-done/workflows/help.md b/get-shit-done/workflows/help.md index 8ce8f620a..30d3207dc 100644 --- a/get-shit-done/workflows/help.md +++ b/get-shit-done/workflows/help.md @@ -153,7 +153,7 @@ Granular flags are composable: `--discuss --research --validate` gives the same Usage: `/gsd-quick` Usage: `/gsd-quick --full` Usage: `/gsd-quick --research --validate` -Result: Creates `.planning/quick/NNN-slug/PLAN.md`, `.planning/quick/NNN-slug/SUMMARY.md` +Result: Creates `.planning/quick/NNN-slug/PLAN.md`, `.planning/quick/NNN-slug/NNN-slug-SUMMARY.md` --- diff --git a/tests/bug-2836-audit-open-summary-uat-drift.test.cjs b/tests/bug-2836-audit-open-summary-uat-drift.test.cjs new file mode 100644 index 000000000..93c2b5919 --- /dev/null +++ b/tests/bug-2836-audit-open-summary-uat-drift.test.cjs @@ -0,0 +1,183 @@ +/** + * Regression tests for bug #2836 + * + * audit-open had two convention drifts vs the documented workflows: + * 1. quick-task scanner looked for bare `SUMMARY.md`, but workflows/quick.md + * mandates `${quick_id}-SUMMARY.md`. Result: every documented quick task + * reported as `status: missing`. + * 2. UAT terminal-status enum only accepted `complete`, but + * workflows/execute-phase.md uses `resolved` post-gap-closure. + * Result: gap-closed UATs reported as open. + * + * Tests structurally invoke auditOpenArtifacts() against real fixtures on disk + * and assert the returned items array — never regex on raw file content. + */ + +'use strict'; + +const { describe, test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); + +const auditModule = require('../get-shit-done/bin/lib/audit.cjs'); +const { auditOpenArtifacts } = auditModule; + +function mkTmp() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'bug-2836-')); +} + +function rmTmp(dir) { + try { fs.rmSync(dir, { recursive: true, force: true }); } catch {} +} + +describe('bug #2836: audit-open quick-task summary filename + UAT terminal status', () => { + // Ensure GSD env vars do not redirect planningDir() away from our fixture. + let prevProject, prevWorkstream; + before(() => { + prevProject = process.env.GSD_PROJECT; + prevWorkstream = process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + delete process.env.GSD_WORKSTREAM; + }); + after(() => { + if (prevProject !== undefined) process.env.GSD_PROJECT = prevProject; + if (prevWorkstream !== undefined) process.env.GSD_WORKSTREAM = prevWorkstream; + }); + + test('quick task with ${quick_id}-SUMMARY.md is recognized as complete (not missing)', () => { + const cwd = mkTmp(); + try { + const quickId = '260429-test-foo'; + const taskDir = path.join(cwd, '.planning', 'quick', quickId); + fs.mkdirSync(taskDir, { recursive: true }); + fs.writeFileSync( + path.join(taskDir, `${quickId}-SUMMARY.md`), + '---\nstatus: complete\n---\ntest summary\n', + 'utf-8' + ); + + const result = auditOpenArtifacts(cwd); + const realQuickTasks = result.items.quick_tasks.filter( + i => !i.scan_error && !i._remainder_count + ); + + assert.equal( + realQuickTasks.length, 0, + `quick task with ${quickId}-SUMMARY.md (status: complete) must not appear ` + + `as an open item; got: ${JSON.stringify(realQuickTasks)}` + ); + assert.equal(result.counts.quick_tasks, 0); + } finally { + rmTmp(cwd); + } + }); + + test('UAT with status: resolved is treated as terminal (not an open gap)', () => { + const cwd = mkTmp(); + try { + const phaseDir = path.join(cwd, '.planning', 'phases', '01-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync( + path.join(phaseDir, '01-UAT.md'), + '---\nstatus: resolved\n---\nUAT body — gap closed via execute-phase flow.\n', + 'utf-8' + ); + + const result = auditOpenArtifacts(cwd); + const realUatGaps = result.items.uat_gaps.filter(i => !i.scan_error); + + assert.equal( + realUatGaps.length, 0, + `UAT with status: resolved must not appear as an open gap; ` + + `got: ${JSON.stringify(realUatGaps)}` + ); + assert.equal(result.counts.uat_gaps, 0); + } finally { + rmTmp(cwd); + } + }); + + test('UAT with status: complete remains terminal (no regression)', () => { + const cwd = mkTmp(); + try { + const phaseDir = path.join(cwd, '.planning', 'phases', '02-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync( + path.join(phaseDir, '02-UAT.md'), + '---\nstatus: complete\n---\nUAT body.\n', + 'utf-8' + ); + + const result = auditOpenArtifacts(cwd); + const realUatGaps = result.items.uat_gaps.filter(i => !i.scan_error); + assert.equal(realUatGaps.length, 0); + } finally { + rmTmp(cwd); + } + }); + + test('UAT with status: pending is still flagged as an open gap', () => { + const cwd = mkTmp(); + try { + const phaseDir = path.join(cwd, '.planning', 'phases', '03-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync( + path.join(phaseDir, '03-UAT.md'), + '---\nstatus: pending\n---\nresult: pending\n', + 'utf-8' + ); + + const result = auditOpenArtifacts(cwd); + const realUatGaps = result.items.uat_gaps.filter(i => !i.scan_error); + assert.equal(realUatGaps.length, 1, 'pending UAT must still be flagged'); + assert.equal(realUatGaps[0].status, 'pending'); + } finally { + rmTmp(cwd); + } + }); + + test('quick task without any SUMMARY file is still flagged as missing', () => { + const cwd = mkTmp(); + try { + const quickId = '260429-test-bar'; + const taskDir = path.join(cwd, '.planning', 'quick', quickId); + fs.mkdirSync(taskDir, { recursive: true }); + // No SUMMARY file at all. + + const result = auditOpenArtifacts(cwd); + const realQuickTasks = result.items.quick_tasks.filter( + i => !i.scan_error && !i._remainder_count + ); + assert.equal(realQuickTasks.length, 1); + assert.equal(realQuickTasks[0].status, 'missing'); + } finally { + rmTmp(cwd); + } + }); +}); + +describe('bug #2836: workflows/help.md one-liner reconciliation', () => { + test('help.md quick-task one-liner uses ${quick_id}-SUMMARY.md pattern', () => { + const helpPath = path.resolve( + __dirname, '..', 'get-shit-done', 'workflows', 'help.md' + ); + const content = fs.readFileSync(helpPath, 'utf-8'); + + // Locate the documented "Result: Creates ..." quick-task one-liner and + // assert it references the per-task SUMMARY filename pattern, not bare + // SUMMARY.md. We parse by line to avoid false positives elsewhere. + const resultLines = content.split('\n').filter(l => + l.includes('Result: Creates') && l.includes('.planning/quick/') + ); + assert.ok(resultLines.length > 0, 'expected a quick-task Result line in help.md'); + for (const line of resultLines) { + assert.ok( + /\$\{quick_id\}-SUMMARY\.md|NNN-slug-SUMMARY\.md/.test(line), + `help.md quick-task Result line must reference per-task SUMMARY filename ` + + `(e.g. \${quick_id}-SUMMARY.md); got: ${line}` + ); + } + }); +});