Merge phase-c-5-p2-cluster: Plan C Phase 5 (P2 cluster — 5 small fixes)

5 commits landing 5 independent P2 fixes:
- ba5d218 inactivity timer resets on all non-passive messages (READ_ONLY_CONTENT_CALLABLE exclusion set in session-timer.ts; index.ts inverts the gate)
- 35444e0 state.gitHost cleared on session expiry (alongside state.manifest)
- e43f121 teardownSettingsCommon extracted; both settings.ts + settings-vault.ts call it (parameterized over each file's own activeKeyHandler module variable)
- 39fac68 Promise.allSettled with per-slot fallback in devices.ts (list_devices+list_revoked + sshFingerprint map). trash.ts is a no-op on this branch — it doesn't have a Promise.all to migrate (single list_trashed call); plan was written against a different snapshot.
- fce1962 MutationObserver scan() debounced to 200ms in content/detector.ts (no test harness on this branch — manual verification per plan note)

377/377 vitest tests pass (baseline 371 + 6 new tests in session-timer + devices). Zero regressions.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
adlee-was-taken
2026-05-30 21:49:17 -04:00
8 changed files with 157 additions and 28 deletions

View File

@@ -93,9 +93,21 @@ setupFillListener();
scan(); scan();
// Watch for DOM changes (SPA navigation, dynamically loaded forms). // Watch for DOM changes (SPA navigation, dynamically loaded forms).
const observer = new MutationObserver(() => { // Plan C Phase 5: SPA churn fires the MutationObserver many times per
scan(); // 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<typeof setTimeout> | 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, { observer.observe(document.body, {
childList: true, childList: true,

View File

@@ -95,6 +95,32 @@ describe('devices view', () => {
expect(app.querySelector<HTMLButtonElement>('#register-confirm-btn')).not.toBeNull(); expect(app.querySelector<HTMLButtonElement>('#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<typeof vi.fn>)
.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<typeof vi.fn>)
.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 () => { it('confirming register sends register_this_device with the entered name', async () => {
(chrome.storage.local.get as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ device_name: 'Unknown' }); (chrome.storage.local.get as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ device_name: 'Unknown' });
// Initial render: list_devices + list_revoked. // Initial render: list_devices + list_revoked.

View File

@@ -31,35 +31,64 @@ export function teardown(): void {
// No cleanup needed // 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 `
<details class="revoked-section">
<summary class="muted">▸ revoked devices</summary>
<div class="revoked-section__body">
<p class="muted">Couldn't load revoked devices.</p>
</div>
</details>
`;
}
export async function renderDevices(app: HTMLElement): Promise<void> { export async function renderDevices(app: HTMLElement): Promise<void> {
// Get current device name from local storage // Get current device name from local storage
const stored = await chrome.storage.local.get(['device_name']); const stored = await chrome.storage.local.get(['device_name']);
const currentDeviceName: string | undefined = stored.device_name as string | undefined; const currentDeviceName: string | undefined = stored.device_name as string | undefined;
// Fetch active device list and revoked list in parallel // Fetch active device list and revoked list in parallel. allSettled so a
const [devicesResp, revokedResp] = await Promise.all([ // rejected secondary feed doesn't kill the whole render.
const [devicesSettled, revokedSettled] = await Promise.allSettled([
sendMessage({ type: 'list_devices' }), sendMessage({ type: 'list_devices' }),
sendMessage({ type: 'list_revoked' }), sendMessage({ type: 'list_revoked' }),
]); ]);
if (!devicesResp.ok) { if (devicesSettled.status === 'rejected' || !devicesSettled.value.ok) {
app.innerHTML = `<div class="pad"><p class="error">Failed to load devices</p></div>`; app.innerHTML = `<div class="pad"><p class="error">Failed to load devices</p></div>`;
return; return;
} }
const devices = (devicesResp.data as { devices: Device[] }).devices; // devicesSettled.value.ok is true here (guarded above), so .data is present.
const revokedDevices: RevokedEntry[] = revokedResp.ok const devicesData = (devicesSettled.value as { ok: true; data: unknown }).data;
? (revokedResp.data as { revoked: RevokedEntry[] }).revoked 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); 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<string, string>(); const fingerprints = new Map<string, string>();
await Promise.all(devices.map(async (d) => { const fpResults = await Promise.allSettled(
const fp = await sshFingerprint(d.public_key); devices.map((d) => sshFingerprint(d.public_key).then((fp) => [d.name, fp] as const)),
fingerprints.set(d.name, fp ?? '(unknown)'); );
})); 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 const activeDevicesHtml = devices.length === 0
? `<p class="muted" style="text-align:center;margin-top:32px;">No devices registered</p>` ? `<p class="muted" style="text-align:center;margin-top:32px;">No devices registered</p>`
@@ -82,7 +111,9 @@ export async function renderDevices(app: HTMLElement): Promise<void> {
`; `;
}).join(''); }).join('');
const revokedSectionHtml = revokedDevices.length === 0 ? '' : ` const revokedSectionHtml = !revokedOk
? revokedLoadErrorHtml()
: revokedDevices.length === 0 ? '' : `
<details class="revoked-section"> <details class="revoked-section">
<summary class="muted">▸ show ${revokedDevices.length} revoked device${revokedDevices.length !== 1 ? 's' : ''}</summary> <summary class="muted">▸ show ${revokedDevices.length} revoked device${revokedDevices.length !== 1 ? 's' : ''}</summary>
<div class="revoked-section__body"> <div class="revoked-section__body">
@@ -117,7 +148,7 @@ export async function renderDevices(app: HTMLElement): Promise<void> {
` : ''} ` : ''}
${devices.length > 0 ? `<div class="section-header">ACTIVE · ${devices.length}</div>` : ''} ${devices.length > 0 ? `<div class="section-header">ACTIVE · ${devices.length}</div>` : ''}
${activeDevicesHtml} ${activeDevicesHtml}
${revokedDevices.length > 0 ? `<div class="section-header">REVOKED · ${revokedDevices.length}</div>` : ''} ${!revokedOk ? `<div class="section-header">REVOKED · ?</div>` : (revokedDevices.length > 0 ? `<div class="section-header">REVOKED · ${revokedDevices.length}</div>` : '')}
${revokedSectionHtml} ${revokedSectionHtml}
</div> </div>
`; `;

View File

@@ -9,6 +9,7 @@ import type {
import type { SessionTimeoutConfig } from '../../shared/messages'; import type { SessionTimeoutConfig } from '../../shared/messages';
import { relativeTime } from '../../shared/relative-time'; import { relativeTime } from '../../shared/relative-time';
import { openGeneratorPanel, closeGeneratorPanel, isGeneratorPanelOpen } from './generator-panel'; import { openGeneratorPanel, closeGeneratorPanel, isGeneratorPanelOpen } from './generator-panel';
import { teardownSettingsCommon } from './settings';
import { GLYPH_NEXT } from '../../shared/glyphs'; import { GLYPH_NEXT } from '../../shared/glyphs';
let pendingSettings: VaultSettings | null = null; let pendingSettings: VaultSettings | null = null;
@@ -17,11 +18,7 @@ let pendingSession: SessionTimeoutConfig | null = null;
let baseSession: SessionTimeoutConfig | null = null; let baseSession: SessionTimeoutConfig | null = null;
export function teardown(): void { export function teardown(): void {
closeGeneratorPanel(); activeKeyHandler = teardownSettingsCommon(activeKeyHandler);
if (activeKeyHandler) {
document.removeEventListener('keydown', activeKeyHandler);
activeKeyHandler = null;
}
pendingSettings = null; pendingSettings = null;
pendingSession = null; pendingSession = null;
baseSession = null; baseSession = null;

View File

@@ -53,13 +53,29 @@ export async function renderSettings(container: HTMLElement): Promise<void> {
await renderSection(activeSection); 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(); closeGeneratorPanel();
teardownSecuritySection(); if (keyHandler) {
if (activeKeyHandler) { document.removeEventListener('keydown', keyHandler);
document.removeEventListener('keydown', activeKeyHandler);
activeKeyHandler = null;
} }
return null;
}
export function teardownSettings(): void {
activeKeyHandler = teardownSettingsCommon(activeKeyHandler);
teardownSecuritySection();
pendingVaultSettings = null; pendingVaultSettings = null;
sessionHandle = null; sessionHandle = null;
} }

View File

@@ -1,5 +1,6 @@
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
import * as timer from '../session-timer'; import * as timer from '../session-timer';
import { READ_ONLY_CONTENT_CALLABLE } from '../session-timer';
describe('session-timer', () => { describe('session-timer', () => {
beforeEach(() => { beforeEach(() => {
@@ -97,3 +98,29 @@ describe('session-timer', () => {
expect(cb).not.toHaveBeenCalled(); 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);
});
});

View File

@@ -2,12 +2,12 @@
/// forwards every message into router/index.route(). /// forwards every message into router/index.route().
import type { Request, Response, SessionTimeoutConfig } from '../shared/messages'; import type { Request, Response, SessionTimeoutConfig } from '../shared/messages';
import { CONTENT_CALLABLE_TYPES } from '../shared/messages';
import type { RouterState } from './router/index'; import type { RouterState } from './router/index';
import { route } from './router/index'; import { route } from './router/index';
import * as vault from './vault'; import * as vault from './vault';
import { clearCurrent } from './session'; import { clearCurrent } from './session';
import * as sessionTimer from './session-timer'; import * as sessionTimer from './session-timer';
import { READ_ONLY_CONTENT_CALLABLE } from './session-timer';
// @ts-ignore TS2307 — resolved by webpack alias / copy // @ts-ignore TS2307 — resolved by webpack alias / copy
import initDefault, { initSync } from '../../wasm/relicario_wasm.js'; import initDefault, { initSync } from '../../wasm/relicario_wasm.js';
@@ -53,6 +53,9 @@ sessionTimer.onExpired(() => {
console.log('[relicario sw] session expired — locking vault'); console.log('[relicario sw] session expired — locking vault');
clearCurrent(); clearCurrent();
state.manifest = null; 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. // Best-effort broadcast — receiver may not exist yet.
chrome.runtime.sendMessage({ type: 'session_expired' }).catch(() => {}); chrome.runtime.sendMessage({ type: 'session_expired' }).catch(() => {});
}); });
@@ -73,7 +76,10 @@ chrome.commands.onCommand.addListener((command) => {
chrome.runtime.onMessage.addListener( chrome.runtime.onMessage.addListener(
(request: Request, sender: chrome.runtime.MessageSender, sendResponse: (r: Response) => void) => { (request: Request, sender: chrome.runtime.MessageSender, sendResponse: (r: Response) => void) => {
(async () => { (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(); sessionTimer.resetTimer();
} }
if (!state.wasm) { if (!state.wasm) {

View File

@@ -48,3 +48,17 @@ export function stopTimer(): void {
timerId = null; 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<string> = new Set([
'get_autofill_candidates',
]);