fix(preview): renewing a remote viewer ignores the destination ticket expiry #588

Activity

1. lukemaj commented on Oct 20, 2026

   

   lukemaj

   on Oct 20, 2026

   ContributorAuthor

   More actions

   What: Reproduced #588 and implemented the server expiry cap with focused regression coverage.
   Why: Renewal returned the source expiry even when the freshly renewed destination ticket expired sooner.
   So what: The fix will be delivered in a PR to main; merging remains a human decision.

   Findings

   • Base: f5f86046860e6dcb4d34179c5b2197bd6e15959f, current origin/main after fetch. PR feat(peer): ship reviewed cross-machine parity work (#379) #584 is merged. Exclusive workspace: /home/luke/.t3/worktrees/chromeria/fix-588-viewer-renew-expiry, branch fix/588-viewer-renew-expiry.
   • Upstream checked: pingdotgg/t3code main 8ecb607de44ec3cef2854bf0ea7ea5c1aed01d08; its ServerBrowserStream.ts has no PreviewViewer/renewal/expiry logic. Open PR and all-state Issue searches for viewer expiry and "preview viewer" found no matching fix. PR #19456 concerns viewport/input handling, not viewer ticket renewal.
   • Baseline command: node_modules/.bin/vp test run apps/server/src/preview/ServerBrowserStream.test.ts -t 'caps remote viewer renewal'. Destination-first regression failed with expected 2036-04-07T00:04:00.000Z, actual 2036-04-07T00:05:00.000Z; source-first case passed.
   • Client inspection: shared state/deviceHubAccess.ts preserves renewed expiresAt as both viewer metadata and epoch milliseconds. Web/desktop and mobile use that shared access path. Browser stream consumers renew upload/download admission and recover on socket closure/refusal; they do not currently schedule browser reconnection against expiresAt. This PR preserves those consumers and corrects the response, without adding a new reconnect timer or claiming timed reconnection proof.
   • Ticket lifetimes, admission checks, contracts, and user documentation stay unchanged. This is a direct existing-component fix, contributing through spec: native cross-machine control across linked environments #379/feat(peer): view and drive remote browsers and devices from a source-only client #383 to remote browser operation for the Objective.

   Elon record

   Requirements and who asked: User requested #588: cap renewed viewer access at the earlier source/destination expiry, failing-before proof, client compatibility, and a PR to main without merging.
   Deleted: Ticket lifetime redesign, new reconnect scheduling, new services, Spec, version bump, dev servers, browsers, and repo-wide checks.
   Bottleneck: The incorrect renewal response in the existing PreviewViewers service; no additional component is needed.
   Checked myself: Read #588 and the issue's cited issuance/renewal paths, reproduced the destination-first failure, checked upstream and traced the shared/web/mobile client consumers.
2. lukemaj commented on Oct 20, 2026

   

   lukemaj

   on Oct 20, 2026

   ContributorAuthor

   More actions

   What: PR #596 fixes server renewal to return the earlier source/destination ticket expiry and verifies that shared client access preserves the returned expiry.
   Why: Renewal previously reported only the source expiry, overstating the destination ticket's remaining lifetime.
   So what: Client expiry-driven reconnection remains open under #588; #596 now uses Refs #588 and will not automatically close this Issue.

   The remaining client behavior is in packages/client-runtime/src/preview/serverBrowserStream.ts:357-379: reconnection is scheduled from the WebSocket close handler, with the retry timer at line 379, rather than from the earlier ticket expiry. Renewed expiry metadata is returned by packages/client-runtime/src/state/deviceHubAccess.ts:154-163, but that path does not schedule a reconnect.

   The second acceptance criterion, reconnecting at the earlier expiry instead of remaining on a dead relay, is not implemented or verified by #596. The previous closing reference overstated the delivered scope and is corrected.
3. lukemaj commented on Oct 20, 2026

   

   lukemaj

   on Oct 20, 2026

   ContributorAuthor

   More actions

   What: The remaining #588 regression is reproduced in shared client code, and an expiry timer now triggers the existing access refresh path.
   Why: The stream client waited for WebSocket closure even when an earlier viewer-ticket expiry had already invalidated the relay.
   So what: Focused proof and independent review will precede a PR to main; no merge or live run is authorized.

   Findings and scope

   • Exclusive task worktree: /home/luke/.t3/worktrees/chromeria/fix-588-viewer-expiry-reconnect, branch fix/588-viewer-expiry-reconnect; clean starting HEAD and fetched origin/main: c6f94f62b8ecb3d0b7a9cd141535e4e0248c2069. Existing fresh task workspace is reused. PR fix(preview): honour destination expiry when renewing viewers #596 is merged as ddaa67fa514846d84a47966ef3a000a466f6921c.
   • Read fix(preview): renewing a remote viewer ignores the destination ticket expiry #588 body and all comments. The fix belongs in packages/client-runtime/src/preview/serverBrowserStream.ts. No Spec or version bump.
   • Upstream checked: pingdotgg/t3code main 7e289ae1f7 has no viewer expiry or renewal timer in its shared browser stream client. Open PR and all-state Issue searches for viewer expiry and "preview viewer" found no applicable fix. PR #19456 is input/viewport work; the Issue search matches concern document viewers rather than viewer-ticket renewal.
   • Web and desktop browser surfaces, floating picture-in-picture, and mobile's browser WebView use the shared stream client and already refresh access through onUnauthorized. Mobile serializes access through JSON, so a direct function-based renewal inside the stream client would miss that surface. Expiry triggers that existing callback and retires the socket and pending retry; it does not alter legacy upgrade-ticket lifetimes.
   • Baseline: node_modules/.bin/vp test run packages/client-runtime/src/preview/serverBrowserStream.test.ts -t 'earlier .* expiry' failed both source-first and destination-first cases: expected refresh once at the 60-second effective expiry, got zero calls. Fake timers are used, with no sleeps or polling.
   • Contributes to the Objective through spec: native cross-machine control across linked environments #379/feat(peer): view and drive remote browsers and devices from a source-only client #383 remote browser operation. No new project concepts, contracts, provider paths, or user documentation changes.

   Elon record

   Requirements and who asked: User requests the remaining #588 client acceptance: reconnect/refresh at the earlier source/destination viewer expiry, failing-before proof, shared client coverage, one independent exact-head review and a PR to main without merging.
   Deleted: New renewal service, new callback API, per-client timers, ticket lifetime redesign, Spec, version bumps, dev servers, browsers, devices and repo-wide checks.
   Bottleneck: The existing shared stream client never schedules against access.expiresAt; add one viewer-only deadline timer and reuse each surface's existing access-refresh owner.
   Checked myself: Read the Issue and all comments, fetched main, searched upstream, traced shared access and web/mobile/picture-in-picture consumers, and observed both baseline regression failures.
4. lukemaj commented on Oct 20, 2026

   

   lukemaj

   on Oct 20, 2026

   Last edited by lukemaj

   ContributorAuthor

   More actions

   What: PR #602 implements viewer-expiry reconnection with a fresh mobile lease while retaining the existing picture-in-picture document/player.
   Why: Socket-only recovery missed the ticket deadline; same-viewer renewal after closing its last socket also violates the server's active-transport requirement.
   So what: Corrected head 7af482c8578511642d15bd57c9e650f10deaf8ac is pushed and independently reviewed with no blocking findings; the PR remains unmerged and awaits human approval.

   Current handoff

   • PR: fix(preview): reconnect viewers when their tickets expire #602, main base c6f94f62b8ecb3d0b7a9cd141535e4e0248c2069.
   • Exclusive workspace /home/luke/.t3/worktrees/chromeria/fix-588-viewer-expiry-reconnect, branch fix/588-viewer-expiry-reconnect; clean and synchronized with the remote branch. Keep it for this open PR; the eventual merger retires it.
   • Both independent findings accepted and fixed: mobile document remount, renewal after transport retirement. Ordinary mobile access refresh now opens a fresh lease through the existing owner and injects it into the same document, preserving its source, video, painter and capture track. Explicit retry/process recovery can rebuild a failed document. Same-viewer renewal after the socket closes was removed.
   • Original source/destination deadline cases failed before the shared fix. The strengthened native regression fails on the second candidate because it attempts same-viewer renewal after retirement. It now verifies fresh access refresh, stable source/document identity under background PiP and manual retry recovery.
   • node_modules/.bin/vp test run packages/client-runtime/src/preview/serverBrowserStream.test.ts packages/client-runtime/src/state/deviceHubAccess.test.ts apps/mobile/src/features/browser/PreviewStreamWebView.test.tsx apps/mobile/src/features/browser/preview-stream.browser.test.ts apps/mobile/src/features/browser/preview-stream-document.test.ts: 40 passed. The updated native test was rerun after the last state change and all 9 cases passed.
   • node_modules/.bin/vp test run apps/server/src/auth/SessionStore.test.ts -t 'admitted .* transport|admits a fresh preview viewer': 3 passed. The new real-store regression expires and releases the last transport, verifies old renewal fails, then issues and admits/rechecks a fresh lease. Shared access tests exercise actual atom refresh and verify a distinct viewer id with no old viewerId in the new open request. Total focused proof: 43 cases.
   • Fake timers/TestClock only, no sleeps/polling. Client-runtime and mobile package typechecks pass. Latest targeted lint is clean; the original shared test file retains its pre-existing inline-schema warning. Formatting/diff checks and committed-head fork check pass (59 features).
   • Web, desktop, web PiP and mobile share the expiry timer. Server implementation, wire contracts, provider adapters, local upgrade-only sessions and user workflows stay unchanged. No marketing/SEO impact.
   • No screenshots or live run were produced. No dev servers, browsers, devices, repo-wide checks, version bumps, deployments or merge actions.
   • Current exact-head independent review passed; the reviewer posted review/independent=success as lukemaj on 7af482c8578511642d15bd57c9e650f10deaf8ac, independently verified from GitHub's newest commit statuses. Review is distinct from CI and human merge approval.

   Elon record

   Requirements and who asked: User requests remaining #588 client reconnection acceptance and shared-client proof; independent review identifies preservation of active mobile PiP and the real server admission lifetime as necessary constraints.
   Deleted: Per-client expiry timers, new renewal services, same-viewer renewal after transport retirement, player rebuilds on ordinary refresh, Spec, version changes and live verification.
   Bottleneck: Shared socket expiry must trigger fresh admission; mobile must deliver that new admission to its existing document without destroying PiP.
   Checked myself: Verified both review findings against native and server code, reproduced the native regression, exercised shared access refresh and real SessionStore admission after old-socket release, and inspected focused test/typecheck/lint/fork-check results.
5. lukemaj

   closed this as completedin #602on Oct 20, 2026