Skip to content

Commit a9eb0d7

Browse files
moeedahmedmoeedahmed
authored
Fix ACP manual-spawn task tracking (#97131)
Co-authored-by: Moeed Ahmed <[email protected]>
1 parent 9202dbb commit a9eb0d7

2 files changed

Lines changed: 369 additions & 3 deletions

File tree

‎src/gateway/server-methods/agent.test.ts‎

Lines changed: 304 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import fs from "node:fs/promises";
44
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
55
import { ErrorCodes } from "../../../packages/gateway-protocol/src/index.js";
6+
import type { readAcpSessionMeta } from "../../acp/runtime/session-meta.js";
67
import {
78
registerExecApprovalFollowupRuntimeHandoff,
89
resetExecApprovalFollowupRuntimeHandoffsForTests,
@@ -48,6 +49,7 @@ const mocks = vi.hoisted(() => ({
4849
getLatestSubagentRunByChildSessionKey: vi.fn(),
4950
replaceSubagentRunAfterSteer: vi.fn(),
5051
resolveExplicitAgentSessionKey: vi.fn(),
52+
readAcpSessionMeta: vi.fn<typeof readAcpSessionMeta>(() => undefined),
5153
listAgentIds: vi.fn(() => ["main"]),
5254
loadConfigReturn: {} as Record<string, unknown>,
5355
loadVoiceWakeRoutingConfig: vi.fn(),
@@ -101,6 +103,16 @@ vi.mock("../../commands/agent.js", () => ({
101103
agentCommandFromIngress: mocks.agentCommand,
102104
}));
103105

106+
vi.mock("../../acp/runtime/session-meta.js", async () => {
107+
const actual = await vi.importActual<typeof import("../../acp/runtime/session-meta.js")>(
108+
"../../acp/runtime/session-meta.js",
109+
);
110+
return {
111+
...actual,
112+
readAcpSessionMeta: mocks.readAcpSessionMeta,
113+
};
114+
});
115+
104116
vi.mock("../../config/config.js", async () => {
105117
const actual =
106118
await vi.importActual<typeof import("../../config/config.js")>("../../config/config.js");
@@ -483,6 +495,25 @@ function backendGatewayClient(): AgentHandlerArgs["client"] {
483495
} as AgentHandlerArgs["client"];
484496
}
485497

498+
// Operator-write client that is NOT the in-process backend ACP spawn caller:
499+
// a control-UI connection with the same operator.write scope. It can set
500+
// acpTurnSource but owns no replacement `acp` task row, so CLI tracking stays on.
501+
function operatorWriteGatewayClient(): AgentHandlerArgs["client"] {
502+
return {
503+
connect: {
504+
minProtocol: 1,
505+
maxProtocol: 1,
506+
client: {
507+
id: "openclaw-control-ui",
508+
version: "test",
509+
platform: "test",
510+
mode: "ui",
511+
},
512+
scopes: ["operator.write"],
513+
},
514+
} as AgentHandlerArgs["client"];
515+
}
516+
486517
async function waitForAgentCommandCall<
487518
T extends AgentCommandCall = AgentCommandCall,
488519
>(): Promise<T> {
@@ -576,6 +607,7 @@ describe("gateway agent handler", () => {
576607
mocks.emitGatewaySessionEndPluginHook.mockReset();
577608
mocks.emitGatewaySessionStartPluginHook.mockReset();
578609
mocks.resolveExplicitAgentSessionKey.mockReset().mockReturnValue(undefined);
610+
mocks.readAcpSessionMeta.mockReset().mockReturnValue(undefined);
579611
mocks.listAgentIds.mockReset().mockReturnValue(["main"]);
580612
mocks.getChannelPlugin.mockReset();
581613
mocks.sendDurableMessageBatch.mockReset();
@@ -4736,6 +4768,277 @@ describe("gateway agent handler", () => {
47364768
});
47374769
});
47384770

4771+
describe("ACP manual-spawn child turn task tracking", () => {
4772+
function mockAcpChildSessionEntry(childSessionKey: string) {
4773+
mocks.loadSessionEntry.mockReturnValue({
4774+
cfg: {},
4775+
storePath: "/tmp/sessions.json",
4776+
entry: { sessionId: "acp-child-session", updatedAt: Date.now() },
4777+
canonicalKey: childSessionKey,
4778+
});
4779+
mocks.updateSessionStore.mockResolvedValue(undefined);
4780+
mocks.agentCommand.mockResolvedValue({
4781+
payloads: [{ text: "ok" }],
4782+
meta: { durationMs: 100 },
4783+
});
4784+
}
4785+
4786+
function spyDetachedCreateRunningTaskRun() {
4787+
const defaultRuntime = getDetachedTaskLifecycleRuntime();
4788+
const createRunningTaskRunSpy = vi.fn(
4789+
(...args: Parameters<typeof defaultRuntime.createRunningTaskRun>) =>
4790+
defaultRuntime.createRunningTaskRun(...args),
4791+
);
4792+
setDetachedTaskLifecycleRuntime({
4793+
...defaultRuntime,
4794+
createRunningTaskRun: createRunningTaskRunSpy,
4795+
});
4796+
return createRunningTaskRunSpy;
4797+
}
4798+
4799+
const confirmedAcpMeta: NonNullable<ReturnType<typeof readAcpSessionMeta>> = {
4800+
backend: "acpx",
4801+
agent: "codex",
4802+
runtimeSessionName: "runtime-1",
4803+
mode: "persistent",
4804+
state: "idle",
4805+
lastActivityAt: Date.now(),
4806+
};
4807+
4808+
it("suppresses the gateway CLI task row for confirmed ACP manual-spawn child turns", async () => {
4809+
await withTempDir({ prefix: "openclaw-gateway-acp-suppress-" }, async (root) => {
4810+
useTestStateDir(root);
4811+
resetTaskRegistryForTests();
4812+
const childSessionKey = "agent:main:acp:child-confirmed";
4813+
mockAcpChildSessionEntry(childSessionKey);
4814+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4815+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4816+
4817+
await invokeAgent(
4818+
{
4819+
message: "acp manual spawn child turn",
4820+
sessionKey: childSessionKey,
4821+
acpTurnSource: "manual_spawn",
4822+
idempotencyKey: "acp-manual-spawn-confirmed",
4823+
},
4824+
{ reqId: "acp-manual-spawn-confirmed", client: backendGatewayClient() },
4825+
);
4826+
await waitForAgentCommandCall();
4827+
4828+
expect(createRunningTaskRunSpy).not.toHaveBeenCalled();
4829+
expect(findTaskByRunId("acp-manual-spawn-confirmed")).toBeUndefined();
4830+
});
4831+
});
4832+
4833+
it("keeps CLI tracking when a non-backend operator-write caller sets acpTurnSource", async () => {
4834+
await withTempDir({ prefix: "openclaw-gateway-acp-operator-write-" }, async (root) => {
4835+
useTestStateDir(root);
4836+
resetTaskRegistryForTests();
4837+
const childSessionKey = "agent:main:acp:child-operator-write";
4838+
mockAcpChildSessionEntry(childSessionKey);
4839+
// Persisted ACP metadata is present and the turn looks like a manual
4840+
// spawn, but the caller is an operator-write control-UI client, not the
4841+
// in-process backend ACP spawn path. That caller never creates a
4842+
// replacement `acp` row, so CLI tracking must stay on to avoid losing the
4843+
// run entirely.
4844+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4845+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4846+
4847+
await invokeAgent(
4848+
{
4849+
message: "operator-write acp manual spawn",
4850+
sessionKey: childSessionKey,
4851+
acpTurnSource: "manual_spawn",
4852+
idempotencyKey: "acp-operator-write",
4853+
},
4854+
{ reqId: "acp-operator-write", client: operatorWriteGatewayClient() },
4855+
);
4856+
await waitForAgentCommandCall();
4857+
4858+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4859+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4860+
runtime: "cli",
4861+
runId: "acp-operator-write",
4862+
childSessionKey,
4863+
});
4864+
await waitForAssertion(() => {
4865+
expectRecordFields(findTaskByRunId("acp-operator-write"), {
4866+
runtime: "cli",
4867+
childSessionKey,
4868+
});
4869+
});
4870+
});
4871+
});
4872+
4873+
it("keeps CLI tracking for ACP-shaped manual-spawn turns without persisted ACP metadata", async () => {
4874+
await withTempDir({ prefix: "openclaw-gateway-acp-no-meta-" }, async (root) => {
4875+
useTestStateDir(root);
4876+
resetTaskRegistryForTests();
4877+
const childSessionKey = "agent:main:acp:child-missing-meta";
4878+
mockAcpChildSessionEntry(childSessionKey);
4879+
mocks.readAcpSessionMeta.mockReturnValue(undefined);
4880+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4881+
4882+
await invokeAgent(
4883+
{
4884+
message: "acp shaped turn without metadata",
4885+
sessionKey: childSessionKey,
4886+
acpTurnSource: "manual_spawn",
4887+
idempotencyKey: "acp-manual-spawn-no-meta",
4888+
},
4889+
{ reqId: "acp-manual-spawn-no-meta", client: backendGatewayClient() },
4890+
);
4891+
await waitForAgentCommandCall();
4892+
4893+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4894+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4895+
runtime: "cli",
4896+
runId: "acp-manual-spawn-no-meta",
4897+
childSessionKey,
4898+
});
4899+
await waitForAssertion(() => {
4900+
expectRecordFields(findTaskByRunId("acp-manual-spawn-no-meta"), {
4901+
runtime: "cli",
4902+
childSessionKey,
4903+
});
4904+
});
4905+
});
4906+
});
4907+
4908+
it("keeps dispatch and CLI tracking when ACP metadata read fails", async () => {
4909+
await withTempDir({ prefix: "openclaw-gateway-acp-meta-throw-" }, async (root) => {
4910+
useTestStateDir(root);
4911+
resetTaskRegistryForTests();
4912+
const childSessionKey = "agent:main:acp:child-meta-throw";
4913+
mockAcpChildSessionEntry(childSessionKey);
4914+
const metadataError = new Error("state db unavailable");
4915+
mocks.readAcpSessionMeta.mockImplementation(() => {
4916+
throw metadataError;
4917+
});
4918+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4919+
const context = makeContext();
4920+
4921+
await invokeAgent(
4922+
{
4923+
message: "acp manual spawn metadata throw",
4924+
sessionKey: childSessionKey,
4925+
acpTurnSource: "manual_spawn",
4926+
idempotencyKey: "acp-manual-spawn-meta-throw",
4927+
},
4928+
{ reqId: "acp-manual-spawn-meta-throw", context, client: backendGatewayClient() },
4929+
);
4930+
await waitForAgentCommandCall();
4931+
4932+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4933+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4934+
runtime: "cli",
4935+
runId: "acp-manual-spawn-meta-throw",
4936+
childSessionKey,
4937+
});
4938+
await waitForAssertion(() => {
4939+
expectRecordFields(findTaskByRunId("acp-manual-spawn-meta-throw"), {
4940+
runtime: "cli",
4941+
childSessionKey,
4942+
status: "succeeded",
4943+
terminalSummary: "completed",
4944+
});
4945+
});
4946+
const warnMock = context.logGateway.warn as ReturnType<typeof vi.fn>;
4947+
expect(
4948+
warnMock.mock.calls.some(([message]) => {
4949+
return (
4950+
typeof message === "string" &&
4951+
message.includes("failed to read ACP session metadata") &&
4952+
message.includes("falling back to cli task tracking")
4953+
);
4954+
}),
4955+
).toBe(true);
4956+
});
4957+
});
4958+
4959+
it("keeps CLI tracking for ACP-shaped turns that are not manual spawns", async () => {
4960+
await withTempDir({ prefix: "openclaw-gateway-acp-not-manual-spawn-" }, async (root) => {
4961+
useTestStateDir(root);
4962+
resetTaskRegistryForTests();
4963+
const childSessionKey = "agent:main:acp:child-not-spawn";
4964+
mockAcpChildSessionEntry(childSessionKey);
4965+
// Metadata is present but the turn lacks acpTurnSource, so the spawn
4966+
// control plane does not own this row; CLI tracking must stay on.
4967+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4968+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4969+
4970+
await invokeAgent(
4971+
{
4972+
message: "acp shaped non-spawn turn",
4973+
sessionKey: childSessionKey,
4974+
idempotencyKey: "acp-not-manual-spawn",
4975+
},
4976+
{ reqId: "acp-not-manual-spawn", client: backendGatewayClient() },
4977+
);
4978+
await waitForAgentCommandCall();
4979+
4980+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4981+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4982+
runtime: "cli",
4983+
runId: "acp-not-manual-spawn",
4984+
childSessionKey,
4985+
});
4986+
});
4987+
});
4988+
4989+
it("does not affect plugin-subagent tracking for confirmed ACP conditions", async () => {
4990+
await withTempDir({ prefix: "openclaw-gateway-acp-plugin-subagent-" }, async (root) => {
4991+
useTestStateDir(root);
4992+
resetTaskRegistryForTests();
4993+
resetSubagentRegistryForTests({ persist: false });
4994+
const childSessionKey = "agent:main:acp:plugin-child";
4995+
const runId = "acp-plugin-subagent-run";
4996+
mockAcpChildSessionEntry(childSessionKey);
4997+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4998+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4999+
5000+
const baseClient = requireValue(backendGatewayClient(), "expected backend client");
5001+
const pluginClient: AgentHandlerArgs["client"] = {
5002+
connect: baseClient.connect,
5003+
internal: {
5004+
...baseClient.internal,
5005+
agentRunTracking: "plugin_subagent",
5006+
pluginRuntimeOwnerId: "memory-core",
5007+
},
5008+
};
5009+
5010+
await invokeAgent(
5011+
{
5012+
message: "plugin subagent over acp child",
5013+
sessionKey: childSessionKey,
5014+
acpTurnSource: "manual_spawn",
5015+
idempotencyKey: runId,
5016+
},
5017+
{ reqId: runId, client: pluginClient },
5018+
);
5019+
await waitForAgentCommandCall();
5020+
5021+
// plugin_subagent precedence means the run is tracked through the
5022+
// subagent registry as a `subagent` row, never a duplicate `cli` row.
5023+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
5024+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
5025+
runtime: "subagent",
5026+
runId,
5027+
});
5028+
expect(
5029+
listTaskRecords().some((task) => task.runId === runId && task.runtime === "cli"),
5030+
).toBe(false);
5031+
await waitForAssertion(() => {
5032+
expectRecordFields(getSubagentRunByChildSessionKey(childSessionKey), {
5033+
runId,
5034+
childSessionKey,
5035+
label: "plugin:memory-core",
5036+
});
5037+
});
5038+
});
5039+
});
5040+
});
5041+
47395042
it("logs a swallowed finalize error without blocking the background run", async () => {
47405043
await withTempDir({ prefix: "openclaw-gateway-agent-finalize-throw-" }, async (root) => {
47415044
useTestStateDir(root);
@@ -5722,6 +6025,7 @@ describe("gateway agent handler chat.abort integration", () => {
57226025
mocks.getLatestSubagentRunByChildSessionKey.mockReset();
57236026
mocks.replaceSubagentRunAfterSteer.mockReset();
57246027
mocks.resolveExplicitAgentSessionKey.mockReset().mockReturnValue(undefined);
6028+
mocks.readAcpSessionMeta.mockReset().mockReturnValue(undefined);
57256029
mocks.listAgentIds.mockReset().mockReturnValue(["main"]);
57266030
mocks.getChannelPlugin.mockReset();
57276031
mocks.sendDurableMessageBatch.mockReset();

0 commit comments

Comments
 (0)