fix(foreignregion): resolve MDS073 rule-ID collision with slide-structure - #773
Draft
jeduden wants to merge 3 commits into
Draft
fix(foreignregion): resolve MDS073 rule-ID collision with slide-structure#773jeduden wants to merge 3 commits into
jeduden wants to merge 3 commits into
Conversation
…ture The foreign-region scanner's malformed-region diagnostic and the slide-structure rule both independently claimed MDS073 when they landed in the same window. Since the scanner's diagnostic rides along the fix/check pipeline outside the rule.Rule registry, .mdsmith.yml could not reliably disable one without ambiguity over the other. Renumber the foreign-region diagnostic to MDS074 (the next free ID) and add a contract test that walks every registered rule.Rule.ID() plus foreignregion.RuleID and fails on any duplicate, so a repeat collision fails CI instead of shipping silently. Also records the architecture audit finding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEzK19viwk2UZozpzLHuf
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code review on PR #773 flagged that the audit workflow docs mandated a plan file for every blocker with no branch for "already fixed in this cycle," which is what this PR's own audit entry did. Codify the exception explicitly in the project's audit-checklist binding instead of leaving the deviation undocumented. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEzK19viwk2UZozpzLHuf
…-cycle exception Third code-review pass on PR #773 flagged that the generic skill checklist's "File a plan" language had no carve-out, contradicting the project-specific same-cycle-fix exception added in the previous commit and the "follow the steps ... exactly" instruction in the project's audit-checklist page. Thread the exception through Step 3 and Step 5 so the generic and project-specific docs agree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEzK19viwk2UZozpzLHuf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A scheduled architecture audit (
/solid-architecture audit) swept the ~90 files touched since the last checkpoint (6680ff5..2ab4b29) and found one blocker: theforeign-regions:scanner and theMDS073Slidevslide-structurerule both landed in the same window and independently claimed rule IDMDS073— one through therule.Ruleregistry, the other as an engine-emitted diagnostic that rides along the fix/check pipeline outside it. That makes.mdsmith.yml's per-ruleenabled: falseambiguous forMDS073: it reliably disables the registered rule but has no effect on the foreign-region diagnostic.MDS073toMDS074(the next free ID) ininternal/foreignregion/scan.go, and updated every comment/doc that names it (internal/config/foreignregion.go,internal/config/foreignregion_more_test.go,internal/engine/runner.go,internal/fix/fix_foreign_region_test.go,docs/reference/foreign-regions.md).internal/integration/rule_id_uniqueness_test.go, a contract test that walks every registeredrule.Rule.ID()plusforeignregion.RuleIDand fails on any duplicate, so a repeat collision fails CI instead of shipping silently. Verified red (collision) before the rename, green after.mdsmith fix .to regenerate the<?catalog?>blocks inCLAUDE.md,AGENTS.md,.github/copilot-instructions.md, anddocs/reference/index.mdthat quoteforeign-regions.md's front-matter summary.docs/development/architecture-audit.md(one blocker, fixed directly; one nice-to-have —mergedriver.go/runner.goapproaching the 1000-line split threshold, no action needed yet) and advancedaudit-from:to the currentorigin/mainSHA.No other blockers or tax findings surfaced in this sweep (no rule-to-rule imports, no reverse-layer imports, no Liskov breaks, no inverted test pyramid, no missing unit tests on a public surface). The one nice-to-have has no actionable fix yet, so no plan file was filed for it.
Test plan
go build ./...go test ./...(all green)go vet ./...go tool -modfile=tools/go.mod golangci-lint runon touched packages (0 issues)mdsmith check .(0 failures)TestRuleIDsAreUniquefails against the pre-fixMDS073collision, then passes after the renameGenerated by Claude Code