Repository navigation
Fix markdown files with trailing punctuation detected as URLs - #4594
Merged
austinywang merged 3 commits intoMay 24, 2026
Merged
austinywang merged 3 commits into
austinywang merged 3 commits into
Conversation
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR centralizes terminal working-directory resolution and deferred cmux file opening in CommandClickFileOpenRouter, updates GhosttyTerminalView to use that router for relative and file-URL routing and for resolved-word working directories, and adds tests for relative-path punctuation trimming and URL-scheme gating. ChangesCommand-click routing refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 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 |
jasonko
force-pushed
the
fix/markdown-trailing-punctuation-4569
branch
from
May 22, 2026 23:37
93015e1 to
146fe04
Compare
…anaflow-ai#4569) Tests the two key behaviors introduced by the fix: 1. cmuxResolveQuicklookPath correctly resolves relative file paths with trailing sentence punctuation against a CWD (the core fix for manaflow-ai#4569). - Relative markdown path with trailing dot - Relative path with trailing comma - Non-existent relative path returns nil 2. URL(string:)?.scheme gate correctly distinguishes schemeless strings (relative paths, bare domains) from strings with URL schemes (https, file, mailto), validating the guard condition that determines whether file-path resolution is attempted before URL classification. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ow-ai#4569) When terminal output contains a file path followed by sentence punctuation (e.g. 'Spec written to docs/specs/test.md.'), Ghostty's link detection matches the entire string including the trailing period as a URL. The OPEN_URL handler then classifies it as a web URL (https://docs/specs/test.md.) instead of recognizing the underlying markdown file. Fix: before URL classification in the OPEN_URL handler, attempt to resolve the raw link text as a local file path using cmuxResolveQuicklookPath, which already handles trailing-punctuation trimming. If the path resolves to an existing file that cmux can preview (markdown or supported file type), route it through the file viewer instead of the browser. Refactoring: extracted three shared helpers into CommandClickFileOpenRouter: - resolveWorkingDirectory: consolidated the 3-step CWD lookup (panel dir → requested working dir → workspace dir) previously duplicated in GhosttyNSView.resolvedWordPathWorkingDirectory and the new code path. - deferredOpenFileInCmux: consolidated the lock-avoidance pattern for deferred file opens (manaflow-ai#3370/manaflow-ai#3560), previously duplicated inline in the file-URL and relative-path handlers. Handles workspace re-resolution, remote-surface re-check, TOCTOU re-validation, and fallback. - Replaced custom regex scheme detection with URL(string:)?.scheme == nil. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jasonko
force-pushed
the
fix/markdown-trailing-punctuation-4569
branch
from
May 22, 2026 23:46
9439724 to
82f8cfb
Compare
jasonko
marked this pull request as ready for review
May 22, 2026 23:49
Contributor
Greptile SummaryThis PR fixes a bug where Ghostty's link detection matched trailing sentence punctuation as part of file paths (e.g.
Confidence Score: 5/5Safe to merge — the pre-classification file-path check is well-guarded, both call sites supply a NSWorkspace fallback, and the refactored deferredOpenFileInCmux faithfully reproduces the original inline logic. The change is a targeted bug fix with clear guards (schemeless, non-absolute, local surface only) and a correct fallback chain. The extracted deferredOpenFileInCmux centralises an already-working pattern without altering its semantics. Tests cover the key regression scenarios. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[GHOSTTY_ACTION_OPEN_URL\nraw urlString] --> B{trimmedUrlString\nempty?}
B -- yes --> Z[return false]
B -- no --> C{isAbsolutePath?}
C -- yes --> G
C -- no --> D{URL scheme == nil?}
D -- no --> G
D -- yes --> E[performOnMain:\nresolveWorkingDirectory\ncmuxResolveQuicklookPath\nshouldRouteInCmux]
E -- path not found\nor not routable --> G
E -- file resolved --> F[deferredOpenFileInCmux\nnext runloop tick\nfallback: NSWorkspace.open]
F --> R[return true]
G[resolveTerminalOpenURLTarget] --> H{target?}
H -- nil --> Z2[return false]
H -- file URL\nlocalhost --> I[performOnMain:\ndeferredOpenFileInCmux\nfallback: NSWorkspace.open]
I --> R2[return true]
H -- external / browser --> J[NSWorkspace.open\nor embedded browser]
J --> R3[return true]
Reviews (2): Last reviewed commit: "Add fallback to relative-path deferred o..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4382-4392: The deferredOpenFileInCmux call currently returns true
immediately without supplying a fallback, so if workspace lookup or split
creation fails the click is swallowed; update the
CommandClickFileOpenRouter.deferredOpenFileInCmux invocation to pass a fallback
closure (same pattern used in the file:// branch) that attempts the
immediate/local open of resolvedPath (e.g., reusing the existing file-open
routine for this surface/workspace or calling the same handler used for file://
opens with workspace, surfaceId and resolvedPath) so failures in the async route
fall back to the immediate open instead of dropping the click.
🪄 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: 1562bf37-33a4-411d-8d74-ab8aa952eb57
📒 Files selected for processing (3)
Sources/CommandClickFileOpenRouter.swiftSources/GhosttyTerminalView.swiftcmuxTests/TerminalAndGhosttyTests.swift
Address CodeRabbit and Greptile review feedback: the relative-path branch of deferredOpenFileInCmux was called without a fallback closure, so if the deferred workspace lookup or split creation failed, the click was silently swallowed (the handler already returned true to Ghostty). Now passes an NSWorkspace.shared.open fallback matching the file:// path's pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
Re: Cmux Swift Moving just this call off-main while the surrounding code stays on-main would be inconsistent and provide no real benefit. A proper fix would require refactoring the entire OPEN_URL handler to separate actor-bound workspace resolution from file I/O — that is a larger change out of scope for this bug fix. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Fixes #4569
Problem
When terminal output contains a file path followed by sentence punctuation (e.g.
Spec written to docs/specs/2026-05-22-test.md.), Ghostty's link detection matches the entire string including the trailing period. TheOPEN_URLhandler classifies it as a web URL (https://docs/specs/2026-05-22-test.md.) and opens the browser sidebar instead of the markdown preview.Fix
Added a file-path resolution step in the
OPEN_URLhandler before URL classification. For strings without a URL scheme and that aren't absolute paths, the handler callscmuxResolveQuicklookPath— which already strips trailing sentence punctuation (.,,,;, etc.) — against the terminal's working directory. If the trimmed path resolves to an existing file that cmux can preview (markdown or supported file type), it routes through the file viewer instead of the browser.Guards
https://,file://,mailto:, etc.)resolveTerminalOpenURLTarget)statfor remote terminals)DispatchQueue.main.asyncsplit-creation pattern to avoid the Ghosttyos_unfair_lockdeadlock (openMarkdownInCmuxViewer: stable ignores setting (opens in system editor), nightly crashes with recursive os_unfair_lock in Surface.encodeKey #3370 / Architectural: dispatch Ghostty OPEN_URL callback async to remove cmux's runloop-tick lock workaround #3560)Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes incorrect URL detection for relative file paths with trailing punctuation by resolving local paths before URL classification, so docs/spec.md. opens in the file viewer instead of the browser. Adds a fallback to open externally if in-app routing fails. Fixes #4569.
Bug Fixes
cmuxResolveQuicklookPathto trim trailing punctuation and route previewable files through the viewer, with a system-opener fallback if routing fails.URL(string:)?.schemegate.Refactors
resolveWorkingDirectoryand used it across the link handler andGhosttyNSViewto unify CWD lookup.deferredOpenFileInCmuxto centralize deferred split creation with workspace re-resolution and TOCTOU checks; replaced regex scheme detection withURL(string:)?.scheme == nil.Written for commit 5a1e70e. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
Tests