mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix: prevent session sidebar stalls and title churn
Large desktop histories were making session discovery parse every JSONL before pagination, while automatic title updates could still overwrite manual names during resumed sessions. This keeps listing bounded to the requested page, preserves custom titles, and blocks deleted placeholder sessions from being recreated by prewarm startup. Constraint: Desktop session storage remains JSONL-compatible with the CLI Rejected: Virtualize the sidebar in this patch | does not fix backend JSONL parsing cost Rejected: Disable title generation globally | would regress useful titles for unnamed sessions Confidence: high Scope-risk: moderate Directive: Do not reintroduce all-file JSONL parsing on /api/sessions list paths without a heavy-session benchmark Tested: ANTHROPIC_API_KEY=test-key bun test src/server/__tests__/title-service.test.ts src/server/__tests__/sessions.test.ts Tested: ANTHROPIC_API_KEY=test-key bun test src/server/__tests__/conversations.test.ts Tested: cd desktop && bun run lint Tested: cd desktop && bun run build Tested: heavy local preview with 240 sessions, 320 messages each, and 40 restored tabs Not-tested: native packaged desktop runtime under Windows with the same heavy fixture Related: https://github.com/NanmiCoder/cc-haha/issues/237 Related: https://github.com/NanmiCoder/cc-haha/issues/248
This commit is contained in:
parent
c13fa75c3b
commit
d50268c07c
@ -17,6 +17,7 @@ vi.mock('../../i18n', () => ({
|
||||
'sidebar.noMatching': 'No matching sessions',
|
||||
'sidebar.sessionListFailed': 'Session list failed',
|
||||
'common.retry': 'Retry',
|
||||
'common.loading': 'Loading...',
|
||||
'common.cancel': 'Cancel',
|
||||
'common.delete': 'Delete',
|
||||
'common.rename': 'Rename',
|
||||
@ -201,4 +202,13 @@ describe('Sidebar', () => {
|
||||
|
||||
expect(screen.getByTestId('sidebar-session-list-section')).toHaveClass('flex', 'flex-1', 'min-h-0', 'flex-col')
|
||||
})
|
||||
|
||||
it('shows a loading state instead of an empty session list while initial fetch is pending', () => {
|
||||
useSessionStore.setState({ isLoading: true, sessions: [] })
|
||||
|
||||
render(<Sidebar />)
|
||||
|
||||
expect(screen.getByText('Loading...')).toBeInTheDocument()
|
||||
expect(screen.queryByText('No sessions')).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
@ -18,6 +18,7 @@ const TIME_GROUP_ORDER: TimeGroup[] = ['today', 'yesterday', 'last7days', 'last3
|
||||
export function Sidebar() {
|
||||
const sessions = useSessionStore((s) => s.sessions)
|
||||
const selectedProjects = useSessionStore((s) => s.selectedProjects)
|
||||
const isLoading = useSessionStore((s) => s.isLoading)
|
||||
const error = useSessionStore((s) => s.error)
|
||||
const fetchSessions = useSessionStore((s) => s.fetchSessions)
|
||||
const deleteSession = useSessionStore((s) => s.deleteSession)
|
||||
@ -63,6 +64,7 @@ export function Sidebar() {
|
||||
}, [sessions, selectedProjects, searchQuery])
|
||||
|
||||
const timeGroups = useMemo(() => groupByTime(filteredSessions), [filteredSessions])
|
||||
const showInitialLoading = isLoading && sessions.length === 0
|
||||
|
||||
const handleContextMenu = useCallback((e: React.MouseEvent, id: string) => {
|
||||
e.preventDefault()
|
||||
@ -245,7 +247,11 @@ export function Sidebar() {
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
{filteredSessions.length === 0 && (
|
||||
{showInitialLoading ? (
|
||||
<div className="px-3 py-4 text-center text-xs text-[var(--color-text-tertiary)]">
|
||||
{t('common.loading')}
|
||||
</div>
|
||||
) : filteredSessions.length === 0 && (
|
||||
<div className="px-3 py-4 text-center text-xs text-[var(--color-text-tertiary)]">
|
||||
{searchQuery ? t('sidebar.noMatching') : t('sidebar.noSessions')}
|
||||
</div>
|
||||
|
||||
@ -411,7 +411,7 @@ const TabItem = forwardRef<HTMLDivElement, {
|
||||
${isDragging ? 'z-20 cursor-grabbing' : 'cursor-grab'}
|
||||
transition-[background-color,box-shadow,opacity,transform] duration-150 ease-out
|
||||
${isActive
|
||||
? 'bg-[var(--color-surface)]'
|
||||
? 'bg-[var(--color-surface)] shadow-[inset_0_-2px_0_var(--color-brand)]'
|
||||
: 'bg-transparent hover:bg-[var(--color-surface-hover)]'
|
||||
}
|
||||
${isDragging ? 'opacity-95 shadow-[0_10px_24px_rgba(0,0,0,0.18)] ring-1 ring-[var(--color-border)]' : ''}
|
||||
|
||||
@ -10,7 +10,7 @@ import * as fs from 'fs/promises'
|
||||
import * as path from 'path'
|
||||
import * as os from 'os'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { ConversationService, conversationService } from '../services/conversationService.js'
|
||||
import { ConversationService, ConversationStartupError, conversationService } from '../services/conversationService.js'
|
||||
import { SessionService } from '../services/sessionService.js'
|
||||
import { ProviderService } from '../services/providerService.js'
|
||||
|
||||
@ -30,6 +30,21 @@ describe('ConversationService', () => {
|
||||
expect(svc.getActiveSessions()).toEqual([])
|
||||
})
|
||||
|
||||
it('should block startup after a session is deleted during prewarm', async () => {
|
||||
const svc = new ConversationService()
|
||||
const sid = crypto.randomUUID()
|
||||
|
||||
svc.markSessionDeleted(sid)
|
||||
|
||||
try {
|
||||
await svc.startSession(sid, process.cwd(), 'ws://127.0.0.1:1/sdk/test')
|
||||
throw new Error('expected startSession to reject')
|
||||
} catch (error) {
|
||||
expect(error).toBeInstanceOf(ConversationStartupError)
|
||||
expect((error as ConversationStartupError).code).toBe('SESSION_DELETED')
|
||||
}
|
||||
})
|
||||
|
||||
it('should return false when sending message to non-existent session', async () => {
|
||||
const svc = new ConversationService()
|
||||
const result = await svc.sendMessage('no-such-session', 'hello')
|
||||
|
||||
@ -374,6 +374,34 @@ describe('SessionService', () => {
|
||||
expect(page2.sessions).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('should only parse the requested page when listing many sessions', async () => {
|
||||
for (let i = 0; i < 12; i++) {
|
||||
const id = `1000000${i.toString(16)}-bbbb-cccc-dddd-eeeeeeeeeeee`
|
||||
const filePath = await writeSessionFile('-tmp-many-sessions', id, [
|
||||
makeSnapshotEntry(),
|
||||
makeUserEntry(`Message ${i}`),
|
||||
])
|
||||
const mtime = new Date(Date.now() - i * 1000)
|
||||
await fs.utimes(filePath, mtime, mtime)
|
||||
}
|
||||
|
||||
const serviceWithSpy = service as unknown as {
|
||||
readJsonlFile: (...args: unknown[]) => Promise<unknown>
|
||||
}
|
||||
const originalReadJsonlFile = serviceWithSpy.readJsonlFile.bind(service)
|
||||
let readCount = 0
|
||||
serviceWithSpy.readJsonlFile = async (...args) => {
|
||||
readCount += 1
|
||||
return originalReadJsonlFile(...args)
|
||||
}
|
||||
|
||||
const result = await service.listSessions({ limit: 3, offset: 0 })
|
||||
|
||||
expect(result.total).toBe(12)
|
||||
expect(result.sessions).toHaveLength(3)
|
||||
expect(readCount).toBe(3)
|
||||
})
|
||||
|
||||
it('should filter sessions by project', async () => {
|
||||
const id1 = 'aaaaaaaa-1111-cccc-dddd-eeeeeeeeeeee'
|
||||
const id2 = 'aaaaaaaa-2222-cccc-dddd-eeeeeeeeeeee'
|
||||
|
||||
@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
|
||||
import * as fs from 'fs/promises'
|
||||
import * as os from 'os'
|
||||
import * as path from 'path'
|
||||
import { generateTitle } from '../services/titleService.js'
|
||||
import { generateTitle, saveAiTitle } from '../services/titleService.js'
|
||||
import { sessionService } from '../services/sessionService.js'
|
||||
|
||||
describe('titleService', () => {
|
||||
let tmpDir: string
|
||||
@ -68,6 +69,21 @@ describe('titleService', () => {
|
||||
server.stop(true)
|
||||
}
|
||||
})
|
||||
|
||||
test('does not persist automatic titles over a user custom title', async () => {
|
||||
const { sessionId } = await sessionService.createSession(os.tmpdir())
|
||||
await sessionService.renameSession(sessionId, 'My fixed name')
|
||||
|
||||
await expect(saveAiTitle(sessionId, 'Automatic topic')).resolves.toBe(false)
|
||||
|
||||
const detail = await sessionService.getSession(sessionId)
|
||||
expect(detail?.title).toBe('My fixed name')
|
||||
|
||||
const found = await sessionService.findSessionFile(sessionId)
|
||||
expect(found).not.toBeNull()
|
||||
const content = await fs.readFile(found!.filePath, 'utf-8')
|
||||
expect(content).not.toContain('"type":"ai-title"')
|
||||
})
|
||||
})
|
||||
|
||||
function restoreEnv(key: string, value: string | undefined) {
|
||||
|
||||
@ -308,6 +308,7 @@ function isSessionNotFoundError(error: unknown): error is Error {
|
||||
}
|
||||
|
||||
async function deleteSession(sessionId: string): Promise<Response> {
|
||||
conversationService.markSessionDeleted(sessionId)
|
||||
await sessionService.deleteSession(sessionId)
|
||||
return Response.json({ ok: true })
|
||||
}
|
||||
|
||||
@ -61,7 +61,8 @@ export class ConversationStartupError extends Error {
|
||||
| 'CLI_AUTH_REQUIRED'
|
||||
| 'CLI_SESSION_CONFLICT'
|
||||
| 'CLI_START_FAILED'
|
||||
| 'CLI_SPAWN_FAILED',
|
||||
| 'CLI_SPAWN_FAILED'
|
||||
| 'SESSION_DELETED',
|
||||
readonly retryable = false,
|
||||
) {
|
||||
super(message)
|
||||
@ -71,6 +72,7 @@ export class ConversationStartupError extends Error {
|
||||
|
||||
export class ConversationService {
|
||||
private sessions = new Map<string, SessionProcess>()
|
||||
private deletedSessions = new Set<string>()
|
||||
private providerService = new ProviderService()
|
||||
|
||||
private buildSessionCliArgs(
|
||||
@ -106,6 +108,12 @@ export class ConversationService {
|
||||
sdkUrl: string,
|
||||
options?: SessionStartOptions,
|
||||
): Promise<void> {
|
||||
if (this.deletedSessions.has(sessionId)) {
|
||||
throw new ConversationStartupError(
|
||||
`Session was deleted before startup completed: ${sessionId}`,
|
||||
'SESSION_DELETED',
|
||||
)
|
||||
}
|
||||
if (this.sessions.has(sessionId)) return
|
||||
|
||||
const launchInfo = await sessionService.getSessionLaunchInfo(sessionId)
|
||||
@ -113,6 +121,13 @@ export class ConversationService {
|
||||
const shouldReplacePlaceholder =
|
||||
!!launchInfo && launchInfo.transcriptMessageCount === 0
|
||||
|
||||
if (this.deletedSessions.has(sessionId)) {
|
||||
throw new ConversationStartupError(
|
||||
`Session was deleted before startup completed: ${sessionId}`,
|
||||
'SESSION_DELETED',
|
||||
)
|
||||
}
|
||||
|
||||
if (shouldReplacePlaceholder) {
|
||||
await sessionService.deleteSessionFile(sessionId)
|
||||
}
|
||||
@ -479,6 +494,11 @@ export class ConversationService {
|
||||
}
|
||||
}
|
||||
|
||||
markSessionDeleted(sessionId: string): void {
|
||||
this.deletedSessions.add(sessionId)
|
||||
this.stopSession(sessionId)
|
||||
}
|
||||
|
||||
getActiveSessions(): string[] {
|
||||
return Array.from(this.sessions.keys())
|
||||
}
|
||||
|
||||
@ -991,13 +991,27 @@ export class SessionService {
|
||||
offset?: number
|
||||
}): Promise<{ sessions: SessionListItem[]; total: number }> {
|
||||
const sessionFiles = await this.discoverSessionFiles(options?.project)
|
||||
const filesWithStats = (await Promise.all(sessionFiles.map(async (sessionFile) => {
|
||||
try {
|
||||
return {
|
||||
...sessionFile,
|
||||
stat: await fs.stat(sessionFile.filePath),
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}))).filter((item): item is NonNullable<typeof item> => item !== null)
|
||||
|
||||
filesWithStats.sort((a, b) => b.stat.mtime.getTime() - a.stat.mtime.getTime())
|
||||
|
||||
const total = filesWithStats.length
|
||||
const offset = options?.offset ?? 0
|
||||
const limit = options?.limit ?? 50
|
||||
const paginatedFiles = filesWithStats.slice(offset, offset + limit)
|
||||
|
||||
// Build session list items with metadata from file stats & first entries
|
||||
const items: SessionListItem[] = []
|
||||
|
||||
for (const { filePath, projectDir, sessionId } of sessionFiles) {
|
||||
const items = (await Promise.all(paginatedFiles.map(async ({ filePath, projectDir, sessionId, stat }) => {
|
||||
try {
|
||||
const stat = await fs.stat(filePath)
|
||||
const entries = await this.readJsonlFile(filePath)
|
||||
const workDir = this.resolveWorkDirFromEntries(entries, projectDir)
|
||||
const workDirExists = await this.pathExists(workDir)
|
||||
@ -1018,7 +1032,7 @@ export class SessionService {
|
||||
}
|
||||
}
|
||||
|
||||
items.push({
|
||||
return {
|
||||
id: sessionId,
|
||||
title,
|
||||
createdAt,
|
||||
@ -1027,21 +1041,14 @@ export class SessionService {
|
||||
projectPath: projectDir,
|
||||
workDir,
|
||||
workDirExists,
|
||||
})
|
||||
}
|
||||
} catch {
|
||||
// Skip unreadable files
|
||||
return null
|
||||
}
|
||||
}
|
||||
}))).filter((item): item is SessionListItem => item !== null)
|
||||
|
||||
// Sort by modifiedAt descending (most recent first)
|
||||
items.sort((a, b) => new Date(b.modifiedAt).getTime() - new Date(a.modifiedAt).getTime())
|
||||
|
||||
const total = items.length
|
||||
const offset = options?.offset ?? 0
|
||||
const limit = options?.limit ?? 50
|
||||
const paginated = items.slice(offset, offset + limit)
|
||||
|
||||
return { sessions: paginated, total }
|
||||
return { sessions: items, total }
|
||||
}
|
||||
|
||||
/**
|
||||
@ -1212,6 +1219,20 @@ export class SessionService {
|
||||
})
|
||||
}
|
||||
|
||||
async getCustomTitle(sessionId: string): Promise<string | null> {
|
||||
const found = await this.findSessionFile(sessionId)
|
||||
if (!found) return null
|
||||
|
||||
const entries = await this.readJsonlFile(found.filePath)
|
||||
let customTitle: string | null = null
|
||||
for (const entry of entries) {
|
||||
if (entry.type === 'custom-title' && typeof entry.customTitle === 'string' && entry.customTitle.trim()) {
|
||||
customTitle = entry.customTitle
|
||||
}
|
||||
}
|
||||
return customTitle
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the actual working directory for a session.
|
||||
* First checks for stored session-meta entry, then falls back to desanitizePath.
|
||||
|
||||
@ -123,7 +123,13 @@ async function shouldDisableThinkingForTitle(presetId: string): Promise<boolean>
|
||||
|
||||
/**
|
||||
* Persist an AI-generated title to the session's JSONL file.
|
||||
* Returns false when a user custom title exists, because custom titles are
|
||||
* intentional and must not be replaced by automatic title refreshes.
|
||||
*/
|
||||
export async function saveAiTitle(sessionId: string, title: string): Promise<void> {
|
||||
export async function saveAiTitle(sessionId: string, title: string): Promise<boolean> {
|
||||
if (await sessionService.getCustomTitle(sessionId)) {
|
||||
return false
|
||||
}
|
||||
await sessionService.appendAiTitle(sessionId, title)
|
||||
return true
|
||||
}
|
||||
|
||||
@ -278,7 +278,12 @@ async function handleUserMessage(
|
||||
// Track user message for title generation
|
||||
let titleState = sessionTitleState.get(sessionId)
|
||||
if (!titleState) {
|
||||
titleState = { userMessageCount: 0, hasCustomTitle: false, firstUserMessage: '', allUserMessages: [] }
|
||||
titleState = {
|
||||
userMessageCount: 0,
|
||||
hasCustomTitle: !!(await sessionService.getCustomTitle(sessionId)),
|
||||
firstUserMessage: '',
|
||||
allUserMessages: [],
|
||||
}
|
||||
sessionTitleState.set(sessionId, titleState)
|
||||
}
|
||||
titleState.userMessageCount++
|
||||
@ -606,7 +611,11 @@ function triggerTitleGeneration(ws: ServerWebSocket<WebSocketData>, sessionId: s
|
||||
if (count === 1) {
|
||||
const placeholder = deriveTitle(text)
|
||||
if (placeholder) {
|
||||
await saveAiTitle(sessionId, placeholder)
|
||||
const saved = await saveAiTitle(sessionId, placeholder)
|
||||
if (!saved) {
|
||||
state.hasCustomTitle = true
|
||||
return
|
||||
}
|
||||
sendMessage(ws, { type: 'session_title_updated', sessionId, title: placeholder })
|
||||
}
|
||||
}
|
||||
@ -614,7 +623,11 @@ function triggerTitleGeneration(ws: ServerWebSocket<WebSocketData>, sessionId: s
|
||||
// Stage 2: AI-generated title
|
||||
const aiTitle = await generateTitle(text, runtimeProviderId)
|
||||
if (aiTitle) {
|
||||
await saveAiTitle(sessionId, aiTitle)
|
||||
const saved = await saveAiTitle(sessionId, aiTitle)
|
||||
if (!saved) {
|
||||
state.hasCustomTitle = true
|
||||
return
|
||||
}
|
||||
sendMessage(ws, { type: 'session_title_updated', sessionId, title: aiTitle })
|
||||
}
|
||||
} catch (err) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user