Repository navigation
Stop the first terminal attach from waiting on a protocol ping - #525
Merged
powerfooI merged 2 commits intoOct 31, 2026
Merged
Conversation
The clipboard relay check pinged Herdr for its protocol without the bridge's cache. Over a remote link that ping takes longer than the 500 ms relay wait, so the first attach of every connection stalled for the full wait before learning that protocol 22 skips the relay. Use the protocol the attach already resolved, and let the direct terminal streams reuse it too.
The "collapse and outside drag" capsule test expected the Send button to re-enable right after dispatching compositionend. TerminalComposer deliberately releases its IME lock one setTimeout(0) tick later, and the test only passed when that timer happened to fire before act() resolved. React's async act() resolves after a setImmediate flush, and in Bun a setImmediate runs before a 0 ms timer unless about a millisecond has already passed. The outcome therefore depended on machine speed and load: the test failed every time on a fast macOS machine and intermittently in CI. Install Bun's fake timers around compositionend, check that the lock is still held before the tick, then advance timers by 0 inside act() and restore real timers in finally.
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.
The first terminal attach on every connection could stall for the full 500 ms clipboard relay wait.
The clipboard relay check asked Herdr for its protocol through
args.herdrProtocol(), bypassing the bridge's cachedbridgeProtocol(). Over a remote link that ping takes longer than the relay wait (CLIPBOARD_RELAY_READY_WAIT_MS), so the first attach waited the whole 500 ms before learning that protocol 22 skips the relay anyway.Changes
bridgeProtocol(), which the attach has already resolved.ThinClientand the legacy direct terminal streams getbridgeProtocoltoo, so they no longer send a control call each to learn the protocol.Verification
bun test server/src/bridge/terminal-bridge.test.ts server/src/bridge/endpoint-terminal-session.test.ts: 167 pass, 0 fail. New test: with fake timers frozen and a protocol probe that only answers once, the first protocol 22 attach completes without a second probe. It fails onmainand passes with this change.bun run format:checkon the changed files: clean.bun run lint: clean apart from the existingTerminalView.tsxexhaustive-deps warning.bun run typecheck: passes, including the web build and asset budget check (initial JS 805.0/805 KiB).bun run test:quickwith an isolatedHOME, onmainplus this change and the other small fixes I am sending alongside: 3965 pass, 4 skip, 2 fail. Both failures (useMobileControlsDrag.keyboard.test.tsand the SSH paths test) fail the same way onmainon macOS.Not measured on this branch: the attach latency over a real remote link.
Compatibility
No API or protocol changes. The protocol is resolved once per bridge, as navigation mode and
tab.movesupport already assume.