fix(agents): fix failure propagation in lead agent dispatch chain#467
Merged
Conversation
✅ Dependency Audit
See the Security tab for detailed findings. Workflow: Dependency Audit |
Security Scan Results
Recommendations
Workflow: Security Scanning |
mvillmow
force-pushed
the
lead-agent-failures
branch
3 times, most recently
from
368fad3 to
2f29039
Compare
mvillmow
force-pushed
the
lead-agent-failures
branch
from
339dd48 to
ac70c80
Compare
- isSubordinateResult() (ModuleLeadAgent) now explicitly excludes TASK_FAILED messages so they always reach processSubordinateFailure() and are never silently treated as successes (Closes #184). - processSubordinateFailure() (LeadAgentBase default) now sends a TASK_FAILED message to coordination_.getRequesterId() once all subordinates have reported terminal events, preventing the ComponentLeadAgent from remaining stuck in WAITING_FOR_MODULES permanently (Closes #185). - Added processTaskResult() to ModuleLeadAgent and ComponentLeadAgent for the gRPC async result path; calls coordination_.recordFailure() when hmas::TaskResult::success() is false so hasFailures() is set correctly and upward propagation fires (Closes #186). - Added unit tests covering all three scenarios, including a three- level integration test that verifies TASK_FAILED arrives at the grandparent inbox. Closes #184 Closes #185 Closes #186 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mvillmow
force-pushed
the
lead-agent-failures
branch
from
ac70c80 to
548b8dc
Compare
Two bugs: 1. processSubordinateResult() transitioned to SYNTHESIZING even when prior failures existed — missing hasFailures() check. Now transitions to ERROR when all subtasks complete but any failed (mirrors gRPC path). 2. FailurePropagatedUpwardToRequester test sent module goal from 'chief' instead of 'component_1', so requester_id_ was set to an unregistered agent and the upward TASK_FAILED was silently dropped. Closes #184 #185 #186 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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
ModuleLeadAgent::isSubordinateResult()now explicitly excludesTASK_FAILEDmessages (action_type != TASK_FAILED) so failures are never silently treated as successes regardless of dispatch ordering.LeadAgentBase::processSubordinateFailure()(default impl) now sends aTASK_FAILEDmessage tocoordination_.getRequesterId()once all subordinates have reported terminal events, preventingComponentLeadAgentfrom remaining permanently stuck inWAITING_FOR_MODULES.processTaskResult(subtask_id, TaskResult)to bothModuleLeadAgentandComponentLeadAgent(gRPC path); callscoordination_.recordFailure()whenresult.success()is false sohasFailures()is set correctly and upward propagation fires via the gRPC async callback.Test plan
ModuleLeadAgentTest.TaskFailedNotTreatedAsSuccessResult— verifies isSubordinateResult() intercepts TASK_FAILED before processSubordinateFailure() #184:TASK_FAILED+command=="response"ends in ERROR, not SYNTHESIZINGModuleLeadAgentTest.FailurePropagatedUpwardToRequester— verifies processSubordinateFailure() does not propagate failure upward to grandparent #185: three-level integration test confirmsComponentLeadAgentinbox receivesTASK_FAILEDModuleLeadAgentTest.FailureNotPropagatedUntilAllTerminal— verifies upward propagation is deferred until all subtasks are terminalComponentLeadAgentTest.ModuleFailurePropagatedUpwardToRequester— verifies processSubordinateFailure() does not propagate failure upward to grandparent #185 from ComponentLead perspectiveSingleTaskFailureTransitionsToError,SynthesizeAfterFailureReturnsErrorMessage,FailureBeforeAllResultsDoesNotDeadlock,SuccessResultAfterFailureStillCountsTowardCompletioncontinue to passjust format-checkpassesCloses #184
Closes #185
Closes #186
🤖 Generated with Claude Code