fix(compile): avoid duplicate global Claude instructions - #2793
fix(compile): avoid duplicate global Claude instructions#2793Josh Smith (cmyui) wants to merge 4 commits into
Conversation
|
Josh Smith (@cmyui) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are consistently routed through shared owners, include explicit safety gates for cleanup, and are backed by targeted unit/integration tests plus architecture guard enforcement.
Pull request overview
This PR updates global user-scope compilation to avoid duplicating Claude instructions when equivalent content is already present in Claude’s native user rules, while preserving fallback output for uncovered/mismatched/unsafe cases and adding a conservative, opt-in cleanup path for redundant generated roots.
Changes:
- Introduces shared “native-rule coverage” logic that compares compiled instruction output against the install-rendered native rule content (including link rewriting/frontmatter effects) and uses it to filter Claude’s global compiled root.
- Adds an explicit
--cleanbehavior forapm compile -gto remove only a verified, unchanged, fully redundantCLAUDE.md(with dry-run preview), without affecting other targets’ outputs. - Registers and enforces the new shared ownership via the architecture linter guard, plus unit/integration coverage and docs/guide updates.
File summaries
| File | Description |
|---|---|
src/apm_cli/compilation/instruction_dedup.py |
New shared dedup/coverage owner that checks installed native rules using the canonical install renderer. |
src/apm_cli/compilation/user_root_context.py |
Filters Claude global compiled output based on native-rule equivalence and implements conservative redundant-root cleanup (clean/dry_run). |
src/apm_cli/compilation/agents_compiler.py |
Routes prior project-mode rule detection/expected-filename logic through the new shared owner (removes local duplicates). |
src/apm_cli/integration/instruction_integrator.py |
Extracts canonical native rule filename computation for reuse by both install and compile/dedup paths. |
src/apm_cli/integration/cleanup.py |
Allows trusted callers to pass an explicit allowed-prefix override through to the existing path validator (without weakening other gates). |
src/apm_cli/commands/compile/cli.py |
Wires global --clean into user-root compilation, improves per-target reporting, and updates summary counts/messages. |
scripts/architecture_linter/checks/registry_owner_guards.py |
Adds an architecture guard ensuring dedup logic stays delegated to the shared owners and preventing local re-implementations. |
.apm/architecture/owners/core-runtime.json |
Registers the “native instruction coverage and deployed rule filenames” decision ownership + guard. |
tests/unit/compilation/test_instruction_dedup.py |
Unit/component coverage for equivalence checks (links, scoped rules, collisions, unreadable/unsafe paths). |
tests/unit/compilation/test_global_claude_dedup.py |
Unit coverage for global Claude filtering and conservative cleanup outcomes (dry-run, symlinks, edited/older/unreadable roots, failures). |
tests/integration/test_global_claude_dedup.py |
End-to-end CLI scenarios covering install→compile dedup and explicit cleanup transitions in isolated homes/config roots. |
tests/unit/integration/test_cleanup_helper.py |
Proves the explicit-prefix override does not bypass containment/provenance/directory guards. |
tests/integration/test_architecture_owner_rule_mutations.py |
Mutation test ensuring the new guard detects bypassing the canonical install renderer. |
tests/unit/scripts/test_architecture_runner.py |
Updates the frozen architecture-rule inventory to include the new guard. |
docs/src/content/docs/reference/cli/compile.md |
Documents global Claude native-rule coverage behavior and the conservative --clean boundary. |
packages/apm-guide/.apm/skills/apm-usage/commands.md |
Updates shipped usage guidance to reflect global Claude dedup + safe cleanup workflow. |
CHANGELOG.md |
Adds an Unreleased/Fixed entry for the user-visible global compile dedup + conservative cleanup behavior. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Fixes microsoft#2792. Compare installed native rules through the existing renderer, preserve uncovered instructions, and allow explicit cleanup of an unchanged redundant global Claude root. apm-spec-waiver: Critical-path integration edits only extract existing rule naming and forward the existing path-validator override; current callers and registered deployment contracts are unchanged. Observable behavior changes are confined to the user-scope compiler.
Run the compiled-output security gate when native Claude coverage suppresses a root write, keep --force-instructions as the global dedupe override, and surface native-rule verification warnings through CommandLogger. Addresses panel follow-ups from supply-chain, DevX, and CLI logging review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f722aeb to
34ecebf
Compare
|
Quick update: my employer’s legal team is reviewing the CLA and the authorization needed for me to sign it. I’ll follow up once that review is complete. Thanks for your patience! |
fix(compile): avoid duplicate global Claude instructions
TL;DR
Global compilation now omits instructions already delivered by equivalent Claude user rules, while retaining uncovered instructions and other targets' output.
apm compile -g --cleancan remove an unchanged, fully redundant Claude root; ordinary compilation leaves it in place and explains the cleanup command.Fixes #2792. Follows the project-mode work in #1138 / #1146 and explicit cleanup in #1729 / #1747.
Important
Cleanup is intentionally conservative: a generated marker alone is insufficient. Older, edited, partially compiled, or symlinked roots are retained when they cannot be verified against the current complete instruction set.
Problem (WHY)
install -gfollowed bycompile -gplaces the same unconditional instruction in both Claude's native rules andCLAUDE.md. The regression test failed onmainatc6fde641for both default and externalCLAUDE_CONFIG_DIRpaths.--clean. Suppressing new output alone would leave existing duplicates active.The context-budget concern is the same one described in Agent Skills: “Every token in your skill competes for the agent’s attention with everything else in that window.” Here the extra text is a second delivery of the same instruction, not additional guidance.
Approach (WHAT)
--cleanwas requested.--dry-runread-only, support custom Claude roots outside home, and report retained files and actual removals.Implementation (HOW)
Paths below are relative to the repository; runtime modules are under
src/apm_cli/.compilation/instruction_dedup.pycompilation/agents_compiler.pyintegration/instruction_integrator.pycompilation/user_root_context.pyintegration/cleanup.pycommands/compile/cli.py--clean, render outcomes, and count removals in summaries..apm/architecture/owners/core-runtime.jsonscripts/architecture_linter/checks/registry_owner_guards.pytests/integration/test_architecture_owner_rule_mutations.pytests/unit/scripts/test_architecture_runner.pytests/unit/compilation/test_instruction_dedup.pytests/unit/compilation/test_global_claude_dedup.pytests/unit/integration/test_cleanup_helper.pytests/integration/test_global_claude_dedup.pydocs/src/content/docs/reference/cli/compile.mdpackages/apm-guide/.apm/skills/apm-usage/commands.mdCHANGELOG.mdDiagrams
Legend: dashed nodes are the new Claude coverage and cleanup decisions; other targets retain their full output.
flowchart LR subgraph Source[Source] A["Global instructions"] end subgraph Coverage[Native coverage] B["uncovered_instructions: compare install-rendered content"] C["Uncovered instructions"] D["All instructions covered"] end subgraph Output[Output] E["CompiledOutputWriter: fallback only"] F["No new Claude root"] G["Existing root retained unless --clean and exact expected output"] H["remove_stale_deployed_files or read-only preview"] I["Other targets: full instruction set"] end A --> B A --> I B --> C --> E B --> D --> F D --> G --> H classDef new stroke-dasharray: 5 5; class B,C,D,F,G,H new;Trade-offs
Benefits
Validation
uv run pytest tests/unit tests/test_console.py -x -q -n 8:uv run pytest tests/integration/test_global_claude_dedup.py -q -n 0:The original two unlinked install/compile cases failed before the fix because the duplicate
CLAUDE.mdexisted. They now pass, along with linked-content and cleanup cases.Ruff lint/format, pylint R0801, YAML I/O, file-length, portable-path, auth-signal, and architecture boundary checks passed. The Mermaid diagram was validated and rendered with
mmdc.Pre-existing architecture mutation-matrix failures
The matrix completeness/order checks fail identically in an untouched worktree at base
c6fde64188c86ed2e6047a35e9ea4716df9eb1fc:Base command:
python -m pytest tests/integration/test_architecture_owner_rule_mutations.py -q -n 0 -k 'matrix_covers or matrix_case_order':Branch command:
uv run pytest tests/integration/test_architecture_owner_rule_mutations.py -q -n 0 -k 'matrix_covers or matrix_case_order or instruction-deduplication':The three checks for the new deduplication guard pass. Unrelated baseline matrix defects are left unchanged; this is not a claim that every CI job is green.
Scenario Evidence
tests/integration/test_global_claude_dedup.py::test_installed_global_rules_are_not_compiled_again(regression-trap for #2792)tests/unit/compilation/test_global_claude_dedup.py::test_only_equivalent_instructions_are_omittedtests/unit/compilation/test_instruction_dedup.py::test_same_filename_omits_only_matching_package_contenttests/integration/test_global_claude_dedup.py::test_global_clean_previews_then_removes_only_redundant_claude_roottests/unit/compilation/test_global_claude_dedup.py::test_cleanup_preserves_unverifiable_content,test_cleanup_preserves_root_symlinks,test_cleanup_preserves_cyclic_root_symlinktests/unit/compilation/test_instruction_dedup.py::test_escaping_native_symlink_keeps_fallback,test_cyclic_native_symlink_keeps_fallbacktests/unit/compilation/test_global_claude_dedup.py::test_cleanup_error_is_reported_without_losing_rootHow to test
uv sync --extra devto install the development dependencies.uv run pytest tests/integration/test_global_claude_dedup.py -q -n 0; all five synthetic CLI scenarios should pass without touching your installed global context.uv run pytest tests/unit/compilation/test_global_claude_dedup.py tests/unit/compilation/test_instruction_dedup.py tests/unit/integration/test_cleanup_helper.py -q -n 0; fallback, ownership, and path-safety cases should pass.bash scripts/lint-architecture-boundaries.sh; it should exit zero. The unrelated mutation-matrix failures are documented above.apm-spec-waiver: Critical-path integration edits only extract existing rule naming and forward the existing path-validator override; current callers and registered deployment contracts are unchanged. Observable behavior changes are confined to the user-scope compiler.