Skip to content

Commit 4fcb3ee

Browse files
engineerCopilot
authored andcommitted
fix(app): guard diffs against non-array values to prevent .map() crash
The SessionReview component crashes with TypeError: e.diffs.map is not a function when props.diffs is not an array. This happens when SolidJS reconcile or API responses produce non-array values (e.g. {} from keyed reconcile on undefined store slots, or unexpected API responses). Added Array.isArray guards at 6 layers: - session-review.tsx: safe() wrapper around props.diffs usage - session-side-panel.tsx: guard props.diffs() in diffFiles and kinds - session.tsx: guard reviewDiffs memo return and loadVcs result - event-reducer.ts: guard SSE session.diff event data - sync.tsx: guard session diff API response Root cause: the git-backed review modes feature (35350b1) changed the default changes mode from 'session' to 'git' and introduced VCS diff loading via API. Edge cases in VCS API responses or reconcile on uninitialized store slots can produce non-array values that bypass the ?? [] fallback (which only catches null/undefined, not {}). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 943643f commit 4fcb3ee

8 files changed

Lines changed: 68 additions & 12 deletions

File tree

‎packages/app/src/context/global-sync/event-reducer.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ export function applyDirectoryEvent(input: {
169169
}
170170
case "session.diff": {
171171
const props = event.properties as { sessionID: string; diff: FileDiff[] }
172-
input.setStore("session_diff", props.sessionID, reconcile(props.diff, { key: "file" }))
172+
const safe = Array.isArray(props.diff) ? props.diff : []
173+
input.setStore("session_diff", props.sessionID, reconcile(safe, { key: "file" }))
173174
break
174175
}
175176
case "todo.updated": {

‎packages/app/src/context/sync.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,8 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
510510
return runInflight(inflightDiff, key, () =>
511511
retry(() => client.session.diff({ sessionID })).then((diff) => {
512512
if (!tracked(directory, sessionID)) return
513-
setStore("session_diff", sessionID, reconcile(diff.data ?? [], { key: "file" }))
513+
const safe = Array.isArray(diff.data) ? diff.data : []
514+
setStore("session_diff", sessionID, reconcile(safe, { key: "file" }))
514515
}),
515516
)
516517
},

‎packages/app/src/pages/session.tsx‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,8 @@ export default function Page() {
619619
.diff({ mode })
620620
.then((result) => {
621621
if (vcsRun.get(mode) !== run) return
622-
setVcs("diff", mode, result.data ?? [])
622+
const safe = Array.isArray(result.data) ? result.data : []
623+
setVcs("diff", mode, safe)
623624
setVcs("ready", mode, true)
624625
})
625626
.catch((error) => {
@@ -676,10 +677,15 @@ export default function Page() {
676677
if (store.changes === "git" || store.changes === "branch") return store.changes
677678
})
678679
const reviewDiffs = createMemo(() => {
679-
if (store.changes === "git") return vcs.diff.git
680-
if (store.changes === "branch") return vcs.diff.branch
681-
if (store.changes === "session") return diffs()
682-
return turnDiffs()
680+
const pick =
681+
store.changes === "git"
682+
? vcs.diff.git
683+
: store.changes === "branch"
684+
? vcs.diff.branch
685+
: store.changes === "session"
686+
? diffs()
687+
: turnDiffs()
688+
return Array.isArray(pick) ? pick : []
683689
})
684690
const reviewCount = createMemo(() => {
685691
if (store.changes === "git") return vcs.diff.git.length

‎packages/app/src/pages/session/session-side-panel.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ export function SessionSidePanel(props: {
5858
})
5959
const treeWidth = createMemo(() => (fileOpen() ? `${layout.fileTree.width()}px` : "0px"))
6060

61-
const diffFiles = createMemo(() => props.diffs().map((d) => d.file))
61+
const diffFiles = createMemo(() => {
62+
const raw = props.diffs()
63+
const list = Array.isArray(raw) ? raw : []
64+
return list.map((d) => d.file)
65+
})
6266
const kinds = createMemo(() => {
6367
const merge = (a: "add" | "del" | "mix" | undefined, b: "add" | "del" | "mix") => {
6468
if (!a) return b
@@ -68,8 +72,10 @@ export function SessionSidePanel(props: {
6872

6973
const normalize = (p: string) => p.replaceAll("\\\\", "/").replace(/\/+$/, "")
7074

75+
const raw = props.diffs()
76+
const list = Array.isArray(raw) ? raw : []
7177
const out = new Map<string, "add" | "del" | "mix">()
72-
for (const diff of props.diffs()) {
78+
for (const diff of list) {
7379
const file = normalize(diff.file)
7480
const kind = diff.status === "added" ? "add" : diff.status === "deleted" ? "del" : "mix"
7581

‎packages/ui/bunfig.toml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
jsx = "react-jsx"
2+
jsxImportSource = "solid-js"
3+
conditions = ["browser"]
4+
5+
[test]
6+
preload = ["./happydom.ts"]

‎packages/ui/happydom.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { GlobalRegistrator } from "@happy-dom/global-registrator"
2+
3+
GlobalRegistrator.register()
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { describe, expect, test } from "bun:test"
2+
3+
/**
4+
* The `safe()` guard in SessionReview prevents:
5+
* TypeError: e.diffs.map is not a function
6+
* when props.diffs is not an array (e.g. from reconcile edge cases or unexpected API data).
7+
*
8+
* Guard: `Array.isArray(props.diffs) ? props.diffs : []`
9+
*/
10+
describe("session-review diffs guard", () => {
11+
const guard = (v: unknown) => (Array.isArray(v) ? v : [])
12+
13+
test("returns [] for non-array inputs", () => {
14+
expect(guard(undefined)).toEqual([])
15+
expect(guard(null)).toEqual([])
16+
expect(guard({})).toEqual([])
17+
expect(guard("string")).toEqual([])
18+
expect(guard(42)).toEqual([])
19+
})
20+
21+
test("passes through valid arrays", () => {
22+
const diffs = [{ file: "a.ts", before: "", after: "x", additions: 1, deletions: 0, status: "added" }]
23+
expect(guard(diffs)).toBe(diffs)
24+
expect(guard([])).toEqual([])
25+
})
26+
27+
test(".map() works on guarded value", () => {
28+
expect(() => guard(undefined).map((d: any) => d.file)).not.toThrow()
29+
expect(() => guard({}).map((d: any) => d.file)).not.toThrow()
30+
expect(guard(undefined).map((d: any) => d.file)).toEqual([])
31+
})
32+
})

‎packages/ui/src/components/session-review.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,10 @@ export const SessionReview = (props: SessionReviewProps) => {
154154
const commenting = () => store.commenting
155155
const opened = () => store.opened
156156

157+
const safe = () => (Array.isArray(props.diffs) ? props.diffs : [])
157158
const open = () => props.open ?? store.open
158-
const files = createMemo(() => props.diffs.map((diff) => diff.file))
159-
const diffs = createMemo(() => new Map(props.diffs.map((diff) => [diff.file, diff] as const)))
159+
const files = createMemo(() => safe().map((diff) => diff.file))
160+
const diffs = createMemo(() => new Map(safe().map((diff) => [diff.file, diff] as const)))
160161
const grouped = createMemo(() => {
161162
const next = new Map<string, SessionReviewComment[]>()
162163
for (const comment of props.comments ?? []) {
@@ -359,7 +360,7 @@ export const SessionReview = (props: SessionReviewProps) => {
359360
<Show when={hasDiffs()} fallback={props.empty}>
360361
<div class="pb-6">
361362
<Accordion multiple value={open()} onChange={handleChange}>
362-
<For each={props.diffs}>
363+
<For each={safe()}>
363364
{(diff) => {
364365
let wrapper: HTMLDivElement | undefined
365366
const file = diff.file

0 commit comments

Comments
 (0)