fix: announce before propagation sync and clarify establish stalls - #720
Merged
Conversation
Debounce an LXMF delivery announce before remote Sync so PNs can return LRPROOF, persist discovered PN pubkeys, and map NoLinkProof / watchdog races instead of a generic unreachable toast.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughPropagation sync now persists propagation identities, debounces LXMF announces before establishment, prevents stale run effects, reports ChangesPropagation synchronization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ReticulumPropagationStore
participant LiveBridge
participant lxmf_delivery
participant PropagationNode
ReticulumPropagationStore->>LiveBridge: start_propagation_sync
LiveBridge->>LiveBridge: rehydrate persisted PN identity
LiveBridge->>lxmf_delivery: ensure debounced LXMF announce
lxmf_delivery->>PropagationNode: send delivery announce
LiveBridge->>PropagationNode: establish pinned propagation link
PropagationNode-->>LiveBridge: link proof or NoLinkProof
LiveBridge-->>ReticulumPropagationStore: sync result or establish error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/stores/reticulumPropagationStore.ts (1)
246-268: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReset sync state when cancellation fails.
cancelSyncclears both watchdogs before awaitingproxyPost, but itscatchreturns without resettingsyncor recording the cancellation reason. A failed sidecar cancellation can therefore leavesync.activestuck attruewith no watchdog left to recover it.Run the same finalization logic on both success and failure paths while preserving any sidecar error already present.
Proposed fix
cancelSync: async (opts) => { + const finalizeCancel = () => { + set((state) => { + const fallback = opts?.reasonKey ?? 'reticulumPropagation.syncCancelled'; + const existing = state.lastSyncError; + const keepSidecar = + existing != null && + existing !== 'reticulumPropagation.syncCancelled' && + existing !== 'reticulumPropagation.syncTimedOut'; + return { + sync: { ...RETICULUM_PROPAGATION_SYNC_IDLE }, + lastSyncError: keepSidecar ? existing : fallback, + }; + }); + }; try { clearPropagationSyncStallWatchdog(); await window.electronAPI.reticulum.proxyPost('/api/v1/propagation/sync/cancel', {}); - set((state) => { - // existing finalization block - }); + finalizeCancel(); return true; } catch (e) { console.warn('[reticulumPropagationStore] cancel ' + errLikeToLogString(e)); + finalizeCancel(); return false; }🤖 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/stores/reticulumPropagationStore.ts` around lines 246 - 268, Update cancelSync so both successful and failed proxyPost attempts execute the same finalization that resets sync to RETICULUM_PROPAGATION_SYNC_IDLE and records the cancellation fallback while preserving an existing sidecar error. Keep the warning and false return for failures, but move or reuse the set logic so the catch path cannot leave sync.active stuck.
🤖 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 `@reticulum-sidecar/src/stack/live.rs`:
- Around line 1943-1952: Scope propagation identity pins and watchdog
cancellation to the individual sync run rather than the shared cancellation
flag. Update the sync emitter and its timeout handling around the pinning flow
in the surrounding live-sync implementation to use a per-run cancellation token
or generation, ensuring an older emitter cannot cancel a newer run or invoke
clear_propagation_identity_pins() for it. Add a regression test that triggers a
rapid re-sync and verifies the newer run’s pins remain intact.
In `@reticulum-sidecar/src/stack/mod.rs`:
- Around line 1045-1048: Update the add_propagation_node flow around
register_propagation_node_identity to carry the announce public_key from
discovery, or resolve it before persistence, instead of relying only on the
bounded outbound cache. Ensure pub_key remains available when the announce has
been evicted so the saved propagation node includes public_key and rehydration
can restore it.
In `@src/renderer/locales/ko/translation.json`:
- Around line 3942-3944: Update the syncEstablishNoLinkProof translation to
replace the malformed “네트워크 발표를 → 시도한 다음” recovery instruction with a
grammatical imperative directing users to announce on the network now, then
retry synchronization.
In `@src/renderer/locales/pt-BR/translation.json`:
- Line 3942: Update the syncEstablishNoLinkProof translation in
src/renderer/locales/pt-BR/translation.json at line 3942 and
src/renderer/locales/tr/translation.json at line 3942 so the arrow appears
between the two menu labels, rather than before “Anúncio de Rede” or its Turkish
equivalent; preserve the surrounding localized message.
In `@src/renderer/locales/uk/translation.json`:
- Line 3944: Update the syncEstablishNoLinkProof translation value to replace
the grammatically incorrect “вашій особи” with the correct Ukrainian wording,
such as “вашій ідентичності,” while preserving the rest of the message and its
meaning.
---
Outside diff comments:
In `@src/renderer/stores/reticulumPropagationStore.ts`:
- Around line 246-268: Update cancelSync so both successful and failed proxyPost
attempts execute the same finalization that resets sync to
RETICULUM_PROPAGATION_SYNC_IDLE and records the cancellation fallback while
preserving an existing sidecar error. Keep the warning and false return for
failures, but move or reuse the set logic so the catch path cannot leave
sync.active stuck.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 41023140-4ddd-4e30-9229-4684025024b3
📒 Files selected for processing (29)
docs/reticulum.mddocs/troubleshooting.mdreticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/lxmf_delivery.rsreticulum-sidecar/src/stack/mod.rsreticulum-sidecar/src/stack/persistence.rsreticulum-sidecar/src/stack/propagation_bridge.rsreticulum-sidecar/src/stack/types.rssrc/renderer/components/ReticulumPropagationSection.tsxsrc/renderer/lib/reticulum/reticulumPropagationSync.test.tssrc/renderer/lib/reticulum/reticulumPropagationSync.tssrc/renderer/locales/cs/translation.jsonsrc/renderer/locales/de/translation.jsonsrc/renderer/locales/en/translation.jsonsrc/renderer/locales/es/translation.jsonsrc/renderer/locales/fr/translation.jsonsrc/renderer/locales/id/translation.jsonsrc/renderer/locales/it/translation.jsonsrc/renderer/locales/ja/translation.jsonsrc/renderer/locales/ko/translation.jsonsrc/renderer/locales/nl/translation.jsonsrc/renderer/locales/pl/translation.jsonsrc/renderer/locales/pt-BR/translation.jsonsrc/renderer/locales/ru/translation.jsonsrc/renderer/locales/tr/translation.jsonsrc/renderer/locales/uk/translation.jsonsrc/renderer/locales/zh/translation.jsonsrc/renderer/stores/reticulumPropagationStore.test.tssrc/renderer/stores/reticulumPropagationStore.ts
Scope sync emitters per run so stale cancel/pin clears cannot race a new sync, persist discovered PN public keys through add, use <output> for the establish hint, and finalize cancelSync on proxy failure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@reticulum-sidecar/src/stack/propagation_bridge.rs`:
- Around line 174-183: Make generation validation and terminal side effects
atomic in the propagation bridge. Update the lifecycle around is_current,
clear_pins, and the related cancellation, pin cleanup, and event-send paths at
the referenced sections to use a shared lifecycle lock or re-check generation
while holding each affected resource lock, preventing an old emitter from acting
on a newer run. Preserve current-run behavior while rejecting stale side effects
under concurrent run replacement.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b42d9f1-6583-4c04-ac62-e56bb75cf9c3
📒 Files selected for processing (12)
reticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/mod.rsreticulum-sidecar/src/stack/propagation_bridge.rsreticulum-sidecar/src/stack/types.rssrc/renderer/components/ReticulumPropagationSection.tsxsrc/renderer/locales/ko/translation.jsonsrc/renderer/locales/pt-BR/translation.jsonsrc/renderer/locales/tr/translation.jsonsrc/renderer/locales/uk/translation.jsonsrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/stores/reticulumPropagationStore.test.tssrc/renderer/stores/reticulumPropagationStore.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/renderer/components/ReticulumPropagationSection.tsx
- src/renderer/stores/reticulumPropagationStore.test.ts
- reticulum-sidecar/src/stack/mod.rs
- src/renderer/locales/uk/translation.json
- reticulum-sidecar/src/stack/types.rs
- src/renderer/locales/tr/translation.json
- src/renderer/locales/pt-BR/translation.json
- src/renderer/locales/ko/translation.json
- reticulum-sidecar/src/stack/live.rs
Hold a shared lifecycle lock around run-id replacement and cancel/pin/event actions so a stale emitter cannot act on a newer sync run.
|
This was referenced Jul 28, 2026
This was referenced Aug 8, 2026
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
NoLinkProofinstead of generic unreachable; dual 60s watchdog no longer overwrites sidecar failure keys.Test plan
Summary by CodeRabbit