diff --git a/CHANGELOG.md b/CHANGELOG.md index 3da84ddc0..d3e6102ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ All notable changes to this project will be documented in this file. ### Changed +- **@overeng/pty-effect/client**: `spawnDaemon` now delegates to `@myobie/pty.spawnDaemon` instead of duplicating the daemon spawn pipeline. The Bun-on-Node case is routed through upstream's new `launcher` option (still honors `NODE_BIN`). Eliminates a divergent in-house spawn path so consumers automatically inherit upstream improvements such as bundle-safe spawn (myobie/pty#38). Public API and `PtyDaemonSpec` schema unchanged. - **@overeng/notion-react**: `` and `` accept `icon={null}` and `cover={null}` as explicit clear sentinels (#618). Dropping the prop is still "no claim" (preserves server state); passing `null` emits `pages.update({icon: null})` / `pages.update({cover: null})`. On a fresh page with no prior icon/cover, `null` is a no-op. - **@overeng/notion-react**: Same-parent `` creates are now sequential — JSX order is preserved 1:1 on the server (#618). Parallel `pages.create` under a common parent yields nondeterministic `child_page` ordering; the driver issues sequential POSTs so no post-create re-fetch is needed. T08 (formerly "concurrent sibling-page order is not authoritative") is now a normative invariant; the deferred `ensureSiblingOrder` sync option is dropped. - **@overeng/notion-react**: `CACHE_SCHEMA_VERSION` bumped `2 → 3` to accommodate per-page cache subtrees (#618). v2 caches fall through the existing `"schema-mismatch"` cold path — transparent, no caller action required. The first sync after upgrade may emit one spurious metadata update per sub-page as response-normalized title/icon/cover is recomputed. diff --git a/packages/@overeng/pty-effect/src/client.ts b/packages/@overeng/pty-effect/src/client.ts index 7737e2275..17948079d 100644 --- a/packages/@overeng/pty-effect/src/client.ts +++ b/packages/@overeng/pty-effect/src/client.ts @@ -8,9 +8,6 @@ * survive process restarts. */ -import { spawn } from 'node:child_process' -import { createRequire } from 'node:module' - import { type EventRecord, type ProcessResources, @@ -31,7 +28,6 @@ import { spawnDaemon as upstreamSpawnDaemon, updateTags as upstreamUpdateTags, validateName, - waitForSocket as upstreamWaitForSocket, } from '@myobie/pty/client' import { Context, @@ -57,7 +53,6 @@ import type { Screenshot } from './Screenshot.ts' /* ───────────────────────────── specs ────────────────────────────── */ const PtyTags = Schema.Record({ key: Schema.String, value: Schema.String }) -const require = createRequire(import.meta.url) /** * Spec for spawning a daemon-mode pty session via `@myobie/pty/client`'s @@ -258,6 +253,12 @@ const buildSpawnOpts = (spec: PtyDaemonSpec): SpawnDaemonOptions => { if (spec.size?.rows !== undefined) opts.rows = spec.size.rows if (spec.size?.cols !== undefined) opts.cols = spec.size.cols if (spec.tags !== undefined && Object.keys(spec.tags).length > 0) opts.tags = { ...spec.tags } + /* When running under Bun, route the detached daemon through Node so the + * PTY server can load its `node-pty` native addon. Honors `NODE_BIN` for + * environments where `node` isn't on PATH. */ + if (process.versions.bun !== undefined) { + opts.launcher = { command: process.env['NODE_BIN'] ?? 'node' } + } return opts } @@ -326,96 +327,18 @@ const withEnvOverrides = ({ }).pipe(Effect.flatMap((saved) => effect.pipe(Effect.ensuring(restore(saved))))) } -const resolvePtyServerModulePath = () => require.resolve('@myobie/pty/server') - -const spawnDaemonViaNode = (spec: PtyDaemonSpec) => - wrapPromise({ - method: 'spawnDaemon', - reason: 'SpawnFailed', - name: spec.name, - thunk: async () => { - const stdout = process.stdout - const rows = spec.size?.rows ?? stdout.rows ?? 24 - const cols = spec.size?.cols ?? stdout.columns ?? 80 - const serverModule = resolvePtyServerModulePath() - const config = JSON.stringify({ - name: spec.name, - command: spec.command, - args: spec.args ?? [], - displayCommand: spec.displayCommand ?? spec.command, - cwd: spec.cwd ?? process.cwd(), - rows, - cols, - ephemeral: spec.ephemeral ?? false, - ...(spec.tags !== undefined && Object.keys(spec.tags).length > 0 - ? { tags: spec.tags } - : {}), - }) - - const child = spawn(process.env['NODE_BIN'] ?? 'node', [serverModule], { - detached: true, - stdio: ['ignore', 'ignore', 'pipe'], - env: { - ...process.env, - ...spec.env, - PTY_SERVER_CONFIG: config, - }, - }) - - let stderrOutput = '' - let earlyExit = false - let earlyExitCode: number | null = null - - child.stderr?.on('data', (data) => { - stderrOutput += data.toString() - }) - child.on('exit', (code) => { - earlyExit = true - earlyExitCode = code - }) - - ;(child.stderr as { unref?: () => void } | null)?.unref?.() - child.unref() - - try { - await upstreamWaitForSocket(spec.name, 3_000, () => { - if (earlyExit === false) return - const details = stderrOutput.trim() - const message = `Daemon process exited immediately (code ${earlyExitCode ?? 'unknown'}).` - throw new Error( - details.length > 0 ? `${message}\n${details}` : `${message} Is the command valid?`, - ) - }) - } catch (error) { - if (earlyExit === false && child.pid !== undefined) { - try { - process.kill(child.pid, 'SIGTERM') - } catch { - // best effort cleanup - } - } - throw error - } - }, - }) - -const spawnDaemonCompat = (spec: PtyDaemonSpec) => - process.versions.bun !== undefined - ? spawnDaemonViaNode(spec) - : withEnvOverrides({ - env: spec.env, - effect: wrapPromise({ - method: 'spawnDaemon', - reason: 'SpawnFailed', - name: spec.name, - thunk: () => upstreamSpawnDaemon(buildSpawnOpts(spec)), - }), - }) - const spawnDaemon = (spec: PtyDaemonSpec): Effect.Effect => Effect.gen(function* () { yield* validateNameOrFail(spec.name) - yield* spawnDaemonCompat(spec) + yield* withEnvOverrides({ + env: spec.env, + effect: wrapPromise({ + method: 'spawnDaemon', + reason: 'SpawnFailed', + name: spec.name, + thunk: () => upstreamSpawnDaemon(buildSpawnOpts(spec)), + }), + }) }).pipe(Effect.withSpan('pty-client.spawnDaemon', { attributes: { 'span.label': spec.name } })) const peek = (input: { diff --git a/packages/@overeng/pty-effect/src/client.unit.test.ts b/packages/@overeng/pty-effect/src/client.unit.test.ts index 7a66967b9..5becf980c 100644 --- a/packages/@overeng/pty-effect/src/client.unit.test.ts +++ b/packages/@overeng/pty-effect/src/client.unit.test.ts @@ -173,23 +173,12 @@ describe('PtyClient client wrapper', () => { ]) }) - it('uses the PTY server module under node when running inside Bun', async () => { + it('passes a Node launcher to upstream spawnDaemon when running inside Bun', async () => { const originalBun = process.versions.bun const originalNodeBin = process.env.NODE_BIN - const waitForSocket = vi.fn( - async (_name: string, _timeoutMs: number, earlyCheck?: () => void) => { - earlyCheck?.() - }, - ) - const child = Object.assign(new EventEmitter(), { - pid: 4242, - stderr: Object.assign(new EventEmitter(), { unref: vi.fn() }), - unref: vi.fn(), - }) - const spawn = vi.fn(() => child) + const spawnDaemon = vi.fn(async () => undefined) - vi.doMock('@myobie/pty/client', () => makeClientMock({ waitForSocket })) - vi.doMock('node:child_process', () => ({ spawn })) + vi.doMock('@myobie/pty/client', () => makeClientMock({ spawnDaemon })) Object.defineProperty(process.versions, 'bun', { configurable: true, @@ -209,7 +198,6 @@ describe('PtyClient client wrapper', () => { args: ['-c', 'true'], cwd: '/tmp', displayCommand: 'shell test', - env: { PTY_EFFECT_TEST_VALUE: 'from-bun-test' }, tags: { owner: 'forge' }, size: { rows: 40, cols: 120 }, }) @@ -217,35 +205,8 @@ describe('PtyClient client wrapper', () => { ) expect(result).toBeUndefined() - expect(spawn).toHaveBeenCalledTimes(1) - const firstCall = spawn.mock.calls.at(0) as ReadonlyArray | undefined - expect(firstCall).toBeDefined() - if (firstCall === undefined) throw new Error('missing spawn call') - const command = firstCall[0] - const args = firstCall[1] - const options = firstCall[2] - if (typeof command !== 'string') throw new Error('expected spawn command') - if (Array.isArray(args) === false) throw new Error('expected spawn args') - if (options === null || typeof options !== 'object') throw new Error('expected spawn options') - expect(command).toBe('node-from-test') - expect(args).toHaveLength(1) - const serverArg = args[0] - expect(serverArg).toBeDefined() - if (typeof serverArg !== 'string') throw new Error('expected server module path') - expect(serverArg).toContain('@myobie/pty') - const spawnOptions = options as { - readonly detached: boolean - readonly stdio: ReadonlyArray - readonly env: Record - } - expect(spawnOptions.detached).toBe(true) - expect(spawnOptions.stdio).toEqual(['ignore', 'ignore', 'pipe']) - expect(spawnOptions.env.PTY_EFFECT_TEST_VALUE).toBe('from-bun-test') - const rawConfig = spawnOptions.env.PTY_SERVER_CONFIG - expect(rawConfig).toBeDefined() - if (rawConfig === undefined) throw new Error('expected PTY_SERVER_CONFIG') - const config = JSON.parse(rawConfig) - expect(config).toEqual({ + expect(spawnDaemon).toHaveBeenCalledTimes(1) + expect(spawnDaemon).toHaveBeenCalledWith({ name: 'unit-bun', command: 'sh', args: ['-c', 'true'], @@ -253,12 +214,10 @@ describe('PtyClient client wrapper', () => { cwd: '/tmp', rows: 40, cols: 120, - ephemeral: false, tags: { owner: 'forge' }, + launcher: { command: 'node-from-test' }, }) - expect(waitForSocket).toHaveBeenCalledWith('unit-bun', 3_000, expect.any(Function)) } finally { - vi.unmock('node:child_process') if (originalBun === undefined) { delete process.versions.bun } else {