From 182856ea665aa32d8c5e34ba5ba9605bd83e6be2 Mon Sep 17 00:00:00 2001 From: JJ Lee Date: Fri, 11 Sep 2026 12:46:11 -0400 Subject: [PATCH] fix: guard against undefined session.title and part.tool in message-part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit taskSession() crashes with 'Cannot read properties of undefined (reading startsWith)' when a child session has no title (spawned but not yet titled, or errored before the LLM generated one). The Session type declares title as required string, but at runtime it can be undefined. Similarly, amicodeReceiptCandidateKey() calls part.tool.startsWith() without checking typeof — a tool-typed part with a missing tool name would crash. Fixes: - Add optional chaining on session.title in taskSession() filters - Add typeof guard on part.tool in amicodeReceiptCandidateKey() - Extract findTaskSession/isAmicodeToolCall into message-part-task.ts with 14 unit tests covering the undefined-title and undefined-tool cases (same extraction pattern as message-part-text.ts) --- .../src/components/message-part-task.test.ts | 115 ++++++++++++++++++ .../src/components/message-part-task.ts | 23 ++++ .../src/components/message-part.tsx | 6 +- 3 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 packages/session-ui/src/components/message-part-task.test.ts create mode 100644 packages/session-ui/src/components/message-part-task.ts diff --git a/packages/session-ui/src/components/message-part-task.test.ts b/packages/session-ui/src/components/message-part-task.test.ts new file mode 100644 index 000000000..91d0310ff --- /dev/null +++ b/packages/session-ui/src/components/message-part-task.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, test } from "bun:test" +import { findTaskSession, isAmicodeToolCall } from "./message-part-task" + +// Minimal session stub — only the fields findTaskSession reads. +function session(overrides: { + id: string + parentID?: string + title?: string + archived?: number + created?: number +}) { + return { + id: overrides.id, + parentID: overrides.parentID, + title: overrides.title as string, // intentionally allow undefined to match runtime + time: { + created: overrides.created ?? Date.now(), + updated: Date.now(), + archived: overrides.archived, + }, + } as any +} + +describe("findTaskSession", () => { + test("returns matching child session id", () => { + const sessions = [ + session({ id: "child-1", parentID: "p1", title: "Fix bug @general", created: 100 }), + ] + expect(findTaskSession(sessions, "p1", "Fix bug", "general")).toBe("child-1") + }) + + test("returns undefined when no sessions match parentID", () => { + const sessions = [ + session({ id: "child-1", parentID: "other", title: "Fix bug @general" }), + ] + expect(findTaskSession(sessions, "p1", "Fix bug", "general")).toBeUndefined() + }) + + test("skips archived sessions", () => { + const sessions = [ + session({ id: "child-1", parentID: "p1", title: "Fix bug @general", archived: 1 }), + ] + expect(findTaskSession(sessions, "p1", "Fix bug", "general")).toBeUndefined() + }) + + test("returns most recently created when multiple match", () => { + const sessions = [ + session({ id: "old", parentID: "p1", title: "Fix bug @general", created: 100 }), + session({ id: "new", parentID: "p1", title: "Fix bug @general", created: 200 }), + ] + expect(findTaskSession(sessions, "p1", "Fix bug", "general")).toBe("new") + }) + + test("does not crash when a session has undefined title", () => { + const sessions = [ + session({ id: "no-title", parentID: "p1", title: undefined }), + session({ id: "titled", parentID: "p1", title: "Fix bug @general", created: 200 }), + ] + // Should not throw — must gracefully skip the untitled session + expect(() => findTaskSession(sessions, "p1", "Fix bug", "general")).not.toThrow() + expect(findTaskSession(sessions, "p1", "Fix bug", "general")).toBe("titled") + }) + + test("does not crash when all sessions have undefined title", () => { + const sessions = [ + session({ id: "a", parentID: "p1", title: undefined }), + session({ id: "b", parentID: "p1", title: undefined }), + ] + expect(() => findTaskSession(sessions, "p1", "task", "general")).not.toThrow() + expect(findTaskSession(sessions, "p1", "task", "general")).toBeUndefined() + }) + + test("skips title filter when description is empty", () => { + const sessions = [ + session({ id: "child-1", parentID: "p1", title: undefined }), + ] + // With empty description the startsWith filter is bypassed, but + // the includes(@agent) filter still runs on undefined title + expect(() => findTaskSession(sessions, "p1", "", "")).not.toThrow() + expect(findTaskSession(sessions, "p1", "", "")).toBe("child-1") + }) + + test("handles empty session list", () => { + expect(findTaskSession([], "p1", "desc", "general")).toBeUndefined() + }) +}) + +describe("isAmicodeToolCall", () => { + test("returns true for amicode_ prefixed tool part", () => { + expect(isAmicodeToolCall({ type: "tool", tool: "amicode_solve" })).toBe(true) + }) + + test("returns false for non-tool part", () => { + expect(isAmicodeToolCall({ type: "text", tool: "amicode_solve" })).toBe(false) + }) + + test("returns false when part is undefined", () => { + expect(isAmicodeToolCall(undefined)).toBe(false) + }) + + test("does not crash when tool property is undefined", () => { + // A tool-type part with missing tool name — should not throw + expect(() => isAmicodeToolCall({ type: "tool", tool: undefined })).not.toThrow() + expect(isAmicodeToolCall({ type: "tool", tool: undefined })).toBe(false) + }) + + test("does not crash when tool property is null", () => { + expect(() => isAmicodeToolCall({ type: "tool", tool: null })).not.toThrow() + expect(isAmicodeToolCall({ type: "tool", tool: null })).toBe(false) + }) + + test("returns false for non-amicode tool", () => { + expect(isAmicodeToolCall({ type: "tool", tool: "bash" })).toBe(false) + }) +}) diff --git a/packages/session-ui/src/components/message-part-task.ts b/packages/session-ui/src/components/message-part-task.ts new file mode 100644 index 000000000..0674bce99 --- /dev/null +++ b/packages/session-ui/src/components/message-part-task.ts @@ -0,0 +1,23 @@ +// Pure task-session matching helpers, extracted from message-part.tsx for +// testability (same pattern as message-part-text.ts / message-part-groups.ts). +// JSX-free, no DOM imports. + +/** Find the best matching child session for a Task tool-call display. */ +export function findTaskSession( + sessions: readonly { id: string; parentID?: string; title: string; time: { created?: number; archived?: number } }[], + parentID: string, + description: string, + agentName: string, +): string | undefined { + return sessions + .filter((session) => session.parentID === parentID && !session.time?.archived) + .filter((session) => (description ? session.title?.startsWith(description) : true)) + .filter((session) => (agentName ? session.title?.includes(`@${agentName}`) : true)) + .sort((a, b) => (b.time.created ?? 0) - (a.time.created ?? 0))[0]?.id +} + +/** Check whether a part-like object is an amicode_* tool call. */ +export function isAmicodeToolCall(part: { type?: string; tool?: unknown } | undefined | null): boolean { + if (!part || part.type !== "tool") return false + return typeof part.tool === "string" && part.tool.startsWith("amicode_") +} diff --git a/packages/session-ui/src/components/message-part.tsx b/packages/session-ui/src/components/message-part.tsx index 4d932c801..7aba7e336 100644 --- a/packages/session-ui/src/components/message-part.tsx +++ b/packages/session-ui/src/components/message-part.tsx @@ -655,8 +655,8 @@ function taskSession( const agent = taskAgent(input.subagent_type, agents).name return (sessions ?? []) .filter((session) => session.parentID === parentID && !session.time?.archived) - .filter((session) => (description ? session.title.startsWith(description) : true)) - .filter((session) => (agent ? session.title.includes(`@${agent}`) : true)) + .filter((session) => (description ? session.title?.startsWith(description) : true)) + .filter((session) => (agent ? session.title?.includes(`@${agent}`) : true)) .sort((a, b) => (b.time.created ?? 0) - (a.time.created ?? 0))[0]?.id } @@ -713,7 +713,7 @@ function index(items: readonly T[]) { // candidates; everything else (still running, errored, not amicode_*, no/ // unparseable sentinel) gets `key: undefined` and can never merge. function amicodeReceiptCandidateKey(part: PartType | undefined): { key?: ReceiptKey; seq?: number } { - if (!part || part.type !== "tool" || !part.tool.startsWith("amicode_")) return {} + if (!part || part.type !== "tool" || typeof part.tool !== "string" || !part.tool.startsWith("amicode_")) return {} if (part.state.status !== "completed") return {} const sentinel = parseDiffSentinel(part.state.output) return { key: receiptRunKey(sentinel), seq: sentinel?.seq }