fix: Codex round 8 - runtime-only kill switch, pre-connect fires, manual lease holder, quiesce race

Addresses the four P2 findings of Codex review 4757872384:

1. permissions.ts: the BOOLEAN `interface.schedules` form is the runtime kill switch
   (read by getLimits), not a permission config. hasExplicitConfig now returns false
   for it and the SCHEDULES.USE seed ignores the boolean (only an object `use` drives
   the permission), so toggling the kill switch never writes SCHEDULES.USE into role
   docs and removing it later can't leave /api/schedules stuck forbidden.

2. fire.ts: a fetch that throws BEFORE connecting (bad SCHEDULES_SELF_URL: DNS,
   connection refused/unreachable, connect-timeout, or TLS handshake failure) is now
   classified a DEFINITE failure - nothing could have started - so the run terminalizes
   as `error` (countable toward auto-disable) instead of lingering as a reconcilable
   orphan later swept to `interrupted`. Mid-flight failures stay ambiguous.

3. schedule.ts: manual run-now leases now use a UNIQUE per-lease holder
   (`manual:<claimToken>`) instead of the constant `manual`. The superseded-fire
   cleanup releases by holder, so a stalled run-now can no longer match and strip the
   fresh lease a newer run-now acquired.

4. schedule.ts + fire.ts: account deletion can hard-delete a schedule that a worker
   already claimed but had not yet inserted a `started` run for. rollbackReservation
   now deletes the reserved row when the schedule no longer exists (new scheduleExists
   check), so a run row can't be left orphaned for a deleted user. A lease takeover
   (schedule still present) still preserves the row for the new holder.

Tests: boolean kill switch leaves the permission untouched; pre-connect fetch failure
-> error; unique manual holder + scheduleExists soft-vs-hard delete; hard-deleted
schedule mid-fire deletes the reservation. Full typecheck + schedules (45) / methods
(48) / permissions (40) / resolution suites green; both dists rebuilt.

packages/api/src/app/permissions.spec.ts

@@ -2936,4 +2936,36 @@ describe('updateInterfacePermissions - permissions', () => {
       expect(call[1][PermissionTypes.SCHEDULES]).toBeUndefined();
     }
   });
+
+  it('treats a boolean schedules kill switch as runtime-only (does not touch the permission)', async () => {
+    // The role currently has schedules enabled.
+    mockGetRoleByName.mockResolvedValue({
+      permissions: {
+        [PermissionTypes.SCHEDULES]: { [Permissions.USE]: true, [Permissions.CREATE]: true },
+      },
+    });
+    // `schedules: false` is the RUNTIME kill switch read by getLimits, NOT a permission
+    // config: it must not write SCHEDULES into the role docs, so removing it later can
+    // never leave USE stuck false (forbidden) until manual repair.
+    const config = {
+      interface: {
+        schedules: false,
+      },
+    };
+    const configDefaults = { interface: {} } as TConfigDefaults;
+    const interfaceConfig = await loadDefaultInterface({ config, configDefaults });
+    const appConfig = { config, interfaceConfig } as unknown as AppConfig;
+
+    await updateInterfacePermissions({
+      appConfig,
+      getRoleByName: mockGetRoleByName,
+      updateAccessPermissions: mockUpdateAccessPermissions,
+    });
+
+    // The kill switch is runtime-only: SCHEDULES is omitted from every role update,
+    // leaving the existing USE:true untouched.
+    for (const call of mockUpdateAccessPermissions.mock.calls) {
+      expect(call[1][PermissionTypes.SCHEDULES]).toBeUndefined();
+    }
+  });
 });

packages/api/src/app/permissions.ts

@@ -50,16 +50,17 @@ function hasExplicitConfig(
     case PermissionTypes.SHARED_LINKS:
       return interfaceConfig?.sharedLinks !== undefined;
     case PermissionTypes.SCHEDULES: {
-      // `schedules` is dual-purpose: a boolean, or an object carrying use/create,
-      // is a permission config — but runtime-only limits (maxPerUser,
-      // fireConcurrency, …) are NOT. Treating their mere presence as a permission
-      // config would re-apply the USE default below and silently re-enable a
-      // DB-disabled schedules permission whenever an operator tunes the limits.
+      // `schedules` is dual-purpose. The BOOLEAN form is the RUNTIME kill switch read
+      // by getLimits, NOT a permission config: treating it as explicit would write
+      // SCHEDULES.USE into the role docs, and removing the kill switch later would
+      // leave that disabled permission stuck (forbidden) until manual repair. Only an
+      // OBJECT carrying explicit use/create is permission intent; runtime-only limits
+      // (maxPerUser, fireConcurrency, …) are not.
       const schedules = interfaceConfig?.schedules;
-      if (typeof schedules === 'boolean') {
-        return true;
+      if (typeof schedules !== 'object' || schedules == null) {
+        return false;
       }
-      return schedules?.use !== undefined || schedules?.create !== undefined;
+      return schedules.use !== undefined || schedules.create !== undefined;
     }
     default:
       return false;
@@ -546,7 +547,12 @@ export async function updateInterfacePermissions({
       },
       [PermissionTypes.SCHEDULES]: {
         [Permissions.USE]: getPermissionValue(
-          getConfigUse(loadedInterface.schedules),
+          // Only an OBJECT `use` drives the permission; the boolean form is the runtime
+          // kill switch and must not seed SCHEDULES.USE (see hasExplicitConfig), so a
+          // removed kill switch can never leave USE stuck false.
+          typeof loadedInterface.schedules === 'object'
+            ? loadedInterface.schedules?.use
+            : undefined,
           defaultPerms[PermissionTypes.SCHEDULES]?.[Permissions.USE],
           schedulesDefaultUse,
         ),

packages/api/src/schedules/fire.spec.ts

@@ -90,6 +90,7 @@ function makeMethods() {
     ),
     revalidateClaim: jest.fn(async () => true),
     holdsLease: jest.fn(async () => true),
+    scheduleExists: jest.fn(async () => true),
     releaseLeaseByHolder: jest.fn(async () => undefined),
     deleteScheduleRun: jest.fn(async (id: string, when: Date, _status?: string) => {
       runs.delete(key(id, when));
@@ -302,6 +303,39 @@ describe('fireSchedule', () => {
     expect([...runs.entries()].some(([k]) => k.startsWith('sched-1:'))).toBe(true);
   });
 
+  it('deletes the reserved run when the schedule was hard-deleted mid-fire', async () => {
+    const { methods, runs } = makeMethods();
+    // At capacity, so this fire rolls back its reservation.
+    for (let i = 0; i < 5; i++) {
+      runs.set(`other-${i}:x`, { status: 'started' });
+    }
+    // Account deletion hard-deleted the schedule after this fire reserved its run:
+    // the lease is not held AND the schedule no longer exists.
+    (methods.holdsLease as jest.Mock).mockResolvedValue(false);
+    (methods.scheduleExists as jest.Mock).mockResolvedValue(false);
+    mockFetch(async () => okResponse());
+    const result = await fireSchedule(makeDeps(methods), makeSchedule(), LIMITS, dueAt());
+    expect(result.skipped).toBe('capacity');
+    // The orphaned reservation (no schedule left to own it) is deleted, not leaked.
+    expect(methods.deleteScheduleRun).toHaveBeenCalledWith('sched-1', expect.any(Date), 'started');
+    expect([...runs.entries()].some(([k]) => k.startsWith('sched-1:'))).toBe(false);
+  });
+
+  it('records a pre-connect fetch failure (bad self URL) as a definite error', async () => {
+    const { methods, runs, calls } = makeMethods();
+    // A DNS/connection failure before the request reaches the server: nothing started,
+    // so it must terminalize as `error` (countable) rather than stay reconcilable.
+    mockFetch(async () => {
+      const err = new TypeError('fetch failed');
+      (err as unknown as { cause: { code: string } }).cause = { code: 'ECONNREFUSED' };
+      throw err;
+    });
+    const result = await fireSchedule(makeDeps(methods), makeSchedule(), LIMITS, dueAt());
+    expect(result.fired).toBe(false);
+    expect(calls.recordOutcome).toEqual([{ status: 'error' }]);
+    expect([...runs.values()][0].status).toBe('error');
+  });
+
   it('skips overlap when a prior run is still active', async () => {
     const { methods, runs, calls } = makeMethods();
     runs.set('sched-1:prior', { status: 'started' });

packages/api/src/schedules/fire.ts

@@ -27,6 +27,43 @@ class ScheduleFireError extends Error {
   }
 }
 
+/** Node/undici error codes for failures that occur BEFORE any request byte is sent
+ *  (DNS, connection refused/unreachable, connect timeout). Nothing could have started,
+ *  so these are DEFINITE fire failures, not ambiguous mid-flight ones. */
+const PRE_CONNECT_ERROR_CODES = new Set([
+  'ECONNREFUSED',
+  'ENOTFOUND',
+  'EAI_AGAIN',
+  'EHOSTUNREACH',
+  'ENETUNREACH',
+  'UND_ERR_CONNECT_TIMEOUT',
+]);
+
+/** Extract a Node error `code` from a thrown fetch error or its undici `cause`. */
+function fetchErrorCode(error: unknown): string | undefined {
+  const read = (value: unknown): string | undefined => {
+    if (value != null && typeof value === 'object' && 'code' in value) {
+      const code = (value as { code?: unknown }).code;
+      return typeof code === 'string' ? code : undefined;
+    }
+    return undefined;
+  };
+  if (error != null && typeof error === 'object') {
+    return read((error as { cause?: unknown }).cause) ?? read(error);
+  }
+  return undefined;
+}
+
+/** Whether a thrown fetch error definitely means nothing was sent/started: a
+ *  pre-connect failure or a TLS handshake failure (both precede any request bytes). */
+function isDefiniteConnectFailure(error: unknown): boolean {
+  const code = fetchErrorCode(error);
+  if (code == null) {
+    return false;
+  }
+  return PRE_CONNECT_ERROR_CODES.has(code) || code.startsWith('ERR_TLS') || code.includes('CERT');
+}
+
 async function postChatMessage(
   deps: ScheduleEngineDeps,
   schedule: FireableSchedule,
@@ -97,10 +134,17 @@ async function postChatMessageInner(
       }),
     });
   } catch (error) {
-    // fetch threw: no response was received. The request may or may not have
-    // been processed — ambiguous, so don't terminalize as a definite error.
+    // fetch threw before a response. A PRE-CONNECT failure (bad SCHEDULES_SELF_URL:
+    // DNS/connection refused/connect-timeout/TLS) means the request never reached this
+    // server, so nothing could have started — a DEFINITE rejection that terminalizes as
+    // `error` (countable, can auto-disable the broken schedule). A mid-flight failure
+    // (reset after send, request timeout) is genuinely ambiguous: the generation may
+    // already be running, so leave the run reconcilable.
     const message = error instanceof Error ? error.message : String(error);
-    throw new ScheduleFireError(`Fire POST network failure: ${message}`, true);
+    throw new ScheduleFireError(
+      `Fire POST network failure: ${message}`,
+      !isDefiniteConnectFailure(error),
+    );
   }
   if (!response.ok) {
     const body = await response.text().catch(() => '');
@@ -175,6 +219,16 @@ export async function fireSchedule(
   const rollbackReservation = async () => {
     if (schedule.leaseBy != null && (await methods.holdsLease(schedule.id, schedule.leaseBy))) {
       await methods.deleteScheduleRun(schedule.id, scheduledFor, 'started');
+      return;
+    }
+    // The schedule was HARD-deleted out from under this fire (account deletion racing
+    // the claim -> reserve window): holdsLease is false because the schedule is GONE,
+    // not because the lease was taken over. The reserved row is now an orphan no
+    // reconciler will own (its schedule no longer exists), so delete it. Guarded on
+    // actual absence so a lease TAKEOVER (schedule still present, different holder)
+    // still leaves the row for whoever now holds the lease.
+    if (!(await methods.scheduleExists(schedule.id))) {
+      await methods.deleteScheduleRun(schedule.id, scheduledFor, 'started');
     }
   };
 

packages/data-schemas/src/methods/schedule.methods.spec.ts

@@ -868,6 +868,38 @@ describe('acquireManualRunLease / releaseLease', () => {
       await methods.acquireManualRunLease(schedule.id, new mongoose.Types.ObjectId(), 60_000),
     ).toBeNull();
   });
+
+  it('uses a unique per-lease holder so a stale run-now cannot strip a replacement lease', async () => {
+    const schedule = await methods.createSchedule(scheduleData());
+    const first = await methods.acquireManualRunLease(schedule.id, schedule.user, 60_000);
+    expect(first?.leaseBy).toMatch(/^manual:/);
+    // Release and re-acquire (a fresh run-now): the replacement lease has a DIFFERENT holder.
+    await methods.releaseLease(schedule.id);
+    const second = await methods.acquireManualRunLease(schedule.id, schedule.user, 60_000);
+    expect(second?.leaseBy).toMatch(/^manual:/);
+    expect(second?.leaseBy).not.toBe(first?.leaseBy);
+    // A stalled fire from the FIRST lease releasing by its (old) holder must NOT clear the
+    // second lease — the unique holder makes releaseLeaseByHolder no-op there.
+    await methods.releaseLeaseByHolder(schedule.id, first!.leaseBy!);
+    const afterStale = await getSchedule(schedule.id);
+    expect(afterStale.leaseBy).toBe(second?.leaseBy);
+    expect(afterStale.leaseUntil).toBeDefined();
+    // Releasing by the CURRENT holder does clear it.
+    await methods.releaseLeaseByHolder(schedule.id, second!.leaseBy!);
+    const cleared = await getSchedule(schedule.id);
+    expect(cleared.leaseBy).toBeUndefined();
+  });
+
+  it('scheduleExists distinguishes a hard-deleted schedule from a soft-deleting one', async () => {
+    const schedule = await methods.createSchedule(scheduleData());
+    expect(await methods.scheduleExists(schedule.id)).toBe(true);
+    // Soft-delete (deleting:true) still EXISTS — its run drains via the reconciler.
+    await methods.markScheduleDeleting(schedule.id, schedule.user);
+    expect(await methods.scheduleExists(schedule.id)).toBe(true);
+    // A hard delete makes it truly gone.
+    await methods.deleteScheduleById(schedule.id, schedule.user);
+    expect(await methods.scheduleExists(schedule.id)).toBe(false);
+  });
 });
 
 describe('claim-token fencing (stale worker cannot mutate an edited/deleted schedule)', () => {

packages/data-schemas/src/methods/schedule.ts

@@ -86,6 +86,7 @@ export type ScheduleMethods = {
   ) => Promise<ISchedule | null>;
   deleteScheduleById: (id: string, userId: string | Types.ObjectId) => Promise<boolean>;
   getScheduleById: (id: string, userId?: string | Types.ObjectId) => Promise<ISchedule | null>;
+  scheduleExists: (id: string) => Promise<boolean>;
   getSchedulesByUser: (userId: string | Types.ObjectId) => Promise<ISchedule[]>;
   countSchedulesByUser: (userId: string | Types.ObjectId) => Promise<number>;
   claimDueSchedule: (params: ClaimDueScheduleParams) => Promise<ISchedule | null>;
@@ -260,6 +261,12 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
     return Schedule().findOne(filter).lean<ISchedule>();
   }
 
+  /** Raw existence check, ignoring the `deleting` soft-delete flag. Distinguishes a
+   *  HARD-deleted schedule (gone) from a lease takeover (schedule still present). */
+  async function scheduleExists(id: string): Promise<boolean> {
+    return (await Schedule().exists({ id })) != null;
+  }
+
   async function getSchedulesByUser(userId: string | Types.ObjectId): Promise<ISchedule[]> {
     // Hide schedules pending erasure (soft-deleted, draining their active runs)
     // so a deleted schedule disappears immediately for the owner.
@@ -332,6 +339,11 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
     // claimDueSchedule), not this worker's clock: a skewed replica must not read a
     // Mongo-written automatic-fire lease as expired early and start a second run.
     const claimToken = randomUUID();
+    // A UNIQUE per-lease holder (not the constant 'manual'): the superseded-fire
+    // cleanup releases by holder (leaseBy), so a stale run-now that stalled past its
+    // lease must not match — and strip — the fresh lease a newer run-now acquired.
+    // The claimToken already fences the lease, so reuse it as the holder discriminator.
+    const leaseBy = `manual:${claimToken}`;
     return Schedule()
       .findOneAndUpdate(
         {
@@ -340,7 +352,7 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
           deleting: { $ne: true },
           $expr: { $lt: [{ $ifNull: ['$leaseUntil', new Date(0)] }, '$$NOW'] },
         },
-        [{ $set: { leaseUntil: { $add: ['$$NOW', leaseMs] }, leaseBy: 'manual', claimToken } }],
+        [{ $set: { leaseUntil: { $add: ['$$NOW', leaseMs] }, leaseBy, claimToken } }],
         { new: true },
       )
       .lean<ISchedule>();
@@ -980,6 +992,7 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
     updateScheduleById,
     deleteScheduleById,
     getScheduleById,
+    scheduleExists,
     getSchedulesByUser,
     countSchedulesByUser,
     claimDueSchedule,