From b0f86671b621e0b52636da5810c68d9406cfece6 Mon Sep 17 00:00:00 2001 From: fix-forge-awg Date: Fri, 16 Oct 2026 21:25:59 +0200 Subject: [PATCH 1/8] =?UTF-8?q?Give=20the=20agent's=20token=20write=20scop?= =?UTF-8?q?es=20and=20the=20agents=20write=20team=20(issue=20948,=20ADR=20?= =?UTF-8?q?0398=20=C2=A71)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/gitea/README.md | 16 ++-- modules/gitea/cmd/agent-token/main.go | 14 +-- modules/gitea/cmd/agent-token/token.go | 82 ++++++++++++++--- modules/gitea/cmd/agent-token/token_test.go | 98 +++++++++++++++++++-- 4 files changed, 180 insertions(+), 30 deletions(-) diff --git a/modules/gitea/README.md b/modules/gitea/README.md index d1a3397b..bd4bd433 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -102,21 +102,23 @@ changed under the commit refused; a commit over its size limits refused; protect forge matches them; a code search stopped at its deadline; every malformed ask refused; and each read summarised. -## Each agent's read credential (novox/hq ADR 0365 §1, §3–§5, issue 737) +## Each agent's credential (novox/hq ADR 0365 §1, §3–§5, issue 737; ADR 0398 §1, issue 948) -An agent clones and fetches the repositories it may read as a forge account of its own, `agent-`. The +An agent clones, fetches and pushes branches of the repositories as a forge account of its own, `agent-`. The module offers the provision `gitea-read`, whose credential its software issues (ADR 0305) one per holder (issue 749): the Go bundle `cmd/agent-token` serves `gitea_agent_token`, which the vault alone asks (ADR 0349 clause 10; any other caller is refused and nothing is minted). For each holder the vault names that is the claude-code module on a machine, it - makes the account `agent-` with the administrator's login, when there is none: no administrator, a random - password sent once and kept nowhere, and **in no team** — it reads public repositories until the operator adds it - to a team that reads more (§3). An account found made an administrator, inactive or barred from logging in is + password sent once and kept nowhere. It is added to the team `agents` of the organisation `novox`, which the + administrator's login makes when there is none and puts back to its shape when found changed: write on code, issues + and pull requests of every repository, no right to make repositories (the shape of the forge tools' team). The + operator's read teams (`agent-read-*`, ADR 0365 §3) are `cmd/agent-teams`'s and stay. An account found made an administrator, inactive or barred from logging in is made a plain account again; -- deletes the account's token by the name `mesh-agent-read` (by its id, every one of them) and mints one whose one - scope is `read:repository`: whatever team it is put in, the forge refuses it a push, a comment and any token or - administration of its own (§5). Every ask mints anew; that is its rotation; +- deletes the account's token by the name `mesh-agent-read` (by its id, every one of them) and mints one whose + scopes are `write:repository` and `write:issue`: the forge refuses it any token or administration of its own + (§5), and the main branch of every repository takes no push from it (ADR 0398 §2). Every ask mints anew; that is its rotation; - answers git's credential-store lines for it — the forge's public origin, read from `origins` (the route's name), and on the forge's own machine its address there too — sealed to the key the vault made for the call. A holder of another module is not answered and said under `skipped`. diff --git a/modules/gitea/cmd/agent-token/main.go b/modules/gitea/cmd/agent-token/main.go index 92f5a6d8..0303b670 100644 --- a/modules/gitea/cmd/agent-token/main.go +++ b/modules/gitea/cmd/agent-token/main.go @@ -1,10 +1,10 @@ -// agent-token: the forge's read credential for each node's agent (novox/hq ADR 0365 clauses 1, 3, 4 and 5; issue -// 737). A process the node's runtime launches and speaks MCP over stdio to, beside the module's other bundles. +// agent-token: the forge's credential for each node's agent (novox/hq ADR 0365 clauses 1, 3, 4 and 5, ADR 0398 §1; issues +// 737, 948). A process the node's runtime launches and speaks MCP over stdio to, beside the module's other bundles. // // It serves one tool, gitea_agent_token, which the vault alone asks (ADR 0349 clause 10, issue 749): for each holder // of the provision `gitea-read` the vault names — the claude-code module on one machine — it makes that machine's -// agent a forge account of its own, `agent-`, no administrator and in no team, mints it a token whose one -// scope is `read:repository`, and answers the git credential lines for it, each holder's sealed to the key the vault +// agent a forge account of its own, `agent-`, no administrator and a member of the `agents` write team, mints it a token whose +// scopes are `write:repository` and `write:issue`, and answers the git credential lines for it, each holder's sealed to the key the vault // made for the call. Asked again, it mints again and deletes the account's token from before: that is its rotation. // // stdout is the protocol; it says what it says on stderr, and never a token or the admin's password. @@ -33,10 +33,10 @@ func main() { func Tools(minter func() (*Minter, error)) []stdio.Tool { return []stdio.Tool{{ Name: ToolName, - Description: "The forge's read credential for each node's agent (novox/hq ADR 0365), answered to the vault " + + Description: "The forge's credential for each node's agent (novox/hq ADR 0365, ADR 0398), answered to the vault " + "alone: for each holder named, a forge account of the node's agent's own (agent-), no administrator " + - "and in no team, and a token of scope read:repository minted for it anew, its git credential lines " + - "sealed to seal_to. Any other caller is refused and nothing is minted.", + "and a member of the agents team (write on every repository), and a token of scopes write:repository and " + + "write:issue minted for it anew, its git credential lines sealed to seal_to. Any other caller is refused and nothing is minted.", Input: map[string]any{ "type": "object", "properties": map[string]any{ diff --git a/modules/gitea/cmd/agent-token/token.go b/modules/gitea/cmd/agent-token/token.go index 33e8de9e..a827312f 100644 --- a/modules/gitea/cmd/agent-token/token.go +++ b/modules/gitea/cmd/agent-token/token.go @@ -23,7 +23,7 @@ import ( const ( // ToolName is the issuing tool the vault asks (the offer's credential tool in module.json). ToolName = "gitea_agent_token" - // Provision is what the tool issues: the forge's read credential, one per agent. + // Provision is what the tool issues: the forge's agent credential (named gitea-read from before it wrote), one per agent. Provision = "gitea-read" // Holder is the one consuming module answered: the agent's own module, which places the credential in the // agent's home (ADR 0365 §4, ADR 0266). A holder of another module is not answered and keeps nothing. @@ -38,10 +38,26 @@ const ( deadline = 45 * time.Second ) -// TokenScopes are the only scopes an agent's token carries (ADR 0365 §4, §5): it reads repositories and nothing -// else, so the forge refuses it a push, a comment, a token of its own and any administration — whatever team the -// account is in. -var TokenScopes = []string{"read:repository"} +// TokenScopes are the only scopes an agent's token carries (ADR 0398 §1, issue 948): it writes repositories and +// issues and nothing else, so the forge refuses it a token of its own and any administration. Where it may write is +// the agents team's (a write team on every repository); main takes no push whatever the token (ADR 0398 §2). +var TokenScopes = []string{"write:repository", "write:issue"} + +const ( + // TeamOrg is the organisation whose repositories are the mesh's. + TeamOrg = "novox" + // TeamName is the one team every agent's account is in (ADR 0398 §1). + TeamName = "agents" +) + +// teamShape is what the agents team grants: write on code, issues and pull requests of every repository of the +// organisation and no more, the shape of the forge tools' team (tools-account.ts). +func teamShape() map[string]any { + return map[string]any{ + "name": TeamName, "permission": "write", "includes_all_repositories": true, "can_create_org_repo": false, + "units_map": map[string]string{"repo.code": "write", "repo.issues": "write", "repo.pulls": "write"}, + } +} var nodeName = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,31}$`) @@ -193,6 +209,9 @@ func (m *Minter) Answer(ask Ask) (Answer, error) { if err := m.ensureAccount(ctx, account); err != nil { return Answer{}, err } + if err := m.ensureInTeam(ctx, account); err != nil { + return Answer{}, err + } token, err := m.mint(ctx, account) if err != nil { return Answer{}, err @@ -208,7 +227,7 @@ func (m *Minter) Answer(ask Ask) (Answer, error) { return Answer{}, fmt.Errorf("the credential for %s could not be sealed: %w", account, err) } out.Holders = append(out.Holders, Sealed{Node: h.Node, Module: h.Module, Sealed: base64.StdEncoding.EncodeToString(sealed)}) - m.say("minted a read token for %s (%s on %s); its token from before is deleted", account, h.Module, h.Node) + m.say("minted a write token for %s (%s on %s); its token from before is deleted", account, h.Module, h.Node) } return out, nil } @@ -264,8 +283,7 @@ type forgeUser struct { } // ensureAccount makes the agent's account, plain, when there is none, and puts back what was changed by hand: never -// an administrator, active, able to use its token. It adds it to no team: what it may read beyond public -// repositories is the operator's to grant (ADR 0365 §3). +// an administrator, active, able to use its token. Its team is ensureInTeam's. func (m *Minter) ensureAccount(ctx context.Context, account string) error { var u forgeUser err := m.Forge.GetJSON(ctx, "/api/v1/users/"+account, &u) @@ -280,7 +298,7 @@ func (m *Minter) ensureAccount(ctx context.Context, account string) error { if err != nil { return fmt.Errorf("the account %s could not be made: %w", account, err) } - m.say("made the forge account %s for that machine's agent, in no team", account) + m.say("made the forge account %s for that machine's agent, in no team yet", account) return nil case err != nil: return fmt.Errorf("the account %s could not be read: %w", account, err) @@ -297,6 +315,50 @@ func (m *Minter) ensureAccount(ctx context.Context, account string) error { return nil } +// ensureInTeam makes the agents team when there is none, puts a team that is there back to its shape, and adds the +// account to it; an account already in it stays (ADR 0398 §1, issue 948). +func (m *Minter) ensureInTeam(ctx context.Context, account string) error { + var id int64 + for page := 1; page <= 20 && id == 0; page++ { + var teams []struct { + ID int64 `json:"id"` + Name string `json:"name"` + } + if err := m.Forge.GetJSON(ctx, fmt.Sprintf("/api/v1/orgs/%s/teams?limit=50&page=%d", TeamOrg, page), &teams); err != nil { + return fmt.Errorf("the teams of %s could not be read: %w", TeamOrg, err) + } + for _, t := range teams { + if t.Name == TeamName { + id = t.ID + } + } + if len(teams) < 50 { + break + } + } + if id != 0 { + if err := m.Forge.SendJSON(ctx, http.MethodPatch, fmt.Sprintf("/api/v1/teams/%d", id), teamShape(), nil); err != nil { + return fmt.Errorf("the team %s could not be put back to its shape: %w", TeamName, err) + } + } else { + var made struct { + ID int64 `json:"id"` + } + if err := m.Forge.SendJSON(ctx, http.MethodPost, "/api/v1/orgs/"+TeamOrg+"/teams", teamShape(), &made); err != nil { + return fmt.Errorf("the team %s could not be made: %w", TeamName, err) + } + id = made.ID + m.say("made the forge team %s in %s: write on every repository", TeamName, TeamOrg) + } + if id == 0 { + return fmt.Errorf("the forge answered no id for the team %s", TeamName) + } + if err := m.Forge.SendJSON(ctx, http.MethodPut, fmt.Sprintf("/api/v1/teams/%d/members/%s", id, account), nil, nil); err != nil { + return fmt.Errorf("the account %s could not be added to the team %s: %w", account, TeamName, err) + } + return nil +} + type forgeToken struct { ID int64 `json:"id"` Name string `json:"name"` @@ -304,7 +366,7 @@ type forgeToken struct { } // mint deletes every token of the account by this module's name, by its id (a name held twice is cleared, not -// refused), and mints one with the read scope alone. +// refused), and mints one with the two write scopes alone. func (m *Minter) mint(ctx context.Context, account string) (string, error) { var held []forgeToken if err := m.Forge.GetJSON(ctx, "/api/v1/users/"+account+"/tokens?limit=50", &held); err != nil { diff --git a/modules/gitea/cmd/agent-token/token_test.go b/modules/gitea/cmd/agent-token/token_test.go index 3f12dc4d..9fbdfb63 100644 --- a/modules/gitea/cmd/agent-token/token_test.go +++ b/modules/gitea/cmd/agent-token/token_test.go @@ -33,13 +33,20 @@ type fakeForge struct { made map[string]map[string]any minted map[string][]string // account -> scopes of its last mint failMint bool + // teams by name; members by team name (issue 948). + teams map[string]map[string]any + teamIDs map[string]int64 + members map[string][]string } func newFakeForge() *fakeForge { return &fakeForge{users: map[string]*forgeUser{}, tokens: map[string][]forgeToken{}, made: map[string]map[string]any{}, - minted: map[string][]string{}, next: 1} + minted: map[string][]string{}, next: 1, + teams: map[string]map[string]any{}, teamIDs: map[string]int64{}, members: map[string][]string{}} } +var teamPath = regexp.MustCompile(`^/api/v1/teams/(\d+)(?:/members/([^/]+))?$`) + var tokenPath = regexp.MustCompile(`^/api/v1/users/([^/]+)/tokens(?:/(\d+))?$`) func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -80,6 +87,40 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { u.Active, _ = in["active"].(bool) u.ProhibitLogin, _ = in["prohibit_login"].(bool) reply(200, map[string]any{}) + case r.URL.Path == "/api/v1/orgs/novox/teams": + if r.Method == http.MethodGet { + var list []map[string]any + for name, id := range f.teamIDs { + list = append(list, map[string]any{"id": id, "name": name}) + } + reply(200, list) + return + } + name, _ := in["name"].(string) + f.teams[name] = in + f.teamIDs[name] = f.next + f.next++ + reply(201, map[string]any{"id": f.teamIDs[name], "name": name}) + case teamPath.MatchString(r.URL.Path): + m := teamPath.FindStringSubmatch(r.URL.Path) + for name, id := range f.teamIDs { + if fmt.Sprint(id) != m[1] { + continue + } + if m[2] == "" && r.Method == http.MethodPatch { + f.teams[name] = in + reply(200, map[string]any{"id": id}) + return + } + if m[2] != "" && r.Method == http.MethodPut { + if !slices.Contains(f.members[name], m[2]) { + f.members[name] = append(f.members[name], m[2]) + } + reply(204, nil) + return + } + } + reply(404, map[string]any{"message": "no such team"}) case tokenPath.MatchString(r.URL.Path): m := tokenPath.FindStringSubmatch(r.URL.Path) account := m[1] @@ -199,7 +240,7 @@ func TestOnlyTheVaultIsAnsweredAndNothingIsMintedForAnyoneElse(t *testing.T) { } } -func TestEachAgentGetsAnAccountOfItsOwnAndAReadTokenSealedForItAlone(t *testing.T) { +func TestEachAgentGetsAnAccountOfItsOwnAndAWriteTokenSealedForItAlone(t *testing.T) { h := newHarness(t) ask, err := readAsk(h.args(HolderAsk{"anchor", "claude-code"}, HolderAsk{"laptop", "claude-code"}, HolderAsk{"laptop", "lab"})) if err != nil { @@ -240,15 +281,23 @@ func TestEachAgentGetsAnAccountOfItsOwnAndAReadTokenSealedForItAlone(t *testing. if made := h.forge.made[account]; made["must_change_password"] != false || made["password"] == "" { t.Errorf("%s was made with %v", account, made) } - if !slices.Equal(h.forge.minted[account], []string{"read:repository"}) { - t.Errorf("%s's token carries %v, want read:repository alone", account, h.forge.minted[account]) + if !slices.Equal(h.forge.minted[account], []string{"write:repository", "write:issue"}) { + t.Errorf("%s's token carries %v, want write:repository and write:issue alone (issue 948)", account, h.forge.minted[account]) } } for _, c := range h.forge.calls { - if strings.Contains(c, "/teams") || strings.Contains(c, "/orgs/") || strings.Contains(c, "collaborators") { - t.Errorf("an agent's account was given access (%s): it starts in no team, the operator grants (ADR 0365 §3)", c) + if strings.Contains(c, "collaborators") { + t.Errorf("an agent's account was given access one repository at a time (%s) (issue 948)", c) } } + for _, account := range []string{"agent-anchor", "agent-laptop"} { + if !slices.Contains(h.forge.members["agents"], account) { + t.Errorf("%s is not in the agents team: %v (issue 948, ADR 0398 §1)", account, h.forge.members) + } + } + if len(h.forge.members) != 1 { + t.Errorf("an agent was put in a team but agents: %v", h.forge.members) + } if _, made := h.forge.users["agent-laptop-lab"]; made || len(h.forge.users) != 2 { t.Errorf("accounts made: %v", h.forge.users) } @@ -374,3 +423,40 @@ func tokenIn(t *testing.T, lines, account string) string { } return all[0][1] } + +func TestTheAgentsTeamIsMadeWithWriteOnEveryRepositoryAndReusedWhenThere(t *testing.T) { + h := newHarness(t) + ask, _ := readAsk(h.args(HolderAsk{"laptop", "claude-code"})) + if _, err := h.minter.Answer(ask); err != nil { + t.Fatal(err) + } + team := h.forge.teams["agents"] + if team == nil { + t.Fatalf("no agents team was made (issue 948): %v", h.forge.teams) + } + if team["permission"] != "write" || team["includes_all_repositories"] != true || team["can_create_org_repo"] != false { + t.Errorf("the agents team is %v, want write on all repositories and no repository creation", team) + } + units, _ := team["units_map"].(map[string]any) + for _, u := range []string{"repo.code", "repo.issues", "repo.pulls"} { + if units[u] != "write" { + t.Errorf("unit %s is %v, want write", u, units[u]) + } + } + id := h.forge.teamIDs["agents"] + // Changed by hand, then asked again for another node: reused, put back, the new account added. + h.forge.teams["agents"] = map[string]any{"name": "agents", "permission": "read"} + ask, _ = readAsk(h.args(HolderAsk{"laptop", "claude-code"}, HolderAsk{"anchor", "claude-code"})) + if _, err := h.minter.Answer(ask); err != nil { + t.Fatal(err) + } + if h.forge.teamIDs["agents"] != id || len(h.forge.teamIDs) != 1 { + t.Errorf("the team was made again: %v", h.forge.teamIDs) + } + if h.forge.teams["agents"]["permission"] != "write" { + t.Errorf("the team was not put back to write: %v", h.forge.teams["agents"]) + } + if !slices.Equal(h.forge.members["agents"], []string{"agent-laptop", "agent-anchor"}) { + t.Errorf("members %v, each once", h.forge.members["agents"]) + } +} From 417f69567cec849373d1f395a94dfeaa80b7aaf4 Mon Sep 17 00:00:00 2001 From: "fix-deliv-awg@novox" Date: Fri, 16 Oct 2026 21:26:53 +0200 Subject: [PATCH 2/8] =?UTF-8?q?Merge=20only=20into=20main:=20a=20ready=20m?= =?UTF-8?q?ark=20on=20another=20base=20is=20left=20to=20its=20author=20(is?= =?UTF-8?q?sue=20948,=20ADR=200398=20=C2=A74)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../mesh-delivery/cmd/mesh-delivery/ready.go | 18 ++++++++++++ .../cmd/mesh-delivery/ready_test.go | 28 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index f6f98672..fa36708a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -69,8 +69,14 @@ type ReadyRun struct { Merged *time.Time `json:"merged,omitempty"` // Failed is why it is not merged and will not be on this head: its check failed. Said once on the pull request. Failed string `json:"failed,omitempty"` + // OtherBase is the branch its pull request merges into when that is not main: never merged here, said once on the + // pull request (novox/hq ADR 0398 §4, issue 948). + OtherBase string `json:"other_base,omitempty"` } +// MergeBranch is the one branch this owner merges into (novox/hq ADR 0398 §4): a merge into any other is its author's. +const MergeBranch = "main" + // mergedBy is who a merge this owner made on a ready mark is said to be by, in its transitions. func (r *ReadyRun) mergedBy() string { if r == nil || r.Merged == nil { @@ -185,6 +191,18 @@ func (h *Holder) readyAsks(now time.Time) []func() { } continue } + if p.Base != MergeBranch { + if r.OtherBase != p.Base { + r.OtherBase = p.Base + h.Logf("[mesh-delivery] %s is marked ready and not merged: it merges into %s, not %s", d.ID, p.Base, MergeBranch) + h.keep(d) + asks = append(asks, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** This pull request is marked ready "+ + "at %s, and it merges into %s. Only a merge into %s is this owner's; a merge into another branch is its "+ + "author's (novox/hq ADR 0398 §4). The mark stays.", mark.Head, p.Base, MergeBranch))) + } + continue + } + r.OtherBase = "" r.Failed = "" waits := h.readyWaits(d) if waits != r.Waits { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go b/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go index e818e9b1..565f08bb 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready_test.go @@ -197,3 +197,31 @@ func TestAGroupMergesItsMarkedMembersInOrderOnlyOnceComposed(t *testing.T) { t.Fatalf("after the first member merged, the next was not: %v", w.forge.merges) } } + +// **A merge into another branch than main is its author's** (novox/hq ADR 0398 §4, issue 948): a ready mark on a pull +// request whose base is not main is never merged by this owner; it says so once on the pull request and leaves the mark. +func TestAMarkedGreenPullRequestIntoAnotherBranchIsNotMergedAndSaidOnce(t *testing.T) { + w := newWorld(t) + p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go") + p.Base = "release/x" + w.h.PullUpdated(p) + w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + open := markedPull("novox/app", 7, readyHead, readyHead) + open.Base = "release/x" + w.forge.open = []OpenPull{open} + for range 3 { + w.h.SeeOpenPulls() + w.settleAll() + w.later(2 * time.Minute) + } + if len(w.forge.merges) != 0 { + t.Fatalf("a pull request into release/x was merged: %v", w.forge.merges) + } + if len(w.forge.comments) != 1 || !strings.Contains(w.forge.comments[0], "another branch") || + !strings.Contains(w.forge.comments[0], "0398") { + t.Fatalf("not said exactly once on the pull request: %v", w.forge.comments) + } + if len(w.forge.unlabels) != 0 { + t.Fatalf("the mark was taken off: %v", w.forge.unlabels) + } +} From 7807135a1bdbd941bfaae0279c435e8d040ba864 Mon Sep 17 00:00:00 2001 From: fix-forge-awg Date: Fri, 16 Oct 2026 21:28:43 +0200 Subject: [PATCH 3/8] =?UTF-8?q?Main=20takes=20no=20push=20and=20only=20mes?= =?UTF-8?q?h-tools=20merges,=20set=20by=20setup=20and=20guarded=20(issue?= =?UTF-8?q?=20948,=20ADR=200398=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/gitea/README.md | 10 ++++++ modules/gitea/client.ts | 40 ++++++++++++++++++++-- modules/gitea/guard.ts | 26 ++++++++++++-- modules/gitea/protection.ts | 18 +++++++++- modules/gitea/test/guard.test.ts | 18 ++++++++++ modules/gitea/test/protection.test.ts | 32 ++++++++++++++++-- modules/gitea/test/token.test.ts | 2 ++ modules/gitea/test/tools-account.test.ts | 43 ++++++++++++++++++++++-- modules/gitea/tools-account.ts | 5 +++ modules/gitea/tools/index.ts | 5 +-- 10 files changed, 187 insertions(+), 12 deletions(-) diff --git a/modules/gitea/README.md b/modules/gitea/README.md index bd4bd433..cb97c96f 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -211,3 +211,13 @@ run before anything is deleted; an unreadable range keeping its whole package; a without saying the password. And the range matcher against npm's rules — exact, `^`, `~`, `x` and `*`, comparisons with partial versions, hyphen ranges, `||`, and the pre-release rule — and each lockfile format. + +## Main takes no push, and only the tools' account merges (novox/hq ADR 0398 §2, issue 948) + +The rule for `main` is the same in every repository of the organisation `novox`: `enable_push` false, and a merge +whitelist of the tools' account (`mesh-tools`) alone. `protection.ts` composes it (`mainRuleBody`, and +`protectionBody` for the branch `main` always carries it); `setUpToolsAccount` puts it right in every repository +whenever the tools' account is set up, with the administrator's login (`GiteaAdmin.ensureMainMerges`) — the tools' +account is no repository's administrator, so no tool can. A rule found keeps the statuses it requires; a repository +with none is given one that requires none. `judgeProtectionChange` refuses, for `main`, a change that enables +push, drops the merge whitelist or widens it past that account. diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index bb8de0d2..e0b80364 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -5,7 +5,7 @@ import { readFileSync } from "node:fs"; import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js"; -import { protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; +import { MAIN_BRANCH, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; import { Refused, checkSentPath, pathSegment, pathSegments } from "./guard.js"; import { isCommit } from "./checks.js"; import { TOOLS_ACCOUNT } from "./tools-account.js"; @@ -713,6 +713,9 @@ export class GiteaClient { enable_status_check: Boolean(b.enable_status_check), status_check_contexts: Array.isArray(b.status_check_contexts) ? b.status_check_contexts.map(String) : [], required_approvals: Number(b.required_approvals ?? 0), + // Not every forge version sends these on a branch; absent they stay absent, never read as "no whitelist". + ...(b.enable_merge_whitelist === undefined ? {} : { enable_merge_whitelist: Boolean(b.enable_merge_whitelist) }), + ...(Array.isArray(b.merge_whitelist_usernames) ? { merge_whitelist_usernames: b.merge_whitelist_usernames.map(String) } : {}), }; } @@ -721,7 +724,7 @@ export class GiteaClient { * asked — or created, refusing direct pushes unless asked otherwise. Answers the rule as it is now. */ async setBranchProtection(owner: string, repo: string, branch: string, want: ProtectionWanted): Promise<{ created: boolean; rule: BranchProtection }> { const had = await this.branchProtection(owner, repo, branch); - const body = protectionBody(want, !had); + const body = protectionBody(want, !had, branch); if (had) { const rule = await this.request(`${repoPath(owner, repo)}/branch_protections/${pathSegments("branch", branch)}`, { method: "PATCH", body: JSON.stringify(body) }); @@ -988,6 +991,39 @@ export class GiteaAdmin { return Array.isArray(res.body) ? res.body : []; } + /** Put main's rule right in every repository of an organisation (novox/hq ADR 0398 §2, issue 948): no push, and + * only the tools' account merges. The rule there is edited, its statuses left as they are; a repository with no + * rule for main gets one that requires none. Answers the repositories it touched. With the administrator's login: + * the tools' account is no repository's administrator, so no tool can do this (ADR 0365 §2). */ + async ensureMainMerges(org: string): Promise { + const repos: string[] = []; + for (let page = 1; page <= 200; page++) { + const res = await this.request(`/orgs/${encodeURIComponent(org)}/repos?limit=50&page=${page}`); + if (res.status !== 200) GiteaAdmin.fail(`/orgs/${org}/repos`, res); + const list = Array.isArray(res.body) ? res.body : []; + for (const r of list) if (r?.name) repos.push(String(r.name)); + if (list.length < 50) break; + } + for (const repo of repos) { + const rule = `${repoPath(org, repo)}/branch_protections/${MAIN_BRANCH}`; + const patch = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); + if (patch.status === 200) continue; + if (patch.status !== 404) GiteaAdmin.fail(rule, patch); + const made = await this.request(`${repoPath(org, repo)}/branch_protections`, { + method: "POST", + body: JSON.stringify({ rule_name: MAIN_BRANCH, enable_status_check: false, status_check_contexts: [], ...mainRuleBody() }), + }); + if (made.status === 201) continue; + if (made.status === 403 || made.status === 409 || made.status === 422) { + // A rule made between the two calls: edit it. + const again = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); + if (again.status === 200) continue; + } + GiteaAdmin.fail(`${repoPath(org, repo)}/branch_protections`, made); + } + return repos; + } + /** Ensure the npm-owner org exists. 201 created, 2xx/404-then-created, and 422/409 (a concurrent * create won the race) are all success. */ async ensureOrg(name: string): Promise { diff --git a/modules/gitea/guard.ts b/modules/gitea/guard.ts index daee4265..fb7a31ee 100644 --- a/modules/gitea/guard.ts +++ b/modules/gitea/guard.ts @@ -17,7 +17,8 @@ // // Pure, so it is tested without a forge; the tools do the asking (tools/index.ts). -import type { BranchProtection, ProtectionWanted } from "./protection.js"; +import { MAIN_BRANCH, type BranchProtection, type ProtectionWanted } from "./protection.js"; +import { TOOLS_ACCOUNT } from "./tools-account.js"; /** A refusal of policy, in the words the caller sees. Never an error of the forge's. */ export class Refused extends Error { @@ -287,7 +288,10 @@ export function judgeContentsWrite(target: string, defaultBranch: string, rules: * override allowed. */ export function judgeProtectionChange(before: BranchProtection | null, want: ProtectionWanted, - covering: BranchProtection[] = []): string | null { + covering: BranchProtection[] = [], branch = ""): string | null { + const isMain = branch === MAIN_BRANCH || (before !== null && (before.rule_name ?? before.branch_name) === MAIN_BRANCH); + const main = isMain ? judgeMain(before, want) : null; + if (main) return main; if (!before) { for (const glob of covering) { const weaker = judgeProtectionChange(glob, { @@ -315,6 +319,24 @@ export function judgeProtectionChange(before: BranchProtection | null, want: Pro `operator's, at the forge as themselves. Nothing was changed`; } +/** Main's rule takes no push and lets only the tools' account merge (novox/hq ADR 0398 §2, issue 948): a change + * that enables push, drops the merge whitelist or widens it past that account is refused, whatever the rule was. */ +function judgeMain(before: BranchProtection | null, want: ProtectionWanted): string | null { + const why: string[] = []; + if (want.push === true) why.push("it lets a person push to main directly"); + if (want.mergeWhitelist !== undefined) { + const names = want.mergeWhitelist ?? []; + if (names.length === 0) why.push("it drops the merge whitelist of main"); + else if (names.some((n) => n !== TOOLS_ACCOUNT)) { + why.push(`it widens who may merge into main beyond ${TOOLS_ACCOUNT}`); + } + } + if (!why.length) return null; + return `the protection of ${before?.rule_name ?? before?.branch_name ?? "main"} is only made stronger through a tool: ` + + `${why.join("; ")}. Main takes no push, and only ${TOOLS_ACCOUNT} merges into it (novox/hq ADR 0398 §2) — ` + + `weakening that is the operator's, at the forge as themselves. Nothing was changed`; +} + /** The token's own user's repositories, untouched for this long, are the only ones a tool deletes. */ export const DELETABLE_AFTER_DAYS = 30; diff --git a/modules/gitea/protection.ts b/modules/gitea/protection.ts index 5751c2c7..923d10c1 100644 --- a/modules/gitea/protection.ts +++ b/modules/gitea/protection.ts @@ -2,6 +2,11 @@ // `mesh/merge-gate`, the module graph's gate, and `mesh/repo-check`, the repository's own tests — before it // may merge. Pure, so it is tested without a forge; the client does the asking (client.ts). +import { TOOLS_ACCOUNT } from "./tools-account.js"; + +/** The branch whose rule takes no push and lets only the tools' account merge (novox/hq ADR 0398 §2, issue 948). */ +export const MAIN_BRANCH = "main"; + /** A branch protection rule, as the forge keeps it — the fields the mesh reads; the forge sends more. */ export interface BranchProtection { rule_name?: string; @@ -11,6 +16,8 @@ export interface BranchProtection { status_check_contexts?: string[] | null; required_approvals?: number; block_admin_merge_override?: boolean; + enable_merge_whitelist?: boolean; + merge_whitelist_usernames?: string[] | null; [field: string]: unknown; } @@ -23,15 +30,24 @@ export interface ProtectionWanted { push?: boolean; /** Whether an administrator is stopped from merging past a status that has not succeeded. */ blockAdminOverride?: boolean; + /** Who may merge into the branch; for main only ever the tools' account (ADR 0398 §2). Not given: left as it is. + * Empty or null asks for no whitelist, which the guard refuses for main; the body for main ignores it. */ + mergeWhitelist?: string[] | null; +} + +/** What main's rule always says: no push, and only the tools' account merges (novox/hq ADR 0398 §2, issue 948). */ +export function mainRuleBody(): Record { + return { enable_push: false, enable_merge_whitelist: true, merge_whitelist_usernames: [TOOLS_ACCOUNT] }; } /** The forge's body for a wanted protection. */ -export function protectionBody(want: ProtectionWanted, creating: boolean): Record { +export function protectionBody(want: ProtectionWanted, creating: boolean, branch = ""): Record { const checks = [...new Set(want.statusChecks.map((c) => c.trim()).filter(Boolean))]; const body: Record = { enable_status_check: checks.length > 0, status_check_contexts: checks }; if (want.push !== undefined) body.enable_push = want.push; else if (creating) body.enable_push = false; if (want.blockAdminOverride !== undefined) body.block_admin_merge_override = want.blockAdminOverride; + if (branch === MAIN_BRANCH) Object.assign(body, mainRuleBody()); // whatever was asked: main takes no push (ADR 0398 §2) return body; } diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 034bf238..fda32d5d 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -491,6 +491,24 @@ test("the judges, without a forge", () => { assert.equal(judgeProtectionChange(protectedMain(), { statusChecks: ["mesh/repo-check", "mesh/merge-gate"] }), null); }); +test("main's protection is never weakened in its push or its merge whitelist (issue 948, ADR 0398 §2)", () => { + const strong = () => ({ ...protectedMain(), enable_merge_whitelist: true, merge_whitelist_usernames: ["mesh-tools"] }); + const checks = ["mesh/merge-gate", "mesh/repo-check"]; + assert.equal(judgeProtectionChange(strong(), { statusChecks: checks }, [], "main"), null); + assert.equal(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: ["mesh-tools"] }, [], "main"), null); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: [] }, [], "main") ?? "", /merge whitelist/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: null }, [], "main") ?? "", /merge whitelist/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: ["mesh-tools", "agent-ace"] }, [], "main") ?? "", /widens/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, mergeWhitelist: ["agent-ace"] }, [], "main") ?? "", /widens/); + assert.match(judgeProtectionChange(strong(), { statusChecks: checks, push: true }, [], "main") ?? "", /push/); + // Even a rule that lets a person push already, or has no rule yet, is not given a push through a tool. + assert.match(judgeProtectionChange({ ...strong(), enable_push: true }, { statusChecks: checks, push: true }, [], "main") ?? "", /push/); + assert.match(judgeProtectionChange(null, { statusChecks: checks, push: true }, [], "main") ?? "", /push/); + assert.match(judgeProtectionChange(null, { statusChecks: checks, mergeWhitelist: ["agent-ace"] }, [], "main") ?? "", /widens/); + // Another branch is judged as before. + assert.equal(judgeProtectionChange(null, { statusChecks: checks, push: true }, [], "feat/x"), null); +}); + test("gitea_api never acts as another user: sudo, in any case, and any query key it does not send are refused", async () => { for (const path of ["/user?sudo=jschoubben", "/user?SUDO=jschoubben", "/repos/novox/mesh-catalog/issues?state=open&Sudo=jschoubben", "/repos/novox/mesh-catalog/issues?token=abc", "/repos/novox/mesh-catalog/issues?access_token=abc"]) { diff --git a/modules/gitea/test/protection.test.ts b/modules/gitea/test/protection.test.ts index 9dc122de..d75422d7 100644 --- a/modules/gitea/test/protection.test.ts +++ b/modules/gitea/test/protection.test.ts @@ -5,7 +5,7 @@ import { test } from "node:test"; // mesh's merge check before it merges. test("the statuses asked for replace the rule's, and an empty list requires none", async () => { - const { protectionBody } = await import("../protection.ts"); + const { protectionBody } = await import("../dist/protection.js"); assert.deepEqual(protectionBody({ statusChecks: ["mesh/merge-gate", " mesh/repo-check", "", "mesh/merge-gate"] }, false), { enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "mesh/repo-check"] }, "an edited rule keeps its pushes as they are, and each status once"); @@ -13,9 +13,37 @@ test("the statuses asked for replace the rule's, and an empty list requires none }); test("a new rule refuses direct pushes unless asked; an administrator's override only when said", async () => { - const { protectionBody } = await import("../protection.ts"); + const { protectionBody } = await import("../dist/protection.js"); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"] }, true).enable_push, false); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"], push: true }, true).enable_push, true); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"] }, true).block_admin_merge_override, undefined); assert.equal(protectionBody({ statusChecks: ["mesh/merge-gate"], blockAdminOverride: true }, false).block_admin_merge_override, true); }); + +// Issue 948 (novox/hq ADR 0398 §2): main takes no push, and only the tools' account merges. +test("main's body always refuses pushes and lets only the tools' account merge, whatever is asked", async () => { + const { protectionBody } = await import("../dist/protection.js"); + const { TOOLS_ACCOUNT } = await import("../dist/tools-account.js"); + for (const creating of [true, false]) { + for (const want of [{ statusChecks: ["mesh/merge-gate"] }, { statusChecks: [], push: true }, { statusChecks: [], push: false }]) { + const body = protectionBody(want, creating, "main"); + assert.equal(body.enable_push, false, "main takes no push, an edit and a push=true included"); + assert.equal(body.enable_merge_whitelist, true); + assert.deepEqual(body.merge_whitelist_usernames, [TOOLS_ACCOUNT]); + } + } + assert.deepEqual(TOOLS_ACCOUNT, "mesh-tools"); +}); + +test("another branch's body carries no merge whitelist", async () => { + const { protectionBody } = await import("../dist/protection.js"); + const body = protectionBody({ statusChecks: ["mesh/merge-gate"] }, false, "release/1"); + assert.equal(body.enable_merge_whitelist, undefined); + assert.equal(body.merge_whitelist_usernames, undefined); + assert.equal(protectionBody({ statusChecks: [] }, false).enable_merge_whitelist, undefined); +}); + +test("the rule main is put right in the forge's own words: the body a setup sends carries no status checks of its own", async () => { + const { mainRuleBody } = await import("../dist/protection.js"); + assert.deepEqual(mainRuleBody(), { enable_push: false, enable_merge_whitelist: true, merge_whitelist_usernames: ["mesh-tools"] }); +}); diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index d72202f2..1df14e36 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -204,6 +204,8 @@ function fakeForge(): Promise { if (req.method === "GET" && p === "/api/v1/orgs/novox/teams") return json(res, 200, [{ id: 7, name: "mesh-tools" }]); if (req.method === "PATCH" && p === "/api/v1/teams/7") return json(res, 200, { id: 7 }); if (req.method === "PUT" && p === "/api/v1/teams/7/members/mesh-tools") return json(res, 204, null); + // Main's rule put right in every repository (issue 948): this forge has none. + if (req.method === "GET" && p === "/api/v1/orgs/novox/repos") return json(res, 200, []); } const adminUser = url.pathname.match(/^\/api\/v1\/admin\/users\/([^/]+)$/); if (adminUser && req.method === "PATCH") { diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 6941a13a..02c947ad 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -31,6 +31,10 @@ function fakeForge() { const tokens: Token[] = []; const teams: Team[] = []; const calls: string[] = []; + // The organisation's repositories, and the rule each has for main, which the administrator's setup puts right + // (issue 948, ADR 0398 §2). + const repos = ["hq", "mesh-catalog"]; + const mainRules = new Map([["hq", { rule_name: "main", enable_status_check: true, status_check_contexts: ["mesh/merge-gate"] }]]); // What the forge does wrong when a test says so: a token mint or delete it fails, a team another process made // between this one's look and its create (gitea answers the create 422). const faults = { mint: false, deleteToken: false, teamRace: false }; @@ -131,6 +135,10 @@ function fakeForge() { if (req.method === "PATCH") { t.units = await read(req); return json(res, 200, { id: t.id }); } if (req.method === "PUT" && m[2]) { t.members.add(decodeURIComponent(m[2])); return json(res, 204, null); } } + if (req.method === "GET" && p === "/orgs/novox/repos") { + if (!isAdmin(me)) return json(res, 403, { message: "must be an owner of the organisation" }); + return json(res, 200, url.searchParams.get("page") === "1" ? repos.map((name) => ({ name })) : []); + } // Repository administration: a site administrator, or a team member whose team administers the repository. m = p.match(/^\/repos\/novox\/([^/]+)\/branches\/(.+)$/); if (m && req.method === "GET") { @@ -140,10 +148,21 @@ function fakeForge() { return json(res, 404, { message: "branch does not exist" }); } // gitea's reqAdmin on the whole group: reading the rules is administration too. - m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/.*)?$/); + m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/(.*))?$/); if (m) { const admins = isAdmin(me) || teams.some((t) => t.members.has(me) && t.units.permission === "admin"); if (!admins) return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" }); + if (req.method === "PATCH" && m[2] === "main") { + const rule = mainRules.get(m[1]); + if (!rule) return json(res, 404, { message: "branch protection does not exist" }); + Object.assign(rule, await read(req)); + return json(res, 200, rule); + } + if (req.method === "POST" && p.endsWith("/branch_protections")) { + const b = await read(req); + mainRules.set(m[1], b); + return json(res, 201, b); + } return json(res, 201, { rule_name: "main" }); } m = p.match(/^\/repos\/novox\/([^/]+)\/statuses\/([0-9a-f]+)$/); @@ -155,11 +174,11 @@ function fakeForge() { if (req.method === "GET" && p === "/repos/search") return json(res, 200, { data: [] }); return json(res, 404, { message: "no such route in the fake" }); }); - return new Promise<{ url: string; users: typeof users; tokens: typeof tokens; teams: typeof teams; calls: string[]; + return new Promise<{ url: string; users: typeof users; tokens: typeof tokens; teams: typeof teams; calls: string[]; mainRules: typeof mainRules; faults: typeof faults; close(): Promise }>((resolve) => { server.listen(0, "127.0.0.1", () => { const { port } = server.address() as { port: number }; - resolve({ url: `http://127.0.0.1:${port}`, users, tokens, teams, calls, faults, + resolve({ url: `http://127.0.0.1:${port}`, users, tokens, teams, calls, faults, mainRules, close: () => new Promise((r) => server.close(() => r())) }); }); }); @@ -330,3 +349,21 @@ test("a team another process made between the look and the create is found, not const team = forge.teams.find((t) => t.name === "mesh-tools"); assert.ok(team?.members.has(TOOLS_ACCOUNT)); }); + +test("setting the tools' account up puts main's rule right in every repository: no push, only the tools' account merges (issue 948)", async () => { + const env = await environment(); + const from = forge.calls.length; + await fresh(env).listRepos(); + const hq = forge.mainRules.get("hq"); + assert.equal(hq.enable_push, false); + assert.equal(hq.enable_merge_whitelist, true); + assert.deepEqual(hq.merge_whitelist_usernames, [TOOLS_ACCOUNT]); + assert.deepEqual(hq.status_check_contexts, ["mesh/merge-gate"], "a rule found keeps the statuses it requires"); + const catalog = forge.mainRules.get("mesh-catalog"); + assert.ok(catalog, "a repository with no rule for main is given one"); + assert.equal(catalog.rule_name, "main"); + assert.equal(catalog.enable_push, false); + assert.deepEqual(catalog.merge_whitelist_usernames, [TOOLS_ACCOUNT]); + assert.ok(forge.calls.slice(from).filter((c) => /branch_protections/.test(c)).every((c) => c.endsWith(` as ${ADMIN}`)), + "only the administrator's login touches a rule: the tools' account is no repository's administrator"); +}); diff --git a/modules/gitea/tools-account.ts b/modules/gitea/tools-account.ts index 03df8e32..087e49d0 100644 --- a/modules/gitea/tools-account.ts +++ b/modules/gitea/tools-account.ts @@ -41,6 +41,8 @@ export interface ToolsForge { ensureUser(username: string, password: string, email: string, plain?: boolean): Promise; ensureTeamWith(org: string, team: string, units: Record): Promise; addUserToTeam(teamId: number, username: string): Promise; + /** Main's rule in every repository of the organisation: no push, only the tools' account merges (ADR 0398 §2). */ + ensureMainMerges?(org: string): Promise; } /** Make the tools' account, plain, a member of its team on every repository of the organisation. Its password is @@ -49,4 +51,7 @@ export async function setUpToolsAccount(forge: ToolsForge, account = TOOLS_ACCOU await forge.ensureUser(account, password(), `${account}@noreply.localhost`, true); const team = await forge.ensureTeamWith(TOOLS_ORG, TOOLS_TEAM, { name: TOOLS_TEAM, ...TOOLS_TEAM_UNITS }); await forge.addUserToTeam(team, account); + // Main takes no push and only this account merges into it (novox/hq ADR 0398 §2, issue 948): put right in every + // repository now, by the administrator's login, which the account itself is not. + await forge.ensureMainMerges?.(TOOLS_ORG); } diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index 27a8c610..6914c412 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -32,6 +32,7 @@ export function summarised(p: BranchProtection) { required_statuses: p.enable_status_check ? (p.status_check_contexts ?? []) : [], required_approvals: p.required_approvals ?? 0, admin_may_override: !(p.block_admin_merge_override ?? false), + merge_whitelist: p.enable_merge_whitelist ? (p.merge_whitelist_usernames ?? []) : null, }; } @@ -411,7 +412,7 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }, { name: "gitea_branch_protection_set", - description: "Only ever stronger (novox/hq ADR 0266): a change that drops a required status, lets a person push directly or lets an administrator merge past a status is refused — that is the operator's, at the forge. Make a branch require commit statuses before a pull request merges into it — the mesh's merge check sets `mesh/merge-gate` (the module graph's gate) and `mesh/repo-check` (the repository's own tests). Edits the branch's rule (its required statuses replaced by these, everything else kept unless given) or creates one, which refuses direct pushes unless push=true. Answers the rule before and after.", + description: "Only ever stronger (novox/hq ADR 0266): a change that drops a required status, lets a person push directly or lets an administrator merge past a status is refused, and on main so is a change that drops the merge whitelist (only mesh-tools merges there) or widens it (novox/hq ADR 0398 §2, issue 948) — that is the operator's, at the forge. Make a branch require commit statuses before a pull request merges into it — the mesh's merge check sets `mesh/merge-gate` (the module graph's gate) and `mesh/repo-check` (the repository's own tests). Edits the branch's rule (its required statuses replaced by these, everything else kept unless given) or creates one, which refuses direct pushes unless push=true. Answers the rule before and after.", input: { owner: { type: "string", description: "the repository owner" }, repo: { type: "string", description: "the repository name" }, @@ -433,7 +434,7 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }; const covering = before ? [] : (await gitea.branchProtections(owner, repo)) .filter((r) => ruleCovers(String(r.rule_name ?? r.branch_name ?? ""), branch)); - const weakens = judgeProtectionChange(before, want, covering); + const weakens = judgeProtectionChange(before, want, covering, branch); if (weakens) throw new Refused(weakens); const { created, rule } = await gitea.setBranchProtection(owner, repo, branch, want); return { branch, created, before: before ? summarised(before) : null, after: summarised(rule) }; From 789aa0133ffe7cdfea3d4bc7c82eaa9a88f9deee Mon Sep 17 00:00:00 2001 From: fix-forge-awg Date: Fri, 16 Oct 2026 21:28:49 +0200 Subject: [PATCH 4/8] Keep the guard's push wording for main (issue 948) --- modules/gitea/guard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/gitea/guard.ts b/modules/gitea/guard.ts index fb7a31ee..1843e38c 100644 --- a/modules/gitea/guard.ts +++ b/modules/gitea/guard.ts @@ -323,7 +323,7 @@ export function judgeProtectionChange(before: BranchProtection | null, want: Pro * that enables push, drops the merge whitelist or widens it past that account is refused, whatever the rule was. */ function judgeMain(before: BranchProtection | null, want: ProtectionWanted): string | null { const why: string[] = []; - if (want.push === true) why.push("it lets a person push to main directly"); + if (want.push === true) why.push("it lets a person push to the branch directly"); if (want.mergeWhitelist !== undefined) { const names = want.mergeWhitelist ?? []; if (names.length === 0) why.push("it drops the merge whitelist of main"); From f61e00aeaa331e0c411f50c33331dc2662a1c534 Mon Sep 17 00:00:00 2001 From: "fix-deliv-awg@novox" Date: Fri, 16 Oct 2026 21:29:55 +0200 Subject: [PATCH 5/8] =?UTF-8?q?Ask=20the=20operator=20to=20Approve=20a=20m?= =?UTF-8?q?erge=20into=20main=20and=20merge=20only=20on=20the=20warrant=20?= =?UTF-8?q?(issue=20948,=20ADR=200398=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery/approval.go | 408 ++++++++++++++++++ .../cmd/mesh-delivery/approval_test.go | 202 +++++++++ .../cmd/mesh-delivery/fakes_test.go | 5 + .../mesh-delivery/cmd/mesh-delivery/holder.go | 7 +- .../mesh-delivery/cmd/mesh-delivery/main.go | 2 + .../cmd/mesh-delivery/manifest_test.go | 8 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 2 + .../cmd/mesh-delivery/operator_test.go | 104 +++++ .../mesh-delivery/cmd/mesh-delivery/ready.go | 62 ++- modules/mesh-delivery/go.mod | 2 +- modules/mesh-delivery/go.sum | 4 +- modules/mesh-delivery/module.json | 9 +- 12 files changed, 806 insertions(+), 9 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/approval.go create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/approval_test.go create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/operator_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval.go b/modules/mesh-delivery/cmd/mesh-delivery/approval.go new file mode 100644 index 00000000..7989ddd3 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval.go @@ -0,0 +1,408 @@ +package main + +import ( + "fmt" + "regexp" + "sort" + "strconv" + "strings" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// A merge into main waits for the operator's Approve (novox/hq ADR 0398 §3, issue 948). +// +// Where a ready mark at the head, a passing check and the group's turn used to call the merge tool at once, this +// owner now asks the operator on the router (the operator channel, as the knowledge base asks for a record): Approve +// or Decline, each option binding the act "merge these heads". It merges only after an Approve whose warrant holds for +// that ask — the same asker, the same ask's digest, an option it offered, in time, and the act that option bound. +// +// - Asked once per head. A group is asked once, for all its members' heads together, by the first member that reaches +// this point; the Approve covers every member at those heads. +// - A push after the ask cancels it (the mark is cleared with it): the new head needs a new mark and a new Approve. +// - A Decline merges nothing, is said once on the pull request, and clears the mark. +// - An ask that ended without a choice (expired, cancelled, refused by the router) is asked again at the next tick. +// - The ask, its state and the warrant's words are kept on the delivery's ReadyRun and shown by `show`. +// +// The terminal Approve (ADR 0398 §3) is issue 953; today the proving channel is the operator's phone. + +// Module is this module's name: the one the bus grants its asks under. +const Module = "mesh-delivery" + +// The two answers, both at the level approve. +const ( + OptionApprove = "approve" + OptionDecline = "decline" +) + +// AskLasts is how long the operator has to answer: a day less a margin, under the router's bound for an approval. +const AskLasts = asks.ApproveLasts - 10*time.Minute + +// Operator is the operator channel as this owner reaches it: asker.Client in the running module. +type Operator interface { + // Ask checks, keeps and publishes an ask under this module's name. + Ask(a asks.Ask) error + // Cancel takes an ask back. + Cancel(id string) error + // Where is an ask's state as the router keeps it, and its warrant once there is one. + Where(id string) (string, *asks.Warrant, error) +} + +// headsString is the heads an act names, as an act's value: "=" for each, sorted, joined by commas. +func headsString(heads map[string]string) string { + var out []string + for id, sha := range heads { + out = append(out, id+"="+sha) + } + sort.Strings(out) + return strings.Join(out, ",") +} + +// mergeAct is what an answer binds: this ask, the answer, and the heads it covers. +func mergeAct(ask, option string, heads map[string]string) asks.Act { + return asks.Act{"verb": Module + ".merge", "ask": ask, "option": option, "heads": headsString(heads)} +} + +var unshownPath = regexp.MustCompile(`[/\\@~$]|\d+\.\d+\.\d+|[A-Za-z0-9+/=_]{24,}|(?i)\b[a-z0-9-]+\.[a-z]{2,12}\b`) + +// phone is a line the operator's phone may be shown: what the router refuses in an ask's words (a path, an address, a +// host-like name, a long unbroken word) is left out, and the line is cut to n characters. +func phone(s string, n int) string { + s = strings.Join(strings.Fields(unshownPath.ReplaceAllString(s, "…")), " ") + if r := []rune(s); len(r) > n { + return string(r[:n-1]) + "…" + } + return s +} + +var issueNamed = regexp.MustCompile(`(?i)\bissues?\s*#?(\d+)`) + +// IssuesIn reads the issues a pull request's title and body name ("issue 948"), ascending, once each. +func IssuesIn(text string) []int { + seen := map[int]bool{} + var out []int + for _, m := range issueNamed.FindAllStringSubmatch(text, -1) { + if n, err := strconv.Atoi(m[1]); err == nil && !seen[n] { + seen[n] = true + out = append(out, n) + } + } + sort.Ints(out) + return out +} + +func issuesSaid(ds []*Delivery) string { + seen := map[int]bool{} + var out []string + for _, d := range ds { + for _, n := range d.Issues { + if !seen[n] { + seen[n] = true + out = append(out, "issue "+strconv.Itoa(n)) + } + } + } + if len(out) == 0 { + return "none named" + } + return strings.Join(out, ", ") +} + +func repoName(repository string) string { + _, name, ok := strings.Cut(repository, "/") + if !ok { + return repository + } + return name +} + +func pullSaid(d *Delivery) string { return fmt.Sprintf("%s#%d", repoName(d.Repository), d.Number) } + +// composeMergeAsk is the ask for a delivery, or for a group's members together (in the order they merge). +func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, heads map[string]string, now time.Time) asks.Ask { + var a asks.Ask + a.ID, a.Who, a.Expires = id, asks.Operator, now.Add(AskLasts) + a.OnExpiry = "nothing is merged; the ask is made again" + a.About = "delivery." + phone(repoName(d.Repository), 40) + "." + strconv.Itoa(d.Number) + a.About = strings.ReplaceAll(a.About, " ", "-") + var reviewers, reaches, details []string + for _, m := range members { + by, at := "a reviewer", "" + if m.Ready != nil { + if m.Ready.By != "" { + by = m.Ready.By + } + at = m.Ready.MarkedAt + } + reviewers = append(reviewers, fmt.Sprintf("%s marked %s ready at %s", by, pullSaid(m), shortOf(m.Commit, 12))) + plan := "no delivery plan yet" + if m.Plan != nil && m.Plan.Summary != "" { + plan = m.Plan.Summary + } + reaches = append(reaches, plan) + details = append(details, fmt.Sprintf("%s %q at %s: issues: %s; reviewer: %s, marked %s; reaches: %s", + pullSaid(m), m.Title, shortOf(m.Commit, 12), issuesSaid([]*Delivery{m}), by, at, plan)) + } + if group == "" { + a.Headline = phone("Merge into main: "+pullSaid(d)+" "+d.Title, asks.HeadlineLength) + a.Explanation = strings.Join([]string{ + phone(fmt.Sprintf("%s %s passed its checks and was reviewed. Approve merges it into main at %s; Decline leaves it unmerged.", + pullSaid(d), d.Title, shortOf(d.Commit, 12)), asks.SummaryLength), + phone("Issues: "+issuesSaid(members), asks.PointLength), + phone("Reviewer: "+reviewers[0], asks.PointLength), + phone("Reaches: "+reaches[0], asks.PointLength), + }, "\n") + } else { + var names []string + for _, m := range members { + names = append(names, pullSaid(m)) + } + a.Headline = phone("Merge into main: group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength) + a.Explanation = strings.Join([]string{ + phone(fmt.Sprintf("The group %s, %d pull requests, passed its checks and was reviewed. Approve merges all of them into main in their order, at the heads named; Decline leaves them unmerged.", + group, len(members)), asks.SummaryLength), + phone("Members, in order: "+strings.Join(names, ", "), asks.PointLength), + phone("Issues: "+issuesSaid(members), asks.PointLength), + phone("Reaches: "+strings.Join(reaches, "; "), asks.PointLength), + }, "\n") + } + a.Details = phone(strings.Join(details, "\n"), 1500) + if len(details) > 0 { // keep the lines apart: phone joined them + a.Details = strings.Join(func() []string { + var out []string + for _, l := range details { + out = append(out, phone(l, 400)) + } + return out + }(), "\n") + } + for _, o := range []struct{ id, label, does string }{ + {OptionApprove, "Approve", "the pull request is merged into main at the head named (a group: all its members, in order)"}, + {OptionDecline, "Decline", "nothing is merged and the ready mark is taken off"}, + } { + binds, _ := asks.ActDigest(mergeAct(id, o.id, heads)) + a.Options = append(a.Options, asks.Option{ID: o.id, Label: o.label, Does: o.does, Level: asks.Approve, Binds: binds}) + } + return a +} + +// askMembers are the deliveries one ask covers: the group's members not yet merged, in the group's order, or the delivery. +func (h *Holder) askMembers(d *Delivery) []*Delivery { + g := h.groups[d.Group] + if d.Group == "" || g == nil { + return []*Delivery{d} + } + var out []*Delivery + seen := map[string]bool{} + for _, id := range append(append([]string(nil), g.Order...), g.Members...) { + if o := h.deliveries[id]; o != nil && !seen[id] && o.MergedAs == "" && !o.State.Final() { + seen[id] = true + out = append(out, o) + } + } + if len(out) == 0 { + return []*Delivery{d} + } + return out +} + +// covering is the ready run holding an ask (pending or approved) that covers d at its head: its own, or a group +// sibling's, whose ask named d's head too. +func (h *Holder) covering(d *Delivery) *ReadyRun { + has := func(o *Delivery) *ReadyRun { + if o != nil && o.Ready != nil && (o.Ready.AskID != "" || o.Ready.Approved != nil) && o.Ready.Heads[d.ID] == d.Commit { + return o.Ready + } + return nil + } + if r := has(d); r != nil { + return r + } + if g := h.groups[d.Group]; d.Group != "" && g != nil { + for _, id := range g.Members { + if r := has(h.deliveries[id]); r != nil { + return r + } + } + } + return nil +} + +// approval is what a delivery that may merge waits on for the operator: empty once its head is approved, and the ask +// to make when none is made yet. Under the lock. +func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { + r := d.Ready + if c := h.covering(d); c != nil { + if c.Approved != nil { + r.Approved, r.Warrant, r.AskID, r.Ask, r.Heads = c.Approved, c.Warrant, c.AskID, c.Ask, c.Heads + return "", nil + } + return "the operator's Approve is asked (" + c.AskID + ")", nil + } + if r.Refused != "" { + return "the operator's answer was refused here: " + r.Refused, nil + } + if h.Operator == nil { + return "the operator cannot be asked from here", nil + } + members := h.askMembers(d) + group := "" + if len(members) > 1 { + group = d.Group + } + heads := map[string]string{} + for _, m := range members { + heads[m.ID] = m.Commit + } + r.Asks++ + id := fmt.Sprintf("merge-%d-%s-%d", d.Number, shortOf(d.Commit, 12), r.Asks) + ask := composeMergeAsk(id, d, group, members, heads, now) + taken := []*Delivery{} + for _, m := range members { + if m.Ready == nil { + continue + } + 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) + h.keep(m) + taken = append(taken, m) + } + h.Logf("[mesh-delivery] %s is marked ready and its checks passed: the operator is asked to Approve its merge into main (%s, ask %d)", + d.ID, id, r.Asks) + return "the operator's Approve is asked (" + id + ")", func() { + if err := h.Operator.Ask(ask); err != nil { + h.mu.Lock() + defer h.mu.Unlock() + for _, m := range taken { + if m.Ready != nil && m.Ready.AskID == id { + m.Ready.AskID, m.Ready.Ask, m.Ready.Heads = "", nil, nil + h.keep(m) + } + } + h.Logf("[mesh-delivery] the operator could not be asked about %s (%v); asked again at the next tick", d.ID, err) + } + } +} + +// dropAsk forgets a pending ask on every delivery it covers and takes it back from the router; under the lock. +func (h *Holder) dropAsk(id, why string) func() { + for _, d := range h.deliveries { + if d.Ready != nil && d.Ready.AskID == id { + d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + h.keep(d) + } + } + h.Logf("[mesh-delivery] the operator's ask %s is cancelled: %s", id, why) + return func() { + if h.Operator == nil { + return + } + if err := h.Operator.Cancel(id); err != nil { + h.Logf("[mesh-delivery] the operator's ask %s could not be cancelled (%v)", id, err) + } + } +} + +// pendingAsks are the asks waiting for an answer, by id; under the lock. +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] { + seen[r.AskID] = true + out = append(out, r.AskID) + } + } + sort.Strings(out) + return out +} + +// hearAsks reads the router's record of every pending ask and applies the warrant of each that closed. +func (h *Holder) hearAsks() { + if h.Operator == nil { + return + } + h.mu.Lock() + ids := h.pendingAsks() + h.mu.Unlock() + for _, id := range ids { + state, warrant, err := h.Operator.Where(id) + if err != nil { + h.Logf("[mesh-delivery] the router's record of the ask %s cannot be read: %v", id, err) + continue + } + if state == "" || state == "open" || state == "held" { + continue + } + h.mu.Lock() + said := h.decided(id, state, warrant) + h.mu.Unlock() + for _, s := range said { + s() + } + } +} + +// decided applies one closed ask's warrant to the deliveries it covers; under the lock. It returns what to say on the +// pull requests. +func (h *Holder) decided(id, state string, w *asks.Warrant) []func() { + var covered []*Delivery + for _, d := range h.deliveries { + if d.Ready != nil && d.Ready.AskID == id { + covered = append(covered, d) + } + } + if len(covered) == 0 { + return nil + } + sort.Slice(covered, func(i, j int) bool { return covered[i].ID < covered[j].ID }) + first, now := covered[0].Ready, h.Now() + end := func(why string) { + for _, d := range covered { + d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + h.keep(d) + } + h.Logf("[mesh-delivery] the operator's ask %s ended without a choice (%s); asked again at the next tick", id, why) + } + if w == nil || w.Outcome != asks.OutcomeChosen { + why := "the router says " + state + if w != nil { + why = "ended " + string(w.Outcome) + if w.Words != "" { + why += " (" + w.Words + ")" + } + } + end(why) + return nil + } + option, err := w.For(Module, *first.Ask) + if err == nil { + err = option.Performs(mergeAct(id, option.ID, first.Heads)) + } + if err != nil { + h.Logf("[mesh-delivery] REFUSED the answer to the ask %s: %v", id, err) + for _, d := range covered { + d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + d.Ready.Refused = "was answered, and the answer does not hold here: " + clip(oneLine(err.Error()), 300) + h.keep(d) + } + return nil + } + var said []func() + for _, d := range covered { + r := d.Ready + r.Warrant = w.Says() + switch option.ID { + case OptionApprove: + r.Approved = &now + h.Logf("[mesh-delivery] %s: the operator approved its merge into main (%s)", d.ID, r.Warrant) + case OptionDecline: + r.Declined = &now + 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. The ready mark is taken off; a new mark, once reviewed, asks again.", d.Commit, r.Warrant))) + } + h.keep(d) + } + return said +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go new file mode 100644 index 00000000..33926550 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go @@ -0,0 +1,202 @@ +package main + +import ( + "strings" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// **A merge into main waits for the operator's Approve** (novox/hq ADR 0398 §3, issue 948): a marked, green delivery +// into main is asked of the operator, and merged only after an Approve whose warrant holds for that ask. + +// markedGreen is a pull request into main, announced, judged green and marked ready at its head; the operator is not +// answering by himself (w.op.auto is off). +func markedGreen(t *testing.T) *world { + w := newWorld(t) + w.op.auto = false + p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go") + p.Title = "Fix the apostrophe (issue 948, issue 12)" + w.h.PullUpdated(p) + w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + w.forge.open = []OpenPull{markedPull("novox/app", 7, readyHead, readyHead)} + w.h.SeeOpenPulls() + w.settleAll() + return w +} + +func TestAMarkedGreenDeliveryIntoMainIsAskedAndNotMergedUntilApproved(t *testing.T) { + w := markedGreen(t) + id := IDOf("novox/app", readyHead) + if len(w.forge.merges) != 0 { + t.Fatalf("merged before the operator's Approve: %v", w.forge.merges) + } + open := w.op.open() + if len(open) != 1 { + t.Fatalf("the operator is asked %v; wanted exactly one ask", open) + } + a := w.op.ask(open[0]) + if !strings.HasPrefix(a.Headline, "Merge into main: app#7") || a.Who != asks.Operator || + len(a.Options) != 2 || a.Options[0].ID != OptionApprove || a.Options[1].ID != OptionDecline { + t.Fatalf("the ask is not the merge ask: %+v", a) + } + for _, want := range []string{"issue 948", "issue 12", "reviewer", "builds app"} { + if !strings.Contains(a.Explanation, want) { + t.Fatalf("the ask does not say %q: %s", want, a.Explanation) + } + } + if err := asks.Glance(a.Explanation); err != nil { + t.Fatalf("the ask does not read at a glance: %v", err) + } + // Ticks and minutes pass without an answer: not merged, not asked again. + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + if len(w.forge.merges) != 0 || w.op.count() != 1 { + t.Fatalf("while waiting: merges %v, asks %d", w.forge.merges, w.op.count()) + } + if d := w.delivery(id); d.Ready.AskID != open[0] || !strings.Contains(d.Ready.Waits, "Approve") { + t.Fatalf("the ask is not kept on the delivery: %+v", d.Ready) + } + w.op.answer(open[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 1 || w.forge.merges[0] != "novox/app#7@"+readyHead { + t.Fatalf("after the Approve it was merged %v; wanted once, at the marked head", w.forge.merges) + } + d := w.delivery(id) + if d.Ready.Approved == nil || !strings.Contains(d.Ready.Warrant, "chose Approve") { + t.Fatalf("the warrant is not kept: %+v", d.Ready) + } + w.h.PullMerged(merged("novox/app", 7, readyHead, "m7m7m7m7m7m7")) + w.h.WalkMoved(w.ctl.put(aWalk("plan-7", "novox/app", "m7m7m7m7m7m7", true))) + w.settleAll() + var last Transition + for _, tr := range w.delivery(id).Transitions { + if tr.Event == EvMerged { + last = tr + } + } + if !strings.Contains(last.By, "approved: the operator, via telegram") { + t.Fatalf("the merge's transition does not name the operator's Approve: %+v", last) + } +} + +func TestAnAnswerThatIsNotForThisAskMergesNothing(t *testing.T) { + w := markedGreen(t) + id := w.op.open()[0] + w.op.answer(id, OptionApprove) + w.op.mu.Lock() + w.op.warrant[id].AskDigest = "sha256:not-the-ask-that-was-shown" + w.op.mu.Unlock() + w.settleAll() + if len(w.forge.merges) != 0 { + t.Fatalf("a warrant for another ask merged: %v", w.forge.merges) + } + d := w.delivery(IDOf("novox/app", readyHead)) + if !strings.Contains(d.Ready.Refused, "digest") || d.Ready.Approved != nil { + t.Fatalf("the refused answer is not kept: %+v", d.Ready) + } + if lines := w.h.Stalled(); len(lines) != 1 || !strings.Contains(lines[0].Says, "answer") { + t.Fatalf("a refused answer is not said loudly: %+v", lines) + } +} + +func TestADeclinedMergeIsNotMergedSaidOnceAndItsMarkTakenOff(t *testing.T) { + w := markedGreen(t) + w.op.answer(w.op.open()[0], OptionDecline) + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + if len(w.forge.merges) != 0 || w.op.count() != 1 { + t.Fatalf("declined, yet merges %v and asks %d", w.forge.merges, w.op.count()) + } + if len(w.forge.comments) != 1 || !strings.Contains(w.forge.comments[0], "declined") || + !strings.Contains(w.forge.comments[0], "chose Decline") { + t.Fatalf("the Decline is not said once on the pull request, with its warrant: %v", w.forge.comments) + } + if len(w.forge.unlabels) == 0 || w.forge.unlabels[0] != "novox/app#7-31" { + t.Fatalf("the mark was not taken off: %v", w.forge.unlabels) + } +} + +func TestAPushAfterTheAskCancelsItAndTheNewHeadNeedsANewMarkAndApprove(t *testing.T) { + w := markedGreen(t) + old := w.op.open()[0] + w.h.PullUpdated(pr("novox/app", 7, pushed, "feat/x", "src/a.go")) + w.h.Checked(judgedAt("novox/app", 7, pushed, "pass", "t1t1t1t1t1t1")) + w.forge.open = []OpenPull{markedPull("novox/app", 7, pushed, readyHead)} // the old mark, on the new head + w.h.SeeOpenPulls() + w.settleAll() + if len(w.op.cancelled) != 1 || w.op.cancelled[0] != old || len(w.op.open()) != 0 { + t.Fatalf("the ask was not cancelled: cancelled %v, open %v", w.op.cancelled, w.op.open()) + } + if len(w.forge.merges) != 0 || w.op.count() != 1 { + t.Fatalf("merges %v, asks %d", w.forge.merges, w.op.count()) + } + if d := w.delivery(IDOf("novox/app", readyHead)); d.Ready.AskID != "" { + t.Fatalf("the cancelled ask is still kept as waiting: %+v", d.Ready) + } + // The new head, marked by its reviewer, is asked afresh. + w.forge.open = []OpenPull{markedPull("novox/app", 7, pushed, pushed)} + w.h.SeeOpenPulls() + w.settleAll() + if open := w.op.open(); len(open) != 1 || open[0] == old { + t.Fatalf("the new head was not asked afresh: %v", open) + } +} + +func TestAnExpiredAskIsAskedAgainOnce(t *testing.T) { + w := markedGreen(t) + first := w.op.open()[0] + w.op.end(first) + w.settleAll() + if w.op.count() != 2 || len(w.op.open()) != 1 || w.op.open()[0] == first { + t.Fatalf("an expired ask was not asked again once: %d asks, open %v", w.op.count(), w.op.open()) + } + for range 3 { + w.later(2 * time.Minute) + w.settleAll() + } + if w.op.count() != 2 || len(w.forge.merges) != 0 { + t.Fatalf("asked more than once again: %d asks, merges %v", w.op.count(), w.forge.merges) + } +} + +func TestAGroupIsAskedOnceForAllItsHeadsAndMergedInOrderOnOneApprove(t *testing.T) { + w := newWorld(t) + w.op.auto = false + const ctlHead, catHead = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2" + ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) + w.ctl.order = func(ms []Member) Order { return Order{Order: []string{ctl, cat}} } + w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) + w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) + w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) + w.h.Checked(judgedAt("novox/mesh-catalog", 2, catHead, "pass", "t3t3t3t3t3t3")) + w.forge.open = []OpenPull{markedPull("novox/mesh-controller", 1, ctlHead, ctlHead), + markedPull("novox/mesh-catalog", 2, catHead, catHead)} + w.h.SeeOpenPulls() + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/x", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.settleAll() + if open := w.op.open(); len(open) != 1 || len(w.forge.merges) != 0 { + t.Fatalf("a group is asked once before any merge: open %v, merges %v", w.op.open(), w.forge.merges) + } + a := w.op.ask(w.op.open()[0]) + if !strings.Contains(a.Headline, "group feat") || !strings.Contains(a.Details, "mesh-controller#1") || + !strings.Contains(a.Details, "mesh-catalog#2") { + t.Fatalf("the ask does not name the group and its members: %+v", a) + } + w.op.answer(a.ID, OptionApprove) + w.settleAll() + if len(w.forge.merges) != 1 || w.forge.merges[0] != "novox/mesh-controller#1@"+ctlHead { + t.Fatalf("the first member was not merged alone on the Approve: %v", w.forge.merges) + } + w.h.PullMerged(merged("novox/mesh-controller", 1, ctlHead, "m1m1m1m1m1m1")) + w.settleAll() + if len(w.forge.merges) != 2 || w.forge.merges[1] != "novox/mesh-catalog#2@"+catHead || w.op.count() != 1 { + t.Fatalf("the next member was not merged on the same Approve: merges %v, asks %d", w.forge.merges, w.op.count()) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index c035e450..effcd900 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -561,6 +561,7 @@ type world struct { store *memStore ctl *fakeController forge *fakeForge + op *fakeOperator now time.Time said []map[string]any mu sync.Mutex @@ -584,6 +585,10 @@ func newWorldOver(t *testing.T, store *memStore, ctl *fakeController, forge *fak w.said = append(w.said, m) return nil }} + // The operator approves at once unless a test says otherwise (w.op.auto = false): most tests are not about the Approve. + w.op = newFakeOperator(w.h.Now) + w.op.auto = true + w.h.Operator = w.op w.h.init() if err := w.h.Load(); err != nil { t.Fatal(err) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 213e3b99..2d40a824 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -27,6 +27,8 @@ type Holder struct { Emit func(event string, body any) error Now func() time.Time Logf func(format string, a ...any) + // Operator is the operator channel, asked for the Approve of a merge into main (approval.go). + Operator Operator mu sync.Mutex deliveries map[string]*Delivery @@ -381,7 +383,7 @@ func (h *Holder) freshAnnouncement(p PullEvent, now time.Time, startedOver strin d := &Delivery{ID: IDOf(p.repository(), p.HeadSHA), Repository: p.repository(), Commit: p.HeadSHA, Number: p.Number, Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, - After: AfterIn(p.Body), Created: now, BaseSHA: p.BaseSHA} + After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, BaseSHA: p.BaseSHA} why := fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number) if startedOver != "" { why = fmt.Sprintf("%s; %s", why, startedOver) @@ -551,7 +553,7 @@ func (h *Holder) PullMerged(p PullEvent) { d = &Delivery{ID: IDOf(p.repository(), head), Repository: p.repository(), Commit: head, Number: p.Number, Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, - After: AfterIn(p.Body), Created: now, FromForge: true} + After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, FromForge: true} // Its check, as the forge holds it on the head: the verdict the pull request merged with. if s := p.HeadChecks["mesh/merge-gate"]; s != "" { d.Check = &Verdict{At: now, Gate: verdictOfState(s), Summary: "the head's mesh/merge-gate, as the forge holds it", @@ -1060,6 +1062,7 @@ func (h *Holder) Tick() { for _, ask := range h.leaveStopped() { ask() } + h.hearAsks() h.mu.Lock() asks := h.readyAsks(h.Now()) h.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 724b6b26..52fa7fa0 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -16,6 +16,7 @@ import ( "time" stdio "git.novox.be/novox/mesh-sdk/go" + "git.novox.be/novox/mesh-sdk/go/asker" ) func logf(format string, a ...any) { fmt.Fprintf(os.Stderr, format+"\n", a...) } @@ -40,6 +41,7 @@ func main() { Forge: toolForge{ask: stdio.Ask}, Emit: func(event string, body any) error { return stdio.Emit(event, body) }, Logf: logf, + Operator: &asker.Client{Module: Module, State: stdio.State("asked")}, } h.init() l := &listening{now: "not yet: starting"} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index ade98c14..1915e3b2 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -21,6 +21,7 @@ type manifest struct { Serves []string `json:"serves"` } `json:"claims"` Seats []any `json:"seats"` + Uses []string `json:"uses"` Consumes []string `json:"consumes"` Emits []string `json:"emits"` Invokes []string `json:"invokes"` @@ -56,7 +57,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { if !reflect.DeepEqual(m.Emits, []string{"transition", "group"}) { t.Fatalf("emits %v", m.Emits) } - if !reflect.DeepEqual(m.State, []string{"deliveries", "groups"}) { + if !reflect.DeepEqual(m.State, []string{"deliveries", "groups", "asked"}) { t.Fatalf("state %v", m.State) } // It calls exactly what its ports ask: the controller's six verbs and the forge's four tools. @@ -68,6 +69,11 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { "gitea.gitea_commit_statuses", "gitea.gitea_contains", "gitea.gitea_open_pulls", // The merge on a ready mark (novox/hq issue 722): the merge tool, a comment, a label taken off. "gitea.gitea_merge_pull_request", "gitea.gitea_add_comment", "gitea.gitea_api") + // A merge into main asks the operator for the Approve (novox/hq ADR 0398 §3, issue 948), as the knowledge base asks. + want = append(want, "seat:operator-channel.notify") + if !reflect.DeepEqual(m.Uses, []string{"operator-channel"}) { + t.Fatalf("uses %v", m.Uses) + } if !reflect.DeepEqual(m.Invokes, want) { t.Fatalf("invokes %v, wanted %v", m.Invokes, want) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 432e079d..d4b4f68e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -35,6 +35,8 @@ type Delivery struct { ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` // After are the repositories its pull request says it goes after (`after: ` lines). After []string `json:"after,omitempty"` + // Issues are the issues its title and body name ("issue 948"), shown to the operator who is asked to approve its merge. + Issues []int `json:"issues,omitempty"` // Group is the delivery group it is in: its head branch name. Empty for a lone delivery. Group string `json:"group,omitempty"` diff --git a/modules/mesh-delivery/cmd/mesh-delivery/operator_test.go b/modules/mesh-delivery/cmd/mesh-delivery/operator_test.go new file mode 100644 index 00000000..6c312b21 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/operator_test.go @@ -0,0 +1,104 @@ +package main + +import ( + "sync" + "time" + + "git.novox.be/novox/mesh-sdk/go/asks" +) + +// fakeOperator is the operator channel: it keeps the asks made, and answers as the test says. In auto mode it +// approves every ask the moment it is read (the tests of other behaviour, which are not about the Approve). +type fakeOperator struct { + mu sync.Mutex + now func() time.Time + auto bool + asked []asks.Ask + cancelled []string + state map[string]string + warrant map[string]*asks.Warrant + refuse error +} + +func newFakeOperator(now func() time.Time) *fakeOperator { + return &fakeOperator{now: now, state: map[string]string{}, warrant: map[string]*asks.Warrant{}} +} + +func (o *fakeOperator) Ask(a asks.Ask) error { + o.mu.Lock() + defer o.mu.Unlock() + if o.refuse != nil { + return o.refuse + } + if err := a.Check(o.now()); err != nil { + return err + } + o.asked = append(o.asked, a) + o.state[a.ID] = "open" + return nil +} + +func (o *fakeOperator) Cancel(id string) error { + o.mu.Lock() + defer o.mu.Unlock() + o.cancelled = append(o.cancelled, id) + o.state[id] = "cancelled" + return nil +} + +func (o *fakeOperator) Where(id string) (string, *asks.Warrant, error) { + o.mu.Lock() + defer o.mu.Unlock() + if o.auto && o.state[id] == "open" { + o.answerLocked(id, OptionApprove) + } + return o.state[id], o.warrant[id], nil +} + +func (o *fakeOperator) ask(id string) asks.Ask { + for _, a := range o.asked { + if a.ID == id { + return a + } + } + panic("no ask " + id) +} + +func (o *fakeOperator) answerLocked(id, option string) { + a := o.ask(id) + opt, _ := a.Option(option) + o.state[id] = "answered" + o.warrant[id] = &asks.Warrant{Ask: id, Asker: Module, Outcome: asks.OutcomeChosen, Option: option, Label: opt.Label, + Level: opt.Level, By: &asks.Person{Who: asks.Operator, Kind: "telegram", Verified: "user id verified"}, + At: o.now(), AskDigest: a.Digest()} +} + +// answer is the operator's choice, proven by the router. +func (o *fakeOperator) answer(id, option string) { + o.mu.Lock() + defer o.mu.Unlock() + o.answerLocked(id, option) +} + +// end is an ask that closed without a choice. +func (o *fakeOperator) end(id string) { + o.mu.Lock() + defer o.mu.Unlock() + o.state[id] = "expired" + o.warrant[id] = &asks.Warrant{Ask: id, Asker: Module, Outcome: asks.OutcomeExpired, At: o.now()} +} + +// open is the ids of the asks waiting for an answer, in the order asked. +func (o *fakeOperator) open() []string { + o.mu.Lock() + defer o.mu.Unlock() + var out []string + for _, a := range o.asked { + if o.state[a.ID] == "open" { + out = append(out, a.ID) + } + } + return out +} + +func (o *fakeOperator) count() int { o.mu.Lock(); defer o.mu.Unlock(); return len(o.asked) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index fa36708a..ca66438b 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" "time" + + "git.novox.be/novox/mesh-sdk/go/asks" ) // Merge on ready (novox/hq issue 722). @@ -72,6 +74,21 @@ type ReadyRun struct { // OtherBase is the branch its pull request merges into when that is not main: never merged here, said once on the // pull request (novox/hq ADR 0398 §4, issue 948). OtherBase string `json:"other_base,omitempty"` + + // The operator's Approve of the merge into main (novox/hq ADR 0398 §3, issue 948, approval.go). AskID is the ask + // waiting for an answer, Ask the ask as made (a warrant is held to it), Heads the heads it covers by delivery id (a + // group's members together), AskedAt when and Asks how many were made for this head. + AskID string `json:"ask_id,omitempty"` + Ask *asks.Ask `json:"ask,omitempty"` + Heads map[string]string `json:"heads,omitempty"` + AskedAt time.Time `json:"asked_at"` + Asks int `json:"asks,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"` } // MergeBranch is the one branch this owner merges into (novox/hq ADR 0398 §4): a merge into any other is its author's. @@ -86,6 +103,9 @@ func (r *ReadyRun) mergedBy() string { if r.By != "" { by += " by " + r.By } + if r.Warrant != "" { + by += "; the merge into main approved: " + r.Warrant + } return by } @@ -156,6 +176,9 @@ func (h *Holder) readyAsks(now time.Time) []func() { continue } m.cleared = now + if old := h.deliveries[IDOf(repository, mark.Head)]; old != nil && old.Ready != nil && old.Ready.AskID != "" { + asks = append(asks, h.dropAsk(old.Ready.AskID, "its head moved on to "+shortOf(p.HeadSHA, 12))) + } asks = append(asks, h.clearMark(k, p, mark)) continue } @@ -163,7 +186,7 @@ func (h *Holder) readyAsks(now time.Time) []func() { if d == nil || d.State.Final() || d.MergedAs != "" { continue // not announced yet, or past its merge } - if d.Ready == nil || d.Ready.Head != mark.Head { + if d.Ready == nil || d.Ready.Head != mark.Head || (d.Ready.Declined != nil && mark.At != d.Ready.MarkedAt) { d.Ready = &ReadyRun{Head: mark.Head, By: mark.By, MarkedAt: mark.At, Seen: now} h.Logf("[mesh-delivery] %s is marked ready at %s by %s: merged once its check passes against its trunk's head", d.ID, shortOf(mark.Head, 12), orNone(mark.By)) @@ -173,6 +196,14 @@ func (h *Holder) readyAsks(now time.Time) []func() { if r.Merged != nil { continue } + if r.Declined != nil { + // The operator declined: nothing is merged, and the mark is taken off (said once when it was declined). + if m.cleared.IsZero() || now.Sub(m.cleared) >= readyTryEvery { + m.cleared = now + asks = append(asks, h.unmark(k, p, mark)) + } + continue + } if d.State == Rejected { if r.Failed == "" { r.Failed = "its check failed" @@ -212,6 +243,17 @@ func (h *Holder) readyAsks(now time.Time) []func() { } h.keep(d) } + var approvalAsk func() + if waits == "" && p.Base == MergeBranch { + waits, approvalAsk = h.approval(d, now) + } + if waits != r.Waits { + r.Waits = waits + h.keep(d) + } + if approvalAsk != nil { + asks = append(asks, approvalAsk) + } if waits != "" || (!r.LastTry.IsZero() && now.Sub(r.LastTry) < readyTryEvery) { continue } @@ -328,6 +370,19 @@ func (h *Holder) clearMark(k string, p OpenPull, mark ReadyMark) func() { } } +// unmark takes the mark off a pull request whose merge the operator declined (already said on it). +func (h *Holder) unmark(k string, p OpenPull, mark ReadyMark) func() { + return func() { + if err := h.Forge.Unlabel(p.Owner, p.Repo, p.Number, mark.LabelID); err != nil { + h.Logf("[mesh-delivery] %s's ready mark could not be taken off after the operator's Decline yet: %v", k, err) + return + } + h.mu.Lock() + delete(h.marks, k) + h.mu.Unlock() + } +} + // clearedSaying is what a cleared mark says on its pull request. A mark with no head is a label no "ready at " // comment of a listed account followed (novox/hq issue 873); one with a head names an older head than the pull's. func clearedSaying(mark ReadyMark, p OpenPull) string { @@ -365,6 +420,11 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { Bound: "0s", H2: "none: a new head marked ready, or a recheck that passes — the operator's", Says: fmt.Sprintf("%s#%d is marked ready at %s and not merged: %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Failed)}) + case r.Refused != "": + out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(), + Bound: "0s", H2: "none: a new head marked ready, which asks the operator again — the operator's", + Says: fmt.Sprintf("%s#%d is marked ready at %s and not merged: the operator's answer %s", d.Repository, + d.Number, shortOf(r.Head, 12), r.Refused)}) case r.RefusedSince != nil && now.Sub(*r.RefusedSince) >= readyRefusedFor: out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: readyRefusedFor.String(), diff --git a/modules/mesh-delivery/go.mod b/modules/mesh-delivery/go.mod index 9ce486e4..8d6748fb 100644 --- a/modules/mesh-delivery/go.mod +++ b/modules/mesh-delivery/go.mod @@ -2,4 +2,4 @@ module mesh-delivery go 1.22 -require git.novox.be/novox/mesh-sdk/go v0.1.7 +require git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44 diff --git a/modules/mesh-delivery/go.sum b/modules/mesh-delivery/go.sum index b474419e..3c290b47 100644 --- a/modules/mesh-delivery/go.sum +++ b/modules/mesh-delivery/go.sum @@ -1,2 +1,2 @@ -git.novox.be/novox/mesh-sdk/go v0.1.7 h1:C0sTQmtTiyYH7bnqZb7PusXnqA37gKuT7Nqjn9gG47w= -git.novox.be/novox/mesh-sdk/go v0.1.7/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44/go.mod h1:GFuZUElBZ9A++mxgIKo97aXXo+kV0uJ/UkbhQPPIbrY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261013182924-5933eb757d44 h1:r5xIo5qWOXQdCSMxvqJy8kV4U+P/QWXFK6971Ha6wU8= diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index b32b6320..f951f918 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -51,11 +51,16 @@ "gitea.gitea_open_pulls", "gitea.gitea_merge_pull_request", "gitea.gitea_add_comment", - "gitea.gitea_api" + "gitea.gitea_api", + "seat:operator-channel.notify" + ], + "uses": [ + "operator-channel" ], "state": [ "deliveries", - "groups" + "groups", + "asked" ], "tools": [ "delivery_status" From 4215b4054737d495fb424621b66b5163133e3a40 Mon Sep 17 00:00:00 2001 From: lead-57 Date: Fri, 16 Oct 2026 21:32:13 +0200 Subject: [PATCH 6/8] Read the whole plan the controller writes: at, from, as, names and credential-tool (issue 954) The client decodes a plan strictly, and since the controller carries these fields (issues 923 and 928) it refused the plans of gitea, keycloak and claude-code. --- .../cmd/mesh-provisioner-client/plan.go | 10 +++++ .../whole_plan_test.go | 39 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 modules/mesh-provisioner-client/cmd/mesh-provisioner-client/whole_plan_test.go diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go index 4e56e034..8caf8968 100644 --- a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/plan.go @@ -97,6 +97,16 @@ type PlannedProvision struct { // Contributes is what this module tells the provider about itself for this provision (a database's name, its // extensions): the consumer manifest's `contributes`, which the provider's helper reads and the client keeps. Contributes map[string]any `json:"contributes,omitempty"` + // At, From and As are where the provider is on the private network, its machine, and who the consumer is at + // its end (issue 788); Names are what the mesh calls the consumer through this provision (issue 792). The + // provider's helper reads them from the plan; the client keeps them, so the plan reads whole (issue 954). + At string `json:"at,omitempty"` + From string `json:"from,omitempty"` + As string `json:"as,omitempty"` + Names map[string]string `json:"names,omitempty"` + // CredentialTool is, for a software-issued offer, the provider's tool the vault asks for the value (issue 928); + // the vault acts on it and this client only reads it. + CredentialTool string `json:"credential-tool,omitempty"` } // issuedForHolder is whether the entry names a value its provider's software issued for this holder alone (issue diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/whole_plan_test.go b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/whole_plan_test.go new file mode 100644 index 00000000..1c2efa79 --- /dev/null +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/whole_plan_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "encoding/json" + "fmt" + "strings" + "testing" +) + +// A plan provision carries everything the controller's PlanProvision writes (issue 954): what the consumer +// contributes, where the provider is and who the consumer is at its end (issue 788), the names the mesh calls it +// (issue 792) and, for a software-issued offer, the tool the vault asks (issue 928). The client still refuses a field +// nobody writes. +func TestAPlanProvisionWithEveryFieldTheControllerWritesIsRead(t *testing.T) { + p, err := json.Marshal(contributor(t, "shell:zsh:normal", "alias nox=mesh-cli")) + if err != nil { + t.Fatal(err) + } + provision := `"provisions":[{"provision":"route","local":"web","provider":"novox","provider-module":"route-proxy",` + + `"credential-form":"pair","contributes":{"label":"git"},"at":"10.10.0.1","from":"novox","as":"gitea",` + + `"names":{"name-web":"git.example.org"}},{"name":"gitea-read","provision":"gitea-read","provider":"novox",` + + `"provider-module":"gitea","credential-form":"issued","credential-tool":"gitea_agent_token",` + + `"path":"/var/lib/claude-code/gitea-read.secret"}],"items":` + value := fmt.Sprintf(`{"node":"g14","module":"mesh-cli","provisioning":%s}`, + strings.Replace(string(p), `"items":`, provision, 1)) + _, got, err := planOf("g14", "g14.mesh-cli", []byte(value)) + if err != nil { + t.Fatalf("a plan with the controller's fields is refused: %v", err) + } + if len(got.Provisions) != 2 || got.Provisions[0].At != "10.10.0.1" || got.Provisions[0].From != "novox" || + got.Provisions[0].As != "gitea" || got.Provisions[0].Names["name-web"] != "git.example.org" || + got.Provisions[1].CredentialTool != "gitea_agent_token" { + t.Fatalf("the plan's provisions read as %+v", got.Provisions) + } + spoilt := strings.Replace(value, `"at":`, `"surprise":1,"at":`, 1) + if _, _, err := planOf("g14", "g14.mesh-cli", []byte(spoilt)); err == nil { + t.Fatalf("a provision with a field nobody writes is read: %s", spoilt) + } +} From 3cdea45f9072befa5971978e87d8dcbc123bd824 Mon Sep 17 00:00:00 2001 From: fix-forge-awg Date: Fri, 16 Oct 2026 21:35:51 +0200 Subject: [PATCH 7/8] Keep main's rule right at every start and copy a covering glob's checks (issue 948, review round 1) --- modules/gitea/client.ts | 33 ++++++++++++++---------- modules/gitea/index.ts | 8 ++++++ modules/gitea/test/tools-account.test.ts | 30 ++++++++++++++++++++- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index e0b80364..536420b8 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -6,7 +6,7 @@ import { readFileSync } from "node:fs"; import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js"; import { MAIN_BRANCH, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; -import { Refused, checkSentPath, pathSegment, pathSegments } from "./guard.js"; +import { Refused, checkSentPath, ruleCovers, pathSegment, pathSegments } from "./guard.js"; import { isCommit } from "./checks.js"; import { TOOLS_ACCOUNT } from "./tools-account.js"; @@ -1005,21 +1005,26 @@ export class GiteaAdmin { if (list.length < 50) break; } for (const repo of repos) { + const rules = await this.branchProtections(org, repo); + const own = rules.find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH); const rule = `${repoPath(org, repo)}/branch_protections/${MAIN_BRANCH}`; - const patch = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); - if (patch.status === 200) continue; - if (patch.status !== 404) GiteaAdmin.fail(rule, patch); - const made = await this.request(`${repoPath(org, repo)}/branch_protections`, { - method: "POST", - body: JSON.stringify({ rule_name: MAIN_BRANCH, enable_status_check: false, status_check_contexts: [], ...mainRuleBody() }), - }); - if (made.status === 201) continue; - if (made.status === 403 || made.status === 409 || made.status === 422) { - // A rule made between the two calls: edit it. - const again = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); - if (again.status === 200) continue; + if (own) { + const patch = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) }); + if (patch.status !== 200) GiteaAdmin.fail(rule, patch); + continue; } - GiteaAdmin.fail(`${repoPath(org, repo)}/branch_protections`, made); + // A rule named exactly main is matched before any glob rule, so it would hide the statuses, approvals and + // administrator limit a glob rule covering main asks: they are copied into it (issue 948, review round 1). + const covering = rules.filter((r) => ruleCovers(String(r.rule_name ?? r.branch_name ?? ""), MAIN_BRANCH)); + const checks = [...new Set(covering.filter((r) => r.enable_status_check).flatMap((r) => (r.status_check_contexts ?? []).filter(Boolean)))]; + const copied: Record = { enable_status_check: checks.length > 0, status_check_contexts: checks }; + const approvals = Math.max(0, ...covering.map((r) => Number(r.required_approvals ?? 0))); + if (approvals > 0) copied.required_approvals = approvals; + if (covering.some((r) => r.block_admin_merge_override === true)) copied.block_admin_merge_override = true; + const made = await this.request(`${repoPath(org, repo)}/branch_protections`, { + method: "POST", body: JSON.stringify({ rule_name: MAIN_BRANCH, ...copied, ...mainRuleBody() }), + }); + if (made.status !== 201) GiteaAdmin.fail(`${repoPath(org, repo)}/branch_protections`, made); } return repos; } diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 4c444060..b80aed1e 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -530,6 +530,7 @@ async function listenForForge(client: GiteaClient): Promise { } } +let mainTouched = -1; if (gitea) { const client = gitea; // A poll that fails says so once, not once a minute: the same reason repeating (the forge not up @@ -557,6 +558,13 @@ if (gitea) { // **The forge's events first, the polls as the safety net** (novox/hq issue 729): while verified deliveries arrive the // polls slow to every five minutes and announce only what no event said (Pace); otherwise they keep 30 s. await listenForForge(client); + // Main takes no push and only the tools' account merges, in every repository, kept and not only when a token is + // minted: at the start and every ten minutes after, so a repository made later gets it too (issue 948, ADR 0398 §2). + tick(async () => { + const touched = await GiteaAdmin.fromEnv().ensureMainMerges("novox"); + if (touched.length !== mainTouched) console.log(`[gitea] main's rule is right in ${touched.length} repositories`); + mainTouched = touched.length; + }, 600_000); tick(() => pollRepos(client), 60_000); tick(() => pollMerged(client), () => pace.every()); tick(() => pollPulls(client), () => pace.every()); diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 02c947ad..249ac5ba 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -33,7 +33,9 @@ function fakeForge() { const calls: string[] = []; // The organisation's repositories, and the rule each has for main, which the administrator's setup puts right // (issue 948, ADR 0398 §2). - const repos = ["hq", "mesh-catalog"]; + const repos = ["hq", "mesh-catalog", "globbed"]; + const globRules = new Map([["globbed", [{ rule_name: "m*", enable_status_check: true, + status_check_contexts: ["mesh/merge-gate", "mesh/repo-check"], block_admin_merge_override: true, required_approvals: 1 }]]]); const mainRules = new Map([["hq", { rule_name: "main", enable_status_check: true, status_check_contexts: ["mesh/merge-gate"] }]]); // What the forge does wrong when a test says so: a token mint or delete it fails, a team another process made // between this one's look and its create (gitea answers the create 422). @@ -152,6 +154,9 @@ function fakeForge() { if (m) { const admins = isAdmin(me) || teams.some((t) => t.members.has(me) && t.units.permission === "admin"); if (!admins) return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" }); + if (req.method === "GET" && m[2] === undefined) { + return json(res, 200, [...(mainRules.has(m[1]) ? [mainRules.get(m[1])] : []), ...(globRules.get(m[1]) ?? [])]); + } if (req.method === "PATCH" && m[2] === "main") { const rule = mainRules.get(m[1]); if (!rule) return json(res, 404, { message: "branch protection does not exist" }); @@ -367,3 +372,26 @@ test("setting the tools' account up puts main's rule right in every repository: assert.ok(forge.calls.slice(from).filter((c) => /branch_protections/.test(c)).every((c) => c.endsWith(` as ${ADMIN}`)), "only the administrator's login touches a rule: the tools' account is no repository's administrator"); }); + +test("a glob rule covering main keeps its statuses when main is given a rule of its own name (issue 948, review round 1)", async () => { + const env = await environment(); + await fresh(env).listRepos(); + const rule = forge.mainRules.get("globbed"); + assert.ok(rule, "main is given its own rule"); + assert.deepEqual(rule.status_check_contexts, ["mesh/merge-gate", "mesh/repo-check"]); + assert.equal(rule.enable_status_check, true); + assert.equal(rule.block_admin_merge_override, true); + assert.equal(rule.required_approvals, 1); + assert.equal(rule.enable_push, false); + assert.deepEqual(rule.merge_whitelist_usernames, [TOOLS_ACCOUNT]); +}); + +test("a start with the token kept still puts main's rules right: the module's start asks for it each time (issue 948, review round 1)", async () => { + const src = await readFile(new URL("../index.ts", import.meta.url), "utf8"); + assert.match(src, /ensureMainMerges\("novox"\)/, "index.ts calls it outside the token's mint"); + const { GiteaAdmin } = await import("../dist/client.js"); + const env = await environment(); + forge.mainRules.delete("hq"); + await GiteaAdmin.fromEnv(env).ensureMainMerges("novox"); + assert.equal(forge.mainRules.get("hq").enable_push, false, "a rule gone is made again with no token minted"); +}); From 4cccd817a040753f6e216e883660e80144979148 Mon Sep 17 00:00:00 2001 From: "fix-deliv-awg@novox" Date: Fri, 16 Oct 2026 21:36:06 +0200 Subject: [PATCH 8/8] =?UTF-8?q?End=20a=20group's=20ask=20and=20Approve=20w?= =?UTF-8?q?hen=20any=20member=20is=20pushed=20(issue=20948,=20ADR=200398?= =?UTF-8?q?=20=C2=A73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery/approval.go | 52 ++++++++++++++++++- .../cmd/mesh-delivery/approval_test.go | 51 ++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 2 + .../mesh-delivery/cmd/mesh-delivery/ready.go | 3 ++ 4 files changed, 107 insertions(+), 1 deletion(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval.go b/modules/mesh-delivery/cmd/mesh-delivery/approval.go index 7989ddd3..41b46e18 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/approval.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval.go @@ -229,11 +229,58 @@ func (h *Holder) covering(d *Delivery) *ReadyRun { return nil } +func headsEqual(a, b map[string]string) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + if b[k] != v { + return false + } + } + return true +} + +// headsHold is whether an ask's heads are still the heads of d's group: every member now at the head asked, and every +// head asked that is no member now a member already merged. +func (h *Holder) headsHold(d *Delivery, c *ReadyRun) bool { + now := map[string]bool{} + for _, m := range h.askMembers(d) { + now[m.ID] = true + if c.Heads[m.ID] != m.Commit { + return false + } + } + for id := range c.Heads { + if !now[id] { + if o := h.deliveries[id]; o == nil || o.MergedAs == "" { + return false + } + } + } + return true +} + +// endStaleAsks ends every ask whose heads are no longer its group's: a push to any member, marked or not, ends the +// approval (ADR 0398 §3). Under the lock; the cancels run with the tick's asks. +func (h *Holder) endStaleAsks() { + for _, d := range h.deliveries { + r := d.Ready + if r == nil || r.Merged != nil || d.MergedAs != "" || (r.AskID == "" && r.Approved == nil) || len(r.Heads) == 0 { + continue + } + if !h.headsHold(d, r) { + h.cancels = append(h.cancels, h.dropAsk(r.AskID, "the heads it covers are no longer the group's heads")) + } + } +} + // approval is what a delivery that may merge waits on for the operator: empty once its head is approved, and the ask // to make when none is made yet. Under the lock. func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { r := d.Ready - if c := h.covering(d); c != nil { + c := h.covering(d) + if c != nil { if c.Approved != nil { r.Approved, r.Warrant, r.AskID, r.Ask, r.Heads = c.Approved, c.Warrant, c.AskID, c.Ask, c.Heads return "", nil @@ -289,6 +336,9 @@ func (h *Holder) dropAsk(id, why string) func() { for _, d := range h.deliveries { if d.Ready != nil && d.Ready.AskID == id { d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil + if d.Ready.Merged == nil { + d.Ready.Approved, d.Ready.Warrant = nil, "" + } h.keep(d) } } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go index 33926550..aa1454b4 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/approval_test.go @@ -200,3 +200,54 @@ func TestAGroupIsAskedOnceForAllItsHeadsAndMergedInOrderOnOneApprove(t *testing. t.Fatalf("the next member was not merged on the same Approve: merges %v, asks %d", w.forge.merges, w.op.count()) } } + +// A push to a group member without a mark ends the group's ask (ADR 0398 §3, issue 948; review round 1): the Approve +// that follows would be for heads the group no longer has. +func TestAPushToAnUnmarkedGroupMemberEndsTheGroupsAskAndApprove(t *testing.T) { + w := newWorld(t) + w.op.auto = false + const ctlHead, catHead, catPushed = "c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1c1", "d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2", "e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3" + ctl, cat := IDOf("novox/mesh-controller", ctlHead), IDOf("novox/mesh-catalog", catHead) + w.ctl.order = func(ms []Member) Order { + var o []string + for _, m := range ms { + if m.Repository == "novox/mesh-controller" { + o = append([]string{m.ID}, o...) + } else { + o = append(o, m.ID) + } + } + return Order{Order: o} + } + _, _ = ctl, cat + w.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go")) + w.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json")) + w.h.Checked(judgedAt("novox/mesh-controller", 1, ctlHead, "pass", "t1t1t1t1t1t1")) + w.h.Checked(judgedAt("novox/mesh-catalog", 2, catHead, "pass", "t3t3t3t3t3t3")) + w.forge.open = []OpenPull{markedPull("novox/mesh-controller", 1, ctlHead, ctlHead), + markedPull("novox/mesh-catalog", 2, catHead, catHead)} + w.h.SeeOpenPulls() + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/x", ID: "check-1", Verdict: "pass", Summary: "both compose"}) + w.settleAll() + first := w.op.open() + if len(first) != 1 { + t.Fatalf("asked %v", first) + } + // The catalog member is pushed; its new head has no mark yet. + w.h.PullUpdated(pr("novox/mesh-catalog", 2, catPushed, "feat/x", "modules/app/module.json")) + w.forge.open = []OpenPull{markedPull("novox/mesh-controller", 1, ctlHead, ctlHead), + {Owner: "novox", Repo: "mesh-catalog", Number: 2, Base: "main", HeadSHA: catPushed}} + w.h.SeeOpenPulls() + w.settleAll() + if len(w.op.cancelled) != 1 || w.op.cancelled[0] != first[0] { + t.Fatalf("the group's ask was not cancelled by the push: %v", w.op.cancelled) + } + w.op.answer(first[0], OptionApprove) // a late answer to the cancelled ask + w.h.Checked(judgedAt("novox/mesh-catalog", 2, catPushed, "pass", "t3t3t3t3t3t3")) + w.h.Checked(CheckedEvent{Group: "feat/x", ID: "check-2", Verdict: "pass", Summary: "both compose again"}) + w.settleAll() + if len(w.forge.merges) != 0 { + t.Fatalf("merged on an Approve of heads the group no longer has: %v", w.forge.merges) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 2d40a824..42626b6a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -29,6 +29,8 @@ type Holder struct { Logf func(format string, a ...any) // Operator is the operator channel, asked for the Approve of a merge into main (approval.go). Operator Operator + // cancels are asks to take back, gathered under the lock and run with the other asks of the tick. + cancels []func() mu sync.Mutex deliveries map[string]*Delivery diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index ca66438b..9ac5f36f 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -161,6 +161,7 @@ func sameFull(mark, head string) bool { // The asking happens outside the lock. func (h *Holder) readyAsks(now time.Time) []func() { var asks []func() + h.endStaleAsks() for k, m := range h.marks { p, mark := m.pull, *m.pull.Ready repository := strings.ToLower(p.Owner + "/" + p.Repo) @@ -262,6 +263,8 @@ func (h *Holder) readyAsks(now time.Time) []func() { h.keep(d) asks = append(asks, h.askMerge(d.ID, p, mark.Head)) } + asks = append(h.cancels, asks...) + h.cancels = nil return asks }