Repository navigation
fix(plugin-browser): finish browser replay follow-ups - #460
Merged
Merged
Conversation
Forget a session after delete or move settles, and stop the tab-list wait when that desktop closes. Attach still has no stable desktop id.
close only schedules the command it wakes, so deleting the retired id before adopt runs lets that command park or replay. The wake carries a retired reason, and the id stays banned for one reconnect grace. Overlapping move and delete extend that deadline.
Leave the command untaken so delete and move reach adopt, and sequence a move and a later delete across the reconnect grace.
Keep the retired-session replay ban and the v2.2.0 browser skill permission.
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.
Issue for this PR
Follow-up to #459.
Type of change
What does this PR do?
Three leftovers from the review of #459, plus a fix for the retired-set race.
Waiting for a new desktop's first tab list races that attachment closing. If the desktop drops before it publishes tabs, the command fails with
browser.disconnectedinstead of sitting for the full 2 second inventory grace.A delete or move does not drop its session id before the woken command runs
adopt.closeonly schedules that command. The wake carries aretiredreason, which the command reads even if it runs after the handler returns, so an untaken command fails with "did not run" instead of parking for the reconnect grace or replaying onto a session that came back. The desktop-facing attach result staysclosed. The id remains retired for onereplayGrace(2 seconds). A later move or delete only extends that deadline, so overlapping events cannot clear each other. When the deadline passes, the id leaves the set.A desktop that attaches again during that ban, and then drops, fails an in-flight command immediately. The ban is still in force, so the drop does not start another 2 second reconnect wait and does not replay the command.
Replay is still session-scoped.
attachacceptssessionID,connectionID, andversion: 4, and a supplieddesktopIDis discarded. The desktop mints a newconnectionIDwithcrypto.randomUUID()on every registration (packages/gui-extensions/src/browser/pane.ts), and nothing persists a desktop identity across reconnects. Pinning replay to the same desktop needs both sides:desktopIDto the attach input and bump the browser RPC version.attach.packages/plugin-browser, plus a desktop test that reconnects with the stored id.That is not started here. A server-only optional field would not be sent by today's desktop, so it would not actually constrain replay.
How did you verify your code works?
bun testandbun typecheckinpackages/plugin-browser(65 tests). The pre-push workspace typecheck passed.The untaken delete and move tests were run against
4e38420944and fail there (retiredis already false). They pass on this branch: the command reports "did not run" with no clock advance,waitingis 0, a re-attach receives no command, and a drop of that re-attach also fails immediately.Overlapping events are sequenced: move, 1 second, delete, 1 second and the session is still retired, 1 second more and it is not.
Screenshots / recordings
Not a UI change.
Checklist