Prevent auto-commit after cancelled units

This commit is contained in:
Mikael Hugo 2026-04-30 09:07:44 +02:00
parent 8487507d1b
commit e62b3854cb
2 changed files with 109 additions and 2 deletions

View file

@ -109,6 +109,28 @@ export function _resolveReportBasePath(
return s.originalBasePath || s.basePath;
}
function clearDeferredCommitAfterCancelledUnit(
s: AutoSession,
ctx: ExtensionContext,
unitType: string,
unitId: string,
reason: string,
): void {
if (!s.stagedPendingCommit && !s.pendingCommitTaskContext) return;
s.stagedPendingCommit = false;
s.pendingCommitTaskContext = null;
debugLog("autoLoop", {
phase: "cancelled-unit-deferred-commit-cleared",
unitType,
unitId,
reason,
});
ctx.ui.notify(
`Cancelled ${unitType} ${unitId}; staged changes were preserved for recovery and not auto-committed.`,
"warning",
);
}
/**
* Resolve the authoritative project base for dispatch guards.
* Prior-milestone completion lives at the project root, even when the active
@ -1874,6 +1896,13 @@ export async function runUnitPhase(
}
if (unitResult.status === "cancelled") {
clearDeferredCommitAfterCancelledUnit(
s,
ctx,
unitType,
unitId,
unitResult.errorContext?.message ?? "cancelled",
);
// Provider-error pause: pauseAuto already handled cleanup and scheduled
// recovery. Don't hard-stop — just break out of the loop (#2762).
if (unitResult.errorContext?.category === "provider") {
@ -1910,7 +1939,6 @@ export async function runUnitPhase(
unitId,
});
await deps.pauseAuto(ctx, pi);
await deps.autoCommitUnit?.(s.basePath, unitType, unitId, ctx);
await emitCancelledUnitEnd(
ic,
unitType,
@ -1931,7 +1959,6 @@ export async function runUnitPhase(
deps.buildSnapshotOpts(unitType, unitId),
);
}
await deps.autoCommitUnit?.(s.basePath, unitType, unitId, ctx);
await emitCancelledUnitEnd(
ic,
unitType,

View file

@ -0,0 +1,80 @@
/**
* Regression guard: cancelled units must not create normal auto-commits.
*
* A runaway-paused execute-task can return `status: "cancelled"` after the
* agent has staged work. That state is recovery territory, not a completed
* task. The cancelled branch must clear the deferred-commit flag and leave
* the operator/recovery path to decide what to do with the patch.
*/
import assert from "node:assert/strict";
import { readFileSync } from "node:fs";
import { join } from "node:path";
import { test } from "node:test";
const src = readFileSync(
join(import.meta.dirname, "..", "auto", "phases.ts"),
"utf-8",
);
function cancelledBranchBody(): string {
const marker = 'if (unitResult.status === "cancelled") {';
const start = src.indexOf(marker);
assert.notEqual(start, -1, "cancelled-unit branch must exist");
const nextMarker = "\n\t// \u2500\u2500 Immediate unit closeout";
const end = src.indexOf(nextMarker, start);
assert.notEqual(end, -1, "cancelled-unit branch end marker must exist");
return src.slice(start, end);
}
test("cancelled unit branch does not call autoCommitUnit", () => {
const body = cancelledBranchBody();
assert.equal(
body.includes("autoCommitUnit"),
false,
"cancelled units must not call normal autoCommitUnit",
);
});
test("cancelled unit branch clears deferred commit state before returning", () => {
const body = cancelledBranchBody();
const callIndex = body.indexOf("clearDeferredCommitAfterCancelledUnit(");
assert.ok(callIndex >= 0, "cancelled units must clear deferred commit state");
const providerReturnIndex = body.indexOf('reason: "provider-pause"');
assert.ok(
providerReturnIndex > callIndex,
"deferred commit state must be cleared before provider cancellation exits",
);
const timeoutReturnIndex = body.indexOf('reason: "session-timeout"');
assert.ok(
timeoutReturnIndex > callIndex,
"deferred commit state must be cleared before timeout cancellation exits",
);
const failedReturnIndex = body.indexOf('reason: "session-failed"');
assert.ok(
failedReturnIndex > callIndex,
"deferred commit state must be cleared before failed cancellation exits",
);
});
test("cancelled unit clear helper resets commit flags without committing", () => {
const helperName = "function clearDeferredCommitAfterCancelledUnit";
const start = src.indexOf(helperName);
assert.notEqual(start, -1, "clear helper must exist");
const end = src.indexOf("\n}\n", start + helperName.length);
assert.notEqual(end, -1, "clear helper must have a body");
const helper = src.slice(start, end);
assert.ok(
helper.includes("s.stagedPendingCommit = false"),
"helper must clear stagedPendingCommit",
);
assert.ok(
helper.includes("s.pendingCommitTaskContext = null"),
"helper must clear pendingCommitTaskContext",
);
assert.equal(
helper.includes("commitStaged(") || helper.includes("autoCommit"),
false,
"clear helper must not commit",
);
});