Repository navigation
Add task manager workspace and terminal jumps - #3471
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThreads CMUX workspace/surface/process identifiers into task‑manager rows, adds view‑workspace/view‑terminal context actions and a graceful+force kill flow, augments process discovery with CMUX scope and PID indexing, annotates top‑level PID/PGID fields, provides a SwiftUI Task Manager view plus command‑palette wiring, and adds two localization keys. ChangesTask Manager Context Menu Navigation & Process Actions
Sequence DiagramsequenceDiagram
actor User
participant RowView as CmuxTaskManagerRowView
participant Model as CmuxTaskManagerModel
participant Snapshot as CmuxTopProcessSnapshot
participant Window as Workspace/Tab Manager
participant System as Darwin (kill)
User->>RowView: tap / open context menu / choose action
RowView->>Model: viewBestTarget / viewWorkspace / viewTerminal / killProcess
Model->>Snapshot: pids(forCMUXSurfaceID:)
alt navigation
Model->>Window: focus workspace/tab/surface (suppressFlash)
Window->>Window: select panel and triggerFlash
else kill flow
Model->>Model: confirmKillProcess (NSAlert)
Model->>System: kill(SIGTERM) for PGIDs/PIDs
System-->>Model: success / errno
Model->>Model: schedule force-kill timer if needed
Model->>System: kill(SIGKILL) survivors after grace
System-->>Model: success / errno
Model->>Snapshot: refresh snapshot
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (4 errors, 1 warning)
✅ Passed checks (8 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 adds workspace/terminal jump navigation to task manager rows, a two-phase Kill Process flow (SIGTERM → 2 s grace → SIGKILL with confirmation), Codex/Claude/OpenCode icons, and a blue-flash after navigation. It also introduces KERN_PROCARGS2-based CMUX environment variable parsing to link arbitrary processes back to their workspace and surface, backed by a pid+start-time keyed scope cache. Confidence Score: 4/5Safe to merge with one known open issue: termination timers are not invalidated in stop(), so closing the Task Manager window during the 2-second kill grace period still fires SIGKILL and restarts the polling loop. The kill flow, scope cache, and navigation logic are well-structured. The termination timers leak past stop() — highlighted in the prior review — remains unresolved in the current code. All other previously raised issues have been fixed. Sources/TaskManagerWindowController.swift — stop() does not invalidate terminationTimers, leaving in-flight force-kill timers running after the window closes. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant V as CmuxTaskManagerRowView
participant M as CmuxTaskManagerModel
participant OS as Darwin/OS
U->>V: Right-click → Kill Process…
V->>M: killProcess(for: row)
M->>M: confirmKillProcess (NSAlert.runModal)
U->>M: Confirms
M->>OS: SIGTERM → gracefulProcessGroupIds
M->>OS: SIGTERM → escalationProcessIds (graceful ∪ killable)
alt All SIGTERMs OK
M->>M: scheduleForceKillIfNeeded (2s Timer)
M->>M: refresh(force: true)
else Partial failures
M->>M: errorMessage = Unable to kill…
M->>M: scheduleForceKillIfNeeded (2s Timer) [if any succeeded]
end
Note over M: 2 second grace period
M->>M: forceKillSurvivors
M->>OS: kill(pid, 0) — isProcessRunning check
M->>OS: SIGKILL → survivors
M->>M: refresh(force: true)
M->>V: snapshot updated (processes gone)
Reviews (9): Last reviewed commit: "Document process scope cache synchroniza..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+113
to
+119
| func viewBestTarget(for row: CmuxTaskManagerRow) { | ||
| if row.canViewTerminal { | ||
| viewTerminal(for: row) | ||
| } else { | ||
| viewWorkspace(for: row) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
viewBestTarget falls through to viewWorkspace whenever canViewTerminal is false, including for window-level rows where workspaceId is also nil. viewWorkspace's guard catches it, so there's no crash, but any tap on a window row silently does nothing. Adding the canViewWorkspace guard here makes the intent explicit and prevents future confusion if the guard in viewWorkspace is ever changed.
Suggested change
| func viewBestTarget(for row: CmuxTaskManagerRow) { | |
| if row.canViewTerminal { | |
| viewTerminal(for: row) | |
| } else { | |
| viewWorkspace(for: row) | |
| } | |
| } | |
| func viewBestTarget(for row: CmuxTaskManagerRow) { | |
| if row.canViewTerminal { | |
| viewTerminal(for: row) | |
| } else if row.canViewWorkspace { | |
| viewWorkspace(for: row) | |
| } | |
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed by guarding viewBestTarget with canViewWorkspace before calling viewWorkspace.
— Claude Code
lawrencecchen
force-pushed
the
task-task-manager-open-workspace-terminal
branch
from
May 4, 2026 03:50
e868bc9 to
5f36d96
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TaskManagerWindowController.swift`:
- Around line 121-132: Add DEBUG-only cmuxDebugLog calls for the new
task-manager navigation: emit an inline UI event where row actions are invoked
(the tap/context-menu handlers that call viewWorkspace(for:) and
viewTerminal(for:)) and emit a second log inside viewWorkspace(for:) and
viewTerminal(for:) immediately before calling manager.focusTab(...). Wrap each
call in `#if` DEBUG / `#endif` and use cmuxDebugLog("...") with clear messages and
tags like "focus.panel" or "tab.select" (include workspaceId and
terminalSurfaceId where available). Ensure logs follow the project's unified
debug log behavior (use CMUXDebugLog.DebugEventLog.shared when appropriate for
dumping) and place calls in Sources/TaskManagerWindowController.swift at the
functions viewWorkspace(for:) and viewTerminal(for:) and at the row action
sites.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d043388-a8de-45bd-ac70-e5989d26796a
📒 Files selected for processing (4)
Resources/Localizable.xcstringsSources/TaskManagerSnapshot.swiftSources/TaskManagerTypes.swiftSources/TaskManagerWindowController.swift
lawrencecchen
force-pushed
the
task-task-manager-open-workspace-terminal
branch
from
May 4, 2026 03:54
5f36d96 to
d9ab634
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
lawrencecchen
force-pushed
the
task-task-manager-open-workspace-terminal
branch
from
May 4, 2026 04:10
d9ab634 to
47fa5e4
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TaskManagerTypes.swift`:
- Around line 63-75: The canKillProcess getter should only be true for concrete
process rows, not aggregates: change canKillProcess to return true only when
this row represents a single process (e.g. processId != nil) and there are
killable IDs, instead of relying solely on killableProcessIds; keep
killableProcessIds as-is but use processId (and/or other row-type indicator) to
scope the destructive action to process rows in TaskManagerWindowController.
In `@Sources/TaskManagerWindowController.swift`:
- Around line 122-127: In viewWorkspace(for:) the code focuses the tab via
manager.focusTab(...) but then flashes row.surfaceId which may not be the active
panel; update the function so the flashed target matches the focused panel:
after getting manager, either call a focus method on the manager to focus the
specific surface (e.g. manager.focusPanel(row.surfaceId)) before calling
flashSelection, or retrieve the workspace’s actual focused surface id from the
manager (e.g. let focusedSurfaceId = manager.currentFocusedSurfaceId ??
row.surfaceId) and pass that to flashSelection(workspaceId:surfaceId:); ensure
any new optional unwrapping follows the existing guard pattern and that
flashSelection is called with the aligned surface id.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 729e406e-cfd6-4a52-a924-d14ee45f3528
📒 Files selected for processing (4)
Resources/Localizable.xcstringsSources/TaskManagerSnapshot.swiftSources/TaskManagerTypes.swiftSources/TaskManagerWindowController.swift
✅ Files skipped from review due to trivial changes (1)
- Resources/Localizable.xcstrings
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/TaskManagerSnapshot.swift
Comment on lines
+110631
to
+110637
| "taskManager.killProcess.error": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { "stringUnit": { "state": "translated", "value": "Unable to kill process %lld: %@" } }, | ||
| "ja": { "stringUnit": { "state": "translated", "value": "プロセス %lld を終了できません: %@" } } | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
taskManager.killProcess.error xcstrings entries use two C-format specifiers (%lld for the PID and %@ for the error message), but the Swift call site builds a single pre-joined detail string and passes it as one String.LocalizationValue interpolation argument. Swift's String(localized:defaultValue:) substitutes positional arguments from the defaultValue interpolation list into the resolved xcstrings template by position. With only one argument supplied and two specifiers in the template, the first specifier (%lld) receives a String where an Int64 is expected, and the second (%@) gets nothing — producing a malformed or literally un-substituted error string in both en and ja locales. The xcstrings entry should use a single %@ to match the one pre-formatted String argument the Swift code actually passes.
Suggested change
| "taskManager.killProcess.error": { | |
| "extractionState": "manual", | |
| "localizations": { | |
| "en": { "stringUnit": { "state": "translated", "value": "Unable to kill process %lld: %@" } }, | |
| "ja": { "stringUnit": { "state": "translated", "value": "プロセス %lld を終了できません: %@" } } | |
| } | |
| }, | |
| "taskManager.killProcess.error": { | |
| "extractionState": "manual", | |
| "localizations": { | |
| "en": { "stringUnit": { "state": "translated", "value": "Unable to kill process: %@" } }, | |
| "ja": { "stringUnit": { "state": "translated", "value": "プロセスを終了できません: %@" } } | |
| } | |
| }, |
Contributor
Author
There was a problem hiding this comment.
Fixed. The error localization now has one %@ placeholder and the call sites format it with the prebuilt error detail string.
— Claude Code
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/CmuxTopSnapshot.swift`:
- Around line 273-288: The code currently skips calling cmuxScope(for:) when
ttyDevice is nil, missing CMUX-only processes; change the assignment so you
always call cmuxScope(for: pid) (remove the ttyDevice == nil ? nil : ...
short-circuit) and let cmuxScope(for:) detect CMUX markers from the process
environment even without a controlling TTY; ensure the returned optional is
still used to populate CmuxTopProcessInfo(cmuxWorkspaceID:, cmuxSurfaceID:) and
that cmuxScope(for:) safely handles processes lacking a TTY.
In `@Sources/TaskManagerWindowController.swift`:
- Around line 178-190: Replace the ad-hoc plural keys
("taskManager.killProcess.pluralTitle" /
"taskManager.killProcess.pluralMessage") with ICU-style plural keys that use the
.one and .other suffixes and supply the process count to the localized call;
specifically, update the branch that sets alert.messageText and
alert.informativeText to use keys like
"taskManager.killProcess.title.one"/".other" and
"taskManager.killProcess.message.one"/".other" (or equivalent naming consistent
with the repo) and call the String(localized: …, defaultValue: …, count:
processIds.count) (or the repository's pluralized localization API) so the
correct plural form is selected based on processIds.count while leaving the
single-process branch behavior intact.
🪄 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: 83714e4f-22db-4b09-9901-e03d0e963902
📒 Files selected for processing (4)
Resources/Localizable.xcstringsSources/CmuxTopSnapshot.swiftSources/TaskManagerWindowController.swiftSources/TerminalControllerTopSupport.swift
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec02017f6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+169
to
+170
| localized: "taskManager.killProcess.message", | ||
| defaultValue: "Send SIGTERM to \(row.title) (PID \(processId))." |
There was a problem hiding this comment.
The new kill-process localization entries in Resources/Localizable.xcstrings use format placeholders (%@, %lld), but this code retrieves them with String(localized:defaultValue:) and no interpolation arguments. Because those keys now exist, the UI will display literal placeholders (for example in the confirmation dialog, and similarly in the error path) instead of the row title/PID details, so users lose critical context before sending SIGTERM. Use localized interpolation or a formatting API that supplies the runtime arguments.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed. The confirmation/error strings now use localized formatting with runtime values, so row titles, PIDs, and error details are substituted instead of showing placeholders.
— Claude Code
Comment on lines
+151
to
+161
| if failures.isEmpty { | ||
| refresh(force: true) | ||
| } else { | ||
| let detail = failures | ||
| .map { "\($0.processId): \($0.reason)" } | ||
| .joined(separator: ", ") | ||
| errorMessage = String( | ||
| localized: "taskManager.killProcess.error", | ||
| defaultValue: "Unable to kill process: \(detail)" | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Darwin.kill call succeeds but others fail, only errorMessage is set and refresh(force: true) is never called. The successfully-killed processes remain visible in the task manager until the next background timer fires (~3 s), leaving the view in a stale state that contradicts what just happened.
Suggested change
| if failures.isEmpty { | |
| refresh(force: true) | |
| } else { | |
| let detail = failures | |
| .map { "\($0.processId): \($0.reason)" } | |
| .joined(separator: ", ") | |
| errorMessage = String( | |
| localized: "taskManager.killProcess.error", | |
| defaultValue: "Unable to kill process: \(detail)" | |
| ) | |
| } | |
| if failures.isEmpty { | |
| refresh(force: true) | |
| } else { | |
| let detail = failures | |
| .map { "\($0.processId): \($0.reason)" } | |
| .joined(separator: ", ") | |
| errorMessage = String( | |
| localized: "taskManager.killProcess.error", | |
| defaultValue: "Unable to kill process: \(detail)" | |
| ) | |
| // Refresh even on partial success so successfully-killed processes | |
| // are no longer shown in the task manager. | |
| if failures.count < processIds.count { | |
| refresh(force: true) | |
| } | |
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed. The task manager now refreshes immediately even when a kill attempt reports failures, so successful parts of a partial kill do not stay stale until the timer.
— Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21b10a442a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let context = rowID(tag, kind: .tag) | ||
| for process in processes { | ||
| appendProcess(process, level: 3, context: context, to: &rows) | ||
| appendProcess(process, level: 3, context: context, workspaceId: workspaceId, terminalSurfaceId: nil, to: &rows) |
There was a problem hiding this comment.
Process rows created under tag groups are always built with terminalSurfaceId: nil, so they can never satisfy canViewTerminal and therefore won't expose “View Terminal” or single-click terminal jump even when the process payload includes cmux_surface_id metadata from the new top snapshot path. This means task-manager navigation silently degrades for tagged agent processes that do belong to a terminal surface; parse and thread the process-level surface ID when no parent terminal surface is provided.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed. Process rows now inherit cmux_surface_id metadata when they are created under tag groups, so tagged agent processes can expose View Terminal and jump to the right surface.
— Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d82e089d42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var sentGracefulSignal = false | ||
| for processGroupId in row.gracefulProcessGroupIds { | ||
| if let reason = sendSignal(SIGTERM, toProcessGroupId: processGroupId) { | ||
| failures.append(("process group \(processGroupId)", reason)) |
There was a problem hiding this comment.
Build the failure target text with localized strings instead of hardcoded English fragments. In the kill flow, failed SIGTERM/SIGKILL entries are recorded as "process group ..." and "PID ...", then interpolated into the localized taskManager.killProcess.error message; in non-English locales this produces mixed-language error dialogs whenever any signal call fails (for example EPERM on one of the selected processes).
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed. Failure targets now use localized labels for process groups and PIDs before they are interpolated into taskManager.killProcess.error.
— Claude Code
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@cmuxTests/TaskManagerResourcesTests.swift`:
- Around line 10-20: The test currently uses a literal [Int] for "pids" so
CmuxTaskManagerResources.intArray(_:) takes the fast Int path and never
exercises the [Any] branch; update the fixture so the payload's "pids" is typed
as [Any] (for example use [101 as Any, 202 as Any] or include a non-Int
representation) so CmuxTaskManagerResources(payload) runs the compactMap(Int)
branch and you still assert resources.processIds == [101, 202].
In `@Sources/TaskManagerWindowController.swift`:
- Around line 154-175: The fallback timer currently only schedules force-kill
for processIds, but the graceful pass also targeted row.gracefulProcessIds so
surviving extra PIDs never get SIGKILL; update the calls to
scheduleForceKillIfNeeded to pass the full gracefulProcessIds set (the same
Array(Set(row.gracefulProcessIds + processIds)).sorted() used above) instead of
just processIds so any PID sent SIGTERM is also included in the force-kill
fallback (refer to gracefulProcessIds, row.gracefulProcessIds and
scheduleForceKillIfNeeded).
🪄 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: 2d18c816-42f5-4c9d-b29a-02647d0b8a8e
📒 Files selected for processing (6)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/TaskManagerTypes.swiftSources/TaskManagerView.swiftSources/TaskManagerWindowController.swiftcmuxTests/TaskManagerResourcesTests.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47db5b5188
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let path = includeProcessDetails ? processPath(pid: pid) : nil | ||
| let rawTTY = Int64(kinfo.kp_eproc.e_tdev) | ||
| let ttyDevice = rawTTY > 0 ? rawTTY : nil | ||
| let cmuxScope = cmuxScope(for: pid) |
There was a problem hiding this comment.
This unconditionally calls cmuxScope(for:) for every PID in the system snapshot, even when process details are disabled. Because Task Manager refreshes repeatedly, this adds one KERN_PROCARGS2 lookup per process per cycle, which can significantly increase CPU and latency on machines with many processes. The scope lookup should be gated or narrowed so periodic sampling does not parse full argv/env for all PIDs.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
Fixed. CMUX scope lookups are now cached by PID and process start time, then pruned to the active process set each sample, so repeated task-manager refreshes do not re-read procargs for every still-running process.
— Claude Code
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Sources/TaskManagerWindowController.swift`:
- Around line 257-260: The SIGKILL failure path currently appends a hardcoded
label "PID \(processId)" to failures; update the loop that iterates over
survivors and calls sendSignal(SIGKILL, toProcessId:) to use the existing helper
processTargetLabel(_:) to produce the target label (so the string is localizable
and consistent with the SIGTERM path), and ensure any new user-facing literal
follows the project convention using String(localized:..., defaultValue:...)
with an appropriate Localizable.xcstrings key if you need to add text.
🪄 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: cfedd232-6019-4f28-9dad-351d95012d7e
📒 Files selected for processing (3)
Resources/Localizable.xcstringsSources/TaskManagerWindowController.swiftcmuxTests/TaskManagerResourcesTests.swift
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@Sources/CmuxTopSnapshot.swift`:
- Around line 322-412: Add unit tests that exercise cmuxScope(fromKernProcArgs:)
with small byte fixtures reproducing the KERN_PROCARGS2 layout and the env-var
fallbacks; build byte arrays that start with a 4-byte argc (Int32
little-endian), a nul-terminated argv[0], then argv strings, then sequences of
nul bytes and environment entries like "CMUX_WORKSPACE_ID=...",
"CMUX_TAB_ID=...", "CMUX_SURFACE_ID=...", and "CMUX_PANEL_ID=..." to verify
workspaceID/surfaceID extraction and fallback behavior; write separate fixtures
for (a) CMUX_WORKSPACE_ID present, (b) only CMUX_TAB_ID present, (c)
CMUX_SURFACE_ID present, (d) only CMUX_PANEL_ID present, and assert
cmuxScope(fromKernProcArgs:) returns the expected CmuxTopProcessScope (use
skipString, skipNulls, and value(inEnvironmentEntry:) behavior as the reference
points).
In `@Sources/CmuxTopSnapshotScopeCache.swift`:
- Around line 1-52: This file implements a reusable, Foundation/Darwin-only
process-scope cache and should be moved out of the app root Sources into its own
SwiftPM target/module; create a new package target (e.g., ProcessScopeCache or
CmuxCore), move the file there, mark types and symbols that need cross-target
visibility as public/internal as appropriate (CmuxTopProcessScopeCacheKey,
CmuxTopProcessScopeCacheValue, cmuxTopScopeCacheLock, cmuxTopScopeCache,
extension on CmuxTopProcessSnapshot, and the functions scopeCacheKey(from:),
cachedCMUXScope(for:cacheKey:), pruneCMUXScopeCache(activeKeys:)), update
imports and module references (including the cmuxScope function consumer) and
add the new target to Package.swift and to cmuxTests so it can be unit-tested
independently.
- Around line 10-42: The current cachedCMUXScope(for:cacheKey:) stores whatever
cmuxScope(for:) returns, including nil; change it to only write to
cmuxTopScopeCache when scope is non-nil. After calling let scope =
cmuxScope(for: pid) check scope != nil and only then acquire
cmuxTopScopeCacheLock and assign cmuxTopScopeCache[cacheKey] =
CmuxTopProcessScopeCacheValue(scope: scope); keep the existing early-read path
and unlock semantics using cmuxTopScopeCacheLock and do not memoize nil results
so transient procargs failures won't overwrite live CMUX markers.
🪄 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: 73d2a69a-7a14-4717-a818-4bc0355166bb
📒 Files selected for processing (4)
GhosttyTabs.xcodeproj/project.pbxprojSources/CmuxTopSnapshot.swiftSources/CmuxTopSnapshotScopeCache.swiftSources/TaskManagerWindowController.swift
Comment on lines
+322
to
+412
| static func cmuxScope(for pid: Int) -> CmuxTopProcessScope? { | ||
| guard pid > 0, pid <= Int(Int32.max) else { return nil } | ||
|
|
||
| var mib: [Int32] = [CTL_KERN, KERN_PROCARGS2, Int32(pid)] | ||
| var size: size_t = 0 | ||
| guard sysctl(&mib, u_int(mib.count), nil, &size, nil, 0) == 0, | ||
| size > MemoryLayout<Int32>.size else { | ||
| return nil | ||
| } | ||
|
|
||
| var buffer = [UInt8](repeating: 0, count: size) | ||
| let success = buffer.withUnsafeMutableBytes { rawBuffer in | ||
| sysctl(&mib, u_int(mib.count), rawBuffer.baseAddress, &size, nil, 0) == 0 | ||
| } | ||
| guard success else { return nil } | ||
|
|
||
| return cmuxScope(fromKernProcArgs: Array(buffer.prefix(Int(size)))) | ||
| } | ||
|
|
||
| private static func cmuxScope(fromKernProcArgs bytes: [UInt8]) -> CmuxTopProcessScope? { | ||
| guard bytes.count > MemoryLayout<Int32>.size else { return nil } | ||
|
|
||
| var argcRaw: Int32 = 0 | ||
| withUnsafeMutableBytes(of: &argcRaw) { rawBuffer in | ||
| rawBuffer.copyBytes(from: bytes.prefix(MemoryLayout<Int32>.size)) | ||
| } | ||
| let argc = Int(Int32(littleEndian: argcRaw)) | ||
| guard argc > 0 else { return nil } | ||
|
|
||
| var index = MemoryLayout<Int32>.size | ||
| skipString(in: bytes, index: &index) | ||
| skipNulls(in: bytes, index: &index) | ||
|
|
||
| for _ in 0..<argc { | ||
| guard index < bytes.count else { return nil } | ||
| skipString(in: bytes, index: &index) | ||
| skipNulls(in: bytes, index: &index) | ||
| } | ||
|
|
||
| var workspaceID: UUID? | ||
| var surfaceID: UUID? | ||
| while index < bytes.count { | ||
| skipNulls(in: bytes, index: &index) | ||
| guard index < bytes.count else { break } | ||
|
|
||
| let start = index | ||
| skipString(in: bytes, index: &index) | ||
| guard start < index, | ||
| let entry = String(bytes: bytes[start..<index], encoding: .utf8) else { | ||
| continue | ||
| } | ||
|
|
||
| if let value = value(inEnvironmentEntry: entry, forKey: "CMUX_WORKSPACE_ID") { | ||
| workspaceID = UUID(uuidString: value) ?? workspaceID | ||
| } else if workspaceID == nil, | ||
| let value = value(inEnvironmentEntry: entry, forKey: "CMUX_TAB_ID") { | ||
| workspaceID = UUID(uuidString: value) | ||
| } else if let value = value(inEnvironmentEntry: entry, forKey: "CMUX_SURFACE_ID") { | ||
| surfaceID = UUID(uuidString: value) ?? surfaceID | ||
| } else if surfaceID == nil, | ||
| let value = value(inEnvironmentEntry: entry, forKey: "CMUX_PANEL_ID") { | ||
| surfaceID = UUID(uuidString: value) | ||
| } | ||
|
|
||
| if workspaceID != nil, surfaceID != nil { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| guard workspaceID != nil || surfaceID != nil else { return nil } | ||
| return CmuxTopProcessScope(workspaceID: workspaceID, surfaceID: surfaceID) | ||
| } | ||
|
|
||
| private static func value(inEnvironmentEntry entry: String, forKey key: String) -> String? { | ||
| let prefix = "\(key)=" | ||
| guard entry.hasPrefix(prefix) else { return nil } | ||
| let value = String(entry.dropFirst(prefix.count)).trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return value.isEmpty ? nil : value | ||
| } | ||
|
|
||
| private static func skipString(in bytes: [UInt8], index: inout Int) { | ||
| while index < bytes.count, bytes[index] != 0 { | ||
| index += 1 | ||
| } | ||
| } | ||
|
|
||
| private static func skipNulls(in bytes: [UInt8], index: inout Int) { | ||
| while index < bytes.count, bytes[index] == 0 { | ||
| index += 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add byte-fixture coverage for the procargs parser.
This path now depends on the exact KERN_PROCARGS2 layout plus CMUX_TAB_ID/CMUX_PANEL_ID fallback behavior. A small fixture-based test around cmuxScope(fromKernProcArgs:) would lock down the detached-session jump/kill path and catch silent env-parsing regressions.
Based on learnings: When a user says tests missed a bug, add or adjust behavior-level coverage around the exact repro path before claiming the fix is complete.
🤖 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/CmuxTopSnapshot.swift` around lines 322 - 412, Add unit tests that
exercise cmuxScope(fromKernProcArgs:) with small byte fixtures reproducing the
KERN_PROCARGS2 layout and the env-var fallbacks; build byte arrays that start
with a 4-byte argc (Int32 little-endian), a nul-terminated argv[0], then argv
strings, then sequences of nul bytes and environment entries like
"CMUX_WORKSPACE_ID=...", "CMUX_TAB_ID=...", "CMUX_SURFACE_ID=...", and
"CMUX_PANEL_ID=..." to verify workspaceID/surfaceID extraction and fallback
behavior; write separate fixtures for (a) CMUX_WORKSPACE_ID present, (b) only
CMUX_TAB_ID present, (c) CMUX_SURFACE_ID present, (d) only CMUX_PANEL_ID
present, and assert cmuxScope(fromKernProcArgs:) returns the expected
CmuxTopProcessScope (use skipString, skipNulls, and value(inEnvironmentEntry:)
behavior as the reference points).
Contributor
Author
There was a problem hiding this comment.
Fixed in e9b1099. Added CmuxTopSnapshotScopeTests with KERN_PROCARGS2 byte fixtures for CMUX_WORKSPACE_ID, CMUX_TAB_ID fallback, CMUX_SURFACE_ID, and CMUX_PANEL_ID fallback.
— Claude Code
There was a problem hiding this comment.
@lawrencecchen Thanks for adding those fixtures! All four fixture cases — CMUX_WORKSPACE_ID primary, CMUX_TAB_ID fallback, CMUX_SURFACE_ID primary, and CMUX_PANEL_ID fallback — are exactly what's needed to lock down the env-var parsing path.
(ﾉ◕ヮ◕)ﾉ*:･ﾟ✧
🧠 Learnings used
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2964
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-17T21:35:25.493Z
Learning: In this repo’s shell session resume flow, always build resume commands using the cwd guard helper exposed by SessionEntry (e.g., resumeCommandWithCwd). The helper should produce a command of the form `cd <shell-quoted cwd> && <resumeCommand>`. Update all call sites that generate “resume” commands (e.g., clipboard actions, drag-drop terminal, and in-app resume) to use this helper so that rc files and newly spawned shells cannot start outside the intended directory. Avoid constructing resume commands directly without the guarded `cd` + shell-quoting + `&&` composition.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:13:36.833Z
Learning: In manaflow-ai/cmux, note that Sources/RestorableAgentSession.swift’s `SessionRestorableAgentSnapshot.resumeCommand` already includes a cwd guard when `workingDirectory` is present (it returns a string like `cd <shell-quoted cwd> && <resumeCommand>`). At call sites (e.g., `Workspace.createPanel(...)`), pass `.resumeCommand` through directly and do not prepend another `cd`/cwd guard or wrap it with an additional `cd <...> &&`—otherwise the working directory may be applied twice or incorrectly.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:14:04.901Z
Learning: In the cmux Swift sources, when restoring a restorable agent session, call sites should pass `resumeCommand` directly (ensuring it has the expected trailing newline if required) to `sendInputWhenReady`. Do not wrap `resumeCommand` with an additional `cd '<cwd>' && ...` guard, because `SessionRestorableAgentSnapshot.resumeCommand` already returns a `cwd`-guarded command; adding another guard can result in `double-cd`. Apply this especially along restore paths (e.g., `Sources/Workspace.swift` restore logic) whenever using `resumeCommand`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3084
File: Sources/AppDelegate.swift:4958-4975
Timestamp: 2026-04-22T11:37:36.238Z
Learning: In Swift code, when re-registering or updating an existing window/session context (e.g., in AppDelegate.registerMainWindow or similar flows), only update an existing *cmuxConfigStore* (or equivalent per-window configuration store) if the incoming configuration/store value is non-nil. Do not overwrite an existing per-window store with nil, so the previous per-window configuration is preserved across re-registration paths.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3128
File: Sources/Panels/BrowserPanelView.swift:4271-4290
Timestamp: 2026-04-23T11:23:49.934Z
Learning: In OmnibarSuggestionsView (and other omnibar-related debug/telemetry logging), never log raw omnibar suggestion content (e.g., URLs, titles, queries). Instead, log only non-sensitive metadata such as suggestion kind/category and the byte length of the text (e.g., "browser.suggestionClick kind=<kind> textBytes=<len>"). Apply this rule consistently to all omnibar-related debug logs to avoid leaking user/search data.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3046
File: Sources/TerminalController.swift:2096-2136
Timestamp: 2026-04-24T22:17:52.550Z
Learning: In Swift request/JSON handlers (e.g., v2 JSON-socket handlers) in Sources, prefer using the v2 helpers for numeric parsing—use `v2Int(params, "<key>")` for normal integer inputs and `v2StrictInt(...)` when strictness is required—rather than casting with `as? Int`. JSONSerialization may yield NSNumber/Double for numeric fields, so v2Int/v2StrictInt ensures correct extraction and type handling. If parsing is used for safety (e.g., timeouts), clamp/validate the parsed value as appropriate (as in `vm.exec` parsing `timeout_ms` and enforcing `>= 1`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3166
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-27T06:58:21.434Z
Learning: In the Swift UI code (e.g., Sources/ContentView.swift), when `preferLiquidGlass`/`materialPolicy.preferLiquidGlass` is enabled for sidebar Liquid Glass, only omit the `Color` tint overlay if `NSGlassEffectView` (native Liquid Glass) is actually available. Compute `usingNativeLiquidGlass = materialPolicy.preferLiquidGlass && SidebarVisualEffectBackground.liquidGlassAvailable`, and when `usingNativeLiquidGlass` is false keep the overlay so the non-native `NSVisualEffectView` fallback still receives the configured tint.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3182
File: Sources/ContentView.swift:10850-10875
Timestamp: 2026-04-27T10:11:36.830Z
Learning: When computing NSTextView content height for NSTextView-based editors (e.g., a method like naturalDocumentHeight(...)), account for trailing newline layout. Specifically, include `layoutManager.extraLineFragmentRect.height` in the measured height only when `extraLineFragmentTextContainer == textContainer`. If you don’t, the caret on the final blank line can be clipped. Apply this rule to future NSTextView-based editors in this repo.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3139
File: Sources/Panels/FilePreviewPanel.swift:534-537
Timestamp: 2026-04-28T05:45:32.192Z
Learning: When implementing workspace/panel teardown or close-confirmation logic in Sources (e.g., close/collapse/workspace-close flows), rely on the shared dirty-state gate driven by the panel’s `isDirty` property rather than adding panel-specific teardown special-casing. Ensure each panel (including `FilePreviewPanel`) exposes and keeps its `isDirty` state up to date (e.g., via `Published private(set) var isDirty` and any subscriptions/synchronization logic), so the generic `panel.isDirty` check correctly covers all panel types during teardown.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3218
File: Sources/AppDelegate.swift:4800-4802
Timestamp: 2026-04-28T11:43:53.356Z
Learning: In Swift code that selects or activates the next main-window context (e.g., iterating window-context collections), avoid iterating `mainWindowContexts.values` directly while calling `resolvedWindow(for:)`. Since `resolvedWindow(for:)` may reindex/mutate `mainWindowContexts`, this can cause mutation-during-enumeration issues. Instead, snapshot first with `Array(mainWindowContexts.values)`, then resolve/reindex against that snapshot, and only then call `activateMainWindowContext(_:)` using the resolved result.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3247
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-29T01:08:39.652Z
Learning: Maintain the behavior contract for “Copy Workspace ID(s)”: when triggered from the sidebar context menu (e.g., in Sources/ContentView.swift TabItemView), the command must copy plain UUIDs (IDs-only), not references/refs. For command palette identifier-copy commands where refs are required, ensure the implementation explicitly passes includeRefs: true. This preserves backward compatibility for scripts expecting UUID-only output while allowing the palette to return richer payloads when needed.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3256
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-04-29T01:20:59.683Z
Learning: In this repo’s Swift implementation, keep browser-creation/open behavior consistent when `BrowserAvailabilitySettings` is disabled. For both V1 and V2, any command that includes a URL when creating/opening a browser (e.g., `open_browser` with a URL, `v2 surface.create` with `type=browser` and `url`, `browser.tab.new` with `url`, `v2 browser.open_split` with `url`) must open the URL externally using `NSWorkspace.shared.open(...)` and return appropriate success metadata. Only URL-less/blank browser creations should fail with the `browser_disabled` error.
Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3430
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-02T06:07:12.997Z
Learning: In this repo’s SwiftUI views, any view placed under LazyVStack, LazyHStack, List, or ForEach must not capture or hold ObservableObject store instances (e.g., a TabManager). Instead, pass immutable value snapshots (e.g., currentSelectedTabId, sidebarIndexForTabId) plus action closures (e.g., moveToExistingWorkspace, moveToNewWorkspace). Prefer refactoring child view APIs to accept the needed values/closures rather than an ObservableObject reference (e.g., SidebarBonsplitTabWorkspaceDropOverlay should take closures instead of a TabManager).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3480
File: Sources/GhosttyTerminalView.swift:0-0
Timestamp: 2026-05-04T05:31:52.905Z
Learning: In this repo’s Swift sources, keep “surface-scoped” Ghostty config reloads strictly scoped to the target surface. Specifically, GhosttyApp.reloadSurfaceConfiguration(_:soft:source:) should update only the surface via ghostty_surface_update_config and invalidate GhosttyConfig’s load cache, but it must not replace or promote the per-surface config into GhosttyApp’s app-level config/cache (e.g., it must not overwrite GhosttyApp.config or modify app-level cached state). App-level helpers like scrollbarVisibility() and focusFollowsMouseEnabled() must continue to read GhosttyApp.config until a full app reload path is taken.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3502
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-04T11:09:27.707Z
Learning: This repo’s CI enforces a Swift file-length budget for large view files (e.g., Sources/ContentView.swift). When adding helper views/small components, avoid bloating the existing file: extract the subview into a dedicated Swift file under Sources (e.g., Sources/SidebarScrim.swift) and keep it under the CI length threshold. Use access control deliberately: if a extracted view/type must be referenced from other files, do not mark it `private` (use `internal` by omitting `private`); only use `private` for declarations that are truly local to the same file.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3471
File: Sources/CmuxTopSnapshotScopeCache.swift:1-54
Timestamp: 2026-05-05T02:19:22.055Z
Learning: In this repo (manafow-ai/cmux), `CmuxTopProcessSnapshot` and `CmuxTopProcessScope` are app-internal types defined under `Sources/`.
When reviewing Swift files under `Sources/`, do not recommend extracting/creating a separate SwiftPM package for extensions over these types (e.g., `CmuxTopSnapshotScopeCache.swift`). Only suggest SwiftPM extraction if the repo introduces a dedicated process-inspection package (e.g., a new `MuxCore`/`process-inspection`-style package) rather than trying to extract app-internal extensions in isolation.
Comment on lines
+1
to
+52
| import Foundation | ||
| import Darwin | ||
|
|
||
| struct CmuxTopProcessScopeCacheKey: Hashable { | ||
| let pid: Int | ||
| let startSeconds: Int | ||
| let startMicroseconds: Int | ||
| } | ||
|
|
||
| private struct CmuxTopProcessScopeCacheValue { | ||
| let scope: CmuxTopProcessScope? | ||
| } | ||
|
|
||
| private let cmuxTopScopeCacheLock = NSLock() | ||
| private var cmuxTopScopeCache: [CmuxTopProcessScopeCacheKey: CmuxTopProcessScopeCacheValue] = [:] | ||
|
|
||
| extension CmuxTopProcessSnapshot { | ||
| static func scopeCacheKey(from kinfo: kinfo_proc) -> CmuxTopProcessScopeCacheKey { | ||
| let startTime = kinfo.kp_proc.p_un.__p_starttime | ||
| return CmuxTopProcessScopeCacheKey( | ||
| pid: Int(kinfo.kp_proc.p_pid), | ||
| startSeconds: Int(startTime.tv_sec), | ||
| startMicroseconds: Int(startTime.tv_usec) | ||
| ) | ||
| } | ||
|
|
||
| static func cachedCMUXScope( | ||
| for pid: Int, | ||
| cacheKey: CmuxTopProcessScopeCacheKey | ||
| ) -> CmuxTopProcessScope? { | ||
| cmuxTopScopeCacheLock.lock() | ||
| if let cached = cmuxTopScopeCache[cacheKey] { | ||
| cmuxTopScopeCacheLock.unlock() | ||
| return cached.scope | ||
| } | ||
| cmuxTopScopeCacheLock.unlock() | ||
|
|
||
| let scope = cmuxScope(for: pid) | ||
|
|
||
| cmuxTopScopeCacheLock.lock() | ||
| cmuxTopScopeCache[cacheKey] = CmuxTopProcessScopeCacheValue(scope: scope) | ||
| cmuxTopScopeCacheLock.unlock() | ||
|
|
||
| return scope | ||
| } | ||
|
|
||
| static func pruneCMUXScopeCache(activeKeys: Set<CmuxTopProcessScopeCacheKey>) { | ||
| cmuxTopScopeCacheLock.lock() | ||
| cmuxTopScopeCache = cmuxTopScopeCache.filter { activeKeys.contains($0.key) } | ||
| cmuxTopScopeCacheLock.unlock() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move this process-scope cache behind a SwiftPM target.
This file is pure Foundation/Darwin domain logic and doesn't depend on app lifecycle concerns. Keeping it in Sources/ continues to couple task-manager process discovery to the app target instead of a reusable, unit-testable package boundary.
As per coding guidelines: {Sources,CLI,Packages,cmuxTests,cmuxUITests}/**/*.swift: Do not implement feature logic that is independent of cmux app lifecycle directly in the app target/module's root Sources/ path when it can compile/test without AppKit, SwiftUI view state, Ghostty globals, or process-wide singletons—extract it behind a SwiftPM package boundary instead.
🤖 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/CmuxTopSnapshotScopeCache.swift` around lines 1 - 52, This file
implements a reusable, Foundation/Darwin-only process-scope cache and should be
moved out of the app root Sources into its own SwiftPM target/module; create a
new package target (e.g., ProcessScopeCache or CmuxCore), move the file there,
mark types and symbols that need cross-target visibility as public/internal as
appropriate (CmuxTopProcessScopeCacheKey, CmuxTopProcessScopeCacheValue,
cmuxTopScopeCacheLock, cmuxTopScopeCache, extension on CmuxTopProcessSnapshot,
and the functions scopeCacheKey(from:), cachedCMUXScope(for:cacheKey:),
pruneCMUXScopeCache(activeKeys:)), update imports and module references
(including the cmuxScope function consumer) and add the new target to
Package.swift and to cmuxTests so it can be unit-tested independently.
Contributor
Author
There was a problem hiding this comment.
Not moving this in this PR. The cache is a small extension over app-internal CmuxTopProcessSnapshot/CmuxTopProcessScope types, and this repo does not currently have a process-inspection SwiftPM package; creating one here would broaden the change beyond the task-manager review fix.
— Claude Code
There was a problem hiding this comment.
@lawrencecchen, understood — since CmuxTopProcessSnapshot and CmuxTopProcessScope are app-internal types and no process-inspection SwiftPM package exists yet, keeping the cache extension in Sources/ alongside those types is the right call for this PR's scope. Happy to revisit the extraction if a broader process-inspection module ever gets introduced.
✏️ Learnings added
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3471
File: Sources/CmuxTopSnapshotScopeCache.swift:1-54
Timestamp: 2026-05-05T02:19:22.055Z
Learning: In manaflow-ai/cmux, `CmuxTopProcessSnapshot` and `CmuxTopProcessScope` are app-internal types that live in `Sources/`. Extensions over these types (e.g., `CmuxTopSnapshotScopeCache.swift`) should remain in `Sources/` rather than being flagged for SwiftPM extraction, because no standalone process-inspection package exists in this repo. Only suggest SwiftPM extraction if a dedicated process-inspection/CmuxCore package is created.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2964
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-17T21:35:25.493Z
Learning: In this repo’s shell session resume flow, always build resume commands using the cwd guard helper exposed by SessionEntry (e.g., resumeCommandWithCwd). The helper should produce a command of the form `cd <shell-quoted cwd> && <resumeCommand>`. Update all call sites that generate “resume” commands (e.g., clipboard actions, drag-drop terminal, and in-app resume) to use this helper so that rc files and newly spawned shells cannot start outside the intended directory. Avoid constructing resume commands directly without the guarded `cd` + shell-quoting + `&&` composition.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:13:36.833Z
Learning: In manaflow-ai/cmux, note that Sources/RestorableAgentSession.swift’s `SessionRestorableAgentSnapshot.resumeCommand` already includes a cwd guard when `workingDirectory` is present (it returns a string like `cd <shell-quoted cwd> && <resumeCommand>`). At call sites (e.g., `Workspace.createPanel(...)`), pass `.resumeCommand` through directly and do not prepend another `cd`/cwd guard or wrap it with an additional `cd <...> &&`—otherwise the working directory may be applied twice or incorrectly.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:14:04.901Z
Learning: In the cmux Swift sources, when restoring a restorable agent session, call sites should pass `resumeCommand` directly (ensuring it has the expected trailing newline if required) to `sendInputWhenReady`. Do not wrap `resumeCommand` with an additional `cd '<cwd>' && ...` guard, because `SessionRestorableAgentSnapshot.resumeCommand` already returns a `cwd`-guarded command; adding another guard can result in `double-cd`. Apply this especially along restore paths (e.g., `Sources/Workspace.swift` restore logic) whenever using `resumeCommand`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3084
File: Sources/AppDelegate.swift:4958-4975
Timestamp: 2026-04-22T11:37:36.238Z
Learning: In Swift code, when re-registering or updating an existing window/session context (e.g., in AppDelegate.registerMainWindow or similar flows), only update an existing *cmuxConfigStore* (or equivalent per-window configuration store) if the incoming configuration/store value is non-nil. Do not overwrite an existing per-window store with nil, so the previous per-window configuration is preserved across re-registration paths.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3128
File: Sources/Panels/BrowserPanelView.swift:4271-4290
Timestamp: 2026-04-23T11:23:49.934Z
Learning: In OmnibarSuggestionsView (and other omnibar-related debug/telemetry logging), never log raw omnibar suggestion content (e.g., URLs, titles, queries). Instead, log only non-sensitive metadata such as suggestion kind/category and the byte length of the text (e.g., "browser.suggestionClick kind=<kind> textBytes=<len>"). Apply this rule consistently to all omnibar-related debug logs to avoid leaking user/search data.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3046
File: Sources/TerminalController.swift:2096-2136
Timestamp: 2026-04-24T22:17:52.550Z
Learning: In Swift request/JSON handlers (e.g., v2 JSON-socket handlers) in Sources, prefer using the v2 helpers for numeric parsing—use `v2Int(params, "<key>")` for normal integer inputs and `v2StrictInt(...)` when strictness is required—rather than casting with `as? Int`. JSONSerialization may yield NSNumber/Double for numeric fields, so v2Int/v2StrictInt ensures correct extraction and type handling. If parsing is used for safety (e.g., timeouts), clamp/validate the parsed value as appropriate (as in `vm.exec` parsing `timeout_ms` and enforcing `>= 1`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3166
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-27T06:58:21.434Z
Learning: In the Swift UI code (e.g., Sources/ContentView.swift), when `preferLiquidGlass`/`materialPolicy.preferLiquidGlass` is enabled for sidebar Liquid Glass, only omit the `Color` tint overlay if `NSGlassEffectView` (native Liquid Glass) is actually available. Compute `usingNativeLiquidGlass = materialPolicy.preferLiquidGlass && SidebarVisualEffectBackground.liquidGlassAvailable`, and when `usingNativeLiquidGlass` is false keep the overlay so the non-native `NSVisualEffectView` fallback still receives the configured tint.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3182
File: Sources/ContentView.swift:10850-10875
Timestamp: 2026-04-27T10:11:36.830Z
Learning: When computing NSTextView content height for NSTextView-based editors (e.g., a method like naturalDocumentHeight(...)), account for trailing newline layout. Specifically, include `layoutManager.extraLineFragmentRect.height` in the measured height only when `extraLineFragmentTextContainer == textContainer`. If you don’t, the caret on the final blank line can be clipped. Apply this rule to future NSTextView-based editors in this repo.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3139
File: Sources/Panels/FilePreviewPanel.swift:534-537
Timestamp: 2026-04-28T05:45:32.192Z
Learning: When implementing workspace/panel teardown or close-confirmation logic in Sources (e.g., close/collapse/workspace-close flows), rely on the shared dirty-state gate driven by the panel’s `isDirty` property rather than adding panel-specific teardown special-casing. Ensure each panel (including `FilePreviewPanel`) exposes and keeps its `isDirty` state up to date (e.g., via `Published private(set) var isDirty` and any subscriptions/synchronization logic), so the generic `panel.isDirty` check correctly covers all panel types during teardown.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3218
File: Sources/AppDelegate.swift:4800-4802
Timestamp: 2026-04-28T11:43:53.356Z
Learning: In Swift code that selects or activates the next main-window context (e.g., iterating window-context collections), avoid iterating `mainWindowContexts.values` directly while calling `resolvedWindow(for:)`. Since `resolvedWindow(for:)` may reindex/mutate `mainWindowContexts`, this can cause mutation-during-enumeration issues. Instead, snapshot first with `Array(mainWindowContexts.values)`, then resolve/reindex against that snapshot, and only then call `activateMainWindowContext(_:)` using the resolved result.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3247
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-29T01:08:39.652Z
Learning: Maintain the behavior contract for “Copy Workspace ID(s)”: when triggered from the sidebar context menu (e.g., in Sources/ContentView.swift TabItemView), the command must copy plain UUIDs (IDs-only), not references/refs. For command palette identifier-copy commands where refs are required, ensure the implementation explicitly passes includeRefs: true. This preserves backward compatibility for scripts expecting UUID-only output while allowing the palette to return richer payloads when needed.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3256
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-04-29T01:20:59.683Z
Learning: In this repo’s Swift implementation, keep browser-creation/open behavior consistent when `BrowserAvailabilitySettings` is disabled. For both V1 and V2, any command that includes a URL when creating/opening a browser (e.g., `open_browser` with a URL, `v2 surface.create` with `type=browser` and `url`, `browser.tab.new` with `url`, `v2 browser.open_split` with `url`) must open the URL externally using `NSWorkspace.shared.open(...)` and return appropriate success metadata. Only URL-less/blank browser creations should fail with the `browser_disabled` error.
Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3430
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-02T06:07:12.997Z
Learning: In this repo’s SwiftUI views, any view placed under LazyVStack, LazyHStack, List, or ForEach must not capture or hold ObservableObject store instances (e.g., a TabManager). Instead, pass immutable value snapshots (e.g., currentSelectedTabId, sidebarIndexForTabId) plus action closures (e.g., moveToExistingWorkspace, moveToNewWorkspace). Prefer refactoring child view APIs to accept the needed values/closures rather than an ObservableObject reference (e.g., SidebarBonsplitTabWorkspaceDropOverlay should take closures instead of a TabManager).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3480
File: Sources/GhosttyTerminalView.swift:0-0
Timestamp: 2026-05-04T05:31:52.905Z
Learning: In this repo’s Swift sources, keep “surface-scoped” Ghostty config reloads strictly scoped to the target surface. Specifically, GhosttyApp.reloadSurfaceConfiguration(_:soft:source:) should update only the surface via ghostty_surface_update_config and invalidate GhosttyConfig’s load cache, but it must not replace or promote the per-surface config into GhosttyApp’s app-level config/cache (e.g., it must not overwrite GhosttyApp.config or modify app-level cached state). App-level helpers like scrollbarVisibility() and focusFollowsMouseEnabled() must continue to read GhosttyApp.config until a full app reload path is taken.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3502
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-04T11:09:27.707Z
Learning: This repo’s CI enforces a Swift file-length budget for large view files (e.g., Sources/ContentView.swift). When adding helper views/small components, avoid bloating the existing file: extract the subview into a dedicated Swift file under Sources (e.g., Sources/SidebarScrim.swift) and keep it under the CI length threshold. Use access control deliberately: if a extracted view/type must be referenced from other files, do not mark it `private` (use `internal` by omitting `private`); only use `private` for declarations that are truly local to the same file.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Sources/CmuxTopSnapshotScopeCache.swift`:
- Around line 14-15: Replace the global NSLock-guarded cache with the
repository-approved Swift concurrency primitive: remove cmuxTopScopeCacheLock
and the global cmuxTopScopeCache, and encapsulate the cache storage and access
inside a concurrency-safe component (e.g., an actor or the project’s existing
cache abstraction) that owns a [CmuxTopProcessScopeCacheKey:
CmuxTopProcessScopeCacheValue]; update all call sites to interact with that
actor/abstraction (e.g., await actor.method(...) or use the existing cache API)
so no manual NSLock usage remains and access is serialized via Swift
concurrency.
🪄 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: 876ffe6c-ccc3-4f13-8cb4-886c95e0b310
📒 Files selected for processing (4)
GhosttyTabs.xcodeproj/project.pbxprojSources/CmuxTopSnapshot.swiftSources/CmuxTopSnapshotScopeCache.swiftcmuxTests/CmuxTopSnapshotScopeTests.swift
Comment on lines
+14
to
+15
| private let cmuxTopScopeCacheLock = NSLock() | ||
| private var cmuxTopScopeCache: [CmuxTopProcessScopeCacheKey: CmuxTopProcessScopeCacheValue] = [:] |
There was a problem hiding this comment.
Avoid introducing an NSLock-guarded global cache here.
This adds lock-based shared mutable state in production Swift, and the repo’s Swift lint explicitly treats locking as a disallowed runtime pattern. Please move this cache behind the concurrency mechanism already approved for this codebase instead of manual NSLock management.
As per coding guidelines, .coderabbit.yaml: CI review bot will run the cmux custom Swift lint checks for all **/*.swift changes and can error on “blocking runtime patterns (semaphores/sleeps/polling/locking)”.
Also applies to: 31-52
🤖 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/CmuxTopSnapshotScopeCache.swift` around lines 14 - 15, Replace the
global NSLock-guarded cache with the repository-approved Swift concurrency
primitive: remove cmuxTopScopeCacheLock and the global cmuxTopScopeCache, and
encapsulate the cache storage and access inside a concurrency-safe component
(e.g., an actor or the project’s existing cache abstraction) that owns a
[CmuxTopProcessScopeCacheKey: CmuxTopProcessScopeCacheValue]; update all call
sites to interact with that actor/abstraction (e.g., await actor.method(...) or
use the existing cache API) so no manual NSLock usage remains and access is
serialized via Swift concurrency.
Contributor
Author
There was a problem hiding this comment.
Addressed in 395ee82 by documenting why this cache uses a tiny synchronous critical section: CmuxTopProcessSnapshot.capture backs both async task-manager sampling and synchronous v2 system.top socket handling. The lock is only held around dictionary reads/writes; procargs/sysctl work stays outside it, which matches the repo rule's low-level bridge exception.
— Claude Code
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the commit 395ee82dc for changes to CmuxTopSnapshotScopeCache.swift
git show 395ee82dc -- Sources/CmuxTopSnapshotScopeCache.swift 2>/dev/null || git log --oneline | head -20
Length of output: 957
@lawrencecchen Thanks for the context. The commit clearly documents the rationale — since CmuxTopProcessSnapshot.capture must remain synchronous to serve both the async task-manager sampling path and the sync v2 system.top socket handler, await-based actor isolation isn't viable here. Keeping the NSLock critical section narrowly scoped to dictionary reads/writes (with procargs/sysctl outside it) is a clean fit for the low-level bridge exception. The documentation in place makes the intent clear for future reviewers. ✅
✏️ Learnings added
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3471
File: Sources/CmuxTopSnapshotScopeCache.swift:18-19
Timestamp: 2026-05-05T02:32:38.143Z
Learning: In manaflow-ai/cmux, `CmuxTopProcessSnapshot.capture` is intentionally synchronous because it backs both async task-manager sampling and the synchronous v2 `system.top` socket handler. For `CmuxTopSnapshotScopeCache.swift`, using `NSLock` around dictionary reads/writes (with `procargs`/`sysctl` work kept outside the critical section) is the approved low-level bridge exception to the repo's general no-locking lint rule. Do not flag this `NSLock` usage as a violation; the accompanying comment documents the exception.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form `#RRGGBB` (no alpha, i.e., do not allow `#RRGGBBAA`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2964
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-17T21:35:25.493Z
Learning: In this repo’s shell session resume flow, always build resume commands using the cwd guard helper exposed by SessionEntry (e.g., resumeCommandWithCwd). The helper should produce a command of the form `cd <shell-quoted cwd> && <resumeCommand>`. Update all call sites that generate “resume” commands (e.g., clipboard actions, drag-drop terminal, and in-app resume) to use this helper so that rc files and newly spawned shells cannot start outside the intended directory. Avoid constructing resume commands directly without the guarded `cd` + shell-quoting + `&&` composition.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:13:36.833Z
Learning: In manaflow-ai/cmux, note that Sources/RestorableAgentSession.swift’s `SessionRestorableAgentSnapshot.resumeCommand` already includes a cwd guard when `workingDirectory` is present (it returns a string like `cd <shell-quoted cwd> && <resumeCommand>`). At call sites (e.g., `Workspace.createPanel(...)`), pass `.resumeCommand` through directly and do not prepend another `cd`/cwd guard or wrap it with an additional `cd <...> &&`—otherwise the working directory may be applied twice or incorrectly.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:14:04.901Z
Learning: In the cmux Swift sources, when restoring a restorable agent session, call sites should pass `resumeCommand` directly (ensuring it has the expected trailing newline if required) to `sendInputWhenReady`. Do not wrap `resumeCommand` with an additional `cd '<cwd>' && ...` guard, because `SessionRestorableAgentSnapshot.resumeCommand` already returns a `cwd`-guarded command; adding another guard can result in `double-cd`. Apply this especially along restore paths (e.g., `Sources/Workspace.swift` restore logic) whenever using `resumeCommand`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3084
File: Sources/AppDelegate.swift:4958-4975
Timestamp: 2026-04-22T11:37:36.238Z
Learning: In Swift code, when re-registering or updating an existing window/session context (e.g., in AppDelegate.registerMainWindow or similar flows), only update an existing *cmuxConfigStore* (or equivalent per-window configuration store) if the incoming configuration/store value is non-nil. Do not overwrite an existing per-window store with nil, so the previous per-window configuration is preserved across re-registration paths.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3128
File: Sources/Panels/BrowserPanelView.swift:4271-4290
Timestamp: 2026-04-23T11:23:49.934Z
Learning: In OmnibarSuggestionsView (and other omnibar-related debug/telemetry logging), never log raw omnibar suggestion content (e.g., URLs, titles, queries). Instead, log only non-sensitive metadata such as suggestion kind/category and the byte length of the text (e.g., "browser.suggestionClick kind=<kind> textBytes=<len>"). Apply this rule consistently to all omnibar-related debug logs to avoid leaking user/search data.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3046
File: Sources/TerminalController.swift:2096-2136
Timestamp: 2026-04-24T22:17:52.550Z
Learning: In Swift request/JSON handlers (e.g., v2 JSON-socket handlers) in Sources, prefer using the v2 helpers for numeric parsing—use `v2Int(params, "<key>")` for normal integer inputs and `v2StrictInt(...)` when strictness is required—rather than casting with `as? Int`. JSONSerialization may yield NSNumber/Double for numeric fields, so v2Int/v2StrictInt ensures correct extraction and type handling. If parsing is used for safety (e.g., timeouts), clamp/validate the parsed value as appropriate (as in `vm.exec` parsing `timeout_ms` and enforcing `>= 1`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3166
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-27T06:58:21.434Z
Learning: In the Swift UI code (e.g., Sources/ContentView.swift), when `preferLiquidGlass`/`materialPolicy.preferLiquidGlass` is enabled for sidebar Liquid Glass, only omit the `Color` tint overlay if `NSGlassEffectView` (native Liquid Glass) is actually available. Compute `usingNativeLiquidGlass = materialPolicy.preferLiquidGlass && SidebarVisualEffectBackground.liquidGlassAvailable`, and when `usingNativeLiquidGlass` is false keep the overlay so the non-native `NSVisualEffectView` fallback still receives the configured tint.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3182
File: Sources/ContentView.swift:10850-10875
Timestamp: 2026-04-27T10:11:36.830Z
Learning: When computing NSTextView content height for NSTextView-based editors (e.g., a method like naturalDocumentHeight(...)), account for trailing newline layout. Specifically, include `layoutManager.extraLineFragmentRect.height` in the measured height only when `extraLineFragmentTextContainer == textContainer`. If you don’t, the caret on the final blank line can be clipped. Apply this rule to future NSTextView-based editors in this repo.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3139
File: Sources/Panels/FilePreviewPanel.swift:534-537
Timestamp: 2026-04-28T05:45:32.192Z
Learning: When implementing workspace/panel teardown or close-confirmation logic in Sources (e.g., close/collapse/workspace-close flows), rely on the shared dirty-state gate driven by the panel’s `isDirty` property rather than adding panel-specific teardown special-casing. Ensure each panel (including `FilePreviewPanel`) exposes and keeps its `isDirty` state up to date (e.g., via `Published private(set) var isDirty` and any subscriptions/synchronization logic), so the generic `panel.isDirty` check correctly covers all panel types during teardown.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3218
File: Sources/AppDelegate.swift:4800-4802
Timestamp: 2026-04-28T11:43:53.356Z
Learning: In Swift code that selects or activates the next main-window context (e.g., iterating window-context collections), avoid iterating `mainWindowContexts.values` directly while calling `resolvedWindow(for:)`. Since `resolvedWindow(for:)` may reindex/mutate `mainWindowContexts`, this can cause mutation-during-enumeration issues. Instead, snapshot first with `Array(mainWindowContexts.values)`, then resolve/reindex against that snapshot, and only then call `activateMainWindowContext(_:)` using the resolved result.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3247
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-29T01:08:39.652Z
Learning: Maintain the behavior contract for “Copy Workspace ID(s)”: when triggered from the sidebar context menu (e.g., in Sources/ContentView.swift TabItemView), the command must copy plain UUIDs (IDs-only), not references/refs. For command palette identifier-copy commands where refs are required, ensure the implementation explicitly passes includeRefs: true. This preserves backward compatibility for scripts expecting UUID-only output while allowing the palette to return richer payloads when needed.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3256
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-04-29T01:20:59.683Z
Learning: In this repo’s Swift implementation, keep browser-creation/open behavior consistent when `BrowserAvailabilitySettings` is disabled. For both V1 and V2, any command that includes a URL when creating/opening a browser (e.g., `open_browser` with a URL, `v2 surface.create` with `type=browser` and `url`, `browser.tab.new` with `url`, `v2 browser.open_split` with `url`) must open the URL externally using `NSWorkspace.shared.open(...)` and return appropriate success metadata. Only URL-less/blank browser creations should fail with the `browser_disabled` error.
Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3430
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-02T06:07:12.997Z
Learning: In this repo’s SwiftUI views, any view placed under LazyVStack, LazyHStack, List, or ForEach must not capture or hold ObservableObject store instances (e.g., a TabManager). Instead, pass immutable value snapshots (e.g., currentSelectedTabId, sidebarIndexForTabId) plus action closures (e.g., moveToExistingWorkspace, moveToNewWorkspace). Prefer refactoring child view APIs to accept the needed values/closures rather than an ObservableObject reference (e.g., SidebarBonsplitTabWorkspaceDropOverlay should take closures instead of a TabManager).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3480
File: Sources/GhosttyTerminalView.swift:0-0
Timestamp: 2026-05-04T05:31:52.905Z
Learning: In this repo’s Swift sources, keep “surface-scoped” Ghostty config reloads strictly scoped to the target surface. Specifically, GhosttyApp.reloadSurfaceConfiguration(_:soft:source:) should update only the surface via ghostty_surface_update_config and invalidate GhosttyConfig’s load cache, but it must not replace or promote the per-surface config into GhosttyApp’s app-level config/cache (e.g., it must not overwrite GhosttyApp.config or modify app-level cached state). App-level helpers like scrollbarVisibility() and focusFollowsMouseEnabled() must continue to read GhosttyApp.config until a full app reload path is taken.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3502
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-04T11:09:27.707Z
Learning: This repo’s CI enforces a Swift file-length budget for large view files (e.g., Sources/ContentView.swift). When adding helper views/small components, avoid bloating the existing file: extract the subview into a dedicated Swift file under Sources (e.g., Sources/SidebarScrim.swift) and keep it under the CI length threshold. Use access control deliberately: if a extracted view/type must be referenced from other files, do not mark it `private` (use `internal` by omitting `private`); only use `private` for declarations that are truly local to the same file.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3471
File: Sources/CmuxTopSnapshotScopeCache.swift:1-54
Timestamp: 2026-05-05T02:19:22.055Z
Learning: In this repo (manafow-ai/cmux), `CmuxTopProcessSnapshot` and `CmuxTopProcessScope` are app-internal types defined under `Sources/`.
When reviewing Swift files under `Sources/`, do not recommend extracting/creating a separate SwiftPM package for extensions over these types (e.g., `CmuxTopSnapshotScopeCache.swift`). Only suggest SwiftPM extraction if the repo introduces a dedicated process-inspection package (e.g., a new `MuxCore`/`process-inspection`-style package) rather than trying to extract app-internal extensions in isolation.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
lawrencecchen
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot]
May 5, 2026 02:45
Superseded by the latest passing CodeRabbit check and review on 395ee82.
This was referenced May 6, 2026
This was referenced May 13, 2026
This branch was successfully deployed
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
1 participant
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
Testing
jq empty Resources/Localizable.xcstringspassedgit diff --checkpassed./scripts/reload.sh --tag tmjumppassedIssues
Summary by CodeRabbit
New Features
New Actions
Improvements
Tests
Documentation