From 516435311db5aaedde289a7aadf897efa6ad0c96 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 9 Nov 2026 01:14:58 -0800 Subject: [PATCH] Refuse agent words on owner-only autoplan answers (#4201) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../refuse-agent-words-on-user-challenges.md | 1 + docs/troubleshooting.md | 7 ++- docs/unattended.md | 5 ++ lib/approval-citations.ts | 3 +- lib/autoplan-carry.ts | 4 +- lib/autoplan-gate.ts | 5 +- lib/gate-list.ts | 3 ++ lib/result-codes.ts | 2 +- test/approval-classes.test.ts | 3 ++ test/autoplan-carry.test.ts | 54 ++++++++++++++++++- 10 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 changelog.d/refuse-agent-words-on-user-challenges.md diff --git a/changelog.d/refuse-agent-words-on-user-challenges.md b/changelog.d/refuse-agent-words-on-user-challenges.md new file mode 100644 index 000000000..1089bb7f5 --- /dev/null +++ b/changelog.d/refuse-agent-words-on-user-challenges.md @@ -0,0 +1 @@ +- **An agent's own words no longer settle a user challenge at the /autoplan gate.** `gstack-autoplan answer --words-by agent:` let a Captain choose a `uc` item (a user challenge both reviewers raised, or a round-cap approval) on its own authority, though only the owner may settle one. Such a reply, whether `rec`, `as-is` or a `uc` token, is now refused with `OWNER_REPLY_REQUIRED`, naming the item and asking for the owner's words, and the gate stays open. The agent can still answer the `d` items in its own words and leave the `uc` items to the owner. The rule that agent words are never the owner's now lives in one place, shared by the answer path, carry-forward and plan-sha citations. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index ed4bc882c..a68f85895 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5078,12 +5078,15 @@ directory, and this one already holds files. ### `OWNER_REPLY_REQUIRED` **Meaning.** An agent answered the gate (`--by agent:`) without recording -what the owner actually said. Nothing was applied. +what the owner actually said, or answered a `uc` item (a user challenge or a +round-cap approval, which only the owner settles) in its own words +(`--words-by agent:`). Nothing was applied. **Fix.** Pass the owner's words verbatim: `--owner-reply ""`, `--owner-reply -` to read them from stdin, or `--owner-reply-file `. They are stored on every decision row; when they parse under the gate grammar they -must resolve to the same choices as `--reply`. +must resolve to the same choices as `--reply`. For a `uc` item, relay the +owner's answer without `--words-by`, or leave that item out of the reply. ### `GATE_OWNER_REPLY_MISMATCH` diff --git a/docs/unattended.md b/docs/unattended.md index 2df004cfb..1246cf76d 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -540,6 +540,11 @@ The example above is the full run. The first `next` also takes `--kind`: are then recorded as `answer_words` with `words_by`, never as `owner_reply`; a later gate never carries them (`CARRY: no`), and `gstack-approvals import` lists that row under the agent, not the owner. + They never settle a `uc` item (a user challenge or a round-cap approval): + a reply in an agent's own words that would choose one, by `rec`, `as-is` + or a token, is refused (`OWNER_REPLY_REQUIRED`) and nothing is applied; + such a reply may still answer the `d` items and leave the `uc` items to + the owner. An unattended run refuses an answer without `--by` (`ANSWER_BY_REQUIRED`, exit 3) and records the value as `answered_by` on every decision row and in the review record's gate line, diff --git a/lib/approval-citations.ts b/lib/approval-citations.ts index 0bea4b3c4..6ec869e39 100644 --- a/lib/approval-citations.ts +++ b/lib/approval-citations.ts @@ -30,6 +30,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { runApprovalScan, type ApprovalHit, type ApprovalScope } from './approval-classes'; +import { agentWords } from './gate-list'; import { sha256File } from './headless-artifacts'; import { isUnavailable, loadPolicyFromBase } from './ship-policy'; @@ -78,7 +79,7 @@ function planGate(dir: string, run: Record): Resolution { const p1 = readJsonl(path.join(dir, 'decisions.jsonl')).find(d => d.label === 'P1' && d.gate_rev === rev); const approved = run.status === 'complete' && typeof gate.plan === 'string' && !['c', 'e'].includes(gate.plan) && p1 && p1.chosen === gate.plan && p1.status !== 'pending'; if (!approved) return { state: 'rejected', why: `run ${run.run} holds this plan but its gate is not APPROVED (status ${run.status}, p1 ${p1?.chosen ?? 'unanswered'})` }; - if (p1.words_by) return { state: 'rejected', why: `run ${run.run}: its gate was answered in ${p1.words_by}'s own words, not the owner's` }; + if (agentWords(p1)) return { state: 'rejected', why: `run ${run.run}: its gate was answered in ${agentWords(p1)}'s own words, not the owner's` }; return { state: 'resolved', evidence: `plan ${run.run} gate_rev=${rev} p1=${gate.plan} answered_by=${gate.answered_by ?? p1.answered_by ?? 'unrecorded'}`, text: fs.readFileSync(planFile, 'utf8') }; } diff --git a/lib/autoplan-carry.ts b/lib/autoplan-carry.ts index ccdbffd4c..441a95873 100644 --- a/lib/autoplan-carry.ts +++ b/lib/autoplan-carry.ts @@ -29,7 +29,7 @@ import { BOUNDARY, RESULT_FORMAT, REVIEWER_TRAILER } from './autoplan-prompts'; import { parseReviewResult } from './autoplan-reconcile'; import { FILES, RunError, journal, now, readDecisions, readJournal, readState, writeDecisions, writeState, type RunState } from './autoplan-state'; import { appendTrailRow } from './decision-trail'; -import { decisionId, parseReply, type GateItem, type GateList } from './gate-list'; +import { agentWords, decisionId, parseReply, type GateItem, type GateList } from './gate-list'; import { readJsonl } from './headless-artifacts'; import { appendLedger, notEli5Only } from './no-stop'; import { modelFamily } from './outside-voice-runner'; @@ -120,7 +120,7 @@ function assess(state: RunState, prev: GateList, list: GateList): Plan { if (p1.chosen !== 'a' && p1.chosen !== 'b') return { reason: `gate_rev ${from} answer was p1${p1.chosen}, not an approval`, from }; if (p1.carried_gate_from) return { reason: `gate_rev ${from} was itself carried; the owner answers this one`, from }; const tokens = parseReply(p1.reply ?? '', prev).mode !== 'rec'; - if (p1.words_by) return { reason: `gate_rev ${from} was answered in ${p1.words_by}'s own words, not the owner's`, from }; + if (agentWords(p1)) return { reason: `gate_rev ${from} was answered in ${agentWords(p1)}'s own words, not the owner's`, from }; const words = p1.owner_reply ?? (p1.answered_by === 'human' ? p1.reply : undefined); if (!words?.trim()) return { reason: `no owner words recorded at gate_rev ${from}`, from }; const amendments = (state.amendments ?? []).filter(a => a.gate_rev === from); diff --git a/lib/autoplan-gate.ts b/lib/autoplan-gate.ts index 66015b05e..3600c6c12 100644 --- a/lib/autoplan-gate.ts +++ b/lib/autoplan-gate.ts @@ -28,7 +28,7 @@ import { evaluateCarry, gateEvidence, voidPendingCheck } from './autoplan-carry' import { buildEli5, capyRelayLine } from './captain-brief'; import { FINDING_ORIGIN, previousReview, reviewRows, settledIds } from './autoplan-confirm'; import { phaseClose } from './autoplan-timing'; -import { ANSWERED_BY_RE, applyReply, autoDefault, decisionId, decisionRows, keepDirection, legacyRecHash, parseReply, pendingHint, recHash, replyShape, validateGateList, type GateChoice, type GateItem, type GateList, type GateOption, type ParsedReply } from './gate-list'; +import { agentWords, ANSWERED_BY_RE, applyReply, autoDefault, decisionId, decisionRows, keepDirection, legacyRecHash, parseReply, pendingHint, recHash, replyShape, validateGateList, type GateChoice, type GateItem, type GateList, type GateOption, type ParsedReply } from './gate-list'; import type { DecisionRow, FindingRow } from './headless-artifacts'; import { readJsonl, sha256File } from './headless-artifacts'; import { closeDraftDirectionsFile, loadBriefInputs } from './owner-brief'; @@ -423,6 +423,9 @@ export function answerGate(state: RunState, gateRev: number, reply: string, answ const provenance = o.carry ? { owner_reply: o.carry.words, carried_gate_from: o.carry.from, carried_gate_rev: o.carry.gate_rev, carry_reason: o.carry.reason, ...(o.carry.by ? { carried_answered_by: o.carry.by } : {}) } : o.ownerReply !== undefined && o.wordsBy ? { answer_words: o.ownerReply, answer_words_parsed: ownerParsed!.valid, words_by: o.wordsBy } : o.ownerReply !== undefined ? { owner_reply: o.ownerReply, owner_reply_parsed: ownerParsed!.valid } : {}; + const agent = agentWords(provenance); + const challenges = agent ? parsed.choices.filter(c => !c.prior && c.kind === 'approval' && c.id !== 'p1').map(c => c.id) : []; + if (challenges.length) throw new RunError('OWNER_REPLY_REQUIRED', `${challenges.map(id => `${id} is ${list.items.find(i => i.id === id)!.cap ? 'a round-cap approval' : 'a user challenge'} only the owner can settle`).join('; ')}, and this reply is in ${agent}'s own words (--words-by); relay the owner's words with --owner-reply "" and no --words-by, or leave ${challenges.join(', ')} out of the reply; nothing was applied`, 3); const shape = o.carry || o.wordsBy ? undefined : replyShape(o.ownerReply ?? reply, list, ownerParsed ?? parsed); const itemOf = (id: string) => list.items.find(i => i.id === id)!; const optionOf = (c: GateChoice) => itemOf(c.id).options.find(o => o.key === c.chosen)!; diff --git a/lib/gate-list.ts b/lib/gate-list.ts index 2b8642e11..72a53cccf 100644 --- a/lib/gate-list.ts +++ b/lib/gate-list.ts @@ -274,6 +274,9 @@ export function decisionRows(list: GateList, extra: Partial = {}): /** `--by` values: the owner typing tokens, or an agent relaying or deciding them. */ export const ANSWERED_BY_RE = /^(human|agent:[A-Za-z0-9_.:-]+)$/; +/** The agent whose own words (`--words-by agent:`) an answer was in; never the owner's words, so they approve no plan, carry no gate and settle no user challenge. */ +export const agentWords = (r: { words_by?: string }): string | undefined => r.words_by || undefined; + /** Who answered and in whose words: a new answer replaces all of it, so an earlier answerer's fields never survive under a later one. */ const PROVENANCE = ['answered_by', 'owner_reply', 'owner_reply_parsed', 'answer_words', 'answer_words_parsed', 'words_by', 'carried_gate_from', 'carried_gate_rev', 'carry_reason', 'carried_answered_by'] as const; diff --git a/lib/result-codes.ts b/lib/result-codes.ts index 6046a8239..512833c93 100644 --- a/lib/result-codes.ts +++ b/lib/result-codes.ts @@ -578,7 +578,7 @@ export const RESULT_CODES = { }, OWNER_REPLY_REQUIRED: { anchor: 'owner-reply-required', - summary: 'an agent answered the gate (`--by agent:`) without the owner\'s verbatim reply', + summary: 'an agent answered the gate (`--by agent:`) without the owner\'s verbatim reply, or answered a user challenge in its own words (`--words-by`)', fix: 'pass the owner\'s words exactly with `--owner-reply ""`, `--owner-reply -` (stdin) or `--owner-reply-file `', }, GATE_OWNER_REPLY_MISMATCH: { diff --git a/test/approval-classes.test.ts b/test/approval-classes.test.ts index 3ca163047..0b9b5e262 100644 --- a/test/approval-classes.test.ts +++ b/test/approval-classes.test.ts @@ -258,6 +258,9 @@ describe('gstack-ship-receipt write enforces declared classes (APPROVAL_CITATION expect(autoplan(['next', '--out', out, '--plan', path.join(dir, 'plan.md'), '--no-ui']).status).toBe(0); runPhase(out, 'ceo'); runPhase(out, 'eng'); + const owners = (JSON.parse(fs.readFileSync(path.join(out, 'gate.json'), 'utf8')).items as Array<{ id: string; recommended: string }>).filter(i => i.id.startsWith('uc')).map(i => `${i.id}${i.recommended}`); + expect(owners.length).toBeGreaterThan(0); + expect(autoplan(['answer', '--by', 'human', '--out', out, '--gate-rev', '1', '--reply', owners.join(' ')]).stdout).toMatch(/^GATE_PENDING: p1 unanswered/m); const reply = recCarrying(out); expect(autoplan(['answer', '--by', 'agent:test', '--words-by', 'agent:test', '--owner-reply', reply, '--out', out, '--gate-rev', '1', '--reply', reply]).stdout).toMatch(/^APPROVED: gate_rev=1/m); const sha = (JSON.parse(fs.readFileSync(path.join(out, 'run.json'), 'utf8')).artifacts['plan.md'].sha256 as string).slice(0, 8); diff --git a/test/autoplan-carry.test.ts b/test/autoplan-carry.test.ts index a87530179..8772eb196 100644 --- a/test/autoplan-carry.test.ts +++ b/test/autoplan-carry.test.ts @@ -25,6 +25,7 @@ const AGREE = { native: [{ id: 'F1', severity: 'High', title: 'Ship the backoff behind a flag first', user_challenge: true, changes: 'scope', fix: 'gate the change behind RETRY_BACKOFF=1' }], outside: [{ id: 'O1', severity: 'High', title: 'Ship the backoff behind a flag first', user_challenge: true, changes: 'scope', fix: 'gate the change behind RETRY_BACKOFF=1' }], }; +const ONE_VOICE = { native: AGREE.native, outside: [] }; const V2 = SMALL_PLAN.replace('cap 30 s', 'cap 30 s, behind RETRY_BACKOFF=1'); const gateOf = (out: string) => JSON.parse(fs.readFileSync(path.join(out, 'gate.json'), 'utf8')); const decisions = (out: string) => readJsonl(path.join(out, 'decisions.jsonl')); @@ -52,7 +53,16 @@ const recHashOf = (out: string) => recHash(gateOf(out)); describe('whose words answered the gate (GEVA-2, P7)', () => { test("an agent's own words are recorded as its own, never as the owner's, never carried, and imported under the agent", () => { - const s = answeredAndRevised([], 'Accepted on the standing approval', AGREE, ['--words-by', 'agent:gstack-minder']); + const dir = path.join(r.host.base, `carry${++n}`); + const s = { out: path.join(dir, 'run') }; + expect(r.ap(['next', '--out', s.out, '--plan', writePlan(dir), '--no-ui']).status).toBe(0); + phaseVia(r, s.out, 'ceo', ONE_VOICE); + phaseVia(r, s.out, 'eng', NONE); + const ans = r.ap(['answer', '--out', s.out, '--gate-rev', '1', '--reply', 'd1b p1b', '--by', 'agent:gstack-minder', '--owner-reply', 'Accepted on the standing approval', '--words-by', 'agent:gstack-minder']); + expect(ans.status, ans.stderr).toBe(0); + expect(ans.stdout).toMatch(/^APPLY: /m); + fs.writeFileSync(path.join(dir, 'plan-v2.md'), V2); + expect(r.ap(['revise', '--out', s.out, '--plan', path.join(dir, 'plan-v2.md'), '--incorporates', 'd1']).status).toBe(0); const p1 = decisions(s.out).find(d => d.gate_rev === 1 && d.label === 'P1'); expect(p1).toMatchObject({ answer_words: 'Accepted on the standing approval', words_by: 'agent:gstack-minder' }); expect(p1.owner_reply).toBeUndefined(); @@ -102,6 +112,48 @@ describe('whose words answered the gate (GEVA-2, P7)', () => { }); }); +describe('a user challenge is settled only in the owner\'s words', () => { + const challenged = () => { + const dir = path.join(r.host.base, `carry${++n}`); + const out = path.join(dir, 'run'); + expect(r.ap(['next', '--out', out, '--plan', writePlan(dir), '--no-ui']).status).toBe(0); + phaseVia(r, out, 'ceo', AGREE); + phaseVia(r, out, 'eng', NONE); + expect(gateOf(out).items.find((i: any) => i.id === 'uc1')).toMatchObject({ kind: 'approval', item_class: 'challenge2' }); + return out; + }; + const agent = ['--by', 'agent:gstack-minder', '--words-by', 'agent:gstack-minder', '--owner-reply', 'Accepted on the standing approval']; + + test('agent words that would settle uc1 are refused, naming the item and the owner words it needs; the gate stays open', () => { + const out = challenged(); + const before = fs.readFileSync(path.join(out, 'decisions.jsonl'), 'utf8'); + for (const reply of ['rec', 'as-is', 'uc1b p1b', 'uc1a']) { + const res = r.ap(['answer', '--out', out, '--gate-rev', '1', '--reply', reply, '--rec-hash', recHashOf(out), ...agent]); + expect(res.status, `${reply}: ${res.stdout}`).toBe(3); + expect(res.stderr).toMatch(/uc1 is a user challenge only the owner can settle.*agent:gstack-minder's own words.*--owner-reply.*\(OWNER_REPLY_REQUIRED\)/); + } + expect(fs.readFileSync(path.join(out, 'decisions.jsonl'), 'utf8')).toBe(before); + expect(readRunJson(out).status).toBe('gate_pending'); + }); + + test("the owner's words settle the same item", () => { + const out = challenged(); + const res = r.ap(['answer', '--out', out, '--gate-rev', '1', '--reply', 'rec', '--rec-hash', recHashOf(out), '--by', 'agent:gstack-minder', '--owner-reply', 'Garry: Do it']); + expect(res.status, res.stderr).toBe(0); + expect(res.stdout).toMatch(/^APPLY: .*uc1/m); + expect(decisions(out).find(d => d.label === 'UC1')).toMatchObject({ chosen: 'b', status: 'approved', owner_reply: 'Garry: Do it' }); + }); + + test('agent words still answer the items an agent may decide, leaving uc1 to the owner', () => { + const out = challenged(); + const d1 = gateOf(out).items.find((i: any) => i.id === 'd1'); + const res = r.ap(['answer', '--out', out, '--gate-rev', '1', '--reply', d1 ? 'd1a p1a' : 'p1a', ...agent]); + expect(res.status, res.stderr).toBe(0); + expect(res.stdout).toMatch(/^GATE_PENDING: uc1 unanswered/m); + expect(decisions(out).find(d => d.label === 'UC1')).toMatchObject({ status: 'pending' }); + }); +}); + describe('carry-forward', () => { test('a second gate with nothing new carries the owner\'s words: CARRIED, APPROVED, the record names its source', () => { const s = answeredAndRevised();