Feature agents-write-git, step 1: agents write with their own accounts, main stays closed, a merge into main waits for the operator (issues 948, 954) #338

Merged
mesh-tools merged 10 commits from feat/agents-write-git into main 2026-10-16 19:45:37 +00:00
29 changed files with 1416 additions and 52 deletions
+19 -7
View File
@@ -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-<node>`. The
An agent clones, fetches and pushes branches of the repositories as a forge account of its own, `agent-<node>`. 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-<node>` 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`.
@@ -209,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.
+44 -3
View File
@@ -5,8 +5,8 @@
import { readFileSync } from "node:fs";
import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js";
import { protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js";
import { Refused, checkSentPath, pathSegment, pathSegments } from "./guard.js";
import { MAIN_BRANCH, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js";
import { Refused, checkSentPath, ruleCovers, 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<BranchProtection>(`${repoPath(owner, repo)}/branch_protections/${pathSegments("branch", branch)}`,
{ method: "PATCH", body: JSON.stringify(body) });
@@ -988,6 +991,44 @@ 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<string[]> {
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 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}`;
if (own) {
const patch = await this.request(rule, { method: "PATCH", body: JSON.stringify(mainRuleBody()) });
if (patch.status !== 200) GiteaAdmin.fail(rule, patch);
continue;
}
// 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<string, unknown> = { 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;
}
/** 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<void> {
+7 -7
View File
@@ -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-<node>`, 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-<node>`, 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-<node>), 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{
+72 -10
View File
@@ -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 {
+92 -6
View File
@@ -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"])
}
}
+24 -2
View File
@@ -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 the branch 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;
+8
View File
@@ -530,6 +530,7 @@ async function listenForForge(client: GiteaClient): Promise<boolean> {
}
}
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());
+17 -1
View File
@@ -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<string, unknown> {
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<string, unknown> {
export function protectionBody(want: ProtectionWanted, creating: boolean, branch = ""): Record<string, unknown> {
const checks = [...new Set(want.statusChecks.map((c) => c.trim()).filter(Boolean))];
const body: Record<string, unknown> = { 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;
}
+18
View File
@@ -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"]) {
+30 -2
View File
@@ -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"] });
});
+2
View File
@@ -204,6 +204,8 @@ function fakeForge(): Promise<Forge> {
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") {
+68 -3
View File
@@ -31,6 +31,12 @@ 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", "globbed"];
const globRules = new Map<string, any[]>([["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<string, any>([["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 +137,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 +150,24 @@ 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 === "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" });
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 +179,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<void> }>((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 +354,44 @@ 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");
});
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");
});
+5
View File
@@ -41,6 +41,8 @@ export interface ToolsForge {
ensureUser(username: string, password: string, email: string, plain?: boolean): Promise<void>;
ensureTeamWith(org: string, team: string, units: Record<string, unknown>): Promise<number>;
addUserToTeam(teamId: number, username: string): Promise<void>;
/** Main's rule in every repository of the organisation: no push, only the tools' account merges (ADR 0398 §2). */
ensureMainMerges?(org: string): Promise<string[]>;
}
/** 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);
}
+3 -2
View File
@@ -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) };
@@ -0,0 +1,458 @@
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: "<id>=<sha>" 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
}
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
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
}
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
if d.Ready.Merged == nil {
d.Ready.Approved, d.Ready.Warrant = 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
}
@@ -0,0 +1,253 @@
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())
}
}
// 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)
}
}
@@ -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)
@@ -27,6 +27,10 @@ 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
// 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
@@ -381,7 +385,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 +555,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 +1064,7 @@ func (h *Holder) Tick() {
for _, ask := range h.leaveStopped() {
ask()
}
h.hearAsks()
h.mu.Lock()
asks := h.readyAsks(h.Now())
h.mu.Unlock()
@@ -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"}
@@ -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)
}
@@ -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: <repository>` 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"`
@@ -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) }
@@ -4,6 +4,8 @@ import (
"fmt"
"strings"
"time"
"git.novox.be/novox/mesh-sdk/go/asks"
)
// Merge on ready (novox/hq issue 722).
@@ -69,8 +71,29 @@ 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"`
// 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.
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 {
@@ -80,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
}
@@ -135,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)
@@ -150,6 +177,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
}
@@ -157,7 +187,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))
@@ -167,6 +197,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"
@@ -185,6 +223,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 {
@@ -194,6 +244,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
}
@@ -202,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
}
@@ -310,6 +373,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 <head>"
// 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 {
@@ -347,6 +423,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(),
@@ -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)
}
}
+1 -1
View File
@@ -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
+2 -2
View File
@@ -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=
+7 -2
View File
@@ -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"
@@ -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
@@ -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)
}
}