courtmitra / REVIEW_FINDINGS.md
Hetansh Waghela
Phase 1 complete: guided frontend + adversarial API fixes + i18n
d0de93d
|
Raw
History Blame Contribute Delete
6.11 kB

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.