ISSUE-371: Flag packs configured in both global and project scope

[bguidolim]

Summary

Installing the same pack globally and in a project leaves two live copies of everything it ships: the hook is registered under two different command strings so it fires twice, skills and commands sit in two directories, and the CLAUDE.md section is composed into both files. #370 stopped new duplicates from being created but deliberately left existing ones alone, so until now nothing found them. Closes #371.

Changes

• mcs doctor, run inside a project, reports each pack present in both scopes and names what is actually duplicated — a pack whose two scopes were customized differently, or that only contributes brew packages, plugins or MCP servers, is not flagged.
• --fix removes the project-scoped copy and keeps the global one, but refuses and explains itself unless it can prove nothing is lost: the project scope must install no component the global scope excludes, both scopes must have answered the pack's prompts identically, and every file it would delete must still match the hash recorded at install.

That last gate matters because pack removal deletes tracked files without consulting the recorded hash (#365), so an edited skill would otherwise be destroyed. Here drift makes the pack un-auto-fixable instead.

Two notes for review. This is the first fix() to drive the sync engine, which the responsibility boundary in CoreDoctorChecks.swift did not previously sanction — that comment now describes the category and the higher bar it carries. And removing a pack from one scope currently strips global gitignore lines another scope still claims, because reference counting covers only brew packages and plugins; this compensates on the doctor path and leaves a comment describing the general fix, which is also a live bug in mcs sync deselection and mcs pack remove today.

Test plan

• swift test passes locally
• swiftformat --lint . and swiftlint pass without violations
• Affected commands verified with a real pack (e.g. mcs sync, mcs doctor)

Manual verification:

1. mcs sync --global and select a pack, then mcs sync in a project and select the same one → mcs doctor reports it under Project, naming the duplicated surfaces.
2. mcs doctor --fix → the fix appears under "Available fixes"; after confirming, re-running doctor reports nothing and the pack's global artifacts still verify.
3. Edit an installed project skill, then mcs doctor --fix → expect a refusal naming the edited path, with the file untouched.
4. On a machine that has never run mcs sync --global → expect no duplication entries at all.

Checklist for engine changes

• Any fix() implementation does cleanup/migration only — never installs or registers resources — with a documented exception: this fix removes a pack from one scope by calling Configurator.unconfigurePack rather than re-implementing removal. It installs nothing, and the three gates above are what justify the wider blast radius.
• Integration tests updated for new features (LifecycleIntegrationTests or DoctorRunnerIntegrationTests)
• New file write/copy/delete paths use PathContainment.safePath() and handle the nil (escape) case — deletion goes through the existing strategy path, which already does
• Docs updated if behavior changed (CLAUDE.md, docs/, techpack.yaml schema in ExternalPackManifest.swift)

https://claude.ai/code/session_01MkBJdGBUoZ2aRtchjaKXMN

[Copilot]

🟡 Changes recommended

--pack parsing does not trim whitespace, so common inputs like --pack "ios, swift" can silently fail to match pack IDs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new mcs doctor check to detect packs that are configured in both global and project scope (where artifacts are truly duplicated), and introduces a guarded --fix path that removes only the project-scoped copy when it can prove the removal is lossless.

Changes:

• Introduces ScopeDuplicationCheck to report per-pack duplication across scopes and (when safe) remove the project-scoped copy while keeping the global copy.
• Centralizes “template dependencies vs excluded components” filtering via excludingDependencies(on:) and reuses it in both sync and doctor.
• Adds FileHasher.drift to unify “content drift” semantics between checks and fix gating; extends project index utilities and adds integration/unit tests for the new behavior.

File summaries

File	Description
Tests/MCSTests/ProjectIndexTests.swift	Adds unit tests for removing a pack from a single project entry without affecting others.
Tests/MCSTests/LifecycleIntegrationTests.swift	Adds integration coverage for scope duplication detection, fix gating (exclusions/prompts/drift), and fix behavior (including gitignore preservation).
Sources/mcs/TechPack/TechPack.swift	Adds excludingDependencies(on:) helper for consistent template filtering by excluded components.
Sources/mcs/Sync/Configurator.swift	Uses excludingDependencies(on:) during template preloading to match doctor’s section-duplication logic.
Sources/mcs/Doctor/ScopeDuplicationCheck.swift	New doctor check to detect duplicated artifacts across scopes and a guarded fix that unconfigures the project-scoped pack copy.
Sources/mcs/Doctor/DoctorRunner.swift	Wires scope-duplication checks into project doctor runs and centralizes --pack parsing into packFilterIDs.
Sources/mcs/Doctor/CoreDoctorChecks.swift	Updates --fix responsibility boundary docs and refactors FileContentCheck to use FileHasher.drift.
Sources/mcs/Core/ProjectIndex.swift	Adds removePack(_:fromProject:in:) to remove one pack from one project without stamping lastSynced.
Sources/mcs/Core/FileHasher.swift	Adds DriftState + drift(of:expecting:) for consistent drift classification across checks and fix gating.
CLAUDE.md	Documents the new ScopeDuplicationCheck behavior and its fix gates.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bguidolim]

Merged main (#368, non-bash hook interpreters) and addressed the review comment.

The --pack whitespace issue is real. The parsing is pre-existing — this PR only hoisted it into one property so the new check and resolveCheckScopes share a definition — so it is not a regression, but it is now fixed in the one place it lives. Ids are trimmed and empties dropped. Added a doctor integration test asserting "pack-a,pack-b" and `" pack-a , pack-b "" produce identical runs; without the trim it reports 3 warnings instead of 1, since both padded ids were being flagged as unregistered.

On the merge: the only conflict was two test suites appended to the same file, resolved by keeping both. #368 changes hook commands to <interpreter> <directory><destination>, which does not affect the duplication reasoning — the interpreter is per component and identical in both scopes, and only the directory is scope-dependent — so the two settings entries are still distinct strings that both fire. Reworded that doc comment to say directory rather than prefix.

Also filed the follow-ups noted in the description: #378 (gitignore entries are not reference-counted — the one with user-visible consequences today), #379, #380.