[minor-16] `encode` panics on unmarshalable `Emission.Detail` #98

New issue

Closed

opened 2026-09-05 02:40:27 +00:00 by crueber · 3 comments

crueber commented

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

Owner

[minor-16] `encode` panics on unmarshalable `Emission.Detail`

internal/notify/notify.go:611-619, emit.go:53: the "shapes are fixed, marshal cannot fail" claim doesn't hold for Detail map[string]any (composition-supplied) — a chan/func value panics the mutating request handler instead of failing the emission.

Fix

Defensive encode: return an error (failing that emission with a log) instead of panicking; harden emit.go:53 call site. Regression test (poison Detail → error, no panic, handler survives). Coverage gate holds.

Acceptance criteria

No panic path; poison input yields an error; test green -race.

# [minor-16] `encode` panics on unmarshalable `Emission.Detail` `internal/notify/notify.go:611-619`, `emit.go:53`: the "shapes are fixed, marshal cannot fail" claim doesn't hold for `Detail map[string]any` (composition-supplied) — a chan/func value panics the mutating request handler instead of failing the emission. ## Fix Defensive encode: return an error (failing that emission with a log) instead of panicking; harden `emit.go:53` call site. Regression test (poison Detail → error, no panic, handler survives). Coverage gate holds. ## Acceptance criteria - [ ] No panic path; poison input yields an error; test green `-race`.

crueber referenced this issue from a commit

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

Fix #98: defensive encode for unmarshalable Emission.Detail (06 §4)

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

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

Fix #98: defensive encode for Detail #108

crueber commented

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

Author

Owner

Fix ready for review: PR #108 (branch fix/issue-98). Defensive encode returning an error + emit-entry Detail screen (before seq reservation); poison Detail (chan/func/nested/panicking-marshaler) drops with a log, zero store writes, handler intact; valid Details unaffected. Tests green -race, covergate notify 96.2% (≥95%).

crueber commented

2026-09-05 03:39:20 +00:00

Author

Owner

PR #108 review (fix/issue-98, commit 1472133): APPROVED — ready to merge.

Encode audit (19/19 propagate): webhooks.go (7: 179,188,283,438,491,615,658), watch.go (3: 61,116,145), tasks.go (1: 631), http.go (3: 180,469,517), emit.go (2: 538,663), activity.go (3: 55,87,90). No bare panic-on-error remains. Residual json.Marshal: notify.go:655 (encode, recover-guarded), notify.go:669 (marshalable, recover-guarded), http.go:602 writeJSON (500 fallback, pre-existing), collab.go:122 ({} fallback, pre-existing), test-only fixed string maps (infallible). Old panicking encode is gone.

recover() placement sane: wraps only the json.Marshal call in each of encode/marshalable — converts panicking MarshalJSON into error/false, masks nothing else.

Entry-screen ordering verified: emit.go:174 screen precedes reserveSeq (212); test asserts zero store writes on poison (no tray, no activity event, collab_state absent → no seq consumed) and post-poison valid emission lands at seq 1.

Valid Details byte-identical: TestEmitValidDetailFansOut (nil/empty/check-shaped/nested round-trip); full suite green with only mechanical encode→mustEncode test edits.

Log-drop follows #92: 'notify: emission dropped: unmarshalable detail' + repo/num/class/actor attrs, matching existing reserve/append drop shape.

Boundary sound: screen at emit entry (Emission is the seam notify never crosses upward); defense-in-depth appendActivity returns error for direct callers; stored-event replays never re-encode Detail.

mustEncode (fakes_test.go:152) uses production encode + Fatalf — fails fast, weakens nothing.

Verify (scratch /tmp/walhub-98 @1472133): gofmt clean, go vet clean, go test -race ./internal/notify/... ok (1.7s), coverage 96.2% (≥95%). No new imports (encoding/json+fmt already imported). Doc entry (06 Decisions) accurate.

No fixes pushed — nothing to fix. MERGE RECOMMENDATION: ready to merge.

PR #108 review (fix/issue-98, commit 1472133): APPROVED — ready to merge. Encode audit (19/19 propagate): webhooks.go (7: 179,188,283,438,491,615,658), watch.go (3: 61,116,145), tasks.go (1: 631), http.go (3: 180,469,517), emit.go (2: 538,663), activity.go (3: 55,87,90). No bare panic-on-error remains. Residual json.Marshal: notify.go:655 (encode, recover-guarded), notify.go:669 (marshalable, recover-guarded), http.go:602 writeJSON (500 fallback, pre-existing), collab.go:122 ({} fallback, pre-existing), test-only fixed string maps (infallible). Old panicking encode is gone. recover() placement sane: wraps only the json.Marshal call in each of encode/marshalable — converts panicking MarshalJSON into error/false, masks nothing else. Entry-screen ordering verified: emit.go:174 screen precedes reserveSeq (212); test asserts zero store writes on poison (no tray, no activity event, collab_state absent → no seq consumed) and post-poison valid emission lands at seq 1. Valid Details byte-identical: TestEmitValidDetailFansOut (nil/empty/check-shaped/nested round-trip); full suite green with only mechanical encode→mustEncode test edits. Log-drop follows #92: 'notify: emission dropped: unmarshalable detail' + repo/num/class/actor attrs, matching existing reserve/append drop shape. Boundary sound: screen at emit entry (Emission is the seam notify never crosses upward); defense-in-depth appendActivity returns error for direct callers; stored-event replays never re-encode Detail. mustEncode (fakes_test.go:152) uses production encode + Fatalf — fails fast, weakens nothing. Verify (scratch /tmp/walhub-98 @1472133): gofmt clean, go vet clean, go test -race ./internal/notify/... ok (1.7s), coverage 96.2% (≥95%). No new imports (encoding/json+fmt already imported). Doc entry (06 Decisions) accurate. No fixes pushed — nothing to fix. MERGE RECOMMENDATION: ready to merge.

crueber closed this issue

2026-09-05 03:39:34 +00:00

crueber commented

2026-09-05 03:39:35 +00:00

Author

Owner

Fixed by PR #108 (review: 19/19 call sites propagate, entry-screen burns no gap, valid Details byte-identical; 96.2% coverage), merged. Closing.

crueber referenced this issue from a commit

2026-09-05 03:39:35 +00:00

Merge pull request 'Fix #98: defensive encode for Detail' (#108) from fix/issue-98 into main

crueber added the

webhooks