diff --git a/modules/mesh-mail/cmd/mesh-mail/claim_test.go b/modules/mesh-mail/cmd/mesh-mail/claim_test.go index cb3ee606..8b2e9535 100644 --- a/modules/mesh-mail/cmd/mesh-mail/claim_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/claim_test.go @@ -378,6 +378,61 @@ func TestClaimQueuedSendRefuseEmitsNothingWhenTheWriteDidNotLand(t *testing.T) { } } +// 1196/AC5 (operator-approved change to clause 10): given a send already sent (and one already failed, +// refused, or not proven), when its message is delivered again, then the record is unchanged, nothing is +// emitted or told, and the message is terminated (claimAlreadyFinished; sender.go's handler maps it to +// Term alone, tested here the same way every other claimOutcome is in this file — by the outcome value). +func TestClaimQueuedSendOnARedeliveryOfAnAlreadyFinishedSendTerminatesWithoutTouchingAnything(t *testing.T) { + for name, outcome := range map[string]string{ + "sent": "sent", + "failed": "failed: 550 no such mailbox", + "refused": "refused: the warrant does not bind the request", + "not proven": "not proven", + } { + t.Run(name, func(t *testing.T) { + sends, router := newMemSends(), newSendRouter() + req := rawQueuedSend(t, sends) + current, rev, ok, err := getSend(sends, req.ID) + if err != nil || !ok { + t.Fatalf("reading it back: %v %v", err, ok) + } + current.Decided.Outcome = outcome + current.Decided.Warrant = "the operator, via telegram (user id verified), chose Send" + current.Decided.Attempt = 1 + if _, err := sends.Update(req.ID, current, rev); err != nil { + t.Fatalf("leaving it %s: %v", outcome, err) + } + before := *current.Decided + + emit := &memEmit{} + _, d, _, got, err := claimQueuedSend(sends, router.Record, emit.emit, req.ID, judgedAt.Add(3*time.Minute)) + if err != nil { + t.Fatalf("claiming: %v", err) + } + if got != claimAlreadyFinished { + t.Fatalf("outcome: %v, want claimAlreadyFinished", got) + } + if d.Outcome != before.Outcome || d.Warrant != before.Warrant || d.Attempt != before.Attempt || !d.At.Equal(before.At) { + t.Fatalf("decision returned: %+v, want unchanged %+v", d, before) + } + if len(emit.calls) != 0 { + t.Fatalf("emitted something for an already-finished send: %+v", emit.calls) + } + kept, _, ok, kerr := getSend(sends, req.ID) + if kerr != nil || !ok || kept.Decided == nil { + t.Fatalf("the send could not be read back: %v %v", kerr, ok) + } + if kept.Decided.Outcome != before.Outcome || kept.Decided.Warrant != before.Warrant || kept.Decided.Attempt != before.Attempt { + t.Fatalf("the record changed: %+v, want unchanged %+v", kept.Decided, before) + } + ids, terr := tellPendingIDs(sends) + if terr != nil || len(ids) != 0 { + t.Fatalf("a tell was queued for an already-finished send: %v %v", terr, ids) + } + }) + } +} + // raceOnFirstUpdate performs one competing write the first time Update is called — simulating another // delivery's claim landing first — so the caller's own compare-and-set loses with stdio.ErrChanged // (novox/hq issue 1196, AC4). diff --git a/modules/mesh-mail/cmd/mesh-mail/send.go b/modules/mesh-mail/cmd/mesh-mail/send.go index 42319036..66d130d3 100644 --- a/modules/mesh-mail/cmd/mesh-mail/send.go +++ b/modules/mesh-mail/cmd/mesh-mail/send.go @@ -726,8 +726,22 @@ const ( // message, racing this one); nothing is recorded here and nothing sent; the caller leaves the message to // the bus entirely — no Ack, no Nak, no Term. claimLostRace + // claimAlreadyFinished: a redelivery of a message whose send already finished (sent, failed, refused + // or not proven) — the sender's own ack was lost, or two deliveries raced and the other one already + // finished it. Nothing is recorded, emitted or told again; the caller only terminates the message + // (novox/hq ADR 0436 clause 10, operator-approved change). + claimAlreadyFinished ) +// isFinishedOutcome says whether a decided send's outcome is already over, in the sense clause 10 means it: +// sent, failed (any reason), refused (any reason, including the sender's own clause-9 refusal), or not +// proven. A redelivery that finds one of these is a repeat, never a new refusal (novox/hq review, the +// operator-approved change to clause 10). +func isFinishedOutcome(outcome string) bool { + return outcome == "sent" || outcome == "not proven" || + strings.HasPrefix(outcome, "failed: ") || strings.HasPrefix(outcome, "refused: ") +} + // claimQueuedSend verifies the operator's warrant for id (mesh-mail's own ask) and claims the send by // compare-and-set from queued or retrying to sending, with the attempt and time, before any connection // (novox/hq ADR 0436 clauses 9, 10). It is never trusted from the queue's message alone: the ask is read @@ -788,6 +802,14 @@ func claimQueuedSend(store SendStore, record RouterRecord, emit Emit, id string, indexErr = errOr(indexErr, fmt.Errorf("dropping %s from the sender-pending index: %w", req.ID, rmErr)) } return req, d, 0, claimNotProven, indexErr + case isFinishedOutcome(req.Decided.Outcome): + // A redelivery of a message whose send already finished — sent, failed, refused by the sender's + // own claim, or not proven: the sender's own ack was lost (a bus reconnect) and the work-queue gave + // it up after AckWait, or two deliveries raced and the other one already finished it. The operator + // was already told (or sent) once; telling, emitting or recording anything again here would be a + // second word about a send that is already over. Just a repeat: terminated, nothing else touched + // (novox/hq ADR 0436 clause 10, operator-approved change). + return req, *req.Decided, 0, claimAlreadyFinished, nil case req.Decided.Outcome != "queued" && req.Decided.Outcome != "retrying": return refuse(fmt.Sprintf("the send is %q, neither queued nor retrying", req.Decided.Outcome)) } diff --git a/modules/mesh-mail/cmd/mesh-mail/sender.go b/modules/mesh-mail/cmd/mesh-mail/sender.go index a424be1c..4217270e 100644 --- a/modules/mesh-mail/cmd/mesh-mail/sender.go +++ b/modules/mesh-mail/cmd/mesh-mail/sender.go @@ -233,7 +233,10 @@ func (h *senderHandler) handle(msg *process.Message) { switch outcome { case claimLostRace: return // another delivery of the same message already owns it. - case claimRefused, claimNotProven: + case claimRefused, claimNotProven, claimAlreadyFinished: + // claimAlreadyFinished: a repeat of a message whose send already finished (the operator was + // already told, or not, exactly once) — terminated, and nothing else touched (novox/hq review, + // operator-approved change to clause 10). if err := msg.Term(); err != nil { h.logf("[mesh-mail] send: terminating %s: %v", q.Send, err) }