mirror of
https://github.com/Gitlawb/openclaude.git
synced 2026-08-24 02:34:15 -05:00
[codex] chore(query): add tool-pairing diagnostics (#1625)
* chore(query): add tool-pairing diagnostics Add a pure validator for tool_use/tool_result pairing issues and feed phase, query source, model, provider, and agent context into the existing pre-API repair log. Keep ensureToolResultPairing behavior intact while making future repair logs identify missing, orphaned, duplicate tool_use, and duplicate tool_result cases. * fix(query): complete pairing diagnostics coverage Address CodeRabbit review by detecting server-side tool use blocks without matching in-message results and by making pairing validation lazy so it only runs after the repair path has actually mutated messages.
This commit is contained in:
Bogdan
@@ -1335,7 +1335,13 @@ async function* queryModel(
|
||||
// Repair tool_use/tool_result pairing mismatches that can occur when resuming
|
||||
// remote/teleport sessions. Inserts synthetic error tool_results for orphaned
|
||||
// tool_uses and strips orphaned tool_results referencing non-existent tool_uses.
|
||||
messagesForAPI = ensureToolResultPairing(messagesForAPI)
|
||||
messagesForAPI = ensureToolResultPairing(messagesForAPI, {
|
||||
phase: 'api_before_repair',
|
||||
querySource: options.querySource,
|
||||
agentId: options.agentId,
|
||||
model: options.model,
|
||||
provider: getAPIProvider(),
|
||||
})
|
||||
|
||||
// Strip advisor blocks — the API rejects them without the beta header.
|
||||
if (!betas.includes(ADVISOR_BETA_HEADER)) {
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
import { expect, test } from 'bun:test'
|
||||
import type { BetaContentBlock } from '@anthropic-ai/sdk/resources/beta/messages/messages.mjs'
|
||||
import {
|
||||
createAssistantMessage,
|
||||
createUserMessage,
|
||||
ensureToolResultPairing,
|
||||
validateToolResultPairing,
|
||||
} from './messages.js'
|
||||
|
||||
function assistantWithToolUses(...ids: string[]) {
|
||||
return createAssistantMessage({
|
||||
content: ids.map(
|
||||
id =>
|
||||
({
|
||||
type: 'tool_use',
|
||||
id,
|
||||
name: 'Read',
|
||||
input: { file_path: '/tmp/example.txt' },
|
||||
}) as BetaContentBlock,
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
function userWithToolResults(...ids: string[]) {
|
||||
return createUserMessage({
|
||||
content: ids.map(id => ({
|
||||
type: 'tool_result' as const,
|
||||
tool_use_id: id,
|
||||
content: `result for ${id}`,
|
||||
})),
|
||||
})
|
||||
}
|
||||
|
||||
test('validateToolResultPairing accepts paired tool uses and results', () => {
|
||||
const assistant = assistantWithToolUses('toolu_ok')
|
||||
const user = userWithToolResults('toolu_ok')
|
||||
|
||||
const result = validateToolResultPairing([assistant, user], {
|
||||
phase: 'api_before_repair',
|
||||
})
|
||||
|
||||
expect(result.valid).toBe(true)
|
||||
expect(result.issues).toEqual([])
|
||||
})
|
||||
|
||||
test('validateToolResultPairing reports missing tool results with phase metadata', () => {
|
||||
const assistant = assistantWithToolUses('toolu_missing')
|
||||
|
||||
const result = validateToolResultPairing([assistant], {
|
||||
phase: 'api_before_repair',
|
||||
querySource: 'repl_main_thread',
|
||||
model: 'glm-5.1',
|
||||
provider: 'openai',
|
||||
})
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.context).toEqual({
|
||||
phase: 'api_before_repair',
|
||||
querySource: 'repl_main_thread',
|
||||
model: 'glm-5.1',
|
||||
provider: 'openai',
|
||||
})
|
||||
expect(result.issues).toEqual([
|
||||
{
|
||||
kind: 'missing_tool_result',
|
||||
toolUseId: 'toolu_missing',
|
||||
assistantIndex: 0,
|
||||
assistantMessageId: assistant.message.id,
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
test('validateToolResultPairing reports orphaned tool results', () => {
|
||||
const user = userWithToolResults('toolu_orphan')
|
||||
|
||||
const result = validateToolResultPairing([user], {
|
||||
phase: 'resume_before_api_repair',
|
||||
})
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.issues).toEqual([
|
||||
{
|
||||
kind: 'orphaned_tool_result',
|
||||
toolUseId: 'toolu_orphan',
|
||||
userIndex: 0,
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
test('validateToolResultPairing reports duplicate tool uses across assistant messages', () => {
|
||||
const first = assistantWithToolUses('toolu_duplicate')
|
||||
const firstResult = userWithToolResults('toolu_duplicate')
|
||||
const second = assistantWithToolUses('toolu_duplicate')
|
||||
const secondResult = userWithToolResults('toolu_duplicate')
|
||||
|
||||
const result = validateToolResultPairing([
|
||||
first,
|
||||
firstResult,
|
||||
second,
|
||||
secondResult,
|
||||
])
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.issues).toContainEqual({
|
||||
kind: 'duplicate_tool_use',
|
||||
toolUseId: 'toolu_duplicate',
|
||||
assistantIndex: 2,
|
||||
assistantMessageId: second.message.id,
|
||||
duplicateOfAssistantIndex: 0,
|
||||
duplicateOfAssistantMessageId: first.message.id,
|
||||
})
|
||||
})
|
||||
|
||||
test('validateToolResultPairing reports duplicate tool results in the paired user message', () => {
|
||||
const assistant = assistantWithToolUses('toolu_duplicate_result')
|
||||
const user = userWithToolResults(
|
||||
'toolu_duplicate_result',
|
||||
'toolu_duplicate_result',
|
||||
)
|
||||
|
||||
const result = validateToolResultPairing([assistant, user])
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.issues).toContainEqual({
|
||||
kind: 'duplicate_tool_result',
|
||||
toolUseId: 'toolu_duplicate_result',
|
||||
assistantIndex: 0,
|
||||
assistantMessageId: assistant.message.id,
|
||||
userIndex: 1,
|
||||
})
|
||||
})
|
||||
|
||||
test('validateToolResultPairing reports server tool uses without in-message results', () => {
|
||||
const assistant = createAssistantMessage({
|
||||
content: [
|
||||
{
|
||||
type: 'server_tool_use',
|
||||
id: 'srvu_missing',
|
||||
name: 'web_search',
|
||||
input: { query: 'openclaude' },
|
||||
} as unknown as BetaContentBlock,
|
||||
],
|
||||
})
|
||||
|
||||
const result = validateToolResultPairing([assistant], {
|
||||
phase: 'api_before_repair',
|
||||
})
|
||||
|
||||
expect(result.valid).toBe(false)
|
||||
expect(result.issues).toContainEqual({
|
||||
kind: 'server_tool_use_without_result',
|
||||
toolUseId: 'srvu_missing',
|
||||
assistantIndex: 0,
|
||||
assistantMessageId: assistant.message.id,
|
||||
})
|
||||
})
|
||||
|
||||
test('ensureToolResultPairing keeps repairing legacy mismatches', () => {
|
||||
const assistant = assistantWithToolUses('toolu_missing')
|
||||
|
||||
const repaired = ensureToolResultPairing([assistant], {
|
||||
phase: 'api_before_repair',
|
||||
})
|
||||
|
||||
expect(repaired).toHaveLength(2)
|
||||
expect(repaired[1]?.type).toBe('user')
|
||||
const content = repaired[1]?.message.content
|
||||
expect(Array.isArray(content)).toBe(true)
|
||||
expect(Array.isArray(content) ? content[0] : undefined).toMatchObject({
|
||||
type: 'tool_result',
|
||||
tool_use_id: 'toolu_missing',
|
||||
is_error: true,
|
||||
})
|
||||
})
|
||||
+271
-1
@@ -5199,6 +5199,248 @@ export function createToolUseSummaryMessage(
|
||||
}
|
||||
}
|
||||
|
||||
export type ToolResultPairingValidationContext = {
|
||||
phase?: string
|
||||
querySource?: string
|
||||
agentId?: string
|
||||
model?: string
|
||||
provider?: string
|
||||
}
|
||||
|
||||
export type ToolResultPairingIssueKind =
|
||||
| 'missing_tool_result'
|
||||
| 'orphaned_tool_result'
|
||||
| 'duplicate_tool_use'
|
||||
| 'duplicate_tool_result'
|
||||
| 'server_tool_use_without_result'
|
||||
|
||||
export type ToolResultPairingIssue = {
|
||||
kind: ToolResultPairingIssueKind
|
||||
toolUseId: string
|
||||
assistantIndex?: number
|
||||
assistantMessageId?: string
|
||||
userIndex?: number
|
||||
duplicateOfAssistantIndex?: number
|
||||
duplicateOfAssistantMessageId?: string
|
||||
}
|
||||
|
||||
export type ToolResultPairingValidationResult = {
|
||||
valid: boolean
|
||||
context: ToolResultPairingValidationContext
|
||||
issues: ToolResultPairingIssue[]
|
||||
}
|
||||
|
||||
function getToolUseId(block: unknown): string | null {
|
||||
if (
|
||||
typeof block === 'object' &&
|
||||
block !== null &&
|
||||
'type' in block &&
|
||||
block.type === 'tool_use' &&
|
||||
'id' in block &&
|
||||
typeof block.id === 'string'
|
||||
) {
|
||||
return block.id
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function getToolResultId(block: unknown): string | null {
|
||||
if (
|
||||
typeof block === 'object' &&
|
||||
block !== null &&
|
||||
'type' in block &&
|
||||
block.type === 'tool_result' &&
|
||||
'tool_use_id' in block &&
|
||||
typeof block.tool_use_id === 'string'
|
||||
) {
|
||||
return block.tool_use_id
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function getServerToolUseId(block: unknown): string | null {
|
||||
if (
|
||||
typeof block === 'object' &&
|
||||
block !== null &&
|
||||
'type' in block &&
|
||||
(block.type === 'server_tool_use' || block.type === 'mcp_tool_use') &&
|
||||
'id' in block &&
|
||||
typeof block.id === 'string'
|
||||
) {
|
||||
return block.id
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function getToolUseIdReference(block: unknown): string | null {
|
||||
if (
|
||||
typeof block === 'object' &&
|
||||
block !== null &&
|
||||
'tool_use_id' in block &&
|
||||
typeof block.tool_use_id === 'string'
|
||||
) {
|
||||
return block.tool_use_id
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
function getToolResultIdsFromUserMessage(message: UserMessage): string[] {
|
||||
if (!Array.isArray(message.message.content)) {
|
||||
return []
|
||||
}
|
||||
return message.message.content
|
||||
.map(block => getToolResultId(block))
|
||||
.filter((id): id is string => id !== null)
|
||||
}
|
||||
|
||||
export function validateToolResultPairing(
|
||||
messages: (UserMessage | AssistantMessage)[],
|
||||
context: ToolResultPairingValidationContext = {},
|
||||
): ToolResultPairingValidationResult {
|
||||
const issues: ToolResultPairingIssue[] = []
|
||||
const seenToolUses = new Map<
|
||||
string,
|
||||
{ assistantIndex: number; assistantMessageId: string }
|
||||
>()
|
||||
|
||||
for (let i = 0; i < messages.length; i++) {
|
||||
const msg = messages[i]!
|
||||
|
||||
if (msg.type === 'user') {
|
||||
if (messages[i - 1]?.type === 'assistant') {
|
||||
continue
|
||||
}
|
||||
for (const toolUseId of getToolResultIdsFromUserMessage(msg)) {
|
||||
issues.push({
|
||||
kind: 'orphaned_tool_result',
|
||||
toolUseId,
|
||||
userIndex: i,
|
||||
})
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
const uniqueToolUseIds = new Set<string>()
|
||||
const serverResultIds = new Set<string>()
|
||||
for (const block of msg.message.content) {
|
||||
const toolUseIdReference = getToolUseIdReference(block)
|
||||
if (toolUseIdReference !== null) {
|
||||
serverResultIds.add(toolUseIdReference)
|
||||
}
|
||||
}
|
||||
|
||||
for (const block of msg.message.content) {
|
||||
const toolUseId = getToolUseId(block)
|
||||
if (toolUseId !== null) {
|
||||
const firstSeen = seenToolUses.get(toolUseId)
|
||||
if (firstSeen) {
|
||||
issues.push({
|
||||
kind: 'duplicate_tool_use',
|
||||
toolUseId,
|
||||
assistantIndex: i,
|
||||
assistantMessageId: msg.message.id,
|
||||
duplicateOfAssistantIndex: firstSeen.assistantIndex,
|
||||
duplicateOfAssistantMessageId: firstSeen.assistantMessageId,
|
||||
})
|
||||
} else {
|
||||
seenToolUses.set(toolUseId, {
|
||||
assistantIndex: i,
|
||||
assistantMessageId: msg.message.id,
|
||||
})
|
||||
}
|
||||
|
||||
uniqueToolUseIds.add(toolUseId)
|
||||
}
|
||||
|
||||
const serverToolUseId = getServerToolUseId(block)
|
||||
if (
|
||||
serverToolUseId !== null &&
|
||||
!serverResultIds.has(serverToolUseId)
|
||||
) {
|
||||
issues.push({
|
||||
kind: 'server_tool_use_without_result',
|
||||
toolUseId: serverToolUseId,
|
||||
assistantIndex: i,
|
||||
assistantMessageId: msg.message.id,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
const nextMsg = messages[i + 1]
|
||||
const toolResultIds =
|
||||
nextMsg?.type === 'user' ? getToolResultIdsFromUserMessage(nextMsg) : []
|
||||
const toolResultIdSet = new Set(toolResultIds)
|
||||
const toolUseIdSet = new Set(uniqueToolUseIds)
|
||||
const seenToolResultIds = new Set<string>()
|
||||
|
||||
for (const toolResultId of toolResultIds) {
|
||||
if (seenToolResultIds.has(toolResultId)) {
|
||||
issues.push({
|
||||
kind: 'duplicate_tool_result',
|
||||
toolUseId: toolResultId,
|
||||
assistantIndex: i,
|
||||
assistantMessageId: msg.message.id,
|
||||
userIndex: i + 1,
|
||||
})
|
||||
}
|
||||
seenToolResultIds.add(toolResultId)
|
||||
}
|
||||
|
||||
for (const toolUseId of toolUseIdSet) {
|
||||
if (!toolResultIdSet.has(toolUseId)) {
|
||||
issues.push({
|
||||
kind: 'missing_tool_result',
|
||||
toolUseId,
|
||||
assistantIndex: i,
|
||||
assistantMessageId: msg.message.id,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
for (const toolResultId of toolResultIdSet) {
|
||||
if (!toolUseIdSet.has(toolResultId)) {
|
||||
issues.push({
|
||||
kind: 'orphaned_tool_result',
|
||||
toolUseId: toolResultId,
|
||||
assistantIndex: i,
|
||||
assistantMessageId: msg.message.id,
|
||||
userIndex: i + 1,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
valid: issues.length === 0,
|
||||
context,
|
||||
issues,
|
||||
}
|
||||
}
|
||||
|
||||
function formatToolResultPairingIssue(
|
||||
issue: ToolResultPairingIssue,
|
||||
): string {
|
||||
const parts = [`kind=${issue.kind}`, `tool_use_id=${issue.toolUseId}`]
|
||||
if (issue.assistantIndex !== undefined) {
|
||||
parts.push(`assistant_index=${issue.assistantIndex}`)
|
||||
}
|
||||
if (issue.assistantMessageId !== undefined) {
|
||||
parts.push(`assistant_message_id=${issue.assistantMessageId}`)
|
||||
}
|
||||
if (issue.userIndex !== undefined) {
|
||||
parts.push(`user_index=${issue.userIndex}`)
|
||||
}
|
||||
if (issue.duplicateOfAssistantIndex !== undefined) {
|
||||
parts.push(`duplicate_of_assistant_index=${issue.duplicateOfAssistantIndex}`)
|
||||
}
|
||||
if (issue.duplicateOfAssistantMessageId !== undefined) {
|
||||
parts.push(
|
||||
`duplicate_of_assistant_message_id=${issue.duplicateOfAssistantMessageId}`,
|
||||
)
|
||||
}
|
||||
return parts.join(',')
|
||||
}
|
||||
|
||||
/**
|
||||
* Defensive validation: ensure tool_use/tool_result pairing is correct.
|
||||
*
|
||||
@@ -5216,6 +5458,7 @@ export function createToolUseSummaryMessage(
|
||||
*/
|
||||
export function ensureToolResultPairing(
|
||||
messages: (UserMessage | AssistantMessage)[],
|
||||
context: ToolResultPairingValidationContext = {},
|
||||
): (UserMessage | AssistantMessage)[] {
|
||||
const result: (UserMessage | AssistantMessage)[] = []
|
||||
let repaired = false
|
||||
@@ -5484,6 +5727,7 @@ export function ensureToolResultPairing(
|
||||
}
|
||||
|
||||
if (repaired) {
|
||||
const validation = validateToolResultPairing(messages, context)
|
||||
// Capture diagnostic info to help identify root cause
|
||||
const messageTypes = messages.map((m, idx) => {
|
||||
if (m.type === 'assistant') {
|
||||
@@ -5522,20 +5766,46 @@ export function ensureToolResultPairing(
|
||||
throw new Error(
|
||||
`ensureToolResultPairing: tool_use/tool_result pairing mismatch detected (strict mode). ` +
|
||||
`Refusing to repair — would inject synthetic placeholders into model context. ` +
|
||||
`Phase: ${validation.context.phase ?? 'unknown'}. ` +
|
||||
`Issues: ${validation.issues.map(formatToolResultPairingIssue).join('; ') || 'none'}. ` +
|
||||
`Message structure: ${messageTypes.join('; ')}. See inc-4977.`,
|
||||
)
|
||||
}
|
||||
|
||||
const issueKinds = [
|
||||
...new Set(validation.issues.map(issue => issue.kind)),
|
||||
].join(',')
|
||||
const issueSummary =
|
||||
validation.issues.map(formatToolResultPairingIssue).join('; ') || 'none'
|
||||
const diagnosticContext =
|
||||
`Phase: ${validation.context.phase ?? 'unknown'}. ` +
|
||||
`Query source: ${validation.context.querySource ?? 'unknown'}. ` +
|
||||
`Provider: ${validation.context.provider ?? 'unknown'}. ` +
|
||||
`Model: ${validation.context.model ?? 'unknown'}. ` +
|
||||
`Issues: ${issueSummary}.`
|
||||
logEvent('tengu_tool_result_pairing_repaired', {
|
||||
messageCount: messages.length,
|
||||
repairedMessageCount: result.length,
|
||||
issueCount: validation.issues.length,
|
||||
phase: (validation.context.phase ??
|
||||
'unknown') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
querySource: (validation.context.querySource ??
|
||||
'unknown') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
agentId: (validation.context.agentId ??
|
||||
'none') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
model: (validation.context.model ??
|
||||
'unknown') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
provider: (validation.context.provider ??
|
||||
'unknown') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
issueKinds: (issueKinds ||
|
||||
'none') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
messageTypes: messageTypes.join(
|
||||
'; ',
|
||||
) as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
})
|
||||
logError(
|
||||
new Error(
|
||||
`ensureToolResultPairing: repaired missing tool_result blocks (${messages.length} -> ${result.length} messages). Message structure: ${messageTypes.join('; ')}`,
|
||||
`ensureToolResultPairing: repaired missing tool_result blocks (${messages.length} -> ${result.length} messages). ${diagnosticContext} Message structure: ${messageTypes.join('; ')}`,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user