Repository navigation
Add Show in Finder to workspace sidebar context menu - #3632
Merged
Merged
Conversation
The workspace sidebar already owns directory ordering for row display, so the Finder action reuses that resolver and only converts the first local directory into a Finder URL at context-menu time. Remote workspaces and missing directories leave the action unavailable instead of sending a remote or nil path to Finder. Constraint: Do not run local tests or direct xcodebuild; CI and the required tagged reload are the allowed verification paths Constraint: SSH workspaces can expose remote cwd strings that must not be opened through local Finder Rejected: Thread a cwd through TabItemView | existing Workspace.sidebarDirectoriesInDisplayOrder() already centralizes row directory resolution Rejected: Add a grep-style context-menu regression test | project test policy requires observable behavior, and the nearby test is source-text based Confidence: high Scope-risk: narrow Directive: Keep workspace-row Finder actions behind Workspace.sidebarFinderDirectoryURL() so remote cwd strings stay gated Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local app/unit tests per user instruction; tagged app build/launch pending after PR CI
|
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:
📝 WalkthroughWalkthroughAdds a "Show in Finder" workspace-sidebar context-menu item. Implements WorkspaceFinderDirectoryResolver (cache, resolver, opener), per-tab cache/state and async refresh in ContentView, Workspace helpers to prefer local directories, Xcode project registration, en/ja localization, changelog entry, and a small CI curl/minisign robustness change. ChangesShow in Finder Feature
Sequence DiagramsequenceDiagram
participant User as User
participant CV as ContentView
participant Resolver as WorkspaceFinderDirectoryResolver
participant FM as FileManager
participant NSW as NSWorkspace
User->>CV: Right-click workspace row (open context menu)
CV->>CV: build cache key (path + refreshID)
CV->>Resolver: cache(for: key)
Resolver->>Resolver: path(for:) `@MainActor`
alt local path present
Resolver->>FM: check existence & isDirectory (background)
FM-->>Resolver: URL or nil
else no local path / remote
Resolver-->>CV: cache with nil URL
end
Resolver-->>CV: WorkspaceFinderDirectoryCache
alt URL available
User->>CV: Click "Show in Finder"
CV->>NSW: reveal URL in Finder
else URL nil
CV-->>User: "Show in Finder" disabled
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (11 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 |
The workflow guard rejects new growth in Workspace.swift, so the Finder URL conversion now lives with the sidebar row action while still using Workspace.sidebarDirectoriesInDisplayOrder() for the cwd source of truth. Constraint: Sources/Workspace.swift is over its file length budget and CI rejects additional lines Rejected: Refresh the Swift file length budget | this change does not need to accept new model-file debt Rejected: Duplicate panel/currentDirectory resolution in the view | Workspace.sidebarDirectoriesInDisplayOrder() already resolves the row directory Confidence: high Scope-risk: narrow Directive: Do not move this helper back into Workspace.swift without reducing that file or updating the budget intentionally Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local tests per user instruction; CI rerun pending
Contributor
Greptile SummaryThis PR adds a "Show in Finder" item to the single-workspace sidebar row context menu, resolving the best-effort local directory through a new
Confidence Score: 5/5Safe to merge — the Show-in-Finder path is fully UI-additive, all file I/O runs off the main actor via detached tasks, and the prior blocking-main-thread and stale-directory issues have been resolved. The new async helpers correctly delegate file-system work to detached tasks and revalidate at click time; the cache-key design avoids the refresh-flash from the earlier iteration; remote and missing-directory cases are properly disabled. No existing behavior is changed. Sources/WorkspaceFinderDirectoryResolver.swift — the two async resolver helpers could use explicit @Concurrent annotations for clearer isolation intent, though the current behavior is correct. Important Files Changed
Sequence DiagramsequenceDiagram
participant View as TabItemView (MainActor)
participant Resolver as WorkspaceFinderDirectoryResolver
participant FMgr as FileManager (detached task)
participant Opener as WorkspaceFinderDirectoryOpener
participant NS as NSWorkspace
Note over View: .task(id: finderDirectoryCacheKey) fires on appear or path change
View->>Resolver: cache(for: key)
Resolver->>FMgr: Task.detached fileExists(atPath:)
FMgr-->>Resolver: URL? directory exists
Resolver-->>View: WorkspaceFinderDirectoryCache
Note over View: Context menu opens - reads cached URL no I/O
View->>View: ".disabled(finderDirectoryURL == nil)"
Note over View: User clicks Show in Finder
View->>View: "workspaceFinderDirectoryOpenRequest = OpenRequest(url)"
Note over View: .task(id: openRequest) fires
View->>Opener: openInFinder(directoryURL)
Opener->>FMgr: Task.detached fileExists re-validate
FMgr-->>Opener: refreshedURL?
alt directory still exists
Opener->>NS: activateFileViewerSelecting([refreshedURL])
else directory gone
Opener->>View: NSSound.beep()
end
View->>View: "workspaceFinderDirectoryOpenRequest = nil"
Reviews (20): Last reviewed commit: "Place Finder action beside workspace cop..." | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The sidebar directory resolver can surface display-order cwd strings with a leading tilde. Expanding that path before the FileManager check keeps valid local home-relative workspaces eligible for the Show in Finder action while preserving the remote and missing-directory gates. Constraint: FileManager.fileExists does not expand tilde paths Rejected: Drop existence checks | the menu should stay disabled for stale or non-directory cwd values Confidence: high Scope-risk: narrow Directive: Keep path normalization before FileManager validation for workspace Finder actions Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local tests per user instruction; CI rerun pending
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The Show in Finder menu only needs a local URL when the workspace has a sidebar directory. Removing the synchronous FileManager existence check keeps right-click menu construction free of disk I/O while still gating remote and missing-cwd cases. Constraint: Context menu builders run on the main actor and must avoid blocking file-system calls Rejected: Precompute Finder URL with @State and a detached task | unnecessary once the menu no longer validates path existence during construction Confidence: high Scope-risk: narrow Directive: Keep workspace context-menu enablement based on model state, not synchronous disk probes Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local tests per user instruction; CI rerun pending
Directory existence is now checked from a row task off the main actor, while the context menu reads only a cached URL. This keeps the menu disabled for deleted or moved folders without putting FileManager work in SwiftUI menu construction. Constraint: Workspace row context menus are main-actor UI surfaces and must not perform synchronous file I/O during evaluation Rejected: Keep optimistic URL construction only | enabled the action for missing directories Confidence: medium Scope-risk: narrow Directive: Keep workspace Finder path existence checks off the context menu builder path Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: local Swift tests or app launch before CI, per task instructions
The row now stores only a small cache while path expansion and off-main existence checks live in a dedicated helper. This keeps the deleted-directory behavior intact without growing ContentView beyond the CI file-length budget. Constraint: ContentView.swift has a strict file-length budget enforced by CI Rejected: Leave the async cache helper inline in ContentView | exceeded workflow guard budget by 27 lines Confidence: medium Scope-risk: narrow Directive: Keep future workspace Finder path helpers outside ContentView unless the budget is reduced elsewhere Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj; wc -l Sources/ContentView.swift Not-tested: local Swift tests or app launch before CI, per task instructions
The workspace row now keys Finder directory availability by path plus a refresh generation. Opening the context menu invalidates the old cache immediately, and canceled async probes no longer overwrite newer cache state. Constraint: Finder availability must stay off the context-menu file-I/O path while still handling same-path directory deletion or recreation Rejected: Refresh only when the path string changes | stale positive cache remained possible Confidence: medium Scope-risk: narrow Directive: Cache writes from async Finder probes must remain keyed or cancellation-guarded Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj; wc -l Sources/ContentView.swift Not-tested: local Swift tests or app launch before CI, per task instructions
The workspace row Finder action needs to stay usable after its context menu refreshes, while also avoiding remote terminal cwd values that can resemble local paths. The row now passes the enabled URL captured during menu construction, and the helper revalidates that directory off-main before opening Finder. Workspace owns the local-only directory selection so remote-panel filtering stays beside the existing sidebar directory ordering logic. Constraint: Do not run local tests or xcodebuild; verification is remote CI plus static checks only Rejected: Re-check FileManager synchronously in the SwiftUI context menu | would reintroduce blocking file I/O during menu construction Rejected: Keep filtering only at the resolver | remote-panel knowledge already belongs to Workspace state Confidence: high Scope-risk: narrow Directive: Do not use the currentDirectory fallback for Finder reveal after filtering remote sidebar panels Tested: git diff --check; ContentView.swift line count 15945; origin verified as manaflow-ai/cmux Not-tested: local tests and local build per repository/user instructions
The previous fix correctly filtered remote terminal directories, but it expanded Workspace.swift beyond the repository file-length guard. This keeps the same local-only Finder directory behavior while compressing the helper back to the tracked budget. Constraint: Workflow guard enforces Sources/Workspace.swift at 13975 lines Rejected: Refresh the budget | this is not intentional file-length debt Confidence: high Scope-risk: narrow Tested: git diff --check; wc -l Sources/Workspace.swift reports 13975 Not-tested: local tests and local build per repository/user instructions
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/ContentView.swift`:
- Around line 13326-13328: The explicit reset of workspaceFinderDirectoryCache
is redundant; just increment workspaceFinderDirectoryRefreshID and let the
existing .task repopulate the cache. Remove the assignment to
workspaceFinderDirectoryCache = WorkspaceFinderDirectoryCache(...) so only
workspaceFinderDirectoryRefreshID &+ 1 is performed; this avoids an extra `@State`
write and the extra WorkspaceFinderDirectoryResolver.path(for:) call while
preserving correct cache invalidation logic keyed by (path, refreshID).
🪄 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: e2fcdda0-6c68-4d8a-8a8b-c3fd189235b6
📒 Files selected for processing (5)
CHANGELOG.mdGhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/ContentView.swiftSources/WorkspaceFinderDirectoryResolver.swift
The context menu only needs to advance the refresh token; the existing task keyed by the cache key owns recomputing directory availability. This keeps menu appearance side effects smaller and matches the row's async validation flow. Constraint: Local tests and direct xcodebuild are prohibited for this task. Rejected: Recreate the finder-directory cache in onAppear | duplicates the task-owned population path and was flagged by review. Confidence: high Scope-risk: narrow Tested: git diff --check; wc -l Sources/ContentView.swift Sources/Workspace.swift Not-tested: Local tests/builds per task restriction; CI will validate remotely.
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
May 6, 2026 08:56
Dismissed as stale: the requested cache simplification was applied in 71ab98a on the current head, the corresponding inline thread is resolved/outdated, and the current CodeRabbit status check is passing.
The previous CircleCI macos-unit-tests run failed before tests during the Zig installer step because curl received HTTP 500 while downloading the Zig minisign signature. The app debug and release build jobs passed the same installer path, so this no-code commit retriggers the remote pipeline without changing product behavior. Constraint: CircleCI API token is not available in this environment for a targeted job rerun. Rejected: Change CI installer logic in this feature branch | failure was an external HTTP 500 and the same step passed in sibling jobs. Confidence: high Scope-risk: narrow Tested: Inspected CircleCI job 3124 step output showing Install zig failed on curl HTTP 500 before tests. Not-tested: No local tests/builds per task restriction.
The Finder context menu validates workspace directories asynchronously from SwiftUI. The repo review rules require nonisolated async helpers that leave UI isolation to declare that boundary explicitly, so the cache and existence helpers now use @Concurrent while keeping the actual FileManager probe in the existing detached utility task. Constraint: SwiftUI row code must not perform filesystem validation on the main actor.\nRejected: Leave the helpers unannotated | CodeRabbit's repo-specific Swift concurrency rule flags that shape.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check\nNot-tested: Local tests/builds per branch instructions; remote CI will validate.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
13325-13331:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh the Finder target before the menu captures it.
Line 13326 bumps
workspaceFinderDirectoryRefreshIDonly after the context menu is already being built, but Lines 13479-13482 still capturefinderDirectoryURLfrom the pre-refresh cache state. In practice, the first open after a workspace path change can leave Show in Finder disabled or pointed at stale data until the user closes and reopens the menu.Possible fix
- Button(String(localized: "contextMenu.showWorkspaceInFinder", defaultValue: "Show in Finder")) { - WorkspaceFinderDirectoryOpener.openInFinder(finderDirectoryURL) - } - .disabled(finderDirectoryURL == nil) + Button(String(localized: "contextMenu.showWorkspaceInFinder", defaultValue: "Show in Finder")) { + Task { + let freshCache = await WorkspaceFinderDirectoryResolver.cache(for: finderDirectoryCacheKey) + guard !Task.isCancelled else { return } + await MainActor.run { + workspaceFinderDirectoryCache = freshCache + WorkspaceFinderDirectoryOpener.openInFinder(freshCache.directoryURL) + } + } + } + .disabled(finderDirectoryPath == nil && finderDirectoryURL == nil)Also applies to: 13479-13482
🤖 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/ContentView.swift` around lines 13325 - 13331, The context menu captures a stale finderDirectoryURL because workspaceFinderDirectoryRefreshID is incremented only after the menu begins building; update the code so the Finder target is refreshed before the menu capture by bumping workspaceFinderDirectoryRefreshID (or triggering the same refresh routine) prior to constructing the context menu and before calling rowInteractionState.contextMenuDidAppear(), and ensure the code paths that compute finderDirectoryURL (those around the current capture at lines referencing finderDirectoryURL) read from the refreshed cache/state rather than the pre-refresh value.
🤖 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.
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 13325-13331: The context menu captures a stale finderDirectoryURL
because workspaceFinderDirectoryRefreshID is incremented only after the menu
begins building; update the code so the Finder target is refreshed before the
menu capture by bumping workspaceFinderDirectoryRefreshID (or triggering the
same refresh routine) prior to constructing the context menu and before calling
rowInteractionState.contextMenuDidAppear(), and ensure the code paths that
compute finderDirectoryURL (those around the current capture at lines
referencing finderDirectoryURL) read from the refreshed cache/state rather than
the pre-refresh value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bf565ea7-4f99-4c39-aae6-fc1fbe84c3a7
📒 Files selected for processing (3)
Sources/ContentView.swiftSources/Workspace.swiftSources/WorkspaceFinderDirectoryResolver.swift
The Finder reveal action still revalidates the directory before opening Finder, but the async side effect now runs from a SwiftUI task keyed by explicit row state instead of an unmanaged Task inside the opener. This keeps cancellation and lifecycle ownership in the view layer while leaving the platform opener as a small async MainActor operation. Constraint: CodeRabbit flags fire-and-forget Tasks with real lifecycle in production Swift.\nRejected: Keep the Task inside WorkspaceFinderDirectoryOpener | it hides cancellation ownership from SwiftUI.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check\nNot-tested: Local tests/builds per branch instructions; remote CI will validate.
CircleCI macOS jobs failed before project build or tests while Homebrew was downloading portable Ruby from GHCR during the Install zig step. The previous rerun API attempt was rejected without a CircleCI token, so this commit only retriggers the remote checks for the same source tree. Constraint: No CircleCI token available for failed-job rerun Constraint: Local tests and direct xcodebuild are prohibited for this branch Rejected: Code change | failure occurred before project checkout/build/test Confidence: medium Scope-risk: narrow Directive: Treat this as CI retry metadata only; do not infer a source change from this commit Tested: Remote CI logs for CircleCI jobs 3450, 3451, and 3452 Not-tested: Local tests/builds per branch instructions
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/WorkspaceFinderDirectoryResolver.swift`:
- Around line 60-69: The failure path currently calls NSSound.beep() even if the
async Task was cancelled; update the cancellation handling so you check
Task.isCancelled before emitting the beep. Specifically, in the else branch that
handles a nil refreshedURL (and optionally immediately after the initial guard
let directoryURL), add a guard or if-check using Task.isCancelled and return
early if cancelled, and only call NSSound.beep() when not cancelled; reference
WorkspaceFinderDirectoryResolver.existingDirectoryURL(for:), Task.isCancelled,
NSWorkspace.shared.activateFileViewerSelecting(_:) and NSSound.beep() to locate
where to add the check.
🪄 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: f5478ecf-b99c-40b4-aa2d-9a530f53457f
📒 Files selected for processing (1)
Sources/WorkspaceFinderDirectoryResolver.swift
The Finder reveal request is owned by SwiftUI task lifecycles, so cancellation should quietly stop both success and failure side effects. The opener now checks Task cancellation before handling nil input and again before beeping after a failed revalidation. Constraint: CodeRabbit flagged cancellation should suppress NSSound.beep failure feedback Rejected: Leave failure beep unconditional | can emit UI feedback after the owning task is cancelled Confidence: high Scope-risk: narrow Directive: Finder open side effects should remain guarded by task cancellation checks Tested: git diff --check Not-tested: Local tests/builds per branch instructions
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 79ebf6f. Configure here.
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
May 6, 2026 19:07
Dismissed as stale after commit 79ebf6f added Task.isCancelled guards before nil-input and failed-revalidation beeps in WorkspaceFinderDirectoryOpener.openInFinder(_:). Current CodeRabbit status check is green on the newer head.
Show in Finder must only reveal local workspace directories. Remote child-exit panels can remain in the sidebar briefly after they are no longer active remote surfaces, so the Finder directory filter now treats pending remote child-exit panels as remote and suppresses fallback selection for mixed local/remote state. Constraint: Cursor Bugbot flagged ended remote panels awaiting cleanup could expose remote cwd strings as local Finder paths Rejected: Rely only on activeRemoteTerminalSurfaceIds | misses the child-exit cleanup window Confidence: high Scope-risk: narrow Directive: Keep sidebar Finder targets gated by every remote-surface lifecycle set, including pending child-exit state Tested: git diff --check Not-tested: Local tests/builds per branch instructions
CircleCI macOS jobs were hanging in the Install zig step before any app build or unit test started. The bootstrap now keeps the existing Zig tarball path but verifies it with the official Zig 0.15.2 SHA256 values for macOS arm64 and x86_64, avoiding a Homebrew minisign install on every macOS lane. Constraint: User forbids local xcodebuild and local tests; validation must come from remote CI. Constraint: Zig checksums were read from https://ziglang.org/download/index.json for version 0.15.2. Rejected: Continue installing minisign through Homebrew | current CircleCI lanes hung in that dependency bootstrap before build/test. Confidence: medium Scope-risk: narrow Directive: Keep CI dependency bootstrap deterministic and avoid Homebrew on the critical macOS build path. Tested: git diff --check Not-tested: Remote CircleCI rerun pending after push
The macOS lanes can stall in the Zig tarball download without producing logs, leaving the workflow pending before build or tests. The existing retry helper now has explicit connect and total transfer limits so CircleCI either completes the bootstrap or retries/fails with output. Constraint: User forbids local xcodebuild and local tests; validation must come from remote CI. Rejected: Wait for CircleCI no-output timeout | it burns 35 minutes per attempt without producing better diagnostics. Confidence: medium Scope-risk: narrow Directive: Keep external downloads in CI bounded with explicit timeouts. Tested: git diff --check Not-tested: Remote CircleCI rerun pending after push
The Zig host responds but full archive transfer can be slow enough that a five-minute total timeout aborts healthy downloads. Keep the bounded curl path, but use a longer max-time with a low-speed timeout so true stalls still fail while slow transfers can complete. Constraint: User forbids local xcodebuild and local tests; validation must come from remote CI. Rejected: Keep a five-minute max-time | local probe showed the official archive can transfer slowly enough to exceed it. Confidence: medium Scope-risk: narrow Directive: Do not remove the speed-time guard if increasing transfer max-time; both are needed to distinguish slow downloads from stalled downloads. Tested: git diff --check Not-tested: Remote CircleCI rerun pending after push
Contributor
|
Current-head CI is green. I reviewed Greptile's remaining supply-chain note on the Zig bootstrap change: this is an intentional CI reliability tradeoff to avoid Homebrew/minisign hangs before any app build starts. The previous minisign public key also lived in branch-controlled CI config, so branch authors could alter either the key or the checksum; the meaningful control is PR review plus a pinned Zig version. The new path keeps a pinned official Zig 0.15.2 SHA256 per macOS architecture, verifies with |
The PR branch had fallen behind main, including a large CI and project-file reshuffle. The merge keeps main's extracted Zig CI script and new project entries while preserving the Show in Finder source registration. Constraint: iterate-pr requires syncing the base branch before handling CI and feedback. Rejected: Keep the old inline CircleCI Zig installer | main replaced it with scripts/install-zig-ci.sh and removed Zig builds from the macOS jobs that now use CMUX_SKIP_ZIG_BUILD. Confidence: medium Scope-risk: moderate Directive: Keep WorkspaceFinderDirectoryResolver.swift registered in the Xcode project when rebasing or regenerating project files. Tested: git diff --check; conflict-marker scan for merged files; upfront tagged reload before merge Not-tested: Local tests not run per repository policy
The branch picked up main's window-drag refactor, but the unit test target still had three stale assertions against helpers that no longer exist. The same fixture already covers the current reference-counted suppression state and temporary movability behavior, so this removes the obsolete compile-only failures instead of restoring dead compatibility shims.\n\nConstraint: Local tests are forbidden in this repo; CircleCI is the verification surface.\nRejected: Recreate temporarilyDisableWindowDragging/restoreWindowDragging | the production API moved to begin/end suppression depth plus withTemporaryWindowMovableEnabled.\nConfidence: high\nScope-risk: narrow\nDirective: Keep these tests aligned with WindowDragHandleView's public helper surface rather than adding test-only compatibility aliases.\nTested: git diff --check; rg found no remaining stale helper references.\nNot-tested: Local tests not run per repo policy.
| .contextMenu { | ||
| workspaceContextMenu | ||
| .onAppear { | ||
| workspaceFinderDirectoryRefreshID &+= 1 |
Contributor
There was a problem hiding this comment.
workspaceFinderDirectoryRefreshID &+= 1 fires inside onAppear, which runs after SwiftUI has already rendered the context menu with the old refreshID. On macOS, SwiftUI context menus back onto NSMenu and do re-evaluate their @ViewBuilder content reactively when state changes. The result is: the menu appears with the button enabled (pre-populated cache key matches refreshID 0), then onAppear fires → refreshID becomes 1 → SwiftUI re-evaluates workspaceContextMenu → cache.url(for: {path, refreshID: 1}) returns nil (cache still holds refreshID 0) → button becomes disabled → the async task completes and re-enables it. Every context menu open produces this transient disabled flash.
WorkspaceFinderDirectoryOpener.openInFinder already calls existingDirectoryURL before invoking activateFileViewerSelecting and beeps on failure, so the deleted-directory safety case is fully covered without the onAppear refresh. Removing the workspaceFinderDirectoryRefreshID &+= 1 line (or replacing it with a non-invalidating "kick a background check without changing the key") would keep the button enabled during the check while still protecting against stale URLs at click time.
Contributor
Author
There was a problem hiding this comment.
Fixed in 32a4ab4 by removing the context-menu refresh id from the Finder directory cache key. The menu now stays on the path-keyed cached state while the opener still revalidates the directory immediately before revealing it in Finder.
— Claude Code
Greptile caught that bumping the Finder cache refresh id from the context menu's onAppear caused the menu to re-render with a new key before the async check completed, creating a transient disabled Show in Finder item. The opener already revalidates the directory immediately before calling Finder, so the cache key only needs to track the resolved path.\n\nConstraint: macOS SwiftUI context menus can re-evaluate their builder after state changes during menu presentation.\nRejected: Add another background refresh side channel | unnecessary because click-time revalidation already covers deleted directories.\nConfidence: high\nScope-risk: narrow\nDirective: Do not mutate the Finder cache key from context-menu presentation; keep the menu state stable while AppKit owns the menu.\nTested: git diff --check.\nNot-tested: Local tests not run per repo policy; CI will verify after push.\nRelated: #3632 (comment)
The base branch moved after the Finder menu cache fix, making the PR unmergeable. The only conflict was in WindowAndDragTests, where main had already replaced the deleted helper coverage with current begin/end suppression behavior; this merge keeps that current API coverage alongside this branch's stale-helper cleanup.\n\nConstraint: iterate-pr requires the PR branch to stay mergeable with origin/main before final CI and dogfood handoff.\nRejected: Reintroduce temporarilyDisableWindowDragging/restoreWindowDragging for compatibility | those helpers are dead API and main now tests the current suppression model directly.\nConfidence: medium\nScope-risk: moderate\nDirective: Keep window-drag tests on begin/end/clear suppression helpers; do not restore removed compatibility shims.\nTested: git diff --check; line-anchored conflict-marker scan excluding known generated/submodule art.\nNot-tested: Local tests not run per repo policy.
The Finder reveal action belongs near the workspace identity utilities rather than the description-editing actions. Moving it below Copy Workspace ID keeps the existing single-workspace guard and disabled URL state while matching the requested menu grouping.\n\nConstraint: Preserve the existing localized label and directory-resolution path.\nRejected: Duplicate the button in both locations | that would create two entry points with identical behavior in one menu.\nConfidence: high\nScope-risk: narrow\nDirective: Keep Show in Finder grouped with workspace identity actions unless the context menu is reorganized holistically.\nTested: git diff --check.\nNot-tested: Local tests not run per repo policy.
ShubhamPatilsd
pushed a commit
to emergent-inc/mosaic
that referenced
this pull request
Jul 9, 2026
* Let users jump from a workspace row to its Finder folder The workspace sidebar already owns directory ordering for row display, so the Finder action reuses that resolver and only converts the first local directory into a Finder URL at context-menu time. Remote workspaces and missing directories leave the action unavailable instead of sending a remote or nil path to Finder. Constraint: Do not run local tests or direct xcodebuild; CI and the required tagged reload are the allowed verification paths Constraint: SSH workspaces can expose remote cwd strings that must not be opened through local Finder Rejected: Thread a cwd through TabItemView | existing Workspace.sidebarDirectoriesInDisplayOrder() already centralizes row directory resolution Rejected: Add a grep-style context-menu regression test | project test policy requires observable behavior, and the nearby test is source-text based Confidence: high Scope-risk: narrow Directive: Keep workspace-row Finder actions behind Workspace.sidebarFinderDirectoryURL() so remote cwd strings stay gated Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local app/unit tests per user instruction; tagged app build/launch pending after PR CI * Keep Finder menu wiring within the sidebar row The workflow guard rejects new growth in Workspace.swift, so the Finder URL conversion now lives with the sidebar row action while still using Workspace.sidebarDirectoriesInDisplayOrder() for the cwd source of truth. Constraint: Sources/Workspace.swift is over its file length budget and CI rejects additional lines Rejected: Refresh the Swift file length budget | this change does not need to accept new model-file debt Rejected: Duplicate panel/currentDirectory resolution in the view | Workspace.sidebarDirectoriesInDisplayOrder() already resolves the row directory Confidence: high Scope-risk: narrow Directive: Do not move this helper back into Workspace.swift without reducing that file or updating the budget intentionally Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local tests per user instruction; CI rerun pending * Resolve tilde workspace paths before Finder validation The sidebar directory resolver can surface display-order cwd strings with a leading tilde. Expanding that path before the FileManager check keeps valid local home-relative workspaces eligible for the Show in Finder action while preserving the remote and missing-directory gates. Constraint: FileManager.fileExists does not expand tilde paths Rejected: Drop existence checks | the menu should stay disabled for stale or non-directory cwd values Confidence: high Scope-risk: narrow Directive: Keep path normalization before FileManager validation for workspace Finder actions Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local tests per user instruction; CI rerun pending * Avoid file stats while building workspace context menus The Show in Finder menu only needs a local URL when the workspace has a sidebar directory. Removing the synchronous FileManager existence check keeps right-click menu construction free of disk I/O while still gating remote and missing-cwd cases. Constraint: Context menu builders run on the main actor and must avoid blocking file-system calls Rejected: Precompute Finder URL with @State and a detached task | unnecessary once the menu no longer validates path existence during construction Confidence: high Scope-risk: narrow Directive: Keep workspace context-menu enablement based on model state, not synchronous disk probes Tested: jq empty Resources/Localizable.xcstrings; git diff --check Not-tested: Local tests per user instruction; CI rerun pending * Disable Finder menu for missing workspace folders Directory existence is now checked from a row task off the main actor, while the context menu reads only a cached URL. This keeps the menu disabled for deleted or moved folders without putting FileManager work in SwiftUI menu construction. Constraint: Workspace row context menus are main-actor UI surfaces and must not perform synchronous file I/O during evaluation Rejected: Keep optimistic URL construction only | enabled the action for missing directories Confidence: medium Scope-risk: narrow Directive: Keep workspace Finder path existence checks off the context menu builder path Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: local Swift tests or app launch before CI, per task instructions * Keep Finder directory checks outside the sidebar row The row now stores only a small cache while path expansion and off-main existence checks live in a dedicated helper. This keeps the deleted-directory behavior intact without growing ContentView beyond the CI file-length budget. Constraint: ContentView.swift has a strict file-length budget enforced by CI Rejected: Leave the async cache helper inline in ContentView | exceeded workflow guard budget by 27 lines Confidence: medium Scope-risk: narrow Directive: Keep future workspace Finder path helpers outside ContentView unless the budget is reduced elsewhere Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj; wc -l Sources/ContentView.swift Not-tested: local Swift tests or app launch before CI, per task instructions * Refresh Finder directory cache per menu request The workspace row now keys Finder directory availability by path plus a refresh generation. Opening the context menu invalidates the old cache immediately, and canceled async probes no longer overwrite newer cache state. Constraint: Finder availability must stay off the context-menu file-I/O path while still handling same-path directory deletion or recreation Rejected: Refresh only when the path string changes | stale positive cache remained possible Confidence: medium Scope-risk: narrow Directive: Cache writes from async Finder probes must remain keyed or cancellation-guarded Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj; wc -l Sources/ContentView.swift Not-tested: local Swift tests or app launch before CI, per task instructions * Keep Finder reveal scoped to local workspace directories The workspace row Finder action needs to stay usable after its context menu refreshes, while also avoiding remote terminal cwd values that can resemble local paths. The row now passes the enabled URL captured during menu construction, and the helper revalidates that directory off-main before opening Finder. Workspace owns the local-only directory selection so remote-panel filtering stays beside the existing sidebar directory ordering logic. Constraint: Do not run local tests or xcodebuild; verification is remote CI plus static checks only Rejected: Re-check FileManager synchronously in the SwiftUI context menu | would reintroduce blocking file I/O during menu construction Rejected: Keep filtering only at the resolver | remote-panel knowledge already belongs to Workspace state Confidence: high Scope-risk: narrow Directive: Do not use the currentDirectory fallback for Finder reveal after filtering remote sidebar panels Tested: git diff --check; ContentView.swift line count 15945; origin verified as manaflow-ai/cmux Not-tested: local tests and local build per repository/user instructions * Keep the local Finder guard within the Workspace budget The previous fix correctly filtered remote terminal directories, but it expanded Workspace.swift beyond the repository file-length guard. This keeps the same local-only Finder directory behavior while compressing the helper back to the tracked budget. Constraint: Workflow guard enforces Sources/Workspace.swift at 13975 lines Rejected: Refresh the budget | this is not intentional file-length debt Confidence: high Scope-risk: narrow Tested: git diff --check; wc -l Sources/Workspace.swift reports 13975 Not-tested: local tests and local build per repository/user instructions * Avoid redundant Finder cache writes while opening menus The context menu only needs to advance the refresh token; the existing task keyed by the cache key owns recomputing directory availability. This keeps menu appearance side effects smaller and matches the row's async validation flow. Constraint: Local tests and direct xcodebuild are prohibited for this task. Rejected: Recreate the finder-directory cache in onAppear | duplicates the task-owned population path and was flagged by review. Confidence: high Scope-risk: narrow Tested: git diff --check; wc -l Sources/ContentView.swift Sources/Workspace.swift Not-tested: Local tests/builds per task restriction; CI will validate remotely. * Retry transient CI dependency download The previous CircleCI macos-unit-tests run failed before tests during the Zig installer step because curl received HTTP 500 while downloading the Zig minisign signature. The app debug and release build jobs passed the same installer path, so this no-code commit retriggers the remote pipeline without changing product behavior. Constraint: CircleCI API token is not available in this environment for a targeted job rerun. Rejected: Change CI installer logic in this feature branch | failure was an external HTTP 500 and the same step passed in sibling jobs. Confidence: high Scope-risk: narrow Tested: Inspected CircleCI job 3124 step output showing Install zig failed on curl HTTP 500 before tests. Not-tested: No local tests/builds per task restriction. * Keep Finder directory probes off the UI actor The Finder context menu validates workspace directories asynchronously from SwiftUI. The repo review rules require nonisolated async helpers that leave UI isolation to declare that boundary explicitly, so the cache and existence helpers now use @Concurrent while keeping the actual FileManager probe in the existing detached utility task. Constraint: SwiftUI row code must not perform filesystem validation on the main actor.\nRejected: Leave the helpers unannotated | CodeRabbit's repo-specific Swift concurrency rule flags that shape.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check\nNot-tested: Local tests/builds per branch instructions; remote CI will validate. * Let SwiftUI own Finder reveal requests The Finder reveal action still revalidates the directory before opening Finder, but the async side effect now runs from a SwiftUI task keyed by explicit row state instead of an unmanaged Task inside the opener. This keeps cancellation and lifecycle ownership in the view layer while leaving the platform opener as a small async MainActor operation. Constraint: CodeRabbit flags fire-and-forget Tasks with real lifecycle in production Swift.\nRejected: Keep the Task inside WorkspaceFinderDirectoryOpener | it hides cancellation ownership from SwiftUI.\nConfidence: high\nScope-risk: narrow\nTested: git diff --check\nNot-tested: Local tests/builds per branch instructions; remote CI will validate. * Retry transient macOS dependency bootstrap CircleCI macOS jobs failed before project build or tests while Homebrew was downloading portable Ruby from GHCR during the Install zig step. The previous rerun API attempt was rejected without a CircleCI token, so this commit only retriggers the remote checks for the same source tree. Constraint: No CircleCI token available for failed-job rerun Constraint: Local tests and direct xcodebuild are prohibited for this branch Rejected: Code change | failure occurred before project checkout/build/test Confidence: medium Scope-risk: narrow Directive: Treat this as CI retry metadata only; do not infer a source change from this commit Tested: Remote CI logs for CircleCI jobs 3450, 3451, and 3452 Not-tested: Local tests/builds per branch instructions * Keep macOS CI out of Homebrew auto-update CircleCI only needs Homebrew as a fallback installer for minisign before downloading and verifying the Zig tarball. Repeated current-head failures died inside Homebrew auto-update with GHCR curl resets before any cmux build or test work, so the install now disables Homebrew auto-update and cleanup for that fallback command. Constraint: CircleCI failed before project build/test in the Install zig command Constraint: Direct local xcodebuild and local tests are prohibited for this branch Rejected: Another empty retry | repeated GHCR/Homebrew auto-update reset showed the bootstrap path itself was flaky Confidence: medium Scope-risk: narrow Directive: Keep Zig installed from the signed upstream tarball; Homebrew remains only the minisign fallback Tested: git diff --check Not-tested: Local tests/builds per branch instructions * Retry transient Zig downloads in macOS CI The macOS debug job reached the verified Zig tarball download but failed with a connection reset before any cmux build or tests ran. The install command now wraps the Zig tarball and minisig downloads in a bounded curl retry helper while preserving signature verification. Constraint: CircleCI macOS networking reset during Zig tarball download before project build/test Constraint: Direct local xcodebuild and local tests are prohibited for this branch Rejected: Remove minisign verification | weakens the CI supply-chain guard Confidence: medium Scope-risk: narrow Directive: Keep retry count bounded and retain minisign verification for the downloaded Zig archive Tested: git diff --check Not-tested: Local tests/builds per branch instructions * Keep Finder directory probing structurally cancellable The directory existence check already runs from an @Concurrent resolver entry point, so wrapping the synchronous FileManager probe in an extra detached task adds no useful isolation and weakens structured cancellation. The resolver now honors cancellation before probing and calls the nonisolated helper directly. Constraint: Greptile flagged the detached probe as redundant inside @Concurrent code Rejected: Keep Task.detached | unnecessary after @Concurrent isolation and leaves less structured cancellation Confidence: medium Scope-risk: narrow Directive: Keep context-menu path checks off MainActor through @Concurrent, not unmanaged child tasks Tested: git diff --check Not-tested: Local tests/builds per branch instructions * Use Swift 5-compatible Finder directory probing The app target is configured in Swift 5 mode and the package manifest is tools-version 5.9, so the Finder resolver should not introduce @concurrent-only syntax. The cache method stays regular async and the filesystem probe uses a bounded detached task with cancellation checks so context-menu construction still avoids disk I/O on the UI path. Constraint: CodeRabbit flagged @Concurrent as unsupported by the configured Swift 5.9/Swift 5 toolchain Rejected: Raise the package tools version | broad repository-level change for a small app-target helper Confidence: medium Scope-risk: narrow Directive: Keep filesystem probing asynchronous through the detached helper unless the project intentionally moves to a toolchain that supports @Concurrent everywhere Tested: git diff --check Not-tested: Local tests/builds per branch instructions * Avoid Finder error feedback after cancellation The Finder reveal request is owned by SwiftUI task lifecycles, so cancellation should quietly stop both success and failure side effects. The opener now checks Task cancellation before handling nil input and again before beeping after a failed revalidation. Constraint: CodeRabbit flagged cancellation should suppress NSSound.beep failure feedback Rejected: Leave failure beep unconditional | can emit UI feedback after the owning task is cancelled Confidence: high Scope-risk: narrow Directive: Finder open side effects should remain guarded by task cancellation checks Tested: git diff --check Not-tested: Local tests/builds per branch instructions * Keep ended remote panels out of Finder targets Show in Finder must only reveal local workspace directories. Remote child-exit panels can remain in the sidebar briefly after they are no longer active remote surfaces, so the Finder directory filter now treats pending remote child-exit panels as remote and suppresses fallback selection for mixed local/remote state. Constraint: Cursor Bugbot flagged ended remote panels awaiting cleanup could expose remote cwd strings as local Finder paths Rejected: Rely only on activeRemoteTerminalSurfaceIds | misses the child-exit cleanup window Confidence: high Scope-risk: narrow Directive: Keep sidebar Finder targets gated by every remote-surface lifecycle set, including pending child-exit state Tested: git diff --check Not-tested: Local tests/builds per branch instructions * Keep Zig bootstrap independent of Homebrew CircleCI macOS jobs were hanging in the Install zig step before any app build or unit test started. The bootstrap now keeps the existing Zig tarball path but verifies it with the official Zig 0.15.2 SHA256 values for macOS arm64 and x86_64, avoiding a Homebrew minisign install on every macOS lane. Constraint: User forbids local xcodebuild and local tests; validation must come from remote CI. Constraint: Zig checksums were read from https://ziglang.org/download/index.json for version 0.15.2. Rejected: Continue installing minisign through Homebrew | current CircleCI lanes hung in that dependency bootstrap before build/test. Confidence: medium Scope-risk: narrow Directive: Keep CI dependency bootstrap deterministic and avoid Homebrew on the critical macOS build path. Tested: git diff --check Not-tested: Remote CircleCI rerun pending after push * Bound Zig download attempts in macOS CI The macOS lanes can stall in the Zig tarball download without producing logs, leaving the workflow pending before build or tests. The existing retry helper now has explicit connect and total transfer limits so CircleCI either completes the bootstrap or retries/fails with output. Constraint: User forbids local xcodebuild and local tests; validation must come from remote CI. Rejected: Wait for CircleCI no-output timeout | it burns 35 minutes per attempt without producing better diagnostics. Confidence: medium Scope-risk: narrow Directive: Keep external downloads in CI bounded with explicit timeouts. Tested: git diff --check Not-tested: Remote CircleCI rerun pending after push * Tolerate slow Zig archive downloads in CI The Zig host responds but full archive transfer can be slow enough that a five-minute total timeout aborts healthy downloads. Keep the bounded curl path, but use a longer max-time with a low-speed timeout so true stalls still fail while slow transfers can complete. Constraint: User forbids local xcodebuild and local tests; validation must come from remote CI. Rejected: Keep a five-minute max-time | local probe showed the official archive can transfer slowly enough to exceed it. Confidence: medium Scope-risk: narrow Directive: Do not remove the speed-time guard if increasing transfer max-time; both are needed to distinguish slow downloads from stalled downloads. Tested: git diff --check Not-tested: Remote CircleCI rerun pending after push * Keep window-drag tests on current suppression API The branch picked up main's window-drag refactor, but the unit test target still had three stale assertions against helpers that no longer exist. The same fixture already covers the current reference-counted suppression state and temporary movability behavior, so this removes the obsolete compile-only failures instead of restoring dead compatibility shims.\n\nConstraint: Local tests are forbidden in this repo; CircleCI is the verification surface.\nRejected: Recreate temporarilyDisableWindowDragging/restoreWindowDragging | the production API moved to begin/end suppression depth plus withTemporaryWindowMovableEnabled.\nConfidence: high\nScope-risk: narrow\nDirective: Keep these tests aligned with WindowDragHandleView's public helper surface rather than adding test-only compatibility aliases.\nTested: git diff --check; rg found no remaining stale helper references.\nNot-tested: Local tests not run per repo policy. * Keep Finder menu cache stable while opening Greptile caught that bumping the Finder cache refresh id from the context menu's onAppear caused the menu to re-render with a new key before the async check completed, creating a transient disabled Show in Finder item. The opener already revalidates the directory immediately before calling Finder, so the cache key only needs to track the resolved path.\n\nConstraint: macOS SwiftUI context menus can re-evaluate their builder after state changes during menu presentation.\nRejected: Add another background refresh side channel | unnecessary because click-time revalidation already covers deleted directories.\nConfidence: high\nScope-risk: narrow\nDirective: Do not mutate the Finder cache key from context-menu presentation; keep the menu state stable while AppKit owns the menu.\nTested: git diff --check.\nNot-tested: Local tests not run per repo policy; CI will verify after push.\nRelated: manaflow-ai/cmux#3632 (comment) * Place Finder action beside workspace copy menu item The Finder reveal action belongs near the workspace identity utilities rather than the description-editing actions. Moving it below Copy Workspace ID keeps the existing single-workspace guard and disabled URL state while matching the requested menu grouping.\n\nConstraint: Preserve the existing localized label and directory-resolution path.\nRejected: Duplicate the button in both locations | that would create two entry points with identical behavior in one menu.\nConfidence: high\nScope-risk: narrow\nDirective: Keep Show in Finder grouped with workspace identity actions unless the context menu is reorganized holistically.\nTested: git diff --check.\nNot-tested: Local tests not run per repo policy.
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3623
Validation
Not run locally per instruction: app/unit tests and direct xcodebuild. Final tagged build/launch will run after CI passes.
Note
Low Risk
Low risk UI enhancement that adds a new context-menu action and small directory-resolution logic; main risk is incorrect path resolution/enablement for edge cases (remote workspaces, missing dirs).
Overview
Adds a "Show in Finder" action to the single-workspace sidebar row context menu, resolving a workspace’s best local directory and opening it via
NSWorkspace(with the menu item disabled for remote workspaces or missing/non-directory paths).Introduces
Workspace.sidebarFinderDirectory()and a small async cache/opener (WorkspaceFinderDirectoryResolver/Opener) to compute and validate the target path off the main thread, plus newen/jalocalization and a changelog entry.Reviewed by Cursor Bugbot for commit 49f28f0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Localization