# DEV-C Architecture Review Notes — TypeScript (Extension + Relay) **Reviewer:** dev-c (TypeScript scope) **Date:** 2026-05-04 **Branch:** main (read-only review) ## Summary The TypeScript layer is **soundly organized but unevenly distributed**: a tight discriminated-union message contract (`shared/messages.ts`) and a clean Manifest V3 trust split (popup/setup/content/SW) anchor a codebase that earns its complexity from the two-factor unlock UX. The strongest part is the **service-worker router**: every popup/content message has a typed handler with origin-aware gating, and content scripts never touch WASM. The weakest parts are **two oversized modules that pre-empt the otherwise-clean component model**: `extension/src/setup/setup.ts` (1220 LOC) bypasses the SW and orchestrates WASM directly, and `extension/src/vault/vault.ts` (1027 LOC) inlines shell, sidebar, list, drawer, type-picker, form-wrapper and routing into one file. **CLI/extension parity is excellent overall** — every CLI subcommand has a UI path through the unified left-nav settings (v0.5.1) or vault tab — with two real gaps: explicit per-attachment detach (extension does it via `update_item`) and a vault-status surface equivalent to `relicario status`. One **broken test** in uncommitted relay code (`tools/relay/queue.test.ts:54`) blocks `bun test` from going green. ## Findings (prioritized: P1 / P2 / P3) ### Extension — service-worker **[P1] router/popup-only.ts:687–703 & router/content-callable.ts:187–205 — duplicated storage helpers (`loadDeviceSettings`, `loadBlacklist`, `saveBlacklist`)** WHY: Three identical chrome.storage.local helpers in both router files; both code paths can mutate blacklist via different definitions, so future drift will silently corrupt one path. DIRECTION: Extract to `service-worker/storage.ts`; import from both routers. **[P1] router/popup-only.ts:~169 & router/content-callable.ts:~169 — `itemToManifestEntry()` duplicated** WHY: Identical 17-line manifest projection in both router files; refactors of the manifest schema will need to be made twice. DIRECTION: Move into `service-worker/vault.ts` and import. **[P2] service-worker/index.ts:76–78 — inactivity timer reset skips content-callable messages** WHY: Only popup/vault sends reset the timer; a user who is actively autofilling but never opens the popup will eventually be force-locked despite continuous use. Conversely, "every_time" mode is fine. DIRECTION: Reset on all messages except known read-only content calls (`get_autofill_candidates`); document the exclusion in the timer module. **[P2] service-worker/index.ts:51–58 — session expiry clears `state.manifest` but leaves `state.gitHost`** WHY: After expiry, the cached git-host client survives; the next unlock could mix with stale connection state if a remote was rotated. DIRECTION: Null `state.gitHost` alongside `state.manifest` in the expiry callback; the initializer rebuilds it on demand. **[P2] service-worker/session.ts:26 — `try { current.free() }` swallows free errors** WHY: A double-free or invalid-handle on the WASM session would be silently ignored, masking a lifecycle bug that could leave the master key zeroed but the JS handle dangling. DIRECTION: Let exceptions propagate (or log + counter); silent swallow is the wrong default for crypto state transitions. **[P3] service-worker/index.ts:61–65 — chrome.storage.local.get on startup swallows errors** WHY: A typo or storage quota event leaves the timer in default state with no signal in the logs. DIRECTION: Surface failure via console.warn with the key name. **[P3] service-worker/git-host.ts vs github.ts/gitea.ts — deploy-key API only on Gitea side** WHY: The interface is asymmetric; either GitHub doesn't need it (document) or it's an unimplemented feature. DIRECTION: Add a doc comment on `GitHost` explaining the asymmetry, or add the GitHub op. ### Extension — popup + components **[P1] settings.ts:56–65 & settings-vault.ts:15–22 — duplicated teardown helpers** WHY: After the stub-restore commits (`8baef5b`, `ddfb95d`), teardown leaks are a known regression class; duplicated cleanup is exactly the shape that re-introduces them. DIRECTION: Extract `teardownSettingsCommon()` into a single helper; have each settings section call it on unmount. **[P2] popup.ts:178–181 — teardown calls every type module unconditionally** WHY: Cycling views runs all 7 type teardowns on each transition; harmless today, but makes the lifecycle hard to reason about and hides which module was actually mounted. DIRECTION: Track last-mounted type and tear down only that one (or use a registry of `() => teardown` returned by mount). **[P2] settings.ts:33–34 — `pendingVaultSettings` and `sessionHandle` are module-scope singletons** WHY: Section navigation can leave stale `pendingVaultSettings` from a previous section in scope; the current code is safe by accident, not by design. DIRECTION: Reset to `null` on each `renderSection` entry, or scope into a closure per render. **[P2] item-list.ts:152–353 — settings-picker popover wires listeners on every render without a reuse path** WHY: Open/close cycles attach + detach listeners cleanly today, but rapid re-opens with throttled `setTimeout` cleanup are exactly the pattern teardown bugs hide in. DIRECTION: Cache the popover DOM and reuse, or use AbortController for the listeners with an explicit `signal` per open. **[P2] devices.ts:47–50 & trash.ts:39–46 — `Promise.all` without per-promise error handling** WHY: A single rejected RPC fails the whole render; the second response is discarded even when its data was fetched successfully. DIRECTION: `Promise.allSettled`, then check `.ok` per response. **[P2] generator-panel.ts:89–261 — module-scope `activePanel`; `closeGeneratorPanel()` not idempotent-guarded** WHY: The cleanup is currently a no-op when called twice, but the contract is implicit. DIRECTION: Add `if (!activePanel) return` at top. **[P3] form-header.ts:20 + types/login.ts — `isInTab()` checked twice (header + caller)** WHY: Redundant coupling; the popout button visibility decision happens in two places. DIRECTION: Pass `showPopout: boolean` into `renderFormHeader` once. **[P3] popup.ts:36–38 — `isInTab()` heuristic is `window.location.search.length > 0`** WHY: Any query string on `popup.html` triggers tab mode; brittle if popup is ever opened with diagnostic params. DIRECTION: Parse a specific param (`?view=tab` or check `window.location.pathname.endsWith('vault.html')`). **[P3] item-form.ts:95–104 — `renderComingSoon()` defined but unused** WHY: Document type now has a real form (`types/document.ts`); the placeholder is dead code. DIRECTION: Delete or leave a one-line comment explaining future use. **[P3] types/login.ts ~700 LOC — `renderForm` mixes HTML, wiring, password-strength, TOTP-ticker, and section editor** WHY: The reference implementation for the other 6 type modules; size makes the per-affordance lifecycle hard to follow for a learner. DIRECTION: Extract `wireUrlField`, `wirePasswordField`, `wireTotpField` from the body of `renderForm`. Other type modules already follow simpler patterns. ### Extension — vault tab **[P1] vault/vault.ts (1027 LOC) — single file owns shell + sidebar + list + drawer + type-picker + form-wrapper + routing + teardown** WHY: A learner opening this file faces all responsibilities at once; teardown coupling (`teardownPaneComponents` lines 813–820) makes adding a new pane view a 5-place edit. DIRECTION: Split into `vault-shell.ts`, `vault-sidebar.ts`, `vault-list.ts`, `vault-drawer.ts`, `vault-form-wrapper.ts`, leaving `vault.ts` to own only routing and state. **[P2] vault/vault.ts:47–74 — vault tab intercepts `vault_locked` errors via the RPC layer; popup uses only the `session_expired` event** WHY: Two different mechanisms reach the same outcome; popup-targeted components running in the vault tab don't know which channel will fire first. DIRECTION: Lift the RPC `vault_locked` intercept into `shared/state.ts` (or a wrapper around `sendMessage`) so both surfaces use one path. **[P2] vault/vault.ts:495–536 — drawer doesn't auto-close on non-list view changes** WHY: Selecting an item opens the drawer; navigating to Trash via the sidebar leaves `state.drawerOpen = true` even though the drawer is no longer rendered. DIRECTION: `state.drawerOpen = false` at the start of `renderPane`, or when a non-list view becomes active. **[P2] vault/vault.ts:648–695 — sidebar categories re-render on every search keystroke without debounce** WHY: Counts and active state must sync, but at hundreds of items the keystroke path stamps the whole sidebar. DIRECTION: 50–100ms debounce on the search input handler before re-rendering. **[P3] vault/vault.ts:18–26 — backup-panel and import-panel are `vault/components/`-only by design** WHY: Correct call (popup has no room for these), but worth noting in the file header so a contributor doesn't try to import them into popup. DIRECTION: Add a short comment in `vault/components/index.ts` (or top of each file) explaining the vault-tab-only scope. ### Extension — content scripts **[P2] content/fill.ts:50–64 — `fillFields()` returns silently when no password field is found** WHY: Dynamic forms (React/SPA mounting after `document_idle`) can race the fill listener; user clicks autofill, nothing visible happens, no error reaches the popup. DIRECTION: Send a `{type: 'fill_failed', reason: 'no_password_field'}` ack back to SW so the icon can flash an error glyph. **[P2] content/detector.ts:96–103 — MutationObserver `scan()` is not debounced** WHY: SPA churn fires DOM mutations many times per second; the detector re-runs the full scan each time. WeakSet stops icon double-injection, but the scan cost is wasted. DIRECTION: Wrap `scan()` in `requestIdleCallback` or a 200ms timer. **[P2] content/icon.ts:169–175 — outside-click listener registered in `setTimeout(0)` not removed on alternate close paths** WHY: Picker can close via Escape or programmatic destroy; the doc-level listener for outside-click leaks unless `closeOverlay()` is the close path. DIRECTION: Store the handler reference and `removeEventListener` in every close path; or `AbortController` scoped to picker open. **[P3] capture.ts vs detector.ts vs fill.ts — username-finder logic copy-pasted three ways** WHY: `findUsernameField`, `findUsernameForFill`, `findUsernameValue` are three forks of the same heuristic. DIRECTION: Extract a shared `getUsernameField(pwField, scope, { valueOnly?: boolean })`. **[P3] capture.ts:269–299 — submit-button hook scope is `form ?? pwField.parentElement`** WHY: A submit button outside the form (e.g. a custom React `