From 641d62291c510954d30d967b2c6fc0c785a6d37f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 5 Nov 2026 08:17:14 -0800 Subject: [PATCH] fix(ship): Step 14.5 docs audit defaults to the owner's agent_model (GST-1439) (#4038) * fix(ship): Step 14.5 docs audit defaults to the owner's agent_model (GST-1439) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): keep the driver and Capy skill inside their size budgets; cover host and bad agent_model (GST-1439) 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-review-dispatch | 8 +++-- capy/skill/gstack/SKILL.md | 2 +- .../capy-gst-1439-docs-audit-agent-model.md | 1 + docs/model-policy.md | 5 +++ lib/review-pass.ts | 17 +++++++++ ship/DRIVER.md | 4 +-- ship/DRIVER.md.tmpl | 4 +-- test/review-pass.test.ts | 35 +++++++++++++++++++ test/ship-driver.test.ts | 6 ++-- 9 files changed, 71 insertions(+), 11 deletions(-) create mode 100644 changelog.d/capy-gst-1439-docs-audit-agent-model.md diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 3232fa4ba..8d0e00551 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -24,7 +24,7 @@ import * as path from 'node:path'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { BRIEF_ROLES, BriefError, briefFile, checkReviewedTree, checkoutRoot, checkoutTree, promptHash, renderBrief, sectionTexts, sliceDiff, sliceNote, withWorkspace, type BriefRole, type Slots, type TreeCheck } from '../lib/review-brief'; import { cleanMarkers, collect, fixableRaises, mayBeUnavailable, parseFindings, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger'; -import { buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass'; +import { auditModel, buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass'; import type { Dispatch } from '../lib/review-depth'; import { runScriptSync } from '../lib/script-invocation'; import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedRead } from '../lib/review-targeted'; @@ -321,9 +321,11 @@ function pass(): number { if (args.includes('--audit-only')) { const extrasFile = flag('--extras'); if (!extrasFile) { process.stderr.write('gstack-review-dispatch pass --audit-only: needs --extras (the audit readers, e.g. Step 14.5\'s docs reader)\n'); return EXIT.usage; } - const extras = JSON.parse(readFile(extrasFile, 'extras')!) as Extra[]; + const given = JSON.parse(readFile(extrasFile, 'extras')!) as Extra[]; + const audit = given.some(e => !e.model) ? auditModel() : null; + const extras = given.map(e => (e.model || !audit?.model ? e : { ...e, model: audit.model })); const m = buildManifest({ pass: id, out, ledger: flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'), base, diffBase: mergeBaseWith(base) ?? 'HEAD', root: git(['rev-parse', '--show-toplevel'])?.trim() ?? process.cwd(), - checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: [], next: 'audit', planLines: ['PASS: audit-only (no reviewer plan; its extras run as one batch)'], extras, voices: args.includes('--voices') }); + checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: [], next: 'audit', planLines: ['PASS: audit-only (no reviewer plan; its extras run as one batch)', ...(audit ? [audit.line] : [])], extras, voices: args.includes('--voices') }); fs.mkdirSync(m.out, { recursive: true, mode: 0o700 }); const t = sectionTexts(ROOT); renderStage(m, 'first', t, branchDiff); diff --git a/capy/skill/gstack/SKILL.md b/capy/skill/gstack/SKILL.md index 9e837424d..43d896f0d 100644 --- a/capy/skill/gstack/SKILL.md +++ b/capy/skill/gstack/SKILL.md @@ -46,7 +46,7 @@ Capy checklist for `/autoplan`: drive `gstack-autoplan`, never a hand adaptation Token caps on /ship: pass a brief's cap as `--token-cap `. /ship reads `cost.tokens` and `cost.credits` from `tools.thread.get({ thread_id: "" })` (its own jam; a subagent reading root `"this"` starts `--subagent --reading-from this --meter-scope tree`) inside the same execute program as the `gstack-ship-budget` command that takes them as `--reading --credits `, never alone. `ship/DRIVER.md` starts `--mode observe`: each program ends with `note`, and only a note past the cap stops (`docs/ship-budget.md`). -/ship on Capy is about a dozen calls, each an execute program (`ship/DRIVER.md`). Each review pass and Step 11 is one call to the checked-in pass program (fill only `input`): `const t0 = await now(); const p = (await tools.capy.bash({ command: ". '' && gstack-review-dispatch program", description: "Pass" })).output.split("\n"); return p[0] + "\n" + (await new (Object.getPrototypeOf(async () => {}).constructor)("tools", "sleep", "now", "input", JSON.parse(p[1]))(tools, sleep, now, { startedAt: t0, launch: "", cwd: "", base: "", out: "/p", pass: "-p", laneModel: "", planFlags: [], logStart: ["review"], jam: "", record: "", step: "9.2" }));` It plans, logs `logStart` (`reservation` mode from `RESERVATION_ID:`), sends each brief on `machine: "shared"`, then polls: one `tools.subagent.list` per ~20 s tick, returning 200 s after the program's first line, where a reader is settled only at status `done` or `failed` or with a `summary` (a just-created `idle` reader has not run); it starts readers under `rejected` with `tools.subagent.start` as slots free and stops on one stuck 20 minutes. It ends on `NEXT:` (`poll`: rerun) or `STOP:`. `models` maps roles to models (`outside-voice`: another family, pinned in `voice-models.json`); Step 11 swaps `logStart` for `stage: "voices"`; a full repeat pass, `diffBase: ""`. Steps 7, 8 and 14.5 (`step: "14.5"`) use `extras`, JSON `{name, prompt_file, first, model}` (prompts verbatim from their sections, Step 7 `first`); they cannot block, so `model: { model_id: "anthropic/claude-sonnet-5-5", reasoning_mode: "low" }` (14.5 once). Waits: never let 5 minutes pass between calls. While readers run, never end the turn on `wait`; rerun the loader. Else re-issue `wait` with `duration_seconds: 200` and operation ids. At Step 22 report the `CALLS:` line of `gstack-ship-budget calls --events ` on your transcript since /ship began. A small waiting child saves nothing. +/ship on Capy is about a dozen calls, each an execute program (`ship/DRIVER.md`). Each review pass and Step 11 is one call to the checked-in pass program (fill only `input`): `const t0 = await now(); const p = (await tools.capy.bash({ command: ". '' && gstack-review-dispatch program", description: "Pass" })).output.split("\n"); return p[0] + "\n" + (await new (Object.getPrototypeOf(async () => {}).constructor)("tools", "sleep", "now", "input", JSON.parse(p[1]))(tools, sleep, now, { startedAt: t0, launch: "", cwd: "", base: "", out: "/p", pass: "-p", laneModel: "", planFlags: [], logStart: ["review"], jam: "", record: "", step: "9.2" }));` It plans, logs `logStart` (`reservation` mode from `RESERVATION_ID:`), sends each brief on `machine: "shared"`, then polls: one `tools.subagent.list` per ~20 s tick, returning 200 s after the program's first line, where a reader is settled only at status `done` or `failed` or with a `summary` (a just-created `idle` reader has not run); it starts readers under `rejected` with `tools.subagent.start` as slots free and stops on one stuck 20 minutes. It ends on `NEXT:` (`poll`: rerun) or `STOP:`. `models` maps roles to models (`outside-voice`: another family, pinned in `voice-models.json`); Step 11 swaps `logStart` for `stage: "voices"`; a full repeat pass, `diffBase: ""`. Steps 7, 8 and 14.5 (`step: "14.5"`) use `extras`, JSON `{name, prompt_file, first, model}` (prompts verbatim from their sections, Step 7 `first`); they cannot block, so `model: { model_id: "anthropic/claude-sonnet-5-5", reasoning_mode: "low" }` (not 14.5). Waits: never let 5 minutes pass between calls. While readers run, never end the turn on `wait`; rerun the loader. Else re-issue `wait` with `duration_seconds: 200` and operation ids. At Step 22 report the `CALLS:` line of `gstack-ship-budget calls --events ` on your transcript since /ship began. A small waiting child saves nothing. ## Plan, approve, ship diff --git a/changelog.d/capy-gst-1439-docs-audit-agent-model.md b/changelog.d/capy-gst-1439-docs-audit-agent-model.md new file mode 100644 index 000000000..6a99391f5 --- /dev/null +++ b/changelog.d/capy-gst-1439-docs-audit-agent-model.md @@ -0,0 +1 @@ +- **/ship's docs audit runs on your model preference.** Step 14.5's docs audit was told to run on Sonnet 5.5 at low effort no matter what model you prefer, so crews set to Opus in `preferences.yaml` still audited on Sonnet. `gstack-review-dispatch pass --audit-only` now gives a docs reader with no model your `agent_model` at low effort and prints the pick as `AUDIT_MODEL:`. With no preference it stays on Sonnet 5.5 at low effort, `host` uses the lane's own model, and a model named in the extras still wins. (GST-1439) diff --git a/docs/model-policy.md b/docs/model-policy.md index 60db5f375..8405d6c5f 100644 --- a/docs/model-policy.md +++ b/docs/model-policy.md @@ -148,6 +148,11 @@ Two cases ignore the preference on purpose: a cross-family outside voice (the `plan-review` role picks the other provider) and a model under test in an eval (`eval-arms` and `/eval-plan` arms). +/ship's Step 14.5 docs audit runs on the preference at low effort when its +extras name no model; unset or unusable, it runs on `anthropic/claude-sonnet-5-5` +at low effort, and `host` runs it on the lane's own model. The pass prints the +pick as `AUDIT_MODEL:`. + A value gstack can't use stops only the agent role, naming the file and the fix. `plan-review`, `implementation` and `eval-arms` keep resolving, and `gstack-models list` shows the problem in its agent block. diff --git a/lib/review-pass.ts b/lib/review-pass.ts index cf988c4c7..8a9f19588 100644 --- a/lib/review-pass.ts +++ b/lib/review-pass.ts @@ -14,6 +14,7 @@ import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { AgentModelError, resolveAgentModel } from './agent-model'; import { BriefError, briefFile, checkoutRoot, checkoutTree, promptHash, renderBrief, sliceDiff, sliceNote, withWorkspace, type BriefRole, type SectionTexts, type Slots } from './review-brief'; import { REVIEW_DIFF_MAX_BYTES, disclosureDigest } from './review-exclude'; import { appendEvent, readerKey } from './review-ledger'; @@ -70,6 +71,22 @@ export interface Manifest { } export interface FixCheckFiles { fix_list: string; hunks: string; tests: string; callers?: string } export interface Extra { name: string; prompt_file: string; first?: boolean; model?: PassModel } +/** Step 14.5's docs audit model when no preference is set (its pre-GST-1439 hard-code). */ +export const DOCS_AUDIT_DEFAULT: PassModel = { model_id: 'anthropic/claude-sonnet-5-5', reasoning_mode: 'low' }; + +/** The model an audit-only extra runs on when it names none (GST-1439): the owner's agent_model at low effort, + * DOCS_AUDIT_DEFAULT with no preference or an unusable one; agent_model host leaves it to the lane's own model. */ +export function auditModel(env: NodeJS.ProcessEnv = process.env): { model?: PassModel; line: string } { + const fallback = (why: string) => ({ model: DOCS_AUDIT_DEFAULT, line: `AUDIT_MODEL: ${DOCS_AUDIT_DEFAULT.model_id} reasoning=low (default; ${why})` }); + let s; + try { s = resolveAgentModel({ env }); } catch (error) { + if (!(error instanceof AgentModelError)) throw error; + return fallback(`${error.problem}, ignored`); + } + if (s.source.kind === 'default') return fallback('no agent_model set'); + if (!s.id) return { line: `AUDIT_MODEL: host (agent_model host in ${s.source.path}: the lane's own model)` }; + return { model: { model_id: s.id, reasoning_mode: 'low' }, line: `AUDIT_MODEL: ${s.id} reasoning=low (agent_model ${s.value} in ${s.source.path})` }; +} /** Step 9.2.1's own first line: its probes are the parent's, so a QA subagent only refuses (C4-36, C5-36: "parent-only"). */ export const PARENT_ONLY_QA = 'Only the parent runs report-only discovery.'; const qaRefusal = (name: string) => new BriefError(`Step 9.2.1 is the parent's report-only discovery (ship/sections/review-army.md: "${PARENT_ONLY_QA}"), never an audit reader: ${name}`); diff --git a/ship/DRIVER.md b/ship/DRIVER.md index 4dfaa7598..ed1001571 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -23,8 +23,8 @@ version, CHANGELOG, commits), **Documentation** (Step 14.5) and **Next steps** ( **Reviewers are the sections' own.** Every reviewer brief, its severity and confidence rules and gates come from its section verbatim; this file never shortens a reviewer's brief. A blocking reader's prompt is its `gstack-review-dispatch render` -file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block, so each runs at low effort on a small model, -Step 14.5 once on Sonnet 5.5; findings go to the PR body. +file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block: low effort, 7 and 8 on a small model, +14.5 once on `AUDIT_MODEL:`; findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's section read or fix-check, plus a keep-alive per 200 s waited. diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index 8974cb376..c50dd0aa4 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -21,8 +21,8 @@ version, CHANGELOG, commits), **Documentation** (Step 14.5) and **Next steps** ( **Reviewers are the sections' own.** Every reviewer brief, its severity and confidence rules and gates come from its section verbatim; this file never shortens a reviewer's brief. A blocking reader's prompt is its `gstack-review-dispatch render` -file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block, so each runs at low effort on a small model, -Step 14.5 once on Sonnet 5.5; findings go to the PR body. +file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block: low effort, 7 and 8 on a small model, +14.5 once on `AUDIT_MODEL:`; findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's section read or fix-check, plus a keep-alive per 200 s waited. diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 2cca6d6ec..218d1ffcd 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -706,6 +706,41 @@ describe('Capy pass program (fake host, real bins)', () => { expect(bin(dir, env, ['pass', '--base', 'main', '--out', path.join(dir, '.state/run/d2'), '--pass', 'x16b', '--audit-only']).status).not.toBe(0); }, 240_000); + // Value: protects=Step 14.5's docs audit runs on the owner's agent_model at low effort, today's Sonnet 5.5 low when no preference is set, and an extras model still wins; fails_when=the audit-only pass ignores preferences.yaml (GST-1439: Opus crews audited on Sonnet), a missing file changes the default, or a given model is overwritten; + // why_new=the audit model was a prose hard-code the driver copied into extras; seam=gstack-review-dispatch pass --audit-only; no_claim=reviewer passes and Steps 7 and 8, which keep their own models + test('the audit-only docs reader defaults to the owner agent_model at low effort; a given model wins', () => { + const { dir, env } = repo(); + const prompt = path.join(dir, '.state/docs.prompt.md'); + fs.mkdirSync(path.dirname(prompt), { recursive: true }); + fs.writeFileSync(prompt, 'Run Step 14.5 as written.\n'); + const prefs = path.join(dir, '.state/preferences.yaml'); + fs.writeFileSync(prefs, 'agent_model: opus\n'); + const audit = (id: string, e: NodeJS.ProcessEnv, model?: object) => { + const extras = path.join(dir, `.state/${id}.json`); + fs.writeFileSync(extras, JSON.stringify([{ name: 'docs', prompt_file: prompt, first: true, ...(model ? { model } : {}) }])); + const out = path.join(dir, '.state/run', id); + const r = bin(dir, e, ['pass', '--base', 'main', '--out', out, '--pass', id, '--audit-only', '--extras', extras]); + expect(r.status, r.stderr).toBe(0); + return { stdout: r.stdout, model: JSON.parse(fs.readFileSync(path.join(out, 'pass-manifest.json'), 'utf8')).readers[0].model }; + }; + const { GSTACK_PREFERENCES_FILE: _, ...noFile } = env; + const opus = audit('d-opus', { ...env, GSTACK_PREFERENCES_FILE: prefs }); + expect(opus.model).toEqual({ model_id: 'anthropic/claude-opus-5-5', reasoning_mode: 'low' }); + expect(opus.stdout).toContain('AUDIT_MODEL: anthropic/claude-opus-5-5 reasoning=low (agent_model'); + expect(audit('d-none', noFile).model).toEqual({ model_id: 'anthropic/claude-sonnet-5-5', reasoning_mode: 'low' }); + const given = { model_id: 'anthropic/claude-haiku-5-5', reasoning_mode: 'low' }; + expect(audit('d-given', { ...env, GSTACK_PREFERENCES_FILE: prefs }, given).model).toEqual(given); + // Value: protects=agent_model host leaves the model to the lane and an unusable value falls back to Sonnet 5.5 low; fails_when=host is sent as a model or a bad value aborts the pass; why_new=only opus/none/given rows existed; seam=none; no_claim=none beyond protects + fs.writeFileSync(prefs, 'agent_model: host\n'); + const host = audit('d-host', { ...env, GSTACK_PREFERENCES_FILE: prefs }); + expect(host.model).toBeUndefined(); + expect(host.stdout).toContain('AUDIT_MODEL: host'); + fs.writeFileSync(prefs, 'agent_model: not a model!\n'); + const bad = audit('d-bad', { ...env, GSTACK_PREFERENCES_FILE: prefs }); + expect(bad.model).toEqual({ model_id: 'anthropic/claude-sonnet-5-5', reasoning_mode: 'low' }); + expect(bad.stdout).toContain('ignored)'); + }, 240_000); + // Value: protects=a pass id typed in capitals is read as its lowercase id, so the manifest, its titles and a resume agree, and an id with other characters is still refused naming the rule; fails_when=--pass P1 is refused as a missing flag (gstack-evals#191 /ship), a capitalized resume of p1 reads as another pass, or p_1 is accepted; // why_new=the id was matched case-sensitively and its refusal read as a missing flag; seam=gstack-review-dispatch pass; no_claim=the reviewer plan, which --audit-only skips test('pass reads its id as lowercase; an id with other characters is still refused with the rule', () => { diff --git a/test/ship-driver.test.ts b/test/ship-driver.test.ts index 749abe859..2f74da5f6 100644 --- a/test/ship-driver.test.ts +++ b/test/ship-driver.test.ts @@ -219,14 +219,14 @@ describe('variant G: the call plan, observe budget, waits and audit readers (GST expect(flat(read('ship/sections/budget-handoff.md.tmpl'))).toContain('the wait until the last settles (`done`, `failed` or a final summary; a fresh `idle` has not run)'); }); - // Value: protects=audits that cannot block (Steps 7, 8, 14.5) run on a small model at low effort with their sections' own briefs, and skip their reader when --audit decides them; fails_when=an audit reader inherits the lane's model and effort (#36: the coverage audit alone was 2.45M at Opus), or an audit brief is paraphrased; + // Value: protects=audits that cannot block (Steps 7, 8, 14.5) run at low effort (7 and 8 on a small model, 14.5 on AUDIT_MODEL, GST-1439) with their sections' own briefs, and skip their reader when --audit decides them; fails_when=an audit reader inherits the lane's model and effort (#36: the coverage audit alone was 2.45M at Opus), or an audit brief is paraphrased; // why_new=audit model routing is new; seam=none; no_claim=the audits' recall test('audit readers run on a small model at low effort, named for Capy, and Phase 3 dispatches only what READER names', () => { - expect(flat(doc)).toContain('Audit readers (Steps 7, 8, 14.5) cannot block, so each runs at low effort on a small model, Step 14.5 once on Sonnet 5.5; findings go to the PR body'); + expect(flat(doc)).toContain('Audit readers (Steps 7, 8, 14.5) cannot block: low effort, 7 and 8 on a small model, 14.5 once on `AUDIT_MODEL:`; findings go to the PR body'); expect(flat(doc)).toContain('`Coverage:` and Step 8\'s line come from `--audit` when no reader ran'); const capy = read('capy/skill/gstack/SKILL.md'); expect(capy).toContain('model: { model_id: "anthropic/claude-sonnet-5-5", reasoning_mode: "low" }'); - expect(capy).toContain('(14.5 once)'); + expect(capy).toContain('reasoning_mode: "low" }` (not 14.5)'); }); // Value: protects=Step 9.2.1's report-only probes run in the parent's own Phase 4 program, as the section says, with its Reads in Phase 3, and