One verdict describes two different delivery outcomes #747

New issue

Closed

opened 2026-08-13 22:16:00 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-13 22:16:00 +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 the

autonomy

headless

label

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

coilyco-ops referenced this issue from a commit

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

fix(telemetry): two delivery failures, two verdicts

coilyco-ops referenced this issue from a pull request that will close it,

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

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

coilyco-ops closed this issue