(restore): a quit during restore neither duplicates a fresh session nor drops an attaching continuation (#550)

CHANGELOG.md

@@ -12,6 +12,7 @@ What changes for you in each release of Switchboard. How to write an entry: [doc
 
 ### Fixed
 - On Windows, opening a project with different letter case no longer makes its scheduled tasks run twice. Existing duplicate registrations are cleaned up automatically. (#607)
+- Quitting while startup is restoring sessions no longer opens an extra new session on the next start or forgets a conversation being reattached in the background. (#550)
 - On Windows, a `/compact` sent by an automation trigger is no longer reported as done because of a transcript file that was replaced while it waited. (#601)
 - A session sent to the background is reattached at every restart, not only the first. (#546)
 - Archiving a merged remote conversation now stops its live members even when the visible session is no longer running. Archiving a remote session whose host has not synced yet in this run (attached, or listed from the saved copy after a restart) now asks the host first, with the button busy, for up to 20 seconds. If the host does not answer, the session is still archived and a notice says it was not stopped and may still be running; archiving a folder or a group works the same way. A row whose host is disabled or removed archives as before. (#556)

docs/session-restore.md

@@ -69,7 +69,7 @@ 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. 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.
+opened it, and passes to the replacement of a fresh entry while that one opens. An entry still awaiting consent that you attach by hand is saved during the ask window. 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

public/app.js

@@ -204,17 +204,23 @@ function persistWorkingSet({ final = false } = {}) {
         ...(fresh ? { fresh: true } : {}),
       });
     }
-    const notStarted = pendingRestoreEntries().filter(({ item }) => !openSessions.has(item.sessionId));
+    const liveAttach = (id) => { const open = openSessions.get(id); return !!open && !open.closed && !!open.attach; };
+    const notStarted = pendingRestoreEntries()
+      .filter(({ item }) => !openSessions.has(item.sessionId) || liveAttach(item.sessionId))
+      .map(entry => ({ ...entry, keepAttached: liveAttach(entry.item.sessionId) }));
     const heldActive = notStarted.find(({ item }) => item.active);
     if (heldActive) for (const e of set) e.active = false;
-    const liveAttach = (id) => { const open = openSessions.get(id); return !!open && !open.closed && !!open.attach; };
     const held = [...skippedWorkingSetEntries.values(), ...notStarted]
       .filter(({ item, keepAttached }) => keepAttached ? liveAttach(item.sessionId) : !openSessions.has(item.sessionId))
       .sort((a, b) => a.index - b.index);
     for (const { item, index, keepAttached } of held) {
       const active = heldActive ? heldActive.item === item : !!keepAttached && item.sessionId === activeSessionId;
       set.splice(Math.min(index, set.length), 0, { sessionId: item.sessionId, projectPath: item.projectPath, active, ...(item.fresh ? { fresh: true } : {}), ...(item.bg || (keepAttached && liveAttach(item.sessionId)) ? { bg: true } : {}) });
     }
+    if (!set.some(e => e.active)) {
+      const opening = set.find(e => pendingSessions.get(e.sessionId)?.openingActive);
+      if (opening) opening.active = true;
+    }
     global.openWorkingSet = set;
     await window.api.setSetting('global', global);
   }).catch((e) => { console.warn('[switchboard] failed to persist working set', e); });
@@ -288,7 +294,7 @@ async function runRestore(list, { retryAfterIndexing = !restoreIndexingDone } =
           const options = await resolveDefaultSessionOptions(project);
           delete options.worktree;
           delete options.worktreeName;
-          const newId = await launchNewSession(project, options);
+          const newId = await launchNewSession(project, options, item.sessionId);
           if (newId) startedAs.set(item.sessionId, newId);
         } finally {
           restoreInFlight.delete(item.sessionId);
@@ -1359,7 +1365,7 @@ async function loadProjects({ resort = false } = {}) {
 // rebindSidebarEvents, buildSessionItem, startRename) → sidebar.js
 
 
-async function launchNewSession(project, sessionOptions) {
+async function launchNewSession(project, sessionOptions, restoreSessionId) {
   const sessionId = crypto.randomUUID();
   const projectPath = project.projectPath;
   const session = {
@@ -1393,9 +1399,18 @@ async function launchNewSession(project, sessionOptions) {
   refreshSidebar();
 
   const entry = createTerminalEntry(session);
+  if (restoreSessionId) {
+    if (restoreInFlight.get(restoreSessionId)?.active) pendingSessions.get(sessionId).openingActive = true;
+    restoreInFlight.delete(restoreSessionId);
+  }
 
   // Open terminal in main process with session options
-  const result = await window.api.openTerminal(sessionId, projectPath, true, sessionOptions || null, entry.initialSize);
+  let result;
+  try {
+    result = await window.api.openTerminal(sessionId, projectPath, true, sessionOptions || null, entry.initialSize);
+  } finally {
+    delete pendingSessions.get(sessionId)?.openingActive;
+  }
   if (!result.ok) {
     entry.terminal.write(`\r\nError: ${result.error}\r\n`);
     entry.closed = true;

test/cli-session-state.test.js

@@ -568,3 +568,45 @@ test('clearOwner: walks the state file pid up to the PTY pid', () => {
     fs.rmSync(dir, { recursive: true, force: true });
   }
 });
+
+test('clearOwner: uses the Windows process table to identify the PTY owner', async () => {
+  const dir = mkTmp();
+  try {
+    const created = n => String(134360000000000000n + BigInt(n) * 1000n);
+    let reads = 0;
+    cliSessionState.init({
+      dir, activeSessions: new Map(), log: silentLog, platform: 'win32',
+      isProcessAlive: pid => pid !== 500,
+      readProcessTable: async () => {
+        reads++;
+        return new Map([
+          [100, { ppid: 1, created: created(10) }],
+          [200, { ppid: 100, created: created(20) }],
+          [300, { ppid: 200, created: created(30) }],
+          [400, { ppid: 1, created: created(40) }],
+        ]);
+      },
+    });
+    writeState(dir, 300, { sessionId: 'cleared', status: 'idle', pidDomain: 'win32:anchor', procStart: created(30) });
+    writeState(dir, 400, { sessionId: 'elsewhere', status: 'idle', pidDomain: 'win32:anchor', procStart: created(40) });
+    writeState(dir, 500, { sessionId: 'dead', status: 'idle' });
+    writeState(dir, 600, { sessionId: 'no-proc', status: 'idle' });
+
+    assert.equal(cliSessionState.clearOwner('cleared', 100), 'pending', 'the table has not arrived yet');
+    await cliSessionState.refreshProcessTable();
+    assert.equal(reads, 1);
+    assert.equal(cliSessionState.clearOwner('cleared', 100), 'mine');
+    assert.equal(cliSessionState.clearOwner('CLEARED', 100), 'mine');
+    assert.equal(cliSessionState.clearOwner('cleared', 300), 'mine', 'the PTY runs the descriptor writer');
+    assert.equal(cliSessionState.clearOwner('elsewhere', 100), 'other');
+    assert.equal(cliSessionState.clearOwner('dead', 100), 'pending');
+    assert.equal(cliSessionState.clearOwner('absent', 100), 'pending');
+    assert.equal(cliSessionState.clearOwner('no-proc', 100), 'pending', 'the pid is absent from the table');
+    assert.equal(cliSessionState.clearOwner('cleared', undefined), 'pending');
+    writeState(dir, 300, { sessionId: 'cleared', status: 'idle', pidDomain: 'win32:anchor', procStart: created(100000) });
+    assert.equal(cliSessionState.clearOwner('cleared', 100), 'other', 'a reused pid is not the descriptor writer');
+  } finally {
+    cliSessionState.stop();
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});

test/restore-in-flight-persist.test.js

@@ -0,0 +1,171 @@
+'use strict';
+
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const vm = require('node:vm');
+const { setup } = require('./continuations-harness');
+const { loadAppFunctions } = require('./app-source');
+const { encodeProjectPath } = require('../encode-project-path');
+
+function setupExitRestore(t, graph, options) {
+  const h = setup(t, graph, options);
+  h.dom.window.encodeProjectPath = encodeProjectPath;
+  vm.runInContext(`
+    var cachedProjects = [], cachedAllProjects = [];
+    function refreshSidebar() {}
+  `, h.ctx, { filename: __filename });
+  Object.assign(h.app, loadAppFunctions(h.ctx, {
+    declarations: ['savedWorkingSetRead', 'persistWorkingSetTimer', 'exitingAppTimer', 'EXIT_FLUSH_GRACE_MS'],
+    functions: ['launchNewSession', 'flushStateForExit'],
+  }));
+  h.ctx.savedEntries = h.saved;
+  vm.runInContext(`
+    savedWorkingSetRead = true;
+    restoreSavedIndex = new Map(savedEntries.map((item, index) => [item.sessionId, index]));
+  `, h.ctx, { filename: __filename });
+  return h;
+}
+
+function holdTerminalOpen(h) {
+  let release;
+  const blocked = new Promise(resolve => { release = () => resolve({ ok: true }); });
+  let reached;
+  const opened = new Promise(resolve => { reached = resolve; });
+  const openTerminal = h.dom.window.api.openTerminal;
+  h.dom.window.api.openTerminal = async (...args) => {
+    await openTerminal(...args);
+    reached(args[0]);
+    return blocked;
+  };
+  return { opened, release };
+}
+
+test('quitting while a fresh restore opens saves only the replacement session', { timeout: 9000 }, async t => {
+  const h = setupExitRestore(t, {}, {
+    savedEntries: [{ sessionId: 'cleared', projectPath: '/project', active: true, fresh: true }],
+  });
+  const gate = holdTerminalOpen(h);
+  const restoring = h.app.runRestore(h.saved);
+  const newId = await gate.opened;
+  try {
+    await h.app.flushStateForExit();
+    assert.deepEqual(h.settings().openWorkingSet, [
+      { sessionId: newId, projectPath: '/project', active: true, fresh: true },
+    ]);
+  } finally {
+    gate.release();
+    await restoring;
+  }
+});
+
+test('quitting while the saved active fresh entry opens keeps the replacement active ahead of a later fresh entry', { timeout: 9000 }, async t => {
+  const h = setupExitRestore(t, {}, {
+    savedEntries: [
+      { sessionId: 'a', projectPath: '/project', active: true, fresh: true },
+      { sessionId: 'b', projectPath: '/project', active: false, fresh: true },
+    ],
+  });
+  const gate = holdTerminalOpen(h);
+  const restoring = h.app.runRestore(h.saved);
+  const newId = await gate.opened;
+  try {
+    await h.app.flushStateForExit();
+    assert.deepEqual(h.settings().openWorkingSet, [
+      { sessionId: newId, projectPath: '/project', active: true, fresh: true },
+      { sessionId: 'b', projectPath: '/project', active: false, fresh: true },
+    ]);
+  } finally {
+    gate.release();
+    await restoring;
+  }
+});
+
+test('quitting while one fresh restore opens keeps another fresh entry in the same project', { timeout: 9000 }, async t => {
+  const h = setupExitRestore(t, {}, {
+    savedEntries: [
+      { sessionId: 'first', projectPath: '/project', active: false, fresh: true },
+      { sessionId: 'second', projectPath: '/project', active: true, fresh: true },
+    ],
+  });
+  const gate = holdTerminalOpen(h);
+  const restoring = h.app.runRestore(h.saved);
+  const newId = await gate.opened;
+  try {
+    await h.app.flushStateForExit();
+    assert.deepEqual(h.settings().openWorkingSet, [
+      { sessionId: newId, projectPath: '/project', active: false, fresh: true },
+      { sessionId: 'second', projectPath: '/project', active: true, fresh: true },
+    ]);
+  } finally {
+    gate.release();
+    await restoring;
+  }
+});
+
+test('quitting while a continuation attaches keeps the resolved background session at its saved position', { timeout: 9000 }, async t => {
+  const h = setupExitRestore(t, { old: ['new'], new: [], later: [] }, {
+    live: { new: { kind: 'bg', jobId: '1234abcd', cwd: '/bg', pid: 1 } },
+    savedEntries: [
+      { sessionId: 'old', projectPath: '/project', active: true },
+      { sessionId: 'later', projectPath: '/project', active: false },
+    ],
+  });
+  const gate = holdTerminalOpen(h);
+  const restoring = h.app.runRestore(h.saved);
+  assert.equal(await gate.opened, 'new');
+  try {
+    await h.app.flushStateForExit();
+    assert.deepEqual(h.settings().openWorkingSet, [
+      { sessionId: 'new', projectPath: '/project', active: true, bg: true },
+      { sessionId: 'later', projectPath: '/project', active: false },
+    ]);
+  } finally {
+    gate.release();
+    await restoring;
+  }
+});
+
+test('quitting while a fresh active entry opens after an attached continuation keeps a single active entry', { timeout: 9000 }, async t => {
+  const h = setupExitRestore(t, { old: ['new'], new: [] }, {
+    live: { new: { kind: 'bg', jobId: '1234abcd', cwd: '/bg', pid: 1 } },
+    savedEntries: [
+      { sessionId: 'old', projectPath: '/project', active: false },
+      { sessionId: 'a', projectPath: '/project', active: true, fresh: true },
+    ],
+  });
+  const openTerminal = h.dom.window.api.openTerminal;
+  let release;
+  const blocked = new Promise(resolve => { release = () => resolve({ ok: true }); });
+  let reached;
+  const opened = new Promise(resolve => { reached = resolve; });
+  let calls = 0;
+  h.dom.window.api.openTerminal = async (...args) => {
+    await openTerminal(...args);
+    if (++calls < 2) return { ok: true };
+    reached(args[0]);
+    return blocked;
+  };
+  const restoring = h.app.runRestore(h.saved);
+  const newId = await opened;
+  try {
+    await h.app.flushStateForExit();
+    const set = h.settings().openWorkingSet;
+    assert.equal(set.filter(entry => entry.active).length, 1);
+    assert.deepEqual(set.find(entry => entry.sessionId === 'new'), { sessionId: 'new', projectPath: '/project', active: true, bg: true });
+    assert.equal(set.find(entry => entry.sessionId === newId).active, false);
+  } finally {
+    release();
+    await restoring;
+  }
+});
+
+test('a failed terminal open does not leave the active mark on the pending session', { timeout: 9000 }, async t => {
+  const h = setupExitRestore(t, {}, {
+    savedEntries: [{ sessionId: 'a', projectPath: '/project', active: true, fresh: true }],
+  });
+  let newId;
+  h.dom.window.api.openTerminal = async id => { newId = id; throw new Error('boom'); };
+  await assert.rejects(h.app.runRestore(h.saved), /boom/);
+  const pending = vm.runInContext('pendingSessions.get', h.ctx);
+  assert.equal(pending.call(vm.runInContext('pendingSessions', h.ctx), newId).openingActive, undefined);
+});

test/schedule-live-resume.test.js

@@ -78,7 +78,7 @@ function setup(t, { sandbox = false } = {}) {
   vm.runInContext(read('public/utils.js'), ctx);
   vm.runInContext(read('public/resume-guard.js'), ctx, { filename: path.join(ROOT, 'public/resume-guard.js') });
   vm.runInContext(`
-    var openSessions = new Map();
+    var openSessions = new Map(), pendingSessions = new Map();
     var sessionMap = new Map();
     var activeSessionId = null;
     var restoringWorkingSet = false;