diff --git a/.ai/contexts/trigger-watcher.md b/.ai/contexts/trigger-watcher.md index d2c63b4a..8dc8d121 100644 --- a/.ai/contexts/trigger-watcher.md +++ b/.ai/contexts/trigger-watcher.md @@ -24,7 +24,7 @@ require('./trigger-watcher').start(createTriggerContext({ activeSessions, log }) // trigger-context.js builds the ctx: { log, // electron-log compatible - getPtyForSession(sessionId), // → { ptyProcess } | null + getPtyForSession(sessionId), // → { ptyProcess, cwd, handle } | null isSessionBusy(sessionId), // → boolean getComposerState(sessionId), // → { pending, lastInputAt } | null isPtyAlive(ptyProcess), // optional; only present when supplied @@ -39,6 +39,37 @@ user typed and has not submitted (`composer-state.js`, fed from `null` for an unknown or exited session, and **a `null` — or an absent `getComposerState` — means busy, never free**. +### Session handle (2026-09-08, issue #220) + +`trigger-watcher.js` never touches `session.pty` or a raw pid. It writes and +probes liveness through a `handle` — `{ write(data), isAlive() }` — that +`getPtyForSession` attaches to the returned entry: + +- `activeSessions` entries now carry `host` (`null` for a local `node-pty` + session; a non-null hostname would mark a session whose process lives + elsewhere) and `kind` (`'local-pty'` today; descriptive metadata, not yet + read by anything). +- `trigger-context.js`'s `createLocalSessionHandle(ptyProcess)` builds the + local handle: `write` calls `ptyProcess.write`, `isAlive` is the same + signal-0 probe (`process.kill(pid, 0)`, `EPERM` counts as alive) that used + to live inline in `trigger-watcher.js` as `defaultIsPtyAlive`. `getPtyForSession` + uses it whenever `session.host == null`; a non-null host would instead take + `session.handle` as given — nothing currently sets that, since no remote + session type exists yet. +- `trigger-watcher.js` deduces the same local handle itself + (`resolveHandle(entry)`, wrapping `entry.ptyProcess`) whenever a ctx doesn't + supply `entry.handle` — this keeps every ctx implementation that predates + this change (all of `test/trigger-watcher.test.js`'s hand-built ctx objects) + working unmodified, since they still only shape `{ ptyProcess, cwd }`. +- `ctx.isPtyAlive`, when supplied, still overrides the handle's own + `isAlive()` entirely (same as it overrode `defaultIsPtyAlive` before) — + tests use this to simulate death without a real dying process. + +This is a seam, not a remote implementation: nothing sets `host` to anything +but `null`, and nothing constructs a non-local handle in production. It only +makes the write/liveness paths a property of the entry instead of an +assumption baked into `trigger-watcher.js`. + ## The submission contract The transport honours `conventions/session-trigger-transport.md` in the harness diff --git a/main.js b/main.js index 4435c220..9c43a176 100644 --- a/main.js +++ b/main.js @@ -2138,6 +2138,8 @@ ipcMain.handle('open-terminal', async (_event, sessionId, projectPath, isNew, se mcpServer, _openedAt: Date.now(), // see docs/automation.md — the trigger watcher's politeness guard composerState: createComposerState(), + // see .ai/contexts/trigger-watcher.md, "Session handle" + host: null, kind: 'local-pty', }; activeSessions.set(sessionId, session); diff --git a/test/trigger-context.test.js b/test/trigger-context.test.js index 1a124c5b..3b8eaf9f 100644 --- a/test/trigger-context.test.js +++ b/test/trigger-context.test.js @@ -4,8 +4,9 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const { createTriggerContext } = require('../trigger-context'); +const { createTriggerContext, createLocalSessionHandle } = require('../trigger-context'); const { createComposerState, noteUserInput } = require('../composer-state'); +const { spawnSync } = require('node:child_process'); const silentLog = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; @@ -77,6 +78,54 @@ test('isSessionBusy reads _cliBusy and is false for an unknown session', () => { assert.equal(ctx.isSessionBusy('nope'), false); }); +test('getPtyForSession attaches a handle: local (host null) writes to session.pty', () => { + const written = []; + const session = makeSession({ pty: { pid: process.pid, write: (d) => written.push(d) } }); + const ctx = ctxWith([['s1', session]]); + + const entry = ctx.getPtyForSession('s1'); + entry.handle.write('hello'); + assert.deepEqual(written, ['hello'], 'the local handle must write into session.pty'); + assert.equal(entry.handle.isAlive(), true, "the local handle probes session.pty's real pid"); +}); + +test('getPtyForSession: a non-null host takes session.handle as given, not session.pty', () => { + const written = []; + const session = makeSession({ + host: 'some-remote-host', + pty: undefined, // deliberately no node-pty on this entry + handle: { write: (d) => written.push(d), isAlive: () => true }, + }); + const ctx = ctxWith([['s1', session]]); + + const entry = ctx.getPtyForSession('s1'); + assert.equal(entry.handle, session.handle, 'a non-local entry must use the supplied handle unchanged'); + entry.handle.write('x'); + assert.deepEqual(written, ['x']); +}); + +test('createLocalSessionHandle.write forwards verbatim to the underlying pty', () => { + const written = []; + const handle = createLocalSessionHandle({ pid: process.pid, write: (d) => written.push(d) }); + handle.write('abc'); + handle.write('\r'); + assert.deepEqual(written, ['abc', '\r']); +}); + +test('createLocalSessionHandle.isAlive reflects the real process, not just an override', () => { + const aliveHandle = createLocalSessionHandle({ pid: process.pid, write() {} }); + assert.equal(aliveHandle.isAlive(), true, 'the current test process must read as alive'); + + // The only test in this suite (or trigger-watcher's) exercising the FALSE + // branch of the real signal-0 probe with a genuinely dead pid -- every + // trigger-watcher test overrides ctx.isPtyAlive instead, so this is the + // one place mutating this probe to always return true is observable. + const child = spawnSync(process.platform === 'win32' ? 'cmd' : 'true', + process.platform === 'win32' ? ['/c', 'exit', '0'] : []); + const deadHandle = createLocalSessionHandle({ pid: child.pid, write() {} }); + assert.equal(deadHandle.isAlive(), false, 'a pid whose process has already exited must read as not alive'); +}); + test('log is forwarded, and isPtyAlive is only present when supplied', () => { const plain = createTriggerContext({ activeSessions: new Map(), log: silentLog }); assert.equal(plain.log, silentLog); diff --git a/test/trigger-watcher.test.js b/test/trigger-watcher.test.js index a9835177..670678dc 100644 --- a/test/trigger-watcher.test.js +++ b/test/trigger-watcher.test.js @@ -5710,3 +5710,62 @@ test('steps_total: a failure path that never sent anything still carries the fie cleanup(tmp); } }); + +// ── Session handle seam (issue #220) ───────────────────────────────────────── +// +// Unlike every other ctx in this file, this one is NOT hand-built: it goes +// through the real createTriggerContext, against a real activeSessions Map, +// with an entry that carries a fully test-supplied handle and no `pty` field +// at all -- nothing node-pty-shaped exists anywhere in this entry. This is +// the proof the seam is real: the injection path must reach this session +// without ever assuming a node-pty. See .ai/contexts/trigger-watcher.md, +// "Session handle". +test('session handle seam: an entry with only a fake handle (no node-pty) is pilotable by the injection path', async () => { + const tmp = mkTmp(); + let watcher; + try { + process.env.SWITCHBOARD_TRIGGERS_DIR = tmp; + process.env.SWITCHBOARD_TRIGGER_IDLE_TIMEOUT_MS = '200'; + + const { createTriggerContext } = require('../trigger-context'); + const { start } = require('../trigger-watcher'); + + const SESSION_ID = 'sess-fake-handle-' + Date.now(); + const written = []; + const activeSessions = new Map([[SESSION_ID, { + exited: false, + _cliBusy: false, + composerState: { pending: 0, lastInputAt: 0 }, + // Non-null host: getPtyForSession must take this entry's handle as + // given rather than deducing one from a `pty` field -- there is none. + host: 'fake-test-host', + kind: 'fake-test', + handle: { + write(data) { written.push(data); }, + isAlive() { return true; }, + }, + }]]); + + const ctx = createTriggerContext({ activeSessions, log: silentLog }); + watcher = start(ctx); + + const uuid = 'fake-handle-' + Date.now(); + writeTrigger(tmp, uuid, { sessionId: SESSION_ID, command: '/help', wait: 'none' }); + + const resultPath = path.join(tmp, 'processed', uuid + '.result.json'); + await waitForFile(resultPath, 2000); + + const result = readResult(path.join(tmp, 'processed'), uuid); + assert.equal(result.ok, true, 'a session carrying only a fake handle must still be drivable'); + // Busy never rises on this fake handle, so submitWithVerify retries the + // Enter once (same pattern as the "W7 default helper" test above). + assert.deepEqual(written, ['/help', '\r', '\r'], + 'the command and its Enter(s) must land in the fake handle, never a node-pty'); + + } finally { + if (watcher) watcher.close(); + delete process.env.SWITCHBOARD_TRIGGERS_DIR; + delete process.env.SWITCHBOARD_TRIGGER_IDLE_TIMEOUT_MS; + cleanup(tmp); + } +}); diff --git a/trigger-context.js b/trigger-context.js index 2c91235e..d083c8ed 100644 --- a/trigger-context.js +++ b/trigger-context.js @@ -1,6 +1,22 @@ // trigger-context.js — see .ai/contexts/trigger-watcher.md 'use strict'; +// Local session handle — see .ai/contexts/trigger-watcher.md, "Session handle". +function createLocalSessionHandle(ptyProcess) { + return { + write(data) { ptyProcess.write(data); }, + isAlive() { + if (!ptyProcess || typeof ptyProcess.pid !== 'number') return false; + try { + process.kill(ptyProcess.pid, 0); + return true; + } catch (e) { + return e.code === 'EPERM'; + } + }, + }; +} + /** * Build the `ctx` object trigger-watcher's `start(ctx)` expects. * @@ -16,8 +32,11 @@ function createTriggerContext({ activeSessions, log, isPtyAlive }) { getPtyForSession(sessionId) { const session = activeSessions.get(sessionId); if (!session || session.exited) return null; + const handle = (session.host == null) + ? createLocalSessionHandle(session.pty) + : session.handle; // cwd: see .ai/contexts/trigger-watcher.md, "Target guard" - return { ptyProcess: session.pty, cwd: session.cwd }; + return { ptyProcess: session.pty, cwd: session.cwd, handle }; }, isSessionBusy(sessionId) { const session = activeSessions.get(sessionId); @@ -34,4 +53,4 @@ function createTriggerContext({ activeSessions, log, isPtyAlive }) { return ctx; } -module.exports = { createTriggerContext }; +module.exports = { createTriggerContext, createLocalSessionHandle }; diff --git a/trigger-watcher.js b/trigger-watcher.js index bd07ecef..306523e8 100644 --- a/trigger-watcher.js +++ b/trigger-watcher.js @@ -36,6 +36,8 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +const { createLocalSessionHandle } = require('./trigger-context'); + const DEFAULT_TRIGGERS_DIR = path.join(os.homedir(), '.switchboard', 'triggers'); // Default idle-wait timeout: 5 minutes. // Rationale: agentic Claude CLI turns can run 10-20 min between idle states. @@ -102,20 +104,13 @@ function classifySubmitted(composerConfirmed, sawBusy) { return composerConfirmed ? SUBMITTED_CONFIRMED : (sawBusy ? SUBMITTED_ACTIVITY : SUBMITTED_ASSUMED); } -// W7 — child-process liveness check. -// node-pty's ptyProcess.write() is silent on a dead child: the bytes land in -// the kernel PTY buffer and are never consumed. Without this check the watcher -// would happily report ok:true on writes nobody will ever read. We use -// signal 0 (POSIX no-op probe) — throws ESRCH if the process is gone, -// throws EPERM if it exists but we can't signal it (still alive, treat as alive). -function defaultIsPtyAlive(ptyProcess) { - if (!ptyProcess || typeof ptyProcess.pid !== 'number') return false; - try { - process.kill(ptyProcess.pid, 0); - return true; - } catch (e) { - return e.code === 'EPERM'; - } +// W7 liveness probe — see .ai/contexts/trigger-watcher.md, "Session handle". +function resolveHandle(entry) { + return entry.handle || createLocalSessionHandle(entry.ptyProcess); +} + +function isEntryAlive(ctx, entry, handle) { + return ctx.isPtyAlive ? ctx.isPtyAlive(entry.ptyProcess) : handle.isAlive(); } // Poll interval (ms) for `delayWithBusyPoll` below. Deliberately finer than @@ -156,12 +151,12 @@ function delayWithBusyPoll(ms, sessionId, ctx) { // busy observed anywhere here cannot be attributed to an Enter that had not // been sent yet. See the "composerConfirmed" gate in submitWithVerify, which // this narrows. -async function submitToPty(ptyProcess, command, sessionId, ctx) { - ptyProcess.write(command); +async function submitToPty(handle, command, sessionId, ctx) { + handle.write(command); const envMs = envNumber('SWITCHBOARD_SUBMIT_ENTER_DELAY_MS'); const delayMs = envMs !== undefined ? envMs : DEFAULT_SUBMIT_ENTER_DELAY_MS; const midBusy = await delayWithBusyPoll(delayMs, sessionId, ctx); - ptyProcess.write('\r'); + handle.write('\r'); return midBusy; } @@ -376,11 +371,11 @@ function pollForBusyObserved(sessionId, ctx, windowMs, deadlineMs) { * the caller keeps the legacy instant-reply semantics — submit_retries traces * that the verification could not confirm a turn started. */ -async function submitWithVerify(ptyProcess, sessionId, command, ctx, deadlineMs) { +async function submitWithVerify(handle, sessionId, command, ctx, deadlineMs) { // Sampled before the write — see .ai/contexts/trigger-watcher.md ("submitted"). const preBusy = ctx.isSessionBusy(sessionId); - const midBusy = await submitToPty(ptyProcess, command, sessionId, ctx); + const midBusy = await submitToPty(handle, command, sessionId, ctx); // Composer read-back: unconditional, immediate, never gated on activity. const postWriteState = (typeof ctx.getComposerState === 'function') @@ -423,7 +418,7 @@ async function submitWithVerify(ptyProcess, sessionId, command, ctx, deadlineMs) } try { - ptyProcess.write('\r'); + handle.write('\r'); } catch (err) { // Surface as a sessionExited-like failure; caller maps to an error result. return { @@ -867,15 +862,14 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR return; } - const { ptyProcess } = sessionEntry; - const isPtyAlive = ctx.isPtyAlive || defaultIsPtyAlive; + const handle = resolveHandle(sessionEntry); // W7 — pre-flight liveness check. main.js may keep a stale entry in its // activeSessions map after a Claude process exited "cleanly" (Ctrl+D, /exit) // without the Switchboard window closing. Without this guard we'd wait the // full idle-timeout for a busy flag that will never flip, then write into a // dead PTY and report ok:true. - if (!isPtyAlive(ptyProcess)) { + if (!isEntryAlive(ctx, sessionEntry, handle)) { ctx.log.warn('[trigger-watcher] Target process not running:', sessionId); await writeResult({ ok: false, error: 'target process not running', sessionId }); return; @@ -971,7 +965,7 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR // deadline; the child may have exited during either while busy-was-true // never flipped. This probe belongs AFTER both waits: run before them it // proves nothing about the moment of the write. - if (!isPtyAlive(ptyProcess)) { + if (!isEntryAlive(ctx, sessionEntry, handle)) { ctx.log.warn('[trigger-watcher] Target process exited during wait:', sessionId); await writeResult({ ok: false, error: 'target process not running', sessionId, waited_ms }); return; @@ -987,7 +981,7 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR let recoverySkipped = false; let recoveryReason = null; try { - const v = await submitWithVerify(ptyProcess, sessionId, command, ctx); + const v = await submitWithVerify(handle, sessionId, command, ctx); submitRetries = v.submit_retries; sawBusy = v.sawBusy; composerConfirmed = !!v.composerConfirmed; @@ -1086,6 +1080,7 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR await writeResult({ ok: false, submitted: (i > 0) ? chainSubmitted : SUBMITTED_NO, error: 'session exited during wait', partial: i > 0, steps_completed: i, sessionId, sent_at: step0SentAt, steps, total_waited_ms: totalWaitedMs }); return; } + const entryHandle = resolveHandle(entry); // Inject the step command const stepSentAt = new Date().toISOString(); @@ -1131,7 +1126,7 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR // main.js still has a stale activeSessions entry. The probe belongs after // both waits. The remaining liveness→write TOCTOU window is bounded by // the try/catch on write. - if (!isPtyAlive(entry.ptyProcess)) { + if (!isEntryAlive(ctx, entry, entryHandle)) { ctx.log.warn(`[trigger-watcher] Target process not running at chain step ${i}:`, sessionId); await writeResult({ ok: false, submitted: (i > 0) ? chainSubmitted : SUBMITTED_NO, error: 'target process not running', partial: i > 0, steps_completed: i, sessionId, sent_at: step0SentAt, steps, total_waited_ms: totalWaitedMs }); return; @@ -1149,7 +1144,7 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR let stepWaitedMs = polite.waited_ms; let verify; try { - verify = await submitWithVerify(entry.ptyProcess, sessionId, step.command, ctx, stepDeadline); + verify = await submitWithVerify(entryHandle, sessionId, step.command, ctx, stepDeadline); } catch (err) { ctx.log.error(`[trigger-watcher] PTY write failed at chain step ${i}:`, err.message); await writeResult({ ok: false, error: 'pty write failed: ' + err.message, partial: true, steps_completed: i, sessionId, sent_at: step0SentAt, steps, total_waited_ms: totalWaitedMs }); @@ -1249,13 +1244,12 @@ async function processTriggerFile(name, ctx, triggersDir, processedDir, onEntryR * both apply identically). See .ai/contexts/trigger-watcher.md, "Startup scan". * * @param {object} ctx - * @param {function} ctx.getPtyForSession (sessionId: string) => { ptyProcess, cwd } | null; - * `cwd` feeds the expectedCwd target - * guard (see .ai/contexts/trigger-watcher.md, "Target guard") + * @param {function} ctx.getPtyForSession (sessionId: string) => { ptyProcess, cwd, handle } | null; + * see .ai/contexts/trigger-watcher.md, "Session handle" and "Target guard" * @param {function} ctx.isSessionBusy (sessionId: string) => boolean * @param {function} [ctx.getComposerState] (sessionId) => { pending, lastInputAt } | null; * absent or null means busy, never free - * @param {function} [ctx.isPtyAlive] (ptyProcess) => boolean (default: signal 0 probe) + * @param {function} [ctx.isPtyAlive] (ptyProcess) => boolean (default: handle.isAlive()) * @param {object} ctx.log electron-log compatible logger * @returns {{ close(): void }} */