Fix/sr observability aml notification gaps - #1410
Merged
GoodnessJohn merged 4 commits intoAug 28, 2026
Merged
Conversation
tracedJob() generated a correlation ID and attached it only as an OTel span attribute — it never called correlationStorage.run(), so every createLogger(...) log line emitted inside a scheduled job read getCorrelationId() as undefined and could not be cross-referenced against the span for that run. - tracedJob() now runs the job body inside correlationStorage.run(correlationId, fn), so logs and the span share one ID. - Wrapped the three AML jobs (aml-periodic-rescreening, aml-travel-rule-transmit, aml-data-retention) in tracedJob, which previously had neither a span nor a correlation ID. - Added a test asserting a log line emitted inside a tracedJob-wrapped function carries the same correlation_id as the span attribute.
…mittance creation TransactionMonitoringService.evaluateTransfer() and TravelRuleService.assess() were fully implemented and unit-tested but only reachable via officer-triggered calls to routes/aml.ts (or the scheduler's transmitPending retry). The live POST /api/remittance path only ran autoFlagIfAboveThreshold — a simple threshold check — so structuring/velocity/corridor monitoring and travel-rule data collection never executed for a real remittance unless an officer manually replayed it through the AML API. - POST /api/remittance now calls monitoring.evaluateTransfer() and travelRule.assess() synchronously right after the transaction row is inserted, mirroring the existing autoFlagIfAboveThreshold best-effort pattern (logged, non-fatal to the response). - Added an integration test asserting that creating a remittance which trips a monitoring rule raises an aml_alerts row with no manual call to /api/aml/monitoring/evaluate.
SarWorkflowService.nextReference() derived the sequence number from SELECT COUNT(*) FROM sar_reports WHERE reference LIKE 'SAR-<year>-%', with no locking, and the subsequent INSERT INTO sar_reports happened in a separate statement. Two officers (or an officer and a batch process) calling createFromAlerts() concurrently within the same year could read the same count before either INSERT committed, so one of the two SAR drafts would fail with a unique-constraint violation on `reference` instead of a clean, distinct number — surfacing as an unhandled 500 for a legally-mandated filing. - Added a sar_reference_counters table (one row per calendar year) and a migration that seeds it from the highest existing reference so numbering continues rather than restarting. - nextReference() now claims its sequence number via a single atomic INSERT ... ON CONFLICT DO UPDATE ... RETURNING statement, which Postgres serializes at the row level, instead of a racy count-then- insert. - Added a concurrency test that fires createFromAlerts() twice (and ten times) in parallel and asserts every reference is distinct.
… live event paths
NotificationService (SendGrid email + Twilio SMS, with SR-035 localized
templates) was only ever referenced inside its own file —
notifyRemittanceStatus()/notifyKycEvent() were never called from
RemittanceEventEmitter, routes/remittance.ts, kyc-webhook-handler.ts, or
the scheduler. Separately, RemittanceEventEmitter.setWebhookService() was
never called in production, so this.webhookService was always undefined
and the webhook fan-out branch in emitStatusChange() never ran.
- RemittanceEventEmitter gained setNotificationService() and now calls
notifyRemittanceStatus() from emitStatusChange() on completed/failed
status changes (best-effort, mirrors the existing webhook branch).
- kyc-webhook-handler.ts now calls notifyKycEvent('kyc_approved', ...)
when an anchor pushes an approval, the one KYC event this handler
observes.
- index.ts now instantiates NotificationService and WebhookService at
startup and wires both into remittanceEventEmitter, so the webhook
fan-out path (setWebhookService) and the notification path
(setNotificationService) both actually fire in production.
- Added a test asserting a status-change event triggers outbound
SendGrid/Twilio calls once wired, and that opt-out/unwired states
correctly send nothing.
|
@dami-005 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@dami-005 is attempting to deploy a commit to the Harold's projects Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Linked Issue
Closes #1279
Closes #1277
Closes #1278
Closes #1280
Type of Change
Checklist
sdk/) if this touches a published packageBreaking Change Notes