Skip to content

fix(reminders): remove execution cap so reminders never skip on capacity - #1839

Merged
Aaronontheweb merged 2 commits into
devfrom
fix/remove-reminder-execution-cap
Aug 9, 2026
Merged

fix(reminders): remove execution cap so reminders never skip on capacity#1839
Aaronontheweb merged 2 commits into
devfrom
fix/remove-reminder-execution-cap

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Problem

The daily vendor-price audit was skipped this morning (Aug 9). Root cause chain:

  1. Capacity gate (MaxConcurrentExecutions = 3, hardcoded) was reached — three tbench/PR test reminders were parked on long executions burning their full 1h timeouts.
  2. PR fix(reminders): retain failed one-shot executions #1812 changed the capacity path from defer-and-redispatch to settle-blocked-occurrence — it acked the occurrence as skipped instead of queueing it.
  3. That settlement ack raced with the cron reschedule (manager re-arms tomorrow's occurrence before acking today's), so the ack returned NotFound, which tripped a reminder.settlement.failed ops alert for a scenario that is not an ops failure.

Fix

Remove the execution cap entirely:

  • Deleted MaxConcurrentExecutions const and the capacity gate in ReminderManagerActor.
  • Reminders now always start executing immediately — no capacity skip, no settlement, no alert noise.
  • Failure/retry semantics are unchanged: Akka.Reminders owns Nack/retry, and every execution still has a one-hour absolute timeout (ExecutionAttemptTimeout), so unbounded scheduling pressure on the LLM is acceptable.
  • Updated the scheduling spec (base + fix(reminders): retain failed one-shot executions #1812 change spec) to match: manager SHALL NOT cap concurrent executions.

Tests

  • Reworked Recurring_occurrence_at_capacity_is_acked_without_execution into Recurring_occurrence_starts_even_while_other_reminders_are_running: three blockers in flight, a fourth reminder fires — asserts it is dispatched (0 skipped duplicates) and executed by the pipeline.
  • Full ReminderManagerActorTests suite passes: 35/35.

Notes

  • The interval in the new test was bumped to 2h so the occurrence passes the pre-existing safe-execution-lease check (deadline = due + interval must exceed 1h+1m); the old test never reached that path because the capacity gate short-circuited first.

…ity (#1812 regression)

The capacity gate (MaxConcurrentExecutions = 3) settled blocked occurrences
instead of deferring them, and when the settlement ack raced with the cron
reschedule it produced a bogus reminder.settlement.failed alert. Worse, it
skipped real reminders (e.g. the daily vendor-price audit) when other
reminders were parked on long-running executions.

Every execution already has a one-hour absolute timeout and Akka.Reminders
owns failure retry, so unbounded scheduling pressure on the LLM is
acceptable. Remove the cap entirely - no capacity skip, no settle, no alert.
@Aaronontheweb Aaronontheweb added reminders Reminder scheduling, execution, and history reliability Retries, resilience, graceful degradation refactoring labels Aug 9, 2026

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we tolerate longer timeouts for reminders, we can afford to apply more scheduling pressure even to smaller, constrained local LLMs. the hard limit of 3 concurrent reminders + buffering we did to the rest is effectively obsolete now

@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) August 9, 2026 12:34
@Aaronontheweb
Aaronontheweb merged commit 13ffc32 into dev Aug 9, 2026
21 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/remove-reminder-execution-cap branch August 9, 2026 13:01
Aaronontheweb added a commit that referenced this pull request Aug 20, 2026
* Sync delta specs for completed OpenSpec changes

Apply the delta specs of 16 completed changes to the main specs. Create four
new capability specs: daemon-shell-path, shell-policy-evaluator-architecture,
skillserver-native-sidecar-sync, and named-model-definitions.

Correct three reminder requirements against the merged code:

- One-shot success removes the definition and its history. Only a poisoned
  one-shot is soft-deleted (PR #1821).
- No execution capacity cap exists (PR #1839). The nack and ack-skip policy
  now covers a duplicate active occurrence and a short acknowledgement lease.
- Every delivery kind now holds its envelope. ReminderDeliveryResult replaces
  ReminderDeliveryObserved.

* Archive 16 completed OpenSpec changes

Move each completed change to openspec/changes/archive/2026-08-19-<name>/.
Their code is merged on dev.

Also tick task 5.3 of consolidate-binding-actor-engines. PR #2005 merged that
work.

Leave eval-run checkboxes unticked. An evals-only gap does not block the
archive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring reliability Retries, resilience, graceful degradation reminders Reminder scheduling, execution, and history

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant