9adfe86 · ed4b08c · 950096f · 3101f28
@@ -0,0 +1,27 @@
/**
 * The repositories a person administers, in one statement: the SQL twin of
 * `canAdmin` for the lists that must not call it once per repository.
 * Ownership, a team with admin in the owning org (an org's Owners team is
 * one), or — for a forge admin — every repository. An agent administers
 * nothing: it proposes, and everything from write up stays with people. That
 * is stricter than `canAdmin`, which lets an agent administer a repository it
 * owns (the fork it pushes to); never wider.
 */
import type { Database } from "bun:sqlite";
import type { User } from "./users.ts";
      
        sentinelcrew
        18d ago
        outdated
      
      Low, for the follow-up. This matches the issue ("an agent gets none") but does not match canAdmin. personalOrDirect returns true when user.id === repo.ownerId whatever the user's kind, so an agent that owns a repository, such as the fork it pushes to, gets canAdmin true and gets nothing here. The difference is safe because this function is narrower, never wider. Two fixes to choose from: say "stricter than canAdmin for agent-owned forks" in the doc comment, or add a bot-owned repository to the test and settle which answer is right before /boards switches over.
    
export function adminRepoIds(db: Database, user: User | undefined): Set<string> {
  if (user === undefined || user.kind === "agent") return new Set();
  const rows = db
      
        junipercrew
        17d ago
        
      
      Nothing calls this yet. Wire /boards in the follow-up so the function is reachable and not dead code.
    
    .query<{ id: string }, [string, number]>(
      `SELECT repo.id FROM repo
       WHERE ?2 = 1
          OR repo.owner_id = ?1
          OR repo.owner_id IN (
            SELECT team.org_id FROM team
            JOIN team_member ON team_member.team_id = team.id
            WHERE team_member.user_id = ?1 AND team.permission = 'admin')`,
    )
    .all(user.id, user.isAdmin ? 1 : 0);
  return new Set(rows.map((r) => r.id));
}
@@ -0,0 +1,144 @@
/** One statement answers the repositories a person administers (#7056). */
import { afterAll, beforeAll, expect, test } from "bun:test";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createApp, type App } from "../src/app.ts";
import { adminRepoIds } from "../src/core/admin_repos.ts";
import { addTeamMember, createOrg, createTeam } from "../src/core/orgs.ts";
import { canAdmin, getRepoById } from "../src/core/repos.ts";
import { createUser, getUserById, type User } from "../src/core/users.ts";
import { id } from "../src/kernel/id.ts";
import { expectBudget } from "./helpers/budget.ts";
import { quietLogs } from "./helpers/globals.ts";
quietLogs();
let dataDir: string;
let app: App;
let root: User;
let pat: User;
let bot: User;
const mine: Record<string, string> = {};
const person = async (username: string): Promise<User> => {
  const made = await createUser(app.db, {
    origin: "seed",
    trusted: true,
    username,
    email: `${username}@forge.local`,
    password: "a-good-passphrase",
    adminIfFirst: true,
  });
  if (!made.ok) throw new Error(String(made.error));
  return made.value;
};
const repo = (ownerId: string, name: string): string => {
  const rid = id();
  app.db
    .query(
      `INSERT INTO repo (id, owner_id, name, visibility, description, default_branch)
       VALUES (?1, ?2, ?3, 'private', '', 'main')`,
    )
    .run(rid, ownerId, name);
  return rid;
};
beforeAll(async () => {
  dataDir = await mkdtemp(join(tmpdir(), "forge-admin-repos-"));
  app = createApp({ dataDir, port: 0 });
  root = await person("alice");
  pat = await person("pat");
  const other = await person("other");
  const org = (n: string, founder: User): User => {
    const r = createOrg(app.db, n, founder.id);
    if (!r.ok) throw new Error(r.error);
    return r.value;
  };
  const owned = org("owned", pat); // pat is in Owners
  const adminOrg = org("adminorg", other);
  const writeOrg = org("writeorg", other);
  const readOrg = org("readorg", other);
  const joined = (o: User, name: string, perm: "admin" | "write" | "read") => {
    const t = createTeam(app.db, o.id, name, perm);
    if (!t.ok) throw new Error(t.error);
    addTeamMember(app.db, t.value, pat.id);
  };
  joined(adminOrg, "leads", "admin");
  joined(writeOrg, "devs", "write");
  joined(readOrg, "viewers", "read");
  mine["own"] = repo(pat.id, "own");
  mine["org-owner"] = repo(owned.id, "a");
  mine["team-admin"] = repo(adminOrg.id, "b");
  mine["writer"] = repo(writeOrg.id, "c");
  mine["reader"] = repo(readOrg.id, "d");
  mine["foreign"] = repo(other.id, "e");
  const botId = id();
  app.db
    .query(
      `INSERT INTO user (id, username, email, password_hash, is_agent)
       VALUES (?1, 'bot', '[email protected]', '!', 1)`,
    )
    .run(botId);
  bot = getUserById(app.db, botId)!;
  app.db
    .query("INSERT INTO repo_crew (repo_id, user_id) VALUES (?1, ?2)")
    .run(mine["writer"]!, botId);
});
afterAll(async () => {
  app.close();
  await rm(dataDir, { recursive: true, force: true });
});
const every = (): string[] =>
  app.db.query<{ id: string }, []>("SELECT id FROM repo").all().map((r) => r.id);
test("each route to admin is in, writer-only and reader-only are out, and it equals canAdmin", () => {
  const got = adminRepoIds(app.db, pat);
  expect([...got].sort()).toEqual(
    [mine["own"]!, mine["org-owner"]!, mine["team-admin"]!].sort(),
  );
  for (const rid of every())
    expect(got.has(rid)).toBe(canAdmin(app.db, getRepoById(app.db, rid)!, pat));
});
test("a forge admin gets every repository, an agent and nobody get none", () => {
  expect(adminRepoIds(app.db, root).size).toBe(every().length);
  expect(adminRepoIds(app.db, bot).size).toBe(0);
  expect(adminRepoIds(app.db, undefined).size).toBe(0);
});
test("it is exactly one statement", () => {
  let n = 0;
  const db = app.db as unknown as { query: (...a: unknown[]) => unknown };
  const real = db.query.bind(db);
  db.query = (...a: unknown[]) => {
    n++;
    return real(...a);
  };
  try {
    adminRepoIds(app.db, pat);
  } finally {
    db.query = real;
  }
  expect(n).toBe(1);
});
test("on 2,000 repositories it costs about one scan of the repository table", () => {
  app.db.transaction(() => {
    for (let i = 0; i < 2000; i++) repo(i % 3 === 0 ? pat.id : root.id, `bulk${i}`);
  })();
  const best = (run: () => unknown): number => {
    let least = Infinity;
    for (let n = 0; n < 5; n++) {
      
        sentinelcrew
        18d ago
        outdated
      
      Nit. A wall-clock assertion inside the gated suite is a possible source of flakes (#1242, #1073). Locally the query takes about 0.7 ms against a 5 ms limit, so it passes with room today. If it ever goes red on a loaded runner, move it behind the per-test time budget instead of raising the number.
    
      const t = performance.now();
      run();
      least = Math.min(least, performance.now() - t);
    }
    return least;
      
        junipercrew
        17d ago
        
      
      The speed check is the one test here that depends on timing. If it flakes under a busy gate, assert the work instead (one statement, already pinned above) and drop it.
    
  };
  // The baseline is a plain scan of the same table, taken in the same run.
  const against = best(() => app.db.query("SELECT id FROM repo").all());
  expectBudget(best(() => adminRepoIds(app.db, pat)), { ms: 5, against });
  expect(adminRepoIds(app.db, pat).size).toBe(667 + 3);
});
		@@ -0,0 +1,27 @@
	1	/**
	2	* The repositories a person administers, in one statement: the SQL twin of
	3	* `canAdmin` for the lists that must not call it once per repository.
	4	* Ownership, a team with admin in the owning org (an org's Owners team is
	5	* one), or — for a forge admin — every repository. An agent administers
	6	* nothing: it proposes, and everything from write up stays with people. That
	7	* is stricter than `canAdmin`, which lets an agent administer a repository it
	8	* owns (the fork it pushes to); never wider.
	9	*/
	10	import type { Database } from "bun:sqlite";
	11	import type { User } from "./users.ts";
	12
		sentinelcrew 18d ago outdated Low, for the follow-up. This matches the issue ("an agent gets none") but does not match `canAdmin`. `personalOrDirect` returns true when `user.id === repo.ownerId` whatever the user's kind, so an agent that owns a repository, such as the fork it pushes to, gets `canAdmin` true and gets nothing here. The difference is safe because this function is narrower, never wider. Two fixes to choose from: say "stricter than canAdmin for agent-owned forks" in the doc comment, or add a bot-owned repository to the test and settle which answer is right before `/boards` switches over.
	13	export function adminRepoIds(db: Database, user: User \| undefined): Set<string> {
	14	if (user === undefined \|\| user.kind === "agent") return new Set();
	15	const rows = db
		junipercrew 17d ago Nothing calls this yet. Wire /boards in the follow-up so the function is reachable and not dead code.
	16	.query<{ id: string }, [string, number]>(
	17	`SELECT repo.id FROM repo
	18	WHERE ?2 = 1
	19	OR repo.owner_id = ?1
	20	OR repo.owner_id IN (
	21	SELECT team.org_id FROM team
	22	JOIN team_member ON team_member.team_id = team.id
	23	WHERE team_member.user_id = ?1 AND team.permission = 'admin')`,
	24	)
	25	.all(user.id, user.isAdmin ? 1 : 0);
	26	return new Set(rows.map((r) => r.id));
	27	}
		@@ -0,0 +1,144 @@
	1	/** One statement answers the repositories a person administers (#7056). */
	2	import { afterAll, beforeAll, expect, test } from "bun:test";
	3	import { mkdtemp, rm } from "node:fs/promises";
	4	import { tmpdir } from "node:os";
	5	import { join } from "node:path";
	6	import { createApp, type App } from "../src/app.ts";
	7	import { adminRepoIds } from "../src/core/admin_repos.ts";
	8	import { addTeamMember, createOrg, createTeam } from "../src/core/orgs.ts";
	9	import { canAdmin, getRepoById } from "../src/core/repos.ts";
	10	import { createUser, getUserById, type User } from "../src/core/users.ts";
	11	import { id } from "../src/kernel/id.ts";
	12	import { expectBudget } from "./helpers/budget.ts";
	13	import { quietLogs } from "./helpers/globals.ts";
	14
	15	quietLogs();
	16
	17	let dataDir: string;
	18	let app: App;
	19	let root: User;
	20	let pat: User;
	21	let bot: User;
	22	const mine: Record<string, string> = {};
	23
	24	const person = async (username: string): Promise<User> => {
	25	const made = await createUser(app.db, {
	26	origin: "seed",
	27	trusted: true,
	28	username,
	29	email: `${username}@forge.local`,
	30	password: "a-good-passphrase",
	31	adminIfFirst: true,
	32	});
	33	if (!made.ok) throw new Error(String(made.error));
	34	return made.value;
	35	};
	36	const repo = (ownerId: string, name: string): string => {
	37	const rid = id();
	38	app.db
	39	.query(
	40	`INSERT INTO repo (id, owner_id, name, visibility, description, default_branch)
	41	VALUES (?1, ?2, ?3, 'private', '', 'main')`,
	42	)
	43	.run(rid, ownerId, name);
	44	return rid;
	45	};
	46
	47	beforeAll(async () => {
	48	dataDir = await mkdtemp(join(tmpdir(), "forge-admin-repos-"));
	49	app = createApp({ dataDir, port: 0 });
	50	root = await person("alice");
	51	pat = await person("pat");
	52	const other = await person("other");
	53	const org = (n: string, founder: User): User => {
	54	const r = createOrg(app.db, n, founder.id);
	55	if (!r.ok) throw new Error(r.error);
	56	return r.value;
	57	};
	58	const owned = org("owned", pat); // pat is in Owners
	59	const adminOrg = org("adminorg", other);
	60	const writeOrg = org("writeorg", other);
	61	const readOrg = org("readorg", other);
	62	const joined = (o: User, name: string, perm: "admin" \| "write" \| "read") => {
	63	const t = createTeam(app.db, o.id, name, perm);
	64	if (!t.ok) throw new Error(t.error);
	65	addTeamMember(app.db, t.value, pat.id);
	66	};
	67	joined(adminOrg, "leads", "admin");
	68	joined(writeOrg, "devs", "write");
	69	joined(readOrg, "viewers", "read");
	70	mine["own"] = repo(pat.id, "own");
	71	mine["org-owner"] = repo(owned.id, "a");
	72	mine["team-admin"] = repo(adminOrg.id, "b");
	73	mine["writer"] = repo(writeOrg.id, "c");
	74	mine["reader"] = repo(readOrg.id, "d");
	75	mine["foreign"] = repo(other.id, "e");
	76	const botId = id();
	77	app.db
	78	.query(
	79	`INSERT INTO user (id, username, email, password_hash, is_agent)
	80	VALUES (?1, 'bot', '[email protected]', '!', 1)`,
	81	)
	82	.run(botId);
	83	bot = getUserById(app.db, botId)!;
	84	app.db
	85	.query("INSERT INTO repo_crew (repo_id, user_id) VALUES (?1, ?2)")
	86	.run(mine["writer"]!, botId);
	87	});
	88	afterAll(async () => {
	89	app.close();
	90	await rm(dataDir, { recursive: true, force: true });
	91	});
	92
	93	const every = (): string[] =>
	94	app.db.query<{ id: string }, []>("SELECT id FROM repo").all().map((r) => r.id);
	95
	96	test("each route to admin is in, writer-only and reader-only are out, and it equals canAdmin", () => {
	97	const got = adminRepoIds(app.db, pat);
	98	expect([...got].sort()).toEqual(
	99	[mine["own"]!, mine["org-owner"]!, mine["team-admin"]!].sort(),
	100	);
	101	for (const rid of every())
	102	expect(got.has(rid)).toBe(canAdmin(app.db, getRepoById(app.db, rid)!, pat));
	103	});
	104
	105	test("a forge admin gets every repository, an agent and nobody get none", () => {
	106	expect(adminRepoIds(app.db, root).size).toBe(every().length);
	107	expect(adminRepoIds(app.db, bot).size).toBe(0);
	108	expect(adminRepoIds(app.db, undefined).size).toBe(0);
	109	});
	110
	111	test("it is exactly one statement", () => {
	112	let n = 0;
	113	const db = app.db as unknown as { query: (...a: unknown[]) => unknown };
	114	const real = db.query.bind(db);
	115	db.query = (...a: unknown[]) => {
	116	n++;
	117	return real(...a);
	118	};
	119	try {
	120	adminRepoIds(app.db, pat);
	121	} finally {
	122	db.query = real;
	123	}
	124	expect(n).toBe(1);
	125	});
	126
	127	test("on 2,000 repositories it costs about one scan of the repository table", () => {
	128	app.db.transaction(() => {
	129	for (let i = 0; i < 2000; i++) repo(i % 3 === 0 ? pat.id : root.id, `bulk${i}`);
	130	})();
	131	const best = (run: () => unknown): number => {
	132	let least = Infinity;
	133	for (let n = 0; n < 5; n++) {
		sentinelcrew 18d ago outdated Nit. A wall-clock assertion inside the gated suite is a possible source of flakes (#1242, #1073). Locally the query takes about 0.7 ms against a 5 ms limit, so it passes with room today. If it ever goes red on a loaded runner, move it behind the per-test time budget instead of raising the number.
	134	const t = performance.now();
	135	run();
	136	least = Math.min(least, performance.now() - t);
	137	}
	138	return least;
		junipercrew 17d ago The speed check is the one test here that depends on timing. If it flakes under a busy gate, assert the work instead (one statement, already pinned above) and drop it.
	139	};
	140	// The baseline is a plain scan of the same table, taken in the same run.
	141	const against = best(() => app.db.query("SELECT id FROM repo").all());
	142	expectBudget(best(() => adminRepoIds(app.db, pat)), { ms: 5, against });
	143	expect(adminRepoIds(app.db, pat).size).toBe(667 + 3);
	144	});