e71d6c0
@@ -139,6 +139,10 @@
export async function runClaude(input: ClaudeInput): Promise<ClaudeOutcome> {
    stdin: "pipe",
    stdout: "pipe",
    stderr: "pipe",
    // Its own session, so a cancel can reach whatever it spawned in turn
    // (a tool call left running) and not just the agent itself: a signal
    // to one pid never touches a grandchild holding stdout open.
    detached: true,
  });
  // What the agent hears: the prompt first, then whatever a person says
  // while it works. Each is a user turn; the session ends when the forge
@@ -243,13 +247,23 @@
export async function runClaude(input: ClaudeInput): Promise<ClaudeOutcome> {
    // `user` records are tool results: long and mostly file contents.
  };
  // Signals the whole process group (a negative pid), so anything the
  // agent spawned in turn goes with it; falls back to the process alone
  // if the group is already gone.
  const signalGroup = (sig: "SIGTERM" | "SIGKILL"): void => {
    try {
      process.kill(-proc.pid, sig);
    } catch {
      proc.kill(sig);
    }
  };
  let why: "timeout" | "cancelled" | undefined;
  const stopChild = (): void => {
    closeInput();
    proc.kill("SIGTERM");
    signalGroup("SIGTERM");
    setTimeout(() => {
      if (proc.exitCode === null && proc.signalCode === null)
        proc.kill("SIGKILL");
        signalGroup("SIGKILL");
    }, 5_000);
  };
  const timer = setTimeout(() => {
@@ -11,6 +11,9 @@
import { join } from "node:path";
 */
describe("a pull closed under a running agent", () => {
  test("the run is cancelled, and its clone goes after it, not before", async () => {
    // Cancellation must beat the fake agent's think by a wide margin, not
    // by luck: the think, the run timeout and this test's own deadline are
    // three different numbers, none of them close to another.
    const { createApp } = await import("../src/app.ts");
    const { serve } = await import("../src/http/server.ts");
    const { startCrew } = await import("../src/crew/dispatch.ts");
@@ -28,7 +31,9 @@
describe("a pull closed under a running agent", () => {
    const dataDir = await mkdtemp(join(tmpdir(), "forge-landing-"));
    const app = createApp({ dataDir, port: 0 });
    const servers = await serve(app);
    // An agent that settles in for a long think.
    // An agent that settles in for a long think — long enough that it
    // cannot possibly finish on its own before the test does, so the only
    // way the run ends is by being cancelled.
    const fake = join(dataDir, "fake-claude");
    writeFileSync(
      fake,
@@ -36,7 +41,7 @@
describe("a pull closed under a running agent", () => {
if [ "$1" = "--version" ]; then echo "2.1.0 (fake)"; exit 0; fi
IFS= read -r first
echo '{"type":"system","subtype":"init","session_id":"s3","model":"fake"}'
sleep 60
sleep 1800
`,
    );
    chmodSync(fake, 0o755);
@@ -46,12 +51,14 @@
sleep 60
      binary: fake,
      concurrency: 1,
      budgetUsd: 1,
      runTimeoutMs: 60_000,
      // Far past anything this test waits for: the run's own timeout must
      // never be what ends it here, only the pull closing.
      runTimeoutMs: 10 * 60_000,
    });
    const until = async <T>(
      read: () => T | undefined,
      what: string,
      ms = 20_000,
      ms = 10_000,
    ): Promise<T> => {
      const deadline = Date.now() + ms;
      while (Date.now() < deadline) {
@@ -124,20 +131,29 @@
sleep 60
      const clone = workdirFor(dataDir, bot, repo, 1);
      await until(() => (existsSync(clone) ? true : undefined), "the clone");
      // The pull closes under it.
      // The pull closes under it. Cancelling is a signal, not a wait for
      // the agent to notice on its own — it must land in a few seconds,
      // not sometime before the fake agent's 1800-second think ends.
      const closedAt = Date.now();
      expect(setIssueState(app, alice, repo, asIssue, "closed").ok).toBe(true);
      const ended = await until(() => {
        const r = listRuns(app.db, repo.id, { kind: "agent" }).find(
          (x) => x.id === run.id,
        );
        return r && r.state !== "running" ? r : undefined;
      }, "the run's end");
      const ended = await until(
        () => {
          const r = listRuns(app.db, repo.id, { kind: "agent" }).find(
            (x) => x.id === run.id,
          );
          return r && r.state !== "running" ? r : undefined;
        },
        "the run's end",
        5_000,
      );
      expect(Date.now() - closedAt).toBeLessThan(5_000);
      expect(ended.state).toBe("cancelled");
      expect(ended.summary).toBe("the pull closed");
      // The clone went with the run, once the run was over.
      await until(
        () => (existsSync(clone) ? undefined : true),
        "the clone's end",
        5_000,
      );
      const log = readFileSync(join(dataDir, "runs", `${run.id}.log`), "utf8");
      expect(log).not.toContain("the run failed");
@@ -154,5 +170,5 @@
sleep 60
      app.close();
      await rm(dataDir, { recursive: true, force: true });
    }
  });
  }, 25_000);
});
		@@ -139,6 +139,10 @@ export async function runClaude(input: ClaudeInput): Promise<ClaudeOutcome> {
139	139	stdin: "pipe",
140	140	stdout: "pipe",
141	141	stderr: "pipe",
	142	// Its own session, so a cancel can reach whatever it spawned in turn
	143	// (a tool call left running) and not just the agent itself: a signal
	144	// to one pid never touches a grandchild holding stdout open.
	145	detached: true,
142	146	});
143	147	// What the agent hears: the prompt first, then whatever a person says
144	148	// while it works. Each is a user turn; the session ends when the forge
		@@ -243,13 +247,23 @@ export async function runClaude(input: ClaudeInput): Promise<ClaudeOutcome> {
243	247	// `user` records are tool results: long and mostly file contents.
244	248	};
245	249
	250	// Signals the whole process group (a negative pid), so anything the
	251	// agent spawned in turn goes with it; falls back to the process alone
	252	// if the group is already gone.
	253	const signalGroup = (sig: "SIGTERM" \| "SIGKILL"): void => {
	254	try {
	255	process.kill(-proc.pid, sig);
	256	} catch {
	257	proc.kill(sig);
	258	}
	259	};
246	260	let why: "timeout" \| "cancelled" \| undefined;
247	261	const stopChild = (): void => {
248	262	closeInput();
249		proc.kill("SIGTERM");
	263	signalGroup("SIGTERM");
250	264	setTimeout(() => {
251	265	if (proc.exitCode === null && proc.signalCode === null)
252		proc.kill("SIGKILL");
	266	signalGroup("SIGKILL");
253	267	}, 5_000);
254	268	};
255	269	const timer = setTimeout(() => {
		@@ -11,6 +11,9 @@ import { join } from "node:path";
11	11	*/
12	12	describe("a pull closed under a running agent", () => {
13	13	test("the run is cancelled, and its clone goes after it, not before", async () => {
	14	// Cancellation must beat the fake agent's think by a wide margin, not
	15	// by luck: the think, the run timeout and this test's own deadline are
	16	// three different numbers, none of them close to another.
14	17	const { createApp } = await import("../src/app.ts");
15	18	const { serve } = await import("../src/http/server.ts");
16	19	const { startCrew } = await import("../src/crew/dispatch.ts");
		@@ -28,7 +31,9 @@ describe("a pull closed under a running agent", () => {
28	31	const dataDir = await mkdtemp(join(tmpdir(), "forge-landing-"));
29	32	const app = createApp({ dataDir, port: 0 });
30	33	const servers = await serve(app);
31		// An agent that settles in for a long think.
	34	// An agent that settles in for a long think — long enough that it
	35	// cannot possibly finish on its own before the test does, so the only
	36	// way the run ends is by being cancelled.
32	37	const fake = join(dataDir, "fake-claude");
33	38	writeFileSync(
34	39	fake,
		@@ -36,7 +41,7 @@ describe("a pull closed under a running agent", () => {
36	41	if [ "$1" = "--version" ]; then echo "2.1.0 (fake)"; exit 0; fi
37	42	IFS= read -r first
38	43	echo '{"type":"system","subtype":"init","session_id":"s3","model":"fake"}'
39		sleep 60
	44	sleep 1800
40	45	`,
41	46	);
42	47	chmodSync(fake, 0o755);
		@@ -46,12 +51,14 @@ sleep 60
46	51	binary: fake,
47	52	concurrency: 1,
48	53	budgetUsd: 1,
49		runTimeoutMs: 60_000,
	54	// Far past anything this test waits for: the run's own timeout must
	55	// never be what ends it here, only the pull closing.
	56	runTimeoutMs: 10 * 60_000,
50	57	});
51	58	const until = async <T>(
52	59	read: () => T \| undefined,
53	60	what: string,
54		ms = 20_000,
	61	ms = 10_000,
55	62	): Promise<T> => {
56	63	const deadline = Date.now() + ms;
57	64	while (Date.now() < deadline) {
		@@ -124,20 +131,29 @@ sleep 60
124	131	const clone = workdirFor(dataDir, bot, repo, 1);
125	132	await until(() => (existsSync(clone) ? true : undefined), "the clone");
126	133
127		// The pull closes under it.
	134	// The pull closes under it. Cancelling is a signal, not a wait for
	135	// the agent to notice on its own — it must land in a few seconds,
	136	// not sometime before the fake agent's 1800-second think ends.
	137	const closedAt = Date.now();
128	138	expect(setIssueState(app, alice, repo, asIssue, "closed").ok).toBe(true);
129		const ended = await until(() => {
130		const r = listRuns(app.db, repo.id, { kind: "agent" }).find(
131		(x) => x.id === run.id,
132		);
133		return r && r.state !== "running" ? r : undefined;
134		}, "the run's end");
	139	const ended = await until(
	140	() => {
	141	const r = listRuns(app.db, repo.id, { kind: "agent" }).find(
	142	(x) => x.id === run.id,
	143	);
	144	return r && r.state !== "running" ? r : undefined;
	145	},
	146	"the run's end",
	147	5_000,
	148	);
	149	expect(Date.now() - closedAt).toBeLessThan(5_000);
135	150	expect(ended.state).toBe("cancelled");
136	151	expect(ended.summary).toBe("the pull closed");
137	152	// The clone went with the run, once the run was over.
138	153	await until(
139	154	() => (existsSync(clone) ? undefined : true),
140	155	"the clone's end",
	156	5_000,
141	157	);
142	158	const log = readFileSync(join(dataDir, "runs", `${run.id}.log`), "utf8");
143	159	expect(log).not.toContain("the run failed");
		@@ -154,5 +170,5 @@ sleep 60
154	170	app.close();
155	171	await rm(dataDir, { recursive: true, force: true });
156	172	}
157		});
	173	}, 25_000);
158	174	});