mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix: honor no-completion subagent cleanup
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
@@ -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<void>;
|
||||
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) => {
|
||||
|
||||
@@ -565,6 +565,7 @@ const subagentLifecycleController = createSubagentRegistryLifecycleController({
|
||||
emitSubagentEndedHookForRun,
|
||||
notifyContextEngineSubagentEnded,
|
||||
resumeSubagentRun,
|
||||
callGateway: (request) => subagentRegistryDeps.callGateway(request),
|
||||
captureSubagentCompletionReply: (sessionKey, options) =>
|
||||
subagentRegistryDeps.captureSubagentCompletionReply(sessionKey, options),
|
||||
cleanupBrowserSessionsForLifecycleEnd: (args) =>
|
||||
|
||||
25
src/agents/subagent-session-cleanup.ts
Normal file
25
src/agents/subagent-session-cleanup.ts
Normal file
@@ -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<void> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user