Skip to content

fix: name the rules gomgr cannot express, and refuse to adopt them - #143

Merged
allanice001 merged 1 commit into
mainfrom
fix/name-unmodeled-ruleset-rules
Sep 7, 2026
Merged

allanice001 merged 1 commit into
mainfrom
fix/name-unmodeled-ruleset-rules

Conversation

@allanice001

Copy link
Copy Markdown
Contributor

gomgr import rulesets refused a live ruleset with:

no rules enabled; a ruleset with no rules enforces nothing

That ruleset had a rule:

{"type": "copilot_code_review",
 "parameters": {"review_draft_pull_requests": true, "review_on_push": true}}

gomgr does not model copilot_code_review, so rulesToConfig dropped it and the converted spec reached the validator empty. The message is correct about the spec and badly wrong about the world — it reads as "this ruleset does nothing, delete it". Acting on it deletes a working rule.

The part that is worse than a message

Had that ruleset also carried a rule gomgr does model, it would have been adopted — minus the rule nobody could see. And since buildRuleset constructs a ruleset from configuration alone while UpdateRuleset replaces what is on GitHub, the next sync would have deleted it. Silently, from a guard rail somebody relies on.

That is the real defect. The message was the symptom that led to it.

The fix

A lossy ruleset is refused, not adopted, and the reason names the rule types:

gomgr cannot express rule type copilot_code_review,
and adopting it without would delete it on the next sync

The same check runs while planning. A ruleset that is already declared — hand-written, or adopted before this and since grown a rule in the web UI — now warns that applying the configuration would delete what gomgr cannot see. That path is how an already-adopted ruleset silently loses a rule, and it is not reachable through the importer at all.

Why reflection

Detection walks github.RepositoryRulesetRules rather than checking rule types one by one, because the point is noticing rules nobody thought about — and a hand-written check can only find the ones somebody already listed.

TestModeledRuleFieldsCoverGoGitHub fails when go-github grows a field that is in neither the modeled set nor the knowingly-refused list, so a dependency bump cannot quietly widen the gap. Today gomgr models 19 rule types and knowingly refuses eight: copilot_code_review, max_file_size, max_file_path_length, and the five repository-target rules.

Verification

Build, gofmt, go vet, golangci-lint (0 issues), full suite — clean. Six new tests covering the detector, the mixed modeled/unmodeled case (where adoption previously succeeded lossily), and that the reason names the rule rather than calling the ruleset empty.

Dry-run against seven live organizations: all plan no changes with zero warnings, so nothing regressed for rulesets gomgr does model.

Patch-level — no config schema change.

🤖 Generated with Claude Code

`gomgr import rulesets` refused a live ruleset with:

    no rules enabled; a ruleset with no rules enforces nothing

That ruleset had a rule. Its only rule was copilot_code_review, which gomgr
does not model, so rulesToConfig dropped it and the converted spec reached
the validator empty. The validator's message is correct about the spec and
badly wrong about the world, and it reads as "this ruleset does nothing,
delete it". Acting on that deletes a working rule.

The deeper problem is what would have happened had the ruleset also carried
a rule gomgr does model. It would have been adopted, minus the rule nobody
could see — and buildRuleset constructs a ruleset from configuration alone
while UpdateRuleset replaces what is on GitHub, so the next sync would have
deleted it. Silently, from a guard rail somebody relies on.

So a lossy ruleset is now refused rather than adopted, and the reason names
the rule types:

    gomgr cannot express rule type copilot_code_review, and adopting it
    without would delete it on the next sync

The same check runs while planning. A ruleset that is already declared —
hand-written, or adopted before this and since grown a rule in the web
interface — warns that applying the configuration would delete what gomgr
cannot see.

Detection is reflection over github.RepositoryRulesetRules rather than a
check per rule, because the whole point is noticing rules nobody thought
about. TestModeledRuleFieldsCoverGoGitHub fails when go-github grows a field
that is in neither the modeled set nor the knowingly-refused list, so a
dependency bump cannot quietly widen the gap. Today that list is
copilot_code_review, max_file_size, max_file_path_length and the five
repository-target rules.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: allanice001 <allanice001@gmail.com>
@allanice001
allanice001 force-pushed the fix/name-unmodeled-ruleset-rules branch from 233c540 to f544100 Compare August 30, 2026 22:30
@allanice001
allanice001 merged commit b2feeb2 into main Sep 7, 2026
8 checks passed
@allanice001
allanice001 deleted the fix/name-unmodeled-ruleset-rules branch September 7, 2026 05:41
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.

1 participant