From 616d54cf1969ba8024c1f57348ad275be0b48dda Mon Sep 17 00:00:00 2001 From: Paco Cartones <253313177+pacocartones@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:41:37 +0200 Subject: [PATCH] fix(config): persist background-degradation entry deletions (#12647) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tell is convincing: `detectionPatterns` in the same function already treats a present stored value as authoritative, so the two halves of one object disagreed. Making `degradationMap` stored-authoritative-when-present is the smaller change and the consistent one. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded with the rest of this batch — zero conflicts between the 19 PRs. - `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, all within the frozen baseline); `check:changelog-integrity` OK - complexity 2802 / baseline 3218 and cognitive-complexity 1267 / baseline 1437 — both under baseline - 226 of 228 focused assertions green across the batch's 23 test files. The 2 remaining belong to #12551, which is held separately. Two batch-owned defects were found and fixed in flight, both pure base drift: `173_xp_action_counts.sql` collided with `173_call_logs_video_content_removed.sql` (renumbered to 176 on #12651 — it aborted every DB open, which is what 53 of the first run's failures were), and the feature-flag catalog was missing the `SERVER_OWNED_TOOL_LOOP_ENABLED` row the base gained after #12552 was written. ⚠️ base-red inherited: #12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` reproduce on the pure tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098, which this batch does not touch). Thanks @pacocartones — the `file:line` citations and the explicit out-of-scope notes on every one of these made a 19-PR batch reviewable in one pass. --- .../12647-background-degradation-deletions.md | 1 + src/lib/config/runtimeSettings.ts | 9 +-- ...ground-degradation-deletions-12424.test.ts | 57 +++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 changelog.d/fixes/12647-background-degradation-deletions.md create mode 100644 tests/unit/settings/background-degradation-deletions-12424.test.ts diff --git a/changelog.d/fixes/12647-background-degradation-deletions.md b/changelog.d/fixes/12647-background-degradation-deletions.md new file mode 100644 index 0000000000..5339d07594 --- /dev/null +++ b/changelog.d/fixes/12647-background-degradation-deletions.md @@ -0,0 +1 @@ +- **fix(config):** Persist deletions of built-in background-degradation entries — when a stored settings record exists its `degradationMap` is now authoritative instead of being merged under the defaults, so an entry the user removed in the dashboard no longer reappears on the next apply or restart ([#12424](https://github.com/diegosouzapw/OmniRoute/issues/12424)) diff --git a/src/lib/config/runtimeSettings.ts b/src/lib/config/runtimeSettings.ts index 0cb93c8fee..ed5930cbb7 100644 --- a/src/lib/config/runtimeSettings.ts +++ b/src/lib/config/runtimeSettings.ts @@ -323,10 +323,11 @@ async function applyBackgroundDegradationSection(backgroundDegradation: JsonReco setBackgroundDegradationConfig({ enabled: backgroundDegradation.enabled === true, - degradationMap: { - ...getDefaultDegradationMap(), - ...normalizeStringRecord(backgroundDegradation.degradationMap), - }, + // #12424: a present stored record is authoritative for degradationMap — do NOT back-fill + // defaults, or a key the user deleted (absent from the stored map) resurrects on every + // apply/restart. Mirrors detectionPatterns below, which already treats a present stored + // value as authoritative and only falls back to defaults when it is empty. + degradationMap: normalizeStringRecord(backgroundDegradation.degradationMap), detectionPatterns: normalizeStringArray(backgroundDegradation.detectionPatterns).length > 0 ? normalizeStringArray(backgroundDegradation.detectionPatterns) diff --git a/tests/unit/settings/background-degradation-deletions-12424.test.ts b/tests/unit/settings/background-degradation-deletions-12424.test.ts new file mode 100644 index 0000000000..f5b157a723 --- /dev/null +++ b/tests/unit/settings/background-degradation-deletions-12424.test.ts @@ -0,0 +1,57 @@ +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"; + +process.env.DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-bgdeg-12424-")); + +const { applyRuntimeSettings, resetRuntimeSettingsStateForTests } = await import( + "../../../src/lib/config/runtimeSettings.ts" +); +const { + getBackgroundDegradationConfig, + getDefaultDegradationMap, + getDefaultDetectionPatterns, + setBackgroundDegradationConfig, +} = await import("../../../open-sse/services/backgroundTaskDetector.ts"); + +// Issue #12424: deleting a built-in background-degradation entry through the dashboard +// did not persist — the runtime loader merged defaults *under* the stored map, so a key +// the user removed (absent from the stored record) was indistinguishable from one never +// touched and always came back on the next apply/restart. +test("stored degradationMap that omits a default key does not resurrect it (#12424)", async () => { + resetRuntimeSettingsStateForTests(); + setBackgroundDegradationConfig({ + enabled: false, + degradationMap: getDefaultDegradationMap(), + detectionPatterns: getDefaultDetectionPatterns(), + }); + + const defaults = getDefaultDegradationMap(); + const deletedKey = "gpt-5"; + const keptKey = "gpt-4o"; + assert.ok( + defaults[deletedKey] && defaults[keptKey], + "fixture assumes these default keys exist in DEFAULT_DEGRADATION_MAP" + ); + + // The stored map is every default except the one the user deleted. + const stored: Record = { ...defaults }; + delete stored[deletedKey]; + + await applyRuntimeSettings( + { backgroundDegradation: JSON.stringify({ enabled: true, degradationMap: stored }) }, + { force: true, source: "test" } + ); + + const applied = getBackgroundDegradationConfig().degradationMap; + + // The entries the user kept still apply… + assert.equal(applied[keptKey], defaults[keptKey], "a kept default entry still applies"); + // …and the one they deleted stays deleted instead of being back-filled from defaults. + assert.ok( + !(deletedKey in applied), + `deleted default '${deletedKey}' must not be re-added from defaults` + ); +});