Clickable orca:// deep-links to focus a terminal tab#4386
Conversation
Register the orca:// URL scheme and route orca://focus/<handle> to the existing terminal-focus action (runtime.focusTerminal), reused by both in-terminal OSC 8 link clicks and OS-level open-url/argv deep-links. No notifications. - shared/orca-deep-link.ts: pure parse + argv extraction (+ tests) - main/startup/orca-deep-link-router.ts: single router for every entry point — open-url, second-instance/argv, and renderer-forwarded OSC 8 clicks; defers cold-launch links until the window is ready (+ tests) - main/index.ts: register scheme, open-url, second-instance, initial argv, ipc bridge, flush on window ready - renderer terminal-osc-link-routing.ts: orca: branch forwards to main via window.api.ui.openOrcaDeepLink (+ test) - preload: openOrcaDeepLink bridge + type + web stub - electron-builder: protocols entry -> CFBundleURLTypes - docs/orca-deep-links.md: link format + OSC 8 emission requirement Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BorjaLL
force-pushed
the
BorjaLL/orca-links
branch
from
56036a5 to
a07be53
Compare
Current main added registerHttpLinkStoreAccessor to http-link-routing.ts, which store/index.ts calls at module import. The osc link routing test mocked the whole module without this export, so the import chain threw at load. Add the missing export to the vi.mock factory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds end-to-end 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 36786141-eb06-496b-b755-0fd836500991
📒 Files selected for processing (11)
config/electron-builder.config.cjssrc/main/index.tssrc/main/startup/orca-deep-link-router.test.tssrc/main/startup/orca-deep-link-router.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/terminal-pane/terminal-osc-link-routing.test.tssrc/renderer/src/components/terminal-pane/terminal-osc-link-routing.tssrc/renderer/src/web/web-preload-api.tssrc/shared/orca-deep-link.test.tssrc/shared/orca-deep-link.ts
| const link = parseOrcaDeepLink(url) | ||
| if (!link) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the parseOrcaDeepLink implementation
fd -t f "orca-deep-link" --extension ts --extension tsx | head -20Repository: stablyai/orca
Length of output: 209
🏁 Script executed:
# First, let's find and examine the parseOrcaDeepLink implementation
fd -t f "orca-deep-link" --extension ts --extension tsx | head -20Repository: stablyai/orca
Length of output: 209
🏁 Script executed:
# Read the shared orca-deep-link module
cat -n src/shared/orca-deep-link.ts 2>/dev/null || echo "File not found, searching..."Repository: stablyai/orca
Length of output: 2598
🏁 Script executed:
# Find all orca-deep-link files
find . -name "*orca-deep-link*" -type f 2>/dev/nullRepository: stablyai/orca
Length of output: 217
🏁 Script executed:
# Search for parseOrcaDeepLink implementation
rg "parseOrcaDeepLink" --type ts --type tsx -A 15 | head -80Repository: stablyai/orca
Length of output: 85
🏁 Script executed:
# Examine the router file completely
cat -n src/main/startup/orca-deep-link-router.ts 2>/dev/null | head -100Repository: stablyai/orca
Length of output: 3574
Contain parser exceptions so malformed deep-links don't crash routing.
At Line 54, parseOrcaDeepLink(url) can throw an unhandled exception. While the parser's URL constructor is wrapped in try-catch, the decodeURIComponent call on line 35 of src/shared/orca-deep-link.ts is not. Malformed percent-encoding (e.g., orca://focus/%) triggers a URIError that escapes to the caller, breaking the router instead of treating the link as invalid. The existing deps.onError callback should handle this case.
🔧 Proposed fix
const handle = (url: string): void => {
- const link = parseOrcaDeepLink(url)
+ let link: ReturnType<typeof parseOrcaDeepLink> | null = null
+ try {
+ link = parseOrcaDeepLink(url)
+ } catch (error) {
+ deps.onError?.(error, url)
+ return
+ }
if (!link) {
return
}| deps.focusWindow() | ||
| void Promise.resolve(deps.focusTerminalByHandle(link.handle)).catch((error) => { | ||
| deps.onError?.(error, url) |
There was a problem hiding this comment.
Prevent focus-steal by focusing the window only after terminal resolution succeeds.
At Lines 58-60, the window is focused before focusTerminalByHandle succeeds. A syntactically valid but non-existent handle still brings Orca to foreground, which conflicts with the “ignore unknown/malicious links” intent.
🔧 Proposed fix
- deps.focusWindow()
- void Promise.resolve(deps.focusTerminalByHandle(link.handle)).catch((error) => {
- deps.onError?.(error, url)
- })
+ void Promise.resolve()
+ .then(() => deps.focusTerminalByHandle(link.handle))
+ .then(() => {
+ deps.focusWindow()
+ })
+ .catch((error) => {
+ deps.onError?.(error, url)
+ })| onSplitTerminal: () => noopUnsubscribe, | ||
| onRenameTerminal: () => noopUnsubscribe, | ||
| onFocusTerminal: () => noopUnsubscribe, | ||
| openOrcaDeepLink: () => {}, |
There was a problem hiding this comment.
Document why openOrcaDeepLink is a no-op in web fallback.
Line 2004 adds a non-obvious no-op behavior without rationale. Add a short why-comment so this doesn’t get “fixed” accidentally later.
Suggested patch
- openOrcaDeepLink: () => {},
+ // Why: web clients do not have an Electron main-process deep-link router;
+ // keep this as a no-op to preserve preload API parity.
+ openOrcaDeepLink: () => {},As per coding guidelines, "When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does. Keep comments short — one or two lines, capturing only the non-obvious reason".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| openOrcaDeepLink: () => {}, | |
| // Why: web clients do not have an Electron main-process deep-link router; | |
| // keep this as a no-op to preserve preload API parity. | |
| openOrcaDeepLink: () => {}, |
Source: Coding guidelines
| const handle = decodeURIComponent(parsed.pathname.replace(/^\/+/, '')) | ||
| if (!TERMINAL_HANDLE_PATTERN.test(handle)) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In JavaScript, does decodeURIComponent throw a URIError for malformed percent-encoded input (e.g. "%E0%A4%A"), and is try/catch the standard way to handle untrusted URL path decoding?
💡 Result:
Yes, decodeURIComponent throws a URIError when it encounters malformed percent-encoded input, such as an incomplete sequence (e.g., "%E0%A4%A" or a lone "%") [1][2][3]. Using try/catch is considered the standard and necessary approach for handling untrusted or potentially malformed URI input when using legacy global functions like decodeURIComponent or decodeURI [4][3]. Because these functions throw a synchronous URIError that can crash execution if unhandled, wrapping them in a try/catch block is essential to safely manage invalid sequences [5][6][3]. However, for modern JavaScript development, it is often recommended to use the URL and URLSearchParams APIs whenever possible [4][3]. These APIs provide more structured, robust, and safer ways to handle URL parsing and decoding automatically, which reduces the need for manual try/catch blocks and mitigates common pitfalls associated with manual percent-decoding [4][3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Malformed_URI
- 3: https://urlencoder.tools/en/blog/javascript-decode-url
- 4: https://urlencoder.tools/en/blog/fix-uri-malformed-error
- 5: https://stackoverflow.com/questions/28063750/decodeuricomponent-throwing-an-error-uri-malformed
- 6: https://stackoverflow.com/questions/15980396/how-to-catch-error-when-using-decodeuri
🏁 Script executed:
cat -n src/shared/orca-deep-link.ts | head -50Repository: stablyai/orca
Length of output: 2400
🏁 Script executed:
cat -n src/shared/orca-deep-link.ts | tail -20Repository: stablyai/orca
Length of output: 722
🏁 Script executed:
find . -name '*orca-deep-link-router*' -type fRepository: stablyai/orca
Length of output: 150
🏁 Script executed:
cat -n src/main/startup/orca-deep-link-router.ts | head -60Repository: stablyai/orca
Length of output: 2917
🏁 Script executed:
cat -n src/main/startup/orca-deep-link-router.ts | sed -n '53,75p'Repository: stablyai/orca
Length of output: 795
Guard decodeURIComponent to prevent unhandled URIError on malformed percent-encoding.
At line 35, decodeURIComponent throws a URIError for malformed percent-encoded sequences (e.g., %E0%A4%A), breaking the intended contract where invalid links are silently ignored. The URL constructor is already guarded (line 24), but the decode call is not. Wrap it in a try/catch block and return null on decode failure.
Proposed patch
- const handle = decodeURIComponent(parsed.pathname.replace(/^\/+/, ''))
+ let handle: string
+ try {
+ handle = decodeURIComponent(parsed.pathname.replace(/^\/+/, ''))
+ } catch {
+ return null
+ }
if (!TERMINAL_HANDLE_PATTERN.test(handle)) {
return null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handle = decodeURIComponent(parsed.pathname.replace(/^\/+/, '')) | |
| if (!TERMINAL_HANDLE_PATTERN.test(handle)) { | |
| let handle: string | |
| try { | |
| handle = decodeURIComponent(parsed.pathname.replace(/^\/+/, '')) | |
| } catch { | |
| return null | |
| } | |
| if (!TERMINAL_HANDLE_PATTERN.test(handle)) { | |
| return null | |
| } |
|
Rebased onto latest main and moved out of draft. Conflicts resolved, and validated locally on a clean install: |
|
This is neat, thanks for putting it together. I think I understand the routing piece now: once an One thing I’m still trying to connect is the adoption path for agents. Since the motivating use case depends on agents emitting OSC 8 hyperlinks, how are you imagining agents will learn to produce those links in practice? Would that be through prompt instructions, a small CLI helper, orchestration output, or something else? |
|
@Jinwoo-H good question, and it's deliberately the part this PR doesn't hardcode. The PR only builds the routing (scheme registration + the focus action) and stays agnostic about who emits the link, because emission itself is already trivial: any How I imagine adoption, smallest-first:
I'd start with (1) since it needs no agent cooperation at all, and add the docs from (2). |
# Conflicts: # src/main/index.ts
|
@Jinwoo-H when you get a chance: this one (#4386) is green and mergeable again after a rebase on latest main, and it now closes #4384, which picked up an external +1 today ("very much needed"). Your earlier question about the agent adoption path is answered in the thread above. Two other PRs you're the requested reviewer on are also green and ready whenever it suits: #4475 (keep the agent-hook payload off the curl command line) and #3337 (close the hidden background tab/PTY when an automation completes, your earlier UX question is addressed there too). No pressure on any of them, and I'm happy to trim scope or make changes. If any aren't on the roadmap, just say and I'll close them out. |
2 participants
Closes #4384.
What
Adds clickable
orca://focus/<terminal-handle>deep-links that focus a specific Orca terminal tab — clickable inside the embedded terminal and from anywhere on the OS. No notifications.<terminal-handle>is the runtime-issued handle fromorca terminal list --json(e.g.term_<uuid>).How
Everything reuses the existing canonical focus action — no new focus logic:
runtime.focusTerminal(handle)→ui:focusTerminal→activateTabAndFocusPane(the same path asorca terminal focus/switchand the notification-click handler).A single router (
src/main/startup/orca-deep-link-router.ts) serves every entry point:linkHandler(alreadyallowNonHttpProtocols: true) →handleOscLink→window.api.ui.openOrcaDeepLink→ routerapp.on('open-url')→ routersecond-instanceargv → routerprocess.argv, deferred until the window is readyParsing is a pure shared module (
src/shared/orca-deep-link.ts) so main and renderer classify links identically. Unknownorca://URLs (e.g. the web-onlyorca://pairflow) are ignored; the window is only focused for a valid action, so a hostile terminal can't steal focus with garbage strings.Scheme registration:
setAsDefaultProtocolClient(with the dev-exe variant) + an electron-builderprotocolsentry that injectsCFBundleURLTypes(macOS) / the Windows scheme.Feasibility note — the "chat surface" is the terminal
In Orca, Claude Code (and every agent) runs inside the xterm terminal — there's no separate React-rendered markdown chat surface — so the delivery vehicle is OSC 8 hyperlinks, which is exactly how a markdown link like
[label](orca://focus/term_…)renders into a terminal. Plain-textorca://…will not auto-linkify (WebLinksAddononly autodetects http/https); the link must be emitted as an OSC 8 hyperlink. Documented indocs/orca-deep-links.md.Scope
orca://worktree/<id>deferred — worktree ids arerepoId::worktreePathand need encoding; focus-by-handle fully delivers the goal.Tests
src/shared/orca-deep-link.test.ts— parse + argv extractionsrc/main/startup/orca-deep-link-router.test.ts— routing, defer/replay, error path, ignore-unknownsrc/renderer/.../terminal-osc-link-routing.test.ts— orca: branch forwards; non-modifier click ignored; http still routes to browserpnpm typecheck(node/web/cli),oxlint, and the new tests all pass locally.🤖 Generated with Claude Code