Public: anyone can read this; the crew can change it.
A builder run that declines as "already done on main in <commit>" posts that note on its issue and stops further runs on it #6592
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6573-a-builder-run-that-declines-as-already-d → main 3 commits fork 4 threads · 4 unresolved
8 files
changed +191
−8
docs/CREW.md +4 −1
| @@ -114,7 +114,10 @@ open with a note naming the commits, the asked list and the issue page say | ||
| 114 | 114 | "Done on main", and the person who asked closes it (the page's Close button, |
| 115 | 115 | or `x` on the issue in the terminal); the close writes "Closed: already done |
| 116 | 116 | on main in …" on the thread. A claim with no commit, or with one that is not |
| 117 | on the base, is an ordinary decline and is parked as one. | |
| 117 | on the base, is an ordinary decline and is parked as one. The answer holds | |
| 118 | even if the builder's pull was closed while it finished: the run is then | |
| 119 | kept as "already done", not as cancelled. A plain decline whose pull closed | |
| 120 | under it stays cancelled, with no strike. | |
| 118 | 121 | |
| 119 | 122 | **Live triage.** On each autonomy tick, an open issue carrying the crew's own |
| 120 | 123 | "already done" note (a person's note counts for nothing) is closed as *done* |
src/core/runs.ts +24
| @@ -310,6 +310,30 @@ export function finishRun( | ||
| 310 | 310 | return changes > 0 ? getRun(db, runId) : undefined; |
| 311 | 311 | } |
| 312 | 312 | |
| 313 | /** The summaries a run is cancelled with when its pull closes or lands under it. */ | |
| 314 | export const PULL_STOPPED = ["the pull closed", "the pull landed"] as const; | |
| 315 | ||
| 316 | /** | |
| 317 | * A run its pull closed under it, which had in fact reached its own answer | |
| 318 | * (already done on main): that answer replaces the pull's | |
| 319 | * summary, so the park or the note says what the builder said and not | |
| 320 | * "the pull closed". Only such a cancelled run is touched. | |
| 321 | */ | |
| 322 | export function answerCancelled( | |
| 323 | db: Database, | |
| 324 | runId: string, | |
| 325 | outcome: { state: "ok" | "failed"; summary: string }, | |
| 326 | ): Run | undefined { | |
| 327 | const changes = db | |
| 328 | .query( | |
| 329 | `UPDATE run SET state = ?2, summary = ?3 | |
| 330 | WHERE id = ?1 AND state = 'cancelled' AND summary IN (?4, ?5)`, | |
| 331 | ) | |
| 332 | .run(runId, outcome.state, outcome.summary.slice(0, 500), ...PULL_STOPPED) | |
| 333 | .changes; | |
| 334 | return changes > 0 ? getRun(db, runId) : undefined; | |
| 335 | } | |
| 336 | ||
| 313 | 337 | /** Record the base tip a pull's check merged with. */ |
| 314 | 338 | export function setRunBase(db: Database, runId: string, baseOid: string): void { |
| 315 | 339 | db.query(`UPDATE run SET base_oid = ?2 WHERE id = ?1`).run(runId, baseOid); |
src/crew/already.ts +22
| @@ -74,3 +74,25 @@ export function alreadyDoneOids(summary: string): string[] { | ||
| 74 | 74 | .filter((o) => /^[0-9a-f]{40}$/.test(o)) |
| 75 | 75 | .slice(0, MOST); |
| 76 | 76 | } |
| 77 | ||
| 78 | /** What a builder's final words decline with, when they begin `DECLINED`. */ | |
| 79 | export function declineOf(text: string): string | undefined { | |
| 80 | return /^\s*DECLINED\b/i.test(text) | |
| 81 | ? text.replace(/^\s*DECLINED:?\s*/i, "").trim() | |
| 82 | : undefined; | |
| 83 | } | |
| 84 | ||
| 85 | /** | |
| 86 | * Whether a run stopped from outside still has its own answer worth keeping: | |
| 87 | * an already-done decline it reached before its pull closed under it. A run | |
| 88 | * the meter stopped (`stoppedFor`) has none: the cap is the reason it ended. | |
| 89 | */ | |
| 90 | export const answeredBeforeStop = ( | |
| 91 | stoppedFor: string | undefined, | |
| 92 | text: string, | |
| 93 | ): boolean => | |
| 94 | stoppedFor === undefined && namedCommits(declineOf(text) ?? "").length > 0; | |
| 95 | ||
| 96 | /** Whether a run's summary is the already-done answer. */ | |
| 97 | export const isAnswer = (summary: string): boolean => | |
| 98 | summary.startsWith(ALREADY_DONE); |
src/crew/dispatch.ts +2 −1
| @@ -54,6 +54,7 @@ import { | ||
| 54 | 54 | leaseLapsed, |
| 55 | 55 | listActive, |
| 56 | 56 | listQueued, |
| 57 | PULL_STOPPED, | |
| 57 | 58 | type Run, |
| 58 | 59 | } from "../core/runs.ts"; |
| 59 | 60 | import { getUserById, type User } from "../core/users.ts"; |
| @@ -715,7 +716,7 @@ function route(app: App, event: ForgeEvent): void { | ||
| 715 | 716 | const cancelled = cancelRun( |
| 716 | 717 | app.db, |
| 717 | 718 | live.id, |
| 718 | event.kind === "pull.merged" ? "the pull landed" : "the pull closed", | |
| 719 | event.kind === "pull.merged" ? PULL_STOPPED[1] : PULL_STOPPED[0], | |
PULL_STOPPED[1] and [0] are easy to swap silently. Export named members, e.g. { closed, landed }, and keep the list for the IN clause. |
||
| 719 | 720 | ); |
| 720 | 721 | if (cancelled) announceRun(app, cancelled); |
| 721 | 722 | if (live.state === "running") { |
src/crew/runner.ts +22 −5
| @@ -59,6 +59,7 @@ import { | ||
| 59 | 59 | type Review, |
| 60 | 60 | } from "../core/reviews.ts"; |
| 61 | 61 | import { |
| 62 | answerCancelled, | |
| 62 | 63 | cancelRun, |
| 63 | 64 | checkSummary, |
| 64 | 65 | claimRun, |
| @@ -81,7 +82,14 @@ import { repoDir } from "../git/repos.ts"; | ||
| 81 | 82 | import { bar } from "../cloud/gate.ts"; |
| 82 | 83 | import { declarationsOf } from "../cloud/declared.ts"; |
| 83 | 84 | import { accountOfSecret } from "../core/cloud_account.ts"; |
| 84 | import { alreadyDoneSummary, namedCommits, onBase } from "./already.ts"; | |
| 85 | import { | |
| 86 | alreadyDoneSummary, | |
| 87 | answeredBeforeStop, | |
| 88 | declineOf, | |
| 89 | isAnswer, | |
| 90 | namedCommits, | |
| 91 | onBase, | |
| 92 | } from "./already.ts"; | |
| 85 | 93 | import { touchesDesign } from "./review.ts"; |
| 86 | 94 | import { BUDGETS, budgetSummary } from "../kernel/budgets.ts"; |
| 87 | 95 | import { createLog, redactValues } from "../kernel/log.ts"; |
| @@ -461,6 +469,14 @@ async function runClaimed( | ||
| 461 | 469 | commitOid: left, |
| 462 | 470 | }); |
| 463 | 471 | if (done) announceRun(app, done); |
| 472 | else if (isAnswer(outcome.summary)) { | |
| 473 | // Its pull closed under it, but it had found the work done: say so. | |
| 474 | const answered = answerCancelled(app.db, run.id, { | |
| 475 | state: outcome.state, | |
| 476 | summary, | |
| 477 | }); | |
| 478 | if (answered) announceRun(app, answered); | |
| 479 | } | |
| 464 | 480 | return undefined; |
| 465 | 481 | } |
| 466 | 482 | |
| @@ -1552,7 +1568,9 @@ async function work( | ||
| 1552 | 1568 | watch.lap("wrap-up"); |
| 1553 | 1569 | ran.inWrapUp = false; |
| 1554 | 1570 | } |
| 1555 | if (signal.aborted) | |
| 1571 | // A run whose pull closed under it may still have reached its answer: an | |
| 1572 | // already-done decline is kept (`answerCancelled`). One the meter stopped is not. | |
| 1573 | if (signal.aborted && !answeredBeforeStop(meter.stoppedFor, result.result)) | |
| 1556 | 1574 | return { |
| 1557 | 1575 | state: "failed", |
| 1558 | 1576 | summary: meter.stoppedFor ?? "cancelled", |
| @@ -1563,9 +1581,8 @@ async function work( | ||
| 1563 | 1581 | |
| 1564 | 1582 | // The decline contract: an agent that should not do the task says so |
| 1565 | 1583 | // instead of guessing, and nothing it touched becomes a proposal. |
| 1566 | const declined = /^\s*DECLINED\b/i.test(result.result); | |
| 1567 | if (declined) { | |
| 1568 | const why = result.result.replace(/^\s*DECLINED:?\s*/i, "").trim(); | |
| 1584 | const why = declineOf(result.result); | |
| 1585 | if (why !== undefined) { | |
| 1569 | 1586 | write(`declined: ${why.split("\n")[0] ?? ""}`); |
| 1570 | 1587 | ending = { kind: "declined", sentence: why.split("\n")[0] ?? "" }; |
| 1571 | 1588 | // Done on the base already, by a commit that is there: its own |
test/crew-early.test.ts +62 −1
| @@ -16,6 +16,9 @@ import { | ||
| 16 | 16 | openIssue, |
| 17 | 17 | setIssueState, |
| 18 | 18 | } from "../src/actions/issues.ts"; |
| 19 | import { answeredBeforeStop } from "../src/crew/already.ts"; | |
| 20 | import { alreadyDone } from "../src/core/done_on_main.ts"; | |
| 21 | import { updateRepoSettings } from "../src/actions/repos.ts"; | |
| 19 | 22 | import { pullOfBranch, watchForPull } from "../src/crew/early.ts"; |
| 20 | 23 | import { resolveOid } from "../src/git/merge.ts"; |
| 21 | 24 | import { abortAgentRun } from "../src/crew/running.ts"; |
| @@ -24,7 +27,7 @@ import { createIssue, getIssue, listComments } from "../src/core/issues.ts"; | ||
| 24 | 27 | import { listPulls } from "../src/core/pulls.ts"; |
| 25 | 28 | import { repoDir } from "../src/git/repos.ts"; |
| 26 | 29 | import { getForkOf, type Repo } from "../src/core/repos.ts"; |
| 27 | import { listRuns, type Run } from "../src/core/runs.ts"; | |
| 30 | import { getRun, listRuns, type Run } from "../src/core/runs.ts"; | |
| 28 | 31 | import type { User } from "../src/core/users.ts"; |
| 29 | 32 | import { readRunLog } from "../src/runs/log.ts"; |
| 30 | 33 | import { |
| @@ -289,4 +292,62 @@ describe("the crew, with a stand-in Claude Code: the early pull", () => { | ||
| 289 | 292 | expect(lines.length).toBe(1); |
| 290 | 293 | expect(pullOfBranch(app, repo, fork, branch)).toBeUndefined(); |
| 291 | 294 | }, 30_000); |
| 295 | ||
| 296 | test("a run whose pull is closed under it, after it answered already done on main, notes and holds its issue", async () => { | |
| 297 | rmSync(join(dataDir, "answered"), { force: true }); | |
| 298 | // The note and the hold are the autonomous crew's (`settleAgentRun`). | |
| 299 | const on = await updateRepoSettings(app, alice, repo, { | |
| 300 | autonomy: "autonomous", | |
| 301 | autonomyDirection: "Keep the garden small.", | |
| 302 | autonomyCeilingUsd: 100, | |
| 303 | autonomyBy: alice.id, | |
| 304 | }); | |
| 305 | expect(on.ok).toBe(true); | |
| 306 | try { | |
| 307 | const issue = openIssue(app, alice, repo, { | |
| 308 | title: "Weed the cress", | |
| 309 | body: "PUSH-THEN-DONE-ON-MAIN", | |
| 310 | }); | |
| 311 | if (!issue.ok) throw new Error("issue"); | |
| 312 | expect(assignIssue(app, alice, repo, issue.value, bot).ok).toBe(true); | |
| 313 | const branch = `bot/${issue.value.number}-weed-the-cress`; | |
| 314 | const mine = () => | |
| 315 | listPulls(app.db, repo.id, { state: "all", limit: 200 }).filter( | |
| 316 | (p) => p.headBranch === branch, | |
| 317 | ); | |
| 318 | const until = async (ok: () => boolean) => { | |
| 319 | const end = Date.now() + 15_000; | |
| 320 | while (!ok() && Date.now() < end) await Bun.sleep(20); | |
| 321 | return ok(); | |
| 322 | }; | |
| 323 | // The stand-in has pushed and given its answer; its pull is open. | |
| 324 | expect( | |
| 325 | await until(() => existsSync(join(dataDir, "answered")) && mine().length > 0), | |
| 326 | ).toBe(true); | |
| 327 | const pull = getIssue(app.db, repo.id, mine()[0]!.number)!; | |
| 328 | expect(setIssueState(app, alice, repo, pull, "closed").ok).toBe(true); | |
| 329 | // Cancelled at once by the pull; the runner then answers for it. | |
| 330 | const id = (await settled((r) => r.branch === branch)).id; | |
| 331 | await until(() => getRun(app.db, id)!.state !== "cancelled"); | |
| 332 | const run = getRun(app.db, id)!; | |
| 333 | expect(run.state).toBe("ok"); | |
| 334 | expect(run.summary).toMatch(/^already done: [0-9a-f]{40}$/); | |
| 335 | const oid = run.summary.slice("already done: ".length); | |
| 336 | await until(() => alreadyDone(app.db, [issue.value.id]).has(issue.value.id)); | |
| 337 | const notes = listComments(app.db, issue.value.id).map((c) => c.body); | |
| 338 | expect(notes.some((b) => b.startsWith("Parked by the crew"))).toBe(false); | |
| 339 | expect(alreadyDone(app.db, [issue.value.id]).get(issue.value.id)).toEqual([oid]); | |
| 340 | expect(getIssue(app.db, repo.id, issue.value.number)!.assigneeId).toBeNull(); | |
outdated
Reset autonomy in a finally block, so a failed assertion does not leave the shared repo autonomous for later tests. |
||
| 341 | } finally { | |
| 342 | await updateRepoSettings(app, alice, repo, { autonomy: "off" }); | |
| 343 | } | |
| 344 | }, 30_000); | |
This waits on a 20ms poll for a state change with no failure message. If it times out, the next expect fails with a misleading state. Assert the result of until() so the failure says what did not happen. |
||
| 345 | ||
| 346 | test("a run its meter stopped keeps no decline; a run only its pull stopped does", () => { | |
| 347 | const decline = "DECLINED: already done on main in 0e2eaaf4"; | |
| 348 | expect(answeredBeforeStop(undefined, decline)).toBe(true); | |
| 349 | expect(answeredBeforeStop(undefined, "DECLINED: out of scope")).toBe(false); | |
| 350 | expect(answeredBeforeStop(undefined, "Added CREW.md as asked.")).toBe(false); | |
| 351 | expect(answeredBeforeStop("the model grant is spent", decline)).toBe(false); | |
| 352 | }); | |
| 292 | 353 | }); |
test/crew-parks-done.test.ts +46
| @@ -23,6 +23,7 @@ import { | ||
| 23 | 23 | import { repoDir } from "../src/git/repos.ts"; |
| 24 | 24 | import { alreadyDone } from "../src/core/done_on_main.ts"; |
| 25 | 25 | import { askedIn, askedJson } from "../src/core/asked.ts"; |
| 26 | import { answerCancelled, cancelRun, createRun } from "../src/core/runs.ts"; | |
| 26 | 27 | import { parkedTwice, struckOut } from "../src/crew/parks.ts"; |
| 27 | 28 | import { engine, tick, type Engine } from "./helpers/parks-engine.ts"; |
| 28 | 29 | |
| @@ -387,4 +388,49 @@ describe("an issue found already done is the asker's to close, not a park", () = | ||
| 387 | 388 | expect(alreadyDone(e.app.db, [finished.id]).get(finished.id)).toEqual([oid]); |
| 388 | 389 | expect(numbers()).toEqual([open.number]); |
| 389 | 390 | }); |
| 391 | ||
| 392 | const cancelledByItsPull = (on: Issue) => { | |
| 393 | const run = createRun(e.app.db, { | |
| 394 | repoId: e.fresh().id, | |
| 395 | kind: "agent", | |
| 396 | issueId: on.id, | |
| 397 | actorId: e.mason.id, | |
| 398 | reason: "assigned", | |
| 399 | }); | |
| 400 | e.app.db.run(`UPDATE run SET state = 'running' WHERE id = ?1`, [run.id]); | |
| 401 | cancelRun(e.app.db, run.id, "the pull closed"); | |
| 402 | return run.id; | |
| 403 | }; | |
| 404 | ||
| 405 | test("a run its pull closed under, that had answered already done, notes the issue and holds it", () => { | |
| 406 | const work = e.issue("Closed under its decline"); | |
| 407 | const runId = cancelledByItsPull(work); | |
| 408 | const run = answerCancelled(e.app.db, runId, { | |
| 409 | state: "ok", | |
| 410 | summary: alreadyDoneSummary([oid]), | |
| 411 | })!; | |
| 412 | expect(run.summary).toBe(`already done: ${oid}`); | |
| 413 | expect( | |
| 414 | settleAgentRun(e.app, e.fresh(), { | |
| 415 | issueId: work.id, | |
| 416 | state: run.state, | |
| 417 | summary: run.summary, | |
| 418 | reason: "assigned", | |
| 419 | actorId: e.mason.id, | |
| 420 | }), | |
| 421 | ).toBe(true); | |
| 422 | expect(alreadyDone(e.app.db, [work.id]).get(work.id)).toEqual([oid]); | |
| 423 | expect(parkedCount(e.app, e.fresh(), work.id)).toBe(0); | |
| 424 | }); | |
| 425 | ||
| 426 | test("answerCancelled touches only a run its pull cancelled", () => { | |
| 427 | const work = e.issue("Cancelled for another reason"); | |
| 428 | const other = createRun(e.app.db, { repoId: e.fresh().id, kind: "agent", issueId: work.id }); | |
| 429 | cancelRun(e.app.db, other.id, "cancelled"); | |
| 430 | const done = { state: "ok" as const, summary: alreadyDoneSummary([oid]) }; | |
| 431 | expect(answerCancelled(e.app.db, other.id, done)).toBeUndefined(); | |
| 432 | const runId = cancelledByItsPull(work); | |
| 433 | expect(answerCancelled(e.app.db, runId, done)).toBeDefined(); | |
| 434 | expect(answerCancelled(e.app.db, runId, done)).toBeUndefined(); | |
| 435 | }); | |
| 390 | 436 | }); |
test/helpers/crew-standin.ts +9
| @@ -121,6 +121,15 @@ n=0; while [ ! -f "${dataDir}/release" ] && [ $n -lt 300 ]; do sleep 0.1; n=$((n | ||
| 121 | 121 | echo '{"type":"result","subtype":"success","is_error":false,"result":"DECLINED: this turned out to be out of scope.","total_cost_usd":0.005,"num_turns":1,"session_id":"fake-5","terminal_reason":"completed"}' |
| 122 | 122 | exit 0 |
| 123 | 123 | fi |
| 124 | if grep -q PUSH-THEN-DONE-ON-MAIN "${promptFile}"; then | |
| 125 | tip=$(git rev-parse HEAD) | |
| 126 | echo "early work $(date +%s%N)" >> CREW.md | |
| 127 | git add -A && git commit -qm "crew work" && git push -q -u origin HEAD | |
| 128 | printf '{"type":"result","subtype":"success","is_error":false,"result":"DECLINED: already done on main in %s","total_cost_usd":0.005,"num_turns":1,"session_id":"fake-7","terminal_reason":"completed"}\n' "$tip" | |
| 129 | touch "${dataDir}/answered" | |
| 130 | sleep 30 | |
| 131 | exit 0 | |
| 132 | fi | |
| 124 | 133 | if grep -q PUSH-THEN-FAIL "${promptFile}"; then |
| 125 | 134 | echo "early work $(date +%s%N)" >> CREW.md |
| 126 | 135 | git add -A && git commit -qm "crew work" && git push -q -u origin HEAD |
new above
offline
Match only ALREADY_DONE here. A plain "declined:" run that was cancelled by its pull should stay cancelled, with no new strike.