diff --git a/extension/src/popup/components/__tests__/devices.test.ts b/extension/src/popup/components/__tests__/devices.test.ts index 60c31b7..2b0233c 100644 --- a/extension/src/popup/components/__tests__/devices.test.ts +++ b/extension/src/popup/components/__tests__/devices.test.ts @@ -95,6 +95,32 @@ describe('devices view', () => { expect(app.querySelector('#register-confirm-btn')).not.toBeNull(); }); + // Plan C Phase 5 — defensive Promise.allSettled: + // a rejected secondary feed (list_revoked) should not kill the whole render. + it('renders devices when revoked list fails (load-error slot shown)', async () => { + (sendMessage as ReturnType) + .mockResolvedValueOnce({ ok: true, data: { devices: [{ name: 'CLI', public_key: 'k', added_at: 1 }] } }) + .mockRejectedValueOnce(new Error('boom')); + + await renderDevices(app); + + // Primary list still rendered. + expect(app.innerHTML).toContain('CLI'); + // Inline fallback slot present. + expect(app.innerHTML).toContain("Couldn't load revoked devices"); + }); + + it('renders devices when revoked list returns {ok:false}', async () => { + (sendMessage as ReturnType) + .mockResolvedValueOnce({ ok: true, data: { devices: [{ name: 'CLI', public_key: 'k', added_at: 1 }] } }) + .mockResolvedValueOnce({ ok: false, error: 'list_revoked_failed' }); + + await renderDevices(app); + + expect(app.innerHTML).toContain('CLI'); + expect(app.innerHTML).toContain("Couldn't load revoked devices"); + }); + it('confirming register sends register_this_device with the entered name', async () => { (chrome.storage.local.get as ReturnType).mockResolvedValueOnce({ device_name: 'Unknown' }); // Initial render: list_devices + list_revoked. diff --git a/extension/src/popup/components/devices.ts b/extension/src/popup/components/devices.ts index 1c83394..8b85373 100644 --- a/extension/src/popup/components/devices.ts +++ b/extension/src/popup/components/devices.ts @@ -31,35 +31,64 @@ export function teardown(): void { // No cleanup needed } +/** + * DEV-C P2: defensive per-slot rendering. The active list is the primary + * feed — if it fails entirely, we still surface an error page. The + * revoked list is secondary — its failure renders an inline "couldn't + * load" slot but doesn't kill the page. + */ +function revokedLoadErrorHtml(): string { + return ` +
+ ▸ revoked devices +
+

Couldn't load revoked devices.

+
+
+ `; +} + export async function renderDevices(app: HTMLElement): Promise { // Get current device name from local storage const stored = await chrome.storage.local.get(['device_name']); const currentDeviceName: string | undefined = stored.device_name as string | undefined; - // Fetch active device list and revoked list in parallel - const [devicesResp, revokedResp] = await Promise.all([ + // Fetch active device list and revoked list in parallel. allSettled so a + // rejected secondary feed doesn't kill the whole render. + const [devicesSettled, revokedSettled] = await Promise.allSettled([ sendMessage({ type: 'list_devices' }), sendMessage({ type: 'list_revoked' }), ]); - if (!devicesResp.ok) { + if (devicesSettled.status === 'rejected' || !devicesSettled.value.ok) { app.innerHTML = `

Failed to load devices

`; return; } - const devices = (devicesResp.data as { devices: Device[] }).devices; - const revokedDevices: RevokedEntry[] = revokedResp.ok - ? (revokedResp.data as { revoked: RevokedEntry[] }).revoked + // devicesSettled.value.ok is true here (guarded above), so .data is present. + const devicesData = (devicesSettled.value as { ok: true; data: unknown }).data; + const devices = (devicesData as { devices: Device[] }).devices; + const revokedOk = revokedSettled.status === 'fulfilled' && revokedSettled.value.ok; + const revokedDevices: RevokedEntry[] = revokedOk + ? ((revokedSettled.value as { ok: true; data: unknown }).data as { revoked: RevokedEntry[] }).revoked : []; const isRegistered = currentDeviceName && devices.some((d) => d.name === currentDeviceName); - // Precompute fingerprints for all active devices + // Precompute fingerprints for all active devices. allSettled so one bad + // public key doesn't kill the whole list — fall back to '(unknown)'. const fingerprints = new Map(); - await Promise.all(devices.map(async (d) => { - const fp = await sshFingerprint(d.public_key); - fingerprints.set(d.name, fp ?? '(unknown)'); - })); + const fpResults = await Promise.allSettled( + devices.map((d) => sshFingerprint(d.public_key).then((fp) => [d.name, fp] as const)), + ); + for (let i = 0; i < devices.length; i += 1) { + const r = fpResults[i]; + if (r.status === 'fulfilled' && r.value[1]) { + fingerprints.set(r.value[0], r.value[1]); + } else { + fingerprints.set(devices[i].name, '(unknown)'); + } + } const activeDevicesHtml = devices.length === 0 ? `

No devices registered

` @@ -82,7 +111,9 @@ export async function renderDevices(app: HTMLElement): Promise { `; }).join(''); - const revokedSectionHtml = revokedDevices.length === 0 ? '' : ` + const revokedSectionHtml = !revokedOk + ? revokedLoadErrorHtml() + : revokedDevices.length === 0 ? '' : `
▸ show ${revokedDevices.length} revoked device${revokedDevices.length !== 1 ? 's' : ''}
@@ -117,7 +148,7 @@ export async function renderDevices(app: HTMLElement): Promise { ` : ''} ${devices.length > 0 ? `
ACTIVE · ${devices.length}
` : ''} ${activeDevicesHtml} - ${revokedDevices.length > 0 ? `
REVOKED · ${revokedDevices.length}
` : ''} + ${!revokedOk ? `
REVOKED · ?
` : (revokedDevices.length > 0 ? `
REVOKED · ${revokedDevices.length}
` : '')} ${revokedSectionHtml}
`;