feat(reticulum): RMAP X of Y status and RNCP catch-up dedupe - #751
Merged
Conversation
Show publishing coverage as X of Y with amber for partial, check Network publish only when every eligible interface is discoverable, and lock the type matrix with tests and docs.
Dedupe enable-modal and dest-share side effects by LXMF message_hash, and rate-limit already-listening auto-share so periodic catch-up cannot spam peers.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds RMAP publishing coverage states, eligible-interface synchronization, and X-of-Y status display. It also adds RNCP/LXMF message deduplication, destination-share compatibility without pending requests, and per-peer auto-share cooldowns. ChangesRMAP publishing coverage
RNCP/LXMF side-effect deduplication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RNCPPeer
participant useReticulumRuntime
participant Deduplication
participant RncpEnableRequestModal
RNCPPeer->>useReticulumRuntime: deliver RNCP control message
useReticulumRuntime->>Deduplication: resolve and reserve message hash
Deduplication-->>useReticulumRuntime: accept first occurrence
useReticulumRuntime->>RncpEnableRequestModal: enqueue enable request
RncpEnableRequestModal->>Deduplication: reserve peer auto-share slot
Deduplication-->>RncpEnableRequestModal: allow one destination share
RncpEnableRequestModal->>RNCPPeer: send receive-destination share
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@src/renderer/components/ReticulumRmapConnectionStatus.test.tsx`:
- Around line 51-86: Add vitest-axe accessibility assertions to both partial and
full publishing tests in ReticulumRmapConnectionStatus: after rendering, hydrate
axe theme colors, obtain the rendered role="status" element, and assert await
axe(status).toHaveNoViolations() while preserving the existing text and class
assertions.
In `@src/renderer/lib/reticulum/reticulumRmapDiscovery.ts`:
- Around line 134-158: Update listReticulumRmapDiscoveryCapable, or the shared
filtering boundary immediately before readRmapAnyPublishing,
readRmapPublishState, and readRmapPublishPartial consume it, to exclude
system-managed rows while retaining user-managed RNode rows. Ensure
system-managed shared-instance rnode entries cannot become publish targets,
affect coverage, or drive synchronization, and add a regression case covering an
enabled system-managed rnode with a serial port.
In `@src/renderer/lib/rncpLxmfControlSideEffectDedup.ts`:
- Around line 60-69: Replace immediate deduplication consumption in
src/renderer/lib/rncpLxmfControlSideEffectDedup.ts:60-69 with reservation,
commit, and release operations that use reservation identity so an old attempt
cannot release a newer reservation. In
src/renderer/runtime/useReticulumRuntime.ts:602-618, release recoverable
destination-share failures and commit successful or terminal invalid-share
outcomes. In src/renderer/components/remote/RncpEnableRequestModal.tsx:120-137,
release the peer cooldown reservation when no destination-share LXMF message is
sent. In src/renderer/components/remote/RncpEnableRequestModal.test.tsx:118-141,
verify a failed initial auto-share permits a later retry for the same peer.
- Around line 76-87: Update tryConsumeRncpAlreadyEnabledAutoShareSlot to prune
entries whose timestamps are outside RNCP_REQUEST_ENABLE_COOLDOWN_MS before
inserting a new peer, then enforce a maximum retained-entry count by removing
older entries when necessary. Preserve the existing cooldown check and
successful slot-consumption behavior while ensuring alreadyEnabledShareAtByPeer
cannot grow without bound.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: c6826358-67c4-4ab4-a5d8-6720cb15a9d0
⛔ Files ignored due to path filters (16)
src/renderer/locales/cs/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/de/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/en/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/es/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/fr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/id/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/it/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ja/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ko/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/nl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pt-BR/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ru/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/tr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/uk/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/zh/translation.jsonis excluded by!src/renderer/locales/**
📒 Files selected for processing (15)
AGENTS.mddocs/reticulum.mddocs/troubleshooting.mdsrc/renderer/components/ReticulumRmapConnectionStatus.test.tsxsrc/renderer/components/ReticulumRmapConnectionStatus.tsxsrc/renderer/components/ReticulumRmapDiscoveryControls.test.tsxsrc/renderer/components/ReticulumRmapDiscoveryControls.tsxsrc/renderer/components/remote/RncpEnableRequestModal.test.tsxsrc/renderer/components/remote/RncpEnableRequestModal.tsxsrc/renderer/components/reticulum/ReticulumInterfacesPanel.test.tsxsrc/renderer/lib/reticulum/reticulumRmapDiscovery.test.tssrc/renderer/lib/reticulum/reticulumRmapDiscovery.tssrc/renderer/lib/rncpLxmfControlSideEffectDedup.test.tssrc/renderer/lib/rncpLxmfControlSideEffectDedup.tssrc/renderer/runtime/useReticulumRuntime.ts
Exclude system-managed RMAP targets, reserve/commit side-effect dedup on terminal outcomes, bound peer cooldown, and add axe coverage.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/runtime/useReticulumRuntime.ts (1)
594-630: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct runtime tests for the RNCP share reservation branch.
Cover commit for success and terminal reasons, and release for retryable failures. Existing tests cover the helpers and apply function separately, not this runtime decision.
🤖 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/renderer/runtime/useReticulumRuntime.ts` around lines 594 - 630, Add direct runtime tests around the RNCP receive-destination share handling branch in useReticulumRuntime, covering reservation commit when applyRncpReceiveDestShareFromLxmf returns ok, no_share, or invalid_sender, and reservation release for retryable failures. Verify the reservation outcome and preserve the existing duplicate/early-return behavior when tryReserveRncpLxmfControlHandled returns no reservation.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/renderer/runtime/useReticulumRuntime.ts`:
- Around line 594-630: Add direct runtime tests around the RNCP
receive-destination share handling branch in useReticulumRuntime, covering
reservation commit when applyRncpReceiveDestShareFromLxmf returns ok, no_share,
or invalid_sender, and reservation release for retryable failures. Verify the
reservation outcome and preserve the existing duplicate/early-return behavior
when tryReserveRncpLxmfControlHandled returns no reservation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: dd1e9f6f-1662-4572-a0b3-46b31afe48a9
📒 Files selected for processing (8)
src/renderer/components/ReticulumRmapConnectionStatus.test.tsxsrc/renderer/components/remote/RncpEnableRequestModal.test.tsxsrc/renderer/components/remote/RncpEnableRequestModal.tsxsrc/renderer/lib/reticulum/reticulumRmapDiscovery.test.tssrc/renderer/lib/reticulum/reticulumRmapDiscovery.tssrc/renderer/lib/rncpLxmfControlSideEffectDedup.test.tssrc/renderer/lib/rncpLxmfControlSideEffectDedup.tssrc/renderer/runtime/useReticulumRuntime.ts
Add useReticulumRuntime tests for terminal apply outcomes committing the control-hash reservation, upsert_failed releasing for retry, and early return when no reservation is available.
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
1 participant
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.
Summary
message_hash, and rate-limit already-listening auto-share so periodic catch-up cannot spam peers.Test plan
X of Y/ amber for partial coverage; full coverage is greenmessage_hashpnpm run test:runfor touched RMAP / RNCP testsSummary by CodeRabbit
New Features
Bug Fixes
Documentation