fix(spawn): embed server source so spawnDaemon works in bundled consumers#38
Closed
schickling-assistant wants to merge 1 commit into
Closed
Conversation
schickling-assistant
force-pushed
the
fix/spawn-daemon-bundle-safe
branch
from
bb522bb to
cd2f9ca
Compare
`spawnDaemon` previously did `spawn('node', [<__dirname>/server.js])`.
Under bundlers that virtualise the filesystem (`bun build --compile`,
esbuild single-file, etc.) `import.meta.url` resolves to a `bunfs:`-style
path the spawned `node` child can't read, and the daemon fails to start.
The first iteration of this PR addressed that with an embedded server-
source bundle materialised to `os.tmpdir()`. That worked for resolving
server.js itself but exposed a deeper failure mode: the materialised
file lives outside the consumer's `node_modules`, and ESM resolution
does not honour `NODE_PATH` — so the daemon can't find its own external
deps (`node-pty`, `@xterm/*`) either, and every consumer would need a
bespoke `node_modules`-symlink dance to recover.
Replace the embedded-source approach with CLI delegation. The resolution
strategy becomes:
1. `setServerModulePath()` override — wins over everything (test
harnesses, supervisors with custom paths).
2. Sibling `__dirname/server.js` readable on disk — direct
`node <server.js>` (existing fast path for ordinary npm installs).
3. Bundled context — sibling unreadable. Shell out to `pty run -d
--name <name> --cwd <cwd> [--isolate-env] [--tag k=v]... -- <cmd>
<args>`. The CLI binary is always a real on-disk file with intact
module resolution, so it sidesteps every bundling failure mode at
once: spawning, server materialisation, daemon module resolution,
native-binding loading.
Tradeoff: the CLI path doesn't surface every `SpawnDaemonOptions` field
(`rows`, `cols`, `displayCommand`, `displayName`, `ephemeral`,
`extraEnv`, `env`, `launcher`). For the dominant consumer pattern
(spawn a shell, attach a UI, resize after attach), only `cwd`, `name`,
`tags`, and `isolateEnv` are load-bearing at spawn time — all
supported. Add CLI flags upstream as concrete needs arise. Consumers
that need full fidelity in a bundled context can still call
`setServerModulePath()` with a real on-disk server.
Drops `scripts/embed-server-source.js`, the `dist/server-source.txt`
artifact, and the `esbuild` dev-dep that the embed pipeline required.
Tests cover all three resolution strategies (on-disk, CLI delegation,
explicit override) and the no-CLI-on-PATH error path.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
schickling-assistant
force-pushed
the
fix/spawn-daemon-bundle-safe
branch
from
cd2f9ca to
86f7837
Compare
Owner
|
Landed via local rebase + fast-forward as commits cebe2d1 and adcec22 (the rebase resolved conflicts with the supervisor fix from b9a07bb, and the follow-up commit shortens session names in the new test to stay under macOS's 104-byte Unix socket path limit). Closing in favor of those direct merges. Thanks @schickling-assistant! |
schickling-assistant
added a commit
to overengineeringstudio/effect-utils
that referenced
this pull request
May 8, 2026
`@overeng/pty-effect/client`'s `spawnDaemon` previously reimplemented the daemon spawn pipeline (server module resolution, child process launch, socket wait, early-exit detection) to work around the lack of a Bun-on-Node escape hatch in upstream `@myobie/pty.spawnDaemon`. Upstream now ships a `launcher` option that covers exactly that case, so the in-house path is obsolete. Replace it with a thin wrapper that calls upstream `spawnDaemon` and sets `launcher` to `NODE_BIN ?? "node"` when running under Bun. Public API and `PtyDaemonSpec` schema unchanged. Consumers automatically inherit upstream improvements such as bundle-safe spawn (myobie/pty#38) without any local workaround. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When consumers compile this package into a single binary (e.g.
bun build --compile),import.meta.urlofdist/spawn.jsresolves to a virtual path likebunfs:/.... The spawnednodedaemon child cannot read that path, sospawnDaemonfails with:The existing
setServerModulePath()escape hatch helps, but it puts the burden on every bundling consumer to ship a real on-disk copy ofdist/server.jsplus its transitive sibling deps and plumb a path through. That's fragile and easy to get wrong.Approach
scripts/embed-server-source.jsruns aftertscinnpm run build. It uses esbuild to bundledist/server.jstogether with its in-package siblings into a single self-contained file (dist/server-source.txt). npm deps (node-pty,@xterm/*) stay external — bundled consumers must keep those resolvable from the spawned process, same as today.src/spawn.tsnow resolves the server module in three steps:setServerModulePath()override wins unconditionally (existing behaviour preserved).server.jsnext tospawn.jsif it's a readable file.os.tmpdir()/myobie-pty-server-<sha256-prefix>/server.jsand spawn that. One file per content hash, so concurrent daemons share a single tmpfile; not unlinked on spawn (the daemon needs it for its lifetime; OS reaps tmpdir).server-source.txtis gitignored and only exists so the upstream test suite (which runs through tsx againstsrc/) can exercise the fallback path.Tests
tests/spawn-bundle-fallback.test.tscovers:server.js,spawnDaemonmaterialises the bundle and the daemon comes up.setServerModulePath()override still wins.npm run build && npx vitest run tests/spawn-bundle-fallback.test.ts tests/spawn-options.test.ts tests/integration.test.ts tests/supervisor.test.ts— 75 passed. Pre-existing failures intests/shells.test.ts(zsh missing) andtests/screenshot.test.ts(vim env-dependent) are unrelated.Context
Hit while bundling a downstream consumer via
bun build --compile: schickling/dotfiles#836.Independent of #37.
Posted on behalf of @schickling
agent_nameagent_session_idagent_toolagent_tool_versionagent_runtimeagent_modelworktreemachinetooling_profile