diff --git a/CHANGELOG.md b/CHANGELOG.md index d8acfe1..dd8df07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +**#173 — parent-chain diagnostics polish (follow-up bundle from the #169 verify).** + +- **Changed** — the `--print-tcc-path` parent-chain walk now makes every early stop visible: hop-cap and cycle stops append a synthetic marker hop (`(chain truncated after 15 hops)` / `(cycle detected)`, carrying the pid the walk stopped at) instead of ending silently; `ps` rows with an empty `comm` keep their pid→ppid linkage as `(unknown)` instead of severing the chain one hop early; `ps` runs with `-ww` so long bundle paths are never width-clamped; non-UTF-8 `ps` output and non-zero exits now surface as `(parent chain unavailable: …)` reasons (with stderr's first line attached) instead of a silently empty chain. The NOTE wording no longer equates the parent chain with macOS's responsible process (it is an approximation), and Claude Desktop users are routed to the sqlite3 TCC query (this shell-invoked chain can never show the Desktop MCP context). 9 new tests including real-subprocess fixture tests for the decode/exit failure paths (`LiveParentChainSourceTests`). + **#169 — `--print-tcc-path` now prints its execution context (parent process chain) + a context-dependence warning.** - **Added** — the `--print-tcc-path` diagnostic output ends with a new "Execution context (parent process chain)" section: the binary's own pid/path marked `(this binary)`, then every ancestor up to launchd (pid 1), captured via a single `ps -A -o pid=,ppid=,comm=` snapshot walked in-memory (cycle-guarded, hop-capped at 15 — a real Claude Code session chain already spends 10 hops). A `NOTE:` warning follows, stating that the EventKit authorization status shown above reflects the CURRENT execution context (the responsible process), not an absolute property of the binary (#168) — to diagnose a specific host, run the command from within that host's environment. `ps` failure/timeout degrades to a visible `(parent chain unavailable: )` line; the rest of the output is unaffected. diff --git a/Sources/CheICalMCP/EventKit/ParentChainSource.swift b/Sources/CheICalMCP/EventKit/ParentChainSource.swift index a4967cd..d47bcb7 100644 --- a/Sources/CheICalMCP/EventKit/ParentChainSource.swift +++ b/Sources/CheICalMCP/EventKit/ParentChainSource.swift @@ -46,11 +46,14 @@ enum ParentChainWalker { guard !trimmed.isEmpty else { continue } // Split into at most 3 columns: pid, ppid, command-with-possible-spaces. let columns = trimmed.split(separator: " ", maxSplits: 2, omittingEmptySubsequences: true) - guard columns.count == 3, + guard columns.count >= 2, let pid = Int32(columns[0]), let ppid = Int32(columns[1]) else { continue } - table[pid] = ProcessEntry(ppid: ppid, command: String(columns[2])) + // Empty comm (#173): keep the row with a visible placeholder — dropping it + // would sever the ppid linkage and end the walk one hop early. + let command = columns.count == 3 ? String(columns[2]) : "(unknown)" + table[pid] = ProcessEntry(ppid: ppid, command: command) } return table } @@ -58,7 +61,10 @@ enum ParentChainWalker { /// Walk from `startPid` toward launchd (pid 1). Termination is guaranteed by three /// guards: a seen-set (kills cycles), a hop cap (kills oversized/corrupt chains), and /// the unknown-pid stop (a pid missing from the table renders as `(unknown)` and ends - /// the walk — its ppid is unknowable). + /// the walk — its ppid is unknowable). Cycle and hop-cap stops append a synthetic + /// marker hop (#173) — a silently-ended chain reads as complete, which for a + /// diagnostic that exists to show the real context is misinformation. The marker's + /// pid is the pid the walk stopped at, so a triager can resume by hand. static func walk( table: [Int32: ProcessEntry], from startPid: Int32, @@ -68,7 +74,20 @@ enum ParentChainWalker { var hops: [ChainHop] = [] var seen: Set = [] var pid = startPid - while hops.count < maxHops, pid > 0, !seen.contains(pid) { + while pid > 0 { + if seen.contains(pid) { + hops.append(ChainHop(pid: pid, command: "(cycle detected)")) + break + } + // The terminal sentinel (pid 1, launchd) is exempt from the cap: it ends the + // walk unconditionally one line below, so admitting it costs at most one + // entry — while marking an already-rooted chain "truncated" would be the + // mirror image of the misinformation this marker exists to eliminate + // (#173 verify LOW-1, DA probe-confirmed). + if hops.count >= maxHops, pid != 1 { + hops.append(ChainHop(pid: pid, command: "(chain truncated after \(maxHops) hops)")) + break + } seen.insert(pid) guard let entry = table[pid] else { hops.append(ChainHop(pid: pid, command: "(unknown)")) @@ -107,13 +126,21 @@ enum ParentChainFormatter { } } lines.append("") + // Wording precision (#173): the responsible process is assigned at spawn time and + // is NOT guaranteed to sit on the getppid chain — describe the chain as an + // approximation. And since this flag is shell-invoked, the chain can never show + // the Claude Desktop MCP context; route Desktop diagnosis to the sqlite3 query + // printed earlier in the same output. lines.append(""" NOTE: the authorization status above reflects the CURRENT execution context - (the responsible process in this chain), not an absolute property of this - binary. Two different binaries under the same host see the same status; the - same binary under different hosts can see different statuses (#168). - To diagnose a specific host (Claude Code / Claude Desktop / Terminal), + (the execution context this query ran under, approximated by the parent chain + above), not an absolute property of this binary. Two different binaries under + the same host see the same status; the same binary under different hosts can + see different statuses (#168). + To diagnose a specific host (Claude Code / Terminal), run this command from within that host's environment. + For Claude Desktop, this shell-invoked chain cannot show its MCP context — + inspect the TCC database directly instead (sqlite3 command above). """) return lines.joined(separator: "\n") } @@ -141,7 +168,10 @@ struct LiveParentChainSource: ParentChainSource { let process = Process() process.executableURL = URL(fileURLWithPath: psPath) - process.arguments = ["-A", "-o", "pid=,ppid=,comm="] + // `-ww` (#173): lift the output-width clamp so long bundle paths are never + // truncated by ps itself (BSD ps applies the clamp even without a tty in + // some configurations; the flag is a no-op when already unclamped). + process.arguments = ["-Aww", "-o", "pid=,ppid=,comm="] let result: SubprocessRunResult do { @@ -153,10 +183,19 @@ struct LiveParentChainSource: ParentChainSource { return ParentChainResult(hops: [], failureReason: "ps timed out after \(timeoutMilliseconds)ms") } guard result.exitStatus == 0 else { - return ParentChainResult(hops: [], failureReason: "ps exited with status \(result.exitStatus)") + // Attach stderr's first line (#173) — it is the actionable part of a ps + // failure. Rendering escapes it (ParentChainFormatter), so raw bytes are safe. + let stderrFirstLine = String(data: result.stderrData, encoding: .utf8)? + .split(separator: "\n").first.map(String.init) ?? "" + let suffix = stderrFirstLine.isEmpty ? "" : ": \(stderrFirstLine)" + return ParentChainResult(hops: [], failureReason: "ps exited with status \(result.exitStatus)\(suffix)") } - let output = String(data: result.stdoutData, encoding: .utf8) ?? "" + // Non-UTF-8 output (#173): report it — a silent empty table would render as a + // "successful" one-hop chain (mirrors ProcessInventorySource's decode handling). + guard let output = String(data: result.stdoutData, encoding: .utf8) else { + return ParentChainResult(hops: [], failureReason: "ps output not UTF-8") + } let table = ParentChainWalker.parseProcessTable(output) return ParentChainResult( hops: ParentChainWalker.walk(table: table, from: startPid), diff --git a/Tests/CheICalMCPTests/LiveParentChainSourceTests.swift b/Tests/CheICalMCPTests/LiveParentChainSourceTests.swift new file mode 100644 index 0000000..f9923bf --- /dev/null +++ b/Tests/CheICalMCPTests/LiveParentChainSourceTests.swift @@ -0,0 +1,56 @@ +import XCTest +@testable import CheICalMCP + +/// Behavior tests for `LiveParentChainSource`'s failure paths (#173, logic F3/F4). +/// The `psPath` init parameter accepts any executable, so each test points it at a +/// small fixture script — real subprocess behavior, no mocks. +final class LiveParentChainSourceTests: XCTestCase { + + private var fixtures: [URL] = [] + + override func tearDown() { + for url in fixtures { try? FileManager.default.removeItem(at: url) } + fixtures = [] + super.tearDown() + } + + /// Write an executable shell script and return its path. + private func makeScript(_ body: String) throws -> String { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("fake-ps-\(UUID().uuidString).sh") + try ("#!/bin/sh\n" + body + "\n").write(to: url, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: url.path) + fixtures.append(url) + return url.path + } + + func testNonUTF8Output_reportsFailureReasonInsteadOfEmptyChain() throws { + // printf emits an invalid UTF-8 byte sequence (lone 0xFF continuation bytes). + let script = try makeScript(#"printf ' 500 400 /bin/\377\377zsh\n'"#) + let result = LiveParentChainSource(psPath: script).captureChain(from: 500) + XCTAssertTrue(result.hops.isEmpty) + XCTAssertEqual(result.failureReason, "ps output not UTF-8", + "silent empty-table success would misreport a decode failure as a clean run") + } + + func testNonZeroExit_reportsStatusAndStderrFirstLine() throws { + let script = try makeScript("echo 'ps: illegal option' >&2; exit 64") + let result = LiveParentChainSource(psPath: script).captureChain(from: 500) + XCTAssertTrue(result.hops.isEmpty) + let reason = try XCTUnwrap(result.failureReason) + XCTAssertTrue(reason.contains("64"), "exit status must be visible — got: \(reason)") + XCTAssertTrue(reason.contains("ps: illegal option"), + "stderr first line is the actionable part of a ps failure — got: \(reason)") + } + + func testHappyPath_stillWalksNormally() throws { + let script = try makeScript(""" + printf ' 500 400 /bin/zsh\\n' + printf ' 400 1 /sbin/launchd-ish\\n' + printf ' 1 0 /sbin/launchd\\n' + """) + let result = LiveParentChainSource(psPath: script).captureChain(from: 500) + XCTAssertNil(result.failureReason) + XCTAssertEqual(result.hops.map(\.pid), [500, 400, 1]) + } +} diff --git a/Tests/CheICalMCPTests/ParentChainFormatterTests.swift b/Tests/CheICalMCPTests/ParentChainFormatterTests.swift index 4121e07..d0d08b3 100644 --- a/Tests/CheICalMCPTests/ParentChainFormatterTests.swift +++ b/Tests/CheICalMCPTests/ParentChainFormatterTests.swift @@ -69,6 +69,30 @@ final class ParentChainFormatterTests: XCTestCase { XCTAssertTrue(s.contains("\\r"), "CR renders visibly so forged lines can't split") } + // MARK: - NOTE wording precision (#173, DA-2/DA-3) + + func testNote_usesApproximationWording_notResponsibleProcessClaim() { + // macOS's responsible process is not guaranteed to sit on the getppid chain — + // the NOTE must not flatten "parent chain" into "responsible process" (#168's + // own nuance). It describes the chain as an approximation instead. + let s = ParentChainFormatter.executionContextSection( + selfPid: 500, selfPath: selfPath, + result: ParentChainResult(hops: [.init(pid: 1, command: "/sbin/launchd")], failureReason: nil)) + XCTAssertFalse(s.contains("the responsible process in this chain")) + XCTAssertTrue(s.contains("approximated by the parent chain")) + } + + func testNote_pointsClaudeDesktopUsersAtTccDbQuery() { + // This command is shell-invoked, so the printed chain can never show the Claude + // Desktop MCP context (DA-2) — the NOTE must route Desktop diagnosis to the + // sqlite3 TCC query printed earlier in the same output. + let s = ParentChainFormatter.executionContextSection( + selfPid: 500, selfPath: selfPath, + result: ParentChainResult(hops: [], failureReason: nil)) + XCTAssertTrue(s.contains("Claude Desktop")) + XCTAssertTrue(s.contains("TCC database")) + } + func testEmptyChainWithoutFailure_stillRendersSelfAndWarning() { // ps succeeded but the table somehow lacked our ppid — degenerate but legal. let s = ParentChainFormatter.executionContextSection( diff --git a/Tests/CheICalMCPTests/ParentChainWalkTests.swift b/Tests/CheICalMCPTests/ParentChainWalkTests.swift index 0086c11..683c265 100644 --- a/Tests/CheICalMCPTests/ParentChainWalkTests.swift +++ b/Tests/CheICalMCPTests/ParentChainWalkTests.swift @@ -60,13 +60,16 @@ final class ParentChainWalkTests: XCTestCase { ]) } - func testWalk_cycle_terminatesWithoutRepeat() { + func testWalk_cycle_terminatesWithVisibleMarker() { + // #173: cycles must stop AND say so — a silently-ended chain reads as complete, + // which for a diagnostic that exists to show the real context is misinformation. let table: [Int32: ParentChainWalker.ProcessEntry] = [ 500: .init(ppid: 400, command: "/a"), 400: .init(ppid: 500, command: "/b"), ] let chain = ParentChainWalker.walk(table: table, from: 500) - XCTAssertEqual(chain.map(\.pid), [500, 400]) + XCTAssertEqual(chain.map(\.pid), [500, 400, 500]) + XCTAssertEqual(chain.last?.command, "(cycle detected)") } func testWalk_orphanPpid_emitsUnknownHopAndStops() { @@ -85,22 +88,87 @@ final class ParentChainWalkTests: XCTestCase { XCTAssertEqual(chain, [ParentChainWalker.ChainHop(pid: 500, command: "(unknown)")]) } - func testWalk_hopCap_boundsOversizedChain() { + func testWalk_hopCap_boundsOversizedChainWithVisibleMarker() { // 20-deep linear chain 500 → 501 → … ; cap at default 15 hops. The default must // exceed a real Claude Code session's depth — an observed chain (swift → zsh → // claude bg×2 → claude → login shell → login → Ghostty → launchd) already spends // exactly 10 hops, so 10 would truncate the host on any deeper nesting (tmux etc.). + // #173: truncation must be visible — 15 real hops + 1 marker entry at the cut point. var table: [Int32: ParentChainWalker.ProcessEntry] = [:] for i in Int32(500)..