review(branch): prioritized action list

Author: claude

docs/reviews/branch-claude-oplog-transformer-collections-2026-06-23-2.md

@@ -157,3 +157,31 @@ Strong baseline: slog-JSON everywhere, `request_id` stamped once at `processOne`
 - `nitpick` — `sourcelookup.go:35`: shared tracer renamed to `"migration"` while `oplog-transformer/historyclient.go:44` still uses `"oplog-transformer"` — two instrumentation-scope names coexist in one binary. Intentional/harmless; note for dashboard authors.
 
 PII/secrets: clean. No log emits source bodies, `statusText` content, or tokens; usernames appear only in error strings (`account %q`), acceptable. Request-ID propagation correct across both services, including the new `handleSubscriptionDeleted`. Pre-existing gap: `inbox-worker/handler.go:119` `unknown event type` warn omits request_id (not introduced here; the new `subscription_deleted` case sits above it).
+
+---
+
+# Prioritized action list
+
+Ordered by severity, then impact ÷ effort.
+
+1. **`high` — Add explicit projections to `FindThreadRoom`/`FindUserID`** (`oplog-collections-transformer/targetstore.go:64,78`). CLAUDE.md §MongoDB hard rule ("every find MUST specify a projection"); flagged by 3 lenses. `FindThreadRoom` needs `{roomId,_id,siteId}`, `FindUserID` needs `{_id}`. Trivial fix, hottest read path. *Effort: XS.*
+
+2. **`medium` — Confirm the `member_added` unknown-user semantics change is intended for live federation** (`inbox-worker/handler.go:192-199`). It now Naks (to MaxDeliver) on any unresolved account on the *shared* path, where it previously Acked. Either confirm + document, or gate the redelivery behavior to migration events only. *Effort: S.*
+
+3. **`medium` — Close the orphan-on-leave edge case** (`subscriptions.go:128-130` + `inbox-worker/handler.go:151-186`). A re-subscribe that lands on a pre-existing different-`_id` row makes the later source-`_id` `subscription_deleted` miss the real row. Confirm the migration always seeds via the adopting `member_added`, or scope `DeleteSubscriptionByID` by `(roomId, account)` as a fallback. *Effort: S–M.*
+
+4. **`medium` — Handle empty `ALL_SITE_IDS`** (`oplog-collections-transformer/users.go:119`). A `statusText` change is silently dropped (no metric) when the env is unset. Add a startup warn + `onSkipped("status_no_sites")`. *Effort: XS.*
+
+5. **`medium` — Replace `ev.Op`-derived skip labels with a fixed enum** (`subscriptions.go:98`, `users.go:56`, `threadsubs.go:58`). Connector-controlled `op` flows straight into a Prometheus label — latent unbounded cardinality. *Effort: XS.*
+
+6. **`medium` — Make subscription hard-delete observable** (`subscriptions.go:80-91`). Add a reason label or dedicated counter so delete volume is distinct from upserts. *Effort: XS.*
+
+7. **`medium` — Add a direct test for `pkg/migration/sourcelookup.go`** (`FindByID` miss/found/decode-error branches). Exported `pkg/` surface with no direct test violates CLAUDE.md §4. *Effort: S (integration) or refactor decode into a pure helper for a unit test.*
+
+8. **`medium` — Test `processOne` disposition wiring** in collections-transformer (`main.go:198`) with a fake `jetstream.Msg`, matching the sibling `oplog-transformer`'s `processone_test.go`. *Effort: S.*
+
+9. **`medium` — Add a `//go:generate mockgen` directive to inbox-worker** so `mock_store_test.go` can't drift silently (and consider a `store.go` to match the per-service layout). *Effort: XS.*
+
+10. **`low` (batch) — Quick hygiene:** hoist `nowMillis()` out of the fan loop (`users.go:114,128`); remove dead `jetstreamPublisher.siteID` (`inboxpublisher.go:18`); fix the stale `(threadRoomId, userId)` comment (`threadsubs.go:45`); converge `mustMarshal` vs `json.Marshal` strategy. *Effort: XS total.*
+
+**Deferred (acknowledged, not actioned):** resolver caches for `account→userId` / `parentMessage→threadRoom` (Performance #2) — user explicitly deferred. The `(threadRoomId,userAccount)` index-recreate crash-loop on pre-existing dup rows is inherent to the key migration (shared with message-worker) and surfaces loudly rather than losing data — ops note, not a code fix.