fix(workflow-executor): merge leading SystemMessages for Anthropic compat#1604
Conversation
…mpat Anthropic's LangChain adapter only accepts a SystemMessage at index 0 (extracted as the `system` parameter); any subsequent SystemMessage throws "System messages are only permitted as the first passed message". OpenAI accepts them anywhere, so the crash only surfaces with `AI_PROVIDER=anthropic`. Every executor builds prompts with multiple consecutive SystemMessages (context + previous steps + step-specific). Merge the leading ones into a single SystemMessage in `invokeWithTools` before invoking the model, joined by blank lines so the context separation is preserved. Also add `assertNoMidArraySystemMessages` to surface a clear developer error if a SystemMessage appears after a non-system message — the merge only covers leading ones, and a mid-array one would still crash with Anthropic. Better to fail at the boundary than leak to the provider. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Use `instanceof SystemMessage` instead of `_getType()` (no-underscore-dangle) - Import `HumanMessage` directly instead of via jest.requireActual Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ssages The PRD-413 fix merges leading SystemMessages into a single one before invoking the model. Step-executor tests that asserted the previous shape (3..5 separate SystemMessages followed by a HumanMessage) now assert that the merged content contains all the expected substrings, followed by the HumanMessage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Coverage Impact Unable to calculate total coverage change because base branch coverage was not found. Modified Files with Diff Coverage (3)
🛟 Help
|
| if (!(msg instanceof SystemMessage)) { | ||
| seenNonSystem = true; | ||
| } else if (seenNonSystem) { | ||
| throw new Error( |
Scra3
left a comment
Scra3
left a comment
There was a problem hiding this comment.
3 findings — 2 non-blocking issues, 1 suggestion. No blocking issues.
| const merged = new SystemMessage( | ||
| messages | ||
| .slice(0, i) | ||
| .map(m => String(m.content)) |
There was a problem hiding this comment.
issue (non-blocking): String(arr) returns "[object Object],..." for complex content — use typeof m.content === 'string' ? m.content : JSON.stringify(m.content) to avoid silent corruption if a SystemMessage is ever constructed with structured content.
| messages | ||
| .slice(0, i) | ||
| .map(m => String(m.content)) | ||
| .join('\n\n'), |
There was a problem hiding this comment.
suggestion: add .filter(Boolean) before .join to prevent a stray leading \n\n when any leading SystemMessage has empty content.
| if (!(msg instanceof SystemMessage)) { | ||
| seenNonSystem = true; | ||
| } else if (seenNonSystem) { | ||
| throw new Error( |
There was a problem hiding this comment.
question: a plain Error is caught by BaseStepExecutor.execute() as 'Unexpected error during step execution' — the invariant message only surfaces in logs, not in the step outcome sent to the orchestrator. Is this intentional, or should it throw a WorkflowExecutorError with a user-facing message?
There was a problem hiding this comment.
Decision: create a dedicated InvalidAiRequestError extends WorkflowExecutorError — symmetrical with InvalidAIResponseError.
- Console (dev):
SystemMessage at position N appears after a non-system message — move all system context to the front of the messages array. - End user (UI):
Step configuration error — please contact your administrator.
- Throw a dedicated InvalidAiRequestError (symmetric with InvalidAIResponseError) instead of a plain Error — the dev message lands in logs while the end user sees "Step configuration error — please contact your administrator." - Include the position of the offending SystemMessage in the message. - Handle non-string SystemMessage content via JSON.stringify (String(arr) would emit "[object Object],..." for structured content). - Filter out empty content before joining so a blank SystemMessage doesn't leave a stray "\n\n" at the start of the merged prompt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Summary
When using
AI_PROVIDER=anthropic, every AI-powered step crashed with:```
Error: System messages are only permitted as the first passed message.
```
Anthropic's LangChain adapter extracts `messages[0]` as the `system` param and throws on any subsequent `SystemMessage`. OpenAI accepts them anywhere — that's why this only surfaced with Anthropic.
Every executor builds prompts as:
so the array starts with 2..N consecutive `SystemMessage`s. With Anthropic that crashes every step.
Fix
Two static helpers added to `BaseStepExecutor`, called in `invokeWithTools` before passing messages to the model:
Test plan
fixes PRD-413
🤖 Generated with Claude Code
Note
Merge leading SystemMessages in
BaseStepExecutorfor Anthropic compatibilitymergeLeadingSystemMessagesto consolidate consecutive leadingSystemMessageinstances into one before model invocation, satisfying Anthropic's message format requirements.assertNoMidArraySystemMessagesto throw synchronously if aSystemMessageappears after any non-system message in the array.invokeWithToolsbefore binding and invoking the model.Changes since #1604 opened
BaseStepExecutorto merge leadingSystemMessageinstances by serializing non-string content withJSON.stringify, filtering out falsy values, and throwingInvalidAiRequestErrorwith position-specific details when aSystemMessageappears after non-system messages [44fd961]InvalidAiRequestErrorclass toworkflow-executorpackage as a newWorkflowExecutorErrorsubclass and exported it from the package root [44fd961]InvalidAiRequestErrorwhen validating system message positioning and updated error message regex expectations [44fd961]Macroscope summarized 66ca13a.