Skip to content

Commit f50fb3a

Browse files
authored
🤖 fix: keep ask_user_question waiting across restart (#1152)
1 parent cd7fb3d commit f50fb3a

File tree

12 files changed

+365
-22
lines changed

12 files changed

+365
-22
lines changed

src/browser/App.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,14 +416,21 @@ function AppInner() {
416416

417417
// Compute streaming models here (only when command palette opens)
418418
const allStates = workspaceStore.getAllStates();
419+
const selectedWorkspaceState = params.selectedWorkspace
420+
? (allStates.get(params.selectedWorkspace.workspaceId) ?? null)
421+
: null;
419422
const streamingModels = new Map<string, string>();
420423
for (const [workspaceId, state] of allStates) {
421424
if (state.canInterrupt && state.currentModel) {
422425
streamingModels.set(workspaceId, state.currentModel);
423426
}
424427
}
425428

426-
const factories = buildCoreSources({ ...params, streamingModels });
429+
const factories = buildCoreSources({
430+
...params,
431+
streamingModels,
432+
selectedWorkspaceState,
433+
});
427434
const actions: CommandAction[] = [];
428435
for (const factory of factories) {
429436
actions.push(...factory());

src/browser/components/tools/AskUserQuestionToolCall.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import assert from "@/common/utils/assert";
22

33
import { useMemo, useState } from "react";
44

5+
import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events";
56
import { useAPI } from "@/browser/contexts/API";
67
import { Checkbox } from "@/browser/components/ui/checkbox";
78
import { Input } from "@/browser/components/ui/input";
@@ -222,6 +223,7 @@ export function AskUserQuestionToolCall(props: {
222223
setSubmitError(null);
223224

224225
let answers: Record<string, string>;
226+
let workspaceId: string;
225227

226228
try {
227229
answers = {};
@@ -237,6 +239,7 @@ export function AskUserQuestionToolCall(props: {
237239

238240
assert(api, "API not connected");
239241
assert(props.workspaceId, "workspaceId is required");
242+
workspaceId = props.workspaceId;
240243
} catch (error) {
241244
const errorMessage = error instanceof Error ? error.message : String(error);
242245
setSubmitError(errorMessage);
@@ -246,14 +249,24 @@ export function AskUserQuestionToolCall(props: {
246249

247250
api.workspace
248251
.answerAskUserQuestion({
249-
workspaceId: props.workspaceId,
252+
workspaceId,
250253
toolCallId: props.toolCallId,
251254
answers,
252255
})
253256
.then((result) => {
254257
if (!result.success) {
255258
setSubmitError(result.error);
259+
return;
256260
}
261+
262+
// If the stream was interrupted (e.g. app restart) we need to explicitly
263+
// kick the resume manager so the assistant continues after answers.
264+
window.dispatchEvent(
265+
createCustomEvent(CUSTOM_EVENTS.RESUME_CHECK_REQUESTED, {
266+
workspaceId,
267+
isManual: true,
268+
})
269+
);
257270
})
258271
.catch((error) => {
259272
const errorMessage = error instanceof Error ? error.message : String(error);

src/browser/hooks/useAIViewKeybinds.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ export function useAIViewKeybinds({
6767
// Interrupt stream: Ctrl+C in vim mode, Esc in normal mode
6868
// Only intercept if actively compacting (otherwise allow browser default for copy in vim mode)
6969
if (matchesKeybind(e, interruptKeybind)) {
70+
// ask_user_question is a special waiting state: don't interrupt it with Esc/Ctrl+C.
71+
// Users can still respond via the questions UI, or type in chat to cancel.
72+
if (aggregator?.hasAwaitingUserQuestion()) {
73+
return;
74+
}
75+
7076
if (canInterrupt && aggregator && isCompactingStream(aggregator)) {
7177
// Ctrl+C during compaction: restore original state and enter edit mode
7278
// Stores cancellation marker in localStorage (persists across reloads)

src/browser/utils/commands/sources.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { CommandIds } from "@/browser/utils/commandIds";
99
import type { ProjectConfig } from "@/node/config";
1010
import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
1111
import type { BranchListResult } from "@/common/orpc/types";
12+
import type { WorkspaceState } from "@/browser/stores/WorkspaceStore";
1213
import type { RuntimeConfig } from "@/common/types/runtime";
1314

1415
export interface BuildSourcesParams {
@@ -17,6 +18,7 @@ export interface BuildSourcesParams {
1718
/** Map of workspace ID to workspace metadata (keyed by metadata.id, not path) */
1819
workspaceMetadata: Map<string, FrontendWorkspaceMetadata>;
1920
theme: ThemeMode;
21+
selectedWorkspaceState?: WorkspaceState | null;
2022
selectedWorkspace: {
2123
projectPath: string;
2224
projectName: string;
@@ -393,6 +395,9 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi
393395
title: "Interrupt Streaming",
394396
section: section.chat,
395397
run: async () => {
398+
if (p.selectedWorkspaceState?.awaitingUserQuestion) {
399+
return;
400+
}
396401
await p.api?.workspace.interruptStream({ workspaceId: id });
397402
},
398403
});

src/browser/utils/messages/StreamingMessageAggregator.status.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,55 @@ afterAll(() => {
4646
}
4747
});
4848

49+
describe("ask_user_question waiting state", () => {
50+
it("treats partial ask_user_question as executing (waiting) not interrupted", () => {
51+
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");
52+
53+
aggregator.loadHistoricalMessages([
54+
{
55+
id: "assistant-1",
56+
role: "assistant" as const,
57+
parts: [
58+
{
59+
type: "dynamic-tool" as const,
60+
toolCallId: "call-ask-1",
61+
toolName: "ask_user_question",
62+
state: "input-available" as const,
63+
input: {
64+
questions: [
65+
{
66+
header: "Approach",
67+
question: "Which approach should we take?",
68+
options: [
69+
{ label: "A", description: "Approach A" },
70+
{ label: "B", description: "Approach B" },
71+
],
72+
multiSelect: false,
73+
},
74+
],
75+
},
76+
},
77+
],
78+
metadata: {
79+
timestamp: 1000,
80+
historySequence: 1,
81+
partial: true,
82+
},
83+
},
84+
]);
85+
86+
const displayed = aggregator.getDisplayedMessages();
87+
const toolMsg = displayed.find((m) => m.type === "tool" && m.toolName === "ask_user_question");
88+
expect(toolMsg).toBeDefined();
89+
if (toolMsg?.type === "tool") {
90+
expect(toolMsg.status).toBe("executing");
91+
expect(toolMsg.isPartial).toBe(true);
92+
}
93+
94+
expect(aggregator.hasAwaitingUserQuestion()).toBe(true);
95+
});
96+
});
97+
4998
describe("StreamingMessageAggregator - Agent Status", () => {
5099
it("should start with undefined agent status", () => {
51100
const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z");

src/browser/utils/messages/StreamingMessageAggregator.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -299,18 +299,19 @@ export class StreamingMessageAggregator {
299299
* Used to show "Awaiting your input" instead of "streaming..." in the UI.
300300
*/
301301
hasAwaitingUserQuestion(): boolean {
302-
// Scan displayed messages for an ask_user_question tool in "executing" state
302+
// Only treat the workspace as "awaiting input" when the *latest* displayed
303+
// message is an executing ask_user_question tool.
304+
//
305+
// This avoids false positives from stale historical partials if the user
306+
// continued the chat after skipping/canceling the questions.
303307
const displayed = this.getDisplayedMessages();
304-
for (const msg of displayed) {
305-
if (
306-
msg.type === "tool" &&
307-
msg.toolName === "ask_user_question" &&
308-
msg.status === "executing"
309-
) {
310-
return true;
311-
}
308+
const last = displayed[displayed.length - 1];
309+
310+
if (last?.type !== "tool") {
311+
return false;
312312
}
313-
return false;
313+
314+
return last.toolName === "ask_user_question" && last.status === "executing";
314315
}
315316

316317
/**
@@ -1095,10 +1096,18 @@ export class StreamingMessageAggregator {
10951096
if (part.state === "output-available") {
10961097
// Check if result indicates failure (for tools that return { success: boolean })
10971098
status = hasFailureResult(part.output) ? "failed" : "completed";
1098-
} else if (part.state === "input-available" && message.metadata?.partial) {
1099-
status = "interrupted";
11001099
} else if (part.state === "input-available") {
1101-
status = "executing";
1100+
// Most unfinished tool calls in partial messages represent an interruption.
1101+
// ask_user_question is different: it's intentionally waiting on user input,
1102+
// so after restart we should keep it answerable ("executing") instead of
1103+
// showing retry/auto-resume UX.
1104+
if (part.toolName === "ask_user_question") {
1105+
status = "executing";
1106+
} else if (message.metadata?.partial) {
1107+
status = "interrupted";
1108+
} else {
1109+
status = "executing";
1110+
}
11021111
} else {
11031112
status = "pending";
11041113
}

src/browser/utils/messages/messageUtils.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,46 @@
11
import { describe, it, expect } from "@jest/globals";
2+
3+
import { shouldShowInterruptedBarrier } from "./messageUtils";
24
import { mergeConsecutiveStreamErrors, computeBashOutputGroupInfo } from "./messageUtils";
35
import type { DisplayedMessage } from "@/common/types/message";
46

7+
describe("shouldShowInterruptedBarrier", () => {
8+
it("returns false for executing ask_user_question", () => {
9+
const msg: DisplayedMessage = {
10+
type: "tool",
11+
id: "tool-1",
12+
historyId: "assistant-1",
13+
toolName: "ask_user_question",
14+
toolCallId: "call-1",
15+
args: { questions: [] },
16+
status: "executing",
17+
isPartial: true,
18+
historySequence: 2,
19+
streamSequence: 0,
20+
isLastPartOfMessage: true,
21+
};
22+
23+
expect(shouldShowInterruptedBarrier(msg)).toBe(false);
24+
});
25+
26+
it("returns true for interrupted tool (non ask_user_question)", () => {
27+
const msg: DisplayedMessage = {
28+
type: "tool",
29+
id: "tool-1",
30+
historyId: "assistant-1",
31+
toolName: "bash",
32+
toolCallId: "call-1",
33+
args: { script: "echo hi", timeout_secs: 1, display_name: "test" },
34+
status: "interrupted",
35+
isPartial: true,
36+
historySequence: 2,
37+
streamSequence: 0,
38+
isLastPartOfMessage: true,
39+
};
40+
41+
expect(shouldShowInterruptedBarrier(msg)).toBe(true);
42+
});
43+
});
544
describe("mergeConsecutiveStreamErrors", () => {
645
it("returns empty array for empty input", () => {
746
const result = mergeConsecutiveStreamErrors([]);

src/browser/utils/messages/messageUtils.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ export function shouldShowInterruptedBarrier(msg: DisplayedMessage): boolean {
5454
)
5555
return false;
5656

57+
// ask_user_question is intentionally a "waiting for input" state. Even if the
58+
// underlying message is a persisted partial (e.g. after app restart), we keep
59+
// it answerable instead of showing "Interrupted".
60+
if (msg.type === "tool" && msg.toolName === "ask_user_question" && msg.status === "executing") {
61+
return false;
62+
}
63+
5764
// Only show on the last part of multi-part messages
5865
if (!msg.isLastPartOfMessage) return false;
5966

src/browser/utils/messages/retryEligibility.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,32 @@ describe("hasInterruptedStream", () => {
5959
expect(hasInterruptedStream(messages)).toBe(true);
6060
});
6161

62+
it("returns false for executing ask_user_question (waiting state)", () => {
63+
const messages: DisplayedMessage[] = [
64+
{
65+
type: "user",
66+
id: "user-1",
67+
historyId: "user-1",
68+
content: "Hello",
69+
historySequence: 1,
70+
},
71+
{
72+
type: "tool",
73+
id: "tool-1",
74+
historyId: "assistant-1",
75+
toolName: "ask_user_question",
76+
toolCallId: "call-1",
77+
args: { questions: [] },
78+
status: "executing",
79+
isPartial: true,
80+
historySequence: 2,
81+
streamSequence: 0,
82+
isLastPartOfMessage: true,
83+
},
84+
];
85+
86+
expect(hasInterruptedStream(messages)).toBe(false);
87+
});
6288
it("returns true for partial tool message", () => {
6389
const messages: DisplayedMessage[] = [
6490
{

src/browser/utils/messages/retryEligibility.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,19 @@ export function hasInterruptedStream(
9090

9191
const lastMessage = messages[messages.length - 1];
9292

93+
// ask_user_question is a special case: an unfinished tool call represents an
94+
// intentional "waiting for user input" state, not a stream interruption.
95+
//
96+
// Treating it as interrupted causes RetryBarrier + auto-resume to fire on app
97+
// restart, which re-runs the LLM call and re-asks the questions.
98+
if (
99+
lastMessage.type === "tool" &&
100+
lastMessage.toolName === "ask_user_question" &&
101+
lastMessage.status === "executing"
102+
) {
103+
return false;
104+
}
105+
93106
return (
94107
lastMessage.type === "stream-error" || // Stream errored out (show UI for ALL error types)
95108
lastMessage.type === "user" || // No response received yet (app restart during slow model)

0 commit comments

Comments
 (0)