From 45d091778c844b1db4d65a63eb5060554af7719e Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 18:26:37 +0200 Subject: [PATCH] =?UTF-8?q?Refuse=20delivery-plan=20and=20forget-checks=20?= =?UTF-8?q?by=20name=20and=20take=20a=20merge's=20warrant=20from=20the=20t?= =?UTF-8?q?eller=20alone,=20no=20longer=20mesh-delivery's=20(issue=201318,?= =?UTF-8?q?=20C3,=20ADR=200450=20=C2=A78)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/mesh-controller/catch_up.go | 10 +-- cmd/mesh-controller/catch_up_test.go | 40 ++------- cmd/mesh-controller/check_builds.go | 24 ------ cmd/mesh-controller/delivery.go | 25 ++---- cmd/mesh-controller/delivery_test.go | 2 +- .../first_builds_integration_test.go | 2 +- cmd/mesh-controller/main.go | 2 +- cmd/mesh-controller/moved.go | 25 +++--- cmd/mesh-controller/moved_asker_test.go | 15 ++-- cmd/mesh-controller/moved_bus_test.go | 3 +- cmd/mesh-controller/moved_test.go | 5 +- cmd/mesh-controller/new_module_bus_test.go | 3 +- cmd/mesh-controller/plans_data_test.go | 85 ------------------- cmd/mesh-controller/seatverbs.go | 20 ++--- cmd/mesh-controller/seatverbs_test.go | 11 ++- 15 files changed, 59 insertions(+), 213 deletions(-) diff --git a/cmd/mesh-controller/catch_up.go b/cmd/mesh-controller/catch_up.go index f3f40bb7..f69e4afc 100644 --- a/cmd/mesh-controller/catch_up.go +++ b/cmd/mesh-controller/catch_up.go @@ -241,10 +241,9 @@ func mergeAskPrefix(number int, head string) string { return fmt.Sprintf("merge-%d-%s-", number, head) } -// askersOfMerges are the askers a merge's warrant is read under: the teller, and mesh-delivery for a merge before release 3e -// (ADR 0437 §6). Release 4's controller work stops accepting mesh-delivery (ADR 0450 §8); a merge asked under it earlier is -// still found here until then. -func askersOfMerges() []string { return []string{sdkdelivery.TellerModule, theDelivery} } +// askersOfMerges are the askers a merge's warrant is read under: the teller alone (ADR 0437 §6), since R4e stopped taking +// mesh-delivery's (ADR 0450 §8; issue 1318). +func askersOfMerges() []string { return []string{sdkdelivery.TellerModule} } // caughtUpWarrant judges a chosen merge ask of the router as the warrant of a merge: the ask is the operator's, the warrant // answers that very ask as it was published, and it is the operator's Approve proved by the channel it came through (ADR 0437 @@ -391,9 +390,6 @@ var findMergeWarrant = func(ctx context.Context, inv catalogueReader, owner, rep if asker == sdkdelivery.TellerModule && !strings.HasPrefix(id, sdkdelivery.AskKindMerge+"-") { continue } - if asker == theDelivery && !named { - continue - } ask, w, err := closedRecordOf(ctx, inv, asker, id) if err != nil { return "", false, "", err diff --git a/cmd/mesh-controller/catch_up_test.go b/cmd/mesh-controller/catch_up_test.go index b120a1cd..eb7be1bb 100644 --- a/cmd/mesh-controller/catch_up_test.go +++ b/cmd/mesh-controller/catch_up_test.go @@ -69,12 +69,12 @@ func TestAForgeThatDoesNotAnswerIsAnErrorNotAnEmptyTrunk(t *testing.T) { func TestACaughtUpMergeIsAcceptedOnTheSameWarrantAsAnAnnouncedOne(t *testing.T) { ok := approvedMerge(t) - for _, asker := range []string{sdkdelivery.TellerModule, theDelivery} { - w := ok.warrant - w.Asker = asker - if _, err := caughtUpWarrant(asker, &ok.ask, &w); err != nil { - t.Errorf("the operator's Approve under %s was refused: %v", asker, err) - } + if _, err := caughtUpWarrant(sdkdelivery.TellerModule, &ok.ask, &ok.warrant); err != nil { + t.Errorf("the operator's Approve under the teller was refused: %v", err) + } + // 1318 (ADR 0450 §8): mesh-delivery's warrant of before the gate is no longer taken. + if _, err := caughtUpWarrant(theDelivery, &ok.ask, &ok.warrant); err == nil { + t.Error("a warrant under mesh-delivery was accepted") } if _, err := caughtUpWarrant("keycloak", &ok.ask, &ok.warrant); err == nil { t.Error("a warrant of an asker the controller takes none from was accepted") @@ -229,34 +229,6 @@ func TestAGroupsWarrantIsFoundByTheAsksWordsForThePullRequestAndItsHead(t *testi } } -// Keycloak's merge was asked as mesh-delivery.merge-378-b9104454f9a5-1, before release 3e: the id names the pull request and -// the first twelve digits of its head, and the warrant is found by them under mesh-delivery. -func TestAWarrantAskedUnderMeshDeliveryIsFoundByThePullRequestAndItsHead(t *testing.T) { - ok := approvedMerge(t) - head := "b9104454f9a5e0c4d3b2a1f0e9d8c7b6a5948372" - askID := "merge-378-b9104454f9a5-1" - ask, w := ok.ask, ok.warrant - ask.ID, w.Ask, w.Asker = askID, askID, theDelivery - w.AskDigest = ask.Digest() - withRouter(t, map[string]map[string]aRouterRecord{theDelivery: { - "merge-377-aaaaaaaaaaaa-1": {}, - askID: {ask, w}, - }}) - why, found, reason, err := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 378, head) - if err != nil || !found || !strings.Contains(why, askID) { - t.Fatalf("the warrant was not found: %q %v %q %v", why, found, reason, err) - } - // Another head names another ask, and none is found. - if _, found, _, _ := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 378, "ffffffffffff0000"); found { - t.Fatal("another head found a warrant") - } - // A router that cannot be read is an error, never "no warrant". - routerAskIDs = func(context.Context, catalogueReader, string) ([]string, error) { return nil, errors.New("no answer") } - if _, found, _, err := findMergeWarrant(t.Context(), nil, "novox", "mesh-catalog", 378, head); found || err == nil { - t.Fatalf("an unreadable router read as no warrant: %v %v", found, err) - } -} - // A trunk shorter than a page, or with none, is read without a fault: the pages end when the forge's do. func TestAShortTrunkIsReadWholeAndAnEmptyOneIsNoMerges(t *testing.T) { was := trunkCommitsPage diff --git a/cmd/mesh-controller/check_builds.go b/cmd/mesh-controller/check_builds.go index b1f78938..51b1c0dd 100644 --- a/cmd/mesh-controller/check_builds.go +++ b/cmd/mesh-controller/check_builds.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "os" - "strconv" "github.com/novox/mesh-controller/internal/builder" "github.com/novox/mesh-controller/internal/inventory" @@ -62,26 +61,3 @@ func forgetChecksOfWalk(ctx context.Context, inv *inventory.Inventory, p invento fmt.Printf("%s: %s\n", p.ID, said) } } - -// forgetChecksCommand is `forget-checks `: the verb the delivery's owner asks when the forge -// says a pull request closed unmerged (novox/hq ADR 0350 §5). -func forgetChecksCommand(ctx context.Context, args []string) error { - if len(args) != 3 { - return errors.New("forget-checks ") - } - number, err := strconv.Atoi(args[2]) - if err != nil || number <= 0 { - return fmt.Errorf("%q is not a pull request's number", args[2]) - } - open, err := openStores(ctx) - if err != nil { - return err - } - defer open.Close() - said, err := forgetChecks(ctx, open.inventory, builder.PullRef{Owner: args[0], Repo: args[1], Number: number}) - if err != nil { - return err - } - fmt.Println(said) - return nil -} diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 4e102431..17a9a16c 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -815,16 +815,14 @@ func deliveryPlanDocument(m orderMember, entries []inventory.Entry, read map[str // delivery walks [-n 50] [--plan ] func deliveryCommand(ctx context.Context, args []string) error { if len(args) == 0 { - return errors.New("delivery plan|order|check|go|stop|walks") + return errors.New("delivery order|check|go|stop|walks") } sub, rest := args[0], args[1:] + if sub == "plan" { + // Removed (ADR 0450 §8; issue 1318): refused by name before any flag is read, naming its replacement. + return removedByName("delivery-plan") + } set := flag.NewFlagSet("delivery "+sub, flag.ContinueOnError) - repository := set.String("repository", "", "owner/repository") - base := set.String("base", "main", "the branch it merges into") - head := set.String("head", "", "the commit at hand") - paths := set.String("paths", "", "the files it changes, comma-separated") - moduleDirs := set.String("module-dirs", "", "the directories holding a module.json at the head, comma-separated") - removed := set.String("removed", "", "the files it deletes, comma-separated") membersJSON := set.String("members", "", "a group's members, as JSON") membersFile := set.String("members-file", "", "a file of a group's members, as JSON: the same as --members, for a list "+ "too long for an argument (novox/hq issue 1272)") @@ -872,19 +870,6 @@ func deliveryCommand(ctx context.Context, args []string) error { return enc.Encode(v) } switch sub { - case "plan": - if *repository == "" || *head == "" && *paths == "" { - return errors.New("delivery plan --repository owner/repo --head --paths a,b") - } - m := changeMember(*repository, *base, *head, splitList(*paths), splitList(*removed), moduleDirsOf(*moduleDirs)) - entries, read, edges, err := theGraph(ctx, inv) - if err != nil { - return err - } - return answer(deliveryPlanDocument(m, entries, read, edges, func(module string) (inventory.Upgrade, bool) { - u, err := inv.UpgradeOf(ctx, module) - return u, err == nil - })) case "order": if len(members) == 0 { return errors.New("delivery order --members : a group has members") diff --git a/cmd/mesh-controller/delivery_test.go b/cmd/mesh-controller/delivery_test.go index a3127130..e21c319e 100644 --- a/cmd/mesh-controller/delivery_test.go +++ b/cmd/mesh-controller/delivery_test.go @@ -139,7 +139,7 @@ func TestTheDeliveryVerbsComposeTheirCommands(t *testing.T) { {"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", "--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/first_builds_integration_test.go b/cmd/mesh-controller/first_builds_integration_test.go index bab2559b..4608f763 100644 --- a/cmd/mesh-controller/first_builds_integration_test.go +++ b/cmd/mesh-controller/first_builds_integration_test.go @@ -64,7 +64,7 @@ func TestAMergeOnTheApproveIntoANewRepositoryRegistersItsModuleFromItsKey(t *tes a := aDeliveryMergeOf(t, head, map[string]string{"novox/mesh-delivery@a1a1a1a1a1a1": head}) if c.approved { record, _ := json.Marshal(map[string]any{"state": "closed", "ask": a.ask, "warrant": a.warrant}) - if _, err := records.Put(ctx, theDelivery+"."+a.merge.AskID, record); err != nil { + if _, err := records.Put(ctx, sdkdelivery.TellerModule+"."+a.merge.AskID, record); err != nil { t.Fatal(err) } } diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 9de2b094..9eab8a5e 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -108,7 +108,7 @@ func run() error { case "kill": return killCommand(ctx, args[1:]) case "forget-checks": - return forgetChecksCommand(ctx, args[1:]) + return removedByName("forget-checks") case "try": return tryCommand(ctx, args[1:]) case "pause", "resume": diff --git a/cmd/mesh-controller/moved.go b/cmd/mesh-controller/moved.go index d1e97efc..6c07c210 100644 --- a/cmd/mesh-controller/moved.go +++ b/cmd/mesh-controller/moved.go @@ -52,23 +52,22 @@ type deliveryMerge struct { Asker string `json:"asker,omitempty"` } -// askerOf is the asker whose warrant the merge is read under: the one the announcement names, mesh-delivery when it -// names none. -func (d deliveryMerge) askerOf() string { - if d.Asker == "" { - return theDelivery - } - return d.Asker -} +// askerOf is the asker whose warrant the merge is read under: the one the announcement names. One that names none is +// from mesh-delivery of before the gate, whose warrant is no longer taken (novox/hq ADR 0450 §8; issue 1318). +func (d deliveryMerge) askerOf() string { return d.Asker } -// askerAccepted is whether the controller takes a warrant from this asker for a merge: the teller, and mesh-delivery -// until the gate takes the seat over (novox/hq ADR 0437 §6). Any other registers no move. +// askerAccepted is whether the controller takes a warrant from this asker for a merge: the teller alone, since the gate +// took the delivery seat over (novox/hq ADR 0437 §6, ADR 0450 §8; issue 1318). Any other registers no move. func askerAccepted(asker string) error { - if asker == theDelivery || asker == sdkdelivery.TellerModule { + if asker == sdkdelivery.TellerModule { return nil } - return fmt.Errorf("the merge names the asker %q, and the controller takes a warrant only from %s and %s", - asker, sdkdelivery.TellerModule, theDelivery) + if asker == "" { + return fmt.Errorf("the merge names no asker, as mesh-delivery's of before the gate did, and the controller takes "+ + "a warrant only from %s (ADR 0450 §8)", sdkdelivery.TellerModule) + } + return fmt.Errorf("the merge names the asker %q, and the controller takes a warrant only from %s (ADR 0450 §8)", + asker, sdkdelivery.TellerModule) } // parseDeliveryMerge reads an announcement, refusing one that names no pull request, head or ask. diff --git a/cmd/mesh-controller/moved_asker_test.go b/cmd/mesh-controller/moved_asker_test.go index 9c6c3e37..2fd529fb 100644 --- a/cmd/mesh-controller/moved_asker_test.go +++ b/cmd/mesh-controller/moved_asker_test.go @@ -48,18 +48,19 @@ func TestAMergeNamingTheTellerIsMovedOnTheTellersWarrant(t *testing.T) { } } -// 1213/AC2: an announcement naming mesh-delivery, or none (today's module): read under mesh-delivery, registered. -func TestAMergeNamingNoAskerIsMovedOnMeshDeliverysWarrant(t *testing.T) { +// 1318 (ADR 0450 §8): a merge naming no asker, or the asker mesh-delivery of before the gate, registers no move; the +// teller's is taken, and its warrant read under its name. +func TestAMergeOfMeshDeliveryOrNoAskerMovesNothing(t *testing.T) { for _, named := range []string{"", theDelivery} { a := askedBy(t, named, theDelivery) - if _, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err != nil { - t.Errorf("asker %q: refused: %v", named, err) + if _, err := movedOnApprove(a.merge, &a.ask, &a.warrant); err == nil || !strings.Contains(err.Error(), "ADR 0450 §8") { + t.Errorf("asker %q: %v", named, err) } } was := routerAskOf wasKept := keptDeliveryMerge t.Cleanup(func() { routerAskOf, keptDeliveryMerge = was, wasKept }) - a := askedBy(t, "", theDelivery) + a := askedBy(t, sdkdelivery.TellerModule, sdkdelivery.TellerModule) keptDeliveryMerge = func(context.Context, string, string, int, string) (deliveryMerge, bool, error) { return a.merge, true, nil } @@ -71,8 +72,8 @@ func TestAMergeNamingNoAskerIsMovedOnMeshDeliverysWarrant(t *testing.T) { if _, err := approvedAnnounced(t.Context(), nil, "novox", "mesh-controller", announcedPull(7, a.merge.HeadSHA)); err != nil { t.Fatal(err) } - if readUnder != theDelivery { - t.Errorf("with no asker named the warrant was read under %q, not %q", readUnder, theDelivery) + if readUnder != sdkdelivery.TellerModule { + t.Errorf("the warrant was read under %q, not %q", readUnder, sdkdelivery.TellerModule) } } diff --git a/cmd/mesh-controller/moved_bus_test.go b/cmd/mesh-controller/moved_bus_test.go index e8aebce2..277295bd 100644 --- a/cmd/mesh-controller/moved_bus_test.go +++ b/cmd/mesh-controller/moved_bus_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "testing" "time" @@ -68,7 +69,7 @@ func TestAModuleBuiltFromANewRepositoryIsRegisteredOnlyOnTheOperatorsApproveOnTh a.merge.Owner, a.merge.Repo, a.merge.Number = "novox", "mesh-host", 7 if c.answer { record, _ := json.Marshal(map[string]any{"state": "closed", "ask": a.ask, "warrant": a.warrant}) - if _, err := records.Put(ctx, theDelivery+"."+a.merge.AskID, record); err != nil { + if _, err := records.Put(ctx, sdkdelivery.TellerModule+"."+a.merge.AskID, record); err != nil { t.Fatal(err) } } diff --git a/cmd/mesh-controller/moved_test.go b/cmd/mesh-controller/moved_test.go index 1a487280..5fa9d85d 100644 --- a/cmd/mesh-controller/moved_test.go +++ b/cmd/mesh-controller/moved_test.go @@ -1,6 +1,7 @@ package main import ( + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "strings" "testing" "time" @@ -21,7 +22,7 @@ func approvedMerge(t *testing.T) anApprovedMerge { heads := map[string]string{"novox/mesh-controller@aaaaaaaaaaaa": "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", "novox/mesh-catalog@bbbbbbbbbbbb": "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2"} m := deliveryMerge{Owner: "novox", Repo: "mesh-controller", Number: 7, HeadSHA: "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1", - Group: "g1", AskID: "merge-g1", Option: optionApprove, Heads: heads} + Group: "g1", AskID: "merge-g1", Option: optionApprove, Heads: heads, Asker: sdkdelivery.TellerModule} bind := func(option string, h map[string]string) string { d, err := asks.ActDigest(mergeAct(m.AskID, option, h)) if err != nil { @@ -35,7 +36,7 @@ func approvedMerge(t *testing.T) anApprovedMerge { Options: []asks.Option{ {ID: optionApprove, Label: "Approve", Does: "merges the group", Level: asks.Approve, Binds: bind(optionApprove, heads)}, {ID: "decline", Label: "Decline", Does: "leaves it", Level: asks.Approve, Binds: bind("decline", heads)}}} - w := asks.Warrant{Ask: m.AskID, Asker: theDelivery, Outcome: asks.OutcomeChosen, Option: optionApprove, Label: "Approve", + w := asks.Warrant{Ask: m.AskID, Asker: sdkdelivery.TellerModule, Outcome: asks.OutcomeChosen, Option: optionApprove, Label: "Approve", Level: asks.Approve, By: &asks.Person{Who: asks.Operator, Kind: "telegram", Identity: "1", Verified: "user id"}, At: at, AskDigest: ask.Digest()} return anApprovedMerge{merge: m, ask: ask, warrant: w} diff --git a/cmd/mesh-controller/new_module_bus_test.go b/cmd/mesh-controller/new_module_bus_test.go index 05f65214..03ff6e9a 100644 --- a/cmd/mesh-controller/new_module_bus_test.go +++ b/cmd/mesh-controller/new_module_bus_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + sdkdelivery "git.novox.be/novox/mesh-sdk/go/delivery" "testing" "time" @@ -69,7 +70,7 @@ func TestANewModuleFromANewRepositoryIsRegisteredOnlyOnTheOperatorsApproveOnTheB map[string]string{"novox/mesh-delivery@a1a1a1a1a1a1": "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1"}) if c.answer { record, _ := json.Marshal(map[string]any{"state": "closed", "ask": a.ask, "warrant": a.warrant}) - if _, err := records.Put(ctx, theDelivery+"."+a.merge.AskID, record); err != nil { + if _, err := records.Put(ctx, sdkdelivery.TellerModule+"."+a.merge.AskID, record); err != nil { t.Fatal(err) } } diff --git a/cmd/mesh-controller/plans_data_test.go b/cmd/mesh-controller/plans_data_test.go index 7250c300..f7fc58c0 100644 --- a/cmd/mesh-controller/plans_data_test.go +++ b/cmd/mesh-controller/plans_data_test.go @@ -1,13 +1,10 @@ package main import ( - "encoding/json" - "reflect" "strings" "testing" "git.novox.be/novox/mesh-sdk/go/catalogue" - "github.com/novox/mesh-controller/internal/inventory" ) // flagsOf reads a command line's flags (and its first positional after --what-if) into a map. @@ -28,74 +25,6 @@ func flagsOf(argv []string) map[string]string { return out } -// **The plan of a change as data from `plans`** (novox/hq ADR 0450 §2, issue 1324 AC1): asked for one change -// both ways, `plans` with data and `delivery-plan` answer the same plan in every field. -func TestPlansWhatIfAsDataAnswersWhatDeliveryPlanAnswers(t *testing.T) { - change := map[string]any{"repository": "novox/mesh-catalog", "paths": "modules/gitea/index.ts,modules/gitea/old.txt", - "head": "abc123", "base": "main", "module-dirs": "modules/gitea", "removed": "modules/gitea/old.txt"} - with := func(extra map[string]any) map[string]any { - out := map[string]any{} - for k, v := range change { - out[k] = v - } - for k, v := range extra { - out[k] = v - } - return out - } - plansArgv, err := argvFor("plans", with(map[string]any{"data": "true"})) - if err != nil { - t.Fatal(err) - } - deliveryArgv, err := argvFor("delivery-plan", change) - if err != nil { - t.Fatal(err) - } - if plansArgv[0] != "plans" || !slicesHas(plansArgv, "--data") || !slicesHas(plansArgv, "--what-if") { - t.Fatalf("plans with data became %q", strings.Join(plansArgv, " ")) - } - p, d := flagsOf(plansArgv), flagsOf(deliveryArgv) - for _, name := range []string{"paths", "head", "base", "module-dirs", "removed"} { - if p[name] == "" || p[name] != d[name] { - t.Errorf("--%s: plans says %q, delivery-plan %q", name, p[name], d[name]) - } - } - if p["what-if"] != d["repository"] { - t.Errorf("repository: plans says %q, delivery-plan %q", p["what-if"], d["repository"]) - } - - // Both commands build their document from the one function, from the same member. - const src = "http://forge.internal:20000/novox/mesh-catalog.git" - gitea := fromRepo("gitea", src, "modules/gitea") - gitea.Source.BuiltFrom = "old" - gitea.On = []string{"anchor", "laptop"} - entries := []inventory.Entry{gitea} - policy := func(string) (inventory.Upgrade, bool) { return inventory.Upgrade{RollOut: true}, true } - member := func(f map[string]string, repository string) orderMember { - return changeMember(repository, f["base"], f["head"], splitList(f["paths"]), splitList(f["removed"]), - moduleDirsOf(f["module-dirs"])) - } - a := deliveryPlanDocument(member(p, p["what-if"]), entries, nil, nil, policy) - b := deliveryPlanDocument(member(d, d["repository"]), entries, nil, nil, policy) - ja, _ := json.Marshal(a) - jb, _ := json.Marshal(b) - if !reflect.DeepEqual(ja, jb) { - t.Errorf("the two plans differ:\n%s\n%s", ja, jb) - } - plan := a["plan"].(interface{}) - raw, _ := json.Marshal(plan) - var fields map[string]any - _ = json.Unmarshal(raw, &fields) - for _, want := range []string{"moved", "tiers", "machines", "summary", "class", "head", "base", "repository"} { - if _, ok := fields[want]; !ok { - t.Errorf("the plan has no %q: %s", want, raw) - } - } - if fields["head"] != "abc123" || fields["base"] != "main" { - t.Errorf("head and base are %v and %v", fields["head"], fields["base"]) - } -} - // Without the data form the text answer is unchanged (AC2): no data flag, and the data-only arguments are not passed. func TestPlansWhatIfWithoutDataIsUnchanged(t *testing.T) { argv, err := argvFor("plans", map[string]any{"repository": "novox/x", "paths": "a,b"}) @@ -114,20 +43,6 @@ func TestPlansWhatIfWithoutDataIsUnchanged(t *testing.T) { } } -// delivery-plan stays and answers as before (AC3, ADR 0450 §8). -func TestDeliveryPlanStaysAsItWas(t *testing.T) { - argv, err := argvFor("delivery-plan", map[string]any{"repository": "novox/x", "paths": "a", "head": "h"}) - if err != nil { - t.Fatal(err) - } - if got := strings.Join(argv, " "); got != "delivery plan --repository novox/x --paths a --head h" { - t.Errorf("delivery-plan reads %q", got) - } - if _, ok := catalogueVerb("delivery-plan"); !ok { - t.Error("delivery-plan is gone") - } -} - func catalogueVerb(name string) (catalogue.Verb, bool) { return controllerVerb(name) } func slicesHas(s []string, v string) bool { diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3b86f5e3..2f73d897 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -412,16 +412,9 @@ func (a *verbArguments) commandLine() ([]string, error) { return argv, nil // The delivery's owner's verbs (novox/hq ADR 0239). case "delivery-plan": - if err := need("repository", "paths"); err != nil { - return nil, err - } - argv := []string{"delivery", "plan", "--repository", str("repository"), "--paths", str("paths")} - for _, flag := range []string{"head", "base", "module-dirs", "removed"} { - if v := str(flag); v != "" { - argv = append(argv, "--"+flag, v) - } - } - return argv, nil + // 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 @@ -493,10 +486,9 @@ func (a *verbArguments) commandLine() ([]string, error) { // As data, the table its output (novox/hq issue 455). return []string{"queue", "--json"}, nil case "forget-checks": - if err := need("owner", "repo", "number"); err != nil { - return nil, err - } - return []string{"forget-checks", str("owner"), str("repo"), str("number")}, nil + // 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 != "" { diff --git a/cmd/mesh-controller/seatverbs_test.go b/cmd/mesh-controller/seatverbs_test.go index 49756a89..3b88d5c5 100644 --- a/cmd/mesh-controller/seatverbs_test.go +++ b/cmd/mesh-controller/seatverbs_test.go @@ -126,18 +126,25 @@ func TestTheRemovedVerbsAreRefusedByName(t *testing.T) { {"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"}, + // 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"}, } { 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) } } - for _, removed := range []string{"build", "rebuild"} { + 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 0501") { + if err == nil || !strings.Contains(err.Error(), "`"+removed+"` is removed") || !strings.Contains(err.Error(), "ADR 05") { t.Errorf("%s: %v", removed, err) } } + if err := deliveryCommand(context.Background(), []string{"plan", "--repository", "novox/a"}); err == nil || + !strings.Contains(err.Error(), "`delivery-plan` is removed") { + t.Errorf("delivery plan: %v", err) + } err := replayCommand(context.Background(), []string{"build-1", "--register"}) if err == nil || !strings.Contains(err.Error(), "`replay --register` is removed") { t.Errorf("replay --register: %v", err)