Skip to content

validate_workflow accepts unknown step-level properties (when, retry, a typo'd relase_pipeline) — close the step object #118

Closed

Description

@dkackman

tool/endpoint: validate_workflow (MCP), POST /api/validate, python -m dw.validate — the workflow schema itself (dw/workflow_schema.json, $defs/step)

repro: validate a workflow whose step carries a property the engine does not read:

{"id": "probe", "steps": [
  {"name": "video", "when": "previous_result:judge.pass",
   "task": {"command": "gather_inputs", "arguments": {}}}
]}

Also with "retry": 3, "select": "argmax", and the typo "relase_pipeline": true in place of when.

expected: valid: false with an error that names the step and the key, in the shape the other reference errors take — steps[0] "video": unknown property "when".

actual: valid: true on every case. The run then proceeds with the property doing nothing. An agent that invents a plausible field (when, retry) or mistypes a real one gets a clean preflight and assumes semantics the engine never implements — the expensive work happens and the input silently had no effect (the same failure shape as ComfyUI #15669).


Facts that decide the policy

Verified on develop @ 32d4250:

  • additionalProperties: false is set on pipeline_configuration, chain, frame_snap and the cost entries. $defs/step has none, so a step is the one object an agent can invent control flow on and be told it validates.
  • The engine reads only the schema-listed step keys (name, for_each, seed, release_pipeline, release_models, task, pipeline, pipeline_reference, workflow, result). Closing the object cannot break anything the engine does.
  • Zero shipped workflows (workflows/**, dw/workflows/**) carry a stray step key. Closing it breaks nothing in the catalog.

Decision (Don): hard error, not a warning

Approved as filed — no proposal needed, this comment is the approval. A warning is what an agent skims on the way to run_workflow. The forward-compatibility argument does not apply: an agent validates against the server it is about to run on, and the catalog ships with the engine, so there is no older-engine-reads-newer-workflow path to protect. pipeline_configuration is already a hard error and it is the layer with the most churn.

Scope

  1. additionalProperties: false on $defs/step.
  2. Render the jsonschema additionalProperties failure as steps[N] "<name>": unknown property "<key>" everywhere errors are surfaced (dw.validate, POST /api/validate errors[], MCP validate_workflow, the pre-queue check in POST /api/jobs). The default "Additional properties are not allowed ('when' was unexpected)" is not actionable enough for an agent.
  3. Same treatment for the other open objects — pipeline, task, workflow_reference, pipeline_component, pipeline_reference — each after its own zero-stray-key sweep of the catalog, since some of those take component names as keys. result and from_pretrained_arguments are typed-open on purpose; leave them. A mistyped transformer_2 on a pipeline is the same silent-no-op class of bug.
  4. Tests: the four repro cases above, plus a baseline that every catalog workflow still validates.
  5. Docs: one line in docs/WORKFLOW_GUIDE.md's Authoring a workflow from an agent section and the matching MCP guide; one release-note line, since a stray key that validated yesterday is a 400 from POST /api/jobs tomorrow. Label breaking-change when it ships so the tester adjusts rather than filing it.

Raised by an external reviewer's forum reply; the sibling enhancement (deterministic N→1 selection) is a separate issue with a proposal.

Activity

  1. dkackman commented on Sep 13, 2026

    @dkackman
    OwnerAuthor

    Implemented as approved. Implementer agent, model opus via provider anthropic.

    Commit 1ead06d (merged to develop as 46a61d5), deployed to lem 2026-09-13 ~19:30Z. Labelled breaking-change - a stray key that validated yesterday is a hard error today.

    Scope 1-2: the step object, and the message

    additionalProperties: false on $defs/step, and the jsonschema failure is rendered as something actionable everywhere errors surface (dw.validate, POST /api/validate, MCP validate_workflow, the pre-queue check in POST /api/jobs - they all go through dw/schema.py's validate_data_all/validate_data, so it was one place).

    Your four repro cases, live on lem:

    "when": "previous_result:judge.pass"
      -> valid: false
      steps[0]: "video": unknown property "when" - the engine reads only for_each,
      name, pipeline, pipeline_reference, release_models, release_pipeline,
      result, seed, task, workflow, so anything else would be silently ignored
    

    "retry": 3, "select": "argmax" and "relase_pipeline": true likewise. Two stray keys in one step come back as one error listing both ("unknown properties ..."), not two.

    The message names the step by its name, the key, and the full set the engine reads - the last part deliberately, because "not allowed" tells an agent what is wrong and not what to write instead. Where the key is a near-miss (relase_pipeline) the correct spelling is in the list it prints.

    Scope 3: the other objects, each after its own sweep

    I swept workflows/** and dw/workflows/** (76 workflow files) per object before closing it:

    object stray keys found closed
    step none yes
    task none yes
    workflow_reference none yes
    pipeline_reference none yes
    pipeline latent_upsampler, prompt_enhancer, processor no
    pipeline_component model_name, weight_name (on ip_adapter) no

    pipeline and pipeline_component are the two you flagged as "some of those take component names as keys", and the sweep confirms it: ltx2/two-stage.json and ltx2/enhance-prompt.json carry components as keys of the pipeline object, and ip-adapter.json carries a differently-shaped component. Closing either needs those promoted to named properties first, which is a bigger change than this ticket. I've pinned the reason in a test (test_the_pipeline_object_stays_open) so a later sweep does not close them by eye - your point about a mistyped transformer_2 being the same bug class still stands, it just needs that work first. Say the word if you want it as its own issue.

    result and from_pretrained_arguments left typed-open, as you specified.

    Scope 4-5: tests and docs

    • The four repro cases, the multi-key case, a well-formed control, and one closure case per swept object.
    • Every catalog workflow still validates - and now also validates untrusted (test_example_workflow_validates_untrusted). The suite had been trusting workflows via an autouse fixture, so that baseline was weaker than it looked; this batch's other changes are why it mattered.
    • docs/WORKFLOW_GUIDE.md's Authoring a workflow from an agent section gains "A step takes only the keys the engine reads", which the MCP workflows guide serves verbatim. It says there is no when/retry/select and that the shape an author wants is a different arrangement of steps.

    Release note

    Breaking: step, task, workflow and pipeline_reference are now closed objects. A property the engine does not read - an invented when/retry, or a mistyped relase_pipeline - is a validation error naming the step and the key, where it previously validated cleanly and did nothing. No shipped workflow carried one. pipeline and pipeline_component stay open: a component's name is one of their keys.

    Proposed regression case

    regression-suite-smoke.md - it is fast, free (validation only) and fundamental, and an agent inventing a plausible step key is the failure mode this exists to catch.

    • validate_workflow on a one-step workflow with "when": "previous_result:judge.pass" on the step -> valid: false, error path steps[0], message contains unknown property "when" and the word release_pipeline (i.e. it lists what the step does take).
    • Same with "relase_pipeline": true -> valid: false (the typo case; this is the one that would otherwise silently not free the pipeline).
    • Same step with both "when" and "retry" -> exactly one error, message contains "unknown properties".
    • Control: the same workflow without the stray key -> valid: true.
  2. added
    breaking-changeInterface change the tester must account for before re-testing
    and removed on Sep 13, 2026
  3. dkackman commented on Sep 13, 2026

    @dkackman
    OwnerAuthor

    Verified over MCP. Tester agent, model opus via provider anthropic. Workspace qa-ep7, dw 0.4.0-beta.3.

    All three properties from the repro are refused, in one error, with the allowed set spelled out:

    validate_workflow(workflow = { "id": "f118a", "steps": [ { "name": "probe",
      "when": "always", "retry": 3, "relase_pipeline": true,
      "task": {...}, "result": {...} } ] })
    
    valid: false
    steps[0]: "probe": unknown properties "relase_pipeline", "retry", "when" - the
      engine reads only for_each, name, pipeline, pipeline_reference,
      release_models, release_pipeline, result, seed, task, workflow, so anything
      else would be silently ignored
    

    Three things I want to record as deliberately good rather than incidental, because they are what makes this usable:

    1. All three at once, not first-one-wins — a workflow with several typos is one round trip.
    2. The step is named ("probe"), so the path plus the name locates it in a long file.
    3. The legal set is listed, which turns relase_pipeline into a self-answering error.

    get_schema agrees: the step definition now carries additionalProperties: false with a $comment saying why, and the same closure is on task, pipeline_reference and workflow_reference. The task one I confirmed separately — an object with name/parameters instead of command/arguments comes back as "stable_diffusion": unknown properties "name", "parameters" - the engine reads only arguments, command, inputs.

    On the breaking-change label

    Adjusting nothing on my side. I re-validated the inline workflows I use as fixtures and none carried an unknown step key, which is the outcome you would expect — a key the engine never read was never doing anything. I am treating the label as correct-but-harmless rather than something that changed my calls, and did not file the stricter behaviour as a bug.

    Not fixed by this, filed separately

    Three sibling objects are still open, so the same typo class survives one level down — pipeline, result, and the top-level workflow object all accept unknown properties silently. Details in the new issue; not held against this one, which asked for the step object and delivered it.

    regression-suite-smoke.md gets a case for this: it is fast, free, fundamental, and a silent regression in it would put every mistyped key back to being a no-op.

  4. added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed on Sep 13, 2026
  5. added a commit that references this issue on Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    breaking-changeInterface change the tester must account for before re-testingbugSomething isn't workingowner:testerTester's turn to actstatus:verifiedTester confirmed the fix via a real MCP call

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions