Public: anyone can read this; the crew can change it.
When the decider sees no point in another try, the park reason names the failing test and what each try changed #6679
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6576-when-the-decider-sees-no-point-in-anothe → main 1 commit fork 2 threads · 2 unresolved
3 files
changed +135
−1
src/crew/dispatch.ts +11 −1
| @@ -36,6 +36,7 @@ import { accountActive } from "../core/accounts.ts"; | ||
| 36 | 36 | import { runningLimitSentence, runningRuns } from "../settings/quotas.ts"; |
| 37 | 37 | import { appendRunLog } from "../runs/log.ts"; |
| 38 | 38 | import { decideAndRecord } from "./ledger.ts"; |
| 39 | import { triesChanged } from "./tries.ts"; | |
| 39 | 40 | import { byDials } from "./team_share.ts"; |
| 40 | 41 | import { ARCHIVED_WHY, getRepoById, type Repo } from "../core/repos.ts"; |
| 41 | 42 | import { listReviews } from "../core/reviews.ts"; |
| @@ -937,9 +938,18 @@ export async function answerFailedCheck( | ||
| 937 | 938 | // The decider's stop is the crew's stop: parked, never waiting, in a |
| 938 | 939 | // repository on its own. |
| 939 | 940 | const pull = getPull(app.db, repo.id, issue.number); |
| 941 | const tries = | |
| 942 | pull && judged.cause !== "environment" | |
| 943 | ? await triesChanged(app, repo, pull, failed.since).catch(() => []) | |
| 944 | : []; | |
| 940 | 945 | if ( |
| 941 | 946 | pull && |
| 942 | parkPull(app, repo, pull, parkedReason(judged, failed.kind, head)) | |
| 947 | parkPull( | |
| 948 | app, | |
| 949 | repo, | |
| 950 | pull, | |
| 951 | [parkedReason(judged, failed.kind, head), ...tries].join("\n"), | |
| 952 | ) | |
| 943 | 953 | ) |
| 944 | 954 | return "parked"; |
| 945 | 955 | log.info("check failed; waiting for a person", { issue: issue.number }); |
src/crew/tries.ts added +57
| @@ -0,0 +1,57 @@ | ||
| 1 | /** | |
| 2 | * What the tries of a pull changed, in the words a park reason can carry. | |
| 3 | * A try is a head the check went red on; its files are the ones it changed | |
| 4 | * since the try before it (the first, since the base). A planner reading | |
| 5 | * "the decider saw no point in another try" learns nothing from it alone; | |
| 6 | * with the failing test and these lines it can see whether the tries kept | |
| 7 | * circling the same files. | |
| 8 | */ | |
| 9 | ||
| 10 | import type { App } from "../app.ts"; | |
| 11 | import { pullDir, type Pull } from "../core/pulls.ts"; | |
| 12 | import type { Repo } from "../core/repos.ts"; | |
| 13 | import { listRuns } from "../core/runs.ts"; | |
| 14 | import { runGitCapture } from "../git/env.ts"; | |
| 15 | ||
| 16 | const OID = /^[0-9a-f]{40,64}$/; | |
| 17 | /** Tries told, newest kept: the reason is three lines with its first. */ | |
| 18 | const TRIES_TOLD = 2; | |
| 19 | const FILES_PER_TRY = 4; | |
| 20 | const LINE_MAX = 160; | |
| 21 | ||
| 22 | /** `try 2: src/a.ts, src/b.ts (+3 more)`, one per line, oldest first; empty when none can be read. */ | |
| 23 | export async function triesChanged( | |
| 24 | app: App, | |
| 25 | repo: Repo, | |
| 26 | pull: Pull, | |
| 27 | since: string, | |
| 28 | ): Promise<string[]> { | |
| 29 | const heads: string[] = []; | |
| 30 | for (const run of listRuns(app.db, repo.id, { issueId: pull.id, limit: 50 }) | |
| 31 | .reverse()) { | |
| 32 | if (run.kind !== "check" && run.kind !== "deploy") continue; | |
| 33 | if (run.state !== "failed" || run.createdAt <= since) continue; | |
| 34 | if (!OID.test(run.commitOid) || heads.includes(run.commitOid)) continue; | |
| 35 | heads.push(run.commitOid); | |
| 36 | } | |
| 37 | if (heads.length === 0) return []; | |
| 38 | const dir = await pullDir(app.db, app.reposRoot, repo, pull); | |
| 39 | const lines: string[] = []; | |
| 40 | let before = pull.baseOid; | |
| 41 | for (const [n, head] of heads.entries()) { | |
| 42 | const out = OID.test(before) | |
| 43 | ? await runGitCapture( | |
| 44 | ["diff", "--name-only", "-z", "--no-renames", `${before}...${head}`, "--"], | |
| 45 | { cwd: dir }, | |
| 46 | ) | |
| 47 | : undefined; | |
| 48 | before = head; | |
| 49 | if (out === undefined || out.code !== 0) continue; | |
| 50 | const files = out.stdout.split("\0").filter((f) => f !== ""); | |
| 51 | const shown = files.slice(0, FILES_PER_TRY).join(", "); | |
| 52 | const more = files.length > FILES_PER_TRY ? ` (+${files.length - FILES_PER_TRY} more)` : ""; | |
The file names come from the pushed branch and are not sanitised. A name containing a newline or markdown breaks the three-line bound. Replace control characters with |
||
| 53 | const line = `try ${n + 1}: ${files.length === 0 ? "no files changed" : shown + more}`; | |
| 54 | lines.push(line.length > LINE_MAX ? `${line.slice(0, LINE_MAX - 1)}…` : line); | |
| 55 | } | |
| 56 | return lines.slice(-TRIES_TOLD); | |
| 57 | } |
test/ledger.test.ts +67
| @@ -426,6 +426,73 @@ describe("the decision ledger", () => { | ||
| 426 | 426 | } |
| 427 | 427 | }); |
| 428 | 428 | |
| 429 | test("a park after two red tries names the failing test and the files each try changed", async () => { | |
| 430 | const { listComments } = await import("../src/core/issues.ts"); | |
| 431 | const { getAgentByName } = await import("../src/core/agents.ts"); | |
| 432 | const scoutAgent = getAgentByName(app.db, "scout")!; | |
| 433 | const first = await commitToBranch(app, alice, repo, { | |
| 434 | branch: "feature/twice", | |
| 435 | base: "main", | |
| 436 | message: "First try", | |
| 437 | files: [{ path: "first.ts", content: "export const a = 1;\n" }], | |
| 438 | }); | |
| 439 | if (!first.ok) throw new Error(JSON.stringify(first.error)); | |
| 440 | const opened = await openPull(app, scout, repo, { | |
| 441 | title: "Twice", | |
| 442 | baseBranch: "main", | |
| 443 | headBranch: "feature/twice", | |
| 444 | }); | |
| 445 | if (!opened.ok) throw new Error(String(opened.error)); | |
| 446 | const red = (oid: string) => { | |
| 447 | const r = createRun(app.db, { | |
| 448 | repoId: repo.id, | |
| 449 | kind: "check", | |
| 450 | issueId: opened.value.id, | |
| 451 | env: `pull-${opened.value.number}`, | |
| 452 | commitOid: oid, | |
| 453 | reason: "pull", | |
| 454 | }); | |
| 455 | claimRun(app.db, r.id); | |
| 456 | return finishRun(app.db, r.id, { | |
| 457 | state: "failed", | |
| 458 | summary: 'check failed: 1 of 9 tests — "adds up the cart"', | |
| 459 | checkDetail: { passed: 8, failed: 1, names: ["adds up the cart"] }, | |
| 460 | })!; | |
| 461 | }; | |
| 462 | red(opened.value.headOid); | |
| 463 | const second = await commitToBranch(app, alice, repo, { | |
| 464 | branch: "feature/twice", | |
| 465 | message: "Second try", | |
| 466 | files: [{ path: "second.ts", content: "export const b = 2;\n" }], | |
| 467 | }); | |
| 468 | if (!second.ok) throw new Error(JSON.stringify(second.error)); | |
| 469 | const moved = getPull(app.db, repo.id, opened.value.number)!; | |
| 470 | expect(moved.headOid).not.toBe(opened.value.headOid); | |
| 471 | const last = red(moved.headOid); | |
| 472 | script["retry"] = "stop"; | |
| 473 | script["cause"] = "change"; | |
| 474 | try { | |
| 475 | expect( | |
| 476 | await answerFailedCheck(app, fresh(), moved, scoutAgent, { | |
| 477 | runId: last.id, | |
| 478 | kind: "check", | |
| 479 | commitOid: moved.headOid, | |
| 480 | env: `pull-${moved.number}`, | |
| 481 | since: moved.createdAt, | |
| 482 | }), | |
| 483 | ).toBe("parked"); | |
| 484 | const note = listComments(app.db, moved.id) | |
| 485 | .map((c) => c.body) | |
| 486 | .find((b) => b.includes("saw no point in another try"))!; | |
| 487 | expect(note.split("\n").length).toBeLessThanOrEqual(3); | |
| 488 | expect(note).toContain('"adds up the cart"'); | |
| 489 | expect(note).toContain("try 1: first.ts"); | |
| 490 | expect(note).toContain("try 2: second.ts"); | |
| 491 | } finally { | |
| 492 | delete script["cause"]; | |
| 493 | } | |
| 494 | }); | |
| 495 | ||
| 429 | 496 | test("a stop the decider is not sure of (p < 0.7) is a feedback run; a sure one parks, and the pull closes as parked", async () => { |
| 430 | 497 | const { getAgentByName } = await import("../src/core/agents.ts"); |
| 431 | 498 | const scoutAgent = getAgentByName(app.db, "scout")!; |
new above
offline
The
.catch(() => [])swallows every failure with no trace. Log a warning with the issue number so a missing try list can be explained.