fix(FN-7226): land graph step-session commits

This commit is contained in:
gsxdsm
2026-06-29 03:15:30 -07:00
parent 399a4a2a5f
commit 50ccd79b9f
6 changed files with 92 additions and 37 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Preserve files changed by workflow-owned parallel step sessions on task branches.
category: fix
dev: Step-session cherry-pick now uses merge-base ranges and skips empty cherry-picks instead of dropping real step commits.

View File

@@ -63,6 +63,7 @@ describe("TaskStore.updateStep step-order guard", () => {
// runnable; TaskStore must not invent a hidden previous-step dependency. // runnable; TaskStore must not invent a hidden previous-step dependency.
const store = harness.store(); const store = harness.store();
const task = await harness.createTaskWithSteps(); const task = await harness.createTaskWithSteps();
await store.updateStep(task.id, 0, "pending");
await store.updateStep(task.id, 1, "in-progress", { source: "graph" }); await store.updateStep(task.id, 1, "in-progress", { source: "graph" });
const updated = await store.updateStep(task.id, 2, "done", { source: "graph" }); const updated = await store.updateStep(task.id, 2, "done", { source: "graph" });
@@ -75,7 +76,9 @@ describe("TaskStore.updateStep step-order guard", () => {
it("graph source: explicit dependsOn still suppresses completion until dependencies finish", async () => { it("graph source: explicit dependsOn still suppresses completion until dependencies finish", async () => {
const store = harness.store(); const store = harness.store();
const task = await harness.createTaskWithSteps(); const task = await harness.createTaskWithSteps();
const steps = task.steps.map((s, i) => (i === 2 ? { ...s, dependsOn: [1] } : { ...s })); await store.updateStep(task.id, 0, "pending");
const primed = await store.getTask(task.id);
const steps = primed.steps.map((s, i) => (i === 2 ? { ...s, dependsOn: [1] } : { ...s }));
await store.updateTask(task.id, { steps }); await store.updateTask(task.id, { steps });
await store.updateStep(task.id, 1, "in-progress", { source: "graph" }); await store.updateStep(task.id, 1, "in-progress", { source: "graph" });
@@ -90,7 +93,9 @@ describe("TaskStore.updateStep step-order guard", () => {
it("graph source: out-of-order done (unmet dependency) is suppressed AND audited loudly", async () => { it("graph source: out-of-order done (unmet dependency) is suppressed AND audited loudly", async () => {
const store = harness.store(); const store = harness.store();
const task = await harness.createTaskWithSteps(); const task = await harness.createTaskWithSteps();
const steps = task.steps.map((s, i) => (i === 1 ? { ...s, dependsOn: [0] } : { ...s })); await store.updateStep(task.id, 0, "pending");
const primed = await store.getTask(task.id);
const steps = primed.steps.map((s, i) => (i === 1 ? { ...s, dependsOn: [0] } : { ...s }));
await store.updateTask(task.id, { steps }); await store.updateTask(task.id, { steps });
await store.updateStep(task.id, 1, "in-progress"); await store.updateStep(task.id, 1, "in-progress");

View File

@@ -719,7 +719,7 @@ Some freeform text without checkboxes.`;
it("does not ask graph-owned step sessions to call task lifecycle tools", () => { it("does not ask graph-owned step sessions to call task lifecycle tools", () => {
const task = makeTaskDetail({ prompt: fullPrompt }); const task = makeTaskDetail({ prompt: fullPrompt });
const result = buildStepPrompt(task, 1); const result = buildStepPrompt(task, 1);
expect(result).toContain("the workflow graph records completion"); expect(result).toContain("The workflow graph records step status, ordering, review, and completion.");
expect(result).not.toContain("fn_task_done()"); expect(result).not.toContain("fn_task_done()");
}); });
@@ -1690,10 +1690,16 @@ describe("StepSessionExecutor", () => {
const session = makeMockSession(); const session = makeMockSession();
mockedCreateFnAgent.mockResolvedValue({ session } as any); mockedCreateFnAgent.mockResolvedValue({ session } as any);
// Make git log return commits, but cherry-pick fails // Make the merge-base bounded commit list return a step commit, but cherry-pick fails.
mockedExecSync.mockImplementation((cmd: string) => { mockedExecSync.mockImplementation((cmd: string) => {
if (typeof cmd === "string" && cmd.includes("git log")) { if (typeof cmd === "string" && cmd.includes("git rev-parse HEAD")) {
return "abc123def Some commit"; return "primary-head";
}
if (typeof cmd === "string" && cmd.includes("git merge-base HEAD primary-head")) {
return "merge-base-sha";
}
if (typeof cmd === "string" && cmd.includes("git rev-list --reverse merge-base-sha..HEAD")) {
return "abc123def";
} }
if (typeof cmd === "string" && cmd.includes("git cherry-pick") && !cmd.includes("--abort")) { if (typeof cmd === "string" && cmd.includes("git cherry-pick") && !cmd.includes("--abort")) {
throw new Error("Merge conflict"); throw new Error("Merge conflict");
@@ -1737,8 +1743,14 @@ describe("StepSessionExecutor", () => {
mockedCreateFnAgent.mockResolvedValue({ session } as any); mockedCreateFnAgent.mockResolvedValue({ session } as any);
mockedExecSync.mockImplementation((cmd: string) => { mockedExecSync.mockImplementation((cmd: string) => {
if (typeof cmd === "string" && cmd.includes("git log")) { if (typeof cmd === "string" && cmd.includes("git rev-parse HEAD")) {
return "abc123def Some commit"; return "primary-head";
}
if (typeof cmd === "string" && cmd.includes("git merge-base HEAD primary-head")) {
return "merge-base-sha";
}
if (typeof cmd === "string" && cmd.includes("git rev-list --reverse merge-base-sha..HEAD")) {
return "abc123def";
} }
if (typeof cmd === "string" && cmd.includes("git cherry-pick") && cmd.includes("--abort")) { if (typeof cmd === "string" && cmd.includes("git cherry-pick") && cmd.includes("--abort")) {
throw new Error("abort failed"); throw new Error("abort failed");
@@ -1775,7 +1787,13 @@ describe("StepSessionExecutor", () => {
}); });
mockedExecSync.mockImplementation((cmd: string) => { mockedExecSync.mockImplementation((cmd: string) => {
if (cmd.includes("git log")) { if (cmd.includes("git rev-parse HEAD")) {
return "primary-head";
}
if (cmd.includes("git merge-base HEAD primary-head")) {
return "merge-base-sha";
}
if (cmd.includes("git rev-list --reverse merge-base-sha..HEAD")) {
return "abc123"; return "abc123";
} }
if (cmd.includes("git cherry-pick") && cmd.includes("--abort")) { if (cmd.includes("git cherry-pick") && cmd.includes("--abort")) {
@@ -2022,7 +2040,13 @@ describe("StepSessionExecutor", () => {
throw new Error("step 2 worktree failed"); throw new Error("step 2 worktree failed");
} }
} }
if (cmd.includes("git log")) { if (cmd.includes("git rev-parse HEAD")) {
return "primary-head";
}
if (cmd.includes("git merge-base HEAD primary-head")) {
return "merge-base-sha";
}
if (cmd.includes("git rev-list --reverse merge-base-sha..HEAD")) {
return "abc123"; return "abc123";
} }
return ""; return "";

View File

@@ -1678,20 +1678,16 @@ export class TaskExecutor {
} }
if (task.paused === true || task.userPaused === true || globalPause) return; if (task.paused === true || task.userPaused === true || globalPause) return;
/* /*
* FNXC:WorkflowLifecycle 2026-06-29-00:57: * FNXC:WorkflowLifecycle 2026-06-29-10:35:
* A stale pause-abort marker must not survive into a fresh unpaused dispatch. * A stale pause-abort marker must not survive into a fresh unpaused dispatch.
* FN-7225 showed graph-owned Plan Review and execution failures being logged * FN-7225/FN-7226 showed graph-owned execution failures being narrated as
* as "engine pause/resume" even though the task row was not paused. Clear the * pause/resume cleanup even though the task row was not paused. Clear the
* volatile marker at dispatch entry so real workflow/execution failures keep * volatile marker silently at dispatch entry so the task log names the real
* their actual cause and do not loop through pause recovery. * workflow failure (`step-execute`, parse, review, etc.) instead of implying
* the engine actually paused.
*/ */
this.clearPausedAborted(task.id); this.clearPausedAborted(task.id);
await this.store.logEntry( executorLog.log(`${task.id}: cleared stale pause-abort marker before unpaused execution dispatch`);
task.id,
"Cleared stale pause-abort marker before unpaused execution dispatch",
undefined,
this.getRunContextFor(task.id),
).catch(() => undefined);
} }
clearPauseAbortStateForManualRetry(taskId: string): void { clearPauseAbortStateForManualRetry(taskId: string): void {

View File

@@ -136,12 +136,11 @@ export async function runTaskStep(
// 1. Projection: step → in-progress (KTD-7). updateStep's own guards apply. // 1. Projection: step → in-progress (KTD-7). updateStep's own guards apply.
try { try {
await store.updateStep( if (opts.projectionSource) {
task.id, await store.updateStep(task.id, stepIndex, "in-progress", { source: opts.projectionSource });
stepIndex, } else {
"in-progress", await store.updateStep(task.id, stepIndex, "in-progress");
opts.projectionSource ? { source: opts.projectionSource } : undefined, }
);
} catch (err) { } catch (err) {
executorLog.warn( executorLog.warn(
`${task.id}: runTaskStep failed to mark step ${stepIndex} in-progress: ${errMsg(err)}`, `${task.id}: runTaskStep failed to mark step ${stepIndex} in-progress: ${errMsg(err)}`,
@@ -175,12 +174,11 @@ export async function runTaskStep(
if (result.success) { if (result.success) {
if (markDoneOnSuccess) { if (markDoneOnSuccess) {
try { try {
await store.updateStep( if (opts.projectionSource) {
task.id, await store.updateStep(task.id, stepIndex, "done", { source: opts.projectionSource });
stepIndex, } else {
"done", await store.updateStep(task.id, stepIndex, "done");
opts.projectionSource ? { source: opts.projectionSource } : undefined, }
);
} catch (err) { } catch (err) {
executorLog.warn( executorLog.warn(
`${task.id}: runTaskStep failed to mark step ${stepIndex} done: ${errMsg(err)}`, `${task.id}: runTaskStep failed to mark step ${stepIndex} done: ${errMsg(err)}`,

View File

@@ -1525,11 +1525,24 @@ Follow instructions precisely and avoid unrelated changes.`,
private async cherryPickCommits(stepIndex: number, worktreePath: string): Promise<void> { private async cherryPickCommits(stepIndex: number, worktreePath: string): Promise<void> {
const { worktreePath: primaryPath, taskDetail } = this.options; const { worktreePath: primaryPath, taskDetail } = this.options;
// Get commits made in the parallel worktree since it was created /*
* FNXC:WorkflowStepControl 2026-06-29-10:31:
* Parallel step-session worktrees must land their step commits back into the primary task worktree before modifiedFiles capture runs. Use the actual merge-base with the primary worktree HEAD; the old time/HEAD~10 range could include already-present ancestors, hit an empty cherry-pick first, and leave the task branch with no files changed.
*/
let commits: string; let commits: string;
try { try {
const { stdout: primaryHeadRaw } = await execAsync("git rev-parse HEAD", {
cwd: primaryPath,
encoding: "utf-8",
});
const primaryHead = primaryHeadRaw.trim();
const { stdout: mergeBaseRaw } = await execAsync(`git merge-base HEAD ${primaryHead}`, {
cwd: worktreePath,
encoding: "utf-8",
});
const mergeBase = mergeBaseRaw.trim();
const { stdout } = await execAsync( const { stdout } = await execAsync(
`git log --oneline --format="%H" HEAD...HEAD~10 --since="1 hour ago"`, `git rev-list --reverse ${mergeBase}..HEAD`,
{ cwd: worktreePath, encoding: "utf-8" }, { cwd: worktreePath, encoding: "utf-8" },
); );
commits = stdout.trim(); commits = stdout.trim();
@@ -1544,18 +1557,30 @@ Follow instructions precisely and avoid unrelated changes.`,
return; return;
} }
const shas = commits.split("\n").filter(Boolean); const shas = commits.split("\n").map((line) => line.trim()).filter(Boolean);
stepExecLog.log( stepExecLog.log(
`Cherry-picking ${shas.length} commit(s) from step ${stepIndex} ` + `Cherry-picking ${shas.length} commit(s) from step ${stepIndex} ` +
`into primary worktree for task ${taskDetail.id}`, `into primary worktree for task ${taskDetail.id}`,
); );
for (const sha of shas.reverse()) { for (const sha of shas) {
try { try {
await execAsync(`git cherry-pick "${sha}"`, { await execAsync(`git cherry-pick "${sha}"`, {
cwd: primaryPath, cwd: primaryPath,
}); });
} catch (err) { } catch (err) {
const errText = `${err instanceof Error ? err.message : String(err)} ${String((err as { stdout?: unknown; stderr?: unknown })?.stdout ?? "")} ${String((err as { stderr?: unknown })?.stderr ?? "")}`;
if (
errText.includes("The previous cherry-pick is now empty") ||
errText.includes("nothing to commit") ||
errText.includes("is empty")
) {
await execAsync("git cherry-pick --skip", { cwd: primaryPath }).catch(async () => {
await execAsync("git cherry-pick --abort", { cwd: primaryPath }).catch(() => undefined);
});
stepExecLog.warn(`Skipped empty cherry-pick for step ${stepIndex}: ${sha}`);
continue;
}
// Cherry-pick conflict — abort and log // Cherry-pick conflict — abort and log
try { try {
await execAsync("git cherry-pick --abort", { cwd: primaryPath }); await execAsync("git cherry-pick --abort", { cwd: primaryPath });