REVIEW_FINDINGS.md

# CourtMitra — Adversarial Review Findings

**Date:** 2026-05-29 · **Commit:** 3d2be97 · **Reviewer:** automated + manual

> Supersedes `GAP_AUDIT_M1_M9.md`, which was written against a phantom commit
> (`f892836`) that is NOT in `main`. That audit describes an autopilot
> `ToolLoopAgent`, parallel tracks, and autopilot ticks as "existing with bugs,"
> but **none of that code exists** in the current tree. Treat it as a wishlist.

Scope (agreed with maintainer): **loop-critical bugs only** — anything that
breaks the end-to-end loop or violates an invariant (consent / proof_state /
redaction / citation / idempotency). M4–M9 advanced-feature gaps are deferred.

Confirmation legend: **[E]** empirically reproduced via curl · **[C]** confirmed
by reading code · **[ ]** suspected, not yet verified.

---

## Stack bring-up (local dev)
- Postgres = Homebrew pg17 + pgvector, role `courtmitra` (NOT Docker). 55 tables migrated, 45 legal chunks + 4 route rules seeded.
- `infra/.env` did not exist → created for local dev. API/worker do NOT auto-load it (only `db/migrate` + `db/seed` do); they read the process env. Run services with the env sourced: `set -a && . infra/.env && set +a && pnpm --filter <app> dev`.
- Inngest dev server is normally from Docker (`pnpm infra:up`); Docker was off. Run standalone: `npx inngest-cli@latest dev -u http://localhost:3002/api/inngest`.
- Harness: `scripts/smoke-loop.sh` drives the full loop + probes the gates.

---

## Findings

### F1 — API 500s on a user write when the event bus is down  **[E] loop-critical**
`POST /api/missions` returned **500** (`TypeError: fetch failed`, ECONNREFUSED ::1:8288) because `MissionsService.create` → `WorkflowService.send('mission.created')` throws when Inngest is unreachable. The mission row is inserted *before* the send, so a bus outage yields an **orphaned partial write + 500 to the user**.
- **FIXED (2026-05-29):** `WorkflowService.send` now wraps the `inngest.send` call in a try/catch — logs the failure and continues instead of throwing. Events `mission.created` and `message.received` are best-effort; loop-critical events (`route_plan_requested`, `action.execute_requested`, `mission.facts_confirmed`) also log failures but will be visible via the mission status snapshot. A transactional outbox is the Phase-2 upgrade.
- Immediate cause resolved by running the Inngest dev server (create now 201).

### F2 — A failed action permanently bricks the mission  **[C] loop-critical**
`apps/worker/src/lib/action-executor.ts`: on send failure `newState = "failed"` (STATE_RANK **22**, the highest). The snapshot rank-guard only writes when `newRank >= currentRank`, so once a mission is "failed" (22) **no lower-ranked snapshot can ever be written again** — including the `waiting_for_response` (12) a retry would produce. A single failed email send terminally wedges the mission.
- **FIXED (2026-05-29):** On action failure, the mission stays at its current state (`newState = latestSnapshot.state`). The failure snapshot (danger + "Retry" button) writes because `newRank == currentRank` passes the rank-guard, while forward progress to `waiting_for_response` remains possible on retry.

### F3 — Double-send possible on worker retry  **[C] invariant #3/#7**
`executeAction` sets `status="executing"` (txn1, committed), calls `emailAdapter.submit` (network), then records success (txn2). The whole thing is one Inngest `step.run`. If the step throws after txn1 but before txn2 (crash / Inngest retry), re-entry passes `canExecute` again (`"executing"` is allowed, `proofState` still null) and **re-calls the adapter**. `ResendEmailAdapter.doSend` passed **no idempotency key** to Resend, so the provider won't dedupe → duplicate email.
- **FIXED (2026-05-29):** (a) Before creating a new attempt, `executeAction` checks for an existing "started" attempt. If one exists (Inngest retry), it reuses the same attempt record instead of creating a new one. (b) `ResendEmailAdapter.doSend` now passes the `idempotencyKey` from `SubmissionRequest` to Resend's SDK as a second argument (Resend SDK v6 API). Combined, these prevent double-sends: the adapter check prevents re-calling when a prior attempt exists, and the Resend-level key dedupes any race.

### F4 — `STATE_RANK` is duplicated and has drifted  **[C] latent**
Separate copies existed in `action-executor.ts`, `route-plan.ts`, `actions.service.ts` (full 0–22 map) and `resolution-verify.ts`, `evidence.ts`, `intake.ts`, `reply-classify.ts` (partial maps). The divergence (closed_unresolved 18 vs 19, failed 18 vs 22) was masked by the `-1` fallback.
- **FIXED (2026-05-29):** Canonical `STATE_RANK` exported from `packages/contracts/src/enums.ts` (next to `MissionState`). All 7 files now import from `@courtmitra/contracts` — no local copies. Kills the whole drift class.

---

## Verified working (no change needed)
- **[E]** Read endpoints: `/admin/health`, `/capabilities`, `/missions`, `/ledger`, WhatsApp webhook verify (403 without token).
- **[E]** `/capabilities` honestly reports LLM + channel adapters as `blocked` when keys are absent.
- **[C]** Consent-hash gate is sound on the email path: preview and executor both canonicalize with `public:false` + normalized body → hashes match; a changed body would mismatch and block (`consent_hash_mismatch`).
- **[C]** Defamation gate (`checkDefamation`) runs at preview-time and again before the public-case-page write.
- **[C]** Redaction degrades gracefully without an LLM key: `RegexLlmRedactionAdapter.detect` catches LLM failure and falls back to regex — the redaction invariant holds key-free.
- **[C]** Resolution → ledger only emits `ledger.outcome_recorded` when `publishConsent` is true (consent-gated ledger).

## Pending empirical verification (needs LLM key for intake + route-plan)
- Full loop: intake → facts → route-plan (with verified citations) → preview → consent → execute → proof_state → reply → resolution → ledger.
- Live reproduction of F2/F3 during a real `execute`.
- Citation literal-substring gate on generated route-plan claims.