Skip to content

Commit 618810c

Browse files
authored
fix(agents-server): stop orphaned cron wakes (#4559)
Fixes orphaned cron wakes in Agents Server after a manifest-backed schedule is deleted. Users should no longer see agents repeatedly woken by a cron schedule that `list_schedules` correctly reports as absent. ## Root Cause Cron schedules are represented by two linked pieces of state: 1. a manifest-backed schedule row on the agent stream, used by tools like `list_schedules` 2. wake-registry + scheduler side effects, used to deliver cron ticks For `/horton/RbYyjlX3ey`, the manifest schedule had been deleted, so `list_schedules` returned `[]`. However, cron wake delivery kept going because stale wake/scheduler side effects could outlive the manifest deletion: - `WakeRegistry` removed cached registrations on Electric shape deletes by deriving the DB id from the shape message key. Shape keys are protocol-level identifiers and are not guaranteed to be the table primary key, so the DB row could be gone while the in-memory registration remained. - Cron ticks rescheduled themselves indefinitely, even when no `wake_registrations` rows still subscribed to that cron stream. ## Approach This PR makes the cron side effects self-healing in both places. ### Wake registry delete handling `WakeRegistry.applyShapeMessage` now treats `old_value.id` as authoritative for delete messages. The shape uses `replica: full`, so deletes should carry the deleted row; if the id is missing, the registry resets its cache to fail closed instead of keeping a potentially stale wake registration alive. ```ts const oldValue = (message as unknown as { old_value?: { id?: unknown } }).old_value const oldId = Number(oldValue?.id) if (Number.isFinite(oldId)) { this.removeCachedRegistrationByDbId(oldId) } else { this.resetCachedRegistrations() } ``` ### Cron chain rescheduling Before inserting the next cron tick, the scheduler checks whether any wake registration still points at that cron stream: ```ts select 1 as exists from wake_registrations where tenant_id = ${tenantId} and source_url = ${streamPath} limit 1 ``` If there are no subscribers, it completes the current task and stops the chain. A future schedule registration will seed a fresh tick through `getOrCreateCronStream` / rehydration. ## Key Invariants - The manifest remains the user-visible source of truth for schedule tools. - A cron stream should only keep producing future ticks while at least one wake registration subscribes to it. - Deleting a manifest-backed cron schedule must remove both durable wake registrations and any effective in-memory wake registrations. - Shared cron streams remain reusable: deleting the last subscriber stops the chain; adding a subscriber later recreates it. ## Non-goals - This does not change the public schedule tool API. - This does not delete historical cron stream events. - This does not introduce per-schedule cron streams; cron streams remain shared by expression/timezone. - This does not attempt to clean up already-delivered wake events from agent timelines. ## Trade-offs An alternative would be to always reschedule cron ticks forever for every cron stream ever created. That is simpler, but it allows orphaned cron streams to keep waking stale in-memory subscribers and creates unnecessary scheduler churn. Checking for subscribers before rescheduling keeps cron streams lazy and aligns their lifecycle with active wake registrations. Another option would be to rely only on wake-registry cache invalidation. The scheduler guard is still useful as a second line of defense and prevents orphaned cron task chains even if a process restarts or a wake cache bug reappears. ## Verification Commands run: ```bash pnpm install pnpm --filter @electric-ax/agents-runtime build pnpm --filter @electric-ax/agents-mcp build pnpm --filter @electric-sql/client build pnpm --filter @electric-ax/agents-server typecheck cd packages/agents-server && pnpm test test/scheduler.test.ts cd packages/agents-server && pnpm test test/wake-registry.test.ts -t "removes cached registrations" GITHUB_BASE_REF=main node scripts/check-changeset.mjs git diff --check ``` Notes: - `pnpm install` completed dependency linking but reported warnings, including `examples/.shared prepare` failing due SST. - The focused wake-registry unit test passes. Running all of `wake-registry.test.ts` locally reached the integration section and timed out with a webhook delivery `fetch failed` warning, so the new unit path was verified with `-t`. - `check-changeset` passes. ## Files changed - `packages/agents-server/src/wake-registry.ts` — remove cached registrations using `old_value.id` from shape delete messages; reset the cache if a delete id is unavailable. - `packages/agents-server/src/scheduler.ts` — stop rescheduling cron ticks when no registrations subscribe to the cron stream; clarify cron payload stream-path extraction and subscriber lookup. - `packages/agents-server/test/scheduler.test.ts` — cover subscriber-present and no-subscriber cron reschedule paths. - `packages/agents-server/test/wake-registry.test.ts` — cover non-id shape keys with delete ids coming from `old_value.id`. - `.changeset/fix-orphaned-cron-wakes.md` — patch changeset for `@electric-ax/agents-server`.
1 parent 0947230 commit 618810c

5 files changed

Lines changed: 117 additions & 2 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@electric-ax/agents-server': patch
3+
---
4+
5+
Stop orphaned cron wakes after schedule deletion by clearing stale wake-registry entries and ending cron tick chains with no subscribers.

packages/agents-server/src/scheduler.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,14 @@ export function isPermanentElectricAgentsError(err: unknown): boolean {
228228
)
229229
}
230230

231+
function cronTaskStreamPath(
232+
payload: DelayedSendPayload | CronTickPayload
233+
): string | null {
234+
return typeof (payload as { streamPath?: unknown }).streamPath === `string`
235+
? (payload as { streamPath: string }).streamPath
236+
: null
237+
}
238+
231239
function normalizeTask(row: ScheduledTaskRow): {
232240
id: number
233241
tenantId: string
@@ -680,6 +688,24 @@ export class Scheduler implements SchedulerClient {
680688
task.fireAt
681689
)
682690

691+
const streamPath = cronTaskStreamPath(task.payload)
692+
const subscriberRows = streamPath
693+
? await sql<Array<{ exists: number }>>`
694+
select 1 as exists
695+
from wake_registrations
696+
where tenant_id = ${tenantId}
697+
and source_url = ${streamPath}
698+
limit 1
699+
`
700+
: []
701+
702+
// Cron streams are virtual shared sources. If no wake registrations
703+
// still point at this cron stream (e.g. the owning manifest schedule was
704+
// deleted), stop the chain here instead of keeping a forever-global tick
705+
// alive. Rehydration/getOrCreateCronStream will seed a fresh tick when a
706+
// subscription is recreated.
707+
if (subscriberRows.length === 0) return
708+
683709
await sql`
684710
insert into scheduled_tasks (
685711
tenant_id,

packages/agents-server/src/wake-registry.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,23 @@ export class WakeRegistry {
739739
}
740740

741741
if (message.headers.operation === `delete`) {
742-
this.removeCachedRegistrationByDbId(Number(message.key))
742+
// Shape keys are protocol-level identifiers and are not guaranteed to be
743+
// the table primary key. The wake_registrations shape uses
744+
// `replica: full`, so deletes should carry the deleted row in old_value;
745+
// use that row id to remove the matching in-memory registration. If the
746+
// id is unavailable, reset the cache so we fail closed rather than
747+
// keeping a stale wake registration alive.
748+
const oldValue = (
749+
message as unknown as {
750+
old_value?: { id?: unknown }
751+
}
752+
).old_value
753+
const oldId = Number(oldValue?.id)
754+
if (Number.isFinite(oldId)) {
755+
this.removeCachedRegistrationByDbId(oldId)
756+
} else {
757+
this.resetCachedRegistrations()
758+
}
743759
return
744760
}
745761

packages/agents-server/test/scheduler.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ describe(`Scheduler`, () => {
314314

315315
it(`reschedules cron from the stored fireAt instead of now`, async () => {
316316
const mock = createMockPgClient({
317-
txResponses: [[{ id: 11 }], []],
317+
txResponses: [[{ id: 11 }], [{ exists: 1 }], []],
318318
})
319319
const scheduler = new Scheduler({
320320
pgClient: mock.pgClient,
@@ -346,6 +346,42 @@ describe(`Scheduler`, () => {
346346
expect(insertCall!.values[2]).toBe(`2026-04-09T10:30:00.000Z`)
347347
})
348348

349+
it(`stops rescheduling cron ticks when the stream has no subscribers`, async () => {
350+
const mock = createMockPgClient({
351+
txResponses: [[{ id: 11 }], []],
352+
})
353+
const scheduler = new Scheduler({
354+
pgClient: mock.pgClient,
355+
instanceId: `instance-1`,
356+
executors: {
357+
delayed_send: vi.fn(),
358+
cron_tick: vi.fn(),
359+
},
360+
})
361+
362+
await (scheduler as any).completeAndRescheduleCron({
363+
id: 11,
364+
kind: `cron_tick`,
365+
payload: { streamPath: `/_cron/test` },
366+
fireAt: new Date(`2026-04-09T10:00:00.000Z`),
367+
cronExpression: `*/30 * * * *`,
368+
cronTimezone: `UTC`,
369+
cronTickNumber: 4,
370+
})
371+
372+
expect(
373+
mock.txCalls.some((call) => call.sql.includes(`update scheduled_tasks`))
374+
).toBe(true)
375+
expect(
376+
mock.txCalls.some((call) => call.sql.includes(`from wake_registrations`))
377+
).toBe(true)
378+
expect(
379+
mock.txCalls.some((call) =>
380+
call.sql.includes(`insert into scheduled_tasks`)
381+
)
382+
).toBe(false)
383+
})
384+
349385
it(`recovers from transient run loop errors instead of stopping permanently`, async () => {
350386
const scheduler = new Scheduler({
351387
pgClient: createMockPgClient().pgClient,

packages/agents-server/test/wake-registry.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,38 @@ describe(`Wake Registry`, () => {
201201
expect(results[0]!.sourceEventKey).toBe(`insert:tick-7`)
202202
})
203203

204+
it(`removes cached registrations from shape delete old_value ids`, async () => {
205+
const registry = new WakeRegistry(createMockDb())
206+
await registry.register({
207+
subscriberUrl: `/watcher/w1`,
208+
sourceUrl: `/_cron/abc`,
209+
condition: { on: `change` },
210+
oneShot: false,
211+
})
212+
213+
const before = registry.evaluate(`/_cron/abc`, {
214+
type: `cron_tick`,
215+
key: `tick-7`,
216+
value: {},
217+
headers: { operation: `insert` },
218+
})
219+
expect(before).toHaveLength(1)
220+
221+
await (registry as any).applyShapeMessage({
222+
key: `shape-key-is-not-the-registration-id`,
223+
old_value: { id: before[0]!.registrationDbId },
224+
headers: { operation: `delete` },
225+
})
226+
227+
const after = registry.evaluate(`/_cron/abc`, {
228+
type: `cron_tick`,
229+
key: `tick-8`,
230+
value: {},
231+
headers: { operation: `insert` },
232+
})
233+
expect(after).toHaveLength(0)
234+
})
235+
204236
it(`keeps distinct registrations distinct for the same source event`, async () => {
205237
const registry = new WakeRegistry(createMockDb())
206238
await registry.register({

0 commit comments

Comments
 (0)