From 7b317a53b2a11ba2b264a84f90248424836369b0 Mon Sep 17 00:00:00 2001 From: medking82 Date: Wed, 11 Nov 2026 08:51:35 +0800 Subject: [PATCH] fix(combos): preserve explicit persisted cooldown clears Keep queued key tombstones through subsequent sets and apply combo-prefix clears to the fresh file under the mutation lock. Add regressions for shorter replacements, peer rows, retry retention, and clear ordering. Follow-up to lidge-jun/opencodex#8117. --- src/combos/cooldown-store.ts | 30 +++++- src/combos/failover.ts | 2 +- structure/fallback-policy.md | 2 +- .../combo-cooldown-persistence.test.ts | 91 ++++++++++++++++++- 4 files changed, 117 insertions(+), 8 deletions(-) diff --git a/src/combos/cooldown-store.ts b/src/combos/cooldown-store.ts index 36e54f6bf41..2ed23eb18ab 100644 --- a/src/combos/cooldown-store.ts +++ b/src/combos/cooldown-store.ts @@ -15,8 +15,10 @@ * - `dropLocal()` (and the Map `delete()`/`clear()` it backs) forgets a row in memory only. Expiry * checks, sweeps and the revision-guarded probe lift use it, so an expired local copy can never * erase a row another process has since extended. - * - `clearPersisted()` / `clearAllPersisted()` are the only deletes written to disk: they record - * user-requested clears (combo save, rename, delete). Clearing everything is a global reset. + * - Explicit key, prefix and global clears are the only deletes written to disk: they record + * user-requested clears (combo save, rename, delete). Prefix clears also remove peer rows absent + * from memory. Deletes exclude old disk rows before dirty values are overlaid, so a subsequent + * `set()` starts a new cooldown even when the cleared row had a later expiry. * A busy lock or I/O failure keeps the pending state and retries with bounded backoff. * * Loading: once per config dir, on first use. Rows already in memory win; a file row loads only @@ -73,6 +75,7 @@ export class PersistedCooldownMap extends Map(); private readonly tombstones = new Set(); + private readonly clearedPrefixes = new Set(); private clearAll = false; private readonly nextRevision: () => number; @@ -103,7 +106,7 @@ export class PersistedCooldownMap extends Map 0 || this.tombstones.size > 0; + return this.clearAll || this.dirty.size > 0 || this.tombstones.size > 0 || this.clearedPrefixes.size > 0; } private schedule(): void { @@ -123,9 +126,11 @@ export class PersistedCooldownMap extends Map = {}; + const clearedPrefixes = [...this.clearedPrefixes]; if (!this.clearAll) { for (const [key, row] of Object.entries(file)) { - if (row.cooldownUntil > now && !this.tombstones.has(key)) out[key] = row; + if (row.cooldownUntil > now && !this.tombstones.has(key) + && !clearedPrefixes.some(prefix => key.startsWith(prefix))) out[key] = row; } } for (const key of this.dirty) { @@ -147,6 +152,7 @@ export class PersistedCooldownMap extends Map extends Map extends Map `${prefix}${key}`)); return; } - targetCooldowns.clearPersisted([...targetCooldowns.keys()].filter(key => key.startsWith(prefix))); + targetCooldowns.clearPrefixPersisted(prefix); } export type ComboFailureDecision = "hop" | "stop"; diff --git a/structure/fallback-policy.md b/structure/fallback-policy.md index 0f9fe0c00a5..dd32e9b4202 100644 --- a/structure/fallback-policy.md +++ b/structure/fallback-policy.md @@ -135,7 +135,7 @@ privacy), `tests/responses/responses-combo-fallback-off.test.ts`, `tests/routing `tests/server/fallback-routes.test.ts`, `tests/clients/client-home-unreachable.test.ts`, `tests/cli/cli-fallback.test.ts` and `gui/tests/fallback-panel.test.tsx`. -Combo target cooldowns persist across restarts: `src/combos/cooldown-store.ts` backs the map in `src/combos/failover.ts` with `/combo-cooldowns.json`, storing absolute expiry epochs (expired or corrupt rows dropped on load, loaded rows get fresh revisions). Writes are debounced, re-read the file under the config mutation lock and overlay only keys this process set, keeping the later expiry; expiry checks, sweeps and the probe lift are memory-only drops, and only `clearComboTargetCooldowns` writes deletes (clearing all is a global reset). A busy lock retries with bounded backoff and shutdown flushes with bounded attempts. Coverage: `tests/routing/combo-cooldown-persistence.test.ts`. +Combo target cooldowns persist across restarts: `src/combos/cooldown-store.ts` backs the map in `src/combos/failover.ts` with `/combo-cooldowns.json`, storing absolute expiry epochs (expired or corrupt rows dropped on load, loaded rows get fresh revisions). Writes are debounced, re-read the file under the config mutation lock and overlay only keys this process set, keeping the later expiry; expiry checks, sweeps and the probe lift are memory-only drops, and only `clearComboTargetCooldowns` writes deletes (clearing all is a global reset). A combo-wide clear records its exact null-delimited prefix, removing matching rows from the fresh locked read even when a peer added them after this process loaded. Explicit key and prefix clears exclude old disk rows before dirty values are overlaid, so clearing then re-cooling the same target keeps the new cooldown rather than resurrecting the old hold. Pending clears survive failed writes and are forgotten only after a successful flush. A busy lock retries with bounded backoff and shutdown flushes with bounded attempts. Coverage: `tests/routing/combo-cooldown-persistence.test.ts`. The opt-in [all-cooled Combo recovery](runtime.md#scoped-provider-quota-for-combo-selection) can reserve saved combo cooldowns in `src/combos/resolve.ts`; only that request bypasses its reserved writes until a successful dispatch conditionally clears its unchanged target, with throwing predicates treated as ineligible during recovery preparation and available siblings preferred without probing cooled targets. Route-level picks and in-flight retry-after-cooldown selections disable reprobes. Recovery does not authorize a fallback hop or bypass the configured fallback policy. diff --git a/tests/routing/combo-cooldown-persistence.test.ts b/tests/routing/combo-cooldown-persistence.test.ts index 6d4f56d29f0..c7525bf4ba1 100644 --- a/tests/routing/combo-cooldown-persistence.test.ts +++ b/tests/routing/combo-cooldown-persistence.test.ts @@ -115,7 +115,7 @@ test("user clears are written: keys, one combo, and everything as a global reset expect(a.flush()).toBe(true); expect(Object.keys(stored()).sort()).toEqual(["c1\0p/b", "c2\0p/a", "foreign"]); - a.clearPersisted([...a.keys()].filter(key => key.startsWith("c2\0"))); + a.clearPrefixPersisted("c2\0"); expect(a.flush()).toBe(true); expect(Object.keys(stored()).sort()).toEqual(["c1\0p/b", "foreign"]); @@ -160,3 +160,92 @@ test("a failed load is retried on a later access", () => { writeFileSync(file(), JSON.stringify({ row: { cooldownUntil: now + 60_000 } })); expect(a.has("row")).toBe(true); }); + +for (const scope of ["key", "combo"] as const) { + test(`a queued ${scope} clear survives a shorter cooldown for the same target`, () => { + const now = Date.now(); + const key = "cleared\0p/a"; + writeFileSync(file(), JSON.stringify({ [key]: { cooldownUntil: now + 86_400_000, status: 503 } })); + const a = peer(); + if (scope === "key") a.clearPersisted([key]); else a.clearPrefixPersisted("cleared\0"); + a.set(key, { cooldownUntil: now + 60_000, revision: nextRevision(), status: 429 }); + a.flushForShutdown(); + expect(stored()[key]).toEqual({ cooldownUntil: now + 60_000, status: 429 }); + expect(peer().get(key)?.cooldownUntil).toBe(now + 60_000); + }); +} + +test("a combo-wide public clear removes peer rows absent from memory, including a later write", async () => { + const combo = "peer-clear"; + const target = { provider: "provider-a", model: "model-x" }; + // Load the production map before the peer writes anything for this combo. + expect(isComboTargetInCooldown(combo, target)).toBe(false); + const b = peer(); + const until = Date.now() + 60_000; + b.set(`${combo}\0provider-a/model-x`, { cooldownUntil: until, revision: nextRevision() }); + b.set(`${combo}-other\0provider-a/model-x`, { cooldownUntil: until, revision: nextRevision() }); + b.flushForShutdown(); + expect(isComboTargetInCooldown(combo, target)).toBe(false); + + clearComboTargetCooldowns(combo); + // This row arrives after the clear was queued, but before its locked flush. + b.set(`${combo}\0provider-b/model-y`, { cooldownUntil: until, revision: nextRevision() }); + b.flushForShutdown(); + await wait(500); + expect(Object.keys(stored())).toEqual([`${combo}-other\0provider-a/model-x`]); + expect(peer().has(`${combo}\0provider-a/model-x`)).toBe(false); +}); + +test("a combo-prefix clear and its new cooldown survive a busy lock", () => { + const now = Date.now(); + const key = "retry-clear\0p/a"; + const a = peer(); + expect(a.size).toBe(0); + a.clearPrefixPersisted("retry-clear\0"); + writeFileSync(file(), JSON.stringify({ + [key]: { cooldownUntil: now + 86_400_000, status: 503 }, + "retry-clear\0p/b": { cooldownUntil: now + 86_400_000 }, + "other\0p/a": { cooldownUntil: now + 86_400_000 }, + })); + a.set(key, { cooldownUntil: now + 60_000, revision: nextRevision(), status: 429 }); + const lock = new Database(join(home, "config-mutation.sqlite"), { create: true }); + lock.exec("BEGIN IMMEDIATE"); + try { + expect(a.flush()).toBe(false); + } finally { + lock.exec("ROLLBACK"); + lock.close(); + } + a.flushForShutdown(); + expect(stored()).toEqual({ + [key]: { cooldownUntil: now + 60_000, status: 429 }, + "other\0p/a": { cooldownUntil: now + 86_400_000 }, + }); +}); + +test("a successful prefix clear does not delete future peer rows on a later write", () => { + const until = Date.now() + 60_000; + writeFileSync(file(), JSON.stringify({ "cleared\0p/a": { cooldownUntil: until } })); + const a = peer(); + a.clearPrefixPersisted("cleared\0"); + a.flushForShutdown(); + expect(stored()).toEqual({}); + const b = peer(); + b.set("cleared\0p/b", { cooldownUntil: until, revision: nextRevision() }); + b.flushForShutdown(); + a.set("other\0p/a", { cooldownUntil: until, revision: nextRevision() }); + a.flushForShutdown(); + expect(Object.keys(stored()).sort()).toEqual(["cleared\0p/b", "other\0p/a"]); +}); + +test("repeated prefix clears discard intervening sets but preserve the last replacement", () => { + const now = Date.now(); + writeFileSync(file(), JSON.stringify({ "cleared\0p/a": { cooldownUntil: now + 86_400_000 } })); + const a = peer(); + a.clearPrefixPersisted("cleared\0"); + a.set("cleared\0p/b", { cooldownUntil: now + 60_000, revision: nextRevision() }); + a.clearPrefixPersisted("cleared\0"); + a.set("cleared\0p/a", { cooldownUntil: now + 30_000, revision: nextRevision() }); + a.flushForShutdown(); + expect(stored()).toEqual({ "cleared\0p/a": { cooldownUntil: now + 30_000 } }); +});