feat(FN-4948): complete Step 2 — wire identity guard into worktree provisioning
Fusion-Task-Id: FN-4948 Fusion-Task-Lineage: dc622643-4c3e-4217-9219-a6a6e5424427
This commit is contained in:
committed by
gsxdsm
parent
f42a149fe3
commit
487dfcc10c
@@ -1,4 +1,5 @@
|
||||
import { vi } from "vitest";
|
||||
import { installTaskWorktreeIdentityGuard } from "../worktree-hooks.js";
|
||||
|
||||
// Mock external dependencies
|
||||
vi.mock("../pi.js", () => ({
|
||||
@@ -115,6 +116,10 @@ vi.mock("../worktree-pool.js", async (importOriginal) => {
|
||||
isUsableTaskWorktree: vi.fn().mockResolvedValue(true),
|
||||
};
|
||||
});
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock("../worktree-stale-lock.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("../worktree-stale-lock.js")>("../worktree-stale-lock.js");
|
||||
return {
|
||||
@@ -268,6 +273,7 @@ export const mockedDescribeRegisteredWorktrees = vi.mocked(describeRegisteredWor
|
||||
export const mockedIsUsableTaskWorktree = vi.mocked(isUsableTaskWorktree);
|
||||
export const mockedClassifyStaleLock = vi.mocked(classifyStaleLock);
|
||||
export const mockedTryRemoveStaleLock = vi.mocked(tryRemoveStaleLock);
|
||||
export const mockedInstallTaskWorktreeIdentityGuard = vi.mocked(installTaskWorktreeIdentityGuard);
|
||||
|
||||
export type EventListener = (...args: unknown[]) => void;
|
||||
|
||||
@@ -349,7 +355,9 @@ export function resetExecutorMocks() {
|
||||
});
|
||||
mockedClassifyStaleLock.mockReset();
|
||||
mockedTryRemoveStaleLock.mockReset();
|
||||
mockedInstallTaskWorktreeIdentityGuard.mockReset();
|
||||
mockedClassifyStaleLock.mockResolvedValue({ kind: "fresh", reason: "fresh" } as any);
|
||||
mockedInstallTaskWorktreeIdentityGuard.mockResolvedValue(undefined);
|
||||
mockedTryRemoveStaleLock.mockResolvedValue({ removed: true });
|
||||
mockExecuteAll.mockResolvedValue([]);
|
||||
mockTerminateAllSessions.mockResolvedValue(undefined);
|
||||
|
||||
@@ -10,6 +10,10 @@
|
||||
* - Crash scenarios are handled gracefully (semaphore release, status cleanup)
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
import { AgentSemaphore } from "../concurrency.js";
|
||||
|
||||
const WAIT_FOR_ASYNC_OPTIONS = { timeout: 2000, interval: 5 };
|
||||
|
||||
@@ -10,6 +10,10 @@ import { AgentLogger } from "../agent-logger.js";
|
||||
import * as worktreeBackendModule from "../worktree-backend.js";
|
||||
import type { TaskDetail, Settings, TaskStore } from "@fusion/core";
|
||||
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
// ── Shared test fixtures ──────────────────────────────────────────────
|
||||
|
||||
function makePrompt(steps: string[]): string {
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
import { acquireTaskWorktree } from "../worktree-acquisition.js";
|
||||
import type { WorktreeBackend } from "../worktree-backend.js";
|
||||
|
||||
|
||||
@@ -8,6 +8,9 @@ const { execMock, existsSyncMock } = vi.hoisted(() => {
|
||||
|
||||
vi.mock("node:child_process", () => ({ exec: execMock }));
|
||||
vi.mock("node:fs", () => ({ existsSync: existsSyncMock }));
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("../worktree-pool.js", async () => {
|
||||
const actual = await vi.importActual<any>("../worktree-pool.js");
|
||||
return { ...actual, isUsableTaskWorktree: vi.fn().mockResolvedValue(true) };
|
||||
|
||||
@@ -8,7 +8,7 @@ import {
|
||||
RemovalReason,
|
||||
} from "../worktree-backend.js";
|
||||
|
||||
const { execMock, accessMock, existsSyncMock, parseIndexLockPathMock, classifyStaleLockMock, tryRemoveStaleLockMock } = vi.hoisted(() => {
|
||||
const { execMock, accessMock, existsSyncMock, parseIndexLockPathMock, classifyStaleLockMock, tryRemoveStaleLockMock, installGuardMock } = vi.hoisted(() => {
|
||||
const mock = vi.fn();
|
||||
(mock as any)[Symbol.for("nodejs.util.promisify.custom")] = mock;
|
||||
return {
|
||||
@@ -18,6 +18,7 @@ const { execMock, accessMock, existsSyncMock, parseIndexLockPathMock, classifySt
|
||||
parseIndexLockPathMock: vi.fn(),
|
||||
classifyStaleLockMock: vi.fn(),
|
||||
tryRemoveStaleLockMock: vi.fn(),
|
||||
installGuardMock: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -27,6 +28,9 @@ vi.mock("node:fs/promises", () => ({ access: accessMock }));
|
||||
vi.mock("../branch-conflicts.js", () => ({
|
||||
inspectBranchConflict: vi.fn().mockResolvedValue({ kind: "stale" }),
|
||||
}));
|
||||
vi.mock("../worktree-hooks.js", () => ({
|
||||
installTaskWorktreeIdentityGuard: installGuardMock,
|
||||
}));
|
||||
vi.mock("../worktree-stale-lock.js", () => ({
|
||||
StaleWorktreeIndexLockError: class StaleWorktreeIndexLockError extends Error {
|
||||
lockPath: string;
|
||||
@@ -54,6 +58,8 @@ beforeEach(() => {
|
||||
parseIndexLockPathMock.mockReset();
|
||||
classifyStaleLockMock.mockReset();
|
||||
tryRemoveStaleLockMock.mockReset();
|
||||
installGuardMock.mockReset();
|
||||
installGuardMock.mockResolvedValue(undefined);
|
||||
parseIndexLockPathMock.mockReturnValue(null);
|
||||
classifyStaleLockMock.mockResolvedValue({ kind: "fresh", reason: "fresh" });
|
||||
tryRemoveStaleLockMock.mockResolvedValue({ removed: true });
|
||||
@@ -77,6 +83,7 @@ describe("NativeWorktreeBackend", () => {
|
||||
'git worktree add -b "fusion/fn-1" "/repo/.worktrees/fn-1" "main"',
|
||||
expect.objectContaining({ cwd: "/repo", timeout: 120000, maxBuffer: 10485760 }),
|
||||
);
|
||||
expect(installGuardMock).toHaveBeenCalledWith({ worktreePath: "/repo/.worktrees/fn-1", taskId: "FN-1" });
|
||||
});
|
||||
|
||||
it("retries with suffix and resolves", async () => {
|
||||
|
||||
@@ -75,6 +75,7 @@ import type { PluginRunner } from "./plugin-runner.js";
|
||||
import { isContextLimitError } from "./context-limit-detector.js";
|
||||
import { StepSessionExecutor } from "./step-session-executor.js";
|
||||
import { acquireTaskWorktree } from "./worktree-acquisition.js";
|
||||
import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js";
|
||||
import {
|
||||
resolveAgentInstructions,
|
||||
buildSystemPromptWithInstructions,
|
||||
@@ -8410,6 +8411,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
}
|
||||
} else {
|
||||
executorLog.log(`Worktree already exists: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
@@ -8456,6 +8458,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
if (attemptNumber > 0) {
|
||||
await this.store.logEntry(taskId, `Worktree created on attempt ${attemptNumber + 1}`, path);
|
||||
}
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
return { path, branch };
|
||||
} catch (initialError: unknown) {
|
||||
const conflictInfo = this.extractWorktreeConflictInfo(initialError);
|
||||
@@ -8466,6 +8469,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
if (recovered) {
|
||||
await createWithBranch(branch);
|
||||
executorLog.log(`Worktree created after stale lock recovery: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
@@ -8525,6 +8529,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
try {
|
||||
await createFromExistingBranch();
|
||||
executorLog.log(`Worktree created from existing branch: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
return { path, branch };
|
||||
} catch (fallbackError: unknown) {
|
||||
const fallbackErrorMessage = fallbackError instanceof Error ? fallbackError.message : String(fallbackError);
|
||||
@@ -8536,6 +8541,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
if (recovered) {
|
||||
await createFromExistingBranch();
|
||||
executorLog.log(`Worktree created from existing branch after stale lock recovery: ${path}`);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: path, taskId });
|
||||
return { path, branch };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ import type { AgentActionGateContext } from "./agent-action-gate.js";
|
||||
import type { SkillSelectionContext } from "./skill-resolver.js";
|
||||
import { generateWorktreeName } from "./worktree-names.js";
|
||||
import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
||||
import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js";
|
||||
import { AgentSemaphore } from "./concurrency.js";
|
||||
import { StuckTaskDetector } from "./stuck-task-detector.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
@@ -1345,6 +1346,11 @@ Follow instructions precisely and avoid unrelated changes.`,
|
||||
throw err;
|
||||
}
|
||||
|
||||
await installTaskWorktreeIdentityGuard({
|
||||
worktreePath,
|
||||
taskId: this.options.taskDetail.id,
|
||||
});
|
||||
|
||||
this.registerParallelWorktree(stepIndex, worktreePath);
|
||||
this.parallelBranches.set(stepIndex, branchName);
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ import type { RunAuditor } from "./run-audit.js";
|
||||
import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
||||
import { inspectBranchConflict } from "./branch-conflicts.js";
|
||||
import { formatError } from "./logger.js";
|
||||
import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js";
|
||||
import {
|
||||
StaleWorktreeIndexLockError,
|
||||
classifyStaleLock,
|
||||
@@ -191,7 +192,9 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
|
||||
let staleLockRecoveryAttempted = false;
|
||||
try {
|
||||
return await createWithBranch(input.branch);
|
||||
const created = await createWithBranch(input.branch);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: created.path, taskId: input.taskId });
|
||||
return created;
|
||||
} catch (error) {
|
||||
const lockPath = parseIndexLockPath(`${(error as { message?: string })?.message ?? ""}\n${getErrorStderr(error) ?? ""}`);
|
||||
if (lockPath && !staleLockRecoveryAttempted) {
|
||||
@@ -221,7 +224,9 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
target: input.worktreePath,
|
||||
metadata: { lockPath },
|
||||
});
|
||||
return await createWithBranch(input.branch);
|
||||
const created = await createWithBranch(input.branch);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: created.path, taskId: input.taskId });
|
||||
return created;
|
||||
}
|
||||
await this.deps.audit?.git({
|
||||
type: "worktree:stale-lock-recovery-failed",
|
||||
@@ -263,7 +268,9 @@ export class NativeWorktreeBackend implements WorktreeBackend {
|
||||
for (let suffix = 2; suffix <= 50; suffix += 1) {
|
||||
const candidateBranch = `${input.branch}-${suffix}`;
|
||||
try {
|
||||
return await createWithBranch(candidateBranch);
|
||||
const created = await createWithBranch(candidateBranch);
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: created.path, taskId: input.taskId });
|
||||
return created;
|
||||
} catch {
|
||||
// continue probing suffixes
|
||||
}
|
||||
@@ -446,6 +453,7 @@ export class WorktrunkWorktreeBackend implements WorktreeBackend {
|
||||
);
|
||||
}
|
||||
|
||||
await installTaskWorktreeIdentityGuard({ worktreePath: resolvedPath, taskId: input.taskId });
|
||||
return { path: resolvedPath, branch: input.branch };
|
||||
}
|
||||
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
import { execFile } from "node:child_process";
|
||||
import { chmod, mkdir, readFile, rename, stat, writeFile } from "node:fs/promises";
|
||||
import { exec } from "node:child_process";
|
||||
import * as fs from "node:fs/promises";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
const execAsync = promisify(exec);
|
||||
|
||||
export const DEFAULT_ALLOWED_BRANCH_PATTERNS = ["^fusion/step-\\d+-[a-z0-9-]+$"] as const;
|
||||
|
||||
@@ -16,9 +16,7 @@ function toShellCasePattern(pattern: string): string {
|
||||
}
|
||||
|
||||
export function buildIdentityGuardHook(taskId: string, allowedBranchPatterns: readonly string[] = DEFAULT_ALLOWED_BRANCH_PATTERNS): string {
|
||||
const allowChecks = allowedBranchPatterns
|
||||
.map((pattern) => ` ${toShellCasePattern(pattern)}) exit 0 ;;`)
|
||||
.join("\n");
|
||||
const allowChecks = allowedBranchPatterns.map((pattern) => ` ${toShellCasePattern(pattern)}) exit 0 ;;`).join("\n");
|
||||
|
||||
return `#!/bin/sh
|
||||
set -eu
|
||||
@@ -56,10 +54,7 @@ exit 1
|
||||
|
||||
async function resolveGitDir(worktreePath: string): Promise<string> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync("git", ["rev-parse", "--git-dir"], {
|
||||
cwd: worktreePath,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
const { stdout } = await execAsync("git rev-parse --git-dir", { cwd: worktreePath, encoding: "utf-8" });
|
||||
return resolve(worktreePath, stdout.trim());
|
||||
} catch (error) {
|
||||
throw new Error(`Failed to resolve git dir for worktree ${worktreePath}: ${(error as Error).message}`);
|
||||
@@ -67,20 +62,13 @@ async function resolveGitDir(worktreePath: string): Promise<string> {
|
||||
}
|
||||
|
||||
async function writeFileAtomic(targetPath: string, content: string, mode?: number): Promise<void> {
|
||||
await mkdir(dirname(targetPath), { recursive: true });
|
||||
await execAsync(`mkdir -p ${JSON.stringify(dirname(targetPath))}`);
|
||||
const tmpPath = `${targetPath}.tmp`;
|
||||
const current = await readFile(targetPath, "utf-8").catch(() => null);
|
||||
if (current === content) {
|
||||
if (mode != null) {
|
||||
await chmod(targetPath, mode);
|
||||
}
|
||||
return;
|
||||
}
|
||||
await writeFile(tmpPath, content, { encoding: "utf-8", mode });
|
||||
if (mode != null) {
|
||||
await chmod(tmpPath, mode);
|
||||
}
|
||||
await rename(tmpPath, targetPath);
|
||||
const current = await fs.readFile(targetPath, "utf-8").catch(() => null);
|
||||
if (current === content) return;
|
||||
await fs.writeFile(tmpPath, content, { encoding: "utf-8", mode });
|
||||
if (mode != null) await fs.chmod(tmpPath, mode);
|
||||
await fs.rename(tmpPath, targetPath);
|
||||
}
|
||||
|
||||
export async function installTaskWorktreeIdentityGuard(input: {
|
||||
@@ -89,16 +77,10 @@ export async function installTaskWorktreeIdentityGuard(input: {
|
||||
allowedBranchPatterns?: readonly string[];
|
||||
}): Promise<void> {
|
||||
const gitDir = await resolveGitDir(input.worktreePath);
|
||||
const guard = buildIdentityGuardHook(input.taskId, input.allowedBranchPatterns ?? DEFAULT_ALLOWED_BRANCH_PATTERNS);
|
||||
|
||||
const hook = buildIdentityGuardHook(input.taskId, input.allowedBranchPatterns ?? DEFAULT_ALLOWED_BRANCH_PATTERNS);
|
||||
const metadataPath = resolve(gitDir, "fusion-task-id");
|
||||
const hookPath = resolve(gitDir, "hooks", "pre-commit");
|
||||
|
||||
await writeFileAtomic(metadataPath, `${input.taskId}\n`);
|
||||
await writeFileAtomic(hookPath, guard, 0o755);
|
||||
|
||||
const hookStat = await stat(hookPath);
|
||||
if ((hookStat.mode & 0o111) === 0) {
|
||||
await chmod(hookPath, 0o755);
|
||||
}
|
||||
await writeFileAtomic(hookPath, hook, 0o755);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user