diff --git a/apps/desktop/scripts/test-tabs.mjs b/apps/desktop/scripts/test-tabs.mjs index 800a6dc80..b78c94acc 100644 --- a/apps/desktop/scripts/test-tabs.mjs +++ b/apps/desktop/scripts/test-tabs.mjs @@ -708,7 +708,8 @@ try { return { remove: i18n.t('canvasList.deleteCanvas'), confirm: i18n.t('spaceHierarchy.deleteCount', { count: 1 }), - blocked: i18n.t('desktopTabs.closeBeforeDeleting'), + cancel: i18n.t('actions.cancel'), + failure: i18n.t('spaceHierarchy.deletionErrors.agentRunning'), }; }); const createdCard = home @@ -718,19 +719,43 @@ try { await home .getByRole('menuitem', { name: deletionLabels.remove, exact: true }) .click(); + await home.getByRole('dialog').waitFor({ state: 'visible' }); + const deletionFailure = 'Deletion failed in smoke test'; + const deleteRoute = `**/api/canvas/${createdTab.canvasId}/delete-tree`; + await home.route(deleteRoute, (route) => + route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ + message: deletionFailure, + code: 'SPACE_AGENT_RUNNING', + }), + }), + ); await home - .getByText(deletionLabels.blocked, { exact: true }) + .getByRole('dialog') + .getByRole('button', { name: deletionLabels.confirm, exact: true }) + .click(); + await home + .getByRole('dialog') + .getByRole('alert') + .getByText(deletionLabels.failure, { exact: true }) .waitFor({ state: 'visible' }); assert.equal( - await home.getByRole('dialog').count(), - 0, - 'An open Space must not enter delete confirmation', + await home.getByText(deletionLabels.failure, { exact: true }).count(), + 1, + 'Deletion errors must appear only in the dialog, not a duplicate toast', ); + await home.unroute(deleteRoute); + await home + .getByRole('dialog') + .getByRole('button', { name: deletionLabels.cancel, exact: true }) + .click(); assert.ok( ( await shell.evaluate(() => window.electronBridge.tabs.getState()) ).tabs.some((tab) => tab.id === createdTab.id), - 'Refusing deletion must keep the open Space Tab', + 'Cancelling deletion must keep the open Space Tab', ); await request(`/canvas/${createdTab.canvasId}`); await shell.evaluate( @@ -1020,28 +1045,125 @@ try { assert.equal(note.content, `# Settled ${side} on outer close`); assert.equal(note.label, `Settled ${side} on outer close`); } - await createdCard.locator('button[aria-haspopup="menu"]').click(); - await home - .getByRole('menuitem', { name: deletionLabels.remove, exact: true }) - .click(); - const deleteDialog = home.getByRole('dialog'); + const deletionParent = await request('/canvas', 'POST', { + title: 'Deletion parent', + }); + await request(`/canvas/${createdTab.canvasId}/parent`, 'PATCH', { + parentSpaceId: deletionParent.canvasId, + expectedParentSpaceId: null, + }); + await home.evaluate( + (canvasId) => window.electronBridge.tabs.open({ canvasId }), + createdTab.canvasId, + ); + const deletingPage = await content(createdTab.canvasId); + const containerPage = await open(shell, deletionParent.canvasId); + await waitFor( + async () => + containerPage.evaluate(async (canvasId) => { + const { useWorkspaceStore } = + await import('/src/store/workspaceStore.ts'); + return useWorkspaceStore.getState().spaceSummaries[canvasId] + ?.parentEntryNodeId; + }, createdTab.canvasId), + 'loading the child Space entry', + ); + await containerPage.evaluate(async (canvasId) => { + const { default: store } = await import('/src/store/canvasStore.ts'); + const { useWorkspaceStore } = await import('/src/store/workspaceStore.ts'); + const entry = + useWorkspaceStore.getState().spaceSummaries[canvasId].parentEntryNodeId; + store + .getState() + .dispatchUiIntent({ type: 'DELETE_NODES', nodeIds: [entry] }); + }, createdTab.canvasId); + const deleteDialog = containerPage.getByRole('dialog'); await deleteDialog.waitFor({ state: 'visible' }); + let releaseDelete; + const deletionGate = new Promise((resolve) => { + releaseDelete = resolve; + }); + await containerPage.route('**/delete-tree', async (route) => { + await deletionGate; + await route.continue(); + }); await deleteDialog .getByRole('button', { name: deletionLabels.confirm, exact: true }) .click(); - await home - .locator(`a[href="/canvas/${createdTab.canvasId}"]`) - .waitFor({ state: 'detached' }); await waitFor( - async () => - !(await shell.evaluate(() => window.electronBridge.tabs.getState())).busy, - 'releasing the closed-Space deletion guard', + async () => deletingPage.evaluate(() => document.body.inert), + 'freezing the child while its parent owns deletion', + ); + assert.equal(await containerPage.evaluate(() => document.body.inert), false); + assert.equal(await deleteDialog.getByRole('status').count(), 1); + assert.match(await deleteDialog.ariaSnapshot(), /status/); + assert.equal( + await containerPage + .locator('dialog[data-desktop-close-controls][open]') + .count(), + 0, + ); + releaseDelete(); + await waitFor(async () => { + const state = await shell.evaluate(() => + window.electronBridge.tabs.getState(), + ); + return ( + !state.busy && + state.tabs.some( + (tab) => + tab.id === state.activeId && tab.canvasId === deletionParent.canvasId, + ) + ); + }, 'remaining in the parent after deleting its child'); + assert.equal( + deletingPage.isClosed(), + true, + 'Deleting an open child must close its Tab automatically', ); assert.equal( (await fetch(`${serverOrigin}/api/canvas/${createdTab.canvasId}`)).status, 404, - 'Closing the Tab must allow deleting the Space from Home', + 'An open Space must be deletable without returning to Home', + ); + await content(deletionParent.canvasId); + const deletionState = await shell.evaluate(() => + window.electronBridge.tabs.getState(), + ); + assert.equal( + deletionState.tabs.filter((tab) => tab.canvasId === deletionParent.canvasId) + .length, + 1, ); + assert.equal( + ( + await shell.evaluate( + (id) => window.electronBridge.tabs.close(id), + deletionState.activeId, + ) + ).ok, + true, + ); + await home.route(/\/api\/canvas(?:\?.*)?$/, (route) => + route.fulfill({ + status: 503, + contentType: 'application/json', + body: JSON.stringify({ message: 'List unavailable' }), + }), + ); + await shell.evaluate(() => + window.electronBridge.tabs.open({ canvasId: null }), + ); + await home + .locator(`a[href="/canvas/${createdTab.canvasId}"]`) + .waitFor({ state: 'detached' }); + await home.unroute(/\/api\/canvas(?:\?.*)?$/); + const parentDeletionPreview = await request( + `/canvas/${deletionParent.canvasId}/deletion-preview`, + ); + await request(`/canvas/${deletionParent.canvasId}/delete-tree`, 'POST', { + token: parentDeletionPreview.token, + }); if (process.env.HUABU_TABS_SCREENSHOT) { await home.screenshot({ path: process.env.HUABU_TABS_SCREENSHOT.replace(/\.png$/, '-home.png'), diff --git a/apps/desktop/src/desktop-tabs.test.ts b/apps/desktop/src/desktop-tabs.test.ts index a557f9e1b..5cc40bc82 100644 --- a/apps/desktop/src/desktop-tabs.test.ts +++ b/apps/desktop/src/desktop-tabs.test.ts @@ -423,18 +423,24 @@ describe('desktop Space Tab lifecycle', () => { await replacing; }); - it('blocks deleting a Space tree if a descendant has an open Tab', () => { - open('child'); - expect(() => - tabs.command( - { - type: 'begin-space-deletion', - canvasId: 'parent', - canvasIds: ['parent', 'child'], - }, - mocks.views[0].webContents as never, - ), - ).toThrow('This Space is open'); + it('prepares an open descendant before deleting a Space tree', async () => { + const child = open('child'); + const preparing = tabs.command( + { + type: 'begin-space-deletion', + canvasId: 'parent', + canvasIds: ['parent', 'child'], + }, + mocks.views[0].webContents as never, + ); + expect(tabs.snapshot.busy).toBe(true); + finish(child); + await preparing; + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['child', 'parent'] }, + mocks.views[0].webContents as never, + ); + expect(child.webContents.close).toHaveBeenCalledOnce(); expect(tabs.snapshot.busy).toBe(false); }); @@ -596,10 +602,10 @@ describe('desktop Space Tab lifecycle', () => { const message = view.webContents.send.mock.calls.findLast( ([channel]) => channel === 'tabs:prepare-close', ); - expect(message).toBeDefined(); + if (!message) throw new Error('Missing preparation request'); tabs.finishDrain( view.webContents as never, - message![1], + typeof message[1] === 'string' ? message[1] : message[1].requestId, ok ? { ok: true } : { ok: false, error: 'Disk full' }, ); } @@ -1510,21 +1516,28 @@ describe('desktop Space Tab lifecycle', () => { expect(tabs.snapshot.tabs.map((tab) => tab.canvasId)).toEqual([null]); }); - it('requires an open Space to be closed before deletion', () => { + it('keeps prepared Space Tabs alive until Home confirms deletion', async () => { const a = open('a'); - expect(() => - tabs.command( - { type: 'begin-space-deletion', canvasId: 'a' }, - mocks.views[0].webContents as never, - ), - ).toThrow('Close its Tab before deleting'); - expect(tabs.snapshot.busy).toBe(false); + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'a' }, + mocks.views[0].webContents as never, + ); + expect(tabs.snapshot.busy).toBe(true); + finish(a); + await preparing; expect(a.webContents.close).not.toHaveBeenCalled(); + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['a'] }, + mocks.views[0].webContents as never, + ); + expect(a.webContents.close).toHaveBeenCalledOnce(); + expect(tabs.snapshot.activeId).toBe('home'); + expect(tabs.snapshot.busy).toBe(false); }); it('blocks opening Tabs and changing Workspace until a closed-Space deletion finishes', async () => { const home = mocks.views[0].webContents; - tabs.command( + await tabs.command( { type: 'begin-space-deletion', canvasId: 'closed' }, home as never, ); @@ -1535,33 +1548,36 @@ describe('desktop Space Tab lifecycle', () => { ); await expect(tabs.prepareCloseAll(true)).rejects.toThrow('in progress'); expect(tabs.snapshot.busy).toBe(true); - tabs.command('end-space-deletion', home as never); + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['closed'] }, + home as never, + ); expect(tabs.snapshot.busy).toBe(false); expect(() => tabs.open({ canvasId: 'another' })).not.toThrow(); }); - it('does not allow a Space renderer to acquire or release Home deletion guards', () => { + it('allows a Space requester but rejects release by another renderer', async () => { const a = open('a'); + await tabs.command( + { type: 'begin-space-deletion', canvasId: 'closed' }, + a.webContents as never, + ); expect(() => tabs.command( - { type: 'begin-space-deletion', canvasId: 'closed' }, - a.webContents as never, + { type: 'end-space-deletion', deletedCanvasIds: [] }, + mocks.views[0].webContents as never, ), - ).toThrow('Only Home'); + ).toThrow('belongs to this Tab'); + expect(tabs.snapshot.busy).toBe(true); tabs.command( - { type: 'begin-space-deletion', canvasId: 'closed' }, - mocks.views[0].webContents as never, + { type: 'end-space-deletion', deletedCanvasIds: [] }, + a.webContents as never, ); - expect(() => - tabs.command('end-space-deletion', a.webContents as never), - ).toThrow('Only Home'); - expect(tabs.snapshot.busy).toBe(true); - tabs.command('end-space-deletion', mocks.views[0].webContents as never); }); - it('releases an abandoned deletion guard when Home crashes without dropping healthy Space Tabs', () => { + it('releases an abandoned deletion guard when Home crashes without dropping healthy Space Tabs', async () => { open('a'); - tabs.command( + await tabs.command( { type: 'begin-space-deletion', canvasId: 'closed' }, mocks.views[0].webContents as never, ); @@ -1571,15 +1587,340 @@ describe('desktop Space Tab lifecycle', () => { expect(tabs.snapshot.tabs.map((tab) => tab.canvasId)).toEqual([null, 'a']); }); - it('does not release a deletion guard when an unrelated Space crashes', () => { + it('does not release a deletion guard when an unrelated Space crashes', async () => { const a = open('a'); - tabs.command( + await tabs.command( { type: 'begin-space-deletion', canvasId: 'closed' }, mocks.views[0].webContents as never, ); crash(a); expect(tabs.snapshot.busy).toBe(true); - tabs.command('end-space-deletion', mocks.views[0].webContents as never); + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: [] }, + mocks.views[0].webContents as never, + ); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('retains all prepared Tabs if saving fails', async () => { + const a = open('a'); + const b = open('b'); + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'a', canvasIds: ['a', 'b'] }, + mocks.views[0].webContents as never, + ); + finish(a); + finish(b, false); + await expect(preparing).rejects.toThrow('Disk full'); + expect(a.webContents.close).not.toHaveBeenCalled(); + expect(b.webContents.close).not.toHaveBeenCalled(); + expect(a.webContents.send).toHaveBeenCalledWith('tabs:resume'); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('closes only confirmed deletions and resumes surviving Tabs after partial failure', async () => { + const parent = open('parent'); + const child = open('child'); + const container = open('container'); + const preparing = tabs.command( + { + type: 'begin-space-deletion', + canvasId: 'parent', + canvasIds: ['parent', 'child'], + }, + container.webContents as never, + ); + finish(parent); + finish(child); + await preparing; + expect(() => + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['unrelated'] }, + container.webContents as never, + ), + ).toThrow('Invalid Space deletion result'); + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['child'] }, + container.webContents as never, + ); + expect(child.webContents.close).toHaveBeenCalledOnce(); + expect(parent.webContents.close).not.toHaveBeenCalled(); + expect(parent.webContents.send).toHaveBeenCalledWith('tabs:resume'); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('releases a Space-owned deletion guard if its requester crashes', async () => { + const parent = open('parent'); + await tabs.command( + { type: 'begin-space-deletion', canvasId: 'child' }, + parent.webContents as never, + ); + crash(parent); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('retains the requesting Tab when deletion fails', async () => { + const a = open('a'); + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'child' }, + a.webContents as never, + ); + await preparing; + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: [] }, + a.webContents as never, + ); + expect(a.webContents.close).not.toHaveBeenCalled(); + expect(a.webContents.send).toHaveBeenCalledWith('tabs:resume'); + expect( + tabs.snapshot.tabs.find((tab) => tab.id === tabs.snapshot.activeId) + ?.canvasId, + ).toBe('a'); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('keeps the active parent unchanged when deleting its child', async () => { + const parent = open('parent'); + const parentId = tabs.snapshot.activeId; + const child = open('child'); + tabs.activate(parentId); + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'child' }, + parent.webContents as never, + ); + finish(child); + await preparing; + tabs.command( + { + type: 'end-space-deletion', + deletedCanvasIds: ['child'], + }, + parent.webContents as never, + ); + expect(tabs.snapshot.activeId).toBe(parentId); + expect(parent.webContents.close).not.toHaveBeenCalled(); + expect(child.webContents.close).toHaveBeenCalledOnce(); + expect(mocks.views).toHaveLength(3); + }); + + it.each([['child'], ['parent', 'child']])( + 'refuses deletion of the requester itself or an ancestor containing it: %j', + async (...canvasIds) => { + const child = open('child'); + await expect( + tabs.command( + { type: 'begin-space-deletion', canvasId: canvasIds[0], canvasIds }, + child.webContents as never, + ), + ).rejects.toThrow('Delete this Space from Home or its parent Space'); + expect(tabs.snapshot.busy).toBe(false); + expect(child.webContents.close).not.toHaveBeenCalled(); + expect(child.webContents.send).not.toHaveBeenCalledWith( + 'tabs:prepare-close', + expect.anything(), + ); + }, + ); + + it('keeps an unrelated active Space when a background subtree is deleted', async () => { + const child = open('child'); + const other = open('other'); + const activeId = tabs.snapshot.activeId; + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'child' }, + other.webContents as never, + ); + finish(child); + await preparing; + tabs.command( + { + type: 'end-space-deletion', + deletedCanvasIds: ['child'], + }, + other.webContents as never, + ); + expect(tabs.snapshot.activeId).toBe(activeId); + expect(tabs.snapshot.tabs.map((tab) => tab.canvasId)).toEqual([ + null, + 'other', + ]); + expect(mocks.views).toHaveLength(3); + }); + + it('preserves remaining Tab order and publishes confirmed deletions to Home', async () => { + const sibling = open('sibling'); + const child = open('child'); + open('a'); + open('b'); + tabs.activate('home'); + const preparing = tabs.command( + { + type: 'begin-space-deletion', + canvasId: 'child', + canvasIds: ['sibling', 'child'], + }, + mocks.views[0].webContents as never, + ); + finish(sibling); + finish(child); + await preparing; + await tabs.command( + { + type: 'end-space-deletion', + deletedCanvasIds: ['sibling', 'child'], + }, + mocks.views[0].webContents as never, + ); + expect(tabs.snapshot.tabs.map((tab) => tab.canvasId)).toEqual([ + null, + 'a', + 'b', + ]); + expect(tabs.snapshot.activeId).toBe('home'); + expect(mocks.views[0].webContents.send).toHaveBeenCalledWith( + 'tabs:spaces-deleted', + { + workspaceId: 'workspace', + deletedCanvasIds: ['sibling', 'child'], + }, + ); + }); + + it('does not publish deletions when none were confirmed', async () => { + const home = mocks.views[0].webContents; + await tabs.command( + { type: 'begin-space-deletion', canvasId: 'closed' }, + home as never, + ); + tabs.command( + { + type: 'end-space-deletion', + deletedCanvasIds: [], + }, + home as never, + ); + expect(home.send).not.toHaveBeenCalledWith( + 'tabs:spaces-deleted', + expect.anything(), + ); + }); + + it('cancels deletion preparation without closing any Tab', async () => { + const a = open('a'); + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'a' }, + mocks.views[0].webContents as never, + ); + const request = a.webContents.send.mock.calls.findLast( + ([channel]) => channel === 'tabs:prepare-close', + ); + if (!request) throw new Error('Missing preparation request'); + tabs.finishDrain(a.webContents as never, request[1].requestId, { + ok: false, + cancelled: true, + error: 'Cancelled', + }); + await expect(preparing).rejects.toThrow('Closing was cancelled'); + expect(a.webContents.close).not.toHaveBeenCalled(); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('bounds deletion preparation despite heartbeats and ignores late replies after timeout', async () => { + vi.useFakeTimers(); + const a = open('a'); + const b = open('b'); + const home = mocks.views[0].webContents; + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'a', canvasIds: ['a', 'b'] }, + home as never, + ); + const rejected = expect(preparing).rejects.toThrow('timed out'); + const request = a.webContents.send.mock.calls.at(-1); + if (!request) throw new Error('Missing preparation request'); + const requestId = request[1].requestId; + finish(b); + for (let elapsed = 0; elapsed < 10_000; elapsed += 5_000) { + await vi.advanceTimersByTimeAsync(5_000); + tabs.keepDrainAlive(a.webContents as never, requestId); + } + await vi.advanceTimersByTimeAsync(4_999); + expect(tabs.snapshot.busy).toBe(true); + await vi.advanceTimersByTimeAsync(1); + await rejected; + expect(tabs.snapshot.busy).toBe(false); + for (const view of [a, b]) { + expect(view.webContents.close).not.toHaveBeenCalled(); + expect(view.webContents.send).toHaveBeenCalledWith('tabs:resume'); + } + const retry = tabs.command( + { type: 'begin-space-deletion', canvasId: 'a' }, + home as never, + ); + tabs.keepDrainAlive(a.webContents as never, requestId); + tabs.finishDrain(a.webContents as never, requestId, { ok: true }); + await Promise.resolve(); + expect(() => + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: [] }, + home as never, + ), + ).toThrow('No Space deletion'); + finish(a); + await retry; + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: [] }, + home as never, + ); + expect(tabs.snapshot.busy).toBe(false); + }); + + it('removes lazy restored Tabs without loading them and persists their removal', async () => { + open('child'); + tabs.activate('home'); + tabs.dispose(); + mocks.views.length = 0; + tabs = makeTabs(); + const home = mocks.views[0].webContents; + tabs.ready(home as never); + tabs.workspaceReady(home as never, 'workspace'); + expect(mocks.views).toHaveLength(1); + await tabs.command( + { + type: 'begin-space-deletion', + canvasId: 'parent', + canvasIds: ['parent', 'child'], + }, + home as never, + ); + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['parent', 'child'] }, + home as never, + ); + expect(mocks.views).toHaveLength(1); + expect(tabs.snapshot.tabs.map((tab) => tab.canvasId)).toEqual([null]); + const saved = JSON.parse( + readFileSync(join(directory, 'desktop-tabs.json'), 'utf8'), + ); + expect(saved[JSON.stringify(['local', 'workspace'])].canvasIds).toEqual([]); + }); + + it('closes deleted pages and releases the lock even if saving the Tab session fails', async () => { + const a = open('a'); + const preparing = tabs.command( + { type: 'begin-space-deletion', canvasId: 'a' }, + mocks.views[0].webContents as never, + ); + finish(a); + await preparing; + mkdirSync(join(directory, 'desktop-tabs.json.tmp')); + expect(() => + tabs.command( + { type: 'end-space-deletion', deletedCanvasIds: ['a'] }, + mocks.views[0].webContents as never, + ), + ).toThrow(); + expect(a.webContents.close).toHaveBeenCalledOnce(); + expect(tabs.snapshot.tabs.map((tab) => tab.canvasId)).toEqual([null]); expect(tabs.snapshot.busy).toBe(false); }); diff --git a/apps/desktop/src/desktop-tabs.ts b/apps/desktop/src/desktop-tabs.ts index 95f2a04e0..41dcb7c71 100644 --- a/apps/desktop/src/desktop-tabs.ts +++ b/apps/desktop/src/desktop-tabs.ts @@ -91,7 +91,8 @@ export class DesktopTabs { private origin: string; private sessionPath: string; private workspaceChanging = false; - private spaceDeleting = false; + private spaceDeleting: { owner: ContentTab; canvasIds: string[] } | null = + null; private legacyStorage: Record = {}; private preferences: Record = {}; @@ -357,11 +358,11 @@ export class DesktopTabs { tab.state.error = message; tab.state.loading = false; if ( - tab.state.id === HOME_ID && - (this.workspaceChanging || this.spaceDeleting) + (tab.state.id === HOME_ID && this.workspaceChanging) || + this.spaceDeleting?.owner === tab ) { this.workspaceChanging = false; - this.spaceDeleting = false; + this.spaceDeleting = null; this.resume(); } else this.publish(); if (notify) @@ -653,7 +654,10 @@ export class DesktopTabs { this.publish(); } - private async drain(tab: ContentTab): Promise { + private async drain( + tab: ContentTab, + reason?: 'space-deletion', + ): Promise { const view = tab.view; if (!view) return; if (!tab.ready) @@ -667,6 +671,8 @@ export class DesktopTabs { await new Promise((resolve, reject) => { const heartbeat = () => { if (settled) return; + // Deletion has no discard/cancel overlay in the affected child. + if (reason === 'space-deletion' && timer !== undefined) return; clearTimeout(timer); timer = setTimeout( () => @@ -685,7 +691,10 @@ export class DesktopTabs { heartbeat, }); heartbeat(); - view.webContents.send('tabs:prepare-close', requestId); + view.webContents.send( + 'tabs:prepare-close', + reason ? { requestId, reason } : requestId, + ); }); } finally { settled = true; @@ -1035,7 +1044,41 @@ export class DesktopTabs { ?.view?.webContents.send('tabs:home-menu', position); } - command(raw: unknown, sender?: WebContents): void { + private async beginSpaceDeletion( + owner: ContentTab, + canvasIds: string[], + ): Promise { + if (this.busy) throw new Error('Another Tab operation is in progress'); + if (owner.state.canvasId && canvasIds.includes(owner.state.canvasId)) + throw new Error('Delete this Space from Home or its parent Space'); + this.busy = true; + this.setHomeMenu(null); + this.publish(); + try { + const results = await Promise.allSettled( + this.tabs + .filter( + (tab) => + tab.state.canvasId && canvasIds.includes(tab.state.canvasId), + ) + .map((tab) => this.drain(tab, 'space-deletion')), + ); + const failure = results.find((result) => result.status === 'rejected'); + if (failure?.status === 'rejected') throw failure.reason; + if (!owner.ready) + throw new Error( + 'The requesting Tab became unavailable during deletion', + ); + // Keep prepared renderers alive until the requester finishes the HTTP operation. + this.spaceDeleting = { owner, canvasIds }; + this.closePrepared(); + } catch (error) { + this.resume(); + throw error; + } + } + + command(raw: unknown, sender?: WebContents): void | Promise { if ( raw && typeof raw === 'object' && @@ -1123,19 +1166,52 @@ export class DesktopTabs { return; } if ( - raw === 'end-space-deletion' || - (raw && - typeof raw === 'object' && - 'type' in raw && - raw.type === 'begin-space-deletion') + raw && + typeof raw === 'object' && + 'type' in raw && + (raw.type === 'begin-space-deletion' || raw.type === 'end-space-deletion') ) { - if (!sender || this.contentTab(sender).state.id !== HOME_ID) - throw new Error('Only Home can delete a Space'); - if (raw === 'end-space-deletion') { - if (!this.spaceDeleting) - throw new Error('No Space deletion is in progress'); - this.spaceDeleting = false; - this.resume(); + if (!sender) throw new Error('Space deletion requires a content Tab'); + const owner = this.contentTab(sender); + if (raw.type === 'end-space-deletion') { + const deletion = this.spaceDeleting; + if (!deletion || deletion.owner !== owner) + throw new Error('No Space deletion belongs to this Tab'); + const deletedIds = + 'deletedCanvasIds' in raw ? raw.deletedCanvasIds : undefined; + if ( + !Array.isArray(deletedIds) || + deletedIds.some( + (id: unknown) => + typeof id !== 'string' || !deletion.canvasIds.includes(id), + ) + ) + throw new Error('Invalid Space deletion result'); + const removed = this.tabs.filter((tab) => + deletedIds.includes(tab.state.canvasId), + ); + const remaining = this.tabs.filter((tab) => !removed.includes(tab)); + const activeId = remaining.some((tab) => tab.state.id === this.activeId) + ? this.activeId + : owner.state.id; + try { + this.persist(remaining, activeId); + } finally { + // Deleted pages must not resume writes, even if session persistence fails. + this.tabs = remaining; + this.activeId = activeId; + for (const tab of removed) this.destroyTab(tab); + this.spaceDeleting = null; + if (deletedIds.length) + this.tabs + .find((tab) => tab.state.id === HOME_ID) + ?.view?.webContents.send('tabs:spaces-deleted', { + workspaceId: this.workspaceId, + deletedCanvasIds: deletedIds, + }); + this.showActiveTab(); + this.resume(); + } return; } if (this.busy) throw new Error('Another Tab operation is in progress'); @@ -1155,13 +1231,7 @@ export class DesktopTabs { ) ) throw new Error('Invalid Space deletion identities'); - if (this.tabs.some((tab) => canvasIds.includes(tab.state.canvasId))) - throw new Error('This Space is open. Close its Tab before deleting.'); - this.spaceDeleting = true; - this.busy = true; - this.setHomeMenu(null); - this.publish(); - return; + return this.beginSpaceDeletion(owner, canvasIds); } if (raw === 'close-home-menu' || raw === 'dismiss-home-menu') { if ( diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index 2236c6fac..20488bcfc 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -121,6 +121,8 @@ const tabs: DesktopTabsApi | undefined = onNavigate: (callback: (target: DesktopTabNavigation) => void) => subscribe('tabs:navigate', callback), onPrepareClose: (callback) => subscribe('tabs:prepare-close', callback), + onSpacesDeleted: (callback) => + subscribe('tabs:spaces-deleted', callback), keepPrepareCloseAlive: (id) => ipcRenderer.send('tabs:preparing-close', id), onClosePrepared: (callback) => diff --git a/apps/server/src/modules/canvas/space-hierarchy.test.ts b/apps/server/src/modules/canvas/space-hierarchy.test.ts index 0b8ab64ac..ec5f18ec6 100644 --- a/apps/server/src/modules/canvas/space-hierarchy.test.ts +++ b/apps/server/src/modules/canvas/space-hierarchy.test.ts @@ -867,6 +867,15 @@ describe('Space ownership', () => { }); await expect(storage.deleteSpace('root')).rejects.toThrow('children'); const preview = deletionPreview(await list(), 'root'); + expect(preview.spaces).toEqual([ + { + canvasId: grandchild.canvasId, + title: 'Grandchild', + parentSpaceId: child.canvasId, + }, + { canvasId: child.canvasId, title: 'Child', parentSpaceId: 'root' }, + { canvasId: 'root', title: 'Root', parentSpaceId: null }, + ]); const publish = vi.spyOn(canvasSync, 'publishCanvasUpdate'); expect(await deleteSpaceTree('root', preview.token)).toEqual([ grandchild.canvasId, diff --git a/apps/server/src/modules/canvas/space-hierarchy.ts b/apps/server/src/modules/canvas/space-hierarchy.ts index d27b5bf85..dfe550bb9 100644 --- a/apps/server/src/modules/canvas/space-hierarchy.ts +++ b/apps/server/src/modules/canvas/space-hierarchy.ts @@ -126,7 +126,11 @@ export function deletionPreview( token: createHash('sha256') .update(JSON.stringify([getWorkspaceKey(), identity])) .digest('hex'), - spaces: subtree.map(({ canvasId, title }) => ({ canvasId, title })), + spaces: subtree.map(({ canvasId, title, parentSpaceId }) => ({ + canvasId, + title, + parentSpaceId: parentSpaceId ?? null, + })), }; } diff --git a/apps/web/e2e/desktop-tab-drag.spec.ts b/apps/web/e2e/desktop-tab-drag.spec.ts index 2859c0d1e..fe357e6b1 100644 --- a/apps/web/e2e/desktop-tab-drag.spec.ts +++ b/apps/web/e2e/desktop-tab-drag.spec.ts @@ -69,6 +69,7 @@ test.beforeEach(async ({ page }) => { onHomeMenu: () => () => {}, onNavigate: () => () => {}, onPrepareClose: () => () => {}, + onSpacesDeleted: () => () => {}, onClosePrepared: () => () => {}, onResume: () => () => {}, ready: () => {}, diff --git a/apps/web/e2e/space-hierarchy.spec.ts b/apps/web/e2e/space-hierarchy.spec.ts index c6f556305..7f432a9b4 100644 --- a/apps/web/e2e/space-hierarchy.spec.ts +++ b/apps/web/e2e/space-hierarchy.spec.ts @@ -593,15 +593,11 @@ test('immediate Undo after child creation keeps its entry and leaves saves usabl .toBe(false); }); -test('leaves a deleted current Space and keeps the partial-cleanup error visible', async ({ +test('Home retains partial-cleanup feedback after deleting a Space', async ({ page, }) => { const id = await create(page, 'Partial cleanup'); - await page.goto(`/canvas/${id}`); - await expect(page.locator('[data-canvas-root]')).toHaveAttribute( - 'aria-busy', - 'false', - ); + await page.goto('/spaces'); await page.route(`**/api/canvas/${id}/delete-tree`, async (route) => { const response = await route.fetch(); expect(response.ok()).toBe(true); @@ -619,21 +615,248 @@ test('leaves a deleted current Space and keeps the partial-cleanup error visible }), }); }); - await page.getByRole('button', { name: 'Space menu', exact: true }).click(); + await page + .getByRole('button', { name: 'Actions for Partial cleanup', exact: true }) + .click(); await page .getByRole('menuitem', { name: 'Delete Space', exact: true }) .click(); + const dialog = page.getByRole('dialog'); + await expect(dialog).toHaveAccessibleName('Delete “Partial cleanup”?'); + await dialog + .getByRole('button', { name: 'Delete Space', exact: true }) + .click(); + await expect(page).toHaveURL(/\/spaces$/); + await expect(dialog).toHaveAccessibleName('“Partial cleanup” was deleted'); + await expect( + page.getByText( + 'Deletion did not finish cleanly. Check the current Space list before continuing.', + { exact: true }, + ), + ).toBeVisible(); +}); + +test('Space menus cannot delete themselves; a parent can delete its child entry', async ({ + page, +}) => { + const parent = await create(page, 'Container deletion parent'); + const response = await page.request.post(`/api/canvas/${parent}/children`, { + data: { title: 'Container deletion child' }, + }); + expect(response.ok()).toBe(true); + const child = await response.json(); + for (const id of [parent, child.canvasId]) { + await page.goto(`/canvas/${id}`); + await page.getByRole('button', { name: 'Space menu', exact: true }).click(); + await expect( + page.getByRole('menuitem', { name: 'Delete Space', exact: true }), + ).toHaveCount(0); + await page.keyboard.press('Escape'); + } + await page.goto(`/canvas/${parent}`); + await expect(page.locator('[data-canvas-root]')).toHaveAttribute( + 'aria-busy', + 'false', + ); + await expect + .poll(() => + page.evaluate(async (id) => { + const path = '/src/store/workspaceStore.ts'; + const { useWorkspaceStore } = (await import( + path + )) as typeof WorkspaceStore; + return useWorkspaceStore.getState().spaceSummaries[id] + ?.parentEntryNodeId; + }, child.canvasId), + ) + .toBe(child.parentEntryNodeId); + await page.evaluate(async (entryId) => { + const path = '/src/store/canvasStore.ts'; + const { default: store } = (await import(path)) as typeof CanvasStore; + store + .getState() + .dispatchUiIntent({ type: 'DELETE_NODES', nodeIds: [entryId] }); + }, child.parentEntryNodeId); const dialog = page.getByRole('dialog', { - name: 'Delete “Partial cleanup”?', + name: 'Delete “Container deletion child”?', exact: true, }); await dialog .getByRole('button', { name: 'Delete Space', exact: true }) .click(); - await expect(page).toHaveURL(/\/spaces$/); + await expect(dialog).toHaveCount(0); + await expect(page).toHaveURL(new RegExp(`/canvas/${parent}$`)); + expect( + (await page.request.get(`/api/canvas/${child.canvasId}`)).status(), + ).toBe(404); + expect((await page.request.get(`/api/canvas/${parent}`)).ok()).toBe(true); +}); + +test('deletion recovery stays localized, traps focus and requires a fresh confirmation', async ({ + page, +}, testInfo) => { + const id = await create(page, '项目资料'); + const childResponse = await page.request.post(`/api/canvas/${id}/children`, { + data: { title: '调研' }, + }); + expect(childResponse.ok()).toBe(true); + const child = (await childResponse.json()).canvasId as string; + await page.goto('/spaces'); + await page.evaluate(async () => { + const path = '/src/i18n/index.ts'; + const { i18n } = await import(path); + await i18n.changeLanguage('zh-CN'); + }); + let requests = 0; + let releaseDeletion!: () => void; + const deletionGate = new Promise((resolve) => { + releaseDeletion = resolve; + }); + await page.route(`**/api/canvas/${id}/delete-tree`, async (route) => { + requests++; + if (requests === 1) await deletionGate; + return route.fulfill({ + status: requests === 1 ? 409 : 500, + contentType: 'application/json', + body: JSON.stringify( + requests === 1 + ? { + code: 'SPACE_AGENT_RUNNING', + message: 'Stop running Agents before deleting this subtree', + } + : { + code: 'SPACE_TREE_DELETE_FAILED', + message: 'Subtree deletion did not complete', + details: { + deletedCanvasIds: [child], + remainingCanvasIds: [id], + cause: 'Storage failure', + }, + }, + ), + }); + }); + await page + .locator(`a[href="/canvas/${id}"]`) + .locator('..') + .locator('button[aria-haspopup="menu"]') + .click(); + await page.getByRole('menuitem', { name: '删除 Space', exact: true }).click(); + const dialog = page.getByRole('dialog'); + const capture = async (name: string) => { + const box = await dialog.boundingBox(); + if (!box) throw new Error('Deletion dialog is missing'); + await page.screenshot({ + path: testInfo.outputPath(name), + animations: 'disabled', + clip: { + x: box.x - 32, + y: box.y - 32, + width: box.width + 64, + height: box.height + 64, + }, + }); + }; + const cancel = dialog.getByRole('button', { name: '取消', exact: true }); + const remove = dialog.getByRole('button', { + name: '删除 2 个 Space', + exact: true, + }); + await expect(remove).toBeEnabled(); + await expect(cancel).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(remove).toBeFocused(); + await page.keyboard.press('Tab'); + await expect(cancel).toBeFocused(); + await page.keyboard.press('Shift+Tab'); + await expect(remove).toBeFocused(); + await cancel.focus(); + await capture('deletion-confirm.png'); + await remove.click(); + await expect(dialog).toHaveAccessibleName('正在删除「项目资料」'); + await expect(dialog.getByRole('status')).toHaveText( + '请稍候，正在保存更改并处理删除。', + ); + await expect(cancel).toBeDisabled(); await expect( - page.getByText('Parent cleanup failed', { exact: true }), - ).toBeVisible(); + dialog.getByRole('button', { name: '正在删除…', exact: true }), + ).toBeDisabled(); + await page.keyboard.press('Escape'); + await page.mouse.click(8, 8); + await expect(dialog).toBeVisible(); + await capture('deletion-busy.png'); + expect(requests).toBe(1); + releaseDeletion(); + const reason = + '此 Space 或子 Space 中仍有 Agent 正在处理任务。请先停止任务或等待完成，再重新检查。'; + await expect(dialog).toHaveAccessibleName('未能完成删除「项目资料」'); + await expect(dialog.getByRole('alert')).toHaveText(reason); + await expect(dialog.getByRole('status')).toHaveCount(0); + await expect(page.getByText(reason, { exact: true })).toHaveCount(1); + await expect(dialog.getByRole('button')).toHaveCount(2); + const recheck = dialog.getByRole('button', { + name: '重新检查', + exact: true, + }); + await recheck.focus(); + await page.keyboard.press('Tab'); + await expect(cancel).toBeFocused(); + await page.keyboard.press('Shift+Tab'); + await expect(recheck).toBeFocused(); + await cancel.focus(); + await capture('deletion-error-light.png'); + const lightSurface = await dialog.evaluate( + (element) => getComputedStyle(element).backgroundColor, + ); + await page.evaluate(() => document.documentElement.classList.add('dark')); + await expect + .poll(() => + dialog.evaluate((element) => getComputedStyle(element).backgroundColor), + ) + .not.toBe(lightSurface); + await capture('deletion-error-dark.png'); + await page.setViewportSize({ width: 390, height: 700 }); + await expect(dialog).toHaveCSS('max-width', '342px'); + await page.screenshot({ + path: testInfo.outputPath('deletion-error-narrow.png'), + animations: 'disabled', + }); + await page.setViewportSize({ width: 1280, height: 800 }); + await page.evaluate(() => document.documentElement.classList.remove('dark')); + await recheck.click(); + await expect(remove).toBeEnabled(); + await expect(cancel).toBeFocused(); + expect(requests).toBe(1); + await remove.click(); + await expect(dialog.getByRole('alert')).toContainText( + '已删除 1 个 Space，已删除的内容无法恢复。', + ); + await expect(dialog.getByRole('button')).toHaveCount(2); + await expect(recheck).toBeEnabled(); + await capture('deletion-partial.png'); + await page.route(`**/api/canvas/${id}/deletion-preview`, (route) => + route.fulfill({ + status: 500, + contentType: 'application/json', + body: JSON.stringify({ + message: 'Network diagnostic detail. '.repeat(60), + }), + }), + ); + await recheck.click(); + await expect(dialog.getByRole('alert')).toContainText( + '暂时无法获取删除范围，请检查连接后重新检查。', + ); + await expect(dialog.locator('details')).not.toHaveAttribute('open'); + await dialog.locator('summary').click(); + await page.setViewportSize({ width: 390, height: 360 }); + await expect(dialog).toHaveCSS('max-height', '312px'); + const bounds = await dialog.boundingBox(); + if (!bounds) throw new Error('Deletion dialog is missing'); + expect(bounds.y).toBeGreaterThanOrEqual(24); + expect(bounds.y + bounds.height).toBeLessThanOrEqual(336); + await cancel.click(); + await expect(dialog).toBeHidden(); }); test('change-parent menu waits for metadata before opening', async ({ diff --git a/apps/web/src/components/Common/Modal.test.tsx b/apps/web/src/components/Common/Modal.test.tsx index 8ed3c3d95..5d4bfaf63 100644 --- a/apps/web/src/components/Common/Modal.test.tsx +++ b/apps/web/src/components/Common/Modal.test.tsx @@ -1,10 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { act } from 'react'; +import { act, createRef } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Button } from './Button'; import { Modal } from './Modal'; globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -37,4 +38,43 @@ describe('Modal overlay', () => { ).toBe(true); expect(overlay?.style.zIndex).toBe('9999'); }); + + it('honors initial focus and restores the trigger on close', async () => { + const trigger = document.createElement('button'); + document.body.append(trigger); + trigger.focus(); + const cancelRef = createRef(); + try { + await act(async () => { + root.render( + Cancel} + />, + ); + }); + await vi.waitFor(() => + expect(document.activeElement).toBe(cancelRef.current), + ); + await act(async () => { + root.render(); + }); + await vi.waitFor(() => expect(document.activeElement).toBe(trigger)); + } finally { + trigger.remove(); + } + }); + + it('focuses an empty dialog and bounds long content to the viewport', async () => { + await act(async () => { + root.render(); + }); + const dialog = document.querySelector('[role="dialog"]'); + await vi.waitFor(() => expect(document.activeElement).toBe(dialog)); + expect(dialog?.classList.contains('overflow-y-auto')).toBe(true); + expect(dialog?.className).toContain('max-h-[calc(100dvh-'); + }); }); diff --git a/apps/web/src/components/Common/Modal.tsx b/apps/web/src/components/Common/Modal.tsx index c88c8a390..074a0ce59 100644 --- a/apps/web/src/components/Common/Modal.tsx +++ b/apps/web/src/components/Common/Modal.tsx @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import { FloatingFocusManager, useFloating } from '@floating-ui/react'; import { useEffect, useId, useMemo, - useRef, type ReactNode, type RefObject, } from 'react'; @@ -60,8 +60,7 @@ export function Modal({ const titleId = useId(); const descriptionId = useId(); - const dialogRef = useRef(null); - const previouslyFocused = useRef(null); + const { refs, context } = useFloating({ open: isOpen }); const labelledBy = useMemo( () => (title ? titleId : undefined), @@ -72,31 +71,16 @@ export function Modal({ [description, descriptionId], ); - // Lock scroll + focus management useEffect(() => { if (!isOpen) return; - previouslyFocused.current = document.activeElement as HTMLElement | null; - const originalOverflow = document.body.style.overflow; document.body.style.overflow = 'hidden'; - const timer = window.setTimeout(() => { - const target = initialFocusRef?.current; - if (target) { - target.focus(); - } else { - dialogRef.current?.focus(); - } - }, 0); - return () => { - window.clearTimeout(timer); document.body.style.overflow = originalOverflow; - previouslyFocused.current?.focus?.(); - previouslyFocused.current = null; }; - }, [isOpen, initialFocusRef]); + }, [isOpen]); // Close on Escape useEffect(() => { @@ -124,43 +108,52 @@ export function Modal({ onClick={closeOnBackdropClick ? onClose : undefined} /> -
- {(title || description) && ( -
- {title && ( -

- {title} -

- )} - {description && ( -
- {description} -
- )} -
- )} - - {children} - - {footer &&
{footer}
} -
+
+ {(title || description) && ( +
+ {title && ( +

+ {title} +

+ )} + {description && ( +
+ {description} +
+ )} +
+ )} + + {children} + + {footer && ( +
{footer}
+ )} +
+ , container ?? document.body, ); diff --git a/apps/web/src/components/Panels/Header/CanvasMenu.tsx b/apps/web/src/components/Panels/Header/CanvasMenu.tsx index 0713851af..51a4b36f6 100644 --- a/apps/web/src/components/Panels/Header/CanvasMenu.tsx +++ b/apps/web/src/components/Panels/Header/CanvasMenu.tsx @@ -6,10 +6,7 @@ import { ChevronDown } from 'lucide-react'; import React, { useCallback, useEffect, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { - REQUEST_DELETE_SPACE, - REQUEST_REPARENT_SPACE, -} from '@/utils/spaceHierarchy'; +import { REQUEST_REPARENT_SPACE } from '@/utils/spaceHierarchy'; import { ApiError } from '../../../api/_client'; import { exportCanvas } from '../../../api/canvas.ts'; @@ -223,16 +220,6 @@ export const CanvasMenu: React.FC = () => { {t('spaceHierarchy.moveToTopLevel')} )} - { - setIsOpen(false); - window.dispatchEvent( - new CustomEvent(REQUEST_DELETE_SPACE, { detail: canvasId }), - ); - }} - > - {t('canvasList.deleteCanvas')} - )} diff --git a/apps/web/src/components/Space/SpaceActions.test.tsx b/apps/web/src/components/Space/SpaceActions.test.tsx new file mode 100644 index 000000000..e4abca971 --- /dev/null +++ b/apps/web/src/components/Space/SpaceActions.test.tsx @@ -0,0 +1,117 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; + +import { REQUEST_DELETE_SPACE } from '@/utils/spaceHierarchy'; + +import { SpaceActions } from './SpaceActions'; + +const mocks = vi.hoisted(() => ({ + canvas: { canvasId: 'parent' }, + workspace: { + workspaceId: 'workspace', + spaceSummaries: { child: { parentSpaceId: 'parent' } }, + }, + navigate: vi.fn(), + toast: vi.fn(), + move: vi.fn(), + translate: (key: string) => key, + dialog: vi.fn< + (props: { + onDeleted: (ids: string[], failure?: string, completed?: boolean) => void; + }) => null + >(() => null), +})); +vi.mock('react-router-dom', () => ({ useNavigate: () => mocks.navigate })); +vi.mock('@/components/Common/Toast', () => ({ toast: mocks.toast })); +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: mocks.translate }), +})); +vi.mock('@/store/canvasStore', () => ({ + default: Object.assign( + (select: (state: typeof mocks.canvas) => unknown) => select(mocks.canvas), + { getState: () => mocks.canvas }, + ), +})); +vi.mock('@/store/workspaceStore', () => ({ + useWorkspaceStore: Object.assign( + (select: (state: typeof mocks.workspace) => unknown) => + select(mocks.workspace), + { getState: () => mocks.workspace }, + ), +})); +vi.mock('./moveNodesToSpace', () => ({ + moveNodesToSpace: mocks.move, +})); +vi.mock('./placeChildSpace', () => ({ placeChildSpace: vi.fn() })); +vi.mock('./SpaceOrganizationPopover', () => ({ + SpaceOrganizationPopover: () => null, +})); +vi.mock('./SpaceDeletionDialog', () => ({ SpaceDeletionDialog: mocks.dialog })); + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; +let host: HTMLDivElement; +let root: Root; +beforeEach(() => { + vi.clearAllMocks(); + host = document.createElement('div'); + document.body.append(host); + root = createRoot(host); + act(() => root.render()); + act(() => + window.dispatchEvent( + new CustomEvent(REQUEST_DELETE_SPACE, { detail: 'child' }), + ), + ); +}); +afterEach(() => { + act(() => root.unmount()); + host.remove(); +}); + +function complete(ids: string[], failure?: string, completed?: boolean) { + const props = mocks.dialog.mock.lastCall?.[0]; + if (!props) throw new Error('Missing deletion dialog'); + act(() => props.onDeleted(ids, failure, completed)); +} + +it('stays in the parent after deleting its child', () => { + complete(['child']); + expect(mocks.navigate).not.toHaveBeenCalled(); + expect(mocks.toast).not.toHaveBeenCalled(); +}); + +it('leaves partial deletion errors in the parent dialog', () => { + complete(['child'], 'Partial deletion'); + expect(mocks.navigate).not.toHaveBeenCalled(); + expect(mocks.toast).not.toHaveBeenCalled(); +}); + +it.each(['parent', 'unrelated'])( + 'refuses deletion outside the child entry: %s', + (id) => { + mocks.dialog.mockClear(); + act(() => + window.dispatchEvent( + new CustomEvent(REQUEST_DELETE_SPACE, { detail: id }), + ), + ); + expect(mocks.dialog).not.toHaveBeenCalled(); + expect(mocks.toast).toHaveBeenCalledWith( + 'spaceHierarchy.deleteFromContainer', + { tone: 'warning' }, + ); + }, +); + +it('reports refresh failure after successfully deleting a child', () => { + complete(['child'], 'Deleted, refresh failed', true); + expect(mocks.toast).toHaveBeenCalledExactlyOnceWith( + 'Deleted, refresh failed', + { tone: 'danger' }, + ); + expect(mocks.navigate).not.toHaveBeenCalled(); +}); diff --git a/apps/web/src/components/Space/SpaceActions.tsx b/apps/web/src/components/Space/SpaceActions.tsx index 2631e55d2..cff97436d 100644 --- a/apps/web/src/components/Space/SpaceActions.tsx +++ b/apps/web/src/components/Space/SpaceActions.tsx @@ -5,6 +5,7 @@ import { useEffect, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { useNavigate } from 'react-router-dom'; +import { toast } from '@/components/Common/Toast'; import useCanvasStore from '@/store/canvasStore'; import { useWorkspaceStore } from '@/store/workspaceStore'; import { @@ -70,7 +71,15 @@ export function SpaceActions() { }; const remove = (event: Event) => { const detail: unknown = (event as CustomEvent).detail; - if (typeof detail === 'string') setDeleting(detail); + const currentId = useCanvasStore.getState().canvasId; + if ( + typeof detail === 'string' && + detail !== currentId && + useWorkspaceStore.getState().spaceSummaries[detail]?.parentSpaceId === + currentId + ) + setDeleting(detail); + else toast(t('spaceHierarchy.deleteFromContainer'), { tone: 'warning' }); }; window.addEventListener(REQUEST_REPARENT_SPACE, move); window.addEventListener(REQUEST_DELETE_SPACE, remove); @@ -78,7 +87,7 @@ export function SpaceActions() { window.removeEventListener(REQUEST_REPARENT_SPACE, move); window.removeEventListener(REQUEST_DELETE_SPACE, remove); }; - }, []); + }, [t]); useEffect(() => { setAction(null); setDeleting(null); @@ -97,11 +106,11 @@ export function SpaceActions() { setDeleting(null)} - onDeleted={(ids, failure) => { - if (!failure) setDeleting(null); - if (ids.includes(useCanvasStore.getState().canvasId)) - navigate('/spaces'); + onDeleted={(_ids, failure, completed) => { + if (!failure || completed) setDeleting(null); + if (completed && failure) toast(failure, { tone: 'danger' }); }} /> )} diff --git a/apps/web/src/components/Space/SpaceDeletionDialog.test.tsx b/apps/web/src/components/Space/SpaceDeletionDialog.test.tsx index 237a0b410..ba9f91af4 100644 --- a/apps/web/src/components/Space/SpaceDeletionDialog.test.tsx +++ b/apps/web/src/components/Space/SpaceDeletionDialog.test.tsx @@ -6,24 +6,50 @@ import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; import { ApiError } from '@/api/_client'; +import { + CanvasConflictError, + NodeDuplicateFilesError, + putNodeContent, +} from '@/api/canvas'; import { deleteSpaceTree, getSpaceDeletionPreview } from '@/api/spaceHierarchy'; import { toast } from '@/components/Common/Toast'; +import { drainPendingSaves } from '@/store/canvasStore'; +import { createNodeContentQueue } from '@/store/canvasStore/save/nodeContentQueue'; +import { ConversationTitleSaveError } from '@/store/conversationTitleStore'; import { deleteWorkspace } from '@/store/previewWorkspace/persistence'; import { useWorkspaceStore } from '@/store/workspaceStore'; +import { pageLifecycle } from '@/utils/pageLifecycle'; +import { saveErrorsHandledByCaller } from '@/utils/saveErrorFeedback'; import { SpaceDeletionDialog } from './SpaceDeletionDialog'; +import type * as CanvasApi from '@/api/canvas'; +import type * as Electron from '@/hooks/useElectron'; + +const desktop = vi.hoisted(() => ({ enabled: false, command: vi.fn() })); +vi.mock('@/hooks/useElectron', async (importOriginal) => ({ + ...(await importOriginal()), + getElectronBridge: () => + desktop.enabled ? { tabs: { command: desktop.command } } : undefined, +})); vi.mock('@/api/spaceHierarchy', () => ({ deleteSpaceTree: vi.fn(), getSpaceDeletionPreview: vi.fn(), })); +vi.mock('@/api/canvas', async (importOriginal) => ({ + ...(await importOriginal()), + putNodeContent: vi.fn(), +})); vi.mock('@/store/canvasStore', () => ({ drainPendingSaves: vi.fn(async () => undefined), })); vi.mock('@/store/previewWorkspace/persistence', () => ({ deleteWorkspace: vi.fn(), })); -vi.mock('@/components/Common/Toast', () => ({ toast: vi.fn() })); +vi.mock('@/components/Common/Toast', () => ({ + toast: vi.fn(), + dismissToast: vi.fn(), +})); vi.mock('@/components/Common/Loading', () => ({ Loading: () => null })); vi.mock('react-i18next', () => ({ initReactI18next: { type: '3rdParty', init: vi.fn() }, @@ -34,12 +60,19 @@ const translate = (key: string, values?: Record) => globalThis.IS_REACT_ACT_ENVIRONMENT = true; let host: HTMLDivElement; let root: Root; +let unregisterSave: () => void; const onDeleted = vi.fn(); const refresh = vi.fn(async (): Promise => undefined); const originalRefresh = useWorkspaceStore.getState().refreshSpaceTitles; beforeEach(async () => { vi.clearAllMocks(); + unregisterSave = pageLifecycle.register('test-deletion-save', { + flush: () => drainPendingSaves(), + }); + vi.spyOn(console, 'error').mockImplementation(() => {}); + desktop.enabled = false; + desktop.command.mockReset().mockResolvedValue({ ok: true }); useWorkspaceStore.setState({ workspaceId: 'workspace', refreshSpaceTitles: refresh, @@ -47,8 +80,8 @@ beforeEach(async () => { vi.mocked(getSpaceDeletionPreview).mockResolvedValue({ token: 'token', spaces: [ - { canvasId: 'parent', title: 'Parent' }, - { canvasId: 'child', title: 'Child' }, + { canvasId: 'parent', title: 'Parent', parentSpaceId: 'grandparent' }, + { canvasId: 'child', title: 'Child', parentSpaceId: 'parent' }, ], }); host = document.createElement('div'); @@ -66,8 +99,11 @@ beforeEach(async () => { }); afterEach(() => { act(() => root.unmount()); + unregisterSave(); host.remove(); useWorkspaceStore.setState({ refreshSpaceTitles: originalRefresh }); + vi.restoreAllMocks(); + vi.useRealTimers(); }); async function remove() { @@ -97,7 +133,7 @@ it('names the target only in the title and summarizes descendant deletion', () = it('shows no descendant summary for an untitled Space without children', async () => { vi.mocked(getSpaceDeletionPreview).mockResolvedValueOnce({ token: 'leaf-token', - spaces: [{ canvasId: 'leaf', title: null }], + spaces: [{ canvasId: 'leaf', title: null, parentSpaceId: null }], }); await act(async () => root.render( @@ -137,15 +173,23 @@ it.each([['child'], ['child', 'parent']])( ), ); await remove(); - expect(onDeleted).toHaveBeenCalledWith(deleted, 'Cleanup failed'); + expect(onDeleted).toHaveBeenCalledWith( + deleted, + 'spaceHierarchy.deletionErrors.interrupted', + ); for (const id of deleted) expect(deleteWorkspace).toHaveBeenCalledWith(id); expect(refresh).toHaveBeenCalled(); - expect(toast).toHaveBeenCalledWith('Cleanup failed', { tone: 'danger' }); + expect(toast).not.toHaveBeenCalled(); expect(document.querySelector('[role="alert"]')?.textContent).toContain( - 'Cleanup failed', + 'spaceHierarchy.deletionErrors.interrupted', ); if (deleted.includes('parent')) - expect(document.body.textContent).not.toContain('spacePreview.retry'); + expect(document.body.textContent).not.toContain( + 'spaceHierarchy.recheckDeletion', + ); + expect(document.body.textContent).toContain( + `spaceHierarchy.deletionProgress {"count":${deleted.length}}`, + ); }, ); @@ -169,7 +213,7 @@ it('does not reconcile malformed or unconfirmed deletion IDs', async () => { expect(onDeleted).not.toHaveBeenCalled(); expect(deleteWorkspace).not.toHaveBeenCalled(); expect(document.querySelector('[role="alert"]')?.textContent).toContain( - 'Cleanup failed', + 'spaceHierarchy.deletionErrors.interrupted', ); }); @@ -199,5 +243,434 @@ it('notifies the parent after refresh when the Workspace is unchanged', async () deletedCanvasIds: ['parent'], }); await remove(); - expect(onDeleted).toHaveBeenCalledExactlyOnceWith(['parent']); + expect(onDeleted).toHaveBeenCalledExactlyOnceWith(['parent'], undefined); +}); + +it('waits for desktop preparation before deleting and reports confirmed IDs on completion', async () => { + desktop.enabled = true; + let prepared!: (result: { ok: true }) => void; + desktop.command.mockImplementationOnce( + () => + new Promise((resolve) => { + prepared = resolve; + }), + ); + vi.mocked(deleteSpaceTree).mockResolvedValue({ + deletedCanvasIds: ['child', 'parent'], + }); + await remove(); + expect(desktop.command).toHaveBeenCalledWith({ + type: 'begin-space-deletion', + canvasId: 'parent', + canvasIds: ['parent', 'child'], + }); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog?.querySelector('h3')?.textContent).toBe( + 'spaceHierarchy.deletingNamedTitle {"name":"Parent"}', + ); + expect(dialog?.querySelector('[role="status"]')?.textContent).toBe( + 'spaceHierarchy.deletionInProgress', + ); + expect(dialog?.textContent).not.toContain('spaceHierarchy.deleteWarning'); + const buttons = [...(dialog?.querySelectorAll('button') ?? [])]; + expect(buttons.map((button) => button.textContent)).toEqual([ + 'actions.cancel', + 'spaceHierarchy.deleting', + ]); + expect(buttons.every((button) => button.disabled)).toBe(true); + await act(async () => prepared({ ok: true })); + expect(deleteSpaceTree).toHaveBeenCalledExactlyOnceWith('parent', 'token'); + expect(desktop.command).toHaveBeenLastCalledWith({ + type: 'end-space-deletion', + deletedCanvasIds: ['child', 'parent'], + }); + expect(dialog?.textContent).not.toContain( + 'spaceHierarchy.deletionInProgress', + ); +}); + +it('reports partial deletion to desktop so only deleted Tabs close', async () => { + desktop.enabled = true; + vi.mocked(deleteSpaceTree).mockRejectedValue( + new ApiError( + 500, + { + code: 'SPACE_TREE_DELETE_FAILED', + message: 'Cleanup failed', + details: { + deletedCanvasIds: ['child'], + remainingCanvasIds: ['parent'], + cause: 'Failure', + }, + }, + 'Failure', + ), + ); + await remove(); + expect(desktop.command).toHaveBeenLastCalledWith({ + type: 'end-space-deletion', + deletedCanvasIds: ['child'], + }); +}); + +it('still closes deleted Tabs when refreshing Space titles fails', async () => { + desktop.enabled = true; + vi.mocked(deleteSpaceTree).mockResolvedValue({ + deletedCanvasIds: ['parent', 'child'], + }); + refresh.mockRejectedValueOnce(new Error('Refresh failed')); + await remove(); + expect(desktop.command).toHaveBeenLastCalledWith({ + type: 'end-space-deletion', + deletedCanvasIds: ['parent', 'child'], + }); + expect(onDeleted).toHaveBeenCalledExactlyOnceWith( + ['parent', 'child'], + 'spaceHierarchy.deletionErrors.deletedRefresh', + true, + ); + expect(document.querySelector('[role="alert"]')).toBeNull(); + expect(toast).not.toHaveBeenCalled(); +}); + +it('keeps preparation failures in the existing dialog without a toast', async () => { + desktop.enabled = true; + desktop.command.mockResolvedValueOnce({ + ok: false, + error: 'Could not save child', + }); + refresh.mockRejectedValueOnce(new Error('Refresh also failed')); + await remove(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain( + 'spaceHierarchy.deletionErrors.prepare', + ); + expect(document.querySelector('details')?.textContent).toContain( + 'Could not save child', + ); + expect(document.querySelector('details')?.textContent).not.toContain( + 'Refresh also failed', + ); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + expect(toast).not.toHaveBeenCalled(); +}); + +it('owns Note save failures from the initial local drain before desktop preparation', async () => { + desktop.enabled = true; + const node = { + id: 'note', + type: 'note', + position: { x: 0, y: 0 }, + data: { content: 'Unsaved note' }, + }; + const queue = createNodeContentQueue({ + delayMs: 500, + getState: () => ({ + canvasId: 'parent', + isLoading: false, + nodes: [node], + _setStateNoAutosave: vi.fn(), + patchNodeSilent: vi.fn(), + }), + }); + vi.mocked(putNodeContent).mockRejectedValue(new Error('Note save offline')); + vi.mocked(drainPendingSaves).mockImplementationOnce(async () => { + queue.scheduleChanges('parent', [], [node]); + await queue.flushAll(); + await queue.flushAllForSubmission('parent'); + }); + await remove(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain( + 'spaceHierarchy.deletionErrors.prepare', + ); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + expect(desktop.command).not.toHaveBeenCalled(); + expect(queue.hasUnsavedContent('parent')).toBe(true); + expect(toast).not.toHaveBeenCalled(); +}); + +it('ends a stalled local drain after 15 seconds without waiting for refresh or deleting on late completion', async () => { + vi.useFakeTimers(); + desktop.enabled = true; + let finishSave!: () => void; + vi.mocked(drainPendingSaves).mockImplementationOnce( + () => + new Promise((resolve) => { + finishSave = resolve; + }), + ); + await remove(); + expect(saveErrorsHandledByCaller()).toBe(true); + await act(async () => vi.advanceTimersByTimeAsync(14_999)); + expect(document.querySelector('[role="status"]')).not.toBeNull(); + await act(async () => vi.advanceTimersByTimeAsync(1)); + expect(document.querySelector('[role="status"]')).toBeNull(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain( + 'spaceHierarchy.deletionErrors.prepare', + ); + expect(document.querySelector('details')?.textContent).toContain( + 'spaceHierarchy.deletionErrors.prepareTimeout', + ); + expect( + [...document.querySelectorAll('button')].every( + (button) => !button.disabled, + ), + ).toBe(true); + expect(refresh).not.toHaveBeenCalled(); + expect(saveErrorsHandledByCaller()).toBe(false); + await act(async () => finishSave()); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + expect(desktop.command).not.toHaveBeenCalled(); + expect(toast).not.toHaveBeenCalled(); +}); + +it('uses the shared preparation to settle editors and await imports before saving', async () => { + const settle = vi.fn(); + const unregisterEditor = pageLifecycle.register('test-editor', { settle }); + let completeImport!: () => void; + const importing = pageLifecycle.run( + 'importing', + () => + new Promise((resolve) => { + completeImport = resolve; + }), + ); + vi.mocked(deleteSpaceTree).mockResolvedValue({ + deletedCanvasIds: ['parent', 'child'], + }); + try { + await remove(); + expect(settle).toHaveBeenCalledOnce(); + expect(drainPendingSaves).not.toHaveBeenCalled(); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + await act(async () => completeImport()); + await importing; + expect(drainPendingSaves).toHaveBeenCalledOnce(); + expect(deleteSpaceTree).toHaveBeenCalledOnce(); + } finally { + unregisterEditor(); + } +}); + +it('unmounting during preparation releases feedback and does not delete after saving finishes', async () => { + let completeSave!: () => void; + vi.mocked(drainPendingSaves).mockImplementationOnce( + () => + new Promise((resolve) => { + completeSave = resolve; + }), + ); + await remove(); + expect(saveErrorsHandledByCaller()).toBe(true); + await act(async () => root.render(null)); + expect(saveErrorsHandledByCaller()).toBe(false); + await act(async () => completeSave()); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + expect(desktop.command).not.toHaveBeenCalled(); +}); + +it('retries a retained title failure through the same local preparation', async () => { + const error = new ConversationTitleSaveError('Title failed'); + const retry = vi.spyOn(error, 'retry').mockResolvedValue(); + vi.mocked(drainPendingSaves).mockRejectedValueOnce(error); + vi.mocked(deleteSpaceTree).mockResolvedValue({ + deletedCanvasIds: ['parent', 'child'], + }); + await remove(); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + const recheck = [...document.querySelectorAll('button')].find((button) => + button.textContent?.includes('spaceHierarchy.recheckDeletion'), + ); + if (!recheck) throw new Error('Missing recheck action'); + await act(async () => recheck.click()); + await remove(); + expect(retry).toHaveBeenCalledOnce(); + expect(deleteSpaceTree).toHaveBeenCalledOnce(); + expect(toast).not.toHaveBeenCalled(); +}); + +it.each([ + new CanvasConflictError({ + code: 'NODE_CONTENT_CONFLICT', + message: 'changed elsewhere', + currentRev: 'server-rev', + }), + new NodeDuplicateFilesError({ + message: 'duplicate files', + duplicateFiles: ['a.md', 'b.md'], + }), +])( + 'defers Note recovery until the deletion dialog closes: %s', + async (error) => { + const node = { + id: 'note', + type: 'note', + position: { x: 0, y: 0 }, + data: { content: 'Unsaved note' }, + }; + const patchNodeSilent = vi.fn(); + const queue = createNodeContentQueue({ + delayMs: 500, + getState: () => ({ + canvasId: 'parent', + isLoading: false, + nodes: [node], + _setStateNoAutosave: vi.fn(), + patchNodeSilent, + }), + }); + vi.mocked(putNodeContent).mockRejectedValue(error); + vi.mocked(drainPendingSaves).mockImplementationOnce(async () => { + queue.scheduleChanges('parent', [], [node]); + await queue.flushAllForSubmission('parent'); + }); + await remove(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain( + 'spaceHierarchy.deletionErrors.prepare', + ); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + expect(queue.hasUnsavedContent('parent')).toBe(true); + expect(node.data.content).toBe('Unsaved note'); + expect(toast).not.toHaveBeenCalled(); + if (error instanceof NodeDuplicateFilesError) { + expect(patchNodeSilent).toHaveBeenCalledWith('note', { + contentDuplicate: true, + duplicateFiles: ['a.md', 'b.md'], + }); + } else { + await queue.flushNow('parent', 'note'); + expect(putNodeContent).toHaveBeenCalledOnce(); + } + await act(async () => root.render(null)); + expect(toast).toHaveBeenCalledOnce(); + if (error instanceof CanvasConflictError) { + const options = vi.mocked(toast).mock.calls[0][1]; + expect(options?.action?.onClick).toBeTypeOf('function'); + vi.mocked(putNodeContent).mockResolvedValue({ + nodeId: 'note', + label: null, + rev: 'saved', + }); + options?.secondaryAction?.onClick(); + await vi.waitFor(() => { + expect(queue.hasUnsavedContent('parent')).toBe(false); + }); + expect(vi.mocked(putNodeContent).mock.calls[1][2]).toMatchObject({ + content: 'Unsaved note', + expectRev: 'server-rev', + }); + } + queue.discardCanvas('parent'); + }, +); + +it.each(['parent', 'unrelated'])( + 'refuses deletion when the live preview is not a child of its container: %s', + async (containerSpaceId) => { + await act(async () => + root.render( + , + ), + ); + await remove(); + expect(deleteSpaceTree).not.toHaveBeenCalled(); + expect(drainPendingSaves).not.toHaveBeenCalled(); + expect(document.querySelector('details')?.textContent).toContain( + 'spaceHierarchy.deleteFromContainer', + ); + }, +); + +it('reports release errors inline and retains the dialog after deletion', async () => { + desktop.enabled = true; + desktop.command.mockResolvedValueOnce({ ok: true }).mockResolvedValueOnce({ + ok: false, + error: 'Could not save Tab session', + }); + vi.mocked(deleteSpaceTree).mockResolvedValue({ + deletedCanvasIds: ['parent', 'child'], + }); + const log = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await remove(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain( + 'spaceHierarchy.deletionErrors.release', + ); + expect(onDeleted).toHaveBeenCalledWith( + ['parent', 'child'], + 'spaceHierarchy.deletionErrors.release', + ); + expect(toast).not.toHaveBeenCalled(); + } finally { + log.mockRestore(); + } +}); + +it('retains the target and groups recovery actions without a disabled delete button', async () => { + vi.mocked(deleteSpaceTree).mockRejectedValueOnce( + new ApiError( + 409, + { code: 'SPACE_AGENT_RUNNING', message: 'Stop running Agents' }, + 'Failure', + ), + ); + await remove(); + const dialog = document.querySelector('[role="dialog"]'); + expect(dialog?.querySelector('h3')?.textContent).toBe( + 'spaceHierarchy.deleteFailedNamedTitle {"name":"Parent"}', + ); + expect(dialog?.querySelector('[role="alert"]')?.textContent).toBe( + 'spaceHierarchy.deletionErrors.agentRunning', + ); + expect(dialog?.textContent).not.toContain('Stop running Agents'); + expect(dialog?.textContent).not.toContain('spaceHierarchy.deleteWarning'); + expect(dialog?.querySelector('details')).toBeNull(); + const buttons = [...(dialog?.querySelectorAll('button') ?? [])]; + expect(buttons.map((button) => button.textContent)).toEqual([ + 'actions.cancel', + 'spaceHierarchy.recheckDeletion', + ]); + expect(buttons[0].parentElement).toBe(buttons[1].parentElement); + expect(toast).not.toHaveBeenCalled(); + + vi.mocked(getSpaceDeletionPreview).mockResolvedValueOnce({ + token: 'updated-token', + spaces: [{ canvasId: 'parent', title: 'Parent', parentSpaceId: null }], + }); + await act(async () => buttons[1].click()); + expect(deleteSpaceTree).toHaveBeenCalledTimes(1); + expect(dialog?.textContent).not.toContain('spaceHierarchy.deleteFailed'); + expect(document.activeElement).toBe(buttons[0]); + vi.mocked(deleteSpaceTree).mockResolvedValueOnce({ + deletedCanvasIds: ['parent'], + }); + await remove(); + expect(deleteSpaceTree).toHaveBeenLastCalledWith('parent', 'updated-token'); +}); + +it('does not reuse the previous preview when rechecking fails', async () => { + vi.mocked(deleteSpaceTree).mockRejectedValueOnce(new Error('Network failed')); + await remove(); + vi.mocked(getSpaceDeletionPreview).mockRejectedValueOnce( + new Error('Preview unavailable'), + ); + const retry = [...document.querySelectorAll('button')].find( + (button) => button.textContent === 'spaceHierarchy.recheckDeletion', + ); + expect(retry).toBeDefined(); + await act(async () => retry?.click()); + expect(document.querySelector('h3')?.textContent).toContain('Parent'); + expect(document.querySelector('[role="alert"]')?.textContent).toBe( + 'spaceHierarchy.deletionErrors.preview', + ); + expect(document.querySelector('details')?.textContent).toContain( + 'Preview unavailable', + ); + expect(document.body.textContent).not.toContain('spaceHierarchy.deleteCount'); + expect(deleteSpaceTree).toHaveBeenCalledTimes(1); }); diff --git a/apps/web/src/components/Space/SpaceDeletionDialog.tsx b/apps/web/src/components/Space/SpaceDeletionDialog.tsx index 40b54ebc6..8a8cc3ac6 100644 --- a/apps/web/src/components/Space/SpaceDeletionDialog.tsx +++ b/apps/web/src/components/Space/SpaceDeletionDialog.tsx @@ -9,19 +9,40 @@ import { deleteSpaceTree, getSpaceDeletionPreview } from '@/api/spaceHierarchy'; import { Button } from '@/components/Common/Button'; import { Loading } from '@/components/Common/Loading'; import { Modal } from '@/components/Common/Modal'; -import { toast } from '@/components/Common/Toast'; +import { Spinner } from '@/components/Common/Spinner'; import { getElectronBridge, requireDesktopTabSuccess, } from '@/hooks/useElectron'; -import { drainPendingSaves } from '@/store/canvasStore'; import { deleteWorkspace } from '@/store/previewWorkspace/persistence'; import { useWorkspaceStore } from '@/store/workspaceStore'; +import { prepareSpaceDeletion } from '@/utils/prepareSpaceDeletion'; +import { holdSaveErrorRecovery } from '@/utils/saveErrorFeedback'; import type { DeleteSpaceTreeFailure, DeleteSpaceTreePreview, } from '@huabu/shared'; +import type { ParseKeys, TFunction } from 'i18next'; + +const deletionErrorKeys: Partial> = { + SPACE_AGENT_RUNNING: 'spaceHierarchy.deletionErrors.agentRunning', + SPACE_TREE_CHANGED: 'spaceHierarchy.deletionErrors.treeChanged', + SPACE_TREE_DELETE_FAILED: 'spaceHierarchy.deletionErrors.interrupted', + SPACE_NOT_FOUND: 'spaceHierarchy.deletionErrors.notFound', +}; + +function describeDeletionError(cause: unknown, fallback: string, t: TFunction) { + console.error('[space-deletion]', cause); + const key = + cause instanceof ApiError && cause.code + ? deletionErrorKeys[cause.code] + : undefined; + return { + message: key ? t(key) : fallback, + details: !key && cause instanceof Error ? cause.message : undefined, + }; +} function isDeletionFailure(value: unknown): value is DeleteSpaceTreeFailure { if (!value || typeof value !== 'object') return false; @@ -39,15 +60,20 @@ function isDeletionFailure(value: unknown): value is DeleteSpaceTreeFailure { export function SpaceDeletionDialog({ canvasId, + containerSpaceId, onClose, onDeleted, }: { canvasId: string; + containerSpaceId?: string; onClose: () => void; - onDeleted: (ids: string[], failure?: string) => void; + onDeleted: (ids: string[], failure?: string, completed?: boolean) => void; }) { const { t } = useTranslation(); + useEffect(holdSaveErrorRecovery, []); const cancelRef = useRef(null); + const preparationRef = useRef(null); + useEffect(() => () => preparationRef.current?.abort(), []); const liveWorkspaceId = useWorkspaceStore((state) => state.workspaceId); const [workspaceId] = useState(liveWorkspaceId); useEffect(() => { @@ -55,25 +81,40 @@ export function SpaceDeletionDialog({ }, [liveWorkspaceId, workspaceId, onClose]); const [preview, setPreview] = useState(null); const [error, setError] = useState(null); + const [errorDetails, setErrorDetails] = useState(null); + const [checking, setChecking] = useState(true); const [busy, setBusy] = useState(false); const [attempt, setAttempt] = useState(0); const [rootDeleted, setRootDeleted] = useState(false); + const [deletedCount, setDeletedCount] = useState(0); const root = preview?.spaces.find((item) => item.canvasId === canvasId); const descendantCount = preview?.spaces.filter((item) => item.canvasId !== canvasId).length ?? 0; useEffect(() => { let active = true; - setPreview(null); + setChecking(true); setError(null); + setErrorDetails(null); void getSpaceDeletionPreview(canvasId) .then((value) => { - if (active) setPreview(value); + if (active) { + setPreview(value); + cancelRef.current?.focus(); + } }) .catch((cause: unknown) => { - if (active) - setError( - cause instanceof Error ? cause.message : t('spaceHierarchy.failed'), + if (active) { + const failure = describeDeletionError( + cause, + t('spaceHierarchy.deletionErrors.preview'), + t, ); + setError(failure.message); + setErrorDetails(failure.details ?? null); + } + }) + .finally(() => { + if (active) setChecking(false); }); return () => { active = false; @@ -81,27 +122,67 @@ export function SpaceDeletionDialog({ }, [canvasId, attempt, t]); const remove = async () => { - if (!preview) return; + if (!preview || !root || checking || busy || error) return; setBusy(true); const tabs = getElectronBridge()?.tabs; let deletionPrepared = false; + let deletedCanvasIds: string[] = []; + let failure: string | undefined; + let refreshFailure: string | undefined; + let details: string | undefined; + let deletionRequested = false; + const reportFailure = (cause: unknown, fallback: string) => { + const { message, details: nextDetails } = describeDeletionError( + cause, + fallback, + t, + ); + failure = + failure && failure !== message ? `${failure}\n${message}` : message; + if (nextDetails) + details = + details && details !== nextDetails + ? `${details}\n${nextDetails}` + : nextDetails; + }; + const reportRefreshFailure = (cause: unknown) => { + console.error( + '[space-deletion] Could not refresh Space metadata:', + cause, + ); + if (!failure && deletedCanvasIds.includes(canvasId)) + refreshFailure = t('spaceHierarchy.deletionErrors.deletedRefresh'); + else if (!failure) + reportFailure(cause, t('spaceHierarchy.deletionErrors.refresh')); + }; try { - await drainPendingSaves(); + if ( + containerSpaceId && + (root.parentSpaceId !== containerSpaceId || + preview.spaces.some((space) => space.canvasId === containerSpaceId)) + ) + throw new Error(t('spaceHierarchy.deleteFromContainer')); + const controller = new AbortController(); + preparationRef.current = controller; + const timer = setTimeout( + () => + controller.abort( + new Error(t('spaceHierarchy.deletionErrors.prepareTimeout')), + ), + 15_000, + ); + try { + await prepareSpaceDeletion(controller.signal); + controller.signal.throwIfAborted(); + } finally { + clearTimeout(timer); + if (preparationRef.current === controller) + preparationRef.current = null; + } if (useWorkspaceStore.getState().workspaceId !== workspaceId) throw new Error(t('spaceHierarchy.workspaceChanged')); if (tabs) { const canvasIds = preview.spaces.map((space) => space.canvasId); - const state = await tabs.getState(); - if ( - state.tabs.some( - (tab) => tab.canvasId && canvasIds.includes(tab.canvasId), - ) - ) { - const message = t('desktopTabs.closeBeforeDeleting'); - setError(message); - toast(message, { tone: 'warning' }); - return; - } requireDesktopTabSuccess( await tabs.command({ type: 'begin-space-deletion', @@ -111,21 +192,16 @@ export function SpaceDeletionDialog({ ); deletionPrepared = true; } + deletionRequested = true; const result = await deleteSpaceTree(canvasId, preview.token); - if (useWorkspaceStore.getState().workspaceId !== workspaceId) return; - for (const id of result.deletedCanvasIds) deleteWorkspace(id); - await useWorkspaceStore - .getState() - .refreshSpaceTitles(result.deletedCanvasIds); - if (useWorkspaceStore.getState().workspaceId !== workspaceId) return; - onDeleted(result.deletedCanvasIds); + deletedCanvasIds = result.deletedCanvasIds; } catch (cause) { - if (useWorkspaceStore.getState().workspaceId !== workspaceId) return; - const message = - cause instanceof Error ? cause.message : t('spaceHierarchy.failed'); - setError(message); - if (tabs) toast(message, { tone: 'danger' }); - setPreview(null); + reportFailure( + cause, + deletionRequested + ? t('spaceHierarchy.deletionErrors.request') + : t('spaceHierarchy.deletionErrors.prepare'), + ); const confirmed = new Set(preview.spaces.map((item) => item.canvasId)); if ( cause instanceof ApiError && @@ -136,28 +212,56 @@ export function SpaceDeletionDialog({ ...cause.details.remainingCanvasIds, ].every((id) => confirmed.has(id)) ) { - const deleted = cause.details.deletedCanvasIds; - for (const id of deleted) deleteWorkspace(id); - setRootDeleted(deleted.includes(canvasId)); - toast(message, { tone: 'danger' }); - onDeleted(deleted, message); + deletedCanvasIds = cause.details.deletedCanvasIds; } - await useWorkspaceStore.getState().refreshSpaceTitles(); } finally { - if (tabs && deletionPrepared) { + if ( + deletionRequested && + useWorkspaceStore.getState().workspaceId === workspaceId + ) { try { - requireDesktopTabSuccess(await tabs.command('end-space-deletion')); + for (const id of deletedCanvasIds) deleteWorkspace(id); + await useWorkspaceStore + .getState() + .refreshSpaceTitles(deletedCanvasIds); + const current = useWorkspaceStore.getState(); + if ( + current.workspaceId === workspaceId && + current.spaceTitlesStatus === 'error' + ) + reportRefreshFailure( + new Error(current.spaceTitlesError || t('spaceHierarchy.failed')), + ); } catch (cause) { - console.error( - '[desktop-tabs] Could not release Space deletion:', - cause, + reportRefreshFailure(cause); + } + } + if (tabs && deletionPrepared) { + try { + requireDesktopTabSuccess( + await tabs.command({ + type: 'end-space-deletion', + deletedCanvasIds, + }), ); - toast(cause instanceof Error ? cause.message : String(cause), { - tone: 'danger', - }); + } catch (cause) { + reportFailure(cause, t('spaceHierarchy.deletionErrors.release')); + } + } + if (useWorkspaceStore.getState().workspaceId === workspaceId) { + setBusy(false); + setRootDeleted(deletedCanvasIds.includes(canvasId)); + setDeletedCount(deletedCanvasIds.length); + if (failure) { + setError(failure); + setErrorDetails(details ?? null); + } + if (deletedCanvasIds.length > 0) { + if (refreshFailure && !failure) + onDeleted(deletedCanvasIds, refreshFailure, true); + else onDeleted(deletedCanvasIds, failure); } } - setBusy(false); } }; return ( @@ -165,13 +269,26 @@ export function SpaceDeletionDialog({ isOpen title={ root - ? t('spaceHierarchy.deleteNamedTitle', { - name: root.title || t('canvasList.untitled'), - }) - : t('canvasList.deleteTitle') + ? t( + rootDeleted + ? 'spaceHierarchy.deletedNamedTitle' + : error + ? 'spaceHierarchy.deleteFailedNamedTitle' + : busy + ? 'spaceHierarchy.deletingNamedTitle' + : 'spaceHierarchy.deleteNamedTitle', + { + name: root.title || t('canvasList.untitled'), + }, + ) + : error + ? t('spaceHierarchy.deleteFailedTitle') + : t('canvasList.deleteTitle') } className="[overflow-wrap:anywhere]" - description={t('spaceHierarchy.deleteWarning')} + description={ + error || busy ? undefined : t('spaceHierarchy.deleteWarning') + } initialFocusRef={cancelRef} onClose={() => { if (!busy) onClose(); @@ -186,24 +303,44 @@ export function SpaceDeletionDialog({ onClick={onClose} disabled={busy} > - {t('actions.cancel')} - - + {error ? ( + !rootDeleted && ( + + ) + ) : ( + + )} } > - {preview ? ( + {busy ? ( +

+ {t('spaceHierarchy.deletionInProgress')} +

+ ) : !error && !checking && preview ? ( descendantCount > 0 ? (

{t('spaceHierarchy.deleteDescendants', { @@ -220,19 +357,23 @@ export function SpaceDeletionDialog({ /> ) : null} {error && ( -

-

{error}

- {!rootDeleted && ( - +
+

{error}

+ {deletedCount > 0 && ( +

+ {t('spaceHierarchy.deletionProgress', { count: deletedCount })} +

)}
)} + {errorDetails && ( +
+ + {t('spaceHierarchy.deletionDetails')} + +

{errorDetails}

+
+ )} ); } diff --git a/apps/web/src/hooks/useDesktopContent.preferences.test.tsx b/apps/web/src/hooks/useDesktopContent.preferences.test.tsx index 58804a9b9..3a12b2fbc 100644 --- a/apps/web/src/hooks/useDesktopContent.preferences.test.tsx +++ b/apps/web/src/hooks/useDesktopContent.preferences.test.tsx @@ -101,6 +101,7 @@ beforeEach(async () => { endWorkspaceChange: vi.fn(), onNavigate: vi.fn(() => vi.fn()), onPrepareClose: vi.fn(() => vi.fn()), + onSpacesDeleted: vi.fn(() => vi.fn()), finishPrepareClose: vi.fn(), keepPrepareCloseAlive: vi.fn(), onClosePrepared: vi.fn(() => vi.fn()), diff --git a/apps/web/src/hooks/useDesktopContent.test.tsx b/apps/web/src/hooks/useDesktopContent.test.tsx index b8fa55f51..79e98d817 100644 --- a/apps/web/src/hooks/useDesktopContent.test.tsx +++ b/apps/web/src/hooks/useDesktopContent.test.tsx @@ -5,12 +5,15 @@ import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { putNodeContent } from '@/api/canvas'; import { queryConversationTitles, setConversationTitle, } from '@/api/conversationTitles'; import useCanvasStore from '@/store/canvasStore'; +import { createNodeContentQueue } from '@/store/canvasStore/save/nodeContentQueue'; import { + ConversationTitleSaveError, renameConversationTitle, refreshConversationTitles, useConversationTitleStore, @@ -21,12 +24,20 @@ import { isDesktopInputFrozen, } from '@/utils/desktopInput'; import { pageLifecycle } from '@/utils/pageLifecycle'; +import { + saveErrorsHandledByCaller, + showSaveErrorRecovery, +} from '@/utils/saveErrorFeedback'; import { useDesktopContent } from './useDesktopContent'; import { getElectronBridge } from './useElectron'; +import type * as CanvasApi from '@/api/canvas'; import type { toast } from '@/components/Common/Toast'; -import type { DesktopTabsSnapshot } from '@huabu/shared/desktop-tabs'; +import type { + DesktopTabsApi, + DesktopTabsSnapshot, +} from '@huabu/shared/desktop-tabs'; const mocks = vi.hoisted(() => ({ context: { role: 'space', canvasId: 'space-a', workspaceId: 'workspace' }, @@ -42,7 +53,7 @@ const mocks = vi.hoisted(() => ({ save: vi.fn(), preprocess: vi.fn(), flush: vi.fn(), - prepare: vi.fn<(listener: (requestId: string) => void) => () => void>(), + prepare: vi.fn(), resume: vi.fn<(listener: () => void) => () => void>(), finish: vi.fn(), heartbeat: vi.fn(), @@ -59,6 +70,10 @@ vi.mock('@/api/conversationTitles', () => ({ setConversationTitle: vi.fn(), queryConversationTitles: vi.fn(), })); +vi.mock('@/api/canvas', async (importOriginal) => ({ + ...(await importOriginal()), + putNodeContent: vi.fn(), +})); vi.mock('./useElectron', () => { const bridge = { platform: 'darwin', @@ -82,7 +97,10 @@ vi.mock('./useElectron', () => { requireDesktopTabSuccess: vi.fn(), }; }); -vi.mock('@/components/Common/Toast', () => ({ toast: mocks.toast })); +vi.mock('@/components/Common/Toast', () => ({ + toast: mocks.toast, + dismissToast: vi.fn(), +})); vi.mock('@/store/canvasStore', () => ({ default: { getState: () => mocks.canvas, @@ -317,6 +335,156 @@ describe('desktop save presentation', () => { }, ); + it('returns deletion preparation failures to the dialog without a toast', async () => { + mocks.save.mockRejectedValue(new Error('Disk full')); + const log = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await render(); + await act(async () => + mocks.prepare.mock.calls[0][0]({ + requestId: 'delete-request', + reason: 'space-deletion', + }), + ); + expect(mocks.finish).toHaveBeenCalledWith('delete-request', { + ok: false, + error: 'Disk full', + }); + expect(mocks.toast).not.toHaveBeenCalled(); + } finally { + log.mockRestore(); + } + }); + + it('suppresses the real Note queue toast during deletion preparation only', async () => { + const node = { + id: 'note', + type: 'note', + position: { x: 0, y: 0 }, + data: { content: 'Unsaved note' }, + }; + const queue = createNodeContentQueue({ + delayMs: 500, + getState: () => ({ + canvasId: 'space-a', + isLoading: false, + nodes: [node], + _setStateNoAutosave: vi.fn(), + patchNodeSilent: vi.fn(), + }), + }); + vi.mocked(putNodeContent).mockRejectedValue(new Error('Note save offline')); + mocks.save.mockImplementation(async () => { + queue.scheduleChanges('space-a', [], [node]); + await queue.flushAll(); + await queue.flushAllForSubmission('space-a'); + }); + await render(); + await act(async () => + mocks.prepare.mock.calls[0][0]({ + requestId: 'delete-note', + reason: 'space-deletion', + }), + ); + expect(mocks.finish).toHaveBeenCalledWith('delete-note', { + ok: false, + error: expect.stringContaining('Content saves failed'), + }); + expect(queue.hasUnsavedContent('space-a')).toBe(true); + expect(mocks.toast).not.toHaveBeenCalled(); + await resume(); + await prepare(); + expect(mocks.toast).toHaveBeenCalled(); + }); + + it('retries title-save failures on the next deletion attempt without needing a toast action', async () => { + const error = new ConversationTitleSaveError('Title save failed'); + const retry = vi.spyOn(error, 'retry').mockResolvedValue(); + mocks.save.mockRejectedValueOnce(error); + const log = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await render(); + await act(async () => + mocks.prepare.mock.calls[0][0]({ + requestId: 'delete-first', + reason: 'space-deletion', + }), + ); + await resume(); + await act(async () => + mocks.prepare.mock.calls[0][0]({ + requestId: 'delete-retry', + reason: 'space-deletion', + }), + ); + expect(retry).toHaveBeenCalledOnce(); + expect(mocks.finish).toHaveBeenLastCalledWith('delete-retry', { + ok: true, + }); + expect(mocks.toast).not.toHaveBeenCalled(); + } finally { + log.mockRestore(); + } + }); + + it('releases feedback on resume during title retry and fences late completion from a replacement', async () => { + const error = new ConversationTitleSaveError('Title save failed'); + let completeOld!: () => void; + let completeNew!: () => void; + const retry = vi + .spyOn(error, 'retry') + .mockImplementationOnce( + () => + new Promise((resolve) => { + completeOld = resolve; + }), + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + completeNew = resolve; + }), + ); + mocks.save.mockRejectedValueOnce(error); + try { + await render(); + const start = (requestId: string) => + act(async () => + mocks.prepare.mock.calls[0][0]({ + requestId, + reason: 'space-deletion', + }), + ); + await start('first-failure'); + await resume(); + mocks.save.mockClear(); + mocks.finish.mockClear(); + await start('stalled-retry'); + expect(saveErrorsHandledByCaller()).toBe(true); + const recovery = vi.fn(); + showSaveErrorRecovery(recovery); + expect(recovery).not.toHaveBeenCalled(); + await resume(); + expect(isDesktopInputFrozen()).toBe(false); + expect(saveErrorsHandledByCaller()).toBe(false); + expect(recovery).toHaveBeenCalledOnce(); + await start('replacement'); + await act(async () => completeOld()); + expect(mocks.save).not.toHaveBeenCalled(); + expect(mocks.finish).not.toHaveBeenCalled(); + expect(saveErrorsHandledByCaller()).toBe(true); + expect(isDesktopInputFrozen()).toBe(true); + await act(async () => completeNew()); + expect(retry).toHaveBeenCalledTimes(2); + expect(mocks.finish).toHaveBeenCalledExactlyOnceWith('replacement', { + ok: true, + }); + expect(saveErrorsHandledByCaller()).toBe(false); + } finally { + retry.mockRestore(); + } + }); + it('hydrates queued World preference notifications without echoing writes to main', async () => { await render(); mocks.setWorkspaceState.mockClear(); diff --git a/apps/web/src/hooks/useDesktopContent.ts b/apps/web/src/hooks/useDesktopContent.ts index 5b3bb2e81..1b5c6a672 100644 --- a/apps/web/src/hooks/useDesktopContent.ts +++ b/apps/web/src/hooks/useDesktopContent.ts @@ -26,6 +26,7 @@ import { subscribeApplicationPreferences, } from '@/utils/desktopStorage'; import { pageLifecycle } from '@/utils/pageLifecycle'; +import { prepareSpaceDeletion } from '@/utils/prepareSpaceDeletion'; import { getElectronBridge, requireDesktopTabSuccess } from './useElectron'; @@ -127,7 +128,7 @@ export function useDesktopContent(): { useEffect(() => { if (!tabs || tabs.context.role === 'shell') return; - const reportSaveError = (error: unknown): string => { + const reportSaveError = (error: unknown, notify = true): string => { const message = error instanceof Error ? error.cause instanceof Error @@ -135,25 +136,31 @@ export function useDesktopContent(): { : error.message : String(error); console.error('[desktop-tabs] Could not save Space:', error); - toast(message, { - tone: 'danger', - ...(error instanceof ConversationTitleSaveError - ? { - action: { - label: i18n.t('chat.retryTitleSave'), - onClick: () => { - void error.retry().catch(reportSaveError); + if (notify) + toast(message, { + tone: 'danger', + ...(error instanceof ConversationTitleSaveError + ? { + action: { + label: i18n.t('chat.retryTitleSave'), + onClick: () => { + void error.retry().catch(reportSaveError); + }, }, - }, - } - : {}), - }); + } + : {}), + }); return message; }; const unsubscribeNavigate = tabs.onNavigate((target) => { navigate(desktopNavigationPath(target), { state: target.state }); }); - const unsubscribePrepare = tabs.onPrepareClose((requestId) => { + const unsubscribePrepare = tabs.onPrepareClose((preparation) => { + const requestId = + typeof preparation === 'string' ? preparation : preparation.requestId; + const deleting = + typeof preparation !== 'string' && + preparation.reason === 'space-deletion'; closeRequest.current?.controller.abort(); const request: NonNullable = { id: requestId, @@ -173,7 +180,8 @@ export function useDesktopContent(): { }, 5_000); void (async () => { try { - await pageLifecycle.prepare(request.controller.signal); + if (deleting) await prepareSpaceDeletion(request.controller.signal); + else await pageLifecycle.prepare(request.controller.signal); if (closeRequest.current === request) { tabs.finishPrepareClose(requestId, { ok: true }); } @@ -183,7 +191,7 @@ export function useDesktopContent(): { request.controller.signal.aborted ) return; - const message = reportSaveError(error); + const message = reportSaveError(error, !deleting); window.clearTimeout(request.overlayTimer); setSaving(false); tabs.finishPrepareClose(requestId, { ok: false, error: message }); diff --git a/apps/web/src/i18n/resources/en/common.json b/apps/web/src/i18n/resources/en/common.json index 7a324c841..5250db2e4 100644 --- a/apps/web/src/i18n/resources/en/common.json +++ b/apps/web/src/i18n/resources/en/common.json @@ -43,8 +43,7 @@ "label": "Open Spaces", "newTab": "New tab", "noMatchingSpaces": "No matching Spaces.", - "closeTab": "Close Tab", - "closeBeforeDeleting": "This Space is open. Close its Tab before deleting it." + "closeTab": "Close Tab" }, "sigil": { "chooseResource": "Choose a resource", @@ -728,6 +727,7 @@ "activationInProgress": "Another Home folder is still opening. Please wait for it to finish." }, "spaceHierarchy": { + "deleteFromContainer": "Delete this Space from Home or its parent Space.", "bring": "Move existing Space here", "alreadyChild": "Already in this Space", "collectAction": "Collect into child Space", @@ -753,6 +753,29 @@ "entryProtected": "This change would remove or retarget a child Space entry. Use Delete Space or Move into another Space instead.", "deleteNamedTitle": "Delete “{{name}}”?", "deleteWarning": "All contents will be permanently deleted. This cannot be undone.", + "deletingNamedTitle": "Deleting “{{name}}”", + "deleting": "Deleting…", + "deletionInProgress": "Please wait while changes are saved and deletion is processed.", + "deleteFailedTitle": "Deletion could not be completed", + "deleteFailedNamedTitle": "Could not finish deleting “{{name}}”", + "deletedNamedTitle": "“{{name}}” was deleted", + "recheckDeletion": "Review again", + "deletionDetails": "Technical details", + "deletionProgress_one": "{{count}} Space has already been deleted and cannot be restored.", + "deletionProgress_other": "{{count}} Spaces have already been deleted and cannot be restored.", + "deletionErrors": { + "deletedRefresh": "The Space was deleted, but the Space list could not be refreshed. Refresh the list to check its current state.", + "agentRunning": "An Agent in this Space or a nested Space is still busy. Stop it or wait for it to finish, then review again.", + "treeChanged": "The Space hierarchy changed. Review the current deletion scope before confirming again.", + "interrupted": "Deletion did not finish cleanly. Check the current Space list before continuing.", + "notFound": "This Space is no longer available. Close this dialog and refresh the Space list.", + "preview": "The deletion scope could not be loaded. Check your connection and review again.", + "prepare": "Changes could not be saved or deletion could not be prepared. No deletion request was sent. Review again after resolving the issue.", + "prepareTimeout": "Saving took too long. Nothing was deleted and your unsaved changes were kept. Close this dialog and try again after saving finishes.", + "request": "The deletion result could not be confirmed. Check your connection and review the current Spaces before trying again.", + "refresh": "The Space list could not be refreshed. Close this dialog and refresh the list to check the current state.", + "release": "Tab cleanup could not be completed. Close this dialog and check the remaining open Spaces." + }, "deleteCount_one": "Delete Space", "deleteCount_other": "Delete {{count}} Spaces", "deleteDescendants_one": "Also deletes {{count}} nested Space", diff --git a/apps/web/src/i18n/resources/zh-CN/common.json b/apps/web/src/i18n/resources/zh-CN/common.json index f5ad03ba9..823a8eb23 100644 --- a/apps/web/src/i18n/resources/zh-CN/common.json +++ b/apps/web/src/i18n/resources/zh-CN/common.json @@ -43,8 +43,7 @@ "label": "已打开的 Space", "newTab": "新标签页", "noMatchingSpaces": "没有匹配的 Space。", - "closeTab": "关闭标签页", - "closeBeforeDeleting": "此 Space 正在打开，请关闭标签页后再删除。" + "closeTab": "关闭标签页" }, "sigil": { "chooseResource": "选择资源", @@ -728,6 +727,7 @@ "activationInProgress": "另一个主目录仍在打开，请等待其完成。" }, "spaceHierarchy": { + "deleteFromContainer": "请从 Home 或父 Space 中删除此 Space。", "bring": "移入已有空间", "alreadyChild": "已在当前空间中", "collectAction": "收纳为子 Space", @@ -753,6 +753,29 @@ "entryProtected": "此操作会移除或改变子 Space 入口。请使用“删除 Space”或“移入其他 Space”。", "deleteNamedTitle": "删除「{{name}}」？", "deleteWarning": "其中的所有内容将被永久删除，无法撤销。", + "deletingNamedTitle": "正在删除「{{name}}」", + "deleting": "正在删除…", + "deletionInProgress": "请稍候，正在保存更改并处理删除。", + "deleteFailedTitle": "未能完成删除", + "deleteFailedNamedTitle": "未能完成删除「{{name}}」", + "deletedNamedTitle": "「{{name}}」已删除", + "recheckDeletion": "重新检查", + "deletionDetails": "技术详情", + "deletionProgress_one": "已删除 {{count}} 个 Space，已删除的内容无法恢复。", + "deletionProgress_other": "已删除 {{count}} 个 Space，已删除的内容无法恢复。", + "deletionErrors": { + "deletedRefresh": "Space 已删除，但列表刷新失败。请刷新列表以查看当前状态。", + "agentRunning": "此 Space 或子 Space 中仍有 Agent 正在处理任务。请先停止任务或等待完成，再重新检查。", + "treeChanged": "Space 的层级已发生变化，请重新检查当前删除范围，再次确认。", + "interrupted": "删除流程未能全部完成，请检查当前 Space 的状态。", + "notFound": "此 Space 已不可用，请关闭弹窗并刷新 Space 列表。", + "preview": "暂时无法获取删除范围，请检查连接后重新检查。", + "prepare": "未能保存更改或完成删除准备，尚未发送删除请求。请处理问题后重新检查。", + "prepareTimeout": "保存等待超时，尚未删除，未保存的更改仍保留。请关闭弹窗，待保存完成后重试。", + "request": "暂时无法确认删除结果，请检查连接，并重新检查当前 Space 后再试。", + "refresh": "未能刷新 Space 列表，请关闭弹窗并刷新列表，确认当前状态。", + "release": "未能完成标签页清理，请关闭弹窗并检查仍打开的 Space。" + }, "deleteCount_one": "删除 Space", "deleteCount_other": "删除 {{count}} 个 Space", "deleteDescendants_one": "包含 {{count}} 个子空间，将一并删除。", diff --git a/apps/web/src/pages/CanvasListPage.test.tsx b/apps/web/src/pages/CanvasListPage.test.tsx index 75de31df7..c99527017 100644 --- a/apps/web/src/pages/CanvasListPage.test.tsx +++ b/apps/web/src/pages/CanvasListPage.test.tsx @@ -63,6 +63,15 @@ const environment = vi.hoisted(() => ({ getState: vi.fn<() => Promise>(), onState: vi.fn<(listener: (state: DesktopTabsSnapshot) => void) => () => void>(), + onSpacesDeleted: + vi.fn< + ( + listener: (result: { + workspaceId: string; + deletedCanvasIds: string[]; + }) => void, + ) => () => void + >(), })); vi.mock('react-i18next', () => ({ @@ -152,6 +161,7 @@ vi.mock('../hooks/useElectron', () => { ? environment.syncTitles(command) : environment.command(command), onState: environment.onState, + onSpacesDeleted: environment.onSpacesDeleted, getState: environment.getState, }, }; @@ -240,12 +250,13 @@ beforeEach(() => { environment.syncTitles.mockResolvedValue({ ok: true }); vi.mocked(getSpaceDeletionPreview).mockResolvedValue({ token: 'token', - spaces: [{ canvasId: 'space-1', title: 'Space One' }], + spaces: [{ canvasId: 'space-1', title: 'Space One', parentSpaceId: null }], }); vi.mocked(deleteSpaceTree).mockResolvedValue({ deletedCanvasIds: ['space-1'], }); environment.onState.mockReturnValue(vi.fn()); + environment.onSpacesDeleted.mockReturnValue(vi.fn()); environment.getState.mockResolvedValue({ tabs: [], activeId: 'home', @@ -698,18 +709,20 @@ describe('CanvasListPage navigation', () => { } it.each([false, true])( - 'blocks deleting an open desktop Space, including lazy restored Tabs (loading=%s)', + 'allows confirming deletion of open and lazy desktop Tabs (loading=%s)', async (loading) => { environment.desktopHome = true; markSpaceOpen(loading); await renderPage(); await requestDeletion(); - expect(toast).toHaveBeenCalledWith('desktopTabs.closeBeforeDeleting', { - tone: 'warning', - }); - expect(container.querySelector('[role="dialog"]')).toBeNull(); + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); expect(deleteSpaceTree).not.toHaveBeenCalled(); expect(environment.command).not.toHaveBeenCalled(); + await confirmDeletion(); + expect(deleteSpaceTree).toHaveBeenCalledExactlyOnceWith( + 'space-1', + 'token', + ); }, ); @@ -736,31 +749,36 @@ describe('CanvasListPage navigation', () => { canvasIds: ['space-1'], }, ], - ['end-space-deletion'], + [ + { + type: 'end-space-deletion', + deletedCanvasIds: ['space-1'], + }, + ], ]); }); - it('rechecks the current desktop Tabs when deletion is confirmed', async () => { + it('delegates Tabs opened during confirmation to main-process preparation', async () => { environment.desktopHome = true; await renderPage(); await requestDeletion(); markSpaceOpen(); await confirmDeletion(); - expect(deleteSpaceTree).not.toHaveBeenCalled(); - expect(environment.command).not.toHaveBeenCalled(); - expect(container.querySelector('[role="dialog"]')).not.toBeNull(); - expect(toast).toHaveBeenCalledWith('desktopTabs.closeBeforeDeleting', { - tone: 'warning', + expect(deleteSpaceTree).toHaveBeenCalledExactlyOnceWith('space-1', 'token'); + expect(environment.command).toHaveBeenCalledWith({ + type: 'begin-space-deletion', + canvasId: 'space-1', + canvasIds: ['space-1'], }); }); - it('blocks deleting an unopened parent when a descendant is open', async () => { + it('includes open descendants in deletion preparation', async () => { environment.desktopHome = true; vi.mocked(getSpaceDeletionPreview).mockResolvedValueOnce({ token: 'tree-token', spaces: [ - { canvasId: 'space-1', title: 'Parent' }, - { canvasId: 'child', title: 'Child' }, + { canvasId: 'space-1', title: 'Parent', parentSpaceId: null }, + { canvasId: 'child', title: 'Child', parentSpaceId: 'space-1' }, ], }); environment.getState.mockResolvedValue({ @@ -783,27 +801,31 @@ describe('CanvasListPage navigation', () => { await renderPage(); await requestDeletion(); await confirmDeletion(); - expect(deleteSpaceTree).not.toHaveBeenCalled(); - expect(environment.command).not.toHaveBeenCalled(); - expect(toast).toHaveBeenCalledWith('desktopTabs.closeBeforeDeleting', { - tone: 'warning', + expect(deleteSpaceTree).toHaveBeenCalledExactlyOnceWith( + 'space-1', + 'tree-token', + ); + expect(environment.command).toHaveBeenCalledWith({ + type: 'begin-space-deletion', + canvasId: 'space-1', + canvasIds: ['space-1', 'child'], }); }); - it('does not delete if current Tab state cannot be read', async () => { + it('does not delete if main cannot prepare Tabs', async () => { environment.desktopHome = true; await renderPage(); - environment.getState.mockRejectedValueOnce( - new Error('Tab state unavailable'), - ); + environment.command.mockResolvedValueOnce({ + ok: false, + error: 'Tab state unavailable', + }); const log = vi.spyOn(console, 'error').mockImplementation(() => {}); try { await requestDeletion(); + await confirmDeletion(); expect(deleteSpaceTree).not.toHaveBeenCalled(); - expect(container.querySelector('[role="dialog"]')).toBeNull(); - expect(toast).toHaveBeenCalledWith('Tab state unavailable', { - tone: 'danger', - }); + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + expect(toast).not.toHaveBeenCalled(); } finally { log.mockRestore(); } @@ -819,13 +841,15 @@ describe('CanvasListPage navigation', () => { const log = vi.spyOn(console, 'error').mockImplementation(() => {}); try { await confirmDeletion(); - expect(environment.command).toHaveBeenLastCalledWith( - 'end-space-deletion', - ); + expect(environment.command).toHaveBeenLastCalledWith({ + type: 'end-space-deletion', + deletedCanvasIds: [], + }); expect( container.querySelector('a[href="/canvas/space-1"]'), ).not.toBeNull(); - expect(toast).toHaveBeenCalledWith('Delete failed', { tone: 'danger' }); + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + expect(toast).not.toHaveBeenCalled(); } finally { log.mockRestore(); } @@ -839,20 +863,176 @@ describe('CanvasListPage navigation', () => { ok: false, error: 'Another Tab operation is in progress', }); + const log = vi.spyOn(console, 'error').mockImplementation(() => {}); try { await confirmDeletion(); expect(deleteSpaceTree).not.toHaveBeenCalled(); expect(environment.command).toHaveBeenCalledTimes(1); - expect(toast).toHaveBeenCalledWith( - 'Another Tab operation is in progress', - { tone: 'danger' }, - ); + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + expect(toast).not.toHaveBeenCalled(); } finally { log.mockRestore(); } }); + it('keeps partial deletion feedback in the dialog without a competing list-refresh toast', async () => { + environment.desktopHome = true; + await renderPage(); + await requestDeletion(); + const listCalls = vi.mocked(listCanvases).mock.calls.length; + vi.mocked(deleteSpaceTree).mockRejectedValueOnce( + new ApiError( + 500, + { + code: 'SPACE_TREE_DELETE_FAILED', + message: 'Cleanup incomplete', + details: { + deletedCanvasIds: ['space-1'], + remainingCanvasIds: [], + cause: 'Cleanup failed', + }, + }, + 'Cleanup incomplete', + ), + ); + await confirmDeletion(); + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + expect(listCanvases).toHaveBeenCalledTimes(listCalls); + expect(toast).not.toHaveBeenCalled(); + }); + + it.each([false, true])( + 'owns refresh feedback through desktop busy transitions (deletion succeeds=%s)', + async (succeeds) => { + environment.desktopHome = true; + await renderPage(); + await requestDeletion(); + const update = environment.onState.mock.calls.at(-1)?.[0]; + if (!update) throw new Error('Missing Home state listener'); + environment.command.mockImplementation( + async (command: DesktopTabCommand) => { + if ( + typeof command === 'object' && + (command.type === 'begin-space-deletion' || + command.type === 'end-space-deletion') + ) { + update({ + activeId: 'home', + workspaceId: 'workspace-1', + busy: command.type === 'begin-space-deletion', + canChangeWorkspace: true, + tabs: [], + }); + } + return { ok: true }; + }, + ); + vi.mocked(listCanvases).mockRejectedValueOnce(new Error('List offline')); + if (succeeds) + vi.mocked(listCanvases).mockRejectedValueOnce( + new Error('List offline'), + ); + if (!succeeds) + vi.mocked(deleteSpaceTree).mockRejectedValueOnce( + new Error('Delete offline'), + ); + await confirmDeletion(); + expect(listCanvases).toHaveBeenCalled(); + if (succeeds) { + expect(container.querySelector('[role="dialog"]')).toBeNull(); + expect(toast).toHaveBeenCalledExactlyOnceWith( + 'spaceHierarchy.deletionErrors.deletedRefresh', + { tone: 'danger' }, + ); + expect(container.querySelector('a[href="/canvas/space-1"]')).toBeNull(); + } else { + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + expect(toast).not.toHaveBeenCalled(); + expect( + container.querySelector('a[href="/canvas/space-1"]'), + ).not.toBeNull(); + } + }, + ); + + it('closes successful deletion and shows one notification when metadata refresh fails', async () => { + await renderPage(); + await requestDeletion(); + environment.refreshTitles.mockRejectedValueOnce( + new Error('Metadata offline'), + ); + await confirmDeletion(); + expect(container.querySelector('[role="dialog"]')).toBeNull(); + expect(container.querySelector('a[href="/canvas/space-1"]')).toBeNull(); + expect(toast).toHaveBeenCalledExactlyOnceWith( + 'spaceHierarchy.deletionErrors.deletedRefresh', + { tone: 'danger' }, + ); + }); + + it('prunes confirmed child deletions from another renderer before a failed Home refresh', async () => { + environment.desktopHome = true; + await renderPage(); + let resolveOld!: (value: Awaited>) => void; + vi.mocked(listCanvases).mockReturnValueOnce( + new Promise((resolve) => { + resolveOld = resolve; + }), + ); + act(() => window.dispatchEvent(new Event('canvas-list-changed'))); + const deleted = environment.onSpacesDeleted.mock.calls.at(-1)?.[0]; + const update = environment.onState.mock.calls.at(-1)?.[0]; + if (!deleted || !update) throw new Error('Missing Home subscriptions'); + await act(async () => + update({ + activeId: 'parent', + workspaceId: 'workspace-1', + busy: true, + canChangeWorkspace: true, + tabs: [], + }), + ); + await act(async () => + deleted({ workspaceId: 'other', deletedCanvasIds: ['space-1'] }), + ); + expect(container.querySelector('a[href="/canvas/space-1"]')).not.toBeNull(); + await act(async () => + deleted({ workspaceId: 'workspace-1', deletedCanvasIds: ['space-1'] }), + ); + expect(container.querySelector('a[href="/canvas/space-1"]')).toBeNull(); + expect(setCanvasCount).toHaveBeenLastCalledWith(0); + await act(async () => + resolveOld({ + canvases: [ + { + canvasId: 'space-1', + title: 'Stale', + nodeCount: 0, + createdAt: 0, + updatedAt: 0, + }, + ], + }), + ); + expect(container.querySelector('a[href="/canvas/space-1"]')).toBeNull(); + vi.mocked(listCanvases).mockRejectedValueOnce(new Error('List offline')); + await act(async () => + update({ + activeId: 'home', + workspaceId: 'workspace-1', + busy: false, + canChangeWorkspace: true, + tabs: [], + }), + ); + expect(container.querySelector('a[href="/canvas/space-1"]')).toBeNull(); + expect(setCanvasCount).toHaveBeenLastCalledWith(0); + expect(toast).toHaveBeenCalledExactlyOnceWith('List offline', { + tone: 'danger', + }); + }); + it('does not resurrect a deleted Space from an older background refresh', async () => { await renderPage(); let resolveOld!: (value: Awaited>) => void; diff --git a/apps/web/src/pages/CanvasListPage.tsx b/apps/web/src/pages/CanvasListPage.tsx index f22c3eb7e..5ec78fdd2 100644 --- a/apps/web/src/pages/CanvasListPage.tsx +++ b/apps/web/src/pages/CanvasListPage.tsx @@ -73,6 +73,7 @@ export default function CanvasListPage() { canvasId: string; title: string | null; } | null>(null); + const deletionOpen = useRef(false); const [showAll, setShowAll] = useState( () => localStorage.getItem('huabu.spaces.showAll') === 'true', ); @@ -111,7 +112,7 @@ export default function CanvasListPage() { const isDesktopHome = getElectronBridge()?.tabs?.context.role === 'home'; const fetchCanvases = useCallback( - async (showLoading = true) => { + async (showLoading = true, failureMessage?: string) => { const workspace = useWorkspaceStore.getState(); if (workspace.isSyncing) return; const requestId = ++listRequestRef.current.version; @@ -134,12 +135,14 @@ export default function CanvasListPage() { } catch (error) { if (isCurrent()) { console.error('Failed to list canvases:', error); - toast( - error instanceof Error - ? error.message - : translations.t('spaceHierarchy.failed'), - { tone: 'danger' }, - ); + if (!deletionOpen.current) + toast( + failureMessage ?? + (error instanceof Error + ? error.message + : translations.t('spaceHierarchy.failed')), + { tone: 'danger' }, + ); } } finally { if (isCurrent()) setIsLoading(false); @@ -148,6 +151,32 @@ export default function CanvasListPage() { [setCanvasCount], ); + const reconcileDeletedSpaces = useCallback( + (ids: string[]) => { + listRequestRef.current.version++; + setCanvases((current) => { + const remaining = current.filter( + (canvas) => !ids.includes(canvas.canvasId), + ); + setCanvasCount(remaining.length); + return remaining; + }); + setIsLoading(false); + }, + [setCanvasCount], + ); + + useEffect(() => { + const tabs = getElectronBridge()?.tabs; + if (!tabs || tabs.context.role !== 'home') return; + return tabs.onSpacesDeleted((result) => { + const workspace = useWorkspaceStore.getState(); + if (workspace.isSyncing || workspace.workspaceId !== result.workspaceId) + return; + reconcileDeletedSpaces(result.deletedCanvasIds); + }); + }, [reconcileDeletedSpaces]); + useEffect(() => { const requests = listRequestRef.current; void fetchCanvases(); @@ -265,28 +294,9 @@ export default function CanvasListPage() { } }; - const reportDeleteError = (error: unknown) => { - console.error('Failed to delete canvas:', error); - toast(error instanceof Error ? error.message : String(error), { - tone: 'danger', - }); - }; - - const canDeleteSpace = async (canvasId: string): Promise => { - const tabs = getElectronBridge()?.tabs; - if (!tabs) return true; - const state = await tabs.getState(); - if (!state.tabs.some((tab) => tab.canvasId === canvasId)) return true; - toast(t('desktopTabs.closeBeforeDeleting'), { tone: 'warning' }); - return false; - }; - - const requestDelete = async (canvasId: string, title: string | null) => { - try { - if (await canDeleteSpace(canvasId)) setPendingDelete({ canvasId, title }); - } catch (error) { - reportDeleteError(error); - } + const requestDelete = (canvasId: string, title: string | null) => { + deletionOpen.current = true; + setPendingDelete({ canvasId, title }); }; const formatDate = (timestamp: number) => { @@ -366,20 +376,24 @@ export default function CanvasListPage() { { + deletionOpen.current = false; setPendingDelete(null); void fetchCanvases(); }} - onDeleted={(ids, failure) => { - listRequestRef.current.version++; - setCanvases((current) => { - const remaining = current.filter( - (canvas) => !ids.includes(canvas.canvasId), - ); - setCanvasCount(remaining.length); - return remaining; - }); - if (!failure) setPendingDelete(null); - void fetchCanvases(false); + onDeleted={(ids, failure, completed) => { + reconcileDeletedSpaces(ids); + if (!failure || completed) { + deletionOpen.current = false; + setPendingDelete(null); + if (failure) toast(failure, { tone: 'danger' }); + else + void fetchCanvases( + false, + translations.t( + 'spaceHierarchy.deletionErrors.deletedRefresh', + ), + ); + } }} /> )} diff --git a/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.baseline.test.ts b/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.baseline.test.ts index 3f1fe959d..c1d7aa9ad 100644 --- a/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.baseline.test.ts +++ b/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.baseline.test.ts @@ -21,6 +21,7 @@ import { putNodeContent, } from '@/api/canvas'; import { toast } from '@/components/Common/Toast'; +import { holdSaveErrorRecovery } from '@/utils/saveErrorFeedback'; import { createNodeContentQueue } from '../nodeContentQueue'; @@ -311,42 +312,51 @@ describe('nodeContentQueue baseline lifecycle', () => { expect(toastMock).toHaveBeenCalledTimes(1); }); - it('adopts contentMissing when Load latest finds a deleted sidecar', async () => { - const node = noteNode('mine'); - const { queue, state } = makeQueue(node); - queue.seedBaselines([node]); - putMock.mockRejectedValueOnce( - new CanvasConflictError({ - code: 'NODE_CONTENT_CONFLICT', - message: 'changed elsewhere', + it.each([false, true])( + 'adopts contentMissing when Load latest finds a deleted sidecar (deferred: %s)', + async (deferred) => { + const node = noteNode('mine'); + const { queue, state } = makeQueue(node); + queue.seedBaselines([node]); + putMock.mockRejectedValueOnce( + new CanvasConflictError({ + code: 'NODE_CONTENT_CONFLICT', + message: 'changed elsewhere', + nodeId: 'n1', + currentRev: 'OTHER', + }), + ); + getMock.mockResolvedValueOnce({ nodeId: 'n1', - currentRev: 'OTHER', - }), - ); - getMock.mockResolvedValueOnce({ - nodeId: 'n1', - type: 'note', - label: null, - content: '', - rev: nodeRevisionOf({}), - contentMissing: true, - }); + type: 'note', + label: null, + content: '', + rev: nodeRevisionOf({}), + contentMissing: true, + }); - await queue.flushNow('c1', 'n1'); - const toastOpts = toastMock.mock.calls[0][1] as { - action: { onClick: () => void }; - }; - toastOpts.action.onClick(); - await vi.waitFor(() => { - expect(state._setStateNoAutosave).toHaveBeenCalledWith({ - nodes: [ - expect.objectContaining({ - data: expect.objectContaining({ contentMissing: true }), - }), - ], + const release = deferred ? holdSaveErrorRecovery() : undefined; + try { + await queue.flushNow('c1', 'n1'); + if (deferred) expect(toastMock).not.toHaveBeenCalled(); + } finally { + release?.(); + } + const toastOpts = toastMock.mock.calls[0][1] as { + action: { onClick: () => void }; + }; + toastOpts.action.onClick(); + await vi.waitFor(() => { + expect(state._setStateNoAutosave).toHaveBeenCalledWith({ + nodes: [ + expect.objectContaining({ + data: expect.objectContaining({ contentMissing: true }), + }), + ], + }); }); - }); - }); + }, + ); it('"Keep mine" re-baselines to the disk rev and force-writes over it', async () => { const node = noteNode('mine'); diff --git a/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.saveFailure.test.ts b/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.saveFailure.test.ts index bcd10b7c1..aa490af1f 100644 --- a/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.saveFailure.test.ts +++ b/apps/web/src/store/canvasStore/save/__tests__/nodeContentQueue.saveFailure.test.ts @@ -20,6 +20,10 @@ import { putNodeContent, } from '@/api/canvas'; import { dismissToast, toast } from '@/components/Common/Toast'; +import { + holdSaveErrorRecovery, + withSaveErrorsHandledByCaller, +} from '@/utils/saveErrorFeedback'; import { createNodeContentQueue } from '../nodeContentQueue'; @@ -69,6 +73,138 @@ beforeEach(() => { }); describe('nodeContentQueue — save-failure surfacing', () => { + it.each([false, true])( + 'duplicate Refresh preserves a newer content conflict and its recovery (deferred: %s)', + async (deferred) => { + const node = noteNode('mine'); + const { queue } = makeQueue(node); + const release = deferred ? holdSaveErrorRecovery() : undefined; + toastMock.mockReturnValue('conflict-toast'); + try { + putMock.mockRejectedValueOnce( + new NodeDuplicateFilesError({ message: 'duplicate' }), + ); + await expect(queue.flushNow('c1', node.id)).rejects.toThrow(); + putMock.mockRejectedValueOnce( + new CanvasConflictError({ + code: 'NODE_CONTENT_CONFLICT', + message: 'changed elsewhere', + currentRev: 'server', + }), + ); + await queue.flushNow('c1', node.id); + vi.mocked(dismissToast).mockClear(); + queue.clearDuplicateGuard(node.id); + expect(dismissToast).not.toHaveBeenCalled(); + await queue.flushNow('c1', node.id); + expect(putMock).toHaveBeenCalledTimes(2); + await expect(queue.flushAllForSubmission('c1')).rejects.toThrow(); + if (deferred) expect(toastMock).not.toHaveBeenCalled(); + } finally { + release?.(); + } + const recovery = toastMock.mock.calls.at(-1)?.[1]; + expect(recovery.secondaryAction.onClick).toBeTypeOf('function'); + expect(recovery.action.onClick).toBeTypeOf('function'); + putMock.mockResolvedValue({ + nodeId: node.id, + label: 'Note', + rev: 'saved', + }); + recovery.secondaryAction.onClick(); + await vi.waitFor(() => expect(queue.hasUnsavedContent('c1')).toBe(false)); + expect(putMock.mock.calls.at(-1)?.[2].expectRev).toBe('server'); + queue.discardCanvas('c1'); + }, + ); + + it.each(['conflict', 'duplicate'] as const)( + 'cancels deferred %s recovery when its failure is retired', + async (kind) => { + const node = noteNode('unsaved'); + const { queue } = makeQueue(node); + const release = holdSaveErrorRecovery(); + try { + putMock.mockRejectedValue( + kind === 'conflict' + ? new CanvasConflictError({ + code: 'NODE_CONTENT_CONFLICT', + message: 'conflict', + currentRev: 'server', + }) + : new NodeDuplicateFilesError({ message: 'duplicate' }), + ); + queue.scheduleChanges('c1', [], [node]); + await expect(queue.flushAllForSubmission('c1')).rejects.toThrow(); + queue.seedBaselines([node]); + queue.scheduleChanges('c1', [], [node]); + await expect(queue.flushAllForSubmission('c1')).rejects.toThrow(); + expect(toastMock).not.toHaveBeenCalled(); + } finally { + release(); + } + expect(toastMock).toHaveBeenCalledOnce(); + queue.discardCanvas('c1'); + }, + ); + + it.each(['resolved', 'forgotten', 'discarded', 'refreshed'] as const)( + 'does not replay a deferred duplicate notification after it is %s', + async (kind) => { + const node = noteNode('unsaved'); + const { queue, state } = makeQueue(node); + const release = holdSaveErrorRecovery(); + try { + putMock.mockRejectedValue( + new NodeDuplicateFilesError({ message: 'duplicate' }), + ); + await expect(queue.flushNow('c1', node.id)).rejects.toThrow(); + if (kind === 'resolved') { + putMock.mockResolvedValue({ + nodeId: node.id, + label: 'Note', + rev: 'saved', + }); + await queue.flushNow('c1', node.id); + expect(state.patchNodeSilent).toHaveBeenLastCalledWith(node.id, { + contentDuplicate: false, + duplicateFiles: [], + }); + } else if (kind === 'forgotten') queue.forgetNode(node.id); + else if (kind === 'discarded') queue.discardCanvas('c1'); + else queue.clearDuplicateGuard(node.id); + } finally { + release(); + } + expect(toastMock).not.toHaveBeenCalled(); + queue.discardCanvas('c1'); + }, + ); + + it('returns preparation save failures to the caller without losing edits or suppressing future autosave errors', async () => { + const node = noteNode('unsaved content'); + const { queue, state } = makeQueue(node); + putMock.mockRejectedValue(new Error('offline')); + await expect( + withSaveErrorsHandledByCaller(async () => { + queue.scheduleChanges('c1', [], [node]); + await queue.flushAll(); + await queue.flushAllForSubmission('c1'); + }), + ).rejects.toThrow('Content saves failed'); + expect(toastMock).not.toHaveBeenCalled(); + expect(state.nodes[0].data.content).toBe('unsaved content'); + expect(queue.hasUnsavedContent('c1')).toBe(true); + queue.retryFailedSaves('c1'); + await expect(queue.flushAllForSubmission('c1')).rejects.toThrow( + 'Content saves failed', + ); + expect(toastMock).toHaveBeenCalledOnce(); + putMock.mockResolvedValue({ nodeId: node.id, label: 'Note', rev: 'saved' }); + queue.retryFailedSaves('c1'); + await queue.flushAllForSubmission('c1'); + expect(queue.hasUnsavedContent('c1')).toBe(false); + }); it('keeps failure records while suspended and permits retry after resuming', async () => { const node = noteNode('v1'); const { queue } = makeQueue(node); diff --git a/apps/web/src/store/canvasStore/save/nodeContentQueue.ts b/apps/web/src/store/canvasStore/save/nodeContentQueue.ts index a1f5b39d8..234e8f706 100644 --- a/apps/web/src/store/canvasStore/save/nodeContentQueue.ts +++ b/apps/web/src/store/canvasStore/save/nodeContentQueue.ts @@ -34,6 +34,10 @@ import { import { dismissToast, toast } from '@/components/Common/Toast'; import { i18n } from '@/i18n'; import { copyToClipboard } from '@/utils/io/clipboard'; +import { + saveErrorsHandledByCaller, + showSaveErrorRecovery, +} from '@/utils/saveErrorFeedback'; import { MD_BACKED_NODE_TYPES, @@ -233,11 +237,22 @@ export function createNodeContentQueue(opts: { const generations = new Map(); const failures = new Map(); const failureToasts = new Map(); + const pendingFailureRecovery = new Map void>(); function dismissFailureToast(nodeId: string): void { + pendingFailureRecovery.get(nodeId)?.(); + pendingFailureRecovery.delete(nodeId); const id = failureToasts.get(nodeId); if (id) dismissToast(id); failureToasts.delete(nodeId); } + function showFailureRecovery(nodeId: string, show: () => void): void { + const canvasId = opts.getState().canvasId; + const cancel = showSaveErrorRecovery(() => { + pendingFailureRecovery.delete(nodeId); + if (opts.getState().canvasId === canvasId) show(); + }); + if (cancel) pendingFailureRecovery.set(nodeId, cancel); + } const pendingFields = new Map>(); const suppressNextNewNodeSave = new Set(); /** @@ -269,7 +284,7 @@ export function createNodeContentQueue(opts: { /** * Node ids for which the persistent `NODE_CONTENT_CONFLICT` toast is - * already showing. Rate-limits the toast to once per node (autosave + * registered (visible or deferred). Rate-limits it to once per node (autosave * would otherwise re-pop it on every keystroke while the node stays * blocked). Cleared on a successful write or a baseline reseed. */ @@ -289,7 +304,7 @@ export function createNodeContentQueue(opts: { const frozen = new Map(); /** - * Node ids for which we have already shown the persistent + * Node ids for which we have registered the persistent * "duplicate files on disk" toast. Autosave retries while the * duplicate persists would otherwise pop a fresh toast on every * keystroke; we toast once and clear the flag on the next @@ -548,25 +563,27 @@ export function createNodeContentQueue(opts: { : i18n.t('node.untitled'); const generation = generations.get(nodeId); dismissFailureToast(nodeId); - const toastId = toast(i18n.t('node.contentConflict', { label }), { - tone: 'danger', - duration: 0, - secondaryAction: { - label: i18n.t('node.contentConflictKeepMine'), - onClick: () => { - if (generations.get(nodeId) !== generation) return; - void resolveKeepMine(nodeId); + showFailureRecovery(nodeId, () => { + const toastId = toast(i18n.t('node.contentConflict', { label }), { + tone: 'danger', + duration: 0, + secondaryAction: { + label: i18n.t('node.contentConflictKeepMine'), + onClick: () => { + if (generations.get(nodeId) !== generation) return; + void resolveKeepMine(nodeId); + }, }, - }, - action: { - label: i18n.t('node.contentConflictLoadLatest'), - onClick: () => { - if (generations.get(nodeId) !== generation) return; - void resolveLoadLatest(nodeId); + action: { + label: i18n.t('node.contentConflictLoadLatest'), + onClick: () => { + if (generations.get(nodeId) !== generation) return; + void resolveLoadLatest(nodeId); + }, }, - }, + }); + failureToasts.set(nodeId, toastId); }); - failureToasts.set(nodeId, toastId); console.warn('[node-content] write refused (stale content):', nodeId); } @@ -745,10 +762,12 @@ export function createNodeContentQueue(opts: { if (duplicateToasted.has(nodeId)) return; duplicateToasted.add(nodeId); dismissFailureToast(nodeId); - failureToasts.set( - nodeId, - toast(err.message, { tone: 'danger', duration: 0, dismissible: true }), - ); + showFailureRecovery(nodeId, () => { + failureToasts.set( + nodeId, + toast(err.message, { tone: 'danger', duration: 0, dismissible: true }), + ); + }); console.error('Node write refused — duplicate files on disk:', nodeId, err); } @@ -855,6 +874,7 @@ export function createNodeContentQueue(opts: { message: string, source: 'user' | 'auto', ): void { + if (saveErrorsHandledByCaller()) return; if (source === 'auto' && saveErrorToasted.has(nodeId)) return; saveErrorToasted.add(nodeId); const generation = generations.get(nodeId); @@ -1163,6 +1183,8 @@ export function createNodeContentQueue(opts: { clearDuplicateGuard(nodeId) { duplicateToasted.delete(nodeId); + if (failures.get(nodeId)?.error instanceof NodeDuplicateFilesError) + dismissFailureToast(nodeId); }, forgetNode(nodeId) { diff --git a/apps/web/src/utils/applicationPreferenceStore.test.ts b/apps/web/src/utils/applicationPreferenceStore.test.ts index c4ffae9c9..45c84e1e6 100644 --- a/apps/web/src/utils/applicationPreferenceStore.test.ts +++ b/apps/web/src/utils/applicationPreferenceStore.test.ts @@ -64,6 +64,7 @@ beforeEach(async () => { endWorkspaceChange: vi.fn(), onNavigate: vi.fn(), onPrepareClose: vi.fn(), + onSpacesDeleted: vi.fn(), finishPrepareClose: vi.fn(), keepPrepareCloseAlive: vi.fn(), onClosePrepared: vi.fn(), diff --git a/apps/web/src/utils/desktopStorage.test.ts b/apps/web/src/utils/desktopStorage.test.ts index 9c9ba52aa..eee04b22f 100644 --- a/apps/web/src/utils/desktopStorage.test.ts +++ b/apps/web/src/utils/desktopStorage.test.ts @@ -54,6 +54,7 @@ function setContext(context: DesktopTabContext) { endWorkspaceChange: vi.fn(), onNavigate: vi.fn(), onPrepareClose: vi.fn(), + onSpacesDeleted: vi.fn(), finishPrepareClose: vi.fn(), keepPrepareCloseAlive: vi.fn(), onClosePrepared: vi.fn(), diff --git a/apps/web/src/utils/pageLifecycle.test.ts b/apps/web/src/utils/pageLifecycle.test.ts index 27f17535f..53391f8b6 100644 --- a/apps/web/src/utils/pageLifecycle.test.ts +++ b/apps/web/src/utils/pageLifecycle.test.ts @@ -18,6 +18,55 @@ function deferred() { } describe('page lifecycle', () => { + it.each(['retry', 'import', 'save'] as const)( + 'retires a stalled %s without allowing late completion to advance preparation', + async (stage) => { + const lifecycle = createPageLifecycle(); + const stalled = deferred(); + const controller = new AbortController(); + const next = vi.fn(async () => {}); + const operation = + stage === 'import' + ? lifecycle.run('importing', () => stalled.promise) + : undefined; + lifecycle.register('first', { + flush: stage === 'save' ? () => stalled.promise : async () => {}, + }); + lifecycle.register('next', { flush: next }); + const first = lifecycle.prepare( + controller.signal, + undefined, + stage === 'retry' ? () => stalled.promise : undefined, + ); + const rejected = expect(first).rejects.toMatchObject({ + name: 'AbortError', + }); + controller.abort(); + await rejected; + expect(next).not.toHaveBeenCalled(); + stalled.resolve(); + await operation; + await Promise.resolve(); + expect(next).not.toHaveBeenCalled(); + await lifecycle.prepare(new AbortController().signal); + expect(next).toHaveBeenCalledOnce(); + }, + ); + + it('does not start another participant after a settler aborts preparation', async () => { + const lifecycle = createPageLifecycle(); + const controller = new AbortController(); + const later = vi.fn(); + const retry = vi.fn(async () => {}); + lifecycle.register('first', { settle: () => controller.abort() }); + lifecycle.register('later', { settle: later, flush: later }); + await expect( + lifecycle.prepare(controller.signal, undefined, retry), + ).rejects.toMatchObject({ name: 'AbortError' }); + expect(later).not.toHaveBeenCalled(); + expect(retry).not.toHaveBeenCalled(); + }); + it('preserves synchronous action setup while tracking its asynchronous lifetime', async () => { const lifecycle = createPageLifecycle(); const started = vi.fn(); diff --git a/apps/web/src/utils/pageLifecycle.ts b/apps/web/src/utils/pageLifecycle.ts index e86376da3..cb2eb7a89 100644 --- a/apps/web/src/utils/pageLifecycle.ts +++ b/apps/web/src/utils/pageLifecycle.ts @@ -23,14 +23,22 @@ type Preparation = { flush?: () => Promise; }; -function untilAborted(work: Promise, signal: AbortSignal): Promise { +function untilAborted( + work: () => Promise, + signal: AbortSignal, +): Promise { signal.throwIfAborted(); return new Promise((resolve, reject) => { - const abort = () => reject(signal.reason); - signal.addEventListener('abort', abort, { once: true }); - void work.then(resolve, reject).finally(() => { + const abort = () => { signal.removeEventListener('abort', abort); - }); + reject(signal.reason); + }; + signal.addEventListener('abort', abort, { once: true }); + void (async () => work())() + .then(resolve, reject) + .finally(() => { + signal.removeEventListener('abort', abort); + }); }); } @@ -79,6 +87,7 @@ export function createPageLifecycle() { async prepare( signal: AbortSignal, onStage?: (stage: PageCloseStage) => void, + retry?: () => Promise, ): Promise { if (closing && !closing.signal.aborted) throw new Error('Page close preparation is already in progress.'); @@ -87,7 +96,11 @@ export function createPageLifecycle() { try { signal.throwIfAborted(); onStage?.('settling'); - for (const preparation of preparations.values()) preparation.settle?.(); + for (const preparation of preparations.values()) { + signal.throwIfAborted(); + preparation.settle?.(); + } + if (retry) await untilAborted(retry, signal); let previous: number; do { previous = generation; @@ -95,7 +108,7 @@ export function createPageLifecycle() { const operations = [...pending.values()]; onStage?.(operations[0].stage); await untilAborted( - Promise.all(operations.map((task) => task.settled)), + () => Promise.all(operations.map((task) => task.settled)), signal, ); } @@ -104,9 +117,10 @@ export function createPageLifecycle() { for (const preparation of preparations.values()) { signal.throwIfAborted(); if (preparation.flush) - await untilAborted(preparation.flush(), signal); + await untilAborted(preparation.flush, signal); } } while (pending.size || previous !== generation); + signal.throwIfAborted(); if (request.errors.length) throw request.errors[0]; } finally { if (closing === request) closing = undefined; diff --git a/apps/web/src/utils/prepareSpaceDeletion.ts b/apps/web/src/utils/prepareSpaceDeletion.ts new file mode 100644 index 000000000..6c8e80f89 --- /dev/null +++ b/apps/web/src/utils/prepareSpaceDeletion.ts @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { ConversationTitleSaveError } from '@/store/conversationTitleStore'; + +import { pageLifecycle } from './pageLifecycle'; +import { withSaveErrorsHandledByCaller } from './saveErrorFeedback'; + +let titleFailure: ConversationTitleSaveError | undefined; + +/** Retry and drain every save owner within the same abortable preparation. */ +export async function prepareSpaceDeletion(signal: AbortSignal): Promise { + const retained = titleFailure; + try { + await withSaveErrorsHandledByCaller(() => + pageLifecycle.prepare( + signal, + undefined, + retained ? () => retained.retry() : undefined, + ), + ); + signal.throwIfAborted(); + if (titleFailure === retained) titleFailure = undefined; + } catch (error) { + if (!signal.aborted && error instanceof ConversationTitleSaveError) + titleFailure = error; + throw error; + } +} diff --git a/apps/web/src/utils/saveErrorFeedback.test.ts b/apps/web/src/utils/saveErrorFeedback.test.ts new file mode 100644 index 000000000..3723fd993 --- /dev/null +++ b/apps/web/src/utils/saveErrorFeedback.test.ts @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { expect, it, vi } from 'vitest'; + +import { + holdSaveErrorRecovery, + saveErrorsHandledByCaller, + showSaveErrorRecovery, + withSaveErrorsHandledByCaller, +} from './saveErrorFeedback'; + +it('retains recovery across nested save barriers until the error surface closes', async () => { + const show = vi.fn(); + const release = holdSaveErrorRecovery(); + const releaseNested = holdSaveErrorRecovery(); + try { + await expect( + withSaveErrorsHandledByCaller(async () => { + expect(saveErrorsHandledByCaller()).toBe(true); + await withSaveErrorsHandledByCaller(async () => { + showSaveErrorRecovery(show); + }); + expect(show).not.toHaveBeenCalled(); + throw new Error('save failed'); + }), + ).rejects.toThrow('save failed'); + expect(saveErrorsHandledByCaller()).toBe(false); + expect(show).not.toHaveBeenCalled(); + } finally { + releaseNested(); + expect(show).not.toHaveBeenCalled(); + release(); + } + expect(show).toHaveBeenCalledOnce(); +}); + +it('cancels stale recovery and waits for a still-running save after the surface closes', async () => { + const show = vi.fn(); + const stale = vi.fn(); + const release = holdSaveErrorRecovery(); + await withSaveErrorsHandledByCaller(async () => { + const cancel = showSaveErrorRecovery(stale); + showSaveErrorRecovery(show); + cancel?.(); + release(); + expect(show).not.toHaveBeenCalled(); + }); + expect(stale).not.toHaveBeenCalled(); + expect(show).toHaveBeenCalledOnce(); +}); + +it('shows normal recovery immediately without claiming ordinary save errors', () => { + const show = vi.fn(); + showSaveErrorRecovery(show); + expect(show).toHaveBeenCalledOnce(); + expect(saveErrorsHandledByCaller()).toBe(false); +}); diff --git a/apps/web/src/utils/saveErrorFeedback.ts b/apps/web/src/utils/saveErrorFeedback.ts new file mode 100644 index 000000000..1b914c1fd --- /dev/null +++ b/apps/web/src/utils/saveErrorFeedback.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +let callerOwnedSaves = 0; +let recoveryHolds = 0; +const pendingRecovery = new Set<() => void>(); + +export function saveErrorsHandledByCaller(): boolean { + return callerOwnedSaves > 0; +} + +function releaseRecovery(): void { + if (callerOwnedSaves || recoveryHolds) return; + for (const callback of pendingRecovery) { + pendingRecovery.delete(callback); + callback(); + } +} + +/** Keep recovery actions available after the caller's error surface closes. */ +export function holdSaveErrorRecovery(): () => void { + recoveryHolds++; + return () => { + recoveryHolds--; + releaseRecovery(); + }; +} + +export function showSaveErrorRecovery( + callback: () => void, +): (() => void) | undefined { + if (!callerOwnedSaves && !recoveryHolds) { + callback(); + return; + } + pendingRecovery.add(callback); + return () => { + pendingRecovery.delete(callback); + }; +} + +/** Covers debounced and already-running writes in this renderer's save barrier. */ +export async function withSaveErrorsHandledByCaller( + operation: () => Promise, +): Promise { + callerOwnedSaves++; + try { + return await operation(); + } finally { + callerOwnedSaves--; + releaseRecovery(); + } +} diff --git a/docs/architecture/desktop-tabs.md b/docs/architecture/desktop-tabs.md index 89df7bde5..a19f14b10 100644 --- a/docs/architecture/desktop-tabs.md +++ b/docs/architecture/desktop-tabs.md @@ -51,6 +51,8 @@ DevTools open in a detached window for startup development tools, diagnostic req ## 3. Saving and lifetime +Deletion preparation in the requesting page and in affected desktop children shares `prepareSpaceDeletion(signal)`, built on the existing `pageLifecycle`. Registered editor settlement, pending mutations/imports, retained title-save retry, and all registered persistence owners run under that preparation's abort signal. Retry is inside the lifecycle lock and abortable wait, never a separate prelude in the desktop hook. Timeout, resume, replacement, or requester unmount releases caller-owned save feedback without waiting for a stalled underlying request. Late completion cannot advance the retired preparation, clear a newer retry failure, or acknowledge a replacement request. Actual writes are not cancelled or rolled back. Ordinary close and sign-out still use `pageLifecycle.prepare` without deletion-specific automatic retry or feedback suppression. + The native macOS Close menu action closes the active closable Tab only when the main window is focused. A focused auxiliary window, including the remote sign-in prompt, closes normally without touching the main window's Tabs; without a focused window the action does nothing. A Tab whose renderer is still initializing, before registering its save handler and without a load/crash failure, closes without a save request or discard warning. This also applies during a reload that has already passed the save/discard boundary. Once ready, the normal save acknowledgement remains mandatory; failed renderers retain explicit discard confirmation. Load failures arriving during or after intentional renderer destruction do not produce error dialogs. @@ -61,11 +63,11 @@ Persistence stays owned by its subsystem rather than a checklist in the desktop After a 150 ms grace period, an unfinished preparation displays an opaque, full-viewport progress overlay, including on Home. It shows only “Saving…” and “Cancel”, without a separate heading, explanatory paragraph or internal operation names. It continues automatically when ready and remains cancellable when this page finishes before another page during a window-wide preparation. Main announces when all pages are prepared so the overlay can disappear before Workspace activation uses its existing loading UI. Cancellation cancels only the pending desktop action: uploads, saves and server execution are not aborted. Main rejects the entire preparation batch and resumes every surviving page; generation/abort fencing prevents obsolete preparation callbacks from acknowledging a later action. Adding or activating a retained Tab does not initiate preparation; same-Tab Space replacement does. -Input freezes synchronously before blur and stays frozen until main resumes or destroys the renderer. The document body remains inert, including React portals, and a capture-phase guard installed before React blocks shortcuts and input. The progress surface uses a native modal dialog because its top-layer inert exemption is necessary to keep the cancel button accessible without unfreezing background content; it reuses Common Button/Loading and semantic colors. Only events inside that open control dialog bypass the input guard, and Canvas shortcuts exclude it. Native menu callbacks retain the same freeze check. The underlying page stays mounted throughout preparation. +Input freezes synchronously before blur and stays frozen until main resumes or destroys the renderer. Preparation makes the document body inert, including React portals, and a capture-phase guard installed before React blocks shortcuts and input. The progress surface uses a native modal dialog because its top-layer inert exemption keeps cancellation accessible; it reuses Common Button/Loading and semantic colors. Only the open preparation controls bypass the input guard; native menu callbacks retain the same freeze check. The underlying page stays mounted throughout preparation. Deletion never includes the requesting container, so its confirmation and pending status stay accessible without a special freeze exemption. A rejected preparation retains the page unless the user explicitly confirms discarding unsaved changes for a single-Tab close or reload. These two actions reuse one native warning after save failure or timeout, with Cancel as the default and escape action; cancellation of the preparation itself never opens a discard prompt. The warning includes the save error and data-loss risk, releases the obsolete preparation overlay while keeping input and the main operation lock frozen, and only the destructive choice permits close or reload. Declining resumes the unchanged page so saving can be retried after connectivity returns. Workspace changes and healthy-page shutdown failures retain their stricter save requirement. -The 15-second timeout detects an unresponsive renderer, not slow local work: a preparing page sends a request-scoped heartbeat every five seconds, allowing a responsive upload/save to finish without a fixed duration limit. Heartbeats and acknowledgements validate exact renderer ownership for live requests; replies for retired requests are ignored and cannot settle or extend a newer request. Repeated close requests share one pending operation. An aborted renderer preparation can be replaced immediately, even before its asynchronous cleanup runs; the old cleanup cannot release the replacement's lock. An unavailable renderer still requires explicit discard confirmation; reload remains available through existing keyboard shortcuts rather than an extra Tab button. +For ordinary close preparation, the 15-second timeout detects an unresponsive renderer, not slow local work: a preparing page sends a request-scoped heartbeat every five seconds, allowing a responsive upload/save to finish without a fixed duration limit. Space-deletion preparation instead uses a fixed 15-second deadline per affected renderer; heartbeats cannot extend it because the active container, not the child, owns the recovery UI. A timeout fails the preparation batch, resumes every surviving page, and retains all Tabs without sending a deletion request. Heartbeats and acknowledgements validate exact renderer ownership for live requests; replies for retired requests are ignored and cannot settle or extend a newer request. Repeated close requests share one pending operation. An aborted renderer preparation can be replaced immediately, even before its asynchronous cleanup runs; the old cleanup cannot release the replacement's lock. An unavailable renderer still requires explicit discard confirmation; reload remains available through existing keyboard shortcuts rather than an extra Tab button. Title-save rejection adds a localized “Retry title save” action to the existing persistent danger toast. It retries all current failed title intents through the title Store, including intents whose inner Chat tabs have closed, without reopening Chat or discarding a rename. Another failure produces fresh feedback with the same action; successful recovery does not resume the rejected desktop operation automatically, so the user can close, reload, switch Workspace, or quit again. Dismissing the toast never clears failed intent; a later save attempt exposes recovery again. @@ -81,9 +83,11 @@ If the initial Workspace save barrier fails, the Store retains the current Works If Home fails after the Workspace save barrier has completed, main releases only the abandoned Workspace transition's lock and keeps the Workspace identity unset. Reload and confirmed shutdown remain available. Home must become ready and confirm the active Workspace through Server initialization before registering an identity and restoring its session; main never guesses the previous identity. A Home failure during the save barrier does not release that barrier early, and readiness is rechecked before destroying Space views. Home failures never unlock an unrelated Tab close, reload or shutdown operation. -Successful Space deletion invalidates pending list requests before removing the card and starts a background refresh. Responses captured before deletion cannot restore the deleted card or overwrite the updated count. +Confirmed Space deletion invalidates pending Home list requests before removing cards and updating the count. Main sends Workspace-scoped `tabs:spaces-deleted` to Home before resuming pages, including when deletion originated in a parent Space. Home ignores results from another Workspace or during Workspace switching. Local completion reuses the same reconciliation before refreshing; background Home refreshes normally on its next activation. Responses captured before deletion cannot restore the deleted cards, and failed refreshes retain the pruned list. + +Space deletion is initiated from Home or a child entry in its parent Space, never from the target's own Space menu. Main rejects a deletion set containing the requesting Space before taking the operation lock. Before the HTTP request, main prepares every open target or descendant through the existing save boundary; lazy Tabs without a renderer need no preparation. A failed or cancelled preparation keeps every Tab and prevents deletion. Prepared pages remain frozen and alive until the request settles. Only the requesting renderer can finish the operation, supplying server-confirmed deleted IDs. Main closes those Tabs, persists the remaining session, and resumes surviving pages. The container remains open, and a surviving active Tab stays active; if a deleted Tab was active, main activates the surviving requester without creating a return Tab. Partial deletion closes only confirmed deletions. Opening Tabs and changing Workspaces remain blocked until completion, and a requester crash releases its abandoned lock. Browser deletion likewise stays in the container. -Home refuses to delete a Space that still has a desktop Tab, including a lazy restored Tab, and shows a localized instruction to close that Tab first. The current main-process snapshot is checked both before showing confirmation and again on confirmation. Deletion reuses the shared Space-tree confirmation dialog. Before sending the tree deletion request, it checks every ID in the server preview, including descendants, against open and lazy Tabs. For a closed Space tree, Home acquires the existing main-process operation lock with the complete preview identity set; main rechecks that none has a Tab and blocks opening Tabs or changing Workspaces until the request settles. Home releases the lock on success or failure, and a Home failure releases an abandoned deletion lock. Deletion never closes or discards an open Space automatically. Browser deletion retains its existing behavior. +Deletion errors belong to the surviving container. Both the initial local drain and desktop preparation mark save feedback as caller-owned, so Note queue write failures still retain unsaved content, log, and reject the barrier without also emitting a save-error toast during the drain. Content-conflict and duplicate-file recovery notifications are deferred rather than discarded: the requesting renderer holds them until its deletion dialog unmounts, while affected background tabs retain their recovery notifications after preparation for when the user returns. Frozen conflict revisions, duplicate banners, and the existing Keep mine / Load latest actions remain intact. Successful writes, authoritative baseline refreshes, duplicate resolution, and node/Space removal cancel obsolete deferred notifications. The existing dialog shows preparation, HTTP, partial-deletion, and release errors inline until release finishes. Home refresh errors during that dialog are logged without another toast or replacement of the primary error, including busy-to-idle refreshes with no confirmed deletion. Successful deletion closes the dialog; a subsequent metadata or list refresh failure produces one notification explicitly saying deletion succeeded but refresh failed. No destination-error handoff or return-target IPC remains. Other close, reload, and ordinary autosave operations retain their existing notifications. ## 4. Persistence and status diff --git a/docs/architecture/note-node.md b/docs/architecture/note-node.md index 7299241de..a47e42934 100644 --- a/docs/architecture/note-node.md +++ b/docs/architecture/note-node.md @@ -43,6 +43,8 @@ The shared NoteContentViewport keeps its `h-full` constraint in both scrollbar m **Saving** — an edit calls `updateNodeData(id, { content })`, which dispatches `UPDATE_NODE_DATA` → `MERGE_NODE_DATA`. There is **no debounce**: every editor `onChange` writes through. Concurrency is handled at the server by rev-CAS, which rejects a stale write with `409 NODE_CONTENT_CONFLICT` rather than merging it. +**Deletion preparation failures** — the save barrier preserves local content and rejects on content conflicts or duplicate sidecars. Conflict freezing and duplicate banners still apply immediately, but new recovery toasts wait for the caller-owned save drain and any deletion dialog in that renderer to finish. Closing the dialog restores the existing Keep mine / Load latest or duplicate-file guidance; successful persistence, authoritative reseeding, duplicate resolution, or node removal cancels stale deferred feedback. Duplicate Refresh clears the notification only when the queue's current failure is still a duplicate-file error; a newer content conflict retains its frozen revision and recovery actions. Ordinary save-failure toasts remain suppressed only during the caller-owned drain, not for the dialog's entire lifetime. + **Height** — new Notes default to a fixed 400 by 300 canvas-unit box, including width-only creation (which retains the requested width and uses height 300). The toolbar still switches between `auto` and `fixed` (`setNoteHeightMode`). Explicit dimensions and copied/moved `data.heightMode: 'auto'` remain authoritative; existing Notes retain their stored or legacy-inferred mode without migration. Measurement, freshness keys and the layout conversion are owned by [node-auto-height.md](./node-auto-height.md). **Note typography sizes** — `nodeTypography.ts` explicitly defines canvas Note H1–H6 at 24 / 22 / 20 / 18 / 18 / 18px, with H1 weight 600 and H2–H6 weight 500, and line-heights 1.3 / 1.3 / 1.35 / 1.4 / 1.4 / 1.4. H1 uses `title`; H2–H6 use independently editable `h2`–`h6` entries, not ratios derived from H1. Note paragraphs and list items use `noteBody` at 18px/1.4. PDF/Web `cardTitle` uses 24px/500, sharing the default title size with Note H1, Question/Agent, Space, and Frame; other card descriptions and Agent/Space status text retain `body` at 18px/1.4, while metadata remains 16px/1.4. Far-zoom labels use `cardTitle.weight`, independently of Note H1. H4–H6 retain semantic levels but share a visual size, distinguished from body text by weight and spacing. Mounted Notes and offscreen measurement receive identical unscaled variables from `NOTE_CONTENT_HOST_STYLE`. These typography rules are scoped to `huabu-note-content-host`; expanded Note WYSIWYG previews, generic Milkdown/chat typography, and raw Markdown source mode remain unchanged. Canvas and expanded-editor spacing remain independent. Height layout version 17 invalidates measurements made before the restored 18px body and revised heading scale. Future changes that affect canvas text height must also bump `HEIGHT_LAYOUT_VERSION`. diff --git a/docs/architecture/space-preview.md b/docs/architecture/space-preview.md index 1390c3bb9..ba2389af4 100644 --- a/docs/architecture/space-preview.md +++ b/docs/architecture/space-preview.md @@ -57,16 +57,20 @@ Both movement menu items remain disabled until the shared metadata is ready and The popover derives its current parent from loaded hierarchy metadata rather than an opening-time snapshot. Until the source Space summary is ready, destinations are unavailable. A parent conflict refreshes metadata and leaves the popover open for a fresh choice against the actual parent. Failures remain visible and refresh authoritative metadata because failed attachment can retain a card. Pending submission blocks dismissal and duplicate requests. Workspace changes close the popover; unmounts and workspace changes suppress stale completion callbacks. Success refreshes Home and shared hierarchy metadata and stays on the current page. Source-initiated moves retain the standard success toast, which automatically dismisses after three seconds (paused during hover or focus), without a Home/destination action or close button. Error toasts retain their persistent, dismissible behavior. No new endpoint, storage migration, Home hierarchy view, or drag-to-reparent behavior is introduced. -New child creation has a primary canvas toolbar button after Sketch, using shortcut `5`; both are unavailable in World. Space icons in creation controls, shortcut pickers, node-type surfaces, and card headers resolve through `NODE_ICON.spacePreview` in `config/nodeIcons.ts`, currently Layers2. Add Space Shortcut remains in the Add Content dropdown. The child button or shortcut always arms a one-shot creation tool, regardless of the current selection. It reuses the Frame pointer gesture: a click/tap creates an empty untitled child at that flow-space position, while a rectangle collects top-level roots using Frame's 50% overlap rule, including descendants of selected Frames. An empty rectangle creates an empty child at its top-left. Both paths keep the user in the source Space without a naming dialog or automatic navigation; the new entry opens the child explicitly. Pressing `5` again, Escape, or switching tools cancels pending placement. The validated child-creation API accepts an optional finite `position`; callers that omit it retain automatic entry placement. Move into another Space opens the shared destination popover; Delete Space opens a confirmation dialog. Both live in the current Space menu. Existing save-draining and per-Space viewport persistence handle navigation. +New child creation has a primary canvas toolbar button after Sketch, using shortcut `5`; both are unavailable in World. Space icons in creation controls, shortcut pickers, node-type surfaces, and card headers resolve through `NODE_ICON.spacePreview` in `config/nodeIcons.ts`, currently Layers2. Add Space Shortcut remains in the Add Content dropdown. The child button or shortcut always arms a one-shot creation tool, regardless of the current selection. It reuses the Frame pointer gesture: a click/tap creates an empty untitled child at that flow-space position, while a rectangle collects top-level roots using Frame's 50% overlap rule, including descendants of selected Frames. An empty rectangle creates an empty child at its top-left. Both paths keep the user in the source Space without a naming dialog or automatic navigation; the new entry opens the child explicitly. Pressing `5` again, Escape, or switching tools cancels pending placement. The validated child-creation API accepts an optional finite `position`; callers that omit it retain automatic entry placement. Move into another Space opens the shared destination popover from the current Space menu. Delete Space opens confirmation from Home or the child's entry in its parent, not from the current Space menu. Existing save-draining and per-Space viewport persistence handle navigation. The multi-selection toolbar exposes Collect into child Space directly; for a single selected node, the action lives in More with its shortcut hint rather than occupying a primary toolbar slot. Both use `Cmd+Option+S` on macOS and `Ctrl+Alt+S` elsewhere (S for Space). The shortcut matches physical `KeyS` as well as the text key so macOS Option-generated characters work. It ignores empty selections, editable fields, composition, repeated keydowns, and non-canvas interaction contexts; `Cmd/Ctrl+G` still groups a Frame. Collection creates an untitled owned child, retains a canonical entry at the moved roots' former top-left, preserves relative layout, and keeps the source Space open. Unsupported nodes are rejected by the existing migration service rather than silently skipped. Home remembers Top level / All selection through the shared keyboard-accessible TabGroup below its heading; selected tabs retain their surface background on hover. A compact search field sits beside Import and New Space and wraps below the heading on narrow screens. Search matches Space names across all levels, not ancestor names or paths; results display canonical ancestry for orientation. This is a metadata search, not recursive content search. -`POST /api/canvas/:canvasId/children` creates a child; `PATCH /api/canvas/:canvasId/parent` uses `expectedParentSpaceId` to reject stale reparenting. `GET /api/canvas/:canvasId/deletion-preview` returns the owned subtree plus a membership token; `POST /api/canvas/:canvasId/delete-tree` requires that token. A changed subtree returns a conflict requiring renewed confirmation. Legacy single-Space DELETE refuses non-leaf Spaces and uses the same lifecycle path for leaves. +`POST /api/canvas/:canvasId/children` creates a child; `PATCH /api/canvas/:canvasId/parent` uses `expectedParentSpaceId` to reject stale reparenting. `GET /api/canvas/:canvasId/deletion-preview` returns the owned subtree with each Space's canonical `parentSpaceId` plus a membership token; `POST /api/canvas/:canvasId/delete-tree` requires that token. Parent identities let the parent-Space dialog check that the target still belongs to its container, and the confirmed subtree must not contain that container. A changed subtree or root parent returns a conflict requiring renewed confirmation. Legacy single-Space DELETE refuses non-leaf Spaces and uses the same lifecycle path for leaves. Confirmed deletion removes descendants before ancestors, never follows ordinary shortcuts, and removes the deleted root's formal entry from its surviving parent. It rejects represented running/busy Agent conversations before deleting anything. Failures report `deletedCanvasIds` and `remainingCanvasIds` explicitly. Generic commands, delta replay, structural PUT, and node-sidecar deletion cannot remove or retarget a live formal entry. A single entry's UI Delete action opens subtree confirmation; mixed selections or Frame deletion containing entries ask the user to delete those Spaces explicitly first. Entry deletion uses the shared node toolbar's More menu (or keyboard Delete), with no duplicate standalone delete button. Confirmation uses Common Modal with initial focus on Cancel, the target Space name in the title, and a count-bearing destructive action. The body states that all contents will be permanently deleted and cannot be restored; when descendants exist, a separate count explicitly says they are deleted too. There is no repeated name card, descendant-name list, or shortcut-retention explanation. System lifecycle/compensation writes are trusted exceptions. +Deletion is available from Home or a formal child entry in its parent Space. The current Space menu does not offer self-deletion, and SpaceActions rejects requests for itself or targets outside its direct children. Ordinary shortcut removal still only removes the shortcut. The live deletion preview rechecks the container relationship before saving or issuing deletion. + +Save failures that prevent deletion belong in the existing dialog, without a competing Note-save toast. The initial local preparation has a fixed 15-second deadline and shares `prepareSpaceDeletion(signal)` with desktop child preparation: settle registered editors, retry retained title failures, await pending imports/mutations, then drain registered Canvas/Note, title, and conversation-draft persistence through `pageLifecycle`. Desktop preparation then has its own fixed 15-second deadline for each affected renderer, running in parallel. Requester unmount aborts local preparation. A preparation failure skips post-deletion metadata refresh because no deletion request was sent, so another network wait cannot trap the error dialog. Timing out only abandons the deletion attempt: saves and local edits remain intact, and late save completion cannot continue the abandoned attempt. These deadlines do not cancel an already-issued deletion request or resolve uncertain server outcomes. New content-conflict and duplicate-file recovery notifications in the requesting renderer wait until the dialog closes; the original recovery actions remain available, and resolved or removed failures are not replayed. Affected desktop child tabs retain recovery feedback for when the user returns to them. If deletion fails, secondary refresh errors are logged without replacing the primary error. A successful deletion closes the dialog even if metadata refresh fails; the surviving container then shows one explicit deletion-success/refresh-failure notification. Home similarly reports a failed post-deletion list refresh outside the closed dialog, retaining its confirmed-pruned list. + Structural saves merge omitted node identity fields with the current server record before validation. Commands, delta replay, structural PUT, and sidecar deletion compare existing shortcut IDs, types, and targets with the proposed result; unchanged identities require no hierarchy lookup. Only removed or retargeted shortcuts with syntactically valid Space IDs read their prior target through the existing Space handle and check its parent/entry metadata. Malformed dangling targets cannot identify canonical children and remain removable or retargetable without a storage lookup. This avoids Workspace-wide membership scans and does not introduce a second hierarchy cache. The check protects transitions from the authoritative prestate, not repair of externally corrupted topology. Before local destructive command effects, the client requires settled ownership metadata for affected Space cards, including descendants of deleted Frames. Unknown ownership blocks the change, starts a metadata refresh, and asks the user to retry; it never permits partial sidecar deletion. Received Space-card insertions invalidate/refetch metadata before exposing the new nodes. Undo and redo validate restored identities before consuming history or scheduling sidecar effects: a snapshot that would remove or retarget a live child entry is refused with an explicit lifecycle-action message, while geometry undo/redo remains available. @@ -81,7 +85,11 @@ Move-success toasts belong to the source Space page: navigating Home, entering a Area-selection preview and cancellation apply current World hierarchy visibility before selecting nodes or edges. Restoring the pre-gesture selection cannot reselect a now-hidden child entry or any edge whose endpoint is no longer visible. -Partial subtree deletion failures reconcile the server-confirmed deleted IDs from the shared error-details contract against the confirmed preview. Deleted Preview persistence is removed, affected lists refresh, and a deleted current Space navigates to Home. The failure remains visible in a toast and, where still mounted, the dialog; a deleted root does not offer a retry that can only return 404. Partial completion is never reported as full success. +Partial subtree deletion failures reconcile the server-confirmed deleted IDs from the shared error-details contract against the confirmed preview. Deleted Preview persistence is removed and affected lists refresh. The requesting container stays open, keeping deletion errors inline rather than transferring them to another page. Desktop main also publishes confirmed IDs to Home before resuming Tabs so a failed refresh cannot preserve deleted cards. A deleted target root does not offer a retry that can only return 404. Partial completion is never reported as full success. + +The failure state retains the target name, replaces the permanent-deletion warning with a localized reason and recovery instruction, and reports the confirmed deletion count when any Spaces were removed. Known server error codes map to localized messages; other failures use stage-specific messages with the original error available under Technical details and in the console. Cancel and Review again share the footer; no disabled destructive action remains in the error state. Review again only fetches a fresh preview and returns focus to Cancel: deletion requires a separate confirmation with the refreshed token. A deleted root instead offers Close. + +While save preparation, deletion, and cleanup are pending, the dialog keeps the target name in a Deleting title, replaces the confirmation copy with a live waiting status, and shows the shared spinner beside Deleting in the disabled action. Cancel, Escape, and backdrop dismissal remain unavailable until the operation settles. The UI does not invent a completion percentage or elapsed-time estimate. Successful deletion rechecks the captured Workspace after awaiting the metadata refresh, before invoking navigation callbacks. An old dialog's asynchronous completion cannot navigate a newly active Workspace. diff --git a/docs/architecture/web-architecture.md b/docs/architecture/web-architecture.md index 34abcd000..857ff78ec 100644 --- a/docs/architecture/web-architecture.md +++ b/docs/architecture/web-architecture.md @@ -47,6 +47,10 @@ config → (leaf — imports nothing internal) Common contains reusable UI primitives; canvas node references live in `components/Nodes/NodeRef/` because they resolve node state and navigate the canvas. Shared viewport geometry and fit/reveal operations live in `utils/canvasViewport.ts`, not in a panel directory, so components, hooks, pages, and the canvas store can reuse them without depending on panel implementation. +Common Modal uses the existing Floating UI focus manager to contain keyboard and assistive-technology focus, honor `initialFocusRef`, and restore focus on close. It locks background scrolling and bounds the scrollable panel to the visible viewport below desktop window chrome, so long error details remain reachable. + +Space deletion runs from Home or a parent Space, so the requesting page and its Common Modal are not among the target pages frozen by desktop preparation. Modal has no deletion-specific freeze exemption. + --- ## 3. Where the canvas engine lives diff --git a/packages/shared/src/types/api/space-hierarchy.ts b/packages/shared/src/types/api/space-hierarchy.ts index 12ba8df2f..af6d399ed 100644 --- a/packages/shared/src/types/api/space-hierarchy.ts +++ b/packages/shared/src/types/api/space-hierarchy.ts @@ -39,7 +39,11 @@ export type SpaceHierarchyResponse = z.infer< export const deleteSpaceTreePreviewSchema = z.object({ token: z.string(), spaces: z.array( - z.object({ canvasId: spaceId, title: z.string().nullable() }), + z.object({ + canvasId: spaceId, + title: z.string().nullable(), + parentSpaceId: spaceId.nullable(), + }), ), }); export type DeleteSpaceTreePreview = z.infer< diff --git a/packages/shared/src/types/desktop-tabs.ts b/packages/shared/src/types/desktop-tabs.ts index 22a442ab2..9ae911f97 100644 --- a/packages/shared/src/types/desktop-tabs.ts +++ b/packages/shared/src/types/desktop-tabs.ts @@ -51,7 +51,10 @@ export type DesktopTabCommand = | 'open-settings' | 'close-home-menu' | 'dismiss-home-menu' - | 'end-space-deletion' + | { + type: 'end-space-deletion'; + deletedCanvasIds: string[]; + } | { type: 'move-tab'; id: string; index: number } | { type: 'sync-workspace-capabilities'; canChangeWorkspace: boolean } | { @@ -118,7 +121,17 @@ export interface DesktopTabsApi { beginWorkspaceChange: () => Promise; endWorkspaceChange: () => Promise; onNavigate: (callback: (target: DesktopTabNavigation) => void) => () => void; - onPrepareClose: (callback: (requestId: string) => void) => () => void; + onPrepareClose: ( + callback: ( + request: string | { requestId: string; reason: 'space-deletion' }, + ) => void, + ) => () => void; + onSpacesDeleted: ( + callback: (result: { + workspaceId: string; + deletedCanvasIds: string[]; + }) => void, + ) => () => void; keepPrepareCloseAlive: (requestId: string) => void; onClosePrepared: (callback: () => void) => () => void; finishPrepareClose: (requestId: string, result: DesktopTabResult) => void;