Skip to content

fix: prevent cancelled reminders from being rescheduled - #2

Open
kernel-oops wants to merge 1 commit into
zelenkovsky:mainfrom
kernel-oops:fix/cancel-in-flight-reminders
Open

fix: prevent cancelled reminders from being rescheduled#2
kernel-oops wants to merge 1 commit into
zelenkovsky:mainfrom
kernel-oops:fix/cancel-in-flight-reminders

Conversation

@kernel-oops

Copy link
Copy Markdown

Summary

  • prevent recurring reminders from being resurrected after cancellation while their prompt is in flight
  • invalidate and abort timers/prompts owned by stale plugin generations during hot reload
  • serialize reminder persistence and retain cancellation tombstones until deletion is complete
  • drain old persistence before replacement initialization restores stored reminders
  • keep valid stored reminders when startup rescheduling encounters a transient write failure

Root cause

The timer handle was removed before session.prompt() completed. A concurrent reminderremove could delete state and storage, but the still-running callback would later save and schedule the same recurring reminder again. Old plugin instances could similarly continue after a hot reload.

The fix gives each project a process-global scheduler generation, each scheduled execution an ownership token and abort controller, and each reminder a serialized persistence chain. Cancellation invalidates ownership before awaiting deletion, so stale continuations cannot recreate a reminder. Replacement initialization invalidates old work synchronously and drains persistence before reading storage.

Tests

  • cancellation while a prompt is pending aborts it and cannot reschedule
  • replacement generation invalidates an active old execution
  • stale generations cannot delete replacement persistence
  • replacement initialization drains an in-flight save before restoring
  • stale remove tools cancel the current replacement reminder
  • failed startup persistence preserves the existing stored reminder
  • future timers from old generations are cleared
  • successful cancellation tombstones retire correctly

Verified in the development environment:

  • bun test — 54 passed, 107 assertions
  • bun x tsc --noEmit
  • git diff --check upstream/main..HEAD

@kernel-oops

Copy link
Copy Markdown
Author

A consolidated integration PR is now available at #5. It deliberately combines #2, #3, and #4, resolves their overlapping scheduler changes, and adds regression fixes found while stress-testing the combined lifecycle. This PR is left open for history/discussion, but the consolidated PR is the recommended merge path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants