Repository navigation
fix(policy): let a pull request register its own exceptions after a success oracle verdict - #3019
Conversation
…uccess oracle verdict CI reads comment-policy exceptions and pinned scope notices from the base commit, so a change that edits a fixture and its registry entry together could never pass: the new entry did not count until it was merged first, and the change could not merge without it. The comment-policy job now counts exceptions and scope-notice pins added by the pull request itself, but only when the oracle reviewer has posted a success verdict for the exact head commit and the completed oracle-review check run came from the oracle-review workflow on that commit. A check that merely passed does not qualify, because it also passes for changes that touch no protected path. Candidate scope notices must name paths already pinned at the base, carry digests that match the head bytes, and stay inside the byte range pinned at the base, so a pin cannot widen into a whole-file exclusion. Any API error, missing token, or malformed response leaves the base policy in force. The job is granted read access to checks and actions so it can look the verdict up.
![benchbox-oracle[bot]](/s/com/githubusercontent/avatars/G.https/u/319264189?s=60&v=4){kind=small}
There was a problem hiding this comment.
oracle-verdict: failure for 3ed3eb19b9e72b24b87239ff3b3cad61fdd2d4e9
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (366 code lines exceed 150; touches 3 areas).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
Same-PR exception/scope self-registration gated only on an oracle head verdict is bypassable: cover is not bound to the scanned checkout, and candidate pins check ranges but not base-byte content, making retained verification tautological.
Reviewers not available
- sol: absent (error)
- luna: absent (error)
… against the base The review-gated registration trusted the pull request event alone. A stale event, or a checkout that is not the reviewed head, could authorize self-registered exceptions on an unreviewed tree. Oracle cover now requires a clean checkout that is the reviewed head, or a merge commit whose second parent is the reviewed head and whose tree is exactly the merge of its parents. Candidate scope notices also had to stay inside the pinned byte range but nothing compared their bytes to the base, and the retained and blob digests were recomputed from the head, so a rewritten notice passed its own pin. The candidate range must now hold bytes identical to the base, and the digests the checker enforces come from the base blob.
There was a problem hiding this comment.
oracle-verdict: success for 153d6a916c8ae990c9c72d8f21d3b0a0b516210a
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (459 code lines exceed 150; touches 3 areas).
Decision: SHIP.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
Prior D1/D2/D3 are fixed: oracle cover is now bound to the scanned checkout, reviewed head, successful bound workflow run and latest success verdict; candidate ranges require containment plus base-byte and base-blob continuity; candidate pins are validated against head then re-anchored to base digests and re-verified. No new merge-blocking defects found in the changed scope/policy gates; remaining edge cases fail closed.
Verified fixed: D1, D2, D3. Resolve their review threads.
Scope: files changed since head 3ed3eb19b9e72b24b87239ff3b3cad61fdd2d4e9 was reviewed.
There was a problem hiding this comment.
oracle-verdict: success for ce72e57e7267f6034a5210b7574a53fa51536701
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (459 code lines exceed 150; touches 3 areas).
Decision: SHIP, carried from head 153d6a916c8ae990c9c72d8f21d3b0a0b516210a.
The patch is unchanged since head 153d6a916c8ae990c9c72d8f21d3b0a0b516210a was reviewed, so its decision is carried to this head without running a reviewer. Its open review threads still apply.
Summary
CI reads comment-policy exceptions and pinned scope notices from the base commit. A change that edits a fixture and its registry entry together therefore could not pass: the new entry only counted after it was merged, and the change could not merge without it.
This lets a pull request register its own exceptions and notice pins, but only after the oracle reviewer has posted a success verdict for the exact head commit.
oracle_review_coversrequires a completedoracle-reviewcheck run from the GitHub Actions app, from theoracle-reviewworkflow, on the head commit, and a success verdict review frombenchbox-oraclefor that same head. A check that merely passed is not enough, because it also passes for changes that touch no protected path. The latest verdict wins.comment-policyjob receivesGITHUB_TOKENwithchecks: readandactions: readso it can look the verdict up.docs/internal/comment-policy.mddescribes the procedure.Fixture digest scoping, which an earlier version of this change also carried, is not part of this change.
Verification
tests/unit/scripts/test_comment_policy.pyandtests/unit/scripts/test_comment_cleanup_scope.pypass (960 tests), including the verdict lookup against fake API responses: foreign app, wrong workflow, other head, failure verdict, non-oracle author, latest verdict, and API errors.make comment-policy-checkandmake pr-preflightpass.