CI fixes

.github/workflows/ci.yml

@@ -3,11 +3,31 @@ name: CI
 on:
   push:
     branches: [main, develop]
+  # `branches` filters on the PR's BASE, not its head. Without it every pull
+  # request runs — including feature -> sandbox, where sandbox is the shared
+  # integration branch and CI is not wanted. The trade-off is deliberate:
+  # nothing gates a merge into sandbox, and breakage surfaces one step later at
+  # the sandbox -> develop PR.
   pull_request:
+    branches: [main, develop]
+  workflow_dispatch:
+
+# One run per ref. A force-push to a PR cancels the superseded run instead of
+# leaving two full pipelines (including the 25-minute docker smoke) racing.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+# Read-only by default; no job here writes to the repo.
+permissions:
+  contents: read
 
 jobs:
   ci:
     runs-on: ubuntu-latest
+    # The postgres service health-check can hang; fail fast rather than sitting
+    # until GitHub's 6h default.
+    timeout-minutes: 20
 
     services:
       # apps/api's integration tests hit a real Postgres and TRUNCATE all 13
@@ -39,6 +59,18 @@ jobs:
       - name: Install dependencies
         run: npm ci
 
+      # Turbo's cache is local-only, so without this every run rebuilds and
+      # re-tests from cold. Keyed on the lockfile plus the SHA so a run restores
+      # the nearest previous cache and then writes its own.
+      - name: Restore turbo cache
+        uses: actions/cache@v4
+        with:
+          path: .turbo
+          key: turbo-${{ runner.os }}-${{ hashFiles('package-lock.json') }}-${{ github.sha }}
+          restore-keys: |
+            turbo-${{ runner.os }}-${{ hashFiles('package-lock.json') }}-
+            turbo-${{ runner.os }}-
+
       - name: Lint
         run: npm run lint
 
@@ -51,14 +83,28 @@ jobs:
       - name: Build
         run: npm run build
 
-      - name: Test
-        run: npm test
+      # One merged coverage run across every workspace instead of `npm test`.
+      # It runs the same 179 tests, so nothing is lost by replacing the plain
+      # test step — and it must come after Build because apps/edge-worker's
+      # suite boots dist/index.js in workerd. Thresholds live in
+      # vitest.config.ts and fail the step on their own.
+      - name: Test + coverage
+        run: npm run test:coverage
         env:
           # Set explicitly rather than relying on the zod default: every var in
           # apps/api/src/env.ts has one, so a mis-wired database never throws at
           # boot — it fails at the first query, much further from the cause.
           DATABASE_URL: postgres://toggleflow:toggleflow@localhost:5434/toggleflow
 
+      - name: Upload coverage report
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage
+          path: coverage/
+          retention-days: 7
+          if-no-files-found: warn
+
   docker-smoke:
     name: Docker prod smoke
     runs-on: ubuntu-latest

README.md

@@ -56,40 +56,40 @@ npm run dev                                   # all workspaces in watch mode (tu
 
 One compose file, three profiles. Every service belongs to a profile, so a bare `docker compose up` starts nothing.
 
-| Command | What it does |
-| --- | --- |
-| `docker compose --profile dev up --build` | Hot-reloading stack: tsx watch, vite, wrangler dev, source bind-mounted |
-| `docker compose --profile prod up --build` | Built artifacts: API on node, dashboard as static assets on nginx |
-| `docker compose --profile tools run --rm migrate` | Apply migrations (idempotent) |
-| `docker compose --profile tools run --rm seed` | Seed demo data (idempotent) |
-| `docker compose --profile dev down` | Stop. Add `-v` to also drop the database and KV volumes |
+| Command                                           | What it does                                                            |
+| ------------------------------------------------- | ----------------------------------------------------------------------- |
+| `docker compose --profile dev up --build`         | Hot-reloading stack: tsx watch, vite, wrangler dev, source bind-mounted |
+| `docker compose --profile prod up --build`        | Built artifacts: API on node, dashboard as static assets on nginx       |
+| `docker compose --profile tools run --rm migrate` | Apply migrations (idempotent)                                           |
+| `docker compose --profile tools run --rm seed`    | Seed demo data (idempotent)                                             |
+| `docker compose --profile dev down`               | Stop. Add `-v` to also drop the database and KV volumes                 |
 
 The **`dev` and `prod` profiles cannot run at the same time** — both routers publish host `:3200`.
 
 ### Routing
 
 Everything enters through an nginx router. This is not decoration: [apps/dashboard/src/api/client.ts](apps/dashboard/src/api/client.ts) hardcodes `fetch('/api' + path)` with no configurable base URL, so the SPA must be served same-origin with something that strips `/api`.
 
-| URL | Goes to |
-| --- | --- |
-| `http://localhost:3200/` | Dashboard (Vite dev server, or static nginx in prod) |
-| `http://localhost:3200/api/*` | Control-plane API, `/api` stripped — e.g. `/api/health` → `/health` |
-| `http://localhost:3200/edge/*` | Edge worker, `/edge` stripped (**dev profile only**) |
-| `http://localhost:3200/healthz` | The router's own liveness probe |
+| URL                             | Goes to                                                             |
+| ------------------------------- | ------------------------------------------------------------------- |
+| `http://localhost:3200/`        | Dashboard (Vite dev server, or static nginx in prod)                |
+| `http://localhost:3200/api/*`   | Control-plane API, `/api` stripped — e.g. `/api/health` → `/health` |
+| `http://localhost:3200/edge/*`  | Edge worker, `/edge` stripped (**dev profile only**)                |
+| `http://localhost:3200/healthz` | The router's own liveness probe                                     |
 
 The edge worker has no prod container on purpose: production is `npm run deploy -w @toggleflow/edge-worker` to Cloudflare, so a local prod container would simulate nothing.
 
 ### Port map
 
 Ports are shared across the VeloBits dev stacks — don't collide.
 
-| Port | Owner |
-| --- | --- |
-| 3200 | ToggleFlow router |
+| Port               | Owner                                                                                  |
+| ------------------ | -------------------------------------------------------------------------------------- |
+| 3200               | ToggleFlow router                                                                      |
 | 4000 / 5173 / 8787 | ToggleFlow api / dashboard / edge worker (dev profile, published for direct debugging) |
-| 5434 | ToggleFlow Postgres (5433 is the fixmytext dev DB) |
-| 80, 8080 | velobits-infra traefik, Keycloak |
-| 3000, 3100–3103 | fixmytext-web-frontend |
+| 5434               | ToggleFlow Postgres (5433 is the fixmytext dev DB)                                     |
+| 80, 8080           | velobits-infra traefik, Keycloak                                                       |
+| 3000, 3100–3103    | fixmytext-web-frontend                                                                 |
 
 ### The Keycloak issuer split
 
@@ -115,8 +115,8 @@ docker/
 
 Three things worth knowing before changing any of it:
 
-- **Every Node image is `bookworm-slim`, never alpine.** Two workspaces spawn `workerd`, which Cloudflare ships glibc-linked: `apps/edge-worker` via `wrangler dev`, and `apps/api` via `miniflare` — a *runtime* dependency, imported whenever `KV_MODE=miniflare` (the default).
-- **The `engine-dev` service is load-bearing.** `packages/engine` resolves through its `exports` to `dist/`, which is gitignored *and* `.dockerignore`d, so nothing else creates it. That service builds it, then watches; every app gates on its healthcheck.
+- **Every Node image is `bookworm-slim`, never alpine.** Two workspaces spawn `workerd`, which Cloudflare ships glibc-linked: `apps/edge-worker` via `wrangler dev`, and `apps/api` via `miniflare` — a _runtime_ dependency, imported whenever `KV_MODE=miniflare` (the default).
+- **The `engine-dev` service is load-bearing.** `packages/engine` resolves through its `exports` to `dist/`, which is gitignored _and_ `.dockerignore`d, so nothing else creates it. That service builds it, then watches; every app gates on its healthcheck.
 - **The miniflare KV store is a named volume, not a bind mount.** `apps/api` writes it and the edge worker reads it — two workerd processes over one SQLite store. On a Windows bind mount that reliably produces `SQLITE_BUSY`. Reset it with `docker volume rm toggleflow_wrangler-state`.
 
 ## Dev infrastructure

apps/api/test/helpers.ts

@@ -3,6 +3,8 @@
  * (:5434), with tokens signed by a local RS256 key and verified through the
  * SAME verifier code path production uses (issuer + audience checks included).
  */
+import { join } from 'node:path';
+
 import { sql } from 'drizzle-orm';
 import { migrate } from 'drizzle-orm/postgres-js/migrator';
 import type { FastifyInstance } from 'fastify';
@@ -44,7 +46,10 @@ export async function setupTestApp(opts: { kv?: KvClient } = {}): Promise<TestHa
     verifier: createTokenVerifier({ issuer: TEST_ISSUER, audience: TEST_AUDIENCE, getKey }),
     kv,
   });
-  await migrate(app.db, { migrationsFolder: 'drizzle' });
+  // Anchored to this file, not the CWD: the merged coverage run at the repo
+  // root (vitest.config.ts `projects`) executes with a different CWD than
+  // `npm test -w @toggleflow/api` does.
+  await migrate(app.db, { migrationsFolder: join(import.meta.dirname, '..', 'drizzle') });
   await resetDb(app.db);
 
   const signToken = (sub: string, opts: SignOptions = {}) =>

apps/api/test/kv-cloudflare.test.ts

@@ -0,0 +1,121 @@
+/**
+ * The production KV client, driven against a stubbed `fetch`. This is the one
+ * path that cannot be exercised for real in CI — it talks to api.cloudflare.com
+ * — so the contract is pinned here instead: URL shape, auth header, the
+ * multipart body, and which HTTP statuses are errors versus expected misses.
+ */
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+import { createCloudflareKvClient } from '../src/lib/kv';
+
+const ACCOUNT = 'acct-123';
+const NAMESPACE = 'ns-456';
+const TOKEN = 'cf-token';
+const BASE = `https://api.cloudflare.com/client/v4/accounts/${ACCOUNT}/storage/kv/namespaces/${NAMESPACE}`;
+
+const client = () =>
+  createCloudflareKvClient({ accountId: ACCOUNT, namespaceId: NAMESPACE, apiToken: TOKEN });
+
+/** Queue one response per expected fetch, in order. */
+function stubFetch(...responses: Response[]) {
+  const spy = vi.fn<typeof fetch>();
+  for (const res of responses) spy.mockResolvedValueOnce(res);
+  vi.stubGlobal('fetch', spy);
+  return spy;
+}
+
+const ok = (body = '', status = 200) => new Response(body, { status });
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+describe('put', () => {
+  it('PUTs a multipart body with value and metadata to the values endpoint', async () => {
+    const spy = stubFetch(ok());
+    await client().put('ruleset:env-1', '{"flags":[]}', { contentHash: 'abc', version: 7 });
+
+    expect(spy).toHaveBeenCalledOnce();
+    const [url, init] = spy.mock.calls[0]!;
+    expect(url).toBe(`${BASE}/values/ruleset%3Aenv-1`);
+    expect(init?.method).toBe('PUT');
+    expect((init?.headers as Record<string, string>).authorization).toBe(`Bearer ${TOKEN}`);
+
+    const form = init?.body as FormData;
+    expect(form.get('value')).toBe('{"flags":[]}');
+    expect(JSON.parse(form.get('metadata') as string)).toEqual({
+      contentHash: 'abc',
+      version: 7,
+    });
+  });
+
+  it('sends an empty metadata object when none is supplied', async () => {
+    const spy = stubFetch(ok());
+    await client().put('keys:env-1', 'payload');
+    const form = spy.mock.calls[0]![1]!.body as FormData;
+    expect(form.get('metadata')).toBe('{}');
+  });
+
+  it('throws with the status when Cloudflare rejects the write', async () => {
+    stubFetch(ok('denied', 403));
+    await expect(client().put('ruleset:env-1', 'x')).rejects.toThrow(
+      'KV put ruleset:env-1 failed: HTTP 403',
+    );
+  });
+});
+
+describe('getWithMetadata', () => {
+  it('returns the value and the metadata result', async () => {
+    const spy = stubFetch(
+      ok('{"flags":[]}'),
+      ok(JSON.stringify({ result: { contentHash: 'abc', version: 7 } })),
+    );
+
+    const entry = await client().getWithMetadata('ruleset:env-1');
+    expect(entry).toEqual({ value: '{"flags":[]}', metadata: { contentHash: 'abc', version: 7 } });
+    expect(spy.mock.calls[0]![0]).toBe(`${BASE}/values/ruleset%3Aenv-1`);
+    expect(spy.mock.calls[1]![0]).toBe(`${BASE}/metadata/ruleset%3Aenv-1`);
+  });
+
+  it('treats a 404 as an empty entry and never asks for metadata', async () => {
+    const spy = stubFetch(ok('', 404));
+    expect(await client().getWithMetadata('missing')).toEqual({ value: null, metadata: null });
+    // A miss is normal — the second round trip would be wasted.
+    expect(spy).toHaveBeenCalledOnce();
+  });
+
+  it('keeps the value when the metadata request fails', async () => {
+    stubFetch(ok('body'), ok('', 500));
+    expect(await client().getWithMetadata('k')).toEqual({ value: 'body', metadata: null });
+  });
+
+  it('nulls the metadata when the response carries no result', async () => {
+    stubFetch(ok('body'), ok(JSON.stringify({})));
+    expect(await client().getWithMetadata('k')).toEqual({ value: 'body', metadata: null });
+  });
+
+  it('throws on a non-404 read failure', async () => {
+    stubFetch(ok('', 500));
+    await expect(client().getWithMetadata('k')).rejects.toThrow('KV get k failed: HTTP 500');
+  });
+});
+
+describe('delete', () => {
+  it('DELETEs the value endpoint', async () => {
+    const spy = stubFetch(ok());
+    await client().delete('ruleset:env-1');
+    const [url, init] = spy.mock.calls[0]!;
+    expect(url).toBe(`${BASE}/values/ruleset%3Aenv-1`);
+    expect(init?.method).toBe('DELETE');
+  });
+
+  it('tolerates a 404 — deleting an absent key is a no-op', async () => {
+    stubFetch(ok('', 404));
+    await expect(client().delete('missing')).resolves.toBeUndefined();
+  });
+
+  it('throws on any other failure', async () => {
+    stubFetch(ok('', 500));
+    await expect(client().delete('k')).rejects.toThrow('KV delete k failed: HTTP 500');
+  });
+});