diff --git a/.changeset/2572-verify-summary-phase-summaries.md b/.changeset/2572-verify-summary-phase-summaries.md new file mode 100644 index 000000000..5f893cbc1 --- /dev/null +++ b/.changeset/2572-verify-summary-phase-summaries.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2685 +--- +**Completing a phase now warns when its SUMMARY claims files that never landed** — `phase complete` runs the artifact check that `verify-summary` has always applied to the research SUMMARY against the completing phase's own `SUMMARY.md` files, and reports any referenced path that is not on disk through its existing `warnings[]` channel. Previously the check was wired to exactly two call sites, both pointed at `.planning/research/SUMMARY.md`, so the summaries that actually assert "I created these files" were never verified and an interrupted phase counted toward 100% silently. Advisory only: it never blocks completion. Paths are recovered heuristically from the SUMMARY body, so globs, URLs, bare hostnames, and paths resolving outside the project are skipped rather than reported; the `key-files:` frontmatter block and commit hashes are deliberately not read. (#2572) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 8e3205729..38167ab10 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -127,6 +127,8 @@ node gsd-tools.cjs phase insert node gsd-tools.cjs phase remove [--force] # Mark phase complete, update state + roadmap +# Also emits advisory `warnings[]` when a phase SUMMARY references a file that +# is not on disk — see "Phase SUMMARY artifact check" below. node gsd-tools.cjs phase complete # Evaluate HUMAN-UAT results for a phase (markdown-aware; ignores false-positive contexts) @@ -140,6 +142,31 @@ node gsd-tools.cjs phase-plan-index node gsd-tools.cjs phases list [--type planned|executed|all] [--phase N] [--include-archived] ``` +### Phase SUMMARY artifact check + +A phase `SUMMARY.md` asserts which files the phase created or modified. On +`phase complete`, each SUMMARY in the phase is scanned for referenced file paths +and any path that is not on disk is reported in the command's existing +`warnings[]` array — the case where a summary reports work that never landed. + +**Advisory only.** Findings never block completion; the completion gate is the +phase's `VERIFICATION.md` status, which this does not touch. `/gsd-execute-phase` +surfaces the warnings before advancing. + +Scope and limits, so the output is not read as more than it is: + +- Paths are recovered heuristically from the SUMMARY body — backticked paths and + `Created:`/`Modified:`-style lines. Globs, URLs, bare hostnames, and paths + resolving outside the project are skipped rather than reported. +- The `key-files:` frontmatter block is **not** read. Its YAML flow-sequence form + (`created: [a.ts, b.ts]`) is not matched by the prose scan, so a summary whose + only file claims live there produces no findings. +- Commit hashes in the SUMMARY are **not** resolved here. The pattern matches any + hex-shaped token in prose, which is too loose to surface. + +Every path the scan does recover is checked — there is no cap. The standalone +`verify-summary` verb keeps its historical default of checking the first two. + --- ## Roadmap Commands diff --git a/src/phase.cts b/src/phase.cts index e38cb6b78..5089447f3 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -62,6 +62,11 @@ import uatPredicate = require('./uat-predicate.cjs'); const { evaluateUatPassed } = uatPredicate; // eslint-disable-next-line @typescript-eslint/no-require-imports -- verification.cjs is an export= CommonJS module import verificationMod = require('./verification.cjs'); +// #2572: the artifact↔disk core behind the `verify-summary` verb. `verify.cts` +// has no transitive import path back to `phase.cts`, so this edge introduces no +// cycle (the reverse edge, `state.cts → verify.cjs`, would). +// eslint-disable-next-line @typescript-eslint/no-require-imports -- verify.cjs is an export= CommonJS module +import verifyMod = require('./verify.cjs'); const { readVerificationStatus } = verificationMod; const { planningDir, withPlanningLock, listAvailableWorkstreams, getActiveWorkstream } = @@ -1757,6 +1762,50 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { * warnings are surfaced this run, not a blocked or corrupted completion. */ } + // #2572: artifact↔disk advisory for the SUMMARYs of the phase being completed. + // + // A SUMMARY asserts "I created these files". Nothing checked that claim for + // phase summaries — the `verify-summary` verb has existed since the beginning + // but was only ever pointed at `.planning/research/SUMMARY.md`. An interrupted + // or over-reported phase therefore counted toward 100% silently. + // + // Joins the same ADVISORY channel as the pre-scan above: findings land in + // `warnings[]` (rendered by execute-phase.md's "If has_warnings is true" + // step), never in the completion GATE (readVerificationStatus below). + // Completion is never blocked. + // + // `checkCommits: false` — only the file-existence half is surfaced here, so + // the `git cat-file` probes would be spawned and their result discarded. The + // hash pattern is a loose `\b[0-9a-f]{7,40}\b` that matches any hex-shaped + // token in prose, too noisy to put in front of a user even as a warning. + // + // `Infinity` — report every referenced file, not the CLI verb's default first + // two, so a phase that lists twelve files and landed three says so. The verb + // keeps its 2-file default; only this caller opts out of the cap. + try { + const phaseDirRel = phaseInfo['directory'] as string; + // `summaries` arrives pre-sorted from the phase locator, so warning order is + // deterministic across platforms rather than readdir-dependent. + const summaryNames = (phaseInfo['summaries'] as string[] | undefined) || []; + for (const summaryName of summaryNames) { + const v = verifyMod.verifySummaryCore( + cwd, + `${phaseDirRel}/${summaryName}`, + Infinity, + { checkCommits: false }, + ); + const missing = v.checks.files_created.missing; + if (missing.length > 0) { + warnings.push( + `${summaryName}: references ${missing.length} file(s) not on disk: ${missing.join(', ')}`, + ); + } + } + } catch { + /* best-effort, same posture as the #2245 pre-scan above: an unreadable + * SUMMARY means one fewer advisory this run, never a blocked completion. */ + } + let nextPhaseNum: string | null = null; let nextPhaseName: string | null = null; let isLastPhase = true; diff --git a/src/verify.cts b/src/verify.cts index a46aa35f8..66d6b402c 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -63,21 +63,52 @@ const { MODEL_PROFILES } = modelProfilesMod; void stripShippedMilestones; void detectSchemaFiles; -function cmdVerifySummary( +interface SummaryVerification { + passed: boolean; + checks: { + summary_exists: boolean; + files_created: { checked: number; found: number; missing: string[] }; + commits_exist: boolean; + self_check: string; + }; + errors: string[]; +} + +/** + * Pure core of `verify-summary` (#2572). + * + * Same artifact↔git checks the CLI verb has always run, lifted out of the + * `output()` wrapper so other verbs can consume the structured + * `{ passed, checks, errors }` contract directly instead of shelling out and + * re-parsing JSON. `cmdVerifySummary` is now a thin adapter over this. + * + * Never throws and never writes to stdout: a missing SUMMARY, a non-repo, or an + * unresolvable commit all come back as structured `false`/`missing` values. + * + * Caveat for callers surfacing `commits_exist`: the hash pattern is a loose + * `\b[0-9a-f]{7,40}\b`, so any hex-shaped token in the prose counts as a + * candidate. That is cheap as an advisory signal and unacceptable as a gate. + * + * @param checkFileCount How many extracted candidates to probe. Defaults to 2 — + * the value the CLI verb has always used. Pass `Infinity` to probe every + * candidate (see `cmdPhaseComplete`, which reports on all of them). + * @param opts.checkCommits When `false`, the `git cat-file` probes are skipped + * entirely and `commits_exist` comes back `false` meaning *not checked*. + * Callers that do not surface `commits_exist` should pass `false` so this + * stays a pure-filesystem check with no subprocess cost. + */ +function verifySummaryCore( cwd: string, summaryPath: string, - checkFileCount: number | undefined, - raw: boolean, -): void { - if (!summaryPath) { - error('summary-path required'); - } - + checkFileCount?: number, + opts?: { checkCommits?: boolean }, +): SummaryVerification { const fullPath = path.join(cwd, summaryPath); const checkCount = checkFileCount || 2; + const checkCommits = opts?.checkCommits !== false; if (!fs.existsSync(fullPath)) { - const result = { + return { passed: false, checks: { summary_exists: false, @@ -87,24 +118,74 @@ function cmdVerifySummary( }, errors: ['SUMMARY.md not found'], }; - output(result, raw, 'failed'); - return; } const content = fs.readFileSync(fullPath, 'utf-8'); const errors: string[] = []; + const projectRoot = path.resolve(cwd); + + /** + * Is `candidate` plausibly a repo-relative file this check should probe? + * + * Deliberately narrowing. This is an ADVISORY, so the two error directions are + * not symmetric: a false positive tells a user their healthy project is + * missing a file that was never claimed, while a false negative just means one + * reference goes unprobed. Every rejection below is a noise class confirmed on + * #2685; when in doubt, skip rather than warn. + */ + const isProbableProjectFile = (candidate: string): boolean => { + // Only repo-relative paths — a bare filename is too ambiguous to locate. + if (!candidate.includes('/')) return false; + // URLs, protocol-relative links, and any other scheme. + if (candidate.startsWith('http') || candidate.startsWith('//')) return false; + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(candidate)) return false; + // Globs name a set, not a file: `src/**/*.cts` is never "missing". + if (/[*?]/.test(candidate)) return false; + // Bare hostnames (`docs.example.com/guide.html`). A repo-relative path's + // first segment is a directory name, which in practice contains a dot only + // when it is a dotfile directory (`.github/`, `.changeset/`, `.planning/`) + // — i.e. the dot is at index 0. A dot anywhere later marks a hostname. + const firstSegment = candidate.split('/')[0] || ''; + if (firstSegment.indexOf('.') > 0) return false; + // Containment guard: a `../`-bearing reference must not turn this advisory + // into a filesystem existence probe outside the project. + const resolved = path.resolve(projectRoot, candidate); + if (resolved !== projectRoot && !resolved.startsWith(projectRoot + path.sep)) return false; + return true; + }; + + // Pattern 2 excludes `[` and `]` from its path class (#2685 Blocker 1). All + // three SUMMARY templates prescribe a YAML flow sequence for `key-files`: + // + // key-files: + // created: [src/auth/login.ts, src/auth/session.ts] + // + // and the label matches `(?:Created|Modified|…):` case-insensitively. Without + // the bracket exclusion the class captures the literal `[` as part of the + // first path, yielding `[src/auth/login.ts` — a candidate that can never exist + // on disk. That fired on healthy projects built from GSD's own shipped + // template. The exclusion also stops a markdown list in the body from + // reintroducing the same artifact. + // + // Stripping frontmatter first was the other remedy offered on #2685. It is a + // verified no-op on top of this exclusion — measured identical extraction + // across all three shipped templates — because the exclusion already makes a + // flow-sequence line contribute nothing. Consequence worth naming: the + // `key-files` block, the most authoritative statement of what a phase created, + // is still not read. Recovering it needs a real frontmatter parse, which is + // deliberately left as a follow-up rather than smuggled in here. const mentionedFiles = new Set(); const patterns = [ /`([^`]+\.[a-zA-Z]+)`/g, - /(?:Created|Modified|Added|Updated|Edited):\s*`?([^\s`]+\.[a-zA-Z]+)`?/gi, + /(?:Created|Modified|Added|Updated|Edited):\s*`?([^\s`[\]]+\.[a-zA-Z]+)`?/gi, ]; for (const pattern of patterns) { let m: RegExpExecArray | null; while ((m = pattern.exec(content)) !== null) { const filePath = m[1]; - if (filePath && !filePath.startsWith('http') && filePath.includes('/')) { + if (filePath && isProbableProjectFile(filePath)) { mentionedFiles.add(filePath); } } @@ -113,13 +194,13 @@ function cmdVerifySummary( const filesToCheck = Array.from(mentionedFiles).slice(0, checkCount); const missing: string[] = []; for (const file of filesToCheck) { - if (!fs.existsSync(path.join(cwd, file))) { + if (!fs.existsSync(path.resolve(projectRoot, file))) { missing.push(file); } } const commitHashPattern = /\b[0-9a-f]{7,40}\b/g; - const hashes = content.match(commitHashPattern) || []; + const hashes = checkCommits ? content.match(commitHashPattern) || [] : []; let commitsExist = false; if (hashes.length > 0) { for (const hash of hashes.slice(0, 3)) { @@ -157,8 +238,21 @@ function cmdVerifySummary( }; const passed = missing.length === 0 && selfCheck !== 'failed'; - const result = { passed, checks, errors }; - output(result, raw, passed ? 'passed' : 'failed'); + return { passed, checks, errors }; +} + +/** CLI adapter over verifySummaryCore — arg guard + output shaping only. */ +function cmdVerifySummary( + cwd: string, + summaryPath: string, + checkFileCount: number | undefined, + raw: boolean, +): void { + if (!summaryPath) { + error('summary-path required'); + } + const result = verifySummaryCore(cwd, summaryPath, checkFileCount); + output(result, raw, result.passed ? 'passed' : 'failed'); } /** @@ -2526,6 +2620,7 @@ export = { scanNegativeGrepCommentEcho, scanFileWideNegativeGateConflict, cmdVerifySummary, + verifySummaryCore, cmdVerifyPlanStructure, cmdVerifyPhaseCompleteness, cmdVerifyReferences, diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 185defae8..bbb81cb5b 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -9449,3 +9449,153 @@ describe('issue #2334: ghost-REQ-ID classification must probe write surfaces, no }, ); }); + +// ─── #2572: phase-SUMMARY artifact↔disk advisory at phase completion ───────── +// +// A SUMMARY asserts "I created these files". Until #2572 nothing checked that +// claim for phase summaries — `verify-summary` existed but was only ever +// pointed at `.planning/research/SUMMARY.md`, so an interrupted or +// over-reported phase counted toward 100% silently. +// +// The advisory joins the existing `warnings[]` channel of `phase complete` +// (rendered by execute-phase.md's "If has_warnings is true" step). It is +// advisory ONLY: the completion GATE is readVerificationStatus, untouched here. + +function build2572SummaryArtifactFixture() { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2572-artifact-')); + const planDir = path.join(tmpDir, '.planning'); + const dirtyDir = path.join(planDir, 'phases', '01-dirty'); + const cleanDir = path.join(planDir, 'phases', '02-clean'); + fs.mkdirSync(dirtyDir, { recursive: true }); + fs.mkdirSync(cleanDir, { recursive: true }); + fs.mkdirSync(path.join(tmpDir, 'src'), { recursive: true }); + + // Only these two land on disk. + fs.writeFileSync(path.join(tmpDir, 'src/landed-one.ts'), 'x\n'); + fs.writeFileSync(path.join(tmpDir, 'src/landed-two.ts'), 'x\n'); + + fs.writeFileSync(path.join(planDir, 'ROADMAP.md'), [ + '# Roadmap', '', + '- [ ] Phase 01: Dirty', + '- [ ] Phase 02: Clean', '', + '### Phase 01: Dirty', + '**Goal:** Ship the dirty thing', + '**Plans:** 1 plans', '', + '### Phase 02: Clean', + '**Goal:** Ship the clean thing', + '**Plans:** 1 plans', '', + '## Progress', '', + '| Phase | Plans Complete | Status | Completed |', + '|-------|----------------|--------|-----------|', + '| 01. Dirty | 0/1 | Not started | - |', + '| 02. Clean | 0/1 | Not started | - |', + '', + ].join('\n')); + + fs.writeFileSync(path.join(planDir, 'STATE.md'), [ + '# State', '', + '**Current Phase:** 01', + '**Current Phase Name:** Dirty', + '**Status:** In progress', + '**Completed Phases:** 0', + '**Total Phases:** 2', + '**Progress:** 0%', + '', + ].join('\n')); + + fs.writeFileSync(path.join(dirtyDir, '01-01-PLAN.md'), '# Plan\nDo the work.\n'); + // Frontmatter uses the shipped template's YAML flow sequence on purpose: the + // bracket artifact it once produced must not surface as a phantom warning. + fs.writeFileSync(path.join(dirtyDir, '01-01-SUMMARY.md'), [ + '---', + 'phase: 01-dirty', + 'key-files:', + ' created: [src/landed-one.ts, src/never-landed.ts]', + 'status: complete', + '---', '', + '# Phase 1 Summary', '', + 'Created `src/landed-one.ts` and `src/never-landed.ts`.', + '', + ].join('\n')); + + fs.writeFileSync(path.join(cleanDir, '02-01-PLAN.md'), '# Plan\nDo the work.\n'); + fs.writeFileSync(path.join(cleanDir, '02-01-SUMMARY.md'), [ + '---', + 'phase: 02-clean', + 'key-files:', + ' created: [src/landed-two.ts]', + 'status: complete', + '---', '', + '# Phase 2 Summary', '', + 'Created `src/landed-two.ts`.', + '', + ].join('\n')); + + return { tmpDir }; +} + +describe('#2572: phase complete warns when a SUMMARY claims files that never landed', () => { + test('#2572-1: a SUMMARY naming a file that is not on disk produces a warning at completion', () => { + const { tmpDir } = build2572SummaryArtifactFixture(); + try { + const { output } = runVerifiedPhaseComplete(['phase', 'complete', '1'], tmpDir); + const parsed = JSON.parse(output); + const warnings = parsed.warnings || []; + assert.ok( + warnings.some((w) => w.includes('src/never-landed.ts')), + `#2572-1 FAILED: expected a warning naming src/never-landed.ts, got: ${JSON.stringify(warnings)}`, + ); + assert.ok( + warnings.some((w) => w.includes('01-01-SUMMARY.md')), + `#2572-1 FAILED: the warning must name the SUMMARY it came from, got: ${JSON.stringify(warnings)}`, + ); + assert.strictEqual(parsed.has_warnings, true, '#2572-1 FAILED: has_warnings must be true'); + } finally { + cleanup(tmpDir); + } + }); + + test('#2572-2: the advisory is ADVISORY — completion still succeeds and reports the phase complete', () => { + const { tmpDir } = build2572SummaryArtifactFixture(); + try { + const { output } = runVerifiedPhaseComplete(['phase', 'complete', '1'], tmpDir); + const parsed = JSON.parse(output); + assert.strictEqual(parsed.completed_phase, '1', '#2572-2 FAILED: completion must not be blocked by the advisory'); + assert.strictEqual(parsed.state_updated, true, '#2572-2 FAILED: STATE.md must still be written'); + } finally { + cleanup(tmpDir); + } + }); + + test('#2572-3 (control): a phase whose SUMMARY files all exist emits NO artifact warning', () => { + const { tmpDir } = build2572SummaryArtifactFixture(); + try { + const { output } = runVerifiedPhaseComplete(['phase', 'complete', '2'], tmpDir); + const parsed = JSON.parse(output); + const warnings = parsed.warnings || []; + assert.ok( + !warnings.some((w) => /not on disk/.test(w)), + `#2572-3 FAILED: a clean phase must emit no artifact advisory. This is the ` + + `"absent for a clean one" half — and the fixture references a real '/'-bearing ` + + `path, so silence here is earned, not vacuous. Got: ${JSON.stringify(warnings)}`, + ); + } finally { + cleanup(tmpDir); + } + }); + + test('#2572-4: the shipped template frontmatter flow sequence produces no phantom "[path" warning', () => { + const { tmpDir } = build2572SummaryArtifactFixture(); + try { + const { output } = runVerifiedPhaseComplete(['phase', 'complete', '2'], tmpDir); + const warnings = JSON.parse(output).warnings || []; + assert.ok( + !warnings.some((w) => w.includes('[src/')), + `#2572-4 FAILED (#2685 Blocker 1): a YAML flow sequence in frontmatter must not ` + + `leak a bracket-prefixed candidate. Got: ${JSON.stringify(warnings)}`, + ); + } finally { + cleanup(tmpDir); + } + }); +}); diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index be6a974d1..af76941fe 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -2664,3 +2664,227 @@ describe('AC3: executable proof — file-wide ban vs region-scoped simultaneousl }); }); } + +// ─── #2572: verify-summary pure core, callable without the CLI wrapper ─────── + +describe('verifySummaryCore — reusable structured contract (#2572)', () => { + const fs = require('node:fs'); + const os = require('node:os'); + const path = require('node:path'); + const { after } = require('node:test'); + const { execSync } = require('node:child_process'); + const { cleanup } = require('./helpers.cjs'); + const { verifySummaryCore } = require('../gsd-core/bin/lib/verify.cjs'); + + const dirs = []; + after(() => { while (dirs.length) cleanup(dirs.pop()); }); + + function repo(summaryBody, extraFiles = {}) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2572-')); + dirs.push(dir); + execSync('git init -q', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.email "t@t.com"', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.name "T"', { cwd: dir, stdio: 'pipe' }); + execSync('git config commit.gpgsign false', { cwd: dir, stdio: 'pipe' }); + for (const [rel, body] of Object.entries(extraFiles)) { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), body); + } + fs.writeFileSync(path.join(dir, 'SUMMARY.md'), summaryBody); + execSync('git add -A && git commit -q -m seed', { cwd: dir, stdio: 'pipe' }); + return dir; + } + + test('returns the structured contract without writing to stdout', () => { + const dir = repo('# Summary\n\nCreated: `src/a.ts`\n', { 'src/a.ts': 'x\n' }); + const r = verifySummaryCore(dir, 'SUMMARY.md', 2); + assert.strictEqual(typeof r, 'object'); + assert.deepStrictEqual(Object.keys(r).sort(), ['checks', 'errors', 'passed']); + assert.strictEqual(r.checks.summary_exists, true); + assert.strictEqual(r.passed, true); + }); + + test('reports a missing referenced file as a structured check, not a throw', () => { + const dir = repo('# Summary\n\nCreated: `src/gone.ts`\n'); + const r = verifySummaryCore(dir, 'SUMMARY.md', 2); + assert.strictEqual(r.passed, false); + assert.ok(r.checks.files_created.missing.includes('src/gone.ts'), + `expected src/gone.ts in missing, got ${JSON.stringify(r.checks.files_created)}`); + }); + + test('absent SUMMARY yields summary_exists false — never throws', () => { + const dir = repo('# Summary\n'); + let r; + assert.doesNotThrow(() => { r = verifySummaryCore(dir, 'nope/SUMMARY.md', 2); }); + assert.strictEqual(r.checks.summary_exists, false); + assert.strictEqual(r.passed, false); + }); + + test('unresolvable commit hash is reported via commits_exist', () => { + const dir = repo('# Summary\n\nCommit: deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n'); + const r = verifySummaryCore(dir, 'SUMMARY.md', 2); + assert.strictEqual(r.checks.commits_exist, false); + assert.ok(r.errors.some((e) => /commit/i.test(e)), `expected a commit error, got ${JSON.stringify(r.errors)}`); + }); + + // ── Blocker 1 (#2685 review): frontmatter must be stripped before extraction ── + // + // All three SUMMARY templates prescribe a YAML flow sequence for key-files: + // key-files: + // created: [src/auth/login.ts, src/auth/session.ts] + // The prose pattern would otherwise capture the literal '[' as part of the + // first path, producing a candidate that can never exist on disk — firing on + // a healthy project built from GSD's own shipped template. + + test('#2685 B1: a template-shaped frontmatter flow sequence yields no phantom "[path" candidate', () => { + const body = [ + '---', + 'phase: 04-auth', + 'key-files:', + ' created: [src/auth/login.ts, src/auth/session.ts]', + ' modified: [src/auth/session.ts]', + 'status: complete', + '---', + '', + '# Phase 4 Summary', + '', + 'Built the login flow in `src/auth/login.ts`.', + '', + ].join('\n'); + const dir = repo(body, { 'src/auth/login.ts': 'x\n', 'src/auth/session.ts': 'x\n' }); + const r = verifySummaryCore(dir, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.deepStrictEqual( + r.checks.files_created.missing, [], + `#2685 B1 FAILED: every named file exists on disk, so nothing may be reported missing. ` + + `Got: ${JSON.stringify(r.checks.files_created)}`, + ); + assert.ok( + !r.checks.files_created.missing.some((f) => f.includes('[')), + 'a bracket-prefixed candidate must never reach the missing list', + ); + }); + + test('#2685 B1: the check is still ABSENT for a clean phase and PRESENT for a dirty one', () => { + const clean = repo('# Summary\n\nBuilt `src/kept.ts` here.\n', { 'src/kept.ts': 'x\n' }); + const rc = verifySummaryCore(clean, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.strictEqual(rc.checks.files_created.checked, 1, 'the clean fixture must actually extract a candidate (not vacuously pass)'); + assert.deepStrictEqual(rc.checks.files_created.missing, [], 'clean phase must not warn'); + + const dirty = repo('# Summary\n\nBuilt `src/kept.ts` and `src/never-landed.ts`.\n', { 'src/kept.ts': 'x\n' }); + const rd = verifySummaryCore(dirty, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.deepStrictEqual( + rd.checks.files_created.missing, ['src/never-landed.ts'], + `dirty phase must name exactly the file that never landed, got ${JSON.stringify(rd.checks.files_created.missing)}`, + ); + }); + + // ── Major 1: checkCount is the single most behavior-defining constant here ── + + test('#2685 M1: checkCount boundary — 1, 2, 3 extractable paths against the default cap', () => { + const mk = (n) => repo('# Summary\n\n' + Array.from({ length: n }, (_, i) => `- \`src/m${i}.ts\``).join('\n') + '\n'); + assert.strictEqual(verifySummaryCore(mk(1), 'SUMMARY.md', undefined, { checkCommits: false }).checks.files_created.checked, 1); + assert.strictEqual(verifySummaryCore(mk(2), 'SUMMARY.md', undefined, { checkCommits: false }).checks.files_created.checked, 2); + assert.strictEqual( + verifySummaryCore(mk(3), 'SUMMARY.md', undefined, { checkCommits: false }).checks.files_created.checked, 2, + 'the CLI default must remain capped at 2 — unchanged from before #2572', + ); + assert.strictEqual( + verifySummaryCore(mk(3), 'SUMMARY.md', Infinity, { checkCommits: false }).checks.files_created.checked, 3, + 'Infinity must lift the cap so an interrupted phase reports every missing file', + ); + }); + + test('#2685 M1: an interrupted phase listing 12 files of which 9 are missing reports all 9', () => { + const refs = Array.from({ length: 12 }, (_, i) => `src/f${i}.ts`); + const present = Object.fromEntries(refs.slice(0, 3).map((f) => [f, 'x\n'])); + const dir = repo('# Summary\n\n' + refs.map((f) => `- built \`${f}\``).join('\n') + '\n', present); + + const capped = verifySummaryCore(dir, 'SUMMARY.md', undefined, { checkCommits: false }); + assert.strictEqual( + capped.checks.files_created.missing.length, 0, + 'precondition: at the 2-file default this real defect is invisible — that is exactly Major 1', + ); + + const all = verifySummaryCore(dir, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.strictEqual(all.checks.files_created.missing.length, 9, + `expected all 9 missing, got ${JSON.stringify(all.checks.files_created.missing)}`); + }); + + // ── Major 3: the advisory path must spawn no git subprocesses ── + + test('#2685 M3: checkCommits:false skips hash resolution entirely', () => { + const body = '# Summary\n\n## Task Commits\n- deadbeefdeadbeefdeadbeefdeadbeefdeadbeef initial\n'; + const dir = repo(body); + const r = verifySummaryCore(dir, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.strictEqual(r.checks.commits_exist, false, 'commits_exist is false meaning NOT CHECKED'); + assert.deepStrictEqual( + r.errors.filter((e) => /commit/i.test(e)), [], + 'with commit checking off, an unresolvable hash must not manufacture an error', + ); + }); + + // ── Major 4 residue: confirmed false-positive classes stay filtered ── + + test('#2685 M4: globs, bare hostnames, and traversal references are never probed', () => { + const body = [ + '# Summary', + '', + // Each noise class is BACKTICKED on purpose: pattern 1 extracts any + // backticked `.`, so these genuinely reach the candidate + // filter. An un-backticked fixture would pass vacuously. + 'Touched `src/**/*.ts` across the tree.', + 'See `docs.example.com/guide.html` for background.', + 'Also `../../../../etc/passwd` and `https://example.com/x.html`.', + 'Real file: `src/real.ts`.', + '', + ].join('\n'); + const dir = repo(body, { 'src/real.ts': 'x\n' }); + const r = verifySummaryCore(dir, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.strictEqual( + r.checks.files_created.checked, 1, + `only src/real.ts is a probeable candidate, got checked=${r.checks.files_created.checked}`, + ); + assert.deepStrictEqual(r.checks.files_created.missing, [], 'no noise class may be reported missing'); + }); + + test('#2685 minor: a dotfile directory is still a valid first segment', () => { + const dir = repo('# Summary\n\nAdded `.github/workflows/ci.yml`.\n', { '.github/workflows/ci.yml': 'x\n' }); + const r = verifySummaryCore(dir, 'SUMMARY.md', Infinity, { checkCommits: false }); + assert.strictEqual(r.checks.files_created.checked, 1, + '.github/... must not be mistaken for a hostname'); + assert.deepStrictEqual(r.checks.files_created.missing, []); + }); + + // ── Minor: property coverage over the extractor (it is a parser) ── + + test('#2685 property: no synthesized SUMMARY body yields a malformed candidate', () => { + const fc = require('./helpers/fast-check-setup.cjs'); + const dir = repo('# seed\n'); + const summaryPath = path.join(dir, 'SUMMARY.md'); + fc.assert( + fc.property(fc.string(), fc.array(fc.string(), { maxLength: 8 }), (prose, paths) => { + const body = [ + '---', + 'key-files:', + ` created: [${paths.join(', ')}]`, + '---', + '', + prose, + ...paths.map((p) => `- built \`${p}\``), + ].join('\n'); + fs.writeFileSync(summaryPath, body); + const r = verifySummaryCore(dir, 'SUMMARY.md', Infinity, { checkCommits: false }); + for (const c of r.checks.files_created.missing) { + assert.ok(!c.includes('['), `bracket artifact leaked: ${JSON.stringify(c)}`); + assert.ok(!c.includes('*') && !c.includes('?'), `glob leaked: ${JSON.stringify(c)}`); + assert.ok( + path.resolve(dir, c).startsWith(path.resolve(dir) + path.sep), + `candidate escaped the project root: ${JSON.stringify(c)}`, + ); + } + return true; + }), + { numRuns: 200 }, + ); + }); +});