fix(engine): recoverable worktree failures + prevent nested/gitlink worktrees
Fixes two classes of task failures found while investigating stuck in-review tasks FN-2165 (worktree base ref missing) and FN-2152 (stray .tmp-fn-2152 gitlink accidentally committed via merger amend). FN-2165 — stale baseBranch: - resolveWorktreeStartPoint now returns null instead of throwing NonRetryableWorktreeError when the stored baseBranch is gone. Caller clears task.baseBranch and falls back to branching from the default base (HEAD) so the task self-heals instead of failing permanently. - New TaskStore.clearStaleBaseBranchReferences() nulls baseBranch on any dependent task when its upstream branch is deleted. Wired into cleanupBranchForTask (archive/delete), merger branch cleanup, self-healing orphan-branch sweep, executor dep-abort and conflict-cleanup paths, and stale-branch recovery. Nested worktrees: - assertWorktreePathNotNested guard in tryCreateWorktree refuses to create a worktree inside another registered worktree (previously produced pathological paths like .worktrees/green-finch/.worktrees/amber-panda when rootDir pointed at a worktree instead of the main repo). Context-overflow recovery (FN-2182 class): - Reduced-prompt retry budget raised from 1 → 3 within the same session. - Adds a fresh-session requeue path when same-session retries still overflow: task moves back to todo with worktree retained, bounded by computeRecoveryDecision / MAX_RECOVERY_RETRIES. Prevents late-step context exhaustion from becoming terminal. Gitlink prevention (FN-2152 class): - .gitignore now excludes .tmp-fn-* and .tmp-kb-* so stray worktrees at the repo root cannot be captured by git add -A. - Merger amend flow now scans staged entries for 160000 gitlinks and unstages them with a loud warning; the project uses no submodules, so any such entry is a bug (this is how f8f90f26 landed in HEAD as .tmp-fn-2152). Tests: new coverage for baseBranch fallback, nested-worktree guard, and clearStaleBaseBranchReferences. Full engine + core + dashboard + cli suites pass (15349 tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -8852,4 +8852,60 @@ describe("RunMutationContext", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("clearStaleBaseBranchReferences (FN-2165)", () => {
|
||||
it("nulls baseBranch on live tasks that reference a deleted branch", async () => {
|
||||
const upstream = await store.createTask({ description: "Upstream" });
|
||||
const dependent = await store.createTask({ description: "Dependent" });
|
||||
await store.updateTask(dependent.id, {
|
||||
baseBranch: `fusion/${upstream.id.toLowerCase()}-2`,
|
||||
});
|
||||
|
||||
const cleared = store.clearStaleBaseBranchReferences([
|
||||
`fusion/${upstream.id.toLowerCase()}-2`,
|
||||
]);
|
||||
|
||||
expect(cleared).toEqual([dependent.id]);
|
||||
const reloaded = await store.getTask(dependent.id);
|
||||
expect(reloaded.baseBranch).toBeUndefined();
|
||||
});
|
||||
|
||||
it("excludes the owner task so archival doesn't null its own baseBranch", async () => {
|
||||
const upstream = await store.createTask({ description: "Upstream" });
|
||||
await store.updateTask(upstream.id, { baseBranch: "fusion/some-base" });
|
||||
|
||||
const cleared = store.clearStaleBaseBranchReferences(
|
||||
["fusion/some-base"],
|
||||
upstream.id,
|
||||
);
|
||||
|
||||
expect(cleared).toEqual([]);
|
||||
const reloaded = await store.getTask(upstream.id);
|
||||
expect(reloaded.baseBranch).toBe("fusion/some-base");
|
||||
});
|
||||
|
||||
it("returns [] and is a no-op when no branches given", () => {
|
||||
expect(store.clearStaleBaseBranchReferences([])).toEqual([]);
|
||||
});
|
||||
|
||||
it("clears baseBranch on multiple dependents in one call", async () => {
|
||||
const [a, b, c] = await Promise.all([
|
||||
store.createTask({ description: "A" }),
|
||||
store.createTask({ description: "B" }),
|
||||
store.createTask({ description: "C" }),
|
||||
]);
|
||||
await store.updateTask(a.id, { baseBranch: "fusion/gone-a" });
|
||||
await store.updateTask(b.id, { baseBranch: "fusion/gone-b" });
|
||||
await store.updateTask(c.id, { baseBranch: "fusion/still-alive" });
|
||||
|
||||
const cleared = store.clearStaleBaseBranchReferences([
|
||||
"fusion/gone-a",
|
||||
"fusion/gone-b",
|
||||
]);
|
||||
|
||||
expect(cleared.sort()).toEqual([a.id, b.id].sort());
|
||||
const cReloaded = await store.getTask(c.id);
|
||||
expect(cReloaded.baseBranch).toBe("fusion/still-alive");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2926,9 +2926,59 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
deleted.push(branch);
|
||||
}
|
||||
}
|
||||
if (deleted.length > 0) {
|
||||
this.clearStaleBaseBranchReferences(deleted, task.id);
|
||||
}
|
||||
return deleted;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear `baseBranch` on any live task whose stored value matches one of the
|
||||
* provided (now-deleted) branch names. Prevents the scenario where a
|
||||
* dependent task was dispatched with baseBranch set to an upstream dep's
|
||||
* conflict-suffixed branch, the upstream dep was later merged and its
|
||||
* branch deleted, and the dependent task then failed permanently trying
|
||||
* to create a worktree from the vanished ref (FN-2165).
|
||||
*
|
||||
* Excludes the owner task (when provided) so a task's own archival doesn't
|
||||
* null its own baseBranch.
|
||||
*
|
||||
* @returns IDs of tasks whose baseBranch was cleared
|
||||
*/
|
||||
clearStaleBaseBranchReferences(deletedBranches: string[], ownerTaskId?: string): string[] {
|
||||
if (deletedBranches.length === 0) return [];
|
||||
const placeholders = deletedBranches.map(() => "?").join(",");
|
||||
const params: string[] = [...deletedBranches];
|
||||
let whereClause = `baseBranch IN (${placeholders})`;
|
||||
if (ownerTaskId) {
|
||||
whereClause += ` AND id != ?`;
|
||||
params.push(ownerTaskId);
|
||||
}
|
||||
const rows = this.db
|
||||
.prepare(`SELECT id FROM tasks WHERE ${whereClause}`)
|
||||
.all(...params) as Array<{ id: string }>;
|
||||
|
||||
if (rows.length === 0) return [];
|
||||
const update = this.db.prepare(
|
||||
`UPDATE tasks SET baseBranch = NULL, updatedAt = ? WHERE id = ?`,
|
||||
);
|
||||
const now = new Date().toISOString();
|
||||
const clearedIds: string[] = [];
|
||||
for (const row of rows) {
|
||||
update.run(now, row.id);
|
||||
clearedIds.push(row.id);
|
||||
if (this.isWatching) {
|
||||
const cached = this.taskCache.get(row.id);
|
||||
if (cached) {
|
||||
cached.baseBranch = undefined;
|
||||
cached.updatedAt = now;
|
||||
}
|
||||
}
|
||||
}
|
||||
this.db.bumpLastModified();
|
||||
return clearedIds;
|
||||
}
|
||||
|
||||
private async collectMergeDetails(_id: string, _branch: string, task: Task, commitMessage: string): Promise<import("./types.js").MergeDetails> {
|
||||
const mergedAt = new Date().toISOString();
|
||||
let commitSha: string | undefined;
|
||||
|
||||
Reference in New Issue
Block a user