diff --git a/README.md b/README.md index e1bda545..bbb159f6 100644 --- a/README.md +++ b/README.md @@ -974,10 +974,16 @@ carries a workflow change. ### Pin bumps -With `--bump `, repeatable, the watcher keeps each named -repository's flake input on the watched repository at the watched base, so -every push to the base reaches those repositories with no one touching them. -It needs `nix` on `PATH`. Once a pass sees the base at a commit a target does +With `--bump`, the watcher keeps the flake input on the watched repository at +the watched base in every other repository the App is installed on, so every +push to the base reaches those repositories with no one touching them. +Installing the App on a repository enrolls it and uninstalling it stops its +bumps; there is no list to keep. Each time the base moves, a pass lists every +installation's repositories, reads each one's `flake.lock` on its default +branch without cloning it, and keeps those that lock the watched repository +through exactly one root input as targets; a repository locking it through +several is reported and skipped. Discovery needs the installations to grant +`contents: read`. It needs `nix` on `PATH`. Once a pass sees the base at a commit a target does not pin yet, it opens or updates that target's one `quest-bump` PR as the App: 1. It reads the target's `flake.lock` on its default branch for the one root diff --git a/quest/m0/pin-bumps/discover.md b/quest/m0/pin-bumps/discover.md deleted file mode 100644 index 5e7deb04..00000000 --- a/quest/m0/pin-bumps/discover.md +++ /dev/null @@ -1,30 +0,0 @@ -# [S] The bump pass finds its own targets - -## Goal - -`quest land watch` bumps every repository its App is installed on whose -`flake.lock` locks the watched repository through exactly one root input, with no list of -repositories on the host: installing moq-bot on a repository enrolls it, and -uninstalling it stops its bumps. - -## Plan - -Decided 2026-11-12 (maintainer, /quest-plan; see the -[epic](/quest/m0/pin-bumps/README.md)): ✅ discover the targets (rejected: -`--bump ` flags in the host's configuration, which went stale). -Replace the repeatable `--bump ` with one switch; nothing is in -production, so drop the list form. - -A pass lists the App's installations and their repositories, reads each -default branch's `flake.lock`, and treats the repositories that lock the -watched repository through exactly one root input as targets, which is what -the bump needs (`questInput` in `src/cli/land-bump.ts`). Skip the watched repository itself. Rereading -every repository each pass costs API quota: keep it to what a pass needs, such -as only when the base moved. - -Update README.md's "Pin bumps" section and its examples to the switch, -matching `quest land watch --help`. - -Tests against a fake GitHub: a repository that locks quest is bumped, one -that doesn't is never cloned, and one dropped from the installation is not -bumped again. diff --git a/src/cli/land-bump.ts b/src/cli/land-bump.ts index 1f3ad1f6..7d308e1b 100644 --- a/src/cli/land-bump.ts +++ b/src/cli/land-bump.ts @@ -60,7 +60,10 @@ export interface BumpConnection { export interface BumpContext { source: Repository; - targets: Repository[]; + /** Every App installation's API access, reading all of its repositories, from which a pass finds its targets. */ + installations: () => Promise; + /** The targets found for the base commit last bumped, so discovery reruns only once the base moves. */ + discovered?: { sha: string; targets: Repository[] }; state: string; connect: (target: Repository) => BumpConnection; /** A read-only token for the source, which the tools fetch from GitHub. */ @@ -84,11 +87,11 @@ const PASSING = ["success", "neutral", "skipped"]; const name = (repository: Repository) => `${repository.owner}/${repository.repo}`; -/** The root input of `lock` that locks `source` on GitHub, and its locked and URL-pinned revisions. */ -export function questInput(lock: ObjectValue, source: Repository): { input: string; locked: string; pinned?: string } { +/** Every root input of `lock` that locks `source` on GitHub, and its locked and URL-pinned revisions. */ +function questInputs(lock: ObjectValue, source: Repository): { input: string; locked: string; pinned?: string }[] { const nodes = object(lock.nodes); const inputs = object(object(nodes[string(lock.root)]).inputs ?? {}); - const matches = Object.entries(inputs).flatMap(([input, node]) => { + return Object.entries(inputs).flatMap(([input, node]) => { // A follows path names another node's input, not a lock of its own. if (typeof node !== "string") return []; const entry = object(nodes[node]); @@ -105,6 +108,11 @@ export function questInput(lock: ObjectValue, source: Repository): { input: stri }, ]; }); +} + +/** The root input of `lock` that locks `source` on GitHub, and its locked and URL-pinned revisions. */ +export function questInput(lock: ObjectValue, source: Repository): { input: string; locked: string; pinned?: string } { + const matches = questInputs(lock, source); if (matches.length !== 1) throw new Error( `flake.lock has ${matches.length} root inputs locking github:${name(source)}, where a bump needs exactly one`, @@ -112,6 +120,76 @@ export function questInput(lock: ObjectValue, source: Repository): { input: stri return matches[0]; } +/** + * The repositories, other than the source, across every installation whose default branch's `flake.lock` + * locks the source through exactly one root input. Only the lock is read; nothing is cloned. A repository + * locking it through several inputs, or whose lock cannot be decoded, is reported in `output` and skipped. A failed + * request, for one repository's lock or an installation's token or listing, is recorded in `failures`, and + * the rest are still searched. + */ +export async function discoverTargets( + source: Repository, + installations: GitHubDependencies[], + output: string[], + failures: string[], +): Promise { + const targets: Repository[] = []; + for (const [index, deps] of installations.entries()) { + const github = new GitHub(deps); + try { + for (let page = 1; ; page++) { + const listed = object( + await (await github.request(`/installation/repositories?per_page=100&page=${page}`)).json(), + ); + const rows = array(listed.repositories); + for (const row of rows) { + const owner = string(object(row.owner).login); + const repo = string(row.name); + const target = { owner, repo }; + if (row.archived === true || name(target).toLowerCase() === name(source).toLowerCase()) continue; + let response: Response; + try { + response = await github.request(`/repos/${owner}/${repo}/contents/flake.lock`, { + allow: [404], + }); + } catch (error) { + // A failed request may be transient, so it leaves discovery incomplete and retried. + failures.push( + `discovery of ${name(target)}: ${error instanceof Error ? error.message : String(error)}`, + ); + continue; + } + if (response.status === 404) continue; + let found: number; + try { + const content = string(object(await response.json()).content); + found = questInputs( + object(JSON.parse(Buffer.from(content, "base64").toString("utf8"))), + source, + ).length; + } catch (error) { + output.push( + `bump skips ${name(target)}: its flake.lock cannot be read: ${error instanceof Error ? error.message : String(error)}`, + ); + continue; + } + if (found === 1) targets.push(target); + else if (found > 1) + output.push( + `bump skips ${name(target)}: flake.lock locks github:${name(source)} through ${found} root inputs`, + ); + } + if (rows.length < 100) break; + } + } catch (error) { + failures.push( + `discovery in installation ${index + 1}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + return targets; +} + /** * `flake` with the revision dropped from the source's URL, so flake.lock alone pins the input and each * bump is a lock-only change. The URL must appear exactly once. @@ -206,7 +284,14 @@ export async function bumpPass(context: BumpContext, sha: string): Promise, - deps: Pick, -): Promise { +/** A request to GitHub's API as the App itself, signed with its key. */ +function appRequest(options: { appId: string }, deps: Pick) { const encode = (value: unknown) => Buffer.from(JSON.stringify(value)).toString("base64url"); const now = Math.floor(deps.now() / 1000); const payload = `${encode({ alg: "RS256", typ: "JWT" })}.${encode({ iat: now - 60, exp: now + 540, iss: options.appId })}`; const signature = createSign("RSA-SHA256").update(payload).sign(readFileSync(keyPath())).toString("base64url"); - const request = async (path: string, body?: unknown) => { + return async (path: string, body?: unknown): Promise => { const response = await deps.fetch(`https://api.github.com${path}`, { method: body === undefined ? "GET" : "POST", headers: { @@ -29,19 +24,50 @@ export async function installationToken( ...(body === undefined ? {} : { body: JSON.stringify(body) }), }); if (!response.ok) throw new Error(`GitHub App request failed: HTTP ${response.status}`); - return object(await response.json()); + return response.json(); }; - const installation = await request(`/repos/${repository.owner}/${repository.repo}/installation`); +} + +/** Mint a short-lived token for this repository, never the installation's other repositories. */ +export async function installationToken( + repository: Repository, + options: { appId: string }, + permissions: Record, + deps: Pick, +): Promise { + const request = appRequest(options, deps); + const installation = object(await request(`/repos/${repository.owner}/${repository.repo}/installation`)); return string( - ( + object( await request(`/app/installations/${Number(installation.id)}/access_tokens`, { repositories: [repository.repo], permissions, - }) + }), ).token, ); } +/** The App's installations, each as a function minting a token for all of that installation's repositories. */ +export async function installations( + options: { appId: string }, + deps: Pick, +): Promise<((permissions: Record) => Promise)[]> { + const ids: number[] = []; + for (let page = 1; ; page++) { + const rows = array(await appRequest(options, deps)(`/app/installations?per_page=100&page=${page}`)); + // GitHub lists a suspended installation but refuses to mint its tokens. + ids.push(...rows.filter((row) => row.suspended_at == null).map((row) => Number(row.id))); + if (rows.length < 100) break; + } + return ids.map( + (id) => async (permissions) => + string( + object(await appRequest(options, deps)(`/app/installations/${id}/access_tokens`, { permissions })) + .token, + ), + ); +} + /** Replace `path` atomically, readable only by the daemon's user. */ export function save(path: string, value: unknown): void { writeFileSync(`${path}.new`, JSON.stringify(value), { mode: 0o600 }); diff --git a/src/cli/land-watch.ts b/src/cli/land-watch.ts index c1bde78d..0e26136a 100644 --- a/src/cli/land-watch.ts +++ b/src/cli/land-watch.ts @@ -27,6 +27,7 @@ import { EXCERPT, excerpt, installationToken, + installations, privateRepository, save, startDaemon, @@ -68,8 +69,8 @@ export interface WatchOptions { fixerTag: string[]; /** Patterns of GitHub branches mirrored into each fixer checkout as `origin/`. */ fixerBranch: string[]; - /** Repositories whose flake input on this repository each pass bumps to this repository's base. */ - bump: Repository[]; + /** Bump every repository the App is installed on whose flake input locks this repository, to this repository's base. */ + bump?: boolean; /** On the own queue, eject fork picks before pushing their build, so a fork's code never reaches a private runner. */ refuseForks?: boolean; } @@ -880,9 +881,6 @@ export async function ownQueue(github: GitHub, repository: Repository, base: str type Permissions = Record; -const same = (a: Repository, b: Repository) => - a.owner.toLowerCase() === b.owner.toLowerCase() && a.repo.toLowerCase() === b.repo.toLowerCase(); - /** * The installation token permissions the watcher asks for. On GitHub's merge queue, `api` reads the * queue's events, comments, and the `merge_group` logs (`actions: read`), and enqueues the armed picks @@ -950,9 +948,7 @@ export async function watch( options.fixer = resolve(options.fixer); accessSync(options.fixer, constants.X_OK); } - if (options.bump.length > 0) checkTools(); - if (options.bump.some((target) => same(target, repository))) - throw new Error("--bump names the watched repository, which cannot bump its own pin"); + if (options.bump) checkTools(); const now = { fetch, now: Date.now }; const mint = (wanted: Permissions) => installationToken(repository, options, wanted, now); // Each mode's token is minted only once a pass needs it, and the mode is read with neither, so an @@ -992,32 +988,38 @@ export async function watch( read, }; const connections = new Map(); - const bump: BumpContext | undefined = - options.bump.length === 0 - ? undefined - : { - source: repository, - targets: options.bump, - state: options.state, - reader: () => mint({ contents: "read" }), - tools: hostTools(repository, options.state), - connect: (target) => { - const key = `${target.owner}/${target.repo}`; - let connection = connections.get(key); - if (connection === undefined) { - const scoped = (wanted: Permissions) => installationToken(target, options, wanted, now); - const push: Permissions = { contents: "write", pull_requests: "write" }; - connection = { - remote: `https://github.com/${key}.git`, - deps: apiDependencies(() => scoped(BUMP_PERMISSIONS), { ...now, sleep, warn }), - token: (workflows) => scoped(workflows ? { ...push, workflows: "write" } : push), - read: authenticatedRead(() => scoped({ contents: "read" })), - }; - connections.set(key, connection); - } - return connection; - }, - }; + const bump: BumpContext | undefined = !options.bump + ? undefined + : { + source: repository, + installations: async () => + (await installations(options, now)).map((mintAll) => + apiDependencies(() => mintAll({ contents: "read", metadata: "read" }), { + ...now, + sleep, + warn, + }), + ), + state: options.state, + reader: () => mint({ contents: "read" }), + tools: hostTools(repository, options.state), + connect: (target) => { + const key = `${target.owner}/${target.repo}`; + let connection = connections.get(key); + if (connection === undefined) { + const scoped = (wanted: Permissions) => installationToken(target, options, wanted, now); + const push: Permissions = { contents: "write", pull_requests: "write" }; + connection = { + remote: `https://github.com/${key}.git`, + deps: apiDependencies(() => scoped(BUMP_PERMISSIONS), { ...now, sleep, warn }), + token: (workflows) => scoped(workflows ? { ...push, workflows: "write" } : push), + read: authenticatedRead(() => scoped({ contents: "read" })), + }; + connections.set(key, connection); + } + return connection; + }, + }; await daemonLoop(async () => { pruneLogs(options.state, deps.now(), deps.warn); const api = new GitHub(probe); diff --git a/src/cli/main.ts b/src/cli/main.ts index 1ad20ea0..9da19a0e 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -39,7 +39,7 @@ import { held, openDrafts, reach, target, upstream, upstreamRefs } from "./claim import { PENDING_MS } from "./admission"; import { gate, QUICK_MS } from "./gate"; import { gitHubDependencies, GitHubInputError, pullRequest, readRemote, remoteRepository } from "./github"; -import { mergedHolds, type Repository } from "../github/client"; +import { mergedHolds } from "../github/client"; import { type Hold, finish, land, release, take } from "./hold"; import { LAND_CHECK } from "../github/land"; import { apply, pushCommand } from "./land-apply"; @@ -789,14 +789,8 @@ export function main(argv: string[], out: Output): number | Promise { "On the own queue, eject each pick whose head is in a fork, or in a deleted repository, before pushing its build, so a fork's code never reaches a private queue runner", ) .option( - "--bump ", - "Keep this repository's flake input in another repository at this repository's base (repeatable): each pass opens or updates that repository's quest-bump PR as the App and lands it", - (value: string, previous: Repository[]) => { - const match = /^([\w.-]+)\/([\w.-]+)$/.exec(value); - if (!match) throw new InvalidArgumentError("expected owner/repo"); - return [...previous, { owner: match[1], repo: match[2] }]; - }, - [] as Repository[], + "--bump", + "Keep this repository's flake input, in every other repository the App is installed on whose flake.lock locks this repository through exactly one root input, at this repository's base: each pass opens or updates that repository's quest-bump PR as the App and lands it", ) .action((options: WatchOptions) => { const origin = readRemote(root(), "origin"); diff --git a/test/land-bump.test.ts b/test/land-bump.test.ts index d3027334..42c4f371 100644 --- a/test/land-bump.test.ts +++ b/test/land-bump.test.ts @@ -15,6 +15,7 @@ import { type BumpTools, bumpBody, bumpPass, + discoverTargets, hostTools, latestRuns, questInput, @@ -129,12 +130,18 @@ function deps(world: World): GitHubDependencies { if (mutation === "markPullRequestReadyForReview") pull.draft = false; return json({ data: {} }); } + if (url.pathname === "/installation/repositories") + return json({ repositories: [{ owner: { login: world.target.owner }, name: world.target.repo }] }); if (url.pathname === path) return json({ default_branch: "main", allow_auto_merge: world.allowAutoMerge }); if (url.pathname === `${path}/git/ref/heads/main`) return json({ object: { sha: world.base } }); if (url.pathname === `${path}/contents/flake.lock`) { const text = world.lock ?? - git(world.remote.slice("file://".length), "show", `${url.searchParams.get("ref")}:flake.lock`); + git( + world.remote.slice("file://".length), + "show", + `${url.searchParams.get("ref") ?? "main"}:flake.lock`, + ); return json({ content: Buffer.from(text).toString("base64") }); } if (url.pathname === `${path}/actions/runs`) { @@ -215,7 +222,7 @@ function tools(findings?: string): BumpTools & { locks: string[] } { function context(world: World, faked = tools()): BumpContext { return { source: SOURCE, - targets: [world.target], + installations: async () => [deps(world)], state: tempDir(), reader: async () => "reader", tools: faked, @@ -589,7 +596,7 @@ describe("bumpPass", () => { }); const good = world(); const ctx = context(good); - ctx.targets = [broken.target, good.target]; + ctx.discovered = { sha: NEW, targets: [broken.target, good.target] }; ctx.connect = (target) => target.repo === "broken" ? { remote: broken.remote, deps: deps(broken), token: async () => "push", read: readRemote } @@ -601,3 +608,118 @@ describe("bumpPass", () => { }); }); }); + +describe("discoverTargets", () => { + /** An installation listing `repositories`, each serving its flake.lock, or none when undefined. */ + function installation(repositories: Record, requests: string[]): GitHubDependencies { + return { + token: "test", + now: () => 0, + sleep: async () => {}, + warn: () => {}, + fetch: (async (input: URL) => { + const url = new URL(input); + requests.push(`${url.pathname}${url.search}`); + if (url.pathname === "/installation/repositories") + return Response.json({ + repositories: Object.keys(repositories).map((key) => { + const [owner, name] = key.split("/"); + return { owner: { login: owner }, name }; + }), + }); + const lock = /^\/repos\/([^/]+\/[^/]+)\/contents\/flake.lock$/.exec(url.pathname); + if (lock && repositories[lock[1]] === "forbidden") + return Response.json({ message: "Forbidden" }, { status: 403 }); + if (lock && repositories[lock[1]] !== undefined) + return Response.json({ content: Buffer.from(repositories[lock[1]]!).toString("base64") }); + if (lock) return Response.json({ message: "Not Found" }, { status: 404 }); + throw new Error(`unexpected request ${url.pathname}`); + }) as typeof fetch, + }; + } + + test("keeps the repositories locking the source once, across installations, skipping the source", async () => { + const requests: string[] = []; + const twice = lockFile(OLD, false).replace('"follows": [', '"again": "quest_2",\n "follows": ['); + const output: string[] = []; + const targets = await discoverTargets( + SOURCE, + [ + installation( + { + "moq-dev/widget": lockFile(OLD, false), + "moq-dev/quest": lockFile(OLD, false), + "moq-dev/plain": undefined, + "moq-dev/broken": "{}", + "moq-dev/other": lockFile(OLD, false).replace(/moq-dev/g, "other"), + }, + requests, + ), + installation({ "kixelated/twice": twice, "kixelated/gadget": lockFile(OLD, true) }, requests), + ], + output, + [], + ); + expect(targets).toEqual([ + { owner: "moq-dev", repo: "widget" }, + { owner: "kixelated", repo: "gadget" }, + ]); + expect(output).toEqual([ + "bump skips moq-dev/broken: its flake.lock cannot be read: invalid GitHub response: expected an object", + "bump skips kixelated/twice: flake.lock locks github:moq-dev/quest through 2 root inputs", + ]); + expect(requests).not.toContain("/repos/moq-dev/quest/contents/flake.lock"); + }); + + test("an unreadable repository or a failing installation does not stop the others", async () => { + const failing: GitHubDependencies = { + ...installation({}, []), + fetch: (async () => Response.json({ message: "Forbidden" }, { status: 403 })) as typeof fetch, + }; + const output: string[] = []; + const failures: string[] = []; + const targets = await discoverTargets( + SOURCE, + [failing, installation({ "moq-dev/locked": "forbidden", "moq-dev/widget": lockFile(OLD, false) }, [])], + output, + failures, + ); + expect(targets).toEqual([{ owner: "moq-dev", repo: "widget" }]); + expect(output).toEqual([]); + expect(failures).toHaveLength(2); + expect(failures[0]).toMatch(/^discovery in installation 1:/); + expect(failures[1]).toMatch(/^discovery of moq-dev\/locked:/); + }); +}); + +describe("bumpPass discovery", () => { + test("never clones a repository that does not lock the source", async () => { + const plain = world({ lock: lockFile(OLD, false).replace(/moq-dev/g, "other") }); + const ctx = context(plain); + ctx.connect = () => { + throw new Error("connected to a repository that is no target"); + }; + await expect(bumpPass(ctx, NEW)).resolves.toEqual([]); + expect(plain.pulls).toHaveLength(0); + }); + + test("rediscovers once the base moves, so an uninstalled repository is not bumped again", async () => { + const enrolled = world(); + const ctx = context(enrolled); + let installed = true; + let listings = 0; + ctx.installations = async () => { + listings++; + return installed ? [deps(enrolled)] : []; + }; + await bumpPass(ctx, NEW); + await bumpPass(ctx, NEW); + expect(listings).toBe(1); + expect(enrolled.pulls).toHaveLength(1); + installed = false; + const before = enrolled.requests.length; + await bumpPass(ctx, "3".repeat(40)); + expect(listings).toBe(2); + expect(enrolled.requests.slice(before)).toEqual([]); + }); +});