Skip to content

fix: harden workflow lifecycle, concurrency, retries, and error propagation - #247

Open
liuedcson wants to merge 4 commits into
mainfrom
fix/workflow-concurrency-lifecycle
Open

liuedcson wants to merge 4 commits into
mainfrom
fix/workflow-concurrency-lifecycle

Conversation

@liuedcson

Copy link
Copy Markdown
Collaborator

Summary

Fixes workflow/subagent concurrency, lifecycle, retry, and error-propagation defects found while reproducing a real "开10个子agent" session that ended in a rejected batch, ten ReferenceError callbacks, and a manual 8+2 rerun.

Workflow lifecycle

  • Cancel and drain running and queued children before publishing a terminal state; terminal states are sealed and the run scope is aborted on exit.
  • Failed/early-returned runs can no longer report completed with agentCalls: 0; a run that returns with unawaited agent()/iterate()/workflow() operations fails closed.
  • The terminal status stays authoritative even when a progress write fails (a full disk can no longer turn a failed run into a hung one or replace the real error).

Concurrency

  • Concurrent workflows share one process-wide admission window; an uncooperative runner keeps its reservation until settlement instead of permanently poisoning the window.
  • Requested concurrency is clamped to a CPU-derived ceiling (max(1, min(16, cpus-2)), optionally lowered by STEP_WORKFLOW_CONCURRENCY_LIMIT, which now ignores blank/invalid values). Clamping is reported as a notice, never as a failure.
  • parallel() accepts functions or promises; ordinary subagent batches larger than the concurrency window queue the excess instead of being rejected (an explicitly configured maxParallelTasks still bounds batch size).
  • Explicit counted fan-out requests ("launch 10 subagents" / "开10个子agent") grant per-turn workflow consent without the Ultraloop keyword.

Error propagation

  • parallel()/pipeline() callback failures are recorded as taskFailures (with script attribution for nested workflows) and surfaced in the live text rows and final result; agent-node failures are no longer double-counted as callback failures.
  • AgentToolResult.isError from child tool results now propagates instead of being reported to the model as success.
  • Temporary 429s get an independent retry budget with Retry-After support and adaptive admission-cap reduction/recovery; terminal quota errors (including OpenCode Zen GoUsageLimitError/FreeUsageLimitError) are never retried.
  • The agent loop finalizes a partial assistant message when a provider stream throws, ends early, or fails during construction — without reclassifying listener/persistence errors as model errors or emitting a second terminal message_end.

Tests

  • New: shell-interpolation regression (broken template → failed with 10 ReferenceErrors; plain string → 10 agents dispatched), nested callback attribution, no-double-counting, unawaited nested workflow, retained-reservation/pool recreation, real child-process exit drain behind a detached descendant, terminal progress-write failure, batch-size vs concurrency semantics, counted opt-in routing, 429 classification.
  • Full suites: coding-agent 4469 passed (3 pre-existing completion-check-cli failures, identical on main), agent-core 536, providers 437; pnpm check clean.
  • Live validation with the user's original prompt: first and only tool call is workflow, observed 8 running + 2 queued with refill before the first wave finished, 10/10 completed in ~23.5s, and each reported value matched the agent's actual command output.

…gation

Workflow runtime:
- Cancel and drain running and queued children before publishing a terminal
  state, seal terminal states, and abort the run scope on exit.
- Share a process-wide admission window across concurrent workflows, clamp
  requested concurrency to a CPU-derived ceiling (optionally lowered by
  STEP_WORKFLOW_CONCURRENCY_LIMIT), and report clamping as a notice instead of
  a failure.
- Record parallel()/pipeline callback failures as taskFailures, including
  nested workflow failures attributed to their script, so a run whose callbacks
  threw can never report completed with zero agent calls.
- Keep agent-node failures out of the callback channel, and fail runs that
  return with unawaited agent/iterate/workflow operations.
- Keep the terminal status authoritative even when a progress write fails.

Subagents and child processes:
- Accept batches larger than the concurrency window and queue the excess; an
  explicitly configured maxParallelTasks still bounds batch size.
- Wait for the OS child to exit through the shared stdio-drain helper so a
  detached descendant holding pipes cannot poison the shared window.
- Propagate AgentToolResult.isError from child tool results.

Retries and streaming:
- Give temporary 429s an independent retry budget with Retry-After support,
  adaptive admission-cap reduction and recovery, and no retries for terminal
  quota errors.
- Finalize a partial assistant message when a provider stream throws, ends
  early, or fails during construction, without reclassifying listener or
  persistence errors as model errors or emitting a second terminal message.
@liuedcson

Copy link
Copy Markdown
Collaborator Author

Heads-up on the first CI run: build-check-test failed once on a 30s test timeout in test/suite/agent-session-tool-output.test.ts > keeps the tail and exit status of a failed command that bash already truncated (a real bash subprocess test on the shared 4-vCPU runner). All 4474 other tests passed in that run. I re-ran the failed job on the identical commit and it passed in 6m22s, so it is a runner flake, not a regression — that test file passes in ~2s locally and is untouched by this change.

A node's own retry honors the server's Retry-After verbatim, but that same
value was passed to the admission window's cooldown. The window is shared by
every workflow in the process and its queue time is excluded from node
timeouts, so a large Retry-After (hours, or a far-future HTTP date) froze
agent() for unrelated workflows indefinitely: nodes sat queued with no error,
no timeout, and no way to fail fast, and the drain timer was re-armed at the
2^31-1 ms ceiling.

Clamp the cooldown to 30 seconds at the semaphore choke point, matching the
cap already used for the jittered backoff fallback. The 25% cap reduction per
congestion epoch, not the cooldown length, is what protects the provider, and
the node's retry still waits the full server-requested delay.
Recording a node failure awaited the journal append and the progress write
before the failure was classified. A disk error (EACCES/ENOSPC/EISDIR) or a
throwing onProgress callback therefore replaced the node's real error, skipped
the nodeErrors marker, and let parallel()/pipeline() report the same failure a
second time as a callback error: one failed node became "1 failed nodes and 1
failed callbacks". The runner-reported failure path had the same problem, where
the journal error surfaced as the node's errorMessage.

Classify before recording, and make every failure-recording write best effort:
the node already failed, so a persistence error must not change the terminal
error or double-count the failure. A completed node's journal append stays
strict, since that is a real persistence failure worth surfacing.

Also tighten the counted fan-out opt-in. Asking about a request is not consent
to perform it, so "开10个子agent要多少钱?", "开10个子agent是不是太多了?" and
"Launch 10 agents? That seems excessive." no longer opt the turn into a
multi-agent workflow, while refusals and deliberation ("还是先用subagent工具吧")
still cancel it. The phrase may now sit anywhere in the message, so "麻烦开3个
子agent查一下", "先看代码,然后开10个子agent", bare "开10个agent", full-width
digits, and mid-sentence English requests are recognized.

Each behavior is pinned by a regression test; removing either fix fails them.
The provider retry-hint parser clamps negative and past HTTP-date values to
zero, but only the positive, invalid, and unbounded cases were pinned. Zero
and past values must never surface as a negative wait or as undefined.
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