Skip to content

Commit 83dbd61

Browse files
jsboigeclaude
andauthored
fix(coord,#16957): gate re-verifies latest-wins checks instead of hashing them (#16967)
surfaces_fingerprint no longer embeds statusCheckRollup: a concluding check (even green) expired dossiers nobody wrote to (7/54 at exact head, 4/4 on one cycle lot killed by perimeter review guard). The gate now recomputes latest-wins verdicts from commits/<sha>/check-runs at evaluation time and refuses a READY dossier whose claim 'checks: latest-wins-green' is contradicted, naming the failing check. Dual acceptance keeps legacy stamps valid while their check state is unchanged; raced legacy stamps are one mechanical --template re-stamp away (a SHA-256 over changed data cannot be re-derived). 40/40 tests, 11 new, incl. two negative controls. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 545d9ec commit 83dbd61

2 files changed

Lines changed: 395 additions & 27 deletions

File tree

‎scripts/check_adjoint_prevalidation.py‎

Lines changed: 189 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,19 @@
3636
[/ADJOINT PREFLIGHT]
3737
3838
The comment count excludes the dossier comment itself. Any observable later
39-
issue comment, review, inline-thread, PR-metadata, check, or head change
40-
invalidates the dossier and requires a fresh one. GitHub does not expose a
41-
stateless audit trail for an event that is later deleted or reverted; this gate
42-
therefore certifies the current surfaces, not erased history.
39+
issue comment, review, inline-thread, PR-metadata, or head change invalidates
40+
the dossier and requires a fresh one. Check-runs are NOT a hashed surface
41+
(#16957): a check that concludes -- even in success -- must not expire a
42+
dossier, because anything that triggers a workflow (a review, a comment, a
43+
sweep) re-opens that race and the dossier writer can never win it. Instead the
44+
gate recomputes the latest-wins check verdicts at evaluation time and refuses
45+
the dossier when they contradict its `checks:` claim, naming the failing
46+
check. Dossiers stamped before #16957 embedded the check state in
47+
surfaces-sha256; their stamps remain accepted while that state is
48+
byte-identical, and need one mechanical re-stamp (--template) once a check
49+
moves. GitHub does not expose a stateless audit trail for an event that is
50+
later deleted or reverted; this gate therefore certifies the current
51+
surfaces, not erased history.
4352
4453
Exit codes -- dossier INTEGRITY and PR MERGEABILITY are two questions, and
4554
conflating them is what this gate used to do (#16800):
@@ -118,6 +127,10 @@
118127
EXIT_NO_DOSSIER = 1
119128
EXIT_UNKNOWN = 2
120129
EXIT_BLOCKED_WITH_SUBSTANCE = 3
130+
# Conclusions that do not refute `checks: latest-wins-green`. `skipped` and
131+
# `neutral` are not failures; anything else completed (failure, timed_out,
132+
# cancelled, action_required, startup_failure, stale...) does (#16957).
133+
GREEN_CONCLUSIONS = {"success", "skipped", "neutral"}
121134
START = "[ADJOINT PREFLIGHT]"
122135
END = "[/ADJOINT PREFLIGHT]"
123136
SHA_RE = re.compile(r"[0-9a-f]{40}")
@@ -265,26 +278,20 @@ def _integer(fields: dict[str, str], key: str, errors: list[str]) -> int | None:
265278
return int(value)
266279

267280

268-
def surfaces_fingerprint(
281+
def _fingerprint_payload(
269282
snapshot: dict[str, Any],
270-
comment_limit: int | None = None,
271-
neutral_after: str | None = None,
272-
) -> str:
273-
"""Hash stable content from every discussion surface plus the PR body.
274-
275-
``neutral_after`` is the dossier's own timestamp. Reviews the coordinator
276-
submitted after it are excluded, because the coordinator authored them; see
277-
``_is_own_later_act``. Rendering a template passes ``None``, so a fresh
278-
dossier still attests every surface that exists when it is written.
279-
"""
283+
comment_limit: int | None,
284+
neutral_after: str | None,
285+
include_checks: bool,
286+
) -> dict[str, Any]:
280287
comments = snapshot.get("comments") or []
281288
if comment_limit is not None:
282289
comments = comments[:comment_limit]
283290
reviews = _attested_reviews(snapshot, neutral_after)
284291

285292
author = _login
286293

287-
payload = {
294+
payload: dict[str, Any] = {
288295
"pr": {
289296
"number": snapshot.get("number"),
290297
"state": snapshot.get("state"),
@@ -314,19 +321,123 @@ def surfaces_fingerprint(
314321
for row in reviews
315322
],
316323
"threads": snapshot.get("threads") or [],
317-
"checks": sorted(
324+
}
325+
if include_checks:
326+
payload["checks"] = sorted(
318327
snapshot.get("statusCheckRollup") or [],
319328
key=lambda row: json.dumps(
320329
row, sort_keys=True, separators=(",", ":")
321330
),
322-
),
323-
}
331+
)
332+
return payload
333+
334+
335+
def _digest(payload: dict[str, Any]) -> str:
324336
encoded = json.dumps(
325337
payload, ensure_ascii=False, sort_keys=True, separators=(",", ":")
326338
).encode("utf-8")
327339
return hashlib.sha256(encoded).hexdigest()
328340

329341

342+
def surfaces_fingerprint(
343+
snapshot: dict[str, Any],
344+
comment_limit: int | None = None,
345+
neutral_after: str | None = None,
346+
) -> str:
347+
"""Hash stable content from every DISCUSSION surface plus the PR body.
348+
349+
Certifies: PR number/state/title/draft/base/body, issue comments,
350+
reviews (minus the coordinator's own later ones), review threads -- as
351+
read when the fingerprint is taken. Does NOT certify check-runs (#16957):
352+
a concluding check must not expire a stamp it contradicts nothing in; the
353+
gate re-verifies the live latest-wins conclusions against the dossier's
354+
``checks:`` claim at evaluation time instead.
355+
356+
``neutral_after`` is the dossier's own timestamp. Reviews the coordinator
357+
submitted after it are excluded, because the coordinator authored them; see
358+
``_is_own_later_act``. Rendering a template passes ``None``, so a fresh
359+
dossier still attests every surface that exists when it is written.
360+
"""
361+
return _digest(
362+
_fingerprint_payload(snapshot, comment_limit, neutral_after, False)
363+
)
364+
365+
366+
def legacy_surfaces_fingerprint(
367+
snapshot: dict[str, Any],
368+
comment_limit: int | None = None,
369+
neutral_after: str | None = None,
370+
) -> str:
371+
"""Pre-#16957 stamp algorithm: the same payload PLUS the check rollup.
372+
373+
Kept so dossiers stamped before #16957 -- whose hash embedded the check
374+
state -- remain verifiable for as long as that state is byte-identical.
375+
Once any check concludes, a legacy stamp stops matching; recovery is one
376+
mechanical re-stamp (--template recomputes every mechanical field, no
377+
re-reading of surfaces), after which no check conclusion can ever expire
378+
the dossier again. A SHA-256 over data that has since changed cannot be
379+
re-derived, which is why zero-touch recovery of raced legacy stamps is
380+
not offered. Drop this function when no open dossier carries a legacy
381+
stamp.
382+
"""
383+
return _digest(
384+
_fingerprint_payload(snapshot, comment_limit, neutral_after, True)
385+
)
386+
387+
388+
def latest_wins_check_runs(check_runs: list[dict[str, Any]] | None) -> dict[str, dict[str, Any]]:
389+
"""Last COMPLETED verdict per check name: group by name, latest ``started_at``
390+
(``id`` as tiebreak), keep that run.
391+
392+
Runs still in flight have no verdict yet and are skipped; the name falls
393+
back to its latest completed run, which is the last word actually said.
394+
Grouping by name -- not reading the rollup twin -- is what avoids painting
395+
a head red with the cancelled run of a superseded pair (#16957).
396+
"""
397+
verdicts: dict[str, tuple[tuple, dict[str, Any]]] = {}
398+
for run in check_runs or []:
399+
if (run.get("status") or "").lower() != "completed":
400+
continue
401+
key = run.get("name") or ""
402+
rank = (run.get("started_at") or "", run.get("id") or 0)
403+
current = verdicts.get(key)
404+
if current is None or rank > current[0]:
405+
verdicts[key] = (rank, run)
406+
return {name: run for name, (rank, run) in verdicts.items()}
407+
408+
409+
def check_claim_contradictions(
410+
claim: str, check_runs: list[dict[str, Any]] | None
411+
) -> list[str]:
412+
"""Re-verify a dossier's ``checks:`` claim against the live latest-wins state.
413+
414+
Hashing check-runs certified their state at stamp time but never that the
415+
claim matched it (#16957); a check concluding after the dossier expired the
416+
stamp instead of being checked. The claim is now compared to what the head
417+
actually carries: every latest-wins conclusion must be green, else the
418+
check is named -- with the run's ``output.title`` when present (the PR-gate
419+
class DWELL/FAIL lives there, as information for the reader, not in the
420+
predicate).
421+
"""
422+
if claim != "latest-wins-green":
423+
return []
424+
contradictions = []
425+
for name, run in sorted(latest_wins_check_runs(check_runs).items()):
426+
conclusion = (run.get("conclusion") or "").lower()
427+
if conclusion in GREEN_CONCLUSIONS:
428+
continue
429+
title = ((run.get("output") or {}).get("title") or "").strip()
430+
detail = f"'{name}' ({conclusion}"
431+
if title:
432+
detail += f"; {title}"
433+
detail += ")"
434+
contradictions.append(
435+
"checks claim 'latest-wins-green' is contradicted by live check "
436+
+ detail
437+
)
438+
return contradictions
439+
440+
330441
def carrying_lane(snapshot: dict[str, Any]) -> str | None:
331442
"""Return the lane that carries this pull request, from its `Grain:` tag.
332443
@@ -376,6 +487,14 @@ def validate_dossier(dossier: Dossier, snapshot: dict[str, Any]) -> list[str]:
376487
errors.append(f"{key} must be {value!r} when verdict is READY")
377488
if f.get("domain") not in {"pass", "not-applicable"}:
378489
errors.append("domain must be 'pass' or 'not-applicable' when verdict is READY")
490+
# The claim is not taken on faith: it is checked against the live
491+
# latest-wins verdicts, naming any contradicting check (#16957).
492+
errors.extend(
493+
check_claim_contradictions(
494+
f.get("checks", ""), snapshot.get("checkRuns")
495+
)
496+
)
497+
379498
dossier_lane = f.get("lane", "")
380499
if dossier_lane not in QUALIFYING_LANES:
381500
errors.append(
@@ -400,13 +519,21 @@ def validate_dossier(dossier: Dossier, snapshot: dict[str, Any]) -> list[str]:
400519
errors.append("head must be a full lowercase 40-character SHA")
401520
if not re.fullmatch(r"[0-9a-f]{64}", f.get("surfaces-sha256", "")):
402521
errors.append("surfaces-sha256 must be a lowercase SHA-256")
522+
# Dual acceptance (#16957): a stamp matches the post-fix fingerprint
523+
# (discussion surfaces only) or the legacy one (which also embedded the
524+
# check rollup). Both certify every discussion surface; the legacy digest
525+
# is strictly more fields, so accepting either weakens nothing.
403526
live_fingerprint = surfaces_fingerprint(
404527
snapshot, dossier.comment_index, dossier.created_at
405528
)
406-
if f.get("surfaces-sha256") != live_fingerprint:
529+
legacy_fingerprint = legacy_surfaces_fingerprint(
530+
snapshot, dossier.comment_index, dossier.created_at
531+
)
532+
if f.get("surfaces-sha256") not in {live_fingerprint, legacy_fingerprint}:
407533
errors.append(
408534
"discussion surfaces changed or were not fully attested: "
409-
f"dossier={f.get('surfaces-sha256', '?')}, live={live_fingerprint}"
535+
f"dossier={f.get('surfaces-sha256', '?')}, live={live_fingerprint} "
536+
"(legacy stamps whose checks moved need one --template re-stamp)"
410537
)
411538

412539
comparisons = {
@@ -568,6 +695,29 @@ def _reviews(pr: int) -> list[dict[str, Any]]:
568695
]
569696

570697

698+
def _head_check_runs(head_sha: str) -> list[dict[str, Any]]:
699+
"""Check-runs of the exact head commit, paginated (#16957).
700+
701+
Read from the commit rather than the PR rollup because the rollup surfaces
702+
the cancelled twin when two runs share a SHA; latest-wins per name is
703+
computed downstream, never on the raw list.
704+
"""
705+
runs: list[dict[str, Any]] = []
706+
page = 1
707+
while True:
708+
data = gh_json([
709+
"api",
710+
f"repos/{REPO}/commits/{head_sha}/check-runs?per_page=100&page={page}",
711+
])
712+
batch = data.get("check_runs") if isinstance(data, dict) else None
713+
if batch is None:
714+
raise RuntimeError("check-runs response has no 'check_runs' array")
715+
runs.extend(batch)
716+
if len(batch) < 100:
717+
return runs
718+
page += 1
719+
720+
571721
def _pr_metadata(pr: int) -> dict[str, Any]:
572722
fields = (
573723
"number,title,body,state,isDraft,baseRefName,headRefOid,updatedAt,"
@@ -598,6 +748,11 @@ def load_snapshot(pr: int) -> dict[str, Any]:
598748
snapshot["comments"] = _issue_comments(pr)
599749
snapshot["reviews"] = _reviews(pr)
600750
snapshot["threads"] = review_threads(pr)
751+
# Fetched inside the before/after bracket: a check concluding during the
752+
# read bumps updatedAt and aborts the snapshot (transient UNKNOWN, the
753+
# caller retries), so the claim verification below never reads a state
754+
# that was already stale when captured.
755+
snapshot["checkRuns"] = _head_check_runs(snapshot["headRefOid"])
601756
after = _pr_metadata(pr)
602757
if _metadata_identity(before) != _metadata_identity(after):
603758
raise RuntimeError("pull request changed while prevalidation snapshot was read")
@@ -670,6 +825,19 @@ def main() -> int:
670825
return 0
671826
if args.fingerprint:
672827
print(surfaces_fingerprint(snapshot))
828+
print(
829+
"certifies: PR body/title/state/base + issue comments + reviews "
830+
"+ review threads, as read just now (#16957)",
831+
file=sys.stderr,
832+
)
833+
print(
834+
"does NOT certify: check-runs. The gate re-verifies the live "
835+
"latest-wins check conclusions against the dossier's "
836+
"'checks:' claim at evaluation time and names any "
837+
"contradicting check. Pre-#16957 stamps that embedded checks "
838+
"stay acceptable only while that state is unchanged.",
839+
file=sys.stderr,
840+
)
673841
return 0
674842
verdict, errors = evaluate(snapshot)
675843
except (

0 commit comments

Comments
 (0)