Repository navigation
Merge knowledge PRs through the publisher's local gate (design v8) - #227
Conversation
The owner decided against GitHub's merge queue and rulesets. The publisher
is now the only thing that merges knowledge PRs, and it runs the gate itself
on the exact tree that would land.
- service: at pr_open it signs a verdict bound to the PR's exact head and
issues a merge item carrying it, at most one per repository. There is no
verdict comment, no wait for a GitHub kb-gate status and no enqueue. Acks:
- merged: the merge SHA and the post-merge check result are recorded, and
a failed post-merge check pauses the repository and goes to people.
- refused because context moved, pages changed, the PR does not merge
cleanly or the manifest differs: rebuild_needed, rebuilt on current main
by the scheduler.
- refused for a transient reason (main kept moving, control changed,
git/gh trouble): back to pr_open, signed again.
- refused because of the change itself: gate_failed, people decide. It is
never retried as is.
An unanswered merge item returns to pr_open when it expires. v7
verdict_posted/queued records are signed again as pr_open.
A receipt still counts when GitHub showed the merge first. A post-merge
check of "unknown" (the publisher could not run it, or the merge was
recovered after a crash) is run by the service on its own clone
(verify_merged) and retried until it has an answer.
- local_gate: on the publisher's installed Copilot version it checks the
verdict (signature, binding, auto only), that every path is a governed page
of the item's own repository, runs verify_change on merge-tree(main, head)
with the verdict handed over and no hold list, and checks the merged tree
for new tree-level issues. post_merge_problems repeats this on the merge
commit GitHub actually made.
- publisher merge action, under a cross-process flock for the whole round (a
second instance does nothing):
- verify the verdict; the PR must be open, target main and still have the
signed head;
- run the local gate, again if main moves (at most 3 times);
- re-read the control record (a pause voids the item);
- write a merge intent, then gh pr merge --merge --match-head-commit;
- run the post-merge check.
Nothing after the merge becomes a failure ack. Each round first recovers
intents that have no completion record, so a lost receipt is re-sent and
the PR is never merged twice. When gh pr merge itself errors, the outcome is
unknown: the intent is kept and nothing is recorded or acked; recovery
settles it from GitHub. A kb:hold label left by a v7 pause is removed (and
confirmed gone) before the gate. Only a PR that GitHub shows as MERGED counts: if
main has a merge queue or auto-merge (gh pr merge only enqueues),
the PR is dequeued and auto-merge disabled, and the item is refused only
once GitHub (GraphQL: mergeQueueEntry, autoMergeRequest) confirms neither
is left; any failure while cancelling counts as unresolved; until then the intent stays and recovery keeps
cancelling (or picks up a merge that happened meanwhile). An item with an unsettled
intent is skipped until recovery hears from GitHub.
- gate_verifier.verify_change takes verdict= and check_holds= (the bundle is
regenerated).
- external: its status set is derived from merge.IN_FLIGHT, so a PR whose
merge is in flight is not judged again.
Tests for the v7 queue, status and enqueue mechanics are rewritten for the
merge path, and the human-approved withdrawal/queue-slot tests are removed:
a human-approved verdict never merges in v8, and that code goes in the
removal PR. New end-to-end tests with real git: a merge only after the local
gate passes, a refused change is never merged, a lost receipt is recovered
without a second merge, a second publisher does nothing, and a pause during
the gate stops the merge. Also tested: a delayed receipt with a failed post-merge
check, a merge reported as failed that did merge, pause -> resume -> merge, a
merge that only queued (queue membership modelled apart from auto-merge; a
failing dequeue; a command error while confirming), and a recovery lookup that fails before it succeeds.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3929000985
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "pr": int(intent["pr"]), "head_sha": intent["head_sha"], "branch": "", "error": "", | ||
| "merge_sha": "", "problems": [], "ok": True, "recovered": True, | ||
| "post_check": "unknown"} | ||
| if pr.get("state") == "MERGED" and merged and pr.get("headRefOid") == intent["head_sha"]: |
There was a problem hiding this comment.
When recovery observes state == "MERGED" but GitHub has not populated mergeCommit yet, this condition falls through to the failure branch and sends an ok=False receipt saying the already-merged PR was not merged by the publisher. The normal merge path explicitly anticipates this temporary missing-SHA state, but recovery instead moves the matching change set to gate_failed; because acknowledgements are collected before GitHub is polled, the service then skips retirement bookkeeping and post-merge verification for a change that actually landed. Treat this as a successful merge with an unknown post-check, or leave the intent pending until the SHA is available.
Useful? React with 👍 / 👎.
| if pending.get("kind") == "merge" and float(pending.get("expires_at") or 0) + CLOCK_SKEW < rt.clock(): | ||
| # expired unanswered: the publisher never ran it (or its record | ||
| # is lost); sign again. A merge it did run still shows as merged. | ||
| rt.ledger.update_changeset(changeset["id"], status="pr_open", pending_item=None) |
There was a problem hiding this comment.
A publisher can validate an item just before expiry and still be running its fetch/gate/merge after this timeout fires, since the service and publisher share no execution lock. Clearing the pending ID here allows a second merge item to be issued while the first can still merge; the first receipt is then ignored because its item ID no longer matches, and if the second publisher run sees the PR already merged its matching failure receipt can leave the ledger in gate_failed even though the PR landed. Expiry needs an explicit settlement/cancellation handshake rather than assuming the publisher never started the item.
Useful? React with 👍 / 👎.
| self._recover_merges() | ||
| return self._round(summary) |
There was a problem hiding this comment.
Recovery runs before _round checks either allow_post or the freshness and generation of the signed control record. For an OPEN intent that GitHub queued or configured for auto-merge, _recover_merges calls _cancel_pending_merge, which performs dequeue and disable-auto writes even when ALLOW_POST=0, the control record is stale, or the service has paused/changed modes. This makes a documented dry-run or stale-control round mutate GitHub; recovery should gate its write portion through the same controls as ordinary actions.
Useful? React with 👍 / 👎.
| except GateError as exc: | ||
| return [str(exc)] |
There was a problem hiding this comment.
If merge-base, diff, or another local Git command fails because of an I/O, clone, or process problem, this converts the GateError into a normal gate-problem list. The publisher consequently emits a GateRefused receipt with nonempty problems, while _apply_merge_ack only applies its transient "git " classification when problems is empty, so the change is permanently marked gate_failed instead of being retried. Reserve problem lists for defects in the proposed change (such as a clean-merge conflict) and propagate operational Git failures as LocalGateError/PublishError.
Useful? React with 👍 / 👎.
PR 1 of the v8 knowledge-service change (design approved by sol 2026-09-29; canvas updated): the publisher is the only thing that merges knowledge PRs, and it runs the gate locally on the exact tree that would land. There is no GitHub merge queue, ruleset or kb-gate status wait.
pr_openit signs a verdict for the exact head and issues amergeitem carrying it, at most one per repository. Acks lead to:An
unknownpost-merge check is re-run by the service itself (verify_merged).local_gate: verdict signature, binding and auto only; single-repository scope;verify_changeonmerge-tree(main, head); merged-tree L1; the same checks on the actual merge commit.merge:flocksingle writer;mainmoves;gh pr merge --merge --match-head-commit;The commit hook approved it; full suite and SPEC freshness pass locally.
🤖 Generated with Claude Code