Report a failed agent's stderr, and stop a flaky test counting a restart

apps/connector/src/acp.test.ts

@@ -155,3 +155,23 @@ test.skipIf(process.platform === "win32")(
 		expect(starts).toEqual([{ how: "exit 127", stderr: "agent: no credentials" }, undefined]);
 	},
 );
+
+test.skipIf(process.platform === "win32")(
+	"an agent that exits before its stderr reaches us still reports what it printed",
+	async () => {
+		let starts: unknown[] = [];
+		// The agent exits at once; a process it left behind prints after the exit.
+		let failed = await runAgent({
+			command: ["/bin/sh", "-c", "(sleep 0.3; echo 'agent: no credentials' >&2) & exit 127"],
+			cwd: process.cwd(),
+			prompt: "Investigate",
+			mcpServers: [],
+			signal: new AbortController().signal,
+			onUpdate: () => {},
+			permission: async () => undefined,
+			onStart: (failure?: unknown) => void starts.push(failure),
+		}).catch(error => error);
+		expect(failed).toBeInstanceOf(Error);
+		expect(starts).toEqual([{ how: "exit 127", stderr: "agent: no credentials" }]);
+	},
+);

apps/connector/src/acp.ts

@@ -85,6 +85,11 @@ function startAgent(
 			(code, signal) => resolve(ended ??= signal ? `signal ${signal}` : `exit ${code}`),
 		);
 	});
+	// "exit" can come before the last of its stderr is read; "close" follows once that stream ends.
+	let closed = new Promise(resolve => {
+		child.once("error", resolve);
+		child.once("close", resolve);
+	});
 	let connection = app.connect(ndJsonStream(
 		Writable.toWeb(child.stdin),
 		Readable.toWeb(child.stdout) as unknown as ReadableStream<Uint8Array>,
@@ -95,8 +100,12 @@ function startAgent(
 		connection,
 		kill,
 		async failure(error: unknown): Promise<AgentFailure> {
-			// Its exit, and the rest of its stderr, may come just after the failure they caused.
-			await Promise.race([exited, new Promise(resolve => setTimeout(resolve, 1000))]);
+			// Its exit, and the rest of its stderr, may come just after the failure they caused. A
+			// process it left holding stderr open is not waited on past a second.
+			await Promise.race([
+				exited.then(() => closed),
+				new Promise(resolve => setTimeout(resolve, 1000)),
+			]);
 			let line = head.split("\n").map(text => text.trim()).find(Boolean);
 			return {
 				how: ended ?? (error instanceof Error ? error.message : String(error)),

apps/connector/src/planner.test.ts

@@ -581,15 +581,23 @@ test.skipIf(process.platform === "win32")(
 		let directory = await mkdtemp(join(scratch, "long-line-"));
 		let script = join(directory, "backend.ts");
 		// The first 8 characters of the credential end a 65,545-character piece without a newline;
-		// the rest comes after a pause, then a normal line.
+		// the rest comes after a pause, then a normal line. Only the first start prints it: the
+		// Planner starts again on a backoff that may be a few milliseconds, and a second start
+		// printing the line before the test stops it would be a second, legitimate drop.
 		await writeFile(
 			script,
 			[
+				"import { existsSync, writeFileSync } from 'node:fs';",
+				`let ran = ${JSON.stringify(join(directory, "ran"))};`,
+				"if (existsSync(ran)) setInterval(() => {}, 1000);",
+				"else {",
+				"writeFileSync(ran, '');",
 				"let token = process.env.CHOPIN_PLANNER_TOKEN;",
 				"process.stderr.write(`${'x'.repeat(65536)} ${token.slice(0, 8)}`, () => setTimeout(() => {",
 				"\tprocess.stderr.write(`${token.slice(8)}\\nnext CHOPIN_PLANNER_TOKEN=${token}\\n`,",
 				"\t\t() => process.exit(127));",
 				"}, 300));",
+				"}",
 				"",
 			].join("\n"),
 		);