fix(ui): keep AskUserQuestion wizard progress across session switches - #5814
UncertaintyDeterminesYou4ndMe wants to merge 1 commit into
Conversation
The multi-question AskUserQuestion prompt kept its progress (current question, committed answers, in-flight text) in component-local useState. Switching to another session unmounts the prompt, so returning restarted the wizard at question 1 and dropped the answers already given. Keep the wizard progress in a small per-request cache keyed by requestId (bounded, FIFO-evicted). The prompt restores from it on mount, mirrors every change into it, and clears it once a submit or stop succeeds. The progress and its requestId live in a single state object so the persistence effect always writes under the key the progress belongs to, even on the commit where the request prop just changed.
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed exact head fbf11d8120a01d1cbe6d40371b6fba5ef6c7f6ce.
The per-request state object and bounded cache fix the normal session-switch/remount path, but the failure-retention contract does not hold through the Desktop production adapters. I found 1 P2 (inline): failed answer/stop operations are caught by their callers and returned as fulfilled promises, so this component clears the cache even though the request is still pending. Switching away after that failure loses the wizard progress again.
Validation: clean install; build:test; full UI suite (697/697); full typecheck; lint; format; ASF headers; renderer architecture; focused prompt and AppShell stop tests; git diff --check. I also reproduced the production Stop path with the real createAppShellStopAction: the IPC rejected and the error toast fired, but remounting the same request restarted at 1 / 2 instead of resuming 2 / 2. Hosted test is green. Current main is 03237142; the PR is 1 commit ahead / 2 behind and merge-tree is clean.
Not verified: packaged Electron, native Windows/macOS, or a real Runtime Host transport failure.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| // Answered: drop the cached progress so a later remount (should the | ||
| // resolved prompt linger) does not resurrect a half-finished wizard. | ||
| // On failure the cache stays, preserving the answers for the retry. | ||
| clearQuestionWizardProgress(requestId); |
There was a problem hiding this comment.
[P2] Do not infer production success from promise fulfillment here. The main-chat adapter catches bridge failures and resolves normally (apps/desktop/src/renderer/app-shell-chat-actions.ts:523-549), and Side Chat does the same (apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts:1694-1703). Consequently, a rejected respondToUserQuestion is shown as an error but this line still clears the cache; switching sessions then loses the answers the user was supposed to retry. The Stop path has the same mismatch: createAppShellStopAction catches and resolves at apps/desktop/src/renderer/app-shell-stop-action.ts:51-79, while Side Chat catches at use-quote-companion.ts:1431-1478, so the clear at lines 185-187 also runs after a failed stop. The new failure tests inject callbacks that reject directly and therefore do not exercise either production adapter. Please propagate an explicit applied/failed result (or preserve rejection through this boundary) and cover the mounted Desktop callback path.
Summary
Fixes #5813.
The multi-question
AskUserQuestionwizard kept its progress (current question index, committed per-question answers, in-flight free-form text) in component-localuseState. Switching to another session unmounts the prompt, so returning restarted the wizard at question 1 and dropped the answers already given.Fix
Keep the wizard progress in a small per-request cache keyed by
requestId(bounded at 32 entries, FIFO-evicted) inpackages/ui/src/user-question-prompt-state.ts:requestIdit belongs to live in a single state object, so the persistence effect always writes under the key the progress actually belongs to — splitting key and progress across separate states let a request-switch commit momentarily write the old request's progress under the new request's id.requestId-change effect hydrates from the target request's cache, so two pending requests in different sessions keep independent progress. A render-time guard prevents the previous request's progress from flashing under the new request's questions on the switch commit.Scope notes
WebContentsVieweach keep their own copy. The same request shown in two surfaces resumes independently; first submit wins. Cross-surface sync is out of scope.requestIdis generated fresh per request and never reused, so entries for requests resolved elsewhere linger only until FIFO eviction and can never resurface under a different request.Test plan
New regression tests in
packages/ui/src/__tests__/user-question-prompt.test.tsx(all passing):tsc -p tsconfig.jsonis clean for the touched files. The 7 failures in the full@maka/uidist suite (Mermaid cache / hover card) reproduce identically on pristine main and are unrelated.