fix: release the concurrency slot Run Now actually holds

Making Run Now subject to the concurrent-request limiter changed the ACQUIRE and
one of the three release points. The HITL pause path releases the slot early (so
a fast /resume is not 429'd) and then RETURNS, bypassing the teardown that was
made manual-aware — so a Run Now that paused for approval took a slot and never
gave it back. Under LIMIT_CONCURRENT_MESSAGES two of those pin the user at the
cap, 429ing their ordinary messages and the very approvals that would settle the
runs, and the counter's TTL re-arms on every attempt so it only clears after a
full idle minute.

Fixed by single-sourcing the rule rather than patching the two sites. The acquire
and both releases now call `exemptFromConcurrencyLimiter`, which lives beside the
IP and user limiter predicates it mirrors. Re-deriving the condition at each site
is exactly what let them drift, and the same shape has produced several of this
series' regressions.

The predicate has real coverage in limiters.spec.ts. Adding it also surfaced a
hand-enumerated `@librechat/api` mock in request.resumeMetadata.spec.js that
silently lacked the new export; it now pulls the real function through
requireActual, so a stub cannot hide the next drift.

Found by auditing this session's own changes for reversals rather than waiting
for review. Also corrects two comments that overclaimed: the pre-drain engine
stop NARROWS the shutdown window rather than closing it, and an unarmed schedule
left by a failed compensation is refused at the deletion barrier rather than
being unreachable.

api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js

@@ -107,6 +107,9 @@ jest.mock('@librechat/api', () => ({
   buildMessageFiles: jest.fn(() => []),
   resolveTitleTiming: jest.fn(() => 'immediate'),
   resolveConversationAnchor: jest.requireActual('@librechat/api').resolveConversationAnchor,
+  // Real predicate, not a stub: it decides whether this request holds a concurrency
+  // slot, and a stub here would hide a drift between the acquire and release sites.
+  exemptFromConcurrencyLimiter: jest.requireActual('@librechat/api').exemptFromConcurrencyLimiter,
   GenerationJobManager: mockGenerationJobManager,
   getReferencedQuotes: jest.fn((quotes) => {
     if (!Array.isArray(quotes)) {

api/server/controllers/agents/client.js

@@ -76,6 +76,7 @@ const {
   resolveYouTubeInjectionConfig,
   decrementPendingRequest,
   maybePrewarmCodeSandbox,
+  exemptFromConcurrencyLimiter,
 } = require('@librechat/api');
 const {
   Callback,
@@ -1525,10 +1526,15 @@ class AgentClient extends BaseClient {
     // teardown (request.js pause branch / resume.js finally) that would otherwise
     // release it, and `/resume` 429s under LIMIT_CONCURRENT_MESSAGES. Idempotent via
     // the flag; if it fails here, the teardown still releases (it checks the flag).
-    // A scheduled fire never acquired an interactive concurrency slot, so it must
-    // not release one on pause (that would clear a real user's counter). Mark it
+    // An AUTOMATIC scheduled fire never acquired an interactive concurrency slot, so it
+    // must not release one on pause (that would clear a real user's counter). Mark it
     // released so downstream teardown skips the decrement too.
-    if (this.options.req?._isScheduledFire) {
+    //
+    // Run Now DOES acquire one — it is user-paced and no longer exempt — so it must
+    // release like any interactive turn. Mirrors `exemptFromConcurrency` at the
+    // increment site; treating it as exempt here leaked a slot on every paused manual
+    // fire, and the pause path returns before the teardown that was made manual-aware.
+    if (exemptFromConcurrencyLimiter(this.options.req)) {
       this.pendingRequestReleased = true;
     } else if (!this.pendingRequestReleased) {
       try {

api/server/controllers/agents/request.js

@@ -12,6 +12,7 @@ const {
   decrementPendingRequest,
   sanitizeMessageForTransmit,
   checkAndIncrementPendingRequest,
+  exemptFromConcurrencyLimiter,
   isUnpersistedPreliminaryParent,
   isScheduleFireRequest,
   resolveConversationAnchor,
@@ -164,7 +165,7 @@ async function finishResumableRequest(req, userId) {
     // Mirrors the increment: only an AUTOMATIC fire skipped the counter, so only it
     // must skip the decrement. Uses the decision captured at request start, not a
     // re-check of the expiring token.
-    if (!req._isScheduledFire || req._isManualScheduledFire === true) {
+    if (!exemptFromConcurrencyLimiter(req)) {
       await decrementPendingRequest(userId);
     }
   }
@@ -376,7 +377,9 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit
   // so exempting it lets one user run unbounded concurrent generations — and unlike an
   // automatic occurrence, a 429 here is surfaced to the clicking user rather than
   // booked against the schedule (see the throttled branch in fireSchedule).
-  const exemptFromConcurrency = isScheduledFire && req._isManualScheduledFire !== true;
+  // Shared predicate, not a hand-rolled copy: the acquire and both release points
+  // live in different files, and re-deriving it at each is what let them drift.
+  const exemptFromConcurrency = exemptFromConcurrencyLimiter(req);
   if (!exemptFromConcurrency) {
     const { allowed, pendingRequests, limit } = await checkAndIncrementPendingRequest(userId);
     if (!allowed) {
@@ -920,7 +923,9 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit
           // (so a fast /resume isn't 429'd); only release here if that didn't happen.
           // Always run the MCP request-context cleanup.
           await cleanupMCPRequestContextForReq(req);
-          if (!isScheduledFire && !client?.pendingRequestReleased) {
+          // Mirrors the increment: only an AUTOMATIC fire skipped the counter. A Run Now
+          // holds a real slot, so it must release here like an interactive turn.
+          if (!exemptFromConcurrency && !client?.pendingRequestReleased) {
             await decrementPendingRequest(userId);
           }
           if (client) {

packages/api/src/crypto/jwt.ts

@@ -108,3 +108,18 @@ export const exemptFromUserLimiter = (req: ClassifiedFireRequest): boolean => {
   const claims = classify(req);
   return claims.scheduled && !claims.manual;
 };
+
+/**
+ * Whether the interactive CONCURRENT-request limiter should be SKIPPED — and, by the
+ * same token, whether this request must NOT release a slot on teardown.
+ *
+ * Same rule as the user limiter: only AUTOMATIC occurrences are exempt. Run Now is
+ * user-paced and holds a real slot, so it increments and must decrement.
+ *
+ * Exported deliberately rather than re-derived at each site. The acquire and the two
+ * release points (the HITL pause path and the normal teardown) live in different files,
+ * and hand-writing the predicate at each let them drift: the acquire became manual-aware
+ * while a release did not, leaking a slot on every paused Run Now.
+ */
+export const exemptFromConcurrencyLimiter = (req: ClassifiedFireRequest): boolean =>
+  exemptFromUserLimiter(req);

packages/api/src/crypto/limiters.spec.ts

@@ -2,6 +2,7 @@ import {
   exemptFromIpLimiter,
   SCHEDULE_FIRE_SCOPE,
   exemptFromUserLimiter,
+  exemptFromConcurrencyLimiter,
   SCHEDULE_MANUAL_CLAIM,
   readScheduleFireClaims,
   generateShortLivedToken,
@@ -72,6 +73,26 @@ describe('message-limiter exemptions for scheduled fires', () => {
     });
   });
 
+  /**
+   * The ACQUIRE and both RELEASE points live in different files (request.js twice,
+   * client.js once on the HITL pause path). Re-deriving this rule at each site is what
+   * let them drift: the acquire became manual-aware while a release did not, so every
+   * paused Run Now leaked a slot until the counter's idle TTL expired.
+   */
+  describe('concurrency limiter', () => {
+    it('exempts an automatic occurrence, which never increments', () => {
+      expect(exemptFromConcurrencyLimiter(request('automatic'))).toBe(true);
+    });
+
+    it('does NOT exempt a Run Now, which holds a real slot and must release it', () => {
+      expect(exemptFromConcurrencyLimiter(request('manual'))).toBe(false);
+    });
+
+    it('does not exempt an ordinary interactive turn', () => {
+      expect(exemptFromConcurrencyLimiter(request('interactive'))).toBe(false);
+    });
+  });
+
   it('limits rather than exempts a request whose token cannot be verified', () => {
     const req = { headers: { 'x-lc-scheduled': '1', authorization: 'Bearer not-a-jwt' } };
     expect(exemptFromIpLimiter(req)).toBe(false);

packages/api/src/schedules/engine.ts

@@ -411,8 +411,10 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine {
   // listener is closing and the generation manager has already begun refusing new jobs.
   // A tick in that window claims a due occurrence, fails its loopback POST against a
   // server that is shutting down, and books the failure against the schedule — walking a
-  // healthy schedule toward auto-disable for nothing more than a restart. Stopping first
-  // means the last claim always has a live server to fire at.
+  // healthy schedule toward auto-disable for nothing more than a restart. This NARROWS
+  // that window rather than closing it: the listener starts closing before pre-drain
+  // runs, so a tick already in flight can still lose its POST. Occurrences skipped by
+  // stopping early are simply still due at restart, within the misfire grace.
   registerShutdownTask(
     'schedule engine',
     () => {

packages/api/src/schedules/handlers.ts

@@ -262,7 +262,9 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
     // Inserted WITHOUT nextRunAt regardless of `enabled`: the engine claims by
     // nextRunAt, so the row is inert until armed below. That is what makes the
     // barrier re-check durable — every failure mode leaves a row that cannot fire,
-    // rather than one that fires for an account already being erased.
+    // rather than one that fires for an account already being erased. The reconciler's
+    // unarmed sweep later arms anything left this way, so an inert row is a delay, not
+    // a permanent state.
     const created = await deps.methods.createScheduleWithSlot(
       {
         ...parsed.data,
@@ -285,9 +287,9 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
     // Re-checking AFTER the write is what makes the barrier authoritative.
     if (await deps.isUserDeleting(user.id)) {
       // Best-effort tidy-up of an unarmed row. Its failure is reported but no longer
-      // load-bearing: an unarmed schedule is never claimed, so the worst case is an
-      // inert row the deletion sweep can reap later, not a live one firing billed
-      // generations for an erased account.
+      // load-bearing for BILLING: the row is unarmed, and even once the reconciler's
+      // sweep arms it, the fire path refuses it at the account-deletion barrier
+      // (isOwnerDeleting). The residual is a retained row, not a billed generation.
       if (!(await compensateLateCreate(deps, id, user.id))) {
         res.status(500).json({ error: 'Failed to roll back schedule creation' });
         return;