review: move to gh-aw v0.83.4; retire the firewall pin and restore sub-agent retries

.changeset/gh-aw-v0834.md

@@ -0,0 +1,13 @@
+---
+"review": patch
+---
+
+Move the reviewer onto gh-aw v0.83.4 (from v0.81.6) and retire the firewall workaround that release makes obsolete. Three parts.
+
+(1) The toolchain: this repo's installed reviewer is recompiled with v0.83.4, which bumps the pinned setup action, the MCP gateway (v0.3.30 to v0.4.6), the GitHub MCP server (v1.4.0 to v1.7.0), the Claude Code CLI (2.1.191 to 2.1.220), and the `actions/*` pins. Two execution-model changes ride along and are the reason this lands as its own change rather than inside a feature PR: the awf sandbox now runs **rootless** (no `sudo`, `install_awf_binary.sh --rootless`) and the MCP gateway moves from host networking to a bridge network the sandbox attaches to (`isolation: true`, `topologyAttach: [awmg-mcpg]`, `MCP_GATEWAY_DOMAIN: awmg-mcpg`). The compiled invariants the reviewer depends on were re-verified under v0.83.4 on a scratch consumer layout: the dispatch-conformance gate still compiles between "Ingest agent output" and "Upload agent artifacts", the `engine.env` Bash-timeout override still replaces the generated 60s defaults on the engine step (`BASH_MAX_TIMEOUT_MS: 1200000`), inline sub-agents still extract to `.claude/agents` (what the dispatcher reads), the safe-output handler set and `upload_artifact` `allowed-paths` are byte-identical, and no step was dropped.
+
+(2) The firewall pin and the `models:` pricing override are removed, from the shared frontmatter and from this repo's installed copy (the latter as a marked `KHAN/ACTIONS LOCAL OVERRIDE`, since the install stays on `review-v1.7.0` until its next bump). **Do not re-pin `sandbox.agent.version` below v0.27.42 while on gh-aw >= v0.83:** v0.83.4 compiles the agent to reach the MCP gateway over a bridge network (`MCP_GATEWAY_DOMAIN: awmg-mcpg`, `network.isolation`, `network.topologyAttach`), and firewall v0.27.27 implements none of those keys — they are absent from its resolved config, and its squid allowlist carries `.host.docker.internal` with no route to `awmg-mcpg`. Measured on two runs of Khan/actions#296: with the pin (run 30290472047), 3 `TCP_DENIED` 403s on `POST awmg-mcpg:8080/mcp/github` and `/mcp/safeoutputs` — a gateway path the lock expects to work, rejected — though MCP still functioned over the path that bypasses squid (10 `tools/call`) and the run's failure was the pre-existing 20-minute step timeout, not the denials; without the pin (run 30292838824, firewall v0.27.42), zero firewall denials, 16 `tools/call` including the safe-output posts, and a posted review. The removal is also correct on its own terms: the pin existed only because claude-fable-5 was absent from the AI-credits pricing table of the firewall the old gh-aw defaulted to (v0.27.11), which 400s an un-priced model, and v0.27.42 prices it while pinning every container by digest. `sandbox.agent.id: awf` stays declared, since the api-proxy is what meters AI credits and caps a runaway fan-out.
+
+(3) Sub-agent retries are restored in code. v0.83 sets `ANTHROPIC_MAX_RETRIES=0` on the engine step so a terminal error (403 `ai_credits_limit_exceeded`) reaches the harness that owns retry/backoff for 429/529 — that harness wraps the orchestrator process only. The dispatcher's sub-agents are spawned inside it with no such wrapper, so they would have inherited 0 and turned any transient overload into a shed lens on every scripted run. `dispatch-runner.ts` now passes the SDK's `env` option with `ANTHROPIC_MAX_RETRIES` back at the SDK default for the sub-agent subprocesses alone (spreading `process.env`, since that option replaces the environment rather than merging it).
+
+Also disables gh-aw's newly generated `agentics-maintenance.yml` (`.github/workflows/aw.json`: `{"maintenance": false}`) in this repo rather than adopting a daily scheduled cleanup workflow as a side effect of a version bump; adopt it deliberately if wanted.

.github/aw/actions-lock.json

@@ -1,14 +1,14 @@
 {
   "entries": {
-    "github/gh-aw-actions/setup-cli@v0.81.6": {
+    "github/gh-aw-actions/setup-cli@v0.83.4": {
       "repo": "github/gh-aw-actions/setup-cli",
-      "version": "v0.81.6",
-      "sha": "ba6380cc6e5be5d21677bebe04d52fb48e3abec7"
+      "version": "v0.83.4",
+      "sha": "e89c65e17eb281bbd5ff2ff9e9199a03e96654c7"
     },
-    "github/gh-aw-actions/setup@v0.81.6": {
+    "github/gh-aw-actions/setup@v0.83.4": {
       "repo": "github/gh-aw-actions/setup",
-      "version": "v0.81.6",
-      "sha": "ba6380cc6e5be5d21677bebe04d52fb48e3abec7"
+      "version": "v0.83.4",
+      "sha": "e89c65e17eb281bbd5ff2ff9e9199a03e96654c7"
     }
   }
 }

.github/workflows/aw.json

@@ -0,0 +1,3 @@
+{
+    "maintenance": false
+}

.github/workflows/review.md

@@ -194,38 +194,25 @@ engine:
 # target sat exactly on the kill line, so shedding could never save them.
 timeout-minutes: 40
 
-# claude-fable-5 (pinned by first-principles and correctness-reviewer) is not in the
-# AI-credits pricing table of the firewall api-proxy that gh-aw <= v0.81.x pins
-# (gh-aw-firewall v0.27.11), and the proxy rejects any un-priced model with a 400,
-# so the first-principles dispatch fails on every run where it is enabled. Two
-# pieces fix that, and BOTH pin to the same upstream source of truth
-# (gh-aw-firewall v0.27.27, the release that added curated Claude 5 pricing:
-# $10/M input, $1/M cache read, $12.50/M cache write, $50/M output):
-#
-# 1. `sandbox.agent.version` below runs that firewall version, whose api-proxy
-#    guard knows the model. This is the piece that actually unblocks the dispatch;
-#    the `models:` frontmatter only feeds gh-aw's cost-summary display and does
-#    NOT reach the proxy guard (verified empirically on gh-aw v0.81.6).
-# 2. The `models:` block keeps the run's cost accounting/display correct for the
-#    same model.
-#
-# Remove both once the workflow runs on a gh-aw release whose default firewall
-# is >= v0.27.27.
+# KHAN/ACTIONS LOCAL OVERRIDE: the `sandbox.agent.version: v0.27.27` pin and the
+# `models:` claude-fable-5 pricing block that review-v1.7.0 ships are deleted here,
+# ahead of the release that removes them upstream, because gh-aw v0.83.4 makes the
+# pin actively BREAK the run rather than merely freeze it. v0.83.4 compiles the
+# agent to reach the MCP gateway over a bridge network (`MCP_GATEWAY_DOMAIN:
+# awmg-mcpg`, `network.isolation`, `network.topologyAttach`); firewall v0.27.27
+# implements none of those keys, drops them from its resolved config, and its squid
+# allowlist therefore has no route to `awmg-mcpg` — so every agent call to the
+# gateway is denied 403. Observed live on run 30290472047 (PR #296): 3 TCP_DENIED
+# POSTs to `awmg-mcpg:8080/mcp/github` and `/mcp/safeoutputs`, zero `tools/call` in
+# the gateway RPC log for the whole run, i.e. no GitHub tools and no ability to post
+# a review; the reviewer fell back to Bash and burned the 20-minute step timeout.
+# Dropping the pin takes the gh-aw default (v0.27.42), which implements the topology
+# keys and also prices claude-fable-5, making the `models:` override redundant.
+# Restore neither. This override goes away when this install bumps to the release
+# carrying the same removal in the shared source.
 sandbox:
   agent:
     id: awf
-    version: v0.27.27
-
-models:
-  providers:
-    anthropic:
-      models:
-        claude-fable-5:
-          cost:
-            input: 1.0e-05
-            output: 5.0e-05
-            cache_read: 1.0e-06
-            cache_write: 1.25e-05
 
 # The shared review workflow is more than this markdown file: its deterministic
 # pieces (the finding schema and validator today; the router, computed verdict, and

workflows/review/lib/dispatch-runner.test.ts

@@ -27,6 +27,9 @@ let session: (
     tools: RegisteredTool[],
 ) => AsyncGenerator<Record<string, unknown>>;
 
+/** The options the runner handed the SDK on the last query. */
+let lastOptions: Record<string, unknown>;
+
 vi.mock("@anthropic-ai/claude-agent-sdk", () => ({
     tool: (
         name: string,
@@ -37,6 +40,7 @@ vi.mock("@anthropic-ai/claude-agent-sdk", () => ({
     createSdkMcpServer: (options: {name: string; tools: RegisteredTool[]}) =>
         options,
     query: ({options}: {prompt: string; options: Record<string, unknown>}) => {
+        lastOptions = options;
         const servers = options["mcpServers"] as
             | Record<string, {tools: RegisteredTool[]}>
             | undefined;
@@ -147,3 +151,44 @@ describe("createSdkRunner submit_result (trial suggestion h)", () => {
         expect(result.output).toBe("free text");
     });
 });
+
+/**
+ * The sub-agent subprocess environment. gh-aw >= v0.83 puts
+ * ANTHROPIC_MAX_RETRIES=0 on the engine step for the orchestrator's benefit
+ * (its harness owns retry/backoff); a sub-agent has no such wrapper, so the
+ * runner must not let it inherit that value.
+ */
+describe("createSdkRunner sub-agent environment", () => {
+    beforeEach(() => {
+        session = async function* () {
+            yield success("free text");
+        };
+    });
+
+    it("restores SDK retries instead of inheriting the engine step's 0", async () => {
+        process.env["ANTHROPIC_MAX_RETRIES"] = "0";
+        try {
+            await (
+                await createSdkRunner()
+            )(request());
+        } finally {
+            delete process.env["ANTHROPIC_MAX_RETRIES"];
+        }
+        const env = lastOptions["env"] as Record<string, string | undefined>;
+        expect(env["ANTHROPIC_MAX_RETRIES"]).toBe("2");
+    });
+
+    it("spreads the inherited environment, since `env` replaces rather than merges", async () => {
+        process.env["GH_AW_DISPATCH_RUNNER_PROBE"] = "inherited";
+        try {
+            await (
+                await createSdkRunner()
+            )(request());
+        } finally {
+            delete process.env["GH_AW_DISPATCH_RUNNER_PROBE"];
+        }
+        const env = lastOptions["env"] as Record<string, string | undefined>;
+        expect(env["GH_AW_DISPATCH_RUNNER_PROBE"]).toBe("inherited");
+        expect(env["PATH"]).toBe(process.env["PATH"]);
+    });
+});

workflows/review/lib/dispatch-runner.ts

@@ -11,6 +11,13 @@
 
 import type {AgentRequest, AgentResult, AgentRunner} from "./dispatch";
 
+/**
+ * Anthropic SDK internal retries for a sub-agent subprocess (the SDK's own
+ * default). Set explicitly because the engine step's environment now disables
+ * them; see the `env` note in the options below.
+ */
+const SUBAGENT_MAX_RETRIES = "2";
+
 /**
  * Build the production runner. The SDK and zod are imported lazily here
  * (both installed by the scripted-mode `npm ci` pre-agent step); zod is the
@@ -57,6 +64,17 @@ export const createSdkRunner = async (): Promise<AgentRunner> => {
             allowedTools,
             permissionMode: "bypassPermissions",
             abortController: abort,
+            // gh-aw >= v0.83 sets ANTHROPIC_MAX_RETRIES=0 on the engine step so
+            // that a terminal error (403 ai_credits_limit_exceeded) reaches its
+            // harness immediately, because that harness owns the retry/backoff
+            // loop for 429/529 — for the ORCHESTRATOR process only. These
+            // sub-agents are spawned by the dispatcher inside that process and
+            // have no such wrapper, so inheriting 0 turns any transient
+            // overload into a shed lens. Restore the SDK default for the
+            // sub-agent subprocesses alone. `env` REPLACES the subprocess
+            // environment rather than merging, so process.env is spread first:
+            // the CLI still needs PATH, HOME, and the proxy's steering vars.
+            env: {...process.env, ANTHROPIC_MAX_RETRIES: SUBAGENT_MAX_RETRIES},
         };
         // The structured-final channel (trial suggestion h): an in-process
         // MCP tool whose handler runs the same contract parse the collection

workflows/review/review.md

@@ -185,38 +185,26 @@ engine:
     BASH_MAX_TIMEOUT_MS: "1200000"
 timeout-minutes: 40
 
-# claude-fable-5 (pinned by first-principles and correctness-reviewer) is not in the
-# AI-credits pricing table of the firewall api-proxy that gh-aw <= v0.81.x pins
-# (gh-aw-firewall v0.27.11), and the proxy rejects any un-priced model with a 400,
-# so the first-principles dispatch fails on every run where it is enabled. Two
-# pieces fix that, and BOTH pin to the same upstream source of truth
-# (gh-aw-firewall v0.27.27, the release that added curated Claude 5 pricing:
-# $10/M input, $1/M cache read, $12.50/M cache write, $50/M output):
-#
-# 1. `sandbox.agent.version` below runs that firewall version, whose api-proxy
-#    guard knows the model. This is the piece that actually unblocks the dispatch;
-#    the `models:` frontmatter only feeds gh-aw's cost-summary display and does
-#    NOT reach the proxy guard (verified empirically on gh-aw v0.81.6).
-# 2. The `models:` block keeps the run's cost accounting/display correct for the
-#    same model.
-#
-# Remove both once the workflow runs on a gh-aw release whose default firewall
-# is >= v0.27.27.
+# The awf sandbox stays declared (its api-proxy is what meters AI credits and
+# caps a runaway fan-out), but its version now floats with the gh-aw release
+# rather than being pinned here. History: claude-fable-5 (pinned by
+# first-principles and correctness-reviewer) was missing from the AI-credits
+# pricing table of the firewall api-proxy that gh-aw <= v0.81.x defaulted to
+# (v0.27.11), and the proxy rejects an un-priced model with a 400, so that
+# dispatch failed on every run. This block therefore pinned v0.27.27 (the
+# release that added curated Claude 5 pricing) and carried a `models:` pricing
+# override for the cost display. gh-aw v0.83.4 defaults to firewall v0.27.42,
+# which prices claude-fable-5 and pins each container by digest, so both are
+# retired: keeping the pin would freeze the firewall at the old floor (and give
+# up those digests) while gh-aw moves on. Re-pin a version here only to hold a
+# firewall release BACK, never to move one forward. Before pinning any sub-agent
+# to a newly shipped model, check that the api-proxy prices it
+# (gh-aw-firewall `containers/api-proxy/ai-credits-pricing.js`, falling back to
+# its bundled `models.dev.catalog.json`); an un-priced model is rejected with a
+# 400 on every dispatch.
 sandbox:
   agent:
     id: awf
-    version: v0.27.27
-
-models:
-  providers:
-    anthropic:
-      models:
-        claude-fable-5:
-          cost:
-            input: 1.0e-05
-            output: 5.0e-05
-            cache_read: 1.0e-06
-            cache_write: 1.25e-05
 
 # The shared review workflow is more than this markdown file: its deterministic
 # pieces (the finding schema and validator today; the router, computed verdict, and