Skip to content

Commit 6c41b54

Browse files
committed
feat(cli): positional <id> args for generated commands
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.
1 parent a9ea711 commit 6c41b54

34 files changed

Lines changed: 1902 additions & 629 deletions

internal/cli/args.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,23 @@ func requireArgs(argNames ...string) cobra.PositionalArgs {
2424
}
2525
}
2626

27+
// requireExactArg returns a positional argument validator that requires exactly
28+
// one argument named name, producing friendly messages that match requireArgs style:
29+
//
30+
// - zero args: "missing <name>. Usage: ..."
31+
// - >1 args: "expects exactly one <name>. Usage: ..."
32+
func requireExactArg(name string) cobra.PositionalArgs {
33+
return func(cmd *cobra.Command, args []string) error {
34+
switch {
35+
case len(args) == 0:
36+
return fmt.Errorf("missing %s. Usage: %s", name, cmd.UseLine())
37+
case len(args) > 1:
38+
return fmt.Errorf("expects exactly one %s. Usage: %s", name, cmd.UseLine())
39+
}
40+
return nil
41+
}
42+
}
43+
2744
// requireExactlyOneFlag validates that exactly one of the named flags is set.
2845
func requireExactlyOneFlag(cmd *cobra.Command, flagNames ...string) error {
2946
set := 0
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
package cli
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
"testing"
7+
)
8+
9+
// TestGenPositionalUseLine asserts the generated --help Use line carries the
10+
// positional placeholder: an *_ids array op renders the singular id followed by
11+
// the variadic marker; an *_id scalar op renders the single id; the override and
12+
// int cases render their pinned field.
13+
func TestGenPositionalUseLine(t *testing.T) {
14+
// (a) and (b) read the generated constructors directly (the curated commands
15+
// own the live `incident ack`/`incident info` path-names, so the generated
16+
// twins are dropped at registration but still constructible for assertion).
17+
ack := genIncidentsAckCmd()
18+
if got := ack.Use; got != "ack <incident-id> [<id2>...]" {
19+
t.Errorf("ack twin Use = %q, want %q", got, "ack <incident-id> [<id2>...]")
20+
}
21+
if ack.Args == nil {
22+
t.Errorf("ack twin has no Args validator")
23+
}
24+
if err := ack.Args(ack, nil); err == nil {
25+
t.Errorf("ack twin Args accepted zero args (want >=1)")
26+
}
27+
if err := ack.Args(ack, []string{"id1"}); err != nil {
28+
t.Errorf("ack twin Args rejected one arg: %v", err)
29+
}
30+
31+
info := genIncidentsInfoCmd()
32+
if got := info.Use; got != "info <incident-id>" {
33+
t.Errorf("info twin Use = %q, want %q", got, "info <incident-id>")
34+
}
35+
if info.Args == nil {
36+
t.Errorf("info twin has no Args validator")
37+
}
38+
if err := info.Args(info, nil); err == nil {
39+
t.Errorf("info twin Args accepted zero args (want exactly one)")
40+
}
41+
if err := info.Args(info, []string{"id1"}); err != nil {
42+
t.Errorf("info twin Args rejected one arg: %v", err)
43+
}
44+
45+
// Override cases: merge pins target_incident_id (NOT source_incident_ids);
46+
// war-room detail pins chat_id.
47+
merge := genIncidentsMergeCmd()
48+
if got, want := merge.Use, "merge <target-incident-id>"; got != want {
49+
t.Errorf("merge override Use = %q, want %q", got, want)
50+
}
51+
if strings.Contains(merge.Use, "source-incident") {
52+
t.Errorf("merge override Use leaked source_incident_ids: %q", merge.Use)
53+
}
54+
detail := genIncidentsWarRoomDetailCmd()
55+
if got, want := detail.Use, "war-room-detail <chat-id>"; got != want {
56+
t.Errorf("war-room detail override Use = %q, want %q", got, want)
57+
}
58+
}
59+
60+
// TestGenPositionalScalarStringRuntime invokes a GENERATED-ONLY string-scalar
61+
// command (`field info <field-id>`) and asserts the positional folds into the
62+
// request body under the wire key.
63+
func TestGenPositionalScalarStringRuntime(t *testing.T) {
64+
saveAndResetGlobals(t)
65+
stub := newGFStub(t)
66+
67+
if _, err := execCommand("field", "info", "fld-123"); err != nil {
68+
t.Fatalf("execCommand field info: %v", err)
69+
}
70+
if stub.lastPath != "/field/info" {
71+
t.Fatalf("path = %q, want /field/info", stub.lastPath)
72+
}
73+
if got := stub.lastBody["field_id"]; got != "fld-123" {
74+
t.Errorf("field_id = %#v, want fld-123", got)
75+
}
76+
}
77+
78+
// TestGenPositionalSliceRuntime invokes a GENERATED-ONLY string-slice command
79+
// (`alert list-by-ids <alert-id> [<id2>...]`, whose alert_ids field is []string)
80+
// and asserts every positional arg folds into the wire array verbatim.
81+
func TestGenPositionalSliceRuntime(t *testing.T) {
82+
saveAndResetGlobals(t)
83+
stub := newGFStub(t)
84+
85+
if _, err := execCommand("alert", "list-by-ids", "a1", "a2", "a3"); err != nil {
86+
t.Fatalf("execCommand alert list-by-ids: %v", err)
87+
}
88+
if stub.lastPath != "/alert/list-by-ids" {
89+
t.Fatalf("path = %q, want /alert/list-by-ids", stub.lastPath)
90+
}
91+
if got, want := fmt.Sprint(stub.bodyStrings("alert_ids")), "[a1 a2 a3]"; got != want {
92+
t.Errorf("alert_ids = %q, want %q", got, want)
93+
}
94+
}
95+
96+
// TestGenPositionalIntSliceRuntime invokes a GENERATED-ONLY int-slice command
97+
// (`team infos <team-id> [<id2>...]`, whose team_ids field is []uint64) and
98+
// asserts each positional arg is PARSED to an int in the wire array. A raw
99+
// []string fold (the wrong kind) would fail SDK binding, so this guards the
100+
// intslice path specifically.
101+
func TestGenPositionalIntSliceRuntime(t *testing.T) {
102+
saveAndResetGlobals(t)
103+
stub := newGFStub(t)
104+
105+
if _, err := execCommand("team", "infos", "11", "22", "33"); err != nil {
106+
t.Fatalf("execCommand team infos: %v", err)
107+
}
108+
if stub.lastPath != "/team/infos" {
109+
t.Fatalf("path = %q, want /team/infos", stub.lastPath)
110+
}
111+
raw, ok := stub.lastBody["team_ids"].([]any)
112+
if !ok || len(raw) != 3 {
113+
t.Fatalf("team_ids = %#v, want a 3-element array", stub.lastBody["team_ids"])
114+
}
115+
// JSON numbers decode to float64 through the stub.
116+
for i, want := range []float64{11, 22, 33} {
117+
if got, _ := raw[i].(float64); got != want {
118+
t.Errorf("team_ids[%d] = %#v, want %v", i, raw[i], want)
119+
}
120+
}
121+
}
122+
123+
// TestGenPositionalIntRuntime invokes a GENERATED-ONLY int-scalar command
124+
// (`schedule info <schedule-id>`) and asserts the positional parses to an int
125+
// in the wire body (schedule_id is Int64Var, so genFoldPositional uses ParseInt).
126+
func TestGenPositionalIntRuntime(t *testing.T) {
127+
saveAndResetGlobals(t)
128+
stub := newGFStub(t)
129+
130+
// schedule info also needs --start/--end (relative-time required flags); supply
131+
// them so the command reaches the wire. The positional is the assertion target.
132+
if _, err := execCommand("schedule", "info", "4242", "--start", "now", "--end", "now"); err != nil {
133+
t.Fatalf("execCommand schedule info: %v", err)
134+
}
135+
if stub.lastPath != "/schedule/info" {
136+
t.Fatalf("path = %q, want /schedule/info", stub.lastPath)
137+
}
138+
// JSON numbers decode to float64 through the stub.
139+
if got, _ := stub.lastBody["schedule_id"].(float64); got != 4242 {
140+
t.Errorf("schedule_id = %#v, want 4242", stub.lastBody["schedule_id"])
141+
}
142+
}
143+
144+
// TestGenPositionalFlagOverridesPositional asserts the overlay order: an
145+
// explicitly-set typed flag for the same field wins over the positional arg
146+
// (positional folds first, the changed flag stamps after).
147+
func TestGenPositionalFlagOverridesPositional(t *testing.T) {
148+
saveAndResetGlobals(t)
149+
stub := newGFStub(t)
150+
151+
if _, err := execCommand("field", "info", "fromArg", "--field-id", "fromFlag"); err != nil {
152+
t.Fatalf("execCommand field info with flag: %v", err)
153+
}
154+
if got := stub.lastBody["field_id"]; got != "fromFlag" {
155+
t.Errorf("field_id = %#v, want fromFlag (explicit flag must override positional)", got)
156+
}
157+
}
158+
159+
// TestGenFoldPositional unit-tests the runtime helper across all three kinds.
160+
func TestGenFoldPositional(t *testing.T) {
161+
// string scalar → args[0]
162+
b := map[string]any{}
163+
if err := genFoldPositional([]string{"abc"}, b, "x_id", "string"); err != nil {
164+
t.Fatalf("string: %v", err)
165+
}
166+
if b["x_id"] != "abc" {
167+
t.Errorf("string: x_id = %#v, want abc", b["x_id"])
168+
}
169+
170+
// string slice → all args
171+
b = map[string]any{}
172+
if err := genFoldPositional([]string{"a", "b"}, b, "x_ids", "slice"); err != nil {
173+
t.Fatalf("slice: %v", err)
174+
}
175+
if got, want := fmt.Sprint(b["x_ids"]), "[a b]"; got != want {
176+
t.Errorf("slice: x_ids = %q, want %q", got, want)
177+
}
178+
179+
// int slice → each arg parsed to int64
180+
b = map[string]any{}
181+
if err := genFoldPositional([]string{"1", "2"}, b, "x_ids", "intslice"); err != nil {
182+
t.Fatalf("intslice: %v", err)
183+
}
184+
if got, want := fmt.Sprint(b["x_ids"]), "[1 2]"; got != want {
185+
t.Errorf("intslice: x_ids = %q, want %q", got, want)
186+
}
187+
if _, ok := b["x_ids"].([]int64); !ok {
188+
t.Errorf("intslice: x_ids type = %T, want []int64", b["x_ids"])
189+
}
190+
191+
// int slice with non-numeric arg → clean error
192+
b = map[string]any{}
193+
if err := genFoldPositional([]string{"1", "x"}, b, "x_ids", "intslice"); err == nil {
194+
t.Errorf("intslice: expected error on non-numeric arg, got nil")
195+
}
196+
197+
// int scalar → ParseInt
198+
b = map[string]any{}
199+
if err := genFoldPositional([]string{"77"}, b, "x_id", "int"); err != nil {
200+
t.Fatalf("int: %v", err)
201+
}
202+
if b["x_id"] != int64(77) {
203+
t.Errorf("int: x_id = %#v, want int64(77)", b["x_id"])
204+
}
205+
206+
// int scalar with non-numeric arg → clean error
207+
b = map[string]any{}
208+
if err := genFoldPositional([]string{"nope"}, b, "x_id", "int"); err == nil {
209+
t.Errorf("int: expected error on non-numeric arg, got nil")
210+
}
211+
}

internal/cli/gen_support.go

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"os"
88
"reflect"
9+
"strconv"
910
"strings"
1011

1112
"github.com/spf13/cobra"
@@ -47,11 +48,12 @@ func resolveDataSource(dataFlag string) (string, error) {
4748
// genAssembleBody builds a request body map from an optional --data JSON blob
4849
// overlaid with explicitly-set typed flags. Flags win over --data so an agent
4950
// can pass a JSON skeleton and override one field. setFlags is called after the
50-
// --data merge to stamp the changed scalar flags.
51+
// --data merge to stamp the changed scalar flags; it may return an error (e.g.
52+
// int-parse failure from a positional argument).
5153
//
5254
// The --data value accepts two source forms (see resolveDataSource): inline
5355
// JSON, or - to read STDIN.
54-
func genAssembleBody(dataFlag string, setFlags func(body map[string]any)) (map[string]any, error) {
56+
func genAssembleBody(dataFlag string, setFlags func(body map[string]any) error) (map[string]any, error) {
5557
dataJSON, err := resolveDataSource(dataFlag)
5658
if err != nil {
5759
return nil, err
@@ -62,7 +64,9 @@ func genAssembleBody(dataFlag string, setFlags func(body map[string]any)) (map[s
6264
return nil, fmt.Errorf("invalid --data JSON: %w", err)
6365
}
6466
}
65-
setFlags(body)
67+
if err := setFlags(body); err != nil {
68+
return nil, err
69+
}
6670
return body, nil
6771
}
6872

@@ -84,6 +88,56 @@ func curatedLong(intro, service, method string) string {
8488
return intro
8589
}
8690

91+
// genFoldPositional folds a generated command's positional argument(s) into the
92+
// request body under wire, BEFORE the typed flags are stamped. The flag for the
93+
// same field is kept; folding the positional first lets an explicitly-set flag
94+
// still override it (matching genAssembleBody's --data-then-flags overlay order).
95+
//
96+
// kind selects how args map onto the body, matching the emitted flag type:
97+
//
98+
// "string" — string scalar → body[wire] = args[0]
99+
// "int" — int64 scalar → body[wire] = ParseInt(args[0]) (schedule_id)
100+
// "slice" — []string variadic → body[wire] = args (string ids)
101+
// "intslice" — []int64 variadic → body[wire] = [ParseInt(a) for a in args]
102+
// (channel_ids, team_ids, … whose SDK field is []uint64)
103+
//
104+
// args is the validated positional slice; the cobra Args validator (requireArgs)
105+
// guarantees the arity below before RunE runs, but the bounds checks keep this
106+
// safe if it is ever called directly. A no-positional command never calls this.
107+
func genFoldPositional(args []string, body map[string]any, wire, kind string) error {
108+
switch kind {
109+
case "slice":
110+
if len(args) > 0 {
111+
body[wire] = args
112+
}
113+
case "intslice":
114+
if len(args) > 0 {
115+
ids := make([]int64, len(args))
116+
for i, a := range args {
117+
n, err := strconv.ParseInt(a, 10, 64)
118+
if err != nil {
119+
return fmt.Errorf("invalid %s %q: must be an integer", wire, a)
120+
}
121+
ids[i] = n
122+
}
123+
body[wire] = ids
124+
}
125+
case "int":
126+
if len(args) > 0 {
127+
n, err := strconv.ParseInt(args[0], 10, 64)
128+
if err != nil {
129+
return fmt.Errorf("invalid %s %q: must be an integer", wire, args[0])
130+
}
131+
body[wire] = n
132+
}
133+
default: // "string"
134+
if len(args) > 0 {
135+
body[wire] = args[0]
136+
}
137+
}
138+
return nil
139+
}
140+
87141
// genBindBody marshals the assembled body map into the typed request struct so
88142
// the call benefits from the SDK's wire encoding (nullable pointers, etc.).
89143
//

internal/cli/monit_agent.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,12 @@ keys in --data.
7878
// Assemble the body the standard way: --data (inline JSON or -
7979
// stdin) overlaid with the typed --target-* flags, mirroring
8080
// genAssembleBody's "typed flags override --data keys".
81-
body, err := genAssembleBody(dataJSON, func(body map[string]any) {
81+
body, err := genAssembleBody(dataJSON, func(body map[string]any) error {
8282
body["target_locator"] = targetLocator
8383
if cmd.Flags().Changed("target-kind") {
8484
body["target_kind"] = targetKind
8585
}
86+
return nil
8687
})
8788
if err != nil {
8889
return err

0 commit comments

Comments
 (0)