Public: anyone can read this; the crew can change it.
Autonomous mode keeps the crew's own stalls off the owner's page: hand-overs and parked issues go to the planner, the home reads as status, and 'need you' appears only for decisions only a person can make #1902
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/1655-autonomous-mode-keeps-the-crew-s-own-sta → main 6 commits fork 3 threads · 3 unresolved
11 files
changed +376
−14
src/crew/autonomy.ts +2 −1
| @@ -92,6 +92,7 @@ import { pace, type Pace } from "./pacing.ts"; | ||
| 92 | 92 | import { BREAKER_NOTE, breakerTripped } from "./breaker.ts"; |
| 93 | 93 | import { |
| 94 | 94 | capReached, |
| 95 | handedOverPulls, | |
| 95 | 96 | ISSUE_RUN_CAP, |
| 96 | 97 | SKIP_SUMMARY, |
| 97 | 98 | skipsReplan, |
| @@ -557,7 +558,7 @@ async function planIfThin( | ||
| 557 | 558 | ready: number, |
| 558 | 559 | now: Date, |
| 559 | 560 | ): Promise<boolean> { |
| 560 | const handed = parkedTwice(app.db, [repo]); | |
| 561 | const handed = [...parkedTwice(app.db, [repo]), ...handedOverPulls(app.db, repo)]; | |
| 561 | 562 | if (ready >= THIN_MAP && handed.length === 0) return false; |
| 562 | 563 | const planner = crew.find((c) => c.role === "planner")?.agent; |
| 563 | 564 | if (planner === undefined) return notPlanned(repo, "no planner on the crew"); |
src/crew/brief.ts +8 −1
| @@ -194,6 +194,8 @@ export interface HandedOver { | ||
| 194 | 194 | readonly reasons: readonly string[]; |
| 195 | 195 | /** Whether an agent wrote it. A person's ask is never dropped. */ |
| 196 | 196 | readonly byAgent?: boolean | undefined; |
| 197 | /** A pull whose three reviews asked for changes, not a parked issue. */ | |
| 198 | readonly pull?: boolean | undefined; | |
| 197 | 199 | } |
| 198 | 200 | |
| 199 | 201 | /** |
| @@ -474,7 +476,7 @@ function parkedTwiceLines(input: TaskInput): string[] { | ||
| 474 | 476 | return [ |
| 475 | 477 | "The crew tried each of these twice and could not finish it as written. They are yours now, before anything new:", |
| 476 | 478 | ...handed.flatMap((issue) => [ |
| 477 | `- Parked twice: #${issue.number} ${issue.title} (${issue.byAgent === true ? "the crew's own" : "a person's ask"})`, | |
| 479 | `- ${issue.pull === true ? "Three reviews asked for changes, pull" : "Parked twice:"} #${issue.number} ${issue.title} (${issue.byAgent === true ? "the crew's own" : "a person's ask"})`, | |
| 478 | 480 | ...issue.reasons.map((reason, n) => ` ${n + 1}. ${reason}`), |
| 479 | 481 | ]), |
| 480 | 482 | "Leave every one of them in exactly one of these states, and say which in a comment on it:", |
| @@ -487,6 +489,11 @@ function parkedTwiceLines(input: TaskInput): string[] { | ||
| 487 | 489 | ]), |
| 488 | 490 | '- Dropped: only an issue the crew wrote, when the direction no longer needs it — say why on it and close it with {"state":"closed","reason":"dropped"}. A person\'s ask is never yours to drop: split it, replace it or defer it.', |
| 489 | 491 | `If other issues wait for one you close, make them wait for what replaces it: DELETE ${issues}/D/dependencies/N, then POST ${issues}/D/dependencies with {"blockedBy":M}.`, |
| 492 | ...(handed.some((h) => h.pull === true) | |
| 493 | ? [ | |
| 494 | "A pull above is not closed by splitting an issue: comment on it with the reviewer's ask restated as one concrete change so the builder has one more round, or close it and open the smaller issues it should have been.", | |
| 495 | ] | |
| 496 | : []), | |
| 490 | 497 | "Never leave one as it was: what you leave stays on the owner's board and comes back to you.", |
| 491 | 498 | ]; |
| 492 | 499 | } |
src/crew/dispatch.ts +1 −1
| @@ -266,7 +266,7 @@ function handOver( | ||
| 266 | 266 | (c) => c.body.startsWith(HANDOVER_NOTE) && c.createdAt >= over.at, |
| 267 | 267 | ); |
| 268 | 268 | if (said) return; |
| 269 | addComment(app, by, repo, issue, handoverNote(over)); | |
| 269 | addComment(app, by, repo, issue, handoverNote(over, repo.autonomy === "autonomous")); | |
| 270 | 270 | log.info("handed to a person after three reviews", { |
| 271 | 271 | repo: `${repo.ownerName}/${repo.name}`, |
| 272 | 272 | pull: issue.number, |
src/crew/guard.ts +47 −1
| @@ -19,6 +19,7 @@ import type { Database } from "bun:sqlite"; | ||
| 19 | 19 | import type { Agent } from "../core/agents.ts"; |
| 20 | 20 | import { recordDecision } from "../core/decisions.ts"; |
| 21 | 21 | import type { Issue } from "../core/issues.ts"; |
| 22 | import { listPulls } from "../core/pulls.ts"; | |
| 22 | 23 | import type { Repo } from "../core/repos.ts"; |
| 23 | 24 | import { directorsAmong } from "./direct.ts"; |
| 24 | 25 | import { nothingNew, plannerNothingNew } from "./news.ts"; |
| @@ -295,8 +296,53 @@ export function handoverLines(h: Handover): string[] { | ||
| 295 | 296 | }); |
| 296 | 297 | } |
| 297 | 298 | |
| 299 | /** | |
| 300 | * The open pulls of an autonomous repository whose review loop is spent: | |
| 301 | * the planner's to re-scope, split or send one more round with the | |
| 302 | * reviewer's ask restated, never the owner's. Shaped like a parked issue so | |
| 303 | * the planner's brief and its once-per-pass clock read both alike. | |
| 304 | */ | |
| 305 | export function handedOverPulls( | |
| 306 | db: Database, | |
| 307 | repo: Repo, | |
| 308 | ): { | |
| 309 | issueId: string; | |
| 310 | repoId: string; | |
| 311 | number: number; | |
| 312 | title: string; | |
| 313 | reasons: string[]; | |
| 314 | byAgent: boolean; | |
| 315 | at: string; | |
| 316 | pull: true; | |
| 317 | }[] { | |
| 318 | if (repo.autonomy !== "autonomous") return []; | |
| 319 | const out = []; | |
| 320 | for (const pull of listPulls(db, repo.id, { sealed: true, state: "open", limit: 50 })) { | |
| 321 | if (!pull.authorIsAgent) continue; | |
| 322 | const over = handoverOf(db, repo, pull.id); | |
| 323 | if (over === undefined) continue; | |
| 324 | out.push({ | |
| 325 | issueId: pull.id, | |
| 326 | repoId: repo.id, | |
| 327 | number: pull.number, | |
| 328 | title: pull.title, | |
| 329 | reasons: handoverLines(over), | |
| 330 | byAgent: true, | |
| 331 | at: over.at, | |
| 332 | pull: true as const, | |
| 333 | }); | |
| 334 | } | |
| 335 | return out; | |
| 336 | } | |
| 337 | ||
| 298 | 338 | /** The note the pull's thread keeps of the hand-over. */ |
| 299 | export function handoverNote(h: Handover): string { | |
| 339 | export function handoverNote(h: Handover, toPlanner = false): string { | |
| 340 | if (toPlanner) | |
| 341 | return [ | |
| 342 | `${HANDOVER_NOTE} the reviewer asked for changes three times with no approval between, so the planner re-scopes it: split, replaced, or one more round with the reviewer's ask restated.`, | |
| 343 | "", | |
| 344 | ...handoverLines(h), | |
| 345 | ].join("\n"); | |
| 300 | 346 | return [ |
| 301 | 347 | `${HANDOVER_NOTE} the reviewer asked for changes three times with no approval between, so no more feedback runs start here until you say what to try.`, |
| 302 | 348 | "", |
src/crew/review.ts +4 −2
| @@ -301,8 +301,10 @@ export async function sendToReview( | ||
| 301 | 301 | const history = reviewAttempts(app.db, pull); |
| 302 | 302 | if (history === undefined) return "wait"; |
| 303 | 303 | const stage = stageOf(history, now); |
| 304 | if (stage === "wait" || stage === "lapsed") return stage; | |
| 305 | const late = stage === "late"; | |
| 304 | // An autonomous repository sends a pull nobody reviewed whatever its age. | |
| 305 | const unreviewed = stage === "lapsed" && history.attempts.length === 0 && repo.autonomy === "autonomous"; | |
| 306 | if (stage === "wait" || (stage === "lapsed" && !unreviewed)) return stage; | |
| 307 | const late = stage === "late" || unreviewed; | |
| 306 | 308 | const issue = getIssueById(app.db, pull.id); |
| 307 | 309 | if (!issue) return "wait"; |
| 308 | 310 | const { diff } = await pullDiff( |
src/crew/runner.ts +2 −2
| @@ -116,7 +116,7 @@ import { historyHolds, scrubTree } from "./leak.ts"; | ||
| 116 | 116 | import { workdirFor } from "./workdir.ts"; |
| 117 | 117 | import { readersAmong, voiceOf } from "./direct.ts"; |
| 118 | 118 | import { feedbackSince } from "./news.ts"; |
| 119 | import { SKIP_SUMMARY, skipsFeedback } from "./guard.ts"; | |
| 119 | import { handedOverPulls, SKIP_SUMMARY, skipsFeedback } from "./guard.ts"; | |
| 120 | 120 | import { passportEnv } from "./passport_env.ts"; |
| 121 | 121 | |
| 122 | 122 | const log = createLog("crew"); |
| @@ -864,7 +864,7 @@ async function work( | ||
| 864 | 864 | // A planning run is given what the crew handed over (`parks.ts`). |
| 865 | 865 | ...(brief.role === "planner" && issue.id === repo.directionIssueId |
| 866 | 866 | ? { |
| 867 | parkedTwice: parkedTwice(app.db, [repo]), | |
| 867 | parkedTwice: [...parkedTwice(app.db, [repo]), ...handedOverPulls(app.db, repo)], | |
| 868 | 868 | laterGate: laterGate(app.db, repo.id), |
| 869 | 869 | } |
| 870 | 870 | : {}), |
src/web/studio/asks.ts +13 −3
| @@ -60,7 +60,7 @@ import { breakerParked, WAITING_NOTES } from "../../crew/breaker.ts"; | ||
| 60 | 60 | import { CHECK_FAILED_LIMIT } from "../../crew/dispatch.ts"; |
| 61 | 61 | import { handoverLines, handoverOf, type Handover } from "../../crew/guard.ts"; |
| 62 | 62 | import { conflictsOf, isStalePull, mergesAgainSince } from "../../crew/stale_asks.ts"; |
| 63 | import { waitsForPerson, waitsForReviewer, type ReviewWait } from "../../crew/review.ts"; | |
| 63 | import { reviewAttempts, waitsForPerson, waitsForReviewer, type ReviewWait } from "../../crew/review.ts"; | |
| 64 | 64 | import type { Ctx } from "../../http/context.ts"; |
| 65 | 65 | import { vitals } from "../../kernel/vitals.ts"; |
| 66 | 66 | import { repoBase } from "../chrome.ts"; |
| @@ -333,6 +333,11 @@ function askCandidatesFor( | ||
| 333 | 333 | // Three reviews without an approval: the crew stopped, and says so |
| 334 | 334 | // before anything about the check (`guard.ts`). |
| 335 | 335 | const over = handoverOf(db, repo, issue.id); |
| 336 | // An autonomous repository's spent review loop is the planner's | |
| 337 | // (`handedOverPulls`), not a question for a person (#1655). | |
| 338 | // A person's pull has no planner pass to take it, so it still asks. | |
| 339 | if (over !== undefined && repo.autonomy === "autonomous" && issue.authorIsAgent) | |
| 340 | continue; | |
| 336 | 341 | if (over !== undefined) { |
| 337 | 342 | out.push({ |
| 338 | 343 | kind: "handover", |
| @@ -406,7 +411,11 @@ function askCandidatesFor( | ||
| 406 | 411 | // an agent's change with no reviewer, or whose review lapsed, waits |
| 407 | 412 | // for a person — and so asks one (#597). |
| 408 | 413 | const wait = waitsForPerson(db, repo.id, pull); |
| 409 | if (wait !== undefined) | |
| 414 | // A review nobody answered is the sweep's to send, whatever its age | |
| 415 | // (`sendToReview`); every other wait is still a person's. | |
| 416 | const resent = | |
| 417 | wait?.why === "lapsed" && reviewAttempts(db, pull)?.attempts.length === 0; | |
| 418 | if (wait !== undefined && !resent) | |
| 410 | 419 | out.push({ |
| 411 | 420 | kind: "review", |
| 412 | 421 | id: `review:${pull.id}`, |
| @@ -447,7 +456,8 @@ function askCandidatesFor( | ||
| 447 | 456 | )) { |
| 448 | 457 | const issue = getIssueById(db, p.issueId); |
| 449 | 458 | const repo = byId.get(p.repoId); |
| 450 | if (issue && repo) | |
| 459 | // An autonomous repository's parked issue is the planner's (`parkedTwice`). | |
| 460 | if (issue && repo && repo.autonomy !== "autonomous") | |
| 451 | 461 | out.push({ |
| 452 | 462 | kind: "parked", |
| 453 | 463 | id: `parked:${issue.id}`, |
test/asks-autonomous.test.ts added +64
| @@ -0,0 +1,64 @@ | ||
| 1 | /** | |
| 2 | * Autonomous mode keeps the crew's own stalls off the owner's asks (#1655): | |
| 3 | * a parked issue is the planner's, an assisted repository's still asks. | |
| 4 | */ | |
| 5 | ||
| 6 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 7 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 8 | import { tmpdir } from "node:os"; | |
| 9 | import { join } from "node:path"; | |
| 10 | import { createApp, type App } from "../src/app.ts"; | |
| 11 | import { addComment, openIssue } from "../src/actions/issues.ts"; | |
| 12 | import { createRepo } from "../src/actions/repos.ts"; | |
| 13 | import { getRepoById, updateRepoSettings, type Repo } from "../src/core/repos.ts"; | |
| 14 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; | |
| 15 | import { BREAKER_NOTE } from "../src/crew/breaker.ts"; | |
| 16 | import { askLinesFor } from "../src/web/studio/asks.ts"; | |
| 17 | import { quietLogs } from "./helpers/globals.ts"; | |
| 18 | ||
| 19 | quietLogs(); | |
| 20 | ||
| 21 | describe("autonomous repositories ask no one about the crew's own stalls", () => { | |
| 22 | let dataDir = ""; | |
| 23 | let app: App; | |
| 24 | let alice: User; | |
| 25 | let repo: Repo; | |
| 26 | ||
| 27 | beforeAll(async () => { | |
| 28 | dataDir = await mkdtemp(join(tmpdir(), "forge-asks-auto-")); | |
| 29 | app = createApp({ dataDir, port: 0 }); | |
| 30 | const made = await createUser(app.db, { | |
| 31 | trusted: true, | |
| 32 | username: "alice", | |
| 33 | email: "[email protected]", | |
| 34 | password: "a-long-standing-passphrase", | |
| 35 | }); | |
| 36 | if (!made.ok) throw new Error("no alice"); | |
| 37 | alice = getUserByName(app.db, "alice")!; | |
| 38 | const r = await createRepo(app, alice, { name: "shelf", visibility: "public" }); | |
| 39 | if (!r.ok) throw new Error(String(r.error)); | |
| 40 | repo = r.value; | |
| 41 | }, 30_000); | |
| 42 | ||
| 43 | afterAll(async () => { | |
| 44 | app.close(); | |
| 45 | await rm(dataDir, { recursive: true, force: true }); | |
| 46 | }); | |
| 47 | ||
| 48 | test("a parked issue asks on an assisted repository and on no autonomous one", () => { | |
| 49 | const made = openIssue(app, alice, repo, { title: "Shelve it" }); | |
| 50 | if (!made.ok) throw new Error(String(made.error)); | |
| 51 | expect(addComment(app, alice, repo, made.value, BREAKER_NOTE).ok).toBe(true); | |
| 52 | const asking = () => | |
| 53 | askLinesFor(app.db, alice).some((r) => r.items.includes(`parked:${made.value.id}`)); | |
| 54 | repo = updateRepoSettings(app.db, repo, { autonomy: "assisted" }); | |
| 55 | expect(asking()).toBe(true); | |
| 56 | repo = updateRepoSettings(app.db, repo, { | |
| 57 | autonomy: "autonomous", | |
| 58 | autonomyBy: alice.id, | |
| 59 | autonomyCeilingUsd: 10, | |
| 60 | }); | |
| 61 | expect(getRepoById(app.db, repo.id)!.autonomy).toBe("autonomous"); | |
| 62 | expect(asking()).toBe(false); | |
| 63 | }); | |
| 64 | }); |
test/crew-loop.test.ts +28 −2
| @@ -14,7 +14,7 @@ import { addComment, openIssue } from "../src/actions/issues.ts"; | ||
| 14 | 14 | import { commitToBranch } from "../src/actions/repos.ts"; |
| 15 | 15 | import { openPull, submitReview } from "../src/actions/pulls.ts"; |
| 16 | 16 | import { getPull, type Pull } from "../src/core/pulls.ts"; |
| 17 | import { HANDOVER_NOTE, handoverOf } from "../src/crew/guard.ts"; | |
| 17 | import { HANDOVER_NOTE, handedOverPulls, handoverOf } from "../src/crew/guard.ts"; | |
| 18 | 18 | import { sendToReview, waitsForPerson } from "../src/crew/review.ts"; |
| 19 | 19 | import { createRepo, updateRepoSettings } from "../src/actions/repos.ts"; |
| 20 | 20 | import { setCrewRole } from "../src/core/agents.ts"; |
| @@ -432,16 +432,40 @@ describe("a crew reviewer's changes-requested wakes the builder", () => { | ||
| 432 | 432 | expect(notes.length).toBe(1); |
| 433 | 433 | expect(notes[0]!.body).toContain("1. sentinel: Round one"); |
| 434 | 434 | expect(notes[0]!.body).toContain("3. sentinel: Round three"); |
| 435 | // Not parked, not closed: it is still open and asks a person. | |
| 435 | expect(notes[0]!.body).toContain("planner"); | |
| 436 | // Not parked, not closed: still open, and the planner's, not a person's (#1655). | |
| 436 | 437 | expect(getIssueById(app.db, pull.id)!.state).toBe("open"); |
| 438 | expect(askLinesFor(app.db, alice).filter((a) => a.kind === "handover")).toEqual([]); | |
| 439 | expect(handedOverPulls(app.db, fresh()).map((h) => h.number)).toContain(pull.number); | |
| 440 | // An assisted repository has no planner: the same hand-over asks a person. | |
| 441 | const assisted = await updateRepoSettings(app, alice, fresh(), { autonomy: "assisted" }); | |
| 442 | if (!assisted.ok) throw new Error(JSON.stringify(assisted.error)); | |
| 437 | 443 | const asked = askLinesFor(app.db, alice).filter((a) => a.kind === "handover"); |
| 438 | 444 | expect(asked.length).toBe(1); |
| 439 | 445 | expect(asked[0]!.number).toBe(pull.number); |
| 446 | expect(handedOverPulls(app.db, fresh())).toEqual([]); | |
| 447 | const back = await updateRepoSettings(app, alice, fresh(), { autonomy: "autonomous" }); | |
| 448 | if (!back.ok) throw new Error(JSON.stringify(back.error)); | |
| 440 | 449 | await verdict(pull, "Round four."); |
| 441 | 450 | expect(feedback(pull).length).toBe(2); |
| 442 | 451 | expect(handoverNotes(pull).length).toBe(1); |
| 443 | 452 | }); |
| 444 | 453 | |
| 454 | test("a person's own pull with a spent review loop still asks the owner: the planner's list is the crew's pulls only (#1655)", async () => { | |
| 455 | const pull = await masonPull("person-spent"); | |
| 456 | await verdict(pull, "One."); | |
| 457 | settleFeedback(pull); | |
| 458 | await verdict(pull, "Two."); | |
| 459 | settleFeedback(pull); | |
| 460 | await verdict(pull, "Three."); | |
| 461 | expect(handedOverPulls(app.db, fresh()).map((h) => h.number)).toContain(pull.number); | |
| 462 | app.db.query("UPDATE issue SET author_id = ?2 WHERE id = ?1").run(pull.id, alice.id); | |
| 463 | expect(handedOverPulls(app.db, fresh()).map((h) => h.number)).not.toContain(pull.number); | |
| 464 | expect( | |
| 465 | askLinesFor(app.db, alice).some((a) => a.number === pull.number && a.kind === "handover"), | |
| 466 | ).toBe(true); | |
| 467 | }); | |
| 468 | ||
| 445 | 469 | test("a person's comment resets the count", async () => { |
| 446 | 470 | const pull = await masonPull("reset-comment"); |
| 447 | 471 | await verdict(pull, "One."); |
| @@ -488,6 +512,7 @@ describe("a crew reviewer's changes-requested wakes the builder", () => { | ||
| 488 | 512 | settleFeedback(pull); |
| 489 | 513 | await verdict(pull, "Two."); |
| 490 | 514 | settleFeedback(pull); |
| 515 | await updateRepoSettings(app, alice, fresh(), { autonomy: "assisted" }); | |
| 491 | 516 | await verdict(pull, "Three."); |
| 492 | 517 | expect(ASK_WORDS.handover.options).toEqual(["Tell them what to try", "Close it"]); |
| 493 | 518 | expect(KIND_TIER.handover).toBeDefined(); |
| @@ -541,6 +566,7 @@ describe("a crew reviewer's changes-requested wakes the builder", () => { | ||
| 541 | 566 | settleFeedback(pull); |
| 542 | 567 | await verdict(pull, "Two."); |
| 543 | 568 | settleFeedback(pull); |
| 569 | await updateRepoSettings(app, alice, fresh(), { autonomy: "assisted" }); | |
| 544 | 570 | await verdict(pull, "Three."); |
| 545 | 571 | const first = askLinesFor(app.db, alice) |
| 546 | 572 | .filter((a) => a.kind === "handover") |
test/crew-parks.test.ts +94
| @@ -41,9 +41,11 @@ import { | ||
| 41 | 41 | listRuns, |
| 42 | 42 | } from "../src/core/runs.ts"; |
| 43 | 43 | import { migrations } from "../src/core/schema.ts"; |
| 44 | import { submitReview } from "../src/core/reviews.ts"; | |
| 44 | 45 | import { createToken } from "../src/core/tokens.ts"; |
| 45 | 46 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; |
| 46 | 47 | import { serve, type Servers } from "../src/http/server.ts"; |
| 48 | import { handedOverPulls } from "../src/crew/guard.ts"; | |
| 47 | 49 | import { driveRepo, parkedCount, settleAgentRun } from "../src/crew/autonomy.ts"; |
| 48 | 50 | import { taskPrompt } from "../src/crew/brief.ts"; |
| 49 | 51 | import { breakerTripped } from "../src/crew/breaker.ts"; |
| @@ -594,6 +596,13 @@ describe("an issue parked twice is the planner's", () => { | ||
| 594 | 596 | const resumed = told({ resuming: true }); |
| 595 | 597 | expect(resumed).toContain(`- Parked twice: #${work.number} Scene graph`); |
| 596 | 598 | expect(resumed).not.toContain("not captured in words"); |
| 599 | // A spent pull reads as a pull, and adds the pull-only instruction. | |
| 600 | const pulled = told({ | |
| 601 | parkedTwice: [{ number: 9, title: "Spent", reasons: ["a"], byAgent: true, pull: true }], | |
| 602 | }); | |
| 603 | expect(pulled).toContain("- Three reviews asked for changes, pull #9 Spent (the crew's own)"); | |
| 604 | expect(pulled).toContain("A pull above is not closed by splitting an issue"); | |
| 605 | expect(brief).not.toContain("A pull above is not closed"); | |
| 597 | 606 | // A builder is never told; a planner handed nothing reads as before. |
| 598 | 607 | expect(told({ role: "builder" })).not.toContain("Parked twice"); |
| 599 | 608 | expect(told({ parkedTwice: [] })).not.toContain("Parked twice"); |
| @@ -1150,6 +1159,91 @@ echo '{"type":"result","subtype":"success","is_error":false,"result":"Nothing to | ||
| 1150 | 1159 | }, 60_000); |
| 1151 | 1160 | }); |
| 1152 | 1161 | |
| 1162 | describe("a pull whose review loop is spent is the planner's (#1655)", () => { | |
| 1163 | let e: Engine; | |
| 1164 | let crew: CrewHandle; | |
| 1165 | let promptFile: string; | |
| 1166 | ||
| 1167 | beforeAll(async () => { | |
| 1168 | e = await engine("forge-spent-pull-"); | |
| 1169 | const seeded = await commitToBranch(e.app, e.alice, e.repo, { | |
| 1170 | message: "Start", | |
| 1171 | files: [{ path: "README.md", content: "# engine\n" }], | |
| 1172 | }); | |
| 1173 | if (!seeded.ok) throw new Error(JSON.stringify(seeded.error)); | |
| 1174 | promptFile = join(e.dataDir, "fake.prompt"); | |
| 1175 | const fake = join(e.dataDir, "fake-claude"); | |
| 1176 | writeFileSync( | |
| 1177 | fake, | |
| 1178 | `#!/bin/sh | |
| 1179 | if [ "$1" = "--version" ]; then echo "2.1.0 (fake)"; exit 0; fi | |
| 1180 | IFS= read -r first; printf '%s\\n' "$first" > "${promptFile}" | |
| 1181 | echo '{"type":"system","subtype":"init","session_id":"fake-plan","model":"fake"}' | |
| 1182 | echo '{"type":"result","subtype":"success","is_error":false,"result":"Nothing to add.","total_cost_usd":0.01,"num_turns":1,"session_id":"fake-plan","terminal_reason":"completed"}' | |
| 1183 | `, | |
| 1184 | ); | |
| 1185 | chmodSync(fake, 0o755); | |
| 1186 | crew = await startCrew(e.app, { | |
| 1187 | forgeUrl: e.servers.url, | |
| 1188 | localUrl: e.servers.url, | |
| 1189 | binary: fake, | |
| 1190 | concurrency: 1, | |
| 1191 | budgetUsd: 1, | |
| 1192 | runTimeoutMs: 60_000, | |
| 1193 | }); | |
| 1194 | }, 30_000); | |
| 1195 | ||
| 1196 | afterAll(async () => { | |
| 1197 | await crew.stop(); | |
| 1198 | await e.close(); | |
| 1199 | }); | |
| 1200 | ||
| 1201 | test("the planner is asked for it and its brief says three reviews asked for changes", async () => { | |
| 1202 | const agent = await createAgent(e.app, e.alice, { username: "sentinel" }); | |
| 1203 | if (!agent.ok) throw new Error(String(agent.error)); | |
| 1204 | expect(addToCrew(e.app, e.alice, e.fresh(), "sentinel").ok).toBe(true); | |
| 1205 | const sentinel = getUserByName(e.app.db, "sentinel")!; | |
| 1206 | setCrewRole(e.app.db, e.repo.id, sentinel.id, "reviewer"); | |
| 1207 | const branch = await commitToBranch(e.app, e.alice, e.fresh(), { | |
| 1208 | branch: "feature/spent", | |
| 1209 | base: "main", | |
| 1210 | message: "spent", | |
| 1211 | files: [{ path: "spent.txt", content: "spent\n" }], | |
| 1212 | }); | |
| 1213 | if (!branch.ok) throw new Error(JSON.stringify(branch.error)); | |
| 1214 | const opened = await openPull(e.app, e.alice, e.fresh(), { | |
| 1215 | title: "Spent loop", | |
| 1216 | baseBranch: "main", | |
| 1217 | headBranch: "feature/spent", | |
| 1218 | }); | |
| 1219 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 1220 | const pull = opened.value; | |
| 1221 | e.app.db.query("UPDATE issue SET author_id = ?2 WHERE id = ?1").run(pull.id, e.mason.id); | |
| 1222 | for (const n of [1, 2, 3]) | |
| 1223 | submitReview(e.app.db, { | |
| 1224 | issueId: pull.id, | |
| 1225 | authorId: sentinel.id, | |
| 1226 | state: "changes_requested", | |
| 1227 | body: `Round ${n}`, | |
| 1228 | commitOid: pull.headOid, | |
| 1229 | }); | |
| 1230 | expect(handedOverPulls(e.app.db, e.fresh()).map((h) => h.number)).toEqual([pull.number]); | |
| 1231 | expect(await driveRepo(e.app, e.fresh())).toBe("planned"); | |
| 1232 | const direction = e.fresh().directionIssueId!; | |
| 1233 | const deadline = Date.now() + 30_000; | |
| 1234 | const settled = () => | |
| 1235 | listRuns(e.app.db, e.repo.id, { kind: "agent" }).find( | |
| 1236 | (r) => r.issueId === direction && ["ok", "failed", "cancelled"].includes(r.state), | |
| 1237 | ); | |
| 1238 | while (settled() === undefined && Date.now() < deadline) await Bun.sleep(100); | |
| 1239 | expect(existsSync(promptFile)).toBe(true); | |
| 1240 | const told = readFileSync(promptFile, "utf8"); | |
| 1241 | expect(told).toContain(`Three reviews asked for changes, pull #${pull.number} Spent loop`); | |
| 1242 | expect(told).toContain("Round 3"); | |
| 1243 | expect(told).toContain("A pull above is not closed by splitting an issue"); | |
| 1244 | }, 60_000); | |
| 1245 | }); | |
| 1246 | ||
| 1153 | 1247 | describe("migration 43", () => { |
| 1154 | 1248 | test("rewords old outage parks so they no longer count, and leaves real parks", () => { |
| 1155 | 1249 | const db = new Database(":memory:"); |
test/review-gate.test.ts +113 −1
| @@ -25,7 +25,8 @@ import { getRepoById, type Repo } from "../src/core/repos.ts"; | ||
| 25 | 25 | import { claimRun, createRun, finishRun, listRuns } from "../src/core/runs.ts"; |
| 26 | 26 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; |
| 27 | 27 | import { landIfGreen, settleAgentRun } from "../src/crew/autonomy.ts"; |
| 28 | import { waitsForPerson } from "../src/crew/review.ts"; | |
| 28 | import { sendToReview, waitsForPerson } from "../src/crew/review.ts"; | |
| 29 | import { askLinesFor } from "../src/web/studio/asks.ts"; | |
| 29 | 30 | |
| 30 | 31 | describe("the review gate", () => { |
| 31 | 32 | let dataDir: string; |
| @@ -312,3 +313,114 @@ describe("the review gate", () => { | ||
| 312 | 313 | expect(getPull(app.db, repo.id, moved.number)?.state).toBe("open"); |
| 313 | 314 | }); |
| 314 | 315 | }); |
| 316 | ||
| 317 | describe("a pull nobody reviewed, however old (#1655)", () => { | |
| 318 | let dataDir: string; | |
| 319 | let app: App; | |
| 320 | let alice: User; | |
| 321 | let mason: User; | |
| 322 | let repo: Repo; | |
| 323 | const fresh = () => getRepoById(app.db, repo.id)!; | |
| 324 | const later = new Date(Date.now() + 30 * 24 * 3_600_000); | |
| 325 | ||
| 326 | beforeAll(async () => { | |
| 327 | dataDir = await mkdtemp(join(tmpdir(), "forge-review-old-")); | |
| 328 | app = createApp({ dataDir, port: 0 }); | |
| 329 | const a = await createUser(app.db, { trusted: true, | |
| 330 | username: "alice", | |
| 331 | email: "[email protected]", | |
| 332 | password: "a-good-passphrase", | |
| 333 | adminIfFirst: true, | |
| 334 | }); | |
| 335 | if (!a.ok) throw new Error(a.error); | |
| 336 | alice = a.value; | |
| 337 | const made = await createRepo(app, alice, { name: "engine", visibility: "private" }); | |
| 338 | if (!made.ok) throw new Error(String(made.error)); | |
| 339 | repo = made.value; | |
| 340 | const seeded = await commitToBranch(app, alice, repo, { | |
| 341 | message: "Start", | |
| 342 | files: [{ path: "README.md", content: "# engine\n" }], | |
| 343 | }); | |
| 344 | if (!seeded.ok) throw new Error(JSON.stringify(seeded.error)); | |
| 345 | for (const name of ["mason", "quill"]) { | |
| 346 | const agent = await createAgent(app, alice, { username: name }); | |
| 347 | if (!agent.ok) throw new Error(String(agent.error)); | |
| 348 | expect(addToCrew(app, alice, repo, name).ok).toBe(true); | |
| 349 | } | |
| 350 | mason = getUserByName(app.db, "mason")!; | |
| 351 | setCrewRole(app.db, repo.id, getUserByName(app.db, "quill")!.id, "reviewer"); | |
| 352 | }); | |
| 353 | ||
| 354 | afterAll(async () => { | |
| 355 | await app.close?.(); | |
| 356 | await rm(dataDir, { recursive: true, force: true }); | |
| 357 | }); | |
| 358 | ||
| 359 | async function greenPull(name: string): Promise<Pull> { | |
| 360 | const branch = await commitToBranch(app, alice, repo, { | |
| 361 | branch: `feature/${name}`, | |
| 362 | base: "main", | |
| 363 | message: name, | |
| 364 | files: [{ path: `${name}.ts`, content: `export const n = "${name}";\n` }], | |
| 365 | }); | |
| 366 | if (!branch.ok) throw new Error(JSON.stringify(branch.error)); | |
| 367 | const opened = await openPull(app, alice, repo, { | |
| 368 | title: name, | |
| 369 | baseBranch: "main", | |
| 370 | headBranch: `feature/${name}`, | |
| 371 | }); | |
| 372 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 373 | app.db.query("UPDATE issue SET author_id = ?2 WHERE id = ?1").run(opened.value.id, mason.id); | |
| 374 | const pull = getPull(app.db, repo.id, opened.value.number)!; | |
| 375 | const check = createRun(app.db, { | |
| 376 | repoId: repo.id, | |
| 377 | kind: "check", | |
| 378 | issueId: pull.id, | |
| 379 | env: `pull-${pull.number}`, | |
| 380 | commitOid: pull.headOid, | |
| 381 | reason: "pull", | |
| 382 | }); | |
| 383 | claimRun(app.db, check.id); | |
| 384 | finishRun(app.db, check.id, { state: "ok", summary: "check passed" }); | |
| 385 | return pull; | |
| 386 | } | |
| 387 | ||
| 388 | const setAutonomy = (mode: "assisted" | "autonomous") => | |
| 389 | app.db | |
| 390 | .query("UPDATE repo SET autonomy = ?2, autonomy_by = ?3 WHERE id = ?1") | |
| 391 | .run(repo.id, mode, alice.id); | |
| 392 | const reviewRuns = (pull: Pull) => | |
| 393 | listRuns(app.db, repo.id, { issueId: pull.id }).filter((r) => r.reason === "review"); | |
| 394 | ||
| 395 | test("an autonomous repository sends it to review at any age; an assisted one leaves it lapsed", async () => { | |
| 396 | const pull = await greenPull("ancient"); | |
| 397 | setAutonomy("assisted"); | |
| 398 | expect(await sendToReview(app, fresh(), pull, later)).toBe("lapsed"); | |
| 399 | expect(reviewRuns(pull)).toHaveLength(0); | |
| 400 | setAutonomy("autonomous"); | |
| 401 | expect(await sendToReview(app, fresh(), pull, later)).toBe("sent"); | |
| 402 | expect(reviewRuns(pull)).toHaveLength(1); | |
| 403 | }); | |
| 404 | ||
| 405 | test("the lapsed-review ask is the sweep's only while nothing was ever sent, and only when autonomous", async () => { | |
| 406 | const pull = await greenPull("lapsing"); | |
| 407 | const asking = () => | |
| 408 | askLinesFor(app.db, alice).some((r) => r.items.includes(`review:${pull.id}`)); | |
| 409 | // Age the green check: the ask reads the wall clock. | |
| 410 | app.db.query("UPDATE run SET created_at = ?2, finished_at = ?2 WHERE issue_id = ?1 AND kind = 'check'") | |
| 411 | .run(pull.id, new Date(Date.now() - 30 * 24 * 3_600_000).toISOString()); | |
| 412 | setAutonomy("assisted"); | |
| 413 | expect(asking()).toBe(false); // assisted repositories ask through "look", not "review" | |
| 414 | setAutonomy("autonomous"); | |
| 415 | expect(asking()).toBe(false); | |
| 416 | // Sent once and unanswered, it is lapsed with a history: it asks a person. | |
| 417 | const old = new Date(Date.now() - 30 * 24 * 3_600_000).toISOString(); | |
| 418 | expect(await sendToReview(app, fresh(), getPull(app.db, repo.id, pull.number)!, later)).toBe("sent"); | |
| 419 | for (const run of reviewRuns(pull).filter((r) => r.state === "queued")) { | |
| 420 | claimRun(app.db, run.id); | |
| 421 | finishRun(app.db, run.id, { state: "failed", summary: "reviewed: no verdict was posted" }); | |
| 422 | } | |
| 423 | app.db.query("UPDATE run SET created_at = ?2, finished_at = ?2 WHERE issue_id = ?1 AND reason = 'review'").run(pull.id, old); | |
| 424 | expect(asking()).toBe(true); | |
| 425 | }); | |
| 426 | }); |
new above
offline
New autonomous send of unreviewed lapsed pulls has no test that fails without it.