Skip to content

Commit 903a14e

Browse files
authored
fix: normalize workflow child return metadata (#1314)
1 parent c005779 commit 903a14e

4 files changed

Lines changed: 54 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
### Fixed
1616
- Add a separate classifier for model context-overflow errors. Thanks to [@srcKod](https://github.com/srcKod) for #1312.
17+
- Normalize child result metadata before workflow return persistence (#1307).
1718
- Quote only confidently identified leading Windows executable paths in acceptance verification commands. Thanks to [@srcKod](https://github.com/srcKod) for #1294.
1819

1920
### Changed

docs/workflows.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ Add `autofix` to `/parallel-review` or `/parallel-cleanup` to apply only the syn
3737

3838
All model-facing subagent execution is expressed through `workflowScript` in the `subagent` tool. Use stable keys and ordinary JavaScript for one child, sequence, and parallelism. For ordinary parallel fanout, use `await runs.all([{ key, agent, task }, ...])`; do not read `.output` from unawaited `runs.run` launches. Store a `runs.run` promise only when the script later observes it with `await`, `Promise.race`, or `Promise.all`, such as steering a live child before awaiting its result. Scripts are ordinary JavaScript statement bodies. Use an explicit `return` for a useful result:
3939

40+
Child results cross into the script as plain JSON data. Non-JSON host metadata is omitted, so use returned fields such as `runId`, `ok`, `output`, and `structuredOutput` for workflow control.
41+
4042
```js
4143
subagent({ workflowScript: `
4244
const scan = await runs.run("scan", { agent: "scout", task: "Scan the codebase" });

src/workflows/scripted-workflow.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,18 @@ function omitUndefinedWorkflowValues(value: unknown, seen = new Set<object>()):
665665
return normalized;
666666
}
667667

668+
function omitNonJsonWorkflowResultMetadata(value: unknown): unknown {
669+
const normalized = omitUndefinedWorkflowValues(value);
670+
if (!isPlainJsonObject(normalized) || !Object.hasOwn(normalized, "results")) return normalized;
671+
try {
672+
assertWorkflowJsonValue(normalized.results, "runs.run result.results");
673+
return normalized;
674+
} catch {
675+
const { results: _results, ...safeResult } = normalized;
676+
return safeResult;
677+
}
678+
}
679+
668680
export function assertWorkflowJsonValue(value: unknown, path = "value", seen = new Set<object>()): void {
669681
if (value === null || typeof value === "string" || typeof value === "boolean") return;
670682
if (typeof value === "number") {
@@ -917,10 +929,21 @@ export async function runWorkflowScript(options: RunWorkflowScriptOptions): Prom
917929
}
918930
if (message.type !== "call" || typeof message.callId !== "number" || typeof message.method !== "string" || !isRecord(message.args)) return;
919931

920-
const respond = (promise: Promise<unknown>) => {
932+
const respond = (promise: Promise<unknown>, responsePath?: string) => {
921933
void promise.then(
922934
(value) => {
923-
if (!settled) worker.postMessage({ type: "response", callId: message.callId, ok: true, value: omitUndefinedWorkflowValues(value) });
935+
if (settled) return;
936+
const normalized = responsePath ? omitNonJsonWorkflowResultMetadata(value) : omitUndefinedWorkflowValues(value);
937+
if (!responsePath) {
938+
worker.postMessage({ type: "response", callId: message.callId, ok: true, value: normalized });
939+
return;
940+
}
941+
try {
942+
assertWorkflowJsonValue(normalized, responsePath);
943+
worker.postMessage({ type: "response", callId: message.callId, ok: true, value: normalized });
944+
} catch (error) {
945+
worker.postMessage({ type: "response", callId: message.callId, ok: false, error: `${responsePath} must contain only JSON data before it can be returned from workflowScript. Return a plain projection such as { runId, ok, output }. ${error instanceof Error ? error.message : String(error)}` });
946+
}
924947
},
925948
(error: unknown) => {
926949
if (!settled) worker.postMessage({ type: "response", callId: message.callId, ok: false, error: error instanceof Error ? error.message : String(error), ...(error instanceof Error && (error as { workflowErrorKind?: unknown }).workflowErrorKind === "detached-child" ? { errorKind: "detached-child" } : {}) });
@@ -1052,7 +1075,7 @@ export async function runWorkflowScript(options: RunWorkflowScriptOptions): Prom
10521075
if (callObserved) existing.observed = true;
10531076
trace.push({ operation: "run", key, state: "reused", ...workflowStringMetadata(params) });
10541077
traceChanged();
1055-
return respond(deliver(existing.promise));
1078+
return respond(deliver(existing.promise), `runs.run('${key}') result`);
10561079
}
10571080

10581081
const startedAt = Date.now();
@@ -1126,7 +1149,7 @@ export async function runWorkflowScript(options: RunWorkflowScriptOptions): Prom
11261149
childOrder.push(key);
11271150
trace.push({ operation: "run", key, state: "started", ...workflowStringMetadata(params) });
11281151
traceChanged();
1129-
respond(deliver(promise));
1152+
respond(deliver(promise), `runs.run('${key}') result`);
11301153
});
11311154

11321155
worker.postMessage({ type: "start", script: options.script, stateEnabled: options.state !== undefined });

test/unit/scripted-workflow.test.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -468,18 +468,30 @@ describe("scripted workflow runtime", () => {
468468
assert.deepEqual(result.value, [{ key: "review", output: "completed", values: [null] }]);
469469
});
470470

471-
it("rejects non-plain child result values", async () => {
472-
await assert.rejects(
473-
runWorkflowScript({
474-
script: `return await runs.run("non-plain", { agent: "worker", task: "write output" });`,
475-
timeoutMs: 2_000,
476-
async launch(key) {
477-
return { key, ok: true, output: "Saved output.", artifactPaths: [], results: [{ metadata: new Map([["source", "worker"]]) }] };
478-
},
479-
async status(key) { return { key, ok: true, output: "ok", artifactPaths: [] }; },
480-
}),
481-
(error: unknown) => error instanceof WorkflowScriptError && /return.*plain JSON objects/i.test(error.message),
482-
);
471+
it("omits non-JSON child result metadata before returning reused runs.run results", async () => {
472+
let launches = 0;
473+
const result = await runWorkflowScript({
474+
script: `
475+
const first = await runs.run("non-plain", { agent: "worker", task: "write output" });
476+
const reused = await runs.run("non-plain", { agent: "worker", task: "write output" });
477+
return [first, reused];
478+
`,
479+
timeoutMs: 2_000,
480+
async launch(key) {
481+
launches++;
482+
return { key, ok: true, output: "Saved output.", artifactPaths: [], results: [{ metadata: new Map([["source", "worker"]]) }] };
483+
},
484+
async status(key) { return { key, ok: true, output: "ok", artifactPaths: [] }; },
485+
});
486+
487+
assert.equal(launches, 1);
488+
assert.deepEqual(result.value, [
489+
{ key: "non-plain", ok: true, output: "Saved output.", artifactPaths: [] },
490+
{ key: "non-plain", ok: true, output: "Saved output.", artifactPaths: [] },
491+
]);
492+
assert.equal((result.value as Array<{ results?: unknown }>)[0]?.results, undefined);
493+
assert.equal((result.value as Array<{ results?: unknown }>)[1]?.results, undefined);
494+
assert.ok(result.children[0]?.results?.[0] && (result.children[0].results[0] as { metadata?: unknown }).metadata instanceof Map);
483495
});
484496

485497
it("passes retained resume items and rejects agent overrides", async () => {

0 commit comments

Comments
 (0)