Public: anyone can read this; the crew can change it.
POST …/plan answers which opened issues will not start yet and why: a hot file, a pin or an open pull #7435
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/7427-post-plan-answers-which-opened-issues-wi → main 2 commits fork 5 threads · 5 unresolved
5 files
changed +172
−5
docs/PLANNING.md +7 −2
| @@ -198,8 +198,13 @@ action as the MCP `plan` tool (`openPlan`): the same items, the same | ||
| 198 | 198 | refusals, all or nothing. An item names its `pillar`, or is `later: true` |
| 199 | 199 | with `why`; one that says neither is refused with `no_pillar` and nothing is |
| 200 | 200 | opened, so the planner reads the refusal, names the pillar or defers the |
| 201 | item, and posts again. The answer is `{ opened, ready, later, gate }`. A | |
| 202 | repository with no umbrella issues has no pillars, and plans as it always | |
| 201 | item, and posts again. The answer is `{ opened, ready, held, later, gate }`. `held` | |
| 202 | lists each opened issue that is ready but would not start now, as `{ key, number, by, | |
| 203 | detail }`: `by` is one of the map's hold names (`HOLDS` in | |
| 204 | `src/crew/held.ts`) and `detail` its sentence. The usual ones are `hot_file` | |
| 205 | (a file on the issue's `Files:` line is hot, or an admin holds it); an open | |
| 206 | pull cannot hold an issue opened a moment ago. The planner can rewrite | |
| 207 | what is held in the same run. A repository with no umbrella issues has no pillars, and plans as it always | |
| 203 | 208 | did. |
| 204 | 209 | |
| 205 | 210 | **An issue opened outside a plan.** A builder that finds its task too big |
src/actions/plan.ts +12 −1
| @@ -13,6 +13,7 @@ | ||
| 13 | 13 | import { sealedScope } from "../core/sealed.ts"; |
| 14 | 14 | import type { App } from "../app.ts"; |
| 15 | 15 | import { isCrew } from "../core/agents.ts"; |
| 16 | import { mapHeldFor, type HoldName } from "../crew/held.ts"; | |
| 16 | 17 | import { addDependency, repoGraph, readyNow } from "../core/dependencies.ts"; |
| 17 | 18 | import { findPillar, pillarsOf, type Pillar } from "../core/pillars.ts"; |
| 18 | 19 | import { |
| @@ -154,6 +155,8 @@ export interface Planned { | ||
| 154 | 155 | readonly ready: number[]; |
| 155 | 156 | /** Numbers of the issues this plan put behind the Later gate. */ |
| 156 | 157 | readonly later: number[]; |
| 158 | /** Opened issues that are ready but would not start now, with the map's words for why. */ | |
| 159 | readonly held: { key: string; number: number; by: HoldName; detail: string }[]; | |
outdated
Type |
||
| 157 | 160 | /** The Later gate's number, when the plan put something behind it. */ |
| 158 | 161 | readonly gate?: number; |
| 159 | 162 | } |
| @@ -437,9 +440,17 @@ export function openPlan( | ||
| 437 | 440 | const deferred = items |
| 438 | 441 | .filter((item) => item.later !== undefined) |
| 439 | 442 | .map((item) => number.get(item.key)!); |
| 443 | const scope = sealedScope(app.db, repo, actor); | |
| 444 | const graph = repoGraph(app.db, repo.id, scope); | |
| 445 | const { holds } = mapHeldFor(app, repo, graph, scope, new Set(opened.map((o) => o.number))); | |
outdated
This asks |
||
| 446 | const held = opened.flatMap((o) => { | |
| 447 | const hold = holds.get(o.number); | |
| 448 | return hold === undefined ? [] : [{ ...o, by: hold.by, detail: hold.detail }]; | |
| 449 | }); | |
| 440 | 450 | return ok({ |
| 441 | 451 | opened, |
| 442 | ready: readyNow(repoGraph(app.db, repo.id, sealedScope(app.db, repo, actor))).map((r) => r.issue.number), | |
| 452 | ready: readyNow(graph).map((r) => r.issue.number), | |
| 453 | held, | |
| 443 | 454 | later: deferred, |
| 444 | 455 | ...(deferred.length > 0 && gate ? { gate: gate.number } : {}), |
| 445 | 456 | }); |
src/crew/held.ts +4 −1
| @@ -284,12 +284,15 @@ export function mapHeldFor( | ||
| 284 | 284 | repo: Repo, |
| 285 | 285 | graph: Graph, |
| 286 | 286 | scope: { sealed?: true; sealedBy?: string }, |
| 287 | /** Only these issue numbers are worked out: the cost grows with them, not with the ready list. */ | |
| 288 | only?: ReadonlySet<number>, | |
| 287 | 289 | ): ReturnType<typeof mapHolds> { |
| 288 | 290 | const admit = admits(app, repo, scope); |
| 289 | 291 | const active = listActive(app.db, "agent").filter((r) => r.repoId === repo.id); |
| 292 | const ready = readyNow(graph).filter((r) => only === undefined || only.has(r.issue.number)); | |
| 290 | 293 | return mapHolds( |
| 291 | 294 | repo, |
| 292 | heldBy(app, repo, readyNow(graph), listCrewRoles(app.db, repo.id), active, admit), | |
| 295 | heldBy(app, repo, ready, listCrewRoles(app.db, repo.id), active, admit), | |
| 293 | 296 | admit, |
| 294 | 297 | ); |
| 295 | 298 | } |
src/http/mcp/tools/plan.ts +2 −1
| @@ -66,7 +66,7 @@ export const tool: Tool = { | ||
| 66 | 66 | } |
| 67 | 67 | const planned = openPlan(ctx.app, user, repo, items.value); |
| 68 | 68 | if (!planned.ok) return bad(planned.error.error, planned.error.message); |
| 69 | const { opened, ready, later, gate } = planned.value; | |
| 69 | const { opened, ready, held, later, gate } = planned.value; | |
| 70 | 70 | const waits = |
| 71 | 71 | gate === undefined |
| 72 | 72 | ? "" |
| @@ -75,6 +75,7 @@ export const tool: Tool = { | ||
| 75 | 75 | says: `Opened ${opened.length} issue${opened.length === 1 ? "" : "s"}: ${opened.map((o) => `${o.key} is #${o.number}`).join(", ")}.${waits}`, |
| 76 | 76 | opened, |
| 77 | 77 | ready, |
| 78 | held, | |
Untested: removing this line keeps every test green. Add a test that calls the MCP |
||
| 78 | 79 | later, |
| 79 | 80 | }); |
| 80 | 81 | }, |
test/plan-held.test.ts added +147
| @@ -0,0 +1,147 @@ | ||
| 1 | /** | |
| 2 | * A plan's answer says which opened issues will not start yet, and why, in | |
| 3 | * the map's own words (#7427): an issue naming a hot file is held, one that | |
| 4 | * names no hot file is not. | |
| 5 | */ | |
| 6 | ||
| 7 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 8 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 9 | import { tmpdir } from "node:os"; | |
| 10 | import { join } from "node:path"; | |
| 11 | import { createApp, type App } from "../src/app.ts"; | |
| 12 | import { updateRepoSettings } from "../src/core/repos.ts"; | |
| 13 | import { createRepo } from "../src/actions/repos.ts"; | |
| 14 | import { openPlan } from "../src/actions/plan.ts"; | |
| 15 | import { holdFile } from "../src/crew/file_holds.ts"; | |
| 16 | import { createToken } from "../src/core/tokens.ts"; | |
| 17 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 18 | import { rememberPullFiles, HOT_PULLS } from "../src/crew/hot_files.ts"; | |
| 19 | import { createUser, type User } from "../src/core/users.ts"; | |
| 20 | ||
| 21 | describe("a plan's answer lists the issues that are held", () => { | |
| 22 | let dataDir: string; | |
| 23 | let app: App; | |
| 24 | let alice: User; | |
| 25 | let servers: Servers; | |
| 26 | let rw = ""; | |
| 27 | ||
| 28 | beforeAll(async () => { | |
| 29 | dataDir = await mkdtemp(join(tmpdir(), "forge-plan-held-")); | |
| 30 | app = createApp({ dataDir, port: 0 }); | |
| 31 | const a = await createUser(app.db, { | |
| 32 | origin: "seed", | |
| 33 | trusted: true, | |
| 34 | username: "alice", | |
| 35 | email: "[email protected]", | |
| 36 | password: "a-good-passphrase", | |
| 37 | adminIfFirst: true, | |
| 38 | }); | |
| 39 | if (!a.ok) throw new Error(a.error); | |
| 40 | alice = a.value; | |
| 41 | rw = createToken(app.db, alice.id, "rw", "rw").token; | |
| 42 | servers = await serve(app); | |
| 43 | }); | |
| 44 | ||
| 45 | afterAll(async () => { | |
| 46 | servers.stop(); | |
| 47 | app.close(); | |
outdated
Add the pinned case beside this one: an admin hold on a file ( |
||
| 48 | await rm(dataDir, { recursive: true, force: true }); | |
| 49 | }); | |
| 50 | ||
| 51 | test("only the item naming a hot file is held, with the map's sentence", async () => { | |
| 52 | const made = await createRepo(app, alice, { name: "held", visibility: "private" }); | |
| 53 | if (!made.ok) throw new Error(String(made.error)); | |
| 54 | const repo = made.value; | |
| 55 | rememberPullFiles( | |
| 56 | repo.id, | |
| 57 | Array.from({ length: HOT_PULLS }, (_, i) => ({ number: 100 + i, files: ["src/busy.ts"] })), | |
| 58 | ); | |
| 59 | const planned = openPlan(app, alice, repo, [ | |
| 60 | { key: "hot", title: "Rework src/busy.ts", body: "Files: src/busy.ts", blockedBy: [] }, | |
| 61 | { key: "cold", title: "Rework src/quiet.ts", body: "Files: src/quiet.ts", blockedBy: [] }, | |
| 62 | ]); | |
| 63 | if (!planned.ok) throw new Error(planned.error.message); | |
| 64 | const { opened, held, ready } = planned.value; | |
| 65 | const number = (key: string) => opened.find((o) => o.key === key)!.number; | |
| 66 | expect(held).toEqual([ | |
| 67 | { | |
| 68 | key: "hot", | |
| 69 | number: number("hot"), | |
| 70 | by: "hot_file", | |
| 71 | detail: `src/busy.ts is in ${HOT_PULLS} open pulls`, | |
| 72 | }, | |
| 73 | ]); | |
| 74 | expect(ready).toContain(number("cold")); | |
| 75 | }); | |
| 76 | ||
| 77 | const heat = (repoId: string) => | |
| 78 | rememberPullFiles( | |
| 79 | repoId, | |
| 80 | Array.from({ length: HOT_PULLS }, (_, i) => ({ number: 100 + i, files: ["src/busy.ts"] })), | |
| 81 | ); | |
| 82 | const items = [ | |
| 83 | { key: "hot", title: "Rework src/busy.ts", body: "Files: src/busy.ts" }, | |
| 84 | { key: "cold", title: "Rework src/quiet.ts", body: "Files: src/quiet.ts" }, | |
| 85 | ]; | |
| 86 | const quiet = async (name: string) => { | |
| 87 | const made = await createRepo(app, alice, { name, visibility: "private" }); | |
| 88 | if (!made.ok) throw new Error(String(made.error)); | |
| 89 | return updateRepoSettings(app.db, made.value, { plansNeedNod: false }); | |
| 90 | }; | |
| 91 | ||
| 92 | test("a file an admin holds holds the item naming it", async () => { | |
| 93 | const repo = await quiet("pinned"); | |
| 94 | expect(holdFile(app.db, repo.id, "src/pinned.ts", alice.id).ok).toBe(true); | |
| 95 | const planned = openPlan(app, alice, repo, [ | |
| 96 | { key: "pin", title: "Rework src/pinned.ts", body: "", blockedBy: [] }, | |
| 97 | { key: "free", title: "Rework src/free.ts", body: "", blockedBy: [] }, | |
| 98 | ]); | |
| 99 | if (!planned.ok) throw new Error(planned.error.message); | |
| 100 | expect(planned.value.held).toEqual([ | |
| 101 | { | |
| 102 | key: "pin", | |
| 103 | number: planned.value.opened[0]!.number, | |
| 104 | by: "hot_file", | |
| 105 | detail: "src/pinned.ts is held by an admin", | |
| 106 | }, | |
| 107 | ]); | |
| 108 | }); | |
| 109 | ||
| 110 | test("POST …/plan answers with held", async () => { | |
| 111 | const repo = await quiet("route"); | |
| 112 | heat(repo.id); | |
| 113 | const res = await fetch(`${servers.url}/api/v1/repos/alice/route/plan`, { | |
| 114 | method: "POST", | |
| 115 | headers: { authorization: `Bearer ${rw}`, "content-type": "application/json" }, | |
| 116 | body: JSON.stringify({ issues: items }), | |
| 117 | }); | |
| 118 | expect(res.status).toBe(201); | |
| 119 | const body = (await res.json()) as any; | |
| 120 | expect(body.held.map((h: any) => [h.key, h.by, h.detail])).toEqual([ | |
| 121 | ["hot", "hot_file", `src/busy.ts is in ${HOT_PULLS} open pulls`], | |
| 122 | ]); | |
| 123 | }); | |
| 124 | ||
| 125 | test("the MCP plan tool answers with held", async () => { | |
| 126 | const repo = await quiet("tool"); | |
| 127 | heat(repo.id); | |
| 128 | const res = await fetch(`${servers.url}/mcp`, { | |
| 129 | method: "POST", | |
| 130 | headers: { | |
| 131 | "content-type": "application/json", | |
| 132 | accept: "application/json, text/event-stream", | |
| 133 | authorization: `Bearer ${rw}`, | |
| 134 | }, | |
| 135 | body: JSON.stringify({ | |
| 136 | jsonrpc: "2.0", | |
| 137 | id: 1, | |
| 138 | method: "tools/call", | |
| 139 | params: { name: "plan", arguments: { app: "alice/tool", issues: items } }, | |
| 140 | }), | |
| 141 | }); | |
| 142 | const out = JSON.parse(((await res.json()) as any).result.content[0].text); | |
| 143 | expect(out.held.map((h: any) => [h.key, h.by, h.detail])).toEqual([ | |
| 144 | ["hot", "hot_file", `src/busy.ts is in ${HOT_PULLS} open pulls`], | |
| 145 | ]); | |
| 146 | }); | |
| 147 | }); |
new above
offline
heldcarries any hold inHOLDS(e.g.flow_only,red_fix), not just hot, pinned or open-pull. Name the field's real range and wrap the line to the file's width.