From 2218ce46fe2e6abb5d4a5dbb688a1a5fbbfc3ca8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 2 May 2026 05:29:57 +0100 Subject: [PATCH] fix: honor no-completion subagent cleanup --- CHANGELOG.md | 1 + src/agents/subagent-announce.ts | 19 ++-- .../subagent-registry-lifecycle.test.ts | 102 +++++++++++++++++- src/agents/subagent-registry-lifecycle.ts | 46 +++++++- src/agents/subagent-registry.ts | 1 + src/agents/subagent-session-cleanup.ts | 25 +++++ 6 files changed, 173 insertions(+), 21 deletions(-) create mode 100644 src/agents/subagent-session-cleanup.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac5ce03427..dc180089e23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Subagents: honor `sessions_spawn` with `expectsCompletionMessage: false` by skipping parent completion handoff delivery while still running child cleanup. Fixes #75848. Thanks @alfredjbclaw. - Gateway/logging: keep deferred channel startup logs on the subsystem logger, so Slack, Discord, Telegram, and voice-call startup messages keep timestamped prefixes. Thanks @vincentkoc. - Discord/threads: ignore webhook-authored copies in already-bound Discord session threads even when the webhook id differs, preventing PluralKit proxy copies from creating duplicate turn pressure. Fixes #52005. Thanks @acgh213. - Discord/threads: return the created thread as partial success when the follow-up initial message fails, so agents do not retry thread creation and create empty duplicate threads. Fixes #48450. Thanks @dahifi. diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index 1345ecffccd..cb741239513 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -43,6 +43,7 @@ import { waitForEmbeddedPiRunEnd, } from "./subagent-announce.runtime.js"; import { getSubagentDepthFromSessionStore } from "./subagent-depth.js"; +import { deleteSubagentSessionForCleanup } from "./subagent-session-cleanup.js"; import type { SpawnSubagentMode } from "./subagent-spawn.types.js"; import { isAnnounceSkip } from "./tools/sessions-send-tokens.js"; @@ -588,19 +589,11 @@ export async function runSubagentAnnounceFlow(params: { } } if (shouldDeleteChildSession) { - try { - await subagentAnnounceDeps.callGateway({ - method: "sessions.delete", - params: { - key: params.childSessionKey, - deleteTranscript: true, - emitLifecycleHooks: params.spawnMode === "session", - }, - timeoutMs: 10_000, - }); - } catch { - // ignore - } + await deleteSubagentSessionForCleanup({ + callGateway: subagentAnnounceDeps.callGateway, + childSessionKey: params.childSessionKey, + spawnMode: params.spawnMode, + }); } } return didAnnounce; diff --git a/src/agents/subagent-registry-lifecycle.test.ts b/src/agents/subagent-registry-lifecycle.test.ts index 35ee9df2338..f49f0e09c2f 100644 --- a/src/agents/subagent-registry-lifecycle.test.ts +++ b/src/agents/subagent-registry-lifecycle.test.ts @@ -9,6 +9,10 @@ const taskExecutorMocks = vi.hoisted(() => ({ setDetachedTaskDeliveryStatusByRunId: vi.fn(), })); +const gatewayMocks = vi.hoisted(() => ({ + callGateway: vi.fn(async () => ({})), +})); + const helperMocks = vi.hoisted(() => ({ persistSubagentSessionTiming: vi.fn(async () => {}), safeRemoveAttachmentsDir: vi.fn(async () => {}), @@ -117,6 +121,7 @@ function createLifecycleController({ emitSubagentEndedHookForRun: vi.fn(async () => {}), notifyContextEngineSubagentEnded: vi.fn(async () => {}), resumeSubagentRun: vi.fn(), + callGateway: gatewayMocks.callGateway, captureSubagentCompletionReply: vi.fn(async () => "final completion reply"), runSubagentAnnounceFlow: vi.fn(async () => true), warn: vi.fn(), @@ -127,6 +132,11 @@ function createLifecycleController({ describe("subagent registry lifecycle hardening", () => { beforeEach(() => { vi.clearAllMocks(); + taskExecutorMocks.completeTaskRunByRunId.mockReset(); + taskExecutorMocks.failTaskRunByRunId.mockReset(); + taskExecutorMocks.setDetachedTaskDeliveryStatusByRunId.mockReset(); + gatewayMocks.callGateway.mockReset(); + gatewayMocks.callGateway.mockResolvedValue({}); browserLifecycleCleanupMocks.cleanupBrowserSessionsForLifecycleEnd.mockClear(); bundleMcpRuntimeMocks.retireSessionMcpRuntimeForSessionKey.mockClear(); bundleMcpRuntimeMocks.retireSessionMcpRuntimeForSessionKey.mockResolvedValue(true); @@ -214,7 +224,7 @@ describe("subagent registry lifecycle hardening", () => { it("cleans up tracked browser sessions before subagent cleanup flow", async () => { const persist = vi.fn(); const entry = createRunEntry({ - expectsCompletionMessage: false, + expectsCompletionMessage: true, }); const runSubagentAnnounceFlow = vi.fn(async () => true); @@ -243,6 +253,92 @@ describe("subagent registry lifecycle hardening", () => { ); }); + it("skips announce delivery when completion messages are disabled", async () => { + const persist = vi.fn(); + const entry = createRunEntry({ + expectsCompletionMessage: false, + retainAttachmentsOnKeep: true, + }); + const runSubagentAnnounceFlow = vi.fn(async () => true); + + const controller = createLifecycleController({ entry, persist, runSubagentAnnounceFlow }); + + await expect( + controller.completeSubagentRun({ + runId: entry.runId, + endedAt: 4_000, + outcome: { status: "ok" }, + reason: SUBAGENT_ENDED_REASON_COMPLETE, + triggerCleanup: true, + }), + ).resolves.toBeUndefined(); + + expect(browserLifecycleCleanupMocks.cleanupBrowserSessionsForLifecycleEnd).toHaveBeenCalledWith( + { + sessionKeys: [entry.childSessionKey], + onWarn: expect.any(Function), + }, + ); + expect(runSubagentAnnounceFlow).not.toHaveBeenCalled(); + expect(taskExecutorMocks.setDetachedTaskDeliveryStatusByRunId).not.toHaveBeenCalledWith( + expect.objectContaining({ + runId: entry.runId, + deliveryStatus: "delivered", + }), + ); + await vi.waitFor(() => expect(entry.cleanupCompletedAt).toBeTypeOf("number")); + expect(entry.completionAnnouncedAt).toBeUndefined(); + }); + + it("archives delete-mode sessions when completion messages are disabled", async () => { + const persist = vi.fn(); + const entry = createRunEntry({ + cleanup: "delete", + expectsCompletionMessage: false, + spawnMode: "session", + }); + const runs = new Map([[entry.runId, entry]]); + const runSubagentAnnounceFlow = vi.fn(async () => true); + + const controller = createLifecycleController({ + entry, + runs, + persist, + runSubagentAnnounceFlow, + }); + + await expect( + controller.completeSubagentRun({ + runId: entry.runId, + endedAt: 4_000, + outcome: { status: "ok" }, + reason: SUBAGENT_ENDED_REASON_COMPLETE, + triggerCleanup: true, + }), + ).resolves.toBeUndefined(); + + await vi.waitFor(() => + expect(gatewayMocks.callGateway).toHaveBeenCalledWith({ + method: "sessions.delete", + params: { + key: entry.childSessionKey, + deleteTranscript: true, + emitLifecycleHooks: true, + }, + timeoutMs: 10_000, + }), + ); + expect(runSubagentAnnounceFlow).not.toHaveBeenCalled(); + expect(taskExecutorMocks.setDetachedTaskDeliveryStatusByRunId).not.toHaveBeenCalledWith( + expect.objectContaining({ + runId: entry.runId, + deliveryStatus: "delivered", + }), + ); + await vi.waitFor(() => expect(runs.has(entry.runId)).toBe(false)); + expect(entry.completionAnnouncedAt).toBeUndefined(); + }); + it("retires bundle MCP runtimes when run-mode cleanup completes", async () => { const entry = createRunEntry({ endedAt: 4_000, @@ -296,7 +392,7 @@ describe("subagent registry lifecycle hardening", () => { const runSubagentAnnounceFlow = vi.fn(async () => true); const entry = createRunEntry({ startedAt: 2_000, - expectsCompletionMessage: false, + expectsCompletionMessage: true, }); const controller = createLifecycleController({ entry, persist, runSubagentAnnounceFlow }); @@ -531,7 +627,7 @@ describe("subagent registry lifecycle hardening", () => { const emitSubagentEndedHookForRun = vi.fn(async () => {}); const entry = createRunEntry({ endedAt: 4_000, - expectsCompletionMessage: false, + expectsCompletionMessage: true, retainAttachmentsOnKeep: false, }); taskExecutorMocks.setDetachedTaskDeliveryStatusByRunId.mockImplementation(() => { diff --git a/src/agents/subagent-registry-lifecycle.ts b/src/agents/subagent-registry-lifecycle.ts index fa7fd930118..6e6fd327c7d 100644 --- a/src/agents/subagent-registry-lifecycle.ts +++ b/src/agents/subagent-registry-lifecycle.ts @@ -1,5 +1,6 @@ import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; import type { cleanupBrowserSessionsForLifecycleEnd } from "../browser-lifecycle-cleanup.js"; +import type { callGateway as defaultCallGateway } from "../gateway/call.js"; import { formatErrorMessage, readErrorName } from "../infra/errors.js"; import { defaultRuntime } from "../runtime.js"; import { emitSessionLifecycleEvent } from "../sessions/session-lifecycle-events.js"; @@ -33,6 +34,7 @@ import { safeRemoveAttachmentsDir, } from "./subagent-registry-helpers.js"; import type { SubagentRunRecord } from "./subagent-registry.types.js"; +import { deleteSubagentSessionForCleanup } from "./subagent-session-cleanup.js"; type CaptureSubagentCompletionReply = (typeof import("./subagent-announce.js"))["captureSubagentCompletionReply"]; @@ -76,6 +78,7 @@ export function createSubagentRegistryLifecycleController(params: { workspaceDir?: string; }): Promise; resumeSubagentRun(runId: string): void; + callGateway: typeof defaultCallGateway; captureSubagentCompletionReply: CaptureSubagentCompletionReply; cleanupBrowserSessionsForLifecycleEnd?: typeof cleanupBrowserSessionsForLifecycleEnd; runSubagentAnnounceFlow: RunSubagentAnnounceFlow; @@ -469,6 +472,7 @@ export function createSubagentRegistryLifecycleController(params: { didAnnounce: boolean, options?: { skipAnnounce?: boolean; + skipDeliveryStatus?: boolean; }, ) => { const entry = params.runs.get(runId); @@ -480,11 +484,13 @@ export function createSubagentRegistryLifecycleController(params: { entry.completionAnnouncedAt = Date.now(); params.persist(); } - safeSetSubagentTaskDeliveryStatus({ - runId, - childSessionKey: entry.childSessionKey, - deliveryStatus: "delivered", - }); + if (!options?.skipDeliveryStatus) { + safeSetSubagentTaskDeliveryStatus({ + runId, + childSessionKey: entry.childSessionKey, + deliveryStatus: "delivered", + }); + } entry.lastAnnounceDeliveryError = undefined; entry.wakeOnDescendantSettle = undefined; entry.fallbackFrozenResultText = undefined; @@ -593,6 +599,36 @@ export function createSubagentRegistryLifecycleController(params: { if (!beginSubagentCleanup(runId)) { return false; } + if (entry.expectsCompletionMessage === false) { + void (async () => { + if (entry.cleanup === "delete") { + await deleteSubagentSessionForCleanup({ + callGateway: params.callGateway, + childSessionKey: entry.childSessionKey, + spawnMode: entry.spawnMode, + onError: (error) => + params.warn("sessions.delete failed during subagent cleanup", { + error: buildSafeLifecycleErrorMeta(error), + runId: maskRunId(runId), + childSessionKey: maskSessionKey(entry.childSessionKey), + }), + }); + } + await finalizeSubagentCleanup(runId, entry.cleanup, true, { + skipAnnounce: true, + skipDeliveryStatus: true, + }); + })().catch((err) => { + defaultRuntime.log(`[warn] subagent cleanup finalize failed (${runId}): ${String(err)}`); + const current = params.runs.get(runId); + if (!current || current.cleanupCompletedAt) { + return; + } + current.cleanupHandled = false; + params.persist(); + }); + return true; + } const requesterOrigin = normalizeDeliveryContext(entry.requesterOrigin); let latestDeliveryError = entry.lastAnnounceDeliveryError; const finalizeAnnounceCleanup = (didAnnounce: boolean) => { diff --git a/src/agents/subagent-registry.ts b/src/agents/subagent-registry.ts index 564c7ac5169..b190ecb3f73 100644 --- a/src/agents/subagent-registry.ts +++ b/src/agents/subagent-registry.ts @@ -565,6 +565,7 @@ const subagentLifecycleController = createSubagentRegistryLifecycleController({ emitSubagentEndedHookForRun, notifyContextEngineSubagentEnded, resumeSubagentRun, + callGateway: (request) => subagentRegistryDeps.callGateway(request), captureSubagentCompletionReply: (sessionKey, options) => subagentRegistryDeps.captureSubagentCompletionReply(sessionKey, options), cleanupBrowserSessionsForLifecycleEnd: (args) => diff --git a/src/agents/subagent-session-cleanup.ts b/src/agents/subagent-session-cleanup.ts new file mode 100644 index 00000000000..88527ae80a7 --- /dev/null +++ b/src/agents/subagent-session-cleanup.ts @@ -0,0 +1,25 @@ +import type { callGateway as defaultCallGateway } from "../gateway/call.js"; +import type { SpawnSubagentMode } from "./subagent-spawn.types.js"; + +type CallGateway = typeof defaultCallGateway; + +export async function deleteSubagentSessionForCleanup(params: { + callGateway: CallGateway; + childSessionKey: string; + spawnMode?: SpawnSubagentMode; + onError?: (error: unknown) => void; +}): Promise { + try { + await params.callGateway({ + method: "sessions.delete", + params: { + key: params.childSessionKey, + deleteTranscript: true, + emitLifecycleHooks: params.spawnMode === "session", + }, + timeoutMs: 10_000, + }); + } catch (error) { + params.onError?.(error); + } +}