From 089a23465d21c6f63474596537876cb0d3c593e5 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 08:45:00 +0000 Subject: [PATCH 01/11] feat: keep a per-project VS Code .code-workspace file in sync with worktrees Opt-in per project: set a .code-workspace path in Settings -> Runtimes and xum reconciles the file's folders with the project's active worktree workspaces on create/rename/fork/archive/unarchive/delete and at startup. Only entries under the project's managed worktree root are ever touched; user folders, comments, and settings blocks are preserved via jsonc edits. Fixes #3722 --- docs/runtime/worktree.mdx | 6 + .../ProjectSidebar/ProjectSidebar.test.tsx | 3 + src/browser/contexts/ProjectContext.tsx | 20 + .../Settings/Sections/RuntimesSection.tsx | 82 ++++ src/common/orpc/schemas/api.ts | 9 + src/common/schemas/project.ts | 4 + src/node/orpc/router.ts | 30 ++ .../builtInSkillContent.generated.ts | 6 + src/node/services/workspaceService.ts | 39 ++ src/node/worktree/codeWorkspaceSync.test.ts | 373 ++++++++++++++++++ src/node/worktree/codeWorkspaceSync.ts | 259 ++++++++++++ 11 files changed, 831 insertions(+) create mode 100644 src/node/worktree/codeWorkspaceSync.test.ts create mode 100644 src/node/worktree/codeWorkspaceSync.ts diff --git a/docs/runtime/worktree.mdx b/docs/runtime/worktree.mdx index 8cec1ba6c83..de823954eb1 100644 --- a/docs/runtime/worktree.mdx +++ b/docs/runtime/worktree.mdx @@ -30,3 +30,9 @@ Example layout: improved-auth-ux/ fix-ci-flakes/ ``` + +## VS Code workspace file sync + +To browse all of a project's worktrees from one VS Code or code-server window, set a `.code-workspace` file path per project in Settings → Runtimes (select the project scope). Xum keeps that file's folder list in sync as worktree workspaces are created, renamed, archived, and deleted. + +Xum only manages folder entries under the project's worktree directory; folders you add yourself, comments, and `settings`/`extensions` blocks are left untouched. diff --git a/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx b/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx index 9ea1eec4690..8b62b0d5742 100644 --- a/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx +++ b/src/browser/components/ProjectSidebar/ProjectSidebar.test.tsx @@ -269,6 +269,7 @@ function createProjectContextValue( updateDisplayName: () => resolveVoidResult(), updateColor: () => resolveVoidResult(), updateCustomInstructions: () => resolveVoidResult(), + updateCodeWorkspaceSyncPath: () => resolveVoidResult(), assignWorkspaceToSubProject: () => resolveVoidResult(), hasAnyProject: false, resolveNewChatProjectPath: () => null, @@ -1334,6 +1335,7 @@ describe("ProjectSidebar multi-project completed-subagent toggles", () => { updateDisplayName: () => resolveVoidResult(), updateColor: () => resolveVoidResult(), updateCustomInstructions: () => resolveVoidResult(), + updateCodeWorkspaceSyncPath: () => resolveVoidResult(), assignWorkspaceToSubProject: () => resolveVoidResult(), hasAnyProject: true, resolveNewChatProjectPath: () => "/projects/demo-project", @@ -1925,6 +1927,7 @@ describe("ProjectSidebar multi-project completed-subagent toggles", () => { updateDisplayName: () => resolveVoidResult(), updateColor: () => resolveVoidResult(), updateCustomInstructions: () => resolveVoidResult(), + updateCodeWorkspaceSyncPath: () => resolveVoidResult(), assignWorkspaceToSubProject: () => resolveVoidResult(), hasAnyProject: true, resolveNewChatProjectPath: () => "/projects/demo-project", diff --git a/src/browser/contexts/ProjectContext.tsx b/src/browser/contexts/ProjectContext.tsx index 100e8ba8120..60b0c985081 100644 --- a/src/browser/contexts/ProjectContext.tsx +++ b/src/browser/contexts/ProjectContext.tsx @@ -101,6 +101,10 @@ export interface ProjectContext { projectPath: string, customInstructions: string | null ) => Promise>; + updateCodeWorkspaceSyncPath: ( + projectPath: string, + codeWorkspaceSyncPath: string | null + ) => Promise>; assignWorkspaceToSubProject: ( projectPath: string, @@ -568,6 +572,20 @@ export function ProjectProvider(props: { children: ReactNode }) { [api, refreshProjects] ); + const updateCodeWorkspaceSyncPath = useCallback( + async (projectPath: string, codeWorkspaceSyncPath: string | null): Promise> => { + if (!api) return { success: false, error: "API not connected" }; + try { + await api.projects.setCodeWorkspaceSyncPath({ projectPath, codeWorkspaceSyncPath }); + await refreshProjects(); + return { success: true, data: undefined }; + } catch (error) { + return { success: false, error: getErrorMessage(error) }; + } + }, + [api, refreshProjects] + ); + const assignWorkspaceToSubProject = useCallback( async ( projectPath: string, @@ -621,6 +639,7 @@ export function ProjectProvider(props: { children: ReactNode }) { updateDisplayName, updateColor, updateCustomInstructions, + updateCodeWorkspaceSyncPath, assignWorkspaceToSubProject, }), [ @@ -647,6 +666,7 @@ export function ProjectProvider(props: { children: ReactNode }) { updateDisplayName, updateColor, updateCustomInstructions, + updateCodeWorkspaceSyncPath, assignWorkspaceToSubProject, ] ); diff --git a/src/browser/features/Settings/Sections/RuntimesSection.tsx b/src/browser/features/Settings/Sections/RuntimesSection.tsx index 3833d807551..570d27df0ac 100644 --- a/src/browser/features/Settings/Sections/RuntimesSection.tsx +++ b/src/browser/features/Settings/Sections/RuntimesSection.tsx @@ -1,6 +1,9 @@ import { useEffect, useRef, useState } from "react"; import { AlertTriangle, Loader2 } from "lucide-react"; +import { Button } from "@/browser/components/Button/Button"; +import { Input } from "@/browser/components/Input/Input"; + import { CoderWorkspaceForm, resolveCoderAvailability, @@ -134,6 +137,81 @@ function deriveProjectOverrideState( }; } +/** + * Per-project opt-in path of a VS Code .code-workspace file that xum keeps in + * sync with the project's active worktrees (issue #3722). Parent keys this by + * project path so drafts reset on scope switches. + */ +function CodeWorkspaceSyncField(props: { projectPath: string }) { + const { userProjects, updateCodeWorkspaceSyncPath } = useProjectContext(); + const savedPath = userProjects.get(props.projectPath)?.codeWorkspaceSyncPath ?? ""; + // null = untouched: the input shows the saved value. + const [draft, setDraft] = useState(null); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); + + const draftValue = draft ?? savedPath; + const isDirty = draftValue.trim() !== savedPath; + + const handleSave = async () => { + setSaving(true); + setError(null); + const result = await updateCodeWorkspaceSyncPath( + props.projectPath, + draftValue.trim() ? draftValue.trim() : null + ); + if (result.success) { + setDraft(null); + } else { + setError(result.error ?? "Failed to save workspace file path"); + } + setSaving(false); + }; + + return ( +
+
VS Code workspace file
+
+ Path of a .code-workspace file kept in sync with this + project's active worktrees (absolute, ~, or + relative to the project). Leave empty to disable. +
+
+ { + setDraft(event.target.value); + }} + onKeyDown={(event) => { + if (event.key === "Enter" && isDirty && !saving) { + event.preventDefault(); + void handleSave(); + } + }} + disabled={saving} + placeholder="e.g. ~/my-project.code-workspace" + aria-label="VS Code workspace file path" + className="max-w-[360px] min-w-0" + /> + +
+ {error && ( +
+ {error} +
+ )} +
+ ); +} + export function RuntimesSection() { const { api } = useAPI(); const { userProjects, refreshProjects } = useProjectContext(); @@ -518,6 +596,10 @@ export function RuntimesSection() { /> ) : null} + + {selectedProjectPath ? ( + + ) : null}
diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index 423d2c15334..5cd70576f47 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -802,6 +802,15 @@ export const projects = { .passthrough(), output: z.void(), }, + setCodeWorkspaceSyncPath: { + input: z + .object({ + projectPath: z.string(), + codeWorkspaceSyncPath: z.string().nullish(), + }) + .passthrough(), + output: z.void(), + }, mcp: { list: { input: z.object({ projectPath: z.string() }), diff --git a/src/common/schemas/project.ts b/src/common/schemas/project.ts index cd9e52c5953..12e9e0fa255 100644 --- a/src/common/schemas/project.ts +++ b/src/common/schemas/project.ts @@ -300,6 +300,10 @@ export const ProjectConfigSchema = z.object({ description: "Custom system prompt appended for every workspace in this project (Settings → Instructions)", }), + codeWorkspaceSyncPath: z.string().optional().meta({ + description: + "Path to a VS Code .code-workspace file kept in sync with this project's active worktrees (relative paths resolve against the project root). Unset = sync disabled.", + }), }); export type WorktreeArchiveSnapshotProject = z.infer; diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index d2b4d3f2ce3..e7aeb838e49 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -13,6 +13,10 @@ import { Err, Ok } from "@/common/types/result"; import { resolveProviderCredentials } from "@/node/utils/providerRequirements"; import { isErrnoWithCode } from "@/node/utils/fs"; import { isPathInsideDir, stripTrailingSlashes } from "@/node/utils/pathUtils"; +import { + CODE_WORKSPACE_EXTENSION, + syncProjectCodeWorkspace, +} from "@/node/worktree/codeWorkspaceSync"; import { generateWorkspaceIdentity } from "@/node/services/workspaceTitleGenerator"; import { WorkspaceGoalChildWorkspaceError, @@ -3428,6 +3432,32 @@ export const router = (authToken?: string) => { return config; }); }), + setCodeWorkspaceSyncPath: t + .input(schemas.projects.setCodeWorkspaceSyncPath.input) + .output(schemas.projects.setCodeWorkspaceSyncPath.output) + .handler(async ({ context, input }) => { + const normalizedPath = stripTrailingSlashes(input.projectPath); + const trimmed = input.codeWorkspaceSyncPath?.trim() ?? ""; + if (trimmed && !trimmed.endsWith(CODE_WORKSPACE_EXTENSION)) { + // The sync read-modify-writes this file, so refuse arbitrary targets. + throw new Error(`Path must end with ${CODE_WORKSPACE_EXTENSION}`); + } + await context.config.editConfig((config) => { + const project = config.projects.get(normalizedPath); + if (!project) { + throw new Error(`Project not found: ${normalizedPath}`); + } + // Store undefined for blank input to keep config.json minimal. + project.codeWorkspaceSyncPath = trimmed ? trimmed : undefined; + return config; + }); + // Sync immediately so enabling takes effect without waiting for the + // next workspace lifecycle event. Clearing or changing the path never + // deletes previously written files (they belong to the user). + if (trimmed) { + await syncProjectCodeWorkspace(context.config, normalizedPath); + } + }), remove: t .input(schemas.projects.remove.input) .output(schemas.projects.remove.output) diff --git a/src/node/services/agentSkills/builtInSkillContent.generated.ts b/src/node/services/agentSkills/builtInSkillContent.generated.ts index 74dd53ef931..32ef568edaa 100644 --- a/src/node/services/agentSkills/builtInSkillContent.generated.ts +++ b/src/node/services/agentSkills/builtInSkillContent.generated.ts @@ -7882,6 +7882,12 @@ export const BUILTIN_SKILL_FILES: Record> = { " fix-ci-flakes/", "```", "", + "## VS Code workspace file sync", + "", + "To browse all of a project's worktrees from one VS Code or code-server window, set a `.code-workspace` file path per project in Settings → Runtimes (select the project scope). Xum keeps that file's folder list in sync as worktree workspaces are created, renamed, archived, and deleted.", + "", + "Xum only manages folder entries under the project's worktree directory; folders you add yourself, comments, and `settings`/`extensions` blocks are left untouched.", + "", ].join("\n"), "references/docs/workspaces/compaction/automatic.mdx": [ "---", diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 2fd9b321372..c885258e83c 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -116,6 +116,7 @@ import { import { isWorktreeRuntime } from "@/node/runtime/worktreeLifecycleHooks"; import { expandTilde, expandTildeForSSH } from "@/node/runtime/tildeExpansion"; import { removeManagedGitWorktree } from "@/node/worktree/removeManagedGitWorktree"; +import { syncProjectCodeWorkspace } from "@/node/worktree/codeWorkspaceSync"; import { copyStagedWorkspaceAttachments, @@ -2907,6 +2908,14 @@ export class WorkspaceService extends EventEmitter { scheduledCount += 1; } + // Repair .code-workspace drift from lifecycle changes that happened while + // the app was not running (best-effort; syncProjectCodeWorkspace never throws). + for (const [projectPath, projectConfig] of this.config.loadConfigOrDefault().projects) { + if (projectConfig.codeWorkspaceSyncPath?.trim()) { + await syncProjectCodeWorkspace(this.config, projectPath); + } + } + log.info("[startup] WorkspaceService.initialize completed", { totalMs: Date.now() - startupStartedAt, scheduledCount, @@ -4410,6 +4419,7 @@ export class WorkspaceService extends EventEmitter { session.emitMetadata(this.enrichFrontendMetadata(completeMetadata)); } + await this.syncCodeWorkspaceFiles(owningProjectPath); eventSpine.emit("workspace.created", { workspaceId }); return Ok({ metadata: this.enrichFrontendMetadata(completeMetadata) }); } catch (error) { @@ -4827,6 +4837,7 @@ export class WorkspaceService extends EventEmitter { session.emitMetadata(this.enrichFrontendMetadata(completeMetadata)); } + await this.syncCodeWorkspaceFiles(completeMetadata.projectPath, completeMetadata.projects); eventSpine.emit("workspace.created", { workspaceId }); return Ok(enrichedMetadata); } catch (error) { @@ -5378,6 +5389,13 @@ export class WorkspaceService extends EventEmitter { removedFromConfig = true; this.autoTitlingWorkspaces.delete(workspaceId); + if (persistedWorkspace) { + await this.syncCodeWorkspaceFiles( + persistedWorkspace.projectPath, + persistedWorkspace.projects + ); + } + this.emit("metadata", { workspaceId, metadata: null, @@ -5398,6 +5416,21 @@ export class WorkspaceService extends EventEmitter { } } + /** + * Best-effort .code-workspace reconcile for every project involved in a + * workspace lifecycle change (multi-project workspaces touch several). + * syncProjectCodeWorkspace never throws, so lifecycle ops cannot fail here. + */ + private async syncCodeWorkspaceFiles( + projectPath: string, + projects?: ReadonlyArray<{ projectPath: string }> + ): Promise { + const involvedPaths = new Set([projectPath, ...(projects ?? []).map((ref) => ref.projectPath)]); + for (const involvedPath of involvedPaths) { + await syncProjectCodeWorkspace(this.config, involvedPath); + } + } + private enrichFrontendMetadata(metadata: FrontendWorkspaceMetadata): FrontendWorkspaceMetadata { const isInitializing = this.initStateManager.getInitState(metadata.id)?.status === "running" || undefined; @@ -6308,6 +6341,8 @@ export class WorkspaceService extends EventEmitter { this.emit("metadata", { workspaceId, metadata: enrichedMetadata }); } + await this.syncCodeWorkspaceFiles(configProjectPath, updatedMetadata.projects); + return Ok({ newWorkspaceId: workspaceId }); } catch (error) { const message = getErrorMessage(error); @@ -7129,6 +7164,7 @@ export class WorkspaceService extends EventEmitter { // disposal here only frees runtimes and spine middleware. Never throws. await agentPluginHookService.disposeWorkspace(workspaceId); + await this.syncCodeWorkspaceFiles(projectPath, beforeArchiveMetadata?.projects); eventSpine.emit("workspace.archived", { workspaceId }); return Ok({ kind: "archived" as const }); } catch (error) { @@ -7256,6 +7292,8 @@ export class WorkspaceService extends EventEmitter { await this.emitCurrentWorkspaceMetadata(workspaceId); } + await this.syncCodeWorkspaceFiles(projectPath, hookMetadata?.projects); + return Ok(undefined); } catch (error) { const message = getErrorMessage(error); @@ -8388,6 +8426,7 @@ export class WorkspaceService extends EventEmitter { const enrichedMetadata = this.enrichFrontendMetadata(metadata); session.emitMetadata(enrichedMetadata); + await this.syncCodeWorkspaceFiles(foundProjectPath, metadata.projects); eventSpine.emit("workspace.created", { workspaceId: newWorkspaceId }); return Ok({ metadata: enrichedMetadata, projectPath: foundProjectPath }); } catch (error) { diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts new file mode 100644 index 00000000000..d61526d1e88 --- /dev/null +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -0,0 +1,373 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import * as fsPromises from "fs/promises"; +import * as os from "os"; +import * as path from "path"; +import * as jsonc from "jsonc-parser"; +import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; +import { Config } from "@/node/config"; +import { + computeManagedWorktreePaths, + syncProjectCodeWorkspace, + updateCodeWorkspaceFile, +} from "./codeWorkspaceSync"; + +let tempDir: string; +let managedRootDir: string; +let workspaceFilePath: string; + +beforeEach(async () => { + tempDir = await fsPromises.mkdtemp(path.join(os.tmpdir(), "code-workspace-sync-")); + managedRootDir = path.join(tempDir, "src", "my-project"); + workspaceFilePath = path.join(tempDir, "my-project.code-workspace"); +}); + +afterEach(async () => { + await fsPromises.rm(tempDir, { recursive: true, force: true }); +}); + +async function readWorkspaceFile(): Promise { + return fsPromises.readFile(workspaceFilePath, "utf-8"); +} + +function parseFolders(text: string): Array<{ path: string }> { + const parsed = jsonc.parse(text) as { folders: Array<{ path: string }> }; + return parsed.folders; +} + +describe("updateCodeWorkspaceFile", () => { + test("creates a missing file with seed folders", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [worktree], + seedFolders: [path.join(tempDir, "project"), worktree], + }); + + const folders = parseFolders(await readWorkspaceFile()); + expect(folders).toEqual([{ path: path.join(tempDir, "project") }, { path: worktree }]); + }); + + test("adds missing worktree entries to an existing file", async () => { + const existing = path.join(managedRootDir, "feature-a"); + const added = path.join(managedRootDir, "feature-b"); + await fsPromises.writeFile( + workspaceFilePath, + JSON.stringify({ folders: [{ path: existing }] }, null, "\t") + ); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [existing, added], + seedFolders: [], + }); + + expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: existing }, { path: added }]); + }); + + test("removes managed entries that are no longer desired", async () => { + const kept = path.join(managedRootDir, "feature-a"); + const removedA = path.join(managedRootDir, "feature-b"); + const removedB = path.join(managedRootDir, "feature-c"); + await fsPromises.writeFile( + workspaceFilePath, + JSON.stringify({ folders: [{ path: removedA }, { path: kept }, { path: removedB }] }) + ); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [kept], + seedFolders: [], + }); + + expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: kept }]); + }); + + test("never removes user-owned entries outside the managed root", async () => { + const projectRoot = path.join(tempDir, "project"); + const userFolder = "/home/user/some-other-folder"; + const relativeUserFolder = "./relative-folder"; + await fsPromises.writeFile( + workspaceFilePath, + JSON.stringify({ + folders: [{ path: projectRoot }, { path: userFolder }, { path: relativeUserFolder }], + }) + ); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [], + seedFolders: [], + }); + + expect(parseFolders(await readWorkspaceFile())).toEqual([ + { path: projectRoot }, + { path: userFolder }, + { path: relativeUserFolder }, + ]); + }); + + test("preserves comments, settings, and folder names through edits", async () => { + const kept = path.join(managedRootDir, "feature-a"); + const removed = path.join(managedRootDir, "feature-b"); + const added = path.join(managedRootDir, "feature-c"); + const content = [ + "{", + "\t// user comment survives", + '\t"folders": [', + `\t\t{ "path": ${JSON.stringify(kept)}, "name": "Kept" },`, + `\t\t{ "path": ${JSON.stringify(removed)} }`, + "\t],", + '\t"settings": { "editor.tabSize": 2 }', + "}", + ].join("\n"); + await fsPromises.writeFile(workspaceFilePath, content); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [kept, added], + seedFolders: [], + }); + + const text = await readWorkspaceFile(); + expect(text).toContain("// user comment survives"); + const parsed = jsonc.parse(text) as { + folders: Array<{ path: string; name?: string }>; + settings: Record; + }; + expect(parsed.settings).toEqual({ "editor.tabSize": 2 }); + expect(parsed.folders).toEqual([{ path: kept, name: "Kept" }, { path: added }]); + }); + + test("adds a folders array to a file that lacks one", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + await fsPromises.writeFile(workspaceFilePath, '{\n\t"settings": {}\n}\n'); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [worktree], + seedFolders: [], + }); + + expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: worktree }]); + }); + + test("leaves a malformed file untouched without throwing", async () => { + const malformed = '{ "folders": [ { "path": broken '; + await fsPromises.writeFile(workspaceFilePath, malformed); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [path.join(managedRootDir, "feature-a")], + seedFolders: [], + }); + + expect(await readWorkspaceFile()).toBe(malformed); + }); + + test("is idempotent: a second sync does not rewrite the file", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + const update = { + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [worktree], + seedFolders: [worktree], + }; + await updateCodeWorkspaceFile(update); + const afterFirst = await fsPromises.stat(workspaceFilePath); + + await updateCodeWorkspaceFile(update); + const afterSecond = await fsPromises.stat(workspaceFilePath); + + expect(afterSecond.mtimeMs).toBe(afterFirst.mtimeMs); + expect(afterSecond.ino).toBe(afterFirst.ino); + }); + + test("two projects sharing one file manage disjoint entries", async () => { + const otherManagedRoot = path.join(tempDir, "src", "other-project"); + const mine = path.join(managedRootDir, "feature-a"); + const theirs = path.join(otherManagedRoot, "feature-x"); + await fsPromises.writeFile( + workspaceFilePath, + JSON.stringify({ folders: [{ path: mine }, { path: theirs }] }) + ); + + // Sync for "my-project" with an empty desired set: must not touch the + // other project's entry. + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [], + seedFolders: [], + }); + + expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: theirs }]); + }); + + test("resolves relative folder entries against the file's directory", async () => { + // Entry is relative but points inside the managed root; it must count as + // present (no duplicate added) and be removable when undesired. + const relative = "./src/my-project/feature-a"; + await fsPromises.writeFile( + workspaceFilePath, + JSON.stringify({ folders: [{ path: relative }] }) + ); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [path.join(managedRootDir, "feature-a")], + seedFolders: [], + }); + expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: relative }]); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDir, + desiredPaths: [], + seedFolders: [], + }); + expect(parseFolders(await readWorkspaceFile())).toEqual([]); + }); +}); + +describe("syncProjectCodeWorkspace", () => { + test("creates the file from project config, resolving relative setting paths", async () => { + const config = new Config(tempDir); + const projectPath = path.join(tempDir, "repo"); + await fsPromises.mkdir(projectPath, { recursive: true }); + const worktreePath = path.join(config.srcDir, "repo", "feat-1"); + await config.editConfig((cfg) => { + cfg.projects.set(projectPath, { + workspaces: [ + { + path: worktreePath, + id: "aaaaaaaaaa", + name: "feat-1", + runtimeConfig: { type: "worktree", srcBaseDir: config.srcDir }, + }, + ], + codeWorkspaceSyncPath: "repo.code-workspace", + }); + return cfg; + }); + + await syncProjectCodeWorkspace(config, projectPath); + + const text = await fsPromises.readFile(path.join(projectPath, "repo.code-workspace"), "utf-8"); + expect(parseFolders(text)).toEqual([{ path: projectPath }, { path: worktreePath }]); + }); + + test("does nothing when the setting is unset", async () => { + const config = new Config(tempDir); + const projectPath = path.join(tempDir, "repo"); + await fsPromises.mkdir(projectPath, { recursive: true }); + await config.editConfig((cfg) => { + cfg.projects.set(projectPath, { workspaces: [] }); + return cfg; + }); + + await syncProjectCodeWorkspace(config, projectPath); + + expect((await fsPromises.readdir(projectPath)).length).toBe(0); + }); + + test("refuses paths without the .code-workspace extension", async () => { + const config = new Config(tempDir); + const projectPath = path.join(tempDir, "repo"); + await fsPromises.mkdir(projectPath, { recursive: true }); + await config.editConfig((cfg) => { + cfg.projects.set(projectPath, { workspaces: [], codeWorkspaceSyncPath: "notes.json" }); + return cfg; + }); + + await syncProjectCodeWorkspace(config, projectPath); + + expect((await fsPromises.readdir(projectPath)).length).toBe(0); + }); +}); + +describe("computeManagedWorktreePaths", () => { + const projectPath = "/home/user/projects/my-project"; + + function makeMetadata(overrides: Partial): FrontendWorkspaceMetadata { + const base: FrontendWorkspaceMetadata = { + id: "abc123def0", + name: "feature-a", + projectName: "my-project", + projectPath, + namedWorkspacePath: path.join("/base/src/my-project", "feature-a"), + runtimeConfig: { type: "worktree", srcBaseDir: "/base/src" }, + }; + return { ...base, ...overrides }; + } + + const managedRoot = "/base/src/my-project"; + + test("includes active worktree workspaces and sorts deduped paths", () => { + const paths = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ name: "b", namedWorkspacePath: `${managedRoot}/b` }), + makeMetadata({ name: "a", namedWorkspacePath: `${managedRoot}/a` }), + makeMetadata({ name: "a-dup", namedWorkspacePath: `${managedRoot}/a` }), + ], + projectPath, + managedRootDir: managedRoot, + }); + expect(paths).toEqual([`${managedRoot}/a`, `${managedRoot}/b`]); + }); + + test("excludes archived, sub-agent, isolation-none, other-project, and out-of-root workspaces", () => { + const paths = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ archivedAt: "2026-01-02T00:00:00Z" }), + makeMetadata({ parentWorkspaceId: "parent1234" }), + makeMetadata({ taskIsolation: "none" }), + makeMetadata({ projectPath: "/home/user/projects/other" }), + makeMetadata({ namedWorkspacePath: "/elsewhere/feature-a" }), + makeMetadata({ runtimeConfig: { type: "local" } }), + ], + projectPath, + managedRootDir: managedRoot, + }); + expect(paths).toEqual([]); + }); + + test("re-includes unarchived workspaces", () => { + const paths = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ + archivedAt: "2026-01-01T00:00:00Z", + unarchivedAt: "2026-01-02T00:00:00Z", + }), + ], + projectPath, + managedRootDir: managedRoot, + }); + expect(paths).toEqual([`${managedRoot}/feature-a`]); + }); + + test("derives secondary checkout paths for multi-project workspaces", () => { + const paths = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ + projectPath: "/home/user/projects/primary", + namedWorkspacePath: "/base/src/primary/feature-a", + projects: [ + { projectPath: "/home/user/projects/primary", projectName: "primary" }, + { projectPath, projectName: "my-project" }, + ], + }), + ], + projectPath, + managedRootDir: managedRoot, + }); + expect(paths).toEqual([`${managedRoot}/feature-a`]); + }); +}); diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts new file mode 100644 index 00000000000..e3b0d18a924 --- /dev/null +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -0,0 +1,259 @@ +import * as fsPromises from "fs/promises"; +import * as path from "path"; +import * as jsonc from "jsonc-parser"; +import writeFileAtomic from "write-file-atomic"; +import { isWorkspaceArchived } from "@/common/utils/archive"; +import { isWorktreeRuntime } from "@/common/types/runtime"; +import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; +import type { Config } from "@/node/config"; +import { expandTilde } from "@/node/runtime/tildeExpansion"; +import { log } from "@/node/services/log"; +import { isPathInsideDir, stripTrailingSlashes } from "@/node/utils/pathUtils"; +import { getProjectName } from "@/node/utils/runtime/helpers"; + +/** + * Opt-in sync of a VS Code `.code-workspace` file's `folders` list with a + * project's active worktree workspaces (issue #3722), so users browsing in + * VS Code / code-server see every xum worktree in one multi-root window. + * + * Managed-entry invariant: xum only ever adds or removes folder entries whose + * resolved path lives under the project's managed worktree root + * (`//`). Everything else in the file (user-added + * folders, the project-root entry seeded on creation, `settings`, + * `extensions`, comments) is never touched, reordered, or rewritten. + */ + +export const CODE_WORKSPACE_EXTENSION = ".code-workspace"; + +// VS Code generates .code-workspace files with tab indentation; match it. +const MODIFY_OPTIONS: jsonc.ModificationOptions = { + formattingOptions: { insertSpaces: false, tabSize: 4, eol: "\n" }, +}; + +function readFolders(text: string): unknown[] | undefined { + const parsed = jsonc.parse(text, undefined, { allowTrailingComma: true }) as + | { folders?: unknown } + | undefined; + return Array.isArray(parsed?.folders) ? parsed.folders : undefined; +} + +// VS Code resolves relative folder paths against the .code-workspace file's +// directory; it does not expand `~` or variables, so neither do we. Such +// entries never match the managed root and stay untouched. +function getEntryPath(entry: unknown, workspaceFileDir: string): string | null { + if (typeof entry !== "object" || entry === null) { + return null; + } + const folderPath = (entry as { path?: unknown }).path; + if (typeof folderPath !== "string" || folderPath.trim() === "") { + return null; + } + return path.resolve(workspaceFileDir, folderPath); +} + +export interface CodeWorkspaceFileUpdate { + /** Absolute path of the .code-workspace file. */ + codeWorkspacePath: string; + /** Absolute directory; only folder entries under it are managed by xum. */ + managedRootDir: string; + /** Absolute worktree paths that should be present as folder entries. */ + desiredPaths: string[]; + /** Folder paths written when the file does not exist yet. */ + seedFolders: string[]; +} + +/** + * Reconcile the file's `folders` array with `desiredPaths` under the + * managed-entry invariant. Creates the file (with `seedFolders`) when missing. + * Uses jsonc-parser edits so user comments and unknown keys survive. + */ +export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): Promise { + const { codeWorkspacePath, managedRootDir, desiredPaths } = update; + const fileDir = path.dirname(codeWorkspacePath); + + let original: string | null; + try { + original = await fsPromises.readFile(codeWorkspacePath, "utf-8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + original = null; + } + + if (original === null) { + const fresh = { folders: update.seedFolders.map((folderPath) => ({ path: folderPath })) }; + await fsPromises.mkdir(fileDir, { recursive: true }); + await writeFileAtomic(codeWorkspacePath, JSON.stringify(fresh, null, "\t") + "\n"); + return; + } + + // Never clobber a file we cannot faithfully edit (self-healing over failing). + const parseErrors: jsonc.ParseError[] = []; + const parsed = jsonc.parse(original, parseErrors, { allowTrailingComma: true }) as unknown; + if ( + parseErrors.length > 0 || + typeof parsed !== "object" || + parsed === null || + Array.isArray(parsed) + ) { + log.warn("Skipping .code-workspace sync: file is not valid JSONC", { codeWorkspacePath }); + return; + } + const existingFolders = (parsed as { folders?: unknown }).folders; + if (existingFolders !== undefined && !Array.isArray(existingFolders)) { + log.warn("Skipping .code-workspace sync: 'folders' is not an array", { codeWorkspacePath }); + return; + } + + const desired = new Set(desiredPaths.map((desiredPath) => path.resolve(desiredPath))); + let text = original; + + if (existingFolders === undefined) { + text = jsonc.applyEdits(text, jsonc.modify(text, ["folders"], [], MODIFY_OPTIONS)); + } + + // Remove managed entries that are no longer desired, one edit at a time + // (indices shift after every removal, so re-parse between edits). + for (;;) { + const folders = readFolders(text) ?? []; + const removeIndex = folders.findIndex((entry) => { + const entryPath = getEntryPath(entry, fileDir); + return ( + entryPath !== null && isPathInsideDir(managedRootDir, entryPath) && !desired.has(entryPath) + ); + }); + if (removeIndex < 0) { + break; + } + text = jsonc.applyEdits( + text, + jsonc.modify(text, ["folders", removeIndex], undefined, MODIFY_OPTIONS) + ); + } + + // Append missing desired entries after the existing ones (user order is preserved). + const presentPaths = new Set( + (readFolders(text) ?? []) + .map((entry) => getEntryPath(entry, fileDir)) + .filter((entryPath): entryPath is string => entryPath !== null) + ); + const additions = [...desired].filter((desiredPath) => !presentPaths.has(desiredPath)).sort(); + for (const folderPath of additions) { + const length = (readFolders(text) ?? []).length; + text = jsonc.applyEdits( + text, + jsonc.modify( + text, + ["folders", length], + { path: folderPath }, + { + ...MODIFY_OPTIONS, + isArrayInsertion: true, + } + ) + ); + } + + if (text !== original) { + await writeFileAtomic(codeWorkspacePath, text); + } +} + +/** + * Compute the worktree paths that belong in a project's .code-workspace file: + * active (non-archived) top-level worktree workspaces under the managed root. + */ +export function computeManagedWorktreePaths(params: { + allMetadata: FrontendWorkspaceMetadata[]; + /** Normalized (no trailing slash) project path. */ + projectPath: string; + managedRootDir: string; +}): string[] { + const { allMetadata, projectPath, managedRootDir } = params; + const desired = new Set(); + for (const metadata of allMetadata) { + if (!isWorktreeRuntime(metadata.runtimeConfig)) { + continue; + } + // Sub-agent child workspaces are transient implementation detail; listing + // them would churn the user's editor window on every spawned task. + if (metadata.parentWorkspaceId) { + continue; + } + // isolation:"none" tasks share an ancestor checkout, not an own worktree. + if (metadata.taskIsolation === "none") { + continue; + } + if (isWorkspaceArchived(metadata.archivedAt, metadata.unarchivedAt)) { + continue; + } + + let worktreePath: string | null = null; + if (stripTrailingSlashes(metadata.projectPath) === projectPath) { + worktreePath = metadata.namedWorkspacePath; + } else if ( + metadata.projects?.some((ref) => stripTrailingSlashes(ref.projectPath) === projectPath) + ) { + // Multi-project workspaces persist only the primary checkout path; each + // secondary checkout lives at // + // (createMultiProject passes directoryName: workspaceName). + worktreePath = path.join(managedRootDir, metadata.name); + } + if (!worktreePath) { + continue; + } + const resolved = path.resolve(worktreePath); + // Only paths under the managed root are synced; anything else is user-owned. + if (!isPathInsideDir(managedRootDir, resolved)) { + continue; + } + desired.add(resolved); + } + return [...desired].sort(); +} + +/** + * Best-effort sync entry point used by workspace lifecycle operations and + * startup reconciliation. Fast no-op when the project has no + * `codeWorkspaceSyncPath` configured. Never throws: sync failures must never + * fail or block a workspace operation. + */ +export async function syncProjectCodeWorkspace(config: Config, projectPath: string): Promise { + try { + const normalizedProjectPath = stripTrailingSlashes(projectPath); + const projectConfig = config.loadConfigOrDefault().projects.get(normalizedProjectPath); + const rawSetting = projectConfig?.codeWorkspaceSyncPath?.trim(); + if (!rawSetting) { + return; + } + + // Relative settings resolve against the project root; `~` is expanded. + const codeWorkspacePath = path.resolve(normalizedProjectPath, expandTilde(rawSetting)); + if (!codeWorkspacePath.endsWith(CODE_WORKSPACE_EXTENSION)) { + // We read-modify-write this file, so never target arbitrary files. + log.warn("Skipping .code-workspace sync: path must end with .code-workspace", { + codeWorkspacePath, + }); + return; + } + + const managedRootDir = path.join( + expandTilde(config.srcDir), + getProjectName(normalizedProjectPath) + ); + const desiredPaths = computeManagedWorktreePaths({ + allMetadata: await config.getAllWorkspaceMetadata(), + projectPath: normalizedProjectPath, + managedRootDir, + }); + await updateCodeWorkspaceFile({ + codeWorkspacePath, + managedRootDir, + desiredPaths, + seedFolders: [normalizedProjectPath, ...desiredPaths], + }); + } catch (error) { + log.warn("Failed to sync .code-workspace file", { projectPath, error }); + } +} From ae0d83e53ff77774bc2b37bf6705ecadf76d2373 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 09:21:50 +0000 Subject: [PATCH 02/11] fix: surface the .code-workspace extension validation error to the UI UAT found the oRPC handler's plain Error mapped to a generic "Internal server error" toast; throw ORPCError(BAD_REQUEST) so the extension hint reaches the user. --- src/node/orpc/router.test.ts | 80 ++++++++++++++++++++++++++++++++++++ src/node/orpc/router.ts | 5 ++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/node/orpc/router.test.ts b/src/node/orpc/router.test.ts index cf52f8e81e5..d660c11b0b7 100644 --- a/src/node/orpc/router.test.ts +++ b/src/node/orpc/router.test.ts @@ -1142,3 +1142,83 @@ describe("router config.saveConfig", () => { expect(savedTaskSettings.proposePlanImplementReplacesChatHistory).toBe(true); }); }); + +describe("projects.setCodeWorkspaceSyncPath", () => { + let tempDir: string; + let config: Config; + let projectPath: string; + + beforeEach(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mux-router-codews-test-")); + config = new Config(tempDir); + projectPath = path.join(tempDir, "project"); + fs.mkdirSync(projectPath, { recursive: true }); + await config.editConfig((current) => { + current.projects.set(projectPath, { workspaces: [] }); + return current; + }); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + function createClient() { + return createRouterClient(router(), { + context: { config } as unknown as ORPCContext, + }); + } + + test("rejects paths without the .code-workspace extension with a client-visible error", async () => { + const client = createClient(); + let thrown: unknown; + try { + await client.projects.setCodeWorkspaceSyncPath({ + projectPath, + codeWorkspaceSyncPath: "/tmp/notes.txt", + }); + } catch (error) { + thrown = error; + } + // Must be an ORPCError so the extension hint reaches the UI instead of + // a generic "Internal server error". + expect(thrown).toBeInstanceOf(ORPCError); + expect((thrown as ORPCError).message).toContain(".code-workspace"); + expect( + config.loadConfigOrDefault().projects.get(projectPath)?.codeWorkspaceSyncPath + ).toBeUndefined(); + }); + + test("stores the path and writes the workspace file immediately", async () => { + const client = createClient(); + await client.projects.setCodeWorkspaceSyncPath({ + projectPath, + codeWorkspaceSyncPath: " proj.code-workspace ", + }); + + expect(config.loadConfigOrDefault().projects.get(projectPath)?.codeWorkspaceSyncPath).toBe( + "proj.code-workspace" + ); + const written = JSON.parse( + fs.readFileSync(path.join(projectPath, "proj.code-workspace"), "utf-8") + ) as { folders: Array<{ path: string }> }; + expect(written.folders).toEqual([{ path: projectPath }]); + }); + + test("clearing the setting removes it without deleting the existing file", async () => { + const client = createClient(); + await client.projects.setCodeWorkspaceSyncPath({ + projectPath, + codeWorkspaceSyncPath: "proj.code-workspace", + }); + await client.projects.setCodeWorkspaceSyncPath({ + projectPath, + codeWorkspaceSyncPath: null, + }); + + expect( + config.loadConfigOrDefault().projects.get(projectPath)?.codeWorkspaceSyncPath + ).toBeUndefined(); + expect(fs.existsSync(path.join(projectPath, "proj.code-workspace"))).toBe(true); + }); +}); diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index e7aeb838e49..15db97f9f67 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -3440,7 +3440,10 @@ export const router = (authToken?: string) => { const trimmed = input.codeWorkspaceSyncPath?.trim() ?? ""; if (trimmed && !trimmed.endsWith(CODE_WORKSPACE_EXTENSION)) { // The sync read-modify-writes this file, so refuse arbitrary targets. - throw new Error(`Path must end with ${CODE_WORKSPACE_EXTENSION}`); + // ORPCError so the message reaches the UI instead of "Internal server error". + throw new ORPCError("BAD_REQUEST", { + message: `Path must end with ${CODE_WORKSPACE_EXTENSION}`, + }); } await context.config.editConfig((config) => { const project = config.projects.get(normalizedPath); From 0602c0db98a075e9974b66e445fd270149474bf3 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 09:55:38 +0000 Subject: [PATCH 03/11] review: address Codex findings - derive managed roots from each workspace's runtimeConfig.srcBaseDir so custom/legacy roots (e.g. pre-rename ~/.mux/src) stay synced - derive per-project checkout paths for multi-project workspaces (primary included; namedWorkspacePath is the _workspaces symlink container) - serialize read-modify-write per canonical file path (two projects can share one file) - write through symlinked workspace files instead of replacing the link - bound startup reconciliation with raceWithAbortAndTimeout (10s) - inline the context method instead of manual useCallback (React Compiler) --- src/browser/contexts/ProjectContext.tsx | 32 ++-- src/node/services/workspaceService.ts | 22 ++- src/node/worktree/codeWorkspaceSync.test.ts | 145 +++++++++++++----- src/node/worktree/codeWorkspaceSync.ts | 154 +++++++++++++++----- 4 files changed, 264 insertions(+), 89 deletions(-) diff --git a/src/browser/contexts/ProjectContext.tsx b/src/browser/contexts/ProjectContext.tsx index 60b0c985081..9a74fd2968e 100644 --- a/src/browser/contexts/ProjectContext.tsx +++ b/src/browser/contexts/ProjectContext.tsx @@ -572,20 +572,6 @@ export function ProjectProvider(props: { children: ReactNode }) { [api, refreshProjects] ); - const updateCodeWorkspaceSyncPath = useCallback( - async (projectPath: string, codeWorkspaceSyncPath: string | null): Promise> => { - if (!api) return { success: false, error: "API not connected" }; - try { - await api.projects.setCodeWorkspaceSyncPath({ projectPath, codeWorkspaceSyncPath }); - await refreshProjects(); - return { success: true, data: undefined }; - } catch (error) { - return { success: false, error: getErrorMessage(error) }; - } - }, - [api, refreshProjects] - ); - const assignWorkspaceToSubProject = useCallback( async ( projectPath: string, @@ -639,7 +625,21 @@ export function ProjectProvider(props: { children: ReactNode }) { updateDisplayName, updateColor, updateCustomInstructions, - updateCodeWorkspaceSyncPath, + // Defined inline (not useCallback): the repo bans new manual useCallback + // memoization, and inlining keeps exhaustive-deps satisfied via `api`. + updateCodeWorkspaceSyncPath: async ( + projectPath: string, + codeWorkspaceSyncPath: string | null + ): Promise> => { + if (!api) return { success: false, error: "API not connected" }; + try { + await api.projects.setCodeWorkspaceSyncPath({ projectPath, codeWorkspaceSyncPath }); + await refreshProjects(); + return { success: true, data: undefined }; + } catch (error) { + return { success: false, error: getErrorMessage(error) }; + } + }, assignWorkspaceToSubProject, }), [ @@ -666,7 +666,7 @@ export function ProjectProvider(props: { children: ReactNode }) { updateDisplayName, updateColor, updateCustomInstructions, - updateCodeWorkspaceSyncPath, + api, assignWorkspaceToSubProject, ] ); diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index c885258e83c..7be7772ecd7 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -321,6 +321,9 @@ const MAX_WORKSPACE_NAME_COLLISION_RETRIES = 3; */ const ORPHAN_SESSION_DIR_GRACE_MS = 24 * 60 * 60 * 1000; +// Upper bound on startup .code-workspace reconciliation (see initialize()). +const STARTUP_CODE_WORKSPACE_SYNC_TIMEOUT_MS = 10_000; + /** * Base name used when /new auto-generates a branch name. Numbered suffixes * (`workspace-1`, `workspace-2`, ...) come from {@link generateForkBranchName} @@ -2909,12 +2912,21 @@ export class WorkspaceService extends EventEmitter { } // Repair .code-workspace drift from lifecycle changes that happened while - // the app was not running (best-effort; syncProjectCodeWorkspace never throws). - for (const [projectPath, projectConfig] of this.config.loadConfigOrDefault().projects) { - if (projectConfig.codeWorkspaceSyncPath?.trim()) { - await syncProjectCodeWorkspace(this.config, projectPath); + // the app was not running. Bounded: a stalled filesystem (e.g. an + // unreachable network mount holding a configured file) must never block + // startup. Past the deadline the loop keeps running in the background; + // syncProjectCodeWorkspace never throws, so the orphaned promise cannot + // reject unhandled. + const codeWorkspaceSyncAll = (async () => { + for (const [projectPath, projectConfig] of this.config.loadConfigOrDefault().projects) { + if (projectConfig.codeWorkspaceSyncPath?.trim()) { + await syncProjectCodeWorkspace(this.config, projectPath); + } } - } + })(); + await raceWithAbortAndTimeout(codeWorkspaceSyncAll, { + timeoutMs: STARTUP_CODE_WORKSPACE_SYNC_TIMEOUT_MS, + }); log.info("[startup] WorkspaceService.initialize completed", { totalMs: Date.now() - startupStartedAt, diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index d61526d1e88..148665f9b9b 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -39,7 +39,7 @@ describe("updateCodeWorkspaceFile", () => { const worktree = path.join(managedRootDir, "feature-a"); await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [worktree], seedFolders: [path.join(tempDir, "project"), worktree], }); @@ -58,7 +58,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [existing, added], seedFolders: [], }); @@ -77,7 +77,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [kept], seedFolders: [], }); @@ -98,7 +98,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [], seedFolders: [], }); @@ -128,7 +128,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [kept, added], seedFolders: [], }); @@ -149,7 +149,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [worktree], seedFolders: [], }); @@ -163,7 +163,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [path.join(managedRootDir, "feature-a")], seedFolders: [], }); @@ -175,7 +175,7 @@ describe("updateCodeWorkspaceFile", () => { const worktree = path.join(managedRootDir, "feature-a"); const update = { codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [worktree], seedFolders: [worktree], }; @@ -202,7 +202,7 @@ describe("updateCodeWorkspaceFile", () => { // other project's entry. await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [], seedFolders: [], }); @@ -210,6 +210,54 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: theirs }]); }); + test("writes through a symlinked workspace file without replacing the link", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + const realFile = path.join(tempDir, "shared-config", "real.code-workspace"); + await fsPromises.mkdir(path.dirname(realFile), { recursive: true }); + await fsPromises.writeFile(realFile, JSON.stringify({ folders: [] })); + const linkPath = path.join(tempDir, "linked.code-workspace"); + await fsPromises.symlink(realFile, linkPath); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: linkPath, + managedRootDirs: [managedRootDir], + desiredPaths: [worktree], + seedFolders: [], + }); + + expect((await fsPromises.lstat(linkPath)).isSymbolicLink()).toBe(true); + expect(parseFolders(await fsPromises.readFile(realFile, "utf-8"))).toEqual([ + { path: worktree }, + ]); + }); + + test("serializes concurrent updates to the same file", async () => { + await fsPromises.writeFile(workspaceFilePath, JSON.stringify({ folders: [] })); + const otherRoot = path.join(tempDir, "src", "other-project"); + const mine = path.join(managedRootDir, "feature-a"); + const theirs = path.join(otherRoot, "feature-x"); + + // Two projects sharing one file sync concurrently; without per-file + // serialization one write clobbers the other's addition. + await Promise.all([ + updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDirs: [managedRootDir], + desiredPaths: [mine], + seedFolders: [], + }), + updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDirs: [otherRoot], + desiredPaths: [theirs], + seedFolders: [], + }), + ]); + + const folders = parseFolders(await readWorkspaceFile()); + expect(folders.map((entry) => entry.path).sort()).toEqual([mine, theirs].sort()); + }); + test("resolves relative folder entries against the file's directory", async () => { // Entry is relative but points inside the managed root; it must count as // present (no duplicate added) and be removable when undesired. @@ -221,7 +269,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [path.join(managedRootDir, "feature-a")], seedFolders: [], }); @@ -229,7 +277,7 @@ describe("updateCodeWorkspaceFile", () => { await updateCodeWorkspaceFile({ codeWorkspacePath: workspaceFilePath, - managedRootDir, + managedRootDirs: [managedRootDir], desiredPaths: [], seedFolders: [], }); @@ -309,22 +357,26 @@ describe("computeManagedWorktreePaths", () => { } const managedRoot = "/base/src/my-project"; + const computeParams = { + projectPath, + projectName: "my-project", + defaultManagedRootDir: managedRoot, + }; test("includes active worktree workspaces and sorts deduped paths", () => { - const paths = computeManagedWorktreePaths({ + const { desiredPaths } = computeManagedWorktreePaths({ allMetadata: [ makeMetadata({ name: "b", namedWorkspacePath: `${managedRoot}/b` }), makeMetadata({ name: "a", namedWorkspacePath: `${managedRoot}/a` }), makeMetadata({ name: "a-dup", namedWorkspacePath: `${managedRoot}/a` }), ], - projectPath, - managedRootDir: managedRoot, + ...computeParams, }); - expect(paths).toEqual([`${managedRoot}/a`, `${managedRoot}/b`]); + expect(desiredPaths).toEqual([`${managedRoot}/a`, `${managedRoot}/b`]); }); test("excludes archived, sub-agent, isolation-none, other-project, and out-of-root workspaces", () => { - const paths = computeManagedWorktreePaths({ + const { desiredPaths } = computeManagedWorktreePaths({ allMetadata: [ makeMetadata({ archivedAt: "2026-01-02T00:00:00Z" }), makeMetadata({ parentWorkspaceId: "parent1234" }), @@ -333,41 +385,68 @@ describe("computeManagedWorktreePaths", () => { makeMetadata({ namedWorkspacePath: "/elsewhere/feature-a" }), makeMetadata({ runtimeConfig: { type: "local" } }), ], - projectPath, - managedRootDir: managedRoot, + ...computeParams, }); - expect(paths).toEqual([]); + expect(desiredPaths).toEqual([]); }); test("re-includes unarchived workspaces", () => { - const paths = computeManagedWorktreePaths({ + const { desiredPaths } = computeManagedWorktreePaths({ allMetadata: [ makeMetadata({ archivedAt: "2026-01-01T00:00:00Z", unarchivedAt: "2026-01-02T00:00:00Z", }), ], - projectPath, - managedRootDir: managedRoot, + ...computeParams, + }); + expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); + }); + + test("keeps worktrees under a custom or legacy srcBaseDir managed", () => { + // Legacy "local"-with-srcBaseDir runtime rooted somewhere other than the + // current global srcDir (e.g. a pre-rename ~/.mux/src) must still sync. + const { desiredPaths, managedRootDirs } = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ + namedWorkspacePath: "/legacy/src/my-project/feature-a", + runtimeConfig: { type: "local", srcBaseDir: "/legacy/src" }, + }), + ], + ...computeParams, }); - expect(paths).toEqual([`${managedRoot}/feature-a`]); + expect(desiredPaths).toEqual(["/legacy/src/my-project/feature-a"]); + expect(managedRootDirs).toEqual(["/base/src/my-project", "/legacy/src/my-project"]); }); - test("derives secondary checkout paths for multi-project workspaces", () => { - const paths = computeManagedWorktreePaths({ + test("derives per-project checkout paths for multi-project workspaces", () => { + const multiProjects = [ + { projectPath: "/home/user/projects/primary", projectName: "primary" }, + { projectPath, projectName: "my-project" }, + ]; + // namedWorkspacePath for multi-project workspaces is the _workspaces/ + // symlink container, never a real checkout, for primary and secondary alike. + const asSecondary = computeManagedWorktreePaths({ allMetadata: [ makeMetadata({ projectPath: "/home/user/projects/primary", - namedWorkspacePath: "/base/src/primary/feature-a", - projects: [ - { projectPath: "/home/user/projects/primary", projectName: "primary" }, - { projectPath, projectName: "my-project" }, - ], + namedWorkspacePath: "/base/src/_workspaces/feature-a", + projects: multiProjects, }), ], - projectPath, - managedRootDir: managedRoot, + ...computeParams, + }); + expect(asSecondary.desiredPaths).toEqual([`${managedRoot}/feature-a`]); + + const asPrimary = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ + namedWorkspacePath: "/base/src/_workspaces/feature-a", + projects: multiProjects, + }), + ], + ...computeParams, }); - expect(paths).toEqual([`${managedRoot}/feature-a`]); + expect(asPrimary.desiredPaths).toEqual([`${managedRoot}/feature-a`]); }); }); diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index e3b0d18a924..1ef20f1798c 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -3,6 +3,7 @@ import * as path from "path"; import * as jsonc from "jsonc-parser"; import writeFileAtomic from "write-file-atomic"; import { isWorkspaceArchived } from "@/common/utils/archive"; +import { isMultiProject } from "@/common/utils/multiProject"; import { isWorktreeRuntime } from "@/common/types/runtime"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import type { Config } from "@/node/config"; @@ -30,6 +31,46 @@ const MODIFY_OPTIONS: jsonc.ModificationOptions = { formattingOptions: { insertSpaces: false, tabSize: 4, eol: "\n" }, }; +// Serialize read-modify-write per canonical file path: two projects may share +// one .code-workspace file, and concurrent lifecycle syncs would otherwise +// lose updates (the later write wins over a stale read). +const fileWriteQueues = new Map>(); + +async function withFileWriteLock(key: string, fn: () => Promise): Promise { + const prev = fileWriteQueues.get(key) ?? Promise.resolve(); + const run = prev.then(fn); + const tail: Promise = run + .then( + () => undefined, + () => undefined + ) + .then(() => { + if (fileWriteQueues.get(key) === tail) { + fileWriteQueues.delete(key); + } + }); + fileWriteQueues.set(key, tail); + return run; +} + +// write-file-atomic renames a temp file over the target, which would replace a +// symlink rather than write through it; resolve the real target first so user +// symlinks to shared editor config survive syncs. +async function resolveRealPath(filePath: string): Promise { + try { + return await fsPromises.realpath(filePath); + } catch { + // File may not exist yet; resolve the parent so directory symlinks are + // still honored, keeping the configured basename. + try { + const realDir = await fsPromises.realpath(path.dirname(filePath)); + return path.join(realDir, path.basename(filePath)); + } catch { + return filePath; + } + } +} + function readFolders(text: string): unknown[] | undefined { const parsed = jsonc.parse(text, undefined, { allowTrailingComma: true }) as | { folders?: unknown } @@ -54,26 +95,40 @@ function getEntryPath(entry: unknown, workspaceFileDir: string): string | null { export interface CodeWorkspaceFileUpdate { /** Absolute path of the .code-workspace file. */ codeWorkspacePath: string; - /** Absolute directory; only folder entries under it are managed by xum. */ - managedRootDir: string; + /** Absolute directories; only folder entries under one of them are managed by xum. */ + managedRootDirs: string[]; /** Absolute worktree paths that should be present as folder entries. */ desiredPaths: string[]; /** Folder paths written when the file does not exist yet. */ seedFolders: string[]; } +function isUnderAnyRoot(managedRootDirs: string[], candidate: string): boolean { + return managedRootDirs.some((rootDir) => isPathInsideDir(rootDir, candidate)); +} + /** * Reconcile the file's `folders` array with `desiredPaths` under the * managed-entry invariant. Creates the file (with `seedFolders`) when missing. * Uses jsonc-parser edits so user comments and unknown keys survive. */ export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): Promise { - const { codeWorkspacePath, managedRootDir, desiredPaths } = update; + const targetPath = await resolveRealPath(update.codeWorkspacePath); + await withFileWriteLock(targetPath, () => updateCodeWorkspaceFileLocked(targetPath, update)); +} + +async function updateCodeWorkspaceFileLocked( + targetPath: string, + update: CodeWorkspaceFileUpdate +): Promise { + const { codeWorkspacePath, managedRootDirs, desiredPaths } = update; + // Relative folder entries resolve against the configured file location, + // matching how VS Code resolves them for the file the user opens. const fileDir = path.dirname(codeWorkspacePath); let original: string | null; try { - original = await fsPromises.readFile(codeWorkspacePath, "utf-8"); + original = await fsPromises.readFile(targetPath, "utf-8"); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") { throw error; @@ -83,8 +138,8 @@ export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): if (original === null) { const fresh = { folders: update.seedFolders.map((folderPath) => ({ path: folderPath })) }; - await fsPromises.mkdir(fileDir, { recursive: true }); - await writeFileAtomic(codeWorkspacePath, JSON.stringify(fresh, null, "\t") + "\n"); + await fsPromises.mkdir(path.dirname(targetPath), { recursive: true }); + await writeFileAtomic(targetPath, JSON.stringify(fresh, null, "\t") + "\n"); return; } @@ -100,6 +155,7 @@ export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): log.warn("Skipping .code-workspace sync: file is not valid JSONC", { codeWorkspacePath }); return; } + const existingFolders = (parsed as { folders?: unknown }).folders; if (existingFolders !== undefined && !Array.isArray(existingFolders)) { log.warn("Skipping .code-workspace sync: 'folders' is not an array", { codeWorkspacePath }); @@ -120,7 +176,7 @@ export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): const removeIndex = folders.findIndex((entry) => { const entryPath = getEntryPath(entry, fileDir); return ( - entryPath !== null && isPathInsideDir(managedRootDir, entryPath) && !desired.has(entryPath) + entryPath !== null && isUnderAnyRoot(managedRootDirs, entryPath) && !desired.has(entryPath) ); }); if (removeIndex < 0) { @@ -156,21 +212,47 @@ export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): } if (text !== original) { - await writeFileAtomic(codeWorkspacePath, text); + await writeFileAtomic(targetPath, text); } } +function belongsToProject(metadata: FrontendWorkspaceMetadata, projectPath: string): boolean { + return ( + stripTrailingSlashes(metadata.projectPath) === projectPath || + (metadata.projects?.some((ref) => stripTrailingSlashes(ref.projectPath) === projectPath) ?? + false) + ); +} + /** - * Compute the worktree paths that belong in a project's .code-workspace file: - * active (non-archived) top-level worktree workspaces under the managed root. + * Compute the worktree paths that belong in a project's .code-workspace file + * (active, top-level worktree workspaces) plus the managed root directories + * that scope which existing entries xum may remove. */ export function computeManagedWorktreePaths(params: { allMetadata: FrontendWorkspaceMetadata[]; /** Normalized (no trailing slash) project path. */ projectPath: string; - managedRootDir: string; -}): string[] { - const { allMetadata, projectPath, managedRootDir } = params; + projectName: string; + defaultManagedRootDir: string; +}): { desiredPaths: string[]; managedRootDirs: string[] } { + const { allMetadata, projectPath, projectName, defaultManagedRootDir } = params; + + // Managed roots come from every persisted worktree workspace of the project + // (any lifecycle state), not just the current global srcDir: worktrees + // created under a custom or legacy srcBaseDir (e.g. pre-rename ~/.mux/src) + // must stay managed after upgrades. Residual: an entry under a custom root + // whose last workspace was deleted is no longer classified as managed and + // can linger until removed by hand. + const roots = new Set([path.resolve(defaultManagedRootDir)]); + for (const metadata of allMetadata) { + if (!belongsToProject(metadata, projectPath) || !isWorktreeRuntime(metadata.runtimeConfig)) { + continue; + } + roots.add(path.resolve(path.join(expandTilde(metadata.runtimeConfig.srcBaseDir), projectName))); + } + const managedRootDirs = [...roots].sort(); + const desired = new Set(); for (const metadata of allMetadata) { if (!isWorktreeRuntime(metadata.runtimeConfig)) { @@ -188,29 +270,33 @@ export function computeManagedWorktreePaths(params: { if (isWorkspaceArchived(metadata.archivedAt, metadata.unarchivedAt)) { continue; } + if (!belongsToProject(metadata, projectPath)) { + continue; + } - let worktreePath: string | null = null; - if (stripTrailingSlashes(metadata.projectPath) === projectPath) { + let worktreePath: string; + if (isMultiProject(metadata)) { + // Multi-project workspaces persist the _workspaces/ symlink + // container as namedWorkspacePath; the real per-project checkout lives + // at // for the primary and + // secondary projects alike (createMultiProject passes + // directoryName: workspaceName). + worktreePath = path.join( + expandTilde(metadata.runtimeConfig.srcBaseDir), + projectName, + metadata.name + ); + } else { worktreePath = metadata.namedWorkspacePath; - } else if ( - metadata.projects?.some((ref) => stripTrailingSlashes(ref.projectPath) === projectPath) - ) { - // Multi-project workspaces persist only the primary checkout path; each - // secondary checkout lives at // - // (createMultiProject passes directoryName: workspaceName). - worktreePath = path.join(managedRootDir, metadata.name); - } - if (!worktreePath) { - continue; } const resolved = path.resolve(worktreePath); - // Only paths under the managed root are synced; anything else is user-owned. - if (!isPathInsideDir(managedRootDir, resolved)) { + // Only paths under a managed root are synced; anything else is user-owned. + if (!isUnderAnyRoot(managedRootDirs, resolved)) { continue; } desired.add(resolved); } - return [...desired].sort(); + return { desiredPaths: [...desired].sort(), managedRootDirs }; } /** @@ -238,18 +324,16 @@ export async function syncProjectCodeWorkspace(config: Config, projectPath: stri return; } - const managedRootDir = path.join( - expandTilde(config.srcDir), - getProjectName(normalizedProjectPath) - ); - const desiredPaths = computeManagedWorktreePaths({ + const projectName = getProjectName(normalizedProjectPath); + const { desiredPaths, managedRootDirs } = computeManagedWorktreePaths({ allMetadata: await config.getAllWorkspaceMetadata(), projectPath: normalizedProjectPath, - managedRootDir, + projectName, + defaultManagedRootDir: path.join(expandTilde(config.srcDir), projectName), }); await updateCodeWorkspaceFile({ codeWorkspacePath, - managedRootDir, + managedRootDirs, desiredPaths, seedFolders: [normalizedProjectPath, ...desiredPaths], }); From 1042b0d82531c1320172a9c0937efe23fa9570f1 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:31:22 +0000 Subject: [PATCH 04/11] review: address Codex round-2 findings - bound every sync internally (raceWithAbortAndTimeout, 10s) so lifecycle operations cannot hang on a stalled filesystem; startup keeps its outer cap - capture a deleted workspace's managed roots before config removal so custom/legacy-root entries are still cleaned up - sanitize non-string codeWorkspaceSyncPath at config load (matches customInstructions handling) so projects.list cannot be bricked - reconcile all projects targeting the same file together so same-basename projects sharing a managed root cannot erase each other's entries - cap file size (1 MiB) before the synchronous JSONC parse --- src/node/config.test.ts | 22 +++ src/node/config.ts | 8 ++ src/node/services/workspaceService.ts | 33 +++-- src/node/worktree/codeWorkspaceSync.test.ts | 82 +++++++++++ src/node/worktree/codeWorkspaceSync.ts | 152 ++++++++++++++++---- 5 files changed, 261 insertions(+), 36 deletions(-) diff --git a/src/node/config.test.ts b/src/node/config.test.ts index c990d0fefd0..d624917816c 100644 --- a/src/node/config.test.ts +++ b/src/node/config.test.ts @@ -729,6 +729,28 @@ describe("Config", () => { "Keep this guidance." ); }); + + it("discards malformed non-string codeWorkspaceSyncPath and keeps valid ones", () => { + const configFile = path.join(tempDir, "config.json"); + fs.writeFileSync( + configFile, + JSON.stringify({ + projects: [ + ["/home/user/number", { workspaces: [], codeWorkspaceSyncPath: 42 }], + ["/home/user/blank", { workspaces: [], codeWorkspaceSyncPath: " " }], + ["/home/user/valid", { workspaces: [], codeWorkspaceSyncPath: "a.code-workspace" }], + ], + }) + ); + + const loaded = config.loadConfigOrDefault(); + + expect(loaded.projects.get("/home/user/number")?.codeWorkspaceSyncPath).toBeUndefined(); + expect(loaded.projects.get("/home/user/blank")?.codeWorkspaceSyncPath).toBeUndefined(); + expect(loaded.projects.get("/home/user/valid")?.codeWorkspaceSyncPath).toBe( + "a.code-workspace" + ); + }); }); describe("legacy workflow schedule cleanup", () => { diff --git a/src/node/config.ts b/src/node/config.ts index 358a9780152..ab3f6882f80 100644 --- a/src/node/config.ts +++ b/src/node/config.ts @@ -681,6 +681,7 @@ function normalizeProjectRuntimeSettings(projectConfig: ProjectConfig): ProjectC runtimeOverridesEnabled?: unknown; projectKind?: unknown; customInstructions?: unknown; + codeWorkspaceSyncPath?: unknown; }; const runtimeEnablement = normalizeRuntimeEnablementOverrides(record.runtimeEnablement); const defaultRuntime = normalizeRuntimeEnablementId(record.defaultRuntime); @@ -725,6 +726,13 @@ function normalizeProjectRuntimeSettings(projectConfig: ProjectConfig): ProjectC delete next.customInstructions; } + // Same hand-edit hazard as customInstructions above. + if (typeof record.codeWorkspaceSyncPath === "string" && record.codeWorkspaceSyncPath.trim()) { + next.codeWorkspaceSyncPath = record.codeWorkspaceSyncPath; + } else { + delete next.codeWorkspaceSyncPath; + } + // Legacy named workflow schedules are intentionally dropped while workflow // scheduling is disabled during the explicit script_path migration. delete (next as ProjectConfig & { workflowSchedules?: unknown }).workflowSchedules; diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 7be7772ecd7..da395956f45 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -116,7 +116,10 @@ import { import { isWorktreeRuntime } from "@/node/runtime/worktreeLifecycleHooks"; import { expandTilde, expandTildeForSSH } from "@/node/runtime/tildeExpansion"; import { removeManagedGitWorktree } from "@/node/worktree/removeManagedGitWorktree"; -import { syncProjectCodeWorkspace } from "@/node/worktree/codeWorkspaceSync"; +import { + managedRootsForWorkspace, + syncProjectCodeWorkspace, +} from "@/node/worktree/codeWorkspaceSync"; import { copyStagedWorkspaceAttachments, @@ -2912,11 +2915,11 @@ export class WorkspaceService extends EventEmitter { } // Repair .code-workspace drift from lifecycle changes that happened while - // the app was not running. Bounded: a stalled filesystem (e.g. an - // unreachable network mount holding a configured file) must never block - // startup. Past the deadline the loop keeps running in the background; - // syncProjectCodeWorkspace never throws, so the orphaned promise cannot - // reject unhandled. + // the app was not running. Each sync is internally bounded, but startup + // additionally caps the whole loop: many enabled projects on a stalled + // filesystem must never delay launch. Past the deadline the loop keeps + // running in the background; syncProjectCodeWorkspace never throws, so + // the orphaned promise cannot reject unhandled. const codeWorkspaceSyncAll = (async () => { for (const [projectPath, projectConfig] of this.config.loadConfigOrDefault().projects) { if (projectConfig.codeWorkspaceSyncPath?.trim()) { @@ -5396,6 +5399,16 @@ export class WorkspaceService extends EventEmitter { this.terminalService?.closeWorkspaceSessions(workspaceId); await this.closeDesktopSessionBestEffort(workspaceId, "remove"); + // Capture managed roots before the config entry disappears: a worktree + // under a custom/legacy srcBaseDir cannot be reconstructed afterwards, + // which would leave its folder entry in the .code-workspace file forever. + const removedMetadata = (await this.config.getAllWorkspaceMetadata()).find( + (m) => m.id === workspaceId + ); + const removedWorkspaceRoots = removedMetadata + ? managedRootsForWorkspace(removedMetadata) + : []; + // Remove from config await this.config.removeWorkspace(workspaceId); removedFromConfig = true; @@ -5404,7 +5417,8 @@ export class WorkspaceService extends EventEmitter { if (persistedWorkspace) { await this.syncCodeWorkspaceFiles( persistedWorkspace.projectPath, - persistedWorkspace.projects + persistedWorkspace.projects, + removedWorkspaceRoots ); } @@ -5435,11 +5449,12 @@ export class WorkspaceService extends EventEmitter { */ private async syncCodeWorkspaceFiles( projectPath: string, - projects?: ReadonlyArray<{ projectPath: string }> + projects?: ReadonlyArray<{ projectPath: string }>, + extraManagedRootDirs?: string[] ): Promise { const involvedPaths = new Set([projectPath, ...(projects ?? []).map((ref) => ref.projectPath)]); for (const involvedPath of involvedPaths) { - await syncProjectCodeWorkspace(this.config, involvedPath); + await syncProjectCodeWorkspace(this.config, involvedPath, { extraManagedRootDirs }); } } diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index 148665f9b9b..d726eb2e128 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -6,6 +6,7 @@ import * as jsonc from "jsonc-parser"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { Config } from "@/node/config"; import { + MAX_CODE_WORKSPACE_FILE_BYTES, computeManagedWorktreePaths, syncProjectCodeWorkspace, updateCodeWorkspaceFile, @@ -157,6 +158,22 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: worktree }]); }); + test("skips files exceeding the size cap without rewriting them", async () => { + // jsonc.parse is synchronous, so oversized (potentially repo-controlled) + // files must be rejected before parsing. + const oversized = `{"folders": [], "pad": "${"x".repeat(MAX_CODE_WORKSPACE_FILE_BYTES + 1)}"}`; + await fsPromises.writeFile(workspaceFilePath, oversized); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: workspaceFilePath, + managedRootDirs: [managedRootDir], + desiredPaths: [path.join(managedRootDir, "feature-a")], + seedFolders: [], + }); + + expect(await readWorkspaceFile()).toBe(oversized); + }); + test("leaves a malformed file untouched without throwing", async () => { const malformed = '{ "folders": [ { "path": broken '; await fsPromises.writeFile(workspaceFilePath, malformed); @@ -326,6 +343,71 @@ describe("syncProjectCodeWorkspace", () => { expect((await fsPromises.readdir(projectPath)).length).toBe(0); }); + test("unions projects that target the same file so they cannot erase each other", async () => { + // Same-basename projects share one managed root (/repo); a sync + // scoped to only one project would remove the other's entries. + const config = new Config(tempDir); + const projectA = path.join(tempDir, "a", "repo"); + const projectB = path.join(tempDir, "b", "repo"); + await fsPromises.mkdir(projectA, { recursive: true }); + await fsPromises.mkdir(projectB, { recursive: true }); + const sharedFile = path.join(tempDir, "shared.code-workspace"); + const worktreeA = path.join(config.srcDir, "repo", "feat-a"); + const worktreeB = path.join(config.srcDir, "repo", "feat-b"); + const workspaceEntry = (worktree: string, id: string) => ({ + path: worktree, + id, + name: path.basename(worktree), + runtimeConfig: { type: "worktree" as const, srcBaseDir: config.srcDir }, + }); + await config.editConfig((cfg) => { + cfg.projects.set(projectA, { + workspaces: [workspaceEntry(worktreeA, "aaaaaaaaaa")], + codeWorkspaceSyncPath: sharedFile, + }); + cfg.projects.set(projectB, { + workspaces: [workspaceEntry(worktreeB, "bbbbbbbbbb")], + codeWorkspaceSyncPath: sharedFile, + }); + return cfg; + }); + + await syncProjectCodeWorkspace(config, projectA); + await syncProjectCodeWorkspace(config, projectB); + + const folders = parseFolders(await fsPromises.readFile(sharedFile, "utf-8")); + const folderPaths = folders.map((entry) => entry.path); + expect(folderPaths).toContain(worktreeA); + expect(folderPaths).toContain(worktreeB); + }); + + test("removes stale entries under extra managed roots after their workspace is gone", async () => { + // Deleting the last workspace under a custom/legacy srcBaseDir removes the + // metadata that reconstructed its root; callers pass the captured root so + // the deleted checkout's entry still gets cleaned up. + const config = new Config(tempDir); + const projectPath = path.join(tempDir, "repo"); + await fsPromises.mkdir(projectPath, { recursive: true }); + const legacyRoot = path.join(tempDir, "legacy-src", "repo"); + const staleEntry = path.join(legacyRoot, "deleted-feature"); + const file = path.join(projectPath, "repo.code-workspace"); + await fsPromises.writeFile( + file, + JSON.stringify({ folders: [{ path: projectPath }, { path: staleEntry }] }) + ); + await config.editConfig((cfg) => { + cfg.projects.set(projectPath, { + workspaces: [], + codeWorkspaceSyncPath: "repo.code-workspace", + }); + return cfg; + }); + + await syncProjectCodeWorkspace(config, projectPath, { extraManagedRootDirs: [legacyRoot] }); + + expect(parseFolders(await fsPromises.readFile(file, "utf-8"))).toEqual([{ path: projectPath }]); + }); + test("refuses paths without the .code-workspace extension", async () => { const config = new Config(tempDir); const projectPath = path.join(tempDir, "repo"); diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index 1ef20f1798c..95cd3f25586 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -9,6 +9,7 @@ import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import type { Config } from "@/node/config"; import { expandTilde } from "@/node/runtime/tildeExpansion"; import { log } from "@/node/services/log"; +import { raceWithAbortAndTimeout } from "@/node/utils/concurrency/withTimeout"; import { isPathInsideDir, stripTrailingSlashes } from "@/node/utils/pathUtils"; import { getProjectName } from "@/node/utils/runtime/helpers"; @@ -26,6 +27,15 @@ import { getProjectName } from "@/node/utils/runtime/helpers"; export const CODE_WORKSPACE_EXTENSION = ".code-workspace"; +// The target file can be repository- or user-controlled, and jsonc.parse is +// synchronous (no timeout can preempt it), so cap the bytes we are willing to +// read and parse. Real .code-workspace files are a few KiB. +export const MAX_CODE_WORKSPACE_FILE_BYTES = 1024 * 1024; + +// Bound each sync: a configured file on an unavailable network mount can hang +// fs calls indefinitely, and workspace lifecycle operations await syncs. +const SYNC_TIMEOUT_MS = 10_000; + // VS Code generates .code-workspace files with tab indentation; match it. const MODIFY_OPTIONS: jsonc.ModificationOptions = { formattingOptions: { insertSpaces: false, tabSize: 4, eol: "\n" }, @@ -128,6 +138,14 @@ async function updateCodeWorkspaceFileLocked( let original: string | null; try { + const stats = await fsPromises.stat(targetPath); + if (stats.size > MAX_CODE_WORKSPACE_FILE_BYTES) { + log.warn("Skipping .code-workspace sync: file exceeds size limit", { + codeWorkspacePath, + sizeBytes: stats.size, + }); + return; + } original = await fsPromises.readFile(targetPath, "utf-8"); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") { @@ -299,44 +317,124 @@ export function computeManagedWorktreePaths(params: { return { desiredPaths: [...desired].sort(), managedRootDirs }; } +/** + * Managed roots contributed by one workspace, computed from its own runtime + * config. Callers capture this BEFORE deleting a workspace: once its config + * entry is gone, a custom/legacy srcBaseDir root can no longer be + * reconstructed and the deleted checkout's folder entry would linger forever. + */ +export function managedRootsForWorkspace( + metadata: Pick +): string[] { + if (!isWorktreeRuntime(metadata.runtimeConfig)) { + return []; + } + const srcBaseDir = expandTilde(metadata.runtimeConfig.srcBaseDir); + const involved = new Set([ + metadata.projectPath, + ...(metadata.projects ?? []).map((ref) => ref.projectPath), + ]); + return [...involved].map((involvedPath) => + path.resolve(path.join(srcBaseDir, getProjectName(stripTrailingSlashes(involvedPath)))) + ); +} + +// Resolve a project's configured setting to an absolute target path, or null +// when unset or invalid. Relative settings resolve against the project root; +// `~` is expanded. +function resolveConfiguredCodeWorkspacePath( + projectPath: string, + setting: string | undefined +): string | null { + const raw = setting?.trim(); + if (!raw) { + return null; + } + const codeWorkspacePath = path.resolve(projectPath, expandTilde(raw)); + if (!codeWorkspacePath.endsWith(CODE_WORKSPACE_EXTENSION)) { + // We read-modify-write this file, so never target arbitrary files. + log.warn("Skipping .code-workspace sync: path must end with .code-workspace", { + codeWorkspacePath, + }); + return null; + } + return codeWorkspacePath; +} + /** * Best-effort sync entry point used by workspace lifecycle operations and * startup reconciliation. Fast no-op when the project has no - * `codeWorkspaceSyncPath` configured. Never throws: sync failures must never - * fail or block a workspace operation. + * `codeWorkspaceSyncPath` configured. Never throws and is bounded by + * SYNC_TIMEOUT_MS: sync failures or stalled filesystems must never fail or + * block a workspace operation. */ -export async function syncProjectCodeWorkspace(config: Config, projectPath: string): Promise { +export async function syncProjectCodeWorkspace( + config: Config, + projectPath: string, + options?: { extraManagedRootDirs?: string[] } +): Promise { try { const normalizedProjectPath = stripTrailingSlashes(projectPath); - const projectConfig = config.loadConfigOrDefault().projects.get(normalizedProjectPath); - const rawSetting = projectConfig?.codeWorkspaceSyncPath?.trim(); - if (!rawSetting) { + const projects = config.loadConfigOrDefault().projects; + const targetFile = resolveConfiguredCodeWorkspacePath( + normalizedProjectPath, + projects.get(normalizedProjectPath)?.codeWorkspaceSyncPath + ); + if (!targetFile) { return; } - // Relative settings resolve against the project root; `~` is expanded. - const codeWorkspacePath = path.resolve(normalizedProjectPath, expandTilde(rawSetting)); - if (!codeWorkspacePath.endsWith(CODE_WORKSPACE_EXTENSION)) { - // We read-modify-write this file, so never target arbitrary files. - log.warn("Skipping .code-workspace sync: path must end with .code-workspace", { - codeWorkspacePath, + // The work never rejects (errors are logged inside), so a timeout that + // orphans it cannot leave an unhandled rejection behind. + const work = (async () => { + try { + // Reconcile every project targeting this file together. Projects can + // share a file, and same-basename projects even share a managed root; + // independent per-project removals would erase each other's entries. + const allMetadata = await config.getAllWorkspaceMetadata(); + const desired = new Set(); + const roots = new Set( + (options?.extraManagedRootDirs ?? []).map((rootDir) => path.resolve(rootDir)) + ); + const participantPaths: string[] = []; + for (const [participantPath, participantConfig] of projects) { + if ( + resolveConfiguredCodeWorkspacePath( + participantPath, + participantConfig.codeWorkspaceSyncPath + ) !== targetFile + ) { + continue; + } + const projectName = getProjectName(participantPath); + const computed = computeManagedWorktreePaths({ + allMetadata, + projectPath: participantPath, + projectName, + defaultManagedRootDir: path.join(expandTilde(config.srcDir), projectName), + }); + participantPaths.push(participantPath); + computed.desiredPaths.forEach((desiredPath) => desired.add(desiredPath)); + computed.managedRootDirs.forEach((rootDir) => roots.add(rootDir)); + } + const desiredPaths = [...desired].sort(); + await updateCodeWorkspaceFile({ + codeWorkspacePath: targetFile, + managedRootDirs: [...roots].sort(), + desiredPaths, + seedFolders: [...participantPaths, ...desiredPaths], + }); + } catch (error) { + log.warn("Failed to sync .code-workspace file", { projectPath, error }); + } + })(); + + const outcome = await raceWithAbortAndTimeout(work, { timeoutMs: SYNC_TIMEOUT_MS }); + if (outcome.kind === "timeout") { + log.warn("Timed out syncing .code-workspace file; continuing in background", { + projectPath, }); - return; } - - const projectName = getProjectName(normalizedProjectPath); - const { desiredPaths, managedRootDirs } = computeManagedWorktreePaths({ - allMetadata: await config.getAllWorkspaceMetadata(), - projectPath: normalizedProjectPath, - projectName, - defaultManagedRootDir: path.join(expandTilde(config.srcDir), projectName), - }); - await updateCodeWorkspaceFile({ - codeWorkspacePath, - managedRootDirs, - desiredPaths, - seedFolders: [normalizedProjectPath, ...desiredPaths], - }); } catch (error) { log.warn("Failed to sync .code-workspace file", { projectPath, error }); } From e22dfea4ccbbcb9df7e61636188e2fc9ea6f644a Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:04:34 +0000 Subject: [PATCH 05/11] review: address Codex round-3 findings - scope deletion-time extra roots per involved project (managedRootsByProject) so one project's file never gains removal rights under another's root - group shared-file participants by canonical (realpath) target so symlink aliases of one file reconcile together - treat a matching subProjectPath as project membership and sync the sub-project's file on lifecycle changes of workspaces assigned to it - surface sync failures from the explicit settings save (ORPCError) and roll the setting back so a broken integration is not silently retried --- src/node/orpc/router.test.ts | 23 ++++ src/node/orpc/router.ts | 16 ++- src/node/services/workspaceService.ts | 62 +++++++--- src/node/worktree/codeWorkspaceSync.test.ts | 57 ++++++++- src/node/worktree/codeWorkspaceSync.ts | 128 ++++++++++++++------ 5 files changed, 225 insertions(+), 61 deletions(-) diff --git a/src/node/orpc/router.test.ts b/src/node/orpc/router.test.ts index d660c11b0b7..8075e0d19fd 100644 --- a/src/node/orpc/router.test.ts +++ b/src/node/orpc/router.test.ts @@ -1205,6 +1205,29 @@ describe("projects.setCodeWorkspaceSyncPath", () => { expect(written.folders).toEqual([{ path: projectPath }]); }); + test("surfaces sync failures on explicit save and rolls the setting back", async () => { + // A malformed target must fail the save visibly instead of persisting a + // broken integration that silently retries on every lifecycle event. + fs.writeFileSync(path.join(projectPath, "broken.code-workspace"), "{ not valid jsonc"); + const client = createClient(); + + let thrown: unknown; + try { + await client.projects.setCodeWorkspaceSyncPath({ + projectPath, + codeWorkspaceSyncPath: "broken.code-workspace", + }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(ORPCError); + expect((thrown as ORPCError).message).toContain("not valid JSONC"); + expect( + config.loadConfigOrDefault().projects.get(projectPath)?.codeWorkspaceSyncPath + ).toBeUndefined(); + }); + test("clearing the setting removes it without deleting the existing file", async () => { const client = createClient(); await client.projects.setCodeWorkspaceSyncPath({ diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index 15db97f9f67..0e67c1c7d5c 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -3445,11 +3445,13 @@ export const router = (authToken?: string) => { message: `Path must end with ${CODE_WORKSPACE_EXTENSION}`, }); } + let previousValue: string | undefined; await context.config.editConfig((config) => { const project = config.projects.get(normalizedPath); if (!project) { throw new Error(`Project not found: ${normalizedPath}`); } + previousValue = project.codeWorkspaceSyncPath; // Store undefined for blank input to keep config.json minimal. project.codeWorkspaceSyncPath = trimmed ? trimmed : undefined; return config; @@ -3458,7 +3460,19 @@ export const router = (authToken?: string) => { // next workspace lifecycle event. Clearing or changing the path never // deletes previously written files (they belong to the user). if (trimmed) { - await syncProjectCodeWorkspace(context.config, normalizedPath); + const result = await syncProjectCodeWorkspace(context.config, normalizedPath); + if (!result.ok) { + // Roll back so a broken integration is not retried on every + // later lifecycle event, and surface the reason to the user. + await context.config.editConfig((config) => { + const project = config.projects.get(normalizedPath); + if (project) { + project.codeWorkspaceSyncPath = previousValue; + } + return config; + }); + throw new ORPCError("BAD_REQUEST", { message: result.error }); + } } }), remove: t diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index da395956f45..41ef328ee3e 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -116,10 +116,7 @@ import { import { isWorktreeRuntime } from "@/node/runtime/worktreeLifecycleHooks"; import { expandTilde, expandTildeForSSH } from "@/node/runtime/tildeExpansion"; import { removeManagedGitWorktree } from "@/node/worktree/removeManagedGitWorktree"; -import { - managedRootsForWorkspace, - syncProjectCodeWorkspace, -} from "@/node/worktree/codeWorkspaceSync"; +import { managedRootsByProject, syncProjectCodeWorkspace } from "@/node/worktree/codeWorkspaceSync"; import { copyStagedWorkspaceAttachments, @@ -4434,7 +4431,7 @@ export class WorkspaceService extends EventEmitter { session.emitMetadata(this.enrichFrontendMetadata(completeMetadata)); } - await this.syncCodeWorkspaceFiles(owningProjectPath); + await this.syncCodeWorkspaceFiles(completeMetadata); eventSpine.emit("workspace.created", { workspaceId }); return Ok({ metadata: this.enrichFrontendMetadata(completeMetadata) }); } catch (error) { @@ -4852,7 +4849,7 @@ export class WorkspaceService extends EventEmitter { session.emitMetadata(this.enrichFrontendMetadata(completeMetadata)); } - await this.syncCodeWorkspaceFiles(completeMetadata.projectPath, completeMetadata.projects); + await this.syncCodeWorkspaceFiles(completeMetadata); eventSpine.emit("workspace.created", { workspaceId }); return Ok(enrichedMetadata); } catch (error) { @@ -5406,18 +5403,20 @@ export class WorkspaceService extends EventEmitter { (m) => m.id === workspaceId ); const removedWorkspaceRoots = removedMetadata - ? managedRootsForWorkspace(removedMetadata) - : []; + ? managedRootsByProject(removedMetadata) + : undefined; // Remove from config await this.config.removeWorkspace(workspaceId); removedFromConfig = true; this.autoTitlingWorkspaces.delete(workspaceId); - if (persistedWorkspace) { + if (removedMetadata || persistedWorkspace) { await this.syncCodeWorkspaceFiles( - persistedWorkspace.projectPath, - persistedWorkspace.projects, + removedMetadata ?? { + projectPath: persistedWorkspace!.projectPath, + projects: persistedWorkspace!.projects, + }, removedWorkspaceRoots ); } @@ -5448,13 +5447,28 @@ export class WorkspaceService extends EventEmitter { * syncProjectCodeWorkspace never throws, so lifecycle ops cannot fail here. */ private async syncCodeWorkspaceFiles( - projectPath: string, - projects?: ReadonlyArray<{ projectPath: string }>, - extraManagedRootDirs?: string[] + workspace: { + projectPath: string; + projects?: ReadonlyArray<{ projectPath: string }>; + subProjectPath?: string; + }, + extraManagedRootDirsByProject?: ReadonlyMap ): Promise { - const involvedPaths = new Set([projectPath, ...(projects ?? []).map((ref) => ref.projectPath)]); + const involvedPaths = new Set([ + workspace.projectPath, + // A registered sub-project's file lists workspaces assigned to it even + // though they live in the parent's bucket. + ...(workspace.subProjectPath != null ? [workspace.subProjectPath] : []), + ...(workspace.projects ?? []).map((ref) => ref.projectPath), + ]); for (const involvedPath of involvedPaths) { - await syncProjectCodeWorkspace(this.config, involvedPath, { extraManagedRootDirs }); + await syncProjectCodeWorkspace(this.config, involvedPath, { + // Extras are scoped per project so one project's file never gains + // removal rights under another project's root. + extraManagedRootDirs: extraManagedRootDirsByProject?.get( + stripTrailingSlashes(involvedPath) + ), + }); } } @@ -6368,7 +6382,7 @@ export class WorkspaceService extends EventEmitter { this.emit("metadata", { workspaceId, metadata: enrichedMetadata }); } - await this.syncCodeWorkspaceFiles(configProjectPath, updatedMetadata.projects); + await this.syncCodeWorkspaceFiles(updatedMetadata); return Ok({ newWorkspaceId: workspaceId }); } catch (error) { @@ -7191,7 +7205,11 @@ export class WorkspaceService extends EventEmitter { // disposal here only frees runtimes and spine middleware. Never throws. await agentPluginHookService.disposeWorkspace(workspaceId); - await this.syncCodeWorkspaceFiles(projectPath, beforeArchiveMetadata?.projects); + await this.syncCodeWorkspaceFiles({ + projectPath, + projects: beforeArchiveMetadata?.projects, + subProjectPath: beforeArchiveMetadata?.subProjectPath, + }); eventSpine.emit("workspace.archived", { workspaceId }); return Ok({ kind: "archived" as const }); } catch (error) { @@ -7319,7 +7337,11 @@ export class WorkspaceService extends EventEmitter { await this.emitCurrentWorkspaceMetadata(workspaceId); } - await this.syncCodeWorkspaceFiles(projectPath, hookMetadata?.projects); + await this.syncCodeWorkspaceFiles({ + projectPath, + projects: hookMetadata?.projects, + subProjectPath: hookMetadata?.subProjectPath, + }); return Ok(undefined); } catch (error) { @@ -8453,7 +8475,7 @@ export class WorkspaceService extends EventEmitter { const enrichedMetadata = this.enrichFrontendMetadata(metadata); session.emitMetadata(enrichedMetadata); - await this.syncCodeWorkspaceFiles(foundProjectPath, metadata.projects); + await this.syncCodeWorkspaceFiles(metadata); eventSpine.emit("workspace.created", { workspaceId: newWorkspaceId }); return Ok({ metadata: enrichedMetadata, projectPath: foundProjectPath }); } catch (error) { diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index d726eb2e128..89ff9244203 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -343,6 +343,49 @@ describe("syncProjectCodeWorkspace", () => { expect((await fsPromises.readdir(projectPath)).length).toBe(0); }); + test("groups symlink aliases of the same file into one reconcile", async () => { + // One project configures the symlink path, another the real path; lexical + // comparison would treat them as different files and let their overlapping + // managed root erase each other's entries. + const config = new Config(tempDir); + const projectA = path.join(tempDir, "a", "repo"); + const projectB = path.join(tempDir, "b", "repo"); + await fsPromises.mkdir(projectA, { recursive: true }); + await fsPromises.mkdir(projectB, { recursive: true }); + const realFile = path.join(tempDir, "real.code-workspace"); + await fsPromises.writeFile(realFile, JSON.stringify({ folders: [] })); + const linkFile = path.join(tempDir, "alias.code-workspace"); + await fsPromises.symlink(realFile, linkFile); + const worktreeA = path.join(config.srcDir, "repo", "feat-a"); + const worktreeB = path.join(config.srcDir, "repo", "feat-b"); + const workspaceEntry = (worktree: string, id: string) => ({ + path: worktree, + id, + name: path.basename(worktree), + runtimeConfig: { type: "worktree" as const, srcBaseDir: config.srcDir }, + }); + await config.editConfig((cfg) => { + cfg.projects.set(projectA, { + workspaces: [workspaceEntry(worktreeA, "aaaaaaaaaa")], + codeWorkspaceSyncPath: linkFile, + }); + cfg.projects.set(projectB, { + workspaces: [workspaceEntry(worktreeB, "bbbbbbbbbb")], + codeWorkspaceSyncPath: realFile, + }); + return cfg; + }); + + await syncProjectCodeWorkspace(config, projectA); + await syncProjectCodeWorkspace(config, projectB); + + const folderPaths = parseFolders(await fsPromises.readFile(realFile, "utf-8")).map( + (entry) => entry.path + ); + expect(folderPaths).toContain(worktreeA); + expect(folderPaths).toContain(worktreeB); + }); + test("unions projects that target the same file so they cannot erase each other", async () => { // Same-basename projects share one managed root (/repo); a sync // scoped to only one project would remove the other's entries. @@ -441,7 +484,6 @@ describe("computeManagedWorktreePaths", () => { const managedRoot = "/base/src/my-project"; const computeParams = { projectPath, - projectName: "my-project", defaultManagedRootDir: managedRoot, }; @@ -485,6 +527,19 @@ describe("computeManagedWorktreePaths", () => { expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); }); + test("includes workspaces assigned to a registered sub-project", () => { + const subProjectPath = `${projectPath}/packages/api`; + // The workspace lives in the parent's bucket and shares the parent repo's + // worktree directory; the sub-project's own file must still list it. + const { desiredPaths, managedRootDirs } = computeManagedWorktreePaths({ + allMetadata: [makeMetadata({ subProjectPath })], + projectPath: subProjectPath, + defaultManagedRootDir: "/base/src/api", + }); + expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); + expect(managedRootDirs).toContain(managedRoot); + }); + test("keeps worktrees under a custom or legacy srcBaseDir managed", () => { // Legacy "local"-with-srcBaseDir runtime rooted somewhere other than the // current global srcDir (e.g. a pre-rename ~/.mux/src) must still sync. diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index 95cd3f25586..8c77f665ddf 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -3,6 +3,7 @@ import * as path from "path"; import * as jsonc from "jsonc-parser"; import writeFileAtomic from "write-file-atomic"; import { isWorkspaceArchived } from "@/common/utils/archive"; +import { getErrorMessage } from "@/common/utils/errors"; import { isMultiProject } from "@/common/utils/multiProject"; import { isWorktreeRuntime } from "@/common/types/runtime"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; @@ -122,15 +123,17 @@ function isUnderAnyRoot(managedRootDirs: string[], candidate: string): boolean { * managed-entry invariant. Creates the file (with `seedFolders`) when missing. * Uses jsonc-parser edits so user comments and unknown keys survive. */ -export async function updateCodeWorkspaceFile(update: CodeWorkspaceFileUpdate): Promise { +export async function updateCodeWorkspaceFile( + update: CodeWorkspaceFileUpdate +): Promise { const targetPath = await resolveRealPath(update.codeWorkspacePath); - await withFileWriteLock(targetPath, () => updateCodeWorkspaceFileLocked(targetPath, update)); + return withFileWriteLock(targetPath, () => updateCodeWorkspaceFileLocked(targetPath, update)); } async function updateCodeWorkspaceFileLocked( targetPath: string, update: CodeWorkspaceFileUpdate -): Promise { +): Promise { const { codeWorkspacePath, managedRootDirs, desiredPaths } = update; // Relative folder entries resolve against the configured file location, // matching how VS Code resolves them for the file the user opens. @@ -144,7 +147,7 @@ async function updateCodeWorkspaceFileLocked( codeWorkspacePath, sizeBytes: stats.size, }); - return; + return { ok: false, error: "Workspace file exceeds the 1 MiB sync limit" }; } original = await fsPromises.readFile(targetPath, "utf-8"); } catch (error) { @@ -158,7 +161,7 @@ async function updateCodeWorkspaceFileLocked( const fresh = { folders: update.seedFolders.map((folderPath) => ({ path: folderPath })) }; await fsPromises.mkdir(path.dirname(targetPath), { recursive: true }); await writeFileAtomic(targetPath, JSON.stringify(fresh, null, "\t") + "\n"); - return; + return { ok: true }; } // Never clobber a file we cannot faithfully edit (self-healing over failing). @@ -171,13 +174,13 @@ async function updateCodeWorkspaceFileLocked( Array.isArray(parsed) ) { log.warn("Skipping .code-workspace sync: file is not valid JSONC", { codeWorkspacePath }); - return; + return { ok: false, error: "Workspace file is not valid JSONC" }; } const existingFolders = (parsed as { folders?: unknown }).folders; if (existingFolders !== undefined && !Array.isArray(existingFolders)) { log.warn("Skipping .code-workspace sync: 'folders' is not an array", { codeWorkspacePath }); - return; + return { ok: false, error: "Workspace file 'folders' is not an array" }; } const desired = new Set(desiredPaths.map((desiredPath) => path.resolve(desiredPath))); @@ -232,16 +235,39 @@ async function updateCodeWorkspaceFileLocked( if (text !== original) { await writeFileAtomic(targetPath, text); } + return { ok: true }; } function belongsToProject(metadata: FrontendWorkspaceMetadata, projectPath: string): boolean { return ( stripTrailingSlashes(metadata.projectPath) === projectPath || + // Workspaces assigned to a registered sub-project live in the parent's + // bucket; the sub-project's own file must still list them. + (metadata.subProjectPath != null && + stripTrailingSlashes(metadata.subProjectPath) === projectPath) || (metadata.projects?.some((ref) => stripTrailingSlashes(ref.projectPath) === projectPath) ?? false) ); } +// The directory that holds this workspace's checkout for the given +// participant project. Sub-project workspaces share the parent repo's +// worktree directory; multi-project participants each have a directory named +// after themselves. +function participantCheckoutDirName( + metadata: FrontendWorkspaceMetadata, + participantPath: string +): string { + const isPrimary = stripTrailingSlashes(metadata.projectPath) === participantPath; + const isRef = + metadata.projects?.some((ref) => stripTrailingSlashes(ref.projectPath) === participantPath) ?? + false; + if (isPrimary || isRef) { + return getProjectName(participantPath); + } + return getProjectName(stripTrailingSlashes(metadata.projectPath)); +} + /** * Compute the worktree paths that belong in a project's .code-workspace file * (active, top-level worktree workspaces) plus the managed root directories @@ -251,23 +277,28 @@ export function computeManagedWorktreePaths(params: { allMetadata: FrontendWorkspaceMetadata[]; /** Normalized (no trailing slash) project path. */ projectPath: string; - projectName: string; defaultManagedRootDir: string; }): { desiredPaths: string[]; managedRootDirs: string[] } { - const { allMetadata, projectPath, projectName, defaultManagedRootDir } = params; + const { allMetadata, projectPath, defaultManagedRootDir } = params; // Managed roots come from every persisted worktree workspace of the project // (any lifecycle state), not just the current global srcDir: worktrees // created under a custom or legacy srcBaseDir (e.g. pre-rename ~/.mux/src) - // must stay managed after upgrades. Residual: an entry under a custom root - // whose last workspace was deleted is no longer classified as managed and - // can linger until removed by hand. + // must stay managed after upgrades. Callers cleaning up after a deletion + // pass the removed workspace's roots explicitly (managedRootsByProject). const roots = new Set([path.resolve(defaultManagedRootDir)]); for (const metadata of allMetadata) { if (!belongsToProject(metadata, projectPath) || !isWorktreeRuntime(metadata.runtimeConfig)) { continue; } - roots.add(path.resolve(path.join(expandTilde(metadata.runtimeConfig.srcBaseDir), projectName))); + roots.add( + path.resolve( + path.join( + expandTilde(metadata.runtimeConfig.srcBaseDir), + participantCheckoutDirName(metadata, projectPath) + ) + ) + ); } const managedRootDirs = [...roots].sort(); @@ -296,12 +327,12 @@ export function computeManagedWorktreePaths(params: { if (isMultiProject(metadata)) { // Multi-project workspaces persist the _workspaces/ symlink // container as namedWorkspacePath; the real per-project checkout lives - // at // for the primary and + // at // for the primary and // secondary projects alike (createMultiProject passes // directoryName: workspaceName). worktreePath = path.join( expandTilde(metadata.runtimeConfig.srcBaseDir), - projectName, + participantCheckoutDirName(metadata, projectPath), metadata.name ); } else { @@ -318,25 +349,31 @@ export function computeManagedWorktreePaths(params: { } /** - * Managed roots contributed by one workspace, computed from its own runtime - * config. Callers capture this BEFORE deleting a workspace: once its config - * entry is gone, a custom/legacy srcBaseDir root can no longer be + * Managed roots contributed by one workspace, keyed by each involved project + * path (normalized). Callers capture this BEFORE deleting a workspace: once + * its config entry is gone, a custom/legacy srcBaseDir root can no longer be * reconstructed and the deleted checkout's folder entry would linger forever. + * Roots are per-project so one project's file is never granted removal rights + * under another project's root. */ -export function managedRootsForWorkspace( - metadata: Pick -): string[] { +export function managedRootsByProject(metadata: FrontendWorkspaceMetadata): Map { + const rootsByProject = new Map(); if (!isWorktreeRuntime(metadata.runtimeConfig)) { - return []; + return rootsByProject; } const srcBaseDir = expandTilde(metadata.runtimeConfig.srcBaseDir); const involved = new Set([ metadata.projectPath, + ...(metadata.subProjectPath != null ? [metadata.subProjectPath] : []), ...(metadata.projects ?? []).map((ref) => ref.projectPath), ]); - return [...involved].map((involvedPath) => - path.resolve(path.join(srcBaseDir, getProjectName(stripTrailingSlashes(involvedPath)))) - ); + for (const involvedPath of involved) { + const normalized = stripTrailingSlashes(involvedPath); + rootsByProject.set(normalized, [ + path.resolve(path.join(srcBaseDir, participantCheckoutDirName(metadata, normalized))), + ]); + } + return rootsByProject; } // Resolve a project's configured setting to an absolute target path, or null @@ -361,18 +398,21 @@ function resolveConfiguredCodeWorkspacePath( return codeWorkspacePath; } +export type CodeWorkspaceSyncResult = { ok: true } | { ok: false; error: string }; + /** * Best-effort sync entry point used by workspace lifecycle operations and * startup reconciliation. Fast no-op when the project has no * `codeWorkspaceSyncPath` configured. Never throws and is bounded by * SYNC_TIMEOUT_MS: sync failures or stalled filesystems must never fail or - * block a workspace operation. + * block a workspace operation. Background callers ignore the returned result; + * the explicit settings save path surfaces it to the user. */ export async function syncProjectCodeWorkspace( config: Config, projectPath: string, options?: { extraManagedRootDirs?: string[] } -): Promise { +): Promise { try { const normalizedProjectPath = stripTrailingSlashes(projectPath); const projects = config.loadConfigOrDefault().projects; @@ -381,16 +421,19 @@ export async function syncProjectCodeWorkspace( projects.get(normalizedProjectPath)?.codeWorkspaceSyncPath ); if (!targetFile) { - return; + return { ok: true }; } - // The work never rejects (errors are logged inside), so a timeout that + // The work never rejects (errors become results), so a timeout that // orphans it cannot leave an unhandled rejection behind. - const work = (async () => { + const work: Promise = (async () => { try { // Reconcile every project targeting this file together. Projects can // share a file, and same-basename projects even share a managed root; // independent per-project removals would erase each other's entries. + // Group by canonical (symlink-resolved) target so aliases of one file + // reconcile together too. + const canonicalTarget = await resolveRealPath(targetFile); const allMetadata = await config.getAllWorkspaceMetadata(); const desired = new Set(); const roots = new Set( @@ -398,27 +441,30 @@ export async function syncProjectCodeWorkspace( ); const participantPaths: string[] = []; for (const [participantPath, participantConfig] of projects) { + const participantFile = resolveConfiguredCodeWorkspacePath( + participantPath, + participantConfig.codeWorkspaceSyncPath + ); if ( - resolveConfiguredCodeWorkspacePath( - participantPath, - participantConfig.codeWorkspaceSyncPath - ) !== targetFile + participantFile === null || + (await resolveRealPath(participantFile)) !== canonicalTarget ) { continue; } - const projectName = getProjectName(participantPath); const computed = computeManagedWorktreePaths({ allMetadata, projectPath: participantPath, - projectName, - defaultManagedRootDir: path.join(expandTilde(config.srcDir), projectName), + defaultManagedRootDir: path.join( + expandTilde(config.srcDir), + getProjectName(participantPath) + ), }); participantPaths.push(participantPath); computed.desiredPaths.forEach((desiredPath) => desired.add(desiredPath)); computed.managedRootDirs.forEach((rootDir) => roots.add(rootDir)); } const desiredPaths = [...desired].sort(); - await updateCodeWorkspaceFile({ + return await updateCodeWorkspaceFile({ codeWorkspacePath: targetFile, managedRootDirs: [...roots].sort(), desiredPaths, @@ -426,16 +472,20 @@ export async function syncProjectCodeWorkspace( }); } catch (error) { log.warn("Failed to sync .code-workspace file", { projectPath, error }); + return { ok: false, error: getErrorMessage(error) }; } })(); const outcome = await raceWithAbortAndTimeout(work, { timeoutMs: SYNC_TIMEOUT_MS }); - if (outcome.kind === "timeout") { + if (outcome.kind !== "ok") { log.warn("Timed out syncing .code-workspace file; continuing in background", { projectPath, }); + return { ok: false, error: "Timed out accessing the workspace file" }; } + return outcome.value; } catch (error) { log.warn("Failed to sync .code-workspace file", { projectPath, error }); + return { ok: false, error: getErrorMessage(error) }; } } From 19a7aeac06350e744c3fcaa476d045907a93b911 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:50:57 +0000 Subject: [PATCH 06/11] review: address Codex round-4 findings - one-pass folders reconcile (single jsonc edit) so repo-controlled files with thousands of entries cannot block the main thread quadratically - reject non-regular targets and read via one descriptor with a byte cap (a symlink to /dev/zero reported size 0 but never reached EOF) - derive desired state inside the per-file critical section so an older lifecycle snapshot cannot overwrite a newer write - bound every canonicalization realpath (1s) so another project's dead mount cannot stall an unrelated sync - guard the setter rollback so a concurrent newer save is never discarded - include devcontainer workspaces (host worktrees under the default root) - capture removed-workspace roots best-effort so removal never fails on it --- src/node/orpc/router.ts | 4 +- src/node/services/workspaceService.ts | 22 +- src/node/worktree/codeWorkspaceSync.test.ts | 29 +++ src/node/worktree/codeWorkspaceSync.ts | 244 ++++++++++++-------- 4 files changed, 191 insertions(+), 108 deletions(-) diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index 0e67c1c7d5c..15706b2a1f4 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -3464,9 +3464,11 @@ export const router = (authToken?: string) => { if (!result.ok) { // Roll back so a broken integration is not retried on every // later lifecycle event, and surface the reason to the user. + // Guarded: a concurrent save may have stored a newer value that + // this failing request must not discard. await context.config.editConfig((config) => { const project = config.projects.get(normalizedPath); - if (project) { + if (project?.codeWorkspaceSyncPath === trimmed) { project.codeWorkspaceSyncPath = previousValue; } return config; diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 41ef328ee3e..44989ca8baa 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -5399,12 +5399,22 @@ export class WorkspaceService extends EventEmitter { // Capture managed roots before the config entry disappears: a worktree // under a custom/legacy srcBaseDir cannot be reconstructed afterwards, // which would leave its folder entry in the .code-workspace file forever. - const removedMetadata = (await this.config.getAllWorkspaceMetadata()).find( - (m) => m.id === workspaceId - ); - const removedWorkspaceRoots = removedMetadata - ? managedRootsByProject(removedMetadata) - : undefined; + // Best-effort: removal must proceed even when the capture fails. + let removedMetadata: FrontendWorkspaceMetadata | undefined; + let removedWorkspaceRoots: Map | undefined; + try { + removedMetadata = (await this.config.getAllWorkspaceMetadata()).find( + (m) => m.id === workspaceId + ); + removedWorkspaceRoots = removedMetadata + ? managedRootsByProject(removedMetadata) + : undefined; + } catch (error) { + log.debug("Failed to capture removed workspace roots for .code-workspace sync", { + workspaceId, + error, + }); + } // Remove from config await this.config.removeWorkspace(workspaceId); diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index 89ff9244203..bdf6aa3bc7e 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -158,6 +158,23 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: worktree }]); }); + test("rejects non-regular targets like device files behind a symlink", async () => { + // A checkout-supplied symlink can point at e.g. /dev/zero, where stat + // reports size 0 but reads never reach EOF. + const linkPath = path.join(tempDir, "device.code-workspace"); + await fsPromises.symlink("/dev/null", linkPath); + + const result = await updateCodeWorkspaceFile({ + codeWorkspacePath: linkPath, + managedRootDirs: [managedRootDir], + desiredPaths: [path.join(managedRootDir, "feature-a")], + seedFolders: [], + }); + + expect(result.ok).toBe(false); + expect((await fsPromises.lstat(linkPath)).isSymbolicLink()).toBe(true); + }); + test("skips files exceeding the size cap without rewriting them", async () => { // jsonc.parse is synchronous, so oversized (potentially repo-controlled) // files must be rejected before parsing. @@ -527,6 +544,18 @@ describe("computeManagedWorktreePaths", () => { expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); }); + test("includes devcontainer workspaces (host worktrees under the default root)", () => { + const { desiredPaths } = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ + runtimeConfig: { type: "devcontainer", configPath: ".devcontainer/devcontainer.json" }, + }), + ], + ...computeParams, + }); + expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); + }); + test("includes workspaces assigned to a registered sub-project", () => { const subProjectPath = `${projectPath}/packages/api`; // The workspace lives in the parent's bucket and shares the parent repo's diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index 8c77f665ddf..5e63f2e7e70 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -5,7 +5,7 @@ import writeFileAtomic from "write-file-atomic"; import { isWorkspaceArchived } from "@/common/utils/archive"; import { getErrorMessage } from "@/common/utils/errors"; import { isMultiProject } from "@/common/utils/multiProject"; -import { isWorktreeRuntime } from "@/common/types/runtime"; +import { isWorktreeRuntime, type RuntimeConfig } from "@/common/types/runtime"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import type { Config } from "@/node/config"; import { expandTilde } from "@/node/runtime/tildeExpansion"; @@ -64,6 +64,18 @@ async function withFileWriteLock(key: string, fn: () => Promise): Promise< return run; } +// Canonicalization of another project's configured path must never stall the +// current sync (its file may sit on a dead network mount), so realpath calls +// used for grouping are individually bounded. +const REALPATH_TIMEOUT_MS = 1_000; + +async function boundedRealPath(filePath: string): Promise { + const outcome = await raceWithAbortAndTimeout(resolveRealPath(filePath), { + timeoutMs: REALPATH_TIMEOUT_MS, + }); + return outcome.kind === "ok" ? outcome.value : null; +} + // write-file-atomic renames a temp file over the target, which would replace a // symlink rather than write through it; resolve the real target first so user // symlinks to shared editor config survive syncs. @@ -82,13 +94,6 @@ async function resolveRealPath(filePath: string): Promise { } } -function readFolders(text: string): unknown[] | undefined { - const parsed = jsonc.parse(text, undefined, { allowTrailingComma: true }) as - | { folders?: unknown } - | undefined; - return Array.isArray(parsed?.folders) ? parsed.folders : undefined; -} - // VS Code resolves relative folder paths against the .code-workspace file's // directory; it does not expand `~` or variables, so neither do we. Such // entries never match the managed root and stay untouched. @@ -141,7 +146,15 @@ async function updateCodeWorkspaceFileLocked( let original: string | null; try { + // Reject non-regular targets before opening: a symlink can point at e.g. + // /dev/zero, where the reported size is 0 but reads never reach EOF. const stats = await fsPromises.stat(targetPath); + if (!stats.isFile()) { + log.warn("Skipping .code-workspace sync: target is not a regular file", { + codeWorkspacePath, + }); + return { ok: false, error: "Workspace file is not a regular file" }; + } if (stats.size > MAX_CODE_WORKSPACE_FILE_BYTES) { log.warn("Skipping .code-workspace sync: file exceeds size limit", { codeWorkspacePath, @@ -149,7 +162,28 @@ async function updateCodeWorkspaceFileLocked( }); return { ok: false, error: "Workspace file exceeds the 1 MiB sync limit" }; } - original = await fsPromises.readFile(targetPath, "utf-8"); + // Byte-limited read through one descriptor: never trust the stat size. + const handle = await fsPromises.open(targetPath, "r"); + try { + const handleStats = await handle.stat(); + if (!handleStats.isFile()) { + log.warn("Skipping .code-workspace sync: target is not a regular file", { + codeWorkspacePath, + }); + return { ok: false, error: "Workspace file is not a regular file" }; + } + const buffer = Buffer.alloc(MAX_CODE_WORKSPACE_FILE_BYTES + 1); + const { bytesRead } = await handle.read(buffer, 0, buffer.length, 0); + if (bytesRead > MAX_CODE_WORKSPACE_FILE_BYTES) { + log.warn("Skipping .code-workspace sync: file exceeds size limit", { + codeWorkspacePath, + }); + return { ok: false, error: "Workspace file exceeds the 1 MiB sync limit" }; + } + original = buffer.subarray(0, bytesRead).toString("utf-8"); + } finally { + await handle.close(); + } } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") { throw error; @@ -184,60 +218,47 @@ async function updateCodeWorkspaceFileLocked( } const desired = new Set(desiredPaths.map((desiredPath) => path.resolve(desiredPath))); - let text = original; - if (existingFolders === undefined) { - text = jsonc.applyEdits(text, jsonc.modify(text, ["folders"], [], MODIFY_OPTIONS)); - } - - // Remove managed entries that are no longer desired, one edit at a time - // (indices shift after every removal, so re-parse between edits). - for (;;) { - const folders = readFolders(text) ?? []; - const removeIndex = folders.findIndex((entry) => { - const entryPath = getEntryPath(entry, fileDir); - return ( - entryPath !== null && isUnderAnyRoot(managedRootDirs, entryPath) && !desired.has(entryPath) - ); - }); - if (removeIndex < 0) { - break; + // One-pass reconcile: a repository-controlled file can hold thousands of + // entries and per-entry jsonc edits reparse the whole document each time + // (quadratic, synchronous, and untouchable by timeouts). Build the new + // folders value once and apply a single edit. Comments inside the folders + // array are not preserved when a change is needed; everything outside it is. + const currentFolders: unknown[] = existingFolders ?? []; + const presentPaths = new Set(); + const kept = currentFolders.filter((entry) => { + const entryPath = getEntryPath(entry, fileDir); + if (entryPath === null) { + return true; } - text = jsonc.applyEdits( - text, - jsonc.modify(text, ["folders", removeIndex], undefined, MODIFY_OPTIONS) - ); - } - + if (isUnderAnyRoot(managedRootDirs, entryPath) && !desired.has(entryPath)) { + return false; + } + presentPaths.add(entryPath); + return true; + }); // Append missing desired entries after the existing ones (user order is preserved). - const presentPaths = new Set( - (readFolders(text) ?? []) - .map((entry) => getEntryPath(entry, fileDir)) - .filter((entryPath): entryPath is string => entryPath !== null) - ); const additions = [...desired].filter((desiredPath) => !presentPaths.has(desiredPath)).sort(); - for (const folderPath of additions) { - const length = (readFolders(text) ?? []).length; - text = jsonc.applyEdits( - text, - jsonc.modify( - text, - ["folders", length], - { path: folderPath }, - { - ...MODIFY_OPTIONS, - isArrayInsertion: true, - } - ) - ); - } - if (text !== original) { - await writeFileAtomic(targetPath, text); + if (kept.length === currentFolders.length && additions.length === 0) { + return { ok: true }; } + const newFolders = [...kept, ...additions.map((folderPath) => ({ path: folderPath }))]; + const text = jsonc.applyEdits( + original, + jsonc.modify(original, ["folders"], newFolders, MODIFY_OPTIONS) + ); + await writeFileAtomic(targetPath, text); return { ok: true }; } +// DevcontainerRuntime also creates a normal host git worktree via +// WorktreeManager under the default srcDir, so its workspaces belong in the +// file too (rooted at the default managed root). +function hasManagedHostWorktree(runtimeConfig: RuntimeConfig | undefined): boolean { + return isWorktreeRuntime(runtimeConfig) || runtimeConfig?.type === "devcontainer"; +} + function belongsToProject(metadata: FrontendWorkspaceMetadata, projectPath: string): boolean { return ( stripTrailingSlashes(metadata.projectPath) === projectPath || @@ -304,7 +325,8 @@ export function computeManagedWorktreePaths(params: { const desired = new Set(); for (const metadata of allMetadata) { - if (!isWorktreeRuntime(metadata.runtimeConfig)) { + const runtimeConfig = metadata.runtimeConfig; + if (!hasManagedHostWorktree(runtimeConfig)) { continue; } // Sub-agent child workspaces are transient implementation detail; listing @@ -324,14 +346,14 @@ export function computeManagedWorktreePaths(params: { } let worktreePath: string; - if (isMultiProject(metadata)) { + if (isMultiProject(metadata) && isWorktreeRuntime(runtimeConfig)) { // Multi-project workspaces persist the _workspaces/ symlink // container as namedWorkspacePath; the real per-project checkout lives // at // for the primary and // secondary projects alike (createMultiProject passes // directoryName: workspaceName). worktreePath = path.join( - expandTilde(metadata.runtimeConfig.srcBaseDir), + expandTilde(runtimeConfig.srcBaseDir), participantCheckoutDirName(metadata, projectPath), metadata.name ); @@ -424,57 +446,77 @@ export async function syncProjectCodeWorkspace( return { ok: true }; } + // Group by canonical (symlink-resolved) target so aliases of one file + // reconcile together. The realpath is bounded so a dead mount holding this + // project's file fails fast instead of hanging until the outer timeout. + const canonicalTarget = await boundedRealPath(targetFile); + if (canonicalTarget === null) { + log.warn("Timed out canonicalizing .code-workspace path", { projectPath }); + return { ok: false, error: "Timed out accessing the workspace file" }; + } + // The work never rejects (errors become results), so a timeout that // orphans it cannot leave an unhandled rejection behind. - const work: Promise = (async () => { - try { - // Reconcile every project targeting this file together. Projects can - // share a file, and same-basename projects even share a managed root; - // independent per-project removals would erase each other's entries. - // Group by canonical (symlink-resolved) target so aliases of one file - // reconcile together too. - const canonicalTarget = await resolveRealPath(targetFile); - const allMetadata = await config.getAllWorkspaceMetadata(); - const desired = new Set(); - const roots = new Set( - (options?.extraManagedRootDirs ?? []).map((rootDir) => path.resolve(rootDir)) - ); - const participantPaths: string[] = []; - for (const [participantPath, participantConfig] of projects) { - const participantFile = resolveConfiguredCodeWorkspacePath( - participantPath, - participantConfig.codeWorkspaceSyncPath + const work: Promise = withFileWriteLock( + canonicalTarget, + async (): Promise => { + try { + // Desired state is derived INSIDE the per-file critical section: + // with derivation outside it, an older lifecycle snapshot could be + // written after a newer one and resurrect stale entries. + // + // Reconcile every project targeting this file together. Projects can + // share a file, and same-basename projects even share a managed + // root; independent per-project removals would erase each other's + // entries. + const currentProjects = config.loadConfigOrDefault().projects; + const allMetadata = await config.getAllWorkspaceMetadata(); + const desired = new Set(); + const roots = new Set( + (options?.extraManagedRootDirs ?? []).map((rootDir) => path.resolve(rootDir)) ); - if ( - participantFile === null || - (await resolveRealPath(participantFile)) !== canonicalTarget - ) { - continue; + const participantPaths: string[] = []; + for (const [participantPath, participantConfig] of currentProjects) { + const participantFile = resolveConfiguredCodeWorkspacePath( + participantPath, + participantConfig.codeWorkspaceSyncPath + ); + if (participantFile === null) { + continue; + } + // A stalled canonicalization of an unrelated project's path only + // drops that project from this round; it cannot block the sync. + if (participantFile !== targetFile) { + const participantCanonical = await boundedRealPath(participantFile); + if (participantCanonical !== canonicalTarget) { + continue; + } + } + const computed = computeManagedWorktreePaths({ + allMetadata, + projectPath: participantPath, + defaultManagedRootDir: path.join( + expandTilde(config.srcDir), + getProjectName(participantPath) + ), + }); + participantPaths.push(participantPath); + computed.desiredPaths.forEach((desiredPath) => desired.add(desiredPath)); + computed.managedRootDirs.forEach((rootDir) => roots.add(rootDir)); } - const computed = computeManagedWorktreePaths({ - allMetadata, - projectPath: participantPath, - defaultManagedRootDir: path.join( - expandTilde(config.srcDir), - getProjectName(participantPath) - ), + const desiredPaths = [...desired].sort(); + return await updateCodeWorkspaceFileLocked(canonicalTarget, { + codeWorkspacePath: targetFile, + managedRootDirs: [...roots].sort(), + desiredPaths, + seedFolders: [...participantPaths, ...desiredPaths], }); - participantPaths.push(participantPath); - computed.desiredPaths.forEach((desiredPath) => desired.add(desiredPath)); - computed.managedRootDirs.forEach((rootDir) => roots.add(rootDir)); + } catch (error) { + log.warn("Failed to sync .code-workspace file", { projectPath, error }); + return { ok: false, error: getErrorMessage(error) }; } - const desiredPaths = [...desired].sort(); - return await updateCodeWorkspaceFile({ - codeWorkspacePath: targetFile, - managedRootDirs: [...roots].sort(), - desiredPaths, - seedFolders: [...participantPaths, ...desiredPaths], - }); - } catch (error) { - log.warn("Failed to sync .code-workspace file", { projectPath, error }); - return { ok: false, error: getErrorMessage(error) }; } - })(); + ); const outcome = await raceWithAbortAndTimeout(work, { timeoutMs: SYNC_TIMEOUT_MS }); if (outcome.kind !== "ok") { From 253966103ba82db6ea86c9d6dbf6d549e0462922 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 12:28:35 +0000 Subject: [PATCH 07/11] review: address Codex round-5 findings --- .../Settings/Sections/RuntimesSection.tsx | 21 ++-- src/node/orpc/router.test.ts | 56 +++++++++++ src/node/orpc/router.ts | 31 ++++++ src/node/worktree/codeWorkspaceSync.test.ts | 35 +++++++ src/node/worktree/codeWorkspaceSync.ts | 98 +++++++++++++++---- 5 files changed, 213 insertions(+), 28 deletions(-) diff --git a/src/browser/features/Settings/Sections/RuntimesSection.tsx b/src/browser/features/Settings/Sections/RuntimesSection.tsx index 570d27df0ac..8099970f37e 100644 --- a/src/browser/features/Settings/Sections/RuntimesSection.tsx +++ b/src/browser/features/Settings/Sections/RuntimesSection.tsx @@ -3,6 +3,7 @@ import { AlertTriangle, Loader2 } from "lucide-react"; import { Button } from "@/browser/components/Button/Button"; import { Input } from "@/browser/components/Input/Input"; +import { getErrorMessage } from "@/common/utils/errors"; import { CoderWorkspaceForm, @@ -153,6 +154,16 @@ function CodeWorkspaceSyncField(props: { projectPath: string }) { const draftValue = draft ?? savedPath; const isDirty = draftValue.trim() !== savedPath; + // DOM attributes must not receive promise-returning handlers + // (@typescript-eslint/no-misused-promises), so instead of awaiting inline, + // unexpected rejections are explicitly routed into the visible error state. + const saveDraft = () => { + void handleSave().catch((saveError: unknown) => { + setError(getErrorMessage(saveError)); + setSaving(false); + }); + }; + const handleSave = async () => { setSaving(true); setError(null); @@ -185,7 +196,7 @@ function CodeWorkspaceSyncField(props: { projectPath: string }) { onKeyDown={(event) => { if (event.key === "Enter" && isDirty && !saving) { event.preventDefault(); - void handleSave(); + saveDraft(); } }} disabled={saving} @@ -193,13 +204,7 @@ function CodeWorkspaceSyncField(props: { projectPath: string }) { aria-label="VS Code workspace file path" className="max-w-[360px] min-w-0" /> -
diff --git a/src/node/orpc/router.test.ts b/src/node/orpc/router.test.ts index 8075e0d19fd..f15ddd9669e 100644 --- a/src/node/orpc/router.test.ts +++ b/src/node/orpc/router.test.ts @@ -6,6 +6,7 @@ import * as os from "os"; import * as path from "path"; import { DEFAULT_TASK_SETTINGS } from "@/common/types/tasks"; import { Config } from "@/node/config"; +import { ProjectService } from "@/node/services/projectService"; import { QuickJSRuntimeFactory } from "@/node/services/ptc/quickjsRuntime"; import { ForegroundWaitBackgroundedError } from "@/node/services/taskService"; import { WorkflowRunStore } from "@/node/services/workflows/WorkflowRunStore"; @@ -1244,4 +1245,59 @@ describe("projects.setCodeWorkspaceSyncPath", () => { ).toBeUndefined(); expect(fs.existsSync(path.join(projectPath, "proj.code-workspace"))).toBe(true); }); + + test("reassigning a workspace between sub-projects syncs both workspace files", async () => { + const subProjectA = path.join(projectPath, "packages", "a"); + const subProjectB = path.join(projectPath, "packages", "b"); + fs.mkdirSync(subProjectA, { recursive: true }); + fs.mkdirSync(subProjectB, { recursive: true }); + const worktreePath = path.join(config.srcDir, "project", "feat-1"); + await config.editConfig((current) => { + current.projects.set(projectPath, { + workspaces: [ + { + path: worktreePath, + id: "aaaaaaaaaa", + name: "feat-1", + runtimeConfig: { type: "worktree", srcBaseDir: config.srcDir }, + subProjectPath: subProjectA, + }, + ], + }); + current.projects.set(subProjectA, { + workspaces: [], + parentProjectPath: projectPath, + codeWorkspaceSyncPath: "a.code-workspace", + }); + current.projects.set(subProjectB, { + workspaces: [], + parentProjectPath: projectPath, + codeWorkspaceSyncPath: "b.code-workspace", + }); + return current; + }); + const fileA = path.join(subProjectA, "a.code-workspace"); + const fileB = path.join(subProjectB, "b.code-workspace"); + // File A already lists the workspace, as a prior sync would have left it. + fs.writeFileSync(fileA, JSON.stringify({ folders: [{ path: worktreePath }] })); + + const client = createRouterClient(router(), { + context: { + config, + projectService: new ProjectService(config), + workspaceService: { refreshAndEmitMetadata: async () => undefined }, + } as unknown as ORPCContext, + }); + const result = await client.projects.subProjects.assignWorkspace({ + projectPath, + workspaceId: "aaaaaaaaaa", + subProjectPath: subProjectB, + }); + + expect(result.success).toBe(true); + const foldersOf = (file: string) => + (JSON.parse(fs.readFileSync(file, "utf-8")) as { folders: Array<{ path: string }> }).folders; + expect(foldersOf(fileA).map((f) => f.path)).not.toContain(worktreePath); + expect(foldersOf(fileB).map((f) => f.path)).toContain(worktreePath); + }); }); diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index 15706b2a1f4..aaf2233d8d2 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -15,6 +15,7 @@ import { isErrnoWithCode } from "@/node/utils/fs"; import { isPathInsideDir, stripTrailingSlashes } from "@/node/utils/pathUtils"; import { CODE_WORKSPACE_EXTENSION, + managedRootsByProject, syncProjectCodeWorkspace, } from "@/node/worktree/codeWorkspaceSync"; import { generateWorkspaceIdentity } from "@/node/services/workspaceTitleGenerator"; @@ -3912,6 +3913,14 @@ export const router = (authToken?: string) => { .input(schemas.projects.subProjects.assignWorkspace.input) .output(schemas.projects.subProjects.assignWorkspace.output) .handler(async ({ context, input }) => { + // Reassignment moves the workspace between sub-project workspace + // files. Capture the previous assignment (and the managed roots + // the workspace contributed to it) before it changes: afterwards + // the old file could no longer remove the entry, for the same + // reason workspace removal captures managedRootsByProject. + const previousMetadata = (await context.config.getAllWorkspaceMetadata()).find( + (m) => m.id === input.workspaceId + ); const result = await context.projectService.assignWorkspaceToSubProject( input.projectPath, input.workspaceId, @@ -3919,6 +3928,28 @@ export const router = (authToken?: string) => { ); if (result.success) { await context.workspaceService.refreshAndEmitMetadata(input.workspaceId); + // Best-effort (results logged inside): reconcile both sides of + // the reassignment so the old file drops the entry and the new + // one gains it without waiting for the next lifecycle event. + if (input.subProjectPath !== null) { + await syncProjectCodeWorkspace(context.config, input.subProjectPath); + } + const previousSubProjectPath = + previousMetadata?.subProjectPath != null + ? stripTrailingSlashes(previousMetadata.subProjectPath) + : null; + const newSubProjectPath = + input.subProjectPath !== null ? stripTrailingSlashes(input.subProjectPath) : null; + if ( + previousMetadata && + previousSubProjectPath !== null && + previousSubProjectPath !== newSubProjectPath + ) { + await syncProjectCodeWorkspace(context.config, previousSubProjectPath, { + extraManagedRootDirs: + managedRootsByProject(previousMetadata).get(previousSubProjectPath), + }); + } } return result; }), diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index bdf6aa3bc7e..df3022b3cbc 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -244,6 +244,23 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: theirs }]); }); + test("creates a dangling symlink's target instead of replacing the link", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + const target = path.join(tempDir, "real-target.code-workspace"); + const linkPath = path.join(tempDir, "dangling.code-workspace"); + await fsPromises.symlink(target, linkPath); + + await updateCodeWorkspaceFile({ + codeWorkspacePath: linkPath, + managedRootDirs: [managedRootDir], + desiredPaths: [worktree], + seedFolders: [worktree], + }); + + expect((await fsPromises.lstat(linkPath)).isSymbolicLink()).toBe(true); + expect(parseFolders(await fsPromises.readFile(target, "utf-8"))).toEqual([{ path: worktree }]); + }); + test("writes through a symlinked workspace file without replacing the link", async () => { const worktree = path.join(managedRootDir, "feature-a"); const realFile = path.join(tempDir, "shared-config", "real.code-workspace"); @@ -556,6 +573,24 @@ describe("computeManagedWorktreePaths", () => { expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); }); + test("keeps a devcontainer workspace assigned to a sub-project under the parent root", () => { + const subProjectPath = `${projectPath}/packages/api`; + // Devcontainer host worktrees live under the PARENT project's directory; + // the sub-project's default root (/base/src/api) does not contain them. + const { desiredPaths, managedRootDirs } = computeManagedWorktreePaths({ + allMetadata: [ + makeMetadata({ + subProjectPath, + runtimeConfig: { type: "devcontainer", configPath: ".devcontainer/devcontainer.json" }, + }), + ], + projectPath: subProjectPath, + defaultManagedRootDir: "/base/src/api", + }); + expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); + expect(managedRootDirs).toContain(managedRoot); + }); + test("includes workspaces assigned to a registered sub-project", () => { const subProjectPath = `${projectPath}/packages/api`; // The workspace lives in the parent's bucket and shares the parent repo's diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index 5e63f2e7e70..c804c9ed68a 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -80,9 +80,31 @@ async function boundedRealPath(filePath: string): Promise { // symlink rather than write through it; resolve the real target first so user // symlinks to shared editor config survive syncs. async function resolveRealPath(filePath: string): Promise { + return resolveRealPathFollowingDanglingLinks(filePath, 10); +} + +async function resolveRealPathFollowingDanglingLinks( + filePath: string, + hopsLeft: number +): Promise { try { return await fsPromises.realpath(filePath); } catch { + // realpath fails on a DANGLING symlink too; follow it manually so the + // creation path writes the link's intended target instead of atomically + // renaming over (and destroying) the link itself. Hops are bounded so + // cyclic links cannot loop forever. + if (hopsLeft > 0) { + try { + const linkTarget = await fsPromises.readlink(filePath); + return await resolveRealPathFollowingDanglingLinks( + path.resolve(path.dirname(filePath), linkTarget), + hopsLeft - 1 + ); + } catch { + // Not a symlink: a plain missing file, resolved via its parent below. + } + } // File may not exist yet; resolve the parent so directory symlinks are // still honored, keeping the configured basename. try { @@ -173,14 +195,29 @@ async function updateCodeWorkspaceFileLocked( return { ok: false, error: "Workspace file is not a regular file" }; } const buffer = Buffer.alloc(MAX_CODE_WORKSPACE_FILE_BYTES + 1); - const { bytesRead } = await handle.read(buffer, 0, buffer.length, 0); - if (bytesRead > MAX_CODE_WORKSPACE_FILE_BYTES) { + // read() may legally return short counts before EOF (notably on network + // filesystems), so loop until EOF or the cap is exceeded; a single read + // could silently truncate a valid file and rewrite it without its tail. + let totalRead = 0; + while (totalRead < buffer.length) { + const { bytesRead } = await handle.read( + buffer, + totalRead, + buffer.length - totalRead, + totalRead + ); + if (bytesRead === 0) { + break; + } + totalRead += bytesRead; + } + if (totalRead > MAX_CODE_WORKSPACE_FILE_BYTES) { log.warn("Skipping .code-workspace sync: file exceeds size limit", { codeWorkspacePath, }); return { ok: false, error: "Workspace file exceeds the 1 MiB sync limit" }; } - original = buffer.subarray(0, bytesRead).toString("utf-8"); + original = buffer.subarray(0, totalRead).toString("utf-8"); } finally { await handle.close(); } @@ -309,17 +346,27 @@ export function computeManagedWorktreePaths(params: { // pass the removed workspace's roots explicitly (managedRootsByProject). const roots = new Set([path.resolve(defaultManagedRootDir)]); for (const metadata of allMetadata) { - if (!belongsToProject(metadata, projectPath) || !isWorktreeRuntime(metadata.runtimeConfig)) { + if ( + !belongsToProject(metadata, projectPath) || + !hasManagedHostWorktree(metadata.runtimeConfig) + ) { continue; } - roots.add( - path.resolve( - path.join( - expandTilde(metadata.runtimeConfig.srcBaseDir), - participantCheckoutDirName(metadata, projectPath) + if (isWorktreeRuntime(metadata.runtimeConfig)) { + roots.add( + path.resolve( + path.join( + expandTilde(metadata.runtimeConfig.srcBaseDir), + participantCheckoutDirName(metadata, projectPath) + ) ) - ) - ); + ); + } else { + // Devcontainer host worktrees live under /; + // for a sub-project participant that differs from defaultManagedRootDir, + // so derive the root from the checkout itself. + roots.add(path.dirname(path.resolve(metadata.namedWorkspacePath))); + } } const managedRootDirs = [...roots].sort(); @@ -475,22 +522,33 @@ export async function syncProjectCodeWorkspace( const roots = new Set( (options?.extraManagedRootDirs ?? []).map((rootDir) => path.resolve(rootDir)) ); - const participantPaths: string[] = []; + const candidates: Array<{ participantPath: string; participantFile: string }> = []; for (const [participantPath, participantConfig] of currentProjects) { const participantFile = resolveConfiguredCodeWorkspacePath( participantPath, participantConfig.codeWorkspaceSyncPath ); - if (participantFile === null) { - continue; + if (participantFile !== null) { + candidates.push({ participantPath, participantFile }); } - // A stalled canonicalization of an unrelated project's path only - // drops that project from this round; it cannot block the sync. - if (participantFile !== targetFile) { - const participantCanonical = await boundedRealPath(participantFile); - if (participantCanonical !== canonicalTarget) { - continue; + } + // Canonicalize candidates concurrently: several stalled mounts under + // unrelated projects' paths cost one shared REALPATH_TIMEOUT_MS in + // total instead of one each, and a stalled path only drops that + // project from this round; it cannot block the sync. + const matchedPaths = await Promise.all( + candidates.map(async ({ participantPath, participantFile }) => { + if (participantFile === targetFile) { + return participantPath; } + const participantCanonical = await boundedRealPath(participantFile); + return participantCanonical === canonicalTarget ? participantPath : null; + }) + ); + const participantPaths: string[] = []; + for (const participantPath of matchedPaths) { + if (participantPath === null) { + continue; } const computed = computeManagedWorktreePaths({ allMetadata, From 0247540cc5f11d8bfff36397a4e5946db537a84d Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 12:45:37 +0000 Subject: [PATCH 08/11] review: validate resolved .code-workspace symlink targets before writing --- src/node/worktree/codeWorkspaceSync.test.ts | 35 +++++++++++++++++++++ src/node/worktree/codeWorkspaceSync.ts | 10 ++++++ 2 files changed, 45 insertions(+) diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index df3022b3cbc..87f7b375a98 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -244,6 +244,41 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: theirs }]); }); + test("refuses to write through a symlink whose target is not a .code-workspace file", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + const externalJson = path.join(tempDir, "external.json"); + const externalContent = JSON.stringify({ folders: [{ path: "/user/data" }] }); + await fsPromises.writeFile(externalJson, externalContent); + const linkPath = path.join(tempDir, "proj.code-workspace"); + await fsPromises.symlink(externalJson, linkPath); + + const result = await updateCodeWorkspaceFile({ + codeWorkspacePath: linkPath, + managedRootDirs: [managedRootDir], + desiredPaths: [worktree], + seedFolders: [worktree], + }); + + expect(result.ok).toBe(false); + expect(await fsPromises.readFile(externalJson, "utf-8")).toBe(externalContent); + + // The dangling variant must not create the non-extension target either. + const danglingLink = path.join(tempDir, "dangling-bad.code-workspace"); + await fsPromises.symlink(path.join(tempDir, "planted.json"), danglingLink); + const danglingResult = await updateCodeWorkspaceFile({ + codeWorkspacePath: danglingLink, + managedRootDirs: [managedRootDir], + desiredPaths: [worktree], + seedFolders: [worktree], + }); + expect(danglingResult.ok).toBe(false); + const plantedExists = await fsPromises.stat(path.join(tempDir, "planted.json")).then( + () => true, + () => false + ); + expect(plantedExists).toBe(false); + }); + test("creates a dangling symlink's target instead of replacing the link", async () => { const worktree = path.join(managedRootDir, "feature-a"); const target = path.join(tempDir, "real-target.code-workspace"); diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index c804c9ed68a..b77f2426aa7 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -162,6 +162,16 @@ async function updateCodeWorkspaceFileLocked( update: CodeWorkspaceFileUpdate ): Promise { const { codeWorkspacePath, managedRootDirs, desiredPaths } = update; + // SECURITY: the configured path was extension-validated, but a symlink at + // that path can live inside the checkout (repo-controlled) and point + // anywhere. Re-validate the RESOLVED target so a planted link cannot + // redirect the write into an arbitrary JSON file (or create one). + if (!targetPath.endsWith(CODE_WORKSPACE_EXTENSION)) { + log.warn("Skipping .code-workspace sync: resolved target is not a .code-workspace file", { + codeWorkspacePath, + }); + return { ok: false, error: "Workspace file symlink does not target a .code-workspace file" }; + } // Relative folder entries resolve against the configured file location, // matching how VS Code resolves them for the file the user opens. const fileDir = path.dirname(codeWorkspacePath); From feb51a2bee17d977051b420d7fdfe53748f4fd1b Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:11:46 +0000 Subject: [PATCH 09/11] review: address Codex round-7 correctness/security findings --- docs/runtime/worktree.mdx | 2 +- src/node/worktree/codeWorkspaceSync.test.ts | 34 +++++++++++++++++++++ src/node/worktree/codeWorkspaceSync.ts | 30 +++++++++++++++--- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/docs/runtime/worktree.mdx b/docs/runtime/worktree.mdx index de823954eb1..ab3d3bac08b 100644 --- a/docs/runtime/worktree.mdx +++ b/docs/runtime/worktree.mdx @@ -35,4 +35,4 @@ Example layout: To browse all of a project's worktrees from one VS Code or code-server window, set a `.code-workspace` file path per project in Settings → Runtimes (select the project scope). Xum keeps that file's folder list in sync as worktree workspaces are created, renamed, archived, and deleted. -Xum only manages folder entries under the project's worktree directory; folders you add yourself, comments, and `settings`/`extensions` blocks are left untouched. +Xum only manages folder entries under the project's worktree directory; folders you add yourself and everything outside the `folders` array (comments, `settings`/`extensions` blocks) are left untouched. When Xum adds or removes an entry it rewrites the `folders` array itself, so comments placed inside that array are not preserved. diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index 87f7b375a98..6af2ed3aab1 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -7,7 +7,9 @@ import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { Config } from "@/node/config"; import { MAX_CODE_WORKSPACE_FILE_BYTES, + MAX_CODE_WORKSPACE_FOLDERS, computeManagedWorktreePaths, + managedRootsByProject, syncProjectCodeWorkspace, updateCodeWorkspaceFile, } from "./codeWorkspaceSync"; @@ -244,6 +246,25 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: theirs }]); }); + test("skips files with more folder entries than the sync cap", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + const filePath = path.join(tempDir, "huge.code-workspace"); + const content = JSON.stringify({ + folders: Array.from({ length: MAX_CODE_WORKSPACE_FOLDERS + 1 }, () => ({})), + }); + await fsPromises.writeFile(filePath, content); + + const result = await updateCodeWorkspaceFile({ + codeWorkspacePath: filePath, + managedRootDirs: [managedRootDir], + desiredPaths: [worktree], + seedFolders: [worktree], + }); + + expect(result.ok).toBe(false); + expect(await fsPromises.readFile(filePath, "utf-8")).toBe(content); + }); + test("refuses to write through a symlink whose target is not a .code-workspace file", async () => { const worktree = path.join(managedRootDir, "feature-a"); const externalJson = path.join(tempDir, "external.json"); @@ -608,6 +629,19 @@ describe("computeManagedWorktreePaths", () => { expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); }); + test("managedRootsByProject derives devcontainer cleanup roots from the checkout", () => { + const subProjectPath = `${projectPath}/packages/api`; + const metadata = makeMetadata({ + subProjectPath, + runtimeConfig: { type: "devcontainer", configPath: ".devcontainer/devcontainer.json" }, + }); + const roots = managedRootsByProject(metadata); + // Cleanup after removal/reassignment must retain the parent checkout root, + // matching what computeManagedWorktreePaths derives while the metadata exists. + expect(roots.get(subProjectPath)).toEqual([managedRoot]); + expect(roots.get(projectPath)).toEqual([managedRoot]); + }); + test("keeps a devcontainer workspace assigned to a sub-project under the parent root", () => { const subProjectPath = `${projectPath}/packages/api`; // Devcontainer host worktrees live under the PARENT project's directory; diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index b77f2426aa7..fe89cb56e13 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -32,6 +32,7 @@ export const CODE_WORKSPACE_EXTENSION = ".code-workspace"; // synchronous (no timeout can preempt it), so cap the bytes we are willing to // read and parse. Real .code-workspace files are a few KiB. export const MAX_CODE_WORKSPACE_FILE_BYTES = 1024 * 1024; +export const MAX_CODE_WORKSPACE_FOLDERS = 2_000; // Bound each sync: a configured file on an unavailable network mount can hang // fs calls indefinitely, and workspace lifecycle operations await syncs. @@ -263,6 +264,17 @@ async function updateCodeWorkspaceFileLocked( log.warn("Skipping .code-workspace sync: 'folders' is not an array", { codeWorkspacePath }); return { ok: false, error: "Workspace file 'folders' is not an array" }; } + // SECURITY: jsonc.modify serialization is synchronous and superlinear in + // entry count, so a repo-controlled file within the byte cap can still hold + // tens of thousands of entries and freeze the main thread for >10s. Real + // multi-root workspaces have well under this many folders. + if (Array.isArray(existingFolders) && existingFolders.length > MAX_CODE_WORKSPACE_FOLDERS) { + log.warn("Skipping .code-workspace sync: too many folder entries", { + codeWorkspacePath, + folderCount: existingFolders.length, + }); + return { ok: false, error: "Workspace file has too many folder entries to sync" }; + } const desired = new Set(desiredPaths.map((desiredPath) => path.resolve(desiredPath))); @@ -437,10 +449,10 @@ export function computeManagedWorktreePaths(params: { */ export function managedRootsByProject(metadata: FrontendWorkspaceMetadata): Map { const rootsByProject = new Map(); - if (!isWorktreeRuntime(metadata.runtimeConfig)) { + const runtimeConfig = metadata.runtimeConfig; + if (!hasManagedHostWorktree(runtimeConfig)) { return rootsByProject; } - const srcBaseDir = expandTilde(metadata.runtimeConfig.srcBaseDir); const involved = new Set([ metadata.projectPath, ...(metadata.subProjectPath != null ? [metadata.subProjectPath] : []), @@ -448,9 +460,17 @@ export function managedRootsByProject(metadata: FrontendWorkspaceMetadata): Map< ]); for (const involvedPath of involved) { const normalized = stripTrailingSlashes(involvedPath); - rootsByProject.set(normalized, [ - path.resolve(path.join(srcBaseDir, participantCheckoutDirName(metadata, normalized))), - ]); + const root = isWorktreeRuntime(runtimeConfig) + ? path.resolve( + path.join( + expandTilde(runtimeConfig.srcBaseDir), + participantCheckoutDirName(metadata, normalized) + ) + ) + : // Devcontainer host worktrees live under the parent project's + // directory, mirroring computeManagedWorktreePaths. + path.dirname(path.resolve(metadata.namedWorkspacePath)); + rootsByProject.set(normalized, [root]); } return rootsByProject; } From 9a5ec0dcaa3c0d3387967ee8d4253fa677cead99 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:35:49 +0000 Subject: [PATCH 10/11] review: exclude transcript-only workspaces from .code-workspace sync --- src/node/orpc/router.test.ts | 2 ++ .../agentSkills/builtInSkillContent.generated.ts | 2 +- src/node/worktree/codeWorkspaceSync.test.ts | 14 ++++++++++++++ src/node/worktree/codeWorkspaceSync.ts | 5 +++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/node/orpc/router.test.ts b/src/node/orpc/router.test.ts index f15ddd9669e..f27c352639a 100644 --- a/src/node/orpc/router.test.ts +++ b/src/node/orpc/router.test.ts @@ -1252,6 +1252,8 @@ describe("projects.setCodeWorkspaceSyncPath", () => { fs.mkdirSync(subProjectA, { recursive: true }); fs.mkdirSync(subProjectB, { recursive: true }); const worktreePath = path.join(config.srcDir, "project", "feat-1"); + // Checkout must exist on disk or metadata is marked transcript-only. + fs.mkdirSync(worktreePath, { recursive: true }); await config.editConfig((current) => { current.projects.set(projectPath, { workspaces: [ diff --git a/src/node/services/agentSkills/builtInSkillContent.generated.ts b/src/node/services/agentSkills/builtInSkillContent.generated.ts index 32ef568edaa..0dab9754dcd 100644 --- a/src/node/services/agentSkills/builtInSkillContent.generated.ts +++ b/src/node/services/agentSkills/builtInSkillContent.generated.ts @@ -7886,7 +7886,7 @@ export const BUILTIN_SKILL_FILES: Record> = { "", "To browse all of a project's worktrees from one VS Code or code-server window, set a `.code-workspace` file path per project in Settings → Runtimes (select the project scope). Xum keeps that file's folder list in sync as worktree workspaces are created, renamed, archived, and deleted.", "", - "Xum only manages folder entries under the project's worktree directory; folders you add yourself, comments, and `settings`/`extensions` blocks are left untouched.", + "Xum only manages folder entries under the project's worktree directory; folders you add yourself and everything outside the `folders` array (comments, `settings`/`extensions` blocks) are left untouched. When Xum adds or removes an entry it rewrites the `folders` array itself, so comments placed inside that array are not preserved.", "", ].join("\n"), "references/docs/workspaces/compaction/automatic.mdx": [ diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index 6af2ed3aab1..54eb4000742 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -398,6 +398,8 @@ describe("syncProjectCodeWorkspace", () => { const projectPath = path.join(tempDir, "repo"); await fsPromises.mkdir(projectPath, { recursive: true }); const worktreePath = path.join(config.srcDir, "repo", "feat-1"); + // Checkout must exist on disk or metadata is marked transcript-only. + await fsPromises.mkdir(worktreePath, { recursive: true }); await config.editConfig((cfg) => { cfg.projects.set(projectPath, { workspaces: [ @@ -448,6 +450,8 @@ describe("syncProjectCodeWorkspace", () => { await fsPromises.symlink(realFile, linkFile); const worktreeA = path.join(config.srcDir, "repo", "feat-a"); const worktreeB = path.join(config.srcDir, "repo", "feat-b"); + await fsPromises.mkdir(worktreeA, { recursive: true }); + await fsPromises.mkdir(worktreeB, { recursive: true }); const workspaceEntry = (worktree: string, id: string) => ({ path: worktree, id, @@ -487,6 +491,8 @@ describe("syncProjectCodeWorkspace", () => { const sharedFile = path.join(tempDir, "shared.code-workspace"); const worktreeA = path.join(config.srcDir, "repo", "feat-a"); const worktreeB = path.join(config.srcDir, "repo", "feat-b"); + await fsPromises.mkdir(worktreeA, { recursive: true }); + await fsPromises.mkdir(worktreeB, { recursive: true }); const workspaceEntry = (worktree: string, id: string) => ({ path: worktree, id, @@ -629,6 +635,14 @@ describe("computeManagedWorktreePaths", () => { expect(desiredPaths).toEqual([`${managedRoot}/feature-a`]); }); + test("excludes transcript-only workspaces whose checkout was deleted", () => { + const { desiredPaths } = computeManagedWorktreePaths({ + allMetadata: [makeMetadata({ transcriptOnly: true })], + ...computeParams, + }); + expect(desiredPaths).toEqual([]); + }); + test("managedRootsByProject derives devcontainer cleanup roots from the checkout", () => { const subProjectPath = `${projectPath}/packages/api`; const metadata = makeMetadata({ diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index fe89cb56e13..ef119d353df 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -407,6 +407,11 @@ export function computeManagedWorktreePaths(params: { if (metadata.taskIsolation === "none") { continue; } + // Transcript-only workspaces (checkout deleted, e.g. archive -> + // delete-worktree -> unarchive) have no directory to open in the editor. + if (metadata.transcriptOnly) { + continue; + } if (isWorkspaceArchived(metadata.archivedAt, metadata.unarchivedAt)) { continue; } From 9abca8a65d86e50a7b78a1deb04a4ee6a57ac872 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:25:40 +0000 Subject: [PATCH 11/11] review: cap final folder count and reject duplicate folders properties --- src/node/worktree/codeWorkspaceSync.test.ts | 45 ++++++++++++++++++++- src/node/worktree/codeWorkspaceSync.ts | 38 +++++++++++++---- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/node/worktree/codeWorkspaceSync.test.ts b/src/node/worktree/codeWorkspaceSync.test.ts index 54eb4000742..306870810a7 100644 --- a/src/node/worktree/codeWorkspaceSync.test.ts +++ b/src/node/worktree/codeWorkspaceSync.test.ts @@ -246,11 +246,12 @@ describe("updateCodeWorkspaceFile", () => { expect(parseFolders(await readWorkspaceFile())).toEqual([{ path: theirs }]); }); - test("skips files with more folder entries than the sync cap", async () => { + test("refuses reconciles whose final folder count exceeds the sync cap", async () => { const worktree = path.join(managedRootDir, "feature-a"); const filePath = path.join(tempDir, "huge.code-workspace"); + // Exactly at the cap: the single addition would push the FINAL count over. const content = JSON.stringify({ - folders: Array.from({ length: MAX_CODE_WORKSPACE_FOLDERS + 1 }, () => ({})), + folders: Array.from({ length: MAX_CODE_WORKSPACE_FOLDERS }, () => ({})), }); await fsPromises.writeFile(filePath, content); @@ -265,6 +266,46 @@ describe("updateCodeWorkspaceFile", () => { expect(await fsPromises.readFile(filePath, "utf-8")).toBe(content); }); + test("refuses to create a file with more seed folders than the sync cap", async () => { + const filePath = path.join(tempDir, "seeded.code-workspace"); + const seedFolders = Array.from({ length: MAX_CODE_WORKSPACE_FOLDERS + 1 }, (_, i) => + path.join(managedRootDir, `feature-${i}`) + ); + + const result = await updateCodeWorkspaceFile({ + codeWorkspacePath: filePath, + managedRootDirs: [managedRootDir], + desiredPaths: seedFolders, + seedFolders, + }); + + expect(result.ok).toBe(false); + const exists = await fsPromises.stat(filePath).then( + () => true, + () => false + ); + expect(exists).toBe(false); + }); + + test("rejects files with duplicate top-level folders properties", async () => { + const worktree = path.join(managedRootDir, "feature-a"); + const filePath = path.join(tempDir, "dupe.code-workspace"); + // jsonc.parse reads the last property but jsonc.modify edits the first, so + // a reconcile would silently no-op while reporting success. + const content = `{"folders": [], "folders": [{"path": "/user/data"}]}`; + await fsPromises.writeFile(filePath, content); + + const result = await updateCodeWorkspaceFile({ + codeWorkspacePath: filePath, + managedRootDirs: [managedRootDir], + desiredPaths: [worktree], + seedFolders: [worktree], + }); + + expect(result.ok).toBe(false); + expect(await fsPromises.readFile(filePath, "utf-8")).toBe(content); + }); + test("refuses to write through a symlink whose target is not a .code-workspace file", async () => { const worktree = path.join(managedRootDir, "feature-a"); const externalJson = path.join(tempDir, "external.json"); diff --git a/src/node/worktree/codeWorkspaceSync.ts b/src/node/worktree/codeWorkspaceSync.ts index ef119d353df..845d060f080 100644 --- a/src/node/worktree/codeWorkspaceSync.ts +++ b/src/node/worktree/codeWorkspaceSync.ts @@ -240,6 +240,13 @@ async function updateCodeWorkspaceFileLocked( } if (original === null) { + if (update.seedFolders.length > MAX_CODE_WORKSPACE_FOLDERS) { + log.warn("Skipping .code-workspace sync: too many folder entries", { + codeWorkspacePath, + folderCount: update.seedFolders.length, + }); + return { ok: false, error: "Workspace file has too many folder entries to sync" }; + } const fresh = { folders: update.seedFolders.map((folderPath) => ({ path: folderPath })) }; await fsPromises.mkdir(path.dirname(targetPath), { recursive: true }); await writeFileAtomic(targetPath, JSON.stringify(fresh, null, "\t") + "\n"); @@ -264,16 +271,19 @@ async function updateCodeWorkspaceFileLocked( log.warn("Skipping .code-workspace sync: 'folders' is not an array", { codeWorkspacePath }); return { ok: false, error: "Workspace file 'folders' is not an array" }; } - // SECURITY: jsonc.modify serialization is synchronous and superlinear in - // entry count, so a repo-controlled file within the byte cap can still hold - // tens of thousands of entries and freeze the main thread for >10s. Real - // multi-root workspaces have well under this many folders. - if (Array.isArray(existingFolders) && existingFolders.length > MAX_CODE_WORKSPACE_FOLDERS) { - log.warn("Skipping .code-workspace sync: too many folder entries", { + // jsonc.parse reads the LAST duplicate property while jsonc.modify edits the + // FIRST, so a hand-edited file with duplicate `folders` keys would silently + // stop syncing while reporting success; reject it instead. + const rootNode = jsonc.parseTree(original, undefined, { allowTrailingComma: true }); + const foldersPropCount = + rootNode?.children?.filter( + (prop) => prop.type === "property" && prop.children?.[0]?.value === "folders" + ).length ?? 0; + if (foldersPropCount > 1) { + log.warn("Skipping .code-workspace sync: duplicate 'folders' properties", { codeWorkspacePath, - folderCount: existingFolders.length, }); - return { ok: false, error: "Workspace file has too many folder entries to sync" }; + return { ok: false, error: "Workspace file has duplicate 'folders' properties" }; } const desired = new Set(desiredPaths.map((desiredPath) => path.resolve(desiredPath))); @@ -303,6 +313,18 @@ async function updateCodeWorkspaceFileLocked( return { ok: true }; } const newFolders = [...kept, ...additions.map((folderPath) => ({ path: folderPath }))]; + // SECURITY: jsonc.modify serialization is synchronous and superlinear in + // entry count, so a file within the byte cap can still hold tens of + // thousands of entries and freeze the main thread for >10s. Checked on the + // FINAL count (not the input) so a write can never push a file over the cap + // and brick later syncs. Real multi-root workspaces stay far below it. + if (newFolders.length > MAX_CODE_WORKSPACE_FOLDERS) { + log.warn("Skipping .code-workspace sync: too many folder entries", { + codeWorkspacePath, + folderCount: newFolders.length, + }); + return { ok: false, error: "Workspace file has too many folder entries to sync" }; + } const text = jsonc.applyEdits( original, jsonc.modify(original, ["folders"], newFolders, MODIFY_OPTIONS)