mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-13 20:07:08 +01:00
Merge remote-tracking branch 'origin/main' into gsta161/opus-lite-loadout
This commit is contained in:
commit
3eb2b5f25d
16 files changed
+362
-54
No files matched your search
+43
@@ -0,0 +1,43 @@
|
||||
Adversarial review (native) of `1b68e637c07c..0a8efed`. HEAD is still `0a8efeda8f79`, the working tree is clean and the same 10 files changed, so the findings below are the ones I reported earlier, rechecked against this range.
|
||||
|
||||
**Coverage:** I read the full diffs of the non-fixture source: `.github/workflows/{ci,sealed,train-gate}.yml`, `scripts/gate.ts`, `gate/manifest.json`, `package.json` and `README.md`. **I reviewed the test files in summary mode only**, so their coverage of the review is reduced:
|
||||
- `tests/ci/gate.test.ts` is new (+299). It covers the train chain walk, `routeDispatch` refusals, `publishable` binding and the gate-surface rule, and `runCases`/junit judging.
|
||||
- `tests/ci/sealed-workflow.test.ts` (+19/−6) adds the push trigger.
|
||||
- `tests/ci/workflow-hygiene.test.ts` is new (+85). It covers SHA pins, read-only tokens, no secrets, the checksummed Bun install, the gstack pin and parity between the manifest and ci.yml.
|
||||
|
||||
**1. A PR in the train controls the workflow that judges the train (INVESTIGATE).** `workflow_dispatch` runs the copy of `train-gate.yml` at the dispatched ref, `train/<nonce>`, and that copy contains every train PR's edits. So a PR that edits `train-gate.yml` changes the `route`, `test` and `publish` jobs themselves.
|
||||
- It can drop the call to the base's `gate.ts publish`. It can also post the `full_gate_check` check run unconditionally with the publish job's `checks: write`, or give itself more `permissions:` and use repository secrets.
|
||||
- The header says "a pull request in the train cannot change how the train is judged". In practice only the case list and `gate.ts` come from the base.
|
||||
- The "a gate surface PR rides alone" rule (`publishable`, `slots > 1`) is enforced by the publish job, which is code the PR can rewrite. So the rule holds only against PRs that don't break it.
|
||||
- `ci.yml`'s new `workflow_dispatch` has the same property.
|
||||
- Real mitigations would run the judging workflow from the default branch, for example `repository_dispatch` or a dispatch on `main` that takes the train head as an input, or have gstack verify which workflow file produced the check. Either one changes the protocol with gstack.
|
||||
- Today an attacker needs a malicious workflow edit to pass review and READY, and no workflow references a secret.
|
||||
|
||||
Severity: High. Blocks: no.
|
||||
|
||||
**2. A case timeout leaks processes and can lose the record (FIXABLE).** `runCases` calls `spawnSync(..., { timeout })` (`scripts/gate.ts:252`), which kills only the `bash` wrapper. Grandchildren such as `bun test` survive with inherited stdio and keep running into later cases.
|
||||
- A 20-minute case timeout (`CASE_TIMEOUT_MS`) plus the other cases can go past the job's 30-minute `timeout-minutes`.
|
||||
- GitHub then kills `gate.ts` before it writes `train-record.json`. That run has no record and fails as infrastructure, not as a red verdict.
|
||||
- Fix: spawn each case detached and kill its whole process group on timeout, and keep the per-case cap well under the job limit (or size the job to cases × cap).
|
||||
|
||||
Severity: Low. Blocks: no.
|
||||
|
||||
**3. Local `run` hides a broken train chain (FIXABLE).** In `gate.ts run` (`:386`), when `trainBase` returns `{ error }`, the code falls back to `merge-base HEAD origin/<default>` without saying so. A malformed train run locally is judged against the wrong base.
|
||||
- CI always passes `--base`, so this affects only local `bun run gate`.
|
||||
- Fix: fail on an error result, the way `route` and `publish` already do.
|
||||
|
||||
Severity: Low. Blocks: no.
|
||||
|
||||
**4. No train policy is on the base, so trains cannot run yet (INVESTIGATE).** `.gstack/ship-policy.json` does not exist at HEAD. Until a commit with a `train` block lands on main, `route` refuses every train and bisect dispatch with "needs a train block". The workflow cannot gate a train at merge, and this PR does not add the file.
|
||||
- Related: `bun.lock` is not in `gate_surface`. A multi-PR train can change resolved dependency versions within `package.json` ranges, for example `@types/bun ^1.4.4`, which changes what `typecheck` and `unit` run against.
|
||||
|
||||
Severity: Low. Blocks: no.
|
||||
|
||||
**Ruled out:**
|
||||
- `grep -q` after `tee` in the sealed case is not a SIGPIPE risk. `SEALED:` is the last and only line on gstack-pregate's stdout.
|
||||
- The empty wiki `--base` on dispatch is safe: `resolveBase` falls back to `origin/HEAD`.
|
||||
- `Bun.Glob("**/tsconfig*.json")` matches `tsconfig.json` at the repository root.
|
||||
- The bisect artifact name matches gstack's `bisectArtifact`.
|
||||
- `route` refuses when the bash T1 lookup and `trainBase` disagree.
|
||||
|
||||
Recommendation: Ship as-is and send finding 1 to the train-protocol owner because the strongest issue, a train PR rewriting `train-gate.yml` to post `full_gate_check` itself, needs a malicious workflow edit to pass review, and fixing it means changing the dispatch protocol with gstack, not this diff.
|
||||
@@ -0,0 +1,5 @@
|
||||
I couldn't run the review because the diff base doesn't exist. `git diff 1b68e637c07cea38f01c01120e02d184575da82c` fails with "bad object", and fetching that sha from origin is refused with "not our ref", so the commit is neither in this checkout nor on the remote.
|
||||
|
||||
This checkout is on `main` at 047dad7 (v1.91.357.0), tracking origin/main with no uncommitted changes, so there's nothing local to review against either. I'm not returning NO FINDINGS, because that would claim a clean diff I never saw.
|
||||
|
||||
To continue I need one of these: a diff base that exists, or the branch or sha holding the c3-36-p1 change. It may only exist on another machine and was never pushed. I've read all three checklists (testing, security, state-machines) and will apply them as soon as I have a diff.
|
||||
+27
-10
@@ -446,21 +446,21 @@ describe("gstack-post pr-body merges the PR body by section", () => {
|
||||
expect(after.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, Why; kept unclosed receipt fence; added none\n");
|
||||
});
|
||||
|
||||
// Value: protects=a file with sections owns the text above the first heading (GST-412): stale lead text and stale text between the receipt and the
|
||||
// Value: protects=a full post (a receipt above its first heading, as /ship composes it) owns the text above the first heading (GST-412): stale lead text and stale text between the receipt and the
|
||||
// first heading are dropped and named on stderr with their first line, the file's own preamble takes their place, a receipt-only or text-only file
|
||||
// keeps the older merge, a repost is byte-identical and an attribution callout is never doubled; fails_when=pr-body keeps a stale intro above the
|
||||
// receipt or a stale note under it after /ship reposts the sections; why_new=lead text was kept unless the file brought its own; seam=fake run;
|
||||
// no_claim=real gh
|
||||
test("a file with sections replaces every text section above the current first heading and names each dropped piece", () => {
|
||||
test("a full post replaces every text section above the current first heading and names each dropped piece", () => {
|
||||
const stale = withBody("Draft intro from gh pr create, long enough to need truncation in the summary line\nsecond line\n\n" + CURRENT, { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" });
|
||||
expect(run(["pr-body", "5", "--body-file", "/b"], stale)).toBe(EXIT.posted);
|
||||
expect(sent(stale).input).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- new\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: do it\n");
|
||||
expect(stale.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, What changed; kept Why, Approvals; added none; dropped leading text (\"Draft intro from gh pr create, long enough to need truncati…\")\n");
|
||||
|
||||
const between = withBody(RECEIPT("aaaa") + "Old note under the receipt\n\n## What changed\n\n- old\n", { "/b": "## What changed\n\n- new\n" });
|
||||
const between = withBody(RECEIPT("aaaa") + "Old note under the receipt\n\n## What changed\n\n- old\n", { "/b": RECEIPT("bbbb") + "## What changed\n\n- new\n" });
|
||||
expect(run(["pr-body", "5", "--body-file", "/b"], between)).toBe(EXIT.posted);
|
||||
expect(sent(between).input).toBe(RECEIPT("aaaa") + "\n## What changed\n\n- new\n");
|
||||
expect(between.stderr()).toBe("gstack-post: pr-body merged by section: replaced What changed; kept ship receipt; added none; dropped trailing text (\"Old note under the receipt\")\n");
|
||||
expect(sent(between).input).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- new\n");
|
||||
expect(between.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, What changed; kept none; added none; dropped trailing text (\"Old note under the receipt\")\n");
|
||||
|
||||
const own = withBody("Stale intro\n\n" + RECEIPT("aaaa") + "Stale note\n\n## Why\n\nold\n", { "/b": "Fresh intro\n\n" + RECEIPT("bbbb") + "Fresh note\n\n## Why\n\nnew\n" });
|
||||
expect(run(["pr-body", "5", "--body-file", "/b"], own)).toBe(EXIT.posted);
|
||||
@@ -469,6 +469,23 @@ describe("gstack-post pr-body merges the PR body by section", () => {
|
||||
expect(own.stderr()).toBe("gstack-post: pr-body merged by section: replaced leading text, ship receipt, trailing text, Why; kept none; added none\n");
|
||||
});
|
||||
|
||||
// Value: protects=a partial post (no receipt above its first `## ` heading, even with a caption of its own or a receipt further down) keeps
|
||||
// the human intro and the note under the receipt; fails_when=a lane posting only `## Screenshots`
|
||||
// deletes the intro or a Review-loop exception line; why_new=GST-412 made any file with a section own the preamble (GstackMinder,
|
||||
// 2026-10-31: "a partial pr-body post must keep the human intro"); seam=fake run; no_claim=real gh
|
||||
test("a partial post keeps the current intro and the note under the receipt", () => {
|
||||
const partial = withBody("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n", { "/b": "## Screenshots\n\n\n" });
|
||||
expect(run(["pr-body", "5", "--body-file", "/b"], partial)).toBe(EXIT.posted);
|
||||
expect(sent(partial).input).toBe("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n\n## Screenshots\n\n\n");
|
||||
expect(partial.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept leading text, ship receipt, trailing text, What changed; added Screenshots\n");
|
||||
|
||||
for (const file of ["Screenshots below.\n\n## Screenshots\n\n\n", "## Screenshots\n\n\n\n" + RECEIPT("bbbb")]) {
|
||||
const merged = mergeBodySections("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n", file);
|
||||
expect(merged.body).toContain("Review-loop exception: accepted by owner");
|
||||
expect(merged.dropped).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=the dropped-piece excerpt on stderr never repeats a secret the merge removed from the body (the scan only reads the merged bytes);
|
||||
// fails_when=a stale intro holding a credential is echoed to stderr while it leaves the PR; why_new=the excerpt is new in GST-412; seam=fake run; no_claim=real gh
|
||||
test("a dropped piece whose first line the redaction scan flags is named without its text", () => {
|
||||
@@ -504,21 +521,21 @@ describe("gstack-post pr-body merges the PR body by section", () => {
|
||||
});
|
||||
|
||||
// Value: protects=--replace keeps the plain overwrite for a caller that composed the whole body, a current body with no H2 headings is kept as the leading
|
||||
// section by a file without sections and dropped (and named) by a file with sections (GST-412), an empty current body posts the new body byte for byte,
|
||||
// section by a file without sections and dropped (and named) by a full post (GST-412), an empty current body posts the new body byte for byte,
|
||||
// the scan runs on the merged bytes, and an unreadable current body is exit 3 with the --replace hint rather than an overwrite; fails_when=--replace
|
||||
// merges, headingless text is dropped by a receipt-only post or kept under new sections, the merge changes a lone new body, a secret in a kept section
|
||||
// passes the scan, or a failed read overwrites; why_new=first pr-body merge tests; seam=fake run; no_claim=real gh
|
||||
test("--replace overwrites; a body without H2 headings is kept by a file without sections and dropped by one with sections; the merged bytes are what is scanned; a failed read refuses", () => {
|
||||
test("--replace overwrites; a body without H2 headings is kept by a file without a receipt and dropped by a full post; the merged bytes are what is scanned; a failed read refuses", () => {
|
||||
const replaced = withBody(CURRENT, { "/b": "## What changed\n\nonly this\n" });
|
||||
expect(run(["pr-body", "5", "--replace", "--body-file", "/b"], replaced)).toBe(EXIT.posted);
|
||||
expect(sent(replaced).input).toBe("## What changed\n\nonly this\n");
|
||||
expect(replaced.stderr()).toBe("");
|
||||
expect(replaced.calls.some(c => c.cmd === "gh" && c.args[1] === "view")).toBe(false);
|
||||
|
||||
const plain = withBody("Just a paragraph.\nAnother line.", { "/b": "## Tests\n\n- one\n" });
|
||||
const plain = withBody("Just a paragraph.\nAnother line.", { "/b": RECEIPT("bbbb") + "\n## Tests\n\n- one\n" });
|
||||
expect(run(["pr-body", "5", "--body-file", "/b"], plain)).toBe(EXIT.posted);
|
||||
expect(sent(plain).input).toBe("## Tests\n\n- one\n");
|
||||
expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept none; added Tests; dropped leading text (\"Just a paragraph.\")\n");
|
||||
expect(sent(plain).input).toBe(RECEIPT("bbbb") + "\n## Tests\n\n- one\n");
|
||||
expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept none; added ship receipt, Tests; dropped leading text (\"Just a paragraph.\")\n");
|
||||
const plainText = withBody("Just a paragraph.\nAnother line.", { "/b": RECEIPT("bbbb") });
|
||||
expect(run(["pr-body", "5", "--body-file", "/b"], plainText)).toBe(EXIT.posted);
|
||||
expect(sent(plainText).input).toBe(RECEIPT("bbbb") + "\nJust a paragraph.\nAnother line.");
|
||||
|
||||
@@ -195,6 +195,7 @@ export const FREE_FIXTURES: Readonly<Record<string, readonly string[]>> = {
|
||||
'test/fixtures/redact-diagram/**': ['test/redact-context-exemptions.test.ts'],
|
||||
'test/fixtures/remote-identity-vectors.json': ['test/bin-context-windows-slug.test.ts', 'test/gstack-slug-parity.test.ts', 'test/remote-identity-vectors.test.ts'],
|
||||
'test/fixtures/review-depth/**': ['test/review-dispatch-compare.test.ts'],
|
||||
'test/fixtures/review-ledger/**': ['test/review-dispatch.test.ts'],
|
||||
'test/fixtures/review-design-lite-reports-ci-36633323521.json': ['test/review-finalization-budget.test.ts'],
|
||||
'test/fixtures/review-gate-integrity-opus-reports.json': ['test/gate-integrity-templates.test.ts'],
|
||||
'test/fixtures/session-update-heartbeat.sh': ['test/session-update-heartbeat.test.ts'],
|
||||
|
||||
@@ -4,7 +4,7 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { BriefError, promptHash, renderBrief, sectionTexts } from '../lib/review-brief';
|
||||
import { judge, parseFindings, type MergedLine } from '../lib/review-ledger';
|
||||
import { collect, judge, parseFindings, type MergedLine } from '../lib/review-ledger';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const BIN = path.join(ROOT, 'bin', 'gstack-review-dispatch');
|
||||
@@ -326,6 +326,148 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
expect(parseFindings('outside-voice', '1. Retry loses writes (queue.ts:78). Severity: Medium\n Fix: reject the batch. Blocks: yes')).toEqual([{ label: 'CRITICAL', raw: '1. Retry loses writes (queue.ts:78). Severity: Medium | Fix: reject the batch. Blocks: yes' }]);
|
||||
});
|
||||
|
||||
// Value: protects=a reader reply with no finding and no clean marker (a refusal, an empty reply, a reader that never saw the diff) is
|
||||
// recorded as a non-review: collect exits 1 and verify reports the brief NOT_COLLECTED, never clean; fails_when=collect hashes the
|
||||
// GSTA-196 refusal ("I'm not returning NO FINDINGS"), an empty file or an "I could not access the diff" reply as findings=0 and verify
|
||||
// reads MATCH (replay C3-36: the batched specialist found no diff base and its brief read MATCH);
|
||||
// why_new=collect recorded whatever parseFindings returned and verify counted any output as collected; seam=collect and verify;
|
||||
// no_claim=that a reply carrying a marker actually reviewed the diff
|
||||
test('a reply with no finding and no clean marker is a non-review: collect exits 1 and verify is NOT_COLLECTED, never clean', () => {
|
||||
const refusal = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/gsta-196-refusal.txt'), 'utf8');
|
||||
expect(promptHash(refusal)).toBe('7150c3b9ffe2698f77a988fa422b66d7004e1939d12e81185e8d556a47dff784');
|
||||
const b = tmp();
|
||||
const batched = field(run(['render', '--role', 'specialist-batched', '--checklists', 'testing,security,state-machines', '--out', b, '--diff-base', SHA]).stdout, 'file');
|
||||
const cases = [
|
||||
{ reader: 'specialist-batched', dir: b, brief: batched, output: refusal, args: ['--role', 'specialist-batched'] },
|
||||
...['', 'I could not access the diff: git diff failed with "bad object", so there is nothing here to review.\n'].map(output => {
|
||||
const p = pass();
|
||||
return { reader: 'specialist/testing', dir: p.out, brief: p.file, output, args: ['--role', 'specialist', '--name', 'testing'] };
|
||||
}),
|
||||
];
|
||||
for (const c of cases) {
|
||||
const ledger = path.join(c.dir, 'review-ledger.jsonl');
|
||||
const merged = path.join(c.dir, 'merged.jsonl');
|
||||
const out = path.join(c.dir, 'reply.out');
|
||||
fs.writeFileSync(out, c.output);
|
||||
expect(run(['sent', '--ledger', ledger, ...c.args, '--sent', c.brief]).status).toBe(0);
|
||||
const col = run(['collect', '--ledger', ledger, ...c.args, '--output', out, '--merged', merged]);
|
||||
expect([col.status, col.stdout.startsWith(`COLLECT_NOT_REVIEWED: reader=${c.reader} findings=0 sha256=${promptHash(c.output)} `)]).toEqual([1, true]);
|
||||
const event = JSON.parse(fs.readFileSync(ledger, 'utf8').trim().split('\n').at(-1)!);
|
||||
expect([event.event, event.ids, event.review]).toEqual(['output', [], false]);
|
||||
fs.writeFileSync(path.join(c.dir, 'judged.jsonl'), '');
|
||||
expect(run(['judge', '--ledger', ledger, '--merged', merged, '--judged', path.join(c.dir, 'judged.jsonl')]).status).toBe(0);
|
||||
const v = run(['verify', '--ledger', ledger]);
|
||||
expect([v.status, v.stdout.includes(`BRIEF ${c.reader} NOT_COLLECTED rendered=`), v.stdout.includes(`OUTPUT ${c.reader} NOT_REVIEWED`), v.stdout.includes(' not_reviewed\n'), v.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true, true, true]);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a clean reply carrying its section's marker and a reply with findings still collect with exit 0 and verify clean under
|
||||
// the non-review rule; fails_when=the rule makes a real NO FINDINGS specialist or a findings reply dirty or exit 1;
|
||||
// why_new=the non-review rule above could over-reach; seam=collect and verify; no_claim=other roles (the matrix below covers them)
|
||||
test('a NO FINDINGS reply and a findings reply still collect with exit 0 and verify clean', () => {
|
||||
for (const [output, findings] of [['NO FINDINGS\n', 0], ['{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"off by one","specialist":"testing"}\n', 1]] as const) {
|
||||
const p = pass();
|
||||
run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]);
|
||||
const col = run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', p.w('reply.out', output), '--merged', p.merged]);
|
||||
expect([col.status, col.stdout.startsWith(`COLLECT: reader=specialist/testing findings=${findings} `)]).toEqual([0, true]);
|
||||
expect(JSON.parse(fs.readFileSync(p.ledger, 'utf8').trim().split('\n').at(-1)!).review).toBe(true);
|
||||
const judged = p.w('judged.jsonl', findings ? JSON.stringify({ id: 'specialist/testing#1', severity: 'CRITICAL' }) + '\n' : '');
|
||||
expect(run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', judged]).status).toBe(0);
|
||||
const v = run(['verify', '--ledger', p.ledger]);
|
||||
expect([v.status, v.stdout.includes(`OUTPUT specialist/testing MATCH sha=${promptHash(output).slice(0, 12)} findings=${findings}`), v.stdout.includes('VERIFY: clean briefs=1 outputs=1')]).toEqual([0, true, true]);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=each role's zero-finding reply counts as a review only with the marker its own section prescribes (NO FINDINGS for
|
||||
// specialists, the batched specialist, Red Team, fix-check and targeted; the Recommendation: line for the outside voice; either for the
|
||||
// native adversarial voice); fails_when=a marker named inside a sentence, another role's marker or an empty reply reads as clean, or a
|
||||
// prescribed marker reads as a non-review; why_new=no marker check existed; seam=collect; no_claim=that the marker's reason is true
|
||||
test('each role takes only the clean marker its section prescribes', () => {
|
||||
const d = tmp();
|
||||
const outsideClean = 'No actionable findings in the supplied diff. Static review only.\n\nRecommendation: approve because the diff introduces no demonstrated bypass.\n';
|
||||
const cases: Array<[string, string, boolean]> = [
|
||||
['specialist/testing', 'NO FINDINGS\n', true], ['specialist-batched', '`NO FINDINGS`\n', true], ['targeted', 'NO FINDINGS\n', true], ['fix-check', 'NO FINDINGS\n', true],
|
||||
['red-team', 'Fixtures were reviewed in summary mode only.\n\nNO FINDINGS\n', true],
|
||||
['adversarial-native', 'Fixtures were reviewed in summary mode.\n\nRecommendation: Ship as-is because no finding survived tracing\n', true], ['adversarial-native', 'NO FINDINGS\n', true],
|
||||
['outside-voice', outsideClean, true], ['outside-voice', 'NO FINDINGS\n', false],
|
||||
['specialist/testing', 'Recommendation: ship because it is clean\n', false], ['specialist/testing', "I'm not returning NO FINDINGS, because I never saw the diff.\n", false],
|
||||
['red-team', '', false], ['fix-check', 'The fixes look fine to me.\n', false], ['adversarial-native', 'I could not read the diff.\n', false],
|
||||
];
|
||||
for (const [i, [reader, output, review]] of cases.entries()) {
|
||||
const ledger = path.join(d, `l${i}.jsonl`);
|
||||
const r = collect(ledger, reader, output, path.join(d, `m${i}.jsonl`));
|
||||
expect([reader, output, r.lines.length, r.review]).toEqual([reader, output, 0, review]);
|
||||
expect(JSON.parse(fs.readFileSync(ledger, 'utf8').trim()).review).toBe(review);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a paragraph finding's unlabeled detail bullets and the Severity/Blocks line after them stay one finding (GSTA-196
|
||||
// native voice: 4 findings collected as 8, the closing Severity lines opening phantoms judge had to fold); a list item's sibling and
|
||||
// a labeled bullet still start their own findings; fails_when=the sample collects more or fewer than 4, or a labeled bullet merges
|
||||
// away; why_new=every margin list item closed the open finding; seam=parseFindings; no_claim=indented detail lists (already attached)
|
||||
test('unlabeled detail bullets and the Severity line after them stay on the open paragraph finding', () => {
|
||||
const native = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/gsta-196-native.txt'), 'utf8');
|
||||
expect(parseFindings('adversarial-native', native).map(f => [f.label, f.raw.slice(0, 12), f.raw.endsWith('| Severity: ' + (f.label === 'High' ? 'High' : 'Low') + '. Blocks: no.')]))
|
||||
.toEqual([['High', '**1. A PR in', true], ['CRITICAL', '**2. A case ', true], ['CRITICAL', '**3. Local `', true], ['Low', '**4. No trai', true]]);
|
||||
expect(parseFindings('outside-voice', '1. FIXABLE: race in a.ts:3\n- Unlabeled sibling at b.ts:9\nSeverity: Medium').map(f => f.label)).toEqual(['FIXABLE', 'Medium']);
|
||||
expect(parseFindings('outside-voice', '**Retry storm in r.ts:2 (INVESTIGATE).**\n- detail\n- High: a second defect in s.ts:4\nSeverity: Medium').map(f => f.label)).toEqual(['INVESTIGATE', 'High']);
|
||||
expect(parseFindings('outside-voice', 'Low: cache TTL mismatch (cache.ts:4)\n\n2. Token never expires (auth.ts:9)\nSeverity: High\nBlocks: yes').map(f => [f.label, f.raw]))
|
||||
.toEqual([['Low', 'Low: cache TTL mismatch (cache.ts:4)'], ['CRITICAL', '2. Token never expires (auth.ts:9) | Severity: High | Blocks: yes']]);
|
||||
expect(parseFindings('outside-voice', '**Race in a.ts:3 (High).**\n- the writer holds no lock\nSeverity: Medium').map(f => f.raw)).toEqual(['**Race in a.ts:3 (High).** | - the writer holds no lock | Severity: Medium']);
|
||||
expect(parseFindings('outside-voice', '**Race in a.ts:3 (Medium).**\n\n- the writer holds no lock\n\nSeverity: Medium. Blocks: yes').map(f => [f.label, f.raw]))
|
||||
.toEqual([['CRITICAL', '**Race in a.ts:3 (Medium).** | - the writer holds no lock | Severity: Medium. Blocks: yes']]);
|
||||
});
|
||||
|
||||
// Value: protects=a non-review the same dispatch later answers with a review is voided (counted, VERIFY clean), and a failed outside
|
||||
// voice recorded with --unavailable reads UNAVAILABLE (missing coverage, never a review) without blocking, while --unavailable is
|
||||
// refused for any other reader; fails_when=one outside-voice timeout or one refusal makes VERIFY dirty for good, or --unavailable
|
||||
// lets a specialist skip coverage; why_new=#3677 Step 11 adversarial voice; seam=collect and verify; no_claim=the review-log row
|
||||
test('a refusal replaced by a review is voided; an outside voice failure is UNAVAILABLE; --unavailable is the outside voice only', () => {
|
||||
const p = pass();
|
||||
const send = () => run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]);
|
||||
const take = (f: string) => run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', f, '--merged', p.merged]);
|
||||
send();
|
||||
expect(take(p.w('refusal.out', 'I could not access the diff.\n')).status).toBe(1);
|
||||
expect(run(['verify', '--ledger', p.ledger]).stdout).toContain('VERIFY: dirty');
|
||||
send();
|
||||
expect(take(p.w('clean.out', 'NO FINDINGS\n')).status).toBe(0);
|
||||
fs.writeFileSync(path.join(p.out, 'judged.jsonl'), '');
|
||||
run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', path.join(p.out, 'judged.jsonl')]);
|
||||
const v = run(['verify', '--ledger', p.ledger]);
|
||||
expect([v.status, v.stdout.includes('replaced_by_a_review'), v.stdout.includes('VERIFY: clean briefs=1 outputs=2 voided=1')]).toEqual([0, true, true]);
|
||||
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 10_000 }).stdout.trim();
|
||||
const o = tmp();
|
||||
const ov = field(run(['render', '--role', 'outside-voice', '--out', o, '--diff-base', head]).stdout, 'file');
|
||||
const ledger = path.join(o, 'review-ledger.jsonl'), merged = path.join(o, 'merged.jsonl');
|
||||
run(['sent', '--ledger', ledger, '--role', 'outside-voice', '--sent', ov]);
|
||||
const fail = path.join(o, 'timeout.out');
|
||||
fs.writeFileSync(fail, 'Codex timed out after 9 minutes and was terminated; this pass produced NO findings.\n');
|
||||
const c = run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', fail, '--merged', merged, '--unavailable', 'timeout']);
|
||||
expect([c.status, c.stdout.startsWith('COLLECT_UNAVAILABLE: reader=outside-voice ')]).toEqual([0, true]);
|
||||
fs.writeFileSync(path.join(o, 'judged.jsonl'), '');
|
||||
run(['judge', '--ledger', ledger, '--merged', merged, '--judged', path.join(o, 'judged.jsonl')]);
|
||||
const w = run(['verify', '--ledger', ledger]);
|
||||
expect([w.status, w.stdout.includes('BRIEF outside-voice UNAVAILABLE'), w.stdout.includes('OUTPUT outside-voice UNAVAILABLE'), w.stdout.includes(' unavailable=1\n')]).toEqual([0, true, true, true]);
|
||||
const q = pass();
|
||||
run(['sent', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--sent', q.file]);
|
||||
const refused = run(['collect', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--output', q.w('t.out', 'timed out\n'), '--merged', q.merged, '--unavailable', 'timeout']);
|
||||
expect([refused.status, refused.stderr.includes('--unavailable is for the outside voice only')]).toEqual([2, true]);
|
||||
const partial = path.join(o, 'partial.out');
|
||||
fs.writeFileSync(partial, '1. **CRITICAL**: token leak in a.ts:3\nCodex then timed out.\n');
|
||||
const kept = run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', partial, '--merged', merged, '--unavailable', 'timeout']);
|
||||
expect([kept.status, kept.stderr.includes('holds findings; collect it without --unavailable')]).toEqual([2, true]);
|
||||
expect(run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', fail, '--merged', merged, '--unavailable', ' ']).status).toBe(2);
|
||||
const n = pass();
|
||||
const takeN = (f: string) => run(['collect', '--ledger', n.ledger, '--role', 'specialist', '--name', 'testing', '--output', f, '--merged', n.merged]);
|
||||
run(['sent', '--ledger', n.ledger, '--role', 'specialist', '--name', 'testing', '--sent', n.file]);
|
||||
takeN(n.w('refusal.out', 'I could not access the diff.\n'));
|
||||
takeN(n.w('clean.out', 'NO FINDINGS\n'));
|
||||
fs.writeFileSync(path.join(n.out, 'judged.jsonl'), '');
|
||||
run(['judge', '--ledger', n.ledger, '--merged', n.merged, '--judged', path.join(n.out, 'judged.jsonl')]);
|
||||
const unsent = run(['verify', '--ledger', n.ledger]);
|
||||
expect([unsent.status, unsent.stdout.includes(' NOT_REVIEWED '), unsent.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]);
|
||||
});
|
||||
|
||||
test('verify on a ledger that kept no prompts says so and exits 3', () => {
|
||||
const d = tmp();
|
||||
const ledger = path.join(d, 'ledger.jsonl');
|
||||
|
||||
@@ -128,6 +128,7 @@ function repo(): { dir: string; env: NodeJS.ProcessEnv } {
|
||||
|
||||
interface Sub { title: string; prompt: string; status: string; polls: number; model?: unknown; summary?: string; late?: boolean }
|
||||
/** A fake Capy host: the shell is real (the repository's bins), subagents are scripted. */
|
||||
const cleanReply = (title: string) => (title.endsWith('outside-voice') ? 'Recommendation: ship as-is because no finding survived' : 'NO FINDINGS');
|
||||
function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) => string; fail?: (title: string) => boolean; reject?: (title: string) => boolean; createThrows?: (title: string) => 'before' | 'after' | undefined; pollsToFinish?: number; lateSummary?: boolean } = {}) {
|
||||
let clock = 0;
|
||||
const subs = new Map<string, Sub>();
|
||||
@@ -135,12 +136,12 @@ function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string)
|
||||
const starts: number[][] = [];
|
||||
const advance = () => {
|
||||
for (const s of subs.values()) {
|
||||
if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? 'NO FINDINGS'; continue; }
|
||||
if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? cleanReply(s.title); continue; }
|
||||
if (s.status !== 'working') continue;
|
||||
if (++s.polls < (o.pollsToFinish ?? 1)) continue;
|
||||
if (o.fail?.(s.title)) s.status = 'failed';
|
||||
else if (o.lateSummary && !s.late) { s.status = 'done'; s.late = true; }
|
||||
else { s.status = 'done'; s.summary = o.output?.(s.title) ?? 'NO FINDINGS'; }
|
||||
else { s.status = 'done'; s.summary = o.output?.(s.title) ?? cleanReply(s.title); }
|
||||
}
|
||||
};
|
||||
const tools = {
|
||||
|
||||
Reference in new issue
Block a user