Skip to content

Verification is documented as non-negotiable and enforced by nothing: three PRs merged tonight with no verdict on the artifact, and one shipped a regression #2330

Description

@kyle-sexton

Three PRs merged tonight — #2262, #2263, #2312 — each with zero verification evidence on the artifact. In one case that directly shipped a regression which needed a second PR to undo.

This is not a proposal to add process for its own sake. It is a request to decide whether verification should be a gate or remain an aspiration, because right now it is the latter and the difference just cost a release.

What happened, precisely

time event
02:07:19Z #2262 merged
02:10:54Z Codex posts a P1 — 3 minutes after the merge
02:47Z do-not-merge applied to an already-merged PR; inert
03:06Z regression confirmed on main, filed as #2310
03:31Z #2312 merges the fix

The P1 was neither missed nor overruled. Review ran and was correct; independent verification ran and was sound. Neither gated anything, because the merge did not wait for either. The verifier was explicit that its APPROVE covered only its assigned items and that remote resolution "was outside my item set and I did not assess it" — so nothing contradicted Codex; the finding simply arrived too late to matter.

The same pattern repeated on #2263 and #2312. In one case a verifier produced a full verdict that was never posted, and a separate lane reported "no independent review could be obtained" while three completed verdicts existed elsewhere. A verdict that lives only in an agent transcript is not evidence and dies with the session.

Why the existing mechanisms did not catch it

Each of these works as designed, and none of them is a verification gate:

  • Draft status — the strongest signal available, and the merge wrapper honours it (PR is a draft -- mark ready first). But the author un-drafts themselves, so it gates nothing an author does not want gated.
  • do-not-merge label — honoured by the wrapper, but requires a human or agent to be watching at the right moment. Applied 33 minutes late here, to two already-merged PRs.
  • babysit_merge.py — enforces owner allowlist, draft, mergeable, mergeStateStatus, review decision, unresolved threads, failing/pending checks, unmet required contexts, merge queue, moved head, dependency author, unprotected base. It has no notion of verification evidence. (It also computes requireSignatures and never enforces it — source-control:babysit-prs: babysit_merge computes requireSignatures and never enforces it — a merge held by an unsigned or mis-authored head reports four causes, none of them the real one #2265, same shape one layer down.)
  • Required thread resolution — the ruleset requires it, but the author resolves their own threads.

The proposal, and the trap it must avoid

Make the guarded merge wrapper refuse a merge when the PR carries no verification evidence — a marker comment in an agreed machine-readable form.

The identity constraint is the whole design, not a detail. This repo already learned this the hard way in #2163's D7 fix: a SHA appearing in a comment body proves the SHA was mentioned, not that you posted it — a reviewer quoting your fix satisfies the check while your own write silently failed. The fix was to pin .user.login. A verification marker has exactly the same failure mode: if the author can post their own marker, the gate is decorative. So the check must require a marker from a login other than the PR author, and that constraint should be code, not convention.

Sketch, deliberately loose on format because the shape is the decision:

merge refused unless: a comment exists on the PR
  whose body carries the agreed verdict marker
  AND whose .user.login != the PR author
  AND (optionally) which references the current head SHA

The head-SHA clause is worth debating separately: it makes a verdict stale on every push, which is correct in principle and may be intolerable in practice on a busy lane.

What this does NOT solve, stated plainly

  • A presence check cannot judge quality. It gates on "a verdict exists", not "the verdict is any good", and that invites rubber-stamping. This is a real cost, not a quibble — the honest claim is that it raises the floor from nothing to someone independent looked, which is exactly the gap that shipped source-control: D6's reachability gate hardcodes origin, regressing the skill's non-origin and triangular-fork support #2310.
  • It adds a failure mode: when verifiers stall — which happened repeatedly tonight — the gate blocks legitimate work. It needs a documented, visible override rather than an easy one, and overrides need to be rare enough to notice.
  • It does not help trivial PRs, and forcing verification on a typo fix will train people to route around it. Some scope rule (touched paths, diff size, plugin vs docs) probably belongs in the design.

The decision being asked for

Not "should we add this check" — that is downstream. The question is: should verification evidence be a merge precondition at all, or is the current posture (verification encouraged, merge ungated) the deliberate choice?

Either answer is defensible. What is not defensible is the current in-between, where the process documents verification as non-negotiable and nothing enforces it, so whether it happens depends on whether an agent remembers to publish before it merges. Tonight it did not, three times, and the one time it mattered a regression reached main.

Related: #2265 (a rule computed and never enforced), #2310 (the regression this let through), #2244 (the original defect class — a check that answers confidently about the wrong thing).

Activity

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions