trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns12g rEventsFlowsAPI

An approved pull has waited 553 hours and the tick only counts it: name it, then land it or close it #12446

Merged scoutcrew opened this in answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/12440-an-approved-pull-has-waited-553-hours-an → main 10 commits fork 1 thread · 1 unresolved

  • Merged. scout/12440-an-approved-pull-has-waited-553-hours-an was merged into main as a1ec9db.

Review

Sign in to review this change.
8 files changed +346 −36
docs/features/approved-named.md added +11
@@ -0,0 +1,11 @@
1 # Naming the approved pull that waits
2
3 The flow tick used to say "oldest approved 553 h" and "$66 stranded" and leave you to hunt for which pulls. Now it names them.
4
5 - **The tick line and misses** carry the oldest approved pull as `owner/repo#number`, and the costliest stranded pulls (up to five) with their dollars. The dollars follow the same rule as `strandedUsd`: a reader without `telemetry:read` sees neither. Private repositories are left out of public readings.
6 - **The needs stack** ("How is my forge?", on the home page, in the terminal screen and in the MCP `forge` tool) gains one row for each pull approved a week (168 h) or more ago and still open, oldest first. Only an admin sees it. At most 20 are rows; past that, the last row says how many more it leaves out. A later `changes_requested` review removes the pull.
7 - **Its one choice is "Open it"**: the link to the pull, where Land or Close is done with the pull's own checks. The row is a question for the owner, so `a` in the terminal and `apply` leave it alone.
8
9 Try it: open `/`, read the flow line, then press the needs key in the terminal (`ssh` command in `FORGE_APP_SSH`) and look for the `approved:` row.
10
11 Not done: the engine landing or closing such a pull on its own is a separate issue.
src/actions/needs.ts +24 −1
@@ -9,6 +9,7 @@
99 import type { App } from "../app.ts";
1010 import { applyAsksBulk, receiptOf } from "./asks_bulk.ts";
1111 import { askStackFor, type askLinesFor } from "../web/studio/asks.ts";
12 import { weekOld } from "../flow/oldest_approved.ts";
1213 import { drillNow } from "./backups_now.ts";
1314 import { builderOf, giveBack, handBack } from "./give_back.ts";
1415 import { getIssueById } from "../core/issues.ts";
@@ -143,6 +144,28 @@ export function forgeRecommended(cause: string, sentence: string): Recommended {
143144 return needsDecision(`${sentence} Look into it?`, "the forge's own health is the owner's to act on", ["Open it", "Leave it"]);
144145 }
145146
147 /**
148 * An approved pull a week old (#12440): named, its one real choice the link to its page.
149 * The oldest `WEEK_OLD_LIMIT` are rows; the last row says how many more it leaves out.
150 */
151 function approvedNeeds(ctx: Home): Need[] {
152 const { pulls, more } = weekOld(ctx.app.db);
153 return pulls.map(
154 (p, i): Need => ({
155 id: `approved:${p.ref}`,
156 sentence:
157 `${p.ref} was approved ${Math.floor(p.hours / 24)} days ago and has not landed.` +
158 (more > 0 && i === pulls.length - 1 ? ` ${more} more approved a week ago are not listed.` : ""),
159 why: "approved work that sits still is the flow's one invariant broken",
160 waits: "it ages, and its branch drifts from main",
161 choices: ["Open it"],
162 recommended: needsDecision(`${p.ref}: open it, then land it or close it.`, "landing or closing is a person's call on the pull's own page", ["Open it"]),
163 link: `/${p.owner}/${p.repo}/pulls/${p.pull}`,
164 items: [],
165 }),
166 );
167 }
168
146169 /** The stack: the forge's own needs first (an admin's), then the asks, most consequential first. */
147170 export function needsStack(ctx: Home, user: User, chips?: Chips): Need[] {
148171 const stack = askStackFor(ctx, user, chips);
@@ -170,7 +193,7 @@ export function needsStack(ctx: Home, user: User, chips?: Chips): Need[] {
170193 items: row.items,
171194 }),
172195 );
173 return [...forge, ...rows];
196 return [...forge, ...(user.isAdmin ? approvedNeeds(ctx) : []), ...rows];
174197 }
175198
176199 /** One or two sentences: serving or not, the crew, what is unusual. */
src/flow/oldest_approved.ts +49 −15
@@ -1,37 +1,71 @@
11 /**
2 * The oldest approved wait (#6422, for #5930): how long the longest-waiting
3 * approved, unlanded pull has waited since its latest approval. A pull whose
4 * latest approval is followed by a changes-requested verdict does not count.
5 * A clog is declared when this passes 24 h. Nothing shows it yet.
2 * Approved and waiting (#6422, for #5930; named for #12440): the approved,
3 * unlanded pulls and how long each has waited since its latest approval. A pull
4 * whose latest approval is followed by a changes-requested verdict does not
5 * count. The one definition of the rule: the oldest (the tick's clog figure)
6 * and the week-old (the needs stack) both read it.
67 */
78
89 import type { Database } from "bun:sqlite";
910
1011 const HOUR_MS = 3_600_000;
12 export const WEEK_HOURS = 168;
13 /** How many week-old pulls the needs stack lists; the rest are counted, not named. */
14 export const WEEK_OLD_LIMIT = 20;
1115
1216 export interface OldestApproved {
1317 readonly pull: number;
1418 readonly repo: string;
19 readonly owner: string;
20 /** `owner/repo#number`, so a person or an agent can act on it in a step. */
21 readonly ref: string;
1522 readonly hours: number;
1623 }
1724
18 /** `publicOnly` reads public repositories alone, as `readVitals` does. */
19 export function readOldestApproved(db: Database, now = new Date(), publicOnly = false): OldestApproved | null {
20 const row = db
21 .query<{ number: number; name: string; approved_at: string }, [number]>(
22 `SELECT i.number AS number, r.name AS name, a.at AS approved_at
25 /**
26 * Approved, unlanded pulls whose latest approval is at least `minHours` old, oldest
27 * first, at most `limit`. `publicOnly` reads public repositories alone, as `readVitals` does.
28 */
29 export function approvedPulls(db: Database, now = new Date(), publicOnly = false, minHours = 0, limit = 1): OldestApproved[] {
30 const cut = new Date(now.getTime() - minHours * HOUR_MS).toISOString();
31 return db
32 .query<{ number: number; name: string; owner: string; approved_at: string }, [number, string, number]>(
33 `SELECT i.number AS number, r.name AS name, o.username AS owner, a.at AS approved_at
2334 FROM pull p
2435 JOIN issue i ON i.id = p.issue_id
2536 JOIN repo r ON r.id = i.repo_id
37 JOIN user o ON o.id = r.owner_id
2638 JOIN (SELECT issue_id, MAX(created_at) AS at FROM review WHERE state = 'approved' GROUP BY issue_id) a
2739 ON a.issue_id = i.id
28 WHERE (?1 = 0 OR r.visibility = 'public') AND i.state = 'open' AND p.merged_at IS NULL
40 WHERE (?1 = 0 OR r.visibility = 'public') AND i.state = 'open' AND p.merged_at IS NULL AND a.at <= ?2
2941 AND NOT EXISTS (SELECT 1 FROM review c WHERE c.issue_id = i.id AND c.state = 'changes_requested' AND c.created_at > a.at)
3042 ORDER BY a.at ASC, i.number ASC
31 LIMIT 1`,
43 LIMIT ?3`,
3244 )
33 .get(publicOnly ? 1 : 0);
34 if (!row) return null;
35 const hours = Math.max(0, (now.getTime() - Date.parse(row.approved_at)) / HOUR_MS);
36 return { pull: row.number, repo: row.name, hours };
45 .all(publicOnly ? 1 : 0, cut, limit)
46 .map((r) => ({
47 pull: r.number,
48 repo: r.name,
49 owner: r.owner,
50 ref: `${r.owner}/${r.name}#${r.number}`,
51 hours: Math.max(0, (now.getTime() - Date.parse(r.approved_at)) / HOUR_MS),
52 }));
53 }
54
55 /** The longest-waiting approved pull; null when none waits. A clog is declared past 24 h. */
56 export function readOldestApproved(db: Database, now = new Date(), publicOnly = false): OldestApproved | null {
57 return approvedPulls(db, now, publicOnly)[0] ?? null;
58 }
59
60 export interface WeekOld {
61 /** The oldest `WEEK_OLD_LIMIT`, oldest first. */
62 readonly pulls: readonly OldestApproved[];
63 /** How many more wait a week that `pulls` leaves out. */
64 readonly more: number;
65 }
66
67 /** Approved a week ago or more and still not landed: the ones that need a person's word. */
68 export function weekOld(db: Database, now = new Date()): WeekOld {
69 const all = approvedPulls(db, now, false, WEEK_HOURS, -1);
70 return { pulls: all.slice(0, WEEK_OLD_LIMIT), more: Math.max(0, all.length - WEEK_OLD_LIMIT) };
3771 }
src/flow/stranded.ts +14 −11
@@ -2,7 +2,8 @@
22 * The stranded spend (#5930): what the crew's runs on pulls that are open and
33 * not landed have cost so far. One read; the same `publicOnly` rule as
44 * `readVitals`, so a reader who may not see private repositories learns
5 * nothing about them. Numbers and dollars only, no repository named.
5 * nothing about them. Each of the costliest is named `owner/repo#number`
6 * (#12440). The sum per pull is a lookup on the `run_issue` index.
67 */
78
89 import type { Database } from "bun:sqlite";
@@ -11,6 +12,8 @@ const COSTLIEST = 5;
1112
1213 export interface StrandedPull {
1314 readonly number: number;
15 /** `owner/repo#number`. */
16 readonly ref: string;
1417 readonly usd: number;
1518 }
1619
@@ -25,28 +28,28 @@ export interface Stranded {
2528
2629 interface Row {
2730 readonly number: number;
31 readonly ref: string;
2832 readonly usd: number;
2933 readonly total_usd: number;
3034 readonly total_pulls: number;
3135 }
3236
33 export function readStranded(db: Database, publicOnly = false): Stranded {
34 const rows = db
35 .query<Row, [number, number]>(
36 `WITH open_pull AS (
37 SELECT i.number AS number,
37 export const STRANDED_SQL = `WITH open_pull AS (
38 SELECT i.number AS number, o.username || '/' || r.name || '#' || i.number AS ref,
3839 COALESCE((SELECT SUM(u.cost_usd) FROM run u WHERE u.issue_id = p.issue_id), 0) AS usd
3940 FROM pull p JOIN issue i ON i.id = p.issue_id JOIN repo r ON r.id = i.repo_id
41 JOIN user o ON o.id = r.owner_id
4042 WHERE (?1 = 0 OR r.visibility = 'public') AND i.state = 'open' AND p.merged_at IS NULL
4143 )
42 SELECT number, usd, SUM(usd) OVER () AS total_usd, COUNT(*) OVER () AS total_pulls
43 FROM open_pull ORDER BY usd DESC, number ASC LIMIT ?2`,
44 )
45 .all(publicOnly ? 1 : 0, COSTLIEST);
44 SELECT number, ref, usd, SUM(usd) OVER () AS total_usd, COUNT(*) OVER () AS total_pulls
45 FROM open_pull ORDER BY usd DESC, number ASC LIMIT ?2`;
46
47 export function readStranded(db: Database, publicOnly = false): Stranded {
48 const rows = db.query<Row, [number, number]>(STRANDED_SQL).all(publicOnly ? 1 : 0, COSTLIEST);
4649 const first = rows[0];
4750 return {
4851 usd: first?.total_usd ?? 0,
4952 pulls: first?.total_pulls ?? 0,
50 costliest: rows.map((r) => ({ number: r.number, usd: r.usd })),
53 costliest: rows.map((r) => ({ number: r.number, ref: r.ref, usd: r.usd })),
5154 };
5255 }
src/flow/vitals.ts +31 −7
@@ -18,6 +18,12 @@ import { readOldestApproved } from "./oldest_approved.ts";
1818 import { readStranded } from "./stranded.ts";
1919 import { kept } from "./kept.ts";
2020
21 /** A stranded pull named with its dollars (#12440). */
22 export interface NamedPull {
23 readonly ref: string;
24 readonly usd: number;
25 }
26
2127 const HOUR_MS = 3_600_000;
2228
2329 export const OPEN_TARGET = 40;
@@ -43,6 +49,10 @@ export interface Vitals {
4349 readonly oldestApprovedHours: number | null;
4450 /** Dollars of crew runs on open, unlanded pulls. */
4551 readonly strandedUsd: number;
52 /** The oldest approved pull as `owner/repo#n` (#12440); left out when none waits. */
53 readonly oldestApprovedRef?: string;
54 /** The costliest stranded pulls, named, dollars each (#12440); left out when none cost. */
55 readonly strandedPulls?: readonly NamedPull[];
4656 /** Check-slot minutes the merge queue's gates spent in the last hour. */
4757 readonly gateMinutes1h: number;
4858 /** Check-slot minutes every other check spent in the last hour. */
@@ -54,12 +64,15 @@ export interface Vitals {
5464 }
5565
5666 /** What a reader is shown: `strandedUsd` is left out for a reader without telemetry:read. */
57 export type ShownVitals = Omit<Vitals, "strandedUsd"> & { readonly strandedUsd?: number };
67 export type ShownVitals = Omit<Vitals, "strandedUsd" | "strandedPulls"> & {
68 readonly strandedUsd?: number;
69 readonly strandedPulls?: readonly NamedPull[];
70 };
5871
5972 /** `v` as a reader may see it: dollars only when `spend` (the owner, or an agent holding telemetry:read). */
6073 export function vitalsFor(v: Vitals, spend: boolean): ShownVitals {
6174 if (spend) return v;
62 const { strandedUsd: _spend, ...rest } = v;
75 const { strandedUsd: _spend, strandedPulls: _pulls, ...rest } = v;
6376 return rest;
6477 }
6578
@@ -94,6 +107,9 @@ export function readVitals(db: Database, now = new Date(), publicOnly = false):
94107 const pile = readChecksPile(db, publicOnly);
95108 const gate = readGateMinutes(db, now, publicOnly);
96109 const batches = readRedBatches(db, now, publicOnly);
110 const oldest = readOldestApproved(db, now, publicOnly);
111 const stranded = readStranded(db, publicOnly);
112 const named: NamedPull[] = stranded.costliest.filter((p) => p.usd > 0).map((p) => ({ ref: p.ref, usd: p.usd }));
97113 return {
98114 open: row?.open ?? 0,
99115 opened1h: row?.opened1h ?? 0,
@@ -103,12 +119,14 @@ export function readVitals(db: Database, now = new Date(), publicOnly = false):
103119 conflicted: conflictedPulls(db, publicOnly).length,
104120 red: pile.red,
105121 queued: pile.queued,
106 oldestApprovedHours: readOldestApproved(db, now, publicOnly)?.hours ?? null,
107 strandedUsd: readStranded(db, publicOnly).usd,
122 oldestApprovedHours: oldest?.hours ?? null,
123 strandedUsd: stranded.usd,
108124 gateMinutes1h: gate.gate1h,
109125 pullCheckMinutes1h: gate.pull1h,
110126 batches6h: batches.finished,
111127 redBatchRate: batches.rate,
128 ...(oldest === null ? {} : { oldestApprovedRef: oldest.ref }),
129 ...(named.length === 0 ? {} : { strandedPulls: named }),
112130 };
113131 }
114132
@@ -119,7 +137,7 @@ export function vitalsMisses(v: ShownVitals): string[] {
119137 if (v.conflicted >= CONFLICTED_TARGET) out.push(`conflicted ${v.conflicted} ≥ ${CONFLICTED_TARGET}`);
120138 if (v.landed6h < v.opened6h) out.push(`landed ${v.landed6h} < opened ${v.opened6h} over 6 h`);
121139 if (v.oldestApprovedHours !== null && v.oldestApprovedHours > APPROVED_WAIT_HOURS)
122 out.push(`oldest approved pull has waited ${Math.floor(v.oldestApprovedHours)} h`);
140 out.push(`oldest approved pull has waited ${Math.floor(v.oldestApprovedHours)} h${v.oldestApprovedRef ? ` (${v.oldestApprovedRef})` : ""}`);
123141 if (v.redBatchRate !== null && v.redBatchRate > RED_BATCH_TARGET && v.batches6h >= RED_BATCH_MIN)
124142 out.push(`${Math.round(v.redBatchRate * 100)}% of merge batches went red in 6 h`);
125143 return out;
@@ -127,14 +145,20 @@ export function vitalsMisses(v: ShownVitals): string[] {
127145
128146 const usd = (n: number) => `$${n.toFixed(2)}`;
129147
148 /** ` (a/b#1 $4.00, a/b#2 $1.00)`, or nothing when no pull is named. */
149 function strandedList(pulls: readonly NamedPull[] | undefined): string {
150 if (!pulls?.length) return "";
151 return ` (${pulls.map((p) => `${p.ref} ${usd(p.usd)}`).join(", ")})`;
152 }
153
130154 /** The new numbers, in the order both lines say them; each part is left out when it has nothing to say. */
131155 function extras(v: ShownVitals): { red: string; oldest: string | null; stranded: string | null; gate: string; batches: string | null } {
132156 return {
133157 gate: `gate ${Math.round(v.gateMinutes1h)} min, other checks ${Math.round(v.pullCheckMinutes1h)} min in 1 h`,
134158 batches: v.redBatchRate === null ? null : `${Math.round(v.redBatchRate * 100)}% of ${v.batches6h} batches red in 6 h`,
135159 red: `${v.red} red, ${v.queued} queued`,
136 oldest: v.oldestApprovedHours === null ? null : `oldest approved ${Math.floor(v.oldestApprovedHours)} h`,
137 stranded: v.strandedUsd === undefined ? null : `${usd(v.strandedUsd)} stranded`,
160 oldest: v.oldestApprovedHours === null ? null : `oldest approved ${Math.floor(v.oldestApprovedHours)} h${v.oldestApprovedRef ? ` (${v.oldestApprovedRef})` : ""}`,
161 stranded: v.strandedUsd === undefined ? null : `${usd(v.strandedUsd)} stranded${strandedList(v.strandedPulls)}`,
138162 };
139163 }
140164
test/flow-oldest-approved.test.ts +13 −1
@@ -73,7 +73,7 @@ describe("readOldestApproved", () => {
7373 test("uses the latest approval of a pull", () => {
7474 clear();
7575 const p = pull({ reviews: [["approved", 100], ["approved", 2]] });
76 expect(readOldestApproved(app.db, NOW, false)).toEqual({ pull: p, repo: "pub", hours: 2 });
76 expect(readOldestApproved(app.db, NOW, false)).toMatchObject({ pull: p, repo: "pub", hours: 2 });
7777 });
7878
7979 test("ignores a landed pull", () => {
@@ -107,3 +107,15 @@ describe("readOldestApproved", () => {
107107 expect(readOldestApproved(app.db, NOW, false)).toBeNull();
108108 });
109109 });
110
111 describe("named pulls (#12440)", () => {
112 test("the oldest approved pull is named, and a week-old one is listed for the needs stack", async () => {
113 const { weekOld } = await import("../src/flow/oldest_approved.ts");
114 clear();
115 pull({ reviews: [["approved", 5]] });
116 const old = pull({ reviews: [["approved", 200]] });
117 const ref = readOldestApproved(app.db, NOW)!.ref;
118 expect(ref).toMatch(new RegExp(`/pub#${old}$`));
119 expect(weekOld(app.db, NOW).pulls.map((p) => p.ref)).toEqual([ref]);
120 });
121 });
test/flow-vitals.test.ts +28 −1
@@ -183,7 +183,7 @@ describe("flow vitals", () => {
183183 expect(read({ kind: "agent", spend: true }).strandedUsd).toBe(3.5);
184184 expect(read({ kind: "owner" }).strandedUsd).toBe(3.5);
185185 expect(read({ kind: "owner" }).says).toContain("$3.50 stranded");
186 expect(read({ kind: "owner" }).misses).toContain("oldest approved pull has waited 30 h");
186 expect(read({ kind: "owner" }).misses).toContain("oldest approved pull has waited 30 h (alice/g#1)");
187187
188188 // 2. telemetryReader marks a granted agent with spend, and refuses one without the grant.
189189 const bot = await createAgent(app, made.value, { username: "muse" });
@@ -209,3 +209,30 @@ describe("flow vitals", () => {
209209 }
210210 }, 30_000);
211211 });
212
213 test("the tick line names the oldest approved pull and each stranded pull (#12440)", () => {
214 const base: Vitals = {
215 open: 1,
216 opened1h: 0,
217 landed1h: 0,
218 opened6h: 0,
219 landed6h: 0,
220 conflicted: 0,
221 red: 0,
222 queued: 0,
223 oldestApprovedHours: 553.2,
224 oldestApprovedRef: "trevato/forge#9",
225 strandedUsd: 66,
226 strandedPulls: [
227 { ref: "trevato/forge#9", usd: 40 },
228 { ref: "trevato/forge#8", usd: 26 },
229 ],
230 gateMinutes1h: 0,
231 pullCheckMinutes1h: 0,
232 batches6h: 0,
233 redBatchRate: null,
234 };
235 expect(vitalsMisses(base)).toEqual(["oldest approved pull has waited 553 h (trevato/forge#9)"]);
236 expect(vitalsWords(base)).toContain("oldest approved 553 h (trevato/forge#9), $66.00 stranded (trevato/forge#9 $40.00, trevato/forge#8 $26.00)");
237 expect(vitalsLine(vitalsFor(base, false))).not.toContain("$40.00");
238 });
test/needs-approved-week.test.ts added +176
@@ -0,0 +1,176 @@
1 /** The week-old approved pull in the needs stack and its boundaries (#12440). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { applyRecommended, needsStack } from "../src/actions/needs.ts";
8 import { createUser, type User } from "../src/core/users.ts";
9 import { approvedPulls, readOldestApproved, weekOld, WEEK_OLD_LIMIT } from "../src/flow/oldest_approved.ts";
10 import { readStranded, STRANDED_SQL } from "../src/flow/stranded.ts";
11 import { readVitals, vitalsFor } from "../src/flow/vitals.ts";
12 import type { Ctx, Screen } from "../src/tui/app/app.ts";
13 import { needsScreen } from "../src/tui/screens/needs.ts";
14 import { Screen as Surface } from "../src/tui/term/screen.ts";
15 import { theme } from "../src/tui/ui/theme.ts";
16 import type { Session } from "../src/tui/session.ts";
17
18 const HOUR = 3_600_000;
19 let dir: string;
20 let app: App;
21 let admin: User;
22 let other: User;
23 let n = 0;
24 const at = (now: Date, h: number): string => new Date(now.getTime() - h * HOUR).toISOString();
25
26 beforeAll(async () => {
27 dir = await mkdtemp(join(tmpdir(), "needs-week-"));
28 app = createApp({ dataDir: dir, port: 0 });
29 const mk = async (username: string, adminIfFirst: boolean): Promise<User> => {
30 const u = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst });
31 if (!u.ok) throw new Error(u.error);
32 return u.value;
33 };
34 admin = await mk("alice", true);
35 other = await mk("bob", false);
36 app.db.run("INSERT INTO repo (id, owner_id, name, visibility) VALUES ('pub', ?1, 'pub', 'public')", [admin.id]);
37 app.db.run("INSERT INTO repo (id, owner_id, name, visibility) VALUES ('priv', ?1, 'priv', 'private')", [admin.id]);
38 });
39 afterAll(async () => {
40 app.close?.();
41 await rm(dir, { recursive: true, force: true });
42 });
43
44 const clear = (): void => void app.db.run("DELETE FROM issue");
45 function pull(reviews: [string, number][], now: Date, repo = "pub"): number {
46 n++;
47 app.db.run("INSERT INTO issue (id, repo_id, number, kind, title, author_id, state) VALUES (?1, ?2, ?3, 'pull', 't', ?4, 'open')", [`w${n}`, repo, n, admin.id]);
48 app.db.run("INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid) VALUES (?1, 'main', ?2, '', 'h')", [`w${n}`, `b${n}`]);
49 reviews.forEach(([state, h], i) =>
50 app.db.run("INSERT INTO review (id, issue_id, author_id, state, created_at) VALUES (?1, ?2, ?3, ?4, ?5)", [`wr${n}-${i}`, `w${n}`, admin.id, state, at(now, h)]),
51 );
52 return n;
53 }
54 const approvedRows = (u: User) => needsStack({ app }, u).filter((r) => r.id.startsWith("approved:"));
55
56 test("an admin sees a week-old approved pull in the stack; a non-admin and a 5-day-old one do not", () => {
57 clear();
58 const now = new Date();
59 const old = pull([["approved", 200]], now);
60 pull([["approved", 120]], now);
61 const rows = approvedRows(admin);
62 expect(rows.map((r) => r.id)).toEqual([`approved:alice/pub#${old}`]);
63 expect(rows[0]!.sentence).toContain(`alice/pub#${old}`);
64 expect(rows[0]!.link).toBe(`/alice/pub/pulls/${old}`);
65 expect(rows[0]!.choices).toEqual(["Open it"]);
66 expect(approvedRows(other)).toEqual([]);
67 });
68
69 test("the approved: id is left for the owner by apply, never acted on", async () => {
70 clear();
71 const now = new Date();
72 const old = pull([["approved", 300]], now);
73 for (const dry of [true, false]) {
74 const out = await applyRecommended({ app }, admin, dry);
75 expect(out.left.map((l) => l.id)).toContain(`approved:alice/pub#${old}`);
76 expect(out.plan.map((p) => p.id)).not.toContain(`approved:alice/pub#${old}`);
77 expect(out.done).toEqual([]);
78 expect(out.refused).toEqual([]);
79 }
80 const only = await applyRecommended({ app }, admin, false, `approved:alice/pub#${old}`);
81 expect(only.done).toEqual([]);
82 expect(app.db.query("SELECT state FROM issue WHERE id = ?1").get(`w${old}`)).toEqual({ state: "open" });
83 });
84
85 test("the terminal's needs screen draws the row", () => {
86 clear();
87 const now = new Date();
88 const old = pull([["approved", 400]], now);
89 const f = { app, db: app.db, user: admin } as unknown as Session;
90 const ctx = { theme: theme("dark"), invalidate() {}, toast() {}, push() {}, pop() {}, copy() {}, hit() {}, setCursor() {}, track: (p: Promise<unknown>) => p } as unknown as Ctx;
91 const screen: Screen = needsScreen(f, ctx);
92 const s = new Surface(140, 20);
93 screen.render({ x: 0, y: 0, w: 140, h: 20 }, s);
94 expect(s.toText()).toContain(`alice/pub#${old} was approved 16 days ago`);
95 });
96
97 const refs = (w: { pulls: readonly { ref: string }[] }): string[] => w.pulls.map((p) => p.ref);
98 const NOW = new Date("2026-10-26T12:00:00.000Z");
99 function cost(issue: number, usd: number): void {
100 app.db.run("INSERT INTO run (repo_id, kind, issue_id, commit_oid, cost_usd) VALUES ((SELECT repo_id FROM issue WHERE id = ?1), 'agent', ?1, 'h', ?2)", [`w${issue}`, usd]);
101 }
102
103 test("weekOld: exactly 168 h is in, just under is out", () => {
104 clear();
105 const exact = pull([["approved", 168]], NOW);
106 pull([["approved", 167.99]], NOW);
107 expect(refs(weekOld(app.db, NOW))).toEqual([`alice/pub#${exact}`]);
108 });
109
110 test("a private repository is seen by weekOld but not by a publicOnly read", () => {
111 clear();
112 const priv = pull([["approved", 200]], NOW, "priv");
113 expect(refs(weekOld(app.db, NOW))).toEqual([`alice/priv#${priv}`]);
114 expect(approvedPulls(app.db, NOW, true, 168)).toEqual([]);
115 });
116
117 test("the oldest approved pull is named for the owner, and a private one never for publicOnly", () => {
118 clear();
119 const priv = pull([["approved", 300]], NOW, "priv");
120 const pub = pull([["approved", 100]], NOW);
121 expect(readOldestApproved(app.db, NOW)?.ref).toBe(`alice/priv#${priv}`);
122 expect(readOldestApproved(app.db, NOW, true)?.ref).toBe(`alice/pub#${pub}`);
123 expect(readVitals(app.db, NOW, true).oldestApprovedRef).toBe(`alice/pub#${pub}`);
124 });
125
126 test("a private pull with run cost is a stranded pull for the owner, not for publicOnly", () => {
127 clear();
128 const priv = pull([], NOW, "priv");
129 const pub = pull([], NOW);
130 cost(priv, 4);
131 cost(pub, 1);
132 expect(readStranded(app.db).costliest.map((p) => [p.ref, p.usd])).toEqual([[`alice/priv#${priv}`, 4], [`alice/pub#${pub}`, 1]]);
133 expect(readStranded(app.db, true).costliest.map((p) => p.ref)).toEqual([`alice/pub#${pub}`]);
134 expect(readVitals(app.db, NOW, true).strandedPulls).toEqual([{ ref: `alice/pub#${pub}`, usd: 1 }]);
135 });
136
137 test("vitalsFor without spend drops strandedPulls and strandedUsd from the object itself", () => {
138 clear();
139 cost(pull([], NOW), 3);
140 const v = readVitals(app.db, NOW);
141 expect(v.strandedPulls?.length).toBe(1);
142 const shown = vitalsFor(v, false);
143 expect("strandedPulls" in shown).toBe(false);
144 expect("strandedUsd" in shown).toBe(false);
145 expect(JSON.stringify(shown)).not.toContain("alice/pub#");
146 });
147
148 test("the stranded sum per pull is a lookup on the run_issue index, not a scan of run", () => {
149 const plan = app.db
150 .query<{ detail: string }, [number, number]>(`EXPLAIN QUERY PLAN ${STRANDED_SQL}`)
151 .all(0, 5)
152 .map((r) => r.detail);
153 expect(plan.some((d) => /SEARCH u USING (COVERING )?INDEX run_issue/.test(d))).toBe(true);
154 expect(plan.some((d) => /^SCAN u\b/.test(d))).toBe(false);
155 });
156
157 test("past the cap the stack lists the oldest and its last row counts the rest", () => {
158 clear();
159 const now = new Date();
160 for (let i = 0; i < WEEK_OLD_LIMIT + 2; i++) pull([["approved", 300 - i]], now);
161 const w = weekOld(app.db, now);
162 expect(w.pulls.length).toBe(WEEK_OLD_LIMIT);
163 expect(w.more).toBe(2);
164 const rows = approvedRows(admin);
165 expect(rows.length).toBe(WEEK_OLD_LIMIT);
166 expect(rows.at(-1)?.sentence).toContain("2 more approved a week ago are not listed.");
167 expect(rows[0]?.sentence).not.toContain("more approved");
168 });
169
170 test("a changes_requested review after the approval takes the pull out; a later approval brings it back", () => {
171 clear();
172 pull([["approved", 300], ["changes_requested", 200]], NOW);
173 expect(weekOld(app.db, NOW).pulls).toEqual([]);
174 const back = pull([["approved", 300], ["changes_requested", 250], ["approved", 200]], NOW);
175 expect(refs(weekOld(app.db, NOW))).toEqual([`alice/pub#${back}`]);
176 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs