refactor(arch): extract buildpathutil to break build→rules DIP violation - #616
Merged
Conversation
internal/build/builder.go and staleness.go imported internal/rules/build for three path helpers, violating the architecture rule that only packages inside internal/rules/ may import a specific rule package. - New internal/rules/buildpathutil exports CheckGlobMatchCap, ResolvePathInRoot, and UnderMdsmithDir; tests migrated from resolve_test.go - internal/rules/build now imports buildpathutil for UnderMdsmithDir - internal/build imports buildpathutil instead of internal/rules/build - TestBuildDoesNotImportRulePackages contract test guards the new boundary - buildpathutil added to allowedRuleHelpers in TestRulesDoNotImportEachOther - Plan files for two remaining tax items (engine wrappers, secreview tests) - Architecture audit log entry for 2026-06-14 Closes plan/2606141910. https://claude.ai/code/session_01PjGC2k64upbXWhKHGQXDTg
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:
|
…lete empty resolve.go Code review round 1 findings: - staleness.go resolveOutputs lacked the .mdsmith/ output guard that builder.go resolveOutputs enforces; CheckStaleness/RecordBuild would accept a target whose outputs are under .mdsmith/ while Build would refuse it - internal/rules/build/resolve.go contained only a package declaration after the functions were moved to buildpathutil; delete the empty file https://claude.ai/code/session_01PjGC2k64upbXWhKHGQXDTg
Code review round 2: the UnderMdsmithDir guard added to staleness.resolveOutputs had no test exercising it. TestStaleness_OutputUnderMdsmithRefused verifies that CheckStaleness returns an error when Target.Outputs contains a .mdsmith/-prefixed path. https://claude.ai/code/session_01PjGC2k64upbXWhKHGQXDTg
…nputs Code review round 3: builder.resolveInputs silently accepted a glob that matched no files (only the cap check ran), while staleness.resolveInputs correctly rejected it. A misconfigured inputs: glob would let Build proceed with an empty input set and record a cache entry that cannot be reproduced by CheckStaleness. Add the same guard staleness.go has (len(matches) == 0 → error) and cover it with TestBuild_ZeroMatchGlobIsError. https://claude.ai/code/session_01PjGC2k64upbXWhKHGQXDTg
jeduden
added
queue
Owner
Author
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
Owner
Author
|
🔵 Merge Queue — CI running Merged into batch branch Next: No action needed — you'll be notified when CI completes. |
jeduden
removed
the
queue:active
Owner
Author
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
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
2 participants
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
internal/buildimportedinternal/rules/buildfor three path helpers (CheckGlobMatchCap,ResolvePathInRoot,UnderMdsmithDir), violating the architecture rule that only packages insideinternal/rules/may import a specific rule packageinternal/rules/buildpathutilpackage extracts those helpers; bothinternal/rules/buildandinternal/buildnow depend on the shared helper instead of each otherTestBuildDoesNotImportRulePackagesguards thatinternal/buildnever again imports a rule package other thanbuildpathutilbuildpathutiladded to theallowedRuleHelpersallowlist inTestRulesDoNotImportEachOtherTest plan
go test ./internal/rules/buildpathutil/...— unit tests for extracted helpersgo test ./internal/build/...— builder/staleness tests still passgo test ./internal/integration/...—TestRulesDoNotImportEachOtherandTestBuildDoesNotImportRulePackagesboth passgo test ./...— full suite green (pre-existinginternal/releasePGO failures excluded)go run ./cmd/mdsmith check .— all Markdown passeshttps://claude.ai/code/session_01PjGC2k64upbXWhKHGQXDTg
Generated by Claude Code