v1.92.65.0 fix(review): every reviewer brief opens with the REVIEWED_TREE step (GST-897) (#3830)

* fix(review): open every outside-voice brief with the REVIEWED_TREE step (GST-897)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: register the review-wrong-tree fixtures as free (GST-897)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: GST-897 fragment

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

@@ -1,5 +1,12 @@
 # Changelog
 
+## [1.92.65.0] - 2026-11-02
+
+- **Every /ship reviewer brief now starts with the `REVIEWED_TREE` step instead of saving it for the last paragraph** (`lib/review-brief.ts`, GST-897). Before, the brief only asked for the fingerprint line in its closing paragraph, after the whole diff and a rule that the reply ends with its `Recommendation:` line. On GST-850, two gpt-6.1-sol outside-voice replies never ran the command, so `collect` refused both (`COLLECT_WRONG_TREE reported="none"`) and their findings were lost. Other readers misread "the line <command> prints" and pasted the command instead of its output. Now the brief opens with "First step, before anything else: run `cd <checkout> && echo "REVIEWED_TREE: …"`", says the command outputs one `REVIEWED_TREE: <tree hash> <path>` line, and asks the reader to copy that output line, not the command, as the first line of its reply. The closing paragraph repeats the request with "(the output, not the command)". `collect` is unchanged and still refuses a reply without the line, and the dispatcher never stamps the tree for a reader.
+
+### Tests
+- `test/review-dispatch.test.ts`: the rendered outside-voice brief's first line is the step, running its command prints a line `collect` accepts, and both captured GST-850 replies (`test/fixtures/review-wrong-tree/`) are still refused with `COLLECT_WRONG_TREE`. On main this test fails because the brief's first line is the filesystem boundary paragraph.
+
 ## [1.92.64.0] - 2026-11-02
 
 **/ship's Step 11 outside voice now defaults to the model it was tested with, and a same-family outside voice is refused.** On a lane running an Anthropic model, the pass program uses `openai/gpt-6.1-sol` at medium reasoning when no outside-voice model is given. That is the pairing no-loss rounds 3 to 6 and A3668 ran. Until now the lane picked the model (gpt-6-astra in replays 6c, 7e and 8g), and the only check was that some model was given, so a Claude outside voice beside a Claude native voice would have run as "outside" (GSTA-167 plan R2 and O2).

VERSION

@@ -1 +1 @@
-1.92.64.0
+1.92.65.0

agents-digest/gstack-AGENTS.md

@@ -1,4 +1,4 @@
-# gstack digest v1.92.64.0 — regenerate/re-copy after upgrading gstack
+# gstack digest v1.92.65.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,

docs/review-policy.md

@@ -580,12 +580,14 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
    fix-check plan renders its reader from `--fix-list`, `--hunks`, `--tests` and `--callers`,
    `--audit-only` runs just the `--extras` readers (Step 14.5's docs audit), and the fork
    point is found against `origin/<base>` first, as preflight does.
-7. Every brief, audit readers' included, ends with a workspace paragraph naming the
-   checkout the bins run in (GST-482): a reader on a shared machine starts in the
-   machine's working directory, so it is told to `cd` there on every command, to give
-   every file tool an absolute path under it, and to start its reply with
-   `REVIEWED_TREE: <HEAD^{tree}> <toplevel>` measured there. The paragraph follows the
-   section's render, which stays byte for byte. `collect` and `mark --state collected`
+7. Every brief, audit readers' included, opens with a fingerprint step and ends with a
+   workspace paragraph naming the checkout the bins run in (GST-482): a reader on a shared
+   machine starts in the machine's working directory, so it is told to `cd` there on every
+   command, to give every file tool an absolute path under it, and to start its reply with
+   the `REVIEWED_TREE: <HEAD^{tree}> <toplevel>` line the step's command outputs there. The
+   step comes first because a reader following a findings-only format skipped the request
+   when it only closed the brief (GST-897). The section's render sits between the two,
+   byte for byte. `collect` and `mark --state collected`
    compare that line with the fingerprint recorded at render: a missing or foreign line is
    `COLLECT_WRONG_TREE`, a later commit in the same checkout `COLLECT_TREE_MOVED`; nothing
    is merged, the reader becomes `failed`, and `verify` reports `NOT_COLLECTED`. An

lib/review-brief.ts

@@ -144,16 +144,27 @@ function renderRole(role: BriefRole, slots: Slots, t: SectionTexts): string {
   }
 }
 
+const quoted = (dir: string) => `'${dir.replace(/'/g, `'\\''`)}'`;
+const treeCommand = (dir: string) => `cd ${quoted(dir)} && echo "REVIEWED_TREE: $(git rev-parse 'HEAD^{tree}') $(git rev-parse --show-toplevel)"`;
+
+/**
+ * The opening step every dispatched brief carries (GST-897): the fingerprint command and what to copy from it. A reader that
+ * follows a findings-only output format can skip a request that only closes a long brief (gpt-6.1-sol did on GST-850), and
+ * "the line <command> prints" was read as the command itself, so the step comes first and names the output line.
+ */
+export function workspaceStep(dir: string): string {
+  return `First step, before anything else: run \`${treeCommand(dir)}\`. It outputs one line, \`REVIEWED_TREE: <tree hash> <path>\`. Copy that output line, not the command, as the first line of your final reply, above everything else, even where this brief limits your output to findings or says how to end it; a reply without it, or from another checkout, does not count.`;
+}
+
 /**
  * The closing paragraph every dispatched brief carries (GST-482): a reader on a shared machine starts in that machine's working
- * directory, and every command runs in a fresh shell, so the brief names the checkout under review and asks for a fingerprint
- * measured there. It is appended after the section's render, so the brief before it stays byte for byte what renderBrief returns.
+ * directory, and every command runs in a fresh shell, so the brief names the checkout under review and asks again for the
+ * fingerprint workspaceStep opens with. The section's render sits between the two, byte for byte what renderBrief returns.
  */
 export function workspaceParagraph(dir: string): string {
-  const q = `'${dir.replace(/'/g, `'\\''`)}'`;
-  return `Workspace: the checkout under review is ${dir}. Your shell and file tools may start in another checkout, and every command runs in a fresh shell: begin each command with \`cd ${q} && \`, and give every file read, search, edit or write an absolute path under ${dir}. Make the first line of your final reply the line \`cd ${q} && echo "REVIEWED_TREE: $(git rev-parse 'HEAD^{tree}') $(git rev-parse --show-toplevel)"\` prints, even where this brief limits your output to findings; a reply without it, or from another checkout, does not count.`;
+  return `Workspace: the checkout under review is ${dir}. Your shell and file tools may start in another checkout, and every command runs in a fresh shell: begin each command with \`cd ${quoted(dir)} && \`, and give every file read, search, edit or write an absolute path under ${dir}. The first line of your final reply is the \`REVIEWED_TREE: <tree hash> <path>\` line that \`${treeCommand(dir)}\` outputs (the output, not the command), even where this brief limits your output to findings; a reply without it, or from another checkout, does not count.`;
 }
-export const withWorkspace = (text: string, dir: string) => `${text}\n\n${workspaceParagraph(dir)}`;
+export const withWorkspace = (text: string, dir: string) => `${workspaceStep(dir)}\n\n${text}\n\n${workspaceParagraph(dir)}`;
 /** The checkout a brief names: the toplevel of the directory the bins run in, where its diff and merge base come from. */
 export function checkoutRoot(): string {
   const r = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 30_000 });

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gstack",
-  "version": "1.92.64",
+  "version": "1.92.65",
   "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
   "license": "MIT",
   "type": "module",

test/fixtures/review-wrong-tree/gpt-6.1-sol-gst850-p1.out

@@ -0,0 +1,11 @@
+1. **Escaped executable URLs bypass the locale checks** — `lib/review-allowlist.ts:143–145,188–192`. Changing a consumed locale JSON value from `"Continue"` to `"java\nscript:alert(1)"` returns `docs` with zero reviewers. Checks inspect the escaped source, but JSON decoding followed by URL parsing produces `javascript:`. A translated `href` can therefore gain script execution without security review. Validate decoded values, including URL normalization. Severity: High | Blocks: yes
+
+2. **SVG attribute interpolation is still executable template content** — `lib/review-allowlist.ts:35–44`. `<svg><text class="{{run()}}">Label</text></svg>` returns `docs` with zero reviewers. AngularJS `templateUrl`/`$compile` consumers evaluate that attribute. Numeric references also bypass the text check: `<text>&#123;&#123;run()&#125;&#125;</text>` is accepted. Reject interpolation in attributes and inspect decoded text, or reject numeric references. Severity: High | Blocks: yes
+
+3. **YAML type changes qualify as string-only edits** — `lib/review-allowlist.ts:163`. `label: Save` → `label: null` or `label: false` returns `docs` with zero reviewers. Native YAML parsing produces null/boolean, not strings; the null reproduction crashes a string consumer. Require string-valued scalars on both sides. Severity: Medium | Blocks: yes
+
+4. **Named gettext placeholders aren't preserved** — `lib/review-allowlist.ts:115,188–192`. `msgstr "Hello %(name)s"` → `msgstr "Hello %(password)s"` is accepted as docs because neither placeholder matches `PLACEHOLDER`. Formatting with the original arguments raises `KeyError('password')`, reproduced with Python. Recognize named percent placeholders or reject unfamiliar percent syntax. Severity: Medium | Blocks: yes
+
+5. **The new import scanner is quadratic** — `lib/review-risk.ts:72,107`. A valid comment containing repeated `import {` forces rescanning from every occurrence. At this HEAD, 128 KB took 1.04 seconds and 524 KB took 13.35 seconds for one `matchRisk` call; the planner calls it twice. Larger attacker-controlled comments can stall review and queue validation. Use a forward-only scanner. Severity: Medium | Blocks: yes
+
+Recommendation: request changes because escaped locale URLs can introduce executable javascript: links while scheduling zero reviewers.

test/fixtures/review-wrong-tree/gpt-6.1-sol-gst850-p2.out

@@ -0,0 +1,7 @@
+1. **Quadratic regex lets a diff stall review planning** — `lib/review-risk.ts:60`. The trailing `\s*,?\s*$` repeatedly repartitions whitespace when an `exec` binding ends with a comment. On HEAD `c1eea4c`, passing `"  exec" + " ".repeat(200000) + "// comment"` to `matchRisk()` took **15.47 seconds**, versus **0.69 ms** on the supplied base. At 50,000 and 100,000 spaces, HEAD took 0.95 and 3.94 seconds, confirming quadratic growth. This is valid inside a multiline import; larger diffs can monopolize the synchronous review classifier. Remove the overlapping whitespace repetitions and add this adversarial regression. Severity: High. Blocks: yes
+
+2. **A literal first argument incorrectly exempts input-controlled destinations** — `lib/review-risk.ts:50–52`. `requests.request("GET", user_url)` and `httpx.request("POST", user_url)` pass a method first, but `NOT_LITERAL` treats it as a safe URL. Node’s `http.get("http://public.example/", { hostname: req.query.target }, cb)` also lets options override the literal destination. All three reproduce `risk=[]`, `depth=light`, omitting the separate Red Team and outside voice. A local Node server confirmed that the hostname override changes the actual destination. Handle destination arguments and overriding options according to each API’s signature. Severity: Medium. Blocks: yes
+
+3. **One type-only specifier hides executable child-process imports** — `lib/review-risk.ts:79`. A complete multiline import containing `type ExecOptions, exec as run,` is rejected wholesale by `!/^type\b/.test(names)`. With `run(cmd)` afterward, the classifier returns no security risk and light depth. Bun’s TypeScript transpiler confirms this retains the executable `exec as run` import. Check individual value specifiers instead of excluding the entire clause when its first specifier is type-only. Severity: Medium. Blocks: yes
+
+Recommendation: request changes because a whitespace-padded exec binding causes quadratic CPU consumption in the synchronous review classifier.

test/helpers/free-fixtures-data.ts

@@ -208,6 +208,7 @@ export const FREE_FIXTURES: Readonly<Record<string, readonly string[]>> = {
   'test/fixtures/review-risk/gbrain-evals-289.json': ['test/review-risk-routing.test.ts'],
   'test/fixtures/review-design-lite-reports-ci-36633323521.json': ['test/review-finalization-budget.test.ts'],
   'test/fixtures/review-gate-integrity-opus-reports.json': ['test/gate-integrity-templates.test.ts'],
+  'test/fixtures/review-wrong-tree/**': ['test/review-dispatch.test.ts'],
   'test/fixtures/session-update-heartbeat.sh': ['test/session-update-heartbeat.test.ts'],
   'test/fixtures/shard-cli-parity/**': ['test/shard-cli-parity.test.ts'],
   'test/fixtures/shard-equivalence/**': ['test/shard-engine-equivalence.test.ts'],

test/review-dispatch.test.ts

@@ -3,7 +3,7 @@ import { spawnSync } from 'node:child_process';
 import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import { BriefError, checkReviewedTree, promptHash, renderBrief, sectionTexts, workspaceParagraph } from '../lib/review-brief';
+import { BriefError, checkReviewedTree, promptHash, renderBrief, sectionTexts, workspaceParagraph, workspaceStep } from '../lib/review-brief';
 import { collect, judge, parseFindings, readJsonl, type MergedLine } from '../lib/review-ledger';
 
 const ROOT = path.resolve(import.meta.dir, '..');
@@ -705,7 +705,7 @@ describe('the workspace paragraph (GST-482)', () => {
       const r = run(['render', '--role', role, '--out', out, ...(role === 'fix-check' ? [] : ['--diff-base', SHA]), ...args]);
       expect(r.status, role + ': ' + r.stderr).toBe(0);
       const main = renderBrief(role as Parameters<typeof renderBrief>[0], { diff_base: role === 'fix-check' ? undefined : SHA, ...slots }, t);
-      expect(fs.readFileSync(field(r.stdout, 'file'), 'utf8'), role).toBe(main + '\n\n' + workspaceParagraph(top));
+      expect(fs.readFileSync(field(r.stdout, 'file'), 'utf8'), role).toBe(workspaceStep(top) + '\n\n' + main + '\n\n' + workspaceParagraph(top));
     }
     expect(workspaceParagraph(top)).toContain(`begin each command with \`cd '${top}' && \``);
     expect(workspaceParagraph(top)).toContain(`absolute path under ${top}`);
@@ -761,6 +761,32 @@ describe('reading the REVIEWED_TREE line (GST-482 review fixes)', () => {
     expect([u.status, u.stdout.startsWith('COLLECT_UNAVAILABLE: reader=outside-voice')]).toEqual([0, true]);
   });
 
+  // Value: protects=the fingerprint request opens every brief and asks for the command's output line, so a reader that
+  //   follows a findings-only format still binds its tree; the captured gpt-6.1-sol replies from GST-850, which never ran
+  //   the command, stay refused; fails_when=the request only closes the brief (GST-850 p1/p2 COLLECT_WRONG_TREE) or reads
+  //   as "copy the command", or collect relaxes for a reply with findings and no line; seam=lib/review-brief.ts
+  //   withWorkspace, bin/gstack-review-dispatch collect; no_claim=a model's compliance (the GST-897 replay measures that)
+  test('the fingerprint step opens the brief and its command prints a line collect accepts; GST-850 gpt-6.1-sol replies stay refused (GST-897)', () => {
+    const out = tmp();
+    const ledger = path.join(out, 'review-ledger.jsonl');
+    const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
+    const file = field(run(['render', '--role', 'outside-voice', '--out', out, '--diff-base', head]).stdout, 'file');
+    const brief = fs.readFileSync(file, 'utf8');
+    const first = brief.split('\n')[0]!;
+    expect(first).toStartWith(`First step, before anything else: run \`cd '${top}' && echo "REVIEWED_TREE: `);
+    expect(first).toContain('Copy that output line, not the command, as the first line of your final reply');
+    expect(brief.slice(brief.lastIndexOf('\n\n') + 2)).toContain('(the output, not the command)');
+    const command = /run `([^`]+)`/.exec(first)![1]!;
+    const printed = spawnSync('bash', ['-c', command], { cwd: os.tmpdir(), encoding: 'utf8', timeout: 30_000 }).stdout.trim();
+    expect(checkReviewedTree(printed + '\nNO FINDINGS', expected).ok).toBe(true);
+    run(['sent', '--ledger', ledger, '--role', 'outside-voice', '--sent', file]);
+    for (const p of ['p1', 'p2']) {
+      const reply = path.join(ROOT, `test/fixtures/review-wrong-tree/gpt-6.1-sol-gst850-${p}.out`);
+      const refused = spawnSync(BIN, ['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', reply, '--merged', path.join(out, `${p}.jsonl`)], { cwd: ROOT, encoding: 'utf8', timeout: 60_000 });
+      expect([p, refused.status, refused.stdout.startsWith('COLLECT_WRONG_TREE: reader=outside-voice reported="none"')]).toEqual([p, 1, true]);
+    }
+  });
+
   test('render outside a readable checkout refuses instead of recording a fingerprint no reply can match', () => {
     const dir = tmp();
     const r = run(['render', '--role', 'adversarial-native', '--out', path.join(dir, 'o'), '--diff-base', SHA], dir);

test/review-pass.test.ts

@@ -7,7 +7,7 @@ import { spawnSync } from 'node:child_process';
 import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import { promptHash, sectionTexts, withWorkspace } from '../lib/review-brief';
+import { promptHash, sectionTexts, withWorkspace, workspaceParagraph, workspaceStep } from '../lib/review-brief';
 import { CHUNK_CHARS, PARENT_ONLY_QA, buildManifest, chunkCount, chunkOf, markReader, renderStage, specialistFindings, type Manifest } from '../lib/review-pass';
 import type { Dispatch } from '../lib/review-depth';
 
@@ -944,7 +944,7 @@ describe('readers on a shared machine review the checkout the pass ships (GST-48
     expect(res).not.toContain('STOP:');
     expect(res).toContain('NEXT: judge (Phase 4)');
     expect(h.creates.length).toBeGreaterThan(2);
-    for (const c of h.creates) expect(c.prompt.endsWith('\n\n' + withWorkspaceTail(wt)), c.title).toBe(true);
+    for (const c of h.creates) expect([c.prompt.startsWith(workspaceStep(wt) + '\n\n'), c.prompt.endsWith('\n\n' + workspaceParagraph(wt))], c.title).toEqual([true, true]);
     expect(bin(wt, env, ['verify', '--ledger', path.join(out, 'review-ledger.jsonl')]).stdout).not.toContain('wrong_tree');
   }, 240_000);
 
@@ -1011,5 +1011,4 @@ describe('readers on a shared machine review the checkout the pass ships (GST-48
     expect(v).toMatch(/^BRIEF outside-voice UNAVAILABLE /m);
   }, 240_000);
 });
-const withWorkspaceTail = (dir: string) => withWorkspace('', dir).slice(2);
 const briefDirOf = (h: ReturnType<typeof host>, title: string) => briefDir(h.creates.find(c => c.title === title)?.prompt ?? '');