feat(cli,ci): 'rocketride validate' subcommand (Python + TS) and validate-pipes GitHub Action#1573
Conversation
…l handlers on Node 24+ The CommonJS interop wrapper (__importStar) only copies an object's own properties. On Node >= 24, process.on/once/emit are inherited from EventEmitter.prototype, so 'import * as process' yields a module object without them and the TypeScript CLI crashes at startup when registering SIGINT/SIGTERM handlers. The Node global 'process' carries the full prototype chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e validatePipeline expects The DAP rrext_validate handler passed the pipeline config flat to rocketlib validatePipeline, but the C++ binding requires the config under a top-level 'pipeline' key — the envelope the HTTP path (modules/pipe pipe_Validate) already builds. As a result every rrext_validate call, including SDK client.validate(), failed with ccode 40 "'pipeline' is missing or invalid" regardless of the config's actual validity (verified against the published engine image v3.3.1.35 and current source). Wrap the resolved payload exactly like pipe_Validate does. Existing handler tests updated to assert the envelope, plus a regression test pinning the wire shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eScript CLIs (rocketride-org#1572) Surfaces the existing SDK validate() (rrext_validate) in both CLIs so .pipe files can be checked from a terminal or CI without executing them: rocketride validate <files...> [--source <id>] [--json] - Files: literal paths and/or glob patterns, expanded in-CLI (recursive, sorted, deduped) so behavior is identical on shells without globbing. - Accepts either the flat pipeline config or the { "pipeline": {...} } wrapper (unwrapped exactly like client.use()); non-object JSON is rejected locally before any server call. - --json emits a single machine-readable document (per-file errors and warnings passed through verbatim from the engine, plus a summary). - Exit codes: 0 all valid; 1 at least one file invalid; 2 usage error, connection failure, or no file received a server verdict. - Connection handling, argument style, and output formatting mirror the existing subcommands in each CLI; no new dependencies. Both CLIs implement the identical contract (byte-aligned human output and JSON shape); covered by 14 pytest and 27 jest unit tests with a mocked client — no server required. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reusable composite action that gates .pipe files in CI: sets up Python, installs the rocketride CLI (fails fast with a clear message when the installed release lacks the validate subcommand; cli-version input to pin), starts the engine image in Docker, waits for the public /version endpoint, validates the repo's .pipe files (changed-only vs the PR base by default), and always removes the container. Referencable as rocketride-org/rocketride-server/.github/actions/validate-pipes@<ref>. No existing workflow files are modified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the validate synopsis, flags, exit-code semantics, and examples to the Python and TypeScript client READMEs and the CLI reference, with a pointer to the validate-pipes GitHub Action. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
nihalnihalani
left a comment
There was a problem hiding this comment.
Self-review from the author: inline notes on the four decisions most worth a reviewer's attention. Full design rationale and live-engine evidence (including the negative control against the unpatched image) are in the PR description.
| # validatePipeline expects the config under a top-level 'pipeline' | ||
| # key — same envelope pipe_Validate (modules/pipe) builds; without | ||
| # it every config fails with "'pipeline' is missing or invalid" | ||
| data = validatePipeline({'pipeline': inner}) |
There was a problem hiding this comment.
This is the load-bearing line of the PR. rocketlib validatePipeline requires the config under a top-level 'pipeline' key; the HTTP path (modules/pipe/pipe_validate.py) already builds this envelope, but the DAP handler passed the config flat — so every rrext_validate call (including SDK client.validate()) failed with ccode 40 "'pipeline' is missing or invalid" on every published engine.
Evidence: against the unpatched ghcr.io/rocketride-org/rocketride-engine:latest (v3.3.1.35), examples/agent-llamaindex.pipe fails with ccode 40; with only this line changed (bind-mounted into the same image), it validates clean and deliberately-broken files return real errors (Duplicate component ..., input references unknown component id: ...). The regression test below pins the wire shape so the two paths can't diverge again.
| raise ValueError(f'Invalid pipeline format in {file_path}: expected a JSON object') | ||
|
|
||
| # .pipe files wrap the config in { "pipeline": { ... } } - unwrap if present | ||
| inner = parsed.get('pipeline') |
There was a problem hiding this comment.
Wrapper handling mirrors client.use() / mixins/execution.py: a .pipe file may be the flat config or {"pipeline": {...}}, and we unwrap only when the value under pipeline is a dict — byte-identical logic to the TS CLI's loadPipelineFile so the two CLIs accept exactly the same inputs. Non-object top-level JSON is rejected locally (before any server call) with the same message string in both CLIs.
One deliberate strictness choice: files are parsed with strict json.load, not the json5-tolerant loader start uses — a CI gate should not accept files the wire format doesn't. Flagged in the docs.
| # registers it outside the standardEndpoints gate), so it works on | ||
| # every published engine image; /ping is gated and 404s on some. | ||
| deadline=$(( SECONDS + 120 )) | ||
| until curl -fsS "http://127.0.0.1:${ENGINE_PORT}/version" > /dev/null 2>&1; do |
There was a problem hiding this comment.
Health probe is /version rather than /ping deliberately: server.py registers /version unconditionally public, while /ping sits behind the standardEndpoints gate — on the published engine image it 404s even with valid auth (verified live). The Helm chart's /ping liveness probes may deserve the same scrutiny; noted in #1572.
Also worth a look two steps down: changed-only fetches the PR base SHA explicitly before git diff, so the action works with actions/checkout's default fetch-depth: 1 — no fetch-depth requirement leaks onto consumers.
| * ``` | ||
| */ | ||
|
|
||
| // NOTE: do not `import * as process from 'process'` here — the CommonJS |
There was a problem hiding this comment.
This hunk is the isolated pre-existing bug fix (first commit, fix(cli): — kept separate from the feature on our internal devil's-advocate review). import * as process compiles to an __importStar wrapper that only copies own properties; on Node >= 24 process.on/once/emit are inherited from EventEmitter.prototype, so the wrapped module lacks them and the CLI crashes at startup when registering SIGINT/SIGTERM handlers. The validate command's engine-connection cleanup relies on those handlers, which is how we hit it.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ 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 |
#Hire-Me-RocketRide
Contribution Type
Feature —
rocketride validateCLI subcommand (Python + TypeScript) and a reusable GitHub Action to gate.pipefiles in CI. Includes one server-side bug fix discovered during live verification.Closes #1572
Summary
RocketRide's wedge is that pipelines are portable, git-versioned JSON — but until now nothing could check a
.pipefile outside the IDE. Validation already existed end-to-end (rrext_validate→ C++validatePipeline) and both SDKs exposeclient.validate(); it was simply never surfaced in either CLI. This PR closes that loop:rocketride validate examples/*.pipe --jsonValidateCommand(cli/commands/validate.py) following the existing command pattern — same registration, connection flags (--uri/--apikey,ROCKETRIDE_*env fallbacks), and output style asstart/status/list.src/cli/rocketride.ts, implemented with exported pure helpers (expandFilePatterns,loadPipelineFile,buildValidateReport,validateExitCode) so the logic is unit-testable without a server.--jsonshape, verified by running both binaries side by side):{"pipeline": {...}}wrapper (unwrapped exactly likeclient.use()); non-object JSON is rejected locally before any server call;--source <id>passes through to the SDK;--jsonprints a single machine-readable document with the engine's error/warning objects verbatim;0all valid ·1≥1 file invalid ·2usage/connection error or no file received a server verdict..github/actions/validate-pipes): sets up Python, installs therocketrideCLI (fails fast with a clear message if the installed release lacksvalidate;cli-versioninput to pin), starts the engine image in Docker, waits for the public/versionendpoint, validates.pipefiles (changed-only vs the PR base by default), and always removes the container. No existing workflow files are touched.Server bug found and fixed during live verification
Running the new CLI against the published engine image (
v3.3.1.35) exposed a real bug: everyrrext_validatecall failed withccode 40 "'pipeline' is missing or invalid", regardless of the config's validity.Root cause: the DAP handler (
cmd_misc.py on_rrext_validate) passed the config flat torocketlib validatePipeline, but the C++ binding requires the config under a top-level'pipeline'key — the envelope the HTTP path (modules/pipe pipe_Validate) already builds:This one-line divergence had gone unnoticed because nothing in the repo calls
rrext_validatewith real payloads — the VS Code extension does its own client-side config checks. Fixed by mirroringpipe_Validate, existing handler tests updated to assert the envelope, plus a regression test pinning the wire shape (test_on_rrext_validate_wraps_config_in_pipeline_envelope).Design decisions (and why)
pipeline_config.cppstay the single source of truth; the CLI is a thin, honest client.json.load/JSON.parsefor.pipefiles (not the json5-tolerant loaderstartuses) so both CLIs reject exactly the same inputs. Called out in docs.exit 2= "no file received a server verdict" (all unreadable, or connection failed); mixed runs where some files validated exit1. Documented in--help, docstrings, and the READMEs so CI semantics are unambiguous./version, not/ping—/versionis registered unconditionally public inserver.py;/pingsits behind thestandardEndpointsgate and 404s on published images (verified live; the Helm chart's/pingprobes may deserve the same look, noted in feat(sdk,ci): 'rocketride validate' CLI subcommand + reusable GitHub Action to gate .pipe files in CI #1572).changed-onlyworks with defaultfetch-depth: 1— the action fetches the PR base commit itself beforegit diff.ROCKETRIDE_APIKEY), never argv; third-party actions pinned by full commit SHA.Validation
Unit (no server required)
pytest packages/client-python/tests/test_validate_cli.py→ 14 passed (glob expansion, wrapper unwrap, non-object rejection, exit codes 0/1/2,--jsonshape,--sourcepassthrough, connection failure).pytest .../test_cmd_misc.py(envelope assertions + regression test) → all passing inside the engine image (see below).tsc -p tsconfig.cli.jsonandtsc -p tsconfig.jsonclean;jest tests/validate.test.ts→ 27 passed.ruff check+ruff format --checkclean on all changed Python files;action.ymlYAML-parses, all run blocks passbash -nandshellcheck.Live end-to-end (published engine image
ghcr.io/rocketride-org/rocketride-engine, amd64 under QEMU)All run against
ghcr.io/rocketride-org/rocketride-engine:latest(v3.3.1.35, amd64 under QEMU):./engine -m pytest, the only environment withengLib): 24 passed, including the newtest_on_rrext_validate_wraps_config_in_pipeline_envelope.rocketride validate examples/agent-llamaindex.pipe→ valid, exit 0 (likewisedb_arango,butterbase-agent,xtrace-memory-agent,agent-workflow,llm-benchmark);rag-pipeline.pipewith--source webhook_1→ valid, exit 0; without--sourcethe engine rejects the request ("multiple source components") → exit 2 per the documented contract;Duplicate component agent_llamaindex_1;input references unknown component id: ghost_component_99);validate <good> <broken> --json→ exit 1,"summary": {"total": 2, "valid": 1, "invalid": 1}with per-fileccodeerrors.✗ 'pipeline' is missing or invalid(ccode 40), proving the one-line server fix is necessary and sufficient.dist/cli) against the patched engine: byte-identical JSON structure, summary, and exit codes vs the Python CLI.One honest caveat:
examples/n8n-roundtrip.pipeandexamples/document-processor.pipefail on this engine build with "references a provider with no registered service definition" (tool_n8n_1,anonymize_1) — engine-build limitations independent of this PR; CI configs asserting "all examples validate" should exclude them or use a fuller engine build.Notes for reviewers
fix(cli): use the Node process global…) is a small pre-existing bug fix isolated on request of our internal review:import * as processbreaks SIGINT/SIGTERM registration on Node ≥ 24 because the CommonJS interop wrapper drops inheritedEventEmittermembers. The validate command's cleanup path needs working signal handlers, so it is a prerequisite, not drive-by churn.rocketridePyPI release containingvalidateis published and an engine image containing thecmd_misc.pyfix is tagged — the README and the fail-fast install check both say so explicitly. Within this repo it can run from source immediately.🤖 Generated with Claude Code