Harden WebSocket transport: errno portability, Future leak, duplicate-ID reject

[dsarno]

Summary

Bundles three small, surgical hardening fixes from the audit-v2 umbrella (#343), all in src/godot_ai/transport/websocket.py. Each fix is independently scoped and pinned by a regression test; bundled because they're tightly themed (transport-layer hardening) and trivially reviewable side-by-side.

Closes findings #2, #4, and #5 of #343.

Fixes

#4 (P1) — errno.EADDRINUSE portability

src/godot_ai/transport/websocket.py:46 previously checked e.errno == 48. That number is macOS-only; Linux uses 98 and Windows 10048, so on those platforms the friendly "port already in use" branch never fired and the OSError propagated, taking the WS server lifespan down with a generic traceback. Replaced with errno.EADDRINUSE.

#5 (P2) — send_command pending-Future leak

send_command registered self._pending[request.request_id] = future then awaited ws.send(...). If ws.send raised (ConnectionClosed mid-send, transport error, cancellation), the pending entry was never popped and the dict grew unbounded under churn. Wrapped the send + wait in try/finally: self._pending.pop(...); the response receiver still pops on the happy path so the finally is a no-op there.

#2 (P1) — Duplicate-ID handshake hijack

_handle_connection registered the inbound peer with session_id = handshake.session_id, silently overwriting both the registry entry and _connections[session_id] when the same ID was already present. session_id format is <slug>@<4hex> — 16 bits of suffix is locally guessable, so any local peer could hijack an active session by sending a handshake with the same ID; subsequent MCP commands would route to the impostor.

Now _handle_connection checks registry.get(handshake.session_id) and rejects with WebSocket close code 4001 (RFC 6455 application-defined range) plus a structured warning log naming the existing peer's PID and project. Legitimate plugin reconnect after editor_reload_plugin first triggers ConnectionClosed → unregister, so the new connect still lands cleanly — pinned by test_reconnect_after_clean_disconnect_succeeds.

Tests

New:

• tests/unit/test_websocket_server_lifecycle.py — parametrized errno coverage (macos 48, linux 98, windows 10048; one runs per CI host) plus a non-EADDRINUSE OSError propagation case.
• tests/integration/test_websocket.py::TestDuplicateHandshake — pins both the reject path and the legitimate-reconnect path.
• tests/integration/test_websocket.py::TestPendingFutureCleanup — pins both the timeout-pop and the send-raise pop behaviors (the latter monkey-patches ws.send to force a synthetic ConnectionError after the pending entry is registered).

Existing: 728 passed, 2 skipped (cross-platform errno parametrizations on the non-host OS).

Live smoke

Spawned a real Godot 4.6.2 editor against test_project/, opened main.tscn, ran via the streamable-HTTP MCP transport:

• editor_state → readiness=ready ✓
• test_run → 1037/1040 passed, 0 failed ✓
• Duplicate-ID handshake against the live session's own ID → server closed the WS with code 4001 and reason 'session id already registered' ✓
• editor_state after the rejected hijack → still readiness=ready (original session unaffected) ✓
• Server log: Rejecting duplicate handshake for session test-project@307e (existing pid=21071, project=/home/user/godot-ai/test_project/) ✓

What's NOT closed

• Add MCP resources and pagination #1 (auth/Origin gap on the WS upgrade) — separate scope, deserves its own design discussion. This PR closes only the takeover-of-existing-session path; an unauthenticated local peer can still register a new session_id.
• Add Phase 2 scene + node write tools (Batches 5-6) #3 (path traversal in script_create / filesystem_write_text) — touches GDScript handlers; deserves its own PR with a regression test against res://../etc/passwd.gd.
• Simplify batch_handler #7 (Pydantic-validate WS events) — bigger; needs new envelope models.
• Add multi-angle coverage and camera control for editor_screenshot #6 (drop SessionRegistry RLock) — already not in the code; no change needed.

Test plan

• ruff check src/ tests/ clean
• ruff format --check src/ tests/ clean
• pytest -q — 728 passed
• Live MCP test_run against Godot 4.6.2 editor — 1037/1040 passed, 0 failed
• Live duplicate-ID handshake reject smoke — 4001 close received, original session unaffected
• Logged server-side warning includes existing-peer pid + project_path

https://claude.ai/code/session_016ijmCD5S6QfwJGJcc5Wirp

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[dsarno]

Closing — landed on the wrong branch. Audit v2 (#343) is baselined on beta; opening a fresh PR against beta with the same three fixes (rebased clean onto aac5483).

---

Generated by Claude Code