Keep delegated-agent notifications alive through permission prompts (#3177)

* fix(agents): preserve permission notification callbacks Permission requests consumed the one-shot completion watcher, so callers never heard about later approvals or the final result. Keep the watcher alive across permission checkpoints and include the actionable request payload in each notification. * fix(agents): notify concurrent permission requests
2026-08-11 00:14:27 +02:00
parent 6aa9f92156
commit 334bf62370
3 changed files with 261 additions and 24 deletions
@@ -46,8 +46,13 @@ interface FinishNotificationScenarioOptions {

 interface FinishNotificationScenario {
  startWatchingChild(): void;
+  requestChildPermission(requestId?: string): void;
+  resolveChildPermission(requestId?: string): void;
+  resolveChildPermissionFromState(requestId?: string): void;
+  resolveChildPermissionWhileIdle(requestId?: string): void;
  finishChild(): void;
  finishChildAndReadParentPrompt(): Promise<string>;
+  parentPrompts(): string[];
  wasParentPrompted(): boolean;
 }

@@ -57,11 +62,13 @@ function createFinishNotificationScenario(
  let subscriber: ((event: AgentManagerEvent) => void) | null = null;
  let resolveParentPrompt: ((prompt: string) => void) | null = null;
  let parentPrompted = false;
+  const parentPrompts: string[] = [];

  const childAgent: ManagedAgent = Object.create(null);
  Reflect.set(childAgent, "id", "child-agent");
  Reflect.set(childAgent, "lifecycle", "idle");
  Reflect.set(childAgent, "config", { title: "Child Agent" });
+  Reflect.set(childAgent, "pendingPermissions", new Map());

  const callerAgent: ManagedAgent = Object.create(null);
  Reflect.set(callerAgent, "id", "caller-agent");
@@ -91,6 +98,7 @@ function createFinishNotificationScenario(
  Reflect.set(agentManager, "hasInFlightRun", () => Boolean(options?.parentPromptError));
  Reflect.set(agentManager, "streamAgent", (_agentId: string, prompt: string) => {
    parentPrompted = true;
+    parentPrompts.push(prompt);
    resolveParentPrompt?.(prompt);
    return (async function* noop() {})();
  });
@@ -123,6 +131,65 @@ function createFinishNotificationScenario(
        logger: options?.logger ?? createTestLogger(),
      });
    },
+    requestChildPermission(requestId = "permission-1") {
+      childAgent.lifecycle = "running";
+      childAgent.pendingPermissions.set(requestId, {
+        id: requestId,
+        provider: "claude",
+        kind: "tool",
+        name: "Run command",
+        description: "Write the QA sentinel",
+        input: {
+          file_path: "/tmp/permission-qa.txt",
+          content: "PASEO_PERMISSION_NOTIFY_QA_OK\n",
+        },
+      });
+      subscriber?.({
+        type: "agent_state",
+        agent: childAgent,
+      });
+      subscriber?.({
+        type: "agent_stream",
+        agentId: "child-agent",
+        event: {
+          type: "permission_requested",
+          provider: "codex",
+          request: childAgent.pendingPermissions.get(requestId)!,
+        },
+      });
+    },
+    resolveChildPermission(requestId = "permission-1") {
+      childAgent.pendingPermissions.delete(requestId);
+      subscriber?.({
+        type: "agent_stream",
+        agentId: "child-agent",
+        event: {
+          type: "permission_resolved",
+          provider: "codex",
+          requestId,
+          resolution: { behavior: "allow" },
+        },
+      });
+    },
+    resolveChildPermissionFromState(requestId = "permission-1") {
+      childAgent.pendingPermissions.delete(requestId);
+      subscriber?.({ type: "agent_state", agent: childAgent });
+    },
+    resolveChildPermissionWhileIdle(requestId = "permission-1") {
+      childAgent.pendingPermissions.delete(requestId);
+      childAgent.lifecycle = "idle";
+      subscriber?.({ type: "agent_state", agent: childAgent });
+      subscriber?.({
+        type: "agent_stream",
+        agentId: "child-agent",
+        event: {
+          type: "permission_resolved",
+          provider: "codex",
+          requestId,
+          resolution: { behavior: "allow" },
+        },
+      });
+    },
    finishChild() {
      childAgent.lifecycle = "running";
      subscriber?.({
@@ -144,6 +211,9 @@ function createFinishNotificationScenario(

      return parentPrompt;
    },
+    parentPrompts() {
+      return parentPrompts;
+    },
    wasParentPrompted() {
      return parentPrompted;
    },
@@ -209,6 +279,107 @@ test("finish notifications tell the parent the child's last assistant message",
  );
 });

+test("finish notifications survive permission responses", async () => {
+  const scenario = createFinishNotificationScenario();
+
+  scenario.startWatchingChild();
+  scenario.requestChildPermission();
+
+  await vi.waitFor(() => {
+    expect(scenario.parentPrompts()).toHaveLength(1);
+  });
+  expect(scenario.parentPrompts()[0]).toContain("needs permission.");
+  const permissionPayload = scenario
+    .parentPrompts()[0]
+    .match(/<permission-request>\n([\s\S]+?)\n<\/permission-request>/)?.[1];
+  expect(permissionPayload).toBeDefined();
+  expect(JSON.parse(permissionPayload!)).toEqual({
+    agentId: "child-agent",
+    requestId: "permission-1",
+    request: {
+      id: "permission-1",
+      provider: "claude",
+      kind: "tool",
+      name: "Run command",
+      description: "Write the QA sentinel",
+      input: {
+        file_path: "/tmp/permission-qa.txt",
+        content: "PASEO_PERMISSION_NOTIFY_QA_OK\n",
+      },
+    },
+  });
+
+  scenario.resolveChildPermission();
+  scenario.finishChild();
+
+  await vi.waitFor(() => {
+    expect(scenario.parentPrompts()).toHaveLength(2);
+  });
+  expect(scenario.parentPrompts()[1]).toContain("finished.");
+});
+
+test("an idle permission resolution waits for the resumed run to finish", async () => {
+  const scenario = createFinishNotificationScenario();
+
+  scenario.startWatchingChild();
+  scenario.requestChildPermission();
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(1));
+
+  scenario.resolveChildPermissionWhileIdle();
+  scenario.requestChildPermission("permission-2");
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(2));
+  expect(scenario.parentPrompts().every((prompt) => prompt.includes("needs permission."))).toBe(
+    true,
+  );
+
+  scenario.resolveChildPermission("permission-2");
+  scenario.finishChild();
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(3));
+  expect(scenario.parentPrompts()[2]).toContain("finished.");
+});
+
+test("finish notifications report every concurrently pending permission", async () => {
+  const scenario = createFinishNotificationScenario();
+
+  scenario.startWatchingChild();
+  scenario.requestChildPermission("permission-1");
+  scenario.requestChildPermission("permission-2");
+
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(2));
+  expect(
+    scenario.parentPrompts().map((prompt) => {
+      const payload = prompt.match(/<permission-request>\n([\s\S]+?)\n<\/permission-request>/)?.[1];
+      return JSON.parse(payload!).requestId;
+    }),
+  ).toEqual(["permission-1", "permission-2"]);
+
+  scenario.resolveChildPermission("permission-1");
+  scenario.resolveChildPermission("permission-2");
+  scenario.finishChild();
+
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(3));
+  expect(scenario.parentPrompts()[2]).toContain("finished.");
+});
+
+test("finish notifications survive repeated permission cycles", async () => {
+  const scenario = createFinishNotificationScenario();
+
+  scenario.startWatchingChild();
+  scenario.requestChildPermission();
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(1));
+  scenario.resolveChildPermissionFromState();
+
+  scenario.requestChildPermission();
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(2));
+  scenario.resolveChildPermission();
+  scenario.finishChild();
+
+  await vi.waitFor(() => expect(scenario.parentPrompts()).toHaveLength(3));
+  expect(
+    scenario.parentPrompts().map((prompt) => prompt.match(/(needs permission|finished)\./)?.[1]),
+  ).toEqual(["needs permission", "needs permission", "finished"]);
+});
+
 test("detaching a child ends its parent-owned finish notification", async () => {
  const scenario = createFinishNotificationScenario({
    childParentAgentId: null,
@@ -258,6 +429,7 @@ it("does not notify archived callers", async () => {
  Reflect.set(childAgent, "id", "child-agent");
  Reflect.set(childAgent, "lifecycle", "idle");
  Reflect.set(childAgent, "config", { title: "Child Agent" });
+  Reflect.set(childAgent, "pendingPermissions", new Map());

  const callerAgent: ManagedAgent = Object.create(null);
  Reflect.set(callerAgent, "id", "caller-agent");
@@ -1,6 +1,10 @@
 import type { Logger } from "pino";

-import type { AgentPromptInput, AgentRunOptions } from "./agent-sdk-types.js";
+import type {
+  AgentPermissionRequest,
+  AgentPromptInput,
+  AgentRunOptions,
+} from "./agent-sdk-types.js";
 import type { AgentManager, ManagedAgent } from "./agent-manager.js";
 import type { AgentStorage } from "./agent-storage.js";
 import { ensureAgentLoaded } from "./agent-loading.js";
@@ -254,15 +258,36 @@ interface FinishNotificationBodyInput {
  title: string;
  reason: "finished" | "errored" | "needs permission";
  lastAssistantMessage: string | null;
+  permissionRequest?: AgentPermissionRequest;
 }

 function formatFinishNotificationBody(params: FinishNotificationBodyInput): string {
  const statusLine = `Agent ${params.childAgentId} (${params.title}) ${params.reason}.`;
-  const lastAssistantMessage = params.lastAssistantMessage?.trim();
-  if (!lastAssistantMessage) {
-    return statusLine;
+  const sections = [statusLine];
+  if (params.reason === "needs permission" && params.permissionRequest) {
+    sections.push(
+      "Respond with `respond_to_permission` using the `agentId` and `requestId` below.",
+      `<permission-request>\n${JSON.stringify(
+        {
+          agentId: params.childAgentId,
+          requestId: params.permissionRequest.id,
+          request: params.permissionRequest,
+        },
+        null,
+        2,
+      )}\n</permission-request>`,
+    );
  }
-  return `${statusLine}\n\n<agent-response>\n${lastAssistantMessage}\n</agent-response>`;
+  const lastAssistantMessage = params.lastAssistantMessage?.trim();
+  if (lastAssistantMessage) {
+    sections.push(`<agent-response>\n${lastAssistantMessage}\n</agent-response>`);
+  }
+  return sections.join("\n\n");
+}
+
+interface NotifySafelyOptions {
+  terminal?: boolean;
+  permissionRequest?: AgentPermissionRequest;
 }

 export function setupFinishNotification(params: SetupFinishNotificationParams): void {
@@ -275,16 +300,21 @@ export function setupFinishNotification(params: SetupFinishNotificationParams):
    logger,
  } = params;
  let hasSeenRunning = false;
-  let fired = false;
+  let stopped = false;
+  const notifiedPermissionRequestIds = new Set<string>();
  let unsubscribe: (() => void) | null = null;
+  let notificationQueue = Promise.resolve();

-  async function notify(reason: "finished" | "errored" | "needs permission"): Promise<void> {
-    if (fired) {
-      return;
-    }
-    fired = true;
+  function stop(): void {
+    if (stopped) return;
+    stopped = true;
    unsubscribe?.();
+  }

+  async function notify(
+    reason: "finished" | "errored" | "needs permission",
+    permissionRequest?: AgentPermissionRequest,
+  ): Promise<void> {
    const callerRecord = await agentStorage.get(callerAgentId);
    if (callerRecord?.archivedAt) {
      return;
@@ -301,6 +331,7 @@ export function setupFinishNotification(params: SetupFinishNotificationParams):
      title,
      reason,
      lastAssistantMessage,
+      permissionRequest,
    });

    await sendPromptToAgent({
@@ -313,24 +344,38 @@ export function setupFinishNotification(params: SetupFinishNotificationParams):
    });
  }

-  function notifySafely(reason: "finished" | "errored" | "needs permission"): void {
-    void notify(reason).catch((error) => {
-      logger.error(
-        { err: error, childAgentId, callerAgentId, reason },
-        "Failed to notify caller agent",
-      );
-    });
+  function notifySafely(
+    reason: "finished" | "errored" | "needs permission",
+    options: NotifySafelyOptions = {},
+  ): void {
+    if (stopped) return;
+    if (options.terminal ?? true) stop();
+    notificationQueue = notificationQueue
+      .then(() => notify(reason, options.permissionRequest))
+      .catch((error) => {
+        logger.error(
+          { err: error, childAgentId, callerAgentId, reason },
+          "Failed to notify caller agent",
+        );
+      });
  }

  unsubscribe = agentManager.subscribe(
    (event) => {
-      if (fired) {
+      if (stopped) {
        return;
      }

      if (event.type === "agent_state") {
+        for (const requestId of notifiedPermissionRequestIds) {
+          if (!event.agent.pendingPermissions.has(requestId)) {
+            notifiedPermissionRequestIds.delete(requestId);
+          }
+        }
        if (event.agent.lifecycle === "running") {
-          hasSeenRunning = true;
+          if (event.agent.pendingPermissions.size === 0) {
+            hasSeenRunning = true;
+          }
          return;
        }
        if (event.agent.lifecycle === "error") {
@@ -342,15 +387,33 @@ export function setupFinishNotification(params: SetupFinishNotificationParams):
          return;
        }
        if (event.agent.lifecycle === "closed") {
-          fired = true;
-          unsubscribe?.();
+          stop();
          return;
        }
        return;
      }

      if (event.event.type === "permission_requested") {
-        notifySafely("needs permission");
+        // A permission pause is an intermediate checkpoint. Forget the run
+        // observed before it so an idle state during follow-up startup cannot
+        // masquerade as the final completion.
+        hasSeenRunning = false;
+        if (!notifiedPermissionRequestIds.has(event.event.request.id)) {
+          notifiedPermissionRequestIds.add(event.event.request.id);
+          notifySafely("needs permission", {
+            terminal: false,
+            permissionRequest: event.event.request,
+          });
+        }
+        return;
+      }
+
+      if (event.event.type === "permission_resolved") {
+        notifiedPermissionRequestIds.delete(event.event.requestId);
+        const childAgent = agentManager.getAgent(childAgentId);
+        if (childAgent?.pendingPermissions.size === 0) {
+          hasSeenRunning = childAgent.lifecycle === "running";
+        }
      }
    },
    { agentId: childAgentId, replayState: false },
@@ -363,7 +426,7 @@ export function setupFinishNotification(params: SetupFinishNotificationParams):
  // transitioning to "running").
  const childSnapshot = agentManager.getAgent(childAgentId);
  if (!childSnapshot || childSnapshot.lifecycle === "closed") {
-    unsubscribe();
+    stop();
    return;
  }
  if (childSnapshot.lifecycle === "running") {