action: bump to 0.1.7

- Migrates `WorkflowRunStatus` from `running | completed | cancelled` to a 9-state mirror of `workflow_run.conclusion`. Backfill: old `completed → success`, `cancelled → failure`. New rows write `hook.workflow_run.conclusion` verbatim via `statusFromConclusion`.
- Adds Discord links to `formatApiKeyErrorSummary` (both missing-key and 401 invalid-key shapes).
- Repo console: `<ChronicFailuresCard>` fires when the last 3 terminal-state runs are all `failure`. Pure DB read; latest-run button hidden for pre-dispatch failures (`runId: null`).
- `StatusIcon` distinguishes `cancelled` (gray X, intentional stop) from `failure` (red X) so the visual matches the chronic-card threshold.
- Pre-dispatch failures (workflow lookup miss, dispatch API error) write `failure` instead of `cancelled` so they feed the card.
- Cascade: every `status: "completed"` filter in billing routes / cron / cohort queries / analyzer becomes `status: "success"`.

Verified end-to-end on `pullfrog/preview-722-failure-surfaces` — Better Stack logs confirm webhooks reached the preview deploy and all three e2e runs got `marked as failure (conclusion=failure)` via the new mapper.

Closes #679, #702.

.github/workflows/test-token.yml

@@ -11,6 +11,10 @@ permissions:
 
 jobs:
   test-token:
+    # only run in the upstream publish target. forks inherit this file but
+    # haven't installed the pullfrog github app — running it there 404s our
+    # token endpoint and pollutes our error logs (see #693).
+    if: github.repository == 'pullfrog/pullfrog'
     runs-on: ubuntu-latest
     steps:
       - name: Get installation token

.github/workflows/test.yml

@@ -55,6 +55,9 @@ jobs:
       OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }}
       MOONSHOT_API_KEY: ${{ secrets.MOONSHOT_API_KEY }}
       OPENCODE_API_KEY: ${{ secrets.OPENCODE_API_KEY }}
+      AWS_BEARER_TOKEN_BEDROCK: ${{ secrets.AWS_BEARER_TOKEN_BEDROCK }}
+      AWS_REGION: us-east-1
+      BEDROCK_MODEL_ID: us.anthropic.claude-opus-4-6-v1
       PULLFROG_MODEL: ${{ vars.PULLFROG_MODEL }}
     steps:
       - uses: actions/checkout@v6

.github/workflows/trigger-sync.yml

@@ -10,8 +10,10 @@ permissions:
 
 jobs:
   trigger:
-    # skip if pushed by our bot (breaks the loop)
-    if: github.actor != 'pullfrog[bot]'
+    # only run in the upstream publish target (forks inherit this file but
+    # can't dispatch into pullfrog/app), and skip if pushed by our bot (breaks
+    # the loop).
+    if: github.repository == 'pullfrog/pullfrog' && github.actor != 'pullfrog[bot]'
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v6

agents/claude.ts

@@ -16,19 +16,26 @@ import { mkdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { performance } from "node:perf_hooks";
 import { pullfrogMcpName } from "../external.ts";
+import { BEDROCK_MODEL_ID_ENV, isBedrockAnthropicId } from "../models.ts";
 
 import { getIdleMs, markActivity } from "../utils/activity.ts";
-import { log } from "../utils/cli.ts";
+import { formatJsonValue, log } from "../utils/cli.ts";
 import { installFromNpmTarball } from "../utils/install.ts";
 import { detectProviderError } from "../utils/providerErrors.ts";
 import { addSkill, installBundledSkills } from "../utils/skills.ts";
-import { SPAWN_ACTIVITY_TIMEOUT_CODE, SpawnTimeoutError, spawn } from "../utils/subprocess.ts";
+import {
+  DEFAULT_MAX_RETAINED_BYTES,
+  SPAWN_ACTIVITY_TIMEOUT_CODE,
+  SpawnTimeoutError,
+  spawn,
+  TailBuffer,
+} from "../utils/subprocess.ts";
 import { ThinkingTimer } from "../utils/timer.ts";
 import type { TodoTracker } from "../utils/todoTracking.ts";
 import { getDevDependencyVersion } from "../utils/version.ts";
 import { buildLearningsReflectionPrompt, runPostRunRetryLoop } from "./postRun.ts";
 import { REVIEWER_AGENT_NAME, REVIEWER_SYSTEM_PROMPT } from "./reviewer.ts";
-import { deriveLabelFromTaskInput } from "./sessionLabeler.ts";
+import { formatWithLabel, ORCHESTRATOR_LABEL, SessionLabeler } from "./sessionLabeler.ts";
 import {
   type AgentResult,
   type AgentRunContext,
@@ -66,17 +73,27 @@ function writeMcpConfig(ctx: AgentRunContext): string {
 
 /**
  * Build the `--agents` JSON definition for the `reviewfrog` subagent.
+ *
+ * The Claude Code path always runs against an Anthropic model (see
+ * resolveAgent), so we hardcode the cheaper-sibling downshift: lenses run
+ * on Sonnet, the orchestrator stays on whatever model `--model` was passed.
+ *
+ * Per-call model override is also possible (Task tool's `model` arg accepts
+ * 'sonnet' | 'opus' | 'haiku') and takes precedence over what's set here —
+ * we don't pass it; the per-subagent `model` field is the right default.
+ *
  * The non-mutative + non-recursive contract is enforced by the prose system
- * prompt baked into the agent — see action/agents/reviewer.ts for why we no
- * longer wire per-agent `disallowedTools` here.
+ * prompt baked into the agent — see action/agents/reviewer.ts for why we
+ * no longer wire per-agent `disallowedTools` here.
  */
 function buildAgentsJson(): string {
   const agents = {
     [REVIEWER_AGENT_NAME]: {
       description:
-        "Read-only review subagent for self-review and lens-based code review. " +
+        "Read-only review subagent for lens-based code review (correctness, security, billing-subsystem, etc.). " +
         "Reads only — no writes, no state-changing shell or MCP calls, no nested subagent dispatch.",
       prompt: REVIEWER_SYSTEM_PROMPT,
+      model: "claude-sonnet-4-6",
     },
   };
   return JSON.stringify(agents);
@@ -90,10 +107,13 @@ function stripProviderPrefix(specifier: string): string {
   return slashIndex > 0 ? specifier.slice(slashIndex + 1) : specifier;
 }
 
-// `max` effort is supported on Opus 4.6 / 4.7; other models fall back to `high`.
-// claude-code deny-lists older opus/sonnet generations from `max` at invocation time.
-function resolveEffort(model: string | undefined): "max" | "high" {
-  if (model?.includes("opus")) return "max";
+// `high` is the model's tuned default ("equivalent to not setting the parameter"
+// per Anthropic docs). `max` is "absolute maximum capability with no constraints
+// on token spending" — meaningfully slower and burns more thinking budget per
+// turn. We default everyone to `high`; PRs that genuinely need full-send can
+// opt in via a future per-run override rather than paying the wall-time cost on
+// every Opus run.
+function resolveEffort(_model: string | undefined): "high" {
   return "high";
 }
 
@@ -111,13 +131,21 @@ interface ContentBlock {
   [key: string]: unknown;
 }
 
+// SDK schema (per claude-agent-sdk docs) puts `session_id` and
+// `parent_tool_use_id` at the top level of every Assistant/User/System/Result
+// message, not inside `message`. Subagent events carry a non-null
+// `parent_tool_use_id` pointing at the orchestrator's Task/Agent tool_use id.
 interface ClaudeSystemEvent {
   type: "system";
+  session_id?: string;
+  parent_tool_use_id?: string | null;
   [key: string]: unknown;
 }
 
 interface ClaudeAssistantEvent {
   type: "assistant";
+  session_id?: string;
+  parent_tool_use_id?: string | null;
   message?: {
     role?: string;
     content?: ContentBlock[];
@@ -135,6 +163,8 @@ interface ClaudeAssistantEvent {
 
 interface ClaudeUserEvent {
   type: "user";
+  session_id?: string;
+  parent_tool_use_id?: string | null;
   message?: {
     role?: string;
     content?: ContentBlock[];
@@ -233,7 +263,37 @@ function tailLines(text: string, maxCodeUnits: number): string {
 export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
   const startTime = performance.now();
   let eventCount = 0;
-  const thinkingTimer = new ThinkingTimer();
+
+  // per-session labeler so parallel subagent log lines can be differentiated.
+  // claude-agent-sdk runs subagents inside the orchestrator's session — they
+  // share `session_id` — and stamps every subagent message with a non-null
+  // `parent_tool_use_id` pointing at the Agent tool_use that spawned them.
+  // we bind each Agent tool_use id to its dispatched label up front, then
+  // labelFor short-circuits to the direct mapping when parent_tool_use_id is
+  // set. orchestrator events (parent_tool_use_id === null) flow through the
+  // sessionID path and bind to ORCHESTRATOR_LABEL on first sighting.
+  const labeler = new SessionLabeler();
+  function eventLabel(event: { session_id?: string; parent_tool_use_id?: string | null }): string {
+    return labeler.labelFor(event.session_id ?? null, event.parent_tool_use_id ?? null);
+  }
+  function withLabel(label: string, message: string): string {
+    return label === ORCHESTRATOR_LABEL ? message : formatWithLabel(label, message);
+  }
+
+  // one ThinkingTimer per session — sharing a single timer across sessions
+  // conflated cross-session interleaving as parent thinking time. each timer
+  // formats its log lines through the session label so attribution is visible.
+  const thinkingTimers = new Map<string, ThinkingTimer>();
+  function timerFor(label: string): ThinkingTimer {
+    let t = thinkingTimers.get(label);
+    if (!t) {
+      const formatLine = (line: string) =>
+        label === ORCHESTRATOR_LABEL ? line : formatWithLabel(label, line);
+      t = new ThinkingTimer(formatLine);
+      thinkingTimers.set(label, t);
+    }
+    return t;
+  }
 
   let finalOutput = "";
   let sessionId: string | undefined;
@@ -277,18 +337,30 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
   }
 
   const handlers = {
-    system: (_event: ClaudeSystemEvent) => {
-      log.debug(`» ${params.label} system event`);
+    system: (event: ClaudeSystemEvent) => {
+      // claude-agent-sdk only emits system:init for the top-level query, so
+      // this binds the orchestrator label and never appears in subagent flow.
+      // we still route through eventLabel so a subagent system event (if the
+      // SDK ever adds one) wouldn't go silently misattributed.
+      const label = eventLabel(event);
+      log.debug(withLabel(label, `» ${params.label} system event`));
     },
     assistant: (event: ClaudeAssistantEvent) => {
       const content = event.message?.content;
       if (!content) return;
 
+      const label = eventLabel(event);
+      const boxTitle = label === ORCHESTRATOR_LABEL ? params.label : `${params.label} [${label}]`;
+
       for (const block of content) {
         if (block.type === "text" && block.text?.trim()) {
           const message = block.text.trim();
-          log.box(message, { title: params.label });
-          finalOutput = message;
+          log.box(message, { title: boxTitle });
+          // only the orchestrator's text becomes the run's "output" — subagent
+          // report-back text would otherwise clobber the parent's final answer.
+          if (label === ORCHESTRATOR_LABEL) {
+            finalOutput = message;
+          }
         } else if (block.type === "tool_use") {
           const toolName = block.name || "unknown";
           if (params.onToolUse) {
@@ -297,23 +369,34 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
               input: block.input,
             });
           }
-          thinkingTimer.markToolCall();
-          log.toolCall({ toolName, input: block.input || {} });
+          timerFor(label).markToolCall();
+          const inputFormatted = formatJsonValue(block.input || {});
+          const toolCallLine =
+            inputFormatted !== "{}" ? `» ${toolName}(${inputFormatted})` : `» ${toolName}()`;
+          log.info(withLabel(label, toolCallLine));
 
-          // surface the subagent identity when the orchestrator dispatches a
-          // Task — claude rolls subagent activity up into a single tool_result
-          // (no per-event session_id in its stream), so this log line is the
-          // only attribution available before the subagent's report-back.
-          if (toolName === "Task" && block.input && typeof block.input === "object") {
+          // when the orchestrator dispatches a subagent, bind the Agent
+          // tool_use id to the dispatched label so future events carrying
+          // `parent_tool_use_id === block.id` resolve directly to the right
+          // lens. v2.1.63+ renamed the tool to "Agent"; older versions
+          // emitted "Task". match both for forward-compat.
+          if (
+            (toolName === "Task" || toolName === "Agent") &&
+            block.input &&
+            typeof block.input === "object"
+          ) {
             const taskInput = block.input as {
               description?: string;
               subagent_type?: string;
               prompt?: string;
             };
-            const label = deriveLabelFromTaskInput(taskInput);
+            const dispatchedLabel = labeler.recordTaskDispatch(taskInput, block.id ?? null);
             log.info(
-              `» dispatching subagent: ${label}` +
-                (taskInput.subagent_type ? ` (subagent_type=${taskInput.subagent_type})` : "")
+              withLabel(
+                label,
+                `» dispatching subagent: ${dispatchedLabel}` +
+                  (taskInput.subagent_type ? ` (subagent_type=${taskInput.subagent_type})` : "")
+              )
             );
           }
 
@@ -323,8 +406,14 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
             params.todoTracker.cancel();
           }
 
-          // parse TodoWrite events for live progress tracking
-          if (toolName === "TodoWrite" && params.todoTracker?.enabled) {
+          // parse TodoWrite events for live progress tracking. only honor the
+          // orchestrator's todos — subagents emit their own todo lists which
+          // would otherwise clobber the visible progress comment.
+          if (
+            toolName === "TodoWrite" &&
+            params.todoTracker?.enabled &&
+            label === ORCHESTRATOR_LABEL
+          ) {
             params.todoTracker.update(block.input);
           }
         }
@@ -345,10 +434,12 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
       const content = event.message?.content;
       if (!content) return;
 
+      const label = eventLabel(event);
+
       for (const block of content) {
         if (typeof block === "string") continue;
         if (block.type === "tool_result") {
-          thinkingTimer.markToolResult();
+          timerFor(label).markToolResult();
 
           const outputContent =
             typeof block.content === "string"
@@ -366,9 +457,9 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
                 : String(block.content);
 
           if (block.is_error) {
-            log.info(`» tool error: ${outputContent}`);
+            log.info(withLabel(label, `» tool error: ${outputContent}`));
           } else {
-            log.debug(`» tool output: ${outputContent}`);
+            log.debug(withLabel(label, `» tool output: ${outputContent}`));
           }
         }
       }
@@ -459,10 +550,18 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
   };
 
   const recentStderr: string[] = [];
+  // ring buffer of recent non-JSON stdout lines. Claude CLI prints
+  // human-readable TTY chrome (status bubbles, quota notices, etc.)
+  // alongside the NDJSON event stream. when the CLI exits non-zero without
+  // emitting a structured error event, these lines are the only actionable
+  // signal — preferring them over the NDJSON tail keeps progress comments
+  // readable. issue #643.
+  const recentNonJsonStdout: string[] = [];
 
   let lastProviderError: string | null = null;
 
-  let output = "";
+  // capped accumulator — see opencode.ts for rationale (issue #680).
+  const output = new TailBuffer(DEFAULT_MAX_RETAINED_BYTES);
   let stdoutBuffer = "";
 
   try {
@@ -480,9 +579,14 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
       // there's no shim-orphan issue like opencode-ai/bin/opencode, but
       // detached + killGroup is the right default for any agent runtime.
       killGroup: true,
+      // claude already drains every chunk via onStdout (NDJSON parsing) and
+      // onStderr (recentStderr ring buffer). retaining a second copy in the
+      // spawn wrapper would grow unbounded for long sessions and previously
+      // crashed the wrapper with RangeError. see issue #680.
+      retain: "none",
       onStdout: async (chunk) => {
         const text = chunk.toString();
-        output += text;
+        output.append(text);
         markActivity();
 
         stdoutBuffer += text;
@@ -498,6 +602,8 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
             event = JSON.parse(trimmed) as ClaudeEvent;
           } catch {
             log.debug(`» non-JSON stdout line: ${trimmed.substring(0, 200)}`);
+            recentNonJsonStdout.push(trimmed);
+            if (recentNonJsonStdout.length > MAX_STDERR_LINES) recentNonJsonStdout.shift();
             continue;
           }
 
@@ -592,20 +698,33 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
       // hides the actionable provider message and pollutes the run log. cap
       // the stdout fallback to the last 2KB so it stays readable when neither
       // a structured error nor stderr is available.
-      const truncatedStdout = result.stdout ? tailLines(result.stdout, 2048) : "";
+      //
+      // result.stdout / result.stderr are empty because we pass retain:"none"
+      // to spawn (see issue #680); the agent layer keeps its own bounded
+      // mirrors via `output` (TailBuffer) and `recentStderr` (ring buffer).
+      const stdoutSnapshot = output.toString();
+      const stderrSnapshot = recentStderr.join("\n");
+      const truncatedStdout = stdoutSnapshot ? tailLines(stdoutSnapshot, 2048) : "";
+      // prefer non-JSON stdout (human-readable TTY chrome the CLI prints,
+      // including status bubbles and quota notices) over the raw NDJSON
+      // tail. when the CLI exits 1 without emitting `is_error` (issue #643),
+      // the NDJSON fallback would otherwise dump 2KB of `system/init` events
+      // into the progress comment with no mention of the actual cause.
+      const nonJsonStdoutSnapshot = recentNonJsonStdout.join("\n");
       const errorMessage =
         lastResultError ||
-        result.stderr ||
+        stderrSnapshot ||
+        nonJsonStdoutSnapshot ||
         truncatedStdout ||
         `unknown error - no output from Claude CLI${errorContext}`;
       log.error(
         `${params.label} exited with code ${result.exitCode}${errorContext}: ${errorMessage}`
       );
-      log.debug(`stdout: ${result.stdout?.substring(0, 500)}`);
-      log.debug(`stderr: ${result.stderr?.substring(0, 500)}`);
+      log.debug(`stdout: ${stdoutSnapshot.substring(0, 500)}`);
+      log.debug(`stderr: ${stderrSnapshot.substring(0, 500)}`);
       return {
         success: false,
-        output: finalOutput || output,
+        output: finalOutput || stdoutSnapshot,
         error: errorMessage,
         usage,
         sessionId,
@@ -615,7 +734,7 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
     if (eventCount === 0 && lastProviderError) {
       return {
         success: false,
-        output: finalOutput || output,
+        output: finalOutput || output.toString(),
         error: `provider error: ${lastProviderError}`,
         usage,
         sessionId,
@@ -625,14 +744,14 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
     if (resultErrorSubtype) {
       return {
         success: false,
-        output: finalOutput || output,
+        output: finalOutput || output.toString(),
         error: lastResultError || `result subtype: ${resultErrorSubtype}`,
         usage,
         sessionId,
       };
     }
 
-    return { success: true, output: finalOutput || output, usage, sessionId };
+    return { success: true, output: finalOutput || output.toString(), usage, sessionId };
   } catch (error) {
     params.todoTracker?.cancel();
     const duration = performance.now() - startTime;
@@ -658,7 +777,7 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
 
     return {
       success: false,
-      output: finalOutput || output,
+      output: finalOutput || output.toString(),
       error: `${errorMessage} [${diagnosis}]`,
       usage: buildUsage(),
       sessionId,
@@ -726,7 +845,22 @@ export const claude = agent({
     const cliPath = await installClaudeCli();
 
     const specifier = ctx.payload.proxyModel ?? ctx.resolvedModel;
-    const model = specifier ? stripProviderPrefix(specifier) : undefined;
+    // claude-code on Bedrock takes the bare AWS model ID — no provider prefix
+    // to strip, since the ID is already in `provider.model` form (e.g.
+    // `us.anthropic.claude-opus-4-7`). detect via the env-var sentinel: if
+    // BEDROCK_MODEL_ID is set and matches the resolved specifier, this is a
+    // bedrock route. see `wiki/model-resolution.md` for the routing pattern.
+    const bedrockModelId = process.env[BEDROCK_MODEL_ID_ENV]?.trim();
+    const isBedrockRoute =
+      specifier !== undefined &&
+      bedrockModelId !== undefined &&
+      bedrockModelId === specifier &&
+      isBedrockAnthropicId(specifier);
+    const model = !specifier
+      ? undefined
+      : isBedrockRoute
+        ? specifier
+        : stripProviderPrefix(specifier);
 
     const homeEnv = {
       HOME: ctx.tmpdir,
@@ -773,10 +907,24 @@ export const claude = agent({
 
     // agent process gets full env — needs LLM API keys, PATH, locale, etc.
     // security is enforced via managed-settings.json, --disallowedTools (Bash), and MCP tool filtering.
+    //
+    // bedrock route: claude-code reads `CLAUDE_CODE_USE_BEDROCK=1` to switch
+    // its provider implementation from the direct Anthropic API to Bedrock.
+    // AWS_BEARER_TOKEN_BEDROCK / AWS_ACCESS_KEY_ID + AWS_SECRET_ACCESS_KEY +
+    // AWS_REGION are already in process.env from the workflow's `env:` block.
+    // see https://docs.claude.com/en/docs/claude-code/amazon-bedrock.
+    //
+    // we only force CLAUDE_CODE_USE_BEDROCK=1 when this is a Pullfrog-routed
+    // bedrock run; if the user has set the env var manually for some other
+    // reason (e.g. always-Bedrock org policy), `...process.env` already
+    // carries it through and we don't disturb it.
     const env: Record<string, string | undefined> = {
       ...process.env,
       ...homeEnv,
     };
+    if (isBedrockRoute) {
+      env.CLAUDE_CODE_USE_BEDROCK = "1";
+    }
 
     const repoDir = process.cwd();
 

agents/opencode.ts

@@ -16,13 +16,19 @@ import { mkdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { performance } from "node:perf_hooks";
 import { pullfrogMcpName } from "../external.ts";
-import { modelAliases } from "../models.ts";
+import { BEDROCK_MODEL_ID_ENV, modelAliases } from "../models.ts";
 import { getIdleMs, markActivity } from "../utils/activity.ts";
 import { formatJsonValue, log } from "../utils/cli.ts";
 import { installFromNpmTarball } from "../utils/install.ts";
 import { detectProviderError } from "../utils/providerErrors.ts";
 import { addSkill, installBundledSkills } from "../utils/skills.ts";
-import { SPAWN_ACTIVITY_TIMEOUT_CODE, SpawnTimeoutError, spawn } from "../utils/subprocess.ts";
+import {
+  DEFAULT_MAX_RETAINED_BYTES,
+  SPAWN_ACTIVITY_TIMEOUT_CODE,
+  SpawnTimeoutError,
+  spawn,
+  TailBuffer,
+} from "../utils/subprocess.ts";
 import { ThinkingTimer } from "../utils/timer.ts";
 import type { TodoTracker } from "../utils/todoTracking.ts";
 import { getDevDependencyVersion } from "../utils/version.ts";
@@ -42,6 +48,7 @@ import {
   logTokenTable,
   MAX_STDERR_LINES,
 } from "./shared.ts";
+import { deriveSubagentModels } from "./subagentModels.ts";
 
 async function installOpencodeCli(): Promise<string> {
   return await installFromNpmTarball({
@@ -59,26 +66,31 @@ type OpenCodeConfig = {
   permission?: Record<string, unknown>;
   provider?: Record<string, unknown>;
   agent?: Record<string, unknown>;
+  experimental?: Record<string, unknown>;
   model?: string;
   enabled_providers?: string[];
   [key: string]: unknown;
 };
 
-/**
- * Per-inference `max_tokens` reservation the agent sends to the upstream
- * model. OpenCode's default is 32_000 (sized for long-running TUI sessions
- * where a human user might want big outputs). Pullfrog runs are headless and
- * short — typical outputs are 1-3K tokens — so we cap at 5_000. This
- * drastically reduces the upfront budget reservation OpenRouter requires per
- * call (~$0.38 vs ~$2.40 for Opus), which is what lets low-wallet runs
- * actually start.
- *
- * Plumbed via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX` env var rather than the
- * config JSON. OpenCode's `OUTPUT_TOKEN_MAX` (session/llm.ts) is sourced
- * exclusively from this env var; top-level `limit.output` in the config
- * has no read site and is silently dropped on merge.
- */
-const PULLFROG_OPENCODE_OUTPUT_LIMIT = 5000;
+// NOTE: OpenCode's per-call `max_tokens` defaults to 32_000. We previously
+// overrode this via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX = 5000` in #616
+// to lower OpenRouter's per-call upfront budget reservation — back when the
+// `ROUTER_PER_RUN_LIMIT_USD = 25` per-run key cap meant that reservation was
+// a hard gate that could lock low-balance accounts out of starting a run.
+//
+// That gate is gone (see `app/api/proxy-token/route.ts` ~line 422 — "Per-run
+// key budget … is decoupled from wallet balance"); the router now mints
+// keys with `keyLimitCents = balance + buffer` ($50 / $5 / $0). The override
+// no longer materially helps, and as a hard per-call output truncation it
+// actively hurt: a single `create_pull_request_review` tool_use with many
+// inline comments would truncate mid-stream past 5K output tokens, the JSON
+// was unparseable, and the tool never invoked. We hit this on PR #710's
+// verify-downshift PR. Removed in #710 — using OpenCode's 32K default.
+//
+// If you need to re-cap output for some reason, set
+// `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX` in the action env. OpenCode's
+// top-level `limit.output` config field has no read site (silently dropped
+// on merge in session/llm.ts), so the env var is the only working knob.
 
 /**
  * upstream opencode hardcodes `thinkingLevel: "high"` as the default for every
@@ -113,7 +125,21 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s
     mcp: {
       [pullfrogMcpName]: { type: "remote", url: ctx.mcpServerUrl },
     },
-    agent: buildReviewerAgentConfig(),
+    agent: (() => {
+      const cfg = buildReviewerAgentConfig(model);
+      const reviewerModel = (cfg[REVIEWER_AGENT_NAME] as { model?: string })?.model ?? "(inherit)";
+      log.info(`» subagent models: reviewfrog=${reviewerModel}`);
+      return cfg;
+    })(),
+    // opt into opencode's experimental `batch` tool (added in
+    // anomalyco/opencode PR #2983, opt-in via `experimental.batch_tool`). it
+    // exposes a single `batch` tool that runs 1-25 independent tool calls
+    // (read/grep/glob/bash/etc.) concurrently in one assistant turn, which
+    // collapses the dominant grep→20×read pattern into a single round trip.
+    // edits are explicitly disallowed inside the batch upstream. paired with
+    // the "Parallel tool execution" guidance in utils/instructions.ts so the
+    // model actually reaches for it. see wiki/prompt.md.
+    experimental: { batch_tool: true },
     provider: {
       google: {
         models: Object.fromEntries(
@@ -143,19 +169,30 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s
 }
 
 /**
- * Read-only subagent for self-review and /anneal lens dispatch. The
- * non-mutative + non-recursive contract is enforced by the prose system
- * prompt — see action/agents/reviewer.ts for why we no longer wire per-agent
- * tool/permission denies here.
+ * Read-only `reviewfrog` subagent for lens-based review.
+ *
+ * Non-mutative + non-recursive — enforced by the prose system prompt in
+ * reviewer.ts.
+ *
+ * Per-subagent `model:` override is driven by the registry in
+ * `action/models.ts` via each alias's `subagentModel` field — see
+ * `deriveSubagentModels` for the reverse-lookup. Currently wired:
+ * Anthropic opus → sonnet, OpenAI gpt-pro → gpt and gpt → gpt-5.4,
+ * Google gemini-pro → gemini-flash. Other providers (xai, deepseek,
+ * moonshot) and already-cheap tiers inherit (no override) — either the
+ * absolute savings are too small to justify or there's no clean
+ * cheaper-but-capable sibling.
  */
-function buildReviewerAgentConfig(): Record<string, unknown> {
+function buildReviewerAgentConfig(orchestratorModel: string | undefined): Record<string, unknown> {
+  const overrides = deriveSubagentModels(orchestratorModel);
   return {
     [REVIEWER_AGENT_NAME]: {
       description:
-        "Read-only review subagent for self-review and lens-based code review. " +
+        "Read-only review subagent for lens-based code review (correctness, security, billing-subsystem, etc.). " +
         "Reads only — no writes, no state-changing shell or MCP calls, no nested subagent dispatch.",
       mode: "subagent",
       prompt: REVIEWER_SYSTEM_PROMPT,
+      ...(overrides.reviewer !== undefined ? { model: overrides.reviewer } : {}),
     },
   };
 }
@@ -193,9 +230,12 @@ function autoSelectModel(cliPath: string): string | undefined {
   const availableSet = new Set(availableModels);
   if (availableSet.size > 0) {
     log.debug(`» opencode models (${availableSet.size}): ${availableModels.join(", ")}`);
+    // skip hidden aliases (internal subagent-tier targets like opencode/gpt-5.4) —
+    // they should never surface as a user-facing orchestrator pick. mirrors the
+    // selectable-list filter in components/ModelSelector.tsx and action/commands/init.ts.
     const match =
-      modelAliases.find((a) => a.preferred && availableSet.has(a.resolve)) ??
-      modelAliases.find((a) => availableSet.has(a.resolve));
+      modelAliases.find((a) => !a.hidden && a.preferred && availableSet.has(a.resolve)) ??
+      modelAliases.find((a) => !a.hidden && availableSet.has(a.resolve));
     if (match) {
       log.info(
         `» model: ${match.resolve} (auto-selected${match.preferred ? " — preferred" : ""} curated match)`
@@ -377,7 +417,6 @@ type RunParams = {
 async function runOpenCode(params: RunParams): Promise<AgentResult> {
   const startTime = performance.now();
   let eventCount = 0;
-  const thinkingTimer = new ThinkingTimer();
 
   let finalOutput = "";
   let accumulatedTokens = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 };
@@ -409,6 +448,23 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
     return label === ORCHESTRATOR_LABEL ? message : formatWithLabel(label, message);
   }
 
+  // one ThinkingTimer per session — sharing a single timer across sessions
+  // conflated cross-session interleaving (parent thinks → child tool_call,
+  // or child returns → parent dispatches next) as parent thinking time. each
+  // timer formats its log lines through the session label so the "thought
+  // for X" attribution is visible in the merged stream.
+  const thinkingTimers = new Map<string, ThinkingTimer>();
+  function timerFor(label: string): ThinkingTimer {
+    let t = thinkingTimers.get(label);
+    if (!t) {
+      const formatLine = (line: string) =>
+        label === ORCHESTRATOR_LABEL ? line : formatWithLabel(label, line);
+      t = new ThinkingTimer(formatLine);
+      thinkingTimers.set(label, t);
+    }
+    return t;
+  }
+
   // tracks per-task dispatch metadata so the matching tool_result can log a
   // labeled "» subagent finished: lens=X duration=Ys" line. this is the most
   // useful per-lens observability available given that subagent-internal
@@ -634,7 +690,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
         });
       }
 
-      thinkingTimer.markToolCall();
+      timerFor(label).markToolCall();
       const inputFormatted = formatJsonValue(event.part?.state?.input || {});
       const toolCallLine =
         inputFormatted !== "{}" ? `» ${toolName}(${inputFormatted})` : `» ${toolName}()`;
@@ -671,7 +727,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
       const output = event.part?.state?.output || event.output;
       const label = eventLabel(event);
 
-      thinkingTimer.markToolResult();
+      timerFor(label).markToolResult();
 
       // surface subagent completion at info level — opencode otherwise hides
       // per-task timing in debug-only logs, so a parallel multi-lens fan-out
@@ -872,7 +928,12 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
   let lastProviderError: string | null = null;
   let agentErrorEvent: OpenCodeErrorEvent | null = null;
 
-  let output = "";
+  // capped accumulator for the agent's narration. used as a post-run fallback
+  // when `finalOutput` (the orchestrator's final assistant message) is empty.
+  // unbounded `output += text` previously grew to ~1 GiB on multi-lens Reviews
+  // and contributed to the wrapper-level RangeError. retain:"none" on spawn
+  // skips the duplicate buffer there; this TailBuffer caps the agent layer.
+  const output = new TailBuffer(DEFAULT_MAX_RETAINED_BYTES);
   let stdoutBuffer = "";
 
   try {
@@ -891,6 +952,11 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
       // never fires — producing zombie runs. detached + killGroup nukes the
       // whole tree.
       killGroup: true,
+      // we already drain every chunk via onStdout/onStderr (NDJSON parsing
+      // + recentStderr ring buffer). retaining a second copy in the spawn
+      // wrapper would grow unbounded for multi-lens Reviews and previously
+      // crashed the wrapper with RangeError at ~1 GiB. see issue #680.
+      retain: "none",
       // NB: we used to pass `isPausedExternally: isSubagentInFlight` to suspend
       // the activity timer during subagent dispatches. unnecessary now that
       // our injected plugin (action/agents/opencodePlugin.ts) re-emits
@@ -900,7 +966,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
       // (~3.3 plugin events/sec during a typical subagent run).
       onStdout: async (chunk) => {
         const text = chunk.toString();
-        output += text;
+        output.append(text);
         markActivity();
 
         stdoutBuffer += text;
@@ -1025,22 +1091,31 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
 
     if (result.exitCode !== 0) {
       const errorContext = lastProviderError ? ` (${lastProviderError})` : "";
+      // result.stdout / result.stderr are empty because we pass retain:"none"
+      // to spawn (see issue #680); use the agent's bounded mirrors instead.
+      const stdoutSnapshot = output.toString();
+      const stderrSnapshot = recentStderr.join("\n");
       const errorMessage =
-        result.stderr ||
-        result.stdout ||
+        stderrSnapshot ||
+        stdoutSnapshot ||
         `unknown error - no output from OpenCode CLI${errorContext}`;
       log.error(
         `${params.label} exited with code ${result.exitCode}${errorContext}: ${errorMessage}`
       );
-      log.debug(`stdout: ${result.stdout?.substring(0, 500)}`);
-      log.debug(`stderr: ${result.stderr?.substring(0, 500)}`);
-      return { success: false, output: finalOutput || output, error: errorMessage, usage };
+      log.debug(`stdout: ${stdoutSnapshot.substring(0, 500)}`);
+      log.debug(`stderr: ${stderrSnapshot.substring(0, 500)}`);
+      return {
+        success: false,
+        output: finalOutput || stdoutSnapshot,
+        error: errorMessage,
+        usage,
+      };
     }
 
     if (eventCount === 0 && lastProviderError) {
       return {
         success: false,
-        output: finalOutput || output,
+        output: finalOutput || output.toString(),
         error: `provider error: ${lastProviderError}`,
         usage,
       };
@@ -1053,13 +1128,13 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
         errorEvent.error?.data?.message || errorEvent.error?.name || JSON.stringify(errorEvent);
       return {
         success: false,
-        output: finalOutput || output,
+        output: finalOutput || output.toString(),
         error: `${errorName}: ${errorMessage}`,
         usage,
       };
     }
 
-    return { success: true, output: finalOutput || output, usage };
+    return { success: true, output: finalOutput || output.toString(), usage };
   } catch (error) {
     params.todoTracker?.cancel();
     const duration = performance.now() - startTime;
@@ -1085,7 +1160,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
 
     return {
       success: false,
-      output: finalOutput || output,
+      output: finalOutput || output.toString(),
       error: `${errorMessage} [${diagnosis}]`,
       usage: buildUsage(),
     };
@@ -1100,7 +1175,24 @@ export const opencode = agent({
   run: async (ctx) => {
     const cliPath = await installOpencodeCli();
 
-    const model = ctx.payload.proxyModel ?? ctx.resolvedModel ?? autoSelectModel(cliPath);
+    const rawModel = ctx.payload.proxyModel ?? ctx.resolvedModel ?? autoSelectModel(cliPath);
+
+    // bedrock route: opencode's `amazon-bedrock` provider expects the model
+    // string in `amazon-bedrock/<bedrock-id>` form. the bare AWS model ID
+    // (what the user puts in `BEDROCK_MODEL_ID`) needs the prefix added.
+    // detect via env-var sentinel — same pattern as claude.ts.
+    //
+    // we deliberately do NOT gate on `!isBedrockAnthropicId(rawModel)` here:
+    // Anthropic-on-Bedrock normally routes to claude-code (per `resolveAgent`),
+    // but `PULLFROG_AGENT=opencode` is the documented escape hatch for forcing
+    // opencode regardless. when that override fires, opencode still needs the
+    // `amazon-bedrock/` prefix or the provider lookup fails with
+    // "Model not found: <modelId>/.". the Anthropic-vs-other discriminant
+    // only belongs in `resolveAgent`.
+    const bedrockModelId = process.env[BEDROCK_MODEL_ID_ENV]?.trim();
+    const isBedrockRoute =
+      rawModel !== undefined && bedrockModelId !== undefined && bedrockModelId === rawModel;
+    const model = isBedrockRoute ? `amazon-bedrock/${rawModel}` : rawModel;
 
     const homeEnv = {
       HOME: ctx.tmpdir,
@@ -1148,7 +1240,6 @@ export const opencode = agent({
       ...homeEnv,
       OPENCODE_CONFIG_CONTENT: buildSecurityConfig(ctx, model),
       OPENCODE_PERMISSION: permissionOverride,
-      OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX: PULLFROG_OPENCODE_OUTPUT_LIMIT.toString(),
       GOOGLE_GENERATIVE_AI_API_KEY:
         process.env.GOOGLE_GENERATIVE_AI_API_KEY || process.env.GEMINI_API_KEY,
     };

agents/postRun.test.ts

@@ -29,9 +29,22 @@ describe("getUnsubmittedReview", () => {
     ).toBeNull();
   });
 
-  it("returns null when report_progress wrote a final summary", () => {
+  it("fires for Review even when report_progress wrote a final summary", () => {
+    // Review's only valid exit is `create_pull_request_review`. a summary
+    // comment is not a substitute, and accepting it here previously let
+    // subagent-flipped `finalSummaryWritten` silence the gate.
     expect(
       getUnsubmittedReview(makeToolState({ selectedMode: "Review", finalSummaryWritten: true }))
+    ).toBe("Review");
+  });
+
+  it("returns null for IncrementalReview when report_progress wrote a final summary", () => {
+    // IncrementalReview treats `report_progress` as a legitimate
+    // "no review warranted" exit, matching the post-failure error message.
+    expect(
+      getUnsubmittedReview(
+        makeToolState({ selectedMode: "IncrementalReview", finalSummaryWritten: true })
+      )
     ).toBeNull();
   });
 

agents/postRun.ts

@@ -30,13 +30,26 @@ import {
  *
  * the gate is anchored to `hadProgressComment` so silent runs (non-issue
  * events, dispatcher skipped seeding) don't fire a nudge there's no UI for.
+ *
+ * `Review` and `IncrementalReview` have different valid exits:
+ *   - Review: only `create_pull_request_review` counts. `report_progress` is
+ *     not a substitute — a Review run that exits with just a summary comment
+ *     has produced nothing reviewable on the PR. matches the hard-fail
+ *     message at `expected = "create_pull_request_review"` below.
+ *   - IncrementalReview: `report_progress` is a legitimate "no review
+ *     warranted" exit, so either toolState flag short-circuits.
+ * splitting per mode also closes the bypass where a subagent (e.g. a
+ * `task`-dispatched `reviewfrog` lens) calls `report_progress` and silences
+ * the gate even though the orchestrator never submitted a review.
  */
 export function getUnsubmittedReview(toolState: ToolState): "Review" | "IncrementalReview" | null {
   const mode = toolState.selectedMode;
-  if (mode !== "Review" && mode !== "IncrementalReview") return null;
-  if (toolState.review || toolState.finalSummaryWritten) return null;
   if (!toolState.hadProgressComment) return null;
-  return mode;
+  if (mode === "Review") return toolState.review ? null : "Review";
+  if (mode === "IncrementalReview") {
+    return toolState.review || toolState.finalSummaryWritten ? null : "IncrementalReview";
+  }
+  return null;
 }
 
 /**
@@ -178,10 +191,16 @@ export async function collectPostRunIssues(
   options: { skipSummaryStale?: boolean } = {}
 ): Promise<PostRunIssues> {
   const issues: PostRunIssues = {};
-  if (ctx.stopScript) {
-    const failure = await executeStopHook(ctx.stopScript);
-    if (failure) issues.stopHook = failure;
-  }
+  // stop hook is disabled — production audit (May 2026) showed 8/9 configured
+  // scripts are foot-guns (duplicates of prepushScript, run on non-committing
+  // modes against unchanged trees) burning the retry budget on un-fixable
+  // gates. re-enable here + the dashboard block in `AgentSettings.tsx` once
+  // we've decided on the right semantics (mode-gating vs. HEAD-changed gating
+  // vs. deletion). see issue #714.
+  // if (ctx.stopScript) {
+  //   const failure = await executeStopHook(ctx.stopScript);
+  //   if (failure) issues.stopHook = failure;
+  // }
   // dirty-tree gate fires only in modes that legitimately commit. Review /
   // IncrementalReview / Plan complete via review submission or a Plan
   // comment, not by touching files — any tree dirt is incidental (e.g. a
@@ -237,6 +256,16 @@ export function buildPostRunPrompt(issues: PostRunIssues): string {
  * the file is the single source of truth — there is no separate MCP tool
  * call. the server reads the file at end-of-run and persists any edits to
  * `Repo.learnings`.
+ *
+ * the prompt copy is shaped by repo-wide audits of the actual content the
+ * agent has been writing (issue #619 in pullfrog/app). recurring failure
+ * modes the framing pushes back on:
+ *  - massive multi-paragraph "bullets" that are really mini-articles
+ *  - PR-/review-/commit-/date-anchored facts that decay within weeks
+ *  - rediscovery of pullfrog-tool quirks that belong in tool descriptions,
+ *    not per-repo learnings
+ *  - sections growing into giant flat lists with no internal structure,
+ *    forcing future runs to read kilobytes to find one fact
  */
 export function buildLearningsReflectionPrompt(filePath: string): string {
   return [
@@ -244,11 +273,25 @@ export function buildLearningsReflectionPrompt(filePath: string): string {
     "",
     `the rolling learnings file is at \`${filePath}\`. read it first if you haven't already, then edit it in place using your native file tools. the server reads this file at end-of-run and persists any changes — there is no tool to call.`,
     "",
-    `keep the file healthy:`,
-    `- only add bullets when the finding is high-confidence AND broadly useful. skip speculative, one-off, or "maybe" findings.`,
-    `- prune bullets that are clearly wrong, no longer relevant, or low-signal (rarely useful). a focused, accurate file beats a long stale one.`,
-    `- format: flat bullet list, one fact per line starting with \`- \`. deduplicate against existing entries — if a bullet covers the same fact, update it in place instead of adding a duplicate.`,
-    `- leave the file alone if you have nothing substantively new to add and the existing entries still look healthy. silence is a valid outcome — just reply "done" and stop.`,
+    `structure:`,
+    `- markdown hierarchy: \`## \` for top-level themes, \`### \` and deeper for sub-themes when a section grows. there is no fixed taxonomy — choose headings that fit THIS repo (e.g. for one repo \`## Migrations\` / \`## Local dev\` may make sense; for another, \`## API quirks\` / \`## Failure modes\`).`,
+    `- **no section over ~300 lines.** when a section is approaching that, split it: introduce \`### \` subsections grouping related bullets, or hoist a coherent group into a new top-level \`## \` section. granular sections mean future runs read targeted line ranges instead of slurping the whole file. this is the most important hygiene rule on long-lived repos.`,
+    `- if you find a flat unstructured list (legacy content from before this format), restructure it: read it, group related bullets, rewrite the file with \`## \` / \`### \` headings around them. don't preserve bad structure — fix it.`,
+    "",
+    `bullet hygiene:`,
+    `- one fact per line starting with \`- \`. each bullet is ONE specific durable fact, not a paragraph or essay.`,
+    `- aim for ≤ 240 chars per bullet. longer bullets are almost always mixing multiple facts that should be split, or burying the durable claim under PR-specific context that should be cut.`,
+    `- only add bullets when the finding is high-confidence AND broadly useful AND will still be true in 3+ months. skip speculative, one-off, or "maybe" findings.`,
+    `- prune bullets that are clearly wrong, no longer relevant, or low-signal. a focused, accurate file beats a long stale one. compressing two overlapping bullets into one tighter bullet counts as progress.`,
+    `- deduplicate against existing entries (in any section) — if a bullet covers the same fact, update it in place instead of adding a duplicate.`,
+    "",
+    `do NOT add bullets for:`,
+    `- pullfrog tool quirks (e.g. "\`shell\` timeout is in milliseconds", "\`git\` args must be a JSON array", "\`create_pull_request_review\` drops out-of-hunk comments", "\`push_branch\` may report timeout when push succeeded"). these are universal across repos and belong in the tool descriptions — flag the gap rather than hoarding the workaround per-repo.`,
+    `- references to specific PR numbers, review IDs, commit SHAs, branch names, or person handles ("PR #595 introduced X", "flagged in review 12345", "as of commit abc123"). repo state changes; these decay into noise within weeks.`,
+    `- dated assertions ("as of May 2026", "currently...", "for now..."). if a fact needs a date to be true, it isn't durable enough to belong here.`,
+    `- play-by-play of what THIS run did. learnings are for the NEXT run, not a retrospective.`,
+    "",
+    `if you have nothing substantively new to add AND the existing entries still look healthy and well-structured, leave the file alone — just reply "done" and stop. silence is a valid outcome.`,
   ].join("\n");
 }
 

agents/sessionLabeler.test.ts

@@ -146,6 +146,40 @@ describe("SessionLabeler", () => {
     ]);
   });
 
+  test("Claude path: parent_tool_use_id resolves directly without consuming FIFO", () => {
+    // Claude runs subagents inside the orchestrator's session — they share
+    // session_id — and stamps subagent messages with parent_tool_use_id.
+    // recording dispatch with the Agent tool_use id binds it directly so
+    // future events resolve regardless of session_id.
+    const labeler = new SessionLabeler();
+    expect(labeler.labelFor("shared-session", null)).toBe(ORCHESTRATOR_LABEL);
+
+    labeler.recordTaskDispatch({ description: "correctness" }, "toolu_01");
+    labeler.recordTaskDispatch({ description: "security" }, "toolu_02");
+
+    // subagent events come through with shared session_id but distinct
+    // parent_tool_use_id — direct mapping wins
+    expect(labeler.labelFor("shared-session", "toolu_01")).toBe("lens:correctness");
+    expect(labeler.labelFor("shared-session", "toolu_02")).toBe("lens:security");
+
+    // orchestrator events on the same session still resolve correctly
+    expect(labeler.labelFor("shared-session", null)).toBe(ORCHESTRATOR_LABEL);
+
+    // pendingLabels is unused on the Claude path — FIFO never consumed
+    expect(labeler.pendingDispatchCount()).toBe(2);
+    expect(labeler.size()).toBe(1);
+  });
+
+  test("Claude path: unknown parent_tool_use_id falls through to sessionID/FIFO logic", () => {
+    // defensive: if a subagent event arrives with a parent_tool_use_id we
+    // never recorded (e.g. orchestrator dispatched off-stream, or a tool we
+    // didn't track), the labeler shouldn't crash — it should fall through
+    // to the sessionID-keyed path.
+    const labeler = new SessionLabeler();
+    labeler.labelFor("shared", null);
+    expect(labeler.labelFor("shared", "unknown-tool-id")).toBe(ORCHESTRATOR_LABEL);
+  });
+
   test("realistic four-lens parallel fan-out — interleaved tool_use stream", () => {
     // simulates the event order we'd see when the orchestrator dispatches
     // 4 lens subagents in a single assistant turn and they all start emitting

agents/sessionLabeler.ts

@@ -67,38 +67,68 @@ export function deriveLabelFromTaskInput(input: TaskDispatchInput): string {
 }
 
 /**
- * Stateful tracker mapping sessionIDs to human labels.
+ * Stateful tracker mapping subagent activity back to human-readable labels.
  *
- * Lifecycle:
- *   - First call to `labelFor()` returns ORCHESTRATOR_LABEL and binds that
- *     sessionID to it. Every subsequent event from that session gets the
- *     same label.
- *   - When the orchestrator emits a Task tool_use, the harness calls
- *     `recordTaskDispatch()` to push the dispatch's derived label onto a
- *     pending FIFO queue.
- *   - The next previously-unseen sessionID consumes the head of the queue.
- *   - If `labelFor()` is called for a new session with an empty queue
- *     (e.g. a subagent emitted events before the parent's tool_use was
- *     parsed, or the runtime spawned a session we didn't expect), the
- *     labeler falls back to `subagent#N` so log lines remain attributable.
+ * Two attribution channels are supported because the runtimes differ:
+ *
+ *   - **OpenCode** spawns each subagent as its own opencode `Session` with
+ *     a distinct `sessionID`. The harness records each Task dispatch into a
+ *     pending FIFO queue; the next previously-unseen sessionID consumes the
+ *     head of the queue and binds it to that label.
+ *
+ *   - **Claude Code** runs subagents inside the orchestrator's session — they
+ *     all share `session_id` — and instead stamps every subagent message with
+ *     `parent_tool_use_id` pointing at the Agent tool_use id that spawned them.
+ *     The harness binds each Agent tool_use id to its dispatched label up
+ *     front, then `labelFor` looks the label up directly when an event arrives
+ *     carrying that `parent_tool_use_id`.
+ *
+ * `labelFor(sessionID, parentToolUseId?)` accepts both: when
+ * `parentToolUseId` is set and known it short-circuits to the direct mapping;
+ * otherwise it falls through to the FIFO/sessionID path.
  */
 export class SessionLabeler {
   private readonly labels = new Map<string, string>();
+  private readonly labelsByToolUseId = new Map<string, string>();
   private readonly pendingLabels: string[] = [];
   private fallbackCounter = 0;
 
-  recordTaskDispatch(input: TaskDispatchInput): string {
+  /**
+   * Record a Task/Agent tool dispatch.
+   *
+   * @param input  Task tool input — used to derive the lens label.
+   * @param toolUseId  Optional Agent tool_use id. When provided, future events
+   *                   carrying `parent_tool_use_id === toolUseId` resolve
+   *                   directly to this label without consuming the FIFO queue
+   *                   (Claude path). Always also pushed to the FIFO queue so
+   *                   the OpenCode path still works when toolUseId is absent.
+   */
+  recordTaskDispatch(input: TaskDispatchInput, toolUseId?: string | null): string {
     const label = deriveLabelFromTaskInput(input);
     this.pendingLabels.push(label);
+    if (toolUseId) this.labelsByToolUseId.set(toolUseId, label);
     return label;
   }
 
   /**
-   * Return a label for the given sessionID. Binds on first call.
-   * Pass undefined/empty for events that lack a session id — the caller
-   * gets ORCHESTRATOR_LABEL so the line is still attributable.
+   * Return a label for the given event.
+   *
+   * @param sessionID         Session id from the event (OpenCode: per-session;
+   *                          Claude: shared across orchestrator + subagents).
+   * @param parentToolUseId   Claude's `parent_tool_use_id` — non-null on
+   *                          subagent messages. When set and known, takes
+   *                          priority over the FIFO/sessionID path.
    */
-  labelFor(sessionID: string | undefined | null): string {
+  labelFor(sessionID: string | undefined | null, parentToolUseId?: string | null): string {
+    // Claude path: subagent messages carry parent_tool_use_id pointing at
+    // the Agent tool_use that spawned them. resolve directly without
+    // touching the sessionID-keyed map (which is bound to the orchestrator
+    // for the shared session_id and would otherwise misattribute).
+    if (parentToolUseId) {
+      const direct = this.labelsByToolUseId.get(parentToolUseId);
+      if (direct) return direct;
+    }
+
     if (!sessionID) return ORCHESTRATOR_LABEL;
     const existing = this.labels.get(sessionID);
     if (existing) return existing;

agents/subagentModels.test.ts

@@ -0,0 +1,110 @@
+import { describe, expect, it } from "vitest";
+import { deriveSubagentModels } from "./subagentModels.ts";
+
+describe("deriveSubagentModels", () => {
+  it("returns no override when orchestrator is undefined", () => {
+    expect(deriveSubagentModels(undefined)).toEqual({ reviewer: undefined });
+  });
+
+  it("returns no override when orchestrator slug isn't registered", () => {
+    expect(deriveSubagentModels("nonexistent/model")).toEqual({ reviewer: undefined });
+  });
+
+  describe("anthropic family — opus → sonnet", () => {
+    it("direct anthropic opus", () => {
+      expect(deriveSubagentModels("anthropic/claude-opus-4-7")).toEqual({
+        reviewer: "anthropic/claude-sonnet-4-6",
+      });
+    });
+    it("opencode-vendored opus stays on opencode prefix", () => {
+      expect(deriveSubagentModels("opencode/claude-opus-4-7")).toEqual({
+        reviewer: "opencode/claude-sonnet-4-6",
+      });
+    });
+    it("openrouter-anthropic-opus-via-anthropic-direct hits anthropic alias's openRouterResolve", () => {
+      // both the anthropic alias and the opencode alias have the same
+      // openRouterResolve. first-match-wins by alias declaration order
+      // (anthropic declared first in providers).
+      expect(deriveSubagentModels("openrouter/anthropic/claude-opus-4.7")).toEqual({
+        reviewer: "openrouter/anthropic/claude-sonnet-4.6",
+      });
+    });
+    it("sonnet has no further downshift", () => {
+      expect(deriveSubagentModels("anthropic/claude-sonnet-4-6")).toEqual({ reviewer: undefined });
+      expect(deriveSubagentModels("opencode/claude-sonnet-4-6")).toEqual({ reviewer: undefined });
+    });
+    it("haiku has no downshift", () => {
+      expect(deriveSubagentModels("anthropic/claude-haiku-4-5")).toEqual({ reviewer: undefined });
+    });
+  });
+
+  describe("openai family", () => {
+    it("gpt-pro → gpt (direct)", () => {
+      expect(deriveSubagentModels("openai/gpt-5.5-pro")).toEqual({ reviewer: "openai/gpt-5.5" });
+    });
+    it("gpt → gpt-5.4 (direct)", () => {
+      expect(deriveSubagentModels("openai/gpt-5.5")).toEqual({ reviewer: "openai/gpt-5.4" });
+    });
+    it("gpt → gpt-5.4 (opencode-vendored)", () => {
+      expect(deriveSubagentModels("opencode/gpt-5.5")).toEqual({ reviewer: "opencode/gpt-5.4" });
+    });
+    it("gpt-pro → gpt (openrouter)", () => {
+      expect(deriveSubagentModels("openrouter/openai/gpt-5.5-pro")).toEqual({
+        reviewer: "openrouter/openai/gpt-5.5",
+      });
+    });
+    it("gpt → gpt-5.4 (openrouter)", () => {
+      expect(deriveSubagentModels("openrouter/openai/gpt-5.5")).toEqual({
+        reviewer: "openrouter/openai/gpt-5.4",
+      });
+    });
+    it("gpt-5.4 itself (the hidden subagent target) has no further downshift", () => {
+      expect(deriveSubagentModels("openai/gpt-5.4")).toEqual({ reviewer: undefined });
+    });
+    it("gpt-mini has no downshift", () => {
+      expect(deriveSubagentModels("openai/gpt-5.4-mini")).toEqual({ reviewer: undefined });
+    });
+  });
+
+  describe("google (gemini) — pro → flash", () => {
+    it("direct google", () => {
+      expect(deriveSubagentModels("google/gemini-3.1-pro-preview")).toEqual({
+        reviewer: "google/gemini-3-flash-preview",
+      });
+    });
+    it("opencode-vendored gemini-pro", () => {
+      expect(deriveSubagentModels("opencode/gemini-3.1-pro")).toEqual({
+        reviewer: "opencode/gemini-3-flash",
+      });
+    });
+    it("openrouter-google-gemini-pro", () => {
+      expect(deriveSubagentModels("openrouter/google/gemini-3.1-pro-preview")).toEqual({
+        reviewer: "openrouter/google/gemini-3-flash-preview",
+      });
+    });
+    it("flash has no downshift", () => {
+      expect(deriveSubagentModels("google/gemini-3-flash-preview")).toEqual({
+        reviewer: undefined,
+      });
+    });
+  });
+
+  describe("providers / models without a subagentModel — inherit", () => {
+    it("xai grok (already cheap flagship)", () => {
+      expect(deriveSubagentModels("xai/grok-4.3")).toEqual({ reviewer: undefined });
+    });
+    it("deepseek", () => {
+      expect(deriveSubagentModels("deepseek/deepseek-v4-pro")).toEqual({ reviewer: undefined });
+    });
+    it("moonshot kimi", () => {
+      expect(deriveSubagentModels("moonshotai/kimi-k2.6")).toEqual({ reviewer: undefined });
+    });
+    it("opencode big-pickle", () => {
+      expect(deriveSubagentModels("opencode/big-pickle")).toEqual({ reviewer: undefined });
+    });
+    it("legacy fallback aliases (gpt-codex, deepseek-reasoner)", () => {
+      expect(deriveSubagentModels("openai/gpt-5.3-codex")).toEqual({ reviewer: undefined });
+      expect(deriveSubagentModels("deepseek/deepseek-reasoner")).toEqual({ reviewer: undefined });
+    });
+  });
+});

agents/subagentModels.ts

@@ -0,0 +1,40 @@
+import { modelAliases } from "../models.ts";
+
+/**
+ * Derive a cheaper subagent model override from the orchestrator's resolved
+ * model spec.
+ *
+ * This is a pure registry lookup: every alias in `action/models.ts` declares
+ * its own `subagentModel` (alias key in the same provider). At runtime we
+ * reverse-lookup the orchestrator's resolved slug to find the alias that
+ * produced it, follow the `subagentModel` pointer, and return the target
+ * alias's resolve / openRouterResolve depending on which route the
+ * orchestrator was using.
+ *
+ * Returns `{ reviewer: undefined }` when the orchestrator's alias has no
+ * `subagentModel` (e.g. it's already at a sufficiently cheap tier, or its
+ * provider doesn't have a clean cheaper-but-capable sibling). See models.ts
+ * for the wiring + per-provider rationale.
+ */
+export function deriveSubagentModels(orchestratorSpec: string | undefined): {
+  reviewer: string | undefined;
+} {
+  if (!orchestratorSpec) return { reviewer: undefined };
+
+  // Reverse-lookup. The same resolve string appears in only one alias
+  // (within its provider), so first match wins. We track which field
+  // matched (resolve vs openRouterResolve) so we can pick the same field
+  // off the subagent target — keeping the orchestrator's route consistent.
+  for (const source of modelAliases) {
+    const matchedDirect = source.resolve === orchestratorSpec;
+    const matchedOR = source.openRouterResolve === orchestratorSpec;
+    if (!matchedDirect && !matchedOR) continue;
+    if (!source.subagentModel) return { reviewer: undefined };
+    const target = modelAliases.find((a) => a.slug === source.subagentModel);
+    if (!target) return { reviewer: undefined };
+    const reviewer = matchedOR ? target.openRouterResolve : target.resolve;
+    return { reviewer };
+  }
+
+  return { reviewer: undefined };
+}

agents/subagentRegistration.test.ts

@@ -0,0 +1,40 @@
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+import { describe, expect, it } from "vitest";
+
+const claudeSource = readFileSync(join(__dirname, "claude.ts"), "utf-8");
+const opencodeSource = readFileSync(join(__dirname, "opencode.ts"), "utf-8");
+
+/**
+ * The Claude Code `--agents` JSON and OpenCode `agent` config block are the
+ * only places where per-subagent model overrides take effect. They're built
+ * by string-only helpers we don't export, so this test reads the source and
+ * asserts the literal model strings + agent names are wired in. A regression
+ * here means the next review run silently runs lenses on Opus instead of
+ * Sonnet.
+ */
+describe("subagent registration source asserts", () => {
+  describe("claude.ts buildAgentsJson", () => {
+    it("registers reviewfrog with sonnet model", () => {
+      expect(claudeSource).toMatch(
+        /\[REVIEWER_AGENT_NAME\]:\s*\{[^}]*model:\s*"claude-sonnet-4-6"/s
+      );
+    });
+    it("imports the reviewer name constant", () => {
+      expect(claudeSource).toMatch(/REVIEWER_AGENT_NAME/);
+    });
+  });
+
+  describe("opencode.ts buildReviewerAgentConfig", () => {
+    it("registers reviewfrog with mode: subagent", () => {
+      expect(opencodeSource).toMatch(/\[REVIEWER_AGENT_NAME\]:[^}]*mode:\s*"subagent"/s);
+    });
+    it("uses deriveSubagentModels for the reviewer model override", () => {
+      expect(opencodeSource).toMatch(/deriveSubagentModels\(/);
+      expect(opencodeSource).toMatch(/overrides\.reviewer/);
+    });
+    it("passes orchestrator model to buildReviewerAgentConfig", () => {
+      expect(opencodeSource).toMatch(/buildReviewerAgentConfig\(model\)/);
+    });
+  });
+});

commands/init.ts

@@ -22,9 +22,16 @@ type CliProvider = {
 
 function buildProviders(): CliProvider[] {
   return Object.entries(providers)
-    .filter(([key]) => key !== "opencode" && key !== "openrouter")
+    .filter(([key]) => key !== "opencode" && key !== "openrouter" && key !== "bedrock")
     .map(([key, config]: [string, ProviderConfig]) => {
-      const aliases = modelAliases.filter((a) => a.provider === key && !a.fallback);
+      // bedrock requires multi-secret setup (auth + region + model id) that
+      // doesn't fit the single-paste flow below — direct users to
+      // https://docs.pullfrog.com/bedrock instead. revisit once the init flow
+      // supports multi-value setup. `hidden` excludes internal-only subagent
+      // targets (e.g. openai/gpt-5.4) per #710.
+      const aliases = modelAliases.filter(
+        (a) => a.provider === key && !a.fallback && !a.routing && !a.hidden
+      );
       const recommended = aliases.find((a) => a.preferred);
       const sorted = [...aliases].sort((a, b) => {
         if (a.preferred && !b.preferred) return -1;

external.ts

@@ -273,6 +273,13 @@ export interface WriteablePayload {
   triggerer?: string | undefined;
   /** event-level instructions for this trigger type (flag-expanded server-side) */
   eventInstructions?: string | undefined;
+  /**
+   * system-injected note about prior superseded runs (e.g. when the
+   * triggering @pullfrog comment is edited). rendered alongside the user's
+   * prompt rather than via eventInstructions so it survives user-prompt
+   * precedence.
+   */
+  previousRunsNote?: string | undefined;
   /** event data from webhook payload - discriminated union based on trigger field */
   event: PayloadEvent;
   /** timeout for agent run (e.g., "10m", "1h30m") - defaults to "1h" */

main.ts

@@ -17,7 +17,11 @@ import {
 } from "./utils/activity.ts";
 import { resolveAgent, resolveModel } from "./utils/agent.ts";
 import { apiFetch } from "./utils/apiFetch.ts";
-import { validateAgentApiKey } from "./utils/apiKeys.ts";
+import {
+  formatApiKeyErrorSummary,
+  isApiKeyAuthError,
+  validateAgentApiKey,
+} from "./utils/apiKeys.ts";
 import { isLocalApiUrl } from "./utils/apiUrl.ts";
 import { resolveBody } from "./utils/body.ts";
 import { formatUsageSummary, log, writeSummary } from "./utils/cli.ts";
@@ -30,7 +34,7 @@ import { createOctokit, writeGitHubUsageSummaryToFile } from "./utils/github.ts"
 import { resolveInstructions } from "./utils/instructions.ts";
 import { readLearningsFile, seedLearningsFile } from "./utils/learnings.ts";
 import { executeLifecycleHook } from "./utils/lifecycle.ts";
-import { normalizeEnv } from "./utils/normalizeEnv.ts";
+import { normalizeEnv, sanitizeSecret } from "./utils/normalizeEnv.ts";
 import { aggregateUsage, patchWorkflowRunFields } from "./utils/patchWorkflowRunFields.ts";
 import { resolvePayload, resolvePromptInput } from "./utils/payload.ts";
 import { isRouterKeylimitExhaustedError } from "./utils/providerErrors.ts";
@@ -183,8 +187,9 @@ function billingConsoleUrl(owner: string, anchor: "billing" | "model-access"): s
  *
  * Branches:
  *   - `router_requires_card`: user is on Router mode with no card AND no
- *     wallet balance. Lead with the carrot ($20 free credit), link to
- *     `#model-access` where the Add Card flow lives.
+ *     wallet balance (signup credit exhausted or not granted). Frame as
+ *     "add a card to continue", link to `#model-access` where the Add
+ *     Card flow lives.
  *   - `router_balance_exhausted`: user has a card on file but auto-reload is
  *     disabled and they've spent past their $5 overdraft buffer. Frame as
  *     "balance ran out" and surface both remediation paths (top up, or flip
@@ -206,7 +211,7 @@ function formatBillingErrorSummary(error: BillingError, owner: string): string {
     return [
       "**Add a card to start using Pullfrog Router.**",
       "",
-      "Router proxies OpenRouter at raw cost — no platform markup, and your first $20 of usage is on us.",
+      "Router proxies OpenRouter at raw cost — no platform markup. Add a card and we'll auto-reload your wallet so runs keep flowing.",
       "",
       `[Add a card →](${billingConsoleUrl(owner, "model-access")})`,
     ].join("\n");
@@ -471,12 +476,16 @@ async function persistLearnings(ctx: ToolContext): Promise<void> {
     });
     if (!response.ok) {
       const error = await response.text().catch(() => "(no body)");
-      log.debug(`learnings persist failed (${response.status}): ${error}`);
+      // promoted from debug → warning: this path means the agent edited the
+      // file (we already short-circuited the unchanged-from-seed case above)
+      // but the PATCH dropped it on the floor. silently losing real work is
+      // worse than the noise of a CI warning.
+      log.warning(`learnings persist failed (${response.status}): ${error}`);
       return;
     }
     log.info("» learnings updated");
   } catch (err) {
-    log.debug(`learnings persist failed: ${err instanceof Error ? err.message : String(err)}`);
+    log.warning(`learnings persist failed: ${err instanceof Error ? err.message : String(err)}`);
   }
 }
 
@@ -554,12 +563,15 @@ export async function main(): Promise<MainResult> {
   const runContext = await resolveRunContextData({ octokit: initialOctokit, token: jobToken });
   timer.checkpoint("runContextData");
 
-  // inject account-level secrets into process.env (YAML secrets take precedence)
+  // inject account-level secrets into process.env (YAML secrets take precedence).
+  // sanitizeSecret trims + masks so accidental trailing whitespace doesn't leak
+  // through GitHub Actions' line-based log masking. whitespace-only values
+  // return null and skip injection so the user sees a clear missing-key error.
   if (runContext.dbSecrets) {
     for (const [key, value] of Object.entries(runContext.dbSecrets)) {
       if (!process.env[key]) {
-        process.env[key] = value;
-        core.setSecret(value);
+        const sanitized = sanitizeSecret(key, value);
+        if (sanitized !== null) process.env[key] = sanitized;
       }
     }
     const count = Object.keys(runContext.dbSecrets).length;
@@ -762,12 +774,10 @@ export async function main(): Promise<MainResult> {
         current: runContext.repoSettings.learnings,
       });
       toolState.learningsFilePath = learningsPath;
-      try {
-        toolState.learningsSeed = await readFile(learningsPath, "utf8");
-      } catch {
-        // intentionally empty — learningsSeed stays undefined, persistLearnings
-        // will treat seed as "" and persist any non-empty content
-      }
+      // file on disk is the verbatim DB body, so the seed used for
+      // change-detection is just `current ?? ""` (trimmed). persistLearnings
+      // byte-compares against the trimmed read-back to skip no-op PATCHes.
+      toolState.learningsSeed = (runContext.repoSettings.learnings ?? "").trim();
       log.info(
         `» learnings seeded at ${learningsPath} (existing=${runContext.repoSettings.learnings ? "yes" : "no"})`
       );
@@ -831,6 +841,7 @@ export async function main(): Promise<MainResult> {
       agentId,
       outputSchema,
       learningsFilePath: toolState.learningsFilePath ?? null,
+      learningsHeadings: runContext.repoSettings.learningsHeadings,
     });
     const logParts = [
       instructions.eventInstructions
@@ -1028,10 +1039,15 @@ export async function main(): Promise<MainResult> {
     // the comment is still around to update; reportErrorToComment sets
     // wasUpdated=true and the !result.success guard skips deletion.
     if (!result.success && toolContext && toolState.progressComment) {
-      await reportErrorToComment({
-        toolState,
-        error: result.error || "agent run failed",
-      }).catch((error) => {
+      const rawError = result.error || "agent run failed";
+      const errorBody = isApiKeyAuthError(rawError)
+        ? formatApiKeyErrorSummary({
+            owner: runContext.repo.owner,
+            name: runContext.repo.name,
+            raw: rawError,
+          })
+        : rawError;
+      await reportErrorToComment({ toolState, error: errorBody }).catch((error) => {
         log.debug(`failure error report failed: ${error}`);
       });
     }
@@ -1107,11 +1123,20 @@ export async function main(): Promise<MainResult> {
       ? new BillingError(errorMessage, { code: "router_keylimit_exhausted" })
       : null;
 
+    const apiKeyErrorSummary =
+      !billingError && isApiKeyAuthError(errorMessage)
+        ? formatApiKeyErrorSummary({
+            owner: runContext.repo.owner,
+            name: runContext.repo.name,
+            raw: errorMessage,
+          })
+        : null;
+
     // best-effort summary — write the error so it's visible in the Actions summary tab
     try {
       const errorSummary = billingError
         ? formatBillingErrorSummary(billingError, runContext.repo.owner)
-        : `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``;
+        : (apiKeyErrorSummary ?? `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``);
       const usageSummary = formatUsageSummary(toolState.usageEntries);
       const parts = [errorSummary, toolState.lastProgressBody, usageSummary].filter(Boolean);
       await writeSummary(parts.join("\n\n"));
@@ -1120,7 +1145,7 @@ export async function main(): Promise<MainResult> {
     try {
       const commentBody = billingError
         ? formatBillingErrorSummary(billingError, runContext.repo.owner)
-        : errorMessage;
+        : (apiKeyErrorSummary ?? errorMessage);
       await reportErrorToComment({ toolState, error: commentBody });
     } catch {
       // error reporting failed, but don't let it mask the original error

mcp/checkout.ts

@@ -593,7 +593,10 @@ export function CheckoutPrTool(ctx: ToolContext) {
     name: "checkout_pr",
     description:
       "Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. " +
-      "Returns diffPath pointing to the formatted diff file.",
+      "Returns diffPath pointing to the formatted diff file. " +
+      "Example: `checkout_pr({ pull_number: 1234 })`. " +
+      "Transient fetch timeouts are common — retry the same call up to a few times before treating the failure as terminal. " +
+      "If the error mentions `.git/shallow.lock: File exists` or `.git/index.lock: File exists`, that's a stale lock from a prior timed-out fetch — remove it via the shell tool (`rm -f .git/shallow.lock .git/index.lock`) and retry.",
     parameters: CheckoutPr,
     execute: execute(async ({ pull_number }) => {
       const prResponse = await ctx.octokit.rest.pulls.get({

mcp/comment.ts

@@ -65,7 +65,9 @@ export function CreateCommentTool(ctx: ToolContext) {
   return tool({
     name: "create_issue_comment",
     description:
-      "Create a comment on a GitHub issue or PR. For progress/plan updates on the current run use report_progress instead. Use type: 'Plan' for plan comments.",
+      "Create a comment on a GitHub issue or PR. " +
+      'Example: `create_issue_comment({ issueNumber: 1234, body: "Thanks for the report." })`. ' +
+      "For progress/plan updates on the current run use report_progress instead. Use type: 'Plan' for plan comments.",
     parameters: Comment,
     execute: execute(async ({ issueNumber, body, type: commentType }) => {
       const bodyWithFooter = addFooter(ctx, body);
@@ -310,7 +312,9 @@ export function ReportProgressTool(ctx: ToolContext) {
   return tool({
     name: "report_progress",
     description:
-      "Share progress on the associated GitHub issue/PR. The first call creates a comment; subsequent calls update it in place. Call this at the end of every run with a brief final summary (1-3 sentences) unless the mode guidance instructs otherwise. The current task list is automatically appended in a collapsible section — do not restate individual steps.",
+      "Share progress on the associated GitHub issue/PR. The first call creates a comment; subsequent calls update it in place. " +
+      'Example: `report_progress({ body: "Implemented the auth check and added tests." })`. ' +
+      "Call this at the end of every run with a brief final summary (1-3 sentences) unless the mode guidance instructs otherwise. The current task list is automatically appended in a collapsible section — do not restate individual steps.",
     parameters: ReportProgress,
     execute: execute(async (params) => {
       let body = params.body;
@@ -445,7 +449,9 @@ export function ReplyToReviewCommentTool(ctx: ToolContext) {
   return tool({
     name: "reply_to_review_comment",
     description:
-      "Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). Call exactly ONCE per parent comment you address in AddressReviews mode — duplicate calls with the same body are a no-op. Keep replies extremely brief (1 sentence max).",
+      "Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). " +
+      'Example: `reply_to_review_comment({ pull_number: 1234, comment_id: 567890, body: "Fixed by adding a null check." })`. ' +
+      "Call exactly ONCE per parent comment you address in AddressReviews mode — duplicate calls with the same body are a no-op. Keep replies extremely brief (1 sentence max).",
     parameters: ReplyToReviewComment,
     execute: execute(async ({ pull_number, comment_id, body }) => {
       const bodyWithFooter = addFooter(ctx, body);

mcp/commitInfo.ts

@@ -15,7 +15,8 @@ export function CommitInfoTool(ctx: ToolContext) {
     name: "get_commit_info",
     description:
       "Retrieve commit metadata and diff via GitHub API. Use this instead of git show for reviewing commits - " +
-      "it works with shallow clones and shows the actual changes in the commit. Returns diffPath pointing to formatted diff file.",
+      "it works with shallow clones and shows the actual changes in the commit. Returns diffPath pointing to formatted diff file. " +
+      'Example: `get_commit_info({ sha: "2a6ab5d" })`.',
     parameters: CommitInfo,
     execute: execute(async ({ sha }) => {
       const response = await ctx.octokit.rest.repos.getCommit({

mcp/geminiSanitizer.ts

@@ -176,13 +176,32 @@ export function sanitizeToolForGemini<T extends Tool<any, any>>(tool: T): T {
 }
 
 /**
- * true when the effective upstream model is served by google's generative
- * language API — directly (`google/*`), via opencode (`opencode/gemini-*`),
- * or via openrouter (`openrouter/google/gemini-*`). slug-substring match
- * works because every gemini route's model id contains "gemini".
+ * true when the effective upstream model is — or might become — google
+ * generative language API traffic. matches:
+ *   - direct `google/*`, opencode `opencode/gemini-*`, openrouter
+ *     `openrouter/google/gemini-*` (slug substring "gemini" wins).
+ *   - any unresolved specifier: `undefined`, `"auto"`, or a slug that
+ *     didn't map through the alias registry (no `provider/` prefix).
+ *     these flow through the agent's own auto-select, which may land
+ *     on gemini *after* the MCP server has already registered tools —
+ *     at which point sanitization is too late to apply. erring on the
+ *     side of sanitizing is safe: cases 1 + 2 are universally
+ *     compatible JSON-Schema normalizations (enum-only → typed string,
+ *     collapsible const-unions → string enum); case 3 is gemini-
+ *     specific but only fires on non-collapsible unions, which arktype
+ *     does not emit for our current tool schemas. see issue #676 for
+ *     the prod failure that motivated this widening.
  */
 export function isGeminiRouted(ctx: ToolContext): boolean {
   const effective = ctx.payload.proxyModel ?? ctx.resolvedModel ?? ctx.payload.model;
-  if (!effective) return false;
-  return effective.toLowerCase().includes("gemini");
+  if (!effective) return true;
+  const normalized = effective.toLowerCase();
+  if (normalized.includes("gemini")) return true;
+  // every concrete model resolved through the registry carries a
+  // `provider/` prefix (e.g. "anthropic/claude-opus-4-7"). anything
+  // without a slash is either the literal `"auto"` alias or an
+  // unrecognized slug that resolveModel logged a warning for — both
+  // route through the agent's late auto-select, which may pick gemini.
+  if (!normalized.includes("/")) return true;
+  return false;
 }

mcp/git.ts

@@ -218,10 +218,12 @@ export function PushBranchTool(ctx: ToolContext) {
     name: "push_branch",
     description:
       "Push the current branch to the remote repository. Omit branchName to push the current branch (recommended). " +
+      'Example: `push_branch({})` to push the current branch. Example: `push_branch({ branchName: "pr-1" })` to push a specific local branch. ' +
       "If specifying branchName, use the LOCAL branch name (e.g., 'pr-1'), not the remote branch name. " +
       "The correct remote and remote branch are determined automatically from branch config set by checkout_pr. " +
       "Requires a clean working tree. Runs the repository prepush hook (if configured) before the network push — hook failure means tests/lint or similar in that script failed, not necessarily a Pullfrog timeout. " +
-      "Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.",
+      "Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode. " +
+      "If the response reports a timeout, the underlying push may have actually succeeded — verify with `git log origin/<branch>` (or this tool with command 'log') before retrying, otherwise you'll push a duplicate.",
     parameters: PushBranch,
     execute: execute(async ({ branchName, force }) => {
       // permission check
@@ -451,7 +453,10 @@ export function GitTool(ctx: ToolContext) {
   return tool({
     name: "git",
     description:
-      "Run git commands. For push/fetch, use the dedicated MCP tools (push_branch, git_fetch). " +
+      "Run a git subcommand. `command` is a single subcommand; flags and positional args go in `args`. " +
+      'Example: `git({ command: "log", args: ["--oneline", "-n", "20"] })`. ' +
+      'Example: `git({ command: "diff", args: ["origin/main..HEAD"] })`. ' +
+      "For push/fetch, use the dedicated MCP tools (push_branch, git_fetch). " +
       "git pull is not available — use git_fetch then this tool with command 'merge'.",
     parameters: Git,
     execute: execute(async (params) => {
@@ -523,7 +528,9 @@ const DEEPEN_RETRY_DEPTH = 1000;
 export function GitFetchTool(ctx: ToolContext) {
   return tool({
     name: "git_fetch",
-    description: "Fetch refs from remote repository. Use this instead of git fetch directly.",
+    description:
+      "Fetch refs from remote repository. Use this instead of git fetch directly. " +
+      'Example: `git_fetch({ ref: "main" })`. With depth: `git_fetch({ ref: "pull/1234/head", depth: 1 })`.',
     parameters: GitFetch,
     execute: execute(async (params) => {
       rejectIfLeadingDash(params.ref, "ref");

mcp/issueComments.ts

@@ -10,7 +10,8 @@ export function GetIssueCommentsTool(ctx: ToolContext) {
   return tool({
     name: "get_issue_comments",
     description:
-      "Get all comments for a GitHub issue. Returns all comments including the issue body and all subsequent discussion comments.",
+      "Get all comments for a GitHub issue. Returns all comments including the issue body and all subsequent discussion comments. " +
+      "Example: `get_issue_comments({ issue_number: 1234 })`.",
     parameters: GetIssueComments,
     execute: execute(async ({ issue_number }) => {
       // set issue context

mcp/issueInfo.ts

@@ -9,7 +9,9 @@ export const IssueInfo = type({
 export function IssueInfoTool(ctx: ToolContext) {
   return tool({
     name: "get_issue",
-    description: "Retrieve GitHub issue information by issue number",
+    description:
+      "Retrieve GitHub issue information by issue number. " +
+      "Example: `get_issue({ issue_number: 1234 })`.",
     parameters: IssueInfo,
     execute: execute(async ({ issue_number }) => {
       const issue = await ctx.octokit.rest.issues.get({

mcp/prInfo.ts

@@ -30,7 +30,9 @@ export function PullRequestInfoTool(ctx: ToolContext) {
   return tool({
     name: "get_pull_request",
     description:
-      "Retrieve PR metadata (title, body, state, branches, author, labels, linked issues). To checkout a PR branch locally, use checkout_pr instead.",
+      "Retrieve PR metadata (title, body, state, branches, author, labels, linked issues). " +
+      "Example: `get_pull_request({ pull_number: 1234 })`. " +
+      "To checkout a PR branch locally, use checkout_pr instead.",
     parameters: PullRequestInfo,
     execute: execute(async ({ pull_number }) => {
       // fetch REST and GraphQL in parallel

mcp/review.ts

@@ -320,14 +320,16 @@ export const CreatePullRequestReview = type({
     )
     .optional(),
   commit_id: type.string
-    .describe("Optional SHA of the commit being reviewed. Defaults to latest.")
+    .describe(
+      "Optional SHA of the commit being reviewed. Defaults to latest. Must be the FULL 40-character SHA — abbreviated SHAs are rejected by GitHub with `422 Unprocessable Entity`. The PR-synchronize event payload's `head_sha` is already full-length."
+    )
     .optional(),
   comments: type({
     path: type.string.describe(
       "The file path to comment on (relative to repo root). Must be a file that appears in the PR diff."
     ),
     line: type.number.describe(
-      "Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format."
+      "Line number to comment on. For multi-line ranges, this is the end line. Use NEW column from diff format. Must sit inside a `@@` hunk in the PR diff — anchors on context-only or untouched lines are dropped silently (the rest of the review still posts; dropped entries are reported under `droppedComments` in the response)."
     ),
     side: type
       .enumerated("LEFT", "RIGHT")
@@ -345,7 +347,7 @@ export const CreatePullRequestReview = type({
       .optional(),
     start_line: type.number
       .describe(
-        "Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces."
+        "Start line for multi-line comment ranges. Omit for single-line comments. The range [start_line, line] defines which lines a suggestion replaces. Both `start_line` and `line` must sit inside the same `@@` hunk — a `start_line` outside the hunk causes the whole comment to be dropped even when `line` is valid. If you need to comment on context just above/below a hunk, shrink the range to a single line that is provably modified."
       )
       .optional(),
   })
@@ -361,6 +363,7 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
     name: "create_pull_request_review",
     description:
       "Submit a review for an existing pull request. " +
+      'Example: `create_pull_request_review({ pull_number: 1234, body: "LGTM", approved: true, comments: [{ path: "src/api.ts", line: 42, body: "nit: rename" }] })`. ' +
       "Each call creates a permanent, visible review on the PR — NEVER submit test or diagnostic reviews. " +
       "Reviews with no body AND no comments are silently skipped (nothing to post). " +
       "IMPORTANT: 95%+ of feedback should be in 'comments' array with file paths and line numbers. " +
@@ -709,7 +712,9 @@ function runDiffCoveragePreflight(params: { ctx: ToolContext }): void {
     .join("\n");
   throw new Error(
     `diff coverage pre-flight: some TOC regions were not read before review submission. ` +
-      `this is a one-time nudge — optionally read the ranges below from ${coverageState.diffPath}, then call create_pull_request_review again with the same arguments. ` +
+      `this is a one-time nudge — read the ranges below from ${coverageState.diffPath} on a best-effort basis, then call create_pull_request_review again. ` +
+      `you are NOT obligated to read generated artifacts (lockfiles like pnpm-lock.yaml / package-lock.json / yarn.lock / Cargo.lock; codegen output like *.gen.*, *.pb.go, *.generated.*; snapshot/fixture dirs like __snapshots__/; migration metadata like drizzle/meta/, prisma migration SQL). ` +
+      `if every unread region is generated, retry immediately without reading. ` +
       `this pre-flight will not block again in this review session.\n\n` +
       `unread TOC regions:\n${unreadText}\n\n` +
       `${coverageState.lastBreakdown}`

mcp/reviewComments.ts

@@ -602,6 +602,7 @@ export function GetReviewCommentsTool(ctx: ToolContext) {
     name: "get_review_comments",
     description:
       "Get review comments for a pull request review with full thread context. " +
+      "Example: `get_review_comments({ pull_number: 1234, review_id: 567890 })`. " +
       "Automatically filters to approved comments when applicable. " +
       "Returns a TOC and commentsPath pointing to a markdown file with full comment details.",
     parameters: GetReviewComments,
@@ -673,7 +674,8 @@ export function ListPullRequestReviewsTool(ctx: ToolContext) {
   return tool({
     name: "list_pull_request_reviews",
     description:
-      "List all reviews for a pull request. Returns all reviews including approvals, request changes, and comments.",
+      "List all reviews for a pull request. Returns all reviews including approvals, request changes, and comments. " +
+      "Example: `list_pull_request_reviews({ pull_number: 1234 })`.",
     parameters: ListPullRequestReviews,
     execute: execute(async (params) => {
       const reviews = await ctx.octokit.paginate(ctx.octokit.rest.pulls.listReviews, {

mcp/selectMode.ts

@@ -121,7 +121,8 @@ export function SelectModeTool(ctx: ToolContext) {
   return tool({
     name: "select_mode",
     description:
-      "Select a mode and receive step-by-step guidance on how to handle the task. Call this to understand the best workflow for the current mode.",
+      "Select a mode and receive step-by-step guidance on how to handle the task. Call this to understand the best workflow for the current mode. " +
+      'Example: `select_mode({ mode: "Review" })` or `select_mode({ mode: "Plan", issue_number: 1234 })`.',
     parameters: SelectModeParams,
     execute: execute(async (params) => {
       if (ctx.toolState.selectedMode) {

mcp/shell.ts

@@ -15,7 +15,9 @@ import { execute, tool } from "./shared.ts";
 export const ShellParams = type({
   command: "string",
   description: "string",
-  "timeout?": "number",
+  "timeout?": type.number.describe(
+    "Timeout in MILLISECONDS (not seconds). Default 30000 (30s), max 120000 (2m). e.g. timeout: 180000 for 3 minutes; timeout: 180 means 180ms and will kill the process almost immediately."
+  ),
   "working_directory?": "string",
   "background?": "boolean",
 });
@@ -187,6 +189,8 @@ export function ShellTool(ctx: ToolContext) {
     name: "shell",
     description: `Execute shell commands securely. Environment is filtered to remove API keys and secrets.
 
+Example: \`shell({ command: "pnpm test", description: "run the test suite" })\`.
+
 Use this tool to:
 - Run shell commands (ls, cat, grep, find, etc.)
 - Execute build tools (npm, pnpm, cargo, make, etc.)

models.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
 import {
   getModelEnvVars,
   getModelProvider,
+  isBedrockAnthropicId,
   modelAliases,
   parseModel,
   providers,
@@ -55,13 +56,13 @@ describe("getModelEnvVars", () => {
 
   it("returns empty env vars for free opencode models", () => {
     expect(getModelEnvVars("opencode/big-pickle")).toEqual([]);
-    expect(getModelEnvVars("opencode/gpt-5-nano")).toEqual([]);
     expect(getModelEnvVars("opencode/mimo-v2-pro-free")).toEqual([]);
     expect(getModelEnvVars("opencode/minimax-m2.5-free")).toEqual([]);
   });
 
   it("still requires OPENCODE_API_KEY for non-free opencode models", () => {
     expect(getModelEnvVars("opencode/claude-opus")).toEqual(["OPENCODE_API_KEY"]);
+    expect(getModelEnvVars("opencode/gpt-5-nano")).toEqual(["OPENCODE_API_KEY"]);
   });
 });
 
@@ -175,7 +176,12 @@ describe("modelAliases registry", () => {
 
   it("has exactly one preferred model per provider", () => {
     for (const providerKey of Object.keys(providers)) {
-      const preferred = modelAliases.filter((a) => a.provider === providerKey && a.preferred);
+      // routing-only providers (bedrock) deliberately have no preferred
+      // model — the user picks the actual model via a per-run env var, so
+      // there's no "preferred default" to surface to auto-select.
+      const aliases = modelAliases.filter((a) => a.provider === providerKey);
+      if (aliases.every((a) => a.routing)) continue;
+      const preferred = aliases.filter((a) => a.preferred);
       expect(preferred.length, `${providerKey} should have exactly 1 preferred model`).toBe(1);
     }
   });
@@ -190,6 +196,10 @@ describe("modelAliases registry", () => {
 
   it("all resolve values follow provider/model format", () => {
     for (const alias of modelAliases) {
+      // routing slugs use a sentinel `resolve` (e.g. "bedrock") that's never
+      // passed to a CLI directly — the harness reads a separate env var to
+      // get the real model ID. format check doesn't apply.
+      if (alias.routing) continue;
       expect(alias.resolve).toContain("/");
     }
   });
@@ -200,6 +210,54 @@ describe("modelAliases registry", () => {
   });
 });
 
+describe("isBedrockAnthropicId", () => {
+  it("matches geo-prefixed Anthropic foundation IDs", () => {
+    expect(isBedrockAnthropicId("us.anthropic.claude-opus-4-7")).toBe(true);
+    expect(isBedrockAnthropicId("eu.anthropic.claude-sonnet-4-6")).toBe(true);
+    expect(isBedrockAnthropicId("global.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe(true);
+  });
+
+  it("matches in-region Anthropic foundation IDs", () => {
+    expect(isBedrockAnthropicId("anthropic.claude-opus-4-7")).toBe(true);
+  });
+
+  it("rejects non-Anthropic foundation IDs", () => {
+    expect(isBedrockAnthropicId("amazon.nova-pro-v1:0")).toBe(false);
+    expect(isBedrockAnthropicId("us.meta.llama4-scout-17b-instruct-v1:0")).toBe(false);
+    expect(isBedrockAnthropicId("deepseek.v3.2")).toBe(false);
+  });
+
+  // regression: PR #720 review caught that a substring-only match was
+  // fragile for inference-profile ARNs (which BEDROCK_MODEL_ID accepts per
+  // the AWS docs). ARN names are user-chosen — both directions of the
+  // heuristic could break depending on what name the operator picked.
+  // We anchor on a discrete dot-segment match (case-insensitive) instead.
+  it("ignores 'anthropic' substrings inside non-segment text", () => {
+    // ARN whose user-chosen profile name happens to contain "anthropic" as
+    // part of a longer word — would route to claude-code under naive
+    // includes("anthropic") even though the backing model is unknown.
+    expect(
+      isBedrockAnthropicId(
+        "arn:aws:bedrock:us-east-2:123456789012:application-inference-profile/my-anthropicish-profile"
+      )
+    ).toBe(false);
+  });
+
+  it("matches when 'anthropic' appears as its own dot-segment in ARN", () => {
+    // ARN whose profile name embeds the foundation segment correctly —
+    // operator chose to surface the backing model in the name.
+    expect(
+      isBedrockAnthropicId(
+        "arn:aws:bedrock:us-east-2:123456789012:application-inference-profile/anthropic.claude-opus-4-7"
+      )
+    ).toBe(true);
+  });
+
+  it("is case-insensitive", () => {
+    expect(isBedrockAnthropicId("US.ANTHROPIC.CLAUDE-OPUS-4-7")).toBe(true);
+  });
+});
+
 describe("providers registry", () => {
   it("every provider has envVars", () => {
     for (const [key, config] of Object.entries(providers)) {

models.ts

@@ -7,6 +7,22 @@
 
 // ── types ──────────────────────────────────────────────────────────────────────
 
+/**
+ * routing discriminant for entries whose `resolve` is dynamic — looked up
+ * from a separate env var at run time rather than fixed in the catalog.
+ *
+ * `"bedrock"` means the actual model ID comes from `BEDROCK_MODEL_ID`
+ * (an AWS-canonical Bedrock model ID like `us.anthropic.claude-opus-4-7`
+ * or `amazon.nova-pro-v1:0`). enterprise Bedrock customers self-select for
+ * version control — silent alias bumps would break compliance review,
+ * model-access enrollment, and provisioned-throughput contracts. so the
+ * single `bedrock/byok` entry is a routing slug, not a model alias: the
+ * harness reads `BEDROCK_MODEL_ID` and routes to claude-code (when the ID
+ * contains "anthropic") or opencode (everything else, with an
+ * `amazon-bedrock/` prefix).
+ */
+export type ModelRouting = "bedrock";
+
 export interface ModelAlias {
   /** stable alias stored in DB, e.g. "anthropic/claude-opus" */
   slug: string;
@@ -14,9 +30,9 @@ export interface ModelAlias {
   provider: string;
   /** human-readable name shown in dropdowns */
   displayName: string;
-  /** concrete models.dev specifier, e.g. "anthropic/claude-opus-4-6" */
+  /** concrete models.dev specifier, e.g. "anthropic/claude-opus-4-6". sentinel for routing entries — never passed to a CLI directly. */
   resolve: string;
-  /** full models.dev specifier for the OpenRouter equivalent (undefined for free models) */
+  /** full models.dev specifier for the OpenRouter equivalent (undefined for free models and routing entries) */
   openRouterResolve: string | undefined;
   /** top-tier pick for this provider — preferred during auto-select */
   preferred: boolean;
@@ -24,6 +40,15 @@ export interface ModelAlias {
   isFree: boolean;
   /** slug of a replacement model — presence implies this model is deprecated */
   fallback: string | undefined;
+  /** dynamic-resolution discriminant — see ModelRouting docs */
+  routing: ModelRouting | undefined;
+  /** alias key (within same provider) of the cheaper sibling reviewfrog should
+   * use as its lens-fanout subagent. e.g. claude-opus → "claude-sonnet". */
+  subagentModel: string | undefined;
+  /** hide from selectable lists (UI dropdowns, CLI pickers). does NOT affect
+   * resolution — for that use `fallback`. used for internal-only tier targets
+   * (e.g. gpt-5.4 as a subagent target without exposing it to users). */
+  hidden: boolean;
 }
 
 interface ModelDef {
@@ -37,6 +62,13 @@ interface ModelDef {
   isFree?: boolean;
   /** slug of a replacement model — presence implies this model is deprecated */
   fallback?: string;
+  /** dynamic-resolution discriminant — see ModelRouting docs */
+  routing?: ModelRouting;
+  /** alias key (within same provider) of the cheaper sibling reviewfrog should
+   * use as its lens-fanout subagent (e.g. claude-opus → "claude-sonnet"). */
+  subagentModel?: string;
+  /** hide from selectable lists. does NOT affect resolution; for that use `fallback`. */
+  hidden?: boolean;
 }
 
 export interface ProviderConfig {
@@ -61,6 +93,7 @@ export const providers = {
         resolve: "anthropic/claude-opus-4-7",
         openRouterResolve: "openrouter/anthropic/claude-opus-4.7",
         preferred: true,
+        subagentModel: "claude-sonnet",
       },
       "claude-sonnet": {
         displayName: "Claude Sonnet",
@@ -83,11 +116,22 @@ export const providers = {
         resolve: "openai/gpt-5.5",
         openRouterResolve: "openrouter/openai/gpt-5.5",
         preferred: true,
+        subagentModel: "gpt-5.4",
       },
       "gpt-pro": {
         displayName: "GPT Pro",
         resolve: "openai/gpt-5.5-pro",
         openRouterResolve: "openrouter/openai/gpt-5.5-pro",
+        subagentModel: "gpt",
+      },
+      // hidden subagent target — `gpt` lenses run against this. surfacing
+      // it in the picker would just confuse users (it's the prior-flagship,
+      // and they already have `gpt` and `gpt-mini` to choose from).
+      "gpt-5.4": {
+        displayName: "GPT 5.4",
+        resolve: "openai/gpt-5.4",
+        openRouterResolve: "openrouter/openai/gpt-5.4",
+        hidden: true,
       },
       "gpt-mini": {
         displayName: "GPT Mini",
@@ -126,6 +170,7 @@ export const providers = {
         resolve: "google/gemini-3.1-pro-preview",
         openRouterResolve: "openrouter/google/gemini-3.1-pro-preview",
         preferred: true,
+        subagentModel: "gemini-flash",
       },
       "gemini-flash": {
         displayName: "Gemini Flash",
@@ -214,6 +259,7 @@ export const providers = {
         displayName: "Claude Opus",
         resolve: "opencode/claude-opus-4-7",
         openRouterResolve: "openrouter/anthropic/claude-opus-4.7",
+        subagentModel: "claude-sonnet",
       },
       "claude-sonnet": {
         displayName: "Claude Sonnet",
@@ -229,11 +275,20 @@ export const providers = {
         displayName: "GPT",
         resolve: "opencode/gpt-5.5",
         openRouterResolve: "openrouter/openai/gpt-5.5",
+        subagentModel: "gpt-5.4",
       },
       "gpt-pro": {
         displayName: "GPT Pro",
         resolve: "opencode/gpt-5.5-pro",
         openRouterResolve: "openrouter/openai/gpt-5.5-pro",
+        subagentModel: "gpt",
+      },
+      // hidden subagent target — see openai provider above for context.
+      "gpt-5.4": {
+        displayName: "GPT 5.4",
+        resolve: "opencode/gpt-5.4",
+        openRouterResolve: "openrouter/openai/gpt-5.4",
+        hidden: true,
       },
       "gpt-mini": {
         displayName: "GPT Mini",
@@ -257,6 +312,7 @@ export const providers = {
         displayName: "Gemini Pro",
         resolve: "opencode/gemini-3.1-pro",
         openRouterResolve: "openrouter/google/gemini-3.1-pro-preview",
+        subagentModel: "gemini-flash",
       },
       "gemini-flash": {
         displayName: "Gemini Flash",
@@ -271,8 +327,7 @@ export const providers = {
       "gpt-5-nano": {
         displayName: "GPT Nano",
         resolve: "opencode/gpt-5-nano",
-        envVars: [],
-        isFree: true,
+        openRouterResolve: "openrouter/openai/gpt-5-nano",
       },
       "mimo-v2-pro-free": {
         displayName: "MiMo V2 Pro",
@@ -289,6 +344,20 @@ export const providers = {
       },
     },
   }),
+  bedrock: provider({
+    displayName: "Amazon Bedrock",
+    envVars: ["AWS_BEARER_TOKEN_BEDROCK", "AWS_REGION", "BEDROCK_MODEL_ID"],
+    models: {
+      // single routing entry — the actual Bedrock model ID is read from
+      // BEDROCK_MODEL_ID at run time. see ModelRouting docs for why we
+      // don't catalog individual Bedrock models.
+      byok: {
+        displayName: "Amazon Bedrock",
+        resolve: "bedrock",
+        routing: "bedrock",
+      },
+    },
+  }),
   openrouter: provider({
     displayName: "OpenRouter",
     envVars: ["OPENROUTER_API_KEY"],
@@ -298,6 +367,7 @@ export const providers = {
         resolve: "openrouter/anthropic/claude-opus-4.7",
         openRouterResolve: "openrouter/anthropic/claude-opus-4.7",
         preferred: true,
+        subagentModel: "claude-sonnet",
       },
       "claude-sonnet": {
         displayName: "Claude Sonnet",
@@ -313,11 +383,20 @@ export const providers = {
         displayName: "GPT",
         resolve: "openrouter/openai/gpt-5.5",
         openRouterResolve: "openrouter/openai/gpt-5.5",
+        subagentModel: "gpt-5.4",
       },
       "gpt-pro": {
         displayName: "GPT Pro",
         resolve: "openrouter/openai/gpt-5.5-pro",
         openRouterResolve: "openrouter/openai/gpt-5.5-pro",
+        subagentModel: "gpt",
+      },
+      // hidden subagent target — see openai provider above for context.
+      "gpt-5.4": {
+        displayName: "GPT 5.4",
+        resolve: "openrouter/openai/gpt-5.4",
+        openRouterResolve: "openrouter/openai/gpt-5.4",
+        hidden: true,
       },
       "gpt-mini": {
         displayName: "GPT Mini",
@@ -346,6 +425,7 @@ export const providers = {
         displayName: "Gemini Pro",
         resolve: "openrouter/google/gemini-3.1-pro-preview",
         openRouterResolve: "openrouter/google/gemini-3.1-pro-preview",
+        subagentModel: "gemini-flash",
       },
       "gemini-flash": {
         displayName: "Gemini Flash",
@@ -433,6 +513,12 @@ export const modelAliases: ModelAlias[] = Object.entries(providers).flatMap(
       preferred: def.preferred ?? false,
       isFree: def.isFree ?? false,
       fallback: def.fallback,
+      routing: def.routing,
+      // subagentModel is stored as an alias key local to the provider; expand
+      // here to a fully-qualified slug so callers can look up the target alias
+      // directly without re-deriving the provider.
+      subagentModel: def.subagentModel ? `${providerKey}/${def.subagentModel}` : undefined,
+      hidden: def.hidden ?? false,
     }))
 );
 
@@ -452,7 +538,7 @@ const MAX_FALLBACK_DEPTH = 10;
  * use this in UI display sites (dropdown trigger labels, PR-comment footers,
  * etc.) so a deprecated stored slug renders as the model the user actually
  * runs against — not the historical name. selectable lists should still hide
- * deprecated aliases by filtering on `!a.fallback`.
+ * deprecated and internal-only aliases by filtering on `!a.fallback && !a.hidden`.
  */
 export function resolveDisplayAlias(slug: string): ModelAlias | undefined {
   let current = slug;
@@ -486,3 +572,40 @@ export function resolveCliModel(slug: string): string | undefined {
 export function resolveOpenRouterModel(slug: string): string | undefined {
   return resolveDisplayAlias(slug)?.openRouterResolve;
 }
+
+// ── bedrock routing ────────────────────────────────────────────────────────────
+
+/** env var that supplies the Bedrock model ID for the `bedrock/byok` slug. */
+export const BEDROCK_MODEL_ID_ENV = "BEDROCK_MODEL_ID";
+
+/**
+ * the Bedrock model ID passed to claude-code or opencode is whatever the
+ * user set in `BEDROCK_MODEL_ID` — Pullfrog never resolves or upgrades it.
+ * we route by checking whether the ID names an Anthropic model: claude-code
+ * handles Anthropic-on-Bedrock natively (with `CLAUDE_CODE_USE_BEDROCK=1`),
+ * everything else goes through opencode's `amazon-bedrock` provider.
+ *
+ * AWS Bedrock IDs come in two shapes:
+ *   - dotted foundation IDs: `us.anthropic.claude-opus-4-7`,
+ *     `anthropic.claude-haiku-4-5-20251001-v1:0`, `amazon.nova-pro-v1:0`,
+ *     `meta.llama4-scout-17b-instruct-v1:0`. AWS-published, lowercase, the
+ *     foundation provider always appears as a discrete dot-segment.
+ *   - inference-profile ARNs: `arn:aws:bedrock:us-east-2:<acct>:application-inference-profile/<user-name>`.
+ *     `<user-name>` is operator-chosen, so a naive substring check is fragile
+ *     in both directions (Anthropic profile named without "anthropic" → routes
+ *     to opencode and misses CLAUDE_CODE_USE_BEDROCK; non-Anthropic profile
+ *     whose name happens to contain "anthropic" → routes to claude-code).
+ *
+ * we anchor on a discrete dot-segment match (case-insensitive). this catches
+ * every published foundation ID and is conservative for ARN-form IDs: ARN
+ * names that don't include "anthropic" as their own dot-segment route to
+ * opencode by default. operators using ARN-form IDs whose backing model is
+ * Anthropic should set `PULLFROG_AGENT=claude` to force the right route, or
+ * include the foundation segment in the profile name.
+ */
+export function isBedrockAnthropicId(bedrockModelId: string): boolean {
+  // split on `.`, `/`, and `:` so the check works for both dotted foundation
+  // IDs (anthropic.* / us.anthropic.*) and ARN-form IDs (where the relevant
+  // foundation segment sits between `/` and `.` inside the resource name).
+  return bedrockModelId.toLowerCase().split(/[./:]/).includes("anthropic");
+}

modes.ts

@@ -155,18 +155,24 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
    - resolve addressed threads via \`${t("resolve_review_thread")}\`
    - call \`${t("report_progress")}\` with a brief summary (or the exact push error if push failed)`,
     },
-    // Review and IncrementalReview use the multi-lens orchestrator pattern
-    // (canonical source: .claude/commands/anneal.md). The orchestrator does
-    // triage → parallel read-only subagent fan-out → aggregate → draft comments
-    // → submit. For someone else's PR, parallel lenses (correctness, security,
-    // research-validated claims, user-journey, etc.) provide breadth across
-    // angles that a single subagent can't carry coherently. Build mode keeps
-    // a single fresh-eyes subagent (different problem shape — orchestrator
-    // wrote the code and bias-mitigation comes from delegating to one
-    // subagent that doesn't share the implementation context).
-    // Deliberate omission vs canonical /anneal: severity categorization in the
-    // final message (the review body has its own CAUTION/IMPORTANT framing
-    // instead of a severity table).
+    // Review and IncrementalReview use a 0-or-2+ lens pattern. The default is
+    // 0 lenses (orchestrator handles the review solo). Multi-lens (2+
+    // reviewfrog subagents in parallel) only fires for substantive PRs or
+    // high-stakes-subsystem touches — and when it fires, ALL lenses must
+    // dispatch in a single assistant turn or the parallelism win disappears.
+    // We never dispatch exactly one lens: a single lens is just a worse,
+    // slower version of doing the work yourself.
+    //
+    // Build mode self-review is a different problem shape: the orchestrator
+    // wrote the code, so bias-mitigation comes from delegating to one
+    // fresh-eyes subagent that doesn't share the implementation context. A
+    // single subagent there is appropriate; the 0-or-2+ rule applies only to
+    // the Review/IncrementalReview lens fan-out where independence between
+    // perspectives is what's being purchased.
+    //
+    // Deliberate omission vs canonical /anneal: severity categorization in
+    // the final message (the review body has its own CAUTION/IMPORTANT
+    // framing instead of a severity table).
     {
       name: "Review",
       description:
@@ -177,9 +183,9 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
 
 2. **checkout**: call \`${t("checkout_pr")}\` — this returns PR metadata and a \`diffPath\`. read the diff TOC end-to-end and treat its file line ranges as your coverage checklist.
 
-3. **triage**: orient yourself on the PR — identify *what kind of thing this is* (domain it touches, seams it crosses, external contracts it depends on, user-facing surfaces it changes). orientation only — defer specific defect-hunting to the subagents; pre-reviewing biases the lenses you pick. use \`${t("get_pull_request")}\` and other read-only GitHub tools for additional context if needed.
+3. **triage**: orient yourself on the PR — identify *what kind of thing this is* (domain it touches, seams it crosses, external contracts it depends on, user-facing surfaces it changes). pull as much context as you need to render a confident, well-grounded review: read related files, grep for callers of changed symbols, check tests that exercise the touched paths, fetch related GitHub state. **you are the synthesizer** — never delegate understanding to subagents.
 
-   if the PR is **genuinely trivial**, skip steps 4–5 entirely and submit a \`No new issues found.\` review per step 6. there's no value in dispatching even one lens for a typo.
+   if the PR is **genuinely trivial**, skip the fan-out entirely and submit a \`No new issues found.\` review per step 7.
 
    "Genuinely trivial" (skip):
    - single-word doc typo, whitespace/format-only, comment-only across any number of files
@@ -198,23 +204,25 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
    - any "typo fix" in user-facing copy that changes meaning ("approved" → "denied")
    - mixed diffs where a semantic 1-liner is buried in whitespace/formatting changes
 
-   When unsure, treat as non-trivial. The cost of one extra subagent is cents; the cost of a missed billing/auth/data bug is much more.
+4. **lens decision — 0 or 2+, NEVER 1**.
 
-   otherwise pick lenses by where the PR concentrates risk — **there's no fixed count**. lens count is judgment, not a formula. concrete shapes to anchor against:
+   The default is **0 lenses**: handle the review yourself end-to-end. Most PRs land here.
 
-   - **1 lens** — pure refactor / mechanical rename across many files (impact); new test file with no source change (test-integrity); small isolated bug fix (correctness); doc-only PR with non-trivial technical content (research-validated or holistic)
-   - **2–3 lenses (most PRs land here)** — new CRUD endpoint (correctness + security + test-integrity); new UI flow (user-journey + correctness); a single bug fix in a non-critical subsystem (correctness + test-integrity); design doc covering one domain (research-validated + correctness or holistic)
-   - **4–5 lenses (high-stakes subsystem touches)** — any billing/payments change (billing-subsystem + correctness + security + operational-readiness); new auth flow (auth-subsystem + correctness + security + test-integrity); schema migration (schema-migration-subsystem + correctness + operational-readiness + impact); cross-subsystem PR that touches billing AND auth AND schema (one subsystem lens per domain + correctness)
-   - **6+ lenses** — almost always a smell; you're either covering overlapping ground or this PR should have been split. push back via the review body rather than expanding lens count.
+   Dispatch **2+ \`${REVIEWER_AGENT_NAME}\` lenses in parallel** ONLY when ALL of the following are true:
+   - the PR is substantive (>5 files changed AND >200 net lines), OR touches a high-stakes subsystem (auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling)
+   - you can name 2+ distinct concrete failure modes that warrant independent lenses (one lens per failure mode; orthogonal, not overlapping)
+   - parallel-orchestrated independent perspectives meaningfully outperform what you'd find solo
 
-   lenses come in two flavors, and you can mix them:
+   **NEVER dispatch exactly one lens.** A single lens is just a more expensive version of doing the work yourself with a worse model — it adds wall time and a context-handoff for no orthogonality benefit. Either you have at least two genuinely independent failure-mode hypotheses (dispatch all in one turn), or you don't (do the review yourself).
+
+   When you do go multi-lens, lens framings come in two flavors:
    - **themed lenses** — a perspective applied across the whole diff (correctness, security, user-journey, performance, etc.).
-   - **subsystem lenses** — a domain-scoped frame for high-stakes subsystems the PR touches (e.g. "the auth lens", "the billing lens", "the schema-migration lens"). a subsystem lens is "review the PR specifically for what could go wrong in this subsystem" and naturally combines theme + scope. **for high-stakes domains, lead with the subsystem lens rather than the generic themed equivalent** — "billing-subsystem" outperforms "correctness on billing code" because the framing primes the subagent to remember domain-specific failure modes (double-charges, refund races, currency rounding, dispute flows) the generic lens misses.
+   - **subsystem lenses** — a domain-scoped frame for high-stakes subsystems the PR touches (e.g. "the auth lens", "the billing lens", "the schema-migration lens"). **for high-stakes domains, lead with the subsystem lens rather than the generic themed equivalent** — "billing-subsystem" outperforms "correctness on billing code" because the framing primes the subagent to remember domain-specific failure modes (double-charges, refund races, currency rounding, dispute flows) the generic lens misses.
 
    starter menu (combine, omit, or invent your own):
    - **correctness & invariants** — bugs, races, error handling, edge cases, state-machine boundaries
-   - **impact** — when the PR removes features, deletes exports, renames identifiers, or changes architectural patterns: stale references in code, tests, docs (\`docs/\`, \`wiki/\`), comments, configs, UI
-   - **research-validated assumptions** — third-party API contracts, SDK semantics, framework directives, version-gated behavior. the subagent must verify load-bearing claims via web search and quote source URLs.
+   - **impact** — stale references in code/tests/docs/configs/UI after rename/remove
+   - **research-validated assumptions** — third-party API contracts, SDK semantics, framework directives, version-gated behavior. **only pick when the PR's correctness depends on the contract behaving a specific way** — not when the API is merely used. The bar is "if the third-party contract differs from what the diff assumes, the PR is incorrect." When dispatched, the subagent must verify load-bearing claims via web search and quote source URLs.
    - **security** — new endpoints, authZ, input validation, secrets handling, replay/CSRF/injection, cross-tenant isolation
    - **user-journey** — UX-touching flows: walk through happy path and failure modes as a user
    - **operational readiness** — observability, alerting, migrations (forward + rollback), feature flags, on-call burden
@@ -224,26 +232,36 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
    - **holistic** — does the PR make sense as a whole? symmetric flows (delete for every create, rollback for every migration)?
    - **subsystem lenses** (invent as the PR demands) — auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling, etc.
 
-4. **fan out**: dispatch one \`${REVIEWER_AGENT_NAME}\` subagent per lens — its baked-in system prompt enforces the non-mutative + non-recursive contract (read-only file/search/web tools and read-only MCP queries; no writes, shell side effects, state-changing MCP calls, or nested subagent dispatch). when picking 2+ lenses, dispatch them in a **single assistant turn with multiple parallel subagent calls**; issuing one and awaiting reply before the next collapses the fan-out into a serial review. if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body — do not skip step 4 entirely on a single subagent failure. each subagent gets:
+   The only subagent type is \`${REVIEWER_AGENT_NAME}\` — used for lens judgment work ("is this safe / correct / well-tested?"), runs on a mid-tier model.
+
+5. **fan out (only if step 4 said 2+ lenses)**: dispatch every \`${REVIEWER_AGENT_NAME}\` subagent for this run **IN A SINGLE ASSISTANT TURN, AS MULTIPLE PARALLEL TASK TOOL_USE BLOCKS IN ONE MESSAGE.**
+
+   ⚠️  CRITICAL — PARALLELISM IS THE ONLY REASON LENSES EXIST. ⚠️
+   The default tool-call behavior of Claude Code (and most agent runtimes) is **serial dispatch**: emit one Task call, await result, emit next, await, etc. This collapses your fan-out into a sequential review where each lens adds N × (orchestrator-think-time + lens-execution-time) to wall time. **YOU MUST OVERRIDE THIS DEFAULT.** Emit ALL of your Task tool_use blocks in the SAME assistant message, BEFORE you read ANY result from ANY of them. If you find yourself emitting one Task call, then thinking about the result, then emitting another — STOP and re-issue them all together. The whole point of going multi-lens is the wall-clock speedup from parallel execution; serial dispatch defeats it entirely.
+
+   ✅ Right pattern: one assistant turn with N Task tool_use blocks → wait → N results arrive together → aggregate.
+   ❌ Wrong pattern: turn 1 = Task(lens A) → turn 2 (after A's result) = Task(lens B) → turn 3 (after B's result) = Task(lens C). This is the failure mode. Do not do this.
+
+   You can also include your own \`read\` / \`grep\` / \`webfetch\` calls in the SAME turn as the parallel \`${REVIEWER_AGENT_NAME}\` dispatches — concurrent context-pulling on the orchestrator side runs in parallel with the lens fan-out and costs zero extra wall time.
+
+   if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body — do not skip the fan-out entirely on a single subagent failure. each subagent gets:
    - the diff path / target — reading the diff and the codebase is its job
    - **only one lens** — never a multi-section "review for X, Y, and Z" prompt
    - **a Task \`description\` set to the lens name** (e.g. \`"security"\`, \`"correctness"\`, \`"billing-subsystem"\`) — the harness reads this field to label the subagent's log lines so parallel runs can be told apart in CI output. without it, every subagent shows up as \`subagent#N\`.
-   - the read-only contract restated in your dispatch instructions so the rule is present twice (the subagent's system prompt also enforces it). The test: would this call still be a no-op if reverted? If not (PR comments, branch pushes, issue updates, set_output, label changes, dependency installs, etc.), don't make it.
    - if the lens touches external contracts, instruct the subagent to verify load-bearing claims via web search rather than trust training data, and to quote source URLs in its reasoning. action runs are non-interactive — there's no human in the loop to catch "I'm pretty sure Stripe does X."
    - ask the subagent to report findings with file paths and NEW line numbers from the diff so you can anchor inline comments without re-reading the entire diff.
 
    delegation discipline:
-   - do NOT lens-review the diff yourself in parallel with the subagents (your job is dispatch + comment-drafting; doing the lens work yourself reintroduces the bias the fan-out avoids)
    - do NOT summarize the PR for them (biases toward a validation frame)
    - do NOT hand them a curated reading list (let them discover scope)
    - do NOT pre-shape their output with a finding schema
    - do NOT mention the other lenses (independence is the point — overlapping findings are a strong signal)
 
-5. **aggregate & draft**: merge findings; de-dup overlaps (two lenses catching the same issue = higher-confidence signal); trace each finding yourself before accepting it. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the PR (heuristic: if the finding's root cause lives in lines this PR added or modified, it's in scope; otherwise drop unless the PR plausibly introduced or amplified the regression), and anything not actionable. also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or worse, degrades elegance to nominally improve correctness) makes the codebase worse, not better.
+6. **aggregate & draft**: when the fan-out lands, merge findings; de-dup overlaps (two lenses catching the same issue = higher-confidence signal); trace each finding yourself before accepting it. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the PR (heuristic: if the finding's root cause lives in lines this PR added or modified, it's in scope; otherwise drop unless the PR plausibly introduced or amplified the regression), and anything not actionable. also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or worse, degrades elegance to nominally improve correctness) makes the codebase worse, not better.
 
    for surviving findings, draft inline comments with NEW line numbers from the diff. every comment must be actionable, 2-3 sentences max. use GitHub permalink format for code references. for impact-analysis findings (stale references after rename/remove), report them in the review body ordered by severity (runtime breakage > incorrect docs > stale comments) rather than as inline comments unless they're anchored to a specific line.
 
-6. **submit**: ALWAYS submit exactly one review via \`${t("create_pull_request_review")}\`. Do NOT call \`report_progress\` — the review is the final record and the progress comment will be cleaned up automatically.
+7. **submit**: ALWAYS submit exactly one review via \`${t("create_pull_request_review")}\`. Do NOT call \`report_progress\` — the review is the final record and the progress comment will be cleaned up automatically.
 
    note: the first create_pull_request_review submission may error with a one-time diff-coverage nudge listing unread TOC regions. retry the same call to proceed — optionally after reading the listed ranges. the pre-flight will not block again this session.
 
@@ -271,10 +289,10 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
 
 ${PR_SUMMARY_FORMAT}`,
     },
-    // IncrementalReview shares Review's multi-lens orchestrator pattern but
-    // scopes the target to the incremental diff. The "issues must be NEW
-    // since the last Pullfrog review" filter lives at aggregation time
-    // (step 6), NOT in the subagent prompt — pushing the filter into
+    // IncrementalReview shares Review's 0-or-2+ lens pattern but scopes the
+    // target to the incremental diff. The "issues must be NEW since the last
+    // Pullfrog review" filter lives at aggregation time (step 8), NOT in the
+    // subagent prompt — pushing the filter into
     // subagents matches the canonical anneal anti-pattern of "list known
     // pre-existing failures — don't flag these" and suppresses signal on
     // regressions the new commits amplified. The review body is just
@@ -294,38 +312,57 @@ ${PR_SUMMARY_FORMAT}`,
 
 3. **incremental scope**: if \`incrementalDiffPath\` is present, read it to see what changed since the last review. this is a range-diff that isolates the net changes, filtering out base branch noise. if not present, fall back to reviewing the full PR diff and determine what changed since Pullfrog's most recent review.
 
-4. **prior feedback**: fetch previous reviews via \`${t("list_pull_request_reviews")}\`. for the most recent Pullfrog review, call \`${t("get_review_comments")}\` with the review ID to retrieve specific prior line-level feedback. you'll use this to filter your aggregation in step 6 — anything already flagged in a prior review and not changed by the new commits should not be re-raised. you do NOT need to render this in the review body; the rolling PR summary snapshot is the durable record of what's been addressed.
+4. **prior feedback**: fetch previous reviews via \`${t("list_pull_request_reviews")}\`. for the most recent Pullfrog review, call \`${t("get_review_comments")}\` with the review ID to retrieve specific prior line-level feedback. you'll use this to filter your aggregation in step 8 — anything already flagged in a prior review and not changed by the new commits should not be re-raised. you do NOT need to render this in the review body; the rolling PR summary snapshot is the durable record of what's been addressed.
 
-5. **triage & fan out**: orient on the *incremental* changes — domain, seams, external contracts, user-facing surfaces.
+5. **triage**: orient on the *incremental* changes — domain, seams, external contracts, user-facing surfaces. pull as much context as you need to render a confident review: read related files, grep for callers of changed symbols, check tests that exercise the touched paths. **you are the synthesizer.**
 
-   if the incremental changes are **genuinely trivial**, skip the fan-out entirely and jump to step 8's non-substantive path (do NOT submit a review).
+   if the incremental changes are **genuinely trivial**, skip the fan-out entirely and jump to step 10's non-substantive path (do NOT submit a review).
 
    "Genuinely trivial" (skip): formatting/comment tweaks, import reordering, lockfile regen, mechanical rename of import paths, whitespace-only.
    "Looks trivial but isn't" (do NOT skip — same anti-patterns as Review mode): 1-line changes to SQL/regex/auth/billing/permissions/signature-verification code; flipping feature-flag defaults or retry/timeout constants; money/tax/HTTP-method/redirect changes; tightening or loosening a comparison operator; mixed diffs with a semantic line buried in formatting.
    When unsure, treat as non-trivial.
 
-   otherwise pick lenses by where the new commits concentrate risk — **there's no fixed count**, same calibration as Review mode (1 lens for pure refactor / isolated fix; 2–3 for typical features; 4–5 for high-stakes subsystem touches; 6+ is a smell). lens framing follows Review mode: themed lenses (correctness & invariants, impact when new commits remove/rename/deprecate things, research-validated assumptions, security, user-journey, operational readiness, integration & cross-cutting, test integrity, performance, holistic) and subsystem lenses (auth, billing, schema migration, etc.) — for high-stakes domains lead with the subsystem lens rather than the generic themed equivalent.
+6. **lens decision — 0 or 2+, NEVER 1**.
 
-   dispatch one \`${REVIEWER_AGENT_NAME}\` subagent per lens — its baked-in system prompt enforces the non-mutative + non-recursive contract (read-only file/search/web tools and read-only MCP queries; no writes, shell side effects, state-changing MCP calls, or nested subagent dispatch). dispatch them in a **single assistant turn with multiple parallel subagent calls** (serial dispatch collapses the fan-out). if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body — do not skip step 5 entirely on a single subagent failure. each subagent gets:
-   - the diff scope (incremental diff path if available, full diff otherwise). do NOT tell them to skip pre-existing issues — that suppresses regressions the new commits amplified; the "issues must be NEW" filter lives at aggregation time (step 6), not in the subagent prompt
+   The default is **0 lenses**: handle the re-review yourself end-to-end. Most incremental reviews land here — especially thread-reply re-reviews where the user is asking "did you address X?" rather than "review the diff again."
+
+   Dispatch **2+ \`${REVIEWER_AGENT_NAME}\` lenses in parallel** ONLY when ALL of the following are true:
+   - the incremental changes are substantive (>5 files changed AND >200 net new lines), OR touch a high-stakes subsystem (auth, billing, payments, schema migration, webhooks, secrets, RBAC, multi-tenant isolation, cron/scheduling)
+   - you can name 2+ distinct concrete failure modes the new commits plausibly introduce that warrant independent lenses
+   - parallel-orchestrated independent perspectives meaningfully outperform what you'd find solo
+
+   **NEVER dispatch exactly one lens.** Single-lens dispatch adds wall time and cost for no orthogonality benefit. Either go multi-lens (≥2 in parallel) or do the re-review yourself.
+
+   Lens framing follows Review mode: themed lenses (correctness, security, etc.) and subsystem lenses (auth, billing, schema-migration, etc.) — for high-stakes domains lead with the subsystem lens.
+
+7. **fan out (only if step 6 said 2+ lenses)**: dispatch every \`${REVIEWER_AGENT_NAME}\` subagent for this run **IN A SINGLE ASSISTANT TURN, AS MULTIPLE PARALLEL TASK TOOL_USE BLOCKS IN ONE MESSAGE.**
+
+   ⚠️  CRITICAL — PARALLELISM IS THE ONLY REASON LENSES EXIST. ⚠️
+   Default tool-call behavior is **serial dispatch**: emit one Task call, await result, emit next, await, etc. This collapses your fan-out into a sequential review where each lens adds N × (orchestrator-think-time + lens-execution-time) to wall time. **YOU MUST OVERRIDE THIS DEFAULT.** Emit ALL of your Task tool_use blocks in the SAME assistant message, BEFORE you read ANY result from ANY of them.
+
+   ✅ Right pattern: one assistant turn with N Task tool_use blocks → wait → N results arrive together → aggregate.
+   ❌ Wrong pattern: turn 1 = Task(lens A) → turn 2 (after A's result) = Task(lens B). This is the failure mode.
+
+   You can also include your own \`read\` / \`grep\` / \`webfetch\` calls in the SAME turn as the parallel \`${REVIEWER_AGENT_NAME}\` dispatches.
+
+   if a subagent errors out, times out, or returns nothing usable, retry once with the same lens; if it still fails, proceed with partial coverage and note the missing lens in the review body. each subagent gets:
+   - the diff scope (incremental diff path if available, full diff otherwise). do NOT tell them to skip pre-existing issues — that suppresses regressions the new commits amplified; the "issues must be NEW" filter lives at aggregation time (step 8), not in the subagent prompt
    - **only one lens** — never a multi-section "review for X, Y, and Z" prompt
-   - **a Task \`description\` set to the lens name** (e.g. \`"security"\`, \`"correctness"\`, \`"billing-subsystem"\`) — the harness reads this field to label the subagent's log lines so parallel runs can be told apart in CI output. without it, every subagent shows up as \`subagent#N\`.
-   - the read-only contract restated in your dispatch instructions so the rule is present twice (the subagent's system prompt also enforces it). The test: would this call still be a no-op if reverted? If not (PR comments, branch pushes, issue updates, set_output, label changes, dependency installs, etc.), don't make it.
-   - if the lens touches external contracts, instruct the subagent to verify load-bearing claims via web search and quote source URLs. action runs are non-interactive — there's no human to catch "I'm pretty sure Stripe does X."
+   - **a Task \`description\` set to the lens name** — the harness reads this field to label log lines so parallel runs can be told apart.
+   - if the lens touches external contracts, instruct the subagent to verify load-bearing claims via web search and quote source URLs.
    - ask the subagent to report findings with file paths and NEW line numbers from the full PR diff so you can anchor inline comments.
 
    delegation discipline:
-   - do NOT lens-review the diff yourself in parallel with the subagents
    - do NOT summarize the changes for them (biases toward validation frame)
    - do NOT hand them a curated reading list (let them discover scope)
    - do NOT pre-shape their output with a finding schema
    - do NOT mention the other lenses (independence is the point)
 
-6. **aggregate, draft, self-critique**: merge findings; de-dup overlaps; trace each finding yourself. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the new commits, anything not actionable, and anything that re-states prior review feedback (heuristic: if the finding's root cause lives in lines the *new commits* added or modified, it's in scope; otherwise drop). also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or degrades elegance to nominally improve correctness) makes the codebase worse, not better. To compute "lines the new commits added or modified": if \`incrementalDiffPath\` from step 2 is present, use it directly. Otherwise, take the prior Pullfrog review's \`commit_id\` (returned alongside each entry from \`${t("list_pull_request_reviews")}\` in step 4) and run \`git diff <prior-review-sha>..HEAD\` to isolate the lines added since that review. draft inline comments with NEW line numbers from the full PR diff — every comment must be actionable, 2-3 sentences max.
+8. **aggregate, draft, self-critique**: merge findings (yours + any subagent output if you went multi-lens); de-dup overlaps; trace each finding yourself. drop praise, style preferences, speculative/unverified claims, findings about pre-existing code unrelated to the new commits, anything not actionable, and anything that re-states prior review feedback (heuristic: if the finding's root cause lives in lines the *new commits* added or modified, it's in scope; otherwise drop). also drop **bloat-shaped findings** — proposed fixes that would add defensive checks for cases that can't happen, abstractions used once, comments restating obvious code, tests asserting tautologies, or "just-in-case" guards. subagents are fallible and bias toward recommending changes; the bar for an actionable inline comment is sound + correct + elegant. recommending a change that improves only one of the three (or degrades elegance to nominally improve correctness) makes the codebase worse, not better. To compute "lines the new commits added or modified": if \`incrementalDiffPath\` from step 2 is present, use it directly. Otherwise, take the prior Pullfrog review's \`commit_id\` (returned alongside each entry from \`${t("list_pull_request_reviews")}\` in step 4) and run \`git diff <prior-review-sha>..HEAD\` to isolate the lines added since that review. draft inline comments with NEW line numbers from the full PR diff — every comment must be actionable, 2-3 sentences max.
 
-7. **build the review body** — a single "Reviewed changes" section: summarize at the logical-change level, not per-file. each bullet starts with a past-tense verb (e.g. \`- Extracted shared CLI runtime into a single module\`, \`- Renamed package to pullfrog\`). avoid file paths unless they add clarity. if the changes can be described in one sentence, use one sentence — no bullets needed. do NOT include a separate "Prior review feedback" checklist; that's tracked in the rolling PR summary snapshot for the next agent run, and surfacing it in the user-facing body is noise (changes that addressed prior feedback are already covered by the Reviewed-changes bullets). in some cases you may receive a complete diff for the whole pull request instead of an incremental one — when this happens, you will need to determine what changes have happened since Pullfrog's most recent review.
+9. **build the review body** — a single "Reviewed changes" section: summarize at the logical-change level, not per-file. each bullet starts with a past-tense verb (e.g. \`- Extracted shared CLI runtime into a single module\`, \`- Renamed package to pullfrog\`). avoid file paths unless they add clarity. if the changes can be described in one sentence, use one sentence — no bullets needed. do NOT include a separate "Prior review feedback" checklist; that's tracked in the rolling PR summary snapshot for the next agent run, and surfacing it in the user-facing body is noise (changes that addressed prior feedback are already covered by the Reviewed-changes bullets). in some cases you may receive a complete diff for the whole pull request instead of an incremental one — when this happens, you will need to determine what changes have happened since Pullfrog's most recent review.
 
-8. Submit — every run must end with EXACTLY ONE of \`${t("create_pull_request_review")}\` (substantive review) or \`${t("report_progress")}\` (no-review acknowledgement). do NOT call \`create_issue_comment\` for review output.
+10. Submit — every run must end with EXACTLY ONE of \`${t("create_pull_request_review")}\` (substantive review) or \`${t("report_progress")}\` (no-review acknowledgement). do NOT call \`create_issue_comment\` for review output.
 
    Same callout-intensity ladder as Review mode — \`[!CAUTION]\` (large red, "will break") → \`[!IMPORTANT]\` (large purple, "must address before merging") → \`[!NOTE]\` (small blue, "FYI") → no callout (plain text). And the same Fix-button lever: the footer renders a Fix button on every non-approving review, so \`approved: true\` suppresses it. Wrapping mergeable feedback in \`[!IMPORTANT]\` trains users to click Fix on reviews that don't need fixing — pick the tier the author's actual next action justifies.
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "pullfrog",
-  "version": "0.1.3",
+  "version": "0.1.7",
   "type": "module",
   "bin": {
     "pullfrog": "dist/cli.mjs",

runCli.ts

@@ -104,6 +104,15 @@ function createRuntimeContext(): RuntimeContext {
   const env: NodeJS.ProcessEnv = { ...process.env };
   env.npm_config_registry = NPM_REGISTRY;
   env.COREPACK_NPM_REGISTRY = NPM_REGISTRY;
+  // bypass customer-side release-age gates (npm's `min-release-age`, pnpm's
+  // `minimumReleaseAge`) so our bootstrap can resolve the latest publish.
+  // pullfrog's npm version is server-stamped from a SHA-pinned action ref the
+  // customer already vets at the action layer — not a customer-vetted dep, so
+  // the gate is the wrong affordance here. env beats .npmrc in both tools.
+  // npm uses `npm_config_*`; pnpm v11+ requires `pnpm_config_*` (the v10→v11
+  // migration renamed the prefix). tracked: #713
+  env.npm_config_min_release_age = "0";
+  env.pnpm_config_minimum_release_age = "0";
   const currentPath = process.env.PATH ?? "";
   env.PATH = currentPath ? `${nodeBinDir}${delimiter}${currentPath}` : nodeBinDir;
 

test/list-aliases.ts

@@ -50,8 +50,12 @@ const FLAGSHIPS = [
 function isPrunablePassthrough(alias: (typeof modelAliases)[number]): boolean {
   if (ROUTING_CANARIES.has(alias.slug)) return false;
   if (alias.provider === "openrouter") return true;
-  // opencode FREE models (big-pickle, mimo, minimax, gpt-5-nano) are unique
-  // to opencode and used in prod — keep them. only prune the keyed mirrors.
+  // routing slugs (bedrock/byok) need a per-run env var to pick the actual
+  // model — there's no generic smoke test, so prune from both matrices.
+  if (alias.routing) return true;
+  // opencode FREE models (big-pickle, mimo-v2-pro-free, minimax-m2.5-free)
+  // are unique to opencode and used in prod — keep them. only prune the keyed
+  // mirrors.
   return alias.provider === "opencode" && !alias.isFree;
 }
 

test/model-smoke.ts

@@ -44,6 +44,11 @@ type Plan =
 async function plan(slug: string): Promise<Plan> {
   const alias = modelAliases.find((a) => a.slug === slug);
   if (!alias) throw new Error(`model-smoke: unknown alias "${slug}"`);
+  if (alias.routing) {
+    throw new Error(
+      `model-smoke: ${slug} is a routing slug (no fixed model). pass an explicit Bedrock model ID via PULLFROG_MODEL or the workflow env block.`
+    );
+  }
 
   // walk the fallback chain so deprecated aliases (those with `fallback` set,
   // e.g. opencode/mimo-v2-pro-free → opencode/big-pickle) hit their replacement

test/models-catalog.main.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { modelAliases } from "../models.ts";
+import { modelAliases, resolveDisplayAlias } from "../models.ts";
 
 // ── catalog drift tests — main-only ─────────────────────────────────────────────
 //
@@ -21,6 +21,7 @@ type ModelsDevModel = {
   name: string;
   status?: string;
   release_date?: string;
+  cost?: { input?: number; output?: number };
 };
 
 type ModelsDevProvider = {
@@ -41,6 +42,11 @@ describe("models.dev validity", async () => {
   const data = await api;
 
   for (const alias of modelAliases) {
+    // routing slugs (e.g. bedrock/byok) have no fixed `resolve` — the actual
+    // model ID is read from a separate env var at run time. skip drift checks
+    // since there's no models.dev entry to validate against.
+    if (alias.routing) continue;
+
     const parsed = parseResolve(alias.resolve);
 
     it(`${alias.resolve} exists on models.dev`, () => {
@@ -112,3 +118,70 @@ describe("openRouterResolve OpenRouter API validity", async () => {
     });
   }
 });
+
+// ── OpenCode Zen served-list + free-cost checks ────────────────────────────────
+//
+// these enforce the two dynamic conditions for "this opencode alias works for a
+// user without OPENCODE_API_KEY" — the gap that let issue #691 ship:
+//   1. the alias's terminal-fallback resolve appears in Zen's /v1/models (Zen
+//      actually serves it). caught nothing in #691 because mimo had a fallback
+//      to big-pickle which IS served, but would catch any future alias that
+//      points at a Zen-removed model without a fallback.
+//   2. for isFree aliases, the terminal-fallback's models.dev `cost.input` is
+//      zero. caught the gpt-5-nano regression: $0.05/M input on models.dev,
+//      marked isFree in our catalog.
+//
+// we check the terminal-fallback (via resolveDisplayAlias) because deprecated
+// aliases legitimately point at dead resolve targets — the terminal is what
+// actually runs at the agent CLI.
+
+type ZenModel = { id: string };
+type ZenModelsResponse = { data: ZenModel[] };
+
+const zenApi = fetch("https://opencode.ai/zen/v1/models").then(
+  (r) => r.json() as Promise<ZenModelsResponse>
+);
+
+describe("opencode Zen served list", async () => {
+  const zenData = await zenApi;
+  const zenIds = new Set(zenData.data.map((m) => m.id));
+  const seen = new Set<string>();
+
+  for (const alias of modelAliases) {
+    const terminal = resolveDisplayAlias(alias.slug);
+    if (!terminal) continue;
+    const parsed = parseResolve(terminal.resolve);
+    if (parsed.provider !== "opencode") continue;
+    if (seen.has(terminal.resolve)) continue;
+    seen.add(terminal.resolve);
+
+    it(`${alias.slug} terminal resolve ${terminal.resolve} is served by Zen`, () => {
+      expect(
+        zenIds.has(parsed.modelId),
+        `terminal resolve "${terminal.resolve}" for alias "${alias.slug}" is not in https://opencode.ai/zen/v1/models — Zen no longer serves it. either point a fallback at a Zen-served alias or remove the entry.`
+      ).toBe(true);
+    });
+  }
+});
+
+describe("isFree models.dev cost", async () => {
+  const data = await api;
+  const seen = new Set<string>();
+
+  for (const alias of modelAliases.filter((a) => a.isFree)) {
+    const terminal = resolveDisplayAlias(alias.slug);
+    if (!terminal) continue;
+    const parsed = parseResolve(terminal.resolve);
+    if (seen.has(terminal.resolve)) continue;
+    seen.add(terminal.resolve);
+
+    it(`${alias.slug} terminal resolve ${terminal.resolve} has cost.input === 0`, () => {
+      const model = data[parsed.provider]?.models[parsed.modelId];
+      expect(model, `terminal resolve "${terminal.resolve}" missing on models.dev`).toBeDefined();
+      expect(
+        model?.cost?.input,
+        `isFree alias "${alias.slug}" walks to "${terminal.resolve}" which reports cost.input=${model?.cost?.input} on models.dev — either repoint the fallback or drop \`isFree\``
+      ).toBe(0);
+    });
+  }
+});

test/models.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { modelAliases, resolveCliModel } from "../models.ts";
+import { getModelEnvVars, modelAliases, resolveCliModel, resolveDisplayAlias } from "../models.ts";
 
 // ── pure alias-registry invariants ──────────────────────────────────────────────
 //
@@ -14,6 +14,10 @@ const BYOK_ONLY_MODELS = new Set(["openai/o3"]);
 describe("openRouterResolve completeness", () => {
   for (const alias of modelAliases) {
     if (alias.isFree) continue;
+    // routing slugs (e.g. bedrock/byok) are inherently BYOK — there's no
+    // single model to map to OpenRouter because the actual model ID is read
+    // from a per-run env var.
+    if (alias.routing) continue;
     if (BYOK_ONLY_MODELS.has(alias.slug)) continue;
     it(`${alias.slug} has openRouterResolve`, () => {
       expect(
@@ -29,6 +33,13 @@ describe("openRouterResolve completeness", () => {
       expect(alias.openRouterResolve).toBeUndefined();
     });
   }
+
+  for (const alias of modelAliases) {
+    if (!alias.routing) continue;
+    it(`${alias.slug} (routing slug) has no openRouterResolve`, () => {
+      expect(alias.openRouterResolve).toBeUndefined();
+    });
+  }
 });
 
 describe("fallback chain resolution", () => {
@@ -42,3 +53,49 @@ describe("fallback chain resolution", () => {
     });
   }
 });
+
+// ── isFree invariants — sanity-check the catalog data shape ─────────────────────
+//
+// these catch the latent regressions that produced issue #691:
+//   - opencode/gpt-5-nano was marked `isFree` despite costing $0.05/M
+//     (no static check existed; demoted to paid in the same PR adding these tests)
+//   - opencode/mimo-v2-pro-free was free + fallback to big-pickle (correct shape),
+//     but nothing enforced that the terminal of an isFree fallback chain is itself
+//     free. if someone repointed big-pickle's fallback at a paid model, all of mimo
+//     and big-pickle's users would silently start hitting a paid endpoint.
+//
+// the cost.input check itself is network-dependent (lives in
+// models-catalog.main.test.ts); these are the static sibling that runs on every PR.
+describe("isFree invariants", () => {
+  for (const alias of modelAliases.filter((a) => a.isFree)) {
+    it(`${alias.slug} lives under the opencode provider`, () => {
+      expect(
+        alias.provider,
+        `isFree alias "${alias.slug}" must be under "opencode" (Zen's keyless gate is opencode-only)`
+      ).toBe("opencode");
+    });
+
+    it(`${alias.slug} has empty envVars`, () => {
+      expect(
+        getModelEnvVars(alias.slug),
+        `isFree alias "${alias.slug}" must declare \`envVars: []\` so validateAgentApiKey doesn't demand OPENCODE_API_KEY`
+      ).toEqual([]);
+    });
+
+    it(`${alias.slug} has no openRouterResolve`, () => {
+      expect(
+        alias.openRouterResolve,
+        `isFree alias "${alias.slug}" must omit \`openRouterResolve\` — free Zen models don't exist on OpenRouter`
+      ).toBeUndefined();
+    });
+
+    it(`${alias.slug} fallback chain terminates at an isFree alias`, () => {
+      const terminal = resolveDisplayAlias(alias.slug);
+      expect(terminal, `fallback chain for "${alias.slug}" is broken`).toBeDefined();
+      expect(
+        terminal?.isFree,
+        `isFree alias "${alias.slug}" walks to "${terminal?.slug}" which is NOT isFree — users would silently start paying`
+      ).toBe(true);
+    });
+  }
+});

test/smoke-models.ts

@@ -28,6 +28,14 @@ const results: { model: string; status: "pass" | "fail" | "skip"; detail?: strin
 const seen = new Set<string>();
 
 for (const alias of modelAliases) {
+  // routing slugs (bedrock/byok) have no fixed `resolve` to test against —
+  // the model ID is supplied at run time via a per-run env var. skipping
+  // here matches the bumps cron + catalog drift test.
+  if (alias.routing) {
+    results.push({ model: alias.slug, status: "skip", detail: "routing slug (no fixed resolve)" });
+    continue;
+  }
+
   if (seen.has(alias.resolve)) {
     results.push({ model: alias.resolve, status: "skip", detail: "duplicate resolve" });
     continue;

utils/agent.test.ts

@@ -1,9 +1,131 @@
-import { describe, expect, it } from "vitest";
-import { resolveAgent } from "./agent.ts";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { resolveAgent, resolveModel } from "./agent.ts";
+
+const savedEnv = { ...process.env };
+
+const STRIPPED = [
+  /_API_KEY$/,
+  /^CLAUDE_CODE_OAUTH_TOKEN$/,
+  /^AWS_BEARER_TOKEN_BEDROCK$/,
+  /^AWS_ACCESS_KEY_ID$/,
+  /^AWS_SECRET_ACCESS_KEY$/,
+  /^AWS_SESSION_TOKEN$/,
+  /^AWS_REGION$/,
+  /^BEDROCK_MODEL_ID$/,
+  /^PULLFROG_MODEL$/,
+  /^PULLFROG_AGENT$/,
+];
+
+beforeEach(() => {
+  for (const key of Object.keys(process.env)) {
+    if (STRIPPED.some((re) => re.test(key))) delete process.env[key];
+  }
+});
+
+afterEach(() => {
+  process.env = { ...savedEnv };
+});
 
 describe("resolveAgent", () => {
-  it("returns opencode", () => {
-    const agent = resolveAgent({});
-    expect(agent.name).toBe("opencode");
+  it("returns opencode by default", () => {
+    expect(resolveAgent({}).name).toBe("opencode");
+  });
+
+  it("routes anthropic/* to claude when ANTHROPIC_API_KEY is set", () => {
+    process.env.ANTHROPIC_API_KEY = "sk-test";
+    expect(resolveAgent({ model: "anthropic/claude-opus-4-7" }).name).toBe("claude");
+  });
+
+  it("falls back to opencode for anthropic/* without claude-code creds", () => {
+    expect(resolveAgent({ model: "anthropic/claude-opus-4-7" }).name).toBe("opencode");
+  });
+
+  describe("bedrock routing", () => {
+    it("routes Anthropic Bedrock IDs to claude", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(resolveAgent({ model: "us.anthropic.claude-opus-4-7" }).name).toBe("claude");
+    });
+
+    it("routes Anthropic Bedrock IDs (no region prefix) to claude", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.BEDROCK_MODEL_ID = "anthropic.claude-haiku-4-5-20251001-v1:0";
+      expect(resolveAgent({ model: "anthropic.claude-haiku-4-5-20251001-v1:0" }).name).toBe(
+        "claude"
+      );
+    });
+
+    it("routes non-Anthropic Bedrock IDs to opencode", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.BEDROCK_MODEL_ID = "amazon.nova-pro-v1:0";
+      expect(resolveAgent({ model: "amazon.nova-pro-v1:0" }).name).toBe("opencode");
+    });
+
+    it("routes Llama IDs to opencode", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.BEDROCK_MODEL_ID = "us.meta.llama4-scout-17b-instruct-v1:0";
+      expect(resolveAgent({ model: "us.meta.llama4-scout-17b-instruct-v1:0" }).name).toBe(
+        "opencode"
+      );
+    });
+
+    it("accepts AWS access keys as auth", () => {
+      process.env.AWS_ACCESS_KEY_ID = "AKIA-test";
+      process.env.AWS_SECRET_ACCESS_KEY = "secret-test";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(resolveAgent({ model: "us.anthropic.claude-opus-4-7" }).name).toBe("claude");
+    });
+
+    it("PULLFROG_AGENT override wins over Anthropic auto-routing", () => {
+      process.env.PULLFROG_AGENT = "opencode";
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(resolveAgent({ model: "us.anthropic.claude-opus-4-7" }).name).toBe("opencode");
+    });
+  });
+});
+
+describe("resolveModel", () => {
+  it("PULLFROG_MODEL override wins", () => {
+    process.env.PULLFROG_MODEL = "anthropic/claude-opus";
+    expect(resolveModel({ slug: "openai/gpt" })).toBe("anthropic/claude-opus-4-7");
+  });
+
+  it("PULLFROG_MODEL bypasses bedrock routing entirely", () => {
+    process.env.PULLFROG_MODEL = "openai/gpt";
+    process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+    expect(resolveModel({ slug: "bedrock/byok" })).toBe("openai/gpt-5.5");
+  });
+
+  it("resolves bedrock/byok to BEDROCK_MODEL_ID", () => {
+    process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+    expect(resolveModel({ slug: "bedrock/byok" })).toBe("us.anthropic.claude-opus-4-7");
+  });
+
+  it("throws when bedrock/byok is selected without BEDROCK_MODEL_ID", () => {
+    expect(() => resolveModel({ slug: "bedrock/byok" })).toThrow("BEDROCK_MODEL_ID");
+  });
+
+  it("returns the alias resolve for normal slugs", () => {
+    expect(resolveModel({ slug: "openai/gpt" })).toBe("openai/gpt-5.5");
+  });
+
+  it("returns undefined for no slug + no PULLFROG_MODEL", () => {
+    expect(resolveModel({})).toBeUndefined();
+  });
+
+  // regression: PR #720 review caught that `resolveCliModel("bedrock/byok")`
+  // returns the literal sentinel `"bedrock"` from the alias's `resolve`
+  // field. Without routing-aware handling, PULLFROG_MODEL=bedrock/byok would
+  // leak that sentinel downstream and break agent dispatch.
+  it("PULLFROG_MODEL=bedrock/byok defers to BEDROCK_MODEL_ID, not the sentinel", () => {
+    process.env.PULLFROG_MODEL = "bedrock/byok";
+    process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+    expect(resolveModel({ slug: "openai/gpt" })).toBe("us.anthropic.claude-opus-4-7");
+  });
+
+  it("PULLFROG_MODEL=bedrock/byok throws if BEDROCK_MODEL_ID is missing", () => {
+    process.env.PULLFROG_MODEL = "bedrock/byok";
+    expect(() => resolveModel({ slug: "openai/gpt" })).toThrow("BEDROCK_MODEL_ID");
   });
 });

utils/agent.ts

@@ -1,6 +1,12 @@
 import type { Agent } from "../agents/index.ts";
 import { agents } from "../agents/index.ts";
-import { getModelProvider, resolveCliModel } from "../models.ts";
+import {
+  BEDROCK_MODEL_ID_ENV,
+  getModelProvider,
+  isBedrockAnthropicId,
+  resolveCliModel,
+  resolveDisplayAlias,
+} from "../models.ts";
 import { log } from "./cli.ts";
 
 function hasEnvVar(name: string): boolean {
@@ -12,6 +18,37 @@ function hasClaudeCodeAuth(): boolean {
   return hasEnvVar("CLAUDE_CODE_OAUTH_TOKEN") || hasEnvVar("ANTHROPIC_API_KEY");
 }
 
+function hasBedrockAuth(): boolean {
+  return (
+    hasEnvVar("AWS_BEARER_TOKEN_BEDROCK") ||
+    (hasEnvVar("AWS_ACCESS_KEY_ID") && hasEnvVar("AWS_SECRET_ACCESS_KEY"))
+  );
+}
+
+/**
+ * resolve a single slug to its CLI-ready model string. routing aliases
+ * (e.g. `bedrock/byok`) defer to their backing env var instead of the
+ * sentinel stored in `resolve`. shared between PULLFROG_MODEL override
+ * and repo-config slug resolution so both paths get the same routing
+ * semantics — without this helper, `PULLFROG_MODEL=bedrock/byok` would
+ * leak the literal sentinel string `"bedrock"` downstream.
+ */
+function resolveSlug(slug: string): string | undefined {
+  const alias = resolveDisplayAlias(slug);
+  if (alias?.routing === "bedrock") {
+    const bedrockId = process.env[BEDROCK_MODEL_ID_ENV]?.trim();
+    if (!bedrockId) {
+      throw new Error(
+        `${BEDROCK_MODEL_ID_ENV} env var is required when the model is set to "${slug}". ` +
+          `set it to an AWS Bedrock model ID (e.g. "us.anthropic.claude-opus-4-7", "amazon.nova-pro-v1:0"). ` +
+          `see https://docs.pullfrog.com/bedrock for setup.`
+      );
+    }
+    return bedrockId;
+  }
+  return resolveCliModel(slug);
+}
+
 /**
  * resolve the effective model for this run.
  *
@@ -19,17 +56,20 @@ function hasClaudeCodeAuth(): boolean {
  *   1. PULLFROG_MODEL env var — resolved through the alias registry first,
  *      so values like "anthropic/claude-opus" become "anthropic/claude-opus-4-7".
  *      raw specifiers (e.g. "anthropic/claude-opus-4-6") pass through unchanged.
- *   2. slug from repo config / payload → alias registry
- *   3. undefined — agent will auto-select
+ *      always wins — bypasses Bedrock routing entirely. to test a different
+ *      Bedrock model, change `BEDROCK_MODEL_ID`, not `PULLFROG_MODEL`.
+ *   2. slug from repo config / payload → alias registry. routing slugs
+ *      (e.g. `bedrock/byok`) defer to a separate env var (`BEDROCK_MODEL_ID`).
+ *   3. undefined — agent will auto-select.
  */
 export function resolveModel(ctx: { slug?: string | undefined }): string | undefined {
   const envModel = process.env.PULLFROG_MODEL?.trim();
   if (envModel) {
-    return resolveCliModel(envModel) ?? envModel;
+    return resolveSlug(envModel) ?? envModel;
   }
 
   if (ctx.slug) {
-    const resolved = resolveCliModel(ctx.slug);
+    const resolved = resolveSlug(ctx.slug);
     if (resolved) {
       return resolved;
     }
@@ -49,7 +89,15 @@ export function resolveAgent(ctx: { model?: string | undefined }): Agent {
     log.warning(`» unknown PULLFROG_AGENT="${envAgent}" — falling through to auto-select`);
   }
 
-  // 2. if model is Anthropic and Claude Code credentials are available, use Claude Code
+  // 2. Bedrock routing: when BEDROCK_MODEL_ID is the resolved model, route
+  //    Anthropic IDs through claude-code (which supports Bedrock natively
+  //    once CLAUDE_CODE_USE_BEDROCK=1) and everything else through opencode's
+  //    `amazon-bedrock` provider.
+  if (ctx.model && hasBedrockAuth() && process.env[BEDROCK_MODEL_ID_ENV]?.trim() === ctx.model) {
+    return isBedrockAnthropicId(ctx.model) ? agents.claude : agents.opencode;
+  }
+
+  // 3. if model is Anthropic and Claude Code credentials are available, use Claude Code
   if (ctx.model) {
     try {
       const provider = getModelProvider(ctx.model);
@@ -61,6 +109,6 @@ export function resolveAgent(ctx: { model?: string | undefined }): Agent {
     }
   }
 
-  // 3. default: OpenCode (universal, supports all providers)
+  // 4. default: OpenCode (universal, supports all providers)
   return agents.opencode;
 }

utils/apiFetch.ts

@@ -37,6 +37,18 @@ export async function apiFetch(options: ApiFetchOptions): Promise<Response> {
     headers["x-vercel-protection-bypass"] = bypassSecret;
   }
 
+  // never send Content-Type on body-less requests. empirically, Vercel's
+  // Next.js lambda adapter (Next 16.1.x) throws `SyntaxError: Unexpected
+  // end of data` before delegating to the route handler — returning a 500 —
+  // when Content-Type is set but no body is present. exact mechanism is
+  // unverified (minified runtime frame), but Content-Type on a body-less
+  // request has no defined semantics per RFC 9110 §8.3 anyway. see #692.
+  if (!options.body) {
+    for (const key of Object.keys(headers)) {
+      if (key.toLowerCase() === "content-type") delete headers[key];
+    }
+  }
+
   log.debug(`api fetch: ${options.method ?? "GET"} ${url.pathname}`);
 
   const init: RequestInit = {

utils/apiKeys.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
-import { validateAgentApiKey } from "./apiKeys.ts";
+import { formatApiKeyErrorSummary, isApiKeyAuthError, validateAgentApiKey } from "./apiKeys.ts";
 
 const base = {
   agent: { name: "opencode" },
@@ -9,10 +9,23 @@ const base = {
 
 const savedEnv = { ...process.env };
 
+// keys that count as provider auth in `knownApiKeys` and would let the
+// auto-select path pass without our intent. strip all of them at test setup
+// so each `it` starts from a clean slate regardless of what's in the dev `.env`.
+const STRIPPED_PREFIXES_OR_NAMES = [
+  /_API_KEY$/,
+  /^CLAUDE_CODE_OAUTH_TOKEN$/,
+  /^AWS_BEARER_TOKEN_BEDROCK$/,
+  /^AWS_ACCESS_KEY_ID$/,
+  /^AWS_SECRET_ACCESS_KEY$/,
+  /^AWS_SESSION_TOKEN$/,
+  /^AWS_REGION$/,
+  /^BEDROCK_MODEL_ID$/,
+];
+
 beforeEach(() => {
-  // strip all known provider keys so tests start clean
   for (const key of Object.keys(process.env)) {
-    if (key.endsWith("_API_KEY") || key === "CLAUDE_CODE_OAUTH_TOKEN") delete process.env[key];
+    if (STRIPPED_PREFIXES_OR_NAMES.some((re) => re.test(key))) delete process.env[key];
   }
 });
 
@@ -27,11 +40,7 @@ describe("validateAgentApiKey", () => {
     });
 
     it("passes for other free opencode models", () => {
-      for (const slug of [
-        "opencode/gpt-5-nano",
-        "opencode/mimo-v2-pro-free",
-        "opencode/minimax-m2.5-free",
-      ]) {
+      for (const slug of ["opencode/mimo-v2-pro-free", "opencode/minimax-m2.5-free"]) {
         expect(() => validateAgentApiKey({ ...base, model: slug })).not.toThrow();
       }
     });
@@ -59,6 +68,12 @@ describe("validateAgentApiKey", () => {
         "no API key found"
       );
     });
+
+    it("throws for opencode/gpt-5-nano without OPENCODE_API_KEY (paid Zen alias)", () => {
+      expect(() => validateAgentApiKey({ ...base, model: "opencode/gpt-5-nano" })).toThrow(
+        "no API key found"
+      );
+    });
   });
 
   describe("no model (auto-select)", () => {
@@ -71,4 +86,118 @@ describe("validateAgentApiKey", () => {
       expect(() => validateAgentApiKey({ ...base, model: undefined })).toThrow("no API key found");
     });
   });
+
+  describe("bedrock routing slug", () => {
+    it("passes with AWS_BEARER_TOKEN_BEDROCK + AWS_REGION + BEDROCK_MODEL_ID", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.AWS_REGION = "us-east-1";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).not.toThrow();
+    });
+
+    it("passes with AWS access keys + region + model id", () => {
+      process.env.AWS_ACCESS_KEY_ID = "AKIA-test";
+      process.env.AWS_SECRET_ACCESS_KEY = "secret-test";
+      process.env.AWS_REGION = "us-east-1";
+      process.env.BEDROCK_MODEL_ID = "amazon.nova-pro-v1:0";
+      expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).not.toThrow();
+    });
+
+    it("throws when BEDROCK_MODEL_ID is missing", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.AWS_REGION = "us-east-1";
+      expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow(
+        "BEDROCK_MODEL_ID"
+      );
+    });
+
+    it("throws when AWS_REGION is missing", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow("AWS_REGION");
+    });
+
+    it("throws when no auth is set", () => {
+      process.env.AWS_REGION = "us-east-1";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow(
+        "AWS_BEARER_TOKEN_BEDROCK"
+      );
+    });
+
+    it("throws when only AWS_ACCESS_KEY_ID is set (missing secret)", () => {
+      process.env.AWS_ACCESS_KEY_ID = "AKIA-test";
+      process.env.AWS_REGION = "us-east-1";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-7";
+      expect(() => validateAgentApiKey({ ...base, model: "bedrock/byok" })).toThrow(
+        "AWS_BEARER_TOKEN_BEDROCK"
+      );
+    });
+
+    // regression: main.ts passes the resolved model into validateAgentApiKey
+    // (`payload.proxyModel ?? resolvedModel ?? payload.model`), which for
+    // bedrock is the raw AWS model ID and has no `/`. parseModel would throw.
+    // see PR #720 e2e run 25821218139 for the original failure mode.
+    it("accepts a raw Bedrock model ID (post-resolveModel) without throwing", () => {
+      process.env.AWS_BEARER_TOKEN_BEDROCK = "bedrock-token";
+      process.env.AWS_REGION = "us-east-1";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-6-v1";
+      expect(() =>
+        validateAgentApiKey({ ...base, model: "us.anthropic.claude-opus-4-6-v1" })
+      ).not.toThrow();
+    });
+
+    it("throws on raw Bedrock model ID when AWS auth is missing", () => {
+      process.env.AWS_REGION = "us-east-1";
+      process.env.BEDROCK_MODEL_ID = "us.anthropic.claude-opus-4-6-v1";
+      expect(() =>
+        validateAgentApiKey({ ...base, model: "us.anthropic.claude-opus-4-6-v1" })
+      ).toThrow("AWS_BEARER_TOKEN_BEDROCK");
+    });
+  });
+});
+
+describe("isApiKeyAuthError", () => {
+  it("matches the missing-key marker thrown by validateAgentApiKey", () => {
+    expect(isApiKeyAuthError("no API key found. Pullfrog needs ...")).toBe(true);
+  });
+
+  it("matches Claude CLI 401 strings", () => {
+    expect(isApiKeyAuthError("Invalid API key · Fix external API key")).toBe(true);
+  });
+
+  it("matches OpenAI / OpenRouter 401 phrasings", () => {
+    expect(isApiKeyAuthError("ProviderAuthError: User not found")).toBe(true);
+    expect(isApiKeyAuthError("401 Invalid authentication")).toBe(true);
+  });
+
+  it("ignores unrelated errors", () => {
+    expect(isApiKeyAuthError("git fetch failed")).toBe(false);
+    expect(isApiKeyAuthError("")).toBe(false);
+  });
+});
+
+describe("formatApiKeyErrorSummary", () => {
+  it("renders the missing-key body when the raw error contains the marker", () => {
+    const msg = formatApiKeyErrorSummary({
+      owner: "acme",
+      name: "repo",
+      raw: "no API key found in this run",
+    });
+    expect(msg).toContain("no API key found");
+    expect(msg).toContain("https://github.com/acme/repo/settings/secrets/actions");
+    expect(msg).toContain("/console/acme/repo");
+    expect(msg).toContain("https://discord.gg/8y96raFg8e");
+  });
+
+  it("renders the invalid-key body for any other auth error", () => {
+    const msg = formatApiKeyErrorSummary({
+      owner: "acme",
+      name: "repo",
+      raw: "Invalid API key · Fix external API key",
+    });
+    expect(msg).toContain("rejected (401)");
+    expect(msg).toContain("https://github.com/acme/repo/settings/secrets/actions");
+    expect(msg).toContain("https://discord.gg/8y96raFg8e");
+  });
 });

utils/apiKeys.ts

@@ -1,28 +1,46 @@
-import { getModelEnvVars, providers } from "../models.ts";
+import {
+  BEDROCK_MODEL_ID_ENV,
+  getModelEnvVars,
+  providers,
+  resolveDisplayAlias,
+} from "../models.ts";
 import { getApiUrl } from "./apiUrl.ts";
 
 const knownApiKeys: Set<string> = new Set(Object.values(providers).flatMap((p) => [...p.envVars]));
 
+/** marker prefix on the throw message for the catch-side reclassification path */
+const MISSING_KEY_MARKER = "no API key found";
+
+/** Markdown body used for both the thrown error and the formatted PR comment summary. */
 function buildMissingApiKeyError(params: { owner: string; name: string }): string {
-  const apiUrl = getApiUrl();
-  const settingsUrl = `${apiUrl}/console/${params.owner}/${params.name}`;
+  const githubSecretsUrl = `https://github.com/${params.owner}/${params.name}/settings/secrets/actions`;
+  const settingsUrl = `${getApiUrl()}/console/${params.owner}/${params.name}`;
 
-  const githubRepoUrl = `https://github.com/${params.owner}/${params.name}`;
-  const githubSecretsUrl = `${githubRepoUrl}/settings/secrets/actions`;
+  return [
+    `**${MISSING_KEY_MARKER}** — Pullfrog needs at least one LLM provider API key (e.g. \`ANTHROPIC_API_KEY\`, \`OPENAI_API_KEY\`, \`GEMINI_API_KEY\`) configured as a GitHub Actions secret.`,
+    "",
+    `[Open repo secrets →](${githubSecretsUrl}) · [Configure model →](${settingsUrl}) · [Setup docs →](https://docs.pullfrog.com/keys) · [Ask in Discord →](https://discord.gg/8y96raFg8e)`,
+  ].join("\n");
+}
 
-  return `no API key found. Pullfrog requires at least one LLM provider API key.
+function buildBedrockSetupError(params: {
+  owner: string;
+  name: string;
+  missing: string[];
+}): string {
+  const githubSecretsUrl = `https://github.com/${params.owner}/${params.name}/settings/secrets/actions`;
 
-to fix this, add the required secret to your GitHub repository:
+  return `Bedrock model selected but required configuration is missing: ${params.missing.join(", ")}.
 
-1. go to: ${githubSecretsUrl}
-2. click "New repository secret"
-3. set the name to your provider's key (e.g., \`ANTHROPIC_API_KEY\`, \`OPENAI_API_KEY\`, \`GEMINI_API_KEY\`)
-4. set the value to your API key
-5. click "Add secret"
+add the missing secret(s) to your GitHub repository at ${githubSecretsUrl}, then reference them in your workflow's \`env:\` block:
 
-configure your model at ${settingsUrl}
+  AWS_BEARER_TOKEN_BEDROCK: \${{ secrets.AWS_BEARER_TOKEN_BEDROCK }}
+  AWS_REGION: \${{ secrets.AWS_REGION }}
+  ${BEDROCK_MODEL_ID_ENV}: \${{ secrets.${BEDROCK_MODEL_ID_ENV} }}
 
-for full setup instructions, see https://docs.pullfrog.com/keys`;
+\`AWS_BEARER_TOKEN_BEDROCK\` may be substituted with \`AWS_ACCESS_KEY_ID\` + \`AWS_SECRET_ACCESS_KEY\` (and optional \`AWS_SESSION_TOKEN\`) if you prefer access keys.
+
+for full setup instructions, see https://docs.pullfrog.com/bedrock`;
 }
 
 function hasEnvVar(name: string): boolean {
@@ -37,6 +55,22 @@ export function hasProviderKey(model: string): boolean {
   return requiredVars.some((v) => hasEnvVar(v));
 }
 
+function validateBedrockSetup(params: { owner: string; name: string }): void {
+  const hasAuth =
+    hasEnvVar("AWS_BEARER_TOKEN_BEDROCK") ||
+    (hasEnvVar("AWS_ACCESS_KEY_ID") && hasEnvVar("AWS_SECRET_ACCESS_KEY"));
+
+  const missing: string[] = [];
+  if (!hasAuth)
+    missing.push("AWS_BEARER_TOKEN_BEDROCK (or AWS_ACCESS_KEY_ID + AWS_SECRET_ACCESS_KEY)");
+  if (!hasEnvVar("AWS_REGION")) missing.push("AWS_REGION");
+  if (!hasEnvVar(BEDROCK_MODEL_ID_ENV)) missing.push(BEDROCK_MODEL_ID_ENV);
+
+  if (missing.length > 0) {
+    throw new Error(buildBedrockSetupError({ owner: params.owner, name: params.name, missing }));
+  }
+}
+
 export function validateAgentApiKey(params: {
   agent: { name: string };
   model: string | undefined;
@@ -45,6 +79,28 @@ export function validateAgentApiKey(params: {
 }): void {
   // if a specific model is configured, only check that model's required env vars
   if (params.model) {
+    // routing slugs (e.g. bedrock) get a tailored validation path because
+    // their auth shape doesn't match the standard "any one envVar present"
+    // rule (Bedrock needs auth + region + model-id, with auth being either
+    // a bearer token OR an access-key pair).
+    const alias = resolveDisplayAlias(params.model);
+    if (alias?.routing === "bedrock") {
+      validateBedrockSetup({ owner: params.owner, name: params.name });
+      return;
+    }
+
+    // upstream `resolveModel` translates `bedrock/byok` into the raw Bedrock
+    // model ID (e.g. `us.anthropic.claude-opus-4-6-v1`), which has no `/`
+    // and so isn't parseable as `provider/model`. these IDs only reach this
+    // function via routing aliases, so re-run the bedrock setup check rather
+    // than falling through to `getModelEnvVars` (which would throw inside
+    // parseModel). resolveModel itself already enforced BEDROCK_MODEL_ID,
+    // but auth + region are still validated here.
+    if (!params.model.includes("/")) {
+      validateBedrockSetup({ owner: params.owner, name: params.name });
+      return;
+    }
+
     const requiredVars = getModelEnvVars(params.model);
     // free models have no required env vars — skip validation entirely
     if (requiredVars.length === 0) return;
@@ -59,3 +115,44 @@ export function validateAgentApiKey(params: {
     throw new Error(buildMissingApiKeyError({ owner: params.owner, name: params.name }));
   }
 }
+
+/**
+ * Detect agent-runtime auth failures that should be reformatted as an actionable
+ * key-fix CTA before being shown to the user. Covers the two shapes we see:
+ *   - missing key (validateAgentApiKey throw): contains MISSING_KEY_MARKER
+ *   - revoked / invalid key (Claude CLI 401 surfaced via api_error_status):
+ *     "Invalid API key · Fix external API key" + similar provider variants
+ */
+export function isApiKeyAuthError(text: string): boolean {
+  if (!text) return false;
+  return (
+    text.includes(MISSING_KEY_MARKER) ||
+    /Invalid API key/i.test(text) ||
+    /\bUser not found\b/i.test(text) ||
+    /\bInvalid authentication\b/i.test(text)
+  );
+}
+
+/**
+ * Friendly Markdown summary for both the missing-key and invalid-key cases.
+ * Used in the catch / result-failure paths in `main.ts` to overwrite the raw
+ * agent error before it's posted to the PR progress comment.
+ */
+export function formatApiKeyErrorSummary(params: {
+  owner: string;
+  name: string;
+  raw: string;
+}): string {
+  if (params.raw.includes(MISSING_KEY_MARKER)) {
+    return buildMissingApiKeyError({ owner: params.owner, name: params.name });
+  }
+
+  const githubSecretsUrl = `https://github.com/${params.owner}/${params.name}/settings/secrets/actions`;
+  const settingsUrl = `${getApiUrl()}/console/${params.owner}/${params.name}`;
+
+  return [
+    `**Your LLM provider API key was rejected (401).** Rotate the key in your provider dashboard, then update the matching GitHub Actions secret.`,
+    "",
+    `[Update repo secret →](${githubSecretsUrl}) · [Model settings →](${settingsUrl}) · [Setup docs →](https://docs.pullfrog.com/keys) · [Ask in Discord →](https://discord.gg/8y96raFg8e)`,
+  ].join("\n");
+}

utils/github.ts

@@ -99,6 +99,22 @@ type AcquireTokenOptions = {
   permissions?: GitHubAppPermissions;
 };
 
+/**
+ * Thrown when our token-exchange endpoint returns a non-2xx response.
+ * The retry policy in `acquireNewToken` looks for this concrete type to
+ * skip retries — 4xx is terminal user state (not-installed, not-authorized)
+ * and 5xx is rare enough that re-running the workflow is the right escape
+ * hatch. Genuine network failures throw plain `Error` and stay retryable.
+ */
+class TokenExchangeError extends Error {
+  readonly status: number;
+  constructor(status: number, message: string) {
+    super(message);
+    this.name = "TokenExchangeError";
+    this.status = status;
+  }
+}
+
 async function acquireTokenViaOIDC(opts?: AcquireTokenOptions): Promise<string> {
   const oidcToken = await core.getIDToken("pullfrog-api");
 
@@ -128,7 +144,22 @@ async function acquireTokenViaOIDC(opts?: AcquireTokenOptions): Promise<string>
     clearTimeout(timeoutId);
 
     if (!tokenResponse.ok) {
-      throw new Error(`Token exchange failed: ${tokenResponse.status} ${tokenResponse.statusText}`);
+      // prefer the server-side `error` field — it's the single source of
+      // truth for the install URL (uses GITHUB_APP_INSTALL_URL, which
+      // varies per env / GITHUB_APP_SLUG). fall back to a generic message
+      // if the body isn't JSON or doesn't carry an `error` field.
+      let serverMessage: string | undefined;
+      try {
+        const body = (await tokenResponse.json()) as { error?: unknown };
+        if (typeof body.error === "string") serverMessage = body.error;
+      } catch {
+        // body wasn't JSON — fall through to the generic message
+      }
+      throw new TokenExchangeError(
+        tokenResponse.status,
+        serverMessage ??
+          `Token exchange failed: ${tokenResponse.status} ${tokenResponse.statusText}`
+      );
     }
 
     const tokenData = (await tokenResponse.json()) as InstallationToken;
@@ -333,13 +364,20 @@ export async function acquireNewToken(opts?: AcquireTokenOptions): Promise<strin
   if (isOIDCAvailable()) {
     return await retry(() => acquireTokenViaOIDC(opts), {
       label: "token exchange",
-      shouldRetry: (error) =>
-        error instanceof Error &&
-        (error.name === "AbortError" ||
-          error.message.includes("fetch failed") ||
-          error.message.includes("ECONNRESET") ||
-          error.message.includes("ETIMEDOUT") ||
-          error.message.includes("Token exchange failed")),
+      shouldRetry: (error) => {
+        // 4xx is terminal user state (app not installed, permissions wrong) —
+        // retrying just triples our log noise and the user's CI bill (see
+        // #693). 5xx/429 are transient (vercel cold start, github outage,
+        // rate limit) and should ride the existing backoff.
+        if (error instanceof TokenExchangeError) return error.status >= 500 || error.status === 429;
+        return (
+          error instanceof Error &&
+          (error.message.includes("timed out") ||
+            error.message.includes("fetch failed") ||
+            error.message.includes("ECONNRESET") ||
+            error.message.includes("ETIMEDOUT"))
+        );
+      },
     });
   } else {
     // local development via GitHub App

utils/instructions.ts

@@ -4,6 +4,7 @@ import { encode as toonEncode } from "@toon-format/toon";
 import { type AgentId, formatMcpToolRef, type PayloadEvent, pullfrogMcpName } from "../external.ts";
 import type { Mode } from "../modes.ts";
 import type { ResolvedPayload } from "./payload.ts";
+import type { LearningsHeading } from "./runContext.ts";
 import type { RunContextData } from "./runContextData.ts";
 
 interface InstructionsContext {
@@ -16,6 +17,10 @@ interface InstructionsContext {
    * couldn't be seeded for some reason. main.ts always seeds, so in
    * practice this is always set; the null case keeps the type honest. */
   learningsFilePath: string | null;
+  /** server-parsed TOC for the body of the learnings tmpfile. rendered
+   * inline into the LEARNINGS prompt section so the agent can `read_file`
+   * targeted line ranges instead of pulling the whole file into context. */
+  learningsHeadings: LearningsHeading[];
 }
 
 interface PromptContext extends InstructionsContext {
@@ -32,6 +37,7 @@ function buildRuntimeContext(ctx: InstructionsContext): string {
     "~pullfrog": _,
     prompt: _p,
     eventInstructions: _ei,
+    previousRunsNote: _prn,
     event: _e,
     ...payloadRest
   } = ctx.payload;
@@ -146,17 +152,23 @@ In case of conflict between instructions, follow this precedence (highest to low
 // section builders
 // ---------------------------------------------------------------------------
 
-// the user's task: blockquoted user prompt, or event-level instructions for auto-triggers
+// the user's task: blockquoted user prompt, or event-level instructions for auto-triggers.
+// `previousRunsNote` is system-injected context (e.g. prior runs superseded by a
+// comment edit); it's appended regardless of which branch wins so it survives
+// user-prompt precedence over eventInstructions.
 function buildTaskSection(ctx: PromptContext): string {
+  const previousRunsNote = ctx.payload.previousRunsNote?.trim() ?? "";
+
   if (ctx.userQuoted) {
+    const parts = [ctx.userQuoted, previousRunsNote].filter(Boolean);
     return `************* YOUR TASK *************
 
-${ctx.userQuoted}`;
+${parts.join("\n\n")}`;
   }
 
   const eventInstructions = ctx.payload.eventInstructions ?? "";
-  if (eventInstructions) {
-    const parts = [ctx.eventTitle, eventInstructions].filter(Boolean);
+  if (eventInstructions || previousRunsNote) {
+    const parts = [ctx.eventTitle, eventInstructions, previousRunsNote].filter(Boolean);
     return `************* YOUR TASK *************
 
 ${parts.join("\n\n")}`;
@@ -276,6 +288,21 @@ ${getStandaloneModeInstructions(ctx.payload.event.trigger, t, ctx.outputSchema)}
 
 Trust the tools — do not repeatedly verify file contents or git status after operations. If a tool reports success, proceed to the next step. Only verify if you encounter an actual error. Exception: right before \`${t("push_branch")}\`, ensure the working tree is clean — that tool rejects dirty trees, and tests you ran earlier often leave untracked output.
 
+### Parallel tool execution
+
+For maximum efficiency, whenever you need to perform multiple independent operations, invoke all relevant tools simultaneously in a single assistant turn rather than sequentially. The dominant failure mode is grep → read → read → read → read across separate turns when one round trip would do. Always parallelize when calls are independent:
+- reading multiple files (especially after a grep returns candidates)
+- multiple greps with different patterns
+- glob + grep + read combos
+- listing multiple directories
+- inspecting multiple MCP tools or resources
+
+Do NOT parallelize operations that depend on prior output (e.g. create a file then read it), or ordered stateful mutations. Edits are not parallelizable — sequence those normally.${
+    ctx.agentId === "opencode"
+      ? `\n\nOn OpenCode you also have a \`batch\` tool that bundles 1-25 independent calls into one wrapper call. Reach for it whenever you have >=2 independent calls. Native parallel tool_use and \`batch\` both achieve one round trip instead of N — use whichever your provider supports best.`
+      : `\n\nEmit multiple \`tool_use\` blocks in the same assistant message for independent calls — the runtime executes them concurrently. Do not wait for one tool result before issuing the next independent call.`
+  }
+
 ### Command execution
 
 Never use \`sleep\` to wait for commands to complete. Commands run synchronously — when the shell tool returns, the command has finished.
@@ -347,6 +374,39 @@ export interface ResolvedInstructions {
   runtime: string;
 }
 
+/** render the heading list as an indented bullet TOC. ranges shown in
+ * parentheses (`(L3-L18)`); the start line is always the heading line
+ * itself, so reading the listed range gives the agent the heading +
+ * body together. shallowest heading depth in the body sits at the root
+ * column; deeper levels indent by `(depth - rootDepth) * 2` spaces. */
+export function renderLearningsToc(headings: LearningsHeading[]): string {
+  if (headings.length === 0) return "";
+  const rootDepth = Math.min(...headings.map((h) => h.depth));
+  return headings
+    .map((h) => {
+      const indent = " ".repeat((h.depth - rootDepth) * 2);
+      return `${indent}- ${h.title} (L${h.startLine}-L${h.endLine})`;
+    })
+    .join("\n");
+}
+
+/** assemble the LEARNINGS prompt section: file path + intro + either
+ * the rendered heading TOC (when the body has structure) or a no-headings
+ * affordance pointing the agent at the reflection turn for restructuring.
+ * empty string when the seed step failed and there's no path to surface. */
+export function buildLearningsSection(ctx: {
+  filePath: string | null;
+  headings: LearningsHeading[];
+}): string {
+  if (!ctx.filePath) return "";
+  const intro = `Repo-level learnings accumulated by previous agent runs live at \`${ctx.filePath}\`. Use this file as durable context (test commands, conventions, gotchas, architecture notes).`;
+  const tocBody =
+    ctx.headings.length === 0
+      ? "(no headings yet — file is empty or a flat list. read the whole file. during the post-run reflection turn, structure it with `## ` / `### ` headings so future runs can read targeted ranges.)"
+      : `Read targeted line ranges via your native file tool — do NOT slurp the whole file. Each range starts at the section heading line, so reading the range gives you heading + body together.\n\n${renderLearningsToc(ctx.headings)}`;
+  return `************* LEARNINGS *************\n\n${intro}\n\n${tocBody}`;
+}
+
 function assembleFullPrompt(ctx: {
   toc: string;
   task: string;
@@ -354,17 +414,18 @@ function assembleFullPrompt(ctx: {
   eventContext: string;
   system: string;
   learningsFilePath: string | null;
+  learningsHeadings: LearningsHeading[];
   runtime: string;
 }): string {
-  // the LEARNINGS section is intentionally tiny — just the file path and a
-  // one-line "read it" instruction. embedding the contents would re-inflate
-  // the prompt every run (the previous design's failure mode) and clutter
-  // CI logs. the agent reads the file with its native file tool; the
-  // post-run reflection turn (action/agents/postRun.ts) is where editing
-  // is encouraged, with the prune-stale framing.
-  const learningsSection = ctx.learningsFilePath
-    ? `************* LEARNINGS *************\n\nRepo-level learnings accumulated by previous agent runs live at \`${ctx.learningsFilePath}\`. Read this file early and let the entries inform your approach (test commands, conventions, gotchas, etc.). The file may be empty if no learnings have been collected yet.`
-    : "";
+  // server-parsed TOC is rendered inline so the agent can target line
+  // ranges via its native file tool. the file body itself is never
+  // inlined — that would re-inflate context every run and clutter CI
+  // logs. post-run reflection (action/agents/postRun.ts) is where
+  // editing is encouraged.
+  const learningsSection = buildLearningsSection({
+    filePath: ctx.learningsFilePath,
+    headings: ctx.learningsHeadings,
+  });
 
   const runtimeSection = `************* RUNTIME *************\n\n${ctx.runtime}`;
 
@@ -399,7 +460,10 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi
     tocEntries.push({ label: "EVENT CONTEXT", description: "related PR/issue data" });
   tocEntries.push({ label: "SYSTEM", description: "persona, security, tools, workflow rules" });
   if (pctx.learningsFilePath)
-    tocEntries.push({ label: "LEARNINGS", description: "repo-specific knowledge file path" });
+    tocEntries.push({
+      label: "LEARNINGS",
+      description: "repo-specific knowledge file path + heading TOC",
+    });
   tocEntries.push({ label: "RUNTIME", description: "environment metadata" });
 
   const toc = buildToc(tocEntries);
@@ -411,6 +475,7 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi
     eventContext,
     system,
     learningsFilePath: pctx.learningsFilePath,
+    learningsHeadings: pctx.learningsHeadings,
     runtime: pctx.runtime,
   });
 

utils/learnings.test.ts

@@ -1,4 +1,4 @@
-import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
@@ -20,51 +20,68 @@ describe("learnings tmpfile round-trip", () => {
     await rm(dir, { recursive: true, force: true });
   });
 
-  it("seeds with existing learnings and reads them back verbatim", async () => {
-    const current = "- run tests with `pnpm -r test`\n- default branch is `main`";
+  it("writes the verbatim DB body to disk and reads it back unchanged", async () => {
+    const current = [
+      "## Build & test",
+      "",
+      "- run tests with `pnpm -r test`",
+      "",
+      "## Architecture",
+      "",
+      "- workers in `worker/`",
+    ].join("\n");
     const path = await seedLearningsFile({ tmpdir: dir, current });
     expect(path).toBe(learningsFilePath(dir));
     expect(path.endsWith(LEARNINGS_FILE_NAME)).toBe(true);
-    const read = await readLearningsFile(path);
-    expect(read).toBe(current);
+
+    expect(await readFile(path, "utf8")).toBe(current);
+    expect(await readLearningsFile(path)).toBe(current);
   });
 
-  it("seeds an empty file when the repo has no learnings yet", async () => {
-    // empty seed (vs scaffold-with-comment) keeps the byte-trim equality
-    // gate clean: an untouched first run reads back as "" and persistLearnings
-    // skips the API round-trip rather than writing a placeholder string into
-    // Repo.learnings.
+  it("seeds an empty file when the repo has no learnings yet, round-trip is empty string", async () => {
     const path = await seedLearningsFile({ tmpdir: dir, current: null });
-    const read = await readLearningsFile(path);
-    expect(read).toBe("");
+    expect(await readFile(path, "utf8")).toBe("");
+    expect(await readLearningsFile(path)).toBe("");
   });
 
   it("returns null when the file is missing (treated as no-change by persist)", async () => {
-    const path = learningsFilePath(dir);
-    const read = await readLearningsFile(path);
-    expect(read).toBeNull();
+    expect(await readLearningsFile(learningsFilePath(dir))).toBeNull();
   });
 
-  it("trims whitespace so trailing newlines never trigger a spurious PATCH", async () => {
-    // editors commonly add a trailing newline on save. without trimming, a
-    // round-trip "read seed → save unchanged" would fail byte-equality and
-    // burn a LearningsRevision row on every run.
-    const current = "- one fact";
+  it("trims trailing whitespace so editor newlines never trigger a spurious PATCH", async () => {
+    const current = "## Build & test\n\n- one fact";
     const path = await seedLearningsFile({ tmpdir: dir, current });
     await writeFile(path, `${current}\n\n  `, "utf8");
-    const read = await readLearningsFile(path);
-    expect(read).toBe(current);
+    expect(await readLearningsFile(path)).toBe(current);
   });
 
-  it("truncates content over the 10k server-side cap", async () => {
-    // server enforces MAX_LEARNINGS_LENGTH = 10_000. truncating client-side
-    // avoids a 400 round-trip and keeps the bytes the agent will see in the
-    // next run aligned with what the server actually stored.
-    const oversized = "x".repeat(11_000);
+  it("truncates over-cap bodies at the last newline boundary so the next-seed TOC parse stays clean", async () => {
+    const padding = `${"x".repeat(80)}\n`.repeat(1300);
+    const oversized = `## Build & test\n\n${padding}`;
     const path = await seedLearningsFile({ tmpdir: dir, current: null });
     await writeFile(path, oversized, "utf8");
     const read = await readLearningsFile(path);
     expect(read).toBeTruthy();
-    expect(read?.length).toBe(10_000);
+    expect(read?.length).toBeLessThanOrEqual(100_000);
+    const tailLine = read?.split("\n").pop() ?? "";
+    expect(/^x+$/.test(tailLine)).toBe(true);
+    expect(tailLine.length).toBe(80);
+  });
+
+  it("falls back to a hard truncate when the only newline is far above the cap (giant single line)", async () => {
+    const oversized = `## Build & test\n${"x".repeat(110_000)}`;
+    const path = await seedLearningsFile({ tmpdir: dir, current: null });
+    await writeFile(path, oversized, "utf8");
+    const read = await readLearningsFile(path);
+    expect(read).toBeTruthy();
+    expect(read?.length).toBe(100_000);
+    expect(read?.startsWith("## Build & test\n")).toBe(true);
+  });
+
+  it("preserves legacy free-text without scaffolding or wrapping", async () => {
+    const legacy = "- this is some old free-text bullet\n- another one";
+    const path = await seedLearningsFile({ tmpdir: dir, current: legacy });
+    expect(await readFile(path, "utf8")).toBe(legacy);
+    expect(await readLearningsFile(path)).toBe(legacy);
   });
 });

utils/learnings.ts

@@ -7,47 +7,75 @@ import { dirname, join } from "node:path";
  * back into future runs as durable context. Modeled on the PR-summary tmpfile
  * pattern (see action/utils/prSummary.ts):
  *
- *   1. server seeds `pullfrog-learnings.md` from `Repo.learnings` (or empty
- *      when the repo has none yet)
- *   2. the agent reads the file at startup as part of its context, and may
- *      edit it in place at end-of-run when prompted by the reflection turn
- *   3. main.ts reads the file back at end-of-run and PATCHes
- *      `/api/repo/[owner]/[repo]/learnings` if it changed (byte-trim equality
- *      against the seed determines change detection)
+ *   1. server seeds `pullfrog-learnings.md` with the verbatim body of
+ *      `Repo.learnings` (or empty for fresh repos), and parses headings
+ *      server-side (`utils/learningsToc.ts`) — the parsed TOC is rendered
+ *      into the LEARNINGS prompt section, not into the file
+ *   2. the agent reads the TOC in the prompt and uses listed line ranges
+ *      to read just the sections relevant to the current task — file can
+ *      grow large, but only targeted ranges hit the agent's context
+ *   3. agent edits the file in place at end-of-run during the reflection
+ *      turn (see action/agents/postRun.ts buildLearningsReflectionPrompt)
+ *   4. main.ts reads the file back at end-of-run and PATCHes
+ *      `/api/repo/[owner]/[repo]/learnings` if the body changed
  *
  * Edit-in-place avoids stuffing the entire learnings list into both the
  * prompt context and an `update_learnings` MCP tool call (which previously
  * required passing the FULL merged list as a string parameter — an
  * output-token tax that grew linearly with the learnings size).
+ *
+ * Section structure is agent-curated. The reflection prompt teaches
+ * hierarchy + a soft 300-line-per-section cap to keep TOC ranges
+ * agent-targetable on long-lived repos; there is no fixed taxonomy.
  */
 
 export const LEARNINGS_FILE_NAME = "pullfrog-learnings.md";
 
 /** server-side cap mirrors `MAX_LEARNINGS_LENGTH` in
  * `app/api/repo/[owner]/[repo]/learnings/route.ts`. truncating client-side
- * keeps the PATCH from being rejected with a 400. */
-const MAX_LEARNINGS_LENGTH = 10_000;
+ * keeps the PATCH from being rejected with a 400. raised from 10k → 100k
+ * once the TOC affordance landed: with line-range reads via the
+ * server-parsed TOC the agent doesn't ingest the whole file, so the cap
+ * can grow to whatever curation discipline allows. 100k holds ~400-500
+ * short bullets. */
+const MAX_LEARNINGS_LENGTH = 100_000;
 
 export function learningsFilePath(tmpdir: string): string {
   return join(tmpdir, LEARNINGS_FILE_NAME);
 }
 
-/** seed the learnings file with the repo's current learnings, or an empty
- * file when the repo has none yet. returns the absolute path. */
+/** seed the rolling learnings tmpfile with the verbatim DB body (or empty
+ * string for fresh repos). returns the absolute path. the parsed TOC is
+ * carried separately via `RepoSettings.learningsHeadings` and rendered
+ * into the prompt by `resolveInstructions`, so the file on disk is just
+ * the body — no markers, no scaffold, no in-file TOC. */
 export async function seedLearningsFile(params: {
   tmpdir: string;
   current: string | null;
 }): Promise<string> {
   const path = learningsFilePath(params.tmpdir);
   await mkdir(dirname(path), { recursive: true });
-  // empty file when no learnings exist yet — the agent reads it, sees
-  // nothing, and the LEARNINGS prompt section explains what the file is for.
-  // a header comment would risk being persisted as part of the first real
-  // edit, polluting the DB row with placeholder text.
   await writeFile(path, params.current ?? "", "utf8");
   return path;
 }
 
+/** truncate at the last newline boundary before `cap` so we don't leave
+ * a partial line at the tail (a half-truncated `## Headi` confuses the
+ * server's next-seed TOC parse and shrinks visible structure). falls
+ * back to a hard `slice` when the line boundary would discard a large
+ * run of content — i.e. when the tail of `head` is one giant line (rare:
+ * minified pastes, fenced log dumps). losing a partial last line is
+ * preferable to losing kilobytes of body. */
+const TRUNCATION_LINE_BOUNDARY_TOLERANCE = 4096;
+function truncateAtLineBoundary(body: string, cap: number): string {
+  if (body.length <= cap) return body;
+  const head = body.slice(0, cap);
+  const lastNewline = head.lastIndexOf("\n");
+  if (lastNewline <= 0) return head;
+  if (cap - lastNewline > TRUNCATION_LINE_BOUNDARY_TOLERANCE) return head;
+  return head.slice(0, lastNewline);
+}
+
 /** read the agent-edited learnings file. returns null when the file is
  * missing or unreadable (treated as "no change"). caps content at the
  * server's max length to avoid a 400 round-trip. */
@@ -58,7 +86,5 @@ export async function readLearningsFile(path: string): Promise<string | null> {
   } catch {
     return null;
   }
-  const trimmed = raw.trim();
-  if (trimmed.length > MAX_LEARNINGS_LENGTH) return trimmed.slice(0, MAX_LEARNINGS_LENGTH);
-  return trimmed;
+  return truncateAtLineBoundary(raw.trim(), MAX_LEARNINGS_LENGTH);
 }

utils/learningsTocRender.test.ts

@@ -0,0 +1,99 @@
+import { describe, expect, it } from "vitest";
+import { buildLearningsSection, renderLearningsToc } from "./instructions.ts";
+import type { LearningsHeading } from "./runContext.ts";
+
+const h = (depth: 1 | 2 | 3 | 4 | 5 | 6, title: string, startLine: number, endLine: number) => ({
+  depth,
+  title,
+  startLine,
+  endLine,
+});
+
+describe("renderLearningsToc", () => {
+  it("renders flat h2 list with parenthesized ranges, no hashes or backticks", () => {
+    const headings: LearningsHeading[] = [
+      h(2, "Build & test", 1, 18),
+      h(2, "Architecture", 19, 60),
+    ];
+    expect(renderLearningsToc(headings)).toBe(
+      `- Build & test (L1-L18)
+- Architecture (L19-L60)`
+    );
+  });
+
+  it("indents deeper headings 2 spaces per depth level past the shallowest", () => {
+    const headings: LearningsHeading[] = [
+      h(2, "Build & test", 1, 42),
+      h(3, "Local", 3, 18),
+      h(3, "CI", 19, 42),
+      h(2, "Architecture", 43, 210),
+      h(3, "Background workers", 80, 210),
+    ];
+    expect(renderLearningsToc(headings)).toBe(
+      `- Build & test (L1-L42)
+  - Local (L3-L18)
+  - CI (L19-L42)
+- Architecture (L43-L210)
+  - Background workers (L80-L210)`
+    );
+  });
+
+  it("treats the shallowest depth as the root column when no h2 is present", () => {
+    const headings: LearningsHeading[] = [h(3, "Only h3", 1, 5), h(4, "Sub h4", 2, 5)];
+    expect(renderLearningsToc(headings)).toBe(
+      `- Only h3 (L1-L5)
+  - Sub h4 (L2-L5)`
+    );
+  });
+
+  it("supports depths up through h6 with stable 2-space indent steps", () => {
+    const headings: LearningsHeading[] = [
+      h(2, "Two", 1, 10),
+      h(3, "Three", 2, 10),
+      h(4, "Four", 3, 10),
+      h(5, "Five", 4, 10),
+      h(6, "Six", 5, 10),
+    ];
+    expect(renderLearningsToc(headings)).toBe(
+      `- Two (L1-L10)
+  - Three (L2-L10)
+    - Four (L3-L10)
+      - Five (L4-L10)
+        - Six (L5-L10)`
+    );
+  });
+});
+
+describe("buildLearningsSection", () => {
+  it("returns empty string when no file path (seed step failed)", () => {
+    expect(buildLearningsSection({ filePath: null, headings: [] })).toBe("");
+  });
+
+  it("renders the no-headings affordance when the body has no structure", () => {
+    const out = buildLearningsSection({
+      filePath: "/tmp/run-1/pullfrog-learnings.md",
+      headings: [],
+    });
+    expect(out).toContain("************* LEARNINGS *************");
+    expect(out).toContain("/tmp/run-1/pullfrog-learnings.md");
+    expect(out).toContain("no headings yet");
+    expect(out).toContain("structure it with");
+    // does not include a TOC list when there are no headings
+    expect(out).not.toMatch(/\(L\d+-L\d+\)/);
+  });
+
+  it("renders the TOC inline with the file path and heading guidance", () => {
+    const out = buildLearningsSection({
+      filePath: "/tmp/run-1/pullfrog-learnings.md",
+      headings: [h(2, "Build & test", 1, 18), h(2, "Architecture", 19, 60)],
+    });
+    expect(out).toContain("************* LEARNINGS *************");
+    expect(out).toContain("/tmp/run-1/pullfrog-learnings.md");
+    expect(out).toContain("- Build & test (L1-L18)");
+    expect(out).toContain("- Architecture (L19-L60)");
+    expect(out).toContain("Each range starts at the section heading line");
+    // explicit "no hashes, no backticks" in the rendered list
+    expect(out).not.toContain("- `## Build");
+    expect(out).not.toContain("`## Build");
+  });
+});

utils/normalizeEnv.test.ts

@@ -0,0 +1,91 @@
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { normalizeEnv, sanitizeSecret } from "./normalizeEnv.ts";
+
+/**
+ * These tests pin the load-bearing invariants of secret sanitisation:
+ *   - sensitive values are trimmed before downstream code reads them
+ *   - whitespace-only values are NOT silently zeroed (leave env unchanged)
+ *   - case normalisation still happens
+ *
+ * Masking (`core.setSecret`) is delegated to `@actions/core` and trusted to
+ * work as documented — we don't spy on stdout to re-test the toolkit.
+ */
+
+describe("normalizeEnv: process.env state contract", () => {
+  let originalEnv: NodeJS.ProcessEnv;
+
+  beforeEach(() => {
+    // normalizeEnv() iterates the entire process.env, so the test must
+    // control it. snapshot + full wipe + restore is the cleanest isolation.
+    originalEnv = { ...process.env };
+    for (const k of Object.keys(process.env)) delete process.env[k];
+  });
+
+  afterEach(() => {
+    for (const k of Object.keys(process.env)) delete process.env[k];
+    Object.assign(process.env, originalEnv);
+  });
+
+  it("trims trailing newline from sensitive env vars", () => {
+    process.env.ANTHROPIC_API_KEY = "sk-ant-secret-value\n";
+    normalizeEnv();
+    expect(process.env.ANTHROPIC_API_KEY).toBe("sk-ant-secret-value");
+  });
+
+  it("trims surrounding whitespace including \\r\\n and spaces", () => {
+    process.env.OPENAI_API_KEY = "  sk-openai-value\r\n  ";
+    normalizeEnv();
+    expect(process.env.OPENAI_API_KEY).toBe("sk-openai-value");
+  });
+
+  it("leaves clean sensitive values untouched", () => {
+    process.env.ANTHROPIC_API_KEY = "sk-ant-clean";
+    normalizeEnv();
+    expect(process.env.ANTHROPIC_API_KEY).toBe("sk-ant-clean");
+  });
+
+  it("ignores non-sensitive env vars", () => {
+    process.env.NODE_ENV = "production\n";
+    normalizeEnv();
+    expect(process.env.NODE_ENV).toBe("production\n");
+  });
+
+  it("canonicalises case and trims the value", () => {
+    process.env.anthropic_api_key = "sk-ant-lowercase\n";
+    normalizeEnv();
+    expect(process.env.ANTHROPIC_API_KEY).toBe("sk-ant-lowercase");
+    expect(process.env.anthropic_api_key).toBeUndefined();
+  });
+
+  it("preserves whitespace-only values rather than silently zeroing them", () => {
+    // contract: don't mutate when value is whitespace-only. caller sees the
+    // misconfigured value verbatim and either fails clearly downstream or
+    // logs a missing-key error.
+    process.env.ANTHROPIC_API_KEY = "   \n  ";
+    normalizeEnv();
+    expect(process.env.ANTHROPIC_API_KEY).toBe("   \n  ");
+  });
+
+  it("preserves embedded newlines (toolkit masks each line)", () => {
+    // multi-line PEMs aren't used in practice, but if one slipped in via a
+    // DB secret we don't want to silently mutate it. trim() only touches
+    // the ends; @actions/core handles per-line masking via the runner.
+    process.env.ANTHROPIC_API_KEY = "line1\nline2";
+    normalizeEnv();
+    expect(process.env.ANTHROPIC_API_KEY).toBe("line1\nline2");
+  });
+});
+
+describe("sanitizeSecret return value", () => {
+  it("returns the trimmed value for a sensitive secret with trailing newline", () => {
+    expect(sanitizeSecret("ANTHROPIC_API_KEY", "sk-ant-secret\n")).toBe("sk-ant-secret");
+  });
+
+  it("returns the value unchanged when no trimming is needed", () => {
+    expect(sanitizeSecret("ANTHROPIC_API_KEY", "sk-ant-clean")).toBe("sk-ant-clean");
+  });
+
+  it("returns null for whitespace-only input so caller can skip injection", () => {
+    expect(sanitizeSecret("ANTHROPIC_API_KEY", "   \n")).toBeNull();
+  });
+});

utils/normalizeEnv.ts

@@ -1,11 +1,40 @@
+import * as core from "@actions/core";
 import { log } from "./cli.ts";
 import { isSensitiveEnvName } from "./secrets.ts";
 
-function maskValue(value: string | undefined) {
-  if (value && typeof value === "string" && value.trim().length > 0) {
-    // ::add-mask::value tells GitHub Actions to mask this value in logs
-    console.log(`::add-mask::${value}`);
+/**
+ * Trim surrounding whitespace from a sensitive value and register it as a
+ * GitHub Actions log mask. Trailing newlines from terminal-copy paste are a
+ * common footgun: the value travels through GH Actions logs and any tool
+ * that re-emits parts of it leaks the unmasked tail. Trimming canonicalises
+ * the value so the mask matches exactly what downstream tools will print.
+ *
+ * Masking is delegated to `core.setSecret` (not raw `console.log`) so the
+ * toolkit percent-encodes `\r`/`\n`; the runner V2 parser decodes them and
+ * registers the full value plus every non-empty line as separate masks. That
+ * keeps us safe for embedded-newline values (PEMs, kubeconfigs, JSON blobs)
+ * even though they aren't currently used.
+ *
+ * Returns the trimmed value, or `null` when the input was whitespace-only —
+ * callers must leave `process.env` untouched in that case so a misconfigured
+ * value surfaces as a clear "missing key" downstream rather than silently
+ * mutating to the empty string.
+ */
+export function sanitizeSecret(key: string, value: string): string | null {
+  const trimmed = value.trim();
+  if (trimmed.length === 0) {
+    log.warning(
+      `» ${key} is whitespace-only — leaving env var unchanged. check your secret value.`
+    );
+    return null;
   }
+  if (trimmed !== value) {
+    log.warning(
+      `» stripped whitespace from ${key} (whitespace in secret values breaks GitHub Actions log masking)`
+    );
+  }
+  core.setSecret(trimmed);
+  return trimmed;
 }
 
 /**
@@ -15,7 +44,8 @@ function maskValue(value: string | undefined) {
  * If there are conflicts (same key with different capitalizations but different values),
  * logs a warning and keeps the uppercase version.
  *
- * Also registers sensitive values as masks in GitHub Actions.
+ * Also trims and masks sensitive values so accidental trailing whitespace
+ * doesn't defeat GitHub Actions log masking.
  */
 export function normalizeEnv(): void {
   const upperKeys = new Map<string, string[]>();
@@ -30,14 +60,6 @@ export function normalizeEnv(): void {
 
   // process each group
   for (const [upperKey, keys] of upperKeys) {
-    // if sensitive, ensure we mask the value (regardless of whether we rename it or not)
-    if (isSensitiveEnvName(upperKey)) {
-      // mask all values associated with this key group
-      for (const key of keys) {
-        maskValue(process.env[key]);
-      }
-    }
-
     if (keys.length === 1) {
       const key = keys[0];
       if (key !== upperKey) {
@@ -71,4 +93,14 @@ export function normalizeEnv(): void {
     // set the uppercase version
     process.env[upperKey] = preferredValue;
   }
+
+  // trim + mask sensitive values after case normalisation so each key is
+  // visited exactly once with its final, canonical value
+  for (const key of Object.keys(process.env)) {
+    if (!isSensitiveEnvName(key)) continue;
+    const value = process.env[key];
+    if (typeof value !== "string" || value.length === 0) continue;
+    const sanitized = sanitizeSecret(key, value);
+    if (sanitized !== null) process.env[key] = sanitized;
+  }
 }

utils/payload.ts

@@ -21,6 +21,7 @@ export const JsonPayload = type({
   "triggerer?": "string | undefined",
 
   "eventInstructions?": "string",
+  "previousRunsNote?": "string",
   "event?": "object",
   "timeout?": "string | undefined",
   "progressComment?": type({
@@ -157,6 +158,7 @@ export function resolvePayload(
       // it's not a common use case but GITHUB_ACTOR can be a user when the workflow is manually triggered by a user through GitHub Actions UI
       (!isPullfrog(process.env.GITHUB_ACTOR) ? process.env.GITHUB_ACTOR : undefined),
     eventInstructions: jsonPayload?.eventInstructions,
+    previousRunsNote: jsonPayload?.previousRunsNote,
     event,
     timeout: inputs.timeout ?? jsonPayload?.timeout,
     cwd: resolveCwd(inputs.cwd),

utils/runContext.ts

@@ -9,6 +9,22 @@ export interface Mode {
   prompt: string;
 }
 
+/**
+ * server-parsed TOC entry for `Repo.learnings`. depth is 1-6 (h1-h6),
+ * line numbers are 1-indexed against the raw body. computed by
+ * `parseLearningsHeadings` in `utils/learningsToc.ts` (server side) and
+ * shipped over the run-context JSON boundary; the canonical declaration
+ * lives there. duplicated here because the action runtime can't reach
+ * across into the proprietary root-level codebase, and the JSON wire
+ * means typecheck can't enforce shape equality across both sides.
+ */
+export interface LearningsHeading {
+  depth: 1 | 2 | 3 | 4 | 5 | 6;
+  title: string;
+  startLine: number;
+  endLine: number;
+}
+
 export interface RepoSettings {
   model: string | null;
   modes: Mode[];
@@ -21,6 +37,7 @@ export interface RepoSettings {
   prApproveEnabled: boolean;
   modeInstructions: Record<string, string>;
   learnings: string | null;
+  learningsHeadings: LearningsHeading[];
   envAllowlist: string | null;
 }
 
@@ -61,6 +78,7 @@ const defaultSettings: RepoSettings = {
   prApproveEnabled: false,
   modeInstructions: {},
   learnings: null,
+  learningsHeadings: [],
   envAllowlist: null,
 };
 
@@ -88,7 +106,6 @@ export async function fetchRunContext(params: {
   try {
     const headers: Record<string, string> = {
       Authorization: `Bearer ${params.token}`,
-      "Content-Type": "application/json",
     };
     if (params.oidcToken) {
       headers["X-GitHub-OIDC-Token"] = params.oidcToken;
@@ -128,6 +145,7 @@ export async function fetchRunContext(params: {
         postCheckoutScript: data.settings?.postCheckoutScript ?? null,
         prepushScript: data.settings?.prepushScript ?? null,
         stopScript: data.settings?.stopScript ?? null,
+        learningsHeadings: data.settings?.learningsHeadings ?? [],
       },
       apiToken: data.apiToken,
       oss: data.oss ?? false,

utils/subprocess.test.ts

@@ -1,6 +1,6 @@
 import { performance } from "node:perf_hooks";
 import { describe, expect, it } from "vitest";
-import { spawn } from "./subprocess.ts";
+import { spawn, TailBuffer } from "./subprocess.ts";
 
 describe("spawn error path", () => {
   it("surfaces ENOENT-style spawn failures in stderr so callers can diagnose", async () => {
@@ -79,6 +79,76 @@ describe("spawn error path", () => {
     expect(elapsed).toBeLessThan(10_000);
   }, 20_000);
 
+  it('retain:"tail" caps stderr at maxRetainedBytes and prepends a truncation sentinel', async () => {
+    // regression for issue #680: unbounded `stderrBuffer += chunk` previously
+    // crashed the wrapper with `RangeError: Invalid string length` once V8's
+    // ~1 GiB kMaxLength was breached on long-lived agent runs. the fix caps
+    // retention with a TailBuffer; this test exercises the cap end-to-end by
+    // emitting ~2 MiB of stderr against a 256 KiB ceiling and asserts the
+    // wrapper does not crash, the result is bounded, and the sentinel is
+    // present so downstream consumers can detect the truncation.
+    const result = await spawn({
+      cmd: "bash",
+      // print ~2 MiB to stderr in 64 KiB chunks. `yes` + head gives us a
+      // reliable byte budget that's well above the 256 KiB cap below.
+      args: ["-c", "yes ABCDEFGH | head -c 2097152 1>&2"],
+      env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "" },
+      activityTimeout: 0,
+      maxRetainedBytes: 256 * 1024,
+    });
+
+    expect(result.exitCode).toBe(0);
+    expect(result.stderr).toMatch(/truncated by retain:tail cap/);
+    expect(result.stderr.length).toBeLessThan(256 * 1024 + 200);
+  }, 15_000);
+
+  it('retain:"none" returns empty stdout/stderr regardless of child output', async () => {
+    // long-lived agent callers (opencode, claude) drain via onStdout/onStderr
+    // and never read result.stdout/result.stderr — they pass retain:"none"
+    // to skip the per-chunk concatenation entirely. assert that contract:
+    // empty strings out, but onStdout still fires.
+    const chunks: string[] = [];
+    const result = await spawn({
+      cmd: "bash",
+      args: ["-c", "echo hello; echo world 1>&2"],
+      env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "" },
+      activityTimeout: 0,
+      retain: "none",
+      onStdout: (chunk) => chunks.push(chunk),
+    });
+
+    expect(result.exitCode).toBe(0);
+    expect(result.stdout).toBe("");
+    expect(result.stderr).toBe("");
+    expect(chunks.join("")).toContain("hello");
+  });
+
+  it('retain defaults to "tail" so short-lived callers keep failure-surfacing snapshots', async () => {
+    // lock the default explicitly. gitAuth, package installs, and lifecycle
+    // hooks all rely on `result.stderr` being non-empty on failure — flipping
+    // the default to "none" would silently break their error messages while
+    // all other tests in this file kept passing.
+    const result = await spawn({
+      cmd: "bash",
+      args: ["-c", "echo -n diagnostic-output 1>&2; exit 7"],
+      env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "" },
+      activityTimeout: 0,
+    });
+
+    expect(result.exitCode).toBe(7);
+    expect(result.stderr).toBe("diagnostic-output");
+  });
+
+  it("TailBuffer drops oldest bytes once the cap is exceeded", () => {
+    const buf = new TailBuffer(10);
+    buf.append("0123456789");
+    expect(buf.toString()).toBe("0123456789");
+    buf.append("abcde");
+    // 0-9 plus abcde = 15 chars; cap is 10, so we keep the last 10 = "56789abcde"
+    expect(buf.toString()).toMatch(/truncated by retain:tail cap/);
+    expect(buf.toString()).toContain("56789abcde");
+  });
+
   it("reports signal-killed subprocesses as failures, not success", async () => {
     // regression: before the fix, `child.on("close", (exitCode) => ...)`
     // discarded the signal parameter and `exitCode || 0` coerced the

utils/subprocess.ts

@@ -88,6 +88,29 @@ function installSignalHandler(): void {
   });
 }
 
+/**
+ * Controls what the wrapper retains in memory across the child's lifetime
+ * for the post-hoc `SpawnResult.stdout` / `SpawnResult.stderr` snapshots.
+ *
+ * Streaming callbacks (`onStdout` / `onStderr`) fire regardless — `retain`
+ * only governs the buffered snapshot returned in `SpawnResult`.
+ *
+ * - `"tail"` (default): keep the last `maxRetainedBytes` UTF-16 code units
+ *   of each stream. Once the cap is exceeded, oldest bytes are sliced off
+ *   and the result is prefixed with a `... [N MiB truncated] ...` sentinel.
+ *   Right default for short-lived commands whose failure mode is in their
+ *   final output (git errors, install failures, hook scripts).
+ * - `"none"`: skip the buffer entirely. `SpawnResult.stdout` / `.stderr`
+ *   are empty strings. Use this for long-lived streaming agents that already
+ *   drain via `onStdout` / `onStderr` and never read the buffered snapshot.
+ *
+ * Default cap is 8 MiB — well below V8's ~1 GiB `kMaxLength` so `+= chunk`
+ * can never throw `RangeError: Invalid string length`.
+ */
+export type RetainMode = "tail" | "none";
+
+export const DEFAULT_MAX_RETAINED_BYTES = 8 * 1024 * 1024;
+
 export interface SpawnOptions {
   cmd: string;
   args: string[];
@@ -115,6 +138,46 @@ export interface SpawnOptions {
   // open, keeps emitting NDJSON, and `child.on("close")` never fires —
   // producing zombie runs that hang until the GitHub Actions job timeout).
   killGroup?: boolean;
+  retain?: RetainMode;
+  maxRetainedBytes?: number;
+}
+
+/**
+ * Bounded string accumulator that keeps the tail of appended chunks.
+ * Once the cap is exceeded, oldest bytes are sliced off and `toString()`
+ * prefixes the survivors with a sentinel describing the elided byte count.
+ *
+ * Exported because long-lived agent runtimes (opencode, claude) also
+ * accumulate per-run narration strings independently of the spawn wrapper
+ * and need the same protection against V8's `kMaxLength`.
+ */
+export class TailBuffer {
+  // explicit field declarations rather than constructor parameter properties:
+  // node's strip-only TS loader (used by action/test/run.ts in CI) rejects
+  // `constructor(private readonly cap: number)` with ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX.
+  private readonly cap: number;
+  private buffer = "";
+  private truncatedBytes = 0;
+
+  constructor(cap: number) {
+    this.cap = cap;
+  }
+
+  append(chunk: string): void {
+    if (this.cap <= 0) return;
+    this.buffer += chunk;
+    if (this.buffer.length > this.cap) {
+      const drop = this.buffer.length - this.cap;
+      this.truncatedBytes += drop;
+      this.buffer = this.buffer.slice(drop);
+    }
+  }
+
+  toString(): string {
+    if (this.truncatedBytes === 0) return this.buffer;
+    const mib = (this.truncatedBytes / 1024 / 1024).toFixed(1);
+    return `... [${mib} MiB truncated by retain:tail cap] ...\n${this.buffer}`;
+  }
 }
 
 export interface SpawnResult {
@@ -133,8 +196,15 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
   installSignalHandler();
 
   const startTime = performance.now();
-  let stdoutBuffer = "";
-  let stderrBuffer = "";
+  // capped accumulators — unbounded `+= chunk` previously crashed the wrapper
+  // with `RangeError: Invalid string length` once V8's ~1 GiB kMaxLength was
+  // breached on long-lived agent subprocesses (e.g. multi-lens opencode
+  // Reviews on large monorepos). retain:"none" skips the buffer entirely
+  // for callers that already drain via onStdout/onStderr.
+  const retain: RetainMode = options.retain ?? "tail";
+  const cap = options.maxRetainedBytes ?? DEFAULT_MAX_RETAINED_BYTES;
+  const stdoutBuffer = retain === "none" ? null : new TailBuffer(cap);
+  const stderrBuffer = retain === "none" ? null : new TailBuffer(cap);
 
   const killGroup = options.killGroup ?? false;
 
@@ -234,20 +304,46 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
       lastActivityTime = performance.now();
     }
 
+    // wrap handlers in try/catch as defense in depth for synchronous throws
+    // inside the listener body. the historical `+= chunk` RangeError was such
+    // a throw — synchronous and fatal under node's default uncaught-exception
+    // policy. with the TailBuffer cap in place the wrapper-side `append` can
+    // no longer throw, but the catch keeps protecting against any future
+    // synchronous regression in this path.
+    //
+    // note: this does NOT catch rejections from async user callbacks —
+    // `options.onStdout?.(chunk)` returns a Promise in the agent callers
+    // (claude.ts, opencode.ts) and a throw inside an async callback surfaces
+    // as an unhandled Promise rejection, not a synchronous exception. agent
+    // callers handle their own NDJSON-parse failures internally; the
+    // synchronous protection here is what matters for the RangeError class
+    // of bugs (issue #680).
     if (child.stdout) {
       child.stdout.on("data", (data: Buffer) => {
-        updateActivity();
-        const chunk = data.toString();
-        stdoutBuffer += chunk;
-        options.onStdout?.(chunk);
+        try {
+          updateActivity();
+          const chunk = data.toString();
+          stdoutBuffer?.append(chunk);
+          options.onStdout?.(chunk);
+        } catch (err) {
+          log.debug(
+            `spawn stdout handler threw: ${err instanceof Error ? err.message : String(err)}`
+          );
+        }
       });
     }
 
     if (child.stderr) {
       child.stderr.on("data", (data: Buffer) => {
-        const chunk = data.toString();
-        stderrBuffer += chunk;
-        options.onStderr?.(chunk);
+        try {
+          const chunk = data.toString();
+          stderrBuffer?.append(chunk);
+          options.onStderr?.(chunk);
+        } catch (err) {
+          log.debug(
+            `spawn stderr handler threw: ${err instanceof Error ? err.message : String(err)}`
+          );
+        }
       });
     }
 
@@ -289,7 +385,7 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
       // appeared to succeed when they'd actually been killed — caller
       // checked `result.exitCode !== 0` and moved on.
       let resolvedExitCode = exitCode ?? 0;
-      let resolvedStderr = stderrBuffer;
+      let resolvedStderr = stderrBuffer?.toString() ?? "";
       if (exitCode === null && signal) {
         const killMsg = `[spawn] ${options.cmd}: killed by signal ${signal}`;
         resolvedStderr = resolvedStderr ? `${resolvedStderr}\n${killMsg}` : killMsg;
@@ -297,7 +393,7 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
       }
 
       resolve({
-        stdout: stdoutBuffer,
+        stdout: stdoutBuffer?.toString() ?? "",
         stderr: resolvedStderr,
         exitCode: resolvedExitCode,
         durationMs,
@@ -319,11 +415,12 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
       // per the guidance, and hit the same wall every run.
       const errMsg = `[spawn] ${options.cmd}: ${error.message}`;
       console.error(errMsg);
-      stderrBuffer = stderrBuffer ? `${stderrBuffer}\n${errMsg}` : errMsg;
+      const existingStderr = stderrBuffer?.toString() ?? "";
+      const finalStderr = existingStderr ? `${existingStderr}\n${errMsg}` : errMsg;
 
       resolve({
-        stdout: stdoutBuffer,
-        stderr: stderrBuffer,
+        stdout: stdoutBuffer?.toString() ?? "",
+        stderr: finalStderr,
         exitCode: 1,
         durationMs,
       });

utils/timer.test.ts

@@ -203,5 +203,18 @@ describe("ThinkingTimer", () => {
       expect(cli.log.info).toHaveBeenNthCalledWith(1, "» thought for 4 seconds");
       expect(cli.log.info).toHaveBeenNthCalledWith(2, "» thought for 5 seconds");
     });
+
+    it("routes log lines through the optional formatLine for per-session prefixing", () => {
+      const startTime = 1000000;
+      vi.mocked(performance.now)
+        .mockReturnValueOnce(startTime) // markToolResult
+        .mockReturnValueOnce(startTime + 4000); // markToolCall
+
+      const timer = new ThinkingTimer((line) => `[lens:security] ${line}`);
+      timer.markToolResult();
+      timer.markToolCall();
+
+      expect(cli.log.info).toHaveBeenCalledWith("[lens:security] » thought for 4 seconds");
+    });
   });
 });

utils/timer.ts

@@ -22,6 +22,15 @@ export class Timer {
 
 const THINKING_THRESHOLD = 3000; // ms
 
+/**
+ * Measures wall-clock gap between the last tool_result and the next tool_call,
+ * surfacing it as a "thought for Xs" log when over `THINKING_THRESHOLD`.
+ *
+ * Use one instance per logical session (orchestrator, each subagent) — sharing
+ * a single timer across sessions conflates cross-session interleaving as
+ * thinking time. The optional `formatLine` lets the caller prefix output with
+ * a session label so attribution is visible in the merged log stream.
+ */
 export class ThinkingTimer {
   private readonly durationFormatter = new Intl.NumberFormat("en-US", {
     style: "unit",
@@ -32,21 +41,32 @@ export class ThinkingTimer {
   });
 
   private lastToolResultTimestamp: number | null = null;
+  private readonly formatLine: (line: string) => string;
+
+  // node's native TS strip-only mode does not support parameter properties,
+  // so the formatter is declared as a field and assigned in the body.
+  constructor(formatLine: (line: string) => string = (l) => l) {
+    this.formatLine = formatLine;
+  }
 
   markToolResult(): void {
     this.lastToolResultTimestamp = performance.now();
-    log.debug(`» thinking timer: markToolResult at ${this.lastToolResultTimestamp}`);
+    log.debug(
+      this.formatLine(`» thinking timer: markToolResult at ${this.lastToolResultTimestamp}`)
+    );
   }
 
   markToolCall(): void {
     const now = performance.now();
     log.debug(
-      `» thinking timer: markToolCall at ${now}, lastToolResult=${this.lastToolResultTimestamp}`
+      this.formatLine(
+        `» thinking timer: markToolCall at ${now}, lastToolResult=${this.lastToolResultTimestamp}`
+      )
     );
     if (this.lastToolResultTimestamp === null) return;
     const elapsed = now - this.lastToolResultTimestamp;
     if (elapsed < THINKING_THRESHOLD) return;
     const seconds = elapsed / 1000;
-    log.info(`» thought for ${this.durationFormatter.format(seconds)}`);
+    log.info(this.formatLine(`» thought for ${this.durationFormatter.format(seconds)}`));
   }
 }