fix(tui): empty-Enter provider dialog + question modal wedge#361
Merged
Conversation
added 2 commits
July 9, 2026 22:06
… prompt With no provider configured the prompt says "press Enter to connect", but submit() returned at the empty-input guard before the no-model handler, so Enter did nothing. Open the provider dialog (promptModelWarning) on an empty Enter when no model is available. Closes #356
The question modal reset its submitting latch only on failure and is reused across queued questions, so after one answer the next question (and Esc) were silently swallowed; a stalled reply latched it forever; a reply for a dead ask never unmounted it. Port the permission modal's fixes: per-request re-arm, a 20s reply timeout, and local removal on success. Closes #357
There was a problem hiding this comment.
Pull request overview
Fixes two first-run and modal-stall issues in the AX Code terminal UI by aligning behavior with existing, hardened modal patterns and by honoring the prompt’s “press Enter to connect” placeholder.
Changes:
- Prompt: when the prompt is empty and no model is configured, pressing Enter now triggers
promptModelWarning()to open the provider dialog (or show the appropriate loading/failed toast). - Question modal: ports the permission modal’s hardening patterns—per-request state reset, a 20s reply timeout via
withTimeout, and optimistic local request removal on success—to prevent the modal from latching/stalling across queued questions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/ax-code/src/cli/cmd/tui/routes/session/question.tsx | Re-arms modal state per request id, adds reply timeout, and locally removes answered questions to prevent wedged/reused modal instances. |
| packages/ax-code/src/cli/cmd/tui/component/prompt/index.tsx | Ensures empty-Enter on first-run (no model) opens the provider dialog via existing promptModelWarning() behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
added 12 commits
July 10, 2026 10:12
sanitizePromptInput used an over-broad regex `/(?:<)?\d+;\d+;\d+[Mm]/g` where the SGR "<" mouse marker was optional, so any bare digit;digit;digit run followed by M/m was stripped from the prompt. That silently deleted real content the user typed or pasted -- ANSI color codes like 1;31;40m or 256-color sequences like 38;5;208m vanished on the keystroke that closed them. Anchor the pattern to the "<" marker that genuine SGR mouse residue always carries, so only real residue is removed and user text is preserved.
The Prompt component computed `keys = Object.keys(props.options)` once at creation and reused the same instance across queued requests (a replied+asked pair can flush together so permissions()[0] swaps without the list emptying). The frozen key list desynced from props.options: an isolation_escalation that hides "Allow always" after an ordinary edit left a blank button that wrongly routed into the persist-permission stage, and the reverse order dropped the "Allow always" button for a request that supported it. Make the option keys a createMemo so the For, keyboard nav, and selection all track the current request, and clamp store.selected back into range whenever the set changes so it never points at a removed option.
…the UI Tool parts were dispatched with a bare <Dynamic> and the app's only ErrorBoundary wraps the whole tree, so any throw inside a single tool renderer (malformed/partially-streamed metadata, server/client version skew, an unexpected tool shape) tore down the entire session view -- transcript, prompt, sidebar -- and the fallback's reset re-threw immediately, forcing a CLI restart. Wrap each tool render in a local ErrorBoundary that degrades to a compact inline 'failed to render <tool> output' row, keeping the rest of the session interactive.
…rkspace createStoreBackedRuntimeSyncActions and createSyncBootstrapFlow captured sdk.directory/sdk.client once at Sync-context init, binding them to the launch directory. sdk.setWorkspace() (opening a session in a git worktree or another project) swaps the client and directory, but the snapshots never updated, so footer toggles (autonomous, isolation, smart-llm, super-long) and MCP/LSP status kept showing the launch directory's values -- and the reconnect-recovery bootstrap actively overwrote the store with base-directory data while the user viewed a different-directory session. Accept live accessors for the values that change on a workspace switch (client and directory; url/fetch are stable) and resolve them per request, matching how sessionSync already reads sdk.client/sdk.directory inside its closures. The accessor params are backward-compatible with the existing value-based callers. Adds a runtime-sync test proving the client and directory accessors are re-read on every call after a simulated setWorkspace().
The prompt pipeline mixes two units: the edit buffer counts display columns (CJK/emoji = 2, \n = 1) while JS strings count UTF-16 units. Several paths sliced one with offsets from the other, corrupting anything a user typed with wide characters: - Submit: promptSubmissionView sliced input.text with display-unit extmark offsets, so CJK/emoji before a paste placeholder garbled the sent message and silently ate characters after it. Convert via stringIndexFromDisplayOffset. - Multi-line history/cursor: cursorOffset === stringWidth(plainText) never holds on multi-line text (\n counted 1 vs 0), breaking down-arrow history and teleporting the cursor. New endDisplayOffset() used at all four sites. - /editor round-trip: stringIndexFromDisplayOffset counted \n as width 0, and relocatePromptPartAfterEditor returned UTF-16 indices consumed as display offsets — corrupting multi-line prompts with placeholders on save. - Autocomplete reopened via backspace stored a UTF-16 @-index consumed as a display offset, deleting into preceding wide text on selection. Also: - Undo→redo permanently unlinked pasted content/attachments from their placeholders (submit sent the literal '[Pasted ~N lines]' with no part). Orphaned parts are now stashed by extmark id and re-linked on redo. - History move() treated history[0] as the draft, so a draft matching the oldest entry was cleared/replaced on navigation. Regression tests cover CJK/emoji offsets, multi-line round-trips, and the draft guard.
- DialogConfirm.show always resolved undefined: handlers deferred onConfirm to
a microtask but dialog.clear()'s onClose resolved first, so Confirm/Skip on
the update dialog and Confirm Redo were dead buttons. Run the handler
synchronously before clear() (the dialog-export-options pattern).
- DialogSelect discarded async onSelect promises: the confirmInFlight latch
released on the next microtask (double-Enter double-fired fork/revert/
provider auth) and row clicks bypassed the latch entirely. The wrapper now
awaits the handlers and the mouse path routes through confirmSelected().
A failing 'Write heap snapshot' no longer hard-exits the TUI via the
unhandledRejection handler (try/catch + toast).
- PageDown/PageUp in select dialogs wrapped to the opposite end instead of
clamping; page jumps now clamp and single-step to the nearest enabled row.
- Multi-prompt provider auth self-destructed: the text DialogPrompt lacked
autoClose={false} (unlike Code/Api methods), so the deferred clear aborted
prompt #2 and the flow silently died; cancels now toast.
… prompt
- Collapsed Edit diffs >30 lines showed 'Error parsing diff': the renderer
truncated the patch mid-hunk before parsing. Pass the full diff and clip the
collapsed preview by height instead.
- The v2 SDK client resolves {error} instead of rejecting, so three flows had
dead .catch handlers: /compact failed silently; rollback reported success and
clobbered the typed prompt when the server rejected the revert; workflow
dialogs toasted 'The operation was aborted.' after a deliberate close. All
sites now check result.error (and suppress abort errors on cancel).
- Expanded tool-output folds collapsed themselves while the turn streamed:
coalesceParts fabricated new wrappers each run so <For> recreated every row.
Wrappers are now identity-cached by part id.
- Fork's pre-filled prompt was dropped when the forked session record was
already in the store (Prompt ref never re-fires on session→session
navigation); a consume-once effect keyed on sessionID applies it now.
- Quality detail pane wedged into 'Quality action unavailable' when readiness
advanced; it now falls back to the workflow's current action.
…h, kv wipe - --prompt re-submitted a brand-new agent run every time Home remounted (the dedupe flag was per-mount). A module-level consumed flag guards both the injection and the submit. - route.navigate() shallow-merged the route store, so a stale initialPrompt leaked into every later Home visit and session open. navigate() now reconciles (replaces) route data, and Home consumes the prompt once. - A malformed custom theme crashed the whole TUI — persistently, since the selection lives in kv.json and the ErrorBoundary swallowed the recovery path. resolveTheme failures now fall back to the built-in theme with a toast + kv reset, and invalid custom themes are filtered out of the picker. - kv.set() racing the initial kv.json read persisted a near-empty snapshot (wiping saved settings) which the late read then re-reverted in memory. Pre-ready writes now buffer and merge on top of the loaded values; the startup THEME_MODE report no longer writes redundantly. - Spurious 'Agent X's configured model is not valid' toast when agents loaded before providers; the check now waits for provider_loaded and dedupes.
- Copying >~1MB on macOS silently failed while toasting 'Copied to clipboard': the text was passed as a single osascript argv element (ARG_MAX), and the spawn failure was swallowed. Feed pbcopy via stdin like the Linux helpers, check exit codes, and only suppress helper errors when OSC 52 actually emitted. - Concurrent setWorkspace calls raced in the worker's startEventStream check-then-act, leaking a never-aborted duplicate stream that kept emitting stale events for the old workspace. Guarded with a generation counter (mirrors context/sdk.tsx's sseGeneration).
…me fixes render-anti-patterns.test.ts pins exact source patterns; update the assertions that referenced the old buggy patterns (stringWidth(input.plainText) cursor check, deferred dialog-confirm microtask, unguarded resolveTheme, rollback error message) to pin the fixed ones instead.
The scanner drift already required a refresh on the desktop branches; this regenerates it for fix/tui and accepts the heuristic findings introduced by the TUI fixes after manual review: the undo/redo orphan map is capped at 50 and cleared on composer replacement, the display-part identity cache is component-scoped, kv pendingWrites drains on load, the flagged home/kv 'map growth' sites are signal/ref setters (not maps), worker 'ssrf' hits are local SDK fetches, and the clipboard 'path traversal' is the pbcopy spawn.
The pull_request synchronize event for 92e4258 never spawned the repo-structure/ax-code workflow runs (previous push's runs were still in flight); empty commit to re-fire CI on the current head.
…tree Both this branch and the desktop work refreshed self-scan-baseline.json, which made the PR conflicting — GitHub silently stops running pull_request workflows when it can't build the merge ref, which is why CI never fired on the last two pushes. The baseline is a deterministic artifact, so the resolution is simply regenerating it from the merged code (self-scan and typecheck both green).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Terminal-UI (TUI) reliability and correctness fixes.
Original modal fixes
submit()returned at the empty-input guard before the no-model handler, so Enter did nothing. Now an empty Enter with no model available opens the provider dialog (via the existingpromptModelWarning()).submittinglatch only on failure and is reused across queued questions, so after answering one, the next (and Esc) were silently swallowed; a stalled reply latched it forever; a reply for a dead ask never unmounted it. Ports the permission modal's three hardening mechanisms: per-request re-arm, a 20s reply timeout, and local removal on success.Additional fixes from the bug-sweep
sanitizePromptInputused/(?:<)?\d+;\d+;\d+[Mm]/gwith the SGR mouse<marker optional, so any baredigit;digit;digitrun followed byM/mwas stripped as you typed/pasted — ANSI color codes like1;31;40mor38;5;208msilently vanished. Anchored to the<marker real mouse residue always carries.keys = Object.keys(props.options)was computed once and frozen; a reused Prompt across queued requests (e.g. an ordinary edit followed by an isolation_escalation that hides "Allow always") rendered a blank/mis-routed button. Made the option keys acreateMemoand clamp the selection back into range when the set changes.<Dynamic>under the app's single top-level ErrorBoundary, so any throw tore down the entire session view and the reset re-threw. Each tool render is now wrapped in a local ErrorBoundary that degrades to an inline "failed to render output" row.createStoreBackedRuntimeSyncActions/createSyncBootstrapFlowsnapshottedsdk.directory/sdk.clientat init, so opening a session in a git worktree or other project left footer toggles (autonomous, isolation, smart-llm, super-long) and MCP/LSP status scoped to the launch directory, and the reconnect-recovery bootstrap overwrote the store with base-directory data. They now take live accessors resolved per request (backward-compatible with existing value callers).Known limitation (not in this PR): the transcript is still hard-capped at the newest 100 messages with no "load older" path — deferred because it needs a design decision (raise cap vs. paginate) rather than a drive-by patch.
Tested: full
ax-codetypecheck (tsgo --noEmit) clean; all 820 TUI unit tests pass (17 skipped); TUI layering + visual-snapshot guardrails pass. New unit test added for the workspace-switch accessor behavior; sanitizer test updated to assert ANSI codes are preserved.Closes #356
Closes #357