Skip to content

feat(server): add POST /api/v1/validate endpoint for pipeline dry-run#335

Merged
streamer45 merged 7 commits into
mainfrom
devin/1776545130-server-validate-endpoint
Apr 19, 2026
Merged

feat(server): add POST /api/v1/validate endpoint for pipeline dry-run#335
streamer45 merged 7 commits into
mainfrom
devin/1776545130-server-validate-endpoint

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor

@staging-devin-ai-integration staging-devin-ai-integration Bot commented Apr 18, 2026

Summary

Adds a new POST /api/v1/validate endpoint for stateless pipeline YAML validation without creating a session.

Request: { "yaml": "<pipeline YAML>", "mode": "dynamic"|"oneshot" (optional) }
Response: { "valid": bool, "errors": [...], "warnings": [...], "graph": { nodes, connections } }

Validation checks: YAML parsing → compilation → empty-pipeline rejection → node kind resolution (registry + synthetics) → per-node permission filtering (synthetics bypass) → param schema validation (unknown + missing required params) → mode-specific checks (dynamic rejects synthetics) → connection validation (pin existence + type compatibility) → file-path security (delegates to existing helpers).

Key design decisions:

  • PipelineMode enum with #[serde(rename_all = "lowercase")] rejects typos at the boundary
  • is_synthetic_kind() backed by LazyLock over synthetic_node_definitions() — prevents drift, avoids rebuilding
  • find_output_pin/find_input_pin prefer exact-name match over dynamic-prefix to avoid ambiguity
  • File-path security reuses existing validate_file_*_paths helpers via collect_file_path_errors
  • DiagnosticKind enum: parse, schema, connection, permission, security
  • Graph always returned (even on failure) so the UI can highlight bad nodes
  • Dynamic pin matching uses shared PinCardinality::is_dynamic_pin_match() from streamkit_core

Files changed:

File Change
crates/core/src/pins.rs Add PinCardinality::is_dynamic_pin_match()
crates/engine/src/dynamic_actor.rs Use shared is_dynamic_pin_match instead of local copy
apps/skit/src/server/mod.rs New endpoint, helpers, 12 unit tests

Review rounds addressed: Round 1 (dynamic-pin drift, synthetic perms, file-path security) → Round 2 (dead code, reuse file-path helpers, derive synthetics, required-param check, mode field, single-pass) → Round 3 (LazyLock, typed enum, collapsed if-let, pin priority, type-mismatch/passthrough/dynamic-mode tests).

Review & Testing Checklist for Human

  • Mode field boundary: { "yaml": "...", "mode": "dynamick" } should fail deserialization (422); "dynamic" with synthetic nodes should produce a schema error
  • File-path security: core::file_reader { path: "../../etc/passwd" } should produce a security diagnostic (not valid: true)
  • Type mismatch: connect a Text-producing node to a Binary-accepting node and verify a connection diagnostic is returned
  • Required param warning: use a node with required params in its schema but omit them — should get a warning
  • Dynamic pin matching parity: verify PinCardinality::is_dynamic_pin_match behaves identically to the old engine-local version
  • Recommended test plan: curl -X POST http://localhost:4545/api/v1/validate -H 'Content-Type: application/json' -d '{"yaml": "<pipeline>"}'

Notes

  • connected_input_pins dead code removed entirely (was leftover after PinCardinality::One warning removal)
  • app_error_message helper exists because AppError does not implement Display (only IntoResponse)
  • No HTTP-level integration test yet — handler wiring (auth gate, body limit, JSON shape) is only exercised indirectly via unit tests

Link to Devin session: https://staging.itsdev.in/sessions/dd617704a71449a4abf2c397268172f9
Requested by: @streamer45


Open in Devin Review (Staging)

Add a new HTTP endpoint that validates pipeline YAML without creating
a session or instantiating any nodes. The endpoint performs:

- YAML parsing and compilation to internal Pipeline format
- Node kind validation against the NodeRegistry
- Parameter schema validation (unknown keys reported as warnings)
- Connection validation: source/destination node existence,
  pin existence (with dynamic pin support), and pin type
  compatibility using can_connect_any()
- Required pin check: warns about unconnected One-cardinality
  input pins
- Returns the parsed graph structure on success for CLI rendering

Gated behind the list_nodes permission, consistent with
list_node_definitions_handler.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

staging-devin-ai-integration[bot]

This comment was marked as resolved.

Add synthetic_node_definitions() helper that provides NodeDefinitions
for streamkit::http_input and streamkit::http_output — virtual nodes
that exist outside the NodeRegistry.  The validate handler now falls
back to these definitions so oneshot pipelines no longer produce
spurious 'Unknown node kind' errors.

Also add unit tests covering:
- Synthetic node recognition
- Unknown node kind detection
- Bad pin name detection via connection validation
- Valid oneshot pipeline end-to-end validation

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
staging-devin-ai-integration[bot]

This comment was marked as resolved.

…rmission checks

- Extract synthetic_node_definitions() as the single source of truth
  for streamkit::http_input and streamkit::http_output definitions,
  now used by both list_node_definitions_handler and validate_nodes.

- Add per-node permission filtering to validate_nodes: checks
  is_node_allowed() and is_plugin_allowed() for each node, matching
  the patterns in list_node_definitions_handler and
  create_session_handler.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
staging-devin-ai-integration[bot]

This comment was marked as resolved.

streamkit-devin and others added 4 commits April 19, 2026 10:36
- Share is_dynamic_pin_match via PinCardinality in streamkit_core
  (eliminates starts_with drift vs engine)
- Add is_synthetic_kind helper; skip per-node permission checks
  for synthetic nodes (matches oneshot handler behavior)
- Add file-path security checks (file_reader, file_writer, script)
- Remove PinCardinality::One unconnected-pin warning (false positive)
- Gate on create_sessions instead of list_nodes; return (StatusCode, String)
- Reject empty pipelines (matches create_session_handler)
- Replace error_type string with DiagnosticKind enum
- Always return graph (even on failure) so UI can highlight bad nodes
- Add DefaultBodyLimit to the validate route
- Use node-based connection IDs instead of index-based
- Use debug! instead of info! for user-input errors
- Add tests: synthetic permission bypass, restricted perms denial,
  empty pipeline rejection

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Remove dead connected_input_pins (was populated but never read)
- Reuse existing validate_file_reader_paths/writer/script helpers via
  collect_file_path_errors (prevents divergence as those helpers evolve)
- Derive is_synthetic_kind from synthetic_node_definitions() (single source)
- Add required-param check: warn when param_schema.required[] params missing
- Add optional 'mode' field on ValidateRequest ("dynamic"|"oneshot");
  rejects synthetic nodes in dynamic mode matching create_session_handler
- Merge validate_nodes into single pass (resolve + params together)
- Add app_error_message helper for AppError -> String conversion

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- is_synthetic_kind: use LazyLock<Vec<String>> instead of rebuilding
  synthetic_node_definitions() on every call
- ValidateRequest.mode: replace Option<String> with typed PipelineMode
  enum (#[serde(rename_all = "lowercase")]) to reject typos at the
  deserialization boundary
- Collapse nested if-let + if-!is_synthetic into
  perms.filter(|_| !is_synthetic_kind(..))
- find_output_pin/find_input_pin: prefer exact-name match over
  dynamic-prefix match to avoid ambiguity
- app_error_message: document why .to_string() is not used (AppError
  does not implement Display)

Tests added:
- type_mismatch_reported: incompatible pin types produce a diagnostic
- passthrough_source_skips_type_check: Passthrough output skips check
- passthrough_destination_skips_type_check: Passthrough input skips check
- dynamic_mode_rejects_synthetic_nodes: synthetic nodes rejected in
  dynamic mode
- oneshot_mode_accepts_synthetic_nodes: synthetic nodes accepted in
  oneshot mode

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Extract the dynamic-mode synthetic-node rejection into a shared
check_mode(pipeline, mode, errors) helper called by both the handler
and the unit tests, so the tests exercise production logic instead of
an inlined simulation.

Also adds a no_mode_accepts_synthetic_nodes test for the None case.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Copy link
Copy Markdown
Contributor Author

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review (Staging)
Debug

Playground

Comment on lines +716 to +737
// Synthetic oneshot nodes bypass per-node permission checks,
// matching the oneshot handler which never filters them.
if let Some(perms) = perms.filter(|_| !is_synthetic_kind(&node.kind)) {
if !perms.is_node_allowed(&node.kind) {
errors.push(ValidateDiagnostic {
kind: DiagnosticKind::Permission,
message: format!("Permission denied: node kind '{}' not allowed", node.kind),
node_id: Some(node_id.clone()),
connection_id: None,
});
continue;
}
if node.kind.starts_with("plugin::") && !perms.is_plugin_allowed(&node.kind) {
errors.push(ValidateDiagnostic {
kind: DiagnosticKind::Permission,
message: format!("Permission denied: plugin '{}' not allowed", node.kind),
node_id: Some(node_id.clone()),
connection_id: None,
});
continue;
}
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Synthetic nodes bypass permission checks in validate but not in list_node_definitions

In validate_nodes at line 718, synthetic nodes (streamkit::http_input/http_output) bypass ALL permission checks via perms.filter(|_| !is_synthetic_kind(&node.kind)). However, in list_node_definitions_handler at lines 499-507, the same synthetic nodes ARE subject to is_node_allowed filtering. This means a user role whose allowed_nodes doesn't glob-match streamkit::* would not see synthetic nodes in the listing but would have them accepted by the validate endpoint. The comment says this matches "the oneshot handler which never filters them," so this appears intentional — but it creates a UX inconsistency where nodes invisible in the listing pass validation. Worth confirming this is the desired behavior.

Open in Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional — discussed in review round 1. The oneshot handler (run_oneshot_pipeline_handler) never runs is_node_allowed on synthetic nodes, so the validate endpoint matches that behaviour. The listing inconsistency is a real UX gap but lives in list_node_definitions_handler, not here — it should probably also expose synthetics unconditionally. Happy to file a tracking issue if desired.

@streamer45 streamer45 merged commit 9502f9e into main Apr 19, 2026
16 checks passed
@streamer45 streamer45 deleted the devin/1776545130-server-validate-endpoint branch April 19, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants