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)

This commit is contained in:
2026-10-22 19:05:05 +02:00
parent d36e0ea278
commit 791c2a3d48
4 changed files with 28 additions and 43 deletions
-2
View File
@@ -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)
-2
View File
@@ -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"}},
+16 -30
View File
@@ -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)
+12 -9
View File
@@ -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") {