fix(providers): don't silently enable rate-limit protection on PATCH unless persisted (#11278)

PUT /api/providers/[id] unconditionally called enableRateLimitProtection(id)
whenever the request body included rateLimitOverrides, even null. Since
EditConnectionModal.tsx sends rateLimitOverrides on every connection save
regardless of whether the operator touched that section, saving any
connection silently started queuing its requests through Bottleneck while
the DB (rate_limit_protection column) and the dashboard toggle both still
showed the feature off.

Only (re)enable the in-memory limiter when updated.rateLimitProtection is
actually true (mapped from the persisted DB row), and explicitly disable it
otherwise so runtime state can never drift ahead of the DB.

.github/workflows/opencode-plugin-ci.yml

@@ -2,11 +2,11 @@ name: opencode-plugin CI
 
 on:
   push:
-    branches: [main, "release/**"]
+    branches: [main, release/v3.8.2]
     paths:
       - "@omniroute/opencode-plugin/**"
   pull_request:
-    branches: [main, "release/**"]
+    branches: [main, release/v3.8.2]
     paths:
       - "@omniroute/opencode-plugin/**"
     types: [opened, synchronize, reopened, ready_for_review]

@omniroute/opencode-plugin/tests/provider.test.ts

@@ -104,10 +104,7 @@ test("models: extracts apiKey from ctx.auth (type=api) and calls fetcher with it
   // #6859: dynamic-hook catalog keys use the unprefixed omnirouteProviderId
   // ("omniroute"), not the OC-gate-prefixed hook.id ("opencode-omniroute") —
   // that prefix must never leak into anything OmniRoute's server parses.
-  // #10345/#10821: bare combo ids (owned_by: "combo") stay unprefixed —
-  // OpenCode looks up `-m <plugin>/<combo>` as model id `<combo>` under the
-  // plugin provider, so `claude-primary` here carries no provider prefix.
-  assert.ok(out["claude-primary"]);
+  assert.ok(out["omniroute/claude-primary"]);
 });
 
 test("models: returns {} when ctx.auth is null/undefined/wrong-type/empty-key", async () => {
@@ -162,15 +159,11 @@ test("models: maps a sample /v1/models entry to ModelV2 (sanity)", async () => {
   // omnirouteProviderId ("omniroute") — the OC-gate prefix ("opencode-")
   // must stay OC-internal (hook.id / AuthHook.provider) and never leak into
   // anything OmniRoute's own server parses for credential lookup.
-  // #10345/#10821: bare **combo** ids (owned_by: "combo", e.g.
-  // "claude-primary") must also stay unprefixed — OpenCode looks up
-  // `-m <plugin>/<combo>` as model id `<combo>` under the plugin provider.
-  const claude = out["claude-primary"];
+  const claude = out["omniroute/claude-primary"];
   assert.ok(claude, "claude-primary present");
-  // `mapRawModelToModelV2` leaves bare combo ids unprefixed (see
-  // src/index.ts mapRawModelToModelV2) so OC's `-m <plugin>/<combo>` lookup
-  // resolves the combo id directly.
-  assert.equal(claude.id, "claude-primary");
+  // `mapRawModelToModelV2` stamps the provider prefix on the id so OC's
+  // static-catalog reader resolves `(providerID, modelID)` from the key.
+  assert.equal(claude.id, "omniroute/claude-primary");
   assert.equal(claude.name, "claude-primary");
   assert.equal(claude.providerID, "omniroute");
   assert.equal(claude.api.id, "openai-compatible");

src/app/api/providers/[id]/route.ts

@@ -29,6 +29,7 @@ import { canUpdateProviderApiKey } from "@/shared/providers/webSessionCredential
 import {
   refreshConnectionRateLimits,
   enableRateLimitProtection,
+  disableRateLimitProtection,
 } from "@/../open-sse/services/rateLimitManager";
 import {
   finalizeValidatedChatGptWebCodexSecrets,
@@ -342,10 +343,18 @@ export async function PUT(request: Request, { params }: { params: Promise<{ id:
 
     // If rateLimitOverrides was included in the request, refresh the in-memory
     // rate limiter state so the change takes effect without a server restart.
-    // Also ensure rate limit protection is active so the limiter is enforced.
+    // Only (re)enable enforcement when rate limit protection is actually
+    // persisted for this connection — this route never lets a caller flip
+    // `rateLimitProtection` itself, so any drift here would silently start
+    // queuing requests through Bottleneck for a connection whose DB row (and
+    // the dashboard toggle reading it) both still say "off" (#11278).
     if (rateLimitOverrides !== undefined) {
       refreshConnectionRateLimits(id, updated?.rateLimitOverrides ?? null);
-      enableRateLimitProtection(id);
+      if (updated?.rateLimitProtection === true) {
+        enableRateLimitProtection(id);
+      } else {
+        disableRateLimitProtection(id);
+      }
     }
 
     // Hide sensitive fields

tests/unit/provider-patch-ratelimit-protection-11278.test.ts

@@ -0,0 +1,139 @@
+// Regression guard for #11278 — PATCH/PUT /api/providers/[id] silently enabled
+// runtime rate-limit protection (Bottleneck queuing) for ANY connection whose
+// request body included the `rateLimitOverrides` key, even `null`, regardless
+// of whether `rate_limit_protection` was actually persisted as on for that
+// connection in the DB.
+//
+// Root cause: src/app/api/providers/[id]/route.ts unconditionally called
+// enableRateLimitProtection(id) whenever `rateLimitOverrides !== undefined`
+// in the validated body. `EditConnectionModal.tsx` sends `rateLimitOverrides`
+// on every save regardless of whether the operator touched that section, so
+// saving ANY connection silently started queuing its requests through
+// Bottleneck — with the DB (`rate_limit_protection` column) and the dashboard
+// toggle both still showing the feature as off.
+//
+// Fix: only (re)enable the in-memory limiter when the persisted connection
+// (`updated.rateLimitProtection`, mapped from the DB row) is actually `true`;
+// otherwise explicitly disable it so runtime state can't drift ahead of the
+// DB. `rateLimitProtection` is never itself part of updateProviderConnectionSchema,
+// so this route can only read it from the persisted row — never set it.
+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";
+import { makeManagementSessionRequest } from "../helpers/managementSession.ts";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-11278-ratelimit-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.APP_LOG_TO_FILE = "false";
+process.env.JWT_SECRET = "test-jwt-secret-11278-ratelimit";
+process.env.INITIAL_PASSWORD = "admin-secret";
+
+const core = await import("../../src/lib/db/core.ts");
+const { createProviderConnection, getProviderConnectionById } =
+  await import("../../src/lib/db/providers.ts");
+const providerByIdRoute = await import("../../src/app/api/providers/[id]/route.ts");
+const rateLimitManager = await import("../../open-sse/services/rateLimitManager.ts");
+
+function resetDb() {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(() => {
+  resetDb();
+});
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+async function createConnection(rateLimitProtection: boolean) {
+  return createProviderConnection({
+    provider: "openai",
+    authType: "apikey",
+    name: "OpenAI key",
+    apiKey: "sk-test-key-value",
+    priority: 1,
+    isActive: true,
+    testStatus: "active",
+    rateLimitProtection,
+  });
+}
+
+test(
+  "PUT /api/providers/[id] does NOT enable rate-limit protection just because " +
+    "rateLimitOverrides is present, when protection is off in the DB (#11278 RED->GREEN)",
+  async () => {
+    const connection = (await createConnection(false)) as Record<string, unknown>;
+    assert.equal(connection.rateLimitProtection, false);
+    assert.equal(rateLimitManager.isRateLimitEnabled(connection.id as string), false);
+
+    // Mirrors EditConnectionModal.tsx's handleSubmit(): it always sends
+    // `rateLimitOverrides` on every save, even when the operator never
+    // touched that section of the form.
+    const payload = {
+      name: connection.name,
+      priority: connection.priority,
+      rateLimitOverrides: null,
+    };
+
+    const request = await makeManagementSessionRequest(
+      `http://localhost/api/providers/${connection.id}`,
+      { method: "PUT", body: payload }
+    );
+    const response = await providerByIdRoute.PUT(request, {
+      params: Promise.resolve({ id: connection.id as string }),
+    });
+    assert.equal(response.status, 200, `expected the save to succeed, got ${response.status}`);
+
+    const persisted = (await getProviderConnectionById(connection.id as string)) as Record<
+      string,
+      unknown
+    >;
+    assert.equal(
+      persisted.rateLimitProtection,
+      false,
+      "DB row must still show protection off — this route never sets rateLimitProtection"
+    );
+    assert.equal(
+      rateLimitManager.isRateLimitEnabled(connection.id as string),
+      false,
+      "in-memory limiter must not silently diverge from the persisted DB state"
+    );
+  }
+);
+
+test(
+  "PUT /api/providers/[id] keeps rate-limit protection ENABLED when it is " +
+    "actually persisted as on in the DB",
+  async () => {
+    const connection = (await createConnection(true)) as Record<string, unknown>;
+    assert.equal(connection.rateLimitProtection, true);
+
+    const payload = {
+      name: connection.name,
+      priority: connection.priority,
+      rateLimitOverrides: { rpm: 30 },
+    };
+
+    const request = await makeManagementSessionRequest(
+      `http://localhost/api/providers/${connection.id}`,
+      { method: "PUT", body: payload }
+    );
+    const response = await providerByIdRoute.PUT(request, {
+      params: Promise.resolve({ id: connection.id as string }),
+    });
+    assert.equal(response.status, 200, `expected the save to succeed, got ${response.status}`);
+
+    const persisted = (await getProviderConnectionById(connection.id as string)) as Record<
+      string,
+      unknown
+    >;
+    assert.equal(persisted.rateLimitProtection, true);
+    assert.equal(rateLimitManager.isRateLimitEnabled(connection.id as string), true);
+  }
+);