From ada970a0977e1ccd911377f146a6587ced66e0dd Mon Sep 17 00:00:00 2001 From: Dora Salib Date: Fri, 20 Mar 2026 19:09:05 -0700 Subject: [PATCH] fix(memory): use field-level guards in applyMemorySearchDefaults Addresses review feedback: the all-or-nothing opt-out guard was inconsistent with sibling apply* functions and would silently suppress all smart defaults for users who set any unrelated memorySearch field (e.g. just changing their embedding provider). Now mirrors applyContextPruningDefaults pattern: each sub-feature is only skipped when its specific controlling field is already set. - Setting memorySearch.provider still gets session indexing + hybrid defaults - Setting memorySearch.sources skips session indexing but still gets hybrid - Setting query.hybrid skips hybrid defaults but still gets session indexing Added three partial-config test cases to cover this behavior. --- .../config.memory-search-defaults.test.ts | 69 ++++++++++++++++--- src/config/defaults.ts | 55 +++++++++------ 2 files changed, 95 insertions(+), 29 deletions(-) diff --git a/src/config/config.memory-search-defaults.test.ts b/src/config/config.memory-search-defaults.test.ts index f18b8734f1c..0bbd2cd3483 100644 --- a/src/config/config.memory-search-defaults.test.ts +++ b/src/config/config.memory-search-defaults.test.ts @@ -8,24 +8,47 @@ describe("config memory search defaults", () => { const cfg = loadConfig(); const ms = cfg.agents?.defaults?.memorySearch; - // Session memory indexing should be on expect((ms?.experimental as Record)?.sessionMemory).toBe(true); expect(ms?.sources).toContain("sessions"); expect(ms?.sources).toContain("memory"); - // Hybrid search with temporal decay const hybrid = ms?.query?.hybrid; expect(hybrid?.enabled).toBe(true); expect(hybrid?.temporalDecay?.enabled).toBe(true); expect(hybrid?.temporalDecay?.halfLifeDays).toBe(30); - - // MMR re-ranking expect(hybrid?.mmr?.enabled).toBe(true); expect(hybrid?.mmr?.lambda).toBe(0.7); }); }); - it("does not overwrite explicit memorySearch config", async () => { + it("still applies hybrid defaults when user only sets an unrelated field like provider", async () => { + await withTempHomeConfig( + { + agents: { + defaults: { + memorySearch: { + provider: "openai", + }, + }, + }, + }, + async () => { + const cfg = loadConfig(); + const ms = cfg.agents?.defaults?.memorySearch; + + // User's explicit field preserved + expect(ms?.provider).toBe("openai"); + + // Smart defaults still applied — user only set provider, not these + expect((ms?.experimental as Record)?.sessionMemory).toBe(true); + expect(ms?.sources).toContain("sessions"); + expect(ms?.query?.hybrid?.temporalDecay?.enabled).toBe(true); + expect(ms?.query?.hybrid?.mmr?.enabled).toBe(true); + }, + ); + }); + + it("does not overwrite explicit sources config", async () => { await withTempHomeConfig( { agents: { @@ -40,12 +63,42 @@ describe("config memory search defaults", () => { const cfg = loadConfig(); const ms = cfg.agents?.defaults?.memorySearch; - // User explicitly set sources — respect it, don't inject sessions + // User explicitly restricted sources — respect it expect(ms?.sources).toEqual(["memory"]); expect(ms?.sources).not.toContain("sessions"); - // No defaults injected since user provided explicit config - expect((ms?.experimental as Record)?.sessionMemory).toBeUndefined(); + // Hybrid defaults still applied since query.hybrid wasn't set + expect(ms?.query?.hybrid?.temporalDecay?.enabled).toBe(true); + }, + ); + }); + + it("does not overwrite explicit query.hybrid config", async () => { + await withTempHomeConfig( + { + agents: { + defaults: { + memorySearch: { + query: { + hybrid: { + enabled: false, + }, + }, + }, + }, + }, + }, + async () => { + const cfg = loadConfig(); + const ms = cfg.agents?.defaults?.memorySearch; + + // User explicitly disabled hybrid — respect it, don't inject our defaults + expect(ms?.query?.hybrid?.enabled).toBe(false); + expect(ms?.query?.hybrid?.temporalDecay).toBeUndefined(); + expect(ms?.query?.hybrid?.mmr).toBeUndefined(); + + // Session indexing defaults still applied since sources/experimental weren't set + expect((ms?.experimental as Record)?.sessionMemory).toBe(true); }, ); }); diff --git a/src/config/defaults.ts b/src/config/defaults.ts index ec511a3992f..d16e6dc2856 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -558,8 +558,39 @@ export function applyMemorySearchDefaults(cfg: OpenClawConfig): OpenClawConfig { return cfg; } - // Don't touch anything if the user has explicitly configured memorySearch - if (defaults.memorySearch !== undefined) { + const existing = defaults.memorySearch ?? {}; + let next = { ...existing }; + let mutated = false; + + // Only apply session indexing defaults if the user hasn't touched either field + if (existing.experimental?.sessionMemory === undefined && existing.sources === undefined) { + next = { + ...next, + experimental: { ...existing.experimental, sessionMemory: true }, + sources: ["memory", "sessions"], + }; + mutated = true; + } + + // Only apply hybrid search defaults if the user hasn't configured query.hybrid at all + if (existing.query?.hybrid === undefined) { + next = { + ...next, + query: { + ...existing.query, + hybrid: { + enabled: true, + vectorWeight: 0.7, + textWeight: 0.3, + temporalDecay: { enabled: true, halfLifeDays: 30 }, + mmr: { enabled: true, lambda: 0.7 }, + }, + }, + }; + mutated = true; + } + + if (!mutated) { return cfg; } @@ -569,25 +600,7 @@ export function applyMemorySearchDefaults(cfg: OpenClawConfig): OpenClawConfig { ...cfg.agents, defaults: { ...defaults, - memorySearch: { - experimental: { sessionMemory: true }, - sources: ["memory", "sessions"], - query: { - hybrid: { - enabled: true, - vectorWeight: 0.7, - textWeight: 0.3, - temporalDecay: { - enabled: true, - halfLifeDays: 30, - }, - mmr: { - enabled: true, - lambda: 0.7, - }, - }, - }, - }, + memorySearch: next, }, }, };