feat: Mercury webhook real-time classification + ChittySchema integration#90
Conversation
…egration Mercury webhook (/api/webhooks/mercury) now persists transactions to the DB with auto-classified suggestedCoaCode populated at ingest time. This completes the L0 ingest path of the trust-path model for the real-time Mercury flow (previously the webhook only deduped and logged, never persisted). Flow: 1. Service-auth check (Bearer token) 2. Zod envelope validation 3. KV idempotency (7-day TTL) 4. Advisory ChittySchema validation (never blocks) 5. DB dedup via externalId (retries-safe) 6. createTransaction with suggestedCoaCode + classificationConfidence 7. ledgerLog audit entry Auto-classification uses findAccountCode() keyword matcher (same path as TurboTenant import). Suspense (9010) gets 0.100 confidence; matched codes get 0.700. ChittySchema integration (server/lib/chittyschema.ts): - New Workers-native client — no process.env at module load - Speaks the real ChittySchema API (/api/health, /api/validate, /api/tables) - Fall-open on 5xx / network error / timeout / malformed body — never blocks financial writes - Advisory pass when table is not registered (surfaces availableTables so operators can see what's in the registry) - Normalizes error shapes (string[] and object[] both supported) - Three functions: validateRow, listTables, checkHealth Cleanup: - Deleted server/lib/chittyschema-validation.ts — stale, used wrong API paths (/api/v1/validate), had Express middleware (dead code), relied on process.env which doesn't work in Workers. Never imported anywhere. - Removed ChittySchemaClient class and getChittySchema factory from chittyos-client.ts — same issues, never imported. Added CHITTYSCHEMA_URL? to the env type for optional override. Tests (25 new, 246 total): - server/__tests__/chittyschema.test.ts (16): valid/invalid results, fall-open on 5xx/network/timeout/malformed-body, unregistered-table advisory pass, default base URL, listTables, checkHealth - server/__tests__/webhooks-mercury.test.ts (10): auth rejection, invalid envelope, successful persistence with auto-classification, suspense fallback, KV dedup, externalId dedup, non-blocking schema validation failure, unreachable schema, envelope-only ack Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR replaces the deprecated ChittySchema integration with a new worker-compatible module. It introduces Changes
Sequence DiagramsequenceDiagram
participant Client as Webhook Client
participant Handler as Mercury Handler
participant KV as KV Store
participant ChittySchema as ChittySchema Service
participant DB as SystemStorage (DB)
participant Audit as ledgerLog
Client->>Handler: POST /api/webhooks/mercury
Handler->>Handler: Parse & validate envelope (Zod)
Handler->>KV: Check eventId exists (7-day TTL)
alt Duplicate Event
KV-->>Handler: Found
Handler->>Client: 202 duplicate: true
else New Event
KV->>KV: Store eventId
Handler->>Handler: Compute COA code & confidence
Handler->>ChittySchema: POST /api/validate (advisory)
alt ChittySchema Responds
ChittySchema-->>Handler: valid/invalid result
Handler->>Handler: Log warnings if invalid
else ChittySchema Fails
Handler->>Handler: Set schemaAdvisory: true
end
Handler->>DB: Check externalId exists
alt Transaction Exists
DB-->>Handler: Existing transactionId
Handler->>Client: 202 with existing ID
else New Transaction
Handler->>DB: createTransaction (with metadata)
DB-->>Handler: Created transactionId
Handler->>Audit: Emit ledgerLog entry
Handler->>Client: 201 with transactionId, COA, confidence
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittyfinance | 5f7ad4f | Apr 10 2026, 11:30 PM |
Code Review — PR #90: Mercury webhook real-time classification + ChittySchema integrationOverall this is solid, well-scoped work. The fall-open philosophy is correctly implemented, the dual-dedup strategy is clever, and the test coverage is thorough. A few issues worth addressing before merge. Bugs / Issues1. KV dedup write races the DB write — KV is written before the duplicate check ( await kv.put(dedupKey, JSON.stringify(rawBody || {}), { expirationTtl: 604800 });
// ... then later ...
const dupRow = await storage.getTransactionByExternalId(externalId, tx.tenantId);The KV entry is committed on line ~125, but if the DB write fails (network blip, constraint error, etc.) the event is permanently swallowed — a retry will hit the KV dedup and return 2. The KV dedup fires before the 3. The envelope schema marks both if (!eventId) {
return c.json({ error: 'missing_event_id' }, 400);
}returns 4. const rawBody = await c.req.json().catch(() => null);
// ...
await kv.put(dedupKey, JSON.stringify(rawBody || {}), ...);If Security5. Service auth uses constant-time comparison? if (!authHeader || !authHeader.startsWith('Bearer ') || authHeader.slice(7) !== expected) {String inequality Design / Consistency6. Confidence values are hardcoded as string literals — same values used in other classifiers? const classificationConfidence = isSuspense ? '0.100' : '0.700';The schema column is 7. type: tx.amount >= 0 ? 'income' : 'expense', // in createTransaction
// ...
type: tx.amount >= 0 ? 'income' : 'expense', // in the validateRow payloadTwo identical expressions in the same function. Extract to a 8. The CLAUDE.md notes that Performance9. const db = createDb(c.env.DATABASE_URL);
const storage = new SystemStorage(db);Webhook routes skip Minor / Style
TestsThe test suite is excellent — 10 cases covering auth, dedup, classification, and all the fall-open paths. One gap: Missing test: DB There's no test for the case where Summary
The KV-before-DB race (#1) is the most important to fix before merge — it can silently lose transactions in retry scenarios. Everything else is low-to-medium severity. Happy to discuss any of these. 🤖 Generated with Claude Code |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f7ad4ff48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (existing) { | ||
| return c.json({ received: true, duplicate: true }, 202); | ||
| } | ||
| await kv.put(dedupKey, JSON.stringify(rawBody || {}), { expirationTtl: 604800 }); |
There was a problem hiding this comment.
The handler stores webhook:mercury:${eventId} in KV before any DB persistence, so a transient failure in getTransactionByExternalId/createTransaction will still mark the event as processed. On retry, the same event returns 202 duplicate and the transaction is permanently dropped unless manually replayed. This turns temporary infra errors into data loss for webhook ingest.
Useful? React with 👍 / 👎.
| const dupRow = await storage.getTransactionByExternalId(externalId, tx.tenantId); | ||
| if (dupRow) { |
There was a problem hiding this comment.
Dedup is implemented as a read (getTransactionByExternalId) followed by a separate insert, which is race-prone under concurrent deliveries of the same webhook. Two requests can both observe no existing row and both insert. Since this code path relies on application-level checks (not an atomic upsert/constraint), duplicate transactions can be created during retries or parallel webhook delivery bursts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds real-time COA suggestion at Mercury webhook ingest and introduces a Workers-native ChittySchema client for advisory validation, aligning the ingest path with the COA trust-path initiative and modernizing schema validation integration.
Changes:
- Persist Mercury webhook transactions with L1 keyword-based
suggestedCoaCode+ confidence at ingest time. - Add
server/lib/chittyschema.tsclient (health, validate, list tables) with fall-open advisory behavior; remove stale/api/v1/*integrations. - Add test coverage for the new webhook ingest flow and ChittySchema client behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/routes/webhooks.ts | Implements Mercury webhook envelope validation, KV+DB dedup, advisory ChittySchema validation, auto-classification, and persistence. |
| server/lib/chittyschema.ts | Adds Workers-native ChittySchema client with fall-open validation semantics and utilities. |
| server/lib/chittyschema-validation.ts | Removes legacy schema validation module using incorrect endpoints and Node env assumptions. |
| server/lib/chittyos-client.ts | Removes deprecated ChittySchemaClient and factory export. |
| server/env.ts | Adds optional CHITTYSCHEMA_URL override. |
| server/tests/webhooks-mercury.test.ts | Adds end-to-end tests for webhook auth/validation/dedup/persistence/classification. |
| server/tests/chittyschema.test.ts | Adds unit tests for ChittySchema client fall-open and response normalization behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const existing = await kv.get(dedupKey); | ||
| if (existing) { | ||
| return c.json({ received: true, duplicate: true }, 202); | ||
| } | ||
| await kv.put(dedupKey, JSON.stringify(rawBody || {}), { expirationTtl: 604800 }); |
There was a problem hiding this comment.
KV idempotency key is written before any DB persistence/dedup happens. If the handler errors after this kv.put (DB outage, transient exception, etc.), subsequent retries with the same eventId will be treated as duplicates and the transaction will never be persisted. Consider only writing the KV marker after the DB insert succeeds (or storing an “inflight” marker with short TTL and overwriting it with a “done/transactionId” marker once persistence completes).
| description: z.string(), | ||
| amount: z.number(), // signed: negative = expense | ||
| category: z.string().optional().nullable(), | ||
| postedAt: z.string(), // ISO 8601 |
There was a problem hiding this comment.
postedAt is only validated as a generic string, but later it’s converted with new Date(tx.postedAt) for insertion into a timestamp column. A non-ISO or otherwise invalid value would pass zod validation and then produce an Invalid Date, likely causing a DB insert error. Consider validating postedAt as an ISO datetime (e.g. zod datetime()) and/or parsing/coercing it to a Date as part of validation.
| postedAt: z.string(), // ISO 8601 | |
| postedAt: z.string().datetime({ offset: true }), // ISO 8601 |
| // 2. KV-based idempotency (7-day TTL dedup window) | ||
| // 3. Parse + validate envelope with zod | ||
| // 4. If payload carries a transaction, persist it to the DB with an | ||
| // auto-suggested COA code (L0 → L1 keyword match at ingest) | ||
| // 5. Advisory validation against ChittySchema's FinancialTransactionsSchema | ||
| // — never blocks the write, only logs warnings | ||
| // | ||
| // Returns: | ||
| // 200 { received, duplicate? } — envelope-only events (no transaction) | ||
| // 201 { received, transactionId, suggestedCoaCode, schemaAdvisory? } — persisted tx | ||
| // 400 on validation failure (envelope or payload) |
There was a problem hiding this comment.
The return-code docs in this comment block don’t match the implementation: envelope-only events and KV duplicates return 202 (not 200), and the flow ordering described above also differs from the actual code (envelope parse happens before KV). Please update the comment to reflect the real behavior so operators don’t rely on incorrect status codes.
| // 2. KV-based idempotency (7-day TTL dedup window) | |
| // 3. Parse + validate envelope with zod | |
| // 4. If payload carries a transaction, persist it to the DB with an | |
| // auto-suggested COA code (L0 → L1 keyword match at ingest) | |
| // 5. Advisory validation against ChittySchema's FinancialTransactionsSchema | |
| // — never blocks the write, only logs warnings | |
| // | |
| // Returns: | |
| // 200 { received, duplicate? } — envelope-only events (no transaction) | |
| // 201 { received, transactionId, suggestedCoaCode, schemaAdvisory? } — persisted tx | |
| // 400 on validation failure (envelope or payload) | |
| // 2. Parse + validate envelope with zod | |
| // 3. KV-based idempotency (7-day TTL dedup window) | |
| // 4. If payload carries a transaction, persist it to the DB with an | |
| // auto-suggested COA code (L0 → L1 keyword match at ingest) | |
| // 5. Advisory validation against ChittySchema's FinancialTransactionsSchema | |
| // — never blocks the write, only logs warnings | |
| // | |
| // Returns: | |
| // 202 { received, duplicate: true } — duplicate event detected via KV | |
| // 202 { received } — envelope-only events (no transaction) | |
| // 201 { received, transactionId, suggestedCoaCode, schemaAdvisory? } — persisted tx | |
| // 400 on validation failure (invalid envelope or transaction payload) |
| const DEFAULT_TIMEOUT_MS = 3000; | ||
|
|
||
| function baseUrlFromEnv(env: { CHITTYSCHEMA_URL?: string }): string { | ||
| return env.CHITTYSCHEMA_URL || DEFAULT_BASE_URL; |
There was a problem hiding this comment.
baseUrlFromEnv() returns the env override verbatim. If CHITTYSCHEMA_URL is configured with a trailing slash, requests will go to ...//api/validate / ...//api/health. Consider normalizing by trimming a trailing slash (consistent with other clients in this repo) before appending paths.
| return env.CHITTYSCHEMA_URL || DEFAULT_BASE_URL; | |
| const baseUrl = env.CHITTYSCHEMA_URL || DEFAULT_BASE_URL; | |
| return baseUrl.replace(/\/+$/, ''); |
Summary
Completes Phase 2 of the COA trust-path initiative by adding real-time classification at Mercury webhook ingest and modernizing the ChittySchema client for Cloudflare Workers.
Mercury webhook flow
The Mercury webhook (
POST /api/webhooks/mercury) previously only deduped and logged. It now persists transactions to the DB with auto-classifiedsuggestedCoaCodepopulated at ingest time (L0 → L1 keyword suggestion).CHITTY_AUTH_SERVICE_TOKEN, else 401{id, type, data.transaction}with UUID tenant/accountwebhook:mercury:${eventId}with 7-day TTLvalidateRow()againstFinancialTransactionsInsertSchema— never blocksexternalId = mercury:${mercuryTransactionId}unique checkfindAccountCode()→suggestedCoaCode+ confidence, thencreateTransaction()ledgerLogentry withschemaValid+schemaAdvisoryflagsEnvelope-only events (e.g.
account.created) still get 202 without persistence.ChittySchema integration
server/lib/chittyschema.tsis a fresh Workers-native client. It replaces two stale modules that were never imported but carried the wrong API paths:server/lib/chittyschema-validation.ts— used/api/v1/validate(404), Express middleware,process.envChittySchemaClientinchittyos-client.ts— same issuesVerified live against https://schema.chitty.cc:
GET /api/health→{status:"ok",service:"chittyschema",version:"1.0.0"}POST /api/validatewith{table,data}→{valid,errors,availableTables?}GET /api/tables→ 35 registered tables across chittyos-core, chittyledger, chittycanonFall-open philosophy
Validation is advisory. External schema services must never block financial writes. The client returns
{ok: true, advisory: true}for:availableTablessurfaced for operators)Only a clean
{valid: false, errors: [...]}response counts as a validation failure, and even then the Mercury handler logs a warning and proceeds.Registry state
chart_of_accountsandclassification_auditare not yet registered in ChittySchema — the Mercury webhook usesFinancialTransactionsInsertSchemawhich is already in the chittyledger database. A follow-up PR can register the COA tables with the schema team once they add achittyfinancedatabase namespace.Tests (25 new, 246 total, 0 TypeScript errors)
server/__tests__/chittyschema.test.ts(16): validation result shapes, fall-open on 5xx/network/timeout/malformed body, unregistered-table advisory, default base URL, listTables, checkHealthserver/__tests__/webhooks-mercury.test.ts(10): auth rejection, invalid envelope, successful persistence with auto-classification, suspense fallback (9010 @ 0.1), KV dedup, externalId dedup, non-blocking schema failure, unreachable schema, envelope-only ackTest plan
CHITTYSCHEMA_URLto an unreachable host → verify webhook still persists withschemaAdvisory: true🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores