fix(transfer): never leave a moved tab open in both windows - #366
Merged
Conversation
A tab transfer may only end two ways: the source still has the tab, or the destination does. Four paths produced a third state -- present in both, each copy with its own auto-save timer writing over the other, last writer wins. **The source's timeout ignored what the destination was doing.** After 15s it cancelled unconditionally, but 15s is a guess about a window that may simply be slow to render a large document. `cancel_detached_tab` now reports whether the payload was already claimed, and the source keeps waiting for `tab-transfer-claimed` when it was. A claimed transfer is correspondingly immune to the source's cancel, so the race cannot go the other way either. **Nothing released a claim that could not be completed.** Since the source can no longer cancel a claimed transfer, the destination is the only party able to end one -- and it never did. An invalid payload, a refusing destination and a thrown render now all hand the claim back so the source recovers its tab. **A failed render left the tab inserted.** The tab has to exist before it can be rendered, so the destination owns the document while the source still shows it. That window is now closed by undoing the insert on any failure. **The menu could stage the same tab twice.** The entry becomes clickable again long before the transfer resolves; a second click staged a second payload, two windows each claimed one and built the tab, and the source removed it once. In-flight transfers are now tracked per tab. The broker underneath is hardened to match: tokens were sequential (`t1`.. `t16`) and every command was unauthenticated, so any webview could enumerate them to read the document text of a drag in progress, or destroy another window's transfer. Tokens are now unguessable and each operation is bound to the window entitled to perform it -- the destination is `window-<token>`, cancellation is the source or that destination and nobody else. Eviction under pressure drops the oldest *unclaimed* entry rather than whatever is oldest, so a transfer mid-handoff is never discarded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 3, 2026
PathGao
added a commit
that referenced
this pull request
Aug 4, 2026
…453) #366 gated claim/complete/cancel on the caller being window-<token>. That label exists only for a window create_transfer_window just built, so offer_tab_to_window's destination -- an already-open window labelled main or window-<some other token> -- was refused 100% of the time. #452 fixed it by recording the target on the pending entry and accepting either label. It reached master, not a release. Nothing in the suite failed. The Rust test asserted the predicate behaves as written, and it did: the gate was self-consistently wrong. The script test asserted that the invoke('offer_tab_to_window', ...) call appears in the source, and it still appeared. Both confirmed an implementation exists and is internally consistent, which is precisely what a reachable path becoming unreachable leaves undisturbed. Four tests now drive the broker from stage to complete: * a transfer to an existing window -- stage from main, record the target the way offer_tab_to_window does, claim as that window, complete. This is the path that was dead. * a transfer to a new window -- claim as window-<token> with no target recorded, so the fix cannot trade one path for the other. * a bystander window refused at claim, complete and cancel, with the transfer left intact for its real destination afterwards. * the recorded target releasing its own claim, the only rollback a claimed transfer has: the source's timeout is deliberately inert once a claim exists, so a refusal here would strand the tab in both windows. The authorisation decision moves out of the three #[tauri::command] bodies onto the broker as claim_as/complete_as/cancel_as, which take the caller's label as &str; each command is now one line passing window.label(). A unit test cannot build a tauri::Window, so the gate was otherwise reachable only through the predicate it was written against. Same peek, same order, same error strings. Reverting the recorded-target arm of is_destination_authority and restoring the two predicate tests to their #366 wording leaves the suite at 3 failed / 151 passed -- all three of them these tests. Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
A tab transfer may only end two ways: the source still has the tab, or the destination does. Four paths produced a third state — present in both, each copy with its own auto-save timer writing over the other, last writer wins.
The source's timeout ignored what the destination was doing. After 15 s it cancelled unconditionally, but 15 s is a guess about a window that may simply be slow to render a large document.
cancel_detached_tabnow reports whether the payload was already claimed, and the source keeps waiting fortab-transfer-claimedwhen it was. A claimed transfer is correspondingly immune to the source's cancel, so the race cannot go the other way either.Nothing released a claim that could not be completed. Since the source can no longer cancel a claimed transfer, the destination is the only party able to end one — and it never did. An invalid payload, a refusing destination and a thrown render now all hand the claim back so the source recovers its tab.
A failed render left the tab inserted. The tab has to exist before it can be rendered, so the destination owns the document while the source still shows it. That window is closed by undoing the insert on any failure.
The menu could stage the same tab twice. The entry becomes clickable again long before the transfer resolves; a second click staged a second payload, two windows each claimed one and built the tab, and the source removed it once. In-flight transfers are now tracked per tab.
Broker hardening
Tokens were sequential —
t1throught16— and every command was unauthenticated. Any webview could enumerate them:Tokens are now unguessable, and each operation is bound to the window entitled to perform it: the destination is
window-<token>, cancellation is the source or that destination and nobody else. Eviction under pressure drops the oldest unclaimed entry rather than whatever is oldest, so a transfer mid-handoff is never discarded — a test caught the naive version evicting the transfer that had just been staged.Validation
npm run check— 0 errors, 0 warningsnpm test— 170/170, including a newscripts/tabTransferHandoff.test.tscovering each of the four paths abovecargo test— 40/40, covering repeat claims, claims from the wrong window, cancel authority, claim-then-complete, and eviction with everything claimed