From 30a600b7ec49b9dd467d8342df1b7021be18403a Mon Sep 17 00:00:00 2001 From: Goon Date: Tue, 19 May 2026 22:18:45 +0700 Subject: [PATCH] feat(cli): add domain coverage P4 UX polish --- CHANGELOG.md | 9 +- README.md | 24 +- cmd/agents_misc.go | 5 +- cmd/api_keys.go | 13 +- cmd/api_keys_helpers.go | 20 ++ cmd/api_keys_rotate.go | 85 ++++++ cmd/chat_ai_commands.go | 27 +- cmd/chat_replay.go | 52 ++++ cmd/chat_sessions.go | 29 ++ cmd/cmd_test.go | 2 + cmd/codex_pool.go | 45 +++ cmd/config_defaults.go | 28 ++ cmd/p4_ux_polish_test.go | 287 ++++++++++++++++++ cmd/providers_codex_pool.go | 7 +- cmd/tools_custom.go | 18 +- cmd/tools_invoke_args.go | 45 +++ docs/codebase-summary.md | 10 +- docs/project-roadmap.md | 20 +- .../phase-04-ux-polish-batch-1.md | 53 ++-- .../plan.md | 11 +- .../reports/validation-red-team-260519-p4.md | 35 +++ 21 files changed, 738 insertions(+), 87 deletions(-) create mode 100644 cmd/api_keys_helpers.go create mode 100644 cmd/api_keys_rotate.go create mode 100644 cmd/chat_replay.go create mode 100644 cmd/chat_sessions.go create mode 100644 cmd/codex_pool.go create mode 100644 cmd/config_defaults.go create mode 100644 cmd/p4_ux_polish_test.go create mode 100644 cmd/tools_invoke_args.go create mode 100644 plans/260503-1907-domain-coverage-p3-plus/reports/validation-red-team-260519-p4.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 20254c7..af4e11c 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–P4) ### Added @@ -34,6 +34,13 @@ 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. + ### 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. diff --git a/README.md b/README.md index 155bd29..709fb7a 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ echo "Analyze this log" | goclaw chat myagent | `agents` | CRUD, shares, delegation links, per-user instances | | `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 | @@ -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,31 @@ 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 +``` + ## API Docs ```bash 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/p4_ux_polish_test.go b/cmd/p4_ux_polish_test.go new file mode 100644 index 0000000..dbca00b --- /dev/null +++ b/cmd/p4_ux_polish_test.go @@ -0,0 +1,287 @@ +package cmd + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "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", "") + 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"); err != nil { + t.Fatalf("tools invoke: %v", err) + } + params := body["parameters"].(map[string]any) + if params["city"] != "Saigon" || params["unit"] != "c" { + t.Fatalf("params = %#v", params) + } +} + +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/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/tools_custom.go b/cmd/tools_custom.go index 06e3879..ba8bfd2 100644 --- a/cmd/tools_custom.go +++ b/cmd/tools_custom.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/url" - "strings" "github.com/nextlevelbuilder/goclaw-cli/internal/output" "github.com/nextlevelbuilder/goclaw-cli/internal/tui" @@ -153,19 +152,9 @@ 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} data, err := c.Post("/v1/tools/invoke", body) @@ -190,6 +179,7 @@ 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") 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/docs/codebase-summary.md b/docs/codebase-summary.md index 8ec5939..32557b8 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 +**Phase Status:** P0-P4 Complete (AI-First Expansion); Super Admin API Parity Complete; Domain Coverage P4 Complete **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`. **Key Metrics:** - **70+ command files** in `cmd/` (modularized for maintainability) @@ -633,6 +633,12 @@ goclaw vault | `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 | diff --git a/docs/project-roadmap.md b/docs/project-roadmap.md index b220ecb..6d30054 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -2,8 +2,24 @@ **Last Updated:** 2026-05-19 **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 P4 ✓ COMPLETE +**Next Phase:** Domain Coverage residuals: P5 team attachment download + evolution skill apply. + +--- + +## 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 ./...`. --- 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/plan.md b/plans/260503-1907-domain-coverage-p3-plus/plan.md index 5dae50e..145293a 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 next; P6 remains server-blocked. ## Summary @@ -12,7 +12,7 @@ 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 | +| 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 | 🟡 | not-started | | P6 | Deferred — blocked on server FRs (traces follow, logs aggregate, providers reconnect, …) | n/a | 🟢 | server-blocked | @@ -25,10 +25,11 @@ 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. - 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 +42,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?