Skip to content

engine: deliverPendingToNode keeps sending higher seqs after a lower-seq send fails — monotonic-ordering contract not enforced #289

Description

@willwashburn

Severity: minor · packages/engine/src/engine/delivery.ts:894-919

for (const row of rows) {           // ordered asc(seq)
  ...
  const sent = await registry.sendToProvider(...);
  if (sent) deliveredIds.push(row.delivery.id);  // no break on a failed lower seq
}
await markDeliveriesDelivered(db, workspaceId, deliveredIds);

The replay loop continues to the next row even when an earlier (lower-seq) send returned false, so seq N+1 can land on the wire while seq N stayed queued — despite the comment at delivery.ts:762-766 stating these rows form "an ordered, seq-monotonic stream" the broker gates on.

Impact is low in practice (a mid-stream failure usually means a dead socket, so subsequent sends also fail, and queued rows are re-replayed next pass) — but the stated contract isn't enforced; a partial-failure transport creates a wire gap the broker must buffer around.

Fix: break on the first failed send within an agent's stream (per-agent, so one agent's failure doesn't block others).


Found during a multi-agent reliability review of the relay tool/protocol.

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