diff --git a/cloudflare/GITHUB.md b/cloudflare/GITHUB.md index cf5eacd9..c5f815d6 100644 --- a/cloudflare/GITHUB.md +++ b/cloudflare/GITHUB.md @@ -8,14 +8,14 @@ and installation tokens stay inside the Worker; agents receive neither. ## Permissions and events -| Repository permission | Why the installation grants it | Per-operation token | -| --------------------- | --------------------------------------------------------------------------- | ---------------------------------------------------------- | -| Metadata: read | Resolve repository identity and maintainer authority | Every operation | -| Contents: write | Read quest files, publish approved quests to service forks, explicit merges | Read for quest reads; write only for publishing or merging | -| Issues: write | Read native issues and publish approved comments | Read or write for that operation | -| Pull requests: write | Observe, create approved draft PRs, explicitly merge an exact head | Read or write for that operation | -| Checks: read | Observe checks on a commit | Check reads only | -| Commit statuses: read | Observe legacy CI status contexts | Check reads only | +| Repository permission | Why the installation grants it | Per-operation token | +| --------------------- | --------------------------------------------------------------------------- | -------------------------------------------------------------- | +| Metadata: read | Resolve repository identity and maintainer authority | Every operation | +| Contents: write | Read quest files, write job branches on service forks and iterated PR heads | Read for quest reads; write only for that publication or merge | +| Issues: write | Read native issues and publish approved comments | Read or write for that operation | +| Pull requests: write | Observe, open and update job PRs, explicitly merge an exact head | Read or write for that operation | +| Checks: read | Observe checks on a commit | Check reads only | +| Commit statuses: read | Observe legacy CI status contexts | Check reads only | The manifest subscribes to push, issues, issue comments, pull requests, reviews, checks and statuses. GitHub also sends installation lifecycle and repository @@ -29,8 +29,9 @@ does not retry a successful write. GitHub also expires tokens after an hour. ## Trusted boundary `src/github/service.ts` provides specific server-only operations for quest -reads, issue/PR reads, check/status reads, comments, approved quest publication, -draft PR creation, and an explicitly authorized merge with an expected head SHA. +reads, issue/PR reads, check/status reads, comments, job publication (see +[Publication](#publication)), and an explicitly authorized merge with an +expected head SHA. Callers pass a user ID obtained from an authenticated local session. The service resolves its linked OAuth account, looks up the current GitHub login by numeric ID, and verifies current repository admin or maintain authority. A browser's @@ -42,25 +43,122 @@ These methods are integration primitives, not public HTTP routes or an agent tool interface. Their trusted caller owns the user's approval of the exact output/action, product policy, branch protections and review/check gating. Webhook receipt never supplies that approval. No automatic comments, PRs, -merges or runner launches occur. Check reads accept a page number (default 1) and return up to 100 checks +merges or runner launches occur; a job publishes only what its maintainer +authorized, on behalf of that maintainer's current authority. Check reads accept a page number (default 1) and return up to 100 checks and status contexts with GitHub's totals; callers must request additional pages before treating the result as complete or approving a merge. The App does not own repositories. Configure `GITHUB_FORK_OWNER` as a separate -service-controlled namespace and install the App on each provisioned fork. -The operator provisions forks using GitHub's fork flow, subject to organization -policy and private-fork restrictions. An installation token cannot impersonate -a user to own a fork; access to upstream does not grant access to its fork. -Destination permissions/installations must be supplied independently. No fork -creation or organization provisioning permission is requested by this service. -Publication is restricted to `quest/*.md` on an existing `quest/...` branch in -that namespace. The service verifies the fork's immediate parent ID is the -upstream ID. PR creation uses an upstream-scoped token and an explicit head -owner and repository; for organization cross-repository PRs GitHub requires -`head_repo`. Installation tokens cannot span two owners, so the fork-scoped -token only pushes and the upstream token cannot grant maintainer edits on the -fork branch; PRs are created with `maintainer_can_modify: false`. -Agents cannot choose another namespace or obtain Git credentials. +service-controlled namespace and install the App on it. The operator provisions +each upstream's fork there under the upstream's repository name, using GitHub's +fork flow, subject to organization policy and private-fork restrictions. An +installation token cannot impersonate a user to own a fork; access to upstream +does not grant access to its fork. No fork creation or organization +provisioning permission is requested by this service. + +## Publication + +A running job publishes its work at `POST /api/runner/publish`, authenticated +with its runner's bearer token (`src/jobs/publish.ts`). The JSON request names +the `job` and `attempt`, the commit the changes build on (`head`, or `null` +before the job's branch exists), a commit `message`, and `changes`: up to 500 +files, 16 MiB in all, each `{ path, content, executable?, symlink? }` with +base64 content, or `null` content to delete. A symlink's content is its target. +Submodule changes cannot be published. An optional `pull: { title, body, draft }` +opens or updates the job's pull request, and `session` names the agent session +to resume if the publication needs a decision. Checkpoints publish work in +progress the same way, so a reassigned job continues from its branch. The reply +is `{ head, pull? }`, or `202 { decision, cancel }` when the job paused. + +Submitted files are data. The publisher never checks them out or runs their +scripts or hooks: it writes blobs, a tree, and a commit with the Git Data API +and moves one branch. Before writing anything it checks, in order: + +- The request comes from the job's current `(attempt, runner)` while it runs. + A cancelling attempt may still publish its partial work. A reassignment + increments the attempt, which fences out the previous runner. +- Every path is a plain work-tree path: no `.git`, `.` or `..` segment, no + absolute path, backslash, or control character. Each workflow publishes + only its own changes: `plan` only quest documents under `quest/` (Markdown + other than `AGENTS.md` and `CLAUDE.md`), `bootstrap` only `flake.nix` and + `flake.lock`, `start` and `iterate` their task's changes. No workflow changes + `.github/workflows/`, because the App does not request the workflows + permission. A refused change pauses the job with a `publication` decision + listing each path and reason, instead of being dropped; the maintainer's + answer resumes the agent's session. +- The job's requester still holds maintainer authority, and the repository's + identity, visibility, installation, and enrollment are checked live, as for + every other operation. +- The branch is where the job left it: absent before the first publication, + then the last commit kix published. A `head` that differs gets a reply naming + the commit to continue from. A branch changed outside the job, or one that + earlier work left on the fork, becomes a decision; it is never overwritten. + +`plan`, `start`, and `bootstrap` jobs write their branch on the service fork +`GITHUB_FORK_OWNER/`, whose immediate parent must be the upstream's +repository ID. An `iterate` job writes its pull request's actual head: an +upstream branch through an upstream-scoped token, or a service fork branch. +GitHub gives an installation no write access to a contributor's fork, so that +case pauses with a decision reporting the blocked update; kix never opens a +replacement pull request. No job writes a repository's default branch, and no +runner ever receives a credential. + +The App authors the commits as well as the pull requests: the service sets no +author or committer, so GitHub attributes and signs them as the App. +Contributor and model attribution comes from the agent's commit message and +pull request body, written to the repository's own instructions. + +Interruptions are reconciled before anything is replayed. The service creates +the commit, records it as pending under the attempt fence, and then moves the +branch with a fast-forward-only update. If the outcome is lost, the next +publication finds the pending commit on the branch and adopts it; until then, +for up to two minutes, other publications of the job wait. GitHub cannot check +kix's fence, so the fence is rechecked immediately before that update: a +fenced-out attempt's write lands only if it passed the check before the +reassignment, and the fast-forward check means it can never overwrite newer +work. Delivery looks for the branch's open pull request before creating one, +and reconciles a refused creation the same way, so a retry never opens a +duplicate. A pull request the maintainer closed is not replaced. + +Planning pull requests are always drafts, as are those of jobs asked to stop. +Other jobs set `draft`, which defaults to `true` while decisions or unfinished +work remain; a later delivery with `draft: false` marks the pull request ready. +kix never converts a ready pull request back. Delivery requires an enabled +project; a paused project's running jobs can still preserve their work on their +branches. Pull requests use `maintainer_can_modify: false`, because an +upstream-scoped token cannot grant edits on the fork branch. + +GitHub keeps pull request status, checks, and reviews. `mergePullRequest` +merges only an exact head a maintainer explicitly chose, with the first method +the repository allows: squash, then a merge commit, then rebase. GitHub's +branch rules and refusals stand, and an agent's recommendation is never +approval. + +Fork pull requests do not make privileged CI safe. Workflows that run a pull +request's code with secrets or write tokens, such as `pull_request_target` or +`workflow_run` jobs that check out its head, run contributed code with upstream +credentials. Upstream workflows must treat kix pull requests as untrusted +contributions like any other fork's. kix grants contributed code no upstream +credentials and never runs it in the publisher. Keep Actions disabled and +secrets absent on service forks, since the bot pushes contributed code there. + +Installation grants are broader than any one publication: `contents: write` +and `pull_requests: write` cover every installed repository. Each token names +one repository ID and only the permissions that step needs. Fork writes get +`contents: write` on the fork alone, pull request delivery gets +`pull_requests: write` on the upstream alone, and only an iteration on an +upstream head gets upstream `contents: write`. That token could write any +unprotected upstream branch; the service restricts it to the pull request's +head and refuses the default branch, and repositories should still protect +their default branch. + +Mocked tests cover this path. These GitHub behaviors still need a live check +against real installations ([operator setup](../quest/m0/kix/operator-setup.md)): +creating a fork-headed pull request with an upstream installation token while +the fork belongs to another installation; creating a fork commit whose parent +is an upstream commit pushed after the fork was made; GitHub's signature on +App-authored API commits; and pushing an upstream pull request head with an +installation token under the repository's branch rules. ## Webhooks and revocation @@ -101,9 +199,10 @@ reconcile current provider state rather than replay issue/PR payloads as facts. intact. Keep the existing `GITHUB_CLIENT_ID`/`GITHUB_CLIENT_SECRET` for OAuth login; they are separate credentials. Never commit conversion responses, PEM keys, installation tokens or `.dev.vars`. -3. Set `GITHUB_FORK_OWNER` to the service-controlled namespace. Provision its - forks/branches and install the public App there and on selected upstream - repositories. Existing installations must accept permission increases before +3. Set `GITHUB_FORK_OWNER` to the service-controlled namespace. Fork each + pilot upstream into it under the same repository name, disable Actions on + those forks and keep secrets off them, and install the public App there and + on selected upstream repositories. Existing installations must accept permission increases before operations needing those grants become available. GitHub's installation UI, not browser form claims, chooses repository access. 4. Run `bun run wrangler d1 migrations apply DB --remote`, then the normal diff --git a/cloudflare/README.md b/cloudflare/README.md index 08adc02c..ad3e9b71 100644 --- a/cloudflare/README.md +++ b/cloudflare/README.md @@ -111,6 +111,15 @@ An account limit pauses the job with an account decision; kix never switches accounts on its own. An answer requeues the job and resumes its session on the runner that holds it. +The current attempt publishes its work over HTTPS at `POST /api/runner/publish`, +not over its connection, so slow GitHub writes never hold up the hub. It sends +file changes as data; the Worker commits them as the GitHub App to the job's +one branch, which a reassigned attempt continues from, and opens or updates +the job's pull request. Each job records its branch's `head` and its `pull`. +Changes a workflow may not publish pause the job with a `publication` +decision. [GITHUB.md](GITHUB.md#publication) describes the request, its +validation, and how interrupted publications are reconciled. + ### Raw output Runners upload raw agent output as `output` messages of at most 1 MiB of UTF-8 @@ -199,8 +208,11 @@ replay, and ownership. Job tests race dispatch passes, reassign a disconnected runner's job and fence its late writes, and cover routing, questions, account limits, and cancellation. They drive real WebSocket connections through the hub, including a replaced connection and the alarm that reassigns work, and check the -maintainer-only routes. Output tests use local R2 for chunk ordering, retries, -the caps, expiry, and cleanup after a disconnect. No OAuth credentials or +maintainer-only routes. Publication tests run against a stateful fake of +GitHub's Git and pull request APIs: fenced and stale writes, workflow path +limits and their decisions, lost branch updates and deliveries, iterated heads, +and token scopes. Output tests use local R2 for chunk ordering, retries, the +caps, expiry, and cleanup after a disconnect. No OAuth credentials or Cloudflare account are required for checks. After changing bindings, regenerate types: diff --git a/cloudflare/migrations/0009_publication.sql b/cloudflare/migrations/0009_publication.sql new file mode 100644 index 00000000..9a8a809f --- /dev/null +++ b/cloudflare/migrations/0009_publication.sql @@ -0,0 +1,28 @@ +-- The commit the job's branch holds, as far as kix has published or observed +-- it. NULL until the first publication creates the branch; an iteration starts +-- at the pull request's head. +ALTER TABLE jobs ADD COLUMN head TEXT; +-- A commit whose branch update is in flight. If the Worker stops before +-- recording the outcome, the next publication finds it on the branch and +-- adopts it instead of treating it as someone else's write. +ALTER TABLE jobs ADD COLUMN pending TEXT; +ALTER TABLE jobs ADD COLUMN pendingAt INTEGER; +-- The pull request this job delivered, which later deliveries update. +ALTER TABLE jobs ADD COLUMN pull INTEGER; +-- Publication decisions are service-authored, unlike agent questions. SQLite +-- cannot alter a CHECK constraint, so rebuild the table. +CREATE TABLE job_questions_new ( + id TEXT PRIMARY KEY NOT NULL, + job TEXT NOT NULL REFERENCES jobs(id) ON DELETE CASCADE, + attempt INTEGER NOT NULL, + kind TEXT NOT NULL CHECK(kind IN ('agent','account','publication')), + prompt TEXT NOT NULL, + answer TEXT, + answeredBy TEXT REFERENCES user(id) ON DELETE SET NULL, + askedAt INTEGER NOT NULL, + answeredAt INTEGER, + UNIQUE(job,attempt) +); +INSERT INTO job_questions_new SELECT id,job,attempt,kind,prompt,answer,answeredBy,askedAt,answeredAt FROM job_questions; +DROP TABLE job_questions; +ALTER TABLE job_questions_new RENAME TO job_questions; diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index 9365ba4d..26bc8679 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -29,6 +29,20 @@ interface Repository { visibility: string; default_branch: string; license?: { spdx_id: string; name: string } | null; + allow_squash_merge?: boolean; + allow_merge_commit?: boolean; + allow_rebase_merge?: boolean; +} +interface PullHead { + number: number; + node_id: string; + html_url: string; + state: string; + draft: boolean; + title: string; + body: string | null; + created_at: string; + head: { ref: string; sha: string; repo: { id: number; name: string; owner: { login: string } } | null }; } export class GitHubError extends Error { constructor( @@ -38,6 +52,31 @@ export class GitHubError extends Error { super(message); } } +/** A publication the bot cannot make, which needs a maintainer's decision rather than a retry. */ +export class PublicationBlocked 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. */ +export type Change = { path: string; content: Uint8Array | null; mode: "100644" | "100755" | "120000" }; +/** What a job may publish: its workflow, its target, and its one branch. */ +export type Contribution = { + workflow: "plan" | "start" | "iterate" | "bootstrap"; + target: string; + branch: string; + pull: number | null; + /** When the job was created, in milliseconds; older pull requests on its branch are not its own. */ + createdAt: number; +}; +/** The one branch a job may write, on the repository the service resolved for it. */ +export interface Branch { + repository: GitHubRepository; + /** The branch's commit, or null when it does not exist yet. */ + head(): Promise; + /** Create a commit of `changes` on `parent` without moving the branch. */ + commit(parent: string, changes: Change[], message: string): Promise; + /** Move the branch from `from` (null creates it) to `to`; false when it moved first. */ + move(from: string | null, to: string): Promise; +} export type ContributionState = "enabled" | "paused"; export type Enrollment = GitHubRepository & { state: ContributionState }; /** A running job attempt whose project was disconnected, which its runner must stop. */ @@ -75,10 +114,14 @@ function sha(value: string) { if (!/^[a-f0-9]{40}$/.test(value)) throw new GitHubError("An exact commit SHA is required"); return value; } -function branch(value: string) { - if (!/^quest\/[a-zA-Z0-9_/-]+$/.test(value) || value.includes("//") || value.endsWith("/")) - throw new GitHubError("A quest contribution branch is required"); - return value; +/** Whether Git accepts `name` as a branch, per `git check-ref-format`. */ +export function refName(name: string) { + return !/[\x00-\x20\x7f~^:?*[\\]|\.\.|@\{|\/\/|(^|\/)\.|\.lock($|\/)|[/.]$|^\/|^@$/.test(name); +} +function pullNumber(target: string) { + const match = target.match(/^pulls\/(\d{1,15})$/); + if (!match) throw new GitHubError("Invalid pull request target"); + return number(Number(match[1])); } /** Server-only integration. Call with an authenticated local session's user ID @@ -634,69 +677,259 @@ export class GitHubService { ); } - async createPullRequest( + /** + * Hand `action` the one branch a job may write: its branch on the service + * fork, or an iterated pull request's head when the bot can update it. + * Rechecks the requester's current authority and the project's enrollment. + */ + contribute( upstream: GitHubRepository, userId: string, - fork: GitHubRepository, - input: { branch: string; base: string; title: string; body: string }, + job: Contribution, + action: (branch: Branch) => Promise, ) { - this.#forkNamespace(fork); - if (!input.title.trim() || !input.base) throw new GitHubError("Approved pull request required"); - return this.#authorized(upstream, userId, { pull_requests: "write" }, async (token) => { - await this.#withRepository(fork, {}, async (_token, current) => { - if (!current.fork || current.parent?.id !== upstream.id) - throw new GitHubError("Contribution repository must fork this upstream"); - }); - return this.#api(token, `${repositoryPath(upstream)}/pulls`, "POST", { - head: `${fork.owner}:${branch(input.branch)}`, - head_repo: fork.name, - base: input.base, - title: input.title, - body: input.body, - draft: true, - // The upstream-scoped token has no write access to the fork's head - // branch, so it cannot grant maintainers edits there. Installation - // tokens cannot span both owners; the fork token only pushes. - maintainer_can_modify: false, + const iterate = job.workflow === "iterate"; + return this.#authorized( + upstream, + userId, + iterate ? { pull_requests: "read" } : {}, + async (token, current, installation) => { + this.#public(current); + if ((await this.#state(upstream, installation)) === null) + throw new GitHubError("Project is not enrolled for this installation"); + if (!iterate) + return this.#fork(upstream, upstream.name, { contents: "write" }, (forkToken, fork) => + action(this.#branch(forkToken, fork, job.branch)), + ); + const pull = await this.#api( + token, + `${repositoryPath(upstream)}/pulls/${pullNumber(job.target)}`, + ); + if (pull.state !== "open" || pull.head.ref !== job.branch) + throw new GitHubError("The pull request was closed or its head branch changed"); + const head = pull.head.repo; + if (head?.id === upstream.id) + return this.#withRepository(upstream, { contents: "write" }, async (writeToken, latest) => + action(this.#branch(writeToken, latest, job.branch)), + ); + if (head && this.#forkOwner(head.owner.login)) + return this.#fork(upstream, head.name, { contents: "write" }, (forkToken, fork) => { + if (fork.id !== head.id) throw new GitHubError("Repository identity or availability changed"); + return action(this.#branch(forkToken, fork, job.branch)); + }); + throw new PublicationBlocked( + "The pull request's head is in a contributor's fork, which the bot cannot update. Ask its author to apply the changes, or close it and start new work.", + ); + }, + ); + } + + #forkOwner(owner: string) { + return !!this.#env.GITHUB_FORK_OWNER && owner.toLowerCase() === this.#env.GITHUB_FORK_OWNER.toLowerCase(); + } + + /** The operator-provisioned fork of `upstream` in the service namespace, verified by parent ID. */ + async #fork( + upstream: GitHubRepository, + repository: string, + permissions: Permissions, + action: (token: string, fork: Repository) => Promise, + ) { + const owner = this.#env.GITHUB_FORK_OWNER; + if (!owner || !name(owner)) throw new GitHubError("Configured service fork namespace required"); + const missing = new GitHubError(`Provision the service fork ${owner}/${repository} of this upstream first`); + // Only a missing installation or repository means the fork is unprovisioned. + let found = false; + try { + return await this.#withInstallation({ owner, name: repository }, permissions, async (token) => { + const fork = await this.#api(token, `/repos/${owner}/${repository}`); + if ( + !fork.fork || + fork.parent?.id !== upstream.id || + !this.#forkOwner(fork.owner.login) || + fork.archived || + fork.disabled + ) + throw missing; + found = true; + return action(token, fork); }); - }); + } catch (error) { + throw !found && error instanceof GitHubError && error.status === 404 ? missing : error; + } } - #forkNamespace(fork: GitHubRepository) { - if (!this.#env.GITHUB_FORK_OWNER || fork.owner.toLowerCase() !== this.#env.GITHUB_FORK_OWNER.toLowerCase()) - throw new GitHubError("Configured service fork namespace required"); + #branch(token: string, repo: Repository, branch: string): Branch { + if (!refName(branch) || branch === repo.default_branch) + throw new PublicationBlocked("Jobs never write a repository's default branch"); + const path = `/repos/${repo.owner.login}/${repo.name}`; + const ref = branch.split("/").map(encodeURIComponent).join("/"); + const text = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); + return { + repository: { owner: repo.owner.login, name: repo.name, id: repo.id }, + head: async () => { + try { + const found = await this.#api<{ object: { sha: string; type: string } }>( + token, + `${path}/git/ref/heads/${ref}`, + ); + if (found.object.type !== "commit") throw new GitHubError("Branch must point at a commit"); + return sha(found.object.sha); + } catch (error) { + if (error instanceof GitHubError && error.status === 404) return null; + throw error; + } + }, + commit: async (parent, changes, message) => { + const base = await this.#api<{ tree: { sha: string } }>(token, `${path}/git/commits/${sha(parent)}`); + const tree = []; + for (const change of changes) { + const mode = change.mode; + if (change.content === null) { + tree.push({ path: change.path, mode, type: "blob", sha: null }); + continue; + } + // Text goes inline in the tree request; other bytes need their own blob. + let content: string | null = null; + try { + content = text.decode(change.content); + } catch {} + if (content !== null && !content.includes("\0")) { + tree.push({ path: change.path, mode, type: "blob", content }); + continue; + } + const blob = await this.#api<{ sha: string }>(token, `${path}/git/blobs`, "POST", { + content: Buffer.from(change.content).toString("base64"), + encoding: "base64", + }); + tree.push({ path: change.path, mode, type: "blob", sha: sha(blob.sha) }); + } + const created = await this.#api<{ sha: string }>(token, `${path}/git/trees`, "POST", { + base_tree: sha(base.tree.sha), + tree, + }); + // No author or committer: GitHub attributes and signs the commit as the App. + const commit = await this.#api<{ sha: string }>(token, `${path}/git/commits`, "POST", { + message, + tree: sha(created.sha), + parents: [parent], + }); + return sha(commit.sha); + }, + move: async (from, to) => { + try { + if (from === null) + await this.#api(token, `${path}/git/refs`, "POST", { + ref: `refs/heads/${branch}`, + sha: sha(to), + }); + // Fast-forward only, so an update built on a stale head cannot overwrite newer work. + else + await this.#api(token, `${path}/git/refs/heads/${ref}`, "PATCH", { + sha: sha(to), + force: false, + }); + return true; + } catch (error) { + if (error instanceof GitHubError && (error.status === 409 || error.status === 422)) return false; + throw error; + } + }, + }; } - publishQuest( + /** + * Open the job's pull request from its service fork branch, or update the + * one it already opened. A closed pull request is not replaced. + */ + deliver( upstream: GitHubRepository, userId: string, - fork: GitHubRepository, - input: { branch: string; path: string; content: string; message: string; previousBlob?: string }, + job: Contribution, + input: { title: string; body: string; draft: boolean }, ) { - this.#forkNamespace(fork); - if (!/^quest\/(?:[a-zA-Z0-9_-]+\/)*[a-zA-Z0-9_-]+\.md$/.test(input.path) || !input.message.trim()) - throw new GitHubError("Approved quest file required"); - return this.#authorized(upstream, userId, {}, async () => - this.#withRepository(fork, { contents: "write" }, async (token, current) => { - if (!current.fork || current.parent?.id !== upstream.id) - throw new GitHubError("Contribution repository must fork this upstream"); - return this.#api(token, `${repositoryPath(fork)}/contents/${input.path}`, "PUT", { - branch: branch(input.branch), - path: input.path, - content: Buffer.from(input.content).toString("base64"), - message: input.message, - ...(input.previousBlob ? { sha: sha(input.previousBlob) } : {}), + if (job.workflow === "iterate") + throw new GitHubError("An iteration updates its pull request's head and opens no pull request"); + return this.#authorized(upstream, userId, { pull_requests: "write" }, async (token, current, installation) => { + this.#public(current); + if ((await this.#state(upstream, installation)) !== "enabled") + throw new GitHubError("Contributions are paused"); + const fork = await this.#fork(upstream, upstream.name, {}, async (_token, fork) => fork); + const pulls = `${repositoryPath(upstream)}/pulls`; + const label = `${fork.owner.login}:${job.branch}`; + // An unrecorded earlier delivery may have opened one, which the maintainer may since have closed. + // GitHub reports whole seconds, so compare against the second the job was created in. + const since = Math.floor(job.createdAt / 1000) * 1000; + const opened = async () => + ( + await this.#api( + token, + `${pulls}?state=all&per_page=100&head=${encodeURIComponent(label)}`, + ) + ).find((pull) => Date.parse(pull.created_at) >= since) ?? null; + let pull = job.pull ? await this.#api(token, `${pulls}/${number(job.pull)}`) : await opened(); + if (pull && (pull.head.repo?.id !== fork.id || pull.head.ref !== job.branch)) + throw new GitHubError("The job's pull request no longer comes from its branch"); + if (pull && pull.state !== "open") + throw new GitHubError("The job's pull request was closed; reopen it on GitHub to deliver again"); + if (!pull) { + try { + pull = await this.#api(token, pulls, "POST", { + head: label, + // GitHub requires the head repository for cross-organization pull requests. + head_repo: fork.name, + base: current.default_branch, + title: input.title, + body: input.body, + draft: input.draft, + // Installation tokens cannot span both owners, so the upstream + // token cannot grant maintainers edits on the fork branch. + maintainer_can_modify: false, + }); + } catch (error) { + // A concurrent delivery may have opened it first. + if (!(error instanceof GitHubError) || error.status !== 422) throw error; + pull = await opened(); + if (pull?.state !== "open") throw error; + } + } + if (pull.title !== input.title || pull.body !== input.body) + pull = await this.#api(token, `${pulls}/${number(pull.number)}`, "PATCH", { + title: input.title, + body: input.body, }); - }), - ); + // Finished work leaves draft; a maintainer's own choice to mark it ready is never reverted. + if (pull.draft && !input.draft) { + const result = await this.#api<{ errors?: unknown[] }>(token, "/graphql", "POST", { + query: "mutation($id: ID!) { markPullRequestReadyForReview(input: {pullRequestId: $id}) { clientMutationId } }", + variables: { id: pull.node_id }, + }); + if (result.errors?.length) throw new GitHubError("Could not mark the pull request ready for review"); + pull = { ...pull, draft: false }; + } + return { number: number(pull.number), url: pull.html_url, draft: pull.draft }; + }); } + /** + * Merge exactly `expectedHead` with the first method the repository + * allows: squash, then a merge commit, then rebase. GitHub enforces branch + * rules, reviews, and checks. + */ mergePullRequest(repo: GitHubRepository, userId: string, pull: number, expectedHead: string) { - return this.#authorized(repo, userId, { contents: "write", pull_requests: "write" }, (token) => - this.#api(token, `${repositoryPath(repo)}/pulls/${number(pull)}/merge`, "PUT", { + return this.#authorized(repo, userId, { contents: "write", pull_requests: "write" }, (token, current) => { + const method = current.allow_squash_merge + ? "squash" + : current.allow_merge_commit + ? "merge" + : current.allow_rebase_merge + ? "rebase" + : null; + if (!method) throw new GitHubError("The repository allows no merge method"); + return this.#api(token, `${repositoryPath(repo)}/pulls/${number(pull)}/merge`, "PUT", { sha: sha(expectedHead), - merge_method: "merge", - }), - ); + merge_method: method, + }); + }); } } diff --git a/cloudflare/src/jobs/publish.ts b/cloudflare/src/jobs/publish.ts new file mode 100644 index 00000000..83aebc38 --- /dev/null +++ b/cloudflare/src/jobs/publish.ts @@ -0,0 +1,201 @@ +// Publishing a job's work as the GitHub App. A runner sends file changes as +// data; the service checks them against the job's workflow, commits them to +// the job's one branch, and opens or updates its pull request. Nothing a runner +// sends is executed, and runners never receive GitHub credentials. +import { isQuest } from "quest/core"; +import { type Branch, type Change, GitHubService, PublicationBlocked, type GitHubRepository } from "../github/service"; +import { id, text } from "./protocol"; +import * as store from "./store"; +import { JobError, type Runner, type Workflow } from "./store"; + +/** Bounds one request's GitHub writes: inline text costs no extra request, other files one each. */ +export const LIMITS = { changes: 500, bytes: 16 * 1024 * 1024, request: 24 * 1024 * 1024 }; + +export type Publication = { + job: string; + attempt: number; + /** The commit the changes build on: the job's branch head, or null before the branch exists. */ + head: string | null; + /** The agent session to resume if the publication needs a decision. */ + session: string | null; + message: string | null; + changes: Change[]; + pull: { title: string; body: string; draft: boolean } | null; +}; + +/** Whether `path` names a file inside a work tree, never Git's own directory. */ +export function validPath(path: string) { + return ( + path.length <= 1024 && + !/[\x00-\x1f\x7f\\]/.test(path) && + path.split("/").every((part) => part !== "" && part !== "." && part !== ".." && part.toLowerCase() !== ".git") + ); +} + +/** + * Why `workflow` may not publish `path`, or null when it may. A refusal + * becomes a maintainer decision rather than silently dropping the change. + */ +export function refusal(workflow: Workflow, path: string): string | null { + if (path === ".github/workflows" || path.startsWith(".github/workflows/")) + return "Changing GitHub Actions workflows needs the workflows permission, which the App does not request"; + if (workflow === "bootstrap" && path !== "flake.nix" && path !== "flake.lock") + return "A devShell bootstrap publishes only flake.nix and flake.lock"; + if (workflow === "plan" && !(path.startsWith("quest/") && isQuest(path))) + return "Planning publishes only quest files"; + return null; +} + +function base64(value: string) { + if (value.length % 4 !== 0 || !/^[A-Za-z0-9+/]*={0,2}$/.test(value)) throw new JobError("Invalid base64 content"); + return new Uint8Array(Buffer.from(value, "base64")); +} + +/** Parse a runner's publication request. Every field is untrusted input. */ +export function parsePublication(body: Record): Publication { + const job = id(body.job); + const attempt = body.attempt; + if (!job || typeof attempt !== "number" || !Number.isSafeInteger(attempt) || attempt < 1) + throw new JobError("Job and attempt required"); + if (body.head !== null && !(typeof body.head === "string" && /^[a-f0-9]{40}$/.test(body.head))) + throw new JobError("The commit the changes build on is required, or null for a new branch"); + const session = body.session === undefined ? null : text(body.session, 200); + if (body.session !== undefined && !session) throw new JobError("Invalid session"); + const list = body.changes ?? []; + if (!Array.isArray(list) || list.length > LIMITS.changes) + throw new JobError(`At most ${LIMITS.changes} changes per publication`); + let bytes = 0; + const changes = list.map((item: Record | null): Change => { + const path = item?.path; + if (typeof path !== "string" || !validPath(path)) throw new JobError("Invalid change path"); + const { executable, symlink } = item!; + if (executable !== undefined && typeof executable !== "boolean") throw new JobError("Invalid executable flag"); + if (symlink !== undefined && typeof symlink !== "boolean") throw new JobError("Invalid symlink flag"); + if (executable && symlink) throw new JobError("A symlink cannot be executable"); + if (item!.content !== null && typeof item!.content !== "string") + throw new JobError("Change content must be base64, or null to delete"); + const content = item!.content === null ? null : base64(item!.content as string); + bytes += content?.length ?? 0; + // A symlink's content is its target. Submodules have no representation here. + return { path, content, mode: symlink ? "120000" : executable ? "100755" : "100644" }; + }); + if (bytes > LIMITS.bytes) throw new JobError("Publication too large"); + if (new Set(changes.map((change) => change.path)).size !== changes.length) + throw new JobError("Duplicate change path"); + const message = changes.length ? text(body.message, 64 * 1024) : null; + if (changes.length && !message) throw new JobError("Commit message required"); + let pull: Publication["pull"] = null; + if (body.pull !== undefined) { + const input = body.pull as Record | null; + const title = text(input?.title, 256); + const description = input?.body; + if ( + !title || + typeof description !== "string" || + description.length > 60_000 || + (input!.draft !== undefined && typeof input!.draft !== "boolean") + ) + throw new JobError("Pull request title and body required"); + pull = { title, body: description, draft: input!.draft !== false }; + } + if (!changes.length && !pull) throw new JobError("Nothing to publish"); + return { job, attempt, head: body.head as string | null, session, message, changes, pull }; +} + +type Decision = { decision: string; cancel: boolean }; +type Published = { head: string | null; pull?: { number: number; url: string; draft: boolean } }; + +/** Pause the attempt on a decision the maintainer must make before this can publish. */ +async function decide(db: D1Database, runner: Runner, input: Publication, prompt: unknown): Promise { + const { question, cancel } = await store.ask( + db, + runner, + input.job, + input.attempt, + { + kind: "publication", + summary: "Publishing this work needs a maintainer's decision", + session: input.session, + prompt, + }, + Date.now(), + ); + return { decision: question, cancel }; +} + +/** + * The job's branch head, unless the branch is not where the job left it. A + * commit whose outcome went unrecorded is adopted first, so an interrupted + * publication is never mistaken for someone else's write. + */ +async function verify(db: D1Database, job: store.Publishing, input: Publication, branch: Branch) { + const current = await branch.head(); + let head = job.head; + if (current !== head && job.pending && current === job.pending) { + await store.adopt(db, job.id, head, job.pending, Date.now()); + head = current; + } + if (current !== head) + throw new PublicationBlocked( + head === null + ? `${branch.repository.owner}/${branch.repository.name} already has the branch ${job.branch} from earlier work. Delete it on GitHub to start over.` + : `The branch ${job.branch} changed outside this job.`, + ); + if (input.head !== head) throw new JobError(`The job's branch is at ${head ?? "no commit yet"}; continue from it`); + return head; +} + +/** Commit the changes on the verified `head` of the job's branch. */ +async function commit( + db: D1Database, + runner: Runner, + job: store.Publishing, + input: Publication, + branch: Branch, + head: string | null, +): Promise { + const next = await branch.commit(head ?? job.revision, input.changes, input.message!); + // Check the fence again immediately before the write that makes the commit visible. + await store.claim(db, runner, job.id, job.attempt, head, next, Date.now()); + // A failure here leaves the claim pending; the next publication finds out whether it landed. + const landed = await branch.move(head, next); + await store.settle(db, job.id, next, landed, Date.now()); + if (!landed) throw new JobError("The job's branch moved; continue from its new head"); + return next; +} + +/** Publish a running attempt's changes and pull request, or pause it on a decision. */ +export async function publish(env: Env, runner: Runner, input: Publication): Promise { + const db = env.DB; + const job = await store.publishing(db, runner, input.job, input.attempt); + if (input.pull && job.workflow === "iterate") + throw new JobError("An iteration updates its pull request's head and opens no pull request"); + const refused = input.changes.flatMap((change) => { + const reason = refusal(job.workflow, change.path); + return reason ? [{ path: change.path, reason }] : []; + }); + if (refused.length) return decide(db, runner, input, { type: "publication", refused }); + const github = new GitHubService(env); + const upstream: GitHubRepository = { owner: job.owner, name: job.name, id: job.repository }; + let head: string | null; + try { + // A pull request opens from the branch, so a delivery alone still checks where it is. + head = await github.contribute(upstream, job.actor, job, async (branch) => { + const head = await verify(db, job, input, branch); + return input.changes.length ? commit(db, runner, job, input, branch, head) : head; + }); + } catch (error) { + if (error instanceof PublicationBlocked) + return decide(db, runner, input, { type: "publication", blocked: error.message }); + throw error; + } + if (!input.pull) return { head }; + if (!head) throw new JobError("Publish the job's changes before opening its pull request"); + // The attempt may have ended, or been asked to stop, while the branch was written. + const current = await store.publishing(db, runner, input.job, input.attempt); + // Planning PRs always await maintainer review; so does work that was asked to stop. + const draft = input.pull.draft || current.workflow === "plan" || current.cancel === 1; + const pull = await github.deliver(upstream, current.actor, current, { ...input.pull, draft }); + await store.delivered(db, job.id, pull.number); + return { head, pull }; +} diff --git a/cloudflare/src/jobs/routes.ts b/cloudflare/src/jobs/routes.ts index 6b8a2635..dacdd3c3 100644 --- a/cloudflare/src/jobs/routes.ts +++ b/cloudflare/src/jobs/routes.ts @@ -1,10 +1,11 @@ import { type Context, Hono } from "hono"; import { createAuth } from "../auth"; -import { GitHubError, GitHubService, type Stopped } from "../github/service"; +import { GitHubError, GitHubService, refName, type Stopped } from "../github/service"; import type { ProjectState, Pull } from "../github/state"; import { RUNNER_HEADER } from "./hub"; import * as output from "./output"; import { data, id, text } from "./protocol"; +import { LIMITS, parsePublication, publish } from "./publish"; import * as store from "./store"; import { JobError } from "./store"; @@ -42,9 +43,12 @@ export async function stop(env: Env, attempts: Stopped[]) { // A runner authenticates with the token its owner registered it with, then // holds one live connection to its owner's hub. Runner requests carry no // cookies, so they need no origin check. -jobs.get("/api/runner/connect", async (c) => { +async function machine(c: Context) { const token = c.req.header("Authorization")?.match(/^Bearer ([A-Za-z0-9_-]{43})$/)?.[1]; - const runner = token && (await store.authenticate(c.env.DB, token)); + return (token && (await store.authenticate(c.env.DB, token))) || null; +} +jobs.get("/api/runner/connect", async (c) => { + const runner = await machine(c); if (!runner) return c.json({ error: "Runner token required" }, 401); if (c.req.header("Upgrade")?.toLowerCase() !== "websocket") return c.json({ error: "WebSocket required" }, 426); const headers = new Headers(c.req.raw.headers); @@ -59,6 +63,20 @@ jobs.get("/api/runner/connect", async (c) => { }); }); +// A running attempt publishes its work over HTTPS rather than its connection, +// so slow GitHub writes never hold up the hub's other runner messages. +jobs.post("/api/runner/publish", async (c) => { + const runner = await machine(c); + if (!runner) return c.json({ error: "Runner token required" }, 401); + const length = Number(c.req.header("Content-Length")); + if (!Number.isSafeInteger(length) || length > LIMITS.request) + return c.json({ error: `A Content-Length of at most ${LIMITS.request} bytes is required` }, 413); + const result = await publish(c.env, runner, parsePublication(await json(c))); + if (!("decision" in result)) return c.json(result); + await hub(c.env, runner.owner).dispatch(runner.owner); + return c.json(result, 202); +}); + // A signed-in user registers their own machines and sees their presence. async function signedIn(c: Context) { const session = await createAuth(c.env).api.getSession({ headers: c.req.raw.headers }); @@ -114,11 +132,6 @@ jobs.get(base, async (c) => { type Plan = Omit; -/** Whether Git accepts `name` as a branch, per `git check-ref-format`. */ -export function refName(name: string) { - return !/[\x00-\x20\x7f~^:?*[\\]|\.\.|@\{|\/\/|(^|\/)\.|\.lock($|\/)|[/.]$|^\/|^@$/.test(name); -} - /** Resolve a requested workflow and target against current observations. */ function resolve(workflow: unknown, target: unknown, state: ProjectState): Plan { const commit = state.project.commit; diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index f76be3dc..a8f9da9f 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -46,13 +46,21 @@ export type Job = { cancel: number; summary: string | null; result: string | null; + /** The commit the job's branch holds; null until its first publication. */ + head: string | null; + /** A commit whose branch update may be in flight, and since when. */ + pending: string | null; + pendingAt: number | null; + /** The pull request this job delivered. */ + pull: number | null; createdAt: number; updatedAt: number; }; +export type QuestionKind = "agent" | "account" | "publication"; export type Question = { id: string; attempt: number; - kind: "agent" | "account"; + kind: QuestionKind; prompt: unknown; answer: unknown; answeredBy: string | null; @@ -359,7 +367,7 @@ export async function ask( runner: Runner, job: string, attempt: number, - input: { kind?: "agent" | "account"; summary: string; session: string | null; prompt: unknown }, + input: { kind?: QuestionKind; summary: string; session: string | null; prompt: unknown }, now: number, ) { const id = crypto.randomUUID(); @@ -463,6 +471,74 @@ export async function finish( if (!landed) throw new JobError("Attempt is no longer current"); } +/** How long an unrecorded branch update blocks another publication of the same job. */ +export const PENDING = 2 * 60_000; + +export type Publishing = Job & { owner: string; name: string; project: "enabled" | "paused" }; + +/** The job a publication is for, only while the publishing attempt is current. */ +export async function publishing(db: D1Database, runner: Runner, job: string, attempt: number) { + const row = await db + .prepare( + "SELECT j.*,p.owner,p.name,p.state AS project FROM jobs j JOIN github_projects p ON p.id=j.repository WHERE j.id=? AND j.attempt=? AND j.runner=? AND j.state='running'", + ) + .bind(job, attempt, runner.id) + .first(); + if (!row) throw new JobError("Attempt is no longer current"); + return row; +} + +/** Record a pending commit that the branch shows landed after all. */ +export async function adopt(db: D1Database, job: string, head: string | null, pending: string, now: number) { + await db + .prepare( + "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND head IS ? AND pending=?", + ) + .bind(now, job, head, pending) + .run(); +} + +/** + * Reserve the branch update to `commit`, immediately before making it: only + * while the attempt is current, the branch is where the job last saw it, and + * no other update of it may still be in flight. + */ +export async function claim( + db: D1Database, + runner: Runner, + job: string, + attempt: number, + head: string | null, + commit: string, + now: number, +) { + const result = await db + .prepare( + "UPDATE jobs SET pending=?,pendingAt=?,updatedAt=? WHERE id=? AND attempt=? AND runner=? AND state='running' AND head IS ? AND (pending IS NULL OR pendingAt<=?)", + ) + .bind(commit, now, now, job, attempt, runner.id, head, now - PENDING) + .run(); + if (result.meta.changes !== 1) + throw new JobError("Attempt is no longer current, or another publication of this job is in flight"); +} + +/** Record whether the reserved branch update landed. The branch's head follows it either way. */ +export async function settle(db: D1Database, job: string, commit: string, landed: boolean, now: number) { + await db + .prepare( + landed + ? "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?" + : "UPDATE jobs SET pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?", + ) + .bind(now, job, commit) + .run(); +} + +/** Remember the pull request a job delivered, so later deliveries update it. */ +export async function delivered(db: D1Database, job: string, pull: number) { + await db.prepare("UPDATE jobs SET pull=? WHERE id=?").bind(pull, job).run(); +} + export type NewJob = Pick & { trusted: boolean; pinRunner?: string | null; @@ -485,8 +561,8 @@ export async function createJob(db: D1Database, input: NewJob, now: number) { const [insert, active, enabled, pinned] = await db.batch([ db .prepare( - `INSERT OR IGNORE INTO jobs(id,repository,workflow,target,revision,branch,actor,trusted,pinRunner,pinAccount,state,createdAt,updatedAt) - SELECT ?,id,?,?,?,?,?,?,?,?,'queued',?,? FROM github_projects WHERE id=? AND state='enabled' AND ${pinnable}`, + `INSERT OR IGNORE INTO jobs(id,repository,workflow,target,revision,branch,head,actor,trusted,pinRunner,pinAccount,state,createdAt,updatedAt) + SELECT ?,id,?,?,?,?,?,?,?,?,?,'queued',?,? FROM github_projects WHERE id=? AND state='enabled' AND ${pinnable}`, ) .bind( id, @@ -494,6 +570,8 @@ export async function createJob(db: D1Database, input: NewJob, now: number) { input.target, input.revision, input.branch, + // An iteration continues the pull request's existing head branch. + input.workflow === "iterate" ? input.revision : null, input.actor, trusted, runner, @@ -560,7 +638,7 @@ export async function answer(db: D1Database, job: string, question: string, user "SELECT q.kind,j.actor,j.trusted FROM job_questions q JOIN jobs j ON j.id=q.job WHERE q.id=? AND q.job=? AND q.answer IS NULL AND q.attempt=j.attempt AND j.state='waiting'", ) .bind(question, job) - .first<{ kind: "agent" | "account"; actor: string; trusted: number }>(); + .first<{ kind: QuestionKind; actor: string; trusted: number }>(); if (!open) throw new JobError("Question is no longer open"); let pin: { runner: string; account: string } | null = null; if (open.kind === "account") { diff --git a/cloudflare/test/github.test.ts b/cloudflare/test/github.test.ts index 13ce96a4..d808334a 100644 --- a/cloudflare/test/github.test.ts +++ b/cloudflare/test/github.test.ts @@ -216,6 +216,7 @@ function github( missing?: boolean; permissions?: Record; parent?: number; + merges?: ("squash" | "merge" | "rebase")[]; cleanupFails?: boolean; failMutation?: boolean; } = {}, @@ -264,6 +265,9 @@ function github( disabled: false, fork: isFork, parent: { id: options.parent ?? 10 }, + allow_squash_merge: (options.merges ?? ["squash", "merge", "rebase"]).includes("squash"), + allow_merge_commit: (options.merges ?? ["squash", "merge", "rebase"]).includes("merge"), + allow_rebase_merge: (options.merges ?? ["squash", "merge", "rebase"]).includes("rebase"), }); } if (method !== "GET") @@ -334,55 +338,16 @@ describe("trusted App operations", () => { await expect(service.comment(repo, "app-test", 1, "Approved")).rejects.toThrow("issues permission"); expect(requests.length).toBe(1); }); - it("uses separate fork and upstream scopes and verifies the parent", async () => { - const input = { branch: "quest/m0/one", base: "main", title: "Approved", body: "Approved" }; - const { service, requests } = github(); - await service.createPullRequest(repo, "app-test", fork, input); - expect(requests.filter((r) => r.path.endsWith("/access_tokens")).map((r) => r.body)).toEqual([ - { repository_ids: [10], permissions: { metadata: "read", pull_requests: "write" } }, - { repository_ids: [20], permissions: { metadata: "read" } }, - ]); - expect(requests.find((r) => r.path.endsWith("/pulls"))?.body).toMatchObject({ - head: "quest-bot-forks:quest/m0/one", - head_repo: "project", - draft: true, - maintainer_can_modify: false, - }); - expect(requests.find((r) => r.path.endsWith("/pulls"))?.authorization).toBe("Bearer upstream-token"); - await expect(github({ parent: 999 }).service.createPullRequest(repo, "app-test", fork, input)).rejects.toThrow( - "fork this upstream", - ); - await expect( - github().service.createPullRequest(repo, "app-test", { ...fork, owner: "attacker" }, input), - ).rejects.toThrow("namespace"); - }); - it("restricts approved publication to quests in service fork branches", async () => { - const { service, requests } = github(); - await service.publishQuest(repo, "app-test", fork, { - branch: "quest/m0/one", - path: "quest/m0/one.md", - content: "# [S] One", - message: "Approved", - }); - expect(requests.filter((r) => r.path.endsWith("/access_tokens")).map((r) => r.body)).toEqual([ - { repository_ids: [10], permissions: { metadata: "read" } }, - { repository_ids: [20], permissions: { metadata: "read", contents: "write" } }, - ]); - expect(() => - service.publishQuest(repo, "app-test", fork, { - branch: "main", - path: ".github/workflows/run.yml", - content: "", - message: "Approved", - }), - ).toThrow("quest file"); - }); - it("pins explicit merges to the approved commit SHA", async () => { - const { service, requests } = github(); + it.each([ + [["squash", "merge", "rebase"], "squash"], + [["merge", "rebase"], "merge"], + [["rebase"], "rebase"], + ] as const)("pins explicit merges to the approved commit SHA with %j allowed", async (merges, method) => { + const { service, requests } = github({ merges: [...merges] }); await service.mergePullRequest(repo, "app-test", 1, "a".repeat(40)); expect(requests.find((r) => r.path.endsWith("/merge"))?.body).toEqual({ sha: "a".repeat(40), - merge_method: "merge", + merge_method: method, }); expect(requests.find((r) => r.path.endsWith("/access_tokens"))?.body.permissions).toEqual({ metadata: "read", @@ -390,6 +355,11 @@ describe("trusted App operations", () => { pull_requests: "write", }); }); + it("refuses to merge when the repository allows no merge method", async () => { + const { service, requests } = github({ merges: [] }); + await expect(service.mergePullRequest(repo, "app-test", 1, "a".repeat(40))).rejects.toThrow("no merge method"); + expect(requests.some((r) => r.path.endsWith("/merge"))).toBe(false); + }); it("reads provider data with read-only permissions", async () => { const { service, requests } = github(); await service.readQuest(repo, "app-test", "quest/one.md", "a".repeat(40)); diff --git a/cloudflare/test/jobs.test.ts b/cloudflare/test/jobs.test.ts index 75050f63..888a167d 100644 --- a/cloudflare/test/jobs.test.ts +++ b/cloudflare/test/jobs.test.ts @@ -6,7 +6,8 @@ import * as output from "../src/jobs/output"; import * as store from "../src/jobs/store"; import { GRACE } from "../src/jobs/store"; import { parse } from "../src/jobs/protocol"; -import { refName, resume } from "../src/jobs/routes"; +import { refName } from "../src/github/service"; +import { resume } from "../src/jobs/routes"; import { privateKey } from "./github-key"; import { identity, oauth, origin } from "./login-fixtures"; diff --git a/cloudflare/test/publish.test.ts b/cloudflare/test/publish.test.ts new file mode 100644 index 00000000..e42459c6 --- /dev/null +++ b/cloudflare/test/publish.test.ts @@ -0,0 +1,716 @@ +import { env } from "cloudflare:workers"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { app } from "../src/index"; +import { refusal, validPath } from "../src/jobs/publish"; +import * as store from "../src/jobs/store"; +import { privateKey } from "./github-key"; +import { origin } from "./login-fixtures"; + +const testEnv = { ...env, GITHUB_APP_PRIVATE_KEY: privateKey }; +const db = env.DB; +const revision = "a".repeat(40); +const prHead = "b".repeat(40); +const user = "publisher"; +const githubUser = 7171; + +type Request = { method: string; path: string; body: any; token: string | null }; +type Commit = { parents: string[]; tree: string; message: string }; + +/** A stateful stand-in for the parts of GitHub's API that publication uses. */ +function fakeGitHub() { + let next = 1; + const oid = () => (next++).toString(16).padStart(40, "c"); + const commits = new Map([ + [revision, { parents: [], tree: "e".repeat(40), message: "base" }], + [prHead, { parents: [revision], tree: "e".repeat(40), message: "pr" }], + ]); + const trees = new Map(); + const refs = new Map(); + const pulls: any[] = []; + const requests: Request[] = []; + const fake = { + requests, + refs, + commits, + trees, + pulls, + role: "maintain", + forkParent: 10, + /** The iterated pull request's head repository. */ + head: { id: 10, name: "project", owner: { login: "upstream" } } as any, + /** Apply the next ref update, then fail as if the response were lost. */ + loseMove: false, + /** GitHub's clock, for pull request creation times. */ + now: 0, + /** Runs while a branch is created, as if the job changed meanwhile. */ + onCreate: null as null | (() => Promise), + }; + const repos: Record = { + "upstream/project": { id: 10, fork: false, default_branch: "main" }, + "quest-bot-forks/project": { id: 20, fork: true, default_branch: "main" }, + }; + function ancestor(of: string, sha: string): boolean { + if (of === sha) return true; + return (commits.get(sha)?.parents ?? []).some((parent) => ancestor(of, parent)); + } + vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => { + const url = new URL(String(input)); + const path = url.pathname; + const method = init?.method ?? "GET"; + const body = init?.body ? JSON.parse(String(init.body)) : undefined; + const token = new Headers(init?.headers).get("Authorization"); + requests.push({ method, path, body, token }); + const json = (value: unknown, status = 200) => Response.json(value, { status }); + const repo = path.match(/^\/repos\/([^/]+\/[^/]+)/)?.[1]; + const rest = repo ? path.slice(`/repos/${repo}`.length) : path; + if (rest === "/installation") + return json({ + id: repo!.startsWith("quest-bot-forks/") ? 6 : 5, + app_id: 123, + suspended_at: null, + permissions: { metadata: "read", contents: "write", pull_requests: "write" }, + }); + if (path.endsWith("/access_tokens")) + return json({ token: path.includes("/6/") ? "fork-token" : "upstream-token" }); + if (path === "/installation/token") return new Response(null, { status: 204 }); + if (path === `/user/${githubUser}`) return json({ id: githubUser, login: "maintainer" }); + if (rest.endsWith("/permission")) + return json({ user: { id: githubUser }, permission: "write", role_name: fake.role }); + if (path === "/graphql") { + const pull = pulls.find((p) => p.node_id === body.variables.id); + if (!pull || !body.query.includes("markPullRequestReadyForReview")) return json({ errors: [{}] }); + pull.draft = false; + return json({ data: {} }); + } + if (!repo || !repos[repo]) throw new Error(`Unexpected request: ${method} ${path}`); + if (rest === "") { + const [owner, name] = repo.split("/"); + return json({ + ...repos[repo], + name, + owner: { login: owner }, + private: false, + visibility: "public", + archived: false, + disabled: false, + parent: repos[repo].fork ? { id: fake.forkParent } : undefined, + }); + } + const ref = rest.match(/^\/git\/refs?\/heads\/(.+)$/)?.[1]; + if (ref && method === "GET") { + const sha = refs.get(`${repo}:${decodeURIComponent(ref)}`); + return sha ? json({ object: { type: "commit", sha } }) : json({}, 404); + } + const commit = rest.match(/^\/git\/commits\/([a-f0-9]{40})$/)?.[1]; + if (commit) return commits.has(commit) ? json({ tree: { sha: commits.get(commit)!.tree } }) : json({}, 404); + if (rest === "/git/blobs") return json({ sha: oid() }); + if (rest === "/git/trees") { + const sha = oid(); + trees.set(sha, { base: body.base_tree, entries: body.tree }); + return json({ sha }); + } + if (rest === "/git/commits") { + const sha = oid(); + commits.set(sha, { parents: body.parents, tree: body.tree, message: body.message }); + return json({ sha }); + } + if (rest === "/git/refs" && method === "POST") { + const key = `${repo}:${body.ref.slice("refs/heads/".length)}`; + if (refs.has(key)) return json({ message: "Reference already exists" }, 422); + refs.set(key, body.sha); + await fake.onCreate?.(); + return json({ object: { sha: body.sha } }, 201); + } + if (ref && method === "PATCH") { + const key = `${repo}:${decodeURIComponent(ref)}`; + const current = refs.get(key); + if (!current || body.force || !ancestor(current, body.sha)) + return json({ message: "Update is not a fast forward" }, 422); + refs.set(key, body.sha); + if (fake.loseMove) { + fake.loseMove = false; + return json({}, 502); + } + return json({ object: { sha: body.sha } }); + } + if (repo === "upstream/project" && rest === "/pulls" && method === "GET") { + const head = url.searchParams.get("head"); + const state = url.searchParams.get("state") ?? "open"; + return json( + pulls + .filter( + (pull) => + (state === "all" || pull.state === state) && + `${pull.head.repo.owner.login}:${pull.head.ref}` === head, + ) + .reverse(), + ); + } + if (repo === "upstream/project" && rest === "/pulls" && method === "POST") { + const [owner, branch] = body.head.split(":"); + if (pulls.some((pull) => pull.state === "open" && pull.head.ref === branch)) + return json({ message: "A pull request already exists" }, 422); + const pull = { + number: 100 + pulls.length, + node_id: `PR_${pulls.length}`, + html_url: `https://github.com/upstream/project/pull/${100 + pulls.length}`, + state: "open", + created_at: new Date(fake.now).toISOString(), + draft: body.draft, + title: body.title, + body: body.body, + base: body.base, + maintainer_can_modify: body.maintainer_can_modify, + head: { + ref: branch, + sha: refs.get(`quest-bot-forks/project:${branch}`), + repo: { id: 20, name: "project", owner: { login: owner } }, + }, + }; + pulls.push(pull); + return json(pull, 201); + } + const number = Number(rest.match(/^\/pulls\/(\d+)$/)?.[1]); + if (repo === "upstream/project" && number === 2) + return json({ number: 2, state: "open", head: { ref: "contributor/fix", sha: prHead, repo: fake.head } }); + const pull = pulls.find((p) => p.number === number); + if (pull && method === "GET") return json(pull); + if (pull && method === "PATCH") return json(Object.assign(pull, body)); + throw new Error(`Unexpected request: ${method} ${path}`); + }); + return fake; +} + +let github: ReturnType; +let machine: store.Runner & { token: string }; + +/** Create and claim a job on the test runner, returning its ID. */ +async function running(extra: Partial = {}) { + const { id } = await store.createJob( + db, + { + repository: 10, + actor: user, + workflow: "start", + target: "quest/first.md", + revision, + branch: "quest/first", + trusted: true, + ...extra, + }, + 0, + ); + await store.dispatch(db, user, new Map([[machine.id, 1]]), 0); + return id; +} +function publish(body: Record, token = machine.token) { + const text = JSON.stringify(body); + return app.request( + `${origin}/api/runner/publish`, + { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + "Content-Length": String(new TextEncoder().encode(text).length), + }, + body: text, + }, + testEnv, + ); +} +const file = (path: string, content: string, executable?: boolean) => ({ + path, + content: Buffer.from(content).toString("base64"), + ...(executable === undefined ? {} : { executable }), +}); +const gitWrites = () => github.requests.filter((r) => r.path.includes("/git/") && r.method !== "GET"); +async function job(id: string) { + return (await store.readJob(db, id))!; +} + +beforeEach(async () => { + await db.batch([ + db.prepare("DELETE FROM jobs"), + db.prepare("DELETE FROM runners"), + db.prepare("DELETE FROM github_projects"), + db.prepare("DELETE FROM github_installations"), + db + .prepare("INSERT OR IGNORE INTO user(id,name,email,emailVerified,createdAt,updatedAt) VALUES(?,?,?,1,0,0)") + .bind(user, user, `${user}@example.test`), + db + .prepare( + "INSERT OR IGNORE INTO account(id,accountId,providerId,userId,createdAt,updatedAt) VALUES(?,?,'github',?,0,0)", + ) + .bind(`${user}-account`, String(githubUser), user), + db + .prepare( + "INSERT INTO github_projects(id,owner,name,installation,state,enrolledBy,updatedAt) VALUES(10,'upstream','project',5,'enabled',?,0)", + ) + .bind(user), + ]); + const { id, token } = await store.createRunner(db, user, "machine", "linux", true, 0); + machine = { id, token, owner: user, name: "machine", platform: "linux", vm: 1 }; + await store.report( + db, + machine, + [{ id: "main", provider: "anthropic", kind: "subscription", label: "main", limited: false }], + [], + 0, + ); + github = fakeGitHub(); +}); +afterEach(() => vi.restoreAllMocks()); + +describe("publication policy", () => { + it("accepts work-tree paths only", () => { + for (const path of ["src/a.ts", "quest/m0/one.md", ".github/CODEOWNERS", "a/.gitignore"]) + expect(validPath(path)).toBe(true); + for (const path of [ + "", + "/abs", + "a//b", + "a/", + "../up", + "a/./b", + ".git/config", + "x/.GIT/hooks/pre-commit", + "a\\b", + "a\nb", + ]) + expect(validPath(path)).toBe(false); + }); + it("limits each workflow to the changes it may publish", () => { + expect(refusal("plan", "quest/m0/one.md")).toBeNull(); + expect(refusal("plan", "src/index.ts")).toContain("quest files"); + for (const path of ["quest/tool.sh", "quest/AGENTS.md", "quest/m0/CLAUDE.md"]) + expect(refusal("plan", path)).toContain("quest files"); + expect(refusal("bootstrap", "flake.nix")).toBeNull(); + expect(refusal("bootstrap", "flake.lock")).toBeNull(); + expect(refusal("bootstrap", "justfile")).toContain("flake"); + expect(refusal("start", "src/index.ts")).toBeNull(); + for (const workflow of ["plan", "start", "iterate", "bootstrap"] as const) + expect(refusal(workflow, ".github/workflows/ci.yml")).toContain("workflows permission"); + }); +}); + +describe("runner publication", () => { + it("commits to the job's service fork branch and opens one draft pull request as the App", async () => { + const id = await running(); + const first = await publish({ + job: id, + attempt: 1, + head: null, + message: "feat: first", + changes: [ + file("src/first.ts", "export {};\n"), + file("scripts/run.sh", "#!/bin/sh\n", true), + { path: "logo.bin", content: Buffer.from([0, 1, 2, 255]).toString("base64") }, + { ...file("latest", "scripts/run.sh"), symlink: true }, + { path: "quest/first.md", content: null }, + ], + }); + expect(first.status).toBe(200); + const { head } = (await first.json()) as { head: string }; + expect(github.refs.get("quest-bot-forks/project:quest/first")).toBe(head); + expect(github.commits.get(head)).toMatchObject({ parents: [revision], message: "feat: first" }); + // No author or committer: GitHub attributes the commit to the App. + const created = github.requests.find((r) => r.path.endsWith("/git/commits") && r.method === "POST")!; + expect(Object.keys(created.body).sort()).toEqual(["message", "parents", "tree"]); + const tree = github.trees.get(github.commits.get(head)!.tree)!; + expect(tree.base).toBe("e".repeat(40)); + expect(tree.entries).toEqual([ + { path: "src/first.ts", mode: "100644", type: "blob", content: "export {};\n" }, + { path: "scripts/run.sh", mode: "100755", type: "blob", content: "#!/bin/sh\n" }, + { path: "logo.bin", mode: "100644", type: "blob", sha: expect.stringMatching(/^[a-f0-9]{40}$/) }, + { path: "latest", mode: "120000", type: "blob", content: "scripts/run.sh" }, + { path: "quest/first.md", mode: "100644", type: "blob", sha: null }, + ]); + // Every Git write uses the fork-scoped token; the upstream token is read-only metadata. + expect( + gitWrites().every((r) => r.token === "Bearer fork-token" && r.path.startsWith("/repos/quest-bot-forks/")), + ).toBe(true); + const scopes = github.requests.filter((r) => r.path.endsWith("/access_tokens")).map((r) => r.body); + expect(scopes).toEqual([ + { repository_ids: [10], permissions: { metadata: "read" } }, + { repositories: ["project"], permissions: { metadata: "read", contents: "write" } }, + ]); + expect((await job(id)).head).toBe(head); + + // A stale base is refused without writing anything. + const before = gitWrites().length; + const stale = await publish({ job: id, attempt: 1, head: null, message: "again", changes: [file("a", "a")] }); + expect(stale.status).toBe(409); + expect(await stale.json()).toEqual({ error: `The job's branch is at ${head}; continue from it` }); + expect(gitWrites()).toHaveLength(before); + + const second = await publish({ + job: id, + attempt: 1, + head, + message: "feat: more", + changes: [file("src/second.ts", "export {};\n")], + pull: { title: "Start first", body: "Summary\n\n(written by Test Model)" }, + }); + expect(second.status).toBe(200); + const delivered = (await second.json()) as { head: string; pull: { number: number; draft: boolean } }; + expect(github.commits.get(delivered.head)!.parents).toEqual([head]); + expect(delivered.pull).toMatchObject({ number: 100, draft: true }); + expect(github.pulls).toEqual([ + expect.objectContaining({ + title: "Start first", + base: "main", + draft: true, + maintainer_can_modify: false, + head: expect.objectContaining({ ref: "quest/first", repo: expect.objectContaining({ id: 20 }) }), + }), + ]); + const create = github.requests.find((r) => r.path === "/repos/upstream/project/pulls" && r.method === "POST")!; + expect(create.token).toBe("Bearer upstream-token"); + expect(create.body).toMatchObject({ head: "quest-bot-forks:quest/first", head_repo: "project" }); + + // Delivering again updates the same pull request and marks finished work ready. + const ready = await publish({ + job: id, + attempt: 1, + head: delivered.head, + pull: { title: "Start first", body: "Done", draft: false }, + }); + expect(await ready.json()).toMatchObject({ pull: { number: 100, draft: false } }); + expect(github.pulls).toHaveLength(1); + expect(github.pulls[0].body).toBe("Done"); + expect(github.requests.filter((r) => r.path === "/graphql").map((r) => r.body.variables)).toEqual([ + { id: "PR_0" }, + ]); + expect((await job(id)).pull).toBe(100); + }); + + it("fences out an attempt that no longer owns the job", async () => { + const id = await running(); + await db.prepare("UPDATE jobs SET attempt=2 WHERE id=?").bind(id).run(); + const response = await publish({ job: id, attempt: 1, head: null, message: "late", changes: [file("a", "a")] }); + expect(response.status).toBe(409); + expect(await response.json()).toEqual({ error: "Attempt is no longer current" }); + expect(github.requests).toEqual([]); + expect( + ( + await publish( + { job: id, attempt: 2, head: null, message: "m", changes: [file("a", "a")] }, + "x".repeat(43), + ) + ).status, + ).toBe(401); + }); + + it("rechecks the requester's maintainer authority before writing", async () => { + const id = await running(); + github.role = "write"; + const response = await publish({ job: id, attempt: 1, head: null, message: "m", changes: [file("a", "a")] }); + expect(response.status).toBe(403); + expect(gitWrites()).toEqual([]); + }); + + it("pauses on a decision instead of dropping changes the workflow may not publish", async () => { + const id = await running({ workflow: "plan", target: "issues/1", branch: "plan/issue-1" }); + const response = await publish({ + job: id, + attempt: 1, + head: null, + session: "s1", + message: "docs: plan", + changes: [ + file("quest/one.md", "# [S] One\n"), + file("src/index.ts", "x"), + file(".github/workflows/ci.yml", "x"), + ], + }); + expect(response.status).toBe(202); + const { decision } = (await response.json()) as { decision: string }; + const paused = await job(id); + expect(paused).toMatchObject({ state: "waiting", session: "s1", head: null }); + expect(paused.questions).toEqual([ + expect.objectContaining({ + id: decision, + kind: "publication", + prompt: { + type: "publication", + refused: [ + { path: "src/index.ts", reason: "Planning publishes only quest files" }, + { path: ".github/workflows/ci.yml", reason: expect.stringContaining("workflows permission") }, + ], + }, + }), + ]); + expect(github.requests).toEqual([]); + // The answer requeues the job, which resumes its session. + await store.answer(db, id, decision, user, "Drop the source change", 1); + expect(await job(id)).toMatchObject({ state: "queued" }); + }); + + it("opens planning pull requests as drafts and bootstraps only flake files", async () => { + const plan = await running({ workflow: "plan", target: "issues/1", branch: "plan/issue-1" }); + const response = await publish({ + job: plan, + attempt: 1, + head: null, + message: "docs: plan", + changes: [file("quest/one.md", "# [S] One\n")], + pull: { title: "Plan", body: "Decisions", draft: false }, + }); + expect(await response.json()).toMatchObject({ pull: { draft: true } }); + expect(github.requests.some((r) => r.path === "/graphql")).toBe(false); + await db.prepare("UPDATE jobs SET state='completed' WHERE id=?").bind(plan).run(); + const bootstrap = await running({ workflow: "bootstrap", target: "devshell", branch: "quest/devshell" }); + const refused = await publish({ + job: bootstrap, + attempt: 1, + head: null, + message: "m", + changes: [file("justfile", "x")], + }); + expect(refused.status).toBe(202); + }); + + it("recovers a branch update whose outcome was lost, without overwriting or duplicating", async () => { + const id = await running(); + github.loseMove = false; + const first = (await ( + await publish({ job: id, attempt: 1, head: null, message: "one", changes: [file("a", "1")] }) + ).json()) as { head: string }; + github.loseMove = true; + const lost = await publish({ + job: id, + attempt: 1, + head: first.head, + message: "two", + changes: [file("a", "2")], + }); + expect(lost.status).toBe(403); + const landed = github.refs.get("quest-bot-forks/project:quest/first")!; + expect(landed).not.toBe(first.head); + expect(await job(id)).toMatchObject({ head: first.head, pending: landed }); + // The next publication finds the pending commit on the branch and adopts it. + const retry = await publish({ + job: id, + attempt: 1, + head: first.head, + message: "two", + changes: [file("a", "2")], + }); + expect(await retry.json()).toEqual({ error: `The job's branch is at ${landed}; continue from it` }); + expect(await job(id)).toMatchObject({ head: landed, pending: null }); + const next = await publish({ job: id, attempt: 1, head: landed, message: "three", changes: [file("a", "3")] }); + expect(next.status).toBe(200); + }); + + it("refuses a write when the branch moved outside the job, and never overwrites it", async () => { + const id = await running(); + const { head } = (await ( + await publish({ job: id, attempt: 1, head: null, message: "one", changes: [file("a", "1")] }) + ).json()) as { head: string }; + github.refs.set("quest-bot-forks/project:quest/first", prHead); + const response = await publish({ + job: id, + attempt: 1, + head, + session: "s", + message: "two", + changes: [file("a", "2")], + }); + expect(response.status).toBe(202); + expect((await job(id)).questions[0]).toMatchObject({ + kind: "publication", + prompt: { blocked: "The branch quest/first changed outside this job." }, + }); + expect(github.refs.get("quest-bot-forks/project:quest/first")).toBe(prHead); + }); + + it("asks before reusing a branch that earlier work left on the fork", async () => { + github.refs.set("quest-bot-forks/project:quest/first", prHead); + const id = await running(); + const response = await publish({ + job: id, + attempt: 1, + head: null, + session: "s", + message: "m", + changes: [file("a", "1")], + }); + expect(response.status).toBe(202); + expect((await job(id)).questions[0]!.prompt).toEqual({ + type: "publication", + blocked: expect.stringContaining("already has the branch quest/first"), + }); + }); + + it("reconciles an interrupted delivery with the pull request it already opened", async () => { + const id = await running(); + const { head } = (await ( + await publish({ + job: id, + attempt: 1, + head: null, + message: "one", + changes: [file("a", "1")], + pull: { title: "T", body: "B" }, + }) + ).json()) as { head: string }; + await db.prepare("UPDATE jobs SET pull=NULL WHERE id=?").bind(id).run(); + const again = await publish({ job: id, attempt: 1, head, pull: { title: "T", body: "B2" } }); + expect(await again.json()).toMatchObject({ pull: { number: 100 } }); + expect(github.pulls).toHaveLength(1); + // A pull request the maintainer closed is not replaced. + github.pulls[0].state = "closed"; + const closed = await publish({ job: id, attempt: 1, head, pull: { title: "T", body: "B3" } }); + expect(closed.status).toBe(403); + expect(await closed.json()).toEqual({ error: expect.stringContaining("was closed") }); + // Nor is one closed after a delivery whose outcome went unrecorded. + await db.prepare("UPDATE jobs SET pull=NULL WHERE id=?").bind(id).run(); + const unrecorded = await publish({ job: id, attempt: 1, head, pull: { title: "T", body: "B3" } }); + expect(await unrecorded.json()).toEqual({ error: expect.stringContaining("was closed") }); + expect(github.pulls).toHaveLength(1); + }); + + it("opens a new pull request when only earlier work's closed one shares the branch", async () => { + github.pulls.push({ + number: 99, + state: "closed", + created_at: new Date(0).toISOString(), + head: { + ref: "quest/first", + sha: prHead, + repo: { id: 20, name: "project", owner: { login: "quest-bot-forks" } }, + }, + }); + const id = await running(); + await db.prepare("UPDATE jobs SET createdAt=5000 WHERE id=?").bind(id).run(); + github.now = 6000; + const response = await publish({ + job: id, + attempt: 1, + head: null, + message: "m", + changes: [file("a", "1")], + pull: { title: "T", body: "B" }, + }); + expect(await response.json()).toMatchObject({ pull: { number: 101 } }); + }); + + it("rechecks the attempt and its cancellation after writing the branch, before delivering", async () => { + const stopped = await running(); + github.onCreate = () => db.prepare("UPDATE jobs SET cancel=1 WHERE id=?").bind(stopped).run(); + const changes = [file("a", "1")]; + const pull = { title: "T", body: "B", draft: false }; + const draft = await publish({ job: stopped, attempt: 1, head: null, message: "m", changes, pull }); + expect(await draft.json()).toMatchObject({ pull: { draft: true } }); + await db.prepare("UPDATE jobs SET state='completed' WHERE id=?").bind(stopped).run(); + const ended = await running({ target: "quest/second.md", branch: "quest/second" }); + github.onCreate = () => db.prepare("UPDATE jobs SET state='completed' WHERE id=?").bind(ended).run(); + const late = await publish({ job: ended, attempt: 1, head: null, message: "m", changes, pull }); + expect(await late.json()).toEqual({ error: "Attempt is no longer current" }); + expect(github.pulls).toHaveLength(1); + }); + + it("pauses a delivery alone when the branch moved outside the job", async () => { + const id = await running(); + const { head } = (await ( + await publish({ job: id, attempt: 1, head: null, message: "one", changes: [file("a", "1")] }) + ).json()) as { head: string }; + github.refs.set("quest-bot-forks/project:quest/first", prHead); + const response = await publish({ job: id, attempt: 1, head, session: "s", pull: { title: "T", body: "B" } }); + expect(response.status).toBe(202); + expect((await job(id)).questions[0]).toMatchObject({ + kind: "publication", + prompt: { blocked: "The branch quest/first changed outside this job." }, + }); + expect(github.pulls).toEqual([]); + }); + + it("keeps publishing a paused project's branch but opens no pull request", async () => { + const id = await running(); + await db.prepare("UPDATE github_projects SET state='paused' WHERE id=10").run(); + const response = await publish({ + job: id, + attempt: 1, + head: null, + message: "m", + changes: [file("a", "1")], + pull: { title: "T", body: "B" }, + }); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ error: "Contributions are paused" }); + expect(github.refs.has("quest-bot-forks/project:quest/first")).toBe(true); + expect(github.pulls).toEqual([]); + }); + + it("updates an iterated pull request's own head, and reports a contributor fork as blocked", async () => { + const id = await running({ + workflow: "iterate", + target: "pulls/2", + revision: prHead, + branch: "contributor/fix", + trusted: false, + }); + github.refs.set("upstream/project:contributor/fix", prHead); + expect((await job(id)).head).toBe(prHead); + const response = await publish({ + job: id, + attempt: 1, + head: prHead, + message: "fix: review", + changes: [file("a", "1")], + }); + expect(response.status).toBe(200); + const { head } = (await response.json()) as { head: string }; + expect(github.refs.get("upstream/project:contributor/fix")).toBe(head); + expect(gitWrites().every((r) => r.token === "Bearer upstream-token")).toBe(true); + expect(github.requests.filter((r) => r.path.endsWith("/access_tokens")).map((r) => r.body.permissions)).toEqual( + [ + { metadata: "read", pull_requests: "read" }, + { metadata: "read", contents: "write" }, + ], + ); + expect((await publish({ job: id, attempt: 1, head, pull: { title: "T", body: "B" } })).status).toBe(409); + + github.head = { id: 99, name: "project", owner: { login: "contributor" } }; + const blocked = await publish({ + job: id, + attempt: 1, + head, + session: "s", + message: "m", + changes: [file("a", "2")], + }); + expect(blocked.status).toBe(202); + expect((await job(id)).questions[0]!.prompt).toEqual({ + type: "publication", + blocked: expect.stringContaining("contributor's fork"), + }); + }); + + it("refuses a service fork that does not fork the upstream", async () => { + github.forkParent = 999; + const id = await running(); + const response = await publish({ job: id, attempt: 1, head: null, message: "m", changes: [file("a", "1")] }); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ + error: "Provision the service fork quest-bot-forks/project of this upstream first", + }); + expect(gitWrites()).toEqual([]); + }); + + it("validates the request before touching the job", async () => { + const id = await running(); + for (const body of [ + { job: id, attempt: 1, message: "m", changes: [file("a", "1")] }, + { job: id, attempt: 1, head: null, changes: [file("a", "1")] }, + { job: id, attempt: 1, head: null, message: "m", changes: [file("../a", "1")] }, + { job: id, attempt: 1, head: null, message: "m", changes: [file("a", "1"), file("a", "2")] }, + { job: id, attempt: 1, head: null, message: "m", changes: [{ path: "a", content: "not base64!" }] }, + { job: id, attempt: 1, head: null, message: "m", changes: [{ ...file("a", "b", true), symlink: true }] }, + { job: id, attempt: 1, head: null }, + ]) + expect((await publish(body)).status).toBe(409); + expect(github.requests).toEqual([]); + }); +}); diff --git a/quest/m0/kix/README.md b/quest/m0/kix/README.md index aaf659b7..26c20eb6 100644 --- a/quest/m0/kix/README.md +++ b/quest/m0/kix/README.md @@ -102,7 +102,6 @@ operator-paid-hosting decisions: - [Answer flow](/quest/m0/kix/answer-flow.md) - move to the next decision after an answer, with neutral refusals - [Agent activity](/quest/m0/kix/activity.md) - agent cards and map highlights built from checkpoint summaries - [Mobile web app](/quest/m0/kix/mobile.md) - an installable PWA with Web Push for decisions -- [Bot publication](/quest/m0/kix/publication.md) - validated agent results become normal GitHub PRs - [Web planning](/quest/m0/kix/planning.md) - issue-to-quest interviews produce draft planning PRs - [Web execution](/quest/m0/kix/execution.md) - spawn and iterate through the UI, ending in contribution PRs - [Onboarding guide](/quest/m0/kix/guide.md) - a complete maintainer path, from runner setup through a reviewed PR diff --git a/quest/m0/kix/merge.md b/quest/m0/kix/merge.md index adbc8da5..06ee897e 100644 --- a/quest/m0/kix/merge.md +++ b/quest/m0/kix/merge.md @@ -11,10 +11,10 @@ using a method the repository allows, or shows GitHub's refusal plainly. Decided 2026-10-18 as a follow-up of the decision inbox (#144), whose PR items offer only "Review on GitHub": -- Build on the bot publication's merge: bound to the current head, with the - method picked from the repository's allowed methods (squash, then merge - commit, then rebase). Keep that logic in publication and only add the - inbox action here. +- Build on `mergePullRequest` in `cloudflare/src/github/service.ts`: bound + to the current head, with the method picked from the repository's allowed + methods (squash, then merge commit, then rebase). Keep that logic there and + only add the inbox action here. - Show the button only when it can work: the PR is open, the viewer passes the per-request maintainer check, and GitHub reports it mergeable. Otherwise keep "Review on GitHub". @@ -28,5 +28,4 @@ offer only "Review on GitHub": ## Required -- [Bot publication](/quest/m0/kix/publication.md) - the head-bound merge and method selection - [Decision inbox](/quest/m0/kix/prompts.md) - the PR items that show the button diff --git a/quest/m0/kix/operator-setup.md b/quest/m0/kix/operator-setup.md index 42eb93a6..329d9b2c 100644 --- a/quest/m0/kix/operator-setup.md +++ b/quest/m0/kix/operator-setup.md @@ -14,7 +14,12 @@ and a bounded test PR verify the integration before contributors are invited. distinct from that namespace; do not assume an App can own a fork by itself. Set it as `GITHUB_FORK_OWNER` (steps 3 and 5 of `cloudflare/GITHUB.md`). - Validate real installation-token behavior across upstream and - service-owned forks. + service-owned forks, which publication's tests only mock (Publication in + `cloudflare/GITHUB.md`): an upstream token opening a pull request whose head + is on the fork's separate installation, a fork commit whose parent is an + upstream commit pushed after the fork was made, GitHub's signature on the + App's API commits, and an iteration pushing an upstream pull request head + under branch rules. Disable Actions on the service forks. - Rehearse on the operator's own runner and an explicitly selected operator account. - Record nonsecret setup/run instructions and deployment identifiers. @@ -23,4 +28,3 @@ and a bounded test PR verify the integration before contributors are invited. ## Required - [Deploy kix.quest](/quest/m0/kix/deploy.md) - the live origin and registered App -- [Bot publication](/quest/m0/kix/publication.md) - the integration exercised by the test PR diff --git a/quest/m0/kix/planning.md b/quest/m0/kix/planning.md index 4538e319..06faf237 100644 --- a/quest/m0/kix/planning.md +++ b/quest/m0/kix/planning.md @@ -33,4 +33,3 @@ interview runs on one of their runners; they never see a terminal. - [Runner](/quest/m0/kix/runner.md) - planning sessions, questions, and resumption - [Decision inbox](/quest/m0/kix/prompts.md) - the interactive rounds and notifications -- [Bot publication](/quest/m0/kix/publication.md) - draft planning PRs and recorded decisions diff --git a/quest/m0/kix/publication.md b/quest/m0/kix/publication.md deleted file mode 100644 index a97738d2..00000000 --- a/quest/m0/kix/publication.md +++ /dev/null @@ -1,55 +0,0 @@ -# [M] Publish agent work through the GitHub bot - -## Goal - -Authorized planning and implementation results become ordinary GitHub PRs -under the App bot's identity, with contributor/model attribution appropriate -to the repository. Repeated delivery updates the same intended PR. Agents -and runners never receive the App's credentials. - -## Plan - -- Keep publication in a trusted service component. Validate the active job, - permitted workflow output, pinned source, and expected branch/head before - writing anything. Treat submitted patches and files as data; do not run - their scripts or hooks in the publisher. -- Use service-controlled fork branches for contributed output, distinct from - the bot identity. Confirm GitHub's cross-installation fork/PR behavior with - an integration test. Keep Actions and secrets off those service forks. -- Plan jobs may publish the authorized quest changes and their interview - paper trail, not unrelated source changes. Implementation/iteration may - publish their selected task's changes. Unsupported permission-sensitive - changes must produce a decision, not be silently dropped. -- Runner checkpoints publish work in progress to the job's fenced branch - through this same validation, so a reassigned job can continue from it. - A fenced-out attempt cannot write to the branch. -- The service may need broader App registration permissions than each - operation uses; document that truthfully and narrow issued tokens. - No job gets an unrestricted push credential or permission to write main. -- For iteration on a human-authored PR, verify authority to update its actual - head, including contributor-fork permissions. If unavailable, report the - blocked action; do not silently open a replacement PR. -- Open planning PRs as drafts for maintainer review. Implementation PRs are - drafts while decisions or unfinished work remain; otherwise follow the - normal repository workflow. Preserve partial output after a stopped run. -- PR status, checks, and reviews remain GitHub's. A maintainer may explicitly - authorize a GitHub merge bound to the current head; honor branch rules and - refusals, and never interpret an agent recommendation as approval. -- Pick the merge method from the repository's allowed methods, preferring - squash, then merge commit, then rebase. Decided 2026-10-14: no per-repo - setting is needed, because GitHub's settings already say what's allowed. The - service's `mergePullRequest` currently hard-codes a merge commit, which - squash-only or rebase-only repositories refuse. -- Fork PRs do not make unsafe privileged CI safe. Document that upstream - workflows must treat contributed code as untrusted; do not grant contributed - code upstream credentials or run it in the publisher. -- Reconcile ambiguous GitHub outcomes before replaying publication so - interruptions cannot create duplicate PRs or overwrite newer work. -- Sources: - [fork API](https://docs.github.com/en/rest/repos/forks#create-a-fork), - [PR API](https://docs.github.com/en/rest/pulls/pulls#create-a-pull-request), - and [installation token scopes](https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/authenticating-as-a-github-app-installation). -- The job's `attempt` is the fence: accept a checkpoint bundle or result only - from the job's current `(attempt, runner)`, as `src/jobs/store.ts` does for - summaries. A completed job's `result` is opaque runner data to validate. - Bootstrap jobs publish only flake files. diff --git a/quest/m0/kix/runner.md b/quest/m0/kix/runner.md index 283c4505..f829587b 100644 --- a/quest/m0/kix/runner.md +++ b/quest/m0/kix/runner.md @@ -75,6 +75,12 @@ replacing the hosted Agent SDK runtime: away. Each account's opaque ID must stay stable across restarts so its preference rank and paused sessions follow it. A CLI sign-in flow and a control to reorder account preference remain here. +- Publish checkpoints and results with `POST /api/runner/publish` before the + `checkpoint` or `result` message: changed files as base64 data on the job's + `head`, plus the pull request's title, body, and draft flag on delivery. + Fetch the job's branch to continue a reassigned job; a `202` decision reply + pauses the attempt like a question. Publication in `cloudflare/GITHUB.md` + has the contract. - Add a runner and account settings page as a secondary view off the decision inbox: each runner's online state, its accounts, and the preference-order control. Decided 2026-10-18 as a follow-up of the