Hold the Approve ask until a review names the head (issue 1803, ADR 0496) #432

Merged
mesh-tools merged 3 commits from fix/1803-hold-ask-without-review into main 2026-10-21 23:48:59 +00:00
Showing only changes of commit 09d5360bce - Show all commits
@@ -519,30 +519,29 @@ func TestAPassingVerdictAsksTheApproveAndItsApproveMergesWithinSecondsOverARealB
b.publish(t, "mesh.mod.gitea.event.pull.updated", announced)
passed := b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/app", 7, head, "pass", "t1t1t1t1t1t1"))
// The ask is written into the gate's bucket asks within seconds of the verdict's event, with no label, comment or mark
// written by anyone, and nothing is asked of a router.
// No review names the head, so nothing is asked (ADR 0496, issue 1803): the delivery waits for a review.
rt.until("the delivery to wait for a review", 5*time.Second, func() bool {
e, err := b.kvs["pulls"].Get(ctx, "novox.app.7")
var k PullKey
return err == nil && json.Unmarshal(e.Value(), &k) == nil && k.Verdict == "pass"
})
time.Sleep(time.Second)
if key := onlyAsk(b); key != "" {
t.Fatalf("a head with no review was asked: %s", key)
}
// The description is edited to add the review record (issue 1079): the forge module's edit event carries it, and the
// ask is written into the gate's bucket asks within seconds, with no label, comment or mark written by anyone, and
// nothing is asked of a router.
edit := time.Now()
b.publish(t, "mesh.mod.gitea.event.forge.pull-request.edited", map[string]any{"owner": "novox", "repo": "app", "number": 7,
"title": announced.Title, "head_sha": head, "base": "main", "body": "Reviewed-by: lead@shanks at " + head})
_ = passed
var key string
rt.until("the ask to be written", 5*time.Second, func() bool { key = onlyAsk(b); return key != "" })
ask := askAt(t, b, key)
t.Logf("the ask was written %s after the verdict's event", time.Since(passed))
if !strings.Contains(ask.Ask.Explanation, "not reviewed at this head") || ask.Ask.ID != sdk.AskID(key) {
t.Fatalf("the first ask does not say the record is missing: %s", ask.Ask.Explanation)
}
// The description is edited to add the review record (issue 1079): the forge module's edit event carries it. The unanswered
// ask is taken away and written again, once, with the record read (the heads are the same, so is the key).
b.publish(t, "mesh.mod.gitea.event.forge.pull-request.edited", map[string]any{"owner": "novox", "repo": "app", "number": 7,
"title": announced.Title, "head_sha": head, "base": "main", "body": "Reviewed-by: lead@shanks at " + head})
rt.until("the ask to be written again with the record", 5*time.Second, func() bool {
a, ok := askNow(b, key) // the key may be absent for an instant while the ask is taken away and written again
return ok && strings.Contains(a.Ask.Explanation, "reviewed by lead on shanks")
})
// The ask is overwritten, never taken away: the key is there at every revision, and the second asking has an id of its own
// (review round 1, G2), which the router would take as new.
if h, err := b.kvs["asks"].History(ctx, key); err != nil || len(h) != 2 || h[1].Operation() != jetstream.KeyValuePut {
t.Fatalf("the ask's history is %d revisions (%v); wanted written and written again", len(h), err)
}
if again := askAt(t, b, key); again.Asking != 2 || again.Ask.ID != sdk.AskingID(key, 2) || again.Ask.ID == ask.Ask.ID {
t.Fatalf("the second asking is %d under %s, the first was %s", again.Asking, again.Ask.ID, ask.Ask.ID)
t.Logf("the ask was written %s after the edit's event", time.Since(edit))
if !strings.Contains(ask.Ask.Explanation, "reviewed by lead on shanks") || ask.Ask.ID != sdk.AskID(key) {
t.Fatalf("the ask does not say the record: %s", ask.Ask.Explanation)
}
if len(rt.asksOf("gitea.gitea_merge_pull_request")) != 0 {
t.Fatalf("merged before the Approve")