Skip to content

Commit 190221e

Browse files
committed
fix(cloud): reject malformed executionOrder instead of silently running 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
1 parent 57711c6 commit 190221e

8 files changed

Lines changed: 412 additions & 89 deletions

File tree

src/commands/cloud.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,9 @@ export const cloudCommand = defineCommand({
569569
excludeFlows,
570570
configFile,
571571
debug,
572+
// Not warnOut: config problems are worth surfacing even under --json,
573+
// and logger.warn writes to stderr so stdout stays parseable.
574+
warn: (m: string) => logger.warn(m),
572575
});
573576

574577
if (debug) {

src/mcp/tools/run-cloud-test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ export function registerRunCloudTest(server: McpServer): void {
151151
excludeTags: args.excludeTags ?? [],
152152
excludeFlows: args.excludeFlows,
153153
configFile: args.configFile,
154+
// stdout is the JSON-RPC channel — config warnings must go to stderr.
155+
warn: logStderr,
154156
});
155157

156158
const commonRoot = computeCommonRoot(

src/services/execution-plan.service.ts

Lines changed: 32 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -5,45 +5,12 @@ import {
55
getFlowsToRunInSequence,
66
isFlowFile,
77
isWorkspaceConfigFile,
8+
loadWorkspaceConfig,
89
processDependencies,
910
readDirectory,
1011
readTestYamlFileAsJson,
11-
readYamlFileAsJson,
1212
} from './execution-plan.utils.js';
13-
14-
/** Email notification configuration */
15-
interface INotificationsConfig {
16-
email?: {
17-
enabled?: boolean;
18-
onSuccess?: boolean;
19-
recipients?: string[];
20-
};
21-
}
22-
23-
/** Workspace configuration from config.yaml */
24-
interface IWorkspaceConfig {
25-
excludeTags?: null | string[];
26-
executionOrder?: IExecutionOrder | null;
27-
flows?: null | string[];
28-
includeTags?: null | string[];
29-
local?: ILocal | null;
30-
notifications?: INotificationsConfig;
31-
platform?: {
32-
android?: { disableAnimations?: boolean };
33-
ios?: { disableAnimations?: boolean };
34-
};
35-
}
36-
37-
/** Local execution configuration */
38-
interface ILocal {
39-
deterministicOrder: boolean | null;
40-
}
41-
42-
/** Sequential execution configuration */
43-
interface IExecutionOrder {
44-
continueOnFailure: boolean;
45-
flowsOrder: string[];
46-
}
13+
import { IWorkspaceConfig } from './workspace-config.schema.js';
4714

4815
/** Options for execution plan generation */
4916
export interface PlanOptions {
@@ -53,6 +20,12 @@ export interface PlanOptions {
5320
excludeTags?: string[];
5421
includeTags?: string[];
5522
input: string;
23+
/**
24+
* Sink for non-fatal config problems. Injected rather than imported so the
25+
* MCP server can route warnings to stderr — its stdout is the JSON-RPC
26+
* channel.
27+
*/
28+
warn?: (message: string) => void;
5629
}
5730

5831
/** Execution plan containing all flows to run with metadata and dependencies */
@@ -146,11 +119,13 @@ function filterFlowFiles(
146119
* Load workspace configuration from config.yaml/yml if present
147120
* @param input - Input directory path
148121
* @param unfilteredFlowFiles - List of discovered flow files
122+
* @param warn - Sink for non-fatal config problems
149123
* @returns Workspace configuration object (empty if no config file found)
150124
*/
151125
function getWorkspaceConfig(
152126
input: string,
153127
unfilteredFlowFiles: string[],
128+
warn: (message: string) => void,
154129
): IWorkspaceConfig {
155130
const possibleConfigPaths = new Set(
156131
[path.join(input, 'config.yaml'), path.join(input, 'config.yml')].map((p) =>
@@ -162,11 +137,7 @@ function getWorkspaceConfig(
162137
possibleConfigPaths.has(path.normalize(file)),
163138
);
164139

165-
const config = configFilePath
166-
? (readYamlFileAsJson(configFilePath) as IWorkspaceConfig)
167-
: {};
168-
169-
return config;
140+
return configFilePath ? loadWorkspaceConfig(configFilePath, warn) : {};
170141
}
171142

172143
/**
@@ -199,11 +170,13 @@ function extractDeviceCloudOverrides(
199170
/**
200171
* Generate execution plan for a single flow file
201172
* @param normalizedInput - Normalized path to the flow file
173+
* @param warn - Sink for non-fatal config problems
202174
* @param resolvedConfigFile - Optional absolute path to a custom config file
203175
* @returns Execution plan for the single file with dependencies
204176
*/
205177
async function planSingleFile(
206178
normalizedInput: string,
179+
warn: (message: string) => void,
207180
resolvedConfigFile?: string,
208181
): Promise<IExecutionPlan> {
209182
const inputBasename = path.basename(normalizedInput);
@@ -232,9 +205,17 @@ async function planSingleFile(
232205
throw new Error(`Config file does not exist: ${resolvedConfigFile}`);
233206
}
234207

235-
workspaceConfig = readYamlFileAsJson(
236-
resolvedConfigFile,
237-
) as IWorkspaceConfig;
208+
workspaceConfig = loadWorkspaceConfig(resolvedConfigFile, warn);
209+
210+
// Sequencing is resolved against a workspace's discovered flows, which a
211+
// single-file input doesn't have — so executionOrder is ignored here. Say so
212+
// rather than accepting a config that reads as if it applied (dcd-cli#110).
213+
if (workspaceConfig.executionOrder?.flowsOrder.length) {
214+
warn(
215+
`Warning: \`executionOrder\` in ${resolvedConfigFile} is ignored when a single flow file is passed.\n` +
216+
`Pass the workspace folder instead so the named flows can be discovered and sequenced.`,
217+
);
218+
}
238219
}
239220

240221
const checkedDependancies = await checkDependencies(normalizedInput);
@@ -386,6 +367,7 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
386367
excludeFlows,
387368
configFile,
388369
debug = false,
370+
warn = (message: string) => console.warn(message),
389371
} = options;
390372
const normalizedInput = path.normalize(input);
391373
const flowMetadata: Record<string, Record<string, unknown>> = {};
@@ -400,7 +382,7 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
400382
}
401383

402384
if (fs.lstatSync(normalizedInput).isFile()) {
403-
return planSingleFile(normalizedInput, resolvedConfigFile);
385+
return planSingleFile(normalizedInput, warn, resolvedConfigFile);
404386
}
405387

406388
let unfilteredFlowFiles = await readDirectory(normalizedInput, isFlowFile);
@@ -420,11 +402,13 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
420402
throw new Error(`Config file does not exist: ${resolvedConfigFile}`);
421403
}
422404

423-
workspaceConfig = readYamlFileAsJson(
424-
resolvedConfigFile,
425-
) as IWorkspaceConfig;
405+
workspaceConfig = loadWorkspaceConfig(resolvedConfigFile, warn);
426406
} else {
427-
workspaceConfig = getWorkspaceConfig(normalizedInput, unfilteredFlowFiles);
407+
workspaceConfig = getWorkspaceConfig(
408+
normalizedInput,
409+
unfilteredFlowFiles,
410+
warn,
411+
);
428412
}
429413

430414
unfilteredFlowFiles = await applyFlowGlobs(

src/services/execution-plan.utils.ts

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@ import * as yaml from 'js-yaml';
33
import * as fs from 'node:fs';
44
import * as path from 'node:path';
55

6+
import {
7+
IWorkspaceConfig,
8+
parseWorkspaceConfig,
9+
WORKSPACE_CONFIG_KEYS,
10+
} from './workspace-config.schema.js';
11+
612
const commandsThatRequireFiles = new Set(['addMedia', 'runFlow', 'runScript']);
713

814
export function getFlowsToRunInSequence(
@@ -60,22 +66,6 @@ export function isFlowFile(filePath: string): boolean {
6066
return filePath.endsWith('.yaml') || filePath.endsWith('.yml');
6167
}
6268

63-
/**
64-
* Top-level keys that only ever appear in a workspace config (see
65-
* IWorkspaceConfig in execution-plan.service.ts). Deliberately excludes keys
66-
* Maestro also allows in flow front matter — appId, name, tags, env,
67-
* onFlowStart, onFlowComplete, jsEngine.
68-
*/
69-
const WORKSPACE_CONFIG_KEYS = new Set([
70-
'excludeTags',
71-
'executionOrder',
72-
'flows',
73-
'includeTags',
74-
'local',
75-
'notifications',
76-
'platform',
77-
]);
78-
7969
/**
8070
* True when a YAML file is a workspace config rather than a runnable flow.
8171
*
@@ -112,27 +102,34 @@ export const readYamlFileAsJson = (filePath: string) => {
112102
const normalizedPath = path.normalize(filePath);
113103
const yamlText = fs.readFileSync(normalizedPath, 'utf8');
114104

115-
const result = yaml.load(yamlText);
116-
117-
// Ensure includeTags and excludeTags are always arrays if present
118-
if (result && typeof result === 'object') {
119-
if ('includeTags' in result && !Array.isArray(result.includeTags)) {
120-
result.includeTags = result.includeTags ? [result.includeTags] : [];
121-
}
122-
123-
if ('excludeTags' in result && !Array.isArray(result.excludeTags)) {
124-
result.excludeTags = result.excludeTags ? [result.excludeTags] : [];
125-
}
126-
}
127-
128-
return result;
105+
return yaml.load(yamlText);
129106
} catch (error) {
130107
throw new Error(`Error parsing YAML file ${filePath}: ${error}`, {
131108
cause: error,
132109
});
133110
}
134111
};
135112

113+
/**
114+
* Load and validate a workspace config file.
115+
*
116+
* The single chokepoint for reading a config: every caller gets a
117+
* runtime-validated object instead of an unchecked `as IWorkspaceConfig` cast.
118+
* Scalar-to-array coercion for `includeTags`/`excludeTags` lives in the schema,
119+
* so `readYamlFileAsJson` stays a plain YAML read.
120+
*
121+
* @param filePath - Path to the config file
122+
* @param warn - Sink for non-fatal problems (unrecognised keys)
123+
* @returns The validated workspace config
124+
* @throws Error if the file is unparseable or the config is invalid
125+
*/
126+
export function loadWorkspaceConfig(
127+
filePath: string,
128+
warn: (message: string) => void,
129+
): IWorkspaceConfig {
130+
return parseWorkspaceConfig(readYamlFileAsJson(filePath), { filePath, warn });
131+
}
132+
136133
export const readTestYamlFileAsJson = (filePath: string) => {
137134
try {
138135
const normalizedPath = path.normalize(filePath);

0 commit comments

Comments
 (0)