fix(orb): guard finalizeRelayFailureRetryRow DB writes against duplicate redelivery (#8332)#8441
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (85.71%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8441 +/- ##
==========================================
- Coverage 92.42% 89.64% -2.79%
==========================================
Files 791 98 -693
Lines 79294 22847 -56447
Branches 23950 3912 -20038
==========================================
- Hits 73291 20481 -52810
+ Misses 4866 2187 -2679
+ Partials 1137 179 -958
Flags with carried forward coverage won't be shown. Click here to find out more.
|
4e8a5c1 to
032fc1b
Compare
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-24 14:04:27 UTC
Review summary Nits — 4 non-blocking
CI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
src/orb/relay.ts'sretryFailedRelaysdoc comment promises "Never throws." Its per-row finalize step,finalizeRelayFailureRetryRow, issued itsDELETE/UPDATEonorb_relay_failureswith no try/catch — a transient D1 write failure right after a successful forward would reject out of thePromise.allbatch, breaking that contract, and would leave the failure row un-deleted despite the event already having been forwarded (risking duplicate redelivery on the next retry tick). Wrapped both writes in a try/catch that logs an alertable structured error (delivery_id,event_name,outcome) and swallows, matching this file's existingconsole.error(JSON.stringify({...}))logging convention used elsewhere in the same file.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Closes #8332
Validation
git diff --checknpm run typechecknpx vitest run test/integration/orb-relay.test.ts --coverage --coverage.include="src/orb/relay.ts"— all 85 tests pass; the two new/changed lines infinalizeRelayFailureRetryRoware fully exercised by two new regression tests (DELETE-path and UPDATE-path DB-write failure)npm run actionlintnpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/orb/relay.tsand its integration test (no UI/MCP/worker/OpenAPI surface touched), so the UI/MCP/workers/OpenAPI-specific checks above were not run locally; they are unaffected by this diff and are still exercised by the full CI gate.Safety
UI Evidencesection below. (N/A — no visible UI change.)UI Evidence
N/A — this is a backend cron/retry-path change with no visible UI surface.
Notes
finalizeRelayFailureRetryRow's two DB writes are guarded; no other logic inretryFailedRelaysor elsewhere in the file was touched, per the issue's own scope note.