From 19ea91752774e86ecfc9513ef171853c93cc2a4c Mon Sep 17 00:00:00 2001 From: abate Date: Mon, 5 Oct 2026 12:30:16 +0200 Subject: [PATCH 01/17] fix(sessions): follow /clear to the new transcript instead of listing it apart /clear makes the CLI open a new jsonl under a new session id that names nothing of the one it replaced, so fork detection never matched it: the terminal stayed on the old row and the new conversation appeared as a separate, unattached session. The CLI's state file switches to the new id on /clear; resolve its pid up to the PTY's and re-key the session like a fork. --- .ai/contexts/cli-session-state.md | 44 ++++++++ CHANGELOG.md | 1 + cli-session-state.js | 27 +++++ main.js | 1 + public/app.js | 5 + session-transitions.js | 44 +++++++- test/cli-session-state.test.js | 28 +++++ test/session-transitions.test.js | 175 +++++++++++++++++++++++++++++- 8 files changed, 318 insertions(+), 7 deletions(-) diff --git a/.ai/contexts/cli-session-state.md b/.ai/contexts/cli-session-state.md index 1ed31dd11..9f610788f 100644 --- a/.ai/contexts/cli-session-state.md +++ b/.ai/contexts/cli-session-state.md @@ -336,6 +336,50 @@ The Agents view (`.ai/contexts/bg-agents.md`) reads the same `jobId`, so `guardResume` can answer a `kind: 'bg'` session with an attach instead of a resume confirmation. +## Owner of a /clear transcript + +`/clear` does not continue the open transcript: the CLI opens a **new jsonl +under a new session id** in the same project folder, and writes into it only +the bookkeeping of the command (a `` record, then the +`/clear` record). Nothing in that file names the +session it replaced — no `forkedFrom`, and the `sessionId` on every record is +the new one. `detectSessionTransitions()` (`session-transitions.js`) used to +match new files by those two fields only, so after a `/clear` the open +terminal stayed on the old row and the new conversation, once its first prompt +made it indexable, showed up as a second, unattached row. + +The state file is what links the two: the CLI rewrites +`~/.claude/sessions/.json` with the new `sessionId` on `/clear` (checked +2026-10-05 on CLI 2.1.28x: live state files whose `sessionId` names a +transcript that starts with the `/clear` record). `clearOwner(newId, ptyPid)` +finds the live pid whose state file names `newId` and walks its parent chain +(`/proc//stat`) up to the PTY's pid. Verdicts: + +- `mine` — the CLI runs under this PTY: re-key, exactly like a fork + (`session-forked` to the renderer). +- `other` — another process (a CLI started outside Switchboard, another tab in + the same folder): not this session's file. +- `pending` — no live state file names `newId` yet (the jsonl can land before + the state file is rewritten): the file stays out of `knownJsonlFiles` and is + rechecked on the next flush, for at most 60 s from its mtime. +- `unknown` — the pid is found but its parent cannot be read (no `/proc`: + macOS, Windows). Matched only when this is the one live Claude PTY in the + folder, since `cwd` cannot tell two sessions apart (see "Matching a state + file" above); with two or more, nothing is re-keyed, the file is recorded + and never rechecked — retrying it would hand it to whichever session + outlives the others, not to its owner. + +`readNewSessionSignals()` recognises the file by its first user record that is +not local-command bookkeeping, through `classifyUserText()` — the caveat record +comes first and must be skipped, not taken as the file's first turn. + +The renderer re-keys as for a fork, and when the old id was not a pending +(transcript-less) row it adds one for the new id titled "New session": the +`/clear` transcript is not indexed until its first prompt (see +session-cache.md), so without it the open terminal would have no row at all +until then. The old conversation stays in the list as an ordinary, +resumable session — it is a real transcript on disk. + ## Canary tests `test/canary-*.test.js` is a convention this module introduces. A canary diff --git a/CHANGELOG.md b/CHANGELOG.md index 9701a1c8e..939137f99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc - When the Claude CLI reports a session's id in a different letter case, the subagents that session started or finished during a turn now show up when the turn ends. (#487) - A session that moves into a worktree while it runs stays in the sidebar, under its project, instead of disappearing while its terminal keeps running. A running session also stays listed when its transcript leaves its folder for any other reason. (#485) - Worktree folders from another host, or whose repository is not listed, show in the sidebar instead of being nested under the wrong folder or not shown at all. The worktrees of a repository hidden with Hide Project stay hidden. (#473) +- `/clear` in a session keeps the open terminal on its sidebar row, which now follows the new conversation, instead of listing that conversation as a separate session. The cleared conversation stays in the list as a past session. (#477) - Remote triggers refuse commands containing invisible format characters, default-ignorable characters or braille blanks, including joined emoji, emoji with variation selectors (such as hearts), soft hyphens and right-to-left marks. Fullwidth slash, exclamation and number-sign prefixes are refused too. (#440) ## v0.0.89 — 2026-10-04 diff --git a/cli-session-state.js b/cli-session-state.js index ccac6d390..abe269fc1 100644 --- a/cli-session-state.js +++ b/cli-session-state.js @@ -434,6 +434,32 @@ async function findLiveProcess(sessionId, { exclude = () => false } = {}) { return (await scanLiveProcesses(new Set([key]), exclude)).get(key) || null; } +// see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") +function clearOwner(sessionId, ptyPid) { + if (typeof sessionId !== 'string' || !sessionId || !Number.isInteger(ptyPid)) return 'pending'; + let names; + try { names = fs.readdirSync(dir); } catch { return 'pending'; } + let pid = null; + for (const name of names) { + if (!STATE_FILE_RE.test(name)) continue; + let raw; + try { raw = JSON.parse(fs.readFileSync(path.join(dir, name), 'utf8')); } catch { continue; } + if (!raw || raw.sessionId !== sessionId || !Number.isInteger(raw.pid) || raw.pid <= 0) continue; + if (!isProcessAlive(raw.pid)) continue; + pid = raw.pid; + break; + } + if (pid == null) return 'pending'; + let current = pid; + for (let depth = 0; depth < 64 && current && current > 1; depth++) { + if (current === ptyPid) return 'mine'; + const parent = readParentPid(current); + if (parent == null) return depth === 0 ? 'unknown' : 'other'; + current = parent; + } + return 'other'; +} + function ownProcessFilter(ptyPids) { const own = new Set(ptyPids()); return (pid) => own.has(pid) || descendsFromThisProcess(pid); @@ -480,6 +506,7 @@ module.exports = { liveElsewhere, liveElsewhereMany, liveElsewhereChecked, + clearOwner, MAX_LIVE_QUERY_IDS, MAX_PROBE_PIDS, probeProcStartWindows, diff --git a/main.js b/main.js index 491ee1e16..0a0c0611b 100644 --- a/main.js +++ b/main.js @@ -3104,6 +3104,7 @@ const sessionTransitions = require('./session-transitions'); sessionTransitions.init({ PROJECTS_DIR, activeSessions, getMainWindow: () => mainWindow, log, rekeyMcpServer, rekeyActivity: (fromId, toId) => activityReporter.rekey(fromId, toId), + clearOwner: (sessionId, ptyPid) => cliSessionState.clearOwner(sessionId, ptyPid), }); const { detectSessionTransitions } = sessionTransitions; diff --git a/public/app.js b/public/app.js index 6420ebab0..e8fb81b4b 100644 --- a/public/app.js +++ b/public/app.js @@ -510,6 +510,11 @@ window.api.onSessionForked((oldId, newId) => { if (pendingEntry) { pendingEntry.sessionId = newId; pendingSessions.set(newId, pendingEntry); + } else if (entry.session.projectPath) { + // see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") + const projectPath = entry.session.projectPath; + const session = { ...entry.session, sessionId: newId, summary: 'New session', firstPrompt: '', name: null, messageCount: 0 }; + pendingSessions.set(newId, { session, projectPath, folder: encodeProjectPath(projectPath) }); } sessionMap.delete(oldId); sessionMap.set(newId, entry.session); diff --git a/session-transitions.js b/session-transitions.js index ec0cf6ad7..7689597bb 100644 --- a/session-transitions.js +++ b/session-transitions.js @@ -1,6 +1,6 @@ const path = require('path'); const fs = require('fs'); -const { readSubagentMeta } = require('./read-session-file'); +const { readSubagentMeta, classifyUserText } = require('./read-session-file'); const { enabled: TRACE, trace } = require('./activity-trace'); const { SUBAGENT_LIVE_TTL_MS } = require('./public/subagent-timing'); @@ -8,7 +8,7 @@ const { SUBAGENT_LIVE_TTL_MS } = require('./public/subagent-timing'); * Fork detection for active PTY sessions. * Call init(ctx) once with shared context. */ -let PROJECTS_DIR, activeSessions, getMainWindow, log, rekeyMcpServer, rekeyActivity; +let PROJECTS_DIR, activeSessions, getMainWindow, log, rekeyMcpServer, rekeyActivity, clearOwner; function init(ctx) { PROJECTS_DIR = ctx.PROJECTS_DIR; @@ -17,6 +17,7 @@ function init(ctx) { log = ctx.log; rekeyMcpServer = ctx.rekeyMcpServer; rekeyActivity = ctx.rekeyActivity || (() => {}); + clearOwner = ctx.clearOwner || (() => 'pending'); } // --- Subagent spawn / completion detection --- @@ -309,6 +310,7 @@ function readNewSessionSignals(filePath) { let slug = null; let parentSessionId = null; let hasSnapshots = false; + let cleared = false; for (const line of lines) { const entry = JSON.parse(line); // Skip snapshot lines — they carry no fork/session signals @@ -317,16 +319,33 @@ function readNewSessionSignals(filePath) { if (entry.slug && !slug) slug = entry.slug; // --fork-session copies messages with original sessionId if (entry.sessionId && !parentSessionId) parentSessionId = entry.sessionId; + if (entry.type === 'user') { + const text = typeof entry.message?.content === 'string' ? entry.message.content : ''; + const kind = classifyUserText(text); + // see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") + if (kind.kind === 'skip') continue; + cleared = kind.kind === 'command' && kind.text === '/clear'; + } // Stop after finding a user or assistant message if (entry.type === 'user' || entry.type === 'assistant') break; } - return { forkedFrom, slug, parentSessionId, hasSnapshots }; + return { forkedFrom, slug, parentSessionId, hasSnapshots, cleared }; } catch { - return { forkedFrom: null, slug: null, parentSessionId: null, hasSnapshots: false }; + return { forkedFrom: null, slug: null, parentSessionId: null, hasSnapshots: false, cleared: false }; + } +} + +const CLEAR_OWNER_WAIT_MS = 60000; + +function isSoleClaudeIn(folder) { + let count = 0; + for (const session of activeSessions.values()) { + if (!session.exited && !session.isPlainTerminal && session.projectFolder === folder) count++; } + return count === 1; } -/** Detect fork transitions for active PTY sessions in a folder */ +/** Detect fork and /clear transitions for active PTY sessions in a folder */ function detectSessionTransitions(folder) { const folderPath = path.join(PROJECTS_DIR, folder); let currentFiles; @@ -405,12 +424,25 @@ function detectSessionTransitions(folder) { matched = true; } + const awaitingFork = Boolean(session.forkFrom && !session.realSessionId); + if (!matched && signals.cleared && !awaitingFork && newId !== (session.realSessionId || sessionId)) { + const owner = clearOwner(newId, session.pty && session.pty.pid); + if (owner === 'mine' || (owner === 'unknown' && isSoleClaudeIn(folder))) { + matched = true; + } else if (owner === 'pending') { + let fresh = false; + try { fresh = Date.now() - fs.statSync(newFilePath).mtimeMs < CLEAR_OWNER_WAIT_MS; } catch {} + if (fresh) emptyFiles.add(newFile); + } + log.info(`[detect] session=${sessionId} /clear file=${newId} owner=${owner} matched=${matched}`); + } + if (session.forkFrom && !matched) { log.info(`[detect] session=${sessionId} NO MATCH for newFile=${newId} forkFrom=${session.forkFrom} parentSessionId=${signals.parentSessionId||'null'} forkedFrom=${signals.forkedFrom||'null'}`); } if (matched) { - log.info(`[session-transition] ${sessionId} → ${newId} (fork)`); + log.info(`[session-transition] ${sessionId} → ${newId} (${signals.cleared && !awaitingFork ? 'clear' : 'fork'})`); session.knownJsonlFiles = new Set(currentFiles); session.realSessionId = newId; // Subagent scanning follows realSessionId into a different directory — diff --git a/test/cli-session-state.test.js b/test/cli-session-state.test.js index 245431346..540d1b9ac 100644 --- a/test/cli-session-state.test.js +++ b/test/cli-session-state.test.js @@ -535,3 +535,31 @@ test('ownProcessFilter is exported and claims our own PTY pids', () => { assert.equal(isOwn(78), false); } finally { fs.rmSync(dir, { recursive: true, force: true }); } }); + +// see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") +test('clearOwner: walks the state file pid up to the PTY pid', () => { + const dir = mkTmp(); + try { + const parents = { 300: 200, 200: 100, 100: 1, 400: 1 }; + cliSessionState.init({ + dir, activeSessions: new Map(), log: silentLog, + isProcessAlive: (pid) => pid !== 500, + readParentPid: (pid) => parents[pid] ?? null, + }); + writeState(dir, 300, { sessionId: 'cleared', status: 'idle' }); + writeState(dir, 400, { sessionId: 'elsewhere', status: 'idle' }); + writeState(dir, 500, { sessionId: 'dead', status: 'idle' }); + writeState(dir, 600, { sessionId: 'no-proc', status: 'idle' }); + + assert.equal(cliSessionState.clearOwner('cleared', 100), 'mine'); + assert.equal(cliSessionState.clearOwner('cleared', 300), 'mine', 'the PTY runs claude itself'); + assert.equal(cliSessionState.clearOwner('elsewhere', 100), 'other'); + assert.equal(cliSessionState.clearOwner('dead', 100), 'pending', 'a dead pid is no owner'); + assert.equal(cliSessionState.clearOwner('absent', 100), 'pending'); + assert.equal(cliSessionState.clearOwner('no-proc', 100), 'unknown', 'parent unreadable'); + assert.equal(cliSessionState.clearOwner('cleared', undefined), 'pending', 'no PTY pid'); + } finally { + cliSessionState.stop(); + fs.rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/test/session-transitions.test.js b/test/session-transitions.test.js index 6362719b4..ebcc0a6fe 100644 --- a/test/session-transitions.test.js +++ b/test/session-transitions.test.js @@ -727,7 +727,7 @@ test('a completed agent whose file is deleted is forgotten (knownSubagents stays }); /** Init the module with a capturing log and a caller-owned activeSessions map. */ -function setupForkDetection(projectsDir) { +function setupForkDetection(projectsDir, extra = {}) { const win = makeMockWindow(); const activeSessions = new Map(); const logLines = []; @@ -738,6 +738,7 @@ function setupForkDetection(projectsDir) { getMainWindow: () => win, log: { info: capture, debug: capture, warn: capture, error: capture }, rekeyMcpServer: () => {}, + ...extra, }); return { events: win._events, activeSessions, logLines }; } @@ -859,3 +860,175 @@ test('fork detection: the re-key drops the subagent scan state, so the new sessi cleanup(tmp); } }); + +// see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") +function writeClearTranscript(folderPath, id) { + const rec = (o) => JSON.stringify({ sessionId: id, ...o }); + fs.writeFileSync(path.join(folderPath, `${id}.jsonl`), [ + rec({ type: 'mode', mode: 'normal' }), + JSON.stringify({ type: 'file-history-snapshot', messageId: 'm1', snapshot: {} }), + rec({ type: 'user', isMeta: true, message: { role: 'user', content: 'Caveat: The messages below were generated by the user while running local commands.' } }), + rec({ type: 'user', message: { role: 'user', content: '/clear\n clear\n ' } }), + ].join('\n') + '\n', 'utf8'); +} + +test('/clear: the new transcript of the CLI under this PTY re-keys the session', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + const calls = []; + const { events, activeSessions } = setupForkDetection(tmp, { + clearOwner: (id, ptyPid) => { calls.push([id, ptyPid]); return 'mine'; }, + }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 4242 } })); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + + assert.deepEqual(calls, [['new-id', 4242]]); + assert.ok(!activeSessions.has('old-id')); + assert.equal(activeSessions.get('new-id').realSessionId, 'new-id'); + const forked = events.find(e => e.channel === 'session-forked'); + assert.equal(forked.payload, 'old-id'); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a transcript owned by another process is recorded and never re-keys', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + const { events, activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'other' }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 1 } })); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + + assert.ok(activeSessions.has('old-id')); + assert.ok(activeSessions.get('old-id').knownJsonlFiles.has('new-id.jsonl'), 'not rechecked'); + assert.equal(events.filter(e => e.channel === 'session-forked').length, 0); + } finally { + cleanup(tmp); + } +}); + +test('/clear: an owner not yet published is rechecked on the next flush, then matched', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + let owner = 'pending'; + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => owner }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 7 } })); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('old-id')); + assert.ok(!activeSessions.get('old-id').knownJsonlFiles.has('new-id.jsonl'), 'kept out of the known set'); + + owner = 'mine'; + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('new-id')); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a pending owner on a transcript older than the wait is given up', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'pending' }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 7 } })); + + writeClearTranscript(folderPath, 'new-id'); + const t = (Date.now() - 120_000) / 1000; + fs.utimesSync(path.join(folderPath, 'new-id.jsonl'), t, t); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.get('old-id').knownJsonlFiles.has('new-id.jsonl')); + } finally { + cleanup(tmp); + } +}); + +test('/clear: an unknown owner matches only the sole Claude PTY in the folder', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'unknown' }); + activeSessions.set('a', makePtySession(folder, { pty: { pid: 1 } })); + activeSessions.set('b', makePtySession(folder, { pty: { pid: 2 } })); + activeSessions.set('term', makePtySession(folder, { isPlainTerminal: true })); + + writeClearTranscript(folderPath, 'new-1'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('a') && activeSessions.has('b') && !activeSessions.has('new-1'), 'two candidates: no guess'); + + activeSessions.get('b').exited = true; + writeClearTranscript(folderPath, 'new-2'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('new-2') && !activeSessions.has('a'), 'one candidate: matched'); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a transcript whose first real turn is a prompt is not a /clear', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + let asked = false; + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => { asked = true; return 'mine'; } }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 1 } })); + + fs.writeFileSync(path.join(folderPath, 'new-id.jsonl'), + JSON.stringify({ sessionId: 'new-id', type: 'user', message: { role: 'user', content: 'hello /clear' } }) + '\n', 'utf8'); + sessionTransitions.detectSessionTransitions(folder); + assert.equal(asked, false); + assert.ok(activeSessions.has('old-id')); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a session that already completed its fork still follows a later /clear', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'mine' }); + activeSessions.set('forked-id', makePtySession(folder, { pty: { pid: 9 }, forkFrom: 'source-id', realSessionId: 'forked-id' })); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('new-id') && !activeSessions.has('forked-id')); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a session still waiting for its fork is not matched to a /clear file', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + let asked = false; + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => { asked = true; return 'mine'; } }); + activeSessions.set('temp-id', makePtySession(folder, { pty: { pid: 9 }, forkFrom: 'source-id' })); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.equal(asked, false); + } finally { + cleanup(tmp); + } +}); From b6b9294751ec85df775d37036460a5d880baf6ee Mon Sep 17 00:00:00 2001 From: abate Date: Thu, 8 Oct 2026 11:54:40 +0200 Subject: [PATCH 02/17] fix(sessions): follow /clear only on proof of ownership, and keep chains on it An owner that cannot be established (no /proc: macOS, Windows) was matched when one Claude PTY ran in the folder, so a claude run outside the app could be taken over; it is now never matched. A re-key kept no trace of the old id, so a trigger chain that sent /clear lost its terminal: the trigger context now resolves ids through the re-keys. A pending owner, or a new file whose first records are not written yet, is rechecked a second later while fresh, instead of waiting for another change in the folder. The snapshot-only fork matcher no longer takes a /clear file. A session with no transcript yet is left out of the saved set, which is saved again once its first prompt makes it real. --- .ai/contexts/cli-session-state.md | 28 +++++-- .ai/contexts/trigger-watcher.md | 15 +++- cli-session-state.js | 4 +- docs/session-restore.md | 8 ++ main.js | 1 + public/app.js | 3 + session-transitions.js | 58 +++++++++++---- test/clear-pending-persist.test.js | 78 ++++++++++++++++++++ test/cli-session-state.test.js | 1 + test/restore-live-elsewhere.test.js | 1 + test/restore-pending-persist.test.js | 1 + test/session-transitions.test.js | 105 ++++++++++++++++++++++++--- trigger-context.js | 19 +++-- 13 files changed, 278 insertions(+), 44 deletions(-) create mode 100644 test/clear-pending-persist.test.js diff --git a/.ai/contexts/cli-session-state.md b/.ai/contexts/cli-session-state.md index 9f610788f..df1984f58 100644 --- a/.ai/contexts/cli-session-state.md +++ b/.ai/contexts/cli-session-state.md @@ -360,14 +360,24 @@ finds the live pid whose state file names `newId` and walks its parent chain - `other` — another process (a CLI started outside Switchboard, another tab in the same folder): not this session's file. - `pending` — no live state file names `newId` yet (the jsonl can land before - the state file is rewritten): the file stays out of `knownJsonlFiles` and is - rechecked on the next flush, for at most 60 s from its mtime. + the state file is rewritten): the file stays out of `knownJsonlFiles`, and + `scheduleRecheck` runs the folder's detection again a second later, while + the file is less than 60 s old. Nothing else would: the state file is not in + the watched folder, and the transcript gets no other write before the first + prompt. A new file whose first records are not written yet is rechecked the + same way. - `unknown` — the pid is found but its parent cannot be read (no `/proc`: - macOS, Windows). Matched only when this is the one live Claude PTY in the - folder, since `cwd` cannot tell two sessions apart (see "Matching a state - file" above); with two or more, nothing is re-keyed, the file is recorded - and never rechecked — retrying it would hand it to whichever session - outlives the others, not to its owner. + macOS, Windows). Never matched, not even when this is the only Claude PTY in + the folder: a `claude` run outside the app in the same folder would be taken + over. The file is recorded and not rechecked, so on macOS and Windows a + `/clear` is followed only when the PTY's own pid is the CLI's. + +Re-keying keeps `oldId → newId` (`currentSessionId`), so a trigger chain that +sent `/clear` reaches the same terminal on its next step (see +trigger-watcher.md, "Re-keyed sessions"). A `/clear` transcript starts with a +`file-history-snapshot` record, so the "snapshot-only fork file" matcher +skips files whose first real turn is `/clear`; otherwise a session waiting for +its fork would take another session's `/clear` as its fork. `readNewSessionSignals()` recognises the file by its first user record that is not local-command bookkeeping, through `classifyUserText()` — the caveat record @@ -377,7 +387,9 @@ The renderer re-keys as for a fork, and when the old id was not a pending (transcript-less) row it adds one for the new id titled "New session": the `/clear` transcript is not indexed until its first prompt (see session-cache.md), so without it the open terminal would have no row at all -until then. The old conversation stays in the list as an ordinary, +until then. Like any pending row, it is left out of the saved working set +until that prompt (docs/session-restore.md, "A session with no transcript +yet"). The old conversation stays in the list as an ordinary, resumable session — it is a real transcript on disk. ## Canary tests diff --git a/.ai/contexts/trigger-watcher.md b/.ai/contexts/trigger-watcher.md index 8f13a7288..2cbe4ba14 100644 --- a/.ai/contexts/trigger-watcher.md +++ b/.ai/contexts/trigger-watcher.md @@ -57,6 +57,18 @@ 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**. +### Re-keyed sessions + +A fork or a `/clear` re-keys the open session in `activeSessions` under its new +id (`session-transitions.js`; see cli-session-state.md, "Owner of a /clear +transcript"), while a chain keeps the id it was given, and `/clear` is one of +the commands a chain may send. `createTriggerContext` takes `resolveSessionId` +(`sessionTransitions.currentSessionId` in main), and every lookup it makes +(`getPtyForSession`, `isSessionBusy`, `getComposerState`, `getTranscriptTurn`, +`getCliStatus`) goes through it, so the old id keeps reaching the same terminal +and the CLI status is read under the id the CLI now writes. Results still +carry the id the trigger named. + ### Session handle (2026-09-08, issue #220) `trigger-watcher.js` never touches `session.pty` or a raw pid. It writes and @@ -969,7 +981,8 @@ session, which has no local descriptor). No new watcher: it reuses the cache the CLI reacted. `idle` alone covers a turn too fast for a poll to see `busy`; `waiting` is a permission dialog our Enter opened. A spinner on the level probe, or a status that began earlier, proves nothing. Otherwise the - existing recovery applies (one bare ` `, only into a free composer, same + existing recovery applies (one bare ` +`, only into a free composer, same window), and the reaction is looked for again. Still nothing: `confirmed: false`. - **The recovery Enter is never written while the descriptor reads `waiting` diff --git a/cli-session-state.js b/cli-session-state.js index abe269fc1..636f8efb7 100644 --- a/cli-session-state.js +++ b/cli-session-state.js @@ -439,12 +439,14 @@ function clearOwner(sessionId, ptyPid) { if (typeof sessionId !== 'string' || !sessionId || !Number.isInteger(ptyPid)) return 'pending'; let names; try { names = fs.readdirSync(dir); } catch { return 'pending'; } + const key = sessionId.toLowerCase(); let pid = null; for (const name of names) { if (!STATE_FILE_RE.test(name)) continue; let raw; try { raw = JSON.parse(fs.readFileSync(path.join(dir, name), 'utf8')); } catch { continue; } - if (!raw || raw.sessionId !== sessionId || !Number.isInteger(raw.pid) || raw.pid <= 0) continue; + if (!raw || typeof raw.sessionId !== 'string' || raw.sessionId.toLowerCase() !== key) continue; + if (!Number.isInteger(raw.pid) || raw.pid <= 0) continue; if (!isProcessAlive(raw.pid)) continue; pid = raw.pid; break; diff --git a/docs/session-restore.md b/docs/session-restore.md index f5f746240..64d066398 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -21,6 +21,14 @@ Each time a session is opened or closed, Switchboard saves the open set — each session's id and project, and which one was active — in its global settings (`openWorkingSet`). +### A session with no transcript yet + +A session started with **+**, or one just cleared with `/clear`, has no +conversation until its first prompt: the CLI has nothing to resume, and the +index does not list it. It is left out of the saved set, so the next start +does not report it as "not in the index". Once its first prompt makes it a +real session, the set is saved again with it. + ## What restore does Restore is a respawn, not a reattach: a session's process is a child of the diff --git a/main.js b/main.js index 0a0c0611b..750f09f25 100644 --- a/main.js +++ b/main.js @@ -3424,6 +3424,7 @@ if (!gotSingleInstanceLock) { require('./trigger-watcher').start(createTriggerContext({ activeSessions, log, getCliStatus: (id) => cliSessionState.getStatus(id), projectsDir: PROJECTS_DIR, getLiveDescriptor: (id) => cliSessionState.findLiveProcess(id), + resolveSessionId: (id) => sessionTransitions.currentSessionId(id), get remote() { const settings = getSetting('global') || {}; if ((settings.remoteTriggers ?? SETTING_DEFAULTS.remoteTriggers) === true) { diff --git a/public/app.js b/public/app.js index e8fb81b4b..18e0cc245 100644 --- a/public/app.js +++ b/public/app.js @@ -181,6 +181,8 @@ function persistWorkingSet() { if (entry.session.type === 'terminal') continue; // exclude plain shells if (entry.attach) continue; // attach tabs are not restored if (entry.closed) continue; + // see docs/session-restore.md ("A session with no transcript yet") + if (pendingSessions.has(sessionId)) continue; set.push({ sessionId, projectPath: entry.session.projectPath, @@ -1162,6 +1164,7 @@ async function loadProjects({ resort = false } = {}) { const realExists = allProjects.some(p => p.sessions.some(s => s.sessionId === sid)); if (realExists) { pendingSessions.delete(sid); + if (openSessions.has(sid)) schedulePersistWorkingSet(); } else { hasReinjected = true; // Still pending — re-inject into cached data diff --git a/session-transitions.js b/session-transitions.js index 7689597bb..f62fab6a0 100644 --- a/session-transitions.js +++ b/session-transitions.js @@ -8,7 +8,7 @@ const { SUBAGENT_LIVE_TTL_MS } = require('./public/subagent-timing'); * Fork detection for active PTY sessions. * Call init(ctx) once with shared context. */ -let PROJECTS_DIR, activeSessions, getMainWindow, log, rekeyMcpServer, rekeyActivity, clearOwner; +let PROJECTS_DIR, activeSessions, getMainWindow, log, rekeyMcpServer, rekeyActivity, clearOwner, setTimeoutFn; function init(ctx) { PROJECTS_DIR = ctx.PROJECTS_DIR; @@ -18,6 +18,34 @@ function init(ctx) { rekeyMcpServer = ctx.rekeyMcpServer; rekeyActivity = ctx.rekeyActivity || (() => {}); clearOwner = ctx.clearOwner || (() => 'pending'); + setTimeoutFn = ctx.setTimeout || setTimeout; + rekeyed.clear(); + for (const timer of recheckTimers.values()) clearTimeout(timer); + recheckTimers.clear(); +} + +// see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") +const rekeyed = new Map(); +const MAX_REKEY_HOPS = 32; + +/** The id a session is keyed under now, following the re-keys of forks and /clear. */ +function currentSessionId(sessionId) { + let id = sessionId; + for (let hop = 0; hop < MAX_REKEY_HOPS && rekeyed.has(id); hop++) id = rekeyed.get(id); + return id; +} + +const recheckTimers = new Map(); +const RECHECK_MS = 1000; + +function scheduleRecheck(folder) { + if (recheckTimers.has(folder)) return; + const timer = setTimeoutFn(() => { + recheckTimers.delete(folder); + detectSessionTransitions(folder); + }, RECHECK_MS); + if (timer && typeof timer.unref === 'function') timer.unref(); + recheckTimers.set(folder, timer); } // --- Subagent spawn / completion detection --- @@ -337,12 +365,8 @@ function readNewSessionSignals(filePath) { const CLEAR_OWNER_WAIT_MS = 60000; -function isSoleClaudeIn(folder) { - let count = 0; - for (const session of activeSessions.values()) { - if (!session.exited && !session.isPlainTerminal && session.projectFolder === folder) count++; - } - return count === 1; +function isFresh(filePath) { + try { return Date.now() - fs.statSync(filePath).mtimeMs < CLEAR_OWNER_WAIT_MS; } catch { return false; } } /** Detect fork and /clear transitions for active PTY sessions in a folder */ @@ -374,6 +398,7 @@ function detectSessionTransitions(folder) { if (newFiles.length === 0) continue; const emptyFiles = new Set(); // files with no signals yet (still being written) + let recheck = false; for (const newFile of newFiles) { const newFilePath = path.join(folderPath, newFile); @@ -397,6 +422,7 @@ function detectSessionTransitions(folder) { log.info(`[detect] session=${sessionId} archiving stale empty file=${newId}`); } else { emptyFiles.add(newFile); + if (isFresh(newFilePath)) recheck = true; } continue; } @@ -419,20 +445,20 @@ function detectSessionTransitions(folder) { if (!matched && session.forkFrom && signals.parentSessionId === session.forkFrom && newId !== session.forkFrom) { matched = true; } - // Fork file with only snapshots — no user turn yet, but this session is waiting for a fork - if (!matched && signals.hasSnapshots && session.forkFrom && !session.realSessionId) { + // Fork file with only snapshots — no user turn yet, but this session is waiting for a fork. + // A /clear transcript also starts with a snapshot; it is never a fork. + if (!matched && signals.hasSnapshots && !signals.cleared && session.forkFrom && !session.realSessionId) { matched = true; } const awaitingFork = Boolean(session.forkFrom && !session.realSessionId); if (!matched && signals.cleared && !awaitingFork && newId !== (session.realSessionId || sessionId)) { const owner = clearOwner(newId, session.pty && session.pty.pid); - if (owner === 'mine' || (owner === 'unknown' && isSoleClaudeIn(folder))) { + if (owner === 'mine') { matched = true; - } else if (owner === 'pending') { - let fresh = false; - try { fresh = Date.now() - fs.statSync(newFilePath).mtimeMs < CLEAR_OWNER_WAIT_MS; } catch {} - if (fresh) emptyFiles.add(newFile); + } else if (owner === 'pending' && isFresh(newFilePath)) { + emptyFiles.add(newFile); + recheck = true; } log.info(`[detect] session=${sessionId} /clear file=${newId} owner=${owner} matched=${matched}`); } @@ -452,6 +478,7 @@ function detectSessionTransitions(folder) { session._subFileList = null; activeSessions.delete(sessionId); activeSessions.set(newId, session); + rekeyed.set(sessionId, newId); // Re-key MCP server to match new session ID rekeyMcpServer(sessionId, newId); rekeyActivity(sessionId, newId); @@ -468,8 +495,9 @@ function detectSessionTransitions(folder) { const updated = new Set(currentFiles); for (const f of emptyFiles) updated.delete(f); session.knownJsonlFiles = updated; + if (recheck) scheduleRecheck(folder); } } -module.exports = { init, detectSessionTransitions, detectSubagentTransitions }; +module.exports = { init, detectSessionTransitions, detectSubagentTransitions, currentSessionId }; diff --git a/test/clear-pending-persist.test.js b/test/clear-pending-persist.test.js new file mode 100644 index 000000000..76dbed617 --- /dev/null +++ b/test/clear-pending-persist.test.js @@ -0,0 +1,78 @@ +'use strict'; + +// see docs/session-restore.md ("A session with no transcript yet") + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { setupSidebarDom } = require('./dom-setup'); +const { loadAppFunctions } = require('./app-source'); + +const PROJECT = { folder: '-srv-app', projectPath: '/srv/app', sessions: [] }; + +function setup() { + const ctx = setupSidebarDom(); + const { window } = ctx; + const settings = { global: { openWorkingSet: [] } }; + let persists = 0; + let projects = [PROJECT]; + Object.assign(window, { + loadingStatus: window.document.createElement('div'), + reportActivityFocus: () => {}, + reportActivityTitles: () => {}, + pollActiveSessions: async () => {}, + refreshSidebar: () => {}, + renderDefaultStatus: () => {}, + encodeProjectPath: (p) => p.replace(/\//g, '-'), + schedulePersistWorkingSet: () => { persists += 1; }, + pendingRestoreEntries: () => [], + }); + window.api = { + getProjects: async () => JSON.parse(JSON.stringify(projects)), + getActiveTerminals: async () => [], + getSetting: async (key) => JSON.parse(JSON.stringify(settings[key] || null)), + setSetting: async (key, value) => { settings[key] = JSON.parse(JSON.stringify(value)); }, + }; + window.cachedProjects = []; + window.cachedAllProjects = []; + const fns = loadAppFunctions(ctx.context, { + functions: ['dedup', 'loadProjects', 'persistWorkingSet'], + declarations: ['_persistChain', 'skippedWorkingSetEntries'], + }); + const open = (sessionId, { pending }) => { + const session = { sessionId, projectPath: '/srv/app', summary: 'New session' }; + window.openSessions.set(sessionId, { session, closed: false }); + if (pending) window.pendingSessions.set(sessionId, { session, projectPath: '/srv/app', folder: '-srv-app' }); + }; + return { + ...fns, open, settings, + persists: () => persists, + setProjects: (list) => { projects = list; }, + saved: () => settings.global.openWorkingSet.map((e) => e.sessionId), + destroy: () => ctx.destroy(), + }; +} + +test('a session with no transcript yet, after /clear or from +, is not saved: there is nothing to resume', async () => { + const h = setup(); + try { + h.open('real', { pending: false }); + h.open('cleared', { pending: true }); + await h.persistWorkingSet(); + assert.deepEqual(h.saved(), ['real']); + } finally { h.destroy(); } +}); + +test('its first prompt makes it real, and the set is saved again with it', async () => { + const h = setup(); + try { + h.open('cleared', { pending: true }); + await h.loadProjects(); + assert.equal(h.persists(), 0, 'still pending'); + h.setProjects([{ ...PROJECT, sessions: [{ sessionId: 'cleared', projectPath: '/srv/app' }] }]); + await h.loadProjects(); + assert.equal(h.persists(), 1); + await h.persistWorkingSet(); + assert.deepEqual(h.saved(), ['cleared']); + } finally { h.destroy(); } +}); diff --git a/test/cli-session-state.test.js b/test/cli-session-state.test.js index 540d1b9ac..5f0b60410 100644 --- a/test/cli-session-state.test.js +++ b/test/cli-session-state.test.js @@ -552,6 +552,7 @@ test('clearOwner: walks the state file pid up to the PTY pid', () => { writeState(dir, 600, { sessionId: 'no-proc', status: 'idle' }); assert.equal(cliSessionState.clearOwner('cleared', 100), 'mine'); + assert.equal(cliSessionState.clearOwner('CLEARED', 100), 'mine', 'ids compare in any letter case, as elsewhere since #487'); assert.equal(cliSessionState.clearOwner('cleared', 300), 'mine', 'the PTY runs claude itself'); assert.equal(cliSessionState.clearOwner('elsewhere', 100), 'other'); assert.equal(cliSessionState.clearOwner('dead', 100), 'pending', 'a dead pid is no owner'); diff --git a/test/restore-live-elsewhere.test.js b/test/restore-live-elsewhere.test.js index 77fb7057c..c38be5d0b 100644 --- a/test/restore-live-elsewhere.test.js +++ b/test/restore-live-elsewhere.test.js @@ -63,6 +63,7 @@ function setup({ savedSet, liveIds, batchFails = false }) { vm.runInContext(read('resume-guard.js'), ctx); vm.runInContext(` var openSessions = new Map(); + var pendingSessions = new Map(); var sessionMap = new Map(); var activeSessionId = null; var restoringWorkingSet = false; diff --git a/test/restore-pending-persist.test.js b/test/restore-pending-persist.test.js index a62857c96..1fd0cf4b3 100644 --- a/test/restore-pending-persist.test.js +++ b/test/restore-pending-persist.test.js @@ -29,6 +29,7 @@ function setup() { vm.runInContext(` var openSessions = new Map([['opened', { session: { projectPath: '/p' }, closed: false }]]); var sessionMap = new Map(); + var pendingSessions = new Map(); var activeSessionId = 'opened'; var _persistChain = Promise.resolve(); var RESTORE_STAGGER_MS = 0; diff --git a/test/session-transitions.test.js b/test/session-transitions.test.js index ebcc0a6fe..72887e55a 100644 --- a/test/session-transitions.test.js +++ b/test/session-transitions.test.js @@ -956,25 +956,20 @@ test('/clear: a pending owner on a transcript older than the wait is given up', } }); -test('/clear: an unknown owner matches only the sole Claude PTY in the folder', () => { +test('/clear: an owner that cannot be established is never matched, even by the only Claude PTY in the folder', () => { const tmp = mkTmp(); try { const folder = 'proj'; const folderPath = path.join(tmp, folder); fs.mkdirSync(folderPath, { recursive: true }); - const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'unknown' }); + const { events, activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'unknown' }); activeSessions.set('a', makePtySession(folder, { pty: { pid: 1 } })); - activeSessions.set('b', makePtySession(folder, { pty: { pid: 2 } })); - activeSessions.set('term', makePtySession(folder, { isPlainTerminal: true })); writeClearTranscript(folderPath, 'new-1'); sessionTransitions.detectSessionTransitions(folder); - assert.ok(activeSessions.has('a') && activeSessions.has('b') && !activeSessions.has('new-1'), 'two candidates: no guess'); - - activeSessions.get('b').exited = true; - writeClearTranscript(folderPath, 'new-2'); - sessionTransitions.detectSessionTransitions(folder); - assert.ok(activeSessions.has('new-2') && !activeSessions.has('a'), 'one candidate: matched'); + assert.ok(activeSessions.has('a') && !activeSessions.has('new-1'), 'a CLI outside the app may own it'); + assert.ok(activeSessions.get('a').knownJsonlFiles.has('new-1.jsonl'), 'recorded, not retried'); + assert.equal(events.filter(e => e.channel === 'session-forked').length, 0); } finally { cleanup(tmp); } @@ -1022,12 +1017,100 @@ test('/clear: a session still waiting for its fork is not matched to a /clear fi const folder = 'proj'; fs.mkdirSync(path.join(tmp, folder), { recursive: true }); let asked = false; - const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => { asked = true; return 'mine'; } }); + const { events, activeSessions } = setupForkDetection(tmp, { clearOwner: () => { asked = true; return 'mine'; } }); activeSessions.set('temp-id', makePtySession(folder, { pty: { pid: 9 }, forkFrom: 'source-id' })); writeClearTranscript(path.join(tmp, folder), 'new-id'); sessionTransitions.detectSessionTransitions(folder); assert.equal(asked, false); + assert.ok(activeSessions.has('temp-id') && !activeSessions.has('new-id'), 'nor taken as its fork, though it starts with a snapshot'); + assert.equal(events.filter(e => e.channel === 'session-forked').length, 0); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a pending owner is rechecked on its own, without waiting for another change in the folder', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + const timers = []; + let owner = 'pending'; + const { activeSessions } = setupForkDetection(tmp, { + clearOwner: () => owner, + setTimeout: (fn, ms) => { timers.push({ fn, ms }); return { unref() {} }; }, + }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 7 } })); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + sessionTransitions.detectSessionTransitions(folder); + assert.equal(timers.length, 1, 'one recheck per folder at a time'); + owner = 'mine'; + timers.shift().fn(); + assert.ok(activeSessions.has('new-id')); + assert.equal(timers.length, 0, 'nothing left to wait for'); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a new transcript whose first records are not written yet is rechecked on its own while it is fresh', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const timers = []; + const { activeSessions } = setupForkDetection(tmp, { + clearOwner: () => 'mine', + setTimeout: (fn) => { timers.push(fn); return { unref() {} }; }, + }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 7 } })); + + fs.writeFileSync(path.join(folderPath, 'new-id.jsonl'), '{"type":"mo', 'utf8'); + sessionTransitions.detectSessionTransitions(folder); + assert.equal(timers.length, 1); + writeClearTranscript(folderPath, 'new-id'); + timers.shift()(); + assert.ok(activeSessions.has('new-id')); + + fs.writeFileSync(path.join(folderPath, 'old-partial.jsonl'), '{"type":"mo', 'utf8'); + const t = (Date.now() - 120_000) / 1000; + fs.utimesSync(path.join(folderPath, 'old-partial.jsonl'), t, t); + sessionTransitions.detectSessionTransitions(folder); + assert.equal(timers.length, 0, 'an old one waits for the next change in the folder, as before'); + } finally { + cleanup(tmp); + } +}); + +test('/clear: the old id still leads to the session, so a trigger chain that sent /clear can go on', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + fs.mkdirSync(path.join(tmp, folder), { recursive: true }); + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'mine' }); + const pty = { pid: 4242, write() {} }; + activeSessions.set('old-id', makePtySession(folder, { pty, cwd: '/w' })); + const { createTriggerContext } = require('../trigger-context'); + const ctx = createTriggerContext({ + activeSessions, log: { info() {}, warn() {}, error() {}, debug() {} }, + getCliStatus: (id) => ({ status: 'idle', id }), + resolveSessionId: (id) => sessionTransitions.currentSessionId(id), + }); + + writeClearTranscript(path.join(tmp, folder), 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + writeClearTranscript(path.join(tmp, folder), 'newer-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('newer-id')); + + assert.equal(sessionTransitions.currentSessionId('old-id'), 'newer-id'); + assert.equal(ctx.getPtyForSession('old-id').ptyProcess, pty); + assert.equal(ctx.getCliStatus('old-id').id, 'newer-id', 'the status is read under the id the CLI writes now'); + assert.equal(sessionTransitions.currentSessionId('never-seen'), 'never-seen'); } finally { cleanup(tmp); } diff --git a/trigger-context.js b/trigger-context.js index 567c30a95..d8cdf56f6 100644 --- a/trigger-context.js +++ b/trigger-context.js @@ -34,10 +34,12 @@ function createLocalSessionHandle(ptyProcess) { */ function createTriggerContext(deps) { const { activeSessions, log, isPtyAlive, getCliStatus, getLiveDescriptor, projectsDir } = deps; + // see .ai/contexts/trigger-watcher.md, "Re-keyed sessions" + const resolve = deps.resolveSessionId || ((id) => id); const ctx = { log, getPtyForSession(sessionId) { - const session = activeSessions.get(sessionId); + const session = activeSessions.get(resolve(sessionId)); if (!session || session.exited) return null; const handle = (session.host == null) ? createLocalSessionHandle(session.pty) @@ -46,18 +48,18 @@ function createTriggerContext(deps) { return { ptyProcess: session.pty, cwd: session.cwd, handle }; }, isSessionBusy(sessionId) { - const session = activeSessions.get(sessionId); + const session = activeSessions.get(resolve(sessionId)); return session ? !!session._cliBusy : false; }, getComposerState(sessionId) { - const session = activeSessions.get(sessionId); + const session = activeSessions.get(resolve(sessionId)); if (!session || session.exited || !session.composerState) return null; const { pending, lastInputAt } = session.composerState; return { pending, lastInputAt }; }, }; if (isPtyAlive) ctx.isPtyAlive = isPtyAlive; - if (getLiveDescriptor) ctx.getLiveDescriptor = (sessionId) => getLiveDescriptor(sessionId); + if (getLiveDescriptor) ctx.getLiveDescriptor = (sessionId) => getLiveDescriptor(resolve(sessionId)); if ('remote' in deps) Object.defineProperty(ctx, 'remote', { get() { const remote = deps.remote; @@ -82,9 +84,9 @@ function createTriggerContext(deps) { const reader = createTranscriptTurnReader(); const readPaths = new Map(); ctx.getTranscriptTurn = (sessionId, compactionCursor) => { - const session = activeSessions.get(sessionId); + const session = activeSessions.get(resolve(sessionId)); if (!session || session.exited || session.host != null || !session.projectFolder) return null; - const id = session.realSessionId || sessionId; + const id = session.realSessionId || resolve(sessionId); const filePath = path.join(projectsDir, session.projectFolder, id + '.jsonl'); readPaths.set(sessionId, filePath); return reader.read(filePath, compactionCursor); @@ -98,9 +100,10 @@ function createTriggerContext(deps) { } if (getCliStatus) { ctx.getCliStatus = (sessionId) => { - const session = activeSessions.get(sessionId); + const id = resolve(sessionId); + const session = activeSessions.get(id); if (!session || session.host != null) return undefined; - return getCliStatus(sessionId); + return getCliStatus(id); }; } return ctx; From d730f48d041308d88e8e5e9967fda63a4c8f370b Mon Sep 17 00:00:00 2001 From: abate Date: Mon, 5 Oct 2026 16:58:57 +0200 Subject: [PATCH 03/17] (sandbox): opt-in writable skills and agents Skills and agents are read-only in a sandboxed session because an unsandboxed claude loads them later. Some workflows need the session to write them, so SWITCHBOARD_SANDBOX_RW_SKILLS=1 and SWITCHBOARD_SANDBOX_RW_AGENTS=1 (Pre-launch Command) make them session state in ~/.claude and every bound .claude. Both stay off by default; docs/sandbox.md states the protection given up. --- CHANGELOG.md | 1 + docs/sandbox.md | 28 +++++++++++++++++++++++++- scripts/claude-sandbox.sh | 14 +++++++++++++ test/sandbox-wrapper.test.js | 39 ++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9701a1c8e..f79e1d5f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ### New - An Agents view lists the sessions the Claude daemon runs in the background (`claude --bg`) and the interactive sessions running outside Switchboard, grouped by state or by project and worktree. Open it from the people icon in the sidebar or with Ctrl+Shift+A (Cmd+Shift+A on macOS); attach to a live one by double click, stop, respawn or delete one, and start a new one with New agent. A background session that is running shows a `bg` badge in the sidebar and is attached instead of resumed. (#374) +- With Sandbox on, `SWITCHBOARD_SANDBOX_RW_SKILLS=1` or `SWITCHBOARD_SANDBOX_RW_AGENTS=1` in the Pre-launch Command lets the session create and edit skills or agents, in `~/.claude` and the project's `.claude`. Both are off by default: what the session writes there is loaded, and run, by later sessions outside the sandbox. (#483) - **Archive folder** on a project header opens a dialog that archives the folder's sessions and disables its enabled schedules, each optional, and hides the folder with its worktrees. The folder comes back, with its settings, when you add it again or a new session starts in it, and then offers to turn back on the schedules the archive disabled. (#473) ### Changed - New scheduled task runs appear under a header named after their schedule, even with only one visible run. Schedules that use the same prompt stay separate; older runs keep their existing grouping. An old expansion choice is inherited only when it identifies one schedule across projects and hosts. (#489) diff --git a/docs/sandbox.md b/docs/sandbox.md index bee6ed721..8370e1f5d 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -137,7 +137,8 @@ with its listed state mounted back read-write on top. Only the state the CLI writes during a session is read-write; every other entry is read-only, whatever it is — settings, hooks, commands, agents, skills, plugins, `keybindings.json`, `CLAUDE.md`, `rules`, `output-styles`, a -status-line script with no extension, a directory a future CLI version adds. +status-line script with no extension, a directory a future CLI version adds. Skills and agents can be made +writable on request: see [Writable skills and agents](#writable-skills-and-agents). | Entry | Inside the sandbox | |---|---| @@ -145,6 +146,7 @@ status-line script with no extension, a directory a future CLI version adds. | In `~/.claude`: `projects` | read-only, except the session's own transcript folder, `projects/`, which is read-write; see [Schedules](#schedules) | | In `~/.claude`: `shell-snapshots`, `session-env`, `backups`, `state` | an empty private tmpfs. Other sessions source the shell snapshots and session hooks kept there before each Bash command, and the CLI offers the backups there as what to copy back over a broken `~/.claude.json`; the session gets its own | | In a project's `.claude`: `worktrees`, `agent-memory`, `agent-memory-local` | read-write | +| `skills` in `~/.claude` and in a project's `.claude`, only with `SWITCHBOARD_SANDBOX_RW_SKILLS=1`; `agents` likewise, only with `SWITCHBOARD_SANDBOX_RW_AGENTS=1` | read-write; see [Writable skills and agents](#writable-skills-and-agents) | | Any other file or directory | read-only | | A symbolic link to one of the read-write entries of `~/.claude` (`todos` kept on another disk, say) | recreated as the same link on the tmpfs, and its target bound read-write at its own path so the link resolves — only when the target is a directory (a file, for `*.json`, `*.jsonl` and `.last-*`) of the same name, and neither `$HOME` nor a parent of it. Any other target is refused | | A symbolic link to one of the read-write entries of a project's `.claude` | left as it is and not followed: a repository can carry such a link, so it resolves only to what the sandbox sees anyway | @@ -179,6 +181,27 @@ A `.claude` that is itself a symbolic link is refused: the link lives in a writable directory, and the session could replace it with a directory of its own. +### Writable skills and agents + +Skills and agents are read-only by default: an unsandboxed `claude` started +later loads them, and a skill can carry scripts it tells Claude to run. To +write them from a sandboxed session, put `SWITCHBOARD_SANDBOX_RW_SKILLS=1`, +`SWITCHBOARD_SANDBOX_RW_AGENTS=1` or both in the session's Pre-launch Command. +Each makes that directory read-write in `~/.claude` and in every bound +directory's `.claude`, as session state: + +- `~/.claude/skills` or `~/.claude/agents` is created before launch when + missing. In a project's `.claude`, which stays read-only, a missing one cannot + be created from inside the sandbox: create it once outside. +- A `~/.claude/skills` or `~/.claude/agents` that is a symbolic link is + followed like the other state links: only to a directory of the same name, + elsewhere, and never to `$HOME` or a parent of it. Links inside it are no + longer followed and protected. + +This gives up part of the protection: what the session writes there is loaded, +and its scripts run, by every later session outside the sandbox, other +projects' included for `~/.claude`. + ### `~/.claude.json` This file holds the MCP server definitions, and the CLI rewrites it throughout @@ -341,6 +364,9 @@ whatever they hold. Pre-launch Command, the rootless podman API socket is bound, and through it the session can start a container with any host path mounted (`podman run -v $HOME:/h …`): enabling it gives up the filesystem boundary. +- **Skills and agents, when enabled.** With `SWITCHBOARD_SANDBOX_RW_SKILLS=1` + or `SWITCHBOARD_SANDBOX_RW_AGENTS=1`, see + [Writable skills and agents](#writable-skills-and-agents). - **Session status.** `~/.claude/sessions` is read-write, and a sandboxed CLI records its PID as seen inside its PID namespace, so Switchboard's check of whether a session is live elsewhere can be misled for a sandboxed session. diff --git a/scripts/claude-sandbox.sh b/scripts/claude-sandbox.sh index a0848ee06..86812e0fc 100755 --- a/scripts/claude-sandbox.sh +++ b/scripts/claude-sandbox.sh @@ -18,6 +18,8 @@ # entry of ~/.claude and of each bound directory's .claude # that is not listed session state, a private copy of # ~/.claude.json, each repository's config and hooks +# (skills and agents become session state with +# $SWITCHBOARD_SANDBOX_RW_SKILLS=1 / $SWITCHBOARD_SANDBOX_RW_AGENTS=1) # network: shared with the host — Claude needs the API, and the # Switchboard IDE bridge listens on localhost # @@ -104,6 +106,18 @@ PROJECT_STATE_ENTRIES=(worktrees agent-memory agent-memory-local) # Created in ~/.claude before launch when missing, so that they are mounted # from the host rather than left to the tmpfs. USER_PRECREATED_DIRS=(projects todos statsig file-history sessions plans tasks ide) +# Opt-in: an unsandboxed claude later loads what is written there; see +# docs/sandbox.md, "Writable skills and agents". +if [ "${SWITCHBOARD_SANDBOX_RW_SKILLS:-0}" = "1" ]; then + USER_STATE_ENTRIES+=(skills) + PROJECT_STATE_ENTRIES+=(skills) + USER_PRECREATED_DIRS+=(skills) +fi +if [ "${SWITCHBOARD_SANDBOX_RW_AGENTS:-0}" = "1" ]; then + USER_STATE_ENTRIES+=(agents) + PROJECT_STATE_ENTRIES+=(agents) + USER_PRECREATED_DIRS+=(agents) +fi # Project directory plus whatever Switchboard forwarded. These must already # exist — creating a mistyped "Additional Directory" on the host would be worse diff --git a/test/sandbox-wrapper.test.js b/test/sandbox-wrapper.test.js index 46b794e0c..e98c1a769 100644 --- a/test/sandbox-wrapper.test.js +++ b/test/sandbox-wrapper.test.js @@ -473,6 +473,45 @@ test('sandbox wrapper: ~/.claude is a private tmpfs, its state bound back read-w } }); +test('sandbox wrapper: skills and agents are read-write only with SWITCHBOARD_SANDBOX_RW_SKILLS=1 and SWITCHBOARD_SANDBOX_RW_AGENTS=1', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + fs.mkdirSync(path.join(dir, 'skills'), { recursive: true }); + const dot = path.join(rig.proj, '.claude'); + for (const d of ['skills', 'agents']) fs.mkdirSync(path.join(dot, d), { recursive: true }); + const deep = path.join(rig.proj, 'deep.md'); + fs.writeFileSync(deep, ''); + fs.symlinkSync(deep, path.join(dir, 'skills', 'x')); + + const off = rig.run(['--version']); + assert.equal(off.status, 0, off.stderr); + let ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(mountAt(ops, path.join(dir, 'skills'))?.op, '--ro-bind', 'skills must be read-only by default'); + assert.equal(mountAt(ops, path.join(dot, 'agents')), undefined, 'project agents stay under the read-only .claude'); + assert.equal(mountAt(ops, deep)?.op, '--ro-bind'); + + const skillsOnly = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }); + assert.equal(skillsOnly.status, 0, skillsOnly.stderr); + ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(mountAt(ops, path.join(dir, 'skills'))?.op, '--bind'); + assert.equal(mountAt(ops, path.join(dot, 'skills'))?.op, '--bind'); + assert.notEqual(mountAt(ops, path.join(dir, 'agents'))?.op, '--bind', 'each flag covers only its own directory'); + assert.equal(mountAt(ops, path.join(dot, 'agents')), undefined); + assert.equal(mountAt(ops, deep), undefined, 'links inside a writable entry are not followed'); + + const both = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1', SWITCHBOARD_SANDBOX_RW_AGENTS: '1' }); + assert.equal(both.status, 0, both.stderr); + ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(mountAt(ops, path.join(dir, 'agents'))?.op, '--bind'); + assert.equal(mountAt(ops, path.join(dot, 'agents'))?.op, '--bind'); + assert.ok(fs.statSync(path.join(dir, 'agents')).isDirectory(), 'a missing ~/.claude/agents is created on the host'); + } finally { + rig.cleanup(); + } + }); + test('sandbox wrapper: a symlinked ~/.claude entry is recreated as a link, and its target is read-only wherever the sandbox could write it', { skip: !LINUX && 'linux only' }, () => { const rig = makeRig({ recordArgs: true }); From ce289fcf97025d533471ee8ba19d5d120eeaa8e5 Mon Sep 17 00:00:00 2001 From: abate Date: Wed, 7 Oct 2026 16:11:59 +0200 Subject: [PATCH 04/17] fix(sandbox): protect git and .claude inside opted-in skills and agents Review of #483: with SWITCHBOARD_SANDBOX_RW_SKILLS/AGENTS, a skills or agents directory linked to a repository elsewhere was bound writable without the protections other writable roots get, so the session could write that repository's hooks or a settings file linked into it. - An opted-in link target is a writable root, so files linked into it from ~/.claude are mounted read-only. - The tree under an opted-in entry, real or linked, gets the same .git and .claude protection as a bound directory, without creating .claude. - The Sandbox tooltip names the opt-in among what is not isolated. - The test rig drops inherited SWITCHBOARD_SANDBOX_* variables, which leaked into the wrapper when the suite ran from a sandboxed session. --- docs/sandbox.md | 12 ++++-- public/index.html | 2 +- scripts/claude-sandbox.sh | 60 +++++++++++++++------------ test/sandbox-wrapper.test.js | 78 +++++++++++++++++++++++++++++++++++- 4 files changed, 120 insertions(+), 32 deletions(-) diff --git a/docs/sandbox.md b/docs/sandbox.md index 8370e1f5d..54d522223 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -20,7 +20,8 @@ project files and stay writable), and changes to ~/.claude.json (MCP servers) ar isolated: network, environment, the project's own files and build scripts, the project's CLAUDE.md and memory files, a new .git/commondir or repository or worktree the session creates (git follows their config and hooks), the claude -binary of the native installer.* The +binary of the native installer, and skills and agents when +SWITCHBOARD_SANDBOX_RW_SKILLS=1 or SWITCHBOARD_SANDBOX_RW_AGENTS=1 is set.* The badge is also shown when Switchboard reattaches to a running sandboxed session. ## Turning it on @@ -195,8 +196,13 @@ directory's `.claude`, as session state: be created from inside the sandbox: create it once outside. - A `~/.claude/skills` or `~/.claude/agents` that is a symbolic link is followed like the other state links: only to a directory of the same name, - elsewhere, and never to `$HOME` or a parent of it. Links inside it are no - longer followed and protected. + elsewhere, and never to `$HOME` or a parent of it. +- Inside the writable directory, or the directory it links to, every + repository keeps its config and hooks read-only and every `.claude` stays + read-only, as in a bound directory (see [Git](#git)); a file linked from + `~/.claude` into it, such as a `settings.json` kept in the same dotfiles + repository, stays read-only too. No `.claude` is created there. Other links + inside it are not followed. This gives up part of the protection: what the session writes there is loaded, and its scripts run, by every later session outside the sandbox, other diff --git a/public/index.html b/public/index.html index f67e7e263..bbe20e386 100644 --- a/public/index.html +++ b/public/index.html @@ -106,7 +106,7 @@
- +
diff --git a/scripts/claude-sandbox.sh b/scripts/claude-sandbox.sh index 86812e0fc..e54ab84bd 100755 --- a/scripts/claude-sandbox.sh +++ b/scripts/claude-sandbox.sh @@ -18,8 +18,6 @@ # entry of ~/.claude and of each bound directory's .claude # that is not listed session state, a private copy of # ~/.claude.json, each repository's config and hooks -# (skills and agents become session state with -# $SWITCHBOARD_SANDBOX_RW_SKILLS=1 / $SWITCHBOARD_SANDBOX_RW_AGENTS=1) # network: shared with the host — Claude needs the API, and the # Switchboard IDE bridge listens on localhost # @@ -106,18 +104,15 @@ PROJECT_STATE_ENTRIES=(worktrees agent-memory agent-memory-local) # Created in ~/.claude before launch when missing, so that they are mounted # from the host rather than left to the tmpfs. USER_PRECREATED_DIRS=(projects todos statsig file-history sessions plans tasks ide) -# Opt-in: an unsandboxed claude later loads what is written there; see -# docs/sandbox.md, "Writable skills and agents". -if [ "${SWITCHBOARD_SANDBOX_RW_SKILLS:-0}" = "1" ]; then - USER_STATE_ENTRIES+=(skills) - PROJECT_STATE_ENTRIES+=(skills) - USER_PRECREATED_DIRS+=(skills) -fi -if [ "${SWITCHBOARD_SANDBOX_RW_AGENTS:-0}" = "1" ]; then - USER_STATE_ENTRIES+=(agents) - PROJECT_STATE_ENTRIES+=(agents) - USER_PRECREATED_DIRS+=(agents) -fi +# see docs/sandbox.md, "Writable skills and agents" +OPT_IN_ENTRIES=() +[ "${SWITCHBOARD_SANDBOX_RW_SKILLS:-0}" = "1" ] && OPT_IN_ENTRIES+=(skills) +[ "${SWITCHBOARD_SANDBOX_RW_AGENTS:-0}" = "1" ] && OPT_IN_ENTRIES+=(agents) +for _e in ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; do + USER_STATE_ENTRIES+=("$_e") + PROJECT_STATE_ENTRIES+=("$_e") + USER_PRECREATED_DIRS+=("$_e") +done # Project directory plus whatever Switchboard forwarded. These must already # exist — creating a mistyped "Additional Directory" on the host would be worse @@ -300,6 +295,9 @@ fi # The host paths the sandbox can write, as bound, and their resolved form. WRITABLE_ROOTS=("$CLAUDE_DIR" "${RW_STATE_DIRS[@]}" "${RW_DIRS[@]}") +for _e in ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; do + [ -L "$CLAUDE_DIR/$_e" ] && [ -d "$CLAUDE_DIR/$_e" ] && WRITABLE_ROOTS+=("$(readlink -f "$CLAUDE_DIR/$_e")") +done WRITABLE_REALS=() for _r in "${WRITABLE_ROOTS[@]}"; do WRITABLE_REALS+=("$(readlink -f "$_r" 2>/dev/null || true)") @@ -485,12 +483,19 @@ bind_user_claude_dir() { mount_op --symlink "$(readlink "$e")" "$e" if in_list "$name" "${USER_STATE_ENTRIES[@]}"; then case "$name" in *.json|*.jsonl|.last-*) kind=file ;; *) kind=dir ;; esac + STATE_LINK_TARGET="" bind_state_link_target "$e" "$kind" + if [ -n "$STATE_LINK_TARGET" ] && in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; then + protect_tree "$STATE_LINK_TARGET" nocreate + fi else protect_link_target "$e" fi elif in_list "$name" "${USER_STATE_ENTRIES[@]}"; then mount_op --bind "$e" "$e" + if [ -d "$e" ] && in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; then + protect_tree "$e" nocreate + fi else mount_op --ro-bind "$e" "$e" [ -d "$e" ] && protect_links_below "$e" @@ -588,7 +593,7 @@ protect_git() { # The .git and .claude of a directory, bound or found below one. protect_repo_root() { - local d="$1" + local d="$1" create="${2:-create}" if [ -L "$d/.git" ]; then fail "refusing to launch: $d/.git is a symbolic link. The sandbox protects the repository's config and hooks with read-only mounts, which cannot stop the link itself from being replaced. Turn Sandbox off for this session." elif [ -d "$d/.git" ]; then @@ -597,34 +602,37 @@ protect_repo_root() { mount_op --ro-bind "$d/.git" "$d/.git" GIT_WORKTREES+=("$d") fi - protect_claude_dir "$d/.claude" + protect_claude_dir "$d/.claude" "$create" } protect_claude_dir() { - local c="$1" + local c="$1" create="${2:-create}" if [ -L "$c" ]; then fail "refusing to launch: $c is a symbolic link. The sandbox protects it with read-only mounts, which cannot stop the link itself from being replaced. Replace the link with the directory it points to, or turn Sandbox off for this session." elif [ -d "$c" ]; then bind_project_claude_dir "$c" elif [ -e "$c" ]; then mount_op --ro-bind "$c" "$c" - elif [ -w "$(dirname "$c")" ]; then + elif [ "$create" = create ] && [ -w "$(dirname "$c")" ]; then MISSING_DIRS+=("$c") fi } bind_project_dir() { - local d="$1" n - mount_op --bind "$d" "$d" - [ -d "$d" ] || return 0 - protect_repo_root "$d" - # Every .claude and repository below, worktrees included; see - # docs/sandbox.md, "Git". + mount_op --bind "$1" "$1" + [ -d "$1" ] || return 0 + protect_tree "$1" create +} + +# .git and .claude at and below $1; see docs/sandbox.md, "Git". +protect_tree() { + local d="$1" create="$2" n + protect_repo_root "$d" "$create" while IFS= read -r -d '' n; do inside_ro_area "$n" && continue case "${n##*/}" in - .claude) protect_claude_dir "$n" ;; - .git) protect_repo_root "$(dirname "$n")" ;; + .claude) protect_claude_dir "$n" "$create" ;; + .git) protect_repo_root "$(dirname "$n")" "$create" ;; esac done < <(find "$d" -xdev -mindepth 2 \( -name node_modules -prune \) -o \ \( -name .git -print0 -prune \) -o \( -name .claude -type d -print0 \) -o \ diff --git a/test/sandbox-wrapper.test.js b/test/sandbox-wrapper.test.js index e98c1a769..9a8123743 100644 --- a/test/sandbox-wrapper.test.js +++ b/test/sandbox-wrapper.test.js @@ -18,12 +18,14 @@ const LINUX = process.platform === 'linux'; /** * The environment every child of these tests runs with: no inherited GIT_* or * HUSKY* variable, so a git command inside the rig can never reach the - * repository the suite runs from (the pre-commit hook sets GIT_INDEX_FILE). + * repository the suite runs from (the pre-commit hook sets GIT_INDEX_FILE), + * and no SWITCHBOARD_SANDBOX_* one, which a suite run from a sandboxed + * session would otherwise pass on to the wrapper under test. */ function cleanEnv(extra = {}) { const env = {}; for (const [k, v] of Object.entries(process.env)) { - if (!/^(GIT_|HUSKY)/.test(k)) env[k] = v; + if (!/^(GIT_|HUSKY|SWITCHBOARD_SANDBOX_)/.test(k)) env[k] = v; } return { ...env, ...extra }; } @@ -512,6 +514,78 @@ test('sandbox wrapper: skills and agents are read-write only with SWITCHBOARD_SA } }); +test('sandbox wrapper: an opted-in skills linked to a repository elsewhere keeps that repository\'s git config, hooks and linked settings read-only', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + fs.mkdirSync(dir); + const skills = path.join(rig.root, 'dotfiles', 'skills'); + fs.mkdirSync(path.join(skills, 'mine'), { recursive: true }); + git(skills, 'init', '-q'); + fs.mkdirSync(path.join(skills, '.claude')); + fs.writeFileSync(path.join(skills, '.claude', 'settings.json'), '{}\n'); + fs.writeFileSync(path.join(skills, 'settings.json'), '{}\n'); + fs.symlinkSync(skills, path.join(dir, 'skills')); + fs.symlinkSync(path.join(skills, 'settings.json'), path.join(dir, 'settings.json')); + + const { status, stderr } = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }); + assert.equal(status, 0, stderr); + const ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(mountAt(ops, skills)?.op, '--bind', 'the opted-in target is writable'); + assert.equal(accessAt(ops, path.join(skills, 'mine', 'SKILL.md')), '--bind'); + for (const p of [path.join(skills, '.git', 'config'), path.join(skills, '.git', 'hooks'), + path.join(skills, 'settings.json'), path.join(skills, '.claude', 'settings.json')]) { + assert.equal(accessAt(ops, p), '--ro-bind', `${p} must stay read-only inside the writable target`); + } + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: a repository inside an opted-in ~/.claude/agents keeps its git config and hooks read-only, and no .claude is created there', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + const vendored = path.join(dir, 'agents', 'vendored'); + fs.mkdirSync(vendored, { recursive: true }); + git(vendored, 'init', '-q'); + + const { status, stderr } = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_AGENTS: '1' }); + assert.equal(status, 0, stderr); + const ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(mountAt(ops, path.join(dir, 'agents'))?.op, '--bind'); + for (const p of [path.join(vendored, '.git', 'config'), path.join(vendored, '.git', 'hooks')]) { + assert.equal(accessAt(ops, p), '--ro-bind', `${p} must stay read-only`); + } + assert.equal(fs.existsSync(path.join(vendored, '.claude')), false, 'no .claude is planted in a skill or agent repository'); + assert.equal(fs.existsSync(path.join(dir, 'agents', '.claude')), false); + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: an opted-in skills in a project\'s .claude that links out of the sandbox is not followed', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + fs.mkdirSync(path.join(rig.home, '.claude')); + const outside = path.join(rig.root, 'elsewhere', 'skills'); + fs.mkdirSync(outside, { recursive: true }); + fs.mkdirSync(path.join(rig.proj, '.claude')); + fs.symlinkSync(outside, path.join(rig.proj, '.claude', 'skills')); + + const { status, stderr } = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }); + assert.equal(status, 0, stderr); + const ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(ops.find(o => o.dest === outside || o.dest.startsWith(outside + '/')), undefined, + 'a target outside what the sandbox sees stays invisible'); + } finally { + rig.cleanup(); + } + }); + test('sandbox wrapper: a symlinked ~/.claude entry is recreated as a link, and its target is read-only wherever the sandbox could write it', { skip: !LINUX && 'linux only' }, () => { const rig = makeRig({ recordArgs: true }); From fca4fc5bc598235aeaa7cafc44feaefc2ff2b22e Mon Sep 17 00:00:00 2001 From: abate Date: Mon, 5 Oct 2026 12:41:38 +0200 Subject: [PATCH 05/17] fix(restore): save the open sessions when the app closes The working set was written on a 500 ms debounce, so the last change before a quit could be lost, and the shutdown's own PTY kills arrived as session exits that could save an empty set. A confirmed close or quit now writes the set at once before answering main, then stops saving it; main stops sending process-exited once before-quit kills the sessions. --- CHANGELOG.md | 1 + docs/session-restore.md | 21 ++++++ main.js | 6 +- public/app.js | 23 ++++++- public/file-panel.js | 8 ++- test/dom-file-panel-unsaved-guard.test.js | 30 +++++++++ test/exit-flush.test.js | 82 +++++++++++++++++++++++ test/restore-live-elsewhere.test.js | 1 + 8 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 test/exit-flush.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 9701a1c8e..d1a23cac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc - When the Claude CLI reports a session's id in a different letter case, the subagents that session started or finished during a turn now show up when the turn ends. (#487) - A session that moves into a worktree while it runs stays in the sidebar, under its project, instead of disappearing while its terminal keeps running. A running session also stays listed when its transcript leaves its folder for any other reason. (#485) - Worktree folders from another host, or whose repository is not listed, show in the sidebar instead of being nested under the wrong folder or not shown at all. The worktrees of a repository hidden with Hide Project stay hidden. (#473) +- Quitting or closing the window saves the open sessions right away, so the next start restores the sessions that were open, not an empty or out-of-date list. (#479) - Remote triggers refuse commands containing invisible format characters, default-ignorable characters or braille blanks, including joined emoji, emoji with variation selectors (such as hearts), soft hyphens and right-to-left marks. Fullwidth slash, exclamation and number-sign prefixes are refused too. (#440) ## v0.0.89 — 2026-10-04 diff --git a/docs/session-restore.md b/docs/session-restore.md index f5f746240..1981f4917 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -21,6 +21,27 @@ Each time a session is opened or closed, Switchboard saves the open set — each session's id and project, and which one was active — in its global settings (`openWorkingSet`). +## Closing the app + +The saved set is written on each open and close half a second later, so +the last change before quitting could be lost, and so could the whole set: +quitting stops every session, and each of those exits was itself a "session +closed" that saved the set again — empty, if the app took longer than that +half second to go away. + +Once a close or quit is confirmed (the window's close, ☰ → Quit, an update +install; see the unsaved-edits guard in `.ai/contexts/viewer-panel.md`), the +renderer writes the set at once, before it answers main (`flushStateForExit` +in `public/app.js`, called by the `unsaved-check` handler in +`public/file-panel.js`, bounded to 2 s so a stuck write cannot hold the +window open), and stops saving it for the next 10 s. Main stops sending +`process-exited` once `before-quit` starts killing the sessions +(`appQuitting`), so the shutdown cannot record them as closed. A reload is +not an exit and does neither. An exit asked during a restore writes +nothing: the set from the previous run is still the one to restore. If the +exit does not happen after all (an installer that fails to start), saving +resumes after the 10 s. + ## What restore does Restore is a respawn, not a reattach: a session's process is a child of the diff --git a/main.js b/main.js index 491ee1e16..2a760b8f4 100644 --- a/main.js +++ b/main.js @@ -2421,7 +2421,7 @@ function wireSessionPty(session, sessionId, ptyProcess) { const realId = session.realSessionId || sessionId; if (TRACE.on) trace('pty.exit', realId, { exitCode, signal: exitSignal, stopped, alsoUnder: realId !== sessionId ? sessionId : null, wasBusy: !!session._cliBusy, sent: !!(mainWindow && !mainWindow.isDestroyed()) }); - if (mainWindow && !mainWindow.isDestroyed()) { + if (mainWindow && !mainWindow.isDestroyed() && !appQuitting) { mainWindow.webContents.send('process-exited', realId, exitCode, exitSignal, stopped, session.generation); // If a fork transition re-keyed this session under realId but the PTY // exited before transition detection ran, also notify the renderer for @@ -3483,6 +3483,8 @@ app.on('window-all-closed', () => { if (process.platform !== 'darwin') app.quit(); }); +let appQuitting = false; + // see .ai/contexts/activitywatch.md ("Quitting") app.on('before-quit', (event) => { if (unsavedGuard.beforeQuit(event, mainWindow)) return; @@ -3511,6 +3513,8 @@ app.on('before-quit', (event) => { remoteWatcher.stopAll(); // Kill all PTY processes on quit + // see docs/session-restore.md ("Closing the app") + appQuitting = true; for (const [id, session] of activeSessions) { if (!session.exited) killPty(session, id); } diff --git a/public/app.js b/public/app.js index 6420ebab0..57db59868 100644 --- a/public/app.js +++ b/public/app.js @@ -172,7 +172,13 @@ const restoreInFlight = new Map(); // cannot interleave and silently drop each other's keys. let _persistChain = Promise.resolve(); -function persistWorkingSet() { +// see docs/session-restore.md ("Closing the app") +let exitingApp = false; +let exitingAppTimer = null; +const EXIT_FLUSH_GRACE_MS = 10000; + +function persistWorkingSet({ final = false } = {}) { + if (exitingApp && !final) return _persistChain; _persistChain = _persistChain.then(async () => { const g = await window.api.getSetting('global'); const global = g || {}; @@ -210,7 +216,7 @@ function pendingRestoreEntries() { } function schedulePersistWorkingSet() { - if (restoringWorkingSet) return; + if (restoringWorkingSet || exitingApp) return; if (persistWorkingSetTimer) clearTimeout(persistWorkingSetTimer); persistWorkingSetTimer = setTimeout(() => { persistWorkingSetTimer = null; @@ -218,6 +224,19 @@ function schedulePersistWorkingSet() { }, 500); } +async function flushStateForExit() { + if (persistWorkingSetTimer) { + clearTimeout(persistWorkingSetTimer); + persistWorkingSetTimer = null; + } + exitingApp = true; + if (exitingAppTimer) clearTimeout(exitingAppTimer); + exitingAppTimer = setTimeout(() => { exitingApp = false; exitingAppTimer = null; }, EXIT_FLUSH_GRACE_MS); + if (restoringWorkingSet) return _persistChain; + return persistWorkingSet({ final: true }); +} +window.flushStateForExit = flushStateForExit; + async function runRestore(list) { const pending = list.filter(item => sessionMap.has(item.sessionId) && !openSessions.has(item.sessionId)); for (const item of pending) restoreInFlight.set(item.sessionId, item); diff --git a/public/file-panel.js b/public/file-panel.js index 806a084a6..0aa27f077 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -53,6 +53,7 @@ let changesListSplitterEl = null; // Row ceiling for the Changes list — see .ai/contexts/changes-view.md ("Untracked files") const MAX_CHANGES_ROWS = 500; const MAX_SUBAGENT_GROUP_ROWS = 100; +const EXIT_FLUSH_BOUND_MS = 2000; const CHANGES_LIST_HEIGHT_KEY = 'changesListHeight'; const DEFAULT_CHANGES_LIST_HEIGHT = 200; @@ -149,10 +150,15 @@ function initFilePanel() { event.returnValue = false; }); if (window.api.onUnsavedCheck) { - window.api.onUnsavedCheck(async (id) => { + window.api.onUnsavedCheck(async (id, reason) => { window.api.unsavedCheckAck(id); let proceed = true; try { proceed = await askAboutUnsavedEdits(); } catch (err) { console.error('[unsaved-check]', err); } + // see docs/session-restore.md ("Closing the app") + if (proceed && reason !== 'reload' && typeof window.flushStateForExit === 'function') { + const bound = new Promise((resolve) => setTimeout(resolve, EXIT_FLUSH_BOUND_MS)); + try { await Promise.race([window.flushStateForExit(), bound]); } catch (err) { console.error('[exit-flush]', err); } + } window.api.unsavedCheckResult(id, proceed); }); } diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 018872dca..371e141b7 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -238,3 +238,33 @@ test('the check is acknowledged on receipt, before any dialog is shown', async ( await flush(); } finally { ctx.destroy(); } }); + +// see docs/session-restore.md ("Closing the app") +test('a confirmed close or quit flushes the app state before answering; a reload does not', async () => { + const ctx = setup(); + try { + const flushed = []; + ctx.window.flushStateForExit = () => { flushed.push(ctx.calls.answers.length); return Promise.resolve(); }; + ctx.calls.check(1, 'quit'); + await flush(); + assert.deepEqual(flushed, [0], 'flushed before the answer was sent'); + assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: true }]); + ctx.calls.check(2, 'reload'); + await flush(); + assert.equal(flushed.length, 1); + } finally { ctx.destroy(); } +}); + +test('a cancelled close does not flush', async () => { + const ctx = setup(); + try { + let flushed = 0; + ctx.window.flushStateForExit = () => { flushed++; return Promise.resolve(); }; + await dirtyTab(ctx, 's1', A); + ctx.calls.check(3, 'quit'); + await flush(); + button(ctx, 'unsaved-cancel').click(); + await flush(); + assert.equal(flushed, 0); + } finally { ctx.destroy(); } +}); diff --git a/test/exit-flush.test.js b/test/exit-flush.test.js new file mode 100644 index 000000000..521035608 --- /dev/null +++ b/test/exit-flush.test.js @@ -0,0 +1,82 @@ +'use strict'; + +// see docs/session-restore.md ("Closing the app") + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const vm = require('node:vm'); +const { loadAppFunctions } = require('./app-source'); + +function setup() { + const store = { global: { windowBounds: { x: 1 } } }; + const writes = []; + const timers = new Map(); + let nextTimer = 1; + const context = vm.createContext({ + console, + Promise, + openSessions: new Map(), + activeSessionId: null, + skippedWorkingSetEntries: new Map(), + setTimeout: (fn) => { const id = nextTimer++; timers.set(id, fn); return id; }, + clearTimeout: (id) => { timers.delete(id); }, + window: { + api: { + getSetting: async (key) => JSON.parse(JSON.stringify(store[key] || null)), + setSetting: async (key, value) => { store[key] = JSON.parse(JSON.stringify(value)); writes.push(JSON.parse(JSON.stringify(value.openWorkingSet.map((e) => e.sessionId)))); }, + }, + }, + }); + const fns = loadAppFunctions(context, { + declarations: ['restoringWorkingSet', 'persistWorkingSetTimer', '_persistChain', 'exitingApp', 'exitingAppTimer', 'EXIT_FLUSH_GRACE_MS'], + functions: ['persistWorkingSet', 'schedulePersistWorkingSet', 'flushStateForExit'], + }); + const open = (id) => context.openSessions.set(id, { session: { sessionId: id, projectPath: '/p' }, closed: false }); + const runTimers = () => { const fns = [...timers.values()]; timers.clear(); for (const fn of fns) fn(); }; + return { ...fns, context, store, writes, timers, open, runTimers }; +} + +test('a confirmed exit writes the open set at once instead of leaving it on the debounce', async () => { + const t = setup(); + t.open('a'); + t.open('b'); + t.schedulePersistWorkingSet(); + assert.equal(t.writes.length, 0, 'still debounced'); + + await t.flushStateForExit(); + assert.deepEqual(t.writes, [['a', 'b']]); + assert.deepEqual(t.store.global.windowBounds, { x: 1 }, 'other global keys are kept'); +}); + +test('sessions killed by the shutdown cannot overwrite the saved set with an empty one', async () => { + const t = setup(); + t.open('a'); + await t.flushStateForExit(); + + t.context.openSessions.get('a').closed = true; + t.schedulePersistWorkingSet(); + await t.persistWorkingSet(); + assert.deepEqual(t.writes, [['a']]); + assert.deepEqual(t.store.global.openWorkingSet.map((e) => e.sessionId), ['a']); +}); + +test('an exit that does not happen gives persistence back after the grace period', async () => { + const t = setup(); + t.open('a'); + await t.flushStateForExit(); + t.runTimers(); + + t.open('b'); + await t.persistWorkingSet(); + assert.deepEqual(t.writes, [['a'], ['a', 'b']]); +}); + +test('an exit during a restore keeps the set saved by the previous run', async () => { + const t = setup(); + t.store.global.openWorkingSet = [{ sessionId: 'a' }, { sessionId: 'b' }]; + vm.runInContext('restoringWorkingSet = true', t.context); + t.open('a'); + await t.flushStateForExit(); + assert.equal(t.writes.length, 0); + assert.equal(t.store.global.openWorkingSet.length, 2); +}); diff --git a/test/restore-live-elsewhere.test.js b/test/restore-live-elsewhere.test.js index 77fb7057c..22fbd3412 100644 --- a/test/restore-live-elsewhere.test.js +++ b/test/restore-live-elsewhere.test.js @@ -69,6 +69,7 @@ function setup({ savedSet, liveIds, batchFails = false }) { var restorePlanner = null; var sessionOpenedOutsideRestore = false; var _persistChain = Promise.resolve(); + var exitingApp = false; var RESTORE_STAGGER_MS = 0; var LIVE_ELSEWHERE_NOTICE_MS = 60000; function createTerminalEntry(session) { From 0d3e510509e80e3069bab4d50e6219c707f08f3c Mon Sep 17 00:00:00 2001 From: abate Date: Thu, 8 Oct 2026 11:16:24 +0200 Subject: [PATCH 06/17] fix(sandbox): refuse an opted-in skills or agents that overlaps a read-only path Linked to the same directory as hooks (or agents, commands), or inside a read-only target such as hooks, plugins or a repository's hooks directory, the writable bind of skills or agents overrode the read-only one and left the protected files writable. The finished mount list is now checked and such a launch refused, whatever order the entries were bound in. --- docs/sandbox.md | 9 ++++ scripts/claude-sandbox.sh | 33 +++++++++++++ test/sandbox-wrapper.test.js | 92 ++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) diff --git a/docs/sandbox.md b/docs/sandbox.md index 54d522223..882150a06 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -203,6 +203,15 @@ directory's `.claude`, as session state: `~/.claude` into it, such as a `settings.json` kept in the same dotfiles repository, stays read-only too. No `.claude` is created there. Other links inside it are not followed. +- The launch is refused when the writable directory is also, or lies inside, + something the sandbox keeps read-only: the target of another `~/.claude` + entry (`hooks` and `skills` linked to the same directory, or `skills` linked + inside the target of `hooks`), a repository's hooks directory, or a read-only + entry such as `~/.claude/plugins`. Two mounts on the same path, or a writable + one inside a read-only one, would leave the protected files writable through + the opted-in directory. A read-only path inside the writable directory is + fine: the deeper mount wins. The check runs on the finished mount list, so + the order in which the entries are bound does not matter. This gives up part of the protection: what the session writes there is loaded, and its scripts run, by every later session outside the sandbox, other diff --git a/scripts/claude-sandbox.sh b/scripts/claude-sandbox.sh index e54ab84bd..9b4ab9118 100755 --- a/scripts/claude-sandbox.sh +++ b/scripts/claude-sandbox.sh @@ -486,6 +486,7 @@ bind_user_claude_dir() { STATE_LINK_TARGET="" bind_state_link_target "$e" "$kind" if [ -n "$STATE_LINK_TARGET" ] && in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; then + OPT_IN_BINDS+=("user:$((${#M_DEST[@]} - 1))") protect_tree "$STATE_LINK_TARGET" nocreate fi else @@ -494,6 +495,7 @@ bind_user_claude_dir() { elif in_list "$name" "${USER_STATE_ENTRIES[@]}"; then mount_op --bind "$e" "$e" if [ -d "$e" ] && in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; then + OPT_IN_BINDS+=("user:$((${#M_DEST[@]} - 1))") protect_tree "$e" nocreate fi else @@ -523,6 +525,7 @@ bind_project_claude_dir() { debug "state link $e left as is" else mount_op --bind "$e" "$e" + in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"} && OPT_IN_BINDS+=("project:$((${#M_DEST[@]} - 1))") fi elif [ -L "$e" ]; then protect_link_target "$e" @@ -693,6 +696,34 @@ pin_protected_paths() { done } +# see docs/sandbox.md, "Writable skills and agents" +check_opt_in_binds() { + local b k t i d best best_len r + for b in ${OPT_IN_BINDS[@]+"${OPT_IN_BINDS[@]}"}; do + k="${b#*:}"; t="${M_DEST[k]}" + best=""; best_len=-1 + for i in "${!M_DEST[@]}"; do + [ "$i" = "$k" ] && continue + case "${M_OP[i]}" in --symlink|--file) continue ;; esac + d="${M_DEST[i]}" + case "$t/" in "$d"/*) ;; *) continue ;; esac + [ "${#d}" -ge "$best_len" ] && { best="$i"; best_len="${#d}"; } + done + if [ "${b%%:*}" = user ] && [ -n "$best" ] && [ "${M_OP[best]}" = --ro-bind ]; then + opt_in_refuse "$t" "${M_DEST[best]}" + fi + for r in "${!PROTECTED_SEEN[@]}"; do + case "$t/" in "$r"/*) opt_in_refuse "$t" "$r" ;; esac + done + done +} + +opt_in_refuse() { + local where="inside" + [ "$1" = "$2" ] && where="also" + fail "refusing to launch: $1, which SWITCHBOARD_SANDBOX_RW_$(printf '%s' "${1##*/}" | tr a-z A-Z)=1 makes writable, is $where $2, which the sandbox keeps read-only; the session could change it through ${1##*/}. Give ${1##*/} a directory of its own, or turn the flag off for this session." +} + # Fills BWRAP_ARGS from what exists now, and MISSING_DIRS with what must be # created before the real launch. Run once for the pre-flight, then again once # the missing directories exist. @@ -702,6 +733,7 @@ build_bwrap_args() { PIN_VIEWS=(); PIN_IDX=() MISSING_DIRS=() GIT_WORKTREES=() + OPT_IN_BINDS=() local d name i mount_op --dev "" /dev mount_op --proc "" /proc @@ -748,6 +780,7 @@ build_bwrap_args() { protect_git "$d" done pin_protected_paths + check_opt_in_binds BWRAP_ARGS=(--unshare-all --share-net --die-with-parent) local order diff --git a/test/sandbox-wrapper.test.js b/test/sandbox-wrapper.test.js index 9a8123743..5831b19a3 100644 --- a/test/sandbox-wrapper.test.js +++ b/test/sandbox-wrapper.test.js @@ -586,6 +586,98 @@ test('sandbox wrapper: an opted-in skills in a project\'s .claude that links out } }); +test('sandbox wrapper: refuses an opted-in skills or agents that is, or is inside, a path another entry keeps read-only', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + const flags = { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }; + const reset = () => { + fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(path.join(rig.proj, '.claude'), { recursive: true, force: true }); + fs.rmSync(path.join(rig.proj, '.git'), { recursive: true, force: true }); + fs.rmSync(path.join(rig.root, 'x'), { recursive: true, force: true }); + fs.mkdirSync(dir); + }; + const refuses = (what) => { + const { status, stderr } = rig.run(['--version'], flags); + assert.equal(status, 125, `${what}: must be refused\n${stderr}`); + assert.match(stderr, /refusing to launch: .*skills.* read-only/, what); + }; + const x = path.join(rig.root, 'x'); + + for (const other of ['hooks', 'agents', 'commands', 'statusline']) { + reset(); + fs.mkdirSync(path.join(x, 'skills'), { recursive: true }); + fs.symlinkSync(path.join(x, 'skills'), path.join(dir, other)); + fs.symlinkSync(path.join(x, 'skills'), path.join(dir, 'skills')); + refuses(`${other} and skills linked to the same directory`); + } + + reset(); + fs.mkdirSync(path.join(dir, 'skills')); + fs.symlinkSync(path.join(dir, 'skills'), path.join(dir, 'commands')); + refuses('commands linked to a real ~/.claude/skills'); + + reset(); + fs.mkdirSync(path.join(x, 'h', 'skills'), { recursive: true }); + fs.symlinkSync(path.join(x, 'h'), path.join(dir, 'hooks')); + fs.symlinkSync(path.join(x, 'h', 'skills'), path.join(dir, 'skills')); + refuses('skills linked inside the target of hooks, which the sandbox does not otherwise see'); + + reset(); + fs.mkdirSync(path.join(dir, 'plugins', 'skills'), { recursive: true }); + fs.symlinkSync(path.join(dir, 'plugins', 'skills'), path.join(dir, 'skills')); + refuses('skills linked inside the read-only ~/.claude/plugins'); + + reset(); + git(rig.proj, 'init', '-q'); + fs.mkdirSync(path.join(rig.proj, '.git', 'hooks', 'skills'), { recursive: true }); + fs.symlinkSync(path.join(rig.proj, '.git', 'hooks', 'skills'), path.join(dir, 'skills')); + refuses('skills linked inside the project\'s git hooks'); + + reset(); + fs.mkdirSync(path.join(rig.proj, '.claude', 'skills'), { recursive: true }); + fs.symlinkSync('skills', path.join(rig.proj, '.claude', 'hooks')); + refuses('the project\'s .claude/hooks linked to its .claude/skills'); + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: an opted-in skills may contain a read-only path, or share its target with the project\'s own skills', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + fs.mkdirSync(dir); + const skills = path.join(rig.root, 'x', 'skills'); + fs.mkdirSync(path.join(skills, 'hk'), { recursive: true }); + fs.symlinkSync(skills, path.join(dir, 'skills')); + fs.symlinkSync(path.join(skills, 'hk'), path.join(dir, 'hooks')); + + let res = rig.run(['--version']); + assert.equal(res.status, 0, `with the flag off nothing is writable, so nothing is refused\n${res.stderr}`); + res = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }); + assert.equal(res.status, 0, res.stderr); + let ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(accessAt(ops, path.join(skills, 'mine')), '--bind'); + assert.equal(accessAt(ops, path.join(skills, 'hk', 'h.sh')), '--ro-bind', 'a deeper read-only mount wins'); + + fs.rmSync(path.join(dir, 'skills')); + fs.rmSync(path.join(dir, 'hooks')); + const own = path.join(rig.proj, '.claude', 'skills'); + fs.mkdirSync(own, { recursive: true }); + fs.symlinkSync(own, path.join(dir, 'skills')); + res = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }); + assert.equal(res.status, 0, res.stderr); + ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(mountAt(ops, own)?.op, '--bind'); + } finally { + rig.cleanup(); + } + }); + test('sandbox wrapper: a symlinked ~/.claude entry is recreated as a link, and its target is read-only wherever the sandbox could write it', { skip: !LINUX && 'linux only' }, () => { const rig = makeRig({ recordArgs: true }); From 7fdbaeee6fb33aae75e3d0af2db958c436ef4468 Mon Sep 17 00:00:00 2001 From: abate Date: Thu, 8 Oct 2026 11:33:50 +0200 Subject: [PATCH 07/17] fix(restore): keep unrestored sessions on exit, and save after an aborted one The final write skipped a restore in progress, so a session stopped during it came back, and wrote [] before the saved set was even restored. It now writes through pendingRestoreEntries, which keeps the sessions a cold index, the Restore prompt or the restore still hold, and writes nothing before the saved set is read. A save asked during the grace period of an exit that did not happen is written when it ends. A Windows logoff asks the renderer to save, best effort. --- docs/session-restore.md | 21 +++++++-- preload.js | 3 ++ public/app.js | 27 +++++++++-- test/exit-flush.test.js | 68 +++++++++++++++++++++++++--- test/restore-pending-persist.test.js | 2 +- test/unsaved-guard.test.js | 4 +- unsaved-guard.js | 11 ++++- 7 files changed, 117 insertions(+), 19 deletions(-) diff --git a/docs/session-restore.md b/docs/session-restore.md index 1981f4917..49b9b135b 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -37,10 +37,23 @@ in `public/app.js`, called by the `unsaved-check` handler in window open), and stops saving it for the next 10 s. Main stops sending `process-exited` once `before-quit` starts killing the sessions (`appQuitting`), so the shutdown cannot record them as closed. A reload is -not an exit and does neither. An exit asked during a restore writes -nothing: the set from the previous run is still the one to restore. If the -exit does not happen after all (an installer that fails to start), saving -resumes after the 10 s. +not an exit and does neither. + +The write keeps the saved sessions that are not open yet: those the index has +not reached, those offered by the Restore prompt and not answered, and those +the restore is still starting (`pendingRestoreEntries`). So quitting during a +cold index, before answering the prompt, or in the middle of a restore keeps +them for the next start, while a session you stopped during the restore is +left out. An exit before the saved set has even been read writes nothing. + +If the exit does not happen after all (an installer that fails to start), +saving resumes after the 10 s, and the set is written then if anything asked +for a save in the meantime. + +A Windows logoff or shutdown (`query-session-end`, `session-end`) does not +ask about unsaved edits; main sends `exit-flush` instead, and the renderer +writes the set without waiting for an answer. That write is best effort: +Windows may end the app before it lands. ## What restore does diff --git a/preload.js b/preload.js index 84e6430bc..a254b8e2c 100644 --- a/preload.js +++ b/preload.js @@ -164,6 +164,9 @@ contextBridge.exposeInMainWorld('api', { }, unsavedCheckAck: (id) => ipcRenderer.send('unsaved-check-ack', id), unsavedCheckResult: (id, proceed) => ipcRenderer.send('unsaved-check-result', id, proceed), + onExitFlush: (callback) => { + ipcRenderer.on('exit-flush', () => callback()); + }, onFullScreenChanged: (callback) => { ipcRenderer.on('full-screen-changed', (_event, isFullScreen) => callback(isFullScreen)); }, diff --git a/public/app.js b/public/app.js index 57db59868..8bcb76404 100644 --- a/public/app.js +++ b/public/app.js @@ -175,10 +175,15 @@ let _persistChain = Promise.resolve(); // see docs/session-restore.md ("Closing the app") let exitingApp = false; let exitingAppTimer = null; +let persistSkippedWhileExiting = false; +let savedWorkingSetRead = false; const EXIT_FLUSH_GRACE_MS = 10000; function persistWorkingSet({ final = false } = {}) { - if (exitingApp && !final) return _persistChain; + if (exitingApp && !final) { + persistSkippedWhileExiting = true; + return _persistChain; + } _persistChain = _persistChain.then(async () => { const g = await window.api.getSetting('global'); const global = g || {}; @@ -216,7 +221,11 @@ function pendingRestoreEntries() { } function schedulePersistWorkingSet() { - if (restoringWorkingSet || exitingApp) return; + if (restoringWorkingSet) return; + if (exitingApp) { + persistSkippedWhileExiting = true; + return; + } if (persistWorkingSetTimer) clearTimeout(persistWorkingSetTimer); persistWorkingSetTimer = setTimeout(() => { persistWorkingSetTimer = null; @@ -230,12 +239,21 @@ async function flushStateForExit() { persistWorkingSetTimer = null; } exitingApp = true; + persistSkippedWhileExiting = false; if (exitingAppTimer) clearTimeout(exitingAppTimer); - exitingAppTimer = setTimeout(() => { exitingApp = false; exitingAppTimer = null; }, EXIT_FLUSH_GRACE_MS); - if (restoringWorkingSet) return _persistChain; + exitingAppTimer = setTimeout(() => { + exitingApp = false; + exitingAppTimer = null; + if (persistSkippedWhileExiting) { + persistSkippedWhileExiting = false; + persistWorkingSet(); + } + }, EXIT_FLUSH_GRACE_MS); + if (!savedWorkingSetRead) return _persistChain; return persistWorkingSet({ final: true }); } window.flushStateForExit = flushStateForExit; +if (window.api.onExitFlush) window.api.onExitFlush(() => { flushStateForExit(); }); async function runRestore(list) { const pending = list.filter(item => sessionMap.has(item.sessionId) && !openSessions.has(item.sessionId)); @@ -282,6 +300,7 @@ async function restoreWorkingSet() { restoreMode = (g && g.restoreOnStartup) || SETTING_DEFAULTS.restoreOnStartup; const savedSet = (g && g.openWorkingSet) || []; restoreSavedIndex = new Map(savedSet.map((item, index) => [item.sessionId, index])); + savedWorkingSetRead = true; document.getElementById('restore-cold-toast')?.remove(); diff --git a/test/exit-flush.test.js b/test/exit-flush.test.js index 521035608..93c2aa6b3 100644 --- a/test/exit-flush.test.js +++ b/test/exit-flush.test.js @@ -5,6 +5,8 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const vm = require('node:vm'); +const fs = require('node:fs'); +const path = require('node:path'); const { loadAppFunctions } = require('./app-source'); function setup() { @@ -28,12 +30,22 @@ function setup() { }, }); const fns = loadAppFunctions(context, { - declarations: ['restoringWorkingSet', 'persistWorkingSetTimer', '_persistChain', 'exitingApp', 'exitingAppTimer', 'EXIT_FLUSH_GRACE_MS'], - functions: ['persistWorkingSet', 'schedulePersistWorkingSet', 'flushStateForExit'], + declarations: ['restoringWorkingSet', 'persistWorkingSetTimer', '_persistChain', 'exitingApp', 'exitingAppTimer', + 'persistSkippedWhileExiting', 'savedWorkingSetRead', 'EXIT_FLUSH_GRACE_MS', 'restorePlanner', 'restoreSavedIndex', + 'restoreAwaitingConsent', 'restoreInFlight'], + functions: ['persistWorkingSet', 'pendingRestoreEntries', 'schedulePersistWorkingSet', 'flushStateForExit'], }); + vm.runInContext(fs.readFileSync(path.join(__dirname, '..', 'public', 'restore-plan.js'), 'utf8'), context); + vm.runInContext('savedWorkingSetRead = true', context); const open = (id) => context.openSessions.set(id, { session: { sessionId: id, projectPath: '/p' }, closed: false }); const runTimers = () => { const fns = [...timers.values()]; timers.clear(); for (const fn of fns) fn(); }; - return { ...fns, context, store, writes, timers, open, runTimers }; + const run = (code) => vm.runInContext(code, context); + const saved = (ids) => { + context.savedSet = ids.map((sessionId) => ({ sessionId, projectPath: '/p', active: false })); + store.global.openWorkingSet = context.savedSet; + run('restoreSavedIndex = new Map(savedSet.map((item, index) => [item.sessionId, index]))'); + }; + return { ...fns, context, store, writes, timers, open, runTimers, run, saved }; } test('a confirmed exit writes the open set at once instead of leaving it on the debounce', async () => { @@ -71,12 +83,56 @@ test('an exit that does not happen gives persistence back after the grace period assert.deepEqual(t.writes, [['a'], ['a', 'b']]); }); -test('an exit during a restore keeps the set saved by the previous run', async () => { +test('an exit before the saved set is read writes nothing', async () => { const t = setup(); t.store.global.openWorkingSet = [{ sessionId: 'a' }, { sessionId: 'b' }]; - vm.runInContext('restoringWorkingSet = true', t.context); - t.open('a'); + t.run('savedWorkingSetRead = false'); await t.flushStateForExit(); assert.equal(t.writes.length, 0); assert.equal(t.store.global.openWorkingSet.length, 2); }); + +test('an exit during a cold index keeps the saved sessions the index has not reached', async () => { + const t = setup(); + t.saved(['a', 'b']); + t.run('restorePlanner = createRestorePlanner({ savedSet })'); + await t.flushStateForExit(); + assert.deepEqual(t.writes, [['a', 'b']]); +}); + +test('an exit before the Restore prompt is answered keeps the sessions it offers', async () => { + const t = setup(); + t.saved(['a', 'b']); + t.run('restoreAwaitingConsent = savedSet.slice()'); + await t.flushStateForExit(); + assert.deepEqual(t.writes, [['a', 'b']]); +}); + +test('an exit during a restore keeps the sessions not started yet and leaves out one stopped', async () => { + const t = setup(); + t.saved(['a', 'b', 'c']); + t.run('restoringWorkingSet = true; restoreInFlight.set("c", savedSet[2])'); + t.open('a'); + t.open('b'); + t.context.openSessions.get('b').closed = true; + await t.flushStateForExit(); + assert.deepEqual(t.writes, [['a', 'c']]); +}); + +test('a save asked during the grace period of an exit that did not happen is written when it ends', async () => { + const t = setup(); + t.open('a'); + await t.flushStateForExit(); + t.open('b'); + t.schedulePersistWorkingSet(); + t.runTimers(); + await t.run('_persistChain'); + assert.deepEqual(t.writes, [['a'], ['a', 'b']]); + + const quiet = setup(); + quiet.open('a'); + await quiet.flushStateForExit(); + quiet.runTimers(); + await quiet.run('_persistChain'); + assert.deepEqual(quiet.writes, [['a']], 'nothing asked, nothing written again'); +}); diff --git a/test/restore-pending-persist.test.js b/test/restore-pending-persist.test.js index a62857c96..31050438f 100644 --- a/test/restore-pending-persist.test.js +++ b/test/restore-pending-persist.test.js @@ -44,7 +44,7 @@ function setup() { `, ctx); const fns = loadAppFunctions(ctx, { functions: ['persistWorkingSet', 'pendingRestoreEntries', 'runRestore'], - declarations: ['skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight'], + declarations: ['skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'exitingApp', 'persistSkippedWhileExiting'], }); vm.runInContext(`restoreSavedIndex = new Map(${JSON.stringify(SAVED.map((item, i) => [item.sessionId, i]))});`, ctx); const stored = () => settings.global.openWorkingSet.map(i => i.sessionId); diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index bed9fe4c9..7ad2b4dc3 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -234,7 +234,7 @@ test('an updater install asks before the installer starts: a no leaves it unstar assert.equal(c.prevented, false); }); -test('a Windows session end approves the quit so logoff never waits on the dialog', () => { +test('a Windows session end approves the quit so logoff never waits on the dialog, and asks the renderer to save', () => { for (const name of ['query-session-end', 'session-end']) { const t = setup(); t.win.emit(name, {}); @@ -243,7 +243,7 @@ test('a Windows session end approves the quit so logoff never waits on the dialo const c = t.closeEvent(); t.win.emit('close', c); assert.equal(c.prevented, false, name); - assert.equal(t.sent.length, 0, name); + assert.deepEqual(t.sent.map((m) => m.channel), ['exit-flush'], `${name}: no dialog, one save request`); } }); diff --git a/unsaved-guard.js b/unsaved-guard.js index 5958f90f1..c5bcba6dc 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -77,8 +77,15 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou let reloading = false; let allowNextUnload = false; - win.on('query-session-end', approveQuit); - win.on('session-end', approveQuit); + // see docs/session-restore.md ("Closing the app") + const endSession = () => { + approveQuit(); + const wc = win.webContents; + if (win.isDestroyed() || !wc || wc.isDestroyed()) return; + try { wc.send('exit-flush'); } catch { /* the window is going anyway */ } + }; + win.on('query-session-end', endSession); + win.on('session-end', endSession); win.on('close', (event) => { if (approved || quitApproved) return; From 93672b51eebd7709959e9ec2696fc50dc540130c Mon Sep 17 00:00:00 2001 From: abate Date: Fri, 9 Oct 2026 15:59:51 +0200 Subject: [PATCH 08/17] fix(sandbox): protect every path a repository is seen at, and compare resolved paths A protected path was mounted read-only under the first writable root that contains it only, so a repository reached through a symlinked Additional Directory and through an opted-in skills target stayed writable at the second path. The opt-in overlap check compared mount destinations, as spelled, with resolved paths, so a project reached through a symbolic link escaped it. The opted-in link is now checked for links the session could re-point, and the search below a bound directory enters one that is a link. --- docs/sandbox.md | 15 +++- scripts/claude-sandbox.sh | 79 +++++++++++++----- test/sandbox-wrapper.test.js | 153 ++++++++++++++++++++++++++++++++++- 3 files changed, 219 insertions(+), 28 deletions(-) diff --git a/docs/sandbox.md b/docs/sandbox.md index 882150a06..f11f3fd0d 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -196,7 +196,10 @@ directory's `.claude`, as session state: be created from inside the sandbox: create it once outside. - A `~/.claude/skills` or `~/.claude/agents` that is a symbolic link is followed like the other state links: only to a directory of the same name, - elsewhere, and never to `$HOME` or a parent of it. + elsewhere, and never to `$HOME` or a parent of it. The launch is refused when + the way to that directory goes through a symbolic link in a directory the + sandbox can write: the session could re-point it, and the next launch would + make another directory writable. - Inside the writable directory, or the directory it links to, every repository keeps its config and hooks read-only and every `.claude` stays read-only, as in a bound directory (see [Git](#git)); a file linked from @@ -211,7 +214,10 @@ directory's `.claude`, as session state: one inside a read-only one, would leave the protected files writable through the opted-in directory. A read-only path inside the writable directory is fine: the deeper mount wins. The check runs on the finished mount list, so - the order in which the entries are bound does not matter. + the order in which the entries are bound does not matter, and it compares + resolved paths: the directory is refused wherever the sandbox sees it + read-only, also when the project, an Additional Directory or `$HOME` is + reached through a symbolic link. This gives up part of the protection: what the session writes there is loaded, and its scripts run, by every later session outside the sandbox, other @@ -246,7 +252,10 @@ nested repositories, submodules — `node_modules` excepted: The paths come from `git rev-parse --git-dir --git-common-dir --git-path hooks` run in the directory before launch; a repository it cannot read is refused -rather than guessed at. +rather than guessed at. A protected path is mounted read-only at every place +the sandbox sees it: a repository reached both through an Additional Directory +that is a symbolic link and through its real path, or through an opted-in +`skills`, is protected at each. The search below a bound directory does not enter `node_modules`, and does not cross into another filesystem (`find -xdev`): a `.git` or `.claude` inside a diff --git a/scripts/claude-sandbox.sh b/scripts/claude-sandbox.sh index 9b4ab9118..539ad900a 100755 --- a/scripts/claude-sandbox.sh +++ b/scripts/claude-sandbox.sh @@ -381,16 +381,27 @@ check_chain() { done } -# Mounts the resolved host path $1 read-only where the sandbox sees it, when -# it lies under a writable root, and remembers it for the pins below. +# Mounts the resolved host path $1 read-only at every place the sandbox sees +# it, under each writable root that contains it, and remembers each for the +# pins below. protect_resolved() { - local real="$1" + local real="$1" i root view found=1 [ -n "${PROTECTED_SEEN[$real]:-}" ] && return 1 PROTECTED_SEEN["$real"]=1 - sandbox_view "$real" || { debug "$real is not visible in the sandbox"; return 1; } - mount_op --ro-bind "$real" "$SV_VIEW" - PIN_VIEWS+=("$SV_VIEW"); PIN_IDX+=("$SV_IDX") - return 0 + for i in "${!WRITABLE_ROOTS[@]}"; do + root="${WRITABLE_REALS[i]}" + [ -n "$root" ] || continue + case "$real" in + "$root") view="${WRITABLE_ROOTS[i]}" ;; + "$root"/*) view="${WRITABLE_ROOTS[i]}${real#"$root"}" ;; + *) continue ;; + esac + found=0 + mount_op --ro-bind "$real" "$view" + PIN_VIEWS+=("$view"); PIN_IDX+=("$i") + done + [ "$found" = 0 ] || debug "$real is not visible in the sandbox" + return "$found" } # Link $1, resolved to $2: its target read-only, and every link below that @@ -469,7 +480,7 @@ bind_projects_dir() { # read-write when listed as state, empty and private when listed as private, # read-only otherwise. Anything created at its top level is discarded. bind_user_claude_dir() { - local dir="$1" e name restore_glob kind + local dir="$1" e name restore_glob kind text mount_op --tmpfs "" "$dir" restore_glob="$(shopt -p nullglob dotglob)" shopt -s nullglob dotglob @@ -484,6 +495,11 @@ bind_user_claude_dir() { if in_list "$name" "${USER_STATE_ENTRIES[@]}"; then case "$name" in *.json|*.jsonl|.last-*) kind=file ;; *) kind=dir ;; esac STATE_LINK_TARGET="" + if in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; then + text="$(readlink "$e")" + case "$text" in /*) ;; *) text="$dir/$text" ;; esac + check_chain "$text" + fi bind_state_link_target "$e" "$kind" if [ -n "$STATE_LINK_TARGET" ] && in_list "$name" ${OPT_IN_ENTRIES[@]+"${OPT_IN_ENTRIES[@]}"}; then OPT_IN_BINDS+=("user:$((${#M_DEST[@]} - 1))") @@ -637,7 +653,7 @@ protect_tree() { .claude) protect_claude_dir "$n" "$create" ;; .git) protect_repo_root "$(dirname "$n")" "$create" ;; esac - done < <(find "$d" -xdev -mindepth 2 \( -name node_modules -prune \) -o \ + done < <(find "$d/" -xdev -mindepth 2 \( -name node_modules -prune \) -o \ \( -name .git -print0 -prune \) -o \( -name .claude -type d -print0 \) -o \ \( -name .claude -type l -print0 \) 2>/dev/null) } @@ -698,26 +714,45 @@ pin_protected_paths() { # see docs/sandbox.md, "Writable skills and agents" check_opt_in_binds() { - local b k t i d best best_len r + [ "${#OPT_IN_BINDS[@]}" -gt 0 ] || return 0 + local -a reals=() + local b k tr t i r v + for i in "${!M_DEST[@]}"; do + case "${M_OP[i]}" in + --bind|--ro-bind) reals[i]="$(readlink -m -- "${M_SRC[i]}")" ;; + esac + done for b in ${OPT_IN_BINDS[@]+"${OPT_IN_BINDS[@]}"}; do - k="${b#*:}"; t="${M_DEST[k]}" - best=""; best_len=-1 - for i in "${!M_DEST[@]}"; do - [ "$i" = "$k" ] && continue - case "${M_OP[i]}" in --symlink|--file) continue ;; esac - d="${M_DEST[i]}" - case "$t/" in "$d"/*) ;; *) continue ;; esac - [ "${#d}" -ge "$best_len" ] && { best="$i"; best_len="${#d}"; } + k="${b#*:}"; t="${M_DEST[k]}"; tr="${reals[k]}" + for i in "${!reals[@]}"; do + r="${reals[i]}" + case "$tr/" in "$r"/*) ;; *) continue ;; esac + v="${M_DEST[i]}${tr#"$r"}" + [ "$v" = "$t" ] && [ "${b%%:*}" = project ] && continue + opt_in_access_at "$v" "$k" + [ "${M_OP[OA_IDX]}" = --ro-bind ] && opt_in_refuse "$t" "${M_DEST[OA_IDX]}" done - if [ "${b%%:*}" = user ] && [ -n "$best" ] && [ "${M_OP[best]}" = --ro-bind ]; then - opt_in_refuse "$t" "${M_DEST[best]}" - fi for r in "${!PROTECTED_SEEN[@]}"; do - case "$t/" in "$r"/*) opt_in_refuse "$t" "$r" ;; esac + case "$tr/" in "$r"/*) opt_in_refuse "$t" "$r" ;; esac done done } +# Sets OA_IDX to the mount that decides access at sandbox path $1, mount $2 +# left out: the deepest at or above it, the later one on the same path. +opt_in_access_at() { + local i d best_len=-1 + OA_IDX="" + for i in "${!M_DEST[@]}"; do + [ "$i" = "$2" ] && continue + case "${M_OP[i]}" in --symlink|--file) continue ;; esac + d="${M_DEST[i]}" + case "$1/" in "$d"/*) ;; *) continue ;; esac + [ "${#d}" -ge "$best_len" ] && { OA_IDX="$i"; best_len="${#d}"; } + done + [ -n "$OA_IDX" ] || OA_IDX="$2" +} + opt_in_refuse() { local where="inside" [ "$1" = "$2" ] && where="also" diff --git a/test/sandbox-wrapper.test.js b/test/sandbox-wrapper.test.js index 5831b19a3..8ccaf2fe9 100644 --- a/test/sandbox-wrapper.test.js +++ b/test/sandbox-wrapper.test.js @@ -18,9 +18,7 @@ const LINUX = process.platform === 'linux'; /** * The environment every child of these tests runs with: no inherited GIT_* or * HUSKY* variable, so a git command inside the rig can never reach the - * repository the suite runs from (the pre-commit hook sets GIT_INDEX_FILE), - * and no SWITCHBOARD_SANDBOX_* one, which a suite run from a sandboxed - * session would otherwise pass on to the wrapper under test. + * repository the suite runs from (the pre-commit hook sets GIT_INDEX_FILE). */ function cleanEnv(extra = {}) { const env = {}; @@ -645,6 +643,155 @@ test('sandbox wrapper: refuses an opted-in skills or agents that is, or is insid } }); +test('sandbox wrapper: refuses an opted-in skills that overlaps a read-only path when the project is reached through a symbolic link', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + fs.mkdirSync(path.join(rig.home, '.claude')); + const flags = { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }; + const hooked = (p, other = 'hooks') => { + fs.mkdirSync(path.join(p, '.claude', 'skills'), { recursive: true }); + fs.symlinkSync('skills', path.join(p, '.claude', other)); + }; + const refuses = (what, res) => { + assert.equal(res.status, 125, `${what}: must be refused\n${res.stderr}`); + assert.match(res.stderr, /refusing to launch: .*skills.* read-only/, what); + }; + + const real = path.join(rig.root, 'real'); + const p = path.join(real, 'p'); + for (const other of ['hooks', 'agents', 'commands']) { + fs.rmSync(p, { recursive: true, force: true }); + hooked(p, other); + let res = rig.run(['--version'], flags, p); + refuses(`${other} -> skills, working directory spelled as its real path`, res); + fs.rmSync(path.join(rig.root, 'lp'), { force: true }); + fs.symlinkSync(real, path.join(rig.root, 'lp')); + const logical = path.join(rig.root, 'lp', 'p'); + res = rig.run(['--version'], { ...flags, PWD: logical }, logical); + refuses(`${other} -> skills, working directory reached through a symlinked parent`, res); + } + + const q = path.join(rig.root, 'q'); + hooked(q); + fs.symlinkSync(q, path.join(rig.root, 'la')); + refuses('an Additional Directory typed through a link', + rig.run(['--version'], { ...flags, SWITCHBOARD_SANDBOX_BINDS: path.join(rig.root, 'la') })); + + const realHome = path.join(rig.root, 'realhome'); + fs.mkdirSync(path.join(realHome, '.claude'), { recursive: true }); + const hp = path.join(realHome, 'hp'); + hooked(hp); + const linkHome = path.join(rig.root, 'lh'); + fs.symlinkSync(realHome, linkHome); + const logical = path.join(linkHome, 'hp'); + refuses('a $HOME that is a link, with the project under it', + rig.run(['--version'], { ...flags, HOME: linkHome, PWD: logical }, logical)); + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: a repository the sandbox sees at two paths keeps its git config and hooks read-only at both', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + fs.mkdirSync(dir); + const dot = path.join(rig.root, 'dot'); + const skills = path.join(dot, 'skills'); + fs.mkdirSync(skills, { recursive: true }); + git(skills, 'init', '-q'); + fs.symlinkSync(skills, path.join(dir, 'skills')); + const flags = { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }; + const readOnlyAt = (ops, base) => { + for (const p of [path.join(base, '.git', 'config'), path.join(base, '.git', 'hooks', 'pre-commit')]) { + assert.equal(accessAt(ops, p), '--ro-bind', `${p} must stay read-only`); + } + }; + + const alias = path.join(rig.root, 'alias'); + fs.symlinkSync(skills, alias); + let res = rig.run(['--version'], { ...flags, SWITCHBOARD_SANDBOX_BINDS: alias }); + assert.equal(res.status, 0, res.stderr); + let ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(accessAt(ops, path.join(skills, 'mine')), '--bind'); + readOnlyAt(ops, skills); + readOnlyAt(ops, alias); + + const anc = path.join(rig.root, 'anc'); + fs.symlinkSync(dot, anc); + res = rig.run(['--version'], { ...flags, SWITCHBOARD_SANDBOX_BINDS: anc }); + assert.equal(res.status, 0, res.stderr); + ops = parseMounts(rig.lastBwrapArgs()); + readOnlyAt(ops, skills); + readOnlyAt(ops, path.join(anc, 'skills')); + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: a repository and a .claude below an Additional Directory that is a link are protected', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + fs.mkdirSync(path.join(rig.home, '.claude')); + const other = path.join(rig.root, 'other'); + const sub = path.join(other, 'sub'); + fs.mkdirSync(path.join(sub, '.claude'), { recursive: true }); + git(sub, 'init', '-q'); + const link = path.join(rig.root, 'link'); + fs.symlinkSync(other, link); + const { status, stderr } = rig.run(['--version'], { SWITCHBOARD_SANDBOX_BINDS: link }); + assert.equal(status, 0, stderr); + const ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(accessAt(ops, path.join(link, 'sub', 'file')), '--bind'); + for (const p of [path.join(link, 'sub', '.git', 'config'), path.join(link, 'sub', '.claude', 'settings.json')]) { + assert.equal(accessAt(ops, p), '--ro-bind', `${p} must stay read-only`); + } + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: refuses an opted-in skills linked through a link the session could re-point', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + fs.mkdirSync(dir); + const target = path.join(rig.root, 'a'); + fs.mkdirSync(path.join(target, 'skills'), { recursive: true }); + fs.symlinkSync(target, path.join(rig.proj, 'lnk')); + fs.symlinkSync(path.join(rig.proj, 'lnk', 'skills'), path.join(dir, 'skills')); + const { status, stderr } = rig.run(['--version'], { SWITCHBOARD_SANDBOX_RW_SKILLS: '1' }); + assert.equal(status, 125, stderr); + assert.match(stderr, /lnk is a symbolic link in a directory the sandbox can write/); + } finally { + rig.cleanup(); + } + }); + +test('sandbox wrapper: an opted-in skills under a read-only directory that is also bound read-write follows the later mount', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + const dir = path.join(rig.home, '.claude'); + fs.mkdirSync(dir); + const nvm = path.join(rig.root, 'nvm'); + fs.mkdirSync(path.join(nvm, 'skills'), { recursive: true }); + fs.symlinkSync(path.join(nvm, 'skills'), path.join(dir, 'skills')); + const { status, stderr } = rig.run(['--version'], + { SWITCHBOARD_SANDBOX_RW_SKILLS: '1', NVM_DIR: nvm, SWITCHBOARD_SANDBOX_BINDS: nvm }); + assert.equal(status, 0, stderr); + const ops = parseMounts(rig.lastBwrapArgs()); + assert.ok(ops.some(o => o.op === '--ro-bind' && o.dest === nvm), 'NVM_DIR is mounted read-only first'); + assert.equal(accessAt(ops, path.join(nvm, 'skills', 'mine')), '--bind'); + } finally { + rig.cleanup(); + } + }); + test('sandbox wrapper: an opted-in skills may contain a read-only path, or share its target with the project\'s own skills', { skip: !LINUX && 'linux only' }, () => { const rig = makeRig({ recordArgs: true }); From 8bf1c901b898ad7e602c201a85fef2dd2e711b1e Mon Sep 17 00:00:00 2001 From: abate Date: Fri, 9 Oct 2026 15:54:53 +0200 Subject: [PATCH 09/17] fix(restore): flush on a quit that joins a reload check, and keep the active marker A quit or close that joins an unanswered reload question now tells the renderer the question is an exit, so the answer still waits for the final write. A cancelled Windows logoff no longer approves every later quit. Quitting before the restore ends keeps the saved active session marked. Tests pin the startup read, the awaited flush, the skip flag on a direct save and the exit-flush listener. --- .ai/contexts/viewer-panel.md | 2 +- docs/session-restore.md | 11 ++-- preload.js | 3 ++ public/app.js | 7 ++- public/file-panel.js | 11 +++- test/dom-file-panel-unsaved-guard.test.js | 34 ++++++++++-- test/exit-flush.test.js | 66 +++++++++++++++++++++-- test/schedule-live-resume.test.js | 2 +- test/unsaved-guard.test.js | 42 ++++++++++++++- unsaved-guard.js | 40 +++++++++----- 10 files changed, 187 insertions(+), 31 deletions(-) diff --git a/.ai/contexts/viewer-panel.md b/.ai/contexts/viewer-panel.md index 6807dc344..d487ac5ad 100644 --- a/.ai/contexts/viewer-panel.md +++ b/.ai/contexts/viewer-panel.md @@ -164,7 +164,7 @@ Each of these reads and clears `pendingTouchedOpen` before `destroyCurrentTab`, Nothing ends the window with unsaved file-panel edits without the user saying so. The scope is the Touched editor in the slot (a tab with an `absolutePath`) and every `touchedStashes` entry, in every session of `filePanelState`; the Memory and Work Files panels, MCP diff tabs and Changes buffers are not covered. -- **Main** (`unsaved-guard.js`, one guard, `attach(win)` per window, `beforeQuit(event, win)` for the app). `main.js`'s `before-quit` handler calls `beforeQuit` first: it prevents the quit, asks the renderer, and calls `app.quit()` again on a yes, so the cleanup that follows (PTYs killed, MCP servers, watchers) runs only once the quit is confirmed and a Cancel leaves the app intact. This covers every `app.quit()` caller (☰ Quit, the last window closing). `updater-install` asks first (`confirmQuit`), because electron-updater's `quitAndInstall()` starts the installer before it calls `app.quit()`: a Cancel must leave the installer unstarted, and a yes pre-approves the quit that follows. A Windows `query-session-end` or `session-end` approves the quit, so logoff and shutdown never wait on the dialog. A window close, a quit and an install share one question while it is open. The window's own `close` event is held the same way for a plain window close. The question is `unsaved-check` (`id`, `'quit'` or `'reload'`); the renderer's `unsaved-check-result` (`id`, `proceed`) approves it. A `will-prevent-unload` (the renderer's `beforeunload` veto, which a reload hits) asks with `'reload'`; on a yes the next unload is allowed once (`allowNextUnload`, answered with `preventDefault()`, which in Electron means "unload anyway") and `webContents.reload()` is called. Every close asks the renderer, even with nothing dirty; a main-side dirty flag would save that round trip and is not built. +- **Main** (`unsaved-guard.js`, one guard, `attach(win)` per window, `beforeQuit(event, win)` for the app). `main.js`'s `before-quit` handler calls `beforeQuit` first: it prevents the quit, asks the renderer, and calls `app.quit()` again on a yes, so the cleanup that follows (PTYs killed, MCP servers, watchers) runs only once the quit is confirmed and a Cancel leaves the app intact. This covers every `app.quit()` caller (☰ Quit, the last window closing). `updater-install` asks first (`confirmQuit`), because electron-updater's `quitAndInstall()` starts the installer before it calls `app.quit()`: a Cancel must leave the installer unstarted, and a yes pre-approves the quit that follows. A Windows `session-end` approves the quit, so logoff and shutdown never wait on the dialog; `query-session-end` approves it for 60 s (`LOGOFF_CANCEL_MS`), so a logoff that is cancelled does not skip the question on every later quit. A window close, a quit and an install share one question while it is open; one that joins a `'reload'` question sends `unsaved-check-reason` (`id`, `'quit'`), and the renderer acts on the reason it holds when the user answers. The window's own `close` event is held the same way for a plain window close. The question is `unsaved-check` (`id`, `'quit'` or `'reload'`); the renderer's `unsaved-check-result` (`id`, `proceed`) approves it. A `will-prevent-unload` (the renderer's `beforeunload` veto, which a reload hits) asks with `'reload'`; on a yes the next unload is allowed once (`allowNextUnload`, answered with `preventDefault()`, which in Electron means "unload anyway") and `webContents.reload()` is called. Every close asks the renderer, even with nothing dirty; a main-side dirty flag would save that round trip and is not built. - **Bounded.** The renderer acknowledges (`unsaved-check-ack`) as soon as it receives the check, before any dialog. The 2.5 s bound (`DEFAULT_TIMEOUT_MS`) covers only send to ack: no ack answers yes, so a hung renderer never keeps the app from quitting. After the ack the guard waits for the answer without a limit, so a slow user or a slow save loses nothing, and answers yes only if the renderer process is gone (`render-process-gone`, `destroyed`) or a send fails. A late answer is ignored. - **Renderer** (`file-panel.js`). `askAboutUnsavedEdits()` lists the dirty tabs (`collectUnsavedFileTabs`) in the `#unsaved-edits-dialog` dialog, built on the add-project dialog's classes (already in the frameless no-drag list). Save writes every dirty entry: the editor in the slot through `handleChangesSave`, a stashed one through `saveFileForPanel(filePath, content, savedContent, {git, version})`, removed from its session once written. A save that fails (including a disk that moved) keeps the dialog open with the reason, and nothing is answered. Discard answers yes. Cancel and Escape answer no. A second request while the dialog is open gets the same dialog's answer. - **`beforeunload`.** The renderer vetoes an unload while an entry is dirty, unless `unloadApproved`, set for 10 s after the user answered yes. That is what stops a reload from the keyboard or devtools from slipping past, and what keeps the veto from asking a second time after an approved close. diff --git a/docs/session-restore.md b/docs/session-restore.md index 49b9b135b..cf0bfb700 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -37,14 +37,17 @@ in `public/app.js`, called by the `unsaved-check` handler in window open), and stops saving it for the next 10 s. Main stops sending `process-exited` once `before-quit` starts killing the sessions (`appQuitting`), so the shutdown cannot record them as closed. A reload is -not an exit and does neither. +not an exit and does neither. A quit or close that arrives while a reload's +question is still open joins it, and main tells the renderer the question is +now an exit (`unsaved-check-reason`), so the answer still waits for the write. The write keeps the saved sessions that are not open yet: those the index has not reached, those offered by the Restore prompt and not answered, and those the restore is still starting (`pendingRestoreEntries`). So quitting during a cold index, before answering the prompt, or in the middle of a restore keeps them for the next start, while a session you stopped during the restore is -left out. An exit before the saved set has even been read writes nothing. +left out. The saved active marker stays on its session until the restore has +opened it. An exit before the saved set has even been read writes nothing. If the exit does not happen after all (an installer that fails to start), saving resumes after the 10 s, and the set is written then if anything asked @@ -53,7 +56,9 @@ for a save in the meantime. A Windows logoff or shutdown (`query-session-end`, `session-end`) does not ask about unsaved edits; main sends `exit-flush` instead, and the renderer writes the set without waiting for an answer. That write is best effort: -Windows may end the app before it lands. +Windows may end the app before it lands. A `query-session-end` approves quits +for 60 s only: if the logoff is cancelled and no `session-end` follows, later +quits ask about unsaved edits and write the set again. ## What restore does diff --git a/preload.js b/preload.js index a254b8e2c..6b16d92aa 100644 --- a/preload.js +++ b/preload.js @@ -162,6 +162,9 @@ contextBridge.exposeInMainWorld('api', { onUnsavedCheck: (callback) => { ipcRenderer.on('unsaved-check', (_event, id, reason) => callback(id, reason)); }, + onUnsavedCheckReason: (callback) => { + ipcRenderer.on('unsaved-check-reason', (_event, id, reason) => callback(id, reason)); + }, unsavedCheckAck: (id) => ipcRenderer.send('unsaved-check-ack', id), unsavedCheckResult: (id, proceed) => ipcRenderer.send('unsaved-check-result', id, proceed), onExitFlush: (callback) => { diff --git a/public/app.js b/public/app.js index 8bcb76404..b7ad92d19 100644 --- a/public/app.js +++ b/public/app.js @@ -198,11 +198,14 @@ function persistWorkingSet({ final = false } = {}) { active: sessionId === activeSessionId, }); } - const held = [...skippedWorkingSetEntries.values(), ...pendingRestoreEntries()] + const notStarted = pendingRestoreEntries().filter(({ item }) => !openSessions.has(item.sessionId)); + const heldActive = notStarted.find(({ item }) => item.active); + if (heldActive) for (const e of set) e.active = false; + const held = [...skippedWorkingSetEntries.values(), ...notStarted] .filter(({ item }) => !openSessions.has(item.sessionId)) .sort((a, b) => a.index - b.index); for (const { item, index } of held) { - set.splice(Math.min(index, set.length), 0, { sessionId: item.sessionId, projectPath: item.projectPath, active: false }); + set.splice(Math.min(index, set.length), 0, { sessionId: item.sessionId, projectPath: item.projectPath, active: heldActive?.item === item }); } global.openWorkingSet = set; await window.api.setSetting('global', global); diff --git a/public/file-panel.js b/public/file-panel.js index 0aa27f077..3b93e6973 100644 --- a/public/file-panel.js +++ b/public/file-panel.js @@ -150,12 +150,21 @@ function initFilePanel() { event.returnValue = false; }); if (window.api.onUnsavedCheck) { + const unsavedCheckReasons = new Map(); + if (window.api.onUnsavedCheckReason) { + window.api.onUnsavedCheckReason((id, reason) => { + if (unsavedCheckReasons.has(id)) unsavedCheckReasons.set(id, reason); + }); + } window.api.onUnsavedCheck(async (id, reason) => { + unsavedCheckReasons.set(id, reason); window.api.unsavedCheckAck(id); let proceed = true; try { proceed = await askAboutUnsavedEdits(); } catch (err) { console.error('[unsaved-check]', err); } + const finalReason = unsavedCheckReasons.get(id); + unsavedCheckReasons.delete(id); // see docs/session-restore.md ("Closing the app") - if (proceed && reason !== 'reload' && typeof window.flushStateForExit === 'function') { + if (proceed && finalReason !== 'reload' && typeof window.flushStateForExit === 'function') { const bound = new Promise((resolve) => setTimeout(resolve, EXIT_FLUSH_BOUND_MS)); try { await Promise.race([window.flushStateForExit(), bound]); } catch (err) { console.error('[exit-flush]', err); } } diff --git a/test/dom-file-panel-unsaved-guard.test.js b/test/dom-file-panel-unsaved-guard.test.js index 371e141b7..ecff8a8c1 100644 --- a/test/dom-file-panel-unsaved-guard.test.js +++ b/test/dom-file-panel-unsaved-guard.test.js @@ -48,6 +48,7 @@ function setup() { window.api = new Proxy({ onMcpOpenFile: (cb) => { calls.openFile = cb; }, onUnsavedCheck: (cb) => { calls.check = cb; }, + onUnsavedCheckReason: (cb) => { calls.reason = cb; }, unsavedCheckAck: (id) => { calls.acks.push({ id, at: calls.answers.length, dialog: !!window.document.getElementById('unsaved-edits-dialog') }); }, unsavedCheckResult: (id, proceed) => { calls.answers.push({ id, proceed }); }, watchFile: () => Promise.resolve({ ok: true }), @@ -240,18 +241,41 @@ test('the check is acknowledged on receipt, before any dialog is shown', async ( }); // see docs/session-restore.md ("Closing the app") -test('a confirmed close or quit flushes the app state before answering; a reload does not', async () => { +test('a confirmed close or quit answers only once the app state is written; a reload does not flush', async () => { const ctx = setup(); try { - const flushed = []; - ctx.window.flushStateForExit = () => { flushed.push(ctx.calls.answers.length); return Promise.resolve(); }; + const flushes = []; + ctx.window.flushStateForExit = () => new Promise((resolve) => flushes.push(resolve)); ctx.calls.check(1, 'quit'); await flush(); - assert.deepEqual(flushed, [0], 'flushed before the answer was sent'); + assert.equal(flushes.length, 1); + assert.deepEqual(ctx.calls.answers, [], 'no answer while the write is in flight'); + flushes[0](); + await flush(); assert.deepEqual(ctx.calls.answers, [{ id: 1, proceed: true }]); ctx.calls.check(2, 'reload'); await flush(); - assert.equal(flushed.length, 1); + assert.equal(flushes.length, 1); + assert.deepEqual(ctx.calls.answers[1], { id: 2, proceed: true }); + } finally { ctx.destroy(); } +}); + +test('a reload check that a quit joins before it is answered flushes once', async () => { + const ctx = setup(); + try { + let flushed = 0; + ctx.window.flushStateForExit = () => { flushed++; return Promise.resolve(); }; + await dirtyTab(ctx, 's1', A); + ctx.calls.check(4, 'reload'); + await flush(); + ctx.calls.reason(4, 'quit'); + button(ctx, 'unsaved-discard').click(); + await flush(); + assert.equal(flushed, 1); + assert.deepEqual(ctx.calls.answers, [{ id: 4, proceed: true }]); + ctx.calls.check(5, 'reload'); + await flush(); + assert.equal(flushed, 1, 'a later reload is not taken for a quit'); } finally { ctx.destroy(); } }); diff --git a/test/exit-flush.test.js b/test/exit-flush.test.js index 93c2aa6b3..ebbc2afc4 100644 --- a/test/exit-flush.test.js +++ b/test/exit-flush.test.js @@ -19,6 +19,7 @@ function setup() { Promise, openSessions: new Map(), activeSessionId: null, + document: { getElementById: () => null }, skippedWorkingSetEntries: new Map(), setTimeout: (fn) => { const id = nextTimer++; timers.set(id, fn); return id; }, clearTimeout: (id) => { timers.delete(id); }, @@ -32,16 +33,17 @@ function setup() { const fns = loadAppFunctions(context, { declarations: ['restoringWorkingSet', 'persistWorkingSetTimer', '_persistChain', 'exitingApp', 'exitingAppTimer', 'persistSkippedWhileExiting', 'savedWorkingSetRead', 'EXIT_FLUSH_GRACE_MS', 'restorePlanner', 'restoreSavedIndex', - 'restoreAwaitingConsent', 'restoreInFlight'], - functions: ['persistWorkingSet', 'pendingRestoreEntries', 'schedulePersistWorkingSet', 'flushStateForExit'], + 'restoreAwaitingConsent', 'restoreInFlight', 'restoreMode'], + functions: ['persistWorkingSet', 'pendingRestoreEntries', 'schedulePersistWorkingSet', 'flushStateForExit', 'restoreWorkingSet'], }); + vm.runInContext(fs.readFileSync(path.join(__dirname, '..', 'public', 'setting-defaults.js'), 'utf8'), context); vm.runInContext(fs.readFileSync(path.join(__dirname, '..', 'public', 'restore-plan.js'), 'utf8'), context); vm.runInContext('savedWorkingSetRead = true', context); const open = (id) => context.openSessions.set(id, { session: { sessionId: id, projectPath: '/p' }, closed: false }); const runTimers = () => { const fns = [...timers.values()]; timers.clear(); for (const fn of fns) fn(); }; const run = (code) => vm.runInContext(code, context); - const saved = (ids) => { - context.savedSet = ids.map((sessionId) => ({ sessionId, projectPath: '/p', active: false })); + const saved = (ids, active) => { + context.savedSet = ids.map((sessionId) => ({ sessionId, projectPath: '/p', active: sessionId === active })); store.global.openWorkingSet = context.savedSet; run('restoreSavedIndex = new Map(savedSet.map((item, index) => [item.sessionId, index]))'); }; @@ -136,3 +138,59 @@ test('a save asked during the grace period of an exit that did not happen is wri await quiet.run('_persistChain'); assert.deepEqual(quiet.writes, [['a']], 'nothing asked, nothing written again'); }); + +test('the startup read of the saved set is what lets a later exit write', async () => { + const t = setup(); + t.run('savedWorkingSetRead = false'); + t.store.global.restoreOnStartup = 'off'; + t.store.global.openWorkingSet = []; + await t.restoreWorkingSet(); + t.open('a'); + await t.flushStateForExit(); + assert.deepEqual(t.writes, [['a']]); +}); + +test('a direct save during the grace period of an exit that did not happen is written when it ends', async () => { + const t = setup(); + t.open('a'); + await t.flushStateForExit(); + t.open('b'); + await t.persistWorkingSet(); + assert.deepEqual(t.writes, [['a']], 'held during the grace period'); + t.runTimers(); + await t.run('_persistChain'); + assert.deepEqual(t.writes, [['a'], ['a', 'b']]); +}); + +test('an exit before the restore ends keeps the saved active session marked', async () => { + const t = setup(); + t.saved(['a', 'b', 'c'], 'b'); + t.run('restoreInFlight.set("b", savedSet[1]); restoreInFlight.set("c", savedSet[2])'); + t.open('a'); + t.context.activeSessionId = 'a'; + await t.flushStateForExit(); + assert.deepEqual(t.store.global.openWorkingSet.map((e) => [e.sessionId, e.active]), [['a', false], ['b', true], ['c', false]]); + + const opened = setup(); + opened.saved(['a', 'b'], 'b'); + opened.open('a'); + opened.open('b'); + opened.context.activeSessionId = 'a'; + await opened.flushStateForExit(); + assert.deepEqual(opened.store.global.openWorkingSet.map((e) => [e.sessionId, e.active]), [['a', true], ['b', false]], 'once restored, the live choice counts'); +}); + +test('a Windows session end flushes the app state', async () => { + const src = fs.readFileSync(path.join(__dirname, '..', 'public', 'app.js'), 'utf8'); + const line = src.split('\n').find((l) => l.includes('window.api.onExitFlush')); + assert.ok(line, 'app.js registers the exit-flush listener'); + const t = setup(); + let listener = null; + t.context.window.api.onExitFlush = (cb) => { listener = cb; }; + t.run(line); + t.open('a'); + t.schedulePersistWorkingSet(); + listener(); + await t.run('_persistChain'); + assert.deepEqual(t.writes, [['a']]); +}); diff --git a/test/schedule-live-resume.test.js b/test/schedule-live-resume.test.js index d68a9cfb1..b63ff2a93 100644 --- a/test/schedule-live-resume.test.js +++ b/test/schedule-live-resume.test.js @@ -101,7 +101,7 @@ function setup(t, { sandbox = false } = {}) { `, ctx); const app = loadAppFunctions(ctx, { functions: ['openSession', 'runRestore', 'pendingRestoreEntries', 'persistWorkingSet', 'showRestoreNotice', 'showLiveElsewhereNotice'], - declarations: ['LIVE_ELSEWHERE_NOTICE_MS', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight'], + declarations: ['LIVE_ELSEWHERE_NOTICE_MS', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'exitingApp', 'persistSkippedWhileExiting'], }); ctx.sessionMap.set(SID, session); t.after(() => { diff --git a/test/unsaved-guard.test.js b/test/unsaved-guard.test.js index 7ad2b4dc3..c94c84610 100644 --- a/test/unsaved-guard.test.js +++ b/test/unsaved-guard.test.js @@ -7,7 +7,7 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const { EventEmitter } = require('node:events'); -const { createUnsavedGuard } = require('../unsaved-guard'); +const { createUnsavedGuard, LOGOFF_CANCEL_MS } = require('../unsaved-guard'); function setup({ timeoutMs = 1000, quit } = {}) { const ipcMain = new EventEmitter(); @@ -259,3 +259,43 @@ test('a window close and a quit share one question, and one answer settles both' const again = t.closeEvent(); assert.equal(t.guard.beforeQuit(again, t.win), false, 'the same yes approved the quit'); }); + +test('a quit that joins an unanswered reload check tells the renderer it is now an exit, once', async () => { + const quits = []; + const t = setup({ quit: () => quits.push(1) }); + t.wc.emit('will-prevent-unload', t.closeEvent()); + const id = t.sent[0].args[0]; + assert.equal(t.sent[0].args[1], 'reload'); + assert.equal(t.guard.beforeQuit(t.closeEvent(), t.win), true); + t.win.emit('close', t.closeEvent()); + assert.deepEqual(t.sent.map((m) => m.channel), ['unsaved-check', 'unsaved-check-reason'], 'one upgrade, no second question'); + assert.deepEqual(t.sent[1].args, [id, 'quit']); + t.answer(id, true); + await tick(); + assert.equal(quits.length, 1); +}); + +test('a reload joining a quit check leaves it a quit', () => { + const t = setup(); + t.win.emit('close', t.closeEvent()); + t.wc.emit('will-prevent-unload', t.closeEvent()); + assert.deepEqual(t.sent.map((m) => m.channel), ['unsaved-check']); +}); + +test('a logoff that never ends the session stops approving quits after a while; a session end keeps it', () => { + const t = setup(); + t.win.emit('query-session-end', {}); + const revert = t.timers.find((x) => x.ms === LOGOFF_CANCEL_MS && !x.cleared); + assert.ok(revert, 'the approval is temporary'); + assert.equal(t.guard.beforeQuit(t.closeEvent(), t.win), false); + revert.fn(); + const e = t.closeEvent(); + assert.equal(t.guard.beforeQuit(e, t.win), true, 'a cancelled logoff asks again on the next quit'); + assert.equal(e.prevented, true); + + const s = setup(); + s.win.emit('query-session-end', {}); + s.win.emit('session-end', {}); + assert.ok(s.timers.filter((x) => x.ms === LOGOFF_CANCEL_MS).every((x) => x.cleared)); + assert.equal(s.guard.beforeQuit(s.closeEvent(), s.win), false); +}); diff --git a/unsaved-guard.js b/unsaved-guard.js index c5bcba6dc..46a5638ed 100644 --- a/unsaved-guard.js +++ b/unsaved-guard.js @@ -3,13 +3,17 @@ // see .ai/contexts/viewer-panel.md ("Unsaved edits on quit, reload and close") const DEFAULT_TIMEOUT_MS = 2500; +const LOGOFF_CANCEL_MS = 60000; -function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeoutFn = setTimeout, clearTimeoutFn = clearTimeout, quit = () => {} }) { +function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, logoffCancelMs = LOGOFF_CANCEL_MS, setTimeoutFn = setTimeout, clearTimeoutFn = clearTimeout, quit = () => {} }) { const pending = new Map(); let nextId = 1; let quitApproved = false; + let logoffApproved = false; + let logoffTimer = null; let quitAsking = false; let inflight = null; + const approvedToQuit = () => quitApproved || logoffApproved; ipcMain.on('unsaved-check-ack', (_event, id) => { const entry = pending.get(id); @@ -25,12 +29,18 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou }); function ask(win, reason) { - if (inflight) return inflight; + if (inflight) { + if (inflight.reason === 'reload') { + inflight.reason = reason; + try { win.webContents.send('unsaved-check-reason', inflight.id, reason); } catch { /* the check ends without it */ } + } + return inflight.asked; + } const wc = win.webContents; if (win.isDestroyed() || !wc || wc.isDestroyed() || wc.isCrashed()) return Promise.resolve(true); let settled = false; + const id = nextId++; const asked = new Promise((resolve) => { - const id = nextId++; const entry = { acked: false, timer: null, finish: null }; const onGone = () => entry.finish(true); entry.finish = (proceed) => { @@ -53,12 +63,12 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou entry.finish(true); } }); - if (!settled) inflight = asked; + if (!settled) inflight = { asked, id, reason }; return asked; } function beforeQuit(event, win) { - if (quitApproved || !win || win.isDestroyed()) return false; + if (approvedToQuit() || !win || win.isDestroyed()) return false; event.preventDefault(); if (quitAsking) return true; quitAsking = true; @@ -77,18 +87,22 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou let reloading = false; let allowNextUnload = false; - // see docs/session-restore.md ("Closing the app") - const endSession = () => { - approveQuit(); + const endSession = (final) => { + logoffApproved = true; + if (logoffTimer) clearTimeoutFn(logoffTimer); + logoffTimer = final ? null : setTimeoutFn(() => { + logoffTimer = null; + logoffApproved = false; + }, logoffCancelMs); const wc = win.webContents; if (win.isDestroyed() || !wc || wc.isDestroyed()) return; try { wc.send('exit-flush'); } catch { /* the window is going anyway */ } }; - win.on('query-session-end', endSession); - win.on('session-end', endSession); + win.on('query-session-end', () => endSession(false)); + win.on('session-end', () => endSession(true)); win.on('close', (event) => { - if (approved || quitApproved) return; + if (approved || approvedToQuit()) return; event.preventDefault(); if (closing) return; closing = true; @@ -101,7 +115,7 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou }); win.webContents.on('will-prevent-unload', (event) => { - if (approved || quitApproved || allowNextUnload) { + if (approved || approvedToQuit() || allowNextUnload) { allowNextUnload = false; event.preventDefault(); return; @@ -131,4 +145,4 @@ function createUnsavedGuard({ ipcMain, timeoutMs = DEFAULT_TIMEOUT_MS, setTimeou return { attach, beforeQuit, confirmQuit, approveQuit }; } -module.exports = { createUnsavedGuard, DEFAULT_TIMEOUT_MS }; +module.exports = { createUnsavedGuard, DEFAULT_TIMEOUT_MS, LOGOFF_CANCEL_MS }; From 6923cd71ab66547f92f6f2c5fba2735f011c1536 Mon Sep 17 00:00:00 2001 From: abate Date: Fri, 9 Oct 2026 16:43:12 +0200 Subject: [PATCH 10/17] fix(sessions): hold header-only files, restore a cleared session, one lock per terminal A transcript read before its /clear record stays eligible until its first turn. A snapshot-only file is taken as an awaited fork only when the CLI under the PTY owns it. A session with no transcript yet is saved as a fresh entry and restored as a new session in its project. Trigger locks are keyed by the PTY, so the old and new id of a re-keyed session share one queue; every older id now resolves to the newest in one lookup. --- .ai/contexts/cli-session-state.md | 35 ++++-- .ai/contexts/session-cache.md | 2 +- .ai/contexts/trigger-watcher.md | 15 ++- CHANGELOG.md | 2 +- docs/session-restore.md | 10 +- public/app.js | 44 ++++++-- public/restore-plan.js | 2 +- session-transitions.js | 55 ++++++---- test/clear-pending-persist.test.js | 115 +++++++++++++++++--- test/session-activity.test.js | 3 +- test/session-transitions.test.js | 169 +++++++++++++++++++++++++++++ test/trigger-clear-chain.test.js | 154 ++++++++++++++++++++++++++ test/trigger-context.test.js | 15 +++ test/trigger-watcher.test.js | 4 +- trigger-watcher.js | 10 +- 15 files changed, 564 insertions(+), 71 deletions(-) create mode 100644 test/trigger-clear-chain.test.js diff --git a/.ai/contexts/cli-session-state.md b/.ai/contexts/cli-session-state.md index df1984f58..6f7f1cfd6 100644 --- a/.ai/contexts/cli-session-state.md +++ b/.ai/contexts/cli-session-state.md @@ -374,10 +374,27 @@ finds the live pid whose state file names `newId` and walks its parent chain Re-keying keeps `oldId → newId` (`currentSessionId`), so a trigger chain that sent `/clear` reaches the same terminal on its next step (see -trigger-watcher.md, "Re-keyed sessions"). A `/clear` transcript starts with a -`file-history-snapshot` record, so the "snapshot-only fork file" matcher -skips files whose first real turn is `/clear`; otherwise a session waiting for -its fork would take another session's `/clear` as its fork. +trigger-watcher.md, "Re-keyed sessions"). On each re-key every older id that +pointed at `oldId` is moved to `newId`, so any id the session ever had resolves +in one lookup, however many times it was cleared. + +**A file with no turn yet.** The CLI writes a `/clear` transcript in several +appends: a mode record and a `file-history-snapshot` can land, and be read, +before the `/clear` record. A new file whose first real turn +(`readNewSessionSignals().hasTurn`) is not written yet stays out of +`knownJsonlFiles`, whatever its other signals, until it shows one or is an +hour old (the same limit as a file with no parseable record). While it is +under 60 s old, `scheduleRecheck` runs the folder again a second later. The +session's own transcript is never held this way. + +**Snapshot-only fork file.** A session started with `--fork-session` waits +for its fork's transcript, which holds only snapshots until the first prompt. +Such a file is taken as the fork only when `clearOwner` (it answers for any +session id, not only a `/clear`) says the CLI under this PTY writes it. A +`pending` owner is rechecked like a `/clear`; `other` or `unknown` leave the +file eligible, and its first turn decides through `forkedFrom` or +`parentSessionId`, as for any fork. Before, any snapshot-only file in the +folder was taken, including another session's `/clear` or new session. `readNewSessionSignals()` recognises the file by its first user record that is not local-command bookkeeping, through `classifyUserText()` — the caveat record @@ -387,10 +404,12 @@ The renderer re-keys as for a fork, and when the old id was not a pending (transcript-less) row it adds one for the new id titled "New session": the `/clear` transcript is not indexed until its first prompt (see session-cache.md), so without it the open terminal would have no row at all -until then. Like any pending row, it is left out of the saved working set -until that prompt (docs/session-restore.md, "A session with no transcript -yet"). The old conversation stays in the list as an ordinary, -resumable session — it is a real transcript on disk. +until then. That row is marked `cleared`, and is not re-added to the default +list when the backend filtered its folder out as archived. Like any local +pending row, it is saved in the working set as a fresh entry +(docs/session-restore.md, "A session with no transcript yet"). The old +conversation stays in the list as an ordinary, resumable session — it is a +real transcript on disk. ## Canary tests diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index cd6e64bc3..62d8f34d7 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -117,7 +117,7 @@ A kept row is recorded with its folder. On PTY exit, `main.js` calls `releaseLiv - **Open question #3 (existing databases)**: repaired on the next index pass, not left alone. `bridgeSessionId` and `mergedIntoSessionId` are added purely via the schema-reconciliation block (not a numbered migration — deliberately, to avoid coupling `migrations.length` to unrelated migration-ordering tests; see `db-schema-reconcile.test.js`'s "foreign higher-version" precedent for why reconciliation is the version-independent mechanism). Their absence sets `mustReindex = true`, which wipes `session_cache` + `cache_meta` + the `initial_scan_complete` marker, forcing every folder through the now-merging indexer on the next scan — the same repair path already used when `fileMtime` (v7) or the fork subagent columns (v4) were introduced. - **"Open on claude.ai" (issue #213).** `buildProjectsFromCache` passes `bridgeSessionId` to the renderer (`null` when absent). `bridgeSessionUrl()` in `public/bridge-url.js` turns it into `https://claude.ai/code/session_`: the transcript record carries `cse_`, the CLI descriptor and the web URL carry `session_`, and the suffix is the same (measured on local transcripts that mention both forms). Any other shape, or an id with a character outside `[A-Za-z0-9]`, gives no URL and the row shows no button. The button (`.session-bridge-btn` in `buildSessionItem`) opens the URL through `window.api.openExternal`, whose main-side handler already refuses anything but `http(s)`. -- **Working-set restore retries until indexing is done, not once.** `populateCacheViaWorker` streams `sessionMap` one folder at a time on a cold start, so a saved working-set id can be missing for many ticks before it's genuinely indexed. `createRestorePlanner()` (`public/restore-plan.js`) is ticked from every `projects-changed` handler and from `updateIndexingBanner` on `payload.done`; it keeps returning `'wait'` until every saved id is indexed or indexing is over (then the rest is presumed deleted), restoring incrementally in `auto` mode and asking once (`askOnce: true`) in `ask` mode instead of re-prompting per tick. The end of indexing reaches the renderer on its own `indexing-finished` channel, sent at the end of **every** `populateCacheViaWorker` run, warm start included: `indexing-progress` is first-run only, so a warm start never told the planner that indexing was over and a saved session missing from the index left the "Finishing indexing" toast up for good. When the planner gives up (indexing over, or the tick cap) it returns the saved entries it never found as `unavailable`, and `tickRestorePlanner` names them in a "Not restored" notice. The end is also pullable (`get-indexing-state`, read once when the planner starts) because the startup scan can finish before the renderer listens, and `markRestoreIndexingDone` reloads the projects before the final tick because the last folders may not have reached `sessionMap` yet. See `test/session-restore-cold-cache.test.js`, `test/restore-unavailable.test.js`. A persist during that window keeps the entries not resolved yet (`pendingRestoreEntries`: the planner's pending ones, the ones awaiting the restore toast, and `restoreInFlight`, those handed to `runRestore` while their live-elsewhere check and open are awaited, even across two overlapping restores), otherwise any click or close would erase them from `openWorkingSet` before they are reached; see `test/restore-pending-persist.test.js`. +- **Working-set restore retries until indexing is done, not once.** `populateCacheViaWorker` streams `sessionMap` one folder at a time on a cold start, so a saved working-set id can be missing for many ticks before it's genuinely indexed. `createRestorePlanner()` (`public/restore-plan.js`) is ticked from every `projects-changed` handler and from `updateIndexingBanner` on `payload.done`; it keeps returning `'wait'` until every saved id is indexed or indexing is over (then the rest is presumed deleted), restoring incrementally in `auto` mode and asking once (`askOnce: true`) in `ask` mode instead of re-prompting per tick. The end of indexing reaches the renderer on its own `indexing-finished` channel, sent at the end of **every** `populateCacheViaWorker` run, warm start included: `indexing-progress` is first-run only, so a warm start never told the planner that indexing was over and a saved session missing from the index left the "Finishing indexing" toast up for good. When the planner gives up (indexing over, or the tick cap) it returns the saved entries it never found as `unavailable`, and `tickRestorePlanner` names them in a "Not restored" notice. The end is also pullable (`get-indexing-state`, read once when the planner starts) because the startup scan can finish before the renderer listens, and `markRestoreIndexingDone` reloads the projects before the final tick because the last folders may not have reached `sessionMap` yet. See `test/session-restore-cold-cache.test.js`, `test/restore-unavailable.test.js`. A persist during that window keeps the entries not resolved yet (`pendingRestoreEntries`: the planner's pending ones, the ones awaiting the restore toast, and `restoreInFlight`, those handed to `runRestore` while their live-elsewhere check and open are awaited, even across two overlapping restores), otherwise any click or close would erase them from `openWorkingSet` before they are reached; see `test/restore-pending-persist.test.js`. A saved entry marked `fresh` (a local session with no transcript yet, see docs/session-restore.md) counts as indexed for the planner, and `runRestore` starts a new session in its project for it; see `test/clear-pending-persist.test.js`. - **Neither the working-set restore nor the reload path resumes a session that is live in another process.** `runRestore` and the post-`loadProjects` re-open of `sessionStorage.activeSessionId` call `openSession(..., { automatic: true })`, which skips the session without a prompt when `guardResume` reports it live elsewhere; the skipped entry is not activated, stays in the persisted working set at its saved position, and is reported by a one-line notice. See `.ai/contexts/cli-session-state.md` ("Live elsewhere"). diff --git a/.ai/contexts/trigger-watcher.md b/.ai/contexts/trigger-watcher.md index 2cbe4ba14..c1f2dbb2b 100644 --- a/.ai/contexts/trigger-watcher.md +++ b/.ai/contexts/trigger-watcher.md @@ -66,8 +66,12 @@ the commands a chain may send. `createTriggerContext` takes `resolveSessionId` (`sessionTransitions.currentSessionId` in main), and every lookup it makes (`getPtyForSession`, `isSessionBusy`, `getComposerState`, `getTranscriptTurn`, `getCliStatus`) goes through it, so the old id keeps reaching the same terminal -and the CLI status is read under the id the CLI now writes. Results still -carry the id the trigger named. +and the CLI status is read under the id the CLI now writes, as is +`getLiveDescriptor`. Results still carry the id the trigger named. +`test/trigger-clear-chain.test.js` runs `[/clear, reply]` through the shipped +watcher, with the re-key done by `session-transitions.js` while step 0 is +verified; without the resolution that chain ends with "session exited during +wait" at step 0. ### Session handle (2026-09-08, issue #220) @@ -344,8 +348,11 @@ in-flight write. Two commands landing in one composer half-typed into each other is a worse failure than anything `submitted` reports: corrupted input, not just a wrong report field. -`start()` keeps `sessionLocks`, a `Map` used as a FIFO -mutex queue (`acquireSessionLock`). `processTriggerFile` awaits it right after +`start()` keeps `sessionLocks`, a `Map` used as a FIFO mutex queue +(`acquireSessionLock`), keyed by the PTY that `getPtyForSession` returns, or by +the session id when there is none. A fork or `/clear` gives one terminal a new +id; keyed by the PTY, triggers naming the old and the new id share one queue +(see "Re-keyed sessions"). `processTriggerFile` awaits it right after `sessionId` is known to be a non-empty string — before any wait, any PTY lookup, any write — and releases it in the function's own `finally`, so every exit path (success, validation refusal, thrown exception) releases exactly diff --git a/CHANGELOG.md b/CHANGELOG.md index 939137f99..3b6ca1578 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc - When the Claude CLI reports a session's id in a different letter case, the subagents that session started or finished during a turn now show up when the turn ends. (#487) - A session that moves into a worktree while it runs stays in the sidebar, under its project, instead of disappearing while its terminal keeps running. A running session also stays listed when its transcript leaves its folder for any other reason. (#485) - Worktree folders from another host, or whose repository is not listed, show in the sidebar instead of being nested under the wrong folder or not shown at all. The worktrees of a repository hidden with Hide Project stay hidden. (#473) -- `/clear` in a session keeps the open terminal on its sidebar row, which now follows the new conversation, instead of listing that conversation as a separate session. The cleared conversation stays in the list as a past session. (#477) +- `/clear` in a session keeps the open terminal on its sidebar row, which now follows the new conversation, instead of listing that conversation as a separate session. The cleared conversation stays in the list as a past session, and a session quit after `/clear` before its first prompt comes back on restore as a new session in the same project. (#477) - Remote triggers refuse commands containing invisible format characters, default-ignorable characters or braille blanks, including joined emoji, emoji with variation selectors (such as hearts), soft hyphens and right-to-left marks. Fullwidth slash, exclamation and number-sign prefixes are refused too. (#440) ## v0.0.89 — 2026-10-04 diff --git a/docs/session-restore.md b/docs/session-restore.md index 64d066398..2965111a0 100644 --- a/docs/session-restore.md +++ b/docs/session-restore.md @@ -25,9 +25,13 @@ session's id and project, and which one was active — in its global settings A session started with **+**, or one just cleared with `/clear`, has no conversation until its first prompt: the CLI has nothing to resume, and the -index does not list it. It is left out of the saved set, so the next start -does not report it as "not in the index". Once its first prompt makes it a -real session, the set is saved again with it. +index does not list it. It is saved as a *fresh* entry (`fresh: true`, with +its project). On the next start a fresh entry needs no index: restore starts +a new session in that project, with the project's current defaults for a new +session, except that it never creates a worktree, since the saved project +path already is the folder to start in. Once its first prompt makes it a real +session, the set is saved again with it as an ordinary entry. A session +without a transcript on a remote host is still left out. ## What restore does diff --git a/public/app.js b/public/app.js index 18e0cc245..efb9d50d4 100644 --- a/public/app.js +++ b/public/app.js @@ -182,18 +182,20 @@ function persistWorkingSet() { if (entry.attach) continue; // attach tabs are not restored if (entry.closed) continue; // see docs/session-restore.md ("A session with no transcript yet") - if (pendingSessions.has(sessionId)) continue; + const fresh = pendingSessions.has(sessionId); + if (fresh && entry.session.remoteAlias) continue; set.push({ sessionId, projectPath: entry.session.projectPath, active: sessionId === activeSessionId, + ...(fresh ? { fresh: true } : {}), }); } const held = [...skippedWorkingSetEntries.values(), ...pendingRestoreEntries()] .filter(({ item }) => !openSessions.has(item.sessionId)) .sort((a, b) => a.index - b.index); for (const { item, index } of held) { - set.splice(Math.min(index, set.length), 0, { sessionId: item.sessionId, projectPath: item.projectPath, active: false }); + set.splice(Math.min(index, set.length), 0, { sessionId: item.sessionId, projectPath: item.projectPath, active: false, ...(item.fresh ? { fresh: true } : {}) }); } global.openWorkingSet = set; await window.api.setSetting('global', global); @@ -221,12 +223,28 @@ function schedulePersistWorkingSet() { } async function runRestore(list) { - const pending = list.filter(item => sessionMap.has(item.sessionId) && !openSessions.has(item.sessionId)); + const pending = list.filter(item => (item.fresh || sessionMap.has(item.sessionId)) && !openSessions.has(item.sessionId)); for (const item of pending) restoreInFlight.set(item.sessionId, item); const skippedNow = []; + const startedAs = new Map(); try { - const liveById = await liveElsewhereMany(pending.map(item => item.sessionId), { api: window.api }); + const liveById = await liveElsewhereMany(pending.filter(item => !item.fresh).map(item => item.sessionId), { api: window.api }); for (const [position, item] of list.entries()) { + if (item.fresh) { + if (!restoreInFlight.has(item.sessionId)) continue; + try { + const project = { projectPath: item.projectPath }; + const options = await resolveDefaultSessionOptions(project); + delete options.worktree; + delete options.worktreeName; + const newId = await launchNewSession(project, options); + if (newId) startedAs.set(item.sessionId, newId); + } finally { + restoreInFlight.delete(item.sessionId); + } + await new Promise(r => setTimeout(r, RESTORE_STAGGER_MS)); + continue; + } const s = sessionMap.get(item.sessionId); if (!s || openSessions.has(item.sessionId)) { restoreInFlight.delete(item.sessionId); @@ -255,8 +273,9 @@ async function runRestore(list) { if (skippedNow.length) showLiveElsewhereNotice(skippedNow); // Activate the entry marked active (or the last one) const activeItem = list.find(i => i.active) || list[list.length - 1]; - if (activeItem && openSessions.has(activeItem.sessionId)) { - showSession(activeItem.sessionId); + const activeId = activeItem && (startedAs.get(activeItem.sessionId) || activeItem.sessionId); + if (activeId && openSessions.has(activeId)) { + showSession(activeId); } } @@ -490,7 +509,7 @@ window.api.onSessionDetected((tempId, realId) => { pollActiveSessions(); }); -window.api.onSessionForked((oldId, newId) => { +function applySessionForked(oldId, newId) { if (window.ATRACE) window.atrace('recv.session-forked', newId, { oldId, known: openSessions.has(oldId) }); const entry = openSessions.get(oldId); if (!entry) return; @@ -516,7 +535,7 @@ window.api.onSessionForked((oldId, newId) => { // see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") const projectPath = entry.session.projectPath; const session = { ...entry.session, sessionId: newId, summary: 'New session', firstPrompt: '', name: null, messageCount: 0 }; - pendingSessions.set(newId, { session, projectPath, folder: encodeProjectPath(projectPath) }); + pendingSessions.set(newId, { session, projectPath, folder: encodeProjectPath(projectPath), cleared: true }); } sessionMap.delete(oldId); sessionMap.set(newId, entry.session); @@ -534,7 +553,9 @@ window.api.onSessionForked((oldId, newId) => { }); schedulePersistWorkingSet(); pollActiveSessions(); -}); +} + +window.api.onSessionForked(applySessionForked); function applyProcessExit(sessionId, exitCode, signal, stopped) { if (window.ATRACE) window.atrace('recv.process-exited', sessionId, { exitCode, signal, stopped }); @@ -1171,6 +1192,8 @@ async function loadProjects({ resort = false } = {}) { for (const projList of [cachedProjects, cachedAllProjects]) { const pendingAlias = pending.session.remoteAlias || null; let proj = projList.find(p => p.projectPath === pending.projectPath && (p.remoteAlias || null) === pendingAlias); + if (!proj && pending.cleared && projList === cachedProjects + && allProjects.some(p => p.projectPath === pending.projectPath && (p.remoteAlias || null) === pendingAlias)) continue; if (!proj) { // Project not in list (no other sessions) — create a synthetic entry proj = { folder: pending.folder, projectPath: pending.projectPath, sessions: [] }; @@ -1255,7 +1278,7 @@ async function launchNewSession(project, sessionOptions) { entry.terminal.write(`\r\nError: ${result.error}\r\n`); entry.closed = true; showSession(sessionId); - return; + return null; } syncPtySizeAfterOpen(entry, result); if (typeof setSessionMcpState === 'function') setSessionMcpState(sessionId, result.mcpState, result.mcpError); @@ -1264,6 +1287,7 @@ async function launchNewSession(project, sessionOptions) { showSession(sessionId); schedulePersistWorkingSet(); pollActiveSessions(); + return sessionId; } // see .ai/contexts/session-cache.md ("Remote hosts — launching a session") diff --git a/public/restore-plan.js b/public/restore-plan.js index d7149658a..3b0dac3c1 100644 --- a/public/restore-plan.js +++ b/public/restore-plan.js @@ -37,7 +37,7 @@ function createRestorePlanner({ savedSet, maxTicks = 50, askOnce = false } = {}) return { action: 'nothing', candidates: [], remaining: 0, unavailable: [] }; } - const indexedIds = [...remaining].filter(id => sessionMap && sessionMap.has(id)); + const indexedIds = [...remaining].filter(id => items.get(id).fresh || (sessionMap && sessionMap.has(id))); const allIndexed = indexedIds.length === remaining.size; if (askOnce) { diff --git a/session-transitions.js b/session-transitions.js index f62fab6a0..5d416e284 100644 --- a/session-transitions.js +++ b/session-transitions.js @@ -26,13 +26,15 @@ function init(ctx) { // see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") const rekeyed = new Map(); -const MAX_REKEY_HOPS = 32; /** The id a session is keyed under now, following the re-keys of forks and /clear. */ function currentSessionId(sessionId) { - let id = sessionId; - for (let hop = 0; hop < MAX_REKEY_HOPS && rekeyed.has(id); hop++) id = rekeyed.get(id); - return id; + return rekeyed.get(sessionId) || sessionId; +} + +function recordRekey(oldId, newId) { + for (const [from, to] of rekeyed) if (to === oldId) rekeyed.set(from, newId); + rekeyed.set(oldId, newId); } const recheckTimers = new Map(); @@ -339,6 +341,7 @@ function readNewSessionSignals(filePath) { let parentSessionId = null; let hasSnapshots = false; let cleared = false; + let hasTurn = false; for (const line of lines) { const entry = JSON.parse(line); // Skip snapshot lines — they carry no fork/session signals @@ -355,18 +358,23 @@ function readNewSessionSignals(filePath) { cleared = kind.kind === 'command' && kind.text === '/clear'; } // Stop after finding a user or assistant message - if (entry.type === 'user' || entry.type === 'assistant') break; + if (entry.type === 'user' || entry.type === 'assistant') { hasTurn = true; break; } } - return { forkedFrom, slug, parentSessionId, hasSnapshots, cleared }; + return { forkedFrom, slug, parentSessionId, hasSnapshots, cleared, hasTurn }; } catch { - return { forkedFrom: null, slug: null, parentSessionId: null, hasSnapshots: false, cleared: false }; + return { forkedFrom: null, slug: null, parentSessionId: null, hasSnapshots: false, cleared: false, hasTurn: false }; } } const CLEAR_OWNER_WAIT_MS = 60000; +const STALE_EMPTY_MS = 3600000; + +function ageMs(filePath) { + try { return Date.now() - fs.statSync(filePath).mtimeMs; } catch { return Infinity; } +} function isFresh(filePath) { - try { return Date.now() - fs.statSync(filePath).mtimeMs < CLEAR_OWNER_WAIT_MS; } catch { return false; } + return ageMs(filePath) < CLEAR_OWNER_WAIT_MS; } /** Detect fork and /clear transitions for active PTY sessions in a folder */ @@ -408,16 +416,10 @@ function detectSessionTransitions(folder) { // File exists but has no parseable content yet — skip and retry next cycle // But if the file's mtime is older than 1 hour, treat it as stale and archive it if (!signals.forkedFrom && !signals.parentSessionId && !signals.slug) { - // Fork file with only snapshots (no user turn yet) — match immediately if (signals.hasSnapshots && session.forkFrom && !session.realSessionId) { - log.info(`[detect] session=${sessionId} matching snapshot-only fork file=${newId}`); - // Fall through to matching logic — will match via the fork-snapshot path below + log.info(`[detect] session=${sessionId} snapshot-only file=${newId} while awaiting a fork`); } else { - let stale = false; - try { - const mtime = fs.statSync(path.join(folderPath, newFile)).mtimeMs; - if (Date.now() - mtime > 3600000) stale = true; - } catch {} + const stale = ageMs(newFilePath) > STALE_EMPTY_MS; if (stale) { log.info(`[detect] session=${sessionId} archiving stale empty file=${newId}`); } else { @@ -445,13 +447,15 @@ function detectSessionTransitions(folder) { if (!matched && session.forkFrom && signals.parentSessionId === session.forkFrom && newId !== session.forkFrom) { matched = true; } - // Fork file with only snapshots — no user turn yet, but this session is waiting for a fork. - // A /clear transcript also starts with a snapshot; it is never a fork. - if (!matched && signals.hasSnapshots && !signals.cleared && session.forkFrom && !session.realSessionId) { - matched = true; - } - const awaitingFork = Boolean(session.forkFrom && !session.realSessionId); + let pendingOwner = false; + // see .ai/contexts/cli-session-state.md ("Owner of a /clear transcript") + if (!matched && awaitingFork && signals.hasSnapshots && !signals.hasTurn) { + const owner = clearOwner(newId, session.pty && session.pty.pid); + if (owner === 'mine') matched = true; + else pendingOwner = owner === 'pending'; + log.info(`[detect] session=${sessionId} snapshot-only fork file=${newId} owner=${owner} matched=${matched}`); + } if (!matched && signals.cleared && !awaitingFork && newId !== (session.realSessionId || sessionId)) { const owner = clearOwner(newId, session.pty && session.pty.pid); if (owner === 'mine') { @@ -463,6 +467,11 @@ function detectSessionTransitions(folder) { log.info(`[detect] session=${sessionId} /clear file=${newId} owner=${owner} matched=${matched}`); } + if (!matched && !signals.hasTurn && newId !== (session.realSessionId || sessionId) && ageMs(newFilePath) <= STALE_EMPTY_MS) { + emptyFiles.add(newFile); + if (isFresh(newFilePath) && (pendingOwner || !awaitingFork)) recheck = true; + } + if (session.forkFrom && !matched) { log.info(`[detect] session=${sessionId} NO MATCH for newFile=${newId} forkFrom=${session.forkFrom} parentSessionId=${signals.parentSessionId||'null'} forkedFrom=${signals.forkedFrom||'null'}`); } @@ -478,7 +487,7 @@ function detectSessionTransitions(folder) { session._subFileList = null; activeSessions.delete(sessionId); activeSessions.set(newId, session); - rekeyed.set(sessionId, newId); + recordRekey(sessionId, newId); // Re-key MCP server to match new session ID rekeyMcpServer(sessionId, newId); rekeyActivity(sessionId, newId); diff --git a/test/clear-pending-persist.test.js b/test/clear-pending-persist.test.js index 76dbed617..8d6c4dbdb 100644 --- a/test/clear-pending-persist.test.js +++ b/test/clear-pending-persist.test.js @@ -4,10 +4,14 @@ const test = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); const { setupSidebarDom } = require('./dom-setup'); const { loadAppFunctions } = require('./app-source'); +const PLAN_SRC = fs.readFileSync(path.join(__dirname, '..', 'public', 'restore-plan.js'), 'utf8'); const PROJECT = { folder: '-srv-app', projectPath: '/srv/app', sessions: [] }; function setup() { @@ -16,54 +20,122 @@ function setup() { const settings = { global: { openWorkingSet: [] } }; let persists = 0; let projects = [PROJECT]; + let archived = []; + const launched = []; + const shown = []; Object.assign(window, { loadingStatus: window.document.createElement('div'), + terminalHeaderId: window.document.createElement('span'), + terminalHeaderName: window.document.createElement('span'), reportActivityFocus: () => {}, reportActivityTitles: () => {}, pollActiveSessions: async () => {}, refreshSidebar: () => {}, renderDefaultStatus: () => {}, + rekeyActivityState: () => {}, + setActiveSession: (id) => { window.activeSessionId = id; }, encodeProjectPath: (p) => p.replace(/\//g, '-'), schedulePersistWorkingSet: () => { persists += 1; }, - pendingRestoreEntries: () => [], + liveElsewhereMany: async () => ({}), + showLiveElsewhereNotice: () => {}, + showSession: (id) => shown.push(id), + openSession: async () => true, + resolveDefaultSessionOptions: async () => ({ permissionMode: 'plan', worktree: true, worktreeName: 'w' }), + launchNewSession: async (project, options) => { + const id = `started-${launched.length + 1}`; + launched.push({ projectPath: project.projectPath, options }); + window.openSessions.set(id, { session: { sessionId: id, projectPath: project.projectPath }, closed: false }); + return id; + }, + RESTORE_STAGGER_MS: 0, }); window.api = { - getProjects: async () => JSON.parse(JSON.stringify(projects)), + getProjects: async (showArchived) => JSON.parse(JSON.stringify(showArchived ? [...projects, ...archived] : projects)), getActiveTerminals: async () => [], getSetting: async (key) => JSON.parse(JSON.stringify(settings[key] || null)), setSetting: async (key, value) => { settings[key] = JSON.parse(JSON.stringify(value)); }, }; window.cachedProjects = []; window.cachedAllProjects = []; + vm.runInContext(PLAN_SRC, ctx.context); const fns = loadAppFunctions(ctx.context, { - functions: ['dedup', 'loadProjects', 'persistWorkingSet'], - declarations: ['_persistChain', 'skippedWorkingSetEntries'], + functions: ['dedup', 'loadProjects', 'persistWorkingSet', 'pendingRestoreEntries', 'runRestore', 'applySessionForked'], + declarations: ['_persistChain', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restorePlanner'], }); - const open = (sessionId, { pending }) => { - const session = { sessionId, projectPath: '/srv/app', summary: 'New session' }; + const open = (sessionId, { pending, remoteAlias } = {}) => { + const session = { sessionId, projectPath: '/srv/app', summary: 'Earlier work', ...(remoteAlias ? { remoteAlias } : {}) }; window.openSessions.set(sessionId, { session, closed: false }); + window.sessionMap.set(sessionId, session); if (pending) window.pendingSessions.set(sessionId, { session, projectPath: '/srv/app', folder: '-srv-app' }); }; return { - ...fns, open, settings, + ...fns, open, settings, window, launched, shown, + run: (code) => vm.runInContext(code, ctx.context), persists: () => persists, setProjects: (list) => { projects = list; }, - saved: () => settings.global.openWorkingSet.map((e) => e.sessionId), + setArchived: (list) => { archived = list; }, + saved: () => settings.global.openWorkingSet, destroy: () => ctx.destroy(), }; } -test('a session with no transcript yet, after /clear or from +, is not saved: there is nothing to resume', async () => { +test('after /clear, the new id has no transcript yet: it is saved as a fresh session, not dropped', async () => { const h = setup(); try { - h.open('real', { pending: false }); + h.open('old-id'); + h.window.activeSessionId = 'old-id'; + h.applySessionForked('old-id', 'new-id'); + assert.ok(h.window.pendingSessions.has('new-id'), 'the cleared session is a pending row'); + await h.persistWorkingSet(); + assert.deepEqual(h.saved(), [{ sessionId: 'new-id', projectPath: '/srv/app', active: true, fresh: true }]); + } finally { h.destroy(); } +}); + +test('a fresh entry is restored by starting a new session in its project, without the index', async () => { + const h = setup(); + try { + h.open('real'); h.open('cleared', { pending: true }); + h.window.activeSessionId = 'cleared'; + await h.persistWorkingSet(); + const savedSet = h.saved(); + + h.window.openSessions.clear(); + h.window.sessionMap.clear(); + h.window.sessionMap.set('real', { sessionId: 'real', projectPath: '/srv/app' }); + h.run(`restorePlanner = createRestorePlanner({ savedSet: ${JSON.stringify(savedSet)} });`); + const plan = h.run('restorePlanner.tick({ sessionMap, openSessions, indexingDone: true })'); + assert.equal(plan.action, 'restore'); + assert.equal(plan.unavailable.length, 0, 'not reported as "not in the index"'); + assert.deepEqual([...plan.candidates].map(c => c.sessionId).sort(), ['cleared', 'real']); + + await h.runRestore(plan.candidates); + assert.deepEqual(h.launched, [{ projectPath: '/srv/app', options: { permissionMode: 'plan' } }], + 'started with the project defaults, in the same folder: no new worktree'); + assert.equal(h.shown.at(-1), 'started-1', 'the restored session that was active is shown'); + } finally { h.destroy(); } +}); + +test('a fresh entry waiting in the restore survives a persist', async () => { + const h = setup(); + try { + const savedSet = [{ sessionId: 'cleared', projectPath: '/srv/app', active: false, fresh: true }]; + h.run(`restoreSavedIndex = new Map([['cleared', 0]]); restorePlanner = createRestorePlanner({ savedSet: ${JSON.stringify(savedSet)} });`); await h.persistWorkingSet(); - assert.deepEqual(h.saved(), ['real']); + assert.deepEqual(h.saved(), savedSet); } finally { h.destroy(); } }); -test('its first prompt makes it real, and the set is saved again with it', async () => { +test('a pending session on a remote host is still left out', async () => { + const h = setup(); + try { + h.open('remote-new', { pending: true, remoteAlias: 'box' }); + await h.persistWorkingSet(); + assert.deepEqual(h.saved(), []); + } finally { h.destroy(); } +}); + +test('its first prompt makes it real, and the set is saved again without the fresh mark', async () => { const h = setup(); try { h.open('cleared', { pending: true }); @@ -73,6 +145,23 @@ test('its first prompt makes it real, and the set is saved again with it', async await h.loadProjects(); assert.equal(h.persists(), 1); await h.persistWorkingSet(); - assert.deepEqual(h.saved(), ['cleared']); + assert.deepEqual(h.saved(), [{ sessionId: 'cleared', projectPath: '/srv/app', active: false }]); + } finally { h.destroy(); } +}); + +test('the row added after /clear does not bring back an archived folder', async () => { + const h = setup(); + try { + h.setProjects([]); + h.setArchived([{ ...PROJECT, sessions: [{ sessionId: 'old-id', projectPath: '/srv/app' }] }]); + h.open('old-id'); + h.applySessionForked('old-id', 'new-id'); + await h.loadProjects(); + assert.ok(!h.window.cachedProjects.some(p => p.projectPath === '/srv/app'), 'hidden like the rest of the folder'); + assert.ok(h.window.cachedAllProjects.some(p => p.sessions.some(s => s.sessionId === 'new-id')), 'still listed with the archived folders'); + + h.window.pendingSessions.set('plus', { session: { sessionId: 'plus', projectPath: '/srv/app' }, projectPath: '/srv/app', folder: '-srv-app' }); + await h.loadProjects(); + assert.ok(h.window.cachedProjects.some(p => p.sessions.some(s => s.sessionId === 'plus')), 'a session started with + still shows, as before'); } finally { h.destroy(); } }); diff --git a/test/session-activity.test.js b/test/session-activity.test.js index d7ec1c266..2ca437ca8 100644 --- a/test/session-activity.test.js +++ b/test/session-activity.test.js @@ -458,7 +458,8 @@ test('public/app.js: the poll reconciles busy state and re-keys it on fork/detec assert.match(poll, /reconcileBusyState\(entries,\s*seq\)/, 'the poll must reconcile the busy state'); assert.match(poll, /entries\.map\(e => e\.sessionId\)/, 'activePtyIds must be rebuilt from the new payload shape'); - const forked = src.slice(src.indexOf('window.api.onSessionForked'), src.indexOf('window.api.onProcessExited')); + assert.match(src, /window\.api\.onSessionForked\(applySessionForked\)/); + const forked = src.slice(src.indexOf('function applySessionForked'), src.indexOf('window.api.onSessionForked')); assert.match(forked, /rekeyActivityState\(oldId, newId\)/, 'a fork must carry the activity state to the new id'); const detected = src.slice(src.indexOf('window.api.onSessionDetected'), src.indexOf('window.api.onSessionForked')); diff --git a/test/session-transitions.test.js b/test/session-transitions.test.js index 72887e55a..417ed9dfa 100644 --- a/test/session-transitions.test.js +++ b/test/session-transitions.test.js @@ -872,6 +872,175 @@ function writeClearTranscript(folderPath, id) { ].join('\n') + '\n', 'utf8'); } +function writeHeaderOnly(folderPath, id) { + fs.writeFileSync(path.join(folderPath, `${id}.jsonl`), [ + JSON.stringify({ sessionId: id, type: 'mode', mode: 'normal' }), + JSON.stringify({ type: 'file-history-snapshot', messageId: 'm1', snapshot: {} }), + JSON.stringify({ sessionId: id, type: 'user', isMeta: true, message: { role: 'user', content: 'Caveat: The messages below were generated by the user while running local commands.' } }), + ].join('\n') + '\n', 'utf8'); +} + +function writeSnapshotOnly(folderPath, id) { + fs.writeFileSync(path.join(folderPath, `${id}.jsonl`), + JSON.stringify({ type: 'file-history-snapshot', messageId: 'm1', snapshot: {} }) + '\n', 'utf8'); +} + +test('/clear: a transcript seen before its /clear record is written is followed once the record lands', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const timers = []; + const { events, activeSessions } = setupForkDetection(tmp, { + clearOwner: () => 'mine', + setTimeout: (fn) => { timers.push(fn); return { unref() {} }; }, + }); + activeSessions.set('old-id', makePtySession(folder, { pty: { pid: 7 } })); + + writeHeaderOnly(folderPath, 'new-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('old-id')); + assert.ok(!activeSessions.get('old-id').knownJsonlFiles.has('new-id.jsonl'), 'a header alone keeps the file eligible'); + assert.equal(timers.length, 1, 'and rechecked on its own while it is fresh'); + + writeClearTranscript(folderPath, 'new-id'); + timers.shift()(); + assert.ok(activeSessions.has('new-id') && !activeSessions.has('old-id')); + assert.equal(events.filter(e => e.channel === 'session-forked').length, 1); + } finally { + cleanup(tmp); + } +}); + +test('/clear: a header-only transcript older than the stale limit is recorded, and the session\'s own file never waits', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const timers = []; + const { activeSessions } = setupForkDetection(tmp, { + clearOwner: () => 'mine', + setTimeout: (fn) => { timers.push(fn); return { unref() {} }; }, + }); + activeSessions.set('own-id', makePtySession(folder, { pty: { pid: 7 } })); + + writeHeaderOnly(folderPath, 'own-id'); + writeHeaderOnly(folderPath, 'old-header'); + const t = (Date.now() - 2 * 3600_000) / 1000; + fs.utimesSync(path.join(folderPath, 'old-header.jsonl'), t, t); + sessionTransitions.detectSessionTransitions(folder); + const known = activeSessions.get('own-id').knownJsonlFiles; + assert.ok(known.has('own-id.jsonl') && known.has('old-header.jsonl')); + assert.equal(timers.length, 0); + } finally { + cleanup(tmp); + } +}); + +test('fork detection: a snapshot-only file is taken as the fork only when the CLI under this PTY owns it', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const owners = { stranger: 'other', mine: 'mine' }; + const { events, activeSessions } = setupForkDetection(tmp, { clearOwner: (id) => owners[id] }); + activeSessions.set('temp-id', makePtySession(folder, { pty: { pid: 9 }, forkFrom: 'source-id' })); + + writeSnapshotOnly(folderPath, 'stranger'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('temp-id'), 'another session\'s snapshot is not taken'); + assert.equal(events.filter(e => e.channel === 'session-forked').length, 0); + assert.ok(!activeSessions.get('temp-id').knownJsonlFiles.has('stranger.jsonl'), 'kept eligible until its first turn'); + + fs.appendFileSync(path.join(folderPath, 'stranger.jsonl'), + JSON.stringify({ sessionId: 'stranger', type: 'user', message: { role: 'user', content: 'hello' } }) + '\n'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('temp-id')); + assert.ok(activeSessions.get('temp-id').knownJsonlFiles.has('stranger.jsonl'), 'its first turn shows it is not the fork'); + + writeSnapshotOnly(folderPath, 'mine'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('mine') && !activeSessions.has('temp-id')); + } finally { + cleanup(tmp); + } +}); + +test('fork detection: a snapshot-only file whose owner is not published yet is rechecked, then matched', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const timers = []; + let owner = 'pending'; + const { activeSessions } = setupForkDetection(tmp, { + clearOwner: () => owner, + setTimeout: (fn) => { timers.push(fn); return { unref() {} }; }, + }); + activeSessions.set('temp-id', makePtySession(folder, { pty: { pid: 9 }, forkFrom: 'source-id' })); + + writeSnapshotOnly(folderPath, 'fork-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('temp-id')); + assert.equal(timers.length, 1); + owner = 'mine'; + timers.shift()(); + assert.ok(activeSessions.has('fork-id')); + } finally { + cleanup(tmp); + } +}); + +test('fork detection: a fork whose owner cannot be read is still matched by its first turn', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const timers = []; + const { activeSessions } = setupForkDetection(tmp, { + clearOwner: () => 'unknown', + setTimeout: (fn) => { timers.push(fn); return { unref() {} }; }, + }); + activeSessions.set('temp-id', makePtySession(folder, { pty: { pid: 9 }, forkFrom: 'source-id' })); + + writeSnapshotOnly(folderPath, 'fork-id'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('temp-id')); + assert.equal(timers.length, 0, 'nothing to wait for: the next change in the folder decides'); + fs.appendFileSync(path.join(folderPath, 'fork-id.jsonl'), + JSON.stringify({ sessionId: 'source-id', type: 'user', message: { role: 'user', content: 'hello' } }) + '\n'); + sessionTransitions.detectSessionTransitions(folder); + assert.ok(activeSessions.has('fork-id')); + } finally { + cleanup(tmp); + } +}); + +test('/clear: the original id resolves to the newest one after any number of clears', () => { + const tmp = mkTmp(); + try { + const folder = 'proj'; + const folderPath = path.join(tmp, folder); + fs.mkdirSync(folderPath, { recursive: true }); + const { activeSessions } = setupForkDetection(tmp, { clearOwner: () => 'mine' }); + activeSessions.set('id-0', makePtySession(folder, { pty: { pid: 7 } })); + for (let i = 1; i <= 40; i++) { + writeClearTranscript(folderPath, `id-${i}`); + sessionTransitions.detectSessionTransitions(folder); + } + assert.ok(activeSessions.has('id-40')); + assert.equal(sessionTransitions.currentSessionId('id-0'), 'id-40'); + assert.equal(sessionTransitions.currentSessionId('id-17'), 'id-40'); + } finally { + cleanup(tmp); + } +}); + test('/clear: the new transcript of the CLI under this PTY re-keys the session', () => { const tmp = mkTmp(); try { diff --git a/test/trigger-clear-chain.test.js b/test/trigger-clear-chain.test.js new file mode 100644 index 000000000..f68a0ce9a --- /dev/null +++ b/test/trigger-clear-chain.test.js @@ -0,0 +1,154 @@ +// see .ai/contexts/trigger-watcher.md, "Re-keyed sessions" +'use strict'; + +process.env.SWITCHBOARD_SUBMIT_ENTER_DELAY_MS = '1'; +process.env.SWITCHBOARD_SUBMIT_VERIFY_MS = '400'; +process.env.SWITCHBOARD_BUSY_FALL_SETTLE_MS = '50'; +process.env.SWITCHBOARD_BUSY_RISE_WAIT_MS = '100'; +process.env.SWITCHBOARD_TRANSCRIPT_QUIET_MS = '300'; +process.env.SWITCHBOARD_PENDING_OWN_ENTRY_MS = '1000'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { start } = require('../trigger-watcher'); +const { createTriggerContext } = require('../trigger-context'); +const sessionTransitions = require('../session-transitions'); + +function mkTmp(prefix) { + return fs.realpathSync.native(fs.mkdtempSync(path.join(os.tmpdir(), prefix))); +} + +const iso = (ms) => new Date(ms).toISOString(); +const jsonl = (entries) => entries.map((e) => JSON.stringify(e)).join('\n') + '\n'; +const prompt = (id, at, content) => ({ sessionId: id, type: 'user', timestamp: iso(at), message: { role: 'user', content } }); +const endTurn = (id, at) => ({ sessionId: id, type: 'assistant', timestamp: iso(at), message: { role: 'assistant', stop_reason: 'end_turn', content: [{ type: 'text', text: 'x' }] } }); +const system = (id, at, subtype) => ({ sessionId: id, type: 'system', subtype, timestamp: iso(at) }); +const noop = () => {}; +const log = { info: noop, warn: noop, error: noop, debug: noop }; + +const CLEAR_TEXT = '/clear\n clear\n '; + +// A CLI under one PTY: /clear moves it to a new transcript and a new id, as the real one does. +function clearingSession(onText = noop) { + const projectsDir = mkTmp('sw-clear-chain-projects-'); + const folderPath = path.join(projectsDir, 'proj'); + fs.mkdirSync(folderPath); + const old = Date.now() - 60_000; + const oldFile = path.join(folderPath, 'old-id.jsonl'); + fs.writeFileSync(oldFile, jsonl([prompt('old-id', old - 1000, 'earlier'), endTurn('old-id', old), system('old-id', old + 100, 'turn_duration')])); + fs.utimesSync(oldFile, new Date(old + 200), new Date(old + 200)); + + let current = 'old-id'; + const status = { 'old-id': { status: 'idle', statusUpdatedAt: old + 300 } }; + const written = []; + let lastText = null; + const pty = { + pid: 4242, + write(data) { + written.push(data); + if (data !== '\r') { lastText = data; onText(data); return; } + if (!fs.existsSync(folderPath)) return; + const at = Date.now(); + if (lastText === '/clear') { + setTimeout(() => { + if (!fs.existsSync(folderPath)) return; + current = 'new-id'; + fs.writeFileSync(path.join(folderPath, 'new-id.jsonl'), jsonl([ + { sessionId: 'new-id', type: 'mode', mode: 'normal' }, + { type: 'file-history-snapshot', messageId: 'm1', snapshot: {} }, + { sessionId: 'new-id', type: 'user', isMeta: true, timestamp: iso(at), message: { role: 'user', content: 'Caveat: synthetic.' } }, + prompt('new-id', at, CLEAR_TEXT), + ])); + status['new-id'] = { status: 'idle', statusUpdatedAt: Date.now() }; + sessionTransitions.detectSessionTransitions('proj'); + }, 30); + } else { + const id = current; + status[id] = { status: 'busy', statusUpdatedAt: Date.now() }; + fs.appendFileSync(path.join(folderPath, id + '.jsonl'), jsonl([prompt(id, at, lastText)])); + setTimeout(() => { + if (!fs.existsSync(folderPath)) return; + fs.appendFileSync(path.join(folderPath, id + '.jsonl'), jsonl([endTurn(id, Date.now()), system(id, Date.now(), 'turn_duration')])); + status[id] = { status: 'idle', statusUpdatedAt: Date.now() }; + }, 100); + } + }, + }; + const session = { + pty, cwd: '/w', projectFolder: 'proj', exited: false, isPlainTerminal: false, + knownJsonlFiles: new Set(['old-id.jsonl']), forkFrom: null, realSessionId: null, + _cliBusy: false, composerState: { pending: 0, lastInputAt: 0 }, + }; + const activeSessions = new Map([['old-id', session]]); + sessionTransitions.init({ + PROJECTS_DIR: projectsDir, activeSessions, getMainWindow: () => null, log, + rekeyMcpServer: noop, clearOwner: (id, pid) => (pid === 4242 ? 'mine' : 'other'), + }); + const ctx = createTriggerContext({ + activeSessions, log, projectsDir, + isPtyAlive: () => true, + getCliStatus: (id) => status[id] && { ...status[id] }, + resolveSessionId: (id) => sessionTransitions.currentSessionId(id), + }); + return { ctx, written, activeSessions, cleanup: () => fs.rmSync(projectsDir, { recursive: true, force: true }) }; +} + +async function runTriggers(ctx, triggers, timeoutMs, tmp = mkTmp('sw-clear-chain-triggers-')) { + process.env.SWITCHBOARD_TRIGGERS_DIR = tmp; + const watcher = start(ctx); + try { + for (const [uuid, trigger] of triggers) { + fs.writeFileSync(path.join(tmp, uuid + '.json'), JSON.stringify({ wait: 'none', timeout_ms: timeoutMs, ...trigger }), 'utf8'); + } + const results = {}; + const deadline = Date.now() + timeoutMs + 5000; + for (const [uuid] of triggers) { + const resultPath = path.join(tmp, 'processed', uuid + '.result.json'); + while (!fs.existsSync(resultPath)) { + if (Date.now() > deadline) throw new Error('no result file for ' + uuid); + await new Promise((r) => setTimeout(r, 20)); + } + await new Promise((r) => setTimeout(r, 20)); + results[uuid] = JSON.parse(fs.readFileSync(resultPath, 'utf8')); + } + return results; + } finally { + watcher.close(); + delete process.env.SWITCHBOARD_TRIGGERS_DIR; + fs.rmSync(tmp, { recursive: true, force: true }); + } +} + +test('a [/clear, reply] chain follows the session to its new id and writes the reply', async () => { + const s = clearingSession(); + try { + const { chain } = await runTriggers(s.ctx, [['chain', { sessionId: 'old-id', chain: [{ command: '/clear' }, { command: 'reply' }] }]], 6000); + assert.ok(s.activeSessions.has('new-id'), 'the session was re-keyed by the /clear'); + assert.equal(chain.ok, true, JSON.stringify(chain)); + assert.deepEqual(s.written.filter((d) => d !== '\r'), ['/clear', 'reply']); + } finally { + s.cleanup(); + } +}); + +test('a trigger for the new id waits for a chain still running under the old id', async () => { + const tmp = mkTmp('sw-clear-chain-triggers-'); + const s = clearingSession((text) => { + if (text !== 'first') return; + fs.writeFileSync(path.join(tmp, 'single.json'), JSON.stringify({ sessionId: 'new-id', command: 'other', wait: 'none', timeout_ms: 6000 }), 'utf8'); + }); + try { + const { chain } = await runTriggers(s.ctx, [['chain', { sessionId: 'old-id', chain: [{ command: '/clear' }, { command: 'first' }, { command: 'second' }] }]], 6000, tmp); + assert.equal(chain.ok, true, JSON.stringify(chain)); + const resultPath = path.join(tmp, 'processed', 'single.result.json'); + const deadline = Date.now() + 6000; + while (!fs.existsSync(resultPath) && Date.now() < deadline) await new Promise((r) => setTimeout(r, 20)); + assert.deepEqual(s.written.filter((d) => d !== '\r'), ['/clear', 'first', 'second', 'other'], 'one terminal, one queue, whichever id names it'); + } finally { + s.cleanup(); + } +}); diff --git a/test/trigger-context.test.js b/test/trigger-context.test.js index 9f56cc78f..f07bc44d8 100644 --- a/test/trigger-context.test.js +++ b/test/trigger-context.test.js @@ -172,3 +172,18 @@ test('getCliStatus is only present when supplied, and answers for local live ses assert.equal(ctx.getCliStatus('unknown'), undefined); assert.deepEqual(seen, ['local']); }); + +test('a re-keyed id is resolved before every lookup, the live descriptor included', () => { + const pty = { pid: 1, write() {} }; + const asked = []; + const ctx = createTriggerContext({ + activeSessions: new Map([['new-id', { pty, host: null, _cliBusy: true }]]), + log: silentLog, + getLiveDescriptor: (id) => { asked.push(id); return { id }; }, + resolveSessionId: (id) => (id === 'old-id' ? 'new-id' : id), + }); + assert.equal(ctx.getPtyForSession('old-id').ptyProcess, pty); + assert.equal(ctx.isSessionBusy('old-id'), true); + assert.deepEqual(ctx.getLiveDescriptor('old-id'), { id: 'new-id' }); + assert.deepEqual(asked, ['new-id']); +}); diff --git a/test/trigger-watcher.test.js b/test/trigger-watcher.test.js index 624a822f7..8d98e732f 100644 --- a/test/trigger-watcher.test.js +++ b/test/trigger-watcher.test.js @@ -5398,11 +5398,11 @@ test('startup scan respects MAX_INFLIGHT: only 8 of 12 pre-existing triggers sta const startedSessions = new Set(); // sessions whose isSessionBusy has been polled at least once const written = []; const composer = { pending: 0, lastInputAt: 0 }; - const ptyProcess = { pid: process.pid, write(data) { written.push(data); } }; + const ptys = new Map(sessionIds.map((sid) => [sid, { pid: process.pid, write(data) { written.push(data); } }])); const ctx = { log: silentLog, - getPtyForSession() { return { ptyProcess }; }, + getPtyForSession(sessionId) { return { ptyProcess: ptys.get(sessionId) }; }, isSessionBusy(sessionId) { startedSessions.add(sessionId); return busy.get(sessionId) === true; diff --git a/trigger-watcher.js b/trigger-watcher.js index 0457b5bf9..86d7720b3 100644 --- a/trigger-watcher.js +++ b/trigger-watcher.js @@ -1922,16 +1922,18 @@ function start(ctx) { const retained = new Set(); // see .ai/contexts/trigger-watcher.md, "Session serialization" - const sessionLocks = new Map(); // sessionId -> tail Promise of the queue + const sessionLocks = new Map(); // PTY (or sessionId) -> tail Promise of the queue function acquireSessionLock(sessionId) { - const previous = sessionLocks.get(sessionId) || Promise.resolve(); + const entry = ctx.getPtyForSession(sessionId); + const key = (entry && entry.ptyProcess) || sessionId; + const previous = sessionLocks.get(key) || Promise.resolve(); let release; const held = new Promise((resolve) => { release = resolve; }); const tail = previous.then(() => held); - sessionLocks.set(sessionId, tail); + sessionLocks.set(key, tail); return previous.then(() => () => { release(); - if (sessionLocks.get(sessionId) === tail) sessionLocks.delete(sessionId); + if (sessionLocks.get(key) === tail) sessionLocks.delete(key); }); } From 81cf86bf35cefe1e61daa13fb79be152e0c9c1e5 Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 11:00:03 +0200 Subject: [PATCH 11/17] test(sandbox): a repository below a symlinked Additional Directory is protected find does not descend into a starting point that is a symbolic link, so a repository or a .claude below an Additional Directory given as a link is never found, and its .git/config, hooks and settings stay writable. --- test/sandbox-wrapper.test.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/sandbox-wrapper.test.js b/test/sandbox-wrapper.test.js index 46b794e0c..d67ebc8d2 100644 --- a/test/sandbox-wrapper.test.js +++ b/test/sandbox-wrapper.test.js @@ -1072,6 +1072,29 @@ test('sandbox wrapper: every .claude and repository below a bound directory is p } }); +test('sandbox wrapper: a repository and a .claude below an Additional Directory that is a link are protected', + { skip: !LINUX && 'linux only' }, () => { + const rig = makeRig({ recordArgs: true }); + try { + fs.mkdirSync(path.join(rig.home, '.claude')); + const other = path.join(rig.root, 'other'); + const sub = path.join(other, 'sub'); + fs.mkdirSync(path.join(sub, '.claude'), { recursive: true }); + git(sub, 'init', '-q'); + const link = path.join(rig.root, 'link'); + fs.symlinkSync(other, link); + const { status, stderr } = rig.run(['--version'], { SWITCHBOARD_SANDBOX_BINDS: link }); + assert.equal(status, 0, stderr); + const ops = parseMounts(rig.lastBwrapArgs()); + assert.equal(accessAt(ops, path.join(link, 'sub', 'file')), '--bind'); + for (const p of [path.join(link, 'sub', '.git', 'config'), path.join(link, 'sub', '.claude', 'settings.json')]) { + assert.equal(accessAt(ops, p), '--ro-bind', `${p} must stay read-only`); + } + } finally { + rig.cleanup(); + } + }); + test('sandbox wrapper: refuses a repository git cannot read, rather than guessing which paths to protect', { skip: !LINUX && 'linux only' }, () => { const rig = makeRig({ recordArgs: true }); From 4fc768f2f8313e891e0822db6a95c8a73d71ad80 Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 11:00:40 +0200 Subject: [PATCH 12/17] fix(sandbox): enter an Additional Directory that is a symbolic link when protecting below it find does not descend into a starting point that is a link; a trailing slash makes it enter the directory the link points to. --- CHANGELOG.md | 3 +++ scripts/claude-sandbox.sh | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8485b6c99..75367011b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased +### Fixed +- In a sandboxed session, a repository or a `.claude` folder below an Additional Directory that is a symbolic link now has its Git config, hooks and Claude settings read-only, like one below a plain directory. Before, they were writable from inside the sandbox. (#538) + ## v0.0.92 — 2026-10-10 ### Fixed diff --git a/scripts/claude-sandbox.sh b/scripts/claude-sandbox.sh index a0848ee06..e44600222 100755 --- a/scripts/claude-sandbox.sh +++ b/scripts/claude-sandbox.sh @@ -612,7 +612,7 @@ bind_project_dir() { .claude) protect_claude_dir "$n" ;; .git) protect_repo_root "$(dirname "$n")" ;; esac - done < <(find "$d" -xdev -mindepth 2 \( -name node_modules -prune \) -o \ + done < <(find "$d/" -xdev -mindepth 2 \( -name node_modules -prune \) -o \ \( -name .git -print0 -prune \) -o \( -name .claude -type d -print0 \) -o \ \( -name .claude -type l -print0 \) 2>/dev/null) } From 9f2cf00bc22271b9377aed13d17761f7340a4ab8 Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 11:00:41 +0200 Subject: [PATCH 13/17] test(sessions): a double click on a session row opens its terminal once Regression test alone, expected to fail: both clicks pass the openSessions check before the first await, so open-terminal is requested twice. --- test/open-session-once.test.js | 89 ++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 test/open-session-once.test.js diff --git a/test/open-session-once.test.js b/test/open-session-once.test.js new file mode 100644 index 000000000..7dc64ec43 --- /dev/null +++ b/test/open-session-once.test.js @@ -0,0 +1,89 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { setupSidebarDom } = require('./dom-setup'); +const { loadAppFunctions } = require('./app-source'); + +const PROJECT = '/home/dev/proj'; + +function setup() { + const ctx = setupSidebarDom(); + const { window } = ctx; + const openTerminalCalls = []; + const releases = []; + Object.assign(window, { + restoringWorkingSet: false, + sessionOpenedOutsideRestore: false, + skippedWorkingSetEntries: new Map(), + restoreSavedIndex: new Map(), + refreshSidebar: () => window.renderProjects(window.cachedProjects, true), + createTerminalEntry: (session) => { + const entry = { session, closed: false, terminal: { write() {}, focus() {} }, initialSize: {} }; + window.openSessions.set(session.sessionId, entry); + return entry; + }, + showSession: () => {}, + destroySession: (id) => { window.openSessions.delete(id); }, + guardResume: async () => true, + resolveResumeSession: async (session) => session, + resolveDefaultSessionOptions: async () => ({}), + syncPtySizeAfterOpen: () => {}, + setSessionMcpState: () => {}, + setSessionSandboxed: () => {}, + forgetSessionExit: () => {}, + beginPtyOpen: () => {}, + settlePtyOpen: () => {}, + schedulePersistWorkingSet: () => {}, + pollActiveSessions: () => {}, + }); + Object.assign(window.api, { + openTerminal: (sessionId) => { + openTerminalCalls.push(sessionId); + return new Promise((resolve) => releases.push(() => resolve({ ok: true }))); + }, + }); + const project = { + projectPath: PROJECT, + sessions: [{ sessionId: 's1', name: 's1', modified: new Date().toISOString(), messageCount: 1 }], + }; + window.cachedProjects = [project]; + window.cachedAllProjects = [project]; + window.sessionMap.set('s1', { sessionId: 's1', projectPath: PROJECT }); + loadAppFunctions(ctx.context, { + declarations: ['continuationRetryCancelled'], + functions: ['openSession'], + }); + window.renderProjects(window.cachedProjects, true); + const row = () => window.document.getElementById('si-s1'); + const release = () => releases.splice(0).forEach((r) => r()); + return { window, row, openTerminalCalls, release, destroy: () => ctx.destroy() }; +} + +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +test('a double click on a session row opens its terminal once', async () => { + const h = setup(); + try { + h.row().click(); + h.row().click(); + await settle(); + h.release(); + await settle(); + assert.deepEqual(h.openTerminalCalls, ['s1']); + } finally { h.destroy(); } +}); + +test('a click after the open has finished shows the session without opening it again', async () => { + const h = setup(); + try { + h.row().click(); + await settle(); + h.release(); + await settle(); + h.row().click(); + await settle(); + assert.deepEqual(h.openTerminalCalls, ['s1']); + } finally { h.destroy(); } +}); From 8da059846487a1da33c9eccce59a238c32b9e28d Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 11:11:59 +0200 Subject: [PATCH 14/17] fix(sessions): open a session once while its open is in flight openSession now keeps each session's open in flight (openingSessions, id -> promise) and returns the same promise to a duplicate call, so a double click requests one terminal and callers reading the result see the real outcome. The body moves to openSessionNow. --- .ai/contexts/session-state.md | 13 ++++++++++ CHANGELOG.md | 3 +++ public/app.js | 13 +++++++++- test/continuations-harness.js | 4 +-- test/open-session-once.test.js | 38 +++++++++++++++++++++++++---- test/open-session-terminal.test.js | 4 +-- test/open-terminal-attach.test.js | 2 +- test/process-exit-status.test.js | 4 ++- test/restore-live-elsewhere.test.js | 4 +-- test/resume-guard.test.js | 2 +- test/schedule-live-resume.test.js | 4 +-- 11 files changed, 74 insertions(+), 17 deletions(-) diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index c235265aa..400a6fac1 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -66,6 +66,19 @@ the terminal header's stop button, and the grid card's stop button all funnel through it) — it now asks `resolveSessionStop` which IPC to call instead of always calling `stopSession`. +### Opening a session once + +`openSession` keeps the open of each session id in flight in `openingSessions` +(id → promise), set before its first `await` and removed when the open +settles. A second call for the same id while the first is still running (a +double click on a sidebar row, a repeated key, a restore reaching a session the +user just clicked) gets the same promise back instead of starting a second +`open-terminal`, so callers that read the result (`runRestore`) see the real +outcome. Without it, both calls passed the `openSessions.has` check before the +first `await`, and since `open-terminal` awaits the MCP server before it +records the PTY, the second request could start a second `claude --resume` on +the same transcript. The body is `openSessionNow`. + ### Reopening a plain terminal A session's `type` is what tells `open-terminal` which of its two branches to diff --git a/CHANGELOG.md b/CHANGELOG.md index 8485b6c99..971fb80ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased +### Fixed +- A double click on a session in the sidebar opens it once, instead of sometimes starting two terminals that resume the same conversation. (#539) + ## v0.0.92 — 2026-10-10 ### Fixed diff --git a/public/app.js b/public/app.js index eae573247..5de58dd43 100644 --- a/public/app.js +++ b/public/app.js @@ -1387,7 +1387,18 @@ async function showTerminalHeader(session) { // Terminal lifecycle (createTerminalEntry, destroySession, showSession, setupDragAndDrop) → terminal-manager.js -async function openSession(session, customOptions, { automatic = false, live, continuationResolved = false, allowBgAttach = false } = {}) { +const openingSessions = new Map(); + +// see .ai/contexts/session-state.md ("Opening a session once") +function openSession(session, ...args) { + const key = session.sessionId; + if (openingSessions.has(key)) return openingSessions.get(key); + const opening = openSessionNow(session, ...args).finally(() => openingSessions.delete(key)); + openingSessions.set(key, opening); + return opening; +} + +async function openSessionNow(session, customOptions, { automatic = false, live, continuationResolved = false, allowBgAttach = false } = {}) { if (!restoringWorkingSet) sessionOpenedOutsideRestore = true; if (!automatic) continuationRetryCancelled = true; if (!continuationResolved && customOptions?.type !== 'attach' && (!openSessions.has(session.sessionId) || openSessions.get(session.sessionId).closed)) { diff --git a/test/continuations-harness.js b/test/continuations-harness.js index 169dd61a7..8f7cac197 100644 --- a/test/continuations-harness.js +++ b/test/continuations-harness.js @@ -72,8 +72,8 @@ function setup(t, graph, { live = {}, answer = false, savedEntries, chunkBytes } vm.runInContext('rows.forEach(row => sessionMap.set(row.sessionId, row));', ctx); dom.window.showRestoreNotice = (_, text) => { dom.window.notice = text; }; const app = loadAppFunctions(ctx, { - declarations: ['skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restoreIndexingDone', 'continuationRetryCancelled'], - functions: ['runRestore', 'openSession', 'persistWorkingSet', 'pendingRestoreEntries', 'showLiveElsewhereNotice'], + declarations: ['openingSessions', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restoreIndexingDone', 'continuationRetryCancelled'], + functions: ['runRestore', 'openSession', 'openSessionNow', 'persistWorkingSet', 'pendingRestoreEntries', 'showLiveElsewhereNotice'], }); return { spawned, prompts, rows, root, db, folder, ctx, diff --git a/test/open-session-once.test.js b/test/open-session-once.test.js index 7dc64ec43..59e2b88bb 100644 --- a/test/open-session-once.test.js +++ b/test/open-session-once.test.js @@ -1,3 +1,5 @@ +// see .ai/contexts/session-state.md ("Opening a session once") + 'use strict'; const test = require('node:test'); @@ -41,7 +43,7 @@ function setup() { Object.assign(window.api, { openTerminal: (sessionId) => { openTerminalCalls.push(sessionId); - return new Promise((resolve) => releases.push(() => resolve({ ok: true }))); + return new Promise((resolve) => releases.push(resolve)); }, }); const project = { @@ -52,13 +54,13 @@ function setup() { window.cachedAllProjects = [project]; window.sessionMap.set('s1', { sessionId: 's1', projectPath: PROJECT }); loadAppFunctions(ctx.context, { - declarations: ['continuationRetryCancelled'], - functions: ['openSession'], + declarations: ['continuationRetryCancelled', 'openingSessions'], + functions: ['openSession', 'openSessionNow'], }); window.renderProjects(window.cachedProjects, true); const row = () => window.document.getElementById('si-s1'); - const release = () => releases.splice(0).forEach((r) => r()); - return { window, row, openTerminalCalls, release, destroy: () => ctx.destroy() }; + const release = (result = { ok: true }) => releases.splice(0).forEach((r) => r(result)); + return { window, row, openSession: (...args) => window.openSession(...args), openTerminalCalls, release, destroy: () => ctx.destroy() }; } const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); @@ -87,3 +89,29 @@ test('a click after the open has finished shows the session without opening it a assert.deepEqual(h.openTerminalCalls, ['s1']); } finally { h.destroy(); } }); + +test('a second open of the same session while the first runs gets the first one\'s result', async () => { + const h = setup(); + try { + h.window.guardResume = async () => false; + const first = h.openSession(h.window.sessionMap.get('s1')); + const second = h.openSession(h.window.sessionMap.get('s1')); + assert.equal(await first, false); + assert.equal(await second, false, 'a caller reading the result sees the refusal, not undefined'); + assert.deepEqual(h.openTerminalCalls, []); + } finally { h.destroy(); } +}); + +test('once an open has failed, the session can be opened again', async () => { + const h = setup(); + try { + h.window.guardResume = async () => false; + assert.equal(await h.openSession(h.window.sessionMap.get('s1')), false); + h.window.guardResume = async () => true; + const opening = h.openSession(h.window.sessionMap.get('s1')); + await settle(); + h.release(); + await opening; + assert.deepEqual(h.openTerminalCalls, ['s1']); + } finally { h.destroy(); } +}); diff --git a/test/open-session-terminal.test.js b/test/open-session-terminal.test.js index c701887ae..619c9a6d6 100644 --- a/test/open-session-terminal.test.js +++ b/test/open-session-terminal.test.js @@ -50,8 +50,8 @@ async function withHarness(setup, fn) { }); setup(window, calls); const { openSession } = loadAppFunctions(ctx.context, { - declarations: ['ptyGenerations', 'pendingOpens', 'continuationRetryCancelled'], - functions: ['beginPtyOpen', 'settlePtyOpen', 'openSession'], + declarations: ['ptyGenerations', 'pendingOpens', 'continuationRetryCancelled', 'openingSessions'], + functions: ['beginPtyOpen', 'settlePtyOpen', 'openSession', 'openSessionNow'], }); await fn({ openSession, window, calls }); } finally { diff --git a/test/open-terminal-attach.test.js b/test/open-terminal-attach.test.js index 1927990eb..70e87e158 100644 --- a/test/open-terminal-attach.test.js +++ b/test/open-terminal-attach.test.js @@ -20,7 +20,7 @@ test('open-terminal builds `claude attach ` for an attach session and never test('a reattach reports whether the live session is an attach, and the renderer keeps it on the tab', () => { assert.match(MAIN, /ok: true, reattached: true, attach: !!session\.isAttach,/); const APP = fs.readFileSync(path.join(__dirname, '..', 'public', 'app.js'), 'utf8'); - const start = APP.indexOf('async function openSession('); + const start = APP.indexOf('async function openSessionNow('); const body = APP.slice(start, APP.indexOf('\nasync function ', start + 10)); const openIdx = body.indexOf('await window.api.openTerminal(sessionId'); assert.ok(openIdx > 0); diff --git a/test/process-exit-status.test.js b/test/process-exit-status.test.js index e21827bfe..bae53cffc 100644 --- a/test/process-exit-status.test.js +++ b/test/process-exit-status.test.js @@ -102,7 +102,9 @@ function setup() { run(extractDeclaration(APP_SRC, 'continuationRetryCancelled'), 'app.js#continuationRetryCancelled'); run(fs.readFileSync(path.join(ROOT, 'public', 'process-exit.js'), 'utf8'), 'process-exit.js'); run(sliceBlock('function updateTerminalHeader() {'), 'app.js#updateTerminalHeader'); - run(sliceBlock('async function openSession(session'), 'app.js#openSession'); + run(extractDeclaration(APP_SRC, 'openingSessions'), 'app.js#openingSessions'); + run(sliceBlock('function openSession(session'), 'app.js#openSession'); + run(sliceBlock('async function openSessionNow(session'), 'app.js#openSessionNow'); run(sliceBlock('function applyProcessExit('), 'app.js#applyProcessExit'); run(sliceBlock('function handleProcessExited('), 'app.js#handleProcessExited'); run(sliceBlock('function beginPtyOpen('), 'app.js#beginPtyOpen'); diff --git a/test/restore-live-elsewhere.test.js b/test/restore-live-elsewhere.test.js index a94abf5e3..27d2a8f83 100644 --- a/test/restore-live-elsewhere.test.js +++ b/test/restore-live-elsewhere.test.js @@ -89,9 +89,9 @@ function setup({ savedSet, liveIds, batchFails = false }) { function schedulePersistWorkingSet() {} function pollActiveSessions() {} `, ctx); - vm.runInContext(topLevelDeclarations(APP_SRC, ['LIVE_ELSEWHERE_NOTICE_MS', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restoreIndexingDone', 'continuationRetryCancelled']) + vm.runInContext(topLevelDeclarations(APP_SRC, ['LIVE_ELSEWHERE_NOTICE_MS', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restoreIndexingDone', 'continuationRetryCancelled', 'openingSessions']) .replace(/^const /gm, 'var ').replace(/^let /gm, 'var '), ctx); - for (const name of ['persistWorkingSet', 'pendingRestoreEntries', 'runRestore', 'openSession']) { + for (const name of ['persistWorkingSet', 'pendingRestoreEntries', 'runRestore', 'openSession', 'openSessionNow']) { vm.runInContext(functionSource(APP_SRC, name), ctx); } if (APP_SRC.includes('function showLiveElsewhereNotice(')) { diff --git a/test/resume-guard.test.js b/test/resume-guard.test.js index 074346a53..f1f898659 100644 --- a/test/resume-guard.test.js +++ b/test/resume-guard.test.js @@ -118,7 +118,7 @@ function functionBody(src, signature) { } test('openSession runs the guard before it asks main for a PTY', () => { - const body = functionBody(APP_SRC, 'async function openSession('); + const body = functionBody(APP_SRC, 'async function openSessionNow('); const guardAt = body.indexOf('guardResume('); const spawnAt = body.indexOf('window.api.openTerminal('); assert.notEqual(guardAt, -1, 'openSession must call guardResume'); diff --git a/test/schedule-live-resume.test.js b/test/schedule-live-resume.test.js index 505cbcffb..19d7f7d7d 100644 --- a/test/schedule-live-resume.test.js +++ b/test/schedule-live-resume.test.js @@ -101,8 +101,8 @@ function setup(t, { sandbox = false } = {}) { function pollActiveSessions() {} `, ctx); const app = loadAppFunctions(ctx, { - functions: ['openSession', 'runRestore', 'pendingRestoreEntries', 'persistWorkingSet', 'showRestoreNotice', 'showLiveElsewhereNotice'], - declarations: ['LIVE_ELSEWHERE_NOTICE_MS', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restoreIndexingDone', 'continuationRetryCancelled'], + functions: ['openSession', 'openSessionNow', 'runRestore', 'pendingRestoreEntries', 'persistWorkingSet', 'showRestoreNotice', 'showLiveElsewhereNotice'], + declarations: ['LIVE_ELSEWHERE_NOTICE_MS', 'openingSessions', 'skippedWorkingSetEntries', 'restoreSavedIndex', 'restoreAwaitingConsent', 'restoreInFlight', 'restoreIndexingDone', 'continuationRetryCancelled'], }); ctx.sessionMap.set(SID, session); t.after(() => { From 5680feef4921845eb42d133dc734fa114fa27b94 Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 18:33:06 +0200 Subject: [PATCH 15/17] test(sandbox): write below a symlinked Additional Directory from inside a real sandbox Also name in the changelog the refusal a linked Additional Directory now shares with a plain one. --- CHANGELOG.md | 2 +- test/sandbox-wrapper.test.js | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75367011b..bf4194cd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc ## Unreleased ### Fixed -- In a sandboxed session, a repository or a `.claude` folder below an Additional Directory that is a symbolic link now has its Git config, hooks and Claude settings read-only, like one below a plain directory. Before, they were writable from inside the sandbox. (#538) +- In a sandboxed session, a repository or a `.claude` folder below an Additional Directory that is a symbolic link now has its Git config, hooks and Claude settings read-only, like one below a plain directory. Before, they were writable from inside the sandbox. Such a directory holding a `.claude` that is itself a symbolic link, or a repository Git cannot read, now refuses to start the session (status 125), as a plain directory already did, instead of starting it unprotected. (#538) ## v0.0.92 — 2026-10-10 diff --git a/test/sandbox-wrapper.test.js b/test/sandbox-wrapper.test.js index d67ebc8d2..c9532375b 100644 --- a/test/sandbox-wrapper.test.js +++ b/test/sandbox-wrapper.test.js @@ -1275,8 +1275,21 @@ test('sandbox wrapper: from inside a real sandbox, every persistence write is re fs.mkdirSync(path.join(C, 'shell-snapshots')); fs.writeFileSync(path.join(C, 'shell-snapshots', 'snapshot-bash-live.sh'), 'export A=1\n'); fs.mkdirSync(path.join(C, 'session-env')); + const other = path.join(rig.root, 'other'); + const sub = path.join(other, 'sub'); + fs.mkdirSync(path.join(sub, '.claude'), { recursive: true }); + fs.writeFileSync(path.join(sub, '.claude', 'settings.json'), '{}\n'); + git(sub, 'init', '-q'); + const linked = path.join(rig.root, 'linked'); + fs.symlinkSync(other, linked); + const linkedAttempts = String.raw` +L='${linked}/sub' +attempt linked-dir-git-config 'git -C "$L" config core.fsmonitor evil' +attempt linked-dir-settings 'echo evil > "$L/.claude/settings.json"' +attempt linked-dir-file 'echo ok > "$L/file"' +`; - const res = rig.run([PERSISTENCE_ATTEMPTS]); + const res = rig.run([PERSISTENCE_ATTEMPTS + linkedAttempts], { SWITCHBOARD_SANDBOX_BINDS: linked }); assert.equal(res.status, 0, res.stderr); const said = res.stdout; const read = (p) => fs.readFileSync(p, 'utf8'); @@ -1324,6 +1337,11 @@ test('sandbox wrapper: from inside a real sandbox, every persistence write is re assert.equal(read(path.join(rig.proj, 'src.js')), 'export {}\n', `a source file must be written\n${said}`); assert.equal(git(rig.proj, 'log', '--format=%s', '-1'), 'src', `a commit must land\n${said}`); assert.match(said, /resolv-conf: written/, 'the resolver configuration must be readable'); + assert.equal(spawnSync('git', ['config', '--get', 'core.fsmonitor'], { cwd: sub, env: cleanEnv() }).status, 1, + `a repository below a linked Additional Directory must keep its config\n${said}`); + assert.equal(read(path.join(sub, '.claude', 'settings.json')), '{}\n', + `a .claude below a linked Additional Directory must be read-only\n${said}`); + assert.equal(read(path.join(sub, 'file')), 'ok\n', `a file below a linked Additional Directory must be written\n${said}`); } finally { rig.cleanup(); } From c638a66f7bd9af981887edcc97389fd8e5c30191 Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 18:33:46 +0200 Subject: [PATCH 16/17] test(sessions): a click is not merged into an automatic open, nor two opens of one continuation An automatic open of a session live elsewhere refuses without asking; a click joining it must still ask. A click resolving to a continuation the restore is opening must not start a second terminal. Also pin that a rejected or failed open lets the session be opened again. --- test/open-session-once.test.js | 106 +++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 4 deletions(-) diff --git a/test/open-session-once.test.js b/test/open-session-once.test.js index 59e2b88bb..099bb24d9 100644 --- a/test/open-session-once.test.js +++ b/test/open-session-once.test.js @@ -43,24 +43,29 @@ function setup() { Object.assign(window.api, { openTerminal: (sessionId) => { openTerminalCalls.push(sessionId); - return new Promise((resolve) => releases.push(resolve)); + return new Promise((resolve, reject) => releases.push({ resolve, reject })); }, }); const project = { projectPath: PROJECT, - sessions: [{ sessionId: 's1', name: 's1', modified: new Date().toISOString(), messageCount: 1 }], + sessions: [ + { sessionId: 's1', name: 's1', modified: new Date().toISOString(), messageCount: 1 }, + { sessionId: 's2', name: 's2', modified: new Date().toISOString(), messageCount: 1 }, + ], }; window.cachedProjects = [project]; window.cachedAllProjects = [project]; window.sessionMap.set('s1', { sessionId: 's1', projectPath: PROJECT }); + window.sessionMap.set('s2', { sessionId: 's2', projectPath: PROJECT }); loadAppFunctions(ctx.context, { declarations: ['continuationRetryCancelled', 'openingSessions'], functions: ['openSession', 'openSessionNow'], }); window.renderProjects(window.cachedProjects, true); const row = () => window.document.getElementById('si-s1'); - const release = (result = { ok: true }) => releases.splice(0).forEach((r) => r(result)); - return { window, row, openSession: (...args) => window.openSession(...args), openTerminalCalls, release, destroy: () => ctx.destroy() }; + const release = (result = { ok: true }) => releases.splice(0).forEach((r) => r.resolve(result)); + const fail = (err) => releases.splice(0).forEach((r) => r.reject(err)); + return { window, row, openSession: (...args) => window.openSession(...args), openTerminalCalls, release, fail, destroy: () => ctx.destroy() }; } const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); @@ -115,3 +120,96 @@ test('once an open has failed, the session can be opened again', async () => { assert.deepEqual(h.openTerminalCalls, ['s1']); } finally { h.destroy(); } }); + +test('a click while an automatic open of a session live elsewhere is in flight asks, and opens it', async () => { + const h = setup(); + try { + h.window.guardResume = require('../public/resume-guard').guardResume; + const lookups = []; + h.window.api.getSessionLiveElsewhere = () => new Promise((resolve) => lookups.push(resolve)); + const confirms = []; + h.window.confirm = (msg) => { confirms.push(msg); return true; }; + const automatic = h.openSession(h.window.sessionMap.get('s1'), undefined, { automatic: true }); + h.row().click(); + assert.equal(h.window.eval('continuationRetryCancelled'), true, 'the click cancels held retries at once'); + await settle(); + lookups.splice(0).forEach((r) => r({ pid: 42 })); + assert.equal(await automatic, false, 'the automatic open still refuses'); + await settle(); + lookups.splice(0).forEach((r) => r({ pid: 42 })); + await settle(); + h.release(); + await settle(); + assert.equal(confirms.length, 1, 'the click asks'); + assert.deepEqual(h.openTerminalCalls, ['s1']); + } finally { h.destroy(); } +}); + +test('a double click on the row of a session live elsewhere asks once', async () => { + const h = setup(); + try { + h.window.guardResume = require('../public/resume-guard').guardResume; + h.window.api.getSessionLiveElsewhere = async () => ({ pid: 42 }); + let confirms = 0; + h.window.confirm = () => { confirms++; return false; }; + h.row().click(); + h.row().click(); + for (let i = 0; i < 5; i++) await settle(); + assert.equal(confirms, 1); + assert.deepEqual(h.openTerminalCalls, []); + } finally { h.destroy(); } +}); + +test('a click while an automatic open runs waits for it and starts no second terminal', async () => { + const h = setup(); + try { + const automatic = h.openSession(h.window.sessionMap.get('s1'), undefined, { automatic: true }); + h.row().click(); + for (let i = 0; i < 3; i++) { await settle(); h.release(); } + await automatic; + await settle(); + assert.deepEqual(h.openTerminalCalls, ['s1']); + } finally { h.destroy(); } +}); + +test('a click that resolves to a continuation already being opened opens it once', async () => { + for (const clickFirst of [false, true]) { + const h = setup(); + try { + const guards = []; + h.window.guardResume = () => new Promise((resolve) => guards.push(resolve)); + h.window.resolveResumeSession = async (session) => (session.sessionId === 's1' ? { ...session, sessionId: 's2' } : session); + const restore = () => h.openSession(h.window.sessionMap.get('s2'), undefined, { automatic: true, continuationResolved: true }); + const click = () => h.openSession(h.window.sessionMap.get('s1')); + const opens = clickFirst ? [click()] : [restore()]; + await settle(); + opens.push(clickFirst ? restore() : click()); + await settle(); + for (let i = 0; i < 3; i++) { guards.splice(0).forEach((r) => r(true)); await settle(); h.release(); await settle(); } + await Promise.all(opens); + assert.deepEqual(h.openTerminalCalls, ['s2'], clickFirst ? 'click, then the restore of its continuation' : 'restore, then a click resolving to it'); + } finally { h.destroy(); } + } +}); + +test('a session whose terminal failed to open can be opened again', async () => { + const h = setup(); + try { + const rejected = h.openSession(h.window.sessionMap.get('s1')); + await settle(); + h.fail(new Error('ipc gone')); + await assert.rejects(rejected); + await h.openSession(h.window.sessionMap.get('s1')); + + h.window.openSessions.clear(); + const failed = h.openSession(h.window.sessionMap.get('s1')); + await settle(); + h.release({ ok: false, error: 'spawn failed' }); + await failed; + const opened = h.openSession(h.window.sessionMap.get('s1')); + await settle(); + h.release(); + await opened; + assert.deepEqual(h.openTerminalCalls, ['s1', 's1', 's1']); + } finally { h.destroy(); } +}); From efbbe718d50610a4b3008a4e35f447fcb1891613 Mon Sep 17 00:00:00 2001 From: abate Date: Sat, 10 Oct 2026 18:35:58 +0200 Subject: [PATCH 17/17] fix(sessions): join only an equivalent open, and guard the resolved continuation id A click finding an automatic open of the same session in flight waits for it and then runs as a click, so a session live elsewhere still asks. An open whose continuation resolves to another id waits for any open of that id and registers under it, so a click and the restore cannot both start it. --- .ai/contexts/session-state.md | 23 +++++++++++++++++++---- public/app.js | 24 +++++++++++++++++------- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index 400a6fac1..e25b52461 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -69,16 +69,31 @@ always calling `stopSession`. ### Opening a session once `openSession` keeps the open of each session id in flight in `openingSessions` -(id → promise), set before its first `await` and removed when the open -settles. A second call for the same id while the first is still running (a -double click on a sidebar row, a repeated key, a restore reaching a session the -user just clicked) gets the same promise back instead of starting a second +(id → `{ promise, byUser }`), set before its first `await` and removed when the +open settles. A second call for the same id while the first is still running +(a double click on a sidebar row, a repeated key, a restore reaching a session +the user just clicked) gets the same promise back instead of starting a second `open-terminal`, so callers that read the result (`runRestore`) see the real outcome. Without it, both calls passed the `openSessions.has` check before the first `await`, and since `open-terminal` awaits the MCP server before it records the PTY, the second request could start a second `claude --resume` on the same transcript. The body is `openSessionNow`. +Only an equivalent open is joined. A user open (`automatic: false`) that finds +an automatic one in flight does not take its result: an automatic open of a +session live elsewhere refuses without asking, and the click must still ask. +It sets the click's flags (`continuationRetryCancelled`, +`sessionOpenedOutsideRestore`) at once, waits for the automatic open to +settle, then runs as usual, so it shows the terminal the automatic open made +or asks when that open refused. An automatic open joins any open in flight. + +The key is the id the caller passed, which for a click is the id before +continuation resolution, while the restore passes the resolved one. So once +`resolveResumeSession` returns another id, the open waits for any open of +that id in flight and registers itself under it too; a click resolving to a +continuation the restore is opening then shows that terminal instead of +starting a second one. + ### Reopening a plain terminal A session's `type` is what tells `open-terminal` which of its two branches to diff --git a/public/app.js b/public/app.js index 5de58dd43..00baa029d 100644 --- a/public/app.js +++ b/public/app.js @@ -1390,17 +1390,24 @@ async function showTerminalHeader(session) { const openingSessions = new Map(); // see .ai/contexts/session-state.md ("Opening a session once") -function openSession(session, ...args) { - const key = session.sessionId; - if (openingSessions.has(key)) return openingSessions.get(key); - const opening = openSessionNow(session, ...args).finally(() => openingSessions.delete(key)); - openingSessions.set(key, opening); - return opening; +function openSession(session, customOptions, flags = {}) { + const inFlight = openingSessions.get(session.sessionId); + const open = { byUser: !flags.automatic, after: null }; + if (inFlight) { + if (inFlight.byUser || !open.byUser) return inFlight.promise; + open.after = inFlight.promise.catch(() => {}); + } + open.promise = openSessionNow(session, customOptions, flags, open).finally(() => { + for (const [id, o] of openingSessions) if (o === open) openingSessions.delete(id); + }); + openingSessions.set(session.sessionId, open); + return open.promise; } -async function openSessionNow(session, customOptions, { automatic = false, live, continuationResolved = false, allowBgAttach = false } = {}) { +async function openSessionNow(session, customOptions, { automatic = false, live, continuationResolved = false, allowBgAttach = false } = {}, open = null) { if (!restoringWorkingSet) sessionOpenedOutsideRestore = true; if (!automatic) continuationRetryCancelled = true; + if (open?.after) await open.after; if (!continuationResolved && customOptions?.type !== 'attach' && (!openSessions.has(session.sessionId) || openSessions.get(session.sessionId).closed)) { const originalId = session.sessionId; session = await resolveResumeSession(session, { automatic, api: window.api, confirm: msg => window.confirm(msg), resolutions: window._startupResumeResolutions }); @@ -1416,6 +1423,9 @@ async function openSessionNow(session, customOptions, { automatic = false, live, skippedWorkingSetEntries.set(session.sessionId, { ...held, item: { ...held.item, sessionId: session.sessionId } }); } skippedWorkingSetEntries.delete(originalId); + const other = openingSessions.get(session.sessionId); + if (open && other && other !== open) await other.promise.catch(() => {}); + if (open) openingSessions.set(session.sessionId, open); } } const { sessionId, projectPath } = session;