fix(session): accept legacy summary diffs (#26579)
Co-authored-by: Developer <temp@example.com>
This commit is contained in:
@@ -28,6 +28,12 @@ import { createOpenSessionFileTab, createSessionTabs, getTabReorderIndex, type S
|
|||||||
import { setSessionHandoff } from "@/pages/session/handoff"
|
import { setSessionHandoff } from "@/pages/session/handoff"
|
||||||
import { useSessionLayout } from "@/pages/session/session-layout"
|
import { useSessionLayout } from "@/pages/session/session-layout"
|
||||||
|
|
||||||
|
type RenderDiff = (SnapshotFileDiff & { file: string }) | VcsFileDiff
|
||||||
|
|
||||||
|
function renderDiff(value: SnapshotFileDiff | VcsFileDiff): value is RenderDiff {
|
||||||
|
return typeof value.file === "string"
|
||||||
|
}
|
||||||
|
|
||||||
export function SessionSidePanel(props: {
|
export function SessionSidePanel(props: {
|
||||||
canReview: () => boolean
|
canReview: () => boolean
|
||||||
diffs: () => (SnapshotFileDiff | VcsFileDiff)[]
|
diffs: () => (SnapshotFileDiff | VcsFileDiff)[]
|
||||||
@@ -70,7 +76,8 @@ export function SessionSidePanel(props: {
|
|||||||
})
|
})
|
||||||
const treeWidth = createMemo(() => (fileOpen() ? `${layout.fileTree.width()}px` : "0px"))
|
const treeWidth = createMemo(() => (fileOpen() ? `${layout.fileTree.width()}px` : "0px"))
|
||||||
|
|
||||||
const diffFiles = createMemo(() => props.diffs().map((d) => d.file))
|
const diffs = createMemo(() => props.diffs().filter(renderDiff))
|
||||||
|
const diffFiles = createMemo(() => diffs().map((d) => d.file))
|
||||||
const kinds = createMemo(() => {
|
const kinds = createMemo(() => {
|
||||||
const merge = (a: "add" | "del" | "mix" | undefined, b: "add" | "del" | "mix") => {
|
const merge = (a: "add" | "del" | "mix" | undefined, b: "add" | "del" | "mix") => {
|
||||||
if (!a) return b
|
if (!a) return b
|
||||||
@@ -81,7 +88,7 @@ export function SessionSidePanel(props: {
|
|||||||
const normalize = (p: string) => p.replaceAll("\\\\", "/").replace(/\/+$/, "")
|
const normalize = (p: string) => p.replaceAll("\\\\", "/").replace(/\/+$/, "")
|
||||||
|
|
||||||
const out = new Map<string, "add" | "del" | "mix">()
|
const out = new Map<string, "add" | "del" | "mix">()
|
||||||
for (const diff of props.diffs()) {
|
for (const diff of diffs()) {
|
||||||
const file = normalize(diff.file)
|
const file = normalize(diff.file)
|
||||||
const kind = diff.status === "added" ? "add" : diff.status === "deleted" ? "del" : "mix"
|
const kind = diff.status === "added" ? "add" : diff.status === "deleted" ? "del" : "mix"
|
||||||
|
|
||||||
|
|||||||
@@ -23,10 +23,10 @@ function span(id: string, value: { value: string; start: number; end: number })
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function diff(kind: string, diffs: { file: string; patch?: string }[] | undefined) {
|
function diff(kind: string, diffs: { file?: string; patch?: string }[] | undefined) {
|
||||||
return diffs?.map((item, i) => ({
|
return diffs?.map((item, i) => ({
|
||||||
...item,
|
...item,
|
||||||
file: redact(`${kind}-file`, String(i), item.file),
|
file: item.file === undefined ? undefined : redact(`${kind}-file`, String(i), item.file),
|
||||||
patch: item.patch === undefined ? undefined : redact(`${kind}-patch`, String(i), item.patch),
|
patch: item.patch === undefined ? undefined : redact(`${kind}-patch`, String(i), item.patch),
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -148,7 +148,9 @@ function stateApi(sync: ReturnType<typeof useSync>): TuiPluginApi["state"] {
|
|||||||
return sync.data.session.length
|
return sync.data.session.length
|
||||||
},
|
},
|
||||||
diff(sessionID) {
|
diff(sessionID) {
|
||||||
return sync.data.session_diff[sessionID] ?? []
|
return (sync.data.session_diff[sessionID] ?? []).flatMap((item) =>
|
||||||
|
item.file === undefined ? [] : [{ ...item, file: item.file }],
|
||||||
|
)
|
||||||
},
|
},
|
||||||
todo(sessionID) {
|
todo(sessionID) {
|
||||||
return sync.data.todo[sessionID] ?? []
|
return sync.data.todo[sessionID] ?? []
|
||||||
|
|||||||
@@ -134,6 +134,7 @@ export const layer = Layer.effect(
|
|||||||
.read<Snapshot.FileDiff[]>(["session_diff", input.sessionID])
|
.read<Snapshot.FileDiff[]>(["session_diff", input.sessionID])
|
||||||
.pipe(Effect.catch(() => Effect.succeed([] as Snapshot.FileDiff[])))
|
.pipe(Effect.catch(() => Effect.succeed([] as Snapshot.FileDiff[])))
|
||||||
const next = diffs.map((item) => {
|
const next = diffs.map((item) => {
|
||||||
|
if (item.file === undefined) return item
|
||||||
const file = unquoteGitPath(item.file)
|
const file = unquoteGitPath(item.file)
|
||||||
if (file === item.file) return item
|
if (file === item.file) return item
|
||||||
return { ...item, file }
|
return { ...item, file }
|
||||||
|
|||||||
@@ -20,10 +20,10 @@ export const Patch = Schema.Struct({
|
|||||||
export type Patch = typeof Patch.Type
|
export type Patch = typeof Patch.Type
|
||||||
|
|
||||||
export const FileDiff = Schema.Struct({
|
export const FileDiff = Schema.Struct({
|
||||||
file: Schema.String,
|
|
||||||
// Optional because legacy/imported `summary_diffs` on disk may omit
|
// Optional because legacy/imported `summary_diffs` on disk may omit
|
||||||
// the patch text (see #26574). Required-Schema rejected the whole
|
// file details and patch text. Required Schema rejected the whole
|
||||||
// /session/<id>/diff response and broke session loading on Desktop.
|
// session response and broke session loading on Desktop.
|
||||||
|
file: Schema.optional(Schema.String),
|
||||||
patch: Schema.optional(Schema.String),
|
patch: Schema.optional(Schema.String),
|
||||||
additions: NonNegativeInt,
|
additions: NonNegativeInt,
|
||||||
deletions: NonNegativeInt,
|
deletions: NonNegativeInt,
|
||||||
|
|||||||
@@ -297,6 +297,36 @@ describe("session HttpApi", () => {
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
it.live(
|
||||||
|
"serves sessions with migrated summary diffs missing file details",
|
||||||
|
withTmp({ git: true, config: { formatter: false, lsp: false } }, (tmp) =>
|
||||||
|
Effect.gen(function* () {
|
||||||
|
const session = yield* createSession(tmp.path, { title: "legacy diff" })
|
||||||
|
yield* Effect.sync(() =>
|
||||||
|
Database.use((db) =>
|
||||||
|
db
|
||||||
|
.update(SessionTable)
|
||||||
|
.set({
|
||||||
|
summary_additions: 1,
|
||||||
|
summary_deletions: 0,
|
||||||
|
summary_files: 1,
|
||||||
|
summary_diffs: [{ additions: 1, deletions: 0 }],
|
||||||
|
})
|
||||||
|
.where(eq(SessionTable.id, session.id))
|
||||||
|
.run(),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
const response = yield* request(pathFor(SessionPaths.get, { sessionID: session.id }), {
|
||||||
|
headers: { "x-opencode-directory": tmp.path },
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(response.status).toBe(200)
|
||||||
|
expect((yield* json<Session.Info>(response)).summary?.diffs).toEqual([{ additions: 1, deletions: 0 }])
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
it.live(
|
it.live(
|
||||||
"serves lifecycle mutation routes",
|
"serves lifecycle mutation routes",
|
||||||
withTmp({ git: true, config: { formatter: false, lsp: false, share: "disabled" } }, (tmp) =>
|
withTmp({ git: true, config: { formatter: false, lsp: false, share: "disabled" } }, (tmp) =>
|
||||||
|
|||||||
@@ -83,6 +83,26 @@ describe("Session.Info", () => {
|
|||||||
expect(Session.Info.zod.parse(input)).toEqual(input)
|
expect(Session.Info.zod.parse(input)).toEqual(input)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("accepts migrated summary diffs without file details", () => {
|
||||||
|
const input = {
|
||||||
|
id: sessionID,
|
||||||
|
slug: "legacy-diff",
|
||||||
|
projectID,
|
||||||
|
directory: "/tmp/proj",
|
||||||
|
title: "Legacy diff",
|
||||||
|
version: "0.1.0",
|
||||||
|
summary: {
|
||||||
|
additions: 1,
|
||||||
|
deletions: 0,
|
||||||
|
files: 1,
|
||||||
|
diffs: [{ additions: 1, deletions: 0 }],
|
||||||
|
},
|
||||||
|
time: { created: 1, updated: 2 },
|
||||||
|
}
|
||||||
|
expect(decode(input)).toEqual(input)
|
||||||
|
expect(Session.Info.zod.parse(input)).toEqual(input)
|
||||||
|
})
|
||||||
|
|
||||||
test("rejects unbranded session id", () => {
|
test("rejects unbranded session id", () => {
|
||||||
const bad = { id: "not-a-session-id" } as unknown
|
const bad = { id: "not-a-session-id" } as unknown
|
||||||
expect(() => decode(bad)).toThrow()
|
expect(() => decode(bad)).toThrow()
|
||||||
|
|||||||
@@ -238,7 +238,7 @@ it.live("tool execution produces non-empty session diff (snapshot race)", () =>
|
|||||||
expect(tool?.state.status).toBe("completed")
|
expect(tool?.state.status).toBe("completed")
|
||||||
|
|
||||||
// Poll for diff — summarize() is fire-and-forget
|
// Poll for diff — summarize() is fire-and-forget
|
||||||
let diff: Array<{ file: string }> = []
|
let diff: Array<{ file?: string }> = []
|
||||||
for (let i = 0; i < 50; i++) {
|
for (let i = 0; i < 50; i++) {
|
||||||
diff = yield* summary.diff({ sessionID: session.id })
|
diff = yield* summary.diff({ sessionID: session.id })
|
||||||
if (diff.length > 0) break
|
if (diff.length > 0) break
|
||||||
|
|||||||
@@ -119,7 +119,7 @@ export type PermissionRequest = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export type SnapshotFileDiff = {
|
export type SnapshotFileDiff = {
|
||||||
file: string
|
file?: string
|
||||||
patch?: string
|
patch?: string
|
||||||
additions: number
|
additions: number
|
||||||
deletions: number
|
deletions: number
|
||||||
|
|||||||
@@ -12,7 +12,8 @@ type LegacyDiff = {
|
|||||||
status?: "added" | "deleted" | "modified"
|
status?: "added" | "deleted" | "modified"
|
||||||
}
|
}
|
||||||
|
|
||||||
type ReviewDiff = SnapshotFileDiff | VcsFileDiff | LegacyDiff
|
type SnapshotDiff = SnapshotFileDiff & { file: string }
|
||||||
|
type ReviewDiff = SnapshotDiff | VcsFileDiff | LegacyDiff
|
||||||
|
|
||||||
export type ViewDiff = {
|
export type ViewDiff = {
|
||||||
file: string
|
file: string
|
||||||
|
|||||||
@@ -62,7 +62,12 @@ export type SessionReviewCommentActions = {
|
|||||||
|
|
||||||
export type SessionReviewFocus = { file: string; id: string }
|
export type SessionReviewFocus = { file: string; id: string }
|
||||||
|
|
||||||
type ReviewDiff = (SnapshotFileDiff | VcsFileDiff) & { preloaded?: PreloadMultiFileDiffResult<any> }
|
type RawReviewDiff = (SnapshotFileDiff | VcsFileDiff) & {
|
||||||
|
preloaded?: PreloadMultiFileDiffResult<any>
|
||||||
|
}
|
||||||
|
type ReviewDiff = ((SnapshotFileDiff & { file: string }) | VcsFileDiff) & {
|
||||||
|
preloaded?: PreloadMultiFileDiffResult<any>
|
||||||
|
}
|
||||||
type Item = ViewDiff & { preloaded?: PreloadMultiFileDiffResult<any> }
|
type Item = ViewDiff & { preloaded?: PreloadMultiFileDiffResult<any> }
|
||||||
|
|
||||||
function diff(value: unknown): value is ReviewDiff {
|
function diff(value: unknown): value is ReviewDiff {
|
||||||
@@ -108,7 +113,7 @@ export interface SessionReviewProps {
|
|||||||
classList?: Record<string, boolean | undefined>
|
classList?: Record<string, boolean | undefined>
|
||||||
classes?: { root?: string; header?: string; container?: string }
|
classes?: { root?: string; header?: string; container?: string }
|
||||||
actions?: JSX.Element
|
actions?: JSX.Element
|
||||||
diffs: ReviewDiff[]
|
diffs: RawReviewDiff[]
|
||||||
onViewFile?: (file: string) => void
|
onViewFile?: (file: string) => void
|
||||||
readFile?: (path: string) => Promise<FileContent | undefined>
|
readFile?: (path: string) => Promise<FileContent | undefined>
|
||||||
lineCommentMention?: LineCommentEditorProps["mention"]
|
lineCommentMention?: LineCommentEditorProps["mention"]
|
||||||
|
|||||||
@@ -90,6 +90,12 @@ function list<T>(value: T[] | undefined | null, fallback: T[]) {
|
|||||||
return fallback
|
return fallback
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type SummaryDiff = SnapshotFileDiff & { file: string }
|
||||||
|
|
||||||
|
function summaryDiff(value: SnapshotFileDiff): value is SummaryDiff {
|
||||||
|
return typeof value.file === "string"
|
||||||
|
}
|
||||||
|
|
||||||
const hidden = new Set(["todowrite"])
|
const hidden = new Set(["todowrite"])
|
||||||
|
|
||||||
function partState(part: PartType, showReasoningSummaries: boolean) {
|
function partState(part: PartType, showReasoningSummaries: boolean) {
|
||||||
@@ -169,7 +175,7 @@ export function SessionTurn(
|
|||||||
const emptyMessages: MessageType[] = []
|
const emptyMessages: MessageType[] = []
|
||||||
const emptyParts: PartType[] = []
|
const emptyParts: PartType[] = []
|
||||||
const emptyAssistant: AssistantMessage[] = []
|
const emptyAssistant: AssistantMessage[] = []
|
||||||
const emptyDiffs: SnapshotFileDiff[] = []
|
const emptyDiffs: SummaryDiff[] = []
|
||||||
const idle = { type: "idle" as const }
|
const idle = { type: "idle" as const }
|
||||||
|
|
||||||
const allMessages = createMemo(() => props.messages ?? list(data.store.message?.[props.sessionID], emptyMessages))
|
const allMessages = createMemo(() => props.messages ?? list(data.store.message?.[props.sessionID], emptyMessages))
|
||||||
@@ -238,7 +244,8 @@ export function SessionTurn(
|
|||||||
|
|
||||||
const seen = new Set<string>()
|
const seen = new Set<string>()
|
||||||
return files
|
return files
|
||||||
.reduceRight<SnapshotFileDiff[]>((result, diff) => {
|
.reduceRight<SummaryDiff[]>((result, diff) => {
|
||||||
|
if (!summaryDiff(diff)) return result
|
||||||
if (seen.has(diff.file)) return result
|
if (seen.has(diff.file)) return result
|
||||||
seen.add(diff.file)
|
seen.add(diff.file)
|
||||||
result.push(diff)
|
result.push(diff)
|
||||||
|
|||||||
Reference in New Issue
Block a user