feat(engine): U2 — runTaskStep/resetStepToBaseline substrate seams with blast-radius guard (RETHINK extraction)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-04 11:50:45 -07:00
parent 2a3c285bae
commit 3d03505a62
5 changed files with 938 additions and 50 deletions

View File

@@ -3583,3 +3583,133 @@ describe("TaskExecutor loop recovery", () => {
// ── Context limit error recovery tests ────────────────────────────────
// ── U2 RETHINK delegation characterization (plan 2026-06-04-001, KTD-2) ──
//
// The legacy in-session fn_review_step RETHINK case now DELEGATES to
// step-runner.ts's resetStepToBaseline. These tests pin that the observable
// side effects are byte-identical to the pre-extraction block: git reset to
// the agent-supplied baseline, session rewind via navigateTree, step→pending,
// and the RETHINK log entry — all reached through the real executor session.
describe("U2: fn_review_step RETHINK delegates to resetStepToBaseline (characterization)", () => {
beforeEach(() => {
resetExecutorMocks();
});
function runRethinkScenario(reviewType: "code" | "plan", navigateTree: any) {
const store = createMockStore();
const baseTask = {
id: "FN-RT-1",
title: "Test",
description: "Test task",
column: "in-progress",
dependencies: [],
steps: [{ name: "Implement", status: "in-progress" }],
currentStep: 0,
log: [],
prompt: "# test\n## Steps\n### Step 1: Implement\n- [ ] implement",
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
store.getTask.mockResolvedValue(baseTask as any);
// updateStep returns the task with the step persisted in-progress so the
// executor's checkpoint-capture path (executor.ts ~6517) populates the
// stepCheckpoints map that RETHINK rewinds to.
store.updateStep.mockResolvedValue({
...baseTask,
steps: [{ name: "Implement", status: "in-progress" }],
} as any);
mockedReviewStep.mockResolvedValue({
verdict: "RETHINK",
review: "wrong approach",
summary: "rejected approach",
} as any);
let reviewToolError: unknown;
mockedCreateFnAgent.mockImplementation((async (opts: any) => {
const tools = opts.customTools || [];
return {
session: {
prompt: vi.fn().mockImplementation(async () => {
// First, flip the step to in-progress via fn_task_update so the
// checkpoint map is populated (mirrors the real session lifecycle).
const updateTool = tools.find((t: any) => t.name === "fn_task_update");
if (updateTool) {
try {
await updateTool.execute("tool-update", { step: 1, status: "in-progress" });
} catch { /* tool param shape varies; ignore */ }
}
const reviewTool = tools.find((t: any) => t.name === "fn_review_step");
if (reviewTool) {
try {
await reviewTool.execute("tool-review", {
step: 1,
type: reviewType,
step_name: "Implement",
baseline: reviewType === "code" ? "agentBaselineSHA" : undefined,
});
} catch (e) {
reviewToolError = e;
}
}
}),
dispose: vi.fn(),
subscribe: vi.fn(),
on: vi.fn(),
navigateTree,
sessionManager: {
getLeafId: vi.fn().mockReturnValue("leaf-pre-step"),
branchWithSummary: vi.fn(),
},
state: {},
},
};
}) as any);
const executor = new TaskExecutor(store, "/tmp/test", {});
return { store, baseTask, executor, getReviewToolError: () => reviewToolError };
}
it("code RETHINK: git reset to baseline, navigateTree rewind, step→pending, RETHINK log", async () => {
const navigateTree = vi.fn().mockResolvedValue(undefined);
const { store, baseTask, executor } = runRethinkScenario("code", navigateTree);
await executor.execute(baseTask as any);
// git reset --hard <baseline> issued in the worktree (via the mocked exec).
const resetIssued = mockedExecSync.mock.calls.some(
(c) => typeof c[0] === "string" && (c[0] as string).includes("git reset --hard agentBaselineSHA"),
);
expect(resetIssued).toBe(true);
// Session rewound to the captured pre-step checkpoint.
expect(navigateTree).toHaveBeenCalledWith("leaf-pre-step", { summarize: false });
// Step reset to pending through the projection sink.
expect(store.updateStep).toHaveBeenCalledWith("FN-RT-1", 0, "pending");
// RETHINK log entry (code-review variant references the git reset).
expect(store.logEntry).toHaveBeenCalledWith(
"FN-RT-1",
expect.stringContaining("git reset to agentBaselineSHA"),
"rejected approach",
);
});
it("plan RETHINK: no git reset, navigateTree rewind, step→pending, plan-rewound log", async () => {
const navigateTree = vi.fn().mockResolvedValue(undefined);
const { store, baseTask, executor } = runRethinkScenario("plan", navigateTree);
await executor.execute(baseTask as any);
const resetIssued = mockedExecSync.mock.calls.some(
(c) => typeof c[0] === "string" && (c[0] as string).includes("git reset --hard"),
);
expect(resetIssued).toBe(false);
expect(navigateTree).toHaveBeenCalledWith("leaf-pre-step", { summarize: false });
expect(store.updateStep).toHaveBeenCalledWith("FN-RT-1", 0, "pending");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-RT-1",
expect.stringContaining("Step 1 plan rewound"),
"rejected approach",
);
});
});

View File

@@ -0,0 +1,397 @@
/**
* Unit tests for the U2 substrate seams (plan 2026-06-04-001, KTD-2):
* - runTaskStep — per-step driver over step-session physics.
* - resetStepToBaseline — verbatim RETHINK mechanics + blast-radius guard.
*
* Fast tests: real git / sessions / StepSessionExecutor are never touched —
* every external is injected via the explicit `deps` object (FN-5048 fake-timer
* convention is moot here since the seams take no clock). The executor's
* delegation of the legacy RETHINK block is characterized separately in
* executor-step-session.test.ts.
*/
import { describe, it, expect, vi, beforeEach } from "vitest";
import {
runTaskStep,
resetStepToBaseline,
makeAncestryBlastRadiusGuard,
type StepRunnerTask,
type SessionRef,
} from "../step-runner.js";
function makeStore() {
return {
updateStep: vi.fn().mockResolvedValue(undefined),
logEntry: vi.fn().mockResolvedValue(undefined),
};
}
function makeTask(steps: Array<{ name?: string; status?: string }>): StepRunnerTask {
return { id: "FN-001", steps };
}
function makeSessionRef(opts?: {
navigateTree?: ReturnType<typeof vi.fn>;
branchWithSummary?: ReturnType<typeof vi.fn>;
leafId?: string;
}): SessionRef {
const navigateTree = opts?.navigateTree ?? vi.fn().mockResolvedValue(undefined);
const branchWithSummary = opts?.branchWithSummary ?? vi.fn();
return {
current: {
navigateTree,
sessionManager: {
branchWithSummary,
getLeafId: vi.fn().mockReturnValue(opts?.leafId ?? "leaf-pre-step"),
},
} as unknown as SessionRef["current"],
};
}
describe("runTaskStep", () => {
beforeEach(() => vi.clearAllMocks());
it("marks the step in-progress then done on success, capturing baseline + checkpoint", async () => {
const store = makeStore();
const task = makeTask([{ name: "Implement", status: "pending" }]);
const gitRevParse = vi.fn().mockResolvedValue("baseSHA123");
const captureCheckpointId = vi.fn().mockReturnValue("leaf-pre-step");
const runStep = vi.fn().mockResolvedValue({ success: true });
const result = await runTaskStep(
{ store, worktreePath: "/wt", runStep, gitRevParse, captureCheckpointId },
task,
0,
);
expect(result).toEqual({ outcome: "success", baselineSha: "baseSHA123", checkpointId: "leaf-pre-step" });
// Baseline is captured BEFORE the step runs.
expect(gitRevParse).toHaveBeenCalledWith("/wt");
expect(runStep).toHaveBeenCalledWith(0);
// Projection ordering: in-progress before done.
expect(store.updateStep.mock.calls).toEqual([
["FN-001", 0, "in-progress"],
["FN-001", 0, "done"],
]);
});
it("captures the baseline before running the step (order check)", async () => {
const store = makeStore();
const order: string[] = [];
const gitRevParse = vi.fn().mockImplementation(async () => {
order.push("baseline");
return "sha";
});
const runStep = vi.fn().mockImplementation(async () => {
order.push("run");
return { success: true };
});
await runTaskStep(
{ store, worktreePath: "/wt", runStep, gitRevParse, captureCheckpointId: () => "leaf" },
makeTask([{ status: "pending" }]),
0,
);
expect(order).toEqual(["baseline", "run"]);
});
it("leaves the step non-done on failure (no 'done'/'skipped' write)", async () => {
const store = makeStore();
const runStep = vi.fn().mockResolvedValue({ success: false, error: "boom" });
const result = await runTaskStep(
{
store,
worktreePath: "/wt",
runStep,
gitRevParse: async () => "baseSHA",
captureCheckpointId: () => "leaf",
},
makeTask([{ status: "pending" }]),
0,
);
expect(result).toEqual({ outcome: "failure", baselineSha: "baseSHA", checkpointId: "leaf" });
// Only the in-progress write happened — the failed step is left non-done.
expect(store.updateStep.mock.calls).toEqual([["FN-001", 0, "in-progress"]]);
expect(store.updateStep).not.toHaveBeenCalledWith("FN-001", 0, "done");
expect(store.updateStep).not.toHaveBeenCalledWith("FN-001", 0, "skipped");
});
it("still returns a result when baseline capture fails (best-effort)", async () => {
const store = makeStore();
const runStep = vi.fn().mockResolvedValue({ success: true });
const gitRevParse = vi.fn().mockRejectedValue(new Error("not a git repo"));
const result = await runTaskStep(
{ store, worktreePath: "/wt", runStep, gitRevParse, captureCheckpointId: () => "leaf" },
makeTask([{ status: "pending" }]),
0,
);
expect(result.outcome).toBe("success");
expect(result.baselineSha).toBeUndefined();
expect(result.checkpointId).toBe("leaf");
});
it("uses the default checkpoint capture from the session ref when none injected", async () => {
const store = makeStore();
const sessionRef = makeSessionRef({ leafId: "leaf-xyz" });
const result = await runTaskStep(
{
store,
worktreePath: "/wt",
runStep: async () => ({ success: true }),
gitRevParse: async () => "sha",
},
makeTask([{ status: "pending" }]),
0,
{ sessionRef },
);
expect(result.checkpointId).toBe("leaf-xyz");
});
});
describe("resetStepToBaseline", () => {
beforeEach(() => vi.clearAllMocks());
it("does git reset + session rewind + step→pending with baseline and checkpoint (code review)", async () => {
const store = makeStore();
const navigateTree = vi.fn().mockResolvedValue(undefined);
const sessionRef = makeSessionRef({ navigateTree });
// We can't observe the real git command without mocking child_process; verify
// the session rewind + projection happen. (The git path is exercised through
// the executor characterization test.)
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "code", summary: "rejected" },
makeTask([{ status: "in-progress" }]),
0,
"baseSHA",
"leaf-checkpoint",
);
expect(result).toEqual({ ok: true });
expect(navigateTree).toHaveBeenCalledWith("leaf-checkpoint", { summarize: false });
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-001",
expect.stringContaining("git reset to baseSHA"),
"rejected",
);
});
it("skips the session rewind when no checkpoint is provided (partial path)", async () => {
const store = makeStore();
const navigateTree = vi.fn();
const branchWithSummary = vi.fn();
const sessionRef = makeSessionRef({ navigateTree, branchWithSummary });
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "code" },
makeTask([{ status: "in-progress" }]),
0,
"baseSHA",
undefined,
);
expect(result.ok).toBe(true);
expect(navigateTree).not.toHaveBeenCalled();
expect(branchWithSummary).not.toHaveBeenCalled();
// Step still flips to pending.
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
});
it("plan review skips git reset, logs the plan-rewound line, still flips pending", async () => {
const store = makeStore();
const navigateTree = vi.fn().mockResolvedValue(undefined);
const sessionRef = makeSessionRef({ navigateTree });
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "plan", summary: "plan rejected" },
makeTask([{ status: "in-progress" }]),
2,
undefined,
"leaf-checkpoint",
);
expect(result.ok).toBe(true);
expect(navigateTree).toHaveBeenCalledWith("leaf-checkpoint", { summarize: false });
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 2, "pending");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-001",
// 0-indexed step 2 → 1-indexed "Step 3"
expect.stringContaining("Step 3 plan rewound"),
"plan rejected",
);
});
it("falls back to branchWithSummary when navigateTree throws", async () => {
const store = makeStore();
const navigateTree = vi.fn().mockRejectedValue(new Error("navigate failed"));
const branchWithSummary = vi.fn();
const sessionRef = makeSessionRef({ navigateTree, branchWithSummary });
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "code", summary: "why" },
makeTask([{ status: "in-progress" }]),
0,
"baseSHA",
"leaf-checkpoint",
);
expect(result.ok).toBe(true);
expect(branchWithSummary).toHaveBeenCalledWith("leaf-checkpoint", "RETHINK: why");
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
});
// ── KTD-2 blast-radius guard refusal cases ──────────────────────────────
it("REFUSES and mutates nothing when the guard reports a violation", async () => {
const store = makeStore();
const navigateTree = vi.fn();
const sessionRef = makeSessionRef({ navigateTree });
const audit = { database: vi.fn().mockResolvedValue(undefined) };
const blastRadiusGuard = vi.fn().mockResolvedValue("baseSHA is not an ancestor of HEAD");
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "code", audit, blastRadiusGuard },
makeTask([{ status: "in-progress" }]),
0,
"baseSHA",
"leaf-checkpoint",
);
expect(result).toEqual({ ok: false, reason: "baseSHA is not an ancestor of HEAD" });
// No mutation: no rewind, no updateStep, no RETHINK logEntry.
expect(navigateTree).not.toHaveBeenCalled();
expect(store.updateStep).not.toHaveBeenCalled();
expect(store.logEntry).not.toHaveBeenCalled();
// Audit warning emitted (task:integrity-warning, database domain).
expect(audit.database).toHaveBeenCalledWith(
expect.objectContaining({
type: "task:integrity-warning",
target: "FN-001",
metadata: expect.objectContaining({
guard: "step-reset-blast-radius",
reason: "baseSHA is not an ancestor of HEAD",
}),
}),
);
});
it("fails closed (refuses) when the guard itself throws", async () => {
const store = makeStore();
const sessionRef = makeSessionRef();
const blastRadiusGuard = vi.fn().mockRejectedValue(new Error("git exploded"));
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "code", blastRadiusGuard },
makeTask([{ status: "in-progress" }]),
0,
"baseSHA",
"leaf",
);
expect(result.ok).toBe(false);
expect(result.reason).toContain("git exploded");
expect(store.updateStep).not.toHaveBeenCalled();
});
it("proceeds with the reset when the guard returns null (safe)", async () => {
const store = makeStore();
const navigateTree = vi.fn().mockResolvedValue(undefined);
const sessionRef = makeSessionRef({ navigateTree });
const blastRadiusGuard = vi.fn().mockResolvedValue(null);
const result = await resetStepToBaseline(
{ store, worktreePath: "/wt", sessionRef, reviewType: "code", blastRadiusGuard },
makeTask([{ status: "in-progress" }]),
0,
"baseSHA",
"leaf-checkpoint",
);
expect(result.ok).toBe(true);
expect(navigateTree).toHaveBeenCalledWith("leaf-checkpoint", { summarize: false });
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
});
});
describe("makeAncestryBlastRadiusGuard", () => {
beforeEach(() => vi.clearAllMocks());
it("refuses when a LATER step is already done", async () => {
const guard = makeAncestryBlastRadiusGuard({
worktreePath: "/wt",
task: makeTask([{ status: "in-progress" }, { status: "done" }]),
stepIndex: 0,
isAncestor: async () => true,
});
const reason = await guard("baseSHA");
expect(reason).toContain("later step 1 is done");
});
it("refuses when a LATER step is already skipped", async () => {
const guard = makeAncestryBlastRadiusGuard({
worktreePath: "/wt",
task: makeTask([{ status: "in-progress" }, { status: "skipped" }]),
stepIndex: 0,
isAncestor: async () => true,
});
const reason = await guard("baseSHA");
expect(reason).toContain("later step 1 is skipped");
});
it("refuses when the baseline is NOT an ancestor of HEAD", async () => {
const guard = makeAncestryBlastRadiusGuard({
worktreePath: "/wt",
task: makeTask([{ status: "in-progress" }]),
stepIndex: 0,
isAncestor: async () => false,
});
const reason = await guard("baseSHA");
expect(reason).toContain("not an ancestor of HEAD");
});
it("allows when baseline is an ancestor and no later step is terminal", async () => {
const isAncestor = vi.fn().mockResolvedValue(true);
const guard = makeAncestryBlastRadiusGuard({
worktreePath: "/wt",
task: makeTask([
{ status: "pending" },
{ status: "in-progress" },
{ status: "pending" },
]),
stepIndex: 1,
isAncestor,
});
const reason = await guard("baseSHA");
expect(reason).toBeNull();
expect(isAncestor).toHaveBeenCalledWith("baseSHA", "/wt");
});
it("allows (skipping ancestry) when no baseline is supplied", async () => {
const isAncestor = vi.fn();
const guard = makeAncestryBlastRadiusGuard({
worktreePath: "/wt",
task: makeTask([{ status: "in-progress" }]),
stepIndex: 0,
isAncestor,
});
const reason = await guard(undefined);
expect(reason).toBeNull();
expect(isAncestor).not.toHaveBeenCalled();
});
it("treats an earlier done step as harmless (only LATER steps matter)", async () => {
const guard = makeAncestryBlastRadiusGuard({
worktreePath: "/wt",
task: makeTask([{ status: "done" }, { status: "in-progress" }]),
stepIndex: 1,
isAncestor: async () => true,
});
const reason = await guard("baseSHA");
expect(reason).toBeNull();
});
});

View File

@@ -103,6 +103,7 @@ import type { StuckTaskDetector, StuckTaskEvent } from "./stuck-task-detector.js
import type { PluginRunner } from "./plugin-runner.js";
import { isContextLimitError } from "./context-limit-detector.js";
import { StepSessionExecutor } from "./step-session-executor.js";
import { resetStepToBaseline } from "./step-runner.js";
import { acquireTaskWorktree } from "./worktree-acquisition.js";
import { resolveCapturedBaseCommitSha } from "./base-commit-capture.js";
import { installTaskWorktreeIdentityGuard } from "./worktree-hooks.js";
@@ -7453,61 +7454,31 @@ export class TaskExecutor {
}
break;
case "RETHINK": {
// For code reviews: git reset to baseline to revert file changes
// For plan reviews: skip git reset (no code has been written yet)
if (reviewType === "code" && baseline) {
try {
await execAsync(`git reset --hard ${baseline}`, { cwd: worktreePath });
executorLog.log(`${taskId}: RETHINK — git reset --hard ${baseline}`);
} catch (gitErr: unknown) {
const gitErrMessage = gitErr instanceof Error ? gitErr.message : String(gitErr);
executorLog.error(`${taskId}: RETHINK git reset failed: ${gitErrMessage}`);
}
} else if (reviewType === "code") {
executorLog.log(`${taskId}: RETHINK — no baseline SHA, skipping git reset`);
}
// Rewind conversation to pre-step checkpoint
// RETHINK mechanics (git reset to baseline + session rewind +
// step→pending + RETHINK log entry) are the U2 substrate seam.
// The legacy in-session path delegates to the single extracted
// implementation in step-runner.ts so there is exactly one copy.
// No blast-radius guard here: this path is intra-session with an
// agent-supplied baseline (KTD-2 — the guard is for graph-owned
// shared-isolation resets), so behavior stays byte-identical.
const checkpointId = stepCheckpoints.get(stepIndex);
if (checkpointId && sessionRef.current) {
try {
await sessionRef.current.navigateTree(checkpointId, { summarize: false });
executorLog.log(`${taskId}: RETHINK — session rewound to checkpoint ${checkpointId}`);
} catch (rewindErr: unknown) {
const msg = rewindErr instanceof Error ? rewindErr.message : String(rewindErr);
executorLog.warn(`${taskId}: RETHINK navigateTree rewind failed, falling back to branchWithSummary: ${msg}`);
// Fallback to branchWithSummary
try {
sessionRef.current.sessionManager.branchWithSummary(
checkpointId,
`RETHINK: ${result.summary || "Approach rejected by reviewer"}`,
);
executorLog.log(`${taskId}: RETHINK — branched from checkpoint ${checkpointId}`);
} catch (branchErr: unknown) {
const branchErrMessage = branchErr instanceof Error ? branchErr.message : String(branchErr);
executorLog.error(`${taskId}: RETHINK session rewind failed: ${branchErrMessage}`);
}
}
} else {
executorLog.log(`${taskId}: RETHINK — no session checkpoint for step ${step}, skipping rewind`);
}
// Reset step status to pending
await store.updateStep(taskId, stepIndex, "pending");
await resetStepToBaseline(
{
store,
worktreePath,
sessionRef,
reviewType: reviewType === "plan" ? "plan" : "code",
summary: result.summary,
},
{ id: taskId, steps: taskSteps },
stepIndex,
reviewType === "code" ? baseline : undefined,
checkpointId,
);
if (reviewType === "plan") {
await store.logEntry(
taskId,
`RETHINK: Step ${step} plan rewound — session checkpoint ${checkpointId || "N/A"}`,
result.summary,
);
text = `RETHINK\n\nYour plan was rejected. Here is why:\n\n${result.review}\n\nTake a different approach to planning this step. Do NOT repeat the rejected strategy.`;
} else {
await store.logEntry(
taskId,
`RETHINK: Step ${step} rewound — git reset to ${baseline || "N/A"}, session checkpoint ${checkpointId || "N/A"}`,
result.summary,
);
text = `RETHINK\n\nYour previous approach was rejected. Here is why:\n\n${result.review}\n\nTake a different approach. Do NOT repeat the rejected strategy. Re-read the step requirements and find an alternative solution.`;
}
break;

View File

@@ -554,6 +554,21 @@ export {
} from "./hold-release.js";
export { StepSessionExecutor } from "./step-session-executor.js";
export type { StepResult, ParallelWave, StepSessionExecutorOptions } from "./step-session-executor.js";
export {
runTaskStep,
resetStepToBaseline,
makeAncestryBlastRadiusGuard,
} from "./step-runner.js";
export type {
RunTaskStepDeps,
RunTaskStepOptions,
RunTaskStepResult,
ResetStepDeps,
ResetStepResult,
RunSingleStep,
SessionRef,
StepRunnerTask,
} from "./step-runner.js";
// Multi-project runtime types
export {
type ProjectRuntime,

View File

@@ -0,0 +1,375 @@
/**
* step-runner — the two substrate seams for graph-owned stepwise execution
* (plan 2026-06-04-001, KTD-2 / U2).
*
* This module exposes exactly two capabilities that the workflow-graph executor
* (U3/U5) will drive — it does NOT wire itself into any graph path here:
*
* - {@link runTaskStep} — run exactly step `i` of a task inside its
* session/worktree and return the outcome plus
* the per-step `baselineSha` / `checkpointId`
* that a later RETHINK needs.
* - {@link resetStepToBaseline} — the RETHINK mechanics, extracted verbatim
* from `executor.ts`'s `fn_review_step` RETHINK
* block (`git reset --hard <baseline>` + session
* rewind via `navigateTree`/`branchWithSummary`
* fallback + `store.updateStep(..., "pending")`),
* plus a defensive blast-radius guard (KTD-2).
*
* Both are parameterized via an explicit `deps` object (the DI style used by
* `hold-release.ts` / `merge-trait.ts`) so they stay unit-testable without real
* git, real sessions, or a real `StepSessionExecutor`. Production callers (U3/U5)
* pass thin adapters over the existing engine machinery; the legacy in-session
* `fn_review_step` path is untouched and keeps its own copy's behavior — this
* extraction is the single implementation the executor's RETHINK block now
* delegates to (see `TaskExecutor.applyStepRethink`).
*/
import { exec } from "node:child_process";
import { promisify } from "node:util";
import type { TaskStore } from "@fusion/core";
const execAsync = promisify(exec);
import type { AgentSession as PiAgentSession } from "@earendil-works/pi-coding-agent";
import { executorLog } from "./logger.js";
import type { RunAuditor } from "./run-audit.js";
// ── Shared minimal shapes ───────────────────────────────────────────────
/** The slice of `Task` the step runner reads. */
export interface StepRunnerTask {
id: string;
steps: Array<{ name?: string; status?: string }>;
}
/** A minimal session ref mirroring the executor's `{ current: AgentSession }`. */
export interface SessionRef {
current: PiAgentSession | null;
}
/**
* Run exactly one step inside the task's session/worktree. Production wires this
* to a {@link import("./step-session-executor.js").StepSessionExecutor} configured
* for a single step (graph-owned runs force step-session physics, KTD-2/KTD-8);
* tests inject a fake. Returns whether the step's session completed successfully.
*/
export type RunSingleStep = (stepIndex: number) => Promise<{ success: boolean; error?: string }>;
// ── runTaskStep ─────────────────────────────────────────────────────────
/** Dependencies for {@link runTaskStep}. */
export interface RunTaskStepDeps {
/** Step-state projection sink (KTD-7). */
store: Pick<TaskStore, "updateStep" | "logEntry">;
/** Absolute path to the task's worktree (where `git rev-parse HEAD` runs). */
worktreePath: string;
/** Run exactly step `i` (step-session physics). */
runStep: RunSingleStep;
/**
* Capture HEAD in the worktree before step work begins (the per-step baseline,
* KTD-2 documented behavior change). Defaults to
* `git rev-parse HEAD` in {@link RunTaskStepDeps.worktreePath}; inject in tests.
*/
gitRevParse?: (worktreePath: string) => Promise<string | undefined>;
/**
* Capture the session checkpoint (leaf) id for the step — observed the same way
* the legacy `stepCheckpoints` map is populated (`session.sessionManager.getLeafId()`).
* Defaults to reading {@link RunTaskStepOptions.sessionRef}; inject in tests.
*/
captureCheckpointId?: () => string | undefined;
}
/** Options for {@link runTaskStep}. */
export interface RunTaskStepOptions {
/** Session ref used for the default checkpoint capture. */
sessionRef?: SessionRef;
}
/** Result of {@link runTaskStep}. */
export interface RunTaskStepResult {
outcome: "success" | "failure";
baselineSha?: string;
checkpointId?: string;
}
/**
* Drive execution of exactly step `stepIndex` of `task`.
*
* Order of operations (matches the legacy step-session lifecycle the
* characterization tests pin):
* 1. mark the step `in-progress` via `store.updateStep` (projection sink);
* 2. capture `baselineSha` = HEAD in the worktree, BEFORE any step work;
* 3. run exactly step `i` as a step-session (the agent authors its own
* `complete Step N` commit — this driver only observes);
* 4. capture `checkpointId` (session leaf) for a later RETHINK rewind;
* 5. on success, mark the step `done`; on failure, leave the step non-done
* (the graph decides routing — KTD-4).
*/
export async function runTaskStep(
deps: RunTaskStepDeps,
task: StepRunnerTask,
stepIndex: number,
opts: RunTaskStepOptions = {},
): Promise<RunTaskStepResult> {
const { store, worktreePath } = deps;
const gitRevParse = deps.gitRevParse ?? defaultGitRevParse;
const captureCheckpointId =
deps.captureCheckpointId ?? (() => defaultCaptureCheckpointId(opts.sessionRef));
// 1. Projection: step → in-progress (KTD-7). updateStep's own guards apply.
try {
await store.updateStep(task.id, stepIndex, "in-progress");
} catch (err) {
executorLog.warn(
`${task.id}: runTaskStep failed to mark step ${stepIndex} in-progress: ${errMsg(err)}`,
);
}
// 2. Baseline capture at instance start, before step work (KTD-2).
let baselineSha: string | undefined;
try {
baselineSha = await gitRevParse(worktreePath);
} catch (err) {
executorLog.warn(`${task.id}: runTaskStep baseline capture failed: ${errMsg(err)}`);
}
// 3. Run exactly step i. The agent authors the commit; we observe only.
const result = await deps.runStep(stepIndex);
// 4. Capture the session checkpoint (leaf) for a later RETHINK rewind.
let checkpointId: string | undefined;
try {
checkpointId = captureCheckpointId() ?? undefined;
} catch (err) {
executorLog.warn(`${task.id}: runTaskStep checkpoint capture failed: ${errMsg(err)}`);
}
// 5. Projection: success → done; failure leaves the step non-done.
if (result.success) {
try {
await store.updateStep(task.id, stepIndex, "done");
} catch (err) {
executorLog.warn(
`${task.id}: runTaskStep failed to mark step ${stepIndex} done: ${errMsg(err)}`,
);
}
return { outcome: "success", baselineSha, checkpointId };
}
return { outcome: "failure", baselineSha, checkpointId };
}
// ── resetStepToBaseline ──────────────────────────────────────────────────
/** Dependencies for {@link resetStepToBaseline}. */
export interface ResetStepDeps {
/** Step-state projection sink (KTD-7). */
store: Pick<TaskStore, "updateStep" | "logEntry">;
/** Absolute path to the task's worktree (where `git reset --hard` runs). */
worktreePath: string;
/** Session ref for the conversation rewind (`navigateTree` / `branchWithSummary`). */
sessionRef: SessionRef;
/**
* Review type — `code` reverts file changes via git reset; `plan` skips the
* git reset (no code was written), matching the legacy RETHINK branch.
*/
reviewType?: "code" | "plan";
/** Optional reviewer summary used as the `branchWithSummary` fallback label. */
summary?: string;
/** Optional auditor for the blast-radius guard refusal warning (KTD-2). */
audit?: Pick<RunAuditor, "database">;
/**
* Blast-radius guard hook (KTD-2, shared isolation). Returns `null` when the
* reset is safe, or a refusal `reason` string when it would destroy other
* steps' approved work (baseline not an ancestor of HEAD, or a later step is
* already done/skipped past the baseline). When omitted the guard is skipped
* (worktree isolation makes it structural — KTD-11). Tests inject a fake;
* production wires {@link makeAncestryBlastRadiusGuard}.
*/
blastRadiusGuard?: (baselineSha: string | undefined) => Promise<string | null>;
}
/** Result of {@link resetStepToBaseline}. */
export interface ResetStepResult {
ok: boolean;
reason?: string;
}
/**
* Reset step `stepIndex` to its per-step baseline — the verbatim RETHINK
* mechanics extracted from `executor.ts` (`fn_review_step` RETHINK case):
*
* - `git reset --hard <baseline>` in the worktree (code review only; skipped
* when `baselineSha` is missing or for plan reviews — today's semantics);
* - session rewind to the pre-step checkpoint via `navigateTree`, falling back
* to `sessionManager.branchWithSummary` (skipped when `checkpointId` is
* missing — today's semantics);
* - `store.updateStep(..., "pending")`.
*
* Before any mutation, the KTD-2 blast-radius guard runs (when provided): on a
* violation it returns `{ ok: false, reason }`, emits an audit warning, and
* mutates NOTHING.
*/
export async function resetStepToBaseline(
deps: ResetStepDeps,
task: StepRunnerTask,
stepIndex: number,
baselineSha?: string,
checkpointId?: string,
): Promise<ResetStepResult> {
const { store, worktreePath, sessionRef } = deps;
const reviewType = deps.reviewType ?? "code";
const taskId = task.id;
const step = stepIndex + 1; // legacy log lines are 1-indexed
// ── KTD-2 blast-radius guard — assert BEFORE mutating anything. ──────────
if (deps.blastRadiusGuard) {
let refusal: string | null = null;
try {
refusal = await deps.blastRadiusGuard(baselineSha);
} catch (err) {
// A guard that itself fails is treated as a refusal — fail closed.
refusal = `blast-radius guard error: ${errMsg(err)}`;
}
if (refusal) {
executorLog.warn(
`${taskId}: RETHINK reset for step ${step} REFUSED by blast-radius guard: ${refusal}`,
);
await deps.audit?.database({
type: "task:integrity-warning",
target: taskId,
metadata: {
guard: "step-reset-blast-radius",
stepIndex,
baselineSha: baselineSha ?? null,
reason: refusal,
},
});
return { ok: false, reason: refusal };
}
}
// ── git reset --hard <baseline> (code reviews only). ─────────────────────
if (reviewType === "code" && baselineSha) {
try {
await execAsync(`git reset --hard ${baselineSha}`, { cwd: worktreePath });
executorLog.log(`${taskId}: RETHINK — git reset --hard ${baselineSha}`);
} catch (gitErr: unknown) {
executorLog.error(`${taskId}: RETHINK git reset failed: ${errMsg(gitErr)}`);
}
} else if (reviewType === "code") {
executorLog.log(`${taskId}: RETHINK — no baseline SHA, skipping git reset`);
}
// ── Rewind conversation to the pre-step checkpoint. ──────────────────────
if (checkpointId && sessionRef.current) {
try {
await sessionRef.current.navigateTree(checkpointId, { summarize: false });
executorLog.log(`${taskId}: RETHINK — session rewound to checkpoint ${checkpointId}`);
} catch (rewindErr: unknown) {
executorLog.warn(
`${taskId}: RETHINK navigateTree rewind failed, falling back to branchWithSummary: ${errMsg(rewindErr)}`,
);
try {
sessionRef.current.sessionManager.branchWithSummary(
checkpointId,
`RETHINK: ${deps.summary || "Approach rejected by reviewer"}`,
);
executorLog.log(`${taskId}: RETHINK — branched from checkpoint ${checkpointId}`);
} catch (branchErr: unknown) {
executorLog.error(`${taskId}: RETHINK session rewind failed: ${errMsg(branchErr)}`);
}
}
} else {
executorLog.log(`${taskId}: RETHINK — no session checkpoint for step ${step}, skipping rewind`);
}
// ── Reset step status to pending (projection sink). ──────────────────────
await store.updateStep(taskId, stepIndex, "pending");
if (reviewType === "plan") {
await store.logEntry(
taskId,
`RETHINK: Step ${step} plan rewound — session checkpoint ${checkpointId || "N/A"}`,
deps.summary,
);
} else {
await store.logEntry(
taskId,
`RETHINK: Step ${step} rewound — git reset to ${baselineSha || "N/A"}, session checkpoint ${checkpointId || "N/A"}`,
deps.summary,
);
}
return { ok: true };
}
// ── Blast-radius guard factory (shared isolation, KTD-2) ─────────────────
/**
* Build the shared-isolation blast-radius guard: a reset for step `stepIndex` is
* legal only when (a) `baselineSha` is an ancestor of HEAD in the worktree
* (`git merge-base --is-ancestor`), and (b) no LATER step is already
* `done`/`skipped` (which would postdate the baseline). On violation it returns
* the refusal reason; otherwise `null`. A missing baseline is allowed (the reset
* simply skips its git portion — today's partial-recovery semantics).
*/
export function makeAncestryBlastRadiusGuard(opts: {
worktreePath: string;
task: StepRunnerTask;
stepIndex: number;
isAncestor?: (baselineSha: string, worktreePath: string) => Promise<boolean>;
}): (baselineSha: string | undefined) => Promise<string | null> {
const isAncestor = opts.isAncestor ?? defaultIsAncestorOfHead;
return async (baselineSha: string | undefined): Promise<string | null> => {
// (b) No later step may already be terminal-done past this baseline.
const laterDone = opts.task.steps.findIndex(
(s, i) => i > opts.stepIndex && (s.status === "done" || s.status === "skipped"),
);
if (laterDone !== -1) {
return `later step ${laterDone} is ${opts.task.steps[laterDone]?.status} — reset would destroy approved work`;
}
// (a) Baseline must be an ancestor of HEAD (skipped when no baseline).
if (baselineSha) {
let ancestor = false;
try {
ancestor = await isAncestor(baselineSha, opts.worktreePath);
} catch (err) {
return `ancestry check failed: ${errMsg(err)}`;
}
if (!ancestor) {
return `baseline ${baselineSha} is not an ancestor of HEAD`;
}
}
return null;
};
}
// ── Defaults (production adapters over real git/session) ─────────────────
async function defaultGitRevParse(worktreePath: string): Promise<string | undefined> {
const { stdout } = await execAsync("git rev-parse HEAD", { cwd: worktreePath });
const sha = stdout.trim();
return sha.length > 0 ? sha : undefined;
}
function defaultCaptureCheckpointId(sessionRef?: SessionRef): string | undefined {
const leaf = sessionRef?.current?.sessionManager?.getLeafId?.();
return leaf ?? undefined;
}
async function defaultIsAncestorOfHead(baselineSha: string, worktreePath: string): Promise<boolean> {
try {
await execAsync(`git merge-base --is-ancestor ${baselineSha} HEAD`, { cwd: worktreePath });
return true;
} catch {
// Non-zero exit → not an ancestor.
return false;
}
}
function errMsg(err: unknown): string {
return err instanceof Error ? err.message : String(err);
}