Skip to content

Commit 34a3c44

Browse files
committed
fix(dynamic-workflow): address PR review findings
Add the Unicode flag to the /workflow model parser so it satisfies the repo's require-unicode-regexp rule. Assert the cleared state in the model-preference test instead of only its status message, and extend the batch propagation test to the resume and retry launch paths. Document the fallback to the calling agent's model and the unresolved-alias fallback in the tool description, not only in the reference docs.
1 parent f7f731e commit 34a3c44

4 files changed

Lines changed: 48 additions & 15 deletions

File tree

apps/pythinker-code/src/tui/commands/dynamic-workflow.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ function withWorkerModelInstruction(prompt: string, model: string | undefined):
111111

112112
/** Returns true when the input was a `model` subcommand and has been handled. */
113113
function handleModelSubcommand(host: SlashCommandHost, input: string): boolean {
114-
const match = /^model(?:\s+(.*))?$/i.exec(input);
114+
const match = /^model(?:\s+(.*))?$/iu.exec(input);
115115
if (match === null) return false;
116116

117117
const value = match[1]?.trim() ?? '';

apps/pythinker-code/test/tui/commands/dynamic-workflow.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ describe('handleDynamicWorkflowCommand', () => {
347347

348348
await handleDynamicWorkflowCommand(host, 'model deepseek-v4');
349349
expect(host.showStatus).toHaveBeenLastCalledWith('Dynamic Workflow subagents will use deepseek-v4.');
350+
expect(host.state.appState.dynamicWorkflowModel).toBe('deepseek-v4');
350351

351352
await handleDynamicWorkflowCommand(host, 'model');
352353
expect(host.showStatus).toHaveBeenLastCalledWith(
@@ -357,6 +358,7 @@ describe('handleDynamicWorkflowCommand', () => {
357358
expect(host.showStatus).toHaveBeenLastCalledWith(
358359
'Dynamic Workflow subagents now use this session model.',
359360
);
361+
expect(host.state.appState.dynamicWorkflowModel).toBeUndefined();
360362

361363
// A model subcommand must never be mistaken for a task prompt.
362364
expect(session.setDynamicWorkflowMode).not.toHaveBeenCalled();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Use DynamicWorkflow when several independent subagents should run in parallel. W
44

55
Use `resume_agent_ids` to continue subagents that already exist from earlier work, such as ones that failed: map each agent id to the prompt for that resumed subagent (usually `continue` if no extra information is needed). You may combine `resume_agent_ids` with `items` in the same call to resume existing subagents and launch new ones. Do not duplicate resumed work in `items`.
66

7-
Use `model` and `effort` to run this workflow's subagents on a different model than the one orchestrating them, such as a cheaper or faster model for mechanical work while the orchestration stays on the current model. Both apply to every subagent in the call. Omit them to use the subagent type's own settings.
7+
Use `model` and `effort` to run this workflow's subagents on a different model than the one orchestrating them, such as a cheaper or faster model for mechanical work while the orchestration stays on the current model. Both apply to every subagent in the call. Omitting either falls back to the subagent type's own setting, and then to your current setting. A `model` that is not a configured alias also falls back to your current model rather than failing the workflow.
88

99
Use enough subagents to keep the work focused and parallel. DynamicWorkflow supports up to 128 subagents, and launches are queued automatically, so it is safe to split large tasks into many clear, independent items. Workflow subagents have no automatic timeout; they run until completion, failure, or user cancellation.
1010

packages/agent-core/test/session/subagent-batch.test.ts

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -669,20 +669,51 @@ describe('SubagentBatch scheduling contract', () => {
669669
}
670670
});
671671

672-
it('carries a task model and effort through to the launcher', async () => {
673-
const { runBatch, attempts } = createMockBatchRunner();
674-
const running = runBatch([routedTask(1), routedTask(2)], { signal });
675-
await Promise.resolve();
676-
677-
// A workflow routed to a cheaper model must reach every spawned child;
678-
// dropping it here would silently run the batch on the parent's model.
679-
expect(attempts).toHaveLength(2);
680-
for (const attempt of attempts) {
681-
expect(attempt.runOptions.modelAlias).toBe('implementer-model');
682-
expect(attempt.runOptions.thinkingLevel).toBe('medium');
683-
attempt.outcome.resolve({ task: attempt.task, status: 'completed', result: 'done' });
672+
it('carries a task model and effort through spawn, resume, and retry attempts', async () => {
673+
vi.useFakeTimers();
674+
try {
675+
const { runBatch, attempts } = createMockBatchRunner();
676+
const resumeTask: QueuedSubagentTask<number> = {
677+
...routedTask(2),
678+
kind: 'resume',
679+
resumeAgentId: 'agent-2',
680+
};
681+
const running = runBatch([routedTask(1), resumeTask, routedTask(3)], { signal });
682+
void running.catch(() => {});
683+
684+
await vi.advanceTimersByTimeAsync(0);
685+
expect(attempts).toHaveLength(3);
686+
attempts.forEach((attempt) => {
687+
attempt.markReady();
688+
});
689+
690+
// Rate-limit one attempt so the batch re-launches it through the retry
691+
// path, which is the third way a child can be started. Another task has
692+
// to finish first to free the shrunken capacity, and a third has to stay
693+
// unfinished so the rate-limited one suspends instead of failing.
694+
const rateLimitedAgentId = `agent-${String(attempts[0]!.task.data)}`;
695+
attempts[0]!.outcome.resolve({ type: 'rate_limited', agentId: rateLimitedAgentId });
696+
attempts[1]!.outcome.resolve({ task: attempts[1]!.task, status: 'completed', result: 'done' });
697+
await vi.advanceTimersByTimeAsync(3000);
698+
expect(attempts).toHaveLength(4);
699+
expect(attempts[3]!.retryAgentId).toBe(rateLimitedAgentId);
700+
701+
// A workflow routed to a cheaper model must reach every child however it
702+
// was started; dropping it on any path silently runs that child on the
703+
// parent's model.
704+
for (const attempt of attempts) {
705+
expect(attempt.runOptions.modelAlias).toBe('implementer-model');
706+
expect(attempt.runOptions.thinkingLevel).toBe('medium');
707+
}
708+
709+
attempts.slice(2).forEach((attempt) => {
710+
attempt.outcome.resolve({ task: attempt.task, status: 'completed', result: 'done' });
711+
});
712+
await vi.advanceTimersByTimeAsync(0);
713+
await running;
714+
} finally {
715+
vi.useRealTimers();
684716
}
685-
await running;
686717
});
687718
});
688719

0 commit comments

Comments
 (0)