From ba5d21884179bd81b14a0229c06cde75ba4948b8 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 30 May 2026 21:42:44 -0400 Subject: [PATCH 1/5] fix(ext/sw): inactivity timer resets on all non-passive messages (Plan C Phase 5) DEV-C P2: an active autofiller never opens the popup, so under the old rule it got force-locked despite continuous use. Inverts the rule: reset on all messages except a documented exclusion set (only get_autofill_candidates today). Co-Authored-By: Claude Opus 4.7 --- .../__tests__/session-timer.test.ts | 27 +++++++++++++++++++ extension/src/service-worker/index.ts | 7 +++-- extension/src/service-worker/session-timer.ts | 14 ++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/extension/src/service-worker/__tests__/session-timer.test.ts b/extension/src/service-worker/__tests__/session-timer.test.ts index 39a56fc..93a04aa 100644 --- a/extension/src/service-worker/__tests__/session-timer.test.ts +++ b/extension/src/service-worker/__tests__/session-timer.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import * as timer from '../session-timer'; +import { READ_ONLY_CONTENT_CALLABLE } from '../session-timer'; describe('session-timer', () => { beforeEach(() => { @@ -97,3 +98,29 @@ describe('session-timer', () => { expect(cb).not.toHaveBeenCalled(); }); }); + +describe('READ_ONLY_CONTENT_CALLABLE — inversion exclusion set', () => { + // The SW handler invokes resetTimer() on every message whose type is NOT + // in this set. These cases encode the documented inversion contract from + // Plan C Phase 5: popup-only messages reset, content-callable writes + // reset, only passive content reads (currently just get_autofill_candidates) + // do NOT reset. + + it('popup-only message would reset the timer (not in exclusion set)', () => { + // e.g. list_items — popup interaction is unambiguously active use + expect(READ_ONLY_CONTENT_CALLABLE.has('list_items')).toBe(false); + }); + + it('content-callable get_autofill_candidates does NOT reset (in exclusion set)', () => { + expect(READ_ONLY_CONTENT_CALLABLE.has('get_autofill_candidates')).toBe(true); + }); + + it('content-callable capture_save_login DOES reset (write op = active use)', () => { + expect(READ_ONLY_CONTENT_CALLABLE.has('capture_save_login')).toBe(false); + }); + + it('content-callable check_credential DOES reset', () => { + // Asking "is this credential already saved" is user-initiated. + expect(READ_ONLY_CONTENT_CALLABLE.has('check_credential')).toBe(false); + }); +}); diff --git a/extension/src/service-worker/index.ts b/extension/src/service-worker/index.ts index e3dec89..cab9a83 100644 --- a/extension/src/service-worker/index.ts +++ b/extension/src/service-worker/index.ts @@ -2,12 +2,12 @@ /// forwards every message into router/index.route(). import type { Request, Response, SessionTimeoutConfig } from '../shared/messages'; -import { CONTENT_CALLABLE_TYPES } from '../shared/messages'; import type { RouterState } from './router/index'; import { route } from './router/index'; import * as vault from './vault'; import { clearCurrent } from './session'; import * as sessionTimer from './session-timer'; +import { READ_ONLY_CONTENT_CALLABLE } from './session-timer'; // @ts-ignore TS2307 — resolved by webpack alias / copy import initDefault, { initSync } from '../../wasm/relicario_wasm.js'; @@ -73,7 +73,10 @@ chrome.commands.onCommand.addListener((command) => { chrome.runtime.onMessage.addListener( (request: Request, sender: chrome.runtime.MessageSender, sendResponse: (r: Response) => void) => { (async () => { - if (!CONTENT_CALLABLE_TYPES.has(request.type as never)) { + // Plan C Phase 5: invert the reset rule. Reset on every message + // except a documented passive-read exclusion set, so an active + // autofiller / content-driven flow keeps the vault alive. + if (!READ_ONLY_CONTENT_CALLABLE.has(request.type)) { sessionTimer.resetTimer(); } if (!state.wasm) { diff --git a/extension/src/service-worker/session-timer.ts b/extension/src/service-worker/session-timer.ts index 9948586..2a7800a 100644 --- a/extension/src/service-worker/session-timer.ts +++ b/extension/src/service-worker/session-timer.ts @@ -48,3 +48,17 @@ export function stopTimer(): void { timerId = null; } } + +/** + * Content-callable message types that should NOT reset the inactivity timer. + * + * Rationale: a content script reading available autofill candidates is a + * passive query — it shouldn't keep the vault alive indefinitely while the + * user isn't actually interacting with it. + * + * Today this is the only known passive read; if a future content message + * is also passive, add it here with a one-line justification. + */ +export const READ_ONLY_CONTENT_CALLABLE: ReadonlySet = new Set([ + 'get_autofill_candidates', +]); From 35444e02be1afe7e8fb0b2f02134a919b501ba25 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 30 May 2026 21:43:50 -0400 Subject: [PATCH 2/5] fix(ext/sw): clear state.gitHost on session expiry (Plan C Phase 5) DEV-C P2: expiry cleared manifest but left the cached git-host client. The initializer rebuilds gitHost on demand, so clearing here is safe. No new test: index.ts has top-level chrome.* side effects that make it expensive to import in a unit test, and the change is a one-liner state mutation in an inline callback. Manually verified by tracing call sites. Co-Authored-By: Claude Opus 4.7 --- extension/src/service-worker/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extension/src/service-worker/index.ts b/extension/src/service-worker/index.ts index cab9a83..2c68a3e 100644 --- a/extension/src/service-worker/index.ts +++ b/extension/src/service-worker/index.ts @@ -53,6 +53,9 @@ sessionTimer.onExpired(() => { console.log('[relicario sw] session expired — locking vault'); clearCurrent(); state.manifest = null; + // Plan C Phase 5: don't leak the cached git-host client across a lock. + // The initializer rebuilds gitHost on demand, so clearing here is safe. + state.gitHost = null; // Best-effort broadcast — receiver may not exist yet. chrome.runtime.sendMessage({ type: 'session_expired' }).catch(() => {}); }); From e43f121dfbf049ed2f5ba51c9843c8af4c287c85 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 30 May 2026 21:44:54 -0400 Subject: [PATCH 3/5] refactor(ext/popup): extract teardownSettingsCommon (Plan C Phase 5) DEV-C P2: settings.ts:56-65 and settings-vault.ts:15-22 had near- identical cleanup paths. Single source for closeGeneratorPanel + activeKeyHandler removal. Helper takes the handler as a parameter and returns null so each caller still owns its own module-scoped handler state. Co-Authored-By: Claude Opus 4.7 --- .../src/popup/components/settings-vault.ts | 7 ++--- extension/src/popup/components/settings.ts | 26 +++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/extension/src/popup/components/settings-vault.ts b/extension/src/popup/components/settings-vault.ts index cc5038e..36add5b 100644 --- a/extension/src/popup/components/settings-vault.ts +++ b/extension/src/popup/components/settings-vault.ts @@ -9,6 +9,7 @@ import type { import type { SessionTimeoutConfig } from '../../shared/messages'; import { relativeTime } from '../../shared/relative-time'; import { openGeneratorPanel, closeGeneratorPanel, isGeneratorPanelOpen } from './generator-panel'; +import { teardownSettingsCommon } from './settings'; import { GLYPH_NEXT } from '../../shared/glyphs'; let pendingSettings: VaultSettings | null = null; @@ -17,11 +18,7 @@ let pendingSession: SessionTimeoutConfig | null = null; let baseSession: SessionTimeoutConfig | null = null; export function teardown(): void { - closeGeneratorPanel(); - if (activeKeyHandler) { - document.removeEventListener('keydown', activeKeyHandler); - activeKeyHandler = null; - } + activeKeyHandler = teardownSettingsCommon(activeKeyHandler); pendingSettings = null; pendingSession = null; baseSession = null; diff --git a/extension/src/popup/components/settings.ts b/extension/src/popup/components/settings.ts index a4021ba..cacd7e6 100644 --- a/extension/src/popup/components/settings.ts +++ b/extension/src/popup/components/settings.ts @@ -53,13 +53,29 @@ export async function renderSettings(container: HTMLElement): Promise { await renderSection(activeSection); } -export function teardownSettings(): void { +/** + * Common cleanup invoked by both the device-settings teardown + * (settings.ts) and the vault-settings teardown (settings-vault.ts). + * Centralized to avoid the "regression class with known prior leaks" + * DEV-C P2 flagged. + * + * Closes the generator popover and detaches the supplied keydown + * handler from the document if present. Returns the new handler value + * (always null), so the caller can do `handler = teardownSettingsCommon(handler)`. + */ +export function teardownSettingsCommon( + keyHandler: ((e: KeyboardEvent) => void) | null, +): null { closeGeneratorPanel(); - teardownSecuritySection(); - if (activeKeyHandler) { - document.removeEventListener('keydown', activeKeyHandler); - activeKeyHandler = null; + if (keyHandler) { + document.removeEventListener('keydown', keyHandler); } + return null; +} + +export function teardownSettings(): void { + activeKeyHandler = teardownSettingsCommon(activeKeyHandler); + teardownSecuritySection(); pendingVaultSettings = null; sessionHandle = null; } From 39fac68fc132815f557355b41486929f2aebb5bc Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 30 May 2026 21:47:15 -0400 Subject: [PATCH 4/5] fix(ext/popup): defensive Promise.allSettled in devices (Plan C Phase 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DEV-C P2: Promise.all meant one rejected RPC failed the whole render. allSettled + per-slot fallback keeps the active-devices surface usable when the revoked-list feed (or one bad ssh fingerprint) is down. Two call sites converted in devices.ts: 1. list_devices + list_revoked pair — revoked failures now render an inline "couldn't load" slot instead of failing the page. 2. sshFingerprint map — one bad public key falls back to '(unknown)' instead of killing the whole device list. trash.ts only has a single sendMessage in its load path on this branch, so it has no Promise.all to migrate. Plan was written against a slightly different snapshot; documented divergence in report. Co-Authored-By: Claude Opus 4.7 --- .../components/__tests__/devices.test.ts | 26 +++++++++ extension/src/popup/components/devices.ts | 57 ++++++++++++++----- 2 files changed, 70 insertions(+), 13 deletions(-) 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}
`; From fce1962315c301bf99239c3dc20c27d851a52f84 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 30 May 2026 21:48:08 -0400 Subject: [PATCH 5/5] perf(ext/content): debounce MutationObserver scan() to 200ms (Plan C Phase 5) DEV-C P2: SPA churn was re-running the full scan many times per second. Trailing-edge debounce coalesces bursts so scan() runs at most once per quiet 200ms window. No test harness exists for content/detector.ts on this branch; relies on manual verification on a real SPA page. Co-Authored-By: Claude Opus 4.7 --- extension/src/content/detector.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/extension/src/content/detector.ts b/extension/src/content/detector.ts index f369f0e..bfc19d8 100644 --- a/extension/src/content/detector.ts +++ b/extension/src/content/detector.ts @@ -93,9 +93,21 @@ setupFillListener(); scan(); // Watch for DOM changes (SPA navigation, dynamically loaded forms). -const observer = new MutationObserver(() => { - scan(); -}); +// Plan C Phase 5: SPA churn fires the MutationObserver many times per +// second. Trailing-edge debounce coalesces bursts so we run the full +// scan() at most once per quiet 200ms window. +const SCAN_DEBOUNCE_MS = 200; +let scanTimer: ReturnType | undefined; + +function scheduleScan(): void { + if (scanTimer !== undefined) clearTimeout(scanTimer); + scanTimer = setTimeout(() => { + scanTimer = undefined; + scan(); + }, SCAN_DEBOUNCE_MS); +} + +const observer = new MutationObserver(scheduleScan); observer.observe(document.body, { childList: true,