Skip to content

fix(core): forget click targets on an interface reload - #1125

Merged
LeTuR merged 1 commit into
mainfrom
fix/stale-click-targets-1118
Sep 13, 2026
Merged

LeTuR merged 1 commit into
mainfrom
fix/stale-click-targets-1118

Conversation

@LeTuR

@LeTuR LeTuR commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Intent

Resolve thurbox issue #1118 (Closes #1118): a press that arrives after an interface reload but before the next paint must never reach a plugin that did not paint the node under the pointer, for left AND right presses alike (the bug predates #1116 and hits both). Root cause per the issue: App::click_targets records plugin INDICES from the last paint; reload_interface (src/coordinator/interface.rs) rebuilds the plugin vector and reset grabbed but not click_targets, so target_at in src/coordinator/mouse.rs resolved a press to whichever plugin moved into the stale index, carrying the old node's id. The issue's own suggested fix was followed: clear click_targets next to grabbed = None in reload_interface, with the comment there updated to cover it — that is why the change is in interface.rs rather than mouse.rs, which the task brief had guessed. Constraints from the requester: write the failing end-to-end test FIRST and watch it fail for the issue's reason (done: tests/tui_e2e.rs a_press_right_after_a_reload_never_reaches_a_pane_that_did_not_paint_it runs the real binary on a pty with three stacked panes gone/aim/near, deletes gone, then sends F10 + a right press + a left press on aim as ONE write so drain_input handles all three before the next paint; before the fix near painted tb-near-R:aimL:aim, after it near hears nothing). No scope beyond the issue; do not widen any public API. Known, deliberately out of scope: pointer_grab also stores a plugin index and is not cleared on reload (reported separately, not fixed here). Known pre-existing flake unrelated to this change: tui_e2e the_wheel_scrolls_the_agents_output_back times out intermittently and fails on unmodified main 116fdb7 too (1 of 3 runs). PR description must follow the Intent/What Changed/Risk Assessment/Testing/Pipeline shape, be concise with no praise, and end with the plain last line '— LeTuR's agent'. Squash-merge title should be a conventional commit, e.g. fix(core): forget click targets on an interface reload.

What Changed

  • reload_interface (src/coordinator/interface.rs) now clears click_targets alongside resetting grabbed, so a stale plugin index from before the reload can no longer be resolved against the rebuilt plugin vector.
  • Updates the surrounding comment to explain that both grabbed and click_targets hold positions from the last paint, and that a reload between a paint and an input can leave them pointing at a different plugin that never painted the node under the pointer (Stale click targets after an interface reload route a press to the wrong plugin #1118).
  • Adds an end-to-end test (tests/tui_e2e.rs) that runs the real binary on a pty with three stacked panes, deletes one, then sends a reload followed by a right press and a left press on another pane as a single input batch, asserting the press never reaches a pane that did not paint that node.

Risk Assessment

✅ Low: The fix is a minimal, correctly-targeted one-line addition (clearing click_targets alongside grabbed in reload_interface) that closes the exact stale-index race described in issue #1118 for both left and right presses, backed by a real pty-driven e2e test that reproduces the bug via a single batched write (F10 + right + left press) and verifies both the fixed path (target_at returns None until the next paint) and the previously-buggy path; the deliberately out-of-scope pointer_grab index is untouched as intended, and the updated comment accurately reflects the new behavior.

Testing

Baseline cargo nextest run --all already passed. Beyond that, I ran the specific new e2e test end-to-end on a real pty: on the fixed commit it passes, and after temporarily reverting only the interface.rs fix (keeping the test), it fails for exactly the reason issue #1118 describes — a stale plugin index causes the pane that moved into a deleted pane's old slot to receive a press (both right-click and left-click) meant for a different pane it never painted. Restored the fix afterward and confirmed the worktree is clean. The related #1116 right-button routing test still passes, showing no regression. This directly demonstrates the intended end-user behavior (mouse press routing after a live interface reload) rather than relying on unit-test pass/fail alone.

Evidence: tui_e2e regression test failing before fix (base commit 116fdb7)
a_press_right_after_a_reload_never_reaches_a_pane_that_did_not_paint_it panicked:
"a press after a reload reached a pane that did not paint the node" — frame shows tb-near-R:aimL:aim, i.e. the stale pane 'near' received both the right-press and left-press meant for 'aim' after the reload.
Evidence: tui_e2e regression test passing after fix (target commit 17af69b)
test a_press_right_after_a_reload_never_reaches_a_pane_that_did_not_paint_it ... ok
test the_right_button_reaches_on_context_and_the_left_one_still_reaches_on_click ... ok
- Outcome: 🔧 1 issue found → auto-fixed ✅ across 2 runs (11m39s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

🔧 **Test** - 1 issue found → auto-fixed ✅
  • 🚨 tests failed with exit code 100
  • cargo nextest run --all

🔧 Fix: Confirm click_targets fix passes; other failures were disk-quota flakes
✅ Re-checked - no issues remain.

  • cargo nextest run --all
  • cargo nextest run --test tui_e2e a_press_right_after_a_reload_never_reaches_a_pane_that_did_not_paint_it (target commit 17af69b) — passes
  • Manually reverted src/coordinator/interface.rs to the pre-fix (base commit 116fdb7f0) version while keeping the new test, reran the same test — fails with the exact reported symptom (stale pane 'near' receives 'aim's right+left press: tb-near-R:aimL:aim), then restored the fixed file and confirmed git status is clean
  • cargo nextest run --test tui_e2e -E 'test(a_press_right_after_a_reload) + test(the_right_button_reaches_on_context)' (target commit) — both pass, confirming no regression in the adjacent #1116 right-button routing test
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

— LeTuR's agent

A reload rebuilds the plugin vector that click targets index into, so a
press read before the next paint reached whichever plugin now sat at the
old index, carrying the node id another plugin painted. Clear the targets
next to grabbed, for left and right presses alike.

Closes #1118

Claude-Session: https://claude.ai/code/session_01EVPegiKH7guuBKUerQriK4
@sonarqubecloud

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents stale click-target indices from being reused after an interface reload and adds an end-to-end regression test for both right and left presses.

  • Clears click_targets when rebuilding the plugin vector.
  • Updates the lifecycle comment to document why pre-reload targets are invalid.
  • Verifies through the PTY-driven test that batched presses cannot reach a plugin that did not paint the targeted node.

Confidence Score: 5/5

The PR appears safe to merge; the state reset is narrowly scoped to invalid paint-derived targets and the regression test covers the stale-index routing failure.

Reloading now invalidates click targets at the same point the indexed plugin vector is rebuilt, while the next paint restores valid targets; no actionable correctness, security, or repository-rule issues remain.

Important Files Changed

Filename Overview
src/coordinator/interface.rs Clears paint-derived click targets during interface reload so stale plugin indices cannot route presses into the rebuilt plugin vector.
tests/tui_e2e.rs Adds a PTY-driven regression test covering batched reload, right-click, and left-click input before repaint.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Paint interface] --> B[Record click targets with plugin indices]
    B --> C[Reload interface and rebuild plugin vector]
    C --> D[Clear grabbed and click targets]
    D --> E{Next paint completed?}
    E -- No --> F[Press has no valid target and is ignored]
    E -- Yes --> G[Record targets for rebuilt plugins]
    G --> H[Route subsequent presses using fresh targets]
Loading

Reviews (1): Last reviewed commit: "fix(core): forget click targets on an in..." | Re-trigger Greptile

@LeTuR
LeTuR merged commit b0d1101 into main Sep 13, 2026
24 checks passed
@LeTuR
LeTuR deleted the fix/stale-click-targets-1118 branch September 13, 2026 06:42
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.

Stale click targets after an interface reload route a press to the wrong plugin

1 participant