Public: anyone can read this; the crew can change it.
A planner's "one more round" comment on a handed-over crew pull gives it back to its builder #6596
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6585-a-planner-s-one-more-round-comment-on-a → main 3 commits fork 3 threads · 3 unresolved
4 files
changed +117
−7
README.md +2 −2
| @@ -40,9 +40,9 @@ when). The terminal's palette has the same as "Crew hand-overs", a read-only | ||
| 40 | 40 | list of the same rows (repo#number, title, reasons; it does not open the |
| 41 | 41 | issue — use the link on the page). Both are a forge admin's only. |
| 42 | 42 | |
| 43 | Home opens with "Since you were here · 2 days": the pulls that landed in public repositories since you last loaded `/` (1 hour to 7 days back, a day on a first visit); the terminal's home shows the same lines. See `docs/features/since-you-were-here.md`. | |
| 43 | A hand-over card on `/asks` (an agent's pull the reviewer stopped on after three rounds) has "Give back to the crew": one press assigns the pull, and the open issue it closes if a person holds it, to the agent that opened it, writes "Given back to the crew by @who", and the card leaves; the next tick starts its run. It is for a writer of the repository, and a pull a person opened, or by an agent gone from the crew, gets no button. In the terminal, `b` on a handed-over row of the On you pane does the same (`src/actions/give_back.ts`). The repository's planner can do it by comment: one that starts "Planner, one more round" on a handed-over crew pull emits the assignment to its builder, which starts the run (the pull is already the builder's, so nothing changes hands). It is the planner's alone; a reader's, the reviewer's or any other agent's does nothing. It is an agent's word, not a director's, so it does not end the hand-over: the card stays on `/asks` and the pull still counts as handed over during the round. | |
| 44 | 44 | |
| 45 | A hand-over card on `/asks` (an agent's pull the reviewer stopped on after three rounds) has "Give back to the crew": one press assigns the pull, and the open issue it closes if a person holds it, to the agent that opened it, writes "Given back to the crew by @who", and the card leaves; the next tick starts its run. It is for a writer of the repository, and a pull a person opened, or by an agent gone from the crew, gets no button. In the terminal, `b` on a handed-over row of the On you pane does the same (`src/actions/give_back.ts`). | |
| 45 | Home opens with "Since you were here · 2 days": the pulls that landed in public repositories since you last loaded `/` (1 hour to 7 days back, a day on a first visit); the terminal's home shows the same lines. See `docs/features/since-you-were-here.md`. | |
| 46 | 46 | |
| 47 | 47 | A pull the merge queue holds as red alone (red on its own merge at its current head; cleared by a new head or a moved main) says so under its checks line: "red alone at <time>, waiting for a new head or a new main", on the pull page, its live checks line and the studio change step, and in the checks row of the pull's terminal screen (`redMark` in `src/checks/queue.ts`). Nothing is drawn when it is not held. |
| 48 | 48 |
src/actions/give_back.ts +40 −2
| @@ -9,8 +9,8 @@ | ||
| 9 | 9 | import type { Database } from "bun:sqlite"; |
| 10 | 10 | import type { App } from "../app.ts"; |
| 11 | 11 | import { assignIssue, addComment } from "./issues.ts"; |
| 12 | import { isCrew } from "../core/agents.ts"; | |
| 13 | import { getIssue, getIssueById, type Issue } from "../core/issues.ts"; | |
| 12 | import { crewRole, isCrew } from "../core/agents.ts"; | |
| 13 | import { getIssue, getIssueById, setAssignee, type Issue } from "../core/issues.ts"; | |
| 14 | 14 | import { closesFromBody } from "../core/pulls.ts"; |
| 15 | 15 | import { canWrite, getRepoById, type Repo } from "../core/repos.ts"; |
| 16 | 16 | import { getUserById, type User } from "../core/users.ts"; |
| @@ -109,3 +109,41 @@ export function giveBack( | ||
| 109 | 109 | } |
| 110 | 110 | |
| 111 | 111 | class Refused extends Error {} |
| 112 | ||
| 113 | /** What a planner's comment starts with to send a handed-over pull one more round. */ | |
| 114 | export const ANOTHER_ROUND = "Planner, one more round"; | |
| 115 | ||
| 116 | /** | |
| 117 | * The planner's "one more round" comment on a handed-over crew pull gives | |
| 118 | * it back to its builder, as the button does. The planner cannot assign an | |
| 119 | * agent (that is a person's to spend), so the rule is this one narrow | |
| 120 | * door: the repository's planner, an open handed-over pull, a comment that | |
| 121 | * starts with the phrase. The assignment event is what starts the run. | |
| 122 | * An agent's word is not a director's: it does not end the hand-over, so the | |
| 123 | * card stays on /asks and the pull counts as handed over during the round. | |
| 124 | * Returns whether the pull was given back. | |
| 125 | */ | |
| 126 | export function planAnotherRound( | |
| 127 | app: App, | |
| 128 | actor: User, | |
| 129 | repo: Repo, | |
| 130 | pull: Issue, | |
| 131 | body: unknown, | |
| 132 | ): boolean { | |
| 133 | if (typeof body !== "string" || !body.startsWith(ANOTHER_ROUND)) return false; | |
| 134 | if (actor.kind !== "agent" || crewRole(app.db, repo.id, actor.id) !== "planner") return false; | |
| 135 | if (pull.kind !== "pull" || pull.state !== "open" || repo.archivedAt !== null) return false; | |
| 136 | if (handoverOf(app.db, repo, pull.id) === undefined) return false; | |
| 137 | const builder = builderOf(app.db, repo, pull); | |
| 138 | if (builder === undefined) return false; | |
| 139 | // A crew pull starts assigned to its builder and hand-over never unassigns, | |
| 140 | // so the builder usually holds it already: the event is what starts the run. | |
| 141 | if (pull.assigneeId !== builder.id) setAssignee(app.db, pull.id, builder.id); | |
| 142 | app.bus.append({ | |
| 143 | kind: "pull.assigned", | |
| 144 | repoId: repo.id, | |
| 145 | actorId: actor.id, | |
| 146 | payload: { number: pull.number, assignee: builder.username, agent: true }, | |
| 147 | }); | |
| 148 | return true; | |
| 149 | } |
src/crew/dispatch.ts +5
| @@ -17,6 +17,7 @@ import { rmSync } from "node:fs"; | ||
| 17 | 17 | import type { App } from "../app.ts"; |
| 18 | 18 | import { transact } from "../kernel/db.ts"; |
| 19 | 19 | import { addComment } from "../actions/issues.ts"; |
| 20 | import { planAnotherRound } from "../actions/give_back.ts"; | |
| 20 | 21 | import { announceRun } from "../actions/runs.ts"; |
| 21 | 22 | import { crewRole, getAgent, type Agent } from "../core/agents.ts"; |
| 22 | 23 | import type { ForgeEvent } from "../core/events.ts"; |
| @@ -554,6 +555,10 @@ function route(app: App, event: ForgeEvent): void { | ||
| 554 | 555 | event.kind === "pull.reviewed" && |
| 555 | 556 | typeof event.payload["state"] === "string" && |
| 556 | 557 | crewRole(app.db, repo.id, actor.id) === "reviewer"; |
| 558 | if (actor.kind === "agent" && event.kind === "pull.commented") { | |
| 559 | const pull = typeof number === "number" ? getIssue(app.db, repo.id, number) : undefined; | |
| 560 | if (pull && planAnotherRound(app, actor, repo, pull, event.payload["body"])) return; | |
| 561 | } | |
| 557 | 562 | if (actor.kind === "agent" && !reviewerVerdict) return; |
| 558 | 563 | // A reader's words stay on the thread and do nothing more: on a |
| 559 | 564 | // public repository anyone may comment, and a run spends the owner's |
test/handover-give-back.test.ts +70 −3
| @@ -5,12 +5,16 @@ | ||
| 5 | 5 | */ |
| 6 | 6 | |
| 7 | 7 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; |
| 8 | import { chmodSync, writeFileSync } from "node:fs"; | |
| 9 | import { addComment } from "../src/actions/issues.ts"; | |
| 10 | import { startCrew } from "../src/crew/dispatch.ts"; | |
| 8 | 11 | import { mkdtemp, rm } from "node:fs/promises"; |
| 9 | 12 | import { tmpdir } from "node:os"; |
| 10 | 13 | import { join } from "node:path"; |
| 11 | 14 | import { createApp, type App } from "../src/app.ts"; |
| 12 | 15 | import { addCrew, createAgentRow, setCrewRole } from "../src/core/agents.ts"; |
| 13 | import { getIssueById, listComments } from "../src/core/issues.ts"; | |
| 16 | import { getIssueById, listComments, setAssignee } from "../src/core/issues.ts"; | |
| 17 | import { handoverOf } from "../src/crew/guard.ts"; | |
| 14 | 18 | import { createRepo, type Repo } from "../src/core/repos.ts"; |
| 15 | 19 | import { submitReview } from "../src/core/reviews.ts"; |
| 16 | 20 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; |
| @@ -35,6 +39,7 @@ describe("give a handed-over pull back to the crew", () => { | ||
| 35 | 39 | let builder: User; |
| 36 | 40 | let reviewer: User; |
| 37 | 41 | let repo: Repo; |
| 42 | let reader: User; | |
| 38 | 43 | let cookie = ""; |
| 39 | 44 | let readerCookie = ""; |
| 40 | 45 | let seq = 0; |
| @@ -69,7 +74,7 @@ describe("give a handed-over pull back to the crew", () => { | ||
| 69 | 74 | return { id: issueId, number }; |
| 70 | 75 | }; |
| 71 | 76 | |
| 72 | const handedOver = (author: User, title: string, body = ""): string => { | |
| 77 | const handedOver = (author: User, title: string, body = "", assigned = false): string => { | |
| 73 | 78 | const issueId = id(); |
| 74 | 79 | const number = nextNumber(); |
| 75 | 80 | app.db |
| @@ -84,6 +89,8 @@ describe("give a handed-over pull back to the crew", () => { | ||
| 84 | 89 | VALUES (?1, 'main', ?2, ?3, ?3, ?4)`, |
| 85 | 90 | ) |
| 86 | 91 | .run(issueId, `b/${++seq}`, ZERO, repo.id); |
| 92 | // As production opens a crew pull: assigned to the agent that made it. | |
| 93 | if (assigned && author.kind === "agent") setAssignee(app.db, issueId, author.id); | |
| 87 | 94 | for (let r = 0; r < 3; r++) |
| 88 | 95 | submitReview(app.db, { |
| 89 | 96 | issueId, |
| @@ -126,7 +133,7 @@ describe("give a handed-over pull back to the crew", () => { | ||
| 126 | 133 | return made.value; |
| 127 | 134 | }; |
| 128 | 135 | owner = await make("alice", false); |
| 129 | await make("reader", false); | |
| 136 | reader = await make("reader", false); | |
| 130 | 137 | builder = await make("builder", true); |
| 131 | 138 | reviewer = await make("reviewer", true); |
| 132 | 139 | promote(app.db, getUserByName(app.db, "alice")!); |
| @@ -209,6 +216,66 @@ describe("give a handed-over pull back to the crew", () => { | ||
| 209 | 216 | expect(asking(crew)).toBe(true); |
| 210 | 217 | }); |
| 211 | 218 | |
| 219 | test("the planner's one-more-round comment gives the pull back and starts a run; no one else's does", async () => { | |
| 220 | const planner = await createUser(app.db, { | |
| 221 | username: "planner", | |
| 222 | email: "[email protected]", | |
| 223 | agent: true, | |
| 224 | origin: "agent", | |
| 225 | }); | |
| 226 | if (!planner.ok) throw new Error(planner.error); | |
| 227 | createAgentRow(app.db, { userId: planner.value.id, brief: "plans." }); | |
| 228 | addCrew(app.db, repo.id, planner.value.id, owner.id); | |
| 229 | setCrewRole(app.db, repo.id, planner.value.id, "planner"); | |
| 230 | const fake = join(dataDir, "fake-claude.sh"); | |
| 231 | writeFileSync( | |
| 232 | fake, | |
| 233 | `#!/bin/sh | |
| 234 | if [ "$1" = "--version" ]; then echo "2.1.0 (fake)"; exit 0; fi | |
| 235 | IFS= read -r first | |
| 236 | echo '{"type":"result","subtype":"success","is_error":false,"result":"ok","total_cost_usd":0.01,"num_turns":1}' | |
| 237 | `, | |
| 238 | ); | |
| 239 | chmodSync(fake, 0o755); | |
| 240 | const crew = await startCrew(app, { | |
| 241 | forgeUrl: servers.url, | |
| 242 | localUrl: servers.url, | |
| 243 | binary: fake, | |
| 244 | concurrency: 1, | |
| 245 | budgetUsd: 1, | |
| 246 | runTimeoutMs: 60_000, | |
| 247 | }); | |
| 248 | const runsOn = (issueId: string): number => | |
| 249 | app.db.query<{ n: number }, [string]>("SELECT COUNT(*) AS n FROM run WHERE issue_id = ?1").get(issueId)!.n; | |
| 250 | const say = (user: User, pullId: string, body: string): void => { | |
| 251 | const pull = getIssueById(app.db, pullId)!; | |
| 252 | const said = addComment(app, user, repo, pull, body); | |
| 253 | if (!said.ok) throw new Error(String(said.error)); | |
| 254 | }; | |
| 255 | try { | |
| 256 | const one = handedOver(builder, "Planner sends it back", "", true); | |
| 257 | const two = handedOver(builder, "Someone else asks", "", true); | |
| 258 | const plain = handedOver(builder, "Planner says something else", "", true); | |
| 259 | const person = handedOver(owner, "A person's pull"); | |
| 260 | say(planner.value, one, "Planner, one more round: change only the heading."); | |
| 261 | say(reader, two, "Planner, one more round: please"); | |
| 262 | say(reviewer, two, "Planner, one more round: please"); | |
| 263 | say(planner.value, plain, "Re-scoped; nothing to add."); | |
| 264 | say(planner.value, person, "Planner, one more round: go"); | |
| 265 | for (let i = 0; i < 100 && runsOn(one) === 0; i++) await Bun.sleep(50); | |
| 266 | expect(getIssueById(app.db, one)!.assigneeId).toBe(builder.id); | |
| 267 | expect(runsOn(one)).toBeGreaterThan(0); | |
| 268 | // The planner is an agent, not a director: the hand-over stands. | |
| 269 | expect(handoverOf(app.db, repo, one)).toBeDefined(); | |
| 270 | for (const other of [two, plain, person]) { | |
| 271 | expect(runsOn(other)).toBe(0); | |
| 272 | } | |
| 273 | expect(getIssueById(app.db, person)!.assigneeId).toBeNull(); | |
| 274 | } finally { | |
| 275 | await crew.stop(); | |
| 276 | } | |
| 277 | }); | |
| 278 | ||
| 212 | 279 | test("the terminal: b gives the row back, and a reader's b does nothing", async () => { |
| 213 | 280 | const crew = handedOver(builder, "Terminal pull"); |
| 214 | 281 | const drive = async (user: User, keys: string[]) => { |
new above
offline
No-op for the normal case: the builder is already the assignee, so this returns undefined and no pull.assigned event or run follows. Emit the event even when already assigned (or unassign first), and test with a pull assigned to the builder.