Skip to content

fix(mcp): execute tool dies on Deno >= 2 (unix socket needs net permission)#100

Merged
Dithilli merged 1 commit into
mainfrom
david/mcp-execute-deno2-unix-socket
Jul 17, 2026
Merged

fix(mcp): execute tool dies on Deno >= 2 (unix socket needs net permission)#100
Dithilli merged 1 commit into
mainfrom
david/mcp-execute-deno2-unix-socket

Conversation

@Dithilli

Copy link
Copy Markdown
Contributor

Problem

Every execute call on @hyperspell/hyperspell-mcp@0.40.0 fails with "Deno exited before being ready" on any modern Deno (found during the 2026-07-16 MCP/CLI test battery). The failure is silent — health checks and tools/list still report the server as connected.

Root cause: Deno 2.x moved unix sockets under the net permission. Listening on the socket @valtown/deno-http-worker uses to talk to the sandbox now requires --allow-net=unix:<path>, but the library (0.0.21 — and still 2.0.3, so a version bump does not fix this; verified empirically) only grants the socket via --allow-read/--allow-write, the Deno 1.x model:

NotCapable: Requires net access to "unix:/var/folders/.../<uuid>-deno-http.sock",
run again with the --allow-net flag

Fix

The socket path is generated inside newDenoHTTPWorker, so no flag can be scoped up front — Deno accepts only exact socket paths (no globs or directory prefixes) and rejects repeated --allow-net flags. Instead, use the library's spawnFunc hook: at spawn time, pluck the socket path out of the --allow-write flag the library injects and graft unix:<sock> onto the existing --allow-net scope (comma-merged into the one flag).

Gated on Deno major ≥ 2: Deno 1.46 panics on a unix: token in --allow-net (tested), and on 1.x the old read/write grant still works.

Verification (all against Deno 2.9.3, driven over real MCP stdio JSON-RPC)

Scenario Result
Published 0.40.0, tools/call execute (return 6 * 7) Deno exited before being ready
This build, same call ✅ returns 42
This build, fetch('https://example.com') from executed code ✅ still denied (NotCapable: Requires net access to "example.com:443") — sandbox scoping unchanged
@valtown/deno-http-worker@2.0.3 (the "just bump it" theory) ❌ fails identically — bump is not a fix
Deno 1.46.3 with a unix: token in --allow-net 💥 panics — hence the version gate

pnpm lint and pnpm test pass; file is prettier-clean.

🤖 Generated with Claude Code

…ssion)

Deno 2.x moved unix sockets under the net permission: listening on the
socket that deno-http-worker uses to talk to the sandbox now requires
--allow-net=unix:<path>. The library (0.0.21, and still 2.0.3) only
grants the socket via --allow-read/--allow-write — the Deno 1.x model —
so on any modern Deno every execute call fails with 'Deno exited before
being ready' (NotCapable: Requires net access to unix:...). Health
checks and tools/list still pass, so the breakage is silent.

The socket path is generated inside newDenoHTTPWorker, so we can't
scope a flag up front (Deno only accepts exact socket paths, no
globs/prefixes, and rejects repeated --allow-net flags). Instead, use
the library's spawnFunc hook to pluck the socket path out of the
--allow-write flag it injects and graft unix:<sock> onto the existing
--allow-net scope at spawn time.

Gated on Deno major >= 2 because Deno 1.46 panics outright on a unix:
token in --allow-net.

Verified end-to-end over MCP stdio against Deno 2.9.3: published 0.40.0
fails, this build returns results; fetch to a non-allowlisted host is
still denied (sandbox scoping unchanged).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@firetiger-agent

firetiger-agent Bot commented Jul 17, 2026

Copy link
Copy Markdown

Firetiger has created a monitoring plan for this PR.

View monitor

@entelligence-ai-pr-reviews

Copy link
Copy Markdown

Confidence Score: 5/5 - Safe to Merge

Safe to merge — this PR correctly addresses a runtime permission failure on Deno >= 2 where unix socket connections require an explicit net permission grant. The fix is appropriately scoped, targeting the MCP tool execution path where the unix socket connection was being denied, and no new issues were introduced by the change. The PR is a targeted bugfix with clear intent and no side effects identified during review.

Key Findings:

  • The fix correctly adds the missing net permission for unix socket usage in Deno >= 2, which is a legitimate breaking change in the Deno runtime that requires explicit opt-in permissions for network/socket operations.
  • No logic regressions, null-safety issues, or security concerns were identified — the change is minimal and directly addresses the stated failure mode.
  • The PR scope is well-contained to the permission declaration needed for unix socket connectivity, without touching unrelated codepaths or introducing new abstractions.

@Dithilli

Copy link
Copy Markdown
Contributor Author

Superseded by #101. This fix works on Deno ≥ 2.9 but the >= 2 version gate provably breaks Deno 2.0–2.8: those versions reject the unix: allow-net token at startup (invalid port in 'unix:…') — verified end-to-end. The enforcement actually starts at Deno 2.9.0 (denoland/deno#34395 / #35835 hardening series), so the gate must be ≥ 2.9. Do not merge.

@Dithilli
Dithilli marked this pull request as ready for review July 17, 2026 00:53
@Dithilli
Dithilli merged commit 9604f1b into main Jul 17, 2026
8 checks passed
@firetiger-agent

Copy link
Copy Markdown

Firetiger is already monitoring this PR at change monitor. I'll watch for deployments and start monitoring when this PR merges.

@aburkard aburkard mentioned this pull request Jul 17, 2026
@icubert

icubert commented Jul 17, 2026

Copy link
Copy Markdown

Review — Cubert (interactive path, node-sdk)

Verdict: LGTM. Root cause and fix both check out. Verified independently:

  • Mechanism is real, not a no-op. spawnFunc is a genuine documented option on newDenoHTTPWorker — confirmed in the published @valtown/deno-http-worker@0.0.21 (DenoWorkerOptions.spawnFunc, default child_process.spawn, and there's an "alternate spawnFunc can be provided" test). So the conditional spread actually takes effect.
  • Socket extraction is correct against the lib's real behavior. The library appends ,${socketFile} onto an existing --allow-write= flag (or pushes --allow-write=${socketFile}), where socketFile = ${crypto.randomUUID()}-deno-http.sock. Plucking the --allow-write= segment ending in -deno-http.sock matches exactly.
  • --allow-net graft is safe. runFlags always contains --allow-net=${baseURLHostname}, so the .map always finds a flag to comma-merge unix:<sock> onto — matching Deno ≥2's "one --allow-net, no repeats" constraint. Sandbox scope is unchanged (net still limited to the API host + the socket); the fetch('https://example.com')-still-denied check confirms no scope creep.
  • Version gate is right. ...(denoMajor >= 2 && {…}) spreads nothing on Deno 1.x (falsy spread = no-op), so 1.x keeps the read/write grant and never gets the unix: token that panics it. Fail-safe on version-detect error (assumes 2.x).
  • No-socket-found path falls back to unmodified args = pre-existing behavior, no regression.

Checks: Entelligence 5/5 (Safe to Merge), build/lint/test green.

One durability note (not a blocker): code-tool.ts carries the Stainless // File generated from our OpenAPI spec banner. Per this repo's CONTRIBUTING.md, manual patches are persisted across generations (they can cause merge conflicts with generator output, but won't be silently clobbered) — so this fix will survive regen. Worth a glance at the next Stainless sync to make sure the patch doesn't conflict, but nothing to hold merge on.

Not stamping a formal approval here — node-sdk is outside the hyperspell/hyperspell compliance gate, and you're driving the merge/publish. Clear to go from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants