fix: exclude dead workers from reassignment to break orchestrator deadlock#344
Open
Matthew-Selvam wants to merge 1 commit into
Open
Conversation
…dlock The stale-task recovery in handleExecutingPhase() reset a dead worker's task back to 'pending' but never touched the worker's own status. Since findBusyAgentForReassign() only excludes *idle* agents (not dead ones) when picking a busy worker to reassign a freed task to, the very next matching pass would hand the task straight back to the same dead worker. That worker dies again next tick, gets 'recovered' again, and reassigned again — an infinite recover-reassign-die loop that never completes the goal, which is what left orchestrator state permanently stuck in 'executing' and blocked all new goal creation. Fix: when a dead worker is detected during stale-task recovery, mark its children row 'dead' and update the agent tracker to match, so it's excluded from future reassignment. Also adds a defense-in-depth check in findBusyAgentForReassign() itself, via the existing isWorkerAlive() hook, in case a task ever ends up assigned to a dead worker through some other path. Adds regression tests: one confirming findBusyAgentForReassign() falls through to self-assignment rather than handing a task back to a worker isWorkerAlive() reports dead, and one confirming the stale-task recovery path in handleExecutingPhase() marks the dead worker's children row and tracker status so the same tick reassigns off it rather than looping back to it. Fixes Conway-Research#266, Fixes Conway-Research#259
Author
|
This is ready for maintainer review whenever you get a chance — happy to make any changes requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #266, fixes #259. The orchestrator's stale-task recovery (
handleExecutingPhase) resets a dead worker's task back topendingwhenisWorkerAlive()says the worker is gone — but never updates the worker's own status. MeanwhilefindBusyAgentForReassign()only excludes idle agents when picking a busy worker to hand a freed task to, not dead ones. So the very next matching pass hands the recovered task straight back to the same dead worker. It dies again, gets "recovered" again, and reassigned again — an infinite recover-reassign-die loop that never completes the goal. That's what leavesorchestrator.statepermanently stuck inexecutingand blocks all new goal creation (both #266 and #259 describe this from different angles — a low-RAM VM where workers OOM, and a stalelocal://worker after a restart).Fix
childrenrow'dead'and callagentTracker.updateStatus(address, "dead"), so it's excluded from future reassignment.findBusyAgentForReassign()now also checks the existingisWorkerAlive()hook directly, in case a task ever ends up assigned to a dead worker through some other path than the recovery block above.Testing
tsc --noEmitpasses.findBusyAgentForReassign(viamatchTaskToAgent) falls through to self-assignment rather than handing a task back to a workerisWorkerAlivereports dead.handleExecutingPhase(viatick()) marks the dead worker'schildrenrow and tracker status, and — since the worker is now excluded — the same tick reassigns the freed task off it instead of looping back to it.orchestrator.test.ts,messaging.test.ts,task-graph.test.ts,orchestrator-harness.test.ts,local-worker-harness.test.ts,attention.test.ts,workspace.test.ts,local-worker-security.test.ts).