From 4414f4cc9377a9df06c7847433fe567ba6398aa8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 12 Nov 2026 23:44:36 +0800 Subject: [PATCH] Stop /ship's docs audit from refusing Next.js bracketed route paths (GST-2554) (#4321) * Stop /ship's docs audit from refusing Next.js bracketed route paths (GST-2554) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * List the bracketed route path rule in the docs audit troubleshooting entry (GST-2554) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-docs-candidate | 15 +++-- ...ocs-audit-accepts-bracketed-route-paths.md | 4 ++ docs/troubleshooting.md | 5 +- lib/ship-docs-result.ts | 16 +++--- test/docs-audit-result.test.ts | 55 ++++++++++++++++++- 5 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 changelog.d/docs-audit-accepts-bracketed-route-paths.md diff --git a/bin/gstack-docs-candidate b/bin/gstack-docs-candidate index de740f9c3..de2059696 100755 --- a/bin/gstack-docs-candidate +++ b/bin/gstack-docs-candidate @@ -75,6 +75,12 @@ function git(args: string[]): string { return r.stdout; } const nul = (text: string) => text.split('\0').filter(Boolean); +/** A listed path holding a glob character is a real file (a Next.js [id] route, #4313) only when it exists from the repository root. */ +function repoFile(): (repoPath: string) => boolean { + const r = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 60_000 }); + const top = r.status === 0 ? r.stdout.trim() : process.cwd(); + return repoPath => fs.existsSync(path.join(top, repoPath)); +} function state(base: string) { const head = git(['rev-parse', 'HEAD']).trim(); @@ -328,11 +334,12 @@ function main(argv: string[]): void { const outDir = path.resolve(one(rest, '--out-dir')); const repairOf = optional(rest, '--repair-of'); const output = fs.readFileSync(file, 'utf8'); + const isFile = repoFile(); let check: DocsResultCheck; - if (!repairOf) check = checkDocsResult(output, auditId); + if (!repairOf) check = checkDocsResult(output, auditId, isFile); else { const original = fs.readFileSync(repairOf, 'utf8'); - const prior = checkDocsResult(original, auditId); + const prior = checkDocsResult(original, auditId, isFile); let first = true; if (!prior.ok && prior.repairable) { fs.mkdirSync(outDir, { recursive: true, mode: 0o700 }); @@ -341,7 +348,7 @@ function main(argv: string[]): void { first = false; } } - check = first ? checkDocsRepair(output, original, auditId) + check = first ? checkDocsRepair(output, original, auditId, isFile) : { ok: false, problems: ['a format repair was already used for this audit', ...(prior.ok ? [] : prior.problems)], trimmed: [], repairable: false, kept: {} }; } if (check.trimmed.length) console.log(`KEYS_TRIMMED: ${check.trimmed.map(k => JSON.stringify(k)).join(' ')}`); @@ -368,7 +375,7 @@ function main(argv: string[]): void { for (let i = 0; i < rest.length; i += 2) if (rest[i] !== '--audit-id') throw new Error(`unknown argument: ${rest[i]}`); const auditId = auditIdArg(rest); const output = fs.readFileSync(file, 'utf8'); - const check = checkDocsResult(output, auditId); + const check = checkDocsResult(output, auditId, repoFile()); if (check.ok || !check.repairable) throw new Error(`${file} is not a format-only failure of the last line, so it has no format repair: ${check.ok ? 'it is a valid result' : 'a content failure counts as an attempt (Blocked recovery, RETRY_NOTE:)'}`); process.stdout.write(docsRepairPrompt(output, auditId, check.problems)); return; diff --git a/changelog.d/docs-audit-accepts-bracketed-route-paths.md b/changelog.d/docs-audit-accepts-bracketed-route-paths.md new file mode 100644 index 000000000..65223ffce --- /dev/null +++ b/changelog.d/docs-audit-accepts-bracketed-route-paths.md @@ -0,0 +1,4 @@ +--- +important: blocker +--- +- **/ship's documentation audit no longer refuses Next.js and SvelteKit route files.** When the Step 14.5 docs audit listed a reviewed or updated file whose path has square brackets, such as `app/api/items/[id]/route.ts`, `[...slug]` or `[[...opt]]`, the result check read the brackets as a glob, refused the result with `DOCS_RESULT_INVALID` on both attempts, and stopped an unattended /ship at Blocked recovery even though the audit found nothing wrong (#4313). A listed path that holds `*`, `?`, `[` or `]` now counts as a file when that exact file exists from the repository root, so file-routed projects ship normally. A pattern such as `docs/*.md` or `docs/[ab].md`, or a bracketed path that names no file, is still refused as a glob. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 412a71527..315a3d55e 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -7146,7 +7146,10 @@ exception. last nonempty line and it does not meet /ship Step 14.5's result contract: a key is missing, unknown or appears twice once whitespace around keys is trimmed, a field has the wrong type, the audit id, schema version or status invariant is -wrong, or `documentation_section` is empty, a placeholder (such as `"x"`) or +wrong, a `files_updated` or `files_reviewed` entry is a duplicate, an absolute +path or a glob (a path holding `*`, `?`, `[` or `]` counts as a file only when that +exact file exists from the repository root, so a route such as +`app/api/items/[id]/route.ts` passes), or `documentation_section` is empty, a placeholder (such as `"x"`) or carries a `## Documentation` heading. Whitespace around a key is trimmed first and reported as `KEYS_TRIMMED:`, so `" documentation_section"` counts as `documentation_section`. The line names each problem. Nothing was saved: the audit's result cannot be diff --git a/lib/ship-docs-result.ts b/lib/ship-docs-result.ts index 292844cca..4a91cbd89 100644 --- a/lib/ship-docs-result.ts +++ b/lib/ship-docs-result.ts @@ -3,7 +3,8 @@ * /document-release child ends with (ship/sections/documentation.md, "Return one JSON object on the LAST nonempty line"). * Whitespace around a key is trimmed (GST-1843: a reader ended with `" documentation_section": "x"`), and every other rule * of the contract stays: all eight fields with their types, the exact audit id, schema 1, the status invariants, unique - * repo-relative paths, and a section that is real Markdown rather than empty or a placeholder. A result that fails names + * repo-relative paths (a path holding *, ?, [ or ] is a glob unless it names a real file, so a Next.js or SvelteKit route such as + * app/api/items/[id]/route.ts passes, #4313), and a section that is real Markdown rather than empty or a placeholder. A result that fails names * each bad key and carries a retry note quoting the exact keys, so the one re-audit is told what to correct. * A failure of shape alone (the line is one object; only keys, types or the section are wrong) is `repairable`: the parent * re-asks for the line once (docsRepairPrompt) without spending an attempt, and checkDocsRepair accepts the reply only when @@ -21,11 +22,12 @@ export interface DocsResult { } export type DocsResultCheck = { ok: true; result: DocsResult; trimmed: string[] } | { ok: false; problems: string[]; trimmed: string[]; repairable: boolean; kept: Record }; -const badPath = (p: string) => p.startsWith('/') || p.includes('\\') || /[*?[\]]/.test(p) || p.split('/').some(part => !part || part === '.' || part === '..'); +export type IsFile = (repoPath: string) => boolean; +const badPath = (p: string, isFile: IsFile) => p.startsWith('/') || p.includes('\\') || p.split('/').some(part => !part || part === '.' || part === '..') || (/[*?[\]]/.test(p) && !isFile(p)); const isPlaceholder = (s: string) => /^[!-~]+$/.test(s) || /^<[^<>]*>$/.test(s); const shown = (x: unknown) => { const s = JSON.stringify(x) ?? String(x); return s.length > 40 ? `${s.slice(0, 40)}…` : s; }; -export function checkDocsResult(output: string, auditId: string): DocsResultCheck { +export function checkDocsResult(output: string, auditId: string, isFile: IsFile = () => false): DocsResultCheck { const last = output.replace(/\r\n/g, '\n').split('\n').map(l => l.trim()).filter(Boolean).at(-1) ?? ''; let parsed: unknown; try { parsed = JSON.parse(last); } catch { parsed = undefined; } @@ -53,7 +55,7 @@ export function checkDocsResult(output: string, auditId: string): DocsResultChec const list = v[key]; const pinsOutcome = key === 'files_updated' || key === 'blockers'; if (!Array.isArray(list) || list.some(x => typeof x !== 'string' || !x.trim())) { problems.push(`"${key}" is not a list of nonempty strings`); bad.add(key); if (pinsOutcome) content = true; continue; } - if (key.startsWith('files_') && (new Set(list).size !== list.length || list.some(badPath))) { problems.push(`"${key}" must list unique repo-relative files, not globs`); bad.add(key); if (pinsOutcome) content = true; } + if (key.startsWith('files_') && (new Set(list).size !== list.length || list.some(p => badPath(p, isFile)))) { problems.push(`"${key}" must list unique repo-relative files, not globs`); bad.add(key); if (pinsOutcome) content = true; } } const section = v.documentation_section; const sectionProblems = problems.length; @@ -96,10 +98,10 @@ export function docsRepairPrompt(output: string, auditId: string, problems: read } /** Checks a format repair's reply: a valid result whose every value the repaired line already gave correctly is unchanged. Never repairable again. */ -export function checkDocsRepair(output: string, original: string, auditId: string): DocsResultCheck { - const before = checkDocsResult(original, auditId); +export function checkDocsRepair(output: string, original: string, auditId: string, isFile: IsFile = () => false): DocsResultCheck { + const before = checkDocsResult(original, auditId, isFile); if (before.ok || !before.repairable) return { ok: false, problems: ['the --repair-of output is not a format-only failure, so it has no format repair'], trimmed: [], repairable: false, kept: {} }; - const after = checkDocsResult(output, auditId); + const after = checkDocsResult(output, auditId, isFile); if (!after.ok) return { ...after, repairable: false }; const result = after.result as unknown as Record; const changed = Object.keys(before.kept).filter(key => JSON.stringify(result[key]) !== JSON.stringify(before.kept[key])); diff --git a/test/docs-audit-result.test.ts b/test/docs-audit-result.test.ts index 067c26077..af056f51c 100644 --- a/test/docs-audit-result.test.ts +++ b/test/docs-audit-result.test.ts @@ -16,8 +16,9 @@ function scratch() { return root; } -function result(output: string, auditId: string, repairOf?: { file: string; out: string }) { +function result(output: string, auditId: string, repairOf?: { file: string; out: string }, files: string[] = []) { const root = scratch(); + for (const f of files) { fs.mkdirSync(path.dirname(path.join(root, f)), { recursive: true }); fs.writeFileSync(path.join(root, f), ''); } const file = path.join(root, 'child.txt'); fs.writeFileSync(file, output); const out = repairOf?.out ?? path.join(root, 'record'); @@ -82,6 +83,58 @@ describe('gstack-docs-candidate result', () => { expect(r.written).toEqual([]); }); + test.each([ + ['a Next.js dynamic route', 'app/api/items/[id]/route.ts'], + ['a catch-all route', 'app/blog/[...slug]/page.tsx'], + ['an optional catch-all route', 'app/shop/[[...opt]]/page.tsx'], + ['a route group around a dynamic segment', 'app/(marketing)/blog/[slug]/page.tsx'], + ['a SvelteKit route', 'src/routes/blog/[slug]/+page.svelte'], + ...(process.platform === 'win32' ? [] : [['a file named with * and ?', 'docs/what-is-*this*?.md']]), + ])('%s that names a real file is a path, not a glob (#4313)', (_name, file) => { + const r = result(line({ documentation_section: SECTION, files_reviewed: ['README.md', file] }) + '\n', 'ship352b-docs-1', undefined, ['README.md', file]); + expect([r.status, r.stderr]).toEqual([0, '']); + expect(r.stdout).toContain('DOCS_RESULT: ok status=current'); + expect(JSON.parse(fs.readFileSync(path.join(r.out, 'ship352b-docs-1-result.json'), 'utf8')).files_reviewed).toEqual(['README.md', file]); + }); + + test('a route group alone was never read as a glob', () => { + const r = result(line({ documentation_section: SECTION, files_reviewed: ['app/(group)/page.tsx'] }) + '\n', 'ship352b-docs-1'); + expect(r.status).toBe(0); + }); + + test('an updated bracketed route file is accepted in files_updated too', () => { + const file = 'app/api/items/[id]/route.ts'; + const r = result(line({ documentation_section: SECTION, status: 'updated', files_updated: [file], files_reviewed: [file] }) + '\n', 'ship352b-docs-1', undefined, [file]); + expect([r.status, r.stderr]).toEqual([0, '']); + expect(r.stdout).toContain('DOCS_RESULT: ok status=updated files_updated=1'); + }); + + test.each([ + ['a bracketed path that names no file', 'app/api/items/[id]/route.ts', []], + ['a glob even when it would match a file', 'docs/*.md', ['docs/a.md']], + ['a character class even when it would match a file', 'docs/[ab].md', ['docs/a.md']], + ])('%s is still refused as a glob', (_name, listed, files) => { + const r = result(line({ documentation_section: SECTION, files_reviewed: [listed] }) + '\n', 'ship352b-docs-1', undefined, files); + expect(r.status).toBe(1); + expect(r.stdout).toContain('(DOCS_RESULT_INVALID)'); + expect(r.stdout).toContain('"files_reviewed" must list unique repo-relative files, not globs'); + expect(r.written).toEqual([]); + }); + + test('a bracketed path is looked up from the repository root when result runs in a subdirectory', () => { + const file = 'app/api/items/[id]/route.ts'; + const root = scratch(); + expect(spawnSync('git', ['init', '-q', root], { encoding: 'utf8', timeout: 20_000 }).status).toBe(0); + fs.mkdirSync(path.join(root, path.dirname(file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), ''); + const sub = path.join(root, 'sub'); + fs.mkdirSync(sub); + const output = path.join(sub, 'child.txt'); + fs.writeFileSync(output, line({ documentation_section: SECTION, files_reviewed: [file] }) + '\n'); + const r = spawnSync(BIN, ['result', output, '--audit-id', 'ship352b-docs-1', '--out-dir', path.join(sub, 'record')], { cwd: sub, encoding: 'utf8', timeout: 20_000 }); + expect([r.status, r.stderr]).toEqual([0, '']); + }); + test('two keys that trim to the same name are refused, not merged', () => { const r = result(line({ documentation_section: SECTION }).replace(/}$/, ',"documentation_section ":"second"}'), 'ship352b-docs-1'); expect(r.status).toBe(1);