feat(gui): desktop shell integration — shell detection, external links, open-in-browser, per-OS install card (desktop stack 3/5) - #5287
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…token in shell UA Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
리뷰 · 우선순위 54 / 80이 PR은 데스크톱 앱 스택의 세 번째 장입니다. 베이스는 바깥 사이트 링크는 App에서 클릭을 가로채 최상위 이동으로 바꿉니다. 그러면 이미 있는 Rust 네비게이션 규칙이 막고, 시스템 브라우저로 엽니다. Usage 설치 카드는 macOS / Windows / Linux를 고를 수 있게 바뀌었고, 셸 안에서는 “데스크톱 앱 안에서 보는 중”과 “브라우저에서 열기”만 보입니다. 브라우저에서 열기는 관리 API
메인테이너의 판단이 필요한 지점 스택을 앞 장(#5260 계열)부터 순서대로 합칠지, 이 장만 너의 추천 앞 장이 먼저 들어간 뒤에 이 PR을 합치는 편이 맞습니다. 이 댓글은 grok-bot이 작성했습니다 |
|
Re grok-bot 5748578867:
|
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Follow-up to my note above: on reflection the registry convention is that every POST with an OS-visible side effect is |
추가 리뷰 · 우선순위 24 / 80이전 리뷰(우선순위 54) 이후 tip이
메인테이너의 판단이 필요한 지점 스택(#5256 → #5260 → 이 PR)을 아래부터 순서대로 합칠지. draft 체크리스트를 언제 채우고 Ready로 올릴지. 너의 추천 앞 장(#5260)이 이 댓글은 grok-bot이 작성했습니다 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 033dafad47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof path !== "string" || !path.startsWith("/") || path.startsWith("//") || path.length > 512) { | ||
| return jsonResponse({ error: "invalid path" }, 400, ctx.req, ctx.config); | ||
| } | ||
| const url = `http://127.0.0.1:${ctx.config.port}${path}`; |
There was a problem hiding this comment.
When the proxy is started with the supported ocx start --port <port> override, or with config.port = 0, ctx.config.port is not the port serving this request; startServer keeps the selected/bound port separately. The desktop shell discovers and loads the real runtime port, but this button opens the configured port (often 10100 or 0), so it fails or can open a different proxy. Derive the loopback URL from the authenticated request's actual port or propagate the bound port through the management context.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| const kind = userAgent.startsWith("OpenCodexMenuBar/") ? "menuBar" | ||
| : userAgent.startsWith("OpenCodexDesktop/") ? "desktop" |
There was a problem hiding this comment.
The new Tauri UA produced by desktop/src-tauri/src/window.rs:12 starts with Mozilla/5.0 and appends OpenCodexDesktop/<version>, so startsWith("OpenCodexDesktop/") can never recognize requests from the actual desktop webview. Consequently the presence record is not refreshed as desktop, and a normal browser viewing the same proxy reports that no desktop app is connected. Parse the marker as a token in the full UA and test with webview_user_agent()'s complete format.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| export async function handleCompanionRoutes(ctx: ManagementContext): Promise<Response | null> { | ||
| if (ctx.url.pathname === "/api/companion/open-in-browser" && ctx.req.method === "POST") { |
There was a problem hiding this comment.
The route registry declares this host-side action session-only, but this handler never checks ctx.principal; an authenticated admin-token caller can therefore trigger system-browser launches despite not holding a dashboard session. This is also demonstrated by the added test invoking the route with "admin-token". Reject callers other than gui-session, as the other session-only management routes do.
AGENTS.md reference: src/AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| "usage.companion.osLinux": "Linux", | ||
| "usage.companion.runningInDesktop": "You’re in the OpenCodex desktop app {version}", | ||
| "usage.companion.openInBrowser": "Open in browser", | ||
| "usage.companion.installMacStep1": "Download OpenCodex-<version>-macos.dmg from the latest release and drag OpenCodex.app to Applications.", |
There was a problem hiding this comment.
This replaces the working macOS ZIP instruction with a DMG and also advertises MSI/AppImage packages, but a repo-wide search finds no release automation that publishes any of those names; .github/workflows/ci.yml:1180-1219 only performs Rust checks with placeholder resources, while the existing macOS release script still emits OpenCodex-<version>-macos-<arch>.zip. Users following the newly visible installation card therefore reach the latest release but cannot find the named downloads. Keep the shipped ZIP instructions or hide the new per-OS copy until the packaging workflow lands.
Useful? React with 👍 / 👎.
|
Superseded by #5318, merged to #5196 landed first as The consolidation applies the tree difference of the stack tip against the point the chain branched from. One file genuinely conflicted and both intents were kept. Everything from the standalone binary through the signed packaging is on Closing as superseded rather than stale. |
1 participant
Summary
Desktop stack 3/5 (stacked on #5260). The dashboard learns it is running inside the Tauri shell and adapts — without granting the loopback page any Tauri IPC.
OpenCodexDesktop/<version>(window::webview_user_agent(), percfg!(target_os)). The GUI detects the shell purely fromnavigator.userAgent;structure/desktop-shell.mdrecords the invariant.gui/src/lib/desktop-shell.ts(isDesktopShell,desktopShellVersion,hostOs,isExternalLink) + a capture-phase click handler inApp.tsx. Cross-originhttp(s)anchors are turned into a top-level navigation, which the existing Ruston_navigationpolicy denies and hands to the system browser — sotarget="_blank"links (docs, GitHub) work in WKWebView/WebView2 without a bridge.POST /api/companion/open-in-browser { path }— accepts only a dashboard path (/…, not//…, ≤512 chars) and openshttp://127.0.0.1:<port><path>viaopenUrl(). Inside the shell the Usage install card becomes "You're in the OpenCodex desktop app vX · Open in browser"; Chrome access to the same loopback URL stays first-class.kind: "menuBar" | "desktop"(from the UA prefix), so the card says "Desktop app connected" vs "Menu bar app connected".chmod +x), per-OS command hint. Keys added to all 10 locales; oldinstallStep1..3removed.Not in this PR: packaging artifacts themselves (stack 5), widget bundling (stack 4).
Verification
bun run typecheck,bun run structure:check,git diff --checkbun test tests/server/companion-settings.test.ts(presence kinds; open-in-browser 400s + mockedopenUrlreceives exactlyhttp://127.0.0.1:10100/#/usage)gui:bun test tests(incl. newtests/desktop-shell.test.ts),bun run lint,bun run lint:i18n,bun run builddesktop/src-tauri:cargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo test(UA marker + platform token):10100with a 120k-row seeded usage log; presence set viacurl -A OpenCodexDesktop/2.61.0; shell state reproduced in Chrome with a DevTools UA override.Screenshots
xattr)chmod +x)Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Link to Devin session: https://app.devin.ai/sessions/f2dce68b7ca349e3ba39dab55bc12300
Open in Devin Desktop: https://app.devin.ai/desktop/session/f2dce68b7ca349e3ba39dab55bc12300?variant=devin
Requested by: @lidge-jun