[minor-7] Silent emission drop on reserve/append failure

internal/notify/emit.go:194-197 (reserveSeq failure → bare return: no notification, no activity event, no log) and :203, :219 (_ = s.appendActivity(...) in the overflow path ignores the error yet still arms the fanout task, whose fanoutOne treats readActivity == nil as done — tasks.go:266-269). A transient store error permanently drops the event's recipients with zero trace.

Fix

Log (at minimum) on every drop path; on append failure do NOT arm fanout for a nonexistent event (or arm a retry that re-reads). Regression tests (fault-injected reserve/append failures → logged + no phantom fanout arming). Coverage gate holds.

Acceptance criteria

No silent drops: every failure path logs or retries; tests green -race.

# [minor-7] Silent emission drop on reserve/append failure `internal/notify/emit.go:194-197` (`reserveSeq` failure → bare `return`: no notification, no activity event, no log) and `:203`, `:219` (`_ = s.appendActivity(...)` in the overflow path ignores the error yet still arms the fanout task, whose `fanoutOne` treats `readActivity == nil` as done — `tasks.go:266-269`). A transient store error permanently drops the event's recipients with zero trace. ## Fix Log (at minimum) on every drop path; on append failure do NOT arm fanout for a nonexistent event (or arm a retry that re-reads). Regression tests (fault-injected reserve/append failures → logged + no phantom fanout arming). Coverage gate holds. ## Acceptance criteria - [ ] No silent drops: every failure path logs or retries; tests green `-race`.

crueber referenced this issue from a commit

2026-09-05 02:57:04 +00:00

Fix #92: no silent emission drops (06 §4)

crueber referenced this issue from a pull request that will close it,

2026-09-05 02:57:06 +00:00

Fix #92: no silent emission drops #101

crueber commented

2026-09-05 02:57:09 +00:00

Author

Owner

Fixed by #101 (branch fix/issue-92): every emit drop path now logs (Service.Logger, events-bridge nil→discard convention) and append failures no longer arm phantom notify-fanout tasks. internal/notify 97.3% cover, -race green.

crueber referenced this issue from a commit

2026-09-05 02:59:03 +00:00

Review fix: clarify reserve-path log has no seq yet (issue #92)

crueber commented

2026-09-05 02:59:11 +00:00

Author

Owner

PR #101 review (branch fix/issue-92, verified in scratch worktree at d19ed98):

DROP PATHS — all four log, fields clean:

emit.go:196-202 reserveSeq failure: logs repo/num/class/actor+err (no seq: none reserved yet — correct).
emit.go:213-218 overflow append failure: logs repo/num/class/actor/seq/recipients-count+err, returns before enqueueFanout/wakeRepo.
emit.go:238-247 shortfall append failure: logs +recipients/done counts, publishes landed done entries, no fanout.
emit.go:260-269 sync append failure: logs, publishes landed tray entries, wakeRepo (webhooks), no fanout.
No secret material: counts only (no principal lists, no Detail/Title, no tokens); err is the store error. (Pre-existing note, out of scope: malformed-repo early return emit.go:171-174 stays unlogged — unreachable via composition.)

NO PHANTOM FANOUT — other arming sites checked: emit.go:219,248 only run after append==nil; tasks.go:487 redrain only enqueues existing events with FanoutPending and no completion record (gaps skipped tasks.go:476-478; transient read errors halt the window, no high-water advance). drainFanout marks done only when fanoutOne==true (tasks.go:332-334).

SYNC SUCCESS PATH unchanged (publish done + wakeRepo, emit.go:270-273). No regression.

LOGGER: log() nil->discard (notify.go:451-456, events-bridge convention); cmd wires slog.Default (cmd/walhub/notify.go:33); only New() call site in cmd; direct test construction safe — covered by TestEmitLogDiscardsWhenUnwired. No nil-pointer.

FANOUTONE CLAIM: issue text stale, code correct — fanoutOne returns false on nil activity (tasks.go:356-359), never marks done for gaps. Real harm was arming a task that drains nothing with zero trace, which this PR removes. Doc wording '(would probe a gap and do nothing)' accurate.

TESTS: updated TestCreateOneStoreError (cover_test.go:393-405) asserts the right new invariant (no fanout for nonexistent event). New emit_drop_test.go (4 drop tests + nil-logger test) genuine: fault-inject reserve/append, assert log substring + no activity + no fanout + tray counts (0/0/0/1 kept). All pass -race.

GATES: internal/notify 97.0% statements (>=95%); go test -race green ./internal/notify/... ./cmd/...; gofmt clean; go vet clean both. No new non-stdlib imports (log/slog stdlib only). Doc entry accurate after review one-liner (d19ed98: reserve-path log has no seq yet).

RECOMMENDATION: ready to merge.

PR #101 review (branch fix/issue-92, verified in scratch worktree at d19ed98): DROP PATHS — all four log, fields clean: - emit.go:196-202 reserveSeq failure: logs repo/num/class/actor+err (no seq: none reserved yet — correct). - emit.go:213-218 overflow append failure: logs repo/num/class/actor/seq/recipients-count+err, returns before enqueueFanout/wakeRepo. - emit.go:238-247 shortfall append failure: logs +recipients/done counts, publishes landed done entries, no fanout. - emit.go:260-269 sync append failure: logs, publishes landed tray entries, wakeRepo (webhooks), no fanout. - No secret material: counts only (no principal lists, no Detail/Title, no tokens); err is the store error. (Pre-existing note, out of scope: malformed-repo early return emit.go:171-174 stays unlogged — unreachable via composition.) NO PHANTOM FANOUT — other arming sites checked: emit.go:219,248 only run after append==nil; tasks.go:487 redrain only enqueues existing events with FanoutPending and no completion record (gaps skipped tasks.go:476-478; transient read errors halt the window, no high-water advance). drainFanout marks done only when fanoutOne==true (tasks.go:332-334). SYNC SUCCESS PATH unchanged (publish done + wakeRepo, emit.go:270-273). No regression. LOGGER: log() nil->discard (notify.go:451-456, events-bridge convention); cmd wires slog.Default (cmd/walhub/notify.go:33); only New() call site in cmd; direct test construction safe — covered by TestEmitLogDiscardsWhenUnwired. No nil-pointer. FANOUTONE CLAIM: issue text stale, code correct — fanoutOne returns false on nil activity (tasks.go:356-359), never marks done for gaps. Real harm was arming a task that drains nothing with zero trace, which this PR removes. Doc wording '(would probe a gap and do nothing)' accurate. TESTS: updated TestCreateOneStoreError (cover_test.go:393-405) asserts the right new invariant (no fanout for nonexistent event). New emit_drop_test.go (4 drop tests + nil-logger test) genuine: fault-inject reserve/append, assert log substring + no activity + no fanout + tray counts (0/0/0/1 kept). All pass -race. GATES: internal/notify 97.0% statements (>=95%); go test -race green ./internal/notify/... ./cmd/...; gofmt clean; go vet clean both. No new non-stdlib imports (log/slog stdlib only). Doc entry accurate after review one-liner (d19ed98: reserve-path log has no seq yet). RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-05 02:59:27 +00:00

crueber commented

2026-09-05 02:59:28 +00:00

Author

Owner

Fixed by PR #101 incl. review doc fixup (all drop paths log, no phantom arming; 97.0% coverage), merged. Closing.

crueber referenced this issue from a commit

2026-09-05 02:59:28 +00:00

Merge pull request 'Fix #92: no silent emission drops' (#101) from fix/issue-92 into main

crueber referenced this issue

2026-09-05 03:38:10 +00:00