Ask tool card crashes the session view on options without a label #53

Closed
opened 2026-10-21 16:49:54 +02:00 by omp-deck · 1 comment
Collaborator

Implementation: #54

Problem

In a plan-mode session the web UI crashed to React Router's default error page:

Unexpected Application Error!
can't access property "endsWith", e is undefined
sd@.../assets/index-B63Nidc2.js:431:7230
RJ/<.children</<.children<.children<@...:431:8643
...

The stack points at stripSuffix in apps/web/src/components/tools/Ask.tsx, called from the nested questions.map → options.map in AskTool (stripSuffix(opt.label), line ~107). AskTool casts args.questions to Question[] without validation. While the ask call's arguments are still streaming (partial JSON), or when an option is not shaped { label: string }, opt.label is undefined and label.endsWith(...) throws. buildSelectionMap has the same unchecked o.label / selectedOptions accesses. Because no tool-card renderer is wrapped in an error boundary, one bad card takes down the whole route.

Plan

1. Normalize ask args in Ask.tsx

  • Add function normalizeQuestions(raw: unknown): Question[] in apps/web/src/components/tools/Ask.tsx:
    • Return [] unless raw is an array.
    • Keep only entries that are non-null objects; id = String(q.id ?? index), question = typeof q.question === "string" ? q.question : "", multi/recommended/context copied only when of the right type (boolean / number / string).
    • options: if not an array → []; each entry maps to { label } where label is the entry itself when it is a string, else entry.label when that is a string; entries yielding no string label are dropped.
  • In AskTool, replace Array.isArray(args.questions) ? (args.questions as Question[]) : [] with normalizeQuestions(args.questions); the (q.options ?? []) fallback becomes q.options.
  • Make stripSuffix total: signature stripSuffix(label: unknown): string, returning "" for non-strings. In buildSelectionMap, filter selectedOptions arrays to strings before mapping ((r.selectedOptions ?? []).filter((v): v is string => typeof v === "string"), same for details.selectedOptions), and guard details.results with Array.isArray.

2. Per-card error boundary

  • New file apps/web/src/components/tools/ToolRenderBoundary.tsx (no existing ErrorBoundary in apps/web/src): class component ToolRenderBoundary with props { resetKey: unknown; children: ReactNode } and state { error: Error | null; resetKey: unknown }.
    • static getDerivedStateFromError(error) → { error }.
    • static getDerivedStateFromProps(props, state): if props.resetKey !== state.resetKey return { error: null, resetKey: props.resetKey }, else null — so a card that crashed on partial streamed args retries when new args/stream arrive.
    • componentDidCatch(error, info) → console.error("tool renderer crashed", error, info.componentStack).
    • Fallback render: <p className="font-mono text-2xs text-danger">Renderer error: {error.message}</p> (same text-2xs/mono style as Ask's "Aborted" line).
  • In apps/web/src/components/tools/ToolCallCard.tsx, wrap {renderTool(effectiveName, effectiveProps)} (line ~101) as <ToolRenderBoundary resetKey={effectiveProps.stream ?? effectiveProps.args}>…</ToolRenderBoundary>. Rationale: stream/args object identity changes on every streaming update. The header (summarizeArgs) stays outside the boundary; it already coerces via String(...).

Verification

  • cd apps/web && bun run typecheck (or the repo's web typecheck script; check apps/web/package.json) passes.
  • Throwaway check in the preview gallery (apps/web/src/Preview.tsx, served by the web dev server): temporarily add an ask fixture with args: { questions: [{ id: "q", question: "Pick", options: [{}, "Plain", { label: "A (Recommended)" }] }] } → card renders options Plain and A (Recommended) with no crash; then a fixture for a renderer forced to throw (e.g. ask with questions whose getter throws, or a temporary throw in a renderer) → only that card shows Renderer error: …, the page stays up. Remove the throwaway fixtures afterwards.
  • Live: start a plan-mode session that issues ask; watch the card while args stream — no route error page.
Implementation: #54 ## Problem In a plan-mode session the web UI crashed to React Router's default error page: ``` Unexpected Application Error! can't access property "endsWith", e is undefined sd@.../assets/index-B63Nidc2.js:431:7230 RJ/<.children</<.children<.children<@...:431:8643 ... ``` The stack points at `stripSuffix` in `apps/web/src/components/tools/Ask.tsx`, called from the nested `questions.map → options.map` in `AskTool` (`stripSuffix(opt.label)`, line ~107). `AskTool` casts `args.questions` to `Question[]` without validation. While the `ask` call's arguments are still streaming (partial JSON), or when an option is not shaped `{ label: string }`, `opt.label` is `undefined` and `label.endsWith(...)` throws. `buildSelectionMap` has the same unchecked `o.label` / `selectedOptions` accesses. Because no tool-card renderer is wrapped in an error boundary, one bad card takes down the whole route. ## Plan ### 1. Normalize ask args in `Ask.tsx` - Add `function normalizeQuestions(raw: unknown): Question[]` in `apps/web/src/components/tools/Ask.tsx`: - Return `[]` unless `raw` is an array. - Keep only entries that are non-null objects; `id` = `String(q.id ?? index)`, `question` = `typeof q.question === "string" ? q.question : ""`, `multi`/`recommended`/`context` copied only when of the right type (boolean / number / string). - `options`: if not an array → `[]`; each entry maps to `{ label }` where `label` is the entry itself when it is a string, else `entry.label` when that is a string; entries yielding no string label are dropped. - In `AskTool`, replace `Array.isArray(args.questions) ? (args.questions as Question[]) : []` with `normalizeQuestions(args.questions)`; the `(q.options ?? [])` fallback becomes `q.options`. - Make `stripSuffix` total: signature `stripSuffix(label: unknown): string`, returning `""` for non-strings. In `buildSelectionMap`, filter `selectedOptions` arrays to strings before mapping (`(r.selectedOptions ?? []).filter((v): v is string => typeof v === "string")`, same for `details.selectedOptions`), and guard `details.results` with `Array.isArray`. ### 2. Per-card error boundary - New file `apps/web/src/components/tools/ToolRenderBoundary.tsx` (no existing ErrorBoundary in `apps/web/src`): class component `ToolRenderBoundary` with props `{ resetKey: unknown; children: ReactNode }` and state `{ error: Error | null; resetKey: unknown }`. - `static getDerivedStateFromError(error)` → `{ error }`. - `static getDerivedStateFromProps(props, state)`: if `props.resetKey !== state.resetKey` return `{ error: null, resetKey: props.resetKey }`, else `null` — so a card that crashed on partial streamed args retries when new args/stream arrive. - `componentDidCatch(error, info)` → `console.error("tool renderer crashed", error, info.componentStack)`. - Fallback render: `<p className="font-mono text-2xs text-danger">Renderer error: {error.message}</p>` (same text-2xs/mono style as Ask's "Aborted" line). - In `apps/web/src/components/tools/ToolCallCard.tsx`, wrap `{renderTool(effectiveName, effectiveProps)}` (line ~101) as `<ToolRenderBoundary resetKey={effectiveProps.stream ?? effectiveProps.args}>…</ToolRenderBoundary>`. Rationale: stream/args object identity changes on every streaming update. The header (`summarizeArgs`) stays outside the boundary; it already coerces via `String(...)`. ### Verification - `cd apps/web && bun run typecheck` (or the repo's web typecheck script; check `apps/web/package.json`) passes. - Throwaway check in the preview gallery (`apps/web/src/Preview.tsx`, served by the web dev server): temporarily add an `ask` fixture with `args: { questions: [{ id: "q", question: "Pick", options: [{}, "Plain", { label: "A (Recommended)" }] }] }` → card renders options `Plain` and `A (Recommended)` with no crash; then a fixture for a renderer forced to throw (e.g. `ask` with `questions` whose getter throws, or a temporary `throw` in a renderer) → only that card shows `Renderer error: …`, the page stays up. Remove the throwaway fixtures afterwards. - Live: start a plan-mode session that issues `ask`; watch the card while args stream — no route error page.
Author
Collaborator

Implementation note: the branch was first cut from a stale local main, 18 commits behind origin/main. Before publishing, I rebased both commits onto origin/main. Upstream touched none of the changed files, and the typecheck still passes.

Decision: the error boundary's resetKey is stream ?? args. Both objects are replaced on every streaming update, so a card that crashed on partial args retries as soon as more data arrives.

Implementation note: the branch was first cut from a stale local `main`, 18 commits behind `origin/main`. Before publishing, I rebased both commits onto `origin/main`. Upstream touched none of the changed files, and the typecheck still passes. Decision: the error boundary's `resetKey` is `stream ?? args`. Both objects are replaced on every streaming update, so a card that crashed on partial args retries as soon as more data arrives.
lucas closed this issue 2026-10-21 16:58:19 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
lucas/omp-deck#53
No description provided.