Conversation
Stuck Direct Completes left reticulum-pending-* twins beside the real hash after hydrate; delete optimistic rows on rekey and on save.
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe change removes optimistic Reticulum pending records when a real message hash is available. Cleanup now also applies while delivery status is ChangesReticulum message cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/ipc/reticulum-db-handlers.ts (2)
177-184: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake pending-row cleanup and message persistence atomic.
Lines 178-183 commit the pending-row deletion before the later lookup, update, or insert. If a later database operation fails, the catch path leaves the pending row deleted without saving its replacement.
Wrap the cleanup and the remaining save path in one SQLite transaction. Add a failure-path test that verifies the pending row remains when the replacement write fails.
As per path instructions, preserve SQLite integrity during hydration and rekeying, including failure handling where relevant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/reticulum-db-handlers.ts` around lines 177 - 184, Make the pending-row deletion and the subsequent message lookup, update, or insert in the handler containing the reticulum message save flow execute within one SQLite transaction, so any replacement-write failure rolls back the cleanup. Preserve existing hydration and rekeying integrity and error handling, and add a failure-path test that confirms the pending row remains when the replacement write fails.Source: Path instructions
177-183: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDelete only the pending row that the real hash replaces.
Line 177 deletes every pending row with the same identity, sender, payload, and a timestamp within 60 seconds. Two distinct sends can satisfy these fields. Finalizing one send can then delete the other send's persisted
sendingstate. A restart before the second send completes cannot hydrate or retry it.Pass an explicit prior pending ID through the save contract. Delete only that exact row. Add a regression test with two identical outbound payloads in the same time window.
As per path instructions, preserve SQLite integrity during hydration and rekeying.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/reticulum-db-handlers.ts` around lines 177 - 183, Update the save/finalization contract around the message persistence handler to carry the exact prior pending message ID, and use that ID in the DELETE query instead of matching identity, sender, payload, and timestamp. Ensure rekeying removes only the pending row replaced by the real hash, preserves SQLite integrity during hydration and rekeying, and add a regression test covering two identical outbound payloads within the same time window.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/ipc/reticulum-db-handlers.ts`:
- Around line 177-184: Make the pending-row deletion and the subsequent message
lookup, update, or insert in the handler containing the reticulum message save
flow execute within one SQLite transaction, so any replacement-write failure
rolls back the cleanup. Preserve existing hydration and rekeying integrity and
error handling, and add a failure-path test that confirms the pending row
remains when the replacement write fails.
- Around line 177-183: Update the save/finalization contract around the message
persistence handler to carry the exact prior pending message ID, and use that ID
in the DELETE query instead of matching identity, sender, payload, and
timestamp. Ensure rekeying removes only the pending row replaced by the real
hash, preserves SQLite integrity during hydration and rekeying, and add a
regression test covering two identical outbound payloads within the same time
window.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 85f718dd-e92f-48d7-96f0-8c28763aef9d
📒 Files selected for processing (4)
src/main/ipc/reticulum-db-handlers.test.tssrc/main/ipc/reticulum-db-handlers.tssrc/renderer/lib/reticulum/reticulumOutboundRetry.test.tssrc/renderer/lib/reticulum/reticulumOutboundRetry.ts
Direct spawn of esbuild/bin/esbuild fails on win32 where postinstall leaves a Node shim; use esbuild.build so dist:win embeds build info.
rinchen
changed the title
Carry replaces_message_hash into saveReticulumMessage and delete+upsert in one transaction so twin payloads stay intact and insert failures roll back pending cleanup.
1 participant
Summary
reticulum-pending-*SQLite rows when LXMF send rekeys to a real message hash, including while delivery is stillsending(also cleans twins indb:saveReticulumMessage). Seen in Runr’s AIBot developer bundles as duplicate stuck-Sending rows after hydrate.dist:winpackaging:#798spawnedesbuild/bin/esbuilddirectly, but on win32 postinstall leaves that path as a Node shim sobuild:main:prodexits 1 with no output. Switchesbuild-main-build.mjsto the esbuild JS API soMESH_CLIENT_BUILD_INFOstill embeds correctly.Test plan
reticulumOutboundRetry.test.ts— pending→hash deletes; empty ids skippedreticulum-db-handlers.test.ts— pending twin dropped while stillsendingesbuild-main-build.test.mjs— JS API contract (nospawnSyncof bin shim)MESH_CLIENT_BUILD_INFO=… pnpm run build:main:prodembeds channel stampdist:wingreen after merge