refactor: extract handleBeforeSend keepAlive + popup handleEncrypt/handleDecrypt closures for testability

[dobby-coder]

Background

Follow-up to #105. The unit-test sweep landed in PRs #110–#115 and brought the suite from 21 passed / 108 todo to 126 passed / 22 todo. The remaining 22 placeholders all sit inside two listener closures that aren't testable in their current shape — they need a production refactor (deps injection), not just test authoring, so they were intentionally split out.

Remaining todos (22)

tests/crypto-popup.test.ts — 19 todos

Cover handleEncrypt / handleDecrypt in src/pages/yivi-popup/yivi-popup.ts:

• init / DOM render paths
• auto-close behavior
• error paths (SDK failure, missing recipients, cancelled QR, etc.)

tests/encryption-flow.test.ts — 3 todos

Cover handleBeforeSend's keepAlive closure body in src/background/background.ts:

• encryption-failure path
• popup-cancel path
• plaintext-leak guard

Suggested approach

Both closures currently capture module-scoped singletons (pg-js SDK, browser.* namespaces, DOM globals) directly. To make them testable:

1. Lift the closures to top-level functions that take their dependencies as parameters (SDK client, runtime, storage, DOM root, etc.).
2. Listener bodies become thin call sites that pass the live deps in — no behavior change.
3. Tests instantiate the function with mocked deps from the existing tests/helpers/browser-mock.ts harness plus a small pg-js mock.

The PR #110 harness already covers most of browser.*. The new lift is the pg-js client and a DOM mock for the popup tests.

Verification

• After refactor: npm test should reach ~148 passing / 0 todo.
• npm run typecheck stays green.
• npm run build:dev still produces a working extension.
• Manual smoke-test the encrypt + decrypt happy paths in Thunderbird, since the closures are the live encryption/decryption code paths.

Risk

This touches the production code that PR #68's persistence fix lives in. The lift must keep the existing guards intact (composeTabs.has() check, persistence on toggle, keepAlive semantics on onBeforeSend). The state-lifecycle tests added in #110 should pin those, but a manual smoke-test through Thunderbird before merge is still warranted.

Out of scope

• Higher-level end-to-end / integration testing — tracked separately in Integration tests #53.
• Any test-authoring work outside these two closures — the rest of the suite is real now.

Follow-up to #105.

[rubenhensen]

/dobby

[dobby-coder]

the bot is taking the request! Reading issue #116 carefully — the bot will lift handleEncrypt/handleDecrypt out of the popup and the keepAlive closure out of handleBeforeSend, wire them to injected deps, and unfreeze the 22 remaining it.todo tests. Will post a plan shortly. 🧹

[dobby-coder]

Plan

Baseline confirmed: npm test = 126 passed / 22 todo, npm run typecheck green, npm run build:dev green.

Refactor strategy

The two listener closures capture module-scoped singletons. Lift them out behind injected deps so tests can pass mocks instead of the live browser.* / pg-js / DOM globals. Listener bodies become thin call sites — no behavior change.

Files to add

• src/background/encryption-flow.ts — extend with runBeforeSendEncryption(state, details, tabId, deps). Deps interface: { listAttachments, getAttachmentFile, getFull, removeAttachment, addAttachment, openCryptoPopup, notifyError, pkgUrl, cryptifyUrl, websiteUrl, pgClientHeader, xPostguardVersion }. Returns { cancel?: boolean; details?: {...}; sentMimeData?: Uint8Array }. Pure with respect to globals — caller assigns state.sentMimeData from the return value.
• src/pages/yivi-popup/encrypt-popup.ts — runEncryptInPopup(pg, data, windowId, deps). Deps: { runtime }. Calls pg.encrypt / pg.email.createEnvelope and posts cryptoPopupDone / cryptoPopupUploadInit via deps.runtime.
• src/pages/yivi-popup/decrypt-popup.ts — runDecryptInPopup(pg, data, windowId, deps). Deps: { runtime, extractFromZip }.
• src/pages/yivi-popup/zip.ts — move extractFromZip here (already pure; gets it out of yivi-popup.ts so the decrypt-flow tests don't pull in DOM globals).
• src/pages/yivi-popup/init-flow.ts — initCryptoPopup({ resolveWindowId, requestInitData, createPg, runEncrypt, runDecrypt, ui, closeWindow, sendError, autoCloseDelayMs }). The ui is an interface { setOperation(op), setLoadingDone(), showError(msg) }. yivi-popup.ts becomes a thin wire-up that constructs the live deps and calls initCryptoPopup.

Files to change

• src/background/background.ts — handleBeforeSend becomes the guard + runBeforeSendEncryption(...) call. The keepAlive("onBeforeSend", ...) wrap stays in background.ts (it needs the live browser.alarms API; testing the alarm scaffolding isn't in scope). The encryption body inside keepAlive is what's lifted.
• src/pages/yivi-popup/yivi-popup.ts — collapses to ~40 lines: pick DOM elements, build deps, call initCryptoPopup.

Tests to author

tests/crypto-popup.test.ts (the 19 todos):

• init (4): stub resolveWindowId / requestInitData / createPg; assert init message routed with correct windowId, error-paths show error and skip pg construction.
• encrypt (5 todos): drive runEncryptInPopup with a fake pg whose encrypt/sign.yivi/email.createEnvelope track args; assert mimeData = fromBase64(input), element selector #yivi-web-form, attachmentBase64+size in the cryptoPopupDone message, auto-close path called once. Use a vi.fn() for runtime.sendMessage to inspect calls.
• decrypt (6): drive runDecryptInPopup with fake pg; assert ciphertext decode for tier-1/2, uuid path for tier-3, element selector, recipient, sender forwarded.
• error (4): simulate pg throwing → assert cryptoPopupError posted with message, showError called, auto-close NOT scheduled.

tests/encryption-flow.test.ts (the 3 todos):

• encryption-failure: deps.openCryptoPopup rejects → returns { cancel: true }, deps.notifyError called with "encryptionError", no addAttachment call.
• popup-cancel: deps.openCryptoPopup rejects with "Popup closed" → returns { cancel: true } and notifyError called (current behavior — same catch).
• plaintext-leak guard: on failure, returned object has no details.body / details.subject (only cancel: true).

Verification

• npm test reaches ≥147 passing / 0 todo (1 todo may remain in extractFromZip if we keep zip tests separate; otherwise 148 / 0).
• npm run typecheck stays green.
• npm run build:dev stays green.
• Manual smoke-test in Thunderbird is out of scope for the bot (no Thunderbird in workspace); will flag this in the PR body so the reviewer can verify before merge — encrypt path and decrypt path both need a live click-through.

Risks

• This touches PR fix: prevent unencrypted send when encryption is enabled #68's encryption-state persistence path. The lift preserves composeTabs.has() and persistEncryptState semantics — those live in the listener, not in the extracted body. Existing state-lifecycle tests pin them.
• keepAlive semantics stay in background.ts; the extracted function returns a plain promise that keepAlive wraps.
• The onUploadInit callback fires inside pg-js's upload-stream start handler — keep it fire-and-forget so a runtime.sendMessage rejection cannot abort the upload. Preserved.

PR will be opened ready-for-review with @rubenhensen as reviewer once CI is green, but flagged in the body that manual Thunderbird smoke-test of encrypt + decrypt happy paths is recommended before merge given this code is the live production crypto path. Closes #116.

[dobby-coder]

PR #117 is open and ready for review. CI is green (build passed in ~11s). Tests went from 126 passed / 22 todo to 149 passed / 0 todo — every placeholder #116 listed is now a real test driving the extracted code. Manual Thunderbird smoke-test of encrypt/decrypt happy paths is recommended before merge (called out in the PR body) since the bot can't click through a live extension.