mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
feat: real global schedules kill switch (base-only + env), gated at the engine tick
`interface.schedules` was doing two unrelated jobs: per-principal availability and an operational kill switch. Rather than make one cached, per-principal, overridable key non-overridable and uncached (which would break its symmetry with every other interface flag and still be eventual across replicas), this splits the READ authority: - BASE config (librechat.yaml) read with `baseOnly: true` is the GLOBAL stop. Base-only means no role/user/tenant DB override can widen past it, so the guarantee is explicit rather than emergent from three interacting override filters. - The principal-merged value stays per-principal availability: an override can narrow (disable for a principal, tune limits) but never re-enable past the global stop. - SCHEDULES_DISABLED env is the incident lever, checked FIRST and without touching config, so it still works when the DB/config plane is the thing failing. Enforced at the engine tick (stop claiming entirely) rather than only in the fire path, so a disabled deployment stops scheduling instead of claiming and then skipping at the last step. Schedule writes were already gated via getLimits (handlers.ts), and base `false` already flows there, so the write side needed no change. reconcile() is deliberately NOT gated: settling in-flight runs must continue while the feature is off, or `started` rows strand and leak capacity forever. Documented as intentional so it doesn't read as a missing check. Tests cover: default off; env lever trips without reading config (proved by a throwing getAppConfig); base-only `false` trips even when the principal-merged view re-enables; a principal-only disable does NOT trip the global stop.
This commit is contained in:
parent
c7f3f8983b
commit
52c0b98268
4 changed files with 88 additions and 7 deletions
|
|
@ -211,12 +211,20 @@ 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).
|
||||
// GLOBAL kill switch: stop claiming entirely. This is the operator's hard stop
|
||||
// (SCHEDULES_DISABLED, or `interface.schedules: false` in the BASE config), which
|
||||
// no principal override can widen — distinct from per-principal availability below.
|
||||
// Deliberately gates CLAIMS only; reconcile() is never gated, because in-flight
|
||||
// runs must still be settled while the feature is off or they strand `started`
|
||||
// rows and leak capacity forever.
|
||||
if (await deps.isGloballyDisabled()) {
|
||||
return 0;
|
||||
}
|
||||
// Do NOT gate claims on the per-principal `enabled`: schedules can be enabled per
|
||||
// user/role/tenant, 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.
|
||||
const limits = await deps.getLimits();
|
||||
let fired = 0;
|
||||
// Cap on ACTIVE scheduled runs, not just per-tick starts: the loopback chat
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ type ActiveRun = { scheduleId: string; scheduledFor: Date; conversationId?: stri
|
|||
|
||||
function makeService(
|
||||
getActiveRunsForUser: jest.Mock<Promise<ActiveRun[]>, [string]>,
|
||||
getAppConfig?: SchedulesServiceDeps['getAppConfig'],
|
||||
): ReturnType<typeof createSchedulesService> {
|
||||
const methods = {
|
||||
disableUserSchedulesForDeletion: jest.fn(async () => undefined),
|
||||
|
|
@ -24,7 +25,7 @@ function makeService(
|
|||
};
|
||||
const deps = {
|
||||
methods,
|
||||
getAppConfig: jest.fn(async () => ({})),
|
||||
getAppConfig: getAppConfig ?? jest.fn(async () => ({})),
|
||||
findUserById: jest.fn(async () => null),
|
||||
findBalance: jest.fn(async () => null),
|
||||
upsertBalance: jest.fn(async () => null),
|
||||
|
|
@ -91,3 +92,53 @@ describe('quiesceUserSchedules drain wait', () => {
|
|||
expect(getActive).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('global kill switch', () => {
|
||||
const noRuns = () => jest.fn<Promise<ActiveRun[]>, [string]>().mockResolvedValue([]);
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.SCHEDULES_DISABLED;
|
||||
});
|
||||
|
||||
it('is off by default', async () => {
|
||||
const service = makeService(noRuns());
|
||||
expect(await service.engineDeps.isGloballyDisabled()).toBe(false);
|
||||
});
|
||||
|
||||
it('trips on the SCHEDULES_DISABLED env lever without reading config', async () => {
|
||||
process.env.SCHEDULES_DISABLED = 'true';
|
||||
// Throwing getAppConfig proves the env lever works even when the config plane is
|
||||
// unhealthy — the case where a config-dependent kill switch would fail.
|
||||
const getAppConfig = jest.fn(async () => {
|
||||
throw new Error('config plane down');
|
||||
}) as unknown as SchedulesServiceDeps['getAppConfig'];
|
||||
const service = makeService(noRuns(), getAppConfig);
|
||||
expect(await service.engineDeps.isGloballyDisabled()).toBe(true);
|
||||
expect(getAppConfig).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('trips on `interface.schedules: false` read from the BASE config only', async () => {
|
||||
const getAppConfig = jest.fn(async (options?: { baseOnly?: boolean }) =>
|
||||
options?.baseOnly === true
|
||||
? { interfaceConfig: { schedules: false } }
|
||||
: // A principal-merged view that re-enables must NOT be consulted: the global
|
||||
// stop is base-only so no role/user/tenant override can widen past it.
|
||||
{ interfaceConfig: { schedules: true } },
|
||||
) as unknown as SchedulesServiceDeps['getAppConfig'];
|
||||
const service = makeService(noRuns(), getAppConfig);
|
||||
expect(await service.engineDeps.isGloballyDisabled()).toBe(true);
|
||||
expect(getAppConfig).toHaveBeenCalledWith({ baseOnly: true });
|
||||
});
|
||||
|
||||
it('does not trip when only a principal-merged config disables it', async () => {
|
||||
// Per-principal availability is NOT the global stop; the engine keeps claiming so
|
||||
// other principals still fire, and the fire path skips this owner's occurrences.
|
||||
const getAppConfig = jest.fn(async (options?: { baseOnly?: boolean }) =>
|
||||
options?.baseOnly === true
|
||||
? { interfaceConfig: { schedules: true } }
|
||||
: { interfaceConfig: { schedules: false } },
|
||||
) as unknown as SchedulesServiceDeps['getAppConfig'];
|
||||
const service = makeService(noRuns(), getAppConfig);
|
||||
expect(await service.engineDeps.isGloballyDisabled()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import { DEFAULT_SCHEDULE_LIMITS } from './types';
|
|||
import { getBalanceConfig } from '../app/config';
|
||||
import { startScheduleEngine } from './engine';
|
||||
import { withCapacitySlot } from './capacity';
|
||||
import { isEnabled } from '../utils/common';
|
||||
|
||||
/** Recordable terminal/paused run outcome, as accepted by `recordRunOutcome`. */
|
||||
type ScheduleRunOutcomeStatus = Parameters<ScheduleMethods['recordRunOutcome']>[0]['status'];
|
||||
|
|
@ -369,6 +370,19 @@ export function createSchedulesService(deps: SchedulesServiceDeps): SchedulesSer
|
|||
// Counted in system scope so the cap is GLOBAL — a per-owner (tenant-scoped)
|
||||
// count would let multiple tenants collectively exceed fireConcurrency.
|
||||
countActiveRunsGlobal: () => runAsSystem(() => methods.countActiveRuns()),
|
||||
isGloballyDisabled: async () => {
|
||||
// Env first: an incident lever that must work even if the DB/config plane is the
|
||||
// thing failing (a kill switch that needs a healthy DB is the one that fails when
|
||||
// you need it).
|
||||
if (isEnabled(process.env.SCHEDULES_DISABLED)) {
|
||||
return true;
|
||||
}
|
||||
// BASE config only: DB principal overrides can narrow availability but must never
|
||||
// widen past an operator's global stop, so `schedules: false` in librechat.yaml is
|
||||
// genuinely non-overridable rather than emergent from the override filters.
|
||||
const base = await deps.getAppConfig({ baseOnly: true });
|
||||
return base?.interfaceConfig?.schedules === false;
|
||||
},
|
||||
// Occupancy is read in SYSTEM scope so the cap is global across tenants (the
|
||||
// owner's tenant context would only see its own runs); the claim itself stays in
|
||||
// the caller's context so the inserted row keeps correct tenant ownership.
|
||||
|
|
|
|||
|
|
@ -97,6 +97,14 @@ export interface ScheduleEngineDeps {
|
|||
clearReconciledJob: (conversationId: string, identity: JobIdentity) => Promise<void>;
|
||||
/** Global in-flight scheduled-run count (system tenant scope) for the fire cap. */
|
||||
countActiveRunsGlobal: () => Promise<number>;
|
||||
/**
|
||||
* The GLOBAL kill switch, deliberately distinct from per-principal availability.
|
||||
* True when scheduling is stopped for the whole deployment: the SCHEDULES_DISABLED
|
||||
* env lever (works even when the config plane is unhealthy), or `interface.schedules:
|
||||
* false` in the BASE config — read base-only so no role/user/tenant override can
|
||||
* re-enable it. Checked once per engine tick, so the uncached read is negligible.
|
||||
*/
|
||||
isGloballyDisabled: () => Promise<boolean>;
|
||||
/**
|
||||
* Runs `claim` against the lowest free GLOBAL capacity slot, retrying the next slot
|
||||
* when the unique partial index rejects a collision. Enforces fireConcurrency in the
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue