diff --git a/README.md b/README.md index 8d92e632..42895f45 100644 --- a/README.md +++ b/README.md @@ -510,16 +510,13 @@ quest merge 128 --land # auto-merge bound to that head; takes no label A taken PR carries the `quest:active` label while an agent works on it. Sessions often share a GitHub account, so taking it prints a hold token: the ID of the timeline event that added the label. Every command refuses while a -`quest:*` label it holds no token for is on the PR, including another -session's `quest:active`; a draft its starter still holds, one without a +`quest:active` or `quest:kix` label it holds no token for is on the PR, +including another session's `quest:active`; a draft its starter still holds, one without a `## Waiting` (or older `## Decisions needed`) section; and a merged or closed PR, since a push after the merge brings the branch back. The refusal names the holder's label, age, and token, so you can decide whether to release a stale hold. When two sessions add labels at once, the first label added keeps the PR and the other -backs off. Older quest versions add `quest:iterate` and `quest:ready`, which -hold a PR like any `quest:*` label; passing their token to `quest iterate` -swaps them for `quest:active`. Their `quest:merge` holds nothing and is -ignored. +backs off. `quest merge` takes no label: the head-pinned auto-merge or merge-queue entry it creates is the queue signal, so a queued PR shows no hold. It refuses while diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index 81af9a78..a8b63c96 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -1,7 +1,7 @@ import { createHash, createPrivateKey, sign } from "node:crypto"; import { - HOLD_PREFIX, InitRefused, + KIX_LABEL, isHoldLabel, latestHold, heldToken, @@ -2595,14 +2595,6 @@ type Holds = { events: { id: string; label: string | null }[]; }; -/** - * The label a merge job holds its pull request with, from the pick until the - * job ends without landing or a landing it started ends unmerged. It is kix's - * own, apart from local sessions' `quest:active`, since it stays on while the - * job's landing is queued; as a `quest:*` label, local sessions respect it. - */ -export const KIX_LABEL = `${HOLD_PREFIX}kix`; - function heldBy(label: string) { return `Another session holds this pull request with ${label}; pick it again once it is released`; } diff --git a/quest/m2/README.md b/quest/m2/README.md index fc36765e..df3f891a 100644 --- a/quest/m2/README.md +++ b/quest/m2/README.md @@ -17,5 +17,4 @@ additional login providers. This milestone does not gate the public release. - [Donor isolation](/quest/m2/donor-isolation.md) - stronger boundaries for untrusted shared jobs - [ChatGPT hosted runs](/quest/m2/chatgpt-hosted.md) - conditional on provider approval and current terms - [ChatGPT approval](/quest/m2/chatgpt-waitlist.md) - resolve hosted-use approval before implementation -- [Delete the legacy hold labels](/quest/m2/legacy-labels.md) - once every gated repository pins quest past #326, only `quest:active` and `quest:kix` are holds - [Worktree hook PATH error](/quest/m2/hook-path-error.md) - a worktree hook without `quest` on `PATH` says so and names the dev-shell launch route diff --git a/quest/m2/legacy-labels.md b/quest/m2/legacy-labels.md deleted file mode 100644 index c08848a7..00000000 --- a/quest/m2/legacy-labels.md +++ /dev/null @@ -1,24 +0,0 @@ -# [XS] Delete the legacy hold labels - -## Goal - -Once every gated repository pins a quest release that includes -[moq-dev/quest#326](https://github.com/moq-dev/quest/pull/326), -`LEGACY_IGNORED` in `src/core/hold.ts` and its notes in the README and -`quest iterate --help` are gone, and only `quest:active` and `quest:kix` -count as holds. - -## Plan - -Decided 2026-10-31 (user, /quest-plan of the follow-ups from moq.pro's and rust2's cutover to the host gate and the fixer repairs [moq-dev/quest#320](https://github.com/moq-dev/quest/pull/320), [#325](https://github.com/moq-dev/quest/pull/325), [#327](https://github.com/moq-dev/quest/pull/327) and [#328](https://github.com/moq-dev/quest/pull/328)). - -- Decide then whether any other `quest:*` label should still count as a - hold; recommend only the two. - -## Waiting - -- rind, rust2: each pins a quest release that includes - [moq-dev/quest#326](https://github.com/moq-dev/quest/pull/326); quest, - moq and moq.pro already do (2026-11-04; rind pins 782132e and rust2 - dc8008b, both older). Check each repository's quest pin - (`flake.nix` or `mise.toml`). diff --git a/src/cli/hold.ts b/src/cli/hold.ts index 202719b5..97f5fd2c 100644 --- a/src/cli/hold.ts +++ b/src/cli/hold.ts @@ -382,8 +382,8 @@ export async function release(pr: PullRequest, token: string, deps: GitHubDepend /** * Take `pr` with `quest:active`, refusing while it is merged or closed, while * another session holds a hold label, or while it is a draft its starter still - * holds. With `token`, the hold it names is kept, or swapped for `quest:active` - * when it names another label, such as one an older quest version added. + * holds. With `token`, the hold it names must be this session's own `quest:active`, + * which is kept; another hold label, such as kix's, is refused rather than taken over. */ async function acquire(labels: Labels, token: string | undefined): Promise<{ hold: Hold; snapshot: Snapshot }> { const label = ACTIVE_LABEL; @@ -392,6 +392,9 @@ async function acquire(labels: Labels, token: string | undefined): Promise<{ hol const mine = token === undefined ? undefined : held(snapshot, token); unheld(snapshot, mine, labels.deps.now()); if (mine === label) return { hold: { label, token: token! }, snapshot }; + if (mine !== undefined) { + throw new Refusal(`${token} names the ${mine} hold on ${snapshot.url}, which this session does not own`); + } await labels.add(label); const after = await labels.read(); @@ -399,7 +402,7 @@ async function acquire(labels: Labels, token: string | undefined): Promise<{ hol if (!ours) throw new Error(`added ${label} to ${after.url}, but its timeline does not show it`); // Another session took the PR at the same time: the first label added keeps it. const rival = after.labels.find((name) => { - if (name === label || name === mine) return false; + if (name === label) return false; const added = latestHold(after.events, name); return !added || after.events.indexOf(added) < after.events.indexOf(ours); }); @@ -407,7 +410,6 @@ async function acquire(labels: Labels, token: string | undefined): Promise<{ hol await labels.remove(label); throw new Refusal(holder(after, rival, labels.deps.now())); } - if (mine !== undefined) await labels.remove(mine); return { hold: { label, token: ours.id }, snapshot: { ...after, labels: [label] } }; } diff --git a/src/cli/main.ts b/src/cli/main.ts index 23282c8e..f98defd2 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -527,15 +527,13 @@ export function main(argv: string[], out: Output): number | Promise { const OWNERSHIP = "Before a session pushes to an existing PR, it takes the PR with iterate, and hands it back with " + "release. A taken PR carries the quest:active label; iterate prints a hold token, the ID of the timeline " + - "event that added it, since sessions often share an account. A command refuses while a quest:* label " + - "it holds no token for is on the PR, and while the PR is merged, closed, or a draft its starter still " + + "event that added it, since sessions often share an account. A command refuses while a quest:active or " + + "quest:kix label it holds no token for is on the PR, and while the PR is merged, closed, or a draft its starter still " + "holds (one whose body has no Waiting heading). A refusal names the holder's label, age, and " + "token; releasing a stale hold with that token is the user's choice. Given a token, a command refuses " + "unless the token is still the latest add of its label. When two sessions add the label at once, the " + "first added keeps the PR; GitHub records no event when a label already on the PR is added again, so two " + - "takes within the same moment are not told apart. Older quest versions' quest:iterate and quest:ready " + - "labels hold a PR like any quest:* label, and taking it with their token swaps them for quest:active; " + - "their quest:merge label holds nothing.\n\n" + + "takes within the same moment are not told apart.\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."; const printToken = (work: Promise) => settle(work, (hold) => print(hold.token)); diff --git a/src/core/hold.ts b/src/core/hold.ts index 2d7d7d33..704ea645 100644 --- a/src/core/hold.ts +++ b/src/core/hold.ts @@ -5,13 +5,12 @@ export const HOLD_PREFIX = "quest:"; export const ACTIVE_LABEL = `${HOLD_PREFIX}active`; /** - * Compatibility with older quest versions, which repositories pin: delete once - * they have upgraded. Their `quest:iterate` and `quest:ready` still hold a PR, - * as every other `quest:*` label does, so nothing here names them. Their - * `quest:merge` stayed on a queued PR after its session ended, and the pinned - * auto-merge is the queue signal, so that label holds nothing. + * The label a merge job holds its pull request with, from the pick until the + * job ends without landing or a landing it started ends unmerged. It is kix's + * own, apart from local sessions' `quest:active`, since it stays on while the + * job's landing is queued; local sessions respect it. */ -const LEGACY_IGNORED = `${HOLD_PREFIX}merge`; +export const KIX_LABEL = `${HOLD_PREFIX}kix`; export interface HoldEvent { id: string; @@ -25,7 +24,7 @@ export interface HoldSnapshot { } export function isHoldLabel(label: string): boolean { - return label.startsWith(HOLD_PREFIX) && label !== LEGACY_IGNORED; + return label === ACTIVE_LABEL || label === KIX_LABEL; } /** Preserve a caller's extra event fields, such as its timestamp. */ diff --git a/src/core/index.ts b/src/core/index.ts index c3817b34..35d5677e 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -66,6 +66,7 @@ export { type Chunk, type Clean, type Conflict, LABELS, chunks, render, resolveL export { HOLD_PREFIX, ACTIVE_LABEL, + KIX_LABEL, type HoldEvent, type HoldSnapshot, isHoldLabel, diff --git a/test/hold-policy.test.ts b/test/hold-policy.test.ts index 3f74a0ee..a8fb33b1 100644 --- a/test/hold-policy.test.ts +++ b/test/hold-policy.test.ts @@ -1,17 +1,24 @@ import { describe, expect, test } from "vitest"; -import { ACTIVE_LABEL, HOLD_PREFIX, heldToken, isHoldLabel, latestHold } from "../src/core"; +import { ACTIVE_LABEL, HOLD_PREFIX, KIX_LABEL, heldToken, isHoldLabel, latestHold } from "../src/core"; const added = { id: "first", label: ACTIVE_LABEL, time: 1 }; describe("shared hold policy", () => { - test("recognizes all hold labels without treating ordinary labels as holds", () => { + test("recognizes only the hold labels, not other quest:* or ordinary labels", () => { expect(HOLD_PREFIX).toBe("quest:"); expect(ACTIVE_LABEL).toBe("quest:active"); - // Older quest versions' quest:iterate and quest:ready still hold a PR. - for (const label of [ACTIVE_LABEL, "quest:kix", "quest:ready", "quest:iterate", "quest:custom"]) - expect(isHoldLabel(label)).toBe(true); - // Their quest:merge only shadowed a queued landing. - for (const label of ["quest", "bug", "other:quest:active", "quest:merge"]) + expect(KIX_LABEL).toBe("quest:kix"); + for (const label of [ACTIVE_LABEL, KIX_LABEL]) expect(isHoldLabel(label)).toBe(true); + // Labels older quest versions added, and quest:merge, hold nothing. + for (const label of [ + "quest", + "bug", + "other:quest:active", + "quest:merge", + "quest:iterate", + "quest:ready", + "quest:custom", + ]) expect(isHoldLabel(label)).toBe(false); }); diff --git a/test/hold.test.ts b/test/hold.test.ts index 7893cdc5..3e14b78b 100644 --- a/test/hold.test.ts +++ b/test/hold.test.ts @@ -236,18 +236,15 @@ describe("taking a PR", () => { expect(pull.labels).toEqual(["quest:active"]); }); - test.each(["quest:iterate", "quest:ready", "quest:kix"])( - "refuses while %s holds it, as an older quest version or kix adds", - async (label) => { - const pull = new FakePull(); - const token = pull.label(label); - expect(await refused(take(PR, undefined, pull.deps()))).toContain(`held by ${label}`); - expect(pull.labels).toEqual([label]); - // Its token releases it. - expect(await release(PR, token!, pull.deps())).toBe(label); - expect((await take(PR, undefined, pull.deps())).label).toBe("quest:active"); - }, - ); + test("refuses while kix holds it, and its token releases it", async () => { + const pull = new FakePull(); + const token = pull.label("quest:kix"); + expect(await refused(take(PR, undefined, pull.deps()))).toContain("held by quest:kix"); + expect(pull.labels).toEqual(["quest:kix"]); + // Its token releases it. + expect(await release(PR, token!, pull.deps())).toBe("quest:kix"); + expect((await take(PR, undefined, pull.deps())).label).toBe("quest:active"); + }); test("ignores the quest:merge label older quest versions left on queued PRs", async () => { const pull = new FakePull(); @@ -260,10 +257,10 @@ describe("taking a PR", () => { const pull = new FakePull(); pull.beforeAdd = () => { pull.beforeAdd = undefined; - pull.label("quest:iterate"); + pull.label("quest:kix"); }; - expect(await refused(take(PR, undefined, pull.deps()))).toContain("held by quest:iterate"); - expect(pull.labels).toEqual(["quest:iterate"]); + expect(await refused(take(PR, undefined, pull.deps()))).toContain("held by quest:kix"); + expect(pull.labels).toEqual(["quest:kix"]); expect(pull.writes).toEqual(["add quest:active", "remove quest:active"]); }); @@ -271,24 +268,33 @@ describe("taking a PR", () => { const pull = new FakePull(); pull.afterAdd = () => { pull.afterAdd = undefined; - pull.label("quest:iterate"); + pull.label("quest:kix"); }; expect(await take(PR, undefined, pull.deps())).toEqual({ label: "quest:active", token: "LabeledEvent_1" }); // The later session sees the earlier label and backs off. - expect(pull.labels).toEqual(["quest:active", "quest:iterate"]); + expect(pull.labels).toEqual(["quest:active", "quest:kix"]); }); - test("keeps the hold its token names, and swaps an older version's label for quest:active", async () => { + test("keeps the hold its token names", async () => { const pull = new FakePull(); const hold = await take(PR, undefined, pull.deps()); expect(await take(PR, hold.token, pull.deps())).toEqual(hold); expect(pull.writes).toEqual(["add quest:active"]); + }); - const legacy = new FakePull(); - const token = legacy.label("quest:ready")!; - expect(await take(PR, token, legacy.deps())).toEqual({ label: "quest:active", token: "LabeledEvent_2" }); - expect(legacy.labels).toEqual(["quest:active"]); - expect(legacy.writes).toEqual(["add quest:active", "remove quest:ready"]); + test("refuses a token naming a label that holds nothing", async () => { + const pull = new FakePull(); + const token = pull.label("quest:ready")!; + expect(await refused(take(PR, token, pull.deps()))).toContain("quest:ready is off"); + expect(pull.writes).toEqual([]); + }); + + test("refuses a token naming kix's hold rather than taking it over", async () => { + const pull = new FakePull(); + const token = pull.label("quest:kix")!; + expect(await refused(take(PR, token, pull.deps()))).toContain("names the quest:kix hold"); + expect(pull.labels).toEqual(["quest:kix"]); + expect(pull.writes).toEqual([]); }); test("refuses a token once another session holds the label", async () => { @@ -343,9 +349,9 @@ describe("merge --land", () => { expect(await refused(land(PR, HEAD, "merge", pull.deps()))).toContain( `held by quest:active, added just now as token ${token}`, ); - pull.label("quest:iterate"); + pull.label("quest:kix"); await release(PR, token, pull.deps()); - expect(await refused(land(PR, HEAD, "merge", pull.deps()))).toContain("held by quest:iterate"); + expect(await refused(land(PR, HEAD, "merge", pull.deps()))).toContain("held by quest:kix"); pull.labels = ["quest:merge"]; await land(PR, HEAD, "merge", pull.deps()); expect(pull.writes.at(-1)).toBe(`enablePullRequestAutoMerge ${HEAD} MERGE`); diff --git a/test/land-watch.test.ts b/test/land-watch.test.ts index 1faac133..e1ef03c2 100644 --- a/test/land-watch.test.ts +++ b/test/land-watch.test.ts @@ -782,7 +782,7 @@ test("an armed pick withdrawn, retargeted, or held while the pass reads it is no if (pull.number === 33) pull.base = "release"; // Disabled and armed again: a new pick, which the next pass weighs on its own. if (pull.number === 34) pull.armed = at(3); - if (pull.number === 36) pull.labels = ["quest:iterate"]; + if (pull.number === 36) pull.labels = ["quest:kix"]; }; expect(await watchPass(REPOSITORY, { state }, deps(github))).toEqual([]); expect(github.mutations).toEqual([]); @@ -1867,7 +1867,7 @@ git commit -qm 'disable render_timeout'`; reads.set(pull.number, read); if (takeAt.get(pull.number) === read) pull.labels = [...(pull.labels ?? []), "quest:active"]; }; - pulls[3].labels = ["quest:iterate"]; + pulls[3].labels = ["quest:kix"]; pulls[4].labels = ["quest:active"]; github.refuse = ["#7 quest land watch: Another session holds"]; github.now = Date.parse(at(7)); diff --git a/test/status.test.ts b/test/status.test.ts index 7bce1b5c..990dbf19 100644 --- a/test/status.test.ts +++ b/test/status.test.ts @@ -568,7 +568,7 @@ const holdEvent = (id: string, label: string, at = "2026-10-07T11:00:00Z") => ({ }); test("reports the latest acquisition, age, and verified paged event URL for each hold label", async () => { - const label = "quest:iterate"; + const label = "quest:kix"; const body = pull({ checks: null, reviews: page([]), @@ -596,11 +596,11 @@ test("reports the latest acquisition, age, and verified paged event URL for each test("retains a hold fact when its acquisition is outside the available timeline", async () => { const fixture = setup([ - pull({ checks: null, reviews: page([]), threads: page([]), labels: { nodes: [{ name: "quest:ready" }] } }), + pull({ checks: null, reviews: page([]), threads: page([]), labels: { nodes: [{ name: "quest:kix" }] } }), ]); const status = await pullRequestStatus(pr, fixture.deps); - expect(status.holds).toEqual([{ label: "quest:ready", token: null, takenAt: null, age: null, eventUrl: null }]); - expect(statusLines(status)).toContain("hold quest:ready - - - unknown age"); + expect(status.holds).toEqual([{ label: "quest:kix", token: null, takenAt: null, age: null, eventUrl: null }]); + expect(statusLines(status)).toContain("hold quest:kix - - - unknown age"); expect(fixture.eventRequests).toEqual([]); });