Skip to content

Cancel obsolete command discovery without crashing the UI - #1244

Merged
trissim merged 2 commits into
mainfrom
fix/command-discovery-navigation
Oct 25, 2026
Merged

trissim merged 2 commits into
mainfrom
fix/command-discovery-navigation

Conversation

@trissim

@trissim trissim commented Oct 25, 2026 •

Copy link
Copy Markdown

Fixes #1243.

Defect and owner

Navigation can change while command discovery awaits preparation. Conversation.command_target_context and Conversation.read_command_catalog currently report that normal lifetime change as ValueError. Textual.Worker treats it as ERROR and terminates the application. ContextualCommand.apply has the sibling target-revalidation path.

Use the existing cancellation owner: CommandDiscoveryCancelled derives from asyncio.CancelledError, which Textual.Worker._run already records as CANCELLED rather than WorkerFailed. Both discovery guards and the target revalidation use it. CommandCatalog.execute catches only that specific cancellation at the submission boundary, tells the user the command was not run, and marks it handled so it cannot fall through into an agent prompt. Genuine task cancellation and unrelated errors are not swallowed. No alternate cache, duplicate navigation state, broad catch, retries or retargeted command execution.

Source evidence

Fresh main at source trace: 18c9906. Existing NRA ModuleSyntaxIndex indexed 1,042 Toad/Core/Textual modules and 7,733 class declarations with zero parse omissions. Traced declarations, imports, guards, writes and all syntactic consumers of command_target_context/read_command_catalog/update_slash_commands/CommandCatalog/TargetContext. Consumers include native and Comms chat submissions, slash completion, target suggestions and sidebar/menu discovery. Dynamic Python method binding is not statically proven. Initial NRA invocation under the installed runtime failed on metaclass-registry incompatibility; the completed pass used NRA's own existing environment, not a copied scanner.

Before: both conversation invalidation guards and ContextualCommand target mismatch raise ordinary ValueError; background discovery and foreground command submission disagree about cancellation. After: one nominal cancellation meaning goes to the inherited worker cancellation owner, with foreground handling only at the shared command execution boundary. The unsafe ValueError paths are deleted in place.

Checks and live path

  • Changed Python files parse; git diff --check passes.
  • No automated-test result claimed. An attempted focused pytest invocation exited before collection because the installed runtime has no pytest; after reading Toad's CLAUDE.md instruction, all test-only edits were removed.
  • Actual Toad/ACP launched in plain st on isolated Xvfb :94 using current configured saved sessions and this source. Saved history and ongoing native responses loaded; 24 physical tab-bar clicks and slash completion were exercised without a fatal frontend error.
  • The reviewed three-file diff is now locally installed in runtime core-66307e785-lock-ab5433750d9946a2. Every installed preimage exactly matched origin/main; originals and before/after SHA256 hashes are retained in the owned installed-hotfix.json receipt.
  • Started a second real st via the normal stack/bin/toad-comms launcher (which removes PYTHONPATH): saved history loaded, ongoing native output remained visible, another 24 tab-bar clicks including returns/closures were exercised, and slash input/target completion appeared. No agent was restarted and no provider input was submitted. Physical events are recorded by the existing XvfbDisplay input owner, with frontend PID/start identity and window checks. This does not claim a deterministically forced occurrence of both invalidation branches or a new full runtime/DM qualification.

Delivery

Tristan explicitly requested this fatal blocker be hotfixed locally for immediate use. Local installed files were changed only from the reviewed three-file source diff, with original preimages retained; no agent restart, root migration or UNKNOWN input replay. The branch remains the durable source repair and lands through the merge queue.

Assumed:

  • The existing asyncio cancellation contract of Textual.Worker is intentional (confirmed in its installed _run implementation).
  • Returning True from CommandCatalog.execute means input handled, not command success (confirmed at both ConversationSubmissions and CommsChat callers).
  • New Toad launches consume a local installed hotfix; an already-running frontend retains its previously loaded code.

@trissim
trissim marked this pull request as ready for review October 25, 2026 08:08
Copilot AI balanced review requested due to automatic review settings October 25, 2026 08:08
@trissim
trissim enabled auto-merge October 25, 2026 08:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
trissim merged commit 3cc8e5d into main Oct 25, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tab or target change during slash-command discovery crashes Toad with WorkerFailed

2 participants