feat(cli): positional <id> args for generated commands (#50)

Generated commands were flag/--data-only, which blocked removing the curated
ID-based shadows (incident/alert verbs) without ergonomic regression. Add
positional-argument support in the generator: a generated command whose request
has a required *_id (scalar) or *_ids (array) body field now accepts that id
positionally (folded onto the existing flag at runtime; the flag and --data
still work and take precedence).

- Selection: required *_id/*_ids, array-wins, with a per-op override map
  (incident merge -> target_incident_id; war-room detail/add-member -> chat_id).
- Suppress positional on `create` verbs (the required id is a parent ref) and on
  rule-move (its rule-ids field is `ids`, not `*_ids`).
- Scalar positionals require exactly one arg; arrays require >=1; friendly errors
  mirror curated requireArgs (new requireExactArg sibling).
- intslice fold for []uint64 id fields (team_ids/person_ids/role_ids/...).
- gen_support.go genFoldPositional helper; selectPositional unit-tested.

123 positionals across 250 generated commands; non-breaking (no curated command
behavior changed); live-verified on testback. Foundation for dropping the clean
curated ID-verb shadows in a reviewed follow-up.

Author: ysyneu

internal/cli/args.go

@@ -24,6 +24,23 @@ func requireArgs(argNames ...string) cobra.PositionalArgs {
 	}
 }
 
+// requireExactArg returns a positional argument validator that requires exactly
+// one argument named name, producing friendly messages that match requireArgs style:
+//
+//   - zero args: "missing <name>. Usage: ..."
+//   - >1 args:   "expects exactly one <name>. Usage: ..."
+func requireExactArg(name string) cobra.PositionalArgs {
+	return func(cmd *cobra.Command, args []string) error {
+		switch {
+		case len(args) == 0:
+			return fmt.Errorf("missing %s. Usage: %s", name, cmd.UseLine())
+		case len(args) > 1:
+			return fmt.Errorf("expects exactly one %s. Usage: %s", name, cmd.UseLine())
+		}
+		return nil
+	}
+}
+
 // requireExactlyOneFlag validates that exactly one of the named flags is set.
 func requireExactlyOneFlag(cmd *cobra.Command, flagNames ...string) error {
 	set := 0

internal/cli/gen_positional_test.go

@@ -0,0 +1,211 @@
+package cli
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+// TestGenPositionalUseLine asserts the generated --help Use line carries the
+// positional placeholder: an *_ids array op renders the singular id followed by
+// the variadic marker; an *_id scalar op renders the single id; the override and
+// int cases render their pinned field.
+func TestGenPositionalUseLine(t *testing.T) {
+	// (a) and (b) read the generated constructors directly (the curated commands
+	// own the live `incident ack`/`incident info` path-names, so the generated
+	// twins are dropped at registration but still constructible for assertion).
+	ack := genIncidentsAckCmd()
+	if got := ack.Use; got != "ack <incident-id> [<id2>...]" {
+		t.Errorf("ack twin Use = %q, want %q", got, "ack <incident-id> [<id2>...]")
+	}
+	if ack.Args == nil {
+		t.Errorf("ack twin has no Args validator")
+	}
+	if err := ack.Args(ack, nil); err == nil {
+		t.Errorf("ack twin Args accepted zero args (want >=1)")
+	}
+	if err := ack.Args(ack, []string{"id1"}); err != nil {
+		t.Errorf("ack twin Args rejected one arg: %v", err)
+	}
+
+	info := genIncidentsInfoCmd()
+	if got := info.Use; got != "info <incident-id>" {
+		t.Errorf("info twin Use = %q, want %q", got, "info <incident-id>")
+	}
+	if info.Args == nil {
+		t.Errorf("info twin has no Args validator")
+	}
+	if err := info.Args(info, nil); err == nil {
+		t.Errorf("info twin Args accepted zero args (want exactly one)")
+	}
+	if err := info.Args(info, []string{"id1"}); err != nil {
+		t.Errorf("info twin Args rejected one arg: %v", err)
+	}
+
+	// Override cases: merge pins target_incident_id (NOT source_incident_ids);
+	// war-room detail pins chat_id.
+	merge := genIncidentsMergeCmd()
+	if got, want := merge.Use, "merge <target-incident-id>"; got != want {
+		t.Errorf("merge override Use = %q, want %q", got, want)
+	}
+	if strings.Contains(merge.Use, "source-incident") {
+		t.Errorf("merge override Use leaked source_incident_ids: %q", merge.Use)
+	}
+	detail := genIncidentsWarRoomDetailCmd()
+	if got, want := detail.Use, "war-room-detail <chat-id>"; got != want {
+		t.Errorf("war-room detail override Use = %q, want %q", got, want)
+	}
+}
+
+// TestGenPositionalScalarStringRuntime invokes a GENERATED-ONLY string-scalar
+// command (`field info <field-id>`) and asserts the positional folds into the
+// request body under the wire key.
+func TestGenPositionalScalarStringRuntime(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+
+	if _, err := execCommand("field", "info", "fld-123"); err != nil {
+		t.Fatalf("execCommand field info: %v", err)
+	}
+	if stub.lastPath != "/field/info" {
+		t.Fatalf("path = %q, want /field/info", stub.lastPath)
+	}
+	if got := stub.lastBody["field_id"]; got != "fld-123" {
+		t.Errorf("field_id = %#v, want fld-123", got)
+	}
+}
+
+// TestGenPositionalSliceRuntime invokes a GENERATED-ONLY string-slice command
+// (`alert list-by-ids <alert-id> [<id2>...]`, whose alert_ids field is []string)
+// and asserts every positional arg folds into the wire array verbatim.
+func TestGenPositionalSliceRuntime(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+
+	if _, err := execCommand("alert", "list-by-ids", "a1", "a2", "a3"); err != nil {
+		t.Fatalf("execCommand alert list-by-ids: %v", err)
+	}
+	if stub.lastPath != "/alert/list-by-ids" {
+		t.Fatalf("path = %q, want /alert/list-by-ids", stub.lastPath)
+	}
+	if got, want := fmt.Sprint(stub.bodyStrings("alert_ids")), "[a1 a2 a3]"; got != want {
+		t.Errorf("alert_ids = %q, want %q", got, want)
+	}
+}
+
+// TestGenPositionalIntSliceRuntime invokes a GENERATED-ONLY int-slice command
+// (`team infos <team-id> [<id2>...]`, whose team_ids field is []uint64) and
+// asserts each positional arg is PARSED to an int in the wire array. A raw
+// []string fold (the wrong kind) would fail SDK binding, so this guards the
+// intslice path specifically.
+func TestGenPositionalIntSliceRuntime(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+
+	if _, err := execCommand("team", "infos", "11", "22", "33"); err != nil {
+		t.Fatalf("execCommand team infos: %v", err)
+	}
+	if stub.lastPath != "/team/infos" {
+		t.Fatalf("path = %q, want /team/infos", stub.lastPath)
+	}
+	raw, ok := stub.lastBody["team_ids"].([]any)
+	if !ok || len(raw) != 3 {
+		t.Fatalf("team_ids = %#v, want a 3-element array", stub.lastBody["team_ids"])
+	}
+	// JSON numbers decode to float64 through the stub.
+	for i, want := range []float64{11, 22, 33} {
+		if got, _ := raw[i].(float64); got != want {
+			t.Errorf("team_ids[%d] = %#v, want %v", i, raw[i], want)
+		}
+	}
+}
+
+// TestGenPositionalIntRuntime invokes a GENERATED-ONLY int-scalar command
+// (`schedule info <schedule-id>`) and asserts the positional parses to an int
+// in the wire body (schedule_id is Int64Var, so genFoldPositional uses ParseInt).
+func TestGenPositionalIntRuntime(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+
+	// schedule info also needs --start/--end (relative-time required flags); supply
+	// them so the command reaches the wire. The positional is the assertion target.
+	if _, err := execCommand("schedule", "info", "4242", "--start", "now", "--end", "now"); err != nil {
+		t.Fatalf("execCommand schedule info: %v", err)
+	}
+	if stub.lastPath != "/schedule/info" {
+		t.Fatalf("path = %q, want /schedule/info", stub.lastPath)
+	}
+	// JSON numbers decode to float64 through the stub.
+	if got, _ := stub.lastBody["schedule_id"].(float64); got != 4242 {
+		t.Errorf("schedule_id = %#v, want 4242", stub.lastBody["schedule_id"])
+	}
+}
+
+// TestGenPositionalFlagOverridesPositional asserts the overlay order: an
+// explicitly-set typed flag for the same field wins over the positional arg
+// (positional folds first, the changed flag stamps after).
+func TestGenPositionalFlagOverridesPositional(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+
+	if _, err := execCommand("field", "info", "fromArg", "--field-id", "fromFlag"); err != nil {
+		t.Fatalf("execCommand field info with flag: %v", err)
+	}
+	if got := stub.lastBody["field_id"]; got != "fromFlag" {
+		t.Errorf("field_id = %#v, want fromFlag (explicit flag must override positional)", got)
+	}
+}
+
+// TestGenFoldPositional unit-tests the runtime helper across all three kinds.
+func TestGenFoldPositional(t *testing.T) {
+	// string scalar → args[0]
+	b := map[string]any{}
+	if err := genFoldPositional([]string{"abc"}, b, "x_id", "string"); err != nil {
+		t.Fatalf("string: %v", err)
+	}
+	if b["x_id"] != "abc" {
+		t.Errorf("string: x_id = %#v, want abc", b["x_id"])
+	}
+
+	// string slice → all args
+	b = map[string]any{}
+	if err := genFoldPositional([]string{"a", "b"}, b, "x_ids", "slice"); err != nil {
+		t.Fatalf("slice: %v", err)
+	}
+	if got, want := fmt.Sprint(b["x_ids"]), "[a b]"; got != want {
+		t.Errorf("slice: x_ids = %q, want %q", got, want)
+	}
+
+	// int slice → each arg parsed to int64
+	b = map[string]any{}
+	if err := genFoldPositional([]string{"1", "2"}, b, "x_ids", "intslice"); err != nil {
+		t.Fatalf("intslice: %v", err)
+	}
+	if got, want := fmt.Sprint(b["x_ids"]), "[1 2]"; got != want {
+		t.Errorf("intslice: x_ids = %q, want %q", got, want)
+	}
+	if _, ok := b["x_ids"].([]int64); !ok {
+		t.Errorf("intslice: x_ids type = %T, want []int64", b["x_ids"])
+	}
+
+	// int slice with non-numeric arg → clean error
+	b = map[string]any{}
+	if err := genFoldPositional([]string{"1", "x"}, b, "x_ids", "intslice"); err == nil {
+		t.Errorf("intslice: expected error on non-numeric arg, got nil")
+	}
+
+	// int scalar → ParseInt
+	b = map[string]any{}
+	if err := genFoldPositional([]string{"77"}, b, "x_id", "int"); err != nil {
+		t.Fatalf("int: %v", err)
+	}
+	if b["x_id"] != int64(77) {
+		t.Errorf("int: x_id = %#v, want int64(77)", b["x_id"])
+	}
+
+	// int scalar with non-numeric arg → clean error
+	b = map[string]any{}
+	if err := genFoldPositional([]string{"nope"}, b, "x_id", "int"); err == nil {
+		t.Errorf("int: expected error on non-numeric arg, got nil")
+	}
+}

internal/cli/gen_support.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"reflect"
+	"strconv"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -47,11 +48,12 @@ func resolveDataSource(dataFlag string) (string, error) {
 // genAssembleBody builds a request body map from an optional --data JSON blob
 // overlaid with explicitly-set typed flags. Flags win over --data so an agent
 // can pass a JSON skeleton and override one field. setFlags is called after the
-// --data merge to stamp the changed scalar flags.
+// --data merge to stamp the changed scalar flags; it may return an error (e.g.
+// int-parse failure from a positional argument).
 //
 // The --data value accepts two source forms (see resolveDataSource): inline
 // JSON, or - to read STDIN.
-func genAssembleBody(dataFlag string, setFlags func(body map[string]any)) (map[string]any, error) {
+func genAssembleBody(dataFlag string, setFlags func(body map[string]any) error) (map[string]any, error) {
 	dataJSON, err := resolveDataSource(dataFlag)
 	if err != nil {
 		return nil, err
@@ -62,7 +64,9 @@ func genAssembleBody(dataFlag string, setFlags func(body map[string]any)) (map[s
 			return nil, fmt.Errorf("invalid --data JSON: %w", err)
 		}
 	}
-	setFlags(body)
+	if err := setFlags(body); err != nil {
+		return nil, err
+	}
 	return body, nil
 }
 
@@ -84,6 +88,56 @@ func curatedLong(intro, service, method string) string {
 	return intro
 }
 
+// genFoldPositional folds a generated command's positional argument(s) into the
+// request body under wire, BEFORE the typed flags are stamped. The flag for the
+// same field is kept; folding the positional first lets an explicitly-set flag
+// still override it (matching genAssembleBody's --data-then-flags overlay order).
+//
+// kind selects how args map onto the body, matching the emitted flag type:
+//
+//	"string"   — string scalar     → body[wire] = args[0]
+//	"int"      — int64 scalar       → body[wire] = ParseInt(args[0]) (schedule_id)
+//	"slice"    — []string variadic  → body[wire] = args (string ids)
+//	"intslice" — []int64 variadic   → body[wire] = [ParseInt(a) for a in args]
+//	             (channel_ids, team_ids, … whose SDK field is []uint64)
+//
+// args is the validated positional slice; the cobra Args validator (requireArgs)
+// guarantees the arity below before RunE runs, but the bounds checks keep this
+// safe if it is ever called directly. A no-positional command never calls this.
+func genFoldPositional(args []string, body map[string]any, wire, kind string) error {
+	switch kind {
+	case "slice":
+		if len(args) > 0 {
+			body[wire] = args
+		}
+	case "intslice":
+		if len(args) > 0 {
+			ids := make([]int64, len(args))
+			for i, a := range args {
+				n, err := strconv.ParseInt(a, 10, 64)
+				if err != nil {
+					return fmt.Errorf("invalid %s %q: must be an integer", wire, a)
+				}
+				ids[i] = n
+			}
+			body[wire] = ids
+		}
+	case "int":
+		if len(args) > 0 {
+			n, err := strconv.ParseInt(args[0], 10, 64)
+			if err != nil {
+				return fmt.Errorf("invalid %s %q: must be an integer", wire, args[0])
+			}
+			body[wire] = n
+		}
+	default: // "string"
+		if len(args) > 0 {
+			body[wire] = args[0]
+		}
+	}
+	return nil
+}
+
 // genBindBody marshals the assembled body map into the typed request struct so
 // the call benefits from the SDK's wire encoding (nullable pointers, etc.).
 //

internal/cli/monit_agent.go

@@ -78,11 +78,12 @@ keys in --data.
 			// Assemble the body the standard way: --data (inline JSON or -
 			// stdin) overlaid with the typed --target-* flags, mirroring
 			// genAssembleBody's "typed flags override --data keys".
-			body, err := genAssembleBody(dataJSON, func(body map[string]any) {
+			body, err := genAssembleBody(dataJSON, func(body map[string]any) error {
 				body["target_locator"] = targetLocator
 				if cmd.Flags().Changed("target-kind") {
 					body["target_kind"] = targetKind
 				}
+				return nil
 			})
 			if err != nil {
 				return err