refactor: centralize duplicate code into shared utilities and services

[electather]

Summary

Centralizes genuinely-duplicated code into single shared helpers/services, removing 22 baselined fallow clone groups. Pure internal refactor — behaviour is preserved exactly at every call site; no public API or end-user behaviour changes.

This came out of analysing the .fallow baselines and inline fallow-ignore markers. Of the 161 baselined duplicate clone groups, only the genuine duplications — where the copies share one intent and would drift into bugs if edited independently — were centralized. The remaining ~136 are coincidental shape matches (distinct zod schemas / TS types), repeated data (theme palettes), or trivial fragments; forcing those into shared abstractions would couple unrelated domains and hurt maintainability, so they were deliberately left.

What was centralized (14 clusters / 22 sites)

Cluster	Shared home
Delivery-failure recording (4 catch arms)	notifications/internal/deliver-handler.ts → recordDeliveryFailure
Watchlist repo row-by-key lookup (4)	media/repo/internal.ts → selectRowByKey
Compact media facets builder	media/internal/facets.ts → buildFacets
Crypto encrypt/decrypt (drop local copies)	reuse crypto/helpers.ts encryptJson / decryptField
Required-userconfig validation	local connections/auth.ts helper
Start-auth-and-store orchestration	local connections/auth.ts helper
RQ optimistic snapshot/rollback (4)	client/shared/lib/query/optimistic.ts
MCP availability map (2)	mcp/composite-tools/_shared.ts → buildAvailabilityMap
Aggregate-feed dispatch (4 methods)	private MediaService.aggregateFeed
Mark-connection-exhausted DB write	db/queries.ts → markConnectionExhausted
Inbox read-state setter	notifications/repo/inbox.ts
Active-rows query	local media/repo/reads.ts helper
require-plugin-row lookup	plugin-runtime/internal/shared-credentials.ts

The crypto cluster also removed a hidden duplicate: connections/helpers.ts's local decryptJson was byte-for-byte crypto/helpers.ts's decryptField. Both local copies are gone and callers now import the canonical crypto helpers directly (no re-export shim, per the shared-package rules). This clears the only duplicate_exports the work would otherwise have introduced.

Deliberately deferred (out of scope for a dedup PR)

• media-item-fields defaults (catalog↔preferences): the only legal shared home sits on the catalog↔preferences cycle. Today catalog → preferences is a compile-erased type-only edge; adding a runtime value-import would turn it into a real runtime cycle and a new circular-dependencies violation. It belongs with the type-relocation that fixes that cycle.
• Generic sweepExpired helper: low-confidence generic-over-table wrapper for an 8-line delete — net-negative indirection.
• catalog↔preferences circular dependency (dead-code baseline): an architectural refactor (relocate MediaItemFields/CandidateFeatures/RawMediaItem to @ent-mcp/shared + invert the manual-rebuild → catalog edge), not duplicate code. Recommended as its own PR.

Heads-up: stale .fallow baselines

The committed .fallow/*-baseline.json files predate the entire library/** feature (they contain zero library entries and line numbers have drifted repo-wide). A wholesale regen here would absorb unrelated library tech-debt and grow the dead-code baseline with a pre-existing library/internal/reads.ts ↔ service.ts cycle, so it was intentionally avoided. Only the 22 clone groups this PR actually eliminated were removed from dupes-baseline.json; dead-code-baseline.json and health-baseline.json are untouched (this PR adds no dead code, cycles, or duplicate-exports — verified). Recommendation: refresh all three baselines as a separate chore once the library branch lands on main.

Type of change

• Refactor / internal cleanup (no behaviour change)

Scope

• @ent-mcp/client
• @ent-mcp/server

Test plan

• vp check passes locally (format, lint, type-check — 1557 files clean)
• vp test passes locally (363 files, 2674 tests)

Two plugin-runtime test mocks were updated to expose the new requirePluginRow export; the auth concurrent-completion test now mocks crypto/helpers instead of the removed connections/helpers crypto copies.

Checklist

• PR title follows Conventional Commits
• Tests added or updated for new behaviour
• Docs updated where relevant (n/a — internal refactor)
• A changeset is included (empty — internal-only change)
• No secrets, credentials, or personal data committed

[github-actions]

Code Review — PR #543: centralize duplicate code

Design-doc sync

SKIPPED — pure internal refactor, no public API or end-user behaviour changes. No design-doc link required.

Fallow baseline

PASSES — dupes-baseline.json only shrinks (22 entries removed). dead-code-baseline.json and health-baseline.json untouched. Baseline rule not violated.

---

Correctness: crypto rename is right, but "byte-for-byte" claim is inaccurate

The PR description says the removed connections/helpers.decryptJson was "byte-for-byte" crypto/helpers.decryptField. Reading both:

connections/helpers.decryptJson (removed):

if (!iv || !data) return null;
const plain = await decrypt(`${iv}:${data}`, env.ENCRYPTION_KEY);
try { return JSON.parse(plain); } catch { return plain; }

crypto/helpers.decryptField (canonical):

const combined = joinCiphertext(iv, data);  // same null-guard, same concat
if (!combined) return null;
const plain = await decrypt(combined, env.ENCRYPTION_KEY);
try { return JSON.parse(plain); } catch { return plain; }

Functionally identical — same null-guard, same "iv:data" format, same JSON-with-fallback semantics. The connections callers (auth.ts, service.ts) are correctly routed to decryptField. No behaviour change. The word "byte-for-byte" is imprecise; "semantically identical" is accurate.

Note also: delivery.ts and plugin-jobs.ts still use crypto/helpers.decryptJson (no fallback — throws on non-JSON payload). This asymmetry predates the PR and is not introduced here, but the split across callers is worth tracking.

---

Minor issues (non-blocking)

buildFacets allocates Date on every call (media/internal/facets.ts:13):

if (meta.year != null && meta.year > new Date().getUTCFullYear())

Called during enrichment, not a hot path, but a module-level const CURRENT_YEAR = new Date().getUTCFullYear() would be cleaner. Not blocking.

requirePluginRow retains an unsafe cast (plugin-runtime/internal/shared-credentials.ts):

return row as PluginRow;

Pre-existing pattern from runtime.ts. Moving it to the extraction point is correct; the risk is unchanged. If the Drizzle schema for plugins gains or changes a column, this cast will silently diverge. Not blocking, but noting for future tightening (use plugins.$inferSelect directly).

No unit tests for new shared helpers. snapshotQuery, buildFacets, selectRowByKey, markConnectionExhausted, buildAvailabilityMap, recordDeliveryFailure — all exercised only via call-site integration tests. Acceptable for a pure refactor where existing test coverage validates the migration. Non-blocking.

---

What's clean

• assertRequiredUserConfig and startAuthAndStore in connections/auth.ts: tight extractions, no signature leakage.
• setInboxReadAtForUser unifying read/unread in notifications/repo/inbox.ts: minimal, correct.
• activeRowsQuery returning a Drizzle builder (thenable): callers await it correctly in both limited and unlimited paths.
• snapshotQuery return shape matches existing { prev } / { snapshot } context expectations across all four mutation hooks.
• buildAvailabilityMap in _shared.ts matches the ent-discover implementation (uses compact) rather than the ent-activity variant (early return), which is the cleaner approach.
• Changeset is correctly empty-frontmatter for an internal-only change.
• Test mock updates (auth.concurrent-completion.test.ts, runtime-allowed-hosts, runtime-pool) are precise and minimal.

Approved. No blocking issues.