feat(kix): show co-maintainers' projects in their inbox

cloudflare/README.md

@@ -167,8 +167,8 @@ and deletes its own object.
 ### Decision inbox
 
 After signing in, a maintainer lands on `/inbox` (`src/inbox/`): one active
-decision thread across the projects they enrolled, then what is up next, what
-is in progress, and their projects. Each item is one job target (`issues/1`,
+decision thread across their projects, then what is up next, what is in
+progress, and their projects. Each item is one job target (`issues/1`,
 `pulls/2`, a quest path), so an issue, its planning job, and that job's
 questions stay one item, and webhook updates and reconnects never add another
 prompt. Agents waiting on an answer come first, then failed jobs, new pull
@@ -186,6 +186,15 @@ awaits a decision, and any other whose issue, pull request, or quest is still
 observed, so finished work stays set aside. Retrying a failed planning
 interview keeps its `plans/<id>` target.
 
+A user's projects are those in `github_project_members`. Enrolling a project
+adds its enroller, and any other user joins by passing the maintainer check on
+one of its pages, such as its setup page; no extra GitHub calls list
+maintainers. Membership only decides what the inbox lists: every action still
+rechecks authority. A check GitHub denies drops the user's membership along with
+their read and skip state for the project, while one that cannot complete, such
+as a rate limit, fails the request and leaves membership as it is.
+Disconnecting a project removes its memberships.
+
 Each maintainer's read state lives in `inbox_items`, keyed by a version of what
 the item showed (a question, a GitHub change, a quest's text). A newer version
 is unread again, and skipping an item lasts until it changes. A finished or

cloudflare/migrations/0012_project_members.sql

@@ -0,0 +1,12 @@
+-- Whose decision inbox lists a project: each user who passed a current
+-- maintainer check on it, the enrolling user included. Membership is
+-- navigation, never authority; every action still rechecks GitHub. A check
+-- GitHub denies removes it, and disconnecting the project cascades.
+-- `github_projects.enrolledBy` stays a record of who enrolled it.
+CREATE TABLE github_project_members (
+  repository INTEGER NOT NULL REFERENCES github_projects(id) ON DELETE CASCADE,
+  user TEXT NOT NULL REFERENCES user(id) ON DELETE CASCADE,
+  PRIMARY KEY(repository, user)
+);
+CREATE INDEX github_project_members_user ON github_project_members(user);
+INSERT INTO github_project_members(repository, user) SELECT id, enrolledBy FROM github_projects;

cloudflare/src/github/service.ts

@@ -56,6 +56,8 @@ export class GitHubError extends Error {
 }
 /** A publication the bot cannot make, which needs a maintainer's decision rather than a retry. */
 export class PublicationBlocked extends GitHubError {}
+/** A maintainer check that was answered with a denial, rather than one that could not complete. */
+export class NotMaintainer extends GitHubError {}
 
 /** One file a publication sets or, with null content, deletes. */
 /** One path's new content, or null to delete it, with its Git mode: a file, an executable, or a symlink. */
@@ -273,27 +275,61 @@ export class GitHubService {
 		});
 	}
 
+	/**
+	 * Verify current maintainer authority, and keep the user's inbox membership
+	 * of an enrolled project in step with the answer. Membership lists the
+	 * project; it never stands in for this check. A check that cannot complete,
+	 * such as an outage or a rate limit, fails without changing membership.
+	 */
 	async #maintainer(token: string, repo: GitHubRepository, userId: string) {
+		const db = this.#env.DB;
+		try {
+			await this.#authority(token, repo, userId);
+		} catch (error) {
+			if (error instanceof NotMaintainer)
+				await db.batch([
+					db
+						.prepare("DELETE FROM github_project_members WHERE repository=? AND user=?")
+						.bind(repo.id, userId),
+					db.prepare("DELETE FROM inbox_items WHERE repository=? AND user=?").bind(repo.id, userId),
+				]);
+			throw error;
+		}
+		await db
+			.prepare(
+				"INSERT INTO github_project_members(repository,user) SELECT id,? FROM github_projects WHERE id=? ON CONFLICT DO NOTHING",
+			)
+			.bind(userId, repo.id)
+			.run();
+	}
+
+	async #authority(token: string, repo: GitHubRepository, userId: string) {
 		const account = await this.#env.DB.prepare(
 			"SELECT accountId FROM account WHERE userId=? AND providerId='github' LIMIT 1",
 		)
 			.bind(userId)
 			.first<{ accountId: string }>();
-		if (!account || !/^\d+$/.test(account.accountId)) throw new GitHubError("Linked GitHub identity required");
+		if (!account || !/^\d+$/.test(account.accountId)) throw new NotMaintainer("Linked GitHub identity required");
 		const user = await this.#api<{ id: number; login: string }>(token, `/user/${account.accountId}`);
 		if (String(user.id) !== account.accountId || !name(user.login))
 			throw new GitHubError("GitHub identity changed");
+		// Every caller has already read the repository with this token, and the
+		// user resolved above, so a 404 here means the user is no collaborator.
 		const permission = await this.#api<{ permission: string; role_name: string; user: { id: number } }>(
 			token,
 			`${repositoryPath(repo)}/collaborators/${user.login}/permission`,
-		);
+		).catch((error: unknown) => {
+			if (error instanceof GitHubError && error.status === 404)
+				throw new NotMaintainer("Current GitHub maintainer authority required");
+			throw error;
+		});
 		// GitHub maps maintain to legacy permission=write. Do not accidentally
 		// grant maintainer authority to ordinary writers or custom roles.
 		if (
 			String(permission.user.id) !== account.accountId ||
 			(permission.permission !== "admin" && permission.role_name !== "maintain")
 		)
-			throw new GitHubError("Current GitHub maintainer authority required");
+			throw new NotMaintainer("Current GitHub maintainer authority required");
 	}
 
 	async #authorized<T>(
@@ -349,11 +385,12 @@ export class GitHubService {
 		return row?.state ?? null;
 	}
 
+	/** The enrolled projects whose inbox the user belongs to. */
 	async projects(userId: string): Promise<Enrollment[]> {
-		// Remembered selections are navigation only, never an authority grant.
+		// Membership is navigation only, never an authority grant.
 		return (
 			await this.#env.DB.prepare(
-				"SELECT id,owner,name,state FROM github_projects WHERE enrolledBy=? ORDER BY owner,name",
+				"SELECT p.id,p.owner,p.name,p.state FROM github_projects p JOIN github_project_members m ON m.repository=p.id WHERE m.user=? ORDER BY p.owner,p.name",
 			)
 				.bind(userId)
 				.all<Enrollment>()
@@ -665,6 +702,10 @@ export class GitHubService {
 							`INSERT INTO github_projects(id,owner,name,installation,state,enrolledBy,updatedAt) VALUES(?,?,?,?,'enabled',?,?)
 					ON CONFLICT(id) DO UPDATE SET owner=excluded.owner,name=excluded.name,installation=excluded.installation,state=excluded.state,updatedAt=excluded.updatedAt`,
 						).bind(repo.id, repo.owner, repo.name, installation, userId, Date.now()),
+						// The project did not exist when the check above ran.
+						this.#env.DB.prepare(
+							"INSERT INTO github_project_members(repository,user) VALUES(?,?) ON CONFLICT DO NOTHING",
+						).bind(repo.id, userId),
 					]);
 				}
 				return [];

cloudflare/src/inbox/model.ts

@@ -5,7 +5,7 @@
 
 import { parse } from "quest/core";
 import { encodePath } from "../board/model";
-import { GitHubError, type Enrollment, GitHubService } from "../github/service";
+import { GitHubError, type Enrollment, GitHubService, NotMaintainer } from "../github/service";
 import type { Activity, ProjectState } from "../github/state";
 import { type Prompt, parsePrompt } from "../jobs/prompt";
 import { hub } from "../jobs/routes";
@@ -196,21 +196,24 @@ export function arrange(items: Item[]) {
 export type Runners = Awaited<ReturnType<typeof store.listRunners>>;
 
 /**
- * Load the inbox across the projects a maintainer enrolled. Every project
- * rechecks current GitHub authority; one that fails is reported, not hidden.
+ * Load the inbox across the projects the user is a member of. Every project
+ * rechecks current GitHub authority. A denied check ends the membership and
+ * hides the project; one that cannot complete is reported, not hidden.
  */
 export async function load(env: Env, user: string) {
 	const github = new GitHubService(env);
-	const projects: ProjectView[] = await Promise.all(
-		(await github.projects(user)).map(async (project) => {
+	const views = await Promise.all(
+		(await github.projects(user)).map(async (project): Promise<ProjectView | null> => {
 			try {
 				return { project, state: await github.projectState(project, user), error: null };
 			} catch (error) {
+				if (error instanceof NotMaintainer) return null;
 				if (!(error instanceof GitHubError)) throw error;
 				return { project, state: null, error: error.message };
 			}
 		}),
 	);
+	const projects = views.filter((view) => view !== null);
 	const usable = projects.filter((view) => !view.error);
 	const found = usable.map((view) => entities(view.project, view.state));
 	const [reads, jobs] = await Promise.all([

cloudflare/src/inbox/page.tsx

@@ -529,12 +529,13 @@ export function InboxPage(props: InboxProps) {
 					</ul>
 				) : (
 					<p>
-						No enrolled projects yet. <a href="/projects">Enroll a project</a> to get its issues, pull
-						requests, and ready quests here.
+						No enrolled projects yet. <a href="/projects">Enroll a project</a>, or open one a co-maintainer
+						enrolled, to get its issues, pull requests, and ready quests here.
 					</p>
 				)}
 				<p class="note-small muted">
-					Lists projects you enrolled. Runners and accounts are set up with <code>quest runner</code>.
+					Lists enrolled projects you maintain, once you open each one's setup page. Runners and accounts are
+					set up with <code>quest runner</code>.
 				</p>
 			</section>
 			{props.skipped.length > 0 && (

cloudflare/test/inbox.test.ts

@@ -4,7 +4,7 @@ import { app } from "../src/index";
 import { questHref } from "../src/inbox/model";
 import * as store from "../src/jobs/store";
 import { privateKey } from "./github-key";
-import { origin } from "./login-fixtures";
+import { identity, oauth, origin } from "./login-fixtures";
 import { commit, delivery, head, maintainerOf, project, pull } from "./project-fixtures";
 
 const testEnv = { ...env, GITHUB_APP_PRIVATE_KEY: privateKey };
@@ -14,6 +14,7 @@ const jobs = "/projects/github/upstream/project/jobs";
 let user = "";
 let cookie = "";
 let github: Awaited<ReturnType<typeof maintainerOf>>["github"];
+let mock: Awaited<ReturnType<typeof maintainerOf>>["mock"];
 
 beforeEach(async () => {
 	await db.batch([
@@ -22,19 +23,49 @@ beforeEach(async () => {
 		db.prepare("DELETE FROM github_projects"),
 		db.prepare("DELETE FROM github_installations"),
 	]);
-	({ user, cookie, github } = await maintainerOf());
+	({ user, cookie, github, mock } = await maintainerOf());
 });
 afterEach(() => vi.restoreAllMocks());
 
-async function inbox(query: Record<string, string> = {}) {
+async function inbox(query: Record<string, string> = {}, session = cookie) {
 	const response = await app.request(
 		`${origin}/inbox?${new URLSearchParams(query)}`,
-		{ headers: { Cookie: cookie } },
+		{ headers: { Cookie: session } },
 		testEnv,
 	);
 	expect(response.status).toBe(200);
 	return response.text();
 }
+/** Sign in another GitHub user, who holds the maintain role when `maintains`. */
+async function other(maintains: boolean) {
+	const who = identity("github");
+	const session = (await oauth(who)).session!;
+	vi.spyOn(globalThis, "fetch").mockImplementation(mock);
+	if (maintains) github.maintainers.push(who.id);
+	const { userId } = (await db
+		.prepare("SELECT userId FROM account WHERE providerId='github' AND accountId=?")
+		.bind(who.id)
+		.first<{ userId: string }>())!;
+	return { session, id: userId };
+}
+/** Open the project's activity page, which checks maintainer authority. */
+function visit(session = cookie) {
+	return app.request(`${origin}/projects/github/upstream/project/state`, { headers: { Cookie: session } }, testEnv);
+}
+async function members() {
+	const { results } = await db
+		.prepare("SELECT user FROM github_project_members WHERE repository=10")
+		.all<{ user: string }>();
+	return results.map((row) => row.user);
+}
+/** How many items the user has read or skipped on the project. */
+async function reads(who = user) {
+	const row = await db
+		.prepare("SELECT count(*) AS n FROM inbox_items WHERE repository=10 AND user=?")
+		.bind(who)
+		.first<{ n: number }>();
+	return row!.n;
+}
 /** The active decision thread's markup. */
 function thread(html: string) {
 	return html.match(/<article class="ql-panel decision"[\s\S]*?<\/article>/)?.[0] ?? "";
@@ -504,15 +535,83 @@ describe("decision inbox", () => {
 		expect(html).not.toContain("Skipped and finished");
 	});
 
-	it("reports a project without current authority and hides its items", async () => {
+	it("shows a co-maintainer the project once they open it", async () => {
+		const co = await other(true);
+		let html = await inbox({}, co.session);
+		expect(html).toContain("No enrolled projects yet");
+		expect(html).not.toContain("Idea");
+		expect((await visit(co.session)).status).toBe(200);
+		expect(await members()).toEqual(expect.arrayContaining([user, co.id]));
+		html = await inbox({}, co.session);
+		expect(html).toContain("Idea");
+		expect(html).not.toContain("Unavailable");
+	});
+
+	it("never lists a project for a non-maintainer", async () => {
+		const outsider = await other(false);
+		const refused = await visit(outsider.session);
+		expect(refused.status).toBe(403);
+		expect(await refused.text()).toContain("Current GitHub maintainer authority required");
+		expect(await members()).toEqual([user]);
+		const html = await inbox({}, outsider.session);
+		expect(html).toContain("No enrolled projects yet");
+		expect(html).not.toContain("Idea");
+	});
+
+	it("drops a revoked maintainer along with their read state", async () => {
+		await submit("/projects/github/upstream/project/skip", [
+			["item", "issues/1"],
+			["version", "0"],
+		]);
+		expect(await reads()).toBe(1);
 		github.role = "write";
-		const html = await inbox({ done: "answer" });
-		expect(html).toContain("Unavailable: Current GitHub maintainer authority required");
+		let html = await inbox();
+		expect(html).not.toContain("Idea");
+		expect(html).not.toContain("Unavailable");
+		expect(html).toContain("No enrolled projects yet");
+		expect(await members()).toEqual([]);
+		expect(await reads()).toBe(0);
+		// Regaining the role does not restore membership until the maintainer opens the project.
+		github.role = "maintain";
+		expect(await inbox()).not.toContain("Idea");
+		expect((await visit()).status).toBe(200);
+		html = await inbox();
+		expect(html.match(/<h2>Up next<\/h2>[\s\S]*?<\/ol>/)![0]).toContain("Idea");
+		expect(html).not.toContain("Skipped and finished");
+	});
+
+	it("drops a maintainer removed from the repository", async () => {
+		await submit("/projects/github/upstream/project/skip", [
+			["item", "issues/1"],
+			["version", "0"],
+		]);
+		github.role = null;
+		const html = await inbox();
+		expect(html).not.toContain("Unavailable");
+		expect(html).toContain("No enrolled projects yet");
+		expect(await members()).toEqual([]);
+		expect(await reads()).toBe(0);
+	});
+
+	it("keeps membership when a maintainer check cannot complete", async () => {
+		await submit("/projects/github/upstream/project/skip", [
+			["item", "issues/1"],
+			["version", "0"],
+		]);
+		github.limited = true;
+		let html = await inbox({ done: "answer" });
+		expect(html).toContain("Unavailable: GitHub request failed (429)");
 		expect(html).not.toContain("Idea");
 		// With no decision left, the notice shows on the empty state.
 		expect(html).toContain("Answer recorded.");
 		expect(html).toContain("Nothing needs a decision right now.");
 		expect(await inbox({ done: "constructor" })).not.toContain('<p class="note" role="status">');
+		expect((await visit()).status).toBe(403);
+		expect(await members()).toEqual([user]);
+		expect(await reads()).toBe(1);
+		github.limited = false;
+		html = await inbox();
+		expect(html).toMatch(/Skipped and finished \(1\)[\s\S]*Idea/);
 	});
 
 	it("keeps paused projects' questions but offers no new work", async () => {