diff --git a/AGENTS.md b/AGENTS.md index 651f895..027ab72 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ changing credentials, claims, Sandboxes, publishing, or GitHub writes. - Build agent output objects with `z.strictObject`. Bound strings and arrays. - Keep task inputs and results small and JSON-serializable. - Open Sandboxes through `withSandbox`. In GitHub tasks, call - `claim.recordSandbox()` immediately after opening one. + `claim.trackSandbox()` immediately after opening one. - Use `ExpectedFailure` only for expected messages safe to publish on GitHub. Keep outcome names in snake_case. - Quote generated shell arguments with `shq`. Put long or diff --git a/app/claims/claim.ts b/app/claims/claim.ts index 2eec774..a90077e 100644 --- a/app/claims/claim.ts +++ b/app/claims/claim.ts @@ -24,14 +24,14 @@ export interface Claim { readonly subject: Subject; readonly traceId: string; readonly runId: string; - /** Fails when a newer run has claimed this issue or PR. Call before durable writes. */ - checkpoint(): Promise; - /** Checks the claim, then shows progress in the status comment. */ - progress(summary: string): Promise; - /** Records an opened sandbox in the status comment, so a run that stops this one can terminate it. Call first thing. */ - recordSandbox(sandbox: RunAgentOptions["sandbox"]): Promise; + /** Ends the run as superseded once a newer run owns this issue or PR. Call before durable writes. */ + assertCurrent(): Promise; + /** Asserts the claim, then publishes `summary` in the status comment. */ + reportProgress(summary: string): Promise; + /** Records the sandbox in the status comment, so a run that stops this one can terminate it. Call right after opening it. */ + trackSandbox(sandbox: { id: string }): Promise; /** runAgent options for `sandbox`, so the agent's events carry this run's trace. */ - agentOptions(sandbox: RunAgentOptions["sandbox"]): RunAgentOptions; + agentRunOptions(sandbox: RunAgentOptions["sandbox"]): RunAgentOptions; } /** How the work ended. `summary` is shown on GitHub, so it must be safe to publish. */ @@ -111,6 +111,13 @@ const AGENT_STOPS: Partial> = { output_invalid: "submitted an invalid result", }; +interface RunningState { + summary: string; + sandboxId?: string; +} + +type RunningUpdate = { summary: string } | { sandboxId: string }; + /** The lifecycle behind a Claim. Only withClaim calls the methods beyond the Claim interface. */ class ClaimedRun implements Claim { readonly gh: GitHubClient; @@ -121,8 +128,7 @@ class ClaimedRun implements Claim { readonly #opts: ClaimOptions; readonly #event: { traceId: string; runId: string; repo: string; subject: string }; readonly #startedAt = Date.now(); - #summary = ""; - #sandboxId: string | undefined; + #running: RunningState = { summary: "" }; constructor(ctx: Pick, opts: ClaimOptions) { ({ traceId: this.traceId, runId: this.runId } = traceIds(ctx)); @@ -133,23 +139,19 @@ class ClaimedRun implements Claim { this.#event = { traceId: this.traceId, runId: this.runId, repo: opts.repo.fullName, subject: subjectName(opts.subject) }; } - async checkpoint(): Promise { + async assertCurrent(): Promise { if (!(await this.#holdsClaim())) throw new Superseded(); } - async progress(summary: string): Promise { - this.#summary = summary; - await this.checkpoint(); - // A newer run can take over between the check and the write; setStatus refuses to overwrite its running status. - if (!(await setStatus(this.#status("running", summary)))) throw new Superseded(); + async reportProgress(summary: string): Promise { + await this.#updateRunning({ summary }); } - async recordSandbox(sandbox: RunAgentOptions["sandbox"]): Promise { - this.#sandboxId = sandbox.id; - await this.progress(this.#summary); + async trackSandbox(sandbox: { id: string }): Promise { + await this.#updateRunning({ sandboxId: sandbox.id }); } - agentOptions(sandbox: RunAgentOptions["sandbox"]): RunAgentOptions { + agentRunOptions(sandbox: RunAgentOptions["sandbox"]): RunAgentOptions { return { sandbox, ...this.#event }; } @@ -179,7 +181,7 @@ class ClaimedRun implements Claim { runs, }); if (!acquired.acquired) throw new Superseded(); - await this.checkpoint(); + await this.assertCurrent(); } async finish(outcome: Outcome): Promise> { @@ -216,6 +218,13 @@ class ClaimedRun implements Claim { return status !== undefined && (status.traceId ?? status.runId) === this.traceId; } + async #updateRunning(update: RunningUpdate): Promise { + this.#running = { ...this.#running, ...update }; + await this.assertCurrent(); + // A newer run can take over between the check and the write; setStatus refuses to overwrite its running status. + if (!(await setStatus(this.#status("running", this.#running.summary)))) throw new Superseded(); + } + #status(state: S, summary: string, links?: StatusLink[]): StatusUpdate & { state: S } { const { task, title } = this.#opts; return { @@ -228,7 +237,7 @@ class ClaimedRun implements Claim { state, summary, links, - sandboxId: this.#sandboxId, + sandboxId: this.#running.sandboxId, }; } diff --git a/app/github/client.ts b/app/github/client.ts index b01f821..7c16a9f 100644 --- a/app/github/client.ts +++ b/app/github/client.ts @@ -162,10 +162,10 @@ export class GitHubClient { } /** - * The open PR from `head` with its title and body refreshed, or a new one. Safe to repeat: a replayed run - * finds the PR the first one opened. + * Makes an open PR from `head` exist with this title and body: updates the open one, or opens a new one. + * Safe to repeat: a replayed run finds the PR the first one opened. */ - async findOrCreatePull(args: { + async ensurePull(args: { head: string; base: string; title: string; diff --git a/app/workflows/autopilot/index.ts b/app/workflows/autopilot/index.ts index 076c5a4..409a32e 100644 --- a/app/workflows/autopilot/index.ts +++ b/app/workflows/autopilot/index.ts @@ -69,16 +69,16 @@ async function autopilot(claim: Claim, input: TriageInput, deps: AutopilotDeps, // Trigger labels are consumed when a run starts. Autopilot adds its label // back only while it is waiting for a human answer. await gh.removeLabel(input.issue, config.labels.autopilot); - await claim.progress("running Triage"); + await claim.reportProgress("running Triage"); const triage = record("triage", await runTriage({ ...base, issue: input.issue, commentId: input.commentId })); if (triage.state === "done" && triage.outcome === "asked") { - await claim.checkpoint(); + await claim.assertCurrent(); await gh.addLabels(input.issue, [config.labels.autopilot]); return finish("waiting_for_answers", triage.summary); } if (triage.state !== "done" || triage.data?.kind !== "spec") return stop("triage", triage); - await claim.progress("implementing the spec"); + await claim.reportProgress("implementing the spec"); const specToPr = record( "spec-to-pr", await runSpecToPr({ @@ -94,7 +94,7 @@ async function autopilot(claim: Claim, input: TriageInput, deps: AutopilotDeps, ); if (specToPr.state !== "done" || specToPr.data?.kind !== "pull_request") return stop("spec-to-pr", specToPr); - await claim.progress("reviewing the draft pull request"); + await claim.reportProgress("reviewing the draft pull request"); const review = record( "pr-review", await runPrReview({ ...base, pr: specToPr.data.pr, headSha: specToPr.data.headSha, workflowIssue: input.issue }), diff --git a/app/workflows/pr-review/index.ts b/app/workflows/pr-review/index.ts index 00ea25c..f3d8119 100644 --- a/app/workflows/pr-review/index.ts +++ b/app/workflows/pr-review/index.ts @@ -52,21 +52,21 @@ export const prReviewWorkflow = defineGitHubTask( const diffLines = await pullDiffLines(gh, input.pr, files); // 2. One sandbox at the PR head: verify, then review the complete change. - await claim.progress("opening a sandbox"); + await claim.reportProgress("opening a sandbox"); const { verify, review } = await withPullSandbox(deps.openSandbox ?? openSandbox, repo, pull, async (sandbox) => { - await claim.recordSandbox(sandbox); + await claim.trackSandbox(sandbox); const baseRef = await mergeBase(sandbox, pull.baseRef); - await claim.progress("running the verify commands"); + await claim.reportProgress("running the verify commands"); const verify = await runVerify(sandbox, repo.config.verify); - await claim.progress("reviewing the pull request"); + await claim.reportProgress("reviewing the pull request"); const message = reviewerMessage({ repo: repo.fullName, pull, baseRef, files, verify }); - const { output: review } = await (deps.runAgent ?? runAgent)(prReviewer, message, claim.agentOptions(sandbox)); + const { output: review } = await (deps.runAgent ?? runAgent)(prReviewer, message, claim.agentRunOptions(sandbox)); return { verify, review }; }); // 3. One COMMENT review on the head commit. Comments GitHub can't place inline go in the body. - await claim.progress("posting the review"); + await claim.reportProgress("posting the review"); const { posted, inlineComments } = await postReview(gh, { pr: input.pr, headSha: input.headSha, diff --git a/app/workflows/spec-to-pr/index.ts b/app/workflows/spec-to-pr/index.ts index fd65b2c..87b3b02 100644 --- a/app/workflows/spec-to-pr/index.ts +++ b/app/workflows/spec-to-pr/index.ts @@ -55,34 +55,34 @@ export const specToPrWorkflow = defineGitHubTask( const branch = branchName(input.issue); const rounds = config.limits.repairRounds; - await claim.progress("opening a sandbox"); + await claim.reportProgress("opening a sandbox"); const open = () => (deps.openSandbox ?? openSandbox)(repo, { kind: "branch", name: repo.config.baseBranch }); return withSandbox(open, async (sandbox) => { - await claim.recordSandbox(sandbox); + await claim.trackSandbox(sandbox); const baseSha = sandbox.headSha; const implement = async (message: string): Promise => - (await (deps.runAgent ?? runAgent)(implementer, message, { ...claim.agentOptions(sandbox), timeoutMs: IMPLEMENT_TIMEOUT_MS })).output; + (await (deps.runAgent ?? runAgent)(implementer, message, { ...claim.agentRunOptions(sandbox), timeoutMs: IMPLEMENT_TIMEOUT_MS })).output; const owned = await ownedCommits(git, branch, baseSha, author); - await claim.progress("implementing the spec"); + await claim.reportProgress("implementing the spec"); let implementation = await implement(implementMessage(repo, issue, spec)); if (!(await hasChanges(sandbox))) return noChanges(); let verification: VerifyRun[]; for (let round = 1; ; round++) { - await claim.progress("running the verify commands"); + await claim.reportProgress("running the verify commands"); verification = await runVerify(sandbox, repo.config.verify); if (allPassed(verification)) break; if (round > rounds) { const after = rounds > 0 ? ` after ${rounds} repair round${rounds === 1 ? "" : "s"}` : ""; throw new ExpectedFailure(`verification failed${after}: ${failureSummary(verification)}. No PR was opened.`, "verification_failed"); } - await claim.progress(`repairing failed verification (round ${round} of ${rounds})`); + await claim.reportProgress(`repairing failed verification (round ${round} of ${rounds})`); implementation = mergeImplementation(implementation, await implement(repairMessage(repo, issue, spec, verification, round, rounds))); } - await claim.progress("publishing the branch"); + await claim.reportProgress("publishing the branch"); const published = await (deps.publish ?? publish)({ sandbox, repo, @@ -98,8 +98,8 @@ export const specToPrWorkflow = defineGitHubTask( throw new ExpectedFailure(refusalSummary(published.refused), "publish_refused"); } - await claim.progress("opening the draft pull request"); - const { pull } = await gh.findOrCreatePull({ + await claim.reportProgress("opening the draft pull request"); + const { pull } = await gh.ensurePull({ head: branch, base: repo.config.baseBranch, title: pullRequestTitle(spec), diff --git a/app/workflows/triage/index.ts b/app/workflows/triage/index.ts index 3138658..3411200 100644 --- a/app/workflows/triage/index.ts +++ b/app/workflows/triage/index.ts @@ -33,16 +33,16 @@ export const triageWorkflow = defineGitHubTask( } const thread = readThread(issue, comments, bot); - await claim.progress("opening a sandbox"); + await claim.reportProgress("opening a sandbox"); const open = () => (deps.openSandbox ?? openSandbox)(repo, { kind: "branch", name: repo.config.baseBranch }); return withSandbox(open, async (sandbox) => { - await claim.recordSandbox(sandbox); - const agents = triageAgents(deps.runAgent ?? runAgent, claim.agentOptions(sandbox)); - await claim.progress("reading the issue and the repository"); + await claim.trackSandbox(sandbox); + const agents = triageAgents(deps.runAgent ?? runAgent, claim.agentRunOptions(sandbox)); + await claim.reportProgress("reading the issue and the repository"); const decision = await agents.triage(triageMessage(repo.fullName, thread)); if (decision.decision === "ask") { - await claim.progress("posting questions"); + await claim.reportProgress("posting questions"); const round = thread.questions ? thread.questions.round + 1 : 1; const body = `${triageQuestionsBody(decision.questions)}\n\n${renderMarker("questions", { round, questions: decision.questions })}`; const comment = thread.questions ? await gh.updateComment(thread.questions.comment.id, body) : await gh.createComment(issue.number, body); @@ -51,14 +51,14 @@ export const triageWorkflow = defineGitHubTask( } if (decision.decision === "decline") { - await claim.progress("posting the decision"); + await claim.reportProgress("posting the decision"); const comment = await gh.createComment(issue.number, triageDeclineBody(decision.rationale)); return { outcome: "declined", summary: "declined the issue.", links: [{ label: "Reason", url: comment.htmlUrl }] }; } - await claim.progress(thread.spec ? "revising the spec" : "drafting the spec"); + await claim.reportProgress(thread.spec ? "revising the spec" : "drafting the spec"); const drafted = await draftSpec(agents, sandbox, specWriterMessage(repo.fullName, thread, decision)); - await claim.progress("posting the spec"); + await claim.reportProgress("posting the spec"); return await publishSpec(gh, issue, thread.spec, drafted); }); }, diff --git a/docs/runbooks/extend.md b/docs/runbooks/extend.md index c46667a..9a71f8e 100644 --- a/docs/runbooks/extend.md +++ b/docs/runbooks/extend.md @@ -108,7 +108,7 @@ Export the agent from `app/agents/index.ts`. Call the agent inside a Sandbox owned by the task: ```typescript -const result = await runAgent(finder, message, claim.agentOptions(sandbox)); +const result = await runAgent(finder, message, claim.agentRunOptions(sandbox)); ``` Use the typed `result.output`. Do not parse assistant text or accept a target @@ -172,11 +172,11 @@ durable writes; a person starts a fresh run instead. ### 3. Use the claim correctly -- Call `claim.progress()` before a major step. +- Call `claim.reportProgress()` before a major step. - Open Sandboxes only through `withSandbox`. -- Call `claim.recordSandbox(sandbox)` immediately after opening one. -- Pass `claim.agentOptions(sandbox)` to `runAgent`. -- Call `claim.checkpoint()` before a durable effect that is not already +- Call `claim.trackSandbox(sandbox)` immediately after opening one. +- Pass `claim.agentRunOptions(sandbox)` to `runAgent`. +- Call `claim.assertCurrent()` before a durable effect that is not already guarded by the claim helper. - Throw `ExpectedFailure` only for an expected message safe to show on GitHub. diff --git a/tests/claims.test.ts b/tests/claims.test.ts index 53ac5f3..44be6af 100644 --- a/tests/claims.test.ts +++ b/tests/claims.test.ts @@ -266,9 +266,11 @@ describe("withClaim", () => { async (run) => { expect(run.runId).toBe("trn-run"); expect(gh.labels.has(WORKING_LABEL)).toBe(true); - await run.recordSandbox({ id: "sbx-run" } as never); + await run.trackSandbox({ id: "sbx-run" } as never); expect(parseMarker(gh.comments[0]?.body ?? "", "status")).toMatchObject({ runId: "trn-run", state: "running", sandboxId: "sbx-run" }); - await run.progress("reading the issue"); + await run.reportProgress("reading the issue"); + expect(gh.comments[0]?.body).toContain("reading the issue"); + expect(parseMarker(gh.comments[0]?.body ?? "", "status")?.sandboxId).toBe("sbx-run"); return { outcome: "asked-questions", summary: "asked 2 questions" }; }, ); @@ -296,7 +298,7 @@ describe("withClaim", () => { gh.permissions = { w: "write" }; const body = vi.fn(async (run: import("../app/claims/index.js").Claim) => { gh.addStatus("trn-new"); - await run.progress("doing expensive work"); + await run.reportProgress("doing expensive work"); return { outcome: "done", summary: "done" }; }); const outcome = await withClaim( diff --git a/tests/github-client.test.ts b/tests/github-client.test.ts index 7d09fb9..aef38b9 100644 --- a/tests/github-client.test.ts +++ b/tests/github-client.test.ts @@ -84,13 +84,13 @@ describe("GitHubClient", () => { html_url: "https://github.test/pull/9", }; - it("findOrCreatePull opens a draft, and recovers from losing a race", async () => { + it("ensurePull opens a draft, and recovers from losing a race", async () => { const fresh = fakeFetch(json([]), json(rawPull, { status: 201 })); - expect((await client(fresh.fetch).findOrCreatePull({ head: "h", base: "main", title: "t", body: "b" })).created).toBe(true); + expect((await client(fresh.fetch).ensurePull({ head: "h", base: "main", title: "t", body: "b" })).created).toBe(true); expect(fresh.calls[1]).toMatchObject({ method: "POST", body: { head: "h", base: "main", draft: true } }); const raced = fakeFetch(json([]), new Response("A pull request already exists", { status: 422 }), json([rawPull])); - expect(await client(raced.fetch).findOrCreatePull({ head: "h", base: "main", title: "t", body: "b" })).toMatchObject({ + expect(await client(raced.fetch).ensurePull({ head: "h", base: "main", title: "t", body: "b" })).toMatchObject({ created: false, pull: { number: 9 }, }); diff --git a/tests/spec-to-pr.test.ts b/tests/spec-to-pr.test.ts index e020b43..32e64d3 100644 --- a/tests/spec-to-pr.test.ts +++ b/tests/spec-to-pr.test.ts @@ -112,7 +112,7 @@ function fakeGitHub(opts: { spec?: unknown; specUpdatedAt?: string; labelEvents? }), getLabelEvents: vi.fn(async (_n: number, label: string) => (opts.labelEvents ?? []).map((createdAt) => ({ label, createdAt, actor: null }))), http: { json: vi.fn(async (_method: string, path: string) => (path === `/users/${encodeURIComponent(BOT)}` ? { id: BOT_ID } : undefined)) }, - findOrCreatePull: vi.fn(async (_args: { head: string; base: string; title: string; body: string; draft?: boolean }) => ({ + ensurePull: vi.fn(async (_args: { head: string; base: string; title: string; body: string; draft?: boolean }) => ({ pull: { number: 34, headSha: PR_HEAD, htmlUrl: `https://github.com/${REPO}/pull/34` }, created: true, })), @@ -283,7 +283,7 @@ describe("implement, verify, and publish", () => { expect(t.publish).toHaveBeenCalledWith( expect.objectContaining({ sandbox: t.sandbox, baseSha: BASE, branch: BRANCH, ownedCommits: [], gitData: t.git }), ); - const pr = t.gh.findOrCreatePull.mock.calls[0]?.[0]; + const pr = t.gh.ensurePull.mock.calls[0]?.[0]; expect(pr).toMatchObject({ head: BRANCH, base: "main", title: SPEC.title, draft: true }); expect(pr?.body).toContain("Added GET /health."); expect(pr?.body).toContain("✅ passed"); @@ -306,7 +306,7 @@ describe("implement, verify, and publish", () => { expect(repair?.message).toContain("expected 200, got 404"); expect(repair?.message).toContain("repair round 1 of 2"); expect(verifyCommands(t.execs)).toEqual([BUILD_COMMAND, TEST_COMMAND, BUILD_COMMAND, TEST_COMMAND]); - const body = t.gh.findOrCreatePull.mock.calls[0]?.[0].body ?? ""; + const body = t.gh.ensurePull.mock.calls[0]?.[0].body ?? ""; expect(body).toContain("Added GET /health."); expect(body).toContain("Add metrics"); }); @@ -323,7 +323,7 @@ describe("implement, verify, and publish", () => { expect(outcome).toMatchObject({ state: "failed", outcome: "verification_failed" }); expect(t.calls.filter((c) => c.agent === "implementer")).toHaveLength(1 + config.limits.repairRounds); expect(t.publish).not.toHaveBeenCalled(); - expect(t.gh.findOrCreatePull).not.toHaveBeenCalled(); + expect(t.gh.ensurePull).not.toHaveBeenCalled(); expect(t.status().body).toContain(`\`${TEST_COMMAND}\` (exit 1)`); expect(t.status().body).not.toContain("SECRET-LOOKING"); expect(finished()).toMatchObject({ state: "failed" });