fix(coding-agent/tools): normalized ask tool call args to prevent TUI render crashes

- Normalized untrusted `questions` arguments by parsing double-encoded JSON strings and skipping invalid question entries before rendering.
- Added option normalization that dropped malformed option items while preserving valid entries in multi-choice rendering.
- Expanded ask tool renderer tests to verify malformed or unparsable questions no longer crash and now fall back safely.
This commit is contained in:
can1357
2026-06-10 06:33:35 +02:00
parent abba626704
commit 1dc95e72fc
3 changed files with 114 additions and 5 deletions
+1
View File
@@ -43,6 +43,7 @@
### Fixed
- Fixed an uncaught `questions.map is not a function` TUI crash in the ask tool's call renderer when a model double-encoded the `questions` array as a JSON string (a bare string passes a truthy `.length` check but has no `.map`): the renderer now normalizes untrusted call args — parsing double-encoded `questions`, dropping malformed entries/options, and falling back to the "No question provided" frame instead of throwing
- Fixed model-provider detection for append-only mode, authoritative Vertex endpoint checks, and upstream-routing selection by switching from URL substring checks to catalog host-matching helpers
- Fixed pasting into the ask tool's "Other (type your own)" text box (and hook input/editor dialogs) on terminals with OSC 5522 enhanced paste (kitty protocol): the enhanced-paste focus routing only targets components exposing a `pasteText` hook, and the dialog wrappers had none, so the payload was stuffed into the main prompt editor hidden behind the dialog. `HookEditorComponent` and `HookInputComponent` now forward `pasteText` to their inner editor/input (pasting also resets the input dialog's timeout countdown like any keystroke).
- Fixed auto-retry giving up after one attempt ("Provider requested Xms wait, exceeds retry.maxDelayMs") on a usage-limit 429 when every sibling account was only momentarily blocked: the retry delay now waits for the earliest sibling unblock when that comes sooner than the provider's multi-hour retry-after, so the next attempt picks up the recovered account instead of failing fast.
+57 -5
View File
@@ -641,6 +641,56 @@ interface AskRenderArgs {
}>;
}
/**
* Coerce an untrusted option list (streamed or model-mangled call args) into
* well-formed render options. Bare strings become labels; entries without a
* string label are dropped.
*/
function normalizeRenderOptions(raw: unknown): AskRenderOption[] | undefined {
if (!Array.isArray(raw)) return undefined;
const out: AskRenderOption[] = [];
for (const entry of raw) {
if (typeof entry === "string") {
out.push({ label: entry });
continue;
}
if (!entry || typeof entry !== "object") continue;
const { label, description } = entry as Partial<AskRenderOption>;
if (typeof label !== "string") continue;
out.push(typeof description === "string" ? { label, description } : { label });
}
return out;
}
/**
* Coerce untrusted `questions` call args into a renderable array. Models
* occasionally double-encode the array as a JSON string — a bare string passes
* a truthy `.length` check but has no `.map`, which used to crash the TUI
* render loop. Partially streamed args can also be missing fields.
*/
function normalizeRenderQuestions(raw: unknown): NonNullable<AskRenderArgs["questions"]> | undefined {
if (typeof raw === "string") {
try {
raw = JSON.parse(raw);
} catch {
return undefined;
}
}
if (!Array.isArray(raw)) return undefined;
const out: NonNullable<AskRenderArgs["questions"]> = [];
for (const entry of raw) {
if (!entry || typeof entry !== "object") continue;
const q = entry as Partial<NonNullable<AskRenderArgs["questions"]>[number]>;
out.push({
id: typeof q.id === "string" ? q.id : "?",
question: typeof q.question === "string" ? q.question : "",
options: normalizeRenderOptions(q.options) ?? [],
multi: q.multi === true,
});
}
return out;
}
/** Render a custom free-text answer as a status line plus indented continuation rows. */
function renderCustomInputLines(uiTheme: Theme, customInput: string): string[] {
const lines = customInput.split("\n");
@@ -724,8 +774,10 @@ export const askToolRenderer = {
new Markdown(text, 1, 0, mdTheme, accentStyle).render(Math.max(1, width - 3 + 1));
// Multi-part questions: one divider-labelled section per question.
if (args.questions && args.questions.length > 0) {
const questions = args.questions;
// Call args are untrusted (partially streamed or model-mangled) and a
// throw here takes down the whole TUI render loop — normalize first.
const questions = normalizeRenderQuestions(args.questions);
if (questions && questions.length > 0) {
const header = `${label} ${uiTheme.fg("muted", `${questions.length} questions`)}`;
return framedBlock(uiTheme, width => {
const sections = questions.map(q => {
@@ -742,7 +794,7 @@ export const askToolRenderer = {
}
// Single question
if (!args.question) {
if (typeof args.question !== "string" || !args.question) {
const errorLine = formatErrorMessage("No question provided", uiTheme);
return framedBlock(uiTheme, width => ({
header: errorLine,
@@ -756,9 +808,9 @@ export const askToolRenderer = {
const question = args.question;
const meta: string[] = [];
if (args.multi) meta.push("multi");
if (args.options?.length) meta.push(`options:${args.options.length}`);
const questionOptions = normalizeRenderOptions(args.options);
if (questionOptions?.length) meta.push(`options:${questionOptions.length}`);
const header = `${label}${formatMeta(meta, uiTheme)}`;
const questionOptions = args.options;
const multi = args.multi;
return framedBlock(uiTheme, width => {
const bodyLines = md(question, width);
@@ -1237,3 +1237,59 @@ describe("AskTool option markers", () => {
expect(text).not.toContain(theme!.radio.selected);
});
});
describe("askToolRenderer malformed call args", () => {
it("renders double-encoded questions string instead of crashing the TUI", async () => {
const theme = await getThemeByName("dark");
expect(theme).toBeDefined();
// Models occasionally JSON-encode the questions array as a string; a bare
// string passes a truthy `.length` check but has no `.map` (TUI crash).
const doubleEncoded = JSON.stringify([
{ id: "q1", question: "Pick one", options: [{ label: "Alpha" }, { label: "Beta" }] },
]);
const rendered = askToolRenderer.renderCall(
{ questions: doubleEncoded } as never,
{ expanded: true, isPartial: false },
theme!,
);
const text = stripAnsi(rendered.render(120).join("\n"));
expect(text).toContain("[q1]");
expect(text).toContain("Pick one");
expect(text).toContain("Alpha");
});
it("falls back to the error frame for unparseable questions without throwing", async () => {
const theme = await getThemeByName("dark");
expect(theme).toBeDefined();
for (const questions of ["[{trunc", 42, { 0: { id: "x" } }]) {
const rendered = askToolRenderer.renderCall(
{ questions } as never,
{ expanded: true, isPartial: true },
theme!,
);
const text = stripAnsi(rendered.render(120).join("\n"));
expect(text).toContain("No question provided");
}
});
it("drops malformed question entries and option items while keeping valid ones", async () => {
const theme = await getThemeByName("dark");
expect(theme).toBeDefined();
const rendered = askToolRenderer.renderCall(
{
questions: [
null,
"garbage",
{ id: "ok", question: "Real question", options: ["BareString", { label: "Proper" }, { nope: 1 }, 7] },
],
} as never,
{ expanded: true, isPartial: true },
theme!,
);
const text = stripAnsi(rendered.render(120).join("\n"));
expect(text).toContain("[ok]");
expect(text).toContain("Real question");
expect(text).toContain("BareString");
expect(text).toContain("Proper");
});
});