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

Landing never stalls the crew: the green sweep runs beside the tick, not in front of it #2016

Merged opened this in answer to

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

Merged

mason/forge:mason/2015-landing-never-stalls-the-crew-the-green → main 3 commits fork 1 thread · 1 unresolved

  • Merged. mason/2015-landing-never-stalls-the-crew-the-green was merged into main as 26ea852.

Review

Sign in to review this change.
3 files changed +165 −2
scripts/gate.ts +1
@@ -151,6 +151,7 @@ export const GATE: readonly string[] = [
151151 "test/map-closing.test.ts",
152152 "test/parked-close.test.ts",
153153 "test/autonomy.test.ts",
154 "test/autonomy_sweep.test.ts",
154155 "test/studio.test.ts",
155156 "test/studio-words.test.ts",
156157 "test/converge-settled.test.ts",
src/crew/autonomy.ts +35 −2
@@ -1000,6 +1000,35 @@ export async function sweepGreen(app: App, repo: Repo): Promise<number> {
10001000 return landed;
10011001 }
10021002
1003 /** The sweep each repository has in flight, by repository id. */
1004 const sweeping = new Map<string, Promise<number>>();
1005
1006 /**
1007 * Starts the green sweep beside the tick, never in front of it: a landing
1008 * gate takes minutes, and builders are picked meanwhile (#2015). One sweep
1009 * per repository is in flight — a tick that finds one running returns it
1010 * rather than start a second. Never rejects; a failure is logged.
1011 */
1012 export function startSweep(
1013 app: App,
1014 repo: Repo,
1015 sweep: (app: App, repo: Repo) => Promise<number> = sweepGreen,
1016 ): Promise<number> {
1017 const running = sweeping.get(repo.id);
1018 if (running) return running;
1019 const started = sweep(app, repo)
1020 .catch((error) => {
1021 log.error("green sweep failed", {
1022 repo: `${repo.ownerName}/${repo.name}`,
1023 error: String(error),
1024 });
1025 return 0;
1026 })
1027 .finally(() => sweeping.delete(repo.id));
1028 sweeping.set(repo.id, started);
1029 return started;
1030 }
1031
10031032 /** The label every direction issue's title starts with. Read elsewhere to
10041033 * find a repository's standing direction — keep it exactly as it is. */
10051034 const DIRECTION_PREFIX = "Direction: ";
@@ -1045,11 +1074,15 @@ export function ensureDirectionIssue(
10451074 }
10461075
10471076 /** Every repository that runs on its own, one turn each. */
1048 export async function autonomyTick(app: App, now = new Date()): Promise<void> {
1077 export async function autonomyTick(
1078 app: App,
1079 now = new Date(),
1080 sweep: (app: App, repo: Repo) => Promise<number> = sweepGreen,
1081 ): Promise<void> {
10491082 if (crewOffInPreview()) return;
10501083 for (const repo of listAutonomousRepos(app.db)) {

Fine. void startSweep is deliberate, and failures are logged inside startSweep. Later, driveRepo could read the sweep's result, so a landing frees a slot sooner.

10511084 try {
1052 await sweepGreen(app, repo);
1085 void startSweep(app, repo, sweep);
10531086 const current = getRepoById(app.db, repo.id) ?? repo;
10541087 const did = await driveRepo(app, current, now);
10551088 // Every tick says what it did and why, so an app that sits still is
test/autonomy_sweep.test.ts added +129
@@ -0,0 +1,129 @@
1 /**
2 * Landing never stalls the crew (#2015): the green sweep runs beside the
3 * tick, not in front of it. A landing gate takes minutes; the tick picks a
4 * builder meanwhile, and a second tick during one sweep starts no second.
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 { addToCrew, createAgent } from "../src/actions/crew.ts";
12 import { openIssue } from "../src/actions/issues.ts";
13 import { commitToBranch, createRepo, updateRepoSettings } from "../src/actions/repos.ts";
14 import { getIssue, type Issue } from "../src/core/issues.ts";
15 import { getRepoById, type Repo } from "../src/core/repos.ts";
16 import { createUser, type User } from "../src/core/users.ts";
17 import { autonomyTick, startSweep } from "../src/crew/autonomy.ts";
18
19 /** A sweep whose landing gate takes 10 s, released early by the test. */
20 function slowSweep() {
21 let calls = 0;
22 const pending: Array<() => void> = [];
23 const sweep = (_app: App, _repo: Repo) => {
24 calls++;
25 return new Promise<number>((resolve) => {
26 const timer = setTimeout(() => resolve(1), 10_000);
27 pending.push(() => {
28 clearTimeout(timer);
29 resolve(1);
30 });
31 });
32 };
33 return {
34 sweep,
35 calls: () => calls,
36 release: () => {
37 for (const done of pending.splice(0)) done();
38 },
39 };
40 }
41
42 describe("the green sweep runs beside the tick", () => {
43 let dataDir: string;
44 let app: App;
45 let alice: User;
46 let repo: Repo;
47 let issue: Issue;
48 const fresh = () => getRepoById(app.db, repo.id)!;
49
50 beforeAll(async () => {
51 dataDir = await mkdtemp(join(tmpdir(), "forge-sweep-"));
52 app = createApp({ dataDir, port: 0 });
53 const a = await createUser(app.db, {
54 trusted: true,
55 username: "alice",
56 email: "[email protected]",
57 password: "a-good-passphrase",
58 adminIfFirst: true,
59 });
60 if (!a.ok) throw new Error(a.error);
61 alice = a.value;
62 const made = await createRepo(app, alice, { name: "engine", visibility: "private" });
63 if (!made.ok) throw new Error(String(made.error));
64 repo = made.value;
65 const seeded = await commitToBranch(app, alice, repo, {
66 message: "Start",
67 files: [{ path: "README.md", content: "# engine\n" }],
68 });
69 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
70 const agent = await createAgent(app, alice, { username: "mason" });
71 if (!agent.ok) throw new Error(String(agent.error));
72 expect(addToCrew(app, alice, repo, "mason").ok).toBe(true);
73 const opened = openIssue(app, alice, repo, { title: "Renderer" });
74 if (!opened.ok) throw new Error(String(opened.error));
75 issue = opened.value;
76 const on = await updateRepoSettings(app, alice, repo, {
77 autonomy: "autonomous",
78 autonomyDirection: "Pull the engine out of the game.",
79 autonomyCeilingUsd: 10,
80 autonomySlots: 1,
81 autonomyBy: alice.id,
82 });
83 expect(on.ok).toBe(true);
84 });
85
86 afterAll(async () => {
87 app.close();
88 await rm(dataDir, { recursive: true, force: true });
89 });
90
91 test("a landing gate that takes 10 s does not delay the tick's pick; two ticks start one sweep", async () => {
92 const slow = slowSweep();
93 const started = Date.now();
94 await autonomyTick(app, new Date(), slow.sweep);
95 expect(Date.now() - started).toBeLessThan(5_000);
96 expect(slow.calls()).toBe(1);
97 expect(getIssue(app.db, repo.id, issue.number)!.assigneeId).not.toBeNull();
98
99 // A second tick while the first sweep is still landing joins it.
100 await autonomyTick(app, new Date(), slow.sweep);
101 expect(slow.calls()).toBe(1);
102
103 // Once it settles, the next tick sweeps again.
104 slow.release();
105 await Bun.sleep(0);
106 await autonomyTick(app, new Date(), slow.sweep);
107 expect(slow.calls()).toBe(2);
108 slow.release();
109 });
110
111 test("a sweep in flight is shared, and a failed one is logged and cleared", async () => {
112 const slow = slowSweep();
113 const first = startSweep(app, fresh(), slow.sweep);
114 expect(startSweep(app, fresh(), slow.sweep)).toBe(first);
115 slow.release();
116 expect(await first).toBe(1);
117
118 const failed = startSweep(app, fresh(), async () => {
119 throw new Error("gate fell over");
120 });
121 expect(await failed).toBe(0);
122 const again = slowSweep();
123 const next = startSweep(app, fresh(), again.sweep);
124 expect(next).not.toBe(failed);
125 expect(again.calls()).toBe(1);
126 again.release();
127 await next;
128 });
129 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs