root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(workspace): address CodeRabbit review findings on the foundation

Browse Source 
Resolves the actionable CodeRabbit threads on the workspace-mode foundation:

- project-resolver: defer saveWorkspaceConfig until after the user confirms init
  and store.init() succeeds (no partial .fusion/ on a declined/non-interactive run).
- git-repository: validate each candidate with a real `git rev-parse` work-tree
  probe before counting it (no false-positive repos from stray .git markers);
  loadWorkspaceConfig now rejects absolute paths, `..` escapes, and non-string
  entries so a corrupt/malicious config can't resolve outside the workspace root.
- executor: gate workspace mode on repos.length > 0 at all three sites so an
  empty { repos: [] } can't bypass the git-repo guard or enable an empty workspace.
- worktree-acquisition: thread the configured-command runner through the workspace
  acquire path (sub-repos run their init setup); validate repoRelPath as an in-root
  relative path before joining; liveness-check a remembered worktree before
  reporting it ready (pruned paths fall through to re-acquire); clear the singular
  task.worktree/branch after persisting per-repo state (per-repo state lives only
  in workspaceWorktrees).
- agent-tools: forward runContext into acquireWorkspaceRepoWorktree for log attribution.

The executor-workspace test's mock-the-subject pattern is left for the
session-scoping follow-up that rewrites it with a real two-repo fixture (FN-5048).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-22 00:09:59 -07:00
parent 79e53e8d97
commit 429258354d
5 changed files with 139 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
packages/cli/src/project-resolver.ts
View File

@@ -632,14 +632,22 @@ export async function registerProjectInteractive(
const gitCheck = spawnSync("git", ["-C", absPath, "rev-parse", "--is-inside-work-tree"], { encoding: "utf8" });
const isGitRepo = gitCheck.status === 0 && gitCheck.stdout.trim() === "true";
/*
FNXC:Workspace 2026-06-22-00:00:
Workspace detection only reports candidate sub-repos here; persistence is deferred until
after the user confirms init AND TaskStore.init() succeeds. Writing .fusion/workspace.json
before confirmation would leave a partial .fusion/ dir when the user declines or runs
non-interactively, polluting a plain non-git directory with stray Fusion state.
*/
let detectedSubRepos: string[] | null = null;
if (!isGitRepo) {
const subRepos = await detectWorkspaceRepos(absPath);
if (subRepos.length > 0) {
console.log(`\n Found ${subRepos.length} git repositories in ${absPath}:`);
subRepos.forEach((r: string) => console.log(` • ${r}`));
console.log(`\n Initializing as a Fusion workspace...\n`);
await saveWorkspaceConfig(absPath, { repos: subRepos });
// Fall through to normal .fusion init
detectedSubRepos = subRepos;
// workspace.json is written below, only after a confirmed store.init() succeeds.
}
// else: fall through to existing error path
}
@@ -653,6 +661,9 @@ export async function registerProjectInteractive(
const { TaskStore } = await import("@fusion/core");
const store = new TaskStore(absPath);
await store.init();
if (detectedSubRepos) {
await saveWorkspaceConfig(absPath, { repos: detectedSubRepos });
}
console.log(` ✓ Initialized fn at ${absPath}`);
} else {
throw new ProjectResolutionError(

42
packages/core/src/git-repository.ts
View File

@@ -114,13 +114,23 @@ export async function detectWorkspaceRepos(dir: string): Promise<string[]> {
const { stat } = await import("node:fs/promises");
const { join } = await import("node:path");
const found: string[] = [];
/*
FNXC:Workspace 2026-06-22-00:00:
A bare `.git` marker (e.g. a stray file copied in, or an unrelated tool's artifact) is not
proof of a git repository. Each candidate child is validated with a real `git rev-parse`
work-tree probe before it counts, so stray `.git` entries do not yield false-positive repos.
*/
for (const entry of entries) {
const candidate = join(dir, entry, ".git");
const childDir = join(dir, entry);
// Cheap pre-filter: skip children with no `.git` marker at all before spawning git.
try {
const s = await stat(candidate);
if (s.isDirectory() || s.isFile()) found.push(entry);
const s = await stat(join(childDir, ".git"));
if (!s.isDirectory() && !s.isFile()) continue;
} catch {
// not a git repo
continue;
}
if (await isInsideGitWorkTree(childDir, runGitCommand, DEFAULT_GIT_TIMEOUT_MS)) {
found.push(entry);
}
}
return found.sort();
@@ -132,9 +142,27 @@ export interface WorkspaceConfig {
const WORKSPACE_CONFIG_FILENAME = "workspace.json";
/*
FNXC:Workspace 2026-06-22-00:00:
Workspace repo entries are later joined onto the workspace root to resolve worktrees, so an
attacker-controlled or corrupted workspace.json with an absolute path or a `..` escape
(`../outside-repo`) would resolve outside the workspace root. Each entry must be a normalized,
relative, in-root path; absolute paths, `..` escapes, and non-string entries are rejected.
*/
function isInRootRelativePath(entry: unknown, pathMod: typeof import("node:path")): entry is string {
if (typeof entry !== "string" || entry.length === 0) return false;
if (pathMod.isAbsolute(entry)) return false;
const normalized = pathMod.normalize(entry);
if (normalized === ".." || normalized.startsWith(`..${pathMod.sep}`) || normalized.startsWith("../")) {
return false;
}
return true;
}
export async function loadWorkspaceConfig(rootDir: string): Promise<WorkspaceConfig | null> {
const { readFile } = await import("node:fs/promises");
const { join } = await import("node:path");
const pathMod = await import("node:path");
const { join } = pathMod;
const configPath = join(rootDir, ".fusion", WORKSPACE_CONFIG_FILENAME);
try {
const raw = await readFile(configPath, "utf-8");
@@ -145,7 +173,9 @@ export async function loadWorkspaceConfig(rootDir: string): Promise<WorkspaceCon
"repos" in parsed &&
Array.isArray((parsed as { repos: unknown }).repos)
) {
return parsed as WorkspaceConfig;
const rawRepos = (parsed as { repos: unknown[] }).repos;
const repos = rawRepos.filter((entry): entry is string => isInRootRelativePath(entry, pathMod));
return { ...(parsed as object), repos };
}
return null;
} catch {

10
packages/engine/src/agent-tools.ts
View File

@@ -3601,8 +3601,12 @@ export function createAcquireRepoWorktreeTool(opts: {
logger?: { log: (m: string) => void; warn: (m: string) => void };
secretsStore?: Pick<import("@fusion/core").SecretsStore, "listEnvExportable">;
runContext?: RunMutationContext;
audit?: Pick<RunAuditor, "git" | "filesystem">;
// FNXC:Workspace 2026-06-22 — thread the configured worktree-init runner so sub-repo worktrees run configured setup.
runConfiguredCommand?: import("./worktree-acquisition.js").AcquireWorkspaceRepoWorktreeOptions["runConfiguredCommand"];
taskEnv?: NodeJS.ProcessEnv;
}): ToolDefinition {
const { workspaceRootDir, workspaceRepos, task, store, settings, logger, secretsStore, runContext } = opts;
const { workspaceRootDir, workspaceRepos, task, store, settings, logger, secretsStore, runContext, audit, runConfiguredCommand, taskEnv } = opts;
return {
name: "fn_acquire_repo_worktree",
label: "Acquire Repo Worktree",
@@ -3629,6 +3633,10 @@ export function createAcquireRepoWorktreeTool(opts: {
settings,
logger,
secretsStore,
runContext,
audit,
runConfiguredCommand,
taskEnv,
});
await store.logEntry(
task.id,

19
packages/engine/src/executor.ts
View File

@@ -7416,7 +7416,15 @@ export class TaskExecutor {
if (this.workspaceConfig === undefined) {
this.workspaceConfig = await loadWorkspaceConfig(this.rootDir);
}
if (!this.workspaceConfig && !await isGitRepository(this.rootDir)) {
/*
FNXC:Workspace 2026-06-22-00:00:
Workspace mode is only meaningful with at least one usable sub-repo. An empty `{ repos: [] }`
must NOT bypass the git-repository guard, inject workspace instructions, or expose the
workspace tool — otherwise a non-git directory with an empty config would skip validation
and enable a workspace with nothing to work on. Gate every workspace check on repos.length > 0.
*/
const hasWorkspaceRepos = (this.workspaceConfig?.repos.length ?? 0) > 0;
if (!hasWorkspaceRepos && !await isGitRepository(this.rootDir)) {
await this.store.logEntry(
task.id,
"Cannot execute task: project directory is not a Git repository. Fusion requires a Git repository for worktree-based task execution.",
@@ -8351,7 +8359,7 @@ export class TaskExecutor {
...getEnabledPluginTools(this.options.pluginRunner),
];
if (this.workspaceConfig) {
if (this.workspaceConfig && this.workspaceConfig.repos.length > 0) {
customTools.push(createAcquireRepoWorktreeTool({
workspaceRootDir: this.rootDir,
workspaceRepos: this.workspaceConfig.repos,
@@ -8361,6 +8369,11 @@ export class TaskExecutor {
logger: executorLog,
secretsStore: this.options.secretsStore,
runContext: engineRunContext,
audit,
taskEnv,
// FNXC:Workspace 2026-06-22 — forward the configured worktree-init runner so sub-repo worktrees run configured setup.
runConfiguredCommand: (command, cwd, timeoutMs, env) =>
runConfiguredCommand(command, cwd, timeoutMs, env, audit),
}));
}
@@ -15940,7 +15953,7 @@ Use \`fn_task_create\` for truly separate follow-up work, including unrelated/pr
If lint is configured and failing, fix that too before completion.
Do not repeatedly rerun a broad failing or hanging workspace command without a new hypothesis and a narrower confirming command.`;
if (workspaceConfig) {
if (workspaceConfig && workspaceConfig.repos.length > 0) {
return executionPrompt + `\n\n## Workspace mode\n` +
`This project is a workspace containing multiple git repositories.\n` +
`Available repos:\n` +

71
packages/engine/src/worktree-acquisition.ts
View File

@@ -604,21 +604,64 @@ export interface AcquireWorkspaceRepoWorktreeOptions {
settings: Partial<Settings>;
logger?: { log: (m: string) => void; warn: (m: string) => void; error?: (m: string) => void };
secretsStore?: Pick<SecretsStore, "listEnvExportable">;
runContext?: RunMutationContext;
audit?: Pick<RunAuditor, "git" | "filesystem">;
runConfiguredCommand?: AcquireTaskWorktreeOptions["runConfiguredCommand"];
taskEnv?: NodeJS.ProcessEnv;
}
/*
FNXC:WorkspaceWorktree 2026-06-22-00:00:
`repoRelPath` is an exported, caller-trusted parameter that is joined onto `workspaceRootDir`.
An absolute path or a `..` escape (`../outside`) would resolve a worktree outside the workspace
root. Validate it is a normalized, relative, in-root path before resolving the absolute path.
*/
function assertInRootRepoRelPath(repoRelPath: string, sep: string, isAbsolute: (p: string) => boolean, normalize: (p: string) => string): void {
if (typeof repoRelPath !== "string" || repoRelPath.length === 0 || isAbsolute(repoRelPath)) {
throw new Error(`Invalid workspace repo path (must be relative and in-root): ${String(repoRelPath)}`);
}
const normalized = normalize(repoRelPath);
if (normalized === ".." || normalized.startsWith(`..${sep}`) || normalized.startsWith("../")) {
throw new Error(`Invalid workspace repo path (escapes workspace root): ${repoRelPath}`);
}
}
export async function acquireWorkspaceRepoWorktree(
opts: AcquireWorkspaceRepoWorktreeOptions,
): Promise<{ worktreePath: string; branch: string; alreadyAcquired: boolean }> {
const { repoRelPath, workspaceRootDir, task, store, settings, logger, secretsStore } = opts;
const { join } = await import("node:path");
const { repoRelPath, workspaceRootDir, task, store, settings, logger, secretsStore, runContext, audit, runConfiguredCommand, taskEnv } = opts;
const { join, isAbsolute, normalize, sep } = await import("node:path");
// FNXC:WorkspaceWorktree 2026-06-22 — reject absolute / `..`-escaping repo paths before resolving.
assertInRootRepoRelPath(repoRelPath, sep, isAbsolute, normalize);
const repoAbsPath = join(workspaceRootDir, repoRelPath);
/*
FNXC:WorkspaceWorktree 2026-06-22-00:00:
A remembered per-repo worktree is only reusable if it still exists and is a registered git
worktree. A pruned/deleted worktree path would otherwise be reported as "ready" without the
resume/classification checks that `acquireTaskWorktree` runs on the singular path. Verify the
remembered path passes the same liveness check (existence + git work-tree classification);
if it is dead, drop it and fall through to re-acquire a fresh worktree.
*/
const existing = task.workspaceWorktrees?.[repoRelPath];
if (existing) {
return { ...existing, alreadyAcquired: true };
let live = existsSync(existing.worktreePath);
if (live) {
try {
const classification = await classifyTaskWorktree(repoAbsPath, existing.worktreePath);
live = classification.ok;
} catch {
live = false;
}
}
if (live) {
return { ...existing, alreadyAcquired: true };
}
logger?.warn(`${task.id}: remembered workspace worktree for ${repoRelPath} is missing/unusable (${existing.worktreePath}); re-acquiring`);
await store.logEntry(task.id, `Remembered workspace worktree for ${repoRelPath} is no longer usable; re-acquiring`, existing.worktreePath, runContext);
}
const repoAbsPath = join(workspaceRootDir, repoRelPath);
/*
FNXC:WorkspaceWorktree 2026-06-21-00:00:
Workspace mode acquires one worktree per sub-repo for a single task. `acquireTaskWorktree`
@@ -629,6 +672,11 @@ export async function acquireWorkspaceRepoWorktree(
contamination. Clear the singular worktree/branch fields on the copy handed to the single-repo
helper so each sub-repo always gets a fresh worktree; per-repo state is tracked in
`task.workspaceWorktrees`, not the singular column.
FNXC:WorkspaceWorktree 2026-06-22-00:00:
`acquireTaskWorktree` only runs the configured worktree-init command when `runConfiguredCommand`
is threaded through. Forward it (plus runContext/audit/taskEnv) so workspace sub-repos run the
same configured setup as the non-workspace acquire path instead of silently skipping it.
*/
const result = await acquireTaskWorktree({
task: { ...task, worktree: undefined, branch: undefined },
@@ -637,6 +685,10 @@ export async function acquireWorkspaceRepoWorktree(
settings,
logger,
secretsStore,
runContext,
audit,
runConfiguredCommand,
taskEnv,
runInitCommand: true,
});
@@ -644,7 +696,14 @@ export async function acquireWorkspaceRepoWorktree(
...(task.workspaceWorktrees ?? {}),
[repoRelPath]: { worktreePath: result.worktreePath, branch: result.branch },
};
await store.updateTask(task.id, { workspaceWorktrees: updated });
/*
FNXC:WorkspaceWorktree 2026-06-22-00:00:
`acquireTaskWorktree` persists the singular `task.worktree`/`task.branch` on the task row.
For a workspace task that pointer would end up referencing whichever sub-repo was acquired last,
violating the contract that per-repo state lives only in `workspaceWorktrees`. Clear the singular
fields in the same update so a workspace task never carries a misleading singular worktree pointer.
*/
await store.updateTask(task.id, { workspaceWorktrees: updated, worktree: null, branch: null });
return { worktreePath: result.worktreePath, branch: result.branch, alreadyAcquired: false };
}