Repository navigation
feat: worktree-backed parallel multi-run for tasks run --multiple - #200
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds ChangesParallel multi-run with git worktree isolation
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant CLICmd as tasks run --multiple --parallel
participant Daemon as Daemon API
participant Scheduler as runTaskMultiParallelQueue
participant Allocator as taskMultiWorktreeAllocator
participant Git as git CLI
User->>CLICmd: tasks run --multiple alpha,beta --parallel --parallel-limit 2
CLICmd->>CLICmd: resolveTaskRunMultipleMode → parallel
CLICmd->>CLICmd: resolveTaskRunMultipleParallelLimit → 2
CLICmd->>Daemon: POST /api/task-runs/multiple {mode=parallel, parallel_limit=2}
Daemon->>Scheduler: runTaskMultiParallelQueue (semaphore=2)
Scheduler->>Allocator: ResolveBase(workspaceRoot)
Allocator->>Git: git rev-parse HEAD / --abbrev-ref HEAD
Git-->>Allocator: branch=main, commit=abc123
loop per child (alpha, beta concurrently)
Scheduler->>Allocator: Allocate(slug, index, parentRunID)
Allocator->>Git: git worktree add --detach path abc123
Git-->>Allocator: worktree created
Scheduler->>Daemon: emit worktree_metadata event
Scheduler->>Daemon: StartRun(child, worktreeWorkspace)
Daemon-->>Scheduler: child run result
end
Scheduler->>Daemon: aggregateTaskMultiParallelResult
Daemon-->>CLICmd: snapshot with per-child WorktreePath
CLICmd->>User: handoff: alpha | completed | run=… | worktree=…
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0810b7f940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // stay short enough for local daemon/socket path constraints. | ||
| taskMultiWorktreeHashLen = 12 | ||
| // taskMultiWorktreeParentShortLen bounds the parent-run path segment. | ||
| taskMultiWorktreeParentShortLen = 12 |
There was a problem hiding this comment.
The 12-character cap collapses every default multi-run parent ID from BuildRunID (for task-multi) to the same sanitized prefix, e.g. tasks-task-multi-... becomes tasks-task-m. Since the path is <workspace-hash>/<parent-short>/<NN-slug> and allocated worktrees are preserved while ensureTaskMultiWorktreeTargetFree rejects existing targets, a second parallel run in the same workspace with the same child index/slug will reuse the previous path and fail before launching that child. Use entropy from the full parent run ID (for example a hash or suffix) rather than only the shared leading prefix.
Useful? React with 👍 / 👎.
|
| Filename | Overview |
|---|---|
| internal/daemon/task_multi.go | Mode-aware scheduler refactor: adds fail-late parallel path with semaphore-bounded fanout, emitMu for journal serialization, and shared queue lifecycle helpers used by both enqueued and parallel branches. |
| internal/daemon/task_multi_worktree.go | New worktree allocator: deterministic path planning with SHA-256-keyed workspace and parent-run segments, detached-HEAD rejection, and git-command abstraction for testability. V1 intentionally never deletes worktrees. |
| internal/store/sqlite.go | Adds _txlock=immediate to all SQLite DSNs; global change that prevents the read-to-write upgrade deadlock exposed by concurrent parallel-mode child writers. |
| internal/cli/daemon_commands.go | Adds --parallel/--parallel-limit flags, resolves mode and limit with precedence rules, rejects --parallel-limit outside parallel mode, and calls streamTaskRunMultipleToTerminal with context-aware handoff skipping on Ctrl+C. |
| internal/cli/run_observe.go | Adds writeTaskRunMultipleHandoff and related formatters; stream output tags child events with worktree=<path> and renders per-child handoff block with - placeholders for backward compat. |
| internal/core/run/ui/multi_remote.go | TUI gains a worktree status line below the tab strip (height bumped to 3), additive applyTaskMultiWorktreeMetadata mirroring the snapshot builder, and em-dash fallbacks for missing fields. |
| internal/daemon/run_manager.go | Adds WorktreesRoot config field, wires newTaskMultiWorktreeAllocator into the RunManager, and adds emitMu sync.Mutex to activeRun for parallel event serialization. |
| internal/core/workspace/config_types.go | Adds RunMultipleParallelLimit *int TOML field, DefaultRunMultipleParallelLimit = 2, and EffectiveRunMultipleParallelLimit() accessor with safe nil handling. |
| internal/daemon/task_multi_test.go | Comprehensive parallel-mode daemon tests: bounded concurrency (channel handshake, not sleeps), fail-late, cancellation, resolved-limit event, and single-ItemQueued-per-child invariant. |
| internal/daemon/task_multi_worktree_test.go | New 598-line test suite covering path determinism, workspace isolation, truncated-prefix collision resistance, slug sanitization, and the full allocator round-trip using real git. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CLI
participant Daemon
participant Coordinator
participant Semaphore
participant Worker1
participant Worker2
participant Git
CLI->>Daemon: "StartTaskRunMultiple(mode=parallel, limit=2)"
Daemon->>Coordinator: runTaskMultiCoordinator
Coordinator->>Coordinator: emitQueueStarted (seeds Slugs)
Coordinator->>Coordinator: resolveTaskMultiParallelBase
Coordinator->>Git: rev-parse --abbrev-ref HEAD
Git-->>Coordinator: main (branch + commit)
par Child alpha
Coordinator->>Semaphore: acquire slot
Semaphore-->>Coordinator: ok
Coordinator->>Worker1: go runTaskMultiParallelChild(alpha)
Worker1->>Git: worktree add --detach .../00-alpha HEAD
Worker1->>Coordinator: emitItemQueued(alpha, worktreePath)
Worker1->>Daemon: "startRun(alpha, workspace=worktree)"
Worker1->>Worker1: waitForTaskMultiChild
and Child beta
Coordinator->>Semaphore: acquire slot
Semaphore-->>Coordinator: ok
Coordinator->>Worker2: go runTaskMultiParallelChild(beta)
Worker2->>Git: worktree add --detach .../01-beta HEAD
Worker2->>Coordinator: emitItemQueued(beta, worktreePath)
Worker2->>Daemon: "startRun(beta, workspace=worktree)"
Worker2->>Worker2: waitForTaskMultiChild
end
Worker1-->>Semaphore: release slot
Worker1-->>Coordinator: "childErrs[0]=nil"
Worker2-->>Semaphore: release slot
Worker2-->>Coordinator: "childErrs[1]=nil"
Coordinator->>Coordinator: wg.Wait() + finalizeTaskMultiParallel
Coordinator->>Coordinator: "aggregateResult -> nil"
Coordinator->>Coordinator: emitQueueCompleted
Daemon-->>CLI: terminal event + handoff block
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CLI
participant Daemon
participant Coordinator
participant Semaphore
participant Worker1
participant Worker2
participant Git
CLI->>Daemon: "StartTaskRunMultiple(mode=parallel, limit=2)"
Daemon->>Coordinator: runTaskMultiCoordinator
Coordinator->>Coordinator: emitQueueStarted (seeds Slugs)
Coordinator->>Coordinator: resolveTaskMultiParallelBase
Coordinator->>Git: rev-parse --abbrev-ref HEAD
Git-->>Coordinator: main (branch + commit)
par Child alpha
Coordinator->>Semaphore: acquire slot
Semaphore-->>Coordinator: ok
Coordinator->>Worker1: go runTaskMultiParallelChild(alpha)
Worker1->>Git: worktree add --detach .../00-alpha HEAD
Worker1->>Coordinator: emitItemQueued(alpha, worktreePath)
Worker1->>Daemon: "startRun(alpha, workspace=worktree)"
Worker1->>Worker1: waitForTaskMultiChild
and Child beta
Coordinator->>Semaphore: acquire slot
Semaphore-->>Coordinator: ok
Coordinator->>Worker2: go runTaskMultiParallelChild(beta)
Worker2->>Git: worktree add --detach .../01-beta HEAD
Worker2->>Coordinator: emitItemQueued(beta, worktreePath)
Worker2->>Daemon: "startRun(beta, workspace=worktree)"
Worker2->>Worker2: waitForTaskMultiChild
end
Worker1-->>Semaphore: release slot
Worker1-->>Coordinator: "childErrs[0]=nil"
Worker2-->>Semaphore: release slot
Worker2-->>Coordinator: "childErrs[1]=nil"
Coordinator->>Coordinator: wg.Wait() + finalizeTaskMultiParallel
Coordinator->>Coordinator: "aggregateResult -> nil"
Coordinator->>Coordinator: emitQueueCompleted
Daemon-->>CLI: terminal event + handoff block
Reviews (5): Last reviewed commit: "test: wrap multi-run UI tests in t.Run("..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
internal/api/httpapi/openapi_contract_test.go (1)
186-193: ⚡ Quick winUse
t.Run("Should...")per schema field assertion.These loops currently collapse multiple cases into one failure path. Wrapping each field check in a subtest will align with the test guidelines and improve failure granularity.
Suggested refactor
- for _, field := range []string{ + for _, field := range []string{ "slugs", "mode", "parallel_limit", "presentation_mode", "runtime_overrides", "workspace", } { - if _, ok := taskRunMultipleProperties[field]; !ok { - t.Fatalf("TaskRunMultipleRequest must expose %s", field) - } + field := field + t.Run("Should expose TaskRunMultipleRequest field "+field, func(t *testing.T) { + t.Parallel() + if _, ok := taskRunMultipleProperties[field]; !ok { + t.Fatalf("TaskRunMultipleRequest must expose %s", field) + } + }) } ... - for _, field := range []string{ + for _, field := range []string{ "slug", "status", "run_id", "error_text", "worktree_path", "base_branch", "base_commit", "worktree_status", } { - if _, ok := multiRunItemProperties[field]; !ok { - t.Fatalf("TaskRunMultipleItem must expose %s", field) - } + field := field + t.Run("Should expose TaskRunMultipleItem field "+field, func(t *testing.T) { + t.Parallel() + if _, ok := multiRunItemProperties[field]; !ok { + t.Fatalf("TaskRunMultipleItem must expose %s", field) + } + }) }As per coding guidelines,
**/*_test.gorequiresMUST use t.Run("Should...") pattern for ALL test casesandUse table-driven tests with subtests (t.Run) as the default pattern.Also applies to: 207-219
🤖 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/httpapi/openapi_contract_test.go` around lines 186 - 193, The field validation loop that iterates through the array of fields (slugs, mode, parallel_limit, presentation_mode, runtime_overrides, workspace) currently checks all fields within a single test path, causing the entire test to fail on the first field error. Refactor this by wrapping each field validation check inside a t.Run() subtest with a descriptive name like t.Run(fmt.Sprintf("Should validate field: %s", field), func(t *testing.T) { ... }) to align with the test guidelines and provide granular failure reporting. Apply this same pattern to all similar field validation loops in the test function where multiple fields are checked in sequence without individual subtests.Source: Coding guidelines
🤖 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 516-518: The test case naming in the table-driven test at the
t.Run call on line 516 does not follow the required "Should..." convention.
Update all test case names in the test table (referenced as tc.name in the t.Run
call) to follow the "Should..." naming pattern as mandated by the coding
guidelines, ensuring each test case name clearly describes what behavior it
should verify using this standard convention.
In `@internal/api/core/handlers_smoke_test.go`:
- Around line 731-775: The test function
TestStartTaskRunMultipleForwardsParallelLimitAndMode needs to follow the
repository's test convention by wrapping its logic in a t.Run() call. Move all
the test setup, HTTP request handling, and assertions from the main function
body into a t.Run("Should forward parallel limit and mode correctly") subtest,
keeping all existing test logic intact while conforming to the required naming
pattern.
In `@internal/cli/tasks_run_parallel_e2e_test.go`:
- Around line 99-127: The test function
TestTasksRunMultipleParallelEndToEndReportsWorktreePaths does not follow the
required t.Run subtest pattern. Wrap the entire test body in a t.Run call with a
descriptive name starting with "Should" that explains what the test verifies
(for example, reporting worktree paths in parallel runs). Apply this same t.Run
pattern to all other test functions in this file that currently have direct test
bodies without subtest wrappers.
In `@internal/core/workspace/config_test.go`:
- Around line 531-533: The table-driven test subtests in the test function are
using simple names like "one", "two", "five" in the tc.name field instead of
following the required "Should..." naming pattern per coding guidelines. Locate
the test case table definition (the slice of struct containing the test cases)
and update each test case's name field to start with "Should" followed by a
descriptive phrase that explains what the test validates, then the
t.Run(tc.name, func(t *testing.T) call will automatically use these properly
formatted names.
In `@internal/daemon/task_multi_test.go`:
- Around line 1542-1544: The error check in the GetRun call for "child-beta" on
line 1542 only verifies that an error occurred (err != nil) but does not assert
the specific error type, which can hide unrelated failures. Replace the weak
error check with an explicit assertion that the returned error is
globaldb.ErrRunNotFound to ensure the correct sentinel error is raised when the
run is not found, following the test guidelines that require specific error
assertions.
In `@internal/daemon/task_multi_worktree_test.go`:
- Around line 208-210: The test cases for sanitizeTaskMultiWorktreeSegment are
using tc.name values that do not follow the required "Should..." naming
convention. Update all test case name entries in the table-driven test to start
with "Should" followed by a descriptive action, so that when t.Run(tc.name, ...)
is called at line 208, each subtest will have a name matching the "Should..."
pattern as required by the test policy.
In `@internal/daemon/task_multi_worktree.go`:
- Around line 177-186: The sanitizeTaskMultiWorktreeSegment function call on
line 177 truncates the ParentRunID to 12 characters, which can cause path
collisions when different parent runs share the same leading characters,
resulting in worktree target already exists errors later in the code. After
sanitizing the ParentRunID using sanitizeTaskMultiWorktreeSegment to keep it
short, append a short hash suffix computed from the full spec.ParentRunID value
to the parent variable to maintain collision resistance while keeping paths
concise. This hash suffix should be deterministic and derived from the complete
parent run ID before truncation, then incorporated into the path construction in
the filepath.Join call.
In `@pkg/compozy/events/kinds/docs_test.go`:
- Around line 16-31: The test function
TestTaskRunMultiplePayloadFieldsDocumented does not follow the repository's
mandatory test convention. Wrap the entire test body after t.Parallel() in a
t.Run subtest with a descriptive name following the "Should..." pattern (for
example, "ShouldDocumentAllTaskRunMultiplePayloadFields"). Move the content
reading, payloadType reflection, field iteration, and validation checks inside
the t.Run callback function to conform to the required test structure.
---
Nitpick comments:
In `@internal/api/httpapi/openapi_contract_test.go`:
- Around line 186-193: The field validation loop that iterates through the array
of fields (slugs, mode, parallel_limit, presentation_mode, runtime_overrides,
workspace) currently checks all fields within a single test path, causing the
entire test to fail on the first field error. Refactor this by wrapping each
field validation check inside a t.Run() subtest with a descriptive name like
t.Run(fmt.Sprintf("Should validate field: %s", field), func(t *testing.T) { ...
}) to align with the test guidelines and provide granular failure reporting.
Apply this same pattern to all similar field validation loops in the test
function where multiple fields are checked in sequence without individual
subtests.
🪄 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: 691a8dfa-423e-473d-a9a2-e9b73236da7d
⛔ Files ignored due to path filters (33)
.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-005.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-006.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-007.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/adrs/adr-008.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/memory/task_07.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_08.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_09.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/memory/task_10.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!**/*.md.compozy/tasks/multi-task-run/task_07.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_08.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_09.mdis excluded by!**/*.md.compozy/tasks/multi-task-run/task_10.mdis excluded by!**/*.mdREADME.mdis excluded by!**/*.mddocs/events.mdis excluded by!**/*.mdopenapi/compozy-daemon.jsonis excluded by!**/*.jsonweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (36)
internal/api/client/client.gointernal/api/client/client_contract_test.gointernal/api/contract/contract_test.gointernal/api/contract/types.gointernal/api/core/handlers.gointernal/api/core/handlers_smoke_test.gointernal/api/httpapi/openapi_contract_test.gointernal/cli/daemon_commands.gointernal/cli/daemon_commands_test.gointernal/cli/root_command_execution_test.gointernal/cli/root_test.gointernal/cli/run_observe.gointernal/cli/state.gointernal/cli/tasks_run_parallel_e2e_test.gointernal/cli/testdata/tasks_run_help.goldeninternal/config/home.gointernal/config/home_test.gointernal/core/run/ui/multi_remote.gointernal/core/run/ui/multi_remote_test.gointernal/core/workspace/config_merge.gointernal/core/workspace/config_test.gointernal/core/workspace/config_types.gointernal/core/workspace/config_validate.gointernal/daemon/host.gointernal/daemon/run_manager.gointernal/daemon/run_manager_test.gointernal/daemon/task_multi.gointernal/daemon/task_multi_test.gointernal/daemon/task_multi_worktree.gointernal/daemon/task_multi_worktree_test.gointernal/store/sqlite.gointernal/version/version.gopkg/compozy/events/docs_test.gopkg/compozy/events/kinds/docs_test.gopkg/compozy/events/kinds/payload_compat_test.gopkg/compozy/events/kinds/task.go
- worktree path appends a SHA-256 digest of the full parent run id so distinct parent runs no longer collide on the shared truncated "task-multi-<date>" prefix; preserved worktrees previously made a re-run fail with "target already exists". - assert globaldb.ErrRunNotFound for the not-started child instead of a bare nil check. - wrap test bodies / rename table cases to Should-prefixed t.Run subtests per the repo convention (contract, config, handlers smoke, parallel e2e, kinds docs, worktree sanitize). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…vent Addresses Greptile review findings on compozy#200: - Ctrl+C during a streamed multi-run no longer surfaces a context-canceled error: the final handoff fetch is skipped when the context is already done (P1). - An explicit --parallel-limit is now rejected when the resolved mode is enqueued instead of being silently discarded (P2). - Parallel mode no longer emits item_queued twice per child; the upfront seeding is skipped for parallel (the started event already seeds the snapshot) so --stream shows each slug queued once (P2). Regression tests added for each. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
pedronauck
requested changes
Jun 15, 2026
pedronauck
left a comment
pedronauck
left a comment
Member
There was a problem hiding this comment.
I reviewed this PR from an isolated worktree and ran three independent review passes (Skeptic, Architect, Minimalist). The core scheduler/worktree implementation looks solid overall, and make verify passes, but I’m requesting changes for one public contract mismatch that will affect generated clients.
Blocking change request
TaskRunMultipleItem.status is inconsistent across the live daemon, event docs, OpenAPI, and generated TypeScript client.
The daemon emits and snapshots "running" for an active child item:
internal/daemon/task_multi.go:25definestaskMultiItemStatusRunning = "running".internal/daemon/task_multi.go:1121copies the event payload status directly into the API snapshot item.docs/events.md:370documents the item status set asqueued,running,completed,failed, orcanceled.
But the public schema and generated TS client still advertise "active" and omit "running":
openapi/compozy-daemon.json:1527has["queued", "active", "completed", "failed", "canceled"].web/src/generated/compozy-openapi.d.ts:987mirrors that as"queued" | "active" | "completed" | "failed" | "canceled".- The new metadata round-trip test in
internal/api/contract/contract_test.go:558also preservesStatus: "active", which reinforces the stale schema value instead of the runtime value.
Failure scenario: while a multi-run child is running, GET /api/task-runs/multiple/:run_id/snapshot can return status: "running", which is outside the generated TypeScript union. Consumers generated from this PR’s OpenAPI contract can reject, mishandle, or fail to exhaustively handle a normal live snapshot. "active" is effectively dead for this path unless the daemon explicitly remaps it.
Please either:
- Update the OpenAPI schema/source and generated TypeScript types to use
"running", and update the contract tests to prove snapshot item statuses match runtime/event statuses, or - Intentionally remap the daemon snapshot status to
"active"everywhere and update the event docs/runtime tests accordingly.
Non-blocking notes
_txlock=immediateininternal/store/sqlite.gois a process-wide SQLite behavior change, not just a parallel multi-run fix. The rationale makes sense for concurrent child writers, but it is worth calling out in the PR because explicit read transactions elsewhere may now wait for or contend on the writer lock.- The TUI now reserves a permanent
worktree —line even for default enqueued multi-runs where worktree metadata is never populated. That is not a merge blocker, but suppressing the line until any worktree metadata exists would preserve transcript space and avoid a confusing placeholder. - If a journal emit fails immediately after worktree allocation, the worktree can exist without metadata in the snapshot. Narrow window, but worth considering in follow-up cleanup/recovery work.
Verification I ran locally in ../_worktrees/pr-200-review:
make verify-> exit 0- Focused checks:
go test ./internal/daemon ...,go test ./internal/cli ...,go test ./internal/api/contract,go test ./internal/api/client,go test ./internal/api/core, narrowinternal/api/httpapiOpenAPI contract test,go test ./pkg/compozy/events/..., andgo test ./internal/core/workspace ./internal/configall passed.
Resolve two conflicts introduced by the upstream TUI redesign (compozy#201): - internal/cli/daemon_commands.go: keep version.UnstampedCommit (same "none" value as upstream's noneValue, but semantically precise for a build commit). - internal/core/run/ui/multi_remote.go: port the multi-run worktree handoff line onto the redesigned tab-strip API (renderBrandTabsRow, charmtheme.Keycap, bg-less renderGap/renderOwnedLineKnownOwned). multiRunTabsHeight stays 3 to reserve the tab line, worktree status line, and separator. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…tract The daemon emits and snapshots "running" for an active multi-run child item (taskMultiItemStatusRunning) and docs/events.md documents the set as queued/running/completed/failed/canceled, but the OpenAPI schema and the generated TypeScript client still advertised "active". A live GET /api/task-runs/multiple/:run_id/snapshot could therefore return a status outside the generated TS union, breaking generated consumers. - openapi/compozy-daemon.json: TaskRunMultipleItem.status enum active -> running - web/src/generated/compozy-openapi.d.ts: regenerated via `bun run codegen` - contract/smoke tests: assert "running" instead of the stale "active" - internal/daemon: add TestTaskRunMultipleItemStatusesMatchOpenAPIEnum proving the runtime status constants exactly match the published OpenAPI enum, so the contract and runtime can no longer drift silently. Addresses PR compozy#200 review (pullrequestreview-4499973566). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
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/run/ui/multi_remote_test.go`:
- Around line 56-83: Wrap the body of test functions in the t.Run() subtest
pattern to align with the repository's testing standards. For the
TestRemoteMultiRunModelAppliesWorkspaceRootToParentAndChildren function and the
test code at lines 296-330, 332-350, and 394-414, move the test logic inside a
t.Run("Should...", func(t *testing.T) { ... }) call, using descriptive subtest
names that clearly explain what the test validates. This ensures all test cases
follow the mandatory t.Run() wrapper pattern required by the coding guidelines.
🪄 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: 14e6d9c0-4e10-4cfd-9eb2-b13a8c303943
⛔ Files ignored due to path filters (3)
docs/events.mdis excluded by!**/*.mdopenapi/compozy-daemon.jsonis excluded by!**/*.jsonweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (18)
internal/api/client/client.gointernal/api/client/client_contract_test.gointernal/api/contract/contract_test.gointernal/api/contract/types.gointernal/api/core/handlers.gointernal/api/core/handlers_smoke_test.gointernal/api/httpapi/openapi_contract_test.gointernal/cli/daemon_commands.gointernal/cli/daemon_commands_test.gointernal/cli/root_command_execution_test.gointernal/cli/run_observe.gointernal/core/run/ui/multi_remote.gointernal/core/run/ui/multi_remote_test.gointernal/core/workspace/config_test.gointernal/core/workspace/config_validate.gointernal/daemon/run_manager.gointernal/daemon/task_multi_test.gopkg/compozy/events/docs_test.go
🚧 Files skipped from review as they are similar to previous changes (16)
- internal/api/client/client.go
- internal/api/contract/types.go
- internal/core/workspace/config_validate.go
- internal/daemon/run_manager.go
- internal/api/core/handlers.go
- pkg/compozy/events/docs_test.go
- internal/api/httpapi/openapi_contract_test.go
- internal/cli/run_observe.go
- internal/api/contract/contract_test.go
- internal/api/client/client_contract_test.go
- internal/core/workspace/config_test.go
- internal/cli/daemon_commands.go
- internal/cli/daemon_commands_test.go
- internal/core/run/ui/multi_remote.go
- internal/cli/root_command_execution_test.go
- internal/daemon/task_multi_test.go
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/run/ui/multi_remote_test.go`:
- Around line 56-83: Wrap the body of test functions in the t.Run() subtest
pattern to align with the repository's testing standards. For the
TestRemoteMultiRunModelAppliesWorkspaceRootToParentAndChildren function and the
test code at lines 296-330, 332-350, and 394-414, move the test logic inside a
t.Run("Should...", func(t *testing.T) { ... }) call, using descriptive subtest
names that clearly explain what the test validates. This ensures all test cases
follow the mandatory t.Run() wrapper pattern required by the coding guidelines.
🪄 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: 14e6d9c0-4e10-4cfd-9eb2-b13a8c303943
⛔ Files ignored due to path filters (3)
docs/events.mdis excluded by!**/*.mdopenapi/compozy-daemon.jsonis excluded by!**/*.jsonweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**
📒 Files selected for processing (18)
internal/api/client/client.gointernal/api/client/client_contract_test.gointernal/api/contract/contract_test.gointernal/api/contract/types.gointernal/api/core/handlers.gointernal/api/core/handlers_smoke_test.gointernal/api/httpapi/openapi_contract_test.gointernal/cli/daemon_commands.gointernal/cli/daemon_commands_test.gointernal/cli/root_command_execution_test.gointernal/cli/run_observe.gointernal/core/run/ui/multi_remote.gointernal/core/run/ui/multi_remote_test.gointernal/core/workspace/config_test.gointernal/core/workspace/config_validate.gointernal/daemon/run_manager.gointernal/daemon/task_multi_test.gopkg/compozy/events/docs_test.go
🚧 Files skipped from review as they are similar to previous changes (16)
- internal/api/client/client.go
- internal/api/contract/types.go
- internal/core/workspace/config_validate.go
- internal/daemon/run_manager.go
- internal/api/core/handlers.go
- pkg/compozy/events/docs_test.go
- internal/api/httpapi/openapi_contract_test.go
- internal/cli/run_observe.go
- internal/api/contract/contract_test.go
- internal/api/client/client_contract_test.go
- internal/core/workspace/config_test.go
- internal/cli/daemon_commands.go
- internal/cli/daemon_commands_test.go
- internal/core/run/ui/multi_remote.go
- internal/cli/root_command_execution_test.go
- internal/daemon/task_multi_test.go
🛑 Comments failed to post (1)
internal/core/run/ui/multi_remote_test.go (1)
56-83:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdopt mandatory
t.Run("Should...")wrappers for these new test cases.These added tests are written as direct top-level bodies instead of the required subtest pattern. Please wrap each case in
t.Run("Should ...", func(t *testing.T) { ... })to align with the repository’s_test.gostandard.As per coding guidelines, "
MUST use t.Run(\"Should...\") pattern for ALL test cases" and "Use table-driven tests with subtests (t.Run) as the default pattern."Also applies to: 296-330, 332-350, 394-414
🤖 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 56 - 83, Wrap the body of test functions in the t.Run() subtest pattern to align with the repository's testing standards. For the TestRemoteMultiRunModelAppliesWorkspaceRootToParentAndChildren function and the test code at lines 296-330, 332-350, and 394-414, move the test logic inside a t.Run("Should...", func(t *testing.T) { ... }) call, using descriptive subtest names that clearly explain what the test validates. This ensures all test cases follow the mandatory t.Run() wrapper pattern required by the coding guidelines.Source: Coding guidelines
CodeRabbit flagged four test functions in multi_remote_test.go that ran their
assertions at the top level instead of the repository's mandatory
t.Run("Should...") subtest pattern. Wrap each body in a descriptive subtest:
- TestRemoteMultiRunModelAppliesWorkspaceRootToParentAndChildren
- TestMultiRunSpinnerSurvivesIdleActiveTab
- TestMultiRunSpinnerStopsWhenNoTabActive
- TestMultiRunTabsShowBrandOnce
No behavior change; assertions are unchanged. Addresses PR compozy#200 CodeRabbit review.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This was referenced Jun 16, 2026
compozybot
added a commit
that referenced
this pull request
Jul 3, 2026
* docs: update skill * fix: support parallel execution when compozy runs inside a git worktree Worktree-backed parallel execution (#200, #217) assumed the workspace root is a primary checkout and was never tested from inside a linked git worktree. Root-caused and fixed four confirmed failure modes: - Git env leakage: daemon/CLI git runners inherited GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, GIT_COMMON_DIR and could resolve the wrong repository. Added internal/core/gitenv with a shared sanitizer applied to the worktree allocator, review-watch runner, CLI preflight, and snapshot runner. - Repo-wide prune blast radius: scoped cleanup no longer runs git worktree prune across the whole repo family; owned paths are removed explicitly and Prune was dropped from the production WorktreeLifecycle surface. - Purge vs live nested runs: run purge now defers when a planned worktree hosts another active run's workspace root, and when the registered workspace root no longer exists; shutdown logs and skips deferred purges instead of failing the pass. - Late detached-HEAD failure: parallel task modes now preflight in the CLI before daemon bootstrap, rejecting compozy-managed worktree recursion and detached HEAD with actionable errors before any run rows, events, or worktrees are created. Refuted with new linked-worktree coverage: fast-forward semantics, purge branch-retention scope, artifact mirroring, workspace registry double identity, and snapshot/scope on .git-file worktrees. Added a linked-worktree fixture and S1-S6 scenario tests (unit + e2e). Co-Authored-By: Claude Fable 5 <[email protected]> * fix: address pr 223 review batch * fix: wrap gitenv run errors * fix: address gitenv review feedback --------- Co-authored-by: Pedro Nauck <[email protected]> Co-authored-by: Claude Fable 5 <[email protected]>
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
2 participants
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.
What this does
compozy tasks run --multiple alpha,beta --parallelnow actually runs the tasks at the same time. Until nowparallelwas an accepted value that quietly fell back to sequential (enqueued) execution, so the flag was a promise the daemon never kept. This change makes it real: each child task runs in its own git worktree, several at once, bounded by a configurable limit.enqueuedstays the default. You only get parallelism when you ask for it, with--parallelor[tasks.run] run_multiple_mode = "parallel".Why
Running independent task batches one after another wastes time when they don't depend on each other. The point of this work is wall-clock time for solo developers running independent tasks, without losing the per-task status and manual-review handoff the existing multi-run flow already gives you. Each child keeps its worktree after the run so you can read the diffs yourself. Nothing is auto-merged or auto-pushed.
The full background lives in the PRD, tech spec, and ADRs under
.compozy/tasks/multi-task-run/. ADR-005 (worktree-backed parallel as opt-in) and ADR-008 (configurable limit) are the load-bearing ones for this PR.What you get
--paralleland--parallel-limit Nontasks run --multiple.[tasks.run] run_multiple_parallel_limit(default2, must be positive).--parallel-limitflag wins, then the config key, then the default of 2. Mode precedence:--parallel, thenrun_multiple_mode, thenenqueued.~/.compozy/state/worktrees/..., kept after the run.failedand names which slugs failed, but only after every child has settled. It endscompletedonly when all children complete.worktree=<path>and prints a final handoff block listing every slug, its status, run id, and worktree.One thing the README is explicit about: worktrees isolate git working state and nothing else. They do not isolate ports, credentials, caches, provider rate limits, or databases, and they will not prevent merge conflicts. Don't expect full sandboxing.
How it works
The daemon already owned a sequential multi-run coordinator (
task_multi). This PR turns it into one mode-aware scheduler that dispatches to either the existing enqueued path (unchanged) or the new parallel path. Both share the same event, snapshot, and cancellation helpers, so there aren't two coordinators drifting apart.The parallel path does four things:
Worktree metadata (path, base branch, base commit, status) travels on the existing
task.multi.*item events and gets rebuilt into the snapshot, so attaching late or recovering after a crash still shows where each child ran.The SQLite change worth a close look
This is the part to review carefully. The daemon's global catalog is SQLite in WAL mode (modernc driver). Running children at the same time surfaced a real bug: two children starting together both open a deferred transaction, both read, then both try to upgrade to a write. SQLite returns
SQLITE_BUSYimmediately for that read-to-write upgrade, andbusy_timeoutdoes not retry it. Under the race detector it reproduced every time; without it the timing usually hid the failure.The fix is
_txlock=immediateon the DSN, so write transactions start withBEGIN IMMEDIATEand take the write lock up front. Concurrent writers then wait onbusy_timeout(5s) instead of deadlocking. It's a one-line DSN change, but it touches every database opened throughstore.OpenSQLiteDatabase, not just multi-run, so I ran it against the whole suite (3669 tests, race detector on) with no regressions. I also checked the modernc driver source to confirm it honors_txlockbefore depending on it.I looked at serializing child startup instead, but that only moves the problem: children also write concurrently during execution and teardown. Fixing the lock mode at the source is the honest fix.
Tests
It all passes under
make verify(fmt, lint at zero issues, race tests, frontend checks, and the Playwright smoke suite).Out of scope
No auto-merge, no auto-push, no conflict detection, no dependency-aware scheduling, no automatic worktree cleanup. Those are later phases in the PRD.
One known rough edge, recorded in the workflow memory: a
--dry-runparallel run still creates real worktrees on disk, because only child execution is short-circuited. A follow-up could plan worktree paths without actually runninggit worktree add.Commits
Ten task commits plus one for the planning docs, each gated by a green
make verify:--parallel/--parallel-limitCLI controls and request wiringtask_multischeduler refactorTrying it
Run it from a git workspace where each slug's task files are committed. The worktrees are detached checkouts of HEAD, so the task directories have to exist in the commit.
🤖 Generated with Claude Code
Summary by CodeRabbit
compozy tasks run --multiplevia--parallel, using git worktree isolation.--parallel-limit(default: 2) andrun_multiple_parallel_limitconfig to cap concurrency; multi-run requests/events/UI now includeparallel_limitand worktree metadata.parallel_limit, including correct request encoding and snapshot reconstruction.