Public: anyone can read this; the crew can change it.
/apps and the dashboard query once per row again: two N+1 guards are red #503
Check · Review · Land · Live done — Landed and live in production.
The dashboard no longer runs extra queries per row: with 30 apps and 30 assigned issues, `/` runs the same number of SQL statements as with 5. All 13 tests in `test/surfaces-adversarial.test.ts` pass, and `bun run check` is green.
shipped production is serving it deploy 2fa0c24
Fixes the dashboard N+1 from #292: with 30 apps and 30 assigned issues, / now runs the same number of SQL statements as with 5. All 13 tests in test/surfaces-adversarial.test.ts pass, and that file now runs in the gate.
The extra query. canSeeOwnerFacts (which is canWrite) ran once per repository. Each call cost an owner lookup (SELECT * FROM user WHERE id = ?1) plus the team-permission read in orgAccess. On the dashboard it came in through needsYouCard → askCountFor → askCandidatesFor in src/web/studio/asks.ts, which checked every visible repo (up to 200) this way before reading pulls. With 30 org apps the page went from 38 to 88 statements, which turned both dashboard guards red. shelfRows (src/web/studio/shelf.ts) and the board's scope (src/web/pages/board.ts) did the same per-row check. The /apps guard named in the issue already passed on main.
The fix.
orgAccessByOrg(src/core/orgs.ts): returns the viewer's highest permission in each org they belong to, from one query.ownerFactRepoIds(src/core/repos.ts): answerscanSeeOwnerFactsfor a whole list from that one read, usingcanWrite's rules exactly (owner, site admin, or a write/admin team in the owning org).- Not
writableRepoIds: its SQL (REPO_WRITABLE) also admits crew members on the repo, which would have changed who sees owner facts. - New test in
test/orgs.test.ts: checks the batched answer againstcanSeeOwnerFactsrepo by repo for an admin, a writer, a reader, an outsider and a signed-out viewer, and that it runs at most one query.
The gate. scripts/suite.ts and its EXCLUDED list no longer exist; the gate is the file list in forge.json's check.run. This pull adds test/surfaces-adversarial.test.ts and test/orgs.test.ts to it. The merge from main conflicted only on that line: main had added test/web-apps.test.ts, and the merged list keeps all three. No existing test was changed.
bun run check is green after the merge (637 tests in 43 files). In an earlier full run before the merge, test/fork-flow.test.ts › "a mirror whose git is not ready yet is fetched again, and lands" failed once. This change doesn't touch that code; the file passed 15/15 twice on its own with the gate's timeout, and every full run since has been green.
Not done: the single-repo pages (now, app, previews) still call canSeeOwnerFacts once per page; that is one call per page, not per row.
To try it: bun test test/surfaces-adversarial.test.ts test/orgs.test.ts, then bun run check.
🤖 Generated with Claude Code
The forge now accepts check.run. On main it was already 998 characters. Merging main into this branch added test/fleet-ops.test.ts, which pushed the list past the forge's 1000-character limit together with the two files this branch gates (test/surfaces-adversarial.test.ts, test/orgs.test.ts). I fixed it by moving the list of test files out of forge.json and into a file, without dropping any of them.
scripts/gate.txtis a new file that lists the 44 test files the forge's check runs, one per line. Blank lines and#comments are skipped. It is the merge ofmain's list and this branch's two extra files.forge.json:check.runis nowbun x tsc --noEmit && bun scripts/test-shards.ts --timeout 60000 --gate scripts/gate.txt.scripts/shards.ts: a newparseGatereads the list. One bad line, a file listed twice, or an empty list rejects the whole file, so a typo can't quietly make the gate run fewer tests.scripts/test-shards.ts: a new--gate <file>option adds the listed files to any named on the command line. After that it works as before: it checks every file exists and weighs files by time fromscripts/test-times.json.bun run check:recordstill works, because it still findsscripts/test-shards.tsonce incheck.run.test/shards.test.tshas new tests forparseGate(comments,./, bad lines, repeated files, empty list). Two more guard the real repo: every file inscripts/gate.txtexists, andcheck.runstays under 1000 characters and points at the list.README.mdandCLAUDE.mdnow say where the gated test files are listed and why.
bun x tsc --noEmit is clean and test/shards.test.ts passes (28 tests). A full bun run check ran 670 tests in 44 files with one failure: "a newer commit supersedes a check still waiting for the same environment" in test/checks.test.ts expected the middle check to be cancelled but got ok. That file doesn't touch anything this change edits, and all 23 of its tests passed when I re-ran it alone. It looks like a timing-sensitive test failing under parallel load, so I left it unchanged and did not re-run the full gate. The merge commit is pushed.
To try it: run bun run check. The first line should read test-shards: 44 files in N shards.
🤖 Generated with Claude Code
Operator: main moved under this branch (forge.json's check command was shortened, among others). Merge main into the branch, resolve the conflicts keeping this change's intent, run the check locally, and push.
/apps, the dashboard, the shelf and the board now check owner facts with one query per page instead of one per row, so both N+1 guards pass again. The fix is the earlier commit 1e8cfed: src/core/repos.ts and src/core/orgs.ts load the facts for all rows at once, and board.ts, shelf.ts and asks.ts call that instead of asking once per row. test/orgs.test.ts now covers it.
The guards live in test/surfaces-adversarial.test.ts and test/orgs.test.ts, and the check never ran either file, which is how both went red unnoticed. Both are now on the test list in forge.json's check.run.
This push merges main into the branch. Main shortened check.run to -- name … short names (splitArgs in scripts/shards.ts). That fixes the same 1000-character limit this branch's scripts/gate.txt and --gate flag were for. In the conflicts I took main's version and removed scripts/gate.txt, parseGate and their wording in README.md and CLAUDE.md, so the branch keeps only its fix and the two added test names (check.run is about 540 characters). bun run check passes locally: 710 tests across 50 files.
To try it, run bun run check, or open /apps and the dashboard on the preview with several apps and repos.
🤖 Generated with Claude Code
The dashboard no longer runs extra queries per row: with 30 apps and 30 assigned issues,
/runs the same number of SQL statements as with 5. All 13 tests intest/surfaces-adversarial.test.tspass, andbun run checkis green.The extra query. I logged every statement and the call stack.
canSeeOwnerFacts, which just callscanWrite, was being called once per repository. Each call cost one owner lookup (SELECT * FROM user WHERE id = ?1) and one team-permission read inorgAccess. On the dashboard it came in throughneedsYouCard→askCountFor→askCandidatesForinsrc/web/studio/asks.ts, which checked every visible repo (up to 200) this way before reading pulls. That took the page from 38 to 88 statements, and caused both red dashboard guards (including theneedsYouCardone).shelfRows(src/web/studio/shelf.ts) and the board's scope (src/web/pages/board.ts) did the same per-row check, so I changed them too. The/appsguard named in the issue already passed on thismain.The fix.
orgAccessByOrg(src/core/orgs.ts): returns the viewer's highest permission in each org they belong to, from one query.ownerFactRepoIds(src/core/repos.ts): the list version ofcanSeeOwnerFacts. It appliescanWrite's rules exactly (owner, site admin, or a write/admin team in the owning org) using that one read.writableRepoIds: its SQL (REPO_WRITABLE) also lets crew members on the repo through, whichcanWritedoesn't, so it would have changed who sees owner facts.test/orgs.test.tschecks the batched answer againstcanSeeOwnerFactsrepo by repo for an admin, a writer, a reader, an outsider and a signed-out viewer, and that the batch runs at most one query.The gate.
scripts/suite.tsand itsEXCLUDEDlist no longer exist; the gate is now the explicit file list inforge.json'scheck.run. I addedtest/surfaces-adversarial.test.tsandtest/orgs.test.tsto that list. No existing test was changed.On the first full gate run,
test/fork-flow.test.ts› "a mirror whose git is not ready yet is fetched again, and lands" failed once. This change doesn't touch that code; the file passes 15/15 on its own with the gate's timeout, and the next full run was green.Not done: other single-repo pages (now, app, previews) still call
canSeeOwnerFactsonce per page; that is one call per page, not per row.To try it:
bun test test/surfaces-adversarial.test.ts test/orgs.test.ts, thenbun run check.🤖 Generated with Claude Code
Closes #292