Skip to content

fix(desktop): terminal page links follow the URL link rules - #7946

Merged
saddlepaddle merged 1 commit into
mainfrom
saddlepaddle/terminal-page-links
Sep 29, 2026
Merged

saddlepaddle merged 1 commit into
mainfrom
saddlepaddle/terminal-page-links

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Page links in the terminal opened on a plain click, even when Settings → Links → URL links says a plain click does nothing. Since #7471, a page link skipped the URL modifier rules entirely and used the single Pages action (default: split pane) for every click.

Change

  • Terminal page links now go through the URL link rules like any other URL — plain / ⇧ / ⌘ / ⌘⇧ click each do what the URL rules say. When a rule opens the link in-app (split pane / new tab), runUrlLinkAction still opens the page pane, not a web view. The hover label follows the same path.
  • The Pages setting is now read only by the workspace Pages menu, so its description (and settings-search text) says that. The old text also mentioned chat and task markdown, which never read it. Translations updated for all 16 locales.

Trade-off

You can no longer send ordinary URLs to the browser on ⌘-click while keeping page links in-app on the same modifier — the case #7471 was built for. Pick different modifiers for each instead (e.g. ⌘ → browser, ⌘⇧ → split pane).

Validation

  • bun run lint, desktop typecheck, bun run check:i18n, desktop renderer tests (6,286 pass) — green locally.
  • Not yet clicked through in a running app.

Summary by cubic

Terminal page links now follow the URL link rules instead of always using the Pages action on any click.

Bug Fixes

  • Plain, ⇧, ⌘, and ⌘⇧ clicks each do what the URL rules say; when an in-app action applies, the page pane opens rather than a web view, and the hover label follows the same path.
  • The Pages setting now only applies to the workspace's Pages menu, so its description and translations were updated.
  • Trade-off: you can no longer use ⌘-click to send URLs to the browser while keeping page links in-app on the same modifier — pick different modifiers instead (e.g., ⌘ → browser, ⌘⇧ → split pane).

Written for commit 09eabba. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Behavior
    • Terminal URL clicks and hover labels now follow the same URL-handling policy, including for Superset pages.
  • Settings
    • Clarified that the Pages setting controls where pages opened from a workspace’s Pages menu appear.
  • Localization
    • Updated the setting description across supported languages.

A page link in the terminal opened on a plain click whatever the URL link
rules said, because the Pages setting (#7471) replaced the modifier tiers
with one action. Page links now go through the URL rules like any other
link; an in-app action still opens the page pane rather than a web view.

The Pages setting now only covers the workspace Pages menu, which is all
that reads it, so its description says so.
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7e152566-03fa-4df5-a63c-37696806dd99

📥 Commits

Reviewing files that changed from the base of the PR and between f5c89b2 and 09eabba.

📒 Files selected for processing (20)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/browser/components/BrowserSettings/components/BrowserLinkSettings/BrowserLinkSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts
  • packages/i18n/locales/cs/messages.po
  • packages/i18n/locales/de/messages.po
  • packages/i18n/locales/en/messages.po
  • packages/i18n/locales/es/messages.po
  • packages/i18n/locales/fr/messages.po
  • packages/i18n/locales/id/messages.po
  • packages/i18n/locales/it/messages.po
  • packages/i18n/locales/ja/messages.po
  • packages/i18n/locales/ko/messages.po
  • packages/i18n/locales/nl/messages.po
  • packages/i18n/locales/pl/messages.po
  • packages/i18n/locales/pt-BR/messages.po
  • packages/i18n/locales/ru/messages.po
  • packages/i18n/locales/tr/messages.po
  • packages/i18n/locales/vi/messages.po
  • packages/i18n/locales/zh-CN/messages.po
  • packages/i18n/locales/zh-TW/messages.po

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

Terminal URL clicks and hover labels now use the URL policy for all URLs. The Pages setting description now refers to pages opened from a workspace’s Pages menu in the settings UI, search text, and supported locale catalogs.

Changes

Pages navigation

Layer / File(s) Summary
Terminal URL click and hover policy
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx
Terminal URL clicks and hover labels now use urlPolicy instead of special Superset-page handling and pageOpenAction.
Pages setting description
apps/desktop/src/renderer/routes/_authenticated/settings/browser/components/BrowserSettings/components/BrowserLinkSettings/BrowserLinkSettings.tsx, apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts, packages/i18n/locales/*/messages.po
The Pages setting description now refers to pages opened from a workspace’s Pages menu in the settings UI, search text, and locale catalogs.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: kitenite

Merge Risk: ⚪ Minimal · up to 09eab

Terminal URL clicks and hover labels follow the configured actions, including in-app Page navigation. The Pages setting copy is updated consistently, and no material merge risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 09eab

The change restores configured click rules and preserves page-pane navigation for in-app actions. Browser opening remains a configured action, and no specific new security flaw was established. End-to-end browser handoff and failure behavior have not been validated in a running app.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is a clicked terminal URL in the user's desktop session: it may navigate the current workspace or be handed to the system browser. Recognized page URLs newly inherit external URL rules; the inspected change does not establish additional service, tenant, or datastore authority.

Trust Boundaries and Controls

  • observed — The external-opening procedure checks isSafeExternalUrl before passing the unchanged URL to shell.openExternal. Page recognition checks the configured web origin and page path but does not reject query, fragment, or userinfo components. This establishes raw-URL handoff, not verified credential disclosure; actual page-link sensitivity remains unresolved.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (17 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commit format and clearly summarizes the main change: terminal page links now follow URL link rules.
Description check ✅ Passed The description explains the problem, change, trade-off, and validation results. It also states that running-app verification is still pending. The required content is mostly present, although the che…
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (17 skipped: 17 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@expo

expo Bot commented Sep 29, 2026

Copy link
Copy Markdown

No installed development build can load this pull request as an update: its native fingerprint is 7084d376924a and no development build has it. Add the mobile-build label to build one.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

@harshithmullapudi

Copy link
Copy Markdown
Collaborator

hurray cool

@saddlepaddle
saddlepaddle merged commit 2d1bff4 into main Sep 29, 2026
22 checks passed
Kitenite added a commit that referenced this pull request Oct 1, 2026
* docs: weekly update for PRs merged 2026-09-23 to 2026-09-30

Source of truth: live Notion SOP "Weekly Docs Update (automation SOP)".
Window: merged:>=2026-09-23 (139 PRs). PRs through the morning of 09-23
were covered by #7771.

Changes
- Pages: no auto-open on publish (#7790); page links and URL links split
  into their own settings (#7916, #7962, #7946); Listening agents menu with
  per-row stop and Add listening agent (#7724, #7820, #7919); globe on the
  Share button (#7766); Report on public pages (#7886); agents offer a page
  (#7759). Files: pages.mdx, cli/cli-reference.mdx.
- Settings moves: Links replaced by Browser and Files & Editor (#7916,
  #7962). Files: editor.mdx, terminal-integration.mdx, customization.mdx.
- Editor: inline rename and drag to move (#7692), Auto Save (#7693, #7916),
  open and save files outside the workspace (#7897, #7982), Copy as Markdown
  (#7591), per-pane error screen (#7983). File: editor.mdx.
- Terminal copy follows Ghostty's whitespace rules (#7868).
  File: terminal-integration.mdx.
- Agents: UFO (#7898); GPT-6 Astra, Sonnet 5.5, retiring GPT-5.5, retired
  GPT-5.4/5.3 Codex (#7783, #7894, #7935). Files: agent-integration.mdx,
  cli/cli-reference.mdx.
- Host: superset start --daemon --auto-update and update-lock permission
  errors (#7735, #7952). Files: cli/host-server.mdx, cli/cli-reference.mdx.
- Removed the mcp CLI section and the Connected Service Plugins section:
  #7776 made plugins/mcp internal, the Plugins page is staff-flagged, and
  apps/docs/AGENTS.md keeps internal commands out. Files: skills.mdx,
  cli/cli-reference.mdx.
- Slack page unfurls and details panel (#7817, #7821, #7914, #7957).
  File: use-with-slack.mdx.
- Workspaces: instant creation with background naming (#7518); recoverable
  project deletion (#7896). File: workspaces.mdx.
- Shortcuts: prev/next workspace includes Pinned and wraps (#7851); ⌘B
  collapses the sidebar to a rail (#7964). File: keyboard-shortcuts.mdx.
- Ship controls and open-in button moved to the right sidebar strip (#7958).
  Files: diff-viewer.mdx, use-with-ide.mdx.

Deliberately skipped
- Cloud workspaces/environments/sandboxes (flag-gated, AGENTS.md rule):
  #7784 #7795 #7824 #7840 #7844 #7845 #7847 #7849 #7850 #7858 #7861 #7863
  #7874 #7911 #7913 #7918 #7922 #7931 #7932 #7937 #7956 #7961 #7969 #7981
  #7637 #7794 #7789 #7838 (its CLI docs already shipped in the PR).
- Staff-only plugins: #7912 Sentry plugin, #7716, #7727, #7955.
- Already documented in their own PRs: #7917, #7921.
- Removed feature never documented: #7796 Google Calendar triggers.
- Agents chip remove/revert/default-off, never documented: #7870 #7871 #7872.
- Billing/backend policy with no docs surface: #7947, #7812, #7786, #7792,
  #7798, #7761, #7948, #7949, #7951, #7827.
- Reverted security trio: #7828 #7829 #7830 (reverted by #7834).
- Fixes restoring expected behavior: #7683 #7696 #7707 #7743 #7751 #7756
  #7764 #7765 #7781 #7788 #7791 #7823 #7825 #7833 #7837 #7852 #7857 #7860
  #7864 #7865 #7869 #7873 #7897-covered #7903 #7938 #7980 #7943 #7964-chrome.
- Mobile (no docs page): #7746 #7747 #7748 #7749 #7750 #7782 #7793 #7810
  #7814 #7816 #7889 #7968.
- Marketing, internal docs, CI, version bumps: #7651 #7744 #7752 #7771 #7774
  #7776-infra #7777 #7785 #7811 #7818 #7822 #7826 #7834 #7884 #7885 #7895
  #7909 #7933 #7934 #7942 #7944 #7965 #7966.

Validation: bun run --cwd apps/docs typecheck, bun run lint:fix,
bun run lint (exit 0), Vale on 13 changed files (0 errors, 0 warnings),
git diff --check.

* chore: log weekly docs update follow-ups

* docs: tighten review findings in the weekly update

Only the page creator can stop or add listening agents. On focus change also
saves on window blur, and Auto Save pauses on conflicts. Taken-down pages do
not unfurl. Model picker coverage matches agent-models.ts.

This branch was previously deployed

1 inactive deployment
preview — 09eabba9 Deployed Sep 29, 2026 by saddlepaddle via Deploy Admin #10564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants