fix: let a user removed from their last organization recover - #314
Open
FrameAutomata wants to merge 3 commits into
Open
fix: let a user removed from their last organization recover#314FrameAutomata wants to merge 3 commits into
FrameAutomata wants to merge 3 commits into
Conversation
The layout pins every zero-project account to /setup
(+layout.svelte:159), and /setup's only branch for a user with no
writable organization was a sentence with no action:
You need an owner, admin, or user role in an organization to
create projects.
So removing someone from their last organization mid-session left them
locked to a dead-end screen with no route forward, which is the stuck
state in #292.
Three parts:
1. POST /api/organizations. Organizations could only be created by
register (auth.controller.go) and SSO finish-setup
(oauth.controller.go), both account-creation paths, so an existing
user had no way to make one. The new endpoint creates an
organization with the caller as owner. Timezone defaults to UTC and
is validated with time.LoadLocation, because on-call schedule
resolution is tz-aware calendar math and a bad zone would surface
much later as wrong shift boundaries. Cloud gating follows the
established hook pattern (ProjectLimitHook, MemberLimitHook,
CheckLimitHook); OrganizationLimitHook is keyed on the user rather
than an org since it runs before the org exists.
2. /setup distinguishes the two cases it used to collapse. No
organizations at all offers a create form; organizations that are
all readonly keeps the existing message, which is correct advice
there.
3. authState.organizations is hydrated from localStorage and only
rewritten on login, so a membership removed mid-session stays
cached. /setup now refreshes from /me/login-bundle on mount and
renders a loading state until it resolves, so the recovery screen
never acts on a stale list.
Also adds routes_test.go, which registers the real route tree. Gin
panics at registration on a wildcard conflict -- a boot-time crash no
handler test would catch -- and POST /api/organizations is a static
sibling of the /api/organizations/:organizationId/... subtree, the
shape most likely to trip it. Nothing else in the suite covered this.
Verified: go vet, gofmt, and the full backend suite pass; the four new
endpoint tests (7 cases) pass; svelte-check reports 0 errors with no
warnings in the changed file; routes_test.go passes under all three
build-tag combinations.
Closes #292
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the review of this branch. Three substantive fixes plus cleanups, all found after actually running the app. Security/policy: POST /api/organizations had no self-hosted single-org gate. Register enforces one (auth.controller.go:112) but the new handler skipped it, and the route carries no role guard and OrganizationLimitHook is nil outside cloud -- so any authenticated user of any role, readonly included, could mint an organization and own it. Now mirrors Register's rule. It answers 422 rather than Register's 409 because the message has to reach the recovery form, and api.ts only extracts response bodies from 401/403/422 -- a 409 surfaces to the user as "API Error: Conflict". Verified in a browser: the message renders in the form. Correctness: the handler skipped PostRegistrationHooks, which both other org-creating paths run for every new org+owner pair. That is the cloud build's provisioning seam, so organizations created here were silently missing whatever cloud wires there. Now runs them, which is why the handler loads the full user rather than just its id. Frontend: the `refreshing` render gate blocked the whole page on a /me/login-bundle round trip for every user, to avoid a branch flip in one rare case. A screenshot caught it still spinning at 3.5s. The cached org list is correct for every case except the mid-session removal, so the page now renders immediately and lets the response correct the branch. Reuse/simplification: - oncall.LoadTimezone instead of an inline time.LoadLocation plus a third bespoke "unknown timezone" message - ErrorAlert instead of a raw <p class="text-destructive">, matching every other inline form error in the app - routes_test.go: dropped the defer/recover, which discarded the panic stack naming the conflicting path; slices.ContainsFunc; cleanup only in the branch that mutates config - organization_create_test.go: newOrgTestUser helper collapses a 10-line preamble repeated four times - dropped a redundant selectedOrgId write the $effect already owns New tests: self-hosted allows only one org, cloud allows more, PostRegistrationHooks run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
npx eslint reports @typescript-eslint/no-explicit-any on the catch in createOrganization. CLAUDE.md's page template shows catch (e: any), but no other route file in src/routes actually uses it, and the rule is on. Narrow with instanceof Error instead. Found while assessing whether the frontend could carry a CI gate: it is the only eslint error in the diff, the other 10 are pre-existing. 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.
Closes #292.
The stuck state
+layout.svelte:159pins every zero-project account to/setup:and
/setup's only branch for a user with no writable org was a sentence with no action:Remove someone from their last organization mid-session and their project list empties, the layout locks them to
/setup, and/setupoffers no way out. Every other route bounces back.Three parts
1.
POST /api/organizations.OrganizationRepository.Createhad exactly two call sites —auth.controller.go:149(register) andoauth.controller.go:270(SSO finish-setup) — both account-creation paths. An existing user had no way to create an organization, so the issue's "asking them if they want to create one" needed a new endpoint. The caller becomesowner.UTCand is validated withtime.LoadLocation. On-call schedule resolution is tz-aware calendar math, so an unparseable zone would surface much later as wrong shift boundaries rather than as an error here.ProjectLimitHook,MemberLimitHook,CheckLimitHook).OrganizationLimitHookis keyed on the user, not an org, since it runs before the org exists — commented at the declaration because it breaks the shape of its siblings.projectNameRegex: that regex rejects,and., which real company names contain.2.
/setupsplits the two cases it used to collapse. Zero organizations gets a create form; organizations that are allreadonlykeeps today's message, which is correct advice in that case.3. Stale membership.
authState.organizationsis hydrated fromlocalStorage(auth.svelte.ts:12) and only rewritten on login, so a mid-session removal stays cached — meaning the recovery screen would otherwise render a selector for an org the user is no longer in and offer a project flow that 403s./setupnow refreshes from/me/login-bundleon mount and shows a loading state until it resolves.Also included
backend/app/controllers/routes_test.go— registers the real route tree. Gin panics at registration on a wildcard conflict, which is a boot-time crash no handler test would catch, andPOST /api/organizationsis a static sibling of the/api/organizations/:organizationId/...subtree — the shape most likely to trip it. Nothing else in the suite covered route registration. Untagged, so it runs in all three CI jobs.Verification
go test ./...(default)CGO_ENABLED=1 go test -tags telemetry_duckdb ./app/controllers/go vet ./.../gofmt -l .routes_test.gounder all three build-tag combosnpm run checkNot verified
I did not run the app and click through the flow — the recovery path is covered by unit tests at the handler level and by
svelte-checkat the type level, but the rendered/setupscreen itself is untested. Worth a manual pass before merge: remove yourself from your last org in a second browser session and confirm the create form appears and lands you in a working project setup.🤖 Generated with Claude Code