Skip to content

fix: let a user removed from their last organization recover - #314

Open
FrameAutomata wants to merge 3 commits into
fix/setup-test-duckdb-build-constraintfrom
fix/292-org-removal-stuck-state
Open

fix: let a user removed from their last organization recover#314
FrameAutomata wants to merge 3 commits into
fix/setup-test-duckdb-build-constraintfrom
fix/292-org-removal-stuck-state

Conversation

@FrameAutomata

Copy link
Copy Markdown
Collaborator

Closes #292.

Stacked on #312. Base is fix/setup-test-duckdb-build-constraint, so the diff here shows only this change and the DuckDB CI job stays meaningful — without #312 that job fails on the unrelated #309 build-constraint bug. GitHub retargets this to main automatically when #312 merges.

The stuck state

+layout.svelte:159 pins every zero-project account to /setup:

const needsSetup = $derived(authState.isAuthenticated && projectsState.projects.length === 0);

and /setup's only branch for a user with no writable org was a sentence with no action:

You need an owner, admin, or user role in an organization to create projects.

Remove someone from their last organization mid-session and their project list empties, the layout locks them to /setup, and /setup offers no way out. Every other route bounces back.

Three parts

1. POST /api/organizations. OrganizationRepository.Create had exactly two call sites — auth.controller.go:149 (register) and oauth.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 becomes owner.

  • Timezone defaults to UTC and is validated with time.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.
  • Cloud gating follows the existing hook pattern (ProjectLimitHook, MemberLimitHook, CheckLimitHook). OrganizationLimitHook is 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.
  • Name validation is length-only (1–100 runes), deliberately not projectNameRegex: that regex rejects , and ., which real company names contain.

2. /setup splits the two cases it used to collapse. Zero organizations gets a create form; organizations that are all readonly keeps today's message, which is correct advice in that case.

3. Stale membership. authState.organizations is hydrated from localStorage (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. /setup now refreshes from /me/login-bundle on 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, 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 route registration. Untagged, so it runs in all three CI jobs.

Verification

Check Result
go test ./... (default) exit 0
CGO_ENABLED=1 go test -tags telemetry_duckdb ./app/controllers/ ok
go vet ./... / gofmt -l . clean
New endpoint tests (4 tests, 7 cases) pass
routes_test.go under all three build-tag combos pass
npm run check 0 errors; 12 pre-existing warnings, none in the changed file

Not 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-check at the type level, but the rendered /setup screen 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

FrameAutomata and others added 2 commits August 26, 2026 12:13
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>
@FrameAutomata FrameAutomata added the ci Run CI on this PR (remove and re-add to re-validate after a push) label Aug 26, 2026
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>
@FrameAutomata FrameAutomata added ci Run CI on this PR (remove and re-add to re-validate after a push) and removed ci Run CI on this PR (remove and re-add to re-validate after a push) labels Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Run CI on this PR (remove and re-add to re-validate after a push)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant