From d25b4a29438b2f4f33ac113e14e661c4eca309e2 Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Tue, 24 Mar 2026 12:54:50 -0500 Subject: [PATCH] fix: fail closed when subagent steer remap fails --- src/agents/subagent-control.test.ts | 76 ++++++++++++++++++++ src/agents/subagent-control.ts | 12 +++- test/fixtures/test-memory-hotspots.unit.json | 40 +++-------- 3 files changed, 97 insertions(+), 31 deletions(-) diff --git a/src/agents/subagent-control.test.ts b/src/agents/subagent-control.test.ts index 7e845b45374..6d69e3085b6 100644 --- a/src/agents/subagent-control.test.ts +++ b/src/agents/subagent-control.test.ts @@ -9,6 +9,7 @@ import { __testing, killSubagentRunAdmin, sendControlledSubagentMessage, + steerControlledSubagentRun, } from "./subagent-control.js"; import { addSubagentRunForTests, @@ -217,3 +218,78 @@ describe("killSubagentRunAdmin", () => { } }); }); + +describe("steerControlledSubagentRun", () => { + afterEach(() => { + resetSubagentRegistryForTests({ persist: false }); + __testing.setDepsForTest(); + }); + + it("returns an error and clears the restart marker when run remap fails", async () => { + addSubagentRunForTests({ + runId: "run-steer-old", + childSessionKey: "agent:main:subagent:steer-worker", + controllerSessionKey: "agent:main:main", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "initial task", + cleanup: "keep", + createdAt: Date.now() - 5_000, + startedAt: Date.now() - 4_000, + }); + + const replaceSpy = vi + .spyOn(await import("./subagent-registry.js"), "replaceSubagentRunAfterSteer") + .mockReturnValue(false); + + __testing.setDepsForTest({ + callGateway: async >(request: CallGatewayOptions) => { + if (request.method === "agent.wait") { + return {} as T; + } + if (request.method === "agent") { + return { runId: "run-steer-new" } as T; + } + throw new Error(`unexpected method: ${request.method}`); + }, + }); + + try { + const result = await steerControlledSubagentRun({ + cfg: {} as OpenClawConfig, + controller: { + controllerSessionKey: "agent:main:main", + callerSessionKey: "agent:main:main", + callerIsSubagent: false, + controlScope: "children", + }, + entry: { + runId: "run-steer-old", + childSessionKey: "agent:main:subagent:steer-worker", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + controllerSessionKey: "agent:main:main", + task: "initial task", + cleanup: "keep", + createdAt: Date.now() - 5_000, + startedAt: Date.now() - 4_000, + }, + message: "updated direction", + }); + + expect(result).toEqual({ + status: "error", + runId: "run-steer-new", + sessionKey: "agent:main:subagent:steer-worker", + sessionId: undefined, + error: "failed to replace steered subagent run", + }); + expect(getSubagentRunByChildSessionKey("agent:main:subagent:steer-worker")).toMatchObject({ + runId: "run-steer-old", + suppressAnnounceReason: undefined, + }); + } finally { + replaceSpy.mockRestore(); + } + }); +}); diff --git a/src/agents/subagent-control.ts b/src/agents/subagent-control.ts index fde6c08564c..96ff9609471 100644 --- a/src/agents/subagent-control.ts +++ b/src/agents/subagent-control.ts @@ -728,12 +728,22 @@ export async function steerControlledSubagentRun(params: { }; } - replaceSubagentRunAfterSteer({ + const replaced = replaceSubagentRunAfterSteer({ previousRunId: params.entry.runId, nextRunId: runId, fallback: params.entry, runTimeoutSeconds: params.entry.runTimeoutSeconds ?? 0, }); + if (!replaced) { + clearSubagentRunSteerRestart(params.entry.runId); + return { + status: "error", + runId, + sessionKey: params.entry.childSessionKey, + sessionId, + error: "failed to replace steered subagent run", + }; + } return { status: "accepted", diff --git a/test/fixtures/test-memory-hotspots.unit.json b/test/fixtures/test-memory-hotspots.unit.json index e1bba1556ec..25ce45ca80a 100644 --- a/test/fixtures/test-memory-hotspots.unit.json +++ b/test/fixtures/test-memory-hotspots.unit.json @@ -6,27 +6,19 @@ "files": { "src/infra/outbound/targets.channel-resolution.test.ts": { "deltaKb": 1111491, - "sources": [ - "openclaw-test-memory-trace:unit-heavy-2" - ] + "sources": ["openclaw-test-memory-trace:unit-heavy-2"] }, "src/media/fetch.telegram-network.test.ts": { "deltaKb": 1101005, - "sources": [ - "openclaw-test-memory-trace:unit-heavy-1" - ] + "sources": ["openclaw-test-memory-trace:unit-heavy-1"] }, "src/cron/isolated-agent/run.skill-filter.test.ts": { "deltaKb": 1069548, - "sources": [ - "openclaw-test-memory-trace:unit-run.skill-filter-memory-isolated" - ] + "sources": ["openclaw-test-memory-trace:unit-run.skill-filter-memory-isolated"] }, "src/infra/outbound/deliver.test.ts": { "deltaKb": 1059062, - "sources": [ - "openclaw-deliver-mem:unit" - ] + "sources": ["openclaw-deliver-mem:unit"] }, "src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts": { "deltaKb": 1048576, @@ -42,33 +34,23 @@ }, "src/cron/isolated-agent/run.cron-model-override.test.ts": { "deltaKb": 946790, - "sources": [ - "openclaw-test-memory-trace:unit-run.cron-model-override-memory-isolated" - ] + "sources": ["openclaw-test-memory-trace:unit-run.cron-model-override-memory-isolated"] }, "src/channels/plugins/plugins-core.test.ts": { "deltaKb": 792269, - "sources": [ - "openclaw-plugins-core-mem:unit" - ] + "sources": ["openclaw-plugins-core-mem:unit"] }, "src/plugins/manifest-registry.test.ts": { "deltaKb": 684749, - "sources": [ - "openclaw-manifest-registry-mem:unit" - ] + "sources": ["openclaw-manifest-registry-mem:unit"] }, "src/plugins/install.test.ts": { "deltaKb": 510874, - "sources": [ - "openclaw-test-memory-trace:unit-install-memory-isolated" - ] + "sources": ["openclaw-test-memory-trace:unit-install-memory-isolated"] }, "ui/src/ui/views/chat.test.ts": { "deltaKb": 509338, - "sources": [ - "openclaw-test-memory-trace:unit-chat-memory-isolated" - ] + "sources": ["openclaw-test-memory-trace:unit-chat-memory-isolated"] }, "src/infra/provider-usage.auth.normalizes-keys.test.ts": { "deltaKb": 494080, @@ -78,9 +60,7 @@ }, "src/plugins/conversation-binding.test.ts": { "deltaKb": 485786, - "sources": [ - "openclaw-test-memory-trace:unit-conversation-binding-memory-isolated" - ] + "sources": ["openclaw-test-memory-trace:unit-conversation-binding-memory-isolated"] } } }