fix: lighten session list payloads (#1538)
* lighten session list * fetch full session on open * sanitize session list * sanitize global sessions * preserve revert markers in session lists * fix: preserve session metadata in list sanitizers --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
Hristo Karamanliev
committed by
GitHub
GitHub
co-authored by
Bohdan Triapitsyn
Bohdan Triapitsyn
parent
1c274d0a56
commit
9685630436
@@ -0,0 +1,154 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import type { Session } from '@opencode-ai/sdk/v2'
|
||||
|
||||
import { stripSessionDiffSnapshots, stripSessionListDetails } from './sanitize'
|
||||
|
||||
describe('stripSessionDiffSnapshots', () => {
|
||||
test('removes oversized revert and summary diff payloads', () => {
|
||||
const session = {
|
||||
id: 'ses_1',
|
||||
slug: 'session-one',
|
||||
projectID: 'proj_1',
|
||||
directory: '/repo/app',
|
||||
title: 'Session',
|
||||
version: '1.0.0',
|
||||
time: { created: 1, updated: 2 },
|
||||
revert: {
|
||||
messageID: 'msg_2',
|
||||
partID: 'part_3',
|
||||
snapshot: 'gitsha',
|
||||
diff: 'diff --git a/file b/file',
|
||||
},
|
||||
summary: {
|
||||
additions: 2,
|
||||
deletions: 1,
|
||||
files: 1,
|
||||
diffs: [{ additions: 2, deletions: 1, before: 'a', after: 'b', patch: '@@ -1 +1 @@' }],
|
||||
},
|
||||
} as unknown as Session
|
||||
|
||||
const next = stripSessionDiffSnapshots(session) as Session & {
|
||||
revert?: { messageID?: string; partID?: string; snapshot?: string; diff?: string }
|
||||
summary?: { diffs?: Array<{ before?: string; after?: string; patch?: string }> }
|
||||
}
|
||||
|
||||
expect(next).not.toBe(session)
|
||||
expect(next.revert).toEqual({ messageID: 'msg_2', partID: 'part_3' })
|
||||
expect(next.summary?.diffs).toEqual([{ additions: 2, deletions: 1 }])
|
||||
})
|
||||
|
||||
test('preserves object identity when nothing changes', () => {
|
||||
const session = {
|
||||
id: 'ses_1',
|
||||
slug: 'session-one',
|
||||
projectID: 'proj_1',
|
||||
directory: '/repo/app',
|
||||
title: 'Session',
|
||||
version: '1.0.0',
|
||||
time: { created: 1, updated: 2 },
|
||||
revert: { messageID: 'msg_2', partID: 'part_3' },
|
||||
summary: { additions: 2, deletions: 1, diffs: [{ additions: 2, deletions: 1 }] },
|
||||
} as unknown as Session
|
||||
|
||||
expect(stripSessionDiffSnapshots(session)).toBe(session)
|
||||
})
|
||||
})
|
||||
|
||||
describe('stripSessionListDetails', () => {
|
||||
test('removes detail-only fields from session list records', () => {
|
||||
const session = {
|
||||
id: 'ses_1',
|
||||
slug: 'session-one',
|
||||
projectID: 'proj_1',
|
||||
directory: '/repo/app',
|
||||
title: 'Session',
|
||||
time: { created: 1, updated: 2 },
|
||||
metadata: {
|
||||
openchamber: {
|
||||
kind: 'review',
|
||||
originalSessionID: 'ses_original',
|
||||
},
|
||||
},
|
||||
permission: [{ permission: 'todowrite' }],
|
||||
revert: {
|
||||
messageID: 'msg_2',
|
||||
partID: 'part_3',
|
||||
snapshot: 'gitsha',
|
||||
diff: 'diff --git a/file b/file',
|
||||
},
|
||||
summary: {
|
||||
additions: 2,
|
||||
deletions: 1,
|
||||
files: 1,
|
||||
diffs: [{ additions: 2, deletions: 1, patch: '@@ -1 +1 @@' }],
|
||||
},
|
||||
} as unknown as Session
|
||||
|
||||
const next = stripSessionListDetails(session) as Session & {
|
||||
metadata?: unknown
|
||||
permission?: unknown
|
||||
revert?: { messageID?: string; partID?: string; snapshot?: string; diff?: string }
|
||||
summary?: { additions?: number; deletions?: number; files?: number; diffs?: unknown[] }
|
||||
}
|
||||
|
||||
expect(next).not.toBe(session)
|
||||
expect(next.metadata).toEqual({
|
||||
openchamber: {
|
||||
kind: 'review',
|
||||
originalSessionID: 'ses_original',
|
||||
},
|
||||
})
|
||||
expect(next.permission).toBe(undefined)
|
||||
expect(next.revert).toEqual({ messageID: 'msg_2', partID: 'part_3' })
|
||||
expect(next.summary).toEqual({ additions: 2, deletions: 1, files: 1 })
|
||||
})
|
||||
|
||||
test('preserves metadata extension fields in session list records', () => {
|
||||
const session = {
|
||||
id: 'ses_1',
|
||||
directory: '/repo/app',
|
||||
title: 'Session',
|
||||
time: { created: 1, updated: 2 },
|
||||
metadata: { custom: { value: 'kept' } },
|
||||
summary: { additions: 2, deletions: 1, files: 1, diffs: [{ patch: '@@ -1 +1 @@' }] },
|
||||
} as unknown as Session
|
||||
|
||||
const next = stripSessionListDetails(session) as Session & {
|
||||
metadata?: unknown
|
||||
summary?: { diffs?: unknown[] }
|
||||
}
|
||||
|
||||
expect(next).not.toBe(session)
|
||||
expect(next.metadata).toEqual({ custom: { value: 'kept' } })
|
||||
expect(next.summary?.diffs).toBe(undefined)
|
||||
})
|
||||
|
||||
test('keeps summary fields other than list-only diffs', () => {
|
||||
const session = {
|
||||
id: 'ses_1',
|
||||
directory: '/repo/app',
|
||||
title: 'Session',
|
||||
time: { created: 1, updated: 2 },
|
||||
summary: { custom: 'kept', diffs: [{ patch: '@@ -1 +1 @@' }] },
|
||||
} as unknown as Session
|
||||
|
||||
const next = stripSessionListDetails(session) as Session & {
|
||||
summary?: { custom?: string; diffs?: unknown[] }
|
||||
}
|
||||
|
||||
expect(next.summary).toEqual({ custom: 'kept' })
|
||||
})
|
||||
|
||||
test('preserves object identity for already lightweight records with revert markers', () => {
|
||||
const session = {
|
||||
id: 'ses_1',
|
||||
directory: '/repo/app',
|
||||
title: 'Session',
|
||||
time: { created: 1, updated: 2 },
|
||||
revert: { messageID: 'msg_2', partID: 'part_3' },
|
||||
summary: { additions: 2, deletions: 1, files: 1 },
|
||||
} as unknown as Session
|
||||
|
||||
expect(stripSessionListDetails(session)).toBe(session)
|
||||
})
|
||||
})
|
||||
@@ -2,13 +2,16 @@
|
||||
// Payload sanitization — strip oversized diff snapshot fields client-side.
|
||||
//
|
||||
// OpenCode session/message snapshots may carry large full-content diff fields
|
||||
// (legacy before/after or from/to). The UI never uses these fields but they
|
||||
// waste browser memory and
|
||||
// can crash tabs for large sessions.
|
||||
// (legacy before/after or from/to). Revert snapshots and diff text can also
|
||||
// carry large file snapshots. The UI derives reverted-state behavior from the
|
||||
// lightweight messageID/partID markers, so these blob fields are intentionally
|
||||
// kept out of client stores.
|
||||
//
|
||||
// Applied at two points:
|
||||
// 1. Event reducer — session.created/session.updated events
|
||||
// 2. Message loading — fetchMessages response
|
||||
// 3. Session list loading — list responses should not populate stores with
|
||||
// detail-only revert/diff blobs
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
import type { Session, Message } from "@opencode-ai/sdk/v2/client"
|
||||
@@ -29,32 +32,134 @@ type SessionSummary = {
|
||||
[key: string]: unknown
|
||||
}
|
||||
|
||||
type SessionRevert = {
|
||||
messageID?: string
|
||||
partID?: string
|
||||
snapshot?: string
|
||||
diff?: string
|
||||
[key: string]: unknown
|
||||
}
|
||||
|
||||
const getSessionListRevertMarker = (revert: unknown): Pick<SessionRevert, "messageID" | "partID"> | undefined => {
|
||||
if (!revert || typeof revert !== "object" || Array.isArray(revert)) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const marker: Pick<SessionRevert, "messageID" | "partID"> = {}
|
||||
const record = revert as SessionRevert
|
||||
if (typeof record.messageID === "string") {
|
||||
marker.messageID = record.messageID
|
||||
}
|
||||
if (typeof record.partID === "string") {
|
||||
marker.partID = record.partID
|
||||
}
|
||||
|
||||
return Object.keys(marker).length > 0 ? marker : undefined
|
||||
}
|
||||
|
||||
const hasSessionListRevertDetails = (revert: unknown): boolean => {
|
||||
if (revert === undefined) {
|
||||
return false
|
||||
}
|
||||
if (!revert || typeof revert !== "object" || Array.isArray(revert)) {
|
||||
return true
|
||||
}
|
||||
|
||||
return Object.keys(revert).some((key) => key !== "messageID" && key !== "partID")
|
||||
}
|
||||
|
||||
const stripDiffSnapshotFields = <T extends DiffEntry>(diff: T, includePatch: boolean): T => {
|
||||
if (!diff || typeof diff !== "object") {
|
||||
return diff
|
||||
}
|
||||
|
||||
const shouldStrip =
|
||||
typeof diff.before === "string"
|
||||
|| typeof diff.after === "string"
|
||||
|| typeof diff.from === "string"
|
||||
|| typeof diff.to === "string"
|
||||
|| (includePatch && "patch" in (diff as T & { patch?: unknown }) && typeof (diff as T & { patch?: unknown }).patch === "string")
|
||||
|
||||
if (!shouldStrip) {
|
||||
return diff
|
||||
}
|
||||
|
||||
const rest = { ...diff } as T & { patch?: string }
|
||||
delete rest.before
|
||||
delete rest.after
|
||||
delete rest.from
|
||||
delete rest.to
|
||||
if (includePatch) {
|
||||
delete rest.patch
|
||||
}
|
||||
return rest
|
||||
}
|
||||
|
||||
/** Strip oversized snapshot fields from summary.diffs on a session object */
|
||||
export function stripSessionDiffSnapshots(session: Session): Session {
|
||||
const summary = (session as { summary?: SessionSummary }).summary
|
||||
if (!summary?.diffs || !Array.isArray(summary.diffs)) return session
|
||||
const revert = (session as { revert?: SessionRevert }).revert
|
||||
|
||||
let nextSession: Session = session
|
||||
let changed = false
|
||||
|
||||
if (revert && (typeof revert.snapshot === "string" || typeof revert.diff === "string")) {
|
||||
const nextRevert = { ...revert }
|
||||
delete nextRevert.snapshot
|
||||
delete nextRevert.diff
|
||||
nextSession = { ...nextSession, revert: nextRevert } as Session
|
||||
changed = true
|
||||
}
|
||||
|
||||
if (!summary?.diffs || !Array.isArray(summary.diffs)) return nextSession
|
||||
|
||||
const stripped = summary.diffs.map((d) => {
|
||||
if (d && (
|
||||
typeof d.before === "string"
|
||||
|| typeof d.after === "string"
|
||||
|| typeof d.from === "string"
|
||||
|| typeof d.to === "string"
|
||||
)) {
|
||||
const rest = { ...d }
|
||||
delete rest.before
|
||||
delete rest.after
|
||||
delete rest.from
|
||||
delete rest.to
|
||||
const nextDiff = stripDiffSnapshotFields(d, true)
|
||||
if (nextDiff !== d) {
|
||||
changed = true
|
||||
return rest
|
||||
}
|
||||
return d
|
||||
return nextDiff
|
||||
})
|
||||
|
||||
if (!changed) return session
|
||||
return { ...session, summary: { ...summary, diffs: stripped } } as Session
|
||||
if (!changed) return nextSession
|
||||
return { ...nextSession, summary: { ...summary, diffs: stripped } } as Session
|
||||
}
|
||||
|
||||
/** Strip detail-only fields from session list records before storing them. */
|
||||
export function stripSessionListDetails(session: Session): Session {
|
||||
const record = session as Session & {
|
||||
summary?: SessionSummary
|
||||
revert?: unknown
|
||||
permission?: unknown
|
||||
}
|
||||
|
||||
const shouldStrip = hasSessionListRevertDetails(record.revert)
|
||||
|| Array.isArray(record.summary?.diffs)
|
||||
|| "permission" in record
|
||||
|
||||
if (!shouldStrip) {
|
||||
return session
|
||||
}
|
||||
|
||||
const stripped = stripSessionDiffSnapshots(session) as typeof record
|
||||
const next: Record<string, unknown> = { ...stripped }
|
||||
delete next.permission
|
||||
|
||||
const revertMarker = getSessionListRevertMarker(stripped.revert)
|
||||
if (revertMarker) {
|
||||
next.revert = revertMarker
|
||||
} else {
|
||||
delete next.revert
|
||||
}
|
||||
|
||||
const summary = stripped.summary
|
||||
if (summary && typeof summary === "object" && !Array.isArray(summary)) {
|
||||
const summaryWithoutDiffs = { ...summary }
|
||||
delete summaryWithoutDiffs.diffs
|
||||
next.summary = summaryWithoutDiffs
|
||||
}
|
||||
|
||||
return next as unknown as Session
|
||||
}
|
||||
|
||||
/** Strip oversized snapshot fields from summary.diffs on a message object */
|
||||
@@ -64,21 +169,11 @@ export function stripMessageDiffSnapshots(message: Message): Message {
|
||||
|
||||
let changed = false
|
||||
const stripped = summary.diffs.map((d) => {
|
||||
if (d && (
|
||||
typeof d.before === "string"
|
||||
|| typeof d.after === "string"
|
||||
|| typeof d.from === "string"
|
||||
|| typeof d.to === "string"
|
||||
)) {
|
||||
const rest = { ...d }
|
||||
delete rest.before
|
||||
delete rest.after
|
||||
delete rest.from
|
||||
delete rest.to
|
||||
const nextDiff = stripDiffSnapshotFields(d, false)
|
||||
if (nextDiff !== d) {
|
||||
changed = true
|
||||
return rest
|
||||
}
|
||||
return d
|
||||
return nextDiff
|
||||
})
|
||||
|
||||
if (!changed) return message
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
|
||||
import { shouldFetchSessionForRenderableSync } from './use-sync'
|
||||
|
||||
describe('shouldFetchSessionForRenderableSync', () => {
|
||||
test('fetches full session detail when a lightweight list session is opened', () => {
|
||||
expect(shouldFetchSessionForRenderableSync({
|
||||
hasSession: true,
|
||||
shouldLoadMessages: true,
|
||||
force: false,
|
||||
})).toBe(true)
|
||||
})
|
||||
|
||||
test('skips session detail fetch when session and messages are already ready', () => {
|
||||
expect(shouldFetchSessionForRenderableSync({
|
||||
hasSession: true,
|
||||
shouldLoadMessages: false,
|
||||
force: false,
|
||||
})).toBe(false)
|
||||
})
|
||||
|
||||
test('fetches when the session record is missing', () => {
|
||||
expect(shouldFetchSessionForRenderableSync({
|
||||
hasSession: false,
|
||||
shouldLoadMessages: false,
|
||||
force: false,
|
||||
})).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -10,7 +10,7 @@ import {
|
||||
} from "./optimistic"
|
||||
import { dropCachedSessionMessageRecordsSnapshots, useDirectoryStore, useSyncSDK, useSyncDirectory, useChildStoreManager } from "./sync-context"
|
||||
import { dropSessionCaches, getProtectedSessionCacheIds } from "./session-cache"
|
||||
import { stripMessageDiffSnapshots } from "./sanitize"
|
||||
import { stripMessageDiffSnapshots, stripSessionDiffSnapshots } from "./sanitize"
|
||||
import { isVSCodeRuntime } from "@/lib/desktop"
|
||||
import { isMobileSurfaceRuntime } from "@/lib/runtimeSurface"
|
||||
import {
|
||||
@@ -121,6 +121,14 @@ function hasUserMessage(messages: Message[] | undefined): boolean {
|
||||
return Boolean(messages?.some(isUserMessage))
|
||||
}
|
||||
|
||||
export function shouldFetchSessionForRenderableSync(input: {
|
||||
hasSession: boolean
|
||||
shouldLoadMessages: boolean
|
||||
force?: boolean
|
||||
}): boolean {
|
||||
return Boolean(input.force) || !input.hasSession || input.shouldLoadMessages
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// useSync — message loading, pagination, optimistic updates
|
||||
// Message loading, pagination, optimistic updates
|
||||
@@ -429,8 +437,8 @@ export function useSync() {
|
||||
})) return
|
||||
}
|
||||
|
||||
const shouldFetchSession = !hasSession || force
|
||||
const shouldLoadMessages = !cachedReady || force
|
||||
const shouldLoadMessages = Boolean(!cachedReady || force)
|
||||
const shouldFetchSession = shouldFetchSessionForRenderableSync({ hasSession, shouldLoadMessages, force: Boolean(force) })
|
||||
const promise = (async () => {
|
||||
await Promise.all([
|
||||
shouldFetchSession
|
||||
@@ -442,13 +450,14 @@ export function useSync() {
|
||||
return response
|
||||
})
|
||||
if (result.data) {
|
||||
const nextSession = stripSessionDiffSnapshots(result.data)
|
||||
const s = store.getState()
|
||||
const sessions = [...s.session]
|
||||
const idx = Binary.search(sessions, sessionID, (s) => s.id)
|
||||
if (idx.found) {
|
||||
sessions[idx.index] = result.data
|
||||
sessions[idx.index] = nextSession
|
||||
} else {
|
||||
sessions.splice(idx.index, 0, result.data)
|
||||
sessions.splice(idx.index, 0, nextSession)
|
||||
}
|
||||
store.setState({ session: sessions })
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user