Skip to content

feat: Semantic History — resolve bare filenames against OSC 7 CWD on click - #2200

Open
jiseongnoh wants to merge 4 commits into
manaflow-ai:mainfrom
jiseongnoh:feature/semantic-history-cwd-resolve
Open

jiseongnoh wants to merge 4 commits into
manaflow-ai:mainfrom
jiseongnoh:feature/semantic-history-cwd-resolve

Conversation

@jiseongnoh

@jiseongnoh jiseongnoh commented Mar 26, 2026 •

Copy link
Copy Markdown

Summary

When a user clicks a bare filename (e.g. README.md, src/main.py) in the terminal, resolve it against the terminal's current working directory (reported via OSC 7) and open it if the file exists on disk.

This is the terminal equivalent of iTerm2's Semantic History feature, and the #1 workflow gap for AI coding agent users (Claude Code, Codex, etc.) who frequently see bare filenames in output.

Changes

  • resolveTerminalOpenURLTarget() now accepts an optional workingDirectory parameter
  • Before falling through to browser URL heuristics (resolveBrowserNavigableURL), it checks whether CWD/clicked_text resolves to an existing file via FileManager.default.fileExists(atPath:)
  • Line/column suffixes (e.g. file.swift:42:10) are stripped before the file existence check
  • GHOSTTY_ACTION_OPEN_URL handler now retrieves the terminal's CWD from workspace.panelDirectories[surfaceId] (per-panel) or workspace.currentDirectory (workspace-level fallback)

Before / After

Scenario Before After
Click README.md Opens https://README.md (bug #1154) or no action Opens /Users/me/project/README.md in editor
Click src/main.py:42 No action Opens /Users/me/project/src/main.py in editor
Click /absolute/path.md Opens correctly ✅ No change ✅
Click https://google.com Opens correctly ✅ No change ✅
Click nonexistent.xyz Falls through Falls through (file doesn't exist, no change)

How it works

User clicks "file.md" in terminal output
  → resolveTerminalOpenURLTarget("file.md", workingDirectory: "/Users/me/project")
  → Not an absolute path
  → CWD check: does /Users/me/project/file.md exist?
  → Yes → return .external(URL(fileURLWithPath: "/Users/me/project/file.md"))
  → NSWorkspace.shared.open() → opens in default editor

Why this matters

AI coding agents output bare filenames constantly:

I've updated the following files:
  proposal_draft.md
  workflow_v2.md
  src/coordinator.ts

In iTerm2, every one of these is Cmd+clickable. In cmux, none of them are. This PR fixes that.

Related

Test plan

  • Click bare filename in terminal output → opens in editor
  • Click file.swift:42 → opens file (line number stripped)
  • Click absolute path → still works as before
  • Click URL → still works as before
  • Click non-existent filename → falls through to existing behavior
  • Works in split panes with different CWDs

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved file path resolution when opening files from terminal output: relative paths and bare filenames (including trailing :line or :line:column numeric suffixes) are now resolved against the terminal's working directory, and files are opened only if the resolved path exists.
  • Other
    • Enhanced debug logging for open-from-terminal actions to include the resolved working directory.

…click

When a user clicks a bare filename (e.g. `README.md`, `src/main.py`) in
the terminal, resolve it against the terminal's current working directory
(reported via OSC 7) and open it if the file exists on disk.

Changes:
- `resolveTerminalOpenURLTarget()` now accepts an optional `workingDirectory`
  parameter. Before falling through to browser URL heuristics, it checks
  whether `CWD + clicked_text` resolves to an existing file.
- Line/column suffixes (e.g. `file.swift:42:10`) are stripped before the
  file existence check.
- The `GHOSTTY_ACTION_OPEN_URL` handler now retrieves the terminal's CWD
  from `workspace.panelDirectories[surfaceId]` (per-panel) or
  `workspace.currentDirectory` (workspace-level fallback) and passes it
  to the resolver.

This matches iTerm2's "Semantic History" feature and is especially useful
for AI coding agents (Claude Code, Codex, etc.) that output bare filenames.

Closes manaflow-ai#2199
@vercel

vercel Bot commented Mar 26, 2026

Copy link
Copy Markdown

@jiseongnoh is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Mar 26, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ba3f44f8-7a55-4748-b4c6-08ab733444e7

📥 Commits

Reviewing files that changed from the base of the PR and between 6f9f3de and 8be163a.

📒 Files selected for processing (1)
  • Sources/GhosttyTerminalView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Sources/GhosttyTerminalView.swift

📝 Walkthrough

Walkthrough

Resolve logic for terminal-opened URLs now accepts an optional workingDirectory and, if initial URL parsing fails, treats non-absolute inputs (optionally stripping numeric :line[:column] suffixes) as relative paths resolved against that workingDirectory, returning an external file target only when the canonicalized path exists.

Changes

Cohort / File(s) Summary
Terminal URL resolution & handler
Sources/GhosttyTerminalView.swift
Changed resolveTerminalOpenURLTarget(_:, workingDirectory: String? = nil) signature. Added branch to treat trimmed, non-absolute inputs as bare filenames/relative paths by optionally stripping trailing numeric :line or :line:column, joining with workingDirectory (canonicalizing), and returning .external(fileURLWithPath:) only if the resolved path exists. Updated GHOSTTY_ACTION_OPEN_URL handling to compute terminal CWD from workspace.panelDirectories[surfaceId] ?? workspace.currentDirectory (using tabManagerFor(tabId:) fallback) and pass it into the resolver.

Sequence Diagram(s)

sequenceDiagram
  participant TerminalView
  participant TabManager
  participant Workspace
  participant Resolver
  participant FileSystem

  TerminalView->>TabManager: tabManagerFor(tabId)
  TabManager->>Workspace: read panelDirectories[surfaceId] / currentDirectory
  TabManager-->>TerminalView: terminalCWD
  TerminalView->>Resolver: resolveTerminalOpenURLTarget(rawValue, workingDirectory=terminalCWD)
  Resolver->>FileSystem: attempt URL parsing / scheme handling
  alt parse succeeds
    Resolver-->>TerminalView: return external or remote URL target
  else parse fails and input not absolute and workingDirectory != nil
    Resolver->>Resolver: strip trailing :line[:column] if numeric
    Resolver->>FileSystem: join workingDirectory + trimmedPath (canonicalize)
    FileSystem-->>Resolver: exists? (yes/no)
    alt exists
      Resolver-->>TerminalView: return .external(fileURLWithPath:)
    else
      Resolver-->>TerminalView: return nil / other fallback
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I sniff the cwd with twitching nose,

I trim the ":12:4" where numbers doze,
I hop the path and canonicalize,
If the file is real, I point your eyes.
A tiny rabbit, opening files with sighs.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding Semantic History functionality to resolve bare filenames against the terminal's OSC 7 working directory on click.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering summary, changes, before/after scenarios, implementation details, and test plan. However, the Testing section is absent from the description.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Mar 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR implements Semantic History — resolving bare filenames and relative paths (e.g. README.md, src/main.py:42) against the terminal's OSC 7 CWD when a user clicks them, matching iTerm2's behavior. The change is confined to Sources/GhosttyTerminalView.swift.\n\nWhat changed:\n- resolveTerminalOpenURLTarget() gains an optional workingDirectory: String? parameter; the CWD block is inserted after known-scheme URL checks to avoid misrouting real URLs.\n- Step 1 tries the raw clicked text appended to CWD (handles filenames that legitimately contain colons).\n- Step 2 strips trailing purely-numeric :line / :line:col suffixes and retries; guarded by !pathPart.isEmpty to prevent \":42\"-style shell error tokens from opening the CWD directory.\n- A path-traversal guard (hasPrefix(cwdPrefix) || == cwdCanonical) prevents ../ escape from the working directory.\n- The GHOSTTY_ACTION_OPEN_URL handler looks up CWD via app.tabManagerFor(tabId:) ?? app.tabManager — the established multi-window pattern used elsewhere in the file — with per-panel (panelDirectories[surfaceId]) then workspace-level (currentDirectory) fallback.\n\nAll three issues raised in the previous review round are addressed:\n- ✅ Multi-window CWD lookup now uses tabManagerFor(tabId:) ?? tabManager\n- ✅ Filenames containing colons are handled (step 1 tries raw value first)\n- ✅ Empty pathPart for inputs like \":42\" is guarded by !pathPart.isEmpty\n\nRemaining minor concern: knownSchemes is a Set<String> literal constructed on every call; hoisting it to a file-level private let would be cleaner (P2 only).

Confidence Score: 4/5

PR is safe to merge; core logic is correct, path-traversal is guarded, and all previously flagged issues are resolved.

All three P1/P0 issues from prior review rounds are addressed (multi-window lookup, colon-in-filename, empty-pathPart). The only remaining item is a P2 style suggestion to hoist knownSchemes to a file-level constant. Logic, edge cases, and security (path-traversal guard) are sound.

No files require special attention beyond the inline knownSchemes P2 note in Sources/GhosttyTerminalView.swift.

Important Files Changed

Filename Overview
Sources/GhosttyTerminalView.swift Adds Semantic History bare-filename resolution: resolveTerminalOpenURLTarget gains a workingDirectory param, tries raw CWD-appended path first (handles colons in names), then strips numeric :line:col suffixes before a second check; GHOSTTY_ACTION_OPEN_URL handler retrieves CWD via the established tabManagerFor(tabId:) ?? tabManager multi-window pattern. All three issues from the previous review round are addressed; one minor P2 remains (knownSchemes inline constant).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["User clicks text in terminal\n(GHOSTTY_ACTION_OPEN_URL)"] --> B["Lookup CWD\ntabManagerFor(tabId) ?? tabManager\n→ panelDirectories[surfaceId]\n  ?? currentDirectory"]
    B --> C["resolveTerminalOpenURLTarget(text, cwd)"]
    C --> D{Absolute path?\nstarts with /}
    D -- Yes --> E["Return .external(fileURL)"]
    D -- No --> F{Known URL scheme?\nhttp/https/ftp/file/…}
    F -- Yes --> G{http/https?}
    G -- Yes --> H["Return .embeddedBrowser(url)"]
    G -- No --> I["Return .external(url)"]
    F -- No --> J{CWD provided?}
    J -- No --> N
    J -- Yes --> K["Step 1: append raw text to CWD\nstandardize + path-traversal check\nFileManager.fileExists?"]
    K -- Exists --> L["Return .external(cwdResolvedURL)"]
    K -- Not found --> M["Step 2: strip :line/:col suffix\nretry CWD resolve + exists check"]
    M -- Exists --> L
    M -- Not found --> N["resolveBrowserNavigableURL()\n(bare-host heuristic)"]
    N -- Match --> O["Return .embeddedBrowser / .external"]
    N -- No match --> P["Return .external(fallback URL)\nor nil"]
Loading

Reviews (3): Last reviewed commit: "fix: comprehensive review round — knownS..." | Re-trigger Greptile

Comment on lines +2605 to +2614
let terminalCWD: String? = {
guard let tabId = surfaceView.tabId,
let surfaceId = surfaceView.terminalSurface?.id,
let tabManager = AppDelegate.shared?.tabManager,
let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else {
return nil
}
return workspace.panelDirectories[surfaceId]
?? workspace.currentDirectory
}()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Multi-window CWD lookup misses non-primary windows

The new CWD lookup hardcodes AppDelegate.shared?.tabManager, which is the primary window's tab manager. In a multi-window cmux setup the clicked terminal's tab may live in a secondary window, whose TabManager is a different object. When that happens, tabManager.tabs.first(where: { $0.id == tabId }) will never find the workspace, the guard fails, terminalCWD is nil, and the feature silently degrades to the old behavior (no bare-filename resolution) for every terminal except those in the primary window.

Every other CWD/tab lookup in the same file uses the tabManagerFor(tabId:) ?? tabManager pattern to handle this correctly — e.g. lines 1277, 2322, 2353, 6344–6345.

Suggested change
let terminalCWD: String? = {
guard let tabId = surfaceView.tabId,
let surfaceId = surfaceView.terminalSurface?.id,
let tabManager = AppDelegate.shared?.tabManager,
let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else {
return nil
}
return workspace.panelDirectories[surfaceId]
?? workspace.currentDirectory
}()
let terminalCWD: String? = {
guard let tabId = surfaceView.tabId,
let surfaceId = surfaceView.terminalSurface?.id,
let app = AppDelegate.shared,
let tabManager = app.tabManagerFor(tabId: tabId) ?? app.tabManager,
let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else {
return nil
}
return workspace.panelDirectories[surfaceId]
?? workspace.currentDirectory
}()

Comment thread Sources/GhosttyTerminalView.swift Outdated
// Semantic History: resolve bare filenames and relative paths against CWD.
// Strip trailing line/column suffix (e.g. "file.swift:42:10") before checking.
if let cwd = workingDirectory, !cwd.isEmpty {
let pathPart = trimmed.components(separatedBy: ":").first ?? trimmed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Colon-split discards the path when the filename itself contains a colon

trimmed.components(separatedBy: ":").first will silently drop everything after the first :. That is fine for file.swift:42, but filenames (especially those with Unicode in them — the PR's own example shows Korean filenames) can legally contain colons on macOS. A file named 사업계획서_초안:v2.md would be checked as 사업계획서_초안, which does not exist, and the resolution would fall through instead of opening the correct file.

A more robust strip is to only drop a suffix that matches the :<digits> line/column pattern:

Suggested change
let pathPart = trimmed.components(separatedBy: ":").first ?? trimmed
let pathPart: String = {
// Strip trailing ":line" or ":line:col" suffixes only.
// Using regex-free approach: check if every component after
// the first is purely numeric.
let parts = trimmed.components(separatedBy: ":")
var end = parts.count
while end > 1, parts[end - 1].allSatisfy(\.isNumber) {
end -= 1
}
return parts[..<end].joined(separator: ":")
}()

…w support

P2 fixes from code review:

1. Move CWD-based file resolution AFTER URL/scheme checks so that
   `localhost:3000`, `http://...`, etc. are never misrouted to local
   files even if a matching path exists in CWD.

2. Use `tabManagerFor(tabId:)` instead of `AppDelegate.shared?.tabManager`
   so Semantic History works in detached/secondary windows, not just the
   primary window.
…lback

1. Colon-split now only strips trailing numeric-only components
   (`:42`, `:42:10`) instead of splitting on the first colon.
   Filenames containing colons (legal on macOS) are preserved.

2. Add `?? app.tabManager` fallback to match codebase convention,
   ensuring CWD lookup works even if tabManagerFor returns nil.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/GhosttyTerminalView.swift`:
- Around line 2605-2614: The closure computing terminalCWD currently probes
AppDelegate.shared?.tabManager?.tabs (via surfaceView.tabId and
surfaceView.terminalSurface?.id) which only looks at the active window's
tabManager and can pick the wrong workspace in multi-window cases; replace that
lookup with AppDelegate.shared?.workspaceFor(tabId: tabId) (or equivalent API)
to find the owning Workspace across all mainWindowContexts and perform this
resolution on the main thread before using the result so
panelDirectories[surfaceId] and currentDirectory come from the correct
workspace.
- Around line 518-523: The current logic strips at the first ":"
unconditionally, which collapses whole-string URLs (e.g. "http://..." or
"file:///...") and causes them to be treated as local paths; update the branch
around workingDirectory to first detect full URLs by attempting URL(string:
trimmed) and checking url.scheme != nil (or checking for "://"/"file://"
prefix), and if it is a URL skip the CWD fallback entirely; only perform the
components(separatedBy: ":").first trimming and the
FileManager.default.fileExists(atPath:) resolution for inputs that are not
recognized as full URLs (reference variables/functions: workingDirectory,
trimmed, pathPart, resolved, FileManager.default.fileExists).
🪄 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: cf1109b7-f65d-423e-bab6-b2892853a54b

📥 Commits

Reviewing files that changed from the base of the PR and between 8a37815 and d5139f7.

📒 Files selected for processing (1)
  • Sources/GhosttyTerminalView.swift

Comment thread Sources/GhosttyTerminalView.swift Outdated
Comment on lines +2605 to +2614
let terminalCWD: String? = {
guard let tabId = surfaceView.tabId,
let surfaceId = surfaceView.terminalSurface?.id,
let tabManager = AppDelegate.shared?.tabManager,
let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else {
return nil
}
return workspace.panelDirectories[surfaceId]
?? workspace.currentDirectory
}()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resolve the panel CWD on main from the owning workspace.

This reads AppDelegate.shared?.tabManager?.tabs inside action_cb before hopping to main, and it only consults the active tab manager. In secondary-window cases that can pick the wrong workspace or miss the panel entirely, so bare filenames fall back incorrectly.

Suggested fix
-            let terminalCWD: String? = {
-                guard let tabId = surfaceView.tabId,
-                      let surfaceId = surfaceView.terminalSurface?.id,
-                      let tabManager = AppDelegate.shared?.tabManager,
-                      let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else {
-                    return nil
-                }
-                return workspace.panelDirectories[surfaceId]
-                    ?? workspace.currentDirectory
-            }()
+            let terminalCWD: String? = performOnMain {
+                guard let tabId = callbackTabId ?? surfaceView.tabId,
+                      let surfaceId = callbackSurfaceId ?? surfaceView.terminalSurface?.id,
+                      let workspace = AppDelegate.shared?.workspaceFor(tabId: tabId) else {
+                    return nil
+                }
+                return workspace.panelDirectories[surfaceId] ?? workspace.currentDirectory
+            }

Based on learnings, AppDelegate.shared?.workspaceFor(tabId: workspaceId) searches across all mainWindowContexts, whereas self.tabManager is active-window only.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 2605 - 2614, The closure
computing terminalCWD currently probes AppDelegate.shared?.tabManager?.tabs (via
surfaceView.tabId and surfaceView.terminalSurface?.id) which only looks at the
active window's tabManager and can pick the wrong workspace in multi-window
cases; replace that lookup with AppDelegate.shared?.workspaceFor(tabId: tabId)
(or equivalent API) to find the owning Workspace across all mainWindowContexts
and perform this resolution on the main thread before using the result so
panelDirectories[surfaceId] and currentDirectory come from the correct
workspace.

@jiseongnoh

Copy link
Copy Markdown
Author

@coderabbitai review

All issues from the initial review have been addressed in follow-up commits:

  1. CWD check moved after URL/scheme checks (commit 8649b32) — localhost:3000 and http://... are no longer misrouted to local files.
  2. Multi-window support (commit 8649b32 + 6f9f3de) — Now uses tabManagerFor(tabId:) ?? app.tabManager fallback pattern, matching the codebase convention.
  3. Smart colon stripping (commit 6f9f3de) — Only strips trailing numeric-only :line or :line:col suffixes. Filenames with colons are preserved.

Please re-review the latest commit (6f9f3de).

@jiseongnoh

Copy link
Copy Markdown
Author

@greptile-apps review

All P1 issues have been fixed:

  1. Multi-window CWD lookup → Now uses tabManagerFor(tabId:) ?? app.tabManager (commit 8649b32 + 6f9f3de)
  2. Colon-split → Now only strips trailing numeric-only components (:42, :42:10). Filenames with colons are preserved (commit 6f9f3de)

Please re-review the latest state.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Sources/GhosttyTerminalView.swift">

<violation number="1" location="Sources/GhosttyTerminalView.swift:542">
P2: Semantic-history CWD resolution is bypassed for bare filenames containing `:line`/`:col` because URL parsing happens first; scheme-like strings such as `file.swift:42` return early as external URLs, so the new suffix-stripping CWD resolution never runs.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Sources/GhosttyTerminalView.swift

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
Sources/GhosttyTerminalView.swift (1)

2615-2628: ⚠️ Potential issue | 🟠 Major

Resolve the panel CWD on main from the callback ids.

This still derives terminalCWD from surfaceView state inside the action callback, even though callbackTabId and callbackSurfaceId were already captured above. During window/panel reattachment that can read a stale workspace/panel mapping and open the wrong file or fall through when the file exists.

Suggested fix
-            let terminalCWD: String? = {
-                guard let tabId = surfaceView.tabId,
-                      let surfaceId = surfaceView.terminalSurface?.id,
-                      let app = AppDelegate.shared,
-                      let tabManager = app.tabManagerFor(tabId: tabId) ?? app.tabManager,
-                      let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else {
-                    return nil
-                }
-                return workspace.panelDirectories[surfaceId]
-                    ?? workspace.currentDirectory
-            }()
+            let terminalCWD: String? = performOnMain {
+                guard let tabId = callbackTabId ?? surfaceView.tabId,
+                      let surfaceId = callbackSurfaceId ?? surfaceView.terminalSurface?.id,
+                      let workspace = AppDelegate.shared?.workspaceFor(tabId: tabId) else {
+                    return nil
+                }
+                return workspace.panelDirectories[surfaceId] ?? workspace.currentDirectory
+            }
             guard let target = resolveTerminalOpenURLTarget(urlString, workingDirectory: terminalCWD) else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 2615 - 2628, The code
currently computes terminalCWD from surfaceView inside the action callback,
which can be stale; instead, on the main thread resolve the workspace and panel
directory using the captured callbackTabId and callbackSurfaceId: fetch
AppDelegate.shared, call app.tabManagerFor(tabId: callbackTabId) ??
app.tabManager, find the workspace with id == callbackTabId, then derive
terminalCWD from workspace.panelDirectories[callbackSurfaceId] ??
workspace.currentDirectory, and pass that into
resolveTerminalOpenURLTarget(urlString, workingDirectory:); update references to
terminalCWD and remove reliance on surfaceView in this callback.
🤖 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/GhosttyTerminalView.swift`:
- Around line 542-559: The existence check currently only tests the stripped
pathPart, which prevents literal filenames like "foo:42" from resolving; update
the logic in the block that uses workingDirectory, trimmed, pathPart and
resolved so it first constructs resolved = (cwd as
NSString).appendingPathComponent(trimmed) and calls
FileManager.default.fileExists(atPath: resolved) and returns
.external(URL(fileURLWithPath: resolved)) if found, and only if that fails then
compute the stripped pathPart (the existing parts/trimming loop) and, if the
stripped result is non-empty and different from trimmed, construct the
alternative resolved and test FileManager.default.fileExists(atPath:) again
before returning .external.

---

Duplicate comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2615-2628: The code currently computes terminalCWD from
surfaceView inside the action callback, which can be stale; instead, on the main
thread resolve the workspace and panel directory using the captured
callbackTabId and callbackSurfaceId: fetch AppDelegate.shared, call
app.tabManagerFor(tabId: callbackTabId) ?? app.tabManager, find the workspace
with id == callbackTabId, then derive terminalCWD from
workspace.panelDirectories[callbackSurfaceId] ?? workspace.currentDirectory, and
pass that into resolveTerminalOpenURLTarget(urlString, workingDirectory:);
update references to terminalCWD and remove reliance on surfaceView in this
callback.
🪄 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: 9a452c45-28fc-4449-9e9a-2eea69a50f79

📥 Commits

Reviewing files that changed from the base of the PR and between d5139f7 and 6f9f3de.

📒 Files selected for processing (1)
  • Sources/GhosttyTerminalView.swift

Comment thread Sources/GhosttyTerminalView.swift
@coderabbitai

coderabbitai Bot commented Mar 26, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the latest PR diff and commit details
gh pr view 2200 --repo manaflow-ai/cmux --json title,body,commits,headRefName | jq '{title, headRefName, commits: [.commits[] | {oid, messageHeadline}]}'

echo "=== PR Files Changed ==="
gh pr diff 2200 --repo manaflow-ai/cmux

Length of output: 4162


🏁 Script executed:

#!/bin/bash
# Check type of tabManager in AppDelegate
rg -n "var tabManager" --type swift -A2

echo "=== tabManagerFor(tabId:) signature ==="
rg -n "func tabManagerFor" --type swift -A3

echo "=== panelDirectories definition ==="
rg -n "panelDirectories" --type swift -A2 -B2

Length of output: 17546


@jiseongnoh Thanks for the follow-up commits — the three fixes are well executed. Here's what the re-review found:


✅ Fix 1 — CWD check after URL/scheme checks

The new block is correctly placed after the URL/scheme resolution path, with an explanatory comment. localhost:3000 and http://... are handled first and returned early. No issues.

✅ Fix 2 — Multi-window support

AppDelegate.tabManager is declared as weak var tabManager: TabManager?, so the ?? expression returns TabManager? and guard let tabManager = app.tabManagerFor(tabId: tabId) ?? app.tabManager is valid Swift — no redundancy warning. The fallback pattern matches the codebase convention used in TerminalController.swift. ✅

✅ Fix 3 — Smart colon stripping

The while end > 1, parts[end - 1].allSatisfy(\.isNumber) loop correctly strips only trailing purely-numeric components, preserving filenames that legitimately contain colons. ✅


🔴 Remaining issue — Absolute paths not guarded in CWD block

The CWD block has no guard for absolute paths (e.g. /tmp/foo.txt typed in terminal output). NSString.appendingPathComponent(_:) normalises away the leading / from the component being appended:

("/home/user" as NSString).appendingPathComponent("/tmp/foo.txt")
// → "/home/user/tmp/foo.txt"  ← not "/tmp/foo.txt"

So an absolute path that isn't caught by the URL/scheme check earlier reaches the CWD block, gets rewritten to CWD/tmp/foo.txt, and if a file happens to exist there it will be silently misrouted instead of opening the actual /tmp/foo.txt.

Suggested fix — add !pathPart.hasPrefix("/") to the guard:

if let cwd = workingDirectory, !cwd.isEmpty {
    let pathPart: String = { ... }()
    // Skip absolute paths — they must not be joined against the CWD.
    guard !pathPart.hasPrefix("/") else { break }
    let resolved = (cwd as NSString).appendingPathComponent(pathPart)
    if FileManager.default.fileExists(atPath: resolved) { ... }
}

Or equivalently, inline the guard in the if condition:

if let cwd = workingDirectory, !cwd.isEmpty, !pathPart.hasPrefix("/") {

💡 Minor follow-up (not a blocker)

The stripped :line / :line:col suffix is discarded after being used for the existence check — the editor opens at line 1. The PR description notes "ideally at the specified line" as a future improvement. Worth tracking as a follow-up issue if desired.


🧠 Learnings used
Learnt from: homanp
Repo: manaflow-ai/cmux PR: 883
File: CLI/cmux.swift:1569-1586
Timestamp: 2026-03-04T22:05:07.913Z
Learning: In manaflow-ai/cmux CLI (CLI/cmux.swift), for parity with existing browser commands, markdown.open intentionally normalizes --surface before resolving --workspace/--window, relying on server-side resolution to disambiguate final routing. Avoid one-off reordering in markdown; consider any change only as a coordinated, cross-command refactor.

Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/ContentView.swift:2156-2171
Timestamp: 2026-03-23T06:08:14.740Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, openFileInTextEditor(_:) must attempt workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that returns nil, fall back to workspace.newTextEditorSurface(inPane:filePath:focus:) using bonsplitController.focusedPaneId. Rationale: avoid dropping file-open requests when the focused panel is not pane-backed.

Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:13.558Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, socket v2 methods that accept surface_id/panel_id must handle cross-window routing. For panel background state mutations, v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:) first try v2ResolveTabManager(params:), but only accept it if that manager actually owns the panelId; otherwise they fall back to AppDelegate.shared?.locateSurface(surfaceId:) to find the correct TabManager across windows. This pattern should be used for any panel_id-only routes to avoid active-window bias.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment on lines +546 to +551
let parts = trimmed.components(separatedBy: ":")
var end = parts.count
while end > 1, parts[end - 1].allSatisfy(\.isNumber) {
end -= 1
}
return parts[..<end].joined(separator: ":")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Empty pathPart when input starts with :

"".allSatisfy(\.isNumber) returns true in Swift (vacuous truth over an empty collection). This means the stripping loop also removes empty components — not just purely-numeric ones. Concretely, for an input like ":42":

  • parts = ["", "42"]
  • "42" is all-digits → end goes to 1
  • "" ends the loop (end > 1 is false)
  • pathPart = ""

(cwd as NSString).appendingPathComponent("") returns cwd unchanged, and FileManager.default.fileExists(atPath: cwd) will return true for any existing working directory. The result is that clicking ":42" (which can appear in shell error messages like bash: line :42: …) would cause the CWD directory itself to be opened in Finder/editor.

The fix is to add a guard after the closure, or add !last.isEmpty to the while predicate:

guard !pathPart.isEmpty else { /* fall through to resolveBrowserNavigableURL */ }

…esolve, path traversal guard, root CWD fix

Addresses all findings from 5 AI reviewers (Codex, Greptile, CodeRabbit, cubic-dev, + 3 internal agents):

1. knownSchemes allowlist (cubic-dev P2): URL(string: "file.swift:42") parses
   scheme="file.swift" via RFC 3986. Only known schemes (http, https, ftp,
   ssh, mailto, file, etc.) are now treated as real URLs. Unknown schemes
   fall through to CWD resolution, fixing the most common Semantic History
   use case (compiler output like `file.swift:42:10`).

2. Raw-first resolve (CodeRabbit Major): Try the literal clicked text against
   CWD before stripping :line:col suffixes. Handles filenames that legitimately
   contain colons (legal on macOS).

3. Path traversal guard (security): Resolved paths are validated against
   CWD prefix via standardizingPath to prevent `../../etc/passwd` from
   opening files outside the working directory.

4. Root CWD fix (Codex P2): When CWD is "/", cwdPrefix is "/" not "//",
   preventing the containment check from rejecting all valid child paths.

5. Empty-string guard (Greptile P1): `!parts[end-1].isEmpty` prevents
   vacuous truth from `"".allSatisfy(\.isNumber)` stripping empty components.
@jiseongnoh

Copy link
Copy Markdown
Author

@coderabbitai review
@greptile-apps review

All findings from all reviewers have been addressed in commit 8be163a:

  1. knownSchemes allowlist (cubic-dev P2) — file.swift:42 no longer parsed as scheme. Only known schemes (http, https, ftp, ssh, mailto, file, etc.) treated as URLs.
  2. Raw-first resolve (CodeRabbit Major) — Literal clicked text tried against CWD before stripping :line:col suffixes. Handles filenames with colons.
  3. Path traversal guard (internal review) — standardizingPath + prefix check prevents ../../etc/passwd from escaping CWD.
  4. Root CWD fix (Codex P2) — cwdPrefix is "/" not "//" when CWD is /.
  5. Empty-string guard (Greptile P1) — !parts[end-1].isEmpty prevents vacuous truth stripping.

Please re-review the latest commit.

@coderabbitai

coderabbitai Bot commented Mar 26, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the latest PR diff for commit 8be163a
gh pr view 2200 --repo manaflow-ai/cmux --json title,commits | jq '{title, commits: [.commits[] | {oid, messageHeadline}]}'

echo "=== Latest diff ==="
gh pr diff 2200 --repo manaflow-ai/cmux

Length of output: 6142


@jiseongnoh All five fixes in commit 8be163a look correct. Here's the verification:


✅ Fix 1 — knownSchemes allowlist

The knownSchemes: Set<String> is properly gated in the URL(string:) branch. file.swift:42 (scheme file.swift) no longer short-circuits into the URL path.

✅ Fix 2 — Raw-first resolve

Step 1 tries the literal trimmed text against the CWD, then Step 2 strips trailing numeric :line/:line:col components and tries again. Filenames that legitimately contain colons are handled correctly.

✅ Fix 3 — Path traversal guard

standardizingPath + hasPrefix(cwdPrefix) || == cwdCanonical is applied to both the raw and stripped branches. A traversal like ../../etc/passwd standardizes to /etc/passwd, which fails the prefix check, and is safely discarded.

✅ Fix 4 — Root CWD prefix

cwdPrefix = cwdCanonical == "/" ? "/" : cwdCanonical + "/" correctly avoids producing "//" when CWD is /.

✅ Fix 5 — Empty-string guard

!parts[end - 1].isEmpty is present in the while condition, preventing vacuous stripping of empty components from a trailing colon.


✅ Previous issue — Absolute paths in CWD block

The prior concern is also resolved. Absolute paths (/tmp/foo.txt) return early from if trimmed.hasPrefix("/") before the CWD block is ever reached. Additionally, the path traversal guard would catch any edge case where an absolute path slipped through.


💡 Nit (non-blocking)

rawCanonical == cwdCanonical in the guard condition could match a directory (since FileManager.fileExists returns true for directories). This would only trigger if someone clicked text that is literally the CWD path itself — an unlikely edge case. Worth a follow-up issue if you want to add an isDirectory check, but not a blocker.

The implementation looks solid. Ready to merge. 🎉


🧠 Learnings used
Learnt from: homanp
Repo: manaflow-ai/cmux PR: 883
File: CLI/cmux.swift:1569-1586
Timestamp: 2026-03-04T22:05:07.913Z
Learning: In manaflow-ai/cmux CLI (CLI/cmux.swift), for parity with existing browser commands, markdown.open intentionally normalizes --surface before resolving --workspace/--window, relying on server-side resolution to disambiguate final routing. Avoid one-off reordering in markdown; consider any change only as a coordinated, cross-command refactor.

Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/ContentView.swift:2156-2171
Timestamp: 2026-03-23T06:08:14.740Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, openFileInTextEditor(_:) must attempt workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that returns nil, fall back to workspace.newTextEditorSurface(inPane:filePath:focus:) using bonsplitController.focusedPaneId. Rationale: avoid dropping file-open requests when the focused panel is not pane-backed.

Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:13.558Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, socket v2 methods that accept surface_id/panel_id must handle cross-window routing. For panel background state mutations, v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:) first try v2ResolveTabManager(params:), but only accept it if that manager actually owns the panelId; otherwise they fall back to AppDelegate.shared?.locateSurface(surfaceId:) to find the correct TabManager across windows. This pattern should be used for any panel_id-only routes to avoid active-window bias.

Learnt from: jt-hsiao
Repo: manaflow-ai/cmux PR: 1423
File: Sources/AppDelegate.swift:11220-11226
Timestamp: 2026-03-14T07:06:01.466Z
Learning: Repo: manaflow-ai/cmux — In Sources/Panels/CmuxWebView.swift, performKeyEquivalent(with:) handles Command-key routing end-to-end: (1) if allowed, route to NSApp.mainMenu.performKeyEquivalent; (2) fall back to AppDelegate.shared?.handleBrowserSurfaceKeyEquivalent(event) for non–menu-backed app shortcuts; (3) fall back to super.performKeyEquivalent. For non-Command keys it calls super directly. Therefore, in NSWindow.cmux_performKeyEquivalent (Sources/AppDelegate.swift), it is correct to call firstResponderWebView.performKeyEquivalent and return its Bool unconditionally to avoid re-entering SwiftUI’s performKeyEquivalent path that can swallow keys after WKWebView focus.

Learnt from: outoftime
Repo: manaflow-ai/cmux PR: 1528
File: Sources/GhosttyTerminalView.swift:3220-3228
Timestamp: 2026-03-17T18:25:33.286Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift within TerminalSurface.createSurface(for:), when constructing XDG_DATA_DIRS for Fish vendor_conf.d auto-sourcing, treat empty or whitespace-only values from initialEnvironmentOverrides, env, getenv, and ProcessInfo as unset before prefixing the integrationDir. This avoids producing a trailing colon. Keep XDG_DATA_DIRS in protectedStartupEnvironmentKeys so initialEnvironmentOverrides cannot overwrite the prefixed value.

Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift and Sources/Panels/BrowserPanel.swift, BiDi override (U+202A–202E, U+2066–2069) and zero-width char (U+200B–200F, U+FEFF) filtering is implemented via a shared `dangerousScalars: Set<UInt32>` in each class. v2SanitizeWebText() truncates to 200 chars; v2SanitizeXPath() caps at 2000 chars for selector fidelity. Both use a shared v2SanitizeScalar() predicate. BrowserPickerMessageHandler.sanitize() uses the same dangerousScalars pattern with a 200-char cap.

Learnt from: arieltobiana
Repo: manaflow-ai/cmux PR: 1873
File: Sources/TerminalController.swift:4071-4087
Timestamp: 2026-03-20T17:18:30.333Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceAction(params:) -> case "set_color": palette names are resolved via WorkspaceTabColorSettings.defaultPaletteWithOverrides(), whose entries are always valid hex (validated by the UI). Therefore, additional normalization of entry.hex is unnecessary.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2034
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-25T07:14:56.211Z
Learning: Repo: manaflow-ai/cmux — Sources/AppDelegate.swift — Pattern for “Open Folder”: In AppDelegate.showOpenFolderPanel(), seed NSOpenPanel.directoryURL using preferredMainWindowContextForWorkspaceCreation(debugSource: …) rather than NSApp.keyWindow, and on selection delegate to openWorkspaceForExternalDirectory(workingDirectory:…, debugSource: …). Rationale: handles auxiliary-key-window cases, ensures shouldBringToFront = true, and unifies menu/shortcut behavior with a consistent fallback to createMainWindow when workspace creation returns nil.

Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: CLI/cmux.swift:1948-1978
Timestamp: 2026-03-25T00:33:26.452Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift, the set-workspace-color command requires exactly one trailing <hex> argument and enforces a strict 6-digit hex format (`#RRGGBB`, optional leading '#'); clear-workspace-color rejects any unexpected positional args beyond --workspace. This matches server-side normalization and prevents malformed inputs.

Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:28:45.700Z
Learning: Repo: manaflow-ai/cmux — In Sources/Panels/BrowserPanel.swift, BrowserPickerMessageHandler uses three sanitizer methods that mirror TerminalController exactly: filtered(_:) strips control chars and dangerousScalars (BiDi/zero-width); sanitizeWebText(_:) applies filtered + prefix(200) + trim for display text; sanitizeXPath(_:) applies filtered + prefix(2000) + trim for selector fidelity. sanitizeXPath is called on body["xpath"] and sanitizeWebText on body["text"] in userContentController(_:didReceive:).

Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:54.956Z
Learning: Repo: manaflow-ai/cmux
PR: `#1089`
File: CLI/cmux.swift
Component: ClaudeHookTagExtractor.extractTags(subtitle:body:)
Learning: For Claude Code session tag extraction, pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.

Learnt from: outoftime
Repo: manaflow-ai/cmux PR: 1528
File: Resources/shell-integration/fish/vendor_conf.d/cmux-fish-integration.fish:541-546
Timestamp: 2026-03-17T13:59:10.665Z
Learning: Repo: manaflow-ai/cmux — The socket command `report_git_branch` (parsed in Sources/TerminalController.swift) expects the branch name as an **unquoted, bare token**. Wrapping the branch name in double quotes causes it to be silently discarded by the parser. This matches the bash/zsh shell integration convention. Do not suggest quoting the branch argument for this command; the fix was attempted in commit 829437c7 and immediately reverted because it broke branch reporting.
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Sources/GhosttyTerminalView.swift">

<violation number="1" location="Sources/GhosttyTerminalView.swift:557">
P2: CWD prefix guard blocks legitimate parent-relative paths (e.g. `../foo.swift`), so relative file links outside the CWD subtree no longer resolve even when the file exists.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

// contain colons (e.g. a file literally named "foo:42").
let rawResolved = (cwd as NSString).appendingPathComponent(trimmed)
let rawCanonical = (rawResolved as NSString).standardizingPath
if rawCanonical.hasPrefix(cwdPrefix) || rawCanonical == cwdCanonical,

@cubic-dev-ai cubic-dev-ai Bot Mar 26, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: CWD prefix guard blocks legitimate parent-relative paths (e.g. ../foo.swift), so relative file links outside the CWD subtree no longer resolve even when the file exists.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 557:

<comment>CWD prefix guard blocks legitimate parent-relative paths (e.g. `../foo.swift`), so relative file links outside the CWD subtree no longer resolve even when the file exists.</comment>

<file context>
@@ -536,26 +544,44 @@ func resolveTerminalOpenURLTarget(_ rawValue: String, workingDirectory: String?
+        //    contain colons (e.g. a file literally named "foo:42").
+        let rawResolved = (cwd as NSString).appendingPathComponent(trimmed)
+        let rawCanonical = (rawResolved as NSString).standardizingPath
+        if rawCanonical.hasPrefix(cwdPrefix) || rawCanonical == cwdCanonical,
+           FileManager.default.fileExists(atPath: rawCanonical) {
+            #if DEBUG
</file context>
Fix with Cubic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional — the CWD containment check is a deliberate security boundary to prevent path traversal attacks (e.g. a malicious server injecting ../../etc/passwd in terminal output).

../foo.swift being blocked is the expected trade-off. In practice:

  • Compiler/linter output uses CWD-relative paths (e.g. src/main.swift:42), not parent-relative paths
  • If a user cds into a subdirectory, OSC 7 updates the CWD, so file.swift in the new CWD still resolves correctly

If maintainers prefer a more permissive approach, we could relax the guard to allow realpath-resolved paths that stay within the project root (e.g. git toplevel), but that adds complexity. Happy to adjust based on project preference.

@jiseongnoh

Copy link
Copy Markdown
Author

Re: @cubic-dev-ai path traversal guard blocking ../foo.swift

This is intentional in the current implementation. However, iTerm2 Semantic History has no such restriction — it resolves ../foo.swift freely via realpath, with no CWD containment check. Their reasoning: clicking a link in terminal output is a deliberate user action, not automatic execution.

Two options for maintainers:

Option A: Keep guard (current) — ../foo.swift blocked, stricter security, some legitimate paths (monorepo cross-package refs) will not resolve.

Option B: Remove guard (iTerm2-style) — ../foo.swift opens correctly, matches iTerm2 behavior, user is responsible for what they click.

Happy to go either direction. If Option B is preferred, removing the hasPrefix(cwdPrefix) guard is a one-line change per branch.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: terminal Ghostty surface, rendering, scrollback, escape sequences, fonts S3: minor Wrong behavior with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Semantic History — resolve bare filenames against OSC 7 CWD on Cmd+click

2 participants