diff --git a/.changeset/3644-completion-seam.md b/.changeset/3644-completion-seam.md new file mode 100644 index 000000000..fd63b25de --- /dev/null +++ b/.changeset/3644-completion-seam.md @@ -0,0 +1,7 @@ +--- +type: Added +pr: 3644 +--- +**The phase-directory membership seam threads the phase ID convention through the completion chain** — the #3511 seam (`isPhaseArtifact` / `scopeToPhase`) now takes the same optional convention every other read-path helper does, and the completion chain threads it: `state json` / `state sync`'s completed-phase counting, the planning snapshot, roadmap analysis, `state validate`'s drift scan, the verification-report resolver, and `phase complete`'s actual completion gate. A bracket directory therefore scopes its listing by its real phase token instead of the include-everything ambiguity fail-safe, so a cross-phase stray (`01-VERIFICATION.md` misfiled into phase 03's directory) can no longer supply the pass/fail verdict for a bracket phase — the same protection #3511 already gives legacy directories, **on the call sites this PR threads**. + +Call sites that do not yet resolve a convention keep the documented include-everything fail-safe on bracket directories, and this PR changes nothing for them: the aggregate scans (`uat`, `audit`, `init`'s projections, `gap-checker`, `phase-locator`); **`phase complete`'s advisory pre-scan** (`cmdPhaseComplete`, `src/phase.cts`), whose UAT and VERIFICATION warning sweeps still call the seam convention-lessly and can therefore surface a spurious warning for a cross-phase stray, although that scan cannot pass or block completion; and **the workstream inventory's per-phase completion projection** (`src/workstream-inventory.cts`), which calls the now-convention-aware `isPhaseComplete` without resolving a convention to pass it and can therefore still project a bracket phase complete or incomplete from a cross-phase stray. Threading those readers is follow-up-slice work alongside the epic's other convention-less readers. A project on any convention other than `"bracket"` is unaffected. (#4142) diff --git a/CONTEXT.md b/CONTEXT.md index dd11b68aa..102d7036e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -15,7 +15,7 @@ Module owning `milestone complete` (archive roadmap/requirements/phases, build M Module that composes Dispatch Policy Module, Query Execution Policy Module, and per-stage handlers (input-validation, plan, execution, result-builder, formatting, error-mapping, observability) into the end-to-end pipeline that produces a `QueryDispatchResult`. The SDK-era pipeline collapsed onto the Command Routing Hub per ADR-0174; current dispatch seam: `gsd-core/bin/lib/command-routing-hub.cjs` (see Command Routing Hub below). ### Phase Id Module -Module owning the pure phase-id parsing and matching helpers: phase-name normalization, phase-token extraction/matching, canonical phase-directory selection, milestone- and phase-dir id parsing, phase-markdown regex builders, and the ADR-612 bracket phase-id round-trip grammar (`escapeRegex`, `normalizePhaseName`, `comparePhaseNum`, `extractPhaseToken`, `phaseTokenMatches`, `matchPhaseDirs`, `phaseNumberForMatch`, `phaseMarkdownRegexSource`/`phaseMarkdownRegexSourceExact`, `getMilestoneFromPhaseId`, `getPhaseDirFromPhaseId`, `parsePhaseId`/`renderPhaseId`/`toDir` over the `PhaseId` type, `isSentinelPhaseId`/`SENTINEL_RANGES`, and the `BRACKET_PHASE_TOKEN_SOURCE`/`PHASE_HEADING_PREFIX_SRC` grammar sources, plus the #612 read-path surface: the one bracket identity grammar `BRACKET_ID_SRC`/`BRACKET_MILESTONE_NUMERIC_SRC`/`BRACKET_DIR_PREFIX_SRC`, the case-folding `foldBracketId`, `bracketQualifiedKey`, and the convention-gated heading-intro selector `phaseHeadingPrefixSrcFor` over `PHASE_HEADING_BASELINE` / `BASE_ANY_BRACKET_HEADING_PREFIX_SRC` / `BASE_PHASE_LABEL_PREFIX_SRC`). Also owns the canonical phase KEY surface (#2562) — `phaseKeyFromToken`/`phaseKeyFromDir`/`phaseKeyFromProse`/`parentPhaseKey` — the padding-, case- and project-code-insensitive identity used whenever two independently-derived phase references (a ROADMAP table cell and a phase directory, say) are compared; deriving one side of such a comparison with a bespoke regex is what silently zeroed a rollup in #2562. Pure string/regex — no I/O, no config, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2a (#865) as the cycle-free leaf that unblocks the roadmap-parser and phase-locator extractions; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-id.cjs` (generated from `src/phase-id.cts`). +Module owning the pure phase-id parsing and matching helpers: phase-name normalization, phase-token extraction/matching, canonical phase-directory selection, milestone- and phase-dir id parsing, phase-markdown regex builders, and the ADR-612 bracket phase-id round-trip grammar (`escapeRegex`, `normalizePhaseName`, `comparePhaseNum`, `extractPhaseToken`, `phaseTokenMatches`, `matchPhaseDirs`, `phaseNumberForMatch`, `phaseMarkdownRegexSource`/`phaseMarkdownRegexSourceExact`, `getMilestoneFromPhaseId`, `getPhaseDirFromPhaseId`, `parsePhaseId`/`renderPhaseId`/`toDir` over the `PhaseId` type, `isSentinelPhaseId`/`SENTINEL_RANGES`, and the `BRACKET_PHASE_TOKEN_SOURCE`/`PHASE_HEADING_PREFIX_SRC` grammar sources, plus the #612 read-path surface: the one bracket identity grammar `BRACKET_ID_SRC`/`BRACKET_MILESTONE_NUMERIC_SRC`/`BRACKET_DIR_PREFIX_SRC`, the case-folding `foldBracketId`, `bracketQualifiedKey`, and the convention-gated heading-intro selector `phaseHeadingPrefixSrcFor` over `PHASE_HEADING_BASELINE` / `BASE_ANY_BRACKET_HEADING_PREFIX_SRC` / `BASE_PHASE_LABEL_PREFIX_SRC`). `matchPhaseDirs` is the single owner of "which directories does this query name" (#2528) and takes the same optional `convention` its `phaseTokenMatches` primitive does, so bracket directories resolve through the shared selector rather than around it; the #3511 membership seam (`isPhaseArtifact`/`scopeToPhase`) takes the same optional `convention`, so call sites that hold the resolved convention scope a bracket directory's listing by its real phase token instead of the include-everything ambiguity fail-safe. Also owns the canonical phase KEY surface (#2562) — `phaseKeyFromToken`/`phaseKeyFromDir`/`phaseKeyFromProse`/`parentPhaseKey` — the padding-, case- and project-code-insensitive identity used whenever two independently-derived phase references (a ROADMAP table cell and a phase directory, say) are compared; deriving one side of such a comparison with a bespoke regex is what silently zeroed a rollup in #2562. Pure string/regex — no I/O, no config, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2a (#865) as the cycle-free leaf that unblocks the roadmap-parser and phase-locator extractions; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-id.cjs` (generated from `src/phase-id.cts`). ### Phase Lifecycle Module Module owning phase create, rename, complete, remove, list, and plan-index operations, plus phase-dir prefix validation, STATE.md staleness detection, and auto-prune behaviour. Entry point: `gsd-core/bin/lib/phase.cjs` (CJS surface). Typed phase events: `GSDPhaseStartEvent`, `GSDPhaseStepStartEvent`, `GSDPhaseStepCompleteEvent`, `GSDPhaseCompleteEvent`. (The SDK native-query surface, the `types.ts` event definitions, `phase-runner.ts`, and `phase-prompt.ts` were retired with the SDK package per ADR-0174.) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 19ecb9195..af558877b 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -243,7 +243,7 @@ derived from the shipped agent declaration. | `dynamic_routing.max_escalations` | integer | `0`, `1`, `2`, … | `1` | Hard cap on retries per agent invocation. Beyond the cap the resolver returns the cap-tier model. Also caps `provider_escalation`. Added in v1.40 | | `dynamic_routing.provider_escalation` | string[] | ordered model IDs | (none) | Opt-in fallback providers tried when a run dies on a quota / rate limit — see [provider escalation](#provider-escalation-on-quota-exceeded--added-in-v143). Added in v1.43 ([#2296](https://github.com/open-gsd/gsd-core/issues/2296)) | | `project_code` | string | any short string | (none) | Prefix for phase directory names (e.g., `"ABC"` produces `ABC-01-setup/`). Added in v1.31 | -| `phase_id_convention` | enum | `"milestone-prefixed"`, `"bracket"`, `null` | `null` | Phase ID naming convention. `null` = legacy numeric IDs (`Phase 1`, `Phase 2`). `"milestone-prefixed"` = globally unique IDs that encode the enclosing milestone (`Phase 1-01`, `Phase 1-02`). Run `gsd-tools roadmap upgrade --convention milestone-prefixed` to migrate an existing ROADMAP.md. `"bracket"` = IDs that carry the milestone in a bracket ahead of the phase number — heading `### [GSD.02] 05: Name`, directory `GSD.02-05-name` — per [ADR-612](adr/612-bracket-phase-id-convention.md). **`"bracket"` currently affects the READ path only:** `roadmap analyze` / `roadmap get-phase`, the W005/W006/W007 phase checks, `validate health` (including an advisory W021 — a bracket phase's milestone disagreeing with its enclosing section, or a phase heading still spelled in legacy form that has not yet been migrated to bracket form), and both `total_phases` derivations recognise the bracket spelling once it is set. There is no bracket migrator and no bracket emit yet, so set it only on a project whose ROADMAP.md already uses that spelling; a project on any other value compiles the same patterns it did before and is unaffected. **What opting in costs:** on a bracket repo a heading whose bracket is followed directly by a digit is read as a phase heading, so shapes that are legal prose headings on any other convention — `### [RFC.2119] 5:`, `### [v1.0] 2024:`, `### [ADR.612] 3:` — are claimed as phases and will move `phase_count`, `total_phases` and W006. A bracket repo cedes that heading shape; that is the trade the opt-in buys, and it is why the widened read is selected at construction time from this value rather than applied everywhere ([#2761](https://github.com/open-gsd/gsd-core/issues/2761)). | +| `phase_id_convention` | enum | `"milestone-prefixed"`, `"bracket"`, `null` | `null` | Phase ID naming convention. `null` = legacy numeric IDs (`Phase 1`, `Phase 2`). `"milestone-prefixed"` = globally unique IDs that encode the enclosing milestone (`Phase 1-01`, `Phase 1-02`). Run `gsd-tools roadmap upgrade --convention milestone-prefixed` to migrate an existing ROADMAP.md. `"bracket"` = IDs that carry the milestone in a bracket ahead of the phase number — heading `### [GSD.02] 05: Name`, directory `GSD.02-05-name` — per [ADR-612](adr/612-bracket-phase-id-convention.md). **`"bracket"` currently affects the READ path only:** `roadmap analyze` / `roadmap get-phase`, the W005/W006/W007 phase checks, `validate health` (including an advisory W021 — a bracket phase's milestone disagreeing with its enclosing section, or a phase heading still spelled in legacy form that has not yet been migrated to bracket form), and both `total_phases` derivations recognise the bracket spelling once it is set. There is no bracket migrator and no bracket emit yet, so set it only on a project whose ROADMAP.md already uses that spelling; a project on any other value compiles the same patterns it did before and is unaffected. **What opting in costs:** on a bracket repo a heading whose bracket is followed directly by a digit is read as a phase heading, so shapes that are legal prose headings on any other convention — `### [RFC.2119] 5:`, `### [v1.0] 2024:`, `### [ADR.612] 3:` — are claimed as phases and will move `phase_count`, `total_phases` and W006. A bracket repo cedes that heading shape; that is the trade the opt-in buys, and it is why the widened read is selected at construction time from this value rather than applied everywhere ([#2761](https://github.com/open-gsd/gsd-core/issues/2761)). **Phase-directory membership** on a bracket repo scopes by the directory's real bracket token, so an artifact misfiled from another phase (`01-VERIFICATION.md` sitting in a phase `03` directory) no longer supplies `phase complete`'s pass/fail verdict for the phase it was misfiled into — the same protection legacy directories already have. Call sites that do not yet resolve a convention keep the wider include-everything fail-safe on bracket directories until they thread one: the aggregate scans (`uat`, `audit`, `init` projections, `gap-checker`, `phase-locator`); `phase complete`'s advisory UAT/VERIFICATION warning pre-scan, which can still surface a spurious warning but cannot decide completion; and the workstream inventory's per-phase completion projection, which can still report a bracket phase complete or incomplete from a cross-phase stray. | | `response_language` | string | language code | (none) | Language for agent responses (e.g., `"pt"`, `"ko"`, `"ja"`). Propagates to all spawned agents for cross-phase language consistency. Added in v1.32. UAT checkpoint frames (`/gsd-verify-work`) render a localized banner/instruction for English, Spanish, French, German, Portuguese, Japanese, Chinese, Korean, Italian, Dutch, Polish, Russian, Ukrainian, Turkish, Hindi, Arabic, Vietnamese, and Indonesian (endonyms and ISO codes also accepted); any other value falls back to the English frame. One deliberate exception: the `spec-phase` edge-completeness probe is fed an English translation of each requirement's text, because its shape cues are English-only — the SPEC itself stays in this language. See [Spec-Phase Edge-Completeness Probe](FEATURES.md#144-spec-phase-edge-completeness-probe). Every workflow is required to carry a directive honouring this setting, including for inter-tool narration; authors add or fix one per [response-language coverage](contributing/response-language-coverage.md), and `npm run lint:response-language` enforces it. | | `context_window` | number | any integer | `200000` | Context window size in tokens. Set `1000000` for 1M-context models (e.g., `claude-fable-5`). Values `>= 500000` enable adaptive context enrichment (full-body reads of prior SUMMARY.md, deeper anti-pattern reads). Configured via `/gsd-config --advanced`. | | `context_profile` | string | `dev`, `research`, `review` | (none) | Execution context preset that applies a pre-configured bundle of mode, model, and workflow settings for the current type of work. Added in v1.34 | diff --git a/scripts/lib/macos-conformance-tier.generated.cjs b/scripts/lib/macos-conformance-tier.generated.cjs index 989afa26a..f16f78c60 100644 --- a/scripts/lib/macos-conformance-tier.generated.cjs +++ b/scripts/lib/macos-conformance-tier.generated.cjs @@ -6,6 +6,7 @@ module.exports = { MACOS_CONFORMANCE_TIER_FILES: [ + "tests/adr-612-bracket-phase-counting.test.cjs", "tests/adr-index-gate.test.cjs", "tests/adr-parser.property.test.cjs", "tests/adr-parser.unit.test.cjs", diff --git a/src/phase-id.cts b/src/phase-id.cts index 2e897274d..434309076 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -959,19 +959,42 @@ function extractPhaseToken(dirName: string, convention?: string | null): string * 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. + * BRACKET CONVENTION (review item 7; reworked by #612/#2761): 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. This + * predicate now takes the same optional trailing `convention` its sibling + * helpers do (ADR-2121 additive shape): when a call site threads + * `'bracket'` and the dir is a WELL-FORMED bracket dir + * (`BRACKET_DIR_TOKEN_RE` — the one grammar owner, shared with + * `extractPhaseToken`'s bracket branch), the dir's real phase token is + * readable and membership is scoped through the identical candidate + * comparison its legacy twin gets — so `GSD.02-02-two` reads exactly what + * `02-two` reads, which is PR-2's core invariant. Convention-less calls, + * bracket-MALFORMED names (1-digit milestone, letter-suffixed token — the + * shapes W005 reports), and legacy-shaped dirs inside a bracket repo all + * resolve to the unchanged body below. + * + * A filename carrying its own bracket-qualified stem is compared on the + * qualified key — see the branch body. An M-NN-stem artifact name + * (`02-01-VERIFICATION.md` in `GSD.02-01-one`) is excluded under the bracket + * convention, deliberately: that is exactly what the legacy twin does with + * the same file, twin-parity is this PR's contract, and migration-window + * artifact renaming belongs to the migrator slice per ADR-612 — the trade is + * pinned in tests/adr-612-bracket-phase-counting.test.cjs. + * + * For those unthreaded/unparseable cases the include-everything trade + * stands: 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), the ambiguous families widen to include-everything, + * accepted in trade for never dropping the phase's own report. Note the + * bracket family splits across BOTH fail-safes, not just one: a short + * digit-bearing project code (`A1.02-…`) lands in `firstLetterPrefixed`, + * but a 3+-letter code (`GSD.02-…`) derives ZERO token segments — + * `PROJECT_CODE_PREFIX_CAPTURE_RE_I` strips `{CODE}-`, not `{CODE}.` — so + * the zero-token fail-safe fires first. A fixer patching only the + * `firstLetterPrefixed` branch covers neither; the convention gate above + * covers both. * * FAIL-SAFE (#3511, unchanged): when dirName's own leading segment carries no * phase-number token at all (`derivePhaseTokenSegments` finds zero segments — @@ -1008,7 +1031,44 @@ function extractPhaseToken(dirName: string, convention?: string | null): string * 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 { +function isPhaseArtifact(fileName: string, phaseDirName: string, convention?: string | null): boolean { + // #612/#2761: the bracket branch — see "BRACKET CONVENTION" in the docblock. + // Gated on the same explicit signal AND the same grammar + // (`BRACKET_DIR_TOKEN_RE`) as `extractPhaseToken`'s bracket branch, so the + // two surfaces can never disagree about which names are bracket dirs. A + // name that fails the grammar falls through to the unchanged body below. + if (convention === 'bracket') { + const bracketDir = phaseDirName.match(BRACKET_DIR_TOKEN_RE); + if (bracketDir) { + // The convention must gate the FILENAME reading, not just the directory + // reading — a milestone-qualified artifact name (the layout the + // read-tolerance suite models) derives zero legacy token segments, so + // without this it enters FIX 2's token-less containment and any bracket + // dir admits it. Compared on the qualified key: the phase's own + // qualified artifact stays included — the exact key, and the dotted + // sub-phase continuation (`GSD.02-05.1-…` is `GSD.02-05`'s own file, + // the same dash-OR-dot rule matchesPhaseTokenCandidates applies; a + // dash-continuation never reaches this comparison, the phase group's + // `(?=-|$)` boundary already stopped the key before it) — while a + // WELL-FORMED qualified stem naming a different phase or milestone is + // excluded. A qualified-SHAPED but grammar-malformed stem (letter + // suffix, 1-digit milestone, 2-level dot — the W005 shapes) yields a + // null fileKey and keeps the module's documented include-everything + // fail-safe via FIX 2 below, same as every other undecidable name. + const fileKey = bracketQualifiedKey(fileName, convention); + if (fileKey !== null) { + const dirKey = bracketQualifiedKey(phaseDirName, convention); + if (dirKey !== null) return fileKey === dirKey || fileKey.startsWith(`${dirKey}.`); + } + // Reached when the filename carries no well-formed qualified stem (the + // common case — every unqualified name), or when the DIR's own key is + // null despite matching the dir grammar (an unsafe-integer milestone or + // phase — bracketQualifiedKey refuses both rather than collide). + // Either way the candidate path decides, deliberately. + return matchesPhaseTokenCandidates(fileName, [bracketDir[1]]); + } + } + const { tokenSegments, firstLetterPrefixed } = derivePhaseTokenSegments(phaseDirName); if (tokenSegments.length === 0) return true; @@ -1020,6 +1080,23 @@ function isPhaseArtifact(fileName: string, phaseDirName: string): boolean { const rawCandidates = [literalToken, strippedToken, leadingRunMatch?.[1]].filter( (t): t is string => Boolean(t), ); + if (matchesPhaseTokenCandidates(fileName, rawCandidates)) return true; + + // Bracket-convention ambiguity fail-safe — see docblock above. + if (firstLetterPrefixed) return true; + + return false; +} + +/** + * Candidate-comparison core shared by `isPhaseArtifact`'s two entry paths — + * the legacy segment-derived readings and the #612 bracket-dir token. One + * comparison rule, not two that agree today and drift tomorrow: the padded/ + * de-padded expansion, the separator class, and FIX 2 apply identically + * whichever path produced the candidates, which is what makes a bracket dir + * read exactly what its legacy twin reads. + */ +function matchesPhaseTokenCandidates(fileName: string, rawCandidates: string[]): boolean { // 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. @@ -1064,12 +1141,7 @@ function isPhaseArtifact(fileName: string, phaseDirName: string): boolean { // 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; + return derivePhaseTokenSegments(fileName).tokenSegments.length === 0; } /** @@ -1119,9 +1191,19 @@ function isPhaseArtifact(fileName: string, phaseDirName: string): boolean { * re-derived per site (CLAUDE.md's Generative Fix Divergence class). * `isPhaseArtifact` stays exported for single-item membership questions and * its own unit tests. + * + * #612/#2761: both helpers take the same optional trailing `convention` their + * sibling primitives (`extractPhaseToken`, `phaseTokenMatches`) do — the + * ADR-2121 additive shape, so every existing two-argument call resolves to + * the unchanged legacy body. A call site that already holds the resolved + * convention threads it and bracket dirs scope exactly like their legacy + * twins; convention-less call sites keep the documented include-everything + * fail-safes and are the follow-up-slice work (they cannot scope a bracket + * dir until they can resolve the convention, and guessing is the one thing + * this predicate must never do). */ -function scopeToPhase(fileNames: string[], phaseDirName: string): string[] { - return fileNames.filter((f) => isPhaseArtifact(f, phaseDirName)); +function scopeToPhase(fileNames: string[], phaseDirName: string, convention?: string | null): string[] { + return fileNames.filter((f) => isPhaseArtifact(f, phaseDirName, convention)); } /** diff --git a/src/phase.cts b/src/phase.cts index 94ffbbc95..67b90e559 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -3640,7 +3640,10 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { // #2617: pass the project's runtime so the blocked-completion error below // suggests the command surface this runtime actually installs // ($gsd-… on Codex) rather than a hard-coded Claude-style string. - const verificationStatus = readVerificationStatus(phaseFullDir, { runtime: resolveRuntime(cwd) }); + const verificationStatus = readVerificationStatus(phaseFullDir, { + runtime: resolveRuntime(cwd), + convention: resolvePhaseIdConvention(cwd), + }); // #3057 B3: the staleness check inside readVerificationStatus can itself // fail (fs / scanPhasePlans / clock error), in which case `status` above // was routed as if nothing were stale (unchanged fail-open routing) — but diff --git a/src/planning-snapshot.cts b/src/planning-snapshot.cts index 0cb753e4d..ed95aac9b 100644 --- a/src/planning-snapshot.cts +++ b/src/planning-snapshot.cts @@ -308,9 +308,12 @@ interface PlanningSnapshot { * uncorrelated (isPhaseComplete's readability check never re-derives or * requires scanPhasePlans, and vice versa). */ -function buildPhaseSnapshot(phasesDir: string, dir: string): PhaseSnapshot { +function buildPhaseSnapshot(phasesDir: string, dir: string, convention: string | null): PhaseSnapshot { const fullPhaseDir = path.join(phasesDir, dir); - const completionResult = isPhaseComplete(fullPhaseDir); + // #612: the snapshot's single federated convention resolution rides into + // completion, so a bracket phase dir resolves and scopes its verification + // report exactly like its legacy twin. + const completionResult = isPhaseComplete(fullPhaseDir, { convention }); const scanResult = scanPhasePlans(fullPhaseDir); return { dir, @@ -895,6 +898,7 @@ function buildResearchValidationStatusField( phasesDir: string, phaseDirNames: string[], enumerationScope: Scope, + convention: string | null, ): { value: { dir: string; hasValidationArchitecture: boolean; hasValidationMd: boolean }[]; scope: Scope; @@ -911,7 +915,9 @@ function buildResearchValidationStatusField( // 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); + // #612: the snapshot's federated convention threaded, so a bracket dir + // scopes by its real token instead of the include-everything fail-safe. + const scopedFiles = scopeToPhase(files, dir, convention); const researchFile = scopedFiles.find((f) => f.endsWith('-RESEARCH.md')); const hasValidationMd = scopedFiles.some((f) => f.endsWith('-VALIDATION.md')); let hasValidationArchitecture = false; @@ -1270,10 +1276,15 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { // genuinely differ, and substituting one for the other would silently re-scope // `phaseDirs` — a change this PR does not need and no test covers. The // federation guarantee PR-2 exists to deliver is delivered where it is - // observable: in the rules that read `snapshot.phaseIdConvention`. + // observable: in the rules that read `snapshot.phaseIdConvention`. Per-phase + // completion and the research/validation scoping below deliberately use the + // FEDERATED `phaseIdConvention` (the same value `snapshot.phaseIdConvention` + // publishes), while `phaseDirs` keeps `listMilestonePhaseDirs`'s lazy + // PROJECT-only resolve — the divergence documented above is unchanged by + // this thread. No behavior change. const phaseDirs = listMilestonePhaseDirs(paths.phases, { cwd }); - const phasesValue = phaseDirs.value.map((dir) => buildPhaseSnapshot(paths.phases, dir)); + const phasesValue = phaseDirs.value.map((dir) => buildPhaseSnapshot(paths.phases, dir, phaseIdConvention)); const stateFields = buildStateFields(paths.state); const allPhaseDirNames = buildAllPhaseDirNamesField(paths.phases); const roadmapDeclared = buildRoadmapDeclaredPhasesField(paths.roadmap, phaseIdConvention); @@ -1301,7 +1312,7 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { stateStatus: stateFields.stateStatus, roadmapDeclaredPhases: roadmapDeclared.declared, roadmapPhaseCheckboxes: buildRoadmapPhaseCheckboxesField(paths.roadmap, phaseIdConvention), - researchValidationStatus: buildResearchValidationStatusField(paths.phases, phaseDirs.value, phaseDirs.scope), + researchValidationStatus: buildResearchValidationStatusField(paths.phases, phaseDirs.value, phaseDirs.scope, phaseIdConvention), milestoneArchiveStatus: buildMilestoneArchiveStatusField(cwd), planningRootFiles: buildPlanningRootFilesField(cwd), allPhaseDirNames, diff --git a/src/roadmap.cts b/src/roadmap.cts index d3ff3e77f..b302f0d85 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -133,7 +133,7 @@ function coerceTruthToString(t: unknown): string { // ─── countPhasePlansAndSummaries ────────────────────────────────────────────── -function countPhasePlansAndSummaries(phaseDir: string): PhasePlansAndSummaries { +function countPhasePlansAndSummaries(phaseDir: string, convention?: string | null): PhasePlansAndSummaries { const { planCount, summaryCount } = scanPhasePlans(phaseDir); // hasContext and hasResearch are not plan-scan concerns — read the directory // once and share the listing for all non-plan metadata that cmdRoadmapAnalyze needs. @@ -156,7 +156,9 @@ function countPhasePlansAndSummaries(phaseDir: string): PhasePlansAndSummaries { // 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)); + // #612: `convention` threaded from the one caller (which already threads it + // into matchPhaseDirs) so a bracket dir scopes by its real token. + const scopedFiles = scopeToPhase(phaseFiles, path.basename(phaseDir), convention); return { planCount, summaryCount, @@ -557,7 +559,7 @@ function collectAnalyzePhases( const dirMatch = matchPhaseDirs(phaseDirNames, normalized, convention).matches[0]; if (dirMatch) { - const counts = countPhasePlansAndSummaries(path.join(phasesDir, dirMatch)); + const counts = countPhasePlansAndSummaries(path.join(phasesDir, dirMatch), convention); planCount = counts.planCount; summaryCount = counts.summaryCount; hasContext = counts.hasContext; @@ -571,7 +573,10 @@ function collectAnalyzePhases( // NOT a precondition, so a zero-plan phase with a passing // `*-VERIFICATION.md` reports complete here too, not just via // `phase.complete`. - const completionResult = isPhaseComplete(path.join(phasesDir, dirMatch)); + // #612: `convention` (a parameter of this function, same thread as + // matchPhaseDirs above) rides into completion so a bracket phase dir + // resolves and scopes its verification report like its legacy twin. + const completionResult = isPhaseComplete(path.join(phasesDir, dirMatch), { convention }); if (completionResult.value.complete) diskStatus = 'complete'; else if (summaryCount > 0) diskStatus = 'partial'; else if (planCount > 0) diskStatus = 'planned'; @@ -1008,7 +1013,11 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und // the same phase (ADR-3180 §7.4's headline: one predicate for the read // path and the write path). const phaseDir = path.join(cwd, phaseInfo!.directory); - const completionResult = isPhaseComplete(phaseDir); + // ADR-3180 §7.4 read/write-path symmetry with the threaded site at ~583: + // thread convention here too, so this write path's completion reading + // agrees with the read path's under the bracket convention. + const convention = resolvePhaseIdConvention(cwd); + const completionResult = isPhaseComplete(phaseDir, { convention }); const verificationResult = completionResult.value.verification; // #2648 precedent, applied at this write site (ADR-3180 §7.4 / #3186): // `isPhaseComplete` deliberately carries NO plan-count precondition — the diff --git a/src/state.cts b/src/state.cts index 6ccb5a6dd..bb5a51a35 100644 --- a/src/state.cts +++ b/src/state.cts @@ -2942,7 +2942,10 @@ function buildStateFrontmatter( // own comment on that field). Folding this consumer onto the raw // summaries-met flag was the exact "consolidate two of three and // leave the third" gap §7.4's forcing function rules out. - if (isPhaseComplete(phaseDir).value.complete) diskCompletedPhases++; + // #612: `phaseConvention` threaded so a bracket phase dir resolves + // and scopes its verification report like its legacy twin — the + // read-side half of the same thread cmdStateSync gets below. + if (isPhaseComplete(phaseDir, { convention: phaseConvention }).value.complete) diskCompletedPhases++; } // Count phase headings from ROADMAP — single source of truth for // total_phases (#549). #612 round-4: shared with cmdStateSync's @@ -5836,9 +5839,13 @@ function cmdStateValidate(cwd: string, raw: boolean, opts: { strict?: boolean } // ("verification passed" drift), not a false S007. const files = fs.readdirSync(phaseDirPath); const phaseDirBaseName = path.basename(phaseDirPath); + // #612: `validateConvention` threaded (already resolved above for + // `phaseKeyFromDir`) so the S006/S007 scan scopes bracket dirs by + // their real token instead of the include-everything fail-safe. const verificationFiles = scopeToPhase( files.filter(f => f.includes('VERIFICATION') && f.endsWith('.md')), phaseDirBaseName, + validateConvention, ); for (const vf of verificationFiles) { try { @@ -6063,7 +6070,10 @@ function cmdStateSync(cwd: string, options: StateSyncOptions | undefined, raw: b // was a second, independent consumer of the same raw field the initial // migration missed — without it, `state sync` and `state json` disagreed // on completed_phases for the identical disk state. - if (isPhaseComplete(dirPath).value.complete) diskCompletedPhases++; + // #612: `syncConvention` threaded — the write-side half of + // buildStateFrontmatter's thread above, so `state sync` and `state json` + // keep agreeing on completed_phases under the bracket convention. + if (isPhaseComplete(dirPath, { convention: syncConvention }).value.complete) diskCompletedPhases++; // Track the highest phase with incomplete plans (or any plans) const phaseMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); diff --git a/src/verification.cts b/src/verification.cts index 433e5cca1..6ed2acd72 100644 --- a/src/verification.cts +++ b/src/verification.cts @@ -515,6 +515,15 @@ interface ResolveVerificationFileOptions { * pick — the original pre-#3357 behavior — never to null. */ phaseDirName?: string; + /** + * #612: the repo's resolved `phase_id_convention`, threaded verbatim into + * `scopeToPhase(candidates, phaseDirName, convention)` so the fallback + * scopes a bracket dir (`{CODE}.{MM}-{PP}-slug`) by its real phase token + * instead of the include-everything ambiguity fail-safe. Same ADR-2121 + * additive shape as every other convention thread: omitted / null resolves + * to the unchanged legacy scoping. + */ + convention?: string | null; } /** @@ -610,7 +619,7 @@ function resolvePhaseArtifactFile( // filtered out, and if that leaves nothing the code falls through to // `allowBare`/`null` deliberately. const scoped = options.phaseDirName - ? scopeToPhase(candidates, options.phaseDirName) + ? scopeToPhase(candidates, options.phaseDirName, options.convention) : candidates; if (scoped.length > 0) return scoped[0]; } @@ -694,6 +703,15 @@ interface ReadVerificationStatusOptions { * this with `phaseDir` unresolved in some branches. */ phaseNumber?: string; + /** + * #612: the repo's resolved `phase_id_convention`, threaded through to + * `resolveVerificationFile` (exact-pin token + fallback scoping) and + * `findStaleVerificationSummary` so a bracket phase dir resolves and + * scopes its report exactly like its legacy twin. Deliberately NOT used + * for the routed command argument — see the derivation comment in the + * body. Omitted / null: unchanged legacy behavior. + */ + convention?: string | null; } interface VerificationStatusResult { @@ -717,6 +735,7 @@ function findStaleVerificationSummary( phaseDir: string, fsImpl: FsLike = defaultFsImpl, phaseCleanCommitTimesMs: PhaseCleanCommitTimesFn = defaultPhaseCleanCommitTimesMs, + convention?: string | null, ): StaleCheckResult { // FS errors (TOCTOU: a SUMMARY listed by scanPhasePlans then removed before statSync; // unreadable dir; broken symlink; file->dir swap) must degrade rather than throw @@ -737,11 +756,15 @@ function findStaleVerificationSummary( // status reader sees, or a bare report could never read `stale` while its // dashed twin could (two answers from one verb). const phaseDirName = path.basename(phaseDir); - const phaseToken = extractPhaseToken(phaseDirName); + // #612: derive the token with the resolved convention so a bracket dir's + // own token is read behind its `{CODE}.{MM}-` prefix. #4187: keep the bare + // report tier aligned with the status reader. + const phaseToken = extractPhaseToken(phaseDirName, convention); const verificationFile = resolveVerificationFile(phaseFiles, { allowBare: true, phaseToken, phaseDirName, + convention, }); if (!verificationFile) return { determined: true, stale: false }; @@ -814,7 +837,12 @@ function readVerificationStatus( opts.phaseCleanCommitTimesMs ?? defaultPhaseCleanCommitTimesMs; const runtime = opts.runtime ?? 'claude'; - // Phase token for the gaps_found command + // Phase token for the gaps_found command — deliberately convention-LESS + // even when `opts.convention` is present: the token becomes a bare COMMAND + // ARGUMENT below, and a bare bracket phase number is milestone-ambiguous + // (`02` cannot tell GSD.01-02 from GSD.02-02), so the argument keeps its + // pre-#612 shape. The convention-aware token is derived separately for + // FILE RESOLUTION only (`resolutionToken`, at the readdir below). const baseName = path.basename(phaseDir); const phaseToken = extractPhaseToken(baseName); const derivedPhaseNumber = phaseToken.length > 0 ? phaseToken : baseName; @@ -831,17 +859,21 @@ function readVerificationStatus( let verificationFile: string | null = null; try { const entries = fsImpl.readdirSync(phaseDir); - // #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. #3511: baseName also scopes the - // fallback path to this same phase (see resolveVerificationFile docs). - // #4187: allowBare — the status reader must recognize a bare - // `VERIFICATION.md` exactly like `verification.resolve-file`, - // `determinePhaseStatus`, and the init verification_path projectors - // already do; without it a verified phase reported `missing` and - // recommended re-running execute-phase. - verificationFile = resolveVerificationFile(entries, { allowBare: true, phaseToken, phaseDirName: baseName }); + // #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 report. #612: derive a separate convention-aware RESOLUTION token + // for bracket directories while the routed command argument above stays + // convention-less and milestone-unambiguous. #4187: keep the bare report + // tier aligned with every other verification reader. + const resolutionToken = opts.convention === 'bracket' + ? extractPhaseToken(baseName, opts.convention) + : phaseToken; + verificationFile = resolveVerificationFile(entries, { + allowBare: true, + phaseToken: resolutionToken, + phaseDirName: baseName, + convention: opts.convention, + }); } catch { // Directory unreadable → treat as missing verificationFile = null; @@ -932,7 +964,12 @@ function readVerificationStatus( computeCoveredDigest(findProjectRoot(phaseDir), coveredFilesVal) !== coveredDigestVal || !allCurrentArtifactsCovered(phaseDir, coveredFilesVal); } else { - const staleCheck = findStaleVerificationSummary(phaseDir, fsImpl, phaseCleanCommitTimesMs); + const staleCheck = findStaleVerificationSummary( + phaseDir, + fsImpl, + phaseCleanCommitTimesMs, + opts.convention, + ); isStale = staleCheck.determined && staleCheck.stale; // staleCheck is either {determined:true, stale:false} (checked; nothing // stale) or {determined:false} (could not check — fs/scan/clock failure). @@ -986,6 +1023,12 @@ interface IsPhaseCompleteDeps { runtime?: string; /** Phase number appended to the routed command (#2617). */ phaseNumber?: string; + /** + * #612: the repo's resolved `phase_id_convention`, threaded through to + * readVerificationStatus so a bracket phase dir resolves and scopes its + * report exactly like its legacy twin. Omitted / null: unchanged. + */ + convention?: string | null; } interface PhaseCompletionValue { @@ -1039,6 +1082,7 @@ function isPhaseComplete( phaseCleanCommitTimesMs: deps.phaseCleanCommitTimesMs, runtime: deps.runtime, phaseNumber: deps.phaseNumber, + convention: deps.convention, }); return { diff --git a/tests/adr-612-bracket-phase-counting.test.cjs b/tests/adr-612-bracket-phase-counting.test.cjs index d6a1c5e75..547b77f5c 100644 --- a/tests/adr-612-bracket-phase-counting.test.cjs +++ b/tests/adr-612-bracket-phase-counting.test.cjs @@ -93,17 +93,39 @@ function writeProject(roadmap, convention, dirs = ['GSD.02-01-setup']) { } /** - * Name each fixture's verification report for that phase's own padded token, - * matching cmdScaffold. A hardcoded `01-VERIFICATION.md` turns every other - * "complete" phase into a cross-phase stray once artifact scans are scoped. - * Prefix stripping stays fixture-local because these cases intentionally model - * the unqualified artifact layout; padding comes from the production owner. + * The verification filename cmdScaffold writes into `dir` when the phase is + * named by its unqualified token — `${normalizePhaseName(phase)}-VERIFICATION.md` + * (`src/commands.cts`): the PHASE'S OWN padded token. + * + * UPDATED at the origin/next merge (#3511): every complete dir used to get a + * hardcoded `01-VERIFICATION.md`, which was harmless while nothing scoped a + * phase directory's listing. Upstream #3511 made membership real — in any dir + * that is not phase 01 that hardcoded name is now (correctly, on BOTH + * conventions once #612 threads `convention` into the seam) a cross-phase + * stray, excluded from completion. These fixtures mean "a COMPLETE phase", + * not "a phase holding another phase's report", so they must write what the + * scaffolder writes. The stray shape is pinned deliberately, as its own + * regression block — see "#612 PR-2: a cross-phase stray in a bracket dir" + * below — so this rename cannot silently stand in for the seam fix. + * + * The padding comes from the production `normalizePhaseName` itself (single + * owner, no fixture drift). What stays fixture-local is the bracket-prefix + * strip below — deliberately: production does NOT strip `{CODE}.` prefixes + * (`normalizePhaseName('GSD.02-01')` returns it unchanged via the custom-id + * arm; scaffold echoes whatever `--phase` form it is given), so a repo's + * artifact layout is unqualified or qualified depending on how phases were + * scaffolded. This file's fixtures model the UNQUALIFIED layout; the + * QUALIFIED layout is modeled by tests/adr-612-bracket-read-tolerance.test.cjs + * and by the qualified-stem assertions in the stray-regression block below. */ function verificationNameFor(dir) { + // Fixture-local unqualified-layout modeling (see docstring) — not a claim + // about scaffold, and not a second grammar owner: the seam's own tests + // assert the production regex. const rest = dir.replace(/^[A-Z][A-Z0-9_]*\.\d+-/i, ''); const token = []; - for (const segment of rest.split('-')) { - if (/^\d+(?:\.\d+)*$/.test(segment)) token.push(segment); + for (const seg of rest.split('-')) { + if (/^\d+(?:\.\d+)*$/.test(seg)) token.push(seg); else break; } assert.ok(token.length > 0, `fixture dir ${dir} carries no phase token to name its verification`); @@ -3219,3 +3241,178 @@ describe('#2761 round-11 BLOCKER: cmdStateUpdateProgress computes+writes a perce assert.strictEqual(parsed.total, 2); }); }); + +describe('#612 PR-2: a cross-phase stray in a bracket dir is excluded — the #3511 seam is convention-aware', () => { + // Pins the seam fix ITSELF, independently of the phase-matched fixture + // names verificationNameFor now writes. With phase-matched names + // everywhere, an inert (include-everything) bracket seam and a scoped one + // are indistinguishable — every earlier assertion in this file would pass + // under both. The genuine stray written here is the one shape that tells + // them apart, so the ALERT'd bracket-inertness (#3511's `isPhaseArtifact` + // fail-safes swallowing every bracket dir) cannot silently return. + const phaseIdMod = require('../gsd-core/bin/lib/phase-id.cjs'); + + beforeEach(() => { tmpDir = createTempProject('adr-612-stray-'); }); + afterEach(() => { cleanup(tmpDir); }); + + const STRAY_ROADMAP_BRACKET = `# Roadmap + +## [GSD.02] v2.0: Current + +### [GSD.02] 01: One +**Goal:** a + +### [GSD.02] 02: Two +**Goal:** b + +### [GSD.02] 03: Three +**Goal:** c +`; + const STRAY_ROADMAP_LEGACY = `# Roadmap + +## v2.0: Current + +### Phase 01: One +**Goal:** a + +### Phase 02: Two +**Goal:** b + +### Phase 03: Three +**Goal:** c +`; + + function plantStray(dir) { + // Another phase's PASSING report, misfiled into the INCOMPLETE phase 03's + // directory — exactly the hardcoded shape the pre-merge fixture wrote by + // accident. An inert seam folds it into phase 03's completion + // (3/3 -> 100); the scoped seam excludes it (2/3 -> 67). + fs.writeFileSync( + path.join(tmpDir, '.planning', 'phases', dir, '01-VERIFICATION.md'), + '---\nstatus: passed\n---\n# Verification\n', 'utf-8'); + } + + test('READ: a stray 01-VERIFICATION.md cannot complete bracket phase 03', () => { + writeProject(STRAY_ROADMAP_BRACKET, 'bracket', + ['GSD.02-01-one', 'GSD.02-02-two', ['GSD.02-03-three', false]]); + plantStray('GSD.02-03-three'); + assert.deepEqual(readProgress(), [3, 2, 3, 2, 67], + 'pinned [3,3,3,2,67] before #612 threaded convention into the seam — the stray ' + + 'counted as phase 03\'s completion; percent is plan-derived, so percent alone ' + + 'cannot catch this: the full vector is the assertion'); + }); + + test('READ: the legacy twin of the stray shape reads identically', () => { + writeProject(STRAY_ROADMAP_BRACKET, 'bracket', + ['GSD.02-01-one', 'GSD.02-02-two', ['GSD.02-03-three', false]]); + plantStray('GSD.02-03-three'); + const bracket = readProgress(); + writeProject(STRAY_ROADMAP_LEGACY, undefined, + ['01-one', '02-two', ['03-three', false]]); + plantStray('03-three'); + const legacy = readProgress(); + assert.deepEqual(bracket, legacy, + 'bracket must exclude the stray exactly as its legacy twin does'); + assert.deepEqual(legacy, [3, 2, 3, 2, 67]); + }); + + test('WRITE: sync agrees — the stray moves neither derivation', () => { + writeProject(STRAY_ROADMAP_BRACKET, 'bracket', + ['GSD.02-01-one', 'GSD.02-02-two', ['GSD.02-03-three', false]]); + plantStray('GSD.02-03-three'); + assert.equal(syncedPercent(), 67); + assert.deepEqual(syncedProgress(), [3, 2, 3, 2, 67], + 'pinned [3,3,3,2,67] before this fix (completed_phases moved; the plan-derived percent did not)'); + assert.deepEqual(readProgress(), syncedProgress()); + }); + + test('SEAM (the ALERT repro): scopeToPhase scopes a 3+-letter-code bracket dir under convention', () => { + // 3+-letter codes (`GSD.02-…`) fell into the ZERO-TOKEN fail-safe — + // `PROJECT_CODE_PREFIX_CAPTURE_RE_I` strips `{CODE}-`, not `{CODE}.` — + // NOT the `firstLetterPrefixed` branch the pre-#612 docblock named for + // the bracket family. Both families are pinned here so a fixer patching + // one branch cannot green this by half. + const files = ['01-01-x-PLAN.md', '01-01-x-SUMMARY.md', '01-VERIFICATION.md']; + assert.deepEqual(phaseIdMod.scopeToPhase(files, '02-two'), [], + 'legacy control: the twin scopes these to nothing'); + assert.deepEqual(phaseIdMod.scopeToPhase(files, 'GSD.02-02-two', 'bracket'), [], + 'pinned all-three-kept (scoping a structural no-op) before this fix'); + assert.deepEqual(phaseIdMod.scopeToPhase(files, 'A1.02-02-two', 'bracket'), [], + 'short digit-bearing codes hit the firstLetterPrefixed fail-safe pre-fix; same pin'); + }); + + test('SEAM: a bracket dir keeps its own artifacts — dash, dot continuation, and token-less containment', () => { + const own = ['02-VERIFICATION.md', '02-01-PLAN.md', '02.1-CONTEXT.md', 'VERIFICATION.md']; + assert.deepEqual(phaseIdMod.scopeToPhase(own, 'GSD.02-02-two', 'bracket'), own, + 'the scoped bracket dir must not drop its own files (over-exclusion is the defect class #3511 exists to fix)'); + // Sub-phase dir: own dotted token matches exactly, its .10 sibling does not + // (mirror of the legacy 35.1/35.10 pin in tests/phase-id.test.cjs). + assert.equal(phaseIdMod.isPhaseArtifact('02.1-VERIFICATION.md', 'GSD.02-02.1-fix', 'bracket'), true); + assert.equal(phaseIdMod.isPhaseArtifact('02.10-VERIFICATION.md', 'GSD.02-02.1-fix', 'bracket'), false); + // Recognition is case-insensitive, like every bracket reader. + assert.equal(phaseIdMod.isPhaseArtifact('02-VERIFICATION.md', 'gsd.02-02-two', 'bracket'), true); + }); + + test('SEAM: exact legacy-twin equality across a spread of filenames', () => { + const spread = ['01-VERIFICATION.md', '02-VERIFICATION.md', '2-VERIFICATION.md', + '02-CORRECTION-VERIFICATION.md', '02.1-CONTEXT.md', '02_VERIFICATION.md', + '03-UAT.md', 'VERIFICATION.md', 'notes.md']; + for (const f of spread) { + assert.equal( + phaseIdMod.isPhaseArtifact(f, 'GSD.02-02-two', 'bracket'), + phaseIdMod.isPhaseArtifact(f, '02-two'), + `bracket and legacy twins must agree on ${f}`); + } + }); + + test('SEAM: everything outside the well-formed-bracket-plus-convention gate is byte-unchanged', () => { + // No convention signal -> the documented include-everything fail-safe + // stands, even for a perfectly-formed bracket dir. + assert.deepEqual(phaseIdMod.scopeToPhase(['01-VERIFICATION.md'], 'GSD.02-02-two'), + ['01-VERIFICATION.md'], 'convention-less callers keep pre-#612 behavior verbatim'); + // Bracket-MALFORMED (1-digit milestone — the shape W005 reports) fails + // BRACKET_DIR_TOKEN_RE and degrades to the same fail-safe. + assert.deepEqual(phaseIdMod.scopeToPhase(['01-VERIFICATION.md'], 'GSD.2-02-two', 'bracket'), + ['01-VERIFICATION.md'], 'malformed bracket dirs keep the include-everything degrade'); + // A legacy-shaped dir in a bracket repo scopes by the LEGACY rule — the + // additive-read guarantee (the "carrying LEGACY-shaped dirs" test above, + // at the seam level). + assert.equal(phaseIdMod.isPhaseArtifact('01-VERIFICATION.md', '02-two', 'bracket'), false); + assert.equal(phaseIdMod.isPhaseArtifact('02-VERIFICATION.md', '02-two', 'bracket'), true); + // The letter-prefixed-decimal ambiguity (`P0.3-2` could be a legacy dir + // in a mid-migration bracket repo; its milestone field is not canonical + // bracket spelling) keeps upstream's pinned include-everything trade even + // WITH the convention threaded. + assert.equal(phaseIdMod.isPhaseArtifact('99-VERIFICATION.md', 'P0.3-2-slug', 'bracket'), true); + }); + + test('SEAM: qualified-stem filenames compare on the qualified key; M-NN stems ratify the twin-equal exclusion', () => { + // Review Major 2: the convention gates the FILENAME reading too. + assert.equal(phaseIdMod.isPhaseArtifact('GSD.02-05-VERIFICATION.md', 'GSD.02-05-real', 'bracket'), true); + assert.equal(phaseIdMod.isPhaseArtifact('GSD.02-01-VERIFICATION.md', 'GSD.02-05-real', 'bracket'), false, + 'pinned true (token-less containment admitted it) before this fix'); + assert.equal(phaseIdMod.isPhaseArtifact('GSD.01-05-VERIFICATION.md', 'GSD.02-05-real', 'bracket'), false, + 'same phase digits, different milestone — the qualified key separates what no bare token can'); + // Round-2 verify blocker: the qualified-key comparison must keep the + // dash-OR-dot continuation rule — a qualified dotted SUB-PHASE artifact is + // the parent phase's own file (see the DOTTED SUB-PHASE CONTINUATION + // docblock), exactly as its legacy twin (05.1-… in 05-parent) is. A strict + // equality here silently excluded it: over-exclusion, the dangerous + // direction for these aggregate scans. + assert.equal(phaseIdMod.isPhaseArtifact('GSD.02-05.1-VERIFICATION.md', 'GSD.02-05-parent', 'bracket'), true, + 'pinned false (strict key equality dropped the dot arm) before the round-2 fix'); + assert.equal(phaseIdMod.isPhaseArtifact('05.1-VERIFICATION.md', '05-parent'), true, + 'the legacy-twin control for the line above'); + assert.equal(phaseIdMod.isPhaseArtifact('GSD.02-01.1-VERIFICATION.md', 'GSD.02-05-parent', 'bracket'), false, + 'a DIFFERENT phase\'s dotted sub-phase artifact is still a stray'); + assert.equal(phaseIdMod.isPhaseArtifact('GSD.02-01-VERIFICATION.md', 'GSD.02-05-real'), true, + 'convention-less: the include-everything fail-safe stands, byte-identical to pre-#612'); + // Review Major 1, RATIFIED as the twin-equal trade: an M-NN-stem report in a + // bracket dir is excluded exactly as its legacy twin excludes the same file; + // migration-window artifact renaming is the migrator slice's job (ADR-612). + assert.equal(phaseIdMod.isPhaseArtifact('02-01-VERIFICATION.md', 'GSD.02-01-one', 'bracket'), false, + 'twin-parity: isPhaseArtifact("02-01-VERIFICATION.md","01-one") is false for the same reason'); + assert.equal(phaseIdMod.isPhaseArtifact('02-01-VERIFICATION.md', '01-one'), false, + 'the legacy-twin control for the line above'); + }); +}); diff --git a/tests/verification-status.test.cjs b/tests/verification-status.test.cjs index 6e0316df7..706547baf 100644 --- a/tests/verification-status.test.cjs +++ b/tests/verification-status.test.cjs @@ -1411,6 +1411,77 @@ describe('#4187: status surface recognizes a bare VERIFICATION.md', () => { }); }); +// ─── #4142: bracket convention reaches the legacy staleness seam ──────────── +// +// readVerificationStatus resolves the report once to read its frontmatter and +// findStaleVerificationSummary resolves it again for the legacy mtime check. +// Both resolutions must receive the same convention or a bracket directory can +// read status from its own report, then compute staleness from a cross-phase +// stray in that same directory. +describe('#4142: opts.convention threads through findStaleVerificationSummary', () => { + test('a bracket phase checks staleness against its own report, not a newer cross-phase stray', (t) => { + const dir = mkPhaseDir('bracket-stale-thread', 'GSD.02-03-three'); + t.after(() => cleanup(path.dirname(dir))); + + writeVerificationMd(dir, '03-VERIFICATION.md', 'passed'); + writeVerificationMd(dir, '01-VERIFICATION.md', 'passed'); + setMtime(path.join(dir, '03-VERIFICATION.md'), '2020-01-01T00:00:00Z'); + setMtime(path.join(dir, '01-VERIFICATION.md'), '2022-01-01T00:00:00Z'); + const summaryPath = path.join(dir, '03-01-SUMMARY.md'); + fs.writeFileSync(summaryPath, '# summary\n'); + setMtime(summaryPath, '2021-01-01T00:00:00Z'); + + const result = readVerificationStatus(dir, { + convention: 'bracket', + phaseCleanCommitTimesMs: () => new Map(), + }); + + assert.equal( + result.status, + 'stale', + 'opts.convention must reach the legacy staleness seam so phase 03 is compared to 03-VERIFICATION.md', + ); + assert.equal(result.next_command, '/gsd-verify-work'); + }); +}); + +// ─── #4142: phase complete must use the convention-aware verdict ─────────── +// +// cmdPhaseComplete has an advisory VERIFICATION pre-scan and a separate +// readVerificationStatus completion gate. Exercise the real CLI verdict so a +// cross-phase report cannot satisfy the gate merely by sitting in the bracket +// phase's directory. +describe('#4142: phase complete verdict scopes bracket verification reports', () => { + const { runGsdTools } = require('./helpers.cjs'); + + test('a passed cross-phase stray cannot complete a bracket phase', (t) => { + const projectDir = createTempGitProject('gsd-4142-phase-complete-'); + t.after(() => cleanup(projectDir)); + + fs.writeFileSync( + path.join(projectDir, '.planning', 'config.json'), + JSON.stringify({ phase_id_convention: 'bracket' }, null, 2), + ); + const phaseDirName = 'GSD.02-03-three'; + const phaseDir = path.join(projectDir, '.planning', 'phases', phaseDirName); + fs.mkdirSync(phaseDir, { recursive: true }); + writeVerificationMd(phaseDir, '01-VERIFICATION.md', 'passed'); + + const result = runGsdTools( + ['--json-errors', 'phase', 'complete', phaseDirName], + projectDir, + ); + + assert.equal( + result.success, + false, + 'phase complete must reject another phase\'s passed verification report', + ); + const errorPayload = JSON.parse(result.error); + assert.equal(errorPayload.reason, 'phase_verification_incomplete'); + }); +}); + // ─── #4187 CLI parity: the two query verbs must agree on the same directory ─── // // The issue's repro shape: run `query verification.resolve-file` and