feat(FN-4811): complete Step 3 — update removeWorktree call sites
Fusion-Task-Id: FN-4811 Fusion-Task-Lineage: f3dac123-cb46-4d31-8a7e-dfc664bfdc5f
This commit is contained in:
committed by
gsxdsm
parent
7a438f63ee
commit
8fd47b619f
@@ -43,7 +43,7 @@ import { resolveSandboxBackend } from "./sandbox/index.js";
|
||||
import type { SandboxBackend } from "./sandbox/types.js";
|
||||
import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession } from "@mariozechner/pi-coding-agent";
|
||||
import { PRIORITY_EXECUTE, type AgentSemaphore } from "./concurrency.js";
|
||||
import { getRegisteredWorktreePaths, isGitRepository, isInsideWorktreesDir, isRegisteredGitWorktree, isUsableTaskWorktree, removeWorktree, type WorktreePool } from "./worktree-pool.js";
|
||||
import { RemovalReason, getRegisteredWorktreePaths, isGitRepository, isInsideWorktreesDir, isRegisteredGitWorktree, isUsableTaskWorktree, removeWorktree, type WorktreePool } from "./worktree-pool.js";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
import {
|
||||
BranchConflictError,
|
||||
@@ -3243,6 +3243,7 @@ export class TaskExecutor {
|
||||
settings,
|
||||
taskId: task.id,
|
||||
audit,
|
||||
reason: RemovalReason.ExecutorTransientRetry,
|
||||
});
|
||||
} catch (wtErr: unknown) {
|
||||
const msg = wtErr instanceof Error ? wtErr.message : String(wtErr);
|
||||
@@ -3328,6 +3329,7 @@ export class TaskExecutor {
|
||||
rootDir: this.rootDir,
|
||||
settings,
|
||||
taskId: task.id,
|
||||
reason: RemovalReason.ExecutorStuckKilled,
|
||||
});
|
||||
} catch (wtErr: unknown) {
|
||||
const msg = wtErr instanceof Error ? wtErr.message : String(wtErr);
|
||||
@@ -4235,6 +4237,7 @@ export class TaskExecutor {
|
||||
settings,
|
||||
taskId: task.id,
|
||||
audit,
|
||||
reason: RemovalReason.ExecutorDispose,
|
||||
});
|
||||
executorLog.log(`Removed old worktree for paused task: ${worktreePath}`);
|
||||
} catch (cleanupErr: unknown) {
|
||||
@@ -4578,6 +4581,7 @@ export class TaskExecutor {
|
||||
settings,
|
||||
taskId: task.id,
|
||||
audit,
|
||||
reason: RemovalReason.ExecutorTransientRetry,
|
||||
});
|
||||
executorLog.log(`Removed old worktree for transient retry: ${worktreePath}`);
|
||||
} catch (cleanupErr: unknown) {
|
||||
@@ -4705,6 +4709,7 @@ export class TaskExecutor {
|
||||
settings,
|
||||
taskId: task.id,
|
||||
audit,
|
||||
reason: RemovalReason.ExecutorStuckKilled,
|
||||
});
|
||||
executorLog.log(`Removed old worktree for stuck-killed retry: ${worktreePath}`);
|
||||
} catch (cleanupErr: unknown) {
|
||||
@@ -5646,6 +5651,7 @@ export class TaskExecutor {
|
||||
rootDir: this.rootDir,
|
||||
settings,
|
||||
taskId,
|
||||
reason: RemovalReason.ExecutorDispose,
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
@@ -8396,6 +8402,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
rootDir: this.rootDir,
|
||||
settings,
|
||||
taskId,
|
||||
reason: RemovalReason.ExecutorDispose,
|
||||
});
|
||||
await this.store.logEntry(taskId, `Removed conflicting worktree`, worktreePath);
|
||||
|
||||
@@ -8605,6 +8612,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
|
||||
rootDir: this.rootDir,
|
||||
settings,
|
||||
taskId,
|
||||
reason: RemovalReason.ExecutorDispose,
|
||||
});
|
||||
executorLog.log(`Cleaned up worktree for ${taskId}`);
|
||||
} catch (err: unknown) {
|
||||
|
||||
@@ -68,7 +68,8 @@ import { accumulateSessionTokenUsage } from "./session-token-usage.js";
|
||||
import { createResolvedAgentSession, extractRuntimeHint, resolveMergerSessionModel } from "./agent-session-helpers.js";
|
||||
import { createFallbackModelObserver } from "./fallback-model-observer.js";
|
||||
import { buildSessionSkillContext } from "./session-skill-context.js";
|
||||
import { removeWorktree, type WorktreePool } from "./worktree-pool.js";
|
||||
import { RemovalReason, removeWorktree, type WorktreePool } from "./worktree-pool.js";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
import { mergerLog } from "./logger.js";
|
||||
import { isUsageLimitError, checkSessionError, type UsageLimitPauser } from "./usage-limit-detector.js";
|
||||
@@ -5843,6 +5844,7 @@ async function removePostMergeWorktree(
|
||||
worktreePath: postMergeWorktree,
|
||||
settings,
|
||||
taskId,
|
||||
reason: RemovalReason.MergerPostMerge,
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
mergerLog.warn(`${taskId}: failed to remove post-merge worktree ${postMergeWorktree}: ${getCommandErrorMessage(err)}`);
|
||||
@@ -7653,19 +7655,31 @@ export async function aiMergeTask(
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
await removeWorktree({
|
||||
rootDir,
|
||||
worktreePath,
|
||||
settings,
|
||||
taskId,
|
||||
audit,
|
||||
});
|
||||
result.worktreeRemoved = true;
|
||||
try {
|
||||
await store.updateTask(taskId, { worktree: null, branch: null });
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
mergerLog.warn(`${taskId}: failed to clear worktree pointer after removal: ${msg}`);
|
||||
if (activeSessionRegistry.isPathActive(worktreePath)) {
|
||||
mergerLog.warn(`${taskId}: skipping worktree cleanup for active session path ${worktreePath}`);
|
||||
await audit?.git({
|
||||
type: "worktree:removal-refused-active-session",
|
||||
target: worktreePath,
|
||||
metadata: { taskId, reason: RemovalReason.MergerCleanup, kind: "merger" },
|
||||
});
|
||||
} else {
|
||||
await removeWorktree({
|
||||
rootDir,
|
||||
worktreePath,
|
||||
settings,
|
||||
taskId,
|
||||
audit,
|
||||
reason: RemovalReason.MergerCleanup,
|
||||
});
|
||||
result.worktreeRemoved = true;
|
||||
}
|
||||
if (result.worktreeRemoved) {
|
||||
try {
|
||||
await store.updateTask(taskId, { worktree: null, branch: null });
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
mergerLog.warn(`${taskId}: failed to clear worktree pointer after removal: ${msg}`);
|
||||
}
|
||||
}
|
||||
} catch { /* non-fatal */ }
|
||||
}
|
||||
|
||||
@@ -29,7 +29,7 @@ import { isAbsolute, join, relative, resolve } from "node:path";
|
||||
import { getInReviewStallReason, getStalePausedReviewSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority } from "@fusion/core";
|
||||
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
||||
import { createLogger } from "./logger.js";
|
||||
import { getRegisteredWorktreePaths, isUsableTaskWorktree, removeWorktree, resolveWorktreeBackend, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js";
|
||||
import { RemovalReason, getRegisteredWorktreePaths, isUsableTaskWorktree, removeWorktree, resolveWorktreeBackend, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js";
|
||||
import {
|
||||
extractMissingWorktreePathFromSessionStartFailure,
|
||||
isMissingWorktreeSessionStartFailure,
|
||||
@@ -896,6 +896,7 @@ export class SelfHealingManager {
|
||||
worktreePath: task.worktree,
|
||||
settings,
|
||||
taskId: task.id,
|
||||
reason: RemovalReason.SelfHealingReclaim,
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
@@ -915,6 +916,7 @@ export class SelfHealingManager {
|
||||
worktreePath: task.worktree,
|
||||
settings,
|
||||
taskId: task.id,
|
||||
reason: RemovalReason.SelfHealingReclaim,
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
@@ -1419,6 +1421,7 @@ export class SelfHealingManager {
|
||||
worktreePath: inspection.livePath,
|
||||
settings,
|
||||
taskId: task.id,
|
||||
reason: RemovalReason.SelfHealingBranchConflict,
|
||||
});
|
||||
}
|
||||
// Branch-level reclaim remains active in worktrunk mode; this is
|
||||
@@ -1519,6 +1522,7 @@ export class SelfHealingManager {
|
||||
worktreePath: inspection.livePath,
|
||||
settings,
|
||||
taskId: task.id,
|
||||
reason: RemovalReason.SelfHealingBranchConflict,
|
||||
});
|
||||
// Branch-level reclaim remains active in worktrunk mode; this is
|
||||
// idempotent git metadata cleanup, not layout ownership.
|
||||
@@ -1993,6 +1997,7 @@ export class SelfHealingManager {
|
||||
worktreePath,
|
||||
settings,
|
||||
taskId,
|
||||
reason: RemovalReason.SelfHealingStaleActiveBranch,
|
||||
});
|
||||
result.worktreeRemoved = true;
|
||||
} catch (err: unknown) {
|
||||
@@ -3765,6 +3770,7 @@ export class SelfHealingManager {
|
||||
worktreePath: task.worktree,
|
||||
settings,
|
||||
taskId: task.id,
|
||||
reason: RemovalReason.SelfHealingOrphanRescue,
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
|
||||
@@ -4967,6 +4973,7 @@ export class SelfHealingManager {
|
||||
rootDir: this.options.rootDir,
|
||||
worktreePath,
|
||||
settings,
|
||||
reason: RemovalReason.SelfHealingIdleSweep,
|
||||
});
|
||||
cleaned++;
|
||||
} catch (err: unknown) {
|
||||
@@ -5301,6 +5308,7 @@ export class SelfHealingManager {
|
||||
rootDir: this.options.rootDir,
|
||||
worktreePath,
|
||||
settings,
|
||||
reason: RemovalReason.SelfHealingIdleSweep,
|
||||
});
|
||||
removed++;
|
||||
} catch (err: unknown) {
|
||||
|
||||
@@ -48,7 +48,7 @@ import {
|
||||
createTaskDocumentWriteTool,
|
||||
createTaskLogTool,
|
||||
} from "./agent-tools.js";
|
||||
import { removeWorktree } from "./worktree-backend.js";
|
||||
import { RemovalReason, removeWorktree } from "./worktree-backend.js";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
|
||||
const stepExecLog = createLogger("step-session-executor");
|
||||
@@ -793,6 +793,7 @@ export class StepSessionExecutor {
|
||||
rootDir: this.options.rootDir,
|
||||
settings: this.options.settings,
|
||||
taskId: this.options.taskDetail.id,
|
||||
reason: RemovalReason.StepSessionCleanup,
|
||||
});
|
||||
} else {
|
||||
stepExecLog.warn(`Parallel worktree for step ${stepIdx} already removed: ${worktreePath}`);
|
||||
@@ -1285,11 +1286,13 @@ Follow instructions precisely and avoid unrelated changes.`,
|
||||
if (worktreePath !== this.options.worktreePath) {
|
||||
try {
|
||||
if (existsSync(worktreePath)) {
|
||||
this.unregisterParallelWorktree(stepIdx);
|
||||
await removeWorktree({
|
||||
worktreePath,
|
||||
rootDir: this.options.rootDir,
|
||||
settings: this.options.settings,
|
||||
taskId: this.options.taskDetail.id,
|
||||
reason: RemovalReason.StepSessionCleanup,
|
||||
});
|
||||
}
|
||||
const branch = this.parallelBranches.get(stepIdx);
|
||||
|
||||
@@ -4,6 +4,7 @@ import { access } from "node:fs/promises";
|
||||
import { basename, resolve } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import type { Settings } from "@fusion/core";
|
||||
import { activeSessionRegistry } from "./active-session-registry.js";
|
||||
import type { RunAuditor } from "./run-audit.js";
|
||||
import { resolveTaskWorktreePath } from "./worktree-paths.js";
|
||||
import { inspectBranchConflict } from "./branch-conflicts.js";
|
||||
@@ -524,10 +525,60 @@ export class WorktrunkWorktreeBackend implements WorktreeBackend {
|
||||
}
|
||||
}
|
||||
|
||||
export const RemovalReason = {
|
||||
HardCancel: "hard-cancel",
|
||||
ExecutorTransientRetry: "executor-transient-retry",
|
||||
ExecutorStuckKilled: "executor-stuck-killed",
|
||||
ExecutorDispose: "executor-dispose",
|
||||
StepSessionCleanup: "step-session-cleanup",
|
||||
MergerPostMerge: "merger-post-merge",
|
||||
MergerCleanup: "merger-cleanup",
|
||||
SelfHealingReclaim: "self-healing-reclaim",
|
||||
SelfHealingStaleActiveBranch: "self-healing-stale-active-branch",
|
||||
SelfHealingBranchConflict: "self-healing-branch-conflict",
|
||||
SelfHealingOrphanRescue: "self-healing-orphan-rescue",
|
||||
SelfHealingIdleSweep: "self-healing-idle-sweep",
|
||||
PoolPrune: "pool-prune",
|
||||
} as const;
|
||||
|
||||
export type RemovalReason = typeof RemovalReason[keyof typeof RemovalReason];
|
||||
|
||||
const ALLOWED_FORCE_REASONS = new Set<RemovalReason>([
|
||||
RemovalReason.HardCancel,
|
||||
RemovalReason.ExecutorDispose,
|
||||
RemovalReason.ExecutorTransientRetry,
|
||||
RemovalReason.ExecutorStuckKilled,
|
||||
]);
|
||||
|
||||
export class InvalidForceUsageError extends Error {
|
||||
constructor(reason: RemovalReason) {
|
||||
super(`force=true is not allowed for removal reason '${reason}'`);
|
||||
this.name = "InvalidForceUsageError";
|
||||
}
|
||||
}
|
||||
|
||||
export class ActiveSessionWorktreeRemovalError extends Error {
|
||||
constructor(public readonly details: {
|
||||
worktreePath: string;
|
||||
taskId: string;
|
||||
kind: string;
|
||||
ownerKey: string;
|
||||
reason: RemovalReason;
|
||||
}) {
|
||||
super(`cannot remove active-session worktree ${details.worktreePath} (${details.taskId}/${details.kind})`);
|
||||
this.name = "ActiveSessionWorktreeRemovalError";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a worktree via configured backend.
|
||||
* Only executor-owned hard-cancel/dispose paths may use force=true.
|
||||
*/
|
||||
export async function removeWorktree(input: {
|
||||
worktreePath: string;
|
||||
rootDir: string;
|
||||
settings: Partial<Settings>;
|
||||
reason: RemovalReason;
|
||||
taskId?: string;
|
||||
audit?: RunAuditor;
|
||||
force?: boolean;
|
||||
@@ -538,6 +589,34 @@ export async function removeWorktree(input: {
|
||||
warn: (_message: string): void => {},
|
||||
};
|
||||
|
||||
if (input.force === true && !ALLOWED_FORCE_REASONS.has(input.reason)) {
|
||||
throw new InvalidForceUsageError(input.reason);
|
||||
}
|
||||
|
||||
const active = activeSessionRegistry.lookupByPath(input.worktreePath);
|
||||
if (active && input.force !== true) {
|
||||
await input.audit?.git({
|
||||
type: "worktree:removal-refused-active-session",
|
||||
target: input.worktreePath,
|
||||
metadata: { taskId: active.taskId, reason: input.reason, kind: active.kind },
|
||||
});
|
||||
throw new ActiveSessionWorktreeRemovalError({
|
||||
worktreePath: input.worktreePath,
|
||||
taskId: active.taskId,
|
||||
kind: active.kind,
|
||||
ownerKey: active.ownerKey,
|
||||
reason: input.reason,
|
||||
});
|
||||
}
|
||||
|
||||
if (active && input.force === true) {
|
||||
await input.audit?.git({
|
||||
type: "worktree:removal-forced-over-active-session",
|
||||
target: input.worktreePath,
|
||||
metadata: { taskId: active.taskId, reason: input.reason, kind: active.kind },
|
||||
});
|
||||
}
|
||||
|
||||
const backend = resolveWorktreeBackend(input.settings, { logger });
|
||||
const removeInput: WorktreeRemoveInput = {
|
||||
rootDir: input.rootDir,
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
resolveWorktrunkBinary,
|
||||
} from "./worktrunk-installer.js";
|
||||
import {
|
||||
RemovalReason,
|
||||
removeWorktree as removeWorktreeViaBackend,
|
||||
resolveWorktreeBackend as resolveWorktreeBackendViaSettings,
|
||||
} from "./worktree-backend.js";
|
||||
@@ -21,7 +22,8 @@ export {
|
||||
removeWorktree,
|
||||
resolveWorktreeBackend,
|
||||
} from "./worktree-backend.js";
|
||||
export type { WorktreeBackend, WorktreeBackendKind } from "./worktree-backend.js";
|
||||
export type { WorktreeBackend, WorktreeBackendKind, RemovalReason } from "./worktree-backend.js";
|
||||
export { RemovalReason } from "./worktree-backend.js";
|
||||
|
||||
// Re-export worktrunk installer types for convenience.
|
||||
export {
|
||||
@@ -535,6 +537,7 @@ export async function cleanupOrphanedWorktrees(
|
||||
rootDir,
|
||||
worktreePath,
|
||||
settings: settings ?? {},
|
||||
reason: RemovalReason.PoolPrune,
|
||||
});
|
||||
} else {
|
||||
if (!isInsideWorktreesDir(rootDir, worktreePath, settings)) {
|
||||
|
||||
Reference in New Issue
Block a user