fix(land-watch): leave a PR another session holds to that session

README.md

@@ -844,9 +844,9 @@ head the watcher itself put back in the queue continues the watcher's pick
 instead, until a handback ends it: the watcher's re-enqueue, GitHub's
 auto-merge of it, or such a workflow picking the fixer's push. A failed build
 or a conflict is recorded for the land fixer in `removal-<n>.json`, with the
-full log beside it. A push since the removal, a person's removal, or a reason
-the watcher does not know ends the pick, and the comment says the PR waits for
-a maintainer to enqueue it again. The state directory remembers each removal
+full log beside it. A push since the removal, a person's removal, a hold, or a
+reason the watcher does not know ends the pick, and the comment says the PR
+waits for a maintainer to enqueue it again. The state directory remembers each removal
 handled, so a restart neither misses nor repeats one; a crash between
 commenting and recording finds its own comment's marker and posts nothing. The
 first pass reads the ten minutes before it.
@@ -861,8 +861,9 @@ enqueues only the head auto-merge was armed on, with `expectedHeadOid` bound to
 it: GitHub keeps auto-merge across a push, but a push by anyone but the land
 fixer ends the pick, and a head it cannot tell yet waits for the next pass. An
 armed PR whose required checks fail, or that conflicts, is left alone for
-`/quest-iterate` and a new pick. The watcher's own re-enqueues are among these
-picks. A refusal is logged once per pick and retried each pass.
+`/quest-iterate` and a new pick. One another session holds waits, armed, until
+its hold clears. The watcher's own re-enqueues are among these picks. A refusal
+is logged once per pick and retried each pass.
 
 With `--fixer`, the watcher takes each recorded removal, oldest first, while
 its PR still waits as the queue dropped it:
@@ -903,6 +904,15 @@ handback, from a stop rule, the fixer, a refusal, a crash, or a timeout, is
 posted with its reason and a recommendation, and the PR leaves the watcher's
 care until a maintainer enqueues it again. A round whose pick ended while it
 ran, by a writer's push or enqueue, is not applied, and its comment says so.
+
+The watcher leaves a PR another session holds to that session: one with a hold
+label, such as the `quest:active` that `/quest-iterate` takes. It reads the
+PR's labels when it handles the removal, before it starts the fixer, and again
+before it pushes a result or re-enqueues the PR, so a hold taken while the
+fixer runs counts too. A hold ends the pick, as a person's removal does: the
+watcher records that first and stops acting on the PR, then posts one comment
+saying so, best-effort and never retried. A maintainer enqueues it again once
+its session is done. A held flaky-test disable PR is left for a maintainer too.
 A round whose collection fails three passes in a row, as when GitHub refuses its
 handback comment, is set aside with a logged warning, freeing its fixer slot.
 

quest/m0/land-fixer/README.md

@@ -153,7 +153,6 @@ work, done once the Required quests below have merged.
 
 ## Required
 
-- [Leave held PRs alone](/quest/m0/land-fixer/holds.md) - the watcher never fixes, pushes to, or re-enqueues a PR another session holds
 - [Workflow permission](/quest/m0/land-fixer/workflows-permission.md) - waits on the maintainer to grant moq-bot `workflows`, so pushes touching workflows are accepted
 
 ## Related

src/cli/land-watch-fix.ts

@@ -2,7 +2,8 @@
 // the background and re-enqueued at the back of the queue, or handed back to a person.
 import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { basename, join } from "node:path";
-import { GitHub, type GitHubDependencies, GitHubGraphQLError, object, type ObjectValue, string } from "./github";
+import { isHoldLabel } from "../core";
+import { array, GitHub, type GitHubDependencies, GitHubGraphQLError, object, type ObjectValue, string } from "./github";
 import { type Queued, queueHead } from "../github/merge";
 import { UNREFUSED } from "./hold";
 import { baseRemote, type Entry, mergeBase, type RemoteRead } from "./land";
@@ -113,12 +114,45 @@ const JUDGED: Record<Cause, string> = {
 
 const PULL = `query($id: ID!) { node(id: $id) { ... on PullRequest {
 	state baseRefName headRefOid mergeStateStatus mergeQueueEntry { enqueuedAt } autoMergeRequest { enabledAt }
+	labels(first: 100) { nodes { name } }
 } } }`;
 
 export async function readPull(github: GitHub, id: string): Promise<ObjectValue> {
 	return object((await github.graphql(PULL, { id })).node);
 }
 
+/** Whether another session holds the PR with a hold label, such as `/quest-iterate`'s `quest:active`. */
+export const isHeld = (pull: ObjectValue): boolean =>
+	array(object(pull.labels).nodes).some((label) => isHoldLabel(string(label.name)));
+
+/** What the watcher says on a held PR, whatever step found the hold and whether a fix was pushed. */
+export const HELD =
+	"Another session holds this pull request, so the watcher stops acting on it and leaves it to that session: the hold ends the pick, and a maintainer enqueues it again once that session is done.";
+
+/**
+ * Say once, best-effort, that the watcher left a held PR to its session. The caller has already ended
+ * the pick in its state, so a comment GitHub refuses is only warned about and never retried.
+ */
+async function sayHeld(
+	github: GitHub,
+	repository: string,
+	entry: Pick<Entry, "number" | "enabledAt">,
+	warn: (text: string) => void,
+): Promise<void> {
+	try {
+		await post(github, { repository }, entry, `quest land watch: ${HELD}`, "held", MERGER);
+	} catch (error) {
+		warn(`quest land watch: hold comment on ${entry.number} not posted: ${String(error)}`);
+	}
+}
+
+/** A hold found right before a push or a re-enqueue. */
+class Held extends Error {
+	constructor() {
+		super("held by another session");
+	}
+}
+
 /**
  * Whether the PR is in the merge queue, or armed since `since` to join it once its required checks
  * pass. An older request is not a pick of the head the queue dropped.
@@ -254,7 +288,7 @@ async function collectAll(
 }
 
 /**
- * Take a removal waiting for the fixer, if its PR still waits as the queue dropped it: merge a
+ * Take a removal waiting for the fixer, if its PR still waits as the queue dropped it, unheld: merge a
  * conflict's base with the quest merge driver, or start the fixer when `slot` allows. Returns what it
  * did, if anything.
  */
@@ -296,6 +330,12 @@ async function take(
 		mark(`${started[1].merged ? "merged" : "fixer"} ${started[0]}`);
 		return undefined;
 	}
+	if (isHeld(pull)) {
+		mark("held");
+		endPick(state, number, entry.enabledAt);
+		await sayHeld(github, repository, entry, deps.warn);
+		return `dropped ${number} held`;
+	}
 	const conflict = removal.reason === "merge_conflict";
 	// A conflict is merged with the current base; a failed build is fixed on the base it was built on.
 	const base = conflict ? current : failure.plan.base;
@@ -413,7 +453,8 @@ function finish(path: string, job: WatchJob): void {
 /**
  * Collect a finished round: record it in its pick's history, and either put the PR back in the queue,
  * at a fixed head or unchanged, or hand it back, as a stop rule, the fixer, or a refusal says. A
- * round whose pick ended while it ran is dropped, and the PR says so. Returns what it did.
+ * round whose pick ended while it ran, or whose PR another session holds, is dropped, and the PR says
+ * so. Returns what it did.
  */
 async function collect(
 	github: GitHub,
@@ -498,6 +539,16 @@ async function collect(
 				: queued(pull, job.dropped) && !ours && job.requeuing === undefined
 					? "a maintainer enqueued it again while the fixer ran, and that pick stands"
 					: undefined;
+	// Leave a held PR to its session: the pick and the round end first, and the hold's comment replaces
+	// the fixer's note.
+	const leave = async (): Promise<string[]> => {
+		endPick(state, number, entry.enabledAt);
+		finish(path, job);
+		await sayHeld(github, job.repository, marked, deps.warn);
+		return [`dropped ${number} held`];
+	};
+	// A head this round already asked GitHub to queue, and GitHub took, stays: the hold came after it.
+	if (isHeld(pull) && !(job.requeuing !== undefined && queued(pull, job.dropped))) return leave();
 	if (ended) {
 		await note();
 		await post(
@@ -579,6 +630,9 @@ async function collect(
 					job.published = head;
 					save(path, job);
 				}
+				// Validating the fix takes a while, and a session may have taken the PR since. A head already
+				// pushed needs no push, and `requeue` checks the hold before any enqueue.
+				if (!ours && isHeld(await readPull(github, entry.id))) throw new Held();
 				await publishHead(github, context.root, job, entry, entry.head, current, head, deps, context.token, {
 					reason: result.reason,
 					attribution,
@@ -591,6 +645,7 @@ async function collect(
 			job.requeued = await requeue(github, entry.id, head, job.base.ref, job.dropped);
 			save(path, job);
 		} catch (failure) {
+			if (failure instanceof Held) return [...said, ...(await leave())];
 			if (!(failure instanceof FixerRefusal)) throw failure;
 			handback = failure.message;
 			if (failure.resolution) job.resolution = failure.resolution;
@@ -633,14 +688,16 @@ async function collect(
 /**
  * Put `head` back in `base`'s merge queue, at the back. A PR queued or armed at it since the queue
  * dropped it, at `dropped`, needs nothing more: a restart finds its own request, and a workflow may pick
- * the fixer's push first. A refusal, or a PR retargeted from `base`, hands the PR back.
+ * the fixer's push first. A refusal, or a PR retargeted from `base`, hands the PR back; a hold leaves
+ * it to the session that holds it.
  */
 async function requeue(github: GitHub, id: string, head: string, base: string, dropped: string): Promise<Queued> {
 	const how = (pull: ObjectValue): Queued => (pull.mergeQueueEntry != null ? "enqueued" : "armed");
 	let pull = await readPull(github, id);
 	if (string(pull.baseRefName) !== base) throw new FixerRefusal(`retargeted from ${base} before it was re-enqueued`);
 	if (string(pull.headRefOid) !== head) throw new FixerRefusal("head moved before it was re-enqueued");
 	if (!queued(pull, dropped)) {
+		if (isHeld(pull)) throw new Held();
 		try {
 			await queueHead(
 				(query, variables) => github.graphql(query, variables),
@@ -665,7 +722,7 @@ async function requeue(github: GitHub, id: string, head: string, base: string, d
 /**
  * Open each validated disable as a PR of its own and enqueue it like any other, with no maintainer's
  * pick: its Review records the fixer's change. A disable that fails to advance is retried on the next
- * pass; one GitHub refuses is left for a maintainer.
+ * pass; one GitHub refuses, or another session holds, is left for a maintainer.
  */
 async function advanceDisables(
 	github: GitHub,
@@ -680,7 +737,15 @@ async function advanceDisables(
 		if (!test.disable || test.disable.done !== undefined) continue;
 		try {
 			output.push(
-				await openDisable(github, repository, context, test, test.disable, () => saveFlaky(state, tests)),
+				await openDisable(
+					github,
+					repository,
+					context,
+					test,
+					test.disable,
+					() => saveFlaky(state, tests),
+					deps.warn,
+				),
 			);
 		} catch (error) {
 			deps.warn(`quest land watch: disable of ${test.disable.quest} pending: ${String(error)}`);
@@ -708,6 +773,7 @@ async function openDisable(
 	test: FlakyTest,
 	disable: Disable,
 	record: () => void,
+	warn: (text: string) => void,
 ): Promise<string> {
 	const name = questName(disable.quest);
 	if (disable.id === undefined) {
@@ -745,14 +811,22 @@ async function openDisable(
 		disable.id = string(pull.node_id);
 		record();
 	}
+	const entry = { number: disable.number!, enabledAt: disable.head };
 	try {
 		disable.done = await requeue(github, disable.id, disable.head, disable.base.ref, "");
 	} catch (error) {
+		if (error instanceof Held) {
+			// Left for a maintainer, as a refused one is, once recorded.
+			disable.done = "held";
+			record();
+			await sayHeld(github, repository, entry, warn);
+			return `disable held ${name} ${disable.number}`;
+		}
 		if (!(error instanceof FixerRefusal)) throw error;
 		await post(
 			github,
 			{ repository },
-			{ number: disable.number!, enabledAt: disable.head },
+			entry,
 			`quest land watch: ${fixerText(error.message)}, so this pull request waits for a maintainer to enqueue it.`,
 			"refused",
 			MERGER,