Skip to content

Commit 14f9250

Browse files
committed
fix(dynamic-workflow): stop blank items from breaking the run they were meant to save
Three defects, all fallout from dropping blank items rather than rejecting the call, and all in the scenario that change exists to handle. The note explaining the drop was prepended to the tool output, but consumers match the result document anchored at the start of that output. A run that dropped an item therefore parsed as unsupported and rendered as failed despite every subagent succeeding. The note now follows the results, and both sides pin the ordering: the producer asserts the envelope comes first, the renderer asserts it still parses with the note attached. The transcript counted blank entries when sizing the panel, so a dropped item left a row queued forever and held the header below its total. The schema still capped the raw item count, so 128 real prompts plus one blank was rejected whole -- reopening the hole at the boundary. The cap now applies to the items that survive, and going over it fails inside the tool with a readable message instead of discarding every prompt.
1 parent d159963 commit 14f9250

5 files changed

Lines changed: 99 additions & 8 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pythoughts/pythinker-code": patch
3+
---
4+
5+
Finish handling blank Dynamic Workflow items. A run that dropped one reported its results after a note explaining the drop, which made the whole result parse as unsupported and rendered a successful run as failed; the note now follows the results. A blank entry also no longer leaves a row queued forever with the header stuck below its total, and no longer pushes a full item list over the subagent cap and back into whole-call rejection.

apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,14 @@ export class DynamicWorkflowMissionControlComponent implements Component {
709709
/** Item list from the completed tool-call `items` argument. */
710710
export function dynamicWorkflowItemsFromArgs(args: Record<string, unknown>): string[] {
711711
const items = args['items'];
712-
return Array.isArray(items) ? items.map(itemLabel) : [];
712+
if (!Array.isArray(items)) return [];
713+
// Blank entries are dropped by the engine before any agent is launched, so
714+
// counting them here would leave a phantom row waiting forever and pin the
715+
// header below its total. Non-strings are kept: the engine rejects those, and
716+
// itemLabel renders them readably.
717+
return items
718+
.filter((item) => typeof item !== 'string' || item.trim().length > 0)
719+
.map(itemLabel);
713720
}
714721

715722
/**

apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,56 @@ describe('DynamicWorkflowMissionControlComponent', () => {
121121
}
122122
});
123123

124+
it('ignores blank items so no phantom row waits forever', () => {
125+
const component = createComponent();
126+
// The engine drops the blank before launching anything, so counting it here
127+
// would leave a third row queued for good and pin the header at 2/3.
128+
component.updateArgs({ items: ['Layout hierarchy', 'Interaction audit', ' '] });
129+
component.markInputComplete();
130+
register(component, 'agent-1');
131+
component.markStarted('agent-1');
132+
component.markCompleted('agent-1', 'Done one');
133+
register(component, 'agent-2');
134+
component.markStarted('agent-2');
135+
component.markCompleted('agent-2', 'Done two');
136+
137+
const output = renderText(component, 120);
138+
expect(memberLine(output, 1)).toContain('✓ DONE');
139+
expect(memberLine(output, 2)).toContain('✓ DONE');
140+
// memberRowCount also counts activity lines, so assert the row's absence.
141+
expect(() => memberLine(output, 3)).toThrow(/Missing Dynamic Workflow member 003/u);
142+
expect(aggregateLine(output)).toContain('2/2 complete');
143+
});
144+
145+
it('still parses a result that carries the dropped-items note', () => {
146+
// agent-core appends this note when it ignores blank items. It used to be
147+
// prepended, which made the envelope regex miss and rendered a successful
148+
// run as "Unsupported Dynamic Workflow result".
149+
const result = [
150+
'<dynamic_workflow_result>',
151+
'<summary>completed: 1</summary>',
152+
'<subagent outcome="completed">Layout hierarchy</subagent>',
153+
'</dynamic_workflow_result>',
154+
'Note: 1 empty item was ignored; the workflow ran without them.',
155+
].join('\n');
156+
157+
expect(dynamicWorkflowResultSummaryFromOutput(result)).toEqual({
158+
completed: 1,
159+
failed: 0,
160+
aborted: 0,
161+
parsed: true,
162+
});
163+
164+
const component = createComponent();
165+
component.updateArgs({ items: ['Layout hierarchy'] });
166+
component.markInputComplete();
167+
expect(component.applyResult(result)).toBe(true);
168+
169+
const output = renderText(component, 120);
170+
expect(memberLine(output, 1)).toContain('✓ DONE');
171+
expect(output).not.toContain('Unsupported');
172+
});
173+
124174
it('decodes escaped XML fields and preserves literal closing-tag text', () => {
125175
const result = [
126176
'<dynamic_workflow_result>',

packages/agent-core/src/tools/builtin/collaboration/dynamic-workflow.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,12 @@ export const DynamicWorkflowToolInputSchema = z
7272
`Optional prompt template for each subagent. The ${PROMPT_TEMPLATE_PLACEHOLDER} placeholder is replaced with each item value. When omitted, each item is used as a complete prompt.`,
7373
),
7474
items: z
75-
// Deliberately NOT `.min(1)` per item: a model that emits a trailing
76-
// empty string would otherwise fail argument validation, which rejects
77-
// the WHOLE call before the tool runs and costs a full re-send of every
78-
// prompt. Empty entries are dropped and reported by the tool instead.
75+
// Deliberately unconstrained per item and unbounded in length: argument
76+
// validation rejects the WHOLE call before the tool runs, so a trailing
77+
// empty string — or one blank entry pushing a full list one over the cap
78+
// — would cost a re-send of every prompt. The tool drops blanks, reports
79+
// how many, and enforces the real cap against the surviving count.
7980
.array(z.string())
80-
.max(MAX_DYNAMIC_WORKFLOW_SUBAGENTS)
8181
.optional()
8282
.describe(
8383
`Each item launches one new subagent. Items fill ${PROMPT_TEMPLATE_PLACEHOLDER} when prompt_template is provided; otherwise they are complete prompts.`,
@@ -262,7 +262,10 @@ export class DynamicWorkflowTool implements BuiltinTool<DynamicWorkflowToolInput
262262
);
263263
const { dropped } = normalizeWorkflowItems(args.items);
264264
if (dropped === 0) return rendered;
265-
return `${droppedItemsNote(dropped)}\n${rendered}`;
265+
// After the envelope, never before it: consumers match the result document
266+
// anchored at the start of the output, so a prefix makes a successful run
267+
// parse as an unsupported result.
268+
return `${rendered}\n${droppedItemsNote(dropped)}`;
266269
}
267270
}
268271

packages/agent-core/test/tools/builtin-current.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,11 @@ describe('current builtin collaboration tools', () => {
386386
expect(queued.map((task) => task.prompt)).toEqual(['Review src/a.ts', 'Review src/b.ts']);
387387
// A quietly shorter workflow must not read as one the model sized right.
388388
expect(result.output).toContain('1 empty item was ignored');
389+
// The note must not precede the envelope: consumers match the result
390+
// document anchored at the start, so a prefix renders a successful run as
391+
// an unsupported result.
392+
const outputText = typeof result.output === 'string' ? result.output : '';
393+
expect(outputText.trimStart().startsWith('<dynamic_workflow_result')).toBe(true);
389394
});
390395

391396
it('DynamicWorkflow says items were dropped when too few survive', async () => {
@@ -456,12 +461,15 @@ describe('current builtin collaboration tools', () => {
456461
items: Array.from({ length: 128 }, (_, index) => `src/${String(index + 1)}.ts`),
457462
}).success,
458463
).toBe(true);
464+
// Over the cap now passes argument validation and fails inside the tool
465+
// with a readable message. Rejecting at the schema would discard the whole
466+
// call -- including the 128 valid prompts -- over one surplus entry.
459467
expect(
460468
DynamicWorkflowToolInputSchema.safeParse({
461469
...input,
462470
items: Array.from({ length: 129 }, (_, index) => `src/${String(index + 1)}.ts`),
463471
}).success,
464-
).toBe(false);
472+
).toBe(true);
465473
expect(tool.parameters).toMatchObject({
466474
type: 'object',
467475
properties: {
@@ -856,6 +864,24 @@ describe('current builtin collaboration tools', () => {
856864
expect(execution.matchesRule).toBeUndefined();
857865
});
858866

867+
it('DynamicWorkflow accepts a full item list carrying a blank entry', async () => {
868+
// 128 real prompts plus one blank used to trip the schema's raw-length cap,
869+
// which rejected the entire call -- the very hole the blank-item handling
870+
// exists to close, reopened at the boundary.
871+
const items = [...Array.from({ length: 128 }, (_, index) => `src/${String(index + 1)}.ts`), ''];
872+
const input = { description: 'Review files', prompt_template: 'Review {{item}}', items };
873+
874+
expect(DynamicWorkflowToolInputSchema.safeParse(input).success).toBe(true);
875+
876+
const host = mockSubagentHost({ runQueued: vi.fn().mockResolvedValue([]) });
877+
const tool = new DynamicWorkflowTool(host, mockDynamicWorkflowMode());
878+
const result = await executeTool(tool, context(input, 'call_dynamic_workflow'));
879+
880+
expect(result.isError).not.toBe(true);
881+
expect(host.runQueued).toHaveBeenCalledTimes(1);
882+
expect((host.runQueued.mock.calls[0]?.[0] as unknown[]).length).toBe(128);
883+
});
884+
859885
it('DynamicWorkflow rejects more than 128 subagents at execution time', async () => {
860886
const host = mockSubagentHost({ runQueued: vi.fn() });
861887
const dynamicWorkflowMode = mockDynamicWorkflowMode();

0 commit comments

Comments
 (0)