From 775fe2737c38013ec02b09161ff3d2d24a69369b Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 1 Nov 2026 14:35:42 -0800 Subject: [PATCH 1/8] =?UTF-8?q?v1.92.43.0=20docs:=20support=20tiers=20?= =?UTF-8?q?=E2=80=94=20full=20for=20macOS=20and=20Linux,=20minimal=20for?= =?UTF-8?q?=20Windows=20and=20Gemini=20(R50)=20(#3812)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: support tiers — full for macOS/Linux with Claude and OpenAI, minimal for Windows and Gemini (R50) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(support-tiers): the Windows suite can also be started by hand Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: Windows CI section says what runs today and what changes once windows-smoke lands 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> --- AGENTS.md | 3 +++ CHANGELOG.md | 8 ++++++++ README.md | 2 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- docs/support-tiers.md | 31 +++++++++++++++++++++++++++++++ package.json | 2 +- 7 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 docs/support-tiers.md diff --git a/AGENTS.md b/AGENTS.md index abe32fea9..febab17af 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -281,6 +281,9 @@ Agents poll `eval:bg:*` logs for the `### gstack-detach EXIT= ###` sentine ## Platform support +Support tiers (full: macOS, Linux; minimal: Windows, Gemini models) are in +[docs/support-tiers.md](docs/support-tiers.md). + - **macOS** + **Linux**: full test suite supported. - **Windows**: the curated Windows-safe subset runs in the `windows-free-tests` CI workflow across six `windows-latest` jobs, packed by Windows-measured diff --git a/CHANGELOG.md b/CHANGELOG.md index c44245e20..c7c6f6500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [1.92.43.0] - 2026-11-01 + +**New page: [docs/support-tiers.md](docs/support-tiers.md) says what gstack supports fully and what it supports minimally.** macOS and Linux, with Claude and OpenAI models, get full support: the whole free suite and the paid evals. Windows (Git Bash or MSYS) and Gemini models get minimal support: install and the core skills (`/ship`, `/review`, `/qa-only`, `/investigate`, `/browse` on gstack's own browser) work without errors, checked on Windows by the `windows-smoke` job, which replaces the six-shard suite as the required Windows check when it lands, and by hand or one cheap smoke run on Gemini. Windows-only and Gemini-only reports that don't break install, startup or a core workflow close as won't fix with a link to the page. + +### Documentation +- `docs/support-tiers.md`: the two tiers, what we stop doing, the Windows CI shape today (the six-shard `windows-free-tests` suite required on every pull request) and once `windows-smoke` lands (`windows-smoke` required, the six-shard suite nightly on main and non-blocking) and what doesn't change (eval models, the cross-family review voice). +- README's platform notes and AGENTS.md's platform support section link to it. + ## [1.92.42.0] - 2026-11-01 **A /ship review pass that only left notes now binds at Step 11.5.** A pass whose findings were all fixed, deferred or noted has no open defect, but a driver could still log it as `issues_found`. Step 11.5 then refused it, and the only way out was an extra rebind pass (CAPY-2, qm#2837). When the tree changes during a pass, the refusal now names the start and end trees and the cheapest repair (GST-430). diff --git a/README.md b/README.md index 011832a81..47fb9c848 100644 --- a/README.md +++ b/README.md @@ -226,6 +226,8 @@ Other references: [docs/gbrain-sync.md](docs/gbrain-sync.md) (sync-specific guid ## Platform notes +macOS and Linux get full support. Windows and Gemini models get minimal support: install and the core skills work, and nothing more is promised. See [support tiers](docs/support-tiers.md). + **macOS:** everything runs. With the [Aside](https://aside.com) browser (macOS 15+) open and signed in, the browser skills, `/make-pdf` and `/diagram` use it and your real sessions; otherwise they use the bundled browser `./setup` builds. gstack never installs Aside. **Linux:** everything runs on the bundled browser; Aside is macOS-only, so the readiness probe says `NEEDS_ASIDE: Linux` once and the skills use gstack's own Chromium. If Chromium installs but cannot launch because the host blocks unprivileged user namespaces (Ubuntu 24.04+ AppArmor default), set `GSTACK_CHROMIUM_NO_SANDBOX=1`. diff --git a/VERSION b/VERSION index bb114e4c2..8cd35b06d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.42.0 +1.92.43.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 0f21b65e0..1c19592b1 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.42.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.43.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/docs/support-tiers.md b/docs/support-tiers.md new file mode 100644 index 000000000..e919aaf4d --- /dev/null +++ b/docs/support-tiers.md @@ -0,0 +1,31 @@ +# Support tiers + +gstack has two support tiers. Full support gets the whole test suite, paid evals and new work. Minimal support means gstack is not totally broken, and nothing more. Garry Tan set this on 2026-11-01: "we don't want it to be totally broken, but we just don't need to invest that much into it." + +## Full support + +- **Operating systems:** macOS and Linux. +- **Models:** Claude and OpenAI models, both as the host agent and as a reviewer. +- **Testing:** the full free suite and the paid evals. + +## Minimal support + +- **Windows** (Git Bash or MSYS): with a default Git for Windows install, `./setup` installs, and the core skills start without errors. The core skills are `/ship`, `/review`, `/qa-only`, `/investigate` and `/browse` on gstack's own browser. Once the `windows-smoke` CI job lands, it checks this on every pull request (see Windows CI below). +- **Gemini models:** the core workflows run to the end without errors when Gemini is the host agent or a reviewer. We check this by hand or with one cheap smoke run, not with an eval cell. + +That is the whole promise. Other skills, optional parts such as `/cso`'s native helper, non-default shell settings and speed are not covered. + +## What we stop doing + +- No Windows-only or Gemini-only features, parity work, tuning or polish. +- No new Windows or Gemini CI jobs or eval cells. Windows CI tuning stops. +- A Windows-only or Gemini-only report that doesn't break install, startup or a core workflow closes as won't fix ([triage rules](triage.md#closing-an-issue)), with a link to this page. A report that does break one of them gets a small fix. + +## Windows CI + +- **Today:** the six-shard Windows free suite (the `Windows Free Tests` workflow, `windows-free-tests`) runs on every pull request and every push to main, and it is a required check. +- **Once `windows-smoke` lands:** `windows-smoke` becomes the required Windows check on pull requests. The six-shard suite then runs nightly on main, or when someone starts it by hand, and never blocks a pull request. A red nightly run gets a fix only when it shows an install, startup or core-workflow break. + +## What doesn't change + +These tiers don't change which models gstack's own evals test, or which cross-family review voice gstack uses. diff --git a/package.json b/package.json index 8acc970b4..46e22f914 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.42", + "version": "1.92.43", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", From 4d388f44bd157f222de54611cb214a9adb237a22 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 1 Nov 2026 14:50:47 -0800 Subject: [PATCH 2/8] v1.92.44.0 fix(pregate): literals stops treating same-stem sibling files as the changed module (GST-820) (#3814) * fix(pregate): literals stops treating a same-stem sibling file as the changed module (GST-820) A path names the module only when its stem is followed by a quote, the line end, or the module's own or a JS/TS extension; test/.txt no longer makes a file a consumer. Red first on main 4f5b043. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(pregate): literals keeps a stem followed by a space, bracket, sentence end or .d.ts as the module (GST-820 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(pregate): literals keeps JS/TS aliases to JS/TS modules and stops matching a file name with a further extension (GST-820 review) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(pregate): literals matches the module only for old values longer than one character (GST-820 docs audit) 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> --- CHANGELOG.md | 7 +++++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- docs/pregate.md | 9 ++++++++- lib/pregate/checks/literals.ts | 14 +++++++++++--- package.json | 2 +- test/pregate-selection.test.ts | 35 ++++++++++++++++++++++++++++++++++ 7 files changed, 64 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7c6f6500..6c67fdace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## [1.92.44.0] - 2026-11-01 + +**`literals` stops treating a same-name sibling file as the changed module (GST-820).** A test that only mentioned `test/postgres-unit-arms.txt` was counted as a consumer of `scripts/postgres-unit-arms.ts`, so any `12` in it failed `PREGATE_LITERALS` when `ARM_SHARDS` moved from 12 to 18. In gbrain #8372 that gave 17 false files. A path now names the module only when its stem is followed by the module's own extension (for a JS/TS module, any JS/TS extension), or by anything other than a letter, digit, `_`, `-` or another extension, and the full file name no longer matches with a further extension such as `.snap`. Extensionless imports, `.js` imports and the module's own path still count. + +### Fixed +- **`literals` module matching.** A sibling file that shares the module's stem (`.txt`, `.json`, `-v2.ts`, `.snap`, or `.ts` beside a shell script) is no longer the module. + ## [1.92.43.0] - 2026-11-01 **New page: [docs/support-tiers.md](docs/support-tiers.md) says what gstack supports fully and what it supports minimally.** macOS and Linux, with Claude and OpenAI models, get full support: the whole free suite and the paid evals. Windows (Git Bash or MSYS) and Gemini models get minimal support: install and the core skills (`/ship`, `/review`, `/qa-only`, `/investigate`, `/browse` on gstack's own browser) work without errors, checked on Windows by the `windows-smoke` job, which replaces the six-shard suite as the required Windows check when it lands, and by hand or one cheap smoke run on Gemini. Windows-only and Gemini-only reports that don't break install, startup or a core workflow close as won't fix with a link to the page. diff --git a/VERSION b/VERSION index 8cd35b06d..fbca8550c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.43.0 +1.92.44.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 1c19592b1..950759039 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.43.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.44.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/docs/pregate.md b/docs/pregate.md index 5d36f49e1..057788ade 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -200,7 +200,14 @@ Two stages with separate clocks, both printed: scratch name (`zz-*`, `tmp*`, `scratch*`) anywhere. - `literals`: for every changed exported constant or named timeout/retry literal, the whole test tree (nightly- and Heavy-only files included) is - searched for the old value; a consumer the branch did not update fails with + searched for the old value in files that name the constant or, when the old + value is longer than one character, its module (the module's file name not + followed by another extension, or a path ending + in its stem followed by its own extension, for a JS/TS module also any JS/TS + extension or `.d.ts`, a sentence-ending `.`, or anything but a word + character, `.` or `-`; so a sibling `.txt`, `-v2.ts` or + `.snap`, or `.ts` beside a shell module, is not the module); a + consumer the branch did not update fails with `file:line`. A version-history row, a line that starts a bracketed row with the old value (`[45, ""]`), is skipped when the same table (a run of consecutive lines starting with `[`) also holds a row for the new value that diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index a769626e4..d3158c481 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -61,8 +61,12 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) : /** * Test files that still carry the old literal beside the constant's name or - * its module. The module counts as its full file name (`install.sh`) or as a - * path fragment (`/retry.js`, `/retry'`): a bare stem such as `install` or `retry` is an + * its module. The module counts as its full file name (`install.sh`, not + * `install.sh.tmpl`) or as a path fragment (`/retry.js`, `/retry'`) whose stem is + * followed by the module's own extension (for a JS/TS module also any JS/TS + * extension or `.d.ts`), a sentence-ending `.`, or anything but a word + * character, `.` or `-`, so a sibling `/retry.txt`, `/retry-v2.ts` or + * `/retry.ts.snap`, or `lib/install.ts` for `install.sh`, is not the module (GST-820): a bare stem such as `install` or `retry` is an * English word in half the test tree, and a one-character old value (`2`) * sits in every test, so that value needs the constant's name (a wrapper * version 2 → 3 flagged 575 consumers through the stem alone). @@ -83,7 +87,11 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan const newRowRe = row(change.new); const base = path.basename(change.file); const stem = base.replace(/\.[^.]+$/, ''); - const moduleRe = new RegExp(`(?:(?:^|[/\\\\])${escape(stem)}|(? { { name: 'MAX', old: '3', new: '4', file: 'f.ts' }, { name: 'timeout', old: '5000', new: '9000', file: 'f.ts' }, ]); }); + // Value: protects=a sibling file that shares the module's stem (`postgres-unit-arms.txt` beside `postgres-unit-arms.ts`) is not the + // module, so a test that only names it is not a consumer, while an extensionless import, a JS/TS import extension, the module's own + // file name and a path ending at the stem (then a quote, space, bracket, sentence end or `.d.ts`) still are, and neither `.snap` nor a JS twin of a + // shell module is; fails_when=`/` again accepts any following `.`, the file name accepts a further extension, + // a non-JS module takes JS/TS extensions, or stops accepting a following space, bracket or sentence end + // (GST-820: 17 false files in gbrain #8372 on ARM_SHARDS 12 -> 18); why_new=no test had a same-stem sibling; seam=none; + // no_claim=name-gated hits (a file naming ARM_SHARDS still counts any 12) + test('consumersOf: a sibling file sharing the module stem is not the module; real imports of it still are (GST-820)', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-stem-')); + const nl = String.fromCharCode(10); + const files: Record = { + 'test/a.test.ts': ['// listed in test/postgres-unit-arms.txt', 'expect(n).toBe(12);', ''].join(nl), + 'test/json.test.ts': ["const arms = read('fixtures/postgres-unit-arms.json');", 'expect(arms.length).toBe(12);', ''].join(nl), + 'test/longer.test.ts': ["run('scripts/postgres-unit-arms-v2.ts');", 'expect(n).toBe(12);', ''].join(nl), + 'test/import.test.ts': ["import { shards } from '../scripts/postgres-unit-arms';", 'expect(shards()).toBe(12);', ''].join(nl), + 'test/import-js.test.ts': ["import { shards } from '../scripts/postgres-unit-arms.js';", 'expect(shards()).toBe(12);', ''].join(nl), + 'test/own.test.ts': ["const out = run('scripts/postgres-unit-arms.ts');", 'expect(out).toBe(12);', ''].join(nl), + 'test/line-end.test.ts': ['// covers scripts/postgres-unit-arms', 'expect(n).toBe(12);', ''].join(nl), + 'test/prose.test.ts': ['// covers scripts/postgres-unit-arms and more', 'expect(n).toBe(12);', ''].join(nl), + 'test/paren.test.ts': ['// (scripts/postgres-unit-arms)', 'expect(n).toBe(12);', ''].join(nl), + 'test/sentence.test.ts': ['// see scripts/postgres-unit-arms.', 'expect(n).toBe(12);', ''].join(nl), + 'test/dts.test.ts': ["import type { Arms } from '../scripts/postgres-unit-arms.d.ts';", 'expect(n).toBe(12);', ''].join(nl), + 'test/sibling-test.test.ts': ["run('scripts/postgres-unit-arms.test.ts');", 'expect(n).toBe(12);', ''].join(nl), + 'test/snap.test.ts': ["read('scripts/__snapshots__/postgres-unit-arms.ts.snap');", 'expect(n).toBe(12);', ''].join(nl), + 'test/sh-twin.test.ts': ["import { arms } from '../lib/arms-tool.ts';", 'expect(n).toBe(12);', ''].join(nl), + 'test/sh-own.test.ts': ["run('scripts/arms-tool.sh');", 'expect(n).toBe(12);', ''].join(nl), + }; + for (const [rel, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), text); } + expect(consumersOf(root, Object.keys(files), { name: 'ARM_SHARDS', old: '12', new: '18', file: 'scripts/postgres-unit-arms.ts' })).toEqual([ + 'test/import.test.ts:2', 'test/import-js.test.ts:2', 'test/own.test.ts:2', 'test/line-end.test.ts:2', + 'test/prose.test.ts:2', 'test/paren.test.ts:2', 'test/sentence.test.ts:2', 'test/dts.test.ts:2', + ]); + expect(consumersOf(root, ['test/sh-twin.test.ts', 'test/sh-own.test.ts'], { name: 'ARMS', old: '12', new: '18', file: 'scripts/arms-tool.sh' })).toEqual(['test/sh-own.test.ts:2']); + fs.rmSync(root, { recursive: true, force: true }); + }); test('consumersOf: a module stem is a path fragment, never a bare word, and a one-character old value needs the constant name (GSTA-114 flood: install.sh 2 -> 3 flagged 575 consumers)', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-')); const nl = String.fromCharCode(10); From 589ff24286c8864838584b61ed5b9c517345d580 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 1 Nov 2026 15:05:58 -0800 Subject: [PATCH 3/8] v1.92.45.0 /ship review scales to risk, with a 2-round cap (R51) (#3820) * review: /ship depth scales to risk, not size; two rounds is the cap (R51, GSTA-167) Non-risky code is light at any size; a risk class (money, auth, secret, security, user data) or gate code makes it full. UI or game text counts as docs. A fix-check counts as a round and round 2 never earns a third. The receipt records review_loop.rounds and review_depth.risk. Repins lib/review-depth.ts and pins lib/review-risk.ts under no-loss replay R51 (test/review-packets-removed.test.ts): the saved panel findings re-scored against this plan lose no blocker, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes (round 2) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * review: UI text and SVGs stay code; a round-2 gate edit stops for the owner; SQL deletes match in any case (R51, Captain choice A) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: restore review-policy.md tail; keep uppercase DELETE FROM (R51 repair, Captain grant) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: R51 fragment matches the final change Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: ship-policy review-loop cap says two rounds (Step 14.5 audit) 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> --- CHANGELOG.md | 10 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-review-plan | 3 +- docs/review-policy.md | 59 +++++++--- docs/ship-policy.md | 8 +- lib/headless-artifacts.ts | 8 +- lib/review-depth.ts | 52 +++++---- lib/review-risk.ts | 85 ++++++++++++++ lib/review-scope.ts | 3 + lib/review-targeted.ts | 36 +----- lib/ship-budget.ts | 2 +- lib/ship-receipt-depth.ts | 4 +- lib/ship-receipt.ts | 3 +- package.json | 2 +- ship/SKILL.md | 4 +- ship/SKILL.md.tmpl | 4 +- test/fixtures/golden/agy-ship-SKILL.md | 4 +- test/fixtures/golden/claude-ship-SKILL.md | 4 +- test/fixtures/golden/codex-ship-SKILL.md | 4 +- test/fixtures/golden/factory-ship-SKILL.md | 4 +- test/review-depth.test.ts | 65 +++++++---- test/review-packets-removed.test.ts | 4 +- test/review-pass.test.ts | 2 +- test/review-plan-cli.test.ts | 18 +-- test/review-risk-routing.test.ts | 123 +++++++++++++++++++++ test/review-scope-followups.test.ts | 2 +- test/review-scope.test.ts | 6 +- test/review-targeted.test.ts | 34 ++++-- test/ship-frozen-scope-actor.test.ts | 2 +- test/ship-receipt-depth.test.ts | 18 +-- 31 files changed, 430 insertions(+), 147 deletions(-) create mode 100644 lib/review-risk.ts create mode 100644 test/review-risk-routing.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c67fdace..c7a2763c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [1.92.45.0] - 2026-11-01 + +### /ship review scales to risk, not size + +- **Review depth follows risk.** Code that touches security, user data, money, secrets, auth or the merge checks gets the full review panel. Other code goes to the light tier at any size, with one batched reader plus the native adversarial pass. Docs, tests and eval write-ups get one reader. UI text and SVG files stay code for now. A larger diff never raises the depth by itself. +- **New risk classes.** `lib/review-risk.ts` holds the money, auth and secret matchers the targeted pass already used, plus new security (trust boundaries, shell and HTML sinks, shells run through argv, recursive deletes, `eval`) and user-data (persistence paths, `DELETE FROM`, `deleteMany`, `destroy_all`) signals. +- **At most 2 review rounds.** A fix-check counts as a round. A blocker that is still open or fixed in round 2 stops for the owner, who may grant one scoped fix-check; there is no round 3. A fix in round 2 that edits gate code also stops for the owner. Below-floor fixes in round 2 finish, with the notes listed. +- **Receipts record tier and rounds.** `review_loop.rounds` and `review_depth.risk` are on the receipt, and the summary line prints `rounds=`. `gstack-ship-queue ready` still refuses a receipt below its head's depth. +- Replayed on the 26-unit no-loss panel, no historical blocker was lost. Over the last 100 main commits (an estimate from replaying the depth rules), 8 go from standard to light, 12 go up to full, and no commit with a risk class goes down. + ## [1.92.44.0] - 2026-11-01 **`literals` stops treating a same-name sibling file as the changed module (GST-820).** A test that only mentioned `test/postgres-unit-arms.txt` was counted as a consumer of `scripts/postgres-unit-arms.ts`, so any `12` in it failed `PREGATE_LITERALS` when `ARM_SHARDS` moved from 12 to 18. In gbrain #8372 that gave 17 false files. A path now names the module only when its stem is followed by the module's own extension (for a JS/TS module, any JS/TS extension), or by anything other than a letter, digit, `_`, `-` or another extension, and the full file name no longer matches with a further extension such as `.snap`. Extensionless imports, `.js` imports and the module's own path still count. diff --git a/VERSION b/VERSION index fbca8550c..603bff1c3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.44.0 +1.92.45.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 950759039..d00c52753 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.44.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.45.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 4e5536d6e..93f6aee5e 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -56,7 +56,8 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do reader last read it: the same patch skips it; any other id, none, or uncommitted edits dispatch it again --fix-check-prompt print the fix-check reviewer's prompt from --fix-list, --hunks, --tests and --callers files -Depth: light (runtime lines <= 50), standard (above 50), full (gate class or any DEPTH_ERROR). TARGETED: the money, +Depth (R51, risk not size): full on gate class, a risk class (money, auth, secret, security, data) or any DEPTH_ERROR; +standard only by --depth-floor or a raise; otherwise light. TARGETED: the money, gate, auth and secret classes the diff touches; any class dispatches the targeted reader at every depth. NEXT: full, scoped (a full pass on the range: fix-check, Red Team, native adversarial), fix-check, stop or done. A full plan from an error exits 0; exit 1 means no plan could be built (git could not list the diff). diff --git a/docs/review-policy.md b/docs/review-policy.md index 6895500fa..1d62b14de 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -93,11 +93,14 @@ incomplete coverage, which blocks until the reviewer is rerun. for every reviewer (the gate scan and QA probes keep the whole candidate) and classifies with `--range-base`. Only a blocking finding in that range or in the diff is `fix`; every below-floor finding is a `note`. -3. **Two full passes is the cap**, plus at most one fix-check; a scoped pass - ([below](#scoped-pass)) is a full pass. A blocking - finding still open after the last allowed step stops the run and names the - finding (path, severity, problem) for the owner; a third full pass is the - owner's call, never the loop's. The cap approves nothing. +3. **Two rounds is the cap** (R51): a full pass, a scoped pass + ([below](#scoped-pass)) or a fix-check is a round. After round 2 the run + finishes with its open notes listed in the PR body and receipt, or stops for + the owner when a blocking finding is open, or was fixed in round 2 and is + unverified, or a round-2 fix edited a gate-class path, naming it (path, + severity, problem). A third round is the owner's + grant of one scoped fix-check (recorded in `review_loop_exception`), never the + loop's. The cap approves nothing. ### Transitions @@ -112,10 +115,12 @@ incomplete coverage, which blocks until the reviewer is rerun. | a full pass that fixed a blocking finding, below the cap, on a diff below full depth with a fix range of at most 150 runtime lines | scoped pass on the range | | a full pass that fixed a blocking finding, below the cap (full depth, a larger range, or a range or depth it could not read) | full pass on the range | | a full pass whose fix touched a gate-class path, below the cap | full pass on the range | -| a full pass that fixed only below-floor findings | one fix-check | -| a full pass at the cap that fixed a blocking finding | one fix-check | +| a full pass that fixed only below-floor findings | one fix-check (round 2) | +| a light pass 1 whose below-floor fixes change no runtime lines | done | +| round 2 (a full pass or a fix-check) that fixed a blocking finding or edited a gate-class path | stop: name it for the owner, who may grant one scoped fix-check | +| round 2 (a full pass or a fix-check) that fixed only below-floor findings | done: notes in the PR body | | a full pass that leaves a blocking finding open (fixed or not) | stop: name it for the owner | -| a fix-check that found nothing blocking (below-floor fixes before it included) | done | +| a fix-check that found nothing blocking | done | | a fix-check that found a blocking finding | stop: name it for the owner | A **fix-check** is one reviewer reading the touched hunks, the fix list, the @@ -186,10 +191,27 @@ in a data file still schedules the outside voice. | Depth | When | Pass 1 dispatches | |---|---|---| -| **light**, runtime 0 | docs and tests only, or only registered outputs | the core checklist only; the outside voice when the diff publishes numbers | -| **light**, runtime 1–50 | `SMALL_DIFF_LINES` or fewer runtime lines | core, one **batched** specialist (the batched checklists below plus the Red Team prompt in one context), the native adversarial pass (kept: backtest inconclusive, GSTA-147 D5) and the outside voice on numbers | -| **standard** | more than 50 runtime lines | one batched specialist at `medium` and the Red Team at 200 runtime lines (or on a CRITICAL); the batched reader leaves docs and registered outputs out of its diff; adversarial and outside voice read everything | -| **full** | gate class, or any classifier error | one batched specialist on the whole diff, the Red Team always, no hit-rate gating | +| **light**, runtime 0 | docs, tests, eval write-ups, or only registered outputs | one reader: the core checklist; the outside voice instead reads a diff that publishes numbers | +| **light**, runtime above 0 | code with no risk class, at any size (R51: size never raises the depth) | core, one **batched** specialist (the batched checklists below plus the Red Team prompt in one context), the native adversarial pass (kept: backtest inconclusive, GSTA-147 D5) and the outside voice on numbers | +| **standard** | `--depth-floor standard`, or a light diff raised by a fixed blocker or a CRITICAL | one batched specialist at `medium` and the Red Team at 200 runtime lines (or on a CRITICAL or a raise); the batched reader leaves docs and registered outputs out of its diff; adversarial and outside voice read everything | +| **full** | gate class, a [risk class](#risk-classes), or any classifier error | one batched specialist on the whole diff, the Red Team always, no hit-rate gating | + + +**Risk classes (R51).** `lib/review-risk.ts` reads a code diff's paths and its +added and deleted lines; any class makes the depth full (`risk:` in +`reasons=`, `review_depth.risk` in the receipt). money, auth and secret are the +[targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code)'s matchers. +**security** is a trust-boundary path (trust, redact, sanitize, neutralize, +consent, escape) or line (`shell: true`, `execSync(`, `eval`, `new Function(`, +raw HTML sinks, a shell run through argv, recursive deletes). **data** is user data: a path naming pii, gdpr, ccpa, privacy, +personal, account, customer, cookie, persistence, storage or database, or a line +that drops a table or column, deletes rows or bulk-deletes records. Auth scope, +migrations and deleted code paths were already gate class. `RISK_RULES` hashes +the patterns and `DEPTH_RULES` includes it. + +**UI text and SVGs** stay code (light unless a risk class applies). A docs path +for UI copy, locale values and chart SVGs waits for an allowlist classifier: three +rounds of R51 review found bypasses in line-pattern versions of both. **Batched checklists (the lean ensemble, GSTA-167 part 2).** One specialist subagent reads Testing and Security on every code or gate diff, plus @@ -230,7 +252,7 @@ and lockfiles stay runtime. To adopt it, commit an entry like this on the base b **Gate paths for the machinery itself.** An edit to `lib/review-depth.ts`, `lib/review-collect.ts`, `bin/gstack-review-plan`, `lib/review-scope.ts`, -`lib/review-targeted.ts`, `review/specialists/targeted.md`, `lib/ship-queue*.ts`, `lib/regen.ts`, `.gstack/generated.json`, +`lib/review-targeted.ts`, `lib/review-risk.ts`, `review/specialists/targeted.md`, `lib/ship-queue*.ts`, `lib/regen.ts`, `.gstack/generated.json`, `ship/sections/review-army.md.tmpl`, `ship/sections/adversarial.md.tmpl` or `scripts/resolvers/review-army.ts` is gate class: changing how much review runs gets the most review. `rules=` (`DEPTH_RULES`) hashes the deciding constants and patterns, @@ -282,6 +304,10 @@ the specialists, audits and watchers at the host default too prompt. At runtime 0 light runs no adversarial pass either. - Standard reads thresholds on runtime lines, not the whole diff, so a test-heavy diff with few runtime lines dispatches fewer specialists. +- R51: size never raises the depth; non-risky code is light at any size, and a risk + class makes it full. Two rounds is the cap, a fix-check + included: a blocker fixed in round 2 stops for the owner instead of earning a + fix-check, and a light pass 1 whose below-floor fixes change no runtime lines is done. - A JSON, CSV, TSV or raster data file no code names is docs, not runtime (GSTA-167 part 4a). A binary image no code names is no longer a `binary_code_path` error that forces full depth; one that code names still is. @@ -354,6 +380,7 @@ puts in the receipt: | `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.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 | @@ -366,7 +393,7 @@ specialist stats they record). The summary line appends `fix_checks=`, loop prints the old line. Beside the counters, `write` records `review_depth` (the depth, runtime lines, -rules hash, base and registry commits, and the gate scan bound to the head) and +rules hash, base and registry commits, the gate scan bound to the head and the risk classes) and the queue refuses a receipt below the depth its head needs ([ship-policy.md#ship-receipt](ship-policy.md#ship-receipt)). A stop the owner accepted after a fix-check edit goes in `review_loop_exception`, never in prose @@ -385,8 +412,8 @@ difference passed a scoped review. | Value | Pass 1 | Blocking finding in unchanged code | Pass 2+ | Cap | |---|---|---|---|---| -| `frozen` (default) | the diff | filed as an issue, listed as deferred | the range since the previous pass, blocking only | 2 full passes + 1 fix-check | -| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 2 full passes + 1 fix-check | +| `frozen` (default) | the diff | filed as an issue, listed as deferred | the range since the previous pass, blocking only | 2 rounds (a fix-check is one) | +| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 2 rounds (a fix-check is one) | | `all` | everything | fixed in this PR | the whole diff, every finding | 3 fix cycles | `diff-plus-blocking` is the fallback for a repository where deferred issues pile diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 7cd24136a..4e0488229 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1221,13 +1221,13 @@ construction: fix-range`) or in the diff is `scope: fix`; nothing below the floor earns a pass. `NO FINDINGS` is the expected reviewer result for a clean range, never a failed review. -3. **Two full passes plus one fix-check is the cap**, stepped by the - [transition table](review-policy.md#transitions); a - [scoped pass](review-policy.md#scoped-pass) counts as a full pass. A pass that applied no edits +3. **Two rounds is the cap** (R51), stepped by the + [transition table](review-policy.md#transitions); a full pass, a + [scoped pass](review-policy.md#scoped-pass) and a fix-check each count as a round. A pass that applied no edits continues to Step 10: the loop ships with its deferred and noted items named in the PR body. A blocking finding still open after the last allowed step stops the run and names the finding (path, severity, problem) for the owner instead - of a third full pass. + of a third round; the owner may grant one scoped fix-check. `gstack-review-scope summary --since ` reads the branch's review-log rows for this invocation and writes the counters the ship receipt carries diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 2bd2a2dc1..1e184b184 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -104,13 +104,13 @@ export interface ShipReceipt { // Related open PRs by other authors (Lane D): `supersedes` / `credit` only on the owner's confirmation. issues?: { fixes: number[]; refs: number[]; flakes: Array<{ issue: number; run: string }>; related?: Array<{ pr: number; repo?: string; author: string; closes: number[]; supersedes?: boolean; credit?: boolean }> }; /** Harness lane E: the review loop's counters (gstack-review-scope summary) and the carry record when the receipt followed a head whose product diff was unchanged (gstack-ship-receipt carry). */ - review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; dispatches?: Record; est_input_tokens?: number }; + review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number }; carry?: { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' }; /** The reviewed head's tree and its product patch-id against the base (gstack-ship-receipt write; carry rebinds both): what a carry across rewritten history compares. */ head_tree?: string; product_patch_id?: string; /** The gstack that wrote the receipt, and the review depth the reviewed head needs, recomputed at write (lib/ship-receipt-depth.ts receiptDepthAtHead). */ gstack_version?: string; - review_depth?: { depth: 'light' | 'standard' | 'full'; class: 'docs' | 'tests' | 'code' | 'gate'; runtime_lines: number; rules: string; base: string; registry: string | null; gate_scan: { tagged: number | null; head: string }; reasons: string[] }; + review_depth?: { depth: 'light' | 'standard' | 'full'; class: 'docs' | 'tests' | 'code' | 'gate'; runtime_lines: number; rules: string; base: string; registry: string | null; gate_scan: { tagged: number | null; head: string }; reasons: string[]; risk?: string[] }; /** The owner's acceptance of a stop after a fix-check edit under frozen scope (#3559); the PR body repeats it word for word. */ review_loop_exception?: { accepted_by: string; date: string; via: string; reason: string; fix_check: { head: string; findings: number; files?: string[] } }; /** The targeted pass on money, gate, auth and secret code (lib/review-targeted.ts), bound to the receipt's product_patch_id; `done` only with its output collected. */ @@ -236,11 +236,11 @@ export const ARTIFACT_SCHEMAS = { thread: str, tree: { enum: ['same-modulo-stamps', 'changed', 'not-compared'] }, gate_reuse: str, policy: str, queue_mode: str, preregistration: str, history: str, pregate: { ...str, description: 'pass|warn|requires-remote cleared … @ from pregate.json, current for this head' }, requires_remote: strList, approval: { type: 'object', description: 'approval classes: { source, status: checked|unavailable, classes: [{ class, scope, effects, citation: verified|unverified, approved_by, date, via, binding, evidence }], report_only }' }, - review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 } } }, + review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 } } }, carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList, trees: strList, ancestry: { enum: ['linear', 'rewritten'] } } }, head_tree: str, product_patch_id: str, gstack_version: str, base_red: { type: 'object', required: ['cases', 'files', 'base', 'lane', 'expires_at'], properties: { cases: strList, files: strList, base: str, lane: str, expires_at: str } }, - review_depth: { type: 'object', required: ['depth', 'class', 'runtime_lines', 'rules', 'base', 'gate_scan'], properties: { depth: { enum: ['light', 'standard', 'full'] }, class: { enum: ['docs', 'tests', 'code', 'gate'] }, runtime_lines: { type: 'integer', minimum: 0 }, rules: str, base: str, registry: nullableStr, gate_scan: { type: 'object', required: ['tagged', 'head'], properties: { tagged: { type: ['integer', 'null'], minimum: 0 }, head: str } }, reasons: strList } }, + review_depth: { type: 'object', required: ['depth', 'class', 'runtime_lines', 'rules', 'base', 'gate_scan'], properties: { depth: { enum: ['light', 'standard', 'full'] }, class: { enum: ['docs', 'tests', 'code', 'gate'] }, runtime_lines: { type: 'integer', minimum: 0 }, rules: str, base: str, registry: nullableStr, gate_scan: { type: 'object', required: ['tagged', 'head'], properties: { tagged: { type: ['integer', 'null'], minimum: 0 }, head: str } }, reasons: strList, risk: { type: 'array', items: { enum: ['money', 'auth', 'secret', 'security', 'data'] } } } }, review_targeted: { type: 'object', required: ['classes', 'rules', 'brief_sha', 'product_patch_id', 'status', 'findings', 'raised_by'], properties: { classes: { type: 'array', items: { enum: ['money', 'gate', 'auth', 'secret'] } }, rules: str, brief_sha: str, product_patch_id: str, status: { enum: ['done', 'failed', 'not-run'] }, findings: { type: 'integer', minimum: 0 }, raised_by: str } }, review_loop_exception: { type: 'object', required: ['accepted_by', 'date', 'via', 'reason', 'fix_check'], properties: { accepted_by: str, date: str, via: str, reason: str, fix_check: { type: 'object', required: ['head', 'findings'], properties: { head: str, findings: { type: 'integer', minimum: 0 }, files: strList } } } }, issues: { type: 'object', properties: { fixes: { type: 'array', items: { type: 'integer' } }, refs: { type: 'array', items: { type: 'integer' } }, flakes: { type: 'array', items: { type: 'object', required: ['issue', 'run'], properties: { issue: { type: 'integer' }, run: str } } }, related: { type: 'array', items: { type: 'object', required: ['pr', 'author', 'closes'], properties: { pr: { type: 'integer' }, repo: str, author: str, closes: { type: 'array', items: { type: 'integer' } }, supersedes: { type: 'boolean' }, credit: { type: 'boolean' } } } } } }, diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 2cdbd8fd9..714e44183 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -13,6 +13,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { ResultCodeName } from './result-codes'; import { matchesAny } from './ship-policy'; +import { matchRisk, RISK_RULES, type RiskClass } from './review-risk'; export type DiffClass = 'docs' | 'tests' | 'code' | 'gate'; export type PathClass = 'docs' | 'tests' | 'code'; @@ -23,6 +24,7 @@ export type Step = 'full' | 'scoped' | 'fix-check' | 'done' | 'stop'; export const SPECIALISTS = ['testing', 'maintainability', 'security', 'performance', 'data-migration', 'api-contract', 'design', 'simplification', 'state-machines'] as const; export type Specialist = (typeof SPECIALISTS)[number]; +/** The review rounds one /ship invocation runs (R51): a full, scoped or fix-check step is a round. */ export const FULL_PASS_CAP = 2; export const SMALL_DIFF_LINES = 50; export const RED_TEAM_LINES = 200; @@ -53,7 +55,7 @@ const RELEASE_METADATA = [/^CHANGELOG\.md$/, /^VERSION$/, /^agents-digest\//]; /** Gate paths, the review-depth machinery included: an edit to how much review runs gets the most review. */ const GATE_PATH = [ /(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/, - /^lib\/review-(depth|collect|scope|targeted)\.ts$/, /^review\/specialists\/targeted\.md$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/, + /^lib\/review-(depth|collect|scope|targeted|risk)\.ts$/, /^review\/specialists\/targeted\.md$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/, ]; /** Data files: JSON, CSV, TSV and raster images. YAML, TOML and SVG are never data here: deploy configs, API specs and executable SVG stay code. */ @@ -66,7 +68,7 @@ const DATA_CONFIG = [ /** A hash of the constants and patterns that decide a depth, not of this file's bytes: a comment edit keeps it, a threshold edit changes it. */ export const DEPTH_RULES = createHash('sha256').update(JSON.stringify({ - SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, BATCH_SCOPED, SCOPED_RANGE_LINES, ROLE_EFFORT, + RISK_RULES, SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, BATCH_SCOPED, SCOPED_RANGE_LINES, ROLE_EFFORT, patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), })).digest('hex').slice(0, 16); @@ -129,8 +131,11 @@ export interface ClassInput { * (lib/review-collect.ts). A candidate not listed counts as docs; 'all' or no scan keeps every data file code. */ consumed?: readonly string[] | 'all'; + /** Added and deleted lines per path (lib/review-collect.ts): the risk classes read them; without them no line signal applies. */ + added?: Record; + removed?: Record; } -export interface ClassResult { class: DiffClass; reasons: string[]; errors: DepthError[]; runtimeLines: number | null } +export interface ClassResult { class: DiffClass; reasons: string[]; errors: DepthError[]; runtimeLines: number | null; risk: RiskClass[] } const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200); const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p); @@ -166,30 +171,33 @@ export function classifyDiff(i: ClassInput): ClassResult { for (const p of i.paths) if (GATE_PATH.some(r => r.test(p))) { raise('gate', `gate path: ${p}`); break; } if (i.registry?.error) errors.push(i.registry.error); } - if (!i.perPath) return { class: cls, reasons, errors, runtimeLines: null }; + const riskHits = CLASS_RANK[cls] >= CLASS_RANK.code ? matchRisk({ codePaths: i.paths.filter(p => pathClass(i, p) === 'code'), added: i.added, removed: i.removed, scopeAuth: s.SCOPE_AUTH === 'true' }) : new Map(); + const risk = [...riskHits.keys()]; + reasons.push(...[...riskHits.values()].map(r => `risk: ${r}`)); + if (!i.perPath) return { class: cls, reasons, errors, runtimeLines: null, risk }; let runtimeLines = 0; for (const e of i.perPath) { if (pathClass(i, e.path) !== 'code' || isOutput(i, e.path)) continue; if (e.binary) errors.push({ code: 'binary_code_path', detail: 'git reports a binary change, so its lines cannot be counted', input: e.path, repair: "register it as an output of an entry with a command in the base branch's .gstack/generated.json, or accept the full review" }); else runtimeLines += e.lines; } - return { class: cls, reasons, errors, runtimeLines }; + return { class: cls, reasons, errors, runtimeLines, risk }; } -/** Any classifier error or gate class is full; runtime above SMALL_DIFF_LINES is standard; otherwise light. A floor only raises. */ -export function depthFor(o: { cls: DiffClass; runtimeLines: number; errors: readonly DepthError[]; floor?: Depth }): Depth { - const d: Depth = o.errors.length || o.cls === 'gate' ? 'full' : o.runtimeLines > SMALL_DIFF_LINES ? 'standard' : 'light'; +/** Risk, not size (R51): any classifier error, gate class or risk class is full; everything else is light. A floor only raises; a fixed blocker or a CRITICAL raises light to standard for the next step (planReview). */ +export function depthFor(o: { cls: DiffClass; runtimeLines: number; errors: readonly DepthError[]; risk?: readonly RiskClass[]; floor?: Depth }): Depth { + const d: Depth = o.errors.length || o.cls === 'gate' || o.risk?.length ? 'full' : 'light'; return o.floor && DEPTH_RANK[o.floor] > DEPTH_RANK[d] ? o.floor : d; } -export interface DepthResult { depth: Depth; class: DiffClass; runtimeLines: number; reasons: string[]; errors: DepthError[]; codes: ResultCodeName[]; rules: string } +export interface DepthResult { depth: Depth; class: DiffClass; runtimeLines: number; reasons: string[]; errors: DepthError[]; codes: ResultCodeName[]; rules: string; risk: RiskClass[] } function decideDepth(c: ClassResult, runtimeLines: number, floor?: Depth): DepthResult { - const unfloored = depthFor({ cls: c.class, runtimeLines, errors: c.errors }); - const reasons = [...c.errors.map(e => `error:${e.code}`), ...(c.class === 'gate' ? ['gate-class'] : []), `runtime=${runtimeLines}${runtimeLines > SMALL_DIFF_LINES ? '>' : '<='}${SMALL_DIFF_LINES}`]; + const unfloored = depthFor({ cls: c.class, runtimeLines, errors: c.errors, risk: c.risk }); + const reasons = [...c.errors.map(e => `error:${e.code}`), ...(c.class === 'gate' ? ['gate-class'] : []), ...c.risk.map(r => `risk:${r}`), `runtime=${runtimeLines}`]; if (floor && DEPTH_RANK[floor] > DEPTH_RANK[unfloored]) reasons.push(`floor:${floor}`); const codes = [...new Set(c.errors.flatMap(e => (e.result ? [e.result] : [])))]; - return { depth: depthFor({ cls: c.class, runtimeLines, errors: c.errors, floor }), class: c.class, runtimeLines, reasons, errors: c.errors, codes, rules: DEPTH_RULES }; + return { depth: depthFor({ cls: c.class, runtimeLines, errors: c.errors, risk: c.risk, floor }), class: c.class, runtimeLines, reasons, errors: c.errors, codes, rules: DEPTH_RULES, risk: c.risk }; } /** The depth of a collected range (lib/review-collect.ts) plus a recorded gate-scan result: what the queue recomputes at a head. */ @@ -249,17 +257,23 @@ export interface NextStep { step: Step; reason: string } * The transition table (docs/review-policy.md#transitions). Pass 1 is full. A fixed * blocking finding earns a second full pass on the range, scoped when `depth` (known * only to planReview) is below full and the fix range has at most SCOPED_RANGE_LINES - * runtime lines; any other fix earns one fix-check; two full passes is the cap (a - * scoped pass is one); a blocker still open after any step stops for the owner. + * runtime lines; any other fix earns one fix-check, except a light pass whose fix range + * changes no runtime lines; two rounds is the cap (R51: a scoped pass or a fix-check is + * one): after round 2, below-floor fixes finish with notes, and a blocker open or fixed + * there, or a gate-class edit there, stops for the owner, whose grant may add one scoped fix-check. */ export function nextStep(h: History, depth?: Depth): NextStep { if (h.last === 'none') return { step: 'full', reason: 'pass 1 reviews the whole diff' }; const fixed = !!(h.fixedBlocking || h.fixedBelow); - if (h.last === 'fix-check') return h.blockingOpen || fixed ? { step: 'stop', reason: 'the fix-check found a blocking finding: name it for the owner, never a third full pass' } : { step: 'done', reason: 'the fix-check found nothing blocking' }; - if (h.blockingOpen && h.fullPasses >= FULL_PASS_CAP) return { step: 'stop', reason: `a blocking finding is open after ${FULL_PASS_CAP} full passes: name it for the owner` }; + if (h.last === 'fix-check') return h.blockingOpen || h.fixedBlocking || h.touchedGate ? { step: 'stop', reason: `the fix-check ${h.touchedGate && !h.blockingOpen && !h.fixedBlocking ? 'edited a gate-class path' : 'found a blocking finding'}: name it for the owner, whose grant may add one scoped fix-check, never a third round` } : { step: 'done', reason: fixed ? 'round cap 2 reached: the below-floor fixes ship as notes in the PR body and receipt' : 'the fix-check found nothing blocking' }; + if (h.blockingOpen && h.fullPasses >= FULL_PASS_CAP) return { step: 'stop', reason: `a blocking finding is open after ${FULL_PASS_CAP} rounds: name it for the owner` }; if (h.blockingOpen) return { step: 'stop', reason: 'a blocking finding is still open in fix scope: name it for the owner' }; if (!fixed) return { step: 'done', reason: 'the step applied no edits' }; - if ((h.fixedBlocking || h.touchedGate) && h.fullPasses < FULL_PASS_CAP) { + if (h.fullPasses >= FULL_PASS_CAP) return h.fixedBlocking || h.touchedGate + ? { step: 'stop', reason: `${h.fixedBlocking ? 'a blocking finding was fixed' : 'a fix edited a gate-class path'} in round ${FULL_PASS_CAP}, the cap: name it for the owner, whose grant may add one scoped fix-check (review_loop_exception)` } + : { step: 'done', reason: `round cap ${FULL_PASS_CAP} reached: the below-floor fixes ship as notes in the PR body and receipt` }; + if (!h.fixedBlocking && !h.touchedGate && depth === 'light' && h.rangeRuntimeLines === 0) return { step: 'done', reason: 'light depth: the below-floor fixes change no runtime lines, so they ship as notes without a fix-check' }; + if (h.fixedBlocking || h.touchedGate) { if (!h.fixedBlocking) return { step: 'full', reason: 'a fix touched a gate-class path: full pass on the range' }; const n = h.rangeRuntimeLines; const keep = h.touchedGate ? 'a fix touched a gate-class path' : depth === undefined ? 'the depth is unknown' : h.pass1Depth === 'full' ? 'pass 1 ran at full depth' : depth === 'full' ? 'the diff is at full depth' @@ -267,7 +281,7 @@ export function nextStep(h: History, depth?: Depth): NextStep { if (keep) return { step: 'full', reason: `a blocking finding was fixed and ${keep}: full pass on the range` }; return { step: 'scoped', reason: `a blocking finding was fixed on a ${depth} diff with ${n} runtime lines in the fix range (<= ${SCOPED_RANGE_LINES}): scoped pass on the range` }; } - return { step: 'fix-check', reason: h.fixedBlocking ? `full-pass cap ${FULL_PASS_CAP} reached: one fix-check reads the fix` : 'only below-floor fixes: one fix-check reads the touched hunks' }; + return { step: 'fix-check', reason: 'only below-floor fixes: one fix-check reads the touched hunks (round 2)' }; } // --------------------------------------------------------------------------- @@ -401,7 +415,7 @@ export function planReview(i: PlanInput): ReviewPlan { } else if (next.step === 'full') { const full = stepDepth === 'full'; const dcls: DiffClass = full ? 'gate' : cls; - const raised = !full && runtimeLines <= SMALL_DIFF_LINES; + const raised = !full && (raise || runtimeLines <= SMALL_DIFF_LINES); const lines = full ? i.diffLines : Math.max(runtimeLines, raised ? SMALL_DIFF_LINES : 0); const checklists = batchChecklists(dcls, i, skipped); if (checklists.length) add('specialist', 'batched', specialist, full ? [] : docsAndOutputs, { checklists }); diff --git a/lib/review-risk.ts b/lib/review-risk.ts new file mode 100644 index 000000000..c1ed328a1 --- /dev/null +++ b/lib/review-risk.ts @@ -0,0 +1,85 @@ +/** + * review-risk — the risk signals behind /ship's review depth (R51, docs/review-policy.md#review-depth) and the + * targeted pass (lib/review-targeted.ts). A leaf module: callers pass only code-class paths with their added + * and deleted lines, so it imports neither the depth nor the targeted module. + * + * money, auth and secret are the targeted pass's matchers (GST-257). security is a trust-boundary path or line + * (a redaction, sanitizer, consent or trust module; a shell, eval or raw HTML sink); data is user data (a path + * naming personal or stored data, or a line that drops or bulk-deletes it). Path and identifier signals are + * token-bounded: costume never names cost. + */ +import { createHash } from 'node:crypto'; + +export const RISK_CLASSES = ['money', 'auth', 'secret', 'security', 'data'] as const; +export type RiskClass = (typeof RISK_CLASSES)[number]; + +export const MONEY_PATH_TOKENS = ['cost', 'costs', 'spend', 'spending', 'budget', 'budgets', 'ledger', 'billing', 'price', 'prices', 'pricing', 'fee', 'fees', 'invoice', 'invoices']; +/** Identifiers ending in _usd, Usd or cap_usd, any spend_cap spelling, and paid-provider call sites (the providers lib/pricing.ts prices). */ +export const MONEY_LINE = [ + /\b[A-Za-z0-9_]*_usd\b/i, /\b[a-z][A-Za-z0-9]*Usd\b/, /\bspend[_-]?cap\b/i, + /\bapi\.(?:anthropic|openai|voyageai)\.com\b/, /\bgenerativelanguage\.googleapis\.com\b/, /\bopenrouter\.ai\b/, + /\bnew\s+(?:Anthropic|OpenAI|AnthropicBedrock|AnthropicVertex)\s*\(/, /\.(?:messages|chat\.completions|responses|embeddings)\.create\s*\(/, +]; +export const AUTH_PATH_TOKENS = ['auth', 'authn', 'authz', 'oauth', 'oauth2', 'jwt', 'session', 'sessions', 'permission', 'permissions', 'role', 'roles']; +export const AUTH_PATH_PREFIXES = ['authenticat', 'authoriz']; +export const SECRET_PATH = /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i; +/** An environment name whose underscore-separated segments include a key word: OPENAI_API_KEY, GH_TOKEN; never MAX_TOKENS. */ +export const KEY_NAME = String.raw`(?:[A-Z0-9]+_)*(?:API_?KEY|SECRET|TOKEN|PASSWORD|PASSWD|PRIVATE_KEY|ACCESS_KEY|CREDENTIALS?)(?:_[A-Z0-9]+)*\b`; +export const SECRET_LINE = [ + new RegExp(String.raw`\b(?:process\.env|Bun\.env|import\.meta\.env)(?:\.|\[\s*['"\x60])${KEY_NAME}`), + new RegExp(String.raw`\b(?:os\.environ(?:\.get\(|\[)|os\.getenv\(|ENV(?:\.fetch\(|\[))\s*['"]${KEY_NAME}`), + new RegExp(String.raw`\$\{?${KEY_NAME}`), + /\b(?:keychain|keytar|find-generic-password|find-internet-password|secret-tool|git-credential|credential\.helper)\b/i, /\bgh auth token\b/, /(?:^|[\s'"/])\.netrc\b/, +]; + +const SECURITY_PATH_TOKENS = ['trust', 'redact', 'redaction', 'redactor', 'sanitize', 'sanitizer', 'neutralize', 'neutralizer', 'consent', 'escape']; +const SECURITY_LINE = [ + /\bshell\s*:\s*true\b/, /\bshell\s*=\s*True\b/, /(? [k, v.map(r => '/' + r.source + '/' + r.flags)]), +})).digest('hex').slice(0, 16); + +export const tokens = (p: string) => p.replace(/([a-z0-9])([A-Z])/g, '$1 $2').toLowerCase().split(/[^a-z0-9]+/).filter(Boolean); + +export interface RiskInput { + /** Code-class paths only: the caller drops docs, tests and data its own classifier set aside. */ + codePaths: readonly string[]; + added?: Record; + removed?: Record; + /** gstack-diff-scope's SCOPE_AUTH: the auth class needs it and an auth token in the path. */ + scopeAuth: boolean; +} + +/** The risk classes the code paths touch, the first reason per class, in RISK_CLASSES order. Nothing fails closed here: input errors are the caller's. */ +export function matchRisk(i: RiskInput): Map { + const why = new Map(); + const hit = (c: RiskClass, reason: string) => { if (!why.has(c)) why.set(c, reason); }; + for (const p of i.codePaths) { + const t = tokens(p); + if (i.scopeAuth && t.some(x => AUTH_PATH_TOKENS.includes(x) || AUTH_PATH_PREFIXES.some(a => x.startsWith(a)))) hit('auth', 'auth path: ' + p); + if (SECRET_PATH.test(p)) hit('secret', 'secret path: ' + p); + if (t.some(x => MONEY_PATH_TOKENS.includes(x))) hit('money', 'money path: ' + p); + if (t.some(x => SECURITY_PATH_TOKENS.includes(x))) hit('security', 'security path: ' + p); + if (t.some(x => DATA_PATH_TOKENS.includes(x))) hit('data', 'data path: ' + p); + for (const line of [...(i.added?.[p] ?? []), ...(i.removed?.[p] ?? [])]) { + if (!why.has('money') && MONEY_LINE.some(r => r.test(line))) hit('money', 'money line: ' + p); + if (!why.has('secret') && SECRET_LINE.some(r => r.test(line))) hit('secret', 'secret line: ' + p); + if (!why.has('security') && SECURITY_LINE.some(r => r.test(line))) hit('security', 'security line: ' + p); + if (!why.has('data') && DATA_LINE.some(r => r.test(line))) hit('data', 'data line: ' + p); + } + } + return new Map(RISK_CLASSES.filter(c => why.has(c)).map(c => [c, why.get(c)!])); +} diff --git a/lib/review-scope.ts b/lib/review-scope.ts index e011ea4e7..833562591 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -300,6 +300,8 @@ export interface ReviewLoopSummary { passes: number; blocking_found: number; deferred: number; noted: number; fixed: number; scope: ReviewScopePolicy | 'unknown'; /** One-reviewer re-checks of a fix (docs/review-policy.md#transitions); never passes. */ fix_checks?: number; + /** Review rounds (R51): passes plus fix-checks; the loop caps them at 2. */ + rounds?: number; /** Reviewer dispatches of this invocation by role (specialist, red-team, adversarial-native, outside-voice, fix-check, targeted). */ dispatches?: Record; /** Prompt plus diff bytes the dispatches read, / 4: a relative estimate, never a bill. */ @@ -370,6 +372,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blocking.size, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; if (fixChecks) out.fix_checks = fixChecks; + out.rounds = passes.length + fixChecks; const byRole: Record = {}; let bytes = 0; let measured = false; for (const row of rows.filter(r => inRun(r) && DISPATCH_SKILLS.includes(r.skill))) { diff --git a/lib/review-targeted.ts b/lib/review-targeted.ts index ef037c683..49d27462a 100644 --- a/lib/review-targeted.ts +++ b/lib/review-targeted.ts @@ -21,29 +21,11 @@ import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { classifyPath } from './review-depth'; +import { AUTH_PATH_PREFIXES, AUTH_PATH_TOKENS, KEY_NAME, MONEY_LINE, MONEY_PATH_TOKENS, SECRET_LINE, SECRET_PATH, matchRisk } from './review-risk'; export const TARGETED_CLASSES = ['money', 'gate', 'auth', 'secret'] as const; export type TargetedClass = (typeof TARGETED_CLASSES)[number]; -const MONEY_PATH_TOKENS = ['cost', 'costs', 'spend', 'spending', 'budget', 'budgets', 'ledger', 'billing', 'price', 'prices', 'pricing', 'fee', 'fees', 'invoice', 'invoices']; -/** Identifiers ending in _usd, Usd or cap_usd, any spend_cap spelling, and paid-provider call sites (the providers lib/pricing.ts prices). */ -const MONEY_LINE = [ - /\b[A-Za-z0-9_]*_usd\b/i, /\b[a-z][A-Za-z0-9]*Usd\b/, /\bspend[_-]?cap\b/i, - /\bapi\.(?:anthropic|openai|voyageai)\.com\b/, /\bgenerativelanguage\.googleapis\.com\b/, /\bopenrouter\.ai\b/, - /\bnew\s+(?:Anthropic|OpenAI|AnthropicBedrock|AnthropicVertex)\s*\(/, /\.(?:messages|chat\.completions|responses|embeddings)\.create\s*\(/, -]; -const AUTH_PATH_TOKENS = ['auth', 'authn', 'authz', 'oauth', 'oauth2', 'jwt', 'session', 'sessions', 'permission', 'permissions', 'role', 'roles']; -const AUTH_PATH_PREFIXES = ['authenticat', 'authoriz']; -const SECRET_PATH = /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i; -/** An environment name whose underscore-separated segments include a key word: OPENAI_API_KEY, GH_TOKEN; never MAX_TOKENS. */ -const KEY_NAME = String.raw`(?:[A-Z0-9]+_)*(?:API_?KEY|SECRET|TOKEN|PASSWORD|PASSWD|PRIVATE_KEY|ACCESS_KEY|CREDENTIALS?)(?:_[A-Z0-9]+)*\b`; -const SECRET_LINE = [ - new RegExp(String.raw`\b(?:process\.env|Bun\.env|import\.meta\.env)(?:\.|\[\s*['"\x60])${KEY_NAME}`), - new RegExp(String.raw`\b(?:os\.environ(?:\.get\(|\[)|os\.getenv\(|ENV(?:\.fetch\(|\[))\s*['"]${KEY_NAME}`), - new RegExp(String.raw`\$\{?${KEY_NAME}`), - /\b(?:keychain|keytar|find-generic-password|find-internet-password|secret-tool|git-credential|credential\.helper)\b/i, /\bgh auth token\b/, /(?:^|[\s'"/])\.netrc\b/, -]; - /** A hash of the patterns that decide the classes, not of this file's bytes: a comment edit keeps it, a pattern edit changes it. */ export const TARGETED_RULES = createHash('sha256').update(JSON.stringify({ TARGETED_CLASSES, MONEY_PATH_TOKENS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME, @@ -65,8 +47,6 @@ export interface TargetedInput { } export interface TargetedResult { classes: TargetedClass[]; reasons: string[]; errors: string[]; rules: string } -const tokens = (p: string) => p.replace(/([a-z0-9])([A-Z])/g, '$1 $2').toLowerCase().split(/[^a-z0-9]+/).filter(Boolean); - /** The classes a diff touches, in TARGETED_CLASSES order, with one reason per class; any input error returns all four. */ export function targetedClasses(i: TargetedInput): TargetedResult { const errors = (i.errors ?? []).map(e => `${e.code} (${e.input})`); @@ -77,18 +57,8 @@ export function targetedClasses(i: TargetedInput): TargetedResult { if (code.length && i.gateTagged === null) errors.push('gate scan unavailable'); if (errors.length) return { classes: [...TARGETED_CLASSES], reasons: errors.map(e => `fail-closed: ${e}`), errors, rules: TARGETED_RULES }; const why = new Map(); - const hit = (c: TargetedClass, reason: string) => { if (!why.has(c)) why.set(c, reason); }; - if ((i.gateTagged ?? 0) > 0) hit('gate', `gate scan tagged ${i.gateTagged} hunk(s)`); - for (const p of code) { - const t = tokens(p); - if (s!.SCOPE_AUTH === 'true' && t.some(x => AUTH_PATH_TOKENS.includes(x) || AUTH_PATH_PREFIXES.some(a => x.startsWith(a)))) hit('auth', `auth path: ${p}`); - if (SECRET_PATH.test(p)) hit('secret', `secret path: ${p}`); - if (t.some(x => MONEY_PATH_TOKENS.includes(x))) hit('money', `money path: ${p}`); - for (const line of [...(i.added?.[p] ?? []), ...(i.removed?.[p] ?? [])]) { - if (!why.has('money') && MONEY_LINE.some(r => r.test(line))) hit('money', `money line: ${p}`); - if (!why.has('secret') && SECRET_LINE.some(r => r.test(line))) hit('secret', `secret line: ${p}`); - } - } + if ((i.gateTagged ?? 0) > 0) why.set('gate', `gate scan tagged ${i.gateTagged} hunk(s)`); + for (const [c, reason] of matchRisk({ codePaths: code, added: i.added, removed: i.removed, scopeAuth: s!.SCOPE_AUTH === 'true' })) if ((TARGETED_CLASSES as readonly string[]).includes(c)) why.set(c as TargetedClass, reason); const classes = TARGETED_CLASSES.filter(c => why.has(c)); return { classes, reasons: classes.map(c => why.get(c)!), errors: [], rules: TARGETED_RULES }; } diff --git a/lib/ship-budget.ts b/lib/ship-budget.ts index ae354cf40..1245f6b9c 100644 --- a/lib/ship-budget.ts +++ b/lib/ship-budget.ts @@ -302,7 +302,7 @@ export function segmentEstimate(cal: Calibration, unit: string, from: string, to * section reads, reconcile: gstack-evals #34's two children cost 4.53M, 2-4.5M each) to save the parent's * context above the host floor on each call of the segment. #34's parent calls ran 0.2-0.3M over a 0.15M * floor, so a fresh child saves at most about 3/8 of a call, and a segment under CHILD_FIXED_COST over that - * share (12M) cannot pay for its child. A light-depth diff (runtime lines at most SMALL_DIFF_LINES) keeps + * share (12M) cannot pay for its child. A small diff (runtime lines at most SMALL_DIFF_LINES) keeps * its segment when the segment's calibrated calls at this run's own per-call charge stay under that. */ export const SMALL_RUN_LINES = SMALL_DIFF_LINES; diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 15d4bdbbf..ad8933cfb 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -70,7 +70,7 @@ export function receiptDepthAtHead(repoRoot: string, base: string, binDir: strin tagged = g?.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null; } const d = depthOfCommitInput({ ...c, scope, gateTagged: tagged }); - return { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12) }; + return { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk }; } export const depthBelow = (have: Depth, need: Depth) => RANK[have] < RANK[need]; @@ -182,7 +182,7 @@ export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceip const need = depthOfCommitInput({ ...c, scope, gateTagged: tagged }); const have = rd?.depth; const inputs = `base@${short(baseSha.ok ? baseSha.out : null)},registry@${short(c.registry.sha)},gate_scan@${scanBound ? short(rd!.gate_scan.head) : 'missing'}`; - const line = `REVIEW_DEPTH: receipt=${have ?? 'none'} required=${need.depth} runtime_lines=${need.runtimeLines} class=${need.class} reasons=${need.reasons.join(',')} inputs=${inputs}`; + const line = `REVIEW_DEPTH: receipt=${have ?? 'none'} required=${need.depth} runtime_lines=${need.runtimeLines} class=${need.class} reasons=${need.reasons.join(',')} inputs=${inputs} rounds=${r.review_loop?.rounds ?? (r.review_loop ? r.review_loop.passes + (r.review_loop.fix_checks ?? 0) : 'none')}`; if (have && !depthBelow(have, need.depth)) return { ok: true, line }; const why = have ? `was reviewed at ${have} depth` : 'records no review_depth though its writer records one'; const errs = need.errors.map(e => `${e.code} (${e.input})`).join('; '); diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 70462347b..aba864ed5 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -129,7 +129,7 @@ export function receiptSummaryLine(r: ShipReceipt): string { if (r.approval) parts.push(`approval=${approvalSummary(r.approval)}`); const loop = parseReviewLoop(r.review_loop); if (loop) parts.push(`passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred}` - + (loop.fix_checks ? ` fix_checks=${loop.fix_checks}` : '') + (loop.dispatches ? ` dispatches=${Object.values(loop.dispatches).reduce((a, c) => a + c, 0)}` : '')); + + (loop.fix_checks ? ` fix_checks=${loop.fix_checks}` : '') + (loop.rounds !== undefined ? ` rounds=${loop.rounds}` : '') + (loop.dispatches ? ` dispatches=${Object.values(loop.dispatches).reduce((a, c) => a + c, 0)}` : '')); if (r.carry && typeof r.carry.from === 'string') parts.push(`carried_from=${r.carry.from.slice(0, 12)}`); if (r.base_red && typeof r.base_red.base === 'string') parts.push(`base_red=${r.base_red.base.slice(0, 12)}`); return `SHIP_RECEIPT: ${oneLine(parts.join(' '))}`; @@ -326,6 +326,7 @@ export function parseReviewLoop(value: unknown): ReviewLoopReceipt | undefined { if (n('fixed') !== undefined) out.fixed = n('fixed'); if (typeof v.scope === 'string') out.scope = v.scope.slice(0, 20); if (n('fix_checks') !== undefined) out.fix_checks = n('fix_checks'); + if (n('rounds') !== undefined) out.rounds = n('rounds'); if (n('est_input_tokens') !== undefined) out.est_input_tokens = n('est_input_tokens'); if (v.dispatches && typeof v.dispatches === 'object' && !Array.isArray(v.dispatches)) { const d = Object.fromEntries(Object.entries(v.dispatches as Record) diff --git a/package.json b/package.json index 9cc0bdbc0..9b0d4049c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.44", + "version": "1.92.45", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/ship/SKILL.md b/ship/SKILL.md index c53c541b5..de5eaa671 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -210,8 +210,8 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): only a finding that would fail a test, break production, or lose data or secrets blocks a pass; the rest is a PR-body note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only -raises it). Pass 1 reads the diff, later passes the range; the cap is two full -passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) +raises it). Pass 1 reads the diff, later passes the range; the cap is two rounds, +fix-checks included. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 45a5c5232..68483a085 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -76,8 +76,8 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): only a finding that would fail a test, break production, or lose data or secrets blocks a pass; the rest is a PR-body note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only -raises it). Pass 1 reads the diff, later passes the range; the cap is two full -passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) +raises it). Pass 1 reads the diff, later passes the range; the cap is two rounds, +fix-checks included. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 431fd2735..8d7459135 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -210,8 +210,8 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): only a finding that would fail a test, break production, or lose data or secrets blocks a pass; the rest is a PR-body note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only -raises it). Pass 1 reads the diff, later passes the range; the cap is two full -passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) +raises it). Pass 1 reads the diff, later passes the range; the cap is two rounds, +fix-checks included. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index c53c541b5..de5eaa671 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -210,8 +210,8 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): only a finding that would fail a test, break production, or lose data or secrets blocks a pass; the rest is a PR-body note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only -raises it). Pass 1 reads the diff, later passes the range; the cap is two full -passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) +raises it). Pass 1 reads the diff, later passes the range; the cap is two rounds, +fix-checks included. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index 44868bb8a..a79b3abb0 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -208,8 +208,8 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): only a finding that would fail a test, break production, or lose data or secrets blocks a pass; the rest is a PR-body note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only -raises it). Pass 1 reads the diff, later passes the range; the cap is two full -passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) +raises it). Pass 1 reads the diff, later passes the range; the cap is two rounds, +fix-checks included. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index dc9aa3ebf..1099c7e0c 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -210,8 +210,8 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): only a finding that would fail a test, break production, or lose data or secrets blocks a pass; the rest is a PR-body note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only -raises it). Pass 1 reads the diff, later passes the range; the cap is two full -passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) +raises it). Pass 1 reads the diff, later passes the range; the cap is two rounds, +fix-checks included. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index 801353112..4769dd47c 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -44,8 +44,23 @@ describe('nextStep: the transition table', () => { test('a fixed blocker earns full pass 2', () => expect(nextStep({ fullPasses: 1, last: 'full', fixedBlocking: true }).step).toBe('full')); test('below-floor fixes earn one fix-check', () => expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true }).step).toBe('fix-check')); test('a fix touching gate code earns a full pass instead of a fix-check', () => expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true, touchedGate: true }).step).toBe('full')); - test('at the full-pass cap a fixed blocker gets a fix-check', () => expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true }).step).toBe('fix-check')); - test('at the cap a gate-touching fix still gets a fix-check, never a third full pass', () => expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true, touchedGate: true }).step).toBe('fix-check')); + // R51: two rounds is the cap, a fix-check included. A blocker open or fixed in round 2 stops for the owner (a grant may add + // one scoped fix-check); below-floor fixes in round 2 finish with notes; a light pass whose fixes change no runtime lines is done. + test('a blocker fixed in round 2 stops for the owner, never a third round', () => { + expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true })).toMatchObject({ step: 'stop', reason: expect.stringContaining('scoped fix-check') }); + expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true, touchedGate: true }).step).toBe('stop'); + }); + test('below-floor fixes in round 2 finish with notes, a fix-check included', () => { + expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBelow: true })).toMatchObject({ step: 'done', reason: expect.stringContaining('notes') }); + expect(nextStep({ fullPasses: 1, last: 'fix-check', fixedBelow: true })).toMatchObject({ step: 'done', reason: expect.stringContaining('notes') }); + expect(nextStep({ fullPasses: 1, last: 'fix-check', fixedBelow: true, fixedBlocking: true }).step).toBe('stop'); + }); + test('a light pass 1 whose below-floor fixes change no runtime lines is done; a runtime fix or a deeper diff keeps the fix-check', () => { + expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true, rangeRuntimeLines: 0 }, 'light').step).toBe('done'); + expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true, rangeRuntimeLines: 3 }, 'light').step).toBe('fix-check'); + expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true, rangeRuntimeLines: 0 }, 'full').step).toBe('fix-check'); + expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true }, 'light').step).toBe('fix-check'); + }); test('a blocker open at the cap stops', () => expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', blockingOpen: true }).step).toBe('stop')); test('a blocker open with no fix stops', () => expect(nextStep({ fullPasses: 1, last: 'full', blockingOpen: true }).step).toBe('stop')); test('a fix-check that finds a blocker stops, never a third full pass', () => { @@ -83,8 +98,12 @@ describe('planReview', () => { expect(p.depth).toBe('light'); expect(roles(p)).toEqual([]); }); - test('code class pass 1: specialists at medium, the native adversarial at full, the outside voice at its own', () => { - const p = planReview(base({})); + test('non-risky code pass 1 is light at any size (R51): one batched reader carrying the Red Team prompt and the native adversarial', () => { + const p = planReview(base({ diffLines: 900 })); + expect([p.depth, roles(p)]).toEqual(['light', ['specialist:batched:full', 'adversarial-native:adversarial:full']]); + }); + test('code class pass 1 at standard: specialists at medium, the native adversarial at full, the outside voice at its own', () => { + const p = planReview(base({ floor: 'standard' })); expect(roles(p)).toEqual(['specialist:batched:medium', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']); expect(batch(p)).toEqual(['testing', 'security']); expect(p.skipped).toContain('maintainability, performance, simplification (lean ensemble: read only when forced)'); @@ -162,10 +181,10 @@ describe('GSTA-131 pass-1 review repairs', () => { expect(scanNumbers({ 'test/fixtures/bench.json': ['{ "ratio": 0.74 }'] }).publishes).toBe(true); }); test('--all-specialists forces every specialist; a code diff past 200 lines or with a CRITICAL adds the Red Team', () => { - const all = planReview(base({ force: 'all' })); + const all = planReview(base({ floor: 'standard', force: 'all' })); expect(all.dispatches.filter(d => d.role === 'specialist').length).toBe(1); expect([...batch(all)!].sort()).toEqual(['api-contract', 'data-migration', 'design', 'maintainability', 'performance', 'security', 'simplification', 'state-machines', 'testing']); - const has = (o: Partial) => roles(planReview(base(o))).some(r => r.startsWith('red-team')); + const has = (o: Partial) => roles(planReview(base({ floor: 'standard', ...o }))).some(r => r.startsWith('red-team')); expect(has({ diffLines: 120 })).toBe(false); expect(has({ diffLines: 201 })).toBe(true); expect(has({ diffLines: 120, criticalFound: true })).toBe(true); @@ -175,20 +194,20 @@ describe('GSTA-131 pass-1 review repairs', () => { describe('GSTA-131 coverage: Red Team and specialist arms', () => { const roles = (p: ReturnType) => p.dispatches.map(d => `${d.role}:${d.name}`); test('Red Team runs on code past 200 lines or after a CRITICAL, and on every gate diff', () => { - expect(roles(planReview(base({ diffLines: 120 })))).not.toContain('red-team:red-team'); - expect(roles(planReview(base({ diffLines: 201 })))).toContain('red-team:red-team'); - expect(roles(planReview(base({ diffLines: 120, criticalFound: true })))).toContain('red-team:red-team'); + expect(roles(planReview(base({ diffLines: 120, floor: 'standard' })))).not.toContain('red-team:red-team'); + expect(roles(planReview(base({ diffLines: 201, floor: 'standard' })))).toContain('red-team:red-team'); + expect(roles(planReview(base({ diffLines: 120, floor: 'standard', criticalFound: true })))).toContain('red-team:red-team'); expect(roles(planReview(base({ diffLines: 20, gateTagged: 1 })))).toContain('red-team:red-team'); }); test('scope arms select their specialist, state machines default on when scope is unknown, and --force all reaches docs class', () => { - const picked = (scope: Record) => batch(planReview(base({ scope: { SCOPE_STATE_MACHINE: 'false', ...scope } }))); + const picked = (scope: Record) => batch(planReview(base({ floor: 'standard', scope: { SCOPE_STATE_MACHINE: 'false', ...scope } }))); expect(picked({ SCOPE_MIGRATIONS: 'true' })).toContain('data-migration'); expect(picked({ SCOPE_API: 'true' })).toContain('api-contract'); expect(picked({ SCOPE_FRONTEND: 'true' })).toEqual(['testing', 'security', 'design']); expect(batch(planReview(base({ scope: {} })))).toContain('state-machines'); expect(batch(planReview(base({ scope: { SCOPE_STATE_MACHINE: 'false' } })))).not.toContain('state-machines'); - expect(batch(planReview(base({ force: ['maintainability'] })))).toEqual(['testing', 'security', 'maintainability']); - expect(batch(planReview(base({ gated: ['design'], scope: { SCOPE_FRONTEND: 'true', SCOPE_STATE_MACHINE: 'false' } })))).toEqual(['testing', 'security']); + expect(batch(planReview(base({ floor: 'standard', force: ['maintainability'] })))).toEqual(['testing', 'security', 'maintainability']); + expect(batch(planReview(base({ floor: 'standard', gated: ['design'], scope: { SCOPE_FRONTEND: 'true', SCOPE_STATE_MACHINE: 'false' } })))).toEqual(['testing', 'security']); const docsAll = planReview(base({ paths: ['docs/a.md'], force: 'all' })); expect(docsAll.class).toBe('docs'); expect(docsAll.dispatches.filter(d => d.role === 'specialist').length).toBeGreaterThan(5); @@ -215,8 +234,12 @@ describe('review depth: the matrix', () => { expect(p.skipped.join('; ')).toContain('red-team (light depth'); expect(planReview(mixed({ perPath: per({ 'src/app.ts': 48 }), adversarialRan: true, history: { fullPasses: 1, last: 'full', touchedGate: true, fixedBelow: true } })).dispatches.some(d => d.role === 'adversarial-native')).toBe(false); }); - test('runtime 51 is standard: thresholds read runtime lines, reviewers read their own hunks', () => { + test('runtime 51 non-risky code stays light (R51: size never raises the depth)', () => { const p = planReview(mixed({ perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 400, 'README.md': 3 }), diffLines: 454 })); + expect([p.depth, p.runtimeLines, roles(p)]).toEqual(['light', 51, ['specialist:batched:full', 'adversarial-native:adversarial:full']]); + }); + test('standard (floor or raise): thresholds read runtime lines, reviewers read their own hunks', () => { + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 400, 'README.md': 3 }), diffLines: 454, floor: 'standard' })); expect([p.depth, p.runtimeLines]).toEqual(['standard', 51]); expect(roles(p)).toEqual(['specialist:batched:medium', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']); expect(batch(p)).toEqual(['testing', 'security']); @@ -274,14 +297,16 @@ describe('review depth: the matrix', () => { expect(depthFor({ cls: 'code', runtimeLines: 10, errors: [], floor: 'full' })).toBe('full'); expect(depthFor({ cls: 'code', runtimeLines: 10, errors: [], floor: 'standard' })).toBe('standard'); expect(depthFor({ cls: 'gate', runtimeLines: 10, errors: [], floor: 'standard' })).toBe('full'); - expect(depthFor({ cls: 'code', runtimeLines: 80, errors: [] })).toBe('standard'); + expect(depthFor({ cls: 'code', runtimeLines: 80, errors: [] })).toBe('light'); + expect(depthFor({ cls: 'code', runtimeLines: 3, errors: [], risk: ['data'] })).toBe('full'); const p = planReview(mixed({ perPath: per({ 'src/app.ts': 10 }), floor: 'full' })); expect(p.depth).toBe('full'); expect(p.depthReasons).toContain('floor:full'); }); test('depthOfCommitInput gives the same depth as the plan for the same input', () => { const i = { paths: ['src/app.ts', 'test/a.test.ts'], perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 9 }), gateTagged: 0 }; - expect(depthOfCommitInput(i)).toMatchObject({ depth: 'standard', runtimeLines: 51, rules: DEPTH_RULES, errors: [] }); + expect(depthOfCommitInput(i)).toMatchObject({ depth: 'light', runtimeLines: 51, rules: DEPTH_RULES, errors: [], risk: [] }); + expect(depthOfCommitInput({ ...i, added: { 'src/app.ts': ['const spend_cap_usd = 5;'] } })).toMatchObject({ depth: 'full', risk: ['money'] }); expect(depthOfCommitInput({ ...i, gateTagged: null }).depth).toBe('full'); }); }); @@ -300,6 +325,7 @@ describe('review depth: diff commands and the rules hash', () => { try { const src = fs.readFileSync(path.join(ROOT, 'lib/review-depth.ts'), 'utf8'); fs.symlinkSync(path.join(ROOT, 'lib/ship-policy.ts'), path.join(dir, 'ship-policy.ts')); + fs.symlinkSync(path.join(ROOT, 'lib/review-risk.ts'), path.join(dir, 'review-risk.ts')); const hashOf = (text: string) => { fs.writeFileSync(path.join(dir, 'review-depth.ts'), text); fs.writeFileSync(path.join(dir, 'print.ts'), "import { DEPTH_RULES } from './review-depth';\nconsole.log(DEPTH_RULES);\n"); @@ -369,7 +395,7 @@ describe('D6: the scoped second pass after a blocking fix (GSTA-152 PR B)', () = expect(r(p)).toEqual(['fix-check:fix-check', 'red-team:red-team', 'adversarial-native:adversarial']); }); test('the scoped pass counts toward the cap, and a blocker it leaves stops for the owner', () => { - expect(nextStep(fixed(10, { fullPasses: FULL_PASS_CAP }), 'standard').step).toBe('fix-check'); + expect(nextStep(fixed(10, { fullPasses: FULL_PASS_CAP }), 'standard').step).toBe('stop'); expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', blockingOpen: true, rangeRuntimeLines: 10 }, 'light').step).toBe('stop'); const at = (m: number) => new Date(Date.UTC(2026, 9, 28, 10, m)).toISOString(); const rows = [ @@ -397,6 +423,7 @@ describe('D6: the scoped second pass after a blocking fix (GSTA-152 PR B)', () = const src = fs.readFileSync(path.join(ROOT, 'lib/review-depth.ts'), 'utf8'); expect(src).toContain('SCOPED_RANGE_LINES = 150'); fs.symlinkSync(path.join(ROOT, 'lib/ship-policy.ts'), path.join(dir, 'ship-policy.ts')); + fs.symlinkSync(path.join(ROOT, 'lib/review-risk.ts'), path.join(dir, 'review-risk.ts')); fs.writeFileSync(path.join(dir, 'review-depth.ts'), src.replace('SCOPED_RANGE_LINES = 150', 'SCOPED_RANGE_LINES = 151')); fs.writeFileSync(path.join(dir, 'print.ts'), "import { DEPTH_RULES } from './review-depth';\nconsole.log(DEPTH_RULES);\n"); const res = spawnSync(process.execPath, ['--no-env-file', '--config=/dev/null', path.join(dir, 'print.ts')], { encoding: 'utf8', timeout: 30_000 }); @@ -491,11 +518,11 @@ describe('lean ensemble: one batched specialist at standard and full depth (GSTA expect(specialists(full).map(d => [d.name, d.effort, d.checklists])).toEqual([['batched', 'medium', ['testing', 'security', 'state-machines']]]); }); test('a standard code plan reads one batched reader on its own hunks, and a forced checklist joins it', () => { - const p = planReview(base({ diffLines: 400, scope: { SCOPE_BACKEND: 'true', SCOPE_STATE_MACHINE: 'false' } })); + const p = planReview(base({ diffLines: 400, floor: 'standard', scope: { SCOPE_BACKEND: 'true', SCOPE_STATE_MACHINE: 'false' } })); expect(specialists(p).map(d => [d.name, d.checklists])).toEqual([['batched', ['testing', 'security']]]); expect(roles(p)).toContain('red-team:red-team:full'); - expect(batch(planReview(base({ force: ['performance', 'simplification'] })))).toEqual(['testing', 'security', 'performance', 'simplification']); - expect(specialists(planReview(base({ tiers: false })))[0]!.effort).toBe('host'); + expect(batch(planReview(base({ floor: 'standard', force: ['performance', 'simplification'] })))).toEqual(['testing', 'security', 'performance', 'simplification']); + expect(specialists(planReview(base({ floor: 'standard', tiers: false })))[0]!.effort).toBe('host'); }); }); diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index ab7ff8240..747723390 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -56,6 +56,7 @@ const NO_LOSS_REPLAYS = { 'G1': "GSTA-167 variant G Arm G1: live no-loss replay of both prose voices with part 1's prompts, 2 passes, normalizer v2, paired rule against pooled C' (parts 1 and 3; specialists at medium as in C and C2)", '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", + '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 = { @@ -77,7 +78,8 @@ const REVIEWER_BRIEFS: Record = { 'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' }, 'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' }, 'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' }, - 'lib/review-depth.ts': { sha: '4542785a1e217d8dd054985978b42c055b9fd79082579cb7664f187f7066e48e', replay: 'GST-257' }, + 'lib/review-depth.ts': { sha: 'd649569a163c03476c6e0c5ea295f5d02088f96aab85fbc7d558f2ac2b27006a', replay: 'R51' }, + 'lib/review-risk.ts': { sha: 'dbef7475d467e17340b21a2bacb52a16a7ccb1ee66291900ed2d36dfb5cfb7f2', replay: 'R51' }, }; describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => { diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index a4d25d144..46f3df0f7 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -522,7 +522,7 @@ describe('Capy pass program (fake host, real bins)', () => { fs.mkdirSync(files, { recursive: true }); const fc: Record = {}; for (const [k, v] of [['fix_list', '1. src/a.ts: guard v80\n'], ['hunks', 'diff --git a/src/a.ts b/src/a.ts\n+export const v81 = 81;\n'], ['tests', 'bun test: 3 pass, 0 fail\n'], ['callers', 'none\n']]) { fc[k] = path.join(files, k + '.txt'); fs.writeFileSync(fc[k]!, v!); } - const planFlags = ['--pass', '2', '--last', 'full', '--fixed-blocking']; + const planFlags = ['--pass', '1', '--last', 'full', '--fixed-below']; const missing = await runPass(host(dir, env), { base: 'main', out: path.join(dir, '.state/run/fc0'), pass: 'x19fc0', planFlags }); expect(missing).toContain('STOP: gstack-review-dispatch pass exited'); expect(missing).toContain('the plan dispatches a fix-check: pass needs --fix-list, --hunks and --tests'); diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index f81dc7d7a..eee3e5159 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -35,18 +35,18 @@ describe('gstack-review-plan', () => { }); test('--json carries the class, the next step, every dispatch with its effort and the audit effort; --no-tiers renders specialists and audits at host effort', () => { const r = repo({ 'src/a.ts': Array.from({ length: 80 }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n' }); - const plan = JSON.parse(run(r, ['--json']).stdout); + const plan = JSON.parse(run(r, ['--json', '--depth-floor', 'standard']).stdout); expect(plan).toMatchObject({ class: 'code', next: { step: 'full' }, pass: 0 }); expect(plan.dispatches.filter((d: any) => d.role === 'specialist').map((d: any) => d.effort)).toEqual(expect.arrayContaining(['medium'])); expect(plan.dispatches.filter((d: any) => d.role === 'specialist').every((d: any) => d.effort === 'medium')).toBe(true); expect(plan.dispatches.find((d: any) => d.role === 'adversarial-native').effort).toBe('full'); expect(plan.audit_effort).toBe('light'); - expect(run(r, []).stdout).toContain('DISPATCH: specialist batched effort=medium checklists=testing,security,state-machines'); - const host = JSON.parse(run(r, ['--json', '--no-tiers']).stdout); + expect(run(r, ['--depth-floor', 'standard']).stdout).toContain('DISPATCH: specialist batched effort=medium checklists=testing,security,state-machines'); + const host = JSON.parse(run(r, ['--json', '--no-tiers', '--depth-floor', 'standard']).stdout); expect(host.audit_effort).toBe('host'); expect(host.dispatches.filter((d: any) => d.role === 'specialist').every((d: any) => d.effort === 'host')).toBe(true); expect(host.dispatches.find((d: any) => d.role === 'adversarial-native').effort).toBe('full'); - expect(run(r, ['--no-tiers']).stdout).toContain('DISPATCH: specialist batched effort=host checklists=testing,security,state-machines'); + expect(run(r, ['--no-tiers', '--depth-floor', 'standard']).stdout).toContain('DISPATCH: specialist batched effort=host checklists=testing,security,state-machines'); }); test('a below-floor fix after pass 1 plans one fix-check', () => { const r = run(repo({ 'src/a.ts': 'export const a = 1;\n' }), ['--pass', '1', '--last', 'full', '--fixed-below']); @@ -132,7 +132,7 @@ describe('gstack-review-plan: review depth', () => { const r = repo({ 'src/a.ts': code(10), 'test/a.test.ts': code(200) }); const out = run(r, []).stdout; const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout.trim(); - expect(out).toMatch(/^REVIEW_DEPTH: light runtime_lines=10 rules=[0-9a-f]{16} reasons=runtime=10<=50 codes=none$/m); + expect(out).toMatch(/^REVIEW_DEPTH: light runtime_lines=10 rules=[0-9a-f]{16} reasons=runtime=10 codes=none$/m); expect(out).toContain(`GATE_SCAN: tagged=0 head=${head}`); expect(out).toContain('DISPATCH: specialist batched effort=full checklists=testing,security,state-machines,red-team'); expect(out).toContain('DISPATCH: adversarial-native adversarial effort=full reason=backtest inconclusive (GSTA-147 D5)'); @@ -149,7 +149,7 @@ describe('gstack-review-plan: review depth', () => { }); test('--json carries depth, runtime_lines, rules, errors, gate_scan and every dispatch\'s exclude array', () => { const r = repo({ 'src/a.ts': code(80), 'README.md': '# y\n' }); - const plan = JSON.parse(run(r, ['--json']).stdout); + const plan = JSON.parse(run(r, ['--json', '--depth-floor', 'standard']).stdout); expect(plan).toMatchObject({ depth: 'standard', step_depth: 'standard', runtime_lines: 80, errors: [], codes: [], gate_scan: { tagged: 0 } }); expect(plan.rules).toMatch(/^[0-9a-f]{16}$/); expect(plan.dispatches.find((d: any) => d.name === 'batched').exclude).toEqual([':(exclude)README.md']); @@ -157,7 +157,7 @@ describe('gstack-review-plan: review depth', () => { }); test('--depth-floor full raises a light diff, never lowers a full one, and an invalid floor is a usage error', () => { const r = repo({ 'src/a.ts': code(5) }); - expect(run(r, ['--depth-floor', 'full']).stdout).toMatch(/^REVIEW_DEPTH: full runtime_lines=5 .*reasons=runtime=5<=50,floor:full /m); + expect(run(r, ['--depth-floor', 'full']).stdout).toMatch(/^REVIEW_DEPTH: full runtime_lines=5 .*reasons=runtime=5,floor:full /m); const gate = repo({ 'src/a.ts': code(5), '.github/workflows/ci.yml': 'on: push\n' }); expect(run(gate, ['--depth-floor', 'standard']).stdout).toMatch(/^REVIEW_DEPTH: full /m); expect(run(r, ['--depth-floor', 'light']).status).toBe(2); @@ -190,7 +190,7 @@ describe('gstack-review-plan: the scoped second pass (GSTA-152 D6)', () => { const out = run(r, [...pass2, '--range-base', rangeBase]).stdout; expect(out).toMatch(/^REVIEW_PLAN: class=code pass=2 next=scoped dispatches=3 /m); expect(out).toContain(`FIX_RANGE: runtime_lines=24 base=${rangeBase}`); - expect(out).toMatch(/^NEXT: scoped \(a blocking finding was fixed on a standard diff with 24 runtime lines in the fix range/m); + expect(out).toMatch(/^NEXT: scoped \(a blocking finding was fixed on a light diff with 24 runtime lines in the fix range/m); expect(out).toContain('DISPATCH: fix-check fix-check effort=full'); expect(out).toContain('DISPATCH: red-team red-team effort=full'); expect(out).toContain('DISPATCH: adversarial-native adversarial effort=full'); @@ -221,7 +221,7 @@ describe('gstack-review-plan: the scoped second pass (GSTA-152 D6)', () => { fs.writeFileSync(path.join(r.dir, 'src', 'b.ts'), code(2, 'b')); git('add', '.'); git('commit', '-qm', 'fix: revert the workflow edit'); const out = run(r, [...pass2, '--range-base', rangeBase]).stdout; - expect(out).toMatch(/^REVIEW_DEPTH: standard /m); + expect(out).toMatch(/^REVIEW_DEPTH: light .* step=standard$/m); expect(out).toMatch(/^NEXT: full \(a blocking finding was fixed and a fix touched a gate-class path/m); expect(JSON.parse(run(r, [...pass2, '--range-base', rangeBase, '--json']).stdout)).toMatchObject({ next: { step: 'full' }, fix_range: { touched_gate: true } }); }, 30_000); diff --git a/test/review-risk-routing.test.ts b/test/review-risk-routing.test.ts new file mode 100644 index 000000000..7466c3bdb --- /dev/null +++ b/test/review-risk-routing.test.ts @@ -0,0 +1,123 @@ +/** + * R51: review scales to risk, not size (docs/review-policy.md#review-depth, #risk-classes). The risk classes + * (lib/review-risk.ts), the round cap, and the routing table: every gate, + * security, user-data and money fixture still plans the full panel, and non-risky code is light at any size. + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { classifyDiff, nextStep, planReview, scanNumbers, type PlanInput } from '../lib/review-depth'; +import { matchRisk, RISK_RULES } from '../lib/review-risk'; +import { targetedClasses } from '../lib/review-targeted'; +import { parseReviewLoop, receiptSummaryLine } from '../lib/ship-receipt'; + +const ROOT = path.join(import.meta.dir, '..'); +const risk = (codePaths: string[], added: Record = {}, scopeAuth = false) => [...matchRisk({ codePaths, added, removed: {}, scopeAuth }).keys()]; + +describe('risk classes', () => { + // Value: protects=security and user-data code reaches the full panel; fails_when=a trust-boundary or data-loss signal is missed, + // or an unrelated path or line raises a class; seam=matchRisk + test('security and data: paths and lines, each with a negative', () => { + expect(risk(['src/core/trust/owner-actions.ts'])).toEqual(['security']); + expect(risk(['lib/redact-patterns.ts'])).toEqual(['security']); + expect(risk(['src/run.ts'], { 'src/run.ts': ['const child = spawn(cmd, { shell: true });'] })).toEqual(['security']); + expect(risk(['src/view.tsx'], { 'src/view.tsx': ['
'] })).toEqual(['security']); + expect(risk(['bin/x.sh'], { 'bin/x.sh': ['eval "$CMD"'] })).toEqual(['security']); + expect(risk(['src/run.ts'], { 'src/run.ts': ['const m = re.exec(line);', 'const s = trustee.name;'] })).toEqual([]); + expect(risk(['src/core/persistence/git-door.ts'])).toEqual(['data']); + expect(risk(['src/db/users.ts'], { 'src/db/users.ts': ['await sql`DELETE FROM users WHERE id = ${id}`;'] })).toEqual(['data']); + expect(risk(['app/models/user.rb'], { 'app/models/user.rb': ['User.where(stale: true).destroy_all'] })).toEqual(['data']); + expect(risk(['src/list.ts'], { 'src/list.ts': ['// delete from the list when done', 'arr.splice(i, 1);'] })).toEqual([]); + for (const line of ['db.exec("delete from users");', 'await sql`delete from sessions where id = 1`;', 'DELETE FROM users;', 'drop database app;', 'DROP SCHEMA s;', + 'await sql`DELETE FROM ${table} WHERE id = ?`;', 'DELETE FROM users u WHERE u.stale', 'DELETE FROM sessions USING users WHERE x', 'DELETE FROM users RETURNING id', 'const q = "DELETE FROM " + table;']) + expect(risk(['src/db.ts'], { 'src/db.ts': [line] })).toEqual(['data']); + for (const line of ["const r = spawn('sh', ['-c', cmd], { timeout: 1 });", "Bun.spawn(['bash', '-lc', x]);", "fs.rmSync(t, { recursive: true, force: true });", 'rm -rf "$DIR"']) + expect(risk(['src/run.ts'], { 'src/run.ts': [line] })).toEqual(['security']); + expect(risk(['scripts/test-pr-profile.ts', 'lib/storyboard.ts'])).toEqual([]); + expect(RISK_RULES).toMatch(/^[0-9a-f]{16}$/); + }); + // Value: protects=the targeted pass keeps its exact classes now that its matchers live in lib/review-risk.ts; fails_when=the move + // changes a money, auth or secret verdict; seam=targetedClasses over matchRisk + test('money, auth and secret come from the same matchers the targeted pass reads', () => { + const lines = { 'src/run.ts': ['const spend_cap_usd = 5;', 'const k = process.env.OPENAI_API_KEY;'] }; + expect(risk(['src/run.ts', 'src/auth/session.ts'], lines, true)).toEqual(['money', 'auth', 'secret']); + expect(targetedClasses({ paths: ['src/run.ts', 'src/auth/session.ts'], added: lines, removed: {}, scope: { SCOPE_AUTH: 'true' }, gateTagged: 0 }).classes).toEqual(['money', 'auth', 'secret']); + }); +}); + +describe('UI text and SVGs stay code (R51: the docs paths are deferred to an allowlist classifier)', () => { + // Value: protects=a copy edit, a Vue binding or a namespaced SVG script never plans as docs; fails_when=any of them leaves the code + // class or its lines skip the risk scan; seam=classifyDiff + test.each([ + ['a JSX copy edit', 'src/App.tsx', ['

Old copy

'], ['

New copy

']], + ['a Vue binding', 'src/App.vue', ['
Profile
'], ['
Profile
']], + ['a locale value', 'locales/en.yml', ['save: "Save"'], ['save: ""']], + ['a namespaced SVG script', 'docs/chart.svg', [], ['window.x=42']], + ])('%s is code', (_n, p, removed, added) => { + expect(classifyDiff({ paths: [p], gateTagged: 0, added: { [p]: added }, removed: { [p]: removed }, perPath: [{ path: p, lines: removed.length + added.length, binary: false }] }).class).toBe('code'); + }); +}); + +describe('round 2 never ships an unread gate edit', () => { + // Value: protects=a gate-class edit after round 1 always gets the owner's eyes; fails_when=a round-2 or fix-check gate edit + // returns done; seam=nextStep + test('a gate edit in round 2 or in the fix-check stops for the owner; a plain below-floor fix there finishes', () => { + expect(nextStep({ fullPasses: 2, last: 'full', fixedBelow: true, touchedGate: true })).toMatchObject({ step: 'stop', reason: expect.stringContaining('gate-class') }); + expect(nextStep({ fullPasses: 1, last: 'fix-check', touchedGate: true })).toMatchObject({ step: 'stop', reason: expect.stringContaining('gate-class') }); + expect(nextStep({ fullPasses: 2, last: 'full', fixedBelow: true }).step).toBe('done'); + }); +}); + +const none = scanNumbers({}); +const plan = (o: Partial) => planReview({ paths: ['src/app.ts'], scope: { SCOPE_STATE_MACHINE: 'false' }, gateTagged: 0, diffLines: 10, numbers: none, history: { fullPasses: 0, last: 'none' }, ...o }); +const roles = (p: ReturnType) => [...new Set(p.dispatches.map(d => d.role))]; +const FULL_PANEL = ['specialist', 'red-team', 'adversarial-native', 'outside-voice']; +const captured = (name: string): Partial => { + const x = JSON.parse(fs.readFileSync(path.join(ROOT, 'test/fixtures/review-depth', name + '.json'), 'utf8')); + return { paths: x.paths, deleted: x.deleted, stampOnly: x.stamp_only ?? [], scope: x.scope, gateTagged: x.gate_tagged, diffLines: x.diff_lines, numbers: x.numbers, perPath: x.per_path }; +}; +const one = (p: string, lines: string[] = ['export const a = 1;']) => ({ paths: [p], added: { [p]: lines }, removed: {}, perPath: [{ path: p, lines: lines.length, binary: false }] }); + +describe('routing: every gate, security, user-data and money fixture plans the full panel', () => { + // Value: protects=R51 never lowers review on risky code; fails_when=a gate or risk fixture plans less than outside voice, Red Team, + // batched specialist and the native adversarial pass, or loses the targeted reader it had; seam=planReview + test.each([ + ['captured gate fixture', captured('code'), []], + ['gate path', one('.github/workflows/ci.yml', ['on: push']), []], + ['gate tag', { ...one('src/a.ts'), gateTagged: 2 }, ['gate']], + ['deleted test', { ...one('src/a.ts'), deleted: ['test/a.test.ts'] }, []], + ['auth scope', { ...one('src/auth/session.ts'), scope: { SCOPE_AUTH: 'true' } }, ['auth']], + ['migration scope', { ...one('db/migrate/001.sql', ['ALTER TABLE users ADD COLUMN email text;']), scope: { SCOPE_MIGRATIONS: 'true' } }, []], + ['secret path', one('lib/credentials.ts'), ['secret']], + ['secret line', one('src/client.ts', ['const k = process.env.OPENAI_API_KEY;']), ['secret']], + ['money path', one('src/billing/invoice.ts'), ['money']], + ['money line', one('src/charge.ts', ['export const spend_cap_usd = 5;']), ['money']], + ['security path', one('lib/redact-patterns.ts'), []], + ['security line', one('src/run.ts', ['const run = new Function(body);']), []], + ['user-data path', one('src/core/persistence/store.ts'), []], + ['user-data line', one('src/db/purge.ts', ['await db.users.deleteMany({ where: { stale: true } });']), []], + ] as const)('%s', (_n, o, targeted) => { + const p = plan({ ...(o as Partial), targeted: { classes: [...targeted] } }); + expect(p.depth).toBe('full'); + expect(FULL_PANEL.filter(r => !roles(p).includes(r as never))).toEqual([]); + expect(roles(p).includes('targeted')).toBe(targeted.length > 0); + }); + test('non-risky code is light at any size: one batched reader with the Red Team prompt and the native adversarial', () => { + const p = plan({ ...one('src/util.ts', Array.from({ length: 900 }, (_, n) => `export const v${n} = ${n};`)), diffLines: 900 }); + expect([p.depth, roles(p)]).toEqual(['light', ['specialist', 'adversarial-native']]); + const docs = plan({ paths: ['docs/evals/run.md'], perPath: [{ path: 'docs/evals/run.md', lines: 400, binary: false }], numbers: scanNumbers({ 'docs/evals/run.md': ['recall 33/41'] }) }); + expect([docs.depth, roles(docs)]).toEqual(['light', ['outside-voice']]); + }); +}); + +describe('the receipt records the tier and the rounds', () => { + // Value: protects=a receipt carries review_loop.rounds and the summary line prints depth and rounds; fails_when=the parser drops + // rounds or a legacy loop without it gains one; seam=parseReviewLoop and receiptSummaryLine + test('rounds parse and print; a legacy loop prints no rounds', () => { + expect(parseReviewLoop({ passes: 1, blocking_found: 0, deferred: 0, fix_checks: 1, rounds: 2 })).toMatchObject({ rounds: 2 }); + const r = { pr: 7, head: 'a'.repeat(40), base: 'main', review_depth: { depth: 'light' }, review_loop: { passes: 1, blocking_found: 0, deferred: 0, fix_checks: 1, rounds: 2 } } as never; + expect(receiptSummaryLine(r)).toContain('depth=light passes=1 blocking_found=0 deferred=0 fix_checks=1 rounds=2'); + const legacy = { pr: 7, head: 'a'.repeat(40), base: 'main', review_loop: { passes: 2, blocking_found: 1, deferred: 0 } } as never; + expect(receiptSummaryLine(legacy)).not.toContain('rounds='); + }); +}); diff --git a/test/review-scope-followups.test.ts b/test/review-scope-followups.test.ts index ec72f31c5..d2ff1aed5 100644 --- a/test/review-scope-followups.test.ts +++ b/test/review-scope-followups.test.ts @@ -30,7 +30,7 @@ describe('late defers from Step 11', () => { { skill: 'adversarial-review', via: 'ship', timestamp: '2026-10-26T10:25:00Z', findings: [{ fingerprint: 'd:1:w', blocking: true, action: 'deferred' }] }, ]; const s = summarizeReviewLoop(rows, '2026-10-26T09:30:00Z'); - expect(s).toEqual({ passes: 2, blocking_found: 3, deferred: 2, noted: 0, fixed: 1, scope: 'frozen' }); + expect(s).toEqual({ passes: 2, blocking_found: 3, deferred: 2, noted: 0, fixed: 1, scope: 'frozen', rounds: 2 }); expect(reviewLoopLine(s)).toBe('REVIEW_LOOP: passes=2 blocking_found=3 deferred=2 noted=0 fixed=1 scope=frozen'); expect(LATE_DEFER_SKILL).toBe('review-late-defer'); // A `review` row logged without a start token would be the newest review row and read UNVERIFIED; the late-defer row is not a diff review at all. diff --git a/test/review-scope.test.ts b/test/review-scope.test.ts index ec9108865..e6bdc2ace 100644 --- a/test/review-scope.test.ts +++ b/test/review-scope.test.ts @@ -208,7 +208,7 @@ describe('loop summary', () => { { skill: 'review', timestamp: '2026-10-26T10:40:00Z', findings: [{ fingerprint: 'e:1:v', blocking: true }] }, ]; const s = summarizeReviewLoop(rows, '2026-10-26T09:30:00Z'); - expect(s).toEqual({ passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen' }); + expect(s).toEqual({ passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen', rounds: 2 }); expect(reviewLoopLine(s)).toBe('REVIEW_LOOP: passes=2 blocking_found=2 deferred=1 noted=1 fixed=1 scope=frozen'); }); @@ -223,7 +223,7 @@ describe('loop summary', () => { { skill: 'review', via: 'ship', review_binding: { state: 'changed', started_at: '2026-10-26T09:00:00.000Z' }, findings: [{ fingerprint: 'old:1:q', action: 'noted' }] }, { skill: 'review', via: 'ship', findings: [{ fingerprint: 'undated:1:q', action: 'noted' }] }, ]; - expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 3, blocking_found: 1, deferred: 0, noted: 2, fixed: 1, scope: 'frozen' }); + expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 3, blocking_found: 1, deferred: 0, noted: 2, fixed: 1, scope: 'frozen', rounds: 3 }); }); }); @@ -421,7 +421,7 @@ describe('gstack-review-scope (bin)', () => { { skill: 'review', timestamp: '2026-10-26T10:40:00Z', findings: [{ fingerprint: 'e:1:v', blocking: true, action: 'fixed' }] }, ]; fs.writeFileSync(path.join(logDir, `${branch}-reviews.jsonl`), rows.map(r => JSON.stringify(r)).join('\n') + '\n{not json\n'); - const expected = { passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen' }; + const expected = { passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen', rounds: 2 }; const out = path.join(repo.root, 'review-loop.json'); const r = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', out, '--json']); expect(r.status).toBe(EXIT.ok); diff --git a/test/review-targeted.test.ts b/test/review-targeted.test.ts index a3232418e..375cf1d01 100644 --- a/test/review-targeted.test.ts +++ b/test/review-targeted.test.ts @@ -110,8 +110,8 @@ describe('planReview dispatches the targeted reader', () => { test('at light, standard and full depth when a class hit, never when none did', () => { expect(plan({ targeted: { classes: ['money'] } })).toMatchObject({ depth: 'light' }); expect(roles(plan({ targeted: { classes: ['money'] } }))).toContain('targeted full'); - expect(plan({ diffLines: 80, perPath: [{ path: 'src/a.ts', lines: 80, binary: false }], targeted: { classes: ['secret'] } }).depth).toBe('standard'); - expect(roles(plan({ diffLines: 80, perPath: [{ path: 'src/a.ts', lines: 80, binary: false }], targeted: { classes: ['secret'] } }))).toContain('targeted full'); + expect(plan({ diffLines: 80, perPath: [{ path: 'src/a.ts', lines: 80, binary: false }], targeted: { classes: ['secret'] }, floor: 'standard' }).depth).toBe('standard'); + expect(roles(plan({ diffLines: 80, perPath: [{ path: 'src/a.ts', lines: 80, binary: false }], targeted: { classes: ['secret'] }, floor: 'standard' }))).toContain('targeted full'); expect(plan({ gateTagged: 1, targeted: { classes: ['gate'] } }).depth).toBe('full'); expect(roles(plan({ gateTagged: 1, targeted: { classes: ['gate'] } }))).toContain('targeted full'); expect(roles(plan({ targeted: { classes: [] } }))).not.toContain('targeted full'); @@ -183,6 +183,18 @@ function planRepo(f: Fixture) { const run = (args: string[]) => spawnSync(PLAN, ['--base', 'main', ...(args.includes('--gate-tagged') ? [] : ['--gate-tagged', '0']), ...args], { cwd: dir, encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_STATE_ROOT: state } }); return { dir, run, write }; } +/** + * R51 (risk, not size): the depth each fixture plans now. Every risk or gate fixture plans full and keeps every role main + * dispatched, plus the full panel; a non-risky fixture keeps main's reviewers except the roles listed (size no longer raises it). + */ +const FULL_PANEL = ['specialist', 'red-team', 'adversarial-native', 'outside-voice']; +const R51: Record = { + 'none-light': { depth: 'light', panel: false }, 'none-standard': { depth: 'light', panel: false, dropped: ['outside-voice'] }, 'none-docs': { depth: 'light', panel: false }, + 'none-tests': { depth: 'light', panel: false }, 'none-costume': { depth: 'light', panel: false }, 'none-gate-path': { depth: 'full', panel: true }, + 'secret-light': { depth: 'full', panel: true }, 'secret-path': { depth: 'full', panel: true }, 'secret-removed': { depth: 'full', panel: true }, 'gate-tagged': { depth: 'full', panel: true }, + 'money-light': { depth: 'full', panel: true }, 'money-path': { depth: 'full', panel: true }, 'money-removed': { depth: 'full', panel: true }, 'money-standard': { depth: 'full', panel: true }, + 'auth-path': { depth: 'full', panel: true }, 'money-fix-check': { depth: 'full', panel: false }, 'money-pass2': { depth: 'full', panel: false }, 'gate-repeat': { depth: 'full', panel: false }, +}; const field = (out: string, prefix: string, key: string) => new RegExp(`^${prefix}.* ${key}=(\\S+)`, 'm').exec(out)?.[1]; describe('gstack-review-plan prints TARGETED and dispatches the targeted reader', () => { @@ -190,17 +202,23 @@ describe('gstack-review-plan prints TARGETED and dispatches the targeted reader' // standard and full depth and on fix-check, scoped and repeat steps, is still dispatched unchanged, and the only addition is the // targeted reader on a class diff; fails_when=the change drops, renames or re-efforts any reviewer, or adds the reader on a diff // with no class; seam=bin/gstack-review-plan end to end; no_claim=that the reader finds the defect (the paid replay's question) - test('the dispatch-superset no-loss check: on every fixture the branch dispatches main\'s reviewers plus only the targeted reader', () => { + // R51 changes the table: risk and gate fixtures plan full (every role main dispatched, plus the full panel), and the one + // non-risky standard fixture drops only the outside voice; fails_when=a risk fixture loses a role main dispatched or misses + // the full panel, or a non-risky fixture loses a role the R51 table does not list + test('the dispatch-superset no-loss check: risk and gate fixtures keep every role main dispatched plus the full panel; non-risky ones drop only what R51 lists', () => { expect(Object.keys(MAIN).sort()).toEqual(Object.keys(FIXTURES).sort()); - expect(new Set(Object.values(MAIN).map(m => m.depth))).toEqual(new Set(['light', 'standard', 'full'])); + expect(Object.keys(R51).sort()).toEqual(Object.keys(FIXTURES).sort()); + const role = (d: string) => d.split(' ')[0]!; for (const [name, f] of Object.entries(FIXTURES)) { const r = planRepo(f).run(f.args ?? []); expect(r.status, `${name}: ${r.stderr}`).toBe(0); const got = r.stdout.split('\n').filter(l => l.startsWith('DISPATCH: ')).map(l => l.slice('DISPATCH: '.length)); - const want = MAIN[name]!; - expect(r.stdout, name).toContain(`REVIEW_DEPTH: ${want.depth} `); - expect(got.filter(d => !want.main.includes(d)), name).toEqual(want.classes === 'none' ? [] : ['targeted targeted effort=full']); - expect(want.main.filter(d => !got.includes(d)), name).toEqual([]); + const want = MAIN[name]!, now = R51[name]!; + expect(r.stdout, name).toContain(`REVIEW_DEPTH: ${now.depth} `); + const roles = new Set(got.map(role)); + expect([...new Set(want.main.map(role))].filter(x => !roles.has(x)), name).toEqual(now.dropped ?? []); + if (now.panel) expect(FULL_PANEL.filter(x => !roles.has(x)), name).toEqual([]); + expect(roles.has('targeted'), name).toBe(want.classes !== 'none'); expect(field(r.stdout, 'TARGETED:', 'classes'), name).toBe(want.classes); } }, 180_000); diff --git a/test/ship-frozen-scope-actor.test.ts b/test/ship-frozen-scope-actor.test.ts index a970913d7..5c65d6ea9 100644 --- a/test/ship-frozen-scope-actor.test.ts +++ b/test/ship-frozen-scope-actor.test.ts @@ -86,7 +86,7 @@ posixTest('a scripted actor that fixes the diff, defers the pre-existing defect await act(f); const verdict = await shipFrozenScopeFailures(f, { exitReason: 'success' }); expect(verdict.failures).toEqual([]); - expect(verdict.evidence.loop).toEqual({ passes: 2, blocking_found: 2, deferred: 1, noted: 0, fixed: 1, scope: 'frozen' }); + expect(verdict.evidence.loop).toEqual({ passes: 2, blocking_found: 2, deferred: 1, noted: 0, fixed: 1, rounds: 2, scope: 'frozen' }); const rows = verdict.evidence.rows as Array>; expect(rows.map(r => r.pass)).toEqual([1, 2]); expect(rows[0]!.findings.map((x: any) => [x.fingerprint, x.action])).toEqual([[FROZEN_SCOPE_FIX_IDENTITY, 'auto-fixed'], [FROZEN_SCOPE_DEFER_IDENTITY, 'deferred']]); diff --git a/test/ship-receipt-depth.test.ts b/test/ship-receipt-depth.test.ts index cb86a7872..4b7cf240f 100644 --- a/test/ship-receipt-depth.test.ts +++ b/test/ship-receipt-depth.test.ts @@ -57,25 +57,26 @@ describe('write records the review depth at the checked-out head', () => { describe('the queue refuses a receipt below the depth its head needs', () => { // Value: protects=a receipt reviewed lighter than its head needs never READYs or seats, and the refusal says what to do; // fails_when=the queue trusts the recorded depth, or prints no required depth, reasons and inputs; seam=bindShippedReceipt - test('light on a standard head, a missing depth from a current writer and an unbound gate scan refuse; a legacy receipt is accepted', () => { + // R51: size never raises the depth, so the head that needs more than light is a risk-class one (a money path needs full). + test('standard on a full (money) head, a missing depth from a current writer and an unbound gate scan refuse; a legacy receipt is accepted', () => { const repo = makeFixtureRepo(); repos.push(repo); - const head = commit(repo, { 'lib/big.js': lines(80) }); + 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(receiptIn(body).review_depth).toMatchObject({ depth: 'standard', runtime_lines: 80 }); + 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]); const good = bind(fs.readFileSync(body, 'utf8')); expect(good.ok).toBe(true); - expect(good.ok && good.depthLine).toContain('REVIEW_DEPTH: receipt=standard required=standard runtime_lines=80 class=code'); + expect(good.ok && good.depthLine).toContain('REVIEW_DEPTH: receipt=full required=full runtime_lines=80 class=code'); - const low = bind(rewrite(body, r => { r.review_depth.depth = 'light'; })); + const low = bind(rewrite(body, r => { r.review_depth.depth = 'standard'; })); expect(low.ok).toBe(false); if (low.ok) return; expect(low.code).toBe('QUEUE_RECEIPT_DEPTH_RAISED'); - expect(low.depthLine).toContain('receipt=light required=standard runtime_lines=80'); + expect(low.depthLine).toContain('receipt=standard required=full runtime_lines=80'); expect(low.depthLine).toContain(`inputs=base@${base},registry@${base},gate_scan@${head.slice(0, 12)}`); - expect(low.message).toContain('runtime=80>50'); + expect(low.message).toContain('risk:money'); expect(low.message).toContain(`the receipt is unchanged; run /ship on ${head.slice(0, 12)}`); const unbound = bind(rewrite(body, r => { r.review_depth.depth = 'standard'; r.review_depth.gate_scan.head = 'a'.repeat(40); })); @@ -218,7 +219,8 @@ describe('the queue and a scoped second pass (GSTA-152 D6)', () => { expect(receiptIn(body).review_loop).toMatchObject({ passes: 2, dispatches: loop.dispatches }); const bound = bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8'), '7', [head]); expect(bound.ok).toBe(true); - expect(bound.ok && bound.depthLine).toContain('REVIEW_DEPTH: receipt=standard required=standard'); + expect(bound.ok && bound.depthLine).toContain('REVIEW_DEPTH: receipt=light required=light'); + expect(bound.ok && bound.depthLine).toMatch(/ rounds=2$/); }, 60_000); }); From dd2416f4eb1aa6099d841c9206dea4f85cec281d Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 1 Nov 2026 15:20:55 -0800 Subject: [PATCH 4/8] v1.92.46.0 fix(brain-sync): never commit a synced JSON file that does not parse (#3763) (#3796) * test: synced JSON that does not parse is never committed and is reported (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(brain-sync): hold a synced JSON file that does not parse; --profile and doctor report it (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(brain-sync): read JSON index blobs with one cat-file batch, no python subprocess (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(brain-sync): block instead of committing a one-sided merge when a held file is mid-merge (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(brain-sync): merge-block status names the real recovery and waiting files (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(brain-sync): JSON holds in the --status, held-files and two-machine sections (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(brain-sync): #3704 fixtures write valid JSON to developer-profile.json (#3763) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(brain-sync): keep the reviewed JSON paragraph wording after the main merge (#3763) 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> --- CHANGELOG.md | 16 +++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-brain-sync | 79 ++++++++++-- bin/gstack-developer-profile | 12 ++ bin/gstack-doctor | 18 ++- docs/gbrain-sync-errors.md | 9 +- docs/gbrain-sync.md | 12 +- package.json | 2 +- test/brain-sync.test.ts | 176 ++++++++++++++++++++++---- test/gstack-developer-profile.test.ts | 18 +++ test/gstack-doctor.test.ts | 19 +++ test/gstack-skill-start.test.ts | 2 +- 13 files changed, 319 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7a2763c2..2619a01d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## [1.92.46.0] - 2026-11-01 + +**brain-sync no longer publishes a synced JSON file that doesn't parse, and the profile and doctor now say when one is broken.** When two machines edit `developer-profile.json` before syncing, git's merge can leave conflict markers in it. brain-sync committed and pushed that file, so every machine got a profile that no longer parsed, while `gstack-developer-profile --profile` and `gstack-doctor` both reported everything ok (#3763). + +### Fixed +- **brain-sync holds back a synced `.json` file that doesn't parse** (`bin/gstack-brain-sync`). Before each commit it parses every staged `*.json` file (today `developer-profile.json` and `projects/*/decisions.active.json`). A file that doesn't parse is held with rule `invalid-json`, along with any file coupled to it (`decisions.jsonl` with its snapshot). It stays queued and syncs on the next run after you repair it; everything else syncs as usual. The fix line says to repair the file. It does not offer `--skip-file`, which would stop the profile syncing for good. +- **A held file in an unfinished git merge stops the sync instead of committing half the merge.** If a held file is part of a merge that git could not finish, committing the merge without it would quietly keep only this machine's version and drop the other machine's edit. brain-sync now commits nothing and reports `blocked` until you repair the file; the merge and the queue are kept. +- **Held-status messages name the right cause.** The skill-start attention line, `--status` and `gstack-doctor` no longer blame the secret scan for every held file; doctor reports how many held files are not valid JSON. +- **`gstack-doctor` warns when a synced JSON file holds conflict markers**, naming the file and how to restore it from the sync repo's history. + +### Changed +- **`gstack-developer-profile --profile` now exits 1 when the profile is not valid JSON.** It prints ` is not valid JSON: ` on stderr (noting conflict markers when it finds them) and nothing on stdout. It used to print the broken file and exit 0, so scripts that read `--profile` should check its exit status. + +### Tests +- `test/brain-sync.test.ts`: a queued profile with conflict markers, a marker-free syntax error, an invalid `decisions.active.json` holding its coupled `decisions.jsonl` until repaired, two machines editing the profile (the other machine's edit stays on the remote), and an unfinished merge with a held profile. `test/gstack-developer-profile.test.ts`: `--profile` on an unparseable profile. `test/gstack-doctor.test.ts`: a conflict-marker warning, and a JSON hold not attributed to the secret scan. All fail on main. + ## [1.92.45.0] - 2026-11-01 ### /ship review scales to risk, not size diff --git a/VERSION b/VERSION index 603bff1c3..cbd266e1d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.45.0 +1.92.46.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index d00c52753..7d20d84f4 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.45.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.46.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index 25625c2bb..c95c6985e 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -38,11 +38,11 @@ # applies; `ok` and `idle` print none. is this script's absolute path # and the held count. Rows are read by test/gstack-skill-start.test.ts. # -# held trigger: status=held: the secret scan held back flagged files and synced the rest -# line: ARTIFACTS_SYNC: attention: status=held held=. The secret scan is holding back file(s); everything else still syncs. See which files and how to fix them: --status -# fix: edit the file so the scan no longer matches, or --skip-file (permanent; undo with --unskip-file ) +# held trigger: status=held: the secret scan or the JSON check held back flagged files and synced the rest +# line: ARTIFACTS_SYNC: attention: status=held held=. Sync is holding back file(s) that the secret scan flagged or that are not valid JSON; everything else still syncs. See which files and how to fix them: --status +# fix: edit the file so the scan no longer matches (or repair a file that is not valid JSON), or --skip-file (permanent; undo with --unskip-file ) # blocked trigger: status=blocked: a re-scan still flagged after the hold, so nothing was committed -# line: ARTIFACTS_SYNC: attention: status=blocked. The secret scan flagged a file it could not hold back on its own, so nothing is syncing. See the fix: --status +# line: ARTIFACTS_SYNC: attention: status=blocked. Sync committed nothing: a flagged file could not be held back on its own, or held files are part of an unfinished git merge. See the fix: --status # fix: edit or skip the files named by --status, or clear the batch with --drop-queue --yes # push_failed trigger: status=push_failed: a commit is saved locally but the push failed # line: ARTIFACTS_SYNC: attention: status=push_failed. Synced files are committed locally but the push failed; it retries at skill start. See the cause: --status @@ -144,7 +144,7 @@ remote_auth_hint() { # Status file schema (one JSON object; readers: --status, --attention, doctor): # status ok | idle | held | collision | blocked | push_failed | error # (collision: files were held only for case-colliding paths) -# held_count files the secret scan held back this drain (hits + dependents) +# held_count files the secret scan or the JSON check held back this drain (hits + dependents) # case_held queued files held because their path differs only by letter # case from another path in the repo or the batch # case_names the colliding path component names, comma-separated and @@ -953,6 +953,8 @@ for rule, path in hits: "rule": rule, "dependents": deps, "fixes": [ + f"{path} is not valid JSON (often git conflict markers from a merge); repair it and the next sync re-checks it" + ] if rule == "invalid-json" else [ f"edit {path} so the secret scan no longer matches ({rule}); the next sync re-scans it", f"{self_bin} --skip-file {path} (permanently excludes it{also} from future syncs; undo with {self_bin} --unskip-file {path})", ], @@ -982,6 +984,52 @@ for path, rows in ((unstage_out, unstage), (wait_out, waiting)): PYEOF } +# Append "invalid-json" to the hits file $2 for every staged *.json +# path in the NUL-separated names file $1 whose index blob does not parse +# (#3763): a git merge can leave conflict markers in a synced JSON file, and +# committing it spreads an unreadable file to every machine. A path the +# secret scan already flagged is held for that reason alone. One +# `git cat-file --batch` reads the index blobs; a staged deletion has none. +append_invalid_json_hits() { + python3 - "$1" "$2" "$1.json" <<'PYEOF' || return 1 +import sys +names_path, hits_path, out_path = sys.argv[1:4] +with open(names_path, "rb") as f: + names = [p for p in f.read().decode("utf-8", "surrogateescape").split("\0") if p.endswith(".json") and "\n" not in p] +with open(hits_path, encoding="utf-8", errors="surrogateescape") as f: + flagged = {l.rstrip("\r\n").split("\t", 1)[-1] for l in f if "\t" in l} +with open(out_path, "w", encoding="utf-8", errors="surrogateescape", newline="\n") as out: + for p in names: + if p not in flagged: + out.write(":" + p + "\n") +PYEOF + [ -s "$1.json" ] || return 0 + git -C "$GSTACK_HOME" cat-file --batch < "$1.json" > "$1.json.blobs" 2>/dev/null || return 1 + python3 - "$1.json" "$1.json.blobs" "$2" <<'PYEOF' +import json, sys +batch_path, blobs_path, hits_path = sys.argv[1:4] +with open(batch_path, encoding="utf-8", errors="surrogateescape") as f: + names = [l[1:].rstrip("\n") for l in f if l.startswith(":")] +with open(blobs_path, "rb") as f: + data = f.read() +pos = 0 +with open(hits_path, "a", encoding="utf-8", errors="surrogateescape", newline="\n") as out: + for p in names: + nl = data.index(b"\n", pos) + header = data[pos:nl].split(b" ") + pos = nl + 1 + if header[-1] == b"missing": + continue + size = int(header[2]) + blob = data[pos:pos + size] + pos += size + 1 + try: + json.loads(blob.decode("utf-8")) + except ValueError: + out.write("invalid-json\t" + p + "\n") +PYEOF +} + # Fail closed: unstage everything, keep the queue, status blocked. block_drain() { git -C "$GSTACK_HOME" reset -q HEAD -- . >/dev/null 2>&1 || true @@ -994,7 +1042,9 @@ block_drain() { # one scanner pass attributes findings to staged paths, plan_holds decides # what is held or waits, those paths are unstaged and leave $1 (their records # stay queued), then the whole remaining staged diff must re-scan clean. -# Sets ST_HELD and WAITING. Returns 1 after falling back to block_drain. +# Sets ST_HELD and WAITING. Returns 1 after falling back to block_drain, or +# when a path would be held during an unfinished merge (the merge commit +# would silently keep only this side of it). hold_flagged_paths() { local paths_file="$1" groups_file="$2" scan="$1.scan" rc=0 gate_rc=0 gate_out WAITING=0 @@ -1003,6 +1053,7 @@ hold_flagged_paths() { git -C "$GSTACK_HOME" diff --cached --no-renames > "$scan.diff" 2>/dev/null || rc=1 git -C "$GSTACK_HOME" diff --cached --no-renames -z --name-only > "$scan.names" 2>/dev/null || rc=1 [ "$rc" -eq 0 ] && { secret_scan attribute "$scan.diff" "$scan.names" "$scan.hits" || rc=1; } + [ "$rc" -eq 0 ] && { append_invalid_json_hits "$scan.names" "$scan.hits" || rc=1; } # A successful add need not stage the requested spelling or current bytes # (case-insensitive indexes and assume-unchanged can both report success). # Reuse the scanner's name snapshot; refusal preserves the index and queue. @@ -1023,6 +1074,12 @@ hold_flagged_paths() { ST_HELD=$(cat "$scan.held") WAITING=$(grep -c . "$scan.wait" 2>/dev/null || true) WAITING=${WAITING:-0} + if [ -s "$scan.unstage" ] && git -C "$GSTACK_HOME" rev-parse -q --verify MERGE_HEAD >/dev/null 2>&1; then + ST_DRAINABLE=0 + write_status "blocked" "a git merge in $GSTACK_HOME is unfinished and $(grep -c . "$scan.unstage") file(s) are held back or wait for their coupled group; committing the merge without them would drop the other side's changes. Repair the files held[] names (they stay queued and sync on the next run), or finish the merge yourself; --skip-file and --drop-queue do not clear this. Nothing was committed; the merge and the queue are kept" + echo "BRAIN_SYNC: blocked: unfinished merge with held files" >&2 + return 1 + fi if [ -s "$scan.unstage" ]; then if ! tr -d '\r' < "$scan.unstage" | git --literal-pathspecs -C "$GSTACK_HOME" reset -q HEAD --pathspec-from-file=- >/dev/null 2>&1; then block_drain "git could not unstage the held files" @@ -1347,11 +1404,13 @@ subcmd_once() { hold_flagged_paths "$paths_file" "$groups_file" || exit 0 ST_DRAINABLE=$(( ADD_FAILED + WAITING )) [ "$WAITING" -gt 0 ] && ST_NOTE="$ST_NOTE; $WAITING file(s) of a coupled group wait for a consistent generation (still queued)" - local held_n ok_code="ok" + local held_n json_n ok_code="ok" held_n=$(printf '%s' "$ST_HELD" | grep -o '"rule"' | wc -l | tr -d ' ') + json_n=$(printf '%s' "$ST_HELD" | grep -o '"rule": "invalid-json"' | wc -l | tr -d ' ') if [ "$held_n" -gt 0 ]; then ok_code="held" - ST_NOTE="$ST_NOTE; the secret scan held back $held_n flagged file(s) (and any coupled files), NOT synced; see held[] for each file's fixes" + [ "$held_n" -gt "$json_n" ] && ST_NOTE="$ST_NOTE; the secret scan held back $(( held_n - json_n )) flagged file(s) (and any coupled files), NOT synced; see held[] for each file's fixes" + [ "$json_n" -gt 0 ] && ST_NOTE="$ST_NOTE; $json_n synced file(s) are not valid JSON (often git conflict markers) and were held back (and any coupled files), NOT synced and still queued; see held[] for each file's fixes" echo "BRAIN_SYNC: held $held_n flagged file(s); see $SELF_BIN --status" >&2 elif [ "$ST_CASE_HELD" -gt 0 ]; then ok_code="collision" @@ -1552,8 +1611,8 @@ subcmd_attention() { case "$last_push" in $iso) ;; *) last_push="" ;; esac case "$code" in - held) echo "ARTIFACTS_SYNC: attention: status=held held=$held. The secret scan is holding back $held file(s); everything else still syncs. See which files and how to fix them: $bin --status" ;; - blocked) echo "ARTIFACTS_SYNC: attention: status=blocked. The secret scan flagged a file it could not hold back on its own, so nothing is syncing. See the fix: $bin --status" ;; + held) echo "ARTIFACTS_SYNC: attention: status=held held=$held. Sync is holding back $held file(s) that the secret scan flagged or that are not valid JSON; everything else still syncs. See which files and how to fix them: $bin --status" ;; + blocked) echo "ARTIFACTS_SYNC: attention: status=blocked. Sync committed nothing: a flagged file could not be held back on its own, or held files are part of an unfinished git merge. See the fix: $bin --status" ;; push_failed) echo "ARTIFACTS_SYNC: attention: status=push_failed. Synced files are committed locally but the push failed; it retries at skill start. See the cause: $bin --status" ;; error) echo "ARTIFACTS_SYNC: attention: status=error. The last sync stopped before committing; the queue is kept and retried. See the cause: $bin --status" ;; unknown) echo "ARTIFACTS_SYNC: attention: status=unknown. The sync status file is unreadable. See it: $bin --status" ;; diff --git a/bin/gstack-developer-profile b/bin/gstack-developer-profile index b5db05262..f64178e2b 100755 --- a/bin/gstack-developer-profile +++ b/bin/gstack-developer-profile @@ -394,6 +394,18 @@ do_read() { # ----------------------------------------------------------------------- do_profile() { ensure_profile + local PARSE_ERR + if ! PARSE_ERR=$(PROFILE_FILE_PATH="$PROFILE_FILE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e " + const text = require('fs').readFileSync(process.env.PROFILE_FILE_PATH, 'utf-8'); + try { JSON.parse(text); } catch (e) { + const markers = /^(<{7}|={7}|>{7})( |\$)/m.test(text) ? ' (it holds git conflict markers from a merge)' : ''; + console.log(e.message + markers); + process.exit(1); + } + " 2>&1); then + echo "gstack-developer-profile: $PROFILE_FILE is not valid JSON: $PARSE_ERR; repair the file or restore it from git -C $GSTACK_HOME log -- developer-profile.json" >&2 + return 1 + fi cat "$PROFILE_FILE" } diff --git a/bin/gstack-doctor b/bin/gstack-doctor index 0690fb3a9..18adb49fc 100755 --- a/bin/gstack-doctor +++ b/bin/gstack-doctor @@ -275,7 +275,7 @@ check_codex_probe() { } check_sync() { - local mode file json status held drainable drain push queue=0 now f detail case_names fix="$BIN/gstack-brain-sync --status" + local mode file json json_held status held drainable drain push queue=0 now f detail case_names fix="$BIN/gstack-brain-sync --status" cfg artifacts_sync_mode off; mode=$_cfg if [ ! -d "$STATE_ROOT/.git" ]; then row "not configured" "artifacts sync" "off; cross-machine artifacts sync is unavailable" "$BIN/gstack-artifacts-init" @@ -285,6 +285,11 @@ check_sync() { row "not configured" "artifacts sync" "artifacts_sync_mode off; cross-machine artifacts sync is unavailable" "$BIN/gstack-config set artifacts_sync_mode artifacts-only" return fi + for f in "$STATE_ROOT/developer-profile.json" "$STATE_ROOT"/projects/*/decisions.active.json; do + [ -f "$f" ] && grep -qE '^(<{7}|>{7})( |$)' "$f" 2>/dev/null || continue + row warn "artifacts sync" "${f#"$STATE_ROOT"/} holds git conflict markers and does not parse" "repair the file, or restore it from git -C $STATE_ROOT log -- ${f#"$STATE_ROOT"/}" + return + done for f in "$STATE_ROOT/.brain-queue.d"/*.json; do [ -f "$f" ] && queue=$((queue + 1)); done for f in "$STATE_ROOT/.brain-queue.jsonl" "$STATE_ROOT/.brain-queue.jsonl.migrating"; do [ -f "$f" ] && queue=$((queue + $(wc -l < "$f" | tr -d ' '))) @@ -306,8 +311,15 @@ check_sync() { [ -n "$push" ] || push=$(epoch_of "$(head -1 "$STATE_ROOT/.brain-last-push" 2>/dev/null)") now=$(date +%s) case "$status" in - held) row warn "artifacts sync" "status held${held:+ ($held file(s) held by the secret scanner)}" "$fix"; return ;; - blocked) row warn "artifacts sync" "status blocked (a re-scan still flags staged content)" "$fix"; return ;; + held) + json_held=$(printf '%s' "$json" | grep -o '"rule": *"invalid-json"' | wc -l | tr -d ' ') + if [ "${json_held:-0}" -gt 0 ]; then + row warn "artifacts sync" "status held ($json_held synced file(s) are not valid JSON${held:+; $held held in all})" "$fix" + else + row warn "artifacts sync" "status held${held:+ ($held file(s) held by the secret scanner)}" "$fix" + fi + return ;; + blocked) row warn "artifacts sync" "status blocked (nothing was committed; --status names the cause)" "$fix"; return ;; push_failed|error|unknown) row warn "artifacts sync" "status $status" "$fix"; return ;; esac if [ -n "$case_names" ]; then diff --git a/docs/gbrain-sync-errors.md b/docs/gbrain-sync-errors.md index cd81aa9dd..2d14d7f07 100644 --- a/docs/gbrain-sync-errors.md +++ b/docs/gbrain-sync-errors.md @@ -38,8 +38,8 @@ cause, the held files and the fix. | State | Trigger | Attention line (after `ARTIFACTS_SYNC: attention: `) | Fix | |---|---|---|---| -| `held` | The secret scan held back flagged files and synced the rest | `status=held held=. The secret scan is holding back file(s); everything else still syncs. See which files and how to fix them: --status` | Edit the file so the scan no longer matches, or ` --skip-file ` (permanent; undo with `--unskip-file `) | -| `blocked` | A re-scan still flagged after the hold, so nothing was committed | `status=blocked. The secret scan flagged a file it could not hold back on its own, so nothing is syncing. See the fix: --status` | Edit or skip the files `--status` names, or clear the batch with ` --drop-queue --yes` | +| `held` | The secret scan or the JSON check held back flagged files and synced the rest | `status=held held=. Sync is holding back file(s) that the secret scan flagged or that are not valid JSON; everything else still syncs. See which files and how to fix them: --status` | Edit the file so the scan no longer matches, or ` --skip-file ` (permanent; undo with `--unskip-file `), or repair a file that is not valid JSON | +| `blocked` | A re-scan still flagged after the hold, or a file would be held during an unfinished git merge, so nothing was committed | `status=blocked. Sync committed nothing: a flagged file could not be held back on its own, or held files are part of an unfinished git merge. See the fix: --status` | Edit or skip the files `--status` names, or clear the batch with ` --drop-queue --yes`. For an unfinished merge, repair the held files or finish the merge; skipping files or dropping the queue does not clear it | | `push_failed` | A commit is saved locally but the push failed | `status=push_failed. Synced files are committed locally but the push failed; it retries at skill start. See the cause: --status` | The fix `--status` names (usually `gh auth status`) | | `error` | The drain stopped before committing and kept the queue | `status=error. The last sync stopped before committing; the queue is kept and retried. See the cause: --status` | The cause `--status` names: a git index lock, a commit hook, a `git add` failure | | `unknown` | The status file holds a status code outside this table | `status=unknown. The sync status file is unreadable. See it: --status` | Run ` --once` to rewrite it | @@ -76,6 +76,11 @@ both fixes: ~/.claude/skills/gstack/bin/gstack-brain-sync --drop-queue --yes ``` +A synced `.json` file that does not parse (often git conflict markers from a +merge) is held the same way under rule `invalid-json`. Its only fix is to +repair the file; `--skip-file` is not offered, because it would stop that +file syncing for good. The next sync re-checks it. + **Git index lock.** A git process killed mid-write leaves `~/.gstack/.git/index.lock`. The sync removes it once it is older than 10 minutes, because every gstack writer to `~/.gstack` takes the same sync lock diff --git a/docs/gbrain-sync.md b/docs/gbrain-sync.md index 43d7f2e00..756e8186f 100644 --- a/docs/gbrain-sync.md +++ b/docs/gbrain-sync.md @@ -91,8 +91,8 @@ gstack-brain-sync --status ``` Shows: the status (`ok`, `idle`, `held`, `collision`, `blocked`, -`push_failed`, `error`), files held back by the secret scan with their -fixes, files held because their path differs only by letter case from +`push_failed`, `error`), files held back by the secret scan or because +they are not valid JSON, with their fixes, files held because their path differs only by letter case from another path (`case_held`, `case_names`; see [troubleshooting](troubleshooting.md#artifact-case-collision)), the `drainable` count, the last sync and push times, pending queue depth, and @@ -156,7 +156,7 @@ to it); everything else syncs. The held file stays queued and is re-scanned at every sync, and skill start prints: ``` -ARTIFACTS_SYNC: attention: status=held held=1. The secret scan is holding back 1 file(s); everything else still syncs. See which files and how to fix them: --status +ARTIFACTS_SYNC: attention: status=held held=1. Sync is holding back 1 file(s) that the secret scan flagged or that are not valid JSON; everything else still syncs. See which files and how to fix them: --status ``` To remediate: @@ -200,6 +200,12 @@ is kept in a git stash; `gstack-brain-sync --status` names the stash and the `git -C ~/.gstack --literal-pathspecs restore --source= -- ` command that brings it back. +JSON files such as `developer-profile.json` have no merge driver, so a +merge can leave conflict markers in them. The sync parses every staged +`.json` file and holds back one that does not parse (status `held`, rule +`invalid-json`) until you repair it, and `gstack-doctor` warns when a synced profile or +decision snapshot holds conflict markers. + ## Cross-machine pull cadence The preamble runs `git fetch` + `git merge --ff-only` once per 24 hours diff --git a/package.json b/package.json index 9b0d4049c..95e77f142 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.45", + "version": "1.92.46", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/brain-sync.test.ts b/test/brain-sync.test.ts index ff7ee1432..0e5d60fde 100644 --- a/test/brain-sync.test.ts +++ b/test/brain-sync.test.ts @@ -843,7 +843,7 @@ describe('gstack-brain-sync per-file hold', () => { } function setup() { init(); - write(SHARED, '# shared v1\n'); + write(SHARED, '{"note":"shared v1"}\n'); write(OTHER, '# other v1\n'); expect(drain().status).toBe(0); expect(statusJson().status).toBe('ok'); @@ -856,15 +856,15 @@ describe('gstack-brain-sync per-file hold', () => { test('a dirty tracked file the remote did not touch rides through the merge', () => { setup(); - peerCommit(SHARED, '# shared from the peer\n'); + peerCommit(SHARED, '{"note":"shared from the peer"}\n'); write(OTHER, '# other, local edit not queued\n', false); write(CLEAN, '# a plan\n'); expect(drain().status).toBe(0); expect(statusJson().status).toBe('ok'); expect(remoteFiles()).toContain(CLEAN); - expect(remoteShow(SHARED).stdout).toBe('# shared from the peer\n'); + expect(remoteShow(SHARED).stdout).toBe('{"note":"shared from the peer"}\n'); expect(fs.readFileSync(path.join(tmpHome, OTHER), 'utf-8')).toBe('# other, local edit not queued\n'); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared from the peer\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared from the peer"}\n'); }); /** The restore command the status names, run as printed. */ @@ -878,17 +878,17 @@ describe('gstack-brain-sync per-file hold', () => { test('a dirty tracked file the remote also changed no longer leaves the push failed', () => { setup(); - peerCommit(SHARED, '# shared from the peer\n'); - write(SHARED, '# shared, local edit not queued\n', false); + peerCommit(SHARED, '{"note":"shared from the peer"}\n'); + write(SHARED, '{"note":"shared, local edit not queued"}\n', false); write(CLEAN, '# a plan\n'); expect(drain().status).toBe(0); const s = statusJson(); expect(s.status).toBe('ok'); expect(remoteFiles()).toContain(CLEAN); - expect(remoteShow(SHARED).stdout).toBe('# shared from the peer\n'); + expect(remoteShow(SHARED).stdout).toBe('{"note":"shared from the peer"}\n'); // The merged version stays in the working tree: no conflict markers, // nothing staged, and nothing stale for a later sync to publish. - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared from the peer\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared from the peer"}\n'); expect(unmerged()).toBe(''); expect(staged()).toBe(''); expect(s.message).toContain(`local edits to ${SHARED} conflicted`); @@ -900,23 +900,23 @@ describe('gstack-brain-sync per-file hold', () => { write(OTHER, '# other v2\n'); expect(drain().status).toBe(0); expect(statusJson().status).toBe('ok'); - expect(remoteShow(SHARED).stdout).toBe('# shared from the peer\n'); - expect(git(['show', `${sha}:${SHARED}`]).stdout).toBe('# shared, local edit not queued\n'); + expect(remoteShow(SHARED).stdout).toBe('{"note":"shared from the peer"}\n'); + expect(git(['show', `${sha}:${SHARED}`]).stdout).toBe('{"note":"shared, local edit not queued"}\n'); // The printed command brings the local edit back. expect(recover(s.message)).toBe(sha!); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared, local edit not queued\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared, local edit not queued"}\n'); }); test('a local deletion of a file the remote changed keeps the remote file and stashes the deletion', () => { setup(); - peerCommit(SHARED, '# shared from the peer\n'); + peerCommit(SHARED, '{"note":"shared from the peer"}\n'); fs.rmSync(path.join(tmpHome, SHARED)); write(CLEAN, '# a plan\n'); expect(drain().status).toBe(0); const s = statusJson(); expect(s.status).toBe('ok'); - expect(remoteShow(SHARED).stdout).toBe('# shared from the peer\n'); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared from the peer\n'); + expect(remoteShow(SHARED).stdout).toBe('{"note":"shared from the peer"}\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared from the peer"}\n'); expect(unmerged()).toBe(''); expect(s.message).toContain(`local edits to ${SHARED} conflicted`); recover(s.message); @@ -934,7 +934,7 @@ describe('gstack-brain-sync per-file hold', () => { } finally { fs.rmSync(peer, { recursive: true, force: true }); } - write(SHARED, '# shared, local edit not queued\n', false); + write(SHARED, '{"note":"shared, local edit not queued"}\n', false); write(CLEAN, '# a plan\n'); expect(drain().status).toBe(0); const s = statusJson(); @@ -943,7 +943,7 @@ describe('gstack-brain-sync per-file hold', () => { expect(fs.existsSync(path.join(tmpHome, SHARED))).toBe(false); expect(unmerged()).toBe(''); recover(s.message); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared, local edit not queued\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared, local edit not queued"}\n'); }); test('a conflicting file whose name looks like a pattern resets only itself', () => { @@ -954,16 +954,16 @@ describe('gstack-brain-sync per-file hold', () => { expect(git(['merge', '-q', '--ff-only', 'origin/main']).status).toBe(0); peerCommit(GLOB, '# glob from the peer\n'); write(GLOB, '# glob, local edit not queued\n', false); - write(SHARED, '# shared, local edit not queued\n', false); + write(SHARED, '{"note":"shared, local edit not queued"}\n', false); write(CLEAN, '# a plan\n'); expect(drain().status).toBe(0); const s = statusJson(); expect(s.status).toBe('ok'); expect(fs.readFileSync(path.join(tmpHome, GLOB), 'utf-8')).toBe('# glob from the peer\n'); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared, local edit not queued\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared, local edit not queued"}\n'); recover(s.message); expect(fs.readFileSync(path.join(tmpHome, GLOB), 'utf-8')).toBe('# glob, local edit not queued\n'); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared, local edit not queued\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared, local edit not queued"}\n'); }); test('a file that never checks out clean does not strand the other local edits', () => { @@ -973,7 +973,7 @@ describe('gstack-brain-sync per-file hold', () => { expect(drain().status).toBe(0); fs.mkdirSync(path.join(tmpHome, '.git', 'info'), { recursive: true }); fs.writeFileSync(path.join(tmpHome, '.git', 'info', 'attributes'), '*.md text eol=lf\n'); - peerCommit(SHARED, '# shared from the peer\n'); + peerCommit(SHARED, '{"note":"shared from the peer"}\n'); write(OTHER, '# other, local edit not queued\n', false); write(CLEAN, '# a plan\n'); expect(drain().status).toBe(0); @@ -987,28 +987,28 @@ describe('gstack-brain-sync per-file hold', () => { setup(); write(OTHER, '# other, older stashed work\n', false); expect(git(['stash', 'push', '-q', '-m', 'older work']).status).toBe(0); - peerCommit(SHARED, '# shared from the peer\n'); - write(SHARED, '# shared, local edit not queued\n', false); + peerCommit(SHARED, '{"note":"shared from the peer"}\n'); + write(SHARED, '{"note":"shared, local edit not queued"}\n', false); write(CLEAN, '# a plan\n'); const shim = gitShim('case " $* " in *" stash push "*) exit 0;; esac'); expect(drain(shim.env).status).toBe(0); expect(statusJson().status).toBe('push_failed'); expect(git(['stash', 'list']).stdout).toContain('older work'); expect(fs.readFileSync(path.join(tmpHome, OTHER), 'utf-8')).toBe('# other v1\n'); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared, local edit not queued\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared, local edit not queued"}\n'); }); test('a conflicting merge is aborted and git\'s reason reaches the status', () => { setup(); - peerCommit(SHARED, '# shared from the peer\n'); - write(SHARED, '# shared, local commit\n'); + peerCommit(SHARED, '{"note":"shared from the peer"}\n'); + write(SHARED, '{"note":"shared, local commit"}\n'); expect(drain().status).toBe(0); const s = statusJson(); expect(s.status).toBe('push_failed'); expect(s.message).toContain(`Merge conflict in ${SHARED}`); expect(mergeHead()).toBe(false); expect(unmerged()).toBe(''); - expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('# shared, local commit\n'); + expect(fs.readFileSync(path.join(tmpHome, SHARED), 'utf-8')).toBe('{"note":"shared, local commit"}\n'); // The next drain is not blocked by a half-finished merge. write(OTHER, '# other v2\n'); expect(drain().status).toBe(0); @@ -1270,6 +1270,130 @@ describe('gstack-brain-sync per-file hold', () => { expect(remoteShow(SNAP).stdout).toContain('"ccc"'); }); }); + + describe('a synced JSON file that does not parse is never committed (#3763)', () => { + const PROFILE = 'developer-profile.json'; + const MARKED = '{\n "sessions": [],\n<<<<<<< HEAD\n "v": "local"\n=======\n "v": "peer"\n>>>>>>> origin/main\n}\n'; + const parses = (text: string) => { try { JSON.parse(text); return true; } catch { return false; } }; + + test('a queued profile holding conflict markers is held back; the rest syncs; a repaired profile syncs next', () => { + init(); + write(PROFILE, MARKED); + write(CLEAN, '# a plan\n'); + expect(drain().status).toBe(0); + expect(remoteFiles()).toContain(CLEAN); + expect(remoteFiles()).not.toContain(PROFILE); + expect(staged()).toBe(''); + expect(spoolText()).toContain(PROFILE); + const s = statusJson(); + expect(s.status).toBe('held'); + expect(s.held.map((h: { path: string; rule: string }) => [h.path, h.rule])).toEqual([[PROFILE, 'invalid-json']]); + expect(s.held[0].fixes[0]).toContain(`${PROFILE} is not valid JSON`); + expect(s.held[0].fixes).toHaveLength(1); + expect(s.message).toContain('not valid JSON'); + expect(s.message).not.toContain('secret scan'); + + write(PROFILE, '{"sessions":[],"v":"repaired"}\n', false); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + expect(remoteShow(PROFILE).stdout).toBe('{"sessions":[],"v":"repaired"}\n'); + }); + + test('a marker-free syntax error is held too', () => { + init(); + write(PROFILE, '{"sessions": [],}\n'); + expect(drain().status).toBe(0); + expect(remoteFiles()).not.toContain(PROFILE); + expect(statusJson().held.map((h: { rule: string }) => h.rule)).toEqual(['invalid-json']); + expect(spoolText()).toContain(PROFILE); + }); + + test('an unparseable decisions.active.json holds its coupled decisions.jsonl until repaired', () => { + init(); + writeDecisions(['a'], ['a']); + expect(drain().status).toBe(0); + write(LOG, [decide('a'), decide('bb')].join('\n') + '\n', false); + write(SNAP, '[{"id":"a"},\n<<<<<<< HEAD\n{"id":"bb"}\n=======\n{"id":"c"}\n>>>>>>> origin/main\n]\n', false); + run(['gstack-brain-enqueue', LOG]); + expect(drain().status).toBe(0); + const s = statusJson(); + expect(s.status).toBe('held'); + expect(s.held.map((h: { path: string; rule: string; dependents: string[] }) => [h.path, h.rule, h.dependents])).toEqual([[SNAP, 'invalid-json', [LOG]]]); + expect(remoteShow(LOG).stdout).toBe(decide('a') + '\n'); + expect(spoolText()).toContain(LOG); + + writeDecisions(['a', 'bb'], ['a', 'bb']); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + expect(remoteShow(LOG).stdout).toBe([decide('a'), decide('bb')].join('\n') + '\n'); + }); + + test('two machines editing the profile never put conflict markers on the remote', () => { + init(); + write(PROFILE, '{"sessions":[],"v":"base"}\n'); + expect(drain().status).toBe(0); + // The retry merge commits with the ambient identity; a CI runner has none. + expect(git(['config', 'user.name', 'fixture']).status).toBe(0); + expect(git(['config', 'user.email', 'fixture@example.com']).status).toBe(0); + const peer = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-sync-peer-')); + try { + expect(git(['clone', '-q', bareRemote, peer]).status).toBe(0); + fs.writeFileSync(path.join(peer, PROFILE), '{"sessions":[],"v":"peer"}\n'); + expect(git(['add', '-f', '--', PROFILE], peer).status).toBe(0); + expect(git(['-c', 'user.name=peer', '-c', 'user.email=peer@example.com', 'commit', '-qm', 'peer profile'], peer).status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD'], peer).status).toBe(0); + } finally { + fs.rmSync(peer, { recursive: true, force: true }); + } + write(PROFILE, '{"sessions":[],"v":"local"}\n'); + expect(drain().status).toBe(0); + // Every later profile write queues the file again. + run(['gstack-brain-enqueue', PROFILE]); + write(CLEAN, '# a plan\n'); + expect(drain().status).toBe(0); + expect(parses(remoteShow(PROFILE).stdout)).toBe(true); + expect(remoteShow(PROFILE).stdout).not.toContain('<<<<<<<'); + expect(remoteShow(PROFILE).stdout).toBe('{"sessions":[],"v":"peer"}\n'); + }); + + test('an unfinished merge with a held profile commits nothing until the profile is repaired', () => { + init(); + write(PROFILE, '{"sessions":[],"v":"base"}\n'); + expect(drain().status).toBe(0); + expect(git(['config', 'user.name', 'fixture']).status).toBe(0); + expect(git(['config', 'user.email', 'fixture@example.com']).status).toBe(0); + const peer = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-sync-peer-')); + try { + expect(git(['clone', '-q', bareRemote, peer]).status).toBe(0); + fs.writeFileSync(path.join(peer, PROFILE), '{"sessions":[],"v":"peer"}\n'); + expect(git(['add', '-f', '--', PROFILE], peer).status).toBe(0); + expect(git(['-c', 'user.name=peer', '-c', 'user.email=peer@example.com', 'commit', '-qm', 'peer profile'], peer).status).toBe(0); + expect(git(['push', '-q', 'origin', 'HEAD'], peer).status).toBe(0); + } finally { + fs.rmSync(peer, { recursive: true, force: true }); + } + fs.writeFileSync(path.join(tmpHome, PROFILE), '{"sessions":[],"v":"local"}\n'); + expect(git(['commit', '-qam', 'local profile']).status).toBe(0); + expect(git(['fetch', '-q', 'origin']).status).toBe(0); + const branch = git(['rev-parse', '--abbrev-ref', 'HEAD']).stdout.trim(); + expect(git(['merge', '--no-edit', `origin/${branch}`]).status).not.toBe(0); + run(['gstack-brain-enqueue', PROFILE]); + write(CLEAN, '# a plan\n'); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('blocked'); + expect(statusJson().message).toContain('unfinished'); + expect(statusJson().message).not.toContain('--skip-file '); + expect(remoteShow(PROFILE).stdout).toBe('{"sessions":[],"v":"peer"}\n'); + expect(remoteFiles()).not.toContain(CLEAN); + expect(git(['rev-parse', '-q', '--verify', 'MERGE_HEAD']).status).toBe(0); + + write(PROFILE, '{"sessions":[],"v":"merged"}\n', false); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('ok'); + expect(remoteShow(PROFILE).stdout).toBe('{"sessions":[],"v":"merged"}\n'); + expect(remoteFiles()).toContain(CLEAN); + }); + }); }); // --------------------------------------------------------------- diff --git a/test/gstack-developer-profile.test.ts b/test/gstack-developer-profile.test.ts index 11685bb64..46a945508 100644 --- a/test/gstack-developer-profile.test.ts +++ b/test/gstack-developer-profile.test.ts @@ -628,6 +628,24 @@ describe('gstack-developer-profile --check-mismatch', () => { // Error handling // ----------------------------------------------------------------------- +describe('gstack-developer-profile --profile on an unparseable profile (#3763)', () => { + test('exits non-zero, prints nothing to stdout, and names the file and the conflict markers', () => { + fs.writeFileSync(path.join(tmpHome, 'developer-profile.json'), '{\n<<<<<<< HEAD\n "sessions": []\n=======\n "sessions": [1]\n>>>>>>> origin/main\n}\n'); + const r = runDev('--profile'); + expect(r.status).not.toBe(0); + expect(r.stdout).toBe(''); + expect(r.stderr).toContain('developer-profile.json is not valid JSON'); + expect(r.stderr).toContain('git conflict markers'); + }); + + test('a valid profile still prints as before', () => { + fs.writeFileSync(path.join(tmpHome, 'developer-profile.json'), '{"sessions":[]}\n'); + const r = runDev('--profile'); + expect(r.status).toBe(0); + expect(r.stdout).toBe('{"sessions":[]}\n'); + }); +}); + describe('gstack-developer-profile errors', () => { test('unknown subcommand exits non-zero', () => { const r = runDev('--not-a-real-subcommand'); diff --git a/test/gstack-doctor.test.ts b/test/gstack-doctor.test.ts index 61a680b40..367034c42 100644 --- a/test/gstack-doctor.test.ts +++ b/test/gstack-doctor.test.ts @@ -136,6 +136,25 @@ describe('gstack-doctor', () => { expect(r.out).not.toContain('.brain-sync-status.json'); }); + test('artifacts sync: a synced JSON file with conflict markers warns by name (#3763)', () => { + const f = makeFixture(); + syncOn(f, { status: 'ok', message: 'pushed 3 file(s)', drainable: 0, last_drain_at: iso(60), last_push_at: iso(60) }); + write(path.join(f.state, 'developer-profile.json'), '{\n<<<<<<< HEAD\n "v": 1\n=======\n "v": 2\n>>>>>>> origin/main\n}\n'); + const r = doctor(f); + expect(r.row('artifacts sync').state).toBe('warn'); + expect(r.row('artifacts sync').detail).toBe('developer-profile.json holds git conflict markers and does not parse'); + write(path.join(f.state, 'developer-profile.json'), '{"v": 2}\n'); + expect(doctor(f).row('artifacts sync').state).toBe('ok'); + }); + + test('artifacts sync: a JSON hold is not attributed to the secret scanner (#3763)', () => { + const f = makeFixture(); + syncOn(f, { status: 'held', message: 'held', held_count: 1, held: [{ path: 'developer-profile.json', rule: 'invalid-json', dependents: [], fixes: ['repair', 'skip'] }], drainable: 0, last_drain_at: iso(60), last_push_at: iso(60) }); + const r = doctor(f); + expect(r.row('artifacts sync').state).toBe('warn'); + expect(r.row('artifacts sync').detail).toBe('status held (1 synced file(s) are not valid JSON; 1 held in all)'); + }); + test('artifacts sync: held, then ok', () => { const f = makeFixture(); syncOn(f, { status: 'held', message: 'held 2 file(s): sk-ant-secret', held_count: 2, held: [{ path: 'projects/p/notes.md', rule: 'anthropic_key', dependents: [], fixes: ['edit', 'skip'] }, { path: 'projects/p/b.md', rule: 'anthropic_key', dependents: [], fixes: ['edit', 'skip'] }], drainable: 0, last_drain_at: iso(60), last_push_at: iso(60) }); diff --git a/test/gstack-skill-start.test.ts b/test/gstack-skill-start.test.ts index 5dca2124f..1fd895132 100644 --- a/test/gstack-skill-start.test.ts +++ b/test/gstack-skill-start.test.ts @@ -816,7 +816,7 @@ describe('ARTIFACTS_SYNC attention lines (sync status table)', () => { const held = runStart([], env); expect(held).toMatch(/^ARTIFACTS_SYNC: mode=full \| last_push=\S+ \| queue=\d+$/m); expect(attention(held)).toEqual([ - `ARTIFACTS_SYNC: attention: status=held held=1. The secret scan is holding back 1 file(s); everything else still syncs. See which files and how to fix them: ${path.join(ROOT, 'bin', 'gstack-brain-sync')} --status`, + `ARTIFACTS_SYNC: attention: status=held held=1. Sync is holding back 1 file(s) that the secret scan flagged or that are not valid JSON; everything else still syncs. See which files and how to fix them: ${path.join(ROOT, 'bin', 'gstack-brain-sync')} --status`, ]); expect(held).not.toContain('ghp_'); expect(remoteFiles()).toContain('projects/p/ceo-plans/clean.md'); From 35a676ff68b88619308591727ace7ac733650841 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 1 Nov 2026 15:36:31 -0800 Subject: [PATCH 5/8] v1.92.47.0 fix(ship): /ship Step 5 runs test lanes one at a time on 4 cores or fewer, with --lanes serial|parallel (GST-851) (#3821) * fix(ship-preflight): run Step 5 lanes one at a time on 4 cores or fewer, with --lanes serial|parallel (GST-851) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: GST-851 fragment Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(ship): Step 5 says lanes run one at a time on 4 or fewer cores and names --lanes (GST-851 docs audit) 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> --- CHANGELOG.md | 7 ++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- lib/ship-bin-common.ts | 34 +++++++++++---- lib/ship-preflight.ts | 16 ++++--- package.json | 2 +- ship/sections/tests.md | 5 ++- ship/sections/tests.md.tmpl | 5 ++- test/ship-preflight.test.ts | 42 +++++++++++++++++++ .../ship/gst-851-serial-lanes.json | 4 ++ 10 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 test/size-ledger/ship/gst-851-serial-lanes.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 2619a01d1..dab48b0c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## [1.92.47.0] - 2026-11-01 + +- **`/ship` Step 5 runs its test lanes one at a time on machines with 4 or fewer cores, and `--lanes serial|parallel` picks the mode by hand** (`gstack-ship-preflight`, GST-851). Preflight used to start every `--lane` at once. On garrytan/gstack-evals#115 six lanes (a 1,451-test bun suite, tsc, a Docker tier and an arm tarball trial) started within 60 ms of each other on a 4-core, 16 GB machine, and running lanes together has taken machines down before (gstack-evals #101, #106, #107). Now, with no flag, preflight runs one lane at a time when `os.availableParallelism()` is 4 or less and all of them at once above that. `--lanes serial` or `--lanes parallel` overrides the default, and any other value, or `--lanes` with no value, is a usage error (exit 2). A new `LANES: serial|parallel (--lanes| cores)` line before the `LANE` lines says which mode ran and why. Each lane still prints its own `LANE` line with its own `tests_ran` count, in the order the lanes were given. Serial runs take the sum of the lane times instead of the longest one. There is no ship-policy key for this: a new key needs key versioning and a `min_gstack_version` bump, and the core-count default already covers the 4-core case. + +### Tests +- `test/ship-preflight.test.ts`: `--lanes serial` runs lanes one after another and keeps each lane's count; `--lanes parallel` keeps lane order when a later lane finishes first; an unflagged run names the core count; a bad or missing `--lanes` value is a usage error; and the core-count default is serial at 4 cores or fewer. + ## [1.92.46.0] - 2026-11-01 **brain-sync no longer publishes a synced JSON file that doesn't parse, and the profile and doctor now say when one is broken.** When two machines edit `developer-profile.json` before syncing, git's merge can leave conflict markers in it. brain-sync committed and pushed that file, so every machine got a profile that no longer parsed, while `gstack-developer-profile --profile` and `gstack-doctor` both reported everything ok (#3763). diff --git a/VERSION b/VERSION index cbd266e1d..f0f525626 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.46.0 +1.92.47.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 7d20d84f4..f4e17a48a 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.46.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.47.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/lib/ship-bin-common.ts b/lib/ship-bin-common.ts index 2702f251f..0bb6e6b05 100644 --- a/lib/ship-bin-common.ts +++ b/lib/ship-bin-common.ts @@ -7,6 +7,7 @@ */ import { spawn, spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { fragmentSlug } from './changelog-fragments'; import { parseUnifiedDiff, stampOnlyLine, versionAtRef } from './restamp'; @@ -150,13 +151,32 @@ export class Report { } } -/** Run each lane through gstack-evidence run in parallel and read its recorded line. */ -export async function runLanes(cwd: string, list: { label: string; command: string }[]): Promise<{ label: string; exit: number | null; testsRan: string; log: string }[]> { - return Promise.all(list.map(async l => { - const r = await binAsync(cwd, 'gstack-evidence', ['run', '--label', l.label, '--', l.command]); - const m = /gstack-evidence: recorded label=\S+ exit=(\S+) tests_ran=(\S+) log=(\S+)/.exec(r.stderr); - return { label: l.label, exit: r.status, testsRan: m?.[2] ?? 'unknown', log: m?.[3] ?? '-' }; - })); +/** + * How many lanes run at once (GST-851): `--lanes serial` one, `--lanes parallel` all; unset, one on a machine with 4 or fewer + * cores (six lanes at once took 4-core machines down on gstack-evals) and all above that. Any other value is a usage error. + */ +export function laneConcurrency(mode: string | undefined, cpus = os.availableParallelism()): number | string { + if (mode === 'serial') return 1; + if (mode === 'parallel') return Infinity; + if (mode !== undefined) return `--lanes ${mode} is not serial or parallel`; + return cpus <= 4 ? 1 : Infinity; +} + +/** Run each lane through gstack-evidence run, at most `concurrency` at a time, and read its recorded line; results keep the lanes' order. */ +export async function runLanes(cwd: string, list: { label: string; command: string }[], concurrency = Infinity): Promise<{ label: string; exit: number | null; testsRan: string; log: string }[]> { + const out: { label: string; exit: number | null; testsRan: string; log: string }[] = []; + let next = 0; + const worker = async () => { + while (next < list.length) { + const i = next++; + const l = list[i]!; + const r = await binAsync(cwd, 'gstack-evidence', ['run', '--label', l.label, '--', l.command]); + const m = /gstack-evidence: recorded label=\S+ exit=(\S+) tests_ran=(\S+) log=(\S+)/.exec(r.stderr); + out[i] = { label: l.label, exit: r.status, testsRan: m?.[2] ?? 'unknown', log: m?.[3] ?? '-' }; + } + }; + await Promise.all(Array.from({ length: Math.max(1, Math.min(concurrency, list.length)) }, worker)); + return out; } /** Step 5's lane verdict: a lane passes on its runner count, never exit 0 alone. */ diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index ed1f48f50..519ee5475 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -6,22 +6,24 @@ * agent (a merge conflict, a failing or zero-run lane, the carry route, the Apple target). */ import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { classifyDiff } from './review-depth'; import { audit } from './ship-audit'; -import { bin, firstLine, flagReader, gh, git, hostedOrigin, laneLine, lanes, Report, runLanes, shipPolicy, short } from './ship-bin-common'; +import { bin, firstLine, flagReader, gh, git, hostedOrigin, laneConcurrency, laneLine, lanes, Report, runLanes, shipPolicy, short } from './ship-bin-common'; export const PREFLIGHT_HELP = `gstack-ship-preflight — /ship Steps 1-3 and Step 5's pregate and lanes in one call (ship/SKILL.md) gstack-ship-preflight --base [--lane