Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion .ai/contexts/trigger-watcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions main.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
}

// Shell profiles → shell-profiles.js
const { discoverShellProfiles, getShellProfiles, resolveShell, isWindows, isWslShell, windowsToWslPath, shellArgs, quoteArgvForShell } = require('./shell-profiles');

Check warning on line 73 in main.js

View workflow job for this annotation

GitHub Actions / lint

'isWindows' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 73 in main.js

View workflow job for this annotation

GitHub Actions / lint

'discoverShellProfiles' is assigned a value but never used. Allowed unused vars must match /^_/u
const { startScheduler } = require('./schedule-runner');
const { encodeProjectPath } = require('./encode-project-path');
const { isSensitivePath, isAllowedMemoryPath: _isAllowedMemoryPath, resolveAllowedMemoryPath: _resolveAllowedMemoryPath, isKnownProjectRoot: _isKnownProjectRoot } = require('./ipc-path-validator');
Expand Down Expand Up @@ -452,8 +452,8 @@
isInitialScanComplete, setInitialScanComplete,
},
});
const { readSessionFile, readFolderFromFilesystem, refreshFolder, reconcileCacheFromFilesystem,

Check warning on line 455 in main.js

View workflow job for this annotation

GitHub Actions / lint

'readFolderFromFilesystem' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 455 in main.js

View workflow job for this annotation

GitHub Actions / lint

'readSessionFile' is assigned a value but never used. Allowed unused vars must match /^_/u
buildProjectsFromCache, notifyRendererProjectsChanged, sendStatus, populateCacheViaWorker,

Check warning on line 456 in main.js

View workflow job for this annotation

GitHub Actions / lint

'sendStatus' is assigned a value but never used. Allowed unused vars must match /^_/u
scanFoldersViaWorker, setRemoteRoots, resolveFolderDir } = sessionCache;
const { resolveJsonlPath, enumerateSessionFiles } = require('./read-session-file');

Expand Down Expand Up @@ -1918,7 +1918,7 @@
// WSL profiles only work for plain terminals — Claude CLI sessions need the
// Windows shell because session data lives on the Windows filesystem.
const requestedProfile = resolveShell(effectiveProfileId);
const useWslProfile = isWslShell(requestedProfile.path) && isPlainTerminal;

Check warning on line 1921 in main.js

View workflow job for this annotation

GitHub Actions / lint

'useWslProfile' is assigned a value but never used. Allowed unused vars must match /^_/u
const shellProfile = (isWslShell(requestedProfile.path) && !isPlainTerminal)
? resolveShell('auto')
: requestedProfile;
Expand Down Expand Up @@ -2138,6 +2138,8 @@
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);

Expand Down
51 changes: 50 additions & 1 deletion test/trigger-context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => {} };

Expand Down Expand Up @@ -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);
Expand Down
59 changes: 59 additions & 0 deletions test/trigger-watcher.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
23 changes: 21 additions & 2 deletions trigger-context.js
Original file line number Diff line number Diff line change
@@ -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.
*
Expand All @@ -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);
Expand All @@ -34,4 +53,4 @@ function createTriggerContext({ activeSessions, log, isPtyAlive }) {
return ctx;
}

module.exports = { createTriggerContext };
module.exports = { createTriggerContext, createLocalSessionHandle };
56 changes: 25 additions & 31 deletions trigger-watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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 });
Expand Down Expand Up @@ -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 }}
*/
Expand Down
Loading