Repository navigation
Move Ghostty rendering into a dedicated worker process - #8320
Closed
azooz2003-bit wants to merge 14 commits into
Closed
azooz2003-bit wants to merge 14 commits into
azooz2003-bit wants to merge 14 commits into
Conversation
added 12 commits
July 16, 2026 22:24
📝 WalkthroughWalkthroughChangesAdds an out-of-process macOS Ghostty render worker with versioned control messages, framed stdin/stdout IPC, authenticated Mach/IOSurface frame transport, worker supervision, crash recovery, and generation-fenced remote presentation. Ghostty render worker
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GhosttyTerminalView
participant TerminalSurface
participant GhosttyRenderRuntimeBridge
participant GhosttyRenderWorkerClient
participant GhosttyRenderWorker
participant GhosttyRemoteIOSurfaceLayer
GhosttyTerminalView->>TerminalSurface: create mirrored surface and forward input
TerminalSurface->>GhosttyRenderRuntimeBridge: enqueue versioned render command
GhosttyRenderRuntimeBridge->>GhosttyRenderWorkerClient: enqueue ordered command
GhosttyRenderWorkerClient->>GhosttyRenderWorker: send framed control message
GhosttyRenderWorker->>GhosttyRenderWorkerClient: send authenticated IOSurface frame
GhosttyRenderWorkerClient->>GhosttyTerminalView: deliver fenced frame
GhosttyTerminalView->>GhosttyRemoteIOSurfaceLayer: present frame
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (7 errors, 1 warning, 1 inconclusive)
✅ Passed checks (16 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Greptile SummaryThis PR moves all Ghostty Metal rendering into a dedicated, sandboxable helper process (
Confidence Score: 4/5Safe to merge once the three issues from prior review threads are resolved: EAGAIN-as-hard-error causing spurious worker restarts under heavy output, deprecated bootstrap_register breaking sandboxed deployments, and the fire-and-forget Task hop for resynchronization sequencing. The architecture is sound — generation fencing, ordered command sink, last-frame retention, and VT snapshot resync are all correctly wired. The three issues flagged in prior threads are real defects on the changed path: EAGAIN treated as a fatal write failure means any burst of PTY output that fills the 64 KB pipe buffer triggers a full worker restart+resync cycle; bootstrap_register will silently fail and disable the worker inside a sandboxed process; and the unstructured Task for resynchronization can deliver a resync command after a subsequent createSurface from ensureRunning. None of the other changed files introduces new correctness problems — the tee installation race fix, the optimistic rendererRealized state, the CA layer generation fencing, and the Mach frame transport all look correct. TerminalRenderMessageChannel.swift (EAGAIN handling), TerminalRenderMachIPC.c (bootstrap_register), Sources/TerminalSurfaceRuntimeWiring.swift (resynchronizationRequired Task isolation) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
PTY[PTY Thread / IO callbacks] -->|mirrorOutput| Sink[GhosttyRenderCommandSink\nserial DispatchQueue]
Main[Main Actor / AppKit events] -->|enqueueRenderCommand| Sink
Sink -->|AsyncStream ordered| Actor[GhosttyRenderWorkerClient\nSwift actor]
Actor -->|nonblocking POSIX pipe write| Pipe[Control Channel\nPOSIX pipe]
Pipe -->|reads commands| Worker[cmux-ghostty-render-worker\nAppKit-free process]
Worker -->|Mach IPC IOSurface| Receiver[TerminalRenderFrameReceiver\nblocking Mach receive thread]
Receiver -->|AsyncStream| FrameActor[actor frame dispatch]
FrameActor -->|MainActor.run| Layer[GhosttyRemoteIOSurfaceLayer\nCALayer generation-fenced]
Worker -->|control events| CtrlReader[control reader Thread]
CtrlReader -->|AsyncStream| Actor
Actor -->|broadcast events| Wiring[TerminalSurfaceRuntimeWiring\nMainActor event loop]
Wiring -->|resync command| Actor
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
PTY[PTY Thread / IO callbacks] -->|mirrorOutput| Sink[GhosttyRenderCommandSink\nserial DispatchQueue]
Main[Main Actor / AppKit events] -->|enqueueRenderCommand| Sink
Sink -->|AsyncStream ordered| Actor[GhosttyRenderWorkerClient\nSwift actor]
Actor -->|nonblocking POSIX pipe write| Pipe[Control Channel\nPOSIX pipe]
Pipe -->|reads commands| Worker[cmux-ghostty-render-worker\nAppKit-free process]
Worker -->|Mach IPC IOSurface| Receiver[TerminalRenderFrameReceiver\nblocking Mach receive thread]
Receiver -->|AsyncStream| FrameActor[actor frame dispatch]
FrameActor -->|MainActor.run| Layer[GhosttyRemoteIOSurfaceLayer\nCALayer generation-fenced]
Worker -->|control events| CtrlReader[control reader Thread]
CtrlReader -->|AsyncStream| Actor
Actor -->|broadcast events| Wiring[TerminalSurfaceRuntimeWiring\nMainActor event loop]
Wiring -->|resync command| Actor
Reviews (2): Last reviewed commit: "fix: bound remote terminal resize backlo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+66
to
+83
| try data.withUnsafeBytes { raw in | ||
| guard let baseAddress = raw.baseAddress else { return } | ||
| var offset = 0 | ||
| while offset < raw.count { | ||
| let written = Darwin.write( | ||
| writeDescriptor, | ||
| baseAddress.advanced(by: offset), | ||
| raw.count - offset | ||
| ) | ||
| if written > 0 { | ||
| offset += written | ||
| } else if written == -1, errno == EINTR { | ||
| continue | ||
| } else { | ||
| throw TerminalRenderChannelError.writeFailed | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
writeAll with O_NONBLOCK set treats EAGAIN the same as any other write error and throws writeFailed. The only errno it retries on is EINTR. Because the host channel is opened with nonblockingWrites: true, any write that returns -1 / EAGAIN (pipe buffer full) lands here and escalates immediately to loseWorker at every call site.
This becomes a live problem under heavy PTY output. The worker reads from this pipe one message at a time inside engineQueue.sync; if Ghostty or Metal briefly holds the engine queue (refresh tick, large output parse), the 64 KB pipe buffer fills up, the next mutateSurface write fails with EAGAIN, and a full worker restart + screen-tail resynchronization is triggered. The restart-then-resync cycle can repeat while output is flowing, producing visible rendering artifacts on the active terminal.
The fix is to add an EAGAIN retry path analogous to the existing EINTR retry — either spin-retry (acceptable for the control pipe because messages are small and the engine queue drains quickly) or back off and enqueue the failed bytes for a later attempt before escalating to loseWorker.
Comment on lines
+55
to
+58
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wdeprecated-declarations" | ||
| result = bootstrap_register(bootstrap_port, (char *)service_name, port); | ||
| #pragma clang diagnostic pop |
Contributor
There was a problem hiding this comment.
bootstrap_register suppressed silently
bootstrap_register has been deprecated since macOS 10.5 and does not work inside a sandboxed process — the bootstrap server rejects the call with BOOTSTRAP_NOT_PRIVILEGED. The #pragma clang diagnostic ignored suppresses the compiler warning rather than addressing it.
If the host app ever gains a sandbox entitlement (even a permissive one), cmux_terminal_render_receiver_create will return a non-KERN_SUCCESS result, TerminalRenderFrameReceiver.init will throw, GhosttyRenderWorkerClient.bundledWorker() will fail, and GhosttyRenderRuntimeBridge will silently skip the render worker on every launch.
The modern alternative is a launchd-managed XPC service or passing the Mach send right over a pre-existing socket rather than going through the bootstrap server.
Comment on lines
+203
to
+211
| case let .resynchronizationRequired(surfaceID, surfaceGeneration): | ||
| Task { @MainActor in | ||
| guard let surface = GhosttyApp.terminalSurfaceRegistry.surface(id: surfaceID) | ||
| as? TerminalSurface, | ||
| let command = await surface.renderWorkerResynchronizationCommand( | ||
| surfaceGeneration: surfaceGeneration | ||
| ) else { return } | ||
| self.enqueueRenderCommand(command) | ||
| } |
Contributor
There was a problem hiding this comment.
Task { @MainActor in } races with the event loop for resynchronization
All other main-actor hops in observeEvents use await MainActor.run { }, which suspends the event-loop iteration until the hop completes. Only the resynchronizationRequired handler uses a fire-and-forget Task { @MainActor in }, so the next event is processed before the resync snapshot is read or the resynchronizeSurface command is enqueued. The generation guards in renderWorkerResynchronizationCommand prevent data corruption, but a resynchronizeSurface for the current generation can arrive after a fresh createSurface from ensureRunning, producing a redundant resync. Using await MainActor.run { } here, consistent with the rest of the function, makes the sequencing explicit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderCommandSink.swift`:
- Around line 18-24: Replace the unbounded command stream in
GhosttyRenderCommandSink.init with a byte-budgeted ingress. In
GhosttyRenderWorkerClient.swift, centralize pending output and subscriber
backlog ownership, bound or coalesce high-frequency control ingress, give event
subscribers an explicit bounded/coalesced policy, and cap pending mutations so
ordered output beyond the cap triggers an authoritative snapshot request; apply
these changes at lines 40-45, 69-74, 126-133, and 207-223 respectively.
In
`@Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift`:
- Around line 13-21: Replace the top-level runGhosttyRenderWorker() entry point
with a constructable runtime owner that accepts an injected
TerminalRenderMessageChannel or its descriptors, while preserving engine
execution and Darwin.exit(status). Update both executable entry points to
instantiate this owner and invoke its run method, removing hard-coded stdio
creation from the public top-level API.
- Around line 257-263: Make resynchronization atomic across the worker and
client: in RunGhosttyRenderWorker.swift lines 257-263, suppress the
realization-triggered refresh when creating a resynchronizing surface; in
RunGhosttyRenderWorker.swift lines 106-117, apply the snapshot and sequence
before issuing the first refresh and acknowledgement; in
GhosttyRenderWorkerClient.swift lines 225-235, retain the recovery fence and
pending mutations until the matching acknowledgement is received.
- Around line 23-28: Unify mutable render state ownership in
GhosttyRenderWorkerEngine by sequencing WorkerSurfacePresentation and all
libghostty access through engineQueue. Remove the separate NSLock-based
synchronization from WorkerSurfacePresentation and ensure frame updates,
presentation, and engine operations execute on the single engine owner. Keep
only the C callback adapters as `@unchecked` Sendable boundary code.
In
`@Packages/macOS/CmuxGhosttyRenderService/Tests/CmuxGhosttyRenderClientTests/GhosttyRenderWorkerClientTests.swift`:
- Around line 171-187: Update restartsHungWorkerOnceWithoutDuplicateExitEvents
and the related test path to inject the watchdog sleeper/clock, advancing it
explicitly to trigger timeout handling instead of relying on real-time waits.
Capture and reuse the events returned by EventCollector.waitUntil, or await
stream completion, before taking subsequent snapshots; remove fixed Task.sleep
delays and other wall-clock timing from these tests.
In
`@Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface`+CopyMode.swift:
- Around line 14-18: Update the binding-action flow around
`ghostty_surface_binding_action` and `enqueueRenderMutation` so stateful
`set_font_size` actions also update the authoritative
`renderMirrorDescriptor.fontSize`, preferably via the existing typed font-size
mutation, before enqueueing `.bindingAction(action)`. Ensure generation-fenced
worker resynchronization uses the updated descriptor value.
In
`@Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h`:
- Line 51: The Ghostty screen-tail stub has an ABI-mismatched declaration and
definition. Update ghostty_surface_read_screen_tail_vt_with_output_sequence in
Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.h:51
and
Packages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.c:118
to use the Ghostty API signature returning bool and accepting ghostty_surface_t,
two uintptr_t values, ghostty_text_s*, and uint64_t*, keeping the declaration
and implementation identical.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderControlProtocol.swift`:
- Around line 11-26: Update TerminalRenderFrameEndpoint with a custom
init(from:) that decodes serviceName and authenticationToken into locals, then
delegates validation to init(serviceName:authenticationToken:). Add a test in
Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift
covering malformed plist decoding with an invalid token and asserting the decode
throws.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderFrameTransport.swift`:
- Around line 109-145: Update start, receiveLoop, and stop so the receive
operation does not retain the transport instance: capture immutable
receive-port/token state and explicitly own the operation’s lifetime. Replace
receiveOne’s 250 ms polling with a blocking wait awakened by port destruction,
using an explicit completion event for coordination. Enforce that start can
succeed only once, preventing repeated or competing consumers while preserving
stop’s wake-and-terminate behavior.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift`:
- Around line 65-81: Update TerminalRenderMessageChannel’s nonblocking write
path at
Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift:65-81,
using the O_NONBLOCK setup at :25-31, to handle EAGAIN/EWOULDBLOCK by waiting
for writability and resuming the same offset until the full frame is sent;
retain EINTR retries and only throw for unrecoverable errors. Update the
corresponding worker-loss handling at
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift:274-278
so a transient backpressure condition does not abandon the in-flight command.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/TerminalRenderMachIPC.c`:
- Around line 165-181: Update the validity check in the frame-message receive
validation to require header.msgh_size to equal
sizeof(cmux_terminal_render_frame_message_s) before evaluating or trusting
message->metadata and frame fields. Preserve the existing descriptor cleanup and
MIG_TYPE_ERROR return for all invalid messages.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift`:
- Around line 84-91: Add a regression test alongside
endpointRejectsWrongTokenSize that encodes a binary property-list representation
containing the same serviceName and 15-byte authenticationToken, then decodes it
as TerminalRenderFrameEndpoint and asserts invalidFrameEndpoint is thrown. Cover
the Codable decode path while preserving the existing throwing-initializer
assertion.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4207-4215: Update the GhosttyRemoteIOSurfaceLayer branch in
present(_:) to pass the worker-applied mirror size from
updateRenderMirrorSize(...) into updateExpectedPixelSize, rather than
constructing it from drawablePixelSize. Preserve the existing retry-state and
backing-scale updates, and ensure bootstrap initialization uses that same
applied size.
In `@Sources/TerminalSurfaceRuntimeWiring.swift`:
- Around line 129-210: Refactor GhosttyRenderRuntimeBridge to use a
constructable, injected actor owner instead of the shared singleton and
revisionLock. Have the actor install event/frame subscriptions before applying
any configuration, retain and lifecycle-manage the observation task, and
serialize configuration revisions with resynchronization commands so
initialization events cannot be lost. Remove the manual lock and ambient runtime
singleton while preserving existing worker routing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f356ba0-8fae-4283-821b-415bb9a7a971
⛔ Files ignored due to path filters (1)
cmux.xcworkspace/contents.xcworkspacedatais excluded by!**/*.xcworkspace/contents.xcworkspacedata
📒 Files selected for processing (53)
Packages/macOS/CmuxGhosttyRenderService/Package.swiftPackages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderCommandSink.swiftPackages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swiftPackages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClientEvent.swiftPackages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swiftPackages/macOS/CmuxGhosttyRenderService/Sources/GhosttyKit/module.modulemapPackages/macOS/CmuxGhosttyRenderService/Sources/cmux-ghostty-render-fixture/main.swiftPackages/macOS/CmuxGhosttyRenderService/Sources/cmux-ghostty-render-worker-test-host/WorkerTestHost.swiftPackages/macOS/CmuxGhosttyRenderService/Tests/CmuxGhosttyRenderClientTests/GhosttyRenderWorkerClientTests.swiftPackages/macOS/CmuxTerminal/Package.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Hosting/GhosttyRemoteIOSurfaceLayer.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Hosting/TerminalSurfaceNativeViewing.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeScreenTailRequest.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Lifecycle/TerminalSurfaceRuntimeTeardownCoordinator.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Runtime/TerminalByteTeeBinding.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Runtime/TerminalRenderWorkerRouting.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Runtime/TerminalSurfaceRuntimeDependencies.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+CopyMode.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+ForceRefresh.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Mobile.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+MobileViewportFit.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Renderer.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeLifecycle.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+RuntimeSurfaceCreation.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+ScreenSnapshot.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Sizing.swiftPackages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/FakeTerminalByteTee.swiftPackages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/GhosttyRemoteIOSurfaceLayerTests.swiftPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/GhosttyRuntimeTestStubs.cPackages/macOS/CmuxTerminal/Tests/GhosttyRuntimeTestStubs/include/GhosttyRuntimeTestStubs.hPackages/macOS/CmuxTerminalRenderTransport/Package.swiftPackages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderControlProtocol.swiftPackages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderFrameTransport.swiftPackages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swiftPackages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/TerminalRenderMachIPC.cPackages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/include/TerminalRenderMachIPC.hPackages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swiftPackages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderFrameTransportTests.swiftPackages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderMessageChannelTests.swiftRenderWorker/GhosttyRenderWorkerMain.swiftSources/GhosttyTerminalView.swiftSources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swiftSources/TerminalOutputTeeCallback.swiftSources/TerminalOutputTeeContext.swiftSources/TerminalSurfaceRuntimeWiring.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/GhosttyDrawableSizeRetryTests.swiftdocs/ghostty-fork.mddocs/ghostty-render-worker.mdghosttyscripts/strip-release-bundle.sh
💤 Files with no reviewable changes (1)
- Sources/cmuxApp.swift
Comment on lines
+18
to
+24
| init() { | ||
| let pair = AsyncStream.makeStream( | ||
| of: TerminalRenderWorkerCommand.self, | ||
| bufferingPolicy: .unbounded | ||
| ) | ||
| self.stream = pair.stream | ||
| self.continuation = pair.continuation |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The output/recovery path has several independent unbounded buffers, allowing worker stalls to become host OOMs.
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderCommandSink.swift#L18-L24: replace the unbounded command stream with a byte-budgeted ingress.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L40-L45: centralize pending output and subscriber backlog ownership.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L69-L74: bound or coalesce high-frequency control ingress.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L126-L133: give event subscribers an explicit bounded/coalesced policy.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L207-L223: cap pending mutations and request an authoritative snapshot when ordered output exceeds that cap.
📍 Affects 2 files
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderCommandSink.swift#L18-L24(this comment)Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L40-L45Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L69-L74Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L126-L133Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L207-L223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderCommandSink.swift`
around lines 18 - 24, Replace the unbounded command stream in
GhosttyRenderCommandSink.init with a byte-budgeted ingress. In
GhosttyRenderWorkerClient.swift, centralize pending output and subscriber
backlog ownership, bound or coalesce high-frequency control ingress, give event
subscribers an explicit bounded/coalesced policy, and cap pending mutations so
ordered output beyond the cap triggers an authoritative snapshot request; apply
these changes at lines 40-45, 69-74, 126-133, and 207-223 respectively.
Source: Coding guidelines
Comment on lines
+13
to
+21
| public func runGhosttyRenderWorker() -> Never { | ||
| let channel = TerminalRenderMessageChannel( | ||
| readDescriptor: STDIN_FILENO, | ||
| writeDescriptor: STDOUT_FILENO | ||
| ) | ||
| let engine = GhosttyRenderWorkerEngine(channel: channel) | ||
| let status = engine.run() | ||
| Darwin.exit(status) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move the public entry point onto a constructable runtime owner.
This top-level function owns process-wide runtime behavior and hard-codes stdio transport creation. Expose a constructable owner receiving its channel or descriptors, and have both executable entry points instantiate that owner.
As per coding guidelines, production Sources code should avoid top-level runtime APIs and use constructable, injectable owners.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift`
around lines 13 - 21, Replace the top-level runGhosttyRenderWorker() entry point
with a constructable runtime owner that accepts an injected
TerminalRenderMessageChannel or its descriptors, while preserving engine
execution and Darwin.exit(status). Update both executable entry points to
instantiate this owner and invoke its run method, removing hard-coded stdio
creation from the public top-level API.
Source: Coding guidelines
Comment on lines
+23
to
+28
| private final class GhosttyRenderWorkerEngine: @unchecked Sendable { | ||
| private let channel: TerminalRenderMessageChannel | ||
| private let engineQueue = DispatchQueue( | ||
| label: "dev.cmux.ghostty-render-worker.engine", | ||
| qos: .userInteractive | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 \
'`@unchecked` Sendable|DispatchQueue|NSLock|scheduleTick|func present' \
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swiftRepository: manaflow-ai/cmux
Length of output: 2184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift | cat -n
printf '\n---\n'
sed -n '260,520p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 22455
Unify render-worker ownership
The engine still splits mutable state between engineQueue and WorkerSurfacePresentation’s NSLock, so the frame path depends on two separate @unchecked Sendable islands. Collapse sequencing and libghostty access under one owner, leaving only the C callback adapters at the boundary.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 23-23: Classes should have an explicit deinit method
(required_deinit)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift`
around lines 23 - 28, Unify mutable render state ownership in
GhosttyRenderWorkerEngine by sequencing WorkerSurfacePresentation and all
libghostty access through engineQueue. Remove the separate NSLock-based
synchronization from WorkerSurfacePresentation and ensure frame updates,
presentation, and engine operations execute on the single engine owner. Keep
only the C callback adapters as `@unchecked` Sendable boundary code.
Sources: Coding guidelines, Path instructions
Comment on lines
+257
to
+263
| ghostty_surface_set_content_scale(handle, descriptor.scaleX, descriptor.scaleY) | ||
| ghostty_surface_set_size(handle, max(descriptor.width, 1), max(descriptor.height, 1)) | ||
| ghostty_surface_set_occlusion(handle, true) | ||
| _ = ghostty_surface_set_renderer_realized(handle, true) | ||
| ghostty_surface_refresh(handle) | ||
| if emitCreated { | ||
| send(.surfaceCreated(id: descriptor.id, generation: descriptor.generation)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Resynchronization is not atomic because the worker refreshes before applying the snapshot and the client removes its fence before acknowledgement.
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift#L257-L263: suppress realization refresh while creating a resynchronizing surface.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift#L106-L117: apply the snapshot and sequence before issuing the first refresh and acknowledgement.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L225-L235: retain the recovery fence and pending mutations until the matching acknowledgement arrives.
📍 Affects 2 files
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift#L257-L263(this comment)Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift#L106-L117Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L225-L235
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderWorker/RunGhosttyRenderWorker.swift`
around lines 257 - 263, Make resynchronization atomic across the worker and
client: in RunGhosttyRenderWorker.swift lines 257-263, suppress the
realization-triggered refresh when creating a resynchronizing surface; in
RunGhosttyRenderWorker.swift lines 106-117, apply the snapshot and sequence
before issuing the first refresh and acknowledgement; in
GhosttyRenderWorkerClient.swift lines 225-235, retain the recovery fence and
pending mutations until the matching acknowledgement is received.
Source: Path instructions
Comment on lines
+171
to
+187
| @Test func restartsHungWorkerOnceWithoutDuplicateExitEvents() async throws { | ||
| let client = try GhosttyRenderWorkerClient( | ||
| executableURL: URL(fileURLWithPath: "/bin/sleep"), | ||
| arguments: ["30"], | ||
| initializationTimeout: .milliseconds(100), | ||
| automaticInitializationRetryLimit: 1 | ||
| ) | ||
| let collector = EventCollector(stream: await client.subscribeEvents()) | ||
| await client.updateConfiguration(configuration()) | ||
|
|
||
| _ = await collector.waitUntil(timeout: .seconds(3)) { events in | ||
| events.filter { event in | ||
| if case .workerExited = event { return true } | ||
| return false | ||
| }.count == 2 | ||
| } | ||
| try? await Task.sleep(for: .milliseconds(200)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Replace real-time watchdog and settle sleeps with deterministic signals.
Inject the watchdog sleeper/clock so the timeout can be advanced explicitly. After observing a crash, use the events returned by waitUntil or await stream completion instead of sleeping before taking another snapshot.
As per coding guidelines, tests under Packages/**/Tests must use completion signals or virtual clocks instead of fixed wall-clock waits.
Also applies to: 221-228
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxGhosttyRenderService/Tests/CmuxGhosttyRenderClientTests/GhosttyRenderWorkerClientTests.swift`
around lines 171 - 187, Update restartsHungWorkerOnceWithoutDuplicateExitEvents
and the related test path to inject the watchdog sleeper/clock, advancing it
explicitly to trigger timeout handling instead of relying on real-time waits.
Capture and reuse the events returned by EventCollector.waitUntil, or await
stream completion, before taking subsequent snapshots; remove fixed Task.sleep
delays and other wall-clock timing from these tests.
Source: Coding guidelines
Comment on lines
+65
to
+81
| private func writeAll(_ data: Data) throws { | ||
| try data.withUnsafeBytes { raw in | ||
| guard let baseAddress = raw.baseAddress else { return } | ||
| var offset = 0 | ||
| while offset < raw.count { | ||
| let written = Darwin.write( | ||
| writeDescriptor, | ||
| baseAddress.advanced(by: offset), | ||
| raw.count - offset | ||
| ) | ||
| if written > 0 { | ||
| offset += written | ||
| } else if written == -1, errno == EINTR { | ||
| continue | ||
| } else { | ||
| throw TerminalRenderChannelError.writeFailed | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files 'Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift' \
'Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift'
printf '\nOutline TerminalRenderMessageChannel.swift:\n'
ast-grep outline Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift --view expanded || true
printf '\nOutline GhosttyRenderWorkerClient.swift:\n'
ast-grep outline Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift --view expanded || true
printf '\nRelevant symbols/search:\n'
rg -n "writeAll|send\\(|nonblockingWrites|O_NONBLOCK|EAGAIN|EINTR|writeDescriptor|readDescriptor|TerminalRenderMessageChannel" \
Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift \
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swiftRepository: manaflow-ai/cmux
Length of output: 8884
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift \
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift
printf '\nTerminalRenderMessageChannel.swift (1-220):\n'
sed -n '1,220p' Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift
printf '\nGhosttyRenderWorkerClient.swift (240-320):\n'
sed -n '240,320p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swiftRepository: manaflow-ai/cmux
Length of output: 7367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swiftRepository: manaflow-ai/cmux
Length of output: 3996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "TerminalRenderMessageChannel\\(|func send\\(|writeAll\\(|EAGAIN|O_NONBLOCK|nonblockingWrites" Packages/macOS -g '*.swift'Repository: manaflow-ai/cmux
Length of output: 10302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'TerminalRenderMessageChannel.swift:\n'
sed -n '1,220p' Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift
printf '\nGhosttyRenderWorkerClient.swift references:\n'
rg -n "TerminalRenderMessageChannel|send\\(|write|nonblockingWrites" Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swiftRepository: manaflow-ai/cmux
Length of output: 5141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '160,260p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift
printf '\n---\n'
sed -n '460,520p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swiftRepository: manaflow-ai/cmux
Length of output: 6208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '400,470p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift
printf '\n---\n'
sed -n '120,170p' Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swiftRepository: manaflow-ai/cmux
Length of output: 4475
Nonblocking control writes still drop the in-flight command on EAGAIN.
Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift#L25-L31,#L65-L81``:O_NONBLOCKis enabled, but `writeAll(_:)` only retries `EINTR`; a full pipe turns a partial frame into `writeFailed` instead of finishing it later.Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L473-L477: the client treats that as worker loss, so the command is abandoned rather than resumed.
📍 Affects 2 files
Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift#L65-L81(this comment)Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift#L25-L31Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift#L274-L278
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift`
around lines 65 - 81, Update TerminalRenderMessageChannel’s nonblocking write
path at
Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderMessageChannel.swift:65-81,
using the O_NONBLOCK setup at :25-31, to handle EAGAIN/EWOULDBLOCK by waiting
for writability and resuming the same offset until the full frame is sent;
retain EINTR retries and only throw for unrecoverable errors. Update the
corresponding worker-loss handling at
Packages/macOS/CmuxGhosttyRenderService/Sources/CmuxGhosttyRenderClient/GhosttyRenderWorkerClient.swift:274-278
so a transient backpressure condition does not abandon the in-flight command.
Sources: Coding guidelines, Path instructions
Comment on lines
+165
to
+181
| bool valid = message->header.msgh_id == CMUX_TERMINAL_RENDER_FRAME_MESSAGE_ID && | ||
| (message->header.msgh_bits & MACH_MSGH_BITS_COMPLEX) != 0 && | ||
| message->body.msgh_descriptor_count == 1 && | ||
| message->surface_port.type == MACH_MSG_PORT_DESCRIPTOR && | ||
| MACH_PORT_VALID(message->surface_port.name) && | ||
| memcmp( | ||
| message->metadata.authentication_token, | ||
| expected_authentication_token, | ||
| CMUX_TERMINAL_RENDER_TOKEN_LENGTH | ||
| ) == 0; | ||
| if (!valid) { | ||
| if (message->body.msgh_descriptor_count == 1 && | ||
| message->surface_port.type == MACH_MSG_PORT_DESCRIPTOR && | ||
| MACH_PORT_VALID(message->surface_port.name)) { | ||
| mach_port_deallocate(mach_task_self(), message->surface_port.name); | ||
| } | ||
| return MIG_TYPE_ERROR; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 'msgh_size|authentication_token|descriptor_count' \
Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC \
Packages/macOS/CmuxTerminalRenderTransport/TestsRepository: manaflow-ai/cmux
Length of output: 11531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the message/metadata struct layout and the receive path around the check.
sed -n '1,140p' Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/include/TerminalRenderMachIPC.h
printf '\n---\n'
sed -n '132,210p' Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/TerminalRenderMachIPC.c
# Read only: look for any size validation elsewhere in this module.
printf '\n--- size checks ---\n'
rg -n 'msgh_size|MACH_RCV_TOO_LARGE|MIG_TYPE_ERROR|authentication_token' \
Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPCRepository: manaflow-ai/cmux
Length of output: 6575
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the wire-format message definitions to check the exact on-wire layout.
sed -n '1,132p' Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/TerminalRenderMachIPC.c
# Report the calculated sizes of the message pieces from the header file.
python3 - <<'PY'
from pathlib import Path
import re
text = Path('Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/include/TerminalRenderMachIPC.h').read_text()
m = re.search(r'typedef struct \{\s*(.*?)\s*\} cmux_terminal_render_frame_metadata_s;', text, re.S)
print("metadata fields found:", bool(m))
PYRepository: manaflow-ai/cmux
Length of output: 3929
Reject truncated frame messages
Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/TerminalRenderMachIPC.c:165-181
Require header.msgh_size == sizeof(cmux_terminal_render_frame_message_s) before trusting message->metadata. A shorter authenticated message can still pass the token check and leave generation/dimension fields zero-filled in the cleared receive buffer. Keep releasing any received descriptor on failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Sources/TerminalRenderMachIPC/TerminalRenderMachIPC.c`
around lines 165 - 181, Update the validity check in the frame-message receive
validation to require header.msgh_size to equal
sizeof(cmux_terminal_render_frame_message_s) before evaluating or trusting
message->metadata and frame fields. Preserve the existing descriptor cleanup and
MIG_TYPE_ERROR return for all invalid messages.
Comment on lines
+84
to
+91
| @Test func endpointRejectsWrongTokenSize() { | ||
| #expect(throws: TerminalRenderProtocolError.invalidFrameEndpoint) { | ||
| try TerminalRenderFrameEndpoint( | ||
| serviceName: "dev.cmux.test.endpoint", | ||
| authenticationToken: Data(repeating: 0, count: 15) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## file listing\n'
git ls-files 'Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift' \
'Packages/macOS/CmuxTerminalRenderTransport' | sed -n '1,200p'
printf '\n## outline tests file\n'
ast-grep outline Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift --view expanded || true
printf '\n## search endpoint symbols\n'
rg -n "TerminalRenderFrameEndpoint|invalidFrameEndpoint|authenticationToken|serviceName" Packages/macOS/CmuxTerminalRenderTransport -g '!**/.git/**'Repository: manaflow-ai/cmux
Length of output: 8321
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift
printf '\n---\n'
rg -n "struct TerminalRenderFrameEndpoint|enum TerminalRenderProtocolError|init\\(|Decodable|Codable|authenticationToken|serviceName" Packages/macOS/CmuxTerminalRenderTransport -g '!**/.git/**'Repository: manaflow-ai/cmux
Length of output: 10767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift
printf '\n--- endpoint source ---\n'
fd -a 'TerminalRenderFrameEndpoint' Packages/macOS/CmuxTerminalRenderTransport || true
rg -n "TerminalRenderFrameEndpoint" Packages/macOS/CmuxTerminalRenderTransport -g '!**/.git/**'Repository: manaflow-ai/cmux
Length of output: 5105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## TerminalRenderControlProtocolTests.swift'
sed -n '1,240p' Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift
echo
echo '## matching symbols'
rg -n "TerminalRenderFrameEndpoint|invalidFrameEndpoint|authenticationToken|serviceName|Decodable|Codable" Packages/macOS/CmuxTerminalRenderTransport -g '!**/.git/**'Repository: manaflow-ai/cmux
Length of output: 11462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift')
print(p.exists(), p.stat().st_size if p.exists() else 'missing')
if p.exists():
text = p.read_text()
for needle in ['TerminalRenderFrameEndpoint', 'invalidFrameEndpoint', 'Decodable', 'Codable', 'authenticationToken', 'serviceName']:
print(f'-- {needle}:', needle in text)
PYRepository: manaflow-ai/cmux
Length of output: 319
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderControlProtocol.swiftRepository: manaflow-ai/cmux
Length of output: 4068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '120,240p' Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport/TerminalRenderControlProtocol.swiftRepository: manaflow-ai/cmux
Length of output: 4755
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "decodeCommand|decodeEvent|PropertyListDecoder|PropertyListEncoder|JSONDecoder|JSONEncoder|decoder" \
Packages/macOS/CmuxTerminalRenderTransport/Sources/CmuxTerminalRenderTransport \
Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests -g '!**/.git/**'Repository: manaflow-ai/cmux
Length of output: 1748
Add a malformed decode regression for TerminalRenderFrameEndpoint (Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift:84-91). The current case only checks the throwing initializer; add a binary-plist decode with the same keys and a 15-byte token so the Codable path also rejects invalid endpoints.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxTerminalRenderTransport/Tests/CmuxTerminalRenderTransportTests/TerminalRenderControlProtocolTests.swift`
around lines 84 - 91, Add a regression test alongside
endpointRejectsWrongTokenSize that encodes a binary property-list representation
containing the same serviceName and 15-byte authenticationToken, then decodes it
as TerminalRenderFrameEndpoint and asserts invalidFrameEndpoint is thrown. Cover
the Codable decode path while preserving the existing throwing-initializer
assertion.
Source: Coding guidelines
Comment on lines
+4207
to
+4215
| } else if let remoteLayer = layer as? GhosttyRemoteIOSurfaceLayer { | ||
| deferredSurfaceSizeNonMetalRetryCount = 0 | ||
| needsSurfaceSizeRetryAfterMetalLayerRealizes = false | ||
| remoteLayer.updateBackingScaleFactor(layerScale) | ||
| remoteLayer.updateExpectedPixelSize(GhosttyRenderPixelSize( | ||
| width: UInt32(clamping: Int(drawablePixelSize.width)), | ||
| height: UInt32(clamping: Int(drawablePixelSize.height)) | ||
| )) | ||
| lastDrawableSize = drawablePixelSize |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# All writers of the remote layer's expected pixel size + the fitted/pinned size sources.
rg -nP -C3 'updateExpectedPixelSize|updateRemoteRendererExpectedSize|updateRenderMirrorSize' \
Packages/macOS/CmuxTerminal Sources/GhosttyTerminalView.swift
# The fence that consumes expectedPixelSize.
rg -nP -C4 'expectedPixelSize' \
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Hosting/GhosttyRemoteIOSurfaceLayer.swift
# Where fitted/pinned sizes diverge from raw drawable.
rg -nP -C3 'mobileViewportFittedSize|assignedGridPinnedSize' Packages/macOS/CmuxTerminalRepository: manaflow-ai/cmux
Length of output: 24342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '4180,4235p' Sources/GhosttyTerminalView.swift
printf '\n----\n'
sed -n '150,230p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Renderer.swift
printf '\n----\n'
sed -n '1,260p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Sizing.swift
printf '\n----\n'
sed -n '1,260p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+MobileViewportFit.swift
printf '\n----\n'
sed -n '1,220p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Hosting/GhosttyRemoteIOSurfaceLayer.swiftRepository: manaflow-ai/cmux
Length of output: 38030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Update path and its early returns.
sed -n '140,330p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Sizing.swift
printf '\n----\n'
# Mirror-size publication path.
sed -n '160,205p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Renderer.swift
printf '\n----\n'
# Remote layer presentation fence.
sed -n '130,185p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Hosting/GhosttyRemoteIOSurfaceLayer.swift
printf '\n----\n'
# The view-level call site around the contested write.
sed -n '4190,4225p' Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux
Length of output: 16198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Where the render mirror descriptor is created/reset.
rg -n -C3 'renderMirrorDescriptor|updateRemoteRendererExpectedSize|updateRemoteRendererWorkerGeneration|destroyRenderMirror' \
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Renderer.swift \
Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface.swift \
Sources/GhosttyTerminalView.swift
printf '\n----\n'
# Read the mirror creation path around the descriptor initialization.
sed -n '1,160p' Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Renderer.swiftRepository: manaflow-ai/cmux
Length of output: 19508
Use the applied mirror size here, not the raw drawable size
present(_:) fences frames against expectedPixelSize, and updateRenderMirrorSize(...) already owns that value from the worker-applied size. This write can leave the layer expecting the raw backing pixels on capped/pinned resizes that coalesce without a size change, so every worker frame gets rejected until the next real resize. If this is only for bootstrap, seed it from the same applied size instead of the drawable size.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/GhosttyTerminalView.swift` around lines 4207 - 4215, Update the
GhosttyRemoteIOSurfaceLayer branch in present(_:) to pass the worker-applied
mirror size from updateRenderMirrorSize(...) into updateExpectedPixelSize,
rather than constructing it from drawablePixelSize. Preserve the existing
retry-state and backing-scale updates, and ensure bootstrap initialization uses
that same applied size.
Source: Path instructions
Comment on lines
+129
to
+210
| final class GhosttyRenderRuntimeBridge: TerminalRenderWorkerRouting, @unchecked Sendable { | ||
| static let shared = GhosttyRenderRuntimeBridge() | ||
|
|
||
| private let client: GhosttyRenderWorkerClient? | ||
| private let revisionLock = NSLock() | ||
| private var nextConfigurationRevision: UInt64 = 1 | ||
| private var observationStarted = false | ||
|
|
||
| private init() { | ||
| do { | ||
| client = try GhosttyRenderWorkerClient.bundledWorker() | ||
| } catch { | ||
| client = nil | ||
| cmuxDebugLog("ghostty render worker unavailable: \(error)") | ||
| } | ||
| } | ||
|
|
||
| func enqueueRenderCommand(_ command: TerminalRenderWorkerCommand) { | ||
| client?.commandSink.enqueue(command) | ||
| } | ||
|
|
||
| func updateConfiguration(_ config: ghostty_config_t) { | ||
| guard let client else { return } | ||
| let serialized = ghostty_config_serialize(config) | ||
| defer { ghostty_string_free(serialized) } | ||
| guard let bytes = serialized.ptr else { return } | ||
| let contents = String( | ||
| decoding: UnsafeRawBufferPointer(start: bytes, count: Int(serialized.len)), | ||
| as: UTF8.self | ||
| ) | ||
| let state = revisionLock.withLock { () -> (revision: UInt64, startObservation: Bool) in | ||
| defer { nextConfigurationRevision &+= 1 } | ||
| let shouldStart = !observationStarted | ||
| observationStarted = true | ||
| return (nextConfigurationRevision, shouldStart) | ||
| } | ||
| let snapshot = TerminalRenderConfigurationSnapshot( | ||
| revision: state.revision, | ||
| contents: contents | ||
| ) | ||
| if state.startObservation { | ||
| Task { | ||
| let events = await client.subscribeEvents() | ||
| let frames = await client.subscribeFrames() | ||
| await client.updateConfiguration(snapshot) | ||
| async let eventObservation: Void = observeEvents(events) | ||
| async let frameObservation: Void = observeFrames(frames) | ||
| _ = await (eventObservation, frameObservation) | ||
| } | ||
| } else { | ||
| Task { await client.updateConfiguration(snapshot) } | ||
| } | ||
| } | ||
|
|
||
| private func observeEvents( | ||
| _ events: AsyncStream<GhosttyRenderWorkerClientEvent> | ||
| ) async { | ||
| var activeWorkerGeneration: UInt64? | ||
| for await event in events { | ||
| switch event { | ||
| case let .initialized(workerGeneration, _): | ||
| activeWorkerGeneration = workerGeneration | ||
| await MainActor.run { | ||
| for case let surface as TerminalSurface in GhosttyApp.terminalSurfaceRegistry.allSurfaces() { | ||
| surface.renderWorkerDidBecomeReady(workerGeneration: workerGeneration) | ||
| } | ||
| } | ||
| case let .surfaceCreated(surfaceID, _): | ||
| guard let activeWorkerGeneration else { continue } | ||
| await MainActor.run { | ||
| guard let surface = GhosttyApp.terminalSurfaceRegistry.surface(id: surfaceID) | ||
| as? TerminalSurface else { return } | ||
| surface.renderWorkerDidBecomeReady(workerGeneration: activeWorkerGeneration) | ||
| } | ||
| case let .resynchronizationRequired(surfaceID, surfaceGeneration): | ||
| Task { @MainActor in | ||
| guard let surface = GhosttyApp.terminalSurfaceRegistry.surface(id: surfaceID) | ||
| as? TerminalSurface, | ||
| let command = await surface.renderWorkerResynchronizationCommand( | ||
| surfaceGeneration: surfaceGeneration | ||
| ) else { return } | ||
| self.enqueueRenderCommand(command) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Serialize worker startup and observation under one injected owner.
The lock assigns revisions but does not order the independent Task blocks. A later update can launch the worker before the first task installs subscribers, permanently losing .initialized and leaving remote frames generation-rejected.
Use a constructable, injected actor to install subscriptions before processing configuration updates, own a stored cancellable observation task, and serialize revisions/resynchronization. This also removes the new runtime singleton and manual lock.
As per coding guidelines, meaningful tasks must be lifecycle-owned, async state should not be repaired with locks, and new ambient runtime singletons should be replaced by injectable owners.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 155-155: Prefer failable String(bytes:encoding:) initializer when converting Data to String
(optional_data_string_conversion)
[Warning] 129-129: Classes should have an explicit deinit method
(required_deinit)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/TerminalSurfaceRuntimeWiring.swift` around lines 129 - 210, Refactor
GhosttyRenderRuntimeBridge to use a constructable, injected actor owner instead
of the shared singleton and revisionLock. Have the actor install event/frame
subscriptions before applying any configuration, retain and lifecycle-manage the
observation task, and serialize configuration revisions with resynchronization
commands so initialization events cannot be lost. Remove the manual lock and
ambient runtime singleton while preserving existing worker routing behavior.
Sources: Coding guidelines, Path instructions
This was referenced Jul 21, 2026
lawrencecchen
added
the
stale-revisit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
swift testinCmuxTerminalRenderTransport: 7 tests passedswift testinCmuxGhosttyRenderService: 6 tests passed, including a distinct real worker PID and real IOSurface frameswift testinCmuxTerminal: 99 Swift Testing tests and 11 XCTest tests passedGhosttyDrawableSizeRetryTests: 2 tests passed; the new regression test fails before the AppKit root-layer sizing fix./scripts/reload.sh --tag gtprocpassed on the final merged-main headcodesign;otool -Lshows no AppKit, SwiftUI, or UIKit dependencyNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Moves Ghostty rendering into a dedicated, supervised worker process and streams frames over authenticated Mach IPC. Adds crash‑resilient frame presentation and bounds remote resize backlog to prevent frame starvation during rapid resizes.
New Features
cmux-ghostty-render-worker(no AppKit/SwiftUI/UIKit) to host all Ghostty Metal renderers.GhosttyRenderWorkerClient) with a single ordered command sink, nonblocking pipes, and generation fencing.CmuxTerminalRenderTransportfor the control protocol, POSIX message channel, and authenticated Mach transfer ofIOSurfaceframes with metadata.GhosttyRemoteIOSurfaceLayerto present remote frames, retain the last accepted frame, and fence by worker generation.Migration
cmux-ghostty-render-workerinContents/Resources/bin(strip script updated).ghosttysubmodule updated for the external presenter and recovery ABI; ensure macOS 14 build withIOSurface/Securitylinked viaCmuxTerminalRenderTransport.DisabledTerminalRenderWorkerRouterif a worker is not installed.Written for commit fb1f4b3. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation