mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(ship): GST-986 write/carry report fail-closed targeted classes; docs, changelog, adversarial repin (brief bodies unchanged)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
e0ff828349
commit
84d644f356
7 files changed
+75
-13
No files matched your search
@@ -268,6 +268,7 @@ function cmdWrite(rest: string[], json: boolean): number {
|
||||
return EXIT.fail;
|
||||
}
|
||||
const targetedVerdict = receiptTargetedAtHead(repoRoot, `origin/${base.replace(/^origin\//, '')}`, receipt, receipt.head);
|
||||
if (targetedVerdict.line.includes('(unchecked: ')) process.stderr.write(targetedVerdict.line + '\n');
|
||||
if (!targetedVerdict.ok) {
|
||||
process.stderr.write(`${targetedVerdict.line}\n${resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${targetedVerdict.message}; run the targeted reader on HEAD and pass its record (--review-targeted @<gstack-review-dispatch verify --targeted-out file>); nothing written`)}\n`);
|
||||
return EXIT.fail;
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
### Fixed
|
||||
|
||||
- **The targeted reader's record is honest from verify to the receipt.** `gstack-review-dispatch verify` no longer marks a completed targeted reader `status=failed findings=0` because a Step 11 fix moved HEAD after it read (gstack-evals#140, C5-36 and C6-36). The record now says `done` with its real finding count and names the commit it read. `gstack-ship-receipt write --review-targeted` binds it only while that commit's product patch matches the receipt's, so a record verified before a product change can no longer vouch for it.
|
||||
- **`write` and `carry` refuse a receipt the queue would refuse (#3854).** When the head touches money, gate, auth or secret code and no done targeted pass covers it, `write` refuses with `RECEIPT_INVALID` and `carry` with `RECEIPT_CARRY_REFUSED`, instead of publishing a receipt `ready` turns away. Legacy receipts still carry.
|
||||
- **/ship passes the targeted record to the receipt (#3853).** Step 11's verify writes `.gstack/tmp/targeted.json`, and the Step 19, handoff and driver receipt writes pass it with `--review-targeted`.
|
||||
- **Fix-check rounds count without hand edits (#3862).** `gstack-review-scope summary --run` counts fix-checks from the dispatch ledgers when no `review-fix-check` row was logged, and counts the blockers those fix-checks followed as fixed.
|
||||
+11
-4
@@ -373,8 +373,15 @@ The brief states the blocking floor in one sentence, and its money block cites
|
||||
the checklist's Forced-CRITICAL rule on uncapped spend, so a money finding can
|
||||
block without new floor words. The reader is read-only, and `NO FINDINGS` is a
|
||||
result. `gstack-review-dispatch render --role targeted --classes <classes>`
|
||||
renders its brief; `verify --targeted-out <file>` writes the record that
|
||||
`gstack-ship-receipt write --review-targeted @<file>` puts in the receipt.
|
||||
renders its brief; `verify --targeted-out .gstack/tmp/targeted.json` writes the record that
|
||||
`gstack-ship-receipt write --review-targeted @.gstack/tmp/targeted.json` puts in the receipt
|
||||
(Step 19, the handoff and the driver pass it when the file exists). The record is `done` when
|
||||
the reader got its brief verbatim and its reply was collected on a committed diff, whatever
|
||||
happened to HEAD after, and it names the commit it read (`head`). `write` binds it only while
|
||||
that commit's product patch is the receipt's: a product change after the read refuses with
|
||||
`RECEIPT_INVALID`, and a test-only or version commit still binds. `write` and `carry` also
|
||||
refuse a receipt whose head needs a class no `done` record covers (`RECEIPT_INVALID`,
|
||||
`RECEIPT_CARRY_REFUSED`), so /ship never publishes a receipt the queue would refuse.
|
||||
`gstack-ship-queue ready` and `train build` print
|
||||
`REVIEW_TARGETED: required=<classes|none> receipt=<classes|none|legacy>
|
||||
verdict=ok|missing` and refuse nothing yet; enforcement comes in a later
|
||||
@@ -402,11 +409,11 @@ puts in the receipt:
|
||||
| `review_loop.noted` | below-floor findings listed in the PR body |
|
||||
| `review_loop.fixed` | findings fixed in this PR |
|
||||
| `review_loop.scope` | the `review_scope` the loop ran under |
|
||||
| `review_loop.fix_checks` | fix-checks this ship ran (never passes) |
|
||||
| `review_loop.fix_checks` | fix-checks this ship ran (never passes): the larger of the `review-fix-check` rows and the ledgers holding a fix-check reader's review. When fix-checks ran without their rows, a blocker no row disposes counts as fixed once a later fix-check ledger followed it (decision 3.5 runs one only after the fixes) |
|
||||
| `review_loop.rounds` | passes plus fix-checks (R51 caps it at 2; the queue's `REVIEW_DEPTH:` line prints it) |
|
||||
| `review_loop.dispatches` | reviewer dispatches by role: `specialist`, `red-team`, `adversarial-native`, `outside-voice`, `fix-check`, `targeted` |
|
||||
| `review_loop.est_input_tokens` | prompt plus diff bytes the dispatches read, divided by 4: a relative estimate for comparing runs, not a bill |
|
||||
| `review_targeted` | the [targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code): the `classes` it covered, `rules`, `brief_sha`, the receipt's own `product_patch_id`, `status` (`done` only with its output collected and the committed diff it was rendered on still the one at verify, else `failed`; absent when write got no record, which the queue reports as `legacy`), `findings` and `raised_by`; a carry keeps it while the product patch is unchanged |
|
||||
| `review_targeted` | the [targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code): the `classes` it covered, `rules`, `brief_sha`, the receipt's own `product_patch_id`, `status` (`done` only with its brief sent verbatim and its output collected on a committed diff, else `failed`; `write` refuses a record whose read commit has another product patch, and refuses to write without a record when the head needs a class), `findings` and `raised_by`; a carry keeps it while the product patch is unchanged |
|
||||
|
||||
Each review row may carry `dispatches: [{role, effort, prompt_bytes,
|
||||
diff_bytes, status}]`, and each finding `raised_by` (the reviewer that raised it,
|
||||
|
||||
+14
-4
@@ -3230,10 +3230,17 @@ for an older PR, write it once by hand with the same command.
|
||||
**Meaning.** The block exists but does not validate against the `ship-receipt`
|
||||
schema (`gstack-artifact schema ship-receipt`). `gstack-ship-receipt write`
|
||||
also refuses with this code, and writes nothing, when `--head` is not the
|
||||
checked-out HEAD: the receipt binds the tree the review read.
|
||||
checked-out HEAD: the receipt binds the tree the review read. It refuses the
|
||||
same way when the head touches money, gate, auth or secret code and no done
|
||||
targeted record covers those classes (`REVIEW_TARGETED: … verdict=missing|failed`),
|
||||
or when `--review-targeted` names a read commit whose product patch is not
|
||||
the receipt's (the product changed after the targeted reader read it).
|
||||
|
||||
**Fix.** Regenerate it with `gstack-ship-receipt write`; never hand-edit the
|
||||
block. For the `--head` refusal, check out that commit and write there.
|
||||
block. For the `--head` refusal, check out that commit and write there. For a
|
||||
targeted refusal, run the targeted reader on HEAD, `gstack-review-dispatch verify
|
||||
--targeted-out .gstack/tmp/targeted.json`, and write again with
|
||||
`--review-targeted @.gstack/tmp/targeted.json`.
|
||||
<a id="execution-unknown"></a>
|
||||
### `EXECUTION_UNKNOWN`
|
||||
|
||||
@@ -5695,7 +5702,10 @@ working tree's status. It also refuses when an owner exception the receipt recor
|
||||
no longer stands, and, for a receipt from a gstack that records review
|
||||
depth, when HEAD needs a deeper review than the receipt records, `--head` is
|
||||
not the checked-out HEAD, or the receipt records `gstack_version` without
|
||||
`review_depth` ([ship-policy.md](ship-policy.md#ship-receipt)). The receipt in
|
||||
`review_depth` ([ship-policy.md](ship-policy.md#ship-receipt)). It refuses, too, when
|
||||
HEAD needs a targeted class (money, gate, auth, secret) that the receipt's
|
||||
`review_targeted` does not cover with a done pass, since the queue would refuse
|
||||
that receipt. The receipt in
|
||||
the body is unchanged and still names the old head, so `gstack-ship-queue
|
||||
ready` refuses `QUEUE_RECEIPT_STALE`.
|
||||
|
||||
@@ -5703,7 +5713,7 @@ ready` refuses `QUEUE_RECEIPT_STALE`.
|
||||
`gstack-ship-receipt carry --pr <n>` at the new head. When the history is
|
||||
already rewritten and the receipt predates `head_tree`, run /ship on the new
|
||||
head; next time merge the base in instead of rebasing. For a retargeted stacked
|
||||
PR whose older receipt lost its parent branch, or a depth or exception
|
||||
PR whose older receipt lost its parent branch, or a depth, targeted or exception
|
||||
refusal, run /ship on the new head. When `--head` is not the checked-out HEAD,
|
||||
check it out and carry there.
|
||||
|
||||
|
||||
@@ -230,9 +230,15 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string,
|
||||
return { ...b, depthLine: d.line, targetedLine };
|
||||
}
|
||||
|
||||
/** receiptTargetedVerdict at the receipt's head, with the classes the queue computes: what write and carry check before they publish (#3854). */
|
||||
/**
|
||||
* receiptTargetedVerdict at the receipt's head, with the classes the queue computes: what write and carry check before they
|
||||
* publish (#3854). Classes that failed closed on an input this checkout cannot read (no origin base, no gate scan) are reported,
|
||||
* never refused on here: the queue reads its own inputs at ready and refuses there if the head needs the reader.
|
||||
*/
|
||||
export function receiptTargetedAtHead(root: string, baseRef: string, r: ShipReceipt, head: string): DepthVerdict {
|
||||
const at = headInputs(root, baseRef, head);
|
||||
const gateTagged = boundScan(r, head) ?? (gateScanNeeded(at.c.paths, at.c.root) ? null : 0);
|
||||
return receiptTargetedVerdict(targetedClasses({ ...at.c, scope: at.scope, gateTagged }).classes, r);
|
||||
const t = targetedClasses({ ...at.c, scope: at.scope, gateTagged });
|
||||
const v = receiptTargetedVerdict(t.classes, r);
|
||||
return v.ok || !t.errors.length ? v : { ok: true, line: `${v.line} (unchecked: ${t.errors.join('; ')}; the queue decides at ready)` };
|
||||
}
|
||||
@@ -8,7 +8,9 @@ import { describe, test, expect } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import { createHash } from 'crypto';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { sectionTexts } from '../lib/review-brief';
|
||||
|
||||
const ROOT = path.join(import.meta.dir, '..');
|
||||
const REMOVED = [
|
||||
@@ -57,12 +59,13 @@ const NO_LOSS_REPLAYS = {
|
||||
'L': "GSTA-167 variant G Arm L: live no-loss replay of one batched reader (testing and security plus scoped checklists) and the Red Team on its findings, 2 passes, scored L∘G1 against G1 on the paired rule (part 2)",
|
||||
'GST-257': "GST-257 PR 1 dispatch-superset no-loss check (test/review-targeted.test.ts): on every fixture diff the branch dispatches main's reviewers at 8be7f21 unchanged plus only the targeted reader, and over the last 100 non-release commits on main it lost none; adds the targeted role and brief to the G1 and L bytes, whose existing reviewer text is unchanged",
|
||||
'GST-850': "GST-850 dispatch-superset no-loss check: on the 26 GSTA-167 no-loss units the branch plan (wider security and money classes, write-up text in the batched diff) drops no reviewer role main dispatches, so no saved blocker can be lost, and over the last 100 main commits no commit with a gate or risk class drops depth (test/review-risk-routing.test.ts, test/review-risk-linear.test.ts); reviewer text is unchanged",
|
||||
'GST-986': "GST-986 brief-identity no-loss check (the brief-body test below): every reviewer brief lib/review-brief.ts cuts from ship/sections/adversarial.md is byte-identical to the bodies at the GST-257 pin; only the lead's Step 11 instruction naming the targeted record's path changed, outside the cut spans",
|
||||
'R51': "R51 risk-tier routing no-loss re-score (GSTA-167): the saved panel findings (349 records over 24 of the 26 no-loss units) re-scored against the branch's plan lose no blocker whose only finders are roles it drops, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel (test/review-targeted.test.ts, test/review-risk-routing.test.ts); reviewer text is unchanged",
|
||||
} as const;
|
||||
type Replay = keyof typeof NO_LOSS_REPLAYS;
|
||||
const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
|
||||
'ship/sections/review-army.md': { sha: 'b7ca291a1cc63750bfb9710a4c12fd971523b9598da2a32ef80fb258eb0eceab', replay: 'L' },
|
||||
'ship/sections/adversarial.md': { sha: 'd776ee5bc6e0c13930f8f4de05bad2f1ed9e52bc9fc5df80ae3ba7680455b2bb', replay: 'GST-257' },
|
||||
'ship/sections/adversarial.md': { sha: 'b70577b6987869acd76cc3865f16eeeed743558c0ea09e5f9933d0d8aec8b91e', replay: 'GST-986' },
|
||||
'review/sections/review-army.md': { sha: '78329b64640260cd8831ca81a9189f3f8dcade9c406431631d3f2cd798cd3967', replay: '#3638' },
|
||||
'review/sections/adversarial.md': { sha: 'e6962fa25b78f362654f2954f79683e524d2ebd452d1494f8a2b099888e8e255', replay: 'G1' },
|
||||
'review/checklist.md': { sha: '607788afeb2c83f2dff1dccd58d212616ec72393d28cda2fa18690c1a83df699', replay: '#3638' },
|
||||
@@ -83,7 +86,34 @@ const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
|
||||
'lib/review-risk.ts': { sha: '18f0330ddfda0032eb93516b0eb941fa4244e0a68b028e19c436c74609093b03', replay: 'GST-850' },
|
||||
};
|
||||
|
||||
/** The reviewer brief bodies lib/review-brief.ts cuts out of the ship sections, at the bytes replays L and GST-257 admitted. */
|
||||
const BRIEF_BODIES: Record<string, string> = {
|
||||
specialist: '1ebe47ddb3291a9e4fe90b5a260eb57e328144959549bc95c119b68d23652cb5',
|
||||
framing: 'aad667b5ea4c6d23d930dafffa38178c04be388cd4364fb73759fc92af17bb63',
|
||||
fixtures: '9517c86befe773e76acbf30a0642ed828e211b8ecb524325702072b26429ef16',
|
||||
redTeam: '1cca7957d13f9da08434870290b92958f37750207047b92c9aa52a802da789e8',
|
||||
native: 'ddb1041ec1e4dd373c3e4ab9017e49316e78c230055c1ef3db1bbba518b143a2',
|
||||
outside: 'bd96406da28b03742384560425a333b6d6d7ed2be95bd2026c90e0039632cee2',
|
||||
};
|
||||
|
||||
describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => {
|
||||
// Value: protects=the GST-986 repin of ship/sections/adversarial.md admits only a lead-instruction edit: every brief a reviewer
|
||||
// reads renders byte-identical to the no-loss-tested bodies; fails_when=an edit inside a cut span changes a brief while the
|
||||
// section pin moves; seam=sectionTexts, the function every render uses; no_claim=the checklists (pinned by file hash below)
|
||||
test('every reviewer brief cut from the ship sections is byte-identical to its pinned body', () => {
|
||||
const hashes = (root: string) => Object.fromEntries(Object.entries(sectionTexts(root)).map(([k, v]) => [k, createHash('sha256').update(v).digest('hex')]));
|
||||
expect(hashes(ROOT)).toEqual(BRIEF_BODIES);
|
||||
const copy = fs.mkdtempSync(path.join(os.tmpdir(), 'brief-bodies-'));
|
||||
try {
|
||||
fs.cpSync(path.join(ROOT, 'ship', 'sections'), path.join(copy, 'ship', 'sections'), { recursive: true });
|
||||
const adversarial = path.join(copy, 'ship', 'sections', 'adversarial.md');
|
||||
fs.writeFileSync(adversarial, fs.readFileSync(adversarial, 'utf8').replace('Recommendation:', 'Recommendation (short):'));
|
||||
expect(hashes(copy)).not.toEqual(BRIEF_BODIES);
|
||||
} finally {
|
||||
fs.rmSync(copy, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('every pinned reviewer brief, checklist and depth rule hashes to its pin', () => {
|
||||
const drift = Object.entries(REVIEWER_BRIEFS).filter(([file, pin]) => {
|
||||
const text = fs.readFileSync(path.join(ROOT, file), 'utf8').replace(/\r\n/g, '\n');
|
||||
|
||||
@@ -19,6 +19,8 @@ afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
|
||||
const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!;
|
||||
const writeReceipt = (repo: FixtureRepo, body: string, extra: string[] = []) =>
|
||||
runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body, ...extra]);
|
||||
/** A done targeted record read at HEAD on every class: since GST-986, write refuses a money, gate, auth or secret head without one. */
|
||||
const targetedAtHead = (repo: FixtureRepo) => ['--review-targeted', JSON.stringify({ classes: ['money', 'gate', 'auth', 'secret'], rules: 'r', brief_sha: 'a'.repeat(64), status: 'done', findings: 0, raised_by: 'targeted', head: repo.git('rev-parse', 'HEAD') })];
|
||||
const lines = (n: number) => Array.from({ length: n }, (_, i) => `exports.f${i} = () => ${i};`).join('\n') + '\n';
|
||||
const rewrite = (body: string, edit: (r: Record<string, any>) => void) => {
|
||||
const text = fs.readFileSync(body, 'utf8');
|
||||
@@ -62,7 +64,7 @@ describe('the queue refuses a receipt below the depth its head needs', () => {
|
||||
const repo = makeFixtureRepo(); repos.push(repo);
|
||||
const head = commit(repo, { 'lib/billing.js': lines(80) });
|
||||
const body = path.join(repo.root, 'body.md');
|
||||
expect(writeReceipt(repo, body).status).toBe(EXIT.ok);
|
||||
expect(writeReceipt(repo, body, targetedAtHead(repo)).status).toBe(EXIT.ok);
|
||||
expect(receiptIn(body).review_depth).toMatchObject({ depth: 'full', runtime_lines: 80, risk: ['money'] });
|
||||
const base = repo.git('rev-parse', 'origin/main').slice(0, 12);
|
||||
const bind = (text: string) => bindShippedReceipt(repo.work, 'origin/main', text, '7', [head]);
|
||||
@@ -100,7 +102,7 @@ describe('the queue refuses a receipt below the depth its head needs', () => {
|
||||
const repo = makeFixtureRepo(); repos.push(repo);
|
||||
const head = commit(repo, { 'lib/auth.js': lines(3) });
|
||||
const body = path.join(repo.root, 'body.md');
|
||||
expect(writeReceipt(repo, body).status).toBe(EXIT.ok);
|
||||
expect(writeReceipt(repo, body, targetedAtHead(repo)).status).toBe(EXIT.ok);
|
||||
expect(receiptIn(body).review_depth).toMatchObject({ depth: 'full', class: 'gate' });
|
||||
const bind = (text: string) => bindShippedReceipt(repo.work, 'origin/main', text, '7', [head]);
|
||||
const light = bind(rewrite(body, r => { r.review_depth.depth = 'light'; }));
|
||||
|
||||
Reference in new issue
Block a user