ISSUE-355: Verify hook event and matcher against the declaring component

Sources/mcs/Core/Constants.swift

@@ -114,6 +114,21 @@ enum Constants {
         static let validRawValues: Set<String> = Set(allCases.map(\.rawValue))
     }
 
+    // MARK: - Hook Commands
+
+    /// Prefixes for the `command` string a hook component registers in settings.
+    ///
+    /// Sync writes these and doctor reads them back to match a settings entry against the
+    /// component that declared it, so both sides must build the string identically — see
+    /// `ComponentDefinition.hookCommand(prefix:)`.
+    enum HookCommand {
+        /// Project scope — hooks live in `<project>/.claude/hooks/`.
+        static let projectPrefix = "bash .claude/hooks/"
+
+        /// Global scope — hooks live in `~/.claude/hooks/`.
+        static let globalPrefix = "bash ~/.claude/hooks/"
+    }
+
     // MARK: - MCP Scopes
 
     enum MCPScope {

Sources/mcs/Core/Settings.swift

@@ -151,6 +151,32 @@ struct Settings: Codable {
         removeHookEntry(event: event.rawValue, command: command)
     }
 
+    /// Where a hook command sits in the tree: the event it is registered under, and the matcher
+    /// of the group holding it. `matcher` lives on the group, not the entry.
+    struct HookPlacement {
+        let event: String
+        let matcher: String?
+    }
+
+    /// Indexes every registered hook command by the placements it appears in, in a single pass.
+    ///
+    /// A command can legitimately appear more than once — `addHookEntry` keys on a group's first
+    /// entry and appends rather than replaces when it finds no match — so placements are collected
+    /// rather than collapsed.
+    func hookPlacementsByCommand() -> [String: [HookPlacement]] {
+        var index: [String: [HookPlacement]] = [:]
+        for (event, groups) in hooks ?? [:] {
+            for group in groups {
+                for entry in group.hooks ?? [] {
+                    guard let command = entry.command else { continue }
+                    index[command, default: []]
+                        .append(HookPlacement(event: event, matcher: group.matcher))
+                }
+            }
+        }
+        return index
+    }
+
     // MARK: - Deep Merge
 
     /// Merge `other` into `self`, preserving existing user values.

Sources/mcs/Doctor/CoreDoctorChecks.swift

@@ -366,12 +366,55 @@ struct ProjectIndexCheck: DoctorCheck {
     }
 }
 
-/// Verifies that pack-contributed hook commands are still present in the settings file.
+/// A hook command a pack contributed, paired with what the pack declared for it.
+struct ExpectedHook {
+    let command: String
+
+    /// Declared registration when the command still traces back to a component, which is the
+    /// normal case — sync only records a hook command for a component that had a
+    /// `HookRegistration`, so every freshly written record has one.
+    ///
+    /// nil means the recorded command no longer maps to any component in the pack: the component
+    /// was removed or its destination renamed since the last sync. There is nothing to compare
+    /// against, so the command falls back to presence-only verification rather than being
+    /// reported as drift.
+    let registration: HookRegistration?
+}
+
+/// Verifies that pack-contributed hook commands are still present in the settings file, and that
+/// they are registered the way the declaring component said they should be.
+///
+/// A hook whose matcher does not match any tool Claude Code emits installs cleanly, registers, and
+/// fires for nothing — the symptom is an empty log, which reads as "no problems" rather than
+/// "never ran". Comparing the installed `event` and `matcher` against the component's
+/// `HookRegistration` turns that silence into a reported finding.
+///
+/// Absence is a failure; a registration that differs from what was declared is a warning, because
+/// `Settings.addHookEntry` rewrites a differing matcher on the next sync — so "run 'mcs sync'" is
+/// a remedy that actually works, and a user who narrowed a matcher deliberately is not blocked.
+///
+/// This proves the declared matcher reached settings, **not** that it matches any tool name Claude
+/// Code actually emits. Only a real session transcript proves that.
 struct HookSettingsCheck: DoctorCheck {
-    let commands: [String]
+    let expectations: [ExpectedHook]
     let settingsPath: URL
     let packName: String
 
+    init(expectations: [ExpectedHook], settingsPath: URL, packName: String) {
+        self.expectations = expectations
+        self.settingsPath = settingsPath
+        self.packName = packName
+    }
+
+    /// Presence-only verification, for callers with no declaration to compare against.
+    init(commands: [String], settingsPath: URL, packName: String) {
+        self.init(
+            expectations: commands.map { ExpectedHook(command: $0, registration: nil) },
+            settingsPath: settingsPath,
+            packName: packName
+        )
+    }
+
     var name: String {
         "Hook entries (\(packName))"
     }
@@ -390,21 +433,81 @@ struct HookSettingsCheck: DoctorCheck {
         } catch {
             return .fail("cannot read settings: \(error.localizedDescription)")
         }
-        let allCommands = (settings.hooks ?? [:]).values
-            .flatMap(\.self)
-            .compactMap(\.hooks)
-            .flatMap(\.self)
-            .compactMap(\.command)
-        let commandSet = Set(allCommands)
-        let missing = commands.filter { !commandSet.contains($0) }
-        if missing.isEmpty {
-            return .pass("all hook commands present")
+
+        // One traversal of the hook tree, then a lookup per expectation.
+        let placementsByCommand = settings.hookPlacementsByCommand()
+        var missing: [String] = []
+        var drift: [String] = []
+        for expectation in expectations {
+            let placements = placementsByCommand[expectation.command] ?? []
+            guard !placements.isEmpty else {
+                missing.append(expectation.command)
+                continue
+            }
+            if let registration = expectation.registration,
+               let finding = driftFinding(
+                   command: expectation.command,
+                   registration: registration,
+                   placements: placements
+               ) {
+                drift.append(finding)
+            }
+        }
+
+        if !missing.isEmpty {
+            // Append drift so a failure never swallows findings from other expectations.
+            let details = ["missing hook commands: \(missing.joined(separator: ", "))"] + drift
+            return .fail(details.joined(separator: "; "))
+        }
+        if !drift.isEmpty {
+            return .warn("\(drift.joined(separator: "; ")) — run 'mcs sync'")
         }
-        return .fail("missing hook commands: \(missing.joined(separator: ", "))")
+        // Claim "as declared" only when every expectation actually had a declaration — a mixed set
+        // would otherwise assert verification the presence-only entries never got.
+        let allDeclared = !expectations.isEmpty && expectations.allSatisfy { $0.registration != nil }
+        return .pass(allDeclared ? "all hook commands registered as declared" : "all hook commands present")
     }
 
     func fix() -> FixResult {
-        .notFixable("Run 'mcs sync' to restore hook entries")
+        .notFixable("Run 'mcs sync' to restore or update hook entries")
+    }
+
+    // MARK: - Helpers
+
+    /// Describes how `placements` differ from what was declared, or nil when they satisfy it.
+    ///
+    /// Extra registrations are not policed — a command may appear under events the pack never
+    /// declared, which is the user's business. Only the declared registration must be present.
+    private func driftFinding(
+        command: String,
+        registration: HookRegistration,
+        placements: [Settings.HookPlacement]
+    ) -> String? {
+        let expectedEvent = registration.event.rawValue
+        let underEvent = placements.filter { $0.event == expectedEvent }
+        guard !underEvent.isEmpty else {
+            let actual = Set(placements.map(\.event)).sorted().joined(separator: ", ")
+            return "'\(command)' is registered under \(actual), pack declares \(expectedEvent)"
+        }
+        // Any group under the declared event carrying the declared matcher satisfies this: a
+        // command can legitimately sit in more than one group, since `addHookEntry` matches on the
+        // group's *first* entry and appends rather than replaces when it finds no match.
+        let expected = normalized(registration.matcher)
+        guard !underEvent.contains(where: { normalized($0.matcher) == expected }) else { return nil }
+        let actual = underEvent.map { describe($0.matcher) }.joined(separator: ", ")
+        return "'\(command)' has matcher \(actual) under \(expectedEvent),"
+            + " pack declares \(describe(registration.matcher))"
+    }
+
+    /// Treats an empty matcher as an absent one, so a written `""` is not reported as drift.
+    private func normalized(_ matcher: String?) -> String? {
+        guard let matcher, !matcher.isEmpty else { return nil }
+        return matcher
+    }
+
+    private func describe(_ matcher: String?) -> String {
+        guard let matcher = normalized(matcher) else { return "no matcher" }
+        return "'\(matcher)'"
     }
 }
 

Sources/mcs/Doctor/DoctorRunner.swift

@@ -42,6 +42,18 @@ struct DoctorRunner {
         let label: String
         let artifactsByPack: [String: PackArtifactRecord]
 
+        /// Prefix the hook commands in `artifactsByPack` were recorded with.
+        ///
+        /// Relies on an invariant every `CheckScope` construction path upholds: a non-nil
+        /// `effectiveProjectRoot` means the records came from project state, which sync wrote
+        /// with the project prefix. Global-only packs get their own scope rather than being
+        /// folded into a project one, so the two never mix.
+        var hookCommandPrefix: String {
+            effectiveProjectRoot != nil
+                ? Constants.HookCommand.projectPrefix
+                : Constants.HookCommand.globalPrefix
+        }
+
         func makeCollisionContext(environment: Environment) -> (any CollisionFilesystemContext)? {
             let trackedFiles = PackArtifactRecord.allTrackedFiles(from: artifactsByPack.values)
             if let root = effectiveProjectRoot {
@@ -474,7 +486,7 @@ struct DoctorRunner {
             if !artifacts.hookCommands.isEmpty {
                 checks.append((
                     check: HookSettingsCheck(
-                        commands: artifacts.hookCommands,
+                        expectations: hookExpectations(for: artifacts, pack: pack, scope: scope),
                         settingsPath: settingsPath,
                         packName: pack.displayName
                     ),
@@ -518,6 +530,33 @@ struct DoctorRunner {
         return checks
     }
 
+    /// Pairs each recorded hook command with the `HookRegistration` of the component that declared
+    /// it, so the check can verify *how* the hook was registered and not merely that it exists.
+    ///
+    /// The join key is the command string, rebuilt with `hookCommand(prefix:)` — the same helper
+    /// sync used to write it. `pack` is already collision-resolved here, so namespaced
+    /// destinations match what sync actually installed. Commands with no matching component
+    /// (hooks supplied via `settingsFile:`, or state files predating this check) get a nil
+    /// registration and keep presence-only semantics.
+    private func hookExpectations(
+        for artifacts: PackArtifactRecord,
+        pack: any TechPack,
+        scope: CheckScope
+    ) -> [ExpectedHook] {
+        var registrationsByCommand: [String: HookRegistration] = [:]
+        for component in pack.components {
+            // Bind the registration explicitly rather than relying on `hookCommand(prefix:)`
+            // already having required one — same shape the sync-side join uses.
+            if let registration = component.hookRegistration,
+               let command = component.hookCommand(prefix: scope.hookCommandPrefix) {
+                registrationsByCommand[command] = registration
+            }
+        }
+        return artifacts.hookCommands.map {
+            ExpectedHook(command: $0, registration: registrationsByCommand[$0])
+        }
+    }
+
     // MARK: - Standalone checks (not tied to any component)
 
     /// Checks that cannot be derived from any ComponentDefinition.

Sources/mcs/Sync/Configurator.swift

@@ -578,10 +578,7 @@ struct Configurator {
                     } else {
                         output.warn("  Could not remove '\(relativePath)' — will retry on next sync")
                     }
-                    if component.type == .hookFile,
-                       component.hookRegistration != nil,
-                       fileType == .hook {
-                        let hookCmd = "\(scope.hookCommandPrefix)\(destination)"
+                    if let hookCmd = component.hookCommand(prefix: scope.hookCommandPrefix) {
                         artifacts.hookCommands.removeAll { $0 == hookCmd }
                     }
 

Sources/mcs/Sync/ConfiguratorSupport.swift

@@ -195,10 +195,8 @@ enum ConfiguratorSupport {
             for component in pack.components {
                 guard !excluded.contains(component.id) else { continue }
 
-                if component.type == .hookFile,
-                   let reg = component.hookRegistration,
-                   case let .copyPackFile(_, destination, .hook) = component.installAction {
-                    let command = "\(hookCommandPrefix)\(destination)"
+                if let reg = component.hookRegistration,
+                   let command = component.hookCommand(prefix: hookCommandPrefix) {
                     if settings.addHookEntry(
                         event: reg.event,
                         command: command,

Sources/mcs/Sync/GlobalSyncStrategy.swift

@@ -116,10 +116,8 @@ struct GlobalSyncStrategy: SyncStrategy {
                     let relativePath = fileRelativePath(destination: destination, fileType: fileType)
                     artifacts.files.append(relativePath)
                     artifacts.fileHashes.merge(result.hashes) { _, new in new }
-                    if component.type == .hookFile,
-                       component.hookRegistration != nil,
-                       fileType == .hook {
-                        artifacts.hookCommands.append("\(scope.hookCommandPrefix)\(destination)")
+                    if let hookCommand = component.hookCommand(prefix: scope.hookCommandPrefix) {
+                        artifacts.hookCommands.append(hookCommand)
                     }
                     output.success("  \(component.displayName) installed")
                 }

Sources/mcs/Sync/ProjectSyncStrategy.swift

@@ -93,10 +93,8 @@ struct ProjectSyncStrategy: SyncStrategy {
                 )
                 artifacts.files.append(contentsOf: result.paths)
                 artifacts.fileHashes.merge(result.hashes) { _, new in new }
-                if component.type == .hookFile,
-                   component.hookRegistration != nil,
-                   fileType == .hook {
-                    artifacts.hookCommands.append("\(scope.hookCommandPrefix)\(destination)")
+                if let hookCommand = component.hookCommand(prefix: scope.hookCommandPrefix) {
+                    artifacts.hookCommands.append(hookCommand)
                 }
                 if !result.paths.isEmpty {
                     output.success("  \(component.displayName) installed")

Sources/mcs/Sync/SyncScope.swift

@@ -75,7 +75,7 @@ extension SyncScope {
             isGlobalScope: false,
             syncHint: "mcs sync",
             labelSuffix: "",
-            hookCommandPrefix: "bash .claude/hooks/",
+            hookCommandPrefix: Constants.HookCommand.projectPrefix,
             fileDisplayPrefix: ".claude/"
         )
     }
@@ -95,7 +95,7 @@ extension SyncScope {
             isGlobalScope: true,
             syncHint: "mcs sync --global",
             labelSuffix: " (global)",
-            hookCommandPrefix: "bash ~/.claude/hooks/",
+            hookCommandPrefix: Constants.HookCommand.globalPrefix,
             fileDisplayPrefix: "~/.claude/"
         )
     }

Sources/mcs/TechPack/Component.swift

@@ -118,6 +118,24 @@ struct ComponentDefinition: Identifiable {
     #endif
 }
 
+extension ComponentDefinition {
+    /// The settings `command` string this component registers, or nil when it registers no hook.
+    ///
+    /// Single source of truth for the sync↔doctor join: sync writes this string into
+    /// `settings.local.json` and records it in `PackArtifactRecord.hookCommands`, and doctor
+    /// rebuilds it to match a recorded command back to the component that declared it. Both sides
+    /// must build it identically — a divergence makes every hook look missing.
+    ///
+    /// - Parameter prefix: Scope-dependent prefix, `Constants.HookCommand.projectPrefix` or
+    ///   `.globalPrefix`.
+    func hookCommand(prefix: String) -> String? {
+        guard type == .hookFile, hookRegistration != nil,
+              case let .copyPackFile(_, destination, .hook) = installAction
+        else { return nil }
+        return prefix + destination
+    }
+}
+
 /// How to install a component
 enum ComponentInstallAction {
     case mcpServer(MCPServerConfig)