Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/src/utils/__tests__/message-block-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ describe('getAgentBaseName', () => {
expect(getAgentBaseName('file-picker')).toBe('file-picker')
})

test('normalizes direct tool aliases to canonical agent names', () => {
expect(getAgentBaseName('code_reviewer_lite')).toBe('code-reviewer-lite')
})

test('handles scoped name without version', () => {
expect(getAgentBaseName('codebuff/file-picker')).toBe('file-picker')
})
Expand Down
83 changes: 83 additions & 0 deletions cli/src/utils/__tests__/sdk-event-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,89 @@ describe('sdk-event-handlers', () => {
expect(getStreamingAgents().has('tool-1-0')).toBe(false)
})

test('matches underscore direct-tool aliases to hyphenated agent ids', () => {
const { ctx, getMessages, getStreamingAgents } = createTestContext()
const handleEvent = createEventHandler(ctx)
const handleChunk = createStreamChunkHandler(ctx)

handleEvent({
type: 'tool_call',
toolCallId: 'tool-1',
toolName: 'spawn_agents',
input: {
agents: [
{
agent_type: 'code_reviewer_lite',
prompt: 'Review this change',
},
],
},
agentId: 'main-agent',
parentAgentId: undefined,
} as any)

handleEvent({
type: 'subagent_start',
agentId: 'agent-real',
agentType: 'code-reviewer-lite',
displayName: 'Code Reviewer Lite',
onlyChild: true,
parentAgentId: undefined,
params: undefined,
prompt: 'Review this change',
})

handleChunk({
type: 'subagent_chunk',
agentId: 'agent-real',
agentType: 'code-reviewer-lite',
chunk: 'streamed review',
})

handleEvent({
type: 'subagent_finish',
agentId: 'agent-real',
agentType: 'code-reviewer-lite',
displayName: 'Code Reviewer Lite',
onlyChild: true,
parentAgentId: undefined,
params: undefined,
prompt: 'Review this change',
})

handleEvent({
type: 'tool_result',
toolCallId: 'tool-1',
toolName: 'spawn_agents',
output: [
{
type: 'json',
value: [
{
agentName: 'code-reviewer-lite',
agentType: 'code-reviewer-lite',
value: 'streamed review',
},
],
},
],
} as any)

const blocks = getMessages()[0].blocks ?? []
expect(blocks).toHaveLength(1)
const agentBlock = blocks[0] as AgentContentBlock
expect(agentBlock.agentId).toBe('agent-real')
expect(agentBlock.agentName).toBe('code-reviewer-lite')
expect(agentBlock.agentType).toBe('code-reviewer-lite')
expect(agentBlock.status).toBe('complete')
expect(agentBlock.blocks).toHaveLength(1)
expect(agentBlock.blocks?.[0]).toMatchObject({
type: 'text',
content: 'streamed review',
})
expect(getStreamingAgents().size).toBe(0)
})

test('handles spawn_agents tool results and clears streaming agents', () => {
const { ctx, getMessages, getStreamingAgents } = createTestContext()
ctx.message.updater.addBlock(
Expand Down
4 changes: 4 additions & 0 deletions cli/src/utils/__tests__/send-message-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,10 @@ describe('getAgentBaseName', () => {
test('returns simple name unchanged', () => {
expect(getAgentBaseName('file-picker')).toBe('file-picker')
})

test('normalizes direct tool aliases to canonical agent names', () => {
expect(getAgentBaseName('code_reviewer_lite')).toBe('code-reviewer-lite')
})
})

describe('agentTypesMatch', () => {
Expand Down
9 changes: 8 additions & 1 deletion cli/src/utils/message-block-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import type {
* getAgentBaseName('codebuff/[email protected]') // 'file-picker'
* getAgentBaseName('[email protected]') // 'file-picker'
* getAgentBaseName('file-picker') // 'file-picker'
* getAgentBaseName('file_picker') // 'file-picker'
*/
export const getAgentBaseName = (type: string): string => {
const segment = type.split('/').pop() ?? type
return segment.split('@')[0]
return segment.split('@')[0].replace(/_/g, '-')
}

/**
Expand Down Expand Up @@ -466,6 +467,7 @@ export const moveSpawnAgentBlock = (
parentId?: string,
params?: Record<string, unknown>,
prompt?: string,
realAgentType?: string,
): ContentBlock[] => {
const updateAgentBlock = (block: ContentBlock): ContentBlock => {
if (block.type !== 'agent') {
Expand All @@ -484,6 +486,11 @@ export const moveSpawnAgentBlock = (
updatedBlock.initialPrompt = prompt
}

if (realAgentType) {
updatedBlock.agentType = realAgentType
updatedBlock.agentName = realAgentType
}

return updatedBlock
}

Expand Down
1 change: 1 addition & 0 deletions cli/src/utils/sdk-event-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ const handleSubagentStart = (
blocks,
match: spawnAgentMatch,
realAgentId: event.agentId,
realAgentType: event.agentType,
parentAgentId: event.parentAgentId,
params: event.params,
prompt: event.prompt,
Expand Down
3 changes: 3 additions & 0 deletions cli/src/utils/spawn-agent-matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const resolveSpawnAgentToReal = (options: {
blocks: ContentBlock[]
match: SpawnAgentMatch
realAgentId: string
realAgentType?: string
parentAgentId?: string
params?: Record<string, unknown>
prompt?: string
Expand All @@ -36,6 +37,7 @@ export const resolveSpawnAgentToReal = (options: {
blocks,
match,
realAgentId,
realAgentType,
parentAgentId,
params: agentParams,
prompt,
Expand All @@ -48,5 +50,6 @@ export const resolveSpawnAgentToReal = (options: {
parentAgentId,
agentParams,
prompt,
realAgentType,
)
}
45 changes: 42 additions & 3 deletions packages/agent-runtime/src/__tests__/tool-validation-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,46 @@ describe('tool validation error handling', () => {

expect('error' in result).toBe(true)
if ('error' in result) {
expect(result.error).toContain('The JSON parser reported:')
expect(result.error).toContain('If the arguments are incomplete')
expect(result.error).toContain(
'expected the tool arguments to be an object, but received a string',
)
expect(result.error).toContain('Parsing as JSON failed:')
expect(result.error).toContain(
'The arguments may be malformed or incomplete',
)
}
})

it('should explain when parsed tool input remains a string', () => {
const input = JSON.stringify(
JSON.stringify(
JSON.stringify(
JSON.stringify({
path: 'test.ts',
instructions: 'Writes a test file',
content: 'console.log("test")\n',
}),
),
),
)

const result = parseRawToolCall({
rawToolCall: {
toolName: 'write_file',
toolCallId: 'over-encoded-tool-call-id',
input,
},
})

expect('error' in result).toBe(true)
if ('error' in result) {
expect(result.error).toContain(
'expected the tool arguments to be an object, but received a string',
)
expect(result.error).toContain(
'Parsing succeeded, but the parsed value was still a string',
)
expect(result.error).not.toContain('malformed or incomplete')
}
})

Expand Down Expand Up @@ -578,8 +616,9 @@ describe('tool validation error handling', () => {
)
expect(errorEvents.length).toBe(1)
expect(errorEvents[0].message).toContain(
'tool arguments were a string, not a JSON object',
'expected the tool arguments to be an object, but received a string',
)
expect(errorEvents[0].message).toContain('Parsing as JSON failed:')
expect(errorEvents[0].message).toContain('Original tool call input:')

expect(result.hadToolCallError).toBe(true)
Expand Down
21 changes: 15 additions & 6 deletions packages/agent-runtime/src/tools/tool-executor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { endsAgentStepParam, toolNames } from '@codebuff/common/tools/constants'
import { toolParams } from '@codebuff/common/tools/list'
import { normalizeAgentIdForLookup } from '@codebuff/common/util/agent-id-parsing'
import { cloneDeep } from 'lodash'

import { getMCPToolData } from '../mcp'
Expand Down Expand Up @@ -130,13 +131,13 @@ function stringInputError(
parseError?: string,
): ToolCallError {
const parseDetails = parseError
? ` The JSON parser reported: ${parseError}. If the arguments are incomplete, re-issue the full object.`
: ''
? ` Parsing as JSON failed: ${parseError}. The arguments may be malformed or incomplete.`
: ' Parsing succeeded, but the parsed value was still a string.'
return {
toolName,
toolCallId,
input: {},
error: `Invalid parameters for ${toolName}: tool arguments were a string, not a JSON object. The runtime tried to parse stringified JSON before validation, but the value was still not a JSON object.${parseDetails} Re-issue the tool call as a JSON object with properly escaped string values.`,
error: `Invalid parameters for ${toolName}: expected the tool arguments to be an object, but received a string.${parseDetails} Re-issue the tool call with the full arguments object and properly escaped string values.`,
}
}

Expand Down Expand Up @@ -371,7 +372,9 @@ export async function executeToolCall<T extends ToolName>(
}
}

let agentIdToLoad = agentTypeStr
let agentIdToLoad = isBaseAgent
? normalizeAgentIdForLookup(agentTypeStr)
: agentTypeStr
if (!isBaseAgent) {
const matchingSpawn = getMatchingSpawn(
agentTemplate.spawnableAgents,
Expand Down Expand Up @@ -420,7 +423,13 @@ export async function executeToolCall<T extends ToolName>(
}
}

return { valid: true as const, agent }
return {
valid: true as const,
agent: {
...(agent as Record<string, unknown>),
agent_type: agentIdToLoad,
},
}
}),
)

Expand Down Expand Up @@ -449,8 +458,8 @@ export async function executeToolCall<T extends ToolName>(
}
const errorMsg = `Some agents could not be spawned: ${errors.join('; ')}. Proceeding with valid agents only.`
onResponseChunk({ type: 'error', message: errorMsg })
effectiveInput = { ...effectiveInput, agents: validAgents }
}
effectiveInput = { ...effectiveInput, agents: validAgents }
}
}

Expand Down
Loading