feat(engine): inbound observability — health endpoint, forged-token signal, log gaps (HT-44) - #73
Conversation
…ignal, log gaps (HT-44) The observability half of HT-44 (the live-threading half passed 2026-07-17 and is recorded on the ticket). Makes the forged-token signal genuinely observable/alertable and closes the two real gaps in the structured logs. - Migration 019: `inbound_deliveries.forged_token_count`, written at the stored transition — decideThreading's forgedTokenCount becomes queryable, not just a log field. WARN-level `forged_token_detected` event per stored delivery carrying forged tokens (sender + target, for triage). - `GET /api/v1/internal/health` (CRON_SECRET-guarded): queue stats + 24h dead-letter growth, 24h ledger outcomes, 24h forged-token aggregate with a burst threshold, per-mailbox status + watch() expiry. 200-when-ok / 503-on-alert, so any status-code HTTP monitor is a complete alerter. - Append-fallback reason (deleted/not-found) surfaced on the stored outcome and log line — previously swallowed, leaving `threading: 'append'` records pointing at freshly created conversations. - `queue_drain` summary line for non-quiet drain ticks — the DrainReport previously existed only in a response body Vercel Cron discards. - Spec §6 rewritten to match (cursor position lives on transport events; forged-token consumption now real); runbook Part G: monitoring, alert codes → first response, log-event reference. Gates: typecheck, lint, test (51 files / 1070 tests) all green by their own exit codes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change persists forged-token counts and structured ingest signals, adds an aggregated internal health report and authenticated endpoint, wires health checks into the composition root, expands coverage, and documents smoke checks, alerting, and log correlation. ChangesHealth and forged-token observability
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant createAppHandler
participant runHealthCheck
participant Database
participant QueueAdapter
Client->>createAppHandler: GET /api/v1/internal/health with CRON_SECRET
createAppHandler->>runHealthCheck: execute health check
runHealthCheck->>Database: read aggregate health metrics
runHealthCheck->>QueueAdapter: getStats()
runHealthCheck-->>createAppHandler: HealthReport
createAppHandler-->>Client: report with HTTP 200 or 503
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/deploy/gmail-inbound-runbook.md`:
- Around line 273-276: Add an appropriate language identifier, such as text or
http, to the fenced code block containing the health-check request in the
deployment runbook, preserving its existing contents.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68a105ae-398d-441e-950b-b3c00fe4d297
📒 Files selected for processing (14)
specs/deploy/gmail-inbound-runbook.mdspecs/mail/inbound-ingestion.mdsrc/composition/app.test.tssrc/composition/app.tssrc/composition/health.test.tssrc/composition/health.tssrc/composition/root.test.tssrc/composition/root.tssrc/db/migrate.test.tssrc/db/migrate.tssrc/mail/ingest.test.tssrc/mail/ingest.tssrc/store/inbound-deliveries.test.tssrc/store/inbound-deliveries.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
The observability half of HT-44 — the live-threading-verification half passed on prod 2026-07-17 and is recorded on the ticket. Scope confirmed with the maintainer this session: close the two genuine log-field gaps, and make the forged-token signal + queue/watch health genuinely alertable with no alerting stack (health endpoint + status-code pivot), including migration 019.
The forged-token signal, consumed three ways (spec §6)
decideThreadinghas emittedforgedTokenCountsince HT-38; nothing consumed it. Now:inbound_deliveries.forged_token_count, written bymarkStoredInTxin the same fenced UPDATE as the stored transition. Queryable/aggregatable, not just a log field. (Stored-transition only, by design — documented in the migration's doc comment.)forged_token_detectedWARN event per stored delivery carrying ≥1 forged token, withsenderAddress+ target conversation — text-searchable by event name, severity-filterable.forged-token-burstat ≥5 forged-token deliveries (a code constant; threading.md §5's exact-threshold question deliberately stays open — the mechanism no longer is).GET /api/v1/internal/health(CRON_SECRET-guarded)Same auth/shape as the existing internal cron endpoints, one new status contract: 200 when
ok, 503 when any alert trips — so any status-code poller that can send one header (UptimeRobot/Checkly free tier) is a complete alerting stack. Report sections: queue (getStats()— previously implemented but unexposed — + 24h dead-letter growth), ledger 24h outcome counts + standing dead-letter total, forged-token 24h aggregate, per-mailbox status +watch()expiry. Six alert codes, each documented in runbook Part G with a first-response playbook.A considered-and-dropped check is documented in
health.ts's module doc: a reconcile-cursor-staleness alert can't be measured honestly with the current schema (gmail_watch_state.updated_atconflates cursor and renewal writes).Log-field gaps closed
deleted/not-found) — previously swallowed inwriteParsedEmail, so a fallback'sinbound_ingestline readthreading: 'append'while pointing at a conversation the write itself created. Now on the stored outcome + log line.queue_drainsummary line — theDrainReport(incl.staleSkipped, the drains-overlapping signal) previously existed only in a response body Vercel Cron discards. Logged for non-quiet ticks.gmail_reconciletransport events; spec §6 now says so explicitly instead of implying it rides each pipeline record. Deliberately NOT threaded throughRawInboundMessage— the pipeline stays provider-agnostic (§2); streams correlate on(mailboxId, providerMessageId).Docs
specs/mail/inbound-ingestion.md§6 rewritten to match reality (fields, cursor ownership, the three consumers).specs/deploy/gmail-inbound-runbook.md: Part F gains the health smoke line; new Part G — endpoint contract, monitor wiring (incl. the CRON_SECRET-sharing tradeoff), alert-code → first-response table, structured-event reference.Mail-semantics note (charter invariant #5)
No threading/ingest behavior changes: the append-fallback path, forged-token handling, and all decisions are untouched — this PR only records and surfaces what already happens. The one store-write change is
markStoredInTxcarrying one extra column in the same fenced UPDATE. All 8 observed-fixture-derived acceptance behaviors still pass through the pipeline suite.Testing
npm run typecheck/npm run lint/npm run test— all green by their own exit codes on the final tree: 51 files / 1070 tests (baseline 50/1055; +15).health.test.ts(PGlite + the realcreatePostgresQueue.getStats(), every alert exercised both tripped and quiet), health-route tests inapp.test.ts(200/503 pivot, 401, 405, verbatim body), a composition drive-through inroot.test.ts, migration-019 non-fresh upgrade test, forged-token pipeline test (ledger column + WARN event), append-fallback assertions on the existing fallback tests.🤖 Generated with Claude Code
Summary by CodeRabbit