Repository navigation
Gate knowledge PRs the service did not open, and issue human-approved verdicts - #215
Conversation
… verdicts
Two paths from the design that had no service side yet. Human-written
knowledge PRs (source 4) need a signed verdict from the gate. PRs that the
auto gate cannot pass (knowledge/tools, skills, ...) need a knowledge
maintainer's approval turned into a human-approved verdict. Until now every
such PR failed kb-gate forever.
- external.poll_external runs from the scheduler (lease held, not globally
paused). It takes each open, non-draft PR that touches knowledge/ and was
not opened by the service, and judges it once per head and path (auto or
human), so a head sent to people on the auto path is judged again as soon
as a maintainer approves it, without a new push:
- kb:human-approved with a knowledge maintainer's (CODEOWNERS of
.github/kb-gate/ on main) standing approval of the CURRENT head: L1 on
the governed pages (the approval replaces the whitelist and L2, never the
rest of L1), then a signed human-approved verdict bound to those reviews.
A later review by the same person replaces an earlier one, read across
every page of reviews;
- otherwise, all paths governed: the full gate (L1, L2, consistency) on
current main plus the PR's changes; a pass gets an auto verdict. The PR's
pre-images must equal main's, or the author is asked to rebase;
like the service's own PRs, an auto verdict needs a current passing
judge calibration (calibration_required otherwise, re-judged only once
calibration is current, so no repeated paid judge calls);
- anything else goes to people once per head.
- The verdict manifest comes straight from git (the full merge-base..head raw
diff), so it covers non-governed paths; context_base_sha is main at judging.
Tests run the real kb-gate verifier on both kinds of verdict.
- An external change set follows the ordinary merge flow. A new head
(head_changed) or a moved context (stale_context) is judged again at the
next poll instead of going to people, and an author closing their own PR
does not trip the overturn breaker.
- A withdrawn approval stops the PR. After signing, every pass re-checks that
each named maintainer's latest review still approves the signed head; if
not, it goes to people and a pause item dequeues it and makes it a draft.
It keeps its status, and so the repository's queue slot, until that pause
is acked (a failed pause is retried), and only then becomes
approval_withdrawn. Until then it keeps holding the queue slot even if it
was verdict_posted with an enqueue in flight, since it may have been queued
meanwhile. A pause acked while an earlier signed enqueue could still run
(the publisher may execute them in either order) does not complete it: the
pause is repeated once that enqueue has certainly expired (plus a clock-skew
margin). Completion is decided by when the acked pause was ISSUED (recorded
on the pending item), never by when its ack arrived. The enqueue expiry is
recovered at withdrawal from every durable record (the recorded value, the
pending item, and unacked signed enqueue items still in the outbox), so a
restart or a pre-upgrade change set cannot lose it. Remaining approvals
never re-stage that head. The kb-gate verifier (bundle
regenerated) now also checks each reviewer's latest review, not only that
the bound review still reads APPROVED.
- External change sets carry no operations, so the rules L1 finds them
retiring or purging are stored and entered in the retirement ledger on
merge, like our own, so later release sweeps can purge them.
- KnowledgeRepo gains fetch_pull (a PR head fetched as data, never checked
out), merge_base, show and raw_manifest.
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: b8dff3d67e
ℹ️ 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".
| if head != changeset["head_sha"] and changeset["kind"] == "external": | ||
| # authors push: the new head is simply judged again at the next poll | ||
| rt.ledger.update_changeset(changeset["id"], status="head_changed", pending_item=None) | ||
| events.append(f"head_changed {changeset['id']}") |
There was a problem hiding this comment.
If an external PR is staged while ready and its author subsequently converts it to draft, this path only checks the head and continues signing and enqueueing it. Publisher._do_enqueue then calls gh pr ready for any draft PR (confirmed by gh pr ready --help: “Mark a pull request as ready for review”), so the service can undo the author's draft decision and merge work that was deliberately withdrawn from review. Re-check the draft state for external changesets and pause or retire them instead of applying the service-owned-PR resume behavior.
Useful? React with 👍 / 👎.
| for pr in _pulls(rt): | ||
| number = int(pr["number"]) | ||
| head = str((pr.get("head") or {}).get("sha") or "") | ||
| labels = {str(label.get("name")) for label in pr.get("labels") or []} | ||
| human = HUMAN_LABEL in labels | ||
| if pr.get("draft") or not head or _ours(rt, number) or _judged(rt, number, head, "human" if human else "auto"): |
There was a problem hiding this comment.
For a PR targeting any branch other than main, the poller still judges its diff against current main and later enables auto-merge on the PR without inspecting pr["base"]. gh pr merge --help confirms that --merge merges into the PR's base branch, so such a PR can be recorded as merged—including retirement-ledger updates—even though its knowledge changes never reached the main branch used by activation and subsequent gates. Skip or send non-main-based PRs to people before staging them.
Useful? React with 👍 / 👎.
| if changeset["kind"] == "external" and changeset["detail"].get("source") == "human-approved" \ | ||
| and changeset["status"] in ("verdict_posted", "queued"): |
There was a problem hiding this comment.
When a maintainer retracts approval after poll_external stages a human-approved changeset but before its first advance, the changeset is still pr_open, so this condition skips the approval check and signs a stale verdict. The verifier then rejects the comment and the merge flow moves the changeset to gate_failed; because that status is treated as already judged, restoring approval on the same head cannot recover it without another push. Include pr_open in the recheck and handle withdrawal before issuing the verdict.
Useful? React with 👍 / 👎.
Knowledge-service redesign follow-up. PRs the service didn't open now get a path to a signed verdict: human-written knowledge PRs (source ④), and changes outside the auto-merge whitelist via maintainer approval. Before this, both failed kb-gate forever.
external.poll_external(from the scheduler, holding the lease): judges each open, non-draft PR that touchesknowledge/and wasn't opened by the service, once per head and path.kb:human-approvedand a knowledge maintainer's (CODEOWNERS of.github/kb-gate/) standing approval of the current head, read across all review pages. Then L1 on the governed pages, then a signedhuman-approvedverdict.merge-base..headdiff), so non-governed paths are covered. Tests run the real kb-gate verifier on both verdict kinds.Commit hook approved; full suite, SPEC freshness and bundle check pass locally.
🤖 Generated with Claude Code