F: merge the task's published PR, not github.pullRequest (#121)

[mchwang]

Closes #121. Replaces #122, which GitHub closed when merging #110 deleted its base branch feat/103-publish-pr.

Rebase onto main (head a2c6dba)

The two #121 commits were replayed onto main (after #110, #113, #114 and #111). Changes from the reviewed head cca2dfd:

• README and pull-request-opening.md: main's text kept, with the F: merge the task's published PR, not github.pullRequest #121 sentences added. F: publish only ledger-recorded commits (snapshot head may hold others) #113's note that "pointing the gate at the published PR is a follow-up" now says the gate inspects and merges the task's published PR.
• test/runner-publishing.test.ts: F: publish only ledger-recorded commits (snapshot head may hold others) #113's merge-gate test builds a RemoteMergeState by hand. It now gives the two new fields, pullRequest and draft.

After the rebase: npm run typecheck is clean. Locally, 1565 tests passed. The only failures were the Docker container tests, because Docker is not running on this host. CI's real-docker job is the record for those.

Problem

Since #103 the runner publishes its own pull request for each task. The guarded merge gate still inspected, merged and polled only github.pullRequest. A runner task's merge was therefore always blocked: the configured PR's head is not the task head, and the task's own PR counted as "another PR" in the already-fixed check. It could also target the wrong PR. Merge attempts did not record their PR, and nothing read the draft flag.

Change

• Target (runner/merge.ts, web/server.ts):
  • With a runner block, the gate targets the task's published PR: the opened record in the configured repository and github.baseBranch whose opening began last.
  • GitHub must show that PR from the task branch (not a fork), into that base, with the record's marker as its first line, and as the only open PR from the branch. Anything else is a pull-request blocker.
  • github.pullRequest is optional with a runner block. If it is set and names another PR, merging is blocked.
  • Without a runner block, github.pullRequest is required (the server refuses to start without it) and is used as before.
  • A draft PR is blocked (draft) in both modes.
• Pinning:
  • Every validation pass, and the third pass in queue mode, must resolve the same PR from the same opening.
  • Store.beginMergeAttempt re-reads the record in the transaction that saves the attempt. It must still be the opening that began last in its base, with the same number, and no opening or update may be in flight.
  • The attempt stores pullRequest in its JSON data, so no migration is needed. Reconciliation, the queue watermark, queue polls and gh pr merge use the attempt's PR and never resolve it again.
  • Attempts saved before F: merge the task's published PR, not github.pullRequest #121 use github.pullRequest.
• Gateway (github/merge.ts):
  • Each call takes a target, and the status cache and in-flight read are keyed by it.
  • pr view now also reads and validates number and isDraft. For a published target it also reads body and isCrossRepository, and lists the open PRs from the task branch (one page; a full page fails closed).
  • If a read fails, the parallel reads are stopped and awaited before the inspection rejects.
• Already-fixed check: the PR being merged plus every PR the task records in the repository count as its own.
• Refactors: openingMarker moved to github/pull-requests.ts, and baseBranch moved to github/validate.ts (re-exported from runner/production.ts). The server can now check both without loading the runner.
• Docs: guarded-merge.md (new "Which pull request (F: merge the task's published PR, not github.pullRequest #121)" section), merge-queue.md, pull-request-opening.md, README.

Evidence (before the rebase)

Validated head: cca2dfd.

• npm run typecheck: clean.
• The 12 test files that touch the change: 513 passed. These are merge-task-pr (45, new), merge, runner-shutdown, review, publish, runner-production, cli, runner-publishing, store, gh-env, already-fixed and runner-start.
• The full local suite could not finish: the host was overloaded (load average 160 to 230 from another VM), so tests timed out in files this change does not touch. Each of those files passed when rerun on its own. CI on this head is the full-suite record.
• Guard proofs: each of these guards was removed and its test failed, then the code was restored:
  • the settlement PR check;
  • the second-pass and third-pass target checks;
  • the store guard against a newer opened record;
  • the store guard against an update in flight;
  • stopping the other reads when one fails;
  • the draft blocker;
  • the configured-PR blocker.

Review findings (independent review agent, round 1)

#	Finding	Outcome	Rule
1	The branch-list and rule reads ran in Promise.all. A failing list read left the rule-read gh processes running past the inspection.	Fixed: one read's failure stops the others, all are awaited, and a test holds a rule read.	Existing rule: "A deadline must abort and await the underlying operation…" and the subprocess-settlement rules
2	The server test was close to vitest's 5 s timeout.	Fixed: a 60 s timeout on that describe block.	One-off: host load
3	The replay test passed even if the refusal was never saved.	Fixed: the resend goes to a coordinator that would now merge.	Existing rule: "A test MUST fail when the feature it tests is broken"
4	A publish could start after admission.	No change: already refused by #assertPublishable (needs running or needs human; refuses during an active merge).	Covered by an existing guard
5	An older opening adopted later is not "latest".	Docs and comments now say "the opening that began last". In that case the branch list blocks, which fails closed.	One-off: wording
6	The docs claimed "no recorded PRs without a runner block".	Fixed the wording.	Existing rule: verify behavioural claims
7–10	Gaps in tests: the settlement interleaving, per-target in-flight sharing, store guard branches, the config-mode draft blocker, the third pass, the watermark PR, a legacy poll leaving the attempt unchanged, malformed list entries, and a recorded PR in another repository.	Tests added for all of them.	Existing rule: give each claim a test that fails if it is false

No new AGENTS.md rule: every finding is covered by an existing rule or is one-off.

Copilot review, round 1 (head cca2dfd)

No findings, so nothing changed or was declined. CI on cca2dfd: test and real-docker passed.

Copilot review, round 2 (rebased head a2c6dba)

No findings ("Approval recommended"), so nothing changed or was declined. CI on a2c6dba: test and real-docker passed.

Review findings (independent review agent, round 2, full diff after the rebase)

The reviewer found no high or medium defects. It checked how the gate interacts with #110, #111, #113 and #114:

• Every PR record write moves the task's state version, which admission re-checks.
• cancelTask refuses during an active attempt.
• A publish is refused while a merge is active.

Commit 6b29a8a:

#	Finding	Outcome
1	Nothing tested that the server passes github.pullRequest as the configured PR.	Fixed: a startServer test with a recorded PR #7 and pullRequest: 5 expects the mismatch blocker. Removing the passing makes it fail.
2	#113's unplanned-blocker test ran the gate in configured-PR mode.	Fixed: the test runs in published mode, checks that the target is the recorded PR, and checks that there is no pull-request blocker.
3	When no target resolved, displayStatus showed only that blocker. It hid the approval, task and merge-attempt blockers (for example, an attempt saved before #121 that has merged).	Fixed: the attempt's blocker comes first, then the target's, then the review's. Two tests fail if the old display is restored.
4	A closed newer PR beside an open older one gives only "Pull request is closed".	No change: merging stays blocked, and the doc describes this case.
5	An opening still in flight in another repository would say "wait" for ever.	No change: the reviewer found no path that produces this state.
6	If the PR's branch is deleted, "Refresh later" is misleading.	No change: merging stays blocked, and only the wording is wrong.
7	guarded-merge.md wrote "(refresh)", which reads like a blocker code.	Fixed.
8	Between the final inspection and gh pr merge, a person could retarget the PR's base or edit its first line.	Out of scope: this was already possible before #121, and #121 does not make it worse.

Tests: npm run typecheck is clean. The 5 affected files (merge-task-pr, runner-publishing, merge, review, runner-shutdown): 277 passed. The full local suite is slowed by host load (load average about 150), so CI is the full-suite record.

Follow-ups: none.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation consistently pins the intended PR, fails closed on mismatches, and includes broad race-focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Targets guarded merges at the runner task’s published PR and pins that PR throughout validation, submission, reconciliation, and queue polling.

Changes:

• Resolves and validates published PR identity, draft state, branch, base, marker, and uniqueness.
• Persists the selected PR on merge attempts and uses it for all later operations.
• Adds comprehensive regression coverage and updates documentation.

File	Description
web/​server.ts	Selects published-PR merge mode and validates configuration.
runner/​merge.ts	Resolves, validates, pins, and reconciles merge targets.
runner/​store.ts	Persists target PRs and guards admission races.
runner/​review.ts	Exposes runner-block presence to merge configuration.
runner/​publish.ts	Reuses the shared opening-marker helper.
runner/​production.ts	Re-exports shared base-branch validation.
github/​merge.ts	Adds target-aware inspection, caching, queueing, and merging.
github/​pull-requests.ts	Exports opening-marker construction.
github/​validate.ts	Hosts shared base-branch validation.
test/​merge-task-pr.test.ts	Covers published-PR targeting and race guards.
test/​merge.test.ts	Updates gateway fixtures for PR identity and draft state.
test/​runner-shutdown.test.ts	Updates shutdown merge fixtures.
test/​runner-publishing.test.ts	Updates publishing merge fixtures.
test/​browser/​review.spec.ts	Updates browser merge fixtures.
README.md	Documents optional configured PRs with runners.
docs/​implementation/​guarded-merge.md	Documents target selection and pinning.
docs/​implementation/​merge-queue.md	Documents queue target persistence.
docs/​implementation/​pull-request-opening.md	Documents published-PR merge behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟢 Approval recommended

The target lifecycle is consistently pinned, fail-closed, documented, and covered by focused race regressions.

Review effort: Balanced
Findings: None