Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/client-rename-removal-reconciliation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@inkeep/open-knowledge": patch
---

Make client-side rename and removal reconciliation consistent across the file tree, editor tabs, and server-driven auth redirects.

Renaming a document, folder, or asset now captures the existing editor snapshot before cleanup, clears stale providers and IndexedDB rows, and only then remaps tabs and navigation. Destination persistence is cleared only when a stale pooled document actually exists, so a destination provider that was already reopened by the server redirect cannot be torn down by a second local cleanup pass. Reusing a previous document name therefore starts from clean local state without duplicating old CRDT content, while renamed tabs retain their order and active target.

Deleting content now closes the affected document, folder, and asset tabs through the same reconciliation controller before clearing deduplicated document persistence. Server-reported renames and removals use that controller too, keeping active documents, inactive tabs, URL hashes, and home redirects aligned regardless of which surface initiated the operation.
54 changes: 39 additions & 15 deletions packages/app/src/components/EditorTabs.dom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,13 @@ let toastErrors: string[] = [];

const activateTab = vi.fn(() => {});
const activateNewTab = vi.fn(() => {});
const closeAndClearForRename = vi.fn(() => Promise.resolve());
const closeNewTab = vi.fn(() => {});
const closeTab = vi.fn(() => {});
const closeTabs = vi.fn(() => {});
const getPoolActiveDocName = vi.fn(() => null as string | null);
const openNewTab = vi.fn(() => {});
const pinTab = vi.fn(() => {});
const reopenClosedTab = vi.fn(() => {});
const remapTabsForRename = vi.fn(() => {});
const reconcileLocalRename = vi.fn(() => Promise.resolve());
const reorderTabs = vi.fn(() => {});
const unpinTab = vi.fn(() => {});

Expand Down Expand Up @@ -186,19 +184,17 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget,
activateNewTab,
activateTab,
closeAndClearForRename,
closeNewTab,
closeTab,
closeTabs,
getPoolActiveDocName,
isNewTabActive,
newTabIds,
openNewTab,
openTabs,
pinTab,
pinnedTabIds,
reopenClosedTab,
remapTabsForRename,
reconcileLocalRename,
reorderTabs,
unpinTab,
visibleTabIds,
Expand Down Expand Up @@ -261,23 +257,19 @@ function resetState() {
for (const fn of [
activateTab,
activateNewTab,
closeAndClearForRename,
closeNewTab,
closeTab,
closeTabs,
getPoolActiveDocName,
openNewTab,
pinTab,
reopenClosedTab,
remapTabsForRename,
reconcileLocalRename,
reorderTabs,
unpinTab,
]) {
fn.mockClear();
}
closeAndClearForRename.mockImplementation(() => Promise.resolve());
getPoolActiveDocName.mockImplementation(() => null);
remapTabsForRename.mockImplementation(() => {});
reconcileLocalRename.mockImplementation(() => Promise.resolve());
Object.defineProperty(window, 'okDesktop', {
configurable: true,
value: undefined,
Expand Down Expand Up @@ -621,12 +613,44 @@ describe('EditorTabs runtime behavior', () => {
expect(globalThis.fetch).not.toHaveBeenCalled();
});

test('rename post-commit reconciliation failures surface the refresh toast and skip navigation', async () => {
test('passes the successful response mapping to reconciliation before navigating', async () => {
activeDocName = 'docs/team/readme';
activeTabId = 'docs/team/readme';
remapTabsForRename.mockImplementation(() => {
throw new Error('idb clear failed');
reconcileLocalRename.mockImplementation((input) => {
expect(input).toEqual({
renamed: [{ fromDocName: 'docs/team/readme', toDocName: 'docs/team/renamed' }],
});
expect(window.location.hash).toBe('');
return Promise.resolve();
});
globalThis.fetch = vi.fn(() =>
Promise.resolve(
new Response(
JSON.stringify({
renamed: [{ fromDocName: 'docs/team/readme', toDocName: 'docs/team/renamed' }],
renamedAssets: [],
}),
{ headers: { 'Content-Type': 'application/json' }, status: 200 },
),
),
) as never;

await renderEditorTabs();
fireEvent.doubleClick(tabButton('docs/team/readme.txt'));
const input = screen.getByTestId('editor-tab-rename-input') as HTMLInputElement;
fireEvent.change(input, { target: { value: 'renamed.txt' } });
fireEvent.keyDown(input, { key: 'Enter' });

await waitFor(() => {
expect(reconcileLocalRename).toHaveBeenCalledTimes(1);
expect(window.location.hash).toBe('#/docs/team/renamed');
});
});

test('rename post-commit reconciliation failures surface the refresh toast and skip navigation', async () => {
activeDocName = 'docs/team/readme';
activeTabId = 'docs/team/readme';
reconcileLocalRename.mockImplementation(() => Promise.reject(new Error('idb clear failed')));
globalThis.fetch = vi.fn(() =>
Promise.resolve(
new Response(
Expand Down
20 changes: 5 additions & 15 deletions packages/app/src/components/EditorTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {
buildRenamedNodePath,
isValidNodeName,
normalizeRenameValue,
planRenameCleanupCalls,
remapActiveDocName,
} from '@/components/file-tree-operations';
import { Button } from '@/components/ui/button';
Expand All @@ -51,7 +50,6 @@ import {
} from '@/components/ui/input-group';
import { Kbd } from '@/components/ui/kbd';
import { useDocumentContext } from '@/editor/DocumentContext';
import { captureRenameSnapshots } from '@/editor/editor-cache';
import {
docTabId,
filterClosableTabIds,
Expand Down Expand Up @@ -451,20 +449,17 @@ export function EditorTabs() {
activeTarget,
activateTab,
activateNewTab,
closeAndClearForRename,
closeNewTab,
closeTab,
closeTabs,
getPoolActiveDocName,
poolHas,
isNewTabActive,
newTabIds,
openNewTab,
openTabs,
pinTab,
pinnedTabIds,
reopenClosedTab,
remapTabsForRename,
reconcileLocalRename,
reorderTabs,
unpinTab,
visibleTabIds,
Expand Down Expand Up @@ -630,20 +625,15 @@ export function EditorTabs() {

// Split try/catch: server-side rename already committed
// (`parsed.ok === true`). A failure inside the post-commit work
// (IDB clear via closeAndClearForRename, tab remap, event dispatch)
// (persistence cleanup, tab remap, event dispatch)
// is a client-side reconciliation failure, NOT a network error.
// Labeling it "Network error — please try again" would misdirect
// the user toward a retry that POSTs against a now-nonexistent
// source path and fails differently. The correct recovery is to
// refresh and resync with disk truth.
captureRenameSnapshots(renamed);
let reconcileOk = true;
try {
// Same gate as FileTree.applyRenamedDocuments — rationale documented
// at `planRenameCleanupCalls` in file-tree-operations.ts.
const cleanupDocNames = planRenameCleanupCalls(renamed, getPoolActiveDocName(), poolHas);
await Promise.all(cleanupDocNames.map((name) => closeAndClearForRename(name)));
remapTabsForRename(renamed);
await reconcileLocalRename({ renamed });
emitDocumentsChanged(['files', 'backlinks', 'graph']);
} catch (reconcileErr) {
reconcileOk = false;
Expand All @@ -664,8 +654,8 @@ export function EditorTabs() {
commitInProgressRef.current = false;
lastFailedValueRef.current = null;

// Skip navigation when reconciliation failed: remapTabsForRename never
// ran, so no tab is keyed to nextActiveDocName. Calling navigateToDoc
// Skip navigation when reconciliation failed: tab identities remain stale,
// so no tab is keyed to nextActiveDocName. Calling navigateToDoc
// would silently open a new tab and contradict the "refresh to resync"
// toast. Refresh recovers consistent state.
if (reconcileOk && nextActiveDocName && nextActiveDocName !== currentActiveDocName) {
Expand Down
12 changes: 4 additions & 8 deletions packages/app/src/components/FileTree.create.dom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ const openTargetMock = vi.fn(() => {});
const notifySidebarFileSelectedMock = vi.fn(() => {});
const closeTabsMock = vi.fn(() => {});
const closeDocumentMock = vi.fn(() => {});
const closeAndClearForRenameMock = vi.fn(async () => {});
const remapTabsForRenameMock = vi.fn(() => {});
const reconcileLocalRenameMock = vi.fn(async () => {});
const reconcileLocalRemovalMock = vi.fn(async () => {});
const dispatchHandoffMock = vi.fn(async () => ({ ok: true as const }));

const DOCUMENTS: FileEntry[] = [
Expand Down Expand Up @@ -261,15 +261,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: { kind: 'doc', target: 'notes/source', docName: 'notes/source' },
closeTabs: closeTabsMock,
closeDocument: closeDocumentMock,
closeAndClearDocument: closeAndClearForRenameMock,
closeAndClearForDelete: closeAndClearForRenameMock,
closeAndClearForRename: closeAndClearForRenameMock,
getPoolActiveDocName: () => 'notes/source',
poolHas: () => false,
isNewTabActive: false,
openTarget: openTargetMock,
prewarm: () => {},
remapTabsForRename: remapTabsForRenameMock,
reconcileLocalRemoval: reconcileLocalRemovalMock,
reconcileLocalRename: reconcileLocalRenameMock,
}),
}));

Expand Down
12 changes: 4 additions & 8 deletions packages/app/src/components/FileTree.duplicate.dom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ const openTargetMock = vi.fn(() => {});
const notifySidebarFileSelectedMock = vi.fn(() => {});
const closeTabsMock = vi.fn(() => {});
const closeDocumentMock = vi.fn(() => {});
const closeAndClearForRenameMock = vi.fn(async () => {});
const remapTabsForRenameMock = vi.fn(() => {});
const reconcileLocalRenameMock = vi.fn(async () => {});
const reconcileLocalRemovalMock = vi.fn(async () => {});
const dispatchHandoffMock = vi.fn(async () => ({ ok: true as const }));

const DOCUMENTS: FileEntry[] = [
Expand Down Expand Up @@ -307,15 +307,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: { kind: 'doc', target: 'notes/source', docName: 'notes/source' },
closeTabs: closeTabsMock,
closeDocument: closeDocumentMock,
closeAndClearDocument: closeAndClearForRenameMock,
closeAndClearForDelete: closeAndClearForRenameMock,
closeAndClearForRename: closeAndClearForRenameMock,
getPoolActiveDocName: () => 'notes/source',
poolHas: () => true,
isNewTabActive: false,
openTarget: openTargetMock,
prewarm: () => {},
remapTabsForRename: remapTabsForRenameMock,
reconcileLocalRemoval: reconcileLocalRemovalMock,
reconcileLocalRename: reconcileLocalRenameMock,
}),
}));

Expand Down
8 changes: 2 additions & 6 deletions packages/app/src/components/FileTree.share.dom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: { kind: 'doc', target: 'notes/source', docName: 'notes/source' },
closeTabs: () => {},
closeDocument: () => {},
closeAndClearDocument: async () => {},
closeAndClearForDelete: async () => {},
closeAndClearForRename: async () => {},
getPoolActiveDocName: () => 'notes/source',
poolHas: () => true,
isNewTabActive: false,
openTarget: () => {},
prewarm: () => {},
remapTabsForRename: () => {},
reconcileLocalRemoval: async () => {},
reconcileLocalRename: async () => {},
}),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,15 +220,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: null,
closeTabs: vi.fn(() => {}),
closeDocument: vi.fn(() => {}),
closeAndClearDocument: vi.fn(async () => {}),
closeAndClearForDelete: vi.fn(async () => {}),
closeAndClearForRename: vi.fn(async () => {}),
getPoolActiveDocName: () => null,
poolHas: () => false,
isNewTabActive: false,
openTarget: openTargetMock,
prewarm: () => {},
remapTabsForRename: vi.fn(() => {}),
reconcileLocalRemoval: vi.fn(async () => {}),
reconcileLocalRename: vi.fn(async () => {}),
}),
}));
vi.doMock('@/components/PageListContext', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,15 +194,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: null,
closeTabs: vi.fn(() => {}),
closeDocument: vi.fn(() => {}),
closeAndClearDocument: vi.fn(async () => {}),
closeAndClearForDelete: vi.fn(async () => {}),
closeAndClearForRename: vi.fn(async () => {}),
getPoolActiveDocName: () => null,
poolHas: () => false,
isNewTabActive: false,
openTarget: vi.fn(() => {}),
prewarm: () => {},
remapTabsForRename: vi.fn(() => {}),
reconcileLocalRemoval: vi.fn(async () => {}),
reconcileLocalRename: vi.fn(async () => {}),
}),
}));
vi.doMock('@/components/PageListContext', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: null,
closeTabs: vi.fn(() => {}),
closeDocument: vi.fn(() => {}),
closeAndClearDocument: vi.fn(async () => {}),
closeAndClearForDelete: vi.fn(async () => {}),
closeAndClearForRename: vi.fn(async () => {}),
getPoolActiveDocName: () => null,
poolHas: () => false,
isNewTabActive: false,
openTarget: vi.fn(() => {}),
prewarm: () => {},
remapTabsForRename: vi.fn(() => {}),
reconcileLocalRemoval: vi.fn(async () => {}),
reconcileLocalRename: vi.fn(async () => {}),
}),
}));
vi.doMock('@/components/PageListContext', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,11 @@ vi.doMock('@/editor/DocumentContext', () => ({
activeTarget: null,
closeTabs: vi.fn(() => {}),
closeDocument: vi.fn(() => {}),
closeAndClearDocument: vi.fn(async () => {}),
closeAndClearForDelete: vi.fn(async () => {}),
closeAndClearForRename: vi.fn(async () => {}),
getPoolActiveDocName: () => null,
poolHas: () => false,
isNewTabActive: false,
openTarget: vi.fn(() => {}),
prewarm: () => {},
remapTabsForRename: vi.fn(() => {}),
reconcileLocalRemoval: vi.fn(async () => {}),
reconcileLocalRename: vi.fn(async () => {}),
}),
}));
vi.doMock('@/components/PageListContext', () => ({
Expand Down
Loading