fix(combos): send null to clear an agent feature instead of omitting it (#12177)

* fix(combos): send null to clear an agent feature instead of omitting it

PUT /api/combos/[id] merges its body over the stored record, so an omitted
field means "leave unchanged". The combos editor deleted a cleared agent
feature from the payload, so unchecking context cache protection -- or
emptying the system message or the tool filter -- never persisted: the old
value survived the merge and the editor reopened with the toggle still on.

updateCombo already deletes any key explicitly set to null, which is how
description and context_length are cleared in the same save handler. Use the
same shape for the three agent fields, and make them nullable in
updateComboSchema so the null survives validation.

The clearing logic moves into comboAgentFeatures.ts so it can be tested
directly, matching comboQuotaOnlyFallback.ts next to it.

Fixes #12158

* chore(changelog): point the fragment at the real PR number

changelog.d/fixes/12177-combo-clear-agent-features.md

@@ -0,0 +1,4 @@
+- **fix(combos):** clearing an agent feature in the combos editor now persists — unchecking
+  context cache protection, or emptying the system message or tool filter, sends an explicit
+  `null` instead of dropping the field from the `PUT` body, which the update merge read as
+  "leave unchanged" ([#12177](https://github.com/diegosouzapw/OmniRoute/pull/12177)) — thanks @foreveryh

src/app/(dashboard)/dashboard/combos/comboAgentFeatures.ts

@@ -0,0 +1,46 @@
+/**
+ * Agent-features clearing for the combos editor (#399 / #401 / #454, fixed in #12158).
+ *
+ * `PUT /api/combos/[id]` merges its body over the stored record, so an omitted field
+ * means "leave unchanged". Deleting a cleared field from the payload therefore left the
+ * previous value in the database: unchecking context cache protection, or emptying the
+ * system message or tool filter, never persisted. Only an explicit `null` reaches
+ * `updateCombo`'s null-means-delete pass.
+ *
+ * On create there is nothing to clear, so an empty field is simply absent — the same
+ * shape `description` and `context_length` already use in this editor.
+ */
+export interface AgentFeatureInput {
+  systemMessage: string;
+  toolFilter: string;
+  contextCache: boolean;
+  isEdit: boolean;
+}
+
+export interface AgentFeaturePatch {
+  system_message?: string | null;
+  tool_filter_regex?: string | null;
+  context_cache_protection?: true | null;
+}
+
+export function buildAgentFeaturePatch({
+  systemMessage,
+  toolFilter,
+  contextCache,
+  isEdit,
+}: AgentFeatureInput): AgentFeaturePatch {
+  const patch: AgentFeaturePatch = {};
+
+  const message = systemMessage.trim();
+  if (message) patch.system_message = message;
+  else if (isEdit) patch.system_message = null;
+
+  const filter = toolFilter.trim();
+  if (filter) patch.tool_filter_regex = filter;
+  else if (isEdit) patch.tool_filter_regex = null;
+
+  if (contextCache) patch.context_cache_protection = true;
+  else if (isEdit) patch.context_cache_protection = null;
+
+  return patch;
+}

src/app/(dashboard)/dashboard/combos/page.tsx

@@ -18,6 +18,7 @@ import { useCopyToClipboard } from "@/shared/hooks/useCopyToClipboard";
 import { FieldLabelWithHelp, WeightTotalBar } from "./parts";
 import { ComboTargetOptions } from "./ComboQuotaOnlyFallbackToggle";
 import { applyQuotaOnlyFallbackConfig, setQuotaOnlyFallback } from "./comboQuotaOnlyFallback";
+import { buildAgentFeaturePatch } from "./comboAgentFeatures";
 import { useComboProxyAssignments } from "./useComboProxyAssignments";
 import { ResponseValidationEditor, type ResponseValidationValue } from "./ResponseValidationEditor";
 import ReasoningTokenBufferToggle from "./ReasoningTokenBufferToggle";
@@ -3011,13 +3012,20 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo
       saveData.config = configToSave;
     }
 
-    // Agent features (#399 / #401 / #454)
-    if (agentSystemMessage.trim()) saveData.system_message = agentSystemMessage.trim();
-    else delete saveData.system_message;
-    if (agentToolFilter.trim()) saveData.tool_filter_regex = agentToolFilter.trim();
-    else delete saveData.tool_filter_regex;
-    if (agentContextCache) saveData.context_cache_protection = true;
-    else delete saveData.context_cache_protection;
+    // Agent features (#399 / #401 / #454). A cleared field is sent as null on edit
+    // rather than omitted, because PUT merges over the stored record (#12158).
+    delete saveData.system_message;
+    delete saveData.tool_filter_regex;
+    delete saveData.context_cache_protection;
+    Object.assign(
+      saveData,
+      buildAgentFeaturePatch({
+        systemMessage: agentSystemMessage,
+        toolFilter: agentToolFilter,
+        contextCache: agentContextCache,
+        isEdit,
+      })
+    );
 
     // Validate and save context_length
     if (contextLength !== undefined && contextLength !== null) {

src/shared/validation/schemas/combo.ts

@@ -418,9 +418,12 @@ export const updateComboSchema = z
     isActive: z.boolean().optional(),
     allowedProviders: z.array(z.string().trim().min(1).max(200)).max(100).optional(),
     allowedModelFamilies: z.array(z.string().trim().min(1).max(100)).max(100).optional(),
-    system_message: z.string().max(50000).optional(),
-    tool_filter_regex: z.string().max(1000).optional(),
-    context_cache_protection: z.boolean().optional(),
+    // Nullable like `description` and `context_length` above: an absent field means
+    // "leave unchanged" because updateCombo merges over the stored record, so clearing
+    // one needs an explicit null for updateCombo's null-means-delete pass (#12158).
+    system_message: z.string().max(50000).optional().nullable(),
+    tool_filter_regex: z.string().max(1000).optional().nullable(),
+    context_cache_protection: z.boolean().optional().nullable(),
     context_length: z.number().int().min(1000).max(2000000).optional().nullable(),
     compressionOverride: comboCompressionOverrideSchema.optional(),
     dimensions: z

tests/unit/combo-clear-agent-features-12158.test.ts

@@ -0,0 +1,162 @@
+/**
+ * #12158 — clearing an "Agent features" field on a combo must persist.
+ *
+ * `updateCombo` merges the PUT body over the stored record, so an absent field
+ * means "leave unchanged" and only an explicit `null` deletes it. `description`
+ * and `context_length` were already nullable in `updateComboSchema`; the three
+ * agent fields were not, so unchecking `context_cache_protection` (or clearing
+ * `system_message` / `tool_filter_regex`) left the old value in place.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-combo-clear-12158-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { updateComboSchema } = await import("../../src/shared/validation/schemas.ts");
+const core = await import("../../src/lib/db/core.ts");
+const combosDb = await import("../../src/lib/db/combos.ts");
+
+async function resetStorage() {
+  core.resetDbInstance();
+  if (fs.existsSync(TEST_DATA_DIR)) {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  }
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(async () => {
+  await resetStorage();
+});
+
+test.after(async () => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+test("updateComboSchema accepts null for each agent feature field", () => {
+  const parsed = updateComboSchema.parse({
+    system_message: null,
+    tool_filter_regex: null,
+    context_cache_protection: null,
+  });
+  assert.equal(parsed.system_message, null);
+  assert.equal(parsed.tool_filter_regex, null);
+  assert.equal(parsed.context_cache_protection, null);
+});
+
+test("a null agent field still counts as a field to update", () => {
+  assert.doesNotThrow(() => updateComboSchema.parse({ context_cache_protection: null }));
+  assert.throws(() => updateComboSchema.parse({}), /No valid fields to update/);
+});
+
+test("a set agent feature value is still accepted and still rejects a bad type", () => {
+  const parsed = updateComboSchema.parse({
+    system_message: "be terse",
+    tool_filter_regex: "^read_",
+    context_cache_protection: true,
+  });
+  assert.equal(parsed.system_message, "be terse");
+  assert.equal(parsed.tool_filter_regex, "^read_");
+  assert.equal(parsed.context_cache_protection, true);
+  assert.throws(() => updateComboSchema.parse({ context_cache_protection: "yes" }));
+});
+
+test("null clears each agent feature through updateCombo", async () => {
+  const created = await combosDb.createCombo({
+    name: "Agent Features Combo",
+    models: [{ provider: "openai", model: "gpt-4.1" }],
+    system_message: "be terse",
+    tool_filter_regex: "^read_",
+    context_cache_protection: true,
+  });
+  assert.equal(created.system_message, "be terse");
+  assert.equal(created.tool_filter_regex, "^read_");
+  assert.equal(created.context_cache_protection, true);
+
+  const cleared = await combosDb.updateCombo(created.id as string, {
+    system_message: null,
+    tool_filter_regex: null,
+    context_cache_protection: null,
+  });
+  assert.ok(cleared);
+  assert.equal(cleared!.system_message, undefined);
+  assert.equal(cleared!.tool_filter_regex, undefined);
+  assert.notEqual(cleared!.context_cache_protection, true);
+
+  // Re-read: this is what the editor reopens with, and what #12158 reported as
+  // still showing the toggle checked.
+  const reread = await combosDb.getComboById(created.id as string);
+  assert.ok(reread);
+  assert.equal(reread!.system_message, undefined);
+  assert.equal(reread!.tool_filter_regex, undefined);
+  assert.notEqual(reread!.context_cache_protection, true);
+});
+
+test("omitting an agent feature still leaves it unchanged", async () => {
+  const created = await combosDb.createCombo({
+    name: "Untouched Combo",
+    models: [{ provider: "openai", model: "gpt-4.1" }],
+    system_message: "be terse",
+    context_cache_protection: true,
+  });
+
+  const updated = await combosDb.updateCombo(created.id as string, { description: "note" });
+  assert.ok(updated);
+  assert.equal(updated!.system_message, "be terse");
+  assert.equal(updated!.context_cache_protection, true);
+});
+
+const { buildAgentFeaturePatch } =
+  await import("../../src/app/(dashboard)/dashboard/combos/comboAgentFeatures.ts");
+
+test("the editor sends null for every cleared agent feature on edit", () => {
+  assert.deepEqual(
+    buildAgentFeaturePatch({
+      systemMessage: "  ",
+      toolFilter: "",
+      contextCache: false,
+      isEdit: true,
+    }),
+    { system_message: null, tool_filter_regex: null, context_cache_protection: null }
+  );
+});
+
+test("the editor omits an empty agent feature on create", () => {
+  assert.deepEqual(
+    buildAgentFeaturePatch({
+      systemMessage: "",
+      toolFilter: "",
+      contextCache: false,
+      isEdit: false,
+    }),
+    {}
+  );
+});
+
+test("the editor still sends set agent features, trimmed", () => {
+  assert.deepEqual(
+    buildAgentFeaturePatch({
+      systemMessage: "  be terse  ",
+      toolFilter: " ^read_ ",
+      contextCache: true,
+      isEdit: true,
+    }),
+    { system_message: "be terse", tool_filter_regex: "^read_", context_cache_protection: true }
+  );
+});
+
+test("clearing one agent feature does not disturb the others", () => {
+  assert.deepEqual(
+    buildAgentFeaturePatch({
+      systemMessage: "keep me",
+      toolFilter: "",
+      contextCache: true,
+      isEdit: true,
+    }),
+    { system_message: "keep me", tool_filter_regex: null, context_cache_protection: true }
+  );
+});