fix: restore reminders after cancellation failure

Author: bittylicious

README.md

@@ -90,6 +90,10 @@ Reconciliation reads are fenced per reminder and revalidated under the mutation
 cancellation returns, a read that completed before deletion cannot restore stale list state or a
 timer, even if its continuation resumes later.
 
+If durable cancellation fails, the plugin re-reads authoritative state through the same fence and
+lock protocol, restores a delayed timer, and returns the original deletion error. The reminder stays
+visible and can be cancelled again without restarting the server.
+
 This is deliberately not a claim of crash-proof exactly-once delivery. Any process, storage, or lock
 failure after the prompt API accepts a request but before its durable transition can lead to retry.
 That is at-least-once crash semantics; true exactly-once delivery requires idempotency support in the

index.ts

@@ -130,7 +130,7 @@ const RemindersPlugin: Plugin = async (ctx) => {
 
         let cancelledCount = 0
         for (const reminder of remindersToCancel) {
-          if (await cancelReminder(reminder.id, ctx, state)) cancelledCount++
+          if (await cancelReminder(reminder.id, ctx, state, config)) cancelledCount++
         }
 
         logger.info(`[RemindersPlugin] Cancelled ${cancelledCount} reminders for session ${sessionID}`)
@@ -140,7 +140,7 @@ const RemindersPlugin: Plugin = async (ctx) => {
     tool: {
       reminderadd: createReminderAddTool(ctx, state, () => config),
       reminderlist: createReminderListTool(state),
-      reminderremove: createReminderRemoveTool(ctx, state),
+      reminderremove: createReminderRemoveTool(ctx, state, () => config),
     },
   }
 }

scheduler.ts

@@ -22,13 +22,15 @@ type ProjectRuntime = {
 type ReminderFence = {
   version: number
   reconciliations: number
+  deletions: number
   tombstone: boolean
 }
 
 type ScheduleOptions = {
   persist?: boolean
   skipOverdue?: boolean
   retryAttempt?: number
+  minimumDelayMs?: number
 }
 
 const RECONCILIATION_DELAY_MS = 250
@@ -57,6 +59,7 @@ function getRuntime(ctx: PluginInput): ProjectRuntime {
     runtimes.set(key, runtime)
   }
   runtime.fences ??= new Map()
+  for (const fence of runtime.fences.values()) fence.deletions ??= 0
   return runtime
 }
 
@@ -139,7 +142,7 @@ function clearStateReminder(id: string, state: State): void {
 function beginReconciliation(id: string, runtime: ProjectRuntime): { fence: ReminderFence; version: number } {
   let fence = runtime.fences.get(id)
   if (!fence) {
-    fence = { version: 0, reconciliations: 0, tombstone: false }
+    fence = { version: 0, reconciliations: 0, deletions: 0, tombstone: false }
     runtime.fences.set(id, fence)
   }
   fence.reconciliations++
@@ -160,21 +163,31 @@ function finishReconciliation(id: string, runtime: ProjectRuntime, fence: Remind
   retireFence(id, runtime, fence)
 }
 
-function invalidateReminder(id: string, runtime: ProjectRuntime): ReminderFence {
+function invalidateReminder(
+  id: string,
+  runtime: ProjectRuntime,
+): { fence: ReminderFence; version: number } {
   let fence = runtime.fences.get(id)
   if (!fence) {
-    fence = { version: 0, reconciliations: 0, tombstone: false }
+    fence = { version: 0, reconciliations: 0, deletions: 0, tombstone: false }
     runtime.fences.set(id, fence)
   }
   fence.version++
+  fence.deletions++
   fence.tombstone = true
-  return fence
+  return { fence, version: fence.version }
+}
+
+function finishInvalidation(id: string, runtime: ProjectRuntime, fence: ReminderFence): void {
+  fence.deletions--
+  retireFence(id, runtime, fence)
 }
 
 function retireFence(id: string, runtime: ProjectRuntime, fence: ReminderFence): void {
   if (
     runtime.fences.get(id) === fence
     && fence.reconciliations === 0
+    && fence.deletions === 0
     && !runtime.executions.has(id)
   ) {
     runtime.fences.delete(id)
@@ -183,17 +196,17 @@ function retireFence(id: string, runtime: ProjectRuntime, fence: ReminderFence):
 
 async function deleteReminderLocked(id: string, ctx: PluginInput, state: State): Promise<void> {
   const runtime = getRuntime(ctx)
-  const fence = invalidateReminder(id, runtime)
+  const { fence, version } = invalidateReminder(id, runtime)
   const execution = stopLocalExecution(id, runtime)
   clearStateReminder(id, state)
   if (execution && execution.state !== state) clearStateReminder(id, execution.state)
   try {
     await deleteReminder(id, ctx)
   } catch (error) {
-    fence.tombstone = false
+    if (runtime.fences.get(id) === fence && fence.version === version) fence.tombstone = false
     throw error
   } finally {
-    retireFence(id, runtime, fence)
+    finishInvalidation(id, runtime, fence)
   }
 }
 
@@ -243,7 +256,7 @@ export async function scheduleTimer(
   }
 
   const now = Date.now()
-  const delay = Math.max(0, reminder.time.nextExecution - now)
+  const delay = Math.max(options.minimumDelayMs ?? 0, reminder.time.nextExecution - now, 0)
   const skipOverdue = options.skipOverdue === true && reminder.time.nextExecution < now
   const retryAttempt = reminder.time.nextExecution < now ? options.retryAttempt ?? 0 : 0
   const timer = setTimeout(async () => {
@@ -292,6 +305,7 @@ function scheduleReconciliation(
     await reconcileReminder(id, ctx, state, config, {
       skipOverdue: options.skipOverdue,
       retryAttempt,
+      minimumDelayMs: options.minimumDelayMs,
     })
   }, delay)
   timer.unref()
@@ -339,6 +353,7 @@ export async function reconcileReminder(
         persist: false,
         skipOverdue: options.skipOverdue,
         retryAttempt: options.retryAttempt,
+        minimumDelayMs: options.minimumDelayMs,
       })
     })
   } catch (error) {
@@ -565,10 +580,16 @@ export async function executeReminder(
   }
 }
 
-export async function cancelReminder(id: string, ctx: PluginInput, state: State): Promise<boolean> {
+export async function cancelReminder(
+  id: string,
+  ctx: PluginInput,
+  state: State,
+  config: PluginConfig,
+): Promise<boolean> {
   const runtime = getRuntime(ctx)
-  const fence = invalidateReminder(id, runtime)
+  const { fence, version } = invalidateReminder(id, runtime)
   const execution = stopLocalExecution(id, runtime)
+  const rollbackState = execution?.state ?? state
   clearStateReminder(id, state)
   if (execution && execution.state !== state) clearStateReminder(id, execution.state)
   try {
@@ -578,13 +599,23 @@ export async function cancelReminder(id: string, ctx: PluginInput, state: State)
       clearStateReminder(id, state)
       if (replacement && replacement.state !== state) clearStateReminder(id, replacement.state)
     })
-    retireFence(id, runtime, fence)
+    finishInvalidation(id, runtime, fence)
     logger.info(`Reminder ${id} cancelled`)
     return true
   } catch (error) {
-    fence.tombstone = false
-    retireFence(id, runtime, fence)
+    const ownsFence = runtime.fences.get(id) === fence && fence.version === version
+    if (ownsFence) fence.tombstone = false
+    finishInvalidation(id, runtime, fence)
     logger.error(`Failed to cancel reminder ${id}:`, error)
+    if (ownsFence) {
+      try {
+        await reconcileReminder(id, ctx, rollbackState, config, {
+          minimumDelayMs: RECONCILIATION_DELAY_MS,
+        })
+      } catch (rollbackError) {
+        logger.error(`Failed to restore reminder ${id} after cancellation failure:`, rollbackError)
+      }
+    }
     throw error
   }
 }

test/README.md

@@ -11,6 +11,8 @@ responsible for closing their children during cleanup.
 
 Cancellation tests also pause a completed reconciliation read, finish durable cancellation, and
 then release the stale continuation to verify it cannot restore local state or a timer.
+Failed-deletion coverage verifies that authoritative state and a future timer are restored before
+the original error is returned, allowing a subsequent cancellation attempt to succeed.
 
 The suite intentionally does not claim exact-once prompt delivery across a crash or storage failure
 after prompt acceptance. It tests the plugin's same-host coordination and reconciliation behavior.

test/integration.test.ts

@@ -264,7 +264,7 @@ describe("Integration Tests", () => {
     await waitFor(async () => (await loadReminder(reminder.id, ctx))!.time.nextExecution > Date.now())
     expect(prompts).toBeLessThanOrEqual(4)
     expect(state.timers.has(reminder.id)).toBe(true)
-    await cancelReminder(reminder.id, ctx, state)
+    await cancelReminder(reminder.id, ctx, state, config)
   })
 
   test("reminderlist tool returns empty for new session", async () => {

test/scheduler.test.ts

@@ -164,19 +164,62 @@ describe("Scheduler", () => {
     expect(state.timers.has(reminder.id)).toBe(true)
     expect(state.reminders.has(reminder.id)).toBe(true)
 
-    await cancelReminder(reminder.id, ctx, state)
+    await cancelReminder(reminder.id, ctx, state, config)
 
     expect(state.timers.has(reminder.id)).toBe(false)
     expect(state.reminders.has(reminder.id)).toBe(false)
   })
 
   test("cancelReminder handles non-existent reminder gracefully", async () => {
-    await cancelReminder("non-existent", ctx, state)
+    await cancelReminder("non-existent", ctx, state, config)
 
     expect(state.timers.size).toBe(0)
     expect(state.reminders.size).toBe(0)
   })
 
+  test("failed durable cancellation restores authoritative state for retry", async () => {
+    const reminder: Reminder = {
+      id: "rem-cancel-delete-failure",
+      sessionID: "ses-test",
+      projectID: state.projectID,
+      type: "recurring",
+      interval: 60000,
+      originalPrompt: "remain retryable",
+      userDescription: "Cancellation delete failure",
+      time: { created: Date.now(), nextExecution: Date.now() + 60000 },
+      status: "active",
+    }
+    let prompts = 0
+    ;(ctx.client.session.prompt as any) = async () => {
+      prompts++
+      return { data: {}, error: undefined, response: {} }
+    }
+    state.reminders.set(reminder.id, reminder)
+    await scheduleTimer(reminder, ctx, state, config)
+
+    const deletionError = Object.assign(new Error("simulated reminder deletion failure"), { code: "EIO" })
+    const rmFile = fsPromises.rm
+    const remove = spyOn(fsPromises, "rm").mockImplementation((async (file: any, options: any) => {
+      if (String(file).endsWith(`${reminder.id}.json`)) throw deletionError
+      return rmFile(file, options)
+    }) as any)
+    try {
+      await expect(cancelReminder(reminder.id, ctx, state, config)).rejects.toBe(deletionError)
+    } finally {
+      remove.mockRestore()
+    }
+
+    expect(await loadReminder(reminder.id, ctx)).toEqual(reminder)
+    expect(state.reminders.get(reminder.id)).toEqual(reminder)
+    expect(state.timers.has(reminder.id)).toBe(true)
+    expect(prompts).toBe(0)
+
+    await cancelReminder(reminder.id, ctx, state, config)
+    expect(await loadReminder(reminder.id, ctx)).toBeNull()
+    expect(state.reminders.has(reminder.id)).toBe(false)
+    expect(state.timers.has(reminder.id)).toBe(false)
+  })
+
   test("executeReminder calls session prompt", async () => {
     let promptCalled = false
     let capturedPrompt = ""
@@ -428,7 +471,7 @@ describe("Scheduler", () => {
     const execution = executeReminder(reminder, ctx, state, config)
     await promptStarted
     const lock = await acquireMutationLock(reminder.id, ctx)
-    const cancellation = cancelReminder(reminder.id, ctx, state)
+    const cancellation = cancelReminder(reminder.id, ctx, state, config)
     await lock.release()
     await cancellation
     finish()
@@ -456,7 +499,7 @@ describe("Scheduler", () => {
     await executeReminder(reminder, ctx, state, config)
     expect(await loadReminder(reminder.id, ctx)).not.toBeNull()
 
-    await cancelReminder(reminder.id, ctx, state)
+    await cancelReminder(reminder.id, ctx, state, config)
     expect(await loadReminder(reminder.id, ctx)).toBeNull()
     expect(state.reminders.has(reminder.id)).toBe(false)
     expect(state.timers.has(reminder.id)).toBe(false)
@@ -630,7 +673,7 @@ describe("Scheduler", () => {
     try {
       const reconciliation = reconcileReminder(reminder.id, ctx, state, config)
       await readCompleted.promise
-      await cancelReminder(reminder.id, ctx, state)
+      await cancelReminder(reminder.id, ctx, state, config)
 
       expect(state.reminders.has(reminder.id)).toBe(false)
       expect(state.timers.has(reminder.id)).toBe(false)
@@ -724,7 +767,7 @@ describe("Scheduler", () => {
     await scheduleTimer(reminder, ctx, state, config)
 
     const signal = await promptStarted.promise
-    await cancelReminder(reminder.id, ctx, state)
+    await cancelReminder(reminder.id, ctx, state, config)
     finishPrompt.resolve()
     const directory = await getStorageDir(ctx)
     await waitFor(async () => {
@@ -780,7 +823,7 @@ describe("Scheduler", () => {
     expect((await loadReminder(reminder.id, ctx))?.time.nextExecution).toBe(
       replacementReminder.time.nextExecution,
     )
-    await cancelReminder(reminder.id, ctx, replacement)
+    await cancelReminder(reminder.id, ctx, replacement, config)
   })
 
   test("forwards a captured agent and omits it for legacy reminders", async () => {

tools/reminderremove.ts

@@ -1,10 +1,14 @@
 import { tool, type PluginInput } from "@opencode-ai/plugin"
-import type { State } from "../types"
+import type { PluginConfig, State } from "../types"
 import { cancelReminder } from "../scheduler"
 import DESCRIPTION from "./reminderremove.txt"
 import { logger } from "../logger"
 
-export function createReminderRemoveTool(ctx: PluginInput, state: State) {
+export function createReminderRemoveTool(
+  ctx: PluginInput,
+  state: State,
+  getConfig: () => PluginConfig,
+) {
   return tool({
     description: DESCRIPTION,
 
@@ -33,7 +37,7 @@ export function createReminderRemoveTool(ctx: PluginInput, state: State) {
       // Cancel all matching reminders
       const cancelledDescriptions: string[] = []
       for (const reminder of matches) {
-        if (await cancelReminder(reminder.id, ctx, state)) {
+        if (await cancelReminder(reminder.id, ctx, state, getConfig())) {
           cancelledDescriptions.push(reminder.userDescription)
           logger.info(`[RemindersPlugin] Cancelled reminder ${reminder.id} via user request`)
         }