feat(agents): implement multi-agent orchestration system#47
Conversation
Implements Issue #11 - Build multi-agent orchestration system ## Changes ### AI Service Layer - Add unified AI client interface supporting multiple providers - Implement Anthropic client with streaming support - Implement OpenAI client with streaming support - Add factory function for provider selection ### Specialized Agents - Add base agent class with tool calling and streaming - Implement Architect agent for system design and planning - Implement Implementer agent for code writing - Implement Reviewer agent for code review - Implement Tester agent for test generation ### Orchestrator - Add task management with dependency resolution - Implement parallel task execution with concurrency control - Add agent lifecycle management (create, remove, status) - Implement event system for progress tracking - Add execution planning with topological sort ### Testing - Add comprehensive tests for AI clients (30 tests) - Add orchestrator tests for all major features - Configure Jest with ts-jest for TypeScript support Closes #11 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a full multi-agent subsystem to the agent-intelligence package: AI provider clients (Anthropic/OpenAI) with streaming, typed AI abstractions, a BaseAgent framework with four concrete agents, an AgentOrchestrator with scheduling/dependency handling and events, package/config/test setup, and comprehensive tests; removes legacy mock orchestrator. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Orchestrator as AgentOrchestrator
participant TaskQueue as Task Queue
participant Agent as BaseAgent
participant AIClient as AIClient
participant Tool as Tool Executor
Client->>Orchestrator: createTask(...)
Orchestrator->>TaskQueue: enqueue(task)
Orchestrator->>Orchestrator: processTaskQueue()
TaskQueue->>Orchestrator: select ready task
Orchestrator->>Agent: assignTask(taskId, agentId)
Orchestrator->>Orchestrator: emitEvent('task_started')
Orchestrator->>Agent: execute(task, context)
Agent->>AIClient: complete/completeStream(messages, options)
AIClient-->>Agent: stream/content blocks/events
alt tool_use encountered
Agent->>Tool: executeTool(name, input, context)
Tool-->>Agent: tool_result
Agent->>AIClient: resume with tool_result
end
AIClient-->>Agent: final CompletionResponse
Agent->>Orchestrator: AgentExecutionResult
Orchestrator->>Orchestrator: update task/agent state
Orchestrator->>Orchestrator: emitEvent('task_completed')
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
The CI uses babel-jest which doesn't allow async generators inside jest.mock() due to scope restrictions. Replaced with simple array-based iterators. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…bility Variables inside jest.mock() must be prefixed with 'mock' (case insensitive) to be allowed by babel-jest's scope restrictions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addresses code quality review comment. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@packages/agent-intelligence/src/__tests__/ai-client.test.ts`:
- Around line 8-50: The mock factory for '@anthropic-ai/sdk' creates an inline
async generator (used for messages.stream) which gets hoisted by Jest and causes
out-of-scope errors; fix it by extracting the async generator to a top-level
named async function* (e.g., anthropicStreamGenerator) outside the jest.mock
factory and have messages.stream.mockImplementation return an object whose
[Symbol.asyncIterator] is that top-level generator; do the same extraction for
the OpenAI mock's stream iterator (create e.g., openAIStreamGenerator) and
reference those top-level generators from the respective mock implementations
(keep existing shape: message_start/content_block_delta/message_stop and
finalMessage mocks).
In `@packages/agent-intelligence/src/__tests__/orchestrator.test.ts`:
- Around line 9-37: The mock's inline async generator inside the jest.mock
factory (the async function* used for messages.stream with Symbol.asyncIterator)
gets hoisted and references helper transforms that are out of scope; fix this by
pulling the async iterator out of the mock factory into a top-level helper
(e.g., createMockStreamAsyncIterator or anthropicStreamGenerator) defined before
the jest.mock call and have the messages.stream implementation return that
helper's iterator, keeping finalMessage and messages.create mocks intact;
alternatively implement the async iterable manually (an object with a
Symbol.asyncIterator method returning an object with an async next) as a
top-level helper and reference it from the jest.mock factory.
In `@packages/agent-intelligence/src/services/agents/architect-agent.ts`:
- Around line 69-74: The capability with id 'interface-design' references a
non-existent tool 'write_file'; update the mapping so it's consistent by either
adding a 'write_file' tool definition to getTools() (ensuring the tool name and
any handler/export match exactly) or removing 'write_file' from the tools array
in the 'interface-design' capability; locate these in the architect-agent.ts
capability object (id: 'interface-design', name: 'Interface Design') and in the
getTools() implementation and make the change so both lists reference the same
set of tool names.
In `@packages/agent-intelligence/src/services/agents/base-agent.ts`:
- Around line 393-408: The catch block in executeStream() currently sets status
and returns an error result but doesn't emit the error event like execute()
does; update executeStream() to call this.emit('error', error) (or
this.emit('error', errorMessage) matching execute()'s behavior) before
returning, ensuring subscribers receive the error; locate executeStream() and
mirror the error emission used in execute() while preserving setStatus('error'),
the returned object shape
(filesCreated/filesModified/filesDeleted/summary/messages/tokensUsed/error), and
use the same error value (error or errorMessage) to keep behavior consistent
with conversationHistory and totalTokens reporting.
- Around line 94-118: getInfo() currently sets createdAt on every call; add a
class field (e.g., private readonly createdAt: string) and initialize it in the
class constructor (this.createdAt = new Date().toISOString()) and then change
getInfo() to return that stored this.createdAt instead of calling new
Date().toISOString() there; keep lastActiveAt behavior as-is or update
separately if needed.
- Around line 360-382: The streaming executor (executeStream) is missing the
same "tool_use" event emissions that execute emits; update the executeStream
flow where it handles response.stopReason === 'tool_use' and toolUseBlocks to
emit a "tool_use" event for each block (including block.id, block.name,
block.input and any context) before calling this.executeTool, and then emit
"tool_result" events (with tool_use_id and content) after the tool completes;
ensure you still call this.setStatus('coding'), call this.executeTool(block.name
|| '', block.input || {}, context) for each block, push the toolResultMessage
into this.conversationHistory exactly as in execute, and keep the same payload
shapes so subscribers receive identical events between execute and
executeStream.
In `@packages/agent-intelligence/src/services/agents/implementer-agent.ts`:
- Around line 70-84: The capabilities list includes undefined tools: add
definitions for "create_component" and "move_file" in getTools() or change the
capabilities to reference existing tools; specifically update the
'component-creation' capability (id 'component-creation') to use an available
tool instead of create_component or implement a create_component tool in
getTools(), and update the 'refactoring' capability (id 'refactoring') to either
reference an existing move/rename tool or add a move_file implementation in
getTools(); ensure the tool names in the capabilities exactly match the keys
returned by getTools().
- Around line 256-257: In the 'run_command' branch in implementer-agent.ts the
code unsafely casts input.args to string[]; replace that cast with a
runtime-safe check and normalization: verify input.args is an array
(Array.isArray), map each element to String (or filter non-primitives) and then
join, falling back to an empty string if not an array or empty; update the case
'run_command' return expression to use this validated/normalized args handling
rather than (input.args as string[])?.join(' ') || '' so join cannot throw at
runtime.
In `@packages/agent-intelligence/src/services/ai/anthropic-client.ts`:
- Around line 46-48: Replace the unsafe cast of m.role in
conversationMessages.map with an explicit mapper function (e.g.,
mapAnthropicRole) that accepts Message['role'] and returns only 'user' |
'assistant' or throws for unsupported roles; update both occurrences (the
mapping at the messages creation and the similar code around lines 84-86) to
call mapAnthropicRole(m.role) instead of casting, and ensure formatContentBlocks
usage remains unchanged; confirm the mapper covers all valid MessageRole values
(adjust mapping if roles like 'tool' or 'system' should be allowed) so Anthropic
only receives permitted roles.
In `@packages/agent-intelligence/src/services/ai/openai-client.ts`:
- Around line 335-347: The mapping in mapOpenAIStopReason incorrectly converts
OpenAI's 'content_filter' stop reason to 'stop_sequence', which loses the
safety/policy signal; update the implementation so 'content_filter' is preserved
as a distinct stop reason (either by adding 'content_filter' to the
CompletionResponse['stopReason'] union in types.ts and returning
'content_filter' from mapOpenAIStopReason, or by mapping it to another distinct
value that conveys a policy block), and adjust any consumers/types that rely on
CompletionResponse['stopReason'] accordingly (refer to mapOpenAIStopReason and
CompletionResponse['stopReason'] for the exact places to change).
- Around line 20-24: The OPENAI_MODELS constant uses the preview alias for GPT-4
Turbo; update the GPT_4_TURBO entry in OPENAI_MODELS to use the current primary
identifier 'gpt-4-turbo' (replace the value for the GPT_4_TURBO key in the
OPENAI_MODELS object) so other code referencing OPENAI_MODELS.GPT_4_TURBO uses
the recommended model name.
In `@packages/agent-intelligence/src/services/orchestrator/orchestrator.ts`:
- Around line 378-383: Replace the non-null assertion on task.startedAt (used
when constructing the task result object with keys like
success/result/startedAt/completedAt/duration) with a defensive fallback value
instead of task.startedAt!; for example use a safe fallback (e.g.,
task.startedAt ?? a sensible default timestamp or null) so the code handles
undefined startedAt, and apply the same change to the other occurrence that
builds a similar object around lines 440–445.
- Around line 452-456: getState() currently returns a shallow copy via the
spread operator so internal collections (agents, taskQueue, activeTasks,
completedTasks) can still be mutated by callers; change getState to return a
deep copy of the OrchestratorState by constructing new containers for each
collection (e.g., new Map(this.state.agents), new Map(this.state.activeTasks),
new Map(this.state.completedTasks), and cloned arrays for taskQueue or other
arrays) and shallow-clone the values inside those containers as needed so
callers cannot mutate the orchestrator's internal Maps or arrays.
🧹 Nitpick comments (15)
packages/agent-intelligence/tsconfig.json (1)
12-15: Clarify declaration emit vs.noEmit.
noEmit: trueprevents.d.tsgeneration even thoughdeclaration/declarationMapare enabled. If you plan to ship types, consider removingnoEmithere (keepingtsc --noEmitonly in the typecheck script) or splitting a build tsconfig. If you don’t need declarations, drop the declaration flags.♻️ Option: allow declarations to emit in a build config
- "noEmit": true,packages/agent-intelligence/package.json (1)
5-11: Exports point to TS sources—confirm runtime can load TS.
main/exportstarget.tsfiles. This only works if every consumer runs through a TS-aware bundler/loader. If this package is ever consumed by plain Node/Jest or published, consider emitting JS to adist/folder and pointingmain/exports(andtypes) there. Please verify consumer toolchains.packages/agent-intelligence/jest.config.js (1)
3-23: Align ts-jest ESM settings with module output.
useESM: truewhile the embedded tsconfig setsmodule: 'commonjs'can create ESM/CJS mismatches in Jest. Either disable ESM here or switch to the ESM preset and an ESM module target. Please verify which mode you intend.♻️ Option: keep CommonJS and disable ESM in ts-jest
- useESM: true,packages/agent-intelligence/src/__tests__/orchestrator.test.ts (1)
315-317: Unused variablestate.The variable is declared but never used in the assertion.
♻️ Suggested fix
- const state = orchestrator.getState(); const tasks = orchestrator.getTasks();packages/agent-intelligence/src/services/ai/index.ts (1)
46-55: Inconsistent error handling for unsupported providers.
createAIClientandgetDefaultModelthrow an error for unsupported providers, butgetAvailableModelssilently returns an empty array. This inconsistency could mask bugs when adding new providers.♻️ Suggested fix for consistency
export function getAvailableModels(provider: AIProvider): string[] { switch (provider) { case 'anthropic': return Object.values(ANTHROPIC_MODELS); case 'openai': return Object.values(OPENAI_MODELS); default: - return []; + throw new Error(`Unsupported AI provider: ${provider}`); } }Alternatively, if returning an empty array is intentional for graceful degradation, document this behavior and update the test to cover it.
packages/agent-intelligence/src/services/agents/reviewer-agent.ts (2)
249-251: Variable declaration in switch case without block scope.Declaring
const locationdirectly in acaseclause without braces can cause issues in some JavaScript environments and reduces readability.♻️ Suggested fix
- case 'add_comment': - const location = input.line ? `:${input.line}` : ''; - return `Added ${input.severity} comment on ${input.path}${location}: ${input.message}`; + case 'add_comment': { + const location = input.line ? `:${input.line}` : ''; + return `Added ${input.severity} comment on ${input.path}${location}: ${input.message}`; + }
256-257: Unsafe type assertion oninput.blocking_issues.The assertion
(input.blocking_issues as string[])will fail at runtime with?.joinreturning undefined if the property is missing or not an array. Consider defensive handling.♻️ Suggested fix
- case 'request_changes': - return `Requested changes: ${input.summary}\nBlocking issues: ${(input.blocking_issues as string[])?.join(', ')}`; + case 'request_changes': { + const issues = Array.isArray(input.blocking_issues) + ? input.blocking_issues.join(', ') + : 'none specified'; + return `Requested changes: ${input.summary}\nBlocking issues: ${issues}`; + }packages/agent-intelligence/src/services/agents/implementer-agent.ts (1)
207-225: Security:run_commandtool requires safeguards before real implementation.This tool accepts arbitrary shell commands. When implementing the real execution logic:
- Use an allowlist of permitted commands
- Sanitize arguments to prevent injection
- Consider sandboxing or restricting the execution environment
- Add timeouts to prevent runaway processes
The current mock is safe, but document these requirements for future implementation.
packages/agent-intelligence/src/services/ai/openai-client.ts (2)
98-100: PreferconstforcollectedToolCalls.The Map is mutated via
set()but never reassigned. Usingconstcommunicates intent more clearly.♻️ Suggested fix
- let collectedToolCalls: Map<number, { id: string; name: string; arguments: string }> = - new Map(); + const collectedToolCalls = new Map<number, { id: string; name: string; arguments: string }>();
194-200: Consider logging when tool argument JSON parsing fails.Silently returning
{}on parse failure can make debugging difficult. Adding a warning log would help identify malformed responses from the API.packages/agent-intelligence/src/services/ai/types.ts (1)
30-41: Consider using discriminated unions forContentBlock.The current design uses optional fields for all content types. A discriminated union would provide better type safety and exhaustiveness checking:
♻️ Example discriminated union approach
export type ContentBlock = | { type: 'text'; text: string } | { type: 'tool_use'; id: string; name: string; input: Record<string, unknown> } | { type: 'tool_result'; tool_use_id: string; content: string };This ensures required fields are present for each type and enables exhaustive pattern matching.
packages/agent-intelligence/src/services/orchestrator/orchestrator.ts (2)
261-264: Polling loop could be replaced with event-driven coordination.The 1-second
setTimeoutcreates a polling loop that may delay task starts or waste cycles. Consider using aPromisethat resolves when any active task completes, or an event-based mechanism.This is acceptable for initial implementation but may impact responsiveness at scale.
320-343: No circular dependency detection in dependency graph.
buildDependencyGraphandbuildExecutionPlandon't detect circular dependencies. Tasks with circular deps would remain inwaitingindefinitely without feedback.Consider adding cycle detection (e.g., during
createTask) or marking cyclic tasks asblockedin the execution plan.packages/agent-intelligence/src/services/agents/base-agent.ts (2)
133-142: Consider wrapping callback invocations in try/catch.If an event callback throws an exception, it will prevent subsequent callbacks from being notified and could propagate the error up to the caller unexpectedly. For event systems, it's typically safer to isolate callback failures.
♻️ Proposed improvement
protected emitEvent(event: Omit<AgentEvent, 'agentId' | 'timestamp'>): void { const fullEvent: AgentEvent = { ...event, agentId: this.id, timestamp: new Date().toISOString(), }; for (const callback of this.eventCallbacks) { - callback(fullEvent); + try { + callback(fullEvent); + } catch (error) { + console.error('Agent event callback error:', error); + } } }
444-492: String-based parsing is functional but fragile.The regex-based extraction of file operations (
Created file:,Modified file:,Deleted file:) depends on tool implementations producing these exact strings. If tool output formats change, this parsing will silently fail to capture file operations.Consider having
executeToolimplementations return structured data (or update agent state directly) rather than relying on string parsing. This is acceptable for now but worth tracking as a future improvement.
| id: 'interface-design', | ||
| name: 'Interface Design', | ||
| description: 'Define TypeScript interfaces and data models', | ||
| requiredCredentials: [], | ||
| tools: ['read_file', 'write_file'], | ||
| }, |
There was a problem hiding this comment.
Capability references undefined tool write_file.
The interface-design capability lists write_file in its tools array, but getTools() does not define this tool. This inconsistency may cause issues if the capability-tool mapping is validated or used to filter available tools.
Either add the write_file tool definition to getTools() or remove it from this capability's tools list.
🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/agents/architect-agent.ts` around
lines 69 - 74, The capability with id 'interface-design' references a
non-existent tool 'write_file'; update the mapping so it's consistent by either
adding a 'write_file' tool definition to getTools() (ensuring the tool name and
any handler/export match exactly) or removing 'write_file' from the tools array
in the 'interface-design' capability; locate these in the architect-agent.ts
capability object (id: 'interface-design', name: 'Interface Design') and in the
getTools() implementation and make the change so both lists reference the same
set of tool names.
| getInfo(): Agent { | ||
| return { | ||
| id: this.id, | ||
| name: this.name, | ||
| role: this.role, | ||
| status: this.status, | ||
| capabilities: this.getCapabilities(), | ||
| metrics: { | ||
| tasksCompleted: 0, | ||
| linesWritten: 0, | ||
| reviewsPerformed: 0, | ||
| averageTaskTime: 0, | ||
| successRate: 0, | ||
| tokensUsed: 0, | ||
| }, | ||
| config: { | ||
| model: this.model, | ||
| temperature: this.temperature, | ||
| maxTokens: this.maxTokens, | ||
| tools: this.getTools().map((t) => t.name), | ||
| }, | ||
| createdAt: new Date().toISOString(), | ||
| lastActiveAt: new Date().toISOString(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
createdAt timestamp should be set once at construction, not on every call.
Currently createdAt is set to new Date().toISOString() each time getInfo() is called, which means the agent's "creation time" changes on every invocation. This is semantically incorrect and could cause issues in monitoring/dashboard scenarios that track agent lifecycle.
🐛 Proposed fix: Store createdAt in constructor
Add a field in the class and initialize it in the constructor:
protected eventCallbacks: AgentEventCallback[] = [];
protected conversationHistory: Message[] = [];
+protected createdAt: string;
constructor(
config: {
// ...
}
) {
// ...
this.temperature = config.temperature || 0.7;
+ this.createdAt = new Date().toISOString();
}Then update getInfo():
config: {
model: this.model,
temperature: this.temperature,
maxTokens: this.maxTokens,
tools: this.getTools().map((t) => t.name),
},
- createdAt: new Date().toISOString(),
+ createdAt: this.createdAt,
lastActiveAt: new Date().toISOString(),
};🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/agents/base-agent.ts` around lines
94 - 118, getInfo() currently sets createdAt on every call; add a class field
(e.g., private readonly createdAt: string) and initialize it in the class
constructor (this.createdAt = new Date().toISOString()) and then change
getInfo() to return that stored this.createdAt instead of calling new
Date().toISOString() there; keep lastActiveAt behavior as-is or update
separately if needed.
| if (response.stopReason === 'tool_use' && toolUseBlocks.length > 0) { | ||
| this.setStatus('coding'); | ||
|
|
||
| const toolResults = await Promise.all( | ||
| toolUseBlocks.map(async (block) => { | ||
| const output = await this.executeTool(block.name || '', block.input || {}, context); | ||
| return { | ||
| type: 'tool_result' as const, | ||
| tool_use_id: block.id, | ||
| content: output, | ||
| }; | ||
| }) | ||
| ); | ||
|
|
||
| const toolResultMessage: Message = { | ||
| role: 'user', | ||
| content: toolResults, | ||
| }; | ||
| this.conversationHistory.push(toolResultMessage); | ||
| } else { | ||
| continueExecution = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent event emission between execute() and executeStream().
The streaming version omits tool_use events that the non-streaming version emits (lines 229-245 in execute()). This inconsistency means event subscribers won't receive tool usage notifications during streaming execution, breaking observability.
🐛 Proposed fix: Add tool_use events to streaming execution
const toolResults = await Promise.all(
toolUseBlocks.map(async (block) => {
+ this.emitEvent({
+ type: 'tool_use',
+ data: {
+ tool: block.name,
+ toolInput: block.input,
+ },
+ });
+
const output = await this.executeTool(block.name || '', block.input || {}, context);
+
+ this.emitEvent({
+ type: 'tool_use',
+ data: {
+ tool: block.name,
+ toolOutput: output,
+ },
+ });
+
return {
type: 'tool_result' as const,
tool_use_id: block.id,
content: output,
};
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response.stopReason === 'tool_use' && toolUseBlocks.length > 0) { | |
| this.setStatus('coding'); | |
| const toolResults = await Promise.all( | |
| toolUseBlocks.map(async (block) => { | |
| const output = await this.executeTool(block.name || '', block.input || {}, context); | |
| return { | |
| type: 'tool_result' as const, | |
| tool_use_id: block.id, | |
| content: output, | |
| }; | |
| }) | |
| ); | |
| const toolResultMessage: Message = { | |
| role: 'user', | |
| content: toolResults, | |
| }; | |
| this.conversationHistory.push(toolResultMessage); | |
| } else { | |
| continueExecution = false; | |
| } | |
| } | |
| if (response.stopReason === 'tool_use' && toolUseBlocks.length > 0) { | |
| this.setStatus('coding'); | |
| const toolResults = await Promise.all( | |
| toolUseBlocks.map(async (block) => { | |
| this.emitEvent({ | |
| type: 'tool_use', | |
| data: { | |
| tool: block.name, | |
| toolInput: block.input, | |
| }, | |
| }); | |
| const output = await this.executeTool(block.name || '', block.input || {}, context); | |
| this.emitEvent({ | |
| type: 'tool_use', | |
| data: { | |
| tool: block.name, | |
| toolOutput: output, | |
| }, | |
| }); | |
| return { | |
| type: 'tool_result' as const, | |
| tool_use_id: block.id, | |
| content: output, | |
| }; | |
| }) | |
| ); | |
| const toolResultMessage: Message = { | |
| role: 'user', | |
| content: toolResults, | |
| }; | |
| this.conversationHistory.push(toolResultMessage); | |
| } else { | |
| continueExecution = false; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/agents/base-agent.ts` around lines
360 - 382, The streaming executor (executeStream) is missing the same "tool_use"
event emissions that execute emits; update the executeStream flow where it
handles response.stopReason === 'tool_use' and toolUseBlocks to emit a
"tool_use" event for each block (including block.id, block.name, block.input and
any context) before calling this.executeTool, and then emit "tool_result" events
(with tool_use_id and content) after the tool completes; ensure you still call
this.setStatus('coding'), call this.executeTool(block.name || '', block.input ||
{}, context) for each block, push the toolResultMessage into
this.conversationHistory exactly as in execute, and keep the same payload shapes
so subscribers receive identical events between execute and executeStream.
| messages: conversationMessages.map((m) => ({ | ||
| role: m.role as 'user' | 'assistant', | ||
| content: typeof m.content === 'string' ? m.content : formatContentBlocks(m.content), |
There was a problem hiding this comment.
Map/guard message roles instead of casting.
Casting m.role to 'user' | 'assistant' bypasses validation. If any other role slips through, Anthropic will receive an invalid role. Prefer an explicit mapping or throw on unsupported roles.
🔧 Add an explicit role mapper
- role: m.role as 'user' | 'assistant',
+ role: mapAnthropicRole(m.role),- role: m.role as 'user' | 'assistant',
+ role: mapAnthropicRole(m.role),If you don’t already have a helper, you can add one like:
function mapAnthropicRole(role: Message['role']): 'user' | 'assistant' {
if (role === 'user' || role === 'assistant') return role;
throw new Error(`Unsupported Anthropic role: ${role}`);
}Please verify the actual MessageRole values and adjust the mapping if tool or other roles are permitted.
#!/bin/bash
# Verify MessageRole values and where it is defined.
# Expect: only 'system' | 'user' | 'assistant' (or update mapping accordingly).
rg -n "MessageRole" --type=tsAlso applies to: 84-86
🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/ai/anthropic-client.ts` around lines
46 - 48, Replace the unsafe cast of m.role in conversationMessages.map with an
explicit mapper function (e.g., mapAnthropicRole) that accepts Message['role']
and returns only 'user' | 'assistant' or throws for unsupported roles; update
both occurrences (the mapping at the messages creation and the similar code
around lines 84-86) to call mapAnthropicRole(m.role) instead of casting, and
ensure formatContentBlocks usage remains unchanged; confirm the mapper covers
all valid MessageRole values (adjust mapping if roles like 'tool' or 'system'
should be allowed) so Anthropic only receives permitted roles.
| export const OPENAI_MODELS = { | ||
| GPT_4_TURBO: 'gpt-4-turbo-preview', | ||
| GPT_4O: 'gpt-4o', | ||
| GPT_4O_MINI: 'gpt-4o-mini', | ||
| } as const; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenAI GPT-4 Turbo model identifier 2025 2026 current version
💡 Result:
Current OpenAI GPT-4 Turbo model identifier: "gpt-4-turbo" (with available snapshot aliases such as "gpt-4-turbo-2024-04-09" and preview aliases like "gpt-4-turbo-preview"/"gpt-4-0125-preview"). See OpenAI model docs. [1][2]
Sources:
[1] OpenAI API — GPT-4 Turbo model page.
[2] Azure / Microsoft documentation summarizing OpenAI model versions.
Update GPT-4 Turbo model identifier to current version.
GPT_4_TURBO: 'gpt-4-turbo-preview' uses a preview alias. The current recommended identifier is 'gpt-4-turbo'. While the preview variant remains functional, update to the primary identifier for consistency with OpenAI's current API.
🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/ai/openai-client.ts` around lines 20
- 24, The OPENAI_MODELS constant uses the preview alias for GPT-4 Turbo; update
the GPT_4_TURBO entry in OPENAI_MODELS to use the current primary identifier
'gpt-4-turbo' (replace the value for the GPT_4_TURBO key in the OPENAI_MODELS
object) so other code referencing OPENAI_MODELS.GPT_4_TURBO uses the recommended
model name.
| function mapOpenAIStopReason(reason: string | null): CompletionResponse['stopReason'] { | ||
| switch (reason) { | ||
| case 'stop': | ||
| return 'end_turn'; | ||
| case 'length': | ||
| return 'max_tokens'; | ||
| case 'tool_calls': | ||
| return 'tool_use'; | ||
| case 'content_filter': | ||
| return 'stop_sequence'; | ||
| default: | ||
| return 'end_turn'; | ||
| } |
There was a problem hiding this comment.
Incorrect mapping: content_filter → stop_sequence loses safety signal.
OpenAI's content_filter indicates content was blocked for policy reasons, which is semantically different from stop_sequence. Mapping it to stop_sequence obscures important safety information from consumers.
Consider either:
- Adding
'content_filter'to theCompletionResponse['stopReason']union type - Mapping to a distinct value that preserves the safety signal
🔧 Option: Add content_filter to union (requires types.ts change)
In types.ts:
export interface CompletionResponse {
id: string;
content: ContentBlock[];
model: string;
- stopReason: 'end_turn' | 'max_tokens' | 'tool_use' | 'stop_sequence';
+ stopReason: 'end_turn' | 'max_tokens' | 'tool_use' | 'stop_sequence' | 'content_filter';
usage: TokenUsage;
}Then in openai-client.ts:
case 'content_filter':
- return 'stop_sequence';
+ return 'content_filter';🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/ai/openai-client.ts` around lines
335 - 347, The mapping in mapOpenAIStopReason incorrectly converts OpenAI's
'content_filter' stop reason to 'stop_sequence', which loses the safety/policy
signal; update the implementation so 'content_filter' is preserved as a distinct
stop reason (either by adding 'content_filter' to the
CompletionResponse['stopReason'] union in types.ts and returning
'content_filter' from mapOpenAIStopReason, or by mapping it to another distinct
value that conveys a policy block), and adjust any consumers/types that rely on
CompletionResponse['stopReason'] accordingly (refer to mapOpenAIStopReason and
CompletionResponse['stopReason'] for the exact places to change).
| success: result.success, | ||
| result, | ||
| startedAt: task.startedAt!, | ||
| completedAt: new Date().toISOString(), | ||
| duration: Date.now() - startTime, | ||
| }; |
There was a problem hiding this comment.
Replace non-null assertions with defensive defaults.
task.startedAt! assumes the field is always set, but the type allows undefined. Use a fallback for robustness.
🔧 Proposed fix
const taskResult: TaskResult = {
taskId,
agentId: task.assignee,
success: result.success,
result,
- startedAt: task.startedAt!,
+ startedAt: task.startedAt ?? new Date().toISOString(),
completedAt: new Date().toISOString(),
duration: Date.now() - startTime,
};Apply similarly at line 442.
Also applies to: 440-445
🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/orchestrator/orchestrator.ts` around
lines 378 - 383, Replace the non-null assertion on task.startedAt (used when
constructing the task result object with keys like
success/result/startedAt/completedAt/duration) with a defensive fallback value
instead of task.startedAt!; for example use a safe fallback (e.g.,
task.startedAt ?? a sensible default timestamp or null) so the code handles
undefined startedAt, and apply the same change to the other occurrence that
builds a similar object around lines 440–445.
| * Get current state | ||
| */ | ||
| getState(): OrchestratorState { | ||
| return { ...this.state }; | ||
| } |
There was a problem hiding this comment.
getState() shallow copy allows mutation of internal Maps.
The spread operator creates a shallow copy, but agents, taskQueue, activeTasks, and completedTasks are references to the internal collections. Callers can mutate the orchestrator's internal state.
🔧 Proposed fix with deep copy
getState(): OrchestratorState {
- return { ...this.state };
+ return {
+ ...this.state,
+ agents: new Map(this.state.agents),
+ taskQueue: [...this.state.taskQueue],
+ activeTasks: new Map(this.state.activeTasks),
+ completedTasks: [...this.state.completedTasks],
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Get current state | |
| */ | |
| getState(): OrchestratorState { | |
| return { ...this.state }; | |
| } | |
| /** | |
| * Get current state | |
| */ | |
| getState(): OrchestratorState { | |
| return { | |
| ...this.state, | |
| agents: new Map(this.state.agents), | |
| taskQueue: [...this.state.taskQueue], | |
| activeTasks: new Map(this.state.activeTasks), | |
| completedTasks: [...this.state.completedTasks], | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@packages/agent-intelligence/src/services/orchestrator/orchestrator.ts` around
lines 452 - 456, getState() currently returns a shallow copy via the spread
operator so internal collections (agents, taskQueue, activeTasks,
completedTasks) can still be mutated by callers; change getState to return a
deep copy of the OrchestratorState by constructing new containers for each
collection (e.g., new Map(this.state.agents), new Map(this.state.activeTasks),
new Map(this.state.completedTasks), and cloned arrays for taskQueue or other
arrays) and shallow-clone the values inside those containers as needed so
callers cannot mutate the orchestrator's internal Maps or arrays.
|
1 participant
Summary
Implements Issue #11 - Build multi-agent orchestration system for ThumbCode
This PR delivers a complete multi-agent AI system with:
Changes
AI Service Layer (
packages/agent-intelligence/src/services/ai/)AIClientinterface withcomplete()andcompleteStream()methodscreateAIClient()for provider selectionSpecialized Agents (
packages/agent-intelligence/src/services/agents/)BaseAgentabstract class with tool calling loop and event systemArchitectAgentfor system design with tools: read_file, analyze_dependencies, create_specImplementerAgentfor code writing with tools: write_file, edit_file, search_code, run_commandReviewerAgentfor code review with tools: get_diff, add_comment, approve_changesTesterAgentfor test generation with tools: run_tests, get_coverage, create_mockOrchestrator (
packages/agent-intelligence/src/services/orchestrator/)Test plan
Closes #11
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.