Skip to content

refactor(coding-agent): delete unreachable empty-selector auto-cancel timers - #1746

Closed
snimu wants to merge 6 commits into
snimu/rpc-start-readinessfrom
snimu/remove-empty-selector-timers
Closed

refactor(coding-agent): delete unreachable empty-selector auto-cancel timers#1746
snimu wants to merge 6 commits into
snimu/rpc-start-readinessfrom
snimu/remove-empty-selector-timers

Conversation

@snimu

@snimu snimu commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What was wrong

Both TUI selectors (fork-message and session-tree) carried a 100ms auto-cancel timer for the empty-collection case — but their only construction sites already return early with a user-facing notice when the collection is empty. Unreachable defensive timers, plus the stale-callback hazard they imply (audit: timeouts.md finding 8).

The fix

Pure deletion of both blocks (+1/−8). No replacement assertions — the callers own the precondition.

How it's verified

Reviewer independently enumerated all construction sites (production + tests) confirming the empty case cannot reach the components, and that the deleted blocks held no listener registrations — nothing leaks. tsgo/biome clean, tree-selector 18/18, full CI-style suite failing set matches stack base. Two-model implement/review loop, approved first pass.

Stacked on #1744 (test the whole stack at the leaf; merge base-first). This completes the deletion-only wave of the audit stack.

Note: intentionally no Linear ticket for this cleanup stack, so that check stays red.


Note

Low Risk
Dead-code removal with no change to reachable paths; callers already enforce non-empty collections before opening selectors.

Overview
Removes 100ms setTimeout auto-onCancel logic from TreeSelectorComponent and UserMessageSelectorComponent constructors when given an empty tree or message list.

Those branches were unreachable in production: showTreeSelector and showUserMessageSelector in interactive mode already bail out with a status message before constructing the components. This is deletion-only cleanup from the timeouts audit (stale callback hazard), with no new guards added inside the components.

A changelog note documents the removal of delayed cancellation from empty interactive selectors.

Reviewed by Cursor Bugbot for commit 2033172. Bugbot is set up for automated code reviews on this repo. Configure here.

Linear ticket: ENG-5663
(ticket linked above)

Note

Remove unreachable empty-selector auto-cancel timers in TreeSelectorComponent and UserMessageSelectorComponent

  • Removes the setTimeout-based auto-cancellation from the constructors of tree-selector.ts and user-message-selector.ts
  • Both selectors previously auto-invoked onCancel after 100ms when given an empty tree or messages array; this logic was unreachable and is now deleted
  • Behavioral Change: empty selectors no longer auto-cancel; they remain active until explicitly canceled by the user or caller

Macroscope summarized 2033172.

@snimu

snimu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1828: GitHub's stack lock prevents retargeting stacked PRs, so this PR was recreated as a plain PR against main (same branch, same content, all review threads here are resolved). Please review/merge #1828 instead.

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.

1 participant