fix(pipeline): guard GitHub fork PR destinations - #891
Conversation
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (2): Last reviewed commit: "no-mistakes(review): Order superseding P..." | Re-trigger Greptile |
|
Speaking as Kun's firstmate: This is a captain-decision hold, not waiting on you. Tip We will not auto-merge. Unconfigured GitHub fork-mode used to raise the parent PR on a gate push (#293). This tip requires a leading contract-class: new-default VISION (tip
|
|
Correction to #891 (comment): that inline reply is superseded. The accepted contract does not require AXI-only provenance and does not reject the existing direct-push intent transport. Authoritative, structure-preserved explicit intent from |
Intent
Superseding Decisions
Later recorded decisions take precedence over conflicting wording in the original intent below. Within this list, later entries supersede earlier conflicting entries.
Authoritative, structure-preserved explicit intent from
no-mistakes axi run --intent,no-mistakes rerun --intent, or the existing notify-push push-option intent may authorize publication whenPR destination: owner/repomatches the selected parent.An intermediate review decision restricted fork-PR authorization to AXI provenance and rejected the direct-push intent transport.
The final decision restores the first rule: authoritative, structure-preserved explicit intent from AXI, rerun, or notify-push may authorize publication. No AXI-provenance machinery remains.
The final entry is authoritative. It also corrects the earlier cost disclosure: it was overstated to say that direct pushes necessarily validate and push before refusing. Only a bare direct push without authoritative intent takes that late-refusal path; a direct push carrying the existing authoritative notify-push intent can authorize publication. The earlier AXI-only wording in this PR body and review discussion is superseded.
What Changed
PR destination: owner/repodeclaration carried by authoritative, structure-preserved run intent.created_atand stable ID, and repeatedA -> B -> Areversals keep the finalA.Closes #884.
Maintainer Reproduction
The maintainer reproduced the defect on current main at
13cdf1aac70e7cd07525e1729f3dd2399bb76217:buildHostininternal/pipeline/steps/host.goselected GitHub--repofromRepo.UpstreamURL, usedRepo.ForkURLonly for the--headowner, andPRStepcould callCreatePRwithout checking that repository choice against the run's delivery intent. This change implements that already-established diagnosis.In our own contribution workflow, we do not yet carry a local automatic guard ourselves; we currently check the target manually.
Observable Publication Contract
pr list,pr edit, orpr createoperation. Ifghis absent or unauthenticated, the existing provider-unavailable skip remains unchanged.<fork-owner>:<branch>as its head, preserving the upstream-contribution path from fix(scm): no-mistakes cannot ship fork-based contributions — push and PR share one upstream_url, no fork→parent routing #293.axi run --intent,rerun --intent, and the existing notify-push push-option intent are all accepted when they preserve the explicit leading destination structure. Repository topology, issue links, incidental prose, and transcript text do not grant authorization.Test Evidence
The narrow regression was added first and run against unmodified
13cdf1aac70e7cd07525e1729f3dd2399bb76217with:Red before: the mismatch, unknown, and ambiguous cases failed with
Execute() error = nil, and the fake GitHub CLI transcript proved PR creation remained reachable. Green after: the same command passed, proving refusal before PR operations and preserving the positive explicit-parent case.Two executable end-to-end proofs also pass:
TestForkRoutingproves an authoritative explicit parent destination completes with a fork push and parent PR.TestForkRoutingDirectPushRefusesPRWithoutDestinationproves a bare direct push completes validation and fork push, then refuses before PR publication. The focused PR-step suite additionally covers the provider-unavailable skip, literal.gitrepository names, mismatch/unknown/ambiguous refusal, explicit-parent publication, and generated decision chronology.For the body-history correction, reverting only the chronology implementation reproduced the old
A -> Brendering with the finalAmissing. At17a39993e318008f859311ccc0242d4e035d2a1c, the focused generated-body test rendersA -> B -> Aand exactly one live pipeline attestation.Broad validation completed with
make lint,go test ./...,make e2e, andgo build -o ./bin/no-mistakes ./cmd/no-mistakes. The managed no-mistakes review, test, document, lint, and push stages completed for exact pushed head17a39993e318008f859311ccc0242d4e035d2a1c. This PR does not claim a green localgo test -race ./...; two attempts hit the repository-documented macOS fork/pre-exec failure in unrelated existing tests.Scope
This deliberately does not implement issue #552's branch/base binding, add a new push-time destination channel, add authorization/provenance machinery, or redesign unrelated SCM routing. The declaration remains
owner/repo; host-qualified destination authorization is a separate follow-up.Pipeline
Updates from git push no-mistakes
17a39993e318008f859311ccc0242d4e035d2a1cpublished fromtiago-peixoto:fm/no-mistakes-pr-destination-guard.