mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
fix(desktop): clarify options defensive rendering and layout fix (#69796)
- Replace undefined CSS class with Tailwind v4's so long choice text wraps properly instead of overflowing the button container. - Add validation that strips non-string items, blanks, newlines, and text >200 chars — preventing garbage/JSON arrays from rendering as raw values. - Add diagnostic logging at both the gateway event handler and the tool-args parser when choices are dropped, so malformed payloads are no longer silent. - Apply in both the gateway event handler () and the inline tool-args parser () for consistent defense in depth. - Add unit tests for covering null, non-array, mixed-type, blank, multiline, and overlong inputs. Closes #69122 Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
This commit is contained in:
parent
46fb10203b
commit
dc861964fc
4 changed files with 89 additions and 7 deletions
|
|
@ -17,7 +17,7 @@ import { modelOptionsQueryKey } from '@/lib/model-options'
|
|||
import { isProviderSetupErrorMessage } from '@/lib/provider-setup-errors'
|
||||
import { reconcileApprovalModeForProfile } from '@/store/approval-mode'
|
||||
import { billingCtaLabel, clearBillingBlock, runBillingRecovery, setBillingBlock } from '@/store/billing-block'
|
||||
import { clearClarifyRequest, setClarifyRequest } from '@/store/clarify'
|
||||
import { clearClarifyRequest, normalizeChoices, setClarifyRequest, warnDroppedChoices } from '@/store/clarify'
|
||||
import { setSessionCompacting } from '@/store/compaction'
|
||||
import { refreshBackgroundProcesses } from '@/store/composer-status'
|
||||
import { $gateway } from '@/store/gateway'
|
||||
|
|
@ -677,14 +677,18 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) {
|
|||
// over; the inline ClarifyTool reads the active session's entry.
|
||||
const requestId = typeof payload?.request_id === 'string' ? payload.request_id : ''
|
||||
const question = typeof payload?.question === 'string' ? payload.question : ''
|
||||
const rawChoices = payload?.choices
|
||||
const choices = normalizeChoices(rawChoices)
|
||||
|
||||
if (requestId && question) {
|
||||
const choices = Array.isArray(payload?.choices) ? payload!.choices!.filter(c => typeof c === 'string') : null
|
||||
if (rawChoices != null && choices.length === 0) {
|
||||
warnDroppedChoices('gateway', question, rawChoices)
|
||||
}
|
||||
|
||||
setClarifyRequest({
|
||||
requestId,
|
||||
question,
|
||||
choices,
|
||||
choices: choices.length > 0 ? choices : null,
|
||||
sessionId: sessionId ?? null
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ import { useI18n } from '@/i18n'
|
|||
import { triggerHaptic } from '@/lib/haptics'
|
||||
import { CircleLetterA, Loader2, MessageQuestion } from '@/lib/icons'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { clearClarifyRequest, sessionClarifyRequest } from '@/store/clarify'
|
||||
import { clearClarifyRequest, normalizeChoices, sessionClarifyRequest, warnDroppedChoices } from '@/store/clarify'
|
||||
import { $gateway } from '@/store/gateway'
|
||||
import { notifyError } from '@/store/notifications'
|
||||
|
||||
|
|
@ -54,11 +54,18 @@ function stringField(row: Record<string, unknown>, ...keys: string[]): string |
|
|||
|
||||
function readClarifyArgs(args: unknown): ClarifyArgs {
|
||||
const row = parseMaybeObject(args)
|
||||
const choices = Array.isArray(row.choices) ? row.choices.filter((c): c is string => typeof c === 'string') : null
|
||||
const rawChoices = row.choices
|
||||
const choices = normalizeChoices(rawChoices)
|
||||
|
||||
const question = stringField(row, 'question')
|
||||
|
||||
if (rawChoices != null && choices.length === 0 && question) {
|
||||
warnDroppedChoices('tool_args', question, rawChoices)
|
||||
}
|
||||
|
||||
return {
|
||||
question: stringField(row, 'question'),
|
||||
choices: choices && choices.length > 0 ? choices : null
|
||||
question,
|
||||
choices: choices.length > 0 ? choices : null
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import {
|
|||
$clarifyRequests,
|
||||
type ClarifyRequest,
|
||||
clearClarifyRequest,
|
||||
normalizeChoices,
|
||||
setClarifyRequest
|
||||
} from './clarify'
|
||||
import { $activeSessionId } from './session'
|
||||
|
|
@ -79,3 +80,43 @@ describe('clarify store', () => {
|
|||
expect($clarifyRequests.get()['session-b']?.requestId).toBe('other')
|
||||
})
|
||||
})
|
||||
|
||||
describe('normalizeChoices', () => {
|
||||
it('returns empty array for null/undefined', () => {
|
||||
expect(normalizeChoices(null)).toEqual([])
|
||||
expect(normalizeChoices(undefined)).toEqual([])
|
||||
})
|
||||
|
||||
it('returns empty array for non-array input', () => {
|
||||
expect(normalizeChoices('hello')).toEqual([])
|
||||
expect(normalizeChoices(42)).toEqual([])
|
||||
expect(normalizeChoices({})).toEqual([])
|
||||
})
|
||||
|
||||
it('filters out non-string items', () => {
|
||||
expect(normalizeChoices(['a', 42, 'b', null, 'c'])).toEqual(['a', 'b', 'c'])
|
||||
})
|
||||
|
||||
it('drops blank and whitespace-only strings', () => {
|
||||
expect(normalizeChoices(['a', '', 'b', ' ', 'c'])).toEqual(['a', 'b', 'c'])
|
||||
})
|
||||
|
||||
it('drops strings with newlines', () => {
|
||||
expect(normalizeChoices(['a', 'b\nc', 'd'])).toEqual(['a', 'd'])
|
||||
})
|
||||
|
||||
it('drops strings over 200 chars', () => {
|
||||
const long = 'x'.repeat(201)
|
||||
const ok = 'y'.repeat(200)
|
||||
expect(normalizeChoices(['a', long, ok])).toEqual(['a', ok])
|
||||
})
|
||||
|
||||
it('drops empty items and keeps valid ones', () => {
|
||||
expect(normalizeChoices(['valid', ' ', '', 'also valid'])).toEqual(['valid', 'also valid'])
|
||||
})
|
||||
|
||||
it('returns empty array when nothing survives', () => {
|
||||
expect(normalizeChoices(['', ' ', null, undefined])).toEqual([])
|
||||
expect(normalizeChoices([])).toEqual([])
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -9,6 +9,36 @@ export interface ClarifyRequest {
|
|||
sessionId: string | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate and normalize a choices array.
|
||||
*
|
||||
* Keeps non-blank, newline-free strings of length ≤ 200; drops everything else
|
||||
* and returns an empty array when nothing usable survives — the caller then
|
||||
* falls back to a free-text answer instead of dead buttons.
|
||||
*/
|
||||
export function normalizeChoices(choices: unknown): string[] {
|
||||
if (!Array.isArray(choices)) {
|
||||
return []
|
||||
}
|
||||
|
||||
return choices.filter(
|
||||
(c): c is string => typeof c === 'string' && c.trim().length > 0 && c.length <= 200 && !c.includes('\n')
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Structured warning for a clarify payload that arrived with choices but had
|
||||
* them all normalized away — keeps the remaining #69122 "no selectable choices"
|
||||
* triggers diagnosable in the field without dead constant fields.
|
||||
*/
|
||||
export function warnDroppedChoices(source: 'gateway' | 'tool_args', question: string, rawChoices: unknown): void {
|
||||
console.warn('[clarify] choices dropped after normalization', {
|
||||
choices_count: Array.isArray(rawChoices) ? rawChoices.length : 0,
|
||||
question_length: question.length,
|
||||
source
|
||||
})
|
||||
}
|
||||
|
||||
// Pending clarify requests keyed by the runtime session id that raised them.
|
||||
// Storing per-session (instead of one shared slot) lets a *background* session
|
||||
// park its clarify request while the user is looking at a different chat, then
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue