fix(hold): keep quest:merge as the pick after a landing ends unmerged

README.md

@@ -414,6 +414,7 @@ comes last, and a missing value prints as `-`:
 | `reacted:<content>` | Actor, time, and the PR or review reacted to, as `quest wait` names reactions. |
 | `thread` | Node ID, `current` or `outdated`, last commenter, latest comment URL, and `path:line` of each unresolved thread. |
 | `hold` | Label, acquisition token, acquisition time, verified GitHub event API URL, and age; `hold -` when no label holds it. Unknown acquisition facts print as `-` and `unknown age`. |
+| `pick` | A `quest:merge` left on after its landing ended unmerged, with the same fields as `hold`: the maintainer's merge pick, which no session holds and `quest merge` takes over. |
 | `starter-hold` | A draft its starter owns: last activity time, PR URL, and `last active` age. Last activity is not when ownership began. |
 
 The node ID is what GraphQL's `addPullRequestReviewThreadReply` and
@@ -463,8 +464,10 @@ On an ungated base, it compares the live base against that head and refuses a
 head that is behind or diverged: merge the base, rerun the repository's checks,
 and land the new head. This is a preflight check; GitHub can still move the base
 between that comparison and its head-pinned merge mutation. `quest:merge` stays
-on while the PR is queued or auto-merging; once the PR leaves the queue unmerged, the next command removes
-it. The commands accept the same PR argument and authentication as
+on while the PR is queued or auto-merging, and after it leaves the queue unmerged
+as the maintainer's pick that no session holds: `quest merge` without a token
+takes it over with a new token, and the other commands refuse it. Remove the
+label to withdraw the pick. The commands accept the same PR argument and authentication as
 `quest wait`, and exit 0 when done, 3 when refused, 2 for invalid arguments,
 and 1 for other failures.
 
@@ -513,8 +516,8 @@ and the prefix before it lands only if a check passed on exactly that prefix,
 since a later PR can mask an earlier one's failure. A failed landing moves the
 branches back, so they stay queued. The rest of the queue waits for the next run.
 
-An ejected PR loses its auto-merge and `quest:merge` label and gets a comment
-with the reason, which `quest status` prints under `landing-stopped`:
+An ejected PR loses its auto-merge, keeps its `quest:merge` label as the
+maintainer's pick, and gets a comment with the reason, which `quest status` prints under `landing-stopped`:
 
 | Reason | Why |
 | --- | --- |

assets/skills/complete.md

@@ -8,6 +8,7 @@ The goal is to evaluate the open PRs in the repository, drafts included, and dec
 For a draft, summarize the open decisions its body lists.
 Read each PR with `quest status` and skip one another session holds, using its `hold` and `starter-hold` records instead of a separate label read. A `hold` names the label, acquisition token/time, event URL, and age. A `starter-hold` identifies a draft without a `## Decisions needed` section and reports when it was last active; that is not an acquisition time.
 List those with how long ago each was last active, and offer to take over only one that looks stale.
+A `pick` record is the user's earlier merge pick, which no session holds: do not ask about that PR, and dispatch `/quest-merge` for it right away, even when an earlier session made the pick.
 The skill argument can be used to filter the PRs in scope.
 Each merge is performed in parallel by a sub-agent, following the guide's Reviews section for the shared design checkpoint across review rounds.
 
@@ -34,9 +35,8 @@ For GitHub CI, review, or merge activity, it uses one `quest wait` and resumes w
 returned cursor as described by `quest skill merge`, rather than polling GitHub.
 
 Merge sub-agents report once their PR is landing; do not watch the queue or wait for merges.
-An open PR that left the queue unmerged shows why under `landing-stopped` in `quest status <pr>`.
-After `merge_conflict` or `failed_checks`, its pick still holds, even from an earlier session: dispatch `/quest-merge` again without asking.
-Any other reason, such as a person removing it, is the user's call, so ask about the PR again.
+An open PR that left the queue unmerged keeps `quest:merge` as a `pick`, and shows why under `landing-stopped` in `quest status <pr>`; dispatch `/quest-merge` for it again without asking.
+A person withdraws a pick by removing the label; ask about that PR again like any other.
 
 Keep going until all PRs have been decided, then wait for all spawned sub-agents to finish.
 Refresh the open PR list whenever a sub-agent reports, and once more before finishing; ask about each new PR in scope the same way.

assets/skills/merge.md

@@ -51,7 +51,7 @@ Leave a summary of the changes and decisions made, then land the head you checke
 It makes a merge commit; add `--squash` or `--rebase` when the repository's root instructions name that method for the PR's base.
 On a base that requires the `quest land` check, it queues the PR for `quest land`, whose chain merges and tests against the current base; do not merge the base or rerun checks per PR just to queue it.
 On an ungated base, the command refuses a head that does not contain the current base. Merge the base locally (never rebase), rerun the repository's checks, push, and land the new head after satisfying the guide's review rule for those changes.
-The `quest:merge` label stays on while the PR is queued or auto-merging.
+The `quest:merge` label stays on while the PR is queued or auto-merging, and after it leaves the queue unmerged, as the user's pick: `quest status` then shows it as `pick`, and `quest merge <pr>` takes it over with a new token.
 Report once it is landing; do not wait for it to merge, since the queue re-checks it without you.
 A PR that left the queue unmerged or lost auto-merge shows why under `landing-stopped` in `quest status <pr>`.
 After `merge_conflict` or `failed_checks`, the pick still holds: fix it and land it again.

cloudflare/src/github/service.ts

@@ -1655,9 +1655,9 @@ export class GitHubService {
 	 * Hold an open pull request for a merge job with the `quest:merge` label,
 	 * the contract `quest merge --help` states for local sessions too. Refused
 	 * while another session holds a `quest:*` label; a `quest:merge` whose
-	 * landing ended unmerged is spent and removed first. When two sessions
-	 * label it at once, the first label added keeps it. Returns the hold's
-	 * token: the timeline event that added the label.
+	 * landing ended unmerged is the maintainer's pick, replaced by this hold.
+	 * When two sessions label it at once, the first label added keeps it.
+	 * Returns the hold's token: the timeline event that added the label.
 	 */
 	holdPullRequest(repo: GitHubRepository, userId: string, pull: number) {
 		return this.#authorized(repo, userId, { issues: "write", pull_requests: "read" }, async (token) => {
@@ -1691,14 +1691,18 @@ export class GitHubService {
 	/**
 	 * Release a merge job's `quest:merge` hold unless its pull request is
 	 * queued or auto-merging. The App acts on its own label here, with no
-	 * maintainer request behind it. Returns whether the hold is over: released
-	 * now, already removed or replaced, or its pull request no longer open.
+	 * maintainer request behind it. A landing that ended unmerged leaves the
+	 * label on as the maintainer's pick, which the next merge takes over; a job
+	 * that ended without landing removes it. Returns whether the hold is over:
+	 * released now, left as a pick, already removed or replaced, or its pull
+	 * request no longer open.
 	 */
 	releasePullRequest(repo: GitHubRepository, pull: number, hold: string) {
 		return this.#withRepository(repo, { issues: "write", pull_requests: "read" }, async (token) => {
 			const holds = await this.#holds(token, repo, pull);
 			if (holds.state !== "OPEN" || heldToken(holds, MERGE_LABEL) !== hold) return true;
 			if (holds.landing) return false;
+			if (spentMergeHold(holds)) return true;
 			await this.#unhold(token, repo, pull);
 			return true;
 		});

cloudflare/src/jobs/landing.ts

@@ -1,7 +1,8 @@
 // A merge job holds its pull request with the `quest:merge` label, as local
-// `quest merge` sessions do: from when the maintainer picks merge until the
-// job ends without landing, or until a landing it started ends unmerged. Then
-// the bot removes the label, and the pull request is an idle decision again.
+// `quest merge` sessions do, from when the maintainer picks merge. A job that
+// ends without landing has the bot remove the label, and the pull request is
+// an idle decision again. A landing it started that ends unmerged leaves the
+// label on as the maintainer's pick, so the next merge takes it over.
 import { GitHubService } from "../github/service";
 import { pullOf } from "./publish";
 import * as store from "./store";

cloudflare/test/jobs.test.ts

@@ -1353,10 +1353,10 @@ describe("job routes", () => {
 		await delivery("pull_request", { ...dequeued, action: "auto_merge_enabled" }, undefined, {}, testEnv);
 		expect(github.labels).toEqual(["quest:merge"]);
 		expect(await state(id)).toMatchObject({ hold: "LE_1" });
-		// It left the queue unmerged: the bot removes the label.
+		// It left the queue unmerged: the job's hold ends, and the label stays on as the maintainer's pick.
 		github.landing = false;
 		await delivery("pull_request", dequeued, undefined, {}, testEnv);
-		expect(github.labels).toEqual([]);
+		expect(github.labels).toEqual(["quest:merge"]);
 		expect(await state(id)).toMatchObject({ hold: null });
 	});
 

cloudflare/test/publish.test.ts

@@ -1100,7 +1100,7 @@ describe("merge jobs", () => {
 		return id;
 	}
 
-	it("merges its base in, lands exactly the head it published, and keeps its hold while queued", async () => {
+	it("merges its base in, lands exactly the head it published, and keeps its pick after the landing ends", async () => {
 		const id = await merging();
 		expect(pull().labels).toEqual(["quest:merge"]);
 		// The base branch moved on; the job merged it into the checkout.
@@ -1141,10 +1141,10 @@ describe("merge jobs", () => {
 		await store.finish(db, machine, id, 1, { outcome: "completed", summary: "Landing", result: undefined }, 0);
 		await landing.ended(testEnv, id);
 		expect(pull().labels).toEqual(["quest:merge"]);
-		// Once the landing ends unmerged, the bot gives the pull request back.
+		// Once the landing ends unmerged, the job's hold ends and the label stays on as the maintainer's pick.
 		pull().landing = false;
 		await landing.release(testEnv, 10, "pulls/2");
-		expect(pull().labels).toEqual([]);
+		expect(pull().labels).toEqual(["quest:merge"]);
 		expect(await job(id)).toMatchObject({ hold: null });
 	});
 

src/cli/hold.ts

@@ -74,6 +74,8 @@ export interface Hold {
 
 export interface HoldFact {
 	label: string;
+	/** A `quest:merge` left on as the maintainer's pick after its landing ended unmerged; no session holds it. */
+	picked: boolean;
 	/** Null when the acquisition is older than the available timeline. */
 	token: string | null;
 	takenAt: string | null;
@@ -98,6 +100,7 @@ export async function readHoldStatus(pr: PullRequest, deps: GitHubDependencies):
 					const event = latestHold(snapshot.events, label);
 					return {
 						label,
+						picked: label === MERGE_LABEL && spentMergeHold(snapshot),
 						token: event?.id ?? null,
 						takenAt: event ? new Date(event.time).toISOString() : null,
 						age: event ? ago(deps.now(), event.time) : null,
@@ -294,7 +297,11 @@ async function acquire(
 	if (snapshot.state !== "OPEN") {
 		throw new Refusal(`${snapshot.url} is ${snapshot.state.toLowerCase()}; a push now would bring its branch back`);
 	}
-	snapshot = await labels.dequeued(snapshot);
+	// A merge pick whose landing ended unmerged is no session's hold; `quest merge` takes it with a fresh token.
+	if (token === undefined && state === "merge" && spentMergeHold(snapshot)) {
+		await labels.remove(MERGE_LABEL);
+		snapshot = { ...snapshot, labels: snapshot.labels.filter((name) => name !== MERGE_LABEL) };
+	}
 	const mine = token === undefined ? undefined : held(snapshot, token);
 	const other = snapshot.labels.find((name) => name !== mine);
 	if (other !== undefined) throw new Refusal(holder(snapshot, other, labels.deps.now()));
@@ -349,6 +356,9 @@ function unknown(snapshot: Snapshot, token: string): string {
 function holder(snapshot: Snapshot, label: string, now: number): string {
 	const added = latestHold(snapshot.events, label);
 	const since = added ? `, added ${ago(now, added.time)} as token ${added.id}` : "";
+	if (label === MERGE_LABEL && spentMergeHold(snapshot)) {
+		return `${snapshot.url} is picked for merge: ${label}${since} stays on after its landing ended unmerged; take it with quest merge, or ask the user before releasing it`;
+	}
 	return `${snapshot.url} is held by ${label}${since}; if it looks stale, ask the user before releasing it`;
 }
 
@@ -421,19 +431,6 @@ class Labels {
 			allow: [404],
 		});
 	}
-
-	/**
-	 * A landing that ended unmerged ends the merge session's hold, so a
-	 * `quest:merge` label is spent once a landing started after it was added.
-	 */
-	async dequeued(snapshot: Snapshot): Promise<Snapshot> {
-		if (!spentMergeHold(snapshot)) return snapshot;
-		await this.remove(MERGE_LABEL);
-		this.deps.warn(
-			`${snapshot.url} left the merge queue unmerged, so ${MERGE_LABEL} was removed; quest status shows why under landing-stopped`,
-		);
-		return { ...snapshot, labels: snapshot.labels.filter((name) => name !== MERGE_LABEL) };
-	}
 }
 
 function event(row: ObjectValue): Event {

src/cli/land.ts

@@ -7,7 +7,6 @@
 import { spawn, spawnSync } from "node:child_process";
 import { resolve } from "node:path";
 
-import { MERGE_LABEL } from "../core";
 import { repositoryPath } from "./claims";
 import {
 	GitHub,
@@ -821,9 +820,10 @@ function validate(chain: Chain, plan: Plan, result: Result): void {
 }
 
 /**
- * Remove `ejection` from the queue: disable its auto-merge, drop `quest:merge`,
- * and comment with the reason. A PR whose auto-merge was enabled again since
- * the queue was read is a new entry and is left alone. Returns whether it ejected.
+ * Remove `ejection` from the queue: disable its auto-merge and comment with the
+ * reason. Its `quest:merge` stays on as the maintainer's pick for the next
+ * `quest merge`. A PR whose auto-merge was enabled again since the queue was
+ * read is a new entry and is left alone. Returns whether it ejected.
  */
 async function eject(github: GitHub, plan: Plan, ejection: Ejection, run: string | undefined): Promise<boolean> {
 	const data = await github.graphql(
@@ -842,7 +842,6 @@ async function eject(github: GitHub, plan: Plan, ejection: Ejection, run: string
 	}
 	const [owner, repo] = plan.repository.split("/");
 	const issue = `${repositoryPath({ owner, repo })}/issues/${ejection.number}`;
-	await github.request(`${issue}/labels/${encodeURIComponent(MERGE_LABEL)}`, { method: "DELETE", allow: [404] });
 	const link = run === undefined ? "" : ` ([run](${run}))`;
 	await github.request(`${issue}/comments`, {
 		body: {

src/cli/main.ts

@@ -433,8 +433,9 @@ export function main(argv: string[], out: Output): number | Promise<number> {
 		"Given a token, a command refuses unless the token is still the latest add of the PR's label, then swaps " +
 		"that label for its own; if a later step then fails, the error names the new token. When two sessions add different labels at once, the first label added keeps the " +
 		"PR; GitHub records no event when a label already on the PR is added again, so two takes of one state " +
-		"within the same moment are not told apart. A quest:merge label is removed once the PR leaves the merge " +
-		"queue unmerged.\n\n" +
+		"within the same moment are not told apart. A quest:merge label stays on after the PR leaves the merge " +
+		"queue unmerged, as the maintainer's pick that no session holds; quest merge without a token takes it " +
+		"over, and the other commands refuse it. Remove the label to withdraw the pick.\n\n" +
 		"Uses GH_TOKEN, GITHUB_TOKEN, or gh auth token. Exits 0 when done, 3 when refused, 2 for invalid " +
 		"arguments, and 1 for other failures.";
 	/** A PR state command: take the PR or swap to `state`, then do its step. Prints the hold token. */
@@ -515,7 +516,7 @@ export function main(argv: string[], out: Output): number | Promise<number> {
 				"fails alone, which is ejected; the rest waits for the next run. Landing moves each PR's branch, " +
 				"with a lease on the head that was tested, to its squash commit, then fast-forwards the base to the " +
 				"tip, so GitHub shows each PR as merged and the base only ever holds a tested tree.\n\n" +
-				"An ejected PR loses auto-merge and `quest:merge`, and gets a comment with the reason, which " +
+				"An ejected PR loses auto-merge, keeps `quest:merge` as its pick, and gets a comment with the reason, which " +
 				"`quest status` prints under landing-stopped: merge_conflict (a conflict `quest resolve` cannot " +
 				"settle), failed_checks (the check failed with it alone on the base, or a required check failed on " +
 				"its head), checks_pending, review_required, changes_requested, unresolved_conversations, " +

src/cli/status.ts

@@ -388,7 +388,7 @@ export function statusLines(status: Status): string[] {
 		...(status.holds.length
 			? status.holds.map(
 					(hold) =>
-						`hold ${hold.label} ${dash(hold.token)} ${dash(hold.takenAt)} ${dash(hold.eventUrl)} ${hold.age ?? "unknown age"}`,
+						`${hold.picked ? "pick" : "hold"} ${hold.label} ${dash(hold.token)} ${dash(hold.takenAt)} ${dash(hold.eventUrl)} ${hold.age ?? "unknown age"}`,
 				)
 			: ["hold -"]),
 		...(status.starterHold

test/hold.test.ts

@@ -427,19 +427,32 @@ describe("merge --land", () => {
 		expect(pull.labels).toEqual(["quest:merge"]);
 	});
 
-	test("a PR that left the queue unmerged drops quest:merge", async () => {
+	test("a PR that left the queue unmerged keeps quest:merge as the pick", async () => {
 		const pull = new FakePull();
 		const { token } = await take(PR, "merge", undefined, pull.deps());
 		await land(PR, token, HEAD, "merge", pull.deps());
 		// Auto-merge is disabled, as when a required check fails.
 		pull.autoMerge = null;
-		const warnings: string[] = [];
-		expect(await refused(land(PR, token, HEAD, "merge", pull.deps(warnings)))).toContain("no longer holds it");
-		expect(warnings).toEqual([
-			`${URL_} left the merge queue unmerged, so quest:merge was removed; quest status shows why under landing-stopped`,
-		]);
-		expect(pull.labels).toEqual([]);
-		expect((await take(PR, "iterate", undefined, pull.deps())).label).toBe("quest:iterate");
+		expect(pull.labels).toEqual(["quest:merge"]);
+		// The pick is no session's hold: other states refuse it, naming the pick.
+		expect(await refused(take(PR, "iterate", undefined, pull.deps()))).toContain("is picked for merge");
+		// A new merge session takes it over with a fresh token.
+		const next = await take(PR, "merge", undefined, pull.deps());
+		expect(next.label).toBe("quest:merge");
+		expect(next.token).not.toBe(token);
+		expect(pull.labels).toEqual(["quest:merge"]);
+		// The taken-over hold is live again, so a third session is refused.
+		expect(await refused(take(PR, "merge", undefined, pull.deps()))).toContain("is held by quest:merge");
+		await land(PR, next.token, HEAD, "merge", pull.deps());
+	});
+
+	test("the session that landed can land its pick again with its token", async () => {
+		const pull = new FakePull();
+		const { token } = await take(PR, "merge", undefined, pull.deps());
+		await land(PR, token, HEAD, "merge", pull.deps());
+		pull.autoMerge = null;
+		expect((await land(PR, token, HEAD, "merge", pull.deps())).token).toBe(token);
+		expect(pull.labels).toEqual(["quest:merge"]);
 	});
 });
 

test/land.test.ts

@@ -329,7 +329,8 @@ describe.skipIf(process.platform === "win32")("quest land", () => {
 		expect(landing.slice(1).map((line) => line.split(" ")[1])).toEqual(["1", "2", "3"]);
 		expect(git(forge.origin, "show", "main:quest/m0/README.md")).toBe(EPIC(["a", "b", "c"]).trim());
 		expect(clash.landing).toBe(false);
-		expect(clash.labels).toEqual([]);
+		// The ejected PR keeps its merge pick for the next `quest merge`.
+		expect(clash.labels).toEqual(["quest:merge"]);
 		expect(forge.comment(4)).toContain("in code.txt");
 		expect(forge.comment(4)).toContain(`([run](${RUN}))`);
 		expect(forge.comment(4)).toMatch(/<!-- quest land: merge_conflict -->$/);