Skip to content

Windows: bind the permission broker to a named pipe (every approval silently becomes a deny) - #42

Closed
E4B-labs wants to merge 2 commits into
milind-soni:mainfrom
E4B-labs:fix/windows-permission-broker
Closed

Windows: bind the permission broker to a named pipe (every approval silently becomes a deny)#42
E4B-labs wants to merge 2 commits into
milind-soni:mainfrom
E4B-labs:fix/windows-permission-broker

Conversation

@E4B-labs

Copy link
Copy Markdown
Contributor

Stacked on #41. The diff below shows #41's commit until that merges; this PR's own change is the second commit (server/drivers/claude.ts + claude.test.ts). #41 and this PR are the only two in the Windows series that depend on each other, and only for the test unskip — see "Why stacked" at the bottom.

What's broken

Every permission prompt on Windows silently ends as a deny. The user is never asked, nothing appears in the UI, and there is no error anywhere to explain it — the turn just quietly loses the action ~15 minutes later with the timeout note.

Root cause

createPermissionBroker binds a unix domain socket in DATA_DIR and hands that path to the permission MCP proxy the CLI spawns. Windows has no unix domain sockets, so server.listen(socketPath) fails immediately.

And the failure was invisible:

server.on("error", () => {});

So: the listen fails, nothing is logged, the proxy cannot connect, the ask never reaches onAsk, no request.opened event is emitted, and the CLI's request sits until the 15-minute broker timeout fires and resolves it as deny with source: "timeout". From the user's side an approval prompt simply never appears and the agent reports it was blocked.

The fix

net.createServer binds a named pipe under exactly the same API, both ends included — so permissionSocketPath() returns \\.\pipe\omb-perm-<pid>-<tag> on win32 and the existing socket path everywhere else. Nothing in the broker or the proxy protocol changes.

The pid is in the pipe name deliberately. The Windows pipe namespace is global and flat, so unlike socket files it is not isolated by DATA_DIR; without the pid, two harnesses running concurrently would collide on omb-perm-<tag>.

The error listener now logs instead of discarding. A broker that cannot come up is worth one line on stderr — that swallowed handler is the reason this failure mode was invisible rather than obvious, on any platform.

Tests

permissionSocketPath is exported so the test that raises an ask over the socket asks the driver for the path instead of rebuilding it by hand. It then follows the platform automatically rather than hardcoding the POSIX shape.

With #41 applied, the whole Claude driver suite runs unskipped on Windows, including the broker round-trip.

How this was tested

  • pnpm typecheck and pnpm test on Windows 10, on this branch: 89 passed / 3 skipped, up from 52 passed / 33 skipped on main. The 3 remaining skips are POSIX-only assertions that are correct to skip.
  • The broker round-trip test (brokers a permission ask into request.opened and answers over the socket) now runs on Windows and passes: proxy connects to the pipe, raises an ask, request.opened is emitted, the answer comes back over the same connection.
  • macOS/Linux behaviour is unchanged — the win32 branch is the only new path, and the socket path off win32 is byte-identical to before.

Why stacked

The Claude driver's tests need a spawnable fake CLI (#41) and a bindable broker (this PR) before they can run on Windows — either one alone still fails. Rather than leave the win32 unskip unowned, it rides here, in the later of the two.

The other Windows fixes in this series are independent of both and branch straight from main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HgJeiantdRcZSBrc5sqqCp

E4B-labs and others added 2 commits August 13, 2026 02:50
On Windows the agent CLIs are never found, so every provider reports
"unavailable" and no turn can start.

Three separate reasons, all in the spawn path:

- libuv does not apply PATHEXT, so spawn("claude") looks for a file
  literally named `claude` and never finds `claude.cmd`.
- Since Node's CVE-2024-27980 fix, spawning a `.cmd` without
  `shell: true` throws synchronously.
- Windows has no `#!` support, so a node-shebang script (every fake CLI
  under server/testing) cannot be executed as itself.

`shell: true` is not an option: the drivers pass raw JSON in argv
(claude's `--mcp-config`), which cmd would mangle.

So resolve the target ourselves. `resolveCliSpawn(cli, args)` does a
PATHEXT-aware `which`, parses npm/pnpm `.cmd` shims down to the `.exe`
or node script they wrap, rewrites `#!node` scripts to `node <script>`,
and only falls back to ComSpec for an unparseable shim — with the
double `^`-escaping that fallback then owns, so a raw-JSON argument
survives byte for byte. Off win32 it is the identity function: POSIX
already resolves PATH and `#!` in the kernel.

Every driver spawn/execFile goes through it: claude (spawn, snapshot,
generateText), codex (spawn, snapshot), acp/core (spawn, snapshot).

The spawn-based suites were skipped on win32 for exactly this reason, so
they are unskipped here; server/testing/setup.ts gets a short retry
around the temp-home cleanup (Windows holds a directory that was a live
process's cwd for a beat after the kill returns) and comms.test.ts
passes SystemRoot through to the child, without which winsock fails to
initialize.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgJeiantdRcZSBrc5sqqCp
…allowing listen errors

Every permission prompt on Windows silently ends as a deny.

The Claude driver brokers approvals over a unix domain socket in
DATA_DIR and hands that path to the injected MCP permission proxy.
Windows has no unix sockets, so `server.listen()` fails immediately —
and the listener was `server.on("error", () => {})`, so nothing was
logged. The proxy then cannot connect, the ask never reaches the UI,
and the request times out into a deny with no diagnostic anywhere.

net.createServer binds a named pipe under the same API, so
permissionSocketPath returns `\.\pipe\omb-perm-<pid>-<tag>` on win32
and the existing socket path elsewhere. The pipe namespace is global and
flat — DATA_DIR does not isolate it the way it does for socket files —
so the pid is in the name to keep concurrent harnesses off each other.

The error listener now logs instead of discarding: a broker that cannot
come up is worth one line of stderr rather than an unexplained deny.

The test that raises an ask over the socket now asks the driver for the
path instead of rebuilding it, so it follows the platform.

Stacked on the CLI-resolver PR: with both applied the driver test suite
runs unskipped on Windows (89 passed / 3 skipped, from 52/33 on main).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HgJeiantdRcZSBrc5sqqCp

@milind-soni milind-soni left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main already uses server/procs.ts::brokerSocketPath: a unique \.\pipe\openmausbot-perm-* named pipe on Windows and a filesystem socket on POSIX, and Claude passes that path through the existing broker/proxy flow. This PR is now superseded by the landed implementation.

milind-soni added a commit that referenced this pull request Aug 14, 2026
Integrate #42: collision-safe Windows permission broker
@milind-soni

Copy link
Copy Markdown
Owner

The remaining permission-broker fix has been integrated into main via #83, after #41/#81. I kept the centralized named-pipe helper, added the harness PID there to prevent cross-instance collisions, surfaced listen failures, and enabled the full Claude approval round-trip suite on Windows. The final integration passed Windows, macOS, Ubuntu, and the packaged Ubuntu smoke test. Thank you for isolating the silent-deny failure and supplying the cross-platform test path.

pull Bot referenced this pull request in dubbypanda/OpenMausBot Aug 14, 2026
Port the permission broker fix through the centralized named-pipe helper and add collision coverage.
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