fix(exec): restore two-phase approval registration flow

src/agents/bash-tools.exec-approval-request.test.ts

@@ -22,7 +22,13 @@ describe("requestExecApprovalDecision", () => {
   });
 
   it("returns string decisions", async () => {
-    vi.mocked(callGatewayTool).mockResolvedValue({ decision: "allow-once" });
+    vi.mocked(callGatewayTool)
+      .mockResolvedValueOnce({
+        status: "accepted",
+        id: "approval-id",
+        expiresAtMs: DEFAULT_APPROVAL_TIMEOUT_MS,
+      })
+      .mockResolvedValueOnce({ decision: "allow-once" });
 
     const result = await requestExecApprovalDecision({
       id: "approval-id",
@@ -52,12 +58,22 @@ describe("requestExecApprovalDecision", () => {
         resolvedPath: "/usr/bin/echo",
         sessionKey: "session",
         timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
+        twoPhase: true,
       },
+      { expectFinal: false },
+    );
+    expect(callGatewayTool).toHaveBeenNthCalledWith(
+      2,
+      "exec.approval.waitDecision",
+      { timeoutMs: DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS },
+      { id: "approval-id" },
     );
   });
 
   it("returns null for missing or non-string decisions", async () => {
-    vi.mocked(callGatewayTool).mockResolvedValueOnce({});
+    vi.mocked(callGatewayTool)
+      .mockResolvedValueOnce({ status: "accepted", id: "approval-id", expiresAtMs: 1234 })
+      .mockResolvedValueOnce({});
     await expect(
       requestExecApprovalDecision({
         id: "approval-id",
@@ -70,7 +86,9 @@ describe("requestExecApprovalDecision", () => {
       }),
     ).resolves.toBeNull();
 
-    vi.mocked(callGatewayTool).mockResolvedValueOnce({ decision: 123 });
+    vi.mocked(callGatewayTool)
+      .mockResolvedValueOnce({ status: "accepted", id: "approval-id-2", expiresAtMs: 1234 })
+      .mockResolvedValueOnce({ decision: 123 });
     await expect(
       requestExecApprovalDecision({
         id: "approval-id-2",
@@ -83,4 +101,20 @@ describe("requestExecApprovalDecision", () => {
       }),
     ).resolves.toBeNull();
   });
+
+  it("returns final decision directly when gateway already replies with decision", async () => {
+    vi.mocked(callGatewayTool).mockResolvedValue({ decision: "deny", id: "approval-id" });
+
+    const result = await requestExecApprovalDecision({
+      id: "approval-id",
+      command: "echo hi",
+      cwd: "/tmp",
+      host: "gateway",
+      security: "allowlist",
+      ask: "on-miss",
+    });
+
+    expect(result).toBe("deny");
+    expect(vi.mocked(callGatewayTool).mock.calls).toHaveLength(1);
+  });
 });

src/agents/bash-tools.exec-approval-request.ts

@@ -18,10 +18,45 @@ export type RequestExecApprovalDecisionParams = {
   sessionKey?: string;
 };
 
-export async function requestExecApprovalDecision(
+type ParsedDecision = { present: boolean; value: string | null };
+
+function parseDecision(value: unknown): ParsedDecision {
+  if (!value || typeof value !== "object") {
+    return { present: false, value: null };
+  }
+  // Distinguish "field missing" from "field present but null/invalid".
+  // Registration responses intentionally omit `decision`; decision waits can include it.
+  if (!Object.hasOwn(value, "decision")) {
+    return { present: false, value: null };
+  }
+  const decision = (value as { decision?: unknown }).decision;
+  return { present: true, value: typeof decision === "string" ? decision : null };
+}
+
+function parseString(value: unknown): string | undefined {
+  return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined;
+}
+
+function parseExpiresAtMs(value: unknown): number | undefined {
+  return typeof value === "number" && Number.isFinite(value) ? value : undefined;
+}
+
+export type ExecApprovalRegistration = {
+  id: string;
+  expiresAtMs: number;
+  finalDecision?: string | null;
+};
+
+export async function registerExecApprovalRequest(
   params: RequestExecApprovalDecisionParams,
-): Promise<string | null> {
-  const decisionResult = await callGatewayTool<{ decision: string }>(
+): Promise<ExecApprovalRegistration> {
+  // Two-phase registration is critical: the ID must be registered server-side
+  // before exec returns `approval-pending`, otherwise `/approve` can race and orphan.
+  const registrationResult = await callGatewayTool<{
+    id?: string;
+    expiresAtMs?: number;
+    decision?: string;
+  }>(
     "exec.approval.request",
     { timeoutMs: DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS },
     {
@@ -36,13 +71,46 @@ export async function requestExecApprovalDecision(
       resolvedPath: params.resolvedPath,
       sessionKey: params.sessionKey,
       timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
+      twoPhase: true,
     },
+    { expectFinal: false },
   );
-  const decisionValue =
-    decisionResult && typeof decisionResult === "object"
-      ? (decisionResult as { decision?: unknown }).decision
-      : undefined;
-  return typeof decisionValue === "string" ? decisionValue : null;
+  const decision = parseDecision(registrationResult);
+  const id = parseString(registrationResult?.id) ?? params.id;
+  const expiresAtMs =
+    parseExpiresAtMs(registrationResult?.expiresAtMs) ?? Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
+  if (decision.present) {
+    return { id, expiresAtMs, finalDecision: decision.value };
+  }
+  return { id, expiresAtMs };
+}
+
+export async function waitForExecApprovalDecision(id: string): Promise<string | null> {
+  try {
+    const decisionResult = await callGatewayTool<{ decision: string }>(
+      "exec.approval.waitDecision",
+      { timeoutMs: DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS },
+      { id },
+    );
+    return parseDecision(decisionResult).value;
+  } catch (err) {
+    // Timeout/cleanup path: treat missing/expired as no decision so askFallback applies.
+    const message = String(err).toLowerCase();
+    if (message.includes("approval expired or not found")) {
+      return null;
+    }
+    throw err;
+  }
+}
+
+export async function requestExecApprovalDecision(
+  params: RequestExecApprovalDecisionParams,
+): Promise<string | null> {
+  const registration = await registerExecApprovalRequest(params);
+  if (Object.hasOwn(registration, "finalDecision")) {
+    return registration.finalDecision ?? null;
+  }
+  return await waitForExecApprovalDecision(registration.id);
 }
 
 export async function requestExecApprovalDecisionForHost(params: {
@@ -70,3 +138,29 @@ export async function requestExecApprovalDecisionForHost(params: {
     sessionKey: params.sessionKey,
   });
 }
+
+export async function registerExecApprovalRequestForHost(params: {
+  approvalId: string;
+  command: string;
+  workdir: string;
+  host: "gateway" | "node";
+  nodeId?: string;
+  security: ExecSecurity;
+  ask: ExecAsk;
+  agentId?: string;
+  resolvedPath?: string;
+  sessionKey?: string;
+}): Promise<ExecApprovalRegistration> {
+  return await registerExecApprovalRequest({
+    id: params.approvalId,
+    command: params.command,
+    cwd: params.workdir,
+    nodeId: params.nodeId,
+    host: params.host,
+    security: params.security,
+    ask: params.ask,
+    agentId: params.agentId,
+    resolvedPath: params.resolvedPath,
+    sessionKey: params.sessionKey,
+  });
+}

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

@@ -17,7 +17,10 @@ import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js";
 import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js";
 import { logInfo } from "../logger.js";
 import { markBackgrounded, tail } from "./bash-process-registry.js";
-import { requestExecApprovalDecisionForHost } from "./bash-tools.exec-approval-request.js";
+import {
+  registerExecApprovalRequestForHost,
+  waitForExecApprovalDecision,
+} from "./bash-tools.exec-approval-request.js";
 import {
   DEFAULT_APPROVAL_TIMEOUT_MS,
   DEFAULT_NOTIFY_TAIL_CHARS,
@@ -135,28 +138,42 @@ export async function processGatewayAllowlist(
   if (requiresAsk) {
     const approvalId = crypto.randomUUID();
     const approvalSlug = createApprovalSlug(approvalId);
-    const expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
     const contextKey = `exec:${approvalId}`;
     const resolvedPath = allowlistEval.segments[0]?.resolution?.resolvedPath;
     const noticeSeconds = Math.max(1, Math.round(params.approvalRunningNoticeMs / 1000));
     const effectiveTimeout =
       typeof params.timeoutSec === "number" ? params.timeoutSec : params.defaultTimeoutSec;
     const warningText = params.warnings.length ? `${params.warnings.join("\n")}\n\n` : "";
+    let expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
+    let preResolvedDecision: string | null | undefined;
+
+    try {
+      // Register first so the returned approval ID is actionable immediately.
+      const registration = await registerExecApprovalRequestForHost({
+        approvalId,
+        command: params.command,
+        workdir: params.workdir,
+        host: "gateway",
+        security: hostSecurity,
+        ask: hostAsk,
+        agentId: params.agentId,
+        resolvedPath,
+        sessionKey: params.sessionKey,
+      });
+      expiresAtMs = registration.expiresAtMs;
+      preResolvedDecision = registration.finalDecision;
+    } catch (err) {
+      throw new Error(`Exec approval registration failed: ${String(err)}`, { cause: err });
+    }
 
     void (async () => {
-      let decision: string | null = null;
+      let decision: string | null = preResolvedDecision ?? null;
       try {
-        decision = await requestExecApprovalDecisionForHost({
-          approvalId,
-          command: params.command,
-          workdir: params.workdir,
-          host: "gateway",
-          security: hostSecurity,
-          ask: hostAsk,
-          agentId: params.agentId,
-          resolvedPath,
-          sessionKey: params.sessionKey,
-        });
+        // Some gateways may return a final decision inline during registration.
+        // Only call waitDecision when registration did not already carry one.
+        if (preResolvedDecision === undefined) {
+          decision = await waitForExecApprovalDecision(approvalId);
+        }
       } catch {
         emitExecSystemEvent(
           `Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`,

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

@@ -14,7 +14,10 @@ import {
 import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js";
 import { buildNodeShellCommand } from "../infra/node-shell.js";
 import { logInfo } from "../logger.js";
-import { requestExecApprovalDecisionForHost } from "./bash-tools.exec-approval-request.js";
+import {
+  registerExecApprovalRequestForHost,
+  waitForExecApprovalDecision,
+} from "./bash-tools.exec-approval-request.js";
 import {
   DEFAULT_APPROVAL_TIMEOUT_MS,
   createApprovalSlug,
@@ -180,25 +183,39 @@ export async function executeNodeHostCommand(
   if (requiresAsk) {
     const approvalId = crypto.randomUUID();
     const approvalSlug = createApprovalSlug(approvalId);
-    const expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
     const contextKey = `exec:${approvalId}`;
     const noticeSeconds = Math.max(1, Math.round(params.approvalRunningNoticeMs / 1000));
     const warningText = params.warnings.length ? `${params.warnings.join("\n")}\n\n` : "";
+    let expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
+    let preResolvedDecision: string | null | undefined;
+
+    try {
+      // Register first so the returned approval ID is actionable immediately.
+      const registration = await registerExecApprovalRequestForHost({
+        approvalId,
+        command: params.command,
+        workdir: params.workdir,
+        host: "node",
+        nodeId,
+        security: hostSecurity,
+        ask: hostAsk,
+        agentId: params.agentId,
+        sessionKey: params.sessionKey,
+      });
+      expiresAtMs = registration.expiresAtMs;
+      preResolvedDecision = registration.finalDecision;
+    } catch (err) {
+      throw new Error(`Exec approval registration failed: ${String(err)}`, { cause: err });
+    }
 
     void (async () => {
-      let decision: string | null = null;
+      let decision: string | null = preResolvedDecision ?? null;
       try {
-        decision = await requestExecApprovalDecisionForHost({
-          approvalId,
-          command: params.command,
-          workdir: params.workdir,
-          host: "node",
-          nodeId,
-          security: hostSecurity,
-          ask: hostAsk,
-          agentId: params.agentId,
-          sessionKey: params.sessionKey,
-        });
+        // Some gateways may return a final decision inline during registration.
+        // Only call waitDecision when registration did not already carry one.
+        if (preResolvedDecision === undefined) {
+          decision = await waitForExecApprovalDecision(approvalId);
+        }
       } catch {
         emitExecSystemEvent(
           `Exec denied (node=${nodeId} id=${approvalId}, approval-request-failed): ${params.command}`,

src/agents/bash-tools.exec.approval-id.test.ts

@@ -65,7 +65,9 @@ describe("exec approvals", () => {
 
     vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
       if (method === "exec.approval.request") {
-        // Approval request now carries the decision directly.
+        return { status: "accepted", id: (params as { id?: string })?.id };
+      }
+      if (method === "exec.approval.waitDecision") {
         return { decision: "allow-once" };
       }
       if (method === "node.invoke") {
@@ -191,6 +193,7 @@ describe("exec approvals", () => {
     expect(result.details.status).toBe("approval-pending");
     await approvalSeen;
     expect(calls).toContain("exec.approval.request");
+    expect(calls).toContain("exec.approval.waitDecision");
   });
 
   it("denies node obfuscated command when approval request times out", async () => {
@@ -204,6 +207,9 @@ describe("exec approvals", () => {
     vi.mocked(callGatewayTool).mockImplementation(async (method) => {
       calls.push(method);
       if (method === "exec.approval.request") {
+        return { status: "accepted", id: "approval-id" };
+      }
+      if (method === "exec.approval.waitDecision") {
         return {};
       }
       if (method === "node.invoke") {
@@ -237,6 +243,9 @@ describe("exec approvals", () => {
 
     vi.mocked(callGatewayTool).mockImplementation(async (method) => {
       if (method === "exec.approval.request") {
+        return { status: "accepted", id: "approval-id" };
+      }
+      if (method === "exec.approval.waitDecision") {
         return {};
       }
       return { ok: true };