mesh-mail: a send still approved (the tools' own publish-then-record race) is redelivered later, never refused or told (review round 1 finding 2; issue 1196)

This commit is contained in:
jochen
2026-10-19 10:31:39 +02:00
parent f140099a75
commit 0a7669b48f
3 changed files with 56 additions and 0 deletions
@@ -192,6 +192,42 @@ func TestClaimQueuedSendOnALostRaceSendsNothingAndLeavesTheMessageToTheBus(t *te
}
}
// 1196/AC5: given a send still approved (publishQueued published to mail-sends before it recorded queued,
// and a message was delivered inside that window), when taken, then it is delivered again later and
// nothing is recorded or told (novox/hq review round 1 finding 2).
func TestClaimQueuedSendOnASendStillApprovedAsksRedeliveryAndRecordsNothing(t *testing.T) {
sends := newMemSends()
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 = "approved" // not yet advanced to "queued" by the tools' own second write
if _, err := sends.Update(req.ID, current, rev); err != nil {
t.Fatalf("leaving it approved: %v", err)
}
req = current
_, d, _, outcome, err := claimQueuedSend(sends, newSendRouter().Record, req.ID, judgedAt.Add(3*time.Minute))
if err != nil {
t.Fatalf("claiming: %v", err)
}
if outcome != claimNotYetQueued {
t.Fatalf("outcome: %v, want claimNotYetQueued", outcome)
}
if d.Outcome != "approved" {
t.Fatalf("decision: %+v, want outcome still approved (unchanged)", d)
}
kept, _, ok, err := getSend(sends, req.ID)
if err != nil || !ok || kept.Decided == nil || kept.Decided.Outcome != "approved" || kept.Decided.Tell != "" {
t.Fatalf("something was recorded or told: %v %v %+v", err, ok, kept.Decided)
}
ids, err := tellPendingIDs(sends)
if err != nil || len(ids) != 0 {
t.Fatalf("a tell was queued despite nothing being told: %v %v", err, 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).
+13
View File
@@ -737,8 +737,17 @@ 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
// claimNotYetQueued: publishQueued (above) publishes to mail-sends before it records queued, so a
// message delivered in that narrow window finds the record still "approved" — the tools really did
// approve it and are about to record queued, so this is not a refusal: nothing is recorded and nobody
// is told; the caller asks the bus to redeliver shortly (novox/hq review round 1 finding 2).
claimNotYetQueued
)
// NotYetQueuedDelay is how long the sender asks the bus to wait before redelivering a message whose record
// is still "approved" (claimNotYetQueued): long enough for the tools' own second write (queued) to land.
const NotYetQueuedDelay = 5 * time.Second
// 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
@@ -776,6 +785,10 @@ func claimQueuedSend(store SendStore, record RouterRecord, id string, now time.T
_ = removeSenderPending(store, req.ID)
_ = addTellPending(store, req.ID)
return req, d, 0, claimNotProven, err
case req.Decided.Outcome == "approved":
// Not yet visible as queued (publishQueued publishes before it records queued): the tools did
// approve it, so this is not a refusal (novox/hq review round 1 finding 2).
return req, *req.Decided, 0, claimNotYetQueued, 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))
}
@@ -225,6 +225,13 @@ func (h *senderHandler) handle(msg *process.Message) {
switch outcome {
case claimLostRace:
return // another delivery of the same message already owns it.
case claimNotYetQueued:
// The tools' own second write (queued) has not landed yet: not a refusal (novox/hq review round 1
// finding 2). Nothing recorded, nobody told; ask the bus to redeliver shortly.
if err := msg.NakWithDelay(NotYetQueuedDelay); err != nil {
h.logf("[mesh-mail] send: asking %s redelivered: %v", q.Send, err)
}
return
case claimRefused, claimNotProven:
if err := msg.Term(); err != nil {
h.logf("[mesh-mail] send: terminating %s: %v", q.Send, err)