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
10 changes: 6 additions & 4 deletions app/src/components/BootCheckGate/BootCheckGate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
clearStoredCoreMode,
clearStoredCoreToken,
isLocalOrPrivateNetworkHost,
normalizeRpcUrl,
storeCoreMode,
storeCoreToken,
storeRpcUrl,
Expand Down Expand Up @@ -122,13 +123,14 @@ function ModePicker({ onConfirm }: PickerProps) {
* paths are passed through verbatim without the bearer value.
*/
const validateInputs = (): { url: string; token: string } | null => {
const trimmedUrl = cloudUrl.trim();
if (!trimmedUrl) {
const rawUrl = cloudUrl.trim();
if (!rawUrl) {
setUrlError(t('bootCheck.invalidUrl'));
return null;
}
const normalizedUrl = normalizeRpcUrl(rawUrl);
try {
const parsed = new URL(trimmedUrl);
const parsed = new URL(normalizedUrl);
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
setUrlError(t('bootCheck.urlMustStartWith'));
return null;
Expand All @@ -152,7 +154,7 @@ function ModePicker({ onConfirm }: PickerProps) {
}
setTokenError(null);

return { url: trimmedUrl, token: trimmedToken };
return { url: normalizedUrl, token: trimmedToken };
};

const handleTestConnection = async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,32 @@ describe('BootCheckGate — picker (unset mode)', () => {
);
});

it('normalizes a cloud core base URL to the /rpc endpoint before continuing', async () => {
mockRunBootCheck.mockResolvedValue({ kind: 'match' });

renderGate();
fireEvent.click(screen.getByText('Run on the Cloud (Complex)'));
fireEvent.change(screen.getByPlaceholderText(/https:\/\/core\.example\.com/), {
target: { value: 'https://example.trycloudflare.com/' },
});
fireEvent.change(screen.getByPlaceholderText(/Bearer token/i), {
target: { value: 'tok-1234' },
});
fireEvent.click(screen.getByRole('button', { name: 'Continue' }));

await waitFor(() => {
expect(screen.getByTestId('app-content')).toBeInTheDocument();
});
expect(mockRunBootCheck).toHaveBeenCalledWith(
expect.objectContaining({
kind: 'cloud',
url: 'https://example.trycloudflare.com/rpc',
token: 'tok-1234',
}),
expect.any(Object)
);
});

it('rejects public HTTP cloud URLs', () => {
renderGate();

Expand Down Expand Up @@ -274,6 +300,26 @@ describe('BootCheckGate — picker test connection', () => {
);
});

it('tests /rpc when the user enters a cloud core base URL', async () => {
mockTestCoreRpcConnection.mockResolvedValue({
ok: true,
status: 200,
json: async () => ({ result: { ok: true } }),
} as unknown as Response);

renderGate();
fillCloudInputs('https://example.trycloudflare.com/');
fireEvent.click(screen.getByRole('button', { name: 'Test Connection' }));

await waitFor(() => {
expect(screen.getByTestId('test-status-ok')).toBeInTheDocument();
});
expect(mockTestCoreRpcConnection).toHaveBeenCalledWith(
'https://example.trycloudflare.com/rpc',
'tok-abc'
);
});

it('shows Auth failed on a 401 response', async () => {
mockTestCoreRpcConnection.mockResolvedValue({
ok: false,
Expand Down
47 changes: 47 additions & 0 deletions app/src/services/__tests__/coreRpcClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,19 @@ describe('coreRpcClient', () => {
});
});

test('normalizes a supplied core base URL before probing', async () => {
vi.resetModules();
vi.mocked(isTauri).mockReturnValue(false);
const { testCoreRpcConnection } = await import('../coreRpcClient');
const fetchMock = vi.mocked(fetch);
fetchMock.mockResolvedValueOnce({ ok: true, status: 200 } as Response);

await testCoreRpcConnection('https://example.trycloudflare.com/');

expect(fetchMock).toHaveBeenCalledTimes(1);
expect(fetchMock.mock.calls[0][0]).toBe('https://example.trycloudflare.com/rpc');
});

test('omits Authorization header when no bearer token is available (non-Tauri)', async () => {
vi.resetModules();
vi.mocked(isTauri).mockReturnValue(false);
Expand Down Expand Up @@ -854,6 +867,11 @@ describe('coreRpcClient — typed errors + auth-expired event', () => {
});

describe('getCoreRpcUrl', () => {
const normalizeMockRpcUrl = (url: string) => {
const trimmed = url.replace(/\/+$/, '');
return trimmed.endsWith('/rpc') ? trimmed : `${trimmed}/rpc`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] normalizeMockRpcUrl is a simplified stub that doesn't match normalizeRpcUrl's behavior for URLs with query strings or leading whitespace. For example, 'https://example.com?next=/' → mock produces 'https://example.com?next=//rpc' (appends /rpc to the whole string including ?next=/), whereas the real function returns 'https://example.com/rpc?next=/'.

The test cases that use this mock all pass pre-normalized RPC URLs through, so it works today. But the test 'in web mode normalizes a stored core base URL' is checking the mock's normalization, not the real module's — if normalizeRpcUrl broke for query-bearing URLs, that test would still pass.

An easy fix: import normalizeRpcUrl from the actual module in the vi.doMock factory (using vi.importActual) rather than re-implementing it. Same pattern applies to the duplicate at line ~1085 in the getCoreRpcToken describe block.

};

// Each test gets a fresh module so module-level caches are cleared
beforeEach(() => {
vi.resetModules();
Expand All @@ -865,6 +883,7 @@ describe('getCoreRpcUrl', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => 'http://custom-host:9999/rpc',
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(false);

Expand All @@ -873,10 +892,24 @@ describe('getCoreRpcUrl', () => {
expect(url).toBe('http://custom-host:9999/rpc');
});

test('in web mode normalizes a stored core base URL', async () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => 'https://example.trycloudflare.com/',
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(false);

const { getCoreRpcUrl: freshGetCoreRpcUrl } = await import('../coreRpcClient');
const url = await freshGetCoreRpcUrl();
expect(url).toBe('https://example.trycloudflare.com/rpc');
});

test('in web mode returns default CORE_RPC_URL when nothing is stored', async () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => null,
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(false);

Expand All @@ -893,6 +926,7 @@ describe('getCoreRpcUrl', () => {
return null;
},
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(false);

Expand All @@ -909,6 +943,7 @@ describe('getCoreRpcUrl', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => storedValue,
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(false);

Expand All @@ -930,6 +965,7 @@ describe('getCoreRpcUrl', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => null,
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
vi.mocked(invoke).mockImplementation(async (cmd: string) => {
Expand All @@ -947,6 +983,7 @@ describe('getCoreRpcUrl', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => 'http://stored-override:4444/rpc',
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
vi.mocked(invoke).mockImplementation(async (cmd: string) => {
Expand All @@ -968,6 +1005,7 @@ describe('getCoreRpcUrl', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => 'http://127.0.0.1:7788/rpc',
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
vi.mocked(invoke).mockImplementation(async (cmd: string) => {
Expand All @@ -987,6 +1025,7 @@ describe('getCoreRpcUrl', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => null,
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
vi.mocked(invoke).mockRejectedValue(new Error('invoke failed'));
Expand All @@ -999,6 +1038,11 @@ describe('getCoreRpcUrl', () => {
});

describe('getCoreRpcToken (cloud-mode persistence)', () => {
const normalizeMockRpcUrl = (url: string) => {
const trimmed = url.replace(/\/+$/, '');
return trimmed.endsWith('/rpc') ? trimmed : `${trimmed}/rpc`;
};

beforeEach(() => {
vi.resetModules();
vi.clearAllMocks();
Expand All @@ -1009,6 +1053,7 @@ describe('getCoreRpcToken (cloud-mode persistence)', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => 'https://core.example.com/rpc',
getStoredCoreToken: () => 'cloud-token-abc',
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
vi.mocked(invoke).mockImplementation(async (cmd: string) => {
Expand Down Expand Up @@ -1038,6 +1083,7 @@ describe('getCoreRpcToken (cloud-mode persistence)', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => 'https://core.example.com/rpc',
getStoredCoreToken: () => storedToken,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
const fetchMock = vi.mocked(fetch);
Expand Down Expand Up @@ -1065,6 +1111,7 @@ describe('getCoreRpcToken (cloud-mode persistence)', () => {
vi.doMock('../../utils/configPersistence', () => ({
peekStoredRpcUrl: () => null,
getStoredCoreToken: () => null,
normalizeRpcUrl: normalizeMockRpcUrl,
}));
vi.mocked(isTauri).mockReturnValue(true);
vi.mocked(invoke).mockImplementation(async (cmd: string) => {
Expand Down
18 changes: 10 additions & 8 deletions app/src/services/coreRpcClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import debug from 'debug';

import { dispatchLocalAiMethod } from '../lib/ai/localCoreAiMemory';
import { CORE_RPC_TIMEOUT_MS, CORE_RPC_URL } from '../utils/config';
import { getStoredCoreToken, peekStoredRpcUrl } from '../utils/configPersistence';
import { getStoredCoreToken, normalizeRpcUrl, peekStoredRpcUrl } from '../utils/configPersistence';
import { redactRpcUrlForLog } from '../utils/redactRpcUrlForLog';
import { sanitizeError } from '../utils/sanitize';
import { isTauri as coreIsTauri } from '../utils/tauriCommands/common';
import { normalizeRpcMethod } from './rpcMethods';
Expand Down Expand Up @@ -278,7 +279,7 @@ export async function getCoreRpcUrl(): Promise<string> {
// null when nothing is stored, which lets us distinguish "user hasn't
// chosen yet" from "user chose a value identical to the default".
const storedUrl = peekStoredRpcUrl();
resolvedCoreRpcUrl = storedUrl ?? CORE_RPC_URL;
resolvedCoreRpcUrl = normalizeRpcUrl(storedUrl ?? CORE_RPC_URL);
return resolvedCoreRpcUrl;
}

Expand All @@ -296,8 +297,8 @@ export async function getCoreRpcUrl(): Promise<string> {
// cloud mode where no local sidecar is running.
const storedUrl = peekStoredRpcUrl();
if (storedUrl) {
resolvedCoreRpcUrl = storedUrl;
return storedUrl;
resolvedCoreRpcUrl = normalizeRpcUrl(storedUrl);
return resolvedCoreRpcUrl;
}

const url = await invoke<string>('core_rpc_url');
Expand All @@ -307,16 +308,16 @@ export async function getCoreRpcUrl(): Promise<string> {
fallback: CORE_RPC_URL,
});
}
resolvedCoreRpcUrl = trimmed || CORE_RPC_URL;
resolvedCoreRpcUrl = normalizeRpcUrl(trimmed || CORE_RPC_URL);
return resolvedCoreRpcUrl || CORE_RPC_URL;
} catch (err) {
// Tauri invoke failed — fall back to stored URL if any, then the
// build-time default. Keep the underlying invoke failure visible so
// port mismatches and shell misconfiguration are diagnosable.
const storedUrl = peekStoredRpcUrl();
resolvedCoreRpcUrl = storedUrl ?? CORE_RPC_URL;
resolvedCoreRpcUrl = normalizeRpcUrl(storedUrl ?? CORE_RPC_URL);
coreRpcError('core_rpc_url invoke failed; using fallback RPC URL', {
fallback: resolvedCoreRpcUrl,
fallback: redactRpcUrlForLog(resolvedCoreRpcUrl),
usedStoredUrl: Boolean(storedUrl),
error: sanitizeError(err),
});
Expand Down Expand Up @@ -397,12 +398,13 @@ export async function testCoreRpcConnection(
tokenOverride?: string,
init?: { signal?: AbortSignal }
): Promise<Response> {
const rpcUrl = normalizeRpcUrl(url);
const token = tokenOverride?.trim() || (await getCoreRpcToken());
const headers: Record<string, string> = { 'Content-Type': 'application/json' };
if (token) {
headers.Authorization = `Bearer ${token}`;
}
return fetch(url, {
return fetch(rpcUrl, {
method: 'POST',
headers,
body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'core.ping', params: {} }),
Expand Down
19 changes: 18 additions & 1 deletion app/src/store/coreModeSlice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ describe('coreModeSlice — sync-localStorage-derived initial state', () => {
it('uses local mode when the E2E default core mode config is local', async () => {
localStorage.clear();
vi.resetModules();
vi.doMock('../utils/config', () => ({ E2E_DEFAULT_CORE_MODE: 'local' }));
vi.doMock('../utils/config', () => ({
CORE_RPC_URL: 'http://127.0.0.1:7788/rpc',
E2E_DEFAULT_CORE_MODE: 'local',
}));
try {
const mod = await import('./coreModeSlice');
const state = mod.default(undefined, { type: '@@INIT' });
Expand Down Expand Up @@ -100,6 +103,20 @@ describe('coreModeSlice — sync-localStorage-derived initial state', () => {
});
});

it('normalizes restored cloud base URLs to the /rpc endpoint', async () => {
localStorage.clear();
localStorage.setItem('openhuman_core_mode', 'cloud');
localStorage.setItem('openhuman_core_rpc_url', 'https://example.trycloudflare.com/');
localStorage.setItem('openhuman_core_rpc_token', 'tok-abc');
const mod = await freshImport();
const state = mod.default(undefined, { type: '@@INIT' });
expect(state.mode).toEqual({
kind: 'cloud',
url: 'https://example.trycloudflare.com/rpc',
token: 'tok-abc',
});
});

it('falls back to unset when cloud marker exists but URL or token is missing', async () => {
localStorage.clear();
localStorage.setItem('openhuman_core_mode', 'cloud');
Expand Down
3 changes: 2 additions & 1 deletion app/src/store/coreModeSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import { createSlice, type PayloadAction } from '@reduxjs/toolkit';

import { E2E_DEFAULT_CORE_MODE } from '../utils/config';
import { normalizeRpcUrl } from '../utils/configPersistence';

export type CoreMode =
| { kind: 'unset' }
Expand Down Expand Up @@ -64,7 +65,7 @@ function deriveInitialMode(): CoreMode {
if (mode === 'cloud') {
const url = localStorage.getItem(RPC_URL_STORAGE_KEY)?.trim();
const token = localStorage.getItem(CORE_TOKEN_STORAGE_KEY)?.trim();
if (url && token) return { kind: 'cloud', url, token };
if (url && token) return { kind: 'cloud', url: normalizeRpcUrl(url), token };
}
} catch {
/* localStorage unavailable — fall through to unset */
Expand Down
Loading
Loading