fix: route osc 8 file links to plugin handlers - #2942
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change routes visible OSC 8 ChangesOSC 8 file link dispatch
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Explicit OSC 8 file targets can now reach matching plugin handlers while unmatched file links remain unconsumed; related tests pass, and no actionable merge-blocking risk remains beyond normal checks. Sequence Diagram(s)sequenceDiagram
participant User
participant url_at_pane_cell
participant PluginHandler
participant SystemURLOpener
User->>url_at_pane_cell: Ctrl-click visible hyperlink
url_at_pane_cell-->>User: Return hyperlink URI
User->>PluginHandler: Try plugin handler
PluginHandler-->>User: Return handled or unhandled
User->>SystemURLOpener: Open only unhandled safe web URL
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1📝 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 |
|
will this allow clicking to |
|
@akbash-bot the implementation review is clean. please rebase onto current master, resolve the changelog conflict, and rerun checks and both review bots. |
7ac49a6 to
1ece701
Compare
Greptile SummaryThis PR allows explicit OSC 8 targets to reach enabled plugin link handlers before applying the browser-opening allowlist.
Confidence Score: 5/5The PR appears safe to merge with the intended plugin-first routing and HTTP(S)-only system-opener boundary intact. The only production consumer of the newly unfiltered OSC 8 target invokes plugin handlers first and independently reapplies the web allowlist before reaching the system opener; unmatched non-web links remain unhandled as documented.
|
| Filename | Overview |
|---|---|
| src/app/actions.rs | Returns explicit OSC 8 URIs unchanged while retaining the existing web-only filter for plain-text URL detection. |
| src/app/input/mod.rs | Routes resolved links through plugin handlers first and gates system-opener fallback to HTTP(S). |
| src/app/input/terminal.rs | Adds an end-to-end input-path test proving matched file links invoke plugins and unmatched file links avoid the system opener. |
| docs/next/CHANGELOG.md | Accurately documents the new OSC 8 plugin-handler behavior and browser fallback boundary. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Modified click on pane cell] --> B{OSC 8 or detected URL?}
B -->|No| C[Continue normal mouse handling]
B -->|Yes| D{Plugin handler matches?}
D -->|Yes| E[Start plugin action]
D -->|No| F{HTTP or HTTPS?}
F -->|Yes| G[Open with system URL opener]
F -->|No| C
Reviews (1): Last reviewed commit: "fix: route osc 8 file links to plugin ha..." | Re-trigger Greptile
yes. ctrl-click is handled by herdr before pane mouse reporting, so the app can keep mouse support enabled. this only applies to explicit OSC 8 file:// links with a matching enabled plugin handler; plain file paths still do nothing. |
refs herdrdev#2941 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Issue
Ctrl-clicking an OSC 8
file://link does nothing even when an enabled plugin link handler matches it, while the same handler works for web links.Problem
Herdr applied its browser-opening web allowlist before checking plugin handlers, so explicit terminal links using other schemes were discarded before a plugin could match them.
How did we fix it?
Explicit OSC 8 targets now reach installed plugin handlers unchanged. The browser fallback remains a separate HTTP(S)-only boundary, so unmatched file links are neither consumed nor sent to the system URL opener; visible plain-text file paths also remain non-clickable.
Verification
Before the change, a production click-path test using the reported OSC 8
file:///tmp/herdr-file-repro.txttarget failed because the Ctrl-click was unhandled. The same test now starts the matching plugin action, and its unmatched case confirms the system opener is not called. Related URL-click, hyperlink resolution, plugin-context, Unix/Windows lint, maintenance, and 3,470 other Rust tests pass; one unrelated live-handoff test remains environment-failing because its replacement server does not start.refs #2941