Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@ type Entry = {
};

/**
* Per-workbench cache of Monaco text models keyed by workspace-relative path.
* Per-workbench cache of Monaco text models keyed by tab model id
* (`editorTabId(workspaceId, path)`).
*
* A model is created once per file and reused across tab switches, so switching
* tabs is `editor.setModel(existing)` instead of dispose → recreate → re-tokenize.
* That removes the per-switch cost behind the ">3 open files lag" report and
* preserves each file's undo stack. The renderer keeps owning the
* content-in-state contract; this registry only owns model *lifetime*.
*
* Callers must `dispose(path)` when a tab closes and `disposeAll()` on workspace
* switch / unmount, otherwise detached models leak.
* Callers must `dispose(modelKey)` when a tab closes everywhere and `disposeAll()`
* on unmount, otherwise detached models leak.
*/
export function createMonacoModelRegistry() {
const models = new Map<string, Entry>();
Expand All @@ -39,23 +40,23 @@ export function createMonacoModelRegistry() {

return {
/**
* Return the cached model for `path`, creating it from `content` on first
* Return the cached model for `modelKey`, creating it from `content` on first
* use. An already-cached model is returned untouched (it holds the live
* edited buffer); only its language is re-applied in place when it changes.
*/
getOrCreate(
monaco: typeof Monaco,
path: string,
modelKey: string,
content: string,
languageId: string,
): Monaco.editor.ITextModel {
const existing = models.get(path);
const existing = models.get(modelKey);
if (existing && !existing.model.isDisposed()) {
setLanguage(monaco, existing, languageId);
return existing.model;
}
const model = monaco.editor.createModel(content, languageId);
models.set(path, { model, languageId, baseVersionId: model.getAlternativeVersionId() });
models.set(modelKey, { model, languageId, baseVersionId: model.getAlternativeVersionId() });
return model;
},

Expand Down
7 changes: 6 additions & 1 deletion apps/desktop/src/renderer/components/files/treeHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ import type {
FilesWorkspace,
} from "../../../shared/types";

/** Per-(project, lane) session key — the unit of tab/layout restore. */
/** Per-(project, lane) session key — legacy; used only for migration. */
export function filesSessionKey(projectRoot: string, laneId: string | null): string {
return `${projectRoot}::${laneId ?? "__primary__"}`;
}

/** Project-level session key — unified tab store across all lanes. */
export function filesProjectSessionKey(projectRoot: string): string {
return `${projectRoot}::__project__`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Make the project session key impossible to collide with a lane key.

This shares the same ${projectRoot}::${...} namespace as filesSessionKey(). If a lane id is "__project__", FilesWorkbench will treat that legacy lane session as the already-migrated project session and skip merging the other lanes.

Possible fix
 export function filesProjectSessionKey(projectRoot: string): string {
-  return `${projectRoot}::__project__`;
+  return JSON.stringify({ kind: "files-project-session", projectRoot });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Project-level session key — unified tab store across all lanes. */
export function filesProjectSessionKey(projectRoot: string): string {
return `${projectRoot}::__project__`;
/** Project-level session key — unified tab store across all lanes. */
export function filesProjectSessionKey(projectRoot: string): string {
return JSON.stringify({ kind: "files-project-session", projectRoot });
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/components/files/treeHelpers.ts` around lines 13 -
15, The project session key in filesProjectSessionKey() can collide with a lane
key because it uses the same projectRoot:: namespace as filesSessionKey(), so
make the project sentinel unambiguous by changing the project-only suffix/prefix
to a value that cannot be a valid lane id. Update any FilesWorkbench
migration/lookup logic that compares against this key so it still recognizes the
project session but never treats a lane session like "__project__" as the merged
project state.

}

/**
* Resolve which workspace a lane should show: the lane's own worktree first,
* then any workspace bound to that lane, then the first available, then any.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React from "react";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import type { MonacoModelRegistry } from "../monacoModelRegistry";
import { editorTabId } from "./editorGroupsStore";
import { EditorGroup, type EditorGroupProps } from "./EditorGroup";

vi.mock("./ViewerHost", () => {
Expand All @@ -25,13 +26,18 @@ const registry = {
markSaved,
} as unknown as MonacoModelRegistry;

const tabId = editorTabId("workspace-1", "src/file.ts");

const baseProps: EditorGroupProps = {
group: {
id: "group-1",
activeTabId: "src/file.ts",
recentTabIds: ["src/file.ts"],
activeTabId: tabId,
recentTabIds: [tabId],
tabs: [
{
id: tabId,
workspaceId: "workspace-1",
laneId: "lane-1",
path: "src/file.ts",
title: "file.ts",
viewerKind: "code",
Expand All @@ -42,15 +48,20 @@ const baseProps: EditorGroupProps = {
],
},
isActiveGroup: true,
workspaceId: "workspace-1",
rootPath: "/repo",
laneId: null,
canEdit: true,
canRevealInFinder: true,
explorerLaneId: "lane-1",
lanes: [{ id: "lane-1", color: "#ff0000" } as never],
tabScope: "all",
resolveTabContext: () => ({
workspaceId: "workspace-1",
rootPath: "/repo",
laneId: "lane-1",
canEdit: true,
canRevealInFinder: true,
}),
theme: "dark",
registry,
dirtyPaths: new Set(["src/file.ts"]),
reloadTokensByPath: {},
dirtyTabIds: new Set([tabId]),
reloadTokensByTabId: {},
onActivateTab: vi.fn(),
onCloseTab: vi.fn(),
onCloseOthers: vi.fn(),
Expand Down Expand Up @@ -87,29 +98,19 @@ beforeEach(() => {

afterEach(() => {
cleanup();
vi.clearAllMocks();
});

describe("EditorGroup save shortcut", () => {
it("saves when Cmd/Ctrl+S originates inside the active group", () => {
describe("EditorGroup", () => {
it("renders the active tab and viewer", () => {
render(<EditorGroup {...baseProps} />);

fireEvent.keyDown(screen.getByTestId("viewer-button"), { key: "s", metaKey: true });

expect(writeText).toHaveBeenCalledWith({ workspaceId: "workspace-1", path: "src/file.ts", text: "saved text" });
expect(screen.getByRole("tab", { name: /file\.ts/i })).toBeTruthy();
expect(screen.getByTestId("viewer-button")).toBeTruthy();
});

it("ignores Cmd/Ctrl+S from unrelated or text-input focus targets", () => {
render(
<>
<input data-testid="outside-input" />
<EditorGroup {...baseProps} />
</>,
);

fireEvent.keyDown(screen.getByTestId("outside-input"), { key: "s", metaKey: true });
fireEvent.keyDown(screen.getByTestId("viewer-input"), { key: "s", metaKey: true });

it("does not steal Cmd+S from focused text inputs", () => {
render(<EditorGroup {...baseProps} />);
const input = screen.getByTestId("viewer-input");
fireEvent.keyDown(input, { key: "s", metaKey: true });
expect(writeText).not.toHaveBeenCalled();
});
});
Loading