Escape uncreatable socket state directory to /tmp; dedupe listener-failure captures per episode - #11050

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

The change updates socket fallback rules, preserves underlying POSIX errnos from wrapped directory errors, and replaces cooldown-based listener failure capture with thread-safe per-episode deduplication. Tests cover fallback paths, errno handling, deduplication, suppression, and episode reset.

Changes

Socket listener handling

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to e33c8

The change adds a bounded socket fallback and episode-based failure reporting to prevent repeated listener failures and telemetry floods. No actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant TerminalController
  participant SocketListenerFailureCaptureGate
  participant Sentry
  TerminalController->>SocketListenerFailureCaptureGate: shouldCapture(message, stage, path, errnoCode)
  SocketListenerFailureCaptureGate-->>TerminalController: capture decision
  TerminalController->>Sentry: capture failure when permitted
  TerminalController->>SocketListenerFailureCaptureGate: listenerDidStart()
  SocketListenerFailureCaptureGate->>SocketListenerFailureCaptureGate: clear episode keys

Suggested reviewers: austinywang

Explanation

The description is detailed, relevant, and explains the changes, rationale, testing, affected issues, and residual risk. It omits the template headings and does not include the review-trigger block, checklist, or behavior-change demo video.

Explanation

Docstring coverage is 52.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 7 files. (1 skipped: 1 too large.)

Explanation

No changed production code matches the stated actor-isolation failures. The new SocketListenerFailureCaptureGate is a Sendable shared reference whose mutable Set is protected by OSAllocatedUnfairLock; its documentation states that it is shared by the main actor and listener queue and is thread-safe. SocketListenerPolicy and SocketTransport remain value-based Sendable types, and the new posixErrnoCode helper is pure. TerminalController remains explicitly @MainActor; the changed nonisolated event seam captures only the locked gate, while UI callbacks retain their existing actor boundaries. The PR does not add an implicitly MainActor model or service protocol, nor does it add background access to a UI store.

Explanation

The production diff adds OSAllocatedUnfairLock<Set<String>> in SocketListenerFailureCaptureGate.swift. Its withLock calls protect mutable capture state accessed by the main actor and listener queue. This is manual locking in a new cross-concurrency gate, and the diff does not document why an actor cannot own the state. The removed NSLock does not remove the new gate's manual-lock implementation. No new sleep, semaphore, delayed dispatch, or polling primitive was found.

Resolution

Replace OSAllocatedUnfairLock with actor-owned state. Make the failure-reporting path asynchronous, or enqueue both failure and successful-start events to one actor so the lifecycle order remains correct. Do not block the listener queue or main actor while obtaining the capture decision.

Explanation

The full PR range changes only socket-listener policy, errno handling, failure capture, related tests, and listener wiring in Sources/TerminalController.swift. The TerminalController diff does not change browser command routing. ControlCommandExecutionPolicy.swift is unchanged, and no added line introduces browser.* commands, WebKit waits, main-actor routing, or worker-lane browser code. The custom check is therefore not triggered.

Explanation

The PR does not add or move an expensive synchronous agent-history load. Added production code only changes socket fallback/errno handling, adds an in-memory locked failure gate, and wires that gate into existing socket lifecycle callbacks. The added production lines contain no RestorableAgentSessionIndex, agent stores, transcripts, trajectories, JSONL loads, directory scans, Data(contentsOf:), JSON decoding, or per-record history syscalls. The existing Task.detached socket connection task and unrelated file/JSON operations are unchanged.

Explanation

PASS: The PR does not substitute a cache for an authoritative read. The production diff changes socket-path fallback, mkdir errno unwrapping, and Sentry failure deduplication. It does not modify persistence, history, undo, or snapshot read paths, and no changed production line introduces a cached value for those consumers. The cache-substitution correctness check is therefore inapplicable.

Explanation

PASS: The pull request changes only Swift files, including production Swift sources and Swift tests. The custom check applies only to production TypeScript, JavaScript, shell, and build/runtime scripts. The PR introduces no covered non-Swift files or delay constructs.

Explanation

No algorithmic-complexity failure is introduced. The new SocketListenerFailureCaptureGate uses a hash Set for expected O(1) deduplication and does not scan workspace-sized data. Its only loop, in SocketTransport+Bind.swift, has the explicit fixed bound of eight underlying-error levels. The policy changes use fixed conditionals, and the tests use small fixed fixtures. No nested collection scan, batch rescan, repeated sorting/filtering, or data-store join appears in the changed production code.

Explanation

PASS. The pull-request diff adds no prohibited legacy async pattern. The new SocketListenerFailureCaptureGate uses OSAllocatedUnfairLock for synchronous state protection and contains no DispatchQueue, Task, Combine, or completion-handler code. The TerminalController change only injects the gate and calls its synchronous methods. The existing Task { @mainactor in ... } path-missing hop is unchanged from origin/main and is an allowed legacy callback-to-main-actor boundary. Added tests also introduce no prohibited runtime pattern.

Explanation

PASS: The PR diff introduces no async, nonisolated async, or @concurrent declaration. The new SocketListenerFailureCaptureGate and posixErrnoCode(from:) APIs are synchronous. The added gate call runs synchronously from the existing @MainActor callback and performs only locked set operations. No new CPU-heavy, file-heavy, parsing-heavy, or network-heavy async call from UI isolation is present.

Explanation

PASS — The pull request keeps the independently testable socket logic behind the existing CmuxControlSocket SwiftPM target. SocketListenerFailureCaptureGate, SocketListenerPolicy changes, and SocketTransport errno handling are under Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket, with matching package tests under its test target. Sources/TerminalController.swift only performs app-lifecycle composition and injects the gate into host callbacks, which the boundary rules allow. No changed production domain logic remains in the app target.

Explanation

PASS. The PR changes only Swift source and test files. Packages/macOS/CmuxControlSocket/Package.swift, .gitignore, cmux.xcodeproj/project.pbxproj, and the root Xcode Package.resolved are unchanged. No package dependency, package-reference, workflow, or ignore-policy change requires a lockfile diff.

Explanation

PASS: The PR adds no print, debugPrint, dump, NSLog, ad hoc file logging, stdout/stderr logging, or Logger declarations. The runtime change only gates existing sentryBreadcrumb/sentryCaptureError calls through SocketListenerFailureCaptureGate; it does not add a logging destination or new sensitive payload. The diff also contains no new secret or personal-data logging.

Explanation

PASS — The production diff changes socket recovery and internal Sentry telemetry only. The failure callback sends breadcrumbs and captured errors through sentryBreadcrumb/sentryCaptureError; it is not wired to UI text, CLI responses, or API error bodies. New strings are socket stages, errno data, and paths, with no vendor/provider details, secrets, tokens, headers, raw upstream messages, or payload dumps. References to Sentry, sudo, and internal incident identifiers occur only in source documentation/comments or tests, which the rule allows.

Explanation

PASS: The diff adds no user-facing UI, menu, alert, command, web, metadata, or localized app text. The new literals (socket.listener.path.missing, failure stages, and socket.listener.start.failed) are socket telemetry/protocol identifiers used for Sentry breadcrumbs and capture, not displayed copy. The added prose is developer-facing Swift documentation and comments. No string catalog or web locale files require updates.

Explanation

PASS: The pull request does not introduce or materially expand SwiftUI state or layout code. The changed app file, Sources/TerminalController.swift, uses AppKit and changes socket listener event wiring only. The new SocketListenerFailureCaptureGate is a non-SwiftUI Sendable class. The diff adds no ObservableObject, @Published, @Observable, GeometryReader, lazy/list row store references, or render-time state mutations.

Explanation

PASS. The diff does not introduce an architectural-rethink failure. The socket fallback and errno changes are small local correctness fixes. The new capture gate has one construction and one event-seam owner. Its lock protects a narrow cross-thread telemetry set; it does not add timing, polling, delayed dispatch, or lifecycle repair. The previous TerminalController NSLock and cooldown cache are removed. SocketControlServer remains the authoritative owner of listener lifecycle state, and the existing synchronous listenerDidStart transition resets the gate. No duplicate entrypoint wiring or UI lifecycle ownership changes were introduced.

Explanation

PASS. The PR changes socket policy, transport error handling, failure capture, and related tests. The diff adds or modifies no user-visible NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup, close-shortcut handler, or window identifier assignment. The deterministic lint also passes: scripts/lint_auxiliary_window_close_shortcuts.py checked 36 identifiers.

Explanation

PASS — The PR changes only Swift source, documentation comments, and Swift test files under the expected source and test directories. The complete diff contains no artifact-like paths, binary files, generated logs, caches, build output, or scratch directories. The /tmp paths appear only as deliberate runtime test fixtures and product fallback strings.

Explanation

PASS: The production diff adds no test/debug seam. SocketListenerFailureCaptureGate is a production component called by TerminalController for real listener-failure telemetry, not an accessor for private state. Its public API is used by the application, and its tests use @testable import from the test target. The other production changes add socket fallback logic and a private errno-unwrapping helper. No new #if DEBUG test accessor, ForTesting/TestHook member, or wrapper around widened state was added.

Explanation

PASS: The diff adds no ambient global state that matches the failure conditions. SocketListenerFailureCaptureGate is a constructable instance type with instance-owned state and is injected at the TerminalController composition seam. Its only static member is the permitted pathMissingMessage constant. The new posixErrnoCode helper is private and scoped to SocketTransport; it is not a top-level API. The diff adds no top-level production API function, mutable global, static-only namespace, or shared/standard/default singleton. It also removes the previous listener capture globals.

• X
• Mastodon
• Reddit
• LinkedIn