diff --git a/apps/desktop/src/main/__tests__/app-shell-first-send-cleanup.test.ts b/apps/desktop/src/main/__tests__/app-shell-first-send-cleanup.test.ts index f963f3d3de..578e321bb0 100644 --- a/apps/desktop/src/main/__tests__/app-shell-first-send-cleanup.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-first-send-cleanup.test.ts @@ -204,6 +204,7 @@ describe('composer first-send cleanup', () => { send: async () => Promise.reject(new Error('Skill discovery failed')), remove: async (sessionId: string) => { removed.push(sessionId); + return { kind: 'removed' as const }; }, }, }); @@ -231,6 +232,7 @@ describe('composer first-send cleanup', () => { // and the rest of the happy path run after the cleanup window closes. remove: async (sessionId: string) => { removed.push(sessionId); + return { kind: 'removed' as const }; }, }, }); @@ -256,6 +258,7 @@ describe('composer first-send cleanup', () => { send: async () => Promise.reject(new Error('Skill discovery failed')), remove: async (sessionId: string) => { removed.push(sessionId); + return { kind: 'removed' as const }; }, }, }); @@ -342,7 +345,7 @@ describe('composer send failure feedback', () => { }, send: async () => Promise.reject(new Error('NO_REAL_CONNECTION:missing_api_key: no ready connection')), - remove: async () => undefined, + remove: async () => ({ kind: 'removed' as const }), }, }); diff --git a/apps/desktop/src/main/__tests__/app-shell-session-purge.test.ts b/apps/desktop/src/main/__tests__/app-shell-session-purge.test.ts index 2ed8dcfd37..373a06c8c8 100644 --- a/apps/desktop/src/main/__tests__/app-shell-session-purge.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-session-purge.test.ts @@ -37,6 +37,7 @@ function installWindow( harness: SweepHarness, options: { rejectIds?: readonly string[]; + restoreIds?: readonly string[]; surviving?: readonly SessionSummary[]; /** Runs after each accepted removal, to model what another client did meanwhile. */ onRemove?: (sessionId: string) => void; @@ -52,8 +53,10 @@ function installWindow( sessions: { remove: async (id: string) => { if (options.rejectIds?.includes(id)) throw new Error(`busy:${id}`); + if (options.restoreIds?.includes(id)) return { kind: 'restored' as const }; harness.removed.push(id); options.onRemove?.(id); + return { kind: 'removed' as const }; }, list: async () => { harness.listCalls += 1; @@ -114,7 +117,13 @@ 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, + restored: 0, + remaining: [], + verified: true, + firstError: undefined, + }); // 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']); @@ -136,6 +145,7 @@ describe('purgeSessions', () => { assert.deepEqual(h.removed, ['doomed']); assert.equal(outcome.removed, 1); + assert.equal(outcome.restored, 0); assert.deepEqual(outcome.remaining, []); }); @@ -157,7 +167,23 @@ describe('purgeSessions', () => { assert.deepEqual(h.removed, ['first']); assert.equal(outcome.removed, 1); + assert.equal(outcome.restored, 0); + assert.deepEqual(outcome.remaining, []); + }); + + it('counts a Host-side restore separately from a failed delete', async () => { + const h = harness(); + const sessions = [summary('kept'), summary('doomed')]; + const restore = installWindow(h, { restoreIds: ['kept'] }); + const actions = createActions({ harness: h, sessions, activeIdRef: { current: undefined } }); + + const outcome = await actions.purgeSessions(['kept', 'doomed']).finally(restore); + + assert.deepEqual(h.removed, ['doomed']); + assert.equal(outcome.removed, 1); + assert.equal(outcome.restored, 1); assert.deepEqual(outcome.remaining, []); + assert.equal(outcome.firstError, undefined); }); it('skips an id whose row action is already in flight instead of racing it', async () => { @@ -176,6 +202,7 @@ describe('purgeSessions', () => { assert.deepEqual(h.removed, ['free']); assert.deepEqual(outcome.remaining, ['busy']); assert.equal(outcome.removed, 1); + assert.equal(outcome.restored, 0); }); it('counts a rejected delete that the catalog no longer lists as removed', async () => { @@ -195,6 +222,7 @@ describe('purgeSessions', () => { assert.equal(h.listCalls, 1); assert.deepEqual(outcome.remaining, ['survivor']); assert.equal(outcome.removed, 1); + assert.equal(outcome.restored, 0); assert.equal((outcome.firstError as Error).message, 'busy:committed'); }); @@ -216,5 +244,6 @@ describe('purgeSessions', () => { assert.equal(outcome.verified, false); assert.deepEqual(outcome.remaining, []); assert.equal(outcome.removed, 0); + assert.equal(outcome.restored, 0); }); }); diff --git a/apps/desktop/src/main/__tests__/app-shell-session-row-actions-revisions.test.ts b/apps/desktop/src/main/__tests__/app-shell-session-row-actions-revisions.test.ts index 9e29039ede..b4919b5d2d 100644 --- a/apps/desktop/src/main/__tests__/app-shell-session-row-actions-revisions.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-session-row-actions-revisions.test.ts @@ -34,7 +34,10 @@ 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 }) => { + calls.push(`remove:${id}:${options?.revisionFamily === true}`); + return { kind: 'removed' as const }; + }, }, }, }, diff --git a/apps/desktop/src/main/__tests__/runtime-host-client-operations.test.ts b/apps/desktop/src/main/__tests__/runtime-host-client-operations.test.ts index 3a3705c0aa..fef5ff7dd1 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-client-operations.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-client-operations.test.ts @@ -133,6 +133,69 @@ test('settles branch cleanup when its target disappears between catalog reads', assert.equal(await client.removeSessionCopy('branch-copy'), 'removed'); }); +test('removes an archived Session on the first matching revision', async () => { + const { client, requests } = clientWithResponses([ + { kind: 'session', session: session('archived', 3, { isArchived: true }) }, + { kind: 'removed', sessionId: 'archived' }, + ]); + + assert.equal(await client.removeSession('archived'), 'removed'); + assert.deepEqual( + requests.filter(({ operation }) => operation === 'session.remove').map(({ input }) => input), + [{ sessionId: 'archived', expectedRevision: 3 }], + ); +}); + +test('retries an archived remove after a same-lifecycle revision conflict', async () => { + const { client, requests } = clientWithResponses([ + { kind: 'session', session: session('archived', 3, { isArchived: true }) }, + { kind: 'revision_conflict', expectedRevision: 3, actualRevision: 4 }, + { kind: 'session', session: session('archived', 4, { isArchived: true }) }, + { kind: 'removed', sessionId: 'archived' }, + ]); + + assert.equal(await client.removeSession('archived'), 'removed'); + assert.deepEqual( + requests.filter(({ operation }) => operation === 'session.remove').map(({ input }) => input), + [ + { sessionId: 'archived', expectedRevision: 3 }, + { sessionId: 'archived', expectedRevision: 4 }, + ], + ); +}); + +test('does not replay a remove after the Session is restored', async () => { + const { client, requests } = clientWithResponses([ + { kind: 'session', session: session('archived', 3, { isArchived: true }) }, + { kind: 'revision_conflict', expectedRevision: 3, actualRevision: 4 }, + { kind: 'session', session: session('archived', 4, { isArchived: false }) }, + ]); + + assert.equal(await client.removeSession('archived'), 'restored'); + assert.deepEqual( + requests.filter(({ operation }) => operation === 'session.remove').map(({ input }) => input), + [{ sessionId: 'archived', expectedRevision: 3 }], + ); +}); + +test('still retries an active Session remove after a rename conflict', async () => { + const { client, requests } = clientWithResponses([ + { kind: 'session', session: session('live', 3) }, + { kind: 'revision_conflict', expectedRevision: 3, actualRevision: 4 }, + { kind: 'session', session: session('live', 4) }, + { kind: 'removed', sessionId: 'live' }, + ]); + + assert.equal(await client.removeSession('live'), 'removed'); + assert.deepEqual( + requests.filter(({ operation }) => operation === 'session.remove').map(({ input }) => input), + [ + { sessionId: 'live', expectedRevision: 3 }, + { sessionId: 'live', expectedRevision: 4 }, + ], + ); +}); + test('settles revision cleanup when abandon observes an already absent target', async () => { const { client } = clientWithResponses([ { diff --git a/apps/desktop/src/main/runtime-host-client.ts b/apps/desktop/src/main/runtime-host-client.ts index 47ba8a108e..4106de6f69 100644 --- a/apps/desktop/src/main/runtime-host-client.ts +++ b/apps/desktop/src/main/runtime-host-client.ts @@ -838,14 +838,17 @@ export class DesktopRuntimeHostClient { ); } - async removeSession(sessionId: string): Promise { + async removeSession(sessionId: string): Promise<'removed' | 'restored'> { + let requiredArchived: boolean | undefined; for (let attempt = 0; attempt < MAX_SESSION_REVISION_ATTEMPTS; attempt += 1) { const current = await this.#requireSession(sessionId); + if (requiredArchived === undefined) requiredArchived = current.isArchived; + else if (requiredArchived && !current.isArchived) return 'restored'; const result = await this.request("session.remove", { sessionId, expectedRevision: current.revision, }); - if (result.kind === "removed") return; + if (result.kind === "removed") return 'removed'; } throw revisionConflict("remove", sessionId); } @@ -859,8 +862,8 @@ export class DesktopRuntimeHostClient { }); return result.kind === 'abandoned' ? 'removed' : 'retained'; } - await this.removeSession(sessionId); - return 'removed'; + const removed = await this.removeSession(sessionId); + return removed === 'removed' ? 'removed' : 'retained'; } catch (error) { if (isMissingSessionError(error)) return 'removed'; throw error; diff --git a/apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts b/apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts index 8168b4bc7b..f9ef3b3a18 100644 --- a/apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts @@ -191,8 +191,10 @@ export function registerRuntimeHostSessionCatalogIpc( ipcMain.handle('sessions:remove', async (_event, sessionId: string, options?: unknown) => { requestsRevisionFamily(options); const ids = await actionIds(sessionId, { revisionFamily: true }); - await deps.client.removeSession(sessionId); + const outcome = await deps.client.removeSession(sessionId); + if (outcome === 'restored') return { kind: 'restored' as const }; await finishSessionRetirement(deps, ids, 'deleted'); + return { kind: 'removed' as const }; }); } diff --git a/apps/desktop/src/preload/bridge-contract.d.ts b/apps/desktop/src/preload/bridge-contract.d.ts index 56aec7f324..a7cd08e701 100644 --- a/apps/desktop/src/preload/bridge-contract.d.ts +++ b/apps/desktop/src/preload/bridge-contract.d.ts @@ -492,7 +492,10 @@ export interface MakaBridge { abandonPlanExecution(sessionId: string, executionId: string): Promise; setModel(sessionId: string, input: { llmConnectionSlug: string; model: string }): Promise; setThinkingLevel(sessionId: string, level: ThinkingLevel | undefined | null): Promise; - remove(sessionId: string, options?: { revisionFamily?: boolean }): Promise; + remove( + sessionId: string, + options?: { revisionFamily?: boolean }, + ): Promise<{ kind: 'removed' } | { kind: 'restored' }>; cleanupSessionCopy(sessionId: string): Promise; abandonSessionCopy(sessionId: string): Promise; }; diff --git a/apps/desktop/src/renderer/app-shell-session-row-actions.ts b/apps/desktop/src/renderer/app-shell-session-row-actions.ts index 4ba076c25c..e971296b81 100644 --- a/apps/desktop/src/renderer/app-shell-session-row-actions.ts +++ b/apps/desktop/src/renderer/app-shell-session-row-actions.ts @@ -24,6 +24,8 @@ type ToastApi = { export interface SessionPurgeOutcome { /** Tasks confirmed gone. */ removed: number; + /** Tasks left alone because they were restored while the sweep ran. */ + restored: number; /** Tasks the catalog still reports. Empty when `verified` is false. */ remaining: string[]; verified: boolean; @@ -129,9 +131,9 @@ export function createAppShellSessionRowActions(deps: { destructive: true, }); if (!ok) return; - await removeSessionFamily(sessionId); + const outcome = await removeSessionFamily(sessionId); await refreshSessions(); - toastApi.success(copy.deletedTitle(name)); + if (outcome === 'removed') toastApi.success(copy.deletedTitle(name)); }); } @@ -141,16 +143,18 @@ export function createAppShellSessionRowActions(deps: { * deletion and released those resources, so the cleanup below is only ever * reached for a task that is really gone. */ - async function removeSessionFamily(sessionId: string): Promise { + async function removeSessionFamily(sessionId: string): Promise<'removed' | 'restored'> { // Read before the write: the family comes off the live catalog, which no // longer lists it afterwards. const familyIds = revisionFamilySessionIds(sessionsRef.current, sessionId); - await window.maka.sessions.remove(sessionId, { revisionFamily: true }); + const outcome = await window.maka.sessions.remove(sessionId, { revisionFamily: true }); + if (outcome.kind === 'restored') return 'restored'; if (activeIdRef.current && familyIds.includes(activeIdRef.current)) { setActiveId(undefined); setMessages([]); } for (const id of familyIds) clearSessionRendererState(id); + return 'removed'; } /** @@ -175,6 +179,7 @@ export function createAppShellSessionRowActions(deps: { const unsettled: string[] = []; let firstError: unknown; let removed = 0; + let restored = 0; for (const sessionId of sessionIds) { // Read the catalog as the sweep reaches each task, not once up front. A // sweep is serial, and a task restored from another window while it runs @@ -198,8 +203,9 @@ export function createAppShellSessionRowActions(deps: { } pendingSessionRowActionsRef.current.add(key); try { - await removeSessionFamily(sessionId); - removed += 1; + const outcome = await removeSessionFamily(sessionId); + if (outcome === 'restored') restored += 1; + else removed += 1; } catch (error) { unsettled.push(sessionId); firstError ??= error; @@ -209,7 +215,7 @@ export function createAppShellSessionRowActions(deps: { } if (unsettled.length === 0) { await refreshSessions(); - return { removed, remaining: [], verified: true, firstError }; + return { removed, restored, remaining: [], verified: true, firstError }; } let listed: SessionSummary[] | undefined; try { @@ -218,10 +224,16 @@ export function createAppShellSessionRowActions(deps: { listed = undefined; } await refreshSessions(); - if (!listed) return { removed, remaining: [], verified: false, firstError }; + if (!listed) return { removed, restored, remaining: [], verified: false, firstError }; const present = new Set(listed.map((session) => session.id)); const remaining = unsettled.filter((sessionId) => present.has(sessionId)); - return { removed: removed + (unsettled.length - remaining.length), remaining, verified: true, firstError }; + return { + removed: removed + (unsettled.length - remaining.length), + restored, + remaining, + verified: true, + firstError, + }; } return {