fix(server): wake PR watches when the merge queue drops a pull request#24142

Approvability

Verdict: Approved at 86f6522

Macroscope's review found this PR approvable — This is a focused fix to an existing pull-request watch: GitHub merge-queue removals are surfaced once through the established notification path. The changes are backward-compatible, scoped to the existing provider/watch pipeline, and supported by targeted tests.

^{You can add or adjust custom eligibility rules. Learn more.}

Explain Change →

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

updated takeover at e68b33b1a4:

• merged current main with a merge commit. no rebase or force-push. the one conflict was the generated pullRequestReads.graphql.ts (main added label description, this pr added the queue-removal selection). both kept; graphqlgen reproduces the resolved file.
• took two fixes from yaacovcorcos's real merge-queue probe:
  • 19fc0166cc: the watch fingerprint counts filteredCount, since totalCount ignores itemTypes and any timeline event forced a full read.
  • e68b33b1a4: removals with reason merged no longer wake the agent. GitHub writes one up to a second before the pr reads as merged. one new case in the existing pullRequestWatch.test.ts.
  • left out the reason-code sentences to keep the change's size; agents still get GitHub's raw reason.
• first-read clock skew (bot thread on pullRequestWatch.ts): kept as is. only the first read compares timestamps, using the same startedAt rule remarks use; later reads compare ids.

verification through blacksmith-testbox: vp run check:changed github/main passed format, lint, both knip checks, both graphql checks, typecheck of 37 packages, and 451 tests in 8 files.

unverified by me: a live removal waking a provider turn. yaacovcorcos's probe above shows the read path against a real queue.

merge-queue removal remains unverified. this pass verified the real watch pipeline and neighboring behavior, but could not reproduce the original bug or prove the new removal wake.

Verified in a real dev environment

• installed and ran github/main at 40d335a7ba in ../main, then stopped it before running the exact pr head 52e41a6fad9693b576e6b9fa86a36d55c2578c81. only one dev server ran at a time, both in the background with isolated empty state.
• used repository playwright and /usr/bin/google-chrome. added the real repository as a project through the app and submitted real codex gpt-6.1-sol turns through the web composer. no runtime mocks, fixtures, or demo data.
• on main: codex called watch_pull_request for this pr; the watch survived a reload; a subsequent codex turn called unwatch_pull_request and confirmed the pr stayed linked with watching: false.
• on the pr: codex started the watch; it survived a reload. clicking the web stop control removed it, and a reload retained the stopped state. restarting the watch then read github's 8 passing required checks, persisted a #24141: checks passed notice, and automatically resumed a real codex turn. that turn stopped the watch and left the pr linked.
• all 267 tests passed in the four existing watch, runtime, service, and github decoding suites through blacksmith-testbox. check output. the first root-level attempt failed loading an excluded mobile dependency; running from the owning packages resolved that setup issue. hosted ci reports 27 successful checks.
• runtime transcript. one startup event-loop stall of 9201 ms recovered; no provider turn stalled for 10 minutes. both dev servers are stopped. no source changes or pushes.

main baseline, ordinary watch active. this is not a reproduction of a queue drop:

main baseline with a real codex watch active

pr head, live required-check notification and automatically resumed codex turn:

pr head with checks-passed notice and resumed real codex turn

pr head, clicking stop removes the watch. the startup stall warning is visible at the beginning. this clip shows the neighboring stop interaction, not a merge-queue removal:

pr web stop control clears the active watch

Not verified

• the changed merge-queue behavior, its reason text, and once-per-removal deduplication through a live provider turn. github's active main-branch rules are deletion, required_status_checks, and pull_request, with no merge-queue rule. this pr has no queue entry and its removal-event query returns an empty node list. generating the event would require changing repository rules or using another pr. neither was done. consequently the original main bug was not reproduced, and this pass does not certify the fix end to end.
• native desktop, ios/android, macos/windows, ghes, other forge providers, comment/conflict/close/merge transitions, and server-restart deduplication. only the web client, codex, and live github reads were exercised here. the focused tests cover additional logic, but do not replace live merge-queue proof.
• inline clip playback in the forge comment. reopening it confirmed that github renders the screenshots, but its image renderer does not play the mp4; git.cafe renders the external media as links. the uploaded mp4 itself was opened in chrome, played, and sought successfully.

@aravhawk Fair question. The timestamp is there because the watch is created with a start time but no baseline removal ID, so the first read needs some way to tell a removal after the watch started from an older one. An ID-only check against the current empty field would report any historical removal.

Your clock-skew concern is valid, though: a server clock ahead of GitHub's can reject a real post-start removal and still save its ID, which suppresses it for good. An explicit ID baseline set when the watch starts, with a distinct "initialized, no removal" state, would avoid comparing clocks. Maria owns the branch now, so I'll leave that change to her.

On the other points: the pre-watch ID assignment is intentional and deserves an inline comment; only the newest removal is reported (last: 1), which should be documented or tested; and the GHES gate is a hostname check rather than a capability check, so the description should state the actual limit.

Re your review: where each point stands at 7c63ff1c4c:

• Clock skew: kept as is. Only the first read compares timestamps; later reads compare removal ids, so skew no longer matters after that. maria's reasoning is in https://github.com/pingdotgg/t3code/pull/24141#discussion_r4488372186.
• Pre-watch removal id: intentional, and now commented in pullRequestWatch.ts (661f904918).
• Newest removal only: the description now says it. The read asks GitHub for last: 1, so two removals between passes give one wake with the newer reason, and the older one is not reported.
• Enterprise Server: the description now says the gate is a hostname check, not a capability check. The 3.18 and 3.22 schemas include the event, but no Enterprise Server has been run, so the gate stays for now.
• Merge with main: mergeQueueRemovalId survived it, watchesEqual compares it (PullRequestWatchReactor.ts:181), and 52e41a6fad tests that the stored id persists across reactor restarts.

On "merge-queue removal remains unverified": here are real removals from a real merge queue, read through this PR's own code.

Setup. A public throwaway repo, ScientFactory/merge-queue-watch-probe, has a merge queue (MERGE, one entry at a time) and one required check, check. That check fails only on merge_group, and only when FAIL_IN_QUEUE exists. The probe reads the PRs with this PR's real GitHubPullRequestApi: getPullRequestWatchFingerprint and getPullRequestDetail. It builds the layer from GitHubPullRequestApi.test.ts with FetchHttpClient and a real token. It waits 10 s between rounds of reads, and 250 ms while the last PR lands. It then replays every read, in order, through evaluatePullRequestWatch and pullRequestWatchMessage. As the reactor does, the watch ends on the first read that is not open.

Result:

In the run with #4–#6 (58 reads), each PR also woke once for its own passing check. Each removal woke once, and later reads did not repeat it. An earlier run (#1–#3, at 7c63ff1c4c unchanged) gave the same wakes. #7 was a single read after its removal.

So the removal wake works against GitHub. Three things turned up that the fixtures can't show:

1. GitHub also records a removal when the queue merges. Each time the queue lands a PR, GitHub writes a RemovedFromMergeQueueEvent with reason merged, at the same second as MergedEvent or one second before it. Examples: probe #3, removal 23:46:54Z and merge 23:46:55Z; ScientFactory/scient-desktop#801, 14:34:33Z and 14:34:34Z. Neither probe run caught a read in that gap. If a read sees the removal while the PR still reads as open, this PR wakes the agent with "removed from the merge queue without merging: merged".
2. Reasons are codes. GitHub's schema types reason as a plain string and lists no values. Across all 471 removals on scient-desktop since 2026-09-29, it sent 363 merged, 73 failed_checks, 22 manual, 5 invalid_merge_commit, 4 merge_conflict and 4 checks_timed_out. The tests here use "Merge conflict" and "CI failed", so a real wake reads "…without merging: failed_checks."
3. totalCount ignores itemTypes. On timelineItems(itemTypes: [REMOVED_FROM_MERGE_QUEUE_EVENT]), totalCount counts every timeline item. Probe #3 had no removals and read 1. Probe #2 went from 1 to 2 to 3 as it was queued and then removed. filteredCount gives 0, 1 and 0. The wake still fires, because a removal still changes the number. But any timeline event (queued, labelled, assigned) forces a full read.

Two commits on top of 7c63ff1c4c fix these: compare.

• yaacovcorcos/t3code@2cebedd3e7: skip merged removals, and turn failed_checks, checks_timed_out, merge_conflict and manual into sentences. Other codes, such as invalid_merge_commit, still show as GitHub gives them. A failed_checks or checks_timed_out wake says the checks ran on the queue's own commit, not on the head, since the head stays green. Its new tests fail on 7c63ff1c4c.
• yaacovcorcos/t3code@0b0375464c: the fingerprint counts filteredCount. It updates the existing fingerprint test so that a timeline item that is not a removal must leave the status alone. On 7c63ff1c4c that test fails.

Both merge cleanly with main at 8ee7183e6c. On that merge, the watch and GitHub tests pass, server and source-control-github typecheck, lint passes, and graphqlgen leaves the generated files unchanged.

Cherry-pick them, or take only the ideas, whichever suits you. I'll keep the probe repo up until this merges, so the events stay inspectable.

@yaacovcorcos Thanks for running this on a real merge queue. Your four removals answer the "remains unverified" note, and they turned up the merged removal and the totalCount count, which our own fixture run today also showed. In that run, in the full app, the watching thread never woke for a failed_checks or manual removal on current main and woke once for each on 7c63ff1c4c. Your 2cebedd3e7 and 0b0375464c cover everything our local fix did and also turn the reason codes into sentences, so we would take yours over ours. maria-rcks owns this branch now, so whether to pick them up is her call.

@maria-rcks I ran your open item on a real GitHub merge queue in the public fixture repo only21mil-test/merge-queue-fixture. Main at f697d4db3e (the main your merge took in) and e68b33b1a4 ran side by side, each a T3 server whose thread watched the same fixture pull requests, with a real Codex turn (gpt-6-luna) behind each thread. Each wake below started a Codex turn that answered with the item, and no removal woke a thread twice on the sweeps through 08:48Z:

• failed_checks removal (GitHub 08:35:01Z): main never woke; e68b33b1a4 woke once at 08:36:07Z with "It was removed from the merge queue without merging: failed_checks."
• manual removal (GitHub 08:38:32Z): main never woke; e68b33b1a4 woke once at 08:40:08Z with "It was removed from the merge queue without merging: manual."
• Queue merge of fixture PR 3 (MergedEvent and the merged removal both at 08:43:09Z): neither thread woke, and both ended the watch as merged (main 08:44:02Z, e68b33b1a4 08:44:09Z).

Not checked: the one-second window e68b33b1a4 guards did not occur, since the next sweep already read PR 3 as merged, so its unit test is still the only cover for that branch. I opened the captures in the Browser panel by clicking each thread; nothing was typed or sent.

e68b33b1a4, both removals and their wakes:

e68b33b1a4: one wake for the failed_checks removal and one for the manual removal

e68b33b1a4, after the queue merged PR 3:

e68b33b1a4: no wake for PR 3's merged removal

Main f697d4db3e, same events, no wake:

main f697d4db3e: no wake for either removal