relicario/docs/superpowers/reviews/2026-05-04-dev-c-notes.md

# 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 `<button onClick={...}>` siblings to the form) won't trigger capture; the user never sees the save prompt.
DIRECTION: Walk up to the nearest form ancestor and listen for `keydown.Enter` on the password field as a fallback.

**[Positive note] content scripts make zero direct WASM calls and re-validate origin in `fill.ts:32–38`** — boundary discipline holds.

### Extension — setup

**[P1] setup/setup.ts:28–37, 1118–1120 — WASM is loaded and called directly inside the setup wizard, bypassing the service-worker abstraction the rest of the codebase uses**
WHY: Popup, vault tab, and content all funnel WASM through the SW; setup is the only surface that imports `relicario-wasm` and orchestrates `unlock`/`embed_image_secret`/`register_device`/`manifest_encrypt` itself. This duplicates ~400 LOC of crypto orchestration that the SW already knows how to do, and it means the setup tab can't be locked by the same session-timer the rest of the extension uses.
DIRECTION: Add `create_vault` and `attach_vault` messages to the SW; turn `setup.ts` into a UI that posts those messages with the gathered config + image bytes. This collapses ~600 LOC into ~200 and unifies the crypto state machine.

**[P2] setup/setup.ts (1220 LOC) — render/attach pairs per step are inlined**
WHY: 6 steps × (renderStepN, attachStepN) = a switch statement that reads procedurally; adding a step means edits to the renderer, the attacher, and the wizard state.
DIRECTION: Step registry: `{ id: 0, render: () => string, attach: (root) => Teardown }[]` keyed by mode. The 1220 LOC drops to ~500 after the WASM extraction in [P1] above and this restructure.

**[P2] setup/setup.ts:69–94 — `WizardState` is module-scope; passphrase + JPEG bytes + WASM handle persist if the user abandons mid-wizard**
WHY: A user who closes the setup tab mid-flow leaves sensitive material in JS memory until the page is unloaded. The handle isn't `free()`d.
DIRECTION: Add a `clearWizardState()` called on `beforeunload` and on returning to step 0; explicitly `Zeroize`-equivalent for the byte arrays where possible.

**[P2] setup/probe.ts:11–23 vs service-worker/vault.ts — manifest path constants duplicated (`.relicario/salt`, `.relicario/params.json`, `manifest.enc`)**
WHY: Two sources of truth; if the metadata layout ever moves, probe and vault disagree about whether a remote is initialized.
DIRECTION: Define `VAULT_PATHS` in `shared/types.ts` (or a new `shared/paths.ts`) and import into both.

**[P3] setup/setup.ts:56, 82–83 — passphrase score uses `-1` as "not yet scored" sentinel**
WHY: The score is conflated with strength label index 0..4; `-1` magic number is checked in two places.
DIRECTION: Use `{ scored: false } | { scored: true; score: number; guessesLog10: number }`.

**[P3] setup/setup.ts:1056–1062 — `recovery_qr_generated_at` written directly to `chrome.storage.local`, bypassing `WizardState`**
WHY: One piece of setup result state lives outside the wizard's own state model.
DIRECTION: Add it to `WizardState`, persist on a single `finishSetup` storage write.

**[P3] setup/setup.ts:1–7 — header comment says "5-step flow"; code is 6 steps (0..5)**
WHY: Cosmetic but actively misleading; line 42 even has a comment about the renumber.
DIRECTION: Update header.

### Extension — shared utilities

**[P1] shared/state.ts:10–35 — entire `StateHost` contract is `any`-typed**
WHY: This is the bridge that lets popup components run in the vault tab; it's also the bridge most likely to drift between the two render targets, and TS gives no signal when it does.
DIRECTION: Define a concrete `StateHost` interface with `state: PopupState`, `navigate: (view: View) => void`, `popOutToTab(): void`, `isInTab(): boolean`, `openVaultTab(hash?: string): void`. Make `getState`/`setState` generic over `keyof PopupState`.

**[P1] shared/state.ts:20 — module-scope `host` singleton with no double-registration guard**
WHY: A second `registerHost()` call silently overwrites; in tests, the leak from a previous test breaks isolation if `beforeEach` forgets a reset.
DIRECTION: Throw on re-register; export a `__resetHostForTests()` helper for vitest.

**[P2] shared/messages.ts:85–87 — base `Response` is `{ data?: unknown }`; every consumer casts via `Extract<Response, { ok: true }>` plus `data: ...`**
WHY: The pattern works but every call site has a hand-written `as ListItemsResponse` cast; the discriminated-union narrowing the rest of the file relies on stops at `ok: true`.
DIRECTION: Generic `Response<TKind extends Request['type']>` mapped from a single `MessageMap` table, so the response type is inferred at the send site.

**[P2] shared/form-affordances/group-autocomplete.ts:26 — `list.innerHTML = ...map(g => …).join('')` with only `replace(/"/g, '&quot;')` sanitization**
WHY: Group names come from user-entered item data. `<` and `>` are not escaped; a malicious / mistaken group name could inject markup into the autocomplete list.
DIRECTION: `document.createElement('option')` per group with `option.value = g; list.appendChild(option)`.

**[P2] shared/messages.ts:56–66 — `restore_backup` flattens `newRemote` inline; other multi-field messages factor out a payload type**
WHY: Inconsistent shape vs sibling messages; harder to reuse the type for the form that posts it.
DIRECTION: Extract `RestoreBackupPayload` and intersect with `{ type: 'restore_backup' }`.

**[P3] glyphs.ts vs popup/components — raw glyph literals (`⧉`, `↻`, `▸`, `▾`, `≡`, `⤓`) appear inline in `item-form.ts`, `item-list.ts`, `settings.ts`, `generator-panel.ts`, `attachments-disclosure.ts`, `fields.ts`**
WHY: The whole point of `glyphs.ts` is the no-inline-emoji rule; partial adoption defeats the audit story.
DIRECTION: Add the missing constants (`GLYPH_COLLAPSE`, `GLYPH_EXPAND`, `GLYPH_ATTACHMENT`, `GLYPH_REGENERATE`, `GLYPH_OVERFLOW`, `GLYPH_DOWNLOAD`) and migrate the call sites. Pair with `extension/src/shared/__tests__/glyphs.test.ts` to lock it in.

**[P3] shared/types.ts:82 — `TotpKind = 'totp' | 'steam' | { hotp: { counter: number } }`**
WHY: Mixed string/object union forces every consumer to do `typeof k === 'string' ? k : k.hotp`; a flat discriminated union is cheaper to read.
DIRECTION: `{ kind: 'totp' } | { kind: 'steam' } | { kind: 'hotp'; counter: number }` with serde rename if the wire format must stay.

**[P3] shared/form-affordances/totp-tools.ts:39–46 — `void tick()` swallows promise rejections**
WHY: TOTP preview RPC can fail (e.g., decrypt error); the user sees a frozen ticker with no signal.
DIRECTION: `try { await tick() } catch (e) { renderError(row, e) }`.

### WASM boundary (JS side)

**[P2] extension/src/wasm.d.ts — declarations are hand-written and explicitly require manual sync with `crates/relicario-wasm/src/lib.rs`**
WHY: Comment at top of the file says so; this is exactly the kind of contract that drifts when one side adds a new export. (Today, declarations and the Rust signatures are aligned.)
DIRECTION: Add a CI check that compares `crates/relicario-wasm/pkg/relicario_wasm.d.ts` (wasm-pack output) against `extension/src/wasm.d.ts`, or import the generated `.d.ts` directly via `wasm-pack build --target web` and the runtime loader. Note for DEV-B in boundary section.

**[P2] extension/src/__stubs__/relicario_wasm.stub.ts — only 7 of ~25 exports are stubbed; the rest will throw `wasm stub: X not mocked`**
WHY: Adding a vitest test that touches a new WASM call needs an ad-hoc mock per test; central stub is incomplete.
DIRECTION: Either round out the stub with throwing stubs for the full surface, or provide a `mockWasm({ unlock, item_decrypt, ... })` test helper.

### Relay tooling

**[P1] tools/relay/queue.test.ts:54 — `assert.ok(!isRole("dev-c"))` fails (verified: `bun test` → 1 fail)**
WHY: Uncommitted change added `dev-c` to the `Role` union and `KNOWN_ROLES` set in `queue.ts`, but the test still asserts the old enum. This is a P1 because it's a real test failure on uncommitted code, blocking a green test run.
DIRECTION: Update the test assertion to `assert.ok(isRole("dev-c"))` and add a negative case (`assert.ok(!isRole("dev-d"))`).

**[P1] tools/relay/start.sh — kitty mode launches Dev-C window? Verify against the script**
WHY: The 4-role refactor (pm/dev-a/dev-b/dev-c) needs a fourth window in the launcher. Per the subagent's read, `start.sh:80` still hardcodes "Dev-B" in user-facing output.
DIRECTION: Update the launcher prints to mention all 4 roles and confirm a 4th `--type=tab --hold` window is opened with the dev-c prompt.

**[P2] tools/relay/call.py and tools/relay/call.ts — untracked, no .gitignore policy**
WHY: Coordination prompts (including this one's "Fallback" section) reference `call.py` by path — it's load-bearing for the multi-agent flow, not an experiment. Untracked load-bearing files are a coordination footgun (a fresh checkout breaks the fallback).
DIRECTION: Track both with a one-line header explaining "MCP-fallback shim for the relay; see docs/superpowers/coordination/...". If either is genuinely scratch, add to `.gitignore` instead — but pick one.

**[P2] tools/relay/queue.ts:21–27 — in-memory queue with no TTL, persistence, or cap**
WHY: A long-running relay accumulates messages indefinitely; if a session sleeps for a day, the queue is the only audit trail and could grow unbounded.
DIRECTION: Document the dev-only ephemeral contract at the top of `queue.ts`, or add a per-role cap (e.g., last 1000 messages).

**[P3] tools/relay/server.ts:115–127 — `makeServer()` factory pattern is correct but unexplained**
WHY: The diff swap from a global `mcpServer` to per-connection factories is load-bearing for concurrent client isolation, but a future contributor might revert it as "unnecessary complexity".
DIRECTION: One-line comment above `makeServer`: "Per-connection MCP server prevents routing collisions across concurrent SSE clients."

### CLI/extension parity gaps

**[P2] No equivalent to `relicario status`** — CLI shows pending sync state, ahead/behind, dirty-tree summary; extension surfaces nothing comparable. The vault-tab footer or a sidebar badge would be the natural home.
DIRECTION: Add a `get_vault_status` message returning `{ ahead: n, behind: n, lastSyncAt: ts, pendingItems: n }` and a small status indicator in the vault sidebar.

**[P3] No explicit per-attachment detach message** — CLI has `relicario detach <item> <aid>`; extension forces a roundtrip through `update_item` with the `attachments[]` mutated client-side. Functional, but it's racy if two devices edit at once.
DIRECTION: Add a `delete_attachment` message that does the surgical remove on the SW side.

**[P3] CLI `list --tag X` filter** — extension does tag filtering client-side after `list_items`; that's fine for the family-vault scale spec but may surprise a learner who reads the CLI side first.
DIRECTION: Document the choice in `messages.ts` near `list_items`.

### Cross-cutting

**[P2] Direct `chrome.storage.local` reads from popup components** — `settings-security.ts:112–113` and `setup.ts:1056–1062` read storage directly while every other popup module routes via `sendMessage`. Inconsistency makes the data-flow story split across two paradigms.
DIRECTION: Pick one: either every storage read goes through `get_settings`/`get_session_config`, or document explicitly that `setup` and `settings-security` are SW-bypass code paths for stated reasons.

**[P2] `bun test` is not the project's intended runner** — `extension/package.json:13` defines `test: vitest run`, but `tools/relay/` uses `node:test` via `bun test`. A learner will hit failures running `bun test` from the repo root because `bun` doesn't load `happy-dom`.
DIRECTION: Add a top-level README note: "extension uses `cd extension && npm run test`; relay uses `cd tools/relay && bun test`."

**[P3] Manifest version 0.5.0** — both `extension/manifest.json` and `manifest.firefox.json` show `0.5.0`, while `package.json` is also `0.5.0`. The roadmap is at v0.5.1-dev; uncommitted bumps to package.json/manifest may be coming. Worth a quick PM check.

## File-by-file walk

### `extension/src/wasm.d.ts` and `__stubs__/relicario_wasm.stub.ts`
Hand-written ambient module declarations mirroring `crates/relicario-wasm/src/lib.rs`. 25+ exports cover unlock/lock, manifest+item+settings encrypt/decrypt, attachment encrypt/decrypt, ID generation, password/passphrase generation + zxcvbn rate, image-secret embed/extract, TOTP compute, device register/sign/get/clear, recovery-QR. The stub used by vitest covers only 7 exports — every other call throws "not mocked" at test time.

### `extension/src/shared/`
`types.ts` (286 LOC) is the canonical TS view of the Rust core schema; well-commented mappings to serde. `messages.ts` (207 LOC) is the discriminated-union message contract and the `POPUP_ONLY_TYPES` / `CONTENT_CALLABLE_TYPES` capability sets that the router uses for sender gating. `state.ts` (62 LOC) is the popup-vs-vault host indirection — currently `any`-typed. `glyphs.ts` (39 LOC, recently edited) is the centralized monospace glyph set. `color-scheme.ts`, `error-copy.ts`, `base32.ts`, `password-coloring.ts`, `toast.ts` are tight focused utilities. `form-affordances/` holds 5 input-behavior wirings (group autocomplete, notes mono toggle, password reveal/strength/QR, TOTP preview/QR, URL fill) plus a tiny `index.ts`.

### `extension/src/service-worker/`
`index.ts` is the entry point: WASM load on first message, RouterState construction, session-timer wiring, `chrome.runtime.onMessage` listener. `vault.ts` (397 LOC) is the encrypted I/O layer — every WASM call passes the `SessionHandle`, never a raw key. `session.ts` is the single-handle store; `session-timer.ts` implements both `inactivity` and `every_time` modes. `devices.ts` reads/writes `.relicario/devices.json` and `revoked.json`. `gitea.ts` and `github.ts` mirror each other (Gitea has deploy-key ops; GitHub doesn't); `git-host.ts` is the abstraction. `router/index.ts` classifies the sender and dispatches to `popup-only.ts` (40+ handlers) or `content-callable.ts` (5 handlers).

### `extension/src/popup/`
`popup.ts` (entry) wires the host registration, view router, and lock/unlock flow. `index.html` is a single `#popup-app` container. `styles.css` carries the popup theme. `components/` holds the visible widgets: `unlock`, `item-list`, `item-detail`, `item-form`, `form-header`, `fields`, `field-history`, `attachments-disclosure`, `generator-panel`, `devices`, `trash`, plus the new `settings.ts` / `settings-vault.ts` / `settings-security.ts` left-nav (v0.5.1). `components/types/` holds the 7 type-specific item modules — `login.ts` is the reference, the others (secure-note, identity, card, key, document, totp) are smaller variants.

### `extension/src/vault/`
`vault.html` (12 LOC) loads `vault.css` + `vault.js`. `vault.css` (~2200 LOC) carries the dark/gold theme + 3-column layout. `vault.ts` (1027 LOC) orchestrates everything: shell init, hash routing, sidebar, list, drawer, type-picker, form-wrapper, deep-link routing, teardown. `components/backup-panel.ts` and `components/import-panel.ts` are vault-tab-only (high-risk operations need fullscreen affordances). Tests: `form-wrapper.test.ts`, `sidebar-glyphs.test.ts`.

### `extension/src/content/`
`detector.ts` finds password fields and injects icons via a MutationObserver. `fill.ts` receives `fill_credentials`, re-validates the page hostname, and uses the native-setter trick to set values past React/Vue. `capture.ts` hooks form submits and shows a closed-Shadow-DOM save prompt. `icon.ts` renders the in-page icon and credentials picker. `shadow.ts` is the closed-Shadow-DOM helper. **Zero WASM calls; clean boundary.**

### `extension/src/setup/`
`setup.ts` (1220 LOC) is the 6-step (0..5) wizard: mode pick → remote config → image select / passphrase → derive+verify or create+embed → device register → recovery QR. Imports WASM directly. `setup-helpers.ts` (84 LOC) is a clean utility module (escapeHtml, debounced rate, strength labels). `probe.ts` (23 LOC) checks for an existing vault on the configured remote.

### `tools/relay/`
`server.ts` is the MCP server with HTTP/SSE transport (per-connection `makeServer()`). `queue.ts` is the in-memory FIFO with `Role` enum and `isRole()` guard. `queue.test.ts` is `node:test` based (4 pass / 1 fail on uncommitted state). `call.py` and `call.ts` are the MCP-fallback shims (untracked). `start.sh` launches manual / tmux / kitty modes. `package.json`, `tsconfig.json` keep the relay self-contained.

## CLI/extension parity table

| CLI capability | Extension equivalent | Notes |
|---|---|---|
| `init --image --output` | `save_setup` + setup wizard step 3-new | ✓ (parity via setup.ts; see [P1] about routing through SW) |
| `add login/secure_note/identity/card/key/document/totp` | `add_item` + `types/<kind>.ts` form | ✓ |
| `get <query> [--show] [--copy]` | `get_item` + popup detail; clipboard via `navigator.clipboard` | ✓ |
| `list [--type] [--group] [--tag] [--trashed]` | `list_items`, `list_trashed`, client-side filter; `list_groups` for autocomplete | ✓ |
| `edit <query> [--totp-qr]` | `update_item` + `wireTotpQr` (jsQR lazy import) | ✓ |
| `history <query>` | `get_field_history` + `field-history.ts` | ✓ |
| `rm <query>` (soft) | `delete_item` | ✓ |
| `restore <query>` | `restore_item` | ✓ |
| `purge <query>` | `purge_item` | ✓ |
| `trash list / empty` | `list_trashed` / `purge_all_trash` + `trash.ts` | ✓ |
| `backup export` | `export_backup` + `backup-panel.ts` | ✓ |
| `backup restore` | `restore_backup` + `backup-panel.ts` (restore path) | ✓ |
| `import lastpass <csv>` | `parse_lastpass_csv` + `import_lastpass_commit` + `import-panel.ts` | ✓ |
| `attach <query> <file>` | `upload_attachment` | ✓ |
| `attachments <query>` | `AttachmentRef[]` already inside `get_item` | ✓ (no separate message; same data) |
| `extract <query> <aid>` | `download_attachment` | ✓ |
| `detach <query> <aid>` | (no message) — done via `update_item` with mutated `attachments[]` | **partial / ✗** |
| `generate --length / --bip39 …` | `generate_password` / `generate_passphrase` + `generator-panel.ts` | ✓ |
| `settings trash-retention / history-retention / attachment-cap / generator-defaults` | `get_vault_settings` / `update_vault_settings` + `settings-vault.ts` | ✓ |
| `sync` | `sync` | ✓ |
| `status` | (no message) | **✗ — gap** |
| `lock` | `lock` | ✓ |
| `completions <shell>` | N/A (CLI-only) | n/a |
| `rate <passphrase>` | `rate_passphrase` (zxcvbn gate) | ✓ |
| `device add / revoke` (+ list) | `add_device` / `register_this_device` / `revoke_device` / `list_devices` / `list_revoked` + `devices.ts` | ✓ |
| `recovery-qr generate / unwrap` | `generate_recovery_qr` / `unwrap_recovery_qr` + `settings-security.ts` | ✓ |
| (browser-only autofill) | `get_autofill_candidates`, `get_credentials`, `check_credential`, `blacklist_site`, `capture_save_login`, `fill_credentials`, `ack_autofill_origin`, `get_blacklist`, `remove_blacklist`, `get_active_tab_url` | extension-only (no CLI counterpart, by design) |
| (browser-only device UX) | `get_settings` / `update_settings` (DeviceSettings: captureEnabled, captureStyle) | extension-only |

**Summary:** 22/23 CLI capabilities have a clean extension path; `status` is the one true gap, and `detach` is partial. No "CLI first, extension follow-up" violations under this lens.

## Boundary notes for DEV-B

1. **`extension/src/wasm.d.ts` is hand-maintained** — every change to `crates/relicario-wasm/src/lib.rs`'s `#[wasm_bindgen]` surface must be mirrored manually. Worth a CI guardrail comparing the wasm-pack–generated `.d.ts` against this file. (See [P2] in WASM boundary.)

2. **The session handle is opaque on the JS side** (`SessionHandle.value: number`, `free()`). DEV-B owns the Rust-side lifecycle: confirm that double-`free()` from JS is a no-op rather than a panic, since `service-worker/session.ts:26` currently swallows free errors silently. If the Rust side could panic on double-free, the silent swallow becomes a crash mask.

3. **`attachment_encrypt(handle, plaintext, max_bytes: bigint)`** — the JS side passes `BigInt` for `max_bytes`. Confirm the Rust binding accepts `u64` and that the conversion is loss-free. The extension is going to push this with γ₁ enforcement (per project memory).

4. **`register_device(name)` returns `{ signing_public_key, deploy_public_key }`** as a plain object (not a class). Setup wizard relies on both being hex strings (`device.public_key` lookup paths). Confirm the device key types stay strings on the Rust side rather than ever moving to `Uint8Array`.

5. **`generate_recovery_qr(handle, passphrase) → string`** and `unwrap_recovery_qr(payload_b64, passphrase) → Uint8Array` — the QR payload format is a contract between this surface and the CLI's `recovery-qr` subcommand. If DEV-B is reviewing recovery-QR end-to-end, confirm that re-deriving from a recovery QR produces the same `master_key` regardless of which side (CLI vs extension) generated it.

6. **`extract_image_secret(image_bytes) → Uint8Array` and `embed_image_secret(carrier, secret) → Uint8Array`** — the central-embed DCT scheme has `MAX_DIMENSION` and `QUANT_STEP = 50.0` constants on the Rust side. The setup wizard does no client-side dimension check before passing the image; if a >MAX_DIMENSION JPEG is selected, the failure message bubbles up generically. DEV-B may want a more specific Rust-side error variant the extension can re-render.

## Beginner-friendliness assessment (TS side)

**Reading order recommendation:** start with `extension/src/shared/messages.ts` and `types.ts` — they tell you the entire vocabulary in 500 lines. Then `service-worker/router/index.ts` to see how messages get routed. Then pick **one** vertical to follow end-to-end: `popup/components/types/login.ts` → `service-worker/router/popup-only.ts` (the `add_item`/`update_item` handlers) → `service-worker/vault.ts`. After that, `content/detector.ts` + `content/fill.ts` + the corresponding `content-callable.ts` handlers cover the autofill story.

**Trip wires:** the two oversized files (`vault.ts` 1027 LOC, `setup.ts` 1220 LOC) are the steepest cliffs. A learner who opens `setup.ts` first will see WASM imported directly and conclude that's the pattern — it isn't; that file is the exception. Adding a short `EXTENSION_ARCH.md` (or a comment block at the top of `setup.ts`) noting "WASM is loaded directly here only because vault creation predates the SW; everywhere else, route through `chrome.runtime.sendMessage`" would save half a day.

**Strongest learning surfaces:** the discriminated-union message contract, the per-type item form modules (small, parallel, easy to diff), and `service-worker/router/router.test.ts`. The shared form-affordances are also a model of pure-function wiring with explicit teardown — once those patterns click, the rest of the popup is "more of the same."

**Weakest learning surface:** `shared/state.ts` — it's the bridge between popup and vault tab, but the `any` typing tells you nothing about what flows through it. Tightening this is a high-leverage change.

## Uncommitted-state read

The working tree has substantial uncommitted changes. Architecturally relevant:

- **`extension/src/vault/vault.ts` (+151/−99) and `vault.css` (+238/−99)** — appear to be active work on the form wrapper sticky bar, drawer field grid, and gold-rebrand color palette (Phase 2B). Treat these as **in flight, ignore for this review** — they don't change the architectural shape, only the visual surface.
- **`extension/src/shared/glyphs.ts` (+/−2) and `__tests__/glyphs.test.ts` (+/−2)** — small additions, likely a new glyph constant. Doesn't affect the [P3] glyph adoption finding (the inline-literal call sites would still need migration).
- **`extension/manifest.json` and `manifest.firefox.json` (+/−2)** — likely a version bump in flight; PM should decide whether v0.5.1-dev gets a synced bump in `package.json` too.
- **`tools/relay/queue.ts`, `server.ts`, `start.sh` (modified) + `call.py`, `call.ts` (untracked)** — these are **architecturally relevant**: the dev-c role expansion is real, and `call.py`/`call.ts` are documented-but-untracked fallback shims. The broken test (`queue.test.ts:54`) is the P1 the rest of this review flags. **Do not ignore.**
- **`Cargo.lock`, `crates/*/Cargo.toml`** — out of scope; flag to DEV-A/DEV-B if not already on their list.
- **`.gitea_env_vars` (untracked)** — name suggests local credentials; should be `.gitignore`d if not already (PM check).
- **`docs/superpowers/coordination/v0.5.1-*` and `architecture-review-*`** — coordination prompt evolution; not architectural.

**My call:** vault/vault.css/glyphs/manifest changes are in-flight and shouldn't change findings. The relay changes ARE architecturally on-the-table because the new dev-c role + factory pattern are visible in the running review process. The relay test failure is treated as a P1 because it blocks `bun test` from going green and the kickoff prompt explicitly says "Review is not gated on green; it's gated on understanding" — but this is one cheap fix away from a green test run, and a learner running `bun test` first will assume the codebase is broken.