diff --git a/reticulum-sidecar/src/stack/mod.rs b/reticulum-sidecar/src/stack/mod.rs index 7b750005d..10c8462f8 100644 --- a/reticulum-sidecar/src/stack/mod.rs +++ b/reticulum-sidecar/src/stack/mod.rs @@ -2098,7 +2098,9 @@ impl StackHandle { tracing::warn!("rncp listener persist failed: {e}"); } } - return live.rncp_listener_status().await; + // Stamp explicit `ok: true` success marker (same RemoteOkResponse + // contract as enable) onto the post-disable listener status. + return with_rncp_listener_ok(live.rncp_listener_status().await); } let mode = if allowed.is_empty() { "ask" @@ -2829,6 +2831,15 @@ fn merge_live_peer_fetch( } } +/// Stamp `ok: true` onto an rncp listener status object so disable returns the +/// same explicit RNCP success marker as enable (`RemoteOkResponse`). +fn with_rncp_listener_ok(mut status: serde_json::Value) -> serde_json::Value { + if let Some(map) = status.as_object_mut() { + map.insert("ok".into(), serde_json::Value::Bool(true)); + } + status +} + #[cfg(test)] mod tests { use super::*; @@ -2844,6 +2855,29 @@ mod tests { (config, storage) } + #[test] + fn with_rncp_listener_ok_stamps_ok_true_on_status() { + let status = serde_json::json!({ + "enabled": false, + "inbound_mode": "off", + "allowed": [], + "blocked": [], + }); + let out = with_rncp_listener_ok(status); + assert_eq!( + out.get("ok").and_then(serde_json::Value::as_bool), + Some(true) + ); + assert_eq!( + out.get("enabled").and_then(serde_json::Value::as_bool), + Some(false) + ); + assert_eq!( + out.get("inbound_mode").and_then(|v| v.as_str()), + Some("off") + ); + } + #[test] fn merge_live_peer_fetch_preserves_cache_on_empty_or_error() { let mut cache = vec![PeerRow { diff --git a/src/renderer/components/remote/RemoteSettingsSection.test.tsx b/src/renderer/components/remote/RemoteSettingsSection.test.tsx index bfa6a826a..e61fd9626 100644 --- a/src/renderer/components/remote/RemoteSettingsSection.test.tsx +++ b/src/renderer/components/remote/RemoteSettingsSection.test.tsx @@ -8,11 +8,18 @@ import { DEFAULT_REMOTE_SETTINGS } from '@/renderer/lib/remoteSettingsStorage'; import { useReticulumInboundPolicyStore } from '@/renderer/stores/reticulumInboundPolicyStore'; import { useRncpTransferStore } from '@/renderer/stores/rncpTransferStore'; +const addToast = vi.fn(); + +vi.mock('@/renderer/components/Toast', () => ({ + useToast: () => ({ addToast }), +})); + describe('RemoteSettingsSection inbound apply', () => { const onSettingsChange = vi.fn(); beforeEach(() => { onSettingsChange.mockReset(); + addToast.mockReset(); useRncpTransferStore.getState().clearAll(); useReticulumInboundPolicyStore.setState({ policies: new Map(), loading: false }); vi.mocked(window.electronAPI.reticulum.rncp.setListener).mockReset(); @@ -31,6 +38,100 @@ describe('RemoteSettingsSection inbound apply', () => { }); }); + it('treats Off success when setListener returns status without ok (legacy shape)', async () => { + const user = userEvent.setup(); + // Pre-#fix sidecar disable returned listener_status with no `ok` field. + vi.mocked(window.electronAPI.reticulum.rncp.setListener).mockResolvedValue({ + enabled: false, + inbound_mode: 'off', + allowed: [], + blocked: [], + } as unknown as { ok: boolean; error?: string }); + useRncpTransferStore.getState().setListener({ + enabled: true, + inbound_mode: 'ask', + destination_hash: 'a'.repeat(32), + allowed: [], + blocked: [], + }); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: 'Off' })); + + await waitFor(() => { + expect(window.electronAPI.reticulum.rncp.setListener).toHaveBeenCalledWith({ + enabled: false, + }); + }); + await waitFor(() => { + expect(onSettingsChange).toHaveBeenCalledWith({ inboundMode: 'off' }); + }); + expect(addToast).not.toHaveBeenCalledWith( + expect.stringMatching(/Failed to apply setting/i), + 'error', + ); + }); + + it('toasts applyFailed when Off setListener returns ok: false', async () => { + const user = userEvent.setup(); + vi.mocked(window.electronAPI.reticulum.rncp.setListener).mockResolvedValue({ + ok: false, + error: 'rncp requires live rns-stack sidecar', + }); + // Sidecar still Ask — sync after failure must not look like a successful Off. + vi.mocked(window.electronAPI.reticulum.rncp.getListener).mockResolvedValue({ + enabled: true, + inbound_mode: 'ask', + destination_hash: 'a'.repeat(32), + allowed: [], + blocked: [], + }); + useRncpTransferStore.getState().setListener({ + enabled: true, + inbound_mode: 'ask', + destination_hash: 'a'.repeat(32), + allowed: [], + blocked: [], + }); + + render( + , + ); + + await user.click(screen.getByRole('button', { name: 'Off' })); + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith( + expect.stringContaining('Failed to apply setting'), + 'error', + ); + }); + expect( + onSettingsChange.mock.calls.some( + (c) => (c[0] as { inboundMode?: string }).inboundMode === 'off', + ), + ).toBe(false); + }); + it('re-picks save dir when persisted path is rejected, then enables without optimistic Ask', async () => { const user = userEvent.setup(); vi.mocked(window.electronAPI.reticulum.rncp.setListener) diff --git a/src/renderer/components/remote/RemoteSettingsSection.tsx b/src/renderer/components/remote/RemoteSettingsSection.tsx index 178fd3ecd..3ae4bc920 100644 --- a/src/renderer/components/remote/RemoteSettingsSection.tsx +++ b/src/renderer/components/remote/RemoteSettingsSection.tsx @@ -12,7 +12,7 @@ import { import { writeClipboardText } from '@/renderer/lib/writeClipboardText'; import { useReticulumInboundPolicyStore } from '@/renderer/stores/reticulumInboundPolicyStore'; import { useRncpTransferStore } from '@/renderer/stores/rncpTransferStore'; -import type { RncpInboundMode } from '@/shared/remote-types'; +import { isRemoteOkFailure, type RncpInboundMode } from '@/shared/remote-types'; /** Sidecar outbound + inbound hard cap (see `MAX_RNCP_FILE_BYTES` in rncp_transfer.rs). */ export const RNCP_MAX_FILE_SIZE_LABEL = '25 MiB'; @@ -106,7 +106,7 @@ export function RemoteSettingsSection({ const res = await window.electronAPI.reticulum.rncp.setListener({ enabled: false, }); - if (!res.ok) { + if (isRemoteOkFailure(res)) { addToast( t('reticulumRemote.settings.applyFailed', { error: res.error ?? '' }), 'error', @@ -157,7 +157,7 @@ export function RemoteSettingsSection({ try { let res = await trySet(dir, jail); - if (!res.ok && isRncpPickerAllowlistError(res.error)) { + if (isRemoteOkFailure(res) && isRncpPickerAllowlistError(res.error)) { addToast(t('reticulumRemote.settings.rechooseSaveDir'), 'info'); // Re-authorize dirs from this session's picker — persisted paths are rejected after restart. const rePicked = await pickSaveDir(); @@ -178,7 +178,7 @@ export function RemoteSettingsSection({ res = await trySet(dir, jail); } - if (!res.ok) { + if (isRemoteOkFailure(res)) { const err = res.error ?? ''; addToast( isRncpPickerAllowlistError(err) @@ -239,7 +239,7 @@ export function RemoteSettingsSection({ allowed, blocked, }); - if (!res.ok && isRncpPickerAllowlistError(res.error)) { + if (isRemoteOkFailure(res) && isRncpPickerAllowlistError(res.error)) { addToast(t('reticulumRemote.settings.rechooseSaveDir'), 'info'); const rePicked = await pickSaveDir(); if (!rePicked) { @@ -266,7 +266,7 @@ export function RemoteSettingsSection({ blocked, }); } - if (!res.ok) { + if (isRemoteOkFailure(res)) { console.warn('[RemoteSettingsSection] pushPolicy ' + (res.error ?? '')); if (isRncpPickerAllowlistError(res.error)) { addToast(t('reticulumRemote.settings.rechooseSaveDir'), 'error'); diff --git a/src/renderer/components/remote/RncpEnableRequestModal.tsx b/src/renderer/components/remote/RncpEnableRequestModal.tsx index 244e835d4..5737dbb78 100644 --- a/src/renderer/components/remote/RncpEnableRequestModal.tsx +++ b/src/renderer/components/remote/RncpEnableRequestModal.tsx @@ -16,6 +16,7 @@ import { useReticulumInboundPolicyStore } from '@/renderer/stores/reticulumInbou import { useReticulumRemoteAddressStore } from '@/renderer/stores/reticulumRemoteAddressStore'; import { useRncpEnableRequestStore } from '@/renderer/stores/rncpEnableRequestStore'; import { useRncpTransferStore } from '@/renderer/stores/rncpTransferStore'; +import { isRemoteOkFailure } from '@/shared/remote-types'; import { canonicalizeReticulumDestinationHash } from '@/shared/reticulumDestinationHash'; import { buildRncpReceiveDestShareBody } from '@/shared/rncpRequestEnable'; @@ -194,7 +195,7 @@ export function RncpEnableRequestModal() { allowed, blocked, }); - if (!res.ok) { + if (isRemoteOkFailure(res)) { addToast( t('reticulumRemote.enableRequest.enableFailed', { error: res.error ?? t('common.error'), diff --git a/src/renderer/lib/pushRncpListenerPolicy.ts b/src/renderer/lib/pushRncpListenerPolicy.ts index fb1ae825e..5af5e484b 100644 --- a/src/renderer/lib/pushRncpListenerPolicy.ts +++ b/src/renderer/lib/pushRncpListenerPolicy.ts @@ -7,7 +7,7 @@ import { import { policiesToRncpLists } from '@/renderer/lib/rncpInboundPolicyLists'; import { useReticulumInboundPolicyStore } from '@/renderer/stores/reticulumInboundPolicyStore'; import { useRncpTransferStore } from '@/renderer/stores/rncpTransferStore'; -import type { RncpListenerRequest } from '@/shared/remote-types'; +import { isRemoteOkFailure, type RncpListenerRequest } from '@/shared/remote-types'; /** * Rebuild sidecar allow/block lists from SQLite policy and re-apply the listener @@ -41,10 +41,8 @@ export async function pushRncpListenerPolicy( }; try { const res = await window.electronAPI.reticulum.rncp.setListener(body); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- Runtime guard protects external or callback-mutated state. - if (res && typeof res === 'object' && 'ok' in res && !res.ok) { - const err = - 'error' in res && typeof res.error === 'string' ? res.error : 'setListener_failed'; + if (isRemoteOkFailure(res)) { + const err = typeof res.error === 'string' ? res.error : 'setListener_failed'; // Sync store from live status so optimistic Ask cannot stick after reject. try { const status = await window.electronAPI.reticulum.rncp.getListener(); diff --git a/src/shared/remote-types.isRemoteOkFailure.test.ts b/src/shared/remote-types.isRemoteOkFailure.test.ts new file mode 100644 index 000000000..31baf4b02 --- /dev/null +++ b/src/shared/remote-types.isRemoteOkFailure.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; + +import { isRemoteOkFailure } from './remote-types'; + +describe('isRemoteOkFailure', () => { + it('is false when ok is missing (legacy listener status shape)', () => { + expect( + isRemoteOkFailure({ + enabled: false, + inbound_mode: 'off', + allowed: [], + blocked: [], + }), + ).toBe(false); + }); + + it('is false when ok is true', () => { + expect(isRemoteOkFailure({ ok: true })).toBe(false); + }); + + it('is true when ok is false', () => { + expect(isRemoteOkFailure({ ok: false, error: 'save_dir_not_from_picker' })).toBe(true); + }); + + it('is false for null/non-objects', () => { + expect(isRemoteOkFailure(null)).toBe(false); + expect(isRemoteOkFailure(undefined)).toBe(false); + expect(isRemoteOkFailure('ok')).toBe(false); + }); +}); diff --git a/src/shared/remote-types.ts b/src/shared/remote-types.ts index 67ab6ffba..d0cfb0d0d 100644 --- a/src/shared/remote-types.ts +++ b/src/shared/remote-types.ts @@ -126,6 +126,17 @@ export interface RemoteOkResponse { error?: string; } +/** + * True only when a Remote-style IPC payload explicitly reports failure. + * Missing `ok` (e.g. legacy listener status JSON) is not a failure — callers + * that used `!res.ok` treated `undefined` as failed after a successful Off. + */ +export function isRemoteOkFailure( + res: unknown, +): res is RemoteOkResponse & { ok: false; error?: string } { + return res != null && typeof res === 'object' && 'ok' in res && res.ok === false; +} + /** `rnsh.stdout` / `rnsh.stderr` WS event payload (base64-encoded chunk). */ export interface RnshStreamEventPayload { session_id: string;