From 530daf02fd2eac721a3ea269626b67789205137d Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 18:49:17 +0000 Subject: [PATCH] fix: pre-landing review fixes - verify requires each output's brief rendered and sent verbatim, reruns judge against the recorded judged.jsonl hash, and flags outputs collected after judge - judge drops self-folds, fold cycles and chains - prose findings parse a leading label or a label heading, not any severity word - releasePaths: manifests count as release metadata only for version-only edits, lockfiles never; TODOS.md stays out of lane-evidence exemptions - push stops when the sealed-output guard did not run - ship-audit reads the test tree once instead of one git grep per file - drop the uncalled --delegate measured mode; doc/help step-number fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 3 +-- bin/gstack-review-dispatch | 2 +- bin/gstack-ship-budget | 11 +++++--- capy/skill/gstack/SKILL.md | 2 +- docs/ship-budget.md | 5 +--- docs/unattended.md | 2 +- lib/review-ledger.ts | 46 ++++++++++++++++++++++++++-------- lib/ship-audit.ts | 4 ++- lib/ship-bin-common.ts | 32 ++++++++++++++++++++--- lib/ship-budget-cli.ts | 6 ++--- lib/ship-budget.ts | 2 +- lib/ship-release.ts | 10 +++++--- test/review-dispatch.test.ts | 29 ++++++++++++++++++++- test/ship-budget-chain.test.ts | 20 ++++++--------- test/ship-release.test.ts | 13 ++++++++++ 15 files changed, 138 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d81d1bef1..d4ce35ac9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,7 @@ ### Added - **`gstack-ship-preflight`** runs Steps 1-3 (branch guard, status, fetch, diff stat, PR and receipt carry, scope, base merge, distribution, policy, runtime lines) and Step 5 (pregate, then the test lanes in parallel) in one call. **`gstack-ship-release`** runs Step 12's version, Steps 15-16's release commit and verification, and Step 17's push. Each prints at most 30 lines; `STOP:` names the decision left to the agent and `NEXT:` the step to run. Under `queue_mode stamp-at-merge` the version step prints `VERSION: skipped (policy: ...)` and writes nothing. - **`ship/DRIVER.md`**, a compact host-neutral driver for a parent agent running `/ship` unattended: one or two calls per phase, and the section each STOP needs. Its audit, review, adversarial and documentation phases run their sections as written, so it never restates or shortens a reviewer's brief. -- **`gstack-ship-budget start --delegate measured`** delegates a segment only when a measured call is 150k or more above the host floor; an unknown charge continues. -- **Small runs keep their segment.** `check --runtime-lines ` continues at the Step 7 and Step 12 boundaries with a `SMALL_RUN:` line when the diff has at most 50 runtime lines and the segment would cost less than a child's fixed startup. +- **Small runs keep their segment.** `check --runtime-lines ` continues at the Step 9 and Step 12 boundaries with a `SMALL_RUN:` line when the diff has at most 50 runtime lines and the segment would cost less than a child's fixed startup. ### Changed - **The Capy meter reads the agent's own thread**, its subagents included, inside the same program as the budget command, never in a call of its own. A handoff child counts from its own `verify-handoff` reading, and `reconcile` charges the `used=` its return prints. diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 641befef5..e7c17de6d 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -108,7 +108,7 @@ function main(): number { const merged = flag('--merged'); const judged = flag('--judged'); if (!merged || !judged) { process.stderr.write('gstack-review-dispatch judge: needs --merged and --judged \n'); return EXIT.usage; } - const r = recordJudgment(ledger, readJsonl(merged), readJsonl(judged), judged); + const r = recordJudgment(ledger, readJsonl(merged), readJsonl(judged), judged, merged); process.stdout.write(r.ok ? `JUDGMENT: ok added=${r.added.length} promoted=${r.promoted.length} folded=${r.folded.length}\n` : `JUDGMENT_REJECTED: dropped=${JSON.stringify(r.dropped)} downgraded=${JSON.stringify(r.downgraded)} unexplained=${JSON.stringify(r.unexplained)} invalid=${JSON.stringify(r.invalid)} (lead judgment may add or promote a finding, never drop or downgrade one)\n`); return r.ok ? EXIT.ok : EXIT.fail; diff --git a/bin/gstack-ship-budget b/bin/gstack-ship-budget index fb2605f78..83831d97f 100755 --- a/bin/gstack-ship-budget +++ b/bin/gstack-ship-budget @@ -8,10 +8,12 @@ * takes --credits beside it. * * start --record [--token-cap ] [--mode admit|observe] [--meter capy|claude|estimate] [--reading ] [--credits ] - * [--prior-reading ] [--session |--nonce ] [--reserve ] [--delegate auto|never|measured] [--calibration ] [--ledger ] + * [--prior-reading ] [--session |--nonce ] [--reserve ] [--delegate auto|never] [--calibration ] [--ledger ] * note --run|--record --step [--label ] --reading [--credits ] [--dispatched-at [--dispatched-credits ] --readers ] - * check --run |--record --step --next [,...]> [--reading ] [--prior-reading ] [--returned [,...]] [--runtime-lines ] - * settle --run|--record --reservation [--unknown] [--reading ] release --run|--record --reservation + * check --run |--record --step --next [,...]> [--reading ] [--credits ] [--prior-reading ] + * [--returned [,...]] [--runtime-lines ] [--dispatched-at --readers ] + * settle --run|--record --reservation [--unknown] [--reading ] [--credits ] [--dispatched-at --readers ] + * release --run|--record --reservation * handoff --run|--record --from --to --base origin/ [--reviewed-tree ] [--trail --trail-run ] * [--sub-cap ] [--reading ] [--child-session ] * launch --in (Claude Code: background headless child with explicit permissions) @@ -38,7 +40,8 @@ const commands: Record Outcome> = { reconcile: () => cmdReconcile(rest, ctx), resume: () => cmdResume(rest, ctx), close: () => cmdClose(rest, ctx), }; if (!command || command === '--help' || command === '-h' || !commands[command]) { - const text = (await Bun.file(import.meta.path).text()).split('\n').slice(2, 27).map(l => l.replace(/^ \* ?/, '')).join('\n'); + const lines = (await Bun.file(import.meta.path).text()).split('\n'); + const text = lines.slice(2, lines.indexOf(' */')).map(l => l.replace(/^ \* ?/, '')).join('\n'); (command && command !== '--help' && command !== '-h' ? console.error : console.log)(text); process.exit(command && command !== '--help' && command !== '-h' ? 2 : 0); } diff --git a/capy/skill/gstack/SKILL.md b/capy/skill/gstack/SKILL.md index 3a2348712..44cbfa774 100644 --- a/capy/skill/gstack/SKILL.md +++ b/capy/skill/gstack/SKILL.md @@ -38,7 +38,7 @@ If no specific skill was requested, use the router's task-to-skill mapping. Read For `/autoplan` driven from Capy, read `autoplan/DRIVER.md` in the checkout (`~/.local/share/gstack/autoplan/DRIVER.md`): the compact phase loop, artifact contract and gate protocol, without the interactive preamble. A checkout without that file predates it; follow its `autoplan/SKILL.md` instead. Run every command with the `GSTACK_LAUNCH` assignments and read outputs as files, not prose: the run directory holds `run.json`, `decisions.jsonl`, `findings.jsonl`, `tasks.jsonl`, `timing.json`, `plan.md` and `review-record.md`, and the run ends with `GSTACK_RESULT: skill=autoplan status=gate_pending run=`. `~/.local/share/gstack/bin/gstack-artifact validate /run.json` is the completion check. The gate is answered through `gstack-autoplan answer` (or `gstack-gate parse`) with the printed `gate_rev`; an unattended run never approves its own gate. `docs/unattended.md` in the checkout is the contract. -For `/ship` driven from Capy, read `ship/DRIVER.md` in the checkout (`~/.local/share/gstack/ship/DRIVER.md`): the phase loop at one or two calls per phase, and the section each STOP needs. A checkout without that file predates it; follow its `ship/SKILL.md` instead. It starts the budget with `--delegate measured`, and its review phases run their sections as written. +For `/ship` driven from Capy, read `ship/DRIVER.md` in the checkout (`~/.local/share/gstack/ship/DRIVER.md`): the phase loop at one or two calls per phase, and the section each STOP needs. A checkout without that file predates it; follow its `ship/SKILL.md` instead. It starts the budget with `--mode observe`, and its review phases run their sections as written. Run directories and scratch on Capy live under `~/.capy/work/` (`--out ~/.capy/work//autoplan`), never `/tmp`: Capy wipes `/tmp` when a machine sleeps, and a vanished run cannot be resumed, answered or exported. The same holds for any file a later turn reads. diff --git a/docs/ship-budget.md b/docs/ship-budget.md index e3e81b721..928132644 100644 --- a/docs/ship-budget.md +++ b/docs/ship-budget.md @@ -191,10 +191,7 @@ line prints `call`, `call_floor` and `above_floor`, so each verdict can be check grandchild's. `--delegate never` turns delegate into stop for a parent on a host without children; a child -continues with a `CONTEXT_HIGH:` note instead. `--delegate measured` (what `ship/DRIVER.md` started -with before observe mode) delegates only when a measured call is 150k or more above the floor at a segment boundary; an -unknown charge continues there. A parent driving /ship from the driver carries no skill text a fresh -child would shed, so a child pays its fixed cost only when the parent's own context grew. +continues with a `CONTEXT_HIGH:` note instead. **Small runs keep their segment.** `check --runtime-lines ` (runtime_lines from `gstack-review-plan --json`, which `gstack-ship-preflight` prints) continues at the Step 9 and diff --git a/docs/unattended.md b/docs/unattended.md index 905de39cc..c047ce19d 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -192,7 +192,7 @@ never on polling wakes. A parent bounds a /ship run with `--token-cap ` (or `GSTACK_SHIP_TOKEN_CAP`). It is an admission cap: /ship meters usage, reserves every step and dispatch batch before it starts, and stops `status=incomplete` with a `RECOVER:` line -instead of starting work that would cross the cap. Long runs hand Steps 7–11.5 +instead of starting work that would cross the cap. Long runs hand Steps 9–11.5 and 12–22 to fresh-context children. The flow, the BUDGET line and the result codes are in [ship-budget.md](ship-budget.md). diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 45c06a9e9..a8d36f477 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -44,24 +44,34 @@ export function rank(label: string): number | null { const r = RANKS[label.trim().replace(/^\[|\]$/g, '').toLowerCase()]; return r === undefined ? null : r; } -const PROSE_LABEL = /(?:^|[\s[(*|:-])(CRITICAL|INFORMATIONAL|Critical|High|Medium|Low|P[0-3]|FIXABLE|INVESTIGATE)(?=$|[\s\]):*|,.-])/; +const LABELS = 'CRITICAL|INFORMATIONAL|Critical|High|Medium|Low|P[0-3]|FIXABLE|INVESTIGATE'; +/** A finding line opens with its label: "- **High**: …", "1. [P1] …", "FIXABLE: …". A label later in a sentence ("no CRITICAL issues remain") is not one. */ +const LEAD_LABEL = new RegExp(`^(?:[-*+]|\\d+[.)])?\\s*[*_[(]*(${LABELS})[*_\\])]*(?=\\s*[:\\-—–]|\\s+\\S)`); +/** A heading that is only a label ("## High", "**Critical findings**") gives its label to the list items under it. */ +const LABEL_HEADING = new RegExp(`^(?:#{1,6}\\s*|\\*\\*)(${LABELS})(?:\\s+(?:findings?|severity|issues?))?\\s*:?\\**\\s*$`); +const LIST_ITEM = /^(?:[-*+]|\d+[.)])\s+\S/; export interface MergedLine { id: string; reader: string; label: string; raw: string; output_sha256: string } -/** One reader's findings as written: a JSON object per line (severity kept as written), else each prose line carrying a severity or FIXABLE/INVESTIGATE label. */ +/** One reader's findings as written: a JSON object per line (severity kept as written); else each prose line that opens with a severity or FIXABLE/INVESTIGATE label, and each list item under a label-only heading. */ export function parseFindings(reader: string, output: string): Array<{ label: string; raw: string }> { const out: Array<{ label: string; raw: string }> = []; + let section: string | null = null; for (const line of output.replace(/\r\n/g, '\n').split('\n')) { const t = line.trim(); if (!t || /^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t)) continue; + const heading = LABEL_HEADING.exec(t); + if (heading) { section = heading[1]!; continue; } + if (/^#{1,6}\s/.test(t)) { section = null; continue; } if (t.startsWith('{')) { try { const o = JSON.parse(t) as Record; if (typeof o.severity === 'string') { out.push({ label: o.severity, raw: t }); continue; } } catch { /* prose that starts with a brace */ } } - const m = PROSE_LABEL.exec(t); + const m = LEAD_LABEL.exec(t); if (m) out.push({ label: m[1]!, raw: t }); + else if (section && LIST_ITEM.test(t)) out.push({ label: section, raw: t }); } return out; } @@ -93,7 +103,8 @@ export function judge(merged: MergedLine[], judged: JudgedLine[]): Judgment { const j = byId.get(m.id); if (!j) { res.dropped.push(m.id); continue; } const target = j.duplicate_of ? byId.get(j.duplicate_of) : j; - if (!target) { res.dropped.push(m.id); continue; } + // A duplicate folds into a different line that stands on its own; a self-fold, a cycle or a chain keeps no survivor. + if (!target || (j.duplicate_of && (j.duplicate_of === j.id || target.duplicate_of))) { res.dropped.push(m.id); continue; } const from = rank(m.label) ?? 0; const to = rank(target.severity); if (to === null) continue; @@ -107,7 +118,7 @@ export function judge(merged: MergedLine[], judged: JudgedLine[]): Judgment { } /** judge, logged: one judgment event per addition and promotion, then the verdict. */ -export function recordJudgment(ledger: string, merged: MergedLine[], judged: JudgedLine[], judgedFile: string): Judgment { +export function recordJudgment(ledger: string, merged: MergedLine[], judged: JudgedLine[], judgedFile: string, mergedFile: string): Judgment { const res = judge(merged, judged); const byId = new Map(judged.map(j => [j.id, j])); const labels = new Map(merged.map(m => [m.id, m.label])); @@ -115,7 +126,7 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud for (const id of res.added) appendEvent(ledger, { event: 'judgment', kind: 'added', id, to: byId.get(id)!.severity, reason: byId.get(id)!.reason }); for (const id of res.promoted) appendEvent(ledger, { event: 'judgment', kind: 'promoted', id, from: labels.get(id), to: byId.get(id)!.severity, reason: byId.get(id)!.reason }); } - appendEvent(ledger, { event: 'judge', ok: res.ok, judged: judgedFile, dropped: res.dropped, downgraded: res.downgraded, unexplained: res.unexplained, invalid: res.invalid, added: res.added.length, promoted: res.promoted.length, folded: res.folded.length }); + appendEvent(ledger, { event: 'judge', ok: res.ok, judged: judgedFile, judged_sha256: promptHash(fs.readFileSync(judgedFile, 'utf8')), merged: mergedFile, merged_ids: merged.map(m => m.id), dropped: res.dropped, downgraded: res.downgraded, unexplained: res.unexplained, invalid: res.invalid, added: res.added.length, promoted: res.promoted.length, folded: res.folded.length }); return res; } @@ -140,6 +151,10 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri const reader = String(o.reader); let status = 'MATCH'; const notes: string[] = []; + const rendered = lastRendered(events, reader); + const sent = events.filter(e => e.event === 'sent' && e.reader === reader); + if (!rendered) { status = 'MISMATCH'; notes.push('brief_not_rendered'); } + else if (!sent.length || sent.some(s => s.sha256 !== rendered.sha256)) { status = 'MISMATCH'; notes.push('brief_not_sent_verbatim'); } if (typeof o.file === 'string' && fs.existsSync(o.file) && promptHash(fs.readFileSync(o.file, 'utf8')) !== o.sha256) { status = 'MISMATCH'; notes.push('output_file_changed'); } const ids = Array.isArray(o.ids) ? (o.ids as string[]) : []; let lines: MergedLine[] = []; @@ -148,10 +163,21 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri if (lines.some(l => l.output_sha256 !== o.sha256)) { status = 'MISMATCH'; notes.push('merged_line_from_other_output'); } rows.push({ kind: 'OUTPUT', reader, status, detail: `sha=${short(o.sha256)} findings=${ids.length}${notes.length ? ' ' + notes.join(' ') : ''}` }); } - const verdicts = events.filter(e => e.event === 'judge'); - if (verdicts.length) { - const v = verdicts[verdicts.length - 1]!; - rows.push({ kind: 'JUDGE', reader: '-', status: v.ok ? 'OK' : 'REJECTED', detail: `dropped=${JSON.stringify(v.dropped)} downgraded=${JSON.stringify(v.downgraded)} added=${v.added} promoted=${v.promoted} folded=${v.folded}` }); + const lastJudge = events.map(e => e.event).lastIndexOf('judge'); + if (lastJudge >= 0) { + const v = events[lastJudge]!; + const notes: string[] = []; + const late = events.slice(lastJudge + 1).filter(e => e.event === 'output').map(e => String(e.reader)); + if (late.length) notes.push(`output_after_judge=${late.join(',')}`); + let rerun: Judgment | null = null; + try { + const judgedText = fs.readFileSync(String(v.judged), 'utf8'); + if (promptHash(judgedText) !== v.judged_sha256) notes.push('judged_changed_since_judge'); + rerun = judge(readJsonl(String(v.merged)), readJsonl(String(v.judged))); + } catch { notes.push('judged_or_merged_unreadable'); } + if (rerun && !rerun.ok) notes.push(`rerun_rejects dropped=${JSON.stringify(rerun.dropped)} downgraded=${JSON.stringify(rerun.downgraded)}`); + const status = !v.ok ? 'REJECTED' : notes.length ? 'MISMATCH' : 'OK'; + rows.push({ kind: 'JUDGE', reader: '-', status, detail: `dropped=${JSON.stringify(v.dropped)} downgraded=${JSON.stringify(v.downgraded)} added=${v.added} promoted=${v.promoted} folded=${v.folded}${notes.length ? ' ' + notes.join(' ') : ''}` }); } else if (events.some(e => e.event === 'output')) rows.push({ kind: 'JUDGE', reader: '-', status: 'MISSING', detail: 'outputs were collected but no judged set was checked' }); const clean = readers.length > 0 && rows.every(r => r.status === 'MATCH' || r.status === 'OK'); return { rows, clean, briefs: readers.length }; diff --git a/lib/ship-audit.ts b/lib/ship-audit.ts index b0da3ba82..7b44095ac 100644 --- a/lib/ship-audit.ts +++ b/lib/ship-audit.ts @@ -80,10 +80,12 @@ export function audit(cwd: string, base: string, out: string): Report { const tests = files.filter(p => TEST_PATH.test(p)).map(p => ({ p, text: readOr(path.join(cwd, p)) })); const runtime = files.filter(p => !TEST_PATH.test(p) && !DATA_PATH.test(p) && fs.existsSync(path.join(cwd, p)) && classifyPath(p, cwd) === 'code'); + // One read of the tracked test tree for every changed file, instead of one full-tree git grep per file. + const testTree = runtime.length ? git(cwd, ['ls-files', '-z', '--', ...TEST_SPECS]).stdout.split('\0').filter(Boolean).map(f => readOr(path.join(cwd, f))) : []; const map = runtime.map(p => ({ p, changed: tests.filter(t => t.p.includes(stem(p)) || t.text.includes(stem(p))).length, - referencing: git(cwd, ['grep', '-l', '-F', '-e', stem(p), '--', ...TEST_SPECS]).stdout.split('\n').filter(Boolean).length, + referencing: testTree.filter(text => text.includes(stem(p))).length, })); const mapFile = path.join(out, 'coverage.txt'); fs.writeFileSync(mapFile, map.map(m => `${m.p} changed_tests=${m.changed} referencing_tests=${m.referencing}\n`).join(''), { mode: 0o600 }); diff --git a/lib/ship-bin-common.ts b/lib/ship-bin-common.ts index c9a13ff4a..03fb9fbcd 100644 --- a/lib/ship-bin-common.ts +++ b/lib/ship-bin-common.ts @@ -6,6 +6,7 @@ * assigns to the agent. */ import { spawn, spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; import * as path from 'node:path'; import { BIN_DIR, runBashSync, runScriptSync, scriptInvocation } from './script-invocation'; @@ -67,13 +68,38 @@ export function shipPolicy(cwd: string, base: string): Record | nul try { return JSON.parse(r.stdout).policy ?? null; } catch { return null; } } -/** Release metadata a version stamp and Steps 13-14 write: the release commit's paths, never a behavior change. */ -export function releasePaths(policy: Record | null): string[] { +/** Manifests a version stamp writes: release metadata only while the edit changes nothing but the version. */ +export const VERSIONED_MANIFESTS = ['package.json', 'package-lock.json', 'npm-shrinkwrap.json']; + +/** True when `p` in the working tree differs from `p` at `ref` (a commit or tree) in its version fields alone; a new, deleted or unparseable file never qualifies. */ +export function versionOnlyEdit(cwd: string, ref: string, p: string): boolean { + const before = git(cwd, ['show', `${ref}:${p}`]); + if (before.status !== 0) return false; + let now: string; + try { now = fs.readFileSync(path.join(cwd, p), 'utf8'); } catch { return false; } + const strip = (s: string) => { + const o = JSON.parse(s); + delete o.version; + if (o.packages?.['']) delete o.packages[''].version; + return JSON.stringify(o); + }; + try { return strip(before.stdout) === strip(now); } catch { return false; } +} + +/** + * Release metadata a version stamp and Steps 13-14 write, compared against `ref`: the release commit's paths, + * never a behavior change. A manifest counts only while its edit is version-only (a dependency, script or + * lockfile change is a build input); bun.lock never counts. + */ +export function releasePaths(policy: Record | null, cwd: string, ref: string): string[] { const list = (v: unknown) => (Array.isArray(v) ? v.filter((s): s is string => typeof s === 'string') : []); - return [...new Set(['VERSION', 'CHANGELOG.md', 'TODOS.md', 'package.json', 'package-lock.json', 'bun.lock', 'agents-digest/gstack-AGENTS.md', + return [...new Set(['VERSION', 'CHANGELOG.md', 'TODOS.md', 'agents-digest/gstack-AGENTS.md', ...VERSIONED_MANIFESTS.filter(p => versionOnlyEdit(cwd, ref, p)), ...(typeof policy?.changelog === 'string' ? [policy.changelog] : []), ...list(policy?.release_outputs), ...list(policy?.stamp_paths)])]; } +/** The paths a lane receipt may ignore: release metadata except TODOS.md, whose Step 14 edit reruns the lanes (ship/SKILL.md Step 16). */ +export const evidenceExempt = (release: readonly string[]) => release.filter(p => p !== 'TODOS.md'); + export const short = (list: readonly string[], n = 6) => list.slice(0, n).join(',') + (list.length > n ? `,+${list.length - n} more` : ''); export const firstLine = (s: string) => s.trim().split('\n').filter(Boolean).at(-1)?.slice(0, 240) ?? ''; diff --git a/lib/ship-budget-cli.ts b/lib/ship-budget-cli.ts index 199c2c60f..afab469e9 100644 --- a/lib/ship-budget-cli.ts +++ b/lib/ship-budget-cli.ts @@ -130,7 +130,7 @@ export function writeReturnFile(file: string, lines: string[]): void { /** * Admission and the delegate rule for one boundary; reserves the admitted work * under a cap (a dispatch batch goes to admitBatch). The rule compares the call's context above the host floor (an - * unknown charge delegates at a segment boundary, never at start; under --delegate measured it continues). A light diff + * unknown charge delegates at a segment boundary, never at start). A light diff * whose segment costs less than a child keeps it (keepsSmallRun). A child * never stops on it: after a completed step it hands its remaining steps to a * grandchild, and before one, at the maximum depth or past its segment it @@ -150,7 +150,7 @@ export function decide(l: Ledger, step: string, next: Next, call: number | null, const boundary = (startThreshold ? next.step === '1' : next.step in SEGMENTS && next.step !== '1'); const to = l.parent ? l.parent.to : SEGMENTS[next.step]!; const small = boundary && !startThreshold ? keepsSmallRun(loadCalibration(l.calibration), { runtimeLines, from: next.step, to, call }) : null; - const wants = boundary && !small && (above === null ? !startThreshold && l.delegate !== 'measured' && !l.parent_metered : above >= threshold); + const wants = boundary && !small && (above === null ? !startThreshold && !l.parent_metered : above >= threshold); const depth = l.depth ?? (l.parent ? 1 : 0); const hold = !wants || !l.parent ? undefined : !l.steps_done?.length ? 'no step completed since verify-handoff' @@ -244,7 +244,7 @@ export function cmdStart(argv: string[], ctx: Ctx): Outcome { const cap = rawCap === undefined || rawCap === 'none' ? null : parseTokens(rawCap, '--token-cap'); const callFloor = tokensOpt(o, 'call-floor'); const delegate = str(o, 'delegate') ?? 'auto'; - if (delegate !== 'auto' && delegate !== 'never' && delegate !== 'measured') throw usageError(`--delegate '${delegate}' is not auto, never or measured`); + if (delegate !== 'auto' && delegate !== 'never') throw usageError(`--delegate '${delegate}' is not auto or never`); const existing = parseRecordBlock(record); if (existing?.ledger && fs.existsSync(existing.ledger) && !readLedger(existing.ledger).closed_at) { const l = readLedger(existing.ledger); diff --git a/lib/ship-budget.ts b/lib/ship-budget.ts index 9eac4fe0a..11bcfcc12 100644 --- a/lib/ship-budget.ts +++ b/lib/ship-budget.ts @@ -73,7 +73,7 @@ export interface Handoff { } export interface Ledger { schema_version: 1; kind: 'gstack-ship-budget'; run: string; dir: string; file: string; record?: string; created_at: string; - cap: number | null; tail: number; stop_overhead: number; delegate: 'auto' | 'never' | 'measured'; role: 'parent' | 'child'; + cap: number | null; tail: number; stop_overhead: number; delegate: 'auto' | 'never'; role: 'parent' | 'child'; parent?: { run: string; ledger: string; handoff_sha: string; from: string; to: string; return_file: string }; meter: { contract: MeterContract; epoch: number; baseline: number; last: number | null; last_at?: string; sessions?: string[]; config_dir?: string }; epochs: Array<{ epoch: number; contract: MeterContract; baseline: number; last: number | null; used: number }>; diff --git a/lib/ship-release.ts b/lib/ship-release.ts index 3c65d4323..3cfc9c19d 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -14,7 +14,7 @@ import * as path from 'node:path'; import { classifyPath } from './review-depth'; import { resolveStateRoot } from './state-root'; import { bumpVersion, cmpVersion, fmtVersion, parseVersion, versionWidth, type Bump } from './version-source'; -import { bin, firstLine, flagReader, gh, git, hostedOrigin, laneLine, lanes, releasePaths, Report, runLanes, sh, shipPolicy, short } from './ship-bin-common'; +import { bin, evidenceExempt, firstLine, flagReader, gh, git, hostedOrigin, laneLine, lanes, releasePaths, Report, runLanes, sh, shipPolicy, short } from './ship-bin-common'; export const RELEASE_HELP = `gstack-ship-release — /ship Steps 12 and 15-17 (ship/SKILL.md) @@ -149,8 +149,9 @@ export async function finish(argv: readonly string[], cwd: string): Promise bin(cwd, 'gstack-evidence', ['check', '--label', l.label, '--expect-cmd', l.command, '--max-age', '24', '--allow-paths', allow], 60_000).stdout; for (const l of list) { let e = check(l); @@ -210,6 +211,7 @@ export function push(argv: readonly string[], cwd: string): Report { const sealed = bin(cwd, 'gstack-pregate', ['sealed'], 120_000); r.add(`SEALED: ${sealed.status === 0 ? 'clear' : sealed.status === 1 ? 'refused' : `not checked (exit ${sealed.status})`}`); if (sealed.status === 1) return r.stop(`sealed output would be published: ${firstLine(sealed.stderr)}; remove it from the commits, then repeat Step 16`); + if (sealed.status !== 0) return r.stop(`the sealed-output guard did not run (exit ${sealed.status}: ${firstLine(sealed.stderr || sealed.stdout)}); repair it, then rerun push`); const local = git(cwd, ['rev-parse', 'HEAD']).stdout.trim(); const remote = git(cwd, ['ls-remote', '--heads', 'origin', `refs/heads/${branch}`], 120_000); if (remote.status !== 0) return r.stop(`cannot verify the remote branch (${firstLine(remote.stderr)}): restore access before pushing`); diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 07959b848..e31d13310 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -107,6 +107,9 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change const spec = p.w('spec.out', '{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"off by one","specialist":"testing"}\n{"severity":"INFORMATIONAL","path":"a.ts","line":9,"category":"style","summary":"name","specialist":"testing"}\n'); const outside = p.w('outside.out', 'Findings:\n1. **High**: retry loop never backs off (q.ts:78)\n2. Low - log line is noisy\nRecommendation: Fix the retry because it DoSes the pool\n'); const c1 = run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', spec, '--merged', p.merged]); + const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 10_000 }).stdout.trim(); + const ov = field(run(['render', '--role', 'outside-voice', '--out', p.out, '--diff-base', head]).stdout, 'file'); + run(['sent', '--ledger', p.ledger, '--role', 'outside-voice', '--sent', ov]); const c2 = run(['collect', '--ledger', p.ledger, '--role', 'outside-voice', '--output', outside, '--merged', p.merged]); expect([c1.stdout.includes('findings=2'), c2.stdout.includes('findings=2')]).toEqual([true, true]); const merged = fs.readFileSync(p.merged, 'utf8').trim().split('\n').map(l => JSON.parse(l) as MergedLine); @@ -129,14 +132,38 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect(ledger).toContain('"kind":"promoted","id":"outside-voice#2","from":"Low","to":"INFORMATIONAL","reason":"it hides a real error line"'); expect(ledger).toContain('"kind":"added","id":"lead#1"'); const v = run(['verify', '--ledger', p.ledger]); - expect([v.status, v.stdout.includes('OUTPUT outside-voice MATCH'), v.stdout.includes('JUDGE - OK'), v.stdout.includes('VERIFY: clean briefs=1 outputs=2')]).toEqual([0, true, true, true]); + expect([v.status, v.stdout.includes('OUTPUT outside-voice MATCH'), v.stdout.includes('JUDGE - OK'), v.stdout.includes('VERIFY: clean briefs=2 outputs=2')]).toEqual([0, true, true, true]); fs.writeFileSync(p.merged, fs.readFileSync(p.merged, 'utf8').replace('"label":"Low"', '"label":"Low","x":1').replace(/"output_sha256":"[0-9a-f]+"}\n$/, '"output_sha256":"' + '0'.repeat(64) + '"}\n')); const tampered = run(['verify', '--ledger', p.ledger]); expect([tampered.status, tampered.stdout.includes('OUTPUT outside-voice MISMATCH')]).toEqual([1, true]); }); + // Value: protects=verify is clean only when every collected output came from a brief rendered and sent verbatim, nothing was collected after the judgment, and the judged set still passes as recorded; a fold keeps a standalone survivor; fails_when=outputs from hand-written briefs, a finding collected after judge, a judged.jsonl edited after judge, a self-fold or a fold cycle still read clean (/ship #3648 pass-1 Red Team); + // why_new=the first gate trusted the last verdict and any duplicate_of; seam=verify and judge; no_claim=the host's dispatch + test('verify is dirty for an unrendered reader, an output after judge and a judged set edited after judge; self-folds and fold cycles are drops', () => { + const p = pass(); + run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]); + const spec = p.w('spec.out', '{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"x","specialist":"testing"}\n'); + run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', spec, '--merged', p.merged]); + const judged = p.w('judged.jsonl', JSON.stringify({ id: 'specialist/testing#1', severity: 'CRITICAL' }) + '\n'); + expect(run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', judged]).status).toBe(0); + expect(run(['verify', '--ledger', p.ledger]).stdout).toContain('VERIFY: clean'); + fs.appendFileSync(judged, JSON.stringify({ id: 'lead#1', severity: 'Low', reason: 'late' }) + '\n'); + expect(run(['verify', '--ledger', p.ledger]).stdout).toContain('judged_changed_since_judge'); + const hand = p.w('hand.out', '{"severity":"CRITICAL","path":"b.ts","line":1,"category":"x","summary":"y","specialist":"red-team"}\n'); + run(['collect', '--ledger', p.ledger, '--role', 'red-team', '--output', hand, '--merged', p.merged]); + const v = run(['verify', '--ledger', p.ledger]); + expect([v.status, v.stdout.includes('OUTPUT red-team MISMATCH'), v.stdout.includes('brief_not_rendered'), v.stdout.includes('output_after_judge=red-team')]).toEqual([1, true, true, true]); + const m: MergedLine[] = [{ id: 'a#1', reader: 'a', label: 'CRITICAL', raw: 'x', output_sha256: 'h' }, { id: 'a#2', reader: 'a', label: 'CRITICAL', raw: 'y', output_sha256: 'h' }]; + expect(judge(m, [{ id: 'a#1', severity: 'CRITICAL', duplicate_of: 'a#1' }, { id: 'a#2', severity: 'CRITICAL' }]).dropped).toEqual(['a#1']); + expect(judge(m, [{ id: 'a#1', severity: 'CRITICAL', duplicate_of: 'a#2' }, { id: 'a#2', severity: 'CRITICAL', duplicate_of: 'a#1' }]).dropped).toEqual(['a#1', 'a#2']); + expect(judge(m, [{ id: 'a#1', severity: 'CRITICAL', duplicate_of: 'a#2' }, { id: 'a#2', severity: 'CRITICAL' }]).ok).toBe(true); + }); + test('judge and parseFindings as functions: an unknown severity is invalid, NO FINDINGS and the Recommendation line are not findings', () => { expect(parseFindings('r', 'NO FINDINGS\n')).toEqual([]); + expect(parseFindings('r', 'Summary: no CRITICAL issues remain.\n')).toEqual([]); + expect(parseFindings('r', '## High\n1. The auth check skips webhooks (a.ts:4)\n- second one\n## Notes\n- not a finding\n').map(f => [f.label, f.raw])).toEqual([['High', '1. The auth check skips webhooks (a.ts:4)'], ['High', '- second one']]); expect(parseFindings('r', 'FIXABLE: race in a.ts:3\nINVESTIGATE: odd lock\nRecommendation: Fix the race because x\n').map(f => f.label)).toEqual(['FIXABLE', 'INVESTIGATE']); const m: MergedLine[] = [{ id: 'r#1', reader: 'r', label: 'FIXABLE', raw: 'x', output_sha256: 'h' }]; expect(judge(m, [{ id: 'r#1', severity: 'severe' }]).invalid).toEqual(['r#1']); diff --git a/test/ship-budget-chain.test.ts b/test/ship-budget-chain.test.ts index 0d9cab3c9..3b4f8c64b 100644 --- a/test/ship-budget-chain.test.ts +++ b/test/ship-budget-chain.test.ts @@ -73,20 +73,14 @@ describe('per-call thresholds above the host floor', () => { expect(h.field(unknown.out, 'BUDGET', 'verdict')).toBe('delegate'); }); - test('--delegate measured (ship/DRIVER.md): an unknown charge continues at a segment boundary; 150k above the floor still delegates', () => { + // Value: protects=start refuses a delegate mode it does not implement, the removed measured mode included; fails_when=an unknown --delegate starts a ledger that silently runs as auto; + // why_new=/ship #3648 pass-1 simplification finding removed the uncalled measured mode; seam=gstack-ship-budget start; no_claim=the auto and never rules + test('--delegate takes only auto or never', () => { const h = fresh(); - const s = start(h, ['--token-cap', '80M', '--reading', '1000000', '--delegate', 'measured']); - const unknown = check(h, s.run, '8', '9', ['--reading', '1100000']); - expect(unknown.code, unknown.out + unknown.err).toBe(0); - expect(h.field(unknown.out, 'BUDGET', 'above_floor')).toBe('unknown'); - expect(h.field(unknown.out, 'BUDGET', 'verdict')).toBe('continue'); - expect(unknown.out).not.toContain('DELEGATE:'); - const high = check(h, s.run, '11.5', '12', ['--reading', '2000000', '--prior-reading', '1700000']); - expect(h.field(high.out, 'BUDGET', 'above_floor')).toBe('150000'); - expect(h.field(high.out, 'BUDGET', 'verdict')).toBe('delegate'); - const bad = h.sb(['start', '--record', h.record(), '--token-cap', '60M', '--reading', '0', '--delegate', 'sometimes']); - expect(bad.code).toBe(2); - expect(bad.err).toContain("--delegate 'sometimes' is not auto, never or measured"); + for (const mode of ['measured', 'sometimes']) { + const bad = h.sb(['start', '--record', h.record(), '--token-cap', '60M', '--reading', '0', '--delegate', mode]); + expect([bad.code, bad.err.includes(`--delegate '${mode}' is not auto or never`)]).toEqual([2, true]); + } }); test('--call-floor overrides the calibrated floor and is recorded in the ledger', () => { diff --git a/test/ship-release.test.ts b/test/ship-release.test.ts index 6a77e38f3..7befdd28d 100644 --- a/test/ship-release.test.ts +++ b/test/ship-release.test.ts @@ -113,6 +113,19 @@ describe('finish (Steps 15-16)', () => { expect(h.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', reviewed3, '--docs-post', post3]).stdout).toContain("STOP: no test lanes: require Step 5's explicit untested-scope approval"); expect(h.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', reviewed3]).status).toBe(2); }); + + // Value: protects=a dependency or script edit to package.json, and any lockfile edit, after review takes the re-review route; a version-only bump stays release metadata; fails_when=finish treats package.json or bun.lock as metadata whatever changed in them (/ship #3648 pass-1 security, CRITICAL); + // why_new=releasePaths exempted the manifests and lockfiles by name; seam=gstack-wtree, git; no_claim=the re-review itself + test('a script or dependency edit to package.json, or a bun.lock edit, after review stops finish', () => { + for (const [file, text] of [['package.json', '{"name":"fx","version":"1.0.0","scripts":{"test":"exit 0"}}\n'], ['bun.lock', '{"lockfileVersion":1}\n']]) { + const f = fx(); + const reviewed = wtree(f); + const post = snapshot(f); + f.write({ [file]: text }); f.commit(`late ${file}`); + const r = f.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', reviewed, '--docs-post', post, '--lane', LANE]); + expect([r.status, r.stdout.includes(`STOP: behavior, tests or build inputs changed since review (${file})`)]).toEqual([3, true]); + } + }); }); describe('push (Step 17)', () => {