Skip to content

Admission engine owns conflict detection: claims by rule id replace the wrapped detector #1322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
malpern merged 1 commit into from
Oct 20, 2026
Merged

Admission engine owns conflict detection: claims by rule id replace the wrapped detector #1322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions Sources/KeyPathAppKit/CLI/RulesFacade.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,10 @@ public struct RulesFacade: Sendable {
guard let collectionLoader else { return [] }
let collections = await collectionLoader()
// Custom rules first, exactly as generateConfiguration orders them.
let after = RuleCollectionDeduplicator.detectConflicts(
in: rules.asRuleCollections() + collections
)
let after = AdmissionEngine.mappingConflicts(in: rules.asRuleCollections() + collections)
guard !after.isEmpty else { return [] }
let before = Set(
RuleCollectionDeduplicator
.detectConflicts(in: baselineRules.asRuleCollections() + collections)
.map(\.description)
AdmissionEngine.mappingConflicts(in: baselineRules.asRuleCollections() + collections).map(\.description)
)
return after.filter { !before.contains($0.description) }
}
Expand Down
10 changes: 3 additions & 7 deletions Sources/KeyPathAppKit/Infrastructure/Config/ConfigurationService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,13 +1111,9 @@ public final class ConfigurationService: FileConfigurationProviding {
let leaderKeyPref = leaderKeyPreference ?? storedLeaderKeyPref
let shortcutInput = shortcutListGenerationInput

// DETECT CONFLICTS BEFORE DEDUPLICATION
// This catches cases where multiple collections map the same key, and where
// a collection activator collides with the system leader key (#463).
let conflicts = RuleCollectionDeduplicator.detectConflicts(
in: collectionsForConfig,
leaderKey: leaderKeyPref
)
// Admission before deduplication: two intentional claims on one slot are
// refused here, so `dedupe` below never resolves one in silence (ADR-039 §1).
let conflicts = AdmissionEngine.mappingConflicts(in: collectionsForConfig, leaderKey: leaderKeyPref)
if !conflicts.isEmpty {
AppLogger.shared.log(
"⚠️ [ConfigService] Mapping conflicts detected: \(conflicts.map(\.description).joined(separator: "; "))"
Expand Down
2 changes: 1 addition & 1 deletion Sources/KeyPathAppKit/Infrastructure/Config/KanataConfiguration+BlockBuilders.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ extension KanataConfiguration {
/// Safety net: detect duplicate alias names and keep only the last definition
/// so kanata doesn't reject the config outright.
///
/// The primary defense is `RuleCollectionDeduplicator.detectConflicts()` which
/// The primary defense is `AdmissionEngine.conflicts(in:leaderKey:)()` which
/// blocks config writes when collections overlap. This function catches edge
/// cases that slip past (e.g., aliases generated from non-mapping paths).
/// It logs warnings so the root cause is visible in diagnostics.
Expand Down
261 changes: 6 additions & 255 deletions Sources/KeyPathAppKit/Models/RuleCollectionDeduplicator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,258 +3,9 @@ import KeyPathCore
import KeyPathRulesCore

enum RuleCollectionDeduplicator {
// MARK: - Conflict Detection

/// Detects mapping conflicts BEFORE deduplication.
/// Returns conflicts where multiple collections map the same key in the same layer.
/// Call this before `dedupe()` to catch conflicts that would otherwise be silently resolved.
///
/// Uses `effectiveMappings(for:)` to include config-generated mappings (Home Row Mods,
/// Layer Toggles, etc.) — not just the raw `.mappings` array.
static func detectConflicts(
in collections: [RuleCollection],
leaderKey: LeaderKeyPreference? = nil
) -> [KeyPathError.MappingConflictInfo] {
struct ClaimInfo {
let collectionName: String
let holdDescription: String?
}

var claimedKeys: [InputKey: [ClaimInfo]] = [:]
// Which collections place a regular mapping on each (input, layer) slot,
// keyed by collection id → name. Mappings only (NOT activators or the leader)
// so it can be cross-checked against activator placements without
// double-reporting. Keyed by id, not name: collection titles aren't unique
// (custom rules become collections titled by editable display text), so a
// name-based self-filter could cancel the real other owner.
var mappingOwners: [InputKey: [UUID: String]] = [:]

for collection in collections where collection.isEnabled {
let mappings = KanataConfiguration.effectiveMappings(for: collection)
var seenKeysInCollection = Set<String>()
for mapping in mappings {
let normalizedInput = KanataKeyConverter.convertToKanataKey(mapping.input)
// Skip duplicate keys within the same collection (e.g., chord groups
// sharing keys). Within-collection conflicts are detected separately.
guard seenKeysInCollection.insert(normalizedInput).inserted else { continue }
let inputKey = InputKey(input: normalizedInput, layer: collection.targetLayer)
mappingOwners[inputKey, default: [:]][collection.id] = collection.name

let holdDesc = mapping.behavior.flatMap { behavior -> String? in
if case let .dualRole(dr) = behavior {
return dr.holdActionString
}
return nil
}

claimedKeys[inputKey, default: []].append(
ClaimInfo(collectionName: collection.name, holdDescription: holdDesc)
)
}
}

// The leader key (#463) also consumes its physical key's BASE-layer slot:
// buildCollectionBlocks emits the leader as a base entry (tap = key, hold =
// layer) before any collection, and deduplicateBlocks keeps it over a later
// base mapping — so a base-layer collection or custom rule that maps the same
// key is silently dropped. Claim that base slot so the collision surfaces.
if let leaderKey, leaderKey.enabled {
let normalizedInput = KanataKeyConverter.convertToKanataKey(leaderKey.key)
let inputKey = InputKey(input: normalizedInput, layer: .base)
claimedKeys[inputKey, default: []].append(
ClaimInfo(
collectionName: "Leader Key",
holdDescription: "\(leaderKey.targetLayer.displayName) layer"
)
)
}

var conflicts: [KeyPathError.MappingConflictInfo] = []
for (inputKey, claims) in claimedKeys where claims.count > 1 {
let collectionNames = claims.map(\.collectionName)
let holdDescriptions = claims.compactMap { claim -> String? in
guard let hold = claim.holdDescription else { return nil }
return "\(claim.collectionName): hold → \(hold)"
}

conflicts.append(KeyPathError.MappingConflictInfo(
inputKey: inputKey.input,
layer: inputKey.layer.displayName,
conflictingCollections: collectionNames,
holdDescriptions: holdDescriptions
))
}

// Momentary activator conflicts (#466): two collections claim the same
// physical activator key from the same source layer but route it to
// different target layers. The silent `seenActivators` dedup in
// buildCollectionBlocks would drop one without notice — surface it via the
// same detect-and-explain path instead.
//
// The claim slot is (sourceLayer, input): a key can only carry one binding
// per layer. The same key activating different targets from *different*
// source layers is a valid chained-layer setup (e.g. `f` → arrows from base
// and `f` → function from nav), not a conflict. Identical activators (same
// slot + same target) are redundant, not conflicting. Both are skipped so
// the detection does not over-fire (matching the toggle-time conflictInfo).
struct ActivatorSlot: Hashable {
let sourceLayer: RuleCollectionLayer
let input: String
}
struct ActivatorClaim {
let collectionName: String
let targetLayer: RuleCollectionLayer
}
var activatorClaims: [ActivatorSlot: [ActivatorClaim]] = [:]
// The system leader key (#463) is an activator too: pressing it from the
// base layer activates its target layer. buildCollectionBlocks seeds it into
// seenActivators before any collection, so a collection activator on the
// same base-layer key is silently dropped — include it as a claim so the
// collision is surfaced instead.
if let leaderKey, leaderKey.enabled {
let slot = ActivatorSlot(
sourceLayer: .base,
input: KanataKeyConverter.convertToKanataKey(leaderKey.key)
)
activatorClaims[slot, default: []].append(
ActivatorClaim(collectionName: "Leader Key", targetLayer: leaderKey.targetLayer)
)
}
for collection in collections where collection.isEnabled {
guard let activator = collection.momentaryActivator else { continue }
// "hyper" activators are folded into the hyper hold action, not emitted
// as standalone layer activators — they never collide in seenActivators.
guard activator.input.lowercased() != "hyper" else { continue }
let normalizedInput = KanataKeyConverter.convertToKanataKey(activator.input)
let slot = ActivatorSlot(sourceLayer: activator.sourceLayer, input: normalizedInput)
activatorClaims[slot, default: []].append(
ActivatorClaim(collectionName: collection.name, targetLayer: activator.targetLayer)
)

// Activator-vs-mapping conflict: this activator occupies its key's slot
// on its source layer (buildCollectionBlocks places the tap-hold there),
// but another collection maps the same key on that layer. The generator
// emits both and silently keeps one (e.g. Home Row Arrows' `f` activator
// shadowing Home Row Mods' hold-`f`). Surface it. Activator-vs-activator
// is handled by the redundant-aware pass below; leader collisions via the
// leader injection above — neither is in `mappingOwners`, so no double-report.
let mappingSlot = InputKey(input: normalizedInput, layer: activator.sourceLayer)
// Exclude the activator's own collection by id (names aren't unique).
let otherMappers = (mappingOwners[mappingSlot] ?? [:])
.filter { $0.key != collection.id }
.values.sorted()
if !otherMappers.isEmpty {
conflicts.append(KeyPathError.MappingConflictInfo(
inputKey: normalizedInput,
layer: activator.sourceLayer.displayName,
conflictingCollections: [collection.name] + otherMappers,
holdDescriptions: ["\(collection.name): activates \(activator.targetLayer.displayName)"]
+ otherMappers.map { "\($0): maps \(normalizedInput)" }
))
}
}
for (slot, claims) in activatorClaims where claims.count > 1 {
// Only a conflict if the activators disagree on which layer to activate.
let distinctTargets = Set(claims.map(\.targetLayer))
guard distinctTargets.count > 1 else { continue }
conflicts.append(KeyPathError.MappingConflictInfo(
inputKey: slot.input,
layer: slot.sourceLayer.displayName,
conflictingCollections: claims.map(\.collectionName),
holdDescriptions: claims.map { "\($0.collectionName): activates \($0.targetLayer.displayName)" }
))
}

// Within-collection chord group conflicts: two chord groups sharing keys
// produces duplicate aliases that kanata rejects.
for collection in collections where collection.isEnabled {
if case let .chordGroups(config) = collection.configuration {
// Duplicate group names (#464): each group renders to a
// `(defchords <name> …)` block keyed by its name, so two groups
// sharing a name produce duplicate blocks that kanata rejects.
for duplicate in config.detectDuplicateGroupNames() {
conflicts.append(KeyPathError.MappingConflictInfo(
inputKey: duplicate.name,
layer: collection.targetLayer.displayName,
conflictingCollections: Array(
repeating: "chord group '\(duplicate.name)'",
count: duplicate.count
),
holdDescriptions: [
"\(duplicate.count) chord groups share the name '\(duplicate.name)' — chord group names must be unique"
]
))
}

let chordConflicts = config.detectCrossGroupConflicts()
for conflict in chordConflicts {
let groupNames = conflict.groups.map(\.name)
let chordDescriptions = conflict.groups.map { group -> String in
let chordOutputs = group.chords
.filter { $0.keys.contains(conflict.key) }
.map { "(\($0.keys.joined(separator: "+")) → \($0.action.displayName))" }
.joined(separator: ", ")
return "\(group.name): \(chordOutputs)"
}
conflicts.append(KeyPathError.MappingConflictInfo(
inputKey: conflict.key,
layer: collection.targetLayer.displayName,
conflictingCollections: groupNames,
holdDescriptions: chordDescriptions
))
}
}
}

// Alias-name collisions (#462): generated alias names are
// `{prefix}_{layer}_{sanitized-input}`, and sanitization maps `-`/space (and,
// for device switches, any non-alphanumeric) to `_`. Two distinct input keys
// can therefore collapse to the same alias name (e.g. `caps-lock` and
// `caps_lock` → `beh_base_caps_lock`), which kanata rejects. These keys don't
// collide as defsrc inputs, so the key-overlap pass above misses them.
struct AliasOrigin {
let collectionName: String
let rawInput: String
let layer: String
}
var aliasOrigins: [String: [AliasOrigin]] = [:]
for collection in collections where collection.isEnabled {
for mapping in KanataConfiguration.effectiveMappings(for: collection) {
// Chord inputs (space-separated) render to defchordsv2, not aliases.
guard !mapping.input.contains(" ") else { continue }
for aliasName in KanataConfiguration.generatedAliasNames(
for: mapping, layer: collection.targetLayer
) {
aliasOrigins[aliasName, default: []].append(AliasOrigin(
collectionName: collection.name,
rawInput: mapping.input,
layer: collection.targetLayer.displayName
))
}
}
}
for (aliasName, origins) in aliasOrigins {
// Only a collision when *different* input keys produce the same alias.
// Same input across collections is already a key-overlap conflict above.
let distinctInputs = Set(origins.map(\.rawInput))
guard distinctInputs.count > 1 else { continue }
conflicts.append(KeyPathError.MappingConflictInfo(
inputKey: distinctInputs.sorted().joined(separator: " / "),
layer: origins.first?.layer ?? RuleCollectionLayer.base.displayName,
conflictingCollections: Array(Set(origins.map(\.collectionName))).sorted(),
holdDescriptions: ["keys \(distinctInputs.sorted().joined(separator: ", ")) all generate the alias '\(aliasName)'"]
))
}

// Total ordering (inputKey, then layer, then explanation) so output is
// deterministic even when two conflicts share an inputKey — e.g. the same
// pair of keys colliding on two different alias prefixes at once.
return conflicts.sorted { lhs, rhs in
if lhs.inputKey != rhs.inputKey { return lhs.inputKey < rhs.inputKey }
if lhs.layer != rhs.layer { return lhs.layer < rhs.layer }
return lhs.holdDescriptions.joined() < rhs.holdDescriptions.joined()
}
}
// Conflict detection lives in `AdmissionEngine.conflicts(in:leaderKey:)`,
// attributed by rule id. This type only applies the generator's structural
// "first claim wins" after admission has refused every intentional overlap.

/// Deduplicates collections by removing duplicate input keys.
/// - First collection to claim an input key (per layer) wins
Expand Down Expand Up @@ -309,9 +60,9 @@ enum RuleCollectionDeduplicator {
let inputKey = InputKey(input: normalizedInput, layer: collection.targetLayer)

if seenInputKeys.contains(inputKey) {
// Admission (detectConflicts / AdmissionEngine) runs before this and
// refuses any two intentional claims on one slot, so reaching here is
// a gap in detection, not a resolution: never silently (ADR-039 §1).
// AdmissionEngine.conflicts runs before this and refuses any two
// intentional claims on one slot, so reaching here is a gap in
// detection, not a resolution: never silently (ADR-039 §1).
AppLogger.shared.warnUnlessQuietTest(
"⚠️ [Dedup] Dropped mapping '\(mapping.input)' from '\(collection.name)' — key already claimed on \(collection.targetLayer.displayName) layer. Admission should have refused this."
)
Expand Down
2 changes: 1 addition & 1 deletion Sources/KeyPathAppKit/Services/Packs/PackInstaller.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ public final class PackInstaller {
let name = RuleCollectionsManager.nameLookup(state)
let verdicts = AdmissionEngine.evaluate(state, profile: profile, fileExists: fileExists).filter { verdict in
if involvesPack(verdict.subject) { return true }
if case let .conflict(_, _, parties) = verdict.reason { return parties.contains(where: involvesPack) }
if case let .conflict(_, _, parties, _) = verdict.reason { return parties.contains(where: involvesPack) }
if case let .packConflict(other, _) = verdict.reason { return other == pack.id }
return false
}
Expand Down
Loading
Loading