diff --git a/CHANGELOG.md b/CHANGELOG.md index b02b925dd7..417a48f9c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Startup: load persisted plan-utilization history away from the main thread so mature histories no longer delay app launch. Thanks @Yuxin-Qiao! - Provider cleanup: prevent in-flight usage, status, token-cost, and cached-hydration work from republishing stale state after a provider is disabled, unavailable, or re-enabled. Thanks @Yuxin-Qiao! - Agent Sessions: coalesce overlapping unchanged remote refresh requests so menu opens do not repeat Tailscale discovery and SSH passes. Thanks @Yuxin-Qiao! +- Agent Sessions: keep Tailscale discovery headless and fall through across installed CLI variants, preventing repeated Tailscale menu-bar launches. Thanks @willsarg! - Cost usage: zero the scanner's 60-second refresh debounce on app-driven fetches so non-forced refreshes (hourly timer, post-launch, scope/settings changes) reflect rows appended between fetches instead of serving a stale snapshot that `UsageStore.tokenFetchTTL` then pins for up to an hour (#2089). Thanks @Yuxin-Qiao! - Codex cost usage: contain interleaved cumulative counters from Ultra-mode fork lineages so repeated lineage switches cannot inflate token and cost history (#2037). Thanks @Zihao-Qi! diff --git a/Sources/CodexBarCore/RemoteSessionFetcher.swift b/Sources/CodexBarCore/RemoteSessionFetcher.swift index b14e8cbf80..e69c23db28 100644 --- a/Sources/CodexBarCore/RemoteSessionFetcher.swift +++ b/Sources/CodexBarCore/RemoteSessionFetcher.swift @@ -21,16 +21,46 @@ public struct RemoteSessionHostResult: Equatable, Sendable, Identifiable { } public enum TailscaleStatusParser { - public static func hosts(from data: Data, excludingLocalHost localHost: String? = nil) -> [String] { - guard let root = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { return [] } - let peers: [[String: Any]] = if let dictionary = root["Peer"] as? [String: [String: Any]] { - Array(dictionary.values) - } else if let array = root["Peer"] as? [[String: Any]] { - array + /// Parses hosts from `tailscale status --json` output. + /// + /// Returns `nil` when `data` is not recognizable Tailscale status JSON — a failed, wrong, or + /// non-Tailscale `tailscale` binary — so callers can fall through to the next candidate. Returns a + /// possibly-empty list for a valid status that simply has no eligible peers (a real answer, stop). + package static func parseHosts(from data: Data, excludingLocalHost localHost: String? = nil) -> [String]? { + guard let root = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { return nil } + if let rawBackendState = root["BackendState"] { + guard let backendState = rawBackendState as? String, + backendState.caseInsensitiveCompare("Running") == .orderedSame + else { return nil } + } + + let selfStatus: [String: Any]? + if let rawSelf = root["Self"] { + guard let parsedSelf = rawSelf as? [String: Any] else { return nil } + selfStatus = parsedSelf } else { - [] + selfStatus = nil + } + + let peers: [[String: Any]] + let hasPeerShape: Bool + switch root["Peer"] { + case let dictionary as [String: [String: Any]]: + peers = Array(dictionary.values) + hasPeerShape = true + case let array as [[String: Any]]: + peers = array + hasPeerShape = true + case is NSNull: + peers = [] + hasPeerShape = true + case nil: + peers = [] + hasPeerShape = false + default: + return nil } - let selfStatus = root["Self"] as? [String: Any] + guard selfStatus != nil || hasPeerShape else { return nil } let localLabels = Set([ localHost, selfStatus?["DNSName"] as? String, @@ -50,6 +80,12 @@ public enum TailscaleStatusParser { }.sorted() } + /// Convenience returning `[]` for unparseable output. Prefer `parseHosts` when the caller needs to + /// distinguish a failed probe from an empty tailnet. + public static func hosts(from data: Data, excludingLocalHost localHost: String? = nil) -> [String] { + self.parseHosts(from: data, excludingLocalHost: localHost) ?? [] + } + private static func firstDNSLabel(_ value: String?) -> String? { guard let value else { return nil } let trimmed = value.trimmingCharacters(in: CharacterSet(charactersIn: ".")) @@ -67,16 +103,38 @@ public struct RemoteSessionFetcher: Sendable { environment: [String: String] = ProcessInfo.processInfo.environment, localHost: String = ProcessInfo.processInfo.hostName) async -> [String] { - guard let tailscale = self.tailscaleBinary(environment: environment), - let result = try? await SubprocessRunner.run( - binary: tailscale, - arguments: ["status", "--json"], - environment: environment, - timeout: 5, - label: "Tailscale session host discovery"), - let data = result.stdout.data(using: .utf8) - else { return [] } - return TailscaleStatusParser.hosts(from: data, excludingLocalHost: localHost) + let probeEnvironment = Self.tailscaleCLIEnvironment(from: environment) + let candidates = Self.tailscaleBinaryCandidates(path: environment["PATH"]) + .filter { FileManager.default.isExecutableFile(atPath: $0) } + return await Self.firstDiscoveredHosts(candidates: candidates, localHost: localHost) { binary in + guard let result = try? await SubprocessRunner.run( + binary: binary, + arguments: ["status", "--json"], + environment: probeEnvironment, + timeout: 5, + label: "Tailscale session host discovery") + else { return nil } + return Data(result.stdout.utf8) + } + } + + /// Runs `tailscale status --json` on each candidate in order, falling through to the next when a + /// candidate fails (`run` returns nil), returns invalid status JSON, or reports an inactive backend. + /// Returns the first candidate's parsed hosts (possibly empty), or `[]` if none succeed. This keeps + /// the app-binary fallback working even when an earlier — but non-functional — `tailscale` variant + /// is installed (e.g. an open-source/Homebrew CLI that isn't the active client). + package static func firstDiscoveredHosts( + candidates: [String], + localHost: String?, + run: (String) async -> Data?) async -> [String] + { + for binary in candidates { + guard let data = await run(binary), + let hosts = TailscaleStatusParser.parseHosts(from: data, excludingLocalHost: localHost) + else { continue } + return hosts + } + return [] } public func fetch( @@ -154,11 +212,42 @@ public struct RemoteSessionFetcher: Sendable { } } - private func tailscaleBinary(environment: [String: String]) -> String? { - self.findExecutable("tailscale", environment: environment) ?? { - let bundled = "/Applications/Tailscale.app/Contents/MacOS/Tailscale" - return FileManager.default.isExecutableFile(atPath: bundled) ? bundled : nil - }() + /// Ordered candidate paths for the `tailscale` CLI, most-preferred first. + /// + /// The macOS app ships its CLI as a thin `/bin/sh` wrapper (usually + /// `/usr/local/bin/tailscale`) around the app's dual-mode binary. We prefer the + /// wrapper, but a GUI-launched CodexBar inherits a minimal `PATH` (`/usr/bin:/bin`) + /// that omits the standard CLI locations, so we also probe them explicitly before + /// falling back to the app binary itself. + package static func tailscaleBinaryCandidates(path: String?) -> [String] { + let pathDirs = path?.split(separator: ":").map(String.init) ?? [] + var seen = Set() + var candidates = (pathDirs + ["/usr/local/bin", "/opt/homebrew/bin"]) + .filter { seen.insert($0).inserted } + .map { $0 + "/tailscale" } + // Last resort: the dual-mode app binary. Must be run via + // `tailscaleCLIEnvironment(from:)` so it stays in CLI mode. + candidates.append("/Applications/Tailscale.app/Contents/MacOS/Tailscale") + return candidates + } + + /// Environment that keeps the dual-mode Tailscale app binary in CLI mode. + /// + /// With no shell/terminal marker present the binary boots the full menu-bar GUI + /// (SkyLight/WindowServer, status icon) instead of running the CLI: it never emits + /// JSON, the probe times out, and the Tailscale icon flickers on every refresh. A + /// set `TERM` or `SHLVL` forces CLI mode (argv[0] casing and `XPC_SERVICE_NAME` do + /// not). `SHLVL` is what the app's own `/bin/sh` CLI wrapper injects, so we mirror it here. + /// + /// Applied to every probe, not just the app-binary fallback: it is redundant but harmless for the + /// CLI wrapper (itself a `/bin/sh` script that already exports `SHLVL`), and injecting it + /// unconditionally keeps CLI mode guaranteed regardless of which binary `tailscaleBinary` resolves. + /// An existing `TERM`/`SHLVL` (real terminal context) is left untouched. + package static func tailscaleCLIEnvironment(from environment: [String: String]) -> [String: String] { + guard environment["TERM"] == nil, environment["SHLVL"] == nil else { return environment } + var environment = environment + environment["SHLVL"] = "1" + return environment } private func findExecutable(_ name: String, environment: [String: String]) -> String? { diff --git a/Tests/CodexBarTests/TailscaleSessionTests.swift b/Tests/CodexBarTests/TailscaleSessionTests.swift index be91d66c4d..7cb4eba3e7 100644 --- a/Tests/CodexBarTests/TailscaleSessionTests.swift +++ b/Tests/CodexBarTests/TailscaleSessionTests.swift @@ -13,6 +13,112 @@ struct TailscaleSessionTests { #expect(hosts == ["clawmac", "linuxbox"]) } + @Test + func `binary candidates prefer the CLI wrapper over the app binary`() throws { + // A GUI-launched app inherits a minimal PATH that omits the CLI locations. + let candidates = RemoteSessionFetcher.tailscaleBinaryCandidates(path: "/usr/bin:/bin") + + // The standard wrapper locations are still probed, ahead of the app binary… + #expect(candidates.contains("/usr/local/bin/tailscale")) + #expect(candidates.contains("/opt/homebrew/bin/tailscale")) + // …and the dual-mode app binary is the last resort. + #expect(candidates.last == "/Applications/Tailscale.app/Contents/MacOS/Tailscale") + #expect(try #require(candidates.firstIndex(of: "/usr/local/bin/tailscale")) < candidates.count - 1) + } + + @Test + func `binary candidates keep PATH entries first and dedupe well-known dirs`() { + let candidates = RemoteSessionFetcher.tailscaleBinaryCandidates(path: "/opt/homebrew/bin:/usr/bin") + + #expect(candidates.first == "/opt/homebrew/bin/tailscale") + #expect(candidates.count(where: { $0 == "/opt/homebrew/bin/tailscale" }) == 1) + } + + @Test + func `cli environment injects a shell marker for the app-binary fallback`() { + // Without a marker the dual-mode binary launches the GUI instead of the CLI. + let env = RemoteSessionFetcher.tailscaleCLIEnvironment(from: ["PATH": "/usr/bin"]) + + #expect(env["SHLVL"] == "1") + } + + @Test + func `cli environment preserves an existing terminal context`() { + // Already CLI-safe: leave TERM alone and don't fabricate a SHLVL… + let withTerm = RemoteSessionFetcher.tailscaleCLIEnvironment(from: ["TERM": "xterm-256color"]) + #expect(withTerm["SHLVL"] == nil) + + // …and never clobber a caller-provided SHLVL. + let withShlvl = RemoteSessionFetcher.tailscaleCLIEnvironment(from: ["SHLVL": "3"]) + #expect(withShlvl["SHLVL"] == "3") + } + + @Test + func `discovery falls through to the next candidate when the first fails`() async { + // First candidate exists but is a wrong/broken tailscale variant: its status output isn't valid + // Tailscale JSON. Discovery must try the next candidate rather than returning no hosts. + let validStatus = Data(#""" + {"Version":"1.0","Self":{"HostName":"local-mac"}, + "Peer":{"n":{"Online":true,"OS":"linux","DNSName":"linuxbox.tail.ts.net."}}} + """#.utf8) + var probed: [String] = [] + let hosts = await RemoteSessionFetcher.firstDiscoveredHosts( + candidates: ["/first/tailscale", "/second/tailscale"], + localHost: "local-mac") + { binary in + probed.append(binary) + return binary == "/first/tailscale" ? Data(#"{"Version":"1.0"}"#.utf8) : validStatus + } + + #expect(hosts == ["linuxbox"]) + #expect(probed == ["/first/tailscale", "/second/tailscale"]) // fell through, in order + } + + @Test + func `discovery falls through when an earlier candidate needs login`() async { + let inactiveStatus = Data(#"{"Version":"1.0","BackendState":"NeedsLogin","Peer":null}"#.utf8) + let runningStatus = Data(#""" + {"Version":"1.0","BackendState":"Running","Self":{"HostName":"local-mac"}, + "Peer":{"n":{"Online":true,"OS":"linux","DNSName":"linuxbox.tail.ts.net."}}} + """#.utf8) + var probed: [String] = [] + let hosts = await RemoteSessionFetcher.firstDiscoveredHosts( + candidates: ["/inactive/tailscale", "/running/tailscale"], + localHost: "local-mac") + { binary in + probed.append(binary) + return binary == "/inactive/tailscale" ? inactiveStatus : runningStatus + } + + #expect(hosts == ["linuxbox"]) + #expect(probed == ["/inactive/tailscale", "/running/tailscale"]) + } + + @Test + func `discovery returns empty when no candidate yields a valid status`() async { + let hosts = await RemoteSessionFetcher.firstDiscoveredHosts( + candidates: ["/a/tailscale", "/b/tailscale"], + localHost: nil) { _ in Data("nope".utf8) } + + #expect(hosts.isEmpty) + } + + @Test + func `parseHosts distinguishes invalid output from an empty tailnet`() { + // Non-status output -> nil so the caller falls through to the next candidate… + #expect(TailscaleStatusParser.parseHosts(from: Data("not json".utf8)) == nil) + #expect(TailscaleStatusParser.parseHosts(from: Data("Tailscale help text".utf8)) == nil) + #expect(TailscaleStatusParser.parseHosts(from: Data(#"{"Version":"1.0"}"#.utf8)) == nil) + #expect(TailscaleStatusParser.parseHosts(from: Data(#"{"Self":null}"#.utf8)) == nil) + #expect(TailscaleStatusParser.parseHosts(from: Data(#"{"Peer":"error"}"#.utf8)) == nil) + // …a valid status with no eligible peers -> [] (a real answer, stop probing). + let empty = TailscaleStatusParser.parseHosts( + from: Data(#"{"Version":"1.0","BackendState":"Running","Self":{},"Peer":null}"#.utf8)) + #expect(empty == []) + #expect(TailscaleStatusParser.parseHosts( + from: Data(#"{"Version":"1.0","BackendState":"NeedsLogin","Peer":null}"#.utf8)) == nil) + } + @Test func `ssh destinations reject options whitespace and controls`() { let hosts = RemoteSessionFetcher.sanitizedHosts([