From aa46032f2be715cba570af61d6a059a2853fba7d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 7 Nov 2026 14:27:38 -0800 Subject: [PATCH] Stop /ship from stalling when the docs checker misspells a field in its reply (GST-1843) (#4147) * Stop /ship from stalling when the docs checker misspells a field in its reply (GST-1843) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Make the docs reply check refuse hidden extra fields and keep old audit ids out of the retry Review-Finding: p1/specialist-batched#1 reader=security Review-Finding: p1/specialist-batched#2 reader=testing Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/red-team#2 reader=red-team Review-Finding: p1/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Let the docs eval fixture run the new reply check, and keep odd audit ids and keys from escaping it Review-Finding: v1/outside-voice#2 reader=outside-voice Review-Finding: v1/adversarial-native#1 reader=adversarial-native Review-Finding: fc1/fix-check#1 reader=fix-check Review-Finding: v1/outside-voice#1 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Add the changelog entry for the docs reply check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * List every way a docs checker reply can be refused in the troubleshooting entry 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 | 38 ++++- bin/gstack-doctor-components.sh | 1 + .../capy-docs-audit-malformed-key-gst-1843.md | 1 + docs/troubleshooting.md | 18 +++ lib/result-codes-release.ts | 5 + lib/ship-docs-result.ts | 68 +++++++++ ship/sections/documentation.md | 23 ++- ship/sections/documentation.md.tmpl | 23 ++- test/docs-audit-result.test.ts | 142 ++++++++++++++++++ test/docsync-command-grammar.test.ts | 24 +++ test/helpers/docsync-interface.ts | 2 +- test/helpers/docsync-observer.ts | 10 +- ...apy-docs-audit-malformed-key-gst-1843.json | 4 + 13 files changed, 339 insertions(+), 20 deletions(-) create mode 100644 changelog.d/capy-docs-audit-malformed-key-gst-1843.md create mode 100644 lib/ship-docs-result.ts create mode 100644 test/docs-audit-result.test.ts create mode 100644 test/size-ledger/ship/capy-docs-audit-malformed-key-gst-1843.json diff --git a/bin/gstack-docs-candidate b/bin/gstack-docs-candidate index 05917afe4..07c0ede12 100755 --- a/bin/gstack-docs-candidate +++ b/bin/gstack-docs-candidate @@ -8,6 +8,7 @@ * [--run ] [--pre [--carry ]] * gstack-docs-candidate compare * gstack-docs-candidate attempt --state + * gstack-docs-candidate result --audit-id --out-dir * gstack-docs-candidate [snapshot|compare] --help * * snapshot saves, NUL-safely and without writing to the repository: base SHA, @@ -46,6 +47,13 @@ * refusal, and no audit JSON) never audited, so it is not counted, once per * invocation (--state, the invocation's record); a second refusal is counted * and stops for the install fix (GST-391, gbrain #8084). + * + * result checks the child's last nonempty line against Step 14.5's JSON contract + * (lib/ship-docs-result.ts), trimming whitespace around keys. On success it saves + * -result.json and the section, unchanged, as -documentation.md + * in --out-dir and prints DOCS_RESULT: ok. Otherwise it writes nothing, prints a + * DOCS_RESULT_INVALID line naming each bad key and a RETRY_NOTE: for the re-audit's + * prompt, and exits 1 (GST-1843). */ import { spawnSync } from 'child_process'; import * as fs from 'fs'; @@ -53,6 +61,7 @@ import * as path from 'path'; import { baseContainsHead } from '../lib/default-base'; import { baseArg } from '../lib/gate-diff/git'; import { resultLine } from '../lib/result-codes'; +import { checkDocsResult, docsRetryNote } from '../lib/ship-docs-result'; import { parseReceiptDocs, type ReceiptDocs } from '../lib/ship-receipt-docs'; function git(args: string[]): string { @@ -130,7 +139,8 @@ function one(args: string[], name: string): string { const USAGE = `usage: gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only --base [--select ]... [--docs [,...]]... [--exception ] [--run ] [--pre [--carry ]] gstack-docs-candidate compare - gstack-docs-candidate attempt --state `; + gstack-docs-candidate attempt --state + gstack-docs-candidate result --audit-id --out-dir `; /** The owner's accepted Step 14.5 exception from the blocked .gstack/tmp/docs-audit.json record, read by the receipt's own parser; throws when the record is not blocked or names no complete exception. */ function acceptedException(file: string): NonNullable & { scope: string } { @@ -196,7 +206,7 @@ function recordProblems(record: any): string[] { function main(argv: string[]): void { const [command, ...args] = argv; const topHelp = (command === '--help' || command === '-h') && !args.length; - const commandHelp = (command === 'snapshot' || command === 'compare' || command === 'attempt') && args.length === 1 && args[0] === '--help'; + const commandHelp = (command === 'snapshot' || command === 'compare' || command === 'attempt' || command === 'result') && args.length === 1 && args[0] === '--help'; if (topHelp || commandHelp) { console.log(USAGE); return; @@ -298,6 +308,30 @@ function main(argv: string[]): void { console.log('ATTEMPT: not-counted (the audit child\'s install check refused document-release)'); return; } + if (command === 'result') { + const [file, ...rest] = args; + if (!file || file.startsWith('--')) throw new Error('result takes the child output file, then --audit-id --out-dir '); + for (let i = 0; i < rest.length; i += 2) if (!['--audit-id', '--out-dir'].includes(rest[i]!)) throw new Error(`unknown argument: ${rest[i]}`); + const auditId = one(rest, '--audit-id'); + if (!/^[A-Za-z0-9._-]+$/.test(auditId) || auditId.includes('..')) throw new Error(`--audit-id ${JSON.stringify(auditId)} must be letters, digits, dot, underscore or dash, with no '..'`); + const outDir = path.resolve(one(rest, '--out-dir')); + const check = checkDocsResult(fs.readFileSync(file, 'utf8'), auditId); + if (check.trimmed.length) console.log(`KEYS_TRIMMED: ${check.trimmed.map(k => JSON.stringify(k)).join(' ')}`); + if (!check.ok) { + console.log(resultLine('gstack-docs-candidate result', 'DOCS_RESULT_INVALID', check.problems.join('; '))); + console.log(`RETRY_NOTE: ${docsRetryNote(check.problems)}`); + process.exit(1); + } + fs.mkdirSync(outDir, { recursive: true, mode: 0o700 }); + const resultFile = path.join(outDir, `${auditId}-result.json`); + const sectionFile = path.join(outDir, `${auditId}-documentation.md`); + fs.writeFileSync(resultFile, JSON.stringify(check.result) + '\n', { mode: 0o600 }); + fs.writeFileSync(sectionFile, check.result.documentation_section, { mode: 0o600 }); + console.log(`DOCS_RESULT: ok status=${check.result.status} files_updated=${check.result.files_updated.length}`); + console.log(`RESULT: ${resultFile}`); + console.log(`SECTION: ${sectionFile}`); + return; + } throw new Error(USAGE); } diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index 0d3750ec0..9214221b0 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -338,6 +338,7 @@ gstack_result_anchor() { RECEIPT_CARRY_PRODUCT_CHANGED) printf '%s' 'receipt-carry-product-changed' ;; RECEIPT_CARRY_SCOPED_REVIEW) printf '%s' 'receipt-carry-scoped-review' ;; DOCS_AUDIT_BLOCKED) printf '%s' 'docs-audit-blocked' ;; + DOCS_RESULT_INVALID) printf '%s' 'docs-result-invalid' ;; RECEIPT_PROSE_MISMATCH) printf '%s' 'receipt-prose-mismatch' ;; RECEIPT_CARRY_PREGATE_STALE) printf '%s' 'receipt-carry-pregate-stale' ;; RECEIPT_GIT_TOO_OLD) printf '%s' 'receipt-git-too-old' ;; diff --git a/changelog.d/capy-docs-audit-malformed-key-gst-1843.md b/changelog.d/capy-docs-audit-malformed-key-gst-1843.md new file mode 100644 index 000000000..9679dcb68 --- /dev/null +++ b/changelog.d/capy-docs-audit-malformed-key-gst-1843.md @@ -0,0 +1 @@ +- **/ship no longer stops when the docs checker's reply has a small mistake in it.** Before shipping, /ship asks a helper to check the docs and reply with a short summary line. When that helper typed a field name with a stray space and left the docs summary as a stand-in like "x", /ship treated the whole check as blocked and an unattended run stopped. Now `gstack-docs-candidate result` reads that last line strictly. It forgives spaces around field names, still refuses anything else that's wrong (a missing or extra field, the wrong audit id, an empty or stand-in summary), names each problem, and hands the one retry a note saying exactly which fields to fix. A reply that passes is saved with its summary for the PR. The docs test fixture accepts the new command, and odd audit ids or field names can no longer write outside the record folder or fake a line in the output. (GST-1843) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 5349179ea..91ed434dc 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -6818,6 +6818,24 @@ reason }` and carry the `Documentation exception: accepted by via `gstack-docs-candidate snapshot --exception .gstack/tmp/docs-audit.json`; `gstack-ship-release finish` stops on a blocked audit whose snapshot lacks the exception. + + +### `DOCS_RESULT_INVALID` + +**Meaning.** `gstack-docs-candidate result` read the /document-release child's +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 +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 +used, but this is a malformed reply, not a blocked audit. + +**Fix.** Run Step 14.5's one remaining re-audit with the printed `RETRY_NOTE:` +text appended to its prompt, word for word; it quotes the exact keys. With no +attempt left, Step 14.5 stops and reports this code and the keys it names +([ship/sections/documentation.md](../ship/sections/documentation.md)). ## gstack-pr-wait diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index d50d7013f..22f748e7d 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -425,6 +425,11 @@ export const RELEASE_RESULT_CODES = { summary: 'the documentation audit (/ship Step 14.5) is blocked: gstack-ship-receipt docs-caps found a declared doc cap over, or gstack-ship-receipt write was given a blocked audit with no owner exception and refused to record the ship as audited', fix: 'repair the audit (or trim the doc named by the DOCS_CAP: over line) and rerun Step 14.5; when the owner accepts the named risk, record it as docs.exception { accepted_by, date, via, reason } and carry the same line in the PR body\'s Documentation section', }, + DOCS_RESULT_INVALID: { + anchor: 'docs-result-invalid', + summary: 'the documentation audit child\'s last line (/ship Step 14.5) does not meet the result contract: a key is missing, unknown or repeated once whitespace around keys is trimmed, a field has the wrong type or value, or documentation_section is empty or a placeholder; the audit result cannot be used, and this is a malformed reply, not a blocked audit', + fix: 'run the one remaining re-audit with the RETRY_NOTE: line appended to its prompt verbatim; with no attempt left, Step 14.5 stops with this code and the keys it names', + }, RECEIPT_PROSE_MISMATCH: { anchor: 'receipt-prose-mismatch', summary: 'the PR body quotes a counter the ship receipt also records (review passes, deferred findings, gate case counts) with a different number, so the prose and the receipt disagree about the same run', diff --git a/lib/ship-docs-result.ts b/lib/ship-docs-result.ts new file mode 100644 index 000000000..3e4275df7 --- /dev/null +++ b/lib/ship-docs-result.ts @@ -0,0 +1,68 @@ +/** + * ship-docs-result — the deterministic check of /ship Step 14.5's documentation-audit result: the one JSON object a + * /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 + * each bad key and carries a retry note quoting the exact keys, so the one re-audit is told what to correct. + */ +export const DOCS_RESULT_KEYS = ['schema_version', 'audit_id', 'status', 'files_updated', 'files_reviewed', 'blockers', 'decisions', 'documentation_section'] as const; +const LISTS = ['files_updated', 'files_reviewed', 'blockers', 'decisions'] as const; + +export interface DocsResult { + schema_version: 1; audit_id: string; status: 'updated' | 'current' | 'blocked'; + files_updated: string[]; files_reviewed: string[]; blockers: string[]; decisions: string[]; documentation_section: string; +} +export type DocsResultCheck = { ok: true; result: DocsResult; trimmed: string[] } | { ok: false; problems: string[]; trimmed: string[] }; + +const badPath = (p: string) => p.startsWith('/') || p.includes('\\') || /[*?[\]]/.test(p) || p.split('/').some(part => !part || part === '.' || part === '..'); +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 { + 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; } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return { ok: false, problems: ['the last nonempty line is not one JSON object'], trimmed: [] }; + const problems: string[] = []; + const trimmed: string[] = []; + const v: Record = Object.create(null); + for (const [raw, value] of Object.entries(parsed)) { + const key = raw.trim(); + if (key !== raw) trimmed.push(raw); + if (Object.hasOwn(v, key)) problems.push(`${shown(key)} appears twice once whitespace around keys is trimmed`); + v[key] = value; + } + const unknown = Object.keys(v).filter(key => !(DOCS_RESULT_KEYS as readonly string[]).includes(key)); + for (const key of unknown.slice(0, 5)) problems.push(`unknown key ${shown(key)}`); + if (unknown.length > 5) problems.push(`${unknown.length - 5} more unknown keys`); + for (const key of DOCS_RESULT_KEYS) if (!Object.hasOwn(v, key)) problems.push(`"${key}" is missing`); + if (Object.hasOwn(v, 'schema_version') && v.schema_version !== 1) problems.push(`"schema_version" is ${shown(v.schema_version)}, not 1`); + if (Object.hasOwn(v, 'audit_id') && v.audit_id !== auditId) problems.push(`"audit_id" is ${shown(v.audit_id)}, not the audit id this audit was given`); + if (Object.hasOwn(v, 'status') && !['updated', 'current', 'blocked'].includes(v.status as string)) problems.push(`"status" is ${shown(v.status)}, not updated, current or blocked`); + for (const key of LISTS) { + if (!Object.hasOwn(v, key)) continue; + const list = v[key]; + if (!Array.isArray(list) || list.some(x => typeof x !== 'string' || !x.trim())) { problems.push(`"${key}" is not a list of nonempty strings`); 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`); + } + const section = v.documentation_section; + if (Object.hasOwn(v, 'documentation_section')) { + if (typeof section !== 'string' || !section.trim()) problems.push('"documentation_section" is empty; it must be the complete Markdown section'); + else if (isPlaceholder(section.trim())) problems.push(`"documentation_section" is a placeholder (${shown(section.trim())}); it must be the complete Markdown section`); + else if (/^\s*##\s+Documentation\s*$/m.test(section)) problems.push('"documentation_section" carries a ## Documentation heading'); + } + const updated = Array.isArray(v.files_updated) ? v.files_updated.length : 0; + const blockers = Array.isArray(v.blockers) ? v.blockers.length : 0; + if (v.status === 'blocked' && !blockers) problems.push('status blocked needs at least one blocker'); + if ((v.status === 'updated' || v.status === 'current') && blockers) problems.push(`status ${v.status} cannot carry blockers; a blocker means status blocked`); + if (v.status === 'current' && updated) problems.push('status current cannot list files_updated; an edit means status updated'); + if (v.status === 'updated' && !updated) problems.push('status updated needs files_updated'); + if (problems.length) return { ok: false, problems, trimmed }; + return { ok: true, result: v as unknown as DocsResult, trimmed }; +} + +/** The sentence a re-audit's prompt ends with, verbatim, after a DOCS_RESULT_INVALID result. */ +export function docsRetryNote(problems: readonly string[]): string { + return `Your previous audit's last line was rejected (DOCS_RESULT_INVALID): ${problems.join('; ')}. End this audit with one JSON object on the last nonempty line whose keys are exactly ${DOCS_RESULT_KEYS.map(k => JSON.stringify(k)).join(', ')}, spelled as here with no spaces inside the quotes; "documentation_section" holds the complete Markdown section itself, never a placeholder such as "x".`; +} diff --git a/ship/sections/documentation.md b/ship/sections/documentation.md index 5688b719f..0106c8c30 100644 --- a/ship/sections/documentation.md +++ b/ship/sections/documentation.md @@ -58,7 +58,7 @@ Reentry never resets the count or authorizes a launch. > - `schema_version`: integer 1; `audit_id`: the exact supplied string. > - `status`: updated/current/blocked. > - `files_updated`, `files_reviewed`, `blockers`, `decisions`: string arrays. Paths are unique repo-relative files, not globs. -> - `documentation_section`: nonempty Markdown with scope, result and debt, without a ## Documentation heading. No extra or legacy fields. +> - `documentation_section`: the complete section itself, never a placeholder: nonempty Markdown with scope, result and debt, without a ## Documentation heading. Spell every key exactly as listed; no extra or legacy fields. > > Completed audits without blockers are `updated` if edited, otherwise `current`; describe scope even without docs. Failed/incomplete audits are `blocked`, with reasons/partial edits. Read-only corrections block. Metadata observations go only in decisions. @@ -69,10 +69,14 @@ Reentry never resets the count or authorizes a launch. 1. **Collect.** Inspect the child handle for terminal completion and final output within ~10 minutes; launch metadata is not completion. On failure/deadline, use recovery before another writer. -2. **Check output.** Parse only the LAST nonempty line. Require every field/type, - exact audit id, schema, status invariant and actual spawned marker above, as - echoed in the child output; state files are not evidence of it. - Never default or reconstruct missing values. +2. **Check output.** Save the output, then run + `~/.claude/skills/gstack/bin/gstack-docs-candidate result --audit-id --out-dir `. + It parses only the LAST nonempty line, trims whitespace around keys and requires + every field/type, exact audit id, schema and status invariant. `DOCS_RESULT: ok` + saves `-result.json` and the section unchanged as `-documentation.md`; + `DOCS_RESULT_INVALID` names each bad key and prints a `RETRY_NOTE:`. Also require + the actual spawned marker above, as echoed in the child output; state files are + not evidence of it. Never default or reconstruct missing values. 3. **Check ownership.** Run `gstack-docs-candidate compare `. Compare actual changes against the candidate, enforcing prompt/audit-scope permissions and protected-file exclusions. HEAD and index @@ -99,8 +103,8 @@ branch's copy (a PR cannot raise its own cap), prints one `DOCS_CAP: -post.json` changed and `--pre ` added, plus `--carry ` on a re-audit so receipt write -resets both audits' files; Step 16 `compare`s it; never type hashes), status, the child's final JSON line as `-result.json`, and the section copied unchanged as the sole content of a private -`-documentation.md`. That file is the section's single source; +resets both audits' files; Step 16 `compare`s it; never type hashes), status, and keep the `-result.json` and private `-documentation.md` +that `result` wrote. That section file is the section's single source; reports and Step 19 insert it by command (`cat`) where they can, never retyped or edited. Save records once; cite files by path, never copying their content. Print `Documentation: updated` with paths or `Documentation: current` with scope, @@ -124,7 +128,9 @@ auto-commit or push unexpected child commits. STOP ship. Reject late results from abandoned ids. 2. If an attempt remains and either the audited inputs changed or a concrete launch/input/permission correction or reviewed patch repair is available, - apply any repair with user approval for risky edits. + apply any repair with user approval for risky edits. A `DOCS_RESULT_INVALID` + result is such a correction: end the remaining attempt's prompt with its + `RETRY_NOTE:` text verbatim. Repeat Prepare using current inputs and a fresh id/snapshot, run the remaining attempt, then validate it through Parent processing. 3. Otherwise STOP before commit/publication and do not launch another child. @@ -141,3 +147,4 @@ auto-commit or push unexpected child commits. (`DOCS_AUDIT_BLOCKED`). Then save Step 16's post snapshot from it: the Prepare `snapshot` with `--out -post.json --exception .gstack/tmp/docs-audit.json`; finish refuses a blocked audit whose post snapshot lacks it. An unattended run has no owner to accept: it stops here. + Its stop names the failure's own code (`DOCS_RESULT_INVALID` and its keys for a malformed result). diff --git a/ship/sections/documentation.md.tmpl b/ship/sections/documentation.md.tmpl index cf7baabbb..7fbbad6e6 100644 --- a/ship/sections/documentation.md.tmpl +++ b/ship/sections/documentation.md.tmpl @@ -56,7 +56,7 @@ Reentry never resets the count or authorizes a launch. > - `schema_version`: integer 1; `audit_id`: the exact supplied string. > - `status`: updated/current/blocked. > - `files_updated`, `files_reviewed`, `blockers`, `decisions`: string arrays. Paths are unique repo-relative files, not globs. -> - `documentation_section`: nonempty Markdown with scope, result and debt, without a ## Documentation heading. No extra or legacy fields. +> - `documentation_section`: the complete section itself, never a placeholder: nonempty Markdown with scope, result and debt, without a ## Documentation heading. Spell every key exactly as listed; no extra or legacy fields. > > Completed audits without blockers are `updated` if edited, otherwise `current`; describe scope even without docs. Failed/incomplete audits are `blocked`, with reasons/partial edits. Read-only corrections block. Metadata observations go only in decisions. @@ -67,10 +67,14 @@ Reentry never resets the count or authorizes a launch. 1. **Collect.** Inspect the child handle for terminal completion and final output within ~10 minutes; launch metadata is not completion. On failure/deadline, use recovery before another writer. -2. **Check output.** Parse only the LAST nonempty line. Require every field/type, - exact audit id, schema, status invariant and actual spawned marker above, as - echoed in the child output; state files are not evidence of it. - Never default or reconstruct missing values. +2. **Check output.** Save the output, then run + `~/.claude/skills/gstack/bin/gstack-docs-candidate result --audit-id --out-dir `. + It parses only the LAST nonempty line, trims whitespace around keys and requires + every field/type, exact audit id, schema and status invariant. `DOCS_RESULT: ok` + saves `-result.json` and the section unchanged as `-documentation.md`; + `DOCS_RESULT_INVALID` names each bad key and prints a `RETRY_NOTE:`. Also require + the actual spawned marker above, as echoed in the child output; state files are + not evidence of it. Never default or reconstruct missing values. 3. **Check ownership.** Run `gstack-docs-candidate compare `. Compare actual changes against the candidate, enforcing prompt/audit-scope permissions and protected-file exclusions. HEAD and index @@ -97,8 +101,8 @@ branch's copy (a PR cannot raise its own cap), prints one `DOCS_CAP: -post.json` changed and `--pre ` added, plus `--carry ` on a re-audit so receipt write -resets both audits' files; Step 16 `compare`s it; never type hashes), status, the child's final JSON line as `-result.json`, and the section copied unchanged as the sole content of a private -`-documentation.md`. That file is the section's single source; +resets both audits' files; Step 16 `compare`s it; never type hashes), status, and keep the `-result.json` and private `-documentation.md` +that `result` wrote. That section file is the section's single source; reports and Step 19 insert it by command (`cat`) where they can, never retyped or edited. Save records once; cite files by path, never copying their content. Print `Documentation: updated` with paths or `Documentation: current` with scope, @@ -122,7 +126,9 @@ auto-commit or push unexpected child commits. STOP ship. Reject late results from abandoned ids. 2. If an attempt remains and either the audited inputs changed or a concrete launch/input/permission correction or reviewed patch repair is available, - apply any repair with user approval for risky edits. + apply any repair with user approval for risky edits. A `DOCS_RESULT_INVALID` + result is such a correction: end the remaining attempt's prompt with its + `RETRY_NOTE:` text verbatim. Repeat Prepare using current inputs and a fresh id/snapshot, run the remaining attempt, then validate it through Parent processing. 3. Otherwise STOP before commit/publication and do not launch another child. @@ -139,3 +145,4 @@ auto-commit or push unexpected child commits. (`DOCS_AUDIT_BLOCKED`). Then save Step 16's post snapshot from it: the Prepare `snapshot` with `--out -post.json --exception .gstack/tmp/docs-audit.json`; finish refuses a blocked audit whose post snapshot lacks it. An unattended run has no owner to accept: it stops here. + Its stop names the failure's own code (`DOCS_RESULT_INVALID` and its keys for a malformed result). diff --git a/test/docs-audit-result.test.ts b/test/docs-audit-result.test.ts new file mode 100644 index 000000000..719948de1 --- /dev/null +++ b/test/docs-audit-result.test.ts @@ -0,0 +1,142 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { parseDocsCompletion } from './helpers/docsync-contract'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const BIN = path.join(ROOT, 'bin', 'gstack-docs-candidate'); +const roots: string[] = []; +afterEach(() => { for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); }); + +function scratch() { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'docs-result-'))); + roots.push(root); + return root; +} + +function result(output: string, auditId: string) { + const root = scratch(); + const file = path.join(root, 'child.txt'); + fs.writeFileSync(file, output); + const out = path.join(root, 'record'); + const r = spawnSync(BIN, ['result', file, '--audit-id', auditId, '--out-dir', out], { cwd: root, encoding: 'utf8', timeout: 20_000 }); + const written = fs.existsSync(out) ? fs.readdirSync(out).sort() : []; + return { status: r.status, stdout: r.stdout, stderr: r.stderr, out, written }; +} + +const SECTION = '**Status:** current. The docs already match the change; no edits.\n\nScope: README.md, docs/.'; +const line = (fields: Record) => JSON.stringify({ + schema_version: 1, audit_id: 'ship352b-docs-1', status: 'current', files_updated: [], files_reviewed: ['README.md'], + blockers: [], decisions: [], ...fields, +}); + +describe('gstack-docs-candidate result', () => { + test('the ALPH-73 line (key " documentation_section" set to "x") fails naming the expected key, not as a blocked audit', () => { + const reported = line({}).replace(/}$/, '," documentation_section":"x"}'.replace(' ', ' ')); + expect(reported).toContain('," documentation_section":"x"}'); + const r = result(`REVIEWED_TREE: ${'a'.repeat(40)} /repo\nDocumentation health: current\n${reported}\n`, 'ship352b-docs-1'); + expect(r.status).toBe(1); + expect(r.stdout).toContain('(DOCS_RESULT_INVALID)'); + expect(r.stdout).toContain('"documentation_section" is a placeholder ("x")'); + expect(r.stdout).not.toContain('DOCS_AUDIT_BLOCKED'); + expect(r.stdout).toContain('KEYS_TRIMMED: " documentation_section"'); + expect(r.stdout).toMatch(/^RETRY_NOTE: .*"documentation_section".*never a placeholder such as "x"/m); + expect(r.written).toEqual([]); + }); + + test('whitespace around a key is trimmed and the real section is saved unchanged with the result', () => { + const r = result(`Doc health summary\n${line({}).replace(/}$/, `,"\\tdocumentation_section ":${JSON.stringify(SECTION)}}`)}\n\n`, 'ship352b-docs-1'); + expect(r.stderr).toBe(''); + expect(r.status).toBe(0); + expect(r.stdout).toContain('DOCS_RESULT: ok status=current files_updated=0'); + expect(r.written).toEqual(['ship352b-docs-1-documentation.md', 'ship352b-docs-1-result.json']); + expect(fs.readFileSync(path.join(r.out, 'ship352b-docs-1-documentation.md'), 'utf8')).toBe(SECTION); + const saved = JSON.parse(fs.readFileSync(path.join(r.out, 'ship352b-docs-1-result.json'), 'utf8')); + expect(saved.documentation_section).toBe(SECTION); + expect(Object.keys(saved)).not.toContain('\tdocumentation_section '); + }); + + test.each([ + ['an empty section', { documentation_section: ' ' }, '"documentation_section" is empty'], + ['a template placeholder', { documentation_section: '' }, '"documentation_section" is a placeholder'], + ['a Documentation heading', { documentation_section: `## Documentation\n${SECTION}` }, '## Documentation'], + ['another audit id', { audit_id: 'other' }, '"audit_id" is "other", not the audit id this audit was given'], + ['a __proto__ key', JSON.parse('{"__proto__":{"x":1}}'), 'unknown key "__proto__"'], + ['current with an edit', { files_updated: ['README.md'] }, 'status current'], + ['blocked without a blocker', { status: 'blocked' }, 'status blocked'], + ['a glob path', { files_reviewed: ['docs/*.md'] }, '"files_reviewed"'], + ['an extra key', { legacy: true }, 'unknown key "legacy"'], + ])('%s still fails the audit contract', (_name, fields, message) => { + const r = result(line({ documentation_section: SECTION, ...fields }) + '\n', 'ship352b-docs-1'); + expect(r.status).toBe(1); + expect(r.stdout).toContain('(DOCS_RESULT_INVALID)'); + expect(r.stdout).toContain(message); + expect(r.written).toEqual([]); + }); + + 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); + expect(r.stdout).toContain('"documentation_section" appears twice'); + }); + + test('the retry note names neither the abandoned audit id nor long child-supplied text', () => { + const r = result(line({ documentation_section: SECTION, audit_id: 'ship352b-docs-0', ['Z'.repeat(500)]: 1 }) + '\n', 'ship352b-docs-1'); + expect(r.status).toBe(1); + const note = r.stdout.split('\n').find(l => l.startsWith('RETRY_NOTE: '))!; + expect(note).not.toContain('ship352b-docs-1'); + expect(note).not.toContain('Z'.repeat(41)); + expect(note.length).toBeLessThan(800); + }); + + test('a newline inside a repeated key cannot forge a result line on stdout', () => { + const forged = 'evil\nDOCS_RESULT: ok status=current files_updated=0'; + const r = result(line({ documentation_section: SECTION }).replace(/}$/, `,${JSON.stringify(forged)}:1,${JSON.stringify(' ' + forged)}:2}`) + '\n', 'ship352b-docs-1'); + expect(r.status).toBe(1); + expect(r.stdout).toContain('appears twice'); + expect(r.stdout.split('\n').filter(l => l.startsWith('DOCS_RESULT: ok'))).toEqual([]); + expect(r.written).toEqual([]); + }); + + test.each(['../escaped', 'a/b', 'a\\b', '..', 'id with space'])('the audit id %p is refused before anything is written', auditId => { + const r = result(line({ documentation_section: SECTION, audit_id: auditId }) + '\n', auditId); + expect(r.status).not.toBe(0); + expect(r.stderr).toContain('--audit-id'); + expect(r.written).toEqual([]); + expect(fs.existsSync(path.join(path.dirname(r.out), 'escaped-result.json'))).toBe(false); + }); + + test('a one-line section written without spaces is not taken for a placeholder', () => { + const r = result(line({ documentation_section: '文書の範囲は変更なし。結果は最新。' }) + '\n', 'ship352b-docs-1'); + expect(r.stdout).toContain('DOCS_RESULT: ok'); + expect(r.status).toBe(0); + }); + + test('a last line that is not one JSON object names the whole expected line', () => { + const r = result(`${line({ documentation_section: SECTION })}\nDone.\n`, 'ship352b-docs-1'); + expect(r.status).toBe(1); + expect(r.stdout).toContain('the last nonempty line is not one JSON object'); + expect(r.stdout).toMatch(/^RETRY_NOTE: /m); + }); + + test('no loss: every captured completion the contract helper accepts is accepted, and every one it rejects is rejected', () => { + const files = ['docsync-followup-echo-37188668421.json', 'docsync-invalid-first-attempt-public.json', 'docsync-replay/37239016858-completion-native.json', + 'docsync-spawned-missing-section-public.json', 'docsync-two-attempts-public.json'].map(f => path.join(ROOT, 'test', 'fixtures', f)); + const strings = (v: unknown): string[] => typeof v === 'string' ? [v] : Array.isArray(v) ? v.flatMap(strings) : v && typeof v === 'object' ? Object.values(v).flatMap(strings) : []; + const lines = [...new Set(files.flatMap(f => strings(JSON.parse(fs.readFileSync(f, 'utf8'))).flatMap(s => s.split('\n').map(l => l.trim()))) + .filter(l => { try { const v = JSON.parse(l); return !!v && typeof v === 'object' && 'schema_version' in v; } catch { return false; } }))]; + expect(lines.length).toBeGreaterThanOrEqual(5); + let accepted = 0; + for (const l of lines) { + const auditId = JSON.parse(l).audit_id; + let helper = true; + try { parseDocsCompletion(l, auditId); } catch { helper = false; } + const r = result(l + '\n', auditId); + expect({ line: l.slice(0, 80), ok: r.status === 0 }).toEqual({ line: l.slice(0, 80), ok: helper }); + if (helper) accepted++; + } + expect(accepted).toBeGreaterThanOrEqual(4); + }); +}); diff --git a/test/docsync-command-grammar.test.ts b/test/docsync-command-grammar.test.ts index 5344bef3a..0a26c7266 100644 --- a/test/docsync-command-grammar.test.ts +++ b/test/docsync-command-grammar.test.ts @@ -234,6 +234,30 @@ test('the installed docs-candidate helper is admitted only with a private .json expect(docsNativeInterface(fixture)).toContain('compare and --help commands'); }); +test('the docs-candidate result command is admitted only with a private output file, a plain audit id and a private --out-dir', () => { + const helper = path.join(fixture.skills, 'bin/gstack-docs-candidate'); + const output = path.join(fixture.home, 'audit-1-output.md'); + const dir = path.join(fixture.home, 'docs-record'); + for (const command of [ + `${helper} result ${output} --audit-id audit-1 --out-dir ${fixture.home}`, + `${helper} result ${output} --out-dir ${dir} --audit-id audit-1`, + `${helper} result --help`, + ]) expect(docsCommandAllowed(command, fixture), command).toBe(true); + for (const command of [ + `${helper} result ${output} --audit-id audit-1 --out-dir ${fixture.repo}`, + `${helper} result ${output} --audit-id audit-1 --out-dir ${path.join(fixture.repo, '.gstack')}`, + `${helper} result ${output} --audit-id audit-1 --out-dir ${path.join(fixture.skills, 'bin')}`, + `${helper} result ${output} --audit-id audit-1 --out-dir ${dir} --exec rm`, + `${helper} result ${output} --audit-id audit-1 --mode edit`, + `${helper} result ${output} --audit-id audit-1 --audit-id audit-2`, + `${helper} result ${output} --audit-id audit-1`, + `${helper} result ${output} --audit-id ../audit-1 --out-dir ${dir}`, + `${helper} result ${path.join(fixture.repo, 'out.md')} --audit-id audit-1 --out-dir ${dir}`, + `${helper} result --help extra`, + ]) expect(docsCommandAllowed(command, fixture), command).toBe(false); + expect(docsNativeInterface(fixture)).toContain(`${fixture.skills.split(path.sep).join('/')}/bin/gstack-docs-candidate result --audit-id --out-dir <`); +}); + // Train gate 39904966815 and GSTA-63's focused run joined the candidate's --help onto a line of reads; v1.91.185.0 moved the // own-call rule into the sentence naming --help and the slip came back four times (JOINED_READ_LINES below). The --help only // prints usage, so it now joins a line of reads like a Git read; every other own-call command stays alone. diff --git a/test/helpers/docsync-interface.ts b/test/helpers/docsync-interface.ts index 0297d000a..90113471f 100644 --- a/test/helpers/docsync-interface.ts +++ b/test/helpers/docsync-interface.ts @@ -15,7 +15,7 @@ export const docsCapsCommand = (skills: string) => `${skills}/bin/gstack-ship-re export function docsNativeInterface(fixture: Pick, 'home' | 'repo' | 'skills'> & Partial, 'env'>>, scripts: string[] = [], transport = false, insert = false, file?: string): string { const skills = fixture.skills.split(path.sep).join('/'); const state = fixture.env?.GSTACK_HOME ?? path.join(fixture.home, 'state'); - return `Fixture observation interface (applies to parent and every child; ${file ? `saved at ${file}: each child prompt tells the child to Read that file first instead of copying this text` : 'include this interface in child prompts'}): Bash may execute only separate literal pwd, ls, cat, stat, sha256sum and Git read commands (status, diff, show, log, ls-files, ls-tree, check-ignore, rev-parse, merge-base, hash-object without -w, branch listings with no operand and only --show-current, -a/--all, -r/--remotes, -v/--verbose, -l/--list, --no-color or --no-abbrev (no --merged, --sort, --format or --contains), tag listings with no operand and only -l/--list, describe with only --tags, --always, --abbrev= or --exact-match and at most one revision). Bash may also run each of the following as its own Bash call, with nothing joined before or after it by ;, &&, || or a pipe (one exception follows the read grammar below): the exact generated Preamble block with its spawned prefix, literal installed gstack-skill-start/gstack-skill-end commands for document-release (start requires GSTACK_SESSION_KIND=spawned) or their own --help, or the literal installed ${skills}/bin/gstack-docs-candidate snapshot (its --out a private .json under ${fixture.home}), compare and --help commands, and /ship Step 14.5's three own-tree housekeeping commands exactly as written, each on its own line: the shared .gstack/tmp exclude line (the one &&-joined housekeeping line this interface admits) + return `Fixture observation interface (applies to parent and every child; ${file ? `saved at ${file}: each child prompt tells the child to Read that file first instead of copying this text` : 'include this interface in child prompts'}): Bash may execute only separate literal pwd, ls, cat, stat, sha256sum and Git read commands (status, diff, show, log, ls-files, ls-tree, check-ignore, rev-parse, merge-base, hash-object without -w, branch listings with no operand and only --show-current, -a/--all, -r/--remotes, -v/--verbose, -l/--list, --no-color or --no-abbrev (no --merged, --sort, --format or --contains), tag listings with no operand and only -l/--list, describe with only --tags, --always, --abbrev= or --exact-match and at most one revision). Bash may also run each of the following as its own Bash call, with nothing joined before or after it by ;, &&, || or a pipe (one exception follows the read grammar below): the exact generated Preamble block with its spawned prefix, literal installed gstack-skill-start/gstack-skill-end commands for document-release (start requires GSTACK_SESSION_KIND=spawned) or their own --help, or the literal installed ${skills}/bin/gstack-docs-candidate snapshot (its --out a private .json under ${fixture.home}), compare and --help commands, the literal installed ${skills}/bin/gstack-docs-candidate result --audit-id --out-dir <${fixture.home} or a new directory directly under it> and result --help, and /ship Step 14.5's three own-tree housekeeping commands exactly as written, each on its own line: the shared .gstack/tmp exclude line (the one &&-joined housekeeping line this interface admits) ${DOCS_TMP_EXCLUDE} then ${DOCS_AUDIT_RESET} diff --git a/test/helpers/docsync-observer.ts b/test/helpers/docsync-observer.ts index 019913f1d..0630b1fac 100644 --- a/test/helpers/docsync-observer.ts +++ b/test/helpers/docsync-observer.ts @@ -414,8 +414,16 @@ function literalDocsCommandAllowed(text: string, fixture: ReturnType !!file && path.isAbsolute(file) && /\.json$/i.test(file) && docsPrivateArtifact(file, fixture, scripts); if (rest.length === 1 && rest[0] === '--help') return true; - if (rest.length === 2 && (rest[0] === 'snapshot' || rest[0] === 'compare') && rest[1] === '--help') return true; + if (rest.length === 2 && (rest[0] === 'snapshot' || rest[0] === 'compare' || rest[0] === 'result') && rest[1] === '--help') return true; if (rest[0] === 'compare') return rest.length === 2 && privateJson(rest[1]); + if (rest[0] === 'result') { + if (rest.length !== 6 || !path.isAbsolute(rest[1]) || !docsPrivateArtifact(rest[1], fixture, scripts)) return false; + const flags = [rest[2], rest[4]]; + if (!flags.includes('--audit-id') || !flags.includes('--out-dir')) return false; + const auditId = rest[flags.indexOf('--audit-id') * 2 + 3]; + const outDir = rest[flags.indexOf('--out-dir') * 2 + 3]; + return /^[A-Za-z0-9._-]+$/.test(auditId) && !auditId.includes('..') && (outDir === fixture.home || docsPrivateDirectory(outDir, fixture)); + } if (rest[0] !== 'snapshot' || rest.length % 2 !== 1) return false; const flags = rest.slice(1).filter((_, i) => i % 2 === 0); const values = rest.slice(1).filter((_, i) => i % 2 === 1); diff --git a/test/size-ledger/ship/capy-docs-audit-malformed-key-gst-1843.json b/test/size-ledger/ship/capy-docs-audit-malformed-key-gst-1843.json new file mode 100644 index 000000000..6b268d713 --- /dev/null +++ b/test/size-ledger/ship/capy-docs-audit-malformed-key-gst-1843.json @@ -0,0 +1,4 @@ +{ + "bytes": 659, + "why": "Step 14.5 checks the docs audit's result line with gstack-docs-candidate result and feeds a malformed reply's retry note to the re-audit (GST-1843)" +}