KeeperHub / KeeperHub/keeperhub
A held idempotency key is never released once the reconciler proves the outcome was a definite failure
- 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
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