Repository navigation
Resolve terminal relative paths from surface context - #8101
Closed
austinywang wants to merge 7 commits into
Closed
austinywang wants to merge 7 commits into
austinywang wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes add shared terminal path-resolution models and probing, preserve line and column metadata through file-opening flows, support configured-editor locations, extract browser helpers, and expand path, editor, browser, fallback-policy, and runtime-stub tests. ChangesTerminal path and link opening flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 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 |
|
To use Codex here, create a Codex account and connect to github. |
austinywang
marked this pull request as ready for review
July 14, 2026 22:59
Contributor
Greptile SummaryThis PR improves terminal file-link handling from detection through opening. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (7): Last reviewed commit: "fix: address command-click review findin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift (1)
94-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the older wrappers if you don’t need the compatibility surface
resolveVisibleLinePathandresolveQuicklookPathare only referenced by tests; production now routes throughresolveVisibleLineReferenceandresolvePath. If no external callers depend on the old API, removing the wrappers would leave one path-resolution surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift` around lines 94 - 155, Remove the legacy resolveQuicklookPath and resolveVisibleLinePath wrappers from TerminalPathResolver, along with any tests or references that require those compatibility-only APIs, so callers use resolvePath and resolveVisibleLineReference directly. Preserve the existing path-resolution behavior through the structured APIs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift`:
- Around line 10-23: Mark the three Sendable value/service types as explicitly
nonisolated: update TerminalPathResolver in
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift:10-23,
TerminalPathResolution in
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolution.swift:2-24,
and TerminalPathResolutionContext in
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolutionContext.swift:2-21.
No other behavior changes are needed.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2970-3016: The relative-path candidate early return after file
resolution must not swallow already-resolved files. Update the
`TerminalPathResolver().isRelativePathReferenceCandidate(trimmedUrlString)`
condition to return early only when `resolvedFileReference` is nil, allowing
resolved non-cmux-routable files to continue to `resolveOpenURLTarget` and the
external editor fallback.
---
Outside diff comments:
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift`:
- Around line 94-155: Remove the legacy resolveQuicklookPath and
resolveVisibleLinePath wrappers from TerminalPathResolver, along with any tests
or references that require those compatibility-only APIs, so callers use
resolvePath and resolveVisibleLineReference directly. Preserve the existing
path-resolution behavior through the structured APIs.
🪄 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: 9ee6e6fa-f436-42b0-ac6c-317b6cbc7f19
📒 Files selected for processing (13)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/String+TerminalPathTokens.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolution.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolutionContext.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/TerminalPathResolverTests.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/FileOpen/PreferredEditorServiceTests.swiftSources/CommandClickFileOpenRouter.swiftSources/GhosttyApp+EmbeddedBrowserLink.swiftSources/GhosttyTerminalView.swiftSources/TerminalBrowserHostNormalizer.swiftcmux.xcodeproj/project.pbxprojcmuxTests/TerminalAndGhosttyTests.swift
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
`@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorLaunchCommand.swift`:
- Around line 15-20: Consolidate the editor-name checks used by
supportsSourceLocation and usesGotoFlag into one shared source of truth, such as
a common supported-editor collection or helper. Update both properties to reuse
it while preserving their existing behavior and editor-specific CLI flag
selection.
🪄 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: 688c6685-c2cb-4bd8-83c7-745ea32ffd29
📒 Files selected for processing (14)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalCommandClickFallbackPolicy.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolution.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolutionContext.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/TerminalCommandClickFallbackPolicyTests.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/TerminalPathResolverTests.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorLaunchCommand.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/FileOpen/PreferredEditorServiceTests.swiftSources/CommandClickFileOpenRouter.swiftSources/GhosttyTerminalView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/TerminalAndGhosttyTests.swiftcmuxTests/TerminalOpenURLTargetResolutionTests.swift
💤 Files with no reviewable changes (1)
- cmuxTests/TerminalAndGhosttyTests.swift
Comment on lines
+15
to
+20
| var supportsSourceLocation: Bool { | ||
| guard let executableName else { return false } | ||
| return [ | ||
| "code", "code-insiders", "codium", "cursor", "subl", "zed", "zed-preview", | ||
| ].contains(executableName) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Two independent editor lists risk silently diverging.
supportsSourceLocation and usesGotoFlag each hardcode their own editor-name list. Adding a new VS Code fork to one list without updating the other would silently produce the wrong CLI syntax for that editor.
♻️ Consolidate into a single source of truth
+private enum KnownEditor: String {
+ case code, codeInsiders = "code-insiders", codium, cursor, subl, zed, zedPreview = "zed-preview"
+
+ var usesGotoFlag: Bool {
+ switch self {
+ case .code, .codeInsiders, .codium, .cursor: return true
+ case .subl, .zed, .zedPreview: return false
+ }
+ }
+}
+
var supportsSourceLocation: Bool {
guard let executableName else { return false }
- return [
- "code", "code-insiders", "codium", "cursor", "subl", "zed", "zed-preview",
- ].contains(executableName)
+ return KnownEditor(rawValue: executableName) != nil
} private var usesGotoFlag: Bool {
guard let executableName else { return false }
- return ["code", "code-insiders", "codium", "cursor"].contains(executableName)
+ return KnownEditor(rawValue: executableName)?.usesGotoFlag ?? false
}Also applies to: 40-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorLaunchCommand.swift`
around lines 15 - 20, Consolidate the editor-name checks used by
supportsSourceLocation and usesGotoFlag into one shared source of truth, such as
a common supported-editor collection or helper. Update both properties to reuse
it while preserving their existing behavior and editor-specific CLI flag
selection.
This was referenced Jul 15, 2026
lawrencecchen
added
the
stale-revisit
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #8097
Related: #1678, #4675
Architecture
TerminalPathResolver in CmuxTerminalCore owns token normalization, source-location parsing, ordered root probing, URL exclusion, and existence gating. The app callback only builds TerminalPathResolutionContext from canonical per-surface panelDirectories and requestedWorkingDirectory metadata plus the tracked repository/workspace roots. PreferredEditorService remains the single external-editor launch path and receives the structured source location.
Testing
Localization
No user-facing strings, settings labels, shortcut metadata, docs text, or localization catalogs changed. The changed Swift files were searched for newly introduced UI text; no localization entries were required.
Review Trigger
@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review
Checklist
Summary by CodeRabbit
line/columnto the configured editor (:line/:line:column).:line/:line:columnparsing, including safer handling that won’t bypass file existence checks for location-suffixed tokens.