Repository navigation
Withdraw reader keys before tab eviction and persist every shown error - #1323
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
trissim
enabled auto-merge
October 29, 2026 07:35
trissim
disabled auto-merge
October 29, 2026 07:42
trissim
enabled auto-merge
October 29, 2026 08:19
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.
Summary
4 commits; 14 production files; +177/-105 lines. ReadyContent construction sites: 3 → 1. Strict/optional Window lookup: 2 paths → 1. MainScreen's optional Conversation probes: 4 → 0. Field-validation logging: one error file per refused edit → zero file writes.
Repairs the missing-Window crash Tristan observed while switching tabs/scrolling and preserves actual exception evidence after an error toast disappears. The first head was sent back because late preparation could restore Ready after eviction, and expected field refusals were being sent to the new log path. This head includes those corrections on the same PR.
1. Content state owns mounted reader availability and completion
Before
The initial mount was already awaited. The competing lifetime decisions were removal without state withdrawal, two false Ready writers, and completion through whichever content happened to be current after awaits. A tab switch could evict a still-opening tree while its original preparation was paused, then that stale operation would publish Ready over nothing.
After
There is one Ready constructor after successful preparation. Superseded completion cannot publish Ready or Failed onto a replacement, and cannot complete the replacement's waiter. Closing an opening tab releases the existing waiter without claiming mounted readiness. Closing switches state before sidebar teardown yields.
ReadyContentowns keyboard scopes, painted-cursor reads, prompt focus and sidebar return focus. Other states have no mounted-reader effects. All four optional Conversation probes in MainScreen and the duplicate optional Window lookup are deleted. The existing nested read acknowledgment/write operations are retained; no extra read-mark store or copied transcript state.Pattern IDEN-3: mounted-reader availability was restated at removal, completion and DOM probes. Derive it through ScreenContent at those consumers, with identity-checked completion and pre-removal withdrawal. No
is_mountedprobe or exception suppression added to keyboard scopes.2. The existing holder passes its screen through eviction
OperationalSessionPresentation.evict(screen)andremove_native_tree(screen)use the MainScreen already held by their callers.NativeSessionSurfaceretains the owner/screen pairs captured from_presentations, derives its budget groups from them, and passes the associated screen when evicting. Dispose passes its screen to the release callback.SessionThreadSidebar has the same
evict(screen)contract. MainScreen and sidebar retirement pass their existing screen through; panel capture no longer re-finds it from a weak reference. The local MainScreen import and Conversation ancestor query in removal are deleted.Readiness withdrawal remains inside actual removal, after capture. P928's pending-Neovim-input refusal must return before capture/removal; that refused eviction therefore retains the actual view, engine and key availability. No competing engine/readiness writer is introduced.
3. Real exceptions are saved; expected refusals are not crash files
The existing diagnostic writer
paths.save_tracebackbecomespaths.save_error(error: BaseException, prefix). It writes only exceptions, retaining complete traceback/chained causes, exclusive creation, mode 0600 and explicit UTF-8. The string branch is deleted from the writer.ThemedApp.show_errorsaves a real exception before showing its short literal toast and saved path.ToadApp._handle_exceptionpasses the exception intact. Store/navigation caught errors retain their objects. The CLI terminal-failure caller shares the same writer, and the separate history save is removed.Log location:
$XDG_STATE_HOME/toad/logs/<title>_<timestamp>.txt, default~/.local/state/toad/logs/. Existing directory/filename ownership is reused; no second logger, daemon or log store. A write failure is not swallowed by show_error.Core's public
ResultShower.show_errorcontract still acceptsException | str(agent_comms/cli_result.py:21). ThemedApp preserves that contract and decodes text at that one boundary into a notification without persistence. Paths receives exceptions only. No narrower incompatible override or compatibility alias was introduced. The four owned text-only callers now use native notifications directly: missing agent, unavailable theme, provider-login exit and the already-recorded agent-failure message.Field refusal paint
ValueEditor no longer calls show_error on invalid edits. It owns transient feedback through Textual's existing
var("", init=False)and emits a Feedback message only when that value changes; ValueView paints it in one inline FieldHelp-derived error-colour display. An accepted edit clears an existing refusal, but a clean edit on a clean field emits nothing. Identical repeated refusal text also emits nothing. The declaration requests no initial watcher, editor repaint or layout; no extra dirty flag or duplicate equality guard is added. This applies to the whole editor family, including multiline/select/checkbox controls, rather than fixing only numeric input. No toast, exception formatting, file creation or filesystem write lies on this refusal path.pathsis imported at module top. No new string-to-exception wrapper manufactures a fake traceback.Source evidence
6d83f98acc0db434aa48365c518a86c0518418cf; Textual79a8a6d69eed1d3300b54b12c490bc7d3753ee9c./home/ts/.cache/agent-scratch/toad-keymap/tab-error-owner-map.jsonandtab-error-rework-owner-map.json.Format.STRINGto respect their TYPE_CHECKING forward references. The first inspection used VALUE evaluation and hit the existing forward-reference NameError; that failed diagnostic is retained, not reported as a pass.git diff --checkpasses. Rework source log:/home/ts/.cache/agent-scratch/toad-keymap/tab-error-rework-source-strings.log. Final feedback source compile/import/declaration inspection:tab-error-feedback-source.log; BoolEditor, LineEditor and MultilineEditor inherit the same feedback declaration, with init/always_update/layout/repaint all false. No widgets instantiated. The retained Package map plus targeted AST/semantic read covers feedback producers, its sole watcher/constructor and ValueView consumer. Textual Reactive._set owns changed-value dispatch; no custom feedback equality mechanism.Unrun controls and Assumed
No author automated tests, pilot, fake app, app launch or provider input: this checkout's CLAUDE.md prohibits them. Source checks are not a live fix claim. Root owns the combined real-app run and installation.
105 lines deleted overall includes moved read-mark code and the replaced transition/probe/logging paths. This is not 103 distinct obligations discharged. The implementation remains under review, not merged or installed, and auto-merge was disabled for rework.