fix(KB-111): implement invalid transition handling in executor
- Detect and handle invalid column transitions during task step updates - Reorder error checks to catch invalid transitions before paused status - Improve log messages with actual column names for better debugging - Add comprehensive tests for invalid transition error scenarios - Include changeset for the invalid transition fix
This commit is contained in:
@@ -3181,3 +3181,104 @@ describe("Per-task model overrides", () => {
|
||||
expect(capturedOptions[0].defaultModelId).toBe("gpt-4o");
|
||||
});
|
||||
});
|
||||
|
||||
// ── Invalid transition error handling tests ─────────────────────────
|
||||
|
||||
describe("Invalid transition error handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
});
|
||||
|
||||
it("does not mark task as failed when invalid transition error occurs on completion", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
// Mock moveTask to throw invalid transition error (task already moved to done)
|
||||
store.moveTask.mockRejectedValue(
|
||||
new Error("Invalid transition: 'done' → 'in-review'. Valid targets: none"),
|
||||
);
|
||||
|
||||
// Mock agent that completes successfully
|
||||
mockedCreateHaiAgent.mockImplementation(async () => {
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Agent completes work but moveTask will fail
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: {
|
||||
getLeafId: vi.fn(),
|
||||
branchWithSummary: vi.fn(),
|
||||
},
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "KB-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Should NOT mark task as failed
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("KB-001", { status: "failed", error: expect.any(String) });
|
||||
|
||||
// Should log informative message
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"KB-001",
|
||||
"Task already moved from 'done' — skipping transition to 'in-review'",
|
||||
expect.stringContaining("Invalid transition"),
|
||||
);
|
||||
});
|
||||
|
||||
it("calls onComplete when invalid transition occurs after successful execution", async () => {
|
||||
const store = createMockStore();
|
||||
const onComplete = vi.fn();
|
||||
|
||||
// Mock moveTask to throw invalid transition error
|
||||
store.moveTask.mockRejectedValue(
|
||||
new Error("Invalid transition: 'in-progress' → 'in-review'. Valid targets: todo, triage"),
|
||||
);
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => {
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: {
|
||||
getLeafId: vi.fn(),
|
||||
branchWithSummary: vi.fn(),
|
||||
},
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onComplete });
|
||||
await executor.execute({
|
||||
id: "KB-002",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// onComplete should be called even when invalid transition occurs
|
||||
expect(onComplete).toHaveBeenCalled();
|
||||
expect(onComplete).toHaveBeenCalledWith(expect.objectContaining({ id: "KB-002" }));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -513,6 +513,17 @@ export class TaskExecutor {
|
||||
// Dependency added mid-execution — discard worktree and move to triage
|
||||
this.depAborted.delete(task.id);
|
||||
await this.handleDepAbortCleanup(task.id, worktreePath);
|
||||
} else if (err.message?.includes("Invalid transition")) {
|
||||
// Task was moved by user/process while executor was running — already in desired state
|
||||
// This check must come before pausedAborted since it's more specific
|
||||
const transitionMatch = err.message.match(/Invalid transition: '([^']+)' → '([^']+)'/);
|
||||
const fromColumn = transitionMatch?.[1] ?? "unknown";
|
||||
const toColumn = transitionMatch?.[2] ?? "unknown";
|
||||
const logMessage = `Task already moved from '${fromColumn}' — skipping transition to '${toColumn}'`;
|
||||
executorLog.log(`${task.id} ${logMessage}`);
|
||||
await this.store.logEntry(task.id, logMessage, err.message);
|
||||
// Task finished successfully (just already moved), so call onComplete
|
||||
this.options.onComplete?.(task);
|
||||
} else if (this.pausedAborted.has(task.id)) {
|
||||
// Task was paused mid-execution — move to todo, don't mark as failed
|
||||
executorLog.log(`${task.id} paused — moving to todo`);
|
||||
|
||||
Reference in New Issue
Block a user