[Feat]: Per-LLM-call credit quota with admin UI (issue #15)

[FireTable]

Closes #15

Problem

The chat agent has no usage cap — a single runaway session can rack up unbounded OpenAI spend with no operator visibility, and users have no per-account budget to constrain themselves with.

What changed

Credit cap (enforcement + log)

• app/api/[..._path]/route.ts — proxy now withAuth-wrapped (anonymous traffic → 401) and runs checkCredit(user.id) on POST/PUT/PATCH. Over-cap → synthesizes an SSE stream carrying a show_credit_card tool_call, no LangGraph round-trip.
• lib/credit/check.ts — UTC-aligned rolling-window SUM over credit_usage_log; admin role short-circuits to unlimited.
• lib/credit/callback.ts — CreditTrackingHandler (BaseCallbackHandler) is the only writer of credit_usage_log. Singleton in backend/callbacks.ts, wired into both agent and background_agent graphs.
• lib/credit/{schema,charge,build-model,zod,errors,invoke,status}.ts — schema, computeCredits math, findProviderId/getModelRate callback helpers, Zod, dead CreditExceededError (kept for symmetry), client-side loadCreditStatus (1s TTL + in-flight collapse).

Provider registry (DB-backed, replaces env-only model lookup)

• lib/provider/{schema,admin,model-registry}.ts — provider table holds encrypted API keys (AES-256-GCM) + per-model rates. getChatModelFromDB returns a ChatOpenAI; getChatModel falls back to env on DB miss. LRU + 60s cross-process TTL.
• backend/model.ts — rewired through getChatModel.

Admin UI

• app/admin/page.tsx + app/admin/admin-tabs.tsx — three tabs (Providers / Roles / Users), FormDialog + DropdownMenu primitives, sentence-case copy, last-admin guard, default-provider protection, router.refresh() after mutations.
• app/api/admin/{providers,roles,users}/**/route.ts — full CUD; 409 PROTECTED on DELETE /api/admin/providers/default, 409 LAST_ADMIN on demote/ban/delete of only admin, 409 DUPLICATE_KEY/DUPLICATE_MODEL on name collisions, 409 ROLE_IN_USE on deleting referenced role.

Auth + ban

• lib/auth/encryption.ts — loadKek, aesGcmEncrypt/Decrypt, deriveKeyName (first-3 + ellipsis + last-4), reads LLM_KEY_ENCRYPTION_KEY lazily per operation.
• lib/auth/with-auth.ts — withAuth({ role }, handler) overload (rule [Feat]: Email verification has no result page between verify and login redirect #9). Exact role match; FK-corrupt roleId falls back to "user" so admin routes still reject.
• lib/auth/config.ts — databaseHooks.session.create.before blocks new sessions for banned users.
• db/migrations/0004_nappy_bloodstorm.sql — user.banned column.
• app/api/admin/users/[id]/route.ts — PATCH banned:true DELETEs every session row in the same handler for immediate cutoff.

Other

• app/api/credit/{status,history}/route.ts — user-facing cap state + paginated call history.
• components/{credit,auth,settings}/ — CreditProgress, CreditHeader, CreditSummaryCard, CreditUsageSlot, CreditHistory (useInfiniteQuery), Settings → Credits tab.
• components/tool-ui/credit/credit-card.tsx — proxy-injected show_credit_card render.
• backend/callbacks.ts — lifted out of observability/ so agent + background_agent share one CapturingHandler + one CreditTrackingHandler.

Tests

pnpm test — 868 / 868 pass (87 files). Tests cover: proxy credit gate, callback lifecycle, encryption round-trip, role gate overload, admin CUD including conflict + last-admin paths, history pagination, model-registry LRU + cache invalidation, schema.

Docs

Per CLAUDE.md rule #1 / #10 / engineering rule 1, all in this PR:

• docs/APIS.md — new Admin (Providers / Roles / Users), Credit history (/api/credit/*), and full Proxy section covering auth gate + credit gate + userId injection + header forwarding + OPTIONS + streaming passthrough.
• docs/ADMIN.md — full route reference + Secrets handling + Provider shape + Role management + Bootstrap admin + UI.
• docs/CREDIT.md (new) — roles & caps, UTC-aligned window, credit math, where the cap is enforced (proxy), where the log is written (callback), metadata contract, failure modes, user APIs.
• docs/AUTH.md — withAuth role gate overload, LLM_KEY_ENCRYPTION_KEY + INITIAL_ADMIN_EMAIL env vars, last-admin / ban mechanics.
• docs/DEPLOY.md + skills/langgraph-app-maintain.md — LLM_KEY_ENCRYPTION_KEY + INITIAL_ADMIN_EMAIL prerequisites + reset scenarios.
• docs/DB.md — role, provider, credit_usage_log tables; UTC-anchored window model; banned column; base_url column; Code → table map.
• docs/PROVIDERS.md (new) — DB-backed registry, LRU + 60s cross-process TTL tradeoff, seeded default row, env fallback.
• docs/TOOLS.md — Credit tool group row + show_credit_card entry (client-only render).
• docs/TODOS.md — deleted (open follow-ups now live as GitHub issues / TODO comments).
• README.md — Experience section, credit/admin features, refreshed env table, project layout, Database section.
• package.json — description, keywords, homepage, bugs, author, repository.

🤖 Generated with Claude Code

---

Summary by cubic

Adds per-user, rolling-window credit caps for all LLM calls and an admin console to manage providers, model rates, roles, and users. Implements issue #15; includes guards, security hardening, and doc updates.

• New Features

  • Enforce per-role credit caps at the HTTP proxy for POST/PUT/PATCH; over-cap runs stream a show_credit_card tool; added /api/credit/{status,history} and a Settings → Credits view.
  • Log every LLM call to credit_usage_log via a shared CreditTrackingHandler; credits derived from per-model rates; registered on chat and background graphs.
  • DB-backed provider registry with AES-256-GCM–encrypted API keys, per-model rates, baseUrl on the provider, and a 60s LRU; agents resolve chat models through the registry with env fallback; admin CUD busts the cache.
  • Admin console for Providers/Models/Keys, Roles (limit/window), and Users (role edit, ban/unban with immediate session cutoff); guards for default-provider delete and last-admin demote/delete; typed 409s on ID/model conflicts; fixes unban of the sole admin.
  • Security/UX: strip apiKeys from provider PATCH (keys must go through /keys encrypt path); withAuth role gate overload; user dropdown shows credit usage; docs updated (required baseUrl, 409 PROTECTED on default provider delete, keys route body).

• Migration

  • Run DB migrations (adds role, provider, credit_usage_log, and user.banned; seeds guest/user/admin; moves baseUrl to provider).
  • Set LLM_KEY_ENCRYPTION_KEY (32-byte hex) and INITIAL_ADMIN_EMAIL.
  • Seed/configure a provider and model rates in /admin (env fallback works until the registry is set).

^{Written for commit aaa75ea. Summary will update on new commits.}

Greptile Summary

This PR introduces per-role rolling-window credit caps on every LLM call, a DB-backed provider registry with AES-256-GCM encrypted API keys, and a full admin console (Providers / Roles / Users) with last-admin and default-provider guards. It is a large, well-tested feature (868 passing tests) with solid architectural choices — the proxy-level gate, callback-only write path, and withAuth role overload are all clean.

• Credit enforcement: checkCredit is called in the proxy before every POST/PUT/PATCH; over-cap requests synthesize a fabricated show_credit_card SSE stream without touching LangGraph, and CreditTrackingHandler writes credit_usage_log entries after every successful or failed LLM call.
• Provider registry: getChatModelFromDB resolves models via a 60s LRU cache; backend/model.ts wraps it with an env-var fallback. Encrypted keys are managed through /api/admin/providers/[id]/keys, though the PATCH /api/admin/providers/[id] endpoint exposes an apiKeys field in its schema that bypasses the encryption flow.
• Admin API: Three resource groups (providers, roles, users) with guards for the default provider and last admin, but the last-admin count check and update run in separate non-transactional steps, creating a TOCTOU window.

Confidence Score: 3/5

The core credit gate and callback accounting work correctly, but four defects in the new admin and credit-check paths need fixing before this ships.

The credit enforcement path is sound and well-tested. Four defects need fixing: checkCredit throws a TypeError rather than returning a graceful result when the user role FK is missing; the provider POST surfaces a raw DB error on duplicate IDs; the last-admin guard in user PATCH/DELETE is a read-then-write without a transaction, allowing two concurrent demotions to clear all admins; and the PATCH provider endpoint lets an admin write unencrypted key material directly to the DB. None affect the happy path in tests, but all are triggered by realistic edge-case inputs or concurrent requests.

lib/credit/check.ts (crash on empty result), app/api/admin/providers/route.ts (duplicate ID unhandled), app/api/admin/users/[id]/route.ts (TOCTOU in last-admin guard), app/api/admin/providers/[id]/route.ts (raw apiKeys in PATCH schema)

Security Review

• Unencrypted API key storage via PATCH: PATCH /api/admin/providers/[id] accepts apiKeys through providerPatchSchema and passes the array directly to db.update() without routing through encryptApiKey. Plaintext sent as encryptedKey is stored verbatim; failure only surfaces at decryption time.
• No hardcoded secrets, credential leakage in logs, or injection vectors identified.
• AES-256-GCM implementation in lib/auth/encryption.ts is correct: random 12-byte IV per encryption, auth-tag appended, GCM tag mismatch propagates as a throw.

Important Files Changed

Filename	Overview
lib/credit/check.ts	Rolling-window credit cap query; crashes with TypeError on empty JOIN result (missing null guard)
app/api/admin/providers/route.ts	Admin provider list + create; POST lacks duplicate-ID guard, propagates DB constraint violation as unhandled 500
app/api/admin/users/[id]/route.ts	User PATCH/DELETE with last-admin guard; TOCTOU race between count check and update allows stranding 0 admins
app/api/admin/providers/[id]/route.ts	Provider PATCH/DELETE; PATCH schema exposes apiKeys field that bypasses encryption, allowing plaintext keys in DB
lib/auth/encryption.ts	New AES-256-GCM encrypt/decrypt helpers with lazy KEK loading; implementation is correct
lib/auth/with-auth.ts	Adds role-gate overload to withAuth; FK-corrupt sessions fall back to user safely
lib/credit/callback.ts	CreditTrackingHandler wires LLM call logging; runId map correctly spans start/end/error callbacks
app/api/[..._path]/route.ts	Proxy now auth-gated and credit-checked; creditBlockedResponse fabricates valid SSE shape
backend/model.ts	Rewired to DB-backed registry with env fallback; bare catch{} silently swallows all DB errors
lib/provider/model-registry.ts	LRU-cached model resolver with 60s TTL; correct cache-key derivation and invalidation
lib/auth/config.ts	Adds ban check in session.create.before and INITIAL_ADMIN_EMAIL bootstrap; uses APIError correctly
db/migrations/0003_melted_bedlam.sql	Adds role, provider, credit_usage_log tables with seed data and FK

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant Proxy as app/api/[..._path]
    participant CreditCheck as checkCredit()
    participant LangGraph
    participant Callback as CreditTrackingHandler
    participant DB as Postgres

    Client->>Proxy: POST /api/threads/.../runs/stream
    Proxy->>Proxy: withAuth - verify session
    Proxy->>CreditCheck: checkCredit(userId)
    CreditCheck->>DB: SELECT role.creditLimit + SUM(credits) WHERE window
    DB-->>CreditCheck: used / limit
    alt Over cap
        CreditCheck-->>Proxy: "allowed=false"
        Proxy-->>Client: SSE stream with show_credit_card tool_call
    else Under cap
        CreditCheck-->>Proxy: "allowed=true"
        Proxy->>LangGraph: Forward request with userId in config.metadata
        LangGraph->>Callback: handleLLMStart(runId, metadata)
        Callback->>Callback: Cache RunMeta by runId
        LangGraph->>LangGraph: Model invocation
        LangGraph->>Callback: handleLLMEnd(output, runId)
        Callback->>DB: findProviderId + getModelRate
        Callback->>DB: INSERT credit_usage_log
        LangGraph-->>Proxy: SSE stream
        Proxy-->>Client: SSE stream passthrough
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant Proxy as app/api/[..._path]
    participant CreditCheck as checkCredit()
    participant LangGraph
    participant Callback as CreditTrackingHandler
    participant DB as Postgres

    Client->>Proxy: POST /api/threads/.../runs/stream
    Proxy->>Proxy: withAuth - verify session
    Proxy->>CreditCheck: checkCredit(userId)
    CreditCheck->>DB: SELECT role.creditLimit + SUM(credits) WHERE window
    DB-->>CreditCheck: used / limit
    alt Over cap
        CreditCheck-->>Proxy: "allowed=false"
        Proxy-->>Client: SSE stream with show_credit_card tool_call
    else Under cap
        CreditCheck-->>Proxy: "allowed=true"
        Proxy->>LangGraph: Forward request with userId in config.metadata
        LangGraph->>Callback: handleLLMStart(runId, metadata)
        Callback->>Callback: Cache RunMeta by runId
        LangGraph->>LangGraph: Model invocation
        LangGraph->>Callback: handleLLMEnd(output, runId)
        Callback->>DB: findProviderId + getModelRate
        Callback->>DB: INSERT credit_usage_log
        LangGraph-->>Proxy: SSE stream
        Proxy-->>Client: SSE stream passthrough
    end

Loading

Comments Outside Diff (1)

1. lib/credit/check.ts, line 597-605 (link)

   Destructure crash on empty query result

   const [{ creditLimit, windowHours, roleName }] destructures the first element of the query result array. If the user row exists but its roleId FK has no matching role row (the innerJoin returns nothing), the array is empty and the destructure throws TypeError: Cannot destructure property 'creditLimit' of 'undefined'. This turns every blocked user's request into an unhandled 500 rather than a graceful deny.

   The comment in lib/auth/role-queries.ts explicitly flags this scenario as "extremely rare, but defensive" — checkCredit should apply the same guard: check for an undefined result and treat the missing row as a deny or fall back to the default user role limits.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: lib/credit/check.ts
   Line: 597-605
   
   Comment:
   **Destructure crash on empty query result**
   
   `const [{ creditLimit, windowHours, roleName }]` destructures the first element of the query result array. If the user row exists but its `roleId` FK has no matching `role` row (the `innerJoin` returns nothing), the array is empty and the destructure throws `TypeError: Cannot destructure property 'creditLimit' of 'undefined'`. This turns every blocked user's request into an unhandled 500 rather than a graceful deny.
   
   The comment in `lib/auth/role-queries.ts` explicitly flags this scenario as "extremely rare, but defensive" — `checkCredit` should apply the same guard: check for an undefined result and treat the missing row as a deny or fall back to the default `user` role limits.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

Fix the following 6 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 6
lib/credit/check.ts:597-605
**Destructure crash on empty query result**

`const [{ creditLimit, windowHours, roleName }]` destructures the first element of the query result array. If the user row exists but its `roleId` FK has no matching `role` row (the `innerJoin` returns nothing), the array is empty and the destructure throws `TypeError: Cannot destructure property 'creditLimit' of 'undefined'`. This turns every blocked user's request into an unhandled 500 rather than a graceful deny.

The comment in `lib/auth/role-queries.ts` explicitly flags this scenario as "extremely rare, but defensive" — `checkCredit` should apply the same guard: check for an undefined result and treat the missing row as a deny or fall back to the default `user` role limits.

### Issue 2 of 6
app/api/admin/providers/route.ts:15-24
**Unhandled DB constraint violation on duplicate provider ID**

`db.insert(provider).values(parsed.data)` will throw a PostgreSQL primary-key constraint violation if a provider with the same `id` already exists. There is no try-catch and no prior uniqueness check, so the error propagates as an unhandled 500. Every other conflicting-insert path in this PR returns a 409 with a typed code (`DUPLICATE_KEY`, `DUPLICATE_MODEL`) — this one is missing the same treatment.

A duplicate `id` can realistically occur after a failed seed or when an operator re-runs the wizard with the same slug.

### Issue 3 of 6
app/api/admin/users/[id]/route.ts:35-72
**TOCTOU race in last-admin guard**

The guard reads the admin count, checks `c <= 1`, and then issues the UPDATE in separate, non-transactional steps. Two concurrent requests to demote two different admins (when exactly 2 admins exist) would each see `c = 2`, each pass the guard, and each apply their update — leaving 0 admins in the system. The same race exists in the `DELETE` handler.

The fix requires wrapping the SELECT count + UPDATE in a single serializable transaction (or a `SELECT … FOR UPDATE` lock). Given this is an admin-only surface the window is narrow, but two admin tabs acting simultaneously could trigger it.

### Issue 4 of 6
app/api/admin/providers/[id]/route.ts:19-45
**PATCH accepts raw `apiKeys` array, bypassing encryption**

`providerPatchSchema` includes `apiKeys: z.array(providerApiKeySchema).optional()`. A caller sending `PATCH /api/admin/providers/[id]` with a hand-crafted `apiKeys` array that contains plaintext as `encryptedKey` will have that value written to the DB verbatim — `aesGcmDecrypt` will fail at call time with no indication the stored material is unencrypted.

The intended key-management path is the dedicated `/keys` sub-resource which routes through `encryptApiKey`. The PATCH endpoint should strip `apiKeys` from its schema or re-encrypt any provided blobs.

### Issue 5 of 6
backend/model.ts:41-49
**Silent DB error swallowing in `getChatModel`**

The bare `catch {}` silently discards every error from `getChatModelFromDB` — including `KekMissingError`, DB connection failures, and "no enabled provider" errors. Production instances where the DB registry is authoritative will silently fall back to the env key with no log or observable signal; operators won't know the DB path is broken until billing diverges.

### Issue 6 of 6
lib/credit/build-model.ts:59-85
**`findProviderId` does a full-table scan per LLM call without caching**

`findProviderId` is called from `CreditTrackingHandler.handleLLMEnd` on every LLM invocation. It issues `db.select().from(provider)` — fetching all rows — with no caching (the LRU in `model-registry.ts` covers `getChatModelFromDB` only). For a small provider table this is negligible, but it scales linearly with provider count and call volume.

_{Reviews (1): Last reviewed commit: "docs: cover proxy gates + key rotate/ren..." | Re-trigger Greptile}

Greptile also left 5 inline comments on this PR.

Context used:

• Context used - CLAUDE.md (source)

[cubic-dev-ai]

All reported issues were addressed across 121 files

<sub>Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic</sub>

[greptile-apps]

Too many files changed for review. (121 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

[cubic-dev-ai]

All reported issues were addressed across 4 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[claude]

Review — PR #35 (Per-LLM-call credit quota + admin UI)

Solid foundation: 868 tests pass, the proxy-level credit gate + callback-only write path are clean, AES-256-GCM is correctly implemented, and docs are in lock-step with the new endpoints (rule #1, #9, #10). A few issues survived that I would like fixed before merge — one P1 and three P2s. None are happy-path blockers; all are triggered by realistic inputs or concurrency.

P1 — crash turns every blocked-user request into a 500

• lib/credit/check.ts:29-37. innerJoin(role, eq(user.roleId, role.id)) returns [] when the user row's FK is missing/corrupt, and the destructure const [{ creditLimit, windowHours, roleName }] throws TypeError: Cannot destructure property 'creditLimit' of 'undefined'. The same defensive note exists in lib/auth/role-queries.ts; checkCredit needs the same null guard. Severity: P1 because this turns every request from a corrupted user into an unhandled 500 instead of a graceful deny / fallback.

P2 — concurrency

• app/api/admin/users/[id]/route.ts:39-50 and 83-94. The last-admin guard is read-then-write with no FOR UPDATE lock or transaction. Two concurrent admin tabs demoting two different admins when exactly 2 admins exist both see c = 2, both pass the guard, and strand the system with 0 admins. Same race on DELETE.
• lib/credit/build-model.ts:71-87. findProviderId runs db.select().from(provider) on every LLM call (in CreditTrackingHandler.handleLLMEnd / handleLLMError). No LRU covers it (the cache in model-registry.ts only wraps getChatModelFromDB). Either cache the result by (baseUrl, modelName) or pre-resolve the id once at model-build time.

P3 — observability

• backend/model.ts:42-46. The bare catch {} swallows every error from getChatModelFromDB — KekMissingError, DB connection failures, "no enabled provider". A DB-broken prod env silently falls back to the env key with no signal. At minimum console.warn (or surface to the existing observability layer); ideally narrow the catch to the actual "DB miss → env fallback" branch.

Already fixed in the current diff (not current issues)

• Greptile flagged "PATCH /api/admin/providers/[id] accepts raw apiKeys array bypassing encryption" — but providerPatchSchema (= providerNoDefaults in lib/credit/zod.ts) intentionally strips apiKeys, and the keys flow through the dedicated /keys sub-resource + encryptApiKey. Confirmed not a current issue.
• Greptile flagged "POST provider doesn't handle duplicate IDs" — but onConflictDoNothing({ target: provider.id }) + the if (!row) branch returns a typed 409 DUPLICATE. Confirmed.

CLAUDE.md compliance

• ✅ Rule feat(001): stage 1 — user auth (Better Auth + email verification + thread ownership) #1 (API docs): docs/APIS.md covers every new endpoint under admin/, credit/, and the full Proxy section. docs/ADMIN.md, docs/CREDIT.md, docs/PROVIDERS.md are in place.
• ✅ Rule [Feat]: Email verification has no result page between verify and login redirect #9 (withAuth-wrapped): every new route.ts is wrapped; the role-gate overload is correctly implemented.
• ✅ Rule [Bug]: Memory tab Socials row missing email per linked provider #10 (lazy third-party key tools): N/A here, but LLM_KEY_ENCRYPTION_KEY is read lazily per operation in lib/auth/encryption.ts.
• ✅ Rule [Feat]: chat attachments backed by Cloudflare R2 #12 (env): no new NEXT_PUBLIC_*; new client-visible value USER_ROLE_NAME is properly surfaced via window.__CONFIG__.
• ✅ Rule [Feat]: Public landing page at / (replace auto-redirect to /chat) #11 (DB backup before out-of-app mutations): migration runner is the canonical entry point.

Posting inline comments on the three concrete locations.

[cubic-dev-ai]

All reported issues were addressed across 4 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[claude]

Reviewed the PR end-to-end (121 files, +10,100/-361). The credit gate, provider registry, and admin UI are well-shaped — clean separation of concerns, the proxy short-circuits before LangGraph (so blocked turns do not burn tokens), the singleton callback wired into both graphs, AES-256-GCM with random IV + auth tag, role-gate overload on withAuth, immediate ban enforcement via session DELETE + databaseHooks.session.create.before, and a thoughtful seed order in migration 0003 that handles the FK default. Greptile initial flag about PATCH /api/admin/providers/[id] accepting raw apiKeys was already fixed in lib/credit/zod.ts (the providerNoDefaults schema intentionally omits the field) — good call to put the no-defaults contract in code rather than comment.

Eight issues worth fixing before merge, ordered by severity:

1. lib/credit/check.ts:29 — innerJoin + array destructure crashes on FK-corrupt row. lib/auth/role-queries.ts already shows the correct leftJoin + explicit if (!row) guard pattern. Mirror it here; right now a user whose roleId FK has been broken gets a 500 instead of a credit-blocked stream.

2. app/api/admin/users/[id]/route.ts:39 + :82 — TOCTOU race in the last-admin guard. Two concurrent demotions when exactly 2 admins exist both pass c <= 1 and leave 0 admins. Wrap the SELECT + UPDATE in a db.transaction and either lock with FOR UPDATE or push the guard into the UPDATE WHERE clause. Admin surface is narrow but two open tabs hit it.

3. app/api/[..._path]/route.ts:179 — proxy userId-injection drops fields. The if (!parsed.configurable) parsed.configurable = configurable block writes the inner configurable (which only has userId) to the top-level parsed.configurable slot, clobbering any pre-existing top-level configurable.foo. Same for the metadata mirror. The LangGraph SDK can speak both shapes; commit to one or merge {...existing, userId} into whichever slot the request actually used.

4. app/api/admin/providers/[id]/route.ts:36 — no PROTECTED analog for enabled. PATCHing enabled: false on the only enabled provider succeeds; combined with backend/model.ts silent catch {}, LangGraph falls back to the env OPENAI_API_KEY and the credit log stops recording — billing silently diverges from the registry.

5. backend/model.ts:42 — bare catch {} swallows every DB-registry error. KekMissingError (when LLM_KEY_ENCRYPTION_KEY is absent), DB outages, and "no enabled provider" all collapse into the env fallback with no log. Per CLAUDE.md rule 5, narrow to typed errors and at least console.warn.

6. lib/credit/callback.ts:36 — agentName contract drift from the subagent rewrites. Inner model nodes are renamed model -> chatModel/codeModel/cryptoModel/weatherModel, so metadata.langgraph_node will record chatModel/codeModel/etc. for the LLM row, not the parent-agent identity. docs/CREDIT.md § Metadata contract still promises "router" | "crypto" | "summarize" | ... Either thread agentName from the parent invocation or update the doc + DB comment so the recorded value matches what admins expect to see.

7. lib/credit/build-model.ts:71 — findProviderId does an unbounded full-table scan per LLM call. No caching (the model-registry.ts LRU covers getChatModelFromDB only), no WHERE enabled = true filter. Add a small module-scope map invalidated alongside invalidateModelCache() and scope the SELECT to enabled rows.

8. lib/credit/callback.ts:69 — runMeta: Map<string, RunMeta> is unbounded. If handleLLMStart runs but neither handleLLMEnd nor handleLLMError fires (network drop, dropped ref), the entry leaks for the life of the process. Cap with an LRUCache({ max: 1000 }) and document the silent-skip behavior when !this.runMeta.has(runId) at End/Error (callers should know that a leaked Start → no log row).

Cross-checked docs/APIS.md, docs/ADMIN.md, docs/CREDIT.md, docs/AUTH.md, docs/PROVIDERS.md, docs/DB.md — they read consistent with the code (incl. the right apiKeys-stripped PATCH contract and the immediate-ban session DELETE). One note that ties to finding #6: docs/CREDIT.md lines 30 and 77 still describe agentName semantics that no longer match what the renamed inner node will produce.

Inline comments attached at each site.

[claude]

Hard crash on role FK mismatch

const [{ creditLimit, windowHours, roleName }] = await db.select(...) destructures index [0], but an innerJoin(user, role) returns [] whenever a user row exists with a roleId that no longer resolves to a row in role (the very scenario lib/auth/role-queries.ts:38-55 documents as "extremely rare, but defensive"). On that path the destructure becomes [{ ... }] = [] → TypeError: Cannot destructure property 'creditLimit' of 'undefined', which the proxy turns into an unhandled 500 for every blocked user (the proxy is the only caller of checkCredit today).

Minimal fix: replace the innerJoin with leftJoin and fall back to the literal { creditLimit: 0, windowHours: 24, name: "Unknown" } (matching the synthetic row role-queries.ts returns), or guard for length === 0 immediately after the query.

Suggested change

	const [{ creditLimit, windowHours, roleName }] = await db
	const rows = await db
	.select({
	creditLimit: role.creditLimit,
	windowHours: role.windowHours,
	roleName: role.name,
	})
	.from(user)
	.innerJoin(role, eq(user.roleId, role.id))
	.where(eq(user.id, userId));
	if (rows.length === 0) {
	// FK-corrupt: user exists but role row is missing. Apply the same
	// defensive fallback as lib/auth/role-queries.ts so the gate fails
	// closed (deny) instead of throwing TypeError through the proxy.
	return {
	allowed: false,
	used: 0,
	limit: 0,
	windowHours: 24,
	resetAt: new Date(),
	roleName: "Unknown",
	};
	}
	const [{ creditLimit, windowHours, roleName }] = rows;

No test covers this branch — please add one (rule #2).

[claude]

TOCTOU race in last-admin guard — PATCH + DELETE

The guard performs a non-transactional read of count(*) then issues the UPDATE/DELETE in a separate round-trip. With exactly two admins and two simultaneous demotion requests (PATCH /users/adminA + PATCH /users/adminB), both requests see c === 2, both pass c <= 1, both apply — leaving 0 admins. The same race exists in the DELETE handler at lines 82-90.

Concrete failure scenario is easy to reach: two open admin tabs triggering the same flow on a slow Postgres, or any retry/backoff loop running both calls.

Fix: wrap count + mutate in db.transaction(async (tx) => { ... }) with SERIALIZABLE, or lock the row(s) being mutated with SELECT ... FOR UPDATE before the count. The transaction must also include the session DELETE on banned: true for the PATCH case so a concurrent ban can't slip a new session in.

Tests (tests/api/admin/users.test.ts:131-152) only cover the serial single-admin path — rule #2 asks for the concurrent scenario to be exercised.

[claude]

Bare catch {} swallows every DB-side failure

getChatModel is the canonical entry point (per the doc-comment on lines 28-38), but a bare catch {} masks every error from getChatModelFromDB — KekMissingError, Postgres going away, "no enabled provider", FK corruption in provider. The fallback to buildEnvModel() always runs, so a misconfigured production deploy with LLM_KEY_ENCRYPTION_KEY set but the DB registry as source-of-truth will silently switch to process.env.OPENAI_API_KEY with no log line. Billing then diverges because CreditTrackingHandler keeps writing to credit_usage_log against whichever key actually served the request, and an operator has no signal until something else breaks.

Minimal fix: re-throw non-recoverable errors (env fallback impossible, KekMissingError), and console.warn the swallowed ones so the operator has something.

Suggested change

	} catch {
	export async function getChatModel(opts: GetChatModelOpts = {}): Promise<ChatOpenAI> {
	try {
	return (await getChatModelFromDB(opts)) as ChatOpenAI;
	} catch (err) {
	if (err instanceof KekMissingError) {
	// Env fallback depends on OPENAI_API_KEY; surface so the deploy
	// doesn't silently regress to an untracked billing source.
	console.warn("provider registry unavailable:", err.message);
	}
	if (!process.env.OPENAI_API_KEY) throw err;
	return buildEnvModel();
	}
	}

Add a KekMissingError import too.

[claude]

Two uncached DB queries per successful LLM call

findProviderId does db.select().from(provider) (line 75 — full table scan) and getModelRate does db.select().from(provider).where(eq(provider.id, providerId)) (line 56). Both run on every handleLLMEnd, and the LRU in lib/provider/model-registry.ts only covers getChatModelFromDB itself — it does NOT cover this callback path, so the credit-callback misses the cache entirely.

For a single-tenant MVP the provider table is tiny so this is not a real perf issue today, but the callback is the accounting-path hot loop (every model call hits it twice). Two cheap improvements: (1) cache a snapshot of provider[] keyed off invalidateModelCache() (the existing CUD invalidation already calls it from every admin route), and (2) make findProviderId+getModelRate share a single SELECT instead of two round-trips — the provider row is identical for both.

Suggested change

	const rows = await db.select().from(provider);
	export async function findProviderId(opts: {
	baseUrl?: string | null;
	modelName?: string;
	}): Promise<string | null> {
	const rows = await listProvidersCached(); // single shared snapshot
	if (opts.baseUrl) {
	for (const row of rows) {
	if (row.baseUrl === opts.baseUrl) return row.id;
	}
	}
	if (opts.modelName) {
	for (const row of rows) {
	if (row.models.some((m) => m.name === opts.modelName && m.enabled)) return row.id;
	}
	}
	return null;
	}

listProvidersCached would live next to invalidateModelCache in lib/provider/model-registry.ts so admin writes drop the cache as today.

[claude]

Documented enforcement site doesn't exist

Per the comment block (lines 43-65) and docs/CREDIT.md, enforcement "lives in backend/model.ts's credit-aware wrapper" because handleLLMStart "swallows throws". But backend/model.ts:39-45 is just the env-fallback wrapper — there is no credit-aware wrapper, and the proxy at app/api/[..._path]/route.ts:130-135 is the only enforcement point today. Meanwhile lib/credit/errors.ts:1-17 and lib/credit/invoke.ts:8-13 document CreditExceededError as "Thrown by CreditTrackingHandler.handleLLMStart", and grep over the codebase shows no instanceof CreditExceededError catch in any agent / node file — so creditExceededReply is dead code.

Pick one of:

1. Wire the wrapper: getChatModel should await checkCredit(userId) (it has opts already, but needs userId from metadata — easy via the resolved runtime) and throw CreditExceededError before returning the model. Then add try { ... } catch (err) { if (err instanceof CreditExceededError) return { messages: [creditExceededReply(err)] }; throw err; } to the four nodes (router-agent-node.ts, chat-agent.ts, code-agent.ts, crypto-agent.ts, weather-agent.ts, thread-summarize-node.ts, rename-thread-agent-node.ts).
2. Delete CreditExceededError, creditExceededReply, and the misleading comments — the proxy gate is sufficient because all SDK traffic goes through the /api/[..._path] proxy.

Right now CreditTrackingHandler.handleLLMStart calls super.handleLLMStart only via BaseCallbackHandler's default, which doesn't throw, and the proxy is the sole gate. The docs/code/comments must agree.

[claude]

Review summary

Nice feature, well-tested at the happy paths, and CLAUDE.md rules are mostly satisfied (withAuth on every route except the Better Auth catch-all — rule #9, APIS.md/TOOLS.md/.env.example kept in sync — rules #1/#10/#12, no new NEXT_PUBLIC_*). Five real issues survived my pass; one is a docs/implementation mismatch that would surprise an operator.

Must fix before merge

1. lib/credit/check.ts:29 — const [{ creditLimit, ... }] = await db.innerJoin(user, role) throws TypeError for any user whose roleId has no matching row in role. The proxy turns that into a generic 500 instead of the intended "blocked" reply. lib/auth/role-queries.ts:38-55 already handles the same FK-corruption case with a synthetic default — checkCredit should mirror it.
2. app/api/admin/users/[id]/route.ts:42 and :83 — last-admin guard runs as a read-then-write with no transaction. Two concurrent demotions when exactly two admins exist both pass (c === 2), both update, and the system is left with zero admins. Same race in DELETE. Wrap the count + mutate pair in a serializable transaction (or SELECT ... FOR UPDATE).
3. backend/model.ts:42 — bare catch {} swallows every error from getChatModelFromDB, including KekMissingError. Production deploys with the DB registry as source-of-truth silently fall back to process.env.OPENAI_API_KEY with no log line — operators don't notice until billing diverges. Warn-log or re-throw non-recoverable cases.
4. lib/credit/build-model.ts:75 — findProviderId (full SELECT * FROM provider) and getModelRate (SELECT ... WHERE id = ?) both fire on every handleLLMEnd, neither is cached. The 60s LRU in model-registry.ts covers the other code path and not this. Cheap fix: cache a provider[] snapshot in model-registry.ts with the existing invalidateModelCache() bust, and combine into one SELECT.
5. Docs ↔ code lie — lib/credit/callback.ts:56-60 says enforcement "lives in backend/model.ts's credit-aware wrapper" because handleLLMStart swallows throws. No such wrapper exists (backend/model.ts:39-45 is just env-fallback). lib/credit/errors.ts + lib/credit/invoke.ts describe CreditExceededError as thrown from handleLLMStart, but grep -r 'CreditExceededError' backend/ returns nothing — creditExceededReply is dead code. Either wire the wrapper or delete the class + the misleading comments; today the proxy gate at app/api/[..._path]/route.ts:130-135 is the sole enforcement point.

Test coverage (rule #2)

None of the five issues above has a failing test that's currently exercised:

• tests/lib/credit/check.test.ts has no FK-missing path.
• tests/api/admin/users.test.ts:131-152 runs the last-admin guard serially, not concurrently.
• tests/lib/credit/build-model.test.ts doesn't exist.
• No test for the credit-aware wrapper (because it doesn't exist).

Notes (not blocking, FYI)

• lib/credit/status.ts:23-25 exports peekCachedStatus but nothing else in the tree calls it; loadCreditStatus recomputes via the network even if the cache is fresh through other state mutations. Either consume it or delete it.
• app/api/[..._path]/route.ts:175-185 sets userId into both parsed.config.configurable / parsed.config.metadata and conditionally into the top-level parsed.metadata / parsed.configurable. Only the first matters to the LangGraph SDK; the top-level writes look defensive but conflict with caller-supplied values. Worth tidying.
• Inline comments with concrete fix snippets are on the specific files.