Skip to content

Commit 7f4e5f6

Browse files
ehsan6shaclaude
andauthored
fix: stop making the user press twice, and let Blox AI find the Blox (#32)
Two defects behind three field reports, both of the same shape: the first attempt fails, the second works. 1. A stale certhash cost the user a click, every time. Opening the plugins screen and installing a plugin both go over libp2p, and both reported a "certificate" failure on the first try and succeeded on the second. `FulaClient.ensureConnected` was doing: if (err.code === 'NO_CERTHASH') this.refreshDiscovery = true; throw err; -- refresh NEXT time, and rethrow now. So the first action after a relay rotated its WebTransport certificate always failed, and the retry the user was owed only happened because they pressed the button again. kubo rotates those certs on a 14-day cycle and advertises current AND next, so a stale hash is routine, not exceptional. It now refreshes discovery and redials once, immediately. `retriedCerthash` keeps it to a single extra attempt, never a loop against a relay that is genuinely unreachable. 2. Blox AI could not find a Blox on the same switch. "Cannot reach your Blox over the local network or Bluetooth" on a Blox that answers ping, on the same LAN. `selectAiTransport` had four LAN tiers -- fresh cache record, typed IP, remembered address, and a `refreshOnce` that asks discovery /find-box for private ip4 entries. That last one needs fula-ota PR-D and returns nothing today, which its own comment says. So a browser with no remembered address and no typed IP had literally nothing left to try and fell through to BLE. Meanwhile the setup screen's own search finds that Blox in seconds. This adds it as a tier: candidate .local names plus a subnet sweep, reading each answer's peer id from blox-ai /diag/relay. That peer id is a STRONGER check than the /health probe the other tiers use -- the address is only used if the box behind it says it is the one we mean, so a private address pointing at a stranger on another network is rejected before anything is sent to it. A found IPv4 address is written back, so the next session takes the remembered tier and never pays for the scan again. Verified: 937 unit tests, tsc --noEmit clean on both the app and the client package, eslint clean. Not addressed: whether the first plugin-screen error was the same certhash failure. It was never read -- the report is "an error I couldn't see" -- so that is inference from the shape, not evidence. Claude-Session: https://claude.ai/code/session_01YJyqpFReP1wbz86nsqsoXq Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e35f3a7 commit 7f4e5f6

3 files changed

Lines changed: 147 additions & 4 deletions

File tree

‎apps/fxblox-web/src/utils/__tests__/aiTransport.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,14 @@ vi.mock('../lanIpCache', () => ({
88
refreshOnce: vi.fn().mockResolvedValue(undefined),
99
rememberedLanIp: vi.fn().mockResolvedValue(null),
1010
noteRecord: vi.fn(),
11+
noteLanIp: vi.fn(),
1112
clear: vi.fn(),
1213
}));
1314

15+
vi.mock('@/services/lanDiscovery', () => ({
16+
discoverBloxesOnLan: vi.fn().mockResolvedValue({ found: [], failure: 'not-found', lna: 'granted' }),
17+
}));
18+
1419
vi.mock('../httpAiClient', async (importOriginal) => {
1520
const actual = await importOriginal<typeof import('../httpAiClient')>();
1621
return { ...actual, HttpAiClient: vi.fn() };
@@ -19,10 +24,13 @@ vi.mock('../httpAiClient', async (importOriginal) => {
1924
import { ipIsPrivateLan, selectAiTransport } from '../aiTransport';
2025
import * as lanIpCache from '../lanIpCache';
2126
import { HttpAiClient } from '../httpAiClient';
27+
import { discoverBloxesOnLan } from '@/services/lanDiscovery';
2228

2329
const findAuthorizedBlox = lanIpCache.findAuthorizedBlox as unknown as ReturnType<typeof vi.fn>;
2430
const refreshOnce = lanIpCache.refreshOnce as unknown as ReturnType<typeof vi.fn>;
2531
const rememberedLanIp = lanIpCache.rememberedLanIp as unknown as ReturnType<typeof vi.fn>;
32+
const noteLanIp = lanIpCache.noteLanIp as unknown as ReturnType<typeof vi.fn>;
33+
const discoverMock = discoverBloxesOnLan as unknown as ReturnType<typeof vi.fn>;
2634
const HttpAiClientMock = HttpAiClient as unknown as ReturnType<typeof vi.fn>;
2735

2836
function record(over: Record<string, unknown> = {}, host = '192.168.1.50') {
@@ -43,6 +51,8 @@ beforeEach(() => {
4351
findAuthorizedBlox.mockReset();
4452
refreshOnce.mockReset().mockResolvedValue(undefined);
4553
rememberedLanIp.mockReset().mockResolvedValue(null);
54+
noteLanIp.mockReset();
55+
discoverMock.mockReset().mockResolvedValue({ found: [], failure: 'not-found', lna: 'granted' });
4656
HttpAiClientMock.mockReset();
4757
});
4858

@@ -349,3 +359,57 @@ describe('selectAiTransport — the address must be the right Blox', () => {
349359
expect(choice.kind).toBe('ble');
350360
});
351361
});
362+
363+
describe('selectAiTransport — searching this network when nothing else is left', () => {
364+
// Before this tier existed, a browser with no remembered address and no typed IP had nothing to try:
365+
// `refreshOnce` only asks discovery `/find-box` for private ip4 entries, which needs unshipped firmware and
366+
// returns nothing. Blox AI reported "cannot reach your Blox" about a Blox on the same switch.
367+
test('finds the Blox by scanning and uses it', async () => {
368+
findAuthorizedBlox.mockReturnValue(null);
369+
discoverMock.mockResolvedValue({ found: [{ host: '192.168.2.159', peerId: 'BLOX1' }], lna: 'granted' });
370+
HttpAiClientMock.mockImplementation(clientHealthyAt('192.168.2.159'));
371+
372+
const choice = await selectAiTransport('BLOX1', 'APP1', { scanIfEmpty: true });
373+
expect(choice.kind).toBe('lan-http');
374+
expect(choice.reason).toContain('network scan found 192.168.2.159');
375+
// Written back, so the next session takes the remembered-address tier and never pays for a scan again.
376+
expect(noteLanIp).toHaveBeenCalledWith(
377+
expect.objectContaining({ ip: '192.168.2.159', bloxPeerId: 'BLOX1', authorizer: 'APP1' }),
378+
);
379+
});
380+
381+
test('a `.local` name is usable, and is not written back to a cache that only holds addresses', async () => {
382+
findAuthorizedBlox.mockReturnValue(null);
383+
discoverMock.mockResolvedValue({ found: [{ host: 'fxblox-rk1.local', peerId: 'BLOX1' }], lna: 'granted' });
384+
HttpAiClientMock.mockImplementation(clientHealthyAt('fxblox-rk1.local'));
385+
386+
const choice = await selectAiTransport('BLOX1', 'APP1', { scanIfEmpty: true });
387+
expect(choice.kind).toBe('lan-http');
388+
expect(noteLanIp).not.toHaveBeenCalled();
389+
});
390+
391+
test('a Blox that is not the one we mean is ignored', async () => {
392+
// The scan reads each answer's peer id, so a private address pointing at someone else's box on another
393+
// network is rejected before anything is sent to it.
394+
findAuthorizedBlox.mockReturnValue(null);
395+
discoverMock.mockResolvedValue({ found: [{ host: '192.168.2.7', peerId: 'SOMEONE-ELSE' }], lna: 'granted' });
396+
HttpAiClientMock.mockImplementation(clientHealthyAt('192.168.2.7'));
397+
398+
const choice = await selectAiTransport('BLOX1', 'APP1', { scanIfEmpty: true });
399+
expect(choice.kind).toBe('ble');
400+
});
401+
402+
test('does not scan when the caller did not ask for it', async () => {
403+
findAuthorizedBlox.mockReturnValue(null);
404+
const choice = await selectAiTransport('BLOX1', 'APP1', { scanIfEmpty: false });
405+
expect(discoverMock).not.toHaveBeenCalled();
406+
expect(choice.kind).toBe('ble');
407+
});
408+
409+
test('a scan that throws falls through to BLE rather than breaking the screen', async () => {
410+
findAuthorizedBlox.mockReturnValue(null);
411+
discoverMock.mockRejectedValue(new Error('permission dismissed'));
412+
const choice = await selectAiTransport('BLOX1', 'APP1', { scanIfEmpty: true });
413+
expect(choice.kind).toBe('ble');
414+
});
415+
});

‎apps/fxblox-web/src/utils/aiTransport.ts‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,21 @@ export async function selectAiTransport(bloxPeerId: string, appPeerId: string, o
123123
hit = lanIpCache.findAuthorizedBlox(bloxPeerId, appPeerId, mdnsMaxAgeMs);
124124
}
125125

126+
// 5) Search this network directly. `refreshOnce` above only asks discovery `/find-box` for private `/ip4/`
127+
// entries, which needs fula-ota PR-D and returns nothing today — so before this tier existed, a browser
128+
// with no remembered address and no typed IP had literally nothing left to try and reported "cannot
129+
// reach your Blox" about a Blox on the same switch, one that the setup screen's own search finds in
130+
// seconds. This is that same search.
131+
if (!hit && scanIfEmpty) {
132+
const scanned = await qualifyLanScan(bloxPeerId, appPeerId, probeTimeoutMs);
133+
if (scanned) return scanned;
134+
}
135+
126136
if (!hit) {
127-
return { kind: 'ble', reason: 'no LAN candidate: no fresh record, no manual IP, no remembered address' };
137+
return {
138+
kind: 'ble',
139+
reason: 'no LAN candidate: no fresh record, no manual IP, no remembered address, nothing on this network',
140+
};
128141
}
129142

130143
const ip = hit.service.txt?.ipAddress ?? hit.service.host ?? '';
@@ -142,6 +155,50 @@ export async function selectAiTransport(bloxPeerId: string, appPeerId: string, o
142155
return { kind: 'ble', reason: `LAN HTTP /health probe failed (latency=${probe.latencyMs}ms)` };
143156
}
144157

158+
/** A `.local` name is local by definition; anything else has to be an RFC1918/link-local address. */
159+
export function hostIsLocal(host: string): boolean {
160+
return host.toLowerCase().endsWith('.local') || ipIsPrivateLan(host);
161+
}
162+
163+
/**
164+
* Search this network for the Blox, the way the setup screen does.
165+
*
166+
* `discoverBloxesOnLan` probes candidate `.local` names AND sweeps the subnets this device is on, reading each
167+
* answer's peer id from blox-ai's `/diag/relay`. That peer id is a STRONGER identity check than the `/health`
168+
* probe the other tiers rely on — the address is only used if the box behind it says it is the one we mean, so
169+
* a private address that points at a stranger on some other network is rejected before anything is sent to it.
170+
*
171+
* A found IPv4 address is written back to the cache, so the next session takes tier 3 and never pays for this
172+
* scan again.
173+
*/
174+
async function qualifyLanScan(
175+
bloxPeerId: string,
176+
appPeerId: string,
177+
probeTimeoutMs: number,
178+
): Promise<AiTransportChoice | null> {
179+
let outcome;
180+
try {
181+
const { discoverBloxesOnLan } = await import('@/services/lanDiscovery');
182+
outcome = await discoverBloxesOnLan();
183+
} catch {
184+
return null;
185+
}
186+
const match = outcome.found.find((blox) => blox.peerId === bloxPeerId);
187+
if (!match || !hostIsLocal(match.host)) return null;
188+
const client = new HttpAiClient(match.host, DEFAULT_BLOX_AI_PORT);
189+
const probe = await client.health(probeTimeoutMs);
190+
if (!probe.ok) return null;
191+
// Only an address is worth remembering; a `.local` name would fail tier 3's RFC1918 gate on the way back in.
192+
if (ipIsPrivateLan(match.host)) {
193+
lanIpCache.noteLanIp({ ip: match.host, bloxPeerId, authorizer: appPeerId });
194+
}
195+
return {
196+
kind: 'lan-http',
197+
httpClient: client,
198+
reason: `network scan found ${match.host}, peer id matched, /health 200 in ${probe.latencyMs}ms`,
199+
};
200+
}
201+
145202
function readBloxAiPortFromTxt(txt: Record<string, unknown> | undefined): number | undefined {
146203
if (!txt) return undefined;
147204
const raw = txt.bloxAiPort ?? txt.ai_port;

‎packages/fula-web-client/src/core/client.ts‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,14 @@ export class FulaClient {
260260
}
261261

262262
/** Reuse the tracked connection, or dial (candidate order: bloxAddr → find-box → relays → hardcoded). */
263-
async ensureConnected(opts: { signal?: AbortSignal; refresh?: boolean } = {}): Promise<TrackedConnection> {
263+
async ensureConnected(
264+
opts: {
265+
signal?: AbortSignal;
266+
refresh?: boolean;
267+
/** Internal: set on the one automatic retry after a stale certhash, so it cannot loop. */
268+
retriedCerthash?: boolean;
269+
} = {},
270+
): Promise<TrackedConnection> {
264271
if (this.closed) throw new FulaWebError('CLIENT_CLOSED', 'the client was shut down');
265272
const box = this.requireBox();
266273
const now = cfg.now;
@@ -330,8 +337,23 @@ export class FulaClient {
330337
} catch (e) {
331338
const err = isFulaWebError(e) ? e : new FulaWebError('DIAL_FAILED', errorMessage(e), { cause: e });
332339
this.lastError = err;
333-
// A stale certhash is the one dial failure discovery can fix → refresh next time.
334-
if (err.code === 'NO_CERTHASH') this.refreshDiscovery = true;
340+
// A stale certhash is the one dial failure discovery can fix, so fix it and try again NOW.
341+
//
342+
// This used to only set the flag and rethrow, which made the refresh happen on the *next* call — so the
343+
// first action after a relay rotated its WebTransport certificate always failed with "certificate"/
344+
// NO_CERTHASH, and the user had to press the button a second time to get the retry they were owed. Two
345+
// taps for every plugin install and every plugin screen, reported from the field exactly that way.
346+
//
347+
// kubo rotates those certs on a 14-day cycle and advertises current AND next, so a stale hash is a
348+
// routine event, not an exceptional one. One retry is enough: `retriedCerthash` makes it a single extra
349+
// attempt with freshly resolved candidates, never a loop against a relay that is genuinely unreachable.
350+
if (err.code === 'NO_CERTHASH') {
351+
this.refreshDiscovery = true;
352+
if (!opts.retriedCerthash) {
353+
log.info('stale certhash — refreshing discovery and redialling once');
354+
return this.ensureConnected({ ...opts, refresh: true, retriedCerthash: true });
355+
}
356+
}
335357
throw err;
336358
}
337359
}

0 commit comments

Comments
 (0)