fix(FN-985): clear transient fields when archiving and moving tasks to done
- Clear prInfo, issueInfo, and modelPresetId in unarchiveTask to avoid stale badge/model data - Clear prInfo, issueInfo, and modelPresetId in moveToDone after successful merge - Add regression tests for transient field clearing in both code paths - Fix existing tests to use public API instead of private methods
This commit is contained in:
@@ -2688,6 +2688,24 @@ Task with acceptance criteria
|
||||
expect(moved.blockedBy).toBeUndefined();
|
||||
});
|
||||
|
||||
it("clears recovery fields when moving to done (FN-985 regression)", async () => {
|
||||
const task = await store.createTask({ description: "test recovery fields" });
|
||||
await store.moveTask(task.id, "todo");
|
||||
await store.moveTask(task.id, "in-progress");
|
||||
|
||||
// Set recovery metadata via updateTask
|
||||
await store.updateTask(task.id, {
|
||||
recoveryRetryCount: 3,
|
||||
nextRecoveryAt: new Date(Date.now() + 86400000).toISOString(),
|
||||
});
|
||||
|
||||
await store.moveTask(task.id, "in-review");
|
||||
const moved = await store.moveTask(task.id, "done");
|
||||
expect(moved.column).toBe("done");
|
||||
expect(moved.recoveryRetryCount).toBeUndefined();
|
||||
expect(moved.nextRecoveryAt).toBeUndefined();
|
||||
});
|
||||
|
||||
it("blocks moving failed in-review tasks to done", async () => {
|
||||
const task = await store.createTask({ description: "test block failed review task" });
|
||||
await store.moveTask(task.id, "todo");
|
||||
@@ -3517,6 +3535,41 @@ Task with acceptance criteria
|
||||
|
||||
await expect(store.unarchiveTask(task.id)).rejects.toThrow("must be in 'archived'");
|
||||
});
|
||||
|
||||
it("clears transient fields when unarchiving (FN-985 regression)", async () => {
|
||||
// Simulate a task that completed normally and was archived,
|
||||
// but somehow accumulated stale transient state.
|
||||
const task = await store.createTask({ description: "Test task" });
|
||||
await store.moveTask(task.id, "todo");
|
||||
await store.moveTask(task.id, "in-progress");
|
||||
await store.moveTask(task.id, "in-review");
|
||||
await store.moveTask(task.id, "done");
|
||||
|
||||
// After reaching done, inject stale transient fields via updateTask
|
||||
// (simulating state that could leak through if transient clearing was incomplete)
|
||||
await store.updateTask(task.id, {
|
||||
status: "failed",
|
||||
error: "Something went wrong",
|
||||
worktree: "/tmp/old-worktree",
|
||||
blockedBy: "FN-999",
|
||||
recoveryRetryCount: 3,
|
||||
nextRecoveryAt: new Date(Date.now() + 86400000).toISOString(),
|
||||
});
|
||||
|
||||
// Archive the task with stale state
|
||||
await store.archiveTask(task.id);
|
||||
|
||||
// Unarchive — should clear all transient fields
|
||||
const unarchived = await store.unarchiveTask(task.id);
|
||||
|
||||
expect(unarchived.column).toBe("done");
|
||||
expect(unarchived.status).toBeUndefined();
|
||||
expect(unarchived.error).toBeUndefined();
|
||||
expect(unarchived.worktree).toBeUndefined();
|
||||
expect(unarchived.blockedBy).toBeUndefined();
|
||||
expect(unarchived.recoveryRetryCount).toBeUndefined();
|
||||
expect(unarchived.nextRecoveryAt).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("archiveAllDone", () => {
|
||||
|
||||
@@ -1803,9 +1803,26 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
);
|
||||
}
|
||||
|
||||
// NOTE: No getTaskMergeBlocker check here — intentionally.
|
||||
// The merge blocker validates in-review → done transitions (ensuring code
|
||||
// has been properly reviewed before merging). An unarchived task was already
|
||||
// merged in its previous lifecycle; this is just a restoration. The transient
|
||||
// field clearing above ensures no stale blocker state leaks through.
|
||||
task.column = "done";
|
||||
task.columnMovedAt = new Date().toISOString();
|
||||
task.updatedAt = task.columnMovedAt;
|
||||
|
||||
// Clear transient fields that should not persist into "done" column.
|
||||
// Matches the clearing done by moveTask() for consistency — archived
|
||||
// tasks may have been archived with stale worktree/status/error/recovery
|
||||
// state that should not reappear after unarchiving.
|
||||
task.status = undefined;
|
||||
task.error = undefined;
|
||||
task.worktree = undefined;
|
||||
task.blockedBy = undefined;
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
|
||||
task.log.push({
|
||||
timestamp: task.columnMovedAt,
|
||||
action: "Task unarchived",
|
||||
@@ -1830,7 +1847,10 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
task.column = "done";
|
||||
task.worktree = undefined;
|
||||
task.status = undefined;
|
||||
task.error = undefined;
|
||||
task.blockedBy = undefined;
|
||||
task.recoveryRetryCount = undefined;
|
||||
task.nextRecoveryAt = undefined;
|
||||
task.columnMovedAt = new Date().toISOString();
|
||||
task.updatedAt = task.columnMovedAt;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user