Skip to content

Commit f01f5f3

Browse files
committed
fix(toolcard): preserve AskUserQuestion draft state
- Keep unsubmitted answers, custom input, and submission state scoped by device surface, session, and tool call across chat switches. - Clear drafts when pending mailboxes, sessions, or device surfaces are removed to prevent stale interaction state. - Deselect blank Other answers and filter empty custom values before submission so models receive only meaningful selections. - Add regression coverage for remount restoration, cleanup, isolation, and multi-select payloads.
1 parent f1bd407 commit f01f5f3

6 files changed

Lines changed: 722 additions & 42 deletions

File tree

src/web-ui/src/flow_chat/store/FlowChatStore.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import { startupTrace } from '@/shared/utils/startupTrace';
1111
import { projectEffectiveToolItem } from '../utils/toolInvocationIdentity';
1212
import { dispatchJobStore } from '@/features/dispatch/dispatchJobStore';
1313
import { resetLiveSessionInteractionStoreForTest } from '../services/liveSessionInteractionStore';
14+
import {
15+
askUserQuestionDraftKey,
16+
askUserQuestionDraftStore,
17+
} from './askUserQuestionDraftStore';
1418

1519
const apiMocks = vi.hoisted(() => ({
1620
listSessions: vi.fn(),
@@ -164,6 +168,7 @@ const resetStore = () => {
164168
activeSessionId: null,
165169
}));
166170
dispatchJobStore.getState().clear();
171+
askUserQuestionDraftStore.setState({ drafts: {} });
167172
resetLiveSessionInteractionStoreForTest();
168173
flowChatStore.registerPersistUnreadCompletionCallback(() => {});
169174
};
@@ -1969,6 +1974,56 @@ describe('FlowChatStore historical session hydration state', () => {
19691974
).toEqual([]);
19701975
});
19711976

1977+
it('clears an unsubmitted question draft when an authoritative mailbox removes the tool', () => {
1978+
flowChatStore.setState(() => ({
1979+
sessions: new Map([[
1980+
'history-1',
1981+
createSession({
1982+
sessionId: 'history-1',
1983+
dialogTurns: [{
1984+
id: 'turn-live',
1985+
sessionId: 'history-1',
1986+
userMessage: { id: 'user-live', content: 'ask me', timestamp: 1 },
1987+
modelRounds: [],
1988+
status: 'processing',
1989+
startTime: 1,
1990+
}],
1991+
}),
1992+
]]),
1993+
activeSessionId: 'history-1',
1994+
}));
1995+
1996+
const pendingQuestion = {
1997+
toolId: 'ask-tool-1',
1998+
sessionId: 'history-1',
1999+
dialogTurnId: 'turn-live',
2000+
modelRoundId: 'round-question',
2001+
questions: {
2002+
questions: [{
2003+
question: 'Which verification should run?',
2004+
header: 'Verification',
2005+
options: [{ label: 'Focused', description: 'Run focused checks.' }],
2006+
}],
2007+
},
2008+
registeredAtMs: 3,
2009+
};
2010+
2011+
expect(flowChatStore.reconcilePendingUserQuestions('history-1', {
2012+
revision: 1,
2013+
questions: [pendingQuestion],
2014+
})).toBe(true);
2015+
2016+
const draftKey = askUserQuestionDraftKey('history-1', 'ask-tool-1');
2017+
askUserQuestionDraftStore.getState().setSingleAnswer(draftKey, 0, 'Focused');
2018+
expect(askUserQuestionDraftStore.getState().drafts[draftKey]).toBeDefined();
2019+
2020+
expect(flowChatStore.reconcilePendingUserQuestions('history-1', {
2021+
revision: 2,
2022+
questions: [],
2023+
})).toBe(true);
2024+
expect(askUserQuestionDraftStore.getState().drafts[draftKey]).toBeUndefined();
2025+
});
2026+
19722027
it('acquires an empty current-Turn base for Runtime event replay before applying interactions', async () => {
19732028
peerModeFlagMock.active = true;
19742029
const pendingQuestion = {

src/web-ui/src/flow_chat/store/FlowChatStore.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ import { sessionMatchesWorkspace } from '../utils/workspaceScope';
9696
import { resolveThreadGoalUserMessageDisplay } from '../utils/threadGoalDisplay';
9797
import { cleanRemoteUserInput } from '../utils/userInputText';
9898
import { useBackgroundSubagentActivityStore } from './backgroundSubagentActivityStore';
99+
import { askUserQuestionDraftStore } from './askUserQuestionDraftStore';
99100
import { sessionComposerStore } from './sessionComposerStore';
100101
import { completeSessionMutationReconciliation } from './sessionMutationStore';
101102
import { recordHistorySessionDiagnosticEvent } from '../services/historySessionDiagnostics';
@@ -5084,6 +5085,7 @@ export class FlowChatStore {
50845085

50855086
const removedSessionIds = this.removeSession(sessionId, options);
50865087
sessionComposerStore.getState().removeDrafts(removedSessionIds);
5088+
askUserQuestionDraftStore.getState().removeSessionDrafts(removedSessionIds);
50875089
this.pendingRemoveSessionOptions.delete(sessionId);
50885090
}
50895091

@@ -5315,6 +5317,7 @@ export class FlowChatStore {
53155317
: [];
53165318
this.surfaceContainers.delete(surfaceId);
53175319
sessionComposerStore.getState().removeSurfaceDrafts(surfaceId);
5320+
askUserQuestionDraftStore.getState().removeSurfaceDrafts(surfaceId);
53185321
this.forgetSurfaceMetadataRequests(surfaceId);
53195322

53205323
for (const [requestKey, request] of this.fullHistoryHydrationRequests) {
@@ -7472,6 +7475,7 @@ export class FlowChatStore {
74727475
? restored.interactionSnapshot.userQuestions
74737476
: undefined;
74747477
let applied = false;
7478+
let pendingQuestionSnapshotApplied = false;
74757479

74767480
this.setState(prev => {
74777481
if (
@@ -7565,6 +7569,7 @@ export class FlowChatStore {
75657569
turnsChanged = true;
75667570
}
75677571
if (questionReconciliation.revisionApplied && pendingUserQuestions) {
7572+
pendingQuestionSnapshotApplied = true;
75687573
this.userQuestionSnapshotRevisions.set(
75697574
sessionId,
75707575
pendingUserQuestions.revision,
@@ -7634,6 +7639,14 @@ export class FlowChatStore {
76347639
};
76357640
});
76367641

7642+
if (pendingQuestionSnapshotApplied && pendingUserQuestions) {
7643+
askUserQuestionDraftStore.getState().reconcilePendingTools(
7644+
scope.surfaceId,
7645+
sessionId,
7646+
pendingUserQuestions.questions.map(question => question.toolId),
7647+
);
7648+
}
7649+
76377650
if (applied) {
76387651
this.seedSessionHistoryLoadedRanges(sessionId, 'initial-tail');
76397652
}
@@ -7789,7 +7802,9 @@ export class FlowChatStore {
77897802
sessionId: string,
77907803
pendingUserQuestions: PendingUserQuestionSnapshot | undefined,
77917804
): boolean {
7805+
const surfaceId = getActiveSurfaceId();
77927806
let applied = false;
7807+
let pendingQuestionSnapshotApplied = false;
77937808
this.setState(prev => {
77947809
const session = prev.sessions.get(sessionId);
77957810
if (!session) {
@@ -7802,6 +7817,7 @@ export class FlowChatStore {
78027817
previousRevision,
78037818
);
78047819
if (reconciliation.revisionApplied && pendingUserQuestions) {
7820+
pendingQuestionSnapshotApplied = true;
78057821
this.userQuestionSnapshotRevisions.set(sessionId, pendingUserQuestions.revision);
78067822
}
78077823
if (!reconciliation.changed) {
@@ -7816,6 +7832,13 @@ export class FlowChatStore {
78167832
applied = true;
78177833
return { ...prev, sessions };
78187834
});
7835+
if (pendingQuestionSnapshotApplied && pendingUserQuestions) {
7836+
askUserQuestionDraftStore.getState().reconcilePendingTools(
7837+
surfaceId,
7838+
sessionId,
7839+
pendingUserQuestions.questions.map(question => question.toolId),
7840+
);
7841+
}
78197842
return applied;
78207843
}
78217844

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import { beforeEach, describe, expect, it } from 'vitest';
2+
3+
import {
4+
LOCAL_SURFACE_ID,
5+
activateSurface,
6+
} from '@/infrastructure/peer-device/deviceSurface';
7+
import {
8+
askUserQuestionDraftKey,
9+
askUserQuestionDraftStore,
10+
} from './askUserQuestionDraftStore';
11+
12+
describe('askUserQuestionDraftStore', () => {
13+
beforeEach(() => {
14+
activateSurface(LOCAL_SURFACE_ID);
15+
askUserQuestionDraftStore.setState({ drafts: {} });
16+
});
17+
18+
it('keeps answers and custom input isolated by session and tool call', () => {
19+
const sessionATool1 = askUserQuestionDraftKey('session-a', 'tool-1');
20+
const sessionATool2 = askUserQuestionDraftKey('session-a', 'tool-2');
21+
const sessionBTool1 = askUserQuestionDraftKey('session-b', 'tool-1');
22+
const store = askUserQuestionDraftStore.getState();
23+
24+
store.setSingleAnswer(sessionATool1, 0, 'PostgreSQL');
25+
store.setMultiAnswer(sessionATool1, 1, 'TypeScript', true);
26+
store.setMultiAnswer(sessionATool1, 1, 'Rust', true);
27+
store.setOtherInput(sessionATool1, 2, 'Custom answer');
28+
store.setSingleAnswer(sessionATool2, 0, 'SQLite');
29+
store.setSingleAnswer(sessionBTool1, 0, 'MySQL');
30+
31+
expect(askUserQuestionDraftStore.getState().drafts[sessionATool1]).toMatchObject({
32+
answers: {
33+
0: 'PostgreSQL',
34+
1: ['TypeScript', 'Rust'],
35+
},
36+
otherInputs: { 2: 'Custom answer' },
37+
});
38+
expect(askUserQuestionDraftStore.getState().drafts[sessionATool2].answers[0]).toBe('SQLite');
39+
expect(askUserQuestionDraftStore.getState().drafts[sessionBTool1].answers[0]).toBe('MySQL');
40+
});
41+
42+
it('keeps equal session and tool ids isolated across device surfaces', () => {
43+
const localKey = askUserQuestionDraftKey('same-session', 'same-tool');
44+
askUserQuestionDraftStore.getState().setSingleAnswer(localKey, 0, 'Local answer');
45+
46+
activateSurface('peer-b');
47+
const peerKey = askUserQuestionDraftKey('same-session', 'same-tool');
48+
askUserQuestionDraftStore.getState().setSingleAnswer(peerKey, 0, 'Peer answer');
49+
50+
expect(peerKey).not.toBe(localKey);
51+
expect(askUserQuestionDraftStore.getState().drafts[localKey].answers[0]).toBe('Local answer');
52+
expect(askUserQuestionDraftStore.getState().drafts[peerKey].answers[0]).toBe('Peer answer');
53+
});
54+
55+
it('preserves pending drafts and removes tools absent from an authoritative mailbox', () => {
56+
const retainedKey = askUserQuestionDraftKey('session-a', 'tool-retained');
57+
const removedKey = askUserQuestionDraftKey('session-a', 'tool-removed');
58+
const otherSessionKey = askUserQuestionDraftKey('session-b', 'tool-removed');
59+
const store = askUserQuestionDraftStore.getState();
60+
store.setSingleAnswer(retainedKey, 0, 'Keep');
61+
store.setSingleAnswer(removedKey, 0, 'Remove');
62+
store.setSingleAnswer(otherSessionKey, 0, 'Other session');
63+
64+
store.reconcilePendingTools(LOCAL_SURFACE_ID, 'session-a', ['tool-retained']);
65+
66+
expect(askUserQuestionDraftStore.getState().drafts[retainedKey]).toBeDefined();
67+
expect(askUserQuestionDraftStore.getState().drafts[removedKey]).toBeUndefined();
68+
expect(askUserQuestionDraftStore.getState().drafts[otherSessionKey]).toBeDefined();
69+
});
70+
71+
it('keeps submission phase across remounts without recreating a cleared draft', () => {
72+
const key = askUserQuestionDraftKey('session-a', 'tool-1');
73+
const store = askUserQuestionDraftStore.getState();
74+
store.setSingleAnswer(key, 0, 'PostgreSQL');
75+
store.setSubmissionPhase(key, 'submitting');
76+
expect(askUserQuestionDraftStore.getState().drafts[key].submissionPhase).toBe('submitting');
77+
78+
store.clearDraft(key);
79+
store.setSubmissionPhase(key, 'submitted');
80+
expect(askUserQuestionDraftStore.getState().drafts[key]).toBeUndefined();
81+
});
82+
83+
it('removes the Other marker when custom input becomes blank', () => {
84+
const multiKey = askUserQuestionDraftKey('session-a', 'tool-multi');
85+
const singleKey = askUserQuestionDraftKey('session-a', 'tool-single');
86+
const store = askUserQuestionDraftStore.getState();
87+
88+
store.setMultiAnswer(multiKey, 0, 'PostgreSQL', true);
89+
store.setMultiAnswer(multiKey, 0, 'Other', true);
90+
store.setOtherInput(multiKey, 0, 'Custom database');
91+
store.setOtherInput(multiKey, 0, ' ');
92+
93+
store.setSingleAnswer(singleKey, 0, 'Other');
94+
store.setOtherInput(singleKey, 0, 'Custom database');
95+
store.setOtherInput(singleKey, 0, '');
96+
97+
expect(askUserQuestionDraftStore.getState().drafts[multiKey]).toMatchObject({
98+
answers: { 0: ['PostgreSQL'] },
99+
otherInputs: { 0: '' },
100+
});
101+
expect(askUserQuestionDraftStore.getState().drafts[singleKey]).toMatchObject({
102+
answers: {},
103+
otherInputs: { 0: '' },
104+
});
105+
});
106+
107+
it('cleans up deleted sessions and discarded surfaces without disturbing others', () => {
108+
const removedSessionKey = askUserQuestionDraftKey('session-a', 'tool-a');
109+
const retainedSessionKey = askUserQuestionDraftKey('session-b', 'tool-b');
110+
let store = askUserQuestionDraftStore.getState();
111+
store.setSingleAnswer(removedSessionKey, 0, 'Remove session');
112+
store.setSingleAnswer(retainedSessionKey, 0, 'Keep session');
113+
114+
activateSurface('peer-b');
115+
const peerKey = askUserQuestionDraftKey('session-a', 'tool-a');
116+
askUserQuestionDraftStore.getState().setSingleAnswer(peerKey, 0, 'Remove peer');
117+
118+
activateSurface(LOCAL_SURFACE_ID);
119+
store = askUserQuestionDraftStore.getState();
120+
store.removeSessionDrafts(['session-a']);
121+
expect(askUserQuestionDraftStore.getState().drafts[removedSessionKey]).toBeUndefined();
122+
expect(askUserQuestionDraftStore.getState().drafts[retainedSessionKey]).toBeDefined();
123+
expect(askUserQuestionDraftStore.getState().drafts[peerKey]).toBeDefined();
124+
125+
store.removeSurfaceDrafts('peer-b');
126+
expect(askUserQuestionDraftStore.getState().drafts[peerKey]).toBeUndefined();
127+
expect(askUserQuestionDraftStore.getState().drafts[retainedSessionKey]).toBeDefined();
128+
});
129+
});

0 commit comments

Comments
 (0)