From 9af69dee6b7a36f9275f63b4545f51aab85f9da9 Mon Sep 17 00:00:00 2001 From: jnjpng Date: Mon, 23 Feb 2026 18:25:01 -0800 Subject: [PATCH] fix(settings): handle home-directory project/global settings collision (#1117) Co-authored-by: Letta --- src/hooks/loader.ts | 26 +++++++++++++++++ src/hooks/writer.ts | 31 ++++++++++++++++++++ src/settings-manager.ts | 40 ++++++++++++++++++++++++- src/tests/hooks/loader.test.ts | 37 +++++++++++++++++++++++ src/tests/settings-manager.test.ts | 47 ++++++++++++++++++++++++++++++ 5 files changed, 180 insertions(+), 1 deletion(-) diff --git a/src/hooks/loader.ts b/src/hooks/loader.ts index 55057b2..12171a5 100644 --- a/src/hooks/loader.ts +++ b/src/hooks/loader.ts @@ -1,6 +1,8 @@ // src/hooks/loader.ts // Loads and matches hooks from settings-manager +import { homedir } from "node:os"; +import { resolve } from "node:path"; import { settingsManager } from "../settings-manager"; import { debugLog } from "../utils/debug"; import { @@ -23,6 +25,25 @@ export function clearHooksCache(): void { // Settings-manager handles caching } +/** + * Check whether project settings path collides with global settings path. + * + * When cwd is HOME, both resolve to ~/.letta/settings.json. In that case, + * treat project hooks as empty so global hooks don't get merged twice. + */ +function isProjectSettingsPathCollidingWithGlobal( + workingDirectory: string, +): boolean { + const home = process.env.HOME || homedir(); + const globalSettingsPath = resolve(home, ".letta", "settings.json"); + const projectSettingsPath = resolve( + workingDirectory, + ".letta", + "settings.json", + ); + return globalSettingsPath === projectSettingsPath; +} + /** * Load global hooks configuration from ~/.letta/settings.json * Uses settings-manager cache (loaded at app startup) @@ -44,6 +65,11 @@ export function loadGlobalHooks(): HooksConfig { export async function loadProjectHooks( workingDirectory: string = process.cwd(), ): Promise { + // Avoid reading global settings as project settings when cwd is HOME. + if (isProjectSettingsPathCollidingWithGlobal(workingDirectory)) { + return {}; + } + try { // Ensure project settings are loaded try { diff --git a/src/hooks/writer.ts b/src/hooks/writer.ts index 7d97451..eded601 100644 --- a/src/hooks/writer.ts +++ b/src/hooks/writer.ts @@ -1,6 +1,8 @@ // src/hooks/writer.ts // Functions to write hooks to settings files via settings-manager +import { homedir } from "node:os"; +import { resolve } from "node:path"; import { settingsManager } from "../settings-manager"; import { debugLog } from "../utils/debug"; import { @@ -18,6 +20,24 @@ import { */ export type SaveLocation = "user" | "project" | "project-local"; +/** + * Check whether project settings path collides with global settings path. + * + * When cwd is HOME, both resolve to ~/.letta/settings.json. + */ +function isProjectSettingsPathCollidingWithGlobal( + workingDirectory: string, +): boolean { + const home = process.env.HOME || homedir(); + const globalSettingsPath = resolve(home, ".letta", "settings.json"); + const projectSettingsPath = resolve( + workingDirectory, + ".letta", + "settings.json", + ); + return globalSettingsPath === projectSettingsPath; +} + /** * Load hooks config from a specific location */ @@ -30,6 +50,10 @@ export function loadHooksFromLocation( case "user": return settingsManager.getSettings().hooks || {}; case "project": + if (isProjectSettingsPathCollidingWithGlobal(workingDirectory)) { + // Avoid showing global hooks twice (once as project, once as user) + return {}; + } return ( settingsManager.getProjectSettings(workingDirectory)?.hooks || {} ); @@ -60,6 +84,13 @@ export async function saveHooksToLocation( settingsManager.updateSettings({ hooks }); break; case "project": + // If cwd is HOME, project settings path equals global settings path. + // Persist as user settings to avoid duplicate project/global hook views. + if (isProjectSettingsPathCollidingWithGlobal(workingDirectory)) { + settingsManager.updateSettings({ hooks }); + break; + } + // Load project settings if not already loaded try { settingsManager.getProjectSettings(workingDirectory); diff --git a/src/settings-manager.ts b/src/settings-manager.ts index b3c4cf8..e02c5c6 100644 --- a/src/settings-manager.ts +++ b/src/settings-manager.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"; import { homedir } from "node:os"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import type { HooksConfig } from "./hooks/types"; import type { PermissionRules } from "./permissions/types"; import { debugWarn } from "./utils/debug.js"; @@ -567,6 +567,14 @@ class SettingsManager { async loadProjectSettings( workingDirectory: string = process.cwd(), ): Promise { + // If cwd is HOME, .letta/settings.json is the global settings file. + // Never treat it as project settings or we risk duplicate project/global behavior. + if (this.isProjectSettingsPathCollidingWithGlobal(workingDirectory)) { + const defaults = { ...DEFAULT_PROJECT_SETTINGS }; + this.projectSettings.set(workingDirectory, defaults); + return defaults; + } + // Check cache first const cached = this.projectSettings.get(workingDirectory); if (cached) { @@ -624,6 +632,22 @@ class SettingsManager { updates: Partial, workingDirectory: string = process.cwd(), ): void { + // If cwd is HOME, project settings path collides with global settings path. + // Route overlapping keys to user settings and avoid writing project scope. + if (this.isProjectSettingsPathCollidingWithGlobal(workingDirectory)) { + const globalUpdates: Partial = {}; + if ("hooks" in updates) { + globalUpdates.hooks = updates.hooks; + } + if ("statusLine" in updates) { + globalUpdates.statusLine = updates.statusLine; + } + if (Object.keys(globalUpdates).length > 0) { + this.updateSettings(globalUpdates); + } + return; + } + const current = this.projectSettings.get(workingDirectory); if (!current) { throw new Error( @@ -699,6 +723,11 @@ class SettingsManager { private async persistProjectSettings( workingDirectory: string, ): Promise { + // Safety guard: never persist project settings into global settings path. + if (this.isProjectSettingsPathCollidingWithGlobal(workingDirectory)) { + return; + } + const settings = this.projectSettings.get(workingDirectory); if (!settings) return; @@ -741,6 +770,15 @@ class SettingsManager { return join(workingDirectory, ".letta", "settings.json"); } + private isProjectSettingsPathCollidingWithGlobal( + workingDirectory: string, + ): boolean { + return ( + resolve(this.getProjectSettingsPath(workingDirectory)) === + resolve(this.getSettingsPath()) + ); + } + private getLocalProjectSettingsPath(workingDirectory: string): string { return join(workingDirectory, ".letta", "settings.local.json"); } diff --git a/src/tests/hooks/loader.test.ts b/src/tests/hooks/loader.test.ts index 39c8395..fe94994 100644 --- a/src/tests/hooks/loader.test.ts +++ b/src/tests/hooks/loader.test.ts @@ -683,5 +683,42 @@ describe("Hooks Loader", () => { "project", ); }); + + test("does not double-load global hooks when cwd is HOME", async () => { + const globalSettingsDir = join(fakeHome, ".letta"); + mkdirSync(globalSettingsDir, { recursive: true }); + + writeFileSync( + join(globalSettingsDir, "settings.json"), + JSON.stringify({ + hooks: { + Notification: [ + { + hooks: [ + { type: "command", command: "echo home-global-notify" }, + ], + }, + ], + }, + }), + ); + + // Re-initialize so global settings are re-read from disk after test writes. + await settingsManager.reset(); + await settingsManager.initialize(); + + const hooks = await loadHooks(fakeHome); + const notificationHooks = getMatchingHooks(hooks, "Notification"); + + expect(notificationHooks).toHaveLength(1); + expect(asCommand(notificationHooks[0])?.command).toBe( + "echo home-global-notify", + ); + + // In HOME, project settings path collides with global settings path. + // Project hooks should be treated as empty to avoid duplicate merging. + const projectHooks = await loadProjectHooks(fakeHome); + expect(projectHooks).toEqual({}); + }); }); }); diff --git a/src/tests/settings-manager.test.ts b/src/tests/settings-manager.test.ts index 37947a9..7d2ba40 100644 --- a/src/tests/settings-manager.test.ts +++ b/src/tests/settings-manager.test.ts @@ -339,6 +339,53 @@ describe("Settings Manager - Project Settings", () => { "Project settings for", ); }); + + test("When cwd is HOME, project settings resolve to defaults (no global collision)", async () => { + await settingsManager.initialize(); + + // Seed a global statusLine config in ~/.letta/settings.json + settingsManager.updateSettings({ + statusLine: { command: "echo global-status" }, + }); + await new Promise((resolve) => setTimeout(resolve, 100)); + + const projectSettings = + await settingsManager.loadProjectSettings(testHomeDir); + expect(projectSettings.localSharedBlockIds).toEqual({}); + expect(projectSettings.statusLine).toBeUndefined(); + }); + + test("When cwd is HOME, project hook/statusLine updates route to global settings", async () => { + await settingsManager.initialize(); + + // Load project settings for HOME (will be defaults due to collision guard) + await settingsManager.loadProjectSettings(testHomeDir); + + settingsManager.updateProjectSettings( + { + statusLine: { command: "echo routed-status" }, + hooks: { + Notification: [ + { + hooks: [{ type: "command", command: "echo routed-hook" }], + }, + ], + }, + }, + testHomeDir, + ); + + await new Promise((resolve) => setTimeout(resolve, 100)); + + const globalSettings = settingsManager.getSettings(); + expect(globalSettings.statusLine?.command).toBe("echo routed-status"); + expect( + asCommand(globalSettings.hooks?.Notification?.[0]?.hooks[0])?.command, + ).toBe("echo routed-hook"); + + // Ensure project-only field is not written into global file by this route + expect(globalSettings).not.toHaveProperty("localSharedBlockIds"); + }); }); // ============================================================================