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();
|
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 () => {
|
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
|
// Reproduces the case where the merger fast-forward-rebased the task branch
|
||||||
// and updated baseCommitSha to the new HEAD; the bounded `base..HEAD` range
|
// and updated baseCommitSha to the new HEAD; the bounded `base..HEAD` range
|
||||||
|
|||||||
@@ -1563,6 +1563,48 @@ export async function resolveConflicts(
|
|||||||
return remainingComplex;
|
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. */
|
/** Build the --author flag for git commits based on project settings. */
|
||||||
function getCommitAuthorArg(settings: {
|
function getCommitAuthorArg(settings: {
|
||||||
commitAuthorEnabled?: boolean;
|
commitAuthorEnabled?: boolean;
|
||||||
@@ -3401,8 +3443,9 @@ async function executeMergeAttempt(
|
|||||||
const escapedLog = commitLog.replace(/"/g, '\\"');
|
const escapedLog = commitLog.replace(/"/g, '\\"');
|
||||||
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
||||||
const authorArg = getCommitAuthorArg(settings);
|
const authorArg = getCommitAuthorArg(settings);
|
||||||
|
const trailerArg = buildTaskIdTrailerArg(taskId);
|
||||||
await execAsync(
|
await execAsync(
|
||||||
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${authorArg}`,
|
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${trailerArg}${authorArg}`,
|
||||||
{ cwd: rootDir },
|
{ cwd: rootDir },
|
||||||
);
|
);
|
||||||
mergerLog.log(`${taskId}: committed after auto-resolving all conflicts`);
|
mergerLog.log(`${taskId}: committed after auto-resolving all conflicts`);
|
||||||
@@ -3667,8 +3710,9 @@ async function attemptWithSideStrategy(
|
|||||||
const escapedLog = commitLog.replace(/"/g, '\\"');
|
const escapedLog = commitLog.replace(/"/g, '\\"');
|
||||||
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
||||||
const authorArg = getCommitAuthorArg(settings);
|
const authorArg = getCommitAuthorArg(settings);
|
||||||
|
const trailerArg = buildTaskIdTrailerArg(taskId);
|
||||||
await execAsync(
|
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 },
|
{ cwd: rootDir },
|
||||||
);
|
);
|
||||||
mergerLog.log(`${taskId}: committed with -X ${side} auto-resolution`);
|
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 escapedLog = commitLog.replace(/"/g, '\\"');
|
||||||
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
|
||||||
const authorArg = getCommitAuthorArg(settings);
|
const authorArg = getCommitAuthorArg(settings);
|
||||||
|
const trailerArg = buildTaskIdTrailerArg(taskId);
|
||||||
await execAsync(
|
await execAsync(
|
||||||
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${authorArg}`,
|
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"${trailerArg}${authorArg}`,
|
||||||
{ cwd: rootDir },
|
{ cwd: rootDir },
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
@@ -3974,6 +4019,12 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
|||||||
// This is an error condition - agent didn't follow instructions
|
// 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}`);
|
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 };
|
return { success: true };
|
||||||
|
|||||||
@@ -427,13 +427,56 @@ export class SelfHealingManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async findLandedTaskCommit(task: Task): Promise<LandedTaskCommit | null> {
|
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 = [
|
const command = [
|
||||||
"git log",
|
"git log",
|
||||||
"--format=%H%x1f%s",
|
"--format=%H%x1f%s",
|
||||||
"--max-count=20",
|
"--max-count=20",
|
||||||
"--fixed-strings",
|
...(fixedStrings ? ["--fixed-strings"] : ["-E"]),
|
||||||
`--grep=${shellQuote(task.id)}`,
|
`--grep=${grepArg}`,
|
||||||
shellQuote(range),
|
shellQuote(range),
|
||||||
].join(" ");
|
].join(" ");
|
||||||
|
|
||||||
@@ -443,27 +486,42 @@ export class SelfHealingManager {
|
|||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
let stdout: string;
|
// Search (2) trailer first, then (3) subject as fallback. Both share the
|
||||||
try {
|
// same range-resolution logic (bounded, then full HEAD if empty).
|
||||||
const result = await readLog(task.baseCommitSha ? `${task.baseCommitSha}..HEAD` : "HEAD");
|
const search = async (grepArg: string, fixedStrings: boolean): Promise<string> => {
|
||||||
stdout = result.stdout;
|
let out: string;
|
||||||
} catch (err: unknown) {
|
try {
|
||||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
const r = await readLog(
|
||||||
log.warn(
|
task.baseCommitSha ? `${task.baseCommitSha}..HEAD` : "HEAD",
|
||||||
`Failed to read git log for landed commit lookup (${task.id}): ${errorMessage} — retrying with HEAD range`,
|
grepArg,
|
||||||
);
|
fixedStrings,
|
||||||
if (!task.baseCommitSha) return null;
|
);
|
||||||
const result = await readLog("HEAD");
|
out = r.stdout;
|
||||||
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 "";
|
||||||
|
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
|
// (2) Trailer — anchored regex so we don't false-match ID substrings.
|
||||||
// baseCommitSha was advanced past it (e.g. the merger fast-forward-rebased
|
const trailerPattern = `^Fusion-Task-Id: ${task.id}$`;
|
||||||
// the task branch onto a newer main, or later commits moved HEAD up).
|
let stdout = await search(shellQuote(trailerPattern), false);
|
||||||
// Re-scan all of HEAD so recovery still finds the merge.
|
|
||||||
if (!stdout.trim() && task.baseCommitSha) {
|
// (3) Subject grep fallback (legacy commits).
|
||||||
const result = await readLog("HEAD");
|
if (!stdout.trim()) {
|
||||||
stdout = result.stdout;
|
stdout = await search(shellQuote(task.id), true);
|
||||||
}
|
}
|
||||||
|
|
||||||
const firstLine = stdout.trim().split("\n").find(Boolean);
|
const firstLine = stdout.trim().split("\n").find(Boolean);
|
||||||
|
|||||||
Reference in New Issue
Block a user