action: bump to 0.1.6

opencode: opt into `experimental.batch_tool` (anomalyco/opencode#2983) so the
`batch` tool registers and the model can bundle 1-25 independent calls into one
round trip. edit calls are excluded upstream.

instructions.ts: add a "Parallel tool execution" section to the SYSTEM Workflow
block, agent-specialized via ctx.agentId. uses Anthropic's canonical wording
("invoke all relevant tools simultaneously...") so Claude reliably emits multiple
tool_use blocks per message; tells OpenCode about the new `batch` affordance.

verified end-to-end against haiku-class models (sonnet for claude, default for
opencode) with a "read 3 files and report first lines" fixture. results:
- opencode used `batch` with 3 nested reads AND emitted 3 native parallel
  read calls in the same assistant turn
- claude went from 3 serial turns (1 read each) to 1 message with 3 parallel
  Read tool_use blocks

.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/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

@@ -18,17 +18,23 @@ import { performance } from "node:perf_hooks";
 import { pullfrogMcpName } from "../external.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,
@@ -114,13 +120,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[];
@@ -138,6 +152,8 @@ interface ClaudeAssistantEvent {
 
 interface ClaudeUserEvent {
   type: "user";
+  session_id?: string;
+  parent_tool_use_id?: string | null;
   message?: {
     role?: string;
     content?: ContentBlock[];
@@ -236,7 +252,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;
@@ -280,18 +326,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) {
@@ -300,23 +358,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})` : "")
+              )
             );
           }
 
@@ -326,8 +395,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);
           }
         }
@@ -348,10 +423,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"
@@ -369,9 +446,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}`));
           }
         }
       }
@@ -465,7 +542,8 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
 
   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 {
@@ -483,9 +561,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;
@@ -595,20 +678,26 @@ 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) : "";
       const errorMessage =
         lastResultError ||
-        result.stderr ||
+        stderrSnapshot ||
         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,
@@ -618,7 +707,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,
@@ -628,14 +717,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;
@@ -661,7 +750,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,

agents/opencode.ts

@@ -22,7 +22,13 @@ 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";
@@ -59,6 +65,7 @@ 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;
@@ -114,6 +121,15 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s
       [pullfrogMcpName]: { type: "remote", url: ctx.mcpServerUrl },
     },
     agent: buildReviewerAgentConfig(),
+    // 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(
@@ -377,7 +393,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 +424,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 +666,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 +703,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 +904,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 +928,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 +942,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 +1067,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 +1104,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 +1136,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
 
     return {
       success: false,
-      output: finalOutput || output,
+      output: finalOutput || output.toString(),
       error: `${errorMessage} [${diagnosis}]`,
       usage: buildUsage(),
     };

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;

main.ts

@@ -30,7 +30,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";
@@ -559,12 +559,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;

modes.ts

@@ -207,6 +207,8 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
    - **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.
 
+   **lens-add discipline.** Each lens needs to clear a specific bar before you dispatch it: name the concrete failure mode this lens would catch *that the diff plausibly introduces*, in one sentence. "Could apply", "good to have", "for completeness" do not qualify. If you can't name what the lens is going to find, drop it. The "when unsure, treat as non-trivial" rule above is for the trivial-vs-non-trivial gate at step 3 — it does not license expanding lens count without articulated risk. Every extra lens adds wall-time, log noise, and pulls subagent attention onto speculative angles, which biases the final review toward bloat-shaped findings.
+
    lenses come in two flavors, and you can mix them:
    - **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.
@@ -214,7 +216,7 @@ For simple, well-defined tasks, skip the plan phase and go straight to build.`,
    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.
+   - **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. An idempotency key as a backstop, a timeout as a hint, a retry as belt-and-suspenders: not load-bearing, skip this lens. 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
@@ -304,7 +306,7 @@ ${PR_SUMMARY_FORMAT}`,
    "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.
+   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). same **lens-add discipline** as Review mode applies: each lens needs to name the concrete failure mode it would catch *that the new commits plausibly introduce* — "could apply" doesn't qualify, drop it. **research-validated assumptions** specifically: only pick when the new commits' correctness depends on a third-party contract behaving a specific way; merely using an API doesn't qualify. 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.
 
    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

package.json

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

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

@@ -283,6 +283,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.

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/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)}`));
   }
 }