diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 097e4962..f698cd7b 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -164,6 +164,104 @@ or deleted from here. directory or handed to `scp` (OpenSSH 9 runs scp over SFTP, where quoting would become part of the name — the validation is the guard, not quoting). +- **Session descriptors ride the SAME ssh call as the inventory (issue #211) + — never a second connection.** The CLI writes one descriptor file per live + session to `~/.claude/sessions/.json` on the remote host, alongside + unrelated `*.key` secret files (mode 600) in the same directory. + `remote-transport.js`'s `LIST_COMMAND` (`remote-transport.js:25-29`) is a + single shell command: the existing `find .claude/projects` + inventory, then a `printf` of a marker line, then a bounded pull of + `.claude/sessions`. `listFiles(alias)` still spawns exactly one `ssh` — the + test "listFiles spawns one bounded ssh…" in `test/remote-transport.test.js` + asserts both `find .claude/projects` and `find .claude/sessions` appear + inside that single command string. + - **The two halves fail independently, in opposite directions — neither + direction should be mistaken for the other.** The inventory `find` is + followed by `|| exit $?`: if it fails (missing `.claude/projects`, an + unmounted home, a permission change, a BusyBox `find` with no `-printf`), + the whole command aborts immediately with that `find`'s own exit status — + `listFiles` still throws and `syncMirror` still refuses to touch the local + mirror, exactly as before issue #211 introduced the sessions pull. The + sessions half's own failure (a missing or unreadable `.claude/sessions`) + is independently swallowed (`2>/dev/null`, and the final pipeline's exit + status is the trailing `while` loop's, not `find`'s) and degrades to zero + descriptors without affecting the inventory half. Before this fix, the `;` + between the two stages let the sessions half's `while` loop mask a failed + inventory `find` — an unreachable/misconfigured host's inventory failure + was silently read as "nothing remote exists", which `syncMirror` then + read as license to delete every locally mirrored file for that host. + - **The marker (`SESSIONS_MARKER = '\u0001SWITCHBOARD-SESSIONS\u0001'`) + cannot collide with real descriptor content, by construction.** It is + wrapped in a raw SOH control byte (0x01) on each side — written in source + with an explicit `\u0001` escape, not a raw byte, so the constant stays + legible in a diff — sent to the shell as `\001` inside a `printf` format + string. Valid JSON text can never contain a raw, unescaped control byte — + the JSON spec requires control characters inside a string to be escaped + (per RFC 8259 section 7) rather than appear as a raw byte — so no + legitimate descriptor line can ever contain the two raw 0x01 bytes that + frame the marker. `splitListOutput(stdout)` (`remote-transport.js`) only + accepts a marker occurrence preceded by a newline (or at byte offset 0 — + the legitimate case when the inventory `find` found zero files) and + followed by a newline, consuming that trailing newline into the boundary; + a marker substring that doesn't sit on its own line, or an absent marker + (an unexpected truncation), both degrade to treating the whole stdout as + the inventory block and return `sessionsBlock: ''` rather than throwing — + `parseInventory` keeps working exactly as before on the degraded input. + - **The command assumes a POSIX-sh-compatible remote login shell.** `;`, + `||`, pipes, `2>/dev/null` and `while … do … done` are `sh`/`bash`/`dash`/ + `ksh` syntax, not portable to `fish` (different loop syntax, no `do`/`done`) + or `csh`/`tcsh` (different control-flow syntax entirely) — a host whose + login shell is one of those would get a syntax error from `ssh`, whereas + the pre-#211 command (a single `find` invocation with no shell operators + at all) was portable to any shell. No such host is declared today; this is + a known, undemonstrated limitation, not something this change attempts to + fix. + - **`-name '[0-9]*.json'` is the only thing standing between this feature and + reading a `.key` secret file** — it structurally cannot match any `*.key` + filename regardless of the digit-prefix part, because the extension itself + is wrong. This is a property of the glob, not an added exclude-filter. + Pinned by two tests in `test/remote-transport.test.js`: an exact-string pin + of `LIST_COMMAND` (so widening the glob, or swapping the `find`+`head -c` + pipeline for a bare `cat *`, changes the string and fails immediately), and + a belt-and-suspenders `!LIST_COMMAND.includes('.key')` check that survives + unrelated wording changes. + - **Byte and count caps are enforced remotely, in the shell command itself** + (defense in depth, not just in JS): `head -c 8192 "$f"` bounds each + descriptor, `head -n 200` bounds the count. Worst case the sessions payload + adds ≈ 200 × 8193 ≈ 1.64 MiB to a cycle, comfortably under the existing + 8 MiB `MAX_LIST_BYTES` combined-output cap on its own — so this addition + cannot by itself push a cycle over that cap. A projects inventory already + near 8 MiB could still combine with this to overflow, but that is the + pre-existing risk of an oversized inventory, not a new failure mode. + `LC_ALL=C sort` orders the descriptor files deterministically (byte order, + not locale-dependent) — cosmetic, but keeps test/log output stable. + - **A missing or unreadable `~/.claude/sessions` degrades to zero + descriptors instead of failing the whole cycle.** The sessions half is a + pipeline ending in `while IFS= read -r f; do …; done`, whose exit status is + the *loop's* status (0, even on empty input) — not the `find`'s. `find`'s + own stderr is swallowed by `2>/dev/null`, so a missing directory produces + empty stdout and exit 0. The two `find` stages are joined by `;`, not + `&&`, and deliberately carry no `set -e`/`pipefail` — a failure in the + sessions half must never fail the inventory half it rides alongside. + - **`parseSessions(block)` preserves every field verbatim** — the schema + belongs to the CLI, not to Switchboard — validating only `pid` (positive + integer) and `sessionId` (non-empty string) before accepting a line. + Malformed lines (a `head -c`-truncated descriptor produces incomplete + JSON) are dropped with a **fixed, generic warning string only** — never the + raw line, the parsed object, or any field value — because descriptor + content must never be logged. `listFiles`'s return contract changed from a + plain array to `{ files, sessions }`; every stub of `transport.listFiles` + across `remote-mirror.test.js`, `remote-index.test.js` and + `remote-indexing-e2e.test.js` was updated to match in the same change. + - **`remote-index.js` keeps the latest descriptors per alias, keyed and + pruned exactly like folder keys.** `createRemoteIndexer()`'s private + `remoteSessions` map is set from `result.sessions` inside `refreshHost()` + and read back through `getRemoteSessions(alias)` (defaults to `[]` for an + alias never refreshed). `pruneUnknownAliases()` deletes its entries for any + alias no longer declared, the same pass that prunes folder keys, so the map + cannot grow unboundedly across host-list edits. No IPC, no renderer surface + and no attach/injection exist yet for this data — that is issue #212's job. + ### `stop()` cancels, `dispose()` ends -- they are not the same thing `createSshTransport().dispose()` is **terminal**: it sets a flag every later @@ -190,8 +288,9 @@ usable" and "dispose() is terminal", in `test/remote-index.test.js`. - `remote-hosts.test.js` — covers folder-key parsing, alias validation and the `isSafeRelPath` guard - `remote-mirror.test.js` — covers the inventory diff, the no-op second pull, deletions, and both failure modes, against a fake transport -- `remote-transport.test.js` — covers the ssh/scp argv, inventory parsing, the timeout kill and `dispose()`, with `spawn` injected -- `remote-index.test.js` — covers "no host declared: no timer, no ssh call", the 60 s floor, per-host failure isolation and alias pruning +- `remote-transport.test.js` — covers the ssh/scp argv, inventory parsing, the timeout kill and `dispose()`, with `spawn` injected; also covers `LIST_COMMAND`'s exact text (issue #211's `.key`-exclusion and single-ssh-call pins), `splitListOutput()` and `parseSessions()` +- `remote-transport-shell.test.js` — runs `LIST_COMMAND` through a real `sh -c`, not a fake stdout fixture: a missing `.claude/projects` must exit non-zero, a missing `.claude/sessions` must still exit 0 with the marker present, and a `.key` file plus a directory named like a descriptor must both be excluded from what reaches stdout +- `remote-index.test.js` — covers "no host declared: no timer, no ssh call", the 60 s floor, per-host failure isolation and alias pruning, and that `getRemoteSessions()` is cleared (not left stale) after a cycle whose `sync()` throws - `remote-indexing-e2e.test.js` — covers the `::` prefix reaching session rows, the search entries, the metrics and the sidebar - `dom-sidebar-remote-session.test.js` — covers the remote badge and the read-only click routing - `derive-project-path.test.js` — covers the worktree-collapse + cwd extraction paths diff --git a/remote-index.js b/remote-index.js index cdd71f37..d8371a6f 100644 --- a/remote-index.js +++ b/remote-index.js @@ -39,6 +39,7 @@ function createRemoteIndexer(ctx) { let timer = null; let inFlight = false; let stopped = false; + const remoteSessions = new Map(); // alias -> sessions array, from the same ssh cycle as the inventory function hosts() { return enabledHosts(ctx.getHosts ? ctx.getHosts() : []); @@ -62,6 +63,9 @@ function createRemoteIndexer(ctx) { ctx.dropFolder(key); dropped++; } + for (const alias of [...remoteSessions.keys()]) { + if (!known.has(alias)) remoteSessions.delete(alias); + } return dropped; } @@ -88,6 +92,8 @@ function createRemoteIndexer(ctx) { log, }); + remoteSessions.set(host.alias, Array.isArray(result.sessions) ? result.sessions : []); + const folderPrefix = host.alias; const toScan = new Set(result.changedFolders); @@ -140,6 +146,7 @@ function createRemoteIndexer(ctx) { try { if (await refreshHost(host)) changed = true; } catch (err) { + remoteSessions.set(host.alias, []); errors.push({ alias: host.alias, error: err.message }); log.warn(`[remote:${host.alias}] refresh failed: ${err.message}`); } @@ -186,7 +193,11 @@ function createRemoteIndexer(ctx) { return start(); } - return { start, stop, dispose, restart, refreshNow, isRunning: () => timer !== null }; + function getRemoteSessions(alias) { + return remoteSessions.get(alias) || []; + } + + return { start, stop, dispose, restart, refreshNow, isRunning: () => timer !== null, getRemoteSessions }; } module.exports = { createRemoteIndexer }; diff --git a/remote-mirror.js b/remote-mirror.js index b3456e1c..3980edb8 100644 --- a/remote-mirror.js +++ b/remote-mirror.js @@ -40,18 +40,18 @@ function pruneEmptyDirs(root, dir) { /** * Bring the local mirror of one host in line with its remote inventory. * Injected transport: - * listFiles(alias) -> Promise<[{ rel, size, mtimeMs }]> + * listFiles(alias) -> Promise<{ files: [{ rel, size, mtimeMs }], sessions: [object] }> * fetchFiles(alias, rels, destRoot) -> Promise<{ fetched: [], failed: [] }> */ async function syncMirror({ alias, transport, projectsDir, manifestPath, log }) { - const inventory = await transport.listFiles(alias); - if (!Array.isArray(inventory)) throw new Error('transport.listFiles did not return an array'); - if (inventory.length > MAX_INVENTORY_ENTRIES) { - throw new Error(`remote inventory too large (${inventory.length} entries)`); + const { files, sessions } = await transport.listFiles(alias); + if (!Array.isArray(files)) throw new Error('transport.listFiles did not return a files array'); + if (files.length > MAX_INVENTORY_ENTRIES) { + throw new Error(`remote inventory too large (${files.length} entries)`); } const want = new Map(); - for (const entry of inventory) { + for (const entry of files) { if (!entry || !isSafeRelPath(entry.rel)) continue; if (!topFolderOf(entry.rel)) continue; // a transcript must live under a project folder want.set(entry.rel, { size: Number(entry.size) || 0, mtimeMs: Number(entry.mtimeMs) || 0 }); @@ -143,6 +143,7 @@ async function syncMirror({ alias, transport, projectsDir, manifestPath, log }) unchanged: want.size - toFetch.length, removed, changedFolders, + sessions: Array.isArray(sessions) ? sessions : [], }; } diff --git a/remote-transport.js b/remote-transport.js index 40784447..bf5c977e 100644 --- a/remote-transport.js +++ b/remote-transport.js @@ -6,6 +6,10 @@ const path = require('path'); const { isSafeRelPath } = require('./remote-hosts'); const REMOTE_PROJECTS_REL = '.claude/projects'; +const REMOTE_SESSIONS_REL = '.claude/sessions'; +const SESSIONS_MARKER = '\u0001SWITCHBOARD-SESSIONS\u0001'; +const MAX_SESSION_DESCRIPTORS = 200; +const MAX_SESSION_DESCRIPTOR_BYTES = 8192; const DEFAULT_CONNECT_TIMEOUT_S = 10; const DEFAULT_LIST_TIMEOUT_MS = 60_000; const DEFAULT_FETCH_TIMEOUT_MS = 120_000; @@ -17,8 +21,12 @@ const SSH_BASE_OPTS = [ '-o', `ConnectTimeout=${DEFAULT_CONNECT_TIMEOUT_S}`, ]; +// see .ai/contexts/session-cache.md ("Remote SSH hosts (issue #211)") const LIST_COMMAND = - `find ${REMOTE_PROJECTS_REL} -type f -name '*.jsonl' -printf '%T@\\t%s\\t%P\\n'`; + `find ${REMOTE_PROJECTS_REL} -type f -name '*.jsonl' -printf '%T@\\t%s\\t%P\\n' || exit $?; ` + + `printf '\\001SWITCHBOARD-SESSIONS\\001\\n'; ` + + `find ${REMOTE_SESSIONS_REL} -maxdepth 1 -type f -name '[0-9]*.json' 2>/dev/null | LC_ALL=C sort | ` + + `head -n ${MAX_SESSION_DESCRIPTORS} | while IFS= read -r f; do head -c ${MAX_SESSION_DESCRIPTOR_BYTES} "$f"; printf '\\n'; done`; function parseInventory(stdout) { const out = []; @@ -36,6 +44,48 @@ function parseInventory(stdout) { return out; } +// see .ai/contexts/session-cache.md ("Remote SSH hosts (issue #211)") +function splitListOutput(stdout) { + const idx = stdout.indexOf(SESSIONS_MARKER); + if (idx === -1) return { inventoryBlock: stdout, sessionsBlock: '' }; + const afterIdx = idx + SESSIONS_MARKER.length; + const validStart = idx === 0 || stdout[idx - 1] === '\n'; + const validEnd = stdout[afterIdx] === '\n'; + if (!validStart || !validEnd) return { inventoryBlock: stdout, sessionsBlock: '' }; + return { inventoryBlock: stdout.slice(0, idx), sessionsBlock: stdout.slice(afterIdx + 1) }; +} + +// see .ai/contexts/session-cache.md ("Remote SSH hosts (issue #211)") +function parseSessions(block) { + const sessions = []; + const warnings = []; + for (const rawLine of block.split('\n')) { + const line = rawLine.replace(/\r$/, ''); + if (!line) continue; + let parsed; + try { + parsed = JSON.parse(line); + } catch { + warnings.push('skipped a session descriptor: invalid JSON'); + continue; + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + warnings.push('skipped a session descriptor: not a JSON object'); + continue; + } + if (!Number.isInteger(parsed.pid) || parsed.pid <= 0) { + warnings.push('skipped a session descriptor: missing/invalid pid'); + continue; + } + if (typeof parsed.sessionId !== 'string' || !parsed.sessionId) { + warnings.push('skipped a session descriptor: missing/invalid sessionId'); + continue; + } + sessions.push(parsed); + } + return { sessions, warnings }; +} + /** * ssh/scp transport. `spawn` is injected so the process bounding, the argv and * the parsing are all testable without a network or an ssh binary. @@ -116,7 +166,11 @@ function createSshTransport(opts = {}) { if (res.timedOut) throw new Error(`ssh inventory timed out after ${listTimeoutMs} ms`); if (res.truncated) throw new Error('ssh inventory output exceeded the size cap'); if (res.code !== 0) throw new Error(`ssh inventory failed (exit ${res.code}): ${res.stderr.trim() || 'no stderr'}`); - return parseInventory(res.stdout); + const { inventoryBlock, sessionsBlock } = splitListOutput(res.stdout); + const files = parseInventory(inventoryBlock); + const { sessions, warnings } = parseSessions(sessionsBlock); + for (const w of warnings) log.warn(`[remote:${alias}] ${w}`); + return { files, sessions }; } async function fetchOne(alias, rel, destRoot) { @@ -182,4 +236,15 @@ function createSshTransport(opts = {}) { return { listFiles, fetchFiles, cancelInFlight, dispose, liveCount: () => live.size }; } -module.exports = { createSshTransport, parseInventory, LIST_COMMAND, REMOTE_PROJECTS_REL }; +module.exports = { + createSshTransport, + parseInventory, + parseSessions, + splitListOutput, + LIST_COMMAND, + REMOTE_PROJECTS_REL, + REMOTE_SESSIONS_REL, + SESSIONS_MARKER, + MAX_SESSION_DESCRIPTORS, + MAX_SESSION_DESCRIPTOR_BYTES, +}; diff --git a/test/remote-index.test.js b/test/remote-index.test.js index 3111c5dc..135c2b76 100644 --- a/test/remote-index.test.js +++ b/test/remote-index.test.js @@ -140,6 +140,70 @@ test('a failing host is logged and does not stop its peer', async () => { } finally { fs.rmSync(dataDir, { recursive: true, force: true }); } }); +test('getRemoteSessions surfaces per-host session descriptors from the same sync cycle', async () => { + const dataDir = tmp('idx-sessions'); + try { + const sessionsByAlias = { + withSessions: [{ pid: 123, sessionId: 'abc' }], + empty: [], + }; + const indexer = createRemoteIndexer({ + getHosts: () => [{ alias: 'withSessions' }, { alias: 'empty' }], + dataDir, + transport: {}, + scanFolders: () => Promise.resolve({ ok: true }), + listIndexedFolderKeys: () => [], + timers: fakeTimers(), + sync: async ({ alias }) => ({ + fetched: 0, unchanged: 0, removed: 0, failed: 0, total: 0, + changedFolders: new Set(), sessions: sessionsByAlias[alias], + }), + }); + + const r = await indexer.refreshNow(); + + assert.deepEqual(r.errors, [], 'both hosts complete without error'); + assert.deepEqual(indexer.getRemoteSessions('withSessions'), sessionsByAlias.withSessions); + assert.deepEqual(indexer.getRemoteSessions('empty'), []); + assert.deepEqual(indexer.getRemoteSessions('some-alias-never-refreshed'), [], + 'an unknown alias must never throw or return undefined'); + } finally { fs.rmSync(dataDir, { recursive: true, force: true }); } +}); + +test('getRemoteSessions is cleared, not left stale, after a cycle where sync() throws', async () => { + const dataDir = tmp('idx-sessions-stale'); + try { + let cycle = 0; + const indexer = createRemoteIndexer({ + getHosts: () => [{ alias: 'planificator' }], + dataDir, + transport: {}, + scanFolders: () => Promise.resolve({ ok: true }), + listIndexedFolderKeys: () => [], + timers: fakeTimers(), + sync: async () => { + cycle++; + if (cycle === 1) { + return { + fetched: 0, unchanged: 0, removed: 0, failed: 0, total: 0, + changedFolders: new Set(), sessions: [{ pid: 1, sessionId: 'still-alive' }], + }; + } + throw new Error('ssh: connect to host planificator port 22: timed out'); + }, + }); + + const r1 = await indexer.refreshNow(); + assert.deepEqual(r1.errors, []); + assert.deepEqual(indexer.getRemoteSessions('planificator'), [{ pid: 1, sessionId: 'still-alive' }]); + + const r2 = await indexer.refreshNow(); + assert.equal(r2.errors.length, 1, 'the second cycle must be reported as failed'); + assert.deepEqual(indexer.getRemoteSessions('planificator'), [], + 'a failed cycle must not keep reporting hours-old sessions as live'); + } finally { fs.rmSync(dataDir, { recursive: true, force: true }); } +}); + test('a mirror already on disk but absent from the cache is indexed once', async () => { const dataDir = tmp('idx-cold'); try { @@ -204,7 +268,7 @@ function lifecycleTransport() { async listFiles(alias) { if (disposed) throw new Error('ssh inventory failed (exit -1): transport disposed'); calls.push(alias); - return []; + return { files: [], sessions: [] }; }, fetchFiles: async () => ({ fetched: [], failed: [] }), cancelInFlight() {}, diff --git a/test/remote-indexing-e2e.test.js b/test/remote-indexing-e2e.test.js index 605201d4..11ca9cb8 100644 --- a/test/remote-indexing-e2e.test.js +++ b/test/remote-indexing-e2e.test.js @@ -88,7 +88,10 @@ function fakeHost() { return { files, async listFiles() { - return Object.entries(files).map(([rel, f]) => ({ rel, size: f.content.length, mtimeMs: f.mtimeMs })); + return { + files: Object.entries(files).map(([rel, f]) => ({ rel, size: f.content.length, mtimeMs: f.mtimeMs })), + sessions: [], + }; }, async fetchFiles(alias, rels, destRoot) { for (const rel of rels) { diff --git a/test/remote-mirror.test.js b/test/remote-mirror.test.js index 090d4898..dea3a0d6 100644 --- a/test/remote-mirror.test.js +++ b/test/remote-mirror.test.js @@ -24,9 +24,12 @@ function fakeTransport(files, opts = {}) { async listFiles() { calls.list++; if (opts.listThrows) throw new Error(opts.listThrows); - return Object.entries(files).map(([rel, f]) => ({ - rel, size: f.content.length, mtimeMs: f.mtimeMs, - })); + return { + files: Object.entries(files).map(([rel, f]) => ({ + rel, size: f.content.length, mtimeMs: f.mtimeMs, + })), + sessions: opts.sessions || [], + }; }, async fetchFiles(alias, rels, destRoot) { calls.fetch++; @@ -218,10 +221,13 @@ test('a file above the per-file ceiling is never fetched', async () => { const manifestPath = path.join(dir, 'manifest.json'); const asked = []; const transport = { - listFiles: async () => ([ - { rel: '-srv-a/small.jsonl', size: 10, mtimeMs: 1 }, - { rel: '-srv-a/huge.jsonl', size: 200 * 1024 * 1024, mtimeMs: 1 }, - ]), + listFiles: async () => ({ + files: [ + { rel: '-srv-a/small.jsonl', size: 10, mtimeMs: 1 }, + { rel: '-srv-a/huge.jsonl', size: 200 * 1024 * 1024, mtimeMs: 1 }, + ], + sessions: [], + }), fetchFiles: async (_alias, rels, destRoot) => { asked.push(...rels); for (const rel of rels) { @@ -242,3 +248,21 @@ test('a file above the per-file ceiling is never fetched', async () => { assert.ok(warned.some(m => m.includes('skipped')), 'the skip must be reported, not silent'); } finally { fs.rmSync(dir, { recursive: true, force: true }); } }); + +// issue #211: a host with no ~/.claude/sessions dir still resolves normally. +test('a host with no sessions dir completes the cycle with zero descriptors', async () => { + const dir = tmp('mirror-nosessions'); + try { + const projectsDir = path.join(dir, 'projects'); + const manifestPath = path.join(dir, 'inventory.json'); + const t = fakeTransport({ + '-srv-a/a.jsonl': { content: line('/srv/a'), mtimeMs: 1000 }, + }); + + const result = await syncMirror({ alias: 'vps', transport: t, projectsDir, manifestPath }); + + assert.deepEqual(result.sessions, []); + assert.equal(result.fetched, 1, 'inventory fetch/deletion behavior is unaffected'); + assert.ok(fs.existsSync(path.join(projectsDir, '-srv-a', 'a.jsonl'))); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}); diff --git a/test/remote-transport-shell.test.js b/test/remote-transport-shell.test.js new file mode 100644 index 00000000..a30ce110 --- /dev/null +++ b/test/remote-transport-shell.test.js @@ -0,0 +1,87 @@ +'use strict'; + +// Real shell execution of LIST_COMMAND, not a fake stdout fixture. Every other +// test in remote-transport.test.js hand-builds stdout or does string equality +// on LIST_COMMAND — none of them run the command through a shell, which is how +// the exit-status-swallowing bug (issue #211 follow-up) shipped past three +// rounds of review. See .ai/contexts/session-cache.md ("Remote SSH hosts (issue #211)"). + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const { LIST_COMMAND, SESSIONS_MARKER, splitListOutput, parseSessions } = require('../remote-transport'); + +function shAvailable() { + const r = spawnSync('sh', ['-c', 'exit 0']); + return !r.error; +} + +const SH_SKIP = shAvailable() ? false : 'sh is not available on this machine'; + +function sandbox() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'switchboard-shell-')); +} + +test('LIST_COMMAND: .claude/projects missing yields a non-zero exit status', { skip: SH_SKIP }, () => { + const dir = sandbox(); + try { + const result = spawnSync('sh', ['-c', LIST_COMMAND], { cwd: dir, encoding: 'utf8' }); + assert.notEqual(result.status, 0, 'a missing inventory root must fail the whole command'); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}); + +test('LIST_COMMAND: .claude/sessions missing but .claude/projects present (even empty) degrades cleanly', { skip: SH_SKIP }, () => { + const dir = sandbox(); + try { + fs.mkdirSync(path.join(dir, '.claude', 'projects'), { recursive: true }); + const result = spawnSync('sh', ['-c', LIST_COMMAND], { cwd: dir, encoding: 'utf8' }); + assert.equal(result.status, 0); + assert.ok(result.stdout.includes(SESSIONS_MARKER), 'the marker must still be emitted'); + const { inventoryBlock, sessionsBlock } = splitListOutput(result.stdout); + assert.equal(inventoryBlock, '', 'a refused split would leave the marker inside inventoryBlock instead of sessionsBlock'); + assert.equal(sessionsBlock, '', + 'an empty projects dir puts the marker at byte offset 0 with no preceding newline — must still split cleanly'); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}); + +test('LIST_COMMAND: only a digit-named .json FILE is read — never a .key file, a directory, or a symlink', { skip: SH_SKIP }, () => { + const dir = sandbox(); + try { + const sessionsDir = path.join(dir, '.claude', 'sessions'); + fs.mkdirSync(path.join(dir, '.claude', 'projects'), { recursive: true }); + fs.mkdirSync(sessionsDir, { recursive: true }); + fs.writeFileSync(path.join(sessionsDir, '1.json'), JSON.stringify({ pid: 1, sessionId: 'abc' })); + fs.writeFileSync(path.join(sessionsDir, '1.abc.key'), 'top-secret-key-material-should-never-appear'); + fs.mkdirSync(path.join(sessionsDir, '2.json'), { recursive: true }); + + const symlinkTarget = path.join(dir, 'outside-target.txt'); + fs.writeFileSync(symlinkTarget, 'symlink-target-should-never-appear'); + let symlinkCreated = false; + try { + fs.symlinkSync(symlinkTarget, path.join(sessionsDir, '3.json'), 'file'); + symlinkCreated = true; + } catch { + // Symlink creation needs an elevated privilege on Windows by default; + // skip only this one assertion below, not the rest of the test. + } + + const result = spawnSync('sh', ['-c', LIST_COMMAND], { cwd: dir, encoding: 'utf8' }); + + assert.equal(result.status, 0); + assert.ok(!result.stdout.includes('top-secret-key-material-should-never-appear'), + 'the .key content must never reach stdout'); + if (symlinkCreated) { + assert.ok(!result.stdout.includes('symlink-target-should-never-appear'), + 'a symlink named like a valid descriptor must never have its target content reach stdout'); + } + const { sessionsBlock } = splitListOutput(result.stdout); + const { sessions } = parseSessions(sessionsBlock); + assert.equal(sessions.length, 1, + 'exactly one descriptor: the directory, the .key file and any symlink are all excluded'); + assert.equal(sessions[0].sessionId, 'abc'); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } +}); diff --git a/test/remote-transport.test.js b/test/remote-transport.test.js index 970806ab..d246f319 100644 --- a/test/remote-transport.test.js +++ b/test/remote-transport.test.js @@ -13,7 +13,10 @@ const path = require('path'); const { EventEmitter } = require('events'); const { Readable } = require('stream'); -const { createSshTransport, parseInventory } = require('../remote-transport'); +const { + createSshTransport, parseInventory, parseSessions, splitListOutput, + LIST_COMMAND, SESSIONS_MARKER, MAX_SESSION_DESCRIPTORS, MAX_SESSION_DESCRIPTOR_BYTES, +} = require('../remote-transport'); function fakeChild() { const child = new EventEmitter(); @@ -55,25 +58,200 @@ test('parseInventory keeps well-formed lines and drops everything else', () => { test('listFiles spawns one bounded ssh with the alias as an operand, never as a shell string', async () => { const spawn = spawnRecorder((child) => { child.stdout.push('1757200000.0\t9\t-srv-a/a.jsonl\n'); + child.stdout.push(SESSIONS_MARKER + '\n'); + child.stdout.push('{"pid":123,"sessionId":"abc"}\n'); child.stdout.push(null); child.emit('close', 0); }); const t = createSshTransport({ spawn }); - const entries = await t.listFiles('planificator'); + const result = await t.listFiles('planificator'); - assert.equal(spawn.calls.length, 1); + assert.equal(spawn.calls.length, 1, 'exactly one ssh call — the inventory and the sessions ride together'); const { cmd, args } = spawn.calls[0]; assert.equal(cmd, 'ssh'); assert.ok(args.includes('BatchMode=yes'), 'must never prompt for a passphrase'); // The alias is its own argv element and the remote command is the last one: // nothing the user typed is ever concatenated into a local shell string. assert.equal(args[args.length - 2], 'planificator'); - assert.match(args[args.length - 1], /^find \.claude\/projects /); - assert.deepEqual(entries, [{ rel: '-srv-a/a.jsonl', size: 9, mtimeMs: 1757200000000 }]); + const command = args[args.length - 1]; + assert.match(command, /^find \.claude\/projects /); + assert.ok(command.includes('find .claude/projects'), 'the inventory half must still run'); + assert.ok(command.includes('find .claude/sessions'), 'the session descriptors must ride the same command'); + assert.deepEqual(result, { + files: [{ rel: '-srv-a/a.jsonl', size: 9, mtimeMs: 1757200000000 }], + sessions: [{ pid: 123, sessionId: 'abc' }], + }); assert.equal(t.liveCount(), 0, 'the child is unregistered once it closes'); }); +// issue #211: -name '[0-9]*.json' is the ONLY thing standing between this +// feature and reading a '*.key' secret file dropped in the same directory. +test('LIST_COMMAND is pinned exactly — any widening of the sessions glob must fail this test', () => { + const expected = + `find .claude/projects -type f -name '*.jsonl' -printf '%T@\\t%s\\t%P\\n' || exit $?; ` + + `printf '\\001SWITCHBOARD-SESSIONS\\001\\n'; ` + + `find .claude/sessions -maxdepth 1 -type f -name '[0-9]*.json' 2>/dev/null | LC_ALL=C sort | ` + + `head -n ${MAX_SESSION_DESCRIPTORS} | while IFS= read -r f; do head -c ${MAX_SESSION_DESCRIPTOR_BYTES} "$f"; printf '\\n'; done`; + assert.equal(LIST_COMMAND, expected); +}); + +test('LIST_COMMAND can never match a .key file, independent of exact wording', () => { + assert.ok(!LIST_COMMAND.includes('.key')); +}); + +test('a host with no sessions dir yields zero descriptors without failing the cycle', async () => { + const spawn = spawnRecorder((child) => { + child.stdout.push('1757200000.0\t9\t-srv-a/a.jsonl\n'); + child.stdout.push(SESSIONS_MARKER + '\n'); + // Nothing after the marker — exactly what the real command produces when + // ~/.claude/sessions is missing or empty (the while-loop still exits 0). + child.stdout.push(null); + child.emit('close', 0); + }); + const t = createSshTransport({ spawn }); + + const result = await t.listFiles('vps'); + + assert.deepEqual(result.sessions, []); + assert.deepEqual(result.files, [{ rel: '-srv-a/a.jsonl', size: 9, mtimeMs: 1757200000000 }]); +}); + +test('listFiles degrades to zero sessions when the marker is unexpectedly absent from stdout', async () => { + const spawn = spawnRecorder((child) => { + child.stdout.push('1757200000.0\t9\t-srv-a/a.jsonl\n'); + child.stdout.push(null); + child.emit('close', 0); + }); + const t = createSshTransport({ spawn }); + + const result = await t.listFiles('vps'); + + assert.deepEqual(result.sessions, []); + assert.deepEqual(result.files, [{ rel: '-srv-a/a.jsonl', size: 9, mtimeMs: 1757200000000 }]); +}); + +// Regression guard: built from the literal wire bytes the real remote +// `printf '\001SWITCHBOARD-SESSIONS\001\n'` produces, NOT from the +// SESSIONS_MARKER export — so this test would catch the constant and the +// real command drifting out of sync with each other, which a fixture built +// from the same constant used by the code under test cannot catch. +test('the real wire-format marker (raw SOH-framed bytes) is recognized with zero warnings', async () => { + const warnings = []; + const log = { info() {}, warn: (m) => warnings.push(m), error() {} }; + const spawn = spawnRecorder((child) => { + child.stdout.push('1757200000.0\t9\t-srv-a/a.jsonl\n'); + child.stdout.push('\x01SWITCHBOARD-SESSIONS\x01\n'); + child.stdout.push(JSON.stringify({ pid: 1, sessionId: 'x' }) + '\n'); + child.stdout.push(null); + child.emit('close', 0); + }); + const t = createSshTransport({ spawn, log }); + + const result = await t.listFiles('vps'); + + assert.deepEqual(result.sessions, [{ pid: 1, sessionId: 'x' }]); + assert.deepEqual(warnings, [], 'a well-formed descriptor after the real marker must never warn'); +}); + +test('splitListOutput: marker present splits inventory from sessions', () => { + const stdout = 'inv-line\n' + SESSIONS_MARKER + '\n{"a":1}\n'; + assert.deepEqual(splitListOutput(stdout), { + inventoryBlock: 'inv-line\n', + sessionsBlock: '{"a":1}\n', + }); +}); + +test('splitListOutput: marker absent degrades to the full stdout as the inventory block', () => { + const stdout = 'inv-line-1\ninv-line-2\n'; + assert.deepEqual(splitListOutput(stdout), { inventoryBlock: stdout, sessionsBlock: '' }); +}); + +// Real remote output always has a trailing '\n' after the marker (it's part of +// the printf format), and this is the legitimate zero-descriptors case: the +// marker is on its own line with nothing following it. +test('splitListOutput: marker present with nothing after it yields an empty, not undefined, sessionsBlock', () => { + const stdout = 'inv-line\n' + SESSIONS_MARKER + '\n'; + const result = splitListOutput(stdout); + assert.equal(result.inventoryBlock, 'inv-line\n'); + assert.equal(result.sessionsBlock, ''); + assert.notEqual(result.sessionsBlock, undefined); +}); + +// The offset-0 legitimate case: .claude/projects found zero files, so the +// marker is the very first thing in stdout with no preceding newline. +test('splitListOutput: marker at byte offset 0 (empty inventory) splits normally', () => { + const stdout = SESSIONS_MARKER + '\n{"a":1}\n'; + assert.deepEqual(splitListOutput(stdout), { + inventoryBlock: '', + sessionsBlock: '{"a":1}\n', + }); +}); + +// A marker-like substring embedded mid-line (not preceded by '\n'/start, or +// not followed by '\n') is not a legitimate marker line: degrade rather than +// mis-split on it. +test('splitListOutput: marker not preceded by a newline degrades instead of splitting', () => { + const stdout = 'inv-line' + SESSIONS_MARKER + '\n{"a":1}\n'; + assert.deepEqual(splitListOutput(stdout), { inventoryBlock: stdout, sessionsBlock: '' }); +}); + +test('splitListOutput: marker not followed by a newline degrades instead of splitting', () => { + const stdout = 'inv-line\n' + SESSIONS_MARKER + 'trailing-garbage\n'; + assert.deepEqual(splitListOutput(stdout), { inventoryBlock: stdout, sessionsBlock: '' }); +}); + +test('parseSessions preserves every field verbatim, including arrays and nested objects', () => { + const descriptor = { + pid: 4242, + sessionId: 'sess-abc', + peerFeatures: ['a', 'b', 'c'], + nested: { model: 'claude-opus-5', extra: { deep: true } }, + }; + const { sessions, warnings } = parseSessions(JSON.stringify(descriptor) + '\n'); + assert.deepEqual(sessions, [descriptor]); + assert.deepEqual(warnings, []); +}); + +test('parseSessions drops invalid JSON without logging its content, keeping the valid neighbors', () => { + const block = [ + JSON.stringify({ pid: 1, sessionId: 'one' }), + 'not valid json {{{', + JSON.stringify({ pid: 2, sessionId: 'two' }), + ].join('\n'); + const { sessions, warnings } = parseSessions(block); + assert.deepEqual(sessions, [{ pid: 1, sessionId: 'one' }, { pid: 2, sessionId: 'two' }]); + assert.equal(warnings.length, 1); + assert.ok(!warnings[0].includes('not valid json'), 'the raw malformed line must never be logged'); + assert.ok(!warnings[0].includes('{{{'), 'no fragment of the malformed content may leak into the warning'); +}); + +test('parseSessions drops a descriptor missing sessionId', () => { + const { sessions, warnings } = parseSessions(JSON.stringify({ pid: 1 }) + '\n'); + assert.deepEqual(sessions, []); + assert.equal(warnings.length, 1); + assert.match(warnings[0], /sessionId/); +}); + +test('parseSessions drops a descriptor with a missing or non-integer pid', () => { + const r1 = parseSessions(JSON.stringify({ sessionId: 'x' }) + '\n'); + assert.deepEqual(r1.sessions, []); + assert.equal(r1.warnings.length, 1); + + const r2 = parseSessions(JSON.stringify({ pid: 'not-a-number', sessionId: 'x' }) + '\n'); + assert.deepEqual(r2.sessions, []); + assert.equal(r2.warnings.length, 1); + + const r3 = parseSessions(JSON.stringify({ pid: -1, sessionId: 'x' }) + '\n'); + assert.deepEqual(r3.sessions, []); + assert.equal(r3.warnings.length, 1); +}); + +test('parseSessions on a blank/empty block returns no sessions and no warnings', () => { + assert.deepEqual(parseSessions(''), { sessions: [], warnings: [] }); + assert.deepEqual(parseSessions('\n\n'), { sessions: [], warnings: [] }); +}); + test('a non-zero ssh exit is an error, not an empty inventory', async () => { const spawn = spawnRecorder((child) => { child.stderr.push('Permission denied (publickey).');