Commit 6baab76
authored
🤖 fix: improve orchestrator dependency sequencing (#2054)
## Summary
Tighten the Orchestrator built-in agent guidance so it reliably
sequences dependent `exec` subtasks and only parallelizes patches that
can apply + verify independently.
## Background
We occasionally spawn a second `exec` task whose
implementation/verification depends on outputs from the first task.
Since child workspaces snapshot at spawn time and `exec` agents are
instructed not to expand scope, the dependent task can become blocked or
forced into scope expansion.
## Implementation
- Add a required dependency analysis checklist (inputs/outputs +
independence criteria) before batching.
- Extend the Orchestrator → Exec task brief template with a
**Dependencies / assumptions** section.
- Replace the vague sequential fallback with an explicit step-by-step
protocol (await → apply patch → then spawn dependent).
- Include a concrete example dependency chain (schema download →
generation).
## Validation
- `make static-check`
---
<details>
<summary>📋 Implementation Plan</summary>
# Improve Orchestrator dependency reasoning (task decomposition +
sequencing)
## Context / Why
The Orchestrator agent (`src/node/builtinAgents/orchestrator.md`)
currently encourages batching “independent” subtasks in parallel, with a
brief sequential fallback note.
In practice it sometimes spawns a second `exec` task whose
work/verification *depends on artifacts produced by the first task*
(e.g., new schema download targets/files). Because `exec` sub-agents are
instructed **not to expand scope**, the dependent task can become
impossible to complete cleanly without either:
- violating its scope constraints, or
- producing a patch that can’t be verified/applied independently.
Goal: update the Orchestrator prompt so it reliably (1) detects
dependency chains, (2) sequences prerequisite tasks *and integrates
their patches* before spawning dependents, and (3) only parallelizes
truly mergeable/standalone tasks.
## Evidence (repo facts consulted)
- `src/node/builtinAgents/orchestrator.md`
- “Patch integration loop (default)” recommends batching independent
subtasks and running them with `run_in_background: true`.
- “Sequential fallback” is currently a single bullet: “If subtasks
depend on one another, do them in order and apply patches between them.”
- `src/node/builtinAgents/exec.md`
- Child `exec` agents: “Take a single narrowly scoped task… Do not
expand scope.”
- `src/node/services/tools/task.ts` +
`src/common/utils/tools/toolDefinitions.ts`
- `task` has only `run_in_background` for sequencing; there is no
first-class `dependsOn` DAG concept.
- Explore reports (authoritative for referenced files):
- Orchestrator prompt location + dependency wording + task spawn logic.
- Task/workspace types show parent/child nesting but no dependency graph
field; sequencing is done via `run_in_background:false` or `task_await`.
## Recommended approach (A): prompt-only improvements (fast + low risk)
**Net LoC estimate (product code):** ~+60 / -10 (mostly
`src/node/builtinAgents/orchestrator.md`).
### Implementation details
#### 1) Strengthen Orchestrator rules for dependency analysis before
batching
**File:** `src/node/builtinAgents/orchestrator.md`
Add an explicit “Dependency analysis (required)” section *above* “Patch
integration loop (default)”. Make it checklist-style to reduce
ambiguity.
Key rules to add:
- **Define subtask IO:** For each candidate subtask, identify:
- **Outputs**: files/targets/artifacts the patch introduces/renames
(including generated artifacts).
- **Inputs**: files/targets/artifacts the task needs to exist to
implement *or verify* its work.
- **Independence test (must pass to parallelize):** Two tasks may run
concurrently only if:
1) each patch can be applied *by itself* on current HEAD (no missing
symbols/files), and
2) each task can run its own acceptance/verification commands without
needing another pending patch, and
3) they don’t both touch the same “high-conflict” files (same Makefile,
same generator entrypoint, etc.).
- **If unsure, default to sequential or merge the tasks.** Parallelism
is an optimization; correctness is the requirement.
Suggested prompt snippet (shape only; adjust wording to match file
tone):
```md
Dependency analysis (required before spawning `exec` tasks):
- For each candidate subtask, write:
- Outputs: …
- Inputs/prereqs (including for verification): …
- A subtask is “independent” only if its patch can land + verify on current HEAD without any other pending patch.
- If task B depends on outputs from task A:
- Do not spawn B until A has completed and A’s patch is applied in the parent workspace.
- If the dependency chain is tight (download → generate → wire-up), prefer a single `exec` task instead of splitting.
```
#### 2) Make “Sequential fallback” operational (concrete steps)
Replace the current one-line “Sequential fallback” with an explicit
protocol:
```md
Sequential protocol (when any dependency exists):
1. Spawn the prerequisite `exec` task with `run_in_background: false` (or spawn, then immediately `task_await`).
2. Apply its patch (dry-run then real).
3. Only after the patch is applied, spawn the dependent `exec` task.
4. Repeat until the chain is complete.
```
Rationale: queued/background tasks inherit the repo snapshot at spawn
time; spawning dependents too early makes them work from the wrong base
and pushes them toward scope expansion.
#### 3) Update the Orchestrator → Exec task brief template to include
dependencies/assumptions
In `src/node/builtinAgents/orchestrator.md`, extend the template with a
dedicated section:
```md
- Dependencies / assumptions:
- Assumes <prereq patch or file> is already merged into the parent workspace.
- If this is not true, stop and report back; do not expand scope to “fix” prerequisites.
```
This gives child agents an explicit “escape hatch” aligned with
`exec.md` (report blockers rather than guessing/expanding scope).
#### 4) Add a short concrete example tied to the failure mode
Add a 6–10 line example showing why “schema download” must precede
“generator uses schema files”. Keep it generic, but concrete enough that
the model recognizes the pattern.
Example (paraphrased):
- Task A outputs new schema download targets/files.
- Task B reads those files and runs generation/verification.
- Therefore: A must be completed + applied before spawning B.
## Optional follow-up (B): first-class task dependencies in code
(stronger enforcement)
**Net LoC estimate (product code):** ~+250–450.
Only do this if prompt-only changes aren’t sufficient.
### Idea
Add an explicit dependency field (e.g., `dependsOnWorkspaceIds:
string[]`) to the workspace/task schema and teach `TaskService` to avoid
starting tasks until prerequisites are `reported`.
### Caveat (important)
Because child workspaces are created at `taskService.create()` time,
“dependency-aware queueing” alone may still start tasks from the wrong
repo snapshot if prerequisites land later.
To fully solve this, dependency enforcement likely needs to:
- **Delay creating the child workspace/worktree until dependencies are
satisfied**, or
- create the workspace but rebase/pull from parent just before starting.
### Likely touchpoints
- `src/common/orpc/schemas/project.ts`: extend workspace/task schema.
- `src/common/utils/tools/toolDefinitions.ts`: optional `dependsOn`
param to `task` tool.
- `src/node/services/taskService.ts`: enforce dependency readiness when
starting queued tasks.
- UI: optionally surface “Blocked by …” in `WorkspaceListItem`.
<details>
<summary>Why not jump straight to code-level dependencies?</summary>
Prompt-only updates are low-risk and align with the current tool model
(`run_in_background:false` + patch-apply between steps). A code-level
DAG is more correct long-term, but it’s easy to implement *part* of it
(queueing) while still leaving the core problem (repo snapshot
correctness) unsolved.
</details>
</details>
---
_Generated with `mux` • Model: `openai:gpt-5.2` • Thinking: `xhigh` •
Cost: `$1.83`_
<!-- mux-attribution: model=openai:gpt-5.2 thinking=xhigh costs=1.83 -->
---------
Signed-off-by: Thomas Kosiewski <[email protected]>1 parent 490ba38 commit 6baab76
File tree
3 files changed
+67
-9
lines changed- docs/agents
- src/node
- builtinAgents
- services/agentDefinitions
3 files changed
+67
-9
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
406 | 406 | | |
407 | 407 | | |
408 | 408 | | |
409 | | - | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
410 | 412 | | |
411 | 413 | | |
412 | 414 | | |
| |||
418 | 420 | | |
419 | 421 | | |
420 | 422 | | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
421 | 426 | | |
422 | 427 | | |
423 | 428 | | |
424 | 429 | | |
425 | 430 | | |
426 | 431 | | |
427 | | - | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
428 | 435 | | |
429 | 436 | | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
430 | 454 | | |
431 | 455 | | |
432 | 456 | | |
| |||
441 | 465 | | |
442 | 466 | | |
443 | 467 | | |
444 | | - | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
445 | 474 | | |
446 | | - | |
| 475 | + | |
447 | 476 | | |
448 | 477 | | |
449 | 478 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
56 | 58 | | |
57 | 59 | | |
58 | 60 | | |
| |||
64 | 66 | | |
65 | 67 | | |
66 | 68 | | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
67 | 72 | | |
68 | 73 | | |
69 | 74 | | |
70 | 75 | | |
71 | 76 | | |
72 | 77 | | |
73 | | - | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
74 | 81 | | |
75 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
76 | 100 | | |
77 | 101 | | |
78 | 102 | | |
| |||
87 | 111 | | |
88 | 112 | | |
89 | 113 | | |
90 | | - | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
91 | 120 | | |
92 | | - | |
| 121 | + | |
93 | 122 | | |
94 | 123 | | |
95 | 124 | | |
| |||
0 commit comments