Skip to content

fix(delivery): ownership-token in-flight lock prevents stale finally double-delivery - #176

Open
donrami wants to merge 1 commit into
QuintinShaw:mainfrom
donrami:delivery-lock-fix
Open

fix(delivery): ownership-token in-flight lock prevents stale finally double-delivery#176
donrami wants to merge 1 commit into
QuintinShaw:mainfrom
donrami:delivery-lock-fix

Conversation

@donrami

@donrami donrami commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #175.

deliverAndAck's in-flight lock (Set<runId>) could be released twice for one run: the failed send's .then releases it, the generation-change retry re-acquires it for the next send, and the original .finally deletes it again — leaving the retry send unprotected so a third caller (bind flush / duplicate complete) can double-deliver the same workflow-result turn.

Change: the lock becomes Map<runId, token>. Each deliver-and-ack captures its own token at acquisition; both .then and .finally release via release(), which deletes only if the stored token still matches. A superseded chain can no longer drop a newer send's lock.

Verification:

  • New regression test a failed send overlapping a re-bind cannot double-deliver — fails on the old Set lock (retry released mid-flight), passes with the token lock.
  • Full suite: 1331/1333 — the 2 failures (workflow-authoring-skill, workflow-release-gate) fail on pristine main too (pack-shape/environment, pre-existing).
  • biome + tsc clean; live omp E2E on this tree: workflow launched in background, workflow-result delivered with pendingDelivery cleared, zero probe entries.

…double-delivery

A failed send's .then released the lock and the generation-change retry
re-acquired it for send 2; the original .finally then unconditionally
deleted that new lock mid-flight, letting a third caller (another bind
flush or complete event) start a duplicate workflow-result turn.

The lock is now runId -> ownership token (Map<string, number>): release
only if you own the token. Regression test covers the fail-overlap-rebind
race. Also removes the scratch repro helper.

Follow-up from QuintinShaw's QuintinShaw#169 approval comment.
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.

Delivery: a failed send overlapping a re-bind can double-deliver workflow results

1 participant