fix(ci): an unwritable checkout dir must not take CI down platform-wide #5598
2 changed files+138−2
Addedsrc/__tests__/repo-name-casing-ratchet.test.ts+106−0View fileUnifiedSplit
@@ -0,0 +1,106 @@
1/**
2 * A ratchet on case-sensitive repository lookups.
3 *
4 * Repository names are stored lowercase going forward (2c6cd93), but rows
5 * created before that keep their capitals — `ccantynz/Gluecron.com`,
6 * `ccantynz/Vapron`. Anything matching with `eq(repositories.name, x)` finds
7 * those only if the caller happened to type the same capitalisation, and
8 * returns "not found" otherwise.
9 *
10 * That failure is quiet in the worst way. A 404 that depends on how a client
11 * capitalised a URL is indistinguishable from "this repo does not exist", and
12 * a polling client reads it as silence. It was found in this session exactly
13 * that way: a CI monitor watched `/api/v2/repos/ccantynz/gluecron.com/...`
14 * and reported nothing at all while the run it was watching finished and
15 * failed, because the stored name is `Gluecron.com`.
16 *
17 * `loadRepoByPath` in src/lib/namespace.ts is the correct resolver and has
18 * been for months. The sweep to route everything through it has been pending
19 * since 2026-08-05 across 127 call sites, and a sweep that size is not
20 * something to do blind.
21 *
22 * So: a ratchet, not a gate. It cannot fix what is there, but it stops the
23 * class growing while it is worked down — and it fails when the number DROPS
24 * too, because a ratchet nobody tightens is just a number that used to be
25 * true.
26 *
27 * Not every site is a bug: some look up a name the platform itself generated
28 * and has never let a user type. The count is deliberately blunt anyway. A
29 * precise list would need a judgement per call site, and the judgement that
30 * matters here is simpler — do not add another one.
31 */
32
33import { describe, expect, test } from "bun:test";
34import { readdirSync, readFileSync, statSync } from "fs";
35import { join, resolve } from "path";
36
37/**
38 * The count as of 2026-08-31, after the v2 API resolver was fixed.
39 *
40 * Lower this when you convert call sites. Never raise it: if a change needs a
41 * new exact-name lookup, it almost certainly wants `loadRepoByPath` instead.
42 */
43const BASELINE = 127;
44
45const SRC = resolve(import.meta.dir, "..");
46const PATTERN = /eq\(\s*repositories\.name\s*,/g;
47
48function walk(dir: string, out: string[] = []): string[] {
49 for (const entry of readdirSync(dir)) {
50 if (entry === "node_modules" || entry === "__tests__") continue;
51 const full = join(dir, entry);
52 if (statSync(full).isDirectory()) walk(full, out);
53 else if (/\.tsx?$/.test(entry)) out.push(full);
54 }
55 return out;
56}
57
58function countExactNameLookups(): { total: number; files: string[] } {
59 const files: string[] = [];
60 let total = 0;
61 for (const file of walk(SRC)) {
62 const text = readFileSync(file, "utf8");
63 const hits = text.match(PATTERN);
64 if (hits) {
65 total += hits.length;
66 files.push(file.slice(SRC.length + 1).replace(/\\/g, "/"));
67 }
68 }
69 return { total, files };
70}
71
72describe("case-sensitive repo lookups do not multiply", () => {
73 test("no new exact-name lookups", () => {
74 const { total, files } = countExactNameLookups();
75 if (total > BASELINE) {
76 throw new Error(
77 `${total} exact-name repository lookups, baseline ${BASELINE}. ` +
78 `A new one was added. Use loadRepoByPath() from src/lib/namespace.ts — ` +
79 `it resolves case-insensitively and handles org-owned repos too. ` +
80 `Files: ${files.join(", ")}`
81 );
82 }
83 expect(total).toBeLessThanOrEqual(BASELINE);
84 });
85
86 test("the baseline is tightened when the sweep makes progress", () => {
87 const { total } = countExactNameLookups();
88 if (total < BASELINE) {
89 throw new Error(
90 `Only ${total} exact-name lookups remain (baseline ${BASELINE}). ` +
91 `Lower BASELINE to ${total} in this file so the ratchet keeps holding. ` +
92 `A ratchet nobody tightens is a number that used to be true.`
93 );
94 }
95 expect(total).toBe(BASELINE);
96 });
97
98 test("the correct resolver still exists and is still case-insensitive", () => {
99 // The ratchet points at loadRepoByPath. If that function stopped doing
100 // what this file says it does, every message above would be advice to
101 // adopt a resolver with the same bug.
102 const ns = readFileSync(resolve(SRC, "lib/namespace.ts"), "utf8");
103 expect(ns).toMatch(/export async function loadRepoByPath/);
104 expect(ns).toMatch(/lower\(\$\{repositories\.name\}\)\s*=\s*lower\(/);
105 });
106});
Modifiedsrc/routes/api-v2.ts+32−2View fileUnifiedSplit
@@ -159,18 +159,48 @@ async function repoPrivacyGate(c: any, next: any) {
159159
160160// ─── Helper ─────────────────────────────────────────────────────────────────
161161
162/**
163 * Resolve `:owner/:repo` for the REST API.
164 *
165 * CASE-INSENSITIVE, matching `loadRepoByPath` in src/lib/namespace.ts, which
166 * is what the web UI and the MCP tools already use. Until 2026-08-31 this
167 * function alone used exact equality, so `/api/v2/repos/ccantynz/gluecron.com`
168 * returned 404 while `/ccantynz/gluecron.com` in a browser rendered the repo
169 * and `/api/v2/repos/ccantynz/Gluecron.com` worked. A 404 that depends on the
170 * capitalisation a client happened to type is indistinguishable from "this
171 * repo does not exist", and a polling client reads it as silence — which is
172 * exactly how it was found: a CI monitor in this session watched that URL and
173 * reported nothing while the run it was watching finished and failed.
174 *
175 * Names are stored lowercase going forward (2c6cd93), but rows created before
176 * that keep their capitals, so exact matching is wrong for real data that
177 * exists today, not merely for careless clients.
178 *
179 * KNOWN GAP, deliberately not fixed here: this resolves USER-owned repos
180 * only. `loadRepoByPath` also resolves org-owned repos, so an organisation's
181 * repository is unreachable through the entire v2 API. That is not a one-line
182 * change — 44 call sites use `resolved.owner` for authorization
183 * (`user.id !== resolved.owner.id`), and an org id in that slot would deny
184 * every request rather than allow the wrong one. It needs its own work with
185 * its own tests, not a drive-by inside a casing fix.
186 */
162187async function resolveRepo(ownerName: string, repoName: string) {
163188 const [owner] = await db
164189 .select()
165190 .from(users)
166 .where(eq(users.username, ownerName))
191 .where(sql`lower(${users.username}) = lower(${ownerName})`)
167192 .limit(1);
168193 if (!owner) return null;
169194
170195 const [repo] = await db
171196 .select()
172197 .from(repositories)
173 .where(and(eq(repositories.ownerId, owner.id), eq(repositories.name, repoName)))
198 .where(
199 and(
200 eq(repositories.ownerId, owner.id),
201 sql`lower(${repositories.name}) = lower(${repoName})`
202 )
203 )
174204 .limit(1);
175205 if (!repo) return null;
176206
177207
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts