Repository navigation
fix(native-chat): deliver AskUserQuestion answer by option number, not label (STA-1860) - #8840
Conversation
…t label
Claude Code's AskUserQuestion is an arrow-navigate selector: a bare Enter
commits the HIGHLIGHTED default (the first option), and pasted label text
does not move the highlight. Native chat delivered the answer as the option
LABEL text + Enter, so every non-first pick was silently committed as the
first option (STA-1860 Bug A).
Deliver the answer by each option's stable 1-based number instead (the marker
the selector commits on), paced so each keystroke lands after the selector
step renders. Covers single/multi-select, multi-question, and free-text
('Type something') answers. Verified live against Claude Code 2.1.210.
- native-chat-interactive-prompt.ts: buildAskAnswerKeys / AskAnswerSelection /
AskAnswerKeyGroup / hasAskAnswer; formatAskAnswer kept (index-based) for the
non-Claude label path
- native-chat-runtime-send.ts: sendNativeChatAskAnswer (paced keystroke groups)
replaces the old label-line sender
- card + interactive hook thread structured { indices, other } selections
STA-1860
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughInteractive question answers now use structured selections containing option indices and free text. Question cards pass these selections to the interactive send hook, which formats answers for non-Claude agents or generates selector keystrokes for Claude. Runtime delivery schedules raw and pasted steps with shared pacing, final submission timing, and cancellation. Tests cover selection mapping, formatting, key generation, routing, pacing, and cancellation. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsx (1)
97-109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the card visible when no answer was sent.
sendAnswerreturns0when the PTY is unavailable or the answer is empty, but this still dismisses the card immediately. Reset the submission guard and return instead.Proposed fix
submittingRef.current = true const settleMs = sendAnswer(card.prompt, selections) + if (settleMs === 0) { + submittingRef.current = false + return + } // Hold the card until the paced write finishes, then mark it answered
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 72cf8e1b-d130-42c7-a395-b99710f8519c
📒 Files selected for processing (9)
src/renderer/src/components/native-chat/NativeChatInteractiveCard.tsxsrc/renderer/src/components/native-chat/NativeChatQuestionCard.test.tsxsrc/renderer/src/components/native-chat/NativeChatQuestionCard.tsxsrc/renderer/src/components/native-chat/native-chat-interactive-prompt.test.tssrc/renderer/src/components/native-chat/native-chat-interactive-prompt.tssrc/renderer/src/components/native-chat/native-chat-runtime-send.test.tssrc/renderer/src/components/native-chat/native-chat-runtime-send.tssrc/renderer/src/components/native-chat/use-native-chat-interactive-send.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-interactive-send.ts
| groups.forEach((group, index) => { | ||
| timers.push( | ||
| setTimeout(() => { | ||
| sendRuntimePtyInput(settings, ptyId, NATIVE_CHAT_SUBMIT) | ||
| }, enterAt) | ||
| const bytes = 'raw' in group ? group.raw : buildNativeChatPasteBytes(group.text) | ||
| sendRuntimePtyInput(settings, ptyId, bytes) | ||
| }, index * NATIVE_CHAT_QUESTION_STEP_MS) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not advance past a text group until its PTY write is accepted.
sendRuntimePtyInput can return optimistically while asynchronous size validation later drops the text. The subsequent Enter still fires, committing an empty answer and dismissing the card. Prevalidate text groups or expose an awaitable acceptance result before scheduling the next control group.
Cancel stale AskUserQuestion timers before replacement UI can commit, and localize the multi-step sending state in every shipped locale.
Reconcile stablyai#8840 (STA-1860: AskUserQuestion answers by option number) with this branch's shared-module refactor: move buildAskAnswerKeys / hasAskAnswer / AskAnswerSelection / AskAnswerKeyGroup and the selection-based formatAskAnswer INTO src/shared/native-chat-ask.ts (the barrel re-exports them), take main's group-based sendNativeChatAskAnswer, and gate the keystroke path on shouldStepNativeChatAskAnswer so OpenClaude takes it too. Keep formatCompleteAskAnswer temporarily for the mobile Ask card (ported next commit). Sync the mobile ask test to the selection-based signature.
Port stablyai#8840's fix to the mobile native chat: the Ask card now tracks per-question option INDICES (+ free text) and the answer-send hook drives Claude's arrow-navigate selector with buildAskAnswerKeys keystroke groups — option numbers, next-tab arrows, Enter — paced one selector step apart, instead of pasting label text that the selector ignores (which silently committed the default option). Non-Claude agents keep the pasted-label path via the selection-based formatAskAnswer. Backcompat: keystrokes are built client-side and written through the EXISTING terminal.send passthrough with enter:false — the same contract the permission card already uses — so an older desktop runtime (SSH/relay included) replays them verbatim; no RPC/contract change in either update order. Free text is newline-sanitized because terminal.send has no paste framing. Drops the now-unused formatCompleteAskAnswer from the shared module.
…5824) * feat(native-chat): add native chat view across mobile * fix(native-chat): address review findings and CodeRabbit threads Correctness: - Restore an independent initial readSession seed and surface initial-drain errors as snapshot frames so the chat view can never strand on 'loading' - Pair mobile tool results to calls by ordinal FIFO (parallel calls no longer misgraft results); clear a pending ask only when its own call resolves - Show a new streaming reply immediately (same-turn suppression, not length) - Delegate mobile noise filtering to the shared harness-injected classifier - Admit soft-leaving mobile clients in beginMobileInputFloor (parity with mobileTookFloor) so grace-window writes aren't dropped - Self-heal a stale 'working' status once this turn's reply lands - Catch RPC rejections in mobile file-open helpers; guard sanitizeToolInput key collisions; settle web/runtime transports on unrecognized first frames and forward snapshot errors Perf: - Throttle the mobile streaming bubble (50ms) so per-part status frames stop re-parsing the whole accumulated markdown - Short-circuit markdown path detection on dot-less or oversized runs (quadratic backtracking guard) UX/minor: - Wire hold-mode dictation through the native chat composer - Allow scoped-package (@) paths in file-path detection - Move caret after mid-text autocomplete insertion; index-prefixed ask option keys; single scroll-to-end effect; bounded wait + toast when image attach races a resubscribe; count-based pending reconciliation; cache-hit search cancels stale debounce; chat-tab toggle wins over in-flight preference load - Share shouldStepNativeChatAskAnswer between desktop and mobile; import block guards/source priority from shared instead of local copies - Defensive non-positive transcript limits; test strengthening (TTL expiry, post-unsubscribe stale frame, lease readiness, filtered console.error) * refactor(native-chat): share desktop/mobile chat logic in src/shared Extract the parity-mirrored native-chat modules into shared implementations both surfaces re-export: ask parsing (registry, parseAskFromStatus, extractPendingAsk, formatAskAnswer), answer stepping offsets/scheduler, diff detection/parsing, harness-noise filtering, tool fold/pair/split, and tool summaries. Removes the hand-synced copies and their stale Metro comments. Divergence reconciliations take the safer side of each: diffs truncate at 120 lines/32KB everywhere (desktop previously unbounded), tool-run summaries cap at 3 parts with bounded-depth previews, nameless tool calls are skipped, and basenames split on both separators. Also: settle and kill every sibling quick-open pass when one reaches maxResults (main rg/git and relay git; relay rg already did) so a capped search cannot leave a scan walking a huge tree; fold window-bounding into the shared merger's applyAppend; localize the web 'Pair a host' snapshot error. * fix(native-chat): address CodeRabbit follow-ups on shared modules - Attachment lease gate re-checks connection/target/tab after the bounded wait, so a tab/host switch or disconnect mid-wait can't send into a stale terminal; a moved-away target drops silently like the pre-wait guard and only an unrecovered lease surfaces the toast. Adds hook tests. - extractPendingAsk parses transcript tool-calls through the same registered-parser + canonical-shape fallback as live status, so a custom question tool that rendered live survives reconnect/replay. - Direct unit tests for the shared ask parser (FIFO ordering, fallback, malformed payloads) and tool-summary bounded preview (depth/collection caps, circular refs, basename/command branches). * fix(native-chat): treat initialLimit 0 as a valid empty window Both engine guards used truthiness, so an explicit zero limit skipped the bounded tail reader and fell back to an unbounded incremental read. Latent only (every caller clamps positive), hardened for consistency with the tail reader's non-positive-limit handling. * fix(mobile): native-chat composer lock UX + send-failure feedback - Distinguish input-lock reasons: transport 'disconnected' shows Reconnecting… instead of mislabeling a reconnect as locked-by-another-client - Guard the composer lock behind a 600ms hold so connState blips / lease hand-offs don't flicker the placeholder; unlock stays instant - Surface a rejected send inline above the composer (a bottom toast hides behind the keyboard); auto-dismisses after 4s - waiting-session hint invites the first message instead of implying the agent is still starting * test(mobile): sync answer-send pacing test to the 500ms advance buffer Missed in merge 8fe3c39, which carried main's NATIVE_CHAT_ADVANCE_BUFFER_MS 300->500 (#8568) into the shared stepping module that mobile derives from. * fix(mobile): restore terminal stream after chat cold start * fix(native-chat): harden retries, optimistic sends, and file scans * fix(mobile): deliver AskUserQuestion answers by option number (STA-1860) Port #8840's fix to the mobile native chat: the Ask card now tracks per-question option INDICES (+ free text) and the answer-send hook drives Claude's arrow-navigate selector with buildAskAnswerKeys keystroke groups — option numbers, next-tab arrows, Enter — paced one selector step apart, instead of pasting label text that the selector ignores (which silently committed the default option). Non-Claude agents keep the pasted-label path via the selection-based formatAskAnswer. Backcompat: keystrokes are built client-side and written through the EXISTING terminal.send passthrough with enter:false — the same contract the permission card already uses — so an older desktop runtime (SSH/relay included) replays them verbatim; no RPC/contract change in either update order. Free text is newline-sanitized because terminal.send has no paste framing. Drops the now-unused formatCompleteAskAnswer from the shared module. * fix native chat send and runtime races * fix mobile native chat formatting * fix(native-chat): mobile empty state matches desktop copy Mobile showed a single generic line ('Send a message to get started') where desktop shows a titled two-line empty state naming the agent ('Start a chat with Claude' + 'Ask Claude to inspect code, explain output, or make a change.'). Align them from one source of truth so they can't drift again: - Extract the agent-type label map + formatAgentTypeLabel to src/shared/agent-type-label.ts (desktop re-exports; mobile imports). - Add src/shared/native-chat-empty-state.ts with the canonical English copy; desktop uses it as its i18n fallbacks (localization unchanged — en/es/ja/ko/zh keys still win), mobile substitutes the agent label and renders it directly (mobile ships English only). - Mobile: render title + subtitle for waiting-session AND ready-but-empty (both are 'start a chat'), error copy for errors; keep the loading spinner. Live-verified on the iOS sim against a pn-dev of this branch. typecheck node/web + mobile tsc clean; 30 mobile + 428 desktop/shared native-chat tests green. * style: oxfmt the empty-state parity test (line wrap) --------- Co-authored-by: Brennan Benson <brennanbenson@Brennans-MacBook-Pro.local> Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
…t label (STA-1860) (stablyai#8840) * fix(native-chat): deliver AskUserQuestion answer by option number, not label Claude Code's AskUserQuestion is an arrow-navigate selector: a bare Enter commits the HIGHLIGHTED default (the first option), and pasted label text does not move the highlight. Native chat delivered the answer as the option LABEL text + Enter, so every non-first pick was silently committed as the first option (STA-1860 Bug A). Deliver the answer by each option's stable 1-based number instead (the marker the selector commits on), paced so each keystroke lands after the selector step renders. Covers single/multi-select, multi-question, and free-text ('Type something') answers. Verified live against Claude Code 2.1.210. - native-chat-interactive-prompt.ts: buildAskAnswerKeys / AskAnswerSelection / AskAnswerKeyGroup / hasAskAnswer; formatAskAnswer kept (index-based) for the non-Claude label path - native-chat-runtime-send.ts: sendNativeChatAskAnswer (paced keystroke groups) replaces the old label-line sender - card + interactive hook thread structured { indices, other } selections STA-1860 * fix(native-chat): harden AskUserQuestion answer lifecycle * fix(native-chat): stop stale paced answer writes * fix(native-chat): cancel paced answers during commit Cancel stale AskUserQuestion timers before replacement UI can commit, and localize the multi-step sending state in every shipped locale.
…tablyai#5824) * feat(native-chat): add native chat view across mobile * fix(native-chat): address review findings and CodeRabbit threads Correctness: - Restore an independent initial readSession seed and surface initial-drain errors as snapshot frames so the chat view can never strand on 'loading' - Pair mobile tool results to calls by ordinal FIFO (parallel calls no longer misgraft results); clear a pending ask only when its own call resolves - Show a new streaming reply immediately (same-turn suppression, not length) - Delegate mobile noise filtering to the shared harness-injected classifier - Admit soft-leaving mobile clients in beginMobileInputFloor (parity with mobileTookFloor) so grace-window writes aren't dropped - Self-heal a stale 'working' status once this turn's reply lands - Catch RPC rejections in mobile file-open helpers; guard sanitizeToolInput key collisions; settle web/runtime transports on unrecognized first frames and forward snapshot errors Perf: - Throttle the mobile streaming bubble (50ms) so per-part status frames stop re-parsing the whole accumulated markdown - Short-circuit markdown path detection on dot-less or oversized runs (quadratic backtracking guard) UX/minor: - Wire hold-mode dictation through the native chat composer - Allow scoped-package (@) paths in file-path detection - Move caret after mid-text autocomplete insertion; index-prefixed ask option keys; single scroll-to-end effect; bounded wait + toast when image attach races a resubscribe; count-based pending reconciliation; cache-hit search cancels stale debounce; chat-tab toggle wins over in-flight preference load - Share shouldStepNativeChatAskAnswer between desktop and mobile; import block guards/source priority from shared instead of local copies - Defensive non-positive transcript limits; test strengthening (TTL expiry, post-unsubscribe stale frame, lease readiness, filtered console.error) * refactor(native-chat): share desktop/mobile chat logic in src/shared Extract the parity-mirrored native-chat modules into shared implementations both surfaces re-export: ask parsing (registry, parseAskFromStatus, extractPendingAsk, formatAskAnswer), answer stepping offsets/scheduler, diff detection/parsing, harness-noise filtering, tool fold/pair/split, and tool summaries. Removes the hand-synced copies and their stale Metro comments. Divergence reconciliations take the safer side of each: diffs truncate at 120 lines/32KB everywhere (desktop previously unbounded), tool-run summaries cap at 3 parts with bounded-depth previews, nameless tool calls are skipped, and basenames split on both separators. Also: settle and kill every sibling quick-open pass when one reaches maxResults (main rg/git and relay git; relay rg already did) so a capped search cannot leave a scan walking a huge tree; fold window-bounding into the shared merger's applyAppend; localize the web 'Pair a host' snapshot error. * fix(native-chat): address CodeRabbit follow-ups on shared modules - Attachment lease gate re-checks connection/target/tab after the bounded wait, so a tab/host switch or disconnect mid-wait can't send into a stale terminal; a moved-away target drops silently like the pre-wait guard and only an unrecovered lease surfaces the toast. Adds hook tests. - extractPendingAsk parses transcript tool-calls through the same registered-parser + canonical-shape fallback as live status, so a custom question tool that rendered live survives reconnect/replay. - Direct unit tests for the shared ask parser (FIFO ordering, fallback, malformed payloads) and tool-summary bounded preview (depth/collection caps, circular refs, basename/command branches). * fix(native-chat): treat initialLimit 0 as a valid empty window Both engine guards used truthiness, so an explicit zero limit skipped the bounded tail reader and fell back to an unbounded incremental read. Latent only (every caller clamps positive), hardened for consistency with the tail reader's non-positive-limit handling. * fix(mobile): native-chat composer lock UX + send-failure feedback - Distinguish input-lock reasons: transport 'disconnected' shows Reconnecting… instead of mislabeling a reconnect as locked-by-another-client - Guard the composer lock behind a 600ms hold so connState blips / lease hand-offs don't flicker the placeholder; unlock stays instant - Surface a rejected send inline above the composer (a bottom toast hides behind the keyboard); auto-dismisses after 4s - waiting-session hint invites the first message instead of implying the agent is still starting * test(mobile): sync answer-send pacing test to the 500ms advance buffer Missed in merge 8fe3c39, which carried main's NATIVE_CHAT_ADVANCE_BUFFER_MS 300->500 (stablyai#8568) into the shared stepping module that mobile derives from. * fix(mobile): restore terminal stream after chat cold start * fix(native-chat): harden retries, optimistic sends, and file scans * fix(mobile): deliver AskUserQuestion answers by option number (STA-1860) Port stablyai#8840's fix to the mobile native chat: the Ask card now tracks per-question option INDICES (+ free text) and the answer-send hook drives Claude's arrow-navigate selector with buildAskAnswerKeys keystroke groups — option numbers, next-tab arrows, Enter — paced one selector step apart, instead of pasting label text that the selector ignores (which silently committed the default option). Non-Claude agents keep the pasted-label path via the selection-based formatAskAnswer. Backcompat: keystrokes are built client-side and written through the EXISTING terminal.send passthrough with enter:false — the same contract the permission card already uses — so an older desktop runtime (SSH/relay included) replays them verbatim; no RPC/contract change in either update order. Free text is newline-sanitized because terminal.send has no paste framing. Drops the now-unused formatCompleteAskAnswer from the shared module. * fix native chat send and runtime races * fix mobile native chat formatting * fix(native-chat): mobile empty state matches desktop copy Mobile showed a single generic line ('Send a message to get started') where desktop shows a titled two-line empty state naming the agent ('Start a chat with Claude' + 'Ask Claude to inspect code, explain output, or make a change.'). Align them from one source of truth so they can't drift again: - Extract the agent-type label map + formatAgentTypeLabel to src/shared/agent-type-label.ts (desktop re-exports; mobile imports). - Add src/shared/native-chat-empty-state.ts with the canonical English copy; desktop uses it as its i18n fallbacks (localization unchanged — en/es/ja/ko/zh keys still win), mobile substitutes the agent label and renders it directly (mobile ships English only). - Mobile: render title + subtitle for waiting-session AND ready-but-empty (both are 'start a chat'), error copy for errors; keep the loading spinner. Live-verified on the iOS sim against a pn-dev of this branch. typecheck node/web + mobile tsc clean; 30 mobile + 428 desktop/shared native-chat tests green. * style: oxfmt the empty-state parity test (line wrap) --------- Co-authored-by: Brennan Benson <brennanbenson@Brennans-MacBook-Pro.local> Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Fixes STA-1860 (Bug A). Pre-existing on
main; independent of the mobile native-chat PR #5824.Bug A — wrong option delivered when answering AskUserQuestion
Repro: In the desktop native chat view, ask Claude Code to use the AskUserQuestion tool (e.g. options "Tabs" / "Spaces"). Pick Spaces, click Send answer.
Before: the agent receives Tabs (the first/default option). Terminal echo:
User answered … → Tabs.After: the agent receives the selected option (Spaces).
Root cause
Claude Code's AskUserQuestion is an arrow-navigate selector: a bare Enter commits the highlighted default (the first option), and pasted label text does not move the highlight. The native chat delivered the answer as the option label text + Enter (
formatAskAnswer→sendNativeChatAnswer), so the label was ignored and the trailing Enter committed the default. Every non-first pick was silently delivered as the first option.Fix
Deliver the answer by each option's stable 1-based number — the marker the selector actually commits on, which already matches the card's badge number — instead of the label text. Keystrokes are written as separate groups paced one selector-step apart, because a navigation/number keystroke batched with the Enter that follows commits before the selector has applied the move.
Verified against the real Claude Code TUI (v2.1.210); covers every card shape:
Non-Claude agents (whose question tools commit a pasted answer) keep the label-text path via
formatAskAnswer.Changes
native-chat-interactive-prompt.ts:buildAskAnswerKeys+AskAnswerSelection/AskAnswerKeyGroup/hasAskAnswer;formatAskAnswernow takes the same index-based selections (kept for the non-Claude label path).native-chat-runtime-send.ts:sendNativeChatAskAnswer(paced keystroke-group sender) replaces the old label-linesendNativeChatAnswer/nativeChatQuestionOffsets.{ indices, other }[]selections instead of pre-joined answer text.Verification
● User answered Claude's questions: · Do you prefer tabs or spaces? → Spacesand it replied "You prefer Spaces for indentation." (Before the fix, the same flow delivered Tabs.){ indices: [1] }resolution. Full native-chat suite green (402 tests);typecheckclean.Bug B (2nd-card render lag) — investigated, not fixed here
Characterized as a transcript-sync issue in a separate subsystem, so — per the ticket — this PR ships Bug A and notes Bug B for follow-up. The native chat view renders the conversation from Claude's on-disk transcript JSONL; Claude buffers/flushes it late (in repro, the session's
.jsonlwas never written while running), so the view falls into "Could not load conversation — No transcript found" and the 2nd tool-call turn (and, in that error state, its card) appear only once the transcript syncs — the multi-minute lag. The interactive card's own data (interactivePrompt) is hook-driven and populates promptly (verified: a 2nd question surfaced in <30s), so the lag is the transcript-load gate, not the answer/parse path.Follow-up: mobile + backcompat (must land on #5824)
Mobile still has Bug A. The mobile answer path (
mobile/src/session/use-mobile-native-chat-answer-send.ts→sendMobileNativeChatMessage) still sends the option label text, so mobile AskUserQuestion answering delivers the wrong option too. Those files live on PR #5824 (not onmain), so they can't be fixed here — the mobile port is a follow-up on #5824, reconciled with this change once #8840 merges.Backcompat is a hard requirement (desktop and mobile update independently — either can update first). The answer→keystroke building must stay client-side: the desktop renderer and the mobile app each build their own keystrokes from the local selection and send them over the existing write-keystrokes / PTY passthrough. Do not add a new RPC method and do not move answer-formatting into the desktop runtime/server. That keeps every combination working:
The mobile fix must send raw keystrokes (numbers / arrows / Enter), like the permission card's
option.send— not bracketed-paste text.This desktop fix already satisfies the constraint. It is entirely renderer-local (all changes under
src/renderer/src/components/native-chat/): the keystrokes are built in the renderer (buildAskAnswerKeys) and written verbatim via the existingsendRuntimePtyInputpassthrough (localpty.writeand the remote runtime write RPC are both unchanged). No new RPC/IPC method, no shared-contract change, and no answer-formatting on the runtime/server — so it composes with the mobile port and with an older runtime over SSH/relay.