From 791c2a3d48b4d11d2df6de1adc122f6bb528135b Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 18:36:18 +0200 Subject: [PATCH] Refuse a removed verb by name before reading its arguments, as the SDK member's table now offers none (issue 1851, A11a; issue 1318, C3) --- cmd/mesh-controller/delivery_test.go | 2 -- cmd/mesh-controller/queue_test.go | 2 -- cmd/mesh-controller/seatverbs.go | 46 ++++++++++----------------- cmd/mesh-controller/seatverbs_test.go | 21 ++++++------ 4 files changed, 28 insertions(+), 43 deletions(-) diff --git a/cmd/mesh-controller/delivery_test.go b/cmd/mesh-controller/delivery_test.go index e21c319e..30478a90 100644 --- a/cmd/mesh-controller/delivery_test.go +++ b/cmd/mesh-controller/delivery_test.go @@ -138,8 +138,6 @@ func TestTheDeliveryVerbsComposeTheirCommands(t *testing.T) { {"delivery-order", map[string]any{"members": "[]"}, []string{"delivery", "order", "--members", "[]"}}, {"delivery-check", map[string]any{"group": "feat/x", "members": "[]"}, []string{"delivery", "check", "--group", "feat/x", "--members", "[]"}}, - {"delivery-plan", map[string]any{"repository": "novox/a", "paths": "x", "head": "c0"}, - []string{"delivery", "plan"}}, {"plans", map[string]any{"go": "plan-1", "why": "down"}, []string{"plans", "go", "plan-1", "--why", "down"}}, } { got, err := argvFor(c.verb, c.args) diff --git a/cmd/mesh-controller/queue_test.go b/cmd/mesh-controller/queue_test.go index da2fea1e..c5a4e281 100644 --- a/cmd/mesh-controller/queue_test.go +++ b/cmd/mesh-controller/queue_test.go @@ -158,9 +158,7 @@ func TestTheQueueVerbsRunTheirCommands(t *testing.T) { {"cancel", map[string]any{"id": "build-1"}, []string{"cancel", "build-1"}}, {"clear", nil, []string{"clear"}}, {"clear", map[string]any{"dead": "true"}, []string{"clear", "--dead"}}, - {"rebuild", map[string]any{"what": "gitea"}, []string{"rebuild"}}, {"replay", map[string]any{"id": "build-1"}, []string{"replay", "build-1"}}, - {"replay", map[string]any{"id": "build-1", "register": "true", "older": "true"}, []string{"replay", "build-1", "--register"}}, {"kill", map[string]any{"id": "build-1"}, []string{"kill", "build-1"}}, {"pause", nil, []string{"pause"}}, {"resume", map[string]any{"node": "ace"}, []string{"resume", "ace"}}, diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 2f73d897..b27187f3 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -52,6 +52,17 @@ type verbAnswer struct { // verb without the machine and ran as a push of every machine behind; a verb that answers "I did // not take that" would have stopped it before anything was sent. func argvFor(verb string, args map[string]any) ([]string, error) { + // **A removed verb is refused by name, naming its replacement, whatever was asked of it** (novox/hq ADR 0501 §5, + // ADR 0503 §3, ADR 0450 §8): before its arguments are read, so an argument a table of before offered is not what the + // caller is told. + if removedVerbs[verb] { + return nil, removedByName(verb) + } + if verb == "replay" { + if v, given := args["register"]; given && v != false && v != "false" { + return nil, removedByName("replay --register") + } + } a, err := readArguments(verb, args) if err != nil { return nil, err @@ -86,13 +97,9 @@ type verbArguments struct { misread []string // arguments the command line read that the schema does not declare: a bug here } -// passOver takes every argument given as read: a removed verb is refused by name whatever was asked of it (novox/hq ADR -// 0501 §5), so no argument of a table of before is refused as passed over in its place. -func (a *verbArguments) passOver() { - for k := range a.given { - a.used[k] = true - } -} +// removedVerbs are the controller seat's verbs R4e removed, kept in its table with no input so a caller is answered +// by name (ADR 0501 §5, ADR 0503 §3, ADR 0450 §8). +var removedVerbs = map[string]bool{"build": true, "rebuild": true, "forget-checks": true, "delivery-plan": true} // controllerVerb is this binary's own definition of a verb: what it runs is what it declares, so the // arguments are checked against the table compiled beside argvFor, not a row a newer or older build @@ -411,10 +418,6 @@ func (a *verbArguments) commandLine() ([]string, error) { } return argv, nil // The delivery's owner's verbs (novox/hq ADR 0239). - case "delivery-plan": - // Removed (ADR 0450 §8; issue 1318): the command refuses it by name, whatever was asked. - a.passOver() - return []string{"delivery", "plan"}, nil case "delivery-order": if err := need("members"); err != nil { return nil, err @@ -485,10 +488,6 @@ func (a *verbArguments) commandLine() ([]string, error) { case "queue": // As data, the table its output (novox/hq issue 455). return []string{"queue", "--json"}, nil - case "forget-checks": - // Removed (ADR 0503 §3; issue 1318): the command refuses it by name, whatever was asked. - a.passOver() - return []string{"forget-checks"}, nil case "try": // A branch tried before its pull request (novox/hq issue 686): asked, or read by the id it answered. if id := str("id"); id != "" { @@ -521,21 +520,12 @@ func (a *verbArguments) commandLine() ([]string, error) { return []string{"clear", "--dead"}, nil } return []string{"clear"}, nil - case "rebuild": - // Removed (ADR 0501 §5): the command refuses it by name, naming its replacement, whatever was asked. - a.passOver() - return []string{"rebuild"}, nil case "replay": if err := need("id"); err != nil { return nil, err } - argv := []string{"replay", str("id")} - // A table of before still offers register: passed on, so the command refuses it by name (ADR 0501 §5). - if on("register") { - argv = append(argv, "--register") - } - a.passOver() - return argv, nil + // A dry run alone (ADR 0429 §18): register is refused by name before this (argvFor). + return []string{"replay", str("id")}, nil case "pause", "resume": if n := str("node"); n != "" { return []string{verb, n}, nil @@ -1073,10 +1063,6 @@ func (a *verbArguments) commandLine() ([]string, error) { return nil, err } return []string{"module", "issue", str("module"), "--node", str("node")}, nil - case "build": - // Removed (ADR 0501 §5): the command refuses it by name, naming its replacement, whatever was asked. - a.passOver() - return []string{"build"}, nil } return nil, fmt.Errorf("%q is a verb of the %s seat's table that this binary has no command line for", verb, catalogue.ControllerSeatName) diff --git a/cmd/mesh-controller/seatverbs_test.go b/cmd/mesh-controller/seatverbs_test.go index 3b88d5c5..b41492dc 100644 --- a/cmd/mesh-controller/seatverbs_test.go +++ b/cmd/mesh-controller/seatverbs_test.go @@ -120,21 +120,24 @@ func TestTheRemovedVerbsAreRefusedByName(t *testing.T) { for _, c := range []struct { verb string args map[string]any - want string + said string }{ - {"build", map[string]any{"repository": "novox/x", "path": "modules/x"}, "build"}, - {"build", map[string]any{"on": "mesh-tools"}, "build"}, - {"rebuild", map[string]any{"what": "gitea"}, "rebuild"}, - {"replay", map[string]any{"id": "build-1", "register": "true", "older": "true"}, "replay build-1 --register"}, + {"build", map[string]any{"repository": "novox/x", "path": "modules/x"}, "`build` is removed"}, + {"build", map[string]any{}, "`build` is removed"}, + {"rebuild", map[string]any{"what": "gitea"}, "`rebuild` is removed"}, + {"replay", map[string]any{"id": "build-1", "register": "true", "older": "true"}, "`replay --register` is removed"}, // 1318: delivery-plan and forget-checks go too. - {"delivery-plan", map[string]any{"repository": "novox/a", "paths": "x"}, "delivery plan"}, - {"forget-checks", map[string]any{"owner": "novox", "repo": "a", "number": "7"}, "forget-checks"}, + {"delivery-plan", map[string]any{"repository": "novox/a", "paths": "x"}, "`delivery-plan` is removed"}, + {"forget-checks", map[string]any{"owner": "novox", "repo": "a", "number": "7"}, "`forget-checks` is removed"}, } { argv, err := argvFor(c.verb, c.args) - if err != nil || strings.Join(argv, " ") != c.want { - t.Errorf("%s %v: %v %v, want %q", c.verb, c.args, argv, err, c.want) + if err == nil || !strings.Contains(err.Error(), c.said) { + t.Errorf("%s %v: %v %v, want refused saying %q", c.verb, c.args, argv, err, c.said) } } + if argv, err := argvFor("replay", map[string]any{"id": "build-1"}); err != nil || strings.Join(argv, " ") != "replay build-1" { + t.Errorf("replay as a dry run: %v %v", argv, err) + } for _, removed := range []string{"build", "rebuild", "forget-checks", "delivery-plan"} { err := removedByName(removed) if err == nil || !strings.Contains(err.Error(), "`"+removed+"` is removed") || !strings.Contains(err.Error(), "ADR 05") {