test: add 25 multi-collection conflict scenario tests

[malpern]

Summary

Closes #534

• 3-way conflict detection (3 tests): three collections on same key, multiple conflicting keys with varying overlap counts, disabled collection excluded
• Conflict resolution cascade (2 tests): revealed conflict after disabling one collection, no conflict after all disabled
• Activator conflicts (3 tests): same key with different target layers, identical activators treated as redundant (not conflict), different activator keys
• Cross-layer edge cases (3 tests): same key different layers (no conflict), same custom layer (conflict), different custom layers (no conflict)
• Deduplicator multi-collection ordering (4 tests): first collection wins, mixed layers not deduped, disabled not counted in dedup, duplicate activator removed
• Neovim Terminal exclusion (2 tests): never conflicts as candidate, doesn't block new collections when existing
• Custom rule conflicts (4 tests): rule vs collection, rule vs other rule, rule on different layer (no conflict), disabled rule (no conflict)
• Key normalization (2 tests): caps/capslock normalize to same key, modifier name variants
• Conflict info (2 tests): description format, hold descriptions

Test plan

• All 25 new tests pass
• Tests run in <1s
• Build passes

[claude]

Code Review: test/534-multi-collection-conflicts

Overall: Good test coverage of a complex domain. The 25 tests are well-organized and cover the important edge cases. A few issues worth noting for future test work in this area.

---

Bugs / Dead Code

testConflictInfo_DifferentCustomLayersNoConflict — dead code (lines ~175–181)

```swift
let symbols = collection(name: "Symbols", mappings: [("h", "!")]) // created, never read

var symbolsOnCustom = symbols // assigned from symbols...
symbolsOnCustom = collection(name: "Symbols", mappings: [("h", "!")], layer: .custom("symbols")) // ...then immediately overwritten
```

Both symbols and the first assignment to symbolsOnCustom are dead code. The test still works because the second assignment wins, but the original intent was probably just:

```swift
let symbolsOnCustom = collection(name: "Symbols", mappings: [("h", "!")], layer: .custom("symbols"))
```

testDeduplicator_DuplicateActivatorRemoved — var should be let

```swift
var a = collection(...) // never mutated
var b = collection(...) // never mutated
```

Both should be let. Likely a leftover from a refactor. SwiftFormat/SwiftLint should catch this, but worth fixing explicitly.

---

Convention Violations (CLAUDE.md)

Should extend KeyPathTestCase, not XCTestCase

CLAUDE.md states: "Never call real pgrep in tests → deadlock. Use KeyPathTestCase base class."

This class extends XCTestCase directly. If RuleCollectionsManager, ConfigurationService, or any transitive dependency triggers a pgrep/launchctl call, these tests will deadlock. Even if they don't today, using KeyPathTestCase is the defensive default for all test classes in this target.

---

Test Isolation Concern

makeManager() uses .shared stores

```swift
RuleCollectionsManager(
ruleCollectionStore: .shared,
customRulesStore: .shared,
configurationService: ConfigurationService()
)
```

If .shared stores carry any pre-existing state (from app startup, other tests, or prior test runs in the suite), tests could produce false passes or false failures. The tests that set manager.ruleCollections = [...] and manager.customRules = [...] directly bypass any store-backed initialization, which is good, but the shared store reference is still risky if any internal path reads back from the store.

Consider injecting a fresh in-memory or stub store instance per test, or at minimum adding a setUp that resets known shared state.

---

Minor Nits

• testConflictInfo_DifferentActivatorsSameKey — the test name says "SameKey" referring to the activator key ("space"), but the mapping keys are different ("h" vs "j"). The comment "Same key ('space') with different target layers should conflict" clarifies it, but the name is slightly ambiguous. Could be testConflictInfo_SameActivatorKeyDifferentTargetLayers.

• testMappingConflictInfo_HoldDescriptions — only checks holdDescriptions[0]. Checking [1] as well would strengthen the assertion for the full count: 2 claim.

• testConflictInfo_CustomRuleConflictsWithOtherCustomRule — no failure message on the final XCTAssertNotNil. All other XCTAssertNotNil calls in this file have descriptive messages; this one is the only exception.

---

What's Good

• Coverage of 3-way conflicts, conflict cascade after disabling, and the Neovim Terminal exclusion is exactly the right set of hard cases to pin down.
• Using MARK sections makes the file easy to navigate.
• Tests are clearly named and each tests exactly one behavior.
• The key normalization tests (caps/capslock, left shift/lshift) are important regression anchors.
• PR description maps cleanly to test sections — easy to audit.

---

Summary: The dead code in testConflictInfo_DifferentCustomLayersNoConflict is the most important fix (it's misleading). The KeyPathTestCase base class is a CLAUDE.md hard requirement. Everything else is low-priority polish.