Conversation
- Added support for JWT authentication, allowing users to exchange API keys for JWTs via the new `/auth/token` endpoint. - Introduced API key management with routes for listing and creating API keys, enhancing security and user control. - Updated the OpenAPI specification to reflect new authentication methods and security schemes. - Enhanced the principal resolution logic to support JWTs and API keys, improving authorization handling across the application. - Added tests to ensure proper functionality of the new authentication features and their integration with existing components.
…ures - Introduced `RateLimiter` Durable Object for per-IP rate limiting, replacing in-memory token buckets. - Added KV caching for session lists with a 15s TTL, including cache invalidation on session updates. - Implemented structured JSON logging with `X-Trace-Id` for enhanced observability and error tracking. - Updated health check endpoint to support deep readiness checks with database connectivity verification. - Enhanced API routes and middleware to integrate new features, ensuring compliance with updated specifications. - Added tests for health checks, rate limiting, and session caching functionalities to ensure reliability.
…ken setup - Clarified the process for creating KV namespaces, specifying that commands should be run from the `apps/api` directory. - Added instructions for copying KV namespace IDs into `wrangler.toml` for staging and production environments. - Included guidance for local development without Cloudflare KV and troubleshooting authentication errors related to the Cloudflare API token.
- Added unit tests for the History and Overview components using Vitest and Testing Library, improving test coverage and reliability. - Introduced contract tests to ensure OpenAPI paths align with expected schemas, enhancing API stability. - Implemented visual regression tests with Playwright to capture UI changes and maintain visual consistency. - Updated README and ROADMAP to reflect new testing capabilities and deployment instructions, including a full staging pipeline command. - Upgraded Wrangler to version 4 across the project for improved development experience.
- Updated session list caching to use a 60s TTL in Cloudflare KV with a 15s HTTP max-age for responses, improving cache efficiency. - Introduced a new utility function `isSessionId` for validating session IDs against a UUID format. - Enhanced session retrieval routes to validate session IDs before processing requests, ensuring better error handling. - Refactored cache-related functions to handle errors gracefully, maintaining API reliability. - Updated documentation to reflect changes in caching strategy and session ID validation.
- Integrated PWA capabilities using vite-plugin-pwa, enabling offline access and caching for session data. - Added i18n support with react-i18next, providing English translations for navigation and session details. - Implemented a scorecard export feature, allowing users to generate print-ready scorecards for sessions. - Introduced an OfflineBanner component to notify users when offline, enhancing user experience. - Updated package dependencies to include necessary libraries for analytics and query persistence. - Enhanced documentation to reflect new features and usage instructions.
… updates - Updated the `FileField` component to include a visible button for file selection, improving user experience. - Implemented state management to display the selected file name, enhancing feedback for users. - Modified tooltip descriptions in the documentation to clarify the new file input pattern and usage guidelines. - Ensured accessibility by maintaining proper aria attributes and labels for the file input.
There was a problem hiding this comment.
24 issues found across 94 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/principal.ts">
<violation number="1" location="apps/api/src/principal.ts:27">
P2: JWT auth can be skipped for valid `Authorization` headers that use a different bearer-scheme case, because parsing currently requires `Bearer ` with exact casing. Since auth-scheme matching is case-insensitive, this can reject otherwise valid JWT requests depending on client behavior. A case-insensitive parse would make JWT auth robust across conforming clients.</violation>
</file>
<file name="packages/db/src/api-keys.ts">
<violation number="1" location="packages/db/src/api-keys.ts:20">
P1: API key exchange can resolve to an owner without proving the provided key when `app.current_owner` is already present in the transaction context. The lookup query in `resolveApiKeyOwner` uses `limit(1)` without filtering by `keyHash`, so it can return any row visible via the owner branch of the new `api_keys_select` policy. Adding an explicit `WHERE key_hash = ... AND revoked_at IS NULL` in the query keeps resolution tied to the submitted key regardless of surrounding session state.</violation>
</file>
<file name="apps/api/src/openapi/spec.ts">
<violation number="1" location="apps/api/src/openapi/spec.ts:170">
P1: Both `/auth/keys` operations are missing the required `responses` field. The OpenAPI 3.1 spec mandates every operation include a `responses` object with at least one response — a spec-compliant code generator or validator will reject these operations as invalid. Add response definitions covering the success and error cases (e.g., 200/201 + 401).</violation>
</file>
<file name="apps/api/src/lib/logger.ts">
<violation number="1" location="apps/api/src/lib/logger.ts:19">
P2: Info-level messages are written to stderr via `console.warn`, which conflates informational diagnostics with warnings. In production, stderr often drives alerting or error log aggregation, so info noise there reduces signal clarity. Recommend routing `'info'` to `console.log` (stdout) and `'warn'` to `console.warn` (stderr) to keep log streams semantically clean.</violation>
</file>
<file name="packages/db/migrations/meta/_journal.json">
<violation number="1" location="packages/db/migrations/meta/_journal.json:16">
P2: Missing snapshot file `meta/0001_snapshot.json` for migration `0001_api_keys`. Drizzle Kit uses the latest snapshot to compute schema diffs when generating the next migration. Without `0001_snapshot.json`, the next `drizzle-kit generate` won't know the current schema state and may produce incorrect or duplicate migrations. Commit the generated snapshot alongside the SQL file and journal entry.</violation>
</file>
<file name="scripts/env-setup.mjs">
<violation number="1" location="scripts/env-setup.mjs:25">
P2: The `.dev.vars.example` template doesn't include `OCHE_JWT_SECRET`, so `setup:dev-vars` (init) and `setup:dev-vars:merge` will both produce a `.dev.vars` file that's missing this key. Only `setup:dev-vars:sync` handles it (because `applyEnvValues` appends unseen keys), but new contributors who follow `init` first will have an incomplete file. Add `OCHE_JWT_SECRET` to `apps/api/.dev.vars.example` to keep the template consistent with the synced variables.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:66">
P3: `deploy:staging:full` runs `db:migrate:staging` twice — once explicitly as the first step and then again as part of `deploy:staging`. Since migrations are idempotent this won't break anything, but it adds unnecessary time to the deploy pipeline and makes the dependency between steps harder to reason about. Recommend removing the explicit `db:migrate:staging` call from `deploy:staging:full` since `deploy:staging` already includes it.</violation>
</file>
<file name=".cursor/rules/60-realtime-do.mdc">
<violation number="1" location=".cursor/rules/60-realtime-do.mdc:5">
P2: This rule now documents that `RateLimiter` must be SQLite-backed, but the `globs` field still only targets `session-room.ts`. The rule won't activate when editing `rate-limiter.ts`, so future changes there can miss the SQLite constraint. Update the globs to include both files.</violation>
</file>
<file name="packages/shared/src/auth.ts">
<violation number="1" location="packages/shared/src/auth.ts:34">
P2: `createdAt` and `revokedAt` in `ApiKeySummary` and `CreateApiKeyResponse` use bare `z.string()` instead of `z.string().datetime()`. The project convention in `schema.ts` applies `.datetime()` to every timestamp field (`ScoreEventSchema.createdAt`, `SessionSchema.createdAt`, `SessionSchema.updatedAt`). Without format validation, invalid date strings (e.g. garbage text, ISO week dates, timestamps in wrong timezone) would pass validation silently, leading to inconsistent data downstream.</violation>
</file>
<file name="apps/web/src/routes/Overview.test.tsx">
<violation number="1" location="apps/web/src/routes/Overview.test.tsx:28">
P0: The test will crash at module evaluation time. `vi.mock` is hoisted and its factory runs before `const patchScores` is initialized, so the factory's reference to `patchScores` throws a `ReferenceError: Cannot access 'patchScores' before initialization`. Define `patchScores` with `vi.hoisted()` so it's available when the mock factory executes.</violation>
</file>
<file name="scripts/export-openapi.mjs">
<violation number="1" location="scripts/export-openapi.mjs:22">
P1: The `shell: true` option breaks the inline TypeScript code because it contains shell metacharacters (`;`, single quotes, newlines). The shell interprets `;` as a command separator and single quotes as shell quoting, so `tsx -e` only sees part of the code. The script will fail on any Unix shell. Remove `shell: true` — the command doesn't need shell features and works fine with direct argument passing.</violation>
</file>
<file name="docs/AUTH.md">
<violation number="1" location="docs/AUTH.md:7">
P2: The auth doc currently describes principal resolution in the opposite order from the API implementation. In practice, `resolvePrincipal` checks Bearer/`?token` first and only falls back to `x-oche-owner`/`?key` if JWT is absent, but this section lists API key first. Aligning this order will avoid incorrect integration assumptions when both credentials are present.</violation>
<violation number="2" location="docs/AUTH.md:10">
P3: Inconsistent path for the API key management UI. The doc first says `/auth/keys` (line 10) but the SPA flow section (line 22) and the cursor rule (`34-auth.mdc`) both say `/settings/keys`. Align the first reference to match the actual route.</violation>
</file>
<file name="apps/web/vite.config.js">
<violation number="1" location="apps/web/vite.config.js:57">
P1: Owner-scoped session data can be reused across users because the new service-worker cache stores `/sessions` responses without partitioning by authentication context. With `NetworkFirst`, a later user on the same browser profile can receive cached data from a previous principal during offline/timeout paths. Consider disabling runtime caching for authenticated `/sessions` responses (or explicitly keying cache entries by principal).</violation>
<violation number="2" location="apps/web/vite.config.js:59">
P1: The `apiOrigin()` helper referenced inside the `runtimeCaching.urlPattern` arrow function won't survive serialization into the generated service worker. workbox-build serializes function-based patterns via `.toString()`, which captures only the arrow function — the outer `apiOrigin` reference will be dangling. This causes a ReferenceError at runtime on every navigation that the sessions route pattern evaluates. Inline the origin logic directly into the arrow function instead, or compute the origin as a const outside the runtimeCaching config and reference that literal string.</violation>
</file>
<file name="apps/api/src/routes/auth.ts">
<violation number="1" location="apps/api/src/routes/auth.ts:39">
P2: Each protected route creates two separate postgres connection pools per request — one in the `auth.use('*')` middleware and another in the route handler. The middleware already has a `db` instance after resolving the principal but doesn't share it with downstream handlers, so `GET /keys`, `POST /keys`, and `DELETE /keys/:id` all call `getDb()` a second time. Store the `db` instance in the request context (e.g., extend `AuthVariables` with `db`) so handlers reuse it instead of creating a redundant pool.</violation>
</file>
<file name="apps/web/vite.config.ts">
<violation number="1" location="apps/web/vite.config.ts:60">
P0: The `urlPattern` callback in `runtimeCaching` calls `apiOrigin()`, but that function won't exist in the generated service worker's scope. Workbox-build serializes function-valued `urlPattern` entries via `.toString()`, which strips all closure and module-scope references — the resulting SW will throw a `ReferenceError` on every `/sessions/*` fetch, breaking the offline/network-first caching entirely. Compute the origin value at config-load time and use a RegExp pattern instead.</violation>
</file>
<file name="apps/web/src/routes/Overview.tsx">
<violation number="1" location="apps/web/src/routes/Overview.tsx:61">
P3: This extra guard is unreachable and adds dead branching to the render path. `panelId` is only `null` when `activeSessions.length === 0`, but that case already returns `QueryEmpty` above. Removing this branch keeps the flow simpler and avoids future confusion about whether `panelId` can be null here.</violation>
</file>
<file name="apps/web/src/components/ui/form-fields.tsx">
<violation number="1" location="apps/web/src/components/ui/form-fields.tsx:129">
P2: After this change, removing/reordering player rows can show the wrong selected filename beside a different player. The `FileField` display is now backed by internal `fileName` state, but this field is used inside an index-keyed list (`CreateSession`), so React instance reuse can carry stale filename UI across rows. Consider making the displayed filename controlled/resettable from parent data (or using stable row keys) so the visible file name stays aligned with the correct player.</violation>
</file>
<file name="apps/api/src/lib/jwt.ts">
<violation number="1" location="apps/api/src/lib/jwt.ts:51">
P2: Token expiration check is off by one second: tokens are still accepted when `exp === now` because verification only rejects `exp < now`. Tightening this to `exp <= now` avoids accepting already-expired JWTs at the boundary.</violation>
</file>
<file name="apps/web/src/main.tsx">
<violation number="1" location="apps/web/src/main.tsx:18">
P2: App startup can fail in storage-restricted environments because the persister reads `window.localStorage` eagerly at module scope. If that access throws, the SPA crashes before rendering. Wrapping storage acquisition in a safe try/catch fallback would keep the app usable even when persistent storage is unavailable.</violation>
</file>
<file name="apps/web/src/lib/query.ts">
<violation number="1" location="apps/web/src/lib/query.ts:18">
P1: Venue-scoped session data is now persisted under shared cache keys, so switching API key/venue can show another venue’s cached sessions/history until a network refetch succeeds. The new persistence filter stores `['sessions'...]` and `['session', id]` queries without any owner dimension, while auth switching only invalidates queries and does not clear the persisted store. Consider scoping persisted query identity by owner (or clearing persisted cache on venue switch) to avoid cross-tenant data exposure.</violation>
</file>
<file name="apps/api/src/middleware.ts">
<violation number="1" location="apps/api/src/middleware.ts:29">
P2: A transient Durable Object failure can currently turn normal requests into 500s because rate-limit checks parse the DO response without any error handling. Since this middleware runs on most routes, an intermittent DO issue can degrade API availability. Consider wrapping the DO fetch/JSON parse in a `try/catch` and falling back to the in-memory bucket (or another explicit fail-open/closed policy) when the DO path is unavailable.</violation>
</file>
<file name="apps/web/src/lib/auth-store.ts">
<violation number="1" location="apps/web/src/lib/auth-store.ts:48">
P2: After JWT expiry, the web client can get stuck sending only an expired bearer token, which causes API calls to fail until session reset. This happens because auth headers stop including `x-oche-owner` once `token` is set, and there is no automatic refresh path wired in. Including the API key header alongside bearer (or adding automatic refresh-on-401) would avoid this failure mode.</violation>
</file>
You're on the cubic free plan with 16 free PR reviews remaining this month. Upgrade for unlimited reviews.
Re-trigger cubic
| scoreEvents: [], | ||
| }; | ||
|
|
||
| const patchScores = vi.fn().mockResolvedValue({ ok: true }); |
There was a problem hiding this comment.
P0: The test will crash at module evaluation time. vi.mock is hoisted and its factory runs before const patchScores is initialized, so the factory's reference to patchScores throws a ReferenceError: Cannot access 'patchScores' before initialization. Define patchScores with vi.hoisted() so it's available when the mock factory executes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/routes/Overview.test.tsx, line 28:
<comment>The test will crash at module evaluation time. `vi.mock` is hoisted and its factory runs before `const patchScores` is initialized, so the factory's reference to `patchScores` throws a `ReferenceError: Cannot access 'patchScores' before initialization`. Define `patchScores` with `vi.hoisted()` so it's available when the mock factory executes.</comment>
<file context>
@@ -0,0 +1,74 @@
+ scoreEvents: [],
+};
+
+const patchScores = vi.fn().mockResolvedValue({ ok: true });
+
+vi.mock('@/hooks/useLiveSession', () => ({
</file context>
| const patchScores = vi.fn().mockResolvedValue({ ok: true }); | |
| const patchScores = vi.hoisted(() => vi.fn().mockResolvedValue({ ok: true })); |
| navigateFallbackDenylist: [/^\/docs\//], | ||
| runtimeCaching: [ | ||
| { | ||
| urlPattern: ({ url }) => url.origin === apiOrigin() && /^\/sessions(\/|$|\?)/.test(url.pathname), |
There was a problem hiding this comment.
P0: The urlPattern callback in runtimeCaching calls apiOrigin(), but that function won't exist in the generated service worker's scope. Workbox-build serializes function-valued urlPattern entries via .toString(), which strips all closure and module-scope references — the resulting SW will throw a ReferenceError on every /sessions/* fetch, breaking the offline/network-first caching entirely. Compute the origin value at config-load time and use a RegExp pattern instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/vite.config.ts, line 60:
<comment>The `urlPattern` callback in `runtimeCaching` calls `apiOrigin()`, but that function won't exist in the generated service worker's scope. Workbox-build serializes function-valued `urlPattern` entries via `.toString()`, which strips all closure and module-scope references — the resulting SW will throw a `ReferenceError` on every `/sessions/*` fetch, breaking the offline/network-first caching entirely. Compute the origin value at config-load time and use a RegExp pattern instead.</comment>
<file context>
@@ -21,7 +31,45 @@ function injectPreconnect(): PluginOption {
+ navigateFallbackDenylist: [/^\/docs\//],
+ runtimeCaching: [
+ {
+ urlPattern: ({ url }) => url.origin === apiOrigin() && /^\/sessions(\/|$|\?)/.test(url.pathname),
+ handler: 'NetworkFirst',
+ options: {
</file context>
| return db.transaction(async (tx) => { | ||
| await tx.execute(sql`set local role oche_app`); | ||
| await tx.execute(sql`select set_config('app.api_key_hash', ${keyHash}, true)`); | ||
| const [row] = await tx.select({ ownerId: apiKeys.ownerId }).from(apiKeys).limit(1); |
There was a problem hiding this comment.
P1: API key exchange can resolve to an owner without proving the provided key when app.current_owner is already present in the transaction context. The lookup query in resolveApiKeyOwner uses limit(1) without filtering by keyHash, so it can return any row visible via the owner branch of the new api_keys_select policy. Adding an explicit WHERE key_hash = ... AND revoked_at IS NULL in the query keeps resolution tied to the submitted key regardless of surrounding session state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/api-keys.ts, line 20:
<comment>API key exchange can resolve to an owner without proving the provided key when `app.current_owner` is already present in the transaction context. The lookup query in `resolveApiKeyOwner` uses `limit(1)` without filtering by `keyHash`, so it can return any row visible via the owner branch of the new `api_keys_select` policy. Adding an explicit `WHERE key_hash = ... AND revoked_at IS NULL` in the query keeps resolution tied to the submitted key regardless of surrounding session state.</comment>
<file context>
@@ -0,0 +1,29 @@
+ return db.transaction(async (tx) => {
+ await tx.execute(sql`set local role oche_app`);
+ await tx.execute(sql`select set_config('app.api_key_hash', ${keyHash}, true)`);
+ const [row] = await tx.select({ ownerId: apiKeys.ownerId }).from(apiKeys).limit(1);
+ return row?.ownerId ?? null;
+ });
</file context>
| }, | ||
| }, | ||
| '/auth/keys': { | ||
| get: { tags: ['auth'], summary: 'List API keys for current owner' }, |
There was a problem hiding this comment.
P1: Both /auth/keys operations are missing the required responses field. The OpenAPI 3.1 spec mandates every operation include a responses object with at least one response — a spec-compliant code generator or validator will reject these operations as invalid. Add response definitions covering the success and error cases (e.g., 200/201 + 401).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/openapi/spec.ts, line 170:
<comment>Both `/auth/keys` operations are missing the required `responses` field. The OpenAPI 3.1 spec mandates every operation include a `responses` object with at least one response — a spec-compliant code generator or validator will reject these operations as invalid. Add response definitions covering the success and error cases (e.g., 200/201 + 401).</comment>
<file context>
@@ -152,16 +153,40 @@ export const openApiDocument = {
+ },
+ },
+ '/auth/keys': {
+ get: { tags: ['auth'], summary: 'List API keys for current owner' },
+ post: { tags: ['auth'], summary: 'Create API key (plaintext returned once)' },
+ },
</file context>
| const result = spawnSync('npx', ['tsx', '-e', inline], { | ||
| cwd: repoRoot, | ||
| encoding: 'utf8', | ||
| shell: true, |
There was a problem hiding this comment.
P1: The shell: true option breaks the inline TypeScript code because it contains shell metacharacters (;, single quotes, newlines). The shell interprets ; as a command separator and single quotes as shell quoting, so tsx -e only sees part of the code. The script will fail on any Unix shell. Remove shell: true — the command doesn't need shell features and works fine with direct argument passing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/export-openapi.mjs, line 22:
<comment>The `shell: true` option breaks the inline TypeScript code because it contains shell metacharacters (`;`, single quotes, newlines). The shell interprets `;` as a command separator and single quotes as shell quoting, so `tsx -e` only sees part of the code. The script will fail on any Unix shell. Remove `shell: true` — the command doesn't need shell features and works fine with direct argument passing.</comment>
<file context>
@@ -0,0 +1,32 @@
+const result = spawnSync('npx', ['tsx', '-e', inline], {
+ cwd: repoRoot,
+ encoding: 'utf8',
+ shell: true,
+});
+
</file context>
| const res = await stub.fetch('https://rate-limiter/check', { | ||
| method: 'POST', | ||
| headers: { 'content-type': 'application/json' }, | ||
| body: JSON.stringify({ limit, windowMs }), | ||
| }); | ||
| const body = (await res.json()) as { allowed: boolean; remaining?: number }; | ||
| return { allowed: body.allowed, remaining: body.remaining }; |
There was a problem hiding this comment.
P2: A transient Durable Object failure can currently turn normal requests into 500s because rate-limit checks parse the DO response without any error handling. Since this middleware runs on most routes, an intermittent DO issue can degrade API availability. Consider wrapping the DO fetch/JSON parse in a try/catch and falling back to the in-memory bucket (or another explicit fail-open/closed policy) when the DO path is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/middleware.ts, line 29:
<comment>A transient Durable Object failure can currently turn normal requests into 500s because rate-limit checks parse the DO response without any error handling. Since this middleware runs on most routes, an intermittent DO issue can degrade API availability. Consider wrapping the DO fetch/JSON parse in a `try/catch` and falling back to the in-memory bucket (or another explicit fail-open/closed policy) when the DO path is unavailable.</comment>
<file context>
@@ -1,4 +1,54 @@
+ if (!ns) return checkMemoryRateLimit(ip, limit, windowMs);
+
+ const stub = ns.get(ns.idFromName(ip));
+ const res = await stub.fetch('https://rate-limiter/check', {
+ method: 'POST',
+ headers: { 'content-type': 'application/json' },
</file context>
| const res = await stub.fetch('https://rate-limiter/check', { | |
| method: 'POST', | |
| headers: { 'content-type': 'application/json' }, | |
| body: JSON.stringify({ limit, windowMs }), | |
| }); | |
| const body = (await res.json()) as { allowed: boolean; remaining?: number }; | |
| return { allowed: body.allowed, remaining: body.remaining }; | |
| try { | |
| const res = await stub.fetch('https://rate-limiter/check', { | |
| method: 'POST', | |
| headers: { 'content-type': 'application/json' }, | |
| body: JSON.stringify({ limit, windowMs }), | |
| }); | |
| if (!res.ok) return checkMemoryRateLimit(ip, limit, windowMs); | |
| const body = (await res.json()) as { allowed: boolean; remaining?: number }; | |
| return { allowed: body.allowed, remaining: body.remaining }; | |
| } catch { | |
| return checkMemoryRateLimit(ip, limit, windowMs); | |
| } |
| } | ||
|
|
||
| export function getAuthHeaders(): Record<string, string> { | ||
| if (token) return { Authorization: `Bearer ${token}` }; |
There was a problem hiding this comment.
P2: After JWT expiry, the web client can get stuck sending only an expired bearer token, which causes API calls to fail until session reset. This happens because auth headers stop including x-oche-owner once token is set, and there is no automatic refresh path wired in. Including the API key header alongside bearer (or adding automatic refresh-on-401) would avoid this failure mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/lib/auth-store.ts, line 48:
<comment>After JWT expiry, the web client can get stuck sending only an expired bearer token, which causes API calls to fail until session reset. This happens because auth headers stop including `x-oche-owner` once `token` is set, and there is no automatic refresh path wired in. Including the API key header alongside bearer (or adding automatic refresh-on-401) would avoid this failure mode.</comment>
<file context>
@@ -0,0 +1,55 @@
+}
+
+export function getAuthHeaders(): Record<string, string> {
+ if (token) return { Authorization: `Bearer ${token}` };
+ return { 'x-oche-owner': apiKey };
+}
</file context>
| "db:rls:check": "node scripts/rls-check.mjs", | ||
| "db:rls:check:staging": "node scripts/with-env.mjs DATABASE_URL_STAGING node scripts/rls-check.mjs", | ||
| "deploy:staging": "node scripts/run-steps.mjs db:migrate:staging deploy:api:staging build:web:staging deploy:web:staging", | ||
| "deploy:staging:full": "node scripts/run-steps.mjs db:migrate:staging db:force-rls:staging db:rls:check:staging db:seed:staging deploy:staging", |
There was a problem hiding this comment.
P3: deploy:staging:full runs db:migrate:staging twice — once explicitly as the first step and then again as part of deploy:staging. Since migrations are idempotent this won't break anything, but it adds unnecessary time to the deploy pipeline and makes the dependency between steps harder to reason about. Recommend removing the explicit db:migrate:staging call from deploy:staging:full since deploy:staging already includes it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 66:
<comment>`deploy:staging:full` runs `db:migrate:staging` twice — once explicitly as the first step and then again as part of `deploy:staging`. Since migrations are idempotent this won't break anything, but it adds unnecessary time to the deploy pipeline and makes the dependency between steps harder to reason about. Recommend removing the explicit `db:migrate:staging` call from `deploy:staging:full` since `deploy:staging` already includes it.</comment>
<file context>
@@ -60,7 +61,9 @@
"db:rls:check": "node scripts/rls-check.mjs",
+ "db:rls:check:staging": "node scripts/with-env.mjs DATABASE_URL_STAGING node scripts/rls-check.mjs",
"deploy:staging": "node scripts/run-steps.mjs db:migrate:staging deploy:api:staging build:web:staging deploy:web:staging",
+ "deploy:staging:full": "node scripts/run-steps.mjs db:migrate:staging db:force-rls:staging db:rls:check:staging db:seed:staging deploy:staging",
"deploy:prod": "node scripts/run-steps.mjs db:migrate:prod deploy:api:prod build:web:prod deploy:web:prod",
"deploy:all": "node scripts/run-steps.mjs deploy:staging deploy:prod",
</file context>
| 1. **API key** — `x-oche-owner` header or `?key=` (WebSocket). Sources: | ||
| - Built-in demo keys (`demo-key-a`, `demo-key-b`) → seed owners A/B | ||
| - `OCHE_API_KEYS` JSON in Worker env (local: `.oche-keys.json`) | ||
| - Hashed rows in `api_keys` table (created via `/auth/keys` UI) |
There was a problem hiding this comment.
P3: Inconsistent path for the API key management UI. The doc first says /auth/keys (line 10) but the SPA flow section (line 22) and the cursor rule (34-auth.mdc) both say /settings/keys. Align the first reference to match the actual route.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/AUTH.md, line 10:
<comment>Inconsistent path for the API key management UI. The doc first says `/auth/keys` (line 10) but the SPA flow section (line 22) and the cursor rule (`34-auth.mdc`) both say `/settings/keys`. Align the first reference to match the actual route.</comment>
<file context>
@@ -0,0 +1,43 @@
+1. **API key** — `x-oche-owner` header or `?key=` (WebSocket). Sources:
+ - Built-in demo keys (`demo-key-a`, `demo-key-b`) → seed owners A/B
+ - `OCHE_API_KEYS` JSON in Worker env (local: `.oche-keys.json`)
+ - Hashed rows in `api_keys` table (created via `/auth/keys` UI)
+2. **JWT** — `Authorization: Bearer <token>` or `?token=` (WebSocket). Minted by `POST /auth/token` using `OCHE_JWT_SECRET` (HS256, 1h TTL).
+3. **RLS** — Worker still calls `withPrincipal()` → `SET LOCAL app.current_owner`. Policies unchanged; JWT/API key only affects which owner id is set.
</file context>
| ); | ||
| } | ||
|
|
||
| if (!panelId) return <SessionOverviewSkeleton />; |
There was a problem hiding this comment.
P3: This extra guard is unreachable and adds dead branching to the render path. panelId is only null when activeSessions.length === 0, but that case already returns QueryEmpty above. Removing this branch keeps the flow simpler and avoids future confusion about whether panelId can be null here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/routes/Overview.tsx, line 61:
<comment>This extra guard is unreachable and adds dead branching to the render path. `panelId` is only `null` when `activeSessions.length === 0`, but that case already returns `QueryEmpty` above. Removing this branch keeps the flow simpler and avoids future confusion about whether `panelId` can be null here.</comment>
<file context>
@@ -62,13 +58,15 @@ export function Overview() {
);
}
+ if (!panelId) return <SessionOverviewSkeleton />;
+
return (
</file context>
/auth/tokenendpoint.Summary by cubic
Adds JWT auth with an
/auth/tokenexchange and hashed API keys, and updates principal resolution to accept JWTs or API keys. Also adds DO-backed rate limiting, KV caching for session lists, structured logging, and SPA upgrades (Owner switcher, API key UI, PWA/offline, i18n, scorecard export).New Features
POST /auth/token(HS256, 1h TTL), API key list/create/revoke (hashed in DB), updated OpenAPI, and JWT/API key resolution across routes; web usesAuthProvider+OwnerSwitcherwith demo keys still supported.RateLimiterDurable Object per IP, KV cache forGET /sessions(60s KV / 15s HTTP,X-Cache), structured JSON logs withX-Trace-Id, and deep readinessGET /health?deep=1./openapi.jsonwith Redoc page, contract + unit + visual tests added, andwrangler@4upgrade.vite-plugin-pwawith offline read-only cache (React Query persistence),react-i18next(en), scorecard export, improved file input, and optional analytics.Migration
api_keys(e.g.npm run db:prepare).OCHE_JWT_SECRET(Wrangler secret) and sync.dev.vars; keep demo keys for local use.RATE_LIMITERDO andCACHEKV inapps/api/wrangler.toml(apply migrations).apps/webbuild).Written for commit 85c2198. Summary will update on new commits.