Count all warnings in mcs doctor summary (#352)

- Inject a shared WarningCounter into CLIOutput so advisories emitted
  outside the check loop (collision renames, unregistered packs) are tallied
- Return DoctorSummary from DoctorRunner.run() and add regression tests

Author: bguidolim

Sources/mcs/Core/CLIOutput.swift

@@ -1,4 +1,26 @@
 import Foundation
+import os
+
+/// Shared, mutable tally of warnings emitted through a `CLIOutput`.
+///
+/// A reference type so that value-type `CLIOutput` copies (e.g. the one handed
+/// to `DestinationCollisionResolver`) all increment the same counter. Lets a
+/// caller like `DoctorRunner` faithfully count every warning shown — including
+/// advisories emitted outside the doctor check loop.
+///
+/// `Sendable` so `CLIOutput` stays `Sendable` (it's captured in isolated
+/// closures, e.g. via `ScriptRunner`); the lock supplies that guarantee.
+final class WarningCounter: Sendable {
+    private let lock = OSAllocatedUnfairLock(initialState: 0)
+
+    var count: Int {
+        lock.withLock { $0 }
+    }
+
+    func increment() {
+        lock.withLock { $0 += 1 }
+    }
+}
 
 /// Terminal output with ANSI color support and structured logging.
 struct CLIOutput {
@@ -10,8 +32,11 @@ struct CLIOutput {
     /// manipulation, ANSI ornamentation) can render. Gate pickers on this.
     let isInteractiveTerminal: Bool
     let style: ANSIStyle
+    /// Optional tally that `warn(_:)` increments. `nil` for most callers; set by
+    /// callers (e.g. `DoctorRunner`) that need to count emitted warnings.
+    let warningCounter: WarningCounter?
 
-    init(colorsEnabled: Bool? = nil) {
+    init(colorsEnabled: Bool? = nil, warningCounter: WarningCounter? = nil) {
         if let explicit = colorsEnabled {
             self.colorsEnabled = explicit
         } else {
@@ -20,6 +45,7 @@ struct CLIOutput {
         hasInteractiveStdin = isatty(STDIN_FILENO) != 0
         isInteractiveTerminal = hasInteractiveStdin && isatty(STDOUT_FILENO) != 0
         style = ANSIStyle(enabled: self.colorsEnabled)
+        self.warningCounter = warningCounter
     }
 
     // MARK: - ANSI Codes (delegate to `style`)
@@ -131,6 +157,7 @@ struct CLIOutput {
     }
 
     func warn(_ message: String) {
+        warningCounter?.increment()
         write("\(yellow)[WARN]\(reset) \(message)\n")
     }
 

Sources/mcs/Doctor/DoctorRunner.swift

@@ -1,5 +1,12 @@
 import Foundation
 
+/// Outcome tallies from a doctor run, captured at summary time.
+struct DoctorSummary {
+    let passed: Int
+    let warnings: Int
+    let issues: Int
+}
+
 /// Orchestrates all doctor checks grouped by section, with optional fix mode.
 ///
 /// **Scope of `--fix`**: Cleanup, migration, and trivial repairs only.
@@ -15,10 +22,13 @@ struct DoctorRunner {
     let globalOnly: Bool
     let registry: TechPackRegistry
 
-    private let output = CLIOutput()
+    /// Counts every warning emitted through `output`, including advisories shown
+    /// outside the check loop (collision renames, unregistered packs, unreadable
+    /// state) so the summary tally is faithful to what the user saw.
+    private let warningCounter = WarningCounter()
+    private let output: CLIOutput
     private var passCount = 0
     private var failCount = 0
-    private var warnCount = 0
     private var fixedCount = 0
     /// Failed checks collected during diagnosis, to be fixed after confirmation.
     private var pendingFixes: [any DoctorCheck] = []
@@ -60,9 +70,11 @@ struct DoctorRunner {
         self.registry = registry
         self.environment = environment
         self.projectRootOverride = projectRootOverride
+        output = CLIOutput(warningCounter: warningCounter)
     }
 
-    mutating func run() throws {
+    @discardableResult
+    mutating func run() throws -> DoctorSummary {
         output.header("Managed Claude Stack — Doctor")
 
         let env = environment
@@ -230,13 +242,19 @@ struct DoctorRunner {
             runChecks(checks)
         }
 
-        // Summary (before fixes, so the user sees the full picture first)
+        // Summary (before fixes, so the user sees the full picture first).
+        // Capture once: fix-phase warnings (below) must not alter the reported total.
+        let summary = DoctorSummary(
+            passed: passCount,
+            warnings: warningCounter.count,
+            issues: failCount
+        )
         output.header("Summary")
         output.doctorSummary(
-            passed: passCount,
+            passed: summary.passed,
             fixed: 0,
-            warnings: warnCount,
-            issues: failCount
+            warnings: summary.warnings,
+            issues: summary.issues
         )
 
         // Phase 2: Confirm and execute pending fixes (after summary)
@@ -247,6 +265,8 @@ struct DoctorRunner {
                 output.success("Applied \(fixedCount) fix\(fixedCount == 1 ? "" : "es").")
             }
         }
+
+        return summary
     }
 
     // MARK: - Scope resolution
@@ -605,7 +625,7 @@ struct DoctorRunner {
     }
 
     private mutating func docWarn(_ name: String, _ msg: String) {
-        warnCount += 1
+        // warningCounter increments inside output.warn — single source of truth.
         output.warn("⚠ \(name): \(msg)")
     }
 

Tests/MCSTests/DoctorRunnerIntegrationTests.swift

@@ -357,3 +357,106 @@ struct DoctorRunnerIntegrationTests {
         try runner.run()
     }
 }
+
+// MARK: - Summary Warning-Count Tests
+
+/// Regression coverage for the doctor summary warning tally. The count must
+/// include warnings emitted *outside* the check loop (collision renames,
+/// unregistered packs), which previously printed but were never counted.
+///
+/// Assertions are deltas: ambient checks (e.g. ProjectIndexCheck) may add
+/// warnings of their own, so each test compares two otherwise-identical runs
+/// that differ only by the single side-channel warning under test.
+struct DoctorSummaryWarningCountTests {
+    /// The injected counter is shared across CLIOutput copies, so a warning
+    /// emitted through any copy (e.g. the one handed to the collision resolver)
+    /// is tallied. This is the mechanism the doctor summary relies on.
+    @Test("WarningCounter is shared across CLIOutput value copies")
+    func warningCounterSharedAcrossCopies() {
+        let counter = WarningCounter()
+        let output = CLIOutput(colorsEnabled: false, warningCounter: counter)
+        let copy = output // value-type copy, same counter instance
+
+        #expect(counter.count == 0)
+        output.warn("first")
+        copy.warn("second")
+        #expect(counter.count == 2)
+    }
+
+    @Test("Skill colliding with a pre-existing unmanaged file is counted in the summary")
+    func collisionWarningCounted() throws {
+        let (home, project) = try makeSandboxProject(label: "warncount-collision")
+        defer { try? FileManager.default.removeItem(at: home) }
+
+        // A pack whose skill targets destination "my-skill".
+        let skillSource = home.appendingPathComponent("pack-my-skill")
+        try FileManager.default.createDirectory(at: skillSource, withIntermediateDirectories: true)
+        try "managed".write(
+            to: skillSource.appendingPathComponent("SKILL.md"), atomically: true, encoding: .utf8
+        )
+        let component = ComponentDefinition(
+            id: "test-pack.my-skill",
+            displayName: "my-skill",
+            description: "Skill",
+            type: .skill,
+            packIdentifier: "test-pack",
+            dependencies: [],
+            isRequired: true,
+            installAction: .copyPackFile(source: skillSource, destination: "my-skill", fileType: .skill)
+        )
+        let pack = MockTechPack(
+            identifier: "test-pack", displayName: "Test Pack", components: [component]
+        )
+        let registry = TechPackRegistry(packs: [pack])
+
+        // Configure the pack (no artifacts → nothing tracked at the destination).
+        var state = try ProjectState(projectRoot: project)
+        state.recordPack("test-pack")
+        try state.save()
+
+        // Baseline: no pre-existing file at the destination → no collision.
+        var baselineRunner = makeRunner(home: home, projectRoot: project, registry: registry)
+        let baseline = try baselineRunner.run()
+
+        // Now plant a pre-existing UNMANAGED skill at the same destination.
+        let existingSkill = project.appendingPathComponent(".claude/skills/my-skill")
+        try FileManager.default.createDirectory(at: existingSkill, withIntermediateDirectories: true)
+        try "user content".write(
+            to: existingSkill.appendingPathComponent("SKILL.md"), atomically: true, encoding: .utf8
+        )
+
+        var collisionRunner = makeRunner(home: home, projectRoot: project, registry: registry)
+        let withCollision = try collisionRunner.run()
+
+        // The only difference between the two runs is the collision warning.
+        #expect(withCollision.warnings == baseline.warnings + 1)
+    }
+
+    @Test("Unregistered --pack filter warning is counted in the summary")
+    func unregisteredPackWarningCounted() throws {
+        let (home, project) = try makeSandboxProject(label: "warncount-unregistered")
+        defer { try? FileManager.default.removeItem(at: home) }
+
+        let pack = MockTechPack(identifier: "test-pack", displayName: "Test Pack")
+        let registry = TechPackRegistry(packs: [pack])
+
+        var state = try ProjectState(projectRoot: project)
+        state.recordPack("test-pack")
+        try state.save()
+
+        // Baseline: filter to the registered pack only.
+        var baselineRunner = makeRunner(
+            home: home, projectRoot: project, registry: registry, packFilter: "test-pack"
+        )
+        let baseline = try baselineRunner.run()
+
+        // Add an unregistered pack id to the filter — same checks, plus one
+        // "not registered" advisory warning.
+        var ghostRunner = makeRunner(
+            home: home, projectRoot: project, registry: registry, packFilter: "test-pack,ghost-pack"
+        )
+        let withGhost = try ghostRunner.run()
+
+        #expect(withGhost.warnings == baseline.warnings + 1)
+    }
+}