What broke

Agents review each other's pull requests and post verdicts: approve, request changes, or comment. A pull request is a proposal to merge a branch, and the branch can move. Each push moves its head to a new commit.

The review record is the server's list of verdicts on the request. A verdict on the current head is a statement about the code that would merge; a verdict on an earlier head is a statement about code that may since have changed. The merge guard reads that record and decides from the newest verdict.

Two clients got the relationship between a verdict and a commit wrong, in opposite directions.

The client that posts a verdict did not say which commit it reviewed. The server binds a review to whatever the branch head is when the request arrives, so a branch that moved between the client's read and its post left a verdict recorded against a commit nobody had named. The same code also read the server's reply without checking it had a head at all, and panicked when it did not.

The client that reads a verdict described it as a verdict on the commit under merge. The guard exists precisely for the case where that is false: a refusal given at an earlier head still blocks, which is why it is worth naming the commit rather than implying it. Three pieces of documentation carried the same misreading, one of them saying that a refusal "of that commit" does not block, which is the opposite of what the guard does.

The belief in both places was that a verdict is about the commit in front of you, when a verdict is about a commit you have to name.

How it showed

The posting defect produced no live wrong merge, and could not be detected from the record afterwards: a verdict bound to an unnamed commit looks exactly like a verdict on the current one. What it produced was a panic, once, on a reply without a head, instead of a sentence saying the server had answered without one.

The reading defect was visible in the sentence a blocked merge prints. Each message begins with a hash, the request's number and a colon. Before the change the sentence read the latest review of this commit is a refusal. After the change it reads the latest verdict on this request is a refusal. and is followed by a second line naming the refusal (its review number, the login that gave it, the date it was submitted and the commit it was given at), and then by a line saying nothing was merged.

The slots are named here rather than filled, because a filled example would have to pair one repository's review with one of that same repository's commits, and the case is about the shape of the sentence rather than about any particular refusal. The shape is not invented: the tool formats it from the string literal review %d by %s requests changes, submitted %s, at %s, quoted here without the two spaces of indent and the newline that surround it in the source. The slots are the record's fields and nothing else.

The first sentence sends a reader to look at a review of the commit they are merging. If the refusal was given at an earlier commit, that review does not exist, and the reader concludes the guard is confused rather than the branch.

What was watching

Nothing recorded which commit a verdict was posted against, which is why the first defect was invisible in the data rather than merely unnoticed. The review record holds a commit for every verdict now; before this change a verdict posted by the tool had no such claim behind it, and the field held whatever the branch pointed at.

The posting path had no test asserting the body it sent. The reading path had tests for its decision and none for its sentence, so the wording could describe a different commit than the one the guard had read.

The detecting sensor was a person reading the guard's message against the guard's logic, and finding that one of them lied about the other.

Where it sits

This is an invariants case with a provenance gap.

The family is invariants because the guard's decision depends on "this verdict is about this commit", a relationship nothing in the record established. It is comprehension because the sentence the guard printed described a different commit from the one its logic used, and three documents described the guard wrongly in the same direction.

Contract tests would have seen the posting half: an assertion that the body carries commit_id, and that a reply with no head produces a sentence rather than a crash. Decision provenance was absent and is the sharper gap: the record could not say which commit a verdict was given at, so no amount of later reading could tell whether the guard's decision was about the right code.

What we did

The posting client reads the request's head and names that commit in the body, so the server binds the verdict where the client intends. A reply without a head produces a sentence naming what was missing instead of a panic, and the test asserts the posted body's commit_id.

The reading client's refusal names the commit it was given at, and says "an unnamed commit" when the record does not carry one, rather than implying the current head. The documentation was corrected in the same change, including the sentence that had the guard's behaviour backwards.

What now fails loudly

A test asserts the commit_id in the posted body, so a change that drops it fails before the server can misbind anything. TestTheRefusalSentenceNamesTheCommitItWasGivenAt requires the refusal text to carry the commit it was given at, so the two halves of the guard cannot drift apart again.

Two paths are not guarded. A reply without a head returns a sentence and has no test of its own: the code is at pr.go:435, the assertion is not. And a server that binds a verdict to a different commit than the one named is the server's contract, measured rather than asserted here; the guard reads the field back rather than trusting its own claim.