Fix clarify icon alignment and spurious error-red on non-zero exit

- clarify-tool: top-align the help icon (items-start + mt-px) so it sits
  beside the first line of a multi-line question instead of floating
  centered against the whole block.
- tool-fallback: a non-zero exit code alone no longer paints the whole
  terminal/execute_code card red. grep no-match, diff differences, and
  piped commands routinely exit non-zero while producing useful output;
  only flag an error when the command produced no output. Explicit error
  signals (error field, success=false, status=error, isError) still go red.
- Add regression tests covering the exit-code -> status matrix.
This commit is contained in:
Brooklyn Nicholson 2026-06-06 09:23:50 -05:00
parent 40386f33ec
commit 6bbc5eefa0
3 changed files with 45 additions and 3 deletions

View file

@ -164,10 +164,10 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
data-slot="clarify-inline"
>
<span aria-hidden className="arc-border" />
<div className="flex items-center gap-2.5">
<div className="flex items-start gap-2.5">
<span
aria-hidden
className="grid size-6 shrink-0 place-items-center rounded-md bg-[color-mix(in_srgb,var(--dt-primary)_14%,transparent)] text-primary ring-1 ring-inset ring-primary/15"
className="mt-px grid size-6 shrink-0 place-items-center rounded-md bg-[color-mix(in_srgb,var(--dt-primary)_14%,transparent)] text-primary ring-1 ring-inset ring-primary/15"
>
<HelpCircle className="size-3.5" />
</span>

View file

@ -33,3 +33,34 @@ describe('buildToolView image handling', () => {
expect(buildToolView(part({ result: { url } }), '').imageUrl).toBe(url)
})
})
describe('buildToolView terminal exit-code status', () => {
const terminal = (result: Record<string, unknown>) =>
buildToolView(part({ result, toolName: 'terminal' }), '')
// A non-zero exit code with real output is not a failure (grep no-match,
// diff differences, piped commands surfacing the last stage's code, etc.) —
// it should render as success so the card isn't painted red.
it('treats non-zero exit with output as success', () => {
expect(terminal({ exit_code: 7, output: 'node ... 5174 (LISTEN)' }).status).toBe('success')
expect(terminal({ exit_code: 1, stdout: 'partial results' }).status).toBe('success')
})
// No output + non-zero exit is a genuine failure worth flagging.
it('treats non-zero exit with no output as error', () => {
expect(terminal({ exit_code: 127, output: '' }).status).toBe('error')
expect(terminal({ exit_code: 1 }).status).toBe('error')
})
it('treats zero exit as success', () => {
expect(terminal({ exit_code: 0, output: 'done' }).status).toBe('success')
})
// Explicit error signals still win regardless of output presence.
it('keeps explicit error signals red even with output', () => {
expect(terminal({ error: 'boom', exit_code: 0, output: 'partial' }).status).toBe('error')
expect(buildToolView(part({ isError: true, result: { output: 'x' }, toolName: 'terminal' }), '').status).toBe(
'error'
)
})
})

View file

@ -742,9 +742,20 @@ function toolErrorText(part: ToolPart, result: Record<string, unknown>): string
return firstStringField(result, ['message', 'reason', 'detail']) || `Tool returned status "${result.status}".`
}
// A non-zero exit code alone is a weak failure signal: grep returns 1 on
// no-match, diff returns 1 on differences, piped commands surface the last
// stage's code, etc. — all routinely produce useful output and aren't
// failures. Only treat it as an error when the command produced no real
// output to show; otherwise render the output normally (not red).
const exit = numberValue(result.exit_code)
return exit !== null && exit !== 0 ? `Command failed with exit code ${exit}.` : ''
if (exit !== null && exit !== 0) {
const hasOutput = Boolean(firstStringField(result, ['output', 'stdout', 'stderr'])?.trim())
return hasOutput ? '' : `Command failed with exit code ${exit}.`
}
return ''
}
function toolStatus(part: ToolPart, resultRecord: Record<string, unknown>): ToolStatus {