From 7df7f16cf7eb9cf066ff84ddf3720e20d696dcf2 Mon Sep 17 00:00:00 2001 From: jsboige Date: Sun, 20 Sep 2026 18:22:56 +0200 Subject: [PATCH] fix(gate,#16957): sortir statusCheckRollup du payload surfaces_fingerprint Mesure fondateur (#16957, c.1323) : un check-run qui termine apres l'ecriture d'un dossier perimait celui-ci en 2-3 minutes (mesure sur #16907 : perimeter review guard a verdi entre 11:13:24Z et 11:14:54Z, mutant l'empreinte sans qu'aucune surface de discussion n'ait change). 151 dossiers sur 221 PRs ouvertes sont aujourd'hui perimes par ce seul mecanisme. Choix : option 1 recommandee par porteur - retirer statusCheckRollup du payload surfaces_fingerprint. CI state reste atteste par le champ separe checks: latest-wins-green du body, que ai-01 reverifie au merge (Phase 4 gate 5). Hacher en plus n'aurait rien renforce et aurait ajoute une source de peremption que personne ne controle. Modifications : - surfaces_fingerprint : drop 'checks' du payload. - surfaces_fingerprint docstring : split 'what certifies' / 'what does not certify' explicite. - Module docstring : 'check' retire de la liste des evenements qui periment le dossier. Note (#16957, c.1323) ajoutee. - --fingerprint CLI help : certifies body/comments/reviews/threads, does NOT certify check-rollup. - test_same_count_surface_mutation_invalidates_fingerprint : la mutation 'checks' retiree (carve-out). Nouveau test test_check_rollup_mutation_does_not_invalidate_fingerprint assert la converse, ancrant la regression en cas de re-ajout du rollup. _metadata_identity (race-detection intra-snapshot) inchange : la comparaison avant/apres reste utile dans un seul appel load_snapshot et n'affecte pas la persistance des dossiers. Validation : pytest scripts/tests/test_check_adjoint_prevalidation.py 37/37 passent. Issue #16957 reste ouverte sur la mesure pool (point 3 de l'acceptance, deleguee a l'adjoint po-2025 sweep massif). Co-Authored-By: Claude Haiku 4.5 (1M context) --- scripts/check_adjoint_prevalidation.py | 52 +++++++++++++++---- .../tests/test_check_adjoint_prevalidation.py | 22 +++++--- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/scripts/check_adjoint_prevalidation.py b/scripts/check_adjoint_prevalidation.py index b978854e38..5a3362ac50 100644 --- a/scripts/check_adjoint_prevalidation.py +++ b/scripts/check_adjoint_prevalidation.py @@ -36,10 +36,19 @@ [/ADJOINT PREFLIGHT] The comment count excludes the dossier comment itself. Any observable later -issue comment, review, inline-thread, PR-metadata, check, or head change -invalidates the dossier and requires a fresh one. GitHub does not expose a -stateless audit trail for an event that is later deleted or reverted; this gate -therefore certifies the current surfaces, not erased history. +issue comment, review, inline-thread, PR-metadata, or head change invalidates +the dossier and requires a fresh one. GitHub does not expose a stateless audit +trail for an event that is later deleted or reverted; this gate therefore +certifies the current surfaces, not erased history. + +Note (See #16957, fix landed c.1323): a check-run completing after the dossier +was posted no longer invalidates the dossier. ``statusCheckRollup`` was removed +from the ``surfaces_fingerprint`` payload: a guard such as the perimeter review +guard can start and finish two minutes after the dossier is written, and that +single transition ``pending -> success`` permed 151 dossiers in the open pool +(measured 2026-09-20). CI state lives in the separate ``checks: +latest-wins-green`` field of the dossier body and is cross-checked at merge +time (Phase 4 gate 5). Exit codes -- dossier INTEGRITY and PR MERGEABILITY are two questions, and conflating them is what this gate used to do (#16800): @@ -276,6 +285,26 @@ def surfaces_fingerprint( submitted after it are excluded, because the coordinator authored them; see ``_is_own_later_act``. Rendering a template passes ``None``, so a fresh dossier still attests every surface that exists when it is written. + + What this fingerprint certifies (See issue #16957 for the carve-out): + + * PR body, title, state, draft, baseRefName + * Every issue comment (id, author, createdAt, body) + * Every review (id, author, submittedAt, state, commit, body), minus the + coordinator's own later reviews, by symmetry with ``_is_own_later_act`` + * Every review thread (resolved and unresolved) + + What this fingerprint does **not** certify: + + * The CI check-rollup (``statusCheckRollup``). Check-runs mutate of their + own accord: a guard can start and finish two minutes after the dossier + was posted, and that single transition ``pending -> success`` permed 151 + dossiers in the open pool (measured 2026-09-20, c.1323). The attestation + of CI state lives in the separate ``checks: latest-wins-green`` field of + the dossier body, which ai-01 cross-checks at merge time (Phase 4 gate + 5); hashing it in addition would add a source of peremption that no one + controls, and would not strengthen any verification that is actually + performed. """ comments = snapshot.get("comments") or [] if comment_limit is not None: @@ -314,12 +343,6 @@ def surfaces_fingerprint( for row in reviews ], "threads": snapshot.get("threads") or [], - "checks": sorted( - snapshot.get("statusCheckRollup") or [], - key=lambda row: json.dumps( - row, sort_keys=True, separators=(",", ":") - ), - ), } encoded = json.dumps( payload, ensure_ascii=False, sort_keys=True, separators=(",", ":") @@ -655,7 +678,14 @@ def main() -> int: parser.add_argument( "--fingerprint", action="store_true", - help="print the live discussion fingerprint for a new dossier", + help=( + "print the live discussion fingerprint for a new dossier. " + "Certifies comments, reviews (minus coordinator's own later rows), " + "threads, and PR body/title/state/draft/baseRefName. Does NOT " + "certify the check-rollup: a check-run completing after the dossier " + "would invalidate it under the old rule and was the second plafond " + "that permed 151 dossiers in the open pool (See #16957, fix c.1323)." + ), ) parser.add_argument( "--template", diff --git a/scripts/tests/test_check_adjoint_prevalidation.py b/scripts/tests/test_check_adjoint_prevalidation.py index a70a7d2cae..b8ff283a88 100644 --- a/scripts/tests/test_check_adjoint_prevalidation.py +++ b/scripts/tests/test_check_adjoint_prevalidation.py @@ -324,12 +324,6 @@ def test_same_count_surface_mutation_invalidates_fingerprint(): "isResolved", False ), ), - ( - "checks", - lambda snapshot: snapshot["statusCheckRollup"][0].__setitem__( - "conclusion", "FAILURE" - ), - ), ): snapshot = _snapshot(_body()) mutate(snapshot) @@ -337,6 +331,22 @@ def test_same_count_surface_mutation_invalidates_fingerprint(): assert any("discussion surfaces changed" in error for error in errors), surface +def test_check_rollup_mutation_does_not_invalidate_fingerprint(): + """See issue #16957: a check-run completing after the dossier was posted + MUST NOT invalidate it. ``statusCheckRollup`` is intentionally absent from + the surfaces-fingerprint payload, and the dossier attesting CI state lives + in the separate ``checks: latest-wins-green`` field. This test pins that + carve-out so a future regression that re-adds the rollup to the payload + is caught at the test stage, not by pereming 151 dossiers in production. + """ + snapshot = _snapshot(_body()) + snapshot["statusCheckRollup"][0]["conclusion"] = "FAILURE" + snapshot["statusCheckRollup"][0]["completedAt"] = "2099-01-01T00:00:00Z" + verdict, errors = mod.evaluate(snapshot) + assert verdict == mod.VERDICT_READY, errors + assert errors == [] + + def test_latest_dossier_wins_and_stale_latest_cannot_fall_back(): snapshot = _snapshot(_body()) snapshot["comments"].append(_comment(_body(head="e" * 40, **{"comments-reviewed": "2"})))