Skip to content

fix(release-daemon): verify PR is actually merged on GitHub before writing "landed"#61

Merged
anbangr merged 1 commit into
mainfrom
fix/daemon-verify-pr-state-after-land
May 20, 2026
Merged

fix(release-daemon): verify PR is actually merged on GitHub before writing "landed"#61
anbangr merged 1 commit into
mainfrom
fix/daemon-verify-pr-state-after-land

Conversation

@anbangr

@anbangr anbangr commented May 20, 2026

Copy link
Copy Markdown
Owner

Symptom

The release-daemon writes status: "landed" to its queue records for PRs that are still OPEN on GitHub. Observed in production on 2026-05-19 (mitosis-control-plane release-queue):

Queue says GitHub reality
PR garrytan#120: landed OPEN, e2e check failing
PR garrytan#122: landed MERGED ✓
PR garrytan#124: landed MERGED ✓
PR garrytan#129: landed OPEN, e2e check failing

50% false-positive rate. The queue records are stuck on landed so discoverQueuedRecords (which filters by status === "queued") never re-fans out for them — silently abandoned, lock released, no remote indication of failure.

Root cause

processReleaseQueueRecord at release-daemon.ts:945-951 classified the sub-agent result based ONLY on exit code:

if (landResult.exitCode !== 0 || landResult.timedOut) {
  return safeUpdate(current, { status: "blocked", ... });
}
return safeUpdate(current, { status: "landed" });

But the /land-and-deploy sub-agent (Kimi / Claude / Codex) gracefully declines to merge in real scenarios — failing CI, no PR found, pre-merge readiness gate fails — and still exits 0 (the subprocess itself ran cleanly; the verdict lives in prose inside the output file).

PR#129's sub-agent transcript (~/.gstack/build-state/release-daemon-pr-129/land-and-deploy-output.md):

# /land-and-deploy Report — PR #129
**Status: BLOCKED — CI failure prevented merge**
...
**Verdict: BLOCKED** — /land-and-deploy did not merge or deploy anything.

Subprocess log: exit: 0, timed_out: false. Daemon writes "landed".

Fix

After the land sub-agent returns exit 0, ask GitHub for the authoritative state.

New helper verifyPrMerged(prNumber, repoIdentity, cwd) runs gh pr view <pr> --repo <owner/name> --json state -q .state. Returns { merged: true } only when state is MERGED; anything else → { merged: false, reason: "..." } and the daemon writes "blocked" with a useful lastError.

Security: repoIdentity is parsed via a strict ^github\.com/([^/]+/[^/]+)$ regex before being passed to --repo. A planted record can't sneak shell-specials through to gh.

Robustness: network/auth failures (gh exits non-zero) are reported as merged: false rather than thrown — better to block the record and retry than crash the daemon and leak the lock.

Injection seam: opts.verifyMerged?: typeof verifyPrMerged so tests can stub the response. Defaults to the real verifyPrMerged.

Scope

Pre-existing bug, NOT a regression from PR #57. PR garrytan#120 was marked landed at 12:30:23Z on 2026-05-19, hours before any work on PR #57 began. The bug has been latent in the daemon since /land-and-deploy first started being invoked as a sub-agent. PR #57's deploy verification surfaced it by running through a real PR (garrytan#129) whose CI was failing.

Tests

release-daemon.test.ts: 60/60 passing (5 new regression tests):

  1. ✅ "classifies as 'blocked' when sub-agent exits 0 but GitHub says PR is still OPEN" — reproduces the exact PR#129 production failure pattern
  2. ✅ "classifies as 'landed' when sub-agent exits 0 AND GitHub confirms MERGED" — happy path
  3. ✅ "blocks when GitHub verification fails (network/auth error) — never silent-success"
  4. ✅ "verifyPrMerged rejects unparseable repoIdentity (defense against planted records)"
  5. ✅ "verifyPrMerged accepts a well-formed github.com identity (positive case for the parser)"

Full orchestrator suite: 1525 pass / 16 pre-existing failures (all 16 confirmed pre-existing on origin/main via git stash --include-untracked --keep-index + re-run — these are skill-md.test.ts checks against templates and one timeout in integration.test.ts; unrelated to this PR).

Production remediation for stranded records

Once this lands, the daemon still needs manual revival of PR#120 and PR#129 on mitosis-control-plane (their queue records say landed but they're really OPEN). The existing retryReleaseQueueRecord CLI only handles blocked → queued, not landed → queued.

Two options for follow-up (out of scope for this PR):

  • One-shot audit script: scan every landed record, cross-check against GitHub, revive disagreeing ones to queued
  • Extend the retry CLI to accept --from-landed for records whose GitHub state disagrees

Test plan

  • ✅ Free tests pass (60/60 release-daemon, no new orchestrator failures)
  • After merge: run the audit on ~/.gstack/build-state/release-queue/ to find all landed records that disagree with GitHub state. Revive them via direct disk edit (set status: "queued", clear lastUpdatedAt).
  • After merge: restart the launchctl release-daemon to pick up the new code.

🤖 Generated with Claude Code

…iting "landed"

The daemon used to classify a sub-agent result based ONLY on exit code:
exit 0 = "landed", non-zero/timed_out = "blocked". But the /land-and-deploy
sub-agent (Kimi / Claude / Codex) gracefully declines to merge in real
scenarios — failing CI, no PR found, pre-merge gate fails — and still
exits 0 (subprocess ran cleanly, verdict lives in prose).

Observed in production on 2026-05-19 (mitosis-control-plane release-queue):

  Queue says    GitHub reality
  PR garrytan#120 landed  OPEN, e2e failing
  PR garrytan#122 landed  MERGED ✓
  PR garrytan#124 landed  MERGED ✓
  PR garrytan#129 landed  OPEN, e2e failing

50% false-positive rate. The queue records are stuck on "landed" so
discoverQueuedRecords (which filters status === "queued") never re-fans
out for them — silently abandoned.

Fix: after the land sub-agent returns exit 0, ask GitHub for the
authoritative PR state. New helper `verifyPrMerged(prNumber, repoIdentity,
cwd)` runs `gh pr view <pr> --repo <owner/name> --json state -q .state`
and returns { merged: true } only on MERGED. Anything else (OPEN, CLOSED,
gh exited non-zero, repoIdentity unparseable) returns { merged: false,
reason: "..." } and the daemon writes "blocked" with a useful lastError.

The repoIdentity is parsed via a strict ^github.com/owner/repo$ regex
so a planted record can't sneak shell-specials through to gh — anything
that doesn't match the regex gets rejected before any subprocess runs.

Network/auth failures (gh exited non-zero) are reported as not-merged
rather than thrown — better to block the record and retry than crash
the daemon and leak the lock. The remediation for any blocked record
is the existing retryReleaseQueueRecord CLI.

Injection seam: opts.verifyMerged?: typeof verifyPrMerged so tests can
control the response. Defaults to the real verifyPrMerged.

Scope: pre-existing bug, not a regression from PR #57. PR garrytan#120 was
marked landed at 12:30Z on 2026-05-19, hours before PR #57's work
began. PR #57's deploy verification surfaced it by running through a
real PR (garrytan#129) with failing e2e.

Tests: 60/60 release-daemon.test.ts (5 new regressions covering the
production bug pattern, the happy path, network failures, identity
parsing, and the positive parse case). Full orchestrator suite: 1525
pass / 16 fail, all 16 pre-existing on origin/main (confirmed via
git stash).

Remediation for stranded records on production right now (PR#120,
PR#129 on mitosis-control-plane): once this lands, the daemon needs
manual revival of those two records. The retryReleaseQueueRecord CLI
only handles blocked→queued, not landed→queued. Suggest a one-shot
audit script or extending the retry CLI to support landed records
whose GitHub state disagrees. Out of scope for this PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@anbangr anbangr marked this pull request as ready for review May 20, 2026 01:47
@anbangr anbangr merged commit 361c195 into main May 20, 2026
@anbangr anbangr deleted the fix/daemon-verify-pr-state-after-land branch May 20, 2026 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant