fix(exec): skip node approval prepare in yolo mode

src/agents/bash-tools.exec-host-node.ts

@@ -1,26 +1,24 @@
-import crypto from "node:crypto";
 import type { AgentToolResult } from "@mariozechner/pi-agent-core";
 import {
-  type ExecApprovalsFile,
   type ExecAsk,
   type ExecSecurity,
-  evaluateShellAllowlist,
-  hasDurableExecApproval,
   requiresExecApproval,
   resolveExecApprovalAllowedDecisions,
-  resolveExecApprovalsFromFile,
 } from "../infra/exec-approvals.js";
-import {
-  describeInterpreterInlineEval,
-  detectInterpreterInlineEvalArgv,
-} from "../infra/exec-inline-eval.js";
-import { buildNodeShellCommand } from "../infra/node-shell.js";
-import { parsePreparedSystemRunPayload } from "../infra/system-run-approval-context.js";
 import {
   buildExecApprovalRequesterContext,
   buildExecApprovalTurnSourceContext,
   registerExecApprovalRequestForHostOrThrow,
 } from "./bash-tools.exec-approval-request.js";
+import {
+  analyzeNodeApprovalRequirement,
+  buildNodeSystemRunInvoke,
+  formatNodeRunToolResult,
+  invokeNodeSystemRunDirect,
+  prepareNodeSystemRun,
+  resolveNodeExecutionTarget,
+  shouldSkipNodeApprovalPrepare,
+} from "./bash-tools.exec-host-node-phases.js";
 import * as execHostShared from "./bash-tools.exec-host-shared.js";
 import {
   DEFAULT_NOTIFY_TAIL_CHARS,
@@ -29,7 +27,6 @@ import {
 } from "./bash-tools.exec-runtime.js";
 import type { ExecToolDetails } from "./bash-tools.exec-types.js";
 import { callGatewayTool } from "./tools/gateway.js";
-import { listNodes, resolveNodeIdFromList } from "./tools/nodes-utils.js";
 
 export type ExecuteNodeHostCommandParams = {
   command: string;
@@ -66,132 +63,27 @@ export async function executeNodeHostCommand(
     ask: params.ask,
     host: "node",
   });
-  if (params.boundNode && params.requestedNode && params.boundNode !== params.requestedNode) {
-    throw new Error(`exec node not allowed (bound to ${params.boundNode})`);
+  const target = await resolveNodeExecutionTarget(params);
+  if (
+    shouldSkipNodeApprovalPrepare({
+      hostSecurity,
+      hostAsk,
+      strictInlineEval: params.strictInlineEval,
+    })
+  ) {
+    return await invokeNodeSystemRunDirect({ request: params, target });
   }
-  const nodeQuery = params.boundNode || params.requestedNode;
-  const nodes = await listNodes({});
-  if (nodes.length === 0) {
-    throw new Error(
-      "exec host=node requires a paired node (none available). This requires a companion app or node host.",
-    );
-  }
-  let nodeId: string;
-  try {
-    nodeId = resolveNodeIdFromList(nodes, nodeQuery, !nodeQuery);
-  } catch (err) {
-    if (!nodeQuery && String(err).includes("node required")) {
-      throw new Error(
-        "exec host=node requires a node id when multiple nodes are available (set tools.exec.node or exec.node).",
-        { cause: err },
-      );
-    }
-    throw err;
-  }
-  const nodeInfo = nodes.find((entry) => entry.nodeId === nodeId);
-  const supportsSystemRun = Array.isArray(nodeInfo?.commands)
-    ? nodeInfo?.commands?.includes("system.run")
-    : false;
-  if (!supportsSystemRun) {
-    throw new Error(
-      "exec host=node requires a node that supports system.run (companion app or node host).",
-    );
-  }
-  const argv = buildNodeShellCommand(params.command, nodeInfo?.platform);
-  const prepareRaw = await callGatewayTool(
-    "node.invoke",
-    { timeoutMs: 15_000 },
-    {
-      nodeId,
-      command: "system.run.prepare",
-      params: {
-        command: argv,
-        rawCommand: params.command,
-        ...(params.workdir != null ? { cwd: params.workdir } : {}),
-        agentId: params.agentId,
-        sessionKey: params.sessionKey,
-      },
-      idempotencyKey: crypto.randomUUID(),
-    },
-  );
-  const prepared = parsePreparedSystemRunPayload(prepareRaw?.payload);
-  if (!prepared) {
-    throw new Error("invalid system.run.prepare response");
-  }
-  const runArgv = prepared.plan.argv;
-  const runRawCommand = prepared.plan.commandText;
-  const runCwd = prepared.plan.cwd ?? params.workdir;
-  const runAgentId = prepared.plan.agentId ?? params.agentId;
-  const runSessionKey = prepared.plan.sessionKey ?? params.sessionKey;
 
-  const nodeEnv = params.requestedEnv ? { ...params.requestedEnv } : undefined;
-  const baseAllowlistEval = evaluateShellAllowlist({
-    command: params.command,
-    allowlist: [],
-    safeBins: new Set(),
-    cwd: params.workdir,
-    env: params.env,
-    platform: nodeInfo?.platform,
-    trustedSafeBinDirs: params.trustedSafeBinDirs,
+  const prepared = await prepareNodeSystemRun({ request: params, target });
+  const approvalAnalysis = await analyzeNodeApprovalRequirement({
+    request: params,
+    target,
+    prepared,
+    hostSecurity,
+    hostAsk,
   });
-  let analysisOk = baseAllowlistEval.analysisOk;
-  let allowlistSatisfied = false;
-  let durableApprovalSatisfied = false;
-  const inlineEvalHit =
-    params.strictInlineEval === true
-      ? (baseAllowlistEval.segments
-          .map((segment) =>
-            detectInterpreterInlineEvalArgv(segment.resolution?.effectiveArgv ?? segment.argv),
-          )
-          .find((entry) => entry !== null) ?? null)
-      : null;
-  if (inlineEvalHit) {
-    params.warnings.push(
-      `Warning: strict inline-eval mode requires explicit approval for ${describeInterpreterInlineEval(
-        inlineEvalHit,
-      )}.`,
-    );
-  }
-  if ((hostAsk === "always" || hostSecurity === "allowlist") && analysisOk) {
-    try {
-      const approvalsSnapshot = await callGatewayTool<{ file: string }>(
-        "exec.approvals.node.get",
-        { timeoutMs: 10_000 },
-        { nodeId },
-      );
-      const approvalsFile =
-        approvalsSnapshot && typeof approvalsSnapshot === "object"
-          ? approvalsSnapshot.file
-          : undefined;
-      if (approvalsFile && typeof approvalsFile === "object") {
-        const resolved = resolveExecApprovalsFromFile({
-          file: approvalsFile as ExecApprovalsFile,
-          agentId: params.agentId,
-          overrides: { security: "full" },
-        });
-        // Allowlist-only precheck; safe bins are node-local and may diverge.
-        const allowlistEval = evaluateShellAllowlist({
-          command: params.command,
-          allowlist: resolved.allowlist,
-          safeBins: new Set(),
-          cwd: params.workdir,
-          env: params.env,
-          platform: nodeInfo?.platform,
-          trustedSafeBinDirs: params.trustedSafeBinDirs,
-        });
-        durableApprovalSatisfied = hasDurableExecApproval({
-          analysisOk: allowlistEval.analysisOk,
-          segmentAllowlistEntries: allowlistEval.segmentAllowlistEntries,
-          allowlist: resolved.allowlist,
-          commandText: runRawCommand,
-        });
-        allowlistSatisfied = allowlistEval.allowlistSatisfied;
-        analysisOk = allowlistEval.analysisOk;
-      }
-    } catch {
-      // Fall back to requiring approval if node approvals cannot be fetched.
-    }
-  }
+  const { analysisOk, allowlistSatisfied, durableApprovalSatisfied, inlineEvalHit } =
+    approvalAnalysis;
   const requiresAsk =
     requiresExecApproval({
       ask: hostAsk,
@@ -200,40 +92,6 @@ export async function executeNodeHostCommand(
       allowlistSatisfied,
       durableApprovalSatisfied,
     }) || inlineEvalHit !== null;
-  const invokeTimeoutMs = Math.max(
-    10_000,
-    (typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec) * 1000 +
-      5_000,
-  );
-  const buildInvokeParams = (
-    approvedByAsk: boolean,
-    approvalDecision: "allow-once" | "allow-always" | null,
-    runId?: string,
-    suppressNotifyOnExit?: boolean,
-  ) =>
-    ({
-      nodeId,
-      command: "system.run",
-      params: {
-        command: runArgv,
-        rawCommand: runRawCommand,
-        systemRunPlan: prepared.plan,
-        cwd: runCwd,
-        env: nodeEnv,
-        timeoutMs: typeof params.timeoutSec === "number" ? params.timeoutSec * 1000 : undefined,
-        agentId: runAgentId,
-        sessionKey: runSessionKey,
-        approved: approvedByAsk,
-        approvalDecision:
-          approvalDecision === "allow-always" && inlineEvalHit !== null
-            ? "allow-once"
-            : (approvalDecision ?? undefined),
-        runId: runId ?? undefined,
-        suppressNotifyOnExit:
-          suppressNotifyOnExit === true || params.notifyOnExit === false ? true : undefined,
-      },
-      idempotencyKey: crypto.randomUUID(),
-    }) satisfies Record<string, unknown>;
 
   let inlineApprovedByAsk = false;
   let inlineApprovalDecision: "allow-once" | "allow-always" | null = null;
@@ -250,15 +108,15 @@ export async function executeNodeHostCommand(
       await registerExecApprovalRequestForHostOrThrow({
         approvalId,
         systemRunPlan: prepared.plan,
-        env: nodeEnv,
-        workdir: runCwd,
+        env: target.env,
+        workdir: prepared.cwd,
         host: "node",
-        nodeId,
+        nodeId: target.nodeId,
         security: hostSecurity,
         ask: hostAsk,
         ...buildExecApprovalRequesterContext({
-          agentId: runAgentId,
-          sessionKey: runSessionKey,
+          agentId: prepared.agentId,
+          sessionKey: prepared.sessionKey,
         }),
         ...buildExecApprovalTurnSourceContext(params),
       });
@@ -324,7 +182,7 @@ export async function executeNodeHostCommand(
           onFailure: () =>
             void execHostShared.sendExecApprovalFollowupResult(
               followupTarget,
-              `Exec denied (node=${nodeId} id=${approvalId}, approval-request-failed): ${params.command}`,
+              `Exec denied (node=${target.nodeId} id=${approvalId}, approval-request-failed): ${params.command}`,
             ),
         });
         if (decision === undefined) {
@@ -366,7 +224,7 @@ export async function executeNodeHostCommand(
         if (deniedReason) {
           await execHostShared.sendExecApprovalFollowupResult(
             followupTarget,
-            `Exec denied (node=${nodeId} id=${approvalId}, ${deniedReason}): ${params.command}`,
+            `Exec denied (node=${target.nodeId} id=${approvalId}, ${deniedReason}): ${params.command}`,
           );
           return;
         }
@@ -374,8 +232,25 @@ export async function executeNodeHostCommand(
         try {
           const raw = await callGatewayTool(
             "node.invoke",
-            { timeoutMs: invokeTimeoutMs },
-            buildInvokeParams(approvedByAsk, approvalDecision, approvalId, true),
+            { timeoutMs: target.invokeTimeoutMs },
+            buildNodeSystemRunInvoke({
+              target,
+              command: prepared.argv,
+              rawCommand: prepared.rawCommand,
+              cwd: prepared.cwd,
+              timeoutSec: params.timeoutSec,
+              agentId: prepared.agentId,
+              sessionKey: prepared.sessionKey,
+              approved: approvedByAsk,
+              approvalDecision:
+                approvalDecision === "allow-always" && inlineEvalHit !== null
+                  ? "allow-once"
+                  : approvalDecision,
+              runId: approvalId,
+              suppressNotifyOnExit: true,
+              notifyOnExit: params.notifyOnExit,
+              systemRunPlan: prepared.plan,
+            }),
           );
           const payload =
             raw?.payload && typeof raw.payload === "object"
@@ -393,13 +268,13 @@ export async function executeNodeHostCommand(
           const output = normalizeNotifyOutput(combined.slice(-DEFAULT_NOTIFY_TAIL_CHARS));
           const exitLabel = payload.timedOut ? "timeout" : `code ${payload.exitCode ?? "?"}`;
           const summary = output
-            ? `Exec finished (node=${nodeId} id=${approvalId}, ${exitLabel})\n${output}`
-            : `Exec finished (node=${nodeId} id=${approvalId}, ${exitLabel})`;
+            ? `Exec finished (node=${target.nodeId} id=${approvalId}, ${exitLabel})\n${output}`
+            : `Exec finished (node=${target.nodeId} id=${approvalId}, ${exitLabel})`;
           await execHostShared.sendExecApprovalFollowupResult(followupTarget, summary);
         } catch {
           await execHostShared.sendExecApprovalFollowupResult(
             followupTarget,
-            `Exec denied (node=${nodeId} id=${approvalId}, invoke-failed): ${params.command}`,
+            `Exec denied (node=${target.nodeId} id=${approvalId}, invoke-failed): ${params.command}`,
           );
         }
       })();
@@ -416,7 +291,7 @@ export async function executeNodeHostCommand(
         sentApproverDms,
         unavailableReason,
         allowedDecisions: resolveExecApprovalAllowedDecisions({ ask: hostAsk }),
-        nodeId,
+        nodeId: target.nodeId,
       });
     }
   }
@@ -424,31 +299,21 @@ export async function executeNodeHostCommand(
   const startedAt = Date.now();
   const raw = await callGatewayTool(
     "node.invoke",
-    { timeoutMs: invokeTimeoutMs },
-    buildInvokeParams(inlineApprovedByAsk, inlineApprovalDecision, inlineApprovalId),
+    { timeoutMs: target.invokeTimeoutMs },
+    buildNodeSystemRunInvoke({
+      target,
+      command: prepared.argv,
+      rawCommand: prepared.rawCommand,
+      cwd: prepared.cwd,
+      timeoutSec: params.timeoutSec,
+      agentId: prepared.agentId,
+      sessionKey: prepared.sessionKey,
+      approved: inlineApprovedByAsk,
+      approvalDecision: inlineApprovalDecision,
+      runId: inlineApprovalId,
+      notifyOnExit: params.notifyOnExit,
+      systemRunPlan: prepared.plan,
+    }),
   );
-  const payload =
-    raw && typeof raw === "object" ? (raw as { payload?: unknown }).payload : undefined;
-  const payloadObj =
-    payload && typeof payload === "object" ? (payload as Record<string, unknown>) : {};
-  const stdout = typeof payloadObj.stdout === "string" ? payloadObj.stdout : "";
-  const stderr = typeof payloadObj.stderr === "string" ? payloadObj.stderr : "";
-  const errorText = typeof payloadObj.error === "string" ? payloadObj.error : "";
-  const success = typeof payloadObj.success === "boolean" ? payloadObj.success : false;
-  const exitCode = typeof payloadObj.exitCode === "number" ? payloadObj.exitCode : null;
-  return {
-    content: [
-      {
-        type: "text",
-        text: stdout || stderr || errorText || "",
-      },
-    ],
-    details: {
-      status: success ? "completed" : "failed",
-      exitCode,
-      durationMs: Date.now() - startedAt,
-      aggregated: [stdout, stderr, errorText].filter(Boolean).join("\n"),
-      cwd: params.workdir,
-    } satisfies ExecToolDetails,
-  };
+  return formatNodeRunToolResult({ raw, startedAt, cwd: params.workdir });
 }