Browse Source

Merge branch 'dev' into refactor/effectify-task-tool

Kit Langton 1 week ago
parent
commit
1033d0c46c

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

@@ -14,6 +14,7 @@ import type {
 import type { State, VcsCache } from "./types"
 import { trimSessions } from "./session-trim"
 import { dropSessionCaches } from "./session-cache"
+import { diffs as list, message as clean } from "@/utils/diffs"
 
 const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
 
@@ -162,7 +163,7 @@ export function applyDirectoryEvent(input: {
     }
     case "session.diff": {
       const props = event.properties as { sessionID: string; diff: SnapshotFileDiff[] }
-      input.setStore("session_diff", props.sessionID, reconcile(props.diff, { key: "file" }))
+      input.setStore("session_diff", props.sessionID, reconcile(list(props.diff), { key: "file" }))
       break
     }
     case "todo.updated": {
@@ -177,7 +178,7 @@ export function applyDirectoryEvent(input: {
       break
     }
     case "message.updated": {
-      const info = (event.properties as { info: Message }).info
+      const info = clean((event.properties as { info: Message }).info)
       const messages = input.store.message[info.sessionID]
       if (!messages) {
         input.setStore("message", info.sessionID, [info])

+ 3 - 2
packages/app/src/context/sync.tsx

@@ -13,6 +13,7 @@ import { useGlobalSync } from "./global-sync"
 import { useSDK } from "./sdk"
 import type { Message, Part } from "@opencode-ai/sdk/v2/client"
 import { SESSION_CACHE_LIMIT, dropSessionCaches, pickSessionCacheEvictions } from "./global-sync/session-cache"
+import { diffs as list, message as clean } from "@/utils/diffs"
 
 const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
 
@@ -300,7 +301,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
         input.client.session.messages({ sessionID: input.sessionID, limit: input.limit, before: input.before }),
       )
       const items = (messages.data ?? []).filter((x) => !!x?.info?.id)
-      const session = items.map((x) => x.info).sort((a, b) => cmp(a.id, b.id))
+      const session = items.map((x) => clean(x.info)).sort((a, b) => cmp(a.id, b.id))
       const part = items.map((message) => ({ id: message.info.id, part: sortParts(message.parts) }))
       const cursor = messages.response.headers.get("x-next-cursor") ?? undefined
       return {
@@ -509,7 +510,7 @@ export const { use: useSync, provider: SyncProvider } = createSimpleContext({
           return runInflight(inflightDiff, key, () =>
             retry(() => client.session.diff({ sessionID })).then((diff) => {
               if (!tracked(directory, sessionID)) return
-              setStore("session_diff", sessionID, reconcile(diff.data ?? [], { key: "file" }))
+              setStore("session_diff", sessionID, reconcile(list(diff.data), { key: "file" }))
             }),
           )
         },

+ 7 - 10
packages/app/src/pages/session.tsx

@@ -58,6 +58,7 @@ import { TerminalPanel } from "@/pages/session/terminal-panel"
 import { useSessionCommands } from "@/pages/session/use-session-commands"
 import { useSessionHashScroll } from "@/pages/session/use-session-hash-scroll"
 import { Identifier } from "@/utils/id"
+import { diffs as list } from "@/utils/diffs"
 import { Persist, persisted } from "@/utils/persist"
 import { extractPromptFromParts } from "@/utils/prompt"
 import { same } from "@/utils/same"
@@ -430,7 +431,7 @@ export default function Page() {
 
   const info = createMemo(() => (params.id ? sync.session.get(params.id) : undefined))
   const isChildSession = createMemo(() => !!info()?.parentID)
-  const diffs = createMemo(() => (params.id ? (sync.data.session_diff[params.id] ?? []) : []))
+  const diffs = createMemo(() => (params.id ? list(sync.data.session_diff[params.id]) : []))
   const sessionCount = createMemo(() => Math.max(info()?.summary?.files ?? 0, diffs().length))
   const hasSessionReview = createMemo(() => sessionCount() > 0)
   const canReview = createMemo(() => !!sync.project)
@@ -611,7 +612,7 @@ export default function Page() {
       .diff({ mode })
       .then((result) => {
         if (vcsRun.get(mode) !== run) return
-        setVcs("diff", mode, result.data ?? [])
+        setVcs("diff", mode, list(result.data))
         setVcs("ready", mode, true)
       })
       .catch((error) => {
@@ -649,7 +650,7 @@ export default function Page() {
     return open
   }, desktopReviewOpen())
 
-  const turnDiffs = createMemo(() => lastUserMessage()?.summary?.diffs ?? [])
+  const turnDiffs = createMemo(() => list(lastUserMessage()?.summary?.diffs))
   const nogit = createMemo(() => !!sync.project && sync.project.vcs !== "git")
   const changesOptions = createMemo<ChangeMode[]>(() => {
     const list: ChangeMode[] = []
@@ -669,15 +670,11 @@ export default function Page() {
     if (store.changes === "git" || store.changes === "branch") return store.changes
   })
   const reviewDiffs = createMemo(() => {
-    if (store.changes === "git") return vcs.diff.git
-    if (store.changes === "branch") return vcs.diff.branch
+    if (store.changes === "git") return list(vcs.diff.git)
+    if (store.changes === "branch") return list(vcs.diff.branch)
     return turnDiffs()
   })
-  const reviewCount = createMemo(() => {
-    if (store.changes === "git") return vcs.diff.git.length
-    if (store.changes === "branch") return vcs.diff.branch.length
-    return turnDiffs().length
-  })
+  const reviewCount = createMemo(() => reviewDiffs().length)
   const hasReview = createMemo(() => reviewCount() > 0)
   const reviewReady = createMemo(() => {
     if (store.changes === "git") return vcs.ready.git

+ 74 - 0
packages/app/src/utils/diffs.test.ts

@@ -0,0 +1,74 @@
+import { describe, expect, test } from "bun:test"
+import type { SnapshotFileDiff } from "@opencode-ai/sdk/v2"
+import type { Message } from "@opencode-ai/sdk/v2/client"
+import { diffs, message } from "./diffs"
+
+const item = {
+  file: "src/app.ts",
+  patch: "@@ -1 +1 @@\n-old\n+new\n",
+  additions: 1,
+  deletions: 1,
+  status: "modified",
+} satisfies SnapshotFileDiff
+
+describe("diffs", () => {
+  test("keeps valid arrays", () => {
+    expect(diffs([item])).toEqual([item])
+  })
+
+  test("wraps a single diff object", () => {
+    expect(diffs(item)).toEqual([item])
+  })
+
+  test("reads keyed diff objects", () => {
+    expect(diffs({ a: item })).toEqual([item])
+  })
+
+  test("drops invalid entries", () => {
+    expect(
+      diffs([
+        item,
+        { file: "src/bad.ts", additions: 1, deletions: 1 },
+        { patch: item.patch, additions: 1, deletions: 1 },
+      ]),
+    ).toEqual([item])
+  })
+})
+
+describe("message", () => {
+  test("normalizes user summaries with object diffs", () => {
+    const input = {
+      id: "msg_1",
+      sessionID: "ses_1",
+      role: "user",
+      time: { created: 1 },
+      agent: "build",
+      model: { providerID: "openai", modelID: "gpt-5" },
+      summary: {
+        title: "Edit",
+        diffs: { a: item },
+      },
+    } as unknown as Message
+
+    expect(message(input)).toMatchObject({
+      summary: {
+        title: "Edit",
+        diffs: [item],
+      },
+    })
+  })
+
+  test("drops invalid user summaries", () => {
+    const input = {
+      id: "msg_1",
+      sessionID: "ses_1",
+      role: "user",
+      time: { created: 1 },
+      agent: "build",
+      model: { providerID: "openai", modelID: "gpt-5" },
+      summary: true,
+    } as unknown as Message
+
+    expect(message(input)).toMatchObject({ summary: undefined })
+  })
+})

+ 49 - 0
packages/app/src/utils/diffs.ts

@@ -0,0 +1,49 @@
+import type { SnapshotFileDiff, VcsFileDiff } from "@opencode-ai/sdk/v2"
+import type { Message } from "@opencode-ai/sdk/v2/client"
+
+type Diff = SnapshotFileDiff | VcsFileDiff
+
+function diff(value: unknown): value is Diff {
+  if (!value || typeof value !== "object" || Array.isArray(value)) return false
+  if (!("file" in value) || typeof value.file !== "string") return false
+  if (!("patch" in value) || typeof value.patch !== "string") return false
+  if (!("additions" in value) || typeof value.additions !== "number") return false
+  if (!("deletions" in value) || typeof value.deletions !== "number") return false
+  if (!("status" in value) || value.status === undefined) return true
+  return value.status === "added" || value.status === "deleted" || value.status === "modified"
+}
+
+function object(value: unknown): value is Record<string, unknown> {
+  return !!value && typeof value === "object" && !Array.isArray(value)
+}
+
+export function diffs(value: unknown): Diff[] {
+  if (Array.isArray(value) && value.every(diff)) return value
+  if (Array.isArray(value)) return value.filter(diff)
+  if (diff(value)) return [value]
+  if (!object(value)) return []
+  return Object.values(value).filter(diff)
+}
+
+export function message(value: Message): Message {
+  if (value.role !== "user") return value
+
+  const raw = value.summary as unknown
+  if (raw === undefined) return value
+  if (!object(raw)) return { ...value, summary: undefined }
+
+  const title = typeof raw.title === "string" ? raw.title : undefined
+  const body = typeof raw.body === "string" ? raw.body : undefined
+  const next = diffs(raw.diffs)
+
+  if (title === raw.title && body === raw.body && next === raw.diffs) return value
+
+  return {
+    ...value,
+    summary: {
+      ...(title === undefined ? {} : { title }),
+      ...(body === undefined ? {} : { body }),
+      diffs: next,
+    },
+  }
+}

+ 2 - 1
packages/opencode/src/provider/transform.ts

@@ -376,7 +376,8 @@ export namespace ProviderTransform {
       id.includes("mistral") ||
       id.includes("kimi") ||
       id.includes("k2p5") ||
-      id.includes("qwen")
+      id.includes("qwen") ||
+      id.includes("big-pickle")
     )
       return {}
 

+ 43 - 0
packages/ui/src/components/apply-patch-file.test.ts

@@ -0,0 +1,43 @@
+import { describe, expect, test } from "bun:test"
+import { patchFiles } from "./apply-patch-file"
+import { text } from "./session-diff"
+
+describe("apply patch file", () => {
+  test("parses patch metadata from the server", () => {
+    const file = patchFiles([
+      {
+        filePath: "/tmp/a.ts",
+        relativePath: "a.ts",
+        type: "update",
+        patch:
+          "Index: a.ts\n===================================================================\n--- a.ts\t\n+++ a.ts\t\n@@ -1,2 +1,2 @@\n one\n-two\n+three\n",
+        additions: 1,
+        deletions: 1,
+      },
+    ])[0]
+
+    expect(file).toBeDefined()
+    expect(file?.view.fileDiff.name).toBe("a.ts")
+    expect(text(file!.view, "deletions")).toBe("one\ntwo\n")
+    expect(text(file!.view, "additions")).toBe("one\nthree\n")
+  })
+
+  test("keeps legacy before and after payloads working", () => {
+    const file = patchFiles([
+      {
+        filePath: "/tmp/a.ts",
+        relativePath: "a.ts",
+        type: "update",
+        before: "one\n",
+        after: "two\n",
+        additions: 1,
+        deletions: 1,
+      },
+    ])[0]
+
+    expect(file).toBeDefined()
+    expect(file?.view.patch).toContain("@@ -1,1 +1,1 @@")
+    expect(text(file!.view, "deletions")).toBe("one\n")
+    expect(text(file!.view, "additions")).toBe("two\n")
+  })
+})

+ 78 - 0
packages/ui/src/components/apply-patch-file.ts

@@ -0,0 +1,78 @@
+import { normalize, type ViewDiff } from "./session-diff"
+
+type Kind = "add" | "update" | "delete" | "move"
+
+type Raw = {
+  filePath?: string
+  relativePath?: string
+  type?: Kind
+  patch?: string
+  diff?: string
+  before?: string
+  after?: string
+  additions?: number
+  deletions?: number
+  movePath?: string
+}
+
+export type ApplyPatchFile = {
+  filePath: string
+  relativePath: string
+  type: Kind
+  additions: number
+  deletions: number
+  movePath?: string
+  view: ViewDiff
+}
+
+function kind(value: unknown) {
+  if (value === "add" || value === "update" || value === "delete" || value === "move") return value
+}
+
+function status(type: Kind): "added" | "deleted" | "modified" {
+  if (type === "add") return "added"
+  if (type === "delete") return "deleted"
+  return "modified"
+}
+
+export function patchFile(raw: unknown): ApplyPatchFile | undefined {
+  if (!raw || typeof raw !== "object") return
+
+  const value = raw as Raw
+  const type = kind(value.type)
+  const filePath = typeof value.filePath === "string" ? value.filePath : undefined
+  const relativePath = typeof value.relativePath === "string" ? value.relativePath : filePath
+  const patch = typeof value.patch === "string" ? value.patch : typeof value.diff === "string" ? value.diff : undefined
+  const before = typeof value.before === "string" ? value.before : undefined
+  const after = typeof value.after === "string" ? value.after : undefined
+
+  if (!type || !filePath || !relativePath) return
+  if (!patch && before === undefined && after === undefined) return
+
+  const additions = typeof value.additions === "number" ? value.additions : 0
+  const deletions = typeof value.deletions === "number" ? value.deletions : 0
+  const movePath = typeof value.movePath === "string" ? value.movePath : undefined
+
+  return {
+    filePath,
+    relativePath,
+    type,
+    additions,
+    deletions,
+    movePath,
+    view: normalize({
+      file: relativePath,
+      patch,
+      before,
+      after,
+      additions,
+      deletions,
+      status: status(type),
+    }),
+  }
+}
+
+export function patchFiles(raw: unknown) {
+  if (!Array.isArray(raw)) return []
+  return raw.map(patchFile).filter((file): file is ApplyPatchFile => !!file)
+}

+ 4 - 25
packages/ui/src/components/message-part.tsx

@@ -54,6 +54,7 @@ import { Spinner } from "./spinner"
 import { TextShimmer } from "./text-shimmer"
 import { AnimatedCountList } from "./tool-count-summary"
 import { ToolStatusTitle } from "./tool-status-title"
+import { patchFiles } from "./apply-patch-file"
 import { animate } from "motion"
 import { useLocation } from "@solidjs/router"
 import { attached, inline, kind } from "./message-file"
@@ -2014,24 +2015,12 @@ ToolRegistry.register({
   },
 })
 
-interface ApplyPatchFile {
-  filePath: string
-  relativePath: string
-  type: "add" | "update" | "delete" | "move"
-  diff: string
-  before: string
-  after: string
-  additions: number
-  deletions: number
-  movePath?: string
-}
-
 ToolRegistry.register({
   name: "apply_patch",
   render(props) {
     const i18n = useI18n()
     const fileComponent = useFileComponent()
-    const files = createMemo(() => (props.metadata.files ?? []) as ApplyPatchFile[])
+    const files = createMemo(() => patchFiles(props.metadata.files))
     const pending = createMemo(() => props.status === "pending" || props.status === "running")
     const single = createMemo(() => {
       const list = files()
@@ -2137,12 +2126,7 @@ ToolRegistry.register({
                           <Accordion.Content>
                             <Show when={visible()}>
                               <div data-component="apply-patch-file-diff">
-                                <Dynamic
-                                  component={fileComponent}
-                                  mode="diff"
-                                  before={{ name: file.filePath, contents: file.before }}
-                                  after={{ name: file.movePath ?? file.filePath, contents: file.after }}
-                                />
+                                <Dynamic component={fileComponent} mode="diff" fileDiff={file.view.fileDiff} />
                               </div>
                             </Show>
                           </Accordion.Content>
@@ -2212,12 +2196,7 @@ ToolRegistry.register({
               }
             >
               <div data-component="apply-patch-file-diff">
-                <Dynamic
-                  component={fileComponent}
-                  mode="diff"
-                  before={{ name: single()!.filePath, contents: single()!.before }}
-                  after={{ name: single()!.movePath ?? single()!.filePath, contents: single()!.after }}
-                />
+                <Dynamic component={fileComponent} mode="diff" fileDiff={single()!.view.fileDiff} />
               </div>
             </ToolFileAccordion>
           </BasicTool>

+ 23 - 1
packages/ui/src/components/session-review.tsx

@@ -65,6 +65,26 @@ export type SessionReviewFocus = { file: string; id: string }
 type ReviewDiff = (SnapshotFileDiff | VcsFileDiff) & { preloaded?: PreloadMultiFileDiffResult<any> }
 type Item = ViewDiff & { preloaded?: PreloadMultiFileDiffResult<any> }
 
+function diff(value: unknown): value is ReviewDiff {
+  if (!value || typeof value !== "object" || Array.isArray(value)) return false
+  if (!("file" in value) || typeof value.file !== "string") return false
+  if (!("additions" in value) || typeof value.additions !== "number") return false
+  if (!("deletions" in value) || typeof value.deletions !== "number") return false
+  if ("patch" in value && value.patch !== undefined && typeof value.patch !== "string") return false
+  if ("before" in value && value.before !== undefined && typeof value.before !== "string") return false
+  if ("after" in value && value.after !== undefined && typeof value.after !== "string") return false
+  if (!("status" in value) || value.status === undefined) return true
+  return value.status === "added" || value.status === "deleted" || value.status === "modified"
+}
+
+function list(value: unknown): ReviewDiff[] {
+  if (Array.isArray(value) && value.every(diff)) return value
+  if (Array.isArray(value)) return value.filter(diff)
+  if (diff(value)) return [value]
+  if (!value || typeof value !== "object") return []
+  return Object.values(value).filter(diff)
+}
+
 export interface SessionReviewProps {
   title?: JSX.Element
   empty?: JSX.Element
@@ -157,7 +177,9 @@ export const SessionReview = (props: SessionReviewProps) => {
   const opened = () => store.opened
 
   const open = () => props.open ?? store.open
-  const items = createMemo<Item[]>(() => props.diffs.map((diff) => ({ ...normalize(diff), preloaded: diff.preloaded })))
+  const items = createMemo<Item[]>(() =>
+    list(props.diffs).map((diff) => ({ ...normalize(diff), preloaded: diff.preloaded })),
+  )
   const files = createMemo(() => items().map((diff) => diff.file))
   const grouped = createMemo(() => {
     const next = new Map<string, SessionReviewComment[]>()