Skip to content

fix(bsk): find the Windows daemon through daemon.json like every other platform - #32

Merged
code-yeongyu merged 2 commits into
code-yeongyu:mainfrom
LilMGenius:fix/windows-daemon-discovery
Sep 30, 2026
Merged

code-yeongyu merged 2 commits into
code-yeongyu:mainfrom
LilMGenius:fix/windows-daemon-discovery

Conversation

@LilMGenius

@LilMGenius LilMGenius commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

On Windows, connectBrowserSkill() and browser-doctor.mjs could not reach a running BrowserSkill daemon unless the caller passed the pipe name by hand. BskIpcClient.resolveSocket threw before reading daemon.json:

Windows named-pipe discovery is not implemented; pass { sockPath: '\\\\.\\pipe\\...' } explicitly

The daemon already publishes the pipe there. On a Windows machine with bsk 0.3.2 and Chrome connected, ~/.bsk/daemon.json reads "sock_path": "\\\\.\\pipe\\bsk-daemon-305fe01ffb83710a", and node:net connect() takes that path unchanged. So the fix removes the Windows branch and lets discovery read daemon.json on every platform.

Evidence (Windows 11, Node 26.10.0)

Check Before After
new BskIpcClient({ autoStart: false }).call("system.status", {}) against the live daemon throws unsupported daemon 0.3.2 browsers chrome
node --test test/bsk-ipc-client.test.mjs 1 of 12 pass 12 of 12 pass
npm run test:node 149 pass, 40 fail 169 pass, 20 fail

The 20 failures left in the full suite fail identically on main and have Windows-local causes outside this change (skill frontmatter read with CRLF, symlink and permission checks in native-host registration, macOS-only browser signal fixtures, live tests without a headless shell). No test that passed on main fails here.

Change

  • src/bsk/ipc-client.js: drop the win32 throw in resolveSocket; discovery, auto-start and retry are unchanged.
  • test/fixtures/fake-bsk-daemon.mjs: on Windows the fake daemon listens on \\\\.\\pipe\\<tmp dir name>, since listen() on a socket file under tmpdir fails with EACCES there. This makes the existing discovery tests run the Windows path; before it, 11 of them failed on setup rather than on the behaviour.

CI runs on ubuntu-latest only, so the Windows rows above come from a local run.


Summary by cubic

Fixes Windows daemon discovery so connectBrowserSkill() and browser-doctor.mjs can reach a running BrowserSkill daemon without the caller passing the pipe name by hand.

  • Removes the win32 throw in resolveSocket so discovery reads daemon.json on every platform; the daemon already publishes the named pipe there.
  • Updates the fake daemon fixture to listen on a named pipe on Windows, where a socket file under tmpdir fails, so existing discovery tests exercise the real platform path.

Written for commit 445dc66. Summary will update on new commits.

Review in cubic

LilMGenius and others added 2 commits October 1, 2026 07:20
…r platform

On Windows the BrowserSkill daemon publishes its named pipe in daemon.json (sock_path \\.\pipe\bsk-daemon-<hash>), exactly as it publishes the Unix socket elsewhere, and node:net connects to a pipe path the same way. resolveSocket threw "unsupported" before reading that file, so every attached-engine call on Windows failed unless the caller hard-coded the pipe name.

The fake daemon fixture now listens on a named pipe on Windows, where a socket file under tmpdir cannot be listened on, so the existing discovery tests exercise the real platform path.
@code-yeongyu
code-yeongyu merged commit 8b57a56 into code-yeongyu:main Sep 30, 2026
3 checks passed
@code-yeongyu

Copy link
Copy Markdown
Owner

Merged into main as 8b57a56. Thank you @LilMGenius: on Windows the IPC client now finds the BrowserSkill daemon through daemon.json like every other platform, instead of a guessed address. main requires branches to be up to date before merging, so I merged current main into your branch first; your commits are unchanged.

@LilMGenius
LilMGenius deleted the fix/windows-daemon-discovery branch October 4, 2026 14:31
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