Retire stale sidebar port badges despite vanished TTYs, privileged owners, and zombies - #9324

sequenceDiagram
  participant PortScanner
  participant ps
  participant ProcessIdentity
  participant PanelSnapshotReconciler
  PortScanner->>ps: scan normalized TTYs
  ps-->>PortScanner: process output and diagnostics
  PortScanner->>ProcessIdentity: validate process identity
  ProcessIdentity-->>PortScanner: valid identity or absent process
  PortScanner->>PanelSnapshotReconciler: publish complete or incomplete scan
  PanelSnapshotReconciler-->>PortScanner: retain or retire missing ports

❌ Failed checks (2 errors, 1 warning)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

@coderabbitai Regarding the outside-diff finding on runPS ("Reject mixed or failed ps results before returning complete emptiness"): the .complete verdict on the all-vanished path rests solely on every requested terminal being explicitly named in an ENOENT diagnostic — not on stdout or overall process health — so none of the listed conditions can produce a false complete:

• executionError: a launch or cancellation failure returns stderr: nil (CommandRunner.swift), so no ENOENT lines can exist, vanished stays empty, and the scan returns .incomplete.
• timedOut: a kill mid-write can only omit ENOENT lines. Any terminal without one stays in remaining, so the scan retries or returns .incomplete — and ps never emits ENOENT for a live terminal, so the lines that did arrive are genuine per-terminal stat evidence.
• Unrecognized diagnostics: a non-ENOENT diagnostic never removes a terminal from remaining, so it blocks the all-vanished branch. Pinned by mixedVanishedAndUnreadableDiagnosticsStayIncomplete (15ca94f): ENOENT for one TTY plus Permission denied for another stays .incomplete.
• parsedEveryRow: BSD ps stats -t devices during option processing and aborts before emitting any rows when one is missing, so the all-vanished case coexists with empty stdout. The branch discards mapping regardless — emptiness follows from the freed ptys, not from stdout parsing.

The comment on lines 372-373 describes exactly this: only ENOENT is treated as absence; every other diagnostic leaves the scan incomplete. No change made.

#!/bin/bash
set -euo pipefail

fd -a -t f 'PortScanner\+Process\.swift|CommandRunner\.swift|PortScannerTests\.swift' . \
  | sort \
  | while IFS= read -r f; do
      echo "===== OUTLINE: $f ====="
      ast-grep outline "$f" --items all --view compact || true
    done

echo "===== runPS and nearby logic ====="
rg -n -C 18 '\brunPS\b|vanished|parsedEveryRow|remaining|ENOENT|\.complete|\.incomplete' Sources cmuxTests \
  -g 'PortScanner+Process.swift' -g 'CommandRunner.swift' -g 'PortScannerTests.swift'

echo "===== cited regression test ====="
rg -n -A 70 -B 12 '\bmixedVanishedAndUnreadableDiagnosticsStayIncomplete\b' cmuxTests -g 'PortScannerTests.swift'

@mykmelez Confirmed. I withdraw the finding.

runPS returns all-vanished .complete only after vanishedTTYNames identifies every requested TTY from an ENOENT diagnostic. This path does not use stdout, parsedEveryRow, or the normal isCompletePSResult verdict.

A non-ENOENT diagnostic does not remove its TTY from remaining. The cited mixedVanishedAndUnreadableDiagnosticsStayIncomplete test verifies this behavior. A timeout or execution failure without qualifying ENOENT diagnostics also remains .incomplete.

The all-vanished result is authoritative negative evidence for the queried TTY set.

🐇

Learnt from: mykmelez
URL: https://github.com/manaflow-ai/cmux/pull/9324

Timestamp: 2026-08-03T03:03:02.803Z
Learning: In `Sources/PortScanner+Process.swift`, `PortScanner.runPS(ttyList:)` can return `([:], .complete)` after `ps` reports ENOENT for every requested TTY. This result is authoritative because each freed pty cannot have attached processes. The branch does not depend on stdout parsing or overall command success. Only requested TTYs explicitly identified by `vanishedTTYNames(inStderr:requested:)` as `": No such file or directory"` are removed; all other diagnostics retain `.incomplete`.

_{You are interacting with an AI system.}

Adversarial maintainer verification

Verdict: fixes #9152.

I traced the full local path (report_tty / ports_kick → batched ps → identity capture → lsof → per-panel completeness → PortScanSnapshotReconciler → lifecycle-gated publication) and confirmed all three root causes claimed in the PR:

1. A vanished TTY poisons BSD ps -t batches. On macOS 26.4.1, a live -t ttys072 query returned its processes, while the same query with ttys9152 appended exited 1, emitted only ps: /dev/ttys9152: No such file or directory, and returned none of the live TTY's rows. The test-only commit fails the retry/authoritative-empty expectations; the fix passes them.
2. proc_pidinfo cannot identify privileged leaders. As an unprivileged user, proc_pidinfo(1, PROC_PIDTBSDINFO, ...) returned 0/136 bytes, while sysctl(KERN_PROC_PID) returned launchd's PID and birth timestamp. That is the same ownership boundary as the root-owned login/session leader in a terminal process group. The shared sysctl identity reader removes the permanent incompletePIDs classification without weakening PID-reuse fencing.
3. Zombies are present to kill(pid, 0) but own nothing. A live macOS 26 check left /usr/bin/true unreaped: p_stat == SZOMB, kill(pid, 0) == 0, and proc_pidinfo still returned no identity. The new reader explicitly rejects SZOMB, and PIDPresence therefore treats it as absent. The behavioral test pins the resulting complete negative port evidence.

Additional gaps found and fixed

I appended four test-first/fix pairs, preserving the contributor's approach:

• 6b54e60f76 / 9f88b6d71a: the reporter's persistent Time Machine SMB warning made every otherwise-valid lsof result globally incomplete. The PID-scoped TCP query now uses lsof -w; real errors remain incomplete.
• 9fcfb5f498 / f6b382aaee: a kick near the end of an existing burst could receive only one later scan, but retirement requires three complete misses. Every kick now remains pending until three scan attempts have begun; a follow-up burst pays any remainder. Normal retirement is about 3.2 seconds after coalescing, rather than unbounded.
• 54854e33a4 / 4636dd9353: shells report /dev/ttys…, while ps -o tty= reports ttys…. Canonicalizing both sides fixes attribution as well as vanished-TTY matching.
• b554509c73 / e7c4253c71: hibernation destroys the PTY but previously retained surfaceListeningPorts, allowed an older queued publication to republish them, and persisted them into a snapshot that has no shell left to kick the scanner. Successful local hibernation now clears/recomputes ports and invalidates the scanner lifecycle; restored hibernated snapshots discard legacy persisted ports.

The corrected hibernation red run is important: the earlier method-filtered runs selected zero Swift Testing tests and were not valid evidence. The class-level test-only run executed 36 tests and failed only the two new regressions (six concrete expectations); the final-head run executed the same 36 and passed.

Safety, race, and performance checks

• No badge flapping: incomplete scans still never remove a known port. Two consecutive complete misses retain it; the third retires it. Positive observations remain immediate. Timeout, malformed output, unrecognized ps diagnostics, and real lsof errors stay incomplete.
• Lifecycle races: panel close, TTY replacement, and hibernation invalidate the MainActor lifecycle revision before queued scanner work can publish. Queue-side TTY/revision checks and the final PortScanPublicationState filter reject stale in-flight results.
• Scale: the vanished-TTY fallback is at most three batched ps calls, not one scan per TTY. Joins remain linear set/map work. Scanner/process work remains off the MainActor; no sidebar rendering path or keystroke-sensitive path changed.
• Persistence/relaunch: ordinary restored terminals spawn a fresh shell/TTY and report into a new scanner lifecycle. The uncovered no-future-kick case was a hibernated terminal; both live hibernation and legacy restored snapshots are now covered.
• Remote/SSH scope: remote workspaces use a separate marker-based script in CmuxRemoteSession; its BSD-ps/lsof fallback still deserves the same hardening in a follow-up. It is not the path in Sidebar port badges never clear: stopped servers stay listed indefinitely and accumulate #9152's local workspace create --cwd /tmp/... repro, so it does not block this verdict, but this PR should not be cited as remote-port parity.

I did not take the automated suggestion to extract a small CmuxCore process package. CmuxCore is deliberately pure values/protocol seams; Darwin process-table and subprocess I/O are infrastructure. A tiny partial port-scanning package would violate the repository's whole-domain package policy and is unrelated to this bug fix. I also did not add annotation-only @concurrent changes to the existing non-actor-isolated Swift-5 app-target methods: they execute on the generic executor, and CommandExecution explicitly bridges blocking descriptor reads to utility queues; this diff adds no MainActor process work.

Red/green and final validation

• Contributor root causes: process tests red, end-to-end red → process green, end-to-end green.
• Persistent lsof warning: process red, retirement red → process green, retirement green.
• Late-burst and full-path gaps: late-burst red, full-path red → combined green.
• Final HEAD: 22/22 process-capture tests, 3/3 end-to-end retirement tests, and 36/36 hibernation tests pass with Xcode 26.3 / SDK 26.2.

The explicit macOS-26 test-e2e jobs did not reach test discovery: Xcode failed the app-host build with duplicate Sentry framework copy tasks on both attempts (retirement, process capture); the macos-latest/Xcode 27 image failed the same way. I did not count those as green.

I then bypassed that unrelated app-host path with an isolated SwiftPM executable harness on this macOS 26.4.1 host. The harness compiled the branch exact PortScanner+Process.swift, AgentPIDProcessIdentity.swift, PIDPresence.swift, and PortLsofScanResult.swift together with the real CmuxCore and CmuxFoundation packages. Against live processes, it recovered the harness PID from a mixed live/vanished TTY batch, joined /dev/ttys… registration to abbreviated ps output, captured PID 1 without an incomplete classification, rejected an unreaped zombie as absent, found a real listener through runLsof, then observed complete emptiness after close. The real PortScanSnapshotReconciler retained that port across an incomplete miss and the first two complete misses, then retired it on the third. The temporary harness was removed and was not committed. This adds executable macOS 26 coverage of the OS-sensitive production implementations, while the linked Xcode 26.3 CI runs remain the evidence for the app-host scheduling and hibernation paths.

git diff --check, test wiring (644 files), package-lock policy, workspace package grouping, and pbxproj normalization all pass. The current origin/main merge-tree is conflict-free; PR checks roll up SUCCESS, there are no unresolved review threads, and GitHub reports MERGEABLE. Localization audit: N/A — no user-facing strings changed.

Verdict: fixes #9152.