Test the delivery routing table, fallthrough and durability #5

Closed
claude wants to merge 0 commits from overnight/delivery-tests into master
Contributor

Tests only. No behaviour changed.

Why. A nudge that does not arrive is worse than no nudge. DESIGN.md makes three strong claims about delivery and none of them were tested end to end.

Commit 1 — the routing table. All 8 cells of the severity x presence table, twice over: once as a pure function, once end to end through DispatchNudge asserting the exact per-channel send count (every channel that should not fire gets 0).

Notably: sev1-2 away really does drop, sev3 away is ntfy exactly once, and sev4 away goes to telegram and repeats until acked — then stops permanently.

Commit 2 — fallthrough and durability. The voice-not-connected fallthrough works per severity, and sev1-2 correctly does not fall through to an away channel (that would defeat the drop rule).

Durability is tested against a real store on a temp file, not a fake. A pending row with no completion becomes unknown at startup, is not resent, is not deleted, reconciliation is idempotent, and a late completion cannot overwrite the unknown. That all holds.

Four contract claims that are not true today. Each has a test written to the spec and skipped with a file:line, so un-skipping is the finish line.

  • Vikunja #368 — an empty Summary sends the full body off-box. dispatcher.go:356 and both away sinks fall back to Body when Summary is empty, deliberately ("terse full beats no message"). Fine for local voice, wrong for ntfy and telegram, which cross the boundary. DESIGN.md says do not make notifications an exfil surface. The 0.8B produces malformed output routinely and an empty field is a likely malformation, so this is a live path, not a theoretical one. This is the one I would fix first.
  • Vikunja #369 — a panic mid-send leaves a row pending forever. dispatcher.go:168 has no recover, and reconciliation only runs at startup. A crash is handled; a panic the process survives is not, and that is the likelier case.
  • Vikunja #370 — the care-away drop is invisible. A bare continue, no row, no log. Correct behaviour, but indistinguishable from nothing happening.
  • Minimal-body is enforced in each sink rather than at the dispatcher, so it is exfil-by-default for any sink added later.

dispatcher_test.go was unformatted on master and is gofmt'd here — that is the only non-test-content change.

Full tree green, vet and gofmt clean on what I touched.

Tests only. No behaviour changed. **Why.** A nudge that does not arrive is worse than no nudge. DESIGN.md makes three strong claims about delivery and none of them were tested end to end. **Commit 1 — the routing table.** All 8 cells of the severity x presence table, twice over: once as a pure function, once end to end through `DispatchNudge` asserting the exact per-channel send count (every channel that should not fire gets 0). Notably: sev1-2 away really does drop, sev3 away is ntfy exactly once, and sev4 away goes to telegram and repeats until acked — then stops permanently. **Commit 2 — fallthrough and durability.** The voice-not-connected fallthrough works per severity, and sev1-2 correctly does *not* fall through to an away channel (that would defeat the drop rule). Durability is tested against a real store on a temp file, not a fake. A `pending` row with no completion becomes `unknown` at startup, is not resent, is not deleted, reconciliation is idempotent, and a late completion cannot overwrite the `unknown`. That all holds. **Four contract claims that are not true today.** Each has a test written to the spec and skipped with a file:line, so un-skipping is the finish line. - **Vikunja #368 — an empty `Summary` sends the full body off-box.** `dispatcher.go:356` and both away sinks fall back to `Body` when `Summary` is empty, deliberately ("terse full beats no message"). Fine for local voice, wrong for ntfy and telegram, which cross the boundary. DESIGN.md says do not make notifications an exfil surface. The 0.8B produces malformed output routinely and an empty field is a likely malformation, so this is a live path, not a theoretical one. **This is the one I would fix first.** - **Vikunja #369 — a panic mid-send leaves a row pending forever.** `dispatcher.go:168` has no `recover`, and reconciliation only runs at startup. A crash is handled; a panic the process survives is not, and that is the likelier case. - **Vikunja #370 — the care-away drop is invisible.** A bare `continue`, no row, no log. Correct behaviour, but indistinguishable from nothing happening. - Minimal-body is enforced in each sink rather than at the dispatcher, so it is exfil-by-default for any sink added later. `dispatcher_test.go` was unformatted on master and is gofmt'd here — that is the only non-test-content change. Full tree green, vet and gofmt clean on what I touched.
claude added 2 commits 2026-07-31 00:29:46 +02:00
Table-driven tests for all four severity bands crossed with present and away,
both as the pure table and end to end through the dispatcher. Three tests are
written to DESIGN.md and skipped because the code does not keep the claim: the
care-away drop is recorded nowhere, and the minimal body is enforced per-sink
rather than by the dispatcher. Also gofmt'd dispatcher_test.go.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
Fallthrough is checked per severity through the outbox trail, so sev3/sev4
reroute and sev1/sev2 still drop. Durability uses a real store on a temp file:
a crash between Begin and Complete becomes unknown, is not resent, is not
dropped, and a late Complete cannot overwrite it. One skipped test marks a real
gap: a panic mid-send leaves a permanent pending row.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
Owner

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47.

Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47. Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.
kami closed this pull request 2026-07-31 20:21:35 +02:00

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#5