feat(server): PGlite-mode parity with Postgres for Agent API#940
Conversation
…r.ts) start-local.ts called initLobuGateway() but threw away the returned Hono app, so the embedded gateway's public Agent API (/lobu/api/v1/agents/*), worker gateway, MCP proxy, and bundled API docs were all unreachable in PGlite mode — every call returned 404. server.ts already mounts the same app at /lobu (PR #637); this aligns the PGlite entrypoint. Reproducer: Before: GET /lobu/health -> 404 After: GET /lobu/health -> 200
The lobuApp middleware only hydrated (user, session) from a Better Auth session (cookie or bearer session-token); owl_pat_* personal access tokens were ignored, so every /lobu/api/v1/agents/* call authenticated with a PAT minted by 'lobu token create' (or returned by /api/local-init's device_token) fell through to the unauthenticated path and the embedded authProvider returned null. The qmsum-demo benchmark worked around this by forging a Better Auth session cookie from BETTER_AUTH_SECRET. Extend the middleware to verify Authorization: Bearer owl_pat_* tokens via PersonalAccessTokenService.verify, look up the bound user, and synthesize the same (user, session) shape the Better Auth path produces. The downstream org-context middleware now honours an org id pinned on the PAT (PAT minted for org A must run against org A) before falling back to the user's default membership. This fixes both PGlite and Postgres assemblies — they share this auth path. Reproducer (PGlite): Before: GET /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_...' -> 401 After: GET /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_...' -> 200 After: GET /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_BAD' -> 401
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds exported createLobuAuthBridge() Hono middleware that authenticates requests via Bearer Personal Access Tokens (owl_pat_*) or falls back to Better Auth sessions, pins organizationId for org-scoped PATs, enforces org membership, and mounts the initialized Lobu gateway at /lobu in local development. ChangesPAT authentication and gateway routing
Integration tests for gateway auth bridge
Sequence Diagram(s)sequenceDiagram
participant Client
participant LobuGateway
participant PersonalAccessTokenService
participant UserTable
participant OrgResolver
Client->>LobuGateway: Request with Authorization: Bearer owl_pat_*
LobuGateway->>PersonalAccessTokenService: verify(token)
PersonalAccessTokenService-->>LobuGateway: token record (userId, orgId, status, exp)
LobuGateway->>UserTable: fetch user(userId)
UserTable-->>LobuGateway: user row
LobuGateway->>LobuGateway: synthesize session (expiresAt, sessionId)
LobuGateway->>OrgResolver: pass pinned organizationId
OrgResolver-->>LobuGateway: resolved org context
LobuGateway-->>Client: Request proceeds with user/session/org context
Possibly related PRs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Round-2 codex review of PR #940 surfaced three defects in the PAT bridge added by 10bb63b (`fix(auth): accept owl_pat_ PATs in embedded Agent API auth bridge`). All three live in the same middleware closure, so fix them together in one extracted `createLobuAuthBridge()` factory. #1 (HIGH) — missing tenant-membership check. After PAT verification the bridge synthesised (user, session) and set `organizationId = patInfo.organizationId` without checking the user was still a member of that org. The canonical REST path enforces this at `workspace/multi-tenant.ts:425` and returns 403 `forbidden`. Mirror that: query the `member` table for `(userId, organizationId)`; reject with the same 403 shape when missing. #2 (MED) — cookie precedence over invalid PAT. The original ordering hydrated Better Auth first and only attempted PAT validation inside an `if (!c.get('user'))` guard. A request carrying both a valid session cookie and `Authorization: Bearer owl_pat_<bad>` therefore authenticated as the cookie user and the invalid PAT was never challenged. Reverse the order: when the `Authorization` header carries `Bearer owl_pat_*`, the PAT path is authoritative — failure short-circuits with 401 regardless of cookie. Better Auth only runs when the header is absent or non-PAT. #3 (MED) — null-org PAT silent re-scoping. `personal_access_tokens.organization_id` is `ON DELETE SET NULL`; a PAT minted for a since-deleted org would fall through to `resolveDefaultOrgId(userId)` and silently bind to the user's earliest membership. Treat PATs with `organizationId === null` as invalid on this path and return 401 with a message pointing at `lobu token`. Refactor: extract the bridge from the closure inside `initLobuGateway` into an exported `createLobuAuthBridge()` factory. The behaviour change is what the bullets above describe; the factory exists so the next commit can exercise the bridge from integration tests without bootstrapping the full gateway.
Round-2 codex review of PR #940 noted that existing PAT coverage hits the MCP routes and the token service, but not the embedded /lobu/api/v1/agents/* auth bridge introduced by 10bb63b. Add a focused integration suite that mounts `createLobuAuthBridge` (exported in the previous commit) on a minimal Hono app and exercises every contract the bridge has to honour. 11 tests across four describe blocks: - Happy path: valid PAT → 200, organizationId pinned to the PAT's org. - Rejection cases: unknown hash, expired, revoked, missing owl_pat_ prefix, empty Authorization, non-Bearer scheme — all 401 (with the bridge's `invalid_token` shape on actual PATs, and the test handler's `no-user` shape on tokens the bridge correctly ignores). - Cookie precedence (codex #2): valid session cookie + invalid PAT → 401 invalid_token, not 200 via cookie fallback. - Tenant membership (codex #1): valid PAT for an org the user has been removed from → 403 forbidden, mirroring multi-tenant.ts:425. Plus a defensive variant for a PAT minted against an org the user never joined. - Null org PAT (codex #3): valid PAT whose organization_id was set to NULL after creation (mirrors the ON DELETE SET NULL collapse path) → 401 invalid_token, not silent re-resolution to the user's earliest membership via resolveDefaultOrgId. Run with LOBU_TEST_BACKEND=pglite — no external Postgres required.
Round 2 — codex findings addressedFindings → commits
#1, #2, and #3 are bundled into one commit because all three live inside the same middleware closure; splitting them would mean intermediate bridge states that don't compose. The commit body maps each fix to its own paragraph. RefactorExtracted the auth-bridge middleware from the closure inside Reproducer (HIGH #1 — membership removed)The test Before fix (against After fix (against Mirrors the canonical REST behaviour at Test coverageBefore this PR: 0 integration tests for the embedded After: 11 tests in
Regression checkExisting PAT coverage in No regression — the existing 36 active MCP-route PAT tests still pass against the refactored bridge (their entry point is the org-scoped Validation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server/src/__tests__/integration/lobu/gateway-auth.test.ts (1)
131-144: 💤 Low valueNarrow the UPDATE to target only the specific PAT created in this test.
The UPDATE statement affects all PATs matching
user_idandorganization_id, not just the one created bycreateTestPATin this test. Since fixtures (user,org) persist across all tests viabeforeAll, PATs from earlier tests (e.g., the happy-path test) could also be affected, making test behavior dependent on execution order.Consider using the returned PAT's identifier (e.g.,
token_hashoridif available fromcreateTestPAT) to scope the update.🤖 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 `@packages/server/src/__tests__/integration/lobu/gateway-auth.test.ts` around lines 131 - 144, The UPDATE currently expires all rows matching user_id and organization_id; change it to only touch the PAT created by this test by using the identifier returned from createTestPAT (e.g., destructure { token, id, token_hash } = await createTestPAT(...)) and add a WHERE clause targeting that identifier (for example WHERE id = ${id} or WHERE token_hash = ${token_hash}) in the sql`UPDATE personal_access_tokens SET expires_at = NOW() - INTERVAL '1 hour' WHERE ...` so only the specific token created for this test is expired.
🤖 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.
Nitpick comments:
In `@packages/server/src/__tests__/integration/lobu/gateway-auth.test.ts`:
- Around line 131-144: The UPDATE currently expires all rows matching user_id
and organization_id; change it to only touch the PAT created by this test by
using the identifier returned from createTestPAT (e.g., destructure { token, id,
token_hash } = await createTestPAT(...)) and add a WHERE clause targeting that
identifier (for example WHERE id = ${id} or WHERE token_hash = ${token_hash}) in
the sql`UPDATE personal_access_tokens SET expires_at = NOW() - INTERVAL '1 hour'
WHERE ...` so only the specific token created for this test is expired.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e11cfe6d-a462-4ae7-9802-fe5656b3d17f
📒 Files selected for processing (2)
packages/server/src/__tests__/integration/lobu/gateway-auth.test.tspackages/server/src/lobu/gateway.ts
… round-2)
Pre-fix: `Authorization: bearer owl_pat_*` (lowercase scheme) failed the
`header.startsWith('Bearer ')` literal match, so the PAT path was skipped
and the bridge fell through to the Better Auth cookie path — a valid
session cookie would silently mask an invalid/revoked PAT.
RFC 7235 §2.1 makes the auth scheme token case-insensitive. Parse it that
way. Token VALUE comparison stays case-sensitive — PAT hashes are.
…odex round-2) Three new tests against `createLobuAuthBridge`: 1. Lowercase `bearer` scheme + invalid PAT + valid session cookie → 401 (proves the case-insensitive parse + PAT precedence hold together; was the evasion gap before the fix). 2. Uppercase `BEARER` scheme + valid PAT → 200 (case-insensitive parse, success direction). 3. Cookie-only request (no Authorization header) → bridge reaches `next()` instead of short-circuiting with its own 401/403 (`error: 'invalid_token'` / `error: 'forbidden'` would indicate the PAT or membership path mistakenly fired). End-to-end Better Auth cookie verification is exercised by entities/member-privacy-contract.test.ts via the full app; this minimal harness only owns the bridge contract.
|
Round-3 done — both codex round-2 findings addressed. Fix 1 — Case-insensitive Bearer scheme (b3eec3c) Fix 2 — Test coverage (f234842)
Local validation
DB-perm note: codex hit Expected codex confidence ≥90% on next pass. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server/src/__tests__/integration/lobu/gateway-auth.test.ts (1)
121-159: 💤 Low valueConsider using
token_prefixfor more precise UPDATE targeting.The UPDATE statements at lines 135-138 and 149-153 match by
user_idandorganization_id, which could affect multiple PAT rows if tests share fixtures or run in parallel. The null-org test at lines 334-339 usestoken_prefixfor precise targeting — consider aligning these earlier tests with that pattern for consistency and robustness.♻️ Example: more precise UPDATE
it('rejects an expired PAT', async () => { const { token } = await createTestPAT(user.id, org.id); const sql = getTestDb(); await sql` UPDATE personal_access_tokens SET expires_at = NOW() - INTERVAL '1 hour' - WHERE user_id = ${user.id} AND organization_id = ${org.id} + WHERE user_id = ${user.id} + AND token_prefix = ${token.substring(0, 12)} `;🤖 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 `@packages/server/src/__tests__/integration/lobu/gateway-auth.test.ts` around lines 121 - 159, The UPDATEs in the "rejects an expired PAT" and "rejects a revoked PAT" tests update rows by user_id and organization_id which can affect multiple PATs; instead, target the specific token by using the token_prefix column (as done in the null-org test). Modify the SQL in the tests that call createTestPAT(user.id, org.id) to extract the token prefix from the returned token and include WHERE token_prefix = <prefix> in the UPDATE for the personal_access_tokens table (keep user_id and organization_id if desired for extra safety) so only the created PAT row is updated.
🤖 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.
Nitpick comments:
In `@packages/server/src/__tests__/integration/lobu/gateway-auth.test.ts`:
- Around line 121-159: The UPDATEs in the "rejects an expired PAT" and "rejects
a revoked PAT" tests update rows by user_id and organization_id which can affect
multiple PATs; instead, target the specific token by using the token_prefix
column (as done in the null-org test). Modify the SQL in the tests that call
createTestPAT(user.id, org.id) to extract the token prefix from the returned
token and include WHERE token_prefix = <prefix> in the UPDATE for the
personal_access_tokens table (keep user_id and organization_id if desired for
extra safety) so only the created PAT row is updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4f02a57-2c53-417f-8656-1a1966bdf055
📒 Files selected for processing (2)
packages/server/src/__tests__/integration/lobu/gateway-auth.test.tspackages/server/src/lobu/gateway.ts
Mirror the Bearer scheme fix at the inner prefix check: `Bearer OWL_PAT_*` now flows through PAT validation instead of falling through to cookie auth. Token value handed to verify() is unchanged — PAT hashes stay byte-exact.
2 participants
Why
Two defects together kept the embedded PGlite assembly (`lobu run` with no
`DATABASE_URL`) from speaking the same Agent API as the Postgres assembly,
forcing external clients to forge Better Auth cookies just to call
`/lobu/api/v1/agents`. Both surfaced live in the qmsum-demo ROUGE
benchmark and were worked around there with hacks.
`start-local.ts` missing `/lobu` prefix mount. `initLobuGateway()`
returns the Hono app that hosts the public Agent API + worker gateway +
MCP proxy + bundled docs, but `start-local.ts` called it for its
side-effects and discarded the return value. `server.ts` had already
been fixed in PR fix(cli): chat/eval target the gateway Agent API under /lobu #637 (`app.route('/lobu', lobuApp)`), but the PGlite
entrypoint was never updated, so every `/lobu/*` request returned 404.
PGlite-mode Agent API rejected PATs. The `lobuApp` auth-hydration
middleware in `packages/server/src/lobu/gateway.ts` only consulted
`auth.api.getSession` (Better Auth session cookie or session-token
bearer). The bearer() plugin doesn't recognise `owl_pat_` PATs — they
live in `personal_access_tokens`, a separate table the embedded
authProvider had no path to. So a PAT minted by `lobu token create` (or
returned by `/api/local-init` as `device_token`) silently failed to
authenticate against `/lobu/api/v1/agents/`, and qmsum-demo's
`scripts/run-benchmark.py` had to mint a session cookie from
`BETTER_AUTH_SECRET` instead. This affects both PGlite and
Postgres assemblies — they share the same auth bridge.
What changed
Test plan
Reproducer (red -> green)
Booted PGlite via `node packages/server/dist/start-local.bundle.mjs` with
`PORT=8811 WORKER_PROXY_PORT=8142 LOBU_DATA_DIR=/tmp/lobu-pglite-test
LOBU_ALLOW_EPHEMERAL_ENCRYPTION_KEY=1`. PAT minted via `POST /api/local-init`
in both runs.
Before (current main):
```
GET /lobu/health -> HTTP 404 ("404 Not Found")
GET /lobu/api/v1/agents -> HTTP 404 ("404 Not Found")
GET /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_...'
-> HTTP 404 ("404 Not Found")
```
After (this PR):
```
GET /lobu/health -> HTTP 200 ({"status":"ok",...})
GET /lobu/api/v1/agents -> HTTP 401 ({"success":false,"error":"Unauthorized"})
GET /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_h87FBj2IXfRaLeIqMxY1nrQkm_Wjf8Ja'
-> HTTP 200 ({"agents":[]})
POST /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_h87FBj2IXfRaLeIqMxY1nrQkm_Wjf8Ja'
-d '{"thread":"verify-pglite-parity"}'
-> HTTP 201 ({"success":true,"agentId":"...","token":"...","sseUrl":"...","messagesUrl":"..."})
GET /lobu/api/v1/agents -H 'Authorization: Bearer owl_pat_invalidxxxxxxxxxxxxxxxxxxxx'
-> HTTP 401 ({"success":false,"error":"Unauthorized"})
```
Notes
Summary by CodeRabbit
New Features
Bug Fixes
Tests