-
Notifications
You must be signed in to change notification settings - Fork 1
fix: dev mode uses real session identity for shared Neon DB #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -196,13 +196,23 @@ const DEV_SESSION: AuthSession = { email: "local@localhost" }; | |
| /** | ||
| * Get the current auth session for a request. | ||
| * | ||
| * - In dev mode: always returns { email: "local@localhost" } | ||
| * - In dev mode: checks for a session cookie first (e.g. from Google OAuth), | ||
| * so the real email is used when sharing a DB with production. | ||
| * Falls back to { email: "local@localhost" } if no session cookie. | ||
| * - In production with built-in auth: returns session if cookie is valid | ||
| * - With custom auth (BYOA): delegates to the custom getSession | ||
| */ | ||
| export async function getSession(event: H3Event): Promise<AuthSession | null> { | ||
| if (isDevMode()) return DEV_SESSION; | ||
| if (authDisabledMode) return DEV_SESSION; | ||
| if (isDevMode() || authDisabledMode) { | ||
| // Check for a real session cookie (created by Google OAuth callback) | ||
| // so dev and prod share the same identity on the same DB | ||
|
Comment on lines
+206
to
+208
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π‘ Missing error handling around DB call newly introduced in dev modePreviously How did I do? React with π or π to help me improve.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed β wrapped getSessionEmail() in try/catch, falls back to DEV_SESSION on error. |
||
| const cookie = getCookie(event, COOKIE_NAME); | ||
| if (cookie) { | ||
| const email = await getSessionEmail(cookie); | ||
| if (email) return { email, token: cookie }; | ||
| } | ||
| return DEV_SESSION; | ||
|
Comment on lines
203
to
+214
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π΄ /api/auth/session endpoint still returns DEV_SESSION β client/server identity splitThe How did I do? React with π or π to help me improve.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed β /api/auth/session endpoints now call getSession(event) instead of returning DEV_SESSION directly. |
||
| } | ||
|
|
||
| if (customGetSession) return customGetSession(event); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -235,8 +235,7 @@ export async function getClients( | |
| * checks only that specific account. | ||
| */ | ||
| export async function isConnected(forEmail?: string): Promise<boolean> { | ||
| // In dev mode, check all accounts regardless of owner | ||
| if (forEmail && forEmail !== "local@localhost") { | ||
| if (forEmail) { | ||
| const accounts = await listOAuthAccountsByOwner("google", forEmail); | ||
| return accounts.length > 0; | ||
|
Comment on lines
237
to
240
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π‘ Mail OAuth callback still stores tokens under
|
||
| } | ||
|
|
@@ -264,8 +263,7 @@ export async function getAuthStatus( | |
| accountId: string; | ||
| tokens: Record<string, unknown>; | ||
| }>; | ||
| // In dev mode (local@localhost), show all accounts regardless of owner | ||
| if (forEmail && forEmail !== "local@localhost") { | ||
| if (forEmail) { | ||
| oauthAccounts = await listOAuthAccountsByOwner("google", forEmail); | ||
| } else { | ||
| oauthAccounts = await listOAuthAccounts("google"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π‘ LEARNINGS.md seeded with uppercase path but all readers use lowercase
learnings.mdThe agent prompt,
resource-readscript, and migration all referencelearnings.md(lowercase), but this seedsLEARNINGS.md. SQLite and Postgres path lookups are exact-match, so the seeded default is never found β and fresh databases can end up with two separate learnings files.How did I do? React with π or π to help me improve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different agent's code. Low priority β will address separately.