Repository navigation
feat: add multi-task run support - #162
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds comprehensive support for coordinated multi-workflow task runs across the entire system stack: API contracts and routing, client methods, HTTP handlers, daemon orchestration with sequential child execution, CLI command with mode resolution and fallback behavior, remote tabbed UI for monitoring, workspace configuration for execution mode, and event system for lifecycle tracking. ChangesMulti-Workflow Orchestration Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (6)
internal/api/client/client_contract_test.go (3)
352-377: ⚡ Quick winRefactor to use table-driven test pattern.
This test checks two methods with identical nil-context validation logic. Consider using a table-driven test with subtests to reduce duplication and improve maintainability.
♻️ Refactor to table-driven test
func TestClientTaskRunMultipleMethodsRejectNilContext(t *testing.T) { t.Parallel() client := &Client{ target: Target{SocketPath: "/tmp/compozy.sock"}, baseURL: "http://daemon", httpClient: &http.Client{ Timeout: time.Second, Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { t.Fatal("unexpected transport call for nil context") return nil, nil }), }, } - var nilCtx context.Context - if _, err := client.StartTaskRunMultiple(nilCtx, apicore.TaskRunMultipleRequest{ - Workspace: "/tmp/workspace", - Slugs: []string{"alpha", "beta"}, - }); !errors.Is(err, ErrDaemonContextRequired) { - t.Fatalf("StartTaskRunMultiple(nil) error = %v, want %v", err, ErrDaemonContextRequired) - } - if _, err := client.GetTaskRunMultipleSnapshot(nilCtx, "multi-run-1"); !errors.Is(err, ErrDaemonContextRequired) { - t.Fatalf("GetTaskRunMultipleSnapshot(nil) error = %v, want %v", err, ErrDaemonContextRequired) - } + cases := []struct { + name string + invoke func(context.Context) error + }{ + { + name: "Should reject nil context in StartTaskRunMultiple", + invoke: func(ctx context.Context) error { + _, err := client.StartTaskRunMultiple(ctx, apicore.TaskRunMultipleRequest{ + Workspace: "/tmp/workspace", + Slugs: []string{"alpha", "beta"}, + }) + return err + }, + }, + { + name: "Should reject nil context in GetTaskRunMultipleSnapshot", + invoke: func(ctx context.Context) error { + _, err := client.GetTaskRunMultipleSnapshot(ctx, "multi-run-1") + return err + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var nilCtx context.Context + if err := tt.invoke(nilCtx); !errors.Is(err, ErrDaemonContextRequired) { + t.Fatalf("error = %v, want %v", err, ErrDaemonContextRequired) + } + }) + } }As per coding guidelines: "Use table-driven tests with subtests (t.Run) as the default pattern"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/client/client_contract_test.go` around lines 352 - 377, Test duplicates nil-context checks for StartTaskRunMultiple and GetTaskRunMultipleSnapshot; refactor TestClientTaskRunMultipleMethodsRejectNilContext into a table-driven test that iterates over cases (e.g., names "StartTaskRunMultiple" and "GetTaskRunMultipleSnapshot") and uses t.Run subtests to call each method with a shared nilCtx and assert errors.Is(err, ErrDaemonContextRequired); keep t.Parallel where appropriate, reuse the client setup and expected request args (apicore.TaskRunMultipleRequest and the snapshot ID) and reference the concrete methods StartTaskRunMultiple and GetTaskRunMultipleSnapshot so reviewers can locate the code to change.
98-146: ⚡ Quick winConsider wrapping test logic in a t.Run subtest.
The test uses
t.Parallel()but doesn't wrap the test logic in at.Runsubtest. As per coding guidelines, tests should use thet.Run("Should...")pattern.♻️ Refactor to use t.Run subtest
func TestClientStartTaskRunMultiplePostsOrderedSlugs(t *testing.T) { t.Parallel() + t.Run("Should POST ordered slugs to /api/task-runs/multiple", func(t *testing.T) { + t.Parallel() + client := &Client{ target: Target{SocketPath: "/tmp/compozy.sock"}, baseURL: "http://daemon", httpClient: &http.Client{ Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { if req.Method != http.MethodPost { t.Fatalf("method = %s, want POST", req.Method) } if req.URL.Path != "/api/task-runs/multiple" { t.Fatalf("path = %s, want /api/task-runs/multiple", req.URL.Path) } var body contract.TaskRunMultipleRequest if err := json.NewDecoder(req.Body).Decode(&body); err != nil { t.Fatalf("decode request body: %v", err) } if body.Workspace != "/tmp/workspace" { t.Fatalf("workspace = %q, want /tmp/workspace", body.Workspace) } if want := []string{"alpha", "beta", "gamma"}; !reflect.DeepEqual(body.Slugs, want) { t.Fatalf("slugs = %#v, want %#v", body.Slugs, want) } if body.Mode != "enqueued" || body.PresentationMode != "stream" { t.Fatalf("mode/presentation = %q/%q, want enqueued/stream", body.Mode, body.PresentationMode) } if string(body.RuntimeOverrides) != `{"persist":true}` { t.Fatalf("runtime_overrides = %s, want persist override", body.RuntimeOverrides) } return jsonResponse(http.StatusCreated, `{"run":{"run_id":"multi-run-1","mode":"task_multi"}}`), nil }), }, } run, err := client.StartTaskRunMultiple(context.Background(), apicore.TaskRunMultipleRequest{ Workspace: " /tmp/workspace ", Slugs: []string{" alpha ", "beta", " gamma "}, Mode: " enqueued ", PresentationMode: " stream ", RuntimeOverrides: json.RawMessage(`{"persist":true}`), }) if err != nil { t.Fatalf("StartTaskRunMultiple() error = %v", err) } if run.RunID != "multi-run-1" || run.Mode != "task_multi" { t.Fatalf("run = %#v, want multi-run parent", run) } + }) }As per coding guidelines: "MUST use t.Run("Should...") pattern for ALL test cases"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/client/client_contract_test.go` around lines 98 - 146, Wrap the existing test body inside a t.Run subtest named like "Should start multiple task run with ordered slugs" while keeping the top-level TestClientStartTaskRunMultiplePostsOrderedSlugs and its t.Parallel() call; move all existing logic (client setup, HTTP roundTripFunc checks, the StartTaskRunMultiple call and assertions on run) into the t.Run callback, and call t.Parallel() inside the subtest if you want the subtest to run in parallel as well so the test follows the t.Run("Should...") pattern and preserves parallelism and existing checks for StartTaskRunMultiple, the HTTP mock, and assertions.
148-193: ⚡ Quick winConsider wrapping test logic in a t.Run subtest.
The test uses
t.Parallel()but doesn't wrap the test logic in at.Runsubtest. As per coding guidelines, tests should use thet.Run("Should...")pattern.♻️ Refactor to use t.Run subtest
func TestClientGetTaskRunMultipleSnapshotUsesDedicatedRouteAndDecodes(t *testing.T) { t.Parallel() + t.Run("Should use dedicated route and decode snapshot", func(t *testing.T) { + t.Parallel() + want := contract.TaskRunMultipleSnapshot{ Run: contract.Run{ RunID: "multi-run-1", Mode: "task_multi", }, Items: []contract.TaskRunMultipleItem{ {Slug: "alpha", Status: "completed", RunID: "run-alpha"}, {Slug: "beta", Status: "failed", RunID: "run-beta", ErrorText: "boom"}, {Slug: "gamma", Status: "canceled"}, }, } // ... rest of test + }) }As per coding guidelines: "MUST use t.Run("Should...") pattern for ALL test cases"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/client/client_contract_test.go` around lines 148 - 193, The test function TestClientGetTaskRunMultipleSnapshotUsesDedicatedRouteAndDecodes currently calls t.Parallel() but does not wrap its assertions in a t.Run subtest; update the test to call t.Run("Should return decoded multiple snapshot" or similar) and move the existing body (including t.Parallel(), mock httpClient setup, call to client.GetTaskRunMultipleSnapshot and assertions) inside that t.Run closure so the test follows the required t.Run("Should...") pattern while preserving parallelism and behavior.internal/daemon/task_multi.go (1)
364-365: ⚡ Quick winAvoid
context.Background()in runtime cancellation paths.These cancellation calls drop caller context metadata and lifecycle intent. Prefer detaching from the current context instead of creating a brand-new background context.
Proposed fix
- childCancelErr = m.Cancel(detachContext(context.Background()), childRun.RunID) + childCancelErr = m.Cancel(detachContext(active.ctx), childRun.RunID) ... - cancelErr := m.Cancel(detachContext(context.Background()), childRun.RunID) + cancelErr := m.Cancel(detachContext(active.ctx), childRun.RunID) ... - if cancelErr := m.Cancel(detachContext(context.Background()), runID); cancelErr != nil { + if cancelErr := m.Cancel(detachContext(ctx), trimmedRunID); cancelErr != nil {As per coding guidelines: "Pass
context.Contextas the first argument to all functions crossing runtime boundaries; avoidcontext.Background()outsidemainand focused tests".Also applies to: 443-444, 508-509
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/task_multi.go` around lines 364 - 365, The call to m.Cancel currently uses detachContext(context.Background()) which drops caller context metadata and lifecycle intent; replace context.Background() with the current request/runtime context (e.g., the local ctx variable) and call detachContext(ctx) so cancellation detaches but preserves caller metadata. Update the same pattern used elsewhere in this file (the other m.Cancel/detachContext invocations) to use detachContext(ctx) and ensure the first argument to m.Cancel is a context.Context passed from the surrounding function rather than context.Background().internal/cli/commands_test.go (1)
113-134: ⚡ Quick winUse the required
t.Run("Should...")wrapper for this case.The command assertions look good; this should be wrapped in a named subtest to satisfy the repo test contract.
As per coding guidelines: "MUST use t.Run("Should...") pattern for ALL test cases".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/commands_test.go` around lines 113 - 134, Wrap the assertions in TestNewTasksCommandRegistersRunMultiple in a subtest using t.Run("Should register run-multiple with correct flags", func(t *testing.T) { ... }) so the test follows the repository contract; move the current body that creates cmd := newTasksCommand(nil, defaultCommandStateDefaults()), finds runMultiple, and checks its Name(), absence of --name flag, presence of --attach flag and its default into the t.Run closure and keep t.Parallel() at the top-level test.internal/daemon/run_manager_test.go (1)
104-132: ⚡ Quick winWrap this case in
t.Run("Should...")to match test contract.The new test is solid functionally, but it should follow the required subtest naming pattern used for test-case readability and consistency.
Suggested structure
func TestRunManagerIncludeCompletedStartsCompletedTaskWorkflow(t *testing.T) { - seenIncludeCompleted := make(chan bool, 1) - env := newRunManagerTestEnv(t, runManagerTestDeps{ - prepare: func(context.Context, *model.RuntimeConfig, model.RunScope) (*model.SolvePreparation, error) { - return &model.SolvePreparation{}, nil - }, - execute: func(_ context.Context, _ *model.SolvePreparation, cfg *model.RuntimeConfig) error { - seenIncludeCompleted <- cfg.IncludeCompleted - return nil - }, - }) - env.writeWorkflowFile(t, env.workflowSlug, "task_01.md", daemonTaskBody("completed", "Done task")) - - const runID = "task-run-include-completed" - run := env.startTaskRun( - t, - runID, - rawJSON(t, `{"run_id":"`+runID+`","include_completed":true}`), - ) - if !waitForBool(t, seenIncludeCompleted) { - t.Fatal("execute saw IncludeCompleted=false, want true") - } - terminal := waitForRun(t, env.globalDB, run.RunID, func(row globaldb.Run) bool { - return row.Status == runStatusCompleted - }) - if terminal.EndedAt == nil { - t.Fatal("EndedAt = nil, want terminal timestamp") - } + t.Run("Should start completed workflow when include_completed is true", func(t *testing.T) { + seenIncludeCompleted := make(chan bool, 1) + env := newRunManagerTestEnv(t, runManagerTestDeps{ + prepare: func(context.Context, *model.RuntimeConfig, model.RunScope) (*model.SolvePreparation, error) { + return &model.SolvePreparation{}, nil + }, + execute: func(_ context.Context, _ *model.SolvePreparation, cfg *model.RuntimeConfig) error { + seenIncludeCompleted <- cfg.IncludeCompleted + return nil + }, + }) + env.writeWorkflowFile(t, env.workflowSlug, "task_01.md", daemonTaskBody("completed", "Done task")) + + const runID = "task-run-include-completed" + run := env.startTaskRun( + t, + runID, + rawJSON(t, `{"run_id":"`+runID+`","include_completed":true}`), + ) + if !waitForBool(t, seenIncludeCompleted) { + t.Fatal("execute saw IncludeCompleted=false, want true") + } + terminal := waitForRun(t, env.globalDB, run.RunID, func(row globaldb.Run) bool { + return row.Status == runStatusCompleted + }) + if terminal.EndedAt == nil { + t.Fatal("EndedAt = nil, want terminal timestamp") + } + }) }As per coding guidelines: "MUST use t.Run("Should...") pattern for ALL test cases".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/run_manager_test.go` around lines 104 - 132, The test function TestRunManagerIncludeCompletedStartsCompletedTaskWorkflow must wrap its case body in a subtest using t.Run("Should ...") to follow the project's test contract; modify the function so the current test logic (the env setup, env.writeWorkflowFile, env.startTaskRun, waitForBool using seenIncludeCompleted, and waitForRun assertions) is moved into a t.Run call (e.g. t.Run("Should include completed tasks when include_completed=true", func(t *testing.T) { ... })) while preserving the existing symbols like seenIncludeCompleted, newRunManagerTestEnv, daemonTaskBody, env.startTaskRun, waitForBool and waitForRun and keeping all assertions intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/api/contract/contract_test.go`:
- Around line 409-410: Rename the subtest name used in the t.Run call so it
follows the required "Should..." convention; locate the t.Run("task run multiple
snapshot", func(t *testing.T) { ... }) and change the first argument to begin
with "Should" (e.g., "Should run task with multiple snapshots" or "Should handle
multiple task snapshots") so the subtest name starts with "Should".
In `@internal/api/core/handlers_contract_test.go`:
- Around line 170-177: Rename the test case entry whose name field is "task run
multiple" to follow the project's t.Run("Should...") convention (for example
"Should run multiple tasks") in the test table defined in
handlers_contract_test.go so the t.Run invocation downstream receives a
"Should..." string; update the name field of that test case (the struct literal
containing name, target, body, requestID, wantStatus, wantRun) to the new
"Should..." style to comply with the test naming policy.
In `@internal/api/core/handlers_smoke_test.go`:
- Around line 335-349: Rename the two test case names that currently read "task
run multiple" and "task run multiple snapshot" to follow the required
t.Run("Should...") pattern (for example "Should run multiple tasks" and "Should
return snapshot for multiple run") and ensure the table-driven test uses these
new strings when calling t.Run so the test harness executes them as
t.Run("Should..."). Locate the case entries in the test cases slice (the string
literals "task run multiple" and "task run multiple snapshot") and replace them
with the corresponding "Should..." names used by the caller.
In `@internal/cli/daemon_commands_test.go`:
- Around line 1039-1095: The subtests in daemon_commands_test.go use informal
names ("default enqueued", "parallel fallback", "invalid internal value",
"fallback write failure"); rename them to follow the required t.Run("Should
...") pattern (e.g., "Should default to enqueued", "Should fallback to enqueued
with warning", "Should return error for invalid internal value", "Should surface
write failure when fallback message can't be written"). Update the t.Run calls
that create CommandState via newCommandState(commandKindTasksRunMultiple,
core.ModePRDTasks) and call state.resolveTaskRunMultipleMode(...) so only the
test names change; leave the setup, assertions, and uses of
resolveTaskRunMultipleMode, newCommandState,
state.projectConfig.Tasks.Run.RunMultipleMode, and failingCLIWriter intact.
In `@internal/cli/daemon_commands.go`:
- Around line 543-565: handleStartedTaskRunMultiple currently returns errors
from watchCLIRunUntilTerminalSuccess directly which breaks exit-code mapping;
change both places that call watchCLIRunUntilTerminalSuccess (the branch after
errors.Is(err, errRunSettledBeforeUIAttach) and the final return) to pass the
returned error through mapDaemonCommandError before returning. Specifically,
update the error handling around attachStartedCLIRunUI and the final watch call
so any error from watchCLIRunUntilTerminalSuccess is wrapped with
mapDaemonCommandError to preserve daemon/API exit-code semantics.
In `@internal/cli/root_command_execution_test.go`:
- Around line 1277-1304: The table-driven tests use t.Run with case names like
"missing", "empty entry", and "duplicate" which violate the project's test
naming rule; update the entries in the test cases slice (the elements consumed
by t.Run in root_command_execution_test.go) to use "Should ..." phrasing (e.g.,
"Should return error for missing slug", "Should return error for empty entry",
"Should return error for duplicate slug") so all subtest names follow the
required t.Run("Should...") pattern while keeping the same test logic and
expectations.
In `@internal/cli/root_test.go`:
- Around line 453-506: Wrap the existing
TestTasksRunMultipleHelpShowsExamplesAndSharedTaskFlags test body in a t.Run
subtest (e.g., t.Run("Should show examples and shared task flags in help
output", func(t *testing.T) { ... })) and move the existing t.Parallel(),
command lookup (findNestedCommand/NewRootCommand), executeRootCommand call, and
all assertions inside that closure; ensure the outer function
TestTasksRunMultipleHelpShowsExamplesAndSharedTaskFlags only contains the t.Run
invocation so the test still runs in parallel and all existing references
(findNestedCommand, NewRootCommand, executeRootCommand, required/forbidden
slices and their checks) remain unchanged except for being enclosed in the
subtest function.
In `@internal/cli/run_observe.go`:
- Around line 557-562: The EventKindTaskRunMultipleQueueCompleted handler
currently prints payload.Total even when the field is zero; update the block
that uses decodeObservedPayload[kinds.TaskRunMultiplePayload] so it checks
payload.Total and only appends "total=%d" when payload.Total > 0 (otherwise omit
the total or print a clearer placeholder like "total=unknown"), ensuring the
fmt.Sprintf call (in the EventKindTaskRunMultipleQueueCompleted branch) builds
the message conditionally based on payload.Total.
In `@internal/core/run/ui/multi_remote_test.go`:
- Around line 369-476: The test function
TestAttachRemoteMultipleFollowsParentAndChildStreams (and the other listed test
functions) must wrap their existing body inside a t.Run("Should ...", func(t
*testing.T) { ... }) block; move everything currently inside the test into that
inner closure (including the original setup/teardown and defer of
setupRemoteMultiRunUISession), keep the outer function signature and do not add
t.Parallel() at top-level so package-level modifications remain correct, and
choose a descriptive "Should ..." string (e.g., "Should follow both parent and
child event streams") to satisfy the test naming guideline.
- Around line 226-248: The test functions
(TestMultiRunCloseTUIDoesNotRequestParentCancel,
TestMultiRunStopRunRequestsParentCancelOnceAndMarksQueuedCanceled,
TestMultiRunQuitKeyDetachAndCompletedQueueCloseImmediately,
TestMultiRunQuitDialogNavigationEscapeAndForceEscalation,
TestMultiRunPayloadFallbacksAndQueueCompletion) must wrap their existing bodies
inside a t.Run("Should ...", func(t *testing.T) { ... }) block per test
guidelines; keep the t.Parallel() call at the top of the outer test, then move
the current body into the inner t.Run with a descriptive "Should ..." label
(e.g., "Should quit without requesting parent cancel when user selects Close
TUI"), preserving all existing setup, assertions, and references such as
multiRunModelForQuitTest and mdl.handleKey; apply the same pattern to the other
listed tests (the other test bodies in the noted ranges) so each uses t.Run for
the actual scenario.
- Around line 21-52: Wrap each test's body in a t.Run("Should ...", func(t
*testing.T) { ... }) subtest while keeping t.Parallel() at the top of the test
function; specifically update
TestMultiRunInitialSnapshotRendersQueuedTabsInOrder,
TestMultiRunChildStartUpdatesOnlyTargetTabState,
TestMultiRunCompletedTabRemainsNavigableAfterActiveAdvances,
TestMultiRunTabNavigationDoesNotCycleChildPaneFocus, and
TestMultiRunTabNavigationUsesHorizontalKeys to place all existing setup,
assertions and calls (e.g. newRemoteMultiRunModel, mdl.handleWindowSize, view
checks, navigation inputs) inside the t.Run anonymous func, using a descriptive
"Should ..." name for each subtest and preserving existing error handling and
expectations.
In `@internal/core/run/ui/multi_remote.go`:
- Around line 359-366: The current ordering in multiRunParentSession.Enqueue
calls observeChild before enqueuing the parent task-multi event, which can cause
child bootstrap events to arrive before the parent tab exists; change the
ordering so the parent event is enqueued first (call s.Session.Enqueue(msg)
before invoking observeChild(childRunID,...)) so tab binding is visible to
handleChildBootstrap, or alternatively make handleChildBootstrap resilient by
buffering unmatched bootstraps keyed by RunID and attempting delivery when a tab
bind occurs (ensure buffering logic uses childRunIDFromTaskMultiEvent for keys
and that observeChild triggers a flush attempt of any buffered bootstrap for
that RunID).
In `@internal/core/run/ui/update_test.go`:
- Around line 363-393: The test TestVerticalKeysNavigateFocusedJobs must be
converted to a table-driven subtest style using t.Run("Should...") for each key
press scenario; refactor the body to build a slice of cases (each with name,
initial model state via newUIModel(3) and m.jobs, the key event to call via
m.handleKey (using keyCode/ keyText), and the expected selectedJob), then
iterate cases and for each call t.Run("Should ...", func(t *testing.T) { ... })
asserting m.selectedJob; keep references to TestVerticalKeysNavigateFocusedJobs,
newUIModel, m.handleKey, keyCode, keyText, selectedJob and
focusedPane/uiPaneJobs so the same logic and assertions are preserved but
executed as table-driven subtests.
In `@internal/core/run/ui/view_test.go`:
- Around line 806-819: Wrap the assertions in
TestJobPaneHelpUsesVerticalNavigation inside a subtest using t.Run with the
"Should..." pattern (e.g., t.Run("Should advertise vertical navigation for job
pane", func(t *testing.T) { ... })); keep the existing setup using
newPopulatedUIModelForTest, setting m.focusedPane = uiPaneJobs, and the checks
that renderHelp contains "↑↓/JK" and does not contain "←→/HL" inside that
subtest so the test name follows the REQUIRED pattern.
In `@internal/core/tasks/slug_list_test.go`:
- Around line 9-57: Combine the four standalone tests into a single table-driven
test named TestParseCommaSeparatedSlugs that iterates over a cases slice (with
each case having a Name starting with "Should ..." and fields input, want,
wantErrSubstring as needed) and call t.Run(case.Name, func(t *testing.T) {
t.Parallel(); got, err := ParseCommaSeparatedSlugs(case.input); assert based on
case.want / case.wantErrSubstring }), preserving the existing assertions and
messages (use slices.Equal and strings.Contains). Ensure each subtest uses t.Run
and t.Parallel, and keep ParseCommaSeparatedSlugs as the function under test
while retaining the original test expectations for order, trimming, empty-entry
error text ("position 2 cannot be empty") and duplicate error text (`duplicate
task slug "alpha"`).
In `@internal/core/workspace/config_test.go`:
- Around line 378-405: TestLoadConfigParsesTaskRunMultipleMode uses table-driven
subtests but does not follow our "Should..." naming and parallelization
conventions; modify the loop to capture the loop variable (tc := tc), change
t.Run(tc.name, ...) to t.Run("Should parse run_multiple_mode="+tc.name, func(t
*testing.T) { t.Parallel(); ... }), and ensure each independent assertion block
inside the subtest (including calls to writeWorkspaceConfig, LoadConfig, and
checks against cfg.Tasks.Run.RunMultipleMode and
cfg.Tasks.Run.EffectiveRunMultipleMode()) runs under that subtest; apply the
same pattern to the sibling table tests around lines 408-481.
In `@internal/daemon/task_multi_test.go`:
- Around line 280-330: These table-driven tests must be converted to parallel
subtests: add t.Parallel() at the top of each test function and wrap the
existing test body inside t.Run("Should ...", func(t *testing.T) { ... }), using
a brief descriptive name (e.g. "Should start completed workflow when
include_completed is true"); apply this change to
TestRunManagerTaskRunMultipleIncludeCompletedStartsCompletedWorkflow,
TestRunManagerTaskRunMultipleChildPollReturnsRunLookupErrors,
TestRunManagerTaskRunMultiplePollLookupErrorCancelsActiveChild,
TestRunManagerTaskRunMultipleSnapshotRejectsNonParentRun, and
TestRunManagerTaskRunMultipleParentRuntimeStartFailureDoesNotStartChildren so
each creates an isolated parallel subtest and the original body is moved into
the subtest closure.
- Around line 21-96: The test functions
TestRunManagerTaskRunMultipleRunsChildrenSequentially,
TestRunManagerTaskRunMultipleStopsOnFirstChildFailure, and
TestRunManagerTaskRunMultipleCancellationCancelsActiveAndQueuedChildren must be
updated to call t.Parallel() at the top and to wrap their existing test bodies
inside t.Run("Should ...", func(t *testing.T) { ... }), e.g., add t.Parallel()
immediately inside each test function and move the current body into a t.Run
block with a descriptive "Should ..." name (preserve existing locals like
started, releaseAlpha, env and the calls to startTaskMultiRun, waitForString,
assertions, etc.); ensure the inner anonymous func uses the testing.T parameter
so subtests run correctly in parallel.
- Around line 198-278: Add t.Parallel() at the start of
TestRunManagerTaskRunMultiplePreflightRejectsInvalidInputBeforeParentRun and
immediately inside each subtest closure passed to t.Run (the three anonymous
funcs) so the top-level test and all subtests run in parallel; keep the existing
"Should..." subtest names as shown and ensure you call t.Parallel() as the first
statement in each subtest function (refer to
TestRunManagerTaskRunMultiplePreflightRejectsInvalidInputBeforeParentRun and the
three inner t.Run closures).
In `@internal/daemon/task_multi.go`:
- Around line 246-249: resolveTaskMultiMode currently treats the parallel input
as enqueued because the switch always returns
workspacecfg.TaskRunMultipleModeEnqueued; update the function
(resolveTaskMultiMode) to return workspacecfg.TaskRunMultipleModeParallel when
the trimmed raw equals workspacecfg.TaskRunMultipleModeParallel, keep returning
workspacecfg.TaskRunMultipleModeEnqueued for empty or enqueued inputs, and
return an error for unknown values so callers can observe and handle invalid
modes.
---
Nitpick comments:
In `@internal/api/client/client_contract_test.go`:
- Around line 352-377: Test duplicates nil-context checks for
StartTaskRunMultiple and GetTaskRunMultipleSnapshot; refactor
TestClientTaskRunMultipleMethodsRejectNilContext into a table-driven test that
iterates over cases (e.g., names "StartTaskRunMultiple" and
"GetTaskRunMultipleSnapshot") and uses t.Run subtests to call each method with a
shared nilCtx and assert errors.Is(err, ErrDaemonContextRequired); keep
t.Parallel where appropriate, reuse the client setup and expected request args
(apicore.TaskRunMultipleRequest and the snapshot ID) and reference the concrete
methods StartTaskRunMultiple and GetTaskRunMultipleSnapshot so reviewers can
locate the code to change.
- Around line 98-146: Wrap the existing test body inside a t.Run subtest named
like "Should start multiple task run with ordered slugs" while keeping the
top-level TestClientStartTaskRunMultiplePostsOrderedSlugs and its t.Parallel()
call; move all existing logic (client setup, HTTP roundTripFunc checks, the
StartTaskRunMultiple call and assertions on run) into the t.Run callback, and
call t.Parallel() inside the subtest if you want the subtest to run in parallel
as well so the test follows the t.Run("Should...") pattern and preserves
parallelism and existing checks for StartTaskRunMultiple, the HTTP mock, and
assertions.
- Around line 148-193: The test function
TestClientGetTaskRunMultipleSnapshotUsesDedicatedRouteAndDecodes currently calls
t.Parallel() but does not wrap its assertions in a t.Run subtest; update the
test to call t.Run("Should return decoded multiple snapshot" or similar) and
move the existing body (including t.Parallel(), mock httpClient setup, call to
client.GetTaskRunMultipleSnapshot and assertions) inside that t.Run closure so
the test follows the required t.Run("Should...") pattern while preserving
parallelism and behavior.
In `@internal/cli/commands_test.go`:
- Around line 113-134: Wrap the assertions in
TestNewTasksCommandRegistersRunMultiple in a subtest using t.Run("Should
register run-multiple with correct flags", func(t *testing.T) { ... }) so the
test follows the repository contract; move the current body that creates cmd :=
newTasksCommand(nil, defaultCommandStateDefaults()), finds runMultiple, and
checks its Name(), absence of --name flag, presence of --attach flag and its
default into the t.Run closure and keep t.Parallel() at the top-level test.
In `@internal/daemon/run_manager_test.go`:
- Around line 104-132: The test function
TestRunManagerIncludeCompletedStartsCompletedTaskWorkflow must wrap its case
body in a subtest using t.Run("Should ...") to follow the project's test
contract; modify the function so the current test logic (the env setup,
env.writeWorkflowFile, env.startTaskRun, waitForBool using seenIncludeCompleted,
and waitForRun assertions) is moved into a t.Run call (e.g. t.Run("Should
include completed tasks when include_completed=true", func(t *testing.T) { ...
})) while preserving the existing symbols like seenIncludeCompleted,
newRunManagerTestEnv, daemonTaskBody, env.startTaskRun, waitForBool and
waitForRun and keeping all assertions intact.
In `@internal/daemon/task_multi.go`:
- Around line 364-365: The call to m.Cancel currently uses
detachContext(context.Background()) which drops caller context metadata and
lifecycle intent; replace context.Background() with the current request/runtime
context (e.g., the local ctx variable) and call detachContext(ctx) so
cancellation detaches but preserves caller metadata. Update the same pattern
used elsewhere in this file (the other m.Cancel/detachContext invocations) to
use detachContext(ctx) and ensure the first argument to m.Cancel is a
context.Context passed from the surrounding function rather than
context.Background().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2689132b-6ced-4c14-a89f-f25cb2991ac9
⛔ Files ignored due to path filters (24)
.compozy/tasks/multi-task-run/_idea.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/_prd.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/_tasks.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/_techspec.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/MEMORY.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_01.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_02.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_03.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_04.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_05.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_06.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_01.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_02.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_03.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_04.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_05.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_06.mdis excluded by!**/*.mdREADME.mdis excluded by!**/*.mdopenapi/compozy-daemon.jsonis excluded by!**/*.jsonweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (50)
internal/api/client/client.gointernal/api/client/client_contract_test.gointernal/api/client/client_transport_test.gointernal/api/client/runs.gointernal/api/contract/contract_test.gointernal/api/contract/routes.gointernal/api/contract/types.gointernal/api/core/handlers.gointernal/api/core/handlers_contract_test.gointernal/api/core/handlers_service_errors_test.gointernal/api/core/handlers_smoke_test.gointernal/api/core/interfaces.gointernal/api/core/routes.gointernal/api/httpapi/openapi_contract_test.gointernal/api/httpapi/transport_integration_test.gointernal/cli/commands_test.gointernal/cli/daemon_commands.gointernal/cli/daemon_commands_test.gointernal/cli/daemon_exec_test_helpers_test.gointernal/cli/extensions_bootstrap.gointernal/cli/root.gointernal/cli/root_command_execution_test.gointernal/cli/root_test.gointernal/cli/run.gointernal/cli/run_observe.gointernal/cli/skills_preflight.gointernal/cli/state.gointernal/cli/validate_tasks_test.gointernal/cli/workspace_config.gointernal/core/run/ui/multi_remote.gointernal/core/run/ui/multi_remote_test.gointernal/core/run/ui/update.gointernal/core/run/ui/update_test.gointernal/core/run/ui/view.gointernal/core/run/ui/view_test.gointernal/core/tasks/slug_list.gointernal/core/tasks/slug_list_test.gointernal/core/workspace/config_merge.gointernal/core/workspace/config_test.gointernal/core/workspace/config_types.gointernal/core/workspace/config_validate.gointernal/daemon/run_manager.gointernal/daemon/run_manager_test.gointernal/daemon/service.gointernal/daemon/task_multi.gointernal/daemon/task_multi_test.gointernal/daemon/task_transport_service.gopkg/compozy/events/event.gopkg/compozy/events/kinds/payload_compat_test.gopkg/compozy/events/kinds/task.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/workspace/config_test.go`:
- Around line 14-15: The test helper stubWorkspaceUserHomeDir mutates the global
osUserHomeDir without using the same mutex used by loadConfigWithIsolatedHome,
causing races; update stubWorkspaceUserHomeDir to lock/unlock
workspaceUserHomeDirMu around any reads or writes to osUserHomeDir (and ensure
any other test helpers that touch osUserHomeDir — e.g., the helpers referenced
around lines 1554-1570 — also use workspaceUserHomeDirMu) so all mutations of
osUserHomeDir are serialized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bc3d106f-0d45-43d7-a346-f6f4aba7313b
📒 Files selected for processing (1)
internal/core/workspace/config_test.go
|
there are some conflicts to solve here @MatheusBBarni |
|
@MatheusBBarni there are any reason why you created a |
I'm not very good with cli commands definitions, but |
|
@MatheusBBarni yeah, it's always better to keep as simple as possible and by introducing a new command instead of use an argument for this type of stuff can generate extra complexity, besides the fact, I guess your final code can be better if you reuse stuff from the run command instead of need to recreate a new one this feature you added is a very nice feature, but needs to be carefully introduced |
|
@pedronauck done, how can we trigger code habbit to run again? |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
internal/cli/daemon_commands_test.go (1)
1101-1245:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRename lifecycle-rendering subtests to follow the required
Should...convention.The new subtests currently run with names like
"queue started"and"item failed"(Line 1184 and Line 1234), which breaks the enforced test naming pattern.As per coding guidelines
**/*_test.go: "MUST use t.Run("Should...") pattern for ALL test cases".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/daemon_commands_test.go` around lines 1101 - 1245, The subtest names in TestRenderObservedTaskMultiLifecycle and TestRenderObservedTaskMultiLifecycleFallbacks (calls to t.Run with names like "queue started", "item failed", etc.) violate the required t.Run("Should...") naming convention; update each t.Run invocation in these two functions so the subtest name strings follow the pattern "Should <describe behavior>" (e.g., "Should render queue started", "Should render item failed") while keeping the same test bodies and referencing the same event kinds (eventspkg.EventKindTaskRunMultipleStarted, EventKindTaskRunMultipleChildFailed, EventKindTaskRunMultipleQueueCompleted, EventKindTaskRunMultipleQueueCanceled, etc.) so only the displayed subtest names change.
🧹 Nitpick comments (2)
internal/api/client/runs.go (1)
204-208: ⚡ Quick winWrap request failures with run context in this client method.
Returning raw
errhere loses operation/run context and makes debugging harder.As per coding guidelines `**/*.go`: Prefer explicit error returns with wrapped context using `fmt.Errorf("context: %w", err)`.Proposed change
path := "/api/task-runs/multiple/" + url.PathEscape(trimmedRunID) + "/snapshot" if _, err := c.doJSON(ctx, http.MethodGet, path, nil, &payload); err != nil { - return apicore.TaskRunMultipleSnapshot{}, err + return apicore.TaskRunMultipleSnapshot{}, fmt.Errorf( + "load task multi-run snapshot %q: %w", + trimmedRunID, + err, + ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/client/runs.go` around lines 204 - 208, The client method currently returns the raw error from c.doJSON which loses run context; update the error return to wrap the underlying error with run and operation information using fmt.Errorf, e.g. on the c.doJSON call that populates payload (and referencing trimmedRunID and path) return fmt.Errorf("failed to fetch multiple snapshot for run %s: %w", trimmedRunID, err) so callers receive contextualized errors; ensure fmt is imported if not already.internal/core/run/ui/multi_remote_test.go (1)
773-807: ⚡ Quick winAssert the status mappings instead of smoke-calling them.
The last two loops ignore the return values, so this test still passes if either mapping regresses. A small table-driven
t.Run("Should ...")set with expected outputs would make these helpers fail on real behavior changes.As per coding guidelines,
**/*_test.go: "MUST test meaningful business logic, not trivial operations" and "Use table-driven tests with subtests (t.Run) as the default pattern".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/core/run/ui/multi_remote_test.go` around lines 773 - 807, Replace the two smoke-test loops with proper table-driven subtests that assert expected mappings instead of ignoring return values: for taskMultiStatusFromRunStatus and taskMultiStatusFromChildRunEvent create slices of structs {input, want} and iterate with t.Run for each case, call the function (taskMultiStatusFromRunStatus or taskMultiStatusFromChildRunEvent) and compare the returned status to want using t.Fatalf or t.Errorf when they differ; ensure you include cases for remoteRunStatusRunning, remoteRunStatusRetrying, remoteRunStatusCompleted, remoteRunStatusFailed, remoteRunStatusCrashed, remoteRunStatusCanceled, "pending" and for eventspkg.EventKindRunCompleted, EventKindRunFailed, EventKindRunCancelled, EventKindJobQueued with their expected task multi-statuses so regressions fail the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/api/client/client_transport_test.go`:
- Around line 843-854: The two nil-client assertions (calls to
nilClient.StartTaskRunMultiple and nilClient.GetTaskRunMultipleSnapshot) must be
moved into explicit subtests using the t.Run("Should...") pattern; wrap each
assertion block in its own t.Run (e.g., t.Run("Should return
ErrDaemonClientRequired for StartTaskRunMultiple", func(t *testing.T) { ... })
and similarly for GetTaskRunMultipleSnapshot) so the tests follow the
repository's required test structure and isolate failures.
In `@internal/api/core/handlers_smoke_test.go`:
- Around line 343-349: The test case "Should return snapshot for multiple run"
currently only asserts the presence of the items array by checking `"items":[`;
update the assertion in internal/api/core/handlers_smoke_test.go for the
"/api/task-runs/multiple/multi-run-1/snapshot" request to validate actual item
data instead of just the array marker — either assert a specific expected JSON
snippet (e.g. expected item ids or keys) or parse the response body as JSON and
assert the items array length and required fields (like item id, run id, status)
for entries that should be present in the multi-run snapshot.
In `@internal/cli/root_command_execution_test.go`:
- Around line 1173-1590: Several top-level tests (e.g.,
TestTasksRunMultipleCommandDetachSendsOrderedSlugs,
TestTasksRunMultipleCommandRejectsInvalidSlugListsBeforeDaemon,
TestTasksRunMultipleCommandParallelConfigFallsBackToEnqueued,
TestTasksRunCommandStillCallsSingleRunClient,
TestTasksRunMultipleCommandInProcessStreamReconstructsParentQueueState,
TestTasksRunMultipleCommandInProcessParallelConfigStreamsEnqueuedQueue,
TestTasksRunMultipleCommandStreamReturnsNonZeroOnParentFailure) are written as
plain top-level test bodies; wrap each function body in a t.Run("Should ...",
func(t *testing.T) { t.Parallel() /* where the test is independent */ ... })
block to follow the repository pattern, preserving the existing assertions and
setup inside the closure and adding t.Parallel() for tests that can run
concurrently; ensure the subtest name starts with "Should" and keep any
installTestCLI... or installInProcessCLIDaemonBootstrap... setup inside the
subtest closure so behavior is unchanged.
In `@internal/core/run/ui/multi_remote.go`:
- Around line 376-391: When bootstrap is true, don't short-circuit to return
when snapshot hydration is unavailable; instead only attempt snapshot loading if
opts.LoadChildSnapshot != nil and if it fails fall through to the streaming
path. Change the bootstrap block to: if bootstrap && opts.LoadChildSnapshot !=
nil { snapshot, err := opts.LoadChildSnapshot(...); if err == nil {
session.Enqueue(multiRunChildBootstrapMsg{RunID: runID, Snapshot: snapshot});
cursor = streamCursorOrZero(snapshot.NextCursor); if
isTerminalRunStatus(snapshot.Run.Status) { return } } } and then continue to
check opts.OpenChildStream; ensure you remove the early returns that fire when
LoadChildSnapshot is nil or when LoadChildSnapshot returns an error so
OpenChildStream can still be used.
In `@internal/core/workspace/config_types.go`:
- Around line 59-64: The EffectiveRunMultipleMode method on TaskRunConfig must
treat a whitespace-only RunMultipleMode as missing; update
TaskRunConfig.EffectiveRunMultipleMode to trim the pointer value (using
strings.TrimSpace) and if the trimmed result is empty return
TaskRunMultipleModeEnqueued, otherwise return the trimmed string, ensuring you
still handle the case where RunMultipleMode is nil.
In `@internal/daemon/task_multi.go`:
- Around line 246-251: The code accepts "parallel" in resolveTaskMultiMode but
the coordinator still runs children serially by waiting in
waitForTaskMultiChild; either implement true parallel execution or disallow
"parallel" now. Fix by updating resolveTaskMultiMode to reject
workspacecfg.TaskRunMultipleModeParallel with a clear error (so callers cannot
request parallel semantics) OR implement a parallel branch in the coordinator
that launches child tasks without blocking on waitForTaskMultiChild (e.g., spawn
goroutines/tasks and collect results concurrently), ensuring functions like
resolveTaskMultiMode and the coordinator code that currently calls
waitForTaskMultiChild are updated consistently so reported mode matches actual
execution.
- Around line 358-360: When startTaskMultiChild fails, before returning update
the RunMultipleSnapshot to mark the current item (item at index) as terminal and
emit the corresponding terminal item/queue events, then cancel all remaining
queued items (items index+1..total-1) and emit cancellation events for them; do
this in the error path that follows m.startTaskMultiChild(active, prepared,
item, index, total) so the snapshot/state and event stream reflect the failure
instead of leaving items erroneously queued. Ensure you reuse the same
event-emission and state-update helpers used elsewhere in task_multi.go for
marking an item terminal and cancelling queued items so behavior stays
consistent.
In `@pkg/compozy/events/kinds/payload_compat_test.go`:
- Around line 145-175: Wrap the existing
TestTaskRunMultiplePayloadJSONCompatibility body in a t.Run subtest (with a name
starting "Should...") so the compatibility check is executed as a named subtest;
move the t.Parallel call into the subtest. Concretely, inside
TestTaskRunMultiplePayloadJSONCompatibility keep creating the
TaskRunMultiplePayload and then call t.Run("Should produce compatible JSON",
func(t *testing.T){ t.Parallel(); got := mustMarshalMap(t, payload); ...
assertions ... }), preserving the use of mustMarshalMap, TaskRunMultiplePayload,
and the reflect.DeepEqual comparison.
---
Duplicate comments:
In `@internal/cli/daemon_commands_test.go`:
- Around line 1101-1245: The subtest names in
TestRenderObservedTaskMultiLifecycle and
TestRenderObservedTaskMultiLifecycleFallbacks (calls to t.Run with names like
"queue started", "item failed", etc.) violate the required t.Run("Should...")
naming convention; update each t.Run invocation in these two functions so the
subtest name strings follow the pattern "Should <describe behavior>" (e.g.,
"Should render queue started", "Should render item failed") while keeping the
same test bodies and referencing the same event kinds
(eventspkg.EventKindTaskRunMultipleStarted, EventKindTaskRunMultipleChildFailed,
EventKindTaskRunMultipleQueueCompleted, EventKindTaskRunMultipleQueueCanceled,
etc.) so only the displayed subtest names change.
---
Nitpick comments:
In `@internal/api/client/runs.go`:
- Around line 204-208: The client method currently returns the raw error from
c.doJSON which loses run context; update the error return to wrap the underlying
error with run and operation information using fmt.Errorf, e.g. on the c.doJSON
call that populates payload (and referencing trimmedRunID and path) return
fmt.Errorf("failed to fetch multiple snapshot for run %s: %w", trimmedRunID,
err) so callers receive contextualized errors; ensure fmt is imported if not
already.
In `@internal/core/run/ui/multi_remote_test.go`:
- Around line 773-807: Replace the two smoke-test loops with proper table-driven
subtests that assert expected mappings instead of ignoring return values: for
taskMultiStatusFromRunStatus and taskMultiStatusFromChildRunEvent create slices
of structs {input, want} and iterate with t.Run for each case, call the function
(taskMultiStatusFromRunStatus or taskMultiStatusFromChildRunEvent) and compare
the returned status to want using t.Fatalf or t.Errorf when they differ; ensure
you include cases for remoteRunStatusRunning, remoteRunStatusRetrying,
remoteRunStatusCompleted, remoteRunStatusFailed, remoteRunStatusCrashed,
remoteRunStatusCanceled, "pending" and for eventspkg.EventKindRunCompleted,
EventKindRunFailed, EventKindRunCancelled, EventKindJobQueued with their
expected task multi-statuses so regressions fail the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fed621c0-f95b-48b3-96d2-11b1d4836ddd
⛔ Files ignored due to path filters (26)
.compozy/tasks/multi-task-run/_idea.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/_prd.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/_tasks.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/_techspec.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/MEMORY.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_01.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_02.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_03.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_04.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_05.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_06.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_01.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_02.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_03.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_04.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_05.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_06.mdis excluded by!**/*.mdREADME.mdis excluded by!**/*.mdopenapi/compozy-daemon.jsonis excluded by!**/*.jsonskills/compozy/SKILL.mdis excluded by!**/*.mdskills/compozy/references/cli-reference.mdis excluded by!**/*.mdweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (49)
internal/api/client/client.gointernal/api/client/client_contract_test.gointernal/api/client/client_transport_test.gointernal/api/client/runs.gointernal/api/contract/contract_test.gointernal/api/contract/routes.gointernal/api/contract/types.gointernal/api/core/handlers.gointernal/api/core/handlers_contract_test.gointernal/api/core/handlers_service_errors_test.gointernal/api/core/handlers_smoke_test.gointernal/api/core/interfaces.gointernal/api/core/routes.gointernal/api/httpapi/openapi_contract_test.gointernal/api/httpapi/transport_integration_test.gointernal/cli/commands_test.gointernal/cli/daemon_commands.gointernal/cli/daemon_commands_test.gointernal/cli/daemon_exec_test_helpers_test.gointernal/cli/extensions_bootstrap.gointernal/cli/root_command_execution_test.gointernal/cli/root_test.gointernal/cli/run.gointernal/cli/run_observe.gointernal/cli/skills_preflight.gointernal/cli/state.gointernal/cli/testdata/tasks_run_help.goldeninternal/cli/validate_tasks_test.gointernal/core/run/ui/multi_remote.gointernal/core/run/ui/multi_remote_test.gointernal/core/run/ui/update.gointernal/core/run/ui/update_test.gointernal/core/run/ui/view.gointernal/core/run/ui/view_test.gointernal/core/tasks/slug_list.gointernal/core/tasks/slug_list_test.gointernal/core/workspace/config_merge.gointernal/core/workspace/config_test.gointernal/core/workspace/config_types.gointernal/core/workspace/config_validate.gointernal/daemon/run_manager.gointernal/daemon/run_manager_test.gointernal/daemon/service.gointernal/daemon/task_multi.gointernal/daemon/task_multi_test.gointernal/daemon/task_transport_service.gopkg/compozy/events/event.gopkg/compozy/events/kinds/payload_compat_test.gopkg/compozy/events/kinds/task.go
Summary
Verification
env -u GOROOT make verifySummary by CodeRabbit
Release Notes
--multipleflag totasks runcommand for executing multiple workflows sequentially in a single daemon-owned queue using comma-separated task slugs