diff --git a/.changeset/workspace-dashboard-floor.md b/.changeset/workspace-dashboard-floor.md new file mode 100644 index 0000000000..47db598dea --- /dev/null +++ b/.changeset/workspace-dashboard-floor.md @@ -0,0 +1,8 @@ +--- +"@runfusion/fusion": minor +--- + +Workspace tasks no longer render blank in the dashboard. Task cards and the task +detail view now surface a workspace task's acquired per-sub-repo worktrees as a +read-only "N repos acquired" placeholder and flat repo → worktree/branch list, +instead of an empty branch area (no `task.worktree`/`task.branch`). diff --git a/.changeset/workspace-per-repo-acquisition-hardening.md b/.changeset/workspace-per-repo-acquisition-hardening.md new file mode 100644 index 0000000000..3ce549f618 --- /dev/null +++ b/.changeset/workspace-per-repo-acquisition-hardening.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": minor +--- + +Workspace mode (Phase A / U2): harden per-repo worktree acquisition. Each sub-repo worktree now gets the task identity guard installed (single-repo parity), a per-repo base commit SHA captured local-first against that sub-repo's resolved integration branch (shared `integrationBranch` override stripped so each repo falls through to its own `origin/HEAD`), and same-sub-repo acquisition exclusivity registered in the path-keyed active-session registry. Re-acquiring an already-acquired `(taskId, repo)` is idempotent, and acquisition failures surface an error plus an audit event instead of silently stalling. diff --git a/.changeset/workspace-phase-a-u1-executor-session-scoping.md b/.changeset/workspace-phase-a-u1-executor-session-scoping.md new file mode 100644 index 0000000000..6fc9911756 --- /dev/null +++ b/.changeset/workspace-phase-a-u1-executor-session-scoping.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": minor +--- + +Workspace mode Phase A (U1): executor session scoping. In workspace mode the executor now skips the root worktree acquisition and every rootDir git preflight (base-commit capture, contamination, worktree-liveness), runs the agent session rooted at the browse-only workspace root, and tracks acquired sub-repo worktrees as a per-task set. Single-repo tasks are unchanged (one-element set, byte-for-byte preflight parity). diff --git a/CONCEPTS.md b/CONCEPTS.md index 8b75953f2c..2afdad7e97 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -61,6 +61,10 @@ sub-directories. Fusion discovers sub-repos at init time and records them in single root-level worktree; instead, the agent acquires per-repo worktrees on demand via `fn_acquire_repo_worktree`. +Workspace-task merges are **non-atomic**: each sub-repo lands on its own local +integration ref independently, so a partial-land window (some sub-repos merged, +others not) is possible mid-task — this state is local and operator-resettable. + ### Project Identity The durable identity a registered Project carries locally so it can be reattached to the central registry after central state is lost or rebuilt, preserving rows keyed by the same project id instead of minting a replacement. diff --git a/docs/plans/2026-06-21-004-feat-workspace-phase-a-plan.md b/docs/plans/2026-06-21-004-feat-workspace-phase-a-plan.md new file mode 100644 index 0000000000..f8b4220cb6 --- /dev/null +++ b/docs/plans/2026-06-21-004-feat-workspace-phase-a-plan.md @@ -0,0 +1,176 @@ +--- +title: "feat: Workspace mode Phase A — session scoping, per-repo acquisition, dashboard floor" +status: active +date: 2026-06-21 +type: feat +origin: docs/plans/2026-06-21-002-feat-workspace-mode-execution-model-plan.md (master plan, Phase A / U1·U2·U10) +depth: deep +--- + +# feat: Workspace mode Phase A — session scoping, per-repo acquisition, dashboard floor + +> **ID namespace:** the `U1·U2·U3` below are **local to this Phase-A plan**. They decompose master-plan **U1, U2, U10** (a separate namespace). "Master-plan U6/U8" references point at the master plan, not these IDs. + +## Summary + +Phase A of the workspace-mode master plan: make a workspace task **run** (acquire → browse → edit per sub-repo), short of capture/review/merge (Phases B–D). Three units: (U1) executor session scoping so the session roots at the non-git workspace root and edits happen only in per-repo worktrees; (U2) per-repo acquisition hardening (identity guard, per-repo base SHA against the resolved integration branch, same-sub-repo exclusivity); (U3 = master U10) a dashboard "doesn't look broken" floor. + +Builds on the **foundation** (PR #1710 — `task.workspaceWorktrees`, `fn_acquire_repo_worktree`, `acquireWorkspaceRepoWorktree`) + **U0** (PR #1711 — `runAiMerge` sole merge path, R7 guard). Settled design: **D2/D3/D5 — land-as-you-go on each repo's LOCAL integration ref** (no remote push), session-time coherence accepted. The R7 merge-boundary guard already exists at the merge chokepoint (U0); U1 must not route around it. + +**Scope out:** capture/contamination/review (master U3/U4 = Phase B), the per-repo merge loop (master U6 = Phase C), self-healing reconcilers (master U8 = Phase D). + +**Stacking:** this branch is off the U0 branch, so the PR diff includes foundation + U0 + Phase A and **must not merge until #1710/#1711 land**. + +--- + +## Problem Frame + +In workspace mode `rootDir` is a **non-git** parent. On the current base the executor still, for every task: acquires one root worktree at `executor.ts:~7430` (`acquireTaskWorktree({rootDir})`), runs preflights (`resolveContaminationBaseRef`, `captureBaseCommitSha`, identity-guard install, `verifyWorktreeInvariants`) against that path, binds the agent session cwd to it, and tracks `activeWorktrees: Map`. Against a non-git root, the root acquisition and every git preflight fail. The foundation gave the agent `fn_acquire_repo_worktree` (per-repo worktrees on demand) but nothing in the executor lifecycle skips the root path or hardens per-repo acquisition. Phase A closes that gap for the **run** stage. + +--- + +## Key Technical Decisions + +### KTD1 — Skip root acquisition + all rootDir preflights; session cwd = workspace root (master KTD1) +When `this.workspaceConfig` is present: skip `acquireTaskWorktree({rootDir})` and gate each intervening preflight so none runs git against the non-git root; set session cwd = `this.rootDir` (browse-only); do not set `task.worktree`; `scopePromptToWorktree` is a no-op. The non-workspace path stays byte-for-byte unchanged (branch on `workspaceConfig`). + +### KTD2 — `activeWorktrees` becomes `taskId → Set` (master KTD1) — VERIFIED consumer list +A workspace task holds N sub-repo worktrees; liveness/owner checks must see all of them. Convert the map and update **every** consumer to membership semantics. The complete, code-verified consumer set (feasibility-checked — the earlier draft mislabeled these): +- **Membership / owner checks:** `findActiveWorktreeOwner` (`:14491`), `hasActiveWorktreeBinding` (`:14518`), the FN-6736 phantom-binding reclaim (`~:2055`). +- **`listWorktreeHolders` (`:14480`)** — emits one `{taskId, worktreePath}` per entry; consumed by the **FN-6782 leaked-slot reaper** (`self-healing.ts:~8310`) and `in-process-runtime.ts:~791`. A workspace task must **flat-map its Set into N holder rows**, or `maxWorktrees`-slot accounting under-counts and leaks/mis-reaps. Verify the reaper math against multi-row holders. +- **Single-path getters — define the Set-collapse contract (KTD-decision):** `getWorktreePath(taskId): string|undefined` (`:15424`), the `verifyWorktreeInvariants` resolution `?? this.activeWorktrees.get(task.id)` (`:10461`), and the conflict-set iteration (`~:14444`, `worktreePath === conflictPath`). **Contract:** for a workspace task these single-path consumers operate per-sub-repo (the caller already has the repo/path in context) — `getWorktreePath` returns `undefined` for a multi-worktree workspace task (callers must use the per-repo `workspaceWorktrees` entry), and `verifyWorktreeInvariants` is iterated per worktree in Phase B (master U3), so its singular resolution is gated off in workspace mode here. +- **Unregister resolvers (`:1586`/`:1603`/`:1618`)** — `deleteActiveSession`/`StepExecutor`/`WorkflowStepSession` each read one path for `activeSessionRegistry.unregisterPath`; with a Set they must unregister **every** path (loop), not one. Plus cleanup at `~:14922`. + +Non-workspace tasks hold a one-element set — behavior unchanged. **Grep all `activeWorktrees.` sites before declaring done** (FN-5893); the list above is the verification spine, not a license to skip the grep. + +### KTD3 — Per-repo base SHA against the *resolved* integration branch, local-first (master KTD3) +`resolveCapturedBaseCommitSha` (`base-commit-capture.ts:26-55`) **hardcodes `main`** and takes `(worktreePath, logger?)`. Extend it to accept the integration branch as an **optional trailing param defaulting to the current `main` literal**, so the existing single-repo caller (`executor.ts:~12075`) and the 4 `base-commit-capture.real-git.test.ts` cases stay green without change. At each sub-repo acquisition capture `baseCommitSha` measured **local-first** (`merge-base HEAD || origin/`), per `docs/solutions/logic-errors/files-changed-inflated-by-origin-first-base-commit.md`. + +> **Integration-branch resolution gotcha (feasibility-verified):** `resolveIntegrationBranch(rootDir, settings)` (`integration-branch.ts:74`) checks `resolveFromSettings(settings)` **FIRST** and returns a populated `settings.integrationBranch` before ever consulting the repo's `origin/HEAD`. So `resolveIntegrationBranch(repoAbsPath, settings)` would return the **shared** override for every sub-repo — the exact thing KTD3 forbids. **Call it with the shared override stripped:** `resolveIntegrationBranch(repoAbsPath, { ...settings, integrationBranch: undefined })`, so each sub-repo falls through to its own `origin/HEAD`. Store as `workspaceWorktrees[repo].baseCommitSha`. + +### KTD4 — Same-sub-repo exclusivity via `activeSessionRegistry` path-keying, not the pool (master KTD6) +`WorktreePool` is a recycle cache (gated on `recycleWorktrees`), **not** a cross-task lock. Serialize two concurrent workspace tasks contending for the same sub-repo via a repo-path exclusivity registry built on `activeSessionRegistry` path-keying (which `runAiMerge` already uses), registered **at acquisition** (U2). Disjoint-scope contention on the same sub-repo is otherwise unprotected (file-scope leases don't catch it). + +### KTD5 — Dashboard floor only (master U10) +Nil-guard components that render `task.worktree`/`task.branch` so a workspace task (no `task.worktree`, populated `workspaceWorktrees`) shows a placeholder or flat per-repo list, never a crash/empty. Ceiling: "doesn't look broken" — no rich per-repo-status component (deferred registration UI). Plus a one-line non-atomic-merge-semantics note in `CONCEPTS.md`/`docs/dashboard-guide.md`. + +--- + +## Implementation Units + +> **Standing requirements (every unit):** `FNXC:Workspace ` comments at non-obvious decision points; a `.changeset/*.md` (`@runfusion/fusion: minor`); FN-5048 (narrow seams, real git only where an invariant requires it, fake timers over polling, no mock-the-world); FN-5893 surface enumeration (update every enumerated consumer, don't half-convert); merge gate (`pnpm lint`, typecheck, `pnpm build`, `pnpm test:gate`). Branch off the U0 branch — do not commit to `main` or the U0 branch. + +### U1. Executor session scoping — skip root acquisition + preflights, browse-only root, activeWorktrees Set + +**Goal:** In workspace mode the executor skips root acquisition and every rootDir git preflight, runs the session rooted at the workspace dir, and tracks per-task worktree *sets*. + +**Requirements:** KTD1, KTD2. + +**Dependencies:** none (foundation + U0 present on the base). + +**Files:** +- `packages/engine/src/executor.ts` (acquisition `~:7430`; preflights `:7525` base capture, `:7536` contamination, identity-guard install, `verifyWorktreeInvariants`; session create `~:8443-8494`; retry session `~:8935`; `activeWorktrees` `:7667` + consumers `findActiveWorktreeOwner`/`hasActiveWorktreeBinding`/`getActiveWorktreeHolders`/FN-6736 reclaim `~:2055`/getters `~:1585`/`:14491`/`:14518`; `scopePromptToWorktree`) +- `packages/engine/src/__tests__/executor-workspace.test.ts` (**rewrite** — replace the `vi.mock`-the-subject tests with a **real two-repo git fixture harness** reusable by U2 and later phases) + +**Approach:** Gate the root acquisition + each preflight behind `!this.workspaceConfig`. In workspace mode set session cwd = `this.rootDir`, leave `task.worktree` unset, no-op `scopePromptToWorktree`. Convert `activeWorktrees` to `taskId → Set`; update each enumerated consumer to membership semantics (a non-workspace task = a one-element set). Mirror the existing `this.workspaceConfig === undefined` lazy-load guard at `executor.ts:7413-7418`. + +**Execution note:** Build the real two-repo fixture harness first (create temp git repos, branch, commit); the foundation's self-mocking test proves nothing. The harness is shared infrastructure for the rest of the phases. + +**Test scenarios:** +- Workspace config present → root `acquireTaskWorktree` NOT called; no preflight runs git against rootDir; session `cwd === rootDir`. (happy path) +- Non-workspace task → acquisition + every preflight called exactly as before; `cwd === worktreePath`. (regression — the singular path is untouched) +- Each enumerated `activeWorktrees` consumer returns correct results when a task holds two sub-repo paths (membership, not equality). (integration) +- Retry session in workspace mode uses `cwd === rootDir`. (edge) +- Workspace task that acquires zero sub-repos reaches `fn_task_done` without throwing on missing `task.worktree`. (edge/empty) + +**Verification:** A workspace task starts a session at the workspace root with no root worktree and no rootDir git preflight; `activeWorktrees` reflects all acquired sub-repo paths; a single-repo task is unchanged. + +--- + +### U2. Per-repo acquisition hardening — identity guard, per-repo base SHA, same-repo exclusivity + +**Goal:** Each sub-repo worktree gets identity hooks, a correct per-repo base SHA (local-first, resolved integration branch), and same-sub-repo concurrency protection — all at acquisition. + +**Requirements:** KTD3, KTD4. + +**Dependencies:** U1 (shares the fixture harness). + +**Files:** +- `packages/engine/src/worktree-acquisition.ts` (`acquireWorkspaceRepoWorktree` `~:598-650`) +- `packages/engine/src/base-commit-capture.ts` (**extend `resolveCapturedBaseCommitSha` to accept the integration branch** — it hardcodes `main`) +- `packages/engine/src/worktree-hooks.ts` (`installTaskWorktreeIdentityGuard`) +- `activeSessionRegistry` path-keying (repo-path exclusivity registry — KTD4; NOT `worktree-pool.ts`) +- `packages/core/src/types.ts` (extend the `Task.workspaceWorktrees` entry with `baseCommitSha?`) +- `packages/engine/src/__tests__/worktree-acquisition-workspace.test.ts` (new — real two-repo git fixture) + +**Approach:** After `acquireTaskWorktree` returns for a sub-repo: (1) install the identity guard via `installTaskWorktreeIdentityGuard`, passing the **same settings args the executor passes** at `executor.ts:14035-14040` (`commitMsgHookEnabled`, `taskPrefix`, `taskAttributionTrailerName`) for single-repo parity — note `acquireWorkspaceRepoWorktree` calls `acquireTaskWorktree` *without* a `createWorktree` override, so the default backend installs **no** guard today (this work is genuinely missing); (2) resolve the integration branch via `resolveIntegrationBranch(repoAbsPath, { ...settings, integrationBranch: undefined })` (strip the shared override — KTD3 gotcha) and capture `baseCommitSha` via the extended `resolveCapturedBaseCommitSha(worktreePath, logger?, integrationBranch?)`; (3) persist `baseCommitSha` into `workspaceWorktrees[repo]`; (4) register same-sub-repo exclusivity in the `activeSessionRegistry` path-keyed registry — choose a **distinct registry kind/ownerKey** for the acquisition-time exclusivity entry so it does not collide with the executor's later session registration on the same sub-repo path (the registry exposes `registerPath`/`lookupByPath`/`isPathActive`/`pathsForTask`). Idempotent across `(taskId, repo)` (re-acquire returns the existing entry, no re-install/re-capture). + +**Execution note:** Real two-repo fixture; commit-without-pushing to exercise the local-ahead-of-origin invariant. + +**Test scenarios:** +- Acquiring repo A captures `baseSha_A` = the local integration tip even when `origin/` is behind. Covers the inflation invariant. (happy path + regression) +- A sub-repo whose integration branch is **not** `main` captures against that branch and does not inherit a shared `settings.integrationBranch`. (KTD3 correction) +- Identity-guard hook present; a commit on a non-`fusion/` branch is rejected. (integration) +- Two concurrent workspace tasks acquiring the same sub-repo (even with disjoint in-repo scopes) are serialized by the exclusivity registry. (concurrency — KTD4) +- Re-acquiring repo A returns the existing entry without re-capture/re-install. (idempotency) +- Acquisition failure persists an audit event and surfaces an error (no swallowed stall). (error path) + +**Verification:** Each sub-repo worktree has identity hooks, a correct per-repo base SHA (local-first, right branch), and same-sub-repo concurrency protection registered at acquisition. + +--- + +### U3. Dashboard "doesn't look broken" floor (master U10) + +**Goal:** Existing task views render a workspace task (no `task.worktree`, populated `workspaceWorktrees`) without breakage. + +**Requirements:** KTD5. + +**Dependencies:** none (independent of U1/U2; reads the data shape the foundation already added). + +**Files:** +- Each `packages/dashboard/app/` component that reads `task.worktree`/`task.branch` for display (grep and enumerate during implementation — task detail view + any task-row/summary) +- `CONCEPTS.md` or `docs/dashboard-guide.md` (one-line non-atomic-merge-semantics note) +- `packages/dashboard/app/__tests__/` (new — graceful render test) + +**Approach:** Add a nil-guard so each affected component renders a static placeholder (e.g. "N repos acquired") or a flat per-repo path list when `task.worktree` is absent and `workspaceWorktrees` is populated. **Ceiling:** placeholder/flat list only — a new rich per-repo-status component crosses into the deferred registration UI. Add the one-line semantics note (workspace-task merges are non-atomic: repos land independently on local integration refs; partial-land is local + operator-resettable). + +**Test scenarios:** +- Task with `task.worktree` undefined + two `workspaceWorktrees` entries → renders a per-repo list/placeholder, no crash/empty. (happy path) +- Single-repo task → unchanged. (regression) + +**Verification:** Workspace tasks are observable (not broken) in the dashboard. + +--- + +## Scope Boundaries + +**In scope:** the **run** stage — session scoping (U1), per-repo acquisition hardening (U2), dashboard breakage floor (U3). + +### Deferred to Follow-Up Work (later master-plan phases) +- Per-repo modified-files capture, contamination, `verifyWorktreeInvariants` iteration (master U3 = Phase B). +- Per-repo review + `fn_task_done` completion verification (master U4 = Phase B). +- The shared landed predicate, per-repo `runAiMerge` clean-room loop, leases (master U5/U6/U7 = Phase C). +- Self-healing reconcilers, e2e harness (master U8/U9 = Phase D). +- Rich dashboard per-repo status / workspace registration UI. + +> **Contamination-window caveat (carried from the master plan):** U1 gates the root preflights off, but per-repo contamination/`verifyWorktreeInvariants` does not return until master U3 (Phase B). Do not run a workspace task for real until Phase B lands — Phase A delivers acquisition + browse, not a verified end-to-end run. + +--- + +## Risks & Dependencies + +- **R1 — Half-converted `activeWorktrees` consumers (FN-5893).** Missing one consumer silently breaks liveness/owner checks for multi-repo tasks. Mitigation: KTD2 enumerates every consumer; grep all `activeWorktrees.get(`/`.has(`/`===`-on-path sites before declaring done. +- **R2 — A preflight left un-gated runs git against the non-git root → crash.** Mitigation: U1 explicitly enumerates and gates each preflight between the workspace guard and session create; test asserts no rootDir git in workspace mode. +- **R3 — Base-commit inflation per repo.** Mitigation: KTD3 extends the hardcoded-`main` helper and captures local-first against the resolved branch; regression test commits-without-pushing + uses a non-`main` integration branch. +- **R4 — Same-sub-repo concurrency unprotected.** Mitigation: KTD4 registers exclusivity at acquisition (U2), not via the recycle pool. +- **R5 — Non-workspace regression.** The whole point of branching on `workspaceConfig` is parity for single-repo tasks. Mitigation: every unit carries a non-workspace "unchanged" regression test; the gate's existing engine-core suite must stay green. +- **Stacking dependency:** builds on foundation #1710 + U0 #1711; the PR diff includes both and must not merge until they land. + +--- + +## Sources & Research + +- Master plan `docs/plans/2026-06-21-002-feat-workspace-mode-execution-model-plan.md` (U1/U2/U10, KTD1/KTD3/KTD6 — KTD7 is Phase B, invariant inventory, D2/D3/D5). +- Codebase anchors (verified this session): `executor.ts` acquisition/preflight/session/`activeWorktrees`; `worktree-acquisition.ts` `acquireWorkspaceRepoWorktree`; `base-commit-capture.ts` hardcoded-`main`; `resolveIntegrationBranch`; `activeSessionRegistry` path-keying; foundation `task.workspaceWorktrees`. +- `docs/solutions/logic-errors/files-changed-inflated-by-origin-first-base-commit.md` → KTD3 (local-first base capture). +- `AGENTS.md`: FN-5048 slow-test rules, FN-5893 surface enumeration, changeset policy, merge gate. diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index d5c93d9dab..d804a752ef 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -2256,8 +2256,14 @@ export interface Task { /** * Workspace mode only. Keyed by repo path relative to workspace rootDir. * Each entry records the on-disk worktree path and git branch for one sub-repo. + * + * FNXC:Workspace 2026-06-21-20:10: + * `baseCommitSha` is the per-repo fork-point captured at acquisition (U2/KTD3) + * against that sub-repo's RESOLVED integration branch, local-first. It is the + * per-repo analogue of the single-repo base-commit capture and prevents + * cross-repo files-changed inflation when local integration is ahead of origin. */ - workspaceWorktrees?: Record; + workspaceWorktrees?: Record; steps: TaskStep[]; currentStep: number; /** diff --git a/packages/dashboard/app/components/TaskCard.tsx b/packages/dashboard/app/components/TaskCard.tsx index 3e5898ec52..6400b3b02e 100644 --- a/packages/dashboard/app/components/TaskCard.tsx +++ b/packages/dashboard/app/components/TaskCard.tsx @@ -35,6 +35,7 @@ import { extractDependencyDeleteConflict, extractLineageDeleteConflict } from ". import { MAX_AUTO_MERGE_RETRIES, type BlockerFanoutEntry } from "../hooks/useBlockerFanout"; import { useRetryWarning } from "../context/RetryWarningContext"; import { useColumnLabel } from "../i18n/labels"; +import { WorkspaceWorktreesSummary, isWorkspaceTask } from "./WorkspaceWorktreesSummary"; /** Per-branch progress snapshot (U13). Surfaced as an optional additive field * on the task payload for the parallel-window badge (U9). */ @@ -625,6 +626,17 @@ function areTaskCardPropsEqual(previous: TaskCardProps, next: TaskCardProps): bo previousTask.blockedBy === nextTask.blockedBy && previousTask.overlapBlockedBy === nextTask.overlapBlockedBy && previousTask.worktree === nextTask.worktree && + // FNXC:Workspace 2026-06-21-22:30: re-render the card when a workspace task acquires/ + // releases sub-repo worktrees so the "N repos acquired" placeholder stays current (U3). + // F7 — compare the sorted key SETS, not just the count: a same-count repo swap (one + // repo released, a different one acquired) keeps the count but must still re-render, + // otherwise the placeholder shows a stale repo set. + // FNXC:Workspace 2026-06-22-09:00: compare full VALUES, not only the key set. A + // pool-reclaim re-acquire keeps the same repo key but produces a different + // worktreePath/branch; a key-set-only check would leave the card showing stale path + // text. Whole-map JSON compare covers keys and values at negligible cost for small N. + JSON.stringify(previousTask.workspaceWorktrees ?? null) === + JSON.stringify(nextTask.workspaceWorktrees ?? null) && previousTask.branch === nextTask.branch && previousTask.baseBranch === nextTask.baseBranch && previousTask.breakIntoSubtasks === nextTask.breakIntoSubtasks && @@ -2186,6 +2198,10 @@ function TaskCardComponent({ ); })()} + {/* FNXC:Workspace 2026-06-21-00:00: workspace tasks have no singular task.branch, + so the branch-metadata row below renders nothing. Surface the acquired sub-repos + as a compact "N repos acquired" placeholder so the card isn't blank (U3/KTD5). */} + {isWorkspaceTask(task) && } {hasBranchMetadata && (
{branchMetadata.branch && ( diff --git a/packages/dashboard/app/components/TaskDetailModal.css b/packages/dashboard/app/components/TaskDetailModal.css index d851e339d6..1954edac2d 100644 --- a/packages/dashboard/app/components/TaskDetailModal.css +++ b/packages/dashboard/app/components/TaskDetailModal.css @@ -2327,3 +2327,39 @@ Narrow mobile task detail surfaces from both Board and List must allow horizonta color: var(--color-error); font-size: 0.75rem; } + +/* +FNXC:Workspace 2026-06-21-00:00: +Flat read-only per-sub-repo worktree list for a workspace task (U3/KTD5 dashboard floor). +Read-only list/placeholder only — not the deferred rich per-repo-status component. +*/ +.workspace-worktrees-summary { + margin: var(--space-sm) 0 0; +} +.workspace-worktrees-placeholder { + font-size: 0.75rem; + font-weight: 600; + color: var(--color-text-secondary, inherit); + margin-bottom: var(--space-xs); +} +.workspace-worktrees-list { + list-style: none; + margin: 0; + padding: 0; + display: flex; + flex-direction: column; + gap: var(--space-xs); +} +.workspace-worktrees-item { + display: flex; + flex-wrap: wrap; + gap: var(--space-xs) var(--space-sm); + font-size: 0.75rem; + font-family: var(--font-mono, monospace); +} +.workspace-worktrees-repo { + font-weight: 600; +} +.workspace-worktrees-branch { + color: var(--color-text-secondary, inherit); +} diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index 2aafea8b95..4b6c5b929e 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -40,6 +40,7 @@ import { TaskChatTab } from "./TaskChatTab"; import { TaskReviewTab } from "./TaskReviewTab"; import { MergeDetails } from "./MergeDetails"; import { TaskChangesTab } from "./TaskChangesTab"; +import { WorkspaceWorktreesSummary, isWorkspaceTask } from "./WorkspaceWorktreesSummary"; import { TaskForm, type PendingImage } from "./TaskForm"; import { useNodes } from "../hooks/useNodes"; import { WorkflowResultsTab } from "./WorkflowResultsTab"; @@ -3144,6 +3145,14 @@ export function TaskDetailContent({ {task.branchContext?.groupId && ( )} + {/* FNXC:Workspace 2026-06-21-00:00: workspace tasks have no singular + task.worktree/task.branch; surface their acquired per-sub-repo worktrees + as a flat read-only list so the detail view isn't blank (U3/KTD5). */} + {/* FNXC:Workspace 2026-06-22-09:00: gate/render off the hydrated + workingTask, not the sparse task row. workspaceWorktrees is only + present in fetched detail, so keying off task renders blank on the + optimistic-open path before the detail fetch resolves. */} + {isWorkspaceTask(workingTask) && } )} {task.status === "failed" && task.error && ( diff --git a/packages/dashboard/app/components/WorkspaceWorktreesSummary.tsx b/packages/dashboard/app/components/WorkspaceWorktreesSummary.tsx new file mode 100644 index 0000000000..90625a96eb --- /dev/null +++ b/packages/dashboard/app/components/WorkspaceWorktreesSummary.tsx @@ -0,0 +1,92 @@ +import { useTranslation } from "react-i18next"; +import type { Task } from "@fusion/core"; + +/* +FNXC:Workspace 2026-06-21-00:00: +Dashboard "doesn't look broken" floor (Phase A U3 / master U10, KTD5). +A workspace-mode task has NO singular `task.worktree`/`task.branch`; instead it carries +`task.workspaceWorktrees` — one acquired git worktree per sub-repo, keyed by repo path +relative to the workspace root. Existing display surfaces (TaskCard branch row, TaskDetail +metadata) key off the singular `task.branch`, so a workspace task would render an EMPTY +branch area — looking broken. This guard renders a static placeholder ("N repos acquired") +plus a flat read-only per-repo path/branch list so the task is observable, never crashing +and never blank. + +Scope ceiling: flat read-only list / placeholder ONLY. A rich per-repo-status component +(live diff/lease/merge state per repo) is the deferred registration UI — out of scope here. +Single-repo rendering is untouched: callers only mount this when `isWorkspaceTask(task)`. +*/ + +/** + * True when the task is a workspace-mode task: no singular `worktree` recorded + * and at least one acquired per-sub-repo worktree in `workspaceWorktrees`. + * Single-repo tasks (populated `worktree`, no `workspaceWorktrees`) return false, + * keeping their existing rendering byte-for-byte unchanged. + */ +export function isWorkspaceTask(task: Pick): boolean { + if (task.worktree) return false; + const entries = task.workspaceWorktrees; + return Boolean(entries && Object.keys(entries).length > 0); +} + +interface WorkspaceWorktreesSummaryProps { + task: Pick; + /** Compact variant for the dense TaskCard surface (placeholder only). */ + compact?: boolean; +} + +/** + * Read-only summary of a workspace task's acquired sub-repo worktrees. + * + * - `compact` (TaskCard): renders just the "N repos acquired" placeholder chip. + * - default (TaskDetail): renders the placeholder plus a flat per-repo list of + * `repo → worktreePath (branch)`. + * + * Renders nothing for non-workspace tasks; mount only behind `isWorkspaceTask`. + */ +export function WorkspaceWorktreesSummary({ task, compact = false }: WorkspaceWorktreesSummaryProps) { + const { t } = useTranslation("app"); + const entries = task.workspaceWorktrees; + if (!isWorkspaceTask(task) || !entries) return null; + + const repos = Object.entries(entries); + const placeholder = t("tasks.workspaceReposAcquired", "{{count}} repos acquired", { count: repos.length }); + + if (compact) { + return ( +
+ + {t("tasks.workspace", "Workspace")} + {placeholder} + +
+ ); + } + + return ( +
+
+ {placeholder} +
+
    + {repos.map(([repoRelPath, info]) => ( +
  • + + {repoRelPath} + + + {info.worktreePath} + + + {info.branch} + +
  • + ))} +
+
+ ); +} diff --git a/packages/dashboard/app/components/__tests__/WorkspaceWorktreesSummary.test.tsx b/packages/dashboard/app/components/__tests__/WorkspaceWorktreesSummary.test.tsx new file mode 100644 index 0000000000..dfd23f3875 --- /dev/null +++ b/packages/dashboard/app/components/__tests__/WorkspaceWorktreesSummary.test.tsx @@ -0,0 +1,89 @@ +import { describe, it, expect } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { WorkspaceWorktreesSummary, isWorkspaceTask } from "../WorkspaceWorktreesSummary"; + +/* +FNXC:Workspace 2026-06-21-00:00: +U3/KTD5 dashboard "doesn't look broken" floor. Asserts the invariant across both surfaces +the summary serves (FN-5893): +- happy path: workspace task (no task.worktree, two workspaceWorktrees entries) renders a + flat per-repo list + "N repos acquired" placeholder — no crash, not blank. +- regression: single-repo task (task.worktree set, no workspaceWorktrees) renders nothing + from this guard, so its existing rendering stays unchanged. +Narrow seam: tests the presentational component directly, no API / SSE / timers (FN-5048). +*/ + +const workspaceTask = { + worktree: undefined, + workspaceWorktrees: { + "repo-a": { worktreePath: "/wt/repo-a", branch: "fusion/fn-1-a" }, + "repo-b": { worktreePath: "/wt/repo-b", branch: "fusion/fn-1-b" }, + }, +} as const; + +const singleRepoTask = { + worktree: "/wt/single", + workspaceWorktrees: undefined, +} as const; + +describe("isWorkspaceTask", () => { + it("is true when worktree is absent and workspaceWorktrees has entries", () => { + expect(isWorkspaceTask(workspaceTask)).toBe(true); + }); + + it("is false for a single-repo task (worktree set)", () => { + expect(isWorkspaceTask(singleRepoTask)).toBe(false); + }); + + it("is false when workspaceWorktrees is an empty record", () => { + expect(isWorkspaceTask({ worktree: undefined, workspaceWorktrees: {} })).toBe(false); + }); + + it("prefers the singular worktree even if workspaceWorktrees is populated", () => { + expect( + isWorkspaceTask({ worktree: "/wt/x", workspaceWorktrees: workspaceTask.workspaceWorktrees }), + ).toBe(false); + }); +}); + +describe("WorkspaceWorktreesSummary", () => { + it("renders a flat per-repo list and placeholder for a two-repo workspace task (no crash, not empty)", () => { + render(); + + // Placeholder reflects the repo count. + expect(screen.getByTestId("workspace-worktrees-placeholder").textContent).toContain("2"); + expect(screen.getByText(/2 repos acquired/i)).toBeTruthy(); + + // Flat per-repo list: each repo path, worktree path, and branch is shown. + const summary = screen.getByTestId("workspace-worktrees-summary"); + expect(summary).toBeTruthy(); + expect(screen.getByText("repo-a")).toBeTruthy(); + expect(screen.getByText("repo-b")).toBeTruthy(); + expect(screen.getByText("/wt/repo-a")).toBeTruthy(); + expect(screen.getByText("/wt/repo-b")).toBeTruthy(); + expect(screen.getByText("fusion/fn-1-a")).toBeTruthy(); + expect(screen.getByText("fusion/fn-1-b")).toBeTruthy(); + }); + + it("renders only the compact placeholder in compact mode", () => { + render(); + expect(screen.getByTestId("workspace-worktrees-placeholder").textContent).toContain("2 repos"); + // Compact variant omits the full per-repo list. + expect(screen.queryByTestId("workspace-worktrees-summary")).toBeNull(); + expect(screen.queryByText("/wt/repo-a")).toBeNull(); + }); + + it("renders nothing for a single-repo task, leaving existing rendering unchanged", () => { + const { container } = render(); + expect(container.firstChild).toBeNull(); + expect(screen.queryByTestId("workspace-worktrees-summary")).toBeNull(); + expect(screen.queryByTestId("workspace-worktrees-placeholder")).toBeNull(); + }); + + it("renders nothing when workspaceWorktrees is empty", () => { + const { container } = render( + , + ); + expect(container.firstChild).toBeNull(); + }); +}); diff --git a/packages/engine/src/__tests__/_workspace-fixture.ts b/packages/engine/src/__tests__/_workspace-fixture.ts new file mode 100644 index 0000000000..5e94b78982 --- /dev/null +++ b/packages/engine/src/__tests__/_workspace-fixture.ts @@ -0,0 +1,66 @@ +/* +FNXC:Workspace 2026-06-21-12:00: +Shared REAL two-repo git fixture for workspace-mode engine tests (U1 + U2 + later phases). The foundation's executor-workspace test self-mocked the functions under test, which proves nothing; this harness instead builds genuine on-disk git repos under a NON-git workspace root so that any leaked rootDir git preflight actually fails. U2 and later units import `createWorkspaceFixture` directly — keep it dependency-light (only node:child_process + node:fs + saveWorkspaceConfig). + +A workspace root is a plain directory (NOT a git repo) containing N sub-repos. Each sub-repo is a real git repo with an initial commit on a default branch. `/.fusion/workspace.json` lists the sub-repo relative paths so `loadWorkspaceConfig(root)` returns a populated config — the exact signal `this.workspaceConfig` keys off in the executor. +*/ +import { execSync, spawnSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { saveWorkspaceConfig } from "@fusion/core"; + +export const hasGit = spawnSync("git", ["--version"], { stdio: "pipe" }).status === 0; + +function git(repo: string, command: string): string { + return execSync(command, { cwd: repo, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim(); +} + +/** Initialize a real git repo at `repoDir` with one commit on `defaultBranch`. */ +export function initRepoWithCommit(repoDir: string, defaultBranch = "main"): void { + mkdirSync(repoDir, { recursive: true }); + git(repoDir, `git init -b ${defaultBranch}`); + git(repoDir, 'git config user.email "test@example.com"'); + git(repoDir, 'git config user.name "Test"'); + writeFileSync(path.join(repoDir, "README.md"), `# ${path.basename(repoDir)}\n`, "utf-8"); + git(repoDir, "git add README.md"); + git(repoDir, "git commit -m 'init'"); +} + +export interface WorkspaceFixture { + /** Absolute path to the non-git workspace root. */ + rootDir: string; + /** Relative sub-repo paths (workspace.json `repos`). */ + repos: string[]; + /** Absolute path to a sub-repo by relative name. */ + repoPath(rel: string): string; + /** Run a git command inside a sub-repo. */ + git(rel: string, command: string): string; + /** Remove all on-disk fixture state. */ + cleanup(): void; +} + +/** + * Create a real two-repo (by default) workspace fixture on disk. + * - `rootDir` is a plain non-git directory. + * - Each `repos[i]` is a real git repo with an initial commit. + * - `/.fusion/workspace.json` is written so loadWorkspaceConfig() resolves. + */ +export async function createWorkspaceFixture( + repos: string[] = ["repo-a", "repo-b"], + defaultBranch = "main", +): Promise { + const rootDir = mkdtempSync(path.join(os.tmpdir(), "fusion-workspace-")); + for (const rel of repos) { + initRepoWithCommit(path.join(rootDir, rel), defaultBranch); + } + await saveWorkspaceConfig(rootDir, { repos }); + + return { + rootDir, + repos, + repoPath: (rel: string) => path.join(rootDir, rel), + git: (rel: string, command: string) => git(path.join(rootDir, rel), command), + cleanup: () => rmSync(rootDir, { recursive: true, force: true }), + }; +} diff --git a/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts b/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts index 351ee343bf..a8237d0755 100644 --- a/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts +++ b/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts @@ -88,7 +88,7 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { // executor must retry the agent session in place rather than bouncing the // task through todo (and must not fire a failure notification). const { store, task, executor } = makeHarness({ column: "todo" }); - (executor as any).activeWorktrees.set(task.id, task.worktree); + (executor as any).addActiveWorktree(task.id, task.worktree); const executeSpy = vi .spyOn(executor as any, "execute") .mockResolvedValue(undefined); @@ -137,7 +137,7 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { // and a retry scheduled); the task then changes state before the timer // fires, and the fire-time re-fetch must abort the dispatch. const { store, task, executor } = makeHarness({ column: "todo" }); - (executor as any).activeWorktrees.set(task.id, task.worktree); + (executor as any).addActiveWorktree(task.id, task.worktree); const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); await invokeGraphFailure(executor, task); @@ -164,7 +164,7 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { status: "failed", error: "Workflow graph failure surfaced after paused engine abort during pause/resume", }); - (executor as any).activeWorktrees.set(task.id, task.worktree); + (executor as any).addActiveWorktree(task.id, task.worktree); const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); await invokeGraphFailure(executor, task); @@ -197,7 +197,7 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { // pause that ended up in todo must stay parked-benign and wait for // explicit resume — auto-resuming it would override the operator's intent. const { store, task, executor } = makeHarness(overrides, provenance); - (executor as any).activeWorktrees.set(task.id, task.worktree); + (executor as any).addActiveWorktree(task.id, task.worktree); const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); await invokeGraphFailure(executor, task); @@ -217,7 +217,7 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { column: "todo", graphResumeRetryCount: 2, }); - (executor as any).activeWorktrees.set(task.id, task.worktree); + (executor as any).addActiveWorktree(task.id, task.worktree); const executeSpy = vi .spyOn(executor as any, "execute") .mockResolvedValue(undefined); @@ -246,7 +246,7 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { status: "failed", error: "Workflow graph failure surfaced after paused engine abort during pause/resume", }); - (executor as any).activeWorktrees.set(task.id, task.worktree); + (executor as any).addActiveWorktree(task.id, task.worktree); await invokeGraphFailure(executor, task); diff --git a/packages/engine/src/__tests__/executor-workspace-session-cwd.test.ts b/packages/engine/src/__tests__/executor-workspace-session-cwd.test.ts new file mode 100644 index 0000000000..19d101f8dc --- /dev/null +++ b/packages/engine/src/__tests__/executor-workspace-session-cwd.test.ts @@ -0,0 +1,120 @@ +/* +FNXC:Workspace 2026-06-21-12:00: +U1 session-cwd scenarios that require driving the real TaskExecutor.execute() to the agent-session boundary. Uses the shared executor-test-helpers harness — it mocks the AI/session/git/fs seams (NOT the workspace gating, NOT acquireTaskWorktree), so setting `(executor as any).workspaceConfig` exercises the genuine KTD1 gate: root acquisition is skipped, and every agent session (initial + retry) is created with `cwd === rootDir` (browse-only workspace root). The non-workspace path is the regression control (cwd === the acquired worktree path). +*/ +import { afterEach, beforeEach, describe, it, expect, vi } from "vitest"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { acquireTaskWorktree } from "../worktree-acquisition.js"; +import type { WorkspaceConfig } from "@fusion/core"; +import { + createMockStore, + mockedCreateFnAgent, + mockedExecSync, + resetExecutorMocks, +} from "./executor-test-helpers.js"; + +vi.mock("../worktree-acquisition.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, acquireTaskWorktree: vi.fn(actual.acquireTaskWorktree) }; +}); + +const mockedAcquireTaskWorktree = vi.mocked(acquireTaskWorktree); + +const ROOT = "/tmp/workspace-root"; + +function inProgressTask(overrides: Record = {}) { + return { + id: "FN-001", + title: "Test", + description: "Test", + column: "in-progress", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + } as any; +} + +describe("U1 KTD1 — session cwd is the browse-only workspace root", () => { + beforeEach(() => { + resetExecutorMocks(); + // Make any accidental git invocation observable: empty stdout keeps real-git + // helpers from throwing, but acquireTaskWorktree assertions catch a leak. + mockedExecSync.mockReturnValue(""); + }); + afterEach(() => vi.restoreAllMocks()); + + it("skips root acquireTaskWorktree and creates every session (initial + retry) with cwd === rootDir", async () => { + const store = createMockStore(); + const mockPrompt = vi.fn().mockResolvedValue(undefined); // no fn_task_done → drives retries too + mockedCreateFnAgent.mockResolvedValue({ + session: { prompt: mockPrompt, dispose: vi.fn() }, + sessionFile: "/tmp/sessions/ws.jsonl", + } as any); + + const executor = new TaskExecutor(store, ROOT); + // Drive the genuine workspace gate (loadWorkspaceConfig is covered elsewhere). + (executor as any).workspaceConfig = { repos: ["repo-a", "repo-b"] } as WorkspaceConfig; + + await executor.execute(inProgressTask({ worktree: null })); + + // KTD1: the non-git root is never acquired as a worktree. + expect(mockedAcquireTaskWorktree).not.toHaveBeenCalled(); + + // Every agent session (initial + the retries fired because fn_task_done was + // never called) is rooted at the workspace root. + expect(mockedCreateFnAgent.mock.calls.length).toBeGreaterThanOrEqual(2); + for (const call of mockedCreateFnAgent.mock.calls) { + expect((call[0] as any).cwd).toBe(ROOT); + } + + // task.worktree is never set in workspace mode. + const worktreeWrites = (store.updateTask as any).mock.calls.filter( + (c: any[]) => c[1] && Object.prototype.hasOwnProperty.call(c[1], "worktree") && c[1].worktree, + ); + expect(worktreeWrites).toHaveLength(0); + }); +}); + +describe("U1 regression — non-workspace task acquires a worktree and roots the session there", () => { + beforeEach(() => { + resetExecutorMocks(); + mockedExecSync.mockReturnValue(""); + }); + afterEach(() => vi.restoreAllMocks()); + + it("calls acquireTaskWorktree and creates the session with cwd === the acquired worktree path", async () => { + const store = createMockStore(); + const ACQUIRED = "/tmp/test/.worktrees/swift-falcon"; + mockedAcquireTaskWorktree.mockResolvedValue({ + worktreePath: ACQUIRED, + branch: "fusion/fn-001", + source: "fresh", + hydrated: false, + isResume: false, + }); + + const mockPrompt = vi.fn().mockResolvedValue(undefined); + mockedCreateFnAgent.mockResolvedValue({ + session: { prompt: mockPrompt, dispose: vi.fn() }, + sessionFile: "/tmp/sessions/ns.jsonl", + } as any); + + const executor = new TaskExecutor(store, "/tmp/test"); + // No workspaceConfig → single-repo path. Pin the lazy-load guard so the real + // loader is never consulted (it would return null for /tmp/test anyway). + (executor as any).workspaceConfig = null; + + await executor.execute(inProgressTask({ worktree: null })); + + expect(mockedAcquireTaskWorktree).toHaveBeenCalledTimes(1); + expect(mockedCreateFnAgent.mock.calls.length).toBeGreaterThanOrEqual(1); + for (const call of mockedCreateFnAgent.mock.calls) { + expect((call[0] as any).cwd).toBe(ACQUIRED); + } + }); +}); diff --git a/packages/engine/src/__tests__/executor-workspace.test.ts b/packages/engine/src/__tests__/executor-workspace.test.ts index 1916b52367..7b0033bb54 100644 --- a/packages/engine/src/__tests__/executor-workspace.test.ts +++ b/packages/engine/src/__tests__/executor-workspace.test.ts @@ -1,89 +1,221 @@ -// @ts-nocheck -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { loadWorkspaceConfig } from "@fusion/core"; -import { acquireWorkspaceRepoWorktree } from "../worktree-acquisition.js"; +/* +FNXC:Workspace 2026-06-21-12:00: +U1 executor session-scoping tests. REWRITTEN from the foundation's self-mocking version (which vi.mock'd the very functions under test and proved nothing). These tests use a REAL two-repo git fixture (`createWorkspaceFixture`) under a NON-git workspace root, so a leaked rootDir git preflight would actually fail. They drive the real TaskExecutor methods that U1 changed: the activeWorktrees Set conversion + every enumerated consumer (KTD2), the preflight gate + browse-only-root scoping (KTD1), and the synthetic-acquisition cwd. -vi.mock("@fusion/core", async (importOriginal) => { - const actual = await importOriginal(); +Seam choice (FN-5048): `(executor as any).workspaceConfig` is set directly to drive the gating with real git — loadWorkspaceConfig is covered by its own unit and is not the subject here. No mock-the-world child_process/fs shell. +*/ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { EventEmitter } from "node:events"; +import { loadWorkspaceConfig, type Task, type TaskStore, type WorkspaceConfig } from "@fusion/core"; +import { TaskExecutor, buildExecutionPrompt } from "../executor.js"; +import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js"; + +const describeIfGit = hasGit ? describe : describe.skip; + +function createStore(overrides: Partial> = {}): TaskStore & EventEmitter { + const emitter = new EventEmitter(); + return Object.assign(emitter, { + updateTask: vi.fn().mockResolvedValue(undefined), + logEntry: vi.fn().mockResolvedValue(undefined), + getSettings: vi.fn().mockResolvedValue({ autoMerge: false }), + on: emitter.on.bind(emitter), + ...overrides, + }) as unknown as TaskStore & EventEmitter; +} + +function makeTask(id = "FN-WS-1", overrides: Partial = {}): Task { return { - ...actual, - loadWorkspaceConfig: vi.fn(), - }; -}); + id, + title: "Workspace task", + description: "", + column: "in-progress", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + } as Task; +} -vi.mock("../worktree-acquisition.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - acquireWorkspaceRepoWorktree: vi.fn(), - }; -}); +const repoAPath = (fx: WorkspaceFixture) => `${fx.repoPath("repo-a")}/.worktrees/fn-ws-1`; +const repoBPath = (fx: WorkspaceFixture) => `${fx.repoPath("repo-b")}/.worktrees/fn-ws-1`; -const mockedLoadWorkspaceConfig = vi.mocked(loadWorkspaceConfig); -const mockedAcquireWorkspaceRepoWorktree = vi.mocked(acquireWorkspaceRepoWorktree); +describeIfGit("workspace fixture", () => { + let fx: WorkspaceFixture; + afterEach(() => fx?.cleanup()); -const MOCK_WORKSPACE_CONFIG = { - repos: ["wolf-server", "wolf-community-frontend-1"], -}; - -describe("acquireWorkspaceRepoWorktree", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("returns alreadyAcquired=false for a fresh repo", async () => { - mockedAcquireWorkspaceRepoWorktree.mockResolvedValueOnce({ - worktreePath: "/workspace/wolf-server/.worktrees/fn-001-abc", - branch: "fusion/fn-001", - alreadyAcquired: false, - }); - - const result = await acquireWorkspaceRepoWorktree({ - repoRelPath: "wolf-server", - workspaceRootDir: "/workspace", - task: { id: "FN-001", workspaceWorktrees: undefined } as never, - store: { getTask: vi.fn(), updateTask: vi.fn(), logEntry: vi.fn() } as never, - settings: {}, - }); - - expect(result.alreadyAcquired).toBe(false); - expect(result.worktreePath).toContain("wolf-server"); - }); - - it("returns alreadyAcquired=true when worktree already acquired", async () => { - mockedAcquireWorkspaceRepoWorktree.mockResolvedValueOnce({ - worktreePath: "/workspace/wolf-server/.worktrees/fn-001-abc", - branch: "fusion/fn-001", - alreadyAcquired: true, - }); - - const result = await acquireWorkspaceRepoWorktree({ - repoRelPath: "wolf-server", - workspaceRootDir: "/workspace", - task: { - id: "FN-001", - workspaceWorktrees: { - "wolf-server": { worktreePath: "/workspace/wolf-server/.worktrees/fn-001-abc", branch: "fusion/fn-001" }, - }, - } as never, - store: { getTask: vi.fn(), updateTask: vi.fn(), logEntry: vi.fn() } as never, - settings: {}, - }); - - expect(result.alreadyAcquired).toBe(true); + it("builds a non-git root with two real git sub-repos and a resolvable workspace config", async () => { + fx = await createWorkspaceFixture(); + // Root is NOT a git repo. Use "." so the check runs in fx.rootDir itself, not + // its parent (".." would resolve to the tmpdir and could pass spuriously). + expect(() => fx.git(".", "git rev-parse --git-dir")).toThrow(); + // Each sub-repo is a real git repo with a commit on main. + expect(fx.git("repo-a", "git rev-parse --abbrev-ref HEAD")).toBe("main"); + expect(fx.git("repo-b", "git rev-list --count HEAD")).toBe("1"); + // loadWorkspaceConfig resolves the on-disk config the executor keys off. + const config = await loadWorkspaceConfig(fx.rootDir); + expect(config?.repos).toEqual(["repo-a", "repo-b"]); }); }); -describe("workspace config", () => { - it("loadWorkspaceConfig returns null for non-workspace", async () => { - mockedLoadWorkspaceConfig.mockResolvedValueOnce(null); - const config = await loadWorkspaceConfig("/some/single-repo"); - expect(config).toBeNull(); +describeIfGit("U1 KTD2 — activeWorktrees Set + every enumerated consumer", () => { + let fx: WorkspaceFixture; + afterEach(() => fx?.cleanup()); + + function workspaceExecutor() { + fx ??= undefined as never; + const store = createStore(); + const executor = new TaskExecutor(store, fx.rootDir); + (executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig; + return executor; + } + + it("a workspace task holding TWO sub-repo paths is found by membership, not equality", async () => { + fx = await createWorkspaceFixture(); + const executor = workspaceExecutor(); + const pA = repoAPath(fx); + const pB = repoBPath(fx); + (executor as any).addActiveWorktree("FN-WS-1", pA); + (executor as any).addActiveWorktree("FN-WS-1", pB); + + // hasActiveWorktreeBinding: both held paths match; an unheld path does not. + expect((executor as any).hasActiveWorktreeBinding("FN-WS-1", pA)).toBe(true); + expect((executor as any).hasActiveWorktreeBinding("FN-WS-1", pB)).toBe(true); + expect((executor as any).hasActiveWorktreeBinding("FN-WS-1", "/nope")).toBe(false); + + // findActiveWorktreeOwner: another task asking about either held path finds FN-WS-1. + await expect((executor as any).findActiveWorktreeOwner(pA, "FN-OTHER")).resolves.toBe("FN-WS-1"); + await expect((executor as any).findActiveWorktreeOwner(pB, "FN-OTHER")).resolves.toBe("FN-WS-1"); + // The owner itself is excluded. + await expect((executor as any).findActiveWorktreeOwner(pA, "FN-WS-1")).resolves.toBeNull(); }); - it("loadWorkspaceConfig returns config for workspace", async () => { - mockedLoadWorkspaceConfig.mockResolvedValueOnce(MOCK_WORKSPACE_CONFIG); - const config = await loadWorkspaceConfig("/some/workspace"); - expect(config?.repos).toEqual(["wolf-server", "wolf-community-frontend-1"]); + it("listWorktreeHolders flat-maps the Set into N holder rows for one task", async () => { + fx = await createWorkspaceFixture(); + const executor = workspaceExecutor(); + const pA = repoAPath(fx); + const pB = repoBPath(fx); + (executor as any).addActiveWorktree("FN-WS-1", pA); + (executor as any).addActiveWorktree("FN-WS-1", pB); + + const holders = executor.listWorktreeHolders(); + expect(holders).toHaveLength(2); + expect(holders).toContainEqual({ taskId: "FN-WS-1", worktreePath: pA }); + expect(holders).toContainEqual({ taskId: "FN-WS-1", worktreePath: pB }); + }); + + it("shouldGenerateNewWorktreeName iterates the Set (conflict membership)", async () => { + fx = await createWorkspaceFixture(); + const store = createStore({ listTasks: vi.fn().mockResolvedValue([]) }); + const executor = new TaskExecutor(store, fx.rootDir); + (executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig; + const pA = repoAPath(fx); + (executor as any).addActiveWorktree("FN-HOLDER", pA); + + // A different task contending for FN-HOLDER's path must be told to generate a new name. + await expect((executor as any).shouldGenerateNewWorktreeName(pA, "FN-WS-1")).resolves.toBe(true); + // The holder asking about its own path is not a conflict (excluded), and the + // DB liveness fallback returns no other user. + await expect((executor as any).shouldGenerateNewWorktreeName(pA, "FN-HOLDER")).resolves.toBe(false); + }); + + it("getWorktreePath returns undefined for a multi-worktree workspace task (Set-collapse contract)", async () => { + fx = await createWorkspaceFixture(); + const executor = workspaceExecutor(); + (executor as any).addActiveWorktree("FN-WS-1", repoAPath(fx)); + (executor as any).addActiveWorktree("FN-WS-1", repoBPath(fx)); + expect(executor.getWorktreePath("FN-WS-1")).toBeUndefined(); + }); + + it("cleanup drops in-memory tracking in workspace mode but never removes the root", async () => { + fx = await createWorkspaceFixture(); + const removeSpy = vi.fn(); + const executor = workspaceExecutor(); + (executor as any).removeOwnWorktreeWithReconcile = removeSpy; + (executor as any).addActiveWorktree("FN-WS-1", repoAPath(fx)); + (executor as any).addActiveWorktree("FN-WS-1", repoBPath(fx)); + + await executor.cleanup("FN-WS-1"); + + expect(executor.getWorktreePath("FN-WS-1")).toBeUndefined(); + expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(false); + // The browse-only root must never be torn down as if it were a worktree. + expect(removeSpy).not.toHaveBeenCalled(); + }); + + it("clearPhantomExecutorBinding (FN-6736) unregisters every held path, not one", async () => { + fx = await createWorkspaceFixture(); + const executor = workspaceExecutor(); + const pA = repoAPath(fx); + const pB = repoBPath(fx); + (executor as any).addActiveWorktree("FN-WS-1", pA); + (executor as any).addActiveWorktree("FN-WS-1", pB); + + const ok = (executor as any).clearPhantomExecutorBinding("FN-WS-1"); + expect(ok).toBe(true); + expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(false); + }); +}); + +describeIfGit("U1 KTD2 — non-workspace task is a one-element Set (regression: unchanged)", () => { + let fx: WorkspaceFixture; + afterEach(() => fx?.cleanup()); + + it("getWorktreePath returns the sole path; listWorktreeHolders emits exactly one row", async () => { + fx = await createWorkspaceFixture(); + const store = createStore(); + const executor = new TaskExecutor(store, fx.repoPath("repo-a")); // single-repo root + // No workspaceConfig set → single-repo mode. + const wt = `${fx.repoPath("repo-a")}/.worktrees/fn-001`; + (executor as any).addActiveWorktree("FN-001", wt); + + expect(executor.getWorktreePath("FN-001")).toBe(wt); + expect(executor.listWorktreeHolders()).toEqual([{ taskId: "FN-001", worktreePath: wt }]); + expect((executor as any).hasActiveWorktreeBinding("FN-001", wt)).toBe(true); + }); +}); + +describeIfGit("U1 KTD1 — verifyWorktreeInvariants gated off in workspace mode", () => { + let fx: WorkspaceFixture; + afterEach(() => fx?.cleanup()); + + it("returns ok for a zero-acquire workspace task (no task.worktree) so fn_task_done does not requeue", async () => { + fx = await createWorkspaceFixture(); + const store = createStore(); + const executor = new TaskExecutor(store, fx.rootDir); + (executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig; + + // A workspace task that acquired ZERO sub-repos has no task.worktree and no + // tracked paths. The singular invariant would otherwise refuse on + // "missing task.worktree"; in workspace mode it is gated OFF. + const result = await (executor as any).verifyWorktreeInvariants(makeTask("FN-WS-1", { worktree: undefined })); + expect(result).toEqual({ ok: true }); + }); + + it("non-workspace task with no worktree still fails the invariant (regression: gate is workspace-only)", async () => { + fx = await createWorkspaceFixture(); + const store = createStore(); + const executor = new TaskExecutor(store, fx.repoPath("repo-a")); + // No workspaceConfig. + const result = await (executor as any).verifyWorktreeInvariants(makeTask("FN-001", { worktree: undefined })); + expect(result.ok).toBe(false); + }); +}); + +describeIfGit("U1 KTD1 — scopePromptToWorktree / buildExecutionPrompt no-op in workspace mode", () => { + let fx: WorkspaceFixture; + afterEach(() => fx?.cleanup()); + + it("does not rewrite root-anchored paths when a workspace config is present", async () => { + fx = await createWorkspaceFixture(); + const task = makeTask("FN-WS-1", { prompt: `Edit ${fx.rootDir}/repo-a/src/index.ts and commit.` }); + const config: WorkspaceConfig = { repos: fx.repos }; + // worktreePath === rootDir in workspace mode; the prompt must be returned verbatim. + const prompt = buildExecutionPrompt(task as any, fx.rootDir, { autoMerge: false } as any, fx.rootDir, undefined, undefined, config); + expect(prompt).toContain(`${fx.rootDir}/repo-a/src/index.ts`); + // The workspace repo list is appended (foundation behavior). + expect(prompt).toContain("repo-a"); }); }); diff --git a/packages/engine/src/__tests__/executor-worktree-conflict.test.ts b/packages/engine/src/__tests__/executor-worktree-conflict.test.ts index c2714fda0b..adbc691c86 100644 --- a/packages/engine/src/__tests__/executor-worktree-conflict.test.ts +++ b/packages/engine/src/__tests__/executor-worktree-conflict.test.ts @@ -39,7 +39,7 @@ describe("FN-4973: executor worktree conflict cleanup", () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test"); store.listTasks.mockResolvedValue([]); - (executor as any).activeWorktrees.set("FN-4973", CONFLICT_PATH); + (executor as any).addActiveWorktree("FN-4973", CONFLICT_PATH); activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: "FN-4973", kind: "executor", ownerKey: "FN-4973" }); vi.spyOn(worktreePoolModule, "removeWorktree").mockRejectedValue( diff --git a/packages/engine/src/__tests__/reliability-interactions/active-worktree-removal-liveness.test.ts b/packages/engine/src/__tests__/reliability-interactions/active-worktree-removal-liveness.test.ts index 4889abc25d..d35c55ff7f 100644 --- a/packages/engine/src/__tests__/reliability-interactions/active-worktree-removal-liveness.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/active-worktree-removal-liveness.test.ts @@ -58,7 +58,7 @@ describe("FN-4811: active worktree removal liveness gate", () => { it("returns the owner taskId when activeWorktrees has another task using the path", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test"); - (executor as any).activeWorktrees.set("FN-OTHER", ACTIVE_PATH); + (executor as any).addActiveWorktree("FN-OTHER", ACTIVE_PATH); const owner = await (executor as any).findActiveWorktreeOwner(ACTIVE_PATH, "FN-4811"); expect(owner).toBe("FN-OTHER"); @@ -67,7 +67,7 @@ describe("FN-4811: active worktree removal liveness gate", () => { it("returns null when activeWorktrees only has the requesting task at the path", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test"); - (executor as any).activeWorktrees.set("FN-4811", ACTIVE_PATH); + (executor as any).addActiveWorktree("FN-4811", ACTIVE_PATH); store.listTasks.mockResolvedValue([]); const owner = await (executor as any).findActiveWorktreeOwner(ACTIVE_PATH, "FN-4811"); @@ -125,7 +125,7 @@ describe("FN-4811: active worktree removal liveness gate", () => { it("refuses removal when worktree is in activeWorktrees for another task", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test"); - (executor as any).activeWorktrees.set("FN-OTHER", ACTIVE_PATH); + (executor as any).addActiveWorktree("FN-OTHER", ACTIVE_PATH); store.listTasks.mockResolvedValue([]); const result = await (executor as any).cleanupConflictingWorktree( @@ -226,7 +226,7 @@ describe("FN-4811: active worktree removal liveness gate", () => { it("returns 'sticky' without invoking inspection when conflict path is actively owned", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test"); - (executor as any).activeWorktrees.set("FN-OWNER", ACTIVE_PATH); + (executor as any).addActiveWorktree("FN-OWNER", ACTIVE_PATH); store.listTasks.mockResolvedValue([]); const inspectSpy = vi.spyOn(branchConflictModule, "inspectBranchConflict"); diff --git a/packages/engine/src/__tests__/reliability-interactions/post-completion-stale-self-owned-binding.test.ts b/packages/engine/src/__tests__/reliability-interactions/post-completion-stale-self-owned-binding.test.ts index abe29b5b53..3717a9e355 100644 --- a/packages/engine/src/__tests__/reliability-interactions/post-completion-stale-self-owned-binding.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/post-completion-stale-self-owned-binding.test.ts @@ -23,7 +23,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin it("reconciles stale same-task registry entry during cleanup()", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, ROOT); - (executor as any).activeWorktrees.set(TASK_ID, PATH); + (executor as any).addActiveWorktree(TASK_ID, PATH); activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID }); (activeSessionRegistry.lookupByPath(PATH) as any).registeredAt = 0; const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined); @@ -56,7 +56,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin it("preserves refusal for truly-live same-task bindings", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, ROOT); - (executor as any).activeWorktrees.set(TASK_ID, PATH); + (executor as any).addActiveWorktree(TASK_ID, PATH); activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID }); vi.spyOn(worktreePoolModule, "removeWorktree").mockRejectedValue( new ActiveSessionWorktreeRemovalError({ @@ -82,7 +82,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin const store = createMockStore(); store.listTasks.mockResolvedValue([]); const executor = new TaskExecutor(store, ROOT); - (executor as any).activeWorktrees.set("FN-FOREIGN", PATH); + (executor as any).addActiveWorktree("FN-FOREIGN", PATH); activeSessionRegistry.registerPath(PATH, { taskId: "FN-FOREIGN", kind: "executor", ownerKey: "FN-FOREIGN" }); const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined); @@ -96,13 +96,13 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin it("is idempotent across repeated cleanup sweeps", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, ROOT); - (executor as any).activeWorktrees.set(TASK_ID, PATH); + (executor as any).addActiveWorktree(TASK_ID, PATH); activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID }); (activeSessionRegistry.lookupByPath(PATH) as any).registeredAt = 0; const removeSpy = vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined); await executor.cleanup(TASK_ID); - (executor as any).activeWorktrees.set(TASK_ID, PATH); + (executor as any).addActiveWorktree(TASK_ID, PATH); await executor.cleanup(TASK_ID); const clearedCalls = (store.logEntry as any).mock.calls.filter( @@ -120,7 +120,7 @@ describe("FN-5346 reliability interactions: post-completion stale self-owned bin const store = createMockStore(); const executor = new TaskExecutor(store, ROOT); - (executor as any).activeWorktrees.set(TASK_ID, PATH); + (executor as any).addActiveWorktree(TASK_ID, PATH); activeSessionRegistry.registerPath(PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID }); vi.spyOn(worktreePoolModule, "removeWorktree").mockResolvedValue(undefined); diff --git a/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-active-session-recovery.test.ts b/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-active-session-recovery.test.ts index f555c81d5f..9ee54c545f 100644 --- a/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-active-session-recovery.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-active-session-recovery.test.ts @@ -118,7 +118,7 @@ describe("FN-4973 reliability interactions: stale self-owned active-session reco const store = createMockStore(); store.listTasks.mockResolvedValue([]); const executor = new TaskExecutor(store, "/tmp/test"); - (executor as any).activeWorktrees.set(TASK_ID, CONFLICT_PATH); + (executor as any).addActiveWorktree(TASK_ID, CONFLICT_PATH); activeSessionRegistry.registerPath(CONFLICT_PATH, { taskId: TASK_ID, kind: "executor", ownerKey: TASK_ID }); vi.spyOn(worktreePoolModule, "removeWorktree").mockRejectedValue( diff --git a/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-session-registry.test.ts b/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-session-registry.test.ts index 91e77b0ac4..869d6303bf 100644 --- a/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-session-registry.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/stale-self-owned-session-registry.test.ts @@ -45,7 +45,7 @@ describe("FN-4976: stale self-owned activeSessionRegistry deadlock backstop", () it("FN-4976 does not clear foreign-owned activeSessionRegistry entry and FN-4811 refusal still fires", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, ROOT); - (executor as any).activeWorktrees.set("FN-OTHER", PATH); + (executor as any).addActiveWorktree("FN-OTHER", PATH); store.listTasks.mockResolvedValue([]); activeSessionRegistry.registerPath(PATH, { taskId: "FN-OTHER", kind: "executor", ownerKey: "FN-OTHER" }); diff --git a/packages/engine/src/__tests__/worktree-acquisition-workspace.test.ts b/packages/engine/src/__tests__/worktree-acquisition-workspace.test.ts new file mode 100644 index 0000000000..3267157db7 --- /dev/null +++ b/packages/engine/src/__tests__/worktree-acquisition-workspace.test.ts @@ -0,0 +1,347 @@ +/* +FNXC:Workspace 2026-06-21-20:10: +U2 per-repo acquisition hardening tests. A REAL two-repo git fixture is required +because the invariants under test are git-shaped: local-ahead-of-origin base +capture, a resolved-per-repo (non-shared) integration branch, and a working +identity-guard hook that actually rejects a commit. The shared harness from +./_workspace-fixture.ts builds genuine on-disk repos under a NON-git workspace +root. The TaskStore is an in-memory fake (no DB / no network) per FN-5048 — real +git only where the invariant needs it; everything else is a narrow seam. +*/ +import { execSync, spawnSync } from "node:child_process"; +import { existsSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import type { Settings, Task, TaskStore } from "@fusion/core"; +import { + acquireWorkspaceRepoWorktree, + WorkspaceRepoAcquireBusyError, +} from "../worktree-acquisition.js"; +import { ActiveSessionRegistry } from "../active-session-registry.js"; +import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js"; + +const describeIfGit = hasGit ? describe : describe.skip; + +function git(repo: string, command: string): string { + return execSync(command, { cwd: repo, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim(); +} + +/** + * Minimal in-memory TaskStore covering exactly what acquireWorkspaceRepoWorktree + * and its acquireTaskWorktree callee touch: updateTask (merge-in-place so the + * idempotency re-read sees persisted workspaceWorktrees), logEntry, getTask. + */ +function makeFakeStore(task: Task): { store: TaskStore; current: () => Task; logs: string[] } { + let current = task; + const logs: string[] = []; + const store = { + async updateTask(id: string, patch: Partial): Promise { + if (id === current.id) current = { ...current, ...patch }; + }, + async logEntry(_id: string, message: string): Promise { + logs.push(message); + }, + async getTask(id: string): Promise { + return id === current.id ? current : null; + }, + } as unknown as TaskStore; + return { store, current: () => current, logs }; +} + +function makeTask(id: string): Task { + return { + id, + title: `task ${id}`, + description: "workspace task", + status: "in-progress", + } as unknown as Task; +} + +const SETTINGS: Partial = { + worktreeNaming: "task-id", + commitMsgHookEnabled: true, + taskPrefix: "FN", + taskAttributionTrailerNames: ["Fusion-Task-Id"], +}; + +describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout: 60_000 }, () => { + let fixture: WorkspaceFixture; + + afterEach(() => { + fixture?.cleanup(); + }); + + it("captures the LOCAL integration tip as baseCommitSha even when origin is behind (inflation invariant)", async () => { + // Give repo-a a real origin so origin/main can lag behind local main. + fixture = await createWorkspaceFixture(["repo-a"]); + const repoA = fixture.repoPath("repo-a"); + const origin = `${repoA}-origin`; + git(repoA, "git init --bare " + JSON.stringify(origin)); + git(repoA, `git remote add origin ${JSON.stringify(origin)}`); + git(repoA, "git push -u origin main"); + + // Local main advances by an unpushed predecessor commit (FN-5937 shape). + git(repoA, "git commit --allow-empty -m 'FN-9000: unpushed predecessor'"); + const localTip = git(repoA, "git rev-parse HEAD"); + const originTip = git(repoA, "git rev-parse origin/main"); + expect(localTip).not.toBe(originTip); + + const { store, current } = makeFakeStore(makeTask("FN-1")); + const registry = new ActiveSessionRegistry(); + const result = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }); + + // Base must be the LOCAL tip, never the behind origin tip. + expect(result.baseCommitSha).toBe(localTip); + expect(current().workspaceWorktrees?.["repo-a"]?.baseCommitSha).toBe(localTip); + }); + + it("captures against a NON-main integration branch and does not inherit a shared settings.integrationBranch (KTD3)", async () => { + // repo-a's default branch is 'develop'; origin/HEAD points at it. A shared + // settings.integrationBranch override must be STRIPPED so per-repo resolution + // falls through to this repo's own origin/HEAD. + fixture = await createWorkspaceFixture(["repo-a"], "develop"); + const repoA = fixture.repoPath("repo-a"); + const origin = `${repoA}-origin`; + git(repoA, "git init --bare " + JSON.stringify(origin)); + git(repoA, `git remote add origin ${JSON.stringify(origin)}`); + git(repoA, "git push -u origin develop"); + // Point origin/HEAD at develop so resolveIntegrationBranch resolves it. + git(repoA, "git remote set-head origin develop"); + const developTip = git(repoA, "git rev-parse develop"); + + const { store, current } = makeFakeStore(makeTask("FN-2")); + const registry = new ActiveSessionRegistry(); + const result = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + // A SHARED integration branch that does NOT exist in this sub-repo. If it + // leaked through, base capture would resolve against 'shared-trunk' and + // (absent that branch) fall back to HEAD — not develop's tip. + settings: { ...SETTINGS, integrationBranch: "shared-trunk" }, + registry, + }); + + expect(result.baseCommitSha).toBe(developTip); + }); + + it("installs the identity-guard hook so a commit on a non-fusion branch is rejected", async () => { + fixture = await createWorkspaceFixture(["repo-a"]); + const { store, current } = makeFakeStore(makeTask("FN-3")); + const registry = new ActiveSessionRegistry(); + const result = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + settings: SETTINGS, + store, + registry, + }); + + const wt = result.worktreePath; + expect(existsSync(join(wt, ".git"))).toBe(true); + git(wt, 'git config user.email "test@example.com"'); + git(wt, 'git config user.name "Test"'); + + // On the fusion/ branch the guard permits a commit (real staged change, + // so the FN-5345 empty-commit guard also installed by the identity guard + // does not refuse it). + git(wt, "git checkout fusion/fn-3"); + writeFileSync(join(wt, "own.txt"), "own work\n", "utf-8"); + git(wt, "git add own.txt"); + git(wt, "git commit -m 'FN-3: ok on own branch'"); + + // Switch to a foreign branch; the pre-commit identity guard must refuse. + git(wt, "git checkout -B rogue-branch"); + writeFileSync(join(wt, "rogue.txt"), "rogue work\n", "utf-8"); + git(wt, "git add rogue.txt"); + const attempt = spawnSync("git", ["commit", "-m", "rogue"], { + cwd: wt, + encoding: "utf-8", + }); + expect(attempt.status).not.toBe(0); + expect(`${attempt.stderr}`).toMatch(/refusing commit/i); + }); + + it("serializes two concurrent acquisitions of the SAME sub-repo via the exclusivity registry (KTD4)", async () => { + fixture = await createWorkspaceFixture(["repo-a"]); + const repoAbs = fixture.repoPath("repo-a"); + const registry = new ActiveSessionRegistry(); + + // Pre-register the sub-repo path as if task FN-A is mid-acquisition, then + // prove a second task is rejected while it is held. + registry.registerPath(repoAbs, { taskId: "FN-A", kind: "workspace-repo-acquire", ownerKey: "workspace-repo-acquire" }); + + const { store, current } = makeFakeStore(makeTask("FN-B")); + await expect( + acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }), + ).rejects.toBeInstanceOf(WorkspaceRepoAcquireBusyError); + + // The holder's entry is untouched by the rejected loser. + expect(registry.lookupByPath(repoAbs)?.taskId).toBe("FN-A"); + + // Once released, the same task acquires cleanly and the registry is freed. + registry.unregisterPath(repoAbs); + const result = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }); + expect(result.alreadyAcquired).toBe(false); + // Acquisition releases its own exclusivity entry on completion. + expect(registry.isPathActive(repoAbs)).toBe(false); + }); + + it("is idempotent across (taskId, repo): re-acquire returns the existing entry without re-capture", async () => { + fixture = await createWorkspaceFixture(["repo-a"]); + const { store, current } = makeFakeStore(makeTask("FN-4")); + const registry = new ActiveSessionRegistry(); + + const first = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }); + expect(first.alreadyAcquired).toBe(false); + + // Re-acquire with the now-populated task: returns the persisted entry, + // does not re-register exclusivity, does not re-create a worktree. + const second = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }); + expect(second.alreadyAcquired).toBe(true); + expect(second.worktreePath).toBe(first.worktreePath); + expect(second.baseCommitSha).toBe(first.baseCommitSha); + expect(registry.isPathActive(fixture.repoPath("repo-a"))).toBe(false); + }); + + it("surfaces an error and persists an audit event when acquisition fails (no swallowed stall)", async () => { + fixture = await createWorkspaceFixture(["repo-a"]); + const { store, current, logs } = makeFakeStore(makeTask("FN-5")); + const registry = new ActiveSessionRegistry(); + const auditEvents: Array<{ type: string }> = []; + const audit = { + async git(e: { type: string }): Promise { + auditEvents.push(e); + }, + async filesystem(): Promise {}, + }; + + await expect( + acquireWorkspaceRepoWorktree({ + repoRelPath: "does-not-exist", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + audit: audit as never, + }), + ).rejects.toThrow(); + + expect(auditEvents.some((e) => e.type === "worktree:workspace-repo-acquire-failed")).toBe(true); + expect(logs.some((m) => /acquisition failed/i.test(m))).toBe(true); + // The exclusivity entry is released even on the failure path. + expect(registry.isPathActive(join(fixture.rootDir, "does-not-exist"))).toBe(false); + }); + + /* + FNXC:Workspace 2026-06-21-22:30: + F4 — resolveFromSettings falls back integrationBranch → settings.baseBranch → + origin/HEAD. A shared settings.baseBranch must be STRIPPED alongside + integrationBranch, otherwise a baseBranch absent from this sub-repo leaks through + and the per-repo base resolves against the wrong branch. Here repo-a's only branch + is its own origin/HEAD (develop); a shared baseBranch of 'shared-trunk' (absent in + the sub-repo) must NOT be honored — the base must resolve to develop's tip. + */ + it("strips a shared settings.baseBranch so the base resolves against the sub-repo's own origin/HEAD (KTD3 / F4)", async () => { + fixture = await createWorkspaceFixture(["repo-a"], "develop"); + const repoA = fixture.repoPath("repo-a"); + const origin = `${repoA}-origin`; + git(repoA, "git init --bare " + JSON.stringify(origin)); + git(repoA, `git remote add origin ${JSON.stringify(origin)}`); + git(repoA, "git push -u origin develop"); + git(repoA, "git remote set-head origin develop"); + const developTip = git(repoA, "git rev-parse develop"); + + const { store, current } = makeFakeStore(makeTask("FN-6")); + const registry = new ActiveSessionRegistry(); + const result = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + // A shared baseBranch (no integrationBranch) that does NOT exist in this + // sub-repo. If it leaked through, base capture would resolve against + // 'shared-trunk' instead of develop. + settings: { ...SETTINGS, baseBranch: "shared-trunk" } as Partial, + registry, + }); + + expect(result.baseCommitSha).toBe(developTip); + }); + + /* + FNXC:Workspace 2026-06-21-22:30: + F5 — two sequential acquires for DIFFERENT sub-repos in one task must each persist + their own workspaceWorktrees entry. The acquisition re-reads the task fresh before + the merge so the second acquire does not clobber the first repo's entry. + */ + it("preserves a sibling sub-repo's workspaceWorktrees entry across two different-repo acquires (F5)", async () => { + fixture = await createWorkspaceFixture(["repo-a", "repo-b"]); + const { store, current } = makeFakeStore(makeTask("FN-7")); + const registry = new ActiveSessionRegistry(); + + const first = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-a", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }); + expect(first.alreadyAcquired).toBe(false); + + const second = await acquireWorkspaceRepoWorktree({ + repoRelPath: "repo-b", + workspaceRootDir: fixture.rootDir, + task: current(), + store, + settings: SETTINGS, + registry, + }); + expect(second.alreadyAcquired).toBe(false); + + // Both entries survive — the second acquire merged into the latest map, not the + // stale snapshot, so repo-a was not clobbered. + const persisted = current().workspaceWorktrees ?? {}; + expect(persisted["repo-a"]?.worktreePath).toBe(first.worktreePath); + expect(persisted["repo-b"]?.worktreePath).toBe(second.worktreePath); + }); +}); diff --git a/packages/engine/src/active-session-registry.ts b/packages/engine/src/active-session-registry.ts index ec28db0158..12168c0cea 100644 --- a/packages/engine/src/active-session-registry.ts +++ b/packages/engine/src/active-session-registry.ts @@ -1,4 +1,13 @@ -export type ActiveSessionKind = "executor" | "step-session" | "workflow-step" | "step-session-parallel" | "ai-merge"; +/* +FNXC:Workspace 2026-06-21-20:10: +"workspace-repo-acquire" is a DISTINCT registry kind reserved for the +acquisition-time same-sub-repo exclusivity entry (U2/KTD4). It is keyed by the +sub-repo absolute path (NOT the worktree path) so two concurrent workspace tasks +contending for the SAME sub-repo are serialized. Keeping it distinct from +"executor"/"step-session" means it does not collide with the executor's later +session registration on the produced worktree path. +*/ +export type ActiveSessionKind = "executor" | "step-session" | "workflow-step" | "step-session-parallel" | "ai-merge" | "workspace-repo-acquire"; export interface ActiveSessionRegistration { taskId: string; diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts index 6f9f4d8fea..87a6701afc 100644 --- a/packages/engine/src/agent-tools.ts +++ b/packages/engine/src/agent-tools.ts @@ -28,7 +28,7 @@ import { computeApprovalDedupeKey } from "./agent-action-gate.js"; import { MessageDeliveryAutoRecoveryHandler } from "./auto-recovery-handlers/message-delivery.js"; import { emitGoalRetrievalAudit } from "./goal-anchoring-audit.js"; import { recordRetry } from "./retry-burned-logger.js"; -import { acquireWorkspaceRepoWorktree } from "./worktree-acquisition.js"; +import { acquireWorkspaceRepoWorktree, WorkspaceRepoAcquireBusyError } from "./worktree-acquisition.js"; // ── Tool parameter schemas (canonical definitions) ──────────────────────── @@ -3888,11 +3888,21 @@ export function createAcquireRepoWorktreeTool(opts: { secretsStore?: Pick; runContext?: RunMutationContext; audit?: Pick; + /* + FNXC:Workspace 2026-06-21-22:30: + F2 — executor-supplied callback invoked after a SUCCESSFUL fresh acquire so the + acquired sub-repo worktree path is registered in the executor's per-task + activeWorktrees Set (KTD2). Without this the Set only ever held the browse-only + root and the "task holds N sub-repo paths" invariant was hollow — owner/liveness + checks never saw live sub-repo worktrees. Not called on the already-acquired + short-circuit (the path was registered on the original fresh acquire). + */ + onAcquired?: (worktreePath: string) => void; // FNXC:Workspace 2026-06-22 — thread the configured worktree-init runner so sub-repo worktrees run configured setup. runConfiguredCommand?: import("./worktree-acquisition.js").AcquireWorkspaceRepoWorktreeOptions["runConfiguredCommand"]; taskEnv?: NodeJS.ProcessEnv; }): ToolDefinition { - const { workspaceRootDir, workspaceRepos, task, store, settings, logger, secretsStore, runContext, audit, runConfiguredCommand, taskEnv } = opts; + const { workspaceRootDir, workspaceRepos, task, store, settings, logger, secretsStore, runContext, audit, onAcquired, runConfiguredCommand, taskEnv } = opts; return { name: "fn_acquire_repo_worktree", label: "Acquire Repo Worktree", @@ -3911,19 +3921,53 @@ export function createAcquireRepoWorktreeTool(opts: { }; } const freshTask = await store.getTask(task.id); - const result = await acquireWorkspaceRepoWorktree({ - repoRelPath: repo, - workspaceRootDir, - task: freshTask, - store, - settings, - logger, - secretsStore, - runContext, - audit, - runConfiguredCommand, - taskEnv, - }); + /* + FNXC:Workspace 2026-06-21-22:30: + F1 — acquireWorkspaceRepoWorktree can throw WorkspaceRepoAcquireBusyError on + same-sub-repo contention (KTD4) or a generic failure. Both must surface as a + structured isError tool result, never an uncaught throw that crashes the agent + loop. The busy message is sanitized — it does NOT leak the holder task id into + agent-facing text (only into details). runContext is forwarded so the helper's + audit/log entries keep run attribution. + */ + let result: Awaited>; + try { + result = await acquireWorkspaceRepoWorktree({ + repoRelPath: repo, + workspaceRootDir, + task: freshTask, + store, + settings, + logger, + secretsStore, + audit, + runContext, + runConfiguredCommand, + taskEnv, + }); + } catch (err) { + if (err instanceof WorkspaceRepoAcquireBusyError) { + return { + content: [{ type: "text" as const, text: `Sub-repo ${repo} is temporarily locked by another task's acquisition; retry fn_acquire_repo_worktree shortly.` }], + details: { holderTaskId: err.holderTaskId }, + isError: true, + }; + } + const message = err instanceof Error ? err.message : String(err); + return { + content: [{ type: "text" as const, text: `ERROR: Failed to acquire worktree for ${repo}: ${message}` }], + details: {}, + isError: true, + }; + } + // FNXC:Workspace 2026-06-21-22:30: F2 — register a freshly-acquired sub-repo worktree in the executor's activeWorktrees Set (KTD2) so owner/liveness checks see live per-repo worktrees, not just the browse-only root. + // FNXC:Workspace 2026-06-22-09:00: register UNCONDITIONALLY, including the + // already-acquired short-circuit. After an executor restart activeWorktrees is an + // empty Map; a resumed workspace task with pre-existing task.workspaceWorktrees hits + // the alreadyAcquired path, so skipping onAcquired left the sub-repo path unregistered + // in-memory and conflict/liveness checks missed it. Set.add is idempotent, so re-firing + // on a fresh acquire is a harmless no-op. + onAcquired?.(result.worktreePath); await store.logEntry( task.id, result.alreadyAcquired diff --git a/packages/engine/src/base-commit-capture.ts b/packages/engine/src/base-commit-capture.ts index c449a97558..4862226f88 100644 --- a/packages/engine/src/base-commit-capture.ts +++ b/packages/engine/src/base-commit-capture.ts @@ -22,15 +22,40 @@ const execAsync = promisify(exec); * * Returns `undefined` only when every git invocation fails (caller treats a * missing base as non-fatal). + * + * FNXC:Workspace 2026-06-21-20:10: + * `integrationBranch` is an OPTIONAL TRAILING param defaulting to the historic + * "main" literal so the single-repo executor caller and the real-git tests stay + * green without change. Workspace mode (U2/KTD3) passes each sub-repo's RESOLVED + * integration branch so per-repo base capture forks against the right branch + * instead of a hardcoded "main". The local-first ordering (merge-base HEAD + * then origin/) is preserved per-branch to keep the + * inflation-prevention invariant (FN-5937) intact for non-main integration + * branches too. */ export async function resolveCapturedBaseCommitSha( worktreePath: string, logger?: { warn: (msg: string) => void }, + integrationBranch: string = "main", ): Promise { + const branch = integrationBranch.trim() || "main"; + /* + FNXC:Workspace 2026-06-22-09:00: + Shell-quote with a real single-quoted POSIX literal, NOT JSON.stringify. A + JSON double-quoted string still lets bash expand `$(...)`, backticks, and `$VAR` + inside it; JSON.stringify is not a shell-quoting function. Git ref names can't + legally contain backticks so there's no live injection path today, but + single-quoting is the idiomatic safe form and stays correct if a caller ever + passes a less-constrained string. A single quote inside the value is escaped as + the standard `'\''` close-reopen sequence. + */ + const shellSingleQuote = (value: string): string => `'${value.replace(/'/g, "'\\''")}'`; + const localRef = shellSingleQuote(branch); + const originRef = shellSingleQuote(`origin/${branch}`); let baseCommitSha: string | undefined; try { const { stdout } = await execAsync( - "git merge-base HEAD main 2>/dev/null || git merge-base HEAD origin/main", + `git merge-base HEAD ${localRef} 2>/dev/null || git merge-base HEAD ${originRef}`, { cwd: worktreePath, encoding: "utf-8" }, ); baseCommitSha = stdout.trim() || undefined; diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index a8b38404e6..795d31b7ee 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -144,7 +144,7 @@ import { isContextLimitError } from "./context-limit-detector.js"; import { StepSessionExecutor } from "./step-session-executor.js"; import { makeAncestryBlastRadiusGuard, resetStepToBaseline, runTaskStep } from "./step-runner.js"; // FNXC:MergerUnification 2026-06-21-19:05: the foundation branch imported `acquireWorkspaceRepoWorktree` here but never used it in executor.ts (the agent tool wraps it via agent-tools.ts), which fails lint on the inherited base. Removed until master-plan U1 re-adds it together with its per-repo acquisition usage. -import { acquireTaskWorktree } from "./worktree-acquisition.js"; +import { acquireTaskWorktree, type AcquireTaskWorktreeResult } from "./worktree-acquisition.js"; import { resolveCapturedBaseCommitSha } from "./base-commit-capture.js"; import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js"; import { @@ -1478,7 +1478,28 @@ interface ActiveExecutorSessionState { } export class TaskExecutor { - private activeWorktrees = new Map(); + /* + FNXC:Workspace 2026-06-21-12:00: + activeWorktrees tracks the worktree paths a task currently holds for liveness/owner checks. In workspace mode a single task acquires N sub-repo worktrees (foundation `task.workspaceWorktrees`), so the value is a SET of paths, not one path. A non-workspace (single-repo) task holds a one-element set — every consumer is converted to membership semantics so the single-repo path is byte-for-byte unchanged (KTD2). Helpers below add/remove/iterate the set. + */ + private activeWorktrees = new Map>(); + + /** + * FNXC:Workspace 2026-06-21-12:00: Register a worktree path under a task's active set, creating the set on first add (KTD2). Single-repo tasks call this once → one-element set. + */ + private addActiveWorktree(taskId: string, worktreePath: string): void { + const set = this.activeWorktrees.get(taskId) ?? new Set(); + set.add(worktreePath); + this.activeWorktrees.set(taskId, set); + } + + /** + * FNXC:Workspace 2026-06-21-12:00: Read-only snapshot of every worktree path a task currently holds (KTD2). Empty when the task holds none. + */ + private getActiveWorktreePaths(taskId: string): string[] { + const set = this.activeWorktrees.get(taskId); + return set ? Array.from(set) : []; + } private executing = new Set(); /** Tasks currently being prepared for unpause resume, before execute() has registered them. */ private resumingUnpaused = new Set(); @@ -1607,9 +1628,10 @@ export class TaskExecutor { this.activeSessions.delete(taskId); // U5: drop the effective column-agent principal for this task's session. this.effectiveColumnAgentByTask.delete(taskId); - const resolvedWorktreePath = worktreePath ?? this.activeWorktrees.get(taskId); - if (resolvedWorktreePath) { - activeSessionRegistry.unregisterPath(resolvedWorktreePath); + // FNXC:Workspace 2026-06-21-12:00: KTD2 — when no explicit path is given, unregister EVERY worktree path the task holds (a workspace task holds N sub-repo paths); single-repo tasks resolve a one-element set. + const resolvedWorktreePaths = worktreePath ? [worktreePath] : this.getActiveWorktreePaths(taskId); + for (const path of resolvedWorktreePaths) { + activeSessionRegistry.unregisterPath(path); } } @@ -1624,9 +1646,10 @@ export class TaskExecutor { this.activeStepExecutorSeenSteeringIds.delete(taskId); // U5: drop the effective column-agent principal for this task's step session. this.effectiveColumnAgentByTask.delete(taskId); - const resolvedWorktreePath = worktreePath ?? this.activeWorktrees.get(taskId); - if (resolvedWorktreePath) { - activeSessionRegistry.unregisterPath(resolvedWorktreePath); + // FNXC:Workspace 2026-06-21-12:00: KTD2 — unregister every held worktree path (Set), not one. + const resolvedWorktreePaths = worktreePath ? [worktreePath] : this.getActiveWorktreePaths(taskId); + for (const path of resolvedWorktreePaths) { + activeSessionRegistry.unregisterPath(path); } } @@ -1639,9 +1662,10 @@ export class TaskExecutor { private deleteActiveWorkflowStepSession(taskId: string, worktreePath?: string): void { this.activeWorkflowStepSessions.delete(taskId); this.activeWorkflowStepSessionSeenSteeringIds.delete(taskId); - const resolvedWorktreePath = worktreePath ?? this.activeWorktrees.get(taskId); - if (resolvedWorktreePath) { - activeSessionRegistry.unregisterPath(resolvedWorktreePath); + // FNXC:Workspace 2026-06-21-12:00: KTD2 — unregister every held worktree path (Set), not one. + const resolvedWorktreePaths = worktreePath ? [worktreePath] : this.getActiveWorktreePaths(taskId); + for (const path of resolvedWorktreePaths) { + activeSessionRegistry.unregisterPath(path); } } @@ -2077,7 +2101,8 @@ export class TaskExecutor { return false; } - const worktreePath = this.activeWorktrees.get(taskId); + // FNXC:Workspace 2026-06-21-12:00: KTD2 — collect every worktree path the task holds (a workspace task holds N) before clearing the binding, so the registry sweep below unregisters all of them, not just one. + const heldWorktreePaths = this.getActiveWorktreePaths(taskId); this.activeWorktrees.delete(taskId); this.executing.delete(taskId); this.recoveringCompleted.delete(taskId); @@ -2087,8 +2112,8 @@ export class TaskExecutor { this.effectiveColumnAgentByTask.delete(taskId); const registeredPaths = new Set(activeSessionRegistry.pathsForTask(taskId)); - if (worktreePath) { - registeredPaths.add(worktreePath); + for (const path of heldWorktreePaths) { + registeredPaths.add(path); } for (const path of registeredPaths) { activeSessionRegistry.unregisterPath(path); @@ -7503,7 +7528,19 @@ export class TaskExecutor { const hadAssignedWorktree = Boolean(task.worktree); const taskCommandAbortController = new AbortController(); this.registerConfiguredCommandController(task.id, taskCommandAbortController); - const acquisition = await (async () => { + /* + FNXC:Workspace 2026-06-21-12:00: + KTD1 — in workspace mode `this.rootDir` is a NON-git parent. Acquiring a root worktree there fails. Skip root acquisition entirely and run the agent session rooted at the browse-only workspace root; the agent acquires per-sub-repo worktrees on demand via fn_acquire_repo_worktree. `task.worktree` stays unset. We synthesize a non-fresh, non-resume acquisition with an empty branch so the downstream env-injection/onStart bookkeeping runs unchanged while every rootDir git preflight (base capture, contamination, liveness) is gated off below. The non-workspace branch is byte-for-byte the original acquisition path. + */ + const acquisition: AcquireTaskWorktreeResult = this.workspaceConfig + ? { + worktreePath: this.rootDir, + branch: "", + source: "existing", + hydrated: true, + isResume: Boolean(task.sessionFile), + } + : await (async () => { try { return await acquireTaskWorktree({ task, @@ -7593,6 +7630,11 @@ export class TaskExecutor { } } + /* + FNXC:Workspace 2026-06-21-12:00: + KTD1 — every preflight below (base-commit capture, contamination check, worktree-liveness gate) runs git against `worktreePath`, which equals the non-git workspace root in workspace mode. They would all fail. Gate the whole block off in workspace mode; the per-repo equivalents return in Phase B (master U3) against each acquired sub-repo worktree. The non-workspace branch is unchanged. + */ + if (!this.workspaceConfig) { // Capture the base commit SHA for diff computation whenever a task // starts with a newly assigned worktree. if (!acquisition.isResume) { @@ -7754,8 +7796,10 @@ export class TaskExecutor { this.options.onError?.(task, new Error(failureMessage)); return; } + } // end !this.workspaceConfig preflight gate (FNXC:Workspace KTD1) - this.activeWorktrees.set(task.id, worktreePath); + // FNXC:Workspace 2026-06-21-12:00: KTD2 — register the worktree path under the task's Set. In workspace mode `worktreePath` is the browse-only root; per-repo sub-repo worktree paths ARE now added to the same Set as the agent acquires them (F2: fn_acquire_repo_worktree's onAcquired callback → addActiveWorktree), so the Set holds root + N sub-repo paths, not just the root. Non-workspace tasks add exactly one path → a one-element set (unchanged liveness/owner semantics). + this.addActiveWorktree(task.id, worktreePath); executorLog.log(`${task.id}: worktree ready at ${worktreePath}`); const injected = await this.buildInjectedRuntimeEnv(task.id, worktreePath, acquisition.branch ?? undefined); @@ -8463,6 +8507,8 @@ export class TaskExecutor { secretsStore: this.options.secretsStore, runContext: engineRunContext, audit, + // FNXC:Workspace 2026-06-21-22:30: F2 — register each freshly-acquired sub-repo worktree path in this task's activeWorktrees Set (KTD2) so owner/liveness checks see live per-repo worktrees, not just the browse-only root. + onAcquired: (worktreePath: string) => this.addActiveWorktree(task.id, worktreePath), taskEnv, // FNXC:Workspace 2026-06-22 — forward the configured worktree-init runner so sub-repo worktrees run configured setup. runConfiguredCommand: (command, cwd, timeoutMs, env) => @@ -10582,8 +10628,13 @@ export class TaskExecutor { options?: { noOpCompletion?: boolean; noOpCompletionReason?: string }, ): Promise<{ ok: true } | { ok: false; reason: "wrong_toplevel" | "wrong_branch" | "no_commits"; observed: string; expected: string }> { const settings = await this.store.getSettings(); + // FNXC:Workspace 2026-06-21-12:00: KTD1/KTD2 — workspace tasks have no root worktree and no single `task.worktree`; the singular per-task invariant is meaningless against the non-git root. Phase B (master U3) iterates this check per sub-repo worktree. Until then it is gated OFF in workspace mode so fn_task_done (its only caller path) does not requeue a zero-acquire workspace task for "missing task.worktree". + if (this.workspaceConfig) { + return { ok: true }; + } const branchName = resolveTaskWorkingBranch(task); - const worktreePath = worktreePathOverride ?? task.worktree ?? this.activeWorktrees.get(task.id) ?? null; + // Non-workspace tasks hold a one-element set; fall back to its sole member to preserve the original singular resolution. + const worktreePath = worktreePathOverride ?? task.worktree ?? this.getActiveWorktreePaths(task.id)[0] ?? null; if (!worktreePath) { return { @@ -14573,9 +14624,9 @@ You have access to the file system to review changes.${verdictBlock}`; conflictPath: string, currentTaskId: string, ): Promise { - // Check if conflicting worktree is in our active set - for (const [taskId, worktreePath] of this.activeWorktrees) { - if (taskId !== currentTaskId && worktreePath === conflictPath) { + // FNXC:Workspace 2026-06-21-12:00: KTD2 — a task may hold N worktree paths; the conflict check is membership across the set, not equality on a single path. + for (const [taskId, worktreePaths] of this.activeWorktrees) { + if (taskId !== currentTaskId && worktreePaths.has(conflictPath)) { return true; } } @@ -14612,8 +14663,11 @@ You have access to the file system to review changes.${verdictBlock}`; */ listWorktreeHolders(): Array<{ taskId: string; worktreePath: string }> { const holders: Array<{ taskId: string; worktreePath: string }> = []; - for (const [taskId, worktreePath] of this.activeWorktrees) { - holders.push({ taskId, worktreePath }); + // FNXC:Workspace 2026-06-21-12:00: KTD2 — flat-map each task's Set into one holder row per worktree path. A workspace task emits N rows; the FN-6782 reaper (self-healing.ts) and in-process-runtime adapter key purely off taskId (verified) and are idempotent across duplicate-task rows, so multi-row holders do not mis-count maxWorktrees slots. + for (const [taskId, worktreePaths] of this.activeWorktrees) { + for (const worktreePath of worktreePaths) { + holders.push({ taskId, worktreePath }); + } } return holders; } @@ -14622,8 +14676,9 @@ You have access to the file system to review changes.${verdictBlock}`; worktreePath: string, requestingTaskId: string, ): Promise { - for (const [taskId, path] of this.activeWorktrees) { - if (taskId !== requestingTaskId && path === worktreePath) { + // FNXC:Workspace 2026-06-21-12:00: KTD2 — membership across the task's worktree set (a workspace task holds N). + for (const [taskId, paths] of this.activeWorktrees) { + if (taskId !== requestingTaskId && paths.has(worktreePath)) { return taskId; } } @@ -14631,10 +14686,18 @@ You have access to the file system to review changes.${verdictBlock}`; const tasks = await this.store.listTasks({ slim: true, includeArchived: false }); for (const t of tasks) { if (t.id === requestingTaskId) continue; - if (t.worktree !== worktreePath) continue; if (t.column !== "in-progress") continue; if (t.paused === true) continue; - return t.id; + if (t.worktree === worktreePath) return t.id; + // FNXC:Workspace 2026-06-22-09:00: workspace tasks hold their worktrees in + // task.workspaceWorktrees, not the singular task.worktree column. The DB liveness + // fallback must check those per-sub-repo paths too — otherwise a conflict against a + // sub-repo worktree owned by an in-progress workspace task is missed, especially + // before its in-memory activeWorktrees entry is (re)registered after restart. + const wsEntries = t.workspaceWorktrees; + if (wsEntries && Object.values(wsEntries).some((entry) => entry.worktreePath === worktreePath)) { + return t.id; + } } } catch (err: unknown) { const msg = err instanceof Error ? err.message : String(err); @@ -14649,12 +14712,9 @@ You have access to the file system to review changes.${verdictBlock}`; * Returns true if cleanup succeeded. */ private hasActiveWorktreeBinding(taskId: string, worktreePath: string): boolean { - for (const [activeTaskId, activePath] of this.activeWorktrees) { - if (activeTaskId === taskId && activePath === worktreePath) { - return true; - } - } - return false; + // FNXC:Workspace 2026-06-21-12:00: KTD2 — membership across the task's worktree set. + const paths = this.activeWorktrees.get(taskId); + return paths ? paths.has(worktreePath) : false; } private async reconcileSelfOwnedBeforeRemove(worktreePath: string, taskId: string): Promise { @@ -15052,11 +15112,18 @@ You have access to the file system to review changes.${verdictBlock}`; * always cleaned up by the merger on a per-task basis. */ async cleanup(taskId: string): Promise { - const worktreePath = this.activeWorktrees.get(taskId); - if (!worktreePath) return; + const worktreePaths = this.getActiveWorktreePaths(taskId); + if (worktreePaths.length === 0) return; this.activeWorktrees.delete(taskId); + // FNXC:Workspace 2026-06-21-12:00: KTD1 — in workspace mode the tracked path is the non-git workspace root (browse-only), never a removable worktree. Drop the in-memory tracking above but never remove the root. Per-repo worktree teardown returns in Phase B. + if (this.workspaceConfig) { + return; + } + // Non-workspace tasks hold a one-element set — preserve the original single-path removal semantics. + const worktreePath = worktreePaths[0]; + // Check if another task still needs this worktree const otherUser = await findWorktreeUser(this.store, worktreePath, taskId); if (otherUser) { @@ -15371,6 +15438,20 @@ You have access to the file system to review changes.${verdictBlock}`; const preserveProgress = settings.preserveProgressOnStuckRequeue !== false; const latestTask = await this.store.getTask(taskId); const worktreePath = this.getWorktreePath(taskId) ?? latestTask.worktree; + /* + FNXC:Workspace 2026-06-21-22:30: + F8 — observability for the workspace case. A workspace task has no singular + worktree (getWorktreePath returns undefined for a multi-worktree task, and + latestTask.worktree is null on the browse-only root), so the removeWorktree + block below silently no-ops. Per-repo teardown is Phase B; until then make + the skip visible rather than silent. Behavior is unchanged. + */ + if (this.workspaceConfig && !worktreePath) { + await this.store.logEntry( + taskId, + `workspace task ${taskId}: no singular worktree to force-requeue (per-repo teardown is Phase B)`, + ); + } await this.store.logEntry( taskId, `Force-kill cleanup starting after stuck-kill unwind timeout — reaping in-flight surfaces and worktree`, @@ -15553,8 +15634,14 @@ You have access to the file system to review changes.${verdictBlock}`; return true; } + /** + * FNXC:Workspace 2026-06-21-12:00: KTD2 single-path-getter contract. Returns the task's sole worktree path for single-repo tasks (one-element set). For a multi-worktree workspace task there is no single answer — callers must read the per-repo `task.workspaceWorktrees` entry instead — so this returns undefined. A workspace task tracked only at the browse-only root also returns undefined, matching the "no removable single worktree" semantics. + */ getWorktreePath(taskId: string): string | undefined { - return this.activeWorktrees.get(taskId); + if (this.workspaceConfig) { + return undefined; + } + return this.getActiveWorktreePaths(taskId)[0]; } // ── Agent Spawning ───────────────────────────────────────────────────── @@ -15865,7 +15952,11 @@ function formatTimestamp(iso: string): string { // Project commands are injected here (for reliability) and also in the PROMPT.md (by triage). // This ensures the executor agent always sees the authoritative commands from settings, // even if the PROMPT.md was written manually or before commands were configured. -function scopePromptToWorktree(prompt: string, rootDir?: string, worktreePath?: string): string { +function scopePromptToWorktree(prompt: string, rootDir?: string, worktreePath?: string, workspaceConfig?: WorkspaceConfig | null): string { + // FNXC:Workspace 2026-06-21-12:00: KTD1 — in workspace mode the session is rooted at the workspace root itself (worktreePath === rootDir) and path rewriting to a per-task root worktree is meaningless: edits happen in per-sub-repo worktrees the agent acquires, not at the root. No-op the rewrite. (The rootDir === worktreePath guard below already covers this, but gate explicitly so intent survives future refactors.) + if (workspaceConfig) { + return prompt; + } if (!rootDir || !worktreePath || rootDir === worktreePath || !prompt.includes(rootDir)) { return prompt; } @@ -15899,7 +15990,7 @@ export function buildExecutionPrompt( customFieldDefs?: WorkflowFieldDefinition[], workspaceConfig?: WorkspaceConfig | null, ): string { - const prompt = scopePromptToWorktree(task.prompt, rootDir, worktreePath); + const prompt = scopePromptToWorktree(task.prompt, rootDir, worktreePath, workspaceConfig); const reviewLevel = parseReviewLevelFromPrompt(prompt); // Build co-author trailer arg for git commits based on settings. The user's diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index 5870a75ad0..2f09727be6 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -99,6 +99,11 @@ export type GitMutationType = | "worktree:incomplete-detected" | "worktree:reanchored" | "worktree:auto-recovered" + // FNXC:Workspace 2026-06-21-20:10: workspace per-repo acquisition audit events (U2). + // -busy: another task holds the same sub-repo's acquisition exclusivity lock (KTD4). + // -failed: a sub-repo worktree acquisition threw; surfaced + audited, never swallowed. + | "worktree:workspace-repo-acquire-busy" + | "worktree:workspace-repo-acquire-failed" /** * worktrunk run-audit metadata shape: * diff --git a/packages/engine/src/worktree-acquisition.ts b/packages/engine/src/worktree-acquisition.ts index 4bd2a1c8db..5c51ce45af 100644 --- a/packages/engine/src/worktree-acquisition.ts +++ b/packages/engine/src/worktree-acquisition.ts @@ -35,6 +35,10 @@ import { import type { RunAuditor } from "./run-audit.js"; import { writeSecretsEnvFile } from "./secrets-env-writer.js"; import { removeDesktopBuildArtifacts } from "./worktree-desktop-artifacts.js"; +import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js"; +import { resolveCapturedBaseCommitSha } from "./base-commit-capture.js"; +import { resolveIntegrationBranch } from "./integration-branch.js"; +import { activeSessionRegistry, type ActiveSessionRegistry } from "./active-session-registry.js"; const execAsync = promisify(exec); @@ -664,8 +668,10 @@ export interface AcquireWorkspaceRepoWorktreeOptions { settings: Partial; logger?: { log: (m: string) => void; warn: (m: string) => void; error?: (m: string) => void }; secretsStore?: Pick; - runContext?: RunMutationContext; audit?: Pick; + runContext?: RunMutationContext; + /** Test seam: inject the path-keyed exclusivity registry (defaults to the process singleton). */ + registry?: ActiveSessionRegistry; runConfiguredCommand?: AcquireTaskWorktreeOptions["runConfiguredCommand"]; taskEnv?: NodeJS.ProcessEnv; } @@ -686,10 +692,20 @@ function assertInRootRepoRelPath(repoRelPath: string, sep: string, isAbsolute: ( } } +/* +FNXC:Workspace 2026-06-21-20:10: +Acquisition-time exclusivity owner key for the same-sub-repo lock (U2/KTD4). The +registry record is keyed by the sub-repo ABSOLUTE path and carries this distinct +ownerKey so it never collides with the executor's later "executor"/"step-session" +registration on the produced WORKTREE path. +*/ +const WORKSPACE_REPO_ACQUIRE_OWNER_KEY = "workspace-repo-acquire"; + export async function acquireWorkspaceRepoWorktree( opts: AcquireWorkspaceRepoWorktreeOptions, -): Promise<{ worktreePath: string; branch: string; alreadyAcquired: boolean }> { - const { repoRelPath, workspaceRootDir, task, store, settings, logger, secretsStore, runContext, audit, runConfiguredCommand, taskEnv } = opts; +): Promise<{ worktreePath: string; branch: string; baseCommitSha?: string; alreadyAcquired: boolean }> { + const { repoRelPath, workspaceRootDir, task, store, settings, logger, secretsStore, audit, runContext, runConfiguredCommand, taskEnv } = opts; + const registry = opts.registry ?? activeSessionRegistry; const { join, isAbsolute, normalize, sep } = await import("node:path"); // FNXC:WorkspaceWorktree 2026-06-22-00:00: reject absolute / `..`-escaping repo paths before resolving. @@ -706,6 +722,12 @@ export async function acquireWorkspaceRepoWorktree( */ const existing = task.workspaceWorktrees?.[repoRelPath]; if (existing) { + /* + FNXC:Workspace 2026-06-21-20:10: + Idempotency across (taskId, repo): a re-acquire of an already-acquired sub-repo + returns the persisted entry verbatim — no second identity-guard install, no + re-capture of the base SHA, no second exclusivity registration. + */ let live = existsSync(existing.worktreePath); if (live) { try { @@ -723,56 +745,263 @@ export async function acquireWorkspaceRepoWorktree( } /* - FNXC:WorkspaceWorktree 2026-06-21-19:05: - Workspace mode acquires one worktree per sub-repo for a single task. `acquireTaskWorktree` - is single-repo: it reads `task.worktree`/`task.branch` to decide resume-vs-fresh and rewrites - those singular fields on the task row after each acquisition. Passing the live task straight - through means the second repo's acquisition sees the first repo's `task.worktree` (which exists - on disk), classifies it as a resume, and reuses repo A's worktree inside repo B — cross-repo - contamination. Clear the singular worktree/branch fields on the copy handed to the single-repo - helper so each sub-repo always gets a fresh worktree; per-repo state is tracked in - `task.workspaceWorktrees`, not the singular column. - - FNXC:WorkspaceWorktree 2026-06-22-00:00: - `acquireTaskWorktree` only runs the configured worktree-init command when `runConfiguredCommand` - is threaded through. Forward it (plus runContext/audit/taskEnv) so workspace sub-repos run the - same configured setup as the non-workspace acquire path instead of silently skipping it. + FNXC:Workspace 2026-06-22-09:00: + Run best-effort observability (task log + audit) for the NON-FATAL post-acquire + steps without letting their own awaited writes escape. logEntry/audit can throw + (DB hiccup, audit sink failure); an unsuppressed throw inside a non-fatal catch + would re-escalate guard/base-capture failures into fatal acquisition errors that + strand the already-created worktree. Mirrors the busy-path swallow above. */ - const result = await acquireTaskWorktree({ - task: { ...task, worktree: undefined, branch: undefined }, - rootDir: repoAbsPath, - store, - settings, - logger, - secretsStore, - runContext, - audit, - runConfiguredCommand, - taskEnv, - runInitCommand: true, + const safeObserve = async (fn: () => Promise): Promise => { + try { + await fn(); + } catch (obsErr) { + logger?.warn( + `${task.id}: workspace acquisition observability failed (suppressed): ${obsErr instanceof Error ? obsErr.message : String(obsErr)}`, + ); + } + }; + + /* + FNXC:Workspace 2026-06-21-20:10: + Same-sub-repo exclusivity (KTD4): register the sub-repo absolute path in the + path-keyed activeSessionRegistry BEFORE acquiring so two concurrent workspace + tasks contending for the SAME sub-repo are serialized. WorktreePool is a recycle + cache, not a cross-task lock, and disjoint-scope contention on one sub-repo is + otherwise unprotected (file-scope leases don't catch it). The entry is keyed by + the sub-repo path with a distinct ownerKey so it does not collide with the + executor's later session registration on the produced worktree path. We release + it once acquisition completes (success or failure) — it guards the acquisition + critical section, not the whole task lifetime. + */ + const exclusivityHolder = registry.lookupByPath(repoAbsPath); + if (exclusivityHolder && exclusivityHolder.ownerKey === WORKSPACE_REPO_ACQUIRE_OWNER_KEY && exclusivityHolder.taskId !== task.id) { + const err = new WorkspaceRepoAcquireBusyError(repoRelPath, exclusivityHolder.taskId, task.id); + /* + FNXC:Workspace 2026-06-21-22:30: + F6 — the busy short-circuit's logEntry/audit are best-effort observability; if + either throws (e.g. a DB write hiccup) it must NOT replace the + WorkspaceRepoAcquireBusyError the caller relies on to classify "serialized, + retry later". Swallow logging failures so the busy error is what propagates. + */ + try { + const message = `sub-repo ${repoRelPath} is being acquired by ${exclusivityHolder.taskId}; serializing concurrent workspace acquisition`; + logger?.warn(`${task.id}: ${message}`); + await store.logEntry(task.id, message, undefined, runContext); + await audit?.git({ + type: "worktree:workspace-repo-acquire-busy", + target: repoAbsPath, + metadata: { repoRelPath, holderTaskId: exclusivityHolder.taskId, requestingTaskId: task.id }, + }); + } catch { + // best-effort observability only — never mask the busy error + } + throw err; + } + /* + FNXC:Workspace 2026-06-21-22:30: + F9 — no `await` may be inserted between lookupByPath and registerPath: the + atomicity of the exclusivity claim depends on staying in one synchronous slice. + An interleaved await would let a second task pass the lookup gate before this + task registers, defeating the same-sub-repo serialization (KTD4). + */ + registry.registerPath(repoAbsPath, { + taskId: task.id, + kind: "workspace-repo-acquire", + ownerKey: WORKSPACE_REPO_ACQUIRE_OWNER_KEY, }); - /* - 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 = { - ...(freshTask.workspaceWorktrees ?? {}), - [repoRelPath]: { worktreePath: result.worktreePath, branch: result.branch }, - }; - /* - FNXC:WorkspaceWorktree 2026-06-22-00:00: - `acquireTaskWorktree` persists the singular `task.worktree`/`task.branch` on the task row. - For a workspace task that pointer would end up referencing whichever sub-repo was acquired last, - violating the contract that per-repo state lives only in `workspaceWorktrees`. Clear the singular - fields in the same update so a workspace task never carries a misleading singular worktree pointer. - */ - await store.updateTask(task.id, { workspaceWorktrees: updated, worktree: null, branch: null }); + try { + /* + FNXC:WorkspaceWorktree 2026-06-21-19:05: + Workspace mode acquires one worktree per sub-repo for a single task. `acquireTaskWorktree` + is single-repo: it reads `task.worktree`/`task.branch` to decide resume-vs-fresh and rewrites + those singular fields on the task row after each acquisition. Passing the live task straight + through means the second repo's acquisition sees the first repo's `task.worktree` (which exists + on disk), classifies it as a resume, and reuses repo A's worktree inside repo B — cross-repo + contamination. Clear the singular worktree/branch fields on the copy handed to the single-repo + helper so each sub-repo always gets a fresh worktree; per-repo state is tracked in + `task.workspaceWorktrees`, not the singular column. + */ + const result = await acquireTaskWorktree({ + task: { ...task, worktree: undefined, branch: undefined }, + rootDir: repoAbsPath, + store, + settings, + logger, + secretsStore, + audit, + runContext, + runConfiguredCommand, + taskEnv, + runInitCommand: true, + }); - return { worktreePath: result.worktreePath, branch: result.branch, alreadyAcquired: false }; + /* + FNXC:Workspace 2026-06-21-22:30: + F3 — post-acquire steps are NON-FATAL. Once acquireTaskWorktree has created the + on-disk worktree, a failure of the identity-guard install or the base-SHA capture + must NOT strand that worktree (the previous catch re-threw, leaving the worktree + orphaned while the exclusivity entry released). The worktree is usable without the + identity guard, and an undefined baseCommitSha is already an accepted state. Only a + failure of acquireTaskWorktree ITSELF fails the acquisition. Each step is wrapped to + log a warning (and emit the existing failure audit event) but CONTINUE. + */ + + /* + FNXC:Workspace 2026-06-21-20:10: + Identity guard (single-repo parity): acquireTaskWorktree above runs WITHOUT a + createWorktree override, so the default native backend installs NO identity + hooks for a sub-repo worktree. Install the same guard the executor installs for + single-repo tasks (executor.ts identity-guard call), passing the SAME settings + args (commitMsgHookEnabled / taskPrefix / first taskAttributionTrailerName) so a + commit on a non-fusion/ branch is refused inside every sub-repo worktree too. + */ + try { + await installTaskWorktreeIdentityGuard({ + worktreePath: result.worktreePath, + taskId: task.id, + commitMsgHookEnabled: settings.commitMsgHookEnabled, + taskPrefix: settings.taskPrefix, + taskAttributionTrailerName: settings.taskAttributionTrailerNames?.[0], + }); + } catch (guardErr) { + // FNXC:Workspace 2026-06-21-22:30: F3 — identity-guard install is non-fatal; worktree is usable without it. + const message = guardErr instanceof Error ? guardErr.message : String(guardErr); + logger?.warn(`${task.id}: identity-guard install failed for sub-repo ${repoRelPath} (non-fatal): ${message}`); + // FNXC:Workspace 2026-06-22-09:00: the observability writes (store.logEntry / audit.git) + // are themselves awaited and can throw; an unwrapped throw here would escape the catch + // and re-escalate this deliberately NON-FATAL step into a fatal acquisition error, + // stranding the already-created worktree. Suppress observability failures via safeObserve. + await safeObserve(async () => { + await store.logEntry(task.id, `Workspace sub-repo identity-guard install failed for ${repoRelPath} (non-fatal): ${message}`, undefined, runContext); + await audit?.git({ + type: "worktree:workspace-repo-acquire-failed", + target: repoAbsPath, + metadata: { repoRelPath, taskId: task.id, error: message, stage: "identity-guard" }, + }); + }); + } + + /* + FNXC:Workspace 2026-06-21-20:10: + Per-repo base SHA (KTD3): resolve THIS sub-repo's integration branch with the + shared settings.integrationBranch AND settings.baseBranch overrides STRIPPED. + resolveFromSettings (integration-branch.ts) falls back integrationBranch → + baseBranch → origin/HEAD, so leaving either set means every sub-repo resolves to + the shared workspace branch — defeating per-repo resolution (F4). With both + undefined, each sub-repo falls through to its own origin/HEAD. Capture the base + local-first against that branch so local-ahead-of-origin integration tips don't + inflate the per-repo diff (FN-5937 invariant, per sub-repo). + */ + let baseCommitSha: string | undefined; + try { + const integrationBranch = await resolveIntegrationBranch( + repoAbsPath, + { ...settings, integrationBranch: undefined, baseBranch: undefined }, + { logger }, + ); + baseCommitSha = await resolveCapturedBaseCommitSha(result.worktreePath, logger, integrationBranch); + } catch (baseErr) { + // FNXC:Workspace 2026-06-21-22:30: F3 — base-SHA capture is non-fatal; an undefined baseCommitSha is an accepted state. + const message = baseErr instanceof Error ? baseErr.message : String(baseErr); + logger?.warn(`${task.id}: base-SHA capture failed for sub-repo ${repoRelPath} (non-fatal): ${message}`); + // FNXC:Workspace 2026-06-22-09:00: same non-fatal contract as the identity-guard catch — + // the awaited observability writes must not re-escalate a non-fatal base-capture failure. + await safeObserve(async () => { + await store.logEntry(task.id, `Workspace sub-repo base-SHA capture failed for ${repoRelPath} (non-fatal): ${message}`, undefined, runContext); + await audit?.git({ + type: "worktree:workspace-repo-acquire-failed", + target: repoAbsPath, + metadata: { repoRelPath, taskId: task.id, error: message, stage: "base-sha-capture" }, + }); + }); + } + + /* + FNXC:Workspace 2026-06-21-22:30: + F5 — re-read the task fresh immediately before building the merged + workspaceWorktrees map. store.updateTask wholesale-replaces the map, and the + `task` snapshot was read earlier; two sequential acquires for DIFFERENT sub-repos + in one task would otherwise clobber a sibling's entry. Merging into the LATEST map + closes the common sequential-tool-call case. NOTE: a fully-atomic store-level + per-repo merge is the complete fix (it also covers truly-concurrent writes); it is + deferred to Phase B, which exercises multi-repo acquisition. + */ + const latest = await store.getTask(task.id); + const updated: Record = { + ...(latest.workspaceWorktrees ?? {}), + [repoRelPath]: { worktreePath: result.worktreePath, branch: result.branch, baseCommitSha }, + }; + /* + FNXC:Workspace 2026-06-22-09:00: + F10 — reset the singular worktree/branch columns to null in the SAME write that + persists workspaceWorktrees. The single-repo `acquireTaskWorktree` above wrote + `task.worktree`/`task.branch` (the sub-repo path/branch) to the real task row; + clearing the in-memory copy passed in only stops the NEXT sub-repo from resuming + into this one's worktree — the DB row stays polluted. A non-null `task.worktree` + makes `isWorkspaceTask(task)` return false (its first guard), so the dashboard + stops rendering WorkspaceWorktreesSummary and instead shows the sub-repo branch in + the standard chip — the blank/wrong-card state U10 prevents. Nulling them here + keeps `task.worktree` null for the workspace task's whole lifetime. + */ + await store.updateTask(task.id, { workspaceWorktrees: updated, worktree: null, branch: null }); + + return { worktreePath: result.worktreePath, branch: result.branch, baseCommitSha, alreadyAcquired: false }; + } catch (err) { + /* + FNXC:Workspace 2026-06-21-20:10: + Acquisition failure must surface an error and leave an audit trail (no swallowed + stall): persist the failure as an audit event + task log, then re-throw so the + caller observes the failure rather than silently proceeding with an unacquired + sub-repo. + */ + if (!(err instanceof WorkspaceRepoAcquireBusyError)) { + const message = err instanceof Error ? err.message : String(err); + logger?.error?.(`${task.id}: workspace sub-repo acquisition failed for ${repoRelPath}: ${message}`); + // FNXC:Workspace 2026-06-22-09:30: the fatal-path observability writes must use safeObserve + // for the same reason as the non-fatal catches — an unsuppressed throw from logEntry/audit + // would replace `err` as the propagated rejection, so a store/audit hiccup could surface a + // non-WorkspaceRepoAcquireBusyError to callers whose `instanceof` type checks then misfire. + // The original acquisition `err` (line below) is the contract; observability is best-effort. + await safeObserve(async () => { + await store.logEntry(task.id, `Workspace sub-repo acquisition failed for ${repoRelPath}: ${message}`, undefined, runContext); + await audit?.git({ + type: "worktree:workspace-repo-acquire-failed", + target: repoAbsPath, + metadata: { repoRelPath, taskId: task.id, error: message }, + }); + }); + } + throw err; + } finally { + /* + FNXC:Workspace 2026-06-21-20:10: + Release the acquisition-time exclusivity entry only when WE hold it. The busy-path + throw above does NOT enter this try (it short-circuits before registerPath), so a + serialized loser never unregisters the winner's entry. + */ + const held = registry.lookupByPath(repoAbsPath); + if (held && held.taskId === task.id && held.ownerKey === WORKSPACE_REPO_ACQUIRE_OWNER_KEY) { + registry.unregisterPath(repoAbsPath); + } + } +} + +/* +FNXC:Workspace 2026-06-21-20:10: +Thrown when a second workspace task tries to acquire a sub-repo already inside +another task's acquisition critical section (KTD4). Distinct from generic +acquisition failures so the caller (and tests) can tell "serialized, retry later" +apart from "this sub-repo is broken". +*/ +export class WorkspaceRepoAcquireBusyError extends Error { + constructor( + public readonly repoRelPath: string, + public readonly holderTaskId: string, + public readonly requestingTaskId: string, + ) { + super(`workspace sub-repo ${repoRelPath} acquisition is in progress for task ${holderTaskId}`); + this.name = "WorkspaceRepoAcquireBusyError"; + } }