feat: Google OAuth via ZeroID + cross-session usage analytics panel - #48
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR changes OAuth to a server-side ZeroID token exchange with Google login, adds usage analytics data collection, protocol support, client state, and sidebar UI, and updates artificial streaming finalization in session persistence. ChangesGoogle OAuth via ZeroID RFC 8693 token exchange
Usage analytics reporting and sidebar UI
Session artificial streaming finalization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
==========================================
- Coverage 81.07% 80.72% -0.35%
==========================================
Files 55 56 +1
Lines 7597 7895 +298
==========================================
+ Hits 6159 6373 +214
- Misses 1438 1522 +84
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.ts (1)
306-310: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep Google OAuth credentials in the validated config contract.
loadConfignow gates OAuth from rawenv.GOOGLE_*, butOAuthSchemaFieldsdoes not validate or preserve those values. This breaks config-file support and makesopts.envdiverge fromserver.ts’s laterprocess.envreads. Add Google client ID/secret to the Zod schema/env overrides and pass the validated values through the daemon config.As per coding guidelines,
src/config.ts: “Validate configuration from ~/.codeoid/config.json with environment variable precedence.”Suggested direction
const OAuthSchemaFields = z .object({ clientId: z.string().optional(), + googleClientId: z.string().trim().min(1).optional(), + googleClientSecret: z.string().trim().min(1).optional(), }) .default({});- const googleOAuthEnabled = - Boolean(env.GOOGLE_CLIENT_ID) && Boolean(env.GOOGLE_CLIENT_SECRET); + const googleOAuthEnabled = + Boolean(parsed.oauth.googleClientId) && Boolean(parsed.oauth.googleClientSecret);Also add
GOOGLE_CLIENT_ID/GOOGLE_CLIENT_SECRETtoENV_OVERRIDESso env values override file config before this assembly step.Also applies to: 559-568
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/config.ts` around lines 306 - 310, The OAuth config contract in OAuthSchemaFields only validates clientId, so Google OAuth values are lost when loadConfig assembles opts.env. Extend OAuthSchemaFields and ENV_OVERRIDES to include GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET, then thread the validated values through the config assembly so config.json values are preserved with env precedence and available to the daemon config.Source: Coding guidelines
🧹 Nitpick comments (1)
src/tests/config.test.ts (1)
292-304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover partial Google credential env cases.
The gate requires both values, but the test only covers none vs both. Add one-missing assertions so a future
||regression doesn’t silently enable a broken OAuth handler.Test coverage addition
it("populates oauth only when Google credentials are set", () => { const none = loadConfig({ configPath, env: {} }); expect(none.oauth).toBeUndefined(); + + expect( + loadConfig({ configPath, env: { GOOGLE_CLIENT_ID: "client-id.apps.googleusercontent.com" } }).oauth, + ).toBeUndefined(); + expect( + loadConfig({ configPath, env: { GOOGLE_CLIENT_SECRET: "secret" } }).oauth, + ).toBeUndefined(); const viaEnv = loadConfig({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tests/config.test.ts` around lines 292 - 304, The OAuth config test only covers the cases where both Google credentials are absent or both are present, so it can miss a regression in the gate logic. Update the loadConfig-based assertions in config.test around oauth to also cover partial env inputs where only GOOGLE_CLIENT_ID or only GOOGLE_CLIENT_SECRET is set, and verify oauth stays undefined in those one-missing cases while still confirming the existing full-credentials path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/daemon/oauth.ts`:
- Around line 34-39: The LocalProvider fallback path is stale because the oauth
flow still exposes local-session support while `#exchangeForZeroIDToken` always
fails when rawIdToken is missing. Update OauthDaemon and related
LocalProvider/session-token code to either remove the local-provider config/docs
and constructor default, or implement the local session token fallback so local
login can succeed without rawIdToken; make sure the behavior and comments around
localSessionSecret, LocalProvider, and `#exchangeForZeroIDToken` are consistent.
- Around line 238-240: The token exchange flow in `OAuthDaemon` is dropping
request context needed by ZeroID. Update `#exchangeForZeroIDToken`, and the
callers around the `accessToken` exchange path, to propagate both
`PendingAuth.scope` and `OAuthConfig.clientId` through to the ZeroID request
payload so the exchanged token carries the requested scopes and client identity.
Ensure the same fields are forwarded in the additional exchange call site
referenced by the review so both paths stay consistent.
- Around line 251-254: The OAuth callback redirect currently appends the bearer
token to the callback URL query string, which exposes it via history, logs, and
referers. Update the callback handling in oauth.ts to avoid putting accessToken
in search params: use a fragment-based handoff or a one-time server-side
exchange, and keep the existing state/redirect flow intact in the callback
redirect logic and the related auth completion paths.
- Around line 283-287: The ZeroID token exchange in the oauth callback path uses
fetch with no abort handling, so it can hang indefinitely after pending state is
consumed. Update the token request in the oauth.ts flow around the fetch call to
use an AbortController with a timeout, and ensure the request is canceled and
handled cleanly if the timeout elapses. Keep the change localized to the token
exchange logic so the existing callback flow continues to use the response path
and error handling.
In `@src/daemon/server.ts`:
- Around line 106-111: The Google OAuth setup in the daemon is reading
credentials directly from process.env instead of the loaded config, which
bypasses the validated config source. Update the GoogleOAuthProvider
construction in server.ts to consume the client ID and client secret from
config.oauth (or the corresponding config object returned by loadConfig) rather
than process.env, so programmatic config and env-precedence behavior remain
consistent with config.ts validation.
In `@web/src/lib/auth.ts`:
- Around line 70-77: The token resolution in resolveToken is letting a persisted
OAuth token from STORAGE_KEY_TOKEN win whenever opts.apiKey is missing, which
can shadow a saved API key. Update the logic so a stored API key from
STORAGE_KEY_API_KEY is preferred for reconnects, and only fall back to the
stored OAuth token when no API key exists; keep the behavior aligned with
resolveToken({ zeroidUrl }) and the returned { token, exchanged } shape.
---
Outside diff comments:
In `@src/config.ts`:
- Around line 306-310: The OAuth config contract in OAuthSchemaFields only
validates clientId, so Google OAuth values are lost when loadConfig assembles
opts.env. Extend OAuthSchemaFields and ENV_OVERRIDES to include GOOGLE_CLIENT_ID
and GOOGLE_CLIENT_SECRET, then thread the validated values through the config
assembly so config.json values are preserved with env precedence and available
to the daemon config.
---
Nitpick comments:
In `@src/tests/config.test.ts`:
- Around line 292-304: The OAuth config test only covers the cases where both
Google credentials are absent or both are present, so it can miss a regression
in the gate logic. Update the loadConfig-based assertions in config.test around
oauth to also cover partial env inputs where only GOOGLE_CLIENT_ID or only
GOOGLE_CLIENT_SECRET is set, and verify oauth stays undefined in those
one-missing cases while still confirming the existing full-credentials path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 53a26cbf-2e5b-4544-993c-2a564a03052b
📒 Files selected for processing (8)
src/config.tssrc/daemon/identity-provider.tssrc/daemon/oauth.tssrc/daemon/server.tssrc/tests/config.test.tsweb/src/App.tsxweb/src/components/SignIn.tsxweb/src/lib/auth.ts
saucam
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/daemon/memory/store.ts`:
- Around line 424-425: The day-range query in the store aggregation is using a
rolling seconds-based cutoff, which can produce a partial extra bucket when
grouped by calendar day. Update the logic in the query used by the memory store
aggregation so the cutoff is aligned to calendar days before grouping by date,
ensuring the buckets returned by the query match the padded date range expected
by the panel. Keep the fix localized around the date grouping/query path in the
store methods that build this report.
In `@src/daemon/session-manager.ts`:
- Around line 1306-1333: The `#usageDaily` handler in SessionManager is returning
store-wide daily and lifetime aggregates without checking the caller’s auth
context. Update this method to inspect auth from the incoming ClientMessage and
scope the lookup to the authenticated caller before calling
this.#memory.store.dailyUsage and this.#memory.store.lifetimeTotals. If the
caller is not authorized for the requested tenant/workspace, return an
appropriate error response instead of aggregated data.
In `@web/src/components/AnalyticsPanel.tsx`:
- Around line 67-82: The bar rendering logic in AnalyticsPanel should not force
a minimum height for zero-cost buckets; remove the hardcoded 2px floor in the
barH calculation so genuine zero-spend days render as zero-height bars. Update
the height/x/y usage in the same bar rendering block to preserve visibility only
for nonzero values, while keeping the existing bucket, maxCost(), and CHART_H
logic intact.
- Line 27: The AnalyticsPanel logic is re-fetching from a reactive createEffect
because fetchAnalytics() reads loading() before its first await, causing the
effect to subscribe and rerun on each setLoading change. Move the initial fetch
out of createEffect and trigger it in a non-reactive lifecycle or event path,
keeping fetchAnalytics() from depending on tracked state before its first await.
Also update the bar rendering in AnalyticsPanel so zero-usage days do not use
Math.max(2, ...); render them as zero-height or handle them with a separate
empty-state treatment instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 21335c7a-33d9-403b-bada-dc0d6b0917b4
📒 Files selected for processing (7)
src/daemon/memory/store.tssrc/daemon/session-manager.tssrc/protocol/types.tsweb/src/components/AnalyticsPanel.tsxweb/src/components/SessionListPane.tsxweb/src/protocol/types.tsweb/src/state/analytics.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/lib/auth.ts (1)
188-194: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBind the OAuth callback to a per-login state.
web/src/lib/auth.ts:188-194starts/auth/authorizewithout a nonce, andsrc/daemon/oauth.ts:386-422stores#token=...from the fragment without checking any returnedstate. Generate/store astatehere, pass it through the authorize request, and refuse to persist the token unless the callback echoes the same value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/lib/auth.ts` around lines 188 - 194, Bind each OAuth login attempt to a unique state value in startOAuthLogin by generating and storing a per-login nonce, adding it to the /auth/authorize request, and ensuring the callback path in src/daemon/oauth.ts only accepts and persists the `#token` when the returned state matches the stored one. Update the callback handling around the token persistence logic to validate and clear the expected state before writing anything, and refuse to store the token if the state is missing or mismatched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web/src/lib/auth.ts`:
- Around line 188-194: Bind each OAuth login attempt to a unique state value in
startOAuthLogin by generating and storing a per-login nonce, adding it to the
/auth/authorize request, and ensuring the callback path in src/daemon/oauth.ts
only accepts and persists the `#token` when the returned state matches the stored
one. Update the callback handling around the token persistence logic to validate
and clear the expected state before writing anything, and refuse to store the
token if the state is missing or mismatched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 09c63e1b-7223-48e8-8dd2-605e0bc80cbd
📒 Files selected for processing (4)
src/config.tssrc/daemon/oauth.tssrc/daemon/server.tsweb/src/lib/auth.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/daemon/server.ts
- src/config.ts
- src/daemon/oauth.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/daemon/session.ts`:
- Around line 2141-2148: The scrollback byte accounting is happening after
`msg.content` and `msg.parts` are already mutated, so
`ScrollbackBuffer.updateMessage` sees no size change when called with the no-op
updater. Update the `Session` flow in `src/daemon/session.ts` so the
content/parts mutation happens inside the `updateMessage(msg.messageId, ...)`
callback, letting the buffer measure the old/new sizes correctly before the
transcript append. Keep the `append` call unchanged, but ensure the updater
returns the updated message state rather than mutating `msg` first.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e9537bc-7700-44a4-946e-b7988b96cacb
📒 Files selected for processing (1)
src/daemon/session.ts
Replaces the hmacSecret / HS256 auth-code approach with the correct
architecture: Google verifies the human user, ZeroID is the authority
that issues the final RS256 access token, the daemon validates it the
same way it validates any other ZeroID JWT.
Flow:
Browser → /auth/authorize (daemon) → Google OAuth
→ /auth/idp-callback (daemon gets Google id_token)
→ daemon exchanges id_token at ZeroID /oauth2/token
(grant_type=token-exchange, subject_token=<google-id-token>,
account_id + project_id from daemon config)
→ ZeroID validates via Google JWKS (external_issuers config)
→ ZeroID returns RS256 access token
→ daemon sends token to /auth/callback (browser stores, redirects)
→ App.tsx auto-bootstraps from stored ZeroID token on next load
Changes:
- OAuthConfig: drop hmacSecret/issuer, add zeroidTokenEndpoint
- OAuthHandler: #completeAuth now calls ZeroID token-exchange instead
of minting HS256 codes; callbackPage simplified (no fetch needed —
token is delivered server-side, page just stores + redirects)
- identity-provider.ts: VerifiedUser carries rawIdToken for forwarding
- config.ts: OAuth block gated on GOOGLE_CLIENT_ID + GOOGLE_CLIENT_SECRET
(not CODEOID_HMAC_SECRET); reads env correctly for testability
- server.ts: always uses GoogleOAuthProvider when config.oauth is set
- web/src/lib/auth.ts: resolveToken checks stored ZeroID token; adds
rememberedOAuthToken, fetchOAuthProvider, startOAuthLogin (simple
redirect — no client-side PKCE, daemon owns the exchange)
- web/src/App.tsx: auto-bootstrap fires on stored OAuth token too
- web/src/components/SignIn.tsx: "Sign in with Google" button, shown
only when /auth/provider returns "google"
ZeroID config required (external_issuers):
issuer: https://accounts.google.com
jwks_uri: https://www.googleapis.com/oauth2/v3/certs
Closes #42
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a 14-day cost/token bar chart to the session list sidebar, toggled
by a new ≋ button in the header. Pulls from the existing turn_usage
SQLite table via a new usage.daily WebSocket message.
- SqliteEpisodeStore.dailyUsage() — GROUP BY day query over turn_usage
- SqliteEpisodeStore.lifetimeTotals() — all-time aggregate
- usage.daily protocol message → response.ok { daily, lifetime }
- web/state/analytics.ts — Solid signals + fetchAnalytics()
- AnalyticsPanel.tsx — 3 stat tiles + SVG bar chart, zero-padded to 14 days
- SessionListPane: ≋ toggle in header renders the panel
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…omments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fragment-based callback: token passed in URL hash, never sent to server - Forward scope + client_id to ZeroID RFC 8693 token exchange - 10s AbortController timeout on ZeroID fetch - Pass Google credentials through OAuthConfig (config.ts) not process.env - Remove stale LocalProvider fallback (constructor now requires IdP) - Stored OAuth token no longer shadows a saved API key on reconnect Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GET /auth/provider was referenced in web auth.ts but never registered
as a route — always returned 404, so the Google login button never
appeared even when GOOGLE_CLIENT_ID was configured.
- OAuthHandler.handleFetch now handles GET /auth/provider → { provider: <name> }
- DaemonServer returns { provider: null } when #oauthHandler is null
(API-key-only mode), so frontend correctly hides the Google button
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ting, reactive effect - store.ts: fix dailyUsage WHERE clause to use calendar-day cutoff (rolling seconds window produced a partial extra bucket on the boundary day) - store.ts + session-manager.ts: scope dailyUsage/lifetimeTotals to sessions owned by the caller's accountId+projectId so cross-identity data isolation is maintained; pass sessionIds from main store to memory store query - session.ts: fix scrollback byte accounting in artificiallyStreamText — reset msg.content/parts to empty before updateMessage so the buffer measures the correct before/after delta; apply final values inside the updater callback - AnalyticsPanel.tsx: move fetchAnalytics from createEffect to onMount so it does not re-run each time the loading signal changes - AnalyticsPanel.tsx: render zero-cost days as zero-height bars (not 2px) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
oauth-handler.test.ts (18 tests) — covers OAuthHandler end-to-end: - GET /auth/provider returns IdP name - GET /auth/authorize: redirect to Google, loopback URI normalization, 400 on bad client_id / redirect_uri - GET /auth/idp-callback: full ZeroID RFC 8693 token exchange with mocked fetch, forwards scope+client_id, token delivered in URL fragment (not query string), error redirect on ZeroID failure / IdP failure / missing state, pending state consumed so replay is rejected - GET /auth/callback: callback landing page (fragment-reads token into localStorage), error page rendered, XSS escaping on error value - Unknown routes return null web/src/lib/auth.test.ts (20 tests) — covers resolveToken precedence and localStorage helpers: - API-key exchange: zid_sk_ key exchanged for JWT, stored key picked up, invalid key / ZeroID error / missing access_token handled - OAuth fallback: codeoid.token in localStorage used when no API key present - Priority: stored API key always beats stored OAuth token - fetchOAuthProvider: "google" / null / 404 / network-error / unknown name - localStorage helpers: rememberApiKey, rememberedApiKey, forgetApiKey (clears both key and OAuth token), rememberedOAuthToken Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
saucam
force-pushed
the
feat/oauth-zeroid-idp-federation
branch
from
July 2, 2026 08:12
113c0f5 to
0c7c833
Compare
…ethods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Jul 2, 2026
Merged
Merged
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
1 participant
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.
Summary
≋button in the Sessions header.Analytics details
SqliteEpisodeStore.dailyUsage(days)—GROUP BY date(created_at/1000, 'unixepoch')overturn_usage; returns daily cost, input/output tokens, turn and session countsSqliteEpisodeStore.lifetimeTotals()— all-time aggregate across all workspacesusage.dailyWebSocket message →response.ok { daily, lifetime }(no separate result message type needed)web/src/state/analytics.ts— Solid signals +fetchAnalytics()AnalyticsPanel.tsx— 3 lifetime stat tiles (cost / turns / tokens) + pure SVG 14-bar chart, zero-padded, today highlighted, no external chart depsTest plan
bun run typecheckpasses (clean)bun test— 609 pass, 0 fail≋button in session list header toggles analytics panel🤖 Generated with Claude Code
Summary by CodeRabbit