fix(chat): prevent chat interface crash on malformed AskUserQuestion payload (#920)

* fix(chat): prevent chat interface crash when AskUserQuestion payload is malformed

Loading a session that contains an AskUserQuestion tool call could crash the
entire chat interface with "TypeError: e.map is not a function".

The AskUserQuestion tool is configured with `defaultOpen: true`, so
QuestionAnswerContent renders as soon as the session loads. Its array guard
(`!questions || questions.length === 0`) only checked for truthiness, and
`q.options` was mapped/iterated with no guard at all. When `questions` or
`options` arrive from the session transcript as a non-array value, the
`.map()` / `.some()` calls throw and take down the whole chat view via the
error boundary.

Guard both with `Array.isArray()` so a single malformed message can no longer
crash the interface.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(chat): cover QuestionAnswerContent against malformed AskUserQuestion payloads

Adds the first frontend regression test, guarding the crash fixed in the
previous commit: a non-array `questions` value or a question missing its
`options` array must render gracefully instead of throwing
"e.map is not a function" and taking down the whole chat interface.

Follows the repo's existing test convention (node:test + tsx); uses
react-dom/server renderToStaticMarkup so no DOM/jsdom is required.
Run with: npx tsx --test src/**/QuestionAnswerContent.test.tsx

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(chat): harden QuestionAnswerContent against malformed question entries

Addresses review feedback: even with the array guards, a malformed transcript
could still crash before the options fallback ran —

- a `questions` entry that is null/non-object threw on `q.question` access
- a non-string `answers[q.question]` threw on `answer.split(', ')`

Skip entries that aren't a proper question object with a string prompt, and
only call string methods on the answer when it is actually a string. Extends
the regression test to cover both vectors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(chat): guard malformed question options

---------

Co-authored-by: hustuhao <hustuhao@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Simos Mikelatos <simosmik@gmail.com>
This commit is contained in:
turato
2026-06-26 22:47:24 +08:00
committed by GitHub
parent 591e8e7642
commit ed4ae3114a
2 changed files with 99 additions and 6 deletions

View File

@@ -0,0 +1,77 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import React from 'react';
import { renderToStaticMarkup } from 'react-dom/server';
import { QuestionAnswerContent } from './QuestionAnswerContent';
// Regression coverage for the chat-interface crash where an AskUserQuestion
// payload loaded from a session transcript arrives with a non-array `questions`
// or a question missing its `options` array. Rendering must degrade gracefully
// instead of throwing "TypeError: e.map is not a function".
test('renders without throwing when questions is a non-array value', () => {
assert.doesNotThrow(() => {
renderToStaticMarkup(
React.createElement(QuestionAnswerContent, {
// Malformed: object instead of an array
questions: { 0: { question: 'q?', options: [{ label: 'a' }] } } as never,
answers: {},
}),
);
});
});
test('renders without throwing when a question is missing options[]', () => {
assert.doesNotThrow(() => {
renderToStaticMarkup(
React.createElement(QuestionAnswerContent, {
questions: [{ question: 'Pick one?', header: 'H' } as never],
answers: { 'Pick one?': 'X' },
}),
);
});
});
test('renders without throwing when options[] contains malformed entries', () => {
assert.doesNotThrow(() => {
renderToStaticMarkup(
React.createElement(QuestionAnswerContent, {
questions: [{ question: 'Pick one?', options: [null, 'oops', { label: 'A' }] } as never],
answers: { 'Pick one?': 'A, Custom' },
}),
);
});
});
test('renders without throwing when a questions entry is null/non-object', () => {
assert.doesNotThrow(() => {
renderToStaticMarkup(
React.createElement(QuestionAnswerContent, {
questions: [null, 'oops', { question: 'Ok?', options: [{ label: 'A' }] }] as never,
answers: {},
}),
);
});
});
test('renders without throwing when an answer is a non-string value', () => {
assert.doesNotThrow(() => {
renderToStaticMarkup(
React.createElement(QuestionAnswerContent, {
questions: [{ question: 'Pick one?', options: [{ label: 'A' }] }],
// Malformed: answer is an object instead of the expected string
answers: { 'Pick one?': { unexpected: true } } as never,
}),
);
});
});
test('still renders a well-formed question + answer', () => {
const html = renderToStaticMarkup(
React.createElement(QuestionAnswerContent, {
questions: [{ question: 'Pick one?', header: 'H', options: [{ label: 'A' }, { label: 'B' }] }],
answers: { 'Pick one?': 'A' },
}),
);
assert.ok(html.includes('Pick one?'));
});

View File

@@ -15,7 +15,11 @@ export const QuestionAnswerContent: React.FC<QuestionAnswerContentProps> = ({
}) => {
const [expandedIdx, setExpandedIdx] = useState<number | null>(null);
if (!questions || questions.length === 0) {
// Tool inputs are runtime data loaded from session transcripts and may be
// malformed (e.g. `questions` arriving as a non-array). Guard with
// Array.isArray so a single bad payload can't crash the whole chat view
// with "e.map is not a function".
if (!Array.isArray(questions) || questions.length === 0) {
return null;
}
@@ -24,11 +28,23 @@ export const QuestionAnswerContent: React.FC<QuestionAnswerContentProps> = ({
return (
<div className={`space-y-2 ${className}`}>
{questions.map((q, idx) => {
{questions.map((rawQuestion, idx) => {
// Entries come from session transcripts and may be malformed; skip
// anything that isn't a proper question object with a string prompt.
if (!rawQuestion || typeof rawQuestion !== 'object' || typeof rawQuestion.question !== 'string') {
return null;
}
const q = rawQuestion;
const answer = answers?.[q.question];
const answerLabels = answer ? answer.split(', ') : [];
// `answer` may be a non-string (or absent) in malformed payloads.
const answerLabels = typeof answer === 'string' ? answer.split(', ') : [];
const skipped = !answer;
const isExpanded = expandedIdx === idx;
// `options` is typed as an array but comes from untrusted runtime data;
// keep only valid entries so `.some`/`.map` below never throw.
const options = Array.isArray(q.options)
? q.options.filter((opt) => opt && typeof opt === 'object' && typeof opt.label === 'string')
: [];
return (
<div
@@ -74,7 +90,7 @@ export const QuestionAnswerContent: React.FC<QuestionAnswerContentProps> = ({
{!isExpanded && answerLabels.length > 0 && (
<div className="mt-1.5 flex flex-wrap gap-1">
{answerLabels.map((lbl) => {
const isCustom = !q.options.some(o => o.label === lbl);
const isCustom = !options.some(o => o.label === lbl);
return (
<span
key={lbl}
@@ -110,7 +126,7 @@ export const QuestionAnswerContent: React.FC<QuestionAnswerContentProps> = ({
{isExpanded && (
<div className="border-t border-gray-100 px-3 pb-2.5 pt-0.5 dark:border-gray-700/40">
<div className="ml-6.5 space-y-1">
{q.options.map((opt) => {
{options.map((opt) => {
const wasSelected = answerLabels.includes(opt.label);
return (
<div
@@ -148,7 +164,7 @@ export const QuestionAnswerContent: React.FC<QuestionAnswerContentProps> = ({
);
})}
{answerLabels.filter(lbl => !q.options.some(o => o.label === lbl)).map(lbl => (
{answerLabels.filter(lbl => !options.some(o => o.label === lbl)).map(lbl => (
<div
key={lbl}
className="flex items-start gap-2 rounded-lg border border-blue-200/60 bg-blue-50/80 px-2.5 py-1.5 text-[12px] dark:border-blue-800/40 dark:bg-blue-900/20"