docs(14-01): complete inbox-module extraction plan

2026-07-23 13:11:21 +02:00
parent c404954bee
commit a47c0c57ee
4 changed files with 152 additions and 12 deletions
@@ -0,0 +1,136 @@
+---
+phase: 14-rss-email-alert-ingestion-module-rollout
+plan: 01
+subsystem: api
+tags: [nestjs, imapflow, httpntlm, ews, vitest, refactor]
+
+# Dependency graph
+requires: []
+provides:
+  - "apps/api/src/inbox/ module: InboxModule, InboxProvider, ImapProvider, ExchangeInboxProvider, InboxConfig/InboxAttachment/InboxEmail/InboxMessage"
+  - "InboxProvider.fetchMessages(config) seam — subject + HTML/text body per unread message, shared by any future module needing mailbox polling"
+  - "DKV switched to the shared inbox/ import path with zero behavior change (regression-gated by full API suite)"
+affects: [14-03-email-alert-adapter]
+
+# Tech tracking
+tech-stack:
+  added: []
+  patterns:
+    - "Shared connection-only module extraction: inbox/ shares IMAP/EWS mechanics, each consuming module keeps its own independent config (D-03)"
+    - "require.cache stub for testing raw CommonJS require() dependencies that vi.mock cannot intercept (httpntlm)"
+
+key-files:
+  created:
+    - apps/api/src/inbox/inbox.module.ts
+    - apps/api/src/inbox/inbox.types.ts
+    - apps/api/src/inbox/inbox-provider.interface.ts
+    - apps/api/src/inbox/imap.provider.ts
+    - apps/api/src/inbox/exchange-inbox.provider.ts
+    - apps/api/src/inbox/imap.provider.spec.ts
+    - apps/api/src/inbox/exchange-inbox.provider.spec.ts
+  modified:
+    - apps/api/src/dkv/dkv.service.ts
+    - apps/api/src/dkv/dkv.module.ts
+    - apps/api/src/dkv/dkv.types.ts
+
+key-decisions:
+  - "Pure move + import-path swap for Task 1 — no logic changes to fetchPdfAttachments, InboxConfig fields, or DKV pipeline (D-01/D-02 regression guard)"
+  - "dkv.types.ts re-exports InboxConfig/InboxAttachment/InboxEmail from ../inbox/inbox.types instead of declaring them, so no DKV consumer import needed to change"
+  - "fetchMessages implemented as a sibling method (findBodyParts for IMAP MIME walk, getItemBodySoap for EWS) — does not touch or call into the existing PDF-attachment code paths"
+  - "httpntlm is loaded via a raw CommonJS require() in exchange-inbox.provider.ts, which vi.mock cannot intercept (verified via repro) — tests instead pre-seed Node's require.cache for the resolved httpntlm path with a stub before dynamically importing the provider in beforeAll"
+
+patterns-established:
+  - "Shared inbox/ module is the seam for any future mailbox-polling module (Plan 14-03's EmailAlertAdapter will inject ImapProvider/ExchangeInboxProvider from InboxModule)"
+
+requirements-completed: [INGEST-05]
+
+coverage:
+  - id: D1
+    description: "DKV's IMAP/Exchange providers moved into shared apps/api/src/inbox/ module; DKV imports switched to the new path with zero behavior change"
+    requirement: "INGEST-05"
+    verification:
+      - kind: unit
+        ref: "cd apps/api && npx tsc --noEmit -p tsconfig.json && npx vitest run — full 301-test suite green (no DKV spec files exist yet, so this is a compile + no-regression-elsewhere gate)"
+        status: pass
+    human_judgment: false
+  - id: D2
+    description: "New fetchMessages(config) method on InboxProvider/ImapProvider/ExchangeInboxProvider returns each unread message's subject + HTML/text body, without touching fetchPdfAttachments"
+    requirement: "INGEST-05"
+    verification:
+      - kind: unit
+        ref: "apps/api/src/inbox/imap.provider.spec.ts (7 tests) + apps/api/src/inbox/exchange-inbox.provider.spec.ts (6 tests)"
+        status: pass
+    human_judgment: false
+
+duration: 13min
+completed: 2026-07-23
+status: complete
+---
+
+# Phase 14 Plan 01: Inbox Module Extraction Summary
+
+**Extracted DKV's IMAP/Exchange inbox providers into a shared `apps/api/src/inbox/` module and added an additive `fetchMessages()` method returning subject + HTML/text body per unread message — the seam Plan 14-03's EmailAlertAdapter will consume.**
+
+## Performance
+
+- **Duration:** ~13 min
+- **Started:** 2026-07-23T12:59:00+02:00 (approx.)
+- **Completed:** 2026-07-23T13:09:45+02:00
+- **Tasks:** 2/2 completed
+- **Files modified:** 10 (5 new, 3 modified in Task 1; 2 new specs + 3 further modified in Task 2)
+
+## Accomplishments
+- Moved `InboxProvider`, `ImapProvider`, `ExchangeInboxProvider`, and `InboxConfig`/`InboxAttachment`/`InboxEmail` verbatim from `apps/api/src/dkv/providers/` into `apps/api/src/inbox/`, with `dkv.types.ts` re-exporting the moved types so no other DKV file's import needed to change
+- New `InboxModule` (NestJS `@Module`) provides + exports both providers; `DkvModule` now imports `InboxModule` instead of declaring the providers directly
+- Added `fetchMessages(config): Promise<InboxMessage[]>` to the interface and both providers, entirely additive — `fetchPdfAttachments` bodies are byte-identical to before the move (verified via `git diff`)
+- Net-new unit coverage: 13 tests across `imap.provider.spec.ts` (7) and `exchange-inbox.provider.spec.ts` (6), covering body extraction, idempotent \Seen/IsRead marking, sender filtering, and graceful `[]` returns on connect/search/HTTP errors
+
+## Task Commits
+
+Each task was committed atomically:
+
+1. **Task 1: Move inbox providers into apps/api/src/inbox/ (behavior-preserving)** - `eb668fd` (refactor)
+2. **Task 2: Add fetchMessages() to InboxProvider + both implementations (net-new coverage)** - `c404954` (feat, TDD RED confirmed via 13 failing "not a function" assertions before implementation)
+
+**Plan metadata:** (this commit, to follow)
+
+## Files Created/Modified
+- `apps/api/src/inbox/inbox.module.ts` - New NestJS module providing/exporting ImapProvider + ExchangeInboxProvider
+- `apps/api/src/inbox/inbox.types.ts` - InboxConfig, InboxAttachment, InboxEmail (moved) + new InboxMessage type
+- `apps/api/src/inbox/inbox-provider.interface.ts` - InboxProvider interface, now with fetchMessages
+- `apps/api/src/inbox/imap.provider.ts` - ImapProvider, moved + fetchMessages/findBodyParts added
+- `apps/api/src/inbox/exchange-inbox.provider.ts` - ExchangeInboxProvider, moved + fetchMessages/getItemBodySoap/fetchMessagesViaEws added
+- `apps/api/src/inbox/imap.provider.spec.ts` - New: mocks ImapFlow, covers fetchMessages
+- `apps/api/src/inbox/exchange-inbox.provider.spec.ts` - New: mocks httpntlm.post via require.cache stub, covers fetchMessages
+- `apps/api/src/dkv/dkv.service.ts` - Import paths for ImapProvider/ExchangeInboxProvider changed to `../inbox/...`
+- `apps/api/src/dkv/dkv.module.ts` - Imports InboxModule instead of declaring the two providers directly
+- `apps/api/src/dkv/dkv.types.ts` - Re-exports InboxConfig/InboxAttachment/InboxEmail from `../inbox/inbox.types`
+
+## Decisions Made
+- **Pure move, no logic changes (Task 1):** `fetchPdfAttachments`, `testConnection`, and all EWS SOAP builders were copied verbatim; only import paths changed. Verified via `git diff HEAD~1 HEAD` showing zero edits inside the pre-existing method bodies.
+- **Re-export shim in dkv.types.ts:** rather than updating every DKV file that imports `InboxConfig`/`InboxAttachment`/`InboxEmail`, `dkv.types.ts` now re-exports them from `../inbox/inbox.types`, so only the two files that imported the *providers* (dkv.service.ts, dkv.module.ts) needed edits.
+- **fetchMessages as a true sibling, not a refactor of fetchPdfAttachments:** IMAP's `findBodyParts` mirrors `collectPdfParts`'s tree-walk shape but is a separate function; EWS's `getItemBodySoap` is a separate SOAP builder from `getItemSoap`. Neither touches the PDF-attachment call path — this was required by D-01/D-02 (DKV must stay behavior-identical) and verified by grepping the diff for edits inside `fetchPdfAttachments`/`fetchViaEws`.
+- **Testing httpntlm's raw `require()`:** `vi.mock('httpntlm', ...)` was verified (via a throwaway repro test) to have zero effect on `exchange-inbox.provider.ts`'s `const httpntlm = require('httpntlm')` — Vitest's mock interception only covers the ESM module graph, and a literal `require()` call resolves through Node's real module cache regardless. The spec instead pre-seeds `require.cache[require.resolve('httpntlm')]` with a stub `{ post }` in a `beforeAll` (dynamic `import()` of the provider, deferred out of module scope so the file stays compatible with the project's `commonjs` tsconfig which rejects top-level `await`/`import.meta`).
+
+## Deviations from Plan
+
+None — plan executed exactly as written. The `require.cache` testing approach for `exchange-inbox.provider.spec.ts` is a test-infrastructure detail within Task 2's stated scope ("mirroring cosinex.adapter.spec.ts's mock style ... stub httpntlm.post") — the plan did not prescribe *how* to intercept a raw CJS `require()`, and the chosen approach achieves the same outcome (httpntlm.post fully mocked, zero real network calls) without touching the provider's production code (which the plan explicitly forbids "improving" or refactoring).
+
+## Issues Encountered
+- Discovered mid-Task-2 that `vi.mock('httpntlm', ...)` does not intercept `exchange-inbox.provider.ts`'s raw `require('httpntlm')` call (confirmed via a minimal repro: `vi.isMockFunction(require('httpntlm').post)` returned `false`, and all 6 initial test attempts hit a real DNS lookup — `getaddrinfo ENOTFOUND mail.example.com` — with 3 "passing" only by coincidence since their assertions expected `[]`, which a failed network call also produces). Resolved via the `require.cache` stub approach described above; re-verified all 6 exchange-inbox tests genuinely exercise the mocked `httpntlm.post` (assertions on `messages[0]` content, not just `[]`).
+
+## User Setup Required
+
+None - no external service configuration required.
+
+## Next Phase Readiness
+- `apps/api/src/inbox/` is ready for Plan 14-03's `EmailAlertAdapter` to inject `ImapProvider`/`ExchangeInboxProvider` (via `InboxModule`) and call `fetchMessages(config)` for the tender-alert mailbox — with its own independent mailbox config (D-03), not DKV's.
+- DKV's regression surface (fetchPdfAttachments, the full pipeline) is unchanged and the full API suite (301 tests) is green; no DKV-specific spec file exists yet, so the regression guard for this plan was the compile (`tsc --noEmit`) plus the rest of the suite not regressing — a gap the plan itself flagged as pre-existing (no `dkv/*.spec.ts` files in the repo).
+
+---
+*Phase: 14-rss-email-alert-ingestion-module-rollout*
+*Completed: 2026-07-23*
+
+## Self-Check: PASSED
+
+All 7 created/modified inbox files verified on disk; both task commits (`eb668fd`, `c404954`) verified in `git log --oneline --all`.