fix(checkpoint): Prevent silent failure and enable for non-Git projects (#4144)

This commit is contained in:
Abhi 2025-07-14 13:23:51 -04:00 committed by GitHub
parent 2f1d6234de
commit 9dc812dd4b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 38 additions and 19 deletions

View File

@ -17,6 +17,7 @@ import {
createContentGeneratorConfig, createContentGeneratorConfig,
} from '../core/contentGenerator.js'; } from '../core/contentGenerator.js';
import { GeminiClient } from '../core/client.js'; import { GeminiClient } from '../core/client.js';
import { GitService } from '../services/gitService.js';
import { loadServerHierarchicalMemory } from '../utils/memoryDiscovery.js'; import { loadServerHierarchicalMemory } from '../utils/memoryDiscovery.js';
// Mock dependencies that might be called during Config construction or createServerConfig // Mock dependencies that might be called during Config construction or createServerConfig
@ -75,6 +76,12 @@ vi.mock('../telemetry/index.js', async (importOriginal) => {
}; };
}); });
vi.mock('../services/gitService.js', () => {
const GitServiceMock = vi.fn();
GitServiceMock.prototype.initialize = vi.fn();
return { GitService: GitServiceMock };
});
describe('Server Config (config.ts)', () => { describe('Server Config (config.ts)', () => {
const MODEL = 'gemini-pro'; const MODEL = 'gemini-pro';
const SANDBOX: SandboxConfig = { const SANDBOX: SandboxConfig = {
@ -108,6 +115,32 @@ describe('Server Config (config.ts)', () => {
vi.clearAllMocks(); vi.clearAllMocks();
}); });
describe('initialize', () => {
it('should throw an error if checkpointing is enabled and GitService fails', async () => {
const gitError = new Error('Git is not installed');
(GitService.prototype.initialize as Mock).mockRejectedValue(gitError);
const config = new Config({
...baseParams,
checkpointing: true,
});
await expect(config.initialize()).rejects.toThrow(gitError);
});
it('should not throw an error if checkpointing is disabled and GitService fails', async () => {
const gitError = new Error('Git is not installed');
(GitService.prototype.initialize as Mock).mockRejectedValue(gitError);
const config = new Config({
...baseParams,
checkpointing: false,
});
await expect(config.initialize()).resolves.toBeUndefined();
});
});
describe('refreshAuth', () => { describe('refreshAuth', () => {
it('should refresh auth and update config', async () => { it('should refresh auth and update config', async () => {
const config = new Config(baseParams); const config = new Config(baseParams);

View File

@ -259,11 +259,7 @@ export class Config {
// Initialize centralized FileDiscoveryService // Initialize centralized FileDiscoveryService
this.getFileService(); this.getFileService();
if (this.getCheckpointingEnabled()) { if (this.getCheckpointingEnabled()) {
try { await this.getGitService();
await this.getGitService();
} catch {
// For now swallow the error, later log it.
}
} }
this.toolRegistry = await this.createToolRegistry(); this.toolRegistry = await this.createToolRegistry();
} }

View File

@ -154,14 +154,6 @@ describe('GitService', () => {
}); });
describe('initialize', () => { describe('initialize', () => {
it('should throw an error if projectRoot is not a Git repository', async () => {
hoistedIsGitRepositoryMock.mockReturnValue(false);
const service = new GitService(mockProjectRoot);
await expect(service.initialize()).rejects.toThrow(
'GitService requires a Git repository',
);
});
it('should throw an error if Git is not available', async () => { it('should throw an error if Git is not available', async () => {
hoistedMockExec.mockImplementation((command, callback) => { hoistedMockExec.mockImplementation((command, callback) => {
callback(new Error('git not found')); callback(new Error('git not found'));
@ -169,7 +161,7 @@ describe('GitService', () => {
}); });
const service = new GitService(mockProjectRoot); const service = new GitService(mockProjectRoot);
await expect(service.initialize()).rejects.toThrow( await expect(service.initialize()).rejects.toThrow(
'GitService requires Git to be installed', 'Checkpointing is enabled, but Git is not installed. Please install Git or disable checkpointing to continue.',
); );
}); });

View File

@ -8,7 +8,6 @@ import * as fs from 'fs/promises';
import * as path from 'path'; import * as path from 'path';
import * as os from 'os'; import * as os from 'os';
import { isNodeError } from '../utils/errors.js'; import { isNodeError } from '../utils/errors.js';
import { isGitRepository } from '../utils/gitUtils.js';
import { exec } from 'node:child_process'; import { exec } from 'node:child_process';
import { simpleGit, SimpleGit, CheckRepoActions } from 'simple-git'; import { simpleGit, SimpleGit, CheckRepoActions } from 'simple-git';
import { getProjectHash, GEMINI_DIR } from '../utils/paths.js'; import { getProjectHash, GEMINI_DIR } from '../utils/paths.js';
@ -26,12 +25,11 @@ export class GitService {
} }
async initialize(): Promise<void> { async initialize(): Promise<void> {
if (!isGitRepository(this.projectRoot)) {
throw new Error('GitService requires a Git repository');
}
const gitAvailable = await this.verifyGitAvailability(); const gitAvailable = await this.verifyGitAvailability();
if (!gitAvailable) { if (!gitAvailable) {
throw new Error('GitService requires Git to be installed'); throw new Error(
'Checkpointing is enabled, but Git is not installed. Please install Git or disable checkpointing to continue.',
);
} }
this.setupShadowGitRepository(); this.setupShadowGitRepository();
} }