fix(auth): stop sessions carrying an organization the user left - #770
Conversation
…orities (#766) * feat(dashboard): recover business context edits and preserve team priorities * fix(dashboard): protect newer context drafts from late saves
* feat(ai): deliver canonical organization context to chat agents * feat(ai): deliver structured team context and mixed provenance * fix(ai): attribute team-defined event and success meanings once * fix(ai): preserve business assertions when references exceed budget * revert(ai): drop ineffective event-attribution prompt rule The three-case present-context follow-up stayed at 16/18 on the small manual rubric, still missed the targeted emitter attribution, and increased words by 7.3%. Remove the extra rule and its tests rather than retaining prompt growth without a demonstrated benefit. Preserve the original attributed-assertions guidance and reference-budget fallback. The small fixture context again matches the original paired run byte for byte. This reverses the rule from 7aa476d after rebasing onto merged parent #766.
session.create.before returned early whenever activeOrganizationId was set, without checking the user was still a member of it. A user removed from an organization keeps that pointer on every subsequent session, so the orphan recovery added in a2b16e5 never runs for them, and the session advertises an organization they have no membership in. That matters because Better Auth falls back to activeOrganizationId whenever hasPermission is called without an explicit organizationId. Production has seven users with no membership at all. One signed up in December and was still signing in on 2026-08-28, with sessions on Mar 26, Jul 27, Jul 28, Aug 10 and Aug 13 all pointing at an organization they are not a member of. Recovery had been skipped every time. The guard now verifies membership before trusting the pointer, clears it when the membership is gone, and threads the cleared value through the remaining paths including the catch, so no branch can hand back a session still carrying it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Greptile SummaryThis PR repairs stale active-organization pointers during session creation and also carries substantial business-context work from earlier changes:
Confidence Score: 3/5The PR is not yet safe to merge because session creation can still preserve a stale organization on membership-query failure, and a stalled generation can bill the same work twice. The session fallback does not clear the organization until after a successful membership lookup, leaving the intended integrity repair incomplete during database failures. Separately, random billing keys do not survive BullMQ crash recovery, allowing repeated charges for one generation. The two repository-rule violations must also be resolved before merging. Files Needing Attention: packages/auth/src/auth.ts; apps/insights/src/organization-business-context.ts; apps/api/src/routes/agent-business-context.test.ts; apps/dashboard/app/(main)/organizations/components/business-context-editor.tsx Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Create session] --> B{Active organization set?}
B -- No --> E[Find another membership or provision default organization]
B -- Yes --> C[Query active membership]
C -- Found --> D[Keep active organization]
C -- Missing --> F[Clear stale pointer]
F --> E
C -- Query throws --> G[Current code returns original session data]
G --> H[Stale organization may remain]
I[Business-context generation job] --> J[Run model phase]
J --> K[Bill using generation + phase + random UUID]
K --> L{Completion persisted?}
L -- Yes --> M[Publish ready draft]
L -- Worker crashes --> N[Stalled job is processed again]
N --> J
N --> O[New UUID defeats charge deduplication]
Reviews (1): Last reviewed commit: "fix(auth): stop sessions carrying an org..." | Re-trigger Greptile |
| } | ||
|
|
||
| return { data: sessionData }; | ||
| return { data: base }; |
There was a problem hiding this comment.
Stale organization survives errors
If the new membership lookup throws, base still contains the original activeOrganizationId, so the catch returns the stale pointer this change is intended to remove. A transient database failure during session creation can therefore persist another session with an organization the user may have left. Initialize the fallback with a cleared organization before the lookup, and restore the original value only after membership is confirmed.
| let billingFailure: Error | undefined; | ||
| const model = getAILogger().wrap(createModelFromId(MODEL)); | ||
| const options = (phase: string) => { | ||
| const key = `org-business-context:${input.generationId}:${phase}:${randomUUID()}`; |
There was a problem hiding this comment.
The billing idempotency key contains a fresh randomUUID(), so processing the same stalled generation again produces a different key. If a worker charges successfully and crashes before saving completion, the still-running job can be processed again and charged under the new key. Use a stable key derived from the generation and phase so the billing provider can deduplicate crash recovery.
| profile: null as OrganizationBusinessProfile | null, | ||
| read: vi.fn(), | ||
| prompts: [] as Parameters<MockLanguageModelV3["doStream"]>[0][], | ||
| contexts: [] as Record<string, unknown>[], |
There was a problem hiding this comment.
Prohibited unknown types added
The new test uses Record<string, unknown> instead of an explicit context type, contrary to the repository directive not to use any, unknown, or never. The same pattern appears in the changed createConfig parameter at line 112 and model wrapper at line 148. Define explicit model-context and input types before merging.
Context Used: Basic guidelines for the project so vibe coders do... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| {review === "generation" | ||
| <Dialog.Title | ||
| render={(props) => ( | ||
| <h2 {...props} ref={reviewTitleRef} tabIndex={-1}> |
There was a problem hiding this comment.
This assigns tabIndex to a non-interactive <h2>, violating the repository directive not to put tabIndex on non-interactive elements. Use the dialog primitive's supported focus target instead. This repository requirement must be satisfied before merging.
Context Used: Ultracite Rules - AI-Ready Formatter and Linter (source)
Found by a referential-integrity sweep of production, not by a report.
The bug
session.create.beforereturned early wheneveractiveOrganizationIdwas set, without checking the user was still a member of it. So a user removed from an organization keeps that pointer on every subsequent session, and the orphan-recovery path added ina2b16e519never runs for them.It is also security-adjacent: Better Auth falls back to
activeOrganizationIdwheneverhasPermissionis called without an explicitorganizationId. A session advertising an organization the user has no membership in is exactly the condition our permission rules warn about.Production evidence
The sweep found 7 users with no organization membership at all (all real accounts, all email-verified) and 21 organizations with zero members, which between them own 7 websites still collecting data nobody can reach.
One user signed up on Dec 17 and was still signing in on 2026-08-28. Their sessions on Mar 26, Jul 27, Jul 28, Aug 10 and Aug 13 all point at an organization they are not a member of. Recovery was skipped every time.
What I ruled out first
The fix
Verify membership before trusting the pointer, clear it when the membership is gone, and thread the cleared value through every remaining branch including the catch — otherwise a throw after detection hands back a session still carrying it.
33 packages typecheck, lint clean.
Not included
The 7 orphaned websites in memberless organizations are a data-recovery decision (reassign or archive), not a code fix.
Also carries #765, #766 and #768, already reviewed.
Summary by cubic
Fixes a session bug where a user who left an organization kept that organization on future sessions, and includes previously reviewed business-context improvements.
activeOrganizationId, clears it when membership is gone, and carries the cleared value through every branch including the catch.Written for commit 4422cd6. Summary will update on new commits.