Skip to content

Commit 5cb9673

Browse files
committed
Handle Grok user question cancellation explicitly
1 parent aa666fb commit 5cb9673

4 files changed

Lines changed: 309 additions & 36 deletions

File tree

apps/server/scripts/acp-mock-agent.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,7 @@ const program = Effect.gen(function* () {
566566
questions: [
567567
{
568568
question: "Which scope should Grok use?",
569+
multiSelect: null,
569570
options: [
570571
{ label: "Workspace", description: "Use the current workspace" },
571572
{ label: "Session", description: "Only use this session" },

apps/server/src/provider/Layers/GrokAdapter.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import {
5959
} from "../acp/GrokAcpSupport.ts";
6060
import {
6161
extractXAiAskUserQuestions,
62+
makeXAiAskUserQuestionCancelledResponse,
6263
makeXAiAskUserQuestionResponse,
6364
XAiAskUserQuestionRequest,
6465
} from "../acp/XAiAcpExtension.ts";
@@ -86,8 +87,12 @@ interface PendingApproval {
8687
readonly decision: Deferred.Deferred<ProviderApprovalDecision>;
8788
}
8889

90+
type PendingUserInputResolution =
91+
| { readonly _tag: "answered"; readonly answers: ProviderUserInputAnswers }
92+
| { readonly _tag: "cancelled" };
93+
8994
interface PendingUserInput {
90-
readonly answers: Deferred.Deferred<ProviderUserInputAnswers>;
95+
readonly resolution: Deferred.Deferred<PendingUserInputResolution>;
9196
}
9297

9398
interface GrokSessionContext {
@@ -116,12 +121,12 @@ function settlePendingApprovalsAsCancelled(
116121
);
117122
}
118123

119-
function settlePendingUserInputsAsEmptyAnswers(
124+
function settlePendingUserInputsAsCancelled(
120125
pendingUserInputs: ReadonlyMap<ApprovalRequestId, PendingUserInput>,
121126
): Effect.Effect<void> {
122127
return Effect.forEach(
123128
Array.from(pendingUserInputs.values()),
124-
(pending) => Deferred.succeed(pending.answers, {}).pipe(Effect.ignore),
129+
(pending) => Deferred.succeed(pending.resolution, { _tag: "cancelled" }).pipe(Effect.ignore),
125130
{ discard: true },
126131
);
127132
}
@@ -308,7 +313,7 @@ export function makeGrokAdapter(grokSettings: GrokSettings, options?: GrokAdapte
308313
if (ctx.stopped) return;
309314
ctx.stopped = true;
310315
yield* settlePendingApprovalsAsCancelled(ctx.pendingApprovals);
311-
yield* settlePendingUserInputsAsEmptyAnswers(ctx.pendingUserInputs);
316+
yield* settlePendingUserInputsAsCancelled(ctx.pendingUserInputs);
312317
if (ctx.notificationFiber) {
313318
yield* Fiber.interrupt(ctx.notificationFiber);
314319
}
@@ -395,8 +400,8 @@ export function makeGrokAdapter(grokSettings: GrokSettings, options?: GrokAdapte
395400
yield* logNative(input.threadId, method, params);
396401
const requestId = ApprovalRequestId.make(yield* randomUUIDv4);
397402
const runtimeRequestId = RuntimeRequestId.make(requestId);
398-
const answers = yield* Deferred.make<ProviderUserInputAnswers>();
399-
pendingUserInputs.set(requestId, { answers });
403+
const resolution = yield* Deferred.make<PendingUserInputResolution>();
404+
pendingUserInputs.set(requestId, { resolution });
400405
yield* offerRuntimeEvent({
401406
type: "user-input.requested",
402407
...(yield* makeEventStamp()),
@@ -411,23 +416,29 @@ export function makeGrokAdapter(grokSettings: GrokSettings, options?: GrokAdapte
411416
payload: params,
412417
},
413418
});
414-
const resolved = yield* Deferred.await(answers);
419+
const resolved = yield* Deferred.await(resolution);
415420
pendingUserInputs.delete(requestId);
421+
const resolvedAnswers = resolved._tag === "answered" ? resolved.answers : {};
416422
yield* offerRuntimeEvent({
417423
type: "user-input.resolved",
418424
...(yield* makeEventStamp()),
419425
provider: PROVIDER,
420426
threadId: input.threadId,
421427
turnId: sessions.get(input.threadId)?.activeTurnId,
422428
requestId: runtimeRequestId,
423-
payload: { answers: resolved },
429+
payload: { answers: resolvedAnswers },
424430
raw: {
425431
source: "acp.grok.extension",
426432
method,
427433
payload: params,
428434
},
429435
});
430-
return makeXAiAskUserQuestionResponse(resolved);
436+
switch (resolved._tag) {
437+
case "answered":
438+
return makeXAiAskUserQuestionResponse(params, resolved.answers);
439+
case "cancelled":
440+
return makeXAiAskUserQuestionCancelledResponse();
441+
}
431442
}),
432443
),
433444
),
@@ -804,7 +815,7 @@ export function makeGrokAdapter(grokSettings: GrokSettings, options?: GrokAdapte
804815
Effect.gen(function* () {
805816
const ctx = yield* requireSession(threadId);
806817
yield* settlePendingApprovalsAsCancelled(ctx.pendingApprovals);
807-
yield* settlePendingUserInputsAsEmptyAnswers(ctx.pendingUserInputs);
818+
yield* settlePendingUserInputsAsCancelled(ctx.pendingUserInputs);
808819
yield* Effect.ignore(
809820
ctx.acp.cancel.pipe(
810821
Effect.mapError((error) =>
@@ -847,7 +858,7 @@ export function makeGrokAdapter(grokSettings: GrokSettings, options?: GrokAdapte
847858
detail: `Unknown pending user-input request: ${requestId}`,
848859
});
849860
}
850-
yield* Deferred.succeed(pending.answers, answers);
861+
yield* Deferred.succeed(pending.resolution, { _tag: "answered", answers });
851862
});
852863

853864
const readThread: GrokAdapterShape["readThread"] = (threadId) =>

apps/server/src/provider/acp/XAiAcpExtension.test.ts

Lines changed: 181 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
1-
import { describe, expect, it } from "vitest";
1+
import { describe, expect, it } from "vite-plus/test";
22
import * as Schema from "effect/Schema";
33

4-
import { extractXAiAskUserQuestions, XAiAskUserQuestionRequest } from "./XAiAcpExtension.ts";
4+
import {
5+
extractXAiAskUserQuestions,
6+
makeXAiAskUserQuestionCancelledResponse,
7+
makeXAiAskUserQuestionResponse,
8+
XAiAskUserQuestionRequest,
9+
} from "./XAiAcpExtension.ts";
510

611
const decodeXAiAskUserQuestionRequest = Schema.decodeUnknownSync(XAiAskUserQuestionRequest);
712

@@ -39,7 +44,7 @@ describe("XAiAcpExtension", () => {
3944

4045
it("extracts questions from wrapped _x.ai extension payloads", () => {
4146
const payload = {
42-
method: "x.ai/ask_user_question",
47+
method: "_x.ai/ask_user_question",
4348
params: {
4449
sessionId: "session-1",
4550
toolCallId: "tool-call-1",
@@ -69,4 +74,177 @@ describe("XAiAcpExtension", () => {
6974
},
7075
]);
7176
});
77+
78+
it("treats nullable multiSelect from Grok as single-select", () => {
79+
const questions = extractXAiAskUserQuestions({
80+
sessionId: "session-1",
81+
toolCallId: "tool-call-1",
82+
mode: "default",
83+
questions: [
84+
{
85+
question: "Which label should Grok use?",
86+
multiSelect: null,
87+
options: [
88+
{ label: "Alpha", description: "Use the Alpha label" },
89+
{ label: "Beta", description: "Use the Beta label" },
90+
{ label: "Other", description: "Use the Other label" },
91+
],
92+
},
93+
],
94+
});
95+
96+
expect(questions).toEqual([
97+
{
98+
id: "Which label should Grok use?",
99+
header: "Question",
100+
question: "Which label should Grok use?",
101+
multiSelect: false,
102+
options: [
103+
{ label: "Alpha", description: "Use the Alpha label" },
104+
{ label: "Beta", description: "Use the Beta label" },
105+
{ label: "Other", description: "Use the Other label" },
106+
],
107+
},
108+
]);
109+
});
110+
111+
it("maps UI question ids back to xAI question text in accepted responses", () => {
112+
const response = makeXAiAskUserQuestionResponse(
113+
{
114+
sessionId: "session-1",
115+
toolCallId: "tool-call-1",
116+
mode: "default",
117+
questions: [
118+
{
119+
id: "scope",
120+
question: "Which scope should Grok use?",
121+
options: [
122+
{ label: "workspace", description: "Use the current workspace" },
123+
{ label: "session", description: "Only use this session" },
124+
],
125+
},
126+
],
127+
},
128+
{ scope: "workspace" },
129+
);
130+
131+
expect(response).toEqual({
132+
outcome: "accepted",
133+
answers: {
134+
"Which scope should Grok use?": ["workspace"],
135+
},
136+
});
137+
});
138+
139+
it("orders accepted answers by the original xAI question order", () => {
140+
const response = makeXAiAskUserQuestionResponse(
141+
{
142+
sessionId: "session-1",
143+
toolCallId: "tool-call-1",
144+
mode: "default",
145+
questions: [
146+
{
147+
id: "first",
148+
question: "First question?",
149+
options: [{ label: "A", description: "A" }],
150+
},
151+
{
152+
id: "second",
153+
question: "Second question?",
154+
options: [{ label: "B", description: "B" }],
155+
},
156+
],
157+
},
158+
{
159+
second: "B",
160+
first: "A",
161+
},
162+
);
163+
164+
expect(Object.keys(response.answers)).toEqual(["First question?", "Second question?"]);
165+
expect(response).toMatchObject({
166+
outcome: "accepted",
167+
answers: {
168+
"First question?": ["A"],
169+
"Second question?": ["B"],
170+
},
171+
});
172+
});
173+
174+
it("encodes typed custom answers as xAI Other annotations", () => {
175+
const response = makeXAiAskUserQuestionResponse(
176+
{
177+
method: "x.ai/ask_user_question",
178+
params: {
179+
sessionId: "session-1",
180+
toolCallId: "tool-call-1",
181+
mode: "default",
182+
questions: [
183+
{
184+
question: "Which ice cream flavor?",
185+
options: [
186+
{ label: "vanilla", description: "Vanilla flavor" },
187+
{ label: "chocolate", description: "Chocolate flavor" },
188+
],
189+
},
190+
],
191+
},
192+
},
193+
{ "Which ice cream flavor?": "pistachio" },
194+
);
195+
196+
expect(response).toEqual({
197+
outcome: "accepted",
198+
answers: {
199+
"Which ice cream flavor?": ["Other"],
200+
},
201+
annotations: {
202+
"Which ice cream flavor?": {
203+
notes: "pistachio",
204+
},
205+
},
206+
});
207+
});
208+
209+
it("encodes interrupted dialogs as xAI cancelled responses", () => {
210+
expect(makeXAiAskUserQuestionCancelledResponse()).toEqual({
211+
outcome: "cancelled",
212+
});
213+
});
214+
215+
it("does not echo preview annotations for multi-select answers", () => {
216+
const response = makeXAiAskUserQuestionResponse(
217+
{
218+
sessionId: "session-1",
219+
toolCallId: "tool-call-1",
220+
mode: "default",
221+
questions: [
222+
{
223+
question: "Which files should Grok touch?",
224+
multiSelect: true,
225+
options: [
226+
{
227+
label: "Tests",
228+
description: "Update tests",
229+
preview: "test preview",
230+
},
231+
{
232+
label: "Docs",
233+
description: "Update docs",
234+
preview: "docs preview",
235+
},
236+
],
237+
},
238+
],
239+
},
240+
{ "Which files should Grok touch?": ["Tests", "Docs"] },
241+
);
242+
243+
expect(response).toEqual({
244+
outcome: "accepted",
245+
answers: {
246+
"Which files should Grok touch?": ["Tests", "Docs"],
247+
},
248+
});
249+
});
72250
});

0 commit comments

Comments
 (0)