From c6610da41a7b0db6b51ed978bfd36caa2409aa73 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Tue, 8 Sep 2026 11:49:08 +0200 Subject: [PATCH] feat(remote): fetch CLI session descriptors on the inventory ssh connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI writes one descriptor per live session to ~/.claude/sessions/.json on a remote host. Fetching those required either a second ssh call per cycle or piggybacking on the single inventory connection remote-transport.js already opens; the latter is the only option that costs nothing extra, since the OpenSSH client shipped on Windows has no ControlMaster to amortize a second connection over. LIST_COMMAND runs the existing projects find, an SOH-framed marker line, then a bounded pull of .claude/sessions (name-filtered to [0-9]*.json, and -type f so a symlink or a directory named like a descriptor is excluded the same way a .key secret file is; capped at 200 files and 8 KiB each, comfortably under the existing 8 MiB combined-output cap) — all in one command, so listFiles() still spawns exactly one ssh. The inventory find is followed by `|| exit $?` so its failure aborts the whole command with its own exit status; without it, the trailing while-loop's exit status (always 0) masked a failed inventory find as "the remote has nothing", which syncMirror then read as license to delete every locally mirrored file for that host. The sessions half fails independently and silently (2>/dev/null, degrading to zero descriptors) without touching that guarantee. Proven with real `sh -c` execution in test/remote-transport-shell.test.js, including a symlink fixture that pins -type f specifically (a directory alone doesn't: head -c on one writes nothing to stdout either way, so only a followed symlink actually distinguishes the guard being present from absent). parseSessions() preserves every field the CLI writes verbatim — the schema is the CLI's, not ours — validating only pid and sessionId before accepting a descriptor, and never logging descriptor content on a parse failure. listFiles()'s contract changes from a bare inventory array to { files, sessions }; syncMirror() and remote-index.js thread the sessions array through per host, exposed via a new getRemoteSessions(alias) accessor. A cycle whose sync() throws clears that host's entry to an empty array rather than leaving the last successful read in place, because the accessor is a liveness signal and a false "still alive" after hours of unreachability is a worse failure mode than a temporary empty result. No IPC, no UI, no attach — that's issue #212. Refs #211 --- .ai/contexts/session-cache.md | 103 ++++++++++++++- remote-index.js | 13 +- remote-mirror.js | 13 +- remote-transport.js | 71 ++++++++++- test/remote-index.test.js | 66 +++++++++- test/remote-indexing-e2e.test.js | 5 +- test/remote-mirror.test.js | 38 ++++-- test/remote-transport-shell.test.js | 87 +++++++++++++ test/remote-transport.test.js | 188 +++++++++++++++++++++++++++- 9 files changed, 558 insertions(+), 26 deletions(-) create mode 100644 test/remote-transport-shell.test.js 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).');