refactor(agent-runtime): align message format with Claude Agent SDK#433
refactor(agent-runtime): align message format with Claude Agent SDK#433jerryliang64 merged 3 commits intomasterfrom
Conversation
Replace custom OpenAI Assistants-style message types (MessageObject, ContentBlockType, MessageRole, etc.) with a lightweight SDK-aligned AgentMessage union type. Key changes: - Define AgentMessage as union of SDK message subtypes (system, stream_event, user, assistant, result, generic) - Remove RunObject/RunRecord.output — messages belong to Thread only - Simplify MessageConverter to only extractUsage and toAgentMessages - OSSAgentStore.getThread filters system messages by default - execRun now yields AgentMessage directly instead of AgentStreamMessage - Update all tests to match new types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughReplaces legacy stream/content-block types with SDK-aligned Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/API
participant Handler as AgentHandler
participant Runtime as AgentRuntime
participant Converter as MessageConverter
participant Store as OSSAgentStore
participant SSE as SSE/EventEmitter
rect rgba(200,220,255,0.5)
Client->>Handler: create run / call execRun
Handler->>Runtime: invoke execRun (AsyncGenerator<AgentMessage>)
end
rect rgba(200,255,200,0.5)
Runtime->>Handler: pull yielded AgentMessage*
Runtime->>Converter: buffer messages -> extractUsage()
Runtime->>Store: appendMessages(threadId, AgentMessage[])
Runtime->>SSE: pushEvent(AgentMessage) -- pass-through event data
end
rect rgba(255,220,200,0.5)
Converter-->>Runtime: usage (from result messages) or undefined
Runtime->>Runtime: complete run with usage
Runtime->>Store: updateRun(status, usage, timestamps)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/agent-runtime/src/AgentRuntime.ts (1)
95-100:⚠️ Potential issue | 🟠 MajorResume detection now misses system-only sessions.
At Line 97,
ensureThread()callsgetThread(input.threadId)without options. SinceOSSAgentStore.getThread()now filterssystemmessages by default, Line 98 can setisResume = falseeven when a session already exists with only system messages.💡 Proposed fix
private async ensureThread(input: CreateRunInput): Promise<{ threadId: string; input: CreateRunInput }> { if (input.threadId) { - const thread = await this.store.getThread(input.threadId); + const thread = await this.store.getThread(input.threadId, { includeSystemMessages: true }); const isResume = thread.messages.length > 0; return { threadId: input.threadId, input: { ...input, isResume } }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-runtime/src/AgentRuntime.ts` around lines 95 - 100, ensureThread currently calls this.store.getThread(input.threadId) which returns a thread with system messages filtered out, causing isResume to be false for sessions that only contain system messages; fix by calling getThread with the option to include system messages (e.g., this.store.getThread(input.threadId, { includeSystemMessages: true })) inside ensureThread so the subsequent isResume = thread.messages.length > 0 reflects system-only sessions as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/agent-runtime/src/MessageConverter.ts`:
- Around line 43-49: The toAgentMessages conversion in MessageConverter
currently coerces all non-system InputMessage entries to type: 'user', which
loses assistant turns; update MessageConverter.toAgentMessages to map
InputMessage.role === 'assistant' to type: 'assistant' and InputMessage.role ===
'user' to type: 'user' (skip 'system' as before) and keep message.payload as {
role, content } so assistant turns round-trip correctly; also update the
corresponding assistant-role unit test to expect AgentMessage.type ===
'assistant' for inputs with role 'assistant' (adjust test assertions referencing
toAgentMessages or MessageConverter).
---
Outside diff comments:
In `@core/agent-runtime/src/AgentRuntime.ts`:
- Around line 95-100: ensureThread currently calls
this.store.getThread(input.threadId) which returns a thread with system messages
filtered out, causing isResume to be false for sessions that only contain system
messages; fix by calling getThread with the option to include system messages
(e.g., this.store.getThread(input.threadId, { includeSystemMessages: true }))
inside ensureThread so the subsequent isResume = thread.messages.length > 0
reflects system-only sessions as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e95c7d0-2f37-4bea-99e2-ff9543c4a262
📒 Files selected for processing (14)
core/agent-runtime/src/AgentRuntime.tscore/agent-runtime/src/MessageConverter.tscore/agent-runtime/src/OSSAgentStore.tscore/agent-runtime/src/RunBuilder.tscore/agent-runtime/test/AgentRuntime.test.tscore/agent-runtime/test/MessageConverter.test.tscore/agent-runtime/test/OSSAgentStore.test.tscore/agent-runtime/test/RunBuilder.test.tscore/controller-decorator/src/decorator/agent/AgentHandler.tscore/controller-decorator/test/fixtures/AgentFooController.tscore/tegg/agent.tscore/types/agent-runtime/AgentMessage.tscore/types/agent-runtime/AgentRuntime.tscore/types/agent-runtime/AgentStore.ts
There was a problem hiding this comment.
Code Review
This pull request refactors the agent runtime to align its message format with SDK standards, replacing internal message objects with a unified AgentMessage type. Key changes include simplifying the MessageConverter logic, removing the output field from run records, and updating OSSAgentStore to support system message filtering. Feedback points out that toAgentMessages incorrectly maps assistant roles to the user type and suggests restoring architectural documentation in the storage layer to preserve context on atomicity and storage layout.
…nment - Fix toAgentMessages to map type from role instead of hardcoding 'user' - Change getThread default filter to only return user+assistant messages (aligned with SDK's getSessionMessages behavior) - Rename includeSystemMessages to includeAllMessages for clarity - Pass through GetThreadOptions in AgentRuntime.getThread - Restore precise test assertions for message counts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…to thread Stream events are incremental token-level deltas only useful during real-time streaming. The final assistant message already contains the complete response, so storing deltas wastes space with no read-time value. Other message types (system, result) are preserved for audit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/agent-runtime/src/AgentRuntime.ts`:
- Around line 207-211: asyncRun() and executeStreamBackground() currently append
raw streamMessages (via this.store.appendMessages) which causes per-token
stream_event deltas to be persisted; update both methods to filter messages for
storage before appending by running the combined messages through
MessageConverter.filterForStorage (the same helper used in syncRun()), i.e.,
replace appends of [...MessageConverter.toAgentMessages(...), ...streamMessages]
with
MessageConverter.filterForStorage([...MessageConverter.toAgentMessages(input.input.messages),
...streamMessages]) and pass that result to this.store.appendMessages(threadId,
...); ensure you use the same MessageConverter.filterForStorage utility and keep
existing use of MessageConverter.toAgentMessages for the input messages.
In `@core/agent-runtime/src/MessageConverter.ts`:
- Around line 53-59: The toAgentMessages conversion in
MessageConverter.toAgentMessages drops InputMessage.metadata so thread history
loses caller-supplied metadata; update MessageConverter.toAgentMessages to copy
metadata through into the AgentMessage.message (preserving existing role and
content) so AgentMessage.message includes metadata from InputMessage.metadata
(e.g., retain m.metadata when building the returned object) and ensure any
downstream consumers of AgentMessage.message still receive the metadata field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c5700e2-ebf4-4ec4-a8a4-50f82502f19e
📒 Files selected for processing (3)
core/agent-runtime/src/AgentRuntime.tscore/agent-runtime/src/MessageConverter.tscore/agent-runtime/test/MessageConverter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- core/agent-runtime/test/MessageConverter.test.ts
…434) ## Summary - Fix missing `filterForStorage` in `asyncRun` and `executeStreamBackground` methods - Previously only `syncRun` filtered out `stream_event` deltas before persisting to thread storage - This caused `stream_event` messages (incremental token-level deltas) to leak into thread history, wasting storage space ## Context Follow-up fix for #433. During local testing, thread history still contained `stream_event` messages because two out of three `appendMessages` call sites were not applying the `MessageConverter.filterForStorage()` filter. ## Test plan - [x] All 116 existing tests pass - [x] Verified locally that thread messages no longer contain `stream_event` entries 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved message persistence to exclude unnecessary stream metadata, resulting in cleaner and more efficient storage of conversation history. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
MessageObject,ContentBlockType,MessageRole,MessageStatus, etc.) with a lightweight SDK-alignedAgentMessageunion type (system | stream_event | user | assistant | result | generic)RunObject.output/RunRecord.output— messages now belong to Thread only (aligned with SDK's session model)MessageConverterfrom ~300 lines to ~50 lines: onlyextractUsage()andtoAgentMessages()remainOSSAgentStore.getThread()now filters out system messages by default, withincludeSystemMessagesoptionexecRunyieldsAgentMessagedirectly (wasAgentStreamMessage), SSE streaming passes SDK messages through as-isBreaking Changes
AgentStreamMessagetype removed → useAgentMessageMessageObject,ContentBlockType,MessageRole,MessageStatusremovedRunObject.outputremoved — query thread messages viagetThread()insteadMessageConverter.toContentBlocks,toMessageObject,createStreamMessage,extractFromStreamMessages,normalizeContentBlocks,mergeContentBlocks,toInputMessageObjectsremovedisTextBlock,isToolUseBlock,isToolResultBlock,isThinkingBlocktype guards removedRunBuilder.complete()signature changed:complete(usage?)instead ofcomplete(output, usage?)Test plan
core/agent-runtimetests pass (112/113, 1 pre-existing timeout flake in reconnect test)core/controller-decoratortests pass (74/74)core/agent-runtime🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
outputfield; completed runs record status, timestamps, usage and metadata only.Behavior Changes
typeas the SSE event name.