From 96c6cd625a6e60282959f0f2b1d07a74a8b8770f Mon Sep 17 00:00:00 2001 From: Adrian Komorek Date: Mon, 11 May 2026 13:29:27 +0200 Subject: [PATCH] fix(smithy): orphan recovery dispatches highest-priority task first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `recoverOrphanedAssignments` selected the first element from `getAgentTasks` without sorting, so the recovered task depended on storage insertion order rather than priority (P3 before P1 if created first). Adds ascending sort by `task.priority` before selecting the task. Adds a regression test: P3 created before P1 — confirms P1 is always recovered first. Co-Authored-By: Claude Sonnet 4.6 --- .../orphan-recovery-priority-ordering.md | 9 +++ .../src/services/dispatch-daemon.bun.test.ts | 58 +++++++++++++++++++ .../smithy/src/services/dispatch-daemon.ts | 7 ++- 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 .changeset/orphan-recovery-priority-ordering.md diff --git a/.changeset/orphan-recovery-priority-ordering.md b/.changeset/orphan-recovery-priority-ordering.md new file mode 100644 index 00000000..b54092e7 --- /dev/null +++ b/.changeset/orphan-recovery-priority-ordering.md @@ -0,0 +1,9 @@ +--- +"@stoneforge/smithy": patch +--- + +fix(smithy): orphan recovery dispatches highest-priority task first + +`recoverOrphanedAssignments` was selecting the first element from `getAgentTasks` without sorting, so the task recovered depended on storage insertion order rather than priority. A P3 task created before a P1 task would be dispatched first. + +Adds an ascending sort by `task.priority` before selecting the task to recover, matching the intent of priority-based scheduling. Adds a regression test with a P3 task created before a P1 task to confirm the P1 task is always recovered first. diff --git a/packages/smithy/src/services/dispatch-daemon.bun.test.ts b/packages/smithy/src/services/dispatch-daemon.bun.test.ts index d278cf55..de995239 100644 --- a/packages/smithy/src/services/dispatch-daemon.bun.test.ts +++ b/packages/smithy/src/services/dispatch-daemon.bun.test.ts @@ -693,6 +693,64 @@ describe('recoverOrphanedAssignments', () => { expect(result.processed).toBe(0); }); + + test('recovers highest-priority task first when multiple are assigned', async () => { + const { createTask: createTaskFn } = await import('@stoneforge/core'); + + const worker = await createTestWorker('priority-grace'); + const workerId = worker.id as unknown as EntityId; + + // Create P3 (low) task FIRST — naive creation-order selection would pick this + const lowPriorityTaskData = await createTaskFn({ + title: 'Low priority task', + createdBy: systemEntity, + status: TaskStatus.OPEN, + priority: Priority.LOW, + assignee: workerId, + }); + const savedLow = await api.create(lowPriorityTaskData as unknown as Record & { createdBy: EntityId }) as Task; + await api.update(savedLow.id, { + metadata: updateOrchestratorTaskMeta(undefined, { + assignedAgent: workerId, + branch: 'agent/priority-grace/low-task-branch', + worktree: '/worktrees/priority-grace/low-task', + }), + }); + + // Create P1 (critical) task SECOND + const highPriorityTaskData = await createTaskFn({ + title: 'High priority task', + createdBy: systemEntity, + status: TaskStatus.OPEN, + priority: Priority.CRITICAL, + assignee: workerId, + }); + const savedHigh = await api.create(highPriorityTaskData as unknown as Record & { createdBy: EntityId }) as Task; + await api.update(savedHigh.id, { + metadata: updateOrchestratorTaskMeta(undefined, { + assignedAgent: workerId, + branch: 'agent/priority-grace/high-task-branch', + worktree: '/worktrees/priority-grace/high-task', + }), + }); + + const result = await daemon.recoverOrphanedAssignments(); + + expect(result.processed).toBe(1); + // Must have spawned a session in the HIGH priority task's worktree, not the low one + expect(sessionManager.startSession).toHaveBeenCalledWith( + workerId, + expect.objectContaining({ + workingDirectory: '/worktrees/priority-grace/high-task', + }) + ); + expect(sessionManager.startSession).not.toHaveBeenCalledWith( + workerId, + expect.objectContaining({ + workingDirectory: '/worktrees/priority-grace/low-task', + }) + ); + }); }); describe('pollWorkflowTasks - merge steward dispatch', () => { diff --git a/packages/smithy/src/services/dispatch-daemon.ts b/packages/smithy/src/services/dispatch-daemon.ts index 550e7989..27cc824e 100644 --- a/packages/smithy/src/services/dispatch-daemon.ts +++ b/packages/smithy/src/services/dispatch-daemon.ts @@ -1248,7 +1248,12 @@ export class DispatchDaemonImpl implements DispatchDaemon { } // 4. Check if the task is stuck in a resume loop - const taskAssignment = workerTasks[0]; + // Sort by priority ascending (lower number = higher priority) so the + // highest-priority assigned task is always recovered first. + const sortedWorkerTasks = [...workerTasks].sort( + (a, b) => (a.task.priority ?? 99) - (b.task.priority ?? 99) + ); + const taskAssignment = sortedWorkerTasks[0]; let resumeCount = taskAssignment.orchestratorMeta?.resumeCount ?? 0; const maxResumes = this.config.maxResumeAttemptsBeforeRecovery;