fix(desktop): keep a remove from destroying a restored task

apps/desktop/src/main/__tests__/app-shell-session-purge.test.ts

@@ -33,8 +33,12 @@ function restored(id: string): SessionSummary {
 
 type SweepHarness = {
   removed: string[];
+  /** Each `remove` call as `[sessionId, requireArchived]`. */
+  removeOptions: Array<[string, boolean]>;
   cleared: string[];
   selections: Array<string | undefined>;
+  /** Titles of the success toasts a row action raised. */
+  toasts: string[];
   listCalls: number;
 };
 
@@ -50,6 +54,12 @@ function installWindow(
     surviving?: readonly SessionSummary[];
     /** Runs after each accepted removal, to model what another client did meanwhile. */
     onRemove?: (sessionId: string) => void;
+    /**
+     * The catalog the fake Host decides against. It checks the archived premise
+     * here, where the real one checks it inside its compare-and-set — not
+     * against whatever the renderer last saw.
+     */
+    catalog?: readonly SessionSummary[];
   } = {},
 ): () => void {
   const target = globalThis as unknown as { window?: unknown };
@@ -60,10 +70,14 @@ function installWindow(
     value: {
       maka: {
         sessions: {
-          remove: async (id: string) => {
+          remove: async (id: string, removeOptions?: { requireArchived?: boolean }) => {
+            harness.removeOptions.push([id, removeOptions?.requireArchived === true]);
             if (options.rejectIds?.includes(id)) throw new Error(`busy:${id}`);
+            const target = options.catalog?.find((session) => session.id === id);
+            if (removeOptions?.requireArchived && target && !target.isArchived) return 'restored';
             harness.removed.push(id);
             options.onRemove?.(id);
+            return 'removed';
           },
           list: async () => {
             harness.listCalls += 1;
@@ -101,12 +115,25 @@ function createActions(input: {
       input.activeIdRef.current = id;
     },
     setMessages: () => undefined,
-    toastApi: { success: () => undefined, error: () => undefined, confirm: async () => true },
+    toastApi: {
+      success: (title: string) => {
+        input.harness.toasts.push(title);
+      },
+      error: () => undefined,
+      confirm: async () => true,
+    },
   });
 }
 
 function harness(): SweepHarness {
-  return { removed: [], cleared: [], selections: [], listCalls: 0 };
+  return {
+    removed: [],
+    removeOptions: [],
+    cleared: [],
+    selections: [],
+    toasts: [],
+    listCalls: 0,
+  };
 }
 
 describe('purgeSessions', () => {
@@ -124,7 +151,18 @@ describe('purgeSessions', () => {
     const outcome = await actions.purgeSessions(['a-v2', 'b']).finally(restore);
 
     assert.deepEqual(h.removed, ['a-v2', 'b']);
-    assert.deepEqual(outcome, { removed: 2, remaining: [], verified: true, firstError: undefined });
+    assert.deepEqual(outcome, {
+      removed: 2,
+      remaining: [],
+      restored: [],
+      verified: true,
+      firstError: undefined,
+    });
+    // Every delete in a sweep carries the archived premise the confirm named.
+    assert.deepEqual(h.removeOptions, [
+      ['a-v2', true],
+      ['b', true],
+    ]);
     // The family goes, not just the representative, and the open member of it
     // stops being the active session.
     assert.deepEqual(h.cleared.sort(), ['a', 'a-v2', 'b']);
@@ -133,43 +171,101 @@ describe('purgeSessions', () => {
     assert.equal(h.listCalls, 0);
   });
 
-  it('leaves a task that stopped being archived before the sweep reached it', async () => {
+  it('reports a task restored before the sweep reached it, rather than dropping it', async () => {
     // The confirm named a set. One restored from another surface while the
     // dialog was up has left it, and a sweep that deleted it anyway would be
-    // acting outside what was agreed to.
+    // acting outside what was agreed to. Reporting it is the other half: the
+    // person agreed to two and one went, which needs saying.
     const h = harness();
-    const sessions = [restored('kept'), summary('doomed')];
-    const restore = installWindow(h);
-    const actions = createActions({ harness: h, sessions, activeIdRef: { current: undefined } });
+    const catalog = [restored('kept'), summary('doomed')];
+    const restore = installWindow(h, { catalog });
+    const actions = createActions({
+      harness: h,
+      sessions: [...catalog],
+      activeIdRef: { current: undefined },
+    });
 
     const outcome = await actions.purgeSessions(['kept', 'doomed']).finally(restore);
 
     assert.deepEqual(h.removed, ['doomed']);
     assert.equal(outcome.removed, 1);
+    assert.deepEqual(outcome.restored, ['kept']);
     assert.deepEqual(outcome.remaining, []);
+    // Kept tasks are settled by the delete itself; nothing to check back.
+    assert.equal(h.listCalls, 0);
   });
 
-  it('leaves a task restored while the sweep was already running', async () => {
+  it('reports a task restored while the sweep was already running', async () => {
     // The page disables its own controls during a sweep, so the restore comes
-    // from a second window. A set of archived ids snapshotted before the loop
-    // would not see it, and would delete a task that had left the set the
-    // confirm named — the catalog has to be read as each task is reached.
+    // from a second window, landing after the sweep started and before it
+    // reached this task.
     const h = harness();
-    const sessions = [summary('first'), summary('second')];
+    const catalog = [summary('first'), summary('second')];
     const restore = installWindow(h, {
+      catalog,
       onRemove: (id) => {
-        if (id === 'first') sessions[1] = restored('second');
+        if (id === 'first') catalog[1] = restored('second');
       },
     });
-    const actions = createActions({ harness: h, sessions, activeIdRef: { current: undefined } });
+    const actions = createActions({
+      harness: h,
+      sessions: [...catalog],
+      activeIdRef: { current: undefined },
+    });
 
     const outcome = await actions.purgeSessions(['first', 'second']).finally(restore);
 
     assert.deepEqual(h.removed, ['first']);
     assert.equal(outcome.removed, 1);
+    assert.deepEqual(outcome.restored, ['second']);
     assert.deepEqual(outcome.remaining, []);
   });
 
+  it('keeps everything the renderer holds for a task the delete left alone', async () => {
+    const h = harness();
+    const catalog = [summary('first'), restored('rescued')];
+    const restore = installWindow(h, { catalog });
+    const activeIdRef = { current: 'rescued' as string | undefined };
+    const actions = createActions({ harness: h, sessions: [...catalog], activeIdRef });
+
+    const outcome = await actions.purgeSessions(['first', 'rescued']).finally(restore);
+
+    assert.deepEqual(outcome.restored, ['rescued']);
+    assert.equal(outcome.firstError, undefined);
+    // A task that is still there keeps its renderer state, including being the
+    // open one.
+    assert.deepEqual(h.cleared, ['first']);
+    assert.deepEqual(h.selections, []);
+    assert.equal(activeIdRef.current, 'rescued');
+  });
+
+  it('sends every id to the delete instead of deciding against its own snapshot', async () => {
+    // The renderer's list is one observer of a state the Host owns, and a
+    // serial sweep gives a second window plenty of room to outdate it — in
+    // either direction. Here the snapshot is stale in the direction that
+    // silently spares a task: it reads `stale` as no longer archived while the
+    // catalog the delete commits against still has it archived. Filtering here
+    // would drop the id with no outcome at all, which is how a confirmed count
+    // stops adding up.
+    const h = harness();
+    const restore = installWindow(h, { catalog: [summary('stale'), summary('plain')] });
+    const actions = createActions({
+      harness: h,
+      sessions: [restored('stale'), summary('plain')],
+      activeIdRef: { current: undefined },
+    });
+
+    const outcome = await actions.purgeSessions(['stale', 'plain']).finally(restore);
+
+    assert.deepEqual(h.removeOptions, [
+      ['stale', true],
+      ['plain', true],
+    ]);
+    assert.deepEqual(h.removed, ['stale', 'plain']);
+    assert.equal(outcome.removed, 2);
+    assert.deepEqual(outcome.restored, []);
+  });
+
   it('skips an id whose row action is already in flight instead of racing it', async () => {
     const h = harness();
     const sessions = [summary('busy'), summary('free')];
@@ -228,3 +324,44 @@ describe('purgeSessions', () => {
     assert.equal(outcome.removed, 0);
   });
 });
+
+describe('deleteSession', () => {
+  it('carries the archived premise of the row the confirm named', async () => {
+    // Deleting from 已归档任务 is the same decision a sweep makes, one row at a
+    // time, so a restore revokes it the same way.
+    const h = harness();
+    const sessions = [summary('archived-row'), restored('active-row')];
+    const restore = installWindow(h);
+    const actions = createActions({ harness: h, sessions, activeIdRef: { current: undefined } });
+
+    await actions.deleteSession('archived-row');
+    await actions.deleteSession('active-row');
+    restore();
+
+    // An active task never had an archived premise to lose, so requiring one
+    // would refuse every delete from the rail.
+    assert.deepEqual(h.removeOptions, [
+      ['archived-row', true],
+      ['active-row', false],
+    ]);
+  });
+
+  it('keeps a task the Host reports as restored, and says so', async () => {
+    const h = harness();
+    // The row was archived when the confirm named it; the catalog the delete
+    // commits against says otherwise by the time it lands.
+    const sessions = [summary('rescued')];
+    const restore = installWindow(h, { catalog: [restored('rescued')] });
+    const activeIdRef = { current: 'rescued' as string | undefined };
+    const actions = createActions({ harness: h, sessions, activeIdRef });
+
+    await actions.deleteSession('rescued');
+    restore();
+
+    assert.deepEqual(h.removed, []);
+    assert.deepEqual(h.cleared, []);
+    assert.equal(activeIdRef.current, 'rescued');
+    // Not "Deleted rescued": nothing was.
+    assert.deepEqual(h.toasts, ['rescued was restored, so it was kept']);
+  });
+});

apps/desktop/src/main/__tests__/app-shell-session-row-actions-revisions.test.ts

@@ -34,7 +34,7 @@ function installWindow(calls: string[]): () => void {
           archive: async (id: string, options?: { revisionFamily?: boolean }) => { calls.push(`archive:${id}:${options?.revisionFamily === true}`); },
           unarchive: async (id: string, options?: { revisionFamily?: boolean }) => { calls.push(`unarchive:${id}:${options?.revisionFamily === true}`); },
           rename: async (id: string, name: string, options?: { revisionFamily?: boolean }) => { calls.push(`rename:${id}:${name}:${options?.revisionFamily === true}`); },
-          remove: async (id: string, options?: { revisionFamily?: boolean }) => { calls.push(`remove:${id}:${options?.revisionFamily === true}`); },
+          remove: async (id: string, options?: { revisionFamily?: boolean; requireArchived?: boolean }) => { calls.push(`remove:${id}:${options?.revisionFamily === true}:${options?.requireArchived === true}`); return 'removed' as const; },
         },
       },
     },
@@ -88,7 +88,9 @@ describe('revision-family session row actions', () => {
       'flag:version:true:true',
       'rename:branch:Independent branch:true',
       'archive:version:true',
-      'remove:root:true',
+      // `root` is not archived, so the delete states no archived premise —
+      // requiring one would refuse every delete from the rail.
+      'remove:root:true:false',
     ]);
     assert.deepEqual(selections, [undefined, undefined]);
     assert.deepEqual(cleared, ['root', 'version', 'root', 'version']);

apps/desktop/src/main/__tests__/runtime-host-client-operations.test.ts

@@ -240,6 +240,57 @@ test('retries a Session update through transient revision churn', async () => {
   assert.equal(updated.collaborationMode, 'plan');
 });
 
+test('abandons a remove whose task was restored under it', async () => {
+  // A lifecycle write bumps the revision, so the conflict IS the restore: the
+  // premise the caller decided on ("this task is archived") no longer holds,
+  // and replaying the delete at the fresh revision destroys a task somebody
+  // just pulled back out of the archive.
+  const { client, requests } = clientWithResponses([
+    { kind: 'session', session: session('session-1', 4, { isArchived: true }) },
+    { kind: 'revision_conflict', expectedRevision: 4, actualRevision: 5 },
+    { kind: 'session', session: session('session-1', 5, { isArchived: false }) },
+    // Only a replayed delete reaches this, and reaching it is the defect.
+    { kind: 'removed' },
+  ]);
+
+  assert.equal(await client.removeSession('session-1', { requireArchived: true }), 'restored');
+  assert.deepEqual(
+    requests.map(({ operation }) => operation),
+    ['session.catalog.query', 'session.remove', 'session.catalog.query'],
+  );
+});
+
+test('retries a remove through revision churn that left the task archived', async () => {
+  // Not every conflict is a restore. A task still archived at the fresh
+  // revision was only written around, and the delete still means what it did.
+  const { client, requests } = clientWithResponses([
+    { kind: 'session', session: session('session-1', 4, { isArchived: true }) },
+    { kind: 'revision_conflict', expectedRevision: 4, actualRevision: 5 },
+    { kind: 'session', session: session('session-1', 5, { isArchived: true }) },
+    { kind: 'removed' },
+  ]);
+
+  assert.equal(await client.removeSession('session-1', { requireArchived: true }), 'removed');
+  assert.deepEqual(
+    requests.filter(({ operation }) => operation === 'session.remove').map(({ input }) => input),
+    [
+      { sessionId: 'session-1', expectedRevision: 4 },
+      { sessionId: 'session-1', expectedRevision: 5 },
+    ],
+  );
+});
+
+test('removes a task that was never archived when no premise was stated', async () => {
+  // Deleting an active task from the rail has no archived premise to lose, so
+  // the precondition is the caller's to ask for, not the client's to assume.
+  const { client } = clientWithResponses([
+    { kind: 'session', session: session('session-1', 4) },
+    { kind: 'removed' },
+  ]);
+
+  assert.equal(await client.removeSession('session-1'), 'removed');
+});
+
 test('rebuilds a Runtime Policy mutation from each fresh CAS projection', async () => {
   const initial = createDefaultRuntimePolicy();
   const concurrent = {

apps/desktop/src/main/__tests__/runtime-host-client-uds.test.ts

@@ -103,6 +103,8 @@ test('drives the renderer Session catalog facade through real UDS framing', asyn
   const base = await mkdtemp(join(tmpdir(), 'maka-desktop-host-ipc-'));
   let host: RuntimeHostKernel | undefined;
   let projected: SessionCatalogProjection | undefined;
+  /** Arms one concurrent restore, landing between the Client's read and its remove. */
+  let restoreUnderNextRemove = false;
   try {
     const capability = await resolveStorageRoot({ path: base, kind: 'interactive' });
     const owner = await tryAcquireInteractiveRootOwner(capability);
@@ -180,6 +182,26 @@ test('drives the renderer Session catalog facade through real UDS framing', asyn
           },
           'session.remove': async (input) => {
             assert.ok(projected);
+            if (restoreUnderNextRemove) {
+              // Another window restored the task between the Client's read and
+              // this write. The Host rejects the stale revision, which is what
+              // a restore looks like from here.
+              restoreUnderNextRemove = false;
+              projected = session(projected.id, {
+                ...projected,
+                revision: projected.revision + 1,
+                isArchived: false,
+                status: 'active',
+              });
+              return {
+                ok: true,
+                result: {
+                  kind: 'revision_conflict',
+                  expectedRevision: input.expectedRevision,
+                  actualRevision: projected.revision,
+                },
+              };
+            }
             assert.equal(input.expectedRevision, projected.revision);
             const sessionId = projected.id;
             projected = undefined;
@@ -240,12 +262,24 @@ test('drives the renderer Session catalog facade through real UDS framing', asyn
     );
     await ipc.invoke('sessions:archive', 'session-ipc');
     assert.equal((await ipc.invoke('sessions:list') as Array<{ isArchived: boolean }>)[0]?.isArchived, true);
-    await ipc.invoke('sessions:remove', 'session-ipc');
+    // A purge sweep asks for the task it saw archived. Restored under it, the
+    // deletion is called off rather than replayed at the fresh revision (#3050).
+    restoreUnderNextRemove = true;
+    assert.equal(
+      await ipc.invoke('sessions:remove', 'session-ipc', { revisionFamily: true, requireArchived: true }),
+      'restored',
+    );
+    assert.equal((await ipc.invoke('sessions:list') as Array<{ isArchived: boolean }>)[0]?.isArchived, false);
+    await ipc.invoke('sessions:archive', 'session-ipc');
+    assert.equal(await ipc.invoke('sessions:remove', 'session-ipc'), 'removed');
     assert.deepEqual(await ipc.invoke('sessions:list'), []);
+    // Nothing was retired for the restored task: no `deleted` between the two
+    // archives, and the renderer keeps everything it holds for it.
     assert.deepEqual(changes, [
       { reason: 'created', sessionId: 'session-ipc' },
       { reason: 'mode-change', sessionId: 'session-ipc' },
       { reason: 'archived', sessionId: 'session-ipc' },
+      { reason: 'archived', sessionId: 'session-ipc' },
       { reason: 'deleted', sessionId: 'session-ipc' },
     ]);