Skip to content

Commit 9b977ee

Browse files
arul28claude
andauthored
Files tab: remove view-only mode, fix remote workspace-id staleness (#780)
The Files tab went read-only when remote-connected: editability was gated on a tab's workspaceId (host-DB lane UUIDs) resolving against the listed workspaces, but restored sessions and module caches keyed only by project root path carried the local machine's stale UUIDs. - Remove the canEdit/canMutate view-only concept entirely: code tabs are always editable and saveable; non-code viewers stay naturally read-only. - Key the workspaces/root-tree module caches by binding identity (local vs remote:<targetId>) so local and remote sessions for the same path no longer pollute each other. - Retry listWorkspaces with capped backoff instead of silently swallowing failures. - Remap stale tab workspace ids to the freshly listed workspaces (by lane, then primary), recomputing composite tab ids, deduping collisions, and migrating dirty/reload state; external-workspace tabs untouched. - Add monacoModelRegistry.rekey so live buffers, undo stacks, and dirty state follow remapped tab ids (dirty buffer wins on collision). Wire fields isReadOnlyByDefault/mobileReadOnly are intentionally unchanged (iOS decodes them strictly); host-side write actions were already allowlisted. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent a3c4e88 commit 9b977ee

14 files changed

Lines changed: 505 additions & 119 deletions

apps/desktop/src/renderer/components/files/FilesExplorer.test.tsx

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,8 @@ function renderExplorer(overrides: Partial<FilesExplorerProps> = {}) {
5151
}
5252

5353
describe("FilesExplorer mutating controls", () => {
54-
it("disables create controls for read-only workspaces", () => {
55-
const props = renderExplorer({ canMutate: false });
56-
57-
const newFile = screen.getByLabelText("New file") as HTMLButtonElement;
58-
const newFolder = screen.getByLabelText("New folder") as HTMLButtonElement;
59-
expect(newFile.disabled).toBe(true);
60-
expect(newFolder.disabled).toBe(true);
61-
62-
fireEvent.click(newFile);
63-
fireEvent.click(newFolder);
64-
expect(props.onCreateFile).not.toHaveBeenCalled();
65-
expect(props.onCreateDirectory).not.toHaveBeenCalled();
66-
});
67-
68-
it("keeps create controls active for mutable workspaces", () => {
69-
const props = renderExplorer({ canMutate: true });
54+
it("keeps create controls active", () => {
55+
const props = renderExplorer();
7056

7157
const newFile = screen.getByLabelText("New file") as HTMLButtonElement;
7258
const newFolder = screen.getByLabelText("New folder") as HTMLButtonElement;

apps/desktop/src/renderer/components/files/FilesExplorer.tsx

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ export type FilesExplorerProps = {
8181
onContextMenu: (event: FilesExplorerContextMenuEvent) => void;
8282
onRenamePath: (sourcePath: string, destinationPath: string) => Promise<void>;
8383
onInlineRenameSettled: () => void;
84-
canMutate?: boolean;
8584
compact?: boolean;
8685
};
8786

@@ -192,7 +191,6 @@ export function FilesExplorer({
192191
onContextMenu,
193192
onRenamePath,
194193
onInlineRenameSettled,
195-
canMutate = true,
196194
compact = false,
197195
}: FilesExplorerProps) {
198196
const scrollRef = useRef<HTMLDivElement | null>(null);
@@ -310,7 +308,6 @@ export function FilesExplorer({
310308
type="button"
311309
title="New file"
312310
aria-label="New file"
313-
disabled={!canMutate}
314311
style={{ ...outlineButton({ height: 24, padding: "0 6px", fontSize: 10 }) }}
315312
onClick={() => onCreateFile(activeContextDir)}
316313
onMouseEnter={(event) => { event.currentTarget.style.borderColor = COLORS.accent; event.currentTarget.style.color = COLORS.accent; }}
@@ -324,7 +321,6 @@ export function FilesExplorer({
324321
type="button"
325322
title="New folder"
326323
aria-label="New folder"
327-
disabled={!canMutate}
328324
style={{ ...outlineButton({ height: 24, padding: "0 6px", fontSize: 10 }) }}
329325
onClick={() => onCreateDirectory(activeContextDir)}
330326
onMouseEnter={(event) => { event.currentTarget.style.borderColor = COLORS.accent; event.currentTarget.style.color = COLORS.accent; }}
@@ -387,7 +383,6 @@ export function FilesExplorer({
387383
type="button"
388384
title="New file"
389385
aria-label="New file"
390-
disabled={!canMutate}
391386
style={{ ...outlineButton({ height: 28, padding: "0 7px", fontSize: 10 }) }}
392387
onClick={() => onCreateFile(activeContextDir)}
393388
onMouseEnter={(event) => { event.currentTarget.style.borderColor = COLORS.accent; event.currentTarget.style.color = COLORS.accent; }}
@@ -401,7 +396,6 @@ export function FilesExplorer({
401396
type="button"
402397
title="New folder"
403398
aria-label="New folder"
404-
disabled={!canMutate}
405399
style={{ ...outlineButton({ height: 28, padding: "0 7px", fontSize: 10 }) }}
406400
onClick={() => onCreateDirectory(activeContextDir)}
407401
onMouseEnter={(event) => { event.currentTarget.style.borderColor = COLORS.accent; event.currentTarget.style.color = COLORS.accent; }}
@@ -490,7 +484,6 @@ export function FilesExplorer({
490484
type="button"
491485
title="New file"
492486
aria-label="New file"
493-
disabled={!canMutate}
494487
style={{ ...outlineButton({ height: 24, padding: "0 6px", fontSize: 10 }) }}
495488
onClick={() => onCreateFile(activeContextDir)}
496489
onMouseEnter={(event) => { event.currentTarget.style.borderColor = COLORS.accent; event.currentTarget.style.color = COLORS.accent; }}
@@ -504,7 +497,6 @@ export function FilesExplorer({
504497
type="button"
505498
title="New folder"
506499
aria-label="New folder"
507-
disabled={!canMutate}
508500
style={{ ...outlineButton({ height: 24, padding: "0 6px", fontSize: 10 }) }}
509501
onClick={() => onCreateDirectory(activeContextDir)}
510502
onMouseEnter={(event) => { event.currentTarget.style.borderColor = COLORS.accent; event.currentTarget.style.color = COLORS.accent; }}

apps/desktop/src/renderer/components/files/monacoModelRegistry.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,62 @@ describe("monacoModelRegistry", () => {
165165
expect(registry.isDirty("a")).toBe(true);
166166
});
167167

168+
it("rekey moves the live buffer, dirty state, and identity to the new key", () => {
169+
const { monaco, createdCount } = createFakeMonaco();
170+
const registry = createMonacoModelRegistry();
171+
172+
const model = registry.getOrCreate(monaco, "stale-ws::src/a.ts", "draft", "typescript") as any;
173+
model.__edit();
174+
registry.rekey("stale-ws::src/a.ts", "host-ws::src/a.ts");
175+
176+
expect(registry.has("stale-ws::src/a.ts")).toBe(false);
177+
expect(registry.getValue("host-ws::src/a.ts")).toBe("draft");
178+
expect(registry.isDirty("host-ws::src/a.ts")).toBe(true);
179+
// Reopening under the new key reuses the moved model — no rebuild from disk.
180+
const reopened = registry.getOrCreate(monaco, "host-ws::src/a.ts", "disk content", "typescript");
181+
expect(reopened).toBe(model);
182+
expect(createdCount()).toBe(1);
183+
expect(model.dispose).not.toHaveBeenCalled();
184+
});
185+
186+
it("rekey collision keeps the dirty buffer and disposes the clean one", () => {
187+
const { monaco } = createFakeMonaco();
188+
const registry = createMonacoModelRegistry();
189+
190+
// Dirty incoming, clean existing → incoming wins.
191+
const dirtyIncoming = registry.getOrCreate(monaco, "old-a", "edited", "plaintext") as any;
192+
dirtyIncoming.__edit();
193+
const cleanExisting = registry.getOrCreate(monaco, "new-a", "disk", "plaintext") as any;
194+
registry.rekey("old-a", "new-a");
195+
expect(cleanExisting.dispose).toHaveBeenCalledTimes(1);
196+
expect(registry.getValue("new-a")).toBe("edited");
197+
expect(registry.isDirty("new-a")).toBe(true);
198+
199+
// Dirty existing → existing wins, incoming disposed.
200+
const cleanIncoming = registry.getOrCreate(monaco, "old-b", "stale", "plaintext") as any;
201+
const dirtyExisting = registry.getOrCreate(monaco, "new-b", "kept edit", "plaintext") as any;
202+
dirtyExisting.__edit();
203+
registry.rekey("old-b", "new-b");
204+
expect(cleanIncoming.dispose).toHaveBeenCalledTimes(1);
205+
expect(dirtyExisting.dispose).not.toHaveBeenCalled();
206+
expect(registry.getValue("new-b")).toBe("kept edit");
207+
expect(registry.has("old-b")).toBe(false);
208+
});
209+
210+
it("rekey is a no-op for identical or unknown keys", () => {
211+
const { monaco } = createFakeMonaco();
212+
const registry = createMonacoModelRegistry();
213+
214+
const model = registry.getOrCreate(monaco, "a", "x", "plaintext") as any;
215+
registry.rekey("a", "a");
216+
registry.rekey("missing", "b");
217+
218+
expect(registry.has("a")).toBe(true);
219+
expect(registry.has("b")).toBe(false);
220+
expect(registry.getValue("a")).toBe("x");
221+
expect(model.dispose).not.toHaveBeenCalled();
222+
});
223+
168224
it("recreates a model whose underlying instance was disposed externally", () => {
169225
const { monaco, createdCount } = createFakeMonaco();
170226
const registry = createMonacoModelRegistry();

apps/desktop/src/renderer/components/files/monacoModelRegistry.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,34 @@ export function createMonacoModelRegistry() {
108108
return Boolean(entry && !entry.model.isDisposed());
109109
},
110110

111+
/**
112+
* Move a cached model to a new key (tab-id remap). The live buffer, undo
113+
* stack, and dirty baseline follow the new key. When the new key already
114+
* holds a live model — the remap collision case, where the stale tab was
115+
* dropped in favor of an authoritative one — the old entry is disposed.
116+
*/
117+
rekey(oldKey: string, newKey: string): void {
118+
if (oldKey === newKey) return;
119+
const entry = models.get(oldKey);
120+
if (!entry) return;
121+
models.delete(oldKey);
122+
const existing = models.get(newKey);
123+
if (existing && !existing.model.isDisposed()) {
124+
// Collision: keep whichever buffer holds unsaved edits.
125+
const entryDirty = !entry.model.isDisposed()
126+
&& entry.model.getAlternativeVersionId() !== entry.baseVersionId;
127+
const existingDirty = existing.model.getAlternativeVersionId() !== existing.baseVersionId;
128+
if (entryDirty && !existingDirty) {
129+
safeDispose(existing.model);
130+
models.set(newKey, entry);
131+
} else {
132+
safeDispose(entry.model);
133+
}
134+
return;
135+
}
136+
models.set(newKey, entry);
137+
},
138+
111139
dispose(modelKey: string): void {
112140
const entry = models.get(modelKey);
113141
if (!entry) return;

apps/desktop/src/renderer/components/files/v2/EditorGroup.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ const baseProps: EditorGroupProps = {
5757
workspaceId: "workspace-1",
5858
rootPath: "/repo",
5959
laneId: "lane-1",
60-
canEdit: true,
6160
canRevealInFinder: true,
6261
}),
6362
theme: "dark",
@@ -107,6 +106,7 @@ describe("EditorGroup", () => {
107106
render(<EditorGroup {...baseProps} />);
108107
expect(screen.getByRole("tab", { name: /file\.ts/i })).toBeTruthy();
109108
expect(screen.getByTestId("viewer-button")).toBeTruthy();
109+
expect(screen.getByTitle("Save (⌘S)")).toBeTruthy();
110110
});
111111

112112
it("marks the visible fallback tab active when lane scope hides the stored active tab", () => {

apps/desktop/src/renderer/components/files/v2/EditorGroup.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export function EditorGroup(props: EditorGroupProps) {
128128
});
129129
return;
130130
}
131-
if (!activeContext.canEdit || activeTab.viewerKind !== "code") return;
131+
if (activeTab.viewerKind !== "code") return;
132132
const text = registry.getValue(activeTab.id);
133133
if (text == null) return;
134134
void window.ade.files
@@ -143,7 +143,7 @@ export function EditorGroup(props: EditorGroupProps) {
143143
}, [activeContext, activeTab, onDirtyChange, onError, registry]);
144144

145145
useEffect(() => {
146-
if (!props.isActiveGroup || !activeTab || !activeContext?.canEdit || activeTab.viewerKind !== "code") return;
146+
if (!props.isActiveGroup || !activeTab || activeTab.viewerKind !== "code") return;
147147
const onKey = (event: KeyboardEvent) => {
148148
const mod = event.metaKey || event.ctrlKey;
149149
if (!mod || event.shiftKey || event.altKey || (event.key !== "s" && event.key !== "S")) return;
@@ -156,7 +156,7 @@ export function EditorGroup(props: EditorGroupProps) {
156156
};
157157
window.addEventListener("keydown", onKey);
158158
return () => window.removeEventListener("keydown", onKey);
159-
}, [activeContext?.canEdit, activeTab, props.isActiveGroup, saveActive]);
159+
}, [activeTab, props.isActiveGroup, saveActive]);
160160

161161
return (
162162
<div
@@ -215,7 +215,7 @@ export function EditorGroup(props: EditorGroupProps) {
215215
})}
216216
</div>
217217
) : null}
218-
{activeTab.viewerKind === "code" && activeContext.canEdit ? (
218+
{activeTab.viewerKind === "code" ? (
219219
<button type="button" onClick={saveActive} title="Save (⌘S)" className="rounded p-1 hover:bg-white/5" style={{ color: COLORS.textMuted }}>
220220
<FloppyDisk size={14} />
221221
</button>
@@ -235,7 +235,6 @@ export function EditorGroup(props: EditorGroupProps) {
235235
workspaceId={activeContext.workspaceId}
236236
rootPath={activeContext.rootPath}
237237
tab={activeTab}
238-
canEdit={activeContext.canEdit}
239238
theme={props.theme}
240239
registry={props.registry}
241240
reloadToken={props.reloadTokensByTabId[activeTab.id] ?? 0}

apps/desktop/src/renderer/components/files/v2/EditorGroups.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ export type TabWorkspaceContext = {
1515
workspaceId: string;
1616
rootPath: string;
1717
laneId: string | null;
18-
canEdit: boolean;
1918
canRevealInFinder: boolean;
2019
};
2120

0 commit comments

Comments
 (0)