fix(cloud): reject malformed executionOrder instead of silently runni… - #117
Merged
Conversation
…ng in parallel A workspace config.yaml was yaml.load'ed and straight-cast to IWorkspaceConfig, so a wrong-shaped executionOrder was never checked. The intuitive bare-list form made executionOrder an Array, .flowsOrder came back undefined, resolveSequentialFlows returned [], and every flow ran in parallel - same cost, wrong semantics, green run. The only symptom was depends_on being null on every result row. Add a zod schema as the single source of truth for the config shape (src/services/workspace-config.schema.ts) and route all three former cast sites through one validated loader, loadWorkspaceConfig: - A malformed executionOrder is now fatal (exit 1), with a message showing what was found next to the expected shape. A bare list is not valid Maestro either, so there is nothing to accept - and a warning in CI logs is exactly what got missed. - Unrecognised top-level keys warn (and are preserved, since the config is forwarded to the API as fields.workspaceConfig), catching flowOrder, a top-level continueOnFailure, tags in place of includeTags, and flowTimeout. - executionOrder on a single-file input warns instead of being dropped: planSingleFile never sequences, so it was silently ignored even when well-formed. - continueOnFailure's real default (true) now lives in the schema instead of being re-specified at three read sites. - WORKSPACE_CONFIG_KEYS is derived from the schema so isWorkspaceConfigFile's detection set can no longer drift from it. - includeTags/excludeTags scalar coercion moves from readYamlFileAsJson into the schema, so the loader is a plain YAML read and the validator is pure. Warnings go through an injected callback: cloud.ts passes logger.warn (stderr, so it survives --json), the MCP tool passes logStderr since its stdout is the JSON-RPC channel. Also fixes two test fixtures that used a tags: key the CLI never read. Verified on dev: the bare-list form now exits 1 before anything is submitted, and a well-formed executionOrder chains depends_on null -> 36962 -> 36963 across results 36962-36964. Fixes #110
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
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.
…ng in parallel
A workspace config.yaml was yaml.load'ed and straight-cast to IWorkspaceConfig, so a wrong-shaped executionOrder was never checked. The intuitive bare-list form made executionOrder an Array, .flowsOrder came back undefined, resolveSequentialFlows returned [], and every flow ran in parallel - same cost, wrong semantics, green run. The only symptom was depends_on being null on every result row.
Add a zod schema as the single source of truth for the config shape (src/services/workspace-config.schema.ts) and route all three former cast sites through one validated loader, loadWorkspaceConfig:
Warnings go through an injected callback: cloud.ts passes logger.warn (stderr, so it survives --json), the MCP tool passes logStderr since its stdout is the JSON-RPC channel.
Also fixes two test fixtures that used a tags: key the CLI never read.
Verified on dev: the bare-list form now exits 1 before anything is submitted, and a well-formed executionOrder chains depends_on null -> 36962 -> 36963 across results 36962-36964.
Fixes #110
What & why
Type of change
fix— bug fixfeat— new featureperf— performance improvementrefactor— code change that's neither a fix nor a featuredocs— documentation onlychore/ci/build/test— tooling, no user-facing change!or PR notes aBREAKING CHANGE:)Checklist
pnpm lintpassespnpm typecheckpassespnpm buildpassesCHANGELOG.md(release-please handles this)README.md/STYLE_GUIDE.mdupdated if behaviour or output changedHow to test
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.