From f2c1a28eab81966747caa60e19db3da427bb5cda Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 22 Jun 2026 00:26:40 -0700 Subject: [PATCH] fix(workspace): re-read task before workspaceWorktrees merge; FNXC format Addresses the follow-up review on the foundation fixes: - Re-read the task via store.getTask immediately before merging the per-repo entry, so a concurrent sibling-repo acquisition that landed since the initial read isn't clobbered by updateTask's wholesale map replace (narrows the read-modify-write window to the store lock; a fully atomic per-repo store-level merge remains a follow-up). - Normalize the inline FNXC comment to the FNXC:Area yyyy-MM-dd-hh:mm: convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/engine/src/worktree-acquisition.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/engine/src/worktree-acquisition.ts b/packages/engine/src/worktree-acquisition.ts index 91fbc75bcd..6e8c4febb4 100644 --- a/packages/engine/src/worktree-acquisition.ts +++ b/packages/engine/src/worktree-acquisition.ts @@ -632,7 +632,7 @@ export async function acquireWorkspaceRepoWorktree( const { repoRelPath, workspaceRootDir, task, store, settings, logger, secretsStore, runContext, audit, runConfiguredCommand, taskEnv } = opts; const { join, isAbsolute, normalize, sep } = await import("node:path"); - // FNXC:WorkspaceWorktree 2026-06-22 — reject absolute / `..`-escaping repo paths before resolving. + // FNXC:WorkspaceWorktree 2026-06-22-00:00: reject absolute / `..`-escaping repo paths before resolving. assertInRootRepoRelPath(repoRelPath, sep, isAbsolute, normalize); const repoAbsPath = join(workspaceRootDir, repoRelPath); @@ -692,8 +692,17 @@ export async function acquireWorkspaceRepoWorktree( runInitCommand: true, }); + /* + FNXC:WorkspaceWorktree 2026-06-22-00:00: + Re-read the task immediately before merging so a concurrent sibling-repo acquisition that + landed between our initial read and now is not clobbered — `updateTask` replaces the + `workspaceWorktrees` map wholesale, so we must merge onto the freshest map, not the stale + snapshot captured before `acquireTaskWorktree`. This narrows the read-modify-write window to + the store's own lock; a fully atomic per-repo store-level merge is a follow-up. + */ + const freshTask = (await store.getTask(task.id)) ?? task; const updated: Record = { - ...(task.workspaceWorktrees ?? {}), + ...(freshTask.workspaceWorktrees ?? {}), [repoRelPath]: { worktreePath: result.worktreePath, branch: result.branch }, }; /*