Skip to content

Effect outbox writes a lease on every claim and never reads it #16848

Description

@Adamulek123

Problem

The effect outbox writes a lease on every claim and never reads it, so a claim whose owner died is never reclaimed.

  • EffectOutbox.ts writes lease_expires_at on every claim.
  • claimableCandidatePredicate filters only on status = 'pending' or status = 'running'. It never reads lease age.
  • The only recovery path is reconcileAfterProcessLoss, which runs only after a process-loss event.

So if a claimer dies or wedges and the process-loss reconcile does not fire, that running row is never reclaimed and its lane stays wedged.

#15048 (merged 2026-10-03) made this worse. It changed claimableCandidatePredicate so an earlier pending row in the same thread also blocks a later one. That ordering is correct, but it means one hung running effect now wedges not just the rest of its own lane but every row queued behind it.

Why this is a question rather than an obvious fix

There is a test on main, added by #2829 itself, that deliberately asserts the opposite behaviour:

it.effect("does not reclaim a running effect after its process-local lease expires")
  ...
  assert.isFalse(yield* worker.runOnce);
  assert.deepEqual(yield* Ref.get(executions), [firstEffectId]);

Same setup as the naive fix, opposite assertion. So upstream chose not to reclaim, and the reason is not recorded in the code.

The likely reason is that reclaiming a running row means re-executing external side effects. A lease protects against a slow claimer, not only a dead one; if the owner is alive and merely slow, reclaiming runs the effect twice. Ownership guards prevent the stale claimant from settling the row afterwards, but they cannot undo work the stale claimant already started.

That is also why closed fork PR #5203 diagnosed this and it did not land.

The question worth answering

Should a running row be reclaimed when its owner is demonstrably dead (process gone, not merely past its lease), as distinct from a row whose owner is alive and slow?

If yes, the natural discriminator is liveness rather than lease age alone, and reconcileAfterProcessLoss is the existing hook. If no, the wedge is accepted and lease_expires_at is dead weight that should either be used or removed.

Secondary observation

EffectWorker.ts derives one worker id per process (orchestration-v2:${process.pid}) and reuses it for every claim, so lease_owner currently identifies a process, not a claim. Distinguishing claims would need a fresh token per claim, which is a prerequisite for any reclaim that must not race a live owner.

Environment

Found by static audit of the merged orchestrator-v2 work (#2829). Re-verified present at main. Not filed as a PR because the fix depends on the answer above.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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