Recovery skips orphaned retrying deliveries whose target type changed to a non-retry type #82

New Issue

clawbot · 2026-08-07T17:12:07+02:00

clawbot commented

2026-08-07 17:12:07 +02:00

Low-severity edge case surfaced by the independent review of PR #81 (delivery Target-interface refactor).

Background

After #81, restart recovery and the 60s sweep look each orphaned retrying delivery's target up in the registry and only reschedule it if the target implements the internal rescheduler (i.e. http/slack). Fire-and-forget targets (database/log) never set status retrying, so under normal operation nothing is dropped — this is correct and was verified in review.

The edge case

If a target's type is mutated in the DB from a retry type (http/slack) to a fire-and-forget type (or an unknown type) while it still has an orphaned retrying delivery, that delivery now stays stuck in retrying forever. The pre-refactor code would eventually re-dispatch it and let the new target type resolve it to delivered/failed.

This requires editing a target's type out from under an in-flight retrying delivery, so it does not arise in normal operation, and neither the old nor the new code handles it "correctly" (old: re-dispatches under a type that never produced the retry; new: leaves it stuck). Noted for completeness; it was explicitly not a merge blocker.

Definition of done

Decide the intended behaviour for a retrying delivery whose target type no longer supports retries, and make recovery/sweep do that deterministically. Options:

terminally mark such orphaned deliveries failed (with a recorded reason) during recovery, or
re-dispatch them once through the current target type (closer to the old behaviour), or
explicitly document that mutating a target's type does not migrate in-flight deliveries and leave them for manual cleanup.

Pick one, implement it, and add a recovery test that seeds a retrying delivery whose target type has changed and asserts the chosen outcome.

Low-severity edge case surfaced by the independent review of PR #81 (delivery Target-interface refactor). ## Background After #81, restart recovery and the 60s sweep look each orphaned `retrying` delivery's target up in the registry and only reschedule it if the target implements the internal `rescheduler` (i.e. `http`/`slack`). Fire-and-forget targets (`database`/`log`) never set status `retrying`, so under normal operation nothing is dropped — this is correct and was verified in review. ## The edge case If a target's `type` is mutated in the DB from a retry type (`http`/`slack`) to a fire-and-forget type (or an unknown type) while it still has an orphaned `retrying` delivery, that delivery now stays stuck in `retrying` forever. The pre-refactor code would eventually re-dispatch it and let the new target type resolve it to `delivered`/`failed`. This requires editing a target's type out from under an in-flight retrying delivery, so it does not arise in normal operation, and neither the old nor the new code handles it "correctly" (old: re-dispatches under a type that never produced the retry; new: leaves it stuck). Noted for completeness; it was explicitly not a merge blocker. ## Definition of done Decide the intended behaviour for a `retrying` delivery whose target type no longer supports retries, and make recovery/sweep do that deterministically. Options: - terminally mark such orphaned deliveries `failed` (with a recorded reason) during recovery, or - re-dispatch them once through the current target type (closer to the old behaviour), or - explicitly document that mutating a target's type does not migrate in-flight deliveries and leave them for manual cleanup. Pick one, implement it, and add a recovery test that seeds a `retrying` delivery whose target type has changed and asserts the chosen outcome.

clawbot referenced this issue

2026-08-07 17:12:13 +02:00

Refactor delivery targets to a Target interface (closes #77) #81

clawbot referenced this issue

2026-08-07 17:20:10 +02:00

Refactor delivery targets to a Target interface (closes #77) #81

clawbot referenced this issue

2026-08-07 18:07:15 +02:00

Design: should delivery targets own their recovery/sweep loop, not just backoff? #85

clawbot referenced this issue

2026-08-07 18:07:44 +02:00

1.0/mvp #33