Skip to content

Commit 689b1a4

Browse files
committed
fix(app): diff list normalization
1 parent d98be39 commit 689b1a4

File tree

6 files changed

+159
-15
lines changed

6 files changed

+159
-15
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
import type { State, VcsCache } from "./types"
1515
import { trimSessions } from "./session-trim"
1616
import { dropSessionCaches } from "./session-cache"
17+
import { diffs as list, message as clean } from "@/utils/diffs"
1718

1819
const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
1920

@@ -162,7 +163,7 @@ export function applyDirectoryEvent(input: {
162163
}
163164
case "session.diff": {
164165
const props = event.properties as { sessionID: string; diff: SnapshotFileDiff[] }
165-
input.setStore("session_diff", props.sessionID, reconcile(props.diff, { key: "file" }))
166+
input.setStore("session_diff", props.sessionID, reconcile(list(props.diff), { key: "file" }))
166167
break
167168
}
168169
case "todo.updated": {
@@ -177,7 +178,7 @@ export function applyDirectoryEvent(input: {
177178
break
178179
}
179180
case "message.updated": {
180-
const info = (event.properties as { info: Message }).info
181+
const info = clean((event.properties as { info: Message }).info)
181182
const messages = input.store.message[info.sessionID]
182183
if (!messages) {
183184
input.setStore("message", info.sessionID, [info])

packages/app/src/context/sync.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { useGlobalSync } from "./global-sync"
1313
import { useSDK } from "./sdk"
1414
import type { Message, Part } from "@opencode-ai/sdk/v2/client"
1515
import { SESSION_CACHE_LIMIT, dropSessionCaches, pickSessionCacheEvictions } from "./global-sync/session-cache"
16+
import { diffs as list, message as clean } from "@/utils/diffs"
1617

1718
const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
1819

@@ -300,7 +301,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
300301
input.client.session.messages({ sessionID: input.sessionID, limit: input.limit, before: input.before }),
301302
)
302303
const items = (messages.data ?? []).filter((x) => !!x?.info?.id)
303-
const session = items.map((x) => x.info).sort((a, b) => cmp(a.id, b.id))
304+
const session = items.map((x) => clean(x.info)).sort((a, b) => cmp(a.id, b.id))
304305
const part = items.map((message) => ({ id: message.info.id, part: sortParts(message.parts) }))
305306
const cursor = messages.response.headers.get("x-next-cursor") ?? undefined
306307
return {
@@ -509,7 +510,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
509510
return runInflight(inflightDiff, key, () =>
510511
retry(() => client.session.diff({ sessionID })).then((diff) => {
511512
if (!tracked(directory, sessionID)) return
512-
setStore("session_diff", sessionID, reconcile(diff.data ?? [], { key: "file" }))
513+
setStore("session_diff", sessionID, reconcile(list(diff.data), { key: "file" }))
513514
}),
514515
)
515516
},

packages/app/src/pages/session.tsx

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ import { TerminalPanel } from "@/pages/session/terminal-panel"
5858
import { useSessionCommands } from "@/pages/session/use-session-commands"
5959
import { useSessionHashScroll } from "@/pages/session/use-session-hash-scroll"
6060
import { Identifier } from "@/utils/id"
61+
import { diffs as list } from "@/utils/diffs"
6162
import { Persist, persisted } from "@/utils/persist"
6263
import { extractPromptFromParts } from "@/utils/prompt"
6364
import { same } from "@/utils/same"
@@ -430,7 +431,7 @@ export default function Page() {
430431

431432
const info = createMemo(() => (params.id ? sync.session.get(params.id) : undefined))
432433
const isChildSession = createMemo(() => !!info()?.parentID)
433-
const diffs = createMemo(() => (params.id ? (sync.data.session_diff[params.id] ?? []) : []))
434+
const diffs = createMemo(() => (params.id ? list(sync.data.session_diff[params.id]) : []))
434435
const sessionCount = createMemo(() => Math.max(info()?.summary?.files ?? 0, diffs().length))
435436
const hasSessionReview = createMemo(() => sessionCount() > 0)
436437
const canReview = createMemo(() => !!sync.project)
@@ -611,7 +612,7 @@ export default function Page() {
611612
.diff({ mode })
612613
.then((result) => {
613614
if (vcsRun.get(mode) !== run) return
614-
setVcs("diff", mode, result.data ?? [])
615+
setVcs("diff", mode, list(result.data))
615616
setVcs("ready", mode, true)
616617
})
617618
.catch((error) => {
@@ -649,7 +650,7 @@ export default function Page() {
649650
return open
650651
}, desktopReviewOpen())
651652

652-
const turnDiffs = createMemo(() => lastUserMessage()?.summary?.diffs ?? [])
653+
const turnDiffs = createMemo(() => list(lastUserMessage()?.summary?.diffs))
653654
const nogit = createMemo(() => !!sync.project && sync.project.vcs !== "git")
654655
const changesOptions = createMemo<ChangeMode[]>(() => {
655656
const list: ChangeMode[] = []
@@ -669,15 +670,11 @@ export default function Page() {
669670
if (store.changes === "git" || store.changes === "branch") return store.changes
670671
})
671672
const reviewDiffs = createMemo(() => {
672-
if (store.changes === "git") return vcs.diff.git
673-
if (store.changes === "branch") return vcs.diff.branch
673+
if (store.changes === "git") return list(vcs.diff.git)
674+
if (store.changes === "branch") return list(vcs.diff.branch)
674675
return turnDiffs()
675676
})
676-
const reviewCount = createMemo(() => {
677-
if (store.changes === "git") return vcs.diff.git.length
678-
if (store.changes === "branch") return vcs.diff.branch.length
679-
return turnDiffs().length
680-
})
677+
const reviewCount = createMemo(() => reviewDiffs().length)
681678
const hasReview = createMemo(() => reviewCount() > 0)
682679
const reviewReady = createMemo(() => {
683680
if (store.changes === "git") return vcs.ready.git
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, expect, test } from "bun:test"
2+
import type { SnapshotFileDiff } from "@opencode-ai/sdk/v2"
3+
import type { Message } from "@opencode-ai/sdk/v2/client"
4+
import { diffs, message } from "./diffs"
5+
6+
const item = {
7+
file: "src/app.ts",
8+
patch: "@@ -1 +1 @@\n-old\n+new\n",
9+
additions: 1,
10+
deletions: 1,
11+
status: "modified",
12+
} satisfies SnapshotFileDiff
13+
14+
describe("diffs", () => {
15+
test("keeps valid arrays", () => {
16+
expect(diffs([item])).toEqual([item])
17+
})
18+
19+
test("wraps a single diff object", () => {
20+
expect(diffs(item)).toEqual([item])
21+
})
22+
23+
test("reads keyed diff objects", () => {
24+
expect(diffs({ a: item })).toEqual([item])
25+
})
26+
27+
test("drops invalid entries", () => {
28+
expect(
29+
diffs([
30+
item,
31+
{ file: "src/bad.ts", additions: 1, deletions: 1 },
32+
{ patch: item.patch, additions: 1, deletions: 1 },
33+
]),
34+
).toEqual([item])
35+
})
36+
})
37+
38+
describe("message", () => {
39+
test("normalizes user summaries with object diffs", () => {
40+
const input = {
41+
id: "msg_1",
42+
sessionID: "ses_1",
43+
role: "user",
44+
time: { created: 1 },
45+
agent: "build",
46+
model: { providerID: "openai", modelID: "gpt-5" },
47+
summary: {
48+
title: "Edit",
49+
diffs: { a: item },
50+
},
51+
} as unknown as Message
52+
53+
expect(message(input)).toMatchObject({
54+
summary: {
55+
title: "Edit",
56+
diffs: [item],
57+
},
58+
})
59+
})
60+
61+
test("drops invalid user summaries", () => {
62+
const input = {
63+
id: "msg_1",
64+
sessionID: "ses_1",
65+
role: "user",
66+
time: { created: 1 },
67+
agent: "build",
68+
model: { providerID: "openai", modelID: "gpt-5" },
69+
summary: true,
70+
} as unknown as Message
71+
72+
expect(message(input)).toMatchObject({ summary: undefined })
73+
})
74+
})

packages/app/src/utils/diffs.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import type { SnapshotFileDiff, VcsFileDiff } from "@opencode-ai/sdk/v2"
2+
import type { Message } from "@opencode-ai/sdk/v2/client"
3+
4+
type Diff = SnapshotFileDiff | VcsFileDiff
5+
6+
function diff(value: unknown): value is Diff {
7+
if (!value || typeof value !== "object" || Array.isArray(value)) return false
8+
if (!("file" in value) || typeof value.file !== "string") return false
9+
if (!("patch" in value) || typeof value.patch !== "string") return false
10+
if (!("additions" in value) || typeof value.additions !== "number") return false
11+
if (!("deletions" in value) || typeof value.deletions !== "number") return false
12+
if (!("status" in value) || value.status === undefined) return true
13+
return value.status === "added" || value.status === "deleted" || value.status === "modified"
14+
}
15+
16+
function object(value: unknown): value is Record<string, unknown> {
17+
return !!value && typeof value === "object" && !Array.isArray(value)
18+
}
19+
20+
export function diffs(value: unknown): Diff[] {
21+
if (Array.isArray(value) && value.every(diff)) return value
22+
if (Array.isArray(value)) return value.filter(diff)
23+
if (diff(value)) return [value]
24+
if (!object(value)) return []
25+
return Object.values(value).filter(diff)
26+
}
27+
28+
export function message(value: Message): Message {
29+
if (value.role !== "user") return value
30+
31+
const raw = value.summary as unknown
32+
if (raw === undefined) return value
33+
if (!object(raw)) return { ...value, summary: undefined }
34+
35+
const title = typeof raw.title === "string" ? raw.title : undefined
36+
const body = typeof raw.body === "string" ? raw.body : undefined
37+
const next = diffs(raw.diffs)
38+
39+
if (title === raw.title && body === raw.body && next === raw.diffs) return value
40+
41+
return {
42+
...value,
43+
summary: {
44+
...(title === undefined ? {} : { title }),
45+
...(body === undefined ? {} : { body }),
46+
diffs: next,
47+
},
48+
}
49+
}

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,26 @@ export type SessionReviewFocus = { file: string; id: string }
6565
type ReviewDiff = (SnapshotFileDiff | VcsFileDiff) & { preloaded?: PreloadMultiFileDiffResult<any> }
6666
type Item = ViewDiff & { preloaded?: PreloadMultiFileDiffResult<any> }
6767

68+
function diff(value: unknown): value is ReviewDiff {
69+
if (!value || typeof value !== "object" || Array.isArray(value)) return false
70+
if (!("file" in value) || typeof value.file !== "string") return false
71+
if (!("additions" in value) || typeof value.additions !== "number") return false
72+
if (!("deletions" in value) || typeof value.deletions !== "number") return false
73+
if ("patch" in value && value.patch !== undefined && typeof value.patch !== "string") return false
74+
if ("before" in value && value.before !== undefined && typeof value.before !== "string") return false
75+
if ("after" in value && value.after !== undefined && typeof value.after !== "string") return false
76+
if (!("status" in value) || value.status === undefined) return true
77+
return value.status === "added" || value.status === "deleted" || value.status === "modified"
78+
}
79+
80+
function list(value: unknown): ReviewDiff[] {
81+
if (Array.isArray(value) && value.every(diff)) return value
82+
if (Array.isArray(value)) return value.filter(diff)
83+
if (diff(value)) return [value]
84+
if (!value || typeof value !== "object") return []
85+
return Object.values(value).filter(diff)
86+
}
87+
6888
export interface SessionReviewProps {
6989
title?: JSX.Element
7090
empty?: JSX.Element
@@ -157,7 +177,9 @@ export const SessionReview = (props: SessionReviewProps) => {
157177
const opened = () => store.opened
158178

159179
const open = () => props.open ?? store.open
160-
const items = createMemo<Item[]>(() => props.diffs.map((diff) => ({ ...normalize(diff), preloaded: diff.preloaded })))
180+
const items = createMemo<Item[]>(() =>
181+
list(props.diffs).map((diff) => ({ ...normalize(diff), preloaded: diff.preloaded })),
182+
)
161183
const files = createMemo(() => items().map((diff) => diff.file))
162184
const grouped = createMemo(() => {
163185
const next = new Map<string, SessionReviewComment[]>()

0 commit comments

Comments
 (0)