From d5ec68df4d6e582097427d1fcfb1eddd4686d311 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 19:43:24 -0800 Subject: [PATCH 1/2] chore(quest): claim quest/m1/wait-merge-heads.md Quest-Claim: 864068858b0db1f0 From bc413971bb03f985606a03e989cefb2b3c758490 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 2 Nov 2026 19:46:20 -0800 Subject: [PATCH 2/2] fix(wait): resolve a starting head that a merge into the branch made `quest wait ` matched the point only among pushes, so a head made by GitHub's Update branch or a merge queue's merge failed with "cannot resolve push time". Match it among every move `quest status` counts (exported `BRANCH_UPDATES`), keep reporting only real pushes as `pushed`, and report a merge into the branch after the point as its new commit instead of swallowing it in the baseline. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 +++-- quest/m1/README.md | 1 - quest/m1/wait-merge-heads.md | 26 -------------------- src/cli/main.ts | 2 +- src/cli/status.ts | 2 +- src/cli/wait.ts | 24 +++++++++++++----- test/wait.test.ts | 47 ++++++++++++++++++++++++++++++++++++ 7 files changed, 71 insertions(+), 37 deletions(-) delete mode 100644 quest/m1/wait-merge-heads.md diff --git a/README.md b/README.md index 31c9c53c..2e2948ad 100644 --- a/README.md +++ b/README.md @@ -327,7 +327,8 @@ Authentication comes from `GH_TOKEN`, then `GITHUB_TOKEN`, then `gh auth token`. Without `--since`, the point is the current head's push time, so an existing review on that head is included. GitHub lists a push seconds to minutes after it lands, so for the current head the wait rereads the listing every 15 seconds -until the push appears, warning once. Any other SHA must be a listed pushed tip; +until the push appears, warning once. Any other SHA must be a tip the listing +shows a push, or a merge into the branch such as **Update branch**, moving it to; otherwise pass an ISO timestamp instead. Forks use the head repository's activity. Deleted head repositories require a timestamp. The initial PR commit listing must match the resolved head; a racing push or @@ -338,7 +339,8 @@ commit), type, actor, and URL. The last line adds a resume cursor. Pass its **last field** as `--since` on the next call. Cursors preserve commit membership and events sharing a timestamp; treat them as opaque, and use them only for the same PR. Explicit timestamps include events at that instant. Historical plain -pushes come from repository activity; pushes observed while waiting come from new +pushes come from repository activity, and a historical merge into the branch +prints as its new commit; pushes observed while waiting come from new commit SHAs, regardless of git author dates. Force pushes come from the PR timeline, excluding the one that created the starting head. A merge through the queue prints `removed_from_merge_queue` about a second before `merged`, so a diff --git a/quest/m1/README.md b/quest/m1/README.md index 1c238be9..de0daf25 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -27,6 +27,5 @@ earlier "Later work" milestone became m2. - [Launch material](/quest/m1/launch.md) - README, quickstart, demo, comparison, and launch-post drafts - [Workers in a 5-minute cache](/quest/m1/worker-cache.md) - re-plan #276's worker/coordinator split and gate tickets, after confirming the cache premise - [Cancelling a gate stops its active phase](/quest/m1/gate-signals.md) - SIGTERM/SIGHUP reach every process the gate owns, so Bash traps and GNU `time` can't defer cancelling a phase; unblocks moq-dev/moq.pro#3137 -- [quest wait after a merge](/quest/m1/wait-merge-heads.md) - `quest wait` resolves a head that reached the branch by a merge into it - [One PR's collection failure can't stall its fixer job](/quest/m1/fixer-isolate-failure.md) - errors stay on their PR, and a PR that keeps failing is set aside - [Retire Decisions needed](/quest/m1/decisions-needed.md) - only `## Waiting` hands a draft back, once no open draft uses the old heading diff --git a/quest/m1/wait-merge-heads.md b/quest/m1/wait-merge-heads.md deleted file mode 100644 index 1c68b6b0..00000000 --- a/quest/m1/wait-merge-heads.md +++ /dev/null @@ -1,26 +0,0 @@ -# [XS] quest wait resolves heads that came from a merge into the branch - -## Goal - -`quest wait ` finds when `` reached the PR's head branch -even when it got there by a merge into the branch, such as GitHub's -**Update branch** or a merge queue's merge, instead of failing with -`cannot resolve push time for ; pass a timestamp with --since`. - -## Plan - -Decided 2026-11-02 (maintainer, /quest-plan of the follow-ups from the -2026-11-02 `/quest-complete` session, raised by -[moq-dev/quest#347](https://github.com/moq-dev/quest/pull/347)): ✅ plan it, -m1, since agents merge locally and push, so it's rare, and `--since` works -around it. - -- `quest wait`'s setup (`src/cli/wait.ts`) matches the point only among the - branch's `push`, `force_push` and `branch_creation` activities. Also - accept `pr_merge` and `merge_queue_merge` when resolving the point, the - types #347 taught `quest status`'s enabled-head lookup to count as moving - the head. Export `BRANCH_UPDATES` from `src/cli/status.ts`, or move it to a - module both import, rather than keeping a second list. -- Use that merge-inclusive list only to find the starting point. The pushes - `quest wait` reports keep using the existing push list, so a merge into the - branch is never reported as `pushed`, and force pushes print as before. diff --git a/src/cli/main.ts b/src/cli/main.ts index e616d616..3a039352 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -425,7 +425,7 @@ export function main(argv: string[], out: Output): number | Promise { "Issue-comment edits print as edited, including updates to older bot status comments. The account's own edits are ignored.", ) .argument("", "GitHub PR URL, or a PR number in origin") - .option("--since ", "ISO timestamp, pushed-tip SHA, or resume cursor (default: current head push time)") + .option("--since ", "ISO timestamp, branch-tip SHA, or resume cursor (default: current head push time)") .option("--timeout ", "Maximum wait in seconds", positiveSeconds) .action((value: string, options: { since?: string; timeout?: number }) => { const pr = pullRequest(value, root()); diff --git a/src/cli/status.ts b/src/cli/status.ts index 1d7feb7e..1eafac34 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -185,7 +185,7 @@ export async function pullRequestStatus(pr: PullRequest, deps: GitHubDependencie } /** Repository activity types that move a branch to a new head, including merges into it. */ -const BRANCH_UPDATES = ["push", "force_push", "branch_creation", "pr_merge", "merge_queue_merge"]; +export const BRANCH_UPDATES = ["push", "force_push", "branch_creation", "pr_merge", "merge_queue_merge"]; /** * The head a PR node's auto-merge request was enabled on, or null when that cannot be told. GitHub diff --git a/src/cli/wait.ts b/src/cli/wait.ts index 308b3934..775726ac 100644 --- a/src/cli/wait.ts +++ b/src/cli/wait.ts @@ -15,7 +15,7 @@ import { string, } from "./github"; import { LAND_CHECK } from "./land"; -import { classifyCheck, enabledHead } from "./status"; +import { BRANCH_UPDATES, classifyCheck, enabledHead } from "./status"; const POLL = 5 * 60_000; /** @@ -28,6 +28,9 @@ const FINAL_POLL = 30_000; const PAGE = 100; /** How often setup rereads repository activity while GitHub has yet to list the head's push. */ const LISTING_POLL = 15_000; + +/** Repository activity types a person's push makes; merges into the branch also move it (`BRANCH_UPDATES`). */ +const PUSHES = ["push", "force_push", "branch_creation"]; const SHA = /^[a-f\d]{7,40}$/i; /** The timeline records a force push a few seconds after repository activity (1-2s observed). */ const TIMELINE_LAG = 60_000; @@ -208,7 +211,9 @@ async function watch( const point = options.since ?? currentSha; const shaPoint = SHA.test(point); const time = shaPoint ? undefined : timestamp(point); - const isPoint = (row: ObjectValue) => string(row.after).startsWith(point.toLowerCase()); + // A merge into the branch, such as GitHub's Update branch, can also have moved the head to the point. + const moved = (row: ObjectValue) => BRANCH_UPDATES.includes(string(row.activity_type)); + const isPoint = (row: ObjectValue) => moved(row) && string(row.after).startsWith(point.toLowerCase()); let activities: ObjectValue[] = []; if (head.repo !== null) { const headRepo = object(head.repo); @@ -236,10 +241,8 @@ async function watch( } else if (shaPoint) { throw new Error("head repository was deleted; pass a timestamp with --since"); } - const pushed = activities.filter((row) => - ["push", "force_push", "branch_creation"].includes(string(row.activity_type)), - ); - const match = shaPoint ? pushed.find(isPoint) : undefined; + const pushed = activities.filter((row) => PUSHES.includes(string(row.activity_type))); + const match = shaPoint ? activities.find(isPoint) : undefined; if (shaPoint && !match) throw new Error(`cannot resolve push time for ${point}; pass a timestamp with --since`); const since = time ?? timestamp(string(match!.timestamp)); // An explicit timestamp includes its own instant; a SHA point excludes its own push. @@ -267,6 +270,15 @@ async function watch( // A push racing setup is reported once, as a push, not again as a new commit. pushedTips = plain.map((row) => string(row.after)); for (const sha of pushedTips) baselineCommits.add(sha); + // A merge into the branch after the point is not a push, so it is reported as its new commit. + for (const row of activities) + if ( + moved(row) && + !PUSHES.includes(string(row.activity_type)) && + after(row) && + !pushedTips.includes(string(row.after)) + ) + baselineCommits.delete(string(row.after)); historical = plain.map((row) => ({ point: string(row.timestamp), time: timestamp(string(row.timestamp)), diff --git a/test/wait.test.ts b/test/wait.test.ts index 64cc9c4a..79ff266f 100644 --- a/test/wait.test.ts +++ b/test/wait.test.ts @@ -689,6 +689,53 @@ test("unresolvable pushed SHA fails with a timestamp recommendation", async () = await expect(waitForActivity(pr, { since: B }, fixture.deps)).rejects.toThrow("pass a timestamp with --since"); }); +test("a SHA a merge into the branch made resolves to the merge's time, and the merge never prints as a push", async () => { + for (const type of ["pr_merge", "merge_queue_merge"]) { + const fixture = setup( + [ + pull(), + response([pushed(A, POINT, type), pushed(B, "2026-10-07T11:00:00Z")]), + response([ + committed(B), + committed(A), + reviewed(1, "2026-10-07T11:30:00Z"), + reviewed(2, "2026-10-07T12:05:00Z"), + ]), + ], + [B, A], + ); + const lines = await waitForActivity(pr, { since: A }, fixture.deps); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("pullrequestreview-2 q1."); + } +}); + +test("the current head a merge into the branch made is the default point", async () => { + const fixture = setup([pull(), response([pushed(A, POINT, "pr_merge")]), response([committed(A), reviewed()])]); + const lines = await waitForActivity(pr, {}, fixture.deps); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain("pullrequestreview-1 q1."); +}); + +test("a merge into the branch after the point is reported as its new commit, not as a push", async () => { + const fixture = setup( + [ + pull(undefined, B), + response([pushed(B, "2026-10-07T12:05:00Z", "pr_merge"), pushed()]), + response([committed(A), committed(B)]), + ], + [A, B], + ); + const lines = await waitForActivity(pr, { since: A }, fixture.deps); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatch(new RegExp(`^${B} committed `)); +}); + +test("a branch update that is not a move never resolves the point", async () => { + const fixture = setup([pull(), response([pushed(), pushed(B, POINT, "branch_deletion")])]); + await expect(waitForActivity(pr, { since: B }, fixture.deps)).rejects.toThrow("cannot resolve push time"); +}); + test("SHA lookup paginates repository activity", async () => { const fixture = setup( [