KeeperHub / KeeperHub/keeperhub

A held idempotency key is never released once the reconciler proves the outcome was a definite failure

Open
#2,373 5 comments 0 reactions 0 assignees View on GitHub
bug confirmed needs-discussion
Dominant language
TypeScript
Stars
24
Forks
93
Avg merge
1d 4h
Merged PRs (30d)
253

Description

## What happens

A held idempotency record is never revisited once the outcome it was waiting on becomes definite.

The route-level rule releases a key when the outcome is definite at request time and holds it when the outcome is unknown. An unreadable receipt is not definite then, so the record is finalized as held. The reconciler is the only thing that can later turn that unknown into a definite answer: it re-reads the receipt and settles the row to `completed` or `failed`.

It never touches idempotency. `lib/execute/reconcile-executions.ts` and `app/api/cron/execution-reconciler/route.ts` contain zero references to it. The only release path is `release()` in `lib/idempotency.ts`, called from route handlers at request time.

So when the reconciler settles a row to `failed` on a conclusive receipt, nothing landed, the outcome has become definite, and the key should be reusable on exactly the reasoning #1840 applies synchronously. Instead the record keeps replaying the stale `unconfirmed` body for the full `COMPLETED_TTL_MS` window.

## Why it is narrow

Severity is bounded and I do not want to oversell it. The held body carries `executionId` and `idempotentReplay: true`, so a caller can poll `GET /api/execute/{executionId}/status` and is not blind. The two 24-hour clocks, `COMPLETED_TTL_MS` and `DROPPED_AFTER_MS`, roughly coincide, so the window closes on its own. It is a liveness gap, not a correctness or double-spend one.

## The part that needs a decision, not code

Only a conclusive receipt should release. The reconciler reaches `failed` by three paths and the other two are not chain answers:

- `chain could not be resolved` means the receipt was never read.
- the dropped-after-24h path is a timeout inference about a transaction that could still be sitting in a mempool.

Releasing on either lets a retry broadcast a second transaction for work whose fate is genuinely unknown, which is the failure the key exists to prevent. A verified success must also keep its key, since a retry must not repeat an action that already took effect.

Two smaller things follow from it. `recordIdempotentResponse` writes `resourceId: null` on the held path, which discards the only link from an execution back to its record, so that has to be kept. And a release must be scoped to the owning organization and must never touch a `processing` row, which belongs to an in-flight request holding a fencing token.

## Related

Follows #1840 and #2020. It is deliberately not part of #2372: that PR ships and is correct with this absent, so per CONTRIBUTING.md they are separate.

I have this written and tested already (a standalone release scoped to held rows, wired to the reconciler's conclusive branch only, plus a `keysReleased` counter on the run summary and seven tests covering which verdicts release and which do not). Happy to open the PR once this is triaged, or to leave it if you would rather the reconciler stayed out of idempotency entirely.

Contributor guide

Open the contributing guide

Research direction

Read lib/idempotency.ts, lib/execute/reconcile-executions.ts, and app/api/cron/execution-reconciler/route.ts, then trace release() and the reconciler's failed verdict paths. Done means only a conclusive receipt releases the organization-scoped held record, resourceId is preserved, processing rows are untouched, and the reconciler tests cover release and non-release verdicts.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.