Skip to content

State the check mode's real coverage, which is wider than the payloads - #543

Merged
ptr727 merged 1 commit into
developfrom
configure-header-coverage
Aug 3, 2026
Merged

State the check mode's real coverage, which is wider than the payloads#543
ptr727 merged 1 commit into
developfrom
configure-header-coverage

Conversation

@ptr727

@ptr727 ptr727 commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Fixes the finding on the promotion #542. A promotion's head is develop, which is protected, so the fix lands here and #542 picks it up when this merges.

The finding

The header comment says check mode verifies "exactly what the payloads declare" and that anything absent from settings.json is unaudited, but the script also asserts derived/dynamic settings (has_discussions, default_branch) and security toggles in addition to payload-driven rulesets/static settings.

Correct on the facts. check_settings asserts has_discussions (computed from private) and default_branch (when main exists), and check_security asserts the two Dependabot features. None of those comes from a payload.

Why it happened, which is the part worth recording

This is an over-correction of the defect #540 fixed, not a new one.

The header used to over-promise: "every applied ruleset, setting, and security feature must match", while the code compared a fraction. #540 rewrote it to "exactly what the payloads declare", which fixed the over-claim by introducing an under-claim.

A coverage note that is wrong in either direction misleads the person reading it to decide what still needs checking by hand. Over-claiming tells them to skip something unverified. Under-claiming tells them to re-check something already covered, and erodes trust in the rest of the note. Landing on the true statement took two passes and a reviewer, which is the honest record.

The change

The header now names the three groups check actually asserts, says which one is payload-driven, and states the single genuine gap rather than implying a larger one:

  • rulesets and static settings, driven by the committed payloads
  • the derived settings apply computes, asserted by name
  • the two Dependabot security features, asserted the same way
  • unaudited: a static setting absent from settings.json, because only that group is payload-driven

Verification

bash -n clean, prose_lint clean, and a live check against this repo still passes with all three groups asserted.

The header claimed check verifies exactly what the payloads declare and that
anything not in settings.json is unaudited. Both halves understate it: the
script also asserts has_discussions and default_branch, which apply computes
rather than reads from a file, and the two Dependabot security features, which
apply enables and no payload declares.

This is an over-correction of the previous defect rather than a new one. The
header used to over-promise, claiming every applied ruleset and setting must
match while the code compared a fraction. Rewriting it to say "exactly what the
payloads declare" fixed the over-claim by introducing an under-claim, and a
coverage note that is wrong in either direction misleads whoever reads it to
decide what still needs checking by hand.

It now names the three groups check actually asserts and says which one is
payload-driven, so the single genuine gap, a static setting absent from
settings.json, is stated rather than implied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 21:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo-config/configure.sh header comment so check mode’s documented coverage matches what the script actually validates (payload-driven rulesets/static settings, derived settings, and Dependabot security toggles), and clearly states the single remaining gap.

Changes:

  • Reword check mode header to describe the three asserted groups (payload-driven + derived + security toggles).
  • Clarify which assertions are payload-driven vs asserted-by-name, and narrow the “unaudited” statement to static settings absent from settings.json.

@ptr727
ptr727 merged commit 99e3c96 into develop Aug 3, 2026
7 checks passed
@ptr727
ptr727 deleted the configure-header-coverage branch August 3, 2026 21:25
ptr727 added a commit that referenced this pull request Aug 3, 2026
The report cited #536, #540 and #543 through `issue-*` link labels while
their definitions pointed at `/pull/` URLs, so the label contradicted
what it resolved to. #456 is a real issue and keeps its label. The
External group is alphabetized, which the ordering had drifted from once
`pr-545` was appended.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants