feat(mobile): preview localhost dev servers with screenshot annotation - #8020
feat(mobile): preview localhost dev servers with screenshot annotation#8020abcdmku wants to merge 1 commit into
Conversation
A paired mobile client can now open a dev server bound to the environment's loopback through the existing T3 connection (LAN, Tailscale, or T3 Connect), capture the viewport, annotate it with numbered pins/boxes and notes, and add the flattened screenshot plus notes to the thread draft. Server: preview.listLocalServers and preview.createProxyTicket RPCs, an HMAC-signed single-use entry ticket exchanged for an HttpOnly session cookie, and a global middleware that proxies the whole origin (documents, assets, fetch, WebSocket HMR) to the validated loopback port. Requests carrying T3 credentials or reserved T3 paths always bypass the proxy, and T3 cookies are stripped from forwarded headers. Mobile: ThreadPreview screen (picker, WebView, capture via react-native-view-shot, annotation editor) attaching through the existing composer image flow, gated on the new previewProxy capability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| accessibilityLabel="Cancel annotation" | ||
| icon="xmark" | ||
| label="Cancel" | ||
| onPress={props.onCancel} |
There was a problem hiding this comment.
🟡 Medium preview/ThreadPreviewRouteScreen.tsx:642
Cancel and marker editing remain active while handleAddToChat is flattening, so cancelling or changing markers can still append the in-flight screenshot and mismatched annotation text to the draft. The async operation closes over the original markers while captureRef captures the later canvas state; disable cancellation and all editing during isFlattening, or abort and validate the operation before appending.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/preview/ThreadPreviewRouteScreen.tsx around line 642:
`Cancel` and marker editing remain active while `handleAddToChat` is flattening, so cancelling or changing markers can still append the in-flight screenshot and mismatched annotation text to the draft. The async operation closes over the original `markers` while `captureRef` captures the later canvas state; disable cancellation and all editing during `isFlattening`, or abort and validate the operation before appending.
| } | ||
|
|
||
| function fallbackServer(url: string): DiscoveredLocalServer { | ||
| return { host: "localhost", port: 80, url, processName: null, pid: null, terminal: null }; |
There was a problem hiding this comment.
🟡 Medium preview/ThreadPreviewRouteScreen.tsx:341
Closing the preview during an in-flight capture opens the annotation editor with httpBaseUrl as pageUrl, so the comment records the T3 environment URL instead of the captured dev-server page. handleCapture can still set capture after handleCloseSession clears session, causing this fallbackServer path to run; invalidate pending captures or preserve the session's server URL for the capture.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/preview/ThreadPreviewRouteScreen.tsx around line 341:
Closing the preview during an in-flight capture opens the annotation editor with `httpBaseUrl` as `pageUrl`, so the comment records the T3 environment URL instead of the captured dev-server page. `handleCapture` can still set `capture` after `handleCloseSession` clears `session`, causing this `fallbackServer` path to run; invalidate pending captures or preserve the session's server URL for the capture.
| if (proxiedUrl === null) return server.url; | ||
| try { | ||
| const parsed = new URL(proxiedUrl); | ||
| return new URL(`${parsed.pathname}${parsed.search}`, server.url).toString(); |
There was a problem hiding this comment.
🟡 Medium preview/ThreadPreviewRouteScreen.tsx:62
resolveAnnotationPageUrl reports the server root for hash-routed pages such as /#/settings, so annotation text does not identify the page that was captured. The reconstruction includes parsed.pathname and parsed.search but drops parsed.hash; preserve the hash when building the page URL.
| return new URL(`${parsed.pathname}${parsed.search}`, server.url).toString(); | |
| return new URL(`${parsed.pathname}${parsed.search}${parsed.hash}`, server.url).toString(); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/preview/ThreadPreviewRouteScreen.tsx around line 62:
`resolveAnnotationPageUrl` reports the server root for hash-routed pages such as `/#/settings`, so annotation text does not identify the page that was captured. The reconstruction includes `parsed.pathname` and `parsed.search` but drops `parsed.hash`; preserve the hash when building the page URL.
|
|
||
| function resolveEntryRedirectTarget(search: URLSearchParams): string { | ||
| const target = search.get("to"); | ||
| if (!target || !target.startsWith("/") || target.startsWith("//")) return "/"; |
There was a problem hiding this comment.
🟠 High preview/ProxyRoutes.ts:178
resolveEntryRedirectTarget accepts /\evil.example, which the browser normalizes as //evil.example when resolving the Location header, redirecting the authenticated preview entry request off-origin. Parse the target against the request origin and require the resulting URL's origin to match before returning it.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/preview/ProxyRoutes.ts around line 178:
`resolveEntryRedirectTarget` accepts `/\evil.example`, which the browser normalizes as `//evil.example` when resolving the `Location` header, redirecting the authenticated preview entry request off-origin. Parse the target against the request origin and require the resulting URL's origin to match before returning it.
| ); | ||
|
|
||
| const handleCloseSession = useCallback(() => { | ||
| endSessionOnServer(); |
There was a problem hiding this comment.
🟠 High preview/ThreadPreviewRouteScreen.tsx:139
Closing a preview can clear the cookie for a newly opened preview, causing the new WebView session to receive 403. handleCloseSession starts endSessionOnServer() without awaiting it, so the picker immediately allows another server to open before /api/preview/exit completes; serialize the exit request before enabling or processing a new open.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/preview/ThreadPreviewRouteScreen.tsx around line 139:
Closing a preview can clear the cookie for a newly opened preview, causing the new WebView session to receive `403`. `handleCloseSession` starts `endSessionOnServer()` without awaiting it, so the picker immediately allows another server to open before `/api/preview/exit` completes; serialize the exit request before enabling or processing a new open.
| readonly note: string; | ||
| } | ||
|
|
||
| const clampUnit = (value: number) => Math.min(1, Math.max(0, value)); |
There was a problem hiding this comment.
🟡 Medium preview/previewAnnotation.ts:31
Pins placed at the image edges produce clipped numbered badges in the flattened screenshot, so their corresponding notes may be impossible to identify. clampUnit allows coordinates of exactly 0 and 1, while the pin renderer centers the badge there and subtracts roughly 13 px from its position inside an overflow-hidden canvas. Clamp pin coordinates to account for the badge footprint, or reposition edge markers during rendering.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/preview/previewAnnotation.ts around line 31:
Pins placed at the image edges produce clipped numbered badges in the flattened screenshot, so their corresponding notes may be impossible to identify. `clampUnit` allows coordinates of exactly `0` and `1`, while the pin renderer centers the badge there and subtracts roughly 13 px from its position inside an `overflow-hidden` canvas. Clamp pin coordinates to account for the badge footprint, or reposition edge markers during rendering.
| * restart also rotates nothing the ticket could still be replayed against | ||
| * inside its two-minute window beyond what a fresh mint would grant. | ||
| */ | ||
| const consumedEntryTickets = new Map<string, number>(); |
There was a problem hiding this comment.
🟠 High preview/ProxyAccess.ts:73
After a server restart, an already redeemed but unexpired entry ticket is accepted again, violating the ticket’s single-use guarantee. consumedEntryTickets is process-local, while the signing secret persists in ServerSecretStore, so the ticket’s valid signature survives the restart but its ticketId record does not. Persist consumed IDs or rotate the signing key on startup.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/preview/ProxyAccess.ts around line 73:
After a server restart, an already redeemed but unexpired entry ticket is accepted again, violating the ticket’s single-use guarantee. `consumedEntryTickets` is process-local, while the signing secret persists in `ServerSecretStore`, so the ticket’s valid signature survives the restart but its `ticketId` record does not. Persist consumed IDs or rotate the signing key on startup.
There was a problem hiding this comment.
Effect service conventions review of the preview proxy code. Two error-modeling issues (missing cause on the new tagged error) and two dependency/state placement notes. Everything else — namespace imports, PortDiscovery/ServerSecretStore/ServerEnvironment acquired from the environment, Layer.succeed only in tests, no ManagedRuntime/runPromise in server or mobile code — looks consistent with the conventions.
Posted via Macroscope — Effect Service Conventions
| const globalWebSocketConstructor: (typeof Socket.WebSocketConstructor)["Service"] = ( | ||
| url, | ||
| protocols, | ||
| ) => new globalThis.WebSocket(url, protocols); |
There was a problem hiding this comment.
This hand-rolled constructor plus the Effect.provideService(Socket.WebSocketConstructor, ...) at line 312 bakes an imperative runtime API into the request handler instead of taking it from the environment, so the upstream socket cannot be substituted in a test. Effect already ships Socket.layerWebSocketConstructorGlobal (used in apps/mobile/src/lib/runtime.ts and apps/web/src/lib/runtime.ts); providing that layer where the preview routes are composed keeps the dependency visible in the effect's requirements.
Posted via Macroscope — Effect Service Conventions
| const ticketId = yield* crypto.randomUUIDv4.pipe( | ||
| Effect.mapError(() => new PreviewProxyTicketError({ reason: "issuance-failed" })), | ||
| ); | ||
| const now = yield* Clock.currentTimeMillis; | ||
| const expiresAt = now + ENTRY_TICKET_TTL_MS; | ||
| const secret = yield* loadSigningSecret.pipe( | ||
| Effect.mapError(() => new PreviewProxyTicketError({ reason: "issuance-failed" })), | ||
| ); |
There was a problem hiding this comment.
Both mapError(() => ...) callbacks discard the immediate underlying failure, so a UUID-generation or signing-key failure surfaces only as the generic "issuance-failed" message with no chain. Consider preserving it as cause (paired with adding the optional cause field to PreviewProxyTicketError).
| const ticketId = yield* crypto.randomUUIDv4.pipe( | |
| Effect.mapError(() => new PreviewProxyTicketError({ reason: "issuance-failed" })), | |
| ); | |
| const now = yield* Clock.currentTimeMillis; | |
| const expiresAt = now + ENTRY_TICKET_TTL_MS; | |
| const secret = yield* loadSigningSecret.pipe( | |
| Effect.mapError(() => new PreviewProxyTicketError({ reason: "issuance-failed" })), | |
| ); | |
| const ticketId = yield* crypto.randomUUIDv4.pipe( | |
| Effect.mapError((cause) => new PreviewProxyTicketError({ reason: "issuance-failed", cause })), | |
| ); | |
| const now = yield* Clock.currentTimeMillis; | |
| const expiresAt = now + ENTRY_TICKET_TTL_MS; | |
| const secret = yield* loadSigningSecret.pipe( | |
| Effect.mapError((cause) => new PreviewProxyTicketError({ reason: "issuance-failed", cause })), | |
| ); |
Posted via Macroscope — Effect Service Conventions
| export class PreviewProxyTicketError extends Schema.TaggedErrorClass<PreviewProxyTicketError>()( | ||
| "PreviewProxyTicketError", | ||
| { | ||
| reason: Schema.Literals(["invalid-url", "not-local", "not-discovered", "issuance-failed"]), |
There was a problem hiding this comment.
PreviewProxyTicketError has no cause, so the issuance-failed path (which always wraps a real failure — a Crypto error or a secret-store error) loses the underlying error and its stack. Repo convention for wrapped failures is cause: Schema.optional(Schema.Defect()) (see PreviewInvalidUrlError just below, and ServerAuthDpopReplayStateRecordError); optional is right here because the validation reasons legitimately have no underlying error.
| reason: Schema.Literals(["invalid-url", "not-local", "not-discovered", "issuance-failed"]), | |
| reason: Schema.Literals(["invalid-url", "not-local", "not-discovered", "issuance-failed"]), | |
| cause: Schema.optional(Schema.Defect()), |
Posted via Macroscope — Effect Service Conventions
| * restart also rotates nothing the ticket could still be replayed against | ||
| * inside its two-minute window beyond what a fresh mint would grant. | ||
| */ | ||
| const consumedEntryTickets = new Map<string, number>(); |
There was a problem hiding this comment.
Replay protection lives in a module-global Map, which puts security-relevant state outside the Effect environment: it is process-global (shared by every environment/server instance in a process, and leaks between tests in the same file) and cannot be substituted or scoped. Consider holding the consumed-ticket set in a service/layer (e.g. a small Context.Service with make/layer, or reuse ServerSecretStore the way auth/dpop.ts records DPoP replay state) so it is acquired with yield* like the other dependencies in this module.
Posted via Macroscope — Effect Service Conventions
1 participant
A dev server running on an environment's machine is invisible from the phone: it binds loopback and has no public URL. This PR lets a paired T3 Code Mobile client browse that dev server through the environment's existing T3 connection (LAN, Tailscale, or T3 Connect), then capture the viewport, annotate it with numbered pins/boxes and notes, and drop the flattened screenshot plus notes into the thread draft.
How it works
Server
preview.listLocalServers(unary snapshot of the existing port-scanner discovery) andpreview.createProxyTicket(mints a single-use, HMAC-signed entry ticket bound to the environment id and a discovered loopback port; 2-minute expiry). Scopes: read / operate. NewpreviewProxycapability on the environment descriptor gates clients under version skew.GET /api/preview/enter/<ticket>redeems the ticket — expired, reused, malformed, and cross-environment tickets all fail — sets an HttpOnly session cookie, and 302s into the proxied origin.GET /api/preview/exitclears it./src/main.tsx,/@vite/client, and HMR sockets work without HTML rewriting.wsTicketparam) or targeting reserved paths (/api/preview/,/api/assets/,/.well-known/t3/) always bypass the proxy. T3 auth headers and cookies are stripped before anything is forwarded; upstreamSet-Cookievalues that would clobber T3 cookie names are dropped.Mobile
ThreadPreviewroute (entry: header action on the thread screen, capability-gated): dev-server picker → WebView with back/reload/capture/close → annotation editor (numbered pins, drag-boxes, per-marker notes) → "Add to chat" flattens the annotated view to PNG and appends it plus the numbered notes to the draft via the existing composer attachment flow. Nothing auto-sends; cancel leaves the draft untouched; the standard 8-attachment / 10 MB limits apply.Testing on the Android dev app
react-native-view-shot), so the dev client must be rebuilt (expo run:android/ a fresh EAS dev build from this branch) — Metro alone won't load it.vp run dev --share(or your usual server) on the host, start any dev server (e.g.npm run devin a Vite app) in a thread terminal or standalone.Surfaces checklist
packages/contracts, scope map updated (type-enforced).docs/user/mobile-preview.md,docs/internals/preview-proxy.md, glossary terms, README index.Tests
apps/server/src/preview/ProxyAccess.test.ts— ticket issue/redeem: invalid/non-loopback/undiscovered targets, single-use, expiry (TestClock), tamper, cross-environment.apps/server/src/preview/ProxyRoutes.test.ts— bypass decision + header hygiene pure functions, plus an integration pass against a real loopback echo server: enter → cookie → proxied fetch with credentials stripped → bypass on Authorization → tampered-cookie 403 → exit.packages/contracts/src/environment.test.ts— capability skew decode.apps/mobile/src/features/preview/previewAnnotation.test.ts— marker numbering/renumbering, rect normalization, note text, attachment limits.Screenshots/video to follow once it's been exercised on the Android dev build (hence draft). WS-upgrade proxying is exercised manually via HMR; the Node path echoes the first offered subprotocol (
vite-hmr) viaws.🤖 Generated with Claude Code
Built by Claude Fable 5 via Claude Code.
Note
Add localhost dev server preview proxy with mobile screenshot annotation
ThreadPreviewRouteScreenthat lists discovered local servers, opens a WebView to the proxied entry URL, captures a screenshot viareact-native-view-shot, and lets users add numbered pin/box markers with notes before attaching the annotated image to a thread draftpreview.listLocalServersandpreview.createProxyTicketWebSocket RPCs, extendsExecutionEnvironmentCapabilitieswith optionalpreviewProxy, and wires server routes, middleware, and WS handlerspreviewProxyMiddlewareLayerruns before auth-free routes inmakeRoutesLayerand intercepts any request carrying the preview cookie; reviewers should verify bypass rules inshouldBypassPreviewProxycover reserved paths (/api/preview/,/api/assets/,/.well-known/t3/) and WebSocket ticket param to avoid shadowing existing routes📊 Macroscope summarized a4586c4. 20 files reviewed, 11 issues evaluated, 3 issues filtered, 7 comments posted
🗂️ Filtered Issues
apps/server/src/preview/ProxyAccess.ts — 1 comment posted, 2 evaluated, 1 filtered
SIGNING_SECRET_NAMEmakes the HMAC key persistent across server restarts, while redeemedticketIdvalues exist only in the in-memoryconsumedEntryTicketsmap. Restarting within the two-minute TTL therefore clears the replay record but preserves signature validity, allowing an already-redeemed entry URL to be redeemed again for a fresh 12-hour proxy session despite the ticket's single-use guarantee. [ Out of scope ]apps/server/src/preview/ProxyRoutes.ts — 1 comment posted, 3 evaluated, 2 filtered
filterForwardedRequestHeadersremoves only a fixed set of hop-by-hop names and ignores the names listed in the incomingConnectionheader. For example,Connection: x-transport-stateplusX-Transport-State: ...dropsConnectionbut still forwardsx-transport-state, contrary to HTTP intermediary semantics; this can leak connection-local metadata to the dev server or alter its request handling. Parse theConnectionoptions first and exclude each nominated field too. [ Out of scope (post-validation triage) ]Connectionvalue. An upstream response such asConnection: x-internalwithX-Internal: ...hasConnectionremoved but still exposesx-internalto the WebView, violating HTTP hop-by-hop handling and potentially changing downstream response semantics. Parse the upstreamConnectionoptions before iterating and suppress every named field. [ Out of scope (post-validation triage) ]