Feature: Recursive Subagent Workflows & Nested Run Log Viewer - #19
Feature: Recursive Subagent Workflows & Nested Run Log Viewer#19Jacobcdsmith wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughSubagent nodes can select custom or template workflows, execute them recursively, and return nested logs and outputs. Generated Python and JavaScript include workflow metadata. The UI renders nested logs and state snapshots. ChangesNested subagent workflows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ParentWorkflow
participant runFlow
participant WorkflowLoader
participant NestedWorkflow
ParentWorkflow->>runFlow: execute subagent with interpolated input
runFlow->>WorkflowLoader: resolve configured workflow
WorkflowLoader-->>runFlow: return workflow definition
runFlow->>NestedWorkflow: execute recursively
NestedWorkflow-->>runFlow: return output and sublogs
runFlow-->>ParentWorkflow: return nested workflow result
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
frontend/src/pages/Index.tsx (1)
83-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the
anycast with a narrow type.
log.outputisunknownin theRunLogcontract. A local type keeps the check type-safe.♻️ Proposed refactor
- const outputObj = log.output as any; - const hasSubLogs = outputObj && Array.isArray(outputObj.subLogs); - const subLogs = hasSubLogs ? (outputObj.subLogs as RunLog[]) : []; + const outputObj = log.output as { subLogs?: RunLog[] } | null | undefined; + const subLogs = Array.isArray(outputObj?.subLogs) ? outputObj.subLogs : []; + const hasSubLogs = subLogs.length > 0;🤖 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 `@frontend/src/pages/Index.tsx` around lines 83 - 85, Replace the any cast in the sub-log extraction around outputObj with a narrow local type describing an optional subLogs array of RunLog. Keep the existing Array.isArray guard and fallback to an empty array, while accessing log.output without bypassing type safety.frontend/src/flow/runFlow.ts (1)
314-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPass the workflow library through
RunOptionsinstead of reading localStorage.
loadWorkflows()runs on every subagent node execution. It parses localStorage each time, and it ignores unsaved canvas edits, so a nested run can use a stale copy of a workflow. An optionalworkflowsfield inRunOptionsremoves the storage dependency from the executor and makes the resolution testable.- const allWfs = [...TEMPLATES, ...loadWorkflows()]; + const allWfs = [...TEMPLATES, ...(opts.workflows ?? loadWorkflows())];🤖 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 `@frontend/src/flow/runFlow.ts` around lines 314 - 346, Add an optional workflows collection to RunOptions and pass it through the runFlow execution path. In the nested workflow resolution near allWfs, use the supplied RunOptions.workflows when available, falling back to loadWorkflows() only when it is absent; preserve the TEMPLATES entries and avoid reading localStorage on each subagent execution when workflows are provided.frontend/src/test/subagents.test.ts (1)
43-114: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a test for nested-workflow cycle protection.
The suite covers one nesting level only. It does not cover a workflow that references another workflow which references the first one. That case currently recurses without limit, as noted in
frontend/src/flow/runFlow.ts. Add a test after the guard exists. Do you want me to generate that test?🤖 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 `@frontend/src/test/subagents.test.ts` around lines 43 - 114, Add a test alongside the existing nested subagent test that constructs two workflows referencing each other and executes one through runFlow. Verify recursive execution terminates via the cycle-protection guard in runFlow, producing bounded logs or the established cycle error rather than recursing indefinitely.
🤖 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 `@frontend/src/flow/codegen.ts`:
- Around line 361-380: Add a shared comment-text sanitizer in
frontend/src/flow/codegen.ts that neutralizes \n, \r\n, \u2028, and \u2029
before metadata is embedded in generated comments; apply it to persisted
workflow names in both generatePython (lines 361-380) and generateJavaScript
(lines 580-592), preserving valid single-line # and // comments. Add coverage
for line-terminator-containing workflow names in
frontend/src/test/subagents.test.ts.
In `@frontend/src/flow/Inspector.tsx`:
- Around line 68-89: Update the subagent graph <select> in Inspector so an
existing non-empty node.data.config graph value that is absent from the filtered
allWorkflows options remains visible by adding a fallback option for that value.
Preserve the existing workflow options and avoid adding a fallback for empty
values.
In `@frontend/src/flow/runFlow.ts`:
- Around line 344-378: Add recursion protection to runFlow and nested
sub-workflow execution: extend RunOptions with a depth counter and visited
workflow-ID set, stop with a bounded fallback when the depth limit is reached or
the current workflow ID is already visited, and propagate updated depth/visited
values through the recursive runFlow call in the nested workflow branch. Ensure
direct and indirect cycles terminate without changing normal acyclic execution.
In `@frontend/src/pages/Index.tsx`:
- Around line 79-80: Update handleLogsExpandAll to generate snapshot keys using
the same step-nodeId-depth format as LogItemRow’s uniqueKey, and traverse nested
logs so every sub-workflow row is included when expanding snapshots. Preserve
the existing expand-all behavior while ensuring the resulting keys match
expandedSnapshots lookups.
---
Nitpick comments:
In `@frontend/src/flow/runFlow.ts`:
- Around line 314-346: Add an optional workflows collection to RunOptions and
pass it through the runFlow execution path. In the nested workflow resolution
near allWfs, use the supplied RunOptions.workflows when available, falling back
to loadWorkflows() only when it is absent; preserve the TEMPLATES entries and
avoid reading localStorage on each subagent execution when workflows are
provided.
In `@frontend/src/pages/Index.tsx`:
- Around line 83-85: Replace the any cast in the sub-log extraction around
outputObj with a narrow local type describing an optional subLogs array of
RunLog. Keep the existing Array.isArray guard and fallback to an empty array,
while accessing log.output without bypassing type safety.
In `@frontend/src/test/subagents.test.ts`:
- Around line 43-114: Add a test alongside the existing nested subagent test
that constructs two workflows referencing each other and executes one through
runFlow. Verify recursive execution terminates via the cycle-protection guard in
runFlow, producing bounded logs or the established cycle error rather than
recursing indefinitely.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ae2b55c-97d6-402f-982e-56174cc7e2b7
⛔ Files ignored due to path filters (1)
dev_server.logis excluded by!**/*.log
📒 Files selected for processing (5)
frontend/src/flow/Inspector.tsxfrontend/src/flow/codegen.tsfrontend/src/flow/runFlow.tsfrontend/src/pages/Index.tsxfrontend/src/test/subagents.test.ts
| case "subagent": { | ||
| let subWfComment = ""; | ||
| try { | ||
| const allWfs = [...TEMPLATES, ...loadWorkflows()]; | ||
| const subWf = allWfs.find((w) => w.id === c.graph); | ||
| if (subWf) { | ||
| subWfComment = `# Nested Subagent details:\n# Name: ${subWf.name}\n# Node Count: ${subWf.nodes.length}\n# Edge Count: ${subWf.edges.length}\n`; | ||
| } | ||
| } catch { | ||
| // ignore | ||
| } | ||
| return [ | ||
| subWfComment + | ||
| `input_val = interpolate(${pyStr(c.input || "input")}, state)`, | ||
| `payload = state.get(input_val, state.last) if input_val in state.data else input_val`, | ||
| `result = await run_subgraph(${pyStr(c.graph || "sub")}, payload, state)`, | ||
| `state.last = result`, | ||
| `return "on_success"`, | ||
| ].join("\n"); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if rg -n '\$\{subWf\.name\}' frontend/src/flow/codegen.ts; then
echo "Unsanitized workflow-name interpolation remains" >&2
exit 1
fi
rg -n 'sanitizeCommentText' frontend/src/flow/codegen.tsRepository: Jacobcdsmith/agent-flow-canvas
Length of output: 552
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== codegen subagent sections =="
sed -n '330,405p' frontend/src/flow/codegen.ts
echo "---"
sed -n '550,605p' frontend/src/flow/codegen.ts
echo "== imports/types around codegen =="
sed -n '1,80p' frontend/src/flow/codegen.ts
echo "== tests mentioning workflow names/subagent generator =="
rg -n 'workflow|subagent|Nested Subagent|run_subgraph|runSubgraph|loadWorkflows|JSON.stringify\(c\.input' frontend/src -S || true
echo "== deterministic terminator behavior probe =="
python3 - <<'PY'
templates = [
f"""def subgraph(input, state):
# Nested Subagent details:
# Name: invalid
print("untrusted")
input_val = interpolate(input, state)
""",
f"""def subgraph(input, state):
# Nested Subagent details:
# Name: safe
input_val = interpolate(input, state)
""",
]
for i, t in enumerate(templates, 1):
print(f"TEMPLATE_{i}_safe={t == '# Nested Subagent details:\\n# Name: safe\\ninput_val = interpolate(input, state)\\n'}")
PY
node - <<'JS'
const templates = [
`function subgraph(input, state) {
// Nested Subagent details:
// Name: invalid
console.log("untrusted")
const inputVal = interpolate(input, state)
}`,
`function subgraph(input, state) {
// Nested Subagent details:
// Name: safe
const inputVal = interpolate(input, state)
}`,
]
for (const [i, t] of templates.entries()) {
console.log(`TEMPLATE_${i + 1}_safe=${t === ['function subgraph(input, state) {','// Nested Subagent details:','// Name: safe','const inputVal = interpolate(input, state)','}'].join('\n')}`)
}
JS
echo "== current files change stat =="
git diff --stat -- frontend/src/flow/codegen.ts frontend/src/test/subagents.test.ts 2>/dev/null || trueRepository: Jacobcdsmith/agent-flow-canvas
Length of output: 22609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== deterministic terminator behavior probe =="
python3 - <<'PY'
line_separator = "invalid\nprint(\"untrusted\")\n"
template = f"""def subgraph(input, state):
# Nested Subagent details:
# Name: {line_separator}input_val = interpolate(input, state)
"""
expected_safe = """def subgraph(input, state):
# Nested Subagent details:
# Name: safe
input_val = interpolate(input, state)
"""
expected_unsafe = """def subgraph(input, state):
# Nested Subagent details:
# Name: invalid
print("untrusted")
input_val = interpolate(input, state)
"""
print("line_break_exits_python_comment", line_separator in template and expected_unsafe == template)
print("line_break_would_execute_untrusted_code", template.index('print("untrusted")') > template.index('# Name: invalid\n'), end="\n\n")
safe_template = template.replace(line_separator, "safe")
print("replace_line_break_contains_untrusted_code", 'print("untrusted")' not in safe_template)
PY
node - <<'JS'
const unsafeName = 'invalid\nconsole.log("untrusted")\n';
const template =
`// Nested Subagent details:\n// Name: ` + unsafeName +
`const inputVal = interpolate(input, state);\n`;
const expectedUnsafe =
`// Nested Subagent details:\n// Name: invalid\nconsole.log("untrusted")\n` +
`const inputVal = interpolate(input, state);\n`;
const expectedSafe =
`// Nested Subagent details:\n// Name: unsafe\nconst inputVal = interpolate(input, state);\n`;
console.log('line_break_exits_js_comment', template.includes('console.log("untrusted")\n'));
console.log('js_unsafe_equals_template', template === expectedUnsafe);
console.log('js_safe_name_preserved', template.includes('// Name: unsafe\nconst inputVal = interpolate(input, state);') === false);
JS
echo "== related tests =="
sed -n '1,130p' frontend/src/test/subagents.test.ts 2>/dev/null || true
echo "== line-terminator tests for subagent metadata in tests =="
rg -n 'line_break|line_break|\\\\n|\\u2028|\\u2029|subagent details|Nested Subagent|subWf|workflowsAndDebugger|subagents' frontend/src/test -S || trueRepository: Jacobcdsmith/agent-flow-canvas
Length of output: 4571
Sanitize persisted workflow metadata before embedding it in generated source.
Persisted workflow names are inserted into generated Python and JavaScript comments without line-terminator handling. A name containing \n, \r\n, \u2028, or \u2029 exits the # or // comment, so the remaining text becomes executable source in the generated subagent function.
Add a shared comment-text sanitizer and apply it in both generatePython and generateJavaScript. Add tests for workflow names containing line terminators in frontend/src/test/subagents.test.ts.
📍 Affects 1 file
frontend/src/flow/codegen.ts#L361-L380(this comment)frontend/src/flow/codegen.ts#L580-L592
🤖 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 `@frontend/src/flow/codegen.ts` around lines 361 - 380, Add a shared
comment-text sanitizer in frontend/src/flow/codegen.ts that neutralizes \n,
\r\n, \u2028, and \u2029 before metadata is embedded in generated comments;
apply it to persisted workflow names in both generatePython (lines 361-380) and
generateJavaScript (lines 580-592), preserving valid single-line # and //
comments. Add coverage for line-terminator-containing workflow names in
frontend/src/test/subagents.test.ts.
| {node.data.kind === "subagent" && f.key === "graph" ? ( | ||
| <select | ||
| value={node.data.config?.[f.key] ?? ""} | ||
| onChange={(e) => | ||
| onChange(node.id, { | ||
| config: { ...node.data.config, [f.key]: e.target.value }, | ||
| }) | ||
| } | ||
| className="mt-1 w-full bg-[hsl(var(--paper))] border border-dashed border-[hsl(var(--ink-faint))] focus:border-[hsl(var(--ink))] outline-none py-1.5 px-2 text-[hsl(var(--ink))]" | ||
| > | ||
| <option value="" disabled> | ||
| -- select a subagent graph -- | ||
| </option> | ||
| {allWorkflows | ||
| .filter((w) => w.id !== activeWorkflowId) | ||
| .map((w) => ( | ||
| <option key={w.id} value={w.id}> | ||
| {w.name} {w.isTemplate ? "(Template)" : "(Custom)"} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| ) : f.type === "textarea" ? ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show existing graph values that do not match a workflow id.
The select only lists workflow ids. Existing nodes can store a free-text graph value. For example, TEMPLATES in frontend/src/flow/workflows.ts defines the node react-fallback with config: { graph: "deep_research", ... }. For such nodes the value matches no <option>, so the control renders empty and the stored value becomes invisible while it stays in the node data. Add a fallback option for an unmatched non-empty value.
🐛 Proposed fix
<option value="" disabled>
-- select a subagent graph --
</option>
+ {node.data.config?.[f.key] &&
+ !allWorkflows.some((w) => w.id === node.data.config?.[f.key]) && (
+ <option value={node.data.config[f.key]}>
+ {node.data.config[f.key]} (unresolved)
+ </option>
+ )}
{allWorkflows🤖 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 `@frontend/src/flow/Inspector.tsx` around lines 68 - 89, Update the subagent
graph <select> in Inspector so an existing non-empty node.data.config graph
value that is absent from the filtered allWorkflows options remains visible by
adding a fallback option for that value. Preserve the existing workflow options
and avoid adding a fallback for empty values.
| // Lookup target nested workflow from Workflows Library (Templates + Custom Workflows) | ||
| const allWfs = [...TEMPLATES, ...loadWorkflows()]; | ||
| const subWf = allWfs.find((w) => w.id === cfg.graph); | ||
|
|
||
| if (!subWf) { | ||
| return { | ||
| simulated: true, | ||
| subagent: cfg.graph || "unknown", | ||
| input: interpolatedInput, | ||
| note: `Sub-workflow ${cfg.graph} not found — simulated fallback`, | ||
| }; | ||
| } | ||
|
|
||
| // Execute sub-workflow recursively in-browser | ||
| const subLogs = await runFlow({ | ||
| nodes: subWf.nodes, | ||
| edges: subWf.edges, | ||
| gateways, | ||
| initialState: subInitialState, | ||
| maxSteps: opts.maxSteps, | ||
| stepDelay: opts.stepDelay, | ||
| onHumanApproval: opts.onHumanApproval, | ||
| globals: globalsList, | ||
| secrets: secretsList, | ||
| }); | ||
|
|
||
| const finalLog = subLogs[subLogs.length - 1]; | ||
| const finalOutput = finalLog ? (finalLog.output ?? finalLog.stateSnapshot?.last_output ?? null) : null; | ||
|
|
||
| return { | ||
| simulated: true, | ||
| subagent: cfg.graph || "unknown", | ||
| input: interpolate(cfg.input || "", state, globalsList, secretsList), | ||
| note: "subagent execution is schematic", | ||
| subagent: subWf.name, | ||
| input: subInitialState, | ||
| output: finalOutput, | ||
| subLogs, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Add a recursion guard for nested workflow execution.
runNode calls runFlow for the resolved sub-workflow, and that flow can contain another subagent node. maxSteps limits steps inside one flow only. It does not limit nesting depth. If workflow A references B and B references A, the recursion never terminates and the browser tab hangs. The Inspector filter only excludes the currently active workflow, so an indirect cycle is reachable from the UI, and imported JSON can also contain a direct self-reference.
Add a depth counter to RunOptions and stop when the limit is reached. A visited-id set also blocks cycles.
🛡️ Proposed fix
export interface RunOptions {
nodes: Node<AgentNodeData>[];
edges: Edge[];
gateways: Gateway[];
initialState?: Record<string, unknown>;
maxSteps?: number;
+ depth?: number;
+ maxDepth?: number;
+ workflowStack?: string[];
onLog?: (log: RunLog) => void; const allWfs = [...TEMPLATES, ...loadWorkflows()];
const subWf = allWfs.find((w) => w.id === cfg.graph);
if (!subWf) {
return {
simulated: true,
subagent: cfg.graph || "unknown",
input: interpolatedInput,
note: `Sub-workflow ${cfg.graph} not found — simulated fallback`,
};
}
+ const depth = opts.depth ?? 0;
+ const maxDepth = opts.maxDepth ?? 5;
+ const stack = opts.workflowStack ?? [];
+ if (depth >= maxDepth || stack.includes(subWf.id)) {
+ return {
+ simulated: true,
+ subagent: subWf.name,
+ input: subInitialState,
+ note:
+ stack.includes(subWf.id)
+ ? `Cycle detected — ${subWf.id} is already running`
+ : `Max nesting depth ${maxDepth} reached`,
+ };
+ }
+
// Execute sub-workflow recursively in-browser
const subLogs = await runFlow({
nodes: subWf.nodes,
edges: subWf.edges,
gateways,
initialState: subInitialState,
maxSteps: opts.maxSteps,
+ depth: depth + 1,
+ maxDepth,
+ workflowStack: [...stack, subWf.id],
stepDelay: opts.stepDelay,runFlow must forward the new fields when it calls runNode; it already passes opts through, so no further change is needed there.
🤖 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 `@frontend/src/flow/runFlow.ts` around lines 344 - 378, Add recursion
protection to runFlow and nested sub-workflow execution: extend RunOptions with
a depth counter and visited workflow-ID set, stop with a bounded fallback when
the depth limit is reached or the current workflow ID is already visited, and
propagate updated depth/visited values through the recursive runFlow call in the
nested workflow branch. Ensure direct and indirect cycles terminate without
changing normal acyclic execution.
| const uniqueKey = `${log.step}-${log.nodeId}-${depth}`; | ||
| const isExpanded = !!expandedSnapshots[uniqueKey]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The snapshot key format no longer matches handleLogsExpandAll.
LogItemRow builds uniqueKey as ${log.step}-${log.nodeId}-${depth}. handleLogsExpandAll (lines 1282-1291) still writes keys as ${l.step}-${l.nodeId}. The "Expand Snapshots" button therefore sets keys that no row reads, and no snapshot expands. Update the handler to the new format, and include nested logs so the action also covers sub-workflow rows.
🐛 Proposed fix (outside the selected range, lines 1282-1291)
const handleLogsExpandAll = () => {
if (!runLogs) return;
const patch: Record<string, boolean> = {};
- runLogs.forEach((l) => {
- if (l.stateSnapshot) {
- patch[`${l.step}-${l.nodeId}`] = true;
- }
- });
+ const walk = (items: RunLog[], depth: number) => {
+ items.forEach((l) => {
+ if (l.stateSnapshot) patch[`${l.step}-${l.nodeId}-${depth}`] = true;
+ const nested = (l.output as { subLogs?: RunLog[] } | undefined)?.subLogs;
+ if (Array.isArray(nested)) walk(nested, depth + 1);
+ });
+ };
+ walk(runLogs, 0);
setExpandedLogSnapshots(patch);
};🤖 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 `@frontend/src/pages/Index.tsx` around lines 79 - 80, Update
handleLogsExpandAll to generate snapshot keys using the same step-nodeId-depth
format as LogItemRow’s uniqueKey, and traverse nested logs so every sub-workflow
row is included when expanding snapshots. Preserve the existing expand-all
behavior while ensuring the resulting keys match expandedSnapshots lookups.
1 participant
Replaces the schematic subagent execution placeholders with fully recursive, live in-browser execution, adds workflow target select dropdown configuration in the node inspector, and renders beautifully indented, collapsible nested sub-workflow step logs in the run drawer.
PR created automatically by Jules for task 13311550323730717229 started by @Jacobcdsmith
Summary by CodeRabbit
New Features
Bug Fixes
Tests