From add6f143ca236d4e965cb114d3f1798492640d0c Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 29 Oct 2026 01:59:03 +0100 Subject: [PATCH] mesh-delivery-gate: an answer stands only for the ask it answers, and a merge never handed says why On 2026-10-29 the group feat/774-vault-mints-l2 was approved and never handed: hq#495 kept a Decline of an earlier ask beside the Approve of the next, readyAsks skipped it and handTurn returned nil for every member, with no log line. The Approve and a new ask now clear an earlier Decline, a run's answer names its ask (answered_ask), an approved merge that waits for nothing and is not handed logs why once and is a stalled line, and delivery_turns lists no unwritten turn while the check gates no merge. --- .../cmd/mesh-delivery-gate/approval.go | 9 +-- .../cmd/mesh-delivery-gate/approval_test.go | 48 +++++++++++++ .../cmd/mesh-delivery-gate/events.go | 2 +- .../cmd/mesh-delivery-gate/group_merge.go | 4 +- .../cmd/mesh-delivery-gate/holder.go | 2 +- .../cmd/mesh-delivery-gate/merges.go | 67 +++++++++++++++++-- .../cmd/mesh-delivery-gate/ready.go | 43 ++++++++++-- .../cmd/mesh-delivery-gate/turns.go | 10 ++- 8 files changed, 165 insertions(+), 20 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index 2a7ba773..bfbcfae4 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -330,7 +330,7 @@ func (h *Holder) endStaleAsks() { h.endStaleSettles() for _, d := range h.deliveries { r := d.Ready - if r == nil || r.Merged != nil || d.MergedAs != "" || r.Declined != nil || (r.AskID == "" && r.Approved == nil) || len(r.Heads) == 0 { + if r == nil || r.Merged != nil || d.MergedAs != "" || r.declined() || (r.AskID == "" && r.Approved == nil) || len(r.Heads) == 0 { continue } switch { @@ -418,6 +418,7 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { h.runOf(m, now) m.Ready.AskID, m.Ready.Ask, m.Ready.Heads, m.Ready.AskedAt, m.Ready.Asks = id, &ask, heads, now, max(m.Ready.Asks, r.Asks) m.Ready.AskedKey, m.Ready.Asking, m.Ready.AskCheck = id, asking, check.Key + m.Ready.Declined, m.Ready.AnsweredAsk = nil, "" // a new ask: an answer to an earlier one does not stand on it h.keep(m) taken = append(taken, m) reads = append(reads, readOf(m, m.Commit)) @@ -608,7 +609,7 @@ func (h *Holder) pendingAsks() []string { seen := map[string]bool{} var out []string for _, d := range h.deliveries { - if r := d.Ready; r != nil && r.AskID != "" && r.Approved == nil && r.Declined == nil && !seen[r.AskID] { + if r := d.Ready; r != nil && r.AskID != "" && r.Approved == nil && !r.declined() && !seen[r.AskID] { seen[r.AskID] = true out = append(out, r.AskID) } @@ -705,10 +706,10 @@ func (h *Holder) answered(key string, ans sdk.Answer) []func() { h.mergeAgainNow(d) // the operator chose: the merge is asked at once switch ans.Option { case OptionApprove: - r.Approved = &now + r.Approved, r.Declined, r.AnsweredAsk = &now, nil, key h.Logf("[mesh-delivery] %s: the operator approved its merge into main (%s)", d.ID, r.Warrant) case OptionDecline: - r.Declined = &now + r.Declined, r.AnsweredAsk = &now, key h.Logf("[mesh-delivery] %s: the operator declined its merge into main (%s)", d.ID, r.Warrant) said = append(said, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** The merge of this pull request into main at %s was "+ "declined: %s. A new head asks the operator again.", d.Commit, r.Warrant))) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go index fde6b6e7..3057f3d8 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval_test.go @@ -405,3 +405,51 @@ func TestAnAnswerToAnEarlierAskingIsNotTakenAndTheMergeNamesTheAnsweredAsking(t t.Fatalf("the merge is announced %v; wanted the ask id of asking 2, %s", say, sdk.AskingID(key, 2)) } } + +// 2026-10-29 (hq#495): a group declined under one ask, a member pushed, and the group asked and approved again at the new +// heads: the member whose head did not move kept the earlier Decline beside the new Approve, and nothing was handed, with +// no word. An answer stands only for the ask it answers; whichever member is pushed, both merge on the new Approve. +func TestADeclineOfAnEarlierAskDoesNotHoldTheApproveOfTheNext(t *testing.T) { + const ctlHead, catHead, pushed = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2", "e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3" + for _, push := range []struct { + repo string + number int + trunk string + }{{"novox/mesh-catalog", 2, "t3t3t3t3t3t3"}, {"novox/mesh-controller", 1, "t1t1t1t1t1t1"}} { + t.Run(push.repo, func(t *testing.T) { + w := newWorld(t) + w.op.auto = false + heads := map[string]string{"novox/mesh-controller": ctlHead, "novox/mesh-catalog": catHead} + paths := map[string]string{"novox/mesh-controller": "internal/a.go", "novox/mesh-catalog": "modules/app/module.json"} + numbers := map[string]int{"novox/mesh-controller": 1, "novox/mesh-catalog": 2} + trunks := map[string]string{"novox/mesh-controller": "t1t1t1t1t1t1", "novox/mesh-catalog": "t3t3t3t3t3t3"} + round := func(check string) { + var open []OpenPull + for _, repo := range []string{"novox/mesh-controller", "novox/mesh-catalog"} { + w.h.PullUpdated(pr(repo, numbers[repo], heads[repo], "feat/x", paths[repo])) + w.h.Checked(judgedAt(repo, numbers[repo], heads[repo], "pass", trunks[repo])) + open = append(open, openPull(repo, numbers[repo], heads[repo])) + } + w.forge.open = open + w.h.SeeOpenPulls() + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/x", ID: check, Verdict: "pass", Summary: "both compose"}) + w.settleAll() + } + round("check-1") + w.op.answer(w.op.open()[0], OptionDecline) + w.settleAll() + heads[push.repo] = pushed + round("check-2") + open := w.op.open() + if len(open) != 1 { + t.Fatalf("the group at its new heads is not asked once: %v", open) + } + w.op.answer(open[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 2 { + t.Fatalf("approved at the new heads, yet merged %v; stalled %+v", w.forge.merges, w.h.Stalled()) + } + }) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go index b0fc97af..be95a0c7 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go @@ -96,7 +96,7 @@ func (h *Holder) withdrawForStatus(e ForgeStatusEvent) { done := map[string]bool{} for _, d := range h.deliveries { r := d.Ready - if !strings.EqualFold(d.Repository, repository) || r == nil || r.AskID == "" || r.Approved != nil || r.Declined != nil || done[r.AskID] { + if !strings.EqualFold(d.Repository, repository) || r == nil || r.AskID == "" || r.Approved != nil || r.declined() || done[r.AskID] { continue } head := d.Commit diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go index 8b2fd6a2..b6e1897c 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go @@ -208,7 +208,7 @@ func (h *Holder) passes() []pass { for _, id := range ids { d := h.deliveries[id] r := d.Ready - if r == nil || r.Approved == nil || r.Declined != nil || r.AskID == "" || len(r.Heads) == 0 || seen[r.AskID] || + if r == nil || r.Approved == nil || r.declined() || r.AskID == "" || len(r.Heads) == 0 || seen[r.AskID] || d.Base != MergeBranch || h.allMerged(r.Heads) { continue } @@ -335,7 +335,7 @@ func (h *Holder) MergeAgain(id, why string) (string, error) { return "", fmt.Errorf("say why: the merge asked again is the operator's word, kept in the log") } c := h.covering(d) - if c == nil || c.Approved == nil || c.Declined != nil || c.Heads[d.ID] != d.Commit { + if c == nil || c.Approved == nil || c.declined() || c.Heads[d.ID] != d.Commit { return "", fmt.Errorf("%s is not approved at its head %s: nothing to ask again (a new head is asked on its own)", id, shortOf(d.Commit, 12)) } if d.MergedAs != "" || (d.Ready != nil && d.Ready.Merged != nil) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 79af7cda..8ba98d5c 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -524,7 +524,7 @@ func (h *Holder) PullEdited(p PullEvent) { if after, _ := reviewWords(d); after == before { return } - if r := h.covering(d); r != nil && r.AskID != "" && r.Approved == nil && r.Declined == nil { + if r := h.covering(d); r != nil && r.AskID != "" && r.Approved == nil && !r.declined() { h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, d.ID+"'s review record changed: "+shortOf(d.ReviewedAt, 12))) } } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go index ec1b6cec..d2634300 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -149,11 +149,10 @@ func (h *Holder) handTurn(g *Group, d *Delivery, now time.Time) func() { return nil } for _, m := range order { - mc := h.covering(m) - if m.Ready == nil || m.Ready.Declined != nil || mc == nil || mc.Approved == nil || mc.Heads[m.ID] != m.Commit || h.readyWaits(m) != "" { - return nil // the others are not approved at their heads yet: nothing is merged before they are + if h.notHandable(m) != "" { + return nil // the others are not approved at their heads yet: nothing is merged before they are (said by notHanded) } - reads = append(reads, readOf(m, mc.Heads[m.ID])) + reads = append(reads, readOf(m, h.covering(m).Heads[m.ID])) } } if c == nil || c.Approved == nil || c.Heads[d.ID] == "" { @@ -169,6 +168,66 @@ func (h *Holder) handTurn(g *Group, d *Delivery, now time.Time) func() { return func() { h.hand(gid, name, key, target, reads, approved) } } +// notHandable is why a group member cannot be handed in its group's pass, empty when it can: handTurn's own test, in words. +// Under the lock. +func (h *Holder) notHandable(m *Delivery) string { + mc := h.covering(m) + switch { + case m.Ready == nil: + return m.ID + " has no ready run (" + string(m.State) + ")" + case m.Ready.declined(): + return m.ID + " stands declined since " + m.Ready.Declined.UTC().Format(time.RFC3339) + case mc == nil || mc.Approved == nil: + return m.ID + " is not approved at its head " + shortOf(m.Commit, 12) + case mc.Heads[m.ID] != m.Commit: + return m.ID + "'s head " + shortOf(m.Commit, 12) + " is not the head approved" + } + if w := h.readyWaits(m); w != "" { + return m.ID + " waits: " + w + } + return "" +} + +// notHanded is why the approved merge of d, which waits for nothing, is not handed: its group's pass cannot start, and +// says which member holds it and why. Empty when nothing holds it, or when the pass is under way or its refusal is said +// already. Under the lock. +func (h *Holder) notHanded(g *Group, d *Delivery) string { + if g == nil { + return "" + } + order := h.unmergedLive(g) + if len(order) == 0 { + return "" + } + if first := order[0].Ready; first != nil && (first.merging || !first.LastTry.IsZero() || first.RefusedSince != nil) { + return "" // the pass is under way, or its refusal is said on its own line + } + for _, m := range order { + if why := h.notHandable(m); why != "" { + return fmt.Sprintf("its group %s is not handed to the forge bridge: %s", g.ID, why) + } + } + return "" +} + +// noteNotHanded keeps on d's run why its approved merge is not handed, said once in the log when it changes, and as a +// stalled line meanwhile (readyLines). Under the lock. +func (h *Holder) noteNotHanded(d *Delivery, why string, now time.Time) { + r := d.Ready + if r == nil || r.NotHanded == why { + return + } + r.NotHanded = why + if why == "" { + r.NotHandedSince = nil + } else { + at := now + r.NotHandedSince = &at + h.Logf("[mesh-delivery] %s is approved, waits for nothing, and is NOT handed to merge: %s", d.ID, why) + } + h.keep(d) +} + // hand reads a group's required statuses and, when all are a success, writes the target as the member being merged. func (h *Holder) hand(gid, name, key, target string, reads []memberRead, approved map[string]string) { if gid != "" && !h.groupMergeable(gid, target, reads) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index 94ffc5b8..3f16589d 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -120,11 +120,13 @@ type ReadyRun struct { // in new words, so it never keeps a claim the check no longer holds. AskCheck string `json:"ask_check,omitempty"` // Approved and Declined are when the operator chose, Warrant what the router's warrant says of who and how; Refused - // why an answer was not taken (it did not hold to the ask). - Approved *time.Time `json:"approved,omitempty"` - Declined *time.Time `json:"declined,omitempty"` - Warrant string `json:"warrant,omitempty"` - Refused string `json:"refused,omitempty"` + // why an answer was not taken (it did not hold to the ask). AnsweredAsk is the ask the answer answers: an answer stands + // only for its own ask, so a Decline of an earlier ask never stands beside the Approve of a later one (declined). + Approved *time.Time `json:"approved,omitempty"` + Declined *time.Time `json:"declined,omitempty"` + AnsweredAsk string `json:"answered_ask,omitempty"` + Warrant string `json:"warrant,omitempty"` + Refused string `json:"refused,omitempty"` // One check per delivery (novox/hq ADR 0548 §2, §4, turns.go): HeldSince is when the approved merge began to wait for the // check of other heads HeldFor names (its verdict key), running on a repository it goes into; HandNow the operator's word @@ -137,6 +139,25 @@ type ReadyRun struct { // (writes.go, issue 1945); CannotWriteSince since when. Said at once as a condition. CannotWrite string `json:"cannot_write,omitempty"` CannotWriteSince *time.Time `json:"cannot_write_since,omitempty"` + + // NotHanded is why an approved merge that waits for nothing is not handed to the forge bridge (a member of its group that + // cannot be handed, and why), NotHandedSince since when: said once in the log and as a stalled line, never silent. + NotHanded string `json:"not_handed,omitempty"` + NotHandedSince *time.Time `json:"not_handed_since,omitempty"` +} + +// declined is whether the operator's Decline stands for this run: it answered the ask the run holds now, or the run holds +// no ask any more. A Decline of an earlier ask, the run asked again since at other heads (a sibling's new head), does not +// stand (2026-10-29: hq#495 declined under one ask and approved under the next was never handed). A run kept before answers +// named their ask stands declined unless an Approve came after the Decline. +func (r *ReadyRun) declined() bool { + if r == nil || r.Declined == nil { + return false + } + if r.AnsweredAsk != "" { + return r.AskID == "" || r.AskID == r.AnsweredAsk + } + return r.Approved == nil || r.Declined.After(*r.Approved) } // MergeBranch is the one branch this owner merges into (novox/hq ADR 0398 §4): a merge into any other is its author's. @@ -197,7 +218,7 @@ func (h *Holder) readyAsks(now time.Time) []func() { asks = append(asks, a) } r := d.Ready - if r == nil || r.Merged != nil || r.Declined != nil { + if r == nil || r.Merged != nil || r.declined() { continue } if d.State == Rejected { @@ -276,9 +297,11 @@ func (h *Holder) readyAsks(now time.Time) []func() { asks = append(asks, approvalAsk) } if waits != "" || r.merging { + h.noteNotHanded(d, "", now) // what it waits for is said by its waits, or it is handed continue } if !r.LastTry.IsZero() { + h.noteNotHanded(d, "", now) continue // refused: asked again at the next event that decides whether it may merge (mergeAgainNow), with no timer } // A group is merged one member at a time, handed to the forge bridge, and the pass stops at the first refusal (ADR @@ -288,7 +311,10 @@ func (h *Holder) readyAsks(now time.Time) []func() { g = nil } if a := h.handTurn(g, d, now); a != nil { + h.noteNotHanded(d, "", now) asks = append(asks, a) + } else { + h.noteNotHanded(d, h.notHanded(g, d), now) // a merge never handed says why (2026-10-29: silent for an hour) } } asks = append(asks, h.turnWrites()...) // the merge turns, after the holds of this look (ADR 0548 §3) @@ -575,6 +601,11 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { Bound: "0s", H2: "none: a new head, which asks the operator again — the operator's", Says: fmt.Sprintf("%s#%d at %s is not merged: the operator's answer %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Refused)}) + case r.NotHanded != "" && r.NotHandedSince != nil && r.Approved != nil: + out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, + For: now.Sub(*r.NotHandedSince).Round(time.Second).String(), Bound: "0s", + H2: "none: the member named is approved at its head and ready, or its group is asked anew by a new head — the operator's", + Says: fmt.Sprintf("%s#%d at %s is approved and %s", d.Repository, d.Number, shortOf(r.Head, 12), r.NotHanded)}) case h.answering(d, now): // The trunk moved and its check was asked again: the gate is answering it, so it is no stall yet (ADR 0505 // §4). `show` lists the refusal meanwhile. diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/turns.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/turns.go index 72b3ab85..48a68aac 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/turns.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/turns.go @@ -110,7 +110,7 @@ func (h *Holder) approvedMerges() []approvedMerge { for _, id := range ids { d := h.deliveries[id] r := d.Ready - if r == nil || r.Approved == nil || r.Declined != nil || r.AskID == "" || len(r.Heads) == 0 || seen[r.AskID] || + if r == nil || r.Approved == nil || r.declined() || r.AskID == "" || len(r.Heads) == 0 || seen[r.AskID] || d.State.Final() || d.State == Rejected || d.Base != MergeBranch || r.RefusedSince != nil || h.allMerged(r.Heads) { continue } @@ -382,7 +382,10 @@ type TurnLine struct { func (h *Holder) TurnsRead() map[string]any { h.mu.Lock() defer h.mu.Unlock() - want := h.wantedTurns() + want := map[string]sdk.Turn{} + if h.checkGates() { + want = h.wantedTurns() // as turnWrites: while the check gates no merge, no turn is written by design + } repos := make([]string, 0, len(want)) for repo := range want { repos = append(repos, repo) @@ -404,6 +407,9 @@ func (h *Holder) TurnsRead() map[string]any { lines = append(lines, l) } out := map[string]any{"turns": lines, "check-gates-merge": h.checkGates(), "bound": turnBound.String()} + if !h.checkGates() { + out["why-none"] = "the check gates no merge (check-gates-merge false): no merge waits for a check, so no turn is written" + } if h.Turns != nil { out["written"] = h.turnsLoaded var unwritten []string