mcp: open shared dashboard on demand, drop the dead redirect#1333
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Blast radius
1 added, 0 removed pie showData title Rebuilt checks by category
"rust" : 42
"image" : 15
"site" : 2
"agent" : 1
"blast" : 1
"eval" : 1
"lint" : 1
flowchart LR
c0["ix-notebook-mcp-module"]
c1["blast-radius-test"]
c2["agent-skills"]
c3["lint"]
c4["rust-mcp.viewSmoke"]
c5["site-test"]
c0 --> k0["agent-skills"]
c0 --> k2["eval"]
c0 --> k3["image-development-base"]
c0 --> k4["image-kernel-dev"]
c0 --> k5["image-minecraft"]
changed checks (62)
|
9febb93 to
1803a2b
Compare
There was a problem hiding this comment.
AI review found issues in this pull request.
Verdict: patch is incorrect
Confidence: 0.86
The shared dashboard launcher has correctness gaps in its process lifecycle and liveness detection: it can leak detached dashboards on failed readiness, can treat an unrelated reused port as the singleton hub, and fails for hostname bind values accepted elsewhere by the CLI.
- P1
packages/mcp/ix_notebook_mcp/cli.py:657Timeout path leaks the detached dashboard process - P2
packages/mcp/ix_notebook_mcp/config.py:172Stale state can be mistaken for a live dashboard after port reuse - P2
packages/mcp/ix_notebook_mcp/cli.py:633Hostname IX_MCP_HOST values cannot start the Rust dashboard
There was a problem hiding this comment.
AI review found issues in this pull request.
Verdict: patch is incorrect
Confidence: 0.88
The shared-dashboard launcher mostly follows the intended shape, but the liveness check can reuse stale state for an unrelated listener, and the new root handler breaks the documented auto-dashboard compatibility path.
- P1
packages/mcp/ix_notebook_mcp/config.py:172Stale hub state can be mistaken for a live dashboard - P2
packages/mcp/ix_notebook_mcp/dashboard.py:73Auto-dashboard mode no longer redirects to its spawned hub
1803a2b to
d87ea7b
Compare
da67a9c to
6091503
Compare
There was a problem hiding this comment.
AI review found issues in this pull request.
Verdict: patch is incorrect
Confidence: 0.88
The shared dashboard launcher has binding/advertisement regressions: it can expose the global dashboard on wildcard interfaces and can publish an unreachable URL after falling back to loopback.
- P1
packages/mcp/ix_notebook_mcp/cli.py:646Shared dashboard can bind to every interface - P2
packages/mcp/ix_notebook_mcp/cli.py:661Fallback bind still advertises the unusable requested host
72e39f2 to
5cd18d7
Compare
There was a problem hiding this comment.
AI review found issues in this pull request.
Verdict: patch is incorrect
Confidence: 0.78
The shared dashboard launcher mostly follows the intended singleton behavior, but it regresses configured IPv6 hosts by passing them through in a form the dashboard binary cannot parse.
- P2
packages/mcp/ix_notebook_mcp/cli.py:625IPv6 hosts are passed to the dashboard binary in an unparsable form
The per-session data API root unconditionally 302'd to config.hub_url(), a free port reserved at startup but never bound when no per-server hub is auto-spawned (the default). Opening DASHBOARD_URL therefore redirected straight into a refused connection. Now / redirects to the shared hub only when one is actually running (recorded in runtime_dir()/hub.json, port-probed via config.live_hub off the event loop), and otherwise serves a short page naming the command. Add a singleton launcher: the dashboard subcommand reuses a running hub or spawns one detached on a stable port (8080, IX_DASH_HUB_PORT) bound to the tailnet IP or IX_MCP_HOST, else loopback (never 0.0.0.0), then opens it, so repeated runs never pile up dashboards.
5cd18d7 to
eac5f75
Compare
Problem
Opening the kernel's
DASHBOARD_URLgaveERR_CONNECTION_REFUSED.The per-session data API's
/handler unconditionally302'd toconfig.hub_url()-- a free port reserved at startup (_free_port()) but never bound, because the human dashboard hub is not auto-spawned per session by default (that was the deliberate fix for "a new dashboard URL per MCP connection"). So the courtesy redirect always pointed at a dead port.There was also no easy way to just open the one shared dashboard.
Change (Python only -- no Rust rebuild)
dashboard.py):/redirects to the shared hub only when one is actually running (recorded inruntime_dir()/hub.json, confirmed live via a TCP probe inconfig.live_hub()), otherwise serves a small page namingix-mcp dashboard. The probe runs viaasyncio.to_threadso it never blocks the shared event loop.cli.pyix-mcp dashboard): reuse a running hub or spawn one detached, then print the URL and open the browser. Repeated runs reuse the same hub -- no pile-up.IX_MCP_HOST/ the tailnet IP / loopback (honoring an operator's loopback pin), never0.0.0.0. Stable port8080, overrideIX_DASH_HUB_PORT.config.py:hub_state_path(),port_open(),live_hub().Validation
nix build .#mcpand.#mcp.tests.dashboardLauncherSmoke(new) green;strictTypecheck+ ruff clean.live_hub(missing / stale-dead-port / live), the landing page, and the real aiohttp/handler (302 to live hub, 200 landing otherwise).ix-mcp dashboardspawns once, reuses on repeat (single process), hub serves 200; bind honorsIX_MCP_HOST.Notes
IX_MCP_HOSTis not pinned to loopback (the hub then binds the tailnet IP), or setIX_MCP_HOST/IX_DASH_HUB_PORT.ix_notebook_mcp servesockets/processes from old sessions.Reviewed by the code-reviewer agent; its correctness blocker (blocking socket call on the event loop) and security finding (wildcard bind) are both fixed above.
Authored by Claude (Sonnet) via Claude Code.
Note
Open a shared dashboard hub on demand from
ix-mcp dashboard, dropping dead redirectsix-mcp dashboardnow spawns or reuses a single shared hub process, persisting state inhub.json(pid, host, port, url) and using anfcntlfile lock to serialize concurrent launches.IX_DASH_HUB_PORT), and IPv6 bracketing for host:port strings.live_hub()in config.py validates a recorded hub by checking pid liveness and TCP reachability before trusting it./index handler in dashboard.py now only redirects to a verified live hub; otherwise it serves a static landing page pointing users toix-mcp dashboard.--no-openflag suppresses browser launch; log messages and docs updated to referenceix-mcp dashboardinstead ofnix run .#dashboard.Macroscope summarized eac5f75.