feat(nodes): add n8n workflow-automation node#1231
Open
kgarg2468 wants to merge 4 commits into
Open
Conversation
Add `tool_n8n`, a dual node (pipeline step + agent tool) that triggers self-hosted n8n workflows and consumes their results. Mirrors tool_github (multi-fn REST + sibling client module) and db_postgres (dual classType). - RR->n8n: webhook trigger -- sync (Respond-to-Webhook) or async polling of executions via a correlation id; public REST API to list/inspect workflows and executions; activate/deactivate gated by a read-only toggle. - Rich I/O: simple or structured payloads (documents + metadata); files/binary both ways via multipart (image/audio/video lanes); table output lane. - Safety/DX: SSRF containment (agent never picks the host); none/header/basic/ bearer/JWT webhook auth; TLS-verify toggle; deploy-aware reachability preflight (host.docker.internal hint); 404->activate and ack->Respond-node guidance; configurable sync/async timeouts; n8n execution deep-links. - n8n->RR + round-trips documented (docs/README-n8n.md) with importable templates under examples/n8n/ (fan-out, agent, round-trip, dispatcher). Verified: ruff clean, 283 unit + contract tests, 15/15 live e2e against a real n8n (sync / async / sequential / round-trip / binary / non-webhook reach). Closes rocketride-org#1230 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kgarg2468
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
Contributor
📝 WalkthroughWalkthroughAdds a dual-mode n8n integration (tool_n8n): HTTP client, IGlobal config, IInstance tool+pipeline faces (SSRF guards, sync/async polling, binary handling), service/schema and package init, runnable examples/docs, requirements/service wiring, and comprehensive unit tests. Changesn8n workflow-automation integration node
Sequence Diagram(s)sequenceDiagram
participant RocketRide as RocketRide
participant Webhook as n8n Webhook
participant API as n8n API
RocketRide->>Webhook: POST workflow payload
alt Sync response
Webhook-->>RocketRide: Return webhook result
else Async start
Webhook-->>RocketRide: Return started acknowledgment
RocketRide->>API: Poll executions by workflow and correlation id
API-->>RocketRide: Return execution result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID. Do not edit or delete. |
The pipe-file/services-json API-key rules allowlist `${VAR}` placeholders,
but the regex `[A-Z_]+` excluded digits — so `${ROCKETRIDE_N8N_KEY}` and
`${ROCKETRIDE_N8N_URL}` (the `8` in n8n) were flagged as secrets while
digit-free names like `${ROCKETRIDE_OPENAI_KEY}` were not. Widen the
allowlist to `[A-Z0-9_]+` so digit-containing env-var names are recognized.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@docs/README-n8n.md`:
- Around line 56-61: Update the fenced code blocks in README-n8n.md to include a
language identifier to satisfy markdownlint MD040: change the opening ``` fences
that wrap the ASCII diagrams (the two blocks shown around the n8n dispatcher /
RR pipeline diagrams) to use ```text so both diagram code fences start with
```text; ensure you update both occurrences (the block at lines ~56-61 and the
other at ~91-97) and leave the diagram content unchanged.
In `@examples/README.md`:
- Around line 88-90: The fenced code block in the examples README is missing a
language tag (MD040); update the fence for the snippet `webhook -> tool_n8n
(triggers n8n workflow "rocketride-demo") -> response` to use a language tag
`text` so the block starts with ```text and ends with ```, ensuring the linter
no longer flags MD040.
In `@nodes/src/nodes/tool_n8n/IInstance.py`:
- Around line 103-115: The _safe_path function currently blocks URLs but allows
path traversal, dot segments, and query/fragment syntax; update _safe_path to
also reject any path containing '?' or '#' and to validate each '/'-separated
segment (no empty segments, no '.' or '..', and only allow a safe charset like
alphanumerics, hyphen, underscore, and maybe dot if desired) so only plain
webhook path segments are accepted before passing into trigger_webhook; ensure
you keep the existing checks (scheme, whitespace) and return the normalized path
without leading slashes.
In `@nodes/src/nodes/tool_n8n/n8n_client.py`:
- Around line 319-321: The timestamp comparison currently compares strings;
instead parse both started_after and summary.get('startedAt') into
timezone-aware datetimes before comparing to avoid lexicographic errors. In the
block that checks started_after (the variables started_after and
summary.get('startedAt') in the if condition), convert summary.get('startedAt')
(handle trailing "Z" or other ISO variants) and started_after to datetime
objects (e.g., using dateutil.parser.isoparse or normalizing "Z" to "+00:00" and
using datetime.fromisoformat) and then perform the comparison; keep the
checked.add(eid) and continue behavior the same when the parsed summary time is
earlier than started_after.
In `@nodes/src/nodes/tool_n8n/README.md`:
- Around line 57-67: Update the README configuration table to match the node
schema in nodes/src/nodes/tool_n8n/services.json: add a "Sync timeout" row (show
default and description) and expand the "Webhook auth" row to list supported
values including bearer and jwt (not just none/header/basic), and verify default
values and descriptions for "Async timeout", "Webhook auth", "Read-only mode",
and "Verify TLS certificate" match the corresponding keys in services.json
(e.g., Webhook auth, syncTimeout, asyncTimeout, readOnly) so the table reflects
the actual contract users configure.
In `@nodes/src/nodes/tool_n8n/requirements.txt`:
- Line 1: The requirements file only pins requests (requests>=2.34.2) and does
not constrain idna, allowing a vulnerable transitive idna to be resolved; add a
pinned safe idna version (the patched fixed release) by adding an explicit
idna==<fixed_version> (or idna>=<fixed_version> if you prefer a floor) either
into the shared nodes/src/nodes/requirements.txt (preferred) or directly into
nodes/src/nodes/tool_n8n/requirements.txt alongside the existing
requests>=2.34.2 entry so the node can no longer resolve the vulnerable idna.
In `@nodes/src/nodes/tool_n8n/services.json`:
- Around line 18-20: The services metadata currently lists outputs for "image",
"audio", and "video" as ["image","text","answers"] but omits "table", which
prevents binary-input pipelines from wiring IInstance.closing() structured
outputs; update the entries for "image", "audio", and "video" in
nodes/src/nodes/tool_n8n/services.json to include "table" alongside the existing
outputs so the binary routes advertise the same "table" lane as text/documents
and can accept structured dict/list results from IInstance.closing().
- Around line 181-193: The smoke test "test" case is missing required workflow
config so IInstance.closing() exits immediately; update the test case in the
"test" block to supply a valid workflow (or switch the "profiles" to one that
defines tool_n8n.workflow) so the harness has a configured workflow before
running — reference the "test" object, the "profiles": ["default"] entry, and
the tool_n8n.workflow setting and ensure the case includes the workflow ID or
uses a profile that sets it so the harness proceeds to call n8n when
ROCKETRIDE_N8N_KEY is present.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0062e832-546d-428b-b06e-b4634b94a878
⛔ Files ignored due to path filters (1)
nodes/src/nodes/tool_n8n/n8n.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
docs/README-n8n.mdexamples/README.mdexamples/n8n-call-rocketride.workflow.jsonexamples/n8n-roundtrip.pipeexamples/n8n/n8n-agent.pipeexamples/n8n/n8n-dispatch.workflow.jsonexamples/n8n/n8n-fanout.pipeexamples/n8n/n8n-roundtrip-target.pipeexamples/n8n/n8n-roundtrip.pipenodes/src/nodes/tool_n8n/IGlobal.pynodes/src/nodes/tool_n8n/IInstance.pynodes/src/nodes/tool_n8n/README.mdnodes/src/nodes/tool_n8n/__init__.pynodes/src/nodes/tool_n8n/n8n_client.pynodes/src/nodes/tool_n8n/requirements.txtnodes/src/nodes/tool_n8n/services.jsonnodes/test/test_tool_n8n.py
Resolve the 9 actionable items from the PR rocketride-org#1231 review: - SSRF: _safe_path rejects path traversal, query/fragment, and backslashes - Normalize tool-face results (binary -> descriptor, JSON scalars -> string) to the advertised object/array/string/null schema - Compare async execution timestamps as tz-aware datetimes, not strings - Pin idna>=3.10 (GHSA-65pc-fj4g-8rjx) in node + shared requirements - Advertise the table lane on image/audio/video routes - Smoke test requires ROCKETRIDE_N8N_WORKFLOW so a workflow is configured - Sync the README config table (Sync timeout; bearer/jwt webhook auth) - MD040: add a language to the README/example fenced diagrams - Extend the unit suite (path guard, result normalization, timestamp compare) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
nodes/src/nodes/tool_n8n/README.md (1)
28-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe lane table understates the non-binary outputs this node can emit.
closing()writes successful non-binary results totext,answers, anddocumentswhenever those listeners are connected, plustablefor dict/list results. The current rows omitdocumentsfortext/questions, and omitanswersfordocuments, so readers can miss supported downstream wiring.Suggested doc update
-| `text` | `text`, `answers`, `table` | Sends the text to the configured workflow; emits the result | -| `questions` | `answers`, `text`, `table` | Sends the question text to the workflow; emits the result | -| `documents` | `documents`, `text`, `table` | Sends the documents to the workflow; emits the result | +| `text` | `text`, `answers`, `documents`, `table` | Sends the text to the configured workflow; emits the result | +| `questions` | `text`, `answers`, `documents`, `table` | Sends the question text to the workflow; emits the result | +| `documents` | `text`, `answers`, `documents`, `table` | Sends the documents to the workflow; emits the result |🤖 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 `@nodes/src/nodes/tool_n8n/README.md` around lines 28 - 33, The lane table is missing some non-binary outputs emitted by closing(): update the table rows so that the `text` and `questions` input lanes list outputs `text`, `answers`, `documents`, `table` and the `documents` input lane lists `documents`, `text`, `answers`, `table`; ensure the `image`/`audio`/`video` row still lists binary and `text` outputs and that `table` remains included for dict/list results to reflect the actual emissions by closing().nodes/test/test_tool_n8n.py (1)
99-110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe bootstrap still leaks stubbed dependencies through the cached
nodes.tool_n8n.*modules.Popping
rocketlib/requests/ai.common.*fromsys.modulesis not enough here:nodes.tool_n8n.n8n_clientandnodes.tool_n8n.IInstancestay cached with those MagicMock-backed imports already bound. Any later test importing these modules in the same pytest session will reuse the stubbed module objects, so suite behavior becomes order-dependent.Suggested fix
client = importlib.import_module('nodes.tool_n8n.n8n_client') instance_mod = importlib.import_module('nodes.tool_n8n.IInstance') # Drop the stubs we injected so they never leak into the shared pytest session. for _name in _added_stubs: sys.modules.pop(_name, None) +for _name in ('nodes.tool_n8n.n8n_client', 'nodes.tool_n8n.IInstance'): + sys.modules.pop(_name, None)This keeps the local
client/instance_modreferences usable in this file while letting later imports load fresh modules with real dependencies.🤖 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 `@nodes/test/test_tool_n8n.py` around lines 99 - 110, The test imports modules while stubbed dependencies are present, leaving nodes.tool_n8n.n8n_client and nodes.tool_n8n.IInstance cached with MagicMock-backed imports and causing later tests to reuse the stubbed module objects; after creating local references client and instance_mod, remove those module entries from sys.modules (e.g., sys.modules.pop('nodes.tool_n8n.n8n_client', None) and sys.modules.pop('nodes.tool_n8n.IInstance', None)) so the local variables remain usable but subsequent imports will load fresh real modules; keep the existing stub cleanup loop for _added_stubs and ensure this pop happens after assigning client and instance_mod.nodes/src/nodes/tool_n8n/IInstance.py (1)
472-488:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
payload_mode='simple'is ignored as soon as a file is attached.
_build_payload()preserves the simple-mode contract by sending{'data': ...}, but_build_multipart()always switches totext/documents. That means the same workflow sees a different request shape depending on whether binary is present, which breaks back-compat for simple-mode webhooks and contradicts the documented payload contract.Suggested fix
def _build_multipart(self): """Build (text fields, files) for a multipart POST when binary input is present.""" fields: Dict[str, Any] = {} text = ''.join(self._text_parts) - if text: - fields['text'] = text - if self._documents: - fields['documents'] = json.dumps(self._documents, default=str) + if self.IGlobal.payload_mode == 'structured': + if text: + fields['text'] = text + if self._documents: + fields['documents'] = json.dumps(self._documents, default=str) + else: + data = text + ''.join(d.get('content', '') for d in self._documents) + if data: + fields['data'] = data files: Dict[str, Any] = {} counters: Dict[str, int] = {} for item in self._binary: kind = item['kind']🤖 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 `@nodes/src/nodes/tool_n8n/IInstance.py` around lines 472 - 488, The multipart builder currently always emits fields 'text'/'documents', breaking payload_mode='simple'; update _build_multipart to respect self.payload_mode (or payload_mode attribute used by _build_payload) and when it's 'simple' build the same simple-mode shape by setting fields['data'] to the combined text or JSON-serialized documents (instead of 'text'/'documents'), while still attaching files into files dict the same way; reference _build_multipart, _build_payload, self._text_parts, self._documents and self._binary when making the change.
🤖 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.
Outside diff comments:
In `@nodes/src/nodes/tool_n8n/IInstance.py`:
- Around line 472-488: The multipart builder currently always emits fields
'text'/'documents', breaking payload_mode='simple'; update _build_multipart to
respect self.payload_mode (or payload_mode attribute used by _build_payload) and
when it's 'simple' build the same simple-mode shape by setting fields['data'] to
the combined text or JSON-serialized documents (instead of 'text'/'documents'),
while still attaching files into files dict the same way; reference
_build_multipart, _build_payload, self._text_parts, self._documents and
self._binary when making the change.
In `@nodes/src/nodes/tool_n8n/README.md`:
- Around line 28-33: The lane table is missing some non-binary outputs emitted
by closing(): update the table rows so that the `text` and `questions` input
lanes list outputs `text`, `answers`, `documents`, `table` and the `documents`
input lane lists `documents`, `text`, `answers`, `table`; ensure the
`image`/`audio`/`video` row still lists binary and `text` outputs and that
`table` remains included for dict/list results to reflect the actual emissions
by closing().
In `@nodes/test/test_tool_n8n.py`:
- Around line 99-110: The test imports modules while stubbed dependencies are
present, leaving nodes.tool_n8n.n8n_client and nodes.tool_n8n.IInstance cached
with MagicMock-backed imports and causing later tests to reuse the stubbed
module objects; after creating local references client and instance_mod, remove
those module entries from sys.modules (e.g.,
sys.modules.pop('nodes.tool_n8n.n8n_client', None) and
sys.modules.pop('nodes.tool_n8n.IInstance', None)) so the local variables remain
usable but subsequent imports will load fresh real modules; keep the existing
stub cleanup loop for _added_stubs and ensure this pop happens after assigning
client and instance_mod.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 416753b1-0928-482b-ae81-975dea9261f9
📒 Files selected for processing (9)
docs/README-n8n.mdexamples/README.mdnodes/src/nodes/requirements.txtnodes/src/nodes/tool_n8n/IInstance.pynodes/src/nodes/tool_n8n/README.mdnodes/src/nodes/tool_n8n/n8n_client.pynodes/src/nodes/tool_n8n/requirements.txtnodes/src/nodes/tool_n8n/services.jsonnodes/test/test_tool_n8n.py
# Conflicts: # examples/README.md
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
examples/README.md (1)
106-113:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to this fenced block.
This snippet will still trigger
MD040; mark it astextso the README stays lint-clean. citeturn0search0Suggested patch
-``` +```text chat -> LlamaIndex agent -> response | +-----+-----+ @@ -``` +```🤖 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 `@examples/README.md` around lines 106 - 113, The fenced code block containing the ASCII diagram "chat -> LlamaIndex agent -> response" should include a language tag to satisfy MD040; update the opening fence from ``` to ```text in the README examples block so the diagram is marked as plain text (ensure only the opening backticks are changed for the block that contains the ASCII art).Source: Linters/SAST tools
🤖 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.
Duplicate comments:
In `@examples/README.md`:
- Around line 106-113: The fenced code block containing the ASCII diagram "chat
-> LlamaIndex agent -> response" should include a language tag to satisfy MD040;
update the opening fence from ``` to ```text in the README examples block so the
diagram is marked as plain text (ensure only the opening backticks are changed
for the block that contains the ASCII art).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07d32de4-6019-4185-9413-4df0197d4b4f
📒 Files selected for processing (1)
examples/README.md
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
1 participant
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
tool_n8n— a dual node (pipeline step and agent tool, likedb_postgres) that triggers self-hosted n8n workflows from a RocketRide pipeline or agent and consumes their results (sync or async).Type
feature (new pipeline + agent-tool node) + docs + tests
Testing
nodes/test/test_tool_n8n.pyruffclean; 283 unit + contract tests pass; 15/15 live e2e against a real n8n (sync, async, sequential, round-trip, binary send/receive, non-webhook reach)./builder testpasses — couldn't run the C++ builder locally (toolchain not installed); the identicaltest_contracts.pypasses via pytest; CI runs the wrapperChecklist
docs/README-n8n.md+ nodeREADME.mdLinked Issue
Closes #1230
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Examples
Tests
Chores