mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 18:28:15 +01:00
fix(autoplan): bind rec_hash to its run and plan; keep legacy gates answerable (GST-1819) (#4137)
* fix(autoplan): bind rec_hash to its run and plan; keep legacy gates answerable (GST-1819) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(autoplan): legacy rec_hash is not run-bound; re-render pending pre-change gates (GST-1819) 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>
This commit is contained in:
1 parent
ddbf1c9a47
commit
1a29fbc344
8 files changed
+70
-9
No files matched your search
@@ -0,0 +1 @@
|
||||
- **An autoplan `rec_hash` now names its run, so two unrelated gates no longer print the same one.** The hash a gate render prints, and `gstack-autoplan answer --rec-hash` checks, covered only the gate revision and the recommended letters, so any two runs with the same decision shape shared it (GST-1749's and GST-1811's gates both printed `f94689f795a5`), and a `rec` relayed for one run passed the check on the other. It now also covers the run id, the reviewed plan's sha256 and each item's title and option keys, and a hash from another run's gate is refused as `GATE_REC_STALE`. Gates written before this keep working. A `gate.json` without the new `rec_hash_v` marker still accepts the hash it printed then, and the answer records that hash. That old hash is not bound to its run, so re-render a gate still pending from before this change and answer with the new hash its render prints. Recorded approvals are untouched, since `gstack-approvals import --autoplan` and the plan record bind by run id and gate revision, never by `rec_hash`. See docs/troubleshooting.md#gate-rec-stale. (GST-1819)
|
||||
@@ -4967,6 +4967,16 @@ recommended `b` pending, and `all` now takes them.
|
||||
and answer with `--rec-hash <h>` from that render. Explicit `<id><option>`
|
||||
tokens are never refused.
|
||||
|
||||
Since GST-1819 the hash also names the run, its reviewed plan's sha256 and each
|
||||
item's title and option keys, so a `--rec-hash` from another run's gate with
|
||||
the same decision shape is refused too: answer each run with the hash its own
|
||||
render printed. A gate written before that (its `gate.json` has no
|
||||
`rec_hash_v`) still accepts the hash it printed then, and the answer records
|
||||
that hash. That old hash is not bound to its run, so two such gates with the
|
||||
same decision shape still accept each other's: re-render a pending one and
|
||||
answer with the new hash its render prints. A `recommend` on it moves it to the
|
||||
new hash.
|
||||
|
||||
<a id="checkpoint-invalid"></a>
|
||||
### `CHECKPOINT_INVALID`
|
||||
|
||||
|
||||
+1
-1
@@ -283,7 +283,7 @@ tree no review saw, so /ship runs again;
|
||||
| File | Schema | Contents |
|
||||
|------|--------|----------|
|
||||
| `run.json` | `run` v1 | The manifest: `run` id, `status`, `required_phases`, each phase's `native` and `outside` outcome (`status`, `model`, `provider`), input `snapshot` hashes, `counts`, every artifact's `path` (relative to the run directory) and `sha256`, `session_kind`, `gate_rev`, `deviations`, `consumed_by`. |
|
||||
| `decisions.jsonl` | `decisions` v1 | One row per decision: `id` (`<run>-<label>` at gate revision 1, `<run>-r<rev>-<label>` after a reopen, since labels restart at each gate), `title`, `options`, `recommended`, `kind` (`auto`, `approval`, `user_challenge`), `status` (`pending`, `approved`, `overridden`, `rejected`, or `auto_kept` in a [CEO-only run](#ceo-only-runs)), `gate_rev`, and once answered `chosen`, `answered_at`, `reply`, plus how the owner answered: `owner_mode` (`rec`, `rec_except`, `tokens`, `as_is` or `unparsed`, parsed from the owner's own words), `owner_named` (his words named this item), `rec_hash` (the recommendations answered against) and `answered_role` (`human`, or `agent` relaying his words); an agent-relayed answer adds `owner_reply`, his verbatim words, which never leave in a [public copy](#the-public-copy). |
|
||||
| `decisions.jsonl` | `decisions` v1 | One row per decision: `id` (`<run>-<label>` at gate revision 1, `<run>-r<rev>-<label>` after a reopen, since labels restart at each gate), `title`, `options`, `recommended`, `kind` (`auto`, `approval`, `user_challenge`), `status` (`pending`, `approved`, `overridden`, `rejected`, or `auto_kept` in a [CEO-only run](#ceo-only-runs)), `gate_rev`, and once answered `chosen`, `answered_at`, `reply`, plus how the owner answered: `owner_mode` (`rec`, `rec_except`, `tokens`, `as_is` or `unparsed`, parsed from the owner's own words), `owner_named` (his words named this item), `rec_hash` (the recommendations answered against, bound to the run and its reviewed plan) and `answered_role` (`human`, or `agent` relaying his words); an agent-relayed answer adds `owner_reply`, his verbatim words, which never leave in a [public copy](#the-public-copy). |
|
||||
| `shadow/` | private, not an artifact | [Shadow mode](#shadow-mode) only: `ceo-only.r<rev>.jsonl` (one hidden prediction per gate item, written before the answer), `r<rev>.prompt.md` and one `<call>.reply.txt` per model call, the audit `salt`, and `r<rev>.pending` while a call is in flight. Never in `export --public`, the gate render or the owner brief. |
|
||||
| `findings.jsonl` | `findings` v1 | One row per reviewer finding: `id`, `phase`, `voice` (`native`/`outside`), `model`, `severity`, `title`, `disposition`, `plan_items`, `native_counterpart` (string or null), `source`. Rows share the review-log row shape, so `gstack-review-log --findings` derives review status from this file. |
|
||||
| `tasks.jsonl` | `tasks` v2 | Implementation tasks with `id`, `title`, `status`, `depends_on`, `findings` (ids in `findings.jsonl`), `blocked_by`, `acceptance`, optional `pr`/`item`/`tier`. |
|
||||
|
||||
@@ -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, parseReply, pendingHint, recHash, replyShape, validateGateList, type GateChoice, type GateItem, type GateList, type GateOption, type ParsedReply } from './gate-list';
|
||||
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 type { DecisionRow, FindingRow } from './headless-artifacts';
|
||||
import { readJsonl, sha256File } from './headless-artifacts';
|
||||
import { closeDraftDirectionsFile, loadBriefInputs } from './owner-brief';
|
||||
@@ -254,7 +254,7 @@ export function buildGate(state: RunState, gateRev: number): GateList {
|
||||
}
|
||||
items.push(...capItems(state, { d: () => `d${++d}`, uc: () => `uc${++uc}` }));
|
||||
items.push(planApproval(state, items));
|
||||
const list: GateList = { schema_version: 1, run: state.run, gate_rev: gateRev, items, title: '/autoplan final approval gate', rec_rev: 0,
|
||||
const list: GateList = { schema_version: 1, run: state.run, gate_rev: gateRev, items, title: '/autoplan final approval gate', rec_rev: 0, rec_hash_v: 2,
|
||||
...(fs.existsSync(state.active_plan) ? { eli5: buildEli5(fs.readFileSync(state.active_plan, 'utf8'), state.active_plan) } : {}) };
|
||||
const problems = validateGateList(list);
|
||||
if (problems.length) throw new Error(`gate list invalid: ${problems.join('; ')}`);
|
||||
@@ -288,6 +288,7 @@ export function recommendGate(state: RunState, gateRev: number, itemId: string,
|
||||
const p1 = planApproval(state, list.items);
|
||||
list.items = list.items.map(i => (i.id === 'p1' ? p1 : i));
|
||||
list.rec_rev = (list.rec_rev ?? 0) + 1;
|
||||
list.rec_hash_v = 2;
|
||||
fs.writeFileSync(gateFile, JSON.stringify(list, null, 2) + '\n');
|
||||
const updated = new Map([item, p1].map(i => [decisionId(list.run, gateRev, i.id), i]));
|
||||
writeDecisions(state.out, readDecisions(state.out).map(r => {
|
||||
@@ -412,7 +413,9 @@ export function answerGate(state: RunState, gateRev: number, reply: string, answ
|
||||
if (parsed.error === 'GATE_REV_STALE') throw new RunError('GATE_REV_STALE', `reply names gate_rev=${gateRev}, current gate_rev=${list.gate_rev}`, 3);
|
||||
if (!parsed.valid) throw new RunError('GATE_REPLY_UNPARSED', parsed.error === 'GATE_EMPTY' ? 'empty reply' : `unparsed: ${parsed.unparsed.join(', ')}`);
|
||||
const currentHash = recHash(list);
|
||||
if (o.recHash !== undefined && o.recHash !== currentHash) throw new RunError('GATE_REC_STALE', `--rec-hash ${o.recHash} was rendered before the recommendations changed; current rec_hash=${currentHash}`, 3);
|
||||
const legacy = list.rec_hash_v === undefined && o.recHash !== undefined && o.recHash === legacyRecHash(list);
|
||||
if (o.recHash !== undefined && o.recHash !== currentHash && !legacy) throw new RunError('GATE_REC_STALE', `--rec-hash ${o.recHash} is not this gate's: it was rendered before the recommendations changed, or for another run; current rec_hash=${currentHash} (run ${list.run})`, 3);
|
||||
const answeredHash = legacy ? o.recHash! : currentHash;
|
||||
if (parsed.mode === 'rec' && list.rec_rev === undefined && o.recHash === undefined) throw new RunError('GATE_REC_STALE', `this gate was written before \`rec\`/\`all\` took plan changes (gate.json has no rec_rev), so the owner saw a different meaning; render it again (gstack-gate render ${gateFile} --all), relay it, and answer with --rec-hash ${currentHash}, or reply with explicit <id><option> tokens`, 3);
|
||||
const takesRecs = parsed.mode === 'rec' || parsed.choices.some(c => c.id === 'p1' && c.chosen === 'b' && !c.prior);
|
||||
if (takesRecs && (list.rec_rev ?? 0) > 0 && o.recHash === undefined) throw new RunError('GATE_REC_STALE', `recommend changed this gate (rec_rev=${list.rec_rev}); \`rec\` and \`p1b\` take its recommendations, so pass --rec-hash from the render the owner saw (current rec_hash=${currentHash})`, 3);
|
||||
@@ -427,7 +430,7 @@ export function answerGate(state: RunState, gateRev: number, reply: string, answ
|
||||
const lines: string[] = o.carry ? [] : voidPendingCheck(state, `the owner answered gate_rev ${gateRev}`);
|
||||
const answeredAt = now();
|
||||
const owner = (r: DecisionRow): Partial<DecisionRow> => (shape && r.gate_rev === list.gate_rev && r.answered_at === answeredAt
|
||||
? { owner_mode: shape.mode, owner_named: shape.named.has((r.label ?? '').toLowerCase()), rec_hash: currentHash, answered_role: answeredBy?.startsWith('agent:') ? 'agent' : 'human' } : {});
|
||||
? { owner_mode: shape.mode, owner_named: shape.named.has((r.label ?? '').toLowerCase()), rec_hash: answeredHash, answered_role: answeredBy?.startsWith('agent:') ? 'agent' : 'human' } : {});
|
||||
const record = (answered = parsed) => writeDecisions(state.out, applyReply(rows, answered, answeredAt, answeredBy, provenance).map(r => {
|
||||
const c = r.gate_rev === list.gate_rev && r.chosen && !r.chosen_text ? itemOf((r.label ?? '').toLowerCase())?.options.find(o => o.key === r.chosen) : undefined;
|
||||
return { ...(c ? { ...r, chosen_text: c.text } : r), ...owner(r) };
|
||||
|
||||
+15
-2
@@ -14,7 +14,8 @@
|
||||
* is ambiguous across multi-option items and is reported as unparsed, never
|
||||
* guessed. A reply binds to the gate revision it answers (`gate_rev`), and a
|
||||
* `rec` reply to the recommendations it saw (`rec_hash`, bumped by
|
||||
* `gstack-autoplan recommend`). Earlier answers at the same revision
|
||||
* `gstack-autoplan recommend`; it also names the run and its plan, so two
|
||||
* runs with the same decision shape never share one). Earlier answers at the same revision
|
||||
* (`prior`) are kept unless a token names the item again: only unresolved
|
||||
* items default.
|
||||
* Rendered by bin/gstack-gate; rows land in decisions.jsonl (lib/headless-artifacts.ts).
|
||||
@@ -50,6 +51,8 @@ export type ItemClass = 'challenge2' | 'challenge1' | 'disagree' | 'agent' | 'ca
|
||||
export interface GateList { schema_version: 1; run: string; gate_rev: number; items: GateItem[]; title?: string;
|
||||
/** Bumped by every `recommend`, so a `rec` relayed before it no longer matches `recHash`. */
|
||||
rec_rev?: number;
|
||||
/** 2: rendered with the run-bound `recHash`; absent on a gate written before it, whose renders printed `legacyRecHash`. */
|
||||
rec_hash_v?: 2;
|
||||
/** The plan's plain-language ask, read from the reviewed plan (lib/captain-brief.ts `buildEli5`); printed first. */
|
||||
eli5?: GateEli5;
|
||||
/** The one-reviewer check of an unreviewed post-cap delta before a carry (lib/autoplan-carry.ts). */
|
||||
@@ -101,8 +104,18 @@ export function validateGateList(list: unknown): string[] {
|
||||
return problems;
|
||||
}
|
||||
|
||||
/** The recommendations a `rec` reply takes, as a short hash the render prints and `answer --rec-hash` checks. */
|
||||
/**
|
||||
* The recommendations a `rec` reply takes, as a short hash the render prints and `answer --rec-hash` checks. It names
|
||||
* the run, the reviewed plan's sha256 and each item's title and option keys, so another run's gate never shares it.
|
||||
*/
|
||||
export function recHash(list: GateList): string {
|
||||
const recs = list.items.map(i => [i.id, i.recommended, i.recommended_basis ?? '', i.recommended_by ?? '']);
|
||||
const shape = list.items.map(i => [i.id, i.kind, i.title, i.options.map(o => [o.key, o.changes_plan ?? null])]);
|
||||
return createHash('sha256').update(JSON.stringify(['rec_hash/2', list.run, list.eli5?.plan_file_sha256 ?? '', list.gate_rev, list.rec_rev ?? 0, recs, shape])).digest('hex').slice(0, 12);
|
||||
}
|
||||
|
||||
/** The hash a gate written before `rec_hash_v` printed (gate revision and recommendations only); `answer` accepts it for that gate alone. */
|
||||
export function legacyRecHash(list: GateList): string {
|
||||
const recs = list.items.map(i => [i.id, i.recommended, i.recommended_basis ?? '', i.recommended_by ?? '']);
|
||||
return createHash('sha256').update(JSON.stringify([list.gate_rev, list.rec_rev ?? 0, recs])).digest('hex').slice(0, 12);
|
||||
}
|
||||
|
||||
+1
-1
@@ -557,7 +557,7 @@ export const RESULT_CODES = {
|
||||
},
|
||||
GATE_REC_STALE: {
|
||||
anchor: 'gate-rec-stale',
|
||||
summary: 'a `rec` reply was rendered before `gstack-autoplan recommend` changed the recommendations it takes, or the gate predates `rec` (a gate.json without rec_rev, rendered when `all` left changes pending)',
|
||||
summary: 'a `rec` reply was rendered before `gstack-autoplan recommend` changed the recommendations it takes, its `--rec-hash` came from another run\'s gate, or the gate predates `rec` (a gate.json without rec_rev, rendered when `all` left changes pending)',
|
||||
fix: 'render the gate again (`gstack-gate render <dir>/gate.json --all`), relay it, and answer with the printed `--rec-hash`',
|
||||
},
|
||||
CHECKPOINT_INVALID: {
|
||||
|
||||
@@ -22,7 +22,7 @@ import { afterAll, 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 { parseReply, renderGate, type GateList } from '../lib/gate-list';
|
||||
import { legacyRecHash, parseReply, recHash, renderGate, type GateList } from '../lib/gate-list';
|
||||
import { attemptsOf, autoplan, readRun, resultOf, review, ROOT, runPhase, startSmallRun } from './helpers/autoplan-run-fixture';
|
||||
|
||||
const TMP = fs.mkdtempSync(path.join(os.tmpdir(), 'autoplan-residuals-'));
|
||||
@@ -186,6 +186,31 @@ describe('B-b/B-c: recommend binds rec to the render the owner saw; the owner re
|
||||
});
|
||||
});
|
||||
|
||||
describe('GST-1819: rec_hash binds to its run; a gate rendered before that keeps its own hash', () => {
|
||||
test('another run\'s rec_hash for the same decision shape is refused; an unmarked gate accepts its legacy hash and records it', () => {
|
||||
const out = agreedRun('run-bound');
|
||||
const gateFile = path.join(out, 'gate.json');
|
||||
const gate = JSON.parse(fs.readFileSync(gateFile, 'utf8')) as GateList;
|
||||
expect(gate.rec_hash_v).toBe(2);
|
||||
const before = fs.readFileSync(path.join(out, 'decisions.jsonl'), 'utf8');
|
||||
for (const hash of [recHash({ ...gate, run: `${gate.run}-other` }), legacyRecHash(gate)]) {
|
||||
const refused = autoplan(['answer', '--out', out, '--gate-rev', '1', '--reply', 'rec', '--rec-hash', hash, '--by', 'human']);
|
||||
expect(refused.status, hash).toBe(3);
|
||||
expect(refused.stderr).toMatch(/is not this gate's: .* for another run; current rec_hash=[0-9a-f]{12} \(run \S+\)\).*\(GATE_REC_STALE\)$/m);
|
||||
}
|
||||
expect(fs.readFileSync(path.join(out, 'decisions.jsonl'), 'utf8')).toBe(before);
|
||||
delete gate.rec_hash_v;
|
||||
fs.writeFileSync(gateFile, JSON.stringify(gate, null, 2) + '\n');
|
||||
const legacy = legacyRecHash(gate);
|
||||
expect(legacy).not.toBe(recHash(gate));
|
||||
const ok = autoplan(['answer', '--out', out, '--gate-rev', '1', '--reply', 'rec', '--rec-hash', legacy, '--by', 'human']);
|
||||
expect(ok.status, ok.stderr).toBe(0);
|
||||
const answered = decisions(out).filter(r => r.answered_at);
|
||||
expect(answered.length).toBe(gate.items.length);
|
||||
for (const r of answered) expect(r.rec_hash).toBe(legacy);
|
||||
});
|
||||
});
|
||||
|
||||
describe('R2: later phases see their predecessors\' consensus', () => {
|
||||
test('the DX prompt carries the CEO consensus block', () => {
|
||||
const out = startSmallRun(path.join(TMP, 'dx-consensus'), ['--developer-tool']);
|
||||
|
||||
@@ -170,6 +170,15 @@ describe('next wave B1/B-b: rec, aliases, quotes and the rec_hash', () => {
|
||||
expect(recHash(moved)).not.toBe(h);
|
||||
expect(renderGate(moved)).toContain('[d1] Delivery shape (approval; recommended: b (both reviewers name it; set by agent:relay))');
|
||||
});
|
||||
test('rec_hash binds to its run and plan: two runs with the same decision shape never share one (GST-1819)', () => {
|
||||
const eli5 = { summary: 's', before_after: '', why: '', cause: '', decisions: '', plan_file_sha256: 'a'.repeat(64) };
|
||||
const first: GateList = { ...held, run: 'run-20261106101500', eli5 };
|
||||
const second: GateList = { ...held, run: 'run-20261107093000', eli5: { ...eli5, plan_file_sha256: 'b'.repeat(64) } };
|
||||
expect(recHash(first)).not.toBe(recHash(second));
|
||||
expect(recHash({ ...first, run: second.run })).not.toBe(recHash(first));
|
||||
expect(recHash({ ...first, eli5: second.eli5 })).not.toBe(recHash(first));
|
||||
expect(recHash({ ...first, eli5: undefined, items: first.items.map(i => (i.id === 'd2' ? { ...i, title: 'another decision' } : i)) })).not.toBe(recHash({ ...first, eli5: undefined }));
|
||||
});
|
||||
});
|
||||
|
||||
describe('decisions.jsonl rows', () => {
|
||||
|
||||
Reference in new issue
Block a user