fix(ui): render the sign-in form on self-hosted deploys instead of looping (#311) - #319
Open
stubbi wants to merge 1 commit into
Open
fix(ui): render the sign-in form on self-hosted deploys instead of looping (#311)#319stubbi wants to merge 1 commit into
stubbi wants to merge 1 commit into
Conversation
…oping Fixes #311. PR #273 replaced the SPA's only auth route with a redirect to the gateway-served page: - <Route path="auth" element={<AuthPage />} /> + <Route path="auth" element={<CloudAuthRedirect />} /> That is right on cloud, where the gateway reserves /auth/* and the SPA never sees those URLs. Everywhere else it is wrong: nothing serves /auth/sign-in, so it falls through to the catch-all, CloudAccessGate sees an unauthenticated visitor and redirects to /auth/sign-in?next=<current> -- which is the page it is already on. `next` nests one level per pass until the request line hits HTTP 414. The form never mounts, so nobody can log into a self-hosted install in `authenticated` mode. AuthPage was never deleted, just left unrouted. Two changes, either of which alone would stop the 414 but neither of which alone is sufficient: - CloudAccessGate no longer redirects when the visitor is already on an auth route. Being unauthenticated on the sign-in page is the normal state, not a reason to redirect. This is the general guard, so a future auth subroute cannot reintroduce the loop. - /auth/sign-in is routed to AuthPage, so the form actually renders once the loop stops. Inert on cloud, where the request never reaches the bundle. Reported by @vfoschi with a clean-profile Playwright repro and a bisect to the exact image revision, which is what made this quick to pin down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Thinking Path
Linked Issues or Issue Description
Fixes #311.
Reported by @vfoschi with a clean-profile Playwright repro and a bisect to the exact image revision (works
b57c11d7, brokene02ab855and later). That bisect is what made the cause quick to confirm — thank you.What Changed
ui/src/components/CloudAccessGate.tsx— the unauthenticated redirect now skips auth routes (isAuthRoute()). Being signed out on the sign-in page is the expected state, not a reason to redirect. This is the general guard: it holds for any future/auth/*subroute.ui/src/App.tsx—/auth/sign-inis routed toAuthPage. Inert on cloud, where the gateway reserves/auth/*and the request never reaches the bundle; on self-hosted it is what actually renders the form once the loop stops.ui/src/components/CloudAccessGate.test.tsx— theuseLocationmock is now per-test mutable (default unchanged), plus two regression tests.Either change alone stops the 414. Neither alone is sufficient: the guard without the route leaves you on a page that renders nothing, and the route without the guard still loops for any other unauthenticated path that lands under
/auth/.Root cause
8695d8854(PR #273) changed one line:CloudAuthRedirectunconditionally runswindow.location.replace("/auth/sign-in?next=…"). With no SPA route for/auth/sign-in, the catch-all takes it, and the gate ran:with no exemption for the sign-in route itself. Upstream
paperclipai/paperclipstill routesAuthPageand is unaffected — this is a fork-only regression, which is why it survived two weeks: it cannot reproduce on the only deployment we look at.Verification
Both new tests were confirmed to fail before the fix and pass after.
Full-suite comparison against a clean
fork/maincheckout on the same machine: 16 test files fail at baseline, the same 16 with this change, identical sets. No regressions.Honest gap: the two new tests cover the loop guard, which is the 414. They do not cover "the form actually mounts at
/auth/sign-in" end to end, because that needs a real self-hostedauthenticateddeployment. There is no regression test today for anonymous user can reach the sign-in form in authenticated mode, and that absence is why this shipped. I have asked @vfoschi for their Playwright script as a basis for one; that belongs in the e2e suite as a follow-up, not in this PR.Risks
Low on cloud: the new route is unreachable there (gateway-reserved), and the gate guard only changes behaviour for paths starting
/auth/, which the SPA never renders on cloud. Cloud sign-out and unauthenticated redirect behaviour from #273 is untouched.The one behaviour change worth naming: an unauthenticated visitor who lands on an
/auth/*path that has no route will now fall through to the catch-all and render the app shell rather than redirecting. That is strictly better than an infinite redirect, but it is a 404-shaped experience./authand/auth/sign-in— the only two auth URLs the app produces — both have routes.Model Used
claude-opus-5[1m], 1M context), extended thinking, via the Claude Code CLI harness with tool use (shell, file edits). Change authored agent-assisted.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template#NNN/github.com/paperclipai/paperclipURLs)docs/...,fix/...) and contains no internal Paperclip ticket id or instance-derived details🤖 Generated with Claude Code