Skip to content

Record the live destination ref separately from the PR comparison base #267

Closed

Description

@joelklabo

Landing inspection currently records GraphQL pullRequest.baseRefOid as the destination revision. On real open Porches PR3347, that field is the historical source base c48a171fabbf2b40e3bb63422ec1858ec7360330, while baseRef.target.oid is current main 36a070385c669b612fd65f80591a276af9580d49. Main advanced without the current LandingRevision changing.

This can miss a destination-context change while a verified PR is queued. Keep the historical comparison base separately and query the actual destination ref target for the landing record, watcher comparison and cancellation validation. Missing or deleted refs must refuse a readiness decision.

The finding is reproduced with GitHub's real open PR, not only a merged PR. The installed upstream pin is 614d2f9. The consuming repository forbids hand-editing vendored pstack. A separate upstream fix is required. Until then the owning shipping pass retains canonical pending-state inspection and explicitly queries the live target before admission and after watcher wakeups. This observation is not an atomic base guard.

Acceptance should cover a real advanced-main PR, moved-target cancellation and preservation of the original comparison base. No branch-protection change or bypass is requested.

Activity

  1. joelklabo commented on Oct 13, 2026

    @joelklabo
    Author

    The live-destination safety defect already has a merged upstream fix in #245 at 472ed21b19bf5434d14446fd4c8924b4967dd64b. The consuming installation is still pinned to 614d2f9791ddf73e0654c9be5a42c5a48d8f5ba0, which predates that fix.

    I checked out the exact merged fix and ran the owning shipping, shipping CLI, GitHub boundary, and watcher integrity tests. All 125 passed. These include stale scalar base values, target movement during cancellation, and missing current refs. No live cancellation or merge mutation was used.

    On the real open Porches PR3347, read-only ship-pr inspect at that exact fix now records 36a070385c669b612fd65f80591a276af9580d49 as revision.baseRefOid. GitHub's independent GraphQL observation still reports historical baseRefOid as c48a171fabbf2b40e3bb63422ec1858ec7360330 and the live baseRef.target.oid as 36a070385c669b612fd65f80591a276af9580d49. The fix therefore observes the required live destination.

    The existing landing field intentionally carries the live destination. The owning shipping records already retain the historical parent separately. No additional provenance-only field or duplicate safety implementation is needed for this incident.

    An isolated preview of the consuming repository's supported vendor sync to the exact fix passed its vendoring check and preserved every project-owned model override, playbook, and verification file. The installation will take up the existing merged fix through that supported sync rather than hand-editing vendored files.

  2. michael-denyer commented on Oct 13, 2026

    @michael-denyer
    Owner

    Thanks @joelklabo. As you found, this was fixed by #245 and released in 0.9.80, so updating your copy to 0.9.80 or later picks it up. I confirmed it on a public PR with a stale base (cli/cli#9847): at 614d2f9 ship-pr inspect records the old base commit, and at 0.9.84 it records the branch's current commit. Closing as already fixed.

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

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions