From 91cbf32fd1cf5dfb5fa94f054b8cfd4bc37bc510 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Fri, 3 Apr 2026 16:00:12 +0800 Subject: [PATCH] fix(cli): address code review feedback for agent/skill/runtime commands - Add --config flag to skill create/update (accepts JSON string) - Add --runtime-config flag to agent create/update (accepts JSON string) - Add --yes flag to skill delete with confirmation prompt - Improve agent skills set error message (guide users to --skill-ids '') - Validate --days range (1-365) in runtime usage - Include last known status in ping/update timeout errors --- server/cmd/multica/cmd_agent.go | 26 +++++++++++++++++---- server/cmd/multica/cmd_runtime.go | 7 ++++-- server/cmd/multica/cmd_skill.go | 38 +++++++++++++++++++++++++++++-- 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/server/cmd/multica/cmd_agent.go b/server/cmd/multica/cmd_agent.go index 11d4834c..a6ca6c2a 100644 --- a/server/cmd/multica/cmd_agent.go +++ b/server/cmd/multica/cmd_agent.go @@ -113,6 +113,7 @@ func init() { agentCreateCmd.Flags().String("description", "", "Agent description") agentCreateCmd.Flags().String("instructions", "", "Agent instructions") agentCreateCmd.Flags().String("runtime-id", "", "Runtime ID (required)") + agentCreateCmd.Flags().String("runtime-config", "", "Runtime config as JSON string") agentCreateCmd.Flags().String("visibility", "private", "Visibility: private or workspace") agentCreateCmd.Flags().Int32("max-concurrent-tasks", 6, "Maximum concurrent tasks") agentCreateCmd.Flags().String("output", "json", "Output format: table or json") @@ -122,6 +123,7 @@ func init() { agentUpdateCmd.Flags().String("description", "", "New description") agentUpdateCmd.Flags().String("instructions", "", "New instructions") agentUpdateCmd.Flags().String("runtime-id", "", "New runtime ID") + agentUpdateCmd.Flags().String("runtime-config", "", "New runtime config as JSON string") agentUpdateCmd.Flags().String("visibility", "", "New visibility: private or workspace") agentUpdateCmd.Flags().String("status", "", "New status") agentUpdateCmd.Flags().Int32("max-concurrent-tasks", 0, "New max concurrent tasks") @@ -314,6 +316,14 @@ func runAgentCreate(cmd *cobra.Command, _ []string) error { if v, _ := cmd.Flags().GetString("instructions"); v != "" { body["instructions"] = v } + if cmd.Flags().Changed("runtime-config") { + v, _ := cmd.Flags().GetString("runtime-config") + var rc any + if err := json.Unmarshal([]byte(v), &rc); err != nil { + return fmt.Errorf("--runtime-config must be valid JSON: %w", err) + } + body["runtime_config"] = rc + } if cmd.Flags().Changed("visibility") { v, _ := cmd.Flags().GetString("visibility") body["visibility"] = v @@ -363,6 +373,14 @@ func runAgentUpdate(cmd *cobra.Command, args []string) error { v, _ := cmd.Flags().GetString("runtime-id") body["runtime_id"] = v } + if cmd.Flags().Changed("runtime-config") { + v, _ := cmd.Flags().GetString("runtime-config") + var rc any + if err := json.Unmarshal([]byte(v), &rc); err != nil { + return fmt.Errorf("--runtime-config must be valid JSON: %w", err) + } + body["runtime_config"] = rc + } if cmd.Flags().Changed("visibility") { v, _ := cmd.Flags().GetString("visibility") body["visibility"] = v @@ -377,7 +395,7 @@ func runAgentUpdate(cmd *cobra.Command, args []string) error { } if len(body) == 0 { - return fmt.Errorf("no fields to update; use --name, --description, --instructions, --runtime-id, --visibility, --status, or --max-concurrent-tasks") + return fmt.Errorf("no fields to update; use --name, --description, --instructions, --runtime-id, --runtime-config, --visibility, --status, or --max-concurrent-tasks") } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) @@ -518,10 +536,10 @@ func runAgentSkillsSet(cmd *cobra.Command, args []string) error { return err } - skillIDs, _ := cmd.Flags().GetStringSlice("skill-ids") - if len(skillIDs) == 0 { - return fmt.Errorf("--skill-ids is required (comma-separated skill IDs, or empty string to clear)") + if !cmd.Flags().Changed("skill-ids") { + return fmt.Errorf("--skill-ids is required (comma-separated skill IDs; use --skill-ids '' to clear all)") } + skillIDs, _ := cmd.Flags().GetStringSlice("skill-ids") // Allow passing empty string to clear all skills. cleanIDs := make([]string, 0, len(skillIDs)) for _, id := range skillIDs { diff --git a/server/cmd/multica/cmd_runtime.go b/server/cmd/multica/cmd_runtime.go index b73593d4..417efec0 100644 --- a/server/cmd/multica/cmd_runtime.go +++ b/server/cmd/multica/cmd_runtime.go @@ -123,6 +123,9 @@ func runRuntimeUsage(cmd *cobra.Command, args []string) error { } days, _ := cmd.Flags().GetInt("days") + if days < 1 || days > 365 { + return fmt.Errorf("--days must be between 1 and 365") + } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() @@ -216,7 +219,7 @@ func runRuntimePing(cmd *cobra.Command, args []string) error { for { select { case <-ctx.Done(): - return fmt.Errorf("timed out waiting for ping") + return fmt.Errorf("timed out waiting for ping (last status: %s)", strVal(ping, "status")) case <-time.After(1 * time.Second): } @@ -278,7 +281,7 @@ func runRuntimeUpdate(cmd *cobra.Command, args []string) error { for { select { case <-ctx.Done(): - return fmt.Errorf("timed out waiting for update") + return fmt.Errorf("timed out waiting for update (last status: %s)", strVal(update, "status")) case <-time.After(2 * time.Second): } diff --git a/server/cmd/multica/cmd_skill.go b/server/cmd/multica/cmd_skill.go index 664de1c1..f6de394a 100644 --- a/server/cmd/multica/cmd_skill.go +++ b/server/cmd/multica/cmd_skill.go @@ -1,9 +1,12 @@ package main import ( + "bufio" "context" + "encoding/json" "fmt" "os" + "strings" "time" "github.com/spf13/cobra" @@ -106,15 +109,18 @@ func init() { skillCreateCmd.Flags().String("name", "", "Skill name (required)") skillCreateCmd.Flags().String("description", "", "Skill description") skillCreateCmd.Flags().String("content", "", "Skill content (SKILL.md body)") + skillCreateCmd.Flags().String("config", "", "Skill config as JSON string") skillCreateCmd.Flags().String("output", "json", "Output format: table or json") // skill update skillUpdateCmd.Flags().String("name", "", "New name") skillUpdateCmd.Flags().String("description", "", "New description") skillUpdateCmd.Flags().String("content", "", "New content") + skillUpdateCmd.Flags().String("config", "", "New config as JSON string") skillUpdateCmd.Flags().String("output", "json", "Output format: table or json") - // skill delete (no extra flags) + // skill delete + skillDeleteCmd.Flags().Bool("yes", false, "Skip confirmation prompt") // skill import skillImportCmd.Flags().String("url", "", "URL to import from (required)") @@ -216,6 +222,14 @@ func runSkillCreate(cmd *cobra.Command, _ []string) error { if v, _ := cmd.Flags().GetString("content"); v != "" { body["content"] = v } + if cmd.Flags().Changed("config") { + v, _ := cmd.Flags().GetString("config") + var config any + if err := json.Unmarshal([]byte(v), &config); err != nil { + return fmt.Errorf("--config must be valid JSON: %w", err) + } + body["config"] = config + } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() @@ -253,9 +267,17 @@ func runSkillUpdate(cmd *cobra.Command, args []string) error { v, _ := cmd.Flags().GetString("content") body["content"] = v } + if cmd.Flags().Changed("config") { + v, _ := cmd.Flags().GetString("config") + var config any + if err := json.Unmarshal([]byte(v), &config); err != nil { + return fmt.Errorf("--config must be valid JSON: %w", err) + } + body["config"] = config + } if len(body) == 0 { - return fmt.Errorf("no fields to update; use --name, --description, or --content") + return fmt.Errorf("no fields to update; use --name, --description, --content, or --config") } ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) @@ -276,6 +298,18 @@ func runSkillUpdate(cmd *cobra.Command, args []string) error { } func runSkillDelete(cmd *cobra.Command, args []string) error { + yes, _ := cmd.Flags().GetBool("yes") + if !yes { + fmt.Printf("Are you sure you want to delete skill %s? This cannot be undone. [y/N] ", args[0]) + reader := bufio.NewReader(os.Stdin) + answer, _ := reader.ReadString('\n') + answer = strings.TrimSpace(strings.ToLower(answer)) + if answer != "y" && answer != "yes" { + fmt.Println("Aborted.") + return nil + } + } + client, err := newAPIClient(cmd) if err != nil { return err