feat(monit-agent): accept --data body from stdin; drop --tool-spec DSL

internal/cli/command_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"strings"
 	"testing"
 
@@ -22,6 +23,7 @@ func saveAndResetGlobals(t *testing.T) {
 	origFlagNoTrunc := flagNoTrunc
 	origFlagAppKey := flagAppKey
 	origFlagBaseURL := flagBaseURL
+	origStdinReader := stdinReader
 
 	// Reset to defaults so tests start clean.
 	flagJSON = false
@@ -35,6 +37,7 @@ func saveAndResetGlobals(t *testing.T) {
 		flagNoTrunc = origFlagNoTrunc
 		flagAppKey = origFlagAppKey
 		flagBaseURL = origFlagBaseURL
+		stdinReader = origStdinReader
 	})
 }
 
@@ -845,6 +848,76 @@ func TestCommandAuditSearchPageUsesCursorPagination(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// CLI-wide --data source forms (inline / stdin), proven on a generated command
+// ---------------------------------------------------------------------------
+
+// A generated command reads its body from STDIN when --data is exactly "-".
+func TestCommandDataFromStdin(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+	stub.data = []any{} // /monit/datasource/list returns a top-level array
+
+	stdinReader = strings.NewReader(`{"type":"prometheus"}`)
+
+	_, err := execCommand("monit", "datasource-list", "--data", "-")
+	if err != nil {
+		t.Fatalf("[data-stdin] unexpected error: %v", err)
+	}
+	if stub.lastPath != "/monit/datasource/list" {
+		t.Fatalf("[data-stdin] expected /monit/datasource/list, got %q", stub.lastPath)
+	}
+	if stub.lastBody["type"] != "prometheus" {
+		t.Errorf("[data-stdin] expected type=prometheus from stdin, got %#v", stub.lastBody["type"])
+	}
+}
+
+// Inline --data still works, and a typed flag overrides a matching --data key.
+func TestCommandDataInlineFlagOverride(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+	stub.data = []any{} // /monit/datasource/list returns a top-level array
+
+	_, err := execCommand(
+		"monit", "datasource-list",
+		"--data", `{"type":"loki"}`,
+		"--type", "prometheus",
+	)
+	if err != nil {
+		t.Fatalf("[data-inline] unexpected error: %v", err)
+	}
+	if stub.lastBody["type"] != "prometheus" {
+		t.Errorf("[data-inline] expected typed --type to win over --data, got %#v", stub.lastBody["type"])
+	}
+}
+
+// With --data absent, stdin is NEVER read (guards against the empty-pipe hang).
+// A non-blocking sentinel reader fails the test if it is ever consumed.
+func TestCommandNoDataDoesNotReadStdin(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+	stub.data = []any{} // /monit/datasource/list returns a top-level array
+
+	stdinReader = readerFunc(func([]byte) (int, error) {
+		t.Fatal("[no-data] stdin was read despite --data being absent")
+		return 0, io.EOF
+	})
+
+	_, err := execCommand("monit", "datasource-list", "--type", "mysql")
+	if err != nil {
+		t.Fatalf("[no-data] unexpected error: %v", err)
+	}
+	if stub.lastBody["type"] != "mysql" {
+		t.Errorf("[no-data] expected type=mysql, got %#v", stub.lastBody["type"])
+	}
+}
+
+// readerFunc adapts a function to io.Reader so a test can assert Read is never
+// called.
+type readerFunc func([]byte) (int, error)
+
+func (f readerFunc) Read(p []byte) (int, error) { return f(p) }
+
 // ---------------------------------------------------------------------------
 // Helpers
 // ---------------------------------------------------------------------------

internal/cli/gen_support.go

@@ -3,12 +3,40 @@ package cli
 import (
 	"encoding/json"
 	"fmt"
+	"io"
+	"os"
 	"reflect"
 	"strings"
 
 	"github.com/spf13/cobra"
 )
 
+// stdinReader is the source read when --data is exactly "-". A package var so
+// tests can substitute a buffer (mirrors newClientFn); production reads os.Stdin.
+var stdinReader io.Reader = os.Stdin
+
+// resolveDataSource turns a --data flag value into the raw JSON body string,
+// supporting two source forms across EVERY --data-bearing command:
+//
+//	--data '<inline json>'  → returned verbatim
+//	--data -                → contents of STDIN
+//
+// STDIN is read ONLY when the flag is exactly "-"; an empty/absent --data is
+// never treated as a stdin request, so commands driven purely by typed flags
+// don't block on an empty pipe. Reading from STDIN lets callers pipe a quoted
+// heredoc, avoiding shell-quoting hell for JSON bodies that contain commas or
+// quotes (e.g. SQL in params).
+func resolveDataSource(dataFlag string) (string, error) {
+	if dataFlag == "-" {
+		b, err := io.ReadAll(stdinReader)
+		if err != nil {
+			return "", fmt.Errorf("failed to read --data from stdin: %w", err)
+		}
+		return string(b), nil
+	}
+	return dataFlag, nil
+}
+
 // This file is the hand-written runtime support for the generated commands in
 // zz_generated_*.go (produced by internal/cmd/cligen). Generated files stay
 // pure data + wiring; all shared behavior lives here so it can be reviewed and
@@ -18,7 +46,14 @@ import (
 // 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.
-func genAssembleBody(dataJSON string, setFlags func(body map[string]any)) (map[string]any, error) {
+//
+// 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) {
+	dataJSON, err := resolveDataSource(dataFlag)
+	if err != nil {
+		return nil, err
+	}
 	body := map[string]any{}
 	if dataJSON != "" {
 		if err := json.Unmarshal([]byte(dataJSON), &body); err != nil {

internal/cli/helpers.go

@@ -1,11 +1,8 @@
 package cli
 
 import (
-	"encoding/json"
 	"fmt"
 	"strings"
-
-	"github.com/flashcatcloud/go-flashduty"
 )
 
 // parseKVSlice converts a slice of "KEY=VALUE" entries into a map.
@@ -25,45 +22,3 @@ func parseKVSlice(entries []string) (map[string]string, error) {
 	}
 	return out, nil
 }
-
-// parseToolSpecs converts a slice of "name=<tool>[,params=<json>]" specs into
-// go-flashduty ToolInvokeRequestToolsItem entries. The `name` key is required;
-// `params` is optional and defaults to an empty object. Splits each spec on ','
-// first then on the first '=', mirroring parseKVSlice — that means params JSON
-// containing commas isn't supported; specs with complex params must keep their
-// objects single-keyed.
-func parseToolSpecs(specs []string) ([]flashduty.ToolInvokeRequestToolsItem, error) {
-	out := make([]flashduty.ToolInvokeRequestToolsItem, 0, len(specs))
-	for _, s := range specs {
-		var name string
-		var rawParams string
-		for _, kv := range strings.Split(s, ",") {
-			i := strings.IndexByte(kv, '=')
-			if i < 0 {
-				return nil, fmt.Errorf("missing '=' in %q", kv)
-			}
-			k, v := kv[:i], kv[i+1:]
-			switch k {
-			case "name":
-				name = v
-			case "params":
-				rawParams = v
-			default:
-				return nil, fmt.Errorf("unknown key %q in tool-spec", k)
-			}
-		}
-		if name == "" {
-			return nil, fmt.Errorf("missing name= in spec %q", s)
-		}
-		// go-flashduty models params as a decoded object. Default to an empty
-		// map so no-arg tools serialize as `{}`.
-		params := map[string]any{}
-		if rawParams != "" {
-			if err := json.Unmarshal([]byte(rawParams), &params); err != nil {
-				return nil, fmt.Errorf("invalid params JSON in spec %q: %w", s, err)
-			}
-		}
-		out = append(out, flashduty.ToolInvokeRequestToolsItem{Tool: name, Params: params})
-	}
-	return out, nil
-}

internal/cli/monit_agent.go

@@ -51,33 +51,60 @@ func newMonitAgentCatalogCmd() *cobra.Command {
 func newMonitAgentInvokeCmd() *cobra.Command {
 	var (
 		targetKind, targetLocator string
-		toolSpecs                 []string
+		dataJSON                  string
 	)
 
 	cmd := &cobra.Command{
 		Use:   "invoke",
 		Short: "Run up to 8 monit-agent tools concurrently on a target",
-		Long:  curatedLong("Run up to 8 monit-agent diagnostic tools concurrently on a target and return their output.", "Diagnostics", "ToolsInvoke"),
+		Long: curatedLong(`Run up to 8 monit-agent diagnostic tools concurrently on a target and return their output.
+
+The tools to run are carried in the --data request body:
+  --data '{"tools":[{"tool":"<name>","params":{<obj>}}, ... up to 8]}'
+params is optional and defaults to {}. --data also accepts - to read stdin,
+which avoids shell-quoting hell for params JSON that contains commas or quotes
+(e.g. SQL). --target-locator (required) and --target-kind override any matching
+keys in --data.
+
+  # heredoc form for quoted/comma SQL:
+  fduty monit-agent invoke --target-locator 'X' --data - <<'FDUTY'
+  {"tools":[{"tool":"mysql.query","params":{"sql":"SELECT a, b FROM t WHERE s='RUNNING'","max_rows":50}}]}
+  FDUTY`, "Diagnostics", "ToolsInvoke"),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if targetLocator == "" {
 				return fmt.Errorf("--target-locator is required")
 			}
-			if len(toolSpecs) == 0 {
-				return fmt.Errorf("--tool-spec is required (repeatable; up to 8)")
-			}
-			if len(toolSpecs) > 8 {
-				return fmt.Errorf("--tool-spec accepts up to 8 entries (got %d)", len(toolSpecs))
+
+			// 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["target_locator"] = targetLocator
+				if cmd.Flags().Changed("target-kind") {
+					body["target_kind"] = targetKind
+				}
+			})
+			if err != nil {
+				return err
 			}
-			parsed, err := parseToolSpecs(toolSpecs)
+
+			tools, err := parseInvokeTools(body["tools"])
 			if err != nil {
-				return fmt.Errorf("invalid --tool-spec: %w", err)
+				return err
+			}
+			if len(tools) == 0 {
+				return fmt.Errorf(`--data must carry a non-empty "tools" array, e.g. --data '{"tools":[{"tool":"os.overview"}]}'`)
+			}
+			if len(tools) > 8 {
+				return fmt.Errorf("at most 8 tools may be invoked at once (got %d)", len(tools))
 			}
 
 			return runCommand(cmd, args, func(ctx *RunContext) error {
+				kind, _ := body["target_kind"].(string)
 				input := &flashduty.ToolInvokeRequest{
-					TargetKind:    targetKind,
+					TargetKind:    kind,
 					TargetLocator: targetLocator,
-					Tools:         parsed,
+					Tools:         tools,
 				}
 				result, _, err := ctx.Client.Diagnostics.ToolsInvoke(cmdContext(ctx.Cmd), input)
 				if err != nil {
@@ -90,9 +117,42 @@ func newMonitAgentInvokeCmd() *cobra.Command {
 
 	cmd.Flags().StringVar(&targetKind, "target-kind", "", "Target kind (host|mysql|redis|…); omit to let the agent infer")
 	cmd.Flags().StringVar(&targetLocator, "target-locator", "", "Target locator: internal IP, hostname, or data-source name (required)")
-	// Use StringArray (not StringSlice) so commas inside params=<json> aren't
-	// mis-parsed as CSV separators — each --tool-spec entry is taken verbatim.
-	cmd.Flags().StringArrayVar(&toolSpecs, "tool-spec", nil, "Tool spec 'name=<tool>[,params=<json>]' (repeatable, max 8)")
+	cmd.Flags().StringVar(&dataJSON, "data", "", `Request body as JSON carrying the tools to run: {"tools":[{"tool":"<name>","params":{<obj>}}, ... max 8]}. Accepts inline JSON, or - to read stdin.`)
 
 	return cmd
 }
+
+// parseInvokeTools converts the decoded "tools" value from the --data body into
+// SDK tool items. Each entry must be an object with a non-empty "tool" string;
+// "params" is optional and defaults to an empty object so no-arg tools serialize
+// as `{}`.
+func parseInvokeTools(raw any) ([]flashduty.ToolInvokeRequestToolsItem, error) {
+	if raw == nil {
+		return nil, nil
+	}
+	arr, ok := raw.([]any)
+	if !ok {
+		return nil, fmt.Errorf(`"tools" must be a JSON array of {"tool":...,"params":...} objects`)
+	}
+	out := make([]flashduty.ToolInvokeRequestToolsItem, 0, len(arr))
+	for i, e := range arr {
+		obj, ok := e.(map[string]any)
+		if !ok {
+			return nil, fmt.Errorf(`tools[%d] must be an object with a "tool" key`, i)
+		}
+		name, _ := obj["tool"].(string)
+		if name == "" {
+			return nil, fmt.Errorf(`tools[%d] is missing a non-empty "tool" name`, i)
+		}
+		params := map[string]any{}
+		if p, ok := obj["params"]; ok && p != nil {
+			m, ok := p.(map[string]any)
+			if !ok {
+				return nil, fmt.Errorf(`tools[%d].params must be a JSON object`, i)
+			}
+			params = m
+		}
+		out = append(out, flashduty.ToolInvokeRequestToolsItem{Tool: name, Params: params})
+	}
+	return out, nil
+}