inventory-plus-plus/work-summaries/work-summary-Claude-2026-08-05-2017.md

Work Summary — 2026-08-05 20:17

Task

Insight #2 from the domains/amazon design review: fire-and-forget listener notifications meant a dropped/failed SSE push was silently permanent, with the underlying event already marked processed. Fixed per a design worked out collaboratively with the user (not unilaterally chosen - this one had real tradeoffs).

Design (agreed with user before implementing)

Non-blocking dispatch is a hard constraint - events come from external systems whose APIs may change unexpectedly, so the dispatcher must never wait on the listener's real work. The agreed shape:

• Three states, driven by two nullable timestamps rather than a stored notify_again_at: unprocessed (notified_at IS NULL) → notified (notified_at set, processed_at still null) → processed (processed_at set).
• The dispatcher sets notified_at and calls the listener; it never sets processed_at itself.
• The listener signals completion via a callback passed into Notify itself: Notify(ctx, e, ack func(context.Context) error) error. ack can be called synchronously (fast listeners, like the real SSE one) or arbitrarily later from elsewhere (slow/async listeners) - exactly once is the contract, and extra calls are safe (see below).
• If an event sits in notified past a retry threshold without being acked, the dispatcher's normal loop re-notifies it - reusing the existing poll/reactive machinery, no new loop. The threshold (notifyRetryAfter) is a Go-side field with a sensible default, not a stored per-row timestamp, so the cadence can change without touching any row (explicit user preference over storing notify_again_at).

Changes

• database_migrations/000031_amazon_event_notify_ack.{up,down}.sql: drops processed/processed_successfully booleans on mock.shop_amazon_events, adds nullable notified_at/processed_at timestamps, updates the partial index accordingly. Applied to both the test DB and the real dev DB.
• domains/amazon/mock.go:
  • MockEventListener.Notify gained the ack parameter.
  • Mocks gained a notifyRetryAfter field (default 30s) and WithNotifyRetryAfter(d) builder for tuning/tests.
  • processUnprocessedEvents's query now selects events that are either brand new or notified-but-stale (processed_at IS NULL AND (notified_at IS NULL OR notified_at < retry_after)).
  • Restructured so the async dispatch to the listener happens after the transaction that recorded notified_at has committed, not from inside it - ack's own independent write can never race a still-open transaction it implicitly depends on being visible.
  • ack is idempotent (... AND processed_at IS NULL in its UPDATE), since a late ack from an earlier notification and a fresh one from a retry can both eventually fire for the same event.
• server/sse/db_event_publisher.go: the real listener now acks inline right after a successful SSE publish - its work is synchronous, so there's no reason to defer completion.
• domains/amazon/mock_test.go: updated notifySpy for the new signature (records ack callbacks, supports disabling auto-ack to simulate a listener that doesn't finish), updated isProcessed/added isNotified to check the new columns, and added TestProcessUnprocessedEvents_RetriesUnackedNotification - the core new-behavior test: a non-acking listener gets re-notified after the retry window (not before), and once any recorded ack for that event fires (including a stale one from an earlier attempt, not just the latest), the event settles into processed and stops being retried.

A test bug caught and fixed along the way (not production code)

First run: TestProcessUnprocessedEvents_ProcessesAllEventsAcrossBatches failed (8/150 events still unprocessed) and the new retry test hung/timed out. Both were bugs in my own notifySpy, not the production code:

• It signaled "notified" before calling the auto-ack, so a test could observe "all N notified" before all N acks had actually landed in the DB.
• waitForCount drained a fixed number of channel signals per call instead of tracking a cumulative total, so calling it twice (once for 1, once for 2) double-counted and hung waiting for a signal that would never come.

Fixed by replacing the channel with a cumulative completedCount incremented only after any auto-ack attempt returns, and made waitForCount poll that count (safe to call repeatedly with increasing thresholds).

Verification

• go build ./... / go vet ./... clean.
• go test ./domains/amazon/... -v -race: all 5 tests pass (4 existing + 1 new).
• 10x repeated runs (-count=1 -race) with no flakes, ~1.3-1.5s each.
• make test and make test-against-dev-db: full suite green both ways; confirmed zero leftover rows in the dev DB afterward.
• Migration applied cleanly to both the test DB and the real dev DB; verified \d mock.shop_amazon_events matches the intended shape on both.

Follow-ups / not done here

• Insights #3 (hardcoded 1-minute poll interval) and #4 (a LISTEN-connection error is fatal to the entire application, most consequential) remain open - continuing one at a time per the user's request.