Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion reticulum-sidecar/src/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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::*;
Expand All @@ -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 {
Expand Down
101 changes: 101 additions & 0 deletions src/renderer/components/remote/RemoteSettingsSection.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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(
<RemoteSettingsSection
sidecarRunning
settings={{
...DEFAULT_REMOTE_SETTINGS,
inboundMode: 'ask',
lastSaveDir: '/tmp/rncp-inbox',
}}
onSettingsChange={onSettingsChange}
/>,
);

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(
<RemoteSettingsSection
sidecarRunning
settings={{
...DEFAULT_REMOTE_SETTINGS,
inboundMode: 'ask',
lastSaveDir: '/tmp/rncp-inbox',
}}
onSettingsChange={onSettingsChange}
/>,
);

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)
Expand Down
12 changes: 6 additions & 6 deletions src/renderer/components/remote/RemoteSettingsSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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();
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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');
Expand Down
3 changes: 2 additions & 1 deletion src/renderer/components/remote/RncpEnableRequestModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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'),
Expand Down
8 changes: 3 additions & 5 deletions src/renderer/lib/pushRncpListenerPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
30 changes: 30 additions & 0 deletions src/shared/remote-types.isRemoteOkFailure.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
11 changes: 11 additions & 0 deletions src/shared/remote-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading