fix(oauth): open OAuth URL on Windows without cmd's & truncation

[naveen-kurra]

Problem

forge init → "Login with ChatGPT" on Windows lands on OpenAI's auth page with:

Authentication Error
An error occurred during authentication. Please try again.
error_code: unknown_error, request_id: 66150e5f-d82b-46b9-8206-1eea4fe1c57d

macOS works fine. Repro'd by Naveen on a fresh Windows install.

Root cause

forge-core/llm/oauth/flow.go:155-156:

case "windows":
    cmd = exec.Command("cmd", "/c", "start", url)

cmd /c start <url> interprets every unquoted & in the URL as the shell "AND" separator. The OAuth authorize URL Forge builds glues eight query parameters with &:

?response_type=code
&client_id=app_EMoamEEZ73f0CkXaXp7hrann
&redirect_uri=http%3A%2F%2Flocalhost%3A1455%2Fauth%2Fcallback
&scope=openid+profile+email+offline_access
&state=<random>
&code_challenge=<base64url>
&code_challenge_method=S256
&id_token_add_organizations=true
&codex_cli_simplified_flow=true

What actually reaches the browser on Windows: only ?response_type=code. Every param after the first & is stripped and interpreted by cmd as separate commands that silently error. OpenAI's auth server rejects the incomplete request with the generic unknown_error — no server-side hint what went wrong. macOS open and Linux xdg-open treat the URL as one atomic argument and are unaffected.

Fix

Switch the Windows branch to rundll32 url.dll,FileProtocolHandler — the Windows shell API that opens URLs via the registered protocol handler without going through cmd's parser. Robust across cmd.exe, PowerShell, and Windows Terminal. Standard Windows API since XP; used by Docker Desktop, GitHub CLI, and every mature Go browser-launcher library for exactly this reason.

case "windows":
    cmd = exec.Command("rundll32", "url.dll,FileProtocolHandler", url)

Tests

New forge-core/llm/oauth/flow_test.go with two pins that would have caught this class of bug:

• TestBuildAuthURL_MultipleParamsAreIntact — asserts every required OAuth 2.0 + PKCE param is present in the built authorize URL, ExtraParams land verbatim, and the URL carries ≥7 & separators. Catches any URL-builder or launcher regression that drops params.
• TestOpenAIConfig_ClientIDAndScopes — pins OpenAI's Codex client_id, verifies offline_access is in the scope list (silent removal would kill refresh tokens ≈1h after login), and that the endpoints are https + the redirect URI targets the 1455 callback port.

Both pass. Full forge-core/llm/oauth suite green; gofmt + golangci-lint clean.

Manual verification path (Windows)

Reviewer with a Windows box, after checking out this branch:

forge init
# Choose Provider: OpenAI → "Sign in with ChatGPT" → browser opens
# to auth.openai.com with the FULL URL (all 8 params visible in
# the address bar) → OpenAI consent screen appears → Continue →
# redirects to localhost:1455/auth/callback → Forge saves token.

Pre-fix: browser lands on the auth error page. Post-fix: consent flow completes.

Test plan

• go test ./forge-core/llm/oauth/... — green (macOS)
• golangci-lint run ./forge-core/... — 0 issues
• Manual: forge init on a Windows machine completes the ChatGPT OAuth flow end-to-end
• Manual: forge init on macOS still works (no regression on the darwin branch)

[initializ-mk]

Review: open OAuth URL on Windows without cmd's & truncation

A precise, well-diagnosed external contribution. The root cause is correct — cmd /c start <url> treats & as the shell command separator, so the 9-param OpenAI authorize URL reached the browser as just ?response_type=code and OpenAI returned a generic unknown_error. The rundll32 url.dll,FileProtocolHandler fix opens the URL through the Windows shell API without cmd's parser. Root-cause writeup and doc comment are excellent. CI fully green, including both Windows builds.

Two things I verified that de-risk this entirely:

• The fix matches existing in-repo precedent. forge-cli/cmd/mcp_browser.go already uses the identical rundll32 url.dll,FileProtocolHandler on its Windows branch — so this isn't just aligned with the de-facto pkg/browser standard, it matches a launcher the project already ships. Any "is rundll32 an acceptable LOLBin?" concern is already settled by the maintainers' own code.
• Security posture improves. The URL is now a single exec.Command argument with no shell in the path — the cmd-parser surface is gone, strictly better than cmd /c start.

Finding 1 (Medium, completeness): the same broken pattern still lives in forge-ui/server.go:216

There are three near-identical platform-switch browser launchers: this OAuth flow (now fixed), mcp_browser.go (already correct), and forge-ui/server.go:216 — still exec.Command("cmd", "/c", "start", url) for the forge ui dashboard. Latent today (the dashboard URL has no & params), but the identical footgun: the moment that URL gains a param (?agent=x&tab=y), forge ui breaks on Windows exactly as OAuth did. Since this PR is the one establishing "we don't use cmd /c start," it's the natural place to either fold in the one-line forge-ui fix or file a follow-up. Best outcome: extract the three copies into one shared openBrowser(url) helper — the divergence is exactly how one copy stayed buggy while another was already fixed.

Finding 2 (Minor, test): the changed line has no direct test

The two new tests pin the URL builder and config, not the launcher — the actual line this PR changes (rundll32 vs cmd) is untested. The ≥7 & assertion is a reasonable proxy (proves the URL has the params that were being truncated), but nothing pins that the Windows branch preserves them through openBrowser. Extracting browserCommand(goos, url string) *exec.Cmd would make the selection table-testable (assert the windows case is rundll32 with the URL as a single un-split arg) — directly locking the fix against regression. Not blocking; just noting the fix itself rides on manual verification.

Finding 3 (Minor, validation): the Windows manual-verification box is unchecked — and it's the fix's target

The test plan's post-fix Windows and macOS-no-regression boxes are both unticked. CI's Windows builds only prove the tests compile/pass there, not that rundll32 launches the browser with the full URL (can't be CI-tested). Low-risk given the mcp_browser.go precedent already ships rundll32 in production, but a maintainer with a Windows box should still run forge init → ChatGPT login end-to-end (and confirm darwin is untouched) before merge.

What's good

• The offline_access scope pin is a genuinely valuable test (verified the scope is real at flow.go:32): silently dropping it would kill refresh tokens ~1h after login, a subtle delayed failure. Sharp instinct to guard, especially from a first-time contributor.
• Root-cause analysis is exemplary — reproduced, traced to the exact line, explained why macOS/Linux are unaffected.

Verdict

Correct, low-risk, merge-ready. The one thing I'd want addressed is finding 1: the sibling cmd /c start in forge-ui/server.go should be fixed here or explicitly deferred to a follow-up (ideally consolidating the three launchers), since this PR is precisely the moment that pattern is being retired. Findings 2–3 are a testability nicety and a manual tick a maintainer owns. Nice contribution.

[initializ-mk]

The fix is correct — and worth noting it matches forge-cli/cmd/mcp_browser.go, which already uses this exact rundll32 url.dll,FileProtocolHandler on its Windows branch. So this aligns with shipped in-repo precedent, not just external libraries.

One testability nicety (non-blocking): the changed line here is the actual fix, but neither new test exercises openBrowser — they pin the URL builder + config instead. Extracting browserCommand(goos, url string) *exec.Cmd and asserting the windows case is rundll32 with the URL as a single un-split argument would lock this line against a future regression, since a shell-truncation bug is invisible to a URL-shape test. See finding 2 in the review body.