Skip to content

Commit a428638

Browse files
moeedahmedmoeedahmed
committed
Fix ACP manual-spawn task tracking
1 parent 1bccd29 commit a428638

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();
@@ -4683,6 +4715,277 @@ describe("gateway agent handler", () => {
46834715
});
46844716
});
46854717

4718+
describe("ACP manual-spawn child turn task tracking", () => {
4719+
function mockAcpChildSessionEntry(childSessionKey: string) {
4720+
mocks.loadSessionEntry.mockReturnValue({
4721+
cfg: {},
4722+
storePath: "/tmp/sessions.json",
4723+
entry: { sessionId: "acp-child-session", updatedAt: Date.now() },
4724+
canonicalKey: childSessionKey,
4725+
});
4726+
mocks.updateSessionStore.mockResolvedValue(undefined);
4727+
mocks.agentCommand.mockResolvedValue({
4728+
payloads: [{ text: "ok" }],
4729+
meta: { durationMs: 100 },
4730+
});
4731+
}
4732+
4733+
function spyDetachedCreateRunningTaskRun() {
4734+
const defaultRuntime = getDetachedTaskLifecycleRuntime();
4735+
const createRunningTaskRunSpy = vi.fn(
4736+
(...args: Parameters<typeof defaultRuntime.createRunningTaskRun>) =>
4737+
defaultRuntime.createRunningTaskRun(...args),
4738+
);
4739+
setDetachedTaskLifecycleRuntime({
4740+
...defaultRuntime,
4741+
createRunningTaskRun: createRunningTaskRunSpy,
4742+
});
4743+
return createRunningTaskRunSpy;
4744+
}
4745+
4746+
const confirmedAcpMeta: NonNullable<ReturnType<typeof readAcpSessionMeta>> = {
4747+
backend: "acpx",
4748+
agent: "codex",
4749+
runtimeSessionName: "runtime-1",
4750+
mode: "persistent",
4751+
state: "idle",
4752+
lastActivityAt: Date.now(),
4753+
};
4754+
4755+
it("suppresses the gateway CLI task row for confirmed ACP manual-spawn child turns", async () => {
4756+
await withTempDir({ prefix: "openclaw-gateway-acp-suppress-" }, async (root) => {
4757+
useTestStateDir(root);
4758+
resetTaskRegistryForTests();
4759+
const childSessionKey = "agent:main:acp:child-confirmed";
4760+
mockAcpChildSessionEntry(childSessionKey);
4761+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4762+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4763+
4764+
await invokeAgent(
4765+
{
4766+
message: "acp manual spawn child turn",
4767+
sessionKey: childSessionKey,
4768+
acpTurnSource: "manual_spawn",
4769+
idempotencyKey: "acp-manual-spawn-confirmed",
4770+
},
4771+
{ reqId: "acp-manual-spawn-confirmed", client: backendGatewayClient() },
4772+
);
4773+
await waitForAgentCommandCall();
4774+
4775+
expect(createRunningTaskRunSpy).not.toHaveBeenCalled();
4776+
expect(findTaskByRunId("acp-manual-spawn-confirmed")).toBeUndefined();
4777+
});
4778+
});
4779+
4780+
it("keeps CLI tracking when a non-backend operator-write caller sets acpTurnSource", async () => {
4781+
await withTempDir({ prefix: "openclaw-gateway-acp-operator-write-" }, async (root) => {
4782+
useTestStateDir(root);
4783+
resetTaskRegistryForTests();
4784+
const childSessionKey = "agent:main:acp:child-operator-write";
4785+
mockAcpChildSessionEntry(childSessionKey);
4786+
// Persisted ACP metadata is present and the turn looks like a manual
4787+
// spawn, but the caller is an operator-write control-UI client, not the
4788+
// in-process backend ACP spawn path. That caller never creates a
4789+
// replacement `acp` row, so CLI tracking must stay on to avoid losing the
4790+
// run entirely.
4791+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4792+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4793+
4794+
await invokeAgent(
4795+
{
4796+
message: "operator-write acp manual spawn",
4797+
sessionKey: childSessionKey,
4798+
acpTurnSource: "manual_spawn",
4799+
idempotencyKey: "acp-operator-write",
4800+
},
4801+
{ reqId: "acp-operator-write", client: operatorWriteGatewayClient() },
4802+
);
4803+
await waitForAgentCommandCall();
4804+
4805+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4806+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4807+
runtime: "cli",
4808+
runId: "acp-operator-write",
4809+
childSessionKey,
4810+
});
4811+
await waitForAssertion(() => {
4812+
expectRecordFields(findTaskByRunId("acp-operator-write"), {
4813+
runtime: "cli",
4814+
childSessionKey,
4815+
});
4816+
});
4817+
});
4818+
});
4819+
4820+
it("keeps CLI tracking for ACP-shaped manual-spawn turns without persisted ACP metadata", async () => {
4821+
await withTempDir({ prefix: "openclaw-gateway-acp-no-meta-" }, async (root) => {
4822+
useTestStateDir(root);
4823+
resetTaskRegistryForTests();
4824+
const childSessionKey = "agent:main:acp:child-missing-meta";
4825+
mockAcpChildSessionEntry(childSessionKey);
4826+
mocks.readAcpSessionMeta.mockReturnValue(undefined);
4827+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4828+
4829+
await invokeAgent(
4830+
{
4831+
message: "acp shaped turn without metadata",
4832+
sessionKey: childSessionKey,
4833+
acpTurnSource: "manual_spawn",
4834+
idempotencyKey: "acp-manual-spawn-no-meta",
4835+
},
4836+
{ reqId: "acp-manual-spawn-no-meta", client: backendGatewayClient() },
4837+
);
4838+
await waitForAgentCommandCall();
4839+
4840+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4841+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4842+
runtime: "cli",
4843+
runId: "acp-manual-spawn-no-meta",
4844+
childSessionKey,
4845+
});
4846+
await waitForAssertion(() => {
4847+
expectRecordFields(findTaskByRunId("acp-manual-spawn-no-meta"), {
4848+
runtime: "cli",
4849+
childSessionKey,
4850+
});
4851+
});
4852+
});
4853+
});
4854+
4855+
it("keeps dispatch and CLI tracking when ACP metadata read fails", async () => {
4856+
await withTempDir({ prefix: "openclaw-gateway-acp-meta-throw-" }, async (root) => {
4857+
useTestStateDir(root);
4858+
resetTaskRegistryForTests();
4859+
const childSessionKey = "agent:main:acp:child-meta-throw";
4860+
mockAcpChildSessionEntry(childSessionKey);
4861+
const metadataError = new Error("state db unavailable");
4862+
mocks.readAcpSessionMeta.mockImplementation(() => {
4863+
throw metadataError;
4864+
});
4865+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4866+
const context = makeContext();
4867+
4868+
await invokeAgent(
4869+
{
4870+
message: "acp manual spawn metadata throw",
4871+
sessionKey: childSessionKey,
4872+
acpTurnSource: "manual_spawn",
4873+
idempotencyKey: "acp-manual-spawn-meta-throw",
4874+
},
4875+
{ reqId: "acp-manual-spawn-meta-throw", context, client: backendGatewayClient() },
4876+
);
4877+
await waitForAgentCommandCall();
4878+
4879+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4880+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4881+
runtime: "cli",
4882+
runId: "acp-manual-spawn-meta-throw",
4883+
childSessionKey,
4884+
});
4885+
await waitForAssertion(() => {
4886+
expectRecordFields(findTaskByRunId("acp-manual-spawn-meta-throw"), {
4887+
runtime: "cli",
4888+
childSessionKey,
4889+
status: "succeeded",
4890+
terminalSummary: "completed",
4891+
});
4892+
});
4893+
const warnMock = context.logGateway.warn as ReturnType<typeof vi.fn>;
4894+
expect(
4895+
warnMock.mock.calls.some(([message]) => {
4896+
return (
4897+
typeof message === "string" &&
4898+
message.includes("failed to read ACP session metadata") &&
4899+
message.includes("falling back to cli task tracking")
4900+
);
4901+
}),
4902+
).toBe(true);
4903+
});
4904+
});
4905+
4906+
it("keeps CLI tracking for ACP-shaped turns that are not manual spawns", async () => {
4907+
await withTempDir({ prefix: "openclaw-gateway-acp-not-manual-spawn-" }, async (root) => {
4908+
useTestStateDir(root);
4909+
resetTaskRegistryForTests();
4910+
const childSessionKey = "agent:main:acp:child-not-spawn";
4911+
mockAcpChildSessionEntry(childSessionKey);
4912+
// Metadata is present but the turn lacks acpTurnSource, so the spawn
4913+
// control plane does not own this row; CLI tracking must stay on.
4914+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4915+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4916+
4917+
await invokeAgent(
4918+
{
4919+
message: "acp shaped non-spawn turn",
4920+
sessionKey: childSessionKey,
4921+
idempotencyKey: "acp-not-manual-spawn",
4922+
},
4923+
{ reqId: "acp-not-manual-spawn", client: backendGatewayClient() },
4924+
);
4925+
await waitForAgentCommandCall();
4926+
4927+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4928+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4929+
runtime: "cli",
4930+
runId: "acp-not-manual-spawn",
4931+
childSessionKey,
4932+
});
4933+
});
4934+
});
4935+
4936+
it("does not affect plugin-subagent tracking for confirmed ACP conditions", async () => {
4937+
await withTempDir({ prefix: "openclaw-gateway-acp-plugin-subagent-" }, async (root) => {
4938+
useTestStateDir(root);
4939+
resetTaskRegistryForTests();
4940+
resetSubagentRegistryForTests({ persist: false });
4941+
const childSessionKey = "agent:main:acp:plugin-child";
4942+
const runId = "acp-plugin-subagent-run";
4943+
mockAcpChildSessionEntry(childSessionKey);
4944+
mocks.readAcpSessionMeta.mockReturnValue(confirmedAcpMeta);
4945+
const createRunningTaskRunSpy = spyDetachedCreateRunningTaskRun();
4946+
4947+
const baseClient = requireValue(backendGatewayClient(), "expected backend client");
4948+
const pluginClient: AgentHandlerArgs["client"] = {
4949+
connect: baseClient.connect,
4950+
internal: {
4951+
...baseClient.internal,
4952+
agentRunTracking: "plugin_subagent",
4953+
pluginRuntimeOwnerId: "memory-core",
4954+
},
4955+
};
4956+
4957+
await invokeAgent(
4958+
{
4959+
message: "plugin subagent over acp child",
4960+
sessionKey: childSessionKey,
4961+
acpTurnSource: "manual_spawn",
4962+
idempotencyKey: runId,
4963+
},
4964+
{ reqId: runId, client: pluginClient },
4965+
);
4966+
await waitForAgentCommandCall();
4967+
4968+
// plugin_subagent precedence means the run is tracked through the
4969+
// subagent registry as a `subagent` row, never a duplicate `cli` row.
4970+
expect(createRunningTaskRunSpy).toHaveBeenCalledTimes(1);
4971+
expectRecordFields(mockCallArg(createRunningTaskRunSpy), {
4972+
runtime: "subagent",
4973+
runId,
4974+
});
4975+
expect(
4976+
listTaskRecords().some((task) => task.runId === runId && task.runtime === "cli"),
4977+
).toBe(false);
4978+
await waitForAssertion(() => {
4979+
expectRecordFields(getSubagentRunByChildSessionKey(childSessionKey), {
4980+
runId,
4981+
childSessionKey,
4982+
label: "plugin:memory-core",
4983+
});
4984+
});
4985+
});
4986+
});
4987+
});
4988+
46864989
it("logs a swallowed finalize error without blocking the background run", async () => {
46874990
await withTempDir({ prefix: "openclaw-gateway-agent-finalize-throw-" }, async (root) => {
46884991
useTestStateDir(root);
@@ -5669,6 +5972,7 @@ describe("gateway agent handler chat.abort integration", () => {
56695972
mocks.getLatestSubagentRunByChildSessionKey.mockReset();
56705973
mocks.replaceSubagentRunAfterSteer.mockReset();
56715974
mocks.resolveExplicitAgentSessionKey.mockReset().mockReturnValue(undefined);
5975+
mocks.readAcpSessionMeta.mockReset().mockReturnValue(undefined);
56725976
mocks.listAgentIds.mockReset().mockReturnValue(["main"]);
56735977
mocks.getChannelPlugin.mockReset();
56745978
mocks.sendDurableMessageBatch.mockReset();

0 commit comments

Comments
 (0)