diff --git a/api/server/services/Schedules/access.js b/api/server/services/Schedules/access.js index 07ad8ecaf5..a278b4e339 100644 --- a/api/server/services/Schedules/access.js +++ b/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 }; diff --git a/client/src/data-provider/Schedules/mutations.ts b/client/src/data-provider/Schedules/mutations.ts index cd613ea6d2..ea1d461cb8 100644 --- a/client/src/data-provider/Schedules/mutations.ts +++ b/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); }, }, ); diff --git a/client/src/hooks/Nav/useSideNavLinks.ts b/client/src/hooks/Nav/useSideNavLinks.ts index 8495b8fe6b..2208a7d2d3 100644 --- a/client/src/hooks/Nav/useSideNavLinks.ts +++ b/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: '', diff --git a/packages/api/src/schedules/access.ts b/packages/api/src/schedules/access.ts new file mode 100644 index 0000000000..83cb793124 --- /dev/null +++ b/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> } | null>; + /** Whether the user's role grants a system capability (the manage:agents bypass). */ + hasCapability: (user: ScheduleUserContext, capability: SystemCapability) => Promise; + /** Resource ACL check for a specific permission bit. */ + checkPermission: (params: { + userId: string; + role?: string; + resourceType: ResourceType; + resourceId: Types.ObjectId; + requiredPermission: PermissionBits; + }) => Promise; +} + +/** + * 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 { + 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'; + }; +} diff --git a/packages/api/src/schedules/engine.ts b/packages/api/src/schedules/engine.ts index ce340ebc89..1e3926b16b 100644 --- a/packages/api/src/schedules/engine.ts +++ b/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 { + // 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 diff --git a/packages/api/src/schedules/handlers.ts b/packages/api/src/schedules/handlers.ts index eb2d77ce17..4dfb430b7f 100644 --- a/packages/api/src/schedules/handlers.ts +++ b/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) - .catch(() => undefined); - res.status(500).json({ error: 'Failed to retain schedule attachments' }); - return; - } res.json(toWireSchedule(schedule)); } diff --git a/packages/api/src/schedules/index.ts b/packages/api/src/schedules/index.ts index 70907349f0..d13bd9d629 100644 --- a/packages/api/src/schedules/index.ts +++ b/packages/api/src/schedules/index.ts @@ -1,3 +1,4 @@ +export * from './access'; export * from './cadence'; export * from './engine'; export * from './fire'; diff --git a/packages/api/src/schedules/service.ts b/packages/api/src/schedules/service.ts index a992fa73a0..d863e7aa06 100644 --- a/packages/api/src/schedules/service.ts +++ b/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 { 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'; }