Repository navigation
fix: a missing rule file only matters once the layer is adopted - #280
Merged
Merged
Conversation
Closes #279. A root AGENTS.md with no host copies beside it made AgentsDrift demand all twelve, blocking every commit in a repository worked on with one agent. The only escape was deleting AGENTS.md, which switches off the drift check for the copies the repository does keep — so the choice was eleven byte-identical duplicates nobody reads, or no canonical instruction file. Check() already had the right rule and said why in its own comment: an AGENTS.md alone is a file many repositories carry for unrelated reasons, and twelve nag lines per gate run would be noise. AgentsDrift did not have it, and AgentsDrift is the one wired into the commit hook. The two disagreed about what a missing copy means and the blocking one was wrong. Drifted and unreadable still block whatever was adopted, which is the distinction the issue drew and the reason to fix it this way rather than with a config key: a stale rule file is another agent being told something this repository stopped believing, and a file that does not exist tells no agent anything. Missing and drifted are different failures. Two tests changed rather than being added around: - TestDriftedRuleFilesBlock asserted the old rule in its first case — twelve blocking findings for an AGENTS.md alone. It now asserts the new one, and TestMissingCopiesOnlyMatterOnceTheLayerIsAdopted covers both sides of the adoption line. - The gate's houseRuleFixture modelled "an AGENTS.md that is somebody else's", which under the new rule reports nothing — so the fixture no longer exercised the agent domain it was there to prove fires. It now carries a drifted copy, which both adopts the layer and pins the stronger case. TestDriftAndCheckAgreeOnMissingCopies fails if the two halves ever disagree again. Verified: a fixture repository with AGENTS.md and no copies passes the gate with 0 blocking; after adopting one host, the other eleven are reported and block. docs: none — no documented behaviour changed; `procoder agents` still blocks on drift, which is what docs/commands.md says
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/documentation issues to address (adoption detection can misclassify unreadable copies, and the changelog entry doesn’t follow the documented “link the shipping PR” rule).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Aligns the agent-layer drift check used by the commit hook (AgentsDrift) with the existing gate-side rule: missing host copies should only be reported once the repository has actually adopted at least one host copy, avoiding blanket blocking in single-host repositories.
Changes:
- Add “adopted layer” detection to
AgentsDriftso missing copies don’t block unless at least one host copy exists. - Update portability and gate fixtures/tests to reflect the new semantics and keep
AgentsDriftandCheck()aligned. - Add an Unreleased changelog entry describing the behavior change.
File summaries
| File | Description |
|---|---|
| internal/portability/drift.go | Gates missing-copy findings on an “adopted” check; expands rationale in comments. |
| internal/portability/drift_test.go | Updates existing drift test expectations and adds coverage to prevent regression/divergence with Check(). |
| internal/gate/scope_test.go | Adjusts the house-rule fixture to include a drifted host copy so the agent-layer domain is exercised under the new rule. |
| CHANGELOG.md | Adds an Unreleased entry describing the fix for single-host repositories. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Three findings from the review on #280. `adopted` counted only copies that stat cleanly, so one unreadable copy — a permission, a broken mount — read as a repository that never adopted the layer, suppressing every missing-copy finding. The check would go quietest exactly where something is wrong. Present, not readable, is the test: an unreadable copy is a file this repository chose to have, and it already blocks on its own account. Both halves now share adoptedLayer(), so the rule cannot diverge again — which was the whole point of the original fix. The comment ended "Missing and drifted are different failures and this is where that stopped being true", which reads as if they had stopped being different. It says what it meant: this function treated them as one until now. The changelog entry linked only the issue. The template in that file is explicit — every entry links the PR that shipped it, and the issue too when there is one — and the test is looser than the rule, so it passed. docs: none — no documented behaviour changed
Merged
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.
Closes #279.
A root
AGENTS.mdwith no host copies madeAgentsDriftdemand all twelve, blocking every commit in a single-agent repository. The only escape was deletingAGENTS.md, which also switches off the drift check for the copies the repository does keep.Check()already had the right rule, and explains it in its own comment — anAGENTS.mdalone is a file many repositories carry for unrelated reasons.AgentsDriftdid not have it, andAgentsDriftis the half wired into the commit hook. This makes them agree, which is the fix the issue suggested first.Drifted and unreadable still block whatever was adopted. That is the distinction worth keeping, and the reason to fix it here rather than with a config key: a stale rule file is another agent being told something this repository stopped believing; a file that does not exist tells no agent anything.
Two tests changed rather than being added around
TestDriftedRuleFilesBlockasserted the old rule in its first case (twelve blocking findings for anAGENTS.mdalone). It now asserts the new one.houseRuleFixturemodelled "an AGENTS.md that is somebody else's" — which under the new rule reports nothing, so the fixture stopped exercising the agent domain it existed to prove fires. It now carries a drifted copy, which both adopts the layer and pins the stronger case.TestDriftAndCheckAgreeOnMissingCopiesfails if the two halves diverge again.Verified
A fixture repository with
AGENTS.mdand no copies passes the gate with 0 blocking; after adopting one host, the other eleven are reported and block. Full suite green, gate 0 blocking.