diff --git a/src/agents/sessions-spawn-hooks.test.ts b/src/agents/sessions-spawn-hooks.test.ts index 6db18f609..9dd9f0891 100644 --- a/src/agents/sessions-spawn-hooks.test.ts +++ b/src/agents/sessions-spawn-hooks.test.ts @@ -45,6 +45,38 @@ vi.mock("../plugins/hook-runner-global.js", () => ({ })), })); +type GatewayRequest = { method?: string; params?: Record }; + +function getGatewayRequests(): GatewayRequest[] { + const callGatewayMock = getCallGatewayMock(); + return callGatewayMock.mock.calls.map((call: [unknown]) => call[0] as GatewayRequest); +} + +function getGatewayMethods(): Array { + return getGatewayRequests().map((request) => request.method); +} + +function findGatewayRequest(method: string): GatewayRequest | undefined { + return getGatewayRequests().find((request) => request.method === method); +} + +function expectSessionsDeleteWithoutAgentStart() { + const methods = getGatewayMethods(); + expect(methods).toContain("sessions.delete"); + expect(methods).not.toContain("agent"); +} + +function mockAgentStartFailure() { + const callGatewayMock = getCallGatewayMock(); + callGatewayMock.mockImplementation(async (opts: unknown) => { + const request = opts as { method?: string }; + if (request.method === "agent") { + throw new Error("spawn failed"); + } + return {}; + }); +} + describe("sessions_spawn subagent lifecycle hooks", () => { beforeEach(() => { hookRunnerMocks.hasSubagentEndedHook = true; @@ -211,19 +243,39 @@ describe("sessions_spawn subagent lifecycle hooks", () => { const details = result.details as { error?: string; childSessionKey?: string }; expect(details.error).toMatch(/thread/i); expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); - const callGatewayMock = getCallGatewayMock(); - const calledMethods = callGatewayMock.mock.calls.map((call: [unknown]) => { - const request = call[0] as { method?: string }; - return request.method; + expectSessionsDeleteWithoutAgentStart(); + const deleteCall = findGatewayRequest("sessions.delete"); + expect(deleteCall?.params).toMatchObject({ + key: details.childSessionKey, + emitLifecycleHooks: false, }); - expect(calledMethods).toContain("sessions.delete"); - expect(calledMethods).not.toContain("agent"); - const deleteCall = callGatewayMock.mock.calls - .map((call: [unknown]) => call[0] as { method?: string; params?: Record }) - .find( - (request: { method?: string; params?: Record }) => - request.method === "sessions.delete", - ); + }); + + it("returns error when thread binding is not marked ready", async () => { + hookRunnerMocks.runSubagentSpawning.mockResolvedValueOnce({ + status: "ok", + threadBindingReady: false, + }); + const tool = await getSessionsSpawnTool({ + agentSessionKey: "main", + agentChannel: "discord", + agentAccountId: "work", + agentTo: "channel:123", + }); + + const result = await tool.execute("call4b", { + task: "do thing", + runTimeoutSeconds: 1, + thread: true, + mode: "session", + }); + + expect(result.details).toMatchObject({ status: "error" }); + const details = result.details as { error?: string; childSessionKey?: string }; + expect(details.error).toMatch(/unable to create or bind a thread/i); + expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); + expectSessionsDeleteWithoutAgentStart(); + const deleteCall = findGatewayRequest("sessions.delete"); expect(deleteCall?.params).toMatchObject({ key: details.childSessionKey, emitLifecycleHooks: false, @@ -269,24 +321,11 @@ describe("sessions_spawn subagent lifecycle hooks", () => { expect(details.error).toMatch(/only discord/i); expect(hookRunnerMocks.runSubagentSpawning).toHaveBeenCalledTimes(1); expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); - const callGatewayMock = getCallGatewayMock(); - const calledMethods = callGatewayMock.mock.calls.map((call: [unknown]) => { - const request = call[0] as { method?: string }; - return request.method; - }); - expect(calledMethods).toContain("sessions.delete"); - expect(calledMethods).not.toContain("agent"); + expectSessionsDeleteWithoutAgentStart(); }); it("runs subagent_ended cleanup hook when agent start fails after successful bind", async () => { - const callGatewayMock = getCallGatewayMock(); - callGatewayMock.mockImplementation(async (opts: unknown) => { - const request = opts as { method?: string }; - if (request.method === "agent") { - throw new Error("spawn failed"); - } - return {}; - }); + mockAgentStartFailure(); const tool = await getSessionsSpawnTool({ agentSessionKey: "main", agentChannel: "discord", @@ -315,12 +354,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { outcome: "error", error: "Session failed to start", }); - const deleteCall = callGatewayMock.mock.calls - .map((call: [unknown]) => call[0] as { method?: string; params?: Record }) - .find( - (request: { method?: string; params?: Record }) => - request.method === "sessions.delete", - ); + const deleteCall = findGatewayRequest("sessions.delete"); expect(deleteCall?.params).toMatchObject({ key: event.targetSessionKey, deleteTranscript: true, @@ -330,14 +364,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { it("falls back to sessions.delete cleanup when subagent_ended hook is unavailable", async () => { hookRunnerMocks.hasSubagentEndedHook = false; - const callGatewayMock = getCallGatewayMock(); - callGatewayMock.mockImplementation(async (opts: unknown) => { - const request = opts as { method?: string }; - if (request.method === "agent") { - throw new Error("spawn failed"); - } - return {}; - }); + mockAgentStartFailure(); const tool = await getSessionsSpawnTool({ agentSessionKey: "main", agentChannel: "discord", @@ -354,17 +381,9 @@ describe("sessions_spawn subagent lifecycle hooks", () => { expect(result.details).toMatchObject({ status: "error" }); expect(hookRunnerMocks.runSubagentEnded).not.toHaveBeenCalled(); - const methods = callGatewayMock.mock.calls.map((call: [unknown]) => { - const request = call[0] as { method?: string }; - return request.method; - }); + const methods = getGatewayMethods(); expect(methods).toContain("sessions.delete"); - const deleteCall = callGatewayMock.mock.calls - .map((call: [unknown]) => call[0] as { method?: string; params?: Record }) - .find( - (request: { method?: string; params?: Record }) => - request.method === "sessions.delete", - ); + const deleteCall = findGatewayRequest("sessions.delete"); expect(deleteCall?.params).toMatchObject({ deleteTranscript: true, emitLifecycleHooks: true, diff --git a/src/agents/subagent-registry.steer-restart.test.ts b/src/agents/subagent-registry.steer-restart.test.ts index 67bd577ce..86eebb8fa 100644 --- a/src/agents/subagent-registry.steer-restart.test.ts +++ b/src/agents/subagent-registry.steer-restart.test.ts @@ -67,6 +67,29 @@ describe("subagent registry steer restarts", () => { await new Promise((resolve) => setImmediate(resolve)); }; + const withPendingAgentWait = async (run: () => Promise): Promise => { + const callGateway = vi.mocked((await import("../gateway/call.js")).callGateway); + const originalCallGateway = callGateway.getMockImplementation(); + callGateway.mockImplementation(async (request: unknown) => { + const typed = request as { method?: string }; + if (typed.method === "agent.wait") { + return new Promise(() => undefined); + } + if (originalCallGateway) { + return originalCallGateway(request as Parameters[0]); + } + return {}; + }); + + try { + return await run(); + } finally { + if (originalCallGateway) { + callGateway.mockImplementation(originalCallGateway); + } + } + }; + afterEach(async () => { announceSpy.mockReset(); announceSpy.mockResolvedValue(true); @@ -135,20 +158,7 @@ describe("subagent registry steer restarts", () => { }); it("defers subagent_ended hook for completion-mode runs until announce delivery resolves", async () => { - const callGateway = vi.mocked((await import("../gateway/call.js")).callGateway); - const originalCallGateway = callGateway.getMockImplementation(); - callGateway.mockImplementation(async (request: unknown) => { - const typed = request as { method?: string }; - if (typed.method === "agent.wait") { - return new Promise(() => undefined); - } - if (originalCallGateway) { - return originalCallGateway(request as Parameters[0]); - } - return {}; - }); - - try { + await withPendingAgentWait(async () => { let resolveAnnounce!: (value: boolean) => void; announceSpy.mockImplementationOnce( () => @@ -196,28 +206,11 @@ describe("subagent registry steer restarts", () => { requesterSessionKey: "agent:main:main", }), ); - } finally { - if (originalCallGateway) { - callGateway.mockImplementation(originalCallGateway); - } - } + }); }); it("does not emit subagent_ended on completion for persistent session-mode runs", async () => { - const callGateway = vi.mocked((await import("../gateway/call.js")).callGateway); - const originalCallGateway = callGateway.getMockImplementation(); - callGateway.mockImplementation(async (request: unknown) => { - const typed = request as { method?: string }; - if (typed.method === "agent.wait") { - return new Promise(() => undefined); - } - if (originalCallGateway) { - return originalCallGateway(request as Parameters[0]); - } - return {}; - }); - - try { + await withPendingAgentWait(async () => { let resolveAnnounce!: (value: boolean) => void; announceSpy.mockImplementationOnce( () => @@ -259,11 +252,7 @@ describe("subagent registry steer restarts", () => { expect(run?.runId).toBe("run-persistent-session"); expect(run?.cleanupCompletedAt).toBeTypeOf("number"); expect(run?.endedHookEmittedAt).toBeUndefined(); - } finally { - if (originalCallGateway) { - callGateway.mockImplementation(originalCallGateway); - } - } + }); }); it("clears announce retry state when replacing after steer restart", () => { @@ -470,66 +459,52 @@ describe("subagent registry steer restarts", () => { }); it("retries completion-mode announce delivery with backoff and then gives up after retry limit", async () => { - const callGateway = vi.mocked((await import("../gateway/call.js")).callGateway); - const originalCallGateway = callGateway.getMockImplementation(); - callGateway.mockImplementation(async (request: unknown) => { - const typed = request as { method?: string }; - if (typed.method === "agent.wait") { - return new Promise(() => undefined); + await withPendingAgentWait(async () => { + vi.useFakeTimers(); + try { + announceSpy.mockResolvedValue(false); + + mod.registerSubagentRun({ + runId: "run-completion-retry", + childSessionKey: "agent:main:subagent:completion", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "completion retry", + cleanup: "keep", + expectsCompletionMessage: true, + }); + + lifecycleHandler?.({ + stream: "lifecycle", + runId: "run-completion-retry", + data: { phase: "end" }, + }); + + await vi.advanceTimersByTimeAsync(0); + expect(announceSpy).toHaveBeenCalledTimes(1); + expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.announceRetryCount).toBe(1); + + await vi.advanceTimersByTimeAsync(999); + expect(announceSpy).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(announceSpy).toHaveBeenCalledTimes(2); + expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.announceRetryCount).toBe(2); + + await vi.advanceTimersByTimeAsync(1_999); + expect(announceSpy).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); + expect(announceSpy).toHaveBeenCalledTimes(3); + expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.announceRetryCount).toBe(3); + + await vi.advanceTimersByTimeAsync(4_001); + expect(announceSpy).toHaveBeenCalledTimes(3); + expect( + mod.listSubagentRunsForRequester("agent:main:main")[0]?.cleanupCompletedAt, + ).toBeTypeOf("number"); + } finally { + vi.useRealTimers(); } - if (originalCallGateway) { - return originalCallGateway(request as Parameters[0]); - } - return {}; }); - - vi.useFakeTimers(); - try { - announceSpy.mockResolvedValue(false); - - mod.registerSubagentRun({ - runId: "run-completion-retry", - childSessionKey: "agent:main:subagent:completion", - requesterSessionKey: "agent:main:main", - requesterDisplayKey: "main", - task: "completion retry", - cleanup: "keep", - expectsCompletionMessage: true, - }); - - lifecycleHandler?.({ - stream: "lifecycle", - runId: "run-completion-retry", - data: { phase: "end" }, - }); - - await vi.advanceTimersByTimeAsync(0); - expect(announceSpy).toHaveBeenCalledTimes(1); - expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.announceRetryCount).toBe(1); - - await vi.advanceTimersByTimeAsync(999); - expect(announceSpy).toHaveBeenCalledTimes(1); - await vi.advanceTimersByTimeAsync(1); - expect(announceSpy).toHaveBeenCalledTimes(2); - expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.announceRetryCount).toBe(2); - - await vi.advanceTimersByTimeAsync(1_999); - expect(announceSpy).toHaveBeenCalledTimes(2); - await vi.advanceTimersByTimeAsync(1); - expect(announceSpy).toHaveBeenCalledTimes(3); - expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.announceRetryCount).toBe(3); - - await vi.advanceTimersByTimeAsync(4_001); - expect(announceSpy).toHaveBeenCalledTimes(3); - expect(mod.listSubagentRunsForRequester("agent:main:main")[0]?.cleanupCompletedAt).toBeTypeOf( - "number", - ); - } finally { - if (originalCallGateway) { - callGateway.mockImplementation(originalCallGateway); - } - vi.useRealTimers(); - } }); it("emits subagent_ended when completion cleanup expires with active descendants", async () => {