feat(workspace): Phase B — per-repo capture, contamination, review, completion verify (U3/U4) (#1714)
> ⚠️ **Draft — do not merge until the stack lands.** Stacks on the workspace foundation (#1710) + U0 (#1711) + Phase A (#1713). Targets `main`; its diff **includes the whole stack**. **Review only the Phase-B commits** (`fc9423e` U1, `81edbee` U2, `453ed92` review fixes). Retarget once the stack merges. ## Workspace mode — Phase B (per-repo capture, contamination, review, completion verify) Implements Phase B of the [master plan](docs/plans/2026-06-21-002-feat-workspace-mode-execution-model-plan.md) — units U3, U4 ([Phase-B plan](docs/plans/2026-06-21-005-feat-workspace-phase-b-plan.md)). The executor's change-capture, contamination, worktree-invariant, review, and completion-verify paths now iterate `task.workspaceWorktrees` **per sub-repo**, each against that repo's own `baseCommitSha` (Phase A), with repo-prefixed file lists. After Phase B a workspace task can run **and** be captured + reviewed + completion-verified — short of merge (Phase C). A feasibility pre-check corrected the plan before implementation: capture/contamination/scope-leak were **not gated** — they silently degraded to empty against the non-git root. Phase B adds the missing workspace branches and **reuses the existing `captureModifiedFiles`** machinery per repo (its `resolveDiffBaseRef` merge-base fallback handles undefined per-repo bases; its `filterFilesToOwnTaskCommits` divergence audit restores contamination for free) rather than hand-building diffs. ### What changed - **U1 — per-repo capture + verify.** Post-session capture loops `workspaceWorktrees`, reusing `captureModifiedFiles(repo.worktreePath, repo.baseCommitSha, …)` and repo-prefixing into `task.modifiedFiles`; branch-attribution runs per sub-repo. `verifyWorktreeInvariants` un-stubbed to iterate every acquired worktree, **preserving its `{ok|reason|observed|expected}` union** (the `reason` enum drives requeue/handoff) with an additive `repo` field. The no-op `assertCleanBranchAtBase` is not iterated. - **U2 — per-repo review (both sites) + completion verify.** A shared `reviewWorkspacePerRepo` loops the existing single-cwd `reviewStep` once per sub-repo (the reviewer agent runs its own `git diff` at that cwd — **N reviewer agents**, an accepted cost) and aggregates repo-tagged verdicts as a **conjunction**. Both entry points iterate — `fn_review_step` and the step-inversion seam (FN-5893). `fn_task_done` runs the per-repo `verifyWorktreeInvariants` and a per-repo `evaluateTaskDoneScopeLeak`. New `workspace-paths.ts` repo-prefix helper. ### Review 4-persona `ce-code-review`. **No P0**; the review conjunction was confirmed safe (no false-done — empty map and per-repo throws both route to UNAVAILABLE → blocks). Fixed in-branch: the scope-leak guard now **fails closed** on a per-repo throw (was fail-open) and **blocks a scoped task that acquired zero worktrees** (was silently passing); the review loop breaks on first non-APPROVE (preserving the verdict); the `.changeset` always-allowed carve-out is honored in workspace mode (wiring in the repo-local helper); deterministic offending-repo ordering. Verified safe: semaphore release on throw + reviewer abort via session disposal. ### Deferred to Phase C - Extract a `workspace-executor.ts` module (executor.ts is 16k+ lines) — to land **at the start of Phase C, before the merge loop** (maintainability P1). - Committed off-scope changes can still slip past per-repo scope-leak when a repo's `baseCommitSha` is undefined and no merge-base resolves (`captureModifiedFiles` returns `[]`) — partially pre-existing, amplified by treating undefined base as normal. - Store-level **atomic** per-repo `workspaceWorktrees` merge (becomes reachable here with multi-repo acquisition). - The merge loop / landed predicate / file-scope leases (master U5/U6/U7). ### Verification Gate green: lint, typecheck (29 projects), build, `test:gate` (649+58); workspace tests 28+ (capture, review conjunction, fail-closed scope guard, zero-acquire block, `.changeset` carve-out). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- stage-review-badge-begin --> --- <a href="https://stagereview.app/Runfusion/Fusion/pull/1714"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://stagereview.app/assets/gh-open-in-stage-dark.svg"> <img src="https://stagereview.app/assets/gh-open-in-stage-light.svg" alt="Open in Stage"> </picture> </a> <!-- stage-review-badge-end --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added workspace mode support for multi-repository task execution with per-repo worktree management. * Dashboard now displays workspace task status with "N repos acquired" summary and per-repo worktree details in task cards and detail modals. * **Bug Fixes** * Fixed workspace tasks rendering blank on the dashboard; task cards and detail views now display acquired repository information. * **Improvements** * Enhanced workspace acquisition reliability with identity guards, per-repo base commit tracking, and serialized acquisition controls to prevent conflicts. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
Workspace mode (Phase B, U1): per-repo post-session change capture, contamination detection, and worktree-invariant verification. In workspace mode the executor now loops `task.workspaceWorktrees`, reusing `captureModifiedFiles` per sub-repo (diffing each against its own `baseCommitSha`, with a merge-base fallback when undefined) to aggregate repo-prefixed `task.modifiedFiles` and surface per-repo contamination, and un-stubs `verifyWorktreeInvariants` to assert each acquired worktree's git toplevel and `fusion/<id>` branch. Single-repo behavior is unchanged.
|
||||
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
Workspace mode (Phase B, U2): per-repo review at both review entry points plus per-repo `fn_task_done` completion + scope-leak verification. In workspace mode both review call sites (the in-session `fn_review_step` tool and the step-inversion review seam) now loop the single-cwd `reviewStep` once per acquired sub-repo (cwd = each repo's worktree) and aggregate the repo-tagged verdicts as a conjunction — the task is reviewed only when every sub-repo approves, and the first failing sub-repo's verdict (with repo-tagged findings) drives the existing verdict→edge mapping. `fn_task_done` now verifies worktree invariants per acquired repo and iterates the scope-leak guard per sub-repo (cwd = repo worktree, repo `baseCommitSha`), blocking completion on any sub-repo carrying off-scope changes and naming the repo. Adds a minimal shared repo-prefix-derivation helper (`workspace-paths.ts`). Single-repo behavior is unchanged.
|
||||
|
||||
Phase-B hardening: the per-repo scope-leak guard now fails CLOSED — a thrown capture/diff error in any sub-repo refuses `fn_task_done` (naming the repo) instead of failing open, and a scoped task that acquired zero sub-repo worktrees is blocked rather than silently passing. A legitimate per-repo `.changeset/` file is no longer falsely flagged off-scope (the always-allowed carve-out now runs against the repo-local path). Per-repo review stops at the first non-APPROVE sub-repo so a later repo's reviewer error can't mask an already-determined REVISE/RETHINK. Per-repo capture failures are isolated (one repo's error no longer drops the whole modified-files write), and the reported offending/failing repo is now deterministic (sorted repo iteration). Single-repo behavior remains unchanged.
|
||||
@@ -155,7 +155,7 @@ The surface-enumeration spine (FN-5893). Every row is a single-worktree / `cwd:r
|
||||
|
||||
Additive only — no migration to existing single-repo tasks:
|
||||
|
||||
```
|
||||
```ts
|
||||
Task.workspaceWorktrees: Record<repoRelPath, {
|
||||
worktreePath: string;
|
||||
branch: string;
|
||||
|
||||
145
docs/plans/2026-06-21-005-feat-workspace-phase-b-plan.md
Normal file
145
docs/plans/2026-06-21-005-feat-workspace-phase-b-plan.md
Normal file
@@ -0,0 +1,145 @@
|
||||
---
|
||||
title: "feat: Workspace mode Phase B — per-repo capture, contamination, review, completion verify"
|
||||
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 B / U3·U4)
|
||||
depth: deep
|
||||
---
|
||||
|
||||
# feat: Workspace mode Phase B — per-repo capture, contamination, review, completion verify
|
||||
|
||||
> **ID namespace:** local `U1·U2` decompose master-plan **U3, U4**.
|
||||
> **Anchors below are feasibility-verified against the Phase-B base** (not the master plan's approximate numbers).
|
||||
|
||||
## Summary
|
||||
|
||||
Phase B makes the executor's capture / contamination / verify / review / completion paths iterate `task.workspaceWorktrees` per sub-repo, using each repo's own `baseCommitSha` (Phase A, U2). It does **not** simply "un-gate stubs" — the feasibility pass found capture/contamination/scope-leak are not gated at all today; they **silently degrade to empty** against the non-git root (git failures swallowed). Phase B adds the missing workspace branches and reuses the existing `captureModifiedFiles` machinery (whose `resolveDiffBaseRef` merge-base fallback + `filterFilesToOwnTaskCommits` contamination audit are exactly what's needed) per repo.
|
||||
|
||||
Builds on Phase A (PR #1713). **Scope out:** the merge loop (master U6 = Phase C), self-healing (master U8 = Phase D).
|
||||
|
||||
**Stacking:** off the Phase-A branch; PR diff includes the stack; must not merge until it lands.
|
||||
|
||||
---
|
||||
|
||||
## Problem Frame
|
||||
|
||||
Phase A rooted workspace sessions at the non-git workspace root and acquired per-repo worktrees, but the executor's change-capture, contamination, worktree-invariant, review, and completion-verify paths still operate on a single `task.worktree`. Against the non-git root they either are explicitly stubbed (one site) or silently produce empty results (the rest). Phase B routes each of these through every acquired sub-repo worktree, `cwd` = the sub-repo, diffing against that repo's `workspaceWorktrees[repo].baseCommitSha`, with repo-prefixed file lists so review/dashboard/later-merge keep repo context.
|
||||
|
||||
---
|
||||
|
||||
## Key Technical Decisions
|
||||
|
||||
### KTD1 — Per-repo change capture by **reusing `captureModifiedFiles`**, not a raw diff (master KTD7)
|
||||
**Verified reality:** capture is **not** workspace-gated. The post-session call `captureModifiedFiles(worktreePath, …, "post-session")` (executor.ts **:7898**) runs ungated with `worktreePath` = the browse-only non-git root and returns `[]` only because `resolveDiffBaseRef`/`resolveContaminationBaseRef` swallow the git failure. So U1 **adds** a workspace branch at :7898 (and the sibling branch-attribution audit at **:7914**), it does not replace one.
|
||||
|
||||
Per repo, call the **existing** `captureModifiedFiles(repo.worktreePath, repo.baseCommitSha, task.id, audit, source)` — NOT a hand-built `git diff <base>..HEAD`. Reasons (all verified): (a) `repo.baseCommitSha` may be **undefined** (Phase A made base capture non-fatal); `resolveDiffBaseRef` (:~12184) handles that via a merge-base fallback. (b) the real **contamination** signal is the `filterFilesToOwnTaskCommits` raw-vs-attributed divergence audit **inside** `captureModifiedFiles` (:~12225-12246) — reusing it restores contamination for free. Prefix each repo's returned files with the repo path and aggregate into `task.modifiedFiles`.
|
||||
|
||||
> **`assertCleanBranchAtBase` is a no-op** (branch-conflicts.ts: `void`s all params — "informational only"). Do **not** add a per-repo iteration of it; it would restore zero protection. Contamination comes from per-repo `captureModifiedFiles`.
|
||||
|
||||
### KTD2 — `verifyWorktreeInvariants` iterates per acquired worktree, preserving its result union (master KTD7)
|
||||
The **one** workspace stub in this region is `verifyWorktreeInvariants` returning `{ok:true}` at executor.ts **:10508** (def **:10500**). Un-stub it: iterate every `workspaceWorktrees` entry, asserting each HEAD is on `fusion/<id>` and toplevel matches the recorded `worktreePath`. **Preserve the exact discriminated union** `{ok:true} | {ok:false; reason:'wrong_toplevel'|'wrong_branch'|'no_commits'; observed; expected}` (consumed at **:10889**; the `reason` enum drives the requeue/handoff branches at :10894-10936) — add a `repo` field to the failure shape; return the **first** failing repo.
|
||||
|
||||
### KTD3 — Per-repo review by looping the **existing single-cwd `reviewStep`** N times (master KTD7)
|
||||
**Decision (user-confirmed): accept the N× reviewer cost.** The reviewer is an **agent** spawned with `cwd` = worktree and told (in prompt text, reviewer.ts:~760) to run `git diff` itself — it does not read a diff passed in code. So per-repo review = spawning **one reviewer agent per sub-repo**. Architecture: the **callers loop** and call the existing single-cwd `reviewStep` (reviewer.ts **:122**) once per acquired worktree (cwd = repo, scope = prefix-derived subset); aggregate repo-tagged verdicts into the task's single review record as a **conjunction** (reviewed only if every repo passes). `reviewStep` itself stays single-cwd.
|
||||
|
||||
**Both review call sites iterate (user-confirmed FN-5893 coverage):**
|
||||
- `createReviewStepTool` → `reviewStep` (executor.ts **:11148**, the in-session `fn_review_step` path).
|
||||
- the **step-inversion seam** `reviewStep(worktreePath=active.worktreePath || detail.worktree || this.rootDir, …)` at executor.ts **:5668** (foreach/step-inversion path).
|
||||
|
||||
### KTD4 — `fn_task_done` completion verification iterates per repo, including the scope-leak guard (master KTD7)
|
||||
`fn_task_done` (`createTaskDoneTool` executor.ts **:10832**) must, in workspace mode: (a) call the per-repo `verifyWorktreeInvariants` (KTD2) for every acquired worktree; (b) iterate the **scope-leak guard** `evaluateTaskDoneScopeLeak` (executor.ts **:10711**, invoked at **:11009**) per repo — it currently runs `captureUncommittedModifiedFiles(worktreePath)` + `captureModifiedFiles(worktreePath, task.baseCommitSha, …)` against the singular root and silently passes; per-repo iteration (cwd = sub-repo, `repo.baseCommitSha`) restores the uncommitted-in-scope block. Block completion on any dirty/misbound repo or uncommitted in-scope change, naming the repo.
|
||||
|
||||
> **Repo-prefix derivation helper** (shared, master U5 will reuse): canonicalize → match first path segment to a configured repo → `unscoped` fallback. New `packages/engine/src/workspace-paths.ts`. Keep it minimal — no lease logic (Phase C / master U7).
|
||||
|
||||
---
|
||||
|
||||
## Implementation Units
|
||||
|
||||
> **Standing requirements:** `FNXC:Workspace <yyyy-MM-dd-hh:mm>` comments; a `.changeset/*.md` (`@runfusion/fusion: minor`); FN-5048 (reuse the Phase-A `_workspace-fixture.ts` harness; real git only where the invariant requires it; fake timers; no mock-the-world); FN-5893 surface enumeration; the merge gate. Branch off Phase A (already checked out: `gsxdsm/workspace-phase-b`).
|
||||
|
||||
### U1. Per-repo capture, contamination, and worktree-invariant verification (master U3)
|
||||
|
||||
**Goal:** Change-capture, contamination, and `verifyWorktreeInvariants` cover every acquired sub-repo worktree with repo context and correct cwd.
|
||||
|
||||
**Requirements:** KTD1, KTD2.
|
||||
|
||||
**Dependencies:** none beyond Phase A.
|
||||
|
||||
**Files:**
|
||||
- `packages/engine/src/executor.ts` — **add** a workspace branch at the post-session capture **:7898** (+ attribution audit **:7914**) that loops `workspaceWorktrees` calling `captureModifiedFiles(repo.worktreePath, repo.baseCommitSha, …)` per repo, repo-prefixing results; **un-stub** `verifyWorktreeInvariants` **:10508** to iterate per worktree preserving the `{ok|reason|observed|expected}` union (+ `repo`).
|
||||
- `packages/engine/src/__tests__/executor-workspace-capture.test.ts` (new — real two-repo fixture via `_workspace-fixture.ts`)
|
||||
|
||||
**Approach:** Per KTD1/KTD2. Reuse `captureModifiedFiles` (do not hand-build `git diff`); do not iterate the no-op `assertCleanBranchAtBase`. Singular non-workspace path unchanged.
|
||||
|
||||
**Execution note:** Reuse `_workspace-fixture.ts`; commit edits onto each sub-repo's `fusion/<id>` branch to exercise real diffs + the divergence audit.
|
||||
|
||||
**Test scenarios:**
|
||||
- Edits in repo A and B → `task.modifiedFiles` carries repo-prefixed paths from both, each diffed against its own `baseCommitSha`. (happy path)
|
||||
- A repo with `baseCommitSha` undefined → capture still works via the merge-base fallback (no `git diff undefined..HEAD`). (edge — Phase A non-fatal base)
|
||||
- A foreign commit in a sub-repo's range → the `filterFilesToOwnTaskCommits` divergence/contamination audit fires for that repo. (contamination)
|
||||
- A worktree HEAD drifted off `fusion/<id>` → `verifyWorktreeInvariants` returns `{ok:false, reason:'wrong_branch', repo, observed, expected}` (not `{ok:true}`); the `reason` enum is preserved for the :10889 consumer. (error path)
|
||||
- Single-repo (non-workspace) task → capture/verify byte-for-byte identical. (regression)
|
||||
|
||||
**Verification:** Capture + contamination audit + invariant verify run per acquired worktree with repo context; the result union is intact; single-repo unchanged.
|
||||
|
||||
---
|
||||
|
||||
### U2. Per-repo review (both call sites) + `fn_task_done` completion + scope-leak verification (master U4)
|
||||
|
||||
**Goal:** Review every acquired sub-repo (both review entry points) and block completion until every sub-repo passes review, invariant, and scope-leak checks.
|
||||
|
||||
**Requirements:** KTD3, KTD4, KTD2.
|
||||
|
||||
**Dependencies:** U1 (per-repo verify + capture).
|
||||
|
||||
**Files:**
|
||||
- `packages/engine/src/executor.ts` — `createReviewStepTool` **:11148** and the step-inversion seam **:5668** loop `reviewStep` per acquired worktree; `createTaskDoneTool` **:10832** calls per-repo verify (U1) + iterates `evaluateTaskDoneScopeLeak` **:10711** per repo.
|
||||
- `packages/engine/src/reviewer.ts` — `reviewStep` (**:122**) stays single-cwd; callers loop. Aggregate repo-tagged verdicts (conjunction) into the task review record; reviewer findings carry the repo tag.
|
||||
- `packages/engine/src/workspace-paths.ts` (new — the repo-prefix-derivation helper; master U5 reuses)
|
||||
- `packages/engine/src/__tests__/reviewer-workspace.test.ts`, `packages/engine/src/__tests__/executor-workspace-taskdone.test.ts` (new)
|
||||
|
||||
**Approach:** Per KTD3/KTD4. Both review sites loop the existing single-cwd `reviewStep` once per sub-repo (N reviewer agents — accepted cost) and aggregate as a conjunction. `fn_task_done` per-repo verify + per-repo scope-leak.
|
||||
|
||||
**Test scenarios:**
|
||||
- Two-repo task → two reviewer passes (one per repo cwd); review record reflects both; reviewed only when both pass. (conjunction)
|
||||
- A reviewer finding in repo B is repo-tagged. (integration)
|
||||
- Step-inversion review seam (:5668) for a workspace task reviews each sub-repo, not the non-git root. (FN-5893 second surface)
|
||||
- `fn_task_done` with an uncommitted in-scope change in repo A → completion blocked, naming repo A (the scope-leak guard fires per-repo). (error path)
|
||||
- `fn_task_done` with a worktree off `fusion/<id>` → blocked via per-repo verify. (error path)
|
||||
- The prefix helper: `wolf-server/src/**` → repo `wolf-server`; non-matching first segment → `unscoped`. (helper)
|
||||
- Single-repo task → one review pass + singular scope-leak/verify, unchanged. (regression)
|
||||
|
||||
**Verification:** A workspace task is reviewed/complete only when every sub-repo passes review + invariant + scope-leak; both review entry points iterate; single-repo unchanged.
|
||||
|
||||
---
|
||||
|
||||
## Scope Boundaries
|
||||
|
||||
**In scope:** per-repo capture/contamination/verify (U1); per-repo review at both call sites + `fn_task_done` verify + scope-leak (U2); the repo-prefix helper.
|
||||
|
||||
### Deferred to Follow-Up Work (later phases)
|
||||
- The per-repo merge loop, the landed predicate, the file-scope leases (master U5/U6/U7 = Phase C).
|
||||
- Self-healing reconcilers, e2e (master U8/U9 = Phase D).
|
||||
- Per-repo worktree teardown (carried Phase-A residual).
|
||||
- Store-level **atomic** per-repo `workspaceWorktrees` merge — Phase A added a re-read mitigation; the fully-atomic merge is still open and **becomes reachable in Phase B** (multi-repo acquisition first exercised here). Track for Phase C.
|
||||
|
||||
---
|
||||
|
||||
## Risks & Dependencies
|
||||
|
||||
- **R1 — "Add a branch" vs "replace a stub" confusion.** Capture/contamination/scope-leak silently degrade (not gated); an implementer expecting a stub to replace won't find one. Mitigation: KTD1/KTD4 + U1/U2 cite the exact add sites (:7898/:7914, :10711) and the one real stub (:10508).
|
||||
- **R2 — Hand-built `git diff` breaks on undefined base.** Mitigation: KTD1 mandates reusing `captureModifiedFiles`; test covers the undefined-base repo.
|
||||
- **R3 — `verifyWorktreeInvariants` union shape.** The `reason` enum is load-bearing at :10889. Mitigation: KTD2 preserves the union; test asserts the `reason`.
|
||||
- **R4 — No-op contamination function.** Mitigation: KTD1 explicitly forbids iterating `assertCleanBranchAtBase`; contamination rides on per-repo `captureModifiedFiles`.
|
||||
- **R5 — N× reviewer cost.** Accepted (user decision). Mitigation: note in the PR; cost scales with repo count (typically 2-3).
|
||||
- **Stacking dependency:** off Phase A (#1713); diff includes the stack.
|
||||
|
||||
---
|
||||
|
||||
## Sources & Research
|
||||
|
||||
- Master plan (U3/U4, KTD7, contamination-window caveat).
|
||||
- Phase B feasibility pre-check (verified anchors: capture not gated/:7898 add-site, `assertCleanBranchAtBase` no-op, undefined-base via `resolveDiffBaseRef`, verify union :10508/:10889, review agent N× cost + the :5668 second surface, scope-leak :10711, anchor corrections).
|
||||
- Phase A (#1713): per-repo `baseCommitSha`, `activeWorktrees` Set, `_workspace-fixture.ts`.
|
||||
- `docs/solutions/logic-errors/files-changed-inflated-by-origin-first-base-commit.md`.
|
||||
244
packages/engine/src/__tests__/executor-workspace-capture.test.ts
Normal file
244
packages/engine/src/__tests__/executor-workspace-capture.test.ts
Normal file
@@ -0,0 +1,244 @@
|
||||
/*
|
||||
FNXC:Workspace 2026-06-21-23:30:
|
||||
U1 per-repo capture + contamination + worktree-invariant tests (KTD1/KTD2). These drive the REAL TaskExecutor methods against a REAL two-repo git fixture under a NON-git workspace root (createWorkspaceFixture), so any leaked rootDir git preflight would actually fail and a hand-built `git diff` against an undefined base would blow up.
|
||||
|
||||
Seam choice (FN-5048): we set `(executor as any).workspaceConfig` directly (loadWorkspaceConfig has its own unit) and create real `fusion/<id>` worktrees per sub-repo with real commits — no mock-the-world child_process. Capture is exercised through `captureWorkspaceModifiedFiles` (the helper the post-session path at executor.ts:7900 calls) and verification through `verifyWorktreeInvariants`. Real git is used only where the invariant requires it.
|
||||
|
||||
Coverage:
|
||||
- happy: edits in repo A + B → aggregated modifiedFiles carry repo-prefixed paths from BOTH, each diffed against its own baseCommitSha.
|
||||
- edge: a repo with baseCommitSha undefined → capture still works via resolveDiffBaseRef's merge-base fallback (no `git diff undefined..HEAD`).
|
||||
- contamination: a foreign commit (feat(FN-OTHER):) in a sub-repo's range → the filterFilesToOwnTaskCommits divergence audit fires (task:worktree-contamination-detected) for that repo, and the foreign file is excluded from attributed files.
|
||||
- error: a worktree HEAD off fusion/<id> → verifyWorktreeInvariants returns {ok:false, reason:'wrong_branch', repo, observed, expected} (NOT {ok:true}); the reason enum is preserved for the :10889 consumer.
|
||||
- regression: a single-repo (non-workspace) task → capture/verify identical to today.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { execSync } from "node:child_process";
|
||||
import { mkdirSync, writeFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { Task, TaskStore, WorkspaceConfig } from "@fusion/core";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js";
|
||||
|
||||
const describeIfGit = hasGit ? describe : describe.skip;
|
||||
|
||||
function createStore(overrides: Partial<Record<string, unknown>> = {}): 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 }),
|
||||
getRunContextFor: vi.fn(),
|
||||
on: emitter.on.bind(emitter),
|
||||
...overrides,
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
function makeTask(id = "FN-WS-1", overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id,
|
||||
title: "Workspace task",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
// Capture attribution requires a digit-form task id (`FN-\d+`); the branch-attribution
|
||||
// subject parser only attributes `feat(FN-1001):` style subjects, so the KTD2-era
|
||||
// `FN-WS-1` placeholder would never attribute a commit. Use a real numeric id here.
|
||||
const TASK_ID = "FN-1001";
|
||||
const BRANCH = "fusion/fn-1001";
|
||||
|
||||
/** Configure git identity in a freshly-created worktree (worktrees don't inherit user.* on all platforms). */
|
||||
function configureIdentity(dir: string): void {
|
||||
execSync('git config user.email "test@example.com"', { cwd: dir, stdio: "pipe" });
|
||||
execSync('git config user.name "Test"', { cwd: dir, stdio: "pipe" });
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a real fusion/<id> worktree to a sub-repo, commit one own-attributed edit
|
||||
* onto that branch, and return { worktreePath, baseCommitSha } for task.workspaceWorktrees.
|
||||
* baseCommitSha is the sub-repo's pre-edit HEAD so the diff range is base..HEAD.
|
||||
*/
|
||||
function addRepoWorktreeWithOwnEdit(
|
||||
fx: WorkspaceFixture,
|
||||
repoRel: string,
|
||||
fileName: string,
|
||||
): { worktreePath: string; baseCommitSha: string } {
|
||||
const repoDir = fx.repoPath(repoRel);
|
||||
const baseCommitSha = fx.git(repoRel, "git rev-parse HEAD");
|
||||
const worktreePath = path.join(repoDir, ".worktrees", "fn-ws-1");
|
||||
fx.git(repoRel, `git worktree add -b ${BRANCH} ${worktreePath} HEAD`);
|
||||
configureIdentity(worktreePath);
|
||||
mkdirSync(path.dirname(path.join(worktreePath, fileName)), { recursive: true });
|
||||
writeFileSync(path.join(worktreePath, fileName), "// own change\n", "utf-8");
|
||||
execSync(`git add ${fileName}`, { cwd: worktreePath, stdio: "pipe" });
|
||||
execSync(`git commit -m "feat(${TASK_ID}): edit ${fileName}"`, { cwd: worktreePath, stdio: "pipe" });
|
||||
return { worktreePath, baseCommitSha };
|
||||
}
|
||||
|
||||
function workspaceExecutor(fx: WorkspaceFixture, store = createStore()): TaskExecutor {
|
||||
const executor = new TaskExecutor(store, fx.rootDir);
|
||||
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
||||
return executor;
|
||||
}
|
||||
|
||||
describeIfGit("U1 KTD1 — per-repo capture aggregates repo-prefixed paths", () => {
|
||||
let fx: WorkspaceFixture;
|
||||
afterEach(() => fx?.cleanup());
|
||||
|
||||
it("happy: edits in repo A + B are diffed against their own base and repo-prefixed", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktreeWithOwnEdit(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktreeWithOwnEdit(fx, "repo-b", "src/b.ts");
|
||||
const executor = workspaceExecutor(fx);
|
||||
const task = makeTask(TASK_ID, {
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const files = await (executor as any).captureWorkspaceModifiedFiles(task);
|
||||
expect(files).toContain("repo-a/src/a.ts");
|
||||
expect(files).toContain("repo-b/src/b.ts");
|
||||
expect(files).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("edge: a repo with undefined baseCommitSha still captures via merge-base fallback (no `git diff undefined..HEAD`)", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktreeWithOwnEdit(fx, "repo-a", "src/a.ts");
|
||||
const executor = workspaceExecutor(fx);
|
||||
const task = makeTask(TASK_ID, {
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
// baseCommitSha intentionally undefined → resolveDiffBaseRef merge-base(HEAD, main).
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH },
|
||||
},
|
||||
});
|
||||
|
||||
const files = await (executor as any).captureWorkspaceModifiedFiles(task);
|
||||
expect(files).toEqual(["repo-a/src/a.ts"]);
|
||||
});
|
||||
|
||||
it("contamination: a foreign commit in a sub-repo range fires the divergence audit and is excluded from attributed files", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktreeWithOwnEdit(fx, "repo-a", "src/a.ts");
|
||||
// Land a FOREIGN commit (different FN-id) onto the same fusion/<id> branch range.
|
||||
const foreignFile = "src/foreign.ts";
|
||||
writeFileSync(path.join(a.worktreePath, "src", "foreign.ts"), "// foreign\n", "utf-8");
|
||||
execSync(`git add ${foreignFile}`, { cwd: a.worktreePath, stdio: "pipe" });
|
||||
execSync('git commit -m "feat(FN-OTHER): sneaky foreign change"', { cwd: a.worktreePath, stdio: "pipe" });
|
||||
|
||||
const dbAudit = vi.fn().mockResolvedValue(undefined);
|
||||
const audit = {
|
||||
database: dbAudit,
|
||||
filesystem: vi.fn().mockResolvedValue(undefined),
|
||||
git: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const executor = workspaceExecutor(fx);
|
||||
const task = makeTask(TASK_ID, {
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const files = await (executor as any).captureWorkspaceModifiedFiles(task, audit as any, "post-session");
|
||||
// Own file attributed, foreign file excluded from the attributed set.
|
||||
expect(files).toEqual(["repo-a/src/a.ts"]);
|
||||
expect(files).not.toContain("repo-a/src/foreign.ts");
|
||||
// The contamination/divergence audit fired for this repo (raw 2 files vs attributed 1).
|
||||
const contaminationCall = dbAudit.mock.calls.find(
|
||||
([evt]) => evt?.type === "task:worktree-contamination-detected",
|
||||
);
|
||||
expect(contaminationCall).toBeTruthy();
|
||||
expect(contaminationCall![0].metadata.rawDiffFileCount).toBeGreaterThan(contaminationCall![0].metadata.attributedFileCount);
|
||||
});
|
||||
});
|
||||
|
||||
describeIfGit("U1 KTD2 — verifyWorktreeInvariants iterates per worktree, preserving the result union", () => {
|
||||
let fx: WorkspaceFixture;
|
||||
afterEach(() => fx?.cleanup());
|
||||
|
||||
it("happy: every worktree on fusion/<id> with matching toplevel → {ok:true}", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktreeWithOwnEdit(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktreeWithOwnEdit(fx, "repo-b", "src/b.ts");
|
||||
const executor = workspaceExecutor(fx);
|
||||
const task = makeTask(TASK_ID, {
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).verifyWorktreeInvariants(task);
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
it("error: a worktree HEAD off fusion/<id> → {ok:false, reason:'wrong_branch', repo, observed, expected} (NOT {ok:true})", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktreeWithOwnEdit(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktreeWithOwnEdit(fx, "repo-b", "src/b.ts");
|
||||
// Drift repo-b's worktree off fusion/<id> onto a different branch.
|
||||
execSync("git checkout -b some-other-branch", { cwd: b.worktreePath, stdio: "pipe" });
|
||||
const executor = workspaceExecutor(fx);
|
||||
const task = makeTask(TASK_ID, {
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).verifyWorktreeInvariants(task);
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.reason).toBe("wrong_branch");
|
||||
expect(result.repo).toBe("repo-b");
|
||||
expect(result.observed).toBe("some-other-branch");
|
||||
expect(result.expected).toBe(BRANCH);
|
||||
});
|
||||
|
||||
it("regression: a zero-acquire workspace task (empty map) verifies vacuously → {ok:true}", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const executor = workspaceExecutor(fx);
|
||||
const task = makeTask(TASK_ID, { branch: BRANCH, workspaceWorktrees: {} });
|
||||
const result = await (executor as any).verifyWorktreeInvariants(task);
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
});
|
||||
|
||||
describeIfGit("U1 — single-repo (non-workspace) task: capture/verify unchanged", () => {
|
||||
let fx: WorkspaceFixture;
|
||||
afterEach(() => fx?.cleanup());
|
||||
|
||||
it("regression: non-workspace verifyWorktreeInvariants still runs the singular path and passes for a real worktree", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
// Single-repo executor rooted at repo-a itself (no workspaceConfig).
|
||||
const repoDir = fx.repoPath("repo-a");
|
||||
const worktreePath = path.join(repoDir, ".worktrees", "fn-001");
|
||||
const base = execSync("git rev-parse HEAD", { cwd: repoDir, encoding: "utf-8" }).trim();
|
||||
execSync(`git worktree add -b fusion/fn-001 ${worktreePath} HEAD`, { cwd: repoDir, stdio: "pipe" });
|
||||
configureIdentity(worktreePath);
|
||||
writeFileSync(path.join(worktreePath, "single.ts"), "// x\n", "utf-8");
|
||||
execSync("git add single.ts", { cwd: worktreePath, stdio: "pipe" });
|
||||
execSync('git commit -m "feat(FN-001): single"', { cwd: worktreePath, stdio: "pipe" });
|
||||
|
||||
const store = createStore();
|
||||
const executor = new TaskExecutor(store, repoDir); // no workspaceConfig → singular path
|
||||
const task = makeTask("FN-001", { branch: "fusion/fn-001", worktree: worktreePath, baseCommitSha: base });
|
||||
|
||||
const result = await (executor as any).verifyWorktreeInvariants(task);
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,291 @@
|
||||
/*
|
||||
FNXC:Workspace 2026-06-22-00:30:
|
||||
U2 KTD4 — per-repo fn_task_done completion verification: per-repo scope-leak guard + per-repo worktree-invariant
|
||||
verify. These drive the REAL TaskExecutor methods against a REAL two-repo git fixture under a NON-git workspace
|
||||
root (createWorkspaceFixture), so a leaked singular-root capture/verify would silently pass and the test would
|
||||
catch it. Narrow seams (FN-5048): we set `(executor as any).workspaceConfig` directly and stub only the store
|
||||
methods the guards read (parseFileScopeFromPrompt, logEntry, getRunContextFor) — no mock-the-world child_process.
|
||||
|
||||
Coverage:
|
||||
- scope-leak error: an uncommitted in-scope vs OFF-scope change in repo A → evaluateTaskDoneScopeLeak blocks,
|
||||
message NAMES repo-a (per-repo guard fires; singular root would silently pass).
|
||||
- verify error: a worktree HEAD off fusion/<id> → verifyWorktreeInvariants blocks (wrong_branch, repo-tagged).
|
||||
- all-clean: a two-repo task with only in-scope changes → scope-leak does NOT block.
|
||||
- helper: deriveRepoForPath / deriveRepoScopeSubset / splitRepoScopedPath unit cases (wolf-server/src/** → wolf-server;
|
||||
non-matching first segment → unscoped).
|
||||
- regression: single-repo (non-workspace) task → singular scope-leak path unchanged.
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { execSync } from "node:child_process";
|
||||
import { mkdirSync, writeFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { Task, TaskStore, WorkspaceConfig, Settings } from "@fusion/core";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import {
|
||||
deriveRepoForPath,
|
||||
deriveRepoScopeSubset,
|
||||
splitRepoScopedPath,
|
||||
UNSCOPED_REPO,
|
||||
} from "../workspace-paths.js";
|
||||
import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js";
|
||||
|
||||
const describeIfGit = hasGit ? describe : describe.skip;
|
||||
|
||||
const TASK_ID = "FN-1001";
|
||||
const BRANCH = "fusion/fn-1001";
|
||||
|
||||
// reviewLevel=1 + block enforcement is the only mode that BLOCKS (else warn).
|
||||
const SETTINGS: Settings = { autoMerge: false, planOnlyScopeLeakEnforcement: "block" } as Settings;
|
||||
const PROMPT = "## Review Level: 1 (Plan Only)\n";
|
||||
|
||||
function createStore(declaredScope: string[]): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
return Object.assign(emitter, {
|
||||
parseFileScopeFromPrompt: vi.fn().mockResolvedValue(declaredScope),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getRunContextFor: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue(SETTINGS),
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
function makeTask(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: TASK_ID,
|
||||
title: "WS",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function configureIdentity(dir: string): void {
|
||||
execSync('git config user.email "test@example.com"', { cwd: dir, stdio: "pipe" });
|
||||
execSync('git config user.name "Test"', { cwd: dir, stdio: "pipe" });
|
||||
}
|
||||
|
||||
/** Add a fusion/<id> worktree to a sub-repo with one committed in-scope edit; return its handle. */
|
||||
function addRepoWorktree(fx: WorkspaceFixture, repoRel: string, fileName: string): { worktreePath: string; baseCommitSha: string } {
|
||||
const repoDir = fx.repoPath(repoRel);
|
||||
const baseCommitSha = fx.git(repoRel, "git rev-parse HEAD");
|
||||
const worktreePath = path.join(repoDir, ".worktrees", "fn-ws-1");
|
||||
fx.git(repoRel, `git worktree add -b ${BRANCH} ${worktreePath} HEAD`);
|
||||
configureIdentity(worktreePath);
|
||||
mkdirSync(path.dirname(path.join(worktreePath, fileName)), { recursive: true });
|
||||
writeFileSync(path.join(worktreePath, fileName), "// in-scope\n", "utf-8");
|
||||
execSync(`git add ${fileName}`, { cwd: worktreePath, stdio: "pipe" });
|
||||
execSync(`git commit -m "feat(${TASK_ID}): edit ${fileName}"`, { cwd: worktreePath, stdio: "pipe" });
|
||||
return { worktreePath, baseCommitSha };
|
||||
}
|
||||
|
||||
function workspaceExecutor(fx: WorkspaceFixture, store: TaskStore & EventEmitter): TaskExecutor {
|
||||
const executor = new TaskExecutor(store, fx.rootDir);
|
||||
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
||||
return executor;
|
||||
}
|
||||
|
||||
describe("U2 — workspace-paths repo-prefix helper (unit)", () => {
|
||||
const repos = ["wolf-server", "repo-a", "apps/web"];
|
||||
it("deriveRepoForPath: first-segment match → that repo", () => {
|
||||
expect(deriveRepoForPath("wolf-server/src/index.ts", repos)).toBe("wolf-server");
|
||||
expect(deriveRepoForPath("repo-a/src/a.ts", repos)).toBe("repo-a");
|
||||
});
|
||||
it("deriveRepoForPath: longest nested-key match wins", () => {
|
||||
expect(deriveRepoForPath("apps/web/page.tsx", repos)).toBe("apps/web");
|
||||
});
|
||||
it("deriveRepoForPath: non-matching first segment → unscoped", () => {
|
||||
expect(deriveRepoForPath(".changeset/x.md", repos)).toBe(UNSCOPED_REPO);
|
||||
expect(deriveRepoForPath("other/thing.ts", repos)).toBe(UNSCOPED_REPO);
|
||||
expect(deriveRepoForPath("repo-ab/x.ts", repos)).toBe(UNSCOPED_REPO); // segment-wise, not substring
|
||||
});
|
||||
it("splitRepoScopedPath: strips the repo prefix for the repo-local remainder", () => {
|
||||
expect(splitRepoScopedPath("wolf-server/src/x.ts", repos)).toEqual({ repo: "wolf-server", relativePath: "src/x.ts" });
|
||||
expect(splitRepoScopedPath("other/x.ts", repos)).toEqual({ repo: UNSCOPED_REPO, relativePath: "other/x.ts" });
|
||||
});
|
||||
it("deriveRepoScopeSubset: returns repo-local scope patterns for one repo", () => {
|
||||
const scope = ["wolf-server/src/**", "repo-a/lib/x.ts", "apps/web/page.tsx"];
|
||||
expect(deriveRepoScopeSubset(scope, "wolf-server")).toEqual(["src/**"]);
|
||||
expect(deriveRepoScopeSubset(scope, "repo-a")).toEqual(["lib/x.ts"]);
|
||||
// repo-root scope entry maps to whole-repo **
|
||||
expect(deriveRepoScopeSubset(["repo-a"], "repo-a")).toEqual(["**"]);
|
||||
});
|
||||
});
|
||||
|
||||
describeIfGit("U2 KTD4 — per-repo scope-leak guard in fn_task_done", () => {
|
||||
let fx: WorkspaceFixture;
|
||||
afterEach(() => fx?.cleanup());
|
||||
|
||||
it("error: an off-scope change in repo A blocks completion and NAMES repo-a", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
|
||||
// Off-scope STAGED-but-uncommitted change in repo-a (outside declared `repo-a/src/**`).
|
||||
// captureUncommittedModifiedFiles reads `git diff`/`--cached`, so the leak must be tracked
|
||||
// (staged) to register — an untracked file is invisible to the guard by design.
|
||||
writeFileSync(path.join(a.worktreePath, "OFFSCOPE.md"), "// leak\n", "utf-8");
|
||||
execSync("git add OFFSCOPE.md", { cwd: a.worktreePath, stdio: "pipe" });
|
||||
// Declared scope is repo-prefixed and only covers src/** in each repo.
|
||||
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
|
||||
const executor = workspaceExecutor(fx, store);
|
||||
const task = makeTask({
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result.message).toContain("repo-a");
|
||||
expect(result.message).toContain("OFFSCOPE.md");
|
||||
});
|
||||
|
||||
it("all-clean: only in-scope changes in both repos → not blocked", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
|
||||
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
|
||||
const executor = workspaceExecutor(fx, store);
|
||||
const task = makeTask({
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
|
||||
expect(result.blocked).toBe(false);
|
||||
});
|
||||
|
||||
// FNXC:Workspace 2026-06-21-15:00: F5 — per-repo `.changeset/` carve-out honored in workspace mode.
|
||||
// A legit sub-repo changeset (`repo-a/.changeset/x.md`) must NOT be flagged off-scope: the always-allowed
|
||||
// filter now runs against the repo-LOCAL remainder (`.changeset/x.md`), so the carve-out matches. Before
|
||||
// the fix the file was prefixed BEFORE filtering, the `.changeset/` startsWith never matched, and
|
||||
// fn_task_done was wrongly REFUSED.
|
||||
it("F5: a sub-repo `.changeset/` file is NOT flagged off-scope (always-allowed honored)", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
|
||||
// A per-repo changeset OUTSIDE the declared `repo-a/src/**` scope — only the always-allowed
|
||||
// carve-out can keep this from being a leak.
|
||||
mkdirSync(path.join(a.worktreePath, ".changeset"), { recursive: true });
|
||||
writeFileSync(path.join(a.worktreePath, ".changeset", "tidy-foo.md"), "---\n'@x': patch\n---\n", "utf-8");
|
||||
execSync("git add .changeset/tidy-foo.md", { cwd: a.worktreePath, stdio: "pipe" });
|
||||
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
|
||||
const executor = workspaceExecutor(fx, store);
|
||||
const task = makeTask({
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
|
||||
expect(result.blocked).toBe(false);
|
||||
});
|
||||
|
||||
// FNXC:Workspace 2026-06-21-15:00: F2 — scoped task that acquired ZERO sub-repo worktrees is blocked.
|
||||
// declaredScope is non-empty but `workspaceWorktrees` is empty → scope cannot be verified at all. The
|
||||
// guard must refuse fn_task_done rather than silently aggregating zero off-scope files and passing.
|
||||
it("F2: scoped task with zero acquired worktrees → blocked (cannot verify scope)", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const store = createStore(["repo-a/src/**"]);
|
||||
const executor = workspaceExecutor(fx, store);
|
||||
const task = makeTask({ branch: BRANCH, workspaceWorktrees: {} });
|
||||
|
||||
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result.message).toContain("acquired no sub-repo worktrees");
|
||||
});
|
||||
|
||||
// FNXC:Workspace 2026-06-21-15:00: F1 — fail CLOSED on a mid-loop capture throw.
|
||||
// If one repo's capture throws (scope is UNVERIFIED for that repo), the guard must BLOCK naming the
|
||||
// repo — not let the outer `.catch()` fail open and proceed with an incomplete scope check.
|
||||
it("F1: a mid-loop capture throw → blocked (fail-closed), names the repo", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
|
||||
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
|
||||
const executor = workspaceExecutor(fx, store);
|
||||
// Narrow seam: force the per-repo uncommitted capture to throw for repo-a's worktree only.
|
||||
const realCapture = (executor as any).captureUncommittedModifiedFiles.bind(executor);
|
||||
vi.spyOn(executor as any, "captureUncommittedModifiedFiles").mockImplementation(async (wt: unknown) => {
|
||||
if (wt === a.worktreePath) throw new Error("simulated capture failure");
|
||||
return realCapture(wt as string);
|
||||
});
|
||||
const task = makeTask({
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result.message).toContain("repo-a");
|
||||
expect(result.message).toContain("refusing fn_task_done");
|
||||
});
|
||||
});
|
||||
|
||||
describeIfGit("U2 KTD4 — per-repo worktree-invariant verify in fn_task_done", () => {
|
||||
let fx: WorkspaceFixture;
|
||||
afterEach(() => fx?.cleanup());
|
||||
|
||||
it("error: a worktree off fusion/<id> blocks completion via per-repo verify", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
|
||||
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
|
||||
execSync("git checkout -b drifted-branch", { cwd: b.worktreePath, stdio: "pipe" });
|
||||
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
|
||||
const executor = workspaceExecutor(fx, store);
|
||||
const task = makeTask({
|
||||
branch: BRANCH,
|
||||
workspaceWorktrees: {
|
||||
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
|
||||
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
|
||||
},
|
||||
});
|
||||
|
||||
const result = await (executor as any).verifyWorktreeInvariants(task);
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.reason).toBe("wrong_branch");
|
||||
expect(result.repo).toBe("repo-b");
|
||||
});
|
||||
});
|
||||
|
||||
describeIfGit("U2 — single-repo (non-workspace) task: scope-leak unchanged", () => {
|
||||
let fx: WorkspaceFixture;
|
||||
afterEach(() => fx?.cleanup());
|
||||
|
||||
it("regression: singular scope-leak path still flags an off-scope change in the singular worktree", async () => {
|
||||
fx = await createWorkspaceFixture();
|
||||
const repoDir = fx.repoPath("repo-a");
|
||||
const worktreePath = path.join(repoDir, ".worktrees", "fn-001");
|
||||
const base = execSync("git rev-parse HEAD", { cwd: repoDir, encoding: "utf-8" }).trim();
|
||||
execSync(`git worktree add -b fusion/fn-001 ${worktreePath} HEAD`, { cwd: repoDir, stdio: "pipe" });
|
||||
configureIdentity(worktreePath);
|
||||
// Off-scope STAGED change (declared scope is `src/**`). Tracked so the guard sees it.
|
||||
writeFileSync(path.join(worktreePath, "OFFSCOPE.md"), "// leak\n", "utf-8");
|
||||
execSync("git add OFFSCOPE.md", { cwd: worktreePath, stdio: "pipe" });
|
||||
|
||||
const store = createStore(["src/**"]);
|
||||
const executor = new TaskExecutor(store, repoDir); // no workspaceConfig → singular path
|
||||
const task = makeTask({ id: "FN-001", branch: "fusion/fn-001", worktree: worktreePath, baseCommitSha: base });
|
||||
|
||||
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, worktreePath, PROMPT, SETTINGS);
|
||||
expect(result.blocked).toBe(true);
|
||||
expect(result.message).toContain("OFFSCOPE.md");
|
||||
// Singular message carries no repo tag.
|
||||
expect(result.message).not.toContain("repo=");
|
||||
});
|
||||
});
|
||||
247
packages/engine/src/__tests__/reviewer-workspace.test.ts
Normal file
247
packages/engine/src/__tests__/reviewer-workspace.test.ts
Normal file
@@ -0,0 +1,247 @@
|
||||
/*
|
||||
FNXC:Workspace 2026-06-22-00:30:
|
||||
U2 KTD3 — per-repo review (BOTH call sites) + conjunction aggregation tests. The reviewer is an AGENT
|
||||
spawned with `cwd = worktree`; per-repo review means ONE reviewer agent per sub-repo with the CALLERS
|
||||
looping the single-cwd `reviewStep`. These tests assert the LOOP + aggregation, not the reviewer's content:
|
||||
`reviewStep` is mocked (the narrow AI seam — FN-5048: no mock-the-world, no real AI spawn) and we record
|
||||
the cwd of each call. Coverage:
|
||||
- conjunction: two-repo task → two reviewer passes (one per repo cwd); review record reflects both; reviewed
|
||||
only when BOTH pass; one repo REVISE → aggregate REVISE tagged with that repo.
|
||||
- finding tag: a finding in repo B is repo-tagged in the aggregated review body.
|
||||
- in-session seam (createReviewStepTool / fn_review_step): a workspace task reviews each sub-repo cwd, not the root.
|
||||
- step-inversion seam (createAuthoritativeWorkflowSeams().stepReview, executor.ts:5668): same — each sub-repo, not root.
|
||||
- regression: single-repo (non-workspace) task → exactly one reviewStep call at the singular worktree.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { ReviewResult } from "../reviewer.js";
|
||||
|
||||
// Narrow AI seam: only reviewStep (the agent boundary) is mocked. Everything else is the real executor.
|
||||
vi.mock("../reviewer.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../reviewer.js")>();
|
||||
return { ...actual, reviewStep: vi.fn() };
|
||||
});
|
||||
|
||||
import { reviewStep as mockedReviewStepFn } from "../reviewer.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { FOREACH_ACTIVE_CONTEXT_KEY } from "../workflow-node-handlers.js";
|
||||
import type { Task, TaskStore, WorkspaceConfig } from "@fusion/core";
|
||||
|
||||
const mockedReviewStep = vi.mocked(mockedReviewStepFn);
|
||||
|
||||
const ROOT = "/tmp/ws-root"; // NON-git workspace root — must never be a review cwd in workspace mode.
|
||||
const WT_A = "/tmp/ws-root/repo-a/.worktrees/fn-1";
|
||||
const WT_B = "/tmp/ws-root/repo-b/.worktrees/fn-1";
|
||||
|
||||
function makeStore(task: Task): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
return Object.assign(emitter, {
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
getSettings: vi.fn().mockResolvedValue({ autoMerge: false }),
|
||||
updateStep: vi.fn().mockResolvedValue(undefined),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getRunContextFor: vi.fn(),
|
||||
// mergeEffectiveSettings degrades to base on any resolver error; these reject → base used.
|
||||
getTaskWorkflowSelection: vi.fn().mockRejectedValue(new Error("no workflow")),
|
||||
getWorkflowDefinition: vi.fn().mockRejectedValue(new Error("no workflow")),
|
||||
getWorkflowSettingValues: vi.fn().mockRejectedValue(new Error("no workflow")),
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
function makeTask(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-1",
|
||||
title: "WS",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [
|
||||
{ name: "Step 0", status: "done" },
|
||||
{ name: "Step 1", status: "in-progress" },
|
||||
],
|
||||
currentStep: 1,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
const TWO_REPO_WORKTREES = {
|
||||
"repo-a": { worktreePath: WT_A, branch: "fusion/fn-1", baseCommitSha: "aaa" },
|
||||
"repo-b": { worktreePath: WT_B, branch: "fusion/fn-1", baseCommitSha: "bbb" },
|
||||
};
|
||||
|
||||
/** Script reviewStep to return a per-cwd verdict and record the cwd it was called with. */
|
||||
function scriptReviewByCwd(byCwd: Record<string, ReviewResult>): string[] {
|
||||
const seenCwds: string[] = [];
|
||||
mockedReviewStep.mockImplementation((async (cwd: string) => {
|
||||
seenCwds.push(cwd);
|
||||
return byCwd[cwd] ?? { verdict: "APPROVE", review: `ok ${cwd}`, summary: `ok ${cwd}` };
|
||||
}) as any);
|
||||
return seenCwds;
|
||||
}
|
||||
|
||||
function workspaceExecutor(store: TaskStore & EventEmitter): TaskExecutor {
|
||||
const executor = new TaskExecutor(store, ROOT);
|
||||
(executor as any).workspaceConfig = { repos: ["repo-a", "repo-b"] } as WorkspaceConfig;
|
||||
return executor;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockedReviewStep.mockReset();
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("U2 KTD3 — reviewWorkspacePerRepo conjunction + tagging (the shared loop both call sites use)", () => {
|
||||
// FNXC:Workspace 2026-06-21-15:00: F7 — the per-repo callback is single-arg `(cwd)` now; tests map
|
||||
// cwd→repo themselves (the loop no longer passes repoRel through to runForCwd).
|
||||
const repoOfCwd = (cwd: string): string => (cwd === WT_A ? "repo-a" : cwd === WT_B ? "repo-b" : cwd);
|
||||
|
||||
it("conjunction: two repos both APPROVE → aggregate APPROVE, one reviewer pass per repo cwd", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
|
||||
const executor = workspaceExecutor(makeStore(task));
|
||||
const seen: string[] = [];
|
||||
const result = await (executor as any).reviewWorkspacePerRepo(task, async (cwd: string) => {
|
||||
seen.push(cwd);
|
||||
return { verdict: "APPROVE", review: `clean in ${repoOfCwd(cwd)}`, summary: `clean ${repoOfCwd(cwd)}` };
|
||||
});
|
||||
expect(seen).toEqual([WT_A, WT_B]); // one pass per sub-repo cwd, never ROOT
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
expect(result.review).toContain("repo-a");
|
||||
expect(result.review).toContain("repo-b");
|
||||
});
|
||||
|
||||
it("conjunction: one repo REVISE → aggregate REVISE, tagged with the failing repo", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
|
||||
const executor = workspaceExecutor(makeStore(task));
|
||||
const result = await (executor as any).reviewWorkspacePerRepo(task, async (cwd: string) => {
|
||||
const repo = repoOfCwd(cwd);
|
||||
return repo === "repo-b"
|
||||
? { verdict: "REVISE", review: `bug in ${repo}`, summary: `revise ${repo}` }
|
||||
: { verdict: "APPROVE", review: `clean ${repo}`, summary: `clean ${repo}` };
|
||||
});
|
||||
expect(result.verdict).toBe("REVISE");
|
||||
expect(result.review).toContain("repo-b"); // finding repo-tagged
|
||||
expect(result.review).toContain("bug in repo-b");
|
||||
expect(result.summary).toMatch(/^repo-b:/);
|
||||
});
|
||||
|
||||
// FNXC:Workspace 2026-06-21-15:00: F3 — break on the FIRST non-APPROVE repo.
|
||||
it("F3: repo-a APPROVE + repo-b REVISE (no throw) → aggregate REVISE tagged repo-b", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
|
||||
const executor = workspaceExecutor(makeStore(task));
|
||||
const result = await (executor as any).reviewWorkspacePerRepo(task, async (cwd: string) => {
|
||||
const repo = repoOfCwd(cwd);
|
||||
return repo === "repo-a"
|
||||
? { verdict: "APPROVE", review: "clean repo-a", summary: "clean a" }
|
||||
: { verdict: "REVISE", review: "bug repo-b", summary: "revise b" };
|
||||
});
|
||||
expect(result.verdict).toBe("REVISE");
|
||||
expect(result.summary).toMatch(/^repo-b:/);
|
||||
});
|
||||
|
||||
it("F3: repo-a REVISE + repo-b throws → REVISE preserved (break before repo-b; NOT masked to UNAVAILABLE)", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
|
||||
const executor = workspaceExecutor(makeStore(task));
|
||||
const seen: string[] = [];
|
||||
const result = await (executor as any).reviewWorkspacePerRepo(task, async (cwd: string) => {
|
||||
seen.push(cwd);
|
||||
if (cwd === WT_B) throw new Error("repo-b reviewer blew up");
|
||||
return { verdict: "REVISE", review: "bug repo-a", summary: "revise a" };
|
||||
});
|
||||
// repo-a recorded the first non-APPROVE and the loop BROKE, so repo-b's reviewer is never invoked.
|
||||
expect(seen).toEqual([WT_A]);
|
||||
expect(result.verdict).toBe("REVISE");
|
||||
expect(result.summary).toMatch(/^repo-a:/);
|
||||
});
|
||||
|
||||
it("zero-acquire workspace task → UNAVAILABLE (caller routes; no fabricated APPROVE)", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: {} });
|
||||
const executor = workspaceExecutor(makeStore(task));
|
||||
const invoke = vi.fn();
|
||||
const result = await (executor as any).reviewWorkspacePerRepo(task, invoke);
|
||||
expect(result.verdict).toBe("UNAVAILABLE");
|
||||
expect(invoke).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("U2 KTD3 — in-session fn_review_step (createReviewStepTool) loops per sub-repo", () => {
|
||||
it("workspace task: code review spawns one reviewer per sub-repo cwd, not the root", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
|
||||
const store = makeStore(task);
|
||||
const executor = workspaceExecutor(store);
|
||||
const seen = scriptReviewByCwd({
|
||||
[WT_A]: { verdict: "APPROVE", review: "a ok", summary: "a" },
|
||||
[WT_B]: { verdict: "APPROVE", review: "b ok", summary: "b" },
|
||||
});
|
||||
const tool = (executor as any).createReviewStepTool(
|
||||
task.id,
|
||||
ROOT, // singular worktreePath = the non-git root; workspace mode must NOT review it
|
||||
"PROMPT",
|
||||
new Map(),
|
||||
{ current: null },
|
||||
new Map(),
|
||||
task,
|
||||
undefined,
|
||||
);
|
||||
const res = await tool.execute("call-1", { step: 1, type: "code", step_name: "Step 1", baseline: "base" });
|
||||
expect(seen).toEqual([WT_A, WT_B]);
|
||||
expect(seen).not.toContain(ROOT);
|
||||
// Aggregate APPROVE flows through the tool's verdict→text mapping unchanged.
|
||||
expect(res.content[0].text).toBe("APPROVE");
|
||||
});
|
||||
|
||||
it("regression: single-repo (non-workspace) task → exactly one reviewStep call at the singular worktree", async () => {
|
||||
const task = makeTask();
|
||||
const store = makeStore(task);
|
||||
const executor = new TaskExecutor(store, ROOT); // no workspaceConfig → singular path
|
||||
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "ok", summary: "ok" } });
|
||||
const tool = (executor as any).createReviewStepTool(
|
||||
task.id,
|
||||
WT_A,
|
||||
"PROMPT",
|
||||
new Map(),
|
||||
{ current: null },
|
||||
new Map(),
|
||||
task,
|
||||
undefined,
|
||||
);
|
||||
await tool.execute("call-1", { step: 1, type: "code", step_name: "Step 1", baseline: "base" });
|
||||
expect(seen).toEqual([WT_A]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per sub-repo", () => {
|
||||
it("workspace task: stepReview spawns one reviewer per sub-repo cwd, not active.worktreePath/root", async () => {
|
||||
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES, worktree: ROOT });
|
||||
const store = makeStore(task);
|
||||
const executor = workspaceExecutor(store);
|
||||
const seen = scriptReviewByCwd({
|
||||
[WT_A]: { verdict: "APPROVE", review: "a", summary: "a" },
|
||||
[WT_B]: { verdict: "APPROVE", review: "b", summary: "b" },
|
||||
});
|
||||
const seams = executor.createAuthoritativeWorkflowSeams({ autoMerge: false } as any);
|
||||
// Drive the foreach-active step-review handler directly with a scripted active context.
|
||||
const context = {
|
||||
[FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: ROOT, baselineSha: "base" },
|
||||
} as any;
|
||||
const result = await seams.stepReview!(task as any, context, { type: "code", advisory: true } as any);
|
||||
expect(seen).toEqual([WT_A, WT_B]);
|
||||
expect(seen).not.toContain(ROOT);
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
});
|
||||
|
||||
it("regression: single-repo stepReview reviews the active worktree once", async () => {
|
||||
const task = makeTask({ worktree: WT_A });
|
||||
const store = makeStore(task);
|
||||
const executor = new TaskExecutor(store, ROOT); // no workspaceConfig
|
||||
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "a", summary: "a" } });
|
||||
const seams = executor.createAuthoritativeWorkflowSeams({ autoMerge: false } as any);
|
||||
const context = { [FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: WT_A, baselineSha: "base" } } as any;
|
||||
await seams.stepReview!(task as any, context, { type: "code", advisory: true } as any);
|
||||
expect(seen).toEqual([WT_A]);
|
||||
});
|
||||
});
|
||||
@@ -66,7 +66,7 @@ import {
|
||||
VERIFICATION_LOG_MAX_CHARS,
|
||||
type VerificationResult,
|
||||
} from "./verification-utils.js";
|
||||
import { canonicalStepInstanceBranchName, generateWorktreeName, resolveTaskWorkingBranch } from "./worktree-names.js";
|
||||
import { canonicalFusionBranchName, canonicalStepInstanceBranchName, generateWorktreeName, resolveTaskWorkingBranch } from "./worktree-names.js";
|
||||
import { resolveTaskWorktreePath, resolveWorktreesDir } from "./worktree-paths.js";
|
||||
import { Type, type Static } from "@earendil-works/pi-ai";
|
||||
import { describeModel, promptWithFallback, compactSessionContext } from "./pi.js";
|
||||
@@ -77,12 +77,18 @@ import {
|
||||
resolveExecutorSessionModel,
|
||||
} from "./agent-session-helpers.js";
|
||||
import { buildSessionSkillContext } from "./session-skill-context.js";
|
||||
import { reviewStep, type ReviewVerdict } from "./reviewer.js";
|
||||
import { reviewStep, type ReviewVerdict, type ReviewResult } from "./reviewer.js";
|
||||
import { selectUserCommentsForAgentContext } from "./agent-user-comments.js";
|
||||
import { resolveSandboxBackend } from "./sandbox/index.js";
|
||||
import type { SandboxBackend } from "./sandbox/types.js";
|
||||
import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession } from "@earendil-works/pi-coding-agent";
|
||||
import { PRIORITY_EXECUTE, type AgentSemaphore } from "./concurrency.js";
|
||||
// FNXC:Workspace 2026-06-21-15:00: F5/F8 — wire in the previously dead workspace-path helpers.
|
||||
// `normalizeRepoRelPath` is the single shared scope-path normalizer (F8); `deriveRepoScopeSubset`
|
||||
// maps the task's repo-prefixed declared File Scope to a repo-LOCAL subset so the per-repo scope-leak
|
||||
// filter reuses the SAME always-allowed/scope-match surface as the non-workspace path (F5). One-way
|
||||
// executor→workspace-paths edge (workspace-paths imports nothing).
|
||||
import { deriveRepoScopeSubset, normalizeRepoRelPath } from "./workspace-paths.js";
|
||||
import { RemovalReason, classifyTaskWorktree, describeRegisteredWorktrees, detectNestedWorktreeRoot, getRegisteredWorktreePaths, isGitRepository, isInsideWorktreesDir, isRegisteredGitWorktree, removeWorktree, type WorktreePool } from "./worktree-pool.js";
|
||||
import { attemptBranchAutocorrect } from "./branch-autocorrect.js";
|
||||
import { ActiveSessionWorktreeRemovalError } from "./worktree-backend.js";
|
||||
@@ -596,13 +602,14 @@ export interface WorkflowRevisionFeedbackPartition {
|
||||
const WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS = 4_000;
|
||||
const WORKFLOW_FEEDBACK_PATH_REGEX = /`([^`\n]+)`|(?<![A-Za-z0-9_.-])((?:\.\.?\/)?(?:@?[A-Za-z0-9._-]+\/)+[A-Za-z0-9._-]+(?:\.[A-Za-z0-9._-]+)?)/g;
|
||||
|
||||
// FNXC:Workspace 2026-06-21-15:00: F8 — delegate to the single shared normalizer (workspace-paths.ts).
|
||||
// Was a near-duplicate that did NOT strip a leading slash and only collapsed a single trailing slash;
|
||||
// the shared `normalizeRepoRelPath` additionally strips leading slashes and collapses repeated trailing
|
||||
// slashes. For repo-relative inputs (the only inputs in practice) the result is unchanged; the extra
|
||||
// canonicalization only hardens absolute/trailing-slash edge cases so workspace and non-workspace scope
|
||||
// matching agree. Kept as a thin alias so existing call sites stay put.
|
||||
function normalizeWorkflowScopePath(pathValue: string): string {
|
||||
return pathValue
|
||||
.trim()
|
||||
.replace(/\\/g, "/")
|
||||
.replace(/^\.\//, "")
|
||||
.replace(/\/+/g, "/")
|
||||
.replace(/\/$/, "");
|
||||
return normalizeRepoRelPath(pathValue);
|
||||
}
|
||||
|
||||
function stripTrailingPathPunctuation(pathValue: string): string {
|
||||
@@ -5704,9 +5711,14 @@ export class TaskExecutor {
|
||||
const settings = await mergeEffectiveSettings(this.store, detail, await this.store.getSettings());
|
||||
|
||||
const sem = this.options.semaphore;
|
||||
const invokeReviewer = () =>
|
||||
// FNXC:Workspace 2026-06-22-00:30: KTD3 — step-inversion review seam loops per sub-repo.
|
||||
// `reviewStep` stays single-cwd; THIS CALLER loops. Single-cwd by default reviews
|
||||
// `worktreePath`; in workspace mode that is the browse-only non-git root, so we instead spawn
|
||||
// one reviewer per acquired sub-repo (cwd = repo.worktreePath) via reviewWorkspacePerRepo and
|
||||
// aggregate as a conjunction. `invokeReviewerForCwd` is the per-cwd reviewStep call both modes share.
|
||||
const invokeReviewerForCwd = (cwd: string) =>
|
||||
reviewStep(
|
||||
worktreePath,
|
||||
cwd,
|
||||
seamTask.id,
|
||||
stepIndex,
|
||||
stepName,
|
||||
@@ -5742,10 +5754,18 @@ export class TaskExecutor {
|
||||
onSessionEnded: (s) => this.unregisterSubagentSession(seamTask.id, s),
|
||||
},
|
||||
);
|
||||
const runForCwd = (cwd: string) => {
|
||||
const invoke = () => invokeReviewerForCwd(cwd);
|
||||
return sem ? sem.runNested(invoke) : invoke();
|
||||
};
|
||||
const invokeReviewer = () =>
|
||||
this.workspaceConfig
|
||||
? this.reviewWorkspacePerRepo(detail, (cwd) => runForCwd(cwd))
|
||||
: runForCwd(worktreePath);
|
||||
|
||||
let review: { verdict: ReviewVerdict; review: string; summary: string };
|
||||
try {
|
||||
review = sem ? await sem.runNested(invokeReviewer) : await invokeReviewer();
|
||||
review = await invokeReviewer();
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
reviewerLog.error(`${seamTask.id}: step-review failed: ${message}`);
|
||||
@@ -7986,6 +8006,47 @@ export class TaskExecutor {
|
||||
const allSuccess = results.every(r => r.success);
|
||||
if (allSuccess) {
|
||||
const updatedTask = await this.store.getTask(task.id);
|
||||
// FNXC:Workspace 2026-06-21-23:30: KTD1 — per-repo post-session capture.
|
||||
// The singular call below runs UNGATED with worktreePath = the browse-only non-git workspace root and silently returns [] (resolveDiffBaseRef swallows the git failure at the root). In workspace mode there is nothing to diff at the root; the real changes live in each acquired sub-repo worktree. So we ADD (not replace) a workspace branch that loops `task.workspaceWorktrees` and reuses the EXISTING captureModifiedFiles per repo — reusing it (rather than hand-building `git diff <base>..HEAD`) gives us the merge-base fallback for an undefined repo.baseCommitSha (resolveDiffBaseRef) AND restores the contamination/divergence audit (filterFilesToOwnTaskCommits) for free per repo. Returned files are repo-prefixed (e.g. `repo-a/src/foo.ts`) and aggregated into task.modifiedFiles.
|
||||
if (this.workspaceConfig) {
|
||||
const workspaceWorktrees = updatedTask.workspaceWorktrees ?? {};
|
||||
const aggregated = await this.captureWorkspaceModifiedFiles(updatedTask, audit, "post-session");
|
||||
for (const [repoRel, repo] of Object.entries(workspaceWorktrees)) {
|
||||
// Per-repo branch-attribution audit (cwd = sub-repo). Run against repo.worktreePath/repo.branch, NOT the non-git root (a root call would fail and surface nothing). The contamination signal already rides on captureWorkspaceModifiedFiles above; this is the supplementary commit-attribution surface (FN-5233 pattern).
|
||||
try {
|
||||
const attributionBase = await this.resolveContaminationBaseRef(repo.worktreePath);
|
||||
if (attributionBase && repo.branch) {
|
||||
const attribution = await reportBranchAttribution(repo.worktreePath, repo.branch, attributionBase, task.id);
|
||||
const hasAnomaly = attribution.foreign.length > 0 || attribution.unattributed.length > 0 || attribution.ownUntrailed.length > 0;
|
||||
if (hasAnomaly) {
|
||||
const summary = `branch-attribution anomalies on ${repoRel}@${repo.branch}: foreign=${attribution.foreign.length}, unattributed=${attribution.unattributed.length}, ownUntrailed=${attribution.ownUntrailed.length}, ownTrailed=${attribution.ownTrailed}`;
|
||||
executorLog.warn(`${task.id}: ${summary}`);
|
||||
await this.store.logEntry(task.id, `[branch-attribution] ${summary}`, undefined, this.getRunContextFor(task.id));
|
||||
await audit.git({
|
||||
type: "branch:attribution-anomaly",
|
||||
target: repo.branch,
|
||||
metadata: {
|
||||
taskId: task.id,
|
||||
repo: repoRel,
|
||||
baseSha: attributionBase,
|
||||
ownTrailed: attribution.ownTrailed,
|
||||
foreign: attribution.foreign,
|
||||
unattributed: attribution.unattributed,
|
||||
ownUntrailed: attribution.ownUntrailed,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
} catch (attributionErr: unknown) {
|
||||
executorLog.warn(`${task.id}: post-session per-repo branch-attribution audit failed for ${repoRel}: ${attributionErr instanceof Error ? attributionErr.message : String(attributionErr)}`);
|
||||
}
|
||||
}
|
||||
if (aggregated.length > 0) {
|
||||
await this.store.updateTask(task.id, { modifiedFiles: aggregated });
|
||||
executorLog.log(`${task.id}: captured ${aggregated.length} modified files across ${Object.keys(workspaceWorktrees).length} sub-repo(s)`);
|
||||
await audit.filesystem({ type: "file:capture-modified", target: task.id, metadata: { files: aggregated } });
|
||||
}
|
||||
} else {
|
||||
const modifiedFiles = await this.captureModifiedFiles(worktreePath, updatedTask.baseCommitSha, task.id, audit, "post-session");
|
||||
if (modifiedFiles.length > 0) {
|
||||
await this.store.updateTask(task.id, { modifiedFiles });
|
||||
@@ -8027,6 +8088,7 @@ export class TaskExecutor {
|
||||
} catch (attributionErr: unknown) {
|
||||
executorLog.warn(`${task.id}: post-session branch-attribution audit failed: ${attributionErr instanceof Error ? attributionErr.message : String(attributionErr)}`);
|
||||
}
|
||||
} // end !this.workspaceConfig singular capture (FNXC:Workspace KTD1)
|
||||
|
||||
this.scheduleCompletedTaskWatchdog(task.id, "step-session completion");
|
||||
if (await this.shouldDeferCompletionForGlobalPause(task.id, "before workflow steps after step-session completion")) {
|
||||
@@ -10631,10 +10693,152 @@ export class TaskExecutor {
|
||||
worktreePathOverride?: string,
|
||||
allowReanchor = true,
|
||||
options?: { noOpCompletion?: boolean; noOpCompletionReason?: string },
|
||||
): Promise<{ ok: true } | { ok: false; reason: "wrong_toplevel" | "wrong_branch" | "no_commits"; observed: string; expected: string }> {
|
||||
): Promise<{ ok: true } | { ok: false; reason: "wrong_toplevel" | "wrong_branch" | "no_commits"; observed: string; expected: string; repo?: 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".
|
||||
// FNXC:Workspace 2026-06-21-23:30: KTD2 — un-stubbed per-repo worktree-invariant verification.
|
||||
// Phase A returned a flat {ok:true} stub here (no root worktree to verify against the non-git root). Phase B iterates every `task.workspaceWorktrees` entry, asserting (a) the sub-repo worktree's git toplevel matches the recorded repo.worktreePath and (b) its HEAD is on the recorded `fusion/<id>` branch (repo.branch). The result union is PRESERVED EXACTLY — `{ok:true} | {ok:false; reason:'wrong_toplevel'|'wrong_branch'|'no_commits'; observed; expected}` — because the :10889 consumer switches on `reason` to drive requeue/handoff (:10894-10936). We ADD an optional `repo` field to the failure shape (purely additive; the consumer only reads reason/observed/expected) and return the FIRST failing repo. A zero-acquire workspace task (empty map) verifies vacuously → {ok:true}, matching Phase A so fn_task_done does not requeue it.
|
||||
if (this.workspaceConfig) {
|
||||
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
||||
// FNXC:Workspace 2026-06-22-00:00: KTD2 — resolve the SAME task-wide no-commit eligibility the singular path
|
||||
// uses (getNoCommitEligibilityReason / no-op-completion sentinel / prompt-derived), once, before the per-repo
|
||||
// loop. When eligible (Plan-Only, verified no-op, etc.) the per-repo no_commits guard below is skipped so an
|
||||
// intentionally commit-free workspace task is not blocked from completion.
|
||||
const workspacePromptContent = (task as Task & { prompt?: unknown }).prompt;
|
||||
const workspacePromptEligibility = evaluatePromptDerivedNoCommitEligibility(
|
||||
task,
|
||||
typeof workspacePromptContent === "string" ? workspacePromptContent : "",
|
||||
);
|
||||
const workspaceNoCommitEligibilityReason =
|
||||
getNoCommitEligibilityReason(task) ??
|
||||
(options?.noOpCompletion
|
||||
? options.noOpCompletionReason ?? "verified no-op/duplicate completion sentinel"
|
||||
: null) ??
|
||||
(workspacePromptEligibility.eligible
|
||||
? workspacePromptEligibility.reason ?? "prompt-derived no-commit eligibility"
|
||||
: null);
|
||||
if (workspaceNoCommitEligibilityReason) {
|
||||
executorLog.log(`${task.id}: workspace fn_task_done no_commits guard skipped (${workspaceNoCommitEligibilityReason})`);
|
||||
}
|
||||
// FNXC:Workspace 2026-06-21-15:00: F6 — iterate sorted repo keys so the FIRST failing repo
|
||||
// returned here is deterministic across runs/rehydrate (the value is surfaced to the operator).
|
||||
for (const repoRel of Object.keys(workspaceWorktrees).sort()) {
|
||||
const repo = workspaceWorktrees[repoRel];
|
||||
const expectedBranch = repo.branch || canonicalFusionBranchName(task.id);
|
||||
// Skip git checks if the worktree dir is gone (mirrors the singular FN-009 carve-out below): completion does not require a live worktree on disk.
|
||||
if (!existsSync(repo.worktreePath)) {
|
||||
executorLog.log(`${task.id}: workspace worktree for ${repoRel} not found at ${repo.worktreePath} — skipping git validation`);
|
||||
continue;
|
||||
}
|
||||
let expectedWorktreeRealpath: string;
|
||||
try {
|
||||
expectedWorktreeRealpath = canonicalizePath(repo.worktreePath);
|
||||
} catch (error) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "wrong_toplevel",
|
||||
repo: repoRel,
|
||||
observed: `unresolvable repo worktree (${repo.worktreePath}): ${error instanceof Error ? error.message : String(error)}`,
|
||||
expected: `resolvable worktree for ${repoRel}`,
|
||||
};
|
||||
}
|
||||
try {
|
||||
const { stdout } = await execAsync("git rev-parse --show-toplevel", {
|
||||
cwd: repo.worktreePath,
|
||||
encoding: "utf-8",
|
||||
timeout: 10_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
});
|
||||
const observedTopLevelRaw = stdout.trim();
|
||||
if (observedTopLevelRaw) {
|
||||
const observedTopLevel = canonicalizePath(observedTopLevelRaw);
|
||||
if (observedTopLevel !== expectedWorktreeRealpath) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "wrong_toplevel",
|
||||
repo: repoRel,
|
||||
observed: observedTopLevel,
|
||||
expected: expectedWorktreeRealpath,
|
||||
};
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "wrong_toplevel",
|
||||
repo: repoRel,
|
||||
observed: error instanceof Error ? error.message : String(error),
|
||||
expected: expectedWorktreeRealpath,
|
||||
};
|
||||
}
|
||||
try {
|
||||
const { stdout } = await execAsync("git rev-parse --abbrev-ref HEAD", {
|
||||
cwd: repo.worktreePath,
|
||||
encoding: "utf-8",
|
||||
timeout: 10_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
});
|
||||
const observedBranch = stdout.trim();
|
||||
if (observedBranch && observedBranch !== expectedBranch) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "wrong_branch",
|
||||
repo: repoRel,
|
||||
observed: observedBranch,
|
||||
expected: expectedBranch,
|
||||
};
|
||||
}
|
||||
} catch (error) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "wrong_branch",
|
||||
repo: repoRel,
|
||||
observed: error instanceof Error ? error.message : String(error),
|
||||
expected: expectedBranch,
|
||||
};
|
||||
}
|
||||
// FNXC:Workspace 2026-06-22-00:00: KTD2 — per-repo no_commits guard (parity with the singular path at :10821).
|
||||
// Phase B originally returned {ok:true} after the toplevel/branch checks, so a workspace task could call
|
||||
// fn_task_done having committed NOTHING in any sub-repo (scope-leak sees zero touched files, branch names match)
|
||||
// and still advance to in-review. Enforce the same `git rev-list --count <base>..HEAD > 0` invariant per repo,
|
||||
// gated by the SAME task-wide no-commit eligibility below so Plan-Only / no-op-sentinel tasks stay exempt.
|
||||
// The first sub-repo with zero commits fails with reason:'no_commits' (consumer-stable union).
|
||||
if (!workspaceNoCommitEligibilityReason) {
|
||||
const repoBaseRef = await this.resolveDiffBaseRef(repo.worktreePath, repo.baseCommitSha);
|
||||
if (repoBaseRef) {
|
||||
try {
|
||||
const { stdout } = await execAsync(`git rev-list --count ${repoBaseRef}..HEAD`, {
|
||||
cwd: repo.worktreePath,
|
||||
encoding: "utf-8",
|
||||
timeout: 10_000,
|
||||
maxBuffer: 1024 * 1024,
|
||||
});
|
||||
const trimmedCount = stdout.trim();
|
||||
if (trimmedCount) {
|
||||
const count = Number.parseInt(trimmedCount, 10);
|
||||
if (!Number.isFinite(count) || count <= 0) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "no_commits",
|
||||
repo: repoRel,
|
||||
observed: Number.isFinite(count) ? String(count) : trimmedCount,
|
||||
expected: "> 0",
|
||||
};
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: "no_commits",
|
||||
repo: repoRel,
|
||||
observed: error instanceof Error ? error.message : String(error),
|
||||
expected: `git rev-list --count ${repoBaseRef}..HEAD > 0`,
|
||||
};
|
||||
}
|
||||
} else {
|
||||
executorLog.warn(`${task.id}: unable to resolve diff base for ${repoRel} no_commits guard; skipping for this sub-repo`);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { ok: true };
|
||||
}
|
||||
const branchName = resolveTaskWorkingBranch(task);
|
||||
@@ -10865,24 +11069,107 @@ export class TaskExecutor {
|
||||
return { blocked: false };
|
||||
}
|
||||
|
||||
const [uncommittedTouchedFiles, branchCommittedFiles] = await Promise.all([
|
||||
this.captureUncommittedModifiedFiles(worktreePath),
|
||||
this.captureModifiedFiles(worktreePath, task.baseCommitSha, task.id, audit, "scope-leak-guard"),
|
||||
]);
|
||||
|
||||
const touchedFiles = [...new Set([...uncommittedTouchedFiles, ...branchCommittedFiles])];
|
||||
if (touchedFiles.length === 0) {
|
||||
return { blocked: false };
|
||||
// FNXC:Workspace 2026-06-22-00:30: KTD4 — per-repo scope-leak guard.
|
||||
// The singular capture below runs `captureUncommittedModifiedFiles` + `captureModifiedFiles`
|
||||
// against `worktreePath`. In workspace mode `worktreePath` is the browse-only non-git workspace
|
||||
// root, so both silently return [] (git failures swallowed) and the uncommitted-in-scope block
|
||||
// never fires — a workspace task could complete with off-scope changes in any sub-repo. So we
|
||||
// ITERATE every acquired sub-repo (cwd = repo.worktreePath, base = repo.baseCommitSha) and block
|
||||
// on the FIRST repo carrying off-scope changes — naming the repo. The task-level preamble above
|
||||
// (scopeOverride / declaredScope / enforcementMode) is shared and runs once. Return shape is
|
||||
// preserved: `{blocked:false} | {blocked:true; message}`.
|
||||
//
|
||||
// FNXC:Workspace 2026-06-21-15:00: F1/F2/F5/F6 hardening of the per-repo scope-leak guard.
|
||||
// F5 (false-block fix + dead-code wiring + single filter surface): we previously repo-prefixed each
|
||||
// touched file (`${repoRel}/${file}`) BEFORE filtering, so `isAlwaysAllowedScopeLeakPath`'s
|
||||
// `startsWith(".changeset/")` carve-out never matched a sub-repo changeset (`repo-a/.changeset/x.md`)
|
||||
// and a legit per-repo changeset was wrongly flagged off-scope → fn_task_done wrongly REFUSED. Now we
|
||||
// derive each repo's repo-LOCAL declared-scope subset (`deriveRepoScopeSubset`) and run the SAME
|
||||
// `workflowPathMatchesDeclaredScope` + `isAlwaysAllowedScopeLeakPath` filter the non-workspace path
|
||||
// uses against the repo-LOCAL touched file — one filter surface, not two. This wires in the formerly
|
||||
// dead `deriveRepoScopeSubset`/`splitRepoScopedPath` helpers.
|
||||
// F1 (fail CLOSED on throw): each repo iteration is wrapped in its own try/catch (like the
|
||||
// attribution-audit loop). A thrown capture/diff error in workspace mode surfaces as a BLOCK naming
|
||||
// the repo instead of bubbling to the outer `.catch()` that fails OPEN — an incomplete scope check
|
||||
// must never let fn_task_done proceed.
|
||||
// F2 (scoped-but-zero-acquire): a scoped task that acquired NO sub-repo worktrees aggregates zero
|
||||
// off-scope files and would silently pass; we block it (scope is declared but unverifiable).
|
||||
// F6 (deterministic ordering): iterate sorted repo keys so the reported offending repo is stable
|
||||
// across runs/rehydrate.
|
||||
let touchedFiles: string[];
|
||||
let offendingRepo: string | undefined;
|
||||
if (this.workspaceConfig) {
|
||||
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
||||
const repoKeys = Object.keys(workspaceWorktrees).sort();
|
||||
// F2: declaredScope is non-empty here (the `declaredScope.length === 0` early-return above
|
||||
// handled the unscoped case). A scoped task that acquired no sub-repo worktrees cannot have its
|
||||
// scope verified at all — refuse rather than silently passing scope enforcement.
|
||||
if (repoKeys.length === 0) {
|
||||
const message = "workspace task declares File Scope but acquired no sub-repo worktrees — cannot verify scope";
|
||||
executorLog.warn(`${task.id}: [scope-leak] ${message}`);
|
||||
await this.store.logEntry(task.id, `[scope-leak] ${message}`, undefined, this.getRunContextFor(task.id));
|
||||
return { blocked: true, message };
|
||||
}
|
||||
const aggregatedOffScope: string[] = [];
|
||||
for (const repoRel of repoKeys) {
|
||||
const repo = workspaceWorktrees[repoRel];
|
||||
try {
|
||||
const [repoUncommitted, repoCommitted] = await Promise.all([
|
||||
this.captureUncommittedModifiedFiles(repo.worktreePath),
|
||||
this.captureModifiedFiles(repo.worktreePath, repo.baseCommitSha, task.id, audit, "scope-leak-guard"),
|
||||
]);
|
||||
// Repo-LOCAL touched files (no `${repoRel}/` prefix) so the always-allowed `.changeset/`
|
||||
// carve-out and the scope match operate as the reviewer/cwd=repo sees them (F5).
|
||||
const repoTouched = [...new Set([...repoUncommitted, ...repoCommitted])];
|
||||
// Repo-LOCAL declared-scope subset for THIS repo (prefix stripped). Same filter as the
|
||||
// non-workspace branch below — one surface.
|
||||
const repoScopeSubset = deriveRepoScopeSubset(declaredScope, repoRel);
|
||||
const repoOffScope = repoTouched
|
||||
.filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, repoScopeSubset))
|
||||
.filter((filePath) => !isAlwaysAllowedScopeLeakPath(filePath))
|
||||
// Re-prefix the surviving off-scope files for the operator-facing message/attribution.
|
||||
.map((filePath) => `${repoRel}/${filePath}`);
|
||||
if (repoOffScope.length > 0) {
|
||||
// First offending repo wins (mirrors verifyWorktreeInvariants' first-failing-repo return).
|
||||
if (!offendingRepo) offendingRepo = repoRel;
|
||||
aggregatedOffScope.push(...repoOffScope);
|
||||
}
|
||||
} catch (repoErr: unknown) {
|
||||
// F1: fail CLOSED. A capture/diff throw means scope is UNVERIFIED for this repo; refuse
|
||||
// fn_task_done as a precaution rather than letting the outer `.catch()` fail open.
|
||||
const errMessage = repoErr instanceof Error ? repoErr.message : String(repoErr);
|
||||
const message = `workspace scope-leak guard failed to evaluate (${repoRel}/${errMessage}) — refusing fn_task_done as a precaution`;
|
||||
executorLog.warn(`${task.id}: [scope-leak] ${message}`);
|
||||
await this.store.logEntry(task.id, `[scope-leak] ${message}`, undefined, this.getRunContextFor(task.id));
|
||||
return { blocked: true, message };
|
||||
}
|
||||
}
|
||||
touchedFiles = aggregatedOffScope;
|
||||
if (touchedFiles.length === 0) {
|
||||
return { blocked: false };
|
||||
}
|
||||
} else {
|
||||
const [uncommittedTouchedFiles, branchCommittedFiles] = await Promise.all([
|
||||
this.captureUncommittedModifiedFiles(worktreePath),
|
||||
this.captureModifiedFiles(worktreePath, task.baseCommitSha, task.id, audit, "scope-leak-guard"),
|
||||
]);
|
||||
touchedFiles = [...new Set([...uncommittedTouchedFiles, ...branchCommittedFiles])];
|
||||
if (touchedFiles.length === 0) {
|
||||
return { blocked: false };
|
||||
}
|
||||
}
|
||||
|
||||
const offScopeFiles = touchedFiles
|
||||
.filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, declaredScope))
|
||||
// FN-4811 follow-up: by convention every task may add its own changeset entry
|
||||
// under `.changeset/`, so changeset files are always considered in-scope and
|
||||
// never flagged by the scope-leak guard. The file-scope invariant at squash and
|
||||
// the broader contamination guards still catch cross-task changeset leakage at
|
||||
// a higher signal-to-noise ratio than the per-execution scope-leak warning.
|
||||
.filter((filePath) => !isAlwaysAllowedScopeLeakPath(filePath));
|
||||
const offScopeFiles = (this.workspaceConfig
|
||||
// In workspace mode `touchedFiles` is already the off-scope set (filtered per repo above).
|
||||
? touchedFiles
|
||||
: touchedFiles
|
||||
.filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, declaredScope))
|
||||
// FN-4811 follow-up: by convention every task may add its own changeset entry
|
||||
// under `.changeset/`, so changeset files are always considered in-scope and
|
||||
// never flagged by the scope-leak guard. The file-scope invariant at squash and
|
||||
// the broader contamination guards still catch cross-task changeset leakage at
|
||||
// a higher signal-to-noise ratio than the per-execution scope-leak warning.
|
||||
.filter((filePath) => !isAlwaysAllowedScopeLeakPath(filePath)));
|
||||
if (offScopeFiles.length === 0) {
|
||||
return { blocked: false };
|
||||
}
|
||||
@@ -10897,14 +11184,16 @@ export class TaskExecutor {
|
||||
|
||||
const offScopePreview = renderListPreview(offScopeFiles);
|
||||
const declaredScopePreview = renderListPreview(declaredScope);
|
||||
const message = `[scope-leak] reviewLevel=${reviewLevel} enforcement=${enforcementMode} off-scope touched files [${offScopePreview}]; declared scope [${declaredScopePreview}]; total off-scope=${offScopeFiles.length} total scope=${declaredScope.length}`;
|
||||
// Name the offending sub-repo in workspace mode so the operator/agent knows where to revert.
|
||||
const repoTag = offendingRepo ? ` repo=${offendingRepo}` : "";
|
||||
const message = `[scope-leak] reviewLevel=${reviewLevel} enforcement=${enforcementMode}${repoTag} off-scope touched files [${offScopePreview}]; declared scope [${declaredScopePreview}]; total off-scope=${offScopeFiles.length} total scope=${declaredScope.length}`;
|
||||
executorLog.warn(`${task.id}: ${message}`);
|
||||
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));
|
||||
|
||||
if (enforcementMode === "block") {
|
||||
return {
|
||||
blocked: true,
|
||||
message: `Plan-Only scope-leak guard refused fn_task_done. Off-scope paths: [${offScopePreview}]. Revert them before retrying (for example: git checkout -- <paths>).`,
|
||||
message: `Plan-Only scope-leak guard refused fn_task_done${offendingRepo ? ` (sub-repo ${offendingRepo})` : ""}. Off-scope paths: [${offScopePreview}]. Revert them before retrying (for example: git checkout -- <paths>).`,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -11350,8 +11639,13 @@ export class TaskExecutor {
|
||||
// result, so the soft breach of `limit` does not push real
|
||||
// LLM-active concurrency above the configured cap.
|
||||
const sem = options.semaphore;
|
||||
const invokeReviewer = () => reviewStep(
|
||||
worktreePath, taskId, step, step_name,
|
||||
// FNXC:Workspace 2026-06-22-00:30: KTD3 — in-session fn_review_step loops per sub-repo.
|
||||
// `reviewStep` stays single-cwd; THIS CALLER loops. Single-cwd by default reviews `worktreePath`;
|
||||
// in workspace mode that is the browse-only non-git root, so we spawn one reviewer per acquired
|
||||
// sub-repo (cwd = repo.worktreePath) via reviewWorkspacePerRepo and aggregate as a conjunction.
|
||||
// `invokeReviewerForCwd` is the per-cwd reviewStep call both modes share.
|
||||
const invokeReviewerForCwd = (cwd: string) => reviewStep(
|
||||
cwd, taskId, step, step_name,
|
||||
reviewType, promptContent, baseline,
|
||||
{
|
||||
onText: (delta) => options.onAgentText?.(taskId, delta),
|
||||
@@ -11391,9 +11685,13 @@ export class TaskExecutor {
|
||||
onSessionEnded: (s) => this.unregisterSubagentSession(taskId, s),
|
||||
},
|
||||
);
|
||||
const result = sem
|
||||
? await sem.runNested(invokeReviewer)
|
||||
: await invokeReviewer();
|
||||
const runForCwd = (cwd: string) => {
|
||||
const invoke = () => invokeReviewerForCwd(cwd);
|
||||
return sem ? sem.runNested(invoke) : invoke();
|
||||
};
|
||||
const result = this.workspaceConfig
|
||||
? await this.reviewWorkspacePerRepo(currentTask, (cwd) => runForCwd(cwd))
|
||||
: await runForCwd(worktreePath);
|
||||
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
@@ -12401,6 +12699,115 @@ ${failureFeedback}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:Workspace 2026-06-21-23:30: KTD1 — per-repo modified-file capture for workspace tasks.
|
||||
* Loops `task.workspaceWorktrees` and REUSES `captureModifiedFiles` per sub-repo (NOT a hand-built `git diff`), so each repo gets: (a) resolveDiffBaseRef's merge-base fallback when repo.baseCommitSha is undefined, and (b) the filterFilesToOwnTaskCommits raw-vs-attributed divergence/contamination audit for free. Returned files are repo-prefixed (`<repoRel>/<file>`) and aggregated, so a downstream File-Scope check / merge can attribute each change to its sub-repo. Returns [] for a zero-acquire workspace task.
|
||||
*/
|
||||
private async captureWorkspaceModifiedFiles(
|
||||
task: Task,
|
||||
audit?: RunAuditor,
|
||||
source = "post-session",
|
||||
): Promise<string[]> {
|
||||
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
||||
// FNXC:Workspace 2026-06-21-15:00: F4/F6 — per-repo error isolation + deterministic ordering.
|
||||
// F4: an unexpected throw from one repo's `captureModifiedFiles` must NOT escape and skip the
|
||||
// downstream `updateTask({modifiedFiles})` write — that would leave `task.modifiedFiles` empty and
|
||||
// blind the merge file audit. Wrap each per-repo call (log + continue), mirroring the post-session
|
||||
// branch-attribution loop. F6: iterate sorted repo keys so aggregation order is stable across runs.
|
||||
const aggregated: string[] = [];
|
||||
for (const repoRel of Object.keys(workspaceWorktrees).sort()) {
|
||||
const repo = workspaceWorktrees[repoRel];
|
||||
try {
|
||||
const repoFiles = await this.captureModifiedFiles(repo.worktreePath, repo.baseCommitSha, task.id, audit, source);
|
||||
for (const file of repoFiles) {
|
||||
aggregated.push(`${repoRel}/${file}`);
|
||||
}
|
||||
} catch (repoErr: unknown) {
|
||||
executorLog.warn(`${task.id}: per-repo modified-file capture failed for ${repoRel}: ${repoErr instanceof Error ? repoErr.message : String(repoErr)}`);
|
||||
}
|
||||
}
|
||||
return aggregated;
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:Workspace 2026-06-22-00:30: KTD3 — per-repo review by looping the EXISTING single-cwd reviewStep.
|
||||
* The reviewer is an AGENT spawned with `cwd = worktree`, told (in prompt text, reviewer.ts) to run `git diff`
|
||||
* itself — it does NOT read a diff passed in code. So per-repo review = ONE reviewer agent per sub-repo. We keep
|
||||
* `reviewStep` single-cwd; the CALLERS loop. This helper is the shared loop+aggregate so both review entry points
|
||||
* (`createReviewStepTool` and the step-inversion `stepReview` seam) iterate identically: it invokes the caller's
|
||||
* own `invokeForCwd(cwd)` once per acquired worktree (cwd = repo.worktreePath) and aggregates the repo-tagged
|
||||
* verdicts as a CONJUNCTION — the task is "reviewed" only if EVERY repo passes; the FIRST non-APPROVE repo's
|
||||
* verdict becomes the aggregate verdict (mirroring verifyWorktreeInvariants' first-failing-repo return), and its
|
||||
* findings are repo-tagged. A zero-acquire workspace task (empty map) returns UNAVAILABLE so the caller routes it
|
||||
* rather than fabricating an APPROVE.
|
||||
*
|
||||
* Verdict severity for the conjunction: any RETHINK/REVISE/UNAVAILABLE fails the whole review; only all-APPROVE
|
||||
* (or all-skipped UNAVAILABLE-advisory, handled by the caller) approves. We surface the first failing repo's exact
|
||||
* verdict so the caller's existing verdict→edge mapping (APPROVE done-marking, REVISE block, RETHINK reset,
|
||||
* UNAVAILABLE retry) is unchanged.
|
||||
*/
|
||||
private async reviewWorkspacePerRepo(
|
||||
// FNXC:Workspace 2026-06-21-15:00: F7 — drop the dead `repoRel` callback param.
|
||||
// Both call sites bind `(cwd) => runForCwd(cwd)` and discard the second arg, so the type wrongly
|
||||
// implied repo identity is observable inside `runForCwd`. Removed until a real consumer needs it
|
||||
// (Phase C). The loop below still tags findings with `repoRel` from its own iteration key.
|
||||
task: Task,
|
||||
invokeForCwd: (cwd: string) => Promise<ReviewResult>,
|
||||
): Promise<ReviewResult> {
|
||||
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
||||
// FNXC:Workspace 2026-06-21-15:00: F6 — sort repo keys so the reported FIRST failing repo is
|
||||
// deterministic across runs/rehydrate.
|
||||
const repoKeys = Object.keys(workspaceWorktrees).sort();
|
||||
if (repoKeys.length === 0) {
|
||||
// No acquired worktree — surface UNAVAILABLE so the caller routes it rather than
|
||||
// fabricating an authoritative APPROVE for an un-reviewable workspace task.
|
||||
return {
|
||||
verdict: "UNAVAILABLE",
|
||||
review: "No acquired sub-repo worktree to review (workspace task with zero worktrees).",
|
||||
summary: "Skipped: no sub-repo worktree",
|
||||
};
|
||||
}
|
||||
|
||||
const reviewSections: string[] = [];
|
||||
const summarySections: string[] = [];
|
||||
let firstFailing: { repo: string; result: ReviewResult } | undefined;
|
||||
for (const repoRel of repoKeys) {
|
||||
const repo = workspaceWorktrees[repoRel];
|
||||
const result = await invokeForCwd(repo.worktreePath);
|
||||
// Tag every per-repo finding with its sub-repo so downstream readers attribute it correctly.
|
||||
reviewSections.push(`### [${repoRel}] ${result.verdict}\n${result.review}`);
|
||||
summarySections.push(`[${repoRel}] ${result.verdict}: ${result.summary}`);
|
||||
if (result.verdict !== "APPROVE") {
|
||||
// FNXC:Workspace 2026-06-21-15:00: F3 — BREAK on the first non-APPROVE repo.
|
||||
// The contract is "the FIRST non-APPROVE repo's verdict becomes the aggregate". Without the
|
||||
// break, a LATER repo's reviewer throwing would discard this already-determined REVISE/RETHINK
|
||||
// and the caller would see UNAVAILABLE — masking the real verdict. Stop at the first failure.
|
||||
firstFailing = { repo: repoRel, result };
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (firstFailing) {
|
||||
// Conjunction failed: the aggregate carries the FIRST failing repo's verdict (so the caller's
|
||||
// verdict→edge mapping is identical to single-cwd), with the full repo-tagged review body.
|
||||
return {
|
||||
verdict: firstFailing.result.verdict,
|
||||
// FNXC:Workspace 2026-06-22-00:00: the conjunction BREAKS on the first non-APPROVE repo,
|
||||
// so reviewSections holds only the repos evaluated up to (and including) the failure — not
|
||||
// every sub-repo. Label it honestly so operators don't read a partial list as exhaustive.
|
||||
review: `Workspace review failed in sub-repo \`${firstFailing.repo}\` (verdict ${firstFailing.result.verdict}). Per-repo verdicts (evaluation stopped at first failure; later repos not reviewed):\n\n${reviewSections.join("\n\n")}`,
|
||||
summary: `${firstFailing.repo}: ${firstFailing.result.verdict} — ${summarySections.join(" | ")}`,
|
||||
};
|
||||
}
|
||||
|
||||
// Every sub-repo approved → the task is reviewed (conjunction satisfied).
|
||||
return {
|
||||
verdict: "APPROVE",
|
||||
review: `All ${repoKeys.length} sub-repo(s) approved. Per-repo verdicts:\n\n${reviewSections.join("\n\n")}`,
|
||||
summary: `APPROVE across ${repoKeys.length} sub-repo(s): ${summarySections.join(" | ")}`,
|
||||
};
|
||||
}
|
||||
|
||||
private async captureUncommittedModifiedFiles(worktreePath: string): Promise<string[]> {
|
||||
try {
|
||||
const [unstaged, staged] = await Promise.all([
|
||||
|
||||
115
packages/engine/src/workspace-paths.ts
Normal file
115
packages/engine/src/workspace-paths.ts
Normal file
@@ -0,0 +1,115 @@
|
||||
/*
|
||||
FNXC:Workspace 2026-06-22-00:30:
|
||||
Minimal shared repo-prefix-derivation helper for workspace mode (Phase B U2; master U5 reuses it). A workspace task's File Scope, modified-file list, and review/scope-leak findings are all repo-prefixed (`<repoRel>/<file>`). Per-repo review and per-repo scope-leak need to map a path → its owning sub-repo, and to derive each repo's File-Scope subset (so a reviewer at `cwd = repo.worktreePath` and a per-repo scope-leak check evaluate only that repo's declared paths).
|
||||
|
||||
NO lease logic lives here (file-scope leases are Phase C / master U7). This module is intentionally dependency-light (pure string/path math) so it can be reused across the executor, reviewer callers, and the later merge loop without pulling in executor state.
|
||||
|
||||
Matching rule: canonicalize the path to forward-slash relative segments, then pick the LONGEST configured repo key that is a path-segment prefix of the file path. Longest-prefix (not naive first-segment) correctly handles nested repo keys like `apps/web` while still satisfying the simple `wolf-server/src/** → wolf-server` case. A path that matches no configured repo (absolute paths outside the workspace, root-level files like `.changeset/x.md`, or a first segment that is not a repo) derives to the `UNSCOPED` sentinel.
|
||||
*/
|
||||
|
||||
/** Sentinel returned when a path does not belong to any configured sub-repo. */
|
||||
export const UNSCOPED_REPO = "unscoped" as const;
|
||||
|
||||
/*
|
||||
FNXC:Workspace 2026-06-21-15:00:
|
||||
F8 — single normalize helper. The executor previously kept its own `normalizeWorkflowScopePath` that was a near-duplicate of this function, differing only in leading-slash stripping (`/^\/+/` here vs none there) and trailing-slash greediness (`/\/+$/` here vs `/\/$/` there). Two slightly-different normalizers meant an absolute or trailing-slash-laden path could derive a different scope key in the two code paths. We promote THIS (more aggressive: strips leading slash + collapses repeated trailing slashes) to the single exported normalizer and have the executor import it for scope-path normalization, so workspace and non-workspace scope matching canonicalize identically. workspace-paths.ts stays dependency-light (imports nothing), so executor→workspace-paths is a one-way, acyclic edge.
|
||||
*/
|
||||
export function normalizeRepoRelPath(value: string): string {
|
||||
return value
|
||||
.trim()
|
||||
.replace(/\\/g, "/")
|
||||
.replace(/^\.\//, "")
|
||||
.replace(/\/+/g, "/")
|
||||
.replace(/^\/+/, "")
|
||||
.replace(/\/+$/, "");
|
||||
}
|
||||
|
||||
/** Split a normalized path into non-empty segments. */
|
||||
function segmentsOf(value: string): string[] {
|
||||
const normalized = normalizeRepoRelPath(value);
|
||||
return normalized ? normalized.split("/") : [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true when `repoSegs` is a leading segment-prefix of `pathSegs`.
|
||||
* Segment-wise (not substring) so `repo-a` does NOT match `repo-ab/...`.
|
||||
*/
|
||||
function isSegmentPrefix(repoSegs: string[], pathSegs: string[]): boolean {
|
||||
if (repoSegs.length === 0 || repoSegs.length > pathSegs.length) return false;
|
||||
for (let i = 0; i < repoSegs.length; i++) {
|
||||
if (repoSegs[i] !== pathSegs[i]) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive the configured sub-repo that owns `filePath`, or {@link UNSCOPED_REPO}.
|
||||
*
|
||||
* `repos` are the configured workspace sub-repo relative keys (from
|
||||
* `workspaceConfig.repos` or `Object.keys(task.workspaceWorktrees)`). The LONGEST
|
||||
* matching repo key wins so nested repos (`apps/web` vs `apps`) resolve to the
|
||||
* most specific owner.
|
||||
*/
|
||||
export function deriveRepoForPath(filePath: string, repos: readonly string[]): string {
|
||||
const pathSegs = segmentsOf(filePath);
|
||||
if (pathSegs.length === 0) return UNSCOPED_REPO;
|
||||
let best: string | null = null;
|
||||
let bestLen = 0;
|
||||
for (const repo of repos) {
|
||||
const repoSegs = segmentsOf(repo);
|
||||
if (repoSegs.length === 0) continue;
|
||||
if (isSegmentPrefix(repoSegs, pathSegs) && repoSegs.length > bestLen) {
|
||||
best = normalizeRepoRelPath(repo);
|
||||
bestLen = repoSegs.length;
|
||||
}
|
||||
}
|
||||
return best ?? UNSCOPED_REPO;
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of splitting a repo-prefixed File-Scope entry into its owning repo and
|
||||
* the repo-relative remainder (the path AS the reviewer at `cwd = repo` sees it).
|
||||
*/
|
||||
export interface RepoScopedPath {
|
||||
/** Owning sub-repo key, or {@link UNSCOPED_REPO}. */
|
||||
repo: string;
|
||||
/** The path with the repo prefix stripped (repo-local). Equals `path` when unscoped. */
|
||||
relativePath: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Split a repo-prefixed path into `{ repo, relativePath }`. For `repo-a/src/x.ts`
|
||||
* with `repos=["repo-a"]` → `{ repo:"repo-a", relativePath:"src/x.ts" }`. An
|
||||
* unscoped path returns the whole normalized path as `relativePath`.
|
||||
*/
|
||||
export function splitRepoScopedPath(filePath: string, repos: readonly string[]): RepoScopedPath {
|
||||
const repo = deriveRepoForPath(filePath, repos);
|
||||
const normalized = normalizeRepoRelPath(filePath);
|
||||
if (repo === UNSCOPED_REPO) {
|
||||
return { repo, relativePath: normalized };
|
||||
}
|
||||
const repoNormalized = normalizeRepoRelPath(repo);
|
||||
const remainder = normalized.slice(repoNormalized.length).replace(/^\/+/, "");
|
||||
return { repo, relativePath: remainder };
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive a single sub-repo's File-Scope subset from the task's full (repo-prefixed)
|
||||
* declared scope. Returns the repo-LOCAL scope patterns (prefix stripped) so a
|
||||
* per-repo reviewer or per-repo scope-leak check — operating with `cwd = repo` —
|
||||
* can compare repo-local paths directly. Entries owned by other repos (or unscoped)
|
||||
* are excluded. A scope entry whose prefix-stripped remainder is empty (the repo
|
||||
* root itself, e.g. `repo-a` or `repo-a/`) maps to `**` (whole-repo scope).
|
||||
*/
|
||||
export function deriveRepoScopeSubset(declaredScope: readonly string[], repoRel: string): string[] {
|
||||
const repoSegs = segmentsOf(repoRel);
|
||||
if (repoSegs.length === 0) return [];
|
||||
const subset: string[] = [];
|
||||
for (const entry of declaredScope) {
|
||||
const entrySegs = segmentsOf(entry);
|
||||
if (!isSegmentPrefix(repoSegs, entrySegs)) continue;
|
||||
const remainder = entrySegs.slice(repoSegs.length).join("/");
|
||||
subset.push(remainder === "" ? "**" : remainder);
|
||||
}
|
||||
return subset;
|
||||
}
|
||||
@@ -867,6 +867,9 @@ export async function acquireWorkspaceRepoWorktree(
|
||||
});
|
||||
} catch (guardErr) {
|
||||
// FNXC:Workspace 2026-06-21-22:30: F3 — identity-guard install is non-fatal; worktree is usable without it.
|
||||
// FNXC:Workspace 2026-06-22-00:00: the non-fatal logEntry/audit are themselves best-effort — if either throws
|
||||
// (e.g. a DB write hiccup) it must NOT promote this non-fatal guard failure into a fatal acquisition failure.
|
||||
// Swallow logging errors so acquisition continues (matching the F6 busy-path defensive wrap above).
|
||||
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)
|
||||
@@ -904,6 +907,8 @@ export async function acquireWorkspaceRepoWorktree(
|
||||
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.
|
||||
// FNXC:Workspace 2026-06-22-00:00: guard the best-effort logEntry/audit so a logging throw cannot promote this
|
||||
// non-fatal capture failure into a fatal acquisition failure (parity with the F6 busy-path defensive wrap).
|
||||
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 —
|
||||
@@ -957,6 +962,9 @@ export async function acquireWorkspaceRepoWorktree(
|
||||
sub-repo.
|
||||
*/
|
||||
if (!(err instanceof WorkspaceRepoAcquireBusyError)) {
|
||||
// FNXC:Workspace 2026-06-22-00:00: wrap the failure logEntry/audit so a throw here cannot replace the ORIGINAL
|
||||
// acquisition `err` the caller must observe — losing it would mask the real cause and the re-throw below would
|
||||
// surface a logging error instead. Best-effort observability; `err` is always re-thrown.
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user