From ab243f18d01b58ce9a3b6513b8c5b9177188ea21 Mon Sep 17 00:00:00 2001 From: Fuksss Date: Tue, 20 Oct 2026 23:43:20 +0200 Subject: [PATCH] fix(grok): close background commands from KillTask results Handle KillTask results when no task completion notification arrives. Keep legacy kill handling and earlier terminal command results. Refs #15888. --- apps/server/scripts/acp-mock-agent.ts | 76 +++++++++++++ .../Adapters/AcpAdapterV2.test.ts | 100 ++++++++++++++++++ packages/provider-acp/src/server/adapter.ts | 18 ++++ .../src/server/xaiAcpExtension.test.ts | 95 +++++++++++++++++ .../src/server/xaiAcpExtension.ts | 9 +- 5 files changed, 295 insertions(+), 3 deletions(-) diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index c2d29e98047e..a406484be833 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -46,6 +46,7 @@ const emitXAiAskUserQuestionThenHang = process.env.T3_ACP_EMIT_XAI_ASK_USER_QUESTION_THEN_HANG === "1"; const emitContentThenHang = process.env.T3_ACP_EMIT_CONTENT_THEN_HANG === "1"; const emitPlanThenHang = process.env.T3_ACP_EMIT_PLAN_THEN_HANG === "1"; +const grokKillTaskShape = process.env.T3_ACP_GROK_KILL_TASK_SHAPE; const emitActiveToolThenHang = process.env.T3_ACP_EMIT_ACTIVE_TOOL_THEN_HANG === "1"; const emitGrokMonitorPostTurnPoll = process.env.T3_ACP_EMIT_GROK_MONITOR_POST_TURN_POLL === "1"; const emitGrokBackgroundTaskStarted = process.env.T3_ACP_EMIT_GROK_BACKGROUND_TASK_STARTED === "1"; @@ -461,6 +462,71 @@ function modelState(): AcpCompat.SessionModelState { }; } +function emitGrokKillTaskScenario( + sessionId: string, + promptId: string | undefined, + shape: string, +): void { + const taskId = "call-fb9d0000-0000-0000-0000-000000000026-29"; + const outcome = + shape === "already_exited" || shape === "known_failure" ? "already_exited" : "killed"; + writeJsonRpcNotification("session/update", { + sessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "kill-target-command", + title: "run_terminal_command", + kind: "execute", + status: "in_progress", + rawInput: { command: "sleep 40" }, + }, + }); + writeJsonRpcNotification("session/update", { + sessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "kill-target-command", + status: "completed", + rawOutput: { + type: "BackgroundTaskStarted", + task_id: taskId, + task_type: "bash", + command: "sleep 40", + }, + }, + }); + if (shape === "known_failure") { + writeJsonRpcNotification("_x.ai/task_completed", { + sessionId, + update: { + sessionUpdate: "task_completed", + task_snapshot: { task_id: taskId, kind: "bash", exit_code: 7, output: "known failure\n" }, + }, + }); + } + writeJsonRpcNotification("session/update", { + sessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "kill-command", + title: `kill ${taskId} (${outcome})`, + kind: "other", + status: "completed", + rawInput: { + task_id: taskId, + ...(shape === "output_only" ? {} : { variant: "KillTask" }), + }, + ...(shape === "variant_only" ? {} : { rawOutput: { type: "KillTask", outcome } }), + }, + }); + writeJsonRpcNotification("_x.ai/session/prompt_complete", { + sessionId, + promptId, + stopReason: "end_turn", + agentResult: null, + }); +} + const program = Effect.gen(function* () { const agent = yield* EffectAcpAgent.AcpAgent; const resumeRelease = yield* Deferred.make(); @@ -1491,6 +1557,16 @@ const program = Effect.gen(function* () { return yield* Effect.never; } + if (grokKillTaskShape !== undefined) { + emitGrokKillTaskScenario( + requestedSessionId, + promptIdFromRequestMeta(request), + grokKillTaskShape, + ); + // Keep the prompt RPC and peer alive after the kill result. + return yield* Effect.never; + } + if (emitPlanThenHang) { yield* agent.client.sessionUpdate({ sessionId: requestedSessionId, diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 82cd3d7a3a12..ba5d88bd107a 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -4040,6 +4040,106 @@ describe("AcpAdapterV2", () => { }).pipe(Effect.provide(layerTest), Effect.scoped), ); + // Oracle: #15888 KillTask reports. Command completion must release the turn even + // when the CLI keeps the prompt RPC and process alive. Kill calls stay visible. + it.live.each([ + { shape: "output_only", expectedCommand: { type: "command_execution", status: "completed" } }, + { shape: "variant_only", expectedCommand: { type: "command_execution", status: "completed" } }, + { shape: "killed", expectedCommand: { type: "command_execution", status: "completed" } }, + { + shape: "already_exited", + expectedCommand: { type: "command_execution", status: "completed" }, + }, + { + shape: "known_failure", + expectedCommand: { type: "command_execution", status: "failed", output: "known failure\n" }, + }, + ] as const)( + "completes a Grok KillTask command ($shape) and preserves any known result", + ({ shape, expectedCommand }) => + Effect.gen(function* () { + const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const path = yield* Path.Path; + const mockAgentPath = yield* path.fromFileUrl( + new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), + ); + const instanceId = ProviderInstanceId.make("grok-kill-completion"); + const adapter = yield* makeGrokAdapterV2({ + instanceId, + settings: DEFAULT_GROK_SETTINGS, + environment: {}, + hostPlatform: yield* HostProcess.Platform, + selfInvocation: yield* resolveSelfInvocation(), + makeRuntime: (input) => + makeMockRuntime({ + childProcessSpawner, + mockAgentPath, + environment: { T3_ACP_GROK_KILL_TASK_SHAPE: shape }, + })(input).pipe(Effect.flatMap(makeXAiPromptCompletionRuntime)), + }); + const threadId = ThreadId.make("grok-kill-completion"); + const modelSelection = { instanceId, model: "default" } as const; + const runtimePolicy = ProviderAdapter.ProviderAdapterV2RuntimePolicy.make({ + runtimeMode: "full-access", + interactionMode: "default", + cwd: process.cwd(), + }); + const runtime = yield* adapter.openSession({ + threadId, + providerSessionId: ProviderSessionId.make("grok-kill-completion"), + modelSelection, + runtimePolicy, + }); + const providerThread = yield* runtime.ensureThread({ + threadId, + modelSelection, + runtimePolicy, + }); + const events = yield* Queue.unbounded(); + yield* runtime.events.pipe( + Stream.runForEach((event) => Queue.offer(events, event)), + Effect.forkScoped, + ); + yield* runtime.startTurn( + makeTurnInput({ + threadId, + providerThread, + instanceId, + runtimePolicy, + now: yield* DateTime.now, + }), + ); + const observed = yield* Stream.fromQueue(events).pipe( + Stream.takeUntil((event) => event.type === "turn.terminal"), + Stream.runCollect, + Effect.timeout("15 seconds"), + ); + const allEvents = [...observed]; + let next = yield* Queue.poll(events); + while (Option.isSome(next)) { + allEvents.push(next.value); + next = yield* Queue.poll(events); + } + const terminals = allEvents.filter((event) => event.type === "turn.terminal"); + assert.deepEqual( + terminals.map((event) => event.status), + ["completed"], + ); + const items = allEvents.flatMap((event) => + event.type === "turn_item.updated" ? [event.turnItem] : [], + ); + assert.isFalse(items.some((item) => item.type === "subagent")); + const command = items.findLast( + (item) => item.nativeItemRef?.nativeId === "kill-target-command", + ); + assert.deepInclude(command, expectedCommand); + assert.isTrue( + items.some((item) => item.nativeItemRef?.nativeId === "kill-command"), + "KillTask must retain its normal tool frame", + ); + }).pipe(Effect.provide(layerTest), Effect.scoped), + ); + it.live("Grok reapplies an explicit return to the session's setup-time model", () => Effect.gen(function* () { const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; diff --git a/packages/provider-acp/src/server/adapter.ts b/packages/provider-acp/src/server/adapter.ts index 443d0906e190..1b76e7f4d477 100644 --- a/packages/provider-acp/src/server/adapter.ts +++ b/packages/provider-acp/src/server/adapter.ts @@ -1284,6 +1284,18 @@ interface AcpNativeChildSession { readonly pending: Array; } +// Keep BackgroundTaskStarted normalization from reopening a completed command. +function retainBackgroundCommandCompletion( + context: ActiveAcpTurn, + taskId: string, + status: "running" | "completed" | "failed", + output: string, +): void { + if (status === "running" || !context.backgroundCommands.has(taskId)) return; + const completion = context.backgroundCommands.get(taskId) ?? { status, output }; + context.backgroundCommands.set(taskId, completion); +} + function acpSubagentStatusIsTerminal(status: OrchestrationV2Subagent["status"]): boolean { return ( status === "completed" || @@ -3557,6 +3569,12 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( backgroundCompletion.appendOutput, ) : { ...target, status: nextStatus }; + retainBackgroundCommandCompletion( + context, + backgroundCompletion.taskId, + backgroundCompletion.status, + toolOutputText(hydrated), + ); yield* emitTool(context, hydrated); } } diff --git a/packages/provider-grok/src/server/xaiAcpExtension.test.ts b/packages/provider-grok/src/server/xaiAcpExtension.test.ts index c31d9438972c..0befc2ce01d2 100644 --- a/packages/provider-grok/src/server/xaiAcpExtension.test.ts +++ b/packages/provider-grok/src/server/xaiAcpExtension.test.ts @@ -795,6 +795,101 @@ describe("XAiAcpExtension", () => { ]); }); + // Oracle: #15888 reports KillTask replies with killed and already_exited outcomes. + it.each([ + { rawInput: { task_id: "call-bg-1" }, rawOutput: { type: "KillTask" } }, + { rawInput: { task_id: "call-bg-1" }, rawOutput: { type: "KillTask", outcome: "killed" } }, + { rawInput: { variant: "KillTask", task_id: "call-bg-1" } }, + { + rawInput: { variant: "KillTask", task_id: "call-bg-1" }, + rawOutput: { type: "KillTask", outcome: "already_exited" }, + }, + ])("closes a background command from structured KillTask fields: %j", (data) => { + expect( + extractXAiKilledBackgroundTasks({ + toolCallId: "call-kill", + title: "kill call-bg-1 (killed)", + status: "completed", + data, + }), + ).toEqual([{ taskId: "call-bg-1", status: "completed", appendOutput: "" }]); + expect( + extractXAiAcpSubagentUpdate({ + toolCallId: "call-kill", + title: "kill call-bg-1 (killed)", + status: "completed", + data, + }), + ).toBeUndefined(); + }); + + // Native child status still comes from its lifecycle/result path, not the kill RPC. + it.each(["Kill", "KillTask"])( + "keeps %s aimed at a native child out of child result parsing", + (variant) => { + expect( + extractXAiAcpSubagentUpdate({ + toolCallId: "call-kill-child", + title: "kill 019f44b8-8e98-7c80-a40e-df1e26a5f9e3 (killed)", + status: "completed", + data: { + rawInput: { variant, task_id: "019f44b8-8e98-7c80-a40e-df1e26a5f9e3" }, + rawOutput: { type: "KillTask", outcome: "killed" }, + }, + }), + ).toBeUndefined(); + }, + ); + + // Oracle: the base legacy-title path closes the target regardless of outcome. + it.each([{ outcome: "failed" }, { type: "KillTask", outcome: "failed" }])( + "closes a legacy-title KillTask target with a failed outcome: %j", + (rawOutput) => { + expect( + extractXAiKilledBackgroundTasks({ + toolCallId: "call-kill-legacy", + title: "kill_command_or_subagent", + status: "completed", + data: { + rawInput: { variant: "Kill", task_id: "call-bg-1" }, + rawOutput, + }, + }), + ).toEqual([{ taskId: "call-bg-1", status: "completed", appendOutput: "" }]); + }, + ); + // A successful RPC is not proof of a successful kill when its outcome says otherwise. + it("ignores a failed KillTask outcome", () => { + expect( + extractXAiKilledBackgroundTasks({ + toolCallId: "call-kill", + title: "kill call-bg-1 (failed)", + status: "completed", + data: { + rawInput: { variant: "KillTask", task_id: "call-bg-1" }, + rawOutput: { type: "KillTask", outcome: "failed" }, + }, + }), + ).toEqual([]); + }); + + it.each(["failed", "inProgress"] as const)( + "does not close the target of a %s KillTask call", + (status) => { + expect( + extractXAiKilledBackgroundTasks({ + toolCallId: "call-kill", + title: "kill call-bg-1 (killed)", + status, + data: { + rawInput: { variant: "KillTask", task_id: "call-bg-1" }, + rawOutput: { type: "KillTask", outcome: "killed" }, + }, + }), + ).toEqual([]); + }, + ); + it("tombstones confirmed kills and ignores failed kill calls", () => { expect( extractXAiKilledBackgroundTasks({ diff --git a/packages/provider-grok/src/server/xaiAcpExtension.ts b/packages/provider-grok/src/server/xaiAcpExtension.ts index 2ad11521abb3..572232f92467 100644 --- a/packages/provider-grok/src/server/xaiAcpExtension.ts +++ b/packages/provider-grok/src/server/xaiAcpExtension.ts @@ -355,8 +355,8 @@ export function extractXAiBackgroundTaskCompletion(toolCall: AcpToolCallState): /** * A finished kill_command_or_subagent call is the genuine end signal for every - * task id it names: the Grok CLI emits no `x.ai/task_completed` for a killed - * task. A failed kill is not proof the target stopped, so leave the existing + * task id it names, even without a separate `x.ai/task_completed` notification. + * A failed kill is not proof the target stopped, so leave the existing * task state unchanged until a genuine terminal signal arrives. */ export function extractXAiKilledBackgroundTasks(toolCall: AcpToolCallState): ReadonlyArray<{ @@ -366,9 +366,12 @@ export function extractXAiKilledBackgroundTasks(toolCall: AcpToolCallState): Rea }> { const title = titleKey(toolCall); const rawInput = unknownRecord(toolCall.data.rawInput); + const rawOutput = unknownRecord(toolCall.data.rawOutput); const variant = nonEmptyString(rawInput?.variant)?.toLowerCase(); - if (!title.includes("kill_command_or_subagent") && variant !== "kill") return []; + const legacy = title.includes("kill_command_or_subagent") || variant === "kill"; + if (!legacy && variant !== "killtask" && rawOutput?.type !== "KillTask") return []; if (toolCall.status !== "completed") return []; + if (!legacy && rawOutput?.outcome === "failed") return []; const taskIds: string[] = []; const push = (value: unknown) => { const id = nonEmptyString(value);