Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
I couldn't verify this end to end. It needs the packaged Electron app with the opencode:// handler registered, and that isn't available here. The parser tests pass (4/4) and packages/desktop type-checks cleanly. I also read through the main-to-renderer delivery path and ran the parser on a few extra inputs.
Public API: this adds the opencode://session/<id> URL form and a sessionHref export from @opencode/app/desktop. Both are additions, so the change is backwards compatible.
Concerns, most important first (details inline):
- Session links get replayed. Links that were already delivered stay queued in the main process, so the next window that opens or reloads navigates to an old session.
- The ID check is loose. A decoded
?or invisible characters get through into the route. - The link always opens on the first server, with no check that the session belongs to it.
- The renderer imports the parser from
src/main. The parser should live insrc/shared.
| const restoreWindows = windows.restore | ||
| const pendingDeepLinks: string[] = [] | ||
| const initialDeepLinks = process.argv.filter((arg) => arg.startsWith("opencode://")) | ||
| const pendingDeepLinks = [...initialDeepLinks] |
There was a problem hiding this comment.
emitDeepLinks (below) pushes every URL into pendingDeepLinks, even when it was delivered live to a window over IPC. Each renderer drains this queue on boot through consumeInitialDeepLinks. Before this PR nothing acted on the queued links. With SessionDeepLinks in place, a session link opened while the app is running will navigate the next window you open (or a reload) to that same session again. Please only queue links that no window received, or clear them once they've been delivered.
| } | ||
|
|
||
| if (!id || id.length > 256 || id === "." || id === "..") return | ||
| if (id.includes("/") || id.includes("\\") || sessionControl.test(id)) return |
There was a problem hiding this comment.
This list of rejected characters still lets through IDs that break the route. opencode://session/ses%3F1 decodes to ses?1, and sessionHref inserts it unencoded, so it becomes /session/ses?1. A zero-width space (%E2%80%8B) is also accepted. Matching against the actual session ID format (e.g. /^[A-Za-z0-9_-]{1,256}$/) would replace most of these checks and close those gaps.
| }) | ||
|
|
||
| createEffect(() => { | ||
| const server = servers.visible[0] ?? servers.list[0] |
There was a problem hiding this comment.
The session is always opened on servers.visible[0] ?? servers.list[0]. With extension or remote servers in the list, the ID may belong to a different server, and the user lands on a broken session page. At minimum, please target the local sidecar explicitly. Also, when several IDs are queued, the loop calls navigate for each one, and only the last navigation has any effect.
| import { ServerConnection, sessionHref, useServers } from "@opencode/app/desktop" | ||
| import { useNavigate } from "@solidjs/router" | ||
| import { createEffect, createSignal, onCleanup, onMount } from "solid-js" | ||
| import { sessionDeepLink } from "../../main/lifecycle/deep-link" |
There was a problem hiding this comment.
The renderer imports from src/main/lifecycle. packages/desktop/AGENTS.md keeps renderer and main separate, so please move the shared parser into src/shared/ and import it from both sides.
Queue a cold-start link until a window exists, and do not replay a link that a live window already received. Accept only session ids made of letters, numbers, underscore, and hyphen. Open the local server once.
c26f2d0 to
a433bb4
Compare
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
No existing issue. The earlier attempt, #54153, targeted
devand was closed because that branch only takes critical fixes. This change is againstv2.Type of change
What does this PR do?
The v2 desktop app already accepts
opencode://console/authorized. This addsopencode://session/<sessionID>.A cold start on Windows puts the URL in
argvand does not raise a second-instance event, so those links are queued with the other pending deep links. After a server is available, the window navigates to the existing session route. Extra segments, userinfo, a port, a query, a hash, and control characters are ignored. Console return links are unchanged.How did you verify your code works?
bun test packages/desktop/src/main/lifecycle/deep-link.test.tsThe parser tests cover a normal id, a decoded space, the 256-character limit, and the rejected forms above. Console links still parse as console links and not as session links.
Screenshots / recordings
No new UI. A valid link opens the existing session page.
Checklist
Added by the opencode bot (AI-generated).
Related: #44167, #44160
Public API: adds the opencode://session/ deep link and exports sessionHref from @opencode/app/desktop (backwards compatible)