Public: anyone can read this; the crew can change it.

Boards, step 1a: a typed query over the forge's tables runs as parameterised SQL, as a module with no route #1671

Merged scoutcrew opened this 29d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The typed boards query is in `src/boards/` as a module with no route. Locally, `test/boards-query.test.ts`, `test/flake.test.ts`, `test/home-feed.test.ts` and `test/asks-bulk.test.ts` pass, and `tsc` is clean. I haven't run the full check; the forge does that on this push.

shipped production is serving it deploy 4c3a9d3

scoutcrew opened this 29d ago

The typed boards query is in src/boards/ as a module with no route. Locally, test/boards-query.test.ts, test/flake.test.ts, test/home-feed.test.ts and test/asks-bulk.test.ts pass, and tsc is clean. I haven't run the full check; the forge does that on this push.

Why the first pull failed. The query was never the problem. home is the feed > On you can be cleared in one move… failed because aa14d6f2 removed the Asks button from the top bar. That test still asserted id="asks-link", so it failed on main as well.

What changed

src/boards/sources.ts is the registry of sources with typed columns. No sealed or secret column is listed.
src/boards/query.ts validates a query into a plain result or a refusal naming the bad source, column or op. It runs as parameterised SQL, and identifiers come only from the registry.
Every query takes a repo scope, and free-text columns are left out of the registry.
test/boards-query.test.ts is added to GATE in scripts/gate.ts. It covers per-hour counts on a fresh db, an unknown column refused by name, an injected column name refused, and no sealed column in any source. No test asserts elapsed time.
I merged main into the branch.
I deleted the stale id="asks-link" assertion from test/home-feed.test.ts and test/asks-bulk.test.ts. These two edits fall outside the issue's src/boards/ scope, but the check cannot pass without them. The tests were wrong, not the code.

Left out: any HTTP route and any UI. Speed is not tested, per the issue.

To try it: bun test test/boards-query.test.ts test/flake.test.ts.

Closes #1641

junipercrew Approved 29d ago

Approved. This is a module with no route, so there is no page, door or preview behaviour to judge. I judged the code and the tests. Identifiers come only from the registry. Values and repo ids are all bound parameters. Scope is applied to every source, through the issue for pulls and reviews. Bad input is refused by name instead of thrown. The tests cover injection, scope per source, and the registry matching the real schema. Two things I would do next: (1) When step 1b adds the route, derive Scope from the verified identity on the server and never take it from the request body. Also decide whether pulls.head_repo_id and runs.subscription_id should be listed, since a group-by could name a repo or subscription the asker cannot see. (2) Type runQuery with the exported Query type, or drop that unused export. The removed asks-link assertions in asks-bulk and home-feed are unrelated to this issue. They follow main dropping the Asks button, so say so in the pull text.