4d84e9b
@@ -34,6 +34,7 @@
export async function revertPull(
  actor: User | undefined,
  repo: Repo,
  pull: Pull,
  body: string = `Reverts #${pull.number}.`,
): Promise<Result<Pull, RevertPullError>> {
  const writer = requireWriter(app, actor, repo);
  if (!writer.ok) return writer;
@@ -77,7 +78,7 @@
export async function revertPull(
  const opened = await openPull(app, writer.value, repo, {
    title: `Revert "${pull.title}"`,
    body: `Reverts #${pull.number}.`,
    body,
    baseBranch: repo.defaultBranch,
    headBranch: branchName,
    revertsIssueId: pull.id,
@@ -0,0 +1,105 @@
/**
 * The way back from a red main (#6377). When main's check goes red and the
 * commits since its last green are exactly one landed pull, the forge
 * proposes the revert of that pull: one pull, `revertsPullId` set, titled
 * for the pull it takes back and naming the failing test. It is only ever
 * proposed — a person lands it. A red that spans more than one landing (or a
 * push no pull carried) is the alarm's, not this module's, and a landing
 * already reverted, open or shipped or closed, is never proposed twice.
 */
import type { App } from "../app.ts";
import { revertPull } from "../actions/revert.ts";
import { getIssueById } from "../core/issues.ts";
import { getPull, type Pull } from "../core/pulls.ts";
import { getRepoById } from "../core/repos.ts";
import { getUserById } from "../core/users.ts";
import { runGit } from "../git/env.ts";
import { repoDir } from "../git/repos.ts";
import { isSafeOid } from "../git/guard.ts";
import { manifestError, redMain, type RedMain } from "./red_main.ts";
/** The reds being answered now: a check finishing twice at once proposes once. */
const working = new Set<string>();
/** The id of the last passed default-branch check before run `below`, with its commit. */
function greenBefore(app: App, repoId: string, branch: string, below: string): string | undefined {
  return app.db
    .query<{ commit_oid: string }, [string, string, string]>(
      `SELECT commit_oid FROM run
        WHERE repo_id = ?1 AND kind = 'check' AND issue_id IS NULL AND env = ?2
          AND state = 'ok' AND id < ?3
        ORDER BY id DESC LIMIT 1`,
    )
    .get(repoId, branch, below)?.commit_oid;
}
/** Whether any pull, in any state, already takes `pullId` back. */
function everReverted(app: App, pullId: string): boolean {
  return (
    app.db
      .query<{ n: number }, [string]>("SELECT COUNT(*) AS n FROM pull WHERE reverts_issue_id = ?1")
      .get(pullId)!.n > 0
  );
}
/**
 * The one pull landed between the last green and `red`, else undefined:
 * every first-parent commit in that range must be a pull's landing, and
 * there must be exactly one.
 */
async function soleLanding(app: App, red: RedMain, green: string): Promise<Pull | undefined> {
  const repo = getRepoById(app.db, red.repoId);
  if (repo === undefined || !isSafeOid(green) || !isSafeOid(red.commitOid)) return undefined;
  const log = await runGit(["rev-list", "--first-parent", `${green}..${red.commitOid}`], {
    cwd: repoDir(app.reposRoot, repo.ownerName, repo.name),
  });
  if (!log.ok) return undefined;
  const commits = log.value.split("\n").filter(Boolean);
  if (commits.length === 0) return undefined;
  const landed = commits.map((oid) =>
    app.db
      .query<{ number: number }, [string, string, string]>(
        `SELECT i.number FROM pull p JOIN issue i ON i.id = p.issue_id
          WHERE i.repo_id = ?1 AND p.merged_oid = ?2 AND p.base_branch = ?3`,
      )
      .get(repo.id, oid, repo.defaultBranch),
  );
  if (landed.length !== 1 || landed[0] === null || landed[0] === undefined) return undefined;
  return getPull(app.db, repo.id, landed[0].number);
}
/** What the revert pull says besides what it reverts: the red it answers. */
export function revertBody(pull: Pick<Pull, "number">, red: RedMain): string {
  const what =
    red.failing[0] !== undefined
      ? `the failing test is "${red.failing[0]}"${red.failing.length > 1 ? ` (and ${red.failing.length - 1} more)` : ""}`
      
        junipercrew
        18d ago
        
      
      Skip when landed.revertsPullId !== null, so a landed revert is not itself reverted.
    
      : red.summary.replace(/^check failed: /, "");
  return `Reverts #${pull.number}, the one pull landed since main was last green.\n\nMain is red at ${red.commitOid.slice(0, 7)}: ${what}.\n\nProposed by the forge; a person lands it.`;
}
/**
 * Called when a default-branch check of `repoId` has finished: opens the
 * revert pull if the rules above hold. Returns it, or undefined when none
 * was proposed.
 */
export async function proposeRevert(app: App, repoId: string): Promise<Pull | undefined> {
  if (working.has(repoId)) return undefined;
  working.add(repoId);
  try {
    const red = redMain(app.db, repoId);
    if (red === undefined || red.limit || manifestError(red) !== undefined) return undefined;
    const green = greenBefore(app, repoId, red.branch, red.runId);
    if (green === undefined) return undefined;
    const landed = await soleLanding(app, red, green);
    if (landed === undefined || everReverted(app, landed.id)) return undefined;
    const repo = getRepoById(app.db, repoId);
    const owner = repo === undefined ? undefined : getUserById(app.db, repo.ownerId);
    if (repo === undefined || owner === undefined || getIssueById(app.db, landed.id) === undefined)
      
        junipercrew
        18d ago
        
      
      This guard re-reads an issue we just resolved through getPull. It is dead weight; remove it.
    
      return undefined;
    const made = await revertPull(app, owner, repo, landed, revertBody(landed, red));
    return made.ok ? made.value : undefined;
  } finally {
    working.delete(repoId);
  }
}
@@ -55,6 +55,7 @@
import { keepCache, restoreCache } from "./cache.ts";
import { MAX_FILED, clearedFiles, fileOrderDependent } from "./order_dependent.ts";
import { withDisk } from "./disk_red.ts";
import { ASKED, redMain, unheld } from "./red_main.ts";
import { proposeRevert } from "./revert_red.ts";
import { rebasesFirst } from "./finishing_first.ts";
import { IMPACT_FILE, keepImpact, seedImpact } from "./impact-store.ts";
import { getIssueById } from "../core/issues.ts";
@@ -633,6 +634,10 @@
export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
      });
      write(outcome.ok ? `passed in ${seconds}s` : `failed: ${outcome.error}`);
      if (done) announceRun(app, done);
      if (!outcome.ok && run.issueId === null && run.env === repo.defaultBranch)
        proposeRevert(app, repo.id).catch((error) =>
          log.error("revert proposal failed", { error: String(error) }),
        );
    } finally {
      release();
    }
@@ -0,0 +1,122 @@
/**
 * A red main after one landed pull gets one revert pull naming the failing
 * test (#6377); after two landings, or a second tick, none.
 */
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { createApp, type App } from "../src/app.ts";
import { mergePull, openPull } from "../src/actions/pulls.ts";
import { commitToBranch, createRepo } from "../src/actions/repos.ts";
import { proposeRevert } from "../src/checks/revert_red.ts";
import { getPull, listPulls, type Pull } from "../src/core/pulls.ts";
import type { Repo } from "../src/core/repos.ts";
import { createRun } from "../src/core/runs.ts";
import { createUser, type User } from "../src/core/users.ts";
import { resolveOid } from "../src/git/merge.ts";
import { repoDir } from "../src/git/repos.ts";
describe("a red main after one landing proposes its revert", () => {
  let dataDir: string;
  let app: App;
  let alice: User;
  let repo: Repo;
  const land = async (branch: string, content: string): Promise<Pull> => {
    await commitToBranch(app, alice, repo, {
      branch,
      base: "main",
      message: branch,
      files: [{ path: "code.txt", content }],
    });
    const opened = await openPull(app, alice, repo, {
      title: `Change ${branch}`,
      baseBranch: "main",
      headBranch: branch,
    });
    if (!opened.ok) throw new Error(JSON.stringify(opened.error));
    const merged = await mergePull(app, alice, repo, opened.value, "squash");
    if (!merged.ok) throw new Error(JSON.stringify(merged.error));
    return getPull(app.db, repo.id, opened.value.number)!;
  };
  const mainCheck = async (state: "ok" | "failed", names: string[] = []): Promise<void> => {
    const oid = await resolveOid(repoDir(app.reposRoot, repo.ownerName, repo.name), "refs/heads/main");
    const run = createRun(app.db, { repoId: repo.id, kind: "check", env: "main", commitOid: oid ?? "" });
    app.db
      .query("UPDATE run SET state = ?2, summary = ?3, check_detail = ?4, finished_at = ?5 WHERE id = ?1")
      .run(
        run.id,
        state,
        state === "ok" ? "check passed in 1s" : "check failed: 1 test failed",
        state === "ok" ? "" : JSON.stringify({ names }),
        new Date().toISOString(),
      );
  };
  const reverts = (): Pull[] =>
    listPulls(app.db, repo.id, { state: "all" }).filter((p) => p.revertsPullId !== null);
  beforeEach(async () => {
    dataDir = await mkdtemp(join(tmpdir(), "forge-revert-red-"));
    app = createApp({ dataDir, port: 0 });
    const created = await createUser(app.db, {
      origin: "seed",
      trusted: true,
      username: "alice",
      email: "[email protected]",
      password: "a-good-passphrase",
      adminIfFirst: true,
    });
    if (!created.ok) throw new Error(created.error);
    alice = created.value;
    const made = await createRepo(app, alice, { name: "app", visibility: "public" });
    if (!made.ok) throw new Error(String(made.error));
    repo = made.value;
    await commitToBranch(app, alice, repo, {
      branch: "main",
      message: "base",
      files: [{ path: "code.txt", content: "alpha\n" }],
    });
  });
  afterEach(async () => {
    app.close();
    await rm(dataDir, { recursive: true, force: true });
  });
  test("one landing then red opens one revert; a second tick opens none", async () => {
    await mainCheck("ok");
    const landed = await land("one", "beta\n");
    await mainCheck("failed", ["parses the thing"]);
    const first = await proposeRevert(app, repo.id);
    expect(first?.revertsPullId).toBe(landed.id);
    expect(first?.title).toContain(landed.title);
    expect(first?.body).toContain("parses the thing");
    expect(first?.mergedAt).toBeNull();
    expect(reverts()).toHaveLength(1);
    expect(await proposeRevert(app, repo.id)).toBeUndefined();
    expect(reverts()).toHaveLength(1);
  });
  test("two landings since the green open none", async () => {
    await mainCheck("ok");
    await land("one", "beta\n");
    await land("two", "gamma\n");
    await mainCheck("failed", ["parses the thing"]);
    expect(await proposeRevert(app, repo.id)).toBeUndefined();
    expect(reverts()).toHaveLength(0);
  });
  test("a green main opens none", async () => {
    await mainCheck("ok");
    await land("one", "beta\n");
    await mainCheck("ok");
    expect(await proposeRevert(app, repo.id)).toBeUndefined();
  });
});
		@@ -34,6 +34,7 @@ export async function revertPull(
34	34	actor: User \| undefined,
35	35	repo: Repo,
36	36	pull: Pull,
	37	body: string = `Reverts #${pull.number}.`,
37	38	): Promise<Result<Pull, RevertPullError>> {
38	39	const writer = requireWriter(app, actor, repo);
39	40	if (!writer.ok) return writer;
		@@ -77,7 +78,7 @@ export async function revertPull(
77	78
78	79	const opened = await openPull(app, writer.value, repo, {
79	80	title: `Revert "${pull.title}"`,
80		body: `Reverts #${pull.number}.`,
	81	body,
81	82	baseBranch: repo.defaultBranch,
82	83	headBranch: branchName,
83	84	revertsIssueId: pull.id,
		@@ -0,0 +1,105 @@
	1	/**
	2	* The way back from a red main (#6377). When main's check goes red and the
	3	* commits since its last green are exactly one landed pull, the forge
	4	* proposes the revert of that pull: one pull, `revertsPullId` set, titled
	5	* for the pull it takes back and naming the failing test. It is only ever
	6	* proposed — a person lands it. A red that spans more than one landing (or a
	7	* push no pull carried) is the alarm's, not this module's, and a landing
	8	* already reverted, open or shipped or closed, is never proposed twice.
	9	*/
	10
	11	import type { App } from "../app.ts";
	12	import { revertPull } from "../actions/revert.ts";
	13	import { getIssueById } from "../core/issues.ts";
	14	import { getPull, type Pull } from "../core/pulls.ts";
	15	import { getRepoById } from "../core/repos.ts";
	16	import { getUserById } from "../core/users.ts";
	17	import { runGit } from "../git/env.ts";
	18	import { repoDir } from "../git/repos.ts";
	19	import { isSafeOid } from "../git/guard.ts";
	20	import { manifestError, redMain, type RedMain } from "./red_main.ts";
	21
	22	/** The reds being answered now: a check finishing twice at once proposes once. */
	23	const working = new Set<string>();
	24
	25	/** The id of the last passed default-branch check before run `below`, with its commit. */
	26	function greenBefore(app: App, repoId: string, branch: string, below: string): string \| undefined {
	27	return app.db
	28	.query<{ commit_oid: string }, [string, string, string]>(
	29	`SELECT commit_oid FROM run
	30	WHERE repo_id = ?1 AND kind = 'check' AND issue_id IS NULL AND env = ?2
	31	AND state = 'ok' AND id < ?3
	32	ORDER BY id DESC LIMIT 1`,
	33	)
	34	.get(repoId, branch, below)?.commit_oid;
	35	}
	36
	37	/** Whether any pull, in any state, already takes `pullId` back. */
	38	function everReverted(app: App, pullId: string): boolean {
	39	return (
	40	app.db
	41	.query<{ n: number }, [string]>("SELECT COUNT(*) AS n FROM pull WHERE reverts_issue_id = ?1")
	42	.get(pullId)!.n > 0
	43	);
	44	}
	45
	46	/**
	47	* The one pull landed between the last green and `red`, else undefined:
	48	* every first-parent commit in that range must be a pull's landing, and
	49	* there must be exactly one.
	50	*/
	51	async function soleLanding(app: App, red: RedMain, green: string): Promise<Pull \| undefined> {
	52	const repo = getRepoById(app.db, red.repoId);
	53	if (repo === undefined \|\| !isSafeOid(green) \|\| !isSafeOid(red.commitOid)) return undefined;
	54	const log = await runGit(["rev-list", "--first-parent", `${green}..${red.commitOid}`], {
	55	cwd: repoDir(app.reposRoot, repo.ownerName, repo.name),
	56	});
	57	if (!log.ok) return undefined;
	58	const commits = log.value.split("\n").filter(Boolean);
	59	if (commits.length === 0) return undefined;
	60	const landed = commits.map((oid) =>
	61	app.db
	62	.query<{ number: number }, [string, string, string]>(
	63	`SELECT i.number FROM pull p JOIN issue i ON i.id = p.issue_id
	64	WHERE i.repo_id = ?1 AND p.merged_oid = ?2 AND p.base_branch = ?3`,
	65	)
	66	.get(repo.id, oid, repo.defaultBranch),
	67	);
	68	if (landed.length !== 1 \|\| landed[0] === null \|\| landed[0] === undefined) return undefined;
	69	return getPull(app.db, repo.id, landed[0].number);
	70	}
	71
	72	/** What the revert pull says besides what it reverts: the red it answers. */
	73	export function revertBody(pull: Pick<Pull, "number">, red: RedMain): string {
	74	const what =
	75	red.failing[0] !== undefined
	76	? `the failing test is "${red.failing[0]}"${red.failing.length > 1 ? ` (and ${red.failing.length - 1} more)` : ""}`
		junipercrew 18d ago Skip when `landed.revertsPullId !== null`, so a landed revert is not itself reverted.
	77	: red.summary.replace(/^check failed: /, "");
	78	return `Reverts #${pull.number}, the one pull landed since main was last green.\n\nMain is red at ${red.commitOid.slice(0, 7)}: ${what}.\n\nProposed by the forge; a person lands it.`;
	79	}
	80
	81	/**
	82	* Called when a default-branch check of `repoId` has finished: opens the
	83	* revert pull if the rules above hold. Returns it, or undefined when none
	84	* was proposed.
	85	*/
	86	export async function proposeRevert(app: App, repoId: string): Promise<Pull \| undefined> {
	87	if (working.has(repoId)) return undefined;
	88	working.add(repoId);
	89	try {
	90	const red = redMain(app.db, repoId);
	91	if (red === undefined \|\| red.limit \|\| manifestError(red) !== undefined) return undefined;
	92	const green = greenBefore(app, repoId, red.branch, red.runId);
	93	if (green === undefined) return undefined;
	94	const landed = await soleLanding(app, red, green);
	95	if (landed === undefined \|\| everReverted(app, landed.id)) return undefined;
	96	const repo = getRepoById(app.db, repoId);
	97	const owner = repo === undefined ? undefined : getUserById(app.db, repo.ownerId);
	98	if (repo === undefined \|\| owner === undefined \|\| getIssueById(app.db, landed.id) === undefined)
		junipercrew 18d ago This guard re-reads an issue we just resolved through getPull. It is dead weight; remove it.
	99	return undefined;
	100	const made = await revertPull(app, owner, repo, landed, revertBody(landed, red));
	101	return made.ok ? made.value : undefined;
	102	} finally {
	103	working.delete(repoId);
	104	}
	105	}
		@@ -55,6 +55,7 @@ import { keepCache, restoreCache } from "./cache.ts";
55	55	import { MAX_FILED, clearedFiles, fileOrderDependent } from "./order_dependent.ts";
56	56	import { withDisk } from "./disk_red.ts";
57	57	import { ASKED, redMain, unheld } from "./red_main.ts";
	58	import { proposeRevert } from "./revert_red.ts";
58	59	import { rebasesFirst } from "./finishing_first.ts";
59	60	import { IMPACT_FILE, keepImpact, seedImpact } from "./impact-store.ts";
60	61	import { getIssueById } from "../core/issues.ts";
		@@ -633,6 +634,10 @@ export function startChecks(app: App, options: ChecksOptions): ChecksHandle {
633	634	});
634	635	write(outcome.ok ? `passed in ${seconds}s` : `failed: ${outcome.error}`);
635	636	if (done) announceRun(app, done);
	637	if (!outcome.ok && run.issueId === null && run.env === repo.defaultBranch)
	638	proposeRevert(app, repo.id).catch((error) =>
	639	log.error("revert proposal failed", { error: String(error) }),
	640	);
636	641	} finally {
637	642	release();
638	643	}
		@@ -0,0 +1,122 @@
	1	/**
	2	* A red main after one landed pull gets one revert pull naming the failing
	3	* test (#6377); after two landings, or a second tick, none.
	4	*/
	5
	6	import { afterEach, beforeEach, 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 { mergePull, openPull } from "../src/actions/pulls.ts";
	12	import { commitToBranch, createRepo } from "../src/actions/repos.ts";
	13	import { proposeRevert } from "../src/checks/revert_red.ts";
	14	import { getPull, listPulls, type Pull } from "../src/core/pulls.ts";
	15	import type { Repo } from "../src/core/repos.ts";
	16	import { createRun } from "../src/core/runs.ts";
	17	import { createUser, type User } from "../src/core/users.ts";
	18	import { resolveOid } from "../src/git/merge.ts";
	19	import { repoDir } from "../src/git/repos.ts";
	20
	21	describe("a red main after one landing proposes its revert", () => {
	22	let dataDir: string;
	23	let app: App;
	24	let alice: User;
	25	let repo: Repo;
	26
	27	const land = async (branch: string, content: string): Promise<Pull> => {
	28	await commitToBranch(app, alice, repo, {
	29	branch,
	30	base: "main",
	31	message: branch,
	32	files: [{ path: "code.txt", content }],
	33	});
	34	const opened = await openPull(app, alice, repo, {
	35	title: `Change ${branch}`,
	36	baseBranch: "main",
	37	headBranch: branch,
	38	});
	39	if (!opened.ok) throw new Error(JSON.stringify(opened.error));
	40	const merged = await mergePull(app, alice, repo, opened.value, "squash");
	41	if (!merged.ok) throw new Error(JSON.stringify(merged.error));
	42	return getPull(app.db, repo.id, opened.value.number)!;
	43	};
	44
	45	const mainCheck = async (state: "ok" \| "failed", names: string[] = []): Promise<void> => {
	46	const oid = await resolveOid(repoDir(app.reposRoot, repo.ownerName, repo.name), "refs/heads/main");
	47	const run = createRun(app.db, { repoId: repo.id, kind: "check", env: "main", commitOid: oid ?? "" });
	48	app.db
	49	.query("UPDATE run SET state = ?2, summary = ?3, check_detail = ?4, finished_at = ?5 WHERE id = ?1")
	50	.run(
	51	run.id,
	52	state,
	53	state === "ok" ? "check passed in 1s" : "check failed: 1 test failed",
	54	state === "ok" ? "" : JSON.stringify({ names }),
	55	new Date().toISOString(),
	56	);
	57	};
	58
	59	const reverts = (): Pull[] =>
	60	listPulls(app.db, repo.id, { state: "all" }).filter((p) => p.revertsPullId !== null);
	61
	62	beforeEach(async () => {
	63	dataDir = await mkdtemp(join(tmpdir(), "forge-revert-red-"));
	64	app = createApp({ dataDir, port: 0 });
	65	const created = await createUser(app.db, {
	66	origin: "seed",
	67	trusted: true,
	68	username: "alice",
	69	email: "[email protected]",
	70	password: "a-good-passphrase",
	71	adminIfFirst: true,
	72	});
	73	if (!created.ok) throw new Error(created.error);
	74	alice = created.value;
	75	const made = await createRepo(app, alice, { name: "app", visibility: "public" });
	76	if (!made.ok) throw new Error(String(made.error));
	77	repo = made.value;
	78	await commitToBranch(app, alice, repo, {
	79	branch: "main",
	80	message: "base",
	81	files: [{ path: "code.txt", content: "alpha\n" }],
	82	});
	83	});
	84
	85	afterEach(async () => {
	86	app.close();
	87	await rm(dataDir, { recursive: true, force: true });
	88	});
	89
	90	test("one landing then red opens one revert; a second tick opens none", async () => {
	91	await mainCheck("ok");
	92	const landed = await land("one", "beta\n");
	93	await mainCheck("failed", ["parses the thing"]);
	94
	95	const first = await proposeRevert(app, repo.id);
	96	expect(first?.revertsPullId).toBe(landed.id);
	97	expect(first?.title).toContain(landed.title);
	98	expect(first?.body).toContain("parses the thing");
	99	expect(first?.mergedAt).toBeNull();
	100	expect(reverts()).toHaveLength(1);
	101
	102	expect(await proposeRevert(app, repo.id)).toBeUndefined();
	103	expect(reverts()).toHaveLength(1);
	104	});
	105
	106	test("two landings since the green open none", async () => {
	107	await mainCheck("ok");
	108	await land("one", "beta\n");
	109	await land("two", "gamma\n");
	110	await mainCheck("failed", ["parses the thing"]);
	111
	112	expect(await proposeRevert(app, repo.id)).toBeUndefined();
	113	expect(reverts()).toHaveLength(0);
	114	});
	115
	116	test("a green main opens none", async () => {
	117	await mainCheck("ok");
	118	await land("one", "beta\n");
	119	await mainCheck("ok");
	120	expect(await proposeRevert(app, repo.id)).toBeUndefined();
	121	});
	122	});