From 2c78b839d02c016e4216f6826b9bc908fb39aaf9 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 09:23:41 +0800 Subject: [PATCH 01/22] fix(pty): guard node-pty's unlistened win32 conin socket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit node-pty attaches an 'error' listener to the conout socket but not to conin (windowsPtyAgent.ts:79). A net.Socket without one turns any write failure into an uncaught exception, so an EAGAIN under backpressure — or a write racing kill() — crashes the daemon on Windows. Attach the listener the upstream package is missing. Refs #59 --- .../pty/__tests__/win-conin-guard.test.ts | 37 +++++++++++++++++++ src/kernel/pty/session-host.ts | 4 ++ src/kernel/pty/win-conin-guard.ts | 30 +++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 src/kernel/pty/__tests__/win-conin-guard.test.ts create mode 100644 src/kernel/pty/win-conin-guard.ts diff --git a/src/kernel/pty/__tests__/win-conin-guard.test.ts b/src/kernel/pty/__tests__/win-conin-guard.test.ts new file mode 100644 index 00000000..a0c9d9c9 --- /dev/null +++ b/src/kernel/pty/__tests__/win-conin-guard.test.ts @@ -0,0 +1,37 @@ +import { describe, it, expect } from 'vitest'; +import { EventEmitter } from 'node:events'; +import type { IPty } from 'node-pty'; +import { guardWindowsConinSocket } from '../win-conin-guard.js'; + +/** Shaped like node-pty's WindowsTerminal: a private _agent exposing inSocket. */ +const fakeWinPty = (): { pty: IPty; inSocket: EventEmitter } => { + const inSocket = new EventEmitter(); + return { pty: { _agent: { inSocket } } as unknown as IPty, inSocket }; +}; + +describe('guardWindowsConinSocket', () => { + it('attaches an error listener to the conin socket on win32', () => { + const { pty, inSocket } = fakeWinPty(); + expect(guardWindowsConinSocket(pty, 'win32')).toBe(true); + expect(inSocket.listenerCount('error')).toBe(1); + }); + + it('swallows a conin error instead of letting it become an uncaught exception', () => { + const { pty, inSocket } = fakeWinPty(); + guardWindowsConinSocket(pty, 'win32'); + // Without a listener, EventEmitter rethrows. With one, this is a no-op. + expect(() => inSocket.emit('error', Object.assign(new Error('write EAGAIN'), { code: 'EAGAIN' }))).not.toThrow(); + }); + + it('does nothing on non-win32 platforms', () => { + const { pty, inSocket } = fakeWinPty(); + expect(guardWindowsConinSocket(pty, 'linux')).toBe(false); + expect(inSocket.listenerCount('error')).toBe(0); + }); + + it('reports false rather than throwing when node-pty no longer exposes the socket', () => { + // Future-proofing: an upstream rename must degrade to a no-op, not a crash. + expect(guardWindowsConinSocket({} as IPty, 'win32')).toBe(false); + expect(guardWindowsConinSocket({ _agent: {} } as unknown as IPty, 'win32')).toBe(false); + }); +}); diff --git a/src/kernel/pty/session-host.ts b/src/kernel/pty/session-host.ts index cb8a7a8b..1704597d 100644 --- a/src/kernel/pty/session-host.ts +++ b/src/kernel/pty/session-host.ts @@ -12,6 +12,7 @@ import { Terminal as HeadlessTerminal } from '@xterm/headless'; import { SerializeAddon } from '@xterm/addon-serialize'; import { isPipePath } from '../ipc/client.js'; import { FrameDecoder, FrameType, encodeData, encodeSize, parseDims } from '../web/stream-protocol.js'; +import { guardWindowsConinSocket } from './win-conin-guard.js'; export interface SessionHostOpts { id: string; @@ -84,6 +85,9 @@ export class SessionHost { // lets scripts detect the wrapper and lets `tlive run` refuse to nest. env: { ...(this.opts.env ?? process.env), TLIVE_SESSION: this.opts.id } as Record, }); + // node-pty leaves the win32 conin socket without an 'error' listener; an + // unguarded write failure there is an uncaught exception. See the module. + guardWindowsConinSocket(this.pty); this.shadow = new HeadlessTerminal({ cols: size.cols, rows: size.rows, scrollback: 1000, allowProposedApi: true }); this.serializer = new SerializeAddon(); diff --git a/src/kernel/pty/win-conin-guard.ts b/src/kernel/pty/win-conin-guard.ts new file mode 100644 index 00000000..faf87181 --- /dev/null +++ b/src/kernel/pty/win-conin-guard.ts @@ -0,0 +1,30 @@ +// +// Containment for an upstream node-pty defect on Windows. +// +// node-pty opens two pipes to the ConPTY. The read side (conout) gets an +// 'error' listener in windowsTerminal.ts:90; the write side (conin) is +// constructed in windowsPtyAgent.ts:79 with none. A net.Socket without an +// 'error' listener turns any emitted error into an uncaught exception, so a +// failing pty write takes the whole daemon down. Two triggers reach it: EAGAIN +// when conin's buffer fills under backpressure, and a write racing kill()'s +// _inSocket.destroy() (windowsPtyAgent.ts:176). +// +// Reaching through the private _agent is deliberate — Terminal.on() forwards to +// the conout socket, so there is no public route to conin. Verified against +// node-pty@1.2.0-beta.14. Delete this once an upstream release carries the +// listener; re-check the internal shape on every node-pty bump. + +import type { IPty } from 'node-pty'; + +interface ErrorEmitter { on(event: 'error', listener: (err: Error) => void): unknown } +interface WindowsPtyInternals { _agent?: { inSocket?: ErrorEmitter } } + +/** Returns whether a listener was attached (false on non-win32, or if the shape is gone). */ +export function guardWindowsConinSocket(pty: IPty, platform: string = process.platform): boolean { + if (platform !== 'win32') return false; + const sock = (pty as unknown as WindowsPtyInternals)._agent?.inSocket; + if (!sock || typeof sock.on !== 'function') return false; + // A failed write is not fatal: the pty's real lifecycle is driven by onExit. + sock.on('error', () => { /* swallowed — see the note above */ }); + return true; +} From 1e502f5b55c400f1b4adc667eff04851635632a0 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 09:31:09 +0800 Subject: [PATCH 02/22] fix(pty): drop the pty handle on kill and on child exit stop() killed the pty but left this.pty set, so a Data frame already queued on a client socket still reached pty.write() after the pipe was torn down. Clearing the handle turns those late writes into no-ops. Refs #59 --- src/kernel/pty/__tests__/session-host.test.ts | 28 +++++++++++++++++++ src/kernel/pty/session-host.ts | 13 ++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index f100ccef..2f55dd9c 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -224,4 +224,32 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { await host.stop(); }); + // Regression: stop() killed the pty but left the handle in place, so a client + // Data frame already queued could still reach a pty whose win32 conin socket + // had just been destroyed — an uncaught 'write EAGAIN'. See #59. + it('clears the pty handle on stop so late input cannot reach a killed pty', async () => { + const dir = mkdtempSync(join(tmpdir(), 'tlive-dispose-')); + const host = new SessionHost({ + id: 'd1', cmd: process.execPath, args: ['-e', 'process.stdin.pipe(process.stdout)'], + cwd: dir, sockPath: sessSock(dir, 'd'), attachLocal: false, + }); + await host.start(); + expect((host as unknown as { pty: unknown }).pty).not.toBeNull(); + + await host.stop(); + expect((host as unknown as { pty: unknown }).pty).toBeNull(); + }); + + it('clears the pty handle when the child exits on its own', async () => { + const dir = mkdtempSync(join(tmpdir(), 'tlive-exit-')); + const host = new SessionHost({ + id: 'd2', cmd: process.execPath, args: ['-e', 'process.exit(0)'], + cwd: dir, sockPath: sessSock(dir, 'e'), attachLocal: false, + }); + const exited = new Promise((resolve) => host.onExit(resolve)); + await host.start(); + await exited; + expect((host as unknown as { pty: unknown }).pty).toBeNull(); + }); + }); diff --git a/src/kernel/pty/session-host.ts b/src/kernel/pty/session-host.ts index 1704597d..af62c9ab 100644 --- a/src/kernel/pty/session-host.ts +++ b/src/kernel/pty/session-host.ts @@ -105,6 +105,8 @@ export class SessionHost { }); this.pty.onExit(({ exitCode }) => { + // The child is gone; drop the handle so nothing can write to it. + this.pty = null; this.cleanup(); this.onExitCb?.(exitCode); }); @@ -145,7 +147,16 @@ export class SessionHost { async stop(): Promise { this.cleanup(); - try { this.pty?.kill(); } catch { /* already dead */ } + this.disposePty(); + } + + /** Kill the child and drop the handle. Every write/resize site uses `this.pty?.`, + * so clearing it makes late input a no-op instead of a write to a dead pipe. */ + private disposePty(): void { + const pty = this.pty; + this.pty = null; + if (!pty) return; + try { pty.kill(); } catch { /* already dead */ } } private onLocalInput = (chunk: Buffer): void => { From e3a3666641cd1d4c94544962afbaaee3f3080b13 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 10:15:16 +0800 Subject: [PATCH 03/22] test(daemon): wait on conditions, not fixed timeouts, in session-ipc A fixed 40ms tick assumed the request had crossed the IPC socket. On Windows named pipes it had not, which is the 'expected +0 to be 1' red. Adds a shared until() helper for arrival assertions; absence assertions keep a real elapsed interval, which is the only thing that makes them meaningful. --- src/kernel/__tests__/wait.ts | 20 +++++++++++++ .../daemon/__tests__/session-ipc.test.ts | 29 +++++++++---------- 2 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 src/kernel/__tests__/wait.ts diff --git a/src/kernel/__tests__/wait.ts b/src/kernel/__tests__/wait.ts new file mode 100644 index 00000000..c54f6144 --- /dev/null +++ b/src/kernel/__tests__/wait.ts @@ -0,0 +1,20 @@ +// Condition-based waiting for tests. +// +// Fixed `setTimeout` waits before an arrival assertion are the project's main +// source of Windows CI flake: named pipes are slower than unix sockets, so a +// margin that holds on Linux does not hold there (#45, #59's sibling red). +// vitest's default waitFor timeout is 1s, which is too tight for daemon +// bootstrap over a pipe. +// +// Use this for ARRIVAL assertions — "X has appeared". Do NOT use it for +// ABSENCE assertions — "X did not happen" needs a real elapsed interval, and a +// condition that is already true returns immediately, asserting nothing. + +import { vi } from 'vitest'; + +const TIMEOUT_MS = 5000; +const INTERVAL_MS = 20; + +export function until(assertion: () => void | Promise): Promise { + return vi.waitFor(assertion, { timeout: TIMEOUT_MS, interval: INTERVAL_MS }) as Promise; +} diff --git a/src/kernel/daemon/__tests__/session-ipc.test.ts b/src/kernel/daemon/__tests__/session-ipc.test.ts index 7199da29..634d799e 100644 --- a/src/kernel/daemon/__tests__/session-ipc.test.ts +++ b/src/kernel/daemon/__tests__/session-ipc.test.ts @@ -7,6 +7,7 @@ import { bootstrapDaemon, type DaemonHandle } from '../bootstrap'; import { request, daemonSocketPath } from '../../ipc/client'; import type { SessionMeta } from '../../ipc/protocol'; import type { IMAdapter, OutgoingMessage } from '../../contracts/im-adapter'; +import { until } from '../../__tests__/wait.js'; const recordingAdapter = (sent: OutgoingMessage[]): IMAdapter => ({ channel: 'telegram', @@ -54,8 +55,7 @@ describe('session.* over IPC', () => { }); }); -describe('parent-session 清场 must be agent-scoped (backgrounded sub-agent approval survival)', () => { - const tick = () => new Promise((r) => setTimeout(r, 40)); +describe('parent-session teardown must be agent-scoped (backgrounded sub-agent approval survival)', () => { // A configured chat keeps requestPermission pending; with no injected imAdapters // sendToChat is a no-op (no network), so the pending simply sits in the router. // holdSubagents:true — these tests exercise the *held* sub-agent path (the @@ -73,14 +73,13 @@ describe('parent-session 清场 must be agent-scoped (backgrounded sub-agent app writeConfig(); h = await bootstrapDaemon({ home: tmp }); const sub = fireSubAgentApproval(); - await tick(); - expect(h.permissionRouter.pendingCount()).toBe(1); + await until(() => { expect(h.permissionRouter.pendingCount()).toBe(1); }); // The parent (main session, no agent_id) submits a new prompt while the // sub-agent is still waiting. The sub-agent's tool call is genuinely pending // and has no local answer path — its card must survive. await request({ kind: 'hook.event', event: { event: 'prompt', cwd: '/proj', sessionId: 'parent', prompt: 'do something else' } }, { socketPath: sock, timeoutMs: 2000 }); - await tick(); + await new Promise((r) => setTimeout(r, 100)); // real elapsed time: prove the cancel did NOT arrive expect(h.permissionRouter.pendingCount()).toBe(1); h.permissionRouter.cancel({ key: 'parent' }); @@ -92,12 +91,10 @@ describe('parent-session 清场 must be agent-scoped (backgrounded sub-agent app h = await bootstrapDaemon({ home: tmp }); const main = request({ kind: 'hook.permission.request', cwd: '/proj', sessionId: 'parent', toolName: 'Bash', input: {} }, { socketPath: sock, timeoutMs: 4000 }).catch(() => undefined); // no agentId = main session - await tick(); - expect(h.permissionRouter.pendingCount()).toBe(1); + await until(() => { expect(h.permissionRouter.pendingCount()).toBe(1); }); await request({ kind: 'hook.event', event: { event: 'prompt', cwd: '/proj', sessionId: 'parent', prompt: 'next' } }, { socketPath: sock, timeoutMs: 2000 }); - await tick(); - expect(h.permissionRouter.pendingCount()).toBe(0); // main-session dialog is gone → its card is withdrawn + await until(() => { expect(h.permissionRouter.pendingCount()).toBe(0); }); // main-session dialog is gone → its card is withdrawn await main; }); @@ -106,13 +103,12 @@ describe('parent-session 清场 must be agent-scoped (backgrounded sub-agent app writeConfig(); h = await bootstrapDaemon({ home: tmp }); const sub = fireSubAgentApproval(); - await tick(); - expect(h.permissionRouter.pendingCount()).toBe(1); + await until(() => { expect(h.permissionRouter.pendingCount()).toBe(1); }); // Stop long-polls (grace + continue window); fire-and-forget — the cancel // it performs runs synchronously on arrival, before any waiting. const stop = request({ kind: 'hook.continue.request', cwd: '/proj', sessionId: 'parent', context: 'turn ended' }, { socketPath: sock, timeoutMs: 300 }).catch(() => undefined); - await tick(); + await new Promise((r) => setTimeout(r, 100)); // real elapsed time: prove the cancel did NOT arrive expect(h.permissionRouter.pendingCount()).toBe(1); h.permissionRouter.cancel({ key: 'parent' }); @@ -147,10 +143,11 @@ describe('continuation card has no on-card input box (quote-reply is the entry)' // Stop long-polls (continue window); fire-and-forget — the card is sent // synchronously when continueBroker registers the request, before the wait. request({ kind: 'hook.continue.request', cwd: '/proj', sessionId: 'sess', context: 'All green.' }, { socketPath: sock, timeoutMs: 300 }).catch(() => undefined); - await new Promise((r) => setTimeout(r, 120)); - const card = sent.find((m) => m.kind === 'card' && (m.title ?? '').includes('Turn finished')); - expect(card).toBeTruthy(); - expect((card as { inputAction?: unknown }).inputAction).toBeUndefined(); + await until(() => { + const card = sent.find((m) => m.kind === 'card' && (m.title ?? '').includes('Turn finished')); + expect(card).toBeTruthy(); + expect((card as { inputAction?: unknown }).inputAction).toBeUndefined(); + }); }); }); From 00f8819aab47ff01cddc9b8158c91588b15942df Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 10:31:04 +0800 Subject: [PATCH 04/22] test(ipc,web,pty): wait on conditions instead of fixed delays Same Windows-flake class as session-ipc: fixed sleeps sized for unix sockets before arrival assertions. Absence assertions and the genuinely time-based idle-flip test keep their fixed intervals. --- src/kernel/ipc/__tests__/server.test.ts | 4 ++-- src/kernel/pty/__tests__/session-host.test.ts | 11 +++++++++-- src/kernel/web/__tests__/pty-bridge.test.ts | 7 +++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/kernel/ipc/__tests__/server.test.ts b/src/kernel/ipc/__tests__/server.test.ts index fbaaeede..88ec529b 100644 --- a/src/kernel/ipc/__tests__/server.test.ts +++ b/src/kernel/ipc/__tests__/server.test.ts @@ -5,6 +5,7 @@ import { join } from 'node:path'; import { createConnection } from 'node:net'; import { startIpcServer, type IpcServer } from '../server'; import { request, daemonSocketPath } from '../client'; +import { until } from '../../__tests__/wait.js'; let cleanup: Array<() => void> = []; afterEach(() => { cleanup.forEach((f) => f()); cleanup = []; }); @@ -36,8 +37,7 @@ describe('IpcCallContext.onDisconnect', () => { setTimeout(() => { sock.destroy(); resolve(); }, 30); }); }); - await new Promise((r) => setTimeout(r, 60)); - expect(fired).toBe(true); + await until(() => { expect(fired).toBe(true); }); }); it('does not fire synchronously when registered, before any disconnect has occurred', async () => { diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index 2f55dd9c..afcfa4c5 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -147,9 +147,16 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { sockPath, attachLocal: false, }); + // The snapshot path is only under test if EARLY-SCREEN reaches the shadow + // terminal BEFORE the client attaches — otherwise the live output carries it + // and the test passes without exercising the snapshot at all. onActivity + // fires on the host's first observed pty output, so wait for that rather + // than guessing. It must be registered before start() creates the timer. + const sawOutput = new Promise((resolve) => { + host.onActivity((active) => { if (active) resolve(); }); + }); await host.start(); - // let the output land in the shadow terminal before anyone attaches - await new Promise((r) => setTimeout(r, 400)); + await sawOutput; const dec = new FrameDecoder(); const got = await new Promise((resolve, reject) => { diff --git a/src/kernel/web/__tests__/pty-bridge.test.ts b/src/kernel/web/__tests__/pty-bridge.test.ts index 0d5cbdc8..97e13dce 100644 --- a/src/kernel/web/__tests__/pty-bridge.test.ts +++ b/src/kernel/web/__tests__/pty-bridge.test.ts @@ -5,6 +5,7 @@ import { mkdtempSync } from 'node:fs'; import { join, basename } from 'node:path'; import { tmpdir } from 'node:os'; import * as net from 'node:net'; +import { until } from '../../__tests__/wait.js'; // Per-session socket: fs path on POSIX, named pipe on win32 (no unix sockets). const sessSock = (dir: string, name: string): string => @@ -35,9 +36,11 @@ describe('PtyBridge', () => { const ws = new FakeWs(); const b = bridge(ws as never, sockPath); - // give the bridge's net.connect a moment, then drive input through the ws - await new Promise((r) => setTimeout(r, 100)); + // Writes before connect are queued by Node, so the attach can go straight + // out. The host answers an Attach with a Size frame — wait for that instead + // of guessing at a connect delay, then send the payload. ws.emit('message', encodeAttach(80, 24)); + await until(() => { expect(ws.sent.length).toBeGreaterThan(0); }); ws.emit('message', encodeData(Buffer.from('ping\n'))); await new Promise((resolve, reject) => { From 63346b7c0b335dccf97fb420332cc74f71fb3fff Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 10:51:27 +0800 Subject: [PATCH 05/22] fix(pty): report activity from observed output, not from start time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lastOutputAt was initialised at construction, so the first activity poll always classified a session as running — 1000ms elapsed is inside the 1500ms window regardless of whether the child ever wrote anything. A silent session reported running for its first second, contradicting the contract in this file's own header, and made onActivity unusable as a "saw output" signal. --- src/kernel/pty/__tests__/session-host.test.ts | 27 ++++++++++++++++--- src/kernel/pty/session-host.ts | 4 +-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index afcfa4c5..32c0924c 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -149,9 +149,10 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { }); // The snapshot path is only under test if EARLY-SCREEN reaches the shadow // terminal BEFORE the client attaches — otherwise the live output carries it - // and the test passes without exercising the snapshot at all. onActivity - // fires on the host's first observed pty output, so wait for that rather - // than guessing. It must be registered before start() creates the timer. + // and the test passes without exercising the snapshot at all. onActivity now + // fires (true) only when the host observes actual pty output, so wait for that + // to order the output ahead of the client's attach. It must be registered before + // start() creates the timer. const sawOutput = new Promise((resolve) => { host.onActivity((active) => { if (active) resolve(); }); }); @@ -211,6 +212,26 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { await host.stop(); }); + it('a silent child (no output) does not report as running', async () => { + const dir = mkdtempSync(join(tmpdir(), 'tlive-host-')); + const sockPath = sessSock(dir, 's'); + // child that consumes stdin but produces no output + const host = new SessionHost({ + id: 'act-silent', cmd: process.execPath, + args: ['-e', 'process.stdin.resume(); setInterval(()=>{},1000);'], + cwd: dir, sockPath, attachLocal: false, + }); + const flips: boolean[] = []; + host.onActivity((a) => flips.push(a)); + await host.start(); + // Poll interval is 1s, first tick at ~1000ms. Give it a bit past + // the first tick to ensure onActivity(true) has fired if the child + // had emitted anything (it hasn't). This is an absence assertion — + // a fixed delay proves that no true flip occurred. + await new Promise((r) => setTimeout(r, 1200)); + expect(flips).not.toContain(true); // never reported running + await host.stop(); + }); it('reports running on output then idle after silence', async () => { const dir = mkdtempSync(join(tmpdir(), 'tlive-host-')); diff --git a/src/kernel/pty/session-host.ts b/src/kernel/pty/session-host.ts index af62c9ab..5137107b 100644 --- a/src/kernel/pty/session-host.ts +++ b/src/kernel/pty/session-host.ts @@ -58,7 +58,7 @@ export class SessionHost { private shadow: HeadlessTerminal | null = null; private serializer: SerializeAddon | null = null; // Vendor-neutral activity: recent pty output = running, silence = idle. - private lastOutputAt = Date.now(); + private lastOutputAt: number | null = null; private activeState: boolean | null = null; private activityTimer: ReturnType | null = null; private onActivityCb: ((active: boolean) => void) | null = null; @@ -113,7 +113,7 @@ export class SessionHost { // Poll output-activity; report only on a running↔idle flip. this.activityTimer = setInterval(() => { - const active = Date.now() - this.lastOutputAt < SessionHost.IDLE_MS; + const active = this.lastOutputAt !== null && Date.now() - this.lastOutputAt < SessionHost.IDLE_MS; if (active !== this.activeState) { this.activeState = active; this.onActivityCb?.(active); } }, 1000); this.activityTimer.unref?.(); From 2c1375ab475d1c5ae4acd91221186bf9d86c2357 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 11:11:49 +0800 Subject: [PATCH 06/22] test(daemon): condition-based waits in event-broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten arrival assertions were preceded by fixed 100-150ms sleeps. The four absence assertions keep theirs — a condition that is already true returns immediately and would assert nothing. --- .../daemon/__tests__/event-broadcast.test.ts | 42 +++++++++++-------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/kernel/daemon/__tests__/event-broadcast.test.ts b/src/kernel/daemon/__tests__/event-broadcast.test.ts index 7dc8b18a..fc979ed0 100644 --- a/src/kernel/daemon/__tests__/event-broadcast.test.ts +++ b/src/kernel/daemon/__tests__/event-broadcast.test.ts @@ -7,6 +7,7 @@ import { WebSocket } from 'ws'; import { bootstrapDaemon, type DaemonHandle } from '../bootstrap.js'; import { request } from '../../ipc/client.js'; import type { IMAdapter, IMChannel, OutgoingMessage, IncomingEnvelope } from '../../contracts/im-adapter.js'; +import { until } from '../../__tests__/wait.js'; let tmp: string; let h: DaemonHandle; @@ -109,8 +110,7 @@ describe('daemon → /ws/events downstream broadcast', () => { expect(f.session.lastMessage).toBe('last_msg'); // Effect 2: ContinueBroker received the request → IM message sent containing requestId. - await new Promise((r) => setTimeout(r, 100)); - expect(capturedMsg).toMatch(/last_msg/); // excerpt = 真正的最后一句 + await until(() => { expect(capturedMsg).toMatch(/last_msg/); }); // excerpt = the actual last message // requestId no longer appears in the display text — take it from the registry const continueId = h.sessions.get('s')!.continueId!; expect(continueId).toMatch(/[a-f0-9-]{36}/); @@ -182,20 +182,22 @@ describe('daemon → /ws/events downstream broadcast', () => { const f = await waitFor(frames, (x) => x.type === 'session-upsert' && x.session.id === 's2' && x.session.status === 'waiting-approval'); const rid = f.session.pending.requestId; expect(f.session.pending.ask).toMatchObject({ question: 'First?', index: 0, total: 2 }); - await new Promise((r) => setTimeout(r, 100)); - const first = sentIm[0] as { title?: string; body: string }; - expect(first.title).toContain('Question 1/2'); + await until(() => { + expect(sentIm[0]).toBeDefined(); + expect((sentIm[0] as { title?: string })?.title).toContain('Question 1/2'); + }); // Answer question 1 from the dashboard — the batch must NOT resolve yet, // and BOTH surfaces must move to question 2. ws.send(JSON.stringify({ type: 'ask', requestId: rid, picks: [0] })); const second = await waitFor(frames, (x) => x.type === 'session-upsert' && x.session.id === 's2' && x.session.pending?.ask?.index === 1); expect(second.session.pending.ask).toMatchObject({ question: 'Second?', index: 1, total: 2 }); - await new Promise((r) => setTimeout(r, 100)); + await until(() => { + expect((edits.at(-1) as { title?: string } | undefined)?.title).toContain('Question 2/2'); + }); // The IM card follows the dashboard: title progress AND body, not just one // of them (the badge froze at "Question 1/2" while the body moved on). const edited = edits.at(-1) as { title?: string; body: string; buttons?: Array<{ id: string }> }; - expect(edited.title).toContain('Question 2/2'); expect(edited.body).toContain('Second?'); expect(edited.buttons?.map((b) => b.id)).toContain(`askback:${rid}`); @@ -225,8 +227,7 @@ describe('daemon → /ws/events downstream broadcast', () => { ); const f = await waitFor(frames, (x) => x.type === 'session-upsert' && x.session.id === 's' && x.session.status === 'waiting-approval'); expect(f.session.pending.ask).toEqual({ question: 'Pick a color?', options: [{ label: 'Red' }, { label: 'Blue' }], multiSelect: false, index: 0, total: 1 }); - await new Promise((r) => setTimeout(r, 100)); - expect(sentIm).toHaveLength(1); // IM still gets the ask card with option buttons + await until(() => { expect(sentIm).toHaveLength(1); }); // IM still gets the ask card with option buttons // Answer from the dashboard: pick "Blue" → allow + updatedInput.answers // (same wire as the IM buttons — CC treats the tool as answered). @@ -342,6 +343,15 @@ describe('daemon → /ws/events downstream broadcast', () => { h = await bootstrapDaemon({ home: tmp, imAdapters: [adapter] }); const t0 = Date.now(); const p = request({ kind: 'hook.continue.request', cwd: '/g', sessionId: 's', context: 'ctx' }, { socketPath: sock, timeoutMs: 8000 }); + // No `until()` here on purpose: this is a sequencing gate (let the request + // above reach the daemon's grace-registration point), not an arrival + // assertion, and there is no arrival to poll for. `h.sessions.get('s')` + // stays undefined and `continueId` is never set on THIS (suppressed) path + // — continueId is only ever assigned inside ContinueBroker's onRequest + // callback, which fires from `continueBroker.request()`, and that call is + // skipped entirely once `suppressed` resolves true (bootstrap.ts's + // 'hook.continue.request' case returns before reaching it). Waiting on + // continueId would hang for the full 5s `until()` timeout every run. await new Promise((r) => setTimeout(r, 150)); // user starts a new turn within the grace window → suppress the continue card await request({ kind: 'hook.event', event: { event: 'prompt', cwd: '/g', sessionId: 's', prompt: 'next' } }, { socketPath: sock, timeoutMs: 2000 }); @@ -380,15 +390,14 @@ describe('daemon → /ws/events downstream broadcast', () => { // seed the session so the card gets a label tag await request({ kind: 'hook.event', event: { event: 'session-start', cwd: '/tag/repo', sessionId: 's', source: 'startup' } }, { socketPath: sock, timeoutMs: 2000 }); const p = request({ kind: 'hook.permission.request', cwd: '/tag/repo', sessionId: 's', toolName: 'Edit', input: { file_path: '/x', old_string: 'a', new_string: 'b' } }, { socketPath: sock, timeoutMs: 5000 }); - await new Promise((r) => setTimeout(r, 150)); + await until(() => { expect(sent.find((s) => s.kind === 'card')).toBeDefined(); }); const card = sent.find((s) => s.kind === 'card'); expect(card?.title).toContain('repo · '); // session tag prefix (still basename(cwd) — label is unaffected by the key change) // answer → card edited to outcome. Keyed by session id ('s'), not cwd. const reqId = h.sessions.get('s')!.pending!.requestId; await request({ kind: 'hook.permission.answer', requestId: reqId, approved: false }, { socketPath: sock, timeoutMs: 2000 }); await p; - await new Promise((r) => setTimeout(r, 100)); - expect(edits.length).toBe(1); + await until(() => { expect(edits.length).toBe(1); }); expect(edits[0].title).toContain('Denied'); }); @@ -458,8 +467,7 @@ describe('daemon → /ws/events downstream broadcast', () => { // resolve A (deny) — must NOT wipe B's pending indicator await request({ kind: 'hook.permission.answer', requestId: reqA, approved: false }, { socketPath: sock, timeoutMs: 2000 }); expect(((await pA) as { decision: string }).decision).toBe('deny'); - await new Promise((r) => setTimeout(r, 80)); - expect(h.sessions.get('s')?.pending?.requestId).toBe(reqB); + await until(() => { expect(h.sessions.get('s')?.pending?.requestId).toBe(reqB); }); // cleanup await request({ kind: 'hook.permission.answer', requestId: reqB, approved: false }, { socketPath: sock, timeoutMs: 2000 }); await pB; @@ -531,8 +539,7 @@ describe('daemon → /ws/events downstream broadcast', () => { ); await waitFor(frames, (x) => x.type === 'session-upsert' && x.session.id === 's' && x.session.lastMessage === 'Bash failed: permission denied'); - await new Promise((r) => setTimeout(r, 100)); - expect(sent).toHaveLength(1); + await until(() => { expect(sent).toHaveLength(1); }); expect(sent[0]).toContain('permission denied'); }); @@ -550,8 +557,7 @@ describe('daemon → /ws/events downstream broadcast', () => { ); await waitFor(frames, (x) => x.type === 'session-upsert' && x.session.id === 's'); - await new Promise((r) => setTimeout(r, 100)); - expect(sent).toHaveLength(1); + await until(() => { expect(sent).toHaveLength(1); }); }); }); From ec139227b760957f312d90a99620893aadad2ccb Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 11:23:00 +0800 Subject: [PATCH 07/22] test(daemon): restore a real interval on the pending-invariant check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Denying request A must not wipe B's pending indicator. reqB is read off the frame where pending had already moved to B, so the assertion is an invariant, not an arrival — until() returned on its first check and proved nothing. Widens the continue-grace sequencing wait too, the one fixed sleep in this file with no daemon-observable substitute. --- .../daemon/__tests__/event-broadcast.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/kernel/daemon/__tests__/event-broadcast.test.ts b/src/kernel/daemon/__tests__/event-broadcast.test.ts index fc979ed0..c0b01788 100644 --- a/src/kernel/daemon/__tests__/event-broadcast.test.ts +++ b/src/kernel/daemon/__tests__/event-broadcast.test.ts @@ -352,7 +352,15 @@ describe('daemon → /ws/events downstream broadcast', () => { // skipped entirely once `suppressed` resolves true (bootstrap.ts's // 'hook.continue.request' case returns before reaching it). Waiting on // continueId would hang for the full 5s `until()` timeout every run. - await new Promise((r) => setTimeout(r, 150)); + // This is the one remaining fixed wait in this file. The margin is widened + // (150ms → 600ms) as the only mitigation available while the daemon + // exposes no grace-registration signal: too short and the suppression + // never arms, failing this test outright on the `toBeLessThan(4000)` below + // rather than merely flaking. 600ms still leaves ample room under that 4s + // budget. If the daemon ever exposes a grace-registration signal (e.g. a + // test-only callback off `bootstrapDaemon`/`DaemonHandle`), convert this to + // an `until()` on it instead. + await new Promise((r) => setTimeout(r, 600)); // user starts a new turn within the grace window → suppress the continue card await request({ kind: 'hook.event', event: { event: 'prompt', cwd: '/g', sessionId: 's', prompt: 'next' } }, { socketPath: sock, timeoutMs: 2000 }); const res = (await p) as { reply: string | null }; @@ -467,7 +475,13 @@ describe('daemon → /ws/events downstream broadcast', () => { // resolve A (deny) — must NOT wipe B's pending indicator await request({ kind: 'hook.permission.answer', requestId: reqA, approved: false }, { socketPath: sock, timeoutMs: 2000 }); expect(((await pA) as { decision: string }).decision).toBe('deny'); - await until(() => { expect(h.sessions.get('s')?.pending?.requestId).toBe(reqB); }); + // Invariant, not an arrival: `reqB` was read off the frame where pending had + // ALREADY moved to B (see the waitFor above), so this asserts that denying A + // did not wipe B's indicator. A condition that is already true needs real + // elapsed time to mean anything — until() would return on its first check and + // prove nothing. + await new Promise((r) => setTimeout(r, 200)); + expect(h.sessions.get('s')?.pending?.requestId).toBe(reqB); // cleanup await request({ kind: 'hook.permission.answer', requestId: reqB, approved: false }, { socketPath: sock, timeoutMs: 2000 }); await pB; From e01d9a9ce3ee15849c7887cfc2abba98a9f6e448 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 11:39:13 +0800 Subject: [PATCH 08/22] test(daemon): condition-based waits in bootstrap Nine arrival assertions, including two setTimeout(0) microtask flushes. The two remaining fixed sleeps are inside fake adapters simulating channel latency, not test waits. --- src/kernel/daemon/__tests__/bootstrap.test.ts | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/kernel/daemon/__tests__/bootstrap.test.ts b/src/kernel/daemon/__tests__/bootstrap.test.ts index 97c6505a..4e2dd073 100644 --- a/src/kernel/daemon/__tests__/bootstrap.test.ts +++ b/src/kernel/daemon/__tests__/bootstrap.test.ts @@ -6,6 +6,7 @@ import { bootstrapDaemon, shouldFastNullContinue, clampPermissionTimeout, makeCo import { request, daemonSocketPath } from '../../ipc/client'; import type { IMAdapter, IMChannel, OutgoingMessage, IncomingEnvelope } from '../../contracts/im-adapter'; import { SessionRegistry } from '../../web/session-registry'; +import { until } from '../../__tests__/wait.js'; // #45 — robustness helpers for this file's "held request" pattern // (const pending = request(...); …asserts…; await pending). On a slow/jittery @@ -177,7 +178,7 @@ describe('local-answer cancel + Stop fast-null (integration)', () => { { socketPath: sock, timeoutMs: 10_000 }, ); held(pending); - await new Promise((r) => setTimeout(r, 100)); // let the card go pending + await until(() => { expect(h.sessions.get('s1')?.pending).toBeDefined(); }); // let the card go pending await request( { kind: 'hook.event', event: { event: 'activity', cwd: '/w', sessionId: 's1', toolName: 'Bash', result: {} } }, { socketPath: sock, timeoutMs: 2000 }, @@ -344,8 +345,7 @@ describe('AskUserQuestion remote card (Task 9)', () => { { socketPath: sock, timeoutMs: 10_000 }, ); held(pending); - await new Promise((r) => setTimeout(r, 100)); - expect(notes).toHaveLength(1); // exactly once — not once per configured channel + await until(() => { expect(notes).toHaveLength(1); }); // exactly once — not once per configured channel expect(notes[0].title).toContain('Bash'); const card = sent[0] as { kind: 'card'; buttons?: Array<{ id: string; label: string }> }; adapter.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x1', text: card.buttons!.find((b) => b.id.startsWith('approve:'))!.id, ts: 0 }); @@ -372,9 +372,8 @@ describe('AskUserQuestion remote card (Task 9)', () => { { socketPath: sock, timeoutMs: 10_000 }, ); held(pending); - await new Promise((r) => setTimeout(r, 150)); + await until(() => { expect(notes).toHaveLength(1); }); expect(sent).toHaveLength(0); // …but the desktop already knows - expect(notes).toHaveLength(1); // Local answer within grace (PostToolUse cancel) → card never sent, toast cleared. await request( { kind: 'hook.event', event: { event: 'activity', cwd: '/w', sessionId: 's1', toolName: 'Bash', result: {} } }, @@ -405,9 +404,8 @@ describe('AskUserQuestion remote card (Task 9)', () => { { socketPath: sock, timeoutMs: 10_000 }, ); held(pending); - await new Promise((r) => setTimeout(r, 100)); + await until(() => { expect(sent).toHaveLength(1); }); expect(notes).toHaveLength(0); // toast gated off; the IM card still went out - expect(sent).toHaveLength(1); const card = sent[0] as { kind: 'card'; buttons?: Array<{ id: string; label: string }> }; adapter.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x1', text: card.buttons!.find((b) => b.id.startsWith('approve:'))!.id, ts: 0 }); await pending; @@ -437,8 +435,7 @@ describe('AskUserQuestion remote card (Task 9)', () => { { socketPath: sock, timeoutMs: 10_000 }, ); held(pending); - await new Promise((r) => setTimeout(r, 150)); - expect(sent).toHaveLength(2); // one card per channel… + await until(() => { expect(sent).toHaveLength(2); }); // one card per channel… expect(notes).toHaveLength(1); // …but a single desktop ping const card = sent[0] as { kind: 'card'; buttons?: Array<{ id: string; label: string }> }; tg.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x1', text: card.buttons!.find((b) => b.id.startsWith('approve:'))!.id, ts: 0 }); @@ -458,9 +455,8 @@ describe('AskUserQuestion remote card (Task 9)', () => { // and Codex turn/completed paths both funnel through). Floating request with // a tiny window; we only assert the notification surfaces, not the reply. void h.continueBroker.request({ cwd: '/w', context: 'Finished building the feature', timeoutSec: 1 }); - await new Promise((r) => setTimeout(r, 50)); + await until(() => { expect(sent).toHaveLength(1); }); // …still surfaced on IM expect(infos).toHaveLength(0); // no desktop flood on completion… - expect(sent).toHaveLength(1); // …still surfaced on IM expect((sent[0] as { title?: string }).title).toContain('Turn finished'); }); @@ -545,7 +541,7 @@ describe('AskUserQuestion remote card (Task 9)', () => { { socketPath: sock, timeoutMs: 10_000 }, ); held(pending); - await new Promise((r) => setTimeout(r, 100)); + await waitForSent(sent); // let the card go out before referencing its messageId const cardMsgId = 'm1'; // interactiveAdapter assigns sequential ids m1, m2, … adapter.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x2', text: 'use mv to the scratchpad instead', replyToMessageId: cardMsgId, ts: 0 }); const r = await pending as { decision?: string; message?: string }; @@ -645,15 +641,13 @@ describe('AskUserQuestion remote card (Task 9)', () => { // fix) — the edit lands on a later microtask, no longer synchronously // within fire(), so each assertion needs a tick to let the queue drain. adapter.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x1', text: toggleBlue.id, ts: 0 }); - await new Promise((r) => setTimeout(r, 0)); - expect(edits).toHaveLength(1); + await until(() => { expect(edits).toHaveLength(1); }); const edited1 = edits[0].msg as { kind: 'card'; buttons?: Array<{ id: string; label: string }> }; expect(edited1.buttons!.map((b) => b.label)).toEqual(['▢ Red', '▣ Blue', 'Submit (1)', 'Skip']); // toggling the same option again flips it back off adapter.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x1', text: toggleBlue.id, ts: 0 }); - await new Promise((r) => setTimeout(r, 0)); - expect(edits).toHaveLength(2); + await until(() => { expect(edits).toHaveLength(2); }); const edited2 = edits[1].msg as { kind: 'card'; buttons?: Array<{ id: string; label: string }> }; expect(edited2.buttons!.map((b) => b.label)).toEqual(['▢ Red', '▢ Blue', 'Submit (0)', 'Skip']); @@ -876,8 +870,7 @@ describe('quoting a live approval card = deny with guidance (Task 7)', () => { expect(r.decision).toBe('deny'); expect(r.message).toBe('Do not use rm -rf, move it to /tmp/.trash instead'); - await new Promise((res) => setTimeout(res, 50)); // let the queued settlement edit land - expect(edits).toHaveLength(1); + await until(() => { expect(edits).toHaveLength(1); }); // let the queued settlement edit land const editedTitle = (edits[0].msg as { title?: string }).title ?? ''; expect(editedTitle).toContain('Denied with guidance'); }); @@ -904,8 +897,7 @@ describe('quoting a live approval card = deny with guidance (Task 7)', () => { adapter.fire({ channel: 'telegram', chatId: 'c1', userId: 'u1', messageId: 'x1', text: denyBtn.id, ts: 0 }); await pending; - await new Promise((res) => setTimeout(res, 50)); - expect(edits).toHaveLength(1); + await until(() => { expect(edits).toHaveLength(1); }); const editedTitle = (edits[0].msg as { title?: string }).title ?? ''; expect(editedTitle).toContain('Denied'); expect(editedTitle).not.toContain('Denied with guidance'); From ff034c46d41bbb45273916efd2617211050721ab Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 11:48:33 +0800 Subject: [PATCH 09/22] ci: add install-smoke-windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows had unit tests and nothing else — the install path a Windows user actually walks (prebuild load, ConPTY spawn, .cmd shim, named-pipe daemon) had no coverage. That gap is why #59, a real Windows daemon crash, sat filed as test flake. Not added to branch protection yet; it needs a track record first. --- .github/scripts/smoke-pty.mjs | 31 +++++++++++++++++++++ .github/workflows/ci.yml | 52 +++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 .github/scripts/smoke-pty.mjs diff --git a/.github/scripts/smoke-pty.mjs b/.github/scripts/smoke-pty.mjs new file mode 100644 index 00000000..54213e5c --- /dev/null +++ b/.github/scripts/smoke-pty.mjs @@ -0,0 +1,31 @@ +// Loads node-pty from a globally installed tlive and spawns a real ConPTY. +// +// A prebuilt binary that resolves can still fail to load, and one that loads +// can still fail to spawn. Only a real install on a real Windows runner sees +// this; the unit-test job installs from the workspace with a full toolchain. + +import { createRequire } from 'node:module'; +import { join } from 'node:path'; + +const globalRoot = process.env.TLIVE_GLOBAL_ROOT; +if (!globalRoot) { + console.error('TLIVE_GLOBAL_ROOT is not set'); + process.exit(1); +} + +const require = createRequire(import.meta.url); +const ptyPath = require.resolve('node-pty', { paths: [join(globalRoot, 'tlive')] }); +const { spawn } = require(ptyPath); + +const p = spawn('cmd.exe', ['/c', 'echo pty-ok'], { cols: 80, rows: 24 }); +let out = ''; +p.onData((d) => { out += d; }); + +setTimeout(() => { + if (!out.includes('pty-ok')) { + console.error('pty produced no output; got: ' + JSON.stringify(out)); + process.exit(1); + } + console.log('pty loads and runs'); + process.exit(0); +}, 5000); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 57bdac28..141599df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,3 +109,55 @@ jobs: ' echo "::endgroup::" done + + # Windows counterpart to install-smoke, and the only end-to-end Windows + # coverage that exists. `test (windows-latest)` runs unit tests; nothing else + # touches what a Windows user hits on day one — the node-pty win32 prebuild + # actually loading, a ConPTY actually spawning, %APPDATA% paths, the .cmd shim + # npm writes for the bin, and the named-pipe daemon. + # + # The linux install-smoke earned its place by catching node-pty 1.1.0 shipping + # no linux prebuild, which no unit test could see. Same reasoning here: issue + # #59 was a real Windows daemon crash that spent months filed as test flake + # precisely because nothing exercised the product on Windows. + # + # Single job, no matrix — a matrix renames the check and required contexts are + # matched by exact name (see the note on install-smoke above). + install-smoke-windows: + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + - uses: pnpm/action-setup@v4 + with: + version: 11 + - uses: actions/setup-node@v4 + with: + node-version: '22' + cache: pnpm + - run: pnpm install --frozen-lockfile + - run: pnpm run build + - run: npm pack + - name: install the real tarball globally + shell: pwsh + run: | + $tgz = Get-ChildItem -Filter tlive-*.tgz | Select-Object -First 1 + npm i -g $tgz.FullName --no-audit --no-fund + - name: tlive resolves through the .cmd shim + shell: pwsh + run: tlive --version + - name: node-pty loads and spawns a ConPTY + shell: pwsh + run: | + $env:TLIVE_GLOBAL_ROOT = (npm root -g).Trim() + node .github/scripts/smoke-pty.mjs + - name: daemon start / status / stop over the named pipe + shell: pwsh + run: | + tlive start + tlive status + tlive stop + - name: stop the daemon even if a step above failed + if: always() + shell: pwsh + run: tlive stop + continue-on-error: true From 1674aa0e21e63d840627c24f98ffcf7cbcd4ff28 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 12:23:49 +0800 Subject: [PATCH 10/22] ci: make the Windows daemon step able to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pwsh does not halt on a native non-zero exit, and GitHub Actions reads $LASTEXITCODE only after the whole script — so the step's verdict came from `tlive stop` alone, which exits 0 when no daemon is running. A completely broken `tlive start` reported green. Gates each command and asserts status output. --- .github/workflows/ci.yml | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 141599df..5fa5ff10 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,14 +148,33 @@ jobs: - name: node-pty loads and spawns a ConPTY shell: pwsh run: | - $env:TLIVE_GLOBAL_ROOT = (npm root -g).Trim() + # `npm root -g` can in principle emit more than one line, which makes + # PowerShell 7+ treat the captured value as a string[] — .Trim() would + # then enumerate silently instead of throwing, and $env:TLIVE_GLOBAL_ROOT + # would end up joined with a space into a corrupted path. Out-String + # collapses it to a single string first. + $env:TLIVE_GLOBAL_ROOT = (npm root -g | Out-String).Trim() node .github/scripts/smoke-pty.mjs - name: daemon start / status / stop over the named pipe shell: pwsh run: | + # pwsh does not halt on a native (non-PowerShell) command's non-zero + # exit, and GitHub Actions only inspects $LASTEXITCODE once, after the + # whole script runs — so without explicit gates the step's verdict + # would come from `tlive stop` alone, which exits 0 even when nothing + # was running (stop is deliberately idempotent so `stop && start` + # chains work). Gate every command, and assert on `status`'s actual + # text rather than just its exit code: match the "running (pid ...)" + # line status.ts prints, not a bare "running" substring — "not + # running" also contains the word "running". tlive start - tlive status + if ($LASTEXITCODE -ne 0) { throw "tlive start failed ($LASTEXITCODE)" } + $s = tlive status | Out-String + if ($LASTEXITCODE -ne 0) { throw "tlive status failed ($LASTEXITCODE)" } + Write-Host $s + if ($s -notmatch 'daemon:\s+running \(pid') { throw "daemon not reported running:`n$s" } tlive stop + if ($LASTEXITCODE -ne 0) { throw "tlive stop failed ($LASTEXITCODE)" } - name: stop the daemon even if a step above failed if: always() shell: pwsh From 225872c00d06a580a2f04baf9185f36ba167bb87 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 12:32:03 +0800 Subject: [PATCH 11/22] test(pty): skip the silent-child activity test on win32 ConPTY emits initialisation sequences before any child output, so a pty that produces nothing does not exist on Windows and the assertion cannot mean what it says. Caught by the first real install-smoke-windows run. --- src/kernel/pty/__tests__/session-host.test.ts | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index 32c0924c..790ce4f2 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { createConnection } from 'node:net'; import { mkdtempSync } from 'node:fs'; import { join, basename } from 'node:path'; @@ -147,21 +147,42 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { sockPath, attachLocal: false, }); - // The snapshot path is only under test if EARLY-SCREEN reaches the shadow - // terminal BEFORE the client attaches — otherwise the live output carries it - // and the test passes without exercising the snapshot at all. onActivity now - // fires (true) only when the host observes actual pty output, so wait for that - // to order the output ahead of the client's attach. It must be registered before - // start() creates the timer. - const sawOutput = new Promise((resolve) => { - host.onActivity((active) => { if (active) resolve(); }); - }); await host.start(); - await sawOutput; + + // Ordering the snapshot path needs: EARLY-SCREEN must reach the shadow + // terminal BEFORE the real client attaches, otherwise the snapshot is empty, + // the live output carries the string anyway, and this test passes without + // touching the snapshot path at all. + // + // A throwaway client proves it: once EARLY-SCREEN has come back over the live + // stream, pty.onData has already fed it to the shadow. onActivity cannot be + // used for this — on Windows, ConPTY emits initialisation sequences before the + // child writes anything, so an activity flip says nothing about child output. + await new Promise((resolve, reject) => { + const t = setTimeout(() => reject(new Error('probe never saw EARLY-SCREEN')), 8000); + const pdec = new FrameDecoder(); + let acc = ''; + const probe = createConnection(sockPath, () => { probe.write(encodeAttach(80, 24)); }); + probe.on('error', reject); + probe.on('data', (chunk: Buffer) => { + for (const f of pdec.push(chunk)) { + if (f.type === FrameType.Data) { + acc += f.payload.toString('utf8'); + if (acc.includes('EARLY-SCREEN')) { + clearTimeout(t); + probe.end(); + // Let the socket fully close before the late joiner connects, ensuring + // the ordering guarantee is complete. + setTimeout(resolve, 100); + } + } + } + }); + }); const dec = new FrameDecoder(); const got = await new Promise((resolve, reject) => { - const t = setTimeout(() => reject(new Error('no snapshot')), 8000); + const t = setTimeout(() => reject(new Error('late joiner got no snapshot')), 8000); const chunks: Buffer[] = []; const sock = createConnection(sockPath, () => { sock.write(encodeAttach(80, 24)); }); sock.on('error', reject); @@ -212,7 +233,7 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { await host.stop(); }); - it('a silent child (no output) does not report as running', async () => { + it.skipIf(process.platform === 'win32')('a silent child (no output) does not report as running', async () => { const dir = mkdtempSync(join(tmpdir(), 'tlive-host-')); const sockPath = sessSock(dir, 's'); // child that consumes stdin but produces no output @@ -228,6 +249,10 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { // the first tick to ensure onActivity(true) has fired if the child // had emitted anything (it hasn't). This is an absence assertion — // a fixed delay proves that no true flip occurred. + // Note: ConPTY emits initialisation sequences before any child output, + // so a pty with no output does not exist on Windows and the assertion + // is therefore meaningless rather than merely flaky. This is why the test + // is skipped there. await new Promise((r) => setTimeout(r, 1200)); expect(flips).not.toContain(true); // never reported running await host.stop(); From bc2d504f0aeeb0bc0f246e42be71a58b496e58ea Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 13:11:49 +0800 Subject: [PATCH 12/22] test(pty): assert the snapshot frame itself, and drop a needless sleep The late joiner accumulated frames until EARLY-SCREEN appeared, which is satisfiable by live output as well as by the snapshot. onClient writes the Size frame then the snapshot as the first Data frame, synchronously, so the first Data frame can be asserted directly. Also removes most of the 100ms wait after the probe detaches: the shadow is written inside pty.onData before the broadcast, so it already holds the bytes the probe just received. A minimal 10ms delay remains to ensure the probe socket's close event is processed on the server side before the late joiner connects. --- src/kernel/pty/__tests__/session-host.test.ts | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index 790ce4f2..86611a02 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { createConnection } from 'node:net'; import { mkdtempSync } from 'node:fs'; import { join, basename } from 'node:path'; @@ -171,9 +171,9 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { if (acc.includes('EARLY-SCREEN')) { clearTimeout(t); probe.end(); - // Let the socket fully close before the late joiner connects, ensuring - // the ordering guarantee is complete. - setTimeout(resolve, 100); + // Small delay to ensure the probe socket is fully torn down on the + // server side before the late joiner tries to connect. + setTimeout(resolve, 10); } } } @@ -183,15 +183,20 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { const dec = new FrameDecoder(); const got = await new Promise((resolve, reject) => { const t = setTimeout(() => reject(new Error('late joiner got no snapshot')), 8000); - const chunks: Buffer[] = []; + let firstDataPayload = ''; + let gotFirstData = false; const sock = createConnection(sockPath, () => { sock.write(encodeAttach(80, 24)); }); sock.on('error', reject); sock.on('data', (chunk: Buffer) => { for (const f of dec.push(chunk)) { - if (f.type === FrameType.Data) { - chunks.push(f.payload); - const s = Buffer.concat(chunks).toString('utf8'); - if (s.includes('EARLY-SCREEN')) { clearTimeout(t); sock.end(); resolve(s); } + if (f.type === FrameType.Data && !gotFirstData) { + // The first Data frame on a first attach is the snapshot (serializer.serialize). + // Live output (from pty.onData) comes after. + gotFirstData = true; + firstDataPayload = f.payload.toString('utf8'); + clearTimeout(t); + sock.end(); + resolve(firstDataPayload); } } }); From f39d14fc6405373590ba21b257a8f683fb4b5cae Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 13:24:49 +0800 Subject: [PATCH 13/22] test(pty): wait for the probe socket to close, not a fixed delay The probe teardown was guarded by an arbitrary sleep. Waiting on the socket's own close event removes the magic number: the client's 'close' event signals that the socket is torn down on this end, and a small follow-up delay (20ms) ensures the server's removeClient and applySize have completed before the late joiner attaches. --- src/kernel/pty/__tests__/session-host.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index 86611a02..e5e9decb 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -162,6 +162,7 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { const t = setTimeout(() => reject(new Error('probe never saw EARLY-SCREEN')), 8000); const pdec = new FrameDecoder(); let acc = ''; + let resolved = false; const probe = createConnection(sockPath, () => { probe.write(encodeAttach(80, 24)); }); probe.on('error', reject); probe.on('data', (chunk: Buffer) => { @@ -171,9 +172,12 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { if (acc.includes('EARLY-SCREEN')) { clearTimeout(t); probe.end(); - // Small delay to ensure the probe socket is fully torn down on the - // server side before the late joiner tries to connect. - setTimeout(resolve, 10); + // The client-side 'close' event fires when the socket is closed on this + // end, but the server still needs to process the close, call removeClient, + // and complete applySize. A small additional delay covers that window. + probe.on('close', () => { + if (!resolved) { resolved = true; setTimeout(resolve, 20); } + }); } } } From 85150c74a5f6a228422a228702d4e7a74450b3a5 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 13:40:07 +0800 Subject: [PATCH 14/22] test(pty): wait on the shadow terminal, not on a live client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The late-joiner snapshot test guarded its precondition with a sleep. The precondition is that EARLY-SCREEN has reached the shadow terminal before the client under test attaches; without it the snapshot is empty, the live stream carries the string anyway, and the test passes without touching the snapshot path. Watching a probe client for EARLY-SCREEN on the live stream does not establish that. pty.onData broadcasts to sockets synchronously, but xterm's write buffer parses on a later macrotask (WriteBuffer.write schedules _innerWrite via setTimeout), so serialize() still returns '' while those bytes are already on the wire. onClient then skips the Data frame entirely (snap.length > 0), the joiner receives only a Size frame, and the test times out — 5/25 full-file runs on Linux. The sleep was covering the write-buffer latency, not socket teardown: the probe was already out of the clients set by then. Wait on the shadow's own content with `until` instead, and drop the probe's detach: a late joiner is one that attaches after the output happened, not the only client, and every client gets its own snapshot on its own first attach. No delays left in the test. 35/35 full-file runs green; deleting the snapshot write still fails it 1/1. --- src/kernel/pty/__tests__/session-host.test.ts | 58 ++++++++----------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index e5e9decb..21fbfafc 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -10,6 +10,7 @@ import { SessionHost, authoritativeSize } from '../session-host'; const sessSock = (dir: string, name: string): string => process.platform === 'win32' ? `\\\\.\\pipe\\tlive-t-${basename(dir)}-${name}` : join(dir, `${name}.sock`); import { FrameDecoder, FrameType, encodeAttach, encodeData, parseDims } from '../../web/stream-protocol.js'; +import { until } from '../../__tests__/wait.js'; describe('authoritativeSize', () => { it('defaults to 80x24 with no sources', () => { @@ -149,40 +150,30 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { }); await host.start(); - // Ordering the snapshot path needs: EARLY-SCREEN must reach the shadow - // terminal BEFORE the real client attaches, otherwise the snapshot is empty, - // the live output carries the string anyway, and this test passes without - // touching the snapshot path at all. + // A client that is already watching the session. It stays attached for the rest + // of the test: "late joiner" means "attached after the output happened", not + // "the only client" — every client gets its own snapshot on its own first attach, + // and the web terminal really does have tabs joining a session others are in. + // The error handler goes on before the first write; see the comment in the + // sizing test above for why. + const probe = createConnection(sockPath); + probe.on('error', () => { /* teardown race, not a test failure */ }); + await new Promise((r) => probe.once('connect', () => r())); + probe.write(encodeAttach(80, 24)); + + // The precondition the snapshot path needs: EARLY-SCREEN must have reached the + // shadow terminal BEFORE the client under test attaches. Otherwise the snapshot + // is empty, the live stream carries the string anyway, and this test passes + // without touching the snapshot path at all. // - // A throwaway client proves it: once EARLY-SCREEN has come back over the live - // stream, pty.onData has already fed it to the shadow. onActivity cannot be - // used for this — on Windows, ConPTY emits initialisation sequences before the - // child writes anything, so an activity flip says nothing about child output. - await new Promise((resolve, reject) => { - const t = setTimeout(() => reject(new Error('probe never saw EARLY-SCREEN')), 8000); - const pdec = new FrameDecoder(); - let acc = ''; - let resolved = false; - const probe = createConnection(sockPath, () => { probe.write(encodeAttach(80, 24)); }); - probe.on('error', reject); - probe.on('data', (chunk: Buffer) => { - for (const f of pdec.push(chunk)) { - if (f.type === FrameType.Data) { - acc += f.payload.toString('utf8'); - if (acc.includes('EARLY-SCREEN')) { - clearTimeout(t); - probe.end(); - // The client-side 'close' event fires when the socket is closed on this - // end, but the server still needs to process the close, call removeClient, - // and complete applySize. A small additional delay covers that window. - probe.on('close', () => { - if (!resolved) { resolved = true; setTimeout(resolve, 20); } - }); - } - } - } - }); - }); + // Seeing EARLY-SCREEN arrive on a live client does NOT establish that: pty.onData + // broadcasts to sockets synchronously, but xterm's write buffer parses on a later + // macrotask, so serialize() can still return '' while those bytes are already on + // the wire. Wait on the shadow's own content instead — the thing the snapshot is + // built from. (onActivity is no use either: on Windows ConPTY emits initialisation + // sequences before the child writes anything, so a flip says nothing about output.) + const shadow = host as unknown as { serializer: { serialize(): string } | null }; + await until(() => { expect(shadow.serializer?.serialize() ?? '').toContain('EARLY-SCREEN'); }); const dec = new FrameDecoder(); const got = await new Promise((resolve, reject) => { @@ -206,6 +197,7 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { }); }); expect(got).toContain('EARLY-SCREEN'); + probe.end(); await host.stop(); }); From 6446aa3b77ebe07e7693594fec8c20cdf201511c Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 13:51:40 +0800 Subject: [PATCH 15/22] docs(pty): link the upstream node-pty issue from the conin guard --- src/kernel/pty/win-conin-guard.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/kernel/pty/win-conin-guard.ts b/src/kernel/pty/win-conin-guard.ts index faf87181..a1c5c967 100644 --- a/src/kernel/pty/win-conin-guard.ts +++ b/src/kernel/pty/win-conin-guard.ts @@ -11,8 +11,9 @@ // // Reaching through the private _agent is deliberate — Terminal.on() forwards to // the conout socket, so there is no public route to conin. Verified against -// node-pty@1.2.0-beta.14. Delete this once an upstream release carries the -// listener; re-check the internal shape on every node-pty bump. +// node-pty@1.2.0-beta.14. Reported upstream as +// https://github.com/microsoft/node-pty/issues/942 — delete this once a release +// carries the listener; re-check the internal shape on every node-pty bump. import type { IPty } from 'node-pty'; From 2d896b997b1863a27aed0c974333dc4eb52c7628 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:17:53 +0800 Subject: [PATCH 16/22] test(daemon): stop implying the continue-grace sleep is the only fixed wait left The comment claimed this was 'the one remaining fixed wait in this file,' which reads as every other wait already being converted. It isn't: four other fixed intervals in this file are deliberate absence assertions (dropped-deny, pending, status, droppable invariants), not leftovers. As written, a future 'clean up the last magic sleep' pass would find those four and convert them too, silently gutting four absence checks. Reword to match what ec13922's commit body already said correctly: this is the one fixed sleep that has no daemon-observable substitute. --- src/kernel/daemon/__tests__/event-broadcast.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/kernel/daemon/__tests__/event-broadcast.test.ts b/src/kernel/daemon/__tests__/event-broadcast.test.ts index c0b01788..789ec87a 100644 --- a/src/kernel/daemon/__tests__/event-broadcast.test.ts +++ b/src/kernel/daemon/__tests__/event-broadcast.test.ts @@ -352,7 +352,9 @@ describe('daemon → /ws/events downstream broadcast', () => { // skipped entirely once `suppressed` resolves true (bootstrap.ts's // 'hook.continue.request' case returns before reaching it). Waiting on // continueId would hang for the full 5s `until()` timeout every run. - // This is the one remaining fixed wait in this file. The margin is widened + // This is the one fixed sleep in this file that has no daemon-observable + // substitute (the other fixed waits elsewhere in this file are deliberate + // absence assertions, not stand-ins for this). The margin is widened // (150ms → 600ms) as the only mitigation available while the daemon // exposes no grace-registration signal: too short and the suppression // never arms, failing this test outright on the `toBeLessThan(4000)` below From 97e52d78da8a1be3e4de0638744114849cc07ea8 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:17:58 +0800 Subject: [PATCH 17/22] ci(smoke): resolve pty-ok on arrival, keep 5s as a failure deadline only The assertion sat behind an unconditional setTimeout(...,5000): every run burned the full 5s even on success, and a ConPTY answering at 5001ms would fail the step. Same pattern this branch removes from the test suite, just left standing in the new CI script. Check for 'pty-ok' inside onData and resolve immediately; the 5s timer is now only a failure deadline, guarded so it's a no-op once onData has already won. Same safety, no fixed margin on the success path. --- .github/scripts/smoke-pty.mjs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/.github/scripts/smoke-pty.mjs b/.github/scripts/smoke-pty.mjs index 54213e5c..a143a481 100644 --- a/.github/scripts/smoke-pty.mjs +++ b/.github/scripts/smoke-pty.mjs @@ -19,13 +19,24 @@ const { spawn } = require(ptyPath); const p = spawn('cmd.exe', ['/c', 'echo pty-ok'], { cols: 80, rows: 24 }); let out = ''; -p.onData((d) => { out += d; }); +let done = false; -setTimeout(() => { - if (!out.includes('pty-ok')) { - console.error('pty produced no output; got: ' + JSON.stringify(out)); - process.exit(1); - } - console.log('pty loads and runs'); - process.exit(0); +// 5s is a failure deadline, not a fixed wait: resolve as soon as the output +// arrives so a passing run doesn't burn 5s of runner time, while a ConPTY +// that never answers still fails instead of hanging the job. +const deadline = setTimeout(() => { + if (done) return; + done = true; + console.error('pty produced no output; got: ' + JSON.stringify(out)); + process.exit(1); }, 5000); + +p.onData((d) => { + out += d; + if (!done && out.includes('pty-ok')) { + done = true; + clearTimeout(deadline); + console.log('pty loads and runs'); + process.exit(0); + } +}); From 46cfcbd2f065be3d13918d34ddef9532de1ad9fa Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:18:02 +0800 Subject: [PATCH 18/22] docs(pty): point the conin-guard comment at the shipped lib/*.js paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit node-pty 1.2.0-beta.14 ships lib/*.js (plus native C++ src/), not the TypeScript sources the comment cited (windowsPtyAgent.ts / windowsTerminal.ts) — those never make it into the published package. Line numbers were already correct and do land on the referenced constructs; only the extensions/dirs were wrong. This comment is the only map to a private-internals reach, so it needs to stay navigable. --- src/kernel/pty/win-conin-guard.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/kernel/pty/win-conin-guard.ts b/src/kernel/pty/win-conin-guard.ts index a1c5c967..3eb9c100 100644 --- a/src/kernel/pty/win-conin-guard.ts +++ b/src/kernel/pty/win-conin-guard.ts @@ -2,12 +2,12 @@ // Containment for an upstream node-pty defect on Windows. // // node-pty opens two pipes to the ConPTY. The read side (conout) gets an -// 'error' listener in windowsTerminal.ts:90; the write side (conin) is -// constructed in windowsPtyAgent.ts:79 with none. A net.Socket without an -// 'error' listener turns any emitted error into an uncaught exception, so a +// 'error' listener in lib/windowsTerminal.js:90; the write side (conin) is +// constructed in lib/windowsPtyAgent.js:79-84 with none. A net.Socket without +// an 'error' listener turns any emitted error into an uncaught exception, so a // failing pty write takes the whole daemon down. Two triggers reach it: EAGAIN // when conin's buffer fills under backpressure, and a write racing kill()'s -// _inSocket.destroy() (windowsPtyAgent.ts:176). +// _inSocket.destroy() (lib/windowsPtyAgent.js:176). // // Reaching through the private _agent is deliberate — Terminal.on() forwards to // the conout socket, so there is no public route to conin. Verified against From a5ace36fc935e5daeecb482f6ca7036bd0eea31d Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:18:17 +0800 Subject: [PATCH 19/22] test(pty): assert guardWindowsConinSocket is wired into SessionHost.start() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit win-conin-guard.test.ts covers guardWindowsConinSocket in isolation, but deleting its call at session-host.ts:90 leaves the whole suite green — on Linux the guard is a no-op by design, so no black-box test can see it. That is the one place dropping the fix for #59 would go unnoticed. Close it platform-independently with vi.mock('../win-conin-guard.js') and assert the call count increases by exactly one across start(). Placed here rather than in win-conin-guard.test.ts because that file unit-tests the real implementation directly; auto-mocking the module there would neuter those tests. Nothing else in this file asserts on the guard, so mocking it file-wide doesn't change any other test's behavior. --- src/kernel/pty/__tests__/session-host.test.ts | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index 21fbfafc..225d7faa 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -1,10 +1,18 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { createConnection } from 'node:net'; import { mkdtempSync } from 'node:fs'; import { join, basename } from 'node:path'; import { tmpdir } from 'node:os'; import { SessionHost, authoritativeSize } from '../session-host'; +// Auto-mocked for the wiring test below: on Linux guardWindowsConinSocket is +// a no-op by design (see win-conin-guard.ts), so no black-box test can see +// whether session-host.ts still calls it. Every other test in this file +// spawns real ptys and never asserts on the guard, so replacing it with a +// no-op spy for the whole file doesn't change their behavior. +vi.mock('../win-conin-guard.js'); +import { guardWindowsConinSocket } from '../win-conin-guard.js'; + // Per-session socket: fs path on POSIX (directly in `dir`, no subdir), named // pipe on win32. basename(dir) is unique per mkdtemp → collision-free pipe. const sessSock = (dir: string, name: string): string => @@ -306,4 +314,24 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { expect((host as unknown as { pty: unknown }).pty).toBeNull(); }); + // Regression guard for #59: guardWindowsConinSocket() is a no-op on Linux + // by design, so nothing black-box can tell whether start() still calls it — + // dropping the call here would leave the whole suite green. Assert the + // wiring directly via the mocked import instead. + it('wires guardWindowsConinSocket into start()', async () => { + const dir = mkdtempSync(join(tmpdir(), 'tlive-guard-')); + const host = new SessionHost({ + id: 'guard-1', cmd: process.execPath, args: ['-e', 'setInterval(()=>{},1000)'], + cwd: dir, sockPath: sessSock(dir, 'g'), attachLocal: false, + }); + // Captured immediately before the action under test, not asserted as an + // absolute count: other tests in this file also call start() against the + // same auto-mocked import, so only the delta from this one call is + // meaningful. + const before = vi.mocked(guardWindowsConinSocket).mock.calls.length; + await host.start(); + expect(vi.mocked(guardWindowsConinSocket).mock.calls.length).toBe(before + 1); + await host.stop(); + }); + }); From f7c9093c7189f6cc8f31306c70f911057168ed97 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:18:26 +0800 Subject: [PATCH 20/22] test(pty): rename the misnamed shadow local to internals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local was typed { serializer: ... }, not a shadow terminal — SessionHost has separate shadow and serializer private fields, so the old name was actively misleading next to the real thing it was casting past. --- src/kernel/pty/__tests__/session-host.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index 225d7faa..b4bfdd6a 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -180,8 +180,8 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { // the wire. Wait on the shadow's own content instead — the thing the snapshot is // built from. (onActivity is no use either: on Windows ConPTY emits initialisation // sequences before the child writes anything, so a flip says nothing about output.) - const shadow = host as unknown as { serializer: { serialize(): string } | null }; - await until(() => { expect(shadow.serializer?.serialize() ?? '').toContain('EARLY-SCREEN'); }); + const internals = host as unknown as { serializer: { serialize(): string } | null }; + await until(() => { expect(internals.serializer?.serialize() ?? '').toContain('EARLY-SCREEN'); }); const dec = new FrameDecoder(); const got = await new Promise((resolve, reject) => { From 5afc61ec6ff71ce45867a878170150b5779d07fc Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:18:38 +0800 Subject: [PATCH 21/22] test(pty): drop two more fixed sleeps in the activity-flip tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'reports running on output then idle after silence' asserted flips[0] === true after a fixed 3200ms sleep. That used to be guaranteed by construction, but after this branch's product fix it depends on the child's first byte beating the poll tick at spawn+~1000ms — a wide margin, but now a timing dependency, in the one branch that should not leave those behind. Replaced with two until() waits on containment instead of index/fixed-delay. 'a silent child (no output) does not report as running' waited a fixed 1200ms against a 1000ms poll tick (~200ms margin), and an empty flips would have passed vacuously. Replaced with until() on the false flip (a genuine, non-vacuous arrival — the first tick always reports idle) followed by the not.toContain(true) check, which is now evaluated against state that can no longer change because this test's child never writes. Not an absence-assertion violation: the arrival gives until() something real to resolve on, and the absence check only holds static state afterward. it.skipIf(win32) and its ConPTY explanation are unchanged. --- src/kernel/pty/__tests__/session-host.test.ts | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index b4bfdd6a..a6eca6a0 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -254,16 +254,18 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { const flips: boolean[] = []; host.onActivity((a) => flips.push(a)); await host.start(); - // Poll interval is 1s, first tick at ~1000ms. Give it a bit past - // the first tick to ensure onActivity(true) has fired if the child - // had emitted anything (it hasn't). This is an absence assertion — - // a fixed delay proves that no true flip occurred. + // Not an absence-assertion violation: the `false` flip below is a genuine + // arrival (the first poll tick, with no output yet, always reports idle), + // so until() has something real to wait on. The `not.toContain(true)` + // check afterwards is then made against state that can no longer change, + // because this test's child never writes — there is no later tick that + // could still flip it to running. // Note: ConPTY emits initialisation sequences before any child output, // so a pty with no output does not exist on Windows and the assertion // is therefore meaningless rather than merely flaky. This is why the test // is skipped there. - await new Promise((r) => setTimeout(r, 1200)); - expect(flips).not.toContain(true); // never reported running + await until(() => { expect(flips).toContain(false); }); // a tick demonstrably ran… + expect(flips).not.toContain(true); // …and it reported idle await host.stop(); }); @@ -279,10 +281,11 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { const flips: boolean[] = []; host.onActivity((a) => flips.push(a)); await host.start(); - // within ~1s: running (true); after IDLE_MS(1.5s)+poll: idle (false) - await new Promise((r) => setTimeout(r, 3200)); - expect(flips[0]).toBe(true); // saw output → running - expect(flips).toContain(false); // then went idle + // Both are arrivals — wait on each instead of a fixed sleep. (flips[0] is + // no longer guaranteed to be `true` by construction: it now races the + // child's first byte against the poll tick at spawn+~1000ms.) + await until(() => { expect(flips).toContain(true); }); + await until(() => { expect(flips).toContain(false); }); await host.stop(); }); From 83eb81ffb19d74561d913831d04a098591f2f659 Mon Sep 17 00:00:00 2001 From: y49 Date: Tue, 28 Jul 2026 14:34:16 +0800 Subject: [PATCH 22/22] test(pty): keep the real conin guard active while asserting it is wired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A file-global vi.mock replaced the guard with a no-op for every test in session-host.test.ts, including the ones that spawn real ptys and drive real writes and kills — the exact place a Windows conin failure would need swallowing. spy: true keeps the implementation and still lets the wiring be asserted. --- src/kernel/pty/__tests__/session-host.test.ts | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/kernel/pty/__tests__/session-host.test.ts b/src/kernel/pty/__tests__/session-host.test.ts index a6eca6a0..dd449384 100644 --- a/src/kernel/pty/__tests__/session-host.test.ts +++ b/src/kernel/pty/__tests__/session-host.test.ts @@ -1,16 +1,20 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, afterAll } from 'vitest'; import { createConnection } from 'node:net'; import { mkdtempSync } from 'node:fs'; import { join, basename } from 'node:path'; import { tmpdir } from 'node:os'; import { SessionHost, authoritativeSize } from '../session-host'; -// Auto-mocked for the wiring test below: on Linux guardWindowsConinSocket is -// a no-op by design (see win-conin-guard.ts), so no black-box test can see -// whether session-host.ts still calls it. Every other test in this file -// spawns real ptys and never asserts on the guard, so replacing it with a -// no-op spy for the whole file doesn't change their behavior. -vi.mock('../win-conin-guard.js'); +// Spied (NOT auto-mocked) for the wiring test below: on Linux +// guardWindowsConinSocket is a no-op by design (see win-conin-guard.ts), so +// no black-box test can see whether session-host.ts still calls it. `vi.mock` +// is file-global and hoisted, so a plain automock would replace the guard +// with a no-op for every OTHER test in this file too — including the ones +// that spawn real ptys and drive real writes and kills, which is exactly +// where a real Windows conin write failure would need swallowing. `spy: true` +// keeps the real implementation running for all of them and only adds +// call/return tracking on top. +vi.mock('../win-conin-guard.js', { spy: true }); import { guardWindowsConinSocket } from '../win-conin-guard.js'; // Per-session socket: fs path on POSIX (directly in `dir`, no subdir), named @@ -320,7 +324,7 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { // Regression guard for #59: guardWindowsConinSocket() is a no-op on Linux // by design, so nothing black-box can tell whether start() still calls it — // dropping the call here would leave the whole suite green. Assert the - // wiring directly via the mocked import instead. + // wiring directly via the spied import instead. it('wires guardWindowsConinSocket into start()', async () => { const dir = mkdtempSync(join(tmpdir(), 'tlive-guard-')); const host = new SessionHost({ @@ -329,12 +333,31 @@ describe('SessionHost (socket-only, attachLocal:false)', () => { }); // Captured immediately before the action under test, not asserted as an // absolute count: other tests in this file also call start() against the - // same auto-mocked import, so only the delta from this one call is - // meaningful. + // same spied import, so only the delta from this one call is meaningful. const before = vi.mocked(guardWindowsConinSocket).mock.calls.length; await host.start(); expect(vi.mocked(guardWindowsConinSocket).mock.calls.length).toBe(before + 1); + // `spy: true` runs the REAL body, not a stub, proven by the return value: + // an automocked function would return undefined, but the real + // implementation returns a platform-dependent boolean (false off win32, + // true on win32 once it finds a live conin socket to attach to). + expect(vi.mocked(guardWindowsConinSocket).mock.results.at(-1)?.value).toBe(process.platform === 'win32'); await host.stop(); }); + // Confirms `spy: true` above is genuinely running the guard's own body for + // EVERY test in this file, not just the one above — an automocked stub + // would return undefined for all of them, whereas the real implementation + // always returns a platform-dependent boolean. If this ever fails, the + // guard has silently gone back to being a no-op across the whole file, + // which is the exact regression this test exists to catch. + afterAll(() => { + const results = vi.mocked(guardWindowsConinSocket).mock.results; + expect(results.length).toBeGreaterThan(1); // every SessionHost.start() above called it too + for (const r of results) { + expect(r.type).toBe('return'); + expect(r.value).toBe(process.platform === 'win32'); + } + }); + });