fix: never wedge or eat input — request timeouts, prompt restore, destroy confirm#18
Merged
Conversation
…troy confirm Four input-safety fixes for the TUI, all traced from a codebase audit: - requests are now bounded: `ClientHandle::request` times out after 30s (RequestTimeout) and a reader-task Drop guard cancels every pending request the moment the socket dies (RequestCancelled) — previously a `/fork` or `/provider` awaited inside the event loop could freeze the UI forever if the daemon died mid-request, recoverable only by kill -9. - the reconnect backoff now drains terminal input: `q`/Ctrl+C quit during the ~31s backoff window (previously the keyboard was dead the whole time) and stray keystrokes are discarded instead of replaying into the prompt after reconnect. - a slash-command typo or a send with no session no longer destroys the prompt text — the draft is restored alongside the error. - `/destroy` now opens the (previously built but unreachable) ConfirmDestroy modal: `y` destroys — surfacing daemon rejections instead of silently pretending — `n`/Esc cancel, everything else is absorbed so a stray key can't confirm. - the documented `m` cycle-mode key actually works now (was a silent no-op): interactive → guarded → autonomous, starting at interactive when the mode is unknown. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
==========================================
+ Coverage 68.24% 70.87% +2.63%
==========================================
Files 32 32
Lines 7976 8319 +343
==========================================
+ Hits 5443 5896 +453
+ Misses 2533 2423 -110
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Patch-coverage top-up for the input-safety changes: the production request() delegation (answered-in-time path), the Action::CycleMode reducer arm, and the /destroy no-session guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r path Coverage top-up #2 for the input-safety PR (84% → ~91% patch): - the reconnect backoff unit is injectable (1s in production, ms in tests), and event_loop/reconnect_with_backoff are generic over the render backend + terminal-event stream — the full 5-attempt budget path and the quit-during-backoff path now run in CI against a dead port on a TestBackend (a stray key is asserted DISCARDED, q quits). - the reader task's cancel-pending guard moved to module scope with a direct drop test (it also covers panic unwinds, which no exit-point call could). - ClientHandle::detached_for_tests() (doc-hidden) lets the TUI exercise request/error paths without a daemon — used to pin "/destroy failed:" surfacing when the socket dies mid-request. Remaining uncovered lines are the live-connection plumbing (run()'s outer reconnect loop, the reconnect success arm, destroy's post-success list refresh) — coverable only with a real WS handshake fixture. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What
Five input-safety fixes from a systematic audit of the TUI, all in the "never freeze, never lose what the user typed, never destroy without asking" family:
1. Requests are now bounded — the UI can no longer freeze forever
/fork,/provider,/destroy(and every otherrequest_okcall) await inside the event loop. PreviouslyClientHandle::requesthad no timeout, and a socket death did not cancel pending requests — if the daemon died mid-request the TUI wedged permanently (no keys, no redraw; recoverable only bykill -9).request()now times out after 30s (ClientError::RequestTimeout), deregistering the entry so a late response hits nothing.RequestCancelled, they don't wait out the timeout.2. The keyboard works during reconnect backoff
The backoff loop (~31s across 5 attempts) previously slept without polling terminal input:
q/Ctrl+C were dead the whole time, and everything typed replayed into the prompt after reconnect. The wait loop now selects over terminal events — quit keys quit, everything else is discarded.3. A slash typo no longer eats the prompt
take_prompt()drained the editor before parsing, so/reveiw <long message>or a send with no session focused destroyed the text with no recovery. Both error paths now restore the draft alongside the error.4.
/destroyasks firstThe
ConfirmDestroymodal existed (render + state + keymap comment) but was unreachable —/destroy(and aliases/close,/delete) destroyed instantly, errors ignored. Now:/destroyopens the modal;ydestroys (surfacing daemon rejections instead of silently pretending);n/Esc/qcancel; every other key is absorbed so a stray keystroke can't confirm./denyis one keystroke away from/destroyin the palette — this gate matters.5.
m(cycle mode) actually worksBound in the keymap and listed in
/help, but the reducer arm wasinfo!("not yet implemented")— a silent no-op a user would trust before an unattended run. Now cycles interactive → guarded → autonomous; an unknown/missing mode starts at interactive (never jumps straight to autonomous).Tests
yconsumes + fires,ncancels; keymap absorbs stray Enter/lettersnext_modecycle (incl. conservative unknown-mode start), quit-key classificationcargo test --workspace: 264 tui + 8 client tests green ·cargo fmt --checkclean🤖 Generated with Claude Code