Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion packages/core/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -414,14 +415,53 @@ 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', () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Nice to have] Indentation regression: this it(...) line was re-indented from 4 spaces to 2 spaces, breaking alignment with sibling test cases in the same describe block. Likely an accidental edit during rebase.

Suggested change
it('refreshes the telemetry session context with the new session ID', () => {
it('refreshes the telemetry session context with the new session ID', () => {

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

const config = new Config(baseParams);
vi.mocked(refreshSessionContext).mockClear();

const newSessionId = config.startNewSession();

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);
const restoredBackup = Object.values(
snapshots[0]?.trackedFileBackups ?? {},
)[0];
expect(restoredBackup?.backupTime).toBeInstanceOf(Date);
});
});

it('should expose LSP status from the configured client', () => {
Expand Down
83 changes: 50 additions & 33 deletions packages/core/src/config/config.ts

Large diffs are not rendered by default.

13 changes: 11 additions & 2 deletions packages/core/src/core/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1275,9 +1275,18 @@ 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}`);
debugLogger.error(
`FileHistory: snapshot/record cycle failed for ${prompt_id}: ${e}`,
);
}
}

Expand Down
28 changes: 28 additions & 0 deletions packages/core/src/services/chatRecordingService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions packages/core/src/services/chatRecordingService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -229,6 +230,7 @@ export interface ChatRecord {
| 'ui_telemetry'
| 'at_command'
| 'attribution_snapshot'
| 'file_history_snapshot'
| 'notification'
| 'cron'
| 'mid_turn_user_message'
Expand Down Expand Up @@ -278,6 +280,7 @@ export interface ChatRecord {
| UiTelemetryRecordPayload
| AtCommandRecordPayload
| AttributionSnapshotPayload
| FileHistorySnapshotRecordPayload
| CustomTitleRecordPayload
| NotificationRecordPayload
| RewindRecordPayload
Expand Down Expand Up @@ -418,6 +421,11 @@ export interface AttributionSnapshotPayload {
snapshot: AttributionSnapshot;
}

export interface FileHistorySnapshotRecordPayload {
version: 1;
snapshot: FileHistorySnapshot;
}

/**
* Stored payload for conversation rewind events.
*/
Expand Down Expand Up @@ -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);
}
}
}
49 changes: 39 additions & 10 deletions packages/core/src/services/fileHistoryService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -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, '');
Expand All @@ -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,
Expand All @@ -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 });
Expand Down Expand Up @@ -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.
Expand All @@ -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 });
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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}`);
Expand Down Expand Up @@ -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);
});
});
Expand Down
48 changes: 46 additions & 2 deletions packages/core/src/services/fileHistoryService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The type guard isFileHistoryBackupRecord accepts any non-null object (!!value && typeof value === 'object'), including arrays and objects without any FileHistoryBackup fields. While downstream toValidDate(backup.backupTime) catches entries missing backupTime, an object with a valid backupTime but missing backupFileName would pass through and produce undefined in path resolution.

Suggested change
function isFileHistoryBackupRecord(value: unknown): value is FileHistoryBackup {
function isFileHistoryBackupRecord(value: unknown): value is FileHistoryBackup {
return (
!!value &&
typeof value === 'object' &&
!Array.isArray(value) &&
'version' in value &&
typeof (value as FileHistoryBackup).version === 'number'
);
}

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

return !!value && typeof value === 'object';
}

function isENOENT(e: unknown): boolean {
return (
typeof e === 'object' &&
Expand Down Expand Up @@ -311,13 +326,42 @@ export class FileHistoryService {
const trackedFiles = new Set<string>();
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<string, FileHistoryBackup> = {};
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;
trackedFileBackups[trackingPath] = {
...backup,
backupTime,
};
}
migrated.push({ ...snapshot, trackedFileBackups });
migrated.push({
...snapshot,
timestamp,
trackedFileBackups,
});
}
this.state = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] restoreFromSnapshots sets this.state.snapshots = migrated without enforcing the MAX_SNAPSHOTS = 100 cap. Eviction only runs inside makeSnapshot. If the JSONL contains more than 100 snapshot records (long session, data corruption), all are loaded into memory and the orphaned-backup cleanup that eviction triggers is never reached for the extras.

Consider capping before assignment:

if (migrated.length > MAX_SNAPSHOTS) {
  migrated = migrated.slice(migrated.length - MAX_SNAPSHOTS);
}

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

snapshots: migrated,
Expand Down
Loading
Loading