Repository navigation
fix(workflow): validate declared input and output schemas at publish and run boundaries (issue #274) - #359
fix(workflow): validate declared input and output schemas at publish and run boundaries (issue #274)#359konlue wants to merge 4 commits into
Conversation
|
|
f19c3d8 to
89b150f
Compare
Meta-schema validation at publish and build timeFixed. Regression coverage
VerificationRebased onto the current |
|
|
…and run boundaries (issue agentic-ai-java#274) - WorkflowDefinitionValidator: parse non-blank inputSchema/outputSchema with the networknt draft 2020-12 registry; blank rejection and no-schema behavior unchanged - WorkflowAgent: compile declared schemas at construction; validate request inputs before the first step (no tool or agent side effects); validate the output snapshot on the shared completion path before delivering COMPLETED; failures carry AgentResult.OutputValidationIssue and terminate the stream with WorkflowValidationException - ExecutionResult.continueWith: keep the suspension-point snapshot so a suspended outcome can be built (pre-existing NPE, untested path) - boundaries: resumes do not re-validate inputs; subflow input schemas are a publish-time contract, not an extra gate inside the parent run - tests: WorkflowSchemaContractTest, 10 cases (red 5F+2E before, green after)
Self-review of the agentic-ai-java#274 branch found that resuming a suspended workflow invoked the wrapped agent continuation twice: continueWith already resumed the continuation, and the composed step lambdas of executeSteps and Loop resumed it again. A real agent would reject the second call with the same resume token. The composed step now receives the resumed result through a ContinuedStep callback, and the resume tests assert a single invocation. Also fixes the indentation of the runSafely/run block (format:apply).
…ema at publish time (issue agentic-ai-java#274)
…s (issue agentic-ai-java#274 review) Resuming a suspended workflow step that suspends again returned the new suspension with a bare continuation: the composed step that continues the remaining steps was dropped, so the run completed without ever executing what followed. executeSteps and Loop now recompose their continue-after step onto every repeated suspension (continueAfter / continueLoopAfter), and two regression tests drive a child that suspends twice — the following step and the remaining loop iterations both run after the second resume.
89b150f to
95e2586
Compare
|
@yuluo-yx Fixed and pushed (95e2586, branch rebased onto the current main first). Root cause: the composition introduced with the single-resume fix only handled one suspension. Fix: the continue-after step is now recomposed onto every repeated suspension instead of being dropped — Regression coverage: two new tests in |
Describe what this PR does / why we need it
Issue #274 reported that
WorkflowDefinitionexposesinputSchema/outputSchema, but the validator only rejected blank strings and the runner never read either field — an illegal schema could be published, a run missing a required input still executed its tools, and the outcome came back COMPLETED. This PR makes the declared schemas real contracts at both the publish and the run boundary, using the same JSON Schema capability the agent layer already ships (networknt, draft 2020-12, version managed by the root pom), so no competing validation stack is introduced and no public API changes.What is delivered
Publish and instantiation
WorkflowDefinitionValidatornow parses every non-blank declared schema with the networknt registry (draft 2020-12) and rejects malformed ones with an error naming the field and the dialect. Blank schemas keep their existing rejection; definitions without schemas keep their existing behavior untouched.Input boundary, before any side effect
On a fresh run,
WorkflowAgentvalidates the request inputs against the declared input schema before the first step executes — the issue's reproduction (missing required input, tool executed, COMPLETED) now fails with zero tool executions. The failure is aWorkflowValidationExceptioncarrying one structured issue per violating location, reusingAgentResult.OutputValidationIssue(errorCode, fieldPath, message)and the existingSCHEMA_MALFORMED/SCHEMA_VIOLATIONerror codes.Output boundary, no COMPLETED for a violating output
Before the COMPLETED outcome is delivered — on the single completion path shared by fresh runs and resumes — the output snapshot is validated against the declared output schema. A violation terminates the stream with the same structured exception, which matches the existing contract that errors terminate the stream exceptionally;
AgentOutcomeitself is unchanged.Documented boundaries
Resumes do not re-validate inputs: the continuation owns the state and a resume request often carries no inputs, so re-validating would make suspended workflows impossible to finish — output acceptance still applies on resume. Subflows share the parent run's inputs, so a nested definition's input schema is a publish-time contract for its own callers and is not applied again inside the parent run. Both boundaries are covered by tests.
Two pre-existing bugs on the resume path, fixed
The workflow module had no suspend/resume tests, and the issue's acceptance criteria ask for that coverage — writing the tests surfaced two latent bugs. First,
ExecutionResult.continueWithwrapped a suspension in a result whosedatawas null while the suspended-outcome construction reads that data, so any suspension NPE'd immediately; the wrapper now keeps the suspension-point snapshot. Second, resuming invoked the wrapped agent continuation twice (once insidecontinueWith, once again in the composed step lambda ofexecuteSteps/Loop) — a real agent rejects a second call carrying the same resume token. The composed step now receives the resumed result instead of re-resuming, and the resume tests assert that the agent is invoked exactly once per resume.Testing
New
WorkflowSchemaContractTestwith 10 cases: malformed input and output schemas rejected at publish, blank-schema regression, unchanged behavior without schemas, missing required input rejected with zero tool executions, type mismatch reporting the field path, valid input and output completing, output violation failing instead of completing, resume completing without input re-validation, resume output violation still failing, and subflow schema validation at publish without gating parent runs. Run against the unmodified code the suite fails 5F+2E (red, including the issue's reproduction); with the fix the whole argi-workflow-core module is 48/48 green, checkstyle clean.Fixes #274