Skip to content

fix(workflow): reject phase() misuse instead of silently running no agents - #283

Merged
liuedcson merged 1 commit into
mainfrom
fix/workflow-phase-misuse
Oct 21, 2026
Merged

liuedcson merged 1 commit into
mainfrom
fix/workflow-phase-misuse

Conversation

@liuedcson

Copy link
Copy Markdown
Collaborator

Problem

phase(title) is a milestone label, but the workflow VM dropped every argument after the title. A script that wrapped its agent calls in a callback, as in phase("Review", async () => { await agent(...) }), ran zero agents. The workflow still finished with status: "completed" and an empty value. The tool description listed phase() alongside the execution primitives and never gave its signature, so the model reasonably wrote it that way.

Changes

  • New features/workflow/script-lint.ts adds a lexical pre-run check, called at the end of resolveWorkflowScript. It rejects any phase(...) call with more than one argument and names the line, for example Workflow script line 2: phase(title) takes one string .... It covers inline, scriptPath, and saved scripts, and it runs before a journal or child process exists.
    • The scanner skips comments, strings, and templates.
    • It ignores obj.phase(...), function phase(...), and method definitions.
    • It accepts a trailing comma after a single argument.
  • The VM's phase global throws the same message when it receives more than one argument. This catches calls the static check cannot see, such as const p = phase; p("x", fn).
  • A run that completes with agentCalls === 0 gets a warning line at the top of its tool result. This is a warning, not an error: a log()-only script is still valid.
  • Prompt text:
    • The workflow tool description now lists agent(), parallel(), pipeline(), and iterate() as the execution primitives, and states that phase(title) takes one string and runs nothing.
    • The system prompt's workflow bullet adds the same parenthetical.
  • docs/orchestration-lifecycle.md documents the check, the runtime guard, and the warning.

Behavior and compatibility

  • The phase signature and DSL semantics are unchanged: phase(title): void. Scripts that already used it correctly are unaffected.
  • Scripts that passed a second argument to phase() now fail instead of running nothing. Previously they never did any work.
  • The workflow tool description and one system-prompt line change. Every session where the workflow tool is registered receives this text.

Validation

  • pnpm run check: passed.
  • From packages/coding-agent: pnpm exec vitest --run test/workflow-*.test.ts test/step-system-prompt.test.ts: 8 files and 184 tests passed.
  • New tests:
    • misuse detection and lookalikes that must not be flagged (test/workflow-script-lint.test.ts)
    • the VM guard
    • inline and scriptPath rejection with line numbers
    • the zero-agent warning, present only on completed runs

Limitations

  • The static check is lexical, not a parser. Aliased calls are caught only at run time by the VM guard.
  • ./test.sh was not used as evidence. On the author's machine it fails in unrelated git-dependent suites that also fail on main.

…gents

phase(title) is a milestone label, but the VM dropped every argument after
the title. A script that wrapped its agent calls in a callback passed to
phase() ran zero agents and still reported a completed workflow.

A lexical pre-run check now rejects phase() calls with more than one
argument for inline, scriptPath, and saved scripts, naming the line before a
journal or child process exists. The VM throws on the same misuse when the
call is not visible statically. A completed run with no agent calls leads its
result with a warning. The tool description and system prompt state that
phase(title) takes one string and runs nothing.
@liuedcson
liuedcson merged commit 993e714 into main Oct 21, 2026
2 checks passed
@liuedcson
liuedcson deleted the fix/workflow-phase-misuse branch October 21, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant