feat(desktop): stable CDP endpoint + browser-use MCP snippet + wider dialog#363
feat(desktop): stable CDP endpoint + browser-use MCP snippet + wider dialog#363
Conversation
…dialog ユーザーが外部ブラウザ MCP を 1 回登録したら Superset を再起動しても 同じ URL が有効なようにする。併せて Connect モーダルの情報量が増えて はみ出していた snippet と狭いダイアログも直す。 ### Stable CDP endpoint (再登録不要に) - browser-mcp-bridge/server.ts: リスン時に `browser-mcp.json` に 書いた前回のポートを最優先、なければ preferred port 47834、最後に kernel-assigned port へフォールバック。次回起動で同じ port を 復元する。 - cdp-filter-proxy.ts: `mintCdpToken` / `resolveTokenToPane` が `~/.superset/browser-mcp-tokens.json` に hydrate/persist する ようになった。同じ sessionId (= terminal pane) を使い続ける限り token も不変。Superset 再起動後も URL が同じ。 ### browser-use を MCP 形式に修正 - CdpEndpointCard の snippet: `claude mcp add browser-use -s user -e BROWSER_USE_CDP_URL=<ws> -- uvx --from \"browser-use[cli]\" browser-use --mcp` - chrome-devtools-mcp の snippet はそのまま (既に正しい形) ### Dialog サイズ + overflow - SessionConnectModal: !max-w 820→1640 (横 2x)、max-h 560→840 + min-h 380→570 (縦 1.5x) - CdpEndpointCard の `<pre>` を `min-w-0 max-w-full break-all` に変えて長い URL snippet がダイアログ幅を超えて漏れないように
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughCDP セッショントークンの永続化ストレージを追加し、ブラウザMCPブリッジサーバーにループバックポート選択メカニズムを実装しました。ディスク上の Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/components/CdpEndpointCard/CdpEndpointCard.tsx (1)
477-486:SetupPanelの未使用 props
session/mcpConfigPath/onCopyが props 型には残っていますが、本文ではserverCommandのみを利用しており他は読まれていません。呼び出し側(Line 249-258)では渡しているので実害はないものの、デッドコードに近い状態です。props 型からも削るか、McpInstallPanelに本当に不要か再確認してから整理すると読みやすくなります。♻️ 提案
-function SetupPanel({ - serverCommand, -}: { - session: AutomationSession; - mcpConfigPath: string | null; - serverCommand?: ServerCommand; - onCopy: () => void; -}) { - return <McpInstallPanel serverCommand={serverCommand} />; -} +function SetupPanel({ serverCommand }: { serverCommand?: ServerCommand }) { + return <McpInstallPanel serverCommand={serverCommand} />; +}呼び出し側の余分な props 指定も併せて削除してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/components/CdpEndpointCard/CdpEndpointCard.tsx` around lines 477 - 486, The SetupPanel component currently declares props session, mcpConfigPath, and onCopy but only uses serverCommand; remove the unused props from the SetupPanel props type and from the component signature, and then remove those unnecessary prop passings at the call site that creates McpInstallPanel/SetupPanel so callers only pass serverCommand; alternatively, if McpInstallPanel truly needs any of these values, re-add usage inside SetupPanel (or forward them) — locate symbols SetupPanel, McpInstallPanel, and serverCommand to update the props interface and all call sites consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/lib/browser-mcp-bridge/cdp-filter-proxy.ts`:
- Around line 2-8: TOKEN_STORE_PATH (browser-mcp-tokens.json) may have weak
permissions even if created with mode; add a best-effort permission repair using
fs.chmodSync when creating/reading the token store. Update the logic around
TOKEN_STORE_PATH handling in cdp-filter-proxy.ts (the code that uses
mkdirSync/writeFileSync/readFileSync for the token store) to call
chmodSync(TOKEN_STORE_PATH, 0o600) after file creation and also after detecting
an existing file before access so file access is always enforced to 0o600;
ensure errors from chmodSync are caught/ignored (best-effort) so it doesn't
crash the process.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/components/CdpEndpointCard/CdpEndpointCard.tsx`:
- Around line 60-65: The browser-use MCP registration currently injects the CDP
endpoint via the non-documented BROWSER_USE_CDP_URL env var (see browserUseCmd
and the BROWSER_USE_CDP_URL usage); update the command to pass the CDP URL using
the documented CLI flag instead (use --cdp-url <url>) so the CDP endpoint is
provided as a flag to the browser-use CLI invocation rather than an env var; if
MCP mode does not accept flags, add a fallback comment and implement the
alternative documented approach (e.g., set cdp_url on the Browser instance) and
ensure chromeDevtoolsCmd and browserUseCmd remain consistent in scoping.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/SessionConnectModal.tsx`:
- Line 165: DialogContent の className で使っている固定幅 "!max-w-[1640px]"
と固定高さが小〜中解像度でモーダルをはみ出させるので、SessionConnectModal 内の DialogContent の className
をビューポート相対値とレスポンシブブレークポイントに置き換えてください(例: max-w を min(1640px,90vw) 相当にし、必要なら lg: や
sm: ブレークポイントで段階的に広げる)。またモーダル内部のグリッドやコンテナ(該当するクラス/コンポーネント)については min-h 固定を避け、max-h
を min(840px,90vh) のように設定して小型ディスプレイで縦が切れないよう調整してください。
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/components/CdpEndpointCard/CdpEndpointCard.tsx`:
- Around line 477-486: The SetupPanel component currently declares props
session, mcpConfigPath, and onCopy but only uses serverCommand; remove the
unused props from the SetupPanel props type and from the component signature,
and then remove those unnecessary prop passings at the call site that creates
McpInstallPanel/SetupPanel so callers only pass serverCommand; alternatively, if
McpInstallPanel truly needs any of these values, re-add usage inside SetupPanel
(or forward them) — locate symbols SetupPanel, McpInstallPanel, and
serverCommand to update the props interface and all call sites consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9038eed-cfb9-437d-b25a-97435c2982bc
📒 Files selected for processing (4)
apps/desktop/src/main/lib/browser-mcp-bridge/cdp-filter-proxy.tsapps/desktop/src/main/lib/browser-mcp-bridge/server.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/SessionConnectModal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/components/CdpEndpointCard/CdpEndpointCard.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d5ab70ced
ℹ️ 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".
- cdp-filter-proxy: chmodSync(TOKEN_STORE_PATH, 0o600) after write so an existing tokens file with broader permissions is always re-tightened. Matches the hardening already done for browser-mcp.json in server.ts. (CodeRabbit Major / codex P2) - CdpEndpointCard: browser-use snippet uses `--cdp-url` as a CLI flag instead of the unverified `BROWSER_USE_CDP_URL` env var. Matches the CLI flag documented for browser-use MCP mode. (CodeRabbit Critical) - SessionConnectModal: dialog width clamps with `min(1640px, 95vw)` and the body with `min(570px,70vh)` / `min(840px,85vh)` so the new 2x/1.5x sizing still fits on 13-14 inch displays without clipping the footer. (CodeRabbit Minor)
目的
外部ブラウザ自動化 MCP (chrome-devtools-mcp / browser-use / playwright-mcp) を Claude / Codex に 1 回登録したら、Superset を再起動しても登録し直し不要 にする。あわせて Connect モーダルの UI フィッシュをまとめて片付け。
主な変更
1. Stable CDP endpoint (再登録不要)
これで
```
claude mcp add chrome-devtools-mcp -s user -- npx -y chrome-devtools-mcp --browser-url http://127.0.0.1:47834/cdp/
```
を 1 回叩けば、それ以降の Superset 再起動後もそのまま使える。
2. browser-use を MCP 形式に修正
旧: `browser-use --cdp-url ` (CLI 起動)
新: `claude mcp add browser-use -s user -e BROWSER_USE_CDP_URL= -- uvx --from "browser-use[cli]" browser-use --mcp`
docs.browser-use.com 通り browser-use 本体の MCP モードを使う。
3. UI
Test plan
Summary by CodeRabbit
リリースノート
バグ修正
スタイル
機能更新