Delete the dead messaging island (4 modules, 719 LOC, never wired) + add knip to CI

[TortoiseWolfe]

Four modules (719 LOC) plus three test files (~1117 LOC) form a closed island with no entry point from the app. They implement a complete alternative message-thread data layer that has never run in production.

useConversationRealtime.ts        ← no importers at all
  ├── useConversationRealtimeSync.ts  (:24)
  │     └── decrypt-message.ts        (:22)
  ├── decrypt-message.ts              (:23)
  │     └── decryption-cache.ts       (:21)
  └── decryption-cache.ts             (:19)

Verified beyond static imports: no import( with a matching specifier, no string-built module paths. The only surviving references in src/ are doc comments pointing into the island.

Provenance: born dead, not orphaned

useConversationRealtime.ts was present in the initial commit (893f552, 2025-12-04) — and page.tsx in that same commit already had the full-refetch loadMessages. Both designs have coexisted since day one; the refetch won by default.

git log --all -S "useConversationRealtime" -- src/app/ src/components/ returns empty across all history (control query with useTypingIndicator over identical paths returns results, proving the query works). There is no commit to roll back to, because it was never wired.

It has already cost real time, twice

• 2026-03-24, a5d4e3d — an engineer fixed Firefox CI Realtime flake by adding 55 lines of polling fallback and E2E DOM signals into the dead hook. It fixed nothing.

• Twelve hours later, 812cead — re-diagnosed and hand-copied 42 lines into page.tsx. The commit title says it outright: "fix: wire polling fallback into messages page (was dead code)".

  The port carried the polling fallback but not the per-message incremental decrypt — which is why the live path now refetches and re-decrypts 50 rows to learn about one new message. Two copies of the subscribe/poll/teardown block now sit side by side and have already drifted.

• 2026-08-02, Message reliability: five undiagnosed E2E-encryption bugs #69 — a code review traced a message-loss symptom to clearDecryptionCaches() in this island and filed it as the confirmed root cause. It cannot be: the function never executes. Correction posted; ~10 of Message reliability: five undiagnosed E2E-encryption bugs #69's 32 checklist items point here.

Deleting is not a cleanup nicety. This island has a demonstrated negative return.

Harvest first, in the same PR

1. upsertMessage's merge semantics (decrypt-message.ts:235-267, 33 lines) → page.tsx:356-368, replacing the content-matching heuristic. Fix two things on the way:
   • The sequence_number sort is wrong for pending rows. assign_sequence_number is COALESCE(MAX(sequence_number), 0) + 1 (migration:1096), so real sequences start at 1 and the seq-0 client placeholders sort to the top of the entire thread. The module docstring at :228-229 claims the created_at tiebreak handles offline placeholders; it only orders seq-0 rows against each other. Use a separate pending bucket pinned after the highest known sequence. (The live path gets this right today by appending — do not regress it.)
   • The anti-downgrade branch (:245-254) keeps stale content while adopting edited/edited_at, so an edit can be silently dropped.
2. The three-way failure triage (:139-162) — "please sign in again" / "sender encryption keys unavailable" / "encrypted with previous keys" are better than the single placeholder at message-service.ts:859.
3. The knowledge that the per-event public-key round-trip is the real waste (message-service.ts:727). The ECDH derive at :801 is cheap.

Do not harvest decryption-cache.ts. Its cache key ${conversationId}:${otherParticipantId} (decrypt-message.ts:99) omits key identity, so it cannot self-heal on rotation; privateKeyCache holds an extractable private key and buys nothing over getCurrentKeys(); and no invalidation is wired at all. A listener-based fix is structurally blind to peer rotation anyway — static-static ECDH means K(A,B) changes when either side rotates. Memoize per (messageId, keyEpoch) with evict-and-retry-once instead, if this is ever needed.

onKeysChanged/notifyKeyChange (key-service.ts:64-77) becomes provably dead once the island goes — delete with it.

Documentation debt this created (fix or the deletion is half-value)

• docs/SECURITY-ARCHITECTURE.md:185-198 documents the shared-secret cache as a live DoS mitigation, quotes "~50ms to ~1ms", and names src/hooks/useConversationRealtime.ts. The mitigation it documents is not in effect in the running app.
• key-service.test.ts:34 and :49 assert an onKeysChanged listener "in decrypt-message.ts" — no such listener exists in that file.
• tests/e2e/messaging/real-time-delivery.spec.ts:169 credits the dead sync hook with data-messages-subscribed, which is actually set by page.tsx:213-218.
• specs/010-group-chats schedules work against it: tasks.md:106 (T038) and :108 (T040) both say "update useConversationRealtime", plus plan.md:80 and research.md:157. Group chats cannot be built by modifying a hook that never runs — retarget these before they're picked up.

Prevent recurrence: add knip to CI

package.json has no knip, ts-prune, unimported, or depcheck. TypeScript's unused-symbol checks do not span modules, so nothing in this repo can mechanically detect a closed dead island — which is how one survived eight months starting from the initial commit's own output. This is the highest-leverage item here; it would also have caught the 131 files deleted in #82 and the remaining leads in #90.

One honest counter-argument

vitest.config.ts:180-185 sets 58% coverage thresholds. Deleting 719 well-covered LOC while the surviving, untested 755-LOC page.tsx stays will move the aggregate down. This is the mechanism, not a measured number — coverage was not run. Measure before opening the PR; if it trips, fix it by adding the page.tsx tests that should exist anyway. A gate that penalises deleting code which never executed is miscalibrated, and that's the framing for review.

Scope

Delete: src/hooks/useConversationRealtime.ts, src/hooks/useConversationRealtimeSync.ts, src/lib/messaging/decrypt-message.ts, src/lib/messaging/decryption-cache.ts, their three test files, and key-service.ts:64-77.

Verify: pnpm build (a missed importer fails compilation), pnpm type-check, pnpm lint, full unit suite, and the messaging E2E specs — real-time-delivery.spec.ts in particular, since its comment references the deleted file.

Resolves the messaging cluster of #90. Related: #69 (checklist retargeted), #82 (same class of finding).

[TortoiseWolfe]

Done on branch chore/delete-messaging-island — and two corrections to this issue

Four commits. Two things this issue got wrong turned up during the work; both made the job smaller.

Correction 1: the harvest was unnecessary. Both items were moot.

Harvest item 1 (upsertMessage merge semantics) is not implementable yet. It dedupes by m.id === incoming.id (decrypt-message.ts:239), but optimistic rows carry optimistic-<timestamp> ids (page.tsx:403) while server rows carry UUIDs. They can never match. The content heuristic at page.tsx:358-366 exists precisely because ids don't match — so porting upsertMessage would not have replaced it. This is blocked on #91 item 2 (client-minted UUIDs), which makes the heuristic deletable for real.

Harvest item 2 (three-way failure triage) was already done. This issue claimed the live path had "the single placeholder at message-service.ts:859". It has two, plus a typed error:

• :767 — "Unable to decrypt — sender keys unavailable" (peer has no public key)
• :859 — "Encrypted with previous keys" (per-message decrypt failure)
• EncryptionLockedError thrown when the user has no keys

All three of the island's cases already have live equivalents with near-identical wording. Nothing to port.

Item 3 was a note, not code, and is captured as #91 item 10.

So the deletion was pure — no behaviour change at all, which is a materially safer PR than this issue described.

Correction 2: the coverage concern does not materialise. Now measured.

This issue flagged, explicitly as an unmeasured mechanism, that deleting 719 well-covered LOC might trip the 58% thresholds in vitest.config.ts:180-185. Measured on the branch:

All files | 68.34 stmts | 60 branch | 70.79 funcs | 69.16 lines    → exit 0

Comfortably above on all four. No mitigation needed and no reason to weaken the gate.

What landed

Commit
c1c6217	Delete the island — 7 files, 1836 LOC
30abb17	Remove onKeysChanged/notifyKeyChange + 5 call sites + its one test
d0d1d3d	Retarget the four documentation sites
325d114	knip + baseline + CI gate

The listener removal is deliberately a separate commit — it touches a live service, whereas the island deletion touched nothing that runs, so it can be reverted independently.

SECURITY-ARCHITECTURE.md now describes what the shipping path actually does — one ECDH derivation per getMessageHistory() batch amortized across up to 50 messages, no client-side cache — instead of documenting the never-wired cache as a live DoS mitigation with a "~50ms to ~1ms" figure.

No references to the island remain anywhere in the repo (checked across .ts, .tsx, .md, .json, .yml).

knip: baselined at 37, mutation-checked

Configured in knip.jsonc with app-router entry points; knip's built-in Next plugin is disabled because its assumptions produce false entries against this static export.

The baseline holds two deliberately-distinct categories, and the config says which is which:

1. 16 one-line barrel files re-exporting live components — an import-style decision, explicitly out of scope per Dead code sweep: 15 orphaned components, 2 deprecated modules, 1 unreachable API route #82.
2. 21 genuine orphans awaiting individual verification under Verify ~48 non-component orphans (hooks, lib, utils) surfaced by the #82 graph #90.

.storybook/mocks/** is ignored for a different reason: reached via path.resolve aliases at .storybook/main.ts:43,47, which knip cannot follow. Reachable, not orphaned.

Mutation check: planting an unreferenced file under src/lib/ turns the gate red and names it (exit 1); removing it returns exit 0. The gate can fail.

Verification

Check	Result
type-check	clean
lint	0 errors (1 pre-existing warning in useCompaniesMapData.ts, not in this diff)
Unit suite	385 files, 4015 passed, 0 failed
Test-count delta	exact match — 4047→4015 is 32 tests; deleted files contained 15 + 9 + 7 + 1 = 32. 388→385 files. No collateral loss.
Coverage	68.34 / 60 / 70.79 / 69.16 vs 58 thresholds — pass
knip	exit 0, mutation-proven
pnpm build	could not verify locally — see below

The build could not be verified locally, and it is not this branch

pnpm build fails with Cannot find module './4873.js' from the static-export worker. main fails identically, after rm -rf .next out and pnpm install --frozen-lockfile, with no dev server running. CI is green on main (5ef426b), and CI runs pnpm build.

So the code builds; this Docker/WSL2 environment does not build it. Pre-existing and unrelated to this branch, but it means the build check for this issue has to come from CI, and it is worth its own issue — a repo whose primary build command fails locally on a clean checkout of main is a real onboarding defect.

Not done here

• Burning down the 37-entry knip baseline — that is Verify ~48 non-component orphans (hooks, lib, utils) surfaced by the #82 graph #90's job, entry by entry.
• Anything in Messaging live path: 5 round-trips per event, lost-update race, content-based dedupe #91.