Ask tool card crashes the session view on options without a label #53
Labels
No labels
Agent
Ignore
Agent
In Progress
Agent
Planned
Agent
Planning
Agent
Ready
Agent
Triage
Compat
Breaking
Kind
Bug
Kind
Documentation
Kind
Enhancement
Kind
Feature
Kind
Security
Kind
Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
lucas/omp-deck#53
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implementation: #54
Problem
In a plan-mode session the web UI crashed to React Router's default error page:
The stack points at
stripSuffixinapps/web/src/components/tools/Ask.tsx, called from the nestedquestions.map → options.mapinAskTool(stripSuffix(opt.label), line ~107).AskToolcastsargs.questionstoQuestion[]without validation. While theaskcall's arguments are still streaming (partial JSON), or when an option is not shaped{ label: string },opt.labelisundefinedandlabel.endsWith(...)throws.buildSelectionMaphas the same uncheckedo.label/selectedOptionsaccesses. 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.tsxfunction normalizeQuestions(raw: unknown): Question[]inapps/web/src/components/tools/Ask.tsx:[]unlessrawis an array.id=String(q.id ?? index),question=typeof q.question === "string" ? q.question : "",multi/recommended/contextcopied only when of the right type (boolean / number / string).options: if not an array →[]; each entry maps to{ label }wherelabelis the entry itself when it is a string, elseentry.labelwhen that is a string; entries yielding no string label are dropped.AskTool, replaceArray.isArray(args.questions) ? (args.questions as Question[]) : []withnormalizeQuestions(args.questions); the(q.options ?? [])fallback becomesq.options.stripSuffixtotal: signaturestripSuffix(label: unknown): string, returning""for non-strings. InbuildSelectionMap, filterselectedOptionsarrays to strings before mapping ((r.selectedOptions ?? []).filter((v): v is string => typeof v === "string"), same fordetails.selectedOptions), and guarddetails.resultswithArray.isArray.2. Per-card error boundary
apps/web/src/components/tools/ToolRenderBoundary.tsx(no existing ErrorBoundary inapps/web/src): class componentToolRenderBoundarywith props{ resetKey: unknown; children: ReactNode }and state{ error: Error | null; resetKey: unknown }.static getDerivedStateFromError(error)→{ error }.static getDerivedStateFromProps(props, state): ifprops.resetKey !== state.resetKeyreturn{ error: null, resetKey: props.resetKey }, elsenull— 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).<p className="font-mono text-2xs text-danger">Renderer error: {error.message}</p>(same text-2xs/mono style as Ask's "Aborted" line).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 viaString(...).Verification
cd apps/web && bun run typecheck(or the repo's web typecheck script; checkapps/web/package.json) passes.apps/web/src/Preview.tsx, served by the web dev server): temporarily add anaskfixture withargs: { questions: [{ id: "q", question: "Pick", options: [{}, "Plain", { label: "A (Recommended)" }] }] }→ card renders optionsPlainandA (Recommended)with no crash; then a fixture for a renderer forced to throw (e.g.askwithquestionswhose getter throws, or a temporarythrowin a renderer) → only that card showsRenderer error: …, the page stays up. Remove the throwaway fixtures afterwards.ask; watch the card while args stream — no route error page.Implementation note: the branch was first cut from a stale local
main, 18 commits behindorigin/main. Before publishing, I rebased both commits ontoorigin/main. Upstream touched none of the changed files, and the typecheck still passes.Decision: the error boundary's
resetKeyisstream ?? args. Both objects are replaced on every streaming update, so a card that crashed on partial args retries as soon as more data arrives.