Skip to content

fix: stabilize local dev auth startup#3252

Closed
huxcrux wants to merge 12 commits into
pingdotgg:mainfrom
huxcrux:dev-fix
Closed

fix: stabilize local dev auth startup#3252
huxcrux wants to merge 12 commits into
pingdotgg:mainfrom
huxcrux:dev-fix

Conversation

@huxcrux

@huxcrux huxcrux commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

What Changed

  • Prevent Clerk-dependent desktop/web code from loading when Clerk local config is absent.
  • Register the desktop app scheme independently so local desktop startup no longer depends on Clerk side effects.
  • Route desktop custom-scheme requests through the app protocol using the Effect/Undici HTTP client.
  • Load the diff worker via the portable worker entry to avoid Vite worker default-export failures.
  • Route configured local-dev HTTP requests through the Vite origin while preserving direct WebSocket targets.
  • Harden pairing bootstrap by deduping same-token submits and tolerating already-authenticated stale token retries.

Why

Local development could fail when Clerk config was missing or when running the web dev flow through vp run dev. Clerk was being loaded too eagerly, desktop scheme registration depended on Clerk bridge setup, diff worker imports could fail under Vite, and pairing-token exchange could consume a one-time token before the browser session was observable on the correct origin.

This keeps local startup resilient without Clerk config, avoids CORS/cookie-origin issues by using the web/Vite origin for browser HTTP requests, and preserves direct loopback WebSockets where required.

UI Changes

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Note

Medium Risk
Desktop auth and renderer URL selection change in development, but production paths with a Clerk key stay on the custom protocol and bridge; mobile list scrolling behavior changes with the keyboard refactor.

Overview
Makes local desktop dev work without Clerk by gating every Clerk touchpoint on whether a valid build-time publishable key resolves to a Frontend API hostname.

Desktop: New DesktopClerkConfig centralizes hostname parsing and isDesktopClerkBridgeEnabled(). The Clerk SDK bridge, preload exposeClerkBridge, and custom-scheme protocol registration are skipped when disabled; in that dev mode the main window loads the Vite dev server URL instead of t3code-dev://. makeDesktopClerkLayer can omit bridge acquire/release while still providing single-instance / second-window behavior.

Mobile: Thread chat drops KeyboardAwareLegendList / shared-value keyboard inset hooks in favor of KeyboardAvoidingLegendList, measured composer overlay height, and direct scrollToEnd after send.

Web: Adds a thin TimelineLegendList type wrapper so the timeline can pass contentInsetEndAdjustment without fighting LegendList typings.

Reviewed by Cursor Bugbot for commit 59f460b. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix local dev auth startup by conditionally initializing the Clerk bridge

  • Introduces DesktopClerkConfig.ts to centralize Clerk bridge enablement checks and hostname resolution from the publishable key.
  • The Electron custom protocol registration, preload bridge exposure, and SDK bridge acquisition are now all skipped when no Clerk publishable key is present (isDesktopClerkBridgeEnabled() === false).
  • Development builds without a publishable key load the renderer directly from the dev server URL instead of the custom t3code-dev protocol.
  • Refactors ThreadDetailScreen and ThreadFeed on mobile to use KeyboardAvoidingLegendList with manual composer layout tracking, removing Legend keyboard helpers.

Macroscope summarized 59f460b.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4b99ca00-2b8d-42cd-a793-51b896f18c02

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 20, 2026
Comment thread apps/desktop/src/electron/ElectronProtocol.ts Outdated
Comment thread apps/web/src/environments/primary/auth.ts Outdated
@huxcrux
huxcrux marked this pull request as draft June 20, 2026 09:42
Comment thread apps/web/src/environments/primary/auth.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

1 blocking correctness issue found. This PR modifies runtime behavior in desktop auth startup and mobile keyboard/scroll handling. Unresolved review comments identify potential bugs in the scroll behavior changes that warrant human review before merging.

You can customize Macroscope's approvability policy. Learn more.

Comment on lines +167 to +180
const effect = Effect.gen(function* () {
const clientRequest = HttpClientRequest.fromWeb(new Request(targetUrl, init));
const clientResponse = yield* httpClient.execute(clientRequest);
return withContentSecurityPolicy(
new Response(
request.method === "HEAD" ? null : Stream.toReadableStream(clientResponse.stream),
{
status: clientResponse.status,
headers: clientResponse.headers,
},
),
contentSecurityPolicy,
);
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this made? i ran into clerk bot protection when the fetch was made using undici earlier

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an attempt to try keep the custom scheme for all app requests. However to avoid alot of the changes we could simply revert back to http://127.0.0.1 when clerk config is not present.

I'll push a fix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be better now

Comment thread apps/desktop/src/preload.ts
huxcrux and others added 12 commits June 30, 2026 15:44
Gate Clerk-dependent renderer and desktop bridge setup when local config is absent, register the desktop schemes independently, proxy custom-scheme requests through the app protocol, and load the diff worker via the portable package worker entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route configured loopback HTTP requests through the Vite origin so browser-session cookies are scoped to the page origin, while preserving direct WebSocket targets. Dedupe same-token pairing submits and tolerate stale one-time-token retries when a session is already authenticated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stream desktop protocol proxy responses instead of buffering complete bodies, and wait for browser sessions to become observable after manual pairing exchanges.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 59f460b. Configure here.

lastScrolledAnchorMessageIdRef.current = null;
freeze.set(false);
}, [freeze, selectedThreadKey]);
}, [selectedThreadKey]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Composer inset stale after changes

Low Severity

Replacing the keyboard inset hooks with composerOverlayHeight state means contentBottomInset can stay at an old measured value when estimatedOverlayHeight drops (for example after switching threads or when the working indicator clears) until onComposerLayout runs again, leaving excess bottom padding on the thread feed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 59f460b. Configure here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

const frame = requestAnimationFrame(() => {

After sending a message from the focused composer, listRef.current?.scrollToEnd() runs immediately in the requestAnimationFrame callback, but at that point the keyboard is still dismissing and the KeyboardAvoidingLegendList has not finished adjusting its content insets. The scroll target is computed against the pre-keyboard-dismiss layout, so the newest message ends up scrolled underneath the closing keyboard instead of being fully visible. The previous useKeyboardScrollToEnd helper coordinated the scroll with the keyboard animation freeze so the list settled after the insets updated; removing it means freshly sent messages are not brought fully into view until the user manually scrolls.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadDetailScreen.tsx around line 289:

After sending a message from the focused composer, `listRef.current?.scrollToEnd()` runs immediately in the `requestAnimationFrame` callback, but at that point the keyboard is still dismissing and the `KeyboardAvoidingLegendList` has not finished adjusting its content insets. The scroll target is computed against the pre-keyboard-dismiss layout, so the newest message ends up scrolled underneath the closing keyboard instead of being fully visible. The previous `useKeyboardScrollToEnd` helper coordinated the scroll with the keyboard animation freeze so the list settled after the insets updated; removing it means freshly sent messages are not brought fully into view until the user manually scrolls.

@huxcrux

huxcrux commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

This has been fixed in main, closing the PR

@huxcrux huxcrux closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants