Fimeg/letta-code
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix: prevent tests from overwriting real settings.json (#49)

Browse Source 
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
Charles Packer
2025-11-03 19:20:55 -08:00
committed by GitHub
parent 5c3b303eac
commit f8107c84f4
2 changed files with 60 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
src/settings-manager.ts
View File

@@ -49,6 +49,7 @@ class SettingsManager {
private projectSettings: Map<string, ProjectSettings> = new Map();
private localProjectSettings: Map<string, LocalProjectSettings> = new Map();
private initialized = false;
private pendingWrites = new Set<Promise<void>>();
/**
* Initialize the settings manager (loads from disk)
@@ -112,10 +113,15 @@ class SettingsManager {
this.settings = { ...this.settings, ...updates };
// Persist asynchronously (fire and forget, errors logged)
this.persistSettings().catch((error) => {
console.error("Failed to persist settings:", error);
});
// Persist asynchronously (track promise for testing)
const writePromise = this.persistSettings()
.catch((error) => {
console.error("Failed to persist settings:", error);
})
.finally(() => {
this.pendingWrites.delete(writePromise);
});
this.pendingWrites.add(writePromise);
}
/**
@@ -189,10 +195,15 @@ class SettingsManager {
const updated = { ...current, ...updates };
this.projectSettings.set(workingDirectory, updated);
// Persist asynchronously
this.persistProjectSettings(workingDirectory).catch((error) => {
console.error("Failed to persist project settings:", error);
});
// Persist asynchronously (track promise for testing)
const writePromise = this.persistProjectSettings(workingDirectory)
.catch((error) => {
console.error("Failed to persist project settings:", error);
})
.finally(() => {
this.pendingWrites.delete(writePromise);
});
this.pendingWrites.add(writePromise);
}
/**
@@ -202,7 +213,8 @@ class SettingsManager {
if (!this.settings) return;
const settingsPath = this.getSettingsPath();
const dirPath = join(homedir(), ".letta");
const home = process.env.HOME || homedir();
const dirPath = join(home, ".letta");
try {
if (!exists(dirPath)) {
@@ -254,7 +266,9 @@ class SettingsManager {
}
private getSettingsPath(): string {
return join(homedir(), ".letta", "settings.json");
// Respect process.env.HOME for testing (homedir() ignores it)
const home = process.env.HOME || homedir();
return join(home, ".letta", "settings.json");
}
private getProjectSettingsPath(workingDirectory: string): string {
@@ -334,10 +348,15 @@ class SettingsManager {
const updated = { ...current, ...updates };
this.localProjectSettings.set(workingDirectory, updated);
// Persist asynchronously
this.persistLocalProjectSettings(workingDirectory).catch((error) => {
console.error("Failed to persist local project settings:", error);
});
// Persist asynchronously (track promise for testing)
const writePromise = this.persistLocalProjectSettings(workingDirectory)
.catch((error) => {
console.error("Failed to persist local project settings:", error);
})
.finally(() => {
this.pendingWrites.delete(writePromise);
});
this.pendingWrites.add(writePromise);
}
/**
@@ -366,13 +385,26 @@ class SettingsManager {
}
/**
* Reset the manager (mainly for testing)
* Wait for all pending writes to complete.
* Useful in tests to ensure writes finish before cleanup.
*/
reset(): void {
async flush(): Promise<void> {
await Promise.all(Array.from(this.pendingWrites));
}
/**
* Reset the manager (mainly for testing).
* Waits for pending writes to complete before resetting.
*/
async reset(): Promise<void> {
// Wait for pending writes BEFORE clearing state
await this.flush();
this.settings = null;
this.projectSettings.clear();
this.localProjectSettings.clear();
this.initialized = false;
this.pendingWrites.clear();
}
}

23
src/tests/settings-manager.test.ts
View File

@@ -11,7 +11,7 @@ let testProjectDir: string;
beforeEach(async () => {
// Reset settings manager FIRST before changing HOME
settingsManager.reset();
await settingsManager.reset();
// Create temporary directories for testing
testHomeDir = await mkdtemp(join(tmpdir(), "letta-test-home-"));
@@ -22,15 +22,16 @@ beforeEach(async () => {
});
afterEach(async () => {
// Wait for all pending writes to complete BEFORE restoring HOME
// This prevents test writes from leaking into real settings after HOME is restored
await settingsManager.reset();
// Clean up test directories
await rm(testHomeDir, { recursive: true, force: true });
await rm(testProjectDir, { recursive: true, force: true });
// Restore original HOME
// Restore original HOME AFTER reset completes
process.env.HOME = originalHome;
// Reset settings manager after each test
settingsManager.reset();
});
// ============================================================================
@@ -63,7 +64,7 @@ describe("Settings Manager - Initialization", () => {
await new Promise((resolve) => setTimeout(resolve, 100));
// Reset and re-initialize
settingsManager.reset();
await settingsManager.reset();
await settingsManager.initialize();
const settings = settingsManager.getSettings();
@@ -194,7 +195,7 @@ describe("Settings Manager - Global Settings", () => {
await new Promise((resolve) => setTimeout(resolve, 100));
// Reset and reload
settingsManager.reset();
await settingsManager.reset();
await settingsManager.initialize();
const settings = settingsManager.getSettings();
@@ -265,7 +266,7 @@ describe("Settings Manager - Project Settings", () => {
await new Promise((resolve) => setTimeout(resolve, 100));
// Clear cache and reload
settingsManager.reset();
await settingsManager.reset();
await settingsManager.initialize();
const reloaded = await settingsManager.loadProjectSettings(testProjectDir);
@@ -356,7 +357,7 @@ describe("Settings Manager - Local Project Settings", () => {
await new Promise((resolve) => setTimeout(resolve, 100));
// Clear cache and reload
settingsManager.reset();
await settingsManager.reset();
await settingsManager.initialize();
const reloaded =
await settingsManager.loadLocalProjectSettings(testProjectDir);
@@ -443,7 +444,7 @@ describe("Settings Manager - Reset", () => {
await settingsManager.initialize();
settingsManager.updateSettings({ lastAgent: "agent-reset-test" });
settingsManager.reset();
await settingsManager.reset();
// Should throw error after reset
expect(() => settingsManager.getSettings()).toThrow();
@@ -456,7 +457,7 @@ describe("Settings Manager - Reset", () => {
// Wait for persist
await new Promise((resolve) => setTimeout(resolve, 100));
settingsManager.reset();
await settingsManager.reset();
await settingsManager.initialize();
const settings = settingsManager.getSettings();