refactor: extract exec outcome and tool result helpers

src/agents/bash-tools.exec-runtime.ts

@@ -19,6 +19,7 @@ export {
 import { logWarn } from "../logger.js";
 import type { ManagedRun } from "../process/supervisor/index.js";
 import { getProcessSupervisor } from "../process/supervisor/index.js";
+import type { RunExit, TerminationReason } from "../process/supervisor/types.js";
 import {
   addSession,
   appendOutput,
@@ -143,15 +144,34 @@ export const execSchema = Type.Object({
   ),
 });
 
-export type ExecProcessOutcome = {
-  status: "completed" | "failed";
-  exitCode: number | null;
-  exitSignal: NodeJS.Signals | number | null;
-  durationMs: number;
-  aggregated: string;
-  timedOut: boolean;
-  reason?: string;
-};
+export type ExecProcessFailureKind =
+  | "shell-command-not-found"
+  | "shell-not-executable"
+  | "overall-timeout"
+  | "no-output-timeout"
+  | "signal"
+  | "aborted"
+  | "runtime-error";
+
+export type ExecProcessOutcome =
+  | {
+      status: "completed";
+      exitCode: number;
+      exitSignal: NodeJS.Signals | number | null;
+      durationMs: number;
+      aggregated: string;
+      timedOut: false;
+    }
+  | {
+      status: "failed";
+      exitCode: number | null;
+      exitSignal: NodeJS.Signals | number | null;
+      durationMs: number;
+      aggregated: string;
+      timedOut: boolean;
+      failureKind: ExecProcessFailureKind;
+      reason: string;
+    };
 
 export type ExecProcessHandle = {
   session: ProcessSession;
@@ -286,6 +306,116 @@ export function emitExecSystemEvent(
   requestHeartbeatNow(scopedHeartbeatWakeOptions(sessionKey, { reason: "exec-event" }));
 }
 
+function joinExecFailureOutput(aggregated: string, reason: string) {
+  return aggregated ? `${aggregated}\n\n${reason}` : reason;
+}
+
+function classifyExecFailureKind(params: {
+  exitReason: TerminationReason;
+  exitCode: number;
+  isShellFailure: boolean;
+  exitSignal: NodeJS.Signals | number | null;
+}): ExecProcessFailureKind {
+  if (params.isShellFailure) {
+    return params.exitCode === 127 ? "shell-command-not-found" : "shell-not-executable";
+  }
+  if (params.exitReason === "overall-timeout") {
+    return "overall-timeout";
+  }
+  if (params.exitReason === "no-output-timeout") {
+    return "no-output-timeout";
+  }
+  if (params.exitSignal != null) {
+    return "signal";
+  }
+  return "aborted";
+}
+
+export function formatExecFailureReason(params: {
+  failureKind: Exclude<ExecProcessFailureKind, "runtime-error">;
+  exitSignal: NodeJS.Signals | number | null;
+  timeoutSec: number | null | undefined;
+}): string {
+  switch (params.failureKind) {
+    case "shell-command-not-found":
+      return "Command not found";
+    case "shell-not-executable":
+      return "Command not executable (permission denied)";
+    case "overall-timeout":
+      return typeof params.timeoutSec === "number" && params.timeoutSec > 0
+        ? `Command timed out after ${params.timeoutSec} seconds. If this command is expected to take longer, re-run with a higher timeout (e.g., exec timeout=300).`
+        : "Command timed out. If this command is expected to take longer, re-run with a higher timeout (e.g., exec timeout=300).";
+    case "no-output-timeout":
+      return "Command timed out waiting for output";
+    case "signal":
+      return `Command aborted by signal ${params.exitSignal}`;
+    case "aborted":
+      return "Command aborted before exit code was captured";
+  }
+}
+
+export function buildExecExitOutcome(params: {
+  exit: RunExit;
+  aggregated: string;
+  durationMs: number;
+  timeoutSec: number | null | undefined;
+}): ExecProcessOutcome {
+  const exitCode = params.exit.exitCode ?? 0;
+  const isNormalExit = params.exit.reason === "exit";
+  const isShellFailure = exitCode === 126 || exitCode === 127;
+  const status: ExecProcessOutcome["status"] =
+    isNormalExit && !isShellFailure ? "completed" : "failed";
+  if (status === "completed") {
+    const exitMsg = exitCode !== 0 ? `\n\n(Command exited with code ${exitCode})` : "";
+    return {
+      status: "completed",
+      exitCode,
+      exitSignal: params.exit.exitSignal,
+      durationMs: params.durationMs,
+      aggregated: params.aggregated + exitMsg,
+      timedOut: false,
+    };
+  }
+  const failureKind = classifyExecFailureKind({
+    exitReason: params.exit.reason,
+    exitCode,
+    isShellFailure,
+    exitSignal: params.exit.exitSignal,
+  });
+  const reason = formatExecFailureReason({
+    failureKind,
+    exitSignal: params.exit.exitSignal,
+    timeoutSec: params.timeoutSec,
+  });
+  return {
+    status: "failed",
+    exitCode: params.exit.exitCode,
+    exitSignal: params.exit.exitSignal,
+    durationMs: params.durationMs,
+    aggregated: params.aggregated,
+    timedOut: params.exit.timedOut,
+    failureKind,
+    reason: joinExecFailureOutput(params.aggregated, reason),
+  };
+}
+
+export function buildExecRuntimeErrorOutcome(params: {
+  error: unknown;
+  aggregated: string;
+  durationMs: number;
+}): ExecProcessOutcome {
+  return {
+    status: "failed",
+    exitCode: null,
+    exitSignal: null,
+    durationMs: params.durationMs,
+    aggregated: params.aggregated,
+    timedOut: false,
+    failureKind: "runtime-error",
+    reason: joinExecFailureOutput(params.aggregated, String(params.error)),
+  };
+}
+
 export async function runExecProcess(opts: {
   command: string;
   // Execute this instead of `command` (which is kept for display/session/logging).
@@ -531,77 +661,36 @@ export async function runExecProcess(opts: {
     .wait()
     .then(async (exit): Promise<ExecProcessOutcome> => {
       const durationMs = Date.now() - startedAt;
-      const isNormalExit = exit.reason === "exit";
-      const exitCode = exit.exitCode ?? 0;
-      // Shell exit codes 126 (not executable) and 127 (command not found) are
-      // unrecoverable infrastructure failures that should surface as real errors
-      // rather than silently completing — e.g. `python: command not found`.
-      const isShellFailure = exitCode === 126 || exitCode === 127;
-      const status: "completed" | "failed" =
-        isNormalExit && !isShellFailure ? "completed" : "failed";
+      const outcome = buildExecExitOutcome({
+        exit,
+        aggregated: session.aggregated.trim(),
+        durationMs,
+        timeoutSec: opts.timeoutSec,
+      });
 
-      markExited(session, exit.exitCode, exit.exitSignal, status);
-      maybeNotifyOnExit(session, status);
+      markExited(session, exit.exitCode, exit.exitSignal, outcome.status);
+      maybeNotifyOnExit(session, outcome.status);
       if (!session.child && session.stdin) {
         session.stdin.destroyed = true;
       }
-      const aggregated = session.aggregated.trim();
       if (opts.sandbox?.finalizeExec) {
         await opts.sandbox.finalizeExec({
-          status,
+          status: outcome.status,
           exitCode: exit.exitCode ?? null,
           timedOut: exit.timedOut,
           token: sandboxFinalizeToken,
         });
       }
-      if (status === "completed") {
-        const exitMsg = exitCode !== 0 ? `\n\n(Command exited with code ${exitCode})` : "";
-        return {
-          status: "completed",
-          exitCode,
-          exitSignal: exit.exitSignal,
-          durationMs,
-          aggregated: aggregated + exitMsg,
-          timedOut: false,
-        };
-      }
-      const reason = isShellFailure
-        ? exitCode === 127
-          ? "Command not found"
-          : "Command not executable (permission denied)"
-        : exit.reason === "overall-timeout"
-          ? typeof opts.timeoutSec === "number" && opts.timeoutSec > 0
-            ? `Command timed out after ${opts.timeoutSec} seconds. If this command is expected to take longer, re-run with a higher timeout (e.g., exec timeout=300).`
-            : "Command timed out. If this command is expected to take longer, re-run with a higher timeout (e.g., exec timeout=300)."
-          : exit.reason === "no-output-timeout"
-            ? "Command timed out waiting for output"
-            : exit.exitSignal != null
-              ? `Command aborted by signal ${exit.exitSignal}`
-              : "Command aborted before exit code was captured";
-      return {
-        status: "failed",
-        exitCode: exit.exitCode,
-        exitSignal: exit.exitSignal,
-        durationMs,
-        aggregated,
-        timedOut: exit.timedOut,
-        reason: aggregated ? `${aggregated}\n\n${reason}` : reason,
-      };
+      return outcome;
     })
     .catch((err): ExecProcessOutcome => {
       markExited(session, null, null, "failed");
       maybeNotifyOnExit(session, "failed");
-      const aggregated = session.aggregated.trim();
-      const message = aggregated ? `${aggregated}\n\n${String(err)}` : String(err);
-      return {
-        status: "failed",
-        exitCode: null,
-        exitSignal: null,
+      return buildExecRuntimeErrorOutcome({
+        error: err,
+        aggregated: session.aggregated.trim(),
         durationMs: Date.now() - startedAt,
-        aggregated,
-        timedOut: false,
-        reason: message,
-      };
+      });
     });
 
   return {

src/agents/bash-tools.exec.ts

@@ -17,6 +17,7 @@ import {
   DEFAULT_MAX_OUTPUT,
   DEFAULT_PATH,
   DEFAULT_PENDING_MAX_OUTPUT,
+  type ExecProcessOutcome,
   applyPathPrepend,
   applyShellPath,
   normalizeExecAsk,
@@ -43,6 +44,7 @@ import {
   truncateMiddle,
 } from "./bash-tools.shared.js";
 import { assertSandboxPath } from "./sandbox-paths.js";
+import { failedTextResult, textResult } from "./tools/common.js";
 
 export type { BashSandboxConfig } from "./bash-tools.shared.js";
 export type {
@@ -51,6 +53,30 @@ export type {
   ExecToolDetails,
 } from "./bash-tools.exec-types.js";
 
+function buildExecForegroundResult(params: {
+  outcome: ExecProcessOutcome;
+  cwd?: string;
+  warningText?: string;
+}): AgentToolResult<ExecToolDetails> {
+  const warningText = params.warningText?.trim() ? `${params.warningText}\n\n` : "";
+  if (params.outcome.status === "failed") {
+    return failedTextResult(`${warningText}${params.outcome.reason}`, {
+      status: "failed",
+      exitCode: params.outcome.exitCode ?? null,
+      durationMs: params.outcome.durationMs,
+      aggregated: params.outcome.aggregated,
+      cwd: params.cwd,
+    });
+  }
+  return textResult(`${warningText}${params.outcome.aggregated || "(no output)"}`, {
+    status: "completed",
+    exitCode: params.outcome.exitCode,
+    durationMs: params.outcome.durationMs,
+    aggregated: params.outcome.aggregated,
+    cwd: params.cwd,
+  });
+}
+
 function extractScriptTargetFromCommand(
   command: string,
 ): { kind: "python"; relOrAbsPath: string } | { kind: "node"; relOrAbsPath: string } | null {
@@ -597,40 +623,13 @@ export function createExecTool(
             if (yielded || run.session.backgrounded) {
               return;
             }
-            if (outcome.status === "failed") {
-              const failText = outcome.reason ?? "Command failed.";
-              resolve({
-                content: [
-                  {
-                    type: "text",
-                    text: `${getWarningText()}${failText}`,
-                  },
-                ],
-                details: {
-                  status: "failed",
-                  exitCode: outcome.exitCode ?? null,
-                  durationMs: outcome.durationMs,
-                  aggregated: outcome.aggregated,
-                  cwd: run.session.cwd,
-                },
-              });
-              return;
-            }
-            resolve({
-              content: [
-                {
-                  type: "text",
-                  text: `${getWarningText()}${outcome.aggregated || "(no output)"}`,
-                },
-              ],
-              details: {
-                status: "completed",
-                exitCode: outcome.exitCode ?? 0,
-                durationMs: outcome.durationMs,
-                aggregated: outcome.aggregated,
+            resolve(
+              buildExecForegroundResult({
+                outcome,
                 cwd: run.session.cwd,
-              },
-            });
+                warningText: getWarningText(),
+              }),
+            );
           })
           .catch((err) => {
             if (yieldTimer) {

src/agents/pi-tool-definition-adapter.ts

@@ -13,7 +13,7 @@ import {
   runBeforeToolCallHook,
 } from "./pi-tools.before-tool-call.js";
 import { normalizeToolName } from "./tool-policy.js";
-import { jsonResult } from "./tools/common.js";
+import { jsonResult, payloadTextResult } from "./tools/common.js";
 
 type AnyAgentTool = AgentTool;
 
@@ -60,21 +60,6 @@ function describeToolExecutionError(err: unknown): {
   return { message: String(err) };
 }
 
-function stringifyToolPayload(payload: unknown): string {
-  if (typeof payload === "string") {
-    return payload;
-  }
-  try {
-    const encoded = JSON.stringify(payload, null, 2);
-    if (typeof encoded === "string") {
-      return encoded;
-    }
-  } catch {
-    // Fall through to String(payload) for non-serializable values.
-  }
-  return String(payload);
-}
-
 function normalizeToolExecutionResult(params: {
   toolName: string;
   result: unknown;
@@ -88,26 +73,21 @@ function normalizeToolExecutionResult(params: {
     logDebug(`tools: ${toolName} returned non-standard result (missing content[]); coercing`);
     const details = "details" in record ? record.details : record;
     const safeDetails = details ?? { status: "ok", tool: toolName };
-    return {
-      content: [
-        {
-          type: "text",
-          text: stringifyToolPayload(safeDetails),
-        },
-      ],
-      details: safeDetails,
-    };
+    return payloadTextResult(safeDetails);
   }
   const safeDetails = result ?? { status: "ok", tool: toolName };
-  return {
-    content: [
-      {
-        type: "text",
-        text: stringifyToolPayload(safeDetails),
-      },
-    ],
-    details: safeDetails,
-  };
+  return payloadTextResult(safeDetails);
+}
+
+function buildToolExecutionErrorResult(params: {
+  toolName: string;
+  message: string;
+}): AgentToolResult<unknown> {
+  return jsonResult({
+    status: "error",
+    tool: params.toolName,
+    error: params.message,
+  });
 }
 
 function splitToolExecuteArgs(args: ToolExecuteArgsAny): {
@@ -182,10 +162,9 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] {
           }
           logError(`[tools] ${normalizedName} failed: ${described.message}`);
 
-          return jsonResult({
-            status: "error",
-            tool: normalizedName,
-            error: described.message,
+          return buildToolExecutionErrorResult({
+            toolName: normalizedName,
+            message: described.message,
           });
         }
       },

src/agents/tools/common.ts

@@ -214,18 +214,48 @@ export function readReactionParams(
   return { emoji, remove, isEmpty: !emoji };
 }
 
-export function jsonResult(payload: unknown): AgentToolResult<unknown> {
+export function stringifyToolPayload(payload: unknown): string {
+  if (typeof payload === "string") {
+    return payload;
+  }
+  try {
+    const encoded = JSON.stringify(payload, null, 2);
+    if (typeof encoded === "string") {
+      return encoded;
+    }
+  } catch {
+    // Fall through to String(payload) for non-serializable values.
+  }
+  return String(payload);
+}
+
+export function textResult<TDetails>(text: string, details: TDetails): AgentToolResult<TDetails> {
   return {
     content: [
       {
         type: "text",
-        text: JSON.stringify(payload, null, 2),
+        text,
       },
     ],
-    details: payload,
+    details,
   };
 }
 
+export function failedTextResult<TDetails extends { status: "failed" }>(
+  text: string,
+  details: TDetails,
+): AgentToolResult<TDetails> {
+  return textResult(text, details);
+}
+
+export function payloadTextResult<TDetails>(payload: TDetails): AgentToolResult<TDetails> {
+  return textResult(stringifyToolPayload(payload), payload);
+}
+
+export function jsonResult(payload: unknown): AgentToolResult<unknown> {
+  return textResult(JSON.stringify(payload, null, 2), payload);
+}
+
 export function wrapOwnerOnlyToolExecution(
   tool: AnyAgentTool,
   senderIsOwner: boolean,