Merge origin/main into codex/telemetry-backend

Author: axAilotl

desktop/context-for-claude/Sources/ContextApp/Integration/ClaudeConnection.swift

@@ -0,0 +1,88 @@
+import AppKit
+import ContextCore
+import Darwin
+import Foundation
+
+/// The Claude connection **as the user experiences it**: what is registered on disk, and whether the
+/// Claude Desktop running right now can actually reach us.
+///
+/// ## Why the disk answer alone is a lie
+///
+/// `ClaudeRegistrar.status()` reads two config files. That is the whole of what every user-facing
+/// surface knew, so both of them said `Connected to Claude Code and Claude Desktop` whenever the
+/// entry was on disk — including for the three days this Mac spent with a Claude Desktop whose
+/// server had failed to spawn at every launch. The config was perfect. The connector was dead. The
+/// app said "Connected", the menu bar said "Connected", and the only place the truth existed was a
+/// line in `~/Library/Logs/Claude/mcp-server-context-for-claude.log` that nothing points a user at.
+///
+/// `ClaudeServerLiveness` has been able to answer this since it was written — it looks for a live
+/// server descending from a running Claude Desktop — but it was reachable only from the tutorial,
+/// which runs once. This type is what puts that evidence in front of the two surfaces a user
+/// actually consults, so a registration that Claude has not picked up can no longer read as success.
+///
+/// ## What it does not do
+///
+/// It does not quit anybody's Claude. `ClaudeHandoff` may offer that, with consent, because it is
+/// about to hand Claude a question and the answer would be wrong without it; a status line has no
+/// such errand and no right to take a conversation away to tidy up its own wording. So the remedy
+/// here is a sentence, and the press stays the user's.
+struct ClaudeConnection: Equatable {
+    /// Registered on disk, pointing at this build's binary.
+    let claudeCode: Bool
+    let claudeDesktop: Bool
+
+    /// Registered for Claude Desktop, and Claude Desktop is demonstrably not serving us.
+    ///
+    /// **Only ever true on evidence.** `.unknown` — Claude Desktop is not running, or the process
+    /// list could not be read — leaves this false, because a surface that nags on absence of
+    /// evidence would tell users to restart an app that is working, or one that is not even open.
+    let desktopNeedsRestart: Bool
+
+    /// The sentence naming the remedy, or nil when there is nothing to remedy. Shared so the menu
+    /// bar and the settings pane cannot drift into describing the same state two different ways.
+    var restartNotice: String? {
+        desktopNeedsRestart ? "Quit and reopen Claude Desktop to finish connecting it." : nil
+    }
+
+    /// Whether Claude can reach this Mac through Claude Desktop *now*, as opposed to after a restart.
+    /// This, not `claudeDesktop`, is what a summary line may call connected.
+    var desktopIsReachable: Bool { claudeDesktop && !desktopNeedsRestart }
+
+    /// Pure, so the whole decision is testable without a Claude on the machine.
+    init(claudeCode: Bool, claudeDesktop: Bool, liveness: ClaudeServerLiveness.State) {
+        self.claudeCode = claudeCode
+        self.claudeDesktop = claudeDesktop
+        self.desktopNeedsRestart = claudeDesktop && liveness == .notServingClaudeDesktop
+    }
+
+    /// Reads both config files and the process list. Off the main actor at every call site: the
+    /// config half JSON-decodes `~/.claude.json`, and the liveness half sweeps every PID on the Mac.
+    static func current() -> ClaudeConnection {
+        let registration = ClaudeRegistrar.status()
+        return ClaudeConnection(
+            claudeCode: registration.claudeCode,
+            claudeDesktop: registration.claudeDesktop,
+            liveness: ClaudeServerLiveness.state(claudeDesktopPIDs: ClaudeDesktopProcesses.pids))
+    }
+}
+
+// MARK: - The running Claude Desktops
+
+/// Claude Desktop, as processes.
+///
+/// Separate from `ClaudeRegistrar` because that file is deliberately free of AppKit — its comment on
+/// `ClaudeServerLiveness.state` says so, and the PIDs are a parameter for exactly that reason. This
+/// is the one place that reads them, so the three callers that used to build the same
+/// `runningApplications(withBundleIdentifier:)` call by hand now share it and cannot disagree about
+/// which bundle identifier Claude Desktop has.
+enum ClaudeDesktopProcesses {
+    static let bundleIdentifier = "com.anthropic.claudefordesktop"
+
+    /// Empty when Claude Desktop is not running, which `ClaudeServerLiveness` reads as `.unknown`
+    /// rather than as an absence — there is nothing for a server to be serving.
+    static var pids: Set<pid_t> {
+        Set(
+            NSRunningApplication.runningApplications(withBundleIdentifier: bundleIdentifier)
+                .map(\.processIdentifier))
+    }
+}

desktop/context-for-claude/Sources/ContextApp/Integration/ClaudeRegistrar.swift

@@ -477,9 +477,10 @@ enum ClaudeServerLiveness {
     /// fails outright on a buffer below it.
     private static let executablePathCapacity = 4 * 1_024
 
-    /// How far up a process tree to look for Claude. The real chain is two hops
-    /// (`context-for-claude-mcp` → `Claude.app/Contents/Helpers/disclaimer` → `Claude`); the bound is
-    /// what stops a cyclic or corrupted parent chain from spinning this loop forever.
+    /// How far up a process tree to look for Claude. The chains are short — two hops to Claude
+    /// Desktop (`context-for-claude-mcp` → `Claude.app/Contents/Helpers/disclaimer` → `Claude`), one
+    /// to a Claude Code that is running inside it; the bound is what stops a cyclic or corrupted
+    /// parent chain from spinning this loop forever.
     private static let maximumAncestorHops = 8
 
     /// - Parameter claudeDesktopPIDs: the running Claude Desktop processes, passed in rather than
@@ -495,14 +496,94 @@ enum ClaudeServerLiveness {
         guard !servers.isEmpty else { return .notServingClaudeDesktop }
 
         // Attribution matters because Claude Code spawns this same binary, and one of *its* servers
-        // must never be read as proof that Claude Desktop has one. A chain that cannot be resolved
-        // counts as Claude's, because the cost of being wrong is asymmetric: the only thing this
-        // state gates is an offer to quit somebody's Claude, and guessing "not Claude's" would take
-        // an open conversation on the strength of a parent lookup that failed.
-        let servesClaude = servers.contains { descends($0, from: claudeDesktopPIDs) ?? true }
+        // must never be read as proof that Claude Desktop has one.
+        let servesClaude = servers.contains { server in
+            switch owner(
+                of: server,
+                claudeDesktopPIDs: claudeDesktopPIDs,
+                parent: parentPID(of:),
+                executablePath: executablePath(of:))
+            {
+            // A chain that cannot be resolved counts as Claude's, because the cost of being wrong is
+            // asymmetric in both directions this state is read: it gates an offer to quit somebody's
+            // Claude, and it gates a status line that would otherwise nag. Guessing "not Claude's"
+            // on a parent lookup that failed would do both on no evidence at all.
+            case .claudeDesktop, .unknown: return true
+            case .claudeCode, .none: return false
+            }
+        }
         return servesClaude ? .servingClaudeDesktop : .notServingClaudeDesktop
     }
 
+    /// Who a server belongs to, decided by the **nearest** owner above it rather than by whether
+    /// Claude Desktop appears anywhere on the chain.
+    ///
+    /// **The distinction is the whole of it, because Claude Code now runs inside Claude Desktop.**
+    /// A Claude Code session opened in the desktop app has this ancestry, read off this Mac:
+    ///
+    /// ```
+    /// context-for-claude-mcp
+    ///   → …/Application Support/Claude/claude-code/2.1.229/claude.app/Contents/MacOS/claude
+    ///     → /Applications/Claude.app/Contents/Helpers/disclaimer
+    ///       → /Applications/Claude.app/Contents/MacOS/Claude     ← a Claude Desktop PID
+    /// ```
+    ///
+    /// A walk that only asks "is a Claude Desktop PID an ancestor" answers yes for every one of
+    /// those, so on any Mac where the user has a Claude Code session open in the desktop app, one of
+    /// *Claude Code's* servers is read as proof that Claude Desktop has one. Measured on the Mac
+    /// whose Claude Desktop connector had been failing to spawn for three days: every disk check
+    /// said connected, and this probe — the one thing that could have contradicted them — agreed,
+    /// because fifteen Claude Code servers were descendants of the same process.
+    ///
+    /// So the first owner met wins. A `claude-code` process between the server and Claude Desktop
+    /// means the server is Claude Code's, and Claude Desktop's own spawn is not on this chain at all.
+    ///
+    /// Pure, with the process table passed in as two lookups, because the tree it has to reason about
+    /// cannot be built inside a test — the real one needs a running Claude Desktop, a running Claude
+    /// Code inside it, and a server under each.
+    enum Owner: Equatable {
+        /// A Claude Code process sits between the server and anything else.
+        case claudeCode
+        /// A running Claude Desktop was reached first.
+        case claudeDesktop
+        /// The walk finished at launchd having met neither.
+        case none
+        /// The walk ran out of parents it could read. Not an answer, and never acted on.
+        case unknown
+    }
+
+    /// Claude Code's own install directory, which every copy of it the desktop app runs sits inside:
+    /// `~/Library/Application Support/Claude/claude-code/<version>/claude.app/…`.
+    ///
+    /// A path marker rather than an executable name because both binaries are called some case of
+    /// "claude", and a case-insensitive name test would classify Claude Desktop itself as Claude
+    /// Code. A Claude Code installed elsewhere — a CLI on `PATH`, say — is not matched and does not
+    /// need to be: it is not a descendant of Claude Desktop, so it was never a false positive here.
+    static let claudeCodePathMarker = "/claude-code/"
+
+    static func owner(
+        of pid: pid_t,
+        claudeDesktopPIDs: Set<pid_t>,
+        parent: (pid_t) -> pid_t?,
+        executablePath: (pid_t) -> String?
+    ) -> Owner {
+        var current = pid
+        for _ in 0..<maximumAncestorHops {
+            guard let ancestor = parent(current) else { return .unknown }
+            // **The line the old walk did not have.** Without it the loop below is the shipped
+            // behaviour — "is a Claude Desktop PID anywhere above this server" — and every Claude
+            // Code session inside the desktop app answers yes. Order between the two tests is
+            // immaterial (no process is both); presence of this one is the whole fix.
+            if executablePath(ancestor)?.contains(claudeCodePathMarker) == true { return .claudeCode }
+            if claudeDesktopPIDs.contains(ancestor) { return .claudeDesktop }
+            // launchd (1) and the kernel (0) top every tree: the walk finished, and neither owner
+            // was anywhere on it.
+            if ancestor <= 1 { return .none }
+            current = ancestor
+        }
+        return .unknown
+    }
+
     /// Every live process whose executable is exactly `binary`, or nil when the process list could
     /// not be read at all.
     ///
@@ -530,21 +611,6 @@ enum ClaudeServerLiveness {
         return String(cString: buffer)
     }
 
-    /// Walks up from `pid` looking for one of `ancestors`. Nil means the walk ran out of parents it
-    /// could read before reaching an answer — "I could not tell", which is not "no".
-    private static func descends(_ pid: pid_t, from ancestors: Set<pid_t>) -> Bool? {
-        var current = pid
-        for _ in 0..<maximumAncestorHops {
-            guard let parent = parentPID(of: current) else { return nil }
-            if ancestors.contains(parent) { return true }
-            // launchd (1) and the kernel (0) top every tree: the walk finished, and Claude was not
-            // anywhere on it.
-            if parent <= 1 { return false }
-            current = parent
-        }
-        return nil
-    }
-
     private static func parentPID(of pid: pid_t) -> pid_t? {
         var info = proc_bsdinfo()
         let size = Int32(MemoryLayout<proc_bsdinfo>.size)

desktop/context-for-claude/Sources/ContextApp/MenuBar/MenuBarPresentation.swift

@@ -201,16 +201,28 @@ struct ClaudeConnectorLine: Equatable {
     /// rather than a second control, which is what stops a second press starting a second write.
     var action: String?
 
-    init(claudeCode: Bool, claudeDesktop: Bool, note: String?, isConnecting: Bool) {
-        switch (claudeCode, claudeDesktop) {
+    init(connection: ClaudeConnection, note: String?, isConnecting: Bool) {
+        // **`desktopIsReachable`, not `claudeDesktop`.** The line reports what Claude can do, and a
+        // Claude Desktop that has not read the registration yet cannot do anything — saying
+        // "Connected to Claude Desktop" over a connector that fails to answer is the exact claim
+        // `ClaudeConnection` exists to stop this surface making.
+        switch (connection.claudeCode, connection.desktopIsReachable) {
         case (true, true): summary = "Connected to Claude Code and Claude Desktop"
         case (true, false): summary = "Connected to Claude Code"
         case (false, true): summary = "Connected to Claude Desktop"
         case (false, false): summary = "Not connected to Claude"
         }
-        isConnected = claudeCode || claudeDesktop
-        self.note = note
-        action = isConnected ? nil : (isConnecting ? "Connecting…" : "Connect")
+        isConnected = connection.claudeCode || connection.desktopIsReachable
+        // The remedy takes the note slot only when there is no fresher one: a sentence describing
+        // the write the user just asked for is about the press they just made, and outranks a
+        // standing condition they can act on whenever they like.
+        self.note = note ?? connection.restartNotice
+        // **Offered only when nothing is registered at all.** `Connect` writes the config files, so
+        // in the needs-restart state it would rewrite two files that are already correct and change
+        // nothing the user can see — a control that answers a real complaint with a no-op is worse
+        // than no control. The sentence in `note` names what actually works.
+        let hasNothingRegistered = !connection.claudeCode && !connection.claudeDesktop
+        action = hasNothingRegistered ? (isConnecting ? "Connecting…" : "Connect") : nil
     }
 }
 

desktop/context-for-claude/Sources/ContextApp/MenuBar/StatusView.swift

@@ -42,7 +42,10 @@ struct StatusView: View {
     @ObservedObject private var auth = OmiAuth.shared
     @ObservedObject private var uploads = ConversationUploader.shared
 
-    @State private var claude: (claudeCode: Bool, claudeDesktop: Bool) = (false, false)
+    /// What is registered on disk *and* whether the running Claude Desktop is serving us. The
+    /// default is the honest one for "not probed yet": nothing connected, nothing to restart.
+    @State private var claude = ClaudeConnection(
+        claudeCode: false, claudeDesktop: false, liveness: .unknown)
     @State private var claudeNote: String?
     /// True while the two config files are being rewritten. A second press cannot start a second
     /// write, which is the same rule the account line's round trip follows.
@@ -512,11 +515,7 @@ struct StatusView: View {
     /// The Claude line as a value, for the reason `account` is one: this view keeps no judgement of
     /// its own about a state it cannot be driven through in a test.
     private var connector: ClaudeConnectorLine {
-        ClaudeConnectorLine(
-            claudeCode: claude.claudeCode,
-            claudeDesktop: claude.claudeDesktop,
-            note: claudeNote,
-            isConnecting: isConnecting)
+        ClaudeConnectorLine(connection: claude, note: claudeNote, isConnecting: isConnecting)
     }
 
     /// Same shape as `refresh()`, and for the same reason: `register()` reads, decodes and rewrites
@@ -528,7 +527,16 @@ struct StatusView: View {
         claudeNote = nil
         Task {
             let result = await Task.detached(priority: .userInitiated) { ClaudeRegistrar.register() }.value
-            claude = (result.claudeCode, result.claudeDesktop)
+            // Re-probed rather than built from `result`: registering writes the config, and Claude
+            // Desktop reads it at *its* launch, so the press that "connects" routinely leaves a
+            // Claude that still cannot answer. That is the state the user most needs to be told
+            // about, and it exists from the instant the write lands.
+            claude = await Task.detached(priority: .userInitiated) {
+                ClaudeConnection(
+                    claudeCode: result.claudeCode,
+                    claudeDesktop: result.claudeDesktop,
+                    liveness: ClaudeServerLiveness.state(claudeDesktopPIDs: ClaudeDesktopProcesses.pids))
+            }.value
             claudeNote = result.message
             isConnecting = false
         }
@@ -651,7 +659,7 @@ struct StatusView: View {
         readAskLedger()
         claudeNote = nil
         Task {
-            claude = await Task.detached(priority: .userInitiated) { ClaudeRegistrar.status() }.value
+            claude = await Task.detached(priority: .userInitiated) { ClaudeConnection.current() }.value
         }
     }
 }