fix: prepare tools for local sub-DAG runs - #2452
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe local subflow runner now installs declared child-DAG tools through a configurable installer, derives the tool PATH from the child execution context, and appends prepared tool environment entries to child agents. Tests verify installer invocation and successful tool execution. ChangesLocal sub-DAG tool preparation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LocalRunner
participant PrepareDAG
participant ToolInstaller
participant ChildAgent
LocalRunner->>PrepareDAG: Prepare declared DAG tools
PrepareDAG->>ToolInstaller: Install configured tool package
ToolInstaller-->>PrepareDAG: Return installed manifest
PrepareDAG-->>LocalRunner: Return tool environment entries
LocalRunner->>ChildAgent: Add entries to ExtraEnvs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
yohamta0
marked this pull request as ready for review
July 27, 2026 07:45
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/subflow/local_test.go (1)
160-207: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the test prove that the injected PATH is required.
The
child-toolscript always exits successfully, and the test does not constrain the inherited PATH. A host-provided executable could therefore make the test pass even iftoolEnvswere not injected. Use a unique executable name or a controlled base PATH, then assert a sentinel/output from that executable.🤖 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/subflow/local_test.go` around lines 160 - 207, Strengthen TestLocalRunPreparesDeclaredTools by ensuring child-tool can only be resolved through the injected toolEnvs PATH: use a unique executable name or constrain the inherited PATH, and make the script emit a sentinel value. Update the child DAG command and assertions to verify that sentinel output, while preserving the existing installer-call and configuration checks.
🤖 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/subflow/local.go`:
- Around line 406-416: The prepareDAGTools method currently derives WorkDir from
dag.WorkingDir instead of the request-level directory used by loadInProcessDAG
and SubDAGExecutor.Execute/Rewire. Update its work-directory resolution to use
the resolved request workDir passed through rCtx, and provide that value to
dagutools.InstallOptions so tool installation matches the child agent’s
execution directory.
---
Nitpick comments:
In `@internal/subflow/local_test.go`:
- Around line 160-207: Strengthen TestLocalRunPreparesDeclaredTools by ensuring
child-tool can only be resolved through the injected toolEnvs PATH: use a unique
executable name or constrain the inherited PATH, and make the script emit a
sentinel value. Update the child DAG command and assertions to verify that
sentinel output, while preserving the existing installer-call and configuration
checks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 552e2c6a-f5c1-4e5a-b33d-ba03a5b56f2a
📒 Files selected for processing (3)
internal/subflow/local.gointernal/subflow/local_helpers.gointernal/subflow/local_test.go
Comment on lines
+406
to
+416
| func (r *Local) prepareDAGTools(ctx context.Context, rCtx exec.Context, dag *core.DAG) ([]string, error) { | ||
| cfg := config.GetConfig(ctx) | ||
| workDir := "" | ||
| if dag != nil { | ||
| workDir = dag.WorkingDir | ||
| } | ||
| return dagutools.PrepareDAG(ctx, dag, r.toolInstaller, dagutools.InstallOptions{ | ||
| ToolsDir: cfg.Paths.ToolsDir, | ||
| DataDir: cfg.Paths.DataDir, | ||
| WorkDir: workDir, | ||
| }, dagToolsBasePathForLocalRunner(rCtx)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
rg -n -C3 '\bWorkDir\b' internal/subflow internal/runtime --glob '*.go'Repository: dagucloud/dagu
Length of output: 7191
🏁 Script executed:
#!/bin/sh
set -eu
echo "== outline internal/subflow/local.go relevant methods =="
ast-grep outline internal/subflow/local.go --view expanded | sed -n '1,220p' || true
echo
echo "== internal/subflow/local.go lines 360-545 =="
sed -n '360,545p' internal/subflow/local.go | nl -ba -v360
echo
echo "== dagutools PrepareDAG signature/usages =="
rg -n -C3 'func PrepareDAG|PrepareDAG\(|type InstallOptions|struct InstallOptions' internal --glob '*.go'
echo
echo "== subworkflow request builders and runParams mappings =="
rg -n -C4 'SubWorkflowRequest|WorkDir|Params|RootDAGRun|ParentDAGRun' internal/runtime/executor internal/runtime/agent internal/subflow --glob '*.go'Repository: dagucloud/dagu
Length of output: 4081
🏁 Script executed:
#!/bin/sh
set -eu
echo "== internal/subflow/local.go lines 360-575 =="
sed -n '360,575p' internal/subflow/local.go
echo
echo "== dagutools PrepareDAG signature/usages =="
rg -n -C3 'func PrepareDAG|PrepareDAG\(|type InstallOptions|struct InstallOptions' internal --glob '*.go'
echo
echo "== subworkflow request builders and runParams mappings =="
rg -n -C5 'SubWorkflowRequest|WorkDir|Params|RootDAGRun|ParentDAGRun' internal/runtime/executor internal/runtime/agent internal/subflow --glob '*.go'
echo
echo "== executor.SubWorkflowRequest definition =="
sed -n '1,80p' internal/runtime/executor/subworkflow.goRepository: dagucloud/dagu
Length of output: 50371
🏁 Script executed:
#!/bin/sh
set -eu
echo "== loadInProcessDAG and newAgent call sites =="
rg -n -C5 'dag, cleanup, err := loadInProcessDAG\(|loadInProcessDAG\(' internal/subflow/local.go internal/subflow/local_test.go
echo
echo "== newAgent callers =="
rg -n -C2 'newAgent\(' internal/subflow/local.go
echo
echo "== PrepareDAG WorkDir handling source =="
sed -n '1,130p' internal/tools/prepare.go
echo
echo "== runParams.WorkDir setters/usages =="
rg -n -C3 'type RunParams|RunParams|WorkDir\(|SetWorkingDir|WorkingDir' internal/runtime/executor internal/runtime/dag --glob '*.go'
echo
echo "== local subflow tests around workdir/workspace =="
rg -n -C4 'WorkDir|workspace|Workspace|with:' internal/subflow/internal/subflow/local_test.go internal/subflow/local_test.go
echo
echo "== files named internal/subflow/internal/subflow/local_test.go if exists and current names =="
find internal/subflow -maxdepth 2 -type f \( -name '*local*test*.go' -o -name 'local_test.go' \) -printRepository: dagucloud/dagu
Length of output: 11703
🏁 Script executed:
#!/bin/sh
set -eu
echo "== ReadStepMessages callers for WorkDir in runtime =="
rg -n -C3 'ReadStepMessages\(|WriteStepMessages\(|WorkDir\(' internal/runtime/agent internal/subflow internal/runtime/runstate --glob '*.go'
echo
echo "== SubDAGExecutor RunParams setters/callers =="
rg -n -C4 'Execute\(ctx .*RunParams|Reuse\(ctx .*RunParams|Retry\(ctx .*RunParams|\.Execute\(|\.Reuse\(|\.Retry\(' internal/runtime/executor internal --glob '*.go' | rg -C4 'SubDAGExecutor|Execute\(|Reuse\(|Retry\(|RunParams|workDir'
echo
echo "== DAG WorkingDir and WorkingDir methods =="
rg -n -C3 'WorkingDir|string WorkingDir|DefaultWorkingDir|WithDefaultWorkingDir|setWorkingDir|workingDir' internal/core internal --glob '*.go' | head -n 220Repository: dagucloud/dagu
Length of output: 50371
Use the request-level working directory for tool preparation.
loadInProcessDAG() builds the child DAG from req.WorkDir, and SubDAGExecutor.Execute/Rewire pass the caller-supplied workDir into the request, but prepareDAGTools() still uses dag.WorkingDir. Pass the resolved request work directory instead so tools are installed relative to the same directory the child agent will execute in.
🤖 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/subflow/local.go` around lines 406 - 416, The prepareDAGTools method
currently derives WorkDir from dag.WorkingDir instead of the request-level
directory used by loadInProcessDAG and SubDAGExecutor.Execute/Rewire. Update its
work-directory resolution to use the resolved request workDir passed through
rCtx, and provide that value to dagutools.InstallOptions so tool installation
matches the child agent’s execution directory.
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PATH, preserving toolset isolation from the parent DAGRoot cause
The in-process subworkflow refactor in #2248 replaced the local
dagu startsubprocess with direct agent construction. The subprocess had entered the normal DAG startup path, which prepares declared tools. The new local runner preserved the logic that removes the parent DAG's Aqua environment, but it did not prepare and inject the child DAG's own tool environment.Official actions execute through this local sub-DAG path, so actions that declare their runtime through
tools:could not find that runtime onPATH.Impact
Child DAGs and official actions now install or reuse their declared toolset and receive the same Aqua environment as root and worker runs.
Closes #2451
Testing
make fmtmake test TEST_TARGET=./internal/subflow/...go test ./internal/subflow ./internal/node ./internal/runtime/builtin/action -count=1go test ./internal/runtime/agent -run 'TestAgent_(LocalSubDAG|DAGRun.*Child|SubDAG)' -count=1duckdb@v1.5.2Summary by cubic
Fixes local sub-DAG runs by preparing declared tools before the child agent starts and injecting a clean tool PATH. Child DAGs and official actions can now resolve their tools like root and worker runs.
dagutools.PrepareDAGwith the defaultdaguaquainstaller; allow override withWithLocalToolInstaller.ExtraEnvsso steps find declared CLIs.Written for commit 7d6d2e2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests