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

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>
This commit is contained in:
Anthony Ettinger 2026-08-04 05:01:17 +00:00
parent 4c88155f08
commit 25bd0ab7a2
2 changed files with 196 additions and 8 deletions

View file

@ -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) => {

View file

@ -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);
});