Repository navigation
feat: low-stock and conflict-needs-review email alerts via SES - #17
Merged
Merged
Conversation
Adds notifyAlerts.ts, a new Lambda consuming inventory_records' DynamoDB Stream — enabled since Phase 1 but never actually wired to anything (the existing real-time push to connected clients goes through wsPush.ts's direct in-process call from conflictResolver.ts instead). This makes the notifications feature a pure read-side addition: zero changes to the correctness-critical write pipeline. For each stream record, compares OLD vs NEW images and fires an email only on the edge that matters: - stock crossing at/under a configurable threshold (default 5), not every write to an already-low item - conflict_status newly becoming needs_review, not every write to an already-flagged item The recipient is looked up dynamically per-shop: Cognito's "owner" group holds every shop's owner together, so notifyAlerts.ts pages through ListUsersInGroup and filters by custom:shop_id client-side (no new shop->owner index needed at this scale). SES send failures are caught and logged, never thrown — same degrade-gracefully principle as bedrock.ts's non-blocking explainer, since alerts are advisory and must never affect the resolver pipeline. SES starts every AWS account in sandbox mode, where both sender and every recipient must be individually verified. config.ts's alertsFromEmail (kisore2004@gmail.com) is used for both roles for now, documented as a known constraint — real per-shop delivery needs a production-access request, same shape as the existing Bedrock account-gate workaround. Also adds one narrow, documented exception to the CDK "no wildcard IAM resources" test: dynamodb:ListStreams has no resource-level IAM scoping at all (confirmed against the synthesized template) — CDK's own DynamoEventSource construct always generates it against Resource: "*". Every other action in the alert function's policy stays scoped. Tests: 11 new API tests (notifyAlerts.test.ts, covering the crossing logic, owner lookup + pagination, REMOVE-event skip, graceful failure on no-owner-found/SES-rejection/malformed-record), 3 new CDK tests for the Notifications construct. Full suites verified green: 100/100 API, 37/37 CDK. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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.
Adds notifyAlerts.ts, a new Lambda consuming inventory_records' DynamoDB Stream — enabled since Phase 1 but never actually wired to anything (the existing real-time push to connected clients goes through wsPush.ts's direct in-process call from conflictResolver.ts instead). This makes the notifications feature a pure read-side addition: zero changes to the correctness-critical write pipeline.
For each stream record, compares OLD vs NEW images and fires an email only on the edge that matters:
The recipient is looked up dynamically per-shop: Cognito's "owner" group holds every shop's owner together, so notifyAlerts.ts pages through ListUsersInGroup and filters by custom:shop_id client-side (no new shop->owner index needed at this scale). SES send failures are caught and logged, never thrown — same degrade-gracefully principle as bedrock.ts's non-blocking explainer, since alerts are advisory and must never affect the resolver pipeline.
SES starts every AWS account in sandbox mode, where both sender and every recipient must be individually verified. config.ts's alertsFromEmail (kisore2004@gmail.com) is used for both roles for now, documented as a known constraint — real per-shop delivery needs a production-access request, same shape as the existing Bedrock account-gate workaround.
Also adds one narrow, documented exception to the CDK "no wildcard IAM resources" test: dynamodb:ListStreams has no resource-level IAM scoping at all (confirmed against the synthesized template) — CDK's own DynamoEventSource construct always generates it against Resource: "*". Every other action in the alert function's policy stays scoped.
Tests: 11 new API tests (notifyAlerts.test.ts, covering the crossing logic, owner lookup + pagination, REMOVE-event skip, graceful failure on no-owner-found/SES-rejection/malformed-record), 3 new CDK tests for the Notifications construct. Full suites verified green: 100/100 API, 37/37 CDK.