fix(backend): restore idempotency store with dry-run safe retention sweep (#1375) - #1491
Open
solaawojobi00-bit wants to merge 1 commit into
Open
solaawojobi00-bit wants to merge 1 commit into
solaawojobi00-bit wants to merge 1 commit into
Conversation
…weep (Junirezz#1375) Commit 5db5f6e replaced idempotency.ts with the Prisma-backed helpers and dropped the Redis/NodeCache IdempotencyStore, leaving pruneStaleIdempotencyRecords, vault endpoints and the transfer orchestrator importing symbols that no longer existed. - Restore IdempotencyStore (localCache, pruneStaleKeys) alongside the Prisma helpers; pruneStaleKeys reports localPruned/redisPruned and a dryRun flag and never mutates state in dry-run mode. - pruneStaleIdempotencyRecords returns the per-backend breakdown, logs dry-run results and leaves sweep metrics untouched on dry runs. - Add dry-run regression tests for local cache, Redis and the sweep. Also fixes pre-existing upstream build breakage blocking backend CI: - schema.prisma: close WalletTenantAssociation model - apiContractSnapshots.ts: close unterminated block, fix `in`/`??` precedence - vaultEndpoints.ts: import readsLimiter, order receipts by timestamp, fix missing return path in strategy handler - tracing.ts: use Resource from the locked @opentelemetry/resources 1.x - schemaSnapshot.ts: use Zod v4 `shape` - swagger.ts: remove duplicate `checks` key - tests: fix mock Request.get typings
|
@solaawojobi00-bit Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(backend): restore idempotency store with a dry-run safe retention sweep (#1375)
Problem
pruneStaleIdempotencyRecordsdelegates toidempotencyStore.pruneStaleKeys(retentionMs, dryRun), but commit5db5f6e0replacedbackend/src/idempotency.tswith the Prisma-backed helpers and deleted the Redis/NodeCacheIdempotencyStore. That removedlocalCache,pruneStaleKeysand itslocalPrunedfield. Nine files still import it (idempotencyRetention.ts,vaultEndpoints.ts,transferOrchestrator.ts,index.ts, and five test suites), so there was no safe way to rehearse a retention sweep in production.POST /admin/idempotency/retention/cleanup?dryRun=trueTypeError: Cannot read properties of undefined (reading 'pruneStaleKeys'){ pruned, localPruned, redisPruned, dryRun: true }; nothing is deletedlocalPrunedand still present afterwardsredisPrunedand still in Redis afterwardslastSweepAt,totalPruned,lastPrunedCountand eviction counter unchangednew IdempotencyStore()in testsIdempotencyStore is not a constructorSeparately, backend CI (Backend Governance / Test Coverage) could not get past
tscorprisma generateonmainbecause of unrelated breakage. The build-level parts are fixed here as well (see Changes).Solution
Restore the store next to the new Prisma helpers, without changing those helpers, and make the dry-run contract explicit:
pruneStaleKeys(retentionMs, dryRun)reads the local cache and Redis (SCAN/GET/TTLonly) and counts stale entries. It deletes entries and bumps evictions only whendryRunis false. It returns{ pruned, localPruned, redisPruned, dryRun }.pruneStaleIdempotencyRecords(dryRun)returns the same breakdown, logs a "dry-run completed" line withwouldPrune, and updates the sweep metrics only for live runs.Changes
backend/src/idempotency.ts: restoresIdempotencyStore,idempotencyStore,IdempotencyConflictError,IdempotentOperationResult,buildIdempotencyFingerprint,getIdempotencyHashThreshold, the privatelocalCache: NodeCacheandpruneStaleKeys, taken from the last version before5db5f6e0. The store's metadata type is renamedIdempotencyStoreKeyMetadataso it doesn't clash with the new PrismaIdempotencyKeyMetadata.backend/src/idempotencyRetention.ts: newIdempotencyRetentionSweepResulttype. A dry run logs what it would prune and doesn't touchretentionState; the result includes the per-backend counts. The admin endpoint spreads the result, so it gets these fields without changes.backend/src/__tests__/idempotencyRetention.test.ts: therateLimitermock can now inject an in-memory Redis fake; four dry-run regression tests added.Pre-existing build fixes (one or two lines each):
prisma/schema.prismaWalletTenantAssociationmodelprisma validate/generatefailed (P1012)src/apiContractSnapshots.tsifblock;key in (x ?? {})tsc; precedence bugsrc/vaultEndpoints.tsreadsLimiter; order/receiptsbytimestamp; add a return path in the/strategycooldown branchReferenceError: readsLimiter is not definedat module load;Transactionhas nocreatedAt; TS7030src/tracing.tsnew Resource(...)instead ofresourceFromAttributes@opentelemetry/resourcesto 1.30.1, which has noresourceFromAttributessrc/schemaSnapshot.ts.shapeinstead of._shapesrc/swagger.tscheckskeysrc/tests/idempotency.test.ts,src/tests/tenantBoundary.test.tsRequest.getoverload typingRegression Tests
localCacheandpruneStaleKeysreturnslocalPrunedprunes stale local idempotency keysstore dry-run reports stale local keys without deleting themstore dry-run reports stale Redis keys without deleting themsweep dry-run leaves the store and sweep metrics untoucheda live sweep after a dry-run prunes the same keys and records metricssupports dry-run retention sweepsTesting
Full backend suite (
npx jest --runInBand), upstreammainvs this branch:No suite that passes on
mainfails on this branch. More tests run because suites that previously failed to load (missing store exports,readsLimiter, the syntax error) now execute.npx tsc --noEmit: 1 blocking syntax error onmain(which hid the rest) → 15 remaining errors, all in the two pre-existing areas listed below.eslinton the changed files: 0 errors.Notes for Reviewers
CI will not be fully green, for pre-existing reasons outside #1375. Backend Governance has failed on
mainsince 2026-08-25, and Test Coverage has no successful run. This PR clears the build-level blockers; these remain and need their own issues:src/sessionAudit.ts(13 TS errors): usesprisma.sessionAuditLog, but noSessionAuditLogmodel or migration was ever added. The file isn't imported anywhere.src/middleware/tenantBoundary.ts(2 TS errors): filtersTransactionbydeletedAtandWebhookEndpointbytenantId; neither column exists. This is a tenant-isolation check, so it needs a schema and migration decision, not a code workaround.main.Overlap: #1486 also restores the idempotency store (for #1318/#1319/#1322). Whichever merges second will conflict in
backend/src/idempotency.ts. This PR keeps the original store implementation and export names, so the resolution should be mechanical.Risk: Low–Medium. It restores previously shipped behavior that vault endpoints and the transfer orchestrator rely on; the Prisma helpers are unchanged. There's no database migration: the schema fix only adds a missing
}.Rollback: a clean
git revert, with no persistent state involved.Closes #1375