fix(web): resolve public /signin /signup /auth/callback 404s - #5780
Conversation
Bare auth URLs were locale-prefixed into /en/signin, /en/signup, and /en/auth/callback, which had no pages. Keep sign-in and create-account as locale-aware public routes that use the canonical mark and send the person to the CWC app, hop OAuth callbacks to app.codewhale.net with the query intact, and fold /login /register aliases. Local CLI use is not presented as requiring an account. Closes #5767 Signed-off-by: CodeWhale Bot <bot@codewhale.net>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @Hmbown's task in 1m 38s —— View job Review: fix(web): resolve public /signin /signup /auth/callback 404sTodo list
SummaryThis is a clean, well-scoped fix. No correctness bugs found. Highlights worth calling out:
Minor, non-blocking note (reuse/simplification): Provider/model routing safety (the v0.8.65 EPIC #2608 invariant) doesn't apply here — this PR touches only the Next.js web app, no Rust workspace or model-routing code. Testing note: I couldn't run |
|
Claude finished @Hmbown's task in 1m 38s —— View job Review: fix(web): resolve public /signin /signup /auth/callback 404s
SummaryClean, well-scoped fix for #5767. Correctness
Security
Provider/model/route safety (EPIC #2608)
Reuse/simplification
Tests
No blocking issues. Approve on the strength of the diff review above (test execution not independently verified — see note). |
There was a problem hiding this comment.
Codewhale review
PR adds locale-aware public /signin, /signup, and OAuth /auth/callback redirects with tests. The core middleware and routing logic is sound, but the PR also contains unrelated changelog entries and leaves the fallback callback page untested and duplicative.
Findings
- [WARNING] Unrelated #5776 changelog entries included in this web auth PR (
CHANGELOG.md)
Both CHANGELOG.md and crates/tui/CHANGELOG.md gain Pod/fleet language entries (#5776) that have nothing to do with fixing public auth routes (#5767). These likely came from an unintended branch base and should be removed to keep the PR focused. - [INFO] Fallback OAuth callback page is untested (
web/app/[locale]/auth/callback/page.tsx)
web/app/[locale]/auth/callback/page.tsx contains query-copying logic that mirrors publicAuthCallbackDestination but has no direct test. Middleware tests cover the redirect path, but if middleware is bypassed the page itself could regress silently. - [INFO] Query forwarding logic is duplicated between middleware helper and fallback page (
web/app/[locale]/auth/callback/page.tsx)
The fallback callback page manually iterates searchParams to rebuild the app callback URL, while public-auth-routes.ts already provides publicAuthCallbackDestination using URL.search. Reusing the helper would avoid future divergence if the callback path or query handling changes.
Assessment
The implementation correctly redirects /auth/callback to the CWC app with query preservation, aliases /login and /register, and adds locale-aware sign-in/signup pages with tests. Main concerns are scope creep from unrelated changelog entries and lack of direct coverage for the fallback callback page. No blocking correctness issues found in the diff.
Advisory review by Codewhale (codewhale review --pr 5780 --post, head 0cf5cf3cc2aaf1f32d2c67a83b8cb51cad303253). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
Summary
Closes #5767.
Public
https://codewhale.net/signin,/signup, and/auth/callbackwere locale-prefixed to/en/...and 404ed. This adds locale-aware public Sign in and Create account pages (canonical mark, account-first not hostage) and hops OAuth callbacks to the CWC app with the query intact.Does not implement CWC registration or OAuth. That remains CWC #305.
Testing
cd web && npx vitest run lib/public-auth-routes.test.ts lib/middleware.test.ts lib/docs-ia.test.ts(39 passed)cargo fmt --all -- --check(no Rust changes)cargo clippy --workspace --all-targets --all-features --lockedcargo test --workspace --all-features --lockedChecklist