fix(sync): setup error handling, Windows browser open, themed callback pages#804
Open
Enclavet wants to merge 1 commit into
Open
fix(sync): setup error handling, Windows browser open, themed callback pages#804Enclavet wants to merge 1 commit into
Enclavet wants to merge 1 commit into
Conversation
…k pages
- Catch errors in 'sync setup' action: AuthError/DiscoveryError (login
timeout, port exhaustion, discovery failures) now print a clean one-line
message and exit 1 instead of crashing Node with an unhandled rejection
stack trace. Also guard the double rejection when all callback ports are
in use (both 'ready' and the callback promise reject).
- Windows: open the auth URL via 'rundll32 url.dll,FileProtocolHandler'
instead of 'cmd /c start'. cmd.exe splits its command line on '&', which
truncated the OAuth query string at the first parameter, so the IdP
received a bare /authorize request ('no parameters'). rundll32 receives
the URL as a plain process argument with no shell parsing.
- Theme the OAuth callback landing pages to match the dashboard (warm
paper background, ink text, forest-green success / terracotta failure),
fully inline with no network or asset dependencies.
AI-Origin: human
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.
Three fixes to the
codeburn sync setuplogin flow based on user feedback.1. Login timeout crashed Node with a raw stack trace
When the 5-minute login window expired,
sync setupdied like this:The
setupaction had no error handling (unlikepush), so the rejectionescaped as an unhandled promise rejection. Now:
AuthError,DiscoveryError— login timeout, callback portexhaustion, discovery/OIDC failures) print a clean one-line
Error: <message>and exit 1.the
readypromise and the callback promise reject; the second one couldcrash the process as a separate unhandled rejection. The callback promise
is now marked handled while
awaitsemantics are preserved.2. Windows: browser opened the auth URL with no query parameters
On Windows the browser opened, but the IdP showed a "no parameters" error.
Cause: the URL was launched via
cmd /c start "" <url>.cmd.exesplits itscommand line on
&, so the OAuth query string was truncated at the firstparameter — the IdP received a bare
/authorizewith noclient_id,redirect_uri, or PKCE challenge, and cmd tried to execute the leftoverfragments as commands.
Fix: launch via
rundll32 url.dll,FileProtocolHandler <url>, which receivesthe URL as a plain process argument with no shell parsing, putting Windows on
the same footing as macOS (
open) and Linux (xdg-open).Verified on Windows 11:
rundll32 url.dll,FileProtocolHandler "https://example.com/?a=1&b=2&c=3"opens the browser with all parametersintact, and the equivalent
execFileSynccall (the exact code path) does too.3. Themed callback landing pages
The success/failure pages were unstyled
<h1>markup. They now match thedashboard theme (
dash/src/index.css— warm paper surfaces, ink text,forest-green success / terracotta failure), via a new
renderCallbackPage()in
src/sync/auth.ts. Everything is inline: the page is served once from athrowaway localhost server, so it has no network font, external CSS, or asset
dependencies.
Testing
All three fixes were manually verified end-to-end, and the full test suite was run.
Manual verification:
and exit 1, no stack trace (previously crashed Node).
rundll32 url.dll,FileProtocolHandler "https://example.com/?a=1&b=2&c=3"opens the browser with all query parameters intact, and the exact
execFileSynccall used by the fix behaves the same. The oldcmd /c startpath reproduces the truncation.states.
openalready passed the URLas a direct argument).
Automated:
npx tsc --noEmitcleannpx vitest run, 217 files / 2456 tests): 2440 passed,11 failed, 5 skipped. All failures are pre-existing on
mainorload-order flakes — verified by running the same failing files on
main(identical failures:
cursor-real-tokens,parser-proxy-pricing) and inisolation on this branch (the CLI/menubar files pass). None touch sync.
tests/sync.test.ts,tests/sync-e2e.test.ts,tests/sync-push.test.ts), including the callback-server timeout test.