Hand a group's long members list to the verb's command in a file, not one argument (issue 1272)

cmd/mesh-controller/delivery.go

@@ -805,6 +805,8 @@ func deliveryCommand(ctx context.Context, args []string) error {
 	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)")
 	group := set.String("group", "", "a delivery group's id")
 	ready := set.Bool("ready", false, "the head's pull request is marked ready: its check is queued before every other check "+
 		"(novox/hq ADR 0406 §3)")
@@ -819,6 +821,13 @@ func deliveryCommand(ctx context.Context, args []string) error {
 		return err
 	}
 	var members []orderMember
+	if *membersFile != "" {
+		raw, err := os.ReadFile(*membersFile)
+		if err != nil {
+			return fmt.Errorf("--members-file: %w", err)
+		}
+		*membersJSON = string(raw)
+	}
 	if *membersJSON != "" {
 		if err := json.Unmarshal([]byte(*membersJSON), &members); err != nil {
 			return fmt.Errorf("the members are not readable: %w", err)

cmd/mesh-controller/members_file_test.go

@@ -0,0 +1,93 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"slices"
+	"strings"
+	"testing"
+)
+
+// aLongGroup is members as JSON over 200 KB: 1666 paths of the shape of mesh-delivery #5's change (novox/hq issue 1272).
+func aLongGroup(t *testing.T) string {
+	t.Helper()
+	var paths []string
+	for i := 0; i < 1666; i++ {
+		paths = append(paths, fmt.Sprintf("modules/mesh-delivery-forge-bridge/vendor/github.com/nats-io/nats-server/v2/server/certstore/certstore_windows_file_%04d.go", i))
+	}
+	body, err := json.Marshal([]map[string]any{{"id": "novox/mesh-delivery#5", "paths": paths}, {"id": "novox/mesh-catalog#374"}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(body) < 200<<10 {
+		t.Fatalf("the members are only %d bytes", len(body))
+	}
+	return string(body)
+}
+
+func readBackFromTheCommand(t *testing.T, argv []string, members string) {
+	t.Helper()
+	for _, a := range argv {
+		if len(a) > 64<<10 {
+			t.Errorf("an argument of %d bytes is handed to the command", len(a))
+		}
+	}
+	i := slices.Index(argv, "--members-file")
+	if i < 0 {
+		t.Fatalf("the command does not name a members file: %.200v", argv)
+	}
+	raw, err := os.ReadFile(argv[i+1])
+	if err != nil || string(raw) != members {
+		t.Errorf("the file does not hold the whole members list (%d bytes, %v)", len(raw), err)
+	}
+	removeMembersFile(argv)
+	if _, err := os.Stat(argv[i+1]); !os.IsNotExist(err) {
+		t.Errorf("the file is still there after the command: %v", err)
+	}
+}
+
+func TestDeliveryCheckHandsALongMembersListInAFile(t *testing.T) { // 1272/AC1
+	members := aLongGroup(t)
+	argv, err := argvFor("delivery-check", map[string]any{"group": "feat/x", "members": members})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if argv[0] != "delivery" || argv[1] != "check" || !slices.Contains(argv, "--group") {
+		t.Errorf("the command is %.100v", strings.Join(argv, " "))
+	}
+	readBackFromTheCommand(t, argv, members)
+}
+
+func TestDeliveryOrderHandsALongMembersListInAFile(t *testing.T) { // 1272/AC2
+	members := aLongGroup(t)
+	argv, err := argvFor("delivery-order", map[string]any{"members": members})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if argv[0] != "delivery" || argv[1] != "order" {
+		t.Errorf("the command is %.100v", strings.Join(argv, " "))
+	}
+	readBackFromTheCommand(t, argv, members)
+}
+
+func TestAShortMembersListIsStillTheArgumentItWas(t *testing.T) { // 1272/AC3
+	members := `[{"id":"novox/a#1"},{"id":"novox/b#2"}]`
+	argv, err := argvFor("delivery-check", map[string]any{"group": "feat/x", "members": members, "ready": "true"})
+	want := []string{"delivery", "check", "--group", "feat/x", "--members", members, "--ready"}
+	if err != nil || !slices.Equal(argv, want) {
+		t.Errorf("a short list gives %v, %v, want %v", argv, err, want)
+	}
+}
+
+func TestTheCommandReadsTheMembersFromTheFileAsFromTheArgument(t *testing.T) { // 1272/AC1
+	members := `[{"id":"novox/a#1"}]`
+	argv, _ := membersFlags(strings.Repeat(" ", membersInArgument) + members)
+	defer removeMembersFile(argv)
+	if len(argv) != 2 || argv[0] != "--members-file" {
+		t.Fatalf("a long list gives %v", argv)
+	}
+	if st, err := os.Stat(argv[1]); err != nil || st.Mode().Perm() != 0o600 {
+		t.Errorf("the file is %v, %v", st, err)
+	}
+}

cmd/mesh-controller/seatverbs.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"slices"
 	"sort"
 	"strings"
@@ -408,14 +409,22 @@ func (a *verbArguments) commandLine() ([]string, error) {
 		if err := need("members"); err != nil {
 			return nil, err
 		}
-		return []string{"delivery", "order", "--members", str("members")}, nil
+		members, err := membersFlags(str("members"))
+		if err != nil {
+			return nil, err
+		}
+		return append([]string{"delivery", "order"}, members...), nil
 	case "delivery-check":
 		if err := need("members"); err != nil {
 			return nil, err
 		}
-		argv := []string{"delivery", "check", "--members", str("members")}
+		members, err := membersFlags(str("members"))
+		if err != nil {
+			return nil, err
+		}
+		argv := append([]string{"delivery", "check"}, members...)
 		if g := str("group"); g != "" {
-			argv = []string{"delivery", "check", "--group", g, "--members", str("members")}
+			argv = append([]string{"delivery", "check", "--group", g}, members...)
 		}
 		// Marked ready (novox/hq ADR 0406 §3): the check is queued before every other check.
 		if r := strings.ToLower(str("ready")); r != "" && r != "false" && r != "no" && r != "0" {
@@ -1193,6 +1202,7 @@ func runVerb(ctx context.Context, argv []string) (verbAnswer, error) {
 	if handingOver.Load() {
 		return verbAnswer{}, fmt.Errorf("%w: this controller is stopping and runs no new command", link.ErrHandingOver)
 	}
+	defer removeMembersFile(argv)
 	cmd := selfCommand(ctx, argv)
 	// And who asked, so an act it does by hand is recorded as theirs (novox/hq to-be 45 §7).
 	caller := link.CallerIn(ctx)
@@ -1858,3 +1868,41 @@ func terminalOnly(argv []string) error {
 		"decide who may become root on it (novox/hq ADR 0266). A verb may run node list and node show. "+
 		"Nothing was done", sub)
 }
+
+// membersInArgument is how long a group's members may be as one argument of the command a verb re-executes: over
+// the kernel's 128 KiB for one argument the command cannot start (novox/hq issue 1272).
+const membersInArgument = 32 << 10
+
+// membersDirPrefix names the private directory a long list of members is handed over in.
+const membersDirPrefix = "mesh-members-"
+
+// membersFlags is a group's members as the flags of the command a verb re-executes: `--members <json>` while the
+// list is short, as a person would type it, and `--members-file <path>` to a file in a private directory of its own
+// when it is not. runVerb removes the directory once the command has ended.
+func membersFlags(members string) ([]string, error) {
+	if len(members) <= membersInArgument {
+		return []string{"--members", members}, nil
+	}
+	dir, err := os.MkdirTemp("", membersDirPrefix)
+	if err != nil {
+		return nil, fmt.Errorf("the members cannot be handed to the command: %w", err)
+	}
+	file := filepath.Join(dir, "members.json")
+	if err := os.WriteFile(file, []byte(members), 0o600); err != nil {
+		os.RemoveAll(dir)
+		return nil, fmt.Errorf("the members cannot be handed to the command: %w", err)
+	}
+	return []string{"--members-file", file}, nil
+}
+
+// removeMembersFile removes the private directory membersFlags made for a command line, and only such a directory.
+func removeMembersFile(argv []string) {
+	for i, a := range argv {
+		if a == "--members-file" && i+1 < len(argv) {
+			dir := filepath.Dir(argv[i+1])
+			if strings.HasPrefix(filepath.Base(dir), membersDirPrefix) {
+				os.RemoveAll(dir)
+			}
+		}
+	}
+}