Skip to content

fix: respect mouse capture when routing hyperlink clicks - #5064

Merged
JJLiebig merged 1 commit into
masterfrom
issue/4874-osc8-plugin-links
Oct 7, 2026
Merged

JJLiebig merged 1 commit into
masterfrom
issue/4874-osc8-plugin-links

Conversation

@JJLiebig

@JJLiebig JJLiebig commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Windows Terminal can open a hyperlink before Herdr receives the Ctrl-click, bypassing matching plugins such as the Obsidian handler in #4874.

This uses the existing mouse-capture setting to choose who handles links:

  • Capture on, the default: Herdr keeps link destinations internally and lets matching plugins handle Ctrl-click first. A plugin launch failure shows an error and stops. Without a matching plugin, only HTTP(S) links open externally in the client's default browser.
  • Capture off: Herdr passes explicit hyperlinks to the outer terminal. The terminal uses its normal shortcuts and warnings. These clicks do not invoke plugins.

Capture on suppresses native opening of labeled links, including macOS Command-click. Switch capture off to restore it. This also disables Herdr's mouse controls, including clicking tabs and dragging split borders.

Windows Terminal can still intercept URLs it detects in visible text. The docs explain setting "experimental.detectURLs": false when reliable plugin priority is needed for those URLs. Herdr does not change terminal settings.

The file/app opener and confirmation dialog are deferred. There is no new setting, dependency or protocol change. The HTTP(S)-only fallback from #2942 is preserved.

Validation:

  • Host-terminal playback verifies link destinations across capture changes, full frames, diffs and both cursor modes.
  • Click tests cover plugin claims, launch failures, HTTP(S) fallback and capture off with a mouse-reporting pane app.
  • Full Windows just check passed with 3,378 Rust tests and 12 existing skips. Formatting, Clippy and script checks passed.
  • The release build and isolated native Windows plugin smoke test passed. The main server remained running and compatible.
  • Physical Windows Terminal clicks passed. Capture on invoked the matching plugin and opened an unmatched web link in the browser. After a live switch to capture off, both web links opened in the browser and the file link opened in Notepad. No additional plugin invocation was recorded. The test session and pane were removed.

Refs #4874.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: eb46b630-0b73-43f8-ab51-bebdf6a9d35b
📥 Commits

Reviewing files that changed from the base of the PR and between 4f296c2 and 9e90688.

📒 Files selected for processing (14)
  • docs/next/website/src/content/docs/ja/plugins.mdx
  • docs/next/website/src/content/docs/ja/quick-start.mdx
  • docs/next/website/src/content/docs/plugins.mdx
  • docs/next/website/src/content/docs/quick-start.mdx
  • docs/next/website/src/content/docs/zh-cn/plugins.mdx
  • docs/next/website/src/content/docs/zh-cn/quick-start.mdx
  • src/app/api/plugins/mod.rs
  • src/client/shell/actions.rs
  • src/client/shell/link_hover.rs
  • src/client/shell/mouse.rs
  • src/client/shell/tests/input.rs
  • src/client/shell/tests/link_hover.rs
  • src/client/shell/tests/mouse_selection.rs
  • src/client/state.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Link activation, fallback replay, and link-hover processing now depend on mouse capture. Failed plugin link handlers return an error and consume the click without fallback replay. Frame presentation can write to a supplied writer and clears frame hyperlinks when shell mouse capture is preferred. Tests cover delayed replies, link routing, hover behavior, and hyperlink composition. Documentation describes link handling for both mouse-capture settings.

Suggested reviewers: ogulcancelik

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 9e906

The reviewed link-routing changes have no identified issue requiring a fix before merge; normal checks can proceed.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: hyperlink routing now respects the mouse-capture setting.
Description check ✅ Passed The description directly explains the mouse-capture behavior, hyperlink routing, plugin handling, fallback behavior, documentation updates, and validation results.
Full details: Docstring Coverage

Explanation

Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@kangal-bot

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

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.

@JJLiebig JJLiebig added the ai-review Trigger automated AI reviews for pull requests admitted by the PR gate label Oct 7, 2026
@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Adjusts how mouse capture affects link handling in the terminal UI.

The PR appears safe to merge; no actionable issue was found in the current changes.

What we checked:

  • Links follow capture changes: The encoder uses checked lookups for destinations. It also compares resolved destinations between frames, so changing capture causes linked cells to be redrawn.
  • Rejected plugins stop fallback: The client shows endpoint errors before handling the result. The plugin_link_failed branch returns no follow-up action and consumes the release if it has not arrived.

Summary

This PR uses the existing mouse-capture setting to choose who handles explicit hyperlinks.

  • With capture enabled, Herdr keeps destinations internally and gives matching plugins first choice.
  • With capture disabled, the outer terminal receives explicit hyperlinks.
  • plugin_link_failed stops fallback and shows an error.
  • Tests cover capture changes, delayed replies, plugin claims, and both cursor modes. English, Japanese, and Chinese docs explain the behavior and the Windows Terminal caveat.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Link[Explicit pane hyperlink] --> Capture{Mouse capture enabled?}
  Capture -->|No| Host[Outer terminal handles the link]
  Capture -->|Yes| Click[Herdr receives Ctrl-click]
  Click --> Plugin{Matching enabled plugin?}
  Plugin -->|Yes| Launch[Start plugin action]
  Launch -->|Rejected| Error[Show error and stop]
  Launch -->|Accepted| Stop[No browser fallback]
  Plugin -->|No| Web{HTTP or HTTPS and click still current?}
  Web -->|Yes| Browser[Open client default browser]
  Web -->|No| Other[No external opening]
Loading

Reviews (2) · Last reviewed commit: "fix: respect mouse capture when routing ..." · Reviewed by Greptile

greptile-apps[bot]
greptile-apps Bot previously approved these changes Oct 7, 2026
@JJLiebig
JJLiebig marked this pull request as draft October 7, 2026 22:29
@JJLiebig

JJLiebig commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

Hmm not merge ready fo sho, pretty sure we break file:/// and similar things

@JJLiebig
JJLiebig force-pushed the issue/4874-osc8-plugin-links branch from 7512870 to 0d90e59 Compare October 7, 2026 22:56
@JJLiebig JJLiebig changed the title fix: keep hyperlink clicks in herdr fix: route hyperlink clicks through plugins before opening apps Oct 7, 2026
@JJLiebig
JJLiebig force-pushed the issue/4874-osc8-plugin-links branch from 0d90e59 to 9e90688 Compare October 7, 2026 23:15
@JJLiebig JJLiebig changed the title fix: route hyperlink clicks through plugins before opening apps fix: respect mouse capture when routing hyperlink clicks Oct 7, 2026
@JJLiebig
JJLiebig marked this pull request as ready for review October 7, 2026 23:15
@JJLiebig

JJLiebig commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review
@greptileai

Please review the current head, 9e90688. This revision uses the existing mouse-capture setting to choose link ownership and removes the file/app opener and confirmation dialog. Full Windows checks and the isolated plugin smoke test passed.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@akbash-bot akbash-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 9e90688. No blocking findings.

Capture-based link ownership is consistent across presentation, hover, and click handling. Internal destinations remain available to plugins, native links return when capture is disabled, and plugin launch errors do not trigger an external fallback. Direct terminal attach retains its separate ANSI path. The HTTP(S)-only opener and documented native-shortcut tradeoff keep this change bounded.

Local verification: just test-one link passed all 102 selected tests. All PR checks are green. An independent code review found no material issues. JJ supplied the physical Windows Terminal verification; I did not repeat that test on Windows.

@JJLiebig
JJLiebig merged commit 4dc23bb into master Oct 7, 2026
9 checks passed
@JJLiebig
JJLiebig deleted the issue/4874-osc8-plugin-links branch October 7, 2026 23:30
alfonsodg added a commit to alfonsodg/herdr that referenced this pull request Oct 8, 2026
maciejul pushed a commit to maciejul/herdr-supercharged that referenced this pull request Oct 28, 2026
haasanen added a commit to haasanen/herdr that referenced this pull request Nov 5, 2026
…am test literal

- mouse.rs: proper fork/base/upstream 3-way (git merge-file). Restores the
  fork's workspace context-menu click path (silently dropped when taking
  the pure v0.9.5 blob: fork CI clippy flagged the now-dead menu_button
  field + 3 chrome_context tests red) while keeping upstream's
  sidebar-toggle relocation (herdrdev#4734) and hyperlink mouse-capture fix (herdrdev#5064).
- platform/*: move supports_browser_open ahead of mod tests
  (clippy items_after_test_module), cargo fmt.
- headless tests: add browser_open field to upstream's new ServerEvent
  literal (E0063 test-target only; cargo check --workspace --tests gates).

Local full-suite residuals proven environmental, not regressions:
container SIGPIPE collateral (remote_bridge child) and upstream tests
that write fixture agent-detection markers into the real herdr-dev
config dir mid-suite -> agent_explain reads garbage override (passes
with pristine XDG dirs / after removing marker residue: 47/47).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Trigger automated AI reviews for pull requests admitted by the PR gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants