fix: Codex round — resume rollback race, engine per-user gate, nav/run-now, retain-first, access TS

Address all 7 findings from Codex review of e281b2b0c:

- Resume rollback race (P2): the capacity reserve-then-verify rollback could flip a
  row a concurrent same-pause resume had taken over. Made capacity a READ-ONLY
  pre-check before the promote and promote WITHOUT rollback — no rollback, no race.
  Per-schedule overlap stays hard-enforced by the partial index; global cap at resume
  is an explicit best-effort soft cap (documented; the fire path is the hard cap).

- Engine base-config gate (P2): runTick no longer returns early on the BASE
  getLimits().enabled — schedules can be enabled per user/role/tenant even when base
  disables them. The fire path re-resolves the OWNER's limits and skips a disabled
  owner, so an owner-scoped disable is still honored; base only sets the claim budget.

- Retain attachments before committing edits (P2): the update handler now retains
  the new files BEFORE updateScheduleById, so a retention failure leaves the entire
  schedule unchanged instead of persisting prompt/cadence/agent/enabled edits.

- Move agent-fire access into TS (P2, CLAUDE.md): resolveAgentFireAccess logic moved
  to packages/api/src/schedules/access.ts (createResolveAgentFireAccess with DI);
  api/.../access.js is now thin wiring.

- Clustered-without-shared-streams (P2): warn at engine init that peer aborts
  (deletion/account-deletion quiescing) and cross-worker orphan recovery need
  USE_REDIS_STREAMS — an inherent limitation of that unsupported topology.

- Client: hide the schedules nav panel when disabled via the object form
  ({ use: false }), not just the boolean (P2); invalidate the schedules query on
  run-now SETTLED (not just success) so 409 paths that disable the schedule refresh
  the card (P3).

api/server/services/Schedules/access.js

@@ -1,57 +1,18 @@
 const mongoose = require('mongoose');
-const {
-  ResourceType,
-  Permissions,
-  PermissionBits,
-  PermissionTypes,
-} = require('librechat-data-provider');
-const { logger, ResourceCapabilityMap } = require('@librechat/data-schemas');
+const { createResolveAgentFireAccess } = require('@librechat/api');
 const { checkPermission } = require('~/server/services/PermissionService');
 const { hasCapability } = require('~/server/middleware/roles/capabilities');
 const { getRoleByName } = require('~/models');
 
-/**
- * Resolves a user's live access to a schedule's target agent, mirroring the
- * loopback chat route's authorization EXACTLY so the create/update precheck and
- * the fire-time precheck accept a schedule iff the actual fire would be accepted:
- *   1) role-level AGENTS:USE (generateCheckAccess on the route; admins bypass)
- *   2) resource VIEW with the manage:agents capability bypass (canAccessResource)
- * The two prechecks must never diverge — a create-time VIEW-only check would let a
- * role without AGENTS:USE schedule runs that every fire then rejects and counts
- * toward auto-disable.
- * @param {string} agentId
- * @param {{ id: string, role?: string }} user
- * @returns {Promise<'ok' | 'missing' | 'forbidden'>}
- */
-async function resolveAgentFireAccess(agentId, user) {
-  const agent = await mongoose.models.Agent.findOne({ id: agentId }).select('_id').lean();
-  if (agent == null) {
-    return 'missing';
-  }
-  // Mirror the chat route's checkAgentAccess (generateCheckAccess → checkAccess),
-  // which does NOT special-case admins: it reads the role's AGENTS:USE permission
-  // directly. An admin whose role has AGENTS:USE disabled is rejected there, so the
-  // precheck must reject too — otherwise every fire 403s and burns failures.
-  const role = await getRoleByName(user.role);
-  if (!role?.permissions?.[PermissionTypes.AGENTS]?.[Permissions.USE]) {
-    return 'forbidden';
-  }
-  const cap = ResourceCapabilityMap[ResourceType.AGENT];
-  try {
-    if (cap != null && (await hasCapability(user, cap))) {
-      return 'ok';
-    }
-  } catch (err) {
-    logger.warn(`[schedules] agent capability check failed, denying bypass: ${err.message}`);
-  }
-  const allowed = await checkPermission({
-    userId: user.id,
-    role: user.role,
-    resourceType: ResourceType.AGENT,
-    resourceId: agent._id,
-    requiredPermission: PermissionBits.VIEW,
-  });
-  return allowed ? 'ok' : 'forbidden';
-}
+// Thin wiring over the TypeScript implementation in packages/api: inject the
+// api-layer lookups (agent id, role, capability, resource ACL); the authorization
+// logic itself lives in @librechat/api so it stays type-checked and in-boundary.
+const resolveAgentFireAccess = createResolveAgentFireAccess({
+  findAgentObjectId: (agentId) =>
+    mongoose.models.Agent.findOne({ id: agentId }).select('_id').lean(),
+  getRoleByName,
+  hasCapability,
+  checkPermission,
+});
 
 module.exports = { resolveAgentFireAccess };

client/src/data-provider/Schedules/mutations.ts

@@ -70,9 +70,14 @@ export const useRunScheduleNowMutation = (
     (id: string) => dataService.runScheduleNow(id),
     {
       ...options,
-      onSuccess: (...args) => {
+      // Invalidate on SETTLED, not just success: several run-now 409 paths are still
+      // server-side mutations (a balance skip updates lastRun/counters and can
+      // auto-disable; agent/permission/invalid-schedule skips disable the schedule
+      // before returning), so the card must refresh on those errors too rather than
+      // wait for the polling interval.
+      onSettled: (...args) => {
         queryClient.invalidateQueries([QueryKeys.schedules]);
-        options?.onSuccess?.(...args);
+        options?.onSettled?.(...args);
       },
     },
   );

client/src/hooks/Nav/useSideNavLinks.ts

@@ -166,7 +166,19 @@ export default function useSideNavLinks({
       });
     }
 
-    if (hasAccessToSchedules && interfaceConfig.schedules !== false) {
+    // Hide the panel when schedules are disabled by EITHER the boolean form
+    // (`false`) or the runtime-config object form (`{ use: false, ... }`) — the
+    // server treats both as disabled, so the nav gate must match to avoid showing
+    // an entry whose create/run operations the backend rejects.
+    const schedulesConfig = interfaceConfig.schedules;
+    const schedulesEnabled =
+      schedulesConfig !== false &&
+      !(
+        typeof schedulesConfig === 'object' &&
+        schedulesConfig != null &&
+        schedulesConfig.use === false
+      );
+    if (hasAccessToSchedules && schedulesEnabled) {
       links.push({
         title: 'com_ui_schedules',
         label: '',

packages/api/src/schedules/access.ts

@@ -0,0 +1,77 @@
+import { logger, ResourceCapabilityMap } from '@librechat/data-schemas';
+import {
+  ResourceType,
+  Permissions,
+  PermissionBits,
+  PermissionTypes,
+} from 'librechat-data-provider';
+import type { SystemCapability } from '@librechat/data-schemas';
+import type { Types } from 'mongoose';
+import type { ScheduleUserContext } from './types';
+
+type AgentAccess = 'ok' | 'missing' | 'forbidden';
+
+export interface AgentFireAccessDeps {
+  /** Resolves an agent's internal `_id` by its custom id, or null when it doesn't exist. */
+  findAgentObjectId: (agentId: string) => Promise<{ _id: Types.ObjectId } | null>;
+  /** Loads a role's permission map by name. */
+  getRoleByName: (
+    role?: string,
+  ) => Promise<{ permissions?: Record<string, Record<string, boolean | undefined>> } | null>;
+  /** Whether the user's role grants a system capability (the manage:agents bypass). */
+  hasCapability: (user: ScheduleUserContext, capability: SystemCapability) => Promise<boolean>;
+  /** Resource ACL check for a specific permission bit. */
+  checkPermission: (params: {
+    userId: string;
+    role?: string;
+    resourceType: ResourceType;
+    resourceId: Types.ObjectId;
+    requiredPermission: PermissionBits;
+  }) => Promise<boolean>;
+}
+
+/**
+ * Resolves a user's live access to a schedule's target agent, mirroring the loopback
+ * chat route's authorization EXACTLY so the create/update precheck and the fire-time
+ * precheck accept a schedule iff the actual fire would be accepted:
+ *   1) role-level AGENTS:USE (checkAccess on the route; admins do NOT bypass)
+ *   2) resource VIEW with the manage:agents capability bypass
+ * The two prechecks must never diverge — a create-time VIEW-only check would let a
+ * role without AGENTS:USE schedule runs that every fire then rejects, burning failures.
+ */
+export function createResolveAgentFireAccess(deps: AgentFireAccessDeps) {
+  return async function resolveAgentFireAccess(
+    agentId: string,
+    user: ScheduleUserContext,
+  ): Promise<AgentAccess> {
+    const agent = await deps.findAgentObjectId(agentId);
+    if (agent == null) {
+      return 'missing';
+    }
+    // Mirror the chat route's checkAccess, which reads the role's AGENTS:USE directly
+    // and does NOT special-case admins: an admin whose role has AGENTS:USE disabled is
+    // rejected there, so the precheck must reject too — otherwise every fire 403s.
+    const role = await deps.getRoleByName(user.role);
+    if (role?.permissions?.[PermissionTypes.AGENTS]?.[Permissions.USE] !== true) {
+      return 'forbidden';
+    }
+    const capability = ResourceCapabilityMap[ResourceType.AGENT];
+    try {
+      if (capability != null && (await deps.hasCapability(user, capability))) {
+        return 'ok';
+      }
+    } catch (err) {
+      logger.warn(
+        `[schedules] agent capability check failed, denying bypass: ${(err as Error).message}`,
+      );
+    }
+    const allowed = await deps.checkPermission({
+      userId: user.id,
+      role: user.role,
+      resourceType: ResourceType.AGENT,
+      resourceId: agent._id,
+      requiredPermission: PermissionBits.VIEW,
+    });
+    return allowed ? 'ok' : 'forbidden';
+  };
+}

packages/api/src/schedules/engine.ts

@@ -211,10 +211,13 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine {
    * owner's context via `runInTenantContext`.
    */
   async function runTick(): Promise<number> {
+    // Do NOT gate claims on the BASE config's `enabled`: schedules can be enabled
+    // per user/role/tenant even when the base config disables them, so gating here
+    // would silently never fire those users' occurrences. The fire path re-resolves
+    // the OWNER's limits and skips ('disabled') any occurrence whose owner has the
+    // feature off, so an owner-scoped disable is still honored. The base config only
+    // supplies the per-tick claim budget (a global throttle).
     const limits = await deps.getLimits();
-    if (!limits.enabled) {
-      return 0;
-    }
     let fired = 0;
     // Cap on ACTIVE scheduled runs, not just per-tick starts: the loopback chat
     // endpoint returns as soon as the generation starts and scheduled fires

packages/api/src/schedules/handlers.ts

@@ -251,22 +251,20 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
       update.balanceSkipCount = 0;
     }
     const unset = reEnabled ? { disabledReason: 1 as const } : undefined;
+    // Retain the new attachments BEFORE committing the edit, so a retention failure
+    // leaves the ENTIRE schedule unchanged rather than persisting prompt/cadence/
+    // agent/enabled changes while only reverting file_ids. A file whose TTL was
+    // cleared before the edit failed simply persists unreferenced (the user's own
+    // upload) — a minor leak, not a partial config change future runs would use.
+    if (parsed.data.file_ids?.length && !(await retainFiles(parsed.data.file_ids, user.id))) {
+      res.status(500).json({ error: 'Failed to retain schedule attachments' });
+      return;
+    }
     const schedule = await deps.methods.updateScheduleById(existing.id, user.id, update, unset);
     if (schedule == null) {
       res.status(404).json({ error: 'Schedule not found' });
       return;
     }
-    if (parsed.data.file_ids?.length && !(await retainFiles(parsed.data.file_ids, user.id))) {
-      // Revert to the prior attachments so the schedule doesn't reference files
-      // whose upload TTL wasn't cleared and would be reaped before the next fire.
-      await deps.methods
-        .updateScheduleById(existing.id, user.id, {
-          file_ids: existing.file_ids ?? [],
-        } as Partial<ISchedule>)
-        .catch(() => undefined);
-      res.status(500).json({ error: 'Failed to retain schedule attachments' });
-      return;
-    }
     res.json(toWireSchedule(schedule));
   }
 

packages/api/src/schedules/index.ts

@@ -1,3 +1,4 @@
+export * from './access';
 export * from './cadence';
 export * from './engine';
 export * from './fire';

packages/api/src/schedules/service.ts

@@ -329,6 +329,18 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
     if (options?.clustered != null) {
       clustered = options.clustered;
     }
+    // A clustered deployment without a SHARED stream backend cannot signal a peer
+    // worker's in-memory job (emitAbort is cross-replica only over Redis), so
+    // deletion quiescing can't abort a scheduled run whose generation lives on
+    // another worker, and orphan reaping is disabled. This is unsupported for
+    // scheduled chats — warn the operator to enable USE_REDIS_STREAMS.
+    if (clustered && !GenerationJobManager.isRedis) {
+      logger.warn(
+        '[schedules] clustered deployment without a shared stream store (USE_REDIS_STREAMS): ' +
+          'scheduled-run peer aborts (deletion/account-deletion quiescing) and cross-worker ' +
+          'orphan recovery are NOT available. Enable USE_REDIS_STREAMS for safe multi-worker scheduling.',
+      );
+    }
     // Explicitly build the Schedule/ScheduleRun indexes first — the unique
     // idempotency index and TTL retention index would otherwise never exist when
     // MONGO_AUTO_INDEX is disabled (the production default). If this fails the
@@ -422,15 +434,6 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
     return false;
   }
 
-  /**
-   * Read-only overlap/capacity pre-check for a HITL resume, run BEFORE the approval
-   * claim so a deferral leaves the approval claimable. A paused run is
-   * `requires_action`, so another `started` run for the schedule (hasActiveRun) is
-   * a DIFFERENT, active occurrence — resuming over it would break per-schedule
-   * overlap. Capacity is checked against the OWNER's fireConcurrency. No mutation:
-   * the actual promotion happens only after the claim is won, so the slot is owned
-   * by the run's driver, never by a losing racer.
-   */
   async function isScheduleLive(scheduleId: string): Promise<boolean> {
     if (!scheduleId) {
       return false;
@@ -438,6 +441,21 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
     return (await methods.getScheduleById(scheduleId)) != null;
   }
 
+  /**
+   * Reservation for a HITL resume, run BEFORE the approval claim so a deferral
+   * leaves the approval claimable. Checks existence/overlap/capacity READ-ONLY
+   * first (a null schedule -> 'gone'; another `started` occurrence -> 'overlap'; the
+   * owner's fireConcurrency saturated -> 'capacity'), then promotes the run into the
+   * single active slot WITHOUT any rollback. No rollback is the key correctness
+   * property: whichever request wins the approval drives whatever is `started`, so a
+   * losing racer can never flip the winner's active row back to requires_action.
+   * Per-schedule overlap is hard-enforced by the partial unique index; the global
+   * cap is a best-effort soft cap here (concurrent resumes of DIFFERENT schedules
+   * can transiently overshoot by the number racing, self-healing when they settle) —
+   * the fire path remains the hard-enforced cap for new load, and enforcing it
+   * atomically here would require either a rollback that races the claim takeover or
+   * a drift-prone global counter.
+   */
   async function reserveScheduledResume(
     scheduleId: string,
     scheduledFor: string | Date,
@@ -452,32 +470,29 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
     if (schedule == null) {
       return 'gone';
     }
-    const when = new Date(scheduledFor);
-    // Promote requires_action -> started. The single-active partial index makes
-    // per-schedule overlap atomic (a newer occurrence already active -> 'overlap').
-    // 'missing' means a concurrent same-pause resume already promoted this row: it
-    // is already active, so proceed WITHOUT rolling it back (that racer owns it) and
-    // without re-counting capacity (we did not add to the population).
-    const promoted = await methods.promoteRunToStarted(scheduleId, when);
-    if (promoted === 'overlap') {
+    // A paused run is `requires_action`, so another `started` run for the schedule is
+    // a DIFFERENT active occurrence — resuming over it would break per-schedule overlap.
+    if (await methods.hasActiveRun(scheduleId)) {
       return 'overlap';
     }
-    if (promoted === 'missing') {
-      return 'ok';
-    }
-    // We added this run to the started population — reserve-then-verify the global
-    // cap against the OWNER's limit, rolling back OUR OWN promotion if over. Doing
-    // this BEFORE the approval claim keeps the approval claimable on a capacity
-    // deferral, and rolling back only our own row avoids the claim-owner race.
+    // Read-only capacity gate BEFORE promoting, so we never mutate a row a concurrent
+    // same-pause resume may already be driving (no rollback path exists).
     const owner = await engineDeps.getUserContext(schedule.user);
     const limits = await getLimits(owner ?? undefined);
     const active = await engineDeps.countActiveRunsGlobal();
-    if (active > limits.fireConcurrency) {
-      await methods
-        .transitionRunStatus(scheduleId, when, 'started', 'requires_action')
-        .catch(() => undefined);
+    if (active >= limits.fireConcurrency) {
       return 'capacity';
     }
+    // Reserve the single active slot. Best-effort: 'overlap' (a different occurrence
+    // won the slot since the check above) leaves the row paused and the resume runs
+    // undercounted until it settles; 'missing' means a concurrent same-pause resume
+    // already promoted it. Never rolled back.
+    const promoted = await methods.promoteRunToStarted(scheduleId, new Date(scheduledFor));
+    if (promoted === 'overlap') {
+      logger.warn(
+        `[schedules] resumed run could not reserve the active slot (overlap): ${scheduleId}`,
+      );
+    }
     return 'ok';
   }