fix: manual-review batch — admin authz parity, cascade order, DB-time advance, fire body timeout

- access.js (#13): resolveAgentFireAccess no longer bypasses AGENTS:USE for admins,
  matching the chat route's checkAccess (which doesn't) — an admin with AGENTS:USE
  disabled is now rejected up front instead of every fire 403ing toward auto-disable.
- schedule.ts (#1): deleteSchedulesByUser deletes runs BEFORE schedules, both
  user-scoped and idempotent, so a partial failure is retryable (was orphaning runs).
- engine.ts (#9): the misfire and exception-branch computeNextRunAt now use the DB
  claim time (leaseUntil - LEASE_MS), so a clock-behind worker can't reschedule to a
  DB-past occurrence and reclaim the same row.
- fire.ts (#6): the loopback POST timeout now covers the response-body reads (not just
  headers) so a server that stalls the body can't hang the tick indefinitely.
This commit is contained in:
Danny Avila 2026-07-22 04:50:23 -04:00
parent 80b9197f23
commit 859987379d
4 changed files with 39 additions and 13 deletions

View file

@ -1,6 +1,5 @@
const mongoose = require('mongoose');
const {
SystemRoles,
ResourceType,
Permissions,
PermissionBits,
@ -29,11 +28,13 @@ async function resolveAgentFireAccess(agentId, user) {
if (agent == null) {
return 'missing';
}
if (user.role !== SystemRoles.ADMIN) {
const role = await getRoleByName(user.role);
if (!role?.permissions?.[PermissionTypes.AGENTS]?.[Permissions.USE]) {
return 'forbidden';
}
// 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 {

View file

@ -202,6 +202,9 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine {
cadence: schedule.cadence,
timezone: schedule.timezone,
scheduleId: schedule.id,
// DB claim time, not this worker's clock: a clock-behind worker would
// otherwise compute another DB-past occurrence and reclaim the same row.
after: new Date(dbNow),
});
if (next == null) {
await deps.methods
@ -230,6 +233,9 @@ export function startScheduleEngine(deps: ScheduleEngineDeps): ScheduleEngine {
cadence: schedule.cadence,
timezone: schedule.timezone,
scheduleId: schedule.id,
// DB claim time (see the misfire branch) so a skewed worker doesn't
// reschedule to a DB-past occurrence and reclaim the same row.
after: new Date(dbNow),
});
if (next == null) {
await deps.methods

View file

@ -37,6 +37,27 @@ async function postChatMessage(
): Promise<{ conversationId: string }> {
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), FIRE_REQUEST_TIMEOUT_MS);
// The timeout must cover the BODY reads below, not just the headers: a server
// that sends headers then stalls the body would otherwise hang this tick forever
// (the abort signal is passed to fetch, so firing it aborts an in-flight read).
try {
return await postChatMessageInner(deps, schedule, userId, scheduledFor, files, conversationId, {
controller,
});
} finally {
clearTimeout(timeout);
}
}
async function postChatMessageInner(
deps: ScheduleEngineDeps,
schedule: FireableSchedule,
userId: string,
scheduledFor: Date,
files: Awaited<ReturnType<ScheduleEngineDeps['resolveFiles']>>,
conversationId: string,
{ controller }: { controller: AbortController },
): Promise<{ conversationId: string }> {
let response: Response;
try {
response = await fetch(`${deps.getSelfUrl()}/api/agents/chat/${EModelEndpoint.agents}`, {
@ -80,8 +101,6 @@ async function postChatMessage(
// been processed — ambiguous, so don't terminalize as a definite error.
const message = error instanceof Error ? error.message : String(error);
throw new ScheduleFireError(`Fire POST network failure: ${message}`, true);
} finally {
clearTimeout(timeout);
}
if (!response.ok) {
const body = await response.text().catch(() => '');

View file

@ -529,12 +529,12 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
/** Cascade for account deletion: removes a user's schedules and their runs. */
async function deleteSchedulesByUser(userId: string | Types.ObjectId): Promise<void> {
const schedules = await Schedule().find({ user: userId }).select('id').lean<{ id: string }[]>();
const ids = schedules.map((s) => s.id);
// Delete RUNS before SCHEDULES so a partial failure is retryable: both are
// user-scoped and idempotent, so a crash after the runs delete leaves the
// schedules for a retry to re-delete (deleting schedules first would orphan the
// runs — a re-run finds no schedules and never removes the leftover run rows).
await ScheduleRun().deleteMany({ user: userId });
await Schedule().deleteMany({ user: userId });
if (ids.length > 0) {
await ScheduleRun().deleteMany({ scheduleId: { $in: ids } });
}
}
async function transitionRunStatus(