diff --git a/.changeset/noble-tunas-travel.md b/.changeset/noble-tunas-travel.md new file mode 100644 index 000000000..988665ccb --- /dev/null +++ b/.changeset/noble-tunas-travel.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4640 +--- +**Milestone-name, branch-name, and phase-insert allocation bugs consolidated at the seam** โ€” a punctuation-only ๐Ÿšง-bullet name (e.g. a malformed `๐Ÿšง **v3.3** ---`) could surface as a real milestone name in two of three capture sites; an undeliverable `phase_slug` produced a branch name ending in the literal `-phase` instead of dropping the segment; `phase insert` (and `phase next-decimal`) could silently reallocate a decimal sub-phase number that existed only as a roadmap checklist bullet, with no way to request a sibling instead of always nesting one level deeper. All three are now single, shared implementations (`hasNameableContent`, `renderPhaseBranchName`, `scanExistingDecimalPhaseNumbers`) applied everywhere the concept is used instead of each consumer reimplementing it independently, with the phase-id anti-divergence guard extended to catch a re-derivation of any of them โ€” and, separately, to catch banned $((10#...)) shell arithmetic on phase-number variables in workflow/reference docs. `phase insert` gains a `--sibling` flag. (#4126, #4433, #4569, #4634) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index a7e8704fa..5f03fcfdb 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -162,7 +162,7 @@ node gsd-tools.cjs phase next-decimal node gsd-tools.cjs phase add # Insert decimal phase after existing -node gsd-tools.cjs phase insert +node gsd-tools.cjs phase insert [--sibling] # Remove phase, renumber subsequent node gsd-tools.cjs phase remove [--force] diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 06bb9ee1d..19ecb9195 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1305,6 +1305,8 @@ non-default branch โ€” see issue #3819. | `{milestone}` | `milestone_branch_template` | `v1.0` | | `{num}` / `{quick}` | `quick_branch_template` | `260317-abc` (quick task ID) | +**`phase_branch_template`'s `{slug}` when no slug can be derived** (no name segment on disk, or a name entirely outside the slug generator's scope): the `{slug}` token โ€” and one adjacent separator โ€” is dropped rather than substituted with a placeholder word, so `gsd/phase-{phase}-{slug}` renders `gsd/phase-08`, not `gsd/phase-08-phase`. This keeps the branch name honest about what it does not know instead of reading as a real (but wrong) name. + Example quick-task branching: ```json diff --git a/scripts/lint-phase-id-drift.cjs b/scripts/lint-phase-id-drift.cjs index 4737b62b9..a46157e1c 100644 --- a/scripts/lint-phase-id-drift.cjs +++ b/scripts/lint-phase-id-drift.cjs @@ -20,6 +20,34 @@ * directly above). Sites that build their regex from `PHASE_NUMBER_TOKEN_SOURCE` * carry no literal grammar and pass automatically. * + * #4634 extends the same pattern with two more detectors: + * + * - name-validity-guard drift: `hasNameableContent(s)` in `src/roadmap-parser.cts` + * is the sole owner of the "does this string have nameable content" predicate + * (`/[\p{L}\p{N}]/u.test(s)`). Any other `src/**` file re-deriving that exact + * character class (regex-literal or `new RegExp` template form) instead of + * calling `hasNameableContent` is drift, sanctioned the same way as the token + * and bracket rules (`// phase-id-owner:` on the nearest preceding non-blank + * line), with a line-level escape for a line that already calls + * `hasNameableContent(`. + * + * - shell phase-number-arithmetic ban: `$((10#...))` base-10-forced arithmetic + * inside `gsd-core/workflows/**\/*.md` and `gsd-core/references/**\/*.md` breaks + * on decimal or multi-segment phase ids and is banned outright. This scan runs + * over markdown, not `.cts` source, so its sanction is an HTML comment on the + * nearest preceding non-blank line: ``. + * + * - branch-slug fallback drift: a `.replace('{slug}', ... || 'phase')` call + * silently substitutes the literal string `'phase'` when a phase's slug + * can't be derived, producing a non-identifying branch name like + * `gsd/phase-08-phase` (#4126, now fixed via the shared renderPhaseBranchName + * owner in phase-id.cts, consumed by both prior call sites). Sanctioned + * the same way as the token/bracket/name-validity rules (`// phase-id-owner:` + * on the nearest preceding non-blank line), with a line-level escape for a + * line that already calls `renderPhaseBranchName(`. Unlike the other + * `.cts`-scanning rules, this one has no per-file exemption โ€” it is a banned + * anti-pattern everywhere, not a grammar with one legitimate owner site. + * * Detection is intentionally NARROW: only the contiguous canonical token * (`\d+[A-Z]?(?:\.\d+)*`, its `[A-Za-z]` and `[.-]` near-variants, in both * regex-literal `\d` and `new RegExp` template `\\d` escaping) is drift. Bare @@ -82,6 +110,18 @@ const BRACKET_CODE_DRIFT_RE = /\[A-Z(?:a-z)?\]\[A-Z(?:a-z)?0-9_\]\*/; const BRACKET_OWNER_HINT = 'BRACKET_PROJECT_CODE_SRC / BRACKET_ID_SRC / bracketMilestoneIntroSrcFor / BRACKET_MILESTONE_INTRO_CAPTURING_SRC'; +/** + * Pure: true if the nearest preceding non-blank line to `lines[i]` is a + * dedicated sanction comment matching `ownerRe`. Shared by every detector in + * this file so the "how do you sanction a finding" walk has one owner instead + * of four independent copies that could silently diverge. + */ +function isSanctionedByPrecedingComment(lines, i, ownerRe) { + let j = i - 1; + while (j >= 0 && lines[j].trim() === '') j--; // nearest preceding non-blank line + return j >= 0 && ownerRe.test(lines[j]); +} + /** * Pure: find every literal re-derivation of the canonical phase-number token in * `text` that is NOT sanctioned. A site is sanctioned when the nearest preceding @@ -100,9 +140,7 @@ function findPhaseIdRegexDrift(text) { const m = TOKEN_DRIFT_RE.exec(line); if (!m) continue; if (line.includes(CANON_REF)) continue; - let j = i - 1; - while (j >= 0 && lines[j].trim() === '') j--; // nearest preceding non-blank line - if (j >= 0 && OWNER_RE.test(lines[j])) continue; + if (isSanctionedByPrecedingComment(lines, i, OWNER_RE)) continue; out.push({ line: i + 1, found: m[0] }); } return out; @@ -122,21 +160,148 @@ function findBracketGrammarDrift(text) { for (let i = 0; i < lines.length; i++) { const m = BRACKET_CODE_DRIFT_RE.exec(lines[i]); if (!m) continue; - let j = i - 1; - while (j >= 0 && lines[j].trim() === '') j--; // nearest preceding non-blank line - if (j >= 0 && OWNER_RE.test(lines[j])) continue; + if (isSanctionedByPrecedingComment(lines, i, OWNER_RE)) continue; out.push({ line: i + 1, found: m[0] }); } return out; } +// #4634: the name-validity-guard grammar โ€” `hasNameableContent(s)` in +// `src/roadmap-parser.cts` is `/[\p{L}\p{N}]/u.test(s)`. Tolerates both the +// regex-literal single-backslash form and the doubled-backslash template +// form (`new RegExp('[\\p{L}\\p{N}]'`), mirroring how TOKEN_DRIFT_RE tolerates +// both escapings. +const NAME_VALIDITY_DRIFT_RE = /\[\\{1,2}p\{L\}\\{1,2}p\{N\}\]/; +const NAME_VALIDITY_CANON_REF = 'hasNameableContent('; + +/** + * Pure: find every literal re-derivation of the canonical name-validity + * character class in `text` that is NOT sanctioned. Same sanction mechanism + * as the token/bracket rules โ€” a dedicated `// phase-id-owner:` comment on + * the nearest preceding non-blank line โ€” plus a line-level escape for a line + * that already calls `hasNameableContent(`. Returns [{ line, found }]. + */ +function findNameValidityDrift(text) { + const out = []; + const lines = text.split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const m = NAME_VALIDITY_DRIFT_RE.exec(line); + if (!m) continue; + if (line.includes(NAME_VALIDITY_CANON_REF)) continue; + if (isSanctionedByPrecedingComment(lines, i, OWNER_RE)) continue; + out.push({ line: i + 1, found: m[0] }); + } + return out; +} + +// #4634: the branch-slug fallback anti-pattern (#4126) โ€” a +// `.replace('{slug}', ... || 'phase')` call silently falls back to the +// literal string `'phase'` when a phase's slug can't be derived, producing a +// non-identifying branch name like `gsd/phase-08-phase`. Now fixed at both +// prior call sites (commands.cts, init.cts) via the shared +// `renderPhaseBranchName` owner in phase-id.cts; this rule is the ratchet +// against a THIRD site reintroducing the inline fallback. Deliberately +// narrow: it requires the literal `'phase'` fallback on the same line as the +// `{slug}` template token, so it does NOT match the sibling milestone-branch +// fallback (`|| 'milestone'`), which is a different, correct-as-is case. +const BRANCH_SLUG_FALLBACK_DRIFT_RE = /\{slug\}'.*\|\|\s*'phase'/; + +// The canonical fix is `renderPhaseBranchName(...)`. There is no "owner file" +// for this rule the way there is for the token/bracket/name-validity +// grammars above โ€” it is a banned anti-pattern everywhere, so no per-file +// exemption exists. +const BRANCH_SLUG_FALLBACK_CANON_REF = 'renderPhaseBranchName('; + +/** + * Pure: find every unsanctioned branch-slug `|| 'phase'` fallback in `text`. + * Same sanction mechanism as the token/bracket/name-validity rules โ€” a + * dedicated `// phase-id-owner:` comment on the nearest preceding non-blank + * line โ€” plus a line-level escape for a line that already calls + * `renderPhaseBranchName(`. Returns [{ line, found }]. + */ +function findBranchSlugFallbackDrift(text) { + const out = []; + const lines = text.split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const m = BRANCH_SLUG_FALLBACK_DRIFT_RE.exec(line); + if (!m) continue; + if (line.includes(BRANCH_SLUG_FALLBACK_CANON_REF)) continue; + if (isSanctionedByPrecedingComment(lines, i, OWNER_RE)) continue; + out.push({ line: i + 1, found: m[0] }); + } + return out; +} + +// #4634: ban base-10-forced shell arithmetic (`$((10#...))`) on any variable โ€” +// this construct is exactly the pattern that breaks on a decimal or +// multi-segment phase id, so any occurrence is banned outright, full stop. +const SHELL_PHASE_ARITH_DRIFT_RE = /\$\(\(\s*10#/; + +// A markdown comment can't easily carry a `//` line, so the sanction for the +// shell-arithmetic rule is an HTML comment on the nearest preceding non-blank +// line: ``. +const MD_OWNER_RE = /^\s*` + * on the nearest preceding non-blank line. Returns [{ line, found }]. + */ +function findShellPhaseArithDrift(text) { + const out = []; + const lines = text.split('\n'); + for (let i = 0; i < lines.length; i++) { + const m = SHELL_PHASE_ARITH_DRIFT_RE.exec(lines[i]); + if (!m) continue; + if (isSanctionedByPrecedingComment(lines, i, MD_OWNER_RE)) continue; + out.push({ line: i + 1, found: m[0] }); + } + return out; +} + +// #4634: the markdown scan roots โ€” shell embedded in workflow/reference docs. +const MD_SCAN_DIRS = [path.join('gsd-core', 'workflows'), path.join('gsd-core', 'references')]; + +/** + * Scan `gsd-core/workflows/**\/*.md` and `gsd-core/references/**\/*.md` for + * unsanctioned `$((10#...))` shell arithmetic. Returns [{ file, line, found }] + * with repo-relative paths. + */ +function scanMarkdownShellArith(root) { + const violations = []; + for (const dir of MD_SCAN_DIRS) { + for (const file of walkMd(path.join(root, dir), [])) { + const rel = path.relative(root, file); + let text; + try { + text = fs.readFileSync(file, 'utf8'); + } catch { + continue; + } + for (const d of findShellPhaseArithDrift(text)) { + violations.push({ file: rel, kind: 'shell-arith', ...d }); + } + } + } + return violations; +} + // Authored TypeScript source only (the generated bin/lib/*.cjs mirror it). const SCAN_DIRS = ['src']; const SCAN_EXT = new Set(['.cts', '.ts', '.mts']); // The canonical owner defines the grammar; it is exempt by construction. const EXEMPT = new Set([path.join('src', 'phase-id.cts')]); -function walk(dir, acc) { +// #4634: the name-validity-guard rule owns a DIFFERENT file (roadmap-parser.cts +// defines `hasNameableContent`), so it needs its own exemption set โ€” the +// token/bracket rules above must NOT start exempting roadmap-parser.cts too, +// since it is not their owner. +const NAME_VALIDITY_EXEMPT = new Set([path.join('src', 'roadmap-parser.cts')]); + +function walk(dir, acc, ext) { + const extSet = ext || SCAN_EXT; let entries; try { entries = fs.readdirSync(dir, { withFileTypes: true }); @@ -147,14 +312,22 @@ function walk(dir, acc) { const full = path.join(dir, entry.name); if (entry.isDirectory()) { if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === '.git') continue; - walk(full, acc); - } else if (entry.isFile() && SCAN_EXT.has(path.extname(entry.name))) { + walk(full, acc, extSet); + } else if (entry.isFile() && extSet.has(path.extname(entry.name))) { acc.push(full); } } return acc; } +// #4634: markdown scan for the shell phase-arithmetic ban walks a disjoint set +// of roots/extensions from the src/**/*.cts scan above, so it gets its own thin +// wrapper over the same `walk` rather than a parallel tree-walker. +const MD_EXT = new Set(['.md']); +function walkMd(dir, acc) { + return walk(dir, acc, MD_EXT); +} + // โ”€โ”€โ”€ #2761 M4: the heading-baseline selector census โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ // // `phaseHeadingPrefixSrcFor(PHASE_HEADING_BASELINE.)` is the other half @@ -239,23 +412,55 @@ function scanRepo(root) { for (const d of findBracketGrammarDrift(text)) { violations.push({ file: rel, kind: 'bracket', ...d }); } + // #4634: name-validity-guard drift, exempting only its own owner file. + if (!NAME_VALIDITY_EXEMPT.has(rel)) { + for (const d of findNameValidityDrift(text)) { + violations.push({ file: rel, kind: 'name-validity', ...d }); + } + } + // #4634: branch-slug fallback anti-pattern, exempt nowhere. + for (const d of findBranchSlugFallbackDrift(text)) { + violations.push({ file: rel, kind: 'branch-slug-fallback', ...d }); + } } } return violations; } +/** + * Scan EVERYTHING this seam guards: the `src/**\/*.cts` token/bracket/ + * name-validity invariants (`scanRepo`) plus the markdown shell + * phase-arithmetic ban (`scanMarkdownShellArith`). This is what the CLI runs; + * `scanRepo` alone stays narrowly scoped to its original src/** contract so + * a live, separately-tracked markdown defect (#4619) cannot make the + * pinned-clean `scanRepo` test spuriously fail. + */ +function scanAll(root) { + return [...scanRepo(root), ...scanMarkdownShellArith(root)]; +} + function main() { const root = path.join(__dirname, '..'); - const violations = scanRepo(root); + const violations = scanAll(root); if (violations.length === 0) { - process.stdout.write('ok phase-id-drift: no unsanctioned phase-token or bracket-grammar re-derivations outside phase-id.cts\n'); + process.stdout.write( + 'ok phase-id-drift: no unsanctioned phase-token, bracket-grammar, name-validity, ' + + 'branch-slug-fallback, or shell phase-arithmetic re-derivations found\n', + ); return; } process.stderr.write('phase-id-drift: literal re-derivation(s) of a canonical grammar found.\n'); process.stderr.write(`Build the regex from phase-id.cjs \`${CANON_REF}\` (or phaseMarkdownRegexSource for a\n`); process.stderr.write(`known number) for the phase-number token, or from ${BRACKET_OWNER_HINT}\n`); - process.stderr.write('for the bracket grammar โ€” or sanction the site with a dedicated\n'); - process.stderr.write('`// phase-id-owner: ` comment on the line directly above the regex:\n'); + process.stderr.write('for the bracket grammar, or call `hasNameableContent(` (src/roadmap-parser.cts) for\n'); + process.stderr.write('the name-validity predicate โ€” or sanction the site with a dedicated\n'); + process.stderr.write('`// phase-id-owner: ` comment on the line directly above the regex.\n'); + process.stderr.write('`$((10#...))` base-10-forced shell arithmetic is banned outright in\n'); + process.stderr.write('gsd-core/workflows/**/*.md and gsd-core/references/**/*.md โ€” sanction with\n'); + process.stderr.write('`` on the line directly above.\n'); + process.stderr.write('A `.replace(\'{slug}\', ... || \'phase\')` fallback is banned outright (#4126) โ€”\n'); + process.stderr.write('use `renderPhaseBranchName(` or sanction with\n'); + process.stderr.write('`// phase-id-owner: ` on the line directly above:\n'); for (const d of violations) { process.stderr.write(` [${d.kind}] ${d.file}:${d.line} ${d.found}\n`); } @@ -267,9 +472,17 @@ if (require.main === module) main(); module.exports = { findPhaseIdRegexDrift, findBracketGrammarDrift, + findNameValidityDrift, + findBranchSlugFallbackDrift, + findShellPhaseArithDrift, + scanMarkdownShellArith, scanRepo, + scanAll, countSelectorBaselines, scanSelectorBaselines, TOKEN_DRIFT_RE, BRACKET_CODE_DRIFT_RE, + NAME_VALIDITY_DRIFT_RE, + BRANCH_SLUG_FALLBACK_DRIFT_RE, + SHELL_PHASE_ARITH_DRIFT_RE, }; diff --git a/src/commands.cts b/src/commands.cts index 95f9ccbe3..39b8c70be 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -23,7 +23,7 @@ import coreUtilsMod = require('./core-utils.cjs'); const { toPosixPath, generateSlugInternal, extractOneLinerFromBody } = coreUtilsMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, comparePhaseNum, extractPhaseToken, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, comparePhaseNum, extractPhaseToken, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId, renderPhaseBranchName } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { getArchivedPhaseDirs, findPhaseInternal, listMilestonePhaseDirs } = phaseLocatorMod; @@ -1489,19 +1489,33 @@ function detectPhaseNumberFromFiles(files: string[] | undefined): string | null const phaseDir = segments[i + 1]; if (!phaseDir) continue; const token = extractPhaseToken(phaseDir); - // extractPhaseToken falls back to returning dirName unchanged when no - // numeric token is found. normalizePhaseName is the canonical arbiter - // of "is this a real phase token": it strips the project-code prefix - // and returns a zero-padded numeric form for a genuine phase token, or - // the input unchanged otherwise. Accept the token only when it - // normalizes to a numeric phase form (the single-owner rule shared by - // every other phase-token reader โ€” see #2528). - const normalized = normalizePhaseName(token); + // normalizePhaseName is the canonical arbiter of "is this a real phase + // token": it strips the project-code prefix and returns a zero-padded + // numeric form for a genuine phase token, or the input unchanged + // otherwise. Accept the token whenever it normalizes to a numeric + // phase form (the single-owner rule shared by every other phase-token + // reader โ€” see #2528). + // + // #4126 fix: this used to also require `token !== phaseDir`, on the + // assumption that extractPhaseToken returning its input unchanged + // always means "no numeric token found" (its no-match fallback). + // That assumption is false for a BARE phase directory with no slug + // remainder (e.g. `.planning/phases/01/`): extractPhaseToken correctly + // reads "01" as the token, which is simply identical to the directory + // name in that case โ€” not a fallback. The stale equality check + // rejected every such directory, leaving `phaseNum` null and silently + // skipping the whole phase-branch block below (undetected because + // `phaseTokenShape.test(normalized)` already excludes genuine + // non-phase fallbacks โ€” e.g. `docs`, `CK-docs` โ€” on its own, since + // extractPhaseToken's real no-match fallback only fires for dirNames + // that do not start with a digit or short letter+digit prefix, which + // normalizePhaseName's leading-`\d+` requirement rejects regardless). // Built from the single-owner PHASE_NUMBER_TOKEN_SOURCE (the canonical // phase-number grammar โ€” #2128 anti-divergence guard) so this read-side // acceptance check cannot drift from every other phase-token reader. + const normalized = normalizePhaseName(token); const phaseTokenShape = new RegExp(`^${PHASE_NUMBER_TOKEN_SOURCE}$`, 'i'); - if (token !== phaseDir && phaseTokenShape.test(normalized)) { + if (phaseTokenShape.test(normalized)) { return token; } } @@ -1990,9 +2004,15 @@ function cmdCommit(cwd: string, message: string | undefined, files: string[] | u if (phaseNum && !isSentinelPhaseId(phaseNum)) { const phaseInfo = findPhaseInternal(cwd, phaseNum) as Record | null; if (phaseInfo) { - branchName = (config['phase_branch_template'] as string) - .replace('{phase}', normalizePhaseName(phaseInfo['phase_number'])) - .replace('{slug}', (phaseInfo['phase_slug'] as string) || 'phase'); + // #4126: shared with init.cts's cmdInitExecutePhase branch_name field + // via the one canonical renderer (src/phase-id.cts) so an undeliverable + // phase_slug degrades identically at both call sites instead of each + // independently substituting the literal word 'phase'. + branchName = renderPhaseBranchName( + config['phase_branch_template'] as string, + phaseInfo['phase_number'], + phaseInfo['phase_slug'], + ); } } } else if (branchingStrategy === 'milestone') { diff --git a/src/init.cts b/src/init.cts index ca21823b5..93bfc37fe 100644 --- a/src/init.cts +++ b/src/init.cts @@ -97,7 +97,7 @@ const { extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { comparePhaseNum, normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId, extractPhaseToken, scopeToPhase } = phaseId; +const { comparePhaseNum, normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId, extractPhaseToken, scopeToPhase, renderPhaseBranchName } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { @@ -989,10 +989,11 @@ function cmdInitExecutePhase( branch_name: config.branching_strategy === 'phase' && phaseInfo - ? (config.phase_branch_template as string) - .replace('{project}', (config.project_code as string) || '') - .replace('{phase}', normalizePhaseName(phaseInfo['phase_number'])) - .replace('{slug}', (phaseInfo['phase_slug'] as string) || 'phase') + ? renderPhaseBranchName( + (config.phase_branch_template as string).replace('{project}', (config.project_code as string) || ''), + phaseInfo['phase_number'], + phaseInfo['phase_slug'], + ) : config.branching_strategy === 'milestone' ? (config.milestone_branch_template as string) .replace('{milestone}', (milestone['version'] as string | undefined) ?? '') diff --git a/src/phase-command-router.cts b/src/phase-command-router.cts index 50c243314..1472b67f4 100644 --- a/src/phase-command-router.cts +++ b/src/phase-command-router.cts @@ -36,7 +36,13 @@ interface PhaseHandlers { cmdPhaseNextDecimal: (cwd: string, arg: string | undefined, raw: boolean) => void; cmdPhaseAdd: (cwd: string, desc: string, raw: boolean, customId: string | null) => void; cmdPhaseAddBatch: (cwd: string, descriptions: string[], raw: boolean) => void; - cmdPhaseInsert: (cwd: string, pos: string | undefined, desc: string, raw: boolean) => void; + cmdPhaseInsert: ( + cwd: string, + pos: string | undefined, + desc: string, + raw: boolean, + allocation?: 'nested' | 'sibling', + ) => void; cmdPhaseRemove: (cwd: string, phaseNum: string, opts: { force: boolean }, raw: boolean) => void; cmdPhaseComplete: (cwd: string, phaseNum: string | undefined, raw: boolean) => void; cmdPhaseUatPassed: (cwd: string, phaseNum: string | undefined, raw: boolean, opts?: { policy?: { requireVerification?: boolean } }) => void; @@ -154,7 +160,19 @@ function routePhaseCommand({ phase, args, cwd, raw, error }: RoutePhaseCommandOp if (args.includes('--dry-run')) { return makeInvalidArgs('--dry-run', 'phase insert does not support --dry-run'); } - phase.cmdPhaseInsert(cwd, args[2], args.slice(3).join(' '), raw); + // #4569: --sibling opts into joining afterPhase's parent decimal level + // instead of nesting one level deeper. Filtered out like other + // boolean flags (see `remove`'s --force handling above) so it never + // leaks into the free-text description. + const sibling = args.includes('--sibling'); + const insertArgs = args.slice(2).filter(token => token !== '--sibling'); + phase.cmdPhaseInsert( + cwd, + insertArgs[0], + insertArgs.slice(1).join(' '), + raw, + sibling ? 'sibling' : 'nested', + ); return { ok: true as const, data: null }; }, remove: (_ctx: Record) => { diff --git a/src/phase-id.cts b/src/phase-id.cts index 4335e5bfd..2e897274d 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -1471,6 +1471,65 @@ function isForeignPrefixedPhaseQuery(phase: unknown, projectCode: unknown): bool * form so a canonical heading (`### Phase 117:`) is preferred over a drifted * prefixed one (`### Phase MANIFOLD-117:`) when both exist in one ROADMAP. */ +// Separator characters a branch-name template may plausibly use to join +// `{slug}` to its neighbours โ€” the four this repo's shipped templates and +// docs/CONFIGURATION.md's custom-template examples use (`/`, `-`, `_`, `.`). +const BRANCH_TEMPLATE_SEP_RE = /[-_./]/; +const BRANCH_TEMPLATE_SEP_RUN_RE = /([-_./])\1+/g; +const BRANCH_TEMPLATE_EDGE_SEP_RE = /^[-_./]+|[-_./]+$/g; + +/** + * #4126: the ONE branch-name-template renderer for the `phase` branching + * strategy, shared by `cmdCommit` (src/commands.cts) and + * `cmdInitExecutePhase` (src/init.cts) so the two call sites cannot diverge + * on how an undeliverable `phaseSlug` degrades (they previously each + * independently substituted the literal word `phase`, producing a + * non-identifying branch name like `gsd/phase-08-phase` that contradicts an + * honestly-reported `phase_slug: null`). + * + * `{project}` is deliberately NOT handled here โ€” init.cts substitutes it + * separately, before calling this, because it is a config-level field with + * its own fallback (`''`), not a phase-derived one. + * + * When `phaseSlug` is a non-empty string, `{slug}` substitutes normally + * (unchanged from prior behavior). When it is empty or not a string (e.g. + * `null`, `undefined`, a number), the `{slug}` token is DROPPED โ€” along with + * one adjacent separator character โ€” rather than replaced with a placeholder + * word: a dropped token keeps the branch name honest about what it does not + * know, where a placeholder reads as a real (but wrong) name. The result is + * always a syntactically plausible git-ref fragment: no leading, trailing, or + * doubled separator, for both the shipped default template + * (`gsd/phase-{phase}-{slug}`) and a differently-shaped custom one (this + * field is user-configurable per docs/CONFIGURATION.md). + */ +function renderPhaseBranchName(template: string, phaseNumber: unknown, phaseSlug: unknown): string | null { + const withPhase = template.replace('{phase}', normalizePhaseName(phaseNumber)); + const slug = typeof phaseSlug === 'string' ? phaseSlug : ''; + if (slug) return withPhase.replace('{slug}', slug); + + const at = withPhase.indexOf('{slug}'); + if (at === -1) return withPhase; + + let before = withPhase.slice(0, at); + let after = withPhase.slice(at + '{slug}'.length); + // Drop exactly ONE adjacent separator โ€” preferring the one immediately + // before the token โ€” so `a-{slug}` and `{slug}-a` both degrade to `a` + // rather than leaving a dangling `a-` / `-a`. + if (before && BRANCH_TEMPLATE_SEP_RE.test(before[before.length - 1])) { + before = before.slice(0, -1); + } else if (after && BRANCH_TEMPLATE_SEP_RE.test(after[0])) { + after = after.slice(1); + } + + // Collapse any doubled separator the drop can leave behind (e.g. a + // `feature//{slug}` template dropping to `feature/`โ†’`feature`), then trim + // a resulting leading/trailing separator โ€” valid for the shipped default + // template AND any user-configured shape, not just the one this repo ships. + const joined = before + after; + const collapsed = joined.replace(BRANCH_TEMPLATE_SEP_RUN_RE, '$1').replace(BRANCH_TEMPLATE_EDGE_SEP_RE, ''); + return collapsed || null; +} + function roadmapPhaseLookupSources(phaseNum: unknown): string[] { const sources: string[] = []; const exactSource = phaseMarkdownRegexSourceExact(phaseNum); @@ -1532,4 +1591,5 @@ export = { stripConfiguredProjectCodePrefix, isForeignPrefixedPhaseQuery, roadmapPhaseLookupSources, + renderPhaseBranchName, }; diff --git a/src/phase.cts b/src/phase.cts index df1679537..94ffbbc95 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -55,7 +55,10 @@ const { import { escapeRegex } from './pattern.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-locator.cjs is an export= CommonJS module import phaseLocatorMod = require('./phase-locator.cjs'); -const { findPhaseInternal, getArchivedPhaseDirs, listMilestonePhaseDirs } = phaseLocatorMod; +const { findPhaseInternal, getArchivedPhaseDirs, listMilestonePhaseDirs, listAllPhaseDirs } = phaseLocatorMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module +import planningScopeMod = require('./planning-scope.cjs'); +const { SCOPE } = planningScopeMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- roadmap-parser.cjs is an export= CommonJS module import roadmapParserMod = require('./roadmap-parser.cjs'); const { stripShippedMilestones, extractCurrentMilestone, currentMilestoneRawRanges, withPhaseSection, findMilestoneScopeHeadingLines } = roadmapParserMod; @@ -338,28 +341,25 @@ function cmdPhaseNextDecimal(cwd: string, basePhase: string, raw: boolean): void const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); const dirs = entries.filter((e) => e.isDirectory()).map((e) => e.name); baseExists = matchPhaseDirs(dirs, normalized).matches.length > 0; - - const dirPattern = new RegExp(`^${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}${escapeRegex(normalized)}\\.(\\d+)`); - for (const dir of dirs) { - const match = dir.match(dirPattern); - if (match) decimalSet.add(parseInt(match[1], 10)); - } } const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); if (fs.existsSync(roadmapPath)) { try { const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); - const phasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+${phaseMarkdownRegexSource(normalized)}\\.(\\d+)${OPTIONAL_PHASE_TAG_SOURCE}\\s*:`, - 'gi', - ); - let pm: RegExpExecArray | null; - while ((pm = phasePattern.exec(roadmapContent)) !== null) { - decimalSet.add(parseInt(pm[1], 10)); + for (const n of scanExistingDecimalPhaseNumbers(phasesDir, roadmapContent, normalized)) { + decimalSet.add(n); } } catch { - /* ROADMAP.md read failure is non-fatal */ + // ROADMAP.md read failure is non-fatal โ€” fall back to the directory-only + // scan (empty rawContent) so on-disk decimal directories are still counted. + for (const n of scanExistingDecimalPhaseNumbers(phasesDir, '', normalized)) { + decimalSet.add(n); + } + } + } else { + for (const n of scanExistingDecimalPhaseNumbers(phasesDir, '', normalized)) { + decimalSet.add(n); } } @@ -1545,7 +1545,79 @@ function cmdPhaseAddBatch(cwd: string, descriptions: string[], raw: boolean): vo publishStateContract(cwd); } -function cmdPhaseInsert(cwd: string, afterPhase: string, description: string, raw: boolean): void { +// #4569: scans all three representations of an existing decimal sub-phase +// under `base` โ€” on-disk `phases/` directories, `### Phase BASE.N:` headings, +// and `- [ ] Phase BASE.N:` roadmap SUMMARY CHECKLIST bullets. A bullet-only +// roadmap with no heading yet and no on-disk directory yet must still be +// seen, or an allocator can silently reallocate an already-used decimal +// number. Shared by `cmdPhaseInsert`'s normalized-base scan and its +// sibling-allocation parent-base scan so the two never drift apart. +function scanExistingDecimalPhaseNumbers(phasesDir: string, rawContent: string, base: string): Set { + const decimalSet = new Set(); + + // #2245 audit: existsSync-guarded, mirroring cmdPhaseNextDecimal's identical + // scan above โ€” a missing phasesDir (no decimal sub-phases yet) is the + // expected, silent case (empty decimalSet). A readdirSync failure once the + // dir is confirmed to EXIST is a genuine anomaly; swallowing it used to let + // `phase insert` proceed with an incomplete decimalSet and risk writing a + // decimal phase number that collides with an existing on-disk directory + // the scan simply never saw โ€” surfaced loud instead, like the sibling. + // + // #4634 (lint-phase-enumeration-drift): routed through the canonical + // PHYSICAL-set owner (`listAllPhaseDirs`, phase-locator.cts) instead of a + // hand-rolled `readdirSync`. This scan โ€” like its sibling `cmdPhaseNextDecimal` + // and its caller `cmdPhaseInsert` (both exempted in the drift guard for the + // same reason) โ€” must see EVERY on-disk decimal sub-phase directory + // regardless of the current milestone window, so `listMilestonePhaseDirs` + // (windowed) is the wrong owner here; `includeSentinels: true` preserves this + // function's pre-existing behavior of never sentinel-filtering (the decimal + // regex below only ever matches `base.N`-shaped names, so sentinel inclusion + // is a no-op either way). + if (fs.existsSync(phasesDir)) { + const { value: dirs, scope } = listAllPhaseDirs(phasesDir, { includeSentinels: true }); + if (scope === SCOPE.UNREADABLE) { + // The dir EXISTS but could not be read (EACCES/EIO) โ€” a genuine anomaly, + // not the expected empty-decimalSet case above. Surfaced loud, matching + // this function's pre-migration `readdirSync` catch: swallowing it would + // let `phase insert` proceed with an incomplete decimalSet and collide + // with an existing on-disk decimal directory the scan never saw. + error(`Failed to scan phase directories for existing decimal phases: unable to read ${phasesDir}`); + } + const decimalPattern = new RegExp(`^${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}${escapeRegex(base)}\\.(\\d+)`); + for (const dir of dirs) { + const dm = dir.match(decimalPattern); + if (dm) decimalSet.add(parseInt(dm[1], 10)); + } + } + + const rmPhasePattern = new RegExp( + `#{2,4}\\s*Phase\\s+${phaseMarkdownRegexSource(base)}\\.(\\d+)${OPTIONAL_PHASE_TAG_SOURCE}\\s*:`, + 'gi', + ); + let rmMatch: RegExpExecArray | null; + while ((rmMatch = rmPhasePattern.exec(rawContent)) !== null) { + decimalSet.add(parseInt(rmMatch[1], 10)); + } + + const checklistDecimalPattern = new RegExp( + `-\\s*\\[[ x]\\]\\s*(?:\\*\\*)?Phase\\s+${phaseMarkdownRegexSource(base)}\\.(\\d+)${OPTIONAL_PHASE_TAG_SOURCE}[:\\s]`, + 'gi', + ); + let clMatch: RegExpExecArray | null; + while ((clMatch = checklistDecimalPattern.exec(rawContent)) !== null) { + decimalSet.add(parseInt(clMatch[1], 10)); + } + + return decimalSet; +} + +function cmdPhaseInsert( + cwd: string, + afterPhase: string, + description: string, + raw: boolean, + allocation: 'nested' | 'sibling' = 'nested', +): void { if (!afterPhase || !description) { error('after-phase and description required for phase insert'); } @@ -1590,49 +1662,22 @@ function cmdPhaseInsert(cwd: string, afterPhase: string, description: string, ra const phasesDir = path.join(planningDir(cwd), 'phases'); const normalizedBase = normalizePhaseName(afterPhase); - const decimalSet = new Set(); - - // #2245 audit: existsSync-guarded, mirroring cmdPhaseNextDecimal's identical - // scan above โ€” a missing phasesDir (no decimal sub-phases yet) is the - // expected, silent case (empty decimalSet). A readdirSync failure once the - // dir is confirmed to EXIST is a genuine anomaly; swallowing it used to let - // `phase insert` proceed with an incomplete decimalSet and risk writing a - // decimal phase number that collides with an existing on-disk directory - // the scan simply never saw โ€” surfaced loud instead, like the sibling. - if (fs.existsSync(phasesDir)) { - // Initialized (not just declared) so TS's definite-assignment check is - // satisfied without relying on control-flow narrowing through error()'s - // `never` return, which TS does not propagate through a destructured - // module-property function reference โ€” error() still halts the process - // before `dirs` below is ever computed from this placeholder value. - let entries: fs.Dirent[] = []; - try { - entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - error(`Failed to scan phase directories for existing decimal phases: ${msg}`); - } - const dirs = entries.filter((e) => e.isDirectory()).map((e) => e.name); - const decimalPattern = new RegExp( - `^${OPTIONAL_PROJECT_CODE_PREFIX_SOURCE}${escapeRegex(normalizedBase)}\\.(\\d+)`, - ); - for (const dir of dirs) { - const dm = dir.match(decimalPattern); - if (dm) decimalSet.add(parseInt(dm[1], 10)); - } - } - - const rmPhasePattern = new RegExp( - `#{2,4}\\s*Phase\\s+${phaseMarkdownRegexSource(normalizedBase)}\\.(\\d+)${OPTIONAL_PHASE_TAG_SOURCE}\\s*:`, - 'gi', - ); - let rmMatch: RegExpExecArray | null; - while ((rmMatch = rmPhasePattern.exec(rawContent)) !== null) { - decimalSet.add(parseInt(rmMatch[1], 10)); - } + const decimalSet = scanExistingDecimalPhaseNumbers(phasesDir, rawContent, normalizedBase); const nextDecimal = decimalSet.size === 0 ? 1 : Math.max(...decimalSet) + 1; - const _decimalPhase = `${normalizedBase}.${nextDecimal}`; + let _decimalPhase = `${normalizedBase}.${nextDecimal}`; + + // #4569: sibling allocation joins afterPhase's PARENT level instead of nesting + // one level deeper under afterPhase itself. A top-level phase (no existing + // decimal segment) has no sibling level to join; nested is the only sensible + // allocation, so we silently fall back for that case. + const lastDotIndex = normalizedBase.lastIndexOf('.'); + if (allocation === 'sibling' && lastDotIndex !== -1) { + const parentBase = normalizedBase.slice(0, lastDotIndex); + const siblingDecimalSet = scanExistingDecimalPhaseNumbers(phasesDir, rawContent, parentBase); + const siblingNextDecimal = siblingDecimalSet.size === 0 ? 1 : Math.max(...siblingDecimalSet) + 1; + _decimalPhase = `${parentBase}.${siblingNextDecimal}`; + } const insertConfig = loadConfig(cwd); const projectCode = (insertConfig.project_code as string) || ''; const pfx = projectCode ? `${projectCode}-` : ''; diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 899ffb006..77d7d3268 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -1580,6 +1580,24 @@ function stripLeadingDelimiter(s: string): string { return s.replace(/^[\sโ€”โ€“:-]+/, '').trim(); } +/** + * #4134/#4433 (ยง7.2 rule 6 floor, applied symmetrically): a captured "name" + * with no letter or digit anywhere is heading/bullet STRUCTURE, not a curated + * name โ€” e.g. the trailing `)` a name-then-version heading leaves after its + * version token, or a malformed ๐Ÿšง-bullet whose only content past the version + * is punctuation (`---`, `***`, a lone `:`). #4134 fixed this for + * `extractMilestoneHeadingName`'s heading path only; #4433 found the sibling + * ๐Ÿšง-bullet capture (`getMilestoneInfo`'s `listMatch`) and the no-STATE.md + * fallback (`inProgressMatch`) both skipped straight to a bare truthiness + * check, so a punctuation-only bullet name passed through as a real one. This + * is now the SOLE name-validity predicate โ€” every capture site in this file + * calls it instead of re-deriving the character class. A name that merely + * CONTAINS punctuation is unaffected; digits alone qualify. + */ +function hasNameableContent(s: string): boolean { + return /[\p{L}\p{N}]/u.test(s); +} + /** * #3216 (ADR-3180 ยง7.2's "Name extraction โ€” pinned rule"): the sole "milestone * heading text โ†’ version + curated name" rule. Strips everything through the @@ -1648,7 +1666,7 @@ function extractMilestoneHeadingName( // scope TRUNCATED) for an unresolvable name. A name that merely CONTAINS // punctuation is untouched โ€” `(` is an ordinary name character (#3171) โ€” // and digits alone qualify (`## v4.0 โ€” 42` is the name `42`). - const name = candidate !== null && /[\p{L}\p{N}]/u.test(candidate) ? candidate : null; + const name = candidate !== null && hasNameableContent(candidate) ? candidate : null; return { version, name }; } @@ -1737,7 +1755,7 @@ function getMilestoneInfo(cwd?: string): { value: MilestoneInfo | null; scope: S ); if (listMatch) { const name = stripLeadingDelimiter(listMatch[1]); - if (name) return scoped({ version: stateVersion, name }, SCOPE.COMPLETE); + if (name && hasNameableContent(name)) return scoped({ version: stateVersion, name }, SCOPE.COMPLETE); } // #3216: heading selection routes through the shared owner @@ -1773,10 +1791,13 @@ function getMilestoneInfo(cwd?: string): { value: MilestoneInfo | null; scope: S // (unchanged from the pre-#3216 fallback). const inProgressMatch = roadmap.match(/๐Ÿšง\s*\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/); if (inProgressMatch) { - return scoped( - { version: 'v' + inProgressMatch[1], name: inProgressMatch[2].trim() }, - SCOPE.COMPLETE, - ); + const inProgressName = inProgressMatch[2].trim(); + if (hasNameableContent(inProgressName)) { + return scoped( + { version: 'v' + inProgressMatch[1], name: inProgressName }, + SCOPE.COMPLETE, + ); + } } // #3216: enumerate every OPEN (non-shipped) milestone heading via the diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 1deaa6605..5078f8200 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -1688,6 +1688,44 @@ describe('commit command', () => { assert.ok(committedFile.includes('# Context'), 'phase commit must land on the phase branch'); }); + // #4126: an undeliverable phase_slug (a bare phase directory with no + // slug remainder) must never produce a branch name ending in the literal + // word "phase" โ€” that reads as a real (but wrong) slug rather than + // honestly reflecting that no slug was derivable. Routed through the + // shared renderPhaseBranchName owner (src/phase-id.cts) so this and + // init.cts's cmdInitExecutePhase can never diverge on the fallback. + test('#4126: an undeliverable phase_slug drops the token instead of substituting the literal "phase"', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ + commit_docs: true, + branching_strategy: 'phase', + phase_branch_template: 'gsd/phase-{phase}-{slug}', + }) + ); + // Bare phase directory โ€” no slug remainder after the phase token, so + // phase-locator resolves phase_slug: null for it. + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# Roadmap\n\n## Phase 1: Setup\nGoal: Initial setup\n' + ); + fs.writeFileSync(path.join(tmpDir, '.planning', 'phases', '01', '01-CONTEXT.md'), '# Context\n'); + + const result = runGsdTools( + 'commit "docs(01): add context" --files .planning/phases/01/01-CONTEXT.md', + tmpDir + ); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.committed, true, 'should have committed'); + + const branch = gitOrThrow(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: tmpDir }).trim(); + assert.ok(!branch.endsWith('-phase'), `branch name must not end in the literal "-phase": ${branch}`); + assert.strictEqual(branch, 'gsd/phase-01', 'expected the {slug} token to be dropped cleanly'); + }); + test('decimal phase numbers are captured correctly in branching strategy', () => { // Configure phase branching strategy fs.writeFileSync( diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 6c8fb5c54..15f9064f6 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -1283,6 +1283,37 @@ describe('init plan-phase zero-padded phase number (bug #2391)', () => { assert.strictEqual(output.branch_name, 'gsd/phase-01-foundation', 'branch_name must use normalized phase number (strip project_code prefix, zero-pad), not raw phase_number'); }); + + // #4126: an undeliverable phase_slug (a bare phase directory with no slug + // remainder, e.g. seeded here as literally "01") must never produce a + // branch name ending in the literal word "phase" โ€” routed through the + // shared renderPhaseBranchName owner (src/phase-id.cts) so this and + // commands.cts's cmdCommit phase-branching arm can never diverge on the + // fallback. + test('#4126: an undeliverable phase_slug drops the token instead of substituting the literal "phase"', () => { + seedPhase(tmpDir, '01', { + '01-01-PLAN.md': '# Plan', + }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ + git: { + branching_strategy: 'phase', + phase_branch_template: 'gsd/phase-{phase}-{slug}', + }, + }, null, 2) + ); + + const result = runGsdTools('init execute-phase 1', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_slug, null, 'precondition: phase_slug must be undeliverable'); + assert.ok(!output.branch_name.endsWith('-phase'), + `branch_name must not end in the literal "-phase": ${output.branch_name}`); + assert.strictEqual(output.branch_name, 'gsd/phase-01', + 'expected the {slug} token to be dropped cleanly'); + }); }); // โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ diff --git a/tests/lint-phase-id-drift.test.cjs b/tests/lint-phase-id-drift.test.cjs new file mode 100644 index 000000000..39130d3f0 --- /dev/null +++ b/tests/lint-phase-id-drift.test.cjs @@ -0,0 +1,88 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { + findNameValidityDrift, + findBranchSlugFallbackDrift, + findShellPhaseArithDrift, +} = require('../scripts/lint-phase-id-drift.cjs'); + +test('findNameValidityDrift flags a regex-literal re-derivation of the name-validity class', () => { + const text = [ + 'function isNameable(s) {', + ' return /[\\p{L}\\p{N}]/u.test(s);', + '}', + ].join('\n'); + const found = findNameValidityDrift(text); + assert.equal(found.length, 1); + assert.equal(found[0].line, 2); +}); + +test('findNameValidityDrift flags the doubled-backslash template-string form', () => { + const text = [ + 'const re = new RegExp(\'[\\\\p{L}\\\\p{N}]\', "u");', + ].join('\n'); + const found = findNameValidityDrift(text); + assert.equal(found.length, 1); +}); + +test('findNameValidityDrift does NOT flag a line that calls hasNameableContent(', () => { + const text = [ + 'function wrapper(s) {', + ' return hasNameableContent(s);', + '}', + ].join('\n'); + assert.deepEqual(findNameValidityDrift(text), []); +}); + +test('findNameValidityDrift does NOT flag a sanctioned site', () => { + const text = [ + '// phase-id-owner: deliberate local copy for perf, tracked in #4634', + 'const re = /[\\p{L}\\p{N}]/u;', + ].join('\n'); + assert.deepEqual(findNameValidityDrift(text), []); +}); + +test('findBranchSlugFallbackDrift flags the commands.cts/init.cts {slug}-fallback shape', () => { + const text = + " .replace('{slug}', (phaseInfo['phase_slug'] as string) || 'phase');"; + const found = findBranchSlugFallbackDrift(text); + assert.equal(found.length, 1); + assert.equal(found[0].line, 1); +}); + +test('findBranchSlugFallbackDrift does NOT flag the milestone-branch || \'milestone\' fallback', () => { + const text = + " .replace('{slug}', generateSlugInternal(milestone.name) || 'milestone');"; + assert.deepEqual(findBranchSlugFallbackDrift(text), []); +}); + +test('findBranchSlugFallbackDrift does NOT flag a sanctioned site', () => { + const text = [ + '// phase-id-owner: deliberate, tracked in #4634', + " .replace('{slug}', (phaseInfo['phase_slug'] as string) || 'phase');", + ].join('\n'); + assert.deepEqual(findBranchSlugFallbackDrift(text), []); +}); + +test('findShellPhaseArithDrift flags $((10#...)) base-10-forced arithmetic', () => { + const text = 'PHASE_N=$((10#$PHASE_NUM))'; + const found = findShellPhaseArithDrift(text); + assert.equal(found.length, 1); + assert.equal(found[0].line, 1); +}); + +test('findShellPhaseArithDrift does NOT flag ordinary arithmetic', () => { + const text = 'NEXT=$((i+1))'; + assert.deepEqual(findShellPhaseArithDrift(text), []); +}); + +test('findShellPhaseArithDrift does NOT flag a site sanctioned with an HTML comment', () => { + const text = [ + '', + 'PHASE_N=$((10#$PHASE_NUM))', + ].join('\n'); + assert.deepEqual(findShellPhaseArithDrift(text), []); +}); diff --git a/tests/phase-id-drift-guard.test.cjs b/tests/phase-id-drift-guard.test.cjs index 6df0196ce..a493a7562 100644 --- a/tests/phase-id-drift-guard.test.cjs +++ b/tests/phase-id-drift-guard.test.cjs @@ -24,7 +24,7 @@ const fs = require('node:fs'); const path = require('node:path'); const ROOT = path.join(__dirname, '..'); -const { findPhaseIdRegexDrift, findBracketGrammarDrift, scanRepo } = require( +const { findPhaseIdRegexDrift, findBracketGrammarDrift, scanRepo, scanMarkdownShellArith } = require( path.join(ROOT, 'scripts', 'lint-phase-id-drift.cjs'), ); const phaseId = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'phase-id.cjs')); @@ -264,7 +264,7 @@ describe('#2761 M3 bracket grammar: one owner, byte-identical to the sites it re }); describe('#2128 phase-id drift scanner: the live repo is clean', () => { - test('scanRepo finds zero unsanctioned re-derivations (token AND bracket)', () => { + test('scanRepo finds zero unsanctioned re-derivations (token, bracket, name-validity, and branch-slug-fallback)', () => { const violations = scanRepo(ROOT); assert.deepEqual( violations, @@ -296,6 +296,90 @@ describe('#2128 phase-id drift scanner: the live repo is clean', () => { cleanup(tmp); } }); + + test('scanRepo actually runs the name-validity rule (coverage, not just a clean result)', () => { + // Same proof shape as the bracket-rule test above, for #4634's new + // name-validity detector: plant a literal re-derivation of + // hasNameableContent's character class in a temp tree and require the + // real scanRepo() to catch it end-to-end. + const os = require('node:os'); + const { cleanup } = require('./helpers.cjs'); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'phase-id-drift-')); + try { + fs.mkdirSync(path.join(tmp, 'src')); + fs.writeFileSync( + path.join(tmp, 'src', 'planted.cts'), + 'function fakeCheck(s) {\n return /[\\p{L}\\p{N}]/u.test(s);\n}\n', + ); + const found = scanRepo(tmp); + assert.equal(found.length, 1, 'scanRepo must report the planted name-validity literal'); + assert.equal(found[0].kind, 'name-validity'); + assert.equal(found[0].file, path.join('src', 'planted.cts')); + } finally { + cleanup(tmp); + } + }); + + test('scanRepo actually runs the branch-slug-fallback rule (coverage, not just a clean result)', () => { + // Same proof shape again, for #4634's branch-slug-fallback detector: plant + // the shipped commands.cts/init.cts shape into a temp tree and require the + // real scanRepo() to catch it end-to-end. + const os = require('node:os'); + const { cleanup } = require('./helpers.cjs'); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'phase-id-drift-')); + try { + fs.mkdirSync(path.join(tmp, 'src')); + fs.writeFileSync( + path.join(tmp, 'src', 'planted.cts'), + "const x = template.replace('{slug}', (phaseInfo['phase_slug'] as string) || 'phase');\n", + ); + const found = scanRepo(tmp); + assert.equal(found.length, 1, 'scanRepo must report the planted branch-slug-fallback literal'); + assert.equal(found[0].kind, 'branch-slug-fallback'); + assert.equal(found[0].file, path.join('src', 'planted.cts')); + } finally { + cleanup(tmp); + } + }); + + test('scanMarkdownShellArith actually runs the shell-arith rule (coverage, not just a clean result)', () => { + // Same proof shape again, for #4634's markdown shell-arithmetic detector: + // plant a `$((10#$VAR))` site under the real scan roots + // (gsd-core/workflows/**/*.md) and require scanMarkdownShellArith() to + // catch it end-to-end, over the real MD_SCAN_DIRS walk. + const os = require('node:os'); + const { cleanup } = require('./helpers.cjs'); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'phase-id-drift-')); + try { + const workflowsDir = path.join(tmp, 'gsd-core', 'workflows'); + fs.mkdirSync(workflowsDir, { recursive: true }); + fs.writeFileSync( + path.join(workflowsDir, 'planted.md'), + '```bash\nPHASE_N=$((10#$PHASE_NUM))\n```\n', + ); + const found = scanMarkdownShellArith(tmp); + assert.equal(found.length, 1, 'scanMarkdownShellArith must report the planted shell-arith literal'); + assert.equal(found[0].kind, 'shell-arith'); + assert.equal(found[0].file, path.join('gsd-core', 'workflows', 'planted.md')); + } finally { + cleanup(tmp); + } + }); + + test( + "#4619 shell-arith violations are known and tracked separately (characterization, not this PR's scope)", + () => { + const violations = scanMarkdownShellArith(ROOT); + assert.equal( + violations.length, + 7, + 'expected exactly the 7 known #4619 sites (workflows/execute-phase.md x4, ' + + 'workflows/execute-phase/steps/completion-reconciliation.md x2, references/tdd.md x1) โ€” ' + + 'if this count changed, either #4619 was fixed (great โ€” update/remove this pin) or a NEW ' + + 'unrelated shell-arith site was introduced (investigate before adjusting the number)', + ); + }, + ); }); describe('#2128 phase-id single-owner identity guard', () => { diff --git a/tests/phase-id.test.cjs b/tests/phase-id.test.cjs index 8eaea334d..d84d35fdd 100644 --- a/tests/phase-id.test.cjs +++ b/tests/phase-id.test.cjs @@ -1507,3 +1507,78 @@ describe('phaseKeyFrom* โ€” one key space for directories and prose', () => { ); }); }); + +// โ”€โ”€โ”€ renderPhaseBranchName (#4126) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// +// The ONE branch-name-template renderer shared by cmdCommit (src/commands.cts) +// and cmdInitExecutePhase (src/init.cts), so the two consumers cannot diverge +// on how an undeliverable phase_slug degrades. +describe('renderPhaseBranchName โ€” shared branch-name template renderer', () => { + const DEFAULT_TEMPLATE = 'gsd/phase-{phase}-{slug}'; + + test('a real slug substitutes normally, unchanged from prior behavior', () => { + assert.strictEqual( + phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', 'my-feature'), + 'gsd/phase-08-my-feature', + ); + }); + + test('an empty slug drops the {slug} token cleanly for the default template', () => { + assert.strictEqual(phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', ''), 'gsd/phase-08'); + assert.strictEqual(phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', null), 'gsd/phase-08'); + assert.strictEqual(phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', undefined), 'gsd/phase-08'); + }); + + test('the literal word "phase" is never substituted for an undeliverable slug', () => { + const result = phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', ''); + assert.ok(!result.endsWith('-phase'), result); + }); + + test('an empty slug at the START of a template drops the token and its trailing separator', () => { + assert.strictEqual(phaseId.renderPhaseBranchName('{slug}-gsd-{phase}', '8', ''), 'gsd-08'); + }); + + test('an empty slug at the END of a template drops the token and its leading separator', () => { + assert.strictEqual(phaseId.renderPhaseBranchName('gsd-{phase}-{slug}', '8', ''), 'gsd-08'); + }); + + test('a doubled-separator template does not produce an invalid leading/trailing separator', () => { + assert.strictEqual(phaseId.renderPhaseBranchName('feature//{slug}', '8', ''), 'feature'); + assert.strictEqual(phaseId.renderPhaseBranchName('feature//{slug}', '8', 'x'), 'feature//x'); + }); + + test('non-string phaseSlug values (digits, objects) degrade like an empty slug', () => { + assert.strictEqual(phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', 42), 'gsd/phase-08'); + assert.strictEqual(phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', {}), 'gsd/phase-08'); + assert.strictEqual(phaseId.renderPhaseBranchName(DEFAULT_TEMPLATE, '8', []), 'gsd/phase-08'); + }); + + test('a template with no {slug} token at all is returned unchanged, just phase-substituted', () => { + assert.strictEqual(phaseId.renderPhaseBranchName('gsd/phase-{phase}', '8', ''), 'gsd/phase-08'); + assert.strictEqual(phaseId.renderPhaseBranchName('gsd/phase-{phase}', '8', 'x'), 'gsd/phase-08'); + }); + + test('a template that is only {slug} degrades to null rather than an empty branch name', () => { + assert.strictEqual(phaseId.renderPhaseBranchName('{slug}', '8', ''), null); + }); + + test('property: an undeliverable slug never leaves {slug} or a leading/trailing separator behind', () => { + // Realistic slugs (as generateSlugInternal produces them: lowercase + // alnum/dash, never separator-edged) are covered by the "real slug" + // tests above; this property targets the DEGRADE path (#4126's actual + // invariant), so the slug domain here is exactly the undeliverable one. + fc.assert( + fc.property( + fc.constantFrom('gsd/phase-{phase}-{slug}', '{slug}-gsd-{phase}', 'gsd-{phase}-{slug}', 'feature//{slug}', '{phase}.{slug}'), + fc.integer({ min: 0, max: 999 }), + fc.constantFrom('', null, undefined, 0, 42, {}, []), + (template, phaseNumber, phaseSlug) => { + const result = phaseId.renderPhaseBranchName(template, phaseNumber, phaseSlug); + if (result === null) return true; + if (result.includes('{slug}')) return false; + return !/^[-_./]/.test(result) && !/[-_./]$/.test(result); + }, + ), + ); + }); +}); diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 5eb1fa273..5267158e5 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -527,6 +527,30 @@ describe('phase next-decimal command', () => { assert.strictEqual(output.found, false, 'base phase not found'); assert.strictEqual(output.next, '06.1', 'should still suggest 06.1'); }); + + // #4569: cmdPhaseNextDecimal migrated to the shared scanExistingDecimalPhaseNumbers + // helper (also consumed by cmdPhaseInsert), which counts a checklist-only decimal + // bullet even when no heading and no on-disk directory exist for it yet. + test('#4569: sees a checklist-only decimal with no heading and no on-disk directory', () => { + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '03-something'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 3: Something +**Goal:** Setup + +- [ ] Phase 3.1: Something +` + ); + + const result = runGsdTools('phase next-decimal 3', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.next, '03.2', 'checklist-only 3.1 must be counted, not just headings/dirs'); + assert.deepStrictEqual(output.existing, ['03.1'], 'checklist-only decimal listed as existing'); + }); }); // โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ @@ -3212,6 +3236,119 @@ describe('phase insert command', () => { const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8'); assert.ok(roadmap.includes('Phase 05.1: Hotfix (INSERTED)'), 'roadmap should include inserted phase'); }); + + // #4569: cmdPhaseInsert's decimal allocation must count an existing decimal + // regardless of WHICH of the three representations (on-disk directory, + // `### Phase N.M:` heading, `- [ ] Phase N.M:` checklist bullet) carries it โ€” + // via the shared scanExistingDecimalPhaseNumbers helper. + test('#4569 core regression: does not reallocate a decimal that exists only as a roadmap checklist bullet', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 3: Something +**Goal:** Setup + +- [ ] Phase 3.1: Something +` + ); + + const result = runGsdTools('phase insert 3 New Thing', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_number, '03.2', 'checklist-only 3.1 must not be reallocated as 03.1'); + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'phases', '03.2-new-thing')), + 'new decimal phase directory should be 03.2, not a collision with the checklist-only 03.1' + ); + }); + + test('#4569 independence check: a decimal present in heading, checklist, and on-disk directory simultaneously is counted once', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 3: Something +**Goal:** Setup + +### Phase 3.1: Existing Decimal +**Goal:** Test + +- [ ] Phase 3.1: Existing Decimal +` + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '03.1-existing-decimal'), { recursive: true }); + + const result = runGsdTools('phase insert 3 New Thing', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual( + output.phase_number, + '03.2', + 'triple-represented 03.1 must count once, not skip ahead or collide' + ); + }); + + test('#4569 negative-space: a checklist bullet for an unrelated phase family does not pollute this phase\'s decimal allocation', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 3: Something +**Goal:** Setup + +### Phase 13: Unrelated +**Goal:** Unrelated + +- [ ] Phase 13.2: Unrelated Decimal +` + ); + + const result = runGsdTools('phase insert 3 New Thing', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual( + output.phase_number, + '03.1', + 'phase 13\'s checklist decimal must not cross-pollute phase 3\'s allocation' + ); + }); + + test('#4569: --sibling flag inserts a sibling of a decimal phase via the real CLI', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 3: Something +**Goal:** Setup + +### Phase 3.2: Existing Decimal +**Goal:** Test +` + ); + const result = runGsdTools('phase insert 3.2 New Thing --sibling', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_number, '03.3', 'sibling allocation must join phase 3\'s level, not nest under 3.2'); + }); + + test('#4569: --sibling flag falls back to nested allocation when afterPhase has no decimal segment', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +### Phase 3: Something +**Goal:** Setup +` + ); + const result = runGsdTools('phase insert 3 New --sibling', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_number, '03.1', '--sibling on a top-level phase must fall back to nested allocation'); + }); }); // โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index b7a638304..0750df01b 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -1111,6 +1111,78 @@ describe('roadmap-parser: getMilestoneInfo #4134 โ€” name-then-version heading', }); }); +// โ”€โ”€โ”€ getMilestoneInfo โ€” #4433 name-validity guard on bullet captures โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// `hasNameableContent` (the #4134 name-validity predicate) is not exported +// from roadmap-parser.cjs; these tests exercise it indirectly through +// getMilestoneInfo's two bullet-capture sites (the STATE-anchored ๐Ÿšง bullet +// and the no-STATE.md in-progress ๐Ÿšง bullet), which #4433 found had skipped +// straight to a bare truthiness check. + +describe('roadmap-parser: getMilestoneInfo #4433 โ€” name-validity guard on bullet captures', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('#4433 โ€” punctuation-only ๐Ÿšง-bullet name (STATE.md version known) falls through to TRUNCATED, not the bullet fragment', () => { + writeState(tmpDir, { milestone: 'v3.3' }); + writeRoadmap(tmpDir, [ + '๐Ÿšง **v3.3** !!!', + '', + '## v3.4: Something Else', + '### Phase 1: Setup', + ].join('\n')); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.TRUNCATED, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v3.3'); + assert.notStrictEqual(info.value.name, '!!!'); + assert.strictEqual(info.value.name, null); + }); + + test('#4433 โ€” real, non-punctuation ๐Ÿšง-bullet name (STATE.md version known) still resolves COMPLETE (regression control)', () => { + writeState(tmpDir, { milestone: 'v3.3' }); + writeRoadmap(tmpDir, [ + '๐Ÿšง **v3.3** Real Feature Name', + '', + '## v3.4: Something Else', + '### Phase 1: Setup', + ].join('\n')); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v3.3'); + assert.match(info.value.name, /Real Feature Name/); + }); + + test('#4433 โ€” punctuation-only in-progress ๐Ÿšง-bullet (no STATE.md) is not returned as COMPLETE with the punctuation as name', () => { + writeRoadmap(tmpDir, '๐Ÿšง **v3.3 !!!**\n### Phase 1: Setup\n'); + const info = getMilestoneInfo(tmpDir); + assert.notStrictEqual(info.value.name, '!!!'); + if (info.scope === SCOPE.COMPLETE) { + assert.fail(`punctuation-only in-progress bullet name leaked through as COMPLETE: ${JSON.stringify(info)}`); + } + assert.strictEqual(info.scope, SCOPE.TRUNCATED, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v3.3'); + assert.strictEqual(info.value.name, null); + }); + + test('#4433 โ€” real in-progress ๐Ÿšง-bullet name (no STATE.md) still resolves COMPLETE (regression control)', () => { + writeRoadmap(tmpDir, '๐Ÿšง **v3.3 Some Real Name**\n### Phase 1: Setup\n'); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v3.3'); + assert.match(info.value.name, /Some Real Name/); + }); + + test('#4433 โ€” digits-only captured name is accepted (boundary: digits alone qualify)', () => { + writeState(tmpDir, { milestone: 'v4.0' }); + writeRoadmap(tmpDir, '## v4.0 โ€” 42\n### Phase 1: Setup\n'); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v4.0'); + assert.strictEqual(info.value.name, '42'); + }); +}); + // โ”€โ”€โ”€ isMilestoneShippedInRoadmap โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ // #2562: this module owns milestone-heading classification, so its own shipped