Public: anyone can read this; the crew can change it.
A finding the sentinel files is a builder's to take, never parked on the sentinel #3005
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/3002-a-finding-the-sentinel-files-is-a-builde → main 2 commits fork 1 thread · 1 unresolved
5 files
changed +91
−26
src/crew/autonomy.ts +27 −1
| @@ -356,7 +356,9 @@ export async function driveRepo( | ||
| 356 | 356 | // operator's; at a wall nothing new starts until the reset. |
| 357 | 357 | const paced = repoPace(app, repo, now); |
| 358 | 358 | if (paced.slots === 0 && slotted.length === 0) return "waiting"; |
| 359 | const held = heldBy(app, repo, readyNow(repoGraph(app.db, repo.id, { sealed: true })), crew, active); | |
| 359 | const sorted = () => heldBy(app, repo, readyNow(repoGraph(app.db, repo.id, { sealed: true })), crew, active); | |
| 360 | let held = sorted(); | |
| 361 | if (freeParked(app, repo, by, held) > 0) held = sorted(); | |
| 360 | 362 | const ready = held.startable; |
| 361 | 363 | if (slotted.length >= paced.slots) { |
| 362 | 364 | // The builders are busy; the planner can still refill the map, when |
| @@ -415,6 +417,30 @@ export async function driveRepo( | ||
| 415 | 417 | return "idle"; |
| 416 | 418 | } |
| 417 | 419 | |
| 420 | /** | |
| 421 | * A ready issue assigned to a planner or a reviewer with no run on it is | |
| 422 | * parked on a crew member that never builds it — a sentinel's finding, a | |
| 423 | * planner's pass that ended (#3002). Its assignee is cleared so a builder | |
| 424 | * may start it this tick. Returns how many were freed. | |
| 425 | */ | |
| 426 | export function freeParked(app: App, repo: Repo, by: User, held: Held): number { | |
| 427 | let freed = 0; | |
| 428 | for (const issue of held.by.planner) { | |
| 429 | const cleared = assignIssue(app, by, repo, issue, null); | |
| 430 | if (!cleared.ok) { | |
| 431 | log.warn("could not free an issue parked on a non-builder", { | |
| 432 | repo: `${repo.ownerName}/${repo.name}`, | |
| 433 | issue: issue.number, | |
| 434 | error: cleared.error, | |
| 435 | }); | |
| 436 | continue; | |
| 437 | } | |
| 438 | log.info("freed", { repo: `${repo.ownerName}/${repo.name}`, issue: issue.number }); | |
| 439 | freed++; | |
| 440 | } | |
| 441 | return freed; | |
| 442 | } | |
| 443 | ||
| 418 | 444 | /** |
| 419 | 445 | * The ready issues a builder may start now: not the direction, and held by |
| 420 | 446 | * none of `held.ts`'s filters. The tick log counts these beside the raw |
src/crew/held.ts +2 −1
| @@ -8,7 +8,8 @@ | ||
| 8 | 8 | * The filters, in the order they are asked: |
| 9 | 9 | * - `running`: a run is in flight on it. |
| 10 | 10 | * - `open_pull`: an open pull already proposes it (`direct.ts`). |
| 11 | * - `planner`: the planner or a reviewer holds it, not a builder. | |
| 11 | * - `planner`: the planner or a reviewer holds it, not a builder. The tick | |
| 12 | * frees these (`freeParked` in `autonomy.ts`): neither ever builds it. | |
| 12 | 13 | * - `with_planner`: its limb was handed to the planner by a person and the |
| 13 | 14 | * planner has not answered (`core/limb_replan.ts`). |
| 14 | 15 | * - `done_on_main`: the crew found it done on the base branch and waits for |
src/crew/sentinel.ts +10 −17
| @@ -23,7 +23,7 @@ import { | ||
| 23 | 23 | } from "../core/capacity.ts"; |
| 24 | 24 | import { decisionsSince } from "../core/decisions.ts"; |
| 25 | 25 | import { getAgentByName } from "../core/agents.ts"; |
| 26 | import { getIssueById, getLabel, setAssignee, setIssueLabels as writeLabels, type Issue } from "../core/issues.ts"; | |
| 26 | import { getIssueById, getLabel, setIssueLabels as writeLabels, type Issue } from "../core/issues.ts"; | |
| 27 | 27 | import { getRepo, type Repo } from "../core/repos.ts"; |
| 28 | 28 | import { leaseLapsed, listRunning } from "../core/runs.ts"; |
| 29 | 29 | import { migrations } from "../core/schema.ts"; |
| @@ -58,7 +58,7 @@ export interface Finding { | ||
| 58 | 58 | readonly evidence: readonly string[]; |
| 59 | 59 | readonly pillar: string; |
| 60 | 60 | readonly lookFor: string; |
| 61 | /** A budget breach: filed at once, labelled, handed to the sentinel's own run. */ | |
| 61 | /** A budget breach: filed at once, labelled, left unassigned for a builder (#3002). */ | |
| 62 | 62 | readonly urgent?: true; |
| 63 | 63 | /** What a person or a run would do, when it is mechanical and reversible. */ |
| 64 | 64 | readonly fix?: string; |
| @@ -553,12 +553,13 @@ const bodyOf = (f: Finding, now: string): string => | ||
| 553 | 553 | ].join("\n"); |
| 554 | 554 | |
| 555 | 555 | /** |
| 556 | * A breach is crew work at once: the P0 Trust label, and the sentinel | |
| 557 | * assigned, whose run the dispatcher queues on the assignment event. Each | |
| 558 | * step may fail alone (no such label, the actor is a person): the issue is | |
| 559 | * filed either way, and the failure is said. | |
| 556 | * A breach is crew work at once: the P0 Trust label, and no assignee, so a | |
| 557 | * builder may start it on the next tick (#3002). The sentinel never builds, | |
| 558 | * so it never assigns itself: a finding parked on it is held from every | |
| 559 | * builder. The label may fail alone (no such label): the issue is filed | |
| 560 | * either way, and the failure is said. | |
| 560 | 561 | */ |
| 561 | function handOver(app: App, repo: Repo, actor: User, issue: Issue): void { | |
| 562 | function labelBreach(app: App, repo: Repo, actor: User, issue: Issue): void { | |
| 562 | 563 | try { |
| 563 | 564 | // The forge acts on its own finding: no access check stands between it |
| 564 | 565 | // and its label, as none stands between it and the issue it just filed. |
| @@ -573,16 +574,8 @@ function handOver(app: App, repo: Repo, actor: User, issue: Issue): void { | ||
| 573 | 574 | payload: { number: issue.number, labels: [label.name] }, |
| 574 | 575 | }); |
| 575 | 576 | } |
| 576 | if (actor.kind !== "agent") return; | |
| 577 | if (setAssignee(app.db, issue.id, actor.id) === undefined) return; | |
| 578 | app.bus.append({ | |
| 579 | kind: "issue.assigned", | |
| 580 | repoId: repo.id, | |
| 581 | actorId: actor.id, | |
| 582 | payload: { number: issue.number, assignee: actor.username, agent: true }, | |
| 583 | }); | |
| 584 | 577 | } catch (error) { |
| 585 | log.error("sentinel could not hand over", { issue: issue.number, error: String(error) }); | |
| 578 | log.error("sentinel could not label", { issue: issue.number, error: String(error) }); | |
| 586 | 579 | } |
| 587 | 580 | } |
| 588 | 581 | |
| @@ -790,7 +783,7 @@ export function hunt( | ||
| 790 | 783 | } |
| 791 | 784 | issueId = made.value.id; |
| 792 | 785 | filed++; |
| 793 | if (f.urgent && f.kept === undefined) handOver(app, repo, actor, made.value); | |
| 786 | if (f.urgent && f.kept === undefined) labelBreach(app, repo, actor, made.value); | |
| 794 | 787 | } |
| 795 | 788 | app.db |
| 796 | 789 | .query("INSERT INTO sentinel_finding (key, issue_id, evidence, first_seen, last_seen, incident) VALUES (?1, ?2, ?3, ?4, ?4, ?5)") |
test/crew-held.test.ts +48 −2
| @@ -16,7 +16,7 @@ import { addComment, assignIssue, openIssue } from "../src/actions/issues.ts"; | ||
| 16 | 16 | import { openPull } from "../src/actions/pulls.ts"; |
| 17 | 17 | import { commitToBranch, createRepo, updateRepoSettings } from "../src/actions/repos.ts"; |
| 18 | 18 | import { getAgentByName, setCrewRole } from "../src/core/agents.ts"; |
| 19 | import { getIssueById, type Issue } from "../src/core/issues.ts"; | |
| 19 | import { getIssueById, listIssues, type Issue } from "../src/core/issues.ts"; | |
| 20 | 20 | import { getRepoById, type Repo } from "../src/core/repos.ts"; |
| 21 | 21 | import { DONE_NOTE } from "../src/core/done_on_main.ts"; |
| 22 | 22 | import { claimRun, createRun, finishRun } from "../src/core/runs.ts"; |
| @@ -27,7 +27,7 @@ import { taskPrompt } from "../src/crew/brief.ts"; | ||
| 27 | 27 | import { ISSUE_RUN_CAP } from "../src/crew/guard.ts"; |
| 28 | 28 | import { heldCounts, heldNow, heldWords, replannable, stuckOf } from "../src/crew/held.ts"; |
| 29 | 29 | import { PARK_NOTE } from "../src/crew/parks.ts"; |
| 30 | import { keyPart } from "../src/crew/sentinel.ts"; | |
| 30 | import { hunt, keyPart } from "../src/crew/sentinel.ts"; | |
| 31 | 31 | import { HELD_TTL_MS, readHeld } from "../src/flow/held.ts"; |
| 32 | 32 | import { serve, type Servers } from "../src/http/server.ts"; |
| 33 | 33 | import { flowLines, heldLines } from "../src/tui/screens/flow.ts"; |
| @@ -204,6 +204,52 @@ describe("the tick says why each ready issue cannot start", () => { | ||
| 204 | 204 | expect(stuckOf(held)).toBeUndefined(); |
| 205 | 205 | }); |
| 206 | 206 | |
| 207 | test("a sentinel finding is a builder's to start on the next tick, never parked on the sentinel (#3002)", async () => { | |
| 208 | const { repo, mason } = await crewed(app, alice, "watched"); | |
| 209 | const fresh = () => getRepoById(app.db, repo.id)!; | |
| 210 | const made = await createAgent(app, alice, { username: "sentinel" }); | |
| 211 | if (!made.ok) throw new Error(String(made.error)); | |
| 212 | expect(addToCrew(app, alice, fresh(), "sentinel").ok).toBe(true); | |
| 213 | const sentinel = getUserByName(app.db, "sentinel")!; | |
| 214 | setCrewRole(app.db, repo.id, sentinel.id, "reviewer"); | |
| 215 | ||
| 216 | // A lane past its budget: the sentinel files it at once, unassigned. | |
| 217 | const now = Date.now(); | |
| 218 | const span = app.db | |
| 219 | .query<{ id: number }, [number]>( | |
| 220 | "INSERT INTO degraded_span (cause, sentence, href, opened_at) VALUES ('stuck:held-test', 'One lane is past its budget.', '/x', ?1) RETURNING id", | |
| 221 | ) | |
| 222 | .get(now - 2 * 60_000)!; | |
| 223 | try { | |
| 224 | hunt(app, `alice/${repo.name}`, now); | |
| 225 | } finally { | |
| 226 | app.db.query("UPDATE degraded_span SET closed_at = ?2 WHERE id = ?1").run(span.id, now); | |
| 227 | } | |
| 228 | const filed = listIssues(app.db, repo.id, { state: "open" } as never).filter((i) => i.title.startsWith("Sentinel: ")); | |
| 229 | const finding = filed.find((i) => i.title.includes("stuck:held-test"))!; | |
| 230 | expect(finding).toBeDefined(); | |
| 231 | expect(finding.assigneeId).toBeNull(); | |
| 232 | // Only the finding is left to start, so the tick's pick is it. | |
| 233 | for (const other of filed) if (other.id !== finding.id) app.db.query("UPDATE issue SET state = 'closed' WHERE id = ?1").run(other.id); | |
| 234 | expect(heldNow(app, fresh()).startable.map((r) => r.issue.number)).toContain(finding.number); | |
| 235 | ||
| 236 | // One parked on the sentinel as it used to be: the tick frees it, and a builder starts it. | |
| 237 | expect(assignIssue(app, alice, fresh(), finding, sentinel).ok).toBe(true); | |
| 238 | expect(heldNow(app, fresh()).by.planner.map((i) => i.number)).toEqual([finding.number]); | |
| 239 | expect(await driveRepo(app, fresh())).toBe("picked"); | |
| 240 | expect(getIssueById(app.db, finding.id)!.assigneeId).toBe(mason.id); | |
| 241 | }); | |
| 242 | ||
| 243 | test("a planner or a reviewer with a run in flight keeps its issue", async () => { | |
| 244 | const { repo, atlas } = await crewed(app, alice, "in-flight"); | |
| 245 | const fresh = () => getRepoById(app.db, repo.id)!; | |
| 246 | const held = issue(fresh(), "The planner's, running"); | |
| 247 | expect(assignIssue(app, alice, fresh(), held, atlas).ok).toBe(true); | |
| 248 | createRun(app.db, { repoId: repo.id, kind: "agent", issueId: held.id, actorId: atlas.id, reason: "assigned" }); | |
| 249 | await driveRepo(app, fresh()); | |
| 250 | expect(getIssueById(app.db, held.id)!.assigneeId).toBe(atlas.id); | |
| 251 | }); | |
| 252 | ||
| 207 | 253 | test("an issue found done on main is held as done_on_main, never re-planned, until the crew parks it", async () => { |
| 208 | 254 | const { repo, mason } = await crewed(app, alice, "closing"); |
| 209 | 255 | const fresh = () => getRepoById(app.db, repo.id)!; |
test/sentinel.test.ts +4 −5
| @@ -696,7 +696,7 @@ describe("a budget breach is crew work at once", () => { | ||
| 696 | 696 | await rm(bdir, { recursive: true, force: true }); |
| 697 | 697 | }); |
| 698 | 698 | |
| 699 | test("an open lane breach is filed once, labelled, and handed to the sentinel's run", () => { | |
| 699 | test("an open lane breach is filed once, labelled, and left unassigned for a builder", () => { | |
| 700 | 700 | bapp.db |
| 701 | 701 | .query( |
| 702 | 702 | "INSERT INTO degraded_span (cause, sentence, href, opened_at) VALUES ('queue:check', '22 checks wait behind one lane; the oldest since 04:13; the lane runs #1212 for 31 min (limit 60).', '/settings/capacity', ?1)", |
| @@ -713,11 +713,10 @@ describe("a budget breach is crew work at once", () => { | ||
| 713 | 713 | const open = listIssues(bapp.db, brepo.id, { state: "open" } as never); |
| 714 | 714 | expect(open.length).toBe(1); |
| 715 | 715 | const issue = open[0]!; |
| 716 | expect(issue.assigneeId).toBe(sentinelId); | |
| 716 | // The sentinel never builds, so it never assigns itself (#3002). | |
| 717 | expect(issue.assigneeId).toBeNull(); | |
| 717 | 718 | expect(labelsFor(bapp.db, issue.id).map((l) => l.name)).toEqual(["P0 Trust"]); |
| 718 | const assigned = listEvents(bapp.db, { kind: "issue.assigned" }); | |
| 719 | expect(assigned.length).toBe(1); | |
| 720 | expect(assigned[0]!.payload["agent"]).toBe(true); | |
| 719 | expect(listEvents(bapp.db, { kind: "issue.assigned" }).length).toBe(0); | |
| 721 | 720 | }); |
| 722 | 721 | |
| 723 | 722 | test("a breach brings a hunt within a tick, inside the signal gap", () => { |
new above
offline
Each freed issue should leave a trace the owner can see (an event or issue comment), not only a log line, since this silently changes who holds the issue.