Public: anyone can read this; the crew can change it.
An issue labelled `polish` is capped small: past its cap the run stops and says what is left in one line #8863
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/8852-an-issue-labelled-polish-is-capped-small → main 5 commits fork 3 threads · 3 unresolved
8 files
changed +229
−5
docs/CREW.md +1 −1
| @@ -291,7 +291,7 @@ at most `slots.smallShare` percent of the builder slots (default 30, rounded up: | ||
| 291 | 291 | skipped, not waited on, unless the owner ranked it. The tick line's `small: 1/2`, the |
| 292 | 292 | Flow command's `small 1/2`, the app's row in Today on `/boards` (`small |
| 293 | 293 | issues: 1 of 2 slots`) and the studio's crew panel show small issues in |
| 294 | flight of what they may hold. When nothing is startable while builder slots are free and some of the | |
| 294 | flight of what they may hold. An issue labelled `polish` is a small issue too, and its run is stopped past `polish.capMinutes` (default 60) with one comment saying what is left (`crew/polish_cap.ts`, `docs/features/polish-cap.md`). When nothing is startable while builder slots are free and some of the | |
| 295 | 295 | held are stalled, capped or parked twice, the planner is owed one pass with |
| 296 | 296 | that breakdown in its brief, to re-plan them into smaller, fresh issues; a |
| 297 | 297 | full plate and its own unanswered decline do not hold that pass, and the |
docs/features/bets.md +1 −1
| @@ -5,7 +5,7 @@ A bet is a name, an appetite (a share of the week), what done looks like in a fe | ||
| 5 | 5 | |
| 6 | 6 | - **Propose.** "Ask the planner to propose bets" groups open issues in no live bet by their first label; a theme with two or more issues becomes a proposal at 20% of the week. At most five proposals a pass. |
| 7 | 7 | - **Back.** The owner (a person who administers the repo, never an agent) backs one with a share. At most 5 run, and their shares stay within 100%. |
| 8 | - **Picks.** Once a repository has ever had a bet backed, builders pick only scopes of a running bet, issues the owner asked for or raised, and small fixes (held to `slots.smallShare`, #7571). An issue in no bet does not start by itself. A repository with no bet picks as before. | |
| 8 | - **Picks.** Once a repository has ever had a bet backed, builders pick only scopes of a running bet, issues the owner asked for or raised, and small fixes, polish included (held to `slots.smallShare`, #7571; a polish run is capped, see polish-cap.md). An issue in no bet does not start by itself. A repository with no bet picks as before. | |
| 9 | 9 | - **Appetite.** The budget is share × builder slots × one week of builder time, counted from the runs on the bet's scopes since it was backed. At the budget the bet stops: no extension. Its left line defaults to the open scopes; the lead rewrites it in one line; the owner re-backs, cuts or ends it (ending as done is for a stopped bet only; a running bet finishes when every scope closes). |
| 10 | 10 | - **Scopes** read *figuring out* (no run yet), *executing*, or *raised hand* (nothing moved for two days), or *done*. |
| 11 | 11 | - **Finishing.** When every scope is closed the bet is finished and its page says "This is done. Try it." with a link; the owner taps *Feels done* or *Not yet, because…* (the bet returns to stopped with that reason). `/changelog` marks each landing with its bet and filters by it (`?bet=`). |
docs/features/polish-cap.md added +10
| @@ -0,0 +1,10 @@ | ||
| 1 | # Polish cap | |
| 2 | ||
| 3 | An issue labelled `polish` is small work, and it is capped small (#8852). A builder run on it that has gone `polish.capMinutes` of run time (default 60) is stopped, not extended. The crew's tick checks every minute, from the run's own start time. | |
| 4 | ||
| 5 | - **Set it.** An admin changes **Run time a polish issue may take (minutes)** on `/settings/capacity`. It is read on every tick. | |
| 6 | - **What you see.** The run ends cancelled with the summary "stopped at the N-minute polish cap", and the issue gets one comment: `Stopped at the 60-minute polish cap, not extended. Left: pull #M is open, unmerged.` or `Left: nothing landed, no pull open.` It is the run's open pull that says what is left; a polish run never produces a second comment. | |
| 7 | - **Who says it.** The person who turned autonomy on, else the user the run acted as. If neither exists the run is left running rather than stopped silently. | |
| 8 | - **Other work.** Only the label `polish` (any case) is capped; polish also counts as a small fix against `slots.smallShare`. | |
| 9 | ||
| 10 | Left out: stopping a polish issue before its run starts, and reading the run's last summary into the line (a running run has none yet). |
src/core/capacity.ts +14 −1
| @@ -25,7 +25,8 @@ export type CapacityKey = | ||
| 25 | 25 | | "cells.egress" |
| 26 | 26 | | "guard_new_accounts" |
| 27 | 27 | | "signin.edge" |
| 28 | | "slots.smallShare"; | |
| 28 | | "slots.smallShare" | |
| 29 | | "polish.capMinutes"; | |
| 29 | 30 | |
| 30 | 31 | interface Definition { |
| 31 | 32 | /** What the settings page and the API call it. */ |
| @@ -116,6 +117,13 @@ export const CAPACITY: Record<CapacityKey, Definition> = { | ||
| 116 | 117 | fallback: "30", |
| 117 | 118 | help: "The most of a repository's builder slots that tests-only and small-addition issues (\"A test pins …\", or a label test, tests, tests-only or small) hold at once, as a percent rounded up: 30 lets 2 of 5 slots go to them. At the cap a small issue is skipped and the slot goes to the next issue in the Next up order; a ranked small issue is still taken. 100 lifts the cap; 0 starts none. Read on every pick.", |
| 118 | 119 | }, |
| 120 | "polish.capMinutes": { | |
| 121 | label: "Run time a polish issue may take (minutes)", | |
| 122 | env: "FORGE_POLISH_CAP_MINUTES", | |
| 123 | kind: { type: "count", min: 1, max: 1440 }, | |
| 124 | fallback: "60", | |
| 125 | help: "An issue labelled polish is small work: a builder run on it that has gone this many minutes is stopped, not extended, and the issue gets one comment saying what is left. Read on every tick.", | |
| 126 | }, | |
| 119 | 127 | }; |
| 120 | 128 | |
| 121 | 129 | export const CAPACITY_KEYS = Object.keys(CAPACITY) as CapacityKey[]; |
| @@ -344,6 +352,11 @@ export function smallShare(db: Database): number { | ||
| 344 | 352 | return Number(capacityValue(db, "slots.smallShare")); |
| 345 | 353 | } |
| 346 | 354 | |
| 355 | /** The minutes of run time a polish issue may take before its run is stopped (#8852). */ | |
| 356 | export function polishCapMinutes(db: Database): number { | |
| 357 | return Number(capacityValue(db, "polish.capMinutes")); | |
| 358 | } | |
| 359 | ||
| 347 | 360 | /** Whether the platform edge's identity is read at all; on every request. */ |
| 348 | 361 | export function edgeSigninOn(db: Database): boolean { |
| 349 | 362 | return capacityValue(db, "signin.edge") !== "off"; |
src/crew/autonomy.ts +2
| @@ -34,6 +34,7 @@ import { queueBatch, queueDepth, smallShare } from "../core/capacity.ts"; | ||
| 34 | 34 | import { shareWords, slotOrder, smallCap, smallIssues, smallOfIds } from "./small_share.ts"; |
| 35 | 35 | import { backedBetOf, betsInUse } from "../core/bets.ts"; |
| 36 | 36 | import { mayPick } from "./bets.ts"; |
| 37 | import { tickPolish } from "./polish_cap.ts"; | |
| 37 | 38 | import { raisedAt } from "../core/thought_links.ts"; |
| 38 | 39 | import { readConflictLane, readConflictRoom } from "./conflict_room.ts"; |
| 39 | 40 | import { mergeable, resolveOid, treeOf } from "../git/merge.ts"; |
| @@ -486,6 +487,7 @@ async function driveOnce( | ||
| 486 | 487 | ): Promise<Drove> { |
| 487 | 488 | if (repo.autonomy === "off") return "off"; |
| 488 | 489 | tickBets(app, repo, now); |
| 490 | tickPolish(app, repo, now); | |
| 489 | 491 | const status = autonomyStatus(app, repo, now); |
| 490 | 492 | if (status.paused !== null) return "paused"; |
| 491 | 493 | const by = getUserById(app.db, repo.autonomyBy!); |
src/crew/polish_cap.ts added +68
| @@ -0,0 +1,68 @@ | ||
| 1 | /** | |
| 2 | * A polish issue is capped small (#8852): a builder run on an issue labelled | |
| 3 | * `polish` that has gone past `polish.capMinutes` of run time is stopped, not | |
| 4 | * extended, and the issue gets one comment with what is left in a line, as a | |
| 5 | * bet past its appetite does (#8713). Time is the run's own: `startedAt` to | |
| 6 | * now, so a fake clock decides it. | |
| 7 | */ | |
| 8 | ||
| 9 | import type { App } from "../app.ts"; | |
| 10 | import { addComment } from "../actions/issues.ts"; | |
| 11 | import { announceRun } from "../actions/runs.ts"; | |
| 12 | import { polishCapMinutes } from "../core/capacity.ts"; | |
| 13 | import { getIssueById, labelsForIssues } from "../core/issues.ts"; | |
| 14 | import { openPullFromHead } from "../core/pulls.ts"; | |
| 15 | import { cancelRun, listActive } from "../core/runs.ts"; | |
| 16 | import { getUserById } from "../core/users.ts"; | |
| 17 | import type { Repo } from "../core/repos.ts"; | |
| 18 | import { abortAgentRun } from "./running.ts"; | |
| 19 | ||
| 20 | /** Whether a run begun at `startedAt` has used `capMinutes` of run time by `now`. */ | |
| 21 | export function pastCap(startedAt: string | null, capMinutes: number, now: Date): boolean { | |
| 22 | if (startedAt === null) return false; | |
| 23 | const from = Date.parse(startedAt); | |
| 24 | return Number.isFinite(from) && now.getTime() - from >= capMinutes * 60_000; | |
| 25 | } | |
| 26 | ||
| 27 | /** What the run left: its open pull, or plainly that nothing landed. */ | |
| 28 | export function leftOf(pullNumber: number | undefined): string { | |
| 29 | return pullNumber === undefined ? "nothing landed, no pull open" : `pull #${pullNumber} is open, unmerged`; | |
| 30 | } | |
| 31 | ||
| 32 | /** The one line a stopped polish run leaves: the cap, and what is left. */ | |
| 33 | export function leftLine(capMinutes: number, left: string): string { | |
| 34 | return `Stopped at the ${capMinutes}-minute polish cap, not extended. Left: ${left}.`; | |
| 35 | } | |
| 36 | ||
| 37 | /** Stops this repository's polish runs past the cap; answers how many. */ | |
| 38 | export function tickPolish(app: App, repo: Repo, now = new Date()): number { | |
| 39 | const runs = listActive(app.db, "agent").filter( | |
| 40 | (r) => r.repoId === repo.id && r.state === "running" && r.issueId !== null, | |
| 41 | ); | |
| 42 | if (runs.length === 0) return 0; | |
| 43 | const cap = polishCapMinutes(app.db); | |
| 44 | const late = runs.filter((r) => pastCap(r.startedAt, cap, now)); | |
| 45 | if (late.length === 0) return 0; | |
| 46 | const labels = labelsForIssues(app.db, late.map((r) => r.issueId!)); | |
| 47 | // The comment is the stop's one line, so it is never silent: the autonomy | |
outdated
Cancel happens before knowing a comment can be posted; with autonomyBy null the stop is silent. Fall back to run.actorId or do not cancel. |
||
| 48 | // author says it, else the user the run acted as. | |
| 49 | const speaker = (actorId: string | null) => { | |
| 50 | const id = repo.autonomyBy ?? actorId; | |
| 51 | return id === null ? undefined : getUserById(app.db, id); | |
| 52 | }; | |
| 53 | let stopped = 0; | |
| 54 | for (const run of late) { | |
| 55 | if (!(labels.get(run.issueId!) ?? []).some((l) => l.name.toLowerCase() === "polish")) continue; | |
| 56 | const by = speaker(run.actorId); | |
| 57 | const issue = getIssueById(app.db, run.issueId!); | |
| 58 | if (by === undefined || issue === undefined) continue; // no one to say it: leave the run, never stop it silently | |
| 59 | const gone = cancelRun(app.db, run.id, `stopped at the ${cap}-minute polish cap`); | |
| 60 | if (gone === undefined) continue; | |
| 61 | abortAgentRun(run.id); | |
| 62 | announceRun(app, gone); | |
| 63 | stopped++; | |
| 64 | const pull = run.branch === "" ? undefined : openPullFromHead(app.db, repo.id, repo.id, run.branch); | |
| 65 | addComment(app, by, repo, issue, leftLine(cap, leftOf(pull?.number))); | |
| 66 | } | |
| 67 | return stopped; | |
| 68 | } |
test/polish-cap.test.ts added +131
| @@ -0,0 +1,131 @@ | ||
| 1 | /** | |
| 2 | * A polish issue is capped small (#8852): past the cap its run is stopped | |
| 3 | * with one comment; under it, or on any other issue, nothing happens. The | |
| 4 | * clock is a Date handed in, never the wall's. | |
| 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 { openIssue, setIssueLabels } from "../src/actions/issues.ts"; | |
| 12 | import { createLabel, createRepo } from "../src/actions/repos.ts"; | |
| 13 | import { listComments, type Issue } from "../src/core/issues.ts"; | |
| 14 | import { getRepoById, type Repo } from "../src/core/repos.ts"; | |
| 15 | import { claimRun, createRun, getRun } from "../src/core/runs.ts"; | |
| 16 | import { createUser, type User } from "../src/core/users.ts"; | |
| 17 | import { isSmall } from "../src/crew/small_share.ts"; | |
| 18 | import { leftLine, leftOf, pastCap, tickPolish } from "../src/crew/polish_cap.ts"; | |
| 19 | ||
| 20 | let dataDir: string; | |
| 21 | let app: App; | |
| 22 | let alice: User; | |
| 23 | let repo: Repo; | |
| 24 | ||
| 25 | function runOn(title: string, labels: string[]) { | |
| 26 | const made = openIssue(app, alice, getRepoById(app.db, repo.id)!, { title }); | |
| 27 | if (!made.ok) throw new Error(String(made.error)); | |
| 28 | const issue: Issue = made.value; | |
| 29 | if (labels.length > 0) expect(setIssueLabels(app, alice, repo, issue, labels).ok).toBe(true); | |
| 30 | const run = createRun(app.db, { repoId: repo.id, kind: "agent", issueId: issue.id, actorId: alice.id, reason: "assigned" }); | |
| 31 | claimRun(app.db, run.id); | |
| 32 | return { issue, run }; | |
| 33 | } | |
| 34 | ||
| 35 | const startedAt = (id: string) => Date.parse(getRun(app.db, id)!.startedAt!); | |
| 36 | const comments = (issue: Issue) => listComments(app.db, issue.id).filter((c) => c.body.includes("polish cap")); | |
| 37 | ||
| 38 | beforeAll(async () => { | |
| 39 | dataDir = await mkdtemp(join(tmpdir(), "forge-polish-")); | |
| 40 | app = createApp({ dataDir, port: 0 }); | |
| 41 | const a = await createUser(app.db, { | |
| 42 | origin: "seed", trusted: true, username: "alice", email: "[email protected]", | |
| 43 | password: "a-good-passphrase", adminIfFirst: true, | |
| 44 | }); | |
| 45 | if (!a.ok) throw new Error(a.error); | |
| 46 | alice = a.value; | |
| 47 | const made = await createRepo(app, alice, { name: "notes", visibility: "private" }); | |
| 48 | if (!made.ok) throw new Error(String(made.error)); | |
| 49 | repo = made.value; | |
| 50 | for (const [n, c] of [["polish", "aabbcc"], ["feature", "112233"]] as const) { | |
| 51 | const l = createLabel(app, alice, repo, n, c); | |
| 52 | if (!l.ok && l.error !== "taken") throw new Error(String(l.error)); | |
| 53 | } | |
| 54 | // The drive reads the repository as stored, with its autonomy author. | |
| 55 | repo = { ...getRepoById(app.db, repo.id)!, autonomyBy: alice.id }; | |
| 56 | }); | |
| 57 | ||
| 58 | afterAll(async () => { | |
| 59 | await app.close?.(); | |
| 60 | await rm(dataDir, { recursive: true, force: true }); | |
| 61 | }); | |
| 62 | ||
| 63 | describe("the rules", () => { | |
| 64 | test("polish counts as small-fixes work", () => { | |
| 65 | expect(isSmall("Round the corners", ["polish"])).toBe(true); | |
| 66 | expect(isSmall("Round the corners", ["Polish"])).toBe(true); | |
| 67 | }); | |
| 68 | ||
| 69 | test("a run is past the cap at exactly 60 minutes, not at 59", () => { | |
| 70 | const at = "2026-11-02T10:00:00.000Z"; | |
| 71 | expect(pastCap(at, 60, new Date("2026-11-02T10:59:59.000Z"))).toBe(false); | |
| 72 | expect(pastCap(at, 60, new Date("2026-11-02T11:00:00.000Z"))).toBe(true); | |
| 73 | expect(pastCap(null, 60, new Date("2030-01-01T00:00:00Z"))).toBe(false); | |
| 74 | }); | |
| 75 | ||
| 76 | test("the line names the cap and what is left, plainly", () => { | |
| 77 | expect(leftLine(60, leftOf(undefined))).toBe( | |
| 78 | "Stopped at the 60-minute polish cap, not extended. Left: nothing landed, no pull open.", | |
| 79 | ); | |
| 80 | expect(leftLine(45, leftOf(12))).toBe( | |
| 81 | "Stopped at the 45-minute polish cap, not extended. Left: pull #12 is open, unmerged.", | |
| 82 | ); | |
| 83 | }); | |
| 84 | }); | |
| 85 | ||
| 86 | describe("the tick", () => { | |
| 87 | test("a polish run under the cap is left running", () => { | |
| 88 | const { issue, run } = runOn("Under: nudge the margin", ["polish"]); | |
| 89 | expect(tickPolish(app, repo, new Date(startedAt(run.id) + 59 * 60_000))).toBe(0); | |
| 90 | expect(getRun(app.db, run.id)!.state).toBe("running"); | |
| 91 | expect(comments(issue)).toHaveLength(0); | |
| 92 | }); | |
| 93 | ||
| 94 | test("past the cap it stops with one comment, however often the tick runs; a non-polish issue is untouched", () => { | |
| 95 | const polish = runOn("Past: nudge the padding", ["polish"]); | |
| 96 | const other = runOn("Past: a feature", ["feature"]); | |
| 97 | const unlabelled = runOn("Past: no label", []); | |
| 98 | const late = new Date(startedAt(polish.run.id) + 61 * 60_000); | |
| 99 | // Earlier tests' runs may also be past by now; this run is the one under test. | |
| 100 | expect(tickPolish(app, repo, late)).toBeGreaterThanOrEqual(1); | |
| 101 | expect(tickPolish(app, repo, late)).toBe(0); | |
| 102 | expect(getRun(app.db, polish.run.id)!.state).toBe("cancelled"); | |
| 103 | expect(getRun(app.db, polish.run.id)!.summary).toBe("stopped at the 60-minute polish cap"); | |
| 104 | const said = comments(polish.issue); | |
| 105 | expect(said).toHaveLength(1); | |
| 106 | expect(said[0]!.body).toContain("Left: nothing landed, no pull open."); | |
| 107 | expect(getRun(app.db, other.run.id)!.state).toBe("running"); | |
| 108 | expect(getRun(app.db, unlabelled.run.id)!.state).toBe("running"); | |
| 109 | expect(comments(other.issue)).toHaveLength(0); | |
| 110 | }); | |
| 111 | }); | |
| 112 | ||
| 113 | describe("who says it", () => { | |
| 114 | test("with no autonomy author the run's actor says it: the stop is never silent", () => { | |
| 115 | const { issue, run } = runOn("Silent: nudge the gap", ["polish"]); | |
| 116 | const bare = { ...repo, autonomyBy: null }; | |
| 117 | tickPolish(app, bare, new Date(startedAt(run.id) + 61 * 60_000)); | |
| 118 | expect(getRun(app.db, run.id)!.state).toBe("cancelled"); | |
| 119 | const said = comments(issue); | |
| 120 | expect(said).toHaveLength(1); | |
| 121 | expect(said[0]!.authorId).toBe(alice.id); | |
| 122 | }); | |
| 123 | ||
| 124 | test("with no one to say it the run is left, not stopped silently", () => { | |
| 125 | const { issue, run } = runOn("Mute: nudge the gap", ["polish"]); | |
| 126 | app.db.query("UPDATE run SET actor_id = NULL WHERE id = ?1").run(run.id); | |
| 127 | tickPolish(app, { ...repo, autonomyBy: null }, new Date(startedAt(run.id) + 61 * 60_000)); | |
| 128 | expect(getRun(app.db, run.id)!.state).toBe("running"); | |
| 129 | expect(comments(issue)).toHaveLength(0); | |
| 130 | }); | |
| 131 | }); |
new above
offline
Filler: "what the run had not landed" says nothing. Name what is actually left.