Skip to content

broker: persisted dedup cache does not protect pending-delivery replay after crash/restart #1263

Description

@willwashburn

Background

Follow-up from the PR #1098 review (persisted dedup cache).

PR #1098 added persistence for the runtime DedupCache (snapshot to dedup-{name}.json, reload on startup dropping expired entries). The original init.rs comment and CHANGELOG framed this as: "a crash + restart no longer re-injects duplicates when the persisted pending file replays."

Finding

That guarantee does not hold. The persisted runtime cache only contains:

  • relaycast spawn/release control-event dedup keys (control:{workspace_id}:agent.spawn_requested:{identity}), and
  • read-ack dedup keys (delivery_read_ack:{worker_name}:{event_id}).

The pending-delivery replay path — handle_maintenance_tickretry_pending_deliveryworkers.deliver(...) — never consults the DedupCache, and no delivery/injection-identity key is ever inserted into it. So after a crash + restart, a persisted pending delivery that was already injected (but not yet cleared from the pending file) is re-injected; the persisted cache does not prevent it.

(Note: the only cache that actually gates injection is the wrap-mode DedupCache in wrap.rs, keyed {workspace_id}:{event_id}, and that one is never persisted.)

What we shipped as the interim fix

Corrected the misleading init.rs comment and the CHANGELOG bullet to describe what the persisted cache actually does (control-event + read-ack dedup across restart). No behavioral change.

What's left (this issue)

If we want the crash/restart duplicate-injection guarantee to actually hold, implement it:

  1. Add a delivery-injection-identity key namespace (e.g. delivery_inject:{worker}:{event_id}), gate retry_pending_delivery on it before workers.deliver.
  2. Seed that key when a delivery is confirmed acked (worker_events.rs, near clear_pending_delivery_if_event_matches), so an already-acked delivery isn't re-injected on replay.
  3. Ensure the key's persistence/clearing stays consistent with pending_deliveries so the two files can't disagree across restarts.

Pointers

  • crates/broker/src/runtime/init.rs — dedup load + comment
  • crates/broker/src/runtime/maintenance.rshandle_maintenance_tick
  • crates/broker/src/runtime/delivery.rsretry_pending_delivery
  • crates/broker/src/dedup.rsDedupCache, key semantics

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions