Fix #98: defensive encode for Detail #108

Merged
crueber merged 1 commit from fix/issue-98 into main 2026-09-05 03:39:33 +00:00
Owner

Fixes #98: encode panicked on unmarshalable Emission.Detail (composition-supplied map[string]any — chan/func values panicked the synchronous mutating handler).

  • encode(v) ([]byte, error) (was []byte + panic); every production call site propagates (CAS closures return it, emission paths log-drop per the #92 convention). A recover closes even a panicking-MarshalJSON path, so no panic path remains.
  • emit screens Detail marshalability at entry, BEFORE the seq reservation: poisoned emission drops with a notify: emission dropped: unmarshalable detail Warn, zero store writes (no gap, no partial tray). Screen lives at the emit boundary, not per composer (notify/feature packages never import each other) — decision appended to docs/features/06_notifications.md Decisions.
  • Regression tests (all green -race, table-driven): TestEmitPoisonDetailDropsLogged (chan/func/nested/panicking-marshaler → logged drop, no seq consumed, handler intact via follow-up valid emission), TestEmitValidDetailFansOut (nil/empty/check-shaped/nested round-trip byte-identically), TestEncodeRejectsUnmarshalable, TestAppendActivityPoisonReturnsError. Test fixtures use new mustEncode helper.
  • Verification: gofmt/go vet clean; go test -race ./internal/notify/ green; repo covergate: notify 96.2% (≥95%, baseline 97.3%); all notify importers build+vet clean. Note: go build ./cmd/walhub fails identically on pristine origin/main (missing generated web/dist in a fresh worktree) — pre-existing, unrelated.

Do NOT merge (per instructions).

Fixes #98: `encode` panicked on unmarshalable `Emission.Detail` (composition-supplied `map[string]any` — chan/func values panicked the synchronous mutating handler). - `encode(v) ([]byte, error)` (was `[]byte` + panic); every production call site propagates (CAS closures return it, emission paths log-drop per the #92 convention). A `recover` closes even a panicking-`MarshalJSON` path, so no panic path remains. - `emit` screens `Detail` marshalability at entry, BEFORE the seq reservation: poisoned emission drops with a `notify: emission dropped: unmarshalable detail` Warn, zero store writes (no gap, no partial tray). Screen lives at the `emit` boundary, not per composer (notify/feature packages never import each other) — decision appended to `docs/features/06_notifications.md` Decisions. - Regression tests (all green `-race`, table-driven): `TestEmitPoisonDetailDropsLogged` (chan/func/nested/panicking-marshaler → logged drop, no seq consumed, handler intact via follow-up valid emission), `TestEmitValidDetailFansOut` (nil/empty/check-shaped/nested round-trip byte-identically), `TestEncodeRejectsUnmarshalable`, `TestAppendActivityPoisonReturnsError`. Test fixtures use new `mustEncode` helper. - Verification: `gofmt`/`go vet` clean; `go test -race ./internal/notify/` green; repo covergate: notify 96.2% (≥95%, baseline 97.3%); all notify importers build+vet clean. Note: `go build ./cmd/walhub` fails identically on pristine origin/main (missing generated `web/dist` in a fresh worktree) — pre-existing, unrelated. Do NOT merge (per instructions).
encode returns an error instead of panicking (recover closes even a
panicking-MarshalJSON path); all call sites propagate (CAS closures
return it, emission paths log-drop it). emit screens Detail
marshalability at entry, before the seq reservation, so a poisoned
emission drops with a log and leaves no gap/partial tray. Screen lives
at the emit boundary, not per composer (notify/feature packages never
import each other).

Regression: TestEmitPoisonDetailDropsLogged, TestEmitValidDetailFansOut,
TestEncodeRejectsUnmarshalable, TestAppendActivityPoisonReturnsError;
test fixtures move to mustEncode. Decision appended to 06 Decisions.
Sign in to join this conversation.
No description provided.