diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts b/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts index 8c6fb7a29b92..6f3cde24db0d 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts @@ -1,4 +1,6 @@ // Shared sessions_spawn test harness for gateway, registry, and lifecycle mocks. +import os from "node:os"; +import path from "node:path"; import { vi, type Mock } from "vitest"; import type { SessionRunStatus } from "../../packages/gateway-protocol/src/schema/sessions-row.js"; import type { SubagentLifecycleHookRunner } from "../plugins/hooks.js"; @@ -142,6 +144,10 @@ const hoisted = vi.hoisted(() => { let cachedCreateSessionsSpawnTool: CreateSessionsSpawnTool | null = null; let cachedSubagentRegistryTesting: SubagentRegistryTesting | null = null; let cachedSubagentSpawnTesting: SubagentSpawnTesting | null = null; +const sessionStorePath = path.join( + os.tmpdir(), + `openclaw-sessions-spawn-test-store-${process.pid}-${process.env.VITEST_POOL_ID ?? "0"}.json`, +); export function getCallGatewayMock(): Mock { return hoisted.callGatewayMock; @@ -393,7 +399,7 @@ vi.mock("../config/sessions.js", () => ({ agentId: string; }) => `agent:${params.agentId}:${params.cfg?.session?.mainKey ?? "main"}`, resolveExistingAgentSessionStoreTargetsSync: () => [], - resolveSessionStorePathCore: () => "/tmp/openclaw-sessions-spawn-test-store.json", + resolveSessionStorePathCore: () => sessionStorePath, updateSessionStore: async ( _storePath: string, mutator: (store: typeof hoisted.sessionStore) => void | Promise, @@ -404,7 +410,8 @@ vi.mock("../config/sessions.js", () => ({ vi.mock("../tasks/detached-task-runtime.js", () => ({ completeTaskRunByRunId: vi.fn(), - createRunningTaskRun: vi.fn(), + createQueuedTaskRun: vi.fn(() => ({})), + createRunningTaskRun: vi.fn(() => ({})), failTaskRunByRunId: vi.fn(), findDetachedTaskRun: vi.fn(() => ({ lookup: "available" as const })), setDetachedTaskDeliveryStatusByRunId: vi.fn(), diff --git a/src/agents/subagents/registry/subagent-registry-run-launch.ts b/src/agents/subagents/registry/subagent-registry-run-launch.ts index b99733265169..3aa4a67314cc 100644 --- a/src/agents/subagents/registry/subagent-registry-run-launch.ts +++ b/src/agents/subagents/registry/subagent-registry-run-launch.ts @@ -84,6 +84,9 @@ export type RegisterSubagentRunParams = { outputSchema?: Record; queuedLaunch?: SwarmQueuedLaunch; queued?: boolean; + /** Required when direct dispatch suppresses Gateway tracking. Out-of-process launches keep + Gateway's existing best-effort CLI policy; other callers create a best-effort row here. */ + taskRowOwnership?: "required" | "gateway_best_effort"; }; export class SubagentLaunchManager extends SubagentRecoveryManager { @@ -179,60 +182,92 @@ export class SubagentLaunchManager extends SubagentRecoveryManager { }); this.options.runs.set(runId, entry); const killReconciliationSnapshots = this.markOlderKillReconciliationsSuperseded(entry); - try { - this.options.persistOrThrow( - runId, - ...[...killReconciliationSnapshots.keys()].map((candidate) => candidate.runId), - ); - } catch (error) { + const registeredKillReconciliationSnapshots = new Map( + [...killReconciliationSnapshots.keys()].map((candidate) => [ + candidate, + structuredClone(candidate.killReconciliation), + ]), + ); + const registeredRunIds = [ + runId, + ...[...killReconciliationSnapshots.keys()].map((candidate) => candidate.runId), + ]; + const rollbackRegistration = () => { this.options.runs.delete(runId); this.restoreKillReconciliationSnapshots(killReconciliationSnapshots); + }; + const restoreDurableRegistration = () => { + this.options.runs.set(runId, entry); + this.restoreKillReconciliationSnapshots(registeredKillReconciliationSnapshots); + }; + const activateRegistrationLifecycle = () => { + this.options.ensureListener(); + // Session-mode and persistence-recovery runs also need TTL cleanup. + this.options.startSweeper(); + if (!queued) { + void this.waitForSubagentCompletion(runId, waitTimeoutMs, entry); + } + }; + try { + this.options.persistOrThrow(...registeredRunIds); + } catch (error) { + rollbackRegistration(); throw error; } - try { - const taskParams = { - runtime: "subagent", - sourceId: runId, - ownerKey: requesterSessionKey, - scopeKind: "session", - // Detached task runtimes are plugin-replaceable. Isolate their input so - // mutation cannot change the already-persisted registry record. - requesterOrigin: requesterOrigin ? structuredClone(requesterOrigin) : undefined, - childSessionKey, - runId, - label: registerParams.label, - task: registerParams.task, - agentId: registerParams.agentId, - requesterAgentId: resolveSubagentRequesterAgentId(cfg, registerParams), - deliveryStatus: - registerParams.expectsCompletionMessage === false ? "not_applicable" : "pending", - } as const; - const task = queued - ? createQueuedTaskRun(taskParams) - : createRunningTaskRun({ - ...taskParams, - startedAt: now, - lastEventAt: now, - }); - if (!task) { - log.warn("Failed to persist background task for subagent run", { - runId: registerParams.runId, - }); + if (registerParams.taskRowOwnership !== "gateway_best_effort") { + try { + const taskParams = { + runtime: "subagent", + sourceId: runId, + ownerKey: requesterSessionKey, + scopeKind: "session", + // Detached task runtimes are plugin-replaceable. Isolate their input so + // mutation cannot change the already-persisted registry record. + requesterOrigin: requesterOrigin ? structuredClone(requesterOrigin) : undefined, + childSessionKey, + runId, + label: registerParams.label, + task: registerParams.task, + agentId: registerParams.agentId, + requesterAgentId: resolveSubagentRequesterAgentId(cfg, registerParams), + deliveryStatus: + registerParams.expectsCompletionMessage === false ? "not_applicable" : "pending", + } as const; + const task = queued + ? createQueuedTaskRun(taskParams) + : createRunningTaskRun({ + ...taskParams, + startedAt: now, + lastEventAt: now, + }); + if (!task) { + if (registerParams.taskRowOwnership === "required") { + throw new Error(`detached task runtime created no task row for run ${runId}`); + } + log.warn("Failed to persist background task for subagent run", { runId }); + } + } catch (error) { + if (registerParams.taskRowOwnership !== "required") { + log.warn("Failed to create background task for subagent run", { runId, error }); + } else { + // Direct dispatch suppressed Gateway's CLI fallback. Persist the rollback before + // asking the caller to abort; if that write fails, memory must match durable state. + rollbackRegistration(); + try { + this.options.persistOrThrow(...registeredRunIds); + } catch (rollbackError) { + restoreDurableRegistration(); + // Durable state still owns this registration. Keep reconciliation active so + // caller cleanup can terminalize it instead of leaving a phantom run. + activateRegistrationLifecycle(); + throw rollbackError; + } + throw error; + } } - } catch (error) { - log.warn("Failed to create background task for subagent run", { - runId: registerParams.runId, - error, - }); - } - this.options.ensureListener(); - // Always start sweeper — session-mode runs (no archiveAtMs) also need TTL cleanup. - this.options.startSweeper(); - // Wait for subagent completion via gateway RPC (cross-process). - // The in-process lifecycle listener is a fallback for embedded runs. - if (!queued) { - void this.waitForSubagentCompletion(runId, waitTimeoutMs, entry); } + // Wait through Gateway RPC; the in-process lifecycle listener is the embedded fallback. + activateRegistrationLifecycle(); }; readonly startQueuedSubagentRun = ( diff --git a/src/agents/subagents/registry/subagent-registry.test.ts b/src/agents/subagents/registry/subagent-registry.test.ts index bb1416ae9a25..91951be567e1 100644 --- a/src/agents/subagents/registry/subagent-registry.test.ts +++ b/src/agents/subagents/registry/subagent-registry.test.ts @@ -5154,24 +5154,35 @@ describe("subagent registry seam flow", () => { }); mockPendingAgentWait(); const defaultRuntime = getDetachedTaskLifecycleRuntime(); - const createMutatingTaskRun = vi.fn( + const mutateRequesterOrigin = ( + taskParams: Parameters[0], + ) => { + if (!taskParams.requesterOrigin) { + throw new Error("expected requester origin"); + } + Object.assign(taskParams.requesterOrigin, { + channel: "mutated", + to: "mutated", + accountId: "mutated", + threadId: "mutated", + }); + }; + const createMutatingQueuedTaskRun = vi.fn( (taskParams: Parameters[0]) => { - if (!taskParams.requesterOrigin) { - throw new Error("expected requester origin"); - } - Object.assign(taskParams.requesterOrigin, { - channel: "mutated", - to: "mutated", - accountId: "mutated", - threadId: "mutated", - }); - return null; + mutateRequesterOrigin(taskParams); + return defaultRuntime.createQueuedTaskRun(taskParams); + }, + ); + const createMutatingRunningTaskRun = vi.fn( + (taskParams: Parameters[0]) => { + mutateRequesterOrigin(taskParams); + return defaultRuntime.createRunningTaskRun(taskParams); }, ); setDetachedTaskLifecycleRuntime({ ...defaultRuntime, - createQueuedTaskRun: createMutatingTaskRun, - createRunningTaskRun: createMutatingTaskRun, + createQueuedTaskRun: createMutatingQueuedTaskRun, + createRunningTaskRun: createMutatingRunningTaskRun, }); mod.registerSubagentRun({ @@ -5181,13 +5192,97 @@ describe("subagent registry seam flow", () => { requesterOrigin, }); - expect(createMutatingTaskRun).toHaveBeenCalledOnce(); + expect( + queued ? createMutatingQueuedTaskRun : createMutatingRunningTaskRun, + ).toHaveBeenCalledOnce(); expect(findRequesterRun(runId)?.requesterOrigin).toEqual(expectedRequesterOrigin); expect(persistedEntry?.requesterOrigin).toEqual(expectedRequesterOrigin); expect(mocks.persistSubagentRunsToDiskOrThrow).toHaveBeenCalledOnce(); expect(mocks.persistSubagentRunsToDisk).not.toHaveBeenCalled(); }); + const optionalTaskRowFaults: Array<[label: string, createTaskRun: () => null]> = [ + ["returns no row", () => null], + [ + "throws", + () => { + throw new Error("task store unavailable"); + }, + ], + ]; + it.each(optionalTaskRowFaults)( + "keeps ACP-style registry ownership when the secondary task runtime %s", + (_label, createTaskRun) => { + const runId = `run-acp-task-fault-${_label.replaceAll(" ", "-")}`; + setDetachedTaskLifecycleRuntime({ + ...getDetachedTaskLifecycleRuntime(), + createQueuedTaskRun: createTaskRun, + createRunningTaskRun: createTaskRun, + }); + mockPendingAgentWait(); + + expect(() => + mod.registerSubagentRun({ + runId, + task: "preserve ACP registry ownership", + }), + ).not.toThrow(); + + expect(findRequesterRun(runId)).toMatchObject({ + runId, + task: "preserve ACP registry ownership", + }); + }, + ); + + it("keeps memory aligned with the durable registration when rollback persistence fails", () => { + const childSessionKey = "agent:main:subagent:task-row-rollback-failure"; + mod.addSubagentRunForTests({ + runId: "run-task-row-rollback-old", + childSessionKey, + task: "preserve the durable predecessor state", + createdAt: Date.now() - 1_000, + endedAt: Date.now() - 500, + endedReason: "subagent-killed", + suppressAnnounceReason: "killed", + killReconciliation: { killedAt: Date.now() - 500 }, + }); + mocks.persistSubagentRunsToDiskOrThrow + .mockImplementationOnce(() => {}) + .mockImplementationOnce(() => { + throw new Error("rollback disk full"); + }); + setDetachedTaskLifecycleRuntime({ + ...getDetachedTaskLifecycleRuntime(), + createRunningTaskRun: () => null, + }); + mockPendingAgentWait(); + + expect(() => + mod.registerSubagentRun({ + runId: "run-task-row-rollback-new", + childSessionKey, + task: "retain the last durable snapshot", + taskRowOwnership: "required", + }), + ).toThrowError("rollback disk full"); + + expect(findRequesterRun("run-task-row-rollback-new")).toMatchObject({ + runId: "run-task-row-rollback-new", + childSessionKey, + }); + expect( + findRequesterRun("run-task-row-rollback-old")?.killReconciliation?.supersededAt, + ).toBeTypeOf("number"); + expect(mocks.persistSubagentRunsToDiskOrThrow).toHaveBeenCalledTimes(2); + expect(mocks.callGateway).toHaveBeenCalledWith( + expect.objectContaining({ + method: "agent.wait", + params: expect.objectContaining({ runId: "run-task-row-rollback-new" }), + }), + ); + }); + it("retains an already-running replacement when its durable write fails", () => { mockPendingAgentWait(); mod.registerSubagentRun({ diff --git a/src/agents/subagents/spawn/acp-spawn.test.ts b/src/agents/subagents/spawn/acp-spawn.test.ts index badc302b954b..cf7d0aaf2bee 100644 --- a/src/agents/subagents/spawn/acp-spawn.test.ts +++ b/src/agents/subagents/spawn/acp-spawn.test.ts @@ -906,6 +906,9 @@ describe("spawnAcpDirect", () => { expect(agentCall?.params?.deliver).toBe(true); expect(agentCall?.params?.lane).toBe("subagent"); expect(agentCall?.params?.acpTurnSource).toBe("manual_spawn"); + expect(hoisted.registerSubagentRunMock.mock.calls[0]?.[0]).not.toHaveProperty( + "requiresTaskRow", + ); const initInput = expectInitializeSessionFields({ agent: "codex", mode: "persistent", diff --git a/src/agents/subagents/spawn/subagent-spawn-gateway.ts b/src/agents/subagents/spawn/subagent-spawn-gateway.ts index 760b5611bb20..648f344f610e 100644 --- a/src/agents/subagents/spawn/subagent-spawn-gateway.ts +++ b/src/agents/subagents/spawn/subagent-spawn-gateway.ts @@ -11,10 +11,14 @@ import { const DEFAULT_SUBAGENT_AGENT_GATEWAY_TIMEOUT_MS = 60_000; const MAX_SUBAGENT_AGENT_GATEWAY_TIMEOUT_MS = 300_000; -export async function callSubagentGateway( +type SubagentGatewayResponse = Awaited>; +type SubagentGatewayDispatchMode = "in_process" | "out_of_process"; + +async function callSubagentGatewayWithDispatchMode( params: Parameters[0], authorization?: SubagentLaunchAuthorization, -): Promise>> { + options?: { agentRunTracking?: "native_subagent" }, +): Promise<{ response: SubagentGatewayResponse; dispatchMode: SubagentGatewayDispatchMode }> { // Subagent lifecycle requires methods spanning multiple scope tiers // (sessions.delete → admin, agent → write). When each call // independently negotiates least-privilege scopes the first connection pairs @@ -57,20 +61,48 @@ export async function callSubagentGateway( // Direct dispatch avoids self-connecting over WS while the same event loop is busy. // Agent launches are host-owned even when the parent request came from CLI/HTTP. // Reusing that external identity makes collector preflight treat the launch as spoofed. - const forceSyntheticClient = request.method === "agent" || scopes != null; - return await deps.dispatchGatewayMethodInProcess( + const isChildRunLaunch = request.method === "agent"; + const forceSyntheticClient = isChildRunLaunch || scopes != null; + const response = await deps.dispatchGatewayMethodInProcess( request.method, request.params as Record, { expectFinal: request.expectFinal, ...(allowModelOverride ? { allowSyntheticModelOverride: true } : {}), + ...(options?.agentRunTracking ? { agentRunTracking: options.agentRunTracking } : {}), ...(forceSyntheticClient ? { forceSyntheticClient: true } : {}), ...(typeof request.timeoutMs === "number" ? { timeoutMs: request.timeoutMs } : {}), ...(scopes != null ? { syntheticScopes: scopes } : {}), }, ); + return { response, dispatchMode: "in_process" }; } - return await deps.callGateway(request); + return { response: await deps.callGateway(request), dispatchMode: "out_of_process" }; +} + +export async function callSubagentGateway( + params: Parameters[0], + authorization?: SubagentLaunchAuthorization, +): Promise { + return (await callSubagentGatewayWithDispatchMode(params, authorization)).response; +} + +export async function callNativeSubagentGateway( + params: Parameters[0], + authorization?: SubagentLaunchAuthorization, +): Promise<{ + response: SubagentGatewayResponse; + taskRowOwnership: "required" | "gateway_best_effort"; +}> { + const result = await callSubagentGatewayWithDispatchMode(params, authorization, { + agentRunTracking: "native_subagent", + }); + return { + response: result.response, + // The trusted marker exists only on direct dispatch. A WebSocket fallback keeps the + // ordinary Gateway CLI policy: tracking is best-effort and never rejects an accepted run. + taskRowOwnership: result.dispatchMode === "in_process" ? "required" : "gateway_best_effort", + }; } export function readGatewayRunId( diff --git a/src/agents/subagents/spawn/subagent-spawn.in-process-gateway.test.ts b/src/agents/subagents/spawn/subagent-spawn.in-process-gateway.test.ts index d84f52ec084b..ecbab3e6b68a 100644 --- a/src/agents/subagents/spawn/subagent-spawn.in-process-gateway.test.ts +++ b/src/agents/subagents/spawn/subagent-spawn.in-process-gateway.test.ts @@ -9,6 +9,7 @@ import { } from "../../../config/config.js"; import { prepareAgentRequestPreflight } from "../../../gateway/agent-turn/agent-request-preflight.js"; import { createAgentTurnIo } from "../../../gateway/agent-turn/io.js"; +import { resolveGatewayAgentTaskTrackingMode } from "../../../gateway/server-methods/agent-task-tracking.js"; import type { GatewayRequestContext, GatewayRequestOptions, @@ -24,6 +25,11 @@ import { resetGatewayWorkAdmission, tryBeginGatewayRootWorkAdmission, } from "../../../process/gateway-work-admission.js"; +import { getDetachedTaskLifecycleRuntime } from "../../../tasks/detached-task-runtime.js"; +import { + resetDetachedTaskLifecycleRuntimeForTests, + setDetachedTaskLifecycleRuntime, +} from "../../../tasks/detached-task-runtime.test-support.js"; import { captureEnv, setTestEnvValue } from "../../../test-utils/env.js"; import { subagentRuns } from "../registry/subagent-registry-memory.js"; import { markSubagentRunTerminated } from "../registry/subagent-registry.js"; @@ -32,6 +38,7 @@ import { testing as subagentRegistryTesting, } from "../registry/subagent-registry.test-helpers.js"; import { testing as swarmSchedulerTesting } from "../swarm/swarm-scheduler.test-support.js"; +import { callSubagentGateway } from "./subagent-spawn-gateway.js"; import { spawnSubagentDirect } from "./subagent-spawn.js"; import { testing as subagentSpawnTesting } from "./subagent-spawn.test-support.js"; @@ -127,6 +134,29 @@ describe("spawnSubagentDirect in-process Gateway collector launch", () => { clearConfigCache(); }); + it("leaves shared agent dispatches unmarked unless native spawn claims the task row", async () => { + const dispatchOptions: Array< + NonNullable[2]> | undefined + > = []; + subagentSpawnTesting.setDepsForTest({ + hasInProcessGatewayContext: () => true, + dispatchGatewayMethodInProcess: async ( + _method: string, + _params: Record, + options?: NonNullable[2]>, + ) => { + dispatchOptions.push(options); + return { runId: "shared-agent-run", status: "accepted" } as T; + }, + }); + + await callSubagentGateway({ method: "agent", params: { sessionKey: "agent:main:acp:test" } }); + + expect(dispatchOptions).toHaveLength(1); + expect(dispatchOptions[0]?.forceSyntheticClient).toBe(true); + expect(dispatchOptions[0]?.agentRunTracking).toBeUndefined(); + }); + afterEach(async () => { clearFallbackGatewayContext(); resetGatewayWorkAdmission(); @@ -134,6 +164,7 @@ describe("spawnSubagentDirect in-process Gateway collector launch", () => { resetSubagentRegistryForTests({ persist: false }); subagentRegistryTesting.setDepsForTest(); subagentSpawnTesting.setDepsForTest(); + resetDetachedTaskLifecycleRuntimeForTests(); clearRuntimeConfigSnapshot(); clearConfigCache(); envSnapshot.restore(); @@ -406,4 +437,336 @@ describe("spawnSubagentDirect in-process Gateway collector launch", () => { }); }); }); + + it("aborts the accepted child run when registry registration fails", async () => { + const gatewayContext = makeGatewayContext(); + const requests: Array<{ method: string; params: Record }> = []; + subagentSpawnTesting.setDepsForTest({ + dispatchGatewayMethodInProcess: async ( + method: string, + params: Record, + ) => { + requests.push({ method, params }); + if (method === "agent") { + return { runId: "gateway-accepted-run", status: "accepted" } as T; + } + if (method === "chat.abort") { + return { aborted: true, runIds: [params.runId] } as T; + } + return {} as T; + }, + }); + // The registry never takes ownership, which is exactly when the suppressed + // gateway CLI row would have been the only record of the accepted run. + subagentRegistryTesting.setDepsForTest({ + loadAgentRuntimePluginRegistryHandle: () => undefined, + persistSubagentRunsToDisk: () => {}, + persistSubagentRunsToDiskOrThrow: () => { + throw new Error("state db unavailable"); + }, + restoreSubagentRunsFromDisk: () => 0, + }); + + const result = await withPluginRuntimeGatewayRequestScope( + { + context: gatewayContext, + client: externalCliClient(), + isWebchatConnect: () => false, + }, + () => + spawnSubagentDirect( + { task: "orphan me", context: "isolated", lightContext: true }, + { agentSessionKey: "agent:main:main", requesterRunId: "parent-run" }, + ), + ); + + expect(result.status).toBe("error"); + expect(result.error ?? "").toContain("Failed to register subagent run"); + // No registry row exists, so an unaborted run would execute with no task row at all. + expect( + requests.some( + (request) => + request.method === "chat.abort" && request.params.runId === "gateway-accepted-run", + ), + ).toBe(true); + }); + + // The registry entry only counts as ownership once the canonical `subagent` task row + // exists. A task runtime is plugin-replaceable and may legally create no row, so both + // fault shapes have to fail registration and abort — otherwise the accepted child keeps + // running with the gateway CLI row suppressed and nothing in the tasks rail. + const taskRowFaults: Array<[label: string, createTaskRun: () => null]> = [ + ["creates no task row", () => null], + [ + "throws while creating the task row", + () => { + throw new Error("task store unavailable"); + }, + ], + ]; + it.each(taskRowFaults)( + "aborts the accepted child run when the task runtime %s", + async (_label, createTaskRun) => { + const gatewayContext = makeGatewayContext(); + const requests: Array<{ method: string; params: Record }> = []; + subagentSpawnTesting.setDepsForTest({ + dispatchGatewayMethodInProcess: async ( + method: string, + params: Record, + ) => { + requests.push({ method, params }); + if (method === "agent") { + return { runId: "gateway-accepted-run", status: "accepted" } as T; + } + if (method === "chat.abort") { + return { aborted: true, runIds: [params.runId] } as T; + } + return {} as T; + }, + }); + // Registry persistence succeeds here; only the task row is missing. + setDetachedTaskLifecycleRuntime({ + ...getDetachedTaskLifecycleRuntime(), + createQueuedTaskRun: createTaskRun, + createRunningTaskRun: createTaskRun, + }); + + const result = await withPluginRuntimeGatewayRequestScope( + { + context: gatewayContext, + client: externalCliClient(), + isWebchatConnect: () => false, + }, + () => + spawnSubagentDirect( + { task: "orphan me", context: "isolated", lightContext: true }, + { agentSessionKey: "agent:main:main", requesterRunId: "parent-run" }, + ), + ); + + expect(result.status).toBe("error"); + expect( + requests.some( + (request) => + request.method === "chat.abort" && request.params.runId === "gateway-accepted-run", + ), + ).toBe(true); + // Rolled back rather than half-registered: a retained entry would report a live run + // that owns no task row. + expect(subagentRuns.size).toBe(0); + }, + ); + + it("keeps the Gateway-owned task row on an out-of-process fallback", async () => { + const gatewayContext = makeGatewayContext(); + const requests: Array<{ method: string; params: Record }> = []; + const createTaskRun = vi.fn(() => { + throw new Error("registry task creation must be skipped"); + }); + subagentSpawnTesting.setDepsForTest({ + hasInProcessGatewayContext: () => false, + callGateway: async (request: { method: string; params?: unknown }) => { + requests.push({ + method: request.method, + params: (request.params ?? {}) as Record, + }); + return { + runId: request.method === "agent" ? "gateway-owned-run" : undefined, + status: "accepted", + } as T; + }, + }); + setDetachedTaskLifecycleRuntime({ + ...getDetachedTaskLifecycleRuntime(), + createQueuedTaskRun: createTaskRun, + createRunningTaskRun: createTaskRun, + }); + + const result = await withPluginRuntimeGatewayRequestScope( + { + context: gatewayContext, + client: externalCliClient(), + isWebchatConnect: () => false, + }, + () => + spawnSubagentDirect( + { task: "use the remote gateway row", context: "isolated", lightContext: true }, + { agentSessionKey: "agent:main:main", requesterRunId: "parent-run" }, + ), + ); + + expect(result.status).toBe("accepted"); + expect(result.runId).toBe("gateway-owned-run"); + expect(createTaskRun).not.toHaveBeenCalled(); + expect(subagentRuns.get("gateway-owned-run")).toMatchObject({ + childSessionKey: result.childSessionKey, + }); + expect(requests.filter((request) => request.method === "agent")).toHaveLength(1); + expect(requests.some((request) => request.method === "chat.abort")).toBe(false); + }); + + it("keeps the queued registry row when a collector starts out of process", async () => { + const gatewayContext = makeGatewayContext(); + const trackingModes: string[] = []; + subagentSpawnTesting.setDepsForTest({ + hasInProcessGatewayContext: () => false, + callGateway: async (request: { method: string; params?: unknown }) => { + const requestParams = (request.params ?? {}) as Record; + if (request.method === "agent") { + const client = createSyntheticPluginRuntimeClient(); + expect(client.internal?.agentRunTracking).toBeUndefined(); + trackingModes.push( + resolveGatewayAgentTaskTrackingMode({ + client, + sessionKey: requestParams.sessionKey as string, + runId: requestParams.idempotencyKey as string, + }), + ); + } + return { + runId: requestParams.idempotencyKey, + status: "accepted", + } as T; + }, + }); + + const result = await withPluginRuntimeGatewayRequestScope( + { + context: gatewayContext, + client: externalCliClient(), + isWebchatConnect: () => false, + }, + () => + spawnSubagentDirect( + { + task: "start after registry ownership", + collect: true, + context: "isolated", + lightContext: true, + groupId: "swarm-out-of-process", + swarmLaunchReplayKey: "code-mode:agentSpawn:out-of-process", + }, + { agentSessionKey: "agent:main:main", requesterRunId: "parent-run" }, + ), + ); + + expect(result.status).toBe("accepted"); + await waitForAssertion(() => { + expect(trackingModes).toEqual(["none"]); + expect(subagentRuns.get(result.runId!)).toMatchObject({ + collect: true, + swarmLaunchPending: false, + }); + }); + }); + + it("does not abort an out-of-process run when registry persistence fails", async () => { + const gatewayContext = makeGatewayContext(); + const requests: Array<{ method: string; params: Record }> = []; + subagentSpawnTesting.setDepsForTest({ + hasInProcessGatewayContext: () => false, + callGateway: async (request: { method: string; params?: unknown }) => { + requests.push({ + method: request.method, + params: (request.params ?? {}) as Record, + }); + return { + runId: request.method === "agent" ? "gateway-owned-unregistered-run" : undefined, + status: "accepted", + } as T; + }, + }); + subagentRegistryTesting.setDepsForTest({ + loadAgentRuntimePluginRegistryHandle: () => undefined, + persistSubagentRunsToDisk: () => {}, + persistSubagentRunsToDiskOrThrow: () => { + throw new Error("state db unavailable"); + }, + restoreSubagentRunsFromDisk: () => 0, + }); + + const result = await withPluginRuntimeGatewayRequestScope( + { + context: gatewayContext, + client: externalCliClient(), + isWebchatConnect: () => false, + }, + () => + spawnSubagentDirect( + { task: "keep remote ownership", context: "isolated", lightContext: true }, + { agentSessionKey: "agent:main:main", requesterRunId: "parent-run" }, + ), + ); + + expect(result.status).toBe("error"); + expect(result.error ?? "").toContain("Failed to register subagent run"); + expect(requests.some((request) => request.method === "chat.abort")).toBe(false); + }); + + it("launches child runs as a Gateway client that does not own a second task row", async () => { + const gatewayContext = makeGatewayContext(); + const agentDispatches: Array<{ + params: Record; + options?: NonNullable[2]>; + }> = []; + subagentSpawnTesting.setDepsForTest({ + dispatchGatewayMethodInProcess: async ( + method: string, + params: Record, + options?: NonNullable[2]>, + ) => { + if (method === "agent") { + agentDispatches.push({ params, options }); + } + return { runId: params.idempotencyKey as string, status: "accepted" } as T; + }, + }); + + const result = await withPluginRuntimeGatewayRequestScope( + { + context: gatewayContext, + client: externalCliClient(), + isWebchatConnect: () => false, + }, + () => + spawnSubagentDirect( + { + task: "summarize the repository", + context: "isolated", + lightContext: true, + }, + { + agentSessionKey: "agent:main:main", + requesterRunId: "parent-run", + }, + ), + ); + + expect(result.status).toBe("accepted"); + const runId = result.runId ?? ""; + expect(runId).toBeTruthy(); + // The registry owns the canonical `subagent` task row for this run. + await waitForAssertion(() => { + expect(subagentRuns.get(runId)).toMatchObject({ + childSessionKey: result.childSessionKey, + }); + }); + + const dispatch = agentDispatches[0]; + expect(dispatch).toBeDefined(); + // Rebuild the exact client the Gateway sees for this launch, then ask the + // real resolver whether it would write its own `cli` task row for the run. + const gatewayClient = createSyntheticPluginRuntimeClient({ + ...(dispatch?.options?.agentRunTracking + ? { agentRunTracking: dispatch.options.agentRunTracking } + : {}), + ...(dispatch?.options?.syntheticScopes ? { scopes: dispatch.options.syntheticScopes } : {}), + }); + expect( + resolveGatewayAgentTaskTrackingMode({ + client: gatewayClient, + sessionKey: dispatch?.params.sessionKey as string, + }), + ).toBe("none"); + }); }); diff --git a/src/agents/subagents/spawn/subagent-spawn.ts b/src/agents/subagents/spawn/subagent-spawn.ts index d7402e35b477..9d153e55537b 100644 --- a/src/agents/subagents/spawn/subagent-spawn.ts +++ b/src/agents/subagents/spawn/subagent-spawn.ts @@ -50,7 +50,7 @@ import type { SpawnSubagentResult, } from "./subagent-spawn-contract.js"; import { setSubagentSpawnDepsForTest } from "./subagent-spawn-deps.js"; -import { callSubagentGateway, readGatewayRunId } from "./subagent-spawn-gateway.js"; +import { callNativeSubagentGateway, readGatewayRunId } from "./subagent-spawn-gateway.js"; import { buildSubagentLaunchRequest } from "./subagent-spawn-launch-request.js"; import { createSubagentSpawnLifecycleEmitter } from "./subagent-spawn-lifecycle.js"; import { resolveSubagentSpawnRequest } from "./subagent-spawn-request.js"; @@ -372,7 +372,7 @@ export async function spawnSubagentDirect( agentId: targetAgentId, }); const launchChildRun = async () => - await callSubagentGateway( + await callNativeSubagentGateway( { method: "agent", params: childLaunch.request, @@ -403,6 +403,10 @@ export async function spawnSubagentDirect( waitForSessionDeletion, }); type SubagentBackendState = { contextEnginePreparation?: SubagentSpawnPreparation }; + // Set once the gateway accepts the child run, so a later failure can tell an + // accepted run apart from one that never started. + let acceptedChildRunId: string | undefined; + let taskRowOwnership: "required" | "gateway_best_effort" = "required"; const adapter: SpawnBackendAdapter = { async initialize() { const result = @@ -424,14 +428,27 @@ export async function spawnSubagentDirect( if (params.collect) { return { runId: childIdem }; } - const response = await launchChildRun(); - return { runId: readGatewayRunId(response) ?? childIdem }; + const launch = await launchChildRun(); + taskRowOwnership = launch.taskRowOwnership; + acceptedChildRunId = readGatewayRunId(launch.response) ?? childIdem; + return { runId: acceptedChildRunId }; }, async cleanupOnFailure({ phase, state }) { if (phase === "initialize") { await cleanupFailedSpawn(); return; } + // The gateway skips its fallback CLI task row because this launch claims + // the run's row, and registration is what delivers it. A register failure + // means no owner ever recorded the run, so abort the run the gateway + // already accepted instead of leaving it executing unrecorded. + if (phase === "register" && acceptedChildRunId && taskRowOwnership === "required") { + await terminateAcceptedCollectorRun({ + childSessionKey, + gatewayRunId: acceptedChildRunId, + ...provisionalSessionIdentity, + }); + } await rollbackPreparedContextEngine(state?.contextEnginePreparation); if (attachmentAbsDir) { try { @@ -518,6 +535,7 @@ export async function spawnSubagentDirect( groupId: swarmGroupId, queuedLaunch, queued: params.collect === true, + taskRowOwnership, attachmentsDir: attachmentAbsDir, attachmentsRootDir: attachmentRootDir, retainAttachmentsOnKeep: retainOnSessionKeep, @@ -549,8 +567,10 @@ export async function spawnSubagentDirect( runId: childRunId, start: async () => { await runWithGatewayIndependentRootWorkContinuation(async () => { - const response = await launchChildRun(); - const gatewayRunId = readGatewayRunId(response) ?? childRunId; + const launch = await launchChildRun(); + // Queued registration already owns the task row before either dispatch route starts. + // Out-of-process Gateway tracking finds that exact runId and suppresses its CLI row. + const gatewayRunId = readGatewayRunId(launch.response) ?? childRunId; try { if (!startQueuedSubagentRun(childRunId, gatewayRunId)) { throw new Error( diff --git a/src/gateway/server-methods/agent-task-tracking.ts b/src/gateway/server-methods/agent-task-tracking.ts index e6abefe0eb3d..2c5f4f44b67d 100644 --- a/src/gateway/server-methods/agent-task-tracking.ts +++ b/src/gateway/server-methods/agent-task-tracking.ts @@ -99,7 +99,8 @@ export function resolveGatewayAgentTaskTrackingMode(params: { if (!params.sessionKey?.trim() || params.inputProvenance?.kind === "inter_session") { return "none"; } - if (params.client?.internal?.agentRunTracking === "plugin_subagent") { + const runTaskOwner = params.client?.internal?.agentRunTracking; + if (runTaskOwner === "plugin_subagent") { return "plugin_subagent"; } // The subagent registry created the authoritative row before its host-owned @@ -111,6 +112,12 @@ export function resolveGatewayAgentTaskTrackingMode(params: { ) { return "none"; } + // The native spawn control plane registers the canonical `subagent` row for + // this same runId once the gateway returns, so tracking here would show one + // run twice. The marker rides an internal synthetic client only. + if (runTaskOwner === "native_subagent") { + return "none"; + } // A confirmed ACP manual-spawn child turn already owns its requester-visible // `acp` task row from the spawn control plane (src/agents/subagents/spawn/acp-spawn.ts). The // Gateway CLI path runs that same childRunId, so tracking it here would emit a diff --git a/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts b/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts index 1a98edeed3e8..97486e662c64 100644 --- a/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts +++ b/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts @@ -46,6 +46,35 @@ import { const mocks = getAgentTestMocks(); +// Shared by every spawn control plane whose child turn reaches the gateway as a +// plain `agent` run: ACP manual spawns, plugin subagents, and native subagents. +function mockSpawnedChildSessionEntry(childSessionKey: string) { + mocks.loadSessionEntry.mockReturnValue({ + cfg: {}, + storePath: "/tmp/sessions.json", + entry: { sessionId: "spawned-child-session", updatedAt: Date.now() }, + canonicalKey: childSessionKey, + }); + mocks.updateSessionStore.mockResolvedValue(undefined); + mocks.agentCommand.mockResolvedValue({ + payloads: [{ text: "ok" }], + meta: { durationMs: 100 }, + }); +} + +function spyDetachedCreateRunningTaskRun() { + const defaultRuntime = getDetachedTaskLifecycleRuntime(); + const createRunningTaskRunSpy = vi.fn( + (...args: Parameters) => + defaultRuntime.createRunningTaskRun(...args), + ); + setDetachedTaskLifecycleRuntime({ + ...defaultRuntime, + createRunningTaskRun: createRunningTaskRunSpy, + }); + return createRunningTaskRunSpy; +} + describe("gateway agent handler", () => { afterEach(describe0AfterEach0); @@ -2526,33 +2555,6 @@ describe("gateway agent handler", () => { }); describe("ACP manual-spawn child turn task tracking", () => { - function mockAcpChildSessionEntry(childSessionKey: string) { - mocks.loadSessionEntry.mockReturnValue({ - cfg: {}, - storePath: "/tmp/sessions.json", - entry: { sessionId: "acp-child-session", updatedAt: Date.now() }, - canonicalKey: childSessionKey, - }); - mocks.updateSessionStore.mockResolvedValue(undefined); - mocks.agentCommand.mockResolvedValue({ - payloads: [{ text: "ok" }], - meta: { durationMs: 100 }, - }); - } - - function spyDetachedCreateRunningTaskRun() { - const defaultRuntime = getDetachedTaskLifecycleRuntime(); - const createRunningTaskRunSpy = vi.fn( - (...args: Parameters) => - defaultRuntime.createRunningTaskRun(...args), - ); - setDetachedTaskLifecycleRuntime({ - ...defaultRuntime, - createRunningTaskRun: createRunningTaskRunSpy, - }); - return createRunningTaskRunSpy; - } - const confirmedAcpMeta: NonNullable> = { backend: "acpx", agent: "codex", @@ -2567,7 +2569,7 @@ describe("gateway agent handler", () => { useTestStateDir(root); resetAgentTaskRegistryForTests(); const childSessionKey = "agent:main:acp:child-confirmed"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta); const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun(); @@ -2593,7 +2595,7 @@ describe("gateway agent handler", () => { resetAgentTaskRegistryForTests(); const childSessionKey = "agent:main:subagent:owned"; const runId = "host-owned-subagent-run"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); getDetachedTaskLifecycleRuntime().createRunningTaskRun({ runtime: "subagent", requesterSessionKey: "agent:main:main", @@ -2624,7 +2626,7 @@ describe("gateway agent handler", () => { useTestStateDir(root); resetAgentTaskRegistryForTests(); const childSessionKey = "agent:main:acp:child-operator-write"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); // Persisted ACP metadata is present and the turn looks like a manual // spawn, but the caller is an operator-write control-UI client, not the // in-process backend ACP spawn path. That caller never creates a @@ -2664,7 +2666,7 @@ describe("gateway agent handler", () => { useTestStateDir(root); resetAgentTaskRegistryForTests(); const childSessionKey = "agent:main:acp:child-missing-meta"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); mocks.readAcpSessionMeta.mockReturnValue(undefined); const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun(); @@ -2699,7 +2701,7 @@ describe("gateway agent handler", () => { useTestStateDir(root); resetAgentTaskRegistryForTests(); const childSessionKey = "agent:main:acp:child-meta-throw"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); const metadataError = new Error("state db unavailable"); mocks.readAcpSessionMeta.mockImplementation(() => { throw metadataError; @@ -2750,7 +2752,7 @@ describe("gateway agent handler", () => { useTestStateDir(root); resetAgentTaskRegistryForTests(); const childSessionKey = "agent:main:acp:child-not-spawn"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); // Metadata is present but the turn lacks acpTurnSource, so the spawn // control plane does not own this row; CLI tracking must stay on. mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta); @@ -2782,7 +2784,7 @@ describe("gateway agent handler", () => { resetSubagentRegistryForTests({ persist: false }); const childSessionKey = "agent:main:acp:plugin-child"; const runId = "acp-plugin-subagent-run"; - mockAcpChildSessionEntry(childSessionKey); + mockSpawnedChildSessionEntry(childSessionKey); mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta); const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun(); @@ -2828,6 +2830,67 @@ describe("gateway agent handler", () => { }); }); + describe("native subagent child run task tracking", () => { + function nativeSubagentClient(): AgentHandlerArgs["client"] { + const baseClient = requireValue(backendGatewayClient(), "expected backend client"); + return { + connect: baseClient.connect, + internal: { ...baseClient.internal, agentRunTracking: "native_subagent" }, + }; + } + + it("suppresses the gateway CLI task row for native subagent child runs", async () => { + await withTestDir({ prefix: "openclaw-gateway-native-subagent-" }, async (root) => { + useTestStateDir(root); + resetAgentTaskRegistryForTests(); + const childSessionKey = "agent:main:subagent:native-child"; + const runId = "native-subagent-run"; + mockSpawnedChildSessionEntry(childSessionKey); + const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun(); + + await invokeAgent( + { + message: "native subagent child run", + sessionKey: childSessionKey, + idempotencyKey: runId, + }, + { reqId: runId, client: nativeSubagentClient() }, + ); + await waitForAgentCommandCall(); + + // src/agents/subagent-spawn.ts owns the `subagent` row for this runId. + expect(createRunningTaskRunSpy).not.toHaveBeenCalled(); + expect(findTaskByRunId(runId)).toBeUndefined(); + }); + }); + + it("keeps CLI tracking for an unmarked backend turn on a subagent session", async () => { + await withTestDir({ prefix: "openclaw-gateway-native-subagent-unmarked-" }, async (root) => { + useTestStateDir(root); + resetAgentTaskRegistryForTests(); + const childSessionKey = "agent:main:subagent:unmarked-child"; + const runId = "native-subagent-unmarked"; + mockSpawnedChildSessionEntry(childSessionKey); + const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun(); + + // An operator follow-up to a subagent session owns no registry row, so + // suppressing here would lose the run from the tasks rail entirely. + await invokeAgent( + { message: "operator follow-up", sessionKey: childSessionKey, idempotencyKey: runId }, + { reqId: runId, client: backendGatewayClient() }, + ); + await waitForAgentCommandCall(); + + expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1); + expectRecordFields(mockCallArg(createRunningTaskRunSpy), { + runtime: "cli", + runId, + childSessionKey, + }); + }); + }); + }); + it("logs a swallowed finalize error without blocking the background run", async () => { await withTestDir({ prefix: "openclaw-gateway-agent-finalize-throw-" }, async (root) => { useTestStateDir(root); diff --git a/src/gateway/server-methods/shared-types.ts b/src/gateway/server-methods/shared-types.ts index 576ecb54181f..d4df6df182e0 100644 --- a/src/gateway/server-methods/shared-types.ts +++ b/src/gateway/server-methods/shared-types.ts @@ -73,6 +73,11 @@ import type { TrustedSessionCreation } from "./session-creation-provenance.js"; */ type SubsystemLogger = ReturnType; +/** Trusted in-process spawn control plane that already owns this run's task row. + Gateway CLI tracking only covers runs nobody else records, so a marked run + must never get a second row. */ +export type GatewayAgentRunTaskOwner = "plugin_subagent" | "native_subagent"; + /** Per-connection client metadata captured after the gateway handshake. */ export type GatewayClient = { connect: ConnectParams; @@ -113,7 +118,7 @@ export type GatewayClient = { cronRunContinuation?: boolean; agentRuntimeIdentity?: AgentRuntimeIdentity; pluginRuntimeOwnerId?: string; - agentRunTracking?: "plugin_subagent"; + agentRunTracking?: GatewayAgentRunTaskOwner; /** Host-captured requester lineage for opt-in plugin subagent completion delivery. */ pluginSubagentRequester?: PluginSubagentRequesterContext; /** Host-owned exact media set for a scoped automatic recovery delivery. */ diff --git a/src/gateway/server-plugin-in-process-dispatch.ts b/src/gateway/server-plugin-in-process-dispatch.ts index f068999506fa..95329e67dc27 100644 --- a/src/gateway/server-plugin-in-process-dispatch.ts +++ b/src/gateway/server-plugin-in-process-dispatch.ts @@ -11,7 +11,11 @@ import { } from "./server-in-process-dispatch.js"; import type { AgentRunRequest } from "./server-methods/agent-request-types.js"; import type { TrustedSessionCreation } from "./server-methods/session-creation-provenance.js"; -import type { GatewayRequestContext, GatewayRequestOptions } from "./server-methods/types.js"; +import type { + GatewayAgentRunTaskOwner, + GatewayRequestContext, + GatewayRequestOptions, +} from "./server-methods/types.js"; import { getFallbackGatewayContext } from "./server-plugin-fallback-context.js"; import { createSyntheticPluginRuntimeClient, @@ -29,7 +33,7 @@ const loadInternalAgentTurnFacade = createLazyRuntimeModule( type DispatchGatewayMethodInProcessOptions = { allowSyntheticModelOverride?: boolean; allowSyntheticCronRunContinuation?: boolean; - agentRunTracking?: "plugin_subagent"; + agentRunTracking?: GatewayAgentRunTaskOwner; disableSyntheticClient?: boolean; expectFinal?: boolean; forceSyntheticClient?: boolean; diff --git a/src/gateway/server-plugin-runtime-client.ts b/src/gateway/server-plugin-runtime-client.ts index 91aea6412a7a..29f94c019537 100644 --- a/src/gateway/server-plugin-runtime-client.ts +++ b/src/gateway/server-plugin-runtime-client.ts @@ -12,11 +12,11 @@ import type { PluginSubagentRequesterContext } from "../plugins/runtime/subagent import type { RuntimePluginToolGrant } from "../plugins/runtime/tool-grant.js"; import { APPROVALS_SCOPE, WRITE_SCOPE } from "./method-scopes.js"; import type { TrustedSessionCreation } from "./server-methods/session-creation-provenance.js"; -import type { GatewayRequestOptions } from "./server-methods/types.js"; +import type { GatewayAgentRunTaskOwner, GatewayRequestOptions } from "./server-methods/types.js"; export function createSyntheticPluginRuntimeClient(params?: { allowModelOverride?: boolean; - agentRunTracking?: "plugin_subagent"; + agentRunTracking?: GatewayAgentRunTaskOwner; cronRunContinuation?: boolean; internalDeliveryMediaUrls?: string[]; internalDeliverySuppressText?: boolean;