Fix race condition in task result caching and wire result streamer#1
Merged
Conversation
- Fix race condition where tasks marked 'completed' before result cached - Reordered handleTaskSuccess() to publish/cache result BEFORE updating state - Prevents clients from seeing 'completed' status before result available - Wire result streamer to task queue - Created taskQueueResultStreamerAdapter to bridge coordinator and taskqueue interfaces - Added SetResultStreamer() call in server initialization - Converts between coordinator and taskqueue types (TaskResult, TaskProgress, TaskResultNotification) - Add retry logic in task.get_result handler - Retries cache lookup for completed/failed tasks to handle remaining race window - Returns clear error if result genuinely missing - Complete Phase 3: Artifact Management implementation - Added artifact.get MCP tool (coordinator handler and worker implementation) - Created gRPC Artifact Service for registration and listing - Updated documentation with artifact.get tool reference - Added E2E test for artifact creation and retrieval All tests passing including E2E integration tests.
- Added 8 test cases covering all code paths in get_handler.go - Tests cover: no workers, missing params, success case, session creation, error handling for enqueue/get task failures, and context session ID - Achieved 97.2% test coverage on artifact handler - Fixed config tools test to include new artifact.get tool (expected 9 tools instead of 8) Coverage breakdown: - artifact.get handler: 97.2% coverage - All new code paths tested with proper mocks - Follows existing test patterns from python/task handlers
- Add tests for taskQueueResultStreamerAdapter in coordinator - Add TestTaskExecutorArtifactGet for artifact.get execution path - Add TestHandleArtifactGetTyped with 3 test cases covering success and error paths - Artifact handler tests achieve comprehensive coverage of new code
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.
Pull Request Summary
Type of Change
Component(s) Affected
Description
What does this PR do?
Fixes a race condition where tasks were marked as 'completed' before their results were cached, and wires the result streamer to the task queue to enable proper result publishing.
Why is this change needed?
E2E tests were failing with "Task completed but result not available in cache" errors. The root cause was:
Changes Made
Code Changes
handleTaskSuccess()to cache result BEFORE updating task statetaskQueueResultStreamerAdapterto bridge coordinator and taskqueue interfacesSetResultStreamer()call in server initializationtask.get_resulthandler for race window mitigationartifact.getMCP tool)Testing
Test Coverage
Test Results
Documentation
Documentation Updates
Checklist