fix(telemetry): two delivery failures, two verdicts #749

Merged

coilyco-ops merged 1 commit from fix/two-failures-two-verdicts into main

2026-08-13 22:17:50 +00:00

coilyco-ops commented

2026-08-13 22:16:24 +00:00

Member

Instance 3 of Quail's list on #675, and it is a defect I introduced:

agent.go:1142 joins the reply send and the undelivered notice, and the classifier emits one verdict for both. After #729, a 403 on either reports identically as rest_error 403 50013. An operator cannot tell whether the member got nothing or got the reply and no failure notice.

Correct. #729 made the classification of the join right and left the join itself wrong. Two different member outcomes shared one verdict:

the reply failed, so the member got nothing
the reply failed and the notice failed, so the member got nothing and no apology
the notice alone failed, so the member got the answer and a silent gap

What ships

deliverOrReport sends, and on failure records the notice outcome separately rather than joining it. The turn's verdict is the send. turn.reply.undelivered now carries its own discord_failure, discord_status and discord_code.

What I did not break, and nearly did

My first attempt made reportUndelivered return nil unconditionally. That would have broken TestAnUndeliverableNoticeIsNotRetried, which asserts:

a notice that could not be sent reported success

That test encodes a real contract and I had not read it before changing the function under it. Caught by running the suite, not by reading. The landed shape keeps the contract: reportUndelivered still reports its own failure, and the call site decides not to make it the turn's verdict.

The test I threw away

My first test asserted properties of discordFailureAttrs directly and the mutation passed — rejoining the two errors broke nothing, because nothing exercised the call site. That is the same defect I criticised on #292: a test asserting the sentence it does not test.

Replaced by driving deliverOrReport with a turn that refuses every send, and asserting the returned error is not a join. Rejoining now fails it by name. The gate also caught a t.Skip I had used as a cop-out in the discarded version.

Verification

ward exec gate PASS on all six steps. Mutation: restoring errors.Join(err, a.reportUndelivered(...)) fails TestTheTurnVerdictDescribesTheSend, in a run where the package compiles.

closes #747

Instance 3 of Quail's list on [#675](https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/675), and it is a defect I introduced: > `agent.go:1142` joins the reply send and the undelivered notice, and the classifier emits one verdict for both. After #729, a 403 on either reports identically as `rest_error 403 50013`. An operator cannot tell whether the member got nothing or got the reply and no failure notice. Correct. [#729](https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/729) made the classification of the join *right* and left the join itself wrong. Two different member outcomes shared one verdict: - the reply failed, so the member got **nothing** - the reply failed and the notice failed, so the member got **nothing and no apology** - the notice alone failed, so the member got the answer and a silent gap ## What ships `deliverOrReport` sends, and on failure records the notice outcome **separately** rather than joining it. The turn's verdict is the send. `turn.reply.undelivered` now carries its own `discord_failure`, `discord_status` and `discord_code`. ## What I did not break, and nearly did My first attempt made `reportUndelivered` return `nil` unconditionally. That would have broken `TestAnUndeliverableNoticeIsNotRetried`, which asserts: > a notice that could not be sent reported success That test encodes a real contract and I had not read it before changing the function under it. Caught by running the suite, not by reading. The landed shape keeps the contract: `reportUndelivered` still reports its own failure, and the **call site** decides not to make it the turn's verdict. ## The test I threw away My first test asserted properties of `discordFailureAttrs` directly and **the mutation passed** — rejoining the two errors broke nothing, because nothing exercised the call site. That is the same defect I criticised on [#292](https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/292): a test asserting the sentence it does not test. Replaced by driving `deliverOrReport` with a turn that refuses every send, and asserting the returned error is not a join. Rejoining now fails it by name. The gate also caught a `t.Skip` I had used as a cop-out in the discarded version. ## Verification `ward exec gate` PASS on all six steps. Mutation: restoring `errors.Join(err, a.reportUndelivered(...))` fails `TestTheTurnVerdictDescribesTheSend`, in a run where the package compiles. closes #747

coilyco-ops added 1 commit

2026-08-13 22:16:24 +00:00

fix(telemetry): two delivery failures, two verdicts

ci / image-build (pull_request) Successful in 29s

Details

ci / test (pull_request) Successful in 48s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

151b60e4c1

729 classified the joined error correctly and left the join wrong. A 403 on
the reply and a 403 on the undelivered notice reported identically, so an
operator could not tell whether the member got nothing or got the answer and
no apology.

deliverOrReport records the notice outcome separately instead of joining it.
The turn's verdict is the send, and turn.reply.undelivered carries its own
classification.

reportUndelivered still returns its own failure: my first attempt returned nil
and would have broken TestAnUndeliverableNoticeIsNotRetried, which asserts
that a notice that could not be sent does not report success.

closes #747

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 22:16:24 +00:00

coilyco-ops merged commit ff3fd72ddd into main

2026-08-13 22:17:50 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-13 22:17:51 +00:00

Merge pull request 'fix(telemetry): two delivery failures, two verdicts' (#749) from fix/two-failures-two-verdicts into main