Observed live on PR #17653, 2026-08-23, minutes before @tobiu merged it.
I reviewed and approved that PR. @neo-opus-vega authored it. We are both Claude-family, so pull-request-workflow.md §6.1 — "No PR may be merged without at least one cross-family Approved review" — was unsatisfied. Every readable signal said otherwise: state=OPEN, mergedAt=null, reviewDecision=APPROVED, gh pr checks exit 0, mergeStateStatus=CLEAN, reviewRequests=[].
Run validateMergeReady against exactly those values:
Zero blockers. Zero advisories. The one gate that actually applied produced no output at all.
The merge itself was correct — the operator holds merge authority and, per @neo-opus-vega, "overrode it knowingly." That override was informed because I wrote the §6.1 problem in prose in an A2A message and a review body. No tool told anyone. This ticket is about the tool.
The Problem
ai/scripts/lifecycle/validateMergeReady.mjs:26 states its own scope: "Validates whether a PR is STRICT merge-ready against the full review/merge contract." It then enumerates five rules that must all hold. §6.1 is not among them:
state === 'OPEN' and mergedAt === null
reviewDecision === 'APPROVED'
checksGreen === true
mergeStateStatus in CLEAN/UNSTABLE
every explicitly-requested reviewer disposed
Rule 2 is the load-bearing one and it is the wrong question. GitHub's reviewDecision answers "did someone with review rights approve?" It cannot answer "was the approval one our rules accept?", because GitHub does not model reviewer family — no API field can express this gate, so no field-mirroring validator will ever catch it.
The module's fail-closed discipline makes this sharper rather than softer. It is careful, and correctly so: an unfetched reviewRequests blocks, UNKNOWN mergeability blocks, an un-queried field "cannot certify a green surface". That rigour is applied to every field it knows about — and a gate it does not model cannot fail closed, because nothing represents it. The care is real and the coverage is narrower than the docblock claims.
Why this is not "just read the rule yourself"
Three properties make it a mechanical problem rather than a diligence one:
It fails silent and green.blockers: [] and advisories: [] are indistinguishable from "genuinely nothing to report". There is no unknown-state to notice.
Freshness discipline cannot detect it. Everything the swarm has learned about merge signals — read the gh pr checks exit code, read mergeStateStatus first, fetch live state before any verdict — is about obtaining the freshest value of a field. All of that was done correctly here. A field that is fresh and irrelevant is invisible to every one of those habits.
It scales the wrong way. The likeliest same-family pair is the pair that reviews each other most, which is the most active pair. The gate is loosest exactly where it matters most.
The Architectural Reality
ai/scripts/lifecycle/validateMergeReady.mjs:26-40 — the docblock's "full review/merge contract" claim and the five-rule list.
ai/services/github-workflow/PullRequestService.mjs:787 — predicate.strictMergeReady && sourceBlockers.length === 0, the consumer that turns the predicate into a reported state.
PullRequestService.mjs:3799 — already names "GitHub's reviewDecision surface, blocking the cross-family review mandate gate", so the tension is documented in the service and absent from the validator.
ai/mcp/server/github-workflow/openapi.yaml:1033 — manage_pr_reviewers is described as the invitation layer of §6.1, with the mandate as "the validation layer (Approved-status before merge)". The invitation layer exists; the validation layer described there is reviewDecision, which is the gap.
Family resolution has an existing authority: §6.1 resolves author family from the §5 Social Name in the PR body, with author.login as fallback. That is a parse of text the PR already carries, not new metadata.
The Fix
Corrected 2026-08-24, before implementation. My first Fix section described building family resolution into the validator. Add family coherence as a sixth rule … Inputs: the author's family and the set of approving reviewers' families, resolved per §6.1.That framing was wrong and would have duplicated working code. V-B-A before writing the first line found ai/services/graph/agentFamilyResolution.mjs, which already exports the whole mechanism: resolveAuthorFamily (PR-body self-id per §5 Social Name), resolveReviewerFamily (login, no fallback, and it returns a {classified, family, login} pair precisely so a consumer can tell "resolved to gpt" from "could not resolve"), groupReviewsByFamily, and hasCrossFamilyReview at :257.
So the real defect is sharper than "the rule is missing". The predicate exists, is unit-tested, and reaches no decision. Its only consumers are narrative:
ai/services/graph/activePrCycleSection.mjs:35 — renders cross-family reviewed: yes/no into a Golden Path report line.
ai/services/graph/GoldenPathSynthesizer.mjs:24/:820 — re-exports it for the same report.
Nothing gates on it. The capability is built, tested, and wired to a sentence.
Two defects in the predicate itself, which is why it cannot simply be called as-is:
It fails OPEN on an unresolvable author.:265 — if (!authorFamily) return true;. Any classified reviewer then satisfies the mandate. Importing that branch into validateMergeReady would contradict the module's stated fail-closed contract, where an un-queried field "cannot certify a green surface". A reporting predicate can afford optimism; a gate cannot.
It counts reviews of ANY state.:259 reads pr.reviews and filters only on reviewer family — never on state === 'APPROVED'. §6.1 requires "at least one cross-family Approved review", so a cross-family COMMENT currently satisfies it. Tonight's #17654 is the shape that hides this: Emmy submitted a CHANGES_REQUESTED, then an APPROVED. A PR where the cross-family reviewer only ever commented would read as covered.
Minor, same function: :262 is review.author?.login || review.author?.name || review.author?.login — the third term repeats the first and is dead.
The prescription:
Give agentFamilyResolution.mjs a verdict-returning strict function — {crossFamily, authorFamily, approvingFamilies, unresolved} — that filters to APPROVED and treats an unresolvable author family as unresolved, not as satisfied. One source of truth.
Keep hasCrossFamilyReview as a thin boolean wrapper over it so the Golden Path report keeps working, rather than growing a second predicate that drifts from the first — the a2aCollisionTags lesson, one directory over.
validateMergeReady consumes the verdict, fail-closed: unresolved ⇒ blocker, same as an unfetched reviewRequests.
PullRequestService.mjs:777 passes it. Its snapshot currently discards what is needed — :422-424 maps approvals to {oid, submittedAt} and drops the reviewer login, and the snapshot carries no PR author. Both must be added. Adding them is drift-safe: the snapshot is compared by stableStringify at :701, and a login is a stable property of a node whose arrival already trips that comparison.
Contract Ledger Matrix
#
Target surface
Source of authority
Before
After
Fallback
Evidence
1
validateMergeReady(pr) rule set
pull-request-workflow.md §6.1
five rules; family unmodelled
six rules; family coherence required
unresolvable family ⇒ blocker (fail-closed, as reviewRequests)
#17653's exact values return strictMergeReady: false with a named blocker
2
validateMergeReady docblock :26
the function's own scope claim
"the full review/merge contract" while omitting §6.1
accurate, or the omission named
none
docblock and rule list agree
3
PullRequestService.mjs:787 consumer
:3799's existing note on the reviewDecision surface
consumes a predicate blind to family
consumes the family-aware predicate
inherits the blocker
a same-family-only PR reports not-ready through the service path
Acceptance Criteria
validateMergeReady returns strictMergeReady: false for PR #17653's exact pre-merge values, with a blocker naming the author family, the approving families, and §6.1.
A genuinely cross-family PR with the same field values returns true — the arm that stops the fix from being "always false".
NON-VACUITY, pinned to the live incident: the arm uses #17653's real values (OPEN / null / APPROVED / true / CLEAN / []), which return strictMergeReady: true today. It must be RED before the change.
Unresolvable or unfetched family blocks, consistent with the module's fail-closed contract — with an arm proving an absent family is not read as "no conflict".
An approval from a third family (neither author's nor the first reviewer's) satisfies the rule — the mandate is cross-family, not one specific other family.
A cross-family review in a NON-approved state (COMMENT / CHANGES_REQUESTED) does not satisfy the mandate — an arm proves it, since today it does.
An unresolvable author family yields unresolved ⇒ blocker, never the current fail-open true.
hasCrossFamilyReview's existing report consumer (activePrCycleSection.mjs:35) still renders, proven by an arm — the boolean wrapper must not change shape under it.
A family recorded as unknown COUNTS as differing — operator ruling, 2026-08-24.cannot satisfy the mandate — I first shipped the opposite, on the argument that a family nobody can state cannot be shown to differ. The operator ruled permissively: a guest seat whose approvals can never unblock anything has no merge-path value, and for a Claude-family author it is the difference between two eligible cross-family seats and three. The trade is real and must be recorded at the constant, not buried: admitting unknown assumes part of what the mandate checks. The arm must assert the decision — one that merely observed 'unknown' !== 'claude' would pass under either policy — so it also pins that two seats both carrying unknown do not differ from each other.
Seat liveness is NOT consulted. The gate asks what an approval WAS, not who is available now; a benched peer's past approval was still genuinely cross-family. Operator-confirmed. This also keeps merge eligibility from being handed out by identityRoots.mjs, whose participationStatus rows are known stale (kimi seats recorded active while benched — @neo-opus-vega's census). That staleness is a data-accuracy defect with its own owner, not a gate defect.
The CANONICAL §5 author decides, not the opener. The GitHub opener can mis-resolve, so a body declaring a Claude author opened under a GPT login would certify on a same-family approval — opener drift deciding merge eligibility. The self-id wins; the opener is the fallback. The snapshot carries the derived login, never the body, so a declared-author change invalidates the drift-compared read while an unrelated prose edit does not. Arm: body Grace/claude, opener Emmy/gpt, sole approver Vega/claude → NOT ready.
A negative over a truncated approvals window is UNRESOLVED, not a negative.reviews(last: 100) is a suffix, so a qualifying older approval can sit outside it. A positive witness inside the window is decisive; finding none is missing evidence rather than evidence of absence, and must not report the factual "mandate unsatisfied". Both controls asserted, plus the query JSDoc corrected — it claimed truncation could change no decision it feeds, which stopped being true when the mandate became one of them.
The docblock at :26 no longer claims a scope wider than the rules implement.
The blocker text is legible to the operator, who is the actual consumer of a merge-readiness report and does not read pull-request-workflow.md mid-merge.
Out of Scope
The operator's merge authority. Merging past this blocker is legitimate and stays legitimate; §critical_gates #1 makes merge human-only and this ticket does not touch that.
#17653's merge. It was knowingly overridden and needs no retraction.
The hold-token mechanism in #17608. Same function, same symptom class, different data source (timeline comments vs reviewer identity). Deliberately a sibling leaf, not folded — one Resolves each.
§6.1's stacked-PR clause (approval belongs to the dev-rebased merge candidate). A second, real gap in the same function; it needs its own reading of baseRefName and should not ride this fix.
Nightly or automated merge sweeps. If one exists or is built, it inherits the repaired predicate rather than reimplementing the rule.
Avoided Traps
Filing this as "the operator merged something they shouldn't have". They merged knowingly, with authority, after I flagged it in prose. The defect is that the flag was prose from a peer who happened to notice rather than a blocker from the validator — a control that depends on a reviewer's attention is not a control.
Making it an advisory. Advisories restore silent-green in a quieter register. §6.1 is a mandate, and the module already reserves blockers for exactly this.
Treating it as a GitHub limitation and stopping there. It is true that no GitHub field can express family — which is precisely why a local validator claiming the "full review/merge contract" must model it. The limitation is the argument for the fix, not against it.
Folding it into #17608. Same function and same symptom, but the fix reads different data and the close targets would collide. Widening a close target mid-lane is the thing I push back on in review; it applies to my own tickets.
Encoding a family allowlist. The rule is difference, not membership. Hardcoding "gpt or gemini must approve" rots the moment the roster changes and would have to be re-audited on every seat addition.
Decision Record impact
none — implements an existing mandate that pull-request-workflow.md §6.1 already states. If review concludes that encoding a workflow mandate inside a lifecycle validator is itself an architectural choice, it becomes aligned-with and says which record.
Related
#17608 — sibling leaf on the same function: an approving reviewer's comment-borne hold also leaves strictMergeReady true. Same symptom class, different data source.
PR #17653 — the live incident; merged bc0e2daae8 at 2026-08-23T23:07:15Z by @tobiu
pull-request-workflow.md §6.1 — the mandate; §5 Social Name is the family authority
AGENTS.md §critical_gates #1 — merge is human-only, which this fix does not alter
Live latest-open sweep: checked latest 20 open issues at 2026-08-23T23:18:34Z; the only merge-readiness ticket is #17608, a different mechanism on the same function, cross-linked above rather than duplicated. A2A in-flight claim sweep at 23:14Z: active claims are #17658 (@neo-gpt-emmy), a downstream app ticket (@neo-gpt), #17629 (@neo-preview); none overlap.
Retrieval Hint: query_raw_memories("strictMergeReady reviewDecision APPROVED same-family cross-family mandate false green validateMergeReady"), or ai/scripts/lifecycle/validateMergeReady.mjsfull review/merge contract.
tobiu referenced in commit a136ed2 - "fix(github-workflow): merge-readiness enforces the cross-family mandate (#17661) (#17662) on Aug 24, 2026, 3:36 AM
Context
Observed live on PR #17653, 2026-08-23, minutes before @tobiu merged it.
I reviewed and approved that PR. @neo-opus-vega authored it. We are both Claude-family, so
pull-request-workflow.md§6.1 — "No PR may be merged without at least one cross-family Approved review" — was unsatisfied. Every readable signal said otherwise:state=OPEN,mergedAt=null,reviewDecision=APPROVED,gh pr checksexit0,mergeStateStatus=CLEAN,reviewRequests=[].Run
validateMergeReadyagainst exactly those values:validateMergeReady({ state: 'OPEN', mergedAt: null, reviewDecision: 'APPROVED', checksGreen: true, mergeStateStatus: 'CLEAN', reviewRequests: [], disposedReviewers: [] }) // → { strictMergeReady: true, blockers: [], advisories: [] }Zero blockers. Zero advisories. The one gate that actually applied produced no output at all.
The merge itself was correct — the operator holds merge authority and, per @neo-opus-vega, "overrode it knowingly." That override was informed because I wrote the §6.1 problem in prose in an A2A message and a review body. No tool told anyone. This ticket is about the tool.
The Problem
ai/scripts/lifecycle/validateMergeReady.mjs:26states its own scope: "Validates whether a PR is STRICT merge-ready against the full review/merge contract." It then enumerates five rules that must all hold. §6.1 is not among them:state === 'OPEN'andmergedAt === nullreviewDecision === 'APPROVED'checksGreen === truemergeStateStatusinCLEAN/UNSTABLERule 2 is the load-bearing one and it is the wrong question. GitHub's
reviewDecisionanswers "did someone with review rights approve?" It cannot answer "was the approval one our rules accept?", because GitHub does not model reviewer family — no API field can express this gate, so no field-mirroring validator will ever catch it.The module's fail-closed discipline makes this sharper rather than softer. It is careful, and correctly so: an unfetched
reviewRequestsblocks,UNKNOWNmergeability blocks, an un-queried field "cannot certify a green surface". That rigour is applied to every field it knows about — and a gate it does not model cannot fail closed, because nothing represents it. The care is real and the coverage is narrower than the docblock claims.Why this is not "just read the rule yourself"
Three properties make it a mechanical problem rather than a diligence one:
blockers: []andadvisories: []are indistinguishable from "genuinely nothing to report". There is no unknown-state to notice.gh pr checksexit code, readmergeStateStatusfirst, fetch live state before any verdict — is about obtaining the freshest value of a field. All of that was done correctly here. A field that is fresh and irrelevant is invisible to every one of those habits.The Architectural Reality
ai/scripts/lifecycle/validateMergeReady.mjs:26-40— the docblock's "full review/merge contract" claim and the five-rule list.:129—return {strictMergeReady: blockers.length === 0, blockers, advisories}.ai/services/github-workflow/PullRequestService.mjs:787—predicate.strictMergeReady && sourceBlockers.length === 0, the consumer that turns the predicate into a reported state.PullRequestService.mjs:3799— already names "GitHub'sreviewDecisionsurface, blocking the cross-family review mandate gate", so the tension is documented in the service and absent from the validator.ai/mcp/server/github-workflow/openapi.yaml:1033—manage_pr_reviewersis described as the invitation layer of §6.1, with the mandate as "the validation layer (Approved-status before merge)". The invitation layer exists; the validation layer described there isreviewDecision, which is the gap.author.loginas fallback. That is a parse of text the PR already carries, not new metadata.The Fix
Corrected 2026-08-24, before implementation. My first Fix section described building family resolution into the validator.
Add family coherence as a sixth rule … Inputs: the author's family and the set of approving reviewers' families, resolved per §6.1.That framing was wrong and would have duplicated working code. V-B-A before writing the first line foundai/services/graph/agentFamilyResolution.mjs, which already exports the whole mechanism:resolveAuthorFamily(PR-body self-id per §5 Social Name),resolveReviewerFamily(login, no fallback, and it returns a{classified, family, login}pair precisely so a consumer can tell "resolved to gpt" from "could not resolve"),groupReviewsByFamily, andhasCrossFamilyReviewat:257.So the real defect is sharper than "the rule is missing". The predicate exists, is unit-tested, and reaches no decision. Its only consumers are narrative:
ai/services/graph/activePrCycleSection.mjs:35— renderscross-family reviewed: yes/nointo a Golden Path report line.ai/services/graph/GoldenPathSynthesizer.mjs:24/:820— re-exports it for the same report.Nothing gates on it. The capability is built, tested, and wired to a sentence.
Two defects in the predicate itself, which is why it cannot simply be called as-is:
:265—if (!authorFamily) return true;. Any classified reviewer then satisfies the mandate. Importing that branch intovalidateMergeReadywould contradict the module's stated fail-closed contract, where an un-queried field "cannot certify a green surface". A reporting predicate can afford optimism; a gate cannot.:259readspr.reviewsand filters only on reviewer family — never onstate === 'APPROVED'. §6.1 requires "at least one cross-family Approved review", so a cross-familyCOMMENTcurrently satisfies it. Tonight's #17654 is the shape that hides this: Emmy submitted aCHANGES_REQUESTED, then anAPPROVED. A PR where the cross-family reviewer only ever commented would read as covered.Minor, same function:
:262isreview.author?.login || review.author?.name || review.author?.login— the third term repeats the first and is dead.The prescription:
agentFamilyResolution.mjsa verdict-returning strict function —{crossFamily, authorFamily, approvingFamilies, unresolved}— that filters toAPPROVEDand treats an unresolvable author family as unresolved, not as satisfied. One source of truth.hasCrossFamilyReviewas a thin boolean wrapper over it so the Golden Path report keeps working, rather than growing a second predicate that drifts from the first — thea2aCollisionTagslesson, one directory over.validateMergeReadyconsumes the verdict, fail-closed: unresolved ⇒ blocker, same as an unfetchedreviewRequests.PullRequestService.mjs:777passes it. Its snapshot currently discards what is needed —:422-424maps approvals to{oid, submittedAt}and drops the reviewer login, and the snapshot carries no PR author. Both must be added. Adding them is drift-safe: the snapshot is compared bystableStringifyat:701, and a login is a stable property of a node whose arrival already trips that comparison.Contract Ledger Matrix
validateMergeReady(pr)rule setpull-request-workflow.md§6.1reviewRequests)strictMergeReady: falsewith a named blockervalidateMergeReadydocblock:26PullRequestService.mjs:787consumer:3799's existing note on thereviewDecisionsurfaceAcceptance Criteria
validateMergeReadyreturnsstrictMergeReady: falsefor PR #17653's exact pre-merge values, with a blocker naming the author family, the approving families, and §6.1.true— the arm that stops the fix from being "always false".OPEN/null/APPROVED/true/CLEAN/[]), which returnstrictMergeReady: truetoday. It must be RED before the change.COMMENT/CHANGES_REQUESTED) does not satisfy the mandate — an arm proves it, since today it does.true.hasCrossFamilyReview's existing report consumer (activePrCycleSection.mjs:35) still renders, proven by an arm — the boolean wrapper must not change shape under it.unknownCOUNTS as differing — operator ruling, 2026-08-24.cannot satisfy the mandate— I first shipped the opposite, on the argument that a family nobody can state cannot be shown to differ. The operator ruled permissively: a guest seat whose approvals can never unblock anything has no merge-path value, and for a Claude-family author it is the difference between two eligible cross-family seats and three. The trade is real and must be recorded at the constant, not buried: admittingunknownassumes part of what the mandate checks. The arm must assert the decision — one that merely observed'unknown' !== 'claude'would pass under either policy — so it also pins that two seats both carryingunknowndo not differ from each other.identityRoots.mjs, whoseparticipationStatusrows are known stale (kimi seats recordedactivewhile benched — @neo-opus-vega's census). That staleness is a data-accuracy defect with its own owner, not a gate defect.reviews(last: 100)is a suffix, so a qualifying older approval can sit outside it. A positive witness inside the window is decisive; finding none is missing evidence rather than evidence of absence, and must not report the factual "mandate unsatisfied". Both controls asserted, plus the query JSDoc corrected — it claimed truncation could change no decision it feeds, which stopped being true when the mandate became one of them.:26no longer claims a scope wider than the rules implement.pull-request-workflow.mdmid-merge.Out of Scope
#17608. Same function, same symptom class, different data source (timeline comments vs reviewer identity). Deliberately a sibling leaf, not folded — oneResolveseach.baseRefNameand should not ride this fix.Avoided Traps
blockersfor exactly this.#17608. Same function and same symptom, but the fix reads different data and the close targets would collide. Widening a close target mid-lane is the thing I push back on in review; it applies to my own tickets.Decision Record impact
none— implements an existing mandate thatpull-request-workflow.md§6.1 already states. If review concludes that encoding a workflow mandate inside a lifecycle validator is itself an architectural choice, it becomesaligned-withand says which record.Related
#17608— sibling leaf on the same function: an approving reviewer's comment-borne hold also leavesstrictMergeReadytrue. Same symptom class, different data source.#17653— the live incident; mergedbc0e2daae8at 2026-08-23T23:07:15Z by @tobiupull-request-workflow.md§6.1 — the mandate; §5 Social Name is the family authorityAGENTS.md§critical_gates #1 — merge is human-only, which this fix does not alterLive latest-open sweep: checked latest 20 open issues at 2026-08-23T23:18:34Z; the only merge-readiness ticket is
#17608, a different mechanism on the same function, cross-linked above rather than duplicated. A2A in-flight claim sweep at 23:14Z: active claims are#17658(@neo-gpt-emmy), a downstream app ticket (@neo-gpt),#17629(@neo-preview); none overlap.Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84
Retrieval Hint:
query_raw_memories("strictMergeReady reviewDecision APPROVED same-family cross-family mandate false green validateMergeReady"), orai/scripts/lifecycle/validateMergeReady.mjsfull review/merge contract.