beaf364
@@ -36,7 +36,7 @@
import { appendRunLog } from "../runs/log.ts";
import { exportTree, readFileAt } from "../apps/checkout.ts";
import { parseCheck } from "../apps/manifest.ts";
import { childEnv, runStep } from "../apps/process.ts";
import { parseBunTestFailure } from "./bun-test.ts";
import { summariseCheckFailure } from "./summary.ts";
import type { CheckDetail } from "../core/runs.ts";
const log = createLog("checks");
@@ -347,24 +347,29 @@
export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
      }
      // The check step's own lines are kept beside what `write` already
      // logs, so a failure can be summarised once here — never by reading
      // the log back, on this run or any later render of it.
      // the log back, on this run or any later render of it. `runStep`
      // echoes the command as its first line before any output of the
      // command's own; that echo is not output to summarise from.
      const checkLines: string[] = [];
      let sawEcho = false;
      const ran = await runStep("check", manifest.run, {
        cwd: dir,
        env,
        log: (line) => {
          checkLines.push(line);
          if (sawEcho) checkLines.push(line);
          sawEcho = true;
          write(line);
        },
        signal,
        timeoutMs: manifest.timeout * 1000,
      });
      if (!ran.ok)
        return {
          ok: false,
          error: ran.error,
          detail: parseBunTestFailure(checkLines),
        };
      if (!ran.ok) {
        const { summary, detail } = summariseCheckFailure(
          checkLines,
          ran.error,
        );
        return { ok: false, error: summary, detail };
      }
      return { ok: true };
    } finally {
      if (existsSync(dir)) rmSync(dir, { recursive: true, force: true });
@@ -0,0 +1,55 @@
/**
 * What a failed check's summary says first: bun test's own name and counts
 * when the check step's output carried them, the output's own last word
 * when it did not, and the bare exit code only when the output said
 * nothing at all. Plain data in (the check step's own lines, and the
 * error `runStep` already gives up), plain data out — never a re-read of
 * the run's log, and never a guess at *why* a test failed.
 */
import type { CheckDetail } from "../core/runs.ts";
import { parseBunTestFailure } from "./bun-test.ts";
/** A summary is one line on a card; a name from a subprocess is bounded to fit it. */
const SUMMARY_MAX = 200;
function truncate(text: string): string {
  return text.length > SUMMARY_MAX
    ? `${text.slice(0, SUMMARY_MAX - 1)}…`
    : text;
}
export interface CheckFailure {
  /** The one line a card shows, already bounded. */
  readonly summary: string;
  /** `bun test`'s own counts and names, when the output was its shape. */
  readonly detail: CheckDetail | undefined;
}
/**
 * `lines` is the check step's own output, nothing from `install` mixed in
 * (`checks/service.ts` keeps them apart already). `fallback` is what
 * `runStep` says when a step exits non-zero — "check exited with code 1",
 * sometimes with a reason it found on its own — used only when this
 * output named nothing better, so a truthful "no output at all" is never
 * dressed up as more than it is.
 */
export function summariseCheckFailure(
  lines: readonly string[],
  fallback: string,
): CheckFailure {
  const detail = parseBunTestFailure(lines);
  if (detail !== undefined) {
    const total = detail.passed + detail.failed;
    const name = detail.names[0] ?? "";
    return {
      summary: truncate(`${detail.failed} of ${total} tests — "${name}"`),
      detail,
    };
  }
  for (let i = lines.length - 1; i >= 0; i--) {
    const line = lines[i]!.trim();
    if (line !== "") return { summary: truncate(line), detail: undefined };
  }
  return { summary: fallback, detail: undefined };
}
@@ -546,8 +546,13 @@
function parkedReason(
  head: string,
): string {
  const what = kind === "deploy" ? "the preview's deploy" : "the check";
  if (judged.cause !== "environment")
    return `${what} failed and the decider saw no point in another try`;
  if (judged.cause !== "environment") {
    const named =
      judged.failingTest === undefined
        ? ""
        : ` ("${judged.failingTest}")`;
    return `${what} failed${named} and the decider saw no point in another try`;
  }
  return retriedChecks.has(head)
    ? `${what} failed in the place it ran, not in the change, and failed the same way when it was run again`
    : `${what} failed in the place it ran, not in the change, and it could not be run again`;
@@ -567,6 +572,8 @@
export interface CheckJudgement {
  readonly retry: "again" | "stop";
  /** Whose fault the failure is, from the tail of the check's log. */
  readonly cause: "change" | "environment" | "base";
  /** The failing test's own name, when the run's output named one. */
  readonly failingTest: string | undefined;
}
const RETRY_OPTIONS = ["again", "stop"];
@@ -589,9 +596,10 @@
export async function judgeFailedCheck(
  runId: string,
  since: string,
): Promise<CheckJudgement> {
  if (app.config.decider === undefined)
    return { retry: "again", cause: "change" };
  const run = getRun(app.db, runId);
  const failingTest = run?.checkDetail?.names[0];
  if (app.config.decider === undefined)
    return { retry: "again", cause: "change", failingTest };
  const tail = readRunLog(app.config.dataDir, runId, 8 * 1024).lines.slice(-25);
  const decided = await decideAndRecord(
    app,
@@ -655,7 +663,7 @@
export async function judgeFailedCheck(
      p: decided.answers.retry.p.toFixed(2),
      cause,
    });
  return { retry, cause };
  return { retry, cause, failingTest };
}
/** When a person last said anything on the conversation, or its start. */
@@ -12,6 +12,7 @@
import { createApp, type App } from "../src/app.ts";
import { openPull } from "../src/actions/pulls.ts";
import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts";
import { parseBunTestFailure } from "../src/checks/bun-test.ts";
import { summariseCheckFailure } from "../src/checks/summary.ts";
import {
  pickNext,
  startChecks,
@@ -75,6 +76,59 @@
describe("parseBunTestFailure: a convenience over one known runner, not a genera
  });
});
describe("summariseCheckFailure: what a failed check's summary says first", () => {
  test("real bun test output with one failure yields the name and the counts", () => {
    const lines = [
      "bun test v1.3.14 (0d9b296a)",
      "",
      "check.test.ts:",
      "1 tests failed:",
      '(fail) the forge\'s own restart never reads as this deploy\'s failure (issue #255) > an install still in flight when the runtime stops finishes cancelled, not failed [30000.49ms]',
      "  ^ this test timed out after 30000ms.",
      "",
      " 409 pass",
      " 1 fail",
      "Ran 410 tests across 1 file. [30.05s]",
    ];
    expect(summariseCheckFailure(lines, "check exited with code 1")).toEqual({
      summary:
        '1 of 410 tests — "the forge\'s own restart never reads as this deploy\'s failure (issue #255) > an install still in flight when the runtime stops finishes cancelled, not failed"',
      detail: {
        passed: 409,
        failed: 1,
        names: [
          "the forge's own restart never reads as this deploy's failure (issue #255) > an install still in flight when the runtime stops finishes cancelled, not failed",
        ],
      },
    });
  });
  test("output with no bun test shape yields the last non-empty line, not the bare exit code", () => {
    const lines = ["src/foo.ts(10,5): error TS2345: not assignable", ""];
    expect(
      summariseCheckFailure(lines, "check exited with code 2"),
    ).toEqual({
      summary: "src/foo.ts(10,5): error TS2345: not assignable",
      detail: undefined,
    });
  });
  test("no output at all falls back to the bare exit code — nothing was there to name", () => {
    expect(summariseCheckFailure([], "check exited with code 1")).toEqual({
      summary: "check exited with code 1",
      detail: undefined,
    });
  });
  test("a very long name is bounded to one line", () => {
    const long = "x".repeat(500);
    const lines = [`(fail) ${long} [1.00ms]`, " 0 pass", " 1 fail"];
    const { summary } = summariseCheckFailure(lines, "check exited with code 1");
    expect(summary.length).toBeLessThanOrEqual(200);
    expect(summary.endsWith("…")).toBe(true);
  });
});
describe("checks", () => {
  let dataDir: string;
  let app: App;
@@ -245,6 +299,9 @@
describe("checks", () => {
      failed: 2,
      names: ["first failing test", "second failing test"],
    });
    expect(failed.summary).toBe(
      'check failed: 2 of 3 tests — "first failing test"',
    );
  }, 30_000);
  test("a fork's pull is checked at the base's snapshot, and follows the fork's pushes", async () => {
@@ -389,7 +389,12 @@
describe("the decision ledger", () => {
    claimRun(app.db, run.id);
    finishRun(app.db, run.id, {
      state: "failed",
      summary: "expect(received).toBe(expected): 1 of 274",
      summary: 'check failed: 1 of 274 tests — "renders the broken widget"',
      checkDetail: {
        passed: 273,
        failed: 1,
        names: ["renders the broken widget"],
      },
    });
    script["retry"] = "stop";
    script["cause"] = "change";
@@ -404,10 +409,13 @@
describe("the decision ledger", () => {
        }),
      ).toBe("parked");
      expect(getPull(app.db, repo.id, pull.number)?.state).toBe("closed");
      const comments = listComments(app.db, pull.id).map((c) => c.body);
      expect(
        listComments(app.db, pull.id).some((c) =>
          c.body.includes("saw no point in another try"),
        ),
        comments.some((b) => b.includes("saw no point in another try")),
      ).toBe(true);
      // The red test's own name, not just "the check failed".
      expect(
        comments.some((b) => b.includes('"renders the broken widget"')),
      ).toBe(true);
    } finally {
      delete script["cause"];
		@@ -36,7 +36,7 @@ import { appendRunLog } from "../runs/log.ts";
36	36	import { exportTree, readFileAt } from "../apps/checkout.ts";
37	37	import { parseCheck } from "../apps/manifest.ts";
38	38	import { childEnv, runStep } from "../apps/process.ts";
39		import { parseBunTestFailure } from "./bun-test.ts";
	39	import { summariseCheckFailure } from "./summary.ts";
40	40	import type { CheckDetail } from "../core/runs.ts";
41	41
42	42	const log = createLog("checks");
		@@ -347,24 +347,29 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
347	347	}
348	348	// The check step's own lines are kept beside what `write` already
349	349	// logs, so a failure can be summarised once here — never by reading
350		// the log back, on this run or any later render of it.
	350	// the log back, on this run or any later render of it. `runStep`
	351	// echoes the command as its first line before any output of the
	352	// command's own; that echo is not output to summarise from.
351	353	const checkLines: string[] = [];
	354	let sawEcho = false;
352	355	const ran = await runStep("check", manifest.run, {
353	356	cwd: dir,
354	357	env,
355	358	log: (line) => {
356		checkLines.push(line);
	359	if (sawEcho) checkLines.push(line);
	360	sawEcho = true;
357	361	write(line);
358	362	},
359	363	signal,
360	364	timeoutMs: manifest.timeout * 1000,
361	365	});
362		if (!ran.ok)
363		return {
364		ok: false,
365		error: ran.error,
366		detail: parseBunTestFailure(checkLines),
367		};
	366	if (!ran.ok) {
	367	const { summary, detail } = summariseCheckFailure(
	368	checkLines,
	369	ran.error,
	370	);
	371	return { ok: false, error: summary, detail };
	372	}
368	373	return { ok: true };
369	374	} finally {
370	375	if (existsSync(dir)) rmSync(dir, { recursive: true, force: true });
		@@ -0,0 +1,55 @@
	1	/**
	2	* What a failed check's summary says first: bun test's own name and counts
	3	* when the check step's output carried them, the output's own last word
	4	* when it did not, and the bare exit code only when the output said
	5	* nothing at all. Plain data in (the check step's own lines, and the
	6	* error `runStep` already gives up), plain data out — never a re-read of
	7	* the run's log, and never a guess at why a test failed.
	8	*/
	9
	10	import type { CheckDetail } from "../core/runs.ts";
	11	import { parseBunTestFailure } from "./bun-test.ts";
	12
	13	/** A summary is one line on a card; a name from a subprocess is bounded to fit it. */
	14	const SUMMARY_MAX = 200;
	15
	16	function truncate(text: string): string {
	17	return text.length > SUMMARY_MAX
	18	? `${text.slice(0, SUMMARY_MAX - 1)}…`
	19	: text;
	20	}
	21
	22	export interface CheckFailure {
	23	/** The one line a card shows, already bounded. */
	24	readonly summary: string;
	25	/** `bun test`'s own counts and names, when the output was its shape. */
	26	readonly detail: CheckDetail \| undefined;
	27	}
	28
	29	/**
	30	* `lines` is the check step's own output, nothing from `install` mixed in
	31	* (`checks/service.ts` keeps them apart already). `fallback` is what
	32	* `runStep` says when a step exits non-zero — "check exited with code 1",
	33	* sometimes with a reason it found on its own — used only when this
	34	* output named nothing better, so a truthful "no output at all" is never
	35	* dressed up as more than it is.
	36	*/
	37	export function summariseCheckFailure(
	38	lines: readonly string[],
	39	fallback: string,
	40	): CheckFailure {
	41	const detail = parseBunTestFailure(lines);
	42	if (detail !== undefined) {
	43	const total = detail.passed + detail.failed;
	44	const name = detail.names[0] ?? "";
	45	return {
	46	summary: truncate(`${detail.failed} of ${total} tests — "${name}"`),
	47	detail,
	48	};
	49	}
	50	for (let i = lines.length - 1; i >= 0; i--) {
	51	const line = lines[i]!.trim();
	52	if (line !== "") return { summary: truncate(line), detail: undefined };
	53	}
	54	return { summary: fallback, detail: undefined };
	55	}
		@@ -546,8 +546,13 @@ function parkedReason(
546	546	head: string,
547	547	): string {
548	548	const what = kind === "deploy" ? "the preview's deploy" : "the check";
549		if (judged.cause !== "environment")
550		return `${what} failed and the decider saw no point in another try`;
	549	if (judged.cause !== "environment") {
	550	const named =
	551	judged.failingTest === undefined
	552	? ""
	553	: ` ("${judged.failingTest}")`;
	554	return `${what} failed${named} and the decider saw no point in another try`;
	555	}
551	556	return retriedChecks.has(head)
552	557	? `${what} failed in the place it ran, not in the change, and failed the same way when it was run again`
553	558	: `${what} failed in the place it ran, not in the change, and it could not be run again`;
		@@ -567,6 +572,8 @@ export interface CheckJudgement {
567	572	readonly retry: "again" \| "stop";
568	573	/** Whose fault the failure is, from the tail of the check's log. */
569	574	readonly cause: "change" \| "environment" \| "base";
	575	/** The failing test's own name, when the run's output named one. */
	576	readonly failingTest: string \| undefined;
570	577	}
571	578
572	579	const RETRY_OPTIONS = ["again", "stop"];
		@@ -589,9 +596,10 @@ export async function judgeFailedCheck(
589	596	runId: string,
590	597	since: string,
591	598	): Promise<CheckJudgement> {
592		if (app.config.decider === undefined)
593		return { retry: "again", cause: "change" };
594	599	const run = getRun(app.db, runId);
	600	const failingTest = run?.checkDetail?.names[0];
	601	if (app.config.decider === undefined)
	602	return { retry: "again", cause: "change", failingTest };
595	603	const tail = readRunLog(app.config.dataDir, runId, 8 * 1024).lines.slice(-25);
596	604	const decided = await decideAndRecord(
597	605	app,
		@@ -655,7 +663,7 @@ export async function judgeFailedCheck(
655	663	p: decided.answers.retry.p.toFixed(2),
656	664	cause,
657	665	});
658		return { retry, cause };
	666	return { retry, cause, failingTest };
659	667	}
660	668
661	669	/** When a person last said anything on the conversation, or its start. */
		@@ -12,6 +12,7 @@ import { createApp, type App } from "../src/app.ts";
12	12	import { openPull } from "../src/actions/pulls.ts";
13	13	import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts";
14	14	import { parseBunTestFailure } from "../src/checks/bun-test.ts";
	15	import { summariseCheckFailure } from "../src/checks/summary.ts";
15	16	import {
16	17	pickNext,
17	18	startChecks,
		@@ -75,6 +76,59 @@ describe("parseBunTestFailure: a convenience over one known runner, not a genera
75	76	});
76	77	});
77	78
	79	describe("summariseCheckFailure: what a failed check's summary says first", () => {
	80	test("real bun test output with one failure yields the name and the counts", () => {
	81	const lines = [
	82	"bun test v1.3.14 (0d9b296a)",
	83	"",
	84	"check.test.ts:",
	85	"1 tests failed:",
	86	'(fail) the forge\'s own restart never reads as this deploy\'s failure (issue #255) > an install still in flight when the runtime stops finishes cancelled, not failed [30000.49ms]',
	87	" ^ this test timed out after 30000ms.",
	88	"",
	89	" 409 pass",
	90	" 1 fail",
	91	"Ran 410 tests across 1 file. [30.05s]",
	92	];
	93	expect(summariseCheckFailure(lines, "check exited with code 1")).toEqual({
	94	summary:
	95	'1 of 410 tests — "the forge\'s own restart never reads as this deploy\'s failure (issue #255) > an install still in flight when the runtime stops finishes cancelled, not failed"',
	96	detail: {
	97	passed: 409,
	98	failed: 1,
	99	names: [
	100	"the forge's own restart never reads as this deploy's failure (issue #255) > an install still in flight when the runtime stops finishes cancelled, not failed",
	101	],
	102	},
	103	});
	104	});
	105
	106	test("output with no bun test shape yields the last non-empty line, not the bare exit code", () => {
	107	const lines = ["src/foo.ts(10,5): error TS2345: not assignable", ""];
	108	expect(
	109	summariseCheckFailure(lines, "check exited with code 2"),
	110	).toEqual({
	111	summary: "src/foo.ts(10,5): error TS2345: not assignable",
	112	detail: undefined,
	113	});
	114	});
	115
	116	test("no output at all falls back to the bare exit code — nothing was there to name", () => {
	117	expect(summariseCheckFailure([], "check exited with code 1")).toEqual({
	118	summary: "check exited with code 1",
	119	detail: undefined,
	120	});
	121	});
	122
	123	test("a very long name is bounded to one line", () => {
	124	const long = "x".repeat(500);
	125	const lines = [`(fail) ${long} [1.00ms]`, " 0 pass", " 1 fail"];
	126	const { summary } = summariseCheckFailure(lines, "check exited with code 1");
	127	expect(summary.length).toBeLessThanOrEqual(200);
	128	expect(summary.endsWith("…")).toBe(true);
	129	});
	130	});
	131
78	132	describe("checks", () => {
79	133	let dataDir: string;
80	134	let app: App;
		@@ -245,6 +299,9 @@ describe("checks", () => {
245	299	failed: 2,
246	300	names: ["first failing test", "second failing test"],
247	301	});
	302	expect(failed.summary).toBe(
	303	'check failed: 2 of 3 tests — "first failing test"',
	304	);
248	305	}, 30_000);
249	306
250	307	test("a fork's pull is checked at the base's snapshot, and follows the fork's pushes", async () => {
		@@ -389,7 +389,12 @@ describe("the decision ledger", () => {
389	389	claimRun(app.db, run.id);
390	390	finishRun(app.db, run.id, {
391	391	state: "failed",
392		summary: "expect(received).toBe(expected): 1 of 274",
	392	summary: 'check failed: 1 of 274 tests — "renders the broken widget"',
	393	checkDetail: {
	394	passed: 273,
	395	failed: 1,
	396	names: ["renders the broken widget"],
	397	},
393	398	});
394	399	script["retry"] = "stop";
395	400	script["cause"] = "change";
		@@ -404,10 +409,13 @@ describe("the decision ledger", () => {
404	409	}),
405	410	).toBe("parked");
406	411	expect(getPull(app.db, repo.id, pull.number)?.state).toBe("closed");
	412	const comments = listComments(app.db, pull.id).map((c) => c.body);
407	413	expect(
408		listComments(app.db, pull.id).some((c) =>
409		c.body.includes("saw no point in another try"),
410		),
	414	comments.some((b) => b.includes("saw no point in another try")),
	415	).toBe(true);
	416	// The red test's own name, not just "the check failed".
	417	expect(
	418	comments.some((b) => b.includes('"renders the broken widget"')),
411	419	).toBe(true);
412	420	} finally {
413	421	delete script["cause"];