Recursive Subagent Execution & Collapsible Tree Log UI & Codegen Documentation - #21
Conversation
…I inspector dropdown configuration, nested collapsible logs rendering, and codegen sub-workflow documentation. Also add comprehensive unit tests.
|
👋 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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughSubagent nodes can select workflows, execute nested workflows, include nested workflow metadata in generated code, and display recursive execution logs. Workflow context now flows through inspectors, code generation, stepper execution, and full flow execution. ChangesRecursive subagent workflows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Inspector
participant ParentFlow
participant runFlow
participant ChildWorkflow
Inspector->>ParentFlow: select child workflow
ParentFlow->>runFlow: execute subagent with workflow context
runFlow->>ChildWorkflow: run nested workflow with initial state
ChildWorkflow-->>runFlow: return logs and output
runFlow-->>ParentFlow: return nested 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: 8
🤖 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`:
- Line 364: Update both workflow lookup sites in frontend/src/flow/codegen.ts at
lines 364-364 and 585-585 to resolve an exact ID match before considering names,
then use a name match only when exactly one workflow has that name; do not
select arbitrarily among duplicate names.
- Line 368: The generated Python and JavaScript comments interpolate
Workflow.name without sanitization, allowing line terminators to escape the
comment. Add or reuse a name-sanitization step before inserting subWf.name at
frontend/src/flow/codegen.ts lines 368-368 and 589-589, applying identical
behavior in both generation paths.
In `@frontend/src/flow/runFlow.ts`:
- Around line 353-363: Update runFlow and its nested invocation to carry
execution ancestry and nesting depth, rejecting a workflow ID that already
appears in the ancestry with a descriptive error before starting the nested run.
Also enforce a maximum nesting depth as a fallback, while preserving normal
acyclic workflow execution and propagating the guarded context through the
existing allWfs-based recursion.
- Around line 365-379: The subflow result handling in runFlow must propagate
failures from the final nested log: before deriving finalResult or returning the
normal subagent result, check lastLog.error and throw it. Preserve the existing
result selection for successful nested flows so the parent flow can use its
error path when a nested flow fails or reaches the max-step guard.
- Around line 328-329: Replace the dynamic execution in the `runFlow` flow
around `new Function` and `fn(state)` so `cfg.input` is never evaluated as
JavaScript. Parse or interpolate the configured value using JSON input or an
existing restricted expression mechanism, preserving the expected state-derived
input behavior without exposing browser globals or arbitrary network and storage
APIs.
In `@frontend/src/pages/Index.tsx`:
- Around line 74-85: Update frontend/src/pages/Index.tsx lines 74-85 in
RunLogItem to accept a parent invocation path and include that path when
constructing uniqueKey, ensuring identical child workflows have distinct
expansion state; update frontend/src/pages/Index.tsx lines 1289-1307 to build
expand-all keys using the same invocation path so both key-generation paths
remain consistent.
- Line 868: Update the useCallback dependency arrays for stepForward,
resumeStepper, and runFlowAction to include workflows, so each execution
callback uses the current workflow collection. Apply this change at
frontend/src/pages/Index.tsx lines 868-868, 1068-1068, and 1225-1225.
In `@frontend/src/test/subagentRecursion.test.tsx`:
- Around line 159-189: The test around Inspector should include a workflow
object with ID "parent-wf-id" in the workflows prop so the active-workflow
filtering is exercised. Keep "child-researcher" available, then assert the
rendered option values contain "child-researcher" and exclude "parent-wf-id".
🪄 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: 8d4a08da-8714-47e8-bc0d-48ff6d8108d8
⛔ Files ignored due to path filters (1)
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/subagentRecursion.test.tsx
| case "subagent": | ||
| case "subagent": { | ||
| const subWfId = c.graph || ""; | ||
| const subWf = workflows?.find((w) => w.id === subWfId || w.name === subWfId); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use deterministic workflow resolution in both generators.
The current lookup can return an earlier name match instead of an exact ID match. Duplicate names can also select an arbitrary workflow.
frontend/src/flow/codegen.ts#L364-L364: resolve an exact ID first, then accept only a unique name match.frontend/src/flow/codegen.ts#L585-L585: apply the same resolution order and duplicate-name handling.
📍 Affects 1 file
frontend/src/flow/codegen.ts#L364-L364(this comment)frontend/src/flow/codegen.ts#L585-L585
🤖 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` at line 364, Update both workflow lookup sites
in frontend/src/flow/codegen.ts at lines 364-364 and 585-585 to resolve an exact
ID match before considering names, then use a name match only when exactly one
workflow has that name; do not select arbitrarily among duplicate names.
| const docComment = subWf | ||
| ? [ | ||
| `# Nested Subagent Workflow Details:`, | ||
| `# Name: ${subWf.name}`, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize workflow names before emitting generated source comments.
Workflow.name is interpolated directly into Python and JavaScript line comments. A line terminator ends the comment, and the remaining text can become executable code inside the generated node function.
frontend/src/flow/codegen.ts#L368-L368: sanitizesubWf.namebefore inserting it into the Python comment.frontend/src/flow/codegen.ts#L589-L589: apply the same sanitization before inserting it into the JavaScript comment.
📍 Affects 1 file
frontend/src/flow/codegen.ts#L368-L368(this comment)frontend/src/flow/codegen.ts#L589-L589
🤖 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` at line 368, The generated Python and
JavaScript comments interpolate Workflow.name without sanitization, allowing
line terminators to escape the comment. Add or reuse a name-sanitization step
before inserting subWf.name at frontend/src/flow/codegen.ts lines 368-368 and
589-589, applying identical behavior in both generation paths.
| const fn = new Function("state", `return ${inputVal};`); | ||
| const evaluated = fn(state); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not execute cfg.input as JavaScript.
cfg.input is workflow configuration. Imported or edited workflow data can execute in the browser global scope through new Function. A malicious workflow can access browser storage and send data through network APIs.
Use interpolation, JSON input, or a restricted expression parser. Do not evaluate configuration as JavaScript.
🧰 Tools
🪛 OpenGrep (1.26.0)
[ERROR] 328-328: new Function() with dynamic input can execute arbitrary code. Avoid dynamic code evaluation entirely, or use a safe alternative.
(coderabbit.code-injection.new-function-js)
🤖 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 328 - 329, Replace the dynamic
execution in the `runFlow` flow around `new Function` and `fn(state)` so
`cfg.input` is never evaluated as JavaScript. Parse or interpolate the
configured value using JSON input or an existing restricted expression
mechanism, preserving the expected state-derived input behavior without exposing
browser globals or arbitrary network and storage APIs.
Source: Linters/SAST tools
| const subLogs = await runFlow({ | ||
| nodes: subWf.nodes, | ||
| edges: subWf.edges, | ||
| gateways, | ||
| initialState: subState, | ||
| stepDelay: opts.stepDelay, | ||
| onHumanApproval: opts.onHumanApproval, | ||
| globals: globalsList, | ||
| secrets: secretsList, | ||
| workflows: allWfs, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Reject recursive workflow cycles before starting the nested run.
The nested call passes allWfs back into runFlow without an ancestry or depth guard. A self-reference, or a cycle such as A → B → A, creates unbounded nested execution. The per-flow maxSteps guard does not limit recursion across calls.
Track selected workflow IDs in the execution context. Throw a descriptive error when a workflow ID repeats. Add a maximum nesting depth as a fallback.
🤖 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 353 - 363, Update runFlow and its
nested invocation to carry execution ancestry and nesting depth, rejecting a
workflow ID that already appears in the ancestry with a descriptive error before
starting the nested run. Also enforce a maximum nesting depth as a fallback,
while preserving normal acyclic workflow execution and propagating the guarded
context through the existing allWfs-based recursion.
| const lastLog = subLogs[subLogs.length - 1]; | ||
| let finalResult: unknown = undefined; | ||
| if (lastLog) { | ||
| if (lastLog.stateSnapshot && "last_output" in lastLog.stateSnapshot) { | ||
| finalResult = lastLog.stateSnapshot.last_output; | ||
| } else { | ||
| finalResult = lastLog.output; | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| simulated: true, | ||
| subagent: cfg.graph || "unknown", | ||
| input: interpolate(cfg.input || "", state, globalsList, secretsList), | ||
| note: "subagent execution is schematic", | ||
| workflowId: subWf.id, | ||
| workflowName: subWf.name, | ||
| subLogs, | ||
| result: finalResult, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate an unhandled nested-flow error to the parent flow.
runFlow records failures in RunLog.error. This code ignores lastLog.error and returns a normal subagent result. The parent flow then follows its success path even when the nested flow stopped on an error or the max-step guard.
If the final nested log has error, throw it from the subagent node so that the parent can select its error edge.
🤖 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 365 - 379, The subflow result
handling in runFlow must propagate failures from the final nested log: before
deriving finalResult or returning the normal subagent result, check
lastLog.error and throw it. Preserve the existing result selection for
successful nested flows so the parent flow can use its error path when a nested
flow fails or reaches the max-step guard.
| const uniqueKey = `${log.step}-${log.nodeId}-${depth}`; | ||
| const isExpanded = !!expandedLogSnapshots[uniqueKey]; | ||
|
|
||
| const hasSubLogs = log.kind === "subagent" && | ||
| log.output && | ||
| typeof log.output === "object" && | ||
| "subLogs" in (log.output as any) && | ||
| Array.isArray((log.output as any).subLogs); | ||
|
|
||
| const subLogs: RunLog[] = hasSubLogs ? (log.output as any).subLogs : []; | ||
| const subLogsKey = `${uniqueKey}-sublogs`; | ||
| const isSubLogsExpanded = expandedLogSnapshots[subLogsKey] !== false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use an invocation path for log expansion keys.
Two invocations of the same child workflow at the same depth produce identical expansion keys. Expanding one nested snapshot or nested execution section also expands the other.
frontend/src/pages/Index.tsx#L74-L85: Pass a parent invocation path intoRunLogItemand include it inuniqueKey.frontend/src/pages/Index.tsx#L1289-L1307: Build expand-all keys from the same invocation path.
📍 Affects 1 file
frontend/src/pages/Index.tsx#L74-L85(this comment)frontend/src/pages/Index.tsx#L1289-L1307
🤖 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 74 - 85, Update
frontend/src/pages/Index.tsx lines 74-85 in RunLogItem to accept a parent
invocation path and include that path when constructing uniqueKey, ensuring
identical child workflows have distinct expansion state; update
frontend/src/pages/Index.tsx lines 1289-1307 to build expand-all keys using the
same invocation path so both key-generation paths remain consistent.
| edges, | ||
| gateways, | ||
| onHumanApproval: approvalPromise, | ||
| workflows: [...TEMPLATES, ...workflows], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include workflows in each execution callback dependency array.
stepForward, resumeStepper, and runFlowAction capture workflows, but their useCallback dependency arrays omit it. If a custom nested workflow changes while the current canvas nodes do not change, these paths use a stale workflow collection.
frontend/src/pages/Index.tsx#L868-L868: AddworkflowstostepForwarddependencies.frontend/src/pages/Index.tsx#L1068-L1068: AddworkflowstoresumeStepperdependencies.frontend/src/pages/Index.tsx#L1225-L1225: AddworkflowstorunFlowActiondependencies.
📍 Affects 1 file
frontend/src/pages/Index.tsx#L868-L868(this comment)frontend/src/pages/Index.tsx#L1068-L1068frontend/src/pages/Index.tsx#L1225-L1225
🤖 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` at line 868, Update the useCallback dependency
arrays for stepForward, resumeStepper, and runFlowAction to include workflows,
so each execution callback uses the current workflow collection. Apply this
change at frontend/src/pages/Index.tsx lines 868-868, 1068-1068, and 1225-1225.
| it("should render dropdown menu with other workflows in Inspector UI, excluding active", () => { | ||
| const activeWorkflowId = "parent-wf-id"; | ||
| const changeMock = () => {}; | ||
| const deleteMock = () => {}; | ||
|
|
||
| const nodeUnderTest: Node<AgentNodeData> = parentNodes.find((n) => n.id === "p2")!; | ||
|
|
||
| const { container } = render( | ||
| <ReactFlowProvider> | ||
| <Inspector | ||
| node={nodeUnderTest} | ||
| edges={parentEdges} | ||
| nodes={parentNodes} | ||
| activeWorkflowId={activeWorkflowId} | ||
| workflows={[subWf]} | ||
| onChange={changeMock} | ||
| onDelete={deleteMock} | ||
| /> | ||
| </ReactFlowProvider> | ||
| ); | ||
|
|
||
| // Verify dropdown is rendered for graph selection | ||
| const selectElem = container.querySelector("select"); | ||
| expect(selectElem).not.toBeNull(); | ||
|
|
||
| // It should have options: child-researcher, and exclude the parent active graph | ||
| const options = container.querySelectorAll("option"); | ||
| const optionValues = Array.from(options).map((o) => o.value); | ||
| expect(optionValues).toContain("child-researcher"); | ||
| expect(optionValues).not.toContain("parent-wf-id"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the active-workflow exclusion assertion effective.
"parent-wf-id" is not present in workflows or in the template list. The assertion passes even if the Inspector does not filter the active workflow.
Add a workflow with ID "parent-wf-id" to workflows. Then assert that this option is absent and that "child-researcher" is present.
🤖 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/subagentRecursion.test.tsx` around lines 159 - 189, The
test around Inspector should include a workflow object with ID "parent-wf-id" in
the workflows prop so the active-workflow filtering is exercised. Keep
"child-researcher" available, then assert the rendered option values contain
"child-researcher" and exclude "parent-wf-id".
There was a problem hiding this comment.
Pull request overview
This PR extends the agent-flow canvas to support recursive execution of subagent nodes, improves the UI for selecting subagent workflows and viewing nested execution logs, and enriches generated Python/JavaScript output with self-documenting subagent workflow details. It also adds unit tests covering subagent recursion, UI dropdown behavior, and codegen annotations.
Changes:
- Implement recursive subagent execution in the simulator (
runFlow) and propagate nested sub-logs/results. - Add collapsible nested sub-log rendering in the run log UI and add workflow selection dropdown behavior in
Inspector. - Add codegen comments for nested subagent workflow metadata and add a new test suite covering recursion/UI/codegen.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server.log | Updates a dev-server output log (appears to be an accidental/ephemeral artifact). |
| frontend/src/test/subagentRecursion.test.tsx | Adds tests for subagent recursion, Inspector workflow dropdown, and codegen comments. |
| frontend/src/pages/Index.tsx | Adds recursive/collapsible nested run-log rendering and threads workflow lists into codegen/runFlow/Inspector. |
| frontend/src/flow/runFlow.ts | Implements actual subagent execution via recursive runFlow and adds workflows to options. |
| frontend/src/flow/Inspector.tsx | Adds workflow dropdown selection for subagent nodes (excluding active workflow). |
| frontend/src/flow/codegen.ts | Adds optional workflow metadata to codegen for self-documenting nested subagent comments. |
Suppressed comments (5)
frontend/src/pages/Index.tsx:156
- After adding a path-based
uniqueKey, recursiveRunLogItemcalls need to pass a child path; otherwise nested expansion state will still collide and the component will be missing required props.
{subLogs.map((sl) => (
<RunLogItem
key={`${sl.step}-${sl.nodeId}`}
log={sl}
expandedLogSnapshots={expandedLogSnapshots}
frontend/src/pages/Index.tsx:1293
handleLogsExpandAllbuilds keys usingstep/nodeId/depth, which will no longer match path-based keys (and also collides across multiple subagent runs). It should generate the same hierarchical path keys asRunLogItem.
const recurseExpand = (logsList: RunLog[], d = 0) => {
logsList.forEach((l) => {
const uniqueKey = `${l.step}-${l.nodeId}-${d}`;
if (l.stateSnapshot) {
patch[uniqueKey] = true;
frontend/src/pages/Index.tsx:2109
- Top-level
RunLogItemcalls also need to provide a stablepath(e.g., list index) so expansion keys are unique per log entry and don’t collide across similar step/node IDs.
{filteredLogs.map((l) => (
<RunLogItem
key={`${l.step}-${l.nodeId}`}
log={l}
expandedLogSnapshots={expandedLogSnapshots}
frontend/src/flow/runFlow.ts:357
- The recursive
runFlowcall should propagate the call-stack/depth guard options so nested subagent calls are also protected.
const subLogs = await runFlow({
nodes: subWf.nodes,
edges: subWf.edges,
gateways,
initialState: subState,
frontend/src/flow/runFlow.ts:320
- Subagent execution can recurse indefinitely if a workflow references itself (directly or indirectly).
Inspectorfilters the active workflow in the UI, but configs can still be set manually or cycles can occur across multiple workflows. A call-stack / max-depth guard avoids runaway recursion and makes failures clearer.
const subWfId = cfg.graph || "";
const allWfs = opts.workflows ?? [...TEMPLATES, ...loadWorkflows()];
const subWf = allWfs.find((w) => w.id === subWfId || w.name === subWfId);
if (!subWf) {
throw new Error(`Subagent workflow with ID or Name "${subWfId}" not found`);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| interface RunLogItemProps { | ||
| log: RunLog; | ||
| expandedLogSnapshots: Record<string, boolean>; | ||
| setExpandedLogSnapshots: React.Dispatch<React.SetStateAction<Record<string, boolean>>>; | ||
| depth?: number; | ||
| } | ||
|
|
||
| function RunLogItem({ log, expandedLogSnapshots, setExpandedLogSnapshots, depth = 0 }: RunLogItemProps) { | ||
| const uniqueKey = `${log.step}-${log.nodeId}-${depth}`; | ||
| const isExpanded = !!expandedLogSnapshots[uniqueKey]; | ||
|
|
||
| const hasSubLogs = log.kind === "subagent" && | ||
| log.output && | ||
| typeof log.output === "object" && | ||
| "subLogs" in (log.output as any) && | ||
| Array.isArray((log.output as any).subLogs); | ||
|
|
||
| const subLogs: RunLog[] = hasSubLogs ? (log.output as any).subLogs : []; | ||
| const subLogsKey = `${uniqueKey}-sublogs`; | ||
| const isSubLogsExpanded = expandedLogSnapshots[subLogsKey] !== false; |
| // Evaluate as JS expression against current parent state | ||
| const fn = new Function("state", `return ${inputVal};`); | ||
| const evaluated = fn(state); |
| globals?: { key: string; value: string }[]; | ||
| secrets?: { key: string; value: string }[]; | ||
| workflows?: Workflow[]; | ||
| } |
| import { Edge, Node } from "reactflow"; | ||
| import { AgentNodeData, NODE_TYPES } from "./types"; | ||
| import type { Gateway } from "./gateways"; | ||
| import { TEMPLATES, Workflow } from "./workflows"; |
| import { Edge, Node } from "reactflow"; | ||
| import { AgentNodeData, AgentNodeKind } from "./types"; | ||
| import { GlobalVar, SecretVar } from "./globals"; | ||
| import { Workflow } from "./workflows"; |
This contribution implements fully functional recursive Subagent (subagent) node execution, dynamic dropdown selection for graph configurations in Inspector.tsx, beautifully structured nested subLogs rendering with collapsible support, and self-documenting comments with node/edge counts in Python and JavaScript code generators. Comprehensive unit tests are also added to verify simulator, UI, and codegen capabilities.
PR created automatically by Jules for task 3153839162970473936 started by @Jacobcdsmith
Summary by CodeRabbit