Skip to content

pr-type consult hard-fails when the PR is already merged: merge-on-receipt workflows can't complete SPIR's review phase #1306

Open

Description

@waleedkadous

Problem (production report, second occurrence class in a week)

Workspaces whose owner's standing word is "merge on receipt" merge the PR before porch reaches the review phase's pr-type consult. The consult then hard-fails — No PR found for branch: <branch> — even though every substantive review (spec, plan, per-phase) already ran. The workspace's only exits are verify --skip ceremony per project, or the #1297 interim manual-repair procedure. Both are workarounds for an ordering assumption, not policy.

Root cause chain (verified)

  1. consult/index.ts:1628-1639 (findPRForCurrentBranch) resolves the PR via the pr-search forge concept.
  2. scripts/forge/github/pr-search.sh execs gh pr list --search ... — and gh pr list defaults to open PRs only. A merged PR is invisible, so merge-before-consult guarantees the failure. (Same root as the swapped-PR pr_exists blocker hit in this repo's spec-1252 endgame.)
  3. History note: the closed-PR exclusion came in via Better handling of builders that stop mid-protocol and create premature PRs #653 deliberately (a stale closed PR must not match as "the" PR). The fix must not regress that.

Asks

  1. New forge concept (per the arch rule: dedicated concept, not env flags on a shared one) — e.g. pr-search-merged: search PRs including merged state, both github/ and gitlab/ implementations. Keeps pr-search's Better handling of builders that stop mid-protocol and create premature PRs #653 semantics untouched.
  2. Consult --type pr falls back to the merged PR when the open search returns nothing: resolve via the new concept, review the merged diff (gh pr diff works on merged PRs), and mark the review as post-merge advisory — semantically identical to ASPIR's verify-phase review of already-merged work. Prefer the MOST RECENTLY merged match for the branch to respect Better handling of builders that stop mid-protocol and create premature PRs #653's staleness concern.
  3. Porch: a sanctioned merged-outside-ordering closure for the pr step — record the merge SHA and proceed — so merge-on-receipt workspaces don't need verify --skip ceremony per project. Overlaps porch cannot express merged-outside-the-loop: add 'porch reconcile' for evidence-based terminal state repair #1297 (porch reconcile); this is the narrower, ordering-specific variant that keeps the review phase itself running (the consult still happens, just against the merged diff).

Non-goal

Changing default gate ordering. For workspaces that hold the PR open through the gate (this repo's convention), nothing changes.

Activity

  1. waleedkadous commented on Jul 31, 2026

    @waleedkadous
    ContributorAuthor

    Additional finding: the merged status.yaml is systematically one step stale — even in the healthy flow

    Follow-up from the same reporting workspace revealed a second store-shaped confusion worth folding into this issue's design:

    1. Porch has exactly ONE store — codev/projects/<id>-<name>/status.yaml resolved from wherever porch runs. There are no porch tables in ~/.agent-farm/global.db (verified: none exist). The reporter's "yaml-vs-db divergence" hypothesis is disproven; afx status's phase column comes from the builders registry and is famously unreliable — watchdogs must never key on it.
    2. But there are two COPIES of the yaml: the builder-worktree copy porch actually writes, and the snapshot that shipped to main inside the PR. Any porch transition that happens AFTER the PR content freezes (the final post-merge advance, verify-phase transitions) lands only in the worktree copy — which afx cleanup then discards. Verified in this repo's own healthy flow: project 1288 completed its protocol fully, yet main's merged status.yaml reads phase: pr forever.
    3. Cosmetic-but-confusing: porch status renders a terminal state as e.g. verified (unknown) because index.ts:189 looks the phase id up in the protocol's phase list, and terminal states aren't phase ids. Reads as corruption; is actually fine.

    Design implication for the merged-outside-ordering closure

    The closure (and #1297's porch reconcile) should define which copy is authoritative at terminal state and make the terminal write land there — either write-through to the main checkout's copy once the PR is merged, or have the pr-phase stamp terminal state before the snapshot freezes. Otherwise every completed project's on-main record understates its own completion, and every future state-repair question starts from the same confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/cross-cuttingTouches multiple areas — needs coordinated handling

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions