diff --git a/cloudflare/GITHUB.md b/cloudflare/GITHUB.md index 0f1fde8d..f1ca7cfc 100644 --- a/cloudflare/GITHUB.md +++ b/cloudflare/GITHUB.md @@ -67,7 +67,9 @@ 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 +progress the same way, so a reassigned job continues from its branch, as does +a retry of a failed job: it starts from the head and pull request the failed +job recorded, and a branch moved since still becomes a decision. 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 diff --git a/cloudflare/README.md b/cloudflare/README.md index 802d4182..0461bfeb 100644 --- a/cloudflare/README.md +++ b/cloudflare/README.md @@ -76,7 +76,8 @@ A job is one maintainer-authorized workflow (`plan`, `start`, `iterate`, or `bootstrap`) on one target, pinned to a revision (`src/jobs/`). Maintainers create, answer, and cancel jobs at `/projects/github///jobs`, and every request rechecks current GitHub authority. `start` takes only quests that -observations show ready and unclaimed. A job on an issue or pull request +observations show ready and unclaimed, except that retrying a failed job does +not count that job's own open pull request as a claim. A job on an issue or pull request rereads it from GitHub, refusing one that closed or whose head moved since observations read it. `iterate` jobs are untrusted and route only to Linux runners that declare VM builds. Without a `flake.nix`, the route @@ -126,8 +127,9 @@ saw, and is refused once observations moved past 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`. +one branch, which a reassigned attempt or a retried failed job 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. diff --git a/cloudflare/src/board/project.ts b/cloudflare/src/board/project.ts index 3f04d6a4..2ef3e6ee 100644 --- a/cloudflare/src/board/project.ts +++ b/cloudflare/src/board/project.ts @@ -28,7 +28,13 @@ export type Snapshot = { }; /** An open change that names a quest: its branch on a contributor's fork. */ -export type Change = { quest: string; author: string; href: string }; +export type Change = { + quest: string; + author: string; + href: string; + /** The pull request's number, when the change is one observed on GitHub. */ + number?: number; +}; /** An issue filed from a fork, quarantined under `issues/` until a maintainer promotes it. */ export type Issue = { path: string; title: string }; diff --git a/cloudflare/src/github/state.ts b/cloudflare/src/github/state.ts index 00836d74..548fda0d 100644 --- a/cloudflare/src/github/state.ts +++ b/cloudflare/src/github/state.ts @@ -201,6 +201,23 @@ export async function reconcile( if (result.at(-1)!.meta.changes !== 1) throw new GitHubError("Project changed during refresh; refresh again"); } +/** + * Whether `quest` can start on current observations: ready and unclaimed, or + * occupied only by `retry`, the open pull request of the quest's own failed + * job, which retrying it continues. Never while occupancy is unknown. + */ +export function startable(state: ProjectState, quest: string, retry: number | null) { + const { available, changes, documents } = state.project; + if (!available) return false; + if (available.includes(quest)) return true; + return ( + retry !== null && + !state.claims.includes(quest) && + changes.every((change) => change.quest !== quest || change.number === retry) && + ready(documents.map((d) => parse(d.path, d.content))).includes(quest) + ); +} + export async function readState(db: D1Database, repo: GitHubRepository): Promise { const records = await db.batch([ db @@ -237,6 +254,7 @@ export async function readState(db: D1Database, repo: GitHubRepository): Promise quest, author: pull.author?.login ?? "GitHub contributor", href: `https://github.com/${repo.owner}/${repo.name}/pull/${pull.number}`, + number: pull.number, }, ] : []; diff --git a/cloudflare/src/inbox/page.tsx b/cloudflare/src/inbox/page.tsx index 06c675e0..36cb42be 100644 --- a/cloudflare/src/inbox/page.tsx +++ b/cloudflare/src/inbox/page.tsx @@ -1,4 +1,4 @@ -import type { Pull } from "../github/state"; +import { type Pull, startable } from "../github/state"; import { type Prompt, parsePrompt } from "../jobs/prompt"; import { type Done, PUBLICATION_REPLY, inboxURL } from "../jobs/routes"; import { Marker } from "../layout"; @@ -265,9 +265,11 @@ function AccountForm(props: { item: Item; runners: Runners }) { /** Why new work cannot start from here yet, and how to fix it; `null` when it can. */ function blocked(item: Item, view: ProjectView | undefined) { if (item.project.state !== "enabled") return "Contributions are paused for this project."; - const available = view?.state?.project.available; - if (!available) return "GitHub reported changes. Refresh to start new work from current state."; - if (item.kind === "quest" && !available.includes(item.target)) + const state = view?.state; + if (!state?.project.available) return "GitHub reported changes. Refresh to start new work from current state."; + // A retry continues its failed job's pull request, so that claim does not block it. + const retry = item.need === "retry" ? (item.job?.pull ?? null) : null; + if (item.kind === "quest" && !startable(state, item.target, retry)) return "This quest is no longer ready to start, such as when it is claimed or removed."; if ((item.kind === "issue" || item.kind === "pull") && !item.activity?.data) return "Refresh to read this from GitHub."; diff --git a/cloudflare/src/jobs/routes.ts b/cloudflare/src/jobs/routes.ts index 3df49478..b94a995d 100644 --- a/cloudflare/src/jobs/routes.ts +++ b/cloudflare/src/jobs/routes.ts @@ -1,7 +1,7 @@ import { type Context, Hono } from "hono"; import { createAuth } from "../auth"; import { GitHubError, GitHubService, refName, type Stopped } from "../github/service"; -import type { ProjectState, Pull } from "../github/state"; +import { type ProjectState, type Pull, startable } from "../github/state"; import { RUNNER_HEADER } from "./hub"; import * as output from "./output"; import { formAnswer, parseAnswer, parsePrompt } from "./prompt"; @@ -170,8 +170,12 @@ jobs.get(base, async (c) => { type Plan = Omit; -/** Resolve a requested workflow and target against current observations. */ -function resolve(workflow: unknown, target: unknown, state: ProjectState): Plan { +/** + * Resolve a requested workflow and target against current observations. + * `failed` is the pull request of the target's failed latest job, which a + * retry continues rather than counting it as another claim. + */ +function resolve(workflow: unknown, target: unknown, state: ProjectState, failed: number | null): Plan { const commit = state.project.commit; const open = (kind: "issue" | "pull", prefix: string) => state.activity.find( @@ -182,7 +186,7 @@ function resolve(workflow: unknown, target: unknown, state: ProjectState): Plan item.data?.state === "OPEN", ); if (workflow === "start") { - if (typeof target !== "string" || !state.project.available?.includes(target)) + if (typeof target !== "string" || !startable(state, target, failed)) throw new JobError("Only ready, unclaimed quests on the default branch can start"); const branch = target.replace(/(\/README)?\.md$/, ""); if (!refName(branch)) throw new JobError("Rename the quest so its path can be a branch name"); @@ -236,8 +240,12 @@ jobs.post(base, async (c) => { const pinAccount = body.account === undefined ? null : id(body.account); if ((body.runner !== undefined && !pinRunner) || (body.account !== undefined && !pinAccount)) throw new JobError("Invalid runner or account"); + const failed = + body.workflow === "start" && typeof body.target === "string" + ? await store.failedPull(c.env.DB, repo.id, body.target) + : null; const { job, enabled, flake } = await github.jobSource(repo, user, (state) => - resolve(body.workflow, body.target, state), + resolve(body.workflow, body.target, state, failed), ); // A decision binds the revision the maintainer saw, such as a pull request's head. if (body.revision !== undefined && body.revision !== job.revision) diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index 22b6db08..adaaa313 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -557,12 +557,17 @@ export async function createJob(db: D1Database, input: NewJob, now: number) { const pin = [runner, runner, input.actor, trusted, account, account]; const id = crypto.randomUUID(); // The insert checks the project and pins again, since a pause or a - // runner's status can commit after the request read them. + // runner's status can commit after the request read them. An iteration + // continues the pull request's existing head branch. Retrying a target + // whose latest job failed continues that job's branch and pull request + // where it left them, as a reassigned attempt would. const [insert, active, enabled, pinned] = await db.batch([ db .prepare( - `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}`, + `INSERT OR IGNORE INTO jobs(id,repository,workflow,target,revision,branch,head,pending,pendingAt,pull,actor,trusted,pinRunner,pinAccount,state,createdAt,updatedAt) + SELECT ?,p.id,?,?,?,?,COALESCE(?,f.head),f.pending,f.pendingAt,f.pull,?,?,?,?,'queued',?,? FROM github_projects p + LEFT JOIN jobs f ON ?<>'iterate' AND f.state='failed' AND f.branch=? AND f.id=(SELECT id FROM jobs WHERE repository=p.id AND target=? ORDER BY createdAt DESC,id LIMIT 1) + WHERE p.id=? AND p.state='enabled' AND ${pinnable}`, ) .bind( id, @@ -570,7 +575,6 @@ 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, @@ -578,6 +582,9 @@ export async function createJob(db: D1Database, input: NewJob, now: number) { account, now, now, + input.workflow, + input.branch, + input.target, input.repository, ...pin, ), @@ -628,6 +635,15 @@ export async function listJobs(db: D1Database, repository: number) { return (await Promise.all(rows.results.map((row) => readJob(db, row.id)))).filter((job) => job !== null); } +/** The pull request of the target's latest job when that job failed: what a retry continues. */ +export async function failedPull(db: D1Database, repository: number, target: string) { + const latest = await db + .prepare("SELECT state,pull FROM jobs WHERE repository=? AND target=? ORDER BY createdAt DESC,id LIMIT 1") + .bind(repository, target) + .first>(); + return latest?.state === "failed" ? latest.pull : null; +} + /** Whether a job on `target` is queued, running, or waiting, and so may still push to it. */ export async function active(db: D1Database, repository: number, target: string) { return !!(await db diff --git a/cloudflare/test/inbox.test.ts b/cloudflare/test/inbox.test.ts index 5474615b..6ac73aab 100644 --- a/cloudflare/test/inbox.test.ts +++ b/cloudflare/test/inbox.test.ts @@ -5,7 +5,7 @@ import { questHref } from "../src/inbox/model"; import * as store from "../src/jobs/store"; import { privateKey } from "./github-key"; import { origin } from "./login-fixtures"; -import { commit, delivery, head, maintainerOf, project } from "./project-fixtures"; +import { commit, delivery, head, maintainerOf, project, pull } from "./project-fixtures"; const testEnv = { ...env, GITHUB_APP_PRIVATE_KEY: privateKey }; const db = env.DB; @@ -454,6 +454,35 @@ describe("decision inbox", () => { expect(active).not.toContain("Try again"); }); + it("retries a failed quest whose own pull request claims it, but no other claim", async () => { + // Pull request #2 now claims quest/first.md from its quest branch. + await db + .prepare("UPDATE github_project_activity SET data=? WHERE id='21'") + .bind(JSON.stringify({ ...pull, headRefName: "quest/first" })) + .run(); + const { id, runner } = await running("start", "quest/first.md"); + await store.delivered(db, id, 3); + await store.finish(db, runner, id, 1, { outcome: "failed", summary: "Broke.", result: null }, 3); + const retry = () => + submit(jobs, [ + ["workflow", "start"], + ["target", "quest/first.md"], + ["revision", commit], + ]); + // Another job's pull request is still someone else's claim. + expect(thread(await inbox({ project: "10", item: "quest/first.md" }))).toContain( + "This quest is no longer ready to start", + ); + expect((await retry()).status).toBe(409); + // The failed job's own pull request is what the retry continues. + await store.delivered(db, id, 2); + expect(thread(await inbox({ project: "10", item: "quest/first.md" }))).toContain("Try again"); + expect((await retry()).status).toBe(303); + const [latest] = await store.listJobs(db, 10); + expect(latest).toMatchObject({ target: "quest/first.md", state: "queued", pull: 2 }); + expect(latest!.id).not.toBe(id); + }); + it("encodes quest paths in board links", () => { const enrolled = { owner: "upstream", name: "project" } as Parameters[0]; expect(questHref(enrolled, "quest/a#b?.md")).toBe("/projects/github/upstream/project/board/quest/a%23b%3F"); diff --git a/cloudflare/test/publish.test.ts b/cloudflare/test/publish.test.ts index e42459c6..de228204 100644 --- a/cloudflare/test/publish.test.ts +++ b/cloudflare/test/publish.test.ts @@ -185,7 +185,7 @@ 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 = {}) { +async function running(extra: Partial = {}, now = 0) { const { id } = await store.createJob( db, { @@ -198,11 +198,15 @@ async function running(extra: Partial = {}) { trusted: true, ...extra, }, - 0, + now, ); - await store.dispatch(db, user, new Map([[machine.id, 1]]), 0); + await store.dispatch(db, user, new Map([[machine.id, 1]]), now); return id; } +/** Fail a job's current attempt, as its runner would. */ +async function fail(id: string) { + await store.finish(db, machine, id, 1, { outcome: "failed", summary: "Failed", result: undefined }, 0); +} function publish(body: Record, token = machine.token) { const text = JSON.stringify(body); return app.request( @@ -626,6 +630,69 @@ describe("runner publication", () => { expect(github.pulls).toEqual([]); }); + it("continues a retried job from the branch and pull request its failed job published", async () => { + const failed = await running(); + const { head } = (await ( + await publish({ + job: failed, + attempt: 1, + head: null, + message: "one", + changes: [file("a", "1")], + pull: { title: "T", body: "B" }, + }) + ).json()) as { head: string }; + await fail(failed); + const id = await running({}, 1000); + expect(await job(id)).toMatchObject({ state: "running", head, pull: 100 }); + const response = await publish({ + job: id, + attempt: 1, + head, + message: "two", + changes: [file("a", "2")], + pull: { title: "T", body: "B2" }, + }); + expect(response.status).toBe(200); + const next = (await response.json()) as { head: string; pull: { number: number } }; + expect(github.commits.get(next.head)!.parents).toEqual([head]); + expect(github.refs.get("quest-bot-forks/project:quest/first")).toBe(next.head); + expect(next.pull.number).toBe(100); + expect(github.pulls).toHaveLength(1); + }); + + it("starts a retried job fresh when its failed job published nothing", async () => { + await fail(await running()); + const id = await running({}, 1000); + expect(await job(id)).toMatchObject({ head: null, pull: null }); + const response = await publish({ job: id, attempt: 1, head: null, message: "m", changes: [file("a", "1")] }); + expect(response.status).toBe(200); + }); + + it("pauses a retried job when its branch moved outside kix after the failure", async () => { + const failed = await running(); + const { head } = (await ( + await publish({ job: failed, attempt: 1, head: null, message: "one", changes: [file("a", "1")] }) + ).json()) as { head: string }; + await fail(failed); + github.refs.set("quest-bot-forks/project:quest/first", prHead); + const id = await running({}, 1000); + 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("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(); diff --git a/quest/m0/kix/README.md b/quest/m0/kix/README.md index 2020c36b..03366547 100644 --- a/quest/m0/kix/README.md +++ b/quest/m0/kix/README.md @@ -104,7 +104,6 @@ operator-paid-hosting decisions: - [Webhook reconcile](/quest/m0/kix/reconcile.md) - new issues and PRs arrive in the inbox ready to act on, without a manual refresh - [Merge jobs](/quest/m0/kix/merge-job.md) - a merge pick fixes conflicts, CI, and reviews on a runner, then lands the PR - [Co-maintainer inbox](/quest/m0/kix/members.md) - every current maintainer sees enrolled projects in their inbox -- [Retry from the branch](/quest/m0/kix/retry.md) - a retried job continues from the head its failed attempt published - [Decision paper trail](/quest/m0/kix/paper-trail.md) - planning PRs carry the recorded questions and answers, rendered by the service - [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 diff --git a/quest/m0/kix/retry.md b/quest/m0/kix/retry.md deleted file mode 100644 index e5c32a3f..00000000 --- a/quest/m0/kix/retry.md +++ /dev/null @@ -1,22 +0,0 @@ -# [XS] Continue a retried job from its published branch - -## Goal - -"Try again" on a failed plan, bootstrap, or start job that already -published to its branch continues from that branch's last published head. -The retry publishes normally, and nobody deletes the branch by hand. - -## Plan - -Decided 2026-10-19 as a follow-up of the decision inbox (#144), whose final -Codex review found that a retry starts with `head` unset, so its first -publication pauses with "already has the branch … Delete it on GitHub to -start over" (`cloudflare/src/jobs/publish.ts`): - -- The retry records the failed job's last published `head` and continues on - the same branch, the way a reassigned job resumes from its checkpoint. The - existing fence and head checks still apply. -- A branch that changed outside kix since that head still pauses on the - existing decision; only a branch at the recorded head continues. -- Test a retry after a published checkpoint, a retry with nothing - published, and a branch moved outside kix.