diff --git a/.changeset/lively-bears-click.md b/.changeset/lively-bears-click.md new file mode 100644 index 000000000..8764c5f32 --- /dev/null +++ b/.changeset/lively-bears-click.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3535 +--- +**A file belonging to another phase no longer blocks the phase you are in** — sixteen scans (plus the single-pick fallback inside `resolveVerificationFile`) collected verification and UAT artifacts from a phase directory without checking they belonged to that phase, so a stray or copied file such as `04-VERIFICATION.md` sitting in phase 03's directory contributed its status to phase 03. The worst case was not cosmetic: a stray file carrying `gaps_found` or `human_needed` pushed a blocker that flipped the UAT-passed predicate to false, and `transition` gates on that — so a leftover file could refuse to let a phase advance. Some scans could also claim the opposite, reporting verification passed on the strength of a file the phase does not own. All of them now check phase membership. Where a directory's own phase cannot be determined from its name, every file is still included, so no scan silently loses a phase's real blockers; where it can, a phase holding only another phase's report now correctly reports having none of its own rather than adopting it. (#3511) diff --git a/src/audit.cts b/src/audit.cts index c336240ed..6404a9ce3 100644 --- a/src/audit.cts +++ b/src/audit.cts @@ -24,7 +24,7 @@ import frontmatter = require('./frontmatter.cjs'); const { extractFrontmatter } = frontmatter; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; +const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod; import { requireSafePath, sanitizeForDisplay } from './security.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -527,7 +527,13 @@ function scanUatGaps(planDir: string): UatGapItem[] { continue; } - for (const file of files.filter(f => f.includes('-UAT') && f.endsWith('.md'))) { + // Scoped to THIS phase's own token (#3511) so a stray, cross-phase, or + // ad-hoc UAT file cannot surface as this phase's gap — same fix as + // scanVerificationGaps below. + for (const file of scopeToPhase( + files.filter(f => f.includes('-UAT') && f.endsWith('.md')), + dir, + )) { const filePath = path.join(phaseDir, file); let safeFilePath: string; @@ -597,7 +603,12 @@ function scanVerificationGaps(planDir: string): VerificationGapItem[] { continue; } - for (const file of files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md'))) { + // Scoped to THIS phase's own token (#3511) so a stray, cross-phase, or + // ad-hoc VERIFICATION file cannot surface as this phase's gap. + for (const file of scopeToPhase( + files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md')), + dir, + )) { const filePath = path.join(phaseDir, file); let safeFilePath: string; diff --git a/src/commands.cts b/src/commands.cts index 17fdb9fb7..3136c12ed 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -170,8 +170,9 @@ function determinePhaseStatus(plans: number, summaries: number, phaseDir: string // #3492: pin selection to THIS phase's own token so a stray cross-phase // or sentinel-numbered canonically-shaped file cannot outrank this // phase's own (possibly non-canonical) report. - const phaseToken = extractPhaseToken(path.basename(phaseDir)); - const verificationFile = resolveVerificationFile(files, { allowBare: true, phaseToken }); + const phaseDirName = path.basename(phaseDir); + const phaseToken = extractPhaseToken(phaseDirName); + const verificationFile = resolveVerificationFile(files, { allowBare: true, phaseToken, phaseDirName }); if (verificationFile) { const verificationFilePath = path.join(phaseDir, verificationFile); const content = platformReadSync(verificationFilePath) || ''; diff --git a/src/core-utils.cts b/src/core-utils.cts index f5ee239b8..9c3603688 100644 --- a/src/core-utils.cts +++ b/src/core-utils.cts @@ -20,7 +20,7 @@ import fs from 'node:fs'; import path from 'node:path'; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdModule = require('./phase-id.cjs'); -const { comparePhaseNum } = phaseIdModule; +const { comparePhaseNum, scopeToPhase } = phaseIdModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); const { findContextMdIn } = planningWorkspace; @@ -199,6 +199,17 @@ interface PlanScanResultShape { * `hasResearch`/`hasContext`/`hasVerification`/`hasReviews` stay on the raw * `readdirSync` listing — they are not plan-scan concerns. * + * #3511 BLOCKER-2: the raw listing is scoped through `scopeToPhase` (keyed on + * `path.basename(phaseDir)`) before any of the four artifact predicates run, + * so a stray cross-phase file (e.g. `04-VERIFICATION.md` sitting inside phase + * 03's directory) cannot flip `hasResearch`/`hasContext`/`hasVerification`/ + * `hasReviews` true for a phase it does not belong to — the same membership + * rule every other aggregate phase-directory scan (`uat.cts`, `audit.cts`, + * `phase.cts`, `state.cts`) already routes through. `hasContext` is scoped by + * passing the already-scoped array into `findContextMdIn` at this call site + * only — `findContextMdIn` itself is unchanged and its other 4 call sites + * (roadmap.cts, gap-checker.cts, init.cts) are unaffected. + * * Degrades on an unreadable directory instead of throwing: empty arrays, * every flag false, scope UNREADABLE (mirroring scanPhasePlans's own * degrade path). @@ -223,13 +234,15 @@ function getPhaseFileStats(phaseDir: string): PhaseFileStats { }; } + const scopedFiles = scopeToPhase(files, path.basename(phaseDir)); + return { plans: scan.planFiles, summaries: scan.summaryFiles, - hasResearch: files.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'), - hasContext: findContextMdIn(files) !== null, - hasVerification: files.some(f => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md'), - hasReviews: files.some(f => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md'), + hasResearch: scopedFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'), + hasContext: findContextMdIn(scopedFiles) !== null, + hasVerification: scopedFiles.some(f => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md'), + hasReviews: scopedFiles.some(f => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md'), scope: scan.scope, }; } diff --git a/src/gap-checker.cts b/src/gap-checker.cts index 19c03a68f..56be6dba2 100644 --- a/src/gap-checker.cts +++ b/src/gap-checker.cts @@ -30,6 +30,9 @@ import { iterateBullets } from './markdown-sectionizer.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import planScanMod = require('./plan-scan.cjs'); const { scanPhasePlans } = planScanMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdMod = require('./phase-id.cjs'); +const { scopeToPhase } = phaseIdMod; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -353,7 +356,13 @@ function runGapAnalysis(cwd: string, phaseDir: string, options: RunGapAnalysisOp if (fs.existsSync(absPhaseDir)) phaseDirFiles = fs.readdirSync(absPhaseDir); } catch { /* unreadable */ } - const ctxFile = findContextMdIn(phaseDirFiles); + // #3511-class: scope the raw listing to this phase dir before the + // phase-numbered -CONTEXT.md predicate. `phaseDirFiles` itself stays raw — + // it is also reused below only as a `.length > 0` guard ahead of + // scanPhasePlans's own plan-sequence-numbered enumeration, a different + // grammar that must not be scoped by phase number. + const scopedPhaseDirFiles = scopeToPhase(phaseDirFiles, path.basename(absPhaseDir)); + const ctxFile = findContextMdIn(scopedPhaseDirFiles); const ctxPath = ctxFile ? path.join(absPhaseDir, ctxFile) : null; const ctxMd = ctxPath ? fs.readFileSync(ctxPath, 'utf-8') : ''; diff --git a/src/init.cts b/src/init.cts index b33cdadbc..3bf407044 100644 --- a/src/init.cts +++ b/src/init.cts @@ -89,7 +89,7 @@ const { extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId, extractPhaseToken } = phaseId; +const { normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId, extractPhaseToken, scopeToPhase } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { @@ -518,7 +518,12 @@ function detectHasPriorPhases(cwd: string, phaseInfo: Record | } catch { continue; } - if (files.some((f) => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md')) { + // #3511-class: scope the raw listing to THIS entry's own phase artifacts + // before the bare `.some()` predicate runs, so a stray `07-VERIFICATION.md` + // physically sitting in another phase's directory cannot make that + // directory appear to have its own verification report. + const scopedFiles = scopeToPhase(files, entry.name); + if (scopedFiles.some((f) => f.endsWith('-VERIFICATION.md') || f === 'VERIFICATION.md')) { return true; } } @@ -706,7 +711,11 @@ function detectUiPhaseActive(cwd: string, phaseInfo: Record | n // comments elsewhere in this file), same technique as detectHasPriorPhases above. const dirName = path.basename(rawDir); const files = fs.readdirSync(path.join(planningDir(cwd), 'phases', dirName)); - hasUiSpecFile = files.some((f) => f.endsWith('-UI-SPEC.md') || f === 'UI-SPEC.md'); + // #3511-class: scope the raw listing to this phase dir before the + // phase-numbered -UI-SPEC.md predicate, so a stray cross-phase + // UI-SPEC file cannot flip this phase's ui-phase-active flag. + const scopedFiles = scopeToPhase(files, dirName); + hasUiSpecFile = scopedFiles.some((f) => f.endsWith('-UI-SPEC.md') || f === 'UI-SPEC.md'); } catch { hasUiSpecFile = false; } @@ -1134,11 +1143,21 @@ function cmdInitPlanPhase( const phaseDirFull = path.join(cwd, phaseInfo['directory'] as string); try { const files = fs.readdirSync(phaseDirFull); - const contextFile = findContextMdIn(phaseDirFull); + const phaseDirName = path.basename(phaseDirFull); + // #3511 BLOCKER-3: scope the raw listing to THIS phase's own artifacts + // before any bare `.find()` predicate runs, so a `04-UAT.md` (or + // `04-RESEARCH.md`/`04-REVIEWS.md`/`04-PATTERNS.md`) sitting in phase + // 03's directory cannot win a phase-03 lookup — the same + // `isPhaseArtifact` membership rule `resolveVerificationFile` already + // applies via `phaseDirName` below. `findContextMdIn` is passed the + // scoped array (rather than the raw directory path) so this call site + // alone is scoped; its other call sites are unaffected. + const scopedFiles = scopeToPhase(files, phaseDirName); + const contextFile = findContextMdIn(scopedFiles); if (contextFile) { result['context_path'] = toPosixPath(path.join(phaseDirFull, contextFile)); } - const researchFile = files.find( + const researchFile = scopedFiles.find( (f) => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md', ); if (researchFile) { @@ -1151,10 +1170,11 @@ function cmdInitPlanPhase( // #3492: pin selection to THIS phase's own token so a stray cross-phase // or sentinel-numbered canonically-shaped file cannot outrank this // phase's own (possibly non-canonical) report. - const phaseToken = extractPhaseToken(path.basename(phaseDirFull)); + const phaseToken = extractPhaseToken(phaseDirName); const verificationFile = resolveVerificationFile(files, { allowBare: true, phaseToken, + phaseDirName, }); if (verificationFile) { result['verification_path'] = toPosixPath(path.join(phaseDirFull, verificationFile)); @@ -1163,21 +1183,24 @@ function cmdInitPlanPhase( // `.find()` over unsorted readdir order had no phase check and no // ordering, so a stray cross-phase 02-UAT.md could become this phase's // uat_path, filesystem-dependently. Pinned to this phase's own token - // (same rule as verification_path above). + // (same rule as verification_path above), and phase-scoped via + // phaseDirName (#3511) so the alphabetically-first fallback tier also + // excludes cross-phase strays. const uatFile = resolveUatFile(files, { allowBare: true, phaseToken, + phaseDirName, }); if (uatFile) { result['uat_path'] = toPosixPath(path.join(phaseDirFull, uatFile)); } - const reviewsFile = files.find( + const reviewsFile = scopedFiles.find( (f) => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md', ); if (reviewsFile) { result['reviews_path'] = toPosixPath(path.join(phaseDirFull, reviewsFile)); } - const patternsFile = files.find( + const patternsFile = scopedFiles.find( (f) => f.endsWith('-PATTERNS.md') || f === 'PATTERNS.md', ); if (patternsFile) { @@ -1976,11 +1999,15 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { const phaseDirFull = path.join(cwd, phaseInfo['directory'] as string); try { const files = fs.readdirSync(phaseDirFull); - const contextFile = findContextMdIn(phaseDirFull); + const phaseDirName = path.basename(phaseDirFull); + // #3511 BLOCKER-3: see the parallel site above — scope before any bare + // `.find()` predicate so a misfiled cross-phase artifact cannot win. + const scopedFiles = scopeToPhase(files, phaseDirName); + const contextFile = findContextMdIn(scopedFiles); if (contextFile) { result['context_path'] = toPosixPath(path.join(phaseDirFull, contextFile)); } - const researchFile = files.find( + const researchFile = scopedFiles.find( (f) => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md', ); if (researchFile) { @@ -1993,10 +2020,11 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { // #3492: pin selection to THIS phase's own token so a stray cross-phase // or sentinel-numbered canonically-shaped file cannot outrank this // phase's own (possibly non-canonical) report. - const phaseToken = extractPhaseToken(path.basename(phaseDirFull)); + const phaseToken = extractPhaseToken(phaseDirName); const verificationFile = resolveVerificationFile(files, { allowBare: true, phaseToken, + phaseDirName, }); if (verificationFile) { result['verification_path'] = toPosixPath(path.join(phaseDirFull, verificationFile)); @@ -2005,15 +2033,18 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { // `.find()` over unsorted readdir order had no phase check and no // ordering, so a stray cross-phase 02-UAT.md could become this phase's // uat_path, filesystem-dependently. Pinned to this phase's own token - // (same rule as verification_path above). + // (same rule as verification_path above), and phase-scoped via + // phaseDirName (#3511) so the alphabetically-first fallback tier also + // excludes cross-phase strays. const uatFile = resolveUatFile(files, { allowBare: true, phaseToken, + phaseDirName, }); if (uatFile) { result['uat_path'] = toPosixPath(path.join(phaseDirFull, uatFile)); } - const reviewsFile = files.find( + const reviewsFile = scopedFiles.find( (f) => f.endsWith('-REVIEWS.md') || f === 'REVIEWS.md', ); if (reviewsFile) { @@ -2324,8 +2355,13 @@ function cmdInitManager(cwd: string, raw: boolean): void { const phaseFiles = fs.readdirSync(fullDir); planCount = listPhasePlanFiles(fullDir).length; summaryCount = listPhaseSummaryFiles(fullDir).length; - hasContext = findContextMdIn(fullDir) !== null; - hasResearch = phaseFiles.some( + // #3511-class: scope the raw listing to THIS phase's own artifacts + // before the hasContext/hasResearch predicates run, so a stray + // cross-phase `-CONTEXT.md`/`-RESEARCH.md` sitting in this directory + // cannot win this phase's lookup. + const scopedFiles = scopeToPhase(phaseFiles, dirMatch); + hasContext = findContextMdIn(scopedFiles) !== null; + hasResearch = scopedFiles.some( (f) => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md', ); completion = buildPhaseCompletionProjection( @@ -2938,7 +2974,12 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md', ); const phaseDirRel = toPosixPath( diff --git a/src/phase-id.cts b/src/phase-id.cts index 08f6a9aea..083133662 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -510,27 +510,20 @@ function comparePhaseNum(a: unknown, b: unknown): number { } /** - * Extract the phase token from a directory name. + * Segmentation core shared by `extractPhaseToken` (the token VALUE) and + * `isPhaseArtifact` (the DERIVABILITY check — #3511). Factored out so the two + * questions ("what is this dir's token" and "did a real token exist at all") + * can never diverge — see CLAUDE.md's "Generative Fix Divergence" note: this + * is exactly a shared parser between two parallel surfaces. + * + * Returns `tokenSegments.length === 0` iff dirName's own leading segment + * carries no phase-number token (the `extractPhaseToken` dirName-unchanged + * fallback) — i.e. the directory name itself does not start with a digit or a + * short letter+digit prefix, so no reliable phase token can be read from it. */ -function extractPhaseToken(dirName: string, convention?: string): string { - // #612 bracket dir form `{CODE}.{MM}-{PP}[.{SS}]-slug` → phase token `PP[.SS]`. - // GATED on convention === 'bracket' (mirrors getMilestoneFromPhaseId's READING-B - // decision above). A bracket dir `{CODE}.{MM}-{PP}` is string-INDISTINGUISHABLE - // from the legacy #2043/#1324 letter-prefixed-decimal family (`P0.3-2`, - // `P0.12-34`) whenever the project code ends in a digit, so NO string-only - // discriminator can separate the two conventions — auto-detecting here silently - // reinterpreted `P0.3-2` → `2` (was `P0.3-2`), a byte-identical-read regression - // on this CRITICAL 6-caller helper (ADR-2121). Requiring an explicit convention - // signal keeps every existing (convention-less) call site byte-identical to - // prior behaviour — see the #2043 numeric-tail characterization in - // tests/phase-id.test.cjs — while keeping the helper pure (optional param, no - // config read). The captured token is dot-only (`PP[.SS]`); the milestone↔phase - // hyphen and any trailing plan/slug are excluded. - if (convention === 'bracket') { - const bracketDir = dirName.match(/^[A-Z][A-Z0-9_]*\.\d+-(\d+(?:\.\d+)?)/); - if (bracketDir) return bracketDir[1]; - } - +function derivePhaseTokenSegments( + dirName: string, +): { prefix: string; tokenSegments: string[]; firstLetterPrefixed: boolean } { const codePrefixMatch = dirName.match(PROJECT_CODE_PREFIX_CAPTURE_RE_I); let prefix = ''; let rest = dirName; @@ -575,6 +568,33 @@ function extractPhaseToken(dirName: string, convention?: string): string { } } + return { prefix, tokenSegments, firstLetterPrefixed }; +} + +/** + * Extract the phase token from a directory name. + */ +function extractPhaseToken(dirName: string, convention?: string): string { + // #612 bracket dir form `{CODE}.{MM}-{PP}[.{SS}]-slug` → phase token `PP[.SS]`. + // GATED on convention === 'bracket' (mirrors getMilestoneFromPhaseId's READING-B + // decision above). A bracket dir `{CODE}.{MM}-{PP}` is string-INDISTINGUISHABLE + // from the legacy #2043/#1324 letter-prefixed-decimal family (`P0.3-2`, + // `P0.12-34`) whenever the project code ends in a digit, so NO string-only + // discriminator can separate the two conventions — auto-detecting here silently + // reinterpreted `P0.3-2` → `2` (was `P0.3-2`), a byte-identical-read regression + // on this CRITICAL 6-caller helper (ADR-2121). Requiring an explicit convention + // signal keeps every existing (convention-less) call site byte-identical to + // prior behaviour — see the #2043 numeric-tail characterization in + // tests/phase-id.test.cjs — while keeping the helper pure (optional param, no + // config read). The captured token is dot-only (`PP[.SS]`); the milestone↔phase + // hyphen and any trailing plan/slug are excluded. + if (convention === 'bracket') { + const bracketDir = dirName.match(/^[A-Z][A-Z0-9_]*\.\d+-(\d+(?:\.\d+)?)/); + if (bracketDir) return bracketDir[1]; + } + + const { prefix, tokenSegments, firstLetterPrefixed } = derivePhaseTokenSegments(dirName); + if (tokenSegments.length === 0) { return dirName; } @@ -614,6 +634,238 @@ function extractPhaseToken(dirName: string, convention?: string): string { return prefix + tokenSegments.join('-'); } +/** + * #3511 (reworked — adversarial review found the membership rule wrong in + * approach, not just detail): predicate for AGGREGATE phase-directory scans + * (every matching file contributes, e.g. + * uat-predicate/phase.cts/state.cts/uat.cts/audit.cts's `*-UAT.md` / + * `*-VERIFICATION.md` scans) — answers "does fileName belong to THIS phase" + * so a stray, cross-phase, or ad-hoc file (`04-VERIFICATION.md` sitting in + * phase 03's directory) cannot contribute its status to phase 03. This is + * deliberately NOT `resolveVerificationFile` (`src/verification.cts`, + * #3357/#3492) — that resolver answers a SINGLE-PICK question ("which one + * candidate is THE report") for a phase dir already known to hold one; this + * answers a per-file membership question for a scan that must fold in EVERY + * match. See "Reconciliation" below — the two do NOT fully agree. + * + * THE ORIGINAL BUG: files are named by `normalizePhaseName` + * (`cmdScaffold`, `src/commands.cts` — PADDED, project-code-STRIPPED), while + * this predicate read the directory's OWN token via the literal, unpadded, + * project-code-CARRYING `extractPhaseToken(phaseDirName)`. Two different + * normalizations of the same phase number, so a literal + * `startsWith(token + '-')` excluded a phase's own artifacts whenever they + * disagreed: `CK-01-foundation` (token `CK-01`, file `01-VERIFICATION.md`), + * `1-unpadded` (token `1`, file `01-VERIFICATION.md`), and the #2528 + * digit-leading-slug family `05-80-20-cleanup` / `10-24-7-autonomy` (token + * over-absorbs past the digit run `cmdScaffold` actually writes: `05-80-20` + * vs the real `05-UAT.md`). + * + * THE FIX: build the set of every phase-number READING this directory could + * plausibly resolve to elsewhere in the module, then check fileName against + * ALL of them — reusing the exact readings `matchPhaseDirs` / + * `phaseNumberForMatch` (#2528, below) already carry for directory + * RESOLUTION, so this membership check can never diverge from what "the + * directory for phase N" means elsewhere in the module (no third + * normalization — CLAUDE.md's Generative Fix Divergence class): + * 1. the literal token (`extractPhaseToken(phaseDirName)` — still correct + * for the common case and for genuine decimal / letter-suffixed + * sub-phase dirs); + * 2. the same token read off the project-code-STRIPPED name + * (`stripProjectCodePrefix` — the exact fallback `phaseTokenMatches` + * already applies, #612/#1324); + * 3. the directory's own LEADING DIGIT RUN on the stripped name + * (`LEADING_DIGIT_RUN_RE` — the #2528 bare-integer-fallback reading, + * the one that actually matches what `cmdScaffold` writes for the + * digit-leading-slug family); and + * 4. each of (1)-(3) additionally passed through `normalizePhaseName`, + * since files always carry the PADDED form and directories often do + * not (`1-unpadded` vs `01-...`). + * A file belongs when it starts with any candidate + `-` OR any candidate + + * `.`, compared case-insensitively (matching `phaseTokenMatches`' own rule — + * review item 8: `03A-VERIFICATION.md` vs `03a-foo`). This is a PREFIX check, + * not a full-token equality — `03-01-SUMMARY.md` (phase 03, plan 01) must + * still match dir `03-foo` on candidate `03`, even though + * `extractPhaseToken('03-01-SUMMARY.md')` would (wrongly, for this purpose) + * read `03-01` as a mis-absorbed 2-digit continuation. + * + * DOTTED SUB-PHASE CONTINUATION: the dot arm of the check exists because this + * module's own token grammar (`PHASE_NUMBER_TOKEN_SOURCE`) admits a dotted + * sub-phase continuation (`(?:\.\d+)*`) alongside dash-continuations — a + * sub-phase artifact `01.1-CONTEXT.md` is `01`'s own file, written into `01`'s + * directory, not a stray from a different phase. A dash-only prefix check + * excluded it (`01.1-` does not start with `01-`), which is over-exclusion: + * the dangerous direction for an aggregate scan whose fail-safes above all + * default to inclusion when membership is unclear. Widening dash-only to + * dash-OR-dot only ever ADDS a match a candidate already earned; it cannot + * newly admit a file whose leading digits differ from `candidate`, so it + * cannot resolve a genuinely different phase's artifact (`02.1-...` still + * fails every `01`-rooted candidate). + * + * BRACKET CONVENTION (review item 7): a letter-prefixed-decimal dir + * (`P0.3-2-slug`) is string-INDISTINGUISHABLE from a bracket-dir token + * (`extractPhaseToken` above, gated on `convention === 'bracket'`) without an + * explicit convention signal — and this predicate is never given one: none + * of its 9 call sites thread `convention`/config through today. Rather than + * guess a reading it cannot know is active and risk excluding the phase's OWN + * artifact (the exact defect class this rework exists to fix), this family + * (`firstLetterPrefixed` dirs) falls into the same include-everything + * fail-safe as the zero-segment case below — a documented, deliberate + * widening (it also stops excluding a genuine stray from a DIFFERENT + * letter-prefixed-decimal phase, narrowly) accepted in trade for never + * dropping the phase's own report. Convention-aware scoping for this family + * is deferred to whenever a call site actually threads `convention` through. + * + * FAIL-SAFE (#3511, unchanged): when dirName's own leading segment carries no + * phase-number token at all (`derivePhaseTokenSegments` finds zero segments — + * the same condition `extractPhaseToken` treats as "return dirName + * unchanged"), no reliable token exists to scope against. Excluding on an + * unreliable token would make an aggregate gate silently PERMISSIVE in the + * wrong direction — dropping the phase's own real blockers — which is worse + * than the cross-phase-contamination bug this predicate exists to fix. + * Instead every file is treated as belonging to the phase (returns `true` + * unconditionally), matching pre-fix (unscoped) behaviour for that directory. + * + * FIX 2 — bare `VERIFICATION.md` / `UAT.md` (no dash, no token of its own): + * `derivePhaseTokenSegments(fileName)` also finds zero segments for these — + * the file carries no phase number to compare against anything. Directory + * containment is the only signal available for a token-less file, and it is + * sufficient: every call site passes `fs.readdirSync` results for ONE + * specific phase dir, so a token-less candidate already reaching this + * predicate (past each call site's own verification/UAT suffix pre-filter) + * is, by construction, that phase's own + * listing. Returns `true` unconditionally, same as the dir-side fail-safe. + * + * RECONCILIATION WITH resolveVerificationFile (#3357/#3492/#3511) — the two + * surfaces now AGREE. `resolveVerificationFile`'s fallback (`verification.cts`, + * "Fallback" step in its own docblock) filters its dashed candidates through + * THIS predicate — `isPhaseArtifact(f, phaseDirName)` — before picking + * alphabetically-first, via a new `phaseDirName` option every call site + * threads in (the same basename each already derives for `phaseToken`). So a + * stray cross-phase file can no longer win the single-pick fallback either: + * it is excluded there for the identical reason it is excluded from the + * aggregate scans here — membership, not canonical shape. The fail-safes stay + * aligned too: when this predicate cannot determine membership for a + * directory (returns `true` unconditionally — see FAIL-SAFE above), + * `resolveVerificationFile`'s filter is a no-op and its fallback degrades to + * the original pre-#3357 "alphabetically first of ALL dashed candidates" + * behavior, exactly as it always did for that directory shape. + */ +function isPhaseArtifact(fileName: string, phaseDirName: string): boolean { + const { tokenSegments, firstLetterPrefixed } = derivePhaseTokenSegments(phaseDirName); + if (tokenSegments.length === 0) return true; + + const literalToken = extractPhaseToken(phaseDirName); + const strippedDir = stripProjectCodePrefix(phaseDirName); + const strippedToken = strippedDir !== phaseDirName ? extractPhaseToken(strippedDir) : literalToken; + const leadingRunMatch = strippedDir.match(LEADING_DIGIT_RUN_RE); + + const rawCandidates = [literalToken, strippedToken, leadingRunMatch?.[1]].filter( + (t): t is string => Boolean(t), + ); + // Each reading is compared in BOTH its padded and de-padded form: files are + // written padded by `normalizePhaseName` (`cmdScaffold`) while directories + // are often not (`1-unpadded`), and legacy trees carry the reverse pairing. + // De-padding is numeric-only — a token with a letter suffix or a dotted + // sub-phase (`03A`, `03.1`) has no meaningful de-padded form and is left + // alone, so this only ever ADDS a reading and can never drop one. + const depad = (t: string): string => (/^\d+$/.test(t) ? String(Number(t)) : t); + const candidates = new Set( + rawCandidates + .flatMap(t => [t, normalizePhaseName(t), depad(t)]) + .map(t => t.toUpperCase()), + ); + + const fileUpper = fileName.toUpperCase(); + for (const candidate of candidates) { + // A dotted sub-phase segment (e.g. `01.1-CONTEXT.md`) is a legitimate + // continuation of `candidate` per this module's own token grammar + // (PHASE_NUMBER_TOKEN_SOURCE admits `(?:\.\d+)*`), so it belongs to + // `candidate`'s own directory just as a dash-continuation does. Inclusion + // is the safe direction for these aggregate scans (see FAIL-SAFE above) — + // widening a dash-only check to dash-OR-dot never drops a genuine match, + // it only stops wrongly excluding one. + // + // Accepted separator class after a matched candidate: `-`, `.`, or `_`. + // The underscore was added for state.cts's `cmdStateValidate` S006/S007 + // scan, whose own pre-filter is deliberately broader than the dashed + // grammar every other call site uses (`.includes('VERIFICATION')`, no + // dash required — see the WARNING-4 comment there), so it admits names + // like `03_VERIFICATION.md`. Before this predicate accepted `_` as a + // boundary, such a file failed the `-`/`.`-only check here even though + // its digits matched `candidate` exactly, and `scopeToPhase` dropped it — + // a real same-phase verification report reported as absent. Widening the + // separator class only ever EXTENDS a candidate whose digits already + // match exactly; it cannot admit a genuinely different phase's file, + // since the candidate comparison itself is unchanged. + if ( + fileUpper.startsWith(`${candidate}-`) || + fileUpper.startsWith(`${candidate}.`) || + fileUpper.startsWith(`${candidate}_`) + ) return true; + } + + // FIX 2: token-less filename (bare "VERIFICATION.md"/"UAT.md") — containment + // in this phase's own directory listing is sufficient. + if (derivePhaseTokenSegments(fileName).tokenSegments.length === 0) return true; + + // Bracket-convention ambiguity fail-safe — see docblock above. + if (firstLetterPrefixed) return true; + + return false; +} + +/** + * #3511: scope `fileNames` to the subset that passes + * `isPhaseArtifact(fileName, phaseDirName)`. The single seam every + * phase-directory scan routes through, so the membership rule has ONE owner. + * + * AN EMPTY RESULT IS A REAL ANSWER — deliberately, and this is the hard-won + * part. An earlier revision of this helper carried an extra rule ("scoping + * must never turn a non-empty set into an empty one": if the filter removed + * every file, return the unfiltered input). It was added to rescue a + * directory whose basename merely PARSES phase-shaped — + * `gsd-651-broad-grep-a1b2`, an `mkdtemp`-style fixture name that + * `extractPhaseToken` reads as project code `gsd` + phase `651` (the capture + * regex is case-INSENSITIVE) — holding only `01-bg-VERIFICATION.md`, which + * the filter then dropped, yielding an empty set indistinguishable from "no + * report exists". + * + * That rescue was wrong, and no local rule can make it right: a directory + * whose own name says phase 651 holding only a file that says phase 01 is + * STRING-INDISTINGUISHABLE from `03-foo/` holding only `04-VERIFICATION.md` + * — the exact cross-phase stray #3511 exists to exclude. Keeping the rule + * meant a real phase directory holding only a MISFILED report would resolve + * to it and publish another phase's `passed` as its own: the reported bug, in + * its single most damaging form. `missing` is the correct answer when a + * phase's own report is genuinely absent, and every caller already has a + * `missing`/`null` branch for it. + * + * The over-exclusion that rule was reaching for is instead handled where it + * is actually determinable, inside `isPhaseArtifact`: the zero-token dir + * fail-safe, the `firstLetterPrefixed` bracket-ambiguity fail-safe, the + * token-less-filename rule, and the multi-reading candidate set (literal / + * project-code-stripped / leading-digit-run, each also padded AND de-padded) + * that covers every normalization a phase directory and its files can + * legitimately disagree on. A file excluded after all of those genuinely + * names a different phase. + * + * SITE DISCIPLINE: every aggregate-scan call site (`uat.cts`, + * `uat-predicate.cts`, `phase.cts`, `audit.cts`, `state.cts`, + * `core-utils.cts`'s `getPhaseFileStats` — #3511 BLOCKER-2 — and + * `init.cts`'s two phase-info-projection sites — #3511 BLOCKER-3, both of + * which scope the raw listing once up front and reuse it for every bare + * `.find()`/`.some()` artifact predicate: context/research/UAT/reviews/ + * patterns) and `resolveVerificationFile`'s single-pick fallback + * (`verification.cts`) MUST route through this helper rather than calling + * `isPhaseArtifact` in a filter position directly, so the rule cannot be + * re-derived per site (CLAUDE.md's Generative Fix Divergence class). + * `isPhaseArtifact` stays exported for single-item membership questions and + * its own unit tests. + */ +function scopeToPhase(fileNames: string[], phaseDirName: string): string[] { + return fileNames.filter((f) => isPhaseArtifact(f, phaseDirName)); +} + /** * Check if a directory name's phase token matches the normalized phase exactly. */ @@ -950,6 +1202,8 @@ export = { phaseMarkdownRegexSourceExact, comparePhaseNum, extractPhaseToken, + isPhaseArtifact, + scopeToPhase, phaseTokenMatches, matchPhaseDirs, phaseNumberForMatch, diff --git a/src/phase.cts b/src/phase.cts index d7f4d9c17..0fd5a9d67 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -43,6 +43,7 @@ const { comparePhaseNum, matchPhaseDirs, isSentinelPhaseId, + scopeToPhase, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE, @@ -1521,12 +1522,39 @@ function renameDecimalPhases( return { renamedDirs, renamedFiles }; } +/** + * Find a free name to move an occupying file aside to, on collision, so the + * intended rename can proceed without destroying either file. Appends the + * literal `.orphaned` suffix to the whole existing filename (never `.md`, + * so no phase-directory scan predicate — all of which filter on + * `.endsWith('.md')` / `.endsWith('-VERIFICATION.md')` etc — can ever pick + * the displaced file back up as any phase's artifact). Falls back to a + * numeric discriminator (`.orphaned.2`, `.orphaned.3`, ...) if `.orphaned` + * itself is taken, bounded at 100 attempts so a pathological directory + * cannot loop forever; returns null if no free name is found within that + * bound, letting the caller fall back to skip-and-report. + */ +function findOrphanedDisplacementName(dir: string, fileName: string): string | null { + const base = `${fileName}.orphaned`; + if (!fs.existsSync(path.join(dir, base))) return base; + for (let n = 2; n <= 100; n++) { + const candidate = `${base}.${n}`; + if (!fs.existsSync(path.join(dir, candidate))) return candidate; + } + return null; +} + function renameIntegerPhases( phasesDir: string, removedInt: number, -): { renamedDirs: { from: string; to: string }[]; renamedFiles: { from: string; to: string }[] } { +): { + renamedDirs: { from: string; to: string }[]; + renamedFiles: { from: string; to: string }[]; + renamedFileCollisions: { from: string; to: string; displaced_to: string | null }[]; +} { const renamedDirs: { from: string; to: string }[] = []; const renamedFiles: { from: string; to: string }[] = []; + const renamedFileCollisions: { from: string; to: string; displaced_to: string | null }[] = []; const dirs = readSubdirectories(phasesDir, true); const toRename: RenameIntInfo[] = dirs .map((dir) => { @@ -1558,20 +1586,76 @@ function renameIntegerPhases( const oldPrefix = `${oldPadded}${letterSuffix}${decimalSuffix}`; const newPrefix = `${newPadded}${letterSuffix}${decimalSuffix}`; const newDirName = `${newPrefix}-${item.slug}`; + // WARNING-3 (#3511 review): the directory match above accepts an + // UNPADDED leading number (`\d+`), so a supported rename can pair a + // 2-padded dir with an unpadded-numbered artifact — dir `9-slug` holding + // `9-VERIFICATION.md`. Renaming files by `f.startsWith(oldPrefix)` alone + // (oldPrefix always 2-padded) misses that file: it becomes desynced from + // its now-renamed directory and the phase reads `missing`. Try the + // UNPADDED old-prefix form as a fallback so such an artifact renames + // alongside its directory. A trailing-digit boundary check keeps the + // unpadded form from over-matching a DIFFERENT phase's file (unpadded + // prefix "1" must not match "10-…"). + const oldPrefixUnpadded = `${item.oldInt}${letterSuffix}${decimalSuffix}`; retryRenameSync(path.join(phasesDir, item.dir), path.join(phasesDir, newDirName)); renamedDirs.push({ from: item.dir, to: newDirName }); for (const f of fs.readdirSync(path.join(phasesDir, newDirName))) { + let matchedPrefix: string | null = null; if (f.startsWith(oldPrefix)) { - const newFileName = newPrefix + f.slice(oldPrefix.length); - retryRenameSync( - path.join(phasesDir, newDirName, f), - path.join(phasesDir, newDirName, newFileName), - ); + matchedPrefix = oldPrefix; + } else if ( + oldPrefixUnpadded !== oldPrefix && + f.startsWith(oldPrefixUnpadded) && + // Token-boundary check: the character immediately after the unpadded + // prefix must be a separator (`-`, `.`) or end-of-name, not any + // non-digit. A bare `!/^\d/` test (prior form) let a LETTER through + // too, so unpadded prefix "2" wrongly matched "2FA-notes.md" (a + // wholly unrelated file whose name merely starts with the digit). + (f.length === oldPrefixUnpadded.length || /^[-.]/.test(f.slice(oldPrefixUnpadded.length))) + ) { + matchedPrefix = oldPrefixUnpadded; + } + if (matchedPrefix) { + const newFileName = newPrefix + f.slice(matchedPrefix.length); + const destPath = path.join(phasesDir, newDirName, newFileName); + // Collision guard: the padded and unpadded prefix forms can both + // resolve to the SAME destination (e.g. `09-VERIFICATION.md` and + // `9-VERIFICATION.md` in one directory both target + // `08-VERIFICATION.md`), and a stray cross-phase file can already sit + // at the destination name (e.g. a leftover `08-VERIFICATION.md` + // belonging to a DIFFERENT phase, inside phase 9's directory). + // Renaming blindly over an existing target silently destroys + // whichever file loses; skipping the rename instead lets the stray + // outrank the phase's own renamed artifact once it lands at the + // canonical name. Neither is acceptable: move the OCCUPYING file + // aside first (never overwrite, never skip the real rename), then + // complete the intended rename so the phase's own artifact takes the + // canonical name. This also handles a target that was already + // claimed by an EARLIER file in this same pass, since that earlier + // rename already created it on disk. + if (fs.existsSync(destPath)) { + const displacedName = findOrphanedDisplacementName( + path.join(phasesDir, newDirName), + newFileName, + ); + if (displacedName === null) { + // No free displacement name within the bounded search — fall + // back to skip-and-report rather than looping or overwriting. + renamedFileCollisions.push({ from: f, to: newFileName, displaced_to: null }); + continue; + } + retryRenameSync(destPath, path.join(phasesDir, newDirName, displacedName)); + retryRenameSync(path.join(phasesDir, newDirName, f), destPath); + renamedFiles.push({ from: f, to: newFileName }); + renamedFileCollisions.push({ from: f, to: newFileName, displaced_to: displacedName }); + continue; + } + retryRenameSync(path.join(phasesDir, newDirName, f), destPath); renamedFiles.push({ from: f, to: newFileName }); } } } - return { renamedDirs, renamedFiles }; + return { renamedDirs, renamedFiles, renamedFileCollisions }; } function decrementRoadmapPhaseNumber(raw: string, removedInt: number): string { @@ -1897,16 +1981,22 @@ function cmdPhaseRemove( let renamedDirs: { from: string; to: string }[] = []; let renamedFiles: { from: string; to: string }[] = []; + let renamedFileCollisions: { from: string; to: string; displaced_to: string | null }[] = []; try { - const renamed = isDecimal - ? renameDecimalPhases( - phasesDir, - parseInt(normalized.split('.')[0], 10), - parseInt(normalized.split('.')[1], 10), - ) - : renameIntegerPhases(phasesDir, parseInt(normalized, 10)); - renamedDirs = renamed.renamedDirs; - renamedFiles = renamed.renamedFiles; + if (isDecimal) { + const renamed = renameDecimalPhases( + phasesDir, + parseInt(normalized.split('.')[0], 10), + parseInt(normalized.split('.')[1], 10), + ); + renamedDirs = renamed.renamedDirs; + renamedFiles = renamed.renamedFiles; + } else { + const renamed = renameIntegerPhases(phasesDir, parseInt(normalized, 10)); + renamedDirs = renamed.renamedDirs; + renamedFiles = renamed.renamedFiles; + renamedFileCollisions = renamed.renamedFileCollisions; + } } catch (e) { // #2245 audit (was ERROR-HIDING): renameDecimalPhases/renameIntegerPhases // rename subsequent phase directories ON DISK one at a time — a mid-loop @@ -1999,6 +2089,7 @@ function cmdPhaseRemove( directory_deleted: targetDir, renamed_directories: renamedDirs, renamed_files: renamedFiles, + renamed_file_collisions: renamedFileCollisions, roadmap_updated: true, state_updated: stateUpdated, }, @@ -2194,8 +2285,15 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { try { const phaseFiles = fs.readdirSync(phaseFullDir); + // #3511: scope this advisory pre-scan to THIS phase's own token so a + // stray, cross-phase, or ad-hoc file cannot name a warning against a + // phase it does not belong to. + const phaseFullDirBaseName = path.basename(phaseFullDir); - for (const file of phaseFiles.filter((f) => f.includes('-UAT') && f.endsWith('.md'))) { + for (const file of scopeToPhase( + phaseFiles.filter((f) => f.includes('-UAT') && f.endsWith('.md')), + phaseFullDirBaseName, + )) { const content = fs.readFileSync(path.join(phaseFullDir, file), 'utf-8'); if (/result: pending/.test(content)) warnings.push(`${file}: has pending tests`); if (/result: blocked/.test(content)) warnings.push(`${file}: has blocked tests`); @@ -2203,8 +2301,9 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { if (/status: diagnosed/.test(content)) warnings.push(`${file}: has diagnosed gaps`); } - for (const file of phaseFiles.filter( - (f) => f.includes('-VERIFICATION') && f.endsWith('.md'), + for (const file of scopeToPhase( + phaseFiles.filter((f) => f.includes('-VERIFICATION') && f.endsWith('.md')), + phaseFullDirBaseName, )) { const verificationFilePath = path.join(phaseFullDir, file); const content = fs.readFileSync(verificationFilePath, 'utf-8'); diff --git a/src/planning-snapshot.cts b/src/planning-snapshot.cts index 82dc0b848..e956f8359 100644 --- a/src/planning-snapshot.cts +++ b/src/planning-snapshot.cts @@ -59,7 +59,7 @@ import worktreeSafetyMod = require('./worktree-safety.cjs'); const { inspectWorktreeHealth } = worktreeSafetyMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { PHASE_NUMBER_TOKEN_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, stripProjectCodePrefix } = phaseIdMod; +const { PHASE_NUMBER_TOKEN_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, stripProjectCodePrefix, scopeToPhase } = phaseIdMod; import { buildRoadmapPhaseVariants, PHASE_TOKEN_FROM_DIR_RE, MILESTONE_ARCHIVE_DIR_RE } from './validate.cjs'; // ─── worstScope — the one new piece of coordination logic ─────────────────── @@ -630,8 +630,13 @@ function buildResearchValidationStatusField( } catch { return { dir, hasValidationArchitecture: false, hasValidationMd: false }; } - const researchFile = files.find((f) => f.endsWith('-RESEARCH.md')); - const hasValidationMd = files.some((f) => f.endsWith('-VALIDATION.md')); + // #3511: scope the raw listing to this phase dir before the two + // phase-numbered-artifact predicates, so a stray cross-phase + // -RESEARCH.md/-VALIDATION.md sitting in the wrong directory cannot flip + // this phase's flags — mirrors core-utils.cts's getPhaseFileStats. + const scopedFiles = scopeToPhase(files, dir); + const researchFile = scopedFiles.find((f) => f.endsWith('-RESEARCH.md')); + const hasValidationMd = scopedFiles.some((f) => f.endsWith('-VALIDATION.md')); let hasValidationArchitecture = false; if (researchFile) { try { diff --git a/src/roadmap.cts b/src/roadmap.cts index b599a628d..1971b851c 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -16,7 +16,7 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, phaseMarkdownRegexSource, matchPhaseDirs, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, phaseMarkdownRegexSource, matchPhaseDirs, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources, isSentinelPhaseId, scopeToPhase } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal, listMilestonePhaseDirs } = phaseLocatorMod; @@ -114,11 +114,17 @@ function countPhasePlansAndSummaries(phaseDir: string): PhasePlansAndSummaries { // once and share the listing for all non-plan metadata that cmdRoadmapAnalyze needs. let phaseFiles: string[] = []; try { phaseFiles = fs.readdirSync(phaseDir); } catch { /* empty */ } + // #3511: scope the raw listing to this phase dir before the + // phase-numbered-artifact predicates (hasContext/hasResearch) — planCount/ + // summaryCount above stay on scanPhasePlans's own unscoped listing since a + // PLAN/SUMMARY leading number is a plan sequence number, not a phase + // number. Mirrors core-utils.cts's getPhaseFileStats. + const scopedFiles = scopeToPhase(phaseFiles, path.basename(phaseDir)); return { planCount, summaryCount, - hasContext: findContextMdIn(phaseFiles) !== null, - hasResearch: phaseFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'), + hasContext: findContextMdIn(scopedFiles) !== null, + hasResearch: scopedFiles.some(f => f.endsWith('-RESEARCH.md') || f === 'RESEARCH.md'), }; } diff --git a/src/state.cts b/src/state.cts index ce40b0053..80e1b3841 100644 --- a/src/state.cts +++ b/src/state.cts @@ -17,7 +17,14 @@ import configLoaderMod = require('./config-loader.cjs'); const { loadConfig } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE, phaseKeyFromToken, phaseKeyFromDir, isSentinelPhaseId } = phaseIdMod; +const { + parsePhaseFromProse, + PHASE_NUMBER_TOKEN_SOURCE, + phaseKeyFromToken, + phaseKeyFromDir, + isSentinelPhaseId, + scopeToPhase, +} = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); const { getMilestoneInfo, extractCurrentMilestone, isMilestoneBoundedInRoadmap, hasMilestoneSectioning } = roadmapParserMod; @@ -4187,9 +4194,35 @@ function cmdStateValidate(cwd: string, raw: boolean): void { )); } - // Check for VERIFICATION.md + // Check for VERIFICATION.md — scoped to THIS phase's own token (#3511) + // so a stray, cross-phase, or ad-hoc VERIFICATION file cannot claim + // this phase's status has drifted. + // + // WARNING-4 (#3511 review): the pre-filter grammar here is + // deliberately BROADER than the `-VERIFICATION.md` suffix every + // other site in the codebase uses — `.includes('VERIFICATION')` + // admits names like `03_VERIFICATION.md` (underscore, no dash) that + // the dashed grammar would reject outright. That breadth predates + // #3511 and is intentional here (this is a best-effort drift + // WARNING scan, not an authoritative single-pick resolver), so it is + // left as-is rather than narrowed to match the dashed sites — doing + // so would be a separate, un-asked-for behavior change (S006/S007). + // What #3511 DOES change is that a name this broader grammar admits + // is now ALSO subject to the same `scopeToPhase` membership check as + // every dashed-grammar site, so a stray `04_VERIFICATION.md`-shaped + // file in phase 03's directory is excluded exactly like a stray + // `04-VERIFICATION.md` would be — while `03_VERIFICATION.md` (own + // phase, underscore separator) is NOT excluded: `isPhaseArtifact` + // (`phase-id.cts`) accepts `_` as a candidate-boundary separator + // alongside `-` and `.` for exactly this reason, so an S006/S007 + // scan of `03-alpha/03_VERIFICATION.md` still resolves to S006 + // ("verification passed" drift), not a false S007. const files = fs.readdirSync(phaseDirPath); - const verificationFiles = files.filter(f => f.includes('VERIFICATION') && f.endsWith('.md')); + const phaseDirBaseName = path.basename(phaseDirPath); + const verificationFiles = scopeToPhase( + files.filter(f => f.includes('VERIFICATION') && f.endsWith('.md')), + phaseDirBaseName, + ); for (const vf of verificationFiles) { try { const vContent = fs.readFileSync(path.join(phaseDirPath, vf), 'utf-8'); diff --git a/src/uat-predicate.cts b/src/uat-predicate.cts index 186874c3b..0b6c4f952 100644 --- a/src/uat-predicate.cts +++ b/src/uat-predicate.cts @@ -22,6 +22,9 @@ const { stripFencedCode } = markdownSectionizer; // eslint-disable-next-line @typescript-eslint/no-require-imports import verification = require('./verification.cjs'); const { readVerificationStatus } = verification; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdMod = require('./phase-id.cjs'); +const { scopeToPhase } = phaseIdMod; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -243,9 +246,18 @@ function evaluateUatPassed( }; } - // Filter UAT and VERIFICATION files using the same filter as cmdPhaseComplete - const uatFileNames = dirEntries.filter(f => f.includes('-UAT') && f.endsWith('.md')); - const verFileNames = dirEntries.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md')); + // Filter UAT and VERIFICATION files using the same filter as cmdPhaseComplete, + // scoped to THIS phase's own token (#3511) — a stray, cross-phase, or ad-hoc + // file can no longer contribute a blocker to a phase it does not belong to. + const phaseDirBaseName = path.basename(phaseFullDir); + const uatFileNames = scopeToPhase( + dirEntries.filter(f => f.includes('-UAT') && f.endsWith('.md')), + phaseDirBaseName, + ); + const verFileNames = scopeToPhase( + dirEntries.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md')), + phaseDirBaseName, + ); // ── Process UAT files ────────────────────────────────────────────────────── for (const file of uatFileNames) { diff --git a/src/uat.cts b/src/uat.cts index 7bdded703..dc3430a4f 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -31,7 +31,7 @@ import frontmatter = require('./frontmatter.cjs'); const { extractFrontmatter } = frontmatter; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; +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; @@ -134,8 +134,12 @@ function cmdAuditUat(cwd: string, raw: boolean): void { const phaseNum = phaseMatch ? phaseMatch[1] : dir; const files = fs.readdirSync(phaseDir); - // Process UAT files - for (const file of files.filter(f => f.includes('-UAT') && f.endsWith('.md'))) { + // Process UAT files — scoped to THIS phase's own token (#3511) via + // scopeToPhase, so a stray, cross-phase, or ad-hoc file cannot be reported + // under this phase's audit-uat entry. A phase whose own UAT file is + // genuinely absent scopes to empty and contributes nothing — correct, and + // the reason scopeToPhase has no unfiltered fallback. + for (const file of scopeToPhase(files.filter(f => f.includes('-UAT') && f.endsWith('.md')), dir)) { const uatFilePath = path.join(phaseDir, file); const content = fs.readFileSync(uatFilePath, 'utf-8'); const items = parseUatItems(content); @@ -153,8 +157,9 @@ function cmdAuditUat(cwd: string, raw: boolean): void { } } - // Process VERIFICATION files - for (const file of files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md'))) { + // Process VERIFICATION files — scoped to THIS phase's own token (#3511) + // for the same reason as the UAT loop above. + for (const file of scopeToPhase(files.filter(f => f.includes('-VERIFICATION') && f.endsWith('.md')), dir)) { const verificationFilePath = path.join(phaseDir, file); const content = fs.readFileSync(verificationFilePath, 'utf-8'); const status = extractFrontmatter(content, verificationFilePath).status as string || 'unknown'; diff --git a/src/verification.cts b/src/verification.cts index ac5588b67..3a2c04a98 100644 --- a/src/verification.cts +++ b/src/verification.cts @@ -43,7 +43,7 @@ import { execGit } from './shell-command-projection.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; const { output, error } = io; -const { extractPhaseToken } = phaseId; +const { extractPhaseToken, scopeToPhase } = phaseId; const { extractFrontmatter } = frontmatterMod; const { SCOPE } = planningScopeMod; type Scope = planningScopeMod.Scope; @@ -309,10 +309,20 @@ interface ResolveVerificationFileOptions { * (`12-review-VERIFICATION.md`) — a regression this option closes. * * Omitted / empty when the token cannot be derived: falls back to plain - * alphabetically-first among all dashed candidates (the original pre-#3357 - * behavior), never to null. + * alphabetically-first among the SCOPED dashed candidates (see + * `phaseDirName` below), never to null. */ phaseToken?: string; + /** + * #3511 reconciliation: the phase directory's own basename (the same value + * every call site already passes through `extractPhaseToken` to derive + * `phaseToken` above) — needed separately because the fallback below scopes + * by `isPhaseArtifact(fileName, phaseDirName)`, not by `phaseToken`. + * + * Omitted: the fallback degrades to the plain (unscoped) alphabetically-first + * pick — the original pre-#3357 behavior — never to null. + */ + phaseDirName?: string; } /** @@ -332,22 +342,59 @@ type ResolveUatFileOptions = ResolveVerificationFileOptions; * `bareName` is the artifact filename WITHOUT the leading dash (`'UAT.md'`); * a "dashed" candidate is any entry ending `-${bareName}`. * - * Selection order (identical to resolveVerificationFile's documented tiers, - * generalized off the suffix): + * Selection order: * 1. `options.phaseToken` given and `-${bareName}` is among * the candidates — that exact file always wins: it is THIS phase's own * artifact, and no other candidate (whichever phase's token it carries) * can outrank it (#3492 / #3518). - * 2. Fallback — no exact phase-token match (or no token given): - * alphabetically first of ALL dashed candidates (deterministic — - * `entries` is an unsorted readdir listing whose order is - * filesystem-dependent, so the sort is what makes the answer - * machine-independent). Load-bearing: a phase whose only artifact is - * non-canonically named must keep resolving to it, not to null — the - * fix must not turn "found an artifact" into "found nothing". - * 3. `options.allowBare` only — a bare `${bareName}`, ranked BELOW both of - * the above (a dashed file names its phase; a bare one does not). - * Reached only when (1) and (2) found no dashed candidate at all. + * 2. Fallback — no exact phase-token match (or no token given): alphabetically + * first of the dashed candidates that are THIS phase's own, per + * `scopeToPhase(candidates, options.phaseDirName)` (#3511 reconciliation, + * below). Load-bearing: a phase whose only artifact is non-canonically + * named must keep resolving to it, not to null — this fix must not turn + * "found an artifact" into "found nothing" for anyone. A + * non-canonically-named artifact of THIS phase (e.g. + * `03-CORRECTION-VERIFICATION.md` in `03-foo`) still passes + * `isPhaseArtifact` (it names phase 03, same as the directory), so it + * is still returned here. + * 3. `options.allowBare` only — a bare `${bareName}`, ranked BELOW both + * of the above. Rationale: a dashed file names its phase, a bare one + * does not, so a dashed file (canonical or not) is always the better + * answer when both exist. Reached when neither (1) nor (2) found any + * candidate — including when (2)'s scoping filtered every dashed + * candidate out as belonging to some OTHER phase. + * + * #3511 RECONCILIATION with `isPhaseArtifact` (`src/phase-id.cts`): that + * predicate's own docblock used to flag this fallback as an open gap — its + * aggregate scans exclude a cross-phase stray, but this single-pick resolver + * did not, so it could return a stray as THE artifact while the aggregate + * scans correctly ignored it. Closed by scoping step (2) above through + * `scopeToPhase` (`src/phase-id.cts`, itself built on `isPhaseArtifact`): + * `options.phaseDirName` threads the phase directory's basename in, and the + * fallback now filters candidates through `scopeToPhase(candidates, + * phaseDirName)` before picking alphabetically-first. This does NOT reopen + * the #3357 guarantee — that guarantee is "a phase whose only report is + * non-canonically named must keep working", and a non-canonically-named + * artifact of THIS phase still passes `isPhaseArtifact` (it is membership by + * phase number, not by canonical shape), so it is still returned. Only a + * file belonging to a DIFFERENT phase is now excluded — and excluding it is + * correct: returning another phase's artifact as this phase's own is worse + * than reporting none (confidently wrong beats honestly empty). + * The fail-safe now lives entirely inside `isPhaseArtifact`, not in + * `scopeToPhase` (which is a plain filter with no unfiltered fallback): + * (a) when phase-number membership cannot be determined for `phaseDirName` at + * all (no reliable token — the zero-token directory case), every candidate is + * treated as belonging to the phase; (b) the `firstLetterPrefixed` + * bracket-ambiguity case, where a letter-prefixed-decimal dir is + * string-indistinguishable from a bracket-dir token, also includes + * everything rather than guess; (c) a token-less filename (bare + * `${bareName}`) is accepted by directory containment alone. Outside + * those cases, when scoping DOES remove every dashed candidate — a real + * cross-phase stray, or a phase whose own artifact is genuinely absent — the + * fallback below correctly falls through to `allowBare`/`null`: reporting no + * artifact, not another phase's. `options.phaseDirName` omitted entirely skips + * the filter outright (the ternary below), which is unscoped, pre-#3511 + * behavior. * * Pure — takes an already-read directory listing and does no I/O of its own, * so every call site keeps its existing `fsImpl` seam and no-throw contract @@ -364,7 +411,16 @@ function resolvePhaseArtifactFile( const thisPhaseFile = `${options.phaseToken}-${bareName}`; if (candidates.includes(thisPhaseFile)) return thisPhaseFile; } - return candidates[0]; + // #3511: scope the fallback to files that belong to THIS phase, so a + // stray cross-phase file can no longer outrank a return of null. + // `phaseDirName` omitted, or membership undeterminable for it, → + // unscoped `candidates` (pre-#3511 behavior); otherwise strays are + // filtered out, and if that leaves nothing the code falls through to + // `allowBare`/`null` deliberately. + const scoped = options.phaseDirName + ? scopeToPhase(candidates, options.phaseDirName) + : candidates; + if (scoped.length > 0) return scoped[0]; } if (options.allowBare && entries.includes(bareName)) return bareName; return null; @@ -384,9 +440,11 @@ function resolvePhaseArtifactFile( * single resolver both now call (#3473 F2). * * Selection order: see `resolvePhaseArtifactFile` (the shared core this - * delegates to since #3518) — phase-token-pinned, then alphabetically-first - * dashed fallback, then (allowBare only) a bare `VERIFICATION.md`. Behavior - * is byte-identical to the pre-#3518 standalone implementation. + * delegates to since #3518, itself phase-scoped since #3511) — + * phase-token-pinned, then phase-scoped alphabetically-first dashed + * fallback, then (allowBare only) a bare `VERIFICATION.md`. #3518 extracted + * this into the shared core without changing behavior; #3511's + * `phaseDirName` scoping now lives inside that shared core rather than here. */ function resolveVerificationFile( entries: string[], @@ -411,7 +469,11 @@ function resolveVerificationFile( * is consumed downstream by workflows that then read the named file, so a * wrong path routes UAT state from another phase. * - * Deterministic by construction: same answer on every machine. + * Deterministic by construction: same answer on every machine. Phase-scoped + * (#3511): passing `options.phaseDirName` filters the alphabetically-first + * fallback (tier 2) to artifacts that belong to THIS phase — see + * `resolvePhaseArtifactFile` for the full selection order and scoping + * rationale. */ function resolveUatFile( entries: string[], @@ -477,9 +539,11 @@ function findStaleVerificationSummary( const phaseFiles = fsImpl.readdirSync(phaseDir); // #3492: pin selection to THIS phase's own token so a stray cross-phase // or sentinel-numbered canonically-shaped file cannot outrank this - // phase's own (possibly non-canonical) report. - const phaseToken = extractPhaseToken(path.basename(phaseDir)); - const verificationFile = resolveVerificationFile(phaseFiles, { phaseToken }); + // phase's own (possibly non-canonical) report. #3511: phaseDirName scopes + // the fallback path to this same phase (see resolveVerificationFile docs). + const phaseDirName = path.basename(phaseDir); + const phaseToken = extractPhaseToken(phaseDirName); + const verificationFile = resolveVerificationFile(phaseFiles, { phaseToken, phaseDirName }); if (!verificationFile) return { determined: true, stale: false }; const summaryFiles = (scanPhasePlans(phaseDir) as { summaryFiles: string[] }).summaryFiles @@ -522,8 +586,8 @@ function findStaleVerificationSummary( * Behavior: * 1. Find the phase's verification report via `resolveVerificationFile` * (canonical `-VERIFICATION.md` preferred; falls back to the - * alphabetically-first `*-VERIFICATION.md` when none is canonical — #3357). - * If none → status 'missing'. + * alphabetically-first `*-VERIFICATION.md` that belongs to THIS phase when + * none is canonical — #3357/#3511). If none → status 'missing'. * 2. Extract `status` from FRONTMATTER ONLY via the shared extractFrontmatter * parser (DEFECT.FRONTMATTER-SCALAR-BROAD-GREP fix — parser anchors at byte 0). * If no frontmatter block or no `status` key → status 'missing'. @@ -569,8 +633,9 @@ function readVerificationStatus( // #3492: pin selection to THIS phase's own token (already derived above // for the routed command argument) so a stray cross-phase or // sentinel-numbered canonically-shaped file cannot outrank this phase's - // own (possibly non-canonical) report. - verificationFile = resolveVerificationFile(entries, { phaseToken }); + // own (possibly non-canonical) report. #3511: baseName also scopes the + // fallback path to this same phase (see resolveVerificationFile docs). + verificationFile = resolveVerificationFile(entries, { phaseToken, phaseDirName: baseName }); } catch { // Directory unreadable → treat as missing verificationFile = null; @@ -775,8 +840,9 @@ function cmdVerificationResolveFile(cwd: string, phaseDirArg: string | undefined let verificationPath = ''; try { const entries = fs.readdirSync(phaseDir); - const phaseToken = extractPhaseToken(path.basename(phaseDir)); - const verificationFile = resolveVerificationFile(entries, { allowBare: true, phaseToken }); + const phaseDirName = path.basename(phaseDir); + const phaseToken = extractPhaseToken(phaseDirName); + const verificationFile = resolveVerificationFile(entries, { allowBare: true, phaseToken, phaseDirName }); if (verificationFile) { verificationPath = path.join(phaseDir, verificationFile); } diff --git a/tests/audit-command-cutover.test.cjs b/tests/audit-command-cutover.test.cjs index 755d338a7..a7e330709 100644 --- a/tests/audit-command-cutover.test.cjs +++ b/tests/audit-command-cutover.test.cjs @@ -738,6 +738,103 @@ describe('bug #2836: audit-open quick-task summary filename + UAT terminal statu } }); + test('#3511: scanUatGaps excludes a cross-phase stray UAT file sitting in the same phase dir; this phase\'s own open gap still reports', () => { + const cwd = mkTmp(); + try { + const phaseDir = path.join(cwd, '.planning', 'phases', '03-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + // This phase's own open UAT gap. + fs.writeFileSync( + path.join(phaseDir, '03-UAT.md'), + '---\nstatus: pending\n---\nresult: pending\n', + 'utf-8', + ); + // Cross-phase stray in the SAME directory — token "04", not "03". + fs.writeFileSync( + path.join(phaseDir, '04-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, + `only this phase's own gap must report; got: ${JSON.stringify(realUatGaps)}`); + assert.equal(realUatGaps[0].file, '03-UAT.md'); + assert.equal(realUatGaps[0].phase, '03'); + assert.ok(!realUatGaps.some(i => i.file === '04-UAT.md'), + 'the cross-phase stray must not appear in uat_gaps'); + assert.equal(result.counts.uat_gaps, 1); + } finally { + cleanup(cwd); + } + }); + + test('#3511: scanVerificationGaps excludes a cross-phase stray VERIFICATION file sitting in the same phase dir; this phase\'s own open gap still reports', () => { + const cwd = mkTmp(); + try { + const phaseDir = path.join(cwd, '.planning', 'phases', '03-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + // This phase's own open VERIFICATION gap. + fs.writeFileSync( + path.join(phaseDir, '03-VERIFICATION.md'), + '---\nstatus: gaps_found\n---\n# Verification\n', + 'utf-8', + ); + // Cross-phase stray in the SAME directory — token "04", not "03". + fs.writeFileSync( + path.join(phaseDir, '04-VERIFICATION.md'), + '---\nstatus: human_needed\n---\n# Verification\n', + 'utf-8', + ); + + const result = auditOpenArtifacts(cwd); + const realVerificationGaps = result.items.verification_gaps.filter(i => !i.scan_error); + + assert.equal(realVerificationGaps.length, 1, + `only this phase's own gap must report; got: ${JSON.stringify(realVerificationGaps)}`); + assert.equal(realVerificationGaps[0].file, '03-VERIFICATION.md'); + assert.equal(realVerificationGaps[0].phase, '03'); + assert.ok(!realVerificationGaps.some(i => i.file === '04-VERIFICATION.md'), + 'the cross-phase stray must not appear in verification_gaps'); + assert.equal(result.counts.verification_gaps, 1); + } finally { + cleanup(cwd); + } + }); + + test('#3511 follow-up: own gap still reports from a NON-canonical dir shape (over-exclusion check)', () => { + const cwd = mkTmp(); + try { + // "1-unpadded" tokenizes to literal "1", but scaffold writes the PADDED + // "01-…" form — a literal token compare excluded the phase's own file. + const phaseDir = path.join(cwd, '.planning', 'phases', '1-unpadded'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync( + path.join(phaseDir, '01-UAT.md'), + '---\nstatus: partial\n---\n\n## Tests\n\n### 1. Test\nexpected: works\nresult: pending\n', + 'utf-8', + ); + fs.writeFileSync( + path.join(phaseDir, '01-VERIFICATION.md'), + '---\nstatus: gaps_found\n---\n# Verification\n', + 'utf-8', + ); + + const result = auditOpenArtifacts(cwd); + const realUatGaps = result.items.uat_gaps.filter(i => !i.scan_error); + const realVerificationGaps = result.items.verification_gaps.filter(i => !i.scan_error); + + assert.equal(realUatGaps.length, 1, + `own UAT gap in an unpadded-dir phase must still report; got: ${JSON.stringify(realUatGaps)}`); + assert.equal(realVerificationGaps.length, 1, + `own VERIFICATION gap in an unpadded-dir phase must still report; got: ${JSON.stringify(realVerificationGaps)}`); + } finally { + cleanup(cwd); + } + }); + test('quick task without any SUMMARY file is still flagged as missing', () => { const cwd = mkTmp(); try { diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 45e9e2711..a31a97b9a 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -2240,6 +2240,27 @@ describe('stats command', () => { assert.strictEqual(phase.status, 'Complete', 'the canonical 03-VERIFICATION.md must win over the CORRECTION worksheet'); }); + // #3511 BLOCKER-2 regression: a cross-phase stray VERIFICATION.md must not + // resolve as THIS phase's report. Phase 03's directory holds only a + // '04-VERIFICATION.md' (belongs to phase 04); an unscoped resolver would + // pick it up as phase 03's own report and read 'Complete'. + test('#3511: phase status is not Complete off a cross-phase stray VERIFICATION.md', () => { + const p1 = path.join(tmpDir, '.planning', 'phases', '03-test'); + fs.mkdirSync(p1, { recursive: true }); + fs.writeFileSync(path.join(p1, '03-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(p1, '03-01-SUMMARY.md'), '# Summary'); + fs.writeFileSync(path.join(p1, '04-VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification'); + + const result = runGsdTools('stats', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const stats = JSON.parse(result.output); + const phase = stats.phases.find(p => p.number === '03'); + assert.ok(phase, 'phase 03 must be present in stats output'); + assert.notStrictEqual(phase.status, 'Complete', + `phase 03 must not report Complete off phase 04's report; got: ${phase.status}`); + }); + test('counts requirements from REQUIREMENTS.md', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), diff --git a/tests/core-utils.test.cjs b/tests/core-utils.test.cjs index 499f742a6..c7c74f563 100644 --- a/tests/core-utils.test.cjs +++ b/tests/core-utils.test.cjs @@ -485,6 +485,22 @@ describe('getPhaseFileStats', () => { const stats = coreUtils.getPhaseFileStats(tmpDir); assert.deepEqual(stats.plans, ['plans/PLAN-01.md']); }); + + test('#3511 BLOCKER-2 regression: a cross-phase stray VERIFICATION.md is scoped out of hasVerification', () => { + // Dir is phase 03's own directory ("03-test" → token "03"); the only + // VERIFICATION.md present belongs to phase 04. scopeToPhase must exclude + // it, so hasVerification reads false — an unscoped implementation would + // wrongly report phase 03 as verified off phase 04's report. + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cu-test-')); + const phaseDir = path.join(tmpDir, '03-test'); + fs.mkdirSync(phaseDir); + fs.writeFileSync(path.join(phaseDir, '03-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '03-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phaseDir, '04-VERIFICATION.md'), '---\nstatus: passed\n---\n'); + const stats = coreUtils.getPhaseFileStats(phaseDir); + assert.strictEqual(stats.hasVerification, false, + `hasVerification must be false — the only VERIFICATION.md belongs to phase 04, not 03; got scopedFiles-derived: ${stats.hasVerification}`); + }); }); // ─── extractOneLinerFromBody ────────────────────────────────────────────────── diff --git a/tests/frontmatter.test.cjs b/tests/frontmatter.test.cjs index 1455342a9..52eaa5980 100644 --- a/tests/frontmatter.test.cjs +++ b/tests/frontmatter.test.cjs @@ -19,6 +19,8 @@ const { parseMustHavesBlock, } = require('../gsd-core/bin/lib/frontmatter.cjs'); +const { normalizePhaseName } = require('../gsd-core/bin/lib/phase-id.cjs'); + // ─── extractFrontmatter ───────────────────────────────────────────────────── describe('extractFrontmatter', () => { @@ -1236,11 +1238,15 @@ function buildRoadmap(numPhases) { */ function createPhaseDirs(phasesDir, count) { for (let i = 1; i <= count; i++) { - const dir = path.join(phasesDir, String(i).padStart(2, '0')); + const dirName = String(i).padStart(2, '0'); + const dir = path.join(phasesDir, dirName); fs.mkdirSync(dir, { recursive: true }); fs.writeFileSync(path.join(dir, `01-PLAN.md`), `# Plan\n`); fs.writeFileSync(path.join(dir, `01-SUMMARY.md`), `# Summary\n`); - fs.writeFileSync(path.join(dir, `01-VERIFICATION.md`), '---\nstatus: passed\n---\n# Verification\n'); + fs.writeFileSync( + path.join(dir, `${normalizePhaseName(dirName)}-VERIFICATION.md`), + '---\nstatus: passed\n---\n# Verification\n', + ); } } diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 5c08f8510..c450e3d51 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -120,6 +120,91 @@ describe('init commands', () => { assert.strictEqual(output.uat_path, absPlanningPath(tmpDir, 'phases', '03-api', '03-UAT.md')); }); + test('#3511-class: init manager has_context/has_research ignore another phase\'s misplaced artifact', () => { + writePlanningDocs(tmpDir); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '## Progress', + '', + '- [ ] **Phase 1: Setup**', + '- [ ] **Phase 2: API**', + '', + '### Phase 1: Setup', + '', + '**Goal:** Build the foundation.', + '', + '### Phase 2: API', + '', + '**Goal:** Build the API.', + '', + ].join('\n'), + ); + + seedPhase(tmpDir, '01-setup', {}); + // Phase 02's directory holds ONLY a stray artifact whose filename token + // ("01-") belongs to phase 01, not to this directory's own phase (02). + seedPhase(tmpDir, '02-api', { + '01-RESEARCH.md': '# Research for phase 01', + '01-CONTEXT.md': '# Context for phase 01', + }); + + const result = runGsdTools('init manager', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const p2 = output.phases.find((p) => p.number === '2'); + assert.strictEqual(p2.has_research, false, + 'phase 2 must not report has_research from a file that belongs to phase 1'); + assert.strictEqual(p2.has_context, false, + 'phase 2 must not report has_context from a file that belongs to phase 1'); + }); + + test('#3511-class: init verify-work ui_phase_active ignores another phase\'s misplaced UI-SPEC file', () => { + writePlanningDocs(tmpDir); + // ui_phase_active is `hasActiveUiStep || hasUiSpecFile` (detectUiPhaseActive, + // src/init.cts) — the `ui` capability's `workflow.ui_phase` config key + // defaults to `true` (capabilities/ui/capability.json), which alone would + // make `hasActiveUiStep` (and therefore the whole OR) true regardless of + // which file the phase directory holds. Disabling it here isolates the + // signal this test actually exercises: the misplaced-file half of the OR. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ workflow: { ui_phase: false } }), + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '### Phase 1: Setup', + '', + '**Goal:** Build the foundation.', + '', + '### Phase 2: API', + '', + '**Goal:** Build the API.', + '', + ].join('\n'), + ); + + seedPhase(tmpDir, '01-setup', {}); + // Phase 02's directory holds ONLY a stray artifact whose filename token + // ("01-") belongs to phase 01, not to this directory's own phase (02). + seedPhase(tmpDir, '02-api', { + '01-UI-SPEC.md': '# UI Spec for phase 01', + }); + + const result = runGsdTools('init verify-work 2', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.ui_phase_active, false, + 'phase 2 must not report ui_phase_active from a UI-SPEC file that belongs to phase 1'); + }); + // #3473 F2 (companion to #3357): init plan-phase's verification_path // projector now resolves via the shared resolveVerificationFile resolver // instead of a hand-rolled `.find()` over unsorted readdir() order. The @@ -1717,6 +1802,27 @@ describe('cmdInitProgress', () => { assert.strictEqual(output.next_phase, null); }); + test('#3511-class: has_research ignores a misplaced RESEARCH.md that belongs to another phase', () => { + // Phase 01 has no artifacts of its own. + const phase1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phase1, { recursive: true }); + + // Phase 02's directory holds ONLY a stray artifact whose filename token + // ("01-") belongs to phase 01, not to this directory's own phase (02). + const phase2 = path.join(tmpDir, '.planning', 'phases', '02-api'); + fs.mkdirSync(phase2, { recursive: true }); + fs.writeFileSync(path.join(phase2, '01-RESEARCH.md'), '# Research for phase 01'); + + const result = runGsdTools('init progress', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const p2 = output.phases.find(p => p.number === '02'); + assert.strictEqual(p2.has_research, false, + 'phase 02 must not report has_research from a file that belongs to phase 01'); + assert.strictEqual(p2.status, 'pending'); + }); + test('implementation-complete phase without passed verification remains current work', () => { const phase1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); fs.mkdirSync(phase1, { recursive: true }); @@ -3465,6 +3571,25 @@ describe('init section manifest', () => { 'gsd-core/workflows/execute-phase/steps/partial-wave.md', ]); }); + + test('#3511-class: regression-gate (state:has-prior-phases) ignores a misplaced VERIFICATION.md that belongs to another phase', (t) => { + const dir = createTempProject('gsd-3511-hasprior-'); + t.after(() => cleanup(dir)); + // Phase 01 is the one being executed. + seedPhase(dir, '01-widgets', {}); + // Phase 02's directory holds ONLY a stray artifact whose filename + // token ("05-") belongs to phase 05, not to this directory's own + // phase (02) — it must not make phase 02 look like it has its own + // verification report. + seedPhase(dir, '02-other', { '05-VERIFICATION.md': '# Verification for phase 05' }); + + const body = parseOkJson(runExecutePhase(['1'], dir), 'stray-verification'); + + assert.ok(body.section_manifest, 'section_manifest must be present'); + assert.ok(!body.section_manifest.included.includes('regression-gate'), + 'regression-gate must not be included when no OTHER phase has its own verification'); + assert.ok(body.section_manifest.excluded.includes('regression-gate')); + }); }); // ── E44-46: phase argument boundary ──────────────────────────────────── diff --git a/tests/phase-id.test.cjs b/tests/phase-id.test.cjs index 273cc7843..8eaea334d 100644 --- a/tests/phase-id.test.cjs +++ b/tests/phase-id.test.cjs @@ -263,6 +263,268 @@ describe('extractPhaseToken', () => { }); }); +// ─── isPhaseArtifact (#3511) ─────────────────────────────────────────────────── +// +// Predicate for AGGREGATE phase-directory scans (uat-predicate.cts, phase.cts, +// state.cts, uat.cts, audit.cts) — answers "does fileName belong to THIS +// phase" so a stray, cross-phase, or ad-hoc file cannot contribute its status +// to a phase it does not belong to. See src/phase-id.cts's isPhaseArtifact +// docblock for the full contract and fail-safe rationale. + +describe('isPhaseArtifact (#3511)', () => { + test('plain: file matches its own phase dir, not a cross-phase dir', () => { + assert.strictEqual(phaseId.isPhaseArtifact('03-VERIFICATION.md', '03-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('04-VERIFICATION.md', '03-foo'), false); + }); + + test('ad-hoc worksheet: 03-CORRECTION-VERIFICATION.md belongs to 03-foo (it is the phase\'s own file)', () => { + // Over-exclusion here would be a worse bug than #3511 itself — a phase's + // own ad-hoc worksheet must never be treated as a stray. + assert.strictEqual(phaseId.isPhaseArtifact('03-CORRECTION-VERIFICATION.md', '03-foo'), true); + }); + + test('letter suffix: token must match exactly, in both directions', () => { + assert.strictEqual(phaseId.isPhaseArtifact('03A-VERIFICATION.md', '03A-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('03-VERIFICATION.md', '03A-foo'), false, + 'a bare "03-" file must not belong to letter-suffixed phase dir "03A-foo"'); + assert.strictEqual(phaseId.isPhaseArtifact('03A-VERIFICATION.md', '03-foo'), false, + 'a letter-suffixed "03A-" file must not belong to bare phase dir "03-foo"'); + }); + + test('decimal sub-phase: token must match exactly, not as a numeric prefix', () => { + assert.strictEqual(phaseId.isPhaseArtifact('35.1-VERIFICATION.md', '35.1-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('35.10-VERIFICATION.md', '35.1-foo'), false, + '35.10-… must not match phase dir 35.1-foo despite the shared numeric prefix'); + }); + + test('plan/summary shapes belong to their phase the same way VERIFICATION/UAT do', () => { + assert.strictEqual(phaseId.isPhaseArtifact('03-01-SUMMARY.md', '03-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('03-01-PLAN.md', '03-foo'), true); + }); + + test('project-code-prefixed dirs: both the prefixed and the project-code-STRIPPED reading belong (#3511 blocker 1)', () => { + // Files are named by normalizePhaseName, which STRIPS the project code + // (cmdScaffold, src/commands.cts) — files never carry the code, only the + // directory does. `01-VERIFICATION.md` IS the real report cmdScaffold + // writes into `CK-01-foundation`; excluding it locked in the #3511 + // follow-up defect (this assertion's polarity was inverted pre-fix). + assert.strictEqual(phaseId.isPhaseArtifact('CK-01-VERIFICATION.md', 'CK-01-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('01-VERIFICATION.md', 'CK-01-foo'), true, + 'the project-code-STRIPPED file is the real scaffold output and must belong to its own project-code-prefixed dir'); + assert.strictEqual(phaseId.isPhaseArtifact('02-VERIFICATION.md', 'CK-01-foo'), false, + 'a different phase number must still be excluded, project code aside'); + }); + + test('#3511 blocker 2: unpadded dir "1-unpadded" — its own padded file belongs', () => { + // #3511 blocker: dir "1-unpadded" has literal token "1"; cmdScaffold + // writes the PADDED "01-VERIFICATION.md" into it via normalizePhaseName. + assert.strictEqual(phaseId.isPhaseArtifact('01-VERIFICATION.md', '1-unpadded'), true); + assert.strictEqual(phaseId.isPhaseArtifact('02-VERIFICATION.md', '1-unpadded'), false); + }); + + test('#3511 blocker 3: digit-leading-slug family — own file uses the LEADING digit run, not the mis-absorbed token', () => { + // "05-80-20-cleanup" tokenizes to "05-80-20" (#2528 mis-absorption), but + // cmdScaffold writes "05-UAT.md"/"05-VERIFICATION.md" (leading digit run + // only) — the same reading matchPhaseDirs' bare-integer fallback uses. + assert.strictEqual(phaseId.isPhaseArtifact('05-UAT.md', '05-80-20-cleanup'), true); + assert.strictEqual(phaseId.isPhaseArtifact('05-VERIFICATION.md', '05-80-20-cleanup'), true); + assert.strictEqual(phaseId.isPhaseArtifact('80-VERIFICATION.md', '05-80-20-cleanup'), false, + 'the slug word "80" must not be mistaken for a real phase number'); + assert.strictEqual(phaseId.isPhaseArtifact('10-UAT.md', '10-24-7-autonomy'), true); + assert.strictEqual(phaseId.isPhaseArtifact('24-UAT.md', '10-24-7-autonomy'), false); + }); + + test('case-insensitive letter suffix (review item 8): "03A-VERIFICATION.md" belongs to lowercase-suffixed "03a-foo"', () => { + assert.strictEqual(phaseId.isPhaseArtifact('03A-VERIFICATION.md', '03a-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('03a-VERIFICATION.md', '03A-foo'), true); + }); + + // WARNING-2 (#3511 review): the `firstLetterPrefixed` include-everything + // branch (bracket-convention ambiguity fail-safe, docblock "BRACKET + // CONVENTION" section) was untested on its own — only the ZERO-SEGMENT + // fail-safe below had direct coverage. Pinned here so a later flip to + // `false` for this family is caught rather than silently shipped. + test('#3511 WARNING-2: bracket-ambiguous letter-prefixed-decimal dirs (firstLetterPrefixed) are the deliberate include-everything fail-safe', () => { + assert.strictEqual(phaseId.isPhaseArtifact('99-VERIFICATION.md', 'P0.3-2-slug'), true); + assert.strictEqual(phaseId.isPhaseArtifact('07-UAT.md', 'v2-migration'), true); + }); + + // WARNING-5 (#3511 review): pin actual behavior for a decimal sub-phase + // token compared against a same-leading-number-but-different-sub-phase + // artifact — the exact-token-match rule (not a numeric-prefix match). + test('#3511 WARNING-5: "35-VERIFICATION.md" (bare, no sub-phase) does not belong to decimal sub-phase dir "35.1-slug"', () => { + assert.strictEqual(phaseId.isPhaseArtifact('35-VERIFICATION.md', '35.1-slug'), false); + }); + + test('fail-safe: a dir name with no derivable token includes EVERY file (never exclude)', () => { + // derivePhaseTokenSegments finds zero segments for these dir names (same + // condition extractPhaseToken treats as "return dirName unchanged" — + // see the 'returns the full dirName when no numeric token found' test + // above). Excluding on an unreliable token would make an aggregate gate + // silently permissive in the wrong direction (dropping the phase's own + // real blockers) — worse than the cross-phase-contamination bug #3511 + // fixes. Every file must be treated as belonging to the phase instead. + assert.strictEqual(phaseId.isPhaseArtifact('04-VERIFICATION.md', 'no-numeric'), true); + assert.strictEqual(phaseId.isPhaseArtifact('anything-at-all.md', 'alpha'), true); + assert.strictEqual(phaseId.isPhaseArtifact('99-UAT.md', 'phase-name-01'), true); + }); + + test('#3511 Fix 2: a bare "VERIFICATION.md"/"UAT.md" (no dash, no token of its own) belongs by containment', () => { + // src/state.cts:3740's S006 filter matches `f.includes('VERIFICATION')` + // with no dash requirement, so a bare `VERIFICATION.md` (a form + // core-utils.cts, init.cts and verification.cts all treat as valid) was + // silently excluded pre-fix — S006 drift detection lost, S007 wrongly + // flipped on. Directory containment is the only signal for a token-less + // file, and it is sufficient. + assert.strictEqual(phaseId.isPhaseArtifact('VERIFICATION.md', '03-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('UAT.md', '03-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('VERIFICATION.md', 'CK-01-foo'), true); + assert.strictEqual(phaseId.isPhaseArtifact('VERIFICATION.md', '1-unpadded'), true); + }); + + // ── Property: over-exclusion (#3511 follow-up) ─────────────────────────────── + // + // Every "own file still contributes" assertion above uses a HAND-PICKED + // fixture. This property generates across the real dirName shapes (padded / + // unpadded, project-code-prefixed, digit-leading slugs, decimals, letter + // suffixes) and asserts, for EACH, that the artifact name `cmdScaffold` + // (src/commands.cts, via `normalizePhaseName`) would actually write into + // that directory is a member — the exact invariant all three #3511 + // follow-up blockers violated, and the one no hand-picked fixture pins on + // its own. + test('property: the file cmdScaffold would write into a phase dir is always isPhaseArtifact-true for that dir', () => { + const artifactType = fc.constantFrom('UAT', 'VERIFICATION', 'CONTEXT'); + + // dirName shape generators mirroring the real on-disk families this + // module's own docblocks (extractPhaseToken, matchPhaseDirs) enumerate. + const letterSuffix = fc.constantFrom('', 'A', 'B', 'C'); + const projectCode = fc.constantFrom(null, 'CK', 'PROJ', 'APP1'); + const slugWord = fc.constantFrom('foo', 'cleanup', 'autonomy', 'follow-up'); + // Digit-leading-slug family (#2528): a second all-digit slug SEGMENT that + // is NOT a genuine sub-phase — 2-3 digit words like "80", "100". + const digitSlugSegment = fc.constantFrom(null, '80', '20', '100'); + + const plainDirNameGen = fc.record({ + code: projectCode, + padded: fc.boolean(), + num: fc.integer({ min: 1, max: 99 }), + letter: letterSuffix, + digitSlug: digitSlugSegment, + slug: slugWord, + }).map(({ code, padded, num, letter, digitSlug, slug }) => { + const numStr = padded ? String(num).padStart(2, '0') : String(num); + const token = `${numStr}${letter}`; + const codePrefix = code ? `${code}-` : ''; + const digitTail = digitSlug ? `-${digitSlug}` : ''; + return { dirName: `${codePrefix}${token}${digitTail}-${slug}`, phase: token }; + }); + + // INFO-2 (#3511 review): a genuine decimal sub-phase dir (`05.3-slug`) — + // the exact-token-match branch, not the digit-leading-slug fallback. + const decimalDirNameGen = fc.record({ + code: projectCode, + padded: fc.boolean(), + major: fc.integer({ min: 1, max: 99 }), + sub: fc.integer({ min: 1, max: 99 }), + slug: slugWord, + }).map(({ code, padded, major, sub, slug }) => { + const majorStr = padded ? String(major).padStart(2, '0') : String(major); + const token = `${majorStr}.${sub}`; + const codePrefix = code ? `${code}-` : ''; + return { dirName: `${codePrefix}${token}-${slug}`, phase: token }; + }); + + // INFO-2 (#3511 review): the letter-prefixed-decimal family (`P0.3-2-slug`) + // — string-indistinguishable from a bracket-dir token without an explicit + // convention signal, so `isPhaseArtifact` treats it as the deliberate + // `firstLetterPrefixed` include-everything fail-safe (see WARNING-2 test + // above): every candidate file belongs, by construction. + const letterPrefixedDecimalDirNameGen = fc.record({ + prefix: fc.constantFrom('P0', 'M1', 'A2'), + major: fc.integer({ min: 1, max: 20 }), + sub: fc.integer({ min: 1, max: 20 }), + slug: slugWord, + }).map(({ prefix, major, sub, slug }) => ({ + dirName: `${prefix}.${major}-${sub}-${slug}`, + phase: `${major}-${sub}`, + })); + + const dirNameGen = fc.oneof(plainDirNameGen, decimalDirNameGen, letterPrefixedDecimalDirNameGen); + + fc.assert( + fc.property(dirNameGen, artifactType, ({ dirName, phase }, type) => { + const padded = phaseId.normalizePhaseName(phase); + const writtenFile = `${padded}-${type}.md`; + return phaseId.isPhaseArtifact(writtenFile, dirName); + }), + { numRuns: 200 }, + ); + }); +}); + +// ─── scopeToPhase (#3511) ─────────────────────────────────────────────────── +// +// scopeToPhase is a plain filter over isPhaseArtifact: `fileNames.filter(f => +// isPhaseArtifact(f, phaseDirName))`. An earlier follow-up ("WARNING 4") added +// a rule that fell back to the unfiltered input whenever scoping would empty +// a non-empty candidate set — but that defeated the actual #3511 fix: a phase +// dir holding only a misfiled cross-phase report would publish that other +// phase's status as its own. The fallback rule has been REMOVED; an empty +// result is now the honest answer for "this phase has none of its own". +describe('scopeToPhase (#3511)', () => { + test('mixed set: own file kept, stray dropped', () => { + assert.deepStrictEqual( + phaseId.scopeToPhase(['03-VERIFICATION.md', '04-VERIFICATION.md'], '03-foo'), + ['03-VERIFICATION.md'], + ); + }); + + test('empty input: returns empty', () => { + assert.deepStrictEqual(phaseId.scopeToPhase([], '03-foo'), []); + }); + + test('underivable dir token: every file passes through unchanged (isPhaseArtifact fail-safe)', () => { + assert.deepStrictEqual( + phaseId.scopeToPhase(['04-VERIFICATION.md', 'anything-at-all.md'], 'no-numeric'), + ['04-VERIFICATION.md', 'anything-at-all.md'], + ); + }); + + test('#3511: a phase dir holding ONLY another phase\'s report scopes to empty — ' + + 'an empty result is the honest answer, not a reason to fall back to the stray', () => { + // Removing this behavior is the whole point of #3511: returning + // 04-VERIFICATION.md here would publish phase 04's status as phase 03's. + assert.deepStrictEqual( + phaseId.scopeToPhase(['04-VERIFICATION.md'], '03-foo'), + [], + ); + }); + + test('property: a phase dir never scopes away its own canonically-named artifact', () => { + fc.assert( + fc.property( + fc.integer({ min: 0, max: 999 }), + fc.stringMatching(/^[a-z]{1,8}$/), + (num, slug) => { + const padded = String(num).padStart(2, '0'); + for (const dirNum of new Set([padded, String(num)])) { + const dirName = `${dirNum}-${slug}`; + for (const fileNum of new Set([padded, String(num)])) { + const own = `${fileNum}-VERIFICATION.md`; + assert.deepStrictEqual( + phaseId.scopeToPhase([own], dirName), + [own], + `${own} is phase ${dirNum}'s own artifact in ${dirName} and must survive scoping`, + ); + } + } + }, + ), + { numRuns: 300 }, + ); + }); +}); + // ─── phaseTokenMatches ──────────────────────────────────────────────────────── describe('phaseTokenMatches', () => { diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index bc9e42639..b9648d1f3 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2473,6 +2473,119 @@ describe('phase remove command', () => { assert.ok(roadmap.includes('Phase 2: Features'), 'phase 3 should be renumbered to 2'); }); + // WARNING-3 (#3511 review): renameIntegerPhases renames a phase directory + // whose leading number is UNPADDED (dir regex accepts bare `\d+`), but + // renamed its artifact files only against the 2-PADDED prefix, so an + // unpadded-numbered artifact desynced from its now-renamed directory and + // the phase read `missing` afterward. + test('#3511 WARNING-3: renames an unpadded-numbered artifact alongside its unpadded dir', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n### Phase 1: A\n**Goal:** A\n### Phase 9: B\n**Goal:** B\n` + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-a'), { recursive: true }); + const p9 = path.join(tmpDir, '.planning', 'phases', '9-slug'); + fs.mkdirSync(p9, { recursive: true }); + // Unpadded artifact filename, paired with the unpadded dir number. + fs.writeFileSync(path.join(p9, '9-VERIFICATION.md'), '---\nstatus: passed\n---\n\nVerified OK.'); + + const result = runGsdTools('phase remove 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const newDir = path.join(tmpDir, '.planning', 'phases', '08-slug'); + assert.ok(fs.existsSync(newDir), 'phase 9 should be renumbered to 08-slug'); + assert.ok( + fs.existsSync(path.join(newDir, '08-VERIFICATION.md')), + 'the unpadded 9-VERIFICATION.md should be renamed to 08-VERIFICATION.md alongside the dir' + ); + assert.ok( + !fs.existsSync(path.join(newDir, '9-VERIFICATION.md')), + 'the stale unpadded-prefix file should no longer exist' + ); + + const findResult = runGsdTools('find-phase 8', tmpDir); + assert.ok(findResult.success, `find-phase failed: ${findResult.error}`); + const findOutput = JSON.parse(findResult.output); + assert.strictEqual(findOutput.found, true, 'renumbered phase 8 should resolve'); + }); + + // #3511 BLOCKER-2 follow-up (security-review regression): the collision + // guard added to stop the original overwrite-and-lose-data bug (BLOCKER-1) + // introduced a NEW wrong-answer regression — skipping the rename let a + // STRAY cross-phase file outrank the phase's own report at the canonical + // name. Phase 9's directory holds its OWN `09-VERIFICATION.md` (gaps_found) + // and a stray `08-VERIFICATION.md` (passed) that actually belongs to phase + // 8. Removing phase 8 renumbers phase 9 -> phase 8 and must displace the + // stray (never overwrite it, never let it win) so the phase's own report + // lands at the canonical name. + test('#3511 BLOCKER-2 follow-up: collision displaces the occupying file instead of skip-or-overwrite', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n### Phase 8: A\n**Goal:** A\n### Phase 9: B\n**Goal:** B\n` + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '08-a'), { recursive: true }); + const p9 = path.join(tmpDir, '.planning', 'phases', '09-foo'); + fs.mkdirSync(p9, { recursive: true }); + // The phase's OWN report. + fs.writeFileSync( + path.join(p9, '09-VERIFICATION.md'), + '---\nstatus: gaps_found\n---\n\nOwn report for phase 9.' + ); + // A STRAY report belonging to phase 8, sitting inside phase 9's directory. + fs.writeFileSync( + path.join(p9, '08-VERIFICATION.md'), + '---\nstatus: passed\n---\n\nStray report belonging to phase 8.' + ); + + const result = runGsdTools('phase remove 8', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + const newDir = path.join(tmpDir, '.planning', 'phases', '08-foo'); + assert.ok(fs.existsSync(newDir), 'phase 9 should be renumbered to 08-foo'); + + // (a) no file is lost — both original contents still exist on disk. + const canonicalPath = path.join(newDir, '08-VERIFICATION.md'); + const displacedPath = path.join(newDir, '08-VERIFICATION.md.orphaned'); + assert.ok(fs.existsSync(canonicalPath), 'canonical 08-VERIFICATION.md must exist'); + assert.ok(fs.existsSync(displacedPath), 'the displaced stray file must still exist on disk'); + const canonical = fs.readFileSync(canonicalPath, 'utf-8'); + const displaced = fs.readFileSync(displacedPath, 'utf-8'); + assert.ok( + displaced.includes('Stray report belonging to phase 8'), + 'displaced file must retain the stray content' + ); + + // (b) 08-VERIFICATION.md in the renamed dir is the phase's OWN former + // 09-VERIFICATION.md — assert on its body/status, not just its name. + assert.ok( + canonical.includes('status: gaps_found'), + `canonical 08-VERIFICATION.md must be the phase's own report (gaps_found), ` + + `not the stray (passed). Got: ${canonical}` + ); + assert.ok( + canonical.includes('Own report for phase 9'), + 'canonical file body must be the phase\'s own former 09-VERIFICATION.md content' + ); + assert.ok( + !fs.existsSync(path.join(newDir, '09-VERIFICATION.md')), + 'the stale 09-VERIFICATION.md name should no longer exist' + ); + + // (c) the displacement is reported in the command output. + assert.ok( + Array.isArray(output.renamed_file_collisions), + 'renamed_file_collisions must be present in the output' + ); + const entry = output.renamed_file_collisions.find((c) => c.to === '08-VERIFICATION.md'); + assert.ok( + entry, + `expected a collision entry for 08-VERIFICATION.md, got: ${JSON.stringify(output.renamed_file_collisions)}` + ); + assert.strictEqual(entry.from, '09-VERIFICATION.md'); + assert.strictEqual(entry.displaced_to, '08-VERIFICATION.md.orphaned'); + }); + test('rejects removal of phase with summaries unless --force', () => { const p1 = path.join(tmpDir, '.planning', 'phases', '01-test'); fs.mkdirSync(p1, { recursive: true }); @@ -8777,7 +8890,7 @@ function writePassedVerificationFile(phaseDir, phase = '01') { * - Phase 01 directory with one plan+summary (to satisfy phase complete guard) * - Phase 02 directory (next phase) */ -function createFixture(prefix = 'gsd-4-regression-') { +function createFixture(prefix = 'gsd-4-regression-', phase01DirName = '01-foundation') { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); const planningDir = path.join(tmpDir, '.planning'); const phasesDir = path.join(planningDir, 'phases'); @@ -8845,7 +8958,7 @@ function createFixture(prefix = 'gsd-4-regression-') { fs.writeFileSync(path.join(planningDir, 'STATE.md'), state); // Phase 01 directory with a PLAN and SUMMARY so phase complete guard passes - const phase01Dir = path.join(phasesDir, '01-foundation'); + const phase01Dir = path.join(phasesDir, phase01DirName); fs.mkdirSync(phase01Dir, { recursive: true }); fs.writeFileSync(path.join(phase01Dir, '01-01-PLAN.md'), '# Plan 1\nDo the work.\n'); fs.writeFileSync(path.join(phase01Dir, '01-01-SUMMARY.md'), '# Summary 1\nDone.\n'); @@ -9231,6 +9344,87 @@ describe('#3057 B3: cmdPhaseComplete — verification staleness-check indetermin ); }); +// ───────────────────────────────────────────────────────────────────────────── +// #3511: cmdPhaseComplete — UAT/VERIFICATION advisory pre-scan is phase-scoped +// +// A cross-phase, stray, or ad-hoc file sitting in this phase's directory must +// not name an advisory warning against this phase — and this phase's own +// UAT/VERIFICATION files must keep warning exactly as before (non-stray case +// unchanged). Covers BOTH loops scoped by #3511 (the UAT loop and the +// VERIFICATION loop). +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3511: cmdPhaseComplete — advisory pre-scan warnings are phase-scoped', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createFixture(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('a cross-phase stray UAT/VERIFICATION file does not contribute a warning; this phase\'s own files still do', (t) => { + const phase01Dir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + + // This phase's own UAT file — must still produce its usual warning. + fs.writeFileSync(path.join(phase01Dir, '01-UAT.md'), [ + '---', 'status: partial', '---', '', + '### 1. Test A', 'expected: A', 'result: pending', '', + ].join('\n')); + + // Cross-phase strays sitting in phase 01's directory — token "02", not + // "01" — must NOT contribute a warning to phase 01's completion. + fs.writeFileSync(path.join(phase01Dir, '02-UAT.md'), [ + '---', 'status: partial', '---', '', + '### 1. Test B', 'expected: B', 'result: blocked', '', + ].join('\n')); + fs.writeFileSync(path.join(phase01Dir, '02-VERIFICATION.md'), [ + '---', 'status: human_needed', '---', '', + '# Verification', '', + ].join('\n')); + + const output = JSON.parse(capturePhaseComplete(t, tmpDir, '1')); + + assert.strictEqual(output.completed_phase, '1'); + assert.ok( + output.warnings.some((w) => w.includes('01-UAT.md') && w.includes('has pending tests')), + `own UAT file must still warn; got: ${JSON.stringify(output.warnings)}`, + ); + assert.ok( + !output.warnings.some((w) => w.includes('02-UAT.md')), + `stray UAT file must not contribute a warning; got: ${JSON.stringify(output.warnings)}`, + ); + assert.ok( + !output.warnings.some((w) => w.includes('02-VERIFICATION.md') || /needs human verification/.test(w)), + `stray VERIFICATION file must not contribute a warning; got: ${JSON.stringify(output.warnings)}`, + ); + }); + + test('#3511 follow-up: own UAT file still warns from a NON-canonical dir shape "1-unpadded" (over-exclusion check)', (t) => { + const unpaddedTmpDir = createFixture('gsd-4-regression-unpadded-', '1-unpadded'); + try { + const phaseDir = path.join(unpaddedTmpDir, '.planning', 'phases', '1-unpadded'); + // "1-unpadded" tokenizes to literal "1"; scaffold writes the PADDED + // "01-…" form. A literal token compare excluded the phase's own file. + fs.writeFileSync(path.join(phaseDir, '01-UAT.md'), [ + '---', 'status: partial', '---', '', + '### 1. Test A', 'expected: A', 'result: pending', '', + ].join('\n')); + + const output = JSON.parse(capturePhaseComplete(t, unpaddedTmpDir, '1')); + + assert.ok( + output.warnings.some((w) => w.includes('01-UAT.md') && w.includes('has pending tests')), + `own UAT file in an unpadded-dir phase must still warn; got: ${JSON.stringify(output.warnings)}`, + ); + } finally { + cleanup(unpaddedTmpDir); + } + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Regressions: phase complete preserves completion date (#1161) // Tests drive the REAL handler (cmdPhaseComplete) via the CLI entry point diff --git a/tests/planning-snapshot.test.cjs b/tests/planning-snapshot.test.cjs index 74bddc183..69bc8fec1 100644 --- a/tests/planning-snapshot.test.cjs +++ b/tests/planning-snapshot.test.cjs @@ -43,6 +43,7 @@ const { worstScope } = planningSnapshotLib; const { SCOPE } = require('../gsd-core/bin/lib/planning-scope.cjs'); const { _unusableInputEmissionCountForTests } = require('../gsd-core/bin/lib/unusable-input.cjs'); +const { normalizePhaseName } = require('../gsd-core/bin/lib/phase-id.cjs'); // Phase 11 (#3309) additions — agent-install fixture helper mirrors // tests/agent-install-check.test.cjs's own EXPECTED_AGENTS/createCompleteAgents @@ -107,9 +108,10 @@ function makeDirUnreadableAsFile(fullPath) { // on a "healthy" phase (Phase 12, #3310) get a genuinely clean baseline // rather than a false positive from a plan that predates the `wave:` field. function makeCompletePhaseDir(cwd, relPhaseDir) { + const phaseNum = normalizePhaseName(path.basename(relPhaseDir)); writeFile(cwd, `${relPhaseDir}/01-01-PLAN.md`, '---\nwave: 1\n---\n\n# Plan\n'); writeFile(cwd, `${relPhaseDir}/01-01-SUMMARY.md`, '# Summary\n'); - writeFile(cwd, `${relPhaseDir}/01-VERIFICATION.md`, '---\nstatus: passed\n---\n'); + writeFile(cwd, `${relPhaseDir}/${phaseNum}-VERIFICATION.md`, '---\nstatus: passed\n---\n'); } function buildHealthyTwoPhaseFixture(cwd) { @@ -955,6 +957,24 @@ describe('researchValidationStatus field (Phase 11, #3309)', () => { assert.deepStrictEqual(entry, { dir: '01-foo', hasValidationArchitecture: false, hasValidationMd: false }); }); + test('#3511-class: a phase dir holding only ANOTHER phase\'s -RESEARCH.md/-VALIDATION.md does not set this phase\'s flags', (t) => { + const cwd = createTempDir('gsd-3511-rvs4-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, ['## v1.0 Current 🚧', '', '### Phase 1: Foo', '', '### Phase 2: Bar'].join('\n')); + // Phase 01's directory is empty. Phase 02's directory holds ONLY stray + // artifacts whose filename token ("01-") belongs to phase 01, not to + // this directory's own phase (02). + fs.mkdirSync(path.join(planningDirOf(cwd), 'phases', '01-foo'), { recursive: true }); + writeFile(cwd, '.planning/phases/02-bar/01-RESEARCH.md', '# Research\n\n## Validation Architecture\n\ntext\n'); + writeFile(cwd, '.planning/phases/02-bar/01-VALIDATION.md', '# Validation\n'); + + const snap = buildPlanningSnapshot(cwd); + const entry = snap.researchValidationStatus.value.find((r) => r.dir === '02-bar'); + assert.deepStrictEqual(entry, { dir: '02-bar', hasValidationArchitecture: false, hasValidationMd: false }, + 'phase 2 must not report validation status from a file that belongs to phase 1'); + }); + test('hostile: an unreadable phase directory degrades that entry to false/false without throwing', (t) => { const cwd = createTempDir('gsd-3309-rvs3-'); t.after(() => cleanup(cwd)); diff --git a/tests/planning-workspace.test.cjs b/tests/planning-workspace.test.cjs index e42c0e781..9e4acc634 100644 --- a/tests/planning-workspace.test.cjs +++ b/tests/planning-workspace.test.cjs @@ -370,7 +370,7 @@ describe('withPlanningLock PID-liveness staleness + EEXIST safety (audit M1+M2)' __foldDescribe("folded:bug-3739-gap-checker-padded-prefix-context (consolidation epic #1969 B3 #1972)", () => { /** * Bug #3739: gap-analysis silently skips CONTEXT.md decisions when the file - * uses the padded-prefix convention (e.g. 01-CONTEXT.md, 02.1-CONTEXT.md). + * uses the padded-prefix convention (e.g. 01-CONTEXT.md, 01.1-CONTEXT.md). * * Verifies: * 1. Padded-prefix CONTEXT.md (NN-CONTEXT.md) decisions ARE included in the @@ -481,10 +481,10 @@ describe('bug #3739 — gap-analysis padded-prefix CONTEXT.md', () => { assert.strictEqual(d05.status, 'Covered', 'D-05 must be Covered'); }); - // ── Test 4: deeper padded prefix (02.1-CONTEXT.md) ─────────────────────── + // ── Test 4: deeper padded prefix (01.1-CONTEXT.md) ─────────────────────── - test('multi-segment padded prefix (02.1-CONTEXT.md) decisions appear in gap report', () => { - writeContextAs('02.1-CONTEXT.md', [ + test('multi-segment padded prefix (01.1-CONTEXT.md) decisions appear in gap report', () => { + writeContextAs('01.1-CONTEXT.md', [ { id: 'D-03', text: 'Use postgres' }, ]); writePlan('01', '# Plan\n\nImplements D-03.\n'); @@ -494,7 +494,7 @@ describe('bug #3739 — gap-analysis padded-prefix CONTEXT.md', () => { const out = JSON.parse(r.output); const d03 = out.rows.find(x => x.item === 'D-03'); - assert.ok(d03, 'D-03 must appear from 02.1-CONTEXT.md'); + assert.ok(d03, 'D-03 must appear from 01.1-CONTEXT.md'); assert.strictEqual(d03.status, 'Covered'); }); diff --git a/tests/post-planning-gaps-2493.test.cjs b/tests/post-planning-gaps-2493.test.cjs index deeb0a35d..6334c02b6 100644 --- a/tests/post-planning-gaps-2493.test.cjs +++ b/tests/post-planning-gaps-2493.test.cjs @@ -312,6 +312,26 @@ describe('gap-analysis CLI (#2493)', () => { assert.strictEqual(out.rows[0].source, 'REQUIREMENTS.md'); }); + test('#3511-class: a phase dir holding only ANOTHER phase\'s -CONTEXT.md is treated as CONTEXT-missing', () => { + // phaseDir is '01-test' for this describe block's own scenarios; use a + // SEPARATE phase 02 directory that holds ONLY a stray artifact whose + // filename token ("01-") belongs to phase 01, not to this directory's + // own phase (02). + const otherPhaseDir = path.join(tmpDir, '.planning', 'phases', '02-other'); + fs.mkdirSync(otherPhaseDir, { recursive: true }); + fs.writeFileSync( + path.join(otherPhaseDir, '01-CONTEXT.md'), + '# Phase Context\n\n\n## Implementation Decisions\n\n- **D-01:** foo\n\n', + ); + fs.writeFileSync(path.join(otherPhaseDir, '02-PLAN.md'), '# Plan mentioning D-01\n'); + + const r = runGsdTools(['gap-analysis', '--phase-dir', otherPhaseDir], tmpDir); + assert.ok(r.success, r.error); + const out = JSON.parse(r.output); + assert.deepStrictEqual(out.rows, [], + 'phase 02 must not report a CONTEXT.md decision that belongs to phase 01\'s file'); + }); + test('both REQUIREMENTS.md and CONTEXT.md missing → no error, empty rows', () => { writePlan('01', '# Plan\n'); const r = runGsdTools(['gap-analysis', '--phase-dir', phaseDir], tmpDir); diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index 7ad7afdb9..336eaef96 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -401,6 +401,39 @@ describe('roadmap analyze disk status variants', () => { assert.strictEqual(output.phases[0].has_research, true, 'has_research should be true'); }); + test('#3511-class: roadmap analyze has_research/has_context ignore another phase\'s misplaced artifact', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 1: Setup +**Goal:** Build the foundation + +### Phase 2: API +**Goal:** Build the API +` + ); + + const p1 = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(p1, { recursive: true }); + // Phase 02's directory holds ONLY a stray artifact whose filename token + // ("01-") belongs to phase 01, not to this directory's own phase (02). + const p2 = path.join(tmpDir, '.planning', 'phases', '02-api'); + fs.mkdirSync(p2, { recursive: true }); + fs.writeFileSync(path.join(p2, '01-RESEARCH.md'), '# Research for phase 01'); + fs.writeFileSync(path.join(p2, '01-CONTEXT.md'), '# Context for phase 01'); + + const result = runGsdTools('roadmap analyze', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const phase2 = output.phases.find((p) => p.number === '2'); + assert.strictEqual(phase2.has_research, false, + 'phase 2 must not report has_research from a file that belongs to phase 1'); + assert.strictEqual(phase2.has_context, false, + 'phase 2 must not report has_context from a file that belongs to phase 1'); + }); + test('returns discussed status for phase dir with only CONTEXT.md', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'), diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 80ef1fe66..04c6414fa 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -4135,6 +4135,122 @@ describe('#3310 state validate — S0NN coded diagnostics', () => { assertNoDriftKey(output); }); + test('#3511: S006 does not fire from a cross-phase stray VERIFICATION file; this phase\'s own file still fires it', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 1\n**Current Plan:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + // No summary — isolates this test from the S007 "all plans summarized" path. + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + // Cross-phase stray sitting in phase 01's directory — token "02", not "01". + fs.writeFileSync(path.join(phaseDir, '02-VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s006Warnings = (output.warnings || []).filter((w) => w.code === 'S006'); + assert.strictEqual(s006Warnings.length, 1, + `exactly one S006 must fire (this phase's own file only); got: ${JSON.stringify(s006Warnings)}`); + assert.ok(s006Warnings[0].message.includes('01-VERIFICATION.md'), + `S006 must name this phase's own file; got: ${s006Warnings[0].message}`); + assert.ok(!s006Warnings[0].message.includes('02-VERIFICATION.md'), + `S006 must not name the cross-phase stray; got: ${s006Warnings[0].message}`); + assertNoDriftKey(output); + }); + + test('#3511: S007 fires when the only VERIFICATION.md present is a cross-phase stray (unscoped counting would have wrongly suppressed it)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 2\n**Current Plan:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-SUMMARY.md'), '# Summary\n'); + // No own VERIFICATION.md — only a cross-phase stray (token "02"). Scoped + // counting must treat this phase as having NO verification file, so S007 + // ("all plans summarized but still executing") must still fire. + fs.writeFileSync(path.join(phaseDir, '02-VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s007 = findWarning(output, 'S007'); + assert.ok(s007, + 'S007 must fire: the only VERIFICATION.md present belongs to a different phase, so this phase has none of its own'); + assert.ok(!findWarning(output, 'S006'), 'the cross-phase stray must not fire S006 either'); + assertNoDriftKey(output); + }); + + test('#3511 follow-up: S006 still fires from a NON-canonical dir shape "1-unpadded" (over-exclusion check)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 1\n**Current Plan:** 1\n`, + ); + // "1-unpadded" tokenizes to literal "1"; scaffold writes the PADDED + // "01-VERIFICATION.md" form. A literal token compare excluded it. + const phaseDir = path.join(tmpDir, '.planning', 'phases', '1-unpadded'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, `S006 must still fire for the phase's own file in a non-canonical dir; got: ${JSON.stringify(output.warnings)}`); + assert.ok(s006.message.includes('01-VERIFICATION.md')); + assertNoDriftKey(output); + }); + + test('#3511 Fix 2: S006 fires from a bare "VERIFICATION.md" (no dash, no token of its own)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 1\n**Current Plan:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + // Bare form (no dash) — core-utils.cts, init.cts and verification.cts all + // treat this as a valid report; a literal `-` prefix check can + // never match it, silently losing S006 drift detection. + fs.writeFileSync(path.join(phaseDir, 'VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, `S006 must fire for a bare VERIFICATION.md; got: ${JSON.stringify(output.warnings)}`); + assert.ok(s006.message.includes('VERIFICATION.md')); + assertNoDriftKey(output); + }); + + test('#3511 WARNING-4: an underscore-separated "03_VERIFICATION.md" (broader .includes(\'VERIFICATION\') grammar) still fires S006 in its own phase dir', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 3\n**Current Phase:** 3\n**Total Plans in Phase:** 1\n**Current Plan:** 1\n`, + ); + // state.cts's own pre-filter (`.includes('VERIFICATION')`, no dash) is + // deliberately broader than the `-VERIFICATION.md` grammar every other + // #3511 site uses, so this underscore-separated name passes it. + // `scopeToPhase`/`isPhaseArtifact` (`phase-id.cts`) accept `_` alongside + // `-` and `.` as a candidate-boundary separator specifically so this + // broader pre-filter's own-phase matches aren't dropped after the fact — + // "03_VERIFICATION.md" in phase 03's directory IS this phase's own file, + // and the aggregate scan must not report it as absent. S006 fires here. + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-foo'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '03-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '03_VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, `S006 must fire for "03_VERIFICATION.md" in its own phase dir; got: ${JSON.stringify(output.warnings)}`); + const s007 = findWarning(output, 'S007'); + assert.ok(!s007, `S007 must not fire once S006 covers the passed own-phase verification; got: ${JSON.stringify(output.warnings)}`); + }); + test('output never carries a drift key, across clean/warning/error shapes (breaking-change proof, test matrix row 18)', () => { // Clean shape. fs.writeFileSync( diff --git a/tests/uat-predicate.test.cjs b/tests/uat-predicate.test.cjs index 799840953..9abab0f15 100644 --- a/tests/uat-predicate.test.cjs +++ b/tests/uat-predicate.test.cjs @@ -1011,6 +1011,83 @@ describe('evaluateUatPassed — output shape (Hyrum\'s Law contract)', () => { }); }); +// ─── #3511: phase-scoped UAT/VERIFICATION scanning — the transition gate ───── +// +// evaluateUatPassed is the CRITICAL anchor for #3511: its `passed`/`blockers` +// fields directly gate a phase transition. A cross-phase stray file sitting +// in a phase directory must never contribute a blocker to a phase it does not +// belong to, and the phase's own artifacts must keep behaving exactly as +// before. + +describe('#3511: evaluateUatPassed — cross-phase stray files do not contribute blockers', () => { + let baseDir; + let phaseDir; + + beforeEach(() => { + baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3511-uat-pred-')); + // Phase-shaped basename ("03-…") so isPhaseArtifact actually scopes — + // extractPhaseToken('03-uat-predicate') derives token "03". + phaseDir = path.join(baseDir, '03-uat-predicate'); + fs.mkdirSync(phaseDir); + }); + + afterEach(() => { + rmDir(baseDir); + }); + + test('passed:true with a passing own UAT + own VERIFICATION, despite a blocking cross-phase stray VERIFICATION', () => { + writeFile(phaseDir, '03-UAT.md', makePassingUat(1)); + writeFile(phaseDir, '03-VERIFICATION.md', '---\nstatus: passed\n---\n\nOK.'); + // Cross-phase stray: a "04" VERIFICATION file sitting in phase 03's + // directory, with a BLOCKING status. Pre-#3511, this unscoped scan would + // have picked it up and blocked phase 03's transition. + writeFile(phaseDir, '04-VERIFICATION.md', '---\nstatus: human_needed\n---\n\nNeeds human check.'); + + const report = evaluateUatPassed(phaseDir); + assert.strictEqual(report.passed, true, + 'a cross-phase stray VERIFICATION file must not block this phase\'s transition'); + assert.ok( + !report.blockers.some(b => /04-VERIFICATION\.md/.test(b) || /human_needed/i.test(b)), + `blockers must not name the stray file; got: ${JSON.stringify(report.blockers)}`, + ); + assert.strictEqual(report.verification_files.includes('04-VERIFICATION.md'), false, + 'the stray must not even be counted as a verification_files entry for this phase'); + }); + + test('passed:false with own report still failing, unaffected by an unrelated passing cross-phase stray (non-stray case unchanged)', () => { + writeFile(phaseDir, '03-UAT.md', makePassingUat(1)); + // This phase's own VERIFICATION is blocking. + writeFile(phaseDir, '03-VERIFICATION.md', '---\nstatus: gaps_found\n---\n\nHas gaps.'); + // A cross-phase stray that is itself passing must not paper over the + // phase's own real failure either — over-exclusion is as dangerous as + // under-exclusion here. + writeFile(phaseDir, '99-VERIFICATION.md', '---\nstatus: passed\n---\n\nOK.'); + + const report = evaluateUatPassed(phaseDir); + assert.strictEqual(report.passed, false, + 'the phase\'s own gaps_found VERIFICATION must still block, exactly as before #3511'); + assert.ok(report.blockers.some(b => /gaps_found/i.test(b)), + `blockers must still name this phase's own gaps_found status; got: ${JSON.stringify(report.blockers)}`); + }); + + test('#3511 follow-up: passed:true from a NON-canonical dir shape "1-unpadded" (over-exclusion / no_uat_artifacts check)', () => { + // "1-unpadded" tokenizes to literal "1"; scaffold writes the PADDED + // "01-…" form (normalizePhaseName). A literal token compare excluded the + // phase's own artifacts here, flipping `no_uat_artifacts: true` and + // false-blocking the transition gate this predicate feeds. + const unpaddedDir = path.join(baseDir, '1-unpadded'); + fs.mkdirSync(unpaddedDir); + writeFile(unpaddedDir, '01-UAT.md', makePassingUat(1)); + writeFile(unpaddedDir, '01-VERIFICATION.md', '---\nstatus: passed\n---\n\nOK.'); + + const report = evaluateUatPassed(unpaddedDir); + assert.strictEqual(report.no_uat_artifacts, false, + `own UAT/VERIFICATION files in an unpadded-dir phase must be found; got: ${JSON.stringify(report)}`); + assert.strictEqual(report.passed, true, + `the phase's own passing files in a non-canonical dir must pass the gate; got: ${JSON.stringify(report)}`); + }); +}); + // ─── FIX A regression: nested-fence (~~~ inside ```) ───────────────────────── describe('FIX A — nested fence: ~~~ inside ``` does not prematurely close outer fence', () => { diff --git a/tests/uat.test.cjs b/tests/uat.test.cjs index b4f9eb61b..051a8f56b 100644 --- a/tests/uat.test.cjs +++ b/tests/uat.test.cjs @@ -433,6 +433,78 @@ All checks passed. assert.strictEqual(output.summary.total_files, 0); }); + // #3511: a cross-phase, stray, or ad-hoc UAT/VERIFICATION file sitting in + // this phase's directory must not surface under this phase's audit-uat + // entry; this phase's own UAT/VERIFICATION artifacts must keep reporting + // exactly as before (non-stray case unchanged). + test('#3511: cross-phase stray UAT/VERIFICATION files in the same dir do not surface; own artifacts still do', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '03-foo'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // This phase's own UAT — must still report its pending item. + fs.writeFileSync(path.join(phaseDir, '03-UAT.md'), [ + '---', 'status: partial', '---', '', + '## Tests', '', + '### 1. Own Test', 'expected: Works', 'result: pending', '', + ].join('\n')); + // This phase's own VERIFICATION — must still report its human-needed item. + fs.writeFileSync(path.join(phaseDir, '03-VERIFICATION.md'), [ + '---', 'status: human_needed', 'phase: 03-foo', '---', '', + '## Human Verification', '', + '1. Own human check', + ].join('\n')); + + // Cross-phase strays sitting in the SAME directory — token "04", not "03". + fs.writeFileSync(path.join(phaseDir, '04-UAT.md'), [ + '---', 'status: partial', '---', '', + '## Tests', '', + '### 1. Stray Test', 'expected: Works', 'result: pending', '', + ].join('\n')); + fs.writeFileSync(path.join(phaseDir, '04-VERIFICATION.md'), [ + '---', 'status: human_needed', 'phase: 04-bar', '---', '', + '## Human Verification', '', + '1. Stray human check', + ].join('\n')); + + const result = runGsdTools('audit-uat --raw', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + assert.strictEqual(output.summary.total_files, 2, + `only this phase's own 2 files must be scanned; got: ${JSON.stringify(output.results.map(r => r.file))}`); + assert.strictEqual(output.summary.total_items, 2, + `1 own UAT item + 1 own VERIFICATION item, strays excluded; got: ${output.summary.total_items}`); + assert.strictEqual(output.summary.by_phase['03'], 2, 'own phase must be credited both items'); + assert.ok(!('04' in output.summary.by_phase), 'the cross-phase stray must not appear in by_phase at all'); + assert.ok(!result.output.includes('04-UAT.md'), 'stray UAT filename must never surface in the output'); + assert.ok(!result.output.includes('04-VERIFICATION.md'), 'stray VERIFICATION filename must never surface in the output'); + assert.ok(output.results.some(r => r.file === '03-UAT.md' && r.items.some(i => i.name === 'Own Test'))); + assert.ok(output.results.some(r => r.file === '03-VERIFICATION.md' && r.items.some(i => i.name === 'Own human check'))); + }); + + // #3511 follow-up: over-exclusion check on the #2528 digit-leading-slug + // family. "05-80-20-cleanup" tokenizes to "05-80-20" (mis-absorbed past + // the digit run scaffold actually writes into), so a literal token compare + // excluded the phase's own report — audit-uat reported total_files: 0. + test('#3511 follow-up: own UAT file still surfaces from the digit-leading-slug dir "05-80-20-cleanup" (over-exclusion check)', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '05-80-20-cleanup'); + fs.mkdirSync(phaseDir, { recursive: true }); + + fs.writeFileSync(path.join(phaseDir, '05-UAT.md'), [ + '---', 'status: partial', '---', '', + '## Tests', '', + '### 1. Own Test', 'expected: Works', 'result: pending', '', + ].join('\n')); + + const result = runGsdTools('audit-uat --raw', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + assert.strictEqual(output.summary.total_files, 1, + `own UAT file in a digit-leading-slug dir must still surface; got: ${JSON.stringify(output)}`); + assert.strictEqual(output.summary.by_phase['05'], 1); + }); + // Regression: #2286 — parseUatItems never scanned a `## Gaps` section, so a // *-UAT.md file recording its only outstanding findings there returned // total_items: 0 (false-clean). Boundary: 0 / 1 / 2+ unresolved entries. diff --git a/tests/verification-status.test.cjs b/tests/verification-status.test.cjs index 12c209e47..66ab24742 100644 --- a/tests/verification-status.test.cjs +++ b/tests/verification-status.test.cjs @@ -59,11 +59,18 @@ const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); // ─── Helpers ───────────────────────────────────────────────────────────────── /** - * Create a temporary phase directory under os.tmpdir(). - * Returns the absolute path; caller must clean up. + * Create a temporary phase directory named like a real one (`NN-slug`), + * inside a throwaway parent. #3511: a phase directory's own name determines + * which files count as ITS artifacts, so a fixture whose basename does not + * name the same phase as the files written into it is not a valid phase dir. + * @param {string} suffix - test-distinguishing suffix for the parent + * @param {string} phaseDirName - basename of the phase dir (default '01-foo') */ -function mkPhaseDir(suffix) { - return fs.mkdtempSync(path.join(os.tmpdir(), `gsd-651-${suffix}-`)); +function mkPhaseDir(suffix, phaseDirName = '01-foo') { + const parent = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-651-${suffix}-`)); + const phaseDir = path.join(parent, phaseDirName); + fs.mkdirSync(phaseDir); + return phaseDir; } /** @@ -99,7 +106,7 @@ describe('verification-status', () => { assert.equal(result.next_command, '', 'next_command must be empty for passed'); assert.ok(result.next_action.length > 0, 'next_action must be non-empty'); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); @@ -129,7 +136,14 @@ describe('verification-status', () => { // ── Case 3: human_needed ────────────────────────────────────────────────── test('status: human_needed → status human_needed, next_command is empty', () => { - const dir = mkPhaseDir('human-needed'); + // Deliberately non-numeric dir basename ("human-needed" has no digits at + // all) — extractPhaseToken has no derivable token, so (a) isPhaseArtifact's + // fail-safe still includes 01-hn-VERIFICATION.md as this "phase"'s own + // report, and (b) the next_command number-append check (which requires a + // PURELY numeric token) never fires. This is what this test is actually + // pinning — see the comment below — so the dir name must stay non-numeric, + // not the realistic 'NN-slug' default. + const dir = mkPhaseDir('human-needed', 'human-needed'); try { writeVerificationMd(dir, '01-hn-VERIFICATION.md', 'human_needed'); const result = readVerificationStatus(dir); @@ -139,13 +153,16 @@ describe('verification-status', () => { assert.equal(result.next_command, '/gsd-verify-work'); assert.ok(result.next_action.length > 0); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); // ── Case 4: no *-VERIFICATION.md → missing ──────────────────────────────── test('no *-VERIFICATION.md file → status missing, next_command execute-phase', () => { - const dir = mkPhaseDir('missing'); + // Non-numeric dir basename: next_command asserts no phase-number argument + // is appended, which requires extractPhaseToken(dirName) to not be purely + // numeric — see the human_needed test above for the same rationale. + const dir = mkPhaseDir('missing', 'missing'); try { // write a non-matching file to confirm it is ignored fs.writeFileSync(path.join(dir, 'README.md'), '# phase'); @@ -158,13 +175,15 @@ describe('verification-status', () => { `next_action must reassure the user execute-phase will not redo work (#1762); got: ${result.next_action}`, ); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); // ── Case 5: unknown frontmatter status value ────────────────────────────── test("frontmatter status 'bogus' → status unknown, next_command execute-phase", () => { - const dir = mkPhaseDir('unknown'); + // Non-numeric dir basename: next_command asserts no phase-number argument + // is appended — see the human_needed test above for the same rationale. + const dir = mkPhaseDir('unknown', 'unknown'); try { writeVerificationMd(dir, '01-u-VERIFICATION.md', 'bogus'); const result = readVerificationStatus(dir); @@ -179,7 +198,7 @@ describe('verification-status', () => { `next_action must acknowledge an unrecognized status may be an intentional marker (#1762); got: ${result.next_action}`, ); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); @@ -225,7 +244,7 @@ describe('verification-status', () => { ); assert.equal(result.next_command, '', 'next_command must be empty for passed'); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); @@ -298,7 +317,7 @@ describe('verification-status', () => { assert.equal(result.status, 'passed', 'CRLF frontmatter must parse to passed'); assert.equal(result.next_command, ''); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); @@ -316,7 +335,7 @@ describe('verification-status', () => { "A body-only status: line must NOT be read — result should be 'missing'", ); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); @@ -333,10 +352,14 @@ describe('verification-status', () => { // of the #3492 phase-pinned rule, not the contract itself — see the // `#3357/#3492` describe block below for the primary, phase-pinned tier // (resolveVerificationFile unit tests are the reliable anchors there). - // mkPhaseDir's random-suffixed basename never matches "01"/"02", so this - // exercises the fallback by construction. + // The dir basename ('multi', no digits) has no derivable phase token, so + // scopeToPhase's isPhaseArtifact fail-safe passes BOTH candidates through + // unfiltered (#3511: scopeToPhase is a plain filter with no other + // fallback — a derivable token that matched neither file would empty the + // set and this test would read 'missing', not exercise the alphabetical + // tiebreak at all). test('multiple *-VERIFICATION.md files, none matching the phase token → alphabetically-first FALLBACK wins', () => { - const dir = mkPhaseDir('multi'); + const dir = mkPhaseDir('multi', 'multi'); try { // Write two files: alphabetically "01-a" comes before "02-b" // "01-a" has passed; "02-b" has gaps_found — first by sort must win @@ -350,7 +373,7 @@ describe('verification-status', () => { 'With no exact phase-token match, the first by lexicographic sort must be used', ); } finally { - cleanup(dir); + cleanup(path.dirname(dir)); } }); @@ -986,6 +1009,106 @@ describe('#3357/#3492: phase-pinned *-VERIFICATION.md resolution when multiple c ); }); + // #3511 reconciliation: resolveVerificationFile's fallback now scopes to + // isPhaseArtifact(fileName, phaseDirName), so a stray cross-phase file can + // no longer win the alphabetical-first fallback tier either — closing the + // gap isPhaseArtifact's own docblock (src/phase-id.cts) used to flag as + // open. The two pure cases below are the reliable anchors; the behavioral + // test after them pins the same contract through the real CLI-facing + // readVerificationStatus call path. + test('#3511: a cross-phase stray is excluded from the fallback → null, not the stray', () => { + assert.equal( + resolveVerificationFile(['04-VERIFICATION.md'], { phaseDirName: '03-foo' }), + null, + '04-VERIFICATION.md belongs to phase 04, not the "03-foo" directory\'s phase 03 — must not be returned', + ); + }); + + test('#3511: a non-canonically-named report OF THIS phase still wins the fallback (the #3357 guarantee survives)', () => { + // The more important of the two #3511 cases: isPhaseArtifact scopes by + // phase-number membership, not by canonical shape, so this file still + // passes and the #3357 "non-canonical report still resolves" guarantee + // is not disturbed by the #3511 scoping. + assert.equal( + resolveVerificationFile(['03-CORRECTION-VERIFICATION.md'], { phaseDirName: '03-foo' }), + '03-CORRECTION-VERIFICATION.md', + '03-CORRECTION-VERIFICATION.md names phase 03, same as directory "03-foo" — must still resolve', + ); + }); + + test('#3511: cross-phase stray alongside this phase\'s own non-canonical report → own report wins, stray excluded (not merely outsorted)', () => { + // Distinguishes "excluded from the fallback" from "just happens to sort + // after" — candidates are sorted at verification.cts's own call site + // before reaching resolveVerificationFile, and '01-VERIFICATION.md' + // sorts BEFORE '03-CORRECTION-VERIFICATION.md' alphabetically, so an + // UNSCOPED (alphabetical-first) fallback would wrongly pick the stray + // here. Scoping must actively exclude it for '03-CORRECTION-…' to win. + assert.equal( + resolveVerificationFile( + ['01-VERIFICATION.md', '03-CORRECTION-VERIFICATION.md'], + { phaseDirName: '03-foo' }, + ), + '03-CORRECTION-VERIFICATION.md', + ); + }); + + // WARNING-2/5/INFO-2 note (#3511 review): the only fallback test above uses + // a token-LESS dir ("03-foo" isn't token-less — this refers to the earlier + // `multiple *-VERIFICATION.md files, none matching the phase token` test, + // which passes no derivable-token distinguishing fixture and so passes + // identically pre-#3511-fix). This test uses a dir WITH a derivable token + // (`03-foo` → token "03") and TWO candidates that BOTH belong to that same + // phase (`03-a-…`/`03-b-…`, no exact `03-VERIFICATION.md`), so scoping + // excludes nothing and the alphabetical-first tie-break still decides — + // pinning that scoping does not disturb the ordinary same-phase-multi-file + // case. + test('#3511: alphabetical fallback when BOTH candidates are this phase\'s own (derivable token, no exact match)', () => { + assert.equal( + resolveVerificationFile(['03-a-VERIFICATION.md', '03-b-VERIFICATION.md'], { phaseDirName: '03-foo' }), + '03-a-VERIFICATION.md', + 'both candidates belong to phase 03 (same as dir "03-foo"); alphabetically-first must still win', + ); + }); + + test('behavioral (readVerificationStatus): a phase dir holding only a cross-phase stray reports missing, not the stray\'s status (#3511)', () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3511-stray-only-')); + const dir = path.join(baseDir, '03-foo'); + fs.mkdirSync(dir); + try { + // Only a stray belonging to phase 04 sits in phase 03's directory. Give + // it a status that would NOT read as missing if it were (wrongly) picked, + // so a regression here is loud rather than accidentally matching. + writeVerificationMd(dir, '04-VERIFICATION.md', 'passed'); + + const result = readVerificationStatus(dir); + assert.equal( + result.status, + 'missing', + 'a phase dir holding only another phase\'s report must report missing, not passed', + ); + } finally { + cleanup(baseDir); + } + }); + + test('behavioral (readVerificationStatus): a phase dir holding only its own non-canonically-named report still resolves it (#3357 guarantee survives #3511 scoping)', () => { + const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3511-own-noncanon-')); + const dir = path.join(baseDir, '03-foo'); + fs.mkdirSync(dir); + try { + writeVerificationMd(dir, '03-CORRECTION-VERIFICATION.md', 'passed'); + + const result = readVerificationStatus(dir); + assert.equal( + result.status, + 'passed', + 'the phase\'s own non-canonically-named report must still resolve, not read as missing', + ); + } finally { + cleanup(baseDir); + } + }); + test('behavioral (readVerificationStatus): a phase with both its own report and a cross-phase stray reports the OWN report\'s status, not the stray\'s', () => { // The directory basename is "03-canonical-test" so extractPhaseToken // derives token "03" — the exact same derivation readVerificationStatus @@ -1123,6 +1246,21 @@ describe('#3473 F2: resolveVerificationFile allowBare option', () => { ); }); + // #3511: allowBare must still fall through to the bare match when the ONLY + // dashed candidate is excluded by phaseDirName scoping (a cross-phase + // stray) — the fallback tier finding nothing phase-owned is the same + // "no dashed candidate at all" case allowBare was always reached from. + test('#3511: allowBare:true, bare + a cross-phase dashed stray scoped out by phaseDirName → the bare file wins', () => { + assert.equal( + resolveVerificationFile( + ['VERIFICATION.md', '04-VERIFICATION.md'], + { allowBare: true, phaseDirName: '03-foo' }, + ), + 'VERIFICATION.md', + '04-VERIFICATION.md belongs to a different phase and is excluded, so bare VERIFICATION.md is the only remaining candidate', + ); + }); + }); // ─── #3518: resolveUatFile — phase-pinned, deterministic *-UAT.md pick ─────── diff --git a/tests/workstream-inventory.test.cjs b/tests/workstream-inventory.test.cjs index d311bc66d..e3c4b8c7b 100644 --- a/tests/workstream-inventory.test.cjs +++ b/tests/workstream-inventory.test.cjs @@ -14,7 +14,7 @@ const { createFixture, seedWorkstream } = require('./fixtures/index.cjs'); const { buildWorkstreamInventory, isCompletedInventory, pickRollupWinners } = require('../gsd-core/bin/lib/workstream-inventory-builder.cjs'); const { inspectWorkstream } = require('../gsd-core/bin/lib/workstream-inventory.cjs'); const { VERIFIER_STATUSES } = require('../gsd-core/bin/lib/verification.cjs'); -const { phaseKeyFromDir, phaseKeyFromProse, phaseKeyFromToken } = require('../gsd-core/bin/lib/phase-id.cjs'); +const { phaseKeyFromDir, phaseKeyFromProse, phaseKeyFromToken, normalizePhaseName } = require('../gsd-core/bin/lib/phase-id.cjs'); const fc = require('fast-check'); const STALE_STATE = 'status: executing\n'; @@ -235,7 +235,7 @@ describe('#2562 — progress/status scoped to the current milestone (derived fro fs.mkdirSync(dir, { recursive: true }); for (let i = 1; i <= plans; i++) fs.writeFileSync(path.join(dir, `0${i}-PLAN.md`), '# plan\n'); for (let i = 1; i <= summaries; i++) fs.writeFileSync(path.join(dir, `0${i}-SUMMARY.md`), '# summary\n'); - if (verification) fs.writeFileSync(path.join(dir, '01-VERIFICATION.md'), `---\nstatus: ${verification}\n---\n`); + if (verification) fs.writeFileSync(path.join(dir, `${normalizePhaseName(slug)}-VERIFICATION.md`), `---\nstatus: ${verification}\n---\n`); } const MS_STATE = 'milestone: v2.0\nstatus: executing\n'; @@ -361,7 +361,7 @@ describe('#2562 — milestone scoping boundaries (one phase-key derivation)', () fs.mkdirSync(dir, { recursive: true }); for (let i = 1; i <= plans; i++) fs.writeFileSync(path.join(dir, `0${i}-PLAN.md`), '# plan\n'); for (let i = 1; i <= summaries; i++) fs.writeFileSync(path.join(dir, `0${i}-SUMMARY.md`), '# summary\n'); - if (verification) fs.writeFileSync(path.join(dir, '01-VERIFICATION.md'), `---\nstatus: ${verification}\n---\n`); + if (verification) fs.writeFileSync(path.join(dir, `${normalizePhaseName(slug)}-VERIFICATION.md`), `---\nstatus: ${verification}\n---\n`); } function roadmapWithRows(rows) { @@ -926,7 +926,7 @@ describe('#2562 — a shipped marker its own artifacts contradict is not asserte fs.mkdirSync(dir, { recursive: true }); for (let i = 1; i <= plans; i++) fs.writeFileSync(path.join(dir, `0${i}-PLAN.md`), '# plan\n'); for (let i = 1; i <= summaries; i++) fs.writeFileSync(path.join(dir, `0${i}-SUMMARY.md`), '# summary\n'); - if (verification) fs.writeFileSync(path.join(dir, '01-VERIFICATION.md'), `---\nstatus: ${verification}\n---\n`); + if (verification) fs.writeFileSync(path.join(dir, `${normalizePhaseName(slug)}-VERIFICATION.md`), `---\nstatus: ${verification}\n---\n`); } function writeSnapshot(wsDir) { @@ -1054,7 +1054,7 @@ describe('#2645 — deleting a verification report must not raise completeness', fs.mkdirSync(dir, { recursive: true }); for (let i = 1; i <= plans; i++) fs.writeFileSync(path.join(dir, `0${i}-PLAN.md`), '# plan\n'); for (let i = 1; i <= summaries; i++) fs.writeFileSync(path.join(dir, `0${i}-SUMMARY.md`), '# summary\n'); - if (verification) fs.writeFileSync(path.join(dir, '01-VERIFICATION.md'), `---\nstatus: ${verification}\n---\n`); + if (verification) fs.writeFileSync(path.join(dir, `${normalizePhaseName(slug)}-VERIFICATION.md`), `---\nstatus: ${verification}\n---\n`); return dir; }