fix(engine): make stale-merge recovery robust to includeTaskIdInCommit=false
recoverInterruptedMergingTasks searched for landed commits by grepping commit subjects for the task ID. Users with includeTaskIdInCommit=false have commit subjects like `feat: ...` (no task ID), so if the merger crashed after committing but before storing mergeDetails, recovery would silently fail to find the commit and incorrectly retry the merge. Three layered defenses: 1. Emit a Fusion-Task-Id: <id> trailer in every Fusion-managed merge commit body. The 4 fallback commit invocations now include `-m "Fusion-Task-Id: ..."`. After the AI agent commits, an idempotent ensureTaskIdTrailerOnHead() amends the trailer in via `git interpret-trailers` (no-op if already present). 2. findLandedTaskCommit now tries three sources in order: a. task.mergeDetails.commitSha (if reachable from HEAD) b. Fusion-Task-Id trailer grep (anchored regex) c. Subject grep (legacy commits) 3. Trailer grep uses an anchored regex `^Fusion-Task-Id: <id>$` so it doesn't false-match task IDs appearing as substrings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1254,6 +1254,68 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("finds landed commit via Fusion-Task-Id trailer when subject lacks the task ID", async () => {
|
||||
// includeTaskIdInCommit=false: commit subject is `feat: ...` with no
|
||||
// task ID. Recovery must locate the commit via the trailer in the body.
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
taskStuckTimeoutMs: 60_000,
|
||||
});
|
||||
const staleUpdatedAt = new Date(Date.now() - 61_000).toISOString();
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-2900",
|
||||
column: "in-review",
|
||||
status: "merging",
|
||||
error: null,
|
||||
paused: false,
|
||||
worktree: "/tmp/test-project/.worktrees/fn-2900",
|
||||
branch: "fusion/fn-2900",
|
||||
baseCommitSha: "base999",
|
||||
updatedAt: staleUpdatedAt,
|
||||
steps: [{ name: "Ship it", status: "done" }],
|
||||
workflowStepResults: [],
|
||||
mergeDetails: undefined,
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
mockedExecSync.mockImplementation((command) => {
|
||||
const cmd = String(command);
|
||||
if (cmd.includes("git log")) {
|
||||
// Recovery searches by trailer first; only the trailer-grep result
|
||||
// returns a match. Subject grep would be empty (no task ID in subj).
|
||||
if (cmd.includes("Fusion-Task-Id: FN-2900")) {
|
||||
return "trailerSha123feat: ship something opaque\n" as any;
|
||||
}
|
||||
if (cmd.includes("--fixed-strings")) return "" as any;
|
||||
}
|
||||
if (cmd.includes("git show --shortstat")) {
|
||||
return " 2 files changed, 5 insertions(+), 1 deletion(-)\n" as any;
|
||||
}
|
||||
return "" as any;
|
||||
});
|
||||
|
||||
const result = await managerWithRecovery.recoverInterruptedMergingTasks();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-2900", {
|
||||
status: null,
|
||||
error: null,
|
||||
mergeRetries: 0,
|
||||
mergeDetails: expect.objectContaining({
|
||||
commitSha: "trailerSha123",
|
||||
mergeConfirmed: true,
|
||||
}),
|
||||
});
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-2900", "done");
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("finalizes stale merging tasks when baseCommitSha was advanced past the landed commit", async () => {
|
||||
// Reproduces the case where the merger fast-forward-rebased the task branch
|
||||
// and updated baseCommitSha to the new HEAD; the bounded `base..HEAD` range
|
||||
|
||||
@@ -1563,6 +1563,48 @@ export async function resolveConflicts(
|
||||
return remainingComplex;
|
||||
}
|
||||
|
||||
/** Trailer key written into every Fusion-managed merge commit body. Used by
|
||||
* recovery (findLandedTaskCommit) to identify a task's commit even when the
|
||||
* configured commit subject doesn't include the task ID
|
||||
* (`includeTaskIdInCommit: false`). */
|
||||
export const FUSION_TASK_ID_TRAILER_KEY = "Fusion-Task-Id";
|
||||
|
||||
/** Build the `-m "Fusion-Task-Id: <id>"` arg fragment used in fallback commit
|
||||
* invocations. Returns a leading space + quoted -m arg. */
|
||||
function buildTaskIdTrailerArg(taskId: string): string {
|
||||
// Task IDs are constrained ([A-Z]+-[0-9]+) so embedding directly is safe.
|
||||
return ` -m "${FUSION_TASK_ID_TRAILER_KEY}: ${taskId}"`;
|
||||
}
|
||||
|
||||
/** Idempotently add the Fusion-Task-Id trailer to HEAD's commit. Used after
|
||||
* the AI agent commits to guarantee the trailer is present even when the
|
||||
* agent didn't include it (especially under includeTaskIdInCommit=false,
|
||||
* where the subject also lacks the task ID and recovery has nothing to
|
||||
* grep against). No-op if the trailer is already on HEAD. */
|
||||
async function ensureTaskIdTrailerOnHead(rootDir: string, taskId: string): Promise<void> {
|
||||
try {
|
||||
const { stdout: existingMessage } = await execAsync("git log -1 --pretty=%B", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
const trailerLine = `${FUSION_TASK_ID_TRAILER_KEY}: ${taskId}`;
|
||||
if (existingMessage.includes(trailerLine)) return;
|
||||
// git interpret-trailers is the canonical way to add trailers without
|
||||
// disturbing the rest of the body. --if-exists addIfDifferentNeighbor
|
||||
// ensures we don't double-up if a slightly different trailer is present.
|
||||
await execAsync(
|
||||
`git -c trailer.ifExists=addIfDifferent commit --amend --no-edit --trailer "${trailerLine}"`,
|
||||
{ cwd: rootDir },
|
||||
);
|
||||
} catch (err) {
|
||||
// Best-effort: if amending fails (detached HEAD, sign-off conflict, etc.)
|
||||
// we still recorded mergeDetails further on. Recovery will fall back to
|
||||
// subject grep. Don't surface this as a merge failure.
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
mergerLog.warn(`${taskId}: failed to add ${FUSION_TASK_ID_TRAILER_KEY} trailer to HEAD (${msg}) — relying on subject grep for recovery`);
|
||||
}
|
||||
}
|
||||
|
||||
/** Build the --author flag for git commits based on project settings. */
|
||||
function getCommitAuthorArg(settings: {
|
||||
commitAuthorEnabled?: boolean;
|
||||
@@ -3401,8 +3443,9 @@ async function executeMergeAttempt(
|
||||
const escapedLog = commitLog.replace(/"/g, '\\"');
|
||||
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
||||
const authorArg = getCommitAuthorArg(settings);
|
||||
const trailerArg = buildTaskIdTrailerArg(taskId);
|
||||
await execAsync(
|
||||
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${authorArg}`,
|
||||
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${trailerArg}${authorArg}`,
|
||||
{ cwd: rootDir },
|
||||
);
|
||||
mergerLog.log(`${taskId}: committed after auto-resolving all conflicts`);
|
||||
@@ -3667,8 +3710,9 @@ async function attemptWithSideStrategy(
|
||||
const escapedLog = commitLog.replace(/"/g, '\\"');
|
||||
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
||||
const authorArg = getCommitAuthorArg(settings);
|
||||
const trailerArg = buildTaskIdTrailerArg(taskId);
|
||||
await execAsync(
|
||||
`git commit -m "${fallbackPrefix}: merge ${branch} (auto-resolved)" -m "${escapedLog}"${authorArg}`,
|
||||
`git commit -m "${fallbackPrefix}: merge ${branch} (auto-resolved)" -m "${escapedLog}"${trailerArg}${authorArg}`,
|
||||
{ cwd: rootDir },
|
||||
);
|
||||
mergerLog.log(`${taskId}: committed with -X ${side} auto-resolution`);
|
||||
@@ -3965,8 +4009,9 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
const escapedLog = commitLog.replace(/"/g, '\\"');
|
||||
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
||||
const authorArg = getCommitAuthorArg(settings);
|
||||
const trailerArg = buildTaskIdTrailerArg(taskId);
|
||||
await execAsync(
|
||||
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${authorArg}`,
|
||||
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${trailerArg}${authorArg}`,
|
||||
{ cwd: rootDir },
|
||||
);
|
||||
} else {
|
||||
@@ -3974,6 +4019,12 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
// This is an error condition - agent didn't follow instructions
|
||||
throw new Error(`Agent did not commit and did not report build failure for ${taskId}`);
|
||||
}
|
||||
} else {
|
||||
// The agent committed. Idempotently ensure the Fusion-Task-Id trailer
|
||||
// is present on HEAD — recovery (findLandedTaskCommit) relies on it
|
||||
// when includeTaskIdInCommit=false, since the subject won't carry the
|
||||
// task ID and subject grep would miss the commit.
|
||||
await ensureTaskIdTrailerOnHead(rootDir, taskId);
|
||||
}
|
||||
|
||||
return { success: true };
|
||||
|
||||
@@ -427,13 +427,56 @@ export class SelfHealingManager {
|
||||
}
|
||||
|
||||
private async findLandedTaskCommit(task: Task): Promise<LandedTaskCommit | null> {
|
||||
const readLog = async (range: string) => {
|
||||
// Search strategies, tried in order of reliability:
|
||||
// 1. mergeDetails.commitSha — already stored by the merger; verify it's
|
||||
// reachable from HEAD before trusting it.
|
||||
// 2. Fusion-Task-Id trailer — emitted into every Fusion-managed merge
|
||||
// commit body; survives `includeTaskIdInCommit: false`.
|
||||
// 3. Subject grep — legacy/AI commits where the task ID lives in the
|
||||
// subject line (e.g. `feat(FN-123): …`).
|
||||
//
|
||||
// (1) gives us the right sha even if the commit subject is exotic; (2)
|
||||
// covers includeTaskIdInCommit=false setups where (3) would silently
|
||||
// miss; (3) catches commits authored before the trailer was introduced.
|
||||
|
||||
// ── (1) Stored sha ────────────────────────────────────────────────────
|
||||
const storedSha = task.mergeDetails?.commitSha;
|
||||
if (storedSha) {
|
||||
try {
|
||||
// Reachable from HEAD? Use --quiet --exit-code on rev-list.
|
||||
await execAsync(
|
||||
`git merge-base --is-ancestor ${shellQuote(storedSha)} HEAD`,
|
||||
{ cwd: this.options.rootDir },
|
||||
);
|
||||
// Yes — fetch its subject + stats.
|
||||
const { stdout } = await execAsync(
|
||||
`git log -1 --format=%H%x1f%s ${shellQuote(storedSha)}`,
|
||||
{ cwd: this.options.rootDir, maxBuffer: 1024 * 1024 },
|
||||
);
|
||||
const [sha, subject] = stdout.trim().split("\x1f");
|
||||
if (sha) {
|
||||
const commit: LandedTaskCommit = { sha, subject };
|
||||
try {
|
||||
const stats = await execAsync(`git show --shortstat --format= ${shellQuote(sha)}`, {
|
||||
cwd: this.options.rootDir,
|
||||
maxBuffer: 1024 * 1024,
|
||||
});
|
||||
Object.assign(commit, parseShortstat(stats.stdout));
|
||||
} catch { /* stats are optional */ }
|
||||
return commit;
|
||||
}
|
||||
} catch {
|
||||
// Not reachable (rebased away, branch reset, etc.) — fall through.
|
||||
}
|
||||
}
|
||||
|
||||
const readLog = async (range: string, grepArg: string, fixedStrings: boolean) => {
|
||||
const command = [
|
||||
"git log",
|
||||
"--format=%H%x1f%s",
|
||||
"--max-count=20",
|
||||
"--fixed-strings",
|
||||
`--grep=${shellQuote(task.id)}`,
|
||||
...(fixedStrings ? ["--fixed-strings"] : ["-E"]),
|
||||
`--grep=${grepArg}`,
|
||||
shellQuote(range),
|
||||
].join(" ");
|
||||
|
||||
@@ -443,27 +486,42 @@ export class SelfHealingManager {
|
||||
});
|
||||
};
|
||||
|
||||
let stdout: string;
|
||||
try {
|
||||
const result = await readLog(task.baseCommitSha ? `${task.baseCommitSha}..HEAD` : "HEAD");
|
||||
stdout = result.stdout;
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to read git log for landed commit lookup (${task.id}): ${errorMessage} — retrying with HEAD range`,
|
||||
);
|
||||
if (!task.baseCommitSha) return null;
|
||||
const result = await readLog("HEAD");
|
||||
stdout = result.stdout;
|
||||
}
|
||||
// Search (2) trailer first, then (3) subject as fallback. Both share the
|
||||
// same range-resolution logic (bounded, then full HEAD if empty).
|
||||
const search = async (grepArg: string, fixedStrings: boolean): Promise<string> => {
|
||||
let out: string;
|
||||
try {
|
||||
const r = await readLog(
|
||||
task.baseCommitSha ? `${task.baseCommitSha}..HEAD` : "HEAD",
|
||||
grepArg,
|
||||
fixedStrings,
|
||||
);
|
||||
out = r.stdout;
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to read git log for landed commit lookup (${task.id}): ${errorMessage} — retrying with HEAD range`,
|
||||
);
|
||||
if (!task.baseCommitSha) return "";
|
||||
const r = await readLog("HEAD", grepArg, fixedStrings);
|
||||
out = r.stdout;
|
||||
}
|
||||
// Bounded range may exclude the landed commit when baseCommitSha was
|
||||
// advanced past it; re-scan all of HEAD if empty.
|
||||
if (!out.trim() && task.baseCommitSha) {
|
||||
const r = await readLog("HEAD", grepArg, fixedStrings);
|
||||
out = r.stdout;
|
||||
}
|
||||
return out;
|
||||
};
|
||||
|
||||
// The bounded `baseCommitSha..HEAD` range excludes the landed commit when
|
||||
// baseCommitSha was advanced past it (e.g. the merger fast-forward-rebased
|
||||
// the task branch onto a newer main, or later commits moved HEAD up).
|
||||
// Re-scan all of HEAD so recovery still finds the merge.
|
||||
if (!stdout.trim() && task.baseCommitSha) {
|
||||
const result = await readLog("HEAD");
|
||||
stdout = result.stdout;
|
||||
// (2) Trailer — anchored regex so we don't false-match ID substrings.
|
||||
const trailerPattern = `^Fusion-Task-Id: ${task.id}$`;
|
||||
let stdout = await search(shellQuote(trailerPattern), false);
|
||||
|
||||
// (3) Subject grep fallback (legacy commits).
|
||||
if (!stdout.trim()) {
|
||||
stdout = await search(shellQuote(task.id), true);
|
||||
}
|
||||
|
||||
const firstLine = stdout.trim().split("\n").find(Boolean);
|
||||
|
||||
Reference in New Issue
Block a user