perf(credshare): list a team's vaults in one query instead of two per vault (#128)

GET /api/credshare/teams/:slug/vaults built its response in a loop, asking
the database for a grant row and a secret count once per vault. libSQL is
remote, so each of those is a network round trip, and the endpoint cost
2N+1 of them.

On a team with 176 vaults that is 353 round trips and ~10.6s of server
time. `logicsrc teams pull` resolves the vault id twice -- once planning
the sync, once reading values -- so a pull of a single ten-key vault took
~24s, nearly all of it spent listing vaults the command does not want.

Replaced with one SELECT carrying two correlated subqueries. Both are
covered by existing primary keys (credshare_secrets is keyed
(vault_id, name), credshare_vault_grants (vault_id, user_id)), so the
per-vault work becomes an index probe inside the database instead of a
round trip across the network. No schema or index change.

Measured on a local libSQL seeded to match that team -- 176 vaults, 17
secrets each -- the endpoint goes from 355 round trips to 3, and returns
identical rows.

The response shape is unchanged: hasAccess is still a real boolean rather
than the 0/1 SQLite hands back, and secretCount is still a number.

Tests pin behaviour and cost separately. The behavioural cases pass
against both the old loop and the new query, which is the point -- only
the round-trip count changed. The regression guard asserts the query
count for 3 vaults EQUALS the count for 30 rather than matching a magic
number, so any future rewrite that reintroduces per-vault I/O fails no
matter what the constant part costs. Against the old loop it reports
9 vs 63.

Two sibling endpoints have the same shape -- /teams/:slug/members and
/vaults/:id/grants both call publicKeyFor() per member. Neither is on the
pull path and both scale with member count rather than vault count, so
they are left alone here.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

apps/pwa/src/routes/credshare.mjs

@@ -157,14 +157,33 @@ credshareRouter.post("/api/credshare/invites/accept", api(async (req, res, user)
 // ---- vaults ----
 credshareRouter.get("/api/credshare/teams/:slug/vaults", api(async (req, res, user) => {
   const ctx = await requireMember(res, req.params.slug, user.id); if (!ctx) return;
-  const vaults = await all(`SELECT * FROM credshare_vaults WHERE team_id = ? ORDER BY name`, [ctx.team.id]);
-  const out = [];
-  for (const v of vaults) {
-    const grant = await get(`SELECT 1 FROM credshare_vault_grants WHERE vault_id = ? AND user_id = ?`, [v.id, user.id]);
-    const count = await get(`SELECT COUNT(*) AS n FROM credshare_secrets WHERE vault_id = ?`, [v.id]);
-    out.push({ id: v.id, name: v.name, hasAccess: Boolean(grant), secretCount: Number(count?.n || 0) });
-  }
-  res.json({ vaults: out });
+  // One statement, not one per vault. libSQL is remote, so every execute() is a
+  // network round trip: the previous loop cost 2N+1 of them, and a team with 176
+  // vaults spent ~10s here -- doubled by `teams pull`, which resolves the vault
+  // id twice. Both correlated subqueries are covered by existing primary keys
+  // (credshare_secrets is keyed (vault_id, name), grants (vault_id, user_id)),
+  // so this is an index scan per vault inside the database rather than a
+  // round trip per vault across the network.
+  const vaults = await all(
+    `SELECT v.id,
+            v.name,
+            (SELECT COUNT(*) FROM credshare_secrets s
+              WHERE s.vault_id = v.id) AS secret_count,
+            EXISTS(SELECT 1 FROM credshare_vault_grants g
+                    WHERE g.vault_id = v.id AND g.user_id = ?) AS has_access
+       FROM credshare_vaults v
+      WHERE v.team_id = ?
+      ORDER BY v.name`,
+    [user.id, ctx.team.id]
+  );
+  res.json({
+    vaults: vaults.map((v) => ({
+      id: v.id,
+      name: v.name,
+      hasAccess: Boolean(v.has_access),
+      secretCount: Number(v.secret_count || 0)
+    }))
+  });
 }));
 
 credshareRouter.post("/api/credshare/teams/:slug/vaults", api(async (req, res, user) => {

apps/pwa/test/credshare-vaults.test.mjs

@@ -0,0 +1,169 @@
+// Integration tests for GET /api/credshare/teams/:slug/vaults.
+//
+// This endpoint used to issue two queries per vault inside a loop. libSQL is a
+// remote database, so each one is a network round trip: a real team with 176
+// vaults spent ~10s in here, and `logicsrc teams pull` resolves the vault id
+// twice, which is where the ~24s pull came from.
+//
+// The behavioural assertions below are the ones the loop already satisfied. The
+// one that matters for the regression is `issues the same number of queries`:
+// it pins the cost as INDEPENDENT of vault count rather than at some magic
+// number, so any future rewrite that reintroduces per-vault I/O fails here
+// regardless of how many statements the constant part happens to use.
+//
+// Runs against an in-memory libSQL database, so DATABASE_URL must be set before
+// anything imports db.mjs.
+process.env.DATABASE_URL = ":memory:";
+
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, join } from "node:path";
+import express from "express";
+
+const here = dirname(fileURLToPath(import.meta.url));
+const { db, run } = await import("../src/db.mjs");
+const { credshareRouter } = await import("../src/routes/credshare.mjs");
+
+async function migrate() {
+  for (const file of ["001_auth.sql", "002_credshare.sql"]) {
+    const sql = readFileSync(join(here, "..", "src", "migrations", file), "utf8");
+    for (const statement of sql.split(/;\s*$/m).map((s) => s.trim()).filter(Boolean)) {
+      await db.execute(statement);
+    }
+  }
+}
+
+async function serve(actingUserId) {
+  const app = express();
+  app.use(express.json());
+  app.use((req, _res, next) => {
+    req.user = { id: actingUserId };
+    next();
+  });
+  app.use(credshareRouter);
+  const server = app.listen(0);
+  await new Promise((resolve) => server.once("listening", resolve));
+  const base = `http://127.0.0.1:${server.address().port}`;
+  return {
+    async get(path) {
+      const res = await fetch(`${base}${path}`);
+      return { status: res.status, body: await res.json() };
+    },
+    close: () => new Promise((resolve) => server.close(resolve))
+  };
+}
+
+const now = Date.now();
+
+/** Count db.execute() calls while fn runs -- one call is one network round trip. */
+async function countQueries(fn) {
+  const original = db.execute.bind(db);
+  let calls = 0;
+  db.execute = (...args) => {
+    calls += 1;
+    return original(...args);
+  };
+  try {
+    await fn();
+  } finally {
+    db.execute = original;
+  }
+  return calls;
+}
+
+/**
+ * A team with `vaultCount` vaults. Every vault gets `secretsPerVault` secrets;
+ * the caller is granted access to all but the last, so hasAccess has both
+ * values in play.
+ */
+async function seedTeam({ teamId, slug, vaultCount, secretsPerVault = 2 }) {
+  await run(`INSERT INTO credshare_teams (id, slug, name, created_by, created_at) VALUES (?,?,?,?,?)`,
+    [teamId, slug, slug, "u_me", now]);
+  await run(`INSERT INTO credshare_members (id, team_id, user_id, email, role, status, created_at) VALUES (?,?,?,?,?,?,?)`,
+    [`m_${teamId}`, teamId, "u_me", "me@example.com", "owner", "active", now]);
+
+  for (let i = 0; i < vaultCount; i += 1) {
+    // Zero-padded so lexical name order matches creation order for the ORDER BY assertion.
+    const vid = `${teamId}_v${String(i).padStart(3, "0")}`;
+    await run(`INSERT INTO credshare_vaults (id, team_id, name, created_by, created_at) VALUES (?,?,?,?,?)`,
+      [vid, teamId, `app${String(i).padStart(3, "0")}--prod`, "u_me", now]);
+    if (i < vaultCount - 1) {
+      await run(`INSERT INTO credshare_vault_grants (vault_id, user_id, wrapped_dek, granted_by, created_at) VALUES (?,?,?,?,?)`,
+        [vid, "u_me", `wrapped-${vid}`, "u_me", now]);
+    }
+    for (let s = 0; s < secretsPerVault; s += 1) {
+      await run(`INSERT INTO credshare_secrets (vault_id, name, nonce, ciphertext, fingerprint, version, updated_by, updated_at) VALUES (?,?,?,?,?,?,?,?)`,
+        [vid, `KEY_${s}`, `n${s}`, `c${s}`, `fp${s}`, 1, "u_me", now]);
+    }
+  }
+}
+
+await migrate();
+await run(`INSERT INTO users (id, email, created_at) VALUES (?,?,?)`, ["u_me", "me@example.com", now]);
+await run(`INSERT INTO users (id, email, created_at) VALUES (?,?,?)`, ["u_outsider", "outsider@example.com", now]);
+await seedTeam({ teamId: "t_small", slug: "small", vaultCount: 3, secretsPerVault: 2 });
+await seedTeam({ teamId: "t_big", slug: "big", vaultCount: 30, secretsPerVault: 4 });
+
+test("reports each vault's secret count and whether the caller holds a grant", async (t) => {
+  const app = await serve("u_me");
+  t.after(() => app.close());
+
+  const res = await app.get("/api/credshare/teams/small/vaults");
+  assert.equal(res.status, 200);
+  assert.equal(res.body.vaults.length, 3);
+
+  // Ordered by name, counts per vault, and the last vault is the ungranted one.
+  assert.deepEqual(res.body.vaults.map((v) => v.name), ["app000--prod", "app001--prod", "app002--prod"]);
+  assert.deepEqual(res.body.vaults.map((v) => v.secretCount), [2, 2, 2]);
+  assert.deepEqual(res.body.vaults.map((v) => v.hasAccess), [true, true, false]);
+
+  // hasAccess must be a real boolean, not SQLite's 0/1 leaking through: clients
+  // branch on it, and `if (0)` and `if ("0")` disagree.
+  for (const v of res.body.vaults) {
+    assert.equal(typeof v.hasAccess, "boolean");
+    assert.equal(typeof v.secretCount, "number");
+  }
+});
+
+test("counts secrets per vault rather than across the team", async (t) => {
+  const app = await serve("u_me");
+  t.after(() => app.close());
+
+  const res = await app.get("/api/credshare/teams/big/vaults");
+  assert.equal(res.status, 200);
+  assert.equal(res.body.vaults.length, 30);
+  // 30 vaults x 4 secrets: a join that multiplied rows would report 120 here.
+  assert.ok(res.body.vaults.every((v) => v.secretCount === 4),
+    `expected every vault to report 4 secrets, got ${JSON.stringify(res.body.vaults.map((v) => v.secretCount))}`);
+});
+
+test("issues the same number of queries for 3 vaults as for 30", async (t) => {
+  const app = await serve("u_me");
+  t.after(() => app.close());
+
+  const small = await countQueries(() => app.get("/api/credshare/teams/small/vaults"));
+  const big = await countQueries(() => app.get("/api/credshare/teams/big/vaults"));
+
+  assert.equal(big, small,
+    `vault listing must not scale with vault count: 3 vaults took ${small} queries, 30 took ${big}`);
+  // And the constant is small -- membership check plus the listing itself.
+  assert.ok(small <= 4, `expected a handful of queries, got ${small}`);
+});
+
+test("refuses a caller who is not a member", async (t) => {
+  const app = await serve("u_outsider");
+  t.after(() => app.close());
+
+  const res = await app.get("/api/credshare/teams/small/vaults");
+  assert.equal(res.status, 403);
+});
+
+test("404s an unknown team", async (t) => {
+  const app = await serve("u_me");
+  t.after(() => app.close());
+
+  const res = await app.get("/api/credshare/teams/nope/vaults");
+  assert.equal(res.status, 404);
+});