Implement structured node inputs and outputs with variable interpolation - #3
Conversation
This commit introduces a structured node/step interface that adds `inputs` and `outputs` properties to graph-based workflow nodes.
Specifically, it implements:
- Updates to `workflows/workflow.schema.json` to include optional `inputs` and `outputs` definitions on nodes.
- DB updates in `worker/schema.sql` adding `inputs` and `outputs` text columns (storing JSON objects) to the `nodes` table.
- Worker logic updates in `worker/src/index.js` to correctly persist, fetch, validate, and execute nodes containing these fields.
- Verification updates in `scripts/validate.js` ensuring `inputs` and `outputs` are objects.
- Migration updates in `scripts/migrate-workflow.js` to generate default empty inputs/outputs.
- Dynamic evaluation/interpolation in `scripts/executor.js` to resolve references to previous nodes' outputs using the format `{{ nodes.NODE_ID.outputs.PATH }}`.
- Structured output storage (such as `response` from HTTP calls, `output` from command line execution) to the step results in execution logs.
- Realistic test coverage and samples in `workflows/example.json` and `scripts/test-api.js` demonstrating n8n-style dynamic branching and context sharing.
Co-authored-by: aethelred-agent-factory <238771426+aethelred-agent-factory@users.noreply.github.com>
|
👋 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. |
📝 WalkthroughWalkthroughNode inputs and outputs are now part of the workflow schema, validation, D1 persistence, API responses, and execution flow. The executor interpolates prior node outputs into later node inputs/configuration and records structured results. ChangesNode input/output support
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant WorkflowAPI
participant Executor
participant nodeResults
participant WorkflowNode
WorkflowAPI->>Executor: load nodes with inputs and outputs
Executor->>nodeResults: resolve prior node output references
Executor->>WorkflowNode: execute resolved inputs and config
WorkflowNode-->>Executor: return normalized outputs
Executor->>nodeResults: store node outputs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🤖 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 `@scripts/executor.js`:
- Around line 102-111: Prevent the interpolation logic using nodeResults and
getNestedValue from substituting untrusted values into run.config.command, which
is executed by /bin/bash. Route dynamic node outputs through a constrained
argument or environment interface instead of shell command text, while
preserving safe interpolation behavior for non-command fields.
- Line 164: Update the body assignment in the step request construction to check
explicitly for null or undefined rather than using a truthiness check,
preserving valid falsy values such as false, 0, and an empty string while still
producing undefined for absent bodies.
In `@worker/schema.sql`:
- Around line 22-23: Add a D1 migration for the existing nodes table that
executes ALTER TABLE nodes ADD COLUMN inputs TEXT and ALTER TABLE nodes ADD
COLUMN outputs TEXT before this Worker version is deployed, while retaining the
CREATE TABLE definitions for new databases.
In `@worker/src/index.js`:
- Around line 589-595: Reject arrays in both the worker validators at
worker/src/index.js lines 589-595 and the CLI validators at scripts/validate.js
lines 64-70 by adding Array.isArray checks to the existing inputs and outputs
object-validation conditions. Keep both validators identical so they match the
workflow schema and cannot diverge.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: adf15dc3-e868-4a47-8dc1-b9fa4ee22c53
📒 Files selected for processing (10)
executions/hello-world-1783998684604.jsonexecutions/hello-world-1784001389646.jsonscripts/executor.jsscripts/migrate-workflow.jsscripts/test-api.jsscripts/validate.jsworker/schema.sqlworker/src/index.jsworkflows/example.jsonworkflows/workflow.schema.json
💤 Files with no reviewable changes (2)
- executions/hello-world-1784001389646.json
- executions/hello-world-1783998684604.json
| return value.replace(/\{\{\s*nodes\.([a-zA-Z0-9_-]+)\.outputs\.([a-zA-Z0-9_\.-]+)\s*\}\}/g, (match, nodeId, path) => { | ||
| const nodeRes = nodeResults[nodeId]; | ||
| if (nodeRes && nodeRes.outputs) { | ||
| const val = getNestedValue(nodeRes.outputs, path); | ||
| if (val !== undefined) { | ||
| return typeof val === 'object' ? JSON.stringify(val) : String(val); | ||
| } | ||
| } | ||
| return match; | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not interpolate untrusted output into shell commands.
Interpolation applies to run.config.command, which is executed by /bin/bash. A response from an HTTP node can therefore inject shell syntax into a later run node. Keep dynamic values out of shell command text; pass them through a constrained argument/environment interface instead.
Also applies to: 134-143
🤖 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 `@scripts/executor.js` around lines 102 - 111, Prevent the interpolation logic
using nodeResults and getNestedValue from substituting untrusted values into
run.config.command, which is executed by /bin/bash. Route dynamic node outputs
through a constrained argument or environment interface instead of shell command
text, while preserving safe interpolation behavior for non-command fields.
| method: step.method || 'GET', | ||
| headers: step.headers || {}, | ||
| body: step.body ? JSON.stringify(step.body) : undefined, | ||
| body: step.body ? (typeof step.body === 'string' ? step.body : JSON.stringify(step.body)) : undefined, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve falsy HTTP request bodies.
false, 0, and "" are valid body values but are converted to undefined here. Check explicitly for null/undefined instead.
Proposed fix
- body: step.body ? (typeof step.body === 'string' ? step.body : JSON.stringify(step.body)) : undefined,
+ body: step.body !== undefined && step.body !== null
+ ? (typeof step.body === 'string' ? step.body : JSON.stringify(step.body))
+ : undefined,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| body: step.body ? (typeof step.body === 'string' ? step.body : JSON.stringify(step.body)) : undefined, | |
| body: step.body !== undefined && step.body !== null | |
| ? (typeof step.body === 'string' ? step.body : JSON.stringify(step.body)) | |
| : undefined, |
🤖 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 `@scripts/executor.js` at line 164, Update the body assignment in the step
request construction to check explicitly for null or undefined rather than using
a truthiness check, preserving valid falsy values such as false, 0, and an empty
string while still producing undefined for absent bodies.
| inputs TEXT, -- JSON string (optional inputs) | ||
| outputs TEXT, -- JSON string (optional outputs) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-filesRepository: quickerup/telegram-workflow-bot
Length of output: 565
🏁 Script executed:
cat -n worker/schema.sql && printf '\n--- wrangler ---\n' && cat -n worker/wrangler.tomlRepository: quickerup/telegram-workflow-bot
Length of output: 3452
🏁 Script executed:
rg -n "inputs|outputs|schema.sql|d1|migrate|CREATE TABLE nodes|ALTER TABLE nodes" worker scripts README.mdRepository: quickerup/telegram-workflow-bot
Length of output: 7478
🏁 Script executed:
cat -n worker/deploy.sh && printf '\n--- readme ---\n' && rg -n "d1 execute|schema.sql|deploy.sh|wrangler d1|migration" README.md worker/package.json worker/src/index.jsRepository: quickerup/telegram-workflow-bot
Length of output: 2717
🏁 Script executed:
sed -n '1,120p' README.mdRepository: quickerup/telegram-workflow-bot
Length of output: 4557
Add a D1 migration for nodes.inputs and nodes.outputs.
CREATE TABLE nodes only applies to new databases; existing D1 installs still need these columns added before this Worker version ships.
Migration SQL
ALTER TABLE nodes ADD COLUMN inputs TEXT;
ALTER TABLE nodes ADD COLUMN outputs TEXT;🤖 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 `@worker/schema.sql` around lines 22 - 23, Add a D1 migration for the existing
nodes table that executes ALTER TABLE nodes ADD COLUMN inputs TEXT and ALTER
TABLE nodes ADD COLUMN outputs TEXT before this Worker version is deployed,
while retaining the CREATE TABLE definitions for new databases.
| if (node.inputs !== undefined && (typeof node.inputs !== 'object' || node.inputs === null)) { | ||
| errors.push(`node ${i} (${node.id || 'unnamed'}): "inputs" must be an object`); | ||
| } | ||
|
|
||
| if (node.outputs !== undefined && (typeof node.outputs !== 'object' || node.outputs === null)) { | ||
| errors.push(`node ${i} (${node.id || 'unnamed'}): "outputs" must be an object`); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep both validators consistent with the JSON Schema. typeof value === 'object' accepts arrays, but workflows/workflow.schema.json declares inputs and outputs as objects. Reject arrays in both validators.
worker/src/index.js#L589-L595: addArray.isArray(node.inputs)andArray.isArray(node.outputs)to the rejection conditions.scripts/validate.js#L64-L70: apply the identical array checks so CLI and API validation cannot diverge.
- typeof node.inputs !== 'object' || node.inputs === null
+ typeof node.inputs !== 'object' || node.inputs === null || Array.isArray(node.inputs)
- typeof node.outputs !== 'object' || node.outputs === null
+ typeof node.outputs !== 'object' || node.outputs === null || Array.isArray(node.outputs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (node.inputs !== undefined && (typeof node.inputs !== 'object' || node.inputs === null)) { | |
| errors.push(`node ${i} (${node.id || 'unnamed'}): "inputs" must be an object`); | |
| } | |
| if (node.outputs !== undefined && (typeof node.outputs !== 'object' || node.outputs === null)) { | |
| errors.push(`node ${i} (${node.id || 'unnamed'}): "outputs" must be an object`); | |
| } | |
| if (node.inputs !== undefined && (typeof node.inputs !== 'object' || node.inputs === null || Array.isArray(node.inputs))) { | |
| errors.push(`node ${i} (${node.id || 'unnamed'}): "inputs" must be an object`); | |
| } | |
| if (node.outputs !== undefined && (typeof node.outputs !== 'object' || node.outputs === null || Array.isArray(node.outputs))) { | |
| errors.push(`node ${i} (${node.id || 'unnamed'}): "outputs" must be an object`); | |
| } |
📍 Affects 2 files
worker/src/index.js#L589-L595(this comment)scripts/validate.js#L64-L70
🤖 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 `@worker/src/index.js` around lines 589 - 595, Reject arrays in both the worker
validators at worker/src/index.js lines 589-595 and the CLI validators at
scripts/validate.js lines 64-70 by adding Array.isArray checks to the existing
inputs and outputs object-validation conditions. Keep both validators identical
so they match the workflow schema and cannot diverge.
2 participants
Define structured
inputsandoutputsfields on nodes to enable data-passing and dynamic interpolation (such as referencing previous step's output values using n8n-style variables) within DAG graph-based workflows.PR created automatically by Jules for task 10442992188653594916 started by @aethelred-agent-factory
Summary by CodeRabbit
New Features
Validation
Documentation