Conversation
Wire sibling lrgp-rs into the sidecar with dedicated games IPC, a Reticulum Games panel, and Ratspeak parity tracking so mesh-client peers can play over LRGP.
|
Warning Review limit reached
Next review available in: 34 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 ignored due to path filters (16)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughAdded LRGP Tic-Tac-Toe and Chess support for Reticulum. The change covers the lrgp-rs stack, sidecar session handling, REST and IPC APIs, WebSocket events, capability-gated UI, game boards, challenge actions, persistence, and tests. ChangesLRGP Games integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant GamesPanel
participant ElectronIPC
participant ReticulumSidecar
participant GamesSessionManager
participant LXMF
Operator->>GamesPanel: Select game or challenge peer
GamesPanel->>ElectronIPC: Send games action
ElectronIPC->>ReticulumSidecar: Validate dedicated games request
ReticulumSidecar->>GamesSessionManager: Prepare and route action
GamesSessionManager->>LXMF: Sign and send LRGP envelope
LXMF-->>GamesSessionManager: Deliver message
GamesSessionManager-->>ElectronIPC: Emit games.update or games.action_result
ElectronIPC-->>GamesPanel: Update session state
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)
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.
Actionable comments posted: 5
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (11)
docs/ci-cd.md-156-158 (1)
156-158: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the earlier CI sibling list.
The new text lists
rsLXSTandlrgp-rs, but the sidecar CI section at Line 79 still lists onlyrsReticulum,rsLXMF, andrsNomad. Update that section to list all five siblings and theirRS_*_REFoverrides. Otherwise, the CI and local-parity instructions conflict.🤖 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 `@docs/ci-cd.md` around lines 156 - 158, Update the sidecar CI sibling list near the existing rsReticulum/rsLXMF/rsNomad entries to include all five repositories: rsReticulum, rsLXMF, rsNomad, rsLXST, and lrgp-rs. Add the corresponding RS_*_REF override documentation for rsLXST and lrgp-rs, matching the established format so the CI and local-parity instructions agree.scripts/clone-ratspeak-stack.test.mjs-145-156 (1)
145-156: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExercise the LRGP pin wiring.
The test name includes optional
RS_LRGP_REFpin support, but the test only checks that the variable appears in the script. It does not verify that thelrgp-rsensure_repocall passes this variable. Add a source-contract assertion for the complete call, or execute the LRGP pin path with a local tag fixture.Suggested source-contract assertion
+ expect(cloneScript).toContain( + 'ensure_repo "${LRGP_DIR}" \'https://github.com/ratspeak/lrgp-rs.git\' "${RS_LRGP_REF}" \'lrgp-rs\'', + );As per path instructions, use source-contract tests where runtime integration is impractical.
🤖 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 `@scripts/clone-ratspeak-stack.test.mjs` around lines 145 - 156, Extend the test named “clones lrgp-rs with optional RS_LRGP_REF pin support” to verify that the lrgp-rs ensure_repo invocation passes the RS_LRGP_REF variable, asserting the complete call in cloneScript rather than only checking the variable declaration. Keep the existing local remote behavior and assertions unchanged.Source: Path instructions
src/renderer/components/games/TicTacToeBoard.tsx-40-41 (1)
40-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a fallback for unknown session statuses.
Line 41 builds the translation key from
session.status.GameSession.statusis typed asstring, and src/shared/games-types.ts Line 100 documents that the sidecar may add new values. If the sidecar reports a status outsideGamesSessionStatus, no matching key exists and i18next renders the raw keygamesPanel.status.<value>in the board.Pass a
defaultValueso an unknown status shows readable text.🌐 Proposed fix for the dynamic status key
} else if (!isActive) { - statusText = t(`gamesPanel.status.${session.status}`); + statusText = t(`gamesPanel.status.${session.status}`, { + defaultValue: t('gamesPanel.status.unknown'), + }); } else if (isMyTurn) {🤖 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/components/games/TicTacToeBoard.tsx` around lines 40 - 41, Update the statusText assignment in TicTacToeBoard’s inactive-session branch to provide i18next’s defaultValue when translating the dynamic session.status key. Use a readable fallback for statuses not present in the gamesPanel.status translations while preserving existing translations for known statuses.src/shared/games-types.ts-138-142 (1)
138-142: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNormalize query strings before the games API gate.
assertProxyApiPathreturnsapiPathunchanged, so/api/v1/games?peer=xbypasses the generic proxy deny gate and uses the shared 300/min proxy bucket instead ofreticulum:games*. Apply an equivalent query-stripping/normalization before comparing against the/api/v1/gamesbase in allreticulum:proxyGet/Post/Deletepaths.🤖 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/shared/games-types.ts` around lines 138 - 142, The games API path check in isGamesApiPath must normalize or strip the query string before comparing against the base path and GAMES_API_PREFIX. Apply this normalized path consistently in the reticulum:proxyGet, reticulum:proxyPost, and reticulum:proxyDelete gating flows so query-bearing games URLs use the reticulum:games* bucket.src/main/ipc/reticulum-handlers.ts-386-424 (1)
386-424: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReturn a consistent invalid-session-id envelope from the games session handlers.
reticulum:gamesResend,reticulum:gamesMarkRead,reticulum:gamesDeleteSession, andreticulum:gamesSessionDetailthrowinvalid_session_idviaassertGamesSessionId, whilereticulum:gamesActionresolves{ ok: false, error }. These IPC promises can reject for malformed input; return the same non-rejection error result across the games channels.🤖 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-handlers.ts` around lines 386 - 424, Update the games session handlers `reticulum:gamesResend`, `reticulum:gamesMarkRead`, `reticulum:gamesDeleteSession`, and `reticulum:gamesSessionDetail` so `assertGamesSessionId` failures return the same `{ ok: false, error }` envelope used by `reticulum:gamesAction` instead of rejecting the IPC promise. Handle validation before constructing the request path, while preserving existing proxy failure handling for valid session IDs.reticulum-sidecar/src/stack/live.rs-3128-3137 (1)
3128-3137: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe resend contract does not match when the envelope is cached.
The doc comment states that resend applies "after a transient send failure".
GamesSessionManager::commit_actionis the only writer oflast_envelope, and it runs only after a successful send.send_game_actioncallsrollback_actionon send failure, which does not cache the envelope. So after a failed send,prepare_resendreturnsno_previous_action.Either cache the envelope bytes before the send attempt, or correct the comment to state that resend applies only to the last successfully sent action.
🤖 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 `@reticulum-sidecar/src/stack/live.rs` around lines 3128 - 3137, The documentation for resend_last_game_action promises retrying transient send failures, but last_envelope is only cached after successful sends. Either update the send/rollback flow so the envelope is cached before the send attempt and remains available after failure, or revise the resend_last_game_action doc comment to specify that it only resends the last successfully sent action; keep prepare_resend behavior consistent with the chosen contract.src/renderer/components/GamesPanel.tsx-161-167 (1)
161-167: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslation keys are built from open-ended sidecar strings without a fallback.
GameSession.statusis typed asstringinsrc/shared/games-types.tsbecause the sidecar can add new values, and FEN piece characters are equally unconstrained. When a value has no matching key, i18next renders the raw key path in the UI or in an accessible name, for examplegamesPanel.status.cancelled. Pass adefaultValueat each interpolation site.
src/renderer/components/GamesPanel.tsx#L161-L167: add{ defaultValue: session.app_id }togamesPanel.apps.${session.app_id}and{ defaultValue: session.status }togamesPanel.status.${session.status}.src/renderer/components/games/ChessBoard.tsx#L92-L92: add{ defaultValue: session.status }togamesPanel.status.${session.status}.src/renderer/components/games/ChessBoard.tsx#L147-L152: add{ defaultValue: piece }togamesPanel.chess.pieceNames.${piece}, so an unexpected FEN character does not leak a key path into the square's accessible name.🤖 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/components/GamesPanel.tsx` around lines 161 - 167, Add i18next defaultValue fallbacks for all open-ended translation interpolations: in src/renderer/components/GamesPanel.tsx lines 161-167, use session.app_id for gamesPanel.apps and session.status for gamesPanel.status; in src/renderer/components/games/ChessBoard.tsx line 92, use session.status for gamesPanel.status; and in lines 147-152, use piece for gamesPanel.chess.pieceNames. Preserve the existing translation keys and rendering behavior when keys exist.src/renderer/components/GamesPanel.tsx-340-348 (1)
340-348: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winConfirm the destructive delete action.
deleteGamesSessionpermanently removes the session and its history. The resign action in the same component already usesConfirmModal, but delete runs on a single click next to the resign and resend controls. A misclick loses the session with no undo.Reuse the existing
ConfirmModalpattern for delete.🛡️ Proposed confirmation for the delete action
- const [confirmResign, setConfirmResign] = useState(false); + const [confirmResign, setConfirmResign] = useState(false); + const [confirmDelete, setConfirmDelete] = useState(false);<button type="button" className="rounded bg-amber-950/60 px-3 py-1 text-xs font-medium text-amber-200/70 disabled:opacity-50" aria-label={t('gamesPanel.deleteSessionAria')} disabled={actionBusy} - onClick={() => void deleteGamesSession(selectedSession.session_id)} + onClick={() => { + setConfirmDelete(true); + }} > {t('gamesPanel.deleteSession')} </button>+ {confirmDelete && selectedSession && ( + <ConfirmModal + title={t('gamesPanel.deleteConfirmTitle')} + message={t('gamesPanel.deleteConfirmMessage')} + confirmLabel={t('gamesPanel.deleteSession')} + danger + onCancel={() => { + setConfirmDelete(false); + }} + onConfirm={() => { + setConfirmDelete(false); + void deleteGamesSession(selectedSession.session_id); + }} + /> + )} </div>Add
gamesPanel.deleteConfirmTitleandgamesPanel.deleteConfirmMessageto every locale file that definesgamesPanel.resignConfirmTitle.🤖 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/components/GamesPanel.tsx` around lines 340 - 348, Update the delete flow around the GamesPanel delete button and deleteGamesSession to require confirmation before permanently removing a session. Reuse the existing ConfirmModal pattern used by the resign action, wiring its confirmation callback to deleteGamesSession and preserving actionBusy behavior. Add gamesPanel.deleteConfirmTitle and gamesPanel.deleteConfirmMessage to every locale that defines gamesPanel.resignConfirmTitle.src/renderer/components/GamesPanel.test.tsx-50-143 (1)
50-143: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd coverage for the draw-offer branch and the resend button.
GamesPanel.test.tsxdoes not cover either conditional path inGamesPanel.tsx:
draw_offered: trueonly exercisesGAMES_CMD.DRAW_OFFER; coverGAMES_CMD.DRAW_ACCEPT/GAMES_CMD.DRAW_DECLINEby selecting a session with that metadata.- The resend button appears only when the selected session has a failed
lastActionResult.session_id; cover that by sending an action that resolves{ ok: false, session_id: 's1' }.🤖 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/components/GamesPanel.test.tsx` around lines 50 - 143, Extend the GamesPanel tests to cover both missing conditional paths: select a session with draw_offered metadata and verify the draw accept/decline actions invoke sendAction with the expected commands, then configure sendAction to resolve { ok: false, session_id: 's1' } for an action and verify the selected session’s resend button appears and triggers the resend behavior. Reuse renderAndSelectSession and the existing session/action helpers.Source: Coding guidelines
src/renderer/lib/reticulum/clearReticulumSessionStores.test.ts-47-51 (1)
47-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSeed a session before asserting that cleanup removes sessions.
Line 48 selects an ID but does not add a session. The length assertion passes even if
clear()stops clearingsessions.Insert a valid
s1session before callingclearReticulumSessionStores().As per path instructions,
**/*.test.ts: Prefer behavioral assertions; skip style-only test nits.`🤖 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/lib/reticulum/clearReticulumSessionStores.test.ts` around lines 47 - 51, Update the “clears the games store” test to seed a valid s1 session in useReticulumGamesStore before calling clearReticulumSessionStores(). Keep the existing selectedSessionId and sessions length assertions so the test verifies cleanup removes the seeded session.Source: Path instructions
src/renderer/components/reticulum/ReticulumGameChallengeButton.tsx-39-41 (1)
39-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent an open challenge menu from sending after disable.
The trigger is disabled at Line 55. If the Reticulum stack stops after the menu opens, the option buttons remain active and can still call
sendGamesChallenge().
src/renderer/components/reticulum/ReticulumGameChallengeButton.tsx#L39-L41: guardhandleChallenge()whendisabledis true.src/renderer/components/reticulum/ReticulumGameChallengeButton.tsx#L66-L80: close or hide the menu when disabled, and disable app option buttons.src/renderer/components/reticulum/ReticulumGameChallengeButton.test.tsx#L40-L43: open the menu, rerender withdisabled, then verify that no option can dispatchsendAction.Proposed fix
useEffect(() => { if (!menuOpen) return; @@ }, [menuOpen]); + useEffect(() => { + if (disabled && menuOpen) setMenuOpen(false); + }, [disabled, menuOpen]); + async function handleChallenge(appId: GamesAppId) { + if (disabled) return; setMenuOpen(false); @@ key={appId} type="button" + disabled={disabled}As per coding guidelines,
Behavioral changes must ship with a passing test.🤖 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/components/reticulum/ReticulumGameChallengeButton.tsx` around lines 39 - 41, The open challenge menu can still dispatch a game challenge after the control becomes disabled. In src/renderer/components/reticulum/ReticulumGameChallengeButton.tsx#L39-L41, guard handleChallenge so disabled prevents sending; at `#L66-L80`, close or hide the menu when disabled and disable its app option buttons. In src/renderer/components/reticulum/ReticulumGameChallengeButton.test.tsx#L40-L43, open the menu, rerender with disabled, and verify selecting an option does not dispatch sendAction.Source: Coding guidelines
🧹 Nitpick comments (3)
reticulum-sidecar/src/stack/games_session.rs (1)
58-59: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the
last_enveloperesend cache.
commit_actioninserts one envelope persession_idand never evicts. Entries are removed only bydelete_session. Sessions accumulate for the process lifetime, so the map and its envelope bytes grow without a limit. Add a cap or evict entries for sessions that the store no longer holds.Also applies to: 437-444
🤖 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 `@reticulum-sidecar/src/stack/games_session.rs` around lines 58 - 59, Bound the resend cache used by commit_action by evicting entries for session IDs no longer retained by the store, or by enforcing a fixed capacity with eviction. Update last_envelope and the associated insertion path so stale entries cannot accumulate for the process lifetime, while preserving resend behavior for active sessions and delete_session cleanup.src/main/ipc/reticulum-proxy-rate-limit.contract.test.ts (1)
57-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTie the rate-limit assertion to the games limiter.
toMatch(/max:\s*600/)matches anymax: 600inreticulum-handlers.ts. The test passes even if the games limiter uses a different ceiling. Assert the label and the max in one match so the 600 ceiling is bound toreticulum:games.The negative proxy assertions match only a single-quoted literal path on the same call. A template literal or a variable would evade them. Consider asserting that no proxy invocation mentions
/api/v1/gamesanywhere in the preload source.♻️ Proposed tightening
- expect(HANDLERS_SOURCE).toContain("label: 'reticulum:games'"); - expect(HANDLERS_SOURCE).toMatch(/max:\s*600/); + expect(HANDLERS_SOURCE).toMatch(/label:\s*'reticulum:games'[\s\S]{0,200}?max:\s*600/); @@ - expect(preload).not.toMatch(/invoke\('reticulum:proxyGet',\s*'\/api\/v1\/games/); - expect(preload).not.toMatch(/invoke\('reticulum:proxyPost',\s*'\/api\/v1\/games/); + expect(preload).not.toMatch(/invoke\(\s*'reticulum:proxy(Get|Post)'[\s\S]{0,80}?\/api\/v1\/games/);🤖 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-proxy-rate-limit.contract.test.ts` around lines 57 - 68, Strengthen the contract assertions in the test around HANDLERS_SOURCE by matching the reticulum:games limiter label and max: 600 in one pattern, ensuring the ceiling is tied to that limiter. Broaden both preload negative assertions to reject any reticulum:proxyGet or reticulum:proxyPost invocation whose source contains /api/v1/games, regardless of whether the path uses a literal, template, or variable.src/renderer/components/games/ChessBoard.test.tsx (1)
43-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the flipped board and the promotion move.
The suite covers the unflipped white path. Two branches introduced in
ChessBoard.tsxhave no test:
flipped(my_color: 'b') reverses rows and columns and remapsactualRankIdx/actualFileIdxat lines 103-104 and 135-136. An index inversion error here is silent.- Promotion at lines 118-120 appends
qto the UCI string. No test asserts thee7e8qform.🧪 Proposed additional tests
it('maps clicks to the correct squares when the board is flipped for black', async () => { const onMove = vi.fn(); render( <ChessBoard session={makeSession({ metadata: { ...makeSession().metadata, my_color: 'b', turn: 'me' }, })} onMove={onMove} />, ); await userEvent.click(screen.getByRole('button', { name: /^e7,/ })); await userEvent.click(screen.getByRole('button', { name: /^e5,/ })); expect(onMove).toHaveBeenCalledWith('e7e5'); }); it('appends the queen promotion suffix when a pawn reaches the last rank', async () => { const onMove = vi.fn(); render( <ChessBoard session={makeSession({ metadata: { ...makeSession().metadata, fen: '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1', }, })} onMove={onMove} />, ); await userEvent.click(screen.getByRole('button', { name: /^e7,/ })); await userEvent.click(screen.getByRole('button', { name: /^e8,/ })); expect(onMove).toHaveBeenCalledWith('e7e8q'); });🤖 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/components/games/ChessBoard.test.tsx` around lines 43 - 152, Add tests in the ChessBoard suite covering both untested branches: render a black-turn session and verify clicks map to e7e5 on the flipped board, then render a promotion position and verify e7 to e8 emits the UCI move e7e8q. Reuse the existing makeSession and onMove patterns without changing production behavior.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.
Inline comments:
In `@reticulum-sidecar/src/api/games.rs`:
- Around line 55-61: Extend input validation in games_action to call
reject_oversize for app_id, command, and session_id in addition to dest_hash,
and apply the same bound to the id path parameter in games_session_detail,
games_session_resend, games_session_read, and games_session_delete. Ensure each
oversized external game/session/action input returns the existing structured
rejection response before persistence, caching, or further processing.
In `@reticulum-sidecar/src/stack/live.rs`:
- Around line 328-335: Move the synchronous SQLite work triggered by
games_session_cb.handle_inbound_lxmf out of the router delivery callback
registered in the live stack. Preserve the existing inbound filtering and
message handling, but dispatch the persistence path asynchronously or otherwise
defer it until after deliver_unpacked_lxmf releases router.lock().await,
ensuring persist_session_from_state never runs while the callback holds the
router lock.
In `@src/renderer/components/games/TicTacToeBoard.tsx`:
- Around line 62-79: Update the cell aria-label in TicTacToeBoard’s cells.map
rendering to include both the localized cell position and whether the cell is
empty or occupied, using new corresponding empty and occupied translation keys
in every locale. Preserve the existing index numbering and update
TicTacToeBoard.test.tsx queries to match the expanded accessible names.
In `@src/renderer/lib/reticulum/reticulumGamesSession.ts`:
- Around line 114-121: Update markGamesSessionRead to capture the session’s
revision before awaiting window.electronAPI.reticulum.games.markRead, then clear
unread only when the session still exists and that revision remains current
after the IPC call. Preserve newer games.update changes, and add a test covering
an update received during markRead.
In `@src/renderer/stores/reticulumGamesStore.ts`:
- Around line 11-18: Validate complete external game contracts before mutating
state: update isGameSession/asGameSession in
src/renderer/stores/reticulumGamesStore.ts:11-18 to require every GameSession
field, including finite timestamps and unread values; reject incomplete rows at
src/renderer/stores/reticulumGamesStore.ts:53-55, validate
GamesUpdateEventPayload before reading its session at :80-100, and validate
complete app manifests before storing them at :103-110. Add malformed
valid-session-id fixtures covering missing required fields in
src/renderer/stores/reticulumGamesStore.test.ts:28-32.
---
Other comments:
In `@docs/ci-cd.md`:
- Around line 156-158: Update the sidecar CI sibling list near the existing
rsReticulum/rsLXMF/rsNomad entries to include all five repositories:
rsReticulum, rsLXMF, rsNomad, rsLXST, and lrgp-rs. Add the corresponding
RS_*_REF override documentation for rsLXST and lrgp-rs, matching the established
format so the CI and local-parity instructions agree.
In `@reticulum-sidecar/src/stack/live.rs`:
- Around line 3128-3137: The documentation for resend_last_game_action promises
retrying transient send failures, but last_envelope is only cached after
successful sends. Either update the send/rollback flow so the envelope is cached
before the send attempt and remains available after failure, or revise the
resend_last_game_action doc comment to specify that it only resends the last
successfully sent action; keep prepare_resend behavior consistent with the
chosen contract.
In `@scripts/clone-ratspeak-stack.test.mjs`:
- Around line 145-156: Extend the test named “clones lrgp-rs with optional
RS_LRGP_REF pin support” to verify that the lrgp-rs ensure_repo invocation
passes the RS_LRGP_REF variable, asserting the complete call in cloneScript
rather than only checking the variable declaration. Keep the existing local
remote behavior and assertions unchanged.
In `@src/main/ipc/reticulum-handlers.ts`:
- Around line 386-424: Update the games session handlers
`reticulum:gamesResend`, `reticulum:gamesMarkRead`,
`reticulum:gamesDeleteSession`, and `reticulum:gamesSessionDetail` so
`assertGamesSessionId` failures return the same `{ ok: false, error }` envelope
used by `reticulum:gamesAction` instead of rejecting the IPC promise. Handle
validation before constructing the request path, while preserving existing proxy
failure handling for valid session IDs.
In `@src/renderer/components/games/TicTacToeBoard.tsx`:
- Around line 40-41: Update the statusText assignment in TicTacToeBoard’s
inactive-session branch to provide i18next’s defaultValue when translating the
dynamic session.status key. Use a readable fallback for statuses not present in
the gamesPanel.status translations while preserving existing translations for
known statuses.
In `@src/renderer/components/GamesPanel.test.tsx`:
- Around line 50-143: Extend the GamesPanel tests to cover both missing
conditional paths: select a session with draw_offered metadata and verify the
draw accept/decline actions invoke sendAction with the expected commands, then
configure sendAction to resolve { ok: false, session_id: 's1' } for an action
and verify the selected session’s resend button appears and triggers the resend
behavior. Reuse renderAndSelectSession and the existing session/action helpers.
In `@src/renderer/components/GamesPanel.tsx`:
- Around line 161-167: Add i18next defaultValue fallbacks for all open-ended
translation interpolations: in src/renderer/components/GamesPanel.tsx lines
161-167, use session.app_id for gamesPanel.apps and session.status for
gamesPanel.status; in src/renderer/components/games/ChessBoard.tsx line 92, use
session.status for gamesPanel.status; and in lines 147-152, use piece for
gamesPanel.chess.pieceNames. Preserve the existing translation keys and
rendering behavior when keys exist.
- Around line 340-348: Update the delete flow around the GamesPanel delete
button and deleteGamesSession to require confirmation before permanently
removing a session. Reuse the existing ConfirmModal pattern used by the resign
action, wiring its confirmation callback to deleteGamesSession and preserving
actionBusy behavior. Add gamesPanel.deleteConfirmTitle and
gamesPanel.deleteConfirmMessage to every locale that defines
gamesPanel.resignConfirmTitle.
In `@src/renderer/components/reticulum/ReticulumGameChallengeButton.tsx`:
- Around line 39-41: The open challenge menu can still dispatch a game challenge
after the control becomes disabled. In
src/renderer/components/reticulum/ReticulumGameChallengeButton.tsx#L39-L41,
guard handleChallenge so disabled prevents sending; at `#L66-L80`, close or hide
the menu when disabled and disable its app option buttons. In
src/renderer/components/reticulum/ReticulumGameChallengeButton.test.tsx#L40-L43,
open the menu, rerender with disabled, and verify selecting an option does not
dispatch sendAction.
In `@src/renderer/lib/reticulum/clearReticulumSessionStores.test.ts`:
- Around line 47-51: Update the “clears the games store” test to seed a valid s1
session in useReticulumGamesStore before calling clearReticulumSessionStores().
Keep the existing selectedSessionId and sessions length assertions so the test
verifies cleanup removes the seeded session.
In `@src/shared/games-types.ts`:
- Around line 138-142: The games API path check in isGamesApiPath must normalize
or strip the query string before comparing against the base path and
GAMES_API_PREFIX. Apply this normalized path consistently in the
reticulum:proxyGet, reticulum:proxyPost, and reticulum:proxyDelete gating flows
so query-bearing games URLs use the reticulum:games* bucket.
---
Nitpick comments:
In `@reticulum-sidecar/src/stack/games_session.rs`:
- Around line 58-59: Bound the resend cache used by commit_action by evicting
entries for session IDs no longer retained by the store, or by enforcing a fixed
capacity with eviction. Update last_envelope and the associated insertion path
so stale entries cannot accumulate for the process lifetime, while preserving
resend behavior for active sessions and delete_session cleanup.
In `@src/main/ipc/reticulum-proxy-rate-limit.contract.test.ts`:
- Around line 57-68: Strengthen the contract assertions in the test around
HANDLERS_SOURCE by matching the reticulum:games limiter label and max: 600 in
one pattern, ensuring the ceiling is tied to that limiter. Broaden both preload
negative assertions to reject any reticulum:proxyGet or reticulum:proxyPost
invocation whose source contains /api/v1/games, regardless of whether the path
uses a literal, template, or variable.
In `@src/renderer/components/games/ChessBoard.test.tsx`:
- Around line 43-152: Add tests in the ChessBoard suite covering both untested
branches: render a black-turn session and verify clicks map to e7e5 on the
flipped board, then render a promotion position and verify e7 to e8 emits the
UCI move e7e8q. Reuse the existing makeSession and onMove patterns without
changing production behavior.
🪄 Autofix
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: 50b9c5e3-c1b2-41d3-9a2c-90a7c8025d53
⛔ Files ignored due to path filters (17)
reticulum-sidecar/Cargo.lockis excluded by!**/*.locksrc/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 (55)
AGENTS.mddocs/ci-cd.mddocs/development-environment.mddocs/reticulum-games-parity.mddocs/reticulum-sidecar-ipc.mddocs/reticulum.mddocs/troubleshooting.mdreticulum-sidecar/Cargo.tomlreticulum-sidecar/README.mdreticulum-sidecar/src/api/games.rsreticulum-sidecar/src/api/mod.rsreticulum-sidecar/src/api/system.rsreticulum-sidecar/src/stack/games_session.rsreticulum-sidecar/src/stack/live.rsreticulum-sidecar/src/stack/mod.rsscripts/clone-ratspeak-stack.shscripts/clone-ratspeak-stack.test.mjsscripts/i18n-unused-keys.mjsscripts/update.shscripts/update.test.mjssrc/main/index.contract.test.tssrc/main/ipc/reticulum-handlers.tssrc/main/ipc/reticulum-proxy-rate-limit.contract.test.tssrc/preload/index.tssrc/renderer/App.tsxsrc/renderer/components/ChatPanel.tsxsrc/renderer/components/GamesPanel.test.tsxsrc/renderer/components/GamesPanel.tsxsrc/renderer/components/ReticulumPeerListPanel.tsxsrc/renderer/components/games/ChessBoard.test.tsxsrc/renderer/components/games/ChessBoard.tsxsrc/renderer/components/games/TicTacToeBoard.test.tsxsrc/renderer/components/games/TicTacToeBoard.tsxsrc/renderer/components/reticulum/ReticulumGameChallengeButton.test.tsxsrc/renderer/components/reticulum/ReticulumGameChallengeButton.tsxsrc/renderer/lazyTabPanels.tssrc/renderer/lib/appTabMappings.test.tssrc/renderer/lib/appTabMappings.tssrc/renderer/lib/icons/tabIcons.test.tsxsrc/renderer/lib/icons/tabIcons.tsxsrc/renderer/lib/radio/BaseRadioProvider.tssrc/renderer/lib/radio/protocol-capabilities.test.tssrc/renderer/lib/reticulum/clearReticulumSessionStores.test.tssrc/renderer/lib/reticulum/clearReticulumSessionStores.tssrc/renderer/lib/reticulum/reticulumGamesMetadata.tssrc/renderer/lib/reticulum/reticulumGamesSession.tssrc/renderer/lib/tabSlotIds.tssrc/renderer/runtime/useReticulumRuntime.games.test.tssrc/renderer/runtime/useReticulumRuntime.tssrc/renderer/stores/reticulumGamesStore.test.tssrc/renderer/stores/reticulumGamesStore.tssrc/renderer/vitest.electronApiMock.tssrc/shared/electron-api.types.tssrc/shared/games-types.test.tssrc/shared/games-types.ts
💤 Files with no reviewable changes (1)
- reticulum-sidecar/src/api/system.rs
Comment on lines
328
to
+335
| router.register_delivery_callback(move |msg| { | ||
| if !msg.incoming { | ||
| return; | ||
| } | ||
| let sender_hex = hex::encode(msg.source_hash); | ||
| if games_session_cb.handle_inbound_lxmf(&msg.fields, &sender_hex, &msg.content) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect where the LXMF router invokes the delivery callback and whether the router lock is held.
set -euo pipefail
fd -t f -e rs . | xargs -r rg -n -C 8 'delivery_callback'Repository: Colorado-Mesh/mesh-client
Length of output: 4372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== live.rs delivery callback section =="
sed -n '300,360p' reticulum-sidecar/src/stack/live.rs
echo "== live.rs wrapper section around 2190-2220 =="
sed -n '2160,2235p' reticulum-sidecar/src/stack/live.rs
echo "== locate handle_inbound_lxmf definition and LrgpStore store accesses =="
rg -n "handle_inbound_lxmf|persist_session_from_state|session_detail|struct LrgpStore|impl LrgpStore|async fn .*LrgpStore|pub async fn|pub fn" reticulum-sidecar/src -g '*.rs'
echo "== lxmf delivery callback lock section =="
sed -n '330,365p' reticulum-sidecar/src/stack/lxmf_delivery.rsRepository: Colorado-Mesh/mesh-client
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== GamesSessionManager and handle_inbound_lxmf files =="
rg -n "struct GamesSessionManager|impl GamesSessionManager|fn spawn|handle_inbound_lxmf|persist_session_from_state|session_detail|LrgpStore|sq" reticulum-sidecar/src -g '*.rs' | head -n 240
echo "== candidate file excerpts =="
for f in $(rg -l "handle_inbound_lxmf|GamesSessionManager" reticulum-sidecar/src -g '*.rs'); do
echo "--- $f ---"
wc -l "$f"
sed -n '1,260p' "$f"
doneRepository: Colorado-Mesh/mesh-client
Length of output: 24486
Keep SQLite work off the router delivery callback.
deliver_unpacked_lxmf holds router.lock().await while calling the delivery callback, and that callback calls games_session_cb.handle_inbound_lxmf(...) synchronously. Inbound LRGP messages then wait on synchronous SQLite storage work if handle_inbound_lxmf reaches persist_session_from_state. Move the relevant store update out of the callback path, or at least move that call itself off the async path.
🤖 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 `@reticulum-sidecar/src/stack/live.rs` around lines 328 - 335, Move the
synchronous SQLite work triggered by games_session_cb.handle_inbound_lxmf out of
the router delivery callback registered in the live stack. Preserve the existing
inbound filtering and message handling, but dispatch the persistence path
asynchronously or otherwise defer it until after deliver_unpacked_lxmf releases
router.lock().await, ensuring persist_session_from_state never runs while the
callback holds the router lock.
Defer inbound LRGP SQLite persist off the LXMF router lock, tighten games input validation and store contracts, and close Games UI races around mark-read, delete confirm, and disabled challenges.
This was referenced Aug 6, 2026
Merged
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
lrgp-rsinto the Reticulum sidecar (LrgpRouter+ SQLite store, LXMF0xFB/0xFD, HTTP/api/v1/games/*, WSgames.*) so mesh-client peers can play Tic-Tac-Toe and Chess over LRGP.reticulum:games-*IPC (rate-limited; blocked on generic proxy),hasLrgpGames, and a full Games left-rail tab with challenge entry points from Peers/Chat.update.sh(games-parity) anddocs/reticulum-games-parity.md.Closes #773
Test plan
lrgp-rs(pnpm/clone script withRS_LRGP_REF); sidecar builds withrns-stack+ optionallrgphasLrgpGamesproxy*rejects/api/v1/games/*games.update/games.action_resultrefresh the Games panel without full poll thrashpnpm run check:pr(or CI) green; i18n keys undergamesPanel.*/tabs.gamespresentSummary by CodeRabbit
New Features
Documentation
Tests