From 28f467651de80ace9dd3a81a729cd06fbf12b1d6 Mon Sep 17 00:00:00 2001 From: "alexgangxi@163.com" Date: Mon, 18 May 2026 00:49:17 +0800 Subject: [PATCH 1/2] fix(core): restore file history snapshots on resume --- packages/core/src/config/config.test.ts | 41 ++++++++++- packages/core/src/config/config.ts | 11 +++ packages/core/src/core/client.ts | 9 ++- .../src/services/chatRecordingService.test.ts | 28 ++++++++ .../core/src/services/chatRecordingService.ts | 26 +++++++ .../core/src/services/fileHistoryService.ts | 17 ++++- .../core/src/services/sessionService.test.ts | 71 +++++++++++++++++++ packages/core/src/services/sessionService.ts | 31 ++++++++ 8 files changed, 230 insertions(+), 4 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 172b3983d1..ff470208aa 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -46,6 +46,7 @@ import { readAutoMemoryIndex } from '../memory/store.js'; import { ExtensionManager } from '../extension/extensionManager.js'; import { SkillManager } from '../skills/skill-manager.js'; import { HookSystem } from '../hooks/index.js'; +import type { ResumedSessionData } from '../services/sessionService.js'; function createToolMock(toolName: string) { const ToolMock = vi.fn(); @@ -414,7 +415,7 @@ describe('Server Config (config.ts)', () => { expect(cache.size()).toBe(0); }); - it('refreshes the telemetry session context with the new session ID', () => { + it('refreshes the telemetry session context with the new session ID', () => { const config = new Config(baseParams); vi.mocked(refreshSessionContext).mockClear(); @@ -422,6 +423,44 @@ describe('Server Config (config.ts)', () => { expect(refreshSessionContext).toHaveBeenCalledWith(newSessionId); }); + + it('restores persisted file-history snapshots when resuming a session', () => { + const config = new Config({ ...baseParams, fileCheckpointingEnabled: true }); + const resumedSessionData: ResumedSessionData = { + conversation: { + sessionId: 'resumed-session', + projectHash: 'test-project-hash', + startTime: '2026-05-17T00:00:00.000Z', + lastUpdated: '2026-05-17T00:00:05.000Z', + messages: [], + }, + filePath: '/tmp/resumed-session.jsonl', + lastCompletedUuid: null, + fileHistorySnapshots: [ + { + promptId: 'prompt-1', + timestamp: '2026-05-17T00:00:00.000Z' as unknown as Date, + trackedFileBackups: { + '/tmp/src/app.ts': { + backupFileName: 'backup-1', + version: 1, + backupTime: '2026-05-17T00:00:01.000Z' as unknown as Date, + }, + }, + }, + ], + }; + + config.startNewSession('resumed-session', resumedSessionData); + + const snapshots = config.getFileHistoryService().getSnapshots(); + expect(snapshots).toHaveLength(1); + expect(snapshots[0]?.promptId).toBe('prompt-1'); + expect(snapshots[0]?.timestamp).toBeInstanceOf(Date); + expect( + snapshots[0]?.trackedFileBackups['/tmp/src/app.ts']?.backupTime, + ).toBeInstanceOf(Date); + }); }); it('should expose LSP status from the configured client', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index a8e5396924..313fa92264 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -766,6 +766,7 @@ export class Config { private readonly checkpointing: boolean; private readonly fileCheckpointingEnabled: boolean; private fileHistoryService: FileHistoryService | undefined; + private fileHistoryRestoredFromSessionData = false; private readonly proxy: string | undefined; private readonly cwd: string; private readonly explicitIncludeDirectories: string[]; @@ -1753,6 +1754,7 @@ export class Config { // cache, not the parent's. this.getFileReadCache().clear(); this.fileHistoryService = undefined; + this.fileHistoryRestoredFromSessionData = false; refreshSessionContext(this.sessionId); // The commit-attribution singleton accumulates per-file AI edits // and a session-scoped prompt counter — both stop being meaningful @@ -2784,6 +2786,15 @@ export class Config { this.cwd, ); } + if ( + !this.fileHistoryRestoredFromSessionData && + this.sessionData?.fileHistorySnapshots + ) { + this.fileHistoryService.restoreFromSnapshots( + this.sessionData.fileHistorySnapshots, + ); + this.fileHistoryRestoredFromSessionData = true; + } return this.fileHistoryService; } diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index b50e5aa411..c832a666e7 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -1275,7 +1275,14 @@ export class GeminiClient { if (messageType === SendMessageType.UserQuery) { try { - await this.config.getFileHistoryService().makeSnapshot(prompt_id); + const fileHistoryService = this.config.getFileHistoryService(); + await fileHistoryService.makeSnapshot(prompt_id); + const latestSnapshot = fileHistoryService.getSnapshots().at(-1); + if (latestSnapshot?.promptId === prompt_id) { + this.config + .getChatRecordingService() + ?.recordFileHistorySnapshot(latestSnapshot); + } } catch (e) { debugLogger.error(`FileHistory: makeSnapshot failed: ${e}`); } diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index c05af4278c..4d620b08b5 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -15,6 +15,7 @@ import { type ChatRecord, type AtCommandRecordPayload, } from './chatRecordingService.js'; +import type { FileHistorySnapshot } from './fileHistoryService.js'; import * as jsonl from '../utils/jsonl-utils.js'; import type { Part } from '@google/genai'; import type { FileDiff } from '../tools/tools.js'; @@ -751,6 +752,33 @@ describe('ChatRecordingService', () => { }); }); + describe('recordFileHistorySnapshot', () => { + it('should append a versioned file-history snapshot system record', async () => { + const snapshot: FileHistorySnapshot = { + promptId: 'prompt-1', + timestamp: new Date('2026-05-17T00:00:00.000Z'), + trackedFileBackups: { + 'src/app.ts': { + backupFileName: 'backup-1', + version: 1, + backupTime: new Date('2026-05-17T00:00:01.000Z'), + }, + }, + }; + + chatRecordingService.recordFileHistorySnapshot(snapshot); + await chatRecordingService.flush(); + + const record = vi.mocked(jsonl.writeLine).mock.calls[0][1] as ChatRecord; + expect(record.type).toBe('system'); + expect(record.subtype).toBe('file_history_snapshot'); + expect(record.systemPayload).toEqual({ + version: 1, + snapshot, + }); + }); + }); + // Note: Session management tests (listSessions, loadSession, deleteSession, etc.) // have been moved to sessionService.test.ts // Session resume integration tests should test via SessionService mock diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 05c06c1929..b24563e84f 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -28,6 +28,7 @@ import type { import type { Status } from '../core/coreToolScheduler.js'; import type { AgentResultDisplay, FileDiff } from '../tools/tools.js'; import type { UiEvent } from '../telemetry/uiTelemetry.js'; +import type { FileHistorySnapshot } from './fileHistoryService.js'; const debugLogger = createDebugLogger('CHAT_RECORDING'); @@ -229,6 +230,7 @@ export interface ChatRecord { | 'ui_telemetry' | 'at_command' | 'attribution_snapshot' + | 'file_history_snapshot' | 'notification' | 'cron' | 'mid_turn_user_message' @@ -278,6 +280,7 @@ export interface ChatRecord { | UiTelemetryRecordPayload | AtCommandRecordPayload | AttributionSnapshotPayload + | FileHistorySnapshotRecordPayload | CustomTitleRecordPayload | NotificationRecordPayload | RewindRecordPayload @@ -418,6 +421,11 @@ export interface AttributionSnapshotPayload { snapshot: AttributionSnapshot; } +export interface FileHistorySnapshotRecordPayload { + version: 1; + snapshot: FileHistorySnapshot; +} + /** * Stored payload for conversation rewind events. */ @@ -1339,4 +1347,22 @@ export class ChatRecordingService { debugLogger.error('Error saving attribution snapshot:', error); } } + + recordFileHistorySnapshot(snapshot: FileHistorySnapshot): void { + try { + const record: ChatRecord = { + ...this.createBaseRecord('system'), + type: 'system', + subtype: 'file_history_snapshot', + systemPayload: { + version: 1, + snapshot, + }, + }; + + this.appendRecord(record); + } catch (error) { + debugLogger.error('Error saving file history snapshot:', error); + } + } } diff --git a/packages/core/src/services/fileHistoryService.ts b/packages/core/src/services/fileHistoryService.ts index ecd57d27ac..0277a6e7e1 100644 --- a/packages/core/src/services/fileHistoryService.ts +++ b/packages/core/src/services/fileHistoryService.ts @@ -315,9 +315,22 @@ export class FileHistoryService { for (const [p, backup] of Object.entries(snapshot.trackedFileBackups)) { const trackingPath = this.maybeShortenFilePath(p); trackedFiles.add(trackingPath); - trackedFileBackups[trackingPath] = backup; + trackedFileBackups[trackingPath] = { + ...backup, + backupTime: + backup.backupTime instanceof Date + ? backup.backupTime + : new Date(backup.backupTime), + }; } - migrated.push({ ...snapshot, trackedFileBackups }); + migrated.push({ + ...snapshot, + timestamp: + snapshot.timestamp instanceof Date + ? snapshot.timestamp + : new Date(snapshot.timestamp), + trackedFileBackups, + }); } this.state = { snapshots: migrated, diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index 24e5942587..b46e73cc00 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -526,6 +526,77 @@ describe('SessionService', () => { expect(assistantMsg?.usageMetadata?.totalTokenCount).toBe(30); expect(assistantMsg?.model).toBe('gemini-pro'); }); + + it('should expose persisted file-history snapshots from the active branch', async () => { + const records: ChatRecord[] = [ + { + uuid: 'u1', + parentUuid: null, + sessionId: 'test', + timestamp: '2024-01-01T00:00:00Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'Hello' }] }, + cwd: '/test/project/root', + version: '1.0.0', + }, + { + uuid: 's1', + parentUuid: 'u1', + sessionId: 'test', + timestamp: '2024-01-01T00:00:01Z', + type: 'system', + subtype: 'file_history_snapshot', + systemPayload: { + version: 1, + snapshot: { + promptId: 'prompt-1', + timestamp: '2024-01-01T00:00:01.000Z' as unknown as Date, + trackedFileBackups: { + 'src/app.ts': { + backupFileName: 'backup-1', + version: 1, + backupTime: '2024-01-01T00:00:01.000Z' as unknown as Date, + }, + }, + }, + }, + cwd: '/test/project/root', + version: '1.0.0', + }, + { + uuid: 'a1', + parentUuid: 's1', + sessionId: 'test', + timestamp: '2024-01-01T00:00:02Z', + type: 'assistant', + message: { role: 'model', parts: [{ text: 'Done' }] }, + cwd: '/test/project/root', + version: '1.0.0', + }, + ]; + + statSyncSpy.mockReturnValue({ + mtimeMs: Date.now(), + isFile: () => true, + } as fs.Stats); + vi.mocked(jsonl.read).mockResolvedValue(records); + + const loaded = await sessionService.loadSession('test'); + + expect(loaded?.fileHistorySnapshots).toEqual([ + { + promptId: 'prompt-1', + timestamp: '2024-01-01T00:00:01.000Z', + trackedFileBackups: { + 'src/app.ts': { + backupFileName: 'backup-1', + version: 1, + backupTime: '2024-01-01T00:00:01.000Z', + }, + }, + }, + ]); + }); }); describe('removeSession', () => { diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index 95f5e7f804..1dd67b92a3 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -15,9 +15,11 @@ import * as jsonl from '../utils/jsonl-utils.js'; import type { ChatCompressionRecordPayload, ChatRecord, + FileHistorySnapshotRecordPayload, TitleSource, UiTelemetryRecordPayload, } from './chatRecordingService.js'; +import type { FileHistorySnapshot } from './fileHistoryService.js'; import { uiTelemetryService } from '../telemetry/uiTelemetry.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { @@ -129,6 +131,8 @@ export interface ResumedSessionData { filePath: string; /** UUID of the last completed message - new messages should use this as parentUuid */ lastCompletedUuid: string | null; + /** File-history snapshots persisted on the active session branch, if any. */ + fileHistorySnapshots?: FileHistorySnapshot[]; } /** @@ -707,10 +711,14 @@ export class SessionService { messages, }; + const fileHistorySnapshots = extractFileHistorySnapshots(messages); + return { conversation, filePath, lastCompletedUuid: lastMessage.uuid, + fileHistorySnapshots: + fileHistorySnapshots.length > 0 ? fileHistorySnapshots : undefined, }; } @@ -1290,6 +1298,29 @@ export function replayUiTelemetryFromConversation( } } +function extractFileHistorySnapshots( + messages: ChatRecord[], +): FileHistorySnapshot[] { + const snapshots: FileHistorySnapshot[] = []; + for (const record of messages) { + if (record.type !== 'system' || record.subtype !== 'file_history_snapshot') { + continue; + } + + const payload = record.systemPayload as + | FileHistorySnapshotRecordPayload + | undefined; + const snapshot = payload?.snapshot; + if (payload?.version !== 1 || !snapshot) { + continue; + } + + snapshots.push(snapshot); + } + + return snapshots; +} + /** * Returns the best available prompt token count for resuming telemetry. * Walks backward through messages and returns the first valid value: From ef246a2c027620b7ef9c6d73e6d47a4b2d312232 Mon Sep 17 00:00:00 2001 From: "alexgangxi@163.com" Date: Mon, 18 May 2026 09:13:46 +0800 Subject: [PATCH 2/2] fix(core): harden file history snapshot restore --- packages/core/src/config/config.test.ts | 7 +- packages/core/src/config/config.ts | 78 ++++++++++--------- packages/core/src/core/client.ts | 4 +- .../src/services/fileHistoryService.test.ts | 49 +++++++++--- .../core/src/services/fileHistoryService.ts | 47 +++++++++-- .../core/src/services/sessionService.test.ts | 57 ++++++++++++-- packages/core/src/services/sessionService.ts | 20 ++++- 7 files changed, 194 insertions(+), 68 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index ff470208aa..1b015af321 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -457,9 +457,10 @@ describe('Server Config (config.ts)', () => { expect(snapshots).toHaveLength(1); expect(snapshots[0]?.promptId).toBe('prompt-1'); expect(snapshots[0]?.timestamp).toBeInstanceOf(Date); - expect( - snapshots[0]?.trackedFileBackups['/tmp/src/app.ts']?.backupTime, - ).toBeInstanceOf(Date); + const restoredBackup = Object.values( + snapshots[0]?.trackedFileBackups ?? {}, + )[0]; + expect(restoredBackup?.backupTime).toBeInstanceOf(Date); }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 313fa92264..5afbc78ca2 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -49,7 +49,7 @@ import { GitWorktreeService } from '../services/gitWorktreeService.js'; import { cleanupStaleAgentWorktrees } from '../services/worktreeCleanup.js'; import { CronScheduler } from '../services/cronScheduler.js'; -// Tools — only lightweight imports; tool classes are lazy-loaded via dynamic import +// Tools �only lightweight imports; tool classes are lazy-loaded via dynamic import import { MCPServerStatus, getMCPServerStatus, @@ -289,8 +289,8 @@ function normalizeGitCoAuthor(value: GitCoAuthorParam | undefined): { } // Default to `true` (the schema default) ONLY when the sub-field // is genuinely absent. For PRESENT-but-non-boolean values, honor - // common string forms (`"true"`/`"yes"`/`"on"`/`"1"` → true, - // `"false"`/`"no"`/`"off"`/`"0"`/`""` → false) and treat anything + // common string forms (`"true"`/`"yes"`/`"on"`/`"1"` �true, + // `"false"`/`"no"`/`"off"`/`"0"`/`""` �false) and treat anything // else as opt-out. settings.json is user-editable, and the previous // "default-to-true on mismatch" policy meant a hand-edited // `{ "commit": "false" }` silently activated attribution against @@ -309,10 +309,10 @@ function normalizeGitCoAuthor(value: GitCoAuthorParam | undefined): { ) { return true; } - // Known disable-intent forms — silent (matches user intent). + // Known disable-intent forms �silent (matches user intent). const knownDisable = ['false', 'no', 'off', '0', 'disabled', '']; if (!knownDisable.includes(lowered)) { - // Unrecognised string — disable (safer-by-default) but log + // Unrecognised string �disable (safer-by-default) but log // so a user wondering "why is my setting being ignored?" // can see the actual coercion in QWEN_DEBUG_LOG_FILE. gitCoAuthorLogger.warn( @@ -384,7 +384,7 @@ export class MCPServerConfig { * Per-server cap on the discovery handshake (`connect` + `tools/list` + * `prompts/list` + `resources/list`). Defaults: 30s for stdio servers, * 5s for remote HTTP/SSE. Tool-call timeout (`timeout` above) is - * unaffected — a long-running tool invocation is not a startup + * unaffected �a long-running tool invocation is not a startup * pathology. Appended at the end of the parameter list to avoid * shifting positional arguments at the many `new MCPServerConfig(...)` * call sites. @@ -594,7 +594,7 @@ export interface ConfigParameters { * Disable all hooks (default: false, hooks enabled). * Migration note: This replaces the deprecated hooksConfig.enabled setting. * Users with old settings.json containing hooksConfig.enabled should migrate - * to use disableAllHooks instead (note: inverted logic - enabled:true → disableAllHooks:false). + * to use disableAllHooks instead (note: inverted logic - enabled:true �disableAllHooks:false). */ disableAllHooks?: boolean; /** @@ -691,7 +691,7 @@ export class Config { private readonly backgroundShellRegistry = new BackgroundShellRegistry(); // Field initializer runs once on the parent Config; child Configs // built via Object.create(parent) intentionally do NOT pick this up - // — see getFileReadCache() for the per-instance lazy initialization + // �see getFileReadCache() for the per-instance lazy initialization // that keeps subagent caches isolated from the parent's. private fileReadCache: FileReadCache = new FileReadCache(); private extensionManager!: ExtensionManager; @@ -1279,7 +1279,7 @@ export class Config { // after the registry exists. This lets `Config.initialize()` (and the // cli's `input_enabled` checkpoint) resolve without waiting on MCP // server response time. Users can opt back into the legacy synchronous - // behavior with `QWEN_CODE_LEGACY_MCP_BLOCKING=1` — kept ≥ 1 release as + // behavior with `QWEN_CODE_LEGACY_MCP_BLOCKING=1` �kept �1 release as // an escape hatch. const legacyBlockingMcp = process.env['QWEN_CODE_LEGACY_MCP_BLOCKING'] === '1'; @@ -1327,7 +1327,7 @@ export class Config { // earlier `agent` runs that exited before their cleanup helper ran // (Ctrl-C, process crash, abrupt shutdown). The sweep only touches // `agent-<7hex>` slugs, skips anything newer than 30 days, and - // is fail-closed against tracked changes or unpushed commits — so + // is fail-closed against tracked changes or unpushed commits �so // running it on every startup cannot destroy user work. We do not // await this: it is a hygiene task that must never delay the // first model turn. @@ -1336,7 +1336,7 @@ export class Config { // directory the worktree creators (`enter_worktree` and // `agent isolation:'worktree'`) write to. Using `this.targetDir` // directly would cause launches from a monorepo subdirectory to - // scan `/.qwen/worktrees/` — which never exists — and the + // scan `/.qwen/worktrees/` �which never exists �and the // sweep would silently be a no-op forever. if (!this.getBareMode()) { void (async () => { @@ -1356,7 +1356,7 @@ export class Config { // Skipped (no worktrees dir) is the common-case happy // path on every CLI start for ~99% of users. `debug` so // operators can opt in via `--debug` when they actually - // want to confirm the sweep is wired up — `info` would + // want to confirm the sweep is wired up �`info` would // be log noise. this.debugLogger.debug( `Stale worktree sweep skipped: ${worktreesDir} does not exist`, @@ -1366,7 +1366,7 @@ export class Config { const removed = await cleanupStaleAgentWorktrees(root); if (removed > 0) { // Only the "actually removed something" path warrants - // `info` — that's the signal an operator chasing a leak + // `info` �that's the signal an operator chasing a leak // would grep for. The "ran, found nothing" path is // reconstructable at `debug` and is otherwise noise: // every CLI start that has any worktree dir would emit @@ -1408,12 +1408,12 @@ export class Config { * `mcp-client-update` event stream the UI subscribes to. * * Defensive against partially-stubbed `ToolRegistry` in some tests, where - * the manager getter is unavailable — we'd rather log-and-skip than crash + * the manager getter is unavailable �we'd rather log-and-skip than crash * the init path in tests that don't exercise MCP at all. */ private startMcpDiscoveryInBackground(): void { // `getMcpClientManager` is a public method on `ToolRegistry`. The - // cast below is NOT defensive against the production type — it + // cast below is NOT defensive against the production type �it // exists only because some tests (e.g. those using // `createMockToolRegistry`) stub `ToolRegistry` as a plain object // that doesn't implement the method. The optional-chaining call @@ -1421,7 +1421,7 @@ export class Config { // of crashing `initialize()` for tests that never exercise MCP. // // Crucially, the inner shape is `ReturnType` - // — not a hand-rolled `{ discoverAllMcpToolsIncremental: ... }` — so + // �not a hand-rolled `{ discoverAllMcpToolsIncremental: ... }` �so // a future rename of `getMcpClientManager` on `ToolRegistry` still // surfaces here as a type error rather than silently falling // through to the `if (!manager) return` branch. @@ -1444,13 +1444,13 @@ export class Config { // After background discovery completes, push the newly-registered // MCP tools into the active GeminiChat so the next model request // sees them. Interactive mode also calls setTools() via - // AppContainer's batch-flush effect — this trailing call is + // AppContainer's batch-flush effect �this trailing call is // idempotent there, but it's the ONLY path that updates // `chat.tools` for non-interactive runs (no AppContainer). // Without this, `chat.tools` would be frozen at the built-in-only - // snapshot taken inside `geminiClient.initialize()` → `startChat()`, + // snapshot taken inside `geminiClient.initialize()` �`startChat()`, // and `runNonInteractive` / stream-json / ACP would silently lose - // every MCP tool — a regression vs the legacy synchronous path. + // every MCP tool �a regression vs the legacy synchronous path. try { await this.geminiClient?.setTools(); } catch (err) { @@ -1473,7 +1473,7 @@ export class Config { * first model request sees the same tool surface the legacy * synchronous-MCP path produced. * - * Interactive code paths should NOT call this — `AppContainer`'s + * Interactive code paths should NOT call this �`AppContainer`'s * `mcp-client-update` subscriber handles `setTools()` refreshes * progressively without blocking the UI. * @@ -1501,7 +1501,7 @@ export class Config { * thread and per-server errors logged to stderr). Under PR-A's * progressive discovery, per-server errors are caught inside * `McpClientManager.discoverAllMcpToolsIncremental` and routed to - * profiler events + `mcp-client-update` notifications — both of which + * profiler events + `mcp-client-update` notifications �both of which * are invisible to a non-interactive run with only built-in stderr. * This helper closes that gap WITHOUT re-introducing the blocking * behavior. @@ -1732,7 +1732,7 @@ export class Config { try { this.chatRecordingService?.finalize(); } catch { - // Best-effort — don't block session switch + // Best-effort �don't block session switch } const previousSessionId = this.sessionId; @@ -1750,14 +1750,14 @@ export class Config { // placeholder despite the new session never having received the // file contents. Use the getter so the lazy own-property // initialization in getFileReadCache() applies even for Configs - // constructed via Object.create — those should clear their own + // constructed via Object.create �those should clear their own // cache, not the parent's. this.getFileReadCache().clear(); this.fileHistoryService = undefined; this.fileHistoryRestoredFromSessionData = false; refreshSessionContext(this.sessionId); // The commit-attribution singleton accumulates per-file AI edits - // and a session-scoped prompt counter — both stop being meaningful + // and a session-scoped prompt counter �both stop being meaningful // when the session resets. Without this, pending attributions // from the previous session could attach to a commit in the new // one, and the "N-shotted" PR label would span sessions. @@ -1776,7 +1776,7 @@ export class Config { // Only refresh when THIS process established its own sidecar at // startup (interactive UI). A non-interactive `/clear` (e.g. // qwen --prompt-interactive) must not delete a sibling shell's - // sidecar that happens to share the outgoing session id — + // sidecar that happens to share the outgoing session id � // mirrors kimi-cli PR #2082's "write only when a session is // established for this process" rule. if (this.runtimeStatusEnabled && previousSessionId !== this.sessionId) { @@ -2136,7 +2136,7 @@ export class Config { this.chatRecordingService?.finalize(); await this.chatRecordingService?.flush(); } catch { - // Best-effort — don't block shutdown + // Best-effort �don't block shutdown } this.skillManager?.stopWatching(); @@ -2196,7 +2196,7 @@ export class Config { * - settings.permissions.allow (persistent rules from all scopes) * - allowedTools param (SDK / argv auto-approve list) * - * Note: coreTools is intentionally excluded here — it has whitelist semantics + * Note: coreTools is intentionally excluded here �it has whitelist semantics * (only listed tools are registered), not auto-approve semantics. It is * handled separately via PermissionManager.coreToolsAllowList. * @@ -2790,9 +2790,15 @@ export class Config { !this.fileHistoryRestoredFromSessionData && this.sessionData?.fileHistorySnapshots ) { - this.fileHistoryService.restoreFromSnapshots( - this.sessionData.fileHistorySnapshots, - ); + try { + this.fileHistoryService.restoreFromSnapshots( + this.sessionData.fileHistorySnapshots, + ); + } catch (e) { + this.debugLogger.error( + `FileHistory: Failed to restore snapshots from session data: ${e}`, + ); + } this.fileHistoryRestoredFromSessionData = true; } return this.fileHistoryService; @@ -3307,7 +3313,7 @@ export class Config { * codebase constructs its Config via `Object.create(parent)`. That * does **not** run instance field initializers, so the parent's * `fileReadCache` field is reachable on the child only by prototype - * lookup — i.e. child and parent end up sharing the same cache. The + * lookup �i.e. child and parent end up sharing the same cache. The * own-property check below detects "this instance was made by * Object.create" and lazily attaches a fresh cache, ensuring * isolation without requiring every Object.create site to remember @@ -3316,7 +3322,7 @@ export class Config { getFileReadCache(): FileReadCache { if (!Object.prototype.hasOwnProperty.call(this, 'fileReadCache')) { // The own-property write needs to bypass `private`'s structural - // check — the field is conceptually still private to the class, + // check �the field is conceptually still private to the class, // we just need TS to let us install an own copy on a child // instance produced by `Object.create(parent)`. (this as unknown as { fileReadCache: FileReadCache }).fileReadCache = @@ -3331,7 +3337,7 @@ export class Config { * `file_unchanged` placeholder, no future prior-read enforcement). * Intended as an escape hatch for sessions where the cache's "model * has already seen this content earlier in the conversation" - * assumption is unreliable — e.g. after context compaction or + * assumption is unreliable �e.g. after context compaction or * transcript transformation. */ getFileReadCacheDisabled(): boolean { @@ -3423,7 +3429,7 @@ export class Config { ); // Helper: check permission then register a lazy factory (no module import - // happens here — the dynamic import() only runs when the tool is first used). + // happens here �the dynamic import() only runs when the tool is first used). const registerLazy = async ( toolName: ToolName, factory: ToolFactory, @@ -3450,7 +3456,7 @@ export class Config { // The synthetic structured_output tool is the terminal contract for // --json-schema runs. It must be registered in BOTH the bare-mode - // branch and the regular branch — without it the model can't finish + // branch and the regular branch �without it the model can't finish // a structured run, so omitting either branch causes // `qwen [--bare] --json-schema X -p "..."` to loop until // maxSessionTurns and exit via the "plain text" failure path. Hoisted @@ -3464,7 +3470,7 @@ export class Config { // and drain loops detect a successful structured_output call as // terminal. A subagent that called the tool would receive the // "Session will end now" llmContent, then keep running because its - // own loop has no termination handler — wasted tokens with no + // own loop has no termination handler �wasted tokens with no // structured payload surfacing on stdout. Strip the registration in // those contexts. const registerStructuredOutputIfRequested = async (): Promise => { diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index c832a666e7..fdba53626c 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -1284,7 +1284,9 @@ export class GeminiClient { ?.recordFileHistorySnapshot(latestSnapshot); } } catch (e) { - debugLogger.error(`FileHistory: makeSnapshot failed: ${e}`); + debugLogger.error( + `FileHistory: snapshot/record cycle failed for ${prompt_id}: ${e}`, + ); } } diff --git a/packages/core/src/services/fileHistoryService.test.ts b/packages/core/src/services/fileHistoryService.test.ts index ff154dbad2..02f760eefe 100644 --- a/packages/core/src/services/fileHistoryService.test.ts +++ b/packages/core/src/services/fileHistoryService.test.ts @@ -16,6 +16,7 @@ import { import { existsSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import type { FileHistoryBackup } from './fileHistoryService.js'; const mockStorageDir = vi.hoisted(() => vi.fn()); vi.mock('../config/storage.js', () => ({ @@ -111,7 +112,7 @@ describe('FileHistoryService', () => { // Replace the backup storage root with a regular file so the recursive // `mkdir(dirname(backupPath))` inside `safeCopyFile` fails with - // ENOTDIR — a non-ENOENT error that propagates back into `trackEdit`'s + // ENOTDIR �a non-ENOENT error that propagates back into `trackEdit`'s // catch. await rm(storageDir, { recursive: true, force: true }); await writeFile(storageDir, ''); @@ -122,8 +123,8 @@ describe('FileHistoryService', () => { // The sticky-failed guard symmetry test for trackEdit. After // makeSnapshot recorded a `failed: true` marker for a file (e.g. - // transient disk full), the next trackEdit invocation — typically - // triggered by a tool about to modify the same file — must NOT + // transient disk full), the next trackEdit invocation �typically + // triggered by a tool about to modify the same file �must NOT // skip just because the entry exists. It must attempt a fresh // backup; on success the failed marker is replaced. Without this // the failed flag stays sticky until the file content changes, @@ -137,7 +138,7 @@ describe('FileHistoryService', () => { // Force makeSnapshot's per-file backup to throw. The file content // is unchanged so checkOriginFileChanged short-circuits to "no - // change" — but we want the failure path here, so modify the file + // change" �but we want the failure path here, so modify the file // first to ensure createBackup is reached. await writeFile(file, 'p2-content'); await rm(storageDir, { recursive: true, force: true }); @@ -220,7 +221,7 @@ describe('FileHistoryService', () => { // When a per-file backup attempt throws inside makeSnapshot, the new // snapshot must NOT silently inherit the previous snapshot's backup - // and present it as the captured state of this turn — that would + // and present it as the captured state of this turn �that would // make a later rewind restore older content while reporting success. // Instead the snapshot records a `failed: true` marker so rewind // surfaces the file via filesFailed and getDiffStats omits it. @@ -232,7 +233,7 @@ describe('FileHistoryService', () => { await service.trackEdit(file); // Modify the file and break the backup target (replace storageDir - // with a regular file → ENOTDIR inside `safeCopyFile`'s recursive + // with a regular file �ENOTDIR inside `safeCopyFile`'s recursive // mkdir). The next makeSnapshot's per-file backup attempt throws. await writeFile(file, 'p2-content'); await rm(storageDir, { recursive: true, force: true }); @@ -314,7 +315,7 @@ describe('FileHistoryService', () => { await service.makeSnapshot('p1'); const file = join(projectDir, 'new-file.txt'); - await service.trackEdit(file); // non-existent → null backup + await service.trackEdit(file); // non-existent �null backup await writeFile(file, 'created'); await service.makeSnapshot('p2'); @@ -512,6 +513,34 @@ describe('FileHistoryService', () => { // Path outside cwd should be preserved as-is. expect(snapshots[0].trackedFileBackups[externalPath]).toBeDefined(); }); + + + it('should rehydrate string dates and skip malformed backups', async () => { + const fresh = new FileHistoryService('test-session', true, projectDir); + const absPath = join(projectDir, 'a.txt'); + + fresh.restoreFromSnapshots([ + { + promptId: 'p1', + trackedFileBackups: { + [absPath]: { + backupFileName: 'deadbeefcafebabe@v1', + version: 1, + backupTime: '2024-01-01T00:00:01.000Z' as unknown as Date, + }, + broken: null as unknown as FileHistoryBackup, + }, + timestamp: '2024-01-01T00:00:02.000Z' as unknown as Date, + }, + ]); + + const snapshot = fresh.getSnapshots()[0]; + expect(snapshot.timestamp).toBeInstanceOf(Date); + expect(snapshot.trackedFileBackups['a.txt']?.backupTime).toBeInstanceOf( + Date, + ); + expect(snapshot.trackedFileBackups['broken']).toBeUndefined(); + }); }); describe('snapshot eviction', () => { @@ -540,7 +569,7 @@ describe('FileHistoryService', () => { service.getSnapshots()[0].trackedFileBackups['a.txt']!.backupFileName!, ); - // 104 more snapshots, each with new content → fresh backup per snapshot. + // 104 more snapshots, each with new content �fresh backup per snapshot. for (let i = 1; i < 105; i++) { await writeFile(file, `v${i}`); await service.makeSnapshot(`p${i}`); @@ -573,12 +602,12 @@ describe('FileHistoryService', () => { const sharedName = service.getSnapshots()[0].trackedFileBackups['a.txt']!.backupFileName!; - // Content never changes → makeSnapshot reuses the same backup reference. + // Content never changes �makeSnapshot reuses the same backup reference. for (let i = 1; i < 105; i++) { await service.makeSnapshot(`p${i}`); } - // Same backupFileName is held by every survivor → must NOT be deleted. + // Same backupFileName is held by every survivor �must NOT be deleted. expect(existsSync(backupPath(sharedName))).toBe(true); }); }); diff --git a/packages/core/src/services/fileHistoryService.ts b/packages/core/src/services/fileHistoryService.ts index 0277a6e7e1..fb1707f40e 100644 --- a/packages/core/src/services/fileHistoryService.ts +++ b/packages/core/src/services/fileHistoryService.ts @@ -61,6 +61,21 @@ export interface RewindResult { const MAX_SNAPSHOTS = 100; const FILE_HISTORY_DIR = 'file-history'; +function toValidDate(value: unknown): Date | null { + if (value instanceof Date) { + return Number.isNaN(value.getTime()) ? null : value; + } + if (typeof value !== 'string' && typeof value !== 'number') { + return null; + } + const normalized = new Date(value); + return Number.isNaN(normalized.getTime()) ? null : normalized; +} + +function isFileHistoryBackupRecord(value: unknown): value is FileHistoryBackup { + return !!value && typeof value === 'object'; +} + function isENOENT(e: unknown): boolean { return ( typeof e === 'object' && @@ -311,24 +326,40 @@ export class FileHistoryService { const trackedFiles = new Set(); const migrated: FileHistorySnapshot[] = []; for (const snapshot of snapshots) { + if ( + !snapshot?.promptId || + !snapshot.trackedFileBackups || + typeof snapshot.trackedFileBackups !== 'object' + ) { + continue; + } + + const timestamp = toValidDate(snapshot.timestamp); + if (!timestamp) { + continue; + } + const trackedFileBackups: Record = {}; for (const [p, backup] of Object.entries(snapshot.trackedFileBackups)) { + if (!isFileHistoryBackupRecord(backup)) { + continue; + } + + const backupTime = toValidDate(backup.backupTime); + if (!backupTime) { + continue; + } + const trackingPath = this.maybeShortenFilePath(p); trackedFiles.add(trackingPath); trackedFileBackups[trackingPath] = { ...backup, - backupTime: - backup.backupTime instanceof Date - ? backup.backupTime - : new Date(backup.backupTime), + backupTime, }; } migrated.push({ ...snapshot, - timestamp: - snapshot.timestamp instanceof Date - ? snapshot.timestamp - : new Date(snapshot.timestamp), + timestamp, trackedFileBackups, }); } diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index b46e73cc00..0b239153d0 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -23,6 +23,7 @@ import { getResumePromptTokenCount, type ConversationRecord, } from './sessionService.js'; +import type { FileHistorySnapshotRecordPayload } from './chatRecordingService.js'; import { CompressionStatus } from '../core/turn.js'; import type { ChatRecord } from './chatRecordingService.js'; import * as jsonl from '../utils/jsonl-utils.js'; @@ -194,7 +195,7 @@ describe('SessionService', () => { it('should NOT populate messageCount during listing', async () => { // Listing must avoid the full-file readline that counting requires - // — message counts are now lazy and provided by + // �message counts are now lazy and provided by // `countSessionMessages(sessionId)` only when a UI surface (e.g. // a session preview) is about to display them. Pinning this // contract here so future refactors can't quietly re-introduce @@ -527,6 +528,48 @@ describe('SessionService', () => { expect(assistantMsg?.model).toBe('gemini-pro'); }); + + it('should ignore malformed file-history snapshot payloads', async () => { + const records: ChatRecord[] = [ + { + uuid: 'u1', + parentUuid: null, + sessionId: 'test', + timestamp: '2024-01-01T00:00:00Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'Hello' }] }, + cwd: '/test/project/root', + version: '1.0.0', + }, + { + uuid: 'bad-s1', + parentUuid: 'u1', + sessionId: 'test', + timestamp: '2024-01-01T00:00:01Z', + type: 'system', + subtype: 'file_history_snapshot', + systemPayload: { + version: 1, + snapshot: { + promptId: 'prompt-1', + }, + } as unknown as FileHistorySnapshotRecordPayload, + cwd: '/test/project/root', + version: '1.0.0', + }, + ]; + + statSyncSpy.mockReturnValue({ + mtimeMs: Date.now(), + isFile: () => true, + } as fs.Stats); + vi.mocked(jsonl.read).mockResolvedValue(records); + + const loaded = await sessionService.loadSession('test'); + + expect(loaded?.fileHistorySnapshots).toBeUndefined(); + }); + it('should expose persisted file-history snapshots from the active branch', async () => { const records: ChatRecord[] = [ { @@ -732,7 +775,7 @@ describe('SessionService', () => { // listSessions. Four contracts to pin: it actually counts what it // promises, it short-circuits on bad input without touching the disk, // it returns 0 on any read failure (caller must not see an exception - // bubble up — the picker treats 0 as "unknown"), and it scopes to + // bubble up �the picker treats 0 as "unknown"), and it scopes to // the current project (mirroring deleteSession/renameSession's // first-record cwd check). @@ -761,7 +804,7 @@ describe('SessionService', () => { } }); const lines = [ - // Two user records sharing a uuid — should be counted once + // Two user records sharing a uuid �should be counted once JSON.stringify({ uuid: 'u1', type: 'user' }), JSON.stringify({ uuid: 'u1', type: 'user' }), JSON.stringify({ uuid: 'a1', type: 'assistant' }), @@ -793,7 +836,7 @@ describe('SessionService', () => { it('should return 0 when the session file is missing (ENOENT)', async () => { // The first-record read fires before the count stream, so simulate - // ENOENT there too — readLines surfaces it as a thrown error. + // ENOENT there too �readLines surfaces it as a thrown error. vi.mocked(jsonl.readLines).mockRejectedValue( Object.assign(new Error('ENOENT'), { code: 'ENOENT' }), ); @@ -822,7 +865,7 @@ describe('SessionService', () => { const count = await sessionService.countSessionMessages(sessionIdA); expect(count).toBe(0); - // No streaming pass should have started — the project check + // No streaming pass should have started �the project check // short-circuits before the expensive part. expect(createReadStreamSpy).not.toHaveBeenCalled(); }); @@ -1596,7 +1639,7 @@ describe('SessionService', () => { }); describe('findSessionTitlesByPrefix', () => { - // Uses real disk like forkSession — readSessionTitleInfoFromFile reads + // Uses real disk like forkSession �readSessionTitleInfoFromFile reads // the file tail for the custom_title record, so mocks would defeat the // method. Mirrors the forkSession describe's setup verbatim so the tmp // sandbox + un-mocked path/jsonl utilities are in place. @@ -1732,7 +1775,7 @@ describe('SessionService', () => { cwd, ); // Same chats dir (sessions are stored under projectHash anyway), but - // the record's cwd belongs to another project → must be skipped. + // the record's cwd belongs to another project �must be skipped. seedSessionWithTitle( '22222222-2222-2222-2222-222222222222', 'shared (Branch 2)', diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index 1dd67b92a3..8b87b7f540 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -1298,6 +1298,21 @@ export function replayUiTelemetryFromConversation( } } + +function isFileHistorySnapshotRecordPayload( + payload: FileHistorySnapshotRecordPayload | undefined, +): payload is FileHistorySnapshotRecordPayload { + const snapshot = payload?.snapshot; + return ( + payload?.version === 1 && + !!snapshot && + typeof snapshot.promptId === 'string' && + !!snapshot.promptId && + !!snapshot.trackedFileBackups && + typeof snapshot.trackedFileBackups === 'object' + ); +} + function extractFileHistorySnapshots( messages: ChatRecord[], ): FileHistorySnapshot[] { @@ -1310,12 +1325,11 @@ function extractFileHistorySnapshots( const payload = record.systemPayload as | FileHistorySnapshotRecordPayload | undefined; - const snapshot = payload?.snapshot; - if (payload?.version !== 1 || !snapshot) { + if (!isFileHistorySnapshotRecordPayload(payload)) { continue; } - snapshots.push(snapshot); + snapshots.push(payload.snapshot); } return snapshots;