refactor(agents): prune dead subagent seams (#122113)

This commit is contained in:
Peter Steinberger
2026-08-11 09:08:07 -07:00
committed by GitHub
parent bf5ce3d55a
commit 9f511cf583
15 changed files with 30 additions and 157 deletions
@@ -184,7 +184,7 @@ const longChildCompletionOutput = [
"34/34 tests pass, clean build. Now docker repro:",
"Root cause: the requester's announce delivery accepted a prefix-only assistant payload as delivered.",
"PR: https://github.com/openclaw/openclaw/pull/12345",
"Verification: pnpm test src/agents/subagent-announce-delivery.test.ts passed with the regression enabled.",
"Verification: pnpm test src/agents/subagents/announce/subagent-announce-delivery.test.ts passed with the regression enabled.",
].join("\n");
const committedSessionSpawnEvidence = {
@@ -101,7 +101,7 @@ export { buildSubagentSystemPrompt } from "../spawn/subagent-system-prompt.js";
export { captureSubagentCompletionReply } from "./subagent-announce-output.js";
export type { SubagentRunOutcome } from "./subagent-announce-output.js";
export type SubagentAnnounceType = "subagent task" | "cron job";
type SubagentAnnounceType = "subagent task" | "cron job";
export type SubagentAnnounceFlowOutcome = NonNullable<
SubagentAnnounceDeliveryResult["disposition"]
>;
@@ -244,7 +244,6 @@ beforeEach(() => {
cleanupBrowserSessionsForLifecycleEnd: async () => {},
ensureContextEnginesInitialized: () => {},
loadAgentRuntimePluginRegistryHandle: () => undefined,
getSubagentRunsSnapshotForRead: (runs) => new Map(runs),
persistSubagentRunsToDisk: () => {},
persistSubagentRunsToDiskOrThrow: () => {},
restoreSubagentRunsFromDisk: () => 0,
@@ -2140,7 +2139,6 @@ describe("killAllControlledSubagentRuns", () => {
cleanupBrowserSessionsForLifecycleEnd: async () => {},
ensureContextEnginesInitialized: () => {},
loadAgentRuntimePluginRegistryHandle: () => undefined,
getSubagentRunsSnapshotForRead: (runs) => new Map(runs),
persistSubagentRunsToDisk: () => {},
persistSubagentRunsToDiskOrThrow: () => {
if (failNextPersistence) {
@@ -12,9 +12,6 @@ import { createLazyImportLoader, createLazyPromiseLoader } from "../../../shared
import { importRuntimeModule } from "../../../shared/runtime-import.js";
import { resolveAgentTimeoutMs } from "../../timeout.js";
import {
getSubagentRunsSnapshotForChildSession,
getSubagentRunsSnapshotForController,
getSubagentRunsSnapshotForRead,
persistSubagentRunsToDisk,
persistSubagentRunsToDiskOrThrow,
restoreSubagentRunsFromDisk,
@@ -38,9 +35,6 @@ export type SubagentRegistryDeps = {
getGatewayRecoveryRuntime: () => GatewayRecoveryRuntime | undefined;
captureSubagentCompletionReply: SubagentAnnounceModule["captureSubagentCompletionReply"];
cleanupBrowserSessionsForLifecycleEnd: typeof cleanupBrowserSessionsForLifecycleEnd;
getSubagentRunsSnapshotForChildSession: typeof getSubagentRunsSnapshotForChildSession;
getSubagentRunsSnapshotForController: typeof getSubagentRunsSnapshotForController;
getSubagentRunsSnapshotForRead: typeof getSubagentRunsSnapshotForRead;
getRuntimeConfig: typeof getRuntimeConfig;
onAgentEvent: (listener: (event: AgentEventPayload) => void) => () => void;
persistSubagentRunsToDisk: typeof persistSubagentRunsToDisk;
@@ -85,9 +79,6 @@ const defaultSubagentRegistryDeps: SubagentRegistryDeps = {
(await loadSubagentAnnounceModule()).captureSubagentCompletionReply(sessionKey, options),
cleanupBrowserSessionsForLifecycleEnd: async (params) =>
(await loadCleanupBrowserSessionsForLifecycleEnd())(params),
getSubagentRunsSnapshotForChildSession,
getSubagentRunsSnapshotForController,
getSubagentRunsSnapshotForRead,
getRuntimeConfig,
onAgentEvent,
persistSubagentRunsToDisk,
@@ -3,7 +3,6 @@ import {
leasePendingAgentSteeringItemsFromSubagentRuns,
releaseLeasedAgentSteeringItemsFromSubagentRuns,
} from "../../agent-steering-queue.js";
import type { SubagentRegistryDeps } from "./subagent-registry-deps.js";
import type { SubagentLifecycleController } from "./subagent-registry-lifecycle.js";
import { getSubagentRunsForChildSession } from "./subagent-registry-memory.js";
import {
@@ -11,27 +10,20 @@ import {
getLatestSubagentRunByChildSessionKeyFromRuns,
} from "./subagent-registry-queries.js";
import { markRequesterTurnYieldedInRuns } from "./subagent-registry-requester-yield.js";
import { getSubagentRunsSnapshotForRead } from "./subagent-registry-state.js";
import type { SubagentRunRecord, SwarmStructuredOutputState } from "./subagent-registry.types.js";
export function createSubagentRegistryPublicApi(config: {
runs: Map<string, SubagentRunRecord>;
deps: () => SubagentRegistryDeps;
persist: (...runIds: string[]) => void;
persistOrThrow: (...runIds: string[]) => void;
restoreOnce: () => void;
startAnnounceCleanup: (runId: string, entry: SubagentRunRecord) => boolean;
settleRequesterTurn: SubagentLifecycleController["settleRequesterTurnAfterSessionSpawns"];
}) {
const {
runs,
deps,
persist,
persistOrThrow,
restoreOnce,
startAnnounceCleanup,
settleRequesterTurn,
} = config;
const readRuns = () => deps().getSubagentRunsSnapshotForRead(runs);
const { runs, persist, persistOrThrow, restoreOnce, startAnnounceCleanup, settleRequesterTurn } =
config;
const readRuns = () => getSubagentRunsSnapshotForRead(runs);
const findRunById = (records: Map<string, SubagentRunRecord>, runId: string) =>
records.get(runId) ?? [...records.values()].find((entry) => entry.swarmRunId === runId);
@@ -7,7 +7,6 @@ import {
} from "../../subagent-test-fixtures.test-helpers.js";
import {
countActiveRunsForSessionFromRuns,
countPendingDescendantRunsExcludingRunFromRuns,
countPendingDescendantRunsFromRuns,
hasDescendantRunAwaitingSettleFromRuns,
getSubagentRunByChildSessionKeyFromRuns,
@@ -398,33 +397,6 @@ describe("subagent registry query regressions", () => {
expect(countPendingDescendantRunsFromRuns(runs, newParentSessionKey)).toBe(1);
});
it("regression excluding current run, countPendingDescendantRunsExcludingRun keeps sibling gating intact", () => {
// Regression guard: excluding the currently announcing run must not hide sibling pending work.
const runs = toRunMap([
makeRun({
runId: "run-self",
childSessionKey: "agent:main:subagent:self",
requesterSessionKey: "agent:main:main",
endedAt: 100,
cleanupCompletedAt: undefined,
}),
makeRun({
runId: "run-sibling",
childSessionKey: "agent:main:subagent:sibling",
requesterSessionKey: "agent:main:main",
endedAt: 101,
cleanupCompletedAt: undefined,
}),
]);
expect(
countPendingDescendantRunsExcludingRunFromRuns(runs, "agent:main:main", "run-self"),
).toBe(1);
expect(
countPendingDescendantRunsExcludingRunFromRuns(runs, "agent:main:main", "run-sibling"),
).toBe(1);
});
it("counts ended orchestrators with pending descendants as active", () => {
const parentSessionKey = "agent:main:subagent:orchestrator";
const runs = toRunMap([
@@ -89,7 +89,6 @@ export type SubagentRunReadIndex<T extends SubagentRunReadRecord = SubagentRunRe
latestRunsByChildSessionKey: ReadonlyMap<string, T>;
countActiveDescendantRuns(rootSessionKey: string): number;
countPendingDescendantRuns(rootSessionKey: string): number;
countPendingDescendantRunsExcludingRun(rootSessionKey: string, excludeRunId: string): number;
hasDescendantRunAwaitingSettle(rootSessionKey: string, excludeRunId?: string): boolean;
listDescendantRunsForRequester(rootSessionKey: string): T[];
runsByControllerSessionKey: ReadonlyMap<string, readonly T[]>;
@@ -291,14 +290,6 @@ export function buildSubagentRunReadIndexFromRuns<T extends SubagentRunReadRecor
return count;
};
const countPendingDescendantRunsExcludingRun = (
rootSessionKey: string,
excludeRunId: string,
): number =>
countPendingDescendantRunsInternal(rootSessionKey, {
excludeRunId,
});
const hasDescendantRunAwaitingSettle = (rootSessionKey: string, excludeRunId?: string): boolean =>
countPendingDescendantRunsInternal(rootSessionKey, {
excludeRunId,
@@ -319,7 +310,6 @@ export function buildSubagentRunReadIndexFromRuns<T extends SubagentRunReadRecor
latestRunsByChildSessionKey,
countActiveDescendantRuns,
countPendingDescendantRuns,
countPendingDescendantRunsExcludingRun,
hasDescendantRunAwaitingSettle,
listDescendantRunsForRequester,
runsByControllerSessionKey,
@@ -485,18 +475,6 @@ export function countPendingDescendantRunsFromRuns(
return buildSubagentRunReadIndexFromRuns({ runs }).countPendingDescendantRuns(rootSessionKey);
}
/** Counts pending descendants while excluding one run id from the total. */
export function countPendingDescendantRunsExcludingRunFromRuns(
runs: Map<string, SubagentRunRecord>,
rootSessionKey: string,
excludeRunId: string,
): number {
return buildSubagentRunReadIndexFromRuns({ runs }).countPendingDescendantRunsExcludingRun(
rootSessionKey,
excludeRunId,
);
}
/**
* True when any descendant below a root session has not reached a terminal
* settle. Differs from the pending count in one way: a run whose final
@@ -9,7 +9,6 @@ import {
buildLatestSubagentRunReadIndexFromRuns,
buildSubagentRunReadIndexFromRuns,
countActiveDescendantRunsFromRuns,
countPendingDescendantRunsExcludingRunFromRuns,
countPendingDescendantRunsFromRuns,
getSubagentRunByChildSessionKeyFromRuns,
hasDescendantRunAwaitingSettleFromRuns,
@@ -142,10 +141,6 @@ describe("subagent registry read index", () => {
countPendingDescendantRunsFromRuns(runs, root),
);
expect(index.countPendingDescendantRuns(root)).toBe(2);
expect(index.countPendingDescendantRunsExcludingRun(root, "run-parent")).toBe(
countPendingDescendantRunsExcludingRunFromRuns(runs, root, "run-parent"),
);
expect(index.countPendingDescendantRunsExcludingRun(root, "run-parent")).toBe(1);
expect(index.hasDescendantRunAwaitingSettle(root)).toBe(
hasDescendantRunAwaitingSettleFromRuns(runs, root),
);
@@ -285,7 +280,6 @@ describe("subagent registry read index", () => {
expect(index.latestRunsByChildSessionKey.size).toBe(100);
expect(index.countActiveDescendantRuns(root)).toBe(100);
expect(index.countPendingDescendantRuns(root)).toBe(100);
expect(index.countPendingDescendantRunsExcludingRun(root, "run-50")).toBe(99);
expect(index.hasDescendantRunAwaitingSettle(root)).toBe(true);
expect(index.listDescendantRunsForRequester(root)).toHaveLength(100);
@@ -297,39 +297,4 @@ describe("subagent registry nested agent tracking", () => {
});
expect(countPendingDescendantRuns(parentSessionKey)).toBe(0);
});
it("countPendingDescendantRunsExcludingRun ignores only the active announce run", () => {
const { addSubagentRunForTests, countPendingDescendantRunsExcludingRun } = subagentRegistry;
addSubagentRunForTests({
runId: "run-self",
childSessionKey: "agent:main:subagent:worker",
requesterSessionKey: "agent:main:main",
requesterDisplayKey: "main",
task: "self",
cleanup: "keep",
createdAt: 1,
startedAt: 1,
endedAt: 2,
cleanupHandled: false,
cleanupCompletedAt: undefined,
});
addSubagentRunForTests({
runId: "run-sibling",
childSessionKey: "agent:main:subagent:sibling",
requesterSessionKey: "agent:main:main",
requesterDisplayKey: "main",
task: "sibling",
cleanup: "keep",
createdAt: 1,
startedAt: 1,
endedAt: 2,
cleanupHandled: false,
cleanupCompletedAt: undefined,
});
expect(countPendingDescendantRunsExcludingRun("agent:main:main", "run-self")).toBe(1);
expect(countPendingDescendantRunsExcludingRun("agent:main:main", "run-sibling")).toBe(1);
});
});
@@ -27,9 +27,6 @@ import {
createSubagentRunRecord,
type SubagentRunRecordOverrides,
} from "../../subagent-test-fixtures.test-helpers.js";
import { subagentRuns } from "./subagent-registry-memory.js";
import { countPendingDescendantRunsExcludingRunFromRuns } from "./subagent-registry-queries.js";
import { getSubagentRunsSnapshotForRead } from "./subagent-registry-state.js";
import type { SubagentRunRecord } from "./subagent-registry.types.js";
type RegistryTestApi = {
@@ -58,7 +55,6 @@ type RegistryDeps = {
| undefined;
captureSubagentCompletionReply: typeof import("../announce/subagent-announce.js").captureSubagentCompletionReply;
cleanupBrowserSessionsForLifecycleEnd: typeof import("../../../browser-lifecycle-cleanup.js").cleanupBrowserSessionsForLifecycleEnd;
getSubagentRunsSnapshotForRead: typeof import("./subagent-registry-state.js").getSubagentRunsSnapshotForRead;
getRuntimeConfig: typeof import("../../../config/config.js").getRuntimeConfig;
onAgentEvent: typeof import("../../../infra/agent-events.js").onAgentEvent;
persistSubagentRunsToDisk: typeof import("./subagent-registry-state.js").persistSubagentRunsToDisk;
@@ -114,17 +110,6 @@ export const testing = {
getRegistryTestApi().testing.setDepsForTest(overrides),
};
export function countPendingDescendantRunsExcludingRun(
rootSessionKey: string,
excludeRunId: string,
) {
return countPendingDescendantRunsExcludingRunFromRuns(
getSubagentRunsSnapshotForRead(subagentRuns),
rootSessionKey,
excludeRunId,
);
}
export function listSessionMaintenanceProtectedSubagentSessionKeys() {
return [...(collectSessionMaintenancePreserveKeys() ?? [])];
}
@@ -569,7 +569,6 @@ export { prependAgentSteeringPrompt };
const publicApi = createSubagentRegistryPublicApi({
runs: subagentRuns,
deps: () => subagentRegistryDeps,
persist: persistSubagentRuns,
persistOrThrow: persistSubagentRunsOrThrow,
restoreOnce: () => subagentRestorer.restoreOnce(),
+14 -5
View File
@@ -244,7 +244,7 @@ vi.mock("../../../tasks/runtime-internal.js", () => ({
listTasksForOwnerKey: hoisted.listTasksForOwnerKeyMock,
}));
const { isSpawnAcpAcceptedResult, spawnAcpDirect } = await import("./acp-spawn.js");
const { spawnAcpDirect } = await import("./acp-spawn.js");
type SpawnRequest = Parameters<typeof spawnAcpDirect>[0];
type SpawnContext = Parameters<typeof spawnAcpDirect>[1];
type SpawnResult = Awaited<ReturnType<typeof spawnAcpDirect>>;
@@ -405,7 +405,7 @@ function expectFailedSpawn(
function expectAcceptedSpawn(result: SpawnResult): Extract<SpawnResult, { status: "accepted" }> {
expect(result.status).toBe("accepted");
if (!isSpawnAcpAcceptedResult(result)) {
if (result.status !== "accepted") {
throw new Error("Expected ACP spawn to be accepted");
}
return result;
@@ -3323,15 +3323,24 @@ describe("spawnAcpDirect", () => {
const accepted = expectAcceptedSpawn(result);
expect(accepted.mode).toBe("session");
expectBindingCallFields({
const binding = expectBindingCallFields({
placement: "current",
conversation: {
channel: "telegram",
accountId: "default",
conversationId: "2",
parentConversationId: "-1003342490704",
},
});
const conversation = expectRecordFields(binding.conversation, {});
const conversationId =
typeof conversation.conversationId === "string" ? conversation.conversationId : "";
const parentConversationId =
typeof conversation.parentConversationId === "string"
? conversation.parentConversationId
: undefined;
const canonicalTopicId = parentConversationId
? `${parentConversationId}:topic:${conversationId}`
: conversationId;
expect(canonicalTopicId).toBe("-1003342490704:topic:2");
const agentCall = hoisted.callGatewayMock.mock.calls
.map((call: unknown[]) => call[0] as { method?: string; params?: Record<string, unknown> })
.find((request) => request.method === "agent");
+5 -12
View File
@@ -95,12 +95,9 @@ import { callSubagentGateway, readGatewayRunId } from "./subagent-spawn-gateway.
import { resolveSubagentSpawnOwnership } from "./subagent-spawn-ownership.js";
import { resolveConfiguredSubagentRunTimeoutSeconds } from "./subagent-spawn-plan.js";
export const ACP_SPAWN_MODES = ["run", "session"] as const;
type SpawnAcpMode = (typeof ACP_SPAWN_MODES)[number];
const ACP_SPAWN_SANDBOX_MODES = ["inherit", "require"] as const;
export type SpawnAcpSandboxMode = (typeof ACP_SPAWN_SANDBOX_MODES)[number];
export const ACP_SPAWN_STREAM_TARGETS = ["parent"] as const;
type SpawnAcpStreamTarget = (typeof ACP_SPAWN_STREAM_TARGETS)[number];
type SpawnAcpMode = "run" | "session";
type SpawnAcpSandboxMode = "inherit" | "require";
type SpawnAcpStreamTarget = "parent";
type SpawnAcpParams = {
task: string;
@@ -121,7 +118,7 @@ type SpawnAcpParams = {
attachments?: AcpTurnAttachment[];
};
export type SpawnAcpContext = {
type SpawnAcpContext = {
agentSessionKey?: string;
requesterTurnRunId?: string;
completionOwnerKey?: string;
@@ -183,11 +180,7 @@ type SpawnAcpFailedResult = SpawnAcpResultFields & {
errorCode: SpawnAcpErrorCode;
};
export type SpawnAcpResult = SpawnAcpAcceptedResult | SpawnAcpFailedResult;
export function isSpawnAcpAcceptedResult(result: SpawnAcpResult): result is SpawnAcpAcceptedResult {
return result.status === "accepted";
}
type SpawnAcpResult = SpawnAcpAcceptedResult | SpawnAcpFailedResult;
const ACP_SPAWN_ACCEPTED_NOTE =
"initial ACP task queued in isolated session; follow-ups continue in the bound thread.";
@@ -36,9 +36,6 @@ vi.mock("../subagents/spawn/subagent-spawn.js", () => ({
}));
vi.mock("../subagents/spawn/acp-spawn.js", () => ({
ACP_SPAWN_MODES: ["run", "session"],
ACP_SPAWN_STREAM_TARGETS: ["parent"],
isSpawnAcpAcceptedResult: (result: { status?: string }) => result?.status === "accepted",
spawnAcpDirect: (...args: unknown[]) => hoisted.spawnAcpDirectMock(...args),
}));
@@ -12,7 +12,7 @@ import { getAcpSessionManager } from "../acp/control-plane/manager.js";
import { getAcpRuntimeBackend } from "../acp/runtime/registry.js";
import { prepareSystemAgentRunAdmission } from "../agents/admitted-run-context.js";
import { isLiveTestEnabled, readLiveTestConfig } from "../agents/live-test-helpers.js";
import { isSpawnAcpAcceptedResult, spawnAcpDirect } from "../agents/subagents/spawn/acp-spawn.js";
import { spawnAcpDirect } from "../agents/subagents/spawn/acp-spawn.js";
import { clearConfigCache, clearRuntimeConfigSnapshot } from "../config/config.js";
import { resolveStorePath } from "../config/sessions/paths.js";
import { loadSessionEntry } from "../config/sessions/session-accessor.js";
@@ -519,12 +519,12 @@ describeLive("gateway live (ACP spawn defaults)", () => {
},
{ agentSessionKey: "agent:main:main" },
);
if (!isSpawnAcpAcceptedResult(configuredDefaultResult)) {
if (configuredDefaultResult.status !== "accepted") {
throw new Error(
`configured default ACP spawn failed (${configuredDefaultResult.errorCode}): ${configuredDefaultResult.error}`,
);
}
expect(isSpawnAcpAcceptedResult(configuredDefaultResult)).toBe(true);
expect(configuredDefaultResult.status).toBe("accepted");
sessionKeys.push(configuredDefaultResult.childSessionKey);
const configuredDefaultEntry = await waitForSessionEntry({
cfg: runtimeCfg,
@@ -542,12 +542,12 @@ describeLive("gateway live (ACP spawn defaults)", () => {
},
{ agentSessionKey: "agent:main:main" },
);
if (!isSpawnAcpAcceptedResult(primaryOnlyResult)) {
if (primaryOnlyResult.status !== "accepted") {
throw new Error(
`primary-only ACP spawn failed (${primaryOnlyResult.errorCode}): ${primaryOnlyResult.error}`,
);
}
expect(isSpawnAcpAcceptedResult(primaryOnlyResult)).toBe(true);
expect(primaryOnlyResult.status).toBe("accepted");
sessionKeys.push(primaryOnlyResult.childSessionKey);
const primaryOnlyEntry = await waitForSessionEntry({
cfg: runtimeCfg,