diff --git a/.changeset/curious-cats-march.md b/.changeset/curious-cats-march.md new file mode 100644 index 000000000..9b412c2dd --- /dev/null +++ b/.changeset/curious-cats-march.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4022 +--- +**`audit-uat` sees workstream phases again** — the audit now enumerates all three phase-archive layouts (flat `milestones/vX.Y-phases/`, archived workstream `milestones/ws-*/phases/`, and active workstream `workstreams//milestones/`), with workstream entries labeled `/` so acknowledge-by-milestone stays unambiguous. A project using workstreams no longer gets an All Clear audit while items are open, and `--ws` no longer empties the report. Phase lookups keep their #2855 workstream scoping unchanged. (#3804) diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 61d902450..85f7d9ef8 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -255,9 +255,10 @@ "files": [ "audit-command-cutover.test.cjs", "audit-fix-command.test.cjs", - "audit-milestone-filename-guard.test.cjs" + "audit-milestone-filename-guard.test.cjs", + "audit-workstream-layouts.test.cjs" ], - "issue": "#3796 \u2014 the third file is a structural guard over the shipped audit-milestone workflow text (writer/reader filename agreement), not a behavior suite of the audit module; consolidation into the CLI suites would put a source-text scan behind spawn-heavy fixtures." + "issue": "#3804 \u2014 the layout tests drive the real audit-uat CLI against on-disk workstream fixtures; the existing audit suites cover the CLI cutover and the filename contract, and this covers phase-directory enumeration" } } } diff --git a/src/audit.cts b/src/audit.cts index 5fe6de592..a320ab4d7 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -31,7 +31,7 @@ import phaseIdMod = require('./phase-id.cjs'); const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocator = require('./phase-locator.cjs'); -const { getArchivedPhaseDirs } = phaseLocator; +const { getAllArchivedPhaseDirs } = phaseLocator; import { requireSafePath, sanitizeForDisplay, sanitizeLabel } from './security.cjs'; import { platformWriteSync } from './shell-command-projection.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -956,8 +956,11 @@ function listAuditPhaseTargets(planDir: string, cwd: string): { targets: AuditPh } } + // #3804: the audit is cross-workstream by design — the shared + // getAllArchivedPhaseDirs helper (root + every workstream, distinct + // '/' labels) owns that walk. try { - for (const archived of getArchivedPhaseDirs(cwd)) { + for (const archived of getAllArchivedPhaseDirs(cwd)) { targets.push({ dir: archived.name, fullPath: archived.fullPath, milestone: archived.milestone }); } } catch { diff --git a/src/phase-locator.cts b/src/phase-locator.cts index 6c2ea2533..08ac3540b 100644 --- a/src/phase-locator.cts +++ b/src/phase-locator.cts @@ -26,7 +26,7 @@ import coreUtilsModule = require('./core-utils.cjs'); const { readSubdirectories, getPhaseFileStats, extractCanonicalPlanId, toPosixPath, findUnsummarizedPlans } = coreUtilsModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); -const { planningDir } = planningWorkspace; +const { planningDir, planningRoot } = planningWorkspace; // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatterModule = require('./frontmatter.cjs'); const { extractFrontmatter } = frontmatterModule; @@ -142,23 +142,56 @@ function compareArchiveVersionDesc(aName: string, bName: string): number { return 0; } -function listArchiveVersionDirs(cwd: string): ArchiveVersionDir[] { - const milestonesDir = path.join(planningDir(cwd), 'milestones'); - if (!fs.existsSync(milestonesDir)) return []; +function listArchiveVersionDirs(cwd: string, wsOverride?: string | null): ArchiveVersionDir[] { + // #3804: enumerate BOTH archive shapes under the CURRENT SCOPE's milestones + // tree (planningDir — GSD_WORKSTREAM/GSD_PROJECT-aware, exactly the + // #2855 scoping findPhaseInternal and getArchivedPhaseDirs rely on): + // flat: /milestones/vX.Y-phases// + // workstream archive: /milestones/ws--/phases// + // Pre-#3804 only the flat shape matched, so workstream-archived milestones + // were invisible (the reporter's repo: 20 hidden phase artifacts). The + // ws-* shape's phase dirs sit one level deeper (under phases/) and its dir + // name fails ^v[\d.]+-phases$ — both the name AND the level are modeled. + // Version labels: flat keeps the bare vX.Y; ws-* shapes carry the dir name + // (no numeric version). Flat-newest-first (compareArchiveVersionDesc), then + // ws dirs by name descending — deterministic. The AUDIT's cross-workstream + // enumeration (audit.cts listAuditPhaseTargets) calls this helper once per + // tree (root + each workstream) rather than widening this scope — the + // #2855 no-leak contract for findPhaseInternal/getArchivedPhaseDirs is + // preserved untouched. + const milestonesDir = path.join(planningDir(cwd, wsOverride ?? undefined), 'milestones'); + const out: ArchiveVersionDir[] = []; + const seen = new Set(); + const pushVersion = (version: string, archivePath: string): void => { + const rel = toPosixPath(path.relative(cwd, archivePath)); + if (seen.has(rel)) return; + seen.add(rel); + out.push({ version, archivePath }); + }; + let entries: fs.Dirent[]; try { - const milestoneEntries = fs.readdirSync(milestonesDir, { withFileTypes: true }); - return milestoneEntries - .filter(e => e.isDirectory() && /^v[\d.]+-phases$/.test(e.name)) - .map(e => e.name) - .sort(compareArchiveVersionDesc) - .map(archiveName => ({ - version: archiveName.match(/^(v[\d.]+)-phases$/)![1], - archivePath: path.join(milestonesDir, archiveName), - })); + entries = fs.readdirSync(milestonesDir, { withFileTypes: true }); } catch { return []; } + const flat = entries + .filter(e => e.isDirectory() && /^v[\d.]+-phases$/.test(e.name)) + .map(e => e.name) + .sort(compareArchiveVersionDesc); + for (const archiveName of flat) { + pushVersion(archiveName.match(/^(v[\d.]+)-phases$/)![1], path.join(milestonesDir, archiveName)); + } + const wsDirs = entries + .filter(e => e.isDirectory() && /^ws-/.test(e.name)) + .map(e => e.name) + .sort() + .reverse(); + for (const wsName of wsDirs) { + pushVersion(wsName, path.join(milestonesDir, wsName, 'phases')); + } + + return out; } function searchPhaseInDir(baseDir: string, relBase: string, normalized: string): PhaseSearchResult | null { @@ -463,14 +496,14 @@ function listAllPhaseDirs( return { value, scope: SCOPE.COMPLETE }; } -function getArchivedPhaseDirs(cwd: string): ArchivedPhaseDir[] { +function getArchivedPhaseDirs(cwd: string, wsOverride?: string | null): ArchivedPhaseDir[] { // #2855: same workstream-scoped resolution as findPhaseInternal above, via // the shared listArchiveVersionDirs helper. `phase.list --include-archived` // (the primary non-init consumer) must not leak a different workstream's // archive either. const results: ArchivedPhaseDir[] = []; - for (const { version, archivePath } of listArchiveVersionDirs(cwd)) { + for (const { version, archivePath } of listArchiveVersionDirs(cwd, wsOverride)) { const dirs = readSubdirectories(archivePath, true); for (const dir of dirs) { @@ -486,10 +519,49 @@ function getArchivedPhaseDirs(cwd: string): ArchivedPhaseDir[] { return results; } +/** + * #3804 — the CROSS-WORKSTREAM archive enumeration the audit surfaces need. + * getArchivedPhaseDirs above is deliberately #2855-SCOPED (an ambient + * workstream must not leak other trees' phases to findPhaseInternal), but + * audit-uat's charter (#2766: outstanding items do not stop mattering) is + * cross-workstream: enumerate the project root plus every workstream's own + * milestones tree, labeling workstream entries '/' so + * acknowledge-by-label stays unambiguous. Deduped by full path (the same + * tree cannot be reached twice, but the guard keeps the invariant explicit). + */ +function getAllArchivedPhaseDirs(cwd: string): ArchivedPhaseDir[] { + const out: ArchivedPhaseDir[] = []; + const seen = new Set(); + const collect = (labelPrefix: string, wsOverride: string | null): void => { + for (const archived of getArchivedPhaseDirs(cwd, wsOverride)) { + const rel = toPosixPath(path.relative(cwd, archived.fullPath)); + if (seen.has(rel)) continue; + seen.add(rel); + out.push({ ...archived, milestone: `${labelPrefix}${archived.milestone}` }); + } + }; + collect('', null); + const workstreamsDir = path.join(planningRoot(cwd), 'workstreams'); + try { + const wsEntries = fs.readdirSync(workstreamsDir, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name) + .sort() + .reverse(); + for (const ws of wsEntries) { + collect(`${ws}/`, ws); + } + } catch { + /* no workstreams dir — the root pass above already ran */ + } + return out; +} + export = { searchPhaseInDir, findPhaseInternal, getArchivedPhaseDirs, + getAllArchivedPhaseDirs, listMilestonePhaseDirs, listAllPhaseDirs, }; diff --git a/src/uat.cts b/src/uat.cts index d40db7b19..5bbc79404 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -34,7 +34,7 @@ import phaseIdMod = require('./phase-id.cjs'); const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocator = require('./phase-locator.cjs'); -const { getArchivedPhaseDirs, listMilestonePhaseDirs } = phaseLocator; +const { listMilestonePhaseDirs, getAllArchivedPhaseDirs } = phaseLocator; import { requireSafePath, sanitizeForDisplay } from './security.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- config-loader.cjs is an export= CommonJS module import configLoader = require('./config-loader.cjs'); @@ -146,10 +146,13 @@ function cmdAuditUat(cwd: string, raw: boolean): void { // mattering when a milestone closes: a deferred human-UAT scenario or a // `skipped` live-stack test is exactly what gets archived still-open. // - // Reuses the canonical `getArchivedPhaseDirs` seam (phase-locator.cts), which + // Reuses the canonical `getAllArchivedPhaseDirs` seam (phase-locator.cts), which // `findPhaseInternal` already uses for this same fallback, so the archive // layout convention stays owned by one module. - const archivedDirs = getArchivedPhaseDirs(cwd); + // #3804: the guard AND the scan use the cross-workstream enumeration — + // a project whose only phases live in workstream milestone trees is a + // fully-populated audit, not a broken install. + const archivedDirs = getAllArchivedPhaseDirs(cwd); if (!hasActivePhases && archivedDirs.length === 0) { error('No phases directory found in planning directory'); } diff --git a/tests/audit-workstream-layouts.test.cjs b/tests/audit-workstream-layouts.test.cjs new file mode 100644 index 000000000..5878dd58b --- /dev/null +++ b/tests/audit-workstream-layouts.test.cjs @@ -0,0 +1,123 @@ +'use strict'; + +// ───────────────────────────────────────────────────────────────────────────── +// #3804 — audit-uat must see all three phase-archive layouts. +// +// listArchiveVersionDirs matched exactly one layout +// (`milestones/vX.Y-phases//`). Two workstream layouts missed: +// archived workstream milestones (`milestones/ws--/phases//`) +// and active workstream milestones +// (`workstreams//milestones/vX.Y-phases//`) — so a project using +// workstreams got an "All Clear" audit while items were open (the reporter's +// repo: 20 hidden phase artifacts), and `--ws` emptied the report entirely. +// ───────────────────────────────────────────────────────────────────────────── + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); + +function seedPhaseArtifact(dir, kind) { + fs.mkdirSync(dir, { recursive: true }); + if (kind === 'deferred') { + fs.writeFileSync(path.join(dir, 'deferred-items.md'), [ + '## Deferred Items', + '', + '- open workstream item needing a human', + '', + ].join('\n')); + } else { + // The canonical UAT.md shape the parser reads: `### N. ` test + // blocks with expected/result field lines (see tests/uat.test.cjs's + // fixtures) — the numbered-list form parses to zero items. + fs.writeFileSync(path.join(dir, `${path.basename(dir)}-UAT.md`), [ + '---', + 'status: testing', + '---', + '', + '## Tests', + '', + '### 1. Boot Flow', + 'expected: Workstream phase boots', + 'result: pending', + '', + ].join('\n')); + } +} + +function runAudit(cwd, extraArgs = []) { + const r = runGsdTools(['query', 'audit-uat', ...extraArgs], cwd); + assert.ok(r.success, r.error); + return JSON.parse(r.output); +} + +describe('#3804: audit-uat sees all three phase layouts', () => { + test('#3804: archived workstream milestones surface in audit-uat', (t) => { + const tmpDir = createTempProject('gsd-3804-wsarch-'); + t.after(() => cleanup(tmpDir)); + const wsArchive = path.join(tmpDir, '.planning', 'milestones', + 'ws-v7-3-catalog-models-2026-08-23', 'phases', '185-catalog-admin'); + seedPhaseArtifact(wsArchive, 'deferred'); + + const out = runAudit(tmpDir); + const phase185 = out.results.filter((r) => String(r.phase).startsWith('185')); + assert.ok( + phase185.length >= 1 && phase185.some((r) => (r.items || []).length > 0), + `#3804: the archived workstream phase must surface with its open item; got ${JSON.stringify(out.results.map((r) => ({ phase: r.phase, items: (r.items || []).length })))}`, + ); + }); + + test('#3804: active workstream milestones surface in audit-uat (and --ws does not empty the report)', (t) => { + const tmpDir = createTempProject('gsd-3804-wsact-'); + t.after(() => cleanup(tmpDir)); + const wsMilestone = path.join(tmpDir, '.planning', 'workstreams', 'v7-2-boot-obs', + 'milestones', 'v1.0-phases', '176-boot-flow'); + seedPhaseArtifact(wsMilestone, 'uat'); + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'v7-2-boot-obs'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'workstreams', 'v7-2-boot-obs', 'config.json'), '{}'); + + const bare = runAudit(tmpDir); + assert.ok( + bare.results.some((r) => String(r.phase).startsWith('176') && (r.items || []).length > 0), + `#3804: a flat (no --ws) audit must see the active workstream milestone's open item; got ${JSON.stringify(bare.summary)}`, + ); + + const scoped = runAudit(tmpDir, ['--ws', 'v7-2-boot-obs']); + assert.ok( + scoped.summary.total_items > 0, + `#3804: --ws must not empty the report; got ${JSON.stringify(scoped.summary)}`, + ); + }); + + test('#3804 control: the flat archive layout still surfaces', (t) => { + const tmpDir = createTempProject('gsd-3804-flat-'); + t.after(() => cleanup(tmpDir)); + seedPhaseArtifact(path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '09-old'), 'deferred'); + + const out = runAudit(tmpDir); + assert.ok( + out.results.some((r) => String(r.phase).startsWith('09') && (r.items || []).length > 0), + 'the pre-existing flat archive layout must keep surfacing', + ); + }); + + test('#3804 review: root and workstream v1.0 archives carry DISTINCT milestone labels', (t) => { + // Two v1.0-phases trees (root + workstream) must not both label 'v1.0' — + // audit acknowledge resolves --archived-milestone by strict label + // equality, first-wins, so an ambiguous label can write the marker into + // the wrong root's phase dir (#2237 class). + const tmpDir = createTempProject('gsd-3804-labels-'); + t.after(() => cleanup(tmpDir)); + seedPhaseArtifact(path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '42-root'), 'deferred'); + fs.mkdirSync(path.join(tmpDir, '.planning', 'workstreams', 'alpha'), { recursive: true }); + seedPhaseArtifact(path.join(tmpDir, '.planning', 'workstreams', 'alpha', 'milestones', 'v1.0-phases', '42-alpha'), 'deferred'); + + const out = runAudit(tmpDir); + const labels = out.results.map((r) => r.archived_milestone).filter(Boolean); + assert.ok(labels.includes('v1.0'), `the root label stays the bare version; got ${JSON.stringify(labels)}`); + assert.ok(labels.includes('alpha/v1.0'), `the workstream label is prefixed; got ${JSON.stringify(labels)}`); + assert.equal(new Set(labels).size, labels.length, 'labels must be unique across roots'); + }); +});