Lost work: editor handoff, steer ordering, and the /models error that said nothing - #6239
Conversation
… call site `/hooks edit` handed the terminal to `$EDITOR` while the TUI's input pump thread kept calling `event::read()` on the same tty. The two readers split the user's keystrokes: `:` and `!` reached `vi`, `Esc` and `Enter` were eaten by the composer, so the editor could not be quit and the failed quit attempts were submitted to the model as messages. On a single-terminal setup the session was unrecoverable. `with_suspended_tui` only ever handled crossterm state, and suspending raw mode does not stop a thread that is blocked in `event::read()`. The pause lived in the caller, and only one of the two callers had it — the composer editor paused, `edit_project_hooks_from_tui` had no `&TerminalInputPump` to pause with and never could. Rather than thread the pump through `apply_command_result`'s twelve call sites to fix one of them, put the pause where the handoff already is. `with_suspended_tui` now acquires an RAII `ChildTerminalInputPause` before it touches any terminal mode and releases it after the modes are restored, so every external-editor entry point is correct by construction and the "caller must remember to pause" rule is gone. It reaches the pump through a process-scoped gate: there is one stdin and one pump reading it, so the gate is a singleton by construction, not by convention. Fails closed. A pump that will not acknowledge the pause means the handoff would reproduce exactly this defect, so the editor does not run and the pump is left reading. Known limitations, written down beside the behaviour: the gate stops the pump reading but cannot drain input the pump already buffered, and cannot refuse the handoff on a pending Esc/Ctrl+C — those need the receiver and the event loop's pending queue, so `prepare_terminal_input_handoff` stays at the call sites that have them. `history.rs`'s `try_open_file_at_line` is still unfixed: it fire-and-forget `spawn()`s `$EDITOR` and never waits, which needs a different shape than a pause. Filed separately. Gates (macOS, this worktree): - `cargo fmt --all -- --check` clean, no files outside this slice touched - `cargo clippy -p codewhale-tui --all-targets --all-features --locked` with CI's allow list: clean - `sh scripts/with-hermetic-test-home.sh cargo test -p codewhale-tui --lib --all-features --locked -- child_terminal_pause input_pump_restart terminal_input_` → test result: ok. 5 passed; 0 failed; 0 ignored; 12734 filtered out - same, `-- external_editor::` → test result: ok. 12 passed; 0 failed; 0 ignored; 12727 filtered out Closes #6165 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Steering did not place the steer as the newest thing in the transcript. It was painted as a settled `HistoryCell::User` and pushed into `api_messages` the moment `EngineHandle::steer` accepted the text — but the steer channel accepting text is not a turn accepting it. The engine queues a mid-stream steer and commits it at the next step boundary, after the assistant message it followed, so the live transcript put the steer above work the record places before it. The two views genuinely disagreed; neither was merely odd. The same early paint produced a worse failure. `next_turn_steer` is a drain-and-discard filter: a steer stamped with a turn that has already moved on is popped and dropped, as are `pending_steers` on interrupt, failure and stream retry. The toast said "sent into turn", the cell stayed in history forever, and the model never saw the message. Move the presentation, not the engine — the record order is right, because the steer scopes the next step: - `steer_user_message` records an `InflightSteer` instead of painting. It renders through the existing pending-input bucket, whose label already says "sending into turn", so no new bucket and no sixteenth locale string. - `apply_engine_session_projection` promotes it: the engine's own record is where acceptance becomes observable and the only place the steer's real message index is known. Match is on the exact text handed to the engine, which it stores as the accepted user message's first text block, searched from the index the steer was sent after so an identical earlier message cannot claim it. Then `flush_active_cell()`, paint, and record the context references against the matched index. Live order equals record order by construction. - `TurnComplete` settles whatever was never accepted into `rejected_steers`, which already renders with a "could not send into turn" label. `next_turn_steer` stays a discard filter — that drop is tested behaviour (`new_turn_does_not_inherit_previous_turn_controls`). The engine, the turn loop and `echo_queued_user_turn`'s own paint-before-acceptance are untouched. Known limitation, written down beside the behaviour: the `+` marker is live-only and is not reconstructable on replay. `live_steer_crosses_message_submit_transform_exactly_once` asserted the transformed steer was in `api_messages` at send time — the reorder this removes. It now pins the same intent one layer up: the transform's output is what the engine was handed, and it is not in the local transcript until the engine records it. Gates (macOS, this worktree): - `cargo fmt --all -- --check` clean, no files outside this slice touched - `cargo clippy -p codewhale-tui --all-targets --all-features --locked` with CI's allow list: clean - `sh scripts/with-hermetic-test-home.sh cargo test -p codewhale-tui --lib --all-features --locked -- steer` → test result: ok. 30 passed; 0 failed; 0 ignored; 12711 filtered out - same, `-- tui::ui::` → test result: ok. 845 passed; 0 failed; 0 ignored; 11897 filtered out - same, `-- tui::app:: tui::widgets::` → test result: ok. 475 passed; 0 failed; 0 ignored; 12267 filtered out Closes #6190 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJENKJ2smviQW4FVGzUTk9
… secrets
`/models` against a geo-blocked Gemini key rendered as:
Failed to fetch models from Google Gemini: Invalid request (400):
It ends at the colon. The colon introduces the provider's reason and the
reason was missing — Google said "User location is not supported for the API
use" and the catalog path discarded it. A geo-block, a bad key and a wrong
endpoint all produced the identical empty message, so the reporter had to
change VPN exits to find out which one it was.
The discard is not an oversight. The catalog probe is the one request that
contacts a `base_url` the user typed during setup, and its URL carries an
opaque pagination cursor, so a provider — or anything answering at that URL
— can echo a key, a custom header value or the cursor back inside an error
body. `later_page_http_and_transport_errors_do_not_expose_cursor_or_key`
mounts exactly that body and pins that none of it reaches the user. Simply
flipping `include_error_body` to `true` surfaces the reason and fails that
canary. That would trade an unhelpful message for a credential leak.
So redact, then surface, and fall back to today's silence if redaction did
not hold. `send_with_retry_error_body`'s bool becomes an
`ErrorBodyDisclosure`: `Full` for established endpoints, unchanged, and
`Guarded` for the catalog probe. Guarded removes every value this client
knows is secret — the active API key, every user-configured HTTP header
value, everything already in `model_bound_secret_values`, and the request's
own query values in both decoded and percent-encoded form — from the raw
body *before* `sanitize_http_error_body` truncates it, so a secret can never
be split across the truncation boundary and survive as a fragment. It then
re-checks the sanitized result and drops it entirely if anything survived.
The fallback is the old contract, not a weaker one.
`catalog_error_secret_values` is deliberately a second, wider list rather
than a widening of `model_bound_secret_values`: they answer different
questions at different trust boundaries. That one is "what must never reach
a *model*", which is why it covers only auth-shaped headers and values of at
least `MIN_EXACT_SECRET_CHARS`. This one is "what must never come back out
of an endpoint the user typed", where a short key is still a key and a
custom header the user configured is still theirs.
Known limitation, in the code beside it: this removes what the *client*
knows is secret. A credential configured outside Codewhale — in a proxy,
say — is not in that set, the same limit `redact_model_bound_text` has.
Deliberately not done: special-casing region errors into bespoke advice.
Showing the provider's own words is the general fix and does not go stale
when Google changes its wording.
The canary test is untouched — it is the gate this had to pass, not a test
to loosen.
Gates (macOS, this worktree):
- `cargo fmt --all -- --check` clean, no files outside this slice touched
- `cargo clippy -p codewhale-tui --all-targets --all-features --locked`
with CI's allow list: clean
- `sh scripts/with-hermetic-test-home.sh cargo test -p codewhale-tui --lib
--all-features --locked -- client::catalog_tests::`
→ test result: ok. 14 passed; 0 failed; 0 ignored; 12728 filtered out
- same, `-- client::`
→ test result: ok. 520 passed; 0 failed; 0 ignored; 12222 filtered out
Closes #6173
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AJENKJ2smviQW4FVGzUTk9
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @Hmbown's task in 5m 40s —— View job Review: PR #6239 — Lost work batch A
Note: I was unable to run SummaryAll three fixes are well-scoped and the diff matches what the PR description claims. No correctness bugs, no provider/model-prefix inference issues (this PR doesn't touch routing), and no security regressions. A few observations below, none blocking. #6165 — terminal input pause (
|
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
Complete review coverage: 1/1 passes, 12 file patches, sha256:ade16ae638dbd851ef89c2b4d438e3126a4c10a38d550d7d2fabc344e4adc479.
Pass 1: Reviewed the 12-file patch for #6165 (pump pause inside with_suspended_tui), #6190 (in-flight steer settlement) and #6173 (guarded catalog error bodies). The design choices are sound and the redaction-before-truncation ordering is correct, but the steer settlement covers only some of the engine's turn-terminal / projection paths, rejected steers can leave a stale queue-time transcript cell, and the new process-global child-terminal gate is neither re-entrant nor reset by its tests.
Findings
- [WARNING] A rejected steer can leave the queue-time transcript cell behind (
crates/tui/src/tui/ui/dispatch.rs:1301)
settle_unaccepted_steers_at_turn_end only pushes the display into rejected_steers; it never touches the transcript cell the same QueuedMessage may already own. The old code path rewrote that cell through paint_user_turn_cell ("A message echoed at queue time already owns a transcript cell"), and the rewrite now happens only on acceptance (settle_accepted_steers). So for a steer that was echoed at queue time (history_echoed) and then dropped by the engine, the user gets both a "could not send into turn" receipt and a permanent transcript cell for input the model never received - the exact class of defect #6190 is fixing. No test covers a dropped steer that had a queue-time echo; both new tests start from a steer with no existing cell. - [WARNING] In-flight steers only settle on TurnComplete and only on SessionUpdated (
crates/tui/src/tui/ui/event_loop.rs:2445)
Acceptance is settled in apply_engine_session_projection and rejection only in the TurnComplete branch. Any other way a turn ends (engine error/abort/interrupt, a cancelled turn, or a session switch) leaves the entry rendering as "sending into turn" forever, and nothing else clears it. The mirror ordering hazard also exists: if the engine commits a steer and TurnComplete reaches the event loop before the projection that contains it, the steer is reported as not sent and is removed from inflight_steers, so the projection that later shows it can never paint its cell - the live transcript then disagrees with api_messages in the other direction. Both paths are untested; the new tests call the settle helpers directly rather than driving the event loop. - [WARNING] The child-terminal pause guard is not re-entrant (
crates/tui/src/tui/ui/terminal_input.rs:140)
ChildTerminalInputPause::drop unconditionally stores false into the shared paused/paused_ack atomics it cloned from the published gate. The pre-existing TerminalInputPump::pause_for_child_terminal toggles the same two atomics, and the doc comment on CHILD_TERMINAL_GATE states that prepare_terminal_input_handoff "stays with the call sites that have them" - i.e. the same call sites that now enter with_suspended_tui. If a call site holds the older pause across a with_suspended_tui call, the inner guard's Drop releases the outer pause early, and the pump reads the tty again while the outer handoff still believes it is paused. Either assert that the two mechanisms are never nested at one site, or make the pause owned/counted so the last holder is the one that resumes. - [WARNING] New tests leave a stale published gate for the rest of the test binary (
crates/tui/src/tui/ui/tests.rs:26200)
Both new terminal-gate tests publish into the process-global CHILD_TERMINAL_GATE and never retract it (retract_child_terminal_gate is private to terminal_input, so the test module cannot call it; the worker thread of the first test is stopped and the second test has no worker at all). Nothing acknowledges the pause afterwards, so any later pause_terminal_input_for_child() in the same test process - including tests that only read the gate, which the lock does not protect, and any test relying on the documented inert-guard behaviour for "no pump published - burns the full TERMINAL_INPUT_CHILD_PAUSE_TIMEOUT and fails the handoff. With --test-threads=2 the ordering is nondeterministic. Add a test-visible retract/reset and call it in a guard at the end of each gate test. - [INFO] Full leak re-check can drop a body that was redacted correctly (
crates/tui/src/client.rs:3536)
catalog_error_secret_values pushes the active key with no length floor, and every configured header value, while the leak check runs message.contains(secret) against the already-substituted text. A short secret that is a substring of the replacement marker (codewhale_config::persistence::REDACTED) therefore reappears in the output and the whole body is dropped - reproducing #6173's empty "Invalid request (400): " with no diagnostic at all, exactly the case the re-check was meant to make impossible. Checking the pre-replacement body (or substituting with a marker that cannot contain a secret) removes the false positive while keeping the fail-closed contract. Separately, redacting every request query value and every header value over-redacts: values such as alt=json or pageSize=100, and structural header values, get rewritten inside otherwise-useful provider prose. - [INFO] Steer acceptance matching is index+exact-text only (
crates/tui/src/tui/ui/dispatch.rs:1286)
accepted_steer_index accepts the first unclaimed Role::User message at or after sent_after_index whose first content block is byte-equal to the string handed to EngineHandle::steer. Two consequences worth pinning: (a) any engine-side normalization of the steer text (trim, prefix, cache_control marker) turns an accepted steer into a false "could not send into turn" receipt, and (b) the lower bound is an index, not a session identity, so a projection from a different session whose length exceeds sent_after_index can donate a coincidentally identical message, attaching the steer's cell and ContextReferences to text the user never steered. Theclaimedset only prevents double-claiming within one pass. - [INFO] Refresh-mode catalog errors still bypass ErrorBodyDisclosure (
crates/tui/src/client.rs:3013)
Only ModelsRequestMode::Interactive was moved onto send_with_retry_error_body with a Guarded disclosure; the Refresh arm still calls build().send() directly, so its error body is produced by whatever path it used before this change - neither the guarded redaction nor the old blanket suppression is applied there. Since the refresh URL also carries the pagination cursor, confirm that this path cannot surface the body to logs or the user unredacted, or route it through the same disclosure value. - [INFO] Pause acknowledgement depends on the pump's poll cadence (
crates/tui/src/tui/ui/terminal_input.rs:108)
pause_terminal_input_for_child fails closed on timeout, so the correctness of /hooks edit now depends entirely on the pump loop leaving crossterm's blocking read often enough to observepausedand set the ack. The new test substitutes a 1ms-sleeping worker, so it cannot catch a pump that only observes the flag after the next keystroke - which would make the handoff fail (or block for TERMINAL_INPUT_CHILD_PAUSE_TIMEOUT) on a quiet terminal. Worth a targeted assertion against the real pump loop's wait path, not just the simulated one.
Suggestions
crates/tui/src/tui/ui/dispatch.rs:1301— Before recording the rejection, drop or rewrite the transcript cell this QueuedMessage already owns (the queue-time echo path handled by paint_user_turn_cell), otherwise a dropped steer leaves a cell the engine's record never had - the same defect #6190 removes for the accepted case. Add a test that steers a message which was echoed at queue time and then drops it.crates/tui/src/tui/ui/event_loop.rs:2445— Settle in-flight steers on every turn-terminal event (error, abort, cancel, session teardown), not only TurnComplete, and make sure the engine's committing projection is always applied before the terminal event settles the remainder - otherwise a steer either renders as "sending into turn" forever or is falsely reported as not sent.crates/tui/src/tui/ui/terminal_input.rs:140— Do not unconditionally clear the shared paused/paused_ack flags on drop. Track the holder (nesting count or an owner token) so an inner guard cannot release a pause held by the pre-existing pause_for_child_terminal at the same call site.crates/tui/src/tui/ui/tests.rs:26200— Expose a test-visible retract/reset for CHILD_TERMINAL_GATE and call it at the end of both new gate tests (and in the restart test). Leaving a published gate whose worker no longer acknowledges makes any later pause_terminal_input_for_child() in the same process time out and refuse the handoff.crates/tui/src/client.rs:3536— Run the leak check against the pre-replacement body (or against a marker that no secret can be a substring of) so a correctly redacted body cannot fail its own re-check and be dropped, which would reproduce the silent empty error from #6173. Consider excluding non-opaque query values and structural header values from the redaction set to avoid mangling the provider's reason.
Assessment
Pass 1: The three fixes are well reasoned and the #6173 ordering (redact raw, then sanitize, then fail closed) is implemented correctly, including the longest-first masking order. The main gaps are in #6190's settlement coverage: only two of the engine's exit paths settle a steer, the rejected path never cleans up the queue-time cell, and the process-global gate introduced for #6165 is not re-entrant and is left published by its own tests. None of these is a certain build break, but several are one step away from the same 'user loses something' symptom the PR is closing, and the new tests exercise the helpers rather than the event-loop paths that decide them. I would land the catalog change as-is and ask for a second pass on steer settlement paths plus test isolation for the gate.
Advisory review by Codewhale (codewhale review --pr 6239 --post, head 87754c1c9fd13227c792857ae862ac83df179a70). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| return; | ||
| } | ||
| for steer in std::mem::take(&mut app.inflight_steers) { | ||
| app.rejected_steers.push_back(steer.message.display); |
There was a problem hiding this comment.
[WARNING] A rejected steer can leave the queue-time transcript cell behind
settle_unaccepted_steers_at_turn_end only pushes the display into rejected_steers; it never touches the transcript cell the same QueuedMessage may already own. The old code path rewrote that cell through paint_user_turn_cell ("A message echoed at queue time already owns a transcript cell"), and the rewrite now happens only on acceptance (settle_accepted_steers). So for a steer that was echoed at queue time (history_echoed) and then dropped by the engine, the user gets both a "could not send into turn" receipt and a permanent transcript cell for input the model never received - the exact class of defect #6190 is fixing. No test covers a dropped steer that had a queue-time echo; both new tests start from a steer with no existing cell.
| // A steer the turn never accepted was dropped by the | ||
| // engine. Report it instead of leaving it "sending" | ||
| // (#6190). | ||
| crate::tui::ui::dispatch::settle_unaccepted_steers_at_turn_end(app); |
There was a problem hiding this comment.
[WARNING] In-flight steers only settle on TurnComplete and only on SessionUpdated
Acceptance is settled in apply_engine_session_projection and rejection only in the TurnComplete branch. Any other way a turn ends (engine error/abort/interrupt, a cancelled turn, or a session switch) leaves the entry rendering as "sending into turn" forever, and nothing else clears it. The mirror ordering hazard also exists: if the engine commits a steer and TurnComplete reaches the event loop before the projection that contains it, the steer is reported as not sent and is removed from inflight_steers, so the projection that later shows it can never paint its cell - the live transcript then disagrees with api_messages in the other direction. Both paths are untested; the new tests call the settle helpers directly rather than driving the event loop.
| impl Drop for ChildTerminalInputPause { | ||
| fn drop(&mut self) { | ||
| if let Some(gate) = self.gate.take() { | ||
| gate.paused_ack.store(false, Ordering::Release); |
There was a problem hiding this comment.
[WARNING] The child-terminal pause guard is not re-entrant
ChildTerminalInputPause::drop unconditionally stores false into the shared paused/paused_ack atomics it cloned from the published gate. The pre-existing TerminalInputPump::pause_for_child_terminal toggles the same two atomics, and the doc comment on CHILD_TERMINAL_GATE states that prepare_terminal_input_handoff "stays with the call sites that have them" - i.e. the same call sites that now enter with_suspended_tui. If a call site holds the older pause across a with_suspended_tui call, the inner guard's Drop releases the outer pause early, and the pump reads the tty again while the outer handoff still believes it is paused. Either assert that the two mechanisms are never nested at one site, or make the pause owned/counted so the last holder is the one that resumes.
| input.resume_after_child_terminal(); | ||
| } | ||
|
|
||
| /// `CHILD_TERMINAL_GATE` is process-global — one stdin, one pump. Any test |
There was a problem hiding this comment.
[WARNING] New tests leave a stale published gate for the rest of the test binary
Both new terminal-gate tests publish into the process-global CHILD_TERMINAL_GATE and never retract it (retract_child_terminal_gate is private to terminal_input, so the test module cannot call it; the worker thread of the first test is stopped and the second test has no worker at all). Nothing acknowledges the pause afterwards, so any later pause_terminal_input_for_child() in the same test process - including tests that only read the gate, which the lock does not protect, and any test relying on the documented inert-guard behaviour for "no pump published - burns the full TERMINAL_INPUT_CHILD_PAUSE_TIMEOUT and fails the handoff. With --test-threads=2 the ordering is nondeterministic. Add a test-visible retract/reset and call it in a guard at the end of each gate test.
| .catalog_error_secret_values | ||
| .iter() | ||
| .chain(request_secrets.iter()) | ||
| .any(|secret| message.contains(secret.as_str())); |
There was a problem hiding this comment.
[INFO] Full leak re-check can drop a body that was redacted correctly
catalog_error_secret_values pushes the active key with no length floor, and every configured header value, while the leak check runs message.contains(secret) against the already-substituted text. A short secret that is a substring of the replacement marker (codewhale_config::persistence::REDACTED) therefore reappears in the output and the whole body is dropped - reproducing #6173's empty "Invalid request (400): " with no diagnostic at all, exactly the case the re-check was meant to make impossible. Checking the pre-replacement body (or substituting with a marker that cannot contain a secret) removes the false positive while keeping the fail-closed contract. Separately, redacting every request query value and every header value over-redacts: values such as alt=json or pageSize=100, and structural header values, get rewritten inside otherwise-useful provider prose.
| return; | ||
| } | ||
| for steer in std::mem::take(&mut app.inflight_steers) { | ||
| app.rejected_steers.push_back(steer.message.display); |
There was a problem hiding this comment.
Before recording the rejection, drop or rewrite the transcript cell this QueuedMessage already owns (the queue-time echo path handled by paint_user_turn_cell), otherwise a dropped steer leaves a cell the engine's record never had - the same defect #6190 removes for the accepted case. Add a test that steers a message which was echoed at queue time and then drops it.
| // A steer the turn never accepted was dropped by the | ||
| // engine. Report it instead of leaving it "sending" | ||
| // (#6190). | ||
| crate::tui::ui::dispatch::settle_unaccepted_steers_at_turn_end(app); |
There was a problem hiding this comment.
Settle in-flight steers on every turn-terminal event (error, abort, cancel, session teardown), not only TurnComplete, and make sure the engine's committing projection is always applied before the terminal event settles the remainder - otherwise a steer either renders as "sending into turn" forever or is falsely reported as not sent.
| impl Drop for ChildTerminalInputPause { | ||
| fn drop(&mut self) { | ||
| if let Some(gate) = self.gate.take() { | ||
| gate.paused_ack.store(false, Ordering::Release); |
There was a problem hiding this comment.
Do not unconditionally clear the shared paused/paused_ack flags on drop. Track the holder (nesting count or an owner token) so an inner guard cannot release a pause held by the pre-existing pause_for_child_terminal at the same call site.
| input.resume_after_child_terminal(); | ||
| } | ||
|
|
||
| /// `CHILD_TERMINAL_GATE` is process-global — one stdin, one pump. Any test |
There was a problem hiding this comment.
Expose a test-visible retract/reset for CHILD_TERMINAL_GATE and call it at the end of both new gate tests (and in the restart test). Leaving a published gate whose worker no longer acknowledges makes any later pause_terminal_input_for_child() in the same process time out and refuse the handoff.
| .catalog_error_secret_values | ||
| .iter() | ||
| .chain(request_secrets.iter()) | ||
| .any(|secret| message.contains(secret.as_str())); |
There was a problem hiding this comment.
Run the leak check against the pre-replacement body (or against a marker that no secret can be a substring of) so a correctly redacted body cannot fail its own re-check and be dropped, which would reproduce the silent empty error from #6173. Consider excluding non-opaque query values and structural header values from the redaction set to avoid mangling the provider's reason.
Hmbown#6239 (Hmbown#6165) added a bounded `thread::sleep` to `tui/ui/terminal_input.rs::pause_for_child_terminal`, which the blocking-calls ratchet has no entry for. The gate is advisory on pull requests and blocking on pushes to main, so the PR was green and main is only green because the budget steps SKIPPED - they are gated on `needs.changes.outputs.heavy == 'true'`, and that merge did not trip it. The violation is real and fires on the next heavy push. Budgeted rather than rewritten, because the site is the case the script's own message names. Its author already wrote the justification at the call site: a bounded retry capped by `TERMINAL_INPUT_CHILD_PAUSE_TIMEOUT`, in a synchronous API whose caller is about to block that very thread on a foreground editor for as long as the user keeps it open. `tokio::time` is not reachable from there and would not change what the thread does. The sibling sleep at :192 is already inside a `thread::Builder::spawn`, so the scanner never counted it. One entry added; the rest of the file is unchanged. check-blocking-calls-budget.py 625 sites across 181 files, within budget Worth a follow-up someone should own: a budget step that skips on a not-heavy-enough diff means a green main is not evidence the ratchet passed. That is how this reached main in the first place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AJENKJ2smviQW4FVGzUTk9
Three of the "user loses something" bugs on the v0.9.14 milestone.
Closes
/hooks edithands the terminal to $EDITOR without pausing the TUI input thread — keystrokes get split between the editor and the composer #6165 —/hooks edithanded the terminal to$EDITORwithout pausing the TUI input thread, so keystrokes split between the editor and the composer/modelsproduced an error that said nothing#6165 — the pause moved, not the call sites
The reporter offered two shapes. This takes the second: the pump pause lives inside
with_suspended_tui, reached through a process-scoped gate on the one pump, so both editor entry points are correct by construction. The alternative — threading&TerminalInputPumpthroughapply_command_result's twelve call sites — would have been correct on the day and quietly wrong the first time someone added a thirteenth.It fails closed: a pump that will not acknowledge means the editor does not run.
ok. 5 passed; 0 failedandok. 12 passed; 0 failed.#6190 — move presentation, not the engine
A steer records an
InflightSteer;apply_engine_session_projectionpromotes it at the matched record index;TurnCompletesettles unaccepted ones intorejected_steers. The engine's ordering is untouched — only where the entry is presented.The existing preview labels ("sending into turn" / "could not send into turn") fit exactly, so this adds no
MessageIdand touches none of the 15 locale files.ok. 30 passed(steer),ok. 845 passed(tui::ui::),ok. 475 passed(tui::app::,tui::widgets::).#6173 — redact, then surface
The failure surfaced an empty error because the body was being dropped wholesale rather than risk leaking a key.
ErrorBodyDisclosure::Guardedinstead strips, from the raw body before truncation: the API key (no length floor), every configured header value,model_bound_secret_values, and the request's query values in both encoded forms — then re-checks and drops everything if any secret survived.The
later_page_..._cursor_or_keycanary is untouched and still passes.ok. 14 passed(catalog_tests),ok. 520 passed(client::).#6207 is deliberately not here
The session-picker refusal was in this batch's scope and is not included. PR #6233 already rewrites the same two hunks of
apply.rsandruntime_store_binding.rs, and its wording is better —codewhale resume <full id>is copy-pasteable where the draft here truncated the id. The draft was dropped rather than landed alongside it. A review note is on #6233: whenvalidate_existing_store()would reject the store,codewhale resume <id>fails too and the new advice becomes a fresh circle; guarding it is ~4 lines at the same site.The adopt/refuse half of #6207 was also examined and deliberately left: the process holds its own store's
RuntimeProcessOwnerLockfor theTaskManager's lifetime, created once atevent_loop.rs:849and held by_task_shutdownfor the whole event loop, withAutomationManagerand the scheduler bound to that instance. Routing the switch throughopen_for_sessionmeans rebuilding the liveTaskManagermid-session — not a bug-fix slice. Worth recording: the launch path skips the guard entirely (apply_loaded_session_with_goalat:786runs beforeTaskManager::startat:849, whiletask_manageris stillNone), so #6207 is an in-session switching problem only.Filed, not fixed
#6235 —
try_open_file_at_line(history.rs:2937) fire-and-forgetspawn()s$EDITORwith no suspension at all. Split out of #6165 rather than buried in it, because it needs a different shape: there is no point at which a pause could end. Read from source; not reproduced on a tty.Evidence
Re-verified independently on this head, not inherited:
Not run locally: the full
codewhale-tui --all-featureslib suite. It wedged holdingruntime-chat.owner.lock(29s CPU over 45 minutes) on an unrelated module and was killed;scripts/dev-test.sh tuiprovisions an isolated build dir and restarts fromproc-macro2, discarding the warm target. The targeted runs above cover every module touched. CI owns the exhaustive run.🤖 Generated with Claude Code