diff --git a/CHANGELOG.md b/CHANGELOG.md index 20254c7..30b26ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). --- -## [Unreleased] — Domain Coverage Expansion (P0–P3) +## [Unreleased] — Domain Coverage Expansion (P0–P5) ### Added @@ -34,9 +34,21 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `goclaw health` — uses WS RPC `health` when authenticated, retaining unauthenticated HTTP `/health` fallback. - `goclaw traces list --since --agent --status --root-only --limit` — expanded filters for automation-friendly trace search. +**P4 — UX polish** +- `goclaw codex-pool activity --agent=|--provider=` — unified Codex pool activity lookup; legacy agent/provider commands remain as deprecated aliases. +- `goclaw api-keys rotate ` — create replacement key, show raw key once, then revoke old key with structured partial-failure reporting. +- `goclaw config defaults` — read-only WS passthrough for server default config values. +- `goclaw chat replay --session=` and `goclaw chat sessions resume --session=` — discoverability wrappers over existing chat session contracts. +- `goclaw tools invoke --args=` — alias for `--params` with file-backed JSON support. + +**P5 — Residual command fillers** +- `goclaw teams attachments download --output ` — authenticated attachment download with required output path and no-overwrite default. +- `goclaw agents evolution skill apply [--skill-draft @file]` — explicit wrapper for approving `skill_add` suggestions through the server evolution approval route. +- `goclaw agents evolution update` now maps `--action=accept|reject` to the server-compatible `status=approved|rejected` payload. + ### Notes - All new commands honor the AI-first ergonomics contract: `--output=json` envelope, central error handler, `--yes` for destructive ops, `--quiet` for CI. -- P4/P5 backlog was re-swept against the current CLI surface; already-covered items were removed from residual scope before the next implementation pass. +- P4/P5 backlog was re-swept against the current CLI surface; already-covered items were removed from residual scope before implementation. - Out of scope: OpenAI-compatible `/chat/completions` and `/v1/responses` endpoints (client APIs, not admin CLI surface). --- diff --git a/README.md b/README.md index 155bd29..db99c34 100644 --- a/README.md +++ b/README.md @@ -54,15 +54,16 @@ echo "Analyze this log" | goclaw chat myagent |---------|-------------| | `auth` | Login, logout, device pairing, profile management | | `profile` | List, create, switch, inspect, and delete CLI profiles | -| `agents` | CRUD, shares, delegation links, per-user instances | +| `agents` | CRUD, shares, delegation links, per-user instances, evolution | | `chat` | Interactive or single-shot messaging with streaming | | `sessions` | List, preview, delete, reset, label, compact | +| `codex-pool` | Unified Codex pool activity lookup for agents/providers | | `skills` | Upload, manage, grant/revoke access | | `mcp` | MCP server management, grants, access requests | | `providers` | LLM provider CRUD, model listing, verification | | `tools` | Custom + built-in tool management, invocation | | `cron` | Scheduled jobs CRUD, trigger, run history | -| `teams` | Team management, task board, workspace | +| `teams` | Team management, task board, workspace, attachments | | `channels` | Channel instances, contacts, pending messages | | `traces` | LLM trace viewer, filters, export | | `memory` | Memory documents, semantic search | @@ -81,7 +82,7 @@ echo "Analyze this log" | goclaw chat myagent | `tts` | Text-to-speech operations | | `media` | Media upload/download | | `activity` | Audit log | -| `api-keys` | API key management (create, list, revoke) | +| `api-keys` | API key management (create, list, revoke, rotate) | | `system upgrade` | Gateway release upgrade status and trigger controls | | `workstations` | Coding-agent workstation CRUD, permissions, activity, and agent links | | `webhooks` | Webhook admin CRUD, secret rotation, and deletion | @@ -300,10 +301,37 @@ goclaw api-keys list # Revoke a key goclaw api-keys revoke + +# Rotate a key by creating a replacement and revoking the old key +goclaw api-keys rotate --name "ci-deploy-v2" --scopes "operator.read,operator.write" --yes ``` Available scopes: `operator.admin`, `operator.read`, `operator.write`, `operator.approvals`, `operator.pairing` +## UX Convenience Commands + +```bash +# Unified Codex pool activity +goclaw codex-pool activity --agent=agent-123 +goclaw codex-pool activity --provider=provider-123 + +# Resolved server defaults +goclaw config defaults -o json + +# Replay or resume a known chat session +goclaw chat replay myagent --session=sess-123 -o json +goclaw chat sessions resume myagent --session=sess-123 -m "Continue" --no-stream + +# Invoke a custom tool with JSON args from file +goclaw tools invoke weather --args=@payload.json + +# Download a team task attachment to an explicit file +goclaw teams attachments download team-123 attachment-456 --output ./artifact.bin + +# Approve a skill_add evolution suggestion, optionally overriding the draft +goclaw agents evolution skill apply agent-123 suggestion-456 --skill-draft @./SKILL.md +``` + ## API Docs ```bash diff --git a/cmd/agents.go b/cmd/agents.go index e1cb6ae..51584ab 100644 --- a/cmd/agents.go +++ b/cmd/agents.go @@ -42,11 +42,11 @@ var agentsListCmd = &cobra.Command{ return err } if cfg.OutputFormat != "table" { - printer.Print(unmarshalList(data)) + printer.Print(unmarshalNamedList(data, "agents")) return nil } tbl := output.NewTable("ID", "KEY", "NAME", "PROVIDER", "MODEL", "STATUS", "TYPE") - for _, a := range unmarshalList(data) { + for _, a := range unmarshalNamedList(data, "agents") { tbl.AddRow(str(a, "id"), str(a, "agent_key"), str(a, "display_name"), str(a, "provider"), str(a, "model"), str(a, "status"), str(a, "agent_type")) } diff --git a/cmd/agents_evolution.go b/cmd/agents_evolution.go index b1e942a..9a128df 100644 --- a/cmd/agents_evolution.go +++ b/cmd/agents_evolution.go @@ -1,7 +1,9 @@ package cmd import ( + "encoding/json" "fmt" + "net/url" "github.com/spf13/cobra" ) @@ -85,26 +87,104 @@ Example: if err != nil { return err } + status := map[string]string{ + "accept": "approved", + "reject": "rejected", + }[action] _, err = c.Patch( - fmt.Sprintf("/v1/agents/%s/evolution/suggestions/%s", args[0], args[1]), - map[string]any{"action": action}, + fmt.Sprintf( + "/v1/agents/%s/evolution/suggestions/%s", + url.PathEscape(args[0]), + url.PathEscape(args[1]), + ), + map[string]any{"status": status}, ) if err != nil { return err } - printer.Success(fmt.Sprintf("Suggestion %s: %sd", args[1], action)) + printer.Success(fmt.Sprintf("Suggestion %s %s", args[1], status)) return nil }, } +var agentsEvolutionSkillCmd = &cobra.Command{ + Use: "skill", + Short: "Apply skill evolution suggestions", +} + +var agentsEvolutionSkillApplyCmd = &cobra.Command{ + Use: "apply ", + Short: "Approve a skill_add evolution suggestion", + Long: `Approve a skill_add evolution suggestion for an agent. + +PATCH /v1/agents/{id}/evolution/suggestions/{suggestionID} + +Example: + goclaw agents evolution skill apply agent-1 sugg-42 + goclaw agents evolution skill apply agent-1 sugg-42 --skill-draft @./SKILL.md`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + body := map[string]any{"status": "approved"} + if cmd.Flags().Changed("skill-draft") { + draft, _ := cmd.Flags().GetString("skill-draft") + content, err := readContent(draft) + if err != nil { + return err + } + body["skill_draft"] = content + } + c, err := newHTTP() + if err != nil { + return err + } + if err := requireSkillAddSuggestion(c, args[0], args[1]); err != nil { + return err + } + data, err := c.Patch( + fmt.Sprintf( + "/v1/agents/%s/evolution/suggestions/%s", + url.PathEscape(args[0]), + url.PathEscape(args[1]), + ), + body, + ) + if err != nil { + return err + } + printer.Print(unmarshalMap(data)) + return nil + }, +} + +func requireSkillAddSuggestion(c interface { + Get(path string) (json.RawMessage, error) +}, agentID, suggestionID string) error { + data, err := c.Get("/v1/agents/" + url.PathEscape(agentID) + "/evolution/suggestions?status=pending&limit=500") + if err != nil { + return err + } + for _, suggestion := range unmarshalList(data) { + if str(suggestion, "id") == suggestionID { + if str(suggestion, "suggestion_type") != "skill_add" { + return fmt.Errorf("suggestion %s is %q, not skill_add", suggestionID, str(suggestion, "suggestion_type")) + } + return nil + } + } + return fmt.Errorf("suggestion %s not found in agent evolution suggestions", suggestionID) +} + func init() { agentsEvolutionUpdateCmd.Flags().String("action", "", "Action: accept or reject") _ = agentsEvolutionUpdateCmd.MarkFlagRequired("action") + agentsEvolutionSkillApplyCmd.Flags().String("skill-draft", "", "Skill draft content or @file") + agentsEvolutionSkillCmd.AddCommand(agentsEvolutionSkillApplyCmd) agentsEvolutionCmd.AddCommand( agentsEvolutionMetricsCmd, agentsEvolutionSuggestionsCmd, agentsEvolutionUpdateCmd, + agentsEvolutionSkillCmd, ) agentsCmd.AddCommand(agentsEvolutionCmd) } diff --git a/cmd/agents_misc.go b/cmd/agents_misc.go index f6b7fc2..fdb45ba 100644 --- a/cmd/agents_misc.go +++ b/cmd/agents_misc.go @@ -35,8 +35,9 @@ Example: } var agentsCodexPoolActivityCmd = &cobra.Command{ - Use: "codex-pool-activity ", - Short: "Get codex pool activity for an agent", + Use: "codex-pool-activity ", + Short: "Get codex pool activity for an agent", + Deprecated: "use 'goclaw codex-pool activity --agent=' instead", Long: `Retrieve recent codex (context pool) activity for an agent. GET /v1/agents/{id}/codex-pool-activity diff --git a/cmd/api_keys.go b/cmd/api_keys.go index 9a8caef..45e7d54 100644 --- a/cmd/api_keys.go +++ b/cmd/api_keys.go @@ -59,16 +59,9 @@ var apiKeysCreateCmd = &cobra.Command{ scopesRaw, _ := cmd.Flags().GetString("scopes") expiresIn, _ := cmd.Flags().GetInt("expires-in") - // Parse comma-separated scopes into slice - var scopes []string - for _, s := range strings.Split(scopesRaw, ",") { - s = strings.TrimSpace(s) - if s != "" { - scopes = append(scopes, s) - } - } - if len(scopes) == 0 { - return fmt.Errorf("at least one scope is required") + scopes, err := parseAPIKeyScopes(scopesRaw) + if err != nil { + return err } body := buildBody("name", name, "scopes", scopes) diff --git a/cmd/api_keys_helpers.go b/cmd/api_keys_helpers.go new file mode 100644 index 0000000..24641bd --- /dev/null +++ b/cmd/api_keys_helpers.go @@ -0,0 +1,20 @@ +package cmd + +import ( + "fmt" + "strings" +) + +func parseAPIKeyScopes(scopesRaw string) ([]string, error) { + var scopes []string + for _, s := range strings.Split(scopesRaw, ",") { + s = strings.TrimSpace(s) + if s != "" { + scopes = append(scopes, s) + } + } + if len(scopes) == 0 { + return nil, fmt.Errorf("at least one scope is required") + } + return scopes, nil +} diff --git a/cmd/api_keys_rotate.go b/cmd/api_keys_rotate.go new file mode 100644 index 0000000..6d2d281 --- /dev/null +++ b/cmd/api_keys_rotate.go @@ -0,0 +1,85 @@ +package cmd + +import ( + "fmt" + "net/url" + + "github.com/nextlevelbuilder/goclaw-cli/internal/output" + "github.com/nextlevelbuilder/goclaw-cli/internal/tui" + "github.com/spf13/cobra" +) + +var apiKeysRotateCmd = &cobra.Command{ + Use: "rotate ", + Short: "Create a replacement API key and revoke the old key", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if !tui.Confirm("Rotate this API key?", cfg.Yes) { + return nil + } + c, err := newHTTP() + if err != nil { + return err + } + name, _ := cmd.Flags().GetString("name") + scopesRaw, _ := cmd.Flags().GetString("scopes") + expiresIn, _ := cmd.Flags().GetInt("expires-in") + scopes, err := parseAPIKeyScopes(scopesRaw) + if err != nil { + return err + } + + body := buildBody("name", name, "scopes", scopes) + if expiresIn > 0 { + body["expires_in"] = expiresIn + } + + data, err := c.Post("/v1/api-keys", body) + if err != nil { + return err + } + result := unmarshalMap(data) + printAPIKeyRotateResult(args[0], result) + + _, err = c.Post("/v1/api-keys/"+url.PathEscape(args[0])+"/revoke", nil) + if err != nil { + return apiKeyRotatePartialError(args[0], result, err) + } + return nil + }, +} + +func printAPIKeyRotateResult(oldKeyID string, result map[string]any) { + if cfg.OutputFormat == "table" { + fmt.Printf("API key rotated: %s\n", str(result, "id")) + fmt.Println("--- IMPORTANT: Copy your replacement API key now. It will not be shown again. ---") + fmt.Printf("Key: %s\n", str(result, "key")) + return + } + result["old_key_id"] = oldKeyID + result["old_revoke_status"] = "pending" + printer.Print(result) +} + +func apiKeyRotatePartialError(oldKeyID string, result map[string]any, revokeErr error) error { + details := map[string]any{ + "new_key_id": str(result, "id"), + "old_key_id": oldKeyID, + "old_revoke_status": "failed", + "revoke_error": revokeErr.Error(), + } + return &output.ErrorDetail{ + Code: "INTERNAL", + Message: "replacement API key was created, but revoking the old key failed", + Details: details, + } +} + +func init() { + apiKeysRotateCmd.Flags().String("name", "", "Human-readable replacement key name") + _ = apiKeysRotateCmd.MarkFlagRequired("name") + apiKeysRotateCmd.Flags().String("scopes", "", "Comma-separated replacement key scopes") + _ = apiKeysRotateCmd.MarkFlagRequired("scopes") + apiKeysRotateCmd.Flags().Int("expires-in", 0, "TTL in seconds (0 = no expiry)") + apiKeysCmd.AddCommand(apiKeysRotateCmd) +} diff --git a/cmd/chat_ai_commands.go b/cmd/chat_ai_commands.go index 145e62f..1cced47 100644 --- a/cmd/chat_ai_commands.go +++ b/cmd/chat_ai_commands.go @@ -43,32 +43,7 @@ Examples: before, _ := cmd.Flags().GetString("before") session, _ := cmd.Flags().GetString("session") - ws, err := newWS("cli") - if err != nil { - return err - } - if _, err := ws.Connect(); err != nil { - return err - } - defer ws.Close() - - params := map[string]any{ - "agent_key": args[0], - "limit": limit, - } - if before != "" { - params["before"] = before - } - if session != "" { - params["session_key"] = session - } - - data, err := ws.Call("chat.history", params) - if err != nil { - return err - } - printer.Print(unmarshalList(data)) - return nil + return runChatHistory(args[0], session, before, limit) }, } diff --git a/cmd/chat_replay.go b/cmd/chat_replay.go new file mode 100644 index 0000000..4d988fb --- /dev/null +++ b/cmd/chat_replay.go @@ -0,0 +1,52 @@ +package cmd + +import "github.com/spf13/cobra" + +var chatReplayCmd = &cobra.Command{ + Use: "replay ", + Short: "Replay history for a specific chat session", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + session, _ := cmd.Flags().GetString("session") + before, _ := cmd.Flags().GetString("before") + limit, _ := cmd.Flags().GetInt("limit") + return runChatHistory(args[0], session, before, limit) + }, +} + +func runChatHistory(agent, session, before string, limit int) error { + ws, err := newWS("cli") + if err != nil { + return err + } + if _, err := ws.Connect(); err != nil { + return err + } + defer ws.Close() + + params := map[string]any{ + "agent_key": agent, + "limit": limit, + } + if before != "" { + params["before"] = before + } + if session != "" { + params["session_key"] = session + } + + data, err := ws.Call("chat.history", params) + if err != nil { + return err + } + printer.Print(unmarshalList(data)) + return nil +} + +func init() { + chatReplayCmd.Flags().String("session", "", "Session key to replay") + _ = chatReplayCmd.MarkFlagRequired("session") + chatReplayCmd.Flags().String("before", "", "Return messages before this RFC3339 timestamp") + chatReplayCmd.Flags().Int("limit", 50, "Maximum number of messages to return") + chatCmd.AddCommand(chatReplayCmd) +} diff --git a/cmd/chat_sessions.go b/cmd/chat_sessions.go new file mode 100644 index 0000000..efb92fc --- /dev/null +++ b/cmd/chat_sessions.go @@ -0,0 +1,29 @@ +package cmd + +import "github.com/spf13/cobra" + +var chatSessionsCmd = &cobra.Command{Use: "sessions", Short: "Chat session convenience commands"} + +var chatSessionsResumeCmd = &cobra.Command{ + Use: "resume ", + Short: "Resume a chat session", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + session, _ := cmd.Flags().GetString("session") + message, _ := cmd.Flags().GetString("message") + noStream, _ := cmd.Flags().GetBool("no-stream") + if message != "" { + return chatSingleShot(args[0], message, session, noStream) + } + return chatInteractive(args[0], session) + }, +} + +func init() { + chatSessionsResumeCmd.Flags().String("session", "", "Session key to resume") + _ = chatSessionsResumeCmd.MarkFlagRequired("session") + chatSessionsResumeCmd.Flags().StringP("message", "m", "", "Message to send") + chatSessionsResumeCmd.Flags().Bool("no-stream", false, "Disable streaming, wait for full response") + chatSessionsCmd.AddCommand(chatSessionsResumeCmd) + chatCmd.AddCommand(chatSessionsCmd) +} diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index d432a91..914e96f 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -25,6 +25,7 @@ func TestAllCommandsRegistered(t *testing.T) { "chat", "config", "contacts", + "codex-pool", "credentials", "cron", "delegations", @@ -100,6 +101,7 @@ func TestCommandUseFields(t *testing.T) { {"chat"}, {"config"}, {"contacts"}, + {"codex-pool"}, {"credentials"}, {"cron"}, {"delegations"}, diff --git a/cmd/codex_pool.go b/cmd/codex_pool.go new file mode 100644 index 0000000..4e48a2d --- /dev/null +++ b/cmd/codex_pool.go @@ -0,0 +1,45 @@ +package cmd + +import ( + "fmt" + "net/url" + + "github.com/spf13/cobra" +) + +var codexPoolCmd = &cobra.Command{Use: "codex-pool", Short: "Inspect Codex pool activity"} + +var codexPoolActivityCmd = &cobra.Command{ + Use: "activity", + Short: "Show Codex pool activity for an agent or provider", + RunE: func(cmd *cobra.Command, args []string) error { + agentID, _ := cmd.Flags().GetString("agent") + providerID, _ := cmd.Flags().GetString("provider") + if (agentID == "") == (providerID == "") { + return fmt.Errorf("provide exactly one of --agent or --provider") + } + c, err := newHTTP() + if err != nil { + return err + } + path := "" + if agentID != "" { + path = "/v1/agents/" + url.PathEscape(agentID) + "/codex-pool-activity" + } else { + path = "/v1/providers/" + url.PathEscape(providerID) + "/codex-pool-activity" + } + data, err := c.Get(path) + if err != nil { + return err + } + printer.Print(unmarshalMap(data)) + return nil + }, +} + +func init() { + codexPoolActivityCmd.Flags().String("agent", "", "Agent ID") + codexPoolActivityCmd.Flags().String("provider", "", "Provider ID") + codexPoolCmd.AddCommand(codexPoolActivityCmd) + rootCmd.AddCommand(codexPoolCmd) +} diff --git a/cmd/config_defaults.go b/cmd/config_defaults.go new file mode 100644 index 0000000..084b3ad --- /dev/null +++ b/cmd/config_defaults.go @@ -0,0 +1,28 @@ +package cmd + +import "github.com/spf13/cobra" + +var configDefaultsCmd = &cobra.Command{ + Use: "defaults", + Short: "Show resolved server configuration defaults", + RunE: func(cmd *cobra.Command, args []string) error { + ws, err := newWS("cli") + if err != nil { + return err + } + if _, err := ws.Connect(); err != nil { + return err + } + defer ws.Close() + data, err := ws.Call("config.defaults", nil) + if err != nil { + return err + } + printer.Print(unmarshalMap(data)) + return nil + }, +} + +func init() { + configCmd.AddCommand(configDefaultsCmd) +} diff --git a/cmd/list_response_helpers.go b/cmd/list_response_helpers.go new file mode 100644 index 0000000..a8491bb --- /dev/null +++ b/cmd/list_response_helpers.go @@ -0,0 +1,41 @@ +package cmd + +import "encoding/json" + +// unmarshalNamedList handles endpoints that wrap arrays in an object envelope. +func unmarshalNamedList(data json.RawMessage, key string) []map[string]any { + if list := unmarshalList(data); list != nil { + return list + } + var envelope map[string]any + if err := json.Unmarshal(data, &envelope); err == nil { + return mapsFromAnyList(envelope[key]) + } + return nil +} + +func mapsFromAnyList(value any) []map[string]any { + switch list := value.(type) { + case []map[string]any: + return list + case []any: + out := make([]map[string]any, 0, len(list)) + for _, item := range list { + if m, ok := item.(map[string]any); ok { + out = append(out, m) + } + } + return out + default: + return nil + } +} + +func strFirst(m map[string]any, keys ...string) string { + for _, key := range keys { + if v := str(m, key); v != "" { + return v + } + } + return "" +} diff --git a/cmd/memory.go b/cmd/memory.go index 3b6eb99..0ce8b90 100644 --- a/cmd/memory.go +++ b/cmd/memory.go @@ -24,9 +24,11 @@ var memoryListCmd = &cobra.Command{ if err != nil { return err } - path := "/v1/memory/" + args[0] + path := "/v1/agents/" + url.PathEscape(args[0]) + "/memory/documents" if v, _ := cmd.Flags().GetString("user"); v != "" { - path += "?user_id=" + v + q := url.Values{} + q.Set("user_id", v) + path += "?" + q.Encode() } data, err := c.Get(path) if err != nil { @@ -54,7 +56,7 @@ var memoryGetCmd = &cobra.Command{ if err != nil { return err } - data, err := c.Get("/v1/memory/" + url.PathEscape(args[0]) + "/" + url.PathEscape(args[1])) + data, err := c.Get(memoryDocumentPath(args[0], args[1])) if err != nil { return err } @@ -77,7 +79,7 @@ var memoryStoreCmd = &cobra.Command{ if err != nil { return err } - _, err = c.Put("/v1/memory/"+url.PathEscape(args[0])+"/"+url.PathEscape(args[1]), + _, err = c.Put(memoryDocumentPath(args[0], args[1]), map[string]any{"content": content}) if err != nil { return err @@ -99,7 +101,7 @@ var memoryDeleteCmd = &cobra.Command{ if err != nil { return err } - _, err = c.Delete("/v1/memory/" + url.PathEscape(args[0]) + "/" + url.PathEscape(args[1])) + _, err = c.Delete(memoryDocumentPath(args[0], args[1])) if err != nil { return err } @@ -120,15 +122,19 @@ var memorySearchCmd = &cobra.Command{ query, _ := cmd.Flags().GetString("query") user, _ := cmd.Flags().GetString("user") body := buildBody("query", query, "user_id", user) - data, err := c.Post("/v1/memory/"+args[0]+"/search", body) + data, err := c.Post("/v1/agents/"+url.PathEscape(args[0])+"/memory/search", body) if err != nil { return err } - printer.Print(unmarshalList(data)) + printer.Print(unmarshalMap(data)) return nil }, } +func memoryDocumentPath(agentID, path string) string { + return "/v1/agents/" + url.PathEscape(agentID) + "/memory/documents/" + url.PathEscape(path) +} + func init() { memoryListCmd.Flags().String("user", "", "Filter by user ID") memoryStoreCmd.Flags().String("content", "", "Content (or @filepath)") diff --git a/cmd/p4_ux_polish_test.go b/cmd/p4_ux_polish_test.go new file mode 100644 index 0000000..9e4e500 --- /dev/null +++ b/cmd/p4_ux_polish_test.go @@ -0,0 +1,472 @@ +package cmd + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gorilla/websocket" + "github.com/nextlevelbuilder/goclaw-cli/internal/output" + "github.com/spf13/cobra" +) + +func TestCodexPoolActivityRoutes(t *testing.T) { + defer resetTestFlag(codexPoolActivityCmd, "agent", "") + defer resetTestFlag(codexPoolActivityCmd, "provider", "") + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + okJSON(t, w, map[string]any{"ok": true}) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + + if err := runCmd(t, "codex-pool", "activity", "--agent=agent-1"); err != nil { + t.Fatalf("codex-pool agent: %v", err) + } + _ = codexPoolActivityCmd.Flags().Set("agent", "") + if err := runCmd(t, "codex-pool", "activity", "--provider=provider-1"); err != nil { + t.Fatalf("codex-pool provider: %v", err) + } + want := []string{ + "/v1/agents/agent-1/codex-pool-activity", + "/v1/providers/provider-1/codex-pool-activity", + } + for i := range want { + if paths[i] != want[i] { + t.Fatalf("path[%d] = %q, want %q", i, paths[i], want[i]) + } + } +} + +func TestAPIKeysRotatePartialFailureMapsExitFive(t *testing.T) { + defer resetTestFlag(apiKeysRotateCmd, "name", "") + defer resetTestFlag(apiKeysRotateCmd, "scopes", "") + defer resetTestFlag(apiKeysRotateCmd, "expires-in", "0") + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + switch r.URL.Path { + case "/v1/api-keys": + okJSON(t, w, map[string]any{"id": "new-key", "key": "goclaw_new_raw", "name": "rotated"}) + case "/v1/api-keys/old-key/revoke": + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"ok":false,"error":{"code":"INTERNAL","message":"boom"}}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + t.Setenv("GOCLAW_OUTPUT", "json") + + stdout, err := captureStdout(t, func() error { + return runCmd(t, "api-keys", "rotate", "old-key", "--name=rotated", "--scopes=operator.read", "--yes") + }) + if err == nil { + t.Fatal("expected partial failure") + } + if strings.Count(stdout, "goclaw_new_raw") != 1 { + t.Fatalf("raw key count in stdout = %d, stdout = %s", strings.Count(stdout, "goclaw_new_raw"), stdout) + } + if got := output.FromError(err); got != output.ExitServer { + t.Fatalf("exit = %d, want %d", got, output.ExitServer) + } + var detail *output.ErrorDetail + if !errors.As(err, &detail) { + t.Fatalf("expected ErrorDetail, got %T", err) + } + details, ok := detail.Details.(map[string]any) + if !ok { + t.Fatalf("details = %#v", detail.Details) + } + if _, ok := details["new_key"]; ok { + t.Fatalf("partial failure details must not repeat raw key: %#v", details) + } + if details["new_key_id"] != "new-key" || details["old_key_id"] != "old-key" || + details["old_revoke_status"] != "failed" { + t.Fatalf("details = %#v", details) + } + if len(paths) != 2 || paths[0] != "/v1/api-keys" || paths[1] != "/v1/api-keys/old-key/revoke" { + t.Fatalf("paths = %#v", paths) + } +} + +func TestAPIKeysCreateUsesSharedScopeParser(t *testing.T) { + defer resetTestFlag(apiKeysCreateCmd, "name", "") + defer resetTestFlag(apiKeysCreateCmd, "scopes", "") + defer resetTestFlag(apiKeysCreateCmd, "expires-in", "0") + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/api-keys" { + w.WriteHeader(http.StatusNotFound) + return + } + _ = json.NewDecoder(r.Body).Decode(&body) + okJSON(t, w, map[string]any{"id": "new-key", "key": "goclaw_new_raw"}) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + t.Setenv("GOCLAW_OUTPUT", "json") + + err := runCmd(t, "api-keys", "create", "--name=ci", "--scopes= operator.read, , operator.write ") + if err != nil { + t.Fatalf("api-keys create: %v", err) + } + scopes := body["scopes"].([]any) + if len(scopes) != 2 || scopes[0] != "operator.read" || scopes[1] != "operator.write" { + t.Fatalf("scopes = %#v", scopes) + } +} + +func TestLegacyCodexPoolAliasesUseCobraDeprecationMetadata(t *testing.T) { + if agentsCodexPoolActivityCmd.Deprecated == "" { + t.Fatal("agents codex-pool-activity should be marked deprecated") + } + if providersCodexPoolActivityCmd.Deprecated == "" { + t.Fatal("providers codex-pool-activity should be marked deprecated") + } +} + +func TestConfigDefaultsUsesWSMethod(t *testing.T) { + seen := make(chan string, 2) + srv := mockRPCServer(t, seen) + defer srv.Close() + setupP3CommandTest(srv.URL) + + if err := configDefaultsCmd.RunE(configDefaultsCmd, nil); err != nil { + t.Fatalf("config defaults: %v", err) + } + <-seen // connect + if method := <-seen; method != "config.defaults" { + t.Fatalf("method = %q", method) + } +} + +func TestToolsInvokeArgsReadsFile(t *testing.T) { + defer resetTestFlag(toolsInvokeCmd, "args", "") + defer resetTestFlag(toolsInvokeCmd, "param", "") + defer resetTestFlag(toolsInvokeCmd, "agent", "") + defer resetTestFlag(toolsInvokeCmd, "action", "") + defer resetTestFlag(toolsInvokeCmd, "session", "") + defer resetTestFlag(toolsInvokeCmd, "dry-run", "false") + var body map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/tools/invoke" { + w.WriteHeader(http.StatusNotFound) + return + } + _ = json.NewDecoder(r.Body).Decode(&body) + okJSON(t, w, map[string]any{"ok": true}) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + + path := filepath.Join(t.TempDir(), "args.json") + if err := os.WriteFile(path, []byte(`{"city":"Saigon"}`), 0o600); err != nil { + t.Fatalf("write args: %v", err) + } + if err := runCmd(t, "tools", "invoke", "weather", "--args=@"+path, "--param=unit=c", + "--agent=goclaw", "--action=forecast", "--session=sess-1", "--dry-run"); err != nil { + t.Fatalf("tools invoke: %v", err) + } + if body["tool"] != "weather" { + t.Fatalf("tool = %#v", body["tool"]) + } + if body["agentId"] != "goclaw" || body["action"] != "forecast" || + body["sessionKey"] != "sess-1" || body["dryRun"] != true { + t.Fatalf("context fields = %#v", body) + } + params := body["args"].(map[string]any) + if params["city"] != "Saigon" || params["unit"] != "c" { + t.Fatalf("params = %#v", params) + } +} + +func TestToolsCustomUnsupportedDoesNotCallServer(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + + err := runCmd(t, "tools", "custom", "list") + if err == nil { + t.Fatal("expected unsupported custom tools error") + } + var detail *output.ErrorDetail + if !errors.As(err, &detail) || detail.Code != "INVALID_REQUEST" { + t.Fatalf("error = %#v, want INVALID_REQUEST detail", err) + } + if requests != 0 { + t.Fatalf("requests = %d, want 0", requests) + } +} + +func TestUsageTimeseriesMapsFlagsToServerContract(t *testing.T) { + defer resetTestFlag(usageTimeseriesCmd, "start", "") + defer resetTestFlag(usageTimeseriesCmd, "end", "") + defer resetTestFlag(usageTimeseriesCmd, "granularity", "day") + defer resetTestFlag(usageTimeseriesCmd, "agent", "") + var query url.Values + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/usage/timeseries" { + w.WriteHeader(http.StatusNotFound) + return + } + query = r.URL.Query() + okJSON(t, w, map[string]any{"series": []map[string]any{}}) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + + if err := runCmd(t, "usage", "timeseries", "--start=2026-05-20", "--end=2026-05-21", "--granularity=day", "--agent=agent-1"); err != nil { + t.Fatalf("usage timeseries: %v", err) + } + if query.Get("from") != "2026-05-20T00:00:00Z" || query.Get("to") != "2026-05-21T00:00:00Z" || + query.Get("group_by") != "day" || query.Get("agent_id") != "agent-1" { + t.Fatalf("query = %#v", query) + } + if query.Has("start") || query.Has("end") || query.Has("granularity") || query.Has("agent") { + t.Fatalf("query contains legacy keys: %#v", query) + } +} + +func TestUsageBreakdownMapsFlagsToServerContract(t *testing.T) { + defer resetTestFlag(usageBreakdownCmd, "start", "") + defer resetTestFlag(usageBreakdownCmd, "end", "") + defer resetTestFlag(usageBreakdownCmd, "by", "agent") + var query url.Values + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/usage/breakdown" { + w.WriteHeader(http.StatusNotFound) + return + } + query = r.URL.Query() + okJSON(t, w, map[string]any{"groups": []map[string]any{}}) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + + if err := runCmd(t, "usage", "breakdown", "--start=2026-05-20", "--end=2026-05-21", "--by=provider"); err != nil { + t.Fatalf("usage breakdown: %v", err) + } + if query.Get("from") != "2026-05-20T00:00:00Z" || query.Get("to") != "2026-05-21T00:00:00Z" || + query.Get("group_by") != "provider" { + t.Fatalf("query = %#v", query) + } + if query.Has("start") || query.Has("end") || query.Has("by") { + t.Fatalf("query contains legacy keys: %#v", query) + } +} + +func TestMemoryCommandsUseAgentScopedHTTPRoutes(t *testing.T) { + defer resetTestFlag(memoryListCmd, "user", "") + defer resetTestFlag(memorySearchCmd, "query", "") + defer resetTestFlag(memorySearchCmd, "user", "") + var paths []string + var searchBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.String()) + switch r.URL.Path { + case "/v1/agents/goclaw/memory/documents": + okJSON(t, w, []map[string]any{{"path": "notes.md"}}) + case "/v1/agents/goclaw/memory/search": + _ = json.NewDecoder(r.Body).Decode(&searchBody) + okJSON(t, w, map[string]any{"results": []map[string]any{}, "count": 0}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + + if err := runCmd(t, "memory", "list", "goclaw", "--user=system"); err != nil { + t.Fatalf("memory list: %v", err) + } + if err := runCmd(t, "memory", "search", "goclaw", "--query=test"); err != nil { + t.Fatalf("memory search: %v", err) + } + if len(paths) != 2 || paths[0] != "/v1/agents/goclaw/memory/documents?user_id=system" || + paths[1] != "/v1/agents/goclaw/memory/search" { + t.Fatalf("paths = %#v", paths) + } + if searchBody["query"] != "test" { + t.Fatalf("search body = %#v", searchBody) + } +} + +func TestListCommandsAcceptObjectWrappedLists(t *testing.T) { + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + switch r.URL.Path { + case "/v1/agents": + okJSON(t, w, map[string]any{"agents": []map[string]any{{"id": "agent-1"}}}) + case "/v1/sessions": + okJSON(t, w, map[string]any{"sessions": []map[string]any{{"session_key": "sess-1"}}}) + case "/v1/tools/builtin": + okJSON(t, w, map[string]any{"tools": []map[string]any{{"name": "sessions_list"}}}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + t.Setenv("GOCLAW_OUTPUT", "json") + + agentsOut, err := captureStdout(t, func() error { + return runCmd(t, "agents", "list") + }) + if err != nil { + t.Fatalf("agents list: %v", err) + } + sessionsOut, err := captureStdout(t, func() error { + return runCmd(t, "sessions", "list") + }) + if err != nil { + t.Fatalf("sessions list: %v", err) + } + toolsOut, err := captureStdout(t, func() error { + return runCmd(t, "tools", "builtin", "list") + }) + if err != nil { + t.Fatalf("tools builtin list: %v", err) + } + if strings.Contains(agentsOut, "null") || !strings.Contains(agentsOut, "agent-1") { + t.Fatalf("agents stdout = %q", agentsOut) + } + if strings.Contains(sessionsOut, "null") || !strings.Contains(sessionsOut, "sess-1") { + t.Fatalf("sessions stdout = %q", sessionsOut) + } + if strings.Contains(toolsOut, "null") || !strings.Contains(toolsOut, "sessions_list") { + t.Fatalf("tools stdout = %q", toolsOut) + } + if len(paths) != 3 || paths[0] != "/v1/agents" || paths[1] != "/v1/sessions" || + paths[2] != "/v1/tools/builtin" { + t.Fatalf("paths = %#v", paths) + } +} + +func TestChatReplayAndResumeUseExistingWSContracts(t *testing.T) { + defer resetTestFlag(chatReplayCmd, "session", "") + defer resetTestFlag(chatReplayCmd, "before", "") + defer resetTestFlag(chatReplayCmd, "limit", "50") + defer resetTestFlag(chatSessionsResumeCmd, "session", "") + defer resetTestFlag(chatSessionsResumeCmd, "message", "") + defer resetTestFlag(chatSessionsResumeCmd, "no-stream", "false") + seen := make(chan wsCall, 4) + srv := mockCaptureRPCServer(t, seen) + defer srv.Close() + setupP3CommandTest(srv.URL) + + _ = chatReplayCmd.Flags().Set("session", "sess-1") + _ = chatReplayCmd.Flags().Set("before", "") + _ = chatReplayCmd.Flags().Set("limit", "50") + if err := chatReplayCmd.RunE(chatReplayCmd, []string{"agent-1"}); err != nil { + t.Fatalf("chat replay: %v", err) + } + <-seen // connect + history := <-seen + if history.Method != "chat.history" || history.Params["agent_key"] != "agent-1" || + history.Params["session_key"] != "sess-1" { + t.Fatalf("history call = %#v", history) + } + + _ = chatSessionsResumeCmd.Flags().Set("session", "sess-2") + _ = chatSessionsResumeCmd.Flags().Set("message", "hi") + _ = chatSessionsResumeCmd.Flags().Set("no-stream", "true") + if err := chatSessionsResumeCmd.RunE(chatSessionsResumeCmd, []string{"agent-1"}); err != nil { + t.Fatalf("chat sessions resume: %v", err) + } + <-seen // connect + send := <-seen + if send.Method != "chat.send" || send.Params["agent_key"] != "agent-1" || + send.Params["session_key"] != "sess-2" || send.Params["message"] != "hi" { + t.Fatalf("send call = %#v", send) + } +} + +func resetTestFlag(cmd *cobra.Command, name, value string) { + flag := cmd.Flags().Lookup(name) + if flag == nil { + return + } + _ = flag.Value.Set(value) + flag.Changed = false +} + +func captureStdout(t *testing.T, fn func() error) (string, error) { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + fnErr := fn() + _ = w.Close() + os.Stdout = old + data, readErr := io.ReadAll(r) + _ = r.Close() + if readErr != nil { + t.Fatalf("read stdout: %v", readErr) + } + return string(data), fnErr +} + +type wsCall struct { + Method string + Params map[string]any +} + +func mockCaptureRPCServer(t *testing.T, seen chan<- wsCall) *httptest.Server { + t.Helper() + upgrader := websocket.Upgrader{CheckOrigin: func(r *http.Request) bool { return true }} + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + t.Fatalf("ws upgrade: %v", err) + } + defer conn.Close() + for { + var req struct { + ID string `json:"id"` + Method string `json:"method"` + Params map[string]any `json:"params"` + } + if err := conn.ReadJSON(&req); err != nil { + return + } + seen <- wsCall{Method: req.Method, Params: req.Params} + payload, _ := json.Marshal([]map[string]any{{"content": "ok"}}) + if req.Method == "chat.send" || req.Method == "connect" { + payload, _ = json.Marshal(map[string]any{"content": "ok"}) + } + _ = conn.WriteJSON(map[string]any{ + "type": "res", + "id": req.ID, + "ok": true, + "payload": json.RawMessage(payload), + }) + } + })) +} diff --git a/cmd/p5_fillers_test.go b/cmd/p5_fillers_test.go new file mode 100644 index 0000000..d5f9ff3 --- /dev/null +++ b/cmd/p5_fillers_test.go @@ -0,0 +1,283 @@ +package cmd + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/nextlevelbuilder/goclaw-cli/internal/config" + "github.com/nextlevelbuilder/goclaw-cli/internal/output" + "github.com/spf13/cobra" +) + +func setupP5HTTPTest(serverURL string) { + cfg = &config.Config{Server: serverURL, Token: "test-token", OutputFormat: "json"} + printer = output.NewPrinter("json") +} + +func resetFlag(t *testing.T, cmd *cobra.Command, name string) { + t.Helper() + flag := cmd.Flags().Lookup(name) + if flag == nil { + t.Fatalf("missing flag %s", name) + } + _ = flag.Value.Set(flag.DefValue) + flag.Changed = false +} + +func TestTeamsAttachmentsDownloadWritesOutputAndSendsAuth(t *testing.T) { + body := []byte("attachment-data") + outFile := filepath.Join(t.TempDir(), "nested", "artifact.bin") + var gotPath, gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + gotAuth = r.Header.Get("Authorization") + if r.Method != http.MethodGet { + t.Errorf("expected GET, got %s", r.Method) + } + _, _ = w.Write(body) + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, teamsAttachmentsDownloadCmd, "output") + resetFlag(t, teamsAttachmentsDownloadCmd, "force") + _ = teamsAttachmentsDownloadCmd.Flags().Set("output", outFile) + + if err := teamsAttachmentsDownloadCmd.RunE(teamsAttachmentsDownloadCmd, []string{"team alpha", "att/42"}); err != nil { + t.Fatalf("download: %v", err) + } + if gotPath != "/v1/teams/team%20alpha/attachments/att%2F42/download" { + t.Fatalf("path = %q", gotPath) + } + if gotAuth != "Bearer test-token" { + t.Fatalf("authorization = %q", gotAuth) + } + got, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read output: %v", err) + } + if string(got) != string(body) { + t.Fatalf("output = %q", got) + } +} + +func TestTeamsAttachmentsDownloadRequiresOutputBeforeNetwork(t *testing.T) { + called := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, teamsAttachmentsDownloadCmd, "output") + resetFlag(t, teamsAttachmentsDownloadCmd, "force") + + err := teamsAttachmentsDownloadCmd.RunE(teamsAttachmentsDownloadCmd, []string{"team-1", "att-1"}) + if err == nil || !strings.Contains(err.Error(), "--output is required") { + t.Fatalf("expected output validation error, got %v", err) + } + if called { + t.Fatal("server was called before output validation") + } +} + +func TestTeamsAttachmentsDownloadRefusesExistingFileUnlessForce(t *testing.T) { + outFile := filepath.Join(t.TempDir(), "artifact.bin") + if err := os.WriteFile(outFile, []byte("old"), 0644); err != nil { + t.Fatalf("seed file: %v", err) + } + called := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + _, _ = w.Write([]byte("new")) + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, teamsAttachmentsDownloadCmd, "output") + resetFlag(t, teamsAttachmentsDownloadCmd, "force") + _ = teamsAttachmentsDownloadCmd.Flags().Set("output", outFile) + + err := teamsAttachmentsDownloadCmd.RunE(teamsAttachmentsDownloadCmd, []string{"team-1", "att-1"}) + if err == nil || !strings.Contains(err.Error(), "already exists") { + t.Fatalf("expected overwrite guard, got %v", err) + } + if called { + t.Fatal("server was called before overwrite guard") + } + + _ = teamsAttachmentsDownloadCmd.Flags().Set("force", "true") + if err := teamsAttachmentsDownloadCmd.RunE(teamsAttachmentsDownloadCmd, []string{"team-1", "att-1"}); err != nil { + t.Fatalf("force download: %v", err) + } + got, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read output: %v", err) + } + if string(got) != "new" { + t.Fatalf("output = %q", got) + } +} + +func TestTeamsAttachmentsDownloadLocalOutputDoesNotOverrideFormat(t *testing.T) { + outFile := filepath.Join(t.TempDir(), "artifact.bin") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("attachment")) + })) + defer srv.Close() + t.Setenv("GOCLAW_SERVER", srv.URL) + t.Setenv("GOCLAW_TOKEN", "test-token") + t.Setenv("GOCLAW_OUTPUT", "json") + resetFlag(t, teamsAttachmentsDownloadCmd, "output") + resetFlag(t, teamsAttachmentsDownloadCmd, "force") + + if err := runCmd(t, "teams", "attachments", "download", "team-1", "att-1", "--output", outFile); err != nil { + t.Fatalf("download: %v", err) + } + if cfg.OutputFormat != "json" { + t.Fatalf("cfg.OutputFormat = %q, want json", cfg.OutputFormat) + } +} + +func TestAgentsEvolutionUpdateMapsActionToStatusAndEscapesRoute(t *testing.T) { + var gotPath string + var gotBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.EscapedPath() + if r.Method != http.MethodPatch { + t.Errorf("expected PATCH, got %s", r.Method) + } + if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Errorf("decode body: %v", err) + } + okJSON(t, w, map[string]any{"status": "approved"}) + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, agentsEvolutionUpdateCmd, "action") + _ = agentsEvolutionUpdateCmd.Flags().Set("action", "accept") + + if err := agentsEvolutionUpdateCmd.RunE(agentsEvolutionUpdateCmd, []string{"agent/1", "sugg 1"}); err != nil { + t.Fatalf("update: %v", err) + } + if gotPath != "/v1/agents/agent%2F1/evolution/suggestions/sugg%201" { + t.Fatalf("path = %q", gotPath) + } + if gotBody["status"] != "approved" || gotBody["action"] != nil { + t.Fatalf("body = %#v", gotBody) + } +} + +func TestAgentsEvolutionUpdateRejectMapsToRejected(t *testing.T) { + var gotBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Errorf("decode body: %v", err) + } + okJSON(t, w, map[string]any{"status": "rejected"}) + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, agentsEvolutionUpdateCmd, "action") + _ = agentsEvolutionUpdateCmd.Flags().Set("action", "reject") + + if err := agentsEvolutionUpdateCmd.RunE(agentsEvolutionUpdateCmd, []string{"agent-1", "sugg-1"}); err != nil { + t.Fatalf("update: %v", err) + } + if gotBody["status"] != "rejected" { + t.Fatalf("body = %#v", gotBody) + } +} + +func TestAgentsEvolutionSkillApplySendsApprovedWithDraftFile(t *testing.T) { + draftPath := filepath.Join(t.TempDir(), "SKILL.md") + if err := os.WriteFile(draftPath, []byte("skill draft\n"), 0644); err != nil { + t.Fatalf("write draft: %v", err) + } + var gotPath string + var gotBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + if r.URL.EscapedPath() != "/v1/agents/agent%2F1/evolution/suggestions" { + t.Errorf("unexpected preflight path: %s", r.URL.EscapedPath()) + } + okJSON(t, w, []map[string]any{{"id": "sugg 1", "suggestion_type": "skill_add"}}) + return + case http.MethodPatch: + gotPath = r.URL.EscapedPath() + if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Errorf("decode body: %v", err) + } + okJSON(t, w, map[string]any{"applied": true}) + return + default: + t.Errorf("unexpected method: %s", r.Method) + } + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, agentsEvolutionSkillApplyCmd, "skill-draft") + _ = agentsEvolutionSkillApplyCmd.Flags().Set("skill-draft", "@"+draftPath) + + if err := agentsEvolutionSkillApplyCmd.RunE(agentsEvolutionSkillApplyCmd, []string{"agent/1", "sugg 1"}); err != nil { + t.Fatalf("skill apply: %v", err) + } + if gotPath != "/v1/agents/agent%2F1/evolution/suggestions/sugg%201" { + t.Fatalf("path = %q", gotPath) + } + if gotBody["status"] != "approved" || gotBody["skill_draft"] != "skill draft\n" { + t.Fatalf("body = %#v", gotBody) + } +} + +func TestAgentsEvolutionSkillApplyWithoutDraftSendsApprovedOnly(t *testing.T) { + var gotBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + okJSON(t, w, []map[string]any{{"id": "sugg-1", "suggestion_type": "skill_add"}}) + return + } + if r.Method != http.MethodPatch { + t.Errorf("expected PATCH, got %s", r.Method) + } else if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Errorf("decode body: %v", err) + } + okJSON(t, w, map[string]any{"applied": true}) + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, agentsEvolutionSkillApplyCmd, "skill-draft") + + if err := agentsEvolutionSkillApplyCmd.RunE(agentsEvolutionSkillApplyCmd, []string{"agent-1", "sugg-1"}); err != nil { + t.Fatalf("skill apply: %v", err) + } + if gotBody["status"] != "approved" || gotBody["skill_draft"] != nil { + t.Fatalf("body = %#v", gotBody) + } +} + +func TestAgentsEvolutionSkillApplyRejectsNonSkillSuggestionBeforePatch(t *testing.T) { + patchCalled := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPatch { + patchCalled = true + t.Error("PATCH should not be called for non-skill suggestion") + return + } + okJSON(t, w, []map[string]any{{"id": "sugg-1", "suggestion_type": "tool_order"}}) + })) + defer srv.Close() + setupP5HTTPTest(srv.URL) + resetFlag(t, agentsEvolutionSkillApplyCmd, "skill-draft") + + err := agentsEvolutionSkillApplyCmd.RunE(agentsEvolutionSkillApplyCmd, []string{"agent-1", "sugg-1"}) + if err == nil || !strings.Contains(err.Error(), "not skill_add") { + t.Fatalf("expected skill_add validation error, got %v", err) + } + if patchCalled { + t.Fatal("PATCH was called") + } +} diff --git a/cmd/providers_codex_pool.go b/cmd/providers_codex_pool.go index ee93649..23ee979 100644 --- a/cmd/providers_codex_pool.go +++ b/cmd/providers_codex_pool.go @@ -8,9 +8,10 @@ import ( // Returns activity data for a provider's Codex pool (code execution sessions). var providersCodexPoolActivityCmd = &cobra.Command{ - Use: "codex-pool-activity ", - Short: "Show Codex pool activity for a provider", - Args: cobra.ExactArgs(1), + Use: "codex-pool-activity ", + Short: "Show Codex pool activity for a provider", + Deprecated: "use 'goclaw codex-pool activity --provider=' instead", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { c, err := newHTTP() if err != nil { diff --git a/cmd/root.go b/cmd/root.go index 68ae70b..2008108 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -31,8 +31,8 @@ var rootCmd = &cobra.Command{ // but re-resolve here so TTY fallback kicks in when neither flag nor env // is set while preserving a profile-level output default. flagVal := "" - if cmd.Flags().Changed("output") { - flagVal, _ = cmd.Flags().GetString("output") + if flag := cmd.Root().PersistentFlags().Lookup("output"); flag != nil && flag.Changed { + flagVal = flag.Value.String() } cfg.OutputFormat = output.ResolveFormatWithDefault(flagVal, cfg.ProfileOutputFormat) diff --git a/cmd/sessions.go b/cmd/sessions.go index 714a281..000297b 100644 --- a/cmd/sessions.go +++ b/cmd/sessions.go @@ -41,13 +41,14 @@ var sessionsListCmd = &cobra.Command{ return err } if cfg.OutputFormat != "table" { - printer.Print(unmarshalList(data)) + printer.Print(unmarshalNamedList(data, "sessions")) return nil } tbl := output.NewTable("KEY", "AGENT", "USER", "LABEL", "INPUT_TOKENS", "OUTPUT_TOKENS") - for _, s := range unmarshalList(data) { - tbl.AddRow(str(s, "session_key"), str(s, "agent_id"), str(s, "user_id"), - str(s, "label"), str(s, "input_tokens"), str(s, "output_tokens")) + for _, s := range unmarshalNamedList(data, "sessions") { + tbl.AddRow(strFirst(s, "session_key", "key"), strFirst(s, "agent_id", "agentID", "agentName"), + strFirst(s, "user_id", "userID"), str(s, "label"), + strFirst(s, "input_tokens", "inputTokens"), strFirst(s, "output_tokens", "outputTokens")) } printer.Print(tbl) return nil diff --git a/cmd/teams.go b/cmd/teams.go index 8137d64..5f86f57 100644 --- a/cmd/teams.go +++ b/cmd/teams.go @@ -144,7 +144,7 @@ func init() { teamsCmd.AddCommand( teamsListCmd, teamsGetCmd, teamsCreateCmd, teamsUpdateCmd, teamsDeleteCmd, teamsMembersCmd, teamsTasksCmd, teamsWorkspaceCmd, - teamsEventsCmd, teamsScopesCmd, + teamsEventsCmd, teamsScopesCmd, teamsAttachmentsCmd, ) rootCmd.AddCommand(teamsCmd) } diff --git a/cmd/teams_attachments.go b/cmd/teams_attachments.go new file mode 100644 index 0000000..d4d9cd2 --- /dev/null +++ b/cmd/teams_attachments.go @@ -0,0 +1,94 @@ +package cmd + +import ( + "fmt" + "io" + "net/url" + "os" + "path/filepath" + + "github.com/spf13/cobra" +) + +// teams_attachments.go — team task attachment downloads. +// HTTP endpoint: GET /v1/teams/{teamId}/attachments/{attachmentId}/download + +var teamsAttachmentsCmd = &cobra.Command{ + Use: "attachments", + Short: "Manage team task attachments", +} + +var teamsAttachmentsDownloadCmd = &cobra.Command{ + Use: "download ", + Short: "Download a team task attachment", + Long: `Download a team task attachment to an explicit local output file. + +GET /v1/teams/{teamId}/attachments/{attachmentId}/download + +Example: + goclaw teams attachments download team-1 att-42 --output ./artifact.bin + goclaw teams attachments download team-1 att-42 -o ./artifact.bin --force`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + outFile, _ := cmd.Flags().GetString("output") + force, _ := cmd.Flags().GetBool("force") + if outFile == "" { + return fmt.Errorf("--output is required") + } + if err := os.MkdirAll(filepath.Dir(outFile), 0755); err != nil { + return fmt.Errorf("create output directory: %w", err) + } + if !force { + if _, err := os.Stat(outFile); err == nil { + return fmt.Errorf("output file already exists: %s (use --force to overwrite)", outFile) + } else if !os.IsNotExist(err) { + return fmt.Errorf("check output file: %w", err) + } + } + + flags := os.O_WRONLY | os.O_CREATE | os.O_EXCL + if force { + flags = os.O_WRONLY | os.O_CREATE | os.O_TRUNC + } + + c, err := newHTTP() + if err != nil { + return err + } + path := fmt.Sprintf( + "/v1/teams/%s/attachments/%s/download", + url.PathEscape(args[0]), + url.PathEscape(args[1]), + ) + resp, err := c.GetRaw(path) + if err != nil { + return err + } + if resp.StatusCode >= 400 { + return rawResponseError(resp) + } + defer resp.Body.Close() + + f, err := os.OpenFile(outFile, flags, 0644) + if err != nil { + if os.IsExist(err) { + return fmt.Errorf("output file already exists: %s (use --force to overwrite)", outFile) + } + return fmt.Errorf("open output file: %w", err) + } + defer f.Close() + + n, err := io.Copy(f, resp.Body) + if err != nil { + return fmt.Errorf("write output file: %w", err) + } + printer.Success(fmt.Sprintf("Downloaded %d bytes to %s", n, outFile)) + return nil + }, +} + +func init() { + teamsAttachmentsDownloadCmd.Flags().StringP("output", "o", "", "Output file path") + teamsAttachmentsDownloadCmd.Flags().Bool("force", false, "Overwrite output file if it exists") + teamsAttachmentsCmd.AddCommand(teamsAttachmentsDownloadCmd) +} diff --git a/cmd/tools.go b/cmd/tools.go index 8364f50..3d04196 100644 --- a/cmd/tools.go +++ b/cmd/tools.go @@ -23,7 +23,7 @@ var toolsBuiltinListCmd = &cobra.Command{ if err != nil { return err } - printer.Print(unmarshalList(data)) + printer.Print(unmarshalNamedList(data, "tools")) return nil }, } diff --git a/cmd/tools_custom.go b/cmd/tools_custom.go index 06e3879..261729c 100644 --- a/cmd/tools_custom.go +++ b/cmd/tools_custom.go @@ -1,13 +1,7 @@ package cmd import ( - "encoding/json" - "fmt" - "net/url" - "strings" - "github.com/nextlevelbuilder/goclaw-cli/internal/output" - "github.com/nextlevelbuilder/goclaw-cli/internal/tui" "github.com/spf13/cobra" ) @@ -19,126 +13,35 @@ var toolsCustomCmd = &cobra.Command{Use: "custom", Short: "Manage custom tools"} var toolsCustomListCmd = &cobra.Command{ Use: "list", Short: "List custom tools", RunE: func(cmd *cobra.Command, args []string) error { - c, err := newHTTP() - if err != nil { - return err - } - path := "/v1/tools/custom" - if v, _ := cmd.Flags().GetString("agent"); v != "" { - path += "?agent_id=" + url.QueryEscape(v) - } - data, err := c.Get(path) - if err != nil { - return err - } - if cfg.OutputFormat != "table" { - printer.Print(unmarshalList(data)) - return nil - } - tbl := output.NewTable("ID", "NAME", "DESCRIPTION", "ENABLED", "TIMEOUT") - for _, t := range unmarshalList(data) { - tbl.AddRow(str(t, "id"), str(t, "name"), str(t, "description"), - str(t, "enabled"), str(t, "timeout_seconds")) - } - printer.Print(tbl) - return nil + return customToolsUnsupported() }, } var toolsCustomGetCmd = &cobra.Command{ Use: "get ", Short: "Get custom tool details", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - c, err := newHTTP() - if err != nil { - return err - } - data, err := c.Get("/v1/tools/custom/" + args[0]) - if err != nil { - return err - } - printer.Print(unmarshalMap(data)) - return nil + return customToolsUnsupported() }, } var toolsCustomCreateCmd = &cobra.Command{ Use: "create", Short: "Create a custom tool", RunE: func(cmd *cobra.Command, args []string) error { - c, err := newHTTP() - if err != nil { - return err - } - name, _ := cmd.Flags().GetString("name") - desc, _ := cmd.Flags().GetString("description") - command, _ := cmd.Flags().GetString("command") - timeout, _ := cmd.Flags().GetInt("timeout") - agent, _ := cmd.Flags().GetString("agent") - paramsJSON, _ := cmd.Flags().GetString("parameters") - body := buildBody("name", name, "description", desc, - "command", command, "timeout_seconds", timeout, "agent_id", agent, "enabled", true) - if paramsJSON != "" { - var params any - if err := json.Unmarshal([]byte(paramsJSON), ¶ms); err != nil { - return fmt.Errorf("invalid parameters JSON: %w", err) - } - body["parameters"] = params - } - data, err := c.Post("/v1/tools/custom", body) - if err != nil { - return err - } - printer.Success(fmt.Sprintf("Tool created: %s", str(unmarshalMap(data), "id"))) - return nil + return customToolsUnsupported() }, } var toolsCustomUpdateCmd = &cobra.Command{ Use: "update ", Short: "Update custom tool", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - c, err := newHTTP() - if err != nil { - return err - } - body := make(map[string]any) - for _, f := range []string{"name", "description", "command"} { - if cmd.Flags().Changed(f) { - v, _ := cmd.Flags().GetString(f) - body[f] = v - } - } - if cmd.Flags().Changed("timeout") { - v, _ := cmd.Flags().GetInt("timeout") - body["timeout_seconds"] = v - } - if cmd.Flags().Changed("enabled") { - v, _ := cmd.Flags().GetBool("enabled") - body["enabled"] = v - } - _, err = c.Put("/v1/tools/custom/"+args[0], body) - if err != nil { - return err - } - printer.Success("Tool updated") - return nil + return customToolsUnsupported() }, } var toolsCustomDeleteCmd = &cobra.Command{ Use: "delete ", Short: "Delete custom tool", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if !tui.Confirm("Delete this tool?", cfg.Yes) { - return nil - } - c, err := newHTTP() - if err != nil { - return err - } - _, err = c.Delete("/v1/tools/custom/" + args[0]) - if err != nil { - return err - } - printer.Success("Tool deleted") - return nil + return customToolsUnsupported() }, } @@ -153,21 +56,22 @@ var toolsInvokeCmd = &cobra.Command{ if err != nil { return err } - paramPairs, _ := cmd.Flags().GetStringSlice("param") - paramsJSON, _ := cmd.Flags().GetString("params") - params := make(map[string]any) - if paramsJSON != "" { - if err := json.Unmarshal([]byte(paramsJSON), ¶ms); err != nil { - return fmt.Errorf("invalid --params JSON: %w", err) - } - } - for _, pair := range paramPairs { - parts := strings.SplitN(pair, "=", 2) - if len(parts) == 2 { - params[parts[0]] = parts[1] - } + params, err := parseToolInvokeParams(cmd) + if err != nil { + return err } - body := map[string]any{"name": args[0], "parameters": params} + agentID, _ := cmd.Flags().GetString("agent") + action, _ := cmd.Flags().GetString("action") + sessionKey, _ := cmd.Flags().GetString("session") + dryRun, _ := cmd.Flags().GetBool("dry-run") + body := buildBody( + "tool", args[0], + "args", params, + "agentId", agentID, + "action", action, + "sessionKey", sessionKey, + "dryRun", dryRun, + ) data, err := c.Post("/v1/tools/invoke", body) if err != nil { return err @@ -177,6 +81,13 @@ var toolsInvokeCmd = &cobra.Command{ }, } +func customToolsUnsupported() error { + return &output.ErrorDetail{ + Code: "INVALID_REQUEST", + Message: "custom tool management is not supported by this GoClaw server; use `tools builtin` or `tools invoke`", + } +} + func init() { toolsCustomListCmd.Flags().String("agent", "", "Filter by agent ID") for _, c := range []*cobra.Command{toolsCustomCreateCmd, toolsCustomUpdateCmd} { @@ -190,6 +101,11 @@ func init() { } toolsInvokeCmd.Flags().StringSlice("param", nil, "Parameter key=value pairs") toolsInvokeCmd.Flags().String("params", "", "Parameters as JSON object") + toolsInvokeCmd.Flags().String("args", "", "Alias for --params; accepts literal JSON or @filepath") + toolsInvokeCmd.Flags().String("agent", "", "Agent key or ID for tool context") + toolsInvokeCmd.Flags().String("action", "", "Optional action to pass to the tool") + toolsInvokeCmd.Flags().String("session", "", "Optional session key for tool context") + toolsInvokeCmd.Flags().Bool("dry-run", false, "Validate tool and return schema without executing it") toolsCustomCmd.AddCommand(toolsCustomListCmd, toolsCustomGetCmd, toolsCustomCreateCmd, toolsCustomUpdateCmd, toolsCustomDeleteCmd) diff --git a/cmd/tools_invoke_args.go b/cmd/tools_invoke_args.go new file mode 100644 index 0000000..f045763 --- /dev/null +++ b/cmd/tools_invoke_args.go @@ -0,0 +1,45 @@ +package cmd + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +func parseToolInvokeParams(cmd *cobra.Command) (map[string]any, error) { + paramPairs, _ := cmd.Flags().GetStringSlice("param") + paramsJSON, _ := cmd.Flags().GetString("params") + argsJSON, _ := cmd.Flags().GetString("args") + paramsChanged := cmd.Flags().Changed("params") + argsChanged := cmd.Flags().Changed("args") + if paramsChanged && argsChanged && paramsJSON != argsJSON { + return nil, fmt.Errorf("use only one of --params or --args") + } + + rawJSON := paramsJSON + flagName := "--params" + if rawJSON == "" && argsJSON != "" { + rawJSON = argsJSON + flagName = "--args" + } + + params := make(map[string]any) + if rawJSON != "" { + content, err := readContent(rawJSON) + if err != nil { + return nil, err + } + if err := json.Unmarshal([]byte(content), ¶ms); err != nil { + return nil, fmt.Errorf("invalid %s JSON: %w", flagName, err) + } + } + for _, pair := range paramPairs { + parts := strings.SplitN(pair, "=", 2) + if len(parts) == 2 { + params[parts[0]] = parts[1] + } + } + return params, nil +} diff --git a/cmd/traces.go b/cmd/traces.go index 5372cfd..851bff1 100644 --- a/cmd/traces.go +++ b/cmd/traces.go @@ -5,6 +5,7 @@ import ( "io" "net/url" "os" + "time" "github.com/nextlevelbuilder/goclaw-cli/internal/output" "github.com/spf13/cobra" @@ -191,10 +192,17 @@ var usageTimeseriesCmd = &cobra.Command{ return err } q := url.Values{} - for _, k := range []string{"start", "end", "granularity", "agent", "user", "tenant"} { - if v, _ := cmd.Flags().GetString(k); v != "" { - q.Set(k, v) - } + if v, _ := cmd.Flags().GetString("start"); v != "" { + q.Set("from", normalizeUsageTimestamp(v)) + } + if v, _ := cmd.Flags().GetString("end"); v != "" { + q.Set("to", normalizeUsageTimestamp(v)) + } + if v, _ := cmd.Flags().GetString("granularity"); v != "" { + q.Set("group_by", v) + } + if v, _ := cmd.Flags().GetString("agent"); v != "" { + q.Set("agent_id", v) } path := "/v1/usage/timeseries" if len(q) > 0 { @@ -217,10 +225,14 @@ var usageBreakdownCmd = &cobra.Command{ return err } q := url.Values{} - for _, k := range []string{"by", "start", "end"} { - if v, _ := cmd.Flags().GetString(k); v != "" { - q.Set(k, v) - } + if v, _ := cmd.Flags().GetString("by"); v != "" { + q.Set("group_by", v) + } + if v, _ := cmd.Flags().GetString("start"); v != "" { + q.Set("from", normalizeUsageTimestamp(v)) + } + if v, _ := cmd.Flags().GetString("end"); v != "" { + q.Set("to", normalizeUsageTimestamp(v)) } path := "/v1/usage/breakdown" if len(q) > 0 { @@ -235,6 +247,13 @@ var usageBreakdownCmd = &cobra.Command{ }, } +func normalizeUsageTimestamp(v string) string { + if t, err := time.Parse("2006-01-02", v); err == nil { + return t.Format(time.RFC3339) + } + return v +} + func init() { tracesListCmd.Flags().String("agent", "", "Filter by agent ID") tracesListCmd.Flags().String("status", "", "Filter: running, success, error") @@ -250,15 +269,15 @@ func init() { usageDetailCmd.Flags().String("from", "", "Start date") usageDetailCmd.Flags().String("to", "", "End date") - usageTimeseriesCmd.Flags().String("start", "", "Start ISO timestamp") - usageTimeseriesCmd.Flags().String("end", "", "End ISO timestamp") - usageTimeseriesCmd.Flags().String("granularity", "day", "Bucket size: hour|day") + usageTimeseriesCmd.Flags().String("start", "", "Start date or RFC3339 timestamp") + usageTimeseriesCmd.Flags().String("end", "", "End date or RFC3339 timestamp") + usageTimeseriesCmd.Flags().String("granularity", "day", "Group by: provider|model|channel|agent|day") usageTimeseriesCmd.Flags().String("agent", "", "Filter by agent") usageTimeseriesCmd.Flags().String("user", "", "Filter by user") usageTimeseriesCmd.Flags().String("tenant", "", "Filter by tenant") - usageBreakdownCmd.Flags().String("by", "agent", "Dimension: agent|user|tenant") - usageBreakdownCmd.Flags().String("start", "", "Start ISO timestamp") - usageBreakdownCmd.Flags().String("end", "", "End ISO timestamp") + usageBreakdownCmd.Flags().String("by", "agent", "Dimension: provider|model|channel|agent|day") + usageBreakdownCmd.Flags().String("start", "", "Start date or RFC3339 timestamp") + usageBreakdownCmd.Flags().String("end", "", "End date or RFC3339 timestamp") tracesCmd.AddCommand(tracesListCmd, tracesGetCmd, tracesExportCmd) usageCmd.AddCommand(usageSummaryCmd, usageDetailCmd, usageCostsCmd, diff --git a/docs/codebase-summary.md b/docs/codebase-summary.md index 8ec5939..a5fc48d 100644 --- a/docs/codebase-summary.md +++ b/docs/codebase-summary.md @@ -1,7 +1,7 @@ # GoClaw CLI - Codebase Summary -**Generated from:** `repomix-output.xml` (2026-04-15), updated manually 2026-05-19 -**Phase Status:** P0-P4 Complete (AI-First Expansion); Super Admin API Parity Complete; Domain Coverage P3 Complete +**Generated from:** `repomix-output.xml` (2026-04-15), updated manually 2026-05-20 +**Phase Status:** P0-P4 Complete (AI-First Expansion); Super Admin API Parity Complete; Domain Coverage P5 Implemented **Total Files:** 80+ **Estimated Tokens:** 80,000+ **Total Size:** 220+ KB @@ -10,7 +10,7 @@ ## Overview -GoClaw CLI is a production-ready Go application providing comprehensive command-line management for GoClaw AI agent gateway servers. Built with Cobra framework, it supports 30+ command groups across modular command files with dual modes: interactive (human) and automation (CI/agent). Phases 0-4 (AI-first expansion) add AI ergonomics, admin/ops, migration, vault, and advanced agent/team/memory support. The 2026-05-18 super-admin parity work adds gateway upgrade, package updates, workstations, webhooks, MCP user credentials, secure env reveal, media/TTS/storage/channel fillers, and focused route-contract tests. The 2026-05-19 P3 filler pass adds first-class profile commands, `GOCLAW_PROFILE`, `sessions compact`, WS health, and trace filter polish. +GoClaw CLI is a production-ready Go application providing comprehensive command-line management for GoClaw AI agent gateway servers. Built with Cobra framework, it supports 30+ command groups across modular command files with dual modes: interactive (human) and automation (CI/agent). Phases 0-4 (AI-first expansion) add AI ergonomics, admin/ops, migration, vault, and advanced agent/team/memory support. The 2026-05-18 super-admin parity work adds gateway upgrade, package updates, workstations, webhooks, MCP user credentials, secure env reveal, media/TTS/storage/channel fillers, and focused route-contract tests. The 2026-05-19 P3/P4 filler pass adds first-class profile commands, `GOCLAW_PROFILE`, `sessions compact`, WS health, trace filter polish, `codex-pool`, `api-keys rotate`, `config defaults`, chat session convenience wrappers, and `tools invoke --args`. The 2026-05-20 P5 filler pass adds team attachment download, skill-specific evolution suggestion apply, and fixes evolution update payload compatibility. **Key Metrics:** - **70+ command files** in `cmd/` (modularized for maintainability) @@ -281,7 +281,7 @@ goclaw (root) │ ├── files (list, get, set) # global AGENTS.md, SOUL.md, IDENTITY.md, ... │ ├── instances (list, get-file, set-file, metadata, update-metadata) │ ├── episodic (list, search) -│ ├── evolution (metrics, suggestions, update) +│ ├── evolution (metrics, suggestions, update, skill apply) │ ├── orchestration / codex-pool-activity │ ├── skills list # skills granted to agent │ ├── v3-flags (get, toggle) @@ -313,7 +313,8 @@ goclaw (root) ├── tools (list, invoke, delete) ├── cron (list, create, update, delete, trigger, history) ├── teams (list, create, members, task-board, export, import [--apply]) -│ └── workspace (list, read, delete, upload, move) +│ ├── workspace (list, read, delete, upload, move) +│ └── attachments download --output ├── channels (list, contacts, pending-messages) ├── traces (list, export) ├── memory (list, search, upsert) @@ -628,16 +629,23 @@ goclaw vault | `agents_sharing.go` | `agents share/unshare/regenerate/resummon` | Agent sharing lifecycle | | `agents_instances.go` | `agents instances list/get-file/set-file/update-metadata/metadata` | Per-user instance management | | `agents_links.go` | `agents links list/create/update/delete` | Delegation link management | -| `agents_evolution.go` | `agents evolution metrics/suggestions/update` | Evolution feedback loop | +| `agents_evolution.go` | `agents evolution metrics/suggestions/update/skill apply` | Evolution feedback loop and skill suggestion approval | | `agents_episodic.go` | `agents episodic list/search` | Episodic memory (semantic search) | | `agents_v3_flags.go` | `agents v3-flags get/toggle` | Experimental feature flags | | `agents_misc.go` | `agents orchestration/codex-pool-activity` | Orchestration + pool status | | `chat_ai_commands.go` | `chat history/inject/session-status` | AI-critical MAX POLISH chat ops | +| `chat_replay.go` | `chat replay` | Session replay wrapper over `chat.history` | +| `chat_sessions.go` | `chat sessions resume` | Resume wrapper over existing chat session flow | +| `codex_pool.go` | `codex-pool activity` | Unified agent/provider Codex pool activity lookup | +| `config_defaults.go` | `config defaults` | Read-only WS config defaults passthrough | +| `api_keys_rotate.go` | `api-keys rotate` | Replacement key creation + old key revoke composite | +| `tools_invoke_args.go` | `tools invoke --args` | JSON arg alias/file parsing for tool invocation | | `teams_members.go` | `teams members list/add/remove` | Team membership | | `teams_tasks.go` | `teams tasks list/get/get-light/create/assign` | Core task CRUD | | `teams_tasks_review.go` | `teams tasks approve/reject/comment/comments` | Task review workflow | | `teams_tasks_advanced.go` | `teams tasks delete/delete-bulk/events/active` | Advanced task ops + follow stream | | `teams_workspace.go` | `teams workspace list/read/delete` | Team workspace files | +| `teams_attachments.go` | `teams attachments download` | Authenticated team task attachment downloads | | `teams_events.go` | `teams events list [--follow]` | Team event stream | | `teams_scopes.go` | `teams scopes ` | Permission scopes | | `memory_kg.go` | `memory kg entities list/get/upsert/delete` | KG entity CRUD | @@ -665,6 +673,7 @@ All `cmd/` files now ≤200 LoC (chat files are 214 lines — overage is entirel | File | Tests | Coverage | |------|-------|---------| | `agents_lifecycle_test.go` | 18 tests | wake, identity, wait (success+timeout+invalid), sync, preview, evolution, episodic, v3-flags, orchestration, codex, instances | +| `p5_fillers_test.go` | 9 tests | team attachment download, output-format guard, evolution payload mapping, skill apply type guard and draft override | | `chat_extensions_test.go` | 11 tests | history (3), inject (5 inc. validation), session-status (2) | | `teams_tasks_test.go` | 16 tests | list, get, get-light, create, assign, delete (yes+declined), delete-bulk (ids+missing), events, active (success+missing), scopes, events-list | | `memory_kg_test.go` | 15 tests | entities (list/get/delete/delete-with-yes), traverse (from-required+success), stats, graph (full+compact), dedup (scan/list/merge/dismiss), chunks, index, index-all, documents-global | diff --git a/docs/project-roadmap.md b/docs/project-roadmap.md index b220ecb..4cb6b20 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -1,9 +1,40 @@ # GoClaw CLI - Project Roadmap -**Last Updated:** 2026-05-19 +**Last Updated:** 2026-05-20 **Phase Structure:** Legacy Phases 1-9 (bootstrap → CI/CD) + AI-First Expansion Phases 0-5 (2026-04-15) -**Current Status:** Legacy Phases 1-9 ✓ COMPLETE; P0-P4 ✓ COMPLETE; Super Admin API Parity ✓ COMPLETE; Domain Coverage P3 ✓ COMPLETE -**Next Phase:** Domain Coverage residuals: P4 UX polish leftovers, then P5 team attachment download + evolution skill apply. +**Current Status:** Legacy Phases 1-9 ✓ COMPLETE; P0-P4 ✓ COMPLETE; Super Admin API Parity ✓ COMPLETE; Domain Coverage P5 implemented pending release. +**Next Phase:** Ship Domain Coverage P5 PR to `dev` and verify beta release. + +--- + +## 2026-05-20: Domain Coverage P5 IMPLEMENTED + +**Objective:** Close final CLI-only residuals after the P5 sweep without adding server routes. + +**Deliverables:** +- [x] Added `teams attachments download --output ` with required output, parent directory creation, no-overwrite default, and `--force`. +- [x] Added `agents evolution skill apply [--skill-draft @file]` as a skill-specific approval wrapper. +- [x] Fixed `agents evolution update --action=accept|reject` to send server-compatible `status=approved|rejected`. +- [x] Added focused regression tests for route escaping, auth header, request bodies, file writes, missing output, overwrite guard, output-format preservation, suggestion type guard, and draft override. +- [x] Synced parent P5 plan and dedicated execution plan. + +**Validation:** `/usr/local/go/bin/go build ./...`; `/usr/local/go/bin/go test ./...`; `/usr/local/go/bin/go vet ./...`. + +--- + +## 2026-05-19: Domain Coverage P4 ✓ COMPLETE + +**Objective:** Close UX polish residuals after the P4/P5 sweep without adding new server contracts. + +**Deliverables:** +- [x] Added `codex-pool activity --agent|--provider` as the unified Codex pool activity command. +- [x] Added `api-keys rotate ` as a non-atomic create-and-revoke composite with emit-before-revoke partial failure handling. +- [x] Added `config defaults` via WS RPC `config.defaults`. +- [x] Added `chat replay --session=` and `chat sessions resume --session=` convenience wrappers. +- [x] Added `tools invoke --args=` alias support. +- [x] Added focused P4 route/contract tests and synced README, changelog, codebase summary, and plan docs. + +**Validation:** `/usr/local/go/bin/go build ./...`; `/usr/local/go/bin/go test ./...`; `/usr/local/go/bin/go vet ./...`. --- @@ -22,7 +53,7 @@ **Validation:** `go test ./...`. -**Backlog Sweep:** P4/P5 verification on 2026-05-19 removed covered items from future scope: `agents prompt-preview`, `storage size`, `channels writers groups`, `contacts unmerge`, `agents instances`, `mcp servers tools`, `agents evolution update`, and `tts synthesize`. +**Backlog Sweep:** P4/P5 verification on 2026-05-19 removed covered items from future scope: `agents prompt-preview`, `storage size`, `channels writers groups`, `contacts unmerge`, `agents instances`, `mcp servers tools`, and `tts synthesize`. Follow-up P5 validation on 2026-05-20 found `agents evolution update` existed as a command surface but needed payload compatibility repair. --- @@ -444,7 +475,6 @@ **Deferred / out of scope:** - OpenAI-compatible `/chat/completions` and `/v1/responses` (client APIs) -- `evolution_skill_apply` (no REST route registered server-side) - `hooks history` pagination (server stub returns empty list pending Phase 4) --- diff --git a/internal/config/config.go b/internal/config/config.go index 414b17e..64919d3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -95,22 +95,22 @@ func Load(cmd *cobra.Command) (*Config, error) { } // 3. Overlay flags (only if explicitly set) - if cmd.Flags().Changed("server") { - cfg.Server, _ = cmd.Flags().GetString("server") + if changed, value := rootOrLocalStringFlag(cmd, "server"); changed { + cfg.Server = value } - if cmd.Flags().Changed("token") { - cfg.Token, _ = cmd.Flags().GetString("token") + if changed, value := rootOrLocalStringFlag(cmd, "token"); changed { + cfg.Token = value } - if cmd.Flags().Changed("output") { - cfg.OutputFormat, _ = cmd.Flags().GetString("output") + if changed, value := rootOrLocalStringFlag(cmd, "output"); changed { + cfg.OutputFormat = value } - if cmd.Flags().Changed("insecure") { - cfg.Insecure, _ = cmd.Flags().GetBool("insecure") + if changed, value := rootOrLocalBoolFlag(cmd, "insecure"); changed { + cfg.Insecure = value } - if cmd.Flags().Changed("verbose") { - cfg.Verbose, _ = cmd.Flags().GetBool("verbose") + if changed, value := rootOrLocalBoolFlag(cmd, "verbose"); changed { + cfg.Verbose = value } - cfg.Yes, _ = cmd.Flags().GetBool("yes") + _, cfg.Yes = rootOrLocalBoolFlag(cmd, "yes") // Tenant ID: env then flag override if v := os.Getenv("GOCLAW_TENANT_ID"); v != "" { @@ -123,6 +123,25 @@ func Load(cmd *cobra.Command) (*Config, error) { return cfg, nil } +func rootOrLocalStringFlag(cmd *cobra.Command, name string) (bool, string) { + if cmd != nil && cmd.Root() != nil { + if flag := cmd.Root().PersistentFlags().Lookup(name); flag != nil { + return flag.Changed, flag.Value.String() + } + } + if cmd != nil { + if flag := cmd.Flags().Lookup(name); flag != nil { + return flag.Changed, flag.Value.String() + } + } + return false, "" +} + +func rootOrLocalBoolFlag(cmd *cobra.Command, name string) (bool, bool) { + changed, raw := rootOrLocalStringFlag(cmd, name) + return changed, raw == "true" +} + func loadFile() (*FileConfig, error) { data, err := os.ReadFile(FilePath()) if err != nil { diff --git a/plans/260503-1907-domain-coverage-p3-plus/phase-04-ux-polish-batch-1.md b/plans/260503-1907-domain-coverage-p3-plus/phase-04-ux-polish-batch-1.md index 357af36..4090186 100644 --- a/plans/260503-1907-domain-coverage-p3-plus/phase-04-ux-polish-batch-1.md +++ b/plans/260503-1907-domain-coverage-p3-plus/phase-04-ux-polish-batch-1.md @@ -1,7 +1,7 @@ # Phase 4 — UX Polish Batch 1 **Priority:** 🟡 medium -**Status:** verified residuals — not-started +**Status:** complete **Estimated LoC:** ~250 (excl. tests) **Estimated PR size:** ≤ 500 LoC incl. tests **Depends on:** P3 (multi-profile stable) @@ -9,6 +9,7 @@ ## Context Links - Gap analysis: `plans/reports/brainstorm-260503-1907-gap-analysis-round2.md` § 5 (P4) +- Validation/red-team: `reports/validation-red-team-260519-p4.md` ## Overview @@ -21,8 +22,8 @@ Composite/UX commands wrapping endpoints already shipped server-side. No server | P2 | `codex-pool activity --agent=… \| --provider=…` | unify `agents codex-pool-activity` + `providers codex-pool-activity` | residual | | D3 | `api-keys rotate ` | composite: create-new + emit raw + revoke-old | residual | | D6 | `config defaults` | WS `config.defaults` (`pkg/protocol/methods.go` ConfigDefaults) | residual | -| E3 | `chat replay ` | composite: `sessions preview` + `chat history` | residual | -| E1 | `chat sessions resume ` | UX wrapper around `chat send --session-key=` | residual | +| E3 | `chat replay --session=` | convenience wrapper over existing `chat history --session` | residual | +| E1 | `chat sessions resume --session=` | discoverability wrapper over existing `chat --session` | residual | | X1 | `agents prompt-preview ` | `cmd/agents_admin.go` | covered | | X6 | `tools invoke ` | `cmd/tools_custom.go`; residual is `--args=@file.json` alias only | partial | | X7 | `storage size` | `cmd/storage.go` | covered | @@ -32,7 +33,7 @@ Composite/UX commands wrapping endpoints already shipped server-side. No server ### Modify - `cmd/api_keys.go` — add `rotate` -- `cmd/chat.go` — add `replay` + `sessions resume` +- `cmd/chat.go` / `cmd/chat_ai_commands.go` — add replay/resume convenience wrappers without duplicating existing session logic - `cmd/tools_custom.go` — add `--args` alias / `@file` support for existing `tools invoke` - `CHANGELOG.md`, `docs/codebase-summary.md` @@ -49,32 +50,34 @@ Composite/UX commands wrapping endpoints already shipped server-side. No server ## Implementation Steps 1. `cmd/codex_pool.go`: register top-level `codex-pool activity` + flag dispatch (`--agent` vs `--provider`). Mark old commands deprecated in Long help. -2. `cmd/api_keys.go::rotate`: orchestrate create → emit raw → revoke-old in single command, JSON output of new key. +2. `cmd/api_keys.go::rotate`: orchestrate create → emit raw → revoke-old in single command, JSON output of new key. Treat as non-atomic composite; preserve machine-readable partial failure output. 3. `cmd/config_defaults.go`: WS `config.defaults`, raw passthrough. -4. Extend `cmd/chat.go` with `replay ` (composite preview+history, stream JSONL to stdout). -5. Add `cmd/chat.go::sessions resume ` as alias for `chat send --session-key=` (read stdin/--message body). -6. Extend existing `tools invoke ` with `--args=@file.json` or literal JSON alias. +4. Add chat wrappers only: `replay` calls existing `chat.history` with `agent_key` + `session_key`; `resume` dispatches to existing chat flow with `--session`. +5. Extend existing `tools invoke ` with `--args=@file.json` or literal JSON alias. Reject simultaneous conflicting `--params` and `--args`. +6. Keep deprecated codex-pool aliases stdout-clean; runtime notices must not break JSON output. 7. Tests for each. Composites: assert sequence of HTTP/WS calls via httptest. 8. Docs sync. ## Todo List -- [ ] cmd/codex_pool.go umbrella + alias deprecation -- [ ] cmd/api_keys.go rotate composite -- [ ] cmd/config_defaults.go -- [ ] cmd/chat.go replay composite -- [ ] cmd/chat.go sessions resume alias +- [x] cmd/codex_pool.go umbrella + alias deprecation +- [x] cmd/api_keys.go rotate composite +- [x] cmd/config_defaults.go +- [x] chat replay convenience wrapper +- [x] chat sessions resume convenience wrapper - [x] cmd/agents prompt-preview -- [ ] cmd/tools invoke `--args` alias (existing `--params` + `--param` already covered) +- [x] cmd/tools invoke `--args` alias + `@file` (existing `--params` + `--param` already covered) - [x] cmd/storage.go (size) -- [ ] tests per command -- [ ] CHANGELOG + docs +- [x] tests per command +- [x] CHANGELOG + docs ## Success Criteria - `goclaw codex-pool activity` works for both --agent and --provider; legacy commands print deprecation notice (stderr only, doesn't break JSON pipe on stdout). -- `goclaw api-keys rotate ` returns new key once + revokes old atomically; if revoke fails, output indicates partial state with exit 5. -- `goclaw chat replay ` outputs JSONL transcript suitable for `| jq` pipeline. +- `goclaw api-keys rotate ` returns new key once + attempts old revoke; if revoke fails, output indicates partial state with exit 5. +- `goclaw config defaults` calls WS `config.defaults` and returns existing printer output. +- `goclaw chat replay --session=` outputs transcript suitable for `| jq` pipeline. +- `goclaw chat sessions resume --session=` reuses existing chat send/interactive path. - `goclaw tools invoke --args=@payload.json` returns server response, exit 0 on success. - All new commands respect `--output`, `--quiet`, `--yes` contracts. - ≥ 60% line coverage on new code. @@ -83,16 +86,24 @@ Composite/UX commands wrapping endpoints already shipped server-side. No server | Risk | Mitigation | |---|---| -| `api-keys rotate` partial failure (new created, old revoke failed) | Output structured JSON `{"new_key":..., "old_revoke_status":"failed", "old_key_id":...}` + exit 5; user can manually revoke | +| `api-keys rotate` partial failure (new created, old revoke failed) | Emit raw new key immediately after create, then return structured partial-failure error with `new_key_id`, `old_key_id`, `old_revoke_status`, and exit 5 if revoke fails | | `tools invoke` argument injection | Server enforces auth + validates payload; CLI only passes through | -| `chat replay` large transcript blows memory | Stream JSONL line-by-line, do not buffer | +| `chat replay` large transcript blows memory | Current wrapper reuses `chat.history`; add pagination/streaming later only if transcript size becomes a real workflow issue | | Deprecated codex-pool aliases confuse users | Help text + CHANGELOG note + sunset version (open question — see plan.md Q2) | +| Runtime deprecation notice breaks JSON pipelines | Prefer Cobra `Deprecated` metadata or stderr-only notices; never write notices to stdout | ## Security Considerations - `api-keys rotate` raw key visible only once via stdout; suggest `--output=json` and `jq -r .key` for capture. - `tools invoke` requires authenticated session; CLI does not bypass. +## Validation Log + +- 2026-05-19 validate/red-team complete. Evidence and decisions recorded in `reports/validation-red-team-260519-p4.md`. +- `config.defaults` contract verified in sibling server: `protocol.MethodConfigDefaults = "config.defaults"` and handler registered as read-only, master-scope gated. +- Existing CLI session support verified; P4 must add discoverability wrappers, not a second session implementation. +- 2026-05-19 implementation complete. Verified with `/usr/local/go/bin/go build ./...`, `/usr/local/go/bin/go test ./...`, and `/usr/local/go/bin/go vet ./...`. + ## Next Steps -P5 = verify sweep + final fillers. +P5 = final fillers: team attachment download + evolution skill apply. diff --git a/plans/260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md b/plans/260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md index e8deea1..ec742f5 100644 --- a/plans/260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md +++ b/plans/260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md @@ -1,13 +1,14 @@ # Phase 5 — Fillers & Verification Batch 2 **Priority:** 🟡 medium -**Status:** verified residuals — not-started +**Status:** implemented pending ship **Estimated LoC:** ~150 (excl. tests) **Depends on:** P3 + P4 merged ## Context Links - Gap analysis: `plans/reports/brainstorm-260503-1907-gap-analysis-round2.md` § 3.C, 3.F, § 5 (P5) +- Detailed execution plan: `../260520-1050-domain-coverage-p5-fillers/plan.md` ## Overview @@ -23,7 +24,7 @@ For each item below, grep both repos and confirm gap before scoping LoC: | C4 | `POST /v1/contacts/unmerge` | `cmd/channels_contacts.go`, `cmd/contacts.go` | covered: `channels contacts unmerge`, `contacts unmerge` | | X3 | `GET /v1/agents/{id}/instances` + files | `cmd/agents_instances.go` | covered: list/get-file/set-file/metadata | | X4 | `GET /v1/mcp/servers/{id}/tools` | `cmd/mcp.go` | covered: `mcp servers tools` | -| X8 | `PATCH /v1/agents/{id}/evolution/suggestions/{sid}` | `cmd/agents_evolution.go` | covered: `agents evolution update` | +| X8 | `PATCH /v1/agents/{id}/evolution/suggestions/{sid}` | `cmd/agents_evolution.go` | covered as command surface, but payload compatibility fix required in P5 | | X11 | `GET /v1/teams/{teamId}/attachments/{aid}/download` | `cmd/teams.go` / `cmd/teams_*.go` | residual | | X12 | `internal/http/evolution_skill_apply.go` | `cmd/agents_evolution.go` | residual | @@ -33,16 +34,17 @@ After sweep, drop covered items, finalize scope. Report sweep results in PR desc | # | Command | Server route | File | |---|---|---|---| -| X11 | `teams attachments download [--out=…]` | `GET …/attachments/{aid}/download` | `cmd/teams_workspace.go` or new `cmd/teams_attachments.go` | -| X12 | `agents evolution skill apply ` | `internal/http/evolution_skill_apply.go` | `cmd/agents_evolution.go` | +| X11 | `teams attachments download --output ` | `GET …/attachments/{aid}/download` | new `cmd/teams_attachments.go` | +| X12 | `agents evolution skill apply [--skill-draft @file]` | `PATCH /v1/agents/{id}/evolution/suggestions/{sid}` with `status=approved` | `cmd/agents_evolution.go` | ## Implementation Steps -1. Extend the existing module file for each confirmed residual (no new files unless >200 LoC pushes existing over budget). -2. `teams attachments download` — binary file save via signed-URL pattern (`internal/client/signed_download.go`); reuse helper. -3. `agents evolution skill apply` — wire the server route and expose structured output. -4. Add `_test.go` cases for each. -5. CHANGELOG + docs sync. +1. Follow detailed plan in `../260520-1050-domain-coverage-p5-fillers/`. +2. `teams attachments download` — authenticated binary file save via `GetRaw`; require `--output/-o`; add `--force` for overwrite. +3. `agents evolution skill apply` — approve `skill_add` suggestions via existing PATCH route and expose structured output. +4. Fix existing `agents evolution update` payload mapping: `accept -> approved`, `reject -> rejected`. +5. Add `_test.go` cases for each. +6. CHANGELOG + docs sync. ## Todo List @@ -52,10 +54,11 @@ After sweep, drop covered items, finalize scope. Report sweep results in PR desc - [x] cmd/channels_contacts.go: unmerge - [x] cmd/agents_instances.go: list + files - [x] cmd/mcp_servers.go: tools subcommand -- [ ] cmd/agents_evolution.go: skill apply -- [ ] teams attachments download (reuse signed_download) -- [ ] tests per command -- [ ] CHANGELOG + docs +- [x] cmd/agents_evolution.go: update payload compatibility +- [x] cmd/agents_evolution.go: skill apply +- [x] teams attachments download (use authenticated GetRaw) +- [x] tests per command +- [x] CHANGELOG + docs ## Success Criteria @@ -69,13 +72,13 @@ After sweep, drop covered items, finalize scope. Report sweep results in PR desc | Risk | Mitigation | |---|---| | Sweep reveals all items already covered | Phase becomes verification-only — close as docs PR | -| Binary download path collisions | Default `--out=./`; respect existing file with --force flag | +| Binary download path collisions | Require `--output/-o`; refuse existing file unless `--force` | | `unmerge` destructive without --yes | Require --yes for unmerge | | Evolution suggestion updates race | Server handles concurrency; CLI passes ETag if available | ## Security Considerations -- Attachment download — write file mode 0644. +- Attachment download — write file mode 0644; no binary stdout mode. - Unmerge requires --yes. ## Next Steps diff --git a/plans/260503-1907-domain-coverage-p3-plus/plan.md b/plans/260503-1907-domain-coverage-p3-plus/plan.md index 5dae50e..4494861 100644 --- a/plans/260503-1907-domain-coverage-p3-plus/plan.md +++ b/plans/260503-1907-domain-coverage-p3-plus/plan.md @@ -3,7 +3,7 @@ **Date:** 2026-05-03 **Branch:** feat/ai-first-cli-expansion **Reference report:** `plans/reports/brainstorm-260503-1907-gap-analysis-round2.md` -**Status:** P3 complete — P4/P5 sweep complete; residual implementation not-started. +**Status:** P3/P4 complete — P5 implemented pending ship; P6 remains server-blocked. ## Summary @@ -12,8 +12,8 @@ Sau R1 (P0–P5) + R2 expansion (P0–P2), CLI đạt ~95% server coverage. R2 r | Phase | Scope | LoC | Tier | Status | |---|---|---|---|---| | P3 | AI-critical fillers (multi-profile, sessions compact, health, traces filter polish) | ~250 | 🔥 | complete | -| P4 | UX polish batch 1 residuals (codex-pool umbrella, api-keys rotate, config defaults, chat replay/resume, tools invoke `--args` alias) | ~250 | 🟡 | not-started | -| P5 | Fillers residuals after sweep (team attachments download, evolution skill apply) | ~150 | 🟡 | not-started | +| P4 | UX polish batch 1 residuals (codex-pool umbrella, api-keys rotate, config defaults, chat replay convenience, tools invoke `--args` alias) | ~250 | 🟡 | complete | +| P5 | Fillers residuals after sweep (team attachments download, evolution skill apply) | ~150 | 🟡 | implemented pending ship | | P6 | Deferred — blocked on server FRs (traces follow, logs aggregate, providers reconnect, …) | n/a | 🟢 | server-blocked | ## Phase Files @@ -25,10 +25,12 @@ Sau R1 (P0–P5) + R2 expansion (P0–P2), CLI đạt ~95% server coverage. R2 r ## Key Dependencies -- Superseded/blocked by `plans/260518-1936-super-admin-api-parity/` for the next implementation slice. The newer plan narrows the backlog to super-admin operational parity after server `v3.12.0-beta.5`. -- P3 multi-profile may refactor `internal/config` singleton — finish before P4/P5. +- Super-admin API parity is already merged; P4 should proceed from current `dev`. +- P3 multi-profile is complete; P4 can build on stable profile/default output behavior. - P5 verify sweep completed 2026-05-19; most suspected gaps already exist under current command paths. +- P5 detailed execution plan: `../260520-1050-domain-coverage-p5-fillers/plan.md`. - P6 = upstream goclaw issues, not CLI work. +- P4 validation/red-team evidence: `reports/validation-red-team-260519-p4.md`; implementation validated with `go build ./...`, `go test ./...`, and `go vet ./...`. ## Success Criteria @@ -41,7 +43,7 @@ Sau R1 (P0–P5) + R2 expansion (P0–P2), CLI đạt ~95% server coverage. R2 r 1. Profile naming: resolved as `profile`; legacy `auth use-context` remains. 2. Codex-pool alias sunset version? -3. Chat replay output: default stdout JSONL; add `--file` only if user workflow demands it. +3. Chat replay output: resolved as existing printer output from `chat.history` (JSON array in JSON mode); add JSONL/streaming only if a real workflow demands it. 4. Health schema: raw passthrough vs normalized? 5. Server FR ownership for P6 items? 6. `tts synthesize` AI use case: already covered by `cmd/tts_http.go`; not a server FR. diff --git a/plans/260503-1907-domain-coverage-p3-plus/reports/validation-red-team-260519-p4.md b/plans/260503-1907-domain-coverage-p3-plus/reports/validation-red-team-260519-p4.md new file mode 100644 index 0000000..8297a62 --- /dev/null +++ b/plans/260503-1907-domain-coverage-p3-plus/reports/validation-red-team-260519-p4.md @@ -0,0 +1,35 @@ +# P4 Validation + Red-Team Report + +**Date:** 2026-05-19 +**Scope:** `phase-04-ux-polish-batch-1.md` +**Verdict:** ready for implementation after tightening scope below. + +## Validation Findings + +| ID | Severity | Finding | Evidence | Decision | +|---|---:|---|---|---| +| V-01 | High | `chat replay/resume` must not duplicate existing session support. CLI already supports `chat history --session=` and `chat --session=`. | `cmd/chat_ai_commands.go:13-70`, `cmd/chat.go:18-52`, `cmd/chat.go:208-211` | Narrow to convenience wrappers only. | +| V-02 | High | `api-keys rotate` cannot be truly atomic with current HTTP contracts; it is a composite create + revoke. Plan must define partial-failure output and tests. | `cmd/api_keys.go:51-103`, `cmd/api_keys.go:106-122` | Keep feature, call it composite, not atomic. | +| V-03 | Medium | `tools invoke` already supports structured JSON through `--params`; residual is only `--args` alias and `@file` support for JSON. | `cmd/tools_custom.go:147-175`, `cmd/tools_custom.go:191-192`, `cmd/helpers.go:63-72` | Keep tiny alias/file change. | +| V-04 | Medium | `config.defaults` exists in server and is read-only/secret-free, but CLI has no command. | `/Volumes/GOON/www/digitop/goclaw/pkg/protocol/methods.go:29-34`, `/Volumes/GOON/www/digitop/goclaw/internal/gateway/methods/config.go:38-47`, `cmd/config_cmd.go:12-113` | Add CLI passthrough. | +| V-05 | Low | `codex-pool` umbrella is an alias/UX improvement; existing agent/provider commands work. | `cmd/agents_misc.go:37-59`, `cmd/providers_codex_pool.go:10-25` | Implement top-level group, keep aliases. | + +## Red-Team Notes + +1. `api-keys rotate` failure mode is riskiest. If create succeeds, stdout must include the raw new key exactly once before the command attempts old-key revoke. If revoke fails, stderr/central error output should explain the old key remains active and the command should exit with code 5. +2. Avoid deprecation text on stdout for legacy `agents codex-pool-activity` and `providers codex-pool-activity`; it would break JSON pipelines. Use command `Deprecated` help metadata or stderr-only notices. +3. `chat replay` should be explicit about the agent key requirement. A session key alone is not enough for current `chat.history`, which requires `agent_key`. +4. `chat sessions resume ` should be framed as discoverability around existing `chat --session `, not a new stateful launcher unless the command accepts/resolves an agent. +5. `config defaults` is master-scope gated server-side. Tests should assert method dispatch, not assume owner-only auth. + +## Required Plan Adjustments + +- Update phase text from `chat replay/resume` to `chat replay/resume convenience wrappers`. +- State that `api-keys rotate` is non-atomic composite with emit-before-revoke partial failure handling. +- State `tools invoke --args` is an alias for `--params` and supports `@file`. +- Add tests for exact command registration and JSON/stdout safety. + +## Unresolved Questions + +1. Should `chat replay --session ` be the command shape instead of `chat replay `? +2. Should legacy codex-pool commands show deprecation notices now, or only mark `Deprecated` in Cobra help without runtime stderr? diff --git a/plans/260520-1050-domain-coverage-p5-fillers/phase-01-scope-lock.md b/plans/260520-1050-domain-coverage-p5-fillers/phase-01-scope-lock.md new file mode 100644 index 0000000..8950169 --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/phase-01-scope-lock.md @@ -0,0 +1,72 @@ +--- +phase: 1 +title: "Scope Lock" +status: complete +priority: P1 +effort: "45m" +dependencies: [] +--- + +# Phase 1: Scope Lock + +## Overview + +Lock exact P5 contract before implementation. This phase prevents reintroducing already-covered backlog items or implementing against stale server assumptions. + +## Requirements + +- Functional: confirm only two residual commands are implemented. +- Functional: verify server route, HTTP method, request body, and auth mode for each command. +- Non-functional: keep implementation CLI-only and small. +- Non-functional: preserve existing command behavior unless a contract mismatch is verified. + +## Architecture + +Inputs: +- Parent plan: `../260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md` +- Server route: `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go` +- Server route: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go` +- Server helper: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_skill_apply.go` + +Decision points: +- Use authenticated `GetRaw()` for attachment binary download because the route accepts Bearer auth. +- Use `PATCH` with `status=approved` for skill apply because server dispatches skill creation from suggestion approval. +- Fix existing `agents evolution update` payload. It currently sends stale `action=accept|reject`, while the server requires `status=approved|rejected`. + +## Related Code Files + +- Read: `cmd/agents_evolution.go` +- Read: `cmd/teams.go` +- Read: `cmd/teams_workspace.go` +- Read: `cmd/agents_lifecycle_test.go` +- Read: `cmd/storage.go` +- Read: `cmd/media.go` +- Read: `internal/client/http.go` + +## Implementation Steps + +1. Confirm `dev` is clean and synced. +2. Re-run grep on current CLI for the seven P5 sweep items. +3. Reconfirm final residual list is only X11 + X12. +4. Verify server route details: + - `GET /v1/teams/{teamId}/attachments/{attachmentId}/download` + - `PATCH /v1/agents/{agentID}/evolution/suggestions/{suggestionID}` +5. Decide file boundaries: + - Create `cmd/teams_attachments.go`; `cmd/teams_workspace.go` is already near 200 LoC. + - Extend `cmd/agents_evolution.go`; expected to remain under 200 LoC. +6. Record the verified evolution update mismatch in parent P5 phase before implementation. + +## Success Criteria + +- [x] Final scope is exactly two new commands plus required evolution update compatibility fix. +- [x] Parent P5 phase links to this dedicated plan. +- [x] No server route changes are planned. +- [x] File ownership is clear before code edits. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Duplicate planning sources | Make this plan the execution plan and link it from the parent P5 phase. | +| Stale server assumptions | Use server source as source of truth before coding. | +| Hidden scope creep | Reject covered backlog items unless current code proves broken. | diff --git a/plans/260520-1050-domain-coverage-p5-fillers/phase-02-team-attachment-download.md b/plans/260520-1050-domain-coverage-p5-fillers/phase-02-team-attachment-download.md new file mode 100644 index 0000000..a97b861 --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/phase-02-team-attachment-download.md @@ -0,0 +1,98 @@ +--- +phase: 2 +title: "Team Attachment Download" +status: complete +priority: P1 +effort: "2h" +dependencies: [1] +--- + +# Phase 2: Team Attachment Download + +## Overview + +Add a direct CLI command for authenticated team task attachment downloads. The command writes binary content to an explicit output file only. + +## Requirements + +- Functional: `goclaw teams attachments download --output ` downloads the response body. +- Functional: `--output/-o` is mandatory. +- Functional: existing output file is refused unless `--force` is set. +- Functional: parent directories for output path are created. +- Functional: HTTP errors use existing raw response error handling. +- Non-functional: no stdout binary output; safer for automation and logs. +- Non-functional: use Bearer auth via `newHTTP().GetRaw()`. + +## Architecture + +Add a new teams subgroup: + +```text +teams + attachments + download --output [--force] +``` + +Data flow: + +```text +Cobra args -> validate output path -> newHTTP() + -> GET /v1/teams/{teamId}/attachments/{attachmentId}/download + -> status check -> open output file -> io.Copy(response.Body, file) + -> printer.Success("Downloaded N bytes to path") +``` + +Implementation details: +- Use `url.PathEscape` for both IDs. +- Use `os.OpenFile(path, O_WRONLY|O_CREATE|O_EXCL, 0644)` by default. +- With `--force`, use `O_WRONLY|O_CREATE|O_TRUNC`. +- Use `os.MkdirAll(filepath.Dir(outFile), 0755)`. +- Use `rawResponseError(resp)` for `resp.StatusCode >= 400`. +- After successful status check, `defer resp.Body.Close()` before opening/copying the output file. +- Do not parse `Content-Disposition` in this phase. + +## Related Code Files + +- Create: `cmd/teams_attachments.go` +- Modify: `cmd/teams.go` +- Read: `cmd/storage.go` +- Read: `cmd/media.go` +- Read: `cmd/backup.go` +- Read: `internal/client/http.go` +- Test: add coverage in a focused `cmd/p5_fillers_test.go` or existing teams test file. + +## Implementation Steps + +1. Create `teamsAttachmentsCmd` and `teamsAttachmentsDownloadCmd`. +2. Add flags: + - `StringP("output", "o", "", "Output file path")` + - `Bool("force", false, "Overwrite output file if it exists")` +3. Validate `--output` before creating HTTP client. +4. Build escaped route path. +5. Stream response to file after status check; close the response body on every success path. +6. Register `teamsAttachmentsCmd` under `teamsCmd`. +7. Add tests: + - success writes file and hits exact route. + - Authorization header is present through `newHTTP`. + - missing `--output` returns validation error before network. + - existing file without `--force` is refused. + - existing file with `--force` overwrites. + - response body closes on success and error paths where practical. + - local file `--output` does not override the root output-format contract. + +## Success Criteria + +- [x] `teams attachments download` appears in command tree. +- [x] Binary content is written exactly to requested output path. +- [x] No accidental overwrite without `--force`. +- [x] Parent directory creation works. +- [x] Tests pass with httptest. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Path traversal in output path | This writes only local user-selected output. Do not sanitize beyond parent dir creation; user controls destination. | +| Route ID special chars | `url.PathEscape` both path segments. | +| Large file memory use | Stream with `io.Copy`; no buffering entire body. | +| Wrong auth mode | Use Bearer `GetRaw`; server accepts Bearer and signed token. | diff --git a/plans/260520-1050-domain-coverage-p5-fillers/phase-03-evolution-skill-apply.md b/plans/260520-1050-domain-coverage-p5-fillers/phase-03-evolution-skill-apply.md new file mode 100644 index 0000000..c80e99f --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/phase-03-evolution-skill-apply.md @@ -0,0 +1,102 @@ +--- +phase: 3 +title: "Evolution Skill Apply" +status: complete +priority: P1 +effort: "2h" +dependencies: [1] +--- + +# Phase 3: Evolution Skill Apply + +## Overview + +Add a clear CLI wrapper for approving `skill_add` evolution suggestions. The server creates the managed skill when the suggestion is approved. + +## Requirements + +- Functional: `goclaw agents evolution skill apply ` sends `status=approved`. +- Functional: command refuses suggestions whose `suggestion_type` is not `skill_add`. +- Functional: optional `--skill-draft ` sends `skill_draft` override. +- Functional: response is printed as structured output. +- Functional: missing/invalid IDs are left to server validation; CLI only enforces arg count. +- Non-functional: no new generic evolution apply surface. +- Non-functional: preserve existing `agents evolution update` UX while fixing stale payload mapping. + +## Architecture + +Command tree: + +```text +agents + evolution + metrics + suggestions + update --action accept|reject + skill + apply [--skill-draft ] +``` + +Skill apply payload: + +```json +{ + "status": "approved", + "skill_draft": "optional override content" +} +``` + +Existing update compatibility: +- Current CLI command should map `--action=accept` to `status=approved`. +- Current CLI command should map `--action=reject` to `status=rejected`. +- This is required: current CLI sends `{"action": ...}` but server reads `status`. +- Fix it in the same file with tests because it shares the exact server route. +- Use `url.PathEscape` for agent and suggestion ID path segments in both the new skill command and the existing update command. + +## Related Code Files + +- Modify: `cmd/agents_evolution.go` +- Test: extend `cmd/agents_lifecycle_test.go` or add `cmd/p5_fillers_test.go` +- Read: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go` +- Read: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_skill_apply.go` + +## Implementation Steps + +1. Add `agentsEvolutionSkillCmd` subgroup. +2. Add `agentsEvolutionSkillApplyCmd`. +3. Add `--skill-draft` flag. +4. Use existing `readContent()` for literal or `@file` draft content. +5. PATCH `/v1/agents/{id}/evolution/suggestions/{suggestionID}` with escaped path segments and `status=approved`. +6. Preflight pending suggestions and verify the selected suggestion has `suggestion_type=skill_add`. +7. Print `unmarshalMap(data)` so JSON/YAML users get server action fields. +8. Change `agentsEvolutionUpdateCmd` body from `{"action": action}` to `{"status": mappedStatus}`. + - `accept` -> `approved` + - `reject` -> `rejected` + - Keep CLI flag as `--action` for backward compatibility. + - Update success text to avoid `acceptd`; use approved/rejected wording. +9. Add tests: + - skill apply sends `status=approved`. + - `--skill-draft=@file` includes exact content. + - non-`skill_add` suggestion is refused before approval PATCH. + - update accept maps to `status=approved`. + - update reject maps to `status=rejected`. + - evolution PATCH routes escape IDs. + - invalid update action still rejects before network. + +## Success Criteria + +- [x] `agents evolution skill apply` appears in command tree. +- [x] PATCH payload matches server contract. +- [x] Non-`skill_add` suggestions are refused before approval. +- [x] Optional draft override works from `@file`. +- [x] Existing update command remains backward-compatible at CLI flag level and sends server-compatible payload. +- [x] Tests cover both new wrapper and any payload mapping fix. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Applying non-skill suggestions | Server enforces suggestion type; CLI command name makes intent explicit. | +| Stale payload in existing update | Validate and fix mapping in same module if needed. | +| Draft content leaking in process list | `--skill-draft @file` documented as preferred for large/sensitive drafts. | +| Over-abstraction | Keep command wrapper direct; no generic suggestion apply framework. | diff --git a/plans/260520-1050-domain-coverage-p5-fillers/phase-04-tests-and-docs.md b/plans/260520-1050-domain-coverage-p5-fillers/phase-04-tests-and-docs.md new file mode 100644 index 0000000..bbb1522 --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/phase-04-tests-and-docs.md @@ -0,0 +1,78 @@ +--- +phase: 4 +title: "Tests and Docs" +status: complete +priority: P2 +effort: "1.5h" +dependencies: [2, 3] +--- + +# Phase 4: Tests and Docs + +## Overview + +Add focused regression tests and sync documentation so P5 is discoverable and the parent roadmap reflects the final residual closure. + +## Requirements + +- Functional: tests cover new commands and failure modes. +- Functional: docs list exact command shapes. +- Non-functional: no broad docs rewrite. +- Non-functional: tests must use real httptest routes, no fake command-only assertions for HTTP contracts. + +## Architecture + +Test strategy: +- HTTP `httptest.NewServer` for binary download and evolution PATCH route. +- Environment-driven auth via existing test helpers where possible. +- File output tests use `t.TempDir()`. +- Avoid touching real filesystem outside temp dirs. + +Docs strategy: +- `README.md`: add concise examples under UX/API/team/evolution sections. +- `CHANGELOG.md`: add P5 entries under Unreleased. +- `docs/codebase-summary.md`: add new files/commands. +- `docs/project-roadmap.md`: mark P5 as planned/in progress only after implementation starts; plan creation can note "P5 execution plan created". +- Parent phase: link this plan and update stale output decisions. +- Parent phase: document verified `agents evolution update` payload mismatch and that P5 fixes it. + +## Related Code Files + +- Modify: `README.md` +- Modify: `CHANGELOG.md` +- Modify: `docs/codebase-summary.md` +- Modify: `docs/project-roadmap.md` +- Modify: `plans/260503-1907-domain-coverage-p3-plus/plan.md` +- Modify: `plans/260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md` +- Test: `cmd/p5_fillers_test.go` or existing adjacent test files. + +## Implementation Steps + +1. Add tests immediately after code changes for each command. +2. Run targeted tests first: + - `/usr/local/go/bin/go test ./cmd -run 'P5|Attachment|Evolution' -count=1` +3. Run full validation: + - `/usr/local/go/bin/go build ./...` + - `/usr/local/go/bin/go test ./...` + - `/usr/local/go/bin/go vet ./...` +4. Update README and CHANGELOG with exact examples. +5. Update `docs/codebase-summary.md` command table. +6. Update `docs/project-roadmap.md` after implementation status is known. +7. Update parent P5 phase to point at this dedicated execution plan. +8. Document red-team/validation outcomes in plan reports. + +## Success Criteria + +- [x] Tests prove route, method, request body, file output, and overwrite behavior. +- [x] Full build/test/vet pass. +- [x] Docs mention both P5 commands. +- [x] Parent P5 plan no longer contradicts required `--output`. +- [x] Parent P5 plan notes the required evolution update compatibility fix. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Tests pass without checking body | Decode request body and assert exact fields. | +| Docs drift from command shape | Copy command examples from tests/help strings. | +| Parent plan stale | Update parent phase in same PR. | diff --git a/plans/260520-1050-domain-coverage-p5-fillers/phase-05-ship-readiness.md b/plans/260520-1050-domain-coverage-p5-fillers/phase-05-ship-readiness.md new file mode 100644 index 0000000..3bcc9e5 --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/phase-05-ship-readiness.md @@ -0,0 +1,73 @@ +--- +phase: 5 +title: "Ship Readiness" +status: pending +priority: P2 +effort: "1h" +dependencies: [4] +--- + +# Phase 5: Ship Readiness + +## Overview + +Prepare the P5 implementation branch for beta shipping. This phase is verification and PR hygiene only. + +## Requirements + +- Functional: final diff is one cohesive P5 PR. +- Functional: PR description lists sweep results and validation commands. +- Non-functional: no direct push to `dev`; use PR to `dev`. +- Non-functional: no untracked generated artifacts in final status. + +## Architecture + +Expected flow: + +```text +dev synced + -> feature branch/worktree + -> implement phases 2-4 + -> local validation + -> ck:git cp + -> ck:ship beta + -> ck:review-pr +``` + +Release expectation: +- Merge to `dev` triggers CI + Release workflows. +- Dev release should publish prerelease through existing semantic-release setup. + +## Implementation Steps + +1. Confirm `git status --short --branch` is clean. +2. Review diff for accidental generated files. +3. Run secret scan on staged diff. +4. Commit with conventional message, recommended: + - `feat(cli): add domain coverage P5 fillers` +5. Push branch and open PR to `dev`. +6. PR body must include: + - Commands added. + - Server contracts used. + - P5 sweep summary. + - Validation commands and results. +7. Run review and fix findings before merge. +8. After merge, watch CI + Release until complete. +9. Confirm beta release appears. + +## Success Criteria + +- [ ] Branch pushed. +- [ ] PR to `dev` opened. +- [ ] Review has no critical/important findings. +- [ ] CI pass. +- [ ] Release pass after merge. +- [ ] Beta prerelease published. + +## Risk Assessment + +| Risk | Mitigation | +|---|---| +| Release workflow fails despite local pass | Watch CI/Release and fix in follow-up branch if needed. | +| PR contains parent-plan-only noise | Keep docs updates scoped to P5 status and command examples. | +| Generated local docs/journals appear ignored | Check `git status --ignored` only if needed; do not force-add ignored journals. | diff --git a/plans/260520-1050-domain-coverage-p5-fillers/plan.md b/plans/260520-1050-domain-coverage-p5-fillers/plan.md new file mode 100644 index 0000000..47fb24f --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/plan.md @@ -0,0 +1,155 @@ +--- +title: "Domain Coverage P5 Fillers" +description: "Finish the final CLI-only P5 residuals: team attachment download and evolution skill apply." +status: in-progress +priority: P2 +branch: "dev" +tags: [domain-coverage, p5, cli] +blockedBy: [] +blocks: [260503-1907-domain-coverage-p3-plus] +created: "2026-05-20T03:50:47.956Z" +createdBy: "ck:plan" +source: skill +--- + +# Domain Coverage P5 Fillers + +## Overview + +Implement the final post-sweep CLI residuals from Domain Coverage P5 in one PR from `dev`. +Scope is intentionally small: add a direct team attachment download command and a clear evolution skill-apply wrapper over the existing server approval contract. + +## Decisions + +- Scope: one PR with both P5 commands. +- Attachment download: `--output/-o` is required. No implicit filename guessing in this round. +- Evolution skill apply: wrapper only for approving `skill_add` suggestions; optional `--skill-draft` override. +- Do not add new server routes. Server already exposes the required contracts. +- Repair existing evolution update payload: current CLI sends stale `action`, while server requires `status`. + +## Phases + +| Phase | Name | Status | +|-------|------|--------| +| 1 | [Scope Lock](./phase-01-scope-lock.md) | Complete | +| 2 | [Team Attachment Download](./phase-02-team-attachment-download.md) | Complete | +| 3 | [Evolution Skill Apply](./phase-03-evolution-skill-apply.md) | Complete | +| 4 | [Tests and Docs](./phase-04-tests-and-docs.md) | Complete | +| 5 | [Ship Readiness](./phase-05-ship-readiness.md) | Pending | + +## Dependencies + +- Parent backlog: `../260503-1907-domain-coverage-p3-plus/plan.md` +- P5 legacy phase: `../260503-1907-domain-coverage-p3-plus/phase-05-fillers-verification-batch-2.md` +- Server evidence: + - `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go` + - `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go` + - `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_skill_apply.go` + +## Command Contract + +### Team Attachment Download + +```bash +goclaw teams attachments download --output ./artifact.bin +goclaw teams attachments download -o ./artifact.bin --force +``` + +Behavior: +- Calls `GET /v1/teams/{teamId}/attachments/{attachmentId}/download` using normal Bearer auth. +- Requires `--output/-o`; refuses missing output before network call. +- Refuses overwrite unless `--force` is set. +- Creates parent directory when needed. +- Streams binary response to disk. + +### Evolution Skill Apply + +```bash +goclaw agents evolution skill apply +goclaw agents evolution skill apply --skill-draft @./SKILL.md +``` + +Behavior: +- Calls `PATCH /v1/agents/{agentID}/evolution/suggestions/{suggestionID}`. +- Preflights pending suggestions and refuses non-`skill_add` suggestion IDs. +- Sends `{"status":"approved"}` by default. +- Adds `skill_draft` when `--skill-draft` is provided; supports literal content or `@file` via existing `readContent()`. +- Prints structured server response with `printer.Print(unmarshalMap(data))`. +- Uses escaped path segments for agent and suggestion IDs. + +## Scope Boundary + +In scope: +- Two new CLI surfaces above. +- Tests for route, body, file writing, required output, overwrite guard, and draft override. +- Docs sync in `CHANGELOG.md`, `README.md`, `docs/codebase-summary.md`, `docs/project-roadmap.md`, and parent P5 phase. + +Out of scope: +- New server routes. +- Generic "apply any evolution suggestion" command. +- Signed URL discovery from team task detail. +- Auto filename extraction from `Content-Disposition`. +- Large refactors of teams or evolution command groups. + +## Validation Gates + +- `/usr/local/go/bin/go build ./...` — pass 2026-05-20 +- `/usr/local/go/bin/go test ./...` — pass 2026-05-20 +- `/usr/local/go/bin/go vet ./...` — pass 2026-05-20 +- Manual PR review against server source contracts. + +## Red Team Review + +### Findings + +| ID | Severity | Finding | Evidence | Disposition | +|---|---|---|---|---| +| RT-01 | High | Existing `agents evolution update` is not just "maybe stale"; it is verified stale and must be fixed with P5. | `cmd/agents_evolution.go:88-90`; `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:199-213` | Accepted | +| RT-02 | Medium | Evolution PATCH paths should escape IDs. New code must not copy the current unescaped `fmt.Sprintf` path pattern. | `cmd/agents_evolution.go:88-89`; `cmd/api_keys_rotate.go:44`; `cmd/storage.go:55` | Accepted | +| RT-03 | Medium | Binary download plan needs explicit `resp.Body.Close()` after successful status check, before file-copy returns. | `cmd/storage.go:62`; `cmd/media.go:55`; `cmd/helpers.go:120-126` | Accepted | +| RT-04 | Low | Optional `--force` was not explicitly in the user decision, but it is low-risk and bounded because default remains no-overwrite. | `plans/260520-1050-domain-coverage-p5-fillers/phase-02-team-attachment-download.md:19-20` | Accepted | +| CR-01 | Medium | Local attachment `--output` flag can shadow the root output-format flag unless config resolution reads the persistent root flag. | `cmd/root.go`; `internal/config/config.go`; `cmd/teams_attachments.go:91` | Accepted | +| CR-02 | Medium | Skill-specific wrapper must not approve non-`skill_add` suggestions through the generic server route. | `/Volumes/GOON/www/digitop/goclaw/internal/http/evolution_handlers.go:223-245` | Accepted | + +### Whole-Plan Consistency Sweep + +- Files reread: `plan.md`, `phase-01-scope-lock.md`, `phase-02-team-attachment-download.md`, `phase-03-evolution-skill-apply.md`, `phase-04-tests-and-docs.md`, `phase-05-ship-readiness.md`. +- Decision deltas checked: stale `action` payload, escaped IDs, response close, `--force` no-overwrite default. +- Reconciled stale references: optional update fix converted to required update fix. +- Unresolved contradictions: 0 + +## Validation Log + +### Verification Results + +- **Tier:** Full +- **Claims checked:** 24 +- **Verified:** 24 | **Failed:** 0 | **Unverified:** 0 + +Verified examples: +- Team attachment route exists and accepts Bearer auth: `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:25-53`. +- Team attachment route validates team and attachment IDs: `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:64-84`. +- Server exposes `Content-Disposition`, but user decision requires explicit `--output`: `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:104-112`. +- Evolution PATCH route exists: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:65-69`. +- Evolution PATCH body requires `status`, not `action`: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:199-213`. +- Skill creation dispatches when `status=approved` and suggestion type is `SuggestSkillAdd`: `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:223-232`. + +### Critical Questions + +| Question | Decision | +|---|---| +| Should P5 ship both residual commands in one PR? | Yes, user confirmed both commands in one PR. | +| Should attachment download infer filename from `Content-Disposition`? | No, user confirmed `--output/-o` is required. | +| Should evolution skill apply be generic or a skill-specific wrapper? | Skill-specific wrapper only, user confirmed recommended approach. | +| Should stale `agents evolution update` payload be handled? | Yes, validation found it is a verified contract mismatch, so fix with P5. | + +### Whole-Plan Consistency Sweep + +- Files reread: `plan.md`, all five phase files. +- Decision deltas checked: required output, skill-specific apply, status payload, path escaping, response close. +- Reconciled stale references: 5. +- Unresolved contradictions: 0 + +## Handoff + +Recommended next gate after implementation: `/ck:git cp` then `/ck:ship beta`. diff --git a/plans/260520-1050-domain-coverage-p5-fillers/reports/red-team-260520-p5.md b/plans/260520-1050-domain-coverage-p5-fillers/reports/red-team-260520-p5.md new file mode 100644 index 0000000..f8f1704 --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/reports/red-team-260520-p5.md @@ -0,0 +1,32 @@ +# Red Team Report: Domain Coverage P5 Fillers + +## Summary + +Adversarial review found one high-severity contract bug already present in the planned touchpoint and two medium implementation hazards. All accepted findings were propagated into plan files. + +## Findings + +| ID | Severity | Finding | Evidence | Disposition | +|---|---|---|---|---| +| RT-01 | High | Existing `agents evolution update` sends `action`, but server requires `status`; P5 must fix it, not leave it conditional. | `cmd/agents_evolution.go:88-90`; `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:199-213` | Accepted | +| RT-02 | Medium | New evolution PATCH code must escape path segments; current update command does not. | `cmd/agents_evolution.go:88-89`; `cmd/api_keys_rotate.go:44`; `cmd/storage.go:55` | Accepted | +| RT-03 | Medium | Binary download plan must explicitly close response body after successful status check. | `cmd/storage.go:62`; `cmd/media.go:55`; `cmd/helpers.go:120-126` | Accepted | +| RT-04 | Low | `--force` was not part of explicit user decision, but default no-overwrite keeps it safe and practical. | `plans/260520-1050-domain-coverage-p5-fillers/phase-02-team-attachment-download.md:19-20` | Accepted | + +## Plan Changes + +- Phase 3 now requires the update payload compatibility fix. +- Phase 3 now requires escaped PATCH path segments. +- Phase 2 now requires response body close on successful download path. +- Parent P5 plan now states X8 is surface-covered but payload compatibility needs P5 fix. + +## Whole-Plan Consistency Sweep + +- Files reread: `plan.md`, `phase-01-scope-lock.md`, `phase-02-team-attachment-download.md`, `phase-03-evolution-skill-apply.md`, `phase-04-tests-and-docs.md`, `phase-05-ship-readiness.md`. +- Decision deltas checked: stale `action` payload, escaped IDs, response close, no-overwrite default. +- Reconciled stale references: 5. +- Unresolved contradictions: 0 + +## Unresolved Questions + +None. diff --git a/plans/260520-1050-domain-coverage-p5-fillers/reports/validation-260520-p5.md b/plans/260520-1050-domain-coverage-p5-fillers/reports/validation-260520-p5.md new file mode 100644 index 0000000..63d8911 --- /dev/null +++ b/plans/260520-1050-domain-coverage-p5-fillers/reports/validation-260520-p5.md @@ -0,0 +1,53 @@ +# Validation Report: Domain Coverage P5 Fillers + +## Summary + +Validation confirmed the plan is implementable from current `dev` with no server changes. User decisions already resolve the main product questions: both commands in one PR, required `--output/-o`, and skill-specific apply wrapper. + +## Verification Results + +- **Tier:** Full +- **Claims checked:** 24 +- **Verified:** 24 +- **Failed:** 0 +- **Unverified:** 0 + +## Verified Claims + +| Claim | Result | Evidence | +|---|---|---| +| Team attachment route exists. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:25-28` | +| Team attachment route accepts Bearer auth. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:42-52` | +| Team attachment route validates team ownership. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:76-85` | +| Team attachment route serves binary file response. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/team_attachments.go:104-117` | +| Existing CLI raw downloads use `GetRaw`. | VERIFIED | `cmd/storage.go:55`; `cmd/media.go:48`; `cmd/backup.go:172` | +| Evolution PATCH route exists. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:65-69` | +| Evolution PATCH body uses `status`. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:199-213` | +| Skill draft override field is `skill_draft`. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:199-203` | +| Skill add is applied on `status=approved`. | VERIFIED | `/Volumes/GOON/www/nlb/goclaw/internal/http/evolution_handlers.go:223-232` | +| Current CLI update payload is stale. | VERIFIED | `cmd/agents_evolution.go:88-90` | + +## Critical Questions + +| Question | Answer | +|---|---| +| What artifact should P5 produce? | One PR implementing `teams attachments download` and `agents evolution skill apply`, plus update payload compatibility fix. | +| Acceptance criteria? | Commands hit verified server routes, tests prove payload/file behavior, build/test/vet pass. | +| Scope boundary? | No server changes, no generic evolution apply, no filename inference. | +| Non-negotiable constraints? | Required `--output/-o`; skill apply wrapper only; PR to `dev`. | +| Touchpoints? | `cmd/teams.go`, new `cmd/teams_attachments.go`, `cmd/agents_evolution.go`, focused tests, README/CHANGELOG/docs/plans. | + +## Whole-Plan Consistency Sweep + +- Files reread: `plan.md`, all five phase files, parent P5 phase. +- Decision deltas checked: required output, skill-specific apply, status payload, escaped IDs, response close. +- Reconciled stale references: 5. +- Unresolved contradictions: 0 + +## Recommendation + +Proceed to implementation after user approval. Suggested command: + +```bash +/ck:cook /Users/duynguyen/.codex/worktrees/a69b/goclaw-cli/plans/260520-1050-domain-coverage-p5-fillers/plan.md +```