Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,15 @@ export interface AcpAdapterV2ExtensionContext {
/**
* Session-scoped background-task lifecycle reported via extension
* notifications (e.g. Grok `x.ai/task_backgrounded`; older builds use the
* underscore alias). Mutations for non-root sessions are ignored.
* underscore alias). Mutations for non-root sessions are ignored. A terminal
* mutation also finishes the tool that registered the task in a settled turn
* still held open for it, with `output` as its final text when given.
*/
readonly applyBackgroundTaskMutation: (mutation: {
readonly sessionId: string;
readonly taskId: string;
readonly status: "running" | "completed" | "failed";
readonly output?: string;
}) => Effect.Effect<void>;
readonly requestUserInput: (
input: AcpAdapterV2UserInputRequest,
Expand Down Expand Up @@ -3578,6 +3581,34 @@ export function makeAcpAdapterV2(options: AcpAdapterV2Options): ProviderAdapterV
}
});

// A structured task end (Grok `task_completed`) is authoritative for the
// tool that registered the task: its own updates only ever say running.
// Finish the row while deferred finalize holds a settled root turn open
// for it, so the turn can settle. While the prompt is still open the
// agent reports the end itself (TaskOutput hydration), and an unreported
// end must keep its post-finalize continuation offer.
const finishRegisteredBackgroundTool = Effect.fnUntraced(function* (mutation: {
readonly taskId: string;
readonly status: "completed" | "failed";
readonly output?: string;
}) {
const context = yield* Ref.get(activeTurn);
if (context === null || context.finalized || !context.promptSettled) return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Adapters/AcpAdapterV2.ts:3596

When task_completed arrives before promptSettled is set, the registered tool row remains inProgress, so deferred finalization stays blocked and the run never completes. finishRegisteredBackgroundTool returns on !context.promptSettled, while applyLateBackgroundMutation has already cleared the running-task set; allow this terminal mutation to finish the row before the completion callback settles the prompt.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts around line 3596:

When `task_completed` arrives before `promptSettled` is set, the registered tool row remains `inProgress`, so deferred finalization stays blocked and the run never completes. `finishRegisteredBackgroundTool` returns on `!context.promptSettled`, while `applyLateBackgroundMutation` has already cleared the running-task set; allow this terminal mutation to finish the row before the completion callback settles the prompt.

const toolCallId = context.toolCallIdsByBackgroundTaskId.get(mutation.taskId);
const tool = toolCallId === undefined ? undefined : context.tools.get(toolCallId);
if (tool === undefined) return;
const status = toolStatus(tool.status);
if (status !== "pending" && status !== "running") return;
context.awaitingBackgroundHydration.delete(mutation.taskId);
const finished = { ...tool, status: mutation.status };
yield* emitTool(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes how a settled, deferred turn finishes its registered monitor tool, but the tests do not assert that a task_completed mutation terminalizes that row with the snapshot output and lets the turn settle without a TaskOutput update. Could you add a focused adapter regression test for that path (including the failed exit-code case)?

Posted via Macroscope — Effect Service Conventions

context,
mutation.output === undefined ? finished : setToolOutputText(finished, mutation.output),
mutation.status,
);
yield* rearmDeferredFinalize(context);
});

const bufferPostSettleWake = Effect.fnUntraced(function* (
notification: EffectAcpSchema.SessionNotification,
) {
Expand Down Expand Up @@ -5569,6 +5600,13 @@ export function makeAcpAdapterV2(options: AcpAdapterV2Options): ProviderAdapterV
// its child session must not gate root wake machinery.
if ((yield* Ref.get(activeSessionId)) !== mutation.sessionId) return;
yield* applyLateBackgroundMutation(mutation.sessionId, mutation);
if (mutation.status !== "running") {
yield* finishRegisteredBackgroundTool({
taskId: mutation.taskId,
status: mutation.status,
...(mutation.output === undefined ? {} : { output: mutation.output }),
});
}
}),
).pipe(Effect.asVoid),
});
Expand Down
110 changes: 87 additions & 23 deletions apps/server/src/provider/acp/XAiAcpExtension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ import {
XAiAskUserQuestionRequest,
} from "./XAiAcpExtension.ts";
import * as AcpSessionRuntime from "./AcpSessionRuntime.ts";
import {
type AcpToolCallState,
mergeToolCallState,
parseSessionUpdateEvent,
} from "./AcpRuntimeModel.ts";

const decodeXAiAskUserQuestionRequest = Schema.decodeUnknownSync(XAiAskUserQuestionRequest);

Expand Down Expand Up @@ -80,6 +85,16 @@ describe("xAiPromptCompleteFromSessionUpdate", () => {
});
});

// Runs a recorded Grok tool_call_update through the adapter's parse path.
function parseRecordedToolCallUpdate(update: Record<string, unknown>): AcpToolCallState {
const [event] = parseSessionUpdateEvent({
sessionId: "01a0d660-c1bb-7842-91a3-02d91dc8b0d2",
update,
} as Parameters<typeof parseSessionUpdateEvent>[0]).events;
if (event?._tag !== "ToolCallUpdated") throw new Error("expected a tool call update");
return event.toolCall;
}

describe("XAiAcpExtension", () => {
it("recognizes Grok Task starts as native subagents", () => {
expect(
Expand Down Expand Up @@ -404,27 +419,77 @@ describe("XAiAcpExtension", () => {
expect(extractXAiMonitorTaskId(normalized)).toBe("019f44a5-87d1-7640-8e35-6a4667ffc873");
});

it("completes Monitor variant tools from structured Bash exit codes", () => {
const toolCall = {
toolCallId: "call-mon-2",
title: "Tool",
status: "inProgress" as const,
data: {
rawInput: {
variant: "Monitor",
command: "echo MON_DONE",
description: "Stream mon lines",
},
rawOutput: {
type: "Bash",
output: Array.from(new TextEncoder().encode("mon_line_1\nMON_DONE\n")),
output_for_prompt: "mon_line_1\nMON_DONE\n",
it("keeps Monitor ticks running while Grok reports them in progress", () => {
// Recorded from Grok 1.0.41: every tick is an in_progress update whose
// Bash-shaped rawOutput already carries exit_code 0. The monitor ends only
// through `_x.ai/task_completed`, never through a completed status.
const command = "for i in 1 2 3; do sleep 8; echo tick $i; done";
const started = parseRecordedToolCallUpdate({
sessionUpdate: "tool_call_update",
toolCallId: "call-6b025a4e-621c-4526-a579-ba86ada2669c-0",
kind: "other",
title: "Start monitor: Watch three ticks eight seconds apart",
locations: [],
rawInput: {
variant: "Monitor",
command,
description: "Watch three ticks eight seconds apart",
timeout_ms: 36000000,
persistent: false,
},
});
const tick = parseRecordedToolCallUpdate({
sessionUpdate: "tool_call_update",
toolCallId: "call-6b025a4e-621c-4526-a579-ba86ada2669c-0",
status: "in_progress",
content: [{ type: "content", content: { type: "text", text: "tick 1\n" } }],
rawOutput: {
type: "Bash",
output: [116, 105, 99, 107, 32, 49, 10],
output_for_prompt: "tick 1\n",
exit_code: 0,
command,
truncated: false,
signal: null,
timed_out: false,
description: null,
current_dir: "<workspace>",
output_file: "",
total_bytes: 7,
},
});
const toolCall = normalizeXAiAcpToolCallState(mergeToolCallState(started, tick));
expect(isXAiMonitorTool(toolCall)).toBe(true);
expect(toolCall.status).toBe("inProgress");
});

it("finishes monitors from the recorded task_completed snapshot", () => {
const completed = {
sessionId: "01a0d660-c1bb-7842-91a3-02d91dc8b0d2",
update: {
sessionUpdate: "task_completed",
task_snapshot: {
task_id: "01a0d660-f98d-7ef3-a291-97af1c2b6455",
output: "tick 1\ntick 2\ntick 3\n",
exit_code: 0,
},
},
};
expect(isXAiMonitorTool(toolCall)).toBe(true);
expect(normalizeXAiAcpToolCallState(toolCall).status).toBe("completed");
expect(xAiBackgroundTaskLifecycleMutation(completed, "completed")).toEqual({
sessionId: "01a0d660-c1bb-7842-91a3-02d91dc8b0d2",
taskId: "01a0d660-f98d-7ef3-a291-97af1c2b6455",
status: "completed",
output: "tick 1\ntick 2\ntick 3\n",
});
expect(
xAiBackgroundTaskLifecycleMutation(
{
...completed,
update: { ...completed.update, task_snapshot: { task_id: "t", exit_code: 2 } },
},
"completed",
)?.status,
).toBe("failed");
});

it("detects Monitor start ACKs from structured rawOutput when title is generic", () => {
Expand All @@ -450,14 +515,13 @@ describe("XAiAcpExtension", () => {
expect(extractXAiMonitorTaskId(toolCall)).toBe("019f44a5-87d1-7640-8e35-6a4667ffc873");
});

it("completes wake re-reports of finished commands despite generic titles", () => {
// Post-settle wake replay: the finished monitor is re-reported with empty
// rawInput and a generic title, so monitor detection cannot match; the
// structured Bash result with exit_code must still terminalize it.
it("completes status-less Bash results despite generic titles", () => {
// A Bash result that carries no status (empty rawInput, generic title) is
// terminalized from its exit_code. An explicit in_progress status wins:
// that is how Grok streams a still-running monitor.
const toolCall = {
toolCallId: "call-wake-1",
title: "Tool",
status: "inProgress" as const,
data: {
rawInput: {},
rawOutput: {
Expand Down Expand Up @@ -818,7 +882,7 @@ describe("XAiAcpExtension", () => {
const mutations: Array<{
readonly sessionId: string;
readonly taskId: string;
readonly status: "running" | "completed";
readonly status: "running" | "completed" | "failed";
}> = [];
const runtime = {
handleExtNotification: (
Expand Down
28 changes: 21 additions & 7 deletions apps/server/src/provider/acp/XAiAcpExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,8 @@ const XAiTaskLifecycleNotification = Schema.Struct({
task_snapshot: Schema.optional(
Schema.Struct({
task_id: Schema.optional(Schema.String),
output: Schema.optional(Schema.String),
exit_code: Schema.optional(Schema.NullOr(Schema.Number)),
}),
),
}),
Expand All @@ -391,7 +393,9 @@ type XAiTaskLifecycleNotification = typeof XAiTaskLifecycleNotification.Type;
export interface XAiBackgroundTaskLifecycleMutation {
readonly sessionId: string;
readonly taskId: string;
readonly status: "running" | "completed";
readonly status: "running" | "completed" | "failed";
/** Final output from `task_completed.task_snapshot`, when Grok sent one. */
readonly output?: string;
}

export function xAiBackgroundTaskLifecycleMutation(
Expand All @@ -404,7 +408,15 @@ export function xAiBackgroundTaskLifecycleMutation(
nonEmptyString(update.task_id) ??
nonEmptyString(update.tool_call_id);
if (taskId === undefined) return null;
return { sessionId: notification.sessionId, taskId, status };
const exitCode = update.task_snapshot?.exit_code;
const output = update.task_snapshot?.output;
return {
sessionId: notification.sessionId,
taskId,
status:
status === "completed" && typeof exitCode === "number" && exitCode !== 0 ? "failed" : status,
...(output === undefined ? {} : { output }),
};
}

/**
Expand Down Expand Up @@ -480,10 +492,11 @@ export function resolveXAiAcpToolTitle(toolCall: AcpToolCallState): string | und
* - replace generic titles ("Tool") with description / variant labels
* - structured Monitor start ACK stays running
* - text start ACK stays running
* - structured Bash results with exit_code are terminal (any tool: post-settle
* wake re-reports of a finished monitor arrive with empty rawInput and a
* generic title, so monitor detection cannot match; without this they replay
* as running and the timeline row spins forever)
* - structured Bash results with exit_code are terminal, unless Grok itself
* still reports the tool running. Grok 1.0.41 streams every monitor tick as
* `status: "in_progress"` with a Bash-shaped rawOutput whose exit_code is
* already 0, and never sends a completed status for the monitor; its end
* arrives as `_x.ai/task_completed`.
*/
export function normalizeXAiAcpToolCallState(toolCall: AcpToolCallState): AcpToolCallState {
const resolvedTitle = resolveXAiAcpToolTitle(toolCall);
Expand All @@ -494,7 +507,8 @@ export function normalizeXAiAcpToolCallState(toolCall: AcpToolCallState): AcpToo

const rawOutput = unknownRecord(withTitle.data.rawOutput);
const outputType = nonEmptyString(rawOutput?.type)?.toLowerCase();
if (outputType === "bash") {
const reportedRunning = withTitle.status === "inProgress" || withTitle.status === "pending";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High acp/XAiAcpExtension.ts:510

A status-less terminal Bash re-report after an earlier inProgress update remains inProgress, so its exit_code is never applied and deferred finalization leaves the turn hanging. mergeToolCallState preserves the previous status via next.status ?? previous.status, causing reportedRunning to misclassify the current update; preserve whether status was present in the incoming update and terminalize status-less Bash results from their exit_code.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/acp/XAiAcpExtension.ts around line 510:

A status-less terminal Bash re-report after an earlier `inProgress` update remains `inProgress`, so its `exit_code` is never applied and deferred finalization leaves the turn hanging. `mergeToolCallState` preserves the previous status via `next.status ?? previous.status`, causing `reportedRunning` to misclassify the current update; preserve whether `status` was present in the incoming update and terminalize status-less Bash results from their `exit_code`.

if (outputType === "bash" && !reportedRunning) {
// Same key set as the adapter's command projection (commandExitCode).
const exitCode = ["exit_code", "exitCode", "code"]
.map((key) => rawOutput?.[key])
Expand Down
Loading