mesh-delivery-gate: ask no Approve of a merge the forge account cannot carry out, and say why (issue 1945)

car-hunter #2 was approved and its commit status then refused 403. Before the ask the
gate reads merge.writes for each repository it covers; where the merging account cannot
write, nothing is asked, the delivery waits on 'the forge account cannot write <repo>:
gitea_grant_write' and a condition is raised. gitea.repo.writable reads it again and
asks an approved, refused merge of it again.

modules/mesh-delivery-gate/README.md

@@ -36,6 +36,14 @@ names every refusal and rejection at once, and the pull request is told at once;
   `needs: new-head` and names its author: the branch, and the session a `Pushed-by: <who@node>` line in the pull
   request's description names.
 
+## A merge the forge account cannot carry out is not asked (issue 1945)
+
+Before the operator's Approve is asked, the gate reads through `gitea_commit_statuses` whether the forge's merging
+account writes each repository the ask covers (`merge.writes`). Where it does not, nothing is asked: the delivery waits
+on "the forge account cannot write <repo>: gitea_grant_write" (`show`), raised at once as a condition. The read holds
+until the gitea module's `gitea.repo.writable` (its grant, read back) or ten minutes, then the ask is made again; an
+approved merge of that repository the forge refused is asked again on the same event (`writes.go`).
+
 ## The seat's hand-over
 
 The gate took the seat `mesh-delivery` over from the module `mesh-delivery` (ADR 0449 §4, ADR 0450 §6, ADR 0454) and carried its

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go

@@ -339,6 +339,9 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 		return "the operator cannot be asked from here", nil
 	}
 	members := h.askMembers(d)
+	if waits := h.writeWaits(d, members, now); waits != "" {
+		return waits, nil // asked only once the forge account can carry the merge out (writes.go, issue 1945)
+	}
 	group := ""
 	if len(members) > 1 {
 		group = d.Group
@@ -423,6 +426,16 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 				h.mu.Unlock()
 			}
 		}
+		// Nothing is asked that the forge account could not merge (writes.go, issue 1945): said at the next look.
+		if says := h.cannotWrite(reads); says != "" {
+			stale()
+			h.mu.Lock()
+			defer h.mu.Unlock()
+			undo()
+			h.Logf("[mesh-delivery] %s passed its checks and the operator is not asked: %s", d.ID, says)
+			h.lookAt(h.Now(), "an ask not made: the forge account cannot write a repository it covers")
+			return
+		}
 		if readErr != nil || holdSays != "" {
 			stale()
 			h.mu.Lock()

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go

@@ -101,7 +101,9 @@ type fakeForge struct {
 	viewCalls int
 	// What Statuses reads: the pull requests by owner/repo#number, the base's required statuses, whether the
 	// protection cannot be read, and what it was asked.
-	pulls          map[string]ForgePull
+	pulls map[string]ForgePull
+	// unwritable are the repositories (owner/repo) the forge's merging account cannot write (issue 1945).
+	unwritable     map[string]bool
 	required       []string
 	protectionDown bool
 	asked          []string
@@ -220,6 +222,10 @@ func (f *fakeForge) Statuses(owner, repo string, number int, commit, base string
 		base = "main"
 	}
 	out.Merge = ForgeMerge{Branch: base, Protected: f.required != nil, Required: f.required}
+	if f.unwritable[owner+"/"+repo] {
+		no := false
+		out.Merge.Writes, out.Merge.CannotWrite = &no, "the forge account cannot write "+owner+"/"+repo+": gitea_grant_write"
+	}
 	if f.protectionDown {
 		out.Merge.Says = "the protection of " + base + " could not be read"
 		return out, nil

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go

@@ -82,6 +82,8 @@ type Holder struct {
 	// unmergeableSince is when the forge first said a group's member does not merge now, by group and member (issue 1275,
 	// group_merge.go): within forgeMergeGrace that is the forge still working out mergeability, asked again shortly.
 	unmergeableSince map[string]time.Time
+	// writeReads is whether the forge's merging account writes each repository, as last read (writes.go, issue 1945).
+	writeReads map[string]*writeRead
 	// Turns is the gate's bucket turns (turns.go, ADR 0548 §3); nil keeps none. turnsHeld are the turns as written there,
 	// by repository, read once at the first look (turnsLoaded).
 	Turns       TurnStore

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go

@@ -34,7 +34,7 @@ func (l *listening) get() string  { l.mu.Lock(); defer l.mu.Unlock(); return l.n
 // The events this owner follows: the forge's pull requests. The forge's status events say that a check answered (events.go).
 var followed = []string{"gitea.pull.updated", "gitea.pull.merged", "gitea.pull.closed",
 	"gitea.forge.status", "gitea.forge.pull-request.edited", "gitea.forge.pull-request.closed",
-	forgePattern, ConfigChangedEvent}
+	forgePattern, ConfigChangedEvent, writableEvent}
 
 // forgePattern is the forge bridge's messages on FORGE (novox/hq ADR 0437 §8): the gate follows a merge and the refusal of one.
 const forgePattern = "mesh-delivery-forge-bridge.forge.**"
@@ -219,6 +219,10 @@ func take(h *Holder, e stdio.Envelope) error {
 		}
 		h.CommitStatus(s)
 		h.KeepCheckVerdict(s)
+	case writableEvent:
+		if err := h.WriteGranted(e.Body); err != nil {
+			h.Refuse(err)
+		}
 	case ConfigChangedEvent:
 		var c ConfigChange
 		if err := json.Unmarshal(e.Body, &c); err != nil {

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go

@@ -76,6 +76,10 @@ type ForgeMerge struct {
 	} `json:"blocking"`
 	Mergeable *bool  `json:"mergeable"`
 	Says      string `json:"says"`
+	// Writes is whether the forge's merging account may write the repository at all (issue 1945): false, its statuses and
+	// its merge are refused, and CannotWrite says so with the tool that grants it. Nil when the forge holder did not say.
+	Writes      *bool  `json:"writes,omitempty"`
+	CannotWrite string `json:"cannot_write,omitempty"`
 }
 
 // Store keeps deliveries and groups: one key each, in the module's own state (ADR 0201).

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go

@@ -128,6 +128,11 @@ type ReadyRun struct {
 	HeldSince *time.Time `json:"held_since,omitempty"`
 	HeldFor   string     `json:"held_for,omitempty"`
 	HandNow   string     `json:"hand_now,omitempty"`
+
+	// CannotWrite is why the operator is not asked: the forge's merging account cannot write a repository the ask covers
+	// (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"`
 }
 
 // MergeBranch is the one branch this owner merges into (novox/hq ADR 0398 §4): a merge into any other is its author's.
@@ -520,6 +525,12 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
 			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(),
 				Bound: headHeardBound.String(), H2: "none: a later announcement of the pull request's head (the merge tool announces it again, the forge's announcer restarts, or a new push) or an edit of its description; a recheck does not fill it — the operator's",
 				Says: fmt.Sprintf("%s#%d passed its checks at %s and waits for %s", d.Repository, d.Number, shortOf(r.Head, 12), waitsForHead)})
+		case r.CannotWrite != "" && r.Approved == nil && r.CannotWriteSince != nil:
+			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State,
+				For: now.Sub(*r.CannotWriteSince).Round(time.Second).String(), Bound: "0s",
+				H2: "none: the forge account given write on the repository (gitea_grant_write; the gitea module grants it itself once a module of it is registered), which asks the operator then",
+				Says: fmt.Sprintf("%s#%d passed its checks at %s and the operator is not asked for its Approve: %s",
+					d.Repository, d.Number, shortOf(r.Head, 12), r.CannotWrite)})
 		case r.Failed != "" && d.State == Rejected:
 			// Its cure is its author's new head: listed here at once (`show`), told on the pull request at once, and the
 			// operator's only once no new head came within the author's bound (novox/hq ADR 0505 §5).

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes.go

@@ -0,0 +1,115 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+	"time"
+)
+
+// **The operator is asked only for a merge the forge account can carry out** (novox/hq issue 1945). car-hunter #2, a
+// repository of a user, was approved, and its first act after the Approve, the commit status, was refused 403: the mesh's
+// merging account (the gitea module's tools, mesh-tools) only read that repository. Before the gate asks the operator's
+// Approve it reads, through the forge holder's gitea_commit_statuses, whether that account writes each repository the
+// ask covers (merge.writes), as the ask is about to be written. Where it does not, nothing is asked: the delivery waits,
+// saying "the forge account cannot write <repo>: gitea_grant_write", and the gate raises it as a condition at once.
+//
+// A repository not written is kept so, and the ask is not composed again, until the gitea module's event
+// gitea.repo.writable (its grant, read back from the forge) or, as a safety net, writeReadLasts after the read. A forge
+// that does not say merge.writes (an older gitea module) is taken as writing: the merge says its own refusal, as before.
+
+// writeReadLasts is how long a read of whether the forge account writes a repository holds.
+const writeReadLasts = 10 * time.Minute
+
+// writableEvent is the gitea module's event of an account given write on a repository.
+const writableEvent = "gitea.repo.writable"
+
+// writeRead is what the forge last said of the merging account on one repository it cannot write.
+type writeRead struct {
+	says string
+	at   time.Time
+}
+
+// cannotWriteSays is what a delivery waits on when the forge account cannot write its repository.
+func cannotWriteSays(repository string) string {
+	return "the forge account cannot write " + repository + ": gitea_grant_write"
+}
+
+// writeWaits is what an ask about to be made waits on: a repository it covers that the forge account was read, within
+// writeReadLasts, not to write. Under the lock.
+func (h *Holder) writeWaits(d *Delivery, members []*Delivery, now time.Time) string {
+	r := d.Ready
+	for _, m := range members {
+		repo := strings.ToLower(m.Repository)
+		w := h.writeReads[repo]
+		if w == nil {
+			continue
+		}
+		if !now.Before(w.at.Add(writeReadLasts)) {
+			delete(h.writeReads, repo) // read again at the ask
+			continue
+		}
+		if r.CannotWrite != w.says {
+			r.CannotWrite, r.CannotWriteSince = w.says, &now
+			h.keep(d)
+		}
+		h.lookAt(w.at.Add(writeReadLasts), "a repository the forge account could not write, read again")
+		return w.says
+	}
+	if r.CannotWrite != "" {
+		r.CannotWrite, r.CannotWriteSince = "", nil
+		h.keep(d)
+	}
+	return ""
+}
+
+// cannotWrite reads, at the ask, whether the forge account writes each repository the ask covers, and answers what the
+// first one it cannot write says; a read that fails says nothing (the merge says its own refusal, as before). Outside the
+// lock; a repository not written is kept, under the lock.
+func (h *Holder) cannotWrite(reads []memberRead) string {
+	for _, m := range reads {
+		fc, err := h.Forge.Statuses(m.owner, m.repo, m.number, m.head, m.base)
+		if err != nil || fc == nil || fc.Merge.Writes == nil || *fc.Merge.Writes {
+			continue
+		}
+		repo := strings.ToLower(m.owner + "/" + m.repo)
+		says := fc.Merge.CannotWrite
+		if says == "" {
+			says = cannotWriteSays(repo)
+		}
+		h.mu.Lock()
+		if h.writeReads == nil {
+			h.writeReads = map[string]*writeRead{}
+		}
+		h.writeReads[repo] = &writeRead{says: says, at: h.Now()}
+		h.mu.Unlock()
+		return says
+	}
+	return ""
+}
+
+// WriteGranted takes the gitea module's gitea.repo.writable: the repository's read is dropped, so the next look reads it
+// again, and an approved merge of it that the forge refused is asked again, its cause gone.
+func (h *Holder) WriteGranted(body json.RawMessage) error {
+	var e struct {
+		Owner string `json:"owner"`
+		Repo  string `json:"repo"`
+	}
+	if err := json.Unmarshal(body, &e); err != nil || e.Owner == "" || e.Repo == "" {
+		return fmt.Errorf("%s names no repository: %s", writableEvent, string(body))
+	}
+	repo := strings.ToLower(e.Owner + "/" + e.Repo)
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	delete(h.writeReads, repo)
+	for _, d := range h.deliveries {
+		if strings.ToLower(d.Repository) != repo || d.Ready == nil || d.Ready.Approved == nil || d.Ready.Merged != nil {
+			continue
+		}
+		d.Ready.Repeats = 0
+		h.mergeAgainNow(d)
+		h.keep(d)
+	}
+	h.Logf("[mesh-delivery] the forge account was given write on %s: it is read again before an ask, and an approved merge of it is asked again", repo)
+	return nil
+}

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes_test.go

@@ -0,0 +1,44 @@
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+// Issue 1945: car-hunter #2 was asked, approved, and its commit status refused 403, since the forge's merging account only
+// read the repository. 1945/AC3: a pull request whose checks passed, in a repository the merging account cannot write, is
+// not asked; it waits saying so, loud as a condition at once. 1945/AC4: the gitea module's gitea.repo.writable asks it then.
+func TestAnUnwritableRepositoryIsNotAskedAndSaysSoUntilTheWriteIsGranted(t *testing.T) {
+	w := announced(t)
+	conds := hearConditions(w)
+	w.forge.unwritable = map[string]bool{"novox/app": true}
+	hear(t, w, shimCheckedKey, judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	w.h.React()
+	if w.op.count() != 0 || len(w.op.open()) != 0 {
+		t.Fatalf("the operator was asked %d times for a merge the forge account cannot carry out", w.op.count())
+	}
+	d := w.h.deliveries[IDOf("novox/app", readyHead)]
+	want := "the forge account cannot write novox/app: gitea_grant_write"
+	if d.Ready.Waits != want || d.Ready.CannotWrite != want {
+		t.Fatalf("waits %q, cannot_write %q; wanted %q", d.Ready.Waits, d.Ready.CannotWrite, want)
+	}
+	if c := conds.last(d.ID); c == nil || c.State != "raised" || !strings.Contains(c.Summary+c.Explanation+c.Headline, "cannot write novox/app") {
+		t.Fatalf("not said as a condition: %+v", c)
+	}
+	// Looked at again with nothing changed, the forge is not read again: the read holds until the grant or its bound.
+	asked := len(w.forge.asked)
+	w.h.React()
+	if len(w.forge.asked) != asked || w.op.count() != 0 {
+		t.Fatalf("read again %d times, asked %d", len(w.forge.asked)-asked, w.op.count())
+	}
+
+	delete(w.forge.unwritable, "novox/app")
+	hear(t, w, writableEvent, map[string]string{"owner": "novox", "repo": "app", "account": "mesh-tools"})
+	w.h.React()
+	if w.op.count() != 1 {
+		t.Fatalf("after the write was granted the operator was asked %d times, wanted once", w.op.count())
+	}
+	if d.Ready.CannotWrite != "" || strings.Contains(d.Ready.Waits, "cannot write") {
+		t.Fatalf("still says it cannot write: %q / %q", d.Ready.CannotWrite, d.Ready.Waits)
+	}
+}

modules/mesh-delivery-gate/module.json

@@ -26,7 +26,8 @@
     "gitea.forge.pull-request.edited",
     "gitea.forge.pull-request.closed",
     "mesh-delivery-forge-bridge.forge.**",
-    "mesh-config.config-changed"
+    "mesh-config.config-changed",
+    "gitea.repo.writable"
   ],
   "emits": [
     "transition",