mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
fix: GST-986 bind a targeted record only to the product patch the reader read
The targeted gate reader at 0715327 found that the classifier-only binding
rule kept a stale record bound when a post-read edit removed an enforcing
line with no money token. Restore main's equal-product-patch check and keep
the gate-scan refusal on top of it. Docs-only binding moves to GST-1031.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
0715327524
commit
cfda4e868d
4 files changed
+33
-17
No files matched your search
@@ -1,6 +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 nothing since that commit carries a money, gate, auth or secret change (a test edit that skips a case counts as gate), so a record can no longer vouch for targeted code it never read, and the docs and TODOS.md commits /ship makes after the read still bind.
|
||||
- **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 is the receipt's and nothing since it carries a money, gate, auth or secret change (a test edit that skips a case counts as gate), so a record can no longer vouch for code it never read.
|
||||
- **`write` and `carry` refuse a receipt the queue refuses (#3854, #3859).** 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.
|
||||
@@ -404,10 +404,10 @@ renders its brief; `verify --targeted-out .gstack/tmp/targeted.json` writes the
|
||||
(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 is an ancestor of HEAD and nothing changed after it carries a targeted class, with
|
||||
the gate scan run over that range: a money, gate, auth or secret change after the read refuses
|
||||
with `RECEIPT_INVALID`, a test edit the gate scan tags (a skipped case) included, while a
|
||||
version, docs, TODOS.md (Steps 14 and 14.5) or untagged test commit still binds. Every verify
|
||||
that commit is an ancestor of HEAD, its product patch is the receipt's, and nothing changed
|
||||
after it carries a targeted class, with the gate scan run over that range: any product change
|
||||
after the read refuses with `RECEIPT_INVALID`, docs and TODOS.md commits included, and so
|
||||
does a test edit the gate scan tags (a skipped case). Every verify
|
||||
removes `--targeted-out` before it writes a record, so an earlier run's record never reaches
|
||||
Step 19. `write` and `carry` (which rescans the gate at the new head) also
|
||||
refuse a receipt whose head needs a class no `done` record covers (`RECEIPT_INVALID`,
|
||||
|
||||
@@ -133,10 +133,10 @@ export type ReviewTargeted = NonNullable<ShipReceipt['review_targeted']>;
|
||||
|
||||
/**
|
||||
* `write --review-targeted` input (gstack-review-dispatch verify --targeted-out), bound to the receipt's product patch-id here.
|
||||
* `head` is the commit the reader read; `productPatchOf` gives that commit's product patch-id by the receipt's own rules, and the
|
||||
* record binds when `classesSince` reads no targeted class in what changed after the read (the docs and TODOS.md /ship commits
|
||||
* after Step 11; a test change the gate scan tags is gate class though the product patch leaves tests out), so a record never
|
||||
* vouches for targeted-class work it did not read (GST-986); without `classesSince` it binds only on an equal product patch.
|
||||
* `head` is the commit the reader read; `productPatchOf` gives that commit's product patch-id by the receipt's own rules. The
|
||||
* record binds only on an equal product patch, and with `classesSince` also only when nothing changed after the read carries a
|
||||
* targeted class (a test change the gate scan tags is gate class though the product patch leaves tests out), so a record never
|
||||
* vouches for work it did not read (GST-986).
|
||||
*/
|
||||
export function parseReviewTargeted(value: unknown, productPatchId: string, productPatchOf: (head: string) => string | undefined, classesSince?: (read: string) => readonly TargetedClass[] | null): { ok: true; targeted: ReviewTargeted } | { ok: false; message: string } {
|
||||
const bad = (message: string) => ({ ok: false as const, message });
|
||||
@@ -150,7 +150,7 @@ export function parseReviewTargeted(value: unknown, productPatchId: string, prod
|
||||
if (typeof v.findings !== 'number' || !Number.isInteger(v.findings) || v.findings < 0) return bad('review_targeted.findings must be an integer >= 0');
|
||||
if (typeof v.head !== 'string' || !/^[0-9a-f]{40,64}$/.test(v.head)) return bad('review_targeted.head must be the commit the targeted reader read (rerun gstack-review-dispatch verify --targeted-out with this gstack)');
|
||||
const read = productPatchOf(v.head);
|
||||
if (classesSince ? classesSince(v.head)?.length !== 0 : read !== productPatchId) return bad(`the targeted reader read ${v.head.slice(0, 12)}, whose product patch ${read?.slice(0, 12) ?? 'cannot be read'} is not this receipt's ${productPatchId.slice(0, 12)}: the product changed after the read, so run the targeted reader on HEAD and verify again`);
|
||||
if (read !== productPatchId || (classesSince && classesSince(v.head)?.length !== 0)) return bad(`the targeted reader read ${v.head.slice(0, 12)}, whose product patch ${read?.slice(0, 12) ?? 'cannot be read'} is not this receipt's ${productPatchId.slice(0, 12)}: the product changed after the read, so run the targeted reader on HEAD and verify again`);
|
||||
const classes = TARGETED_CLASSES.filter(c => (v.classes as string[]).includes(c));
|
||||
return { ok: true, targeted: { classes, rules: v.rules as string, brief_sha: v.brief_sha, product_patch_id: productPatchId, status: v.status as ReviewTargeted['status'], findings: v.findings, raised_by: (v.raised_by as string).trim() } };
|
||||
}
|
||||
|
||||
@@ -150,11 +150,10 @@ describe('write binds a targeted record only to the product patch the reader rea
|
||||
expect(old.err).toContain('review_targeted.head');
|
||||
});
|
||||
|
||||
// Value: protects=/ship's Steps 14 and 14.5 commit TODOS.md and docs after the Step 11 read, and those commits carry no targeted
|
||||
// class, so the record still binds (pass 1 Red Team CRITICAL on this PR: every money, gate, auth or secret ship whose doc audit
|
||||
// edited a file stopped at Step 19 with RECEIPT_INVALID); a money edit after the read is still refused; fails_when=write
|
||||
// refuses a docs-only change after the read or accepts a class-bearing one; seam=gstack-ship-receipt write --review-targeted
|
||||
test('a docs and TODOS.md commit after the read binds; a money edit after it is refused', () => {
|
||||
// Value: protects=a record binds only to the product patch the reader read, so a docs or TODOS.md commit after the read (which
|
||||
// changes the product patch) is refused like a money edit; binding docs-only commits is planned under GST-1031 with a full
|
||||
// review; fails_when=write binds a record whose read product patch is not the receipt's; seam=gstack-ship-receipt write --review-targeted
|
||||
test('a docs and TODOS.md commit after the read is refused; a money edit after it is refused', () => {
|
||||
const { repo, base, head } = moneyBranch();
|
||||
const record = path.join(repo.root, 'targeted.json');
|
||||
expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record).status).toBe(0);
|
||||
@@ -162,8 +161,8 @@ describe('write binds a targeted record only to the product patch the reader rea
|
||||
commit(repo, 'docs/charges.md', '# Charges\n\nThe cap is enforced per run.\n', 'Step 14.5 docs');
|
||||
commit(repo, 'TODOS.md', '# TODOS\n\n- [x] enforce the cap\n', 'Step 14 TODOS');
|
||||
const docs = write(repo, ['--review-targeted', `@${record}`]);
|
||||
expect(docs.status, docs.err).toBe(EXIT.ok);
|
||||
expect(JSON.parse(docs.out).review_targeted).toMatchObject({ classes: ['money'], status: 'done' });
|
||||
expect(docs.status).toBe(EXIT.fail);
|
||||
expect(docs.err).toContain(`the targeted reader read ${head.slice(0, 12)}`);
|
||||
|
||||
commit(repo, 'src/charge.ts', 'export const spend_cap_usd = 50;\n', 'money edit after verify');
|
||||
const stale = write(repo, ['--review-targeted', `@${record}`]);
|
||||
@@ -171,6 +170,23 @@ describe('write binds a targeted record only to the product patch the reader rea
|
||||
expect(stale.err).toContain(`the targeted reader read ${head.slice(0, 12)}`);
|
||||
});
|
||||
|
||||
// Value: protects=a code edit after the read that drops a cap's enforcing line carries no money token, so the classifier tags
|
||||
// nothing, but the product patch the reader read is gone and the record must not bind (targeted gate reader on this PR at
|
||||
// 071532752, CRITICAL lib/ship-receipt-depth.ts:153); fails_when=write binds a record whose read product patch is not the
|
||||
// receipt's; seam=gstack-ship-receipt write --review-targeted
|
||||
test('a code edit after the read that removes an enforcing line with no money token is refused', () => {
|
||||
const { repo, base } = moneyBranch();
|
||||
commit(repo, 'src/runner.ts', 'export function run(spent: number, limit: number) {\n if (spent > limit) throw new Error("over");\n}\n', 'enforce the limit');
|
||||
const head = repo.git('rev-parse', 'HEAD');
|
||||
const record = path.join(repo.root, 'targeted.json');
|
||||
expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record).status).toBe(0);
|
||||
expect(write(repo, ['--review-targeted', `@${record}`]).status).toBe(EXIT.ok);
|
||||
commit(repo, 'src/runner.ts', 'export function run(spent: number, limit: number) {\n}\n', 'drop the enforcing line');
|
||||
const dropped = write(repo, ['--review-targeted', `@${record}`]);
|
||||
expect(dropped.status).toBe(EXIT.fail);
|
||||
expect(dropped.err).toContain(`the targeted reader read ${head.slice(0, 12)}`);
|
||||
});
|
||||
|
||||
// Value: protects=a test edit after the read that skips a case is gate-class work the reader never saw, so the record does not
|
||||
// bind though the product patch (which leaves tests out) is unchanged (round 2 outside voice and adversarial on this PR);
|
||||
// a record whose read commit is not an ancestor of the head is refused; fails_when=write binds either;
|
||||
|
||||
Reference in new issue
Block a user