fix(settings): handle home-directory project/global settings collision (#1117)
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
@@ -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<HooksConfig> {
|
||||
// Avoid reading global settings as project settings when cwd is HOME.
|
||||
if (isProjectSettingsPathCollidingWithGlobal(workingDirectory)) {
|
||||
return {};
|
||||
}
|
||||
|
||||
try {
|
||||
// Ensure project settings are loaded
|
||||
try {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<ProjectSettings> {
|
||||
// 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<ProjectSettings>,
|
||||
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<Settings> = {};
|
||||
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<void> {
|
||||
// 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");
|
||||
}
|
||||
|
||||
@@ -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({});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
// ============================================================================
|
||||
|
||||
Reference in New Issue
Block a user