Restore task grouping in sidebar for pipeline execution path#692
Merged
Conversation
The pipeline/bridge execution path (default since #659) created instances via InstanceFactory.CreateInstance() but never added them to the ultraplan's InstanceGroup, causing execution tasks to appear ungrouped in the sidebar. Wire SessionRecorderDeps.OnAssign to Coordinator.AssignTaskInstance, which populates TaskToInstance (for correct subgroup routing) then adds the instance to the correct "Group N" subgroup. The method serializes under c.mu to prevent concurrent map read/write between parallel bridge goroutines. Also fix ordering in the legacy ExecutionOrchestrator path where AddInstanceToGroup was called before AssignTaskToInstance, causing determineSubgroupType to return Unknown instead of Execution.
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
InstanceFactory.CreateInstance()but never registered them with the ultraplanInstanceGroup, causing tasks to appear ungrouped in the sidebarSessionRecorderDeps.OnAssignto newCoordinator.AssignTaskInstancemethod, which populatesTaskToInstancefirst (for correct "Group N" subgroup routing) then adds the instance to the groupExecutionOrchestratorpath whereAddInstanceToGroupwas called beforeAssignTaskToInstanceAssignTaskInstanceunderc.muto prevent concurrent map read/write between parallel bridge goroutinesTest plan
TestCoordinator_AssignTaskInstancewith 6 subtests: happy path, nil guards, subgroup fallback, multi-pass, and concurrent race detectiongo test -racego test ./...)go vet ./...andgofmtclean