fix(pi-workflow): preserve recovery gate ownership
This commit is contained in:
@@ -1508,6 +1508,147 @@ test("Git or ownership safety failures open Recovery before remediation", async
|
||||
});
|
||||
});
|
||||
|
||||
test("Recovery retry reruns failed verification after the remediation limit", async () => {
|
||||
await withTempDir(async (directory) => {
|
||||
const setup = await fixture(directory);
|
||||
let commandRuns = 0;
|
||||
const hostCommands: HostCommandExecutor = {
|
||||
async run() {
|
||||
commandRuns++;
|
||||
return {
|
||||
result: {
|
||||
code: commandRuns === 1 ? 1 : 0,
|
||||
stdout: commandRuns === 1 ? "failed" : "passed",
|
||||
stderr: "",
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
const effectiveConfig = {
|
||||
...structuredClone(DEFAULT_WORKFLOW_CONFIG),
|
||||
maxReviewCycles: 1,
|
||||
};
|
||||
setup.executor.enqueue("discover", {
|
||||
...discoveryWithQuestions(),
|
||||
questions: [],
|
||||
});
|
||||
setup.executor.enqueue("design", design("minor"));
|
||||
setup.executor.enqueue("implement", {
|
||||
summary: "Initial implementation",
|
||||
changedPaths: ["src/index.ts"],
|
||||
focusedTests: ["initial test"],
|
||||
});
|
||||
setup.executor.enqueue("verify", {
|
||||
succeeded: true,
|
||||
focusedBehavioralTests: true,
|
||||
summary: "Corrected implementation verified",
|
||||
});
|
||||
setup.executor.enqueue("review", {
|
||||
specification: { verdict: "approved", findings: [] },
|
||||
standards: { verdict: "approved", findings: [] },
|
||||
});
|
||||
|
||||
const planned = await setup.engine("session-1", {
|
||||
hostCommands,
|
||||
effectiveConfig,
|
||||
}).execute(startCommand());
|
||||
const approved = await setup.engine("session-1", {
|
||||
hostCommands,
|
||||
effectiveConfig,
|
||||
}).execute({
|
||||
action: "decide",
|
||||
runId: planned.runId,
|
||||
gateId: planned.openGate?.id ?? "missing",
|
||||
expectedRevision: planned.revision,
|
||||
decision: {
|
||||
kind: "approve-plan",
|
||||
approved: true,
|
||||
acceptedUiImpact: "minor",
|
||||
},
|
||||
idempotencyKey: "approve-failed-verification-retry",
|
||||
});
|
||||
const waiting = await setup.engine("session-1", {
|
||||
hostCommands,
|
||||
effectiveConfig,
|
||||
}).execute({
|
||||
action: "decide",
|
||||
runId: approved.runId,
|
||||
gateId: approved.openGate?.id ?? "missing",
|
||||
expectedRevision: approved.revision,
|
||||
decision: {
|
||||
kind: "select-implementation-context",
|
||||
strategy: "current-session",
|
||||
},
|
||||
idempotencyKey: "context-failed-verification-retry",
|
||||
});
|
||||
assert.equal(waiting.openGate?.kind, "recovery");
|
||||
assert.equal(commandRuns, 1);
|
||||
|
||||
const transientRecovery = setup.engine("transient-session", {
|
||||
hostCommands,
|
||||
effectiveConfig,
|
||||
processId: 200,
|
||||
isProcessAlive: () => false,
|
||||
});
|
||||
assert.deepEqual(await transientRecovery.recover(), []);
|
||||
const untouched = await setup.store.loadRun(waiting.runId);
|
||||
assert.equal(untouched.revision, waiting.revision);
|
||||
assert.equal(untouched.lease?.activeParentSessionId, "session-1");
|
||||
|
||||
const liveStolen = structuredClone(untouched);
|
||||
liveStolen.revision++;
|
||||
liveStolen.parentSessionIds.push("transient-session");
|
||||
liveStolen.lease = {
|
||||
processId: 200,
|
||||
activeParentSessionId: "transient-session",
|
||||
heartbeatAt: 0,
|
||||
worktreeIdentity: liveStolen.workspace.root,
|
||||
};
|
||||
await setup.store.saveRun(liveStolen, untouched.revision);
|
||||
|
||||
await assert.rejects(
|
||||
setup.engine("session-1", {
|
||||
hostCommands,
|
||||
effectiveConfig,
|
||||
processId: 300,
|
||||
isProcessAlive: (processId) => processId === 200,
|
||||
}).execute({
|
||||
action: "decide",
|
||||
runId: waiting.runId,
|
||||
gateId: waiting.openGate?.id ?? "missing",
|
||||
expectedRevision: liveStolen.revision,
|
||||
decision: { kind: "recover", resolution: "retry" },
|
||||
idempotencyKey: "live-owner-retry-failed-verification",
|
||||
}),
|
||||
/Historical parent sessions/,
|
||||
);
|
||||
|
||||
const staleStolen = structuredClone(liveStolen);
|
||||
staleStolen.revision++;
|
||||
staleStolen.lease!.processId = 201;
|
||||
await setup.store.saveRun(staleStolen, liveStolen.revision);
|
||||
|
||||
const completed = await setup.engine("session-1", {
|
||||
hostCommands,
|
||||
effectiveConfig,
|
||||
processId: 300,
|
||||
isProcessAlive: () => false,
|
||||
}).execute({
|
||||
action: "decide",
|
||||
runId: waiting.runId,
|
||||
gateId: waiting.openGate?.id ?? "missing",
|
||||
expectedRevision: staleStolen.revision,
|
||||
decision: { kind: "recover", resolution: "retry" },
|
||||
idempotencyKey: "retry-failed-verification",
|
||||
});
|
||||
|
||||
assert.equal(completed.status, "completed");
|
||||
assert.equal(commandRuns, 2);
|
||||
const persisted = await setup.store.loadRun(completed.runId);
|
||||
assert.equal(persisted.verificationEvidence?.commands[0]?.status, "succeeded");
|
||||
});
|
||||
});
|
||||
|
||||
test("changes requested trigger serialized remediation, verification, and fresh review", async () => {
|
||||
await withTempDir(async (directory) => {
|
||||
const setup = await fixture(directory);
|
||||
|
||||
@@ -1261,7 +1261,7 @@ export class WorkflowEngine {
|
||||
validateWorkflowCommand(command);
|
||||
if (command.action === "start") return this.start(command);
|
||||
if (command.action === "start-analysis") return this.startAnalysis(command);
|
||||
const run = await this.options.store.loadRun(command.runId);
|
||||
let run = await this.options.store.loadRun(command.runId);
|
||||
if (command.action === "inspect") return view(run);
|
||||
if (command.action === "consume-context-handoff") {
|
||||
return this.consumeContextHandoff(run, command);
|
||||
@@ -1269,6 +1269,7 @@ export class WorkflowEngine {
|
||||
if (command.action === "recover-context-intent") {
|
||||
return this.recoverContextIntent(run, command);
|
||||
}
|
||||
run = this.reclaimStaleRecoveryLease(run, command);
|
||||
this.assertActiveParent(run);
|
||||
if (command.action === "abort") return this.abort(run, command);
|
||||
if (command.action === "decide") return this.decide(run, command);
|
||||
@@ -1332,6 +1333,8 @@ export class WorkflowEngine {
|
||||
assignment.status === "running" ||
|
||||
(assignment.status === "queued" && Boolean(assignment.executionToken)),
|
||||
);
|
||||
const alreadyWaiting = activeGate(run)?.kind === "recovery";
|
||||
if (running.length === 0 && alreadyWaiting) continue;
|
||||
if (await this.hasActiveAssignmentLease(run.runId, running)) continue;
|
||||
if (!this.leaseIsStale(run)) continue;
|
||||
run = await this.transition(run, (next) => {
|
||||
@@ -1362,8 +1365,6 @@ export class WorkflowEngine {
|
||||
);
|
||||
}
|
||||
}
|
||||
const alreadyWaiting = activeGate(run)?.kind === "recovery";
|
||||
if (running.length === 0 && alreadyWaiting) continue;
|
||||
run = await this.transition(run, (next) => {
|
||||
const interruptedIds = new Set(running.map((assignment) => assignment.id));
|
||||
for (const assignment of next.assignments) {
|
||||
@@ -4519,6 +4520,31 @@ export class WorkflowEngine {
|
||||
});
|
||||
}
|
||||
|
||||
private reclaimStaleRecoveryLease(
|
||||
run: WorkflowRun,
|
||||
command: WorkflowCommand,
|
||||
) {
|
||||
if (
|
||||
command.action !== "decide" ||
|
||||
command.decision.kind !== "recover" ||
|
||||
command.decision.resolution !== "retry" ||
|
||||
activeGate(run)?.id !== command.gateId ||
|
||||
!run.parentSessionIds.includes(this.options.parentSessionId) ||
|
||||
!run.lease ||
|
||||
this.isProcessAlive(run.lease.processId)
|
||||
) {
|
||||
return run;
|
||||
}
|
||||
const claimed = structuredClone(run);
|
||||
claimed.lease = {
|
||||
processId: this.processId,
|
||||
activeParentSessionId: this.options.parentSessionId,
|
||||
heartbeatAt: this.now(),
|
||||
worktreeIdentity: claimed.workspace.root,
|
||||
};
|
||||
return claimed;
|
||||
}
|
||||
|
||||
private assertActiveParent(run: WorkflowRun) {
|
||||
if (
|
||||
run.lease &&
|
||||
|
||||
Reference in New Issue
Block a user