From dad4b898ce68526f7fe1826d1d00d165784f5c4e Mon Sep 17 00:00:00 2001 From: Goon Date: Wed, 20 May 2026 14:25:04 +0700 Subject: [PATCH] feat(cli): add domain coverage p5 fillers --- CHANGELOG.md | 9 +- README.md | 10 +- cmd/agents_evolution.go | 86 +++++- cmd/p5_fillers_test.go | 283 ++++++++++++++++++ cmd/root.go | 4 +- cmd/teams.go | 2 +- cmd/teams_attachments.go | 94 ++++++ docs/codebase-summary.md | 15 +- docs/project-roadmap.md | 24 +- internal/config/config.go | 41 ++- .../phase-05-fillers-verification-batch-2.md | 33 +- .../plan.md | 5 +- .../phase-01-scope-lock.md | 72 +++++ .../phase-02-team-attachment-download.md | 98 ++++++ .../phase-03-evolution-skill-apply.md | 102 +++++++ .../phase-04-tests-and-docs.md | 78 +++++ .../phase-05-ship-readiness.md | 73 +++++ .../plan.md | 155 ++++++++++ .../reports/red-team-260520-p5.md | 32 ++ .../reports/validation-260520-p5.md | 53 ++++ 20 files changed, 1220 insertions(+), 49 deletions(-) create mode 100644 cmd/p5_fillers_test.go create mode 100644 cmd/teams_attachments.go create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/phase-01-scope-lock.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/phase-02-team-attachment-download.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/phase-03-evolution-skill-apply.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/phase-04-tests-and-docs.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/phase-05-ship-readiness.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/plan.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/reports/red-team-260520-p5.md create mode 100644 plans/260520-1050-domain-coverage-p5-fillers/reports/validation-260520-p5.md diff --git a/CHANGELOG.md b/CHANGELOG.md index af4e11c..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–P4) +## [Unreleased] — Domain Coverage Expansion (P0–P5) ### Added @@ -41,9 +41,14 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `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 709fb7a..db99c34 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ 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 | @@ -63,7 +63,7 @@ echo "Analyze this log" | goclaw chat myagent | `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 | @@ -324,6 +324,12 @@ 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 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/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/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/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/docs/codebase-summary.md b/docs/codebase-summary.md index 32557b8..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 P4 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/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`. +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,7 +629,7 @@ 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 | @@ -644,6 +645,7 @@ goclaw vault | `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 | @@ -671,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 6d30054..4cb6b20 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -1,9 +1,24 @@ # 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 P4 ✓ COMPLETE -**Next Phase:** Domain Coverage residuals: 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 ./...`. --- @@ -38,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. --- @@ -460,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-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 145293a..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/P4 complete — P5 next; P6 remains server-blocked. +**Status:** P3/P4 complete — P5 implemented pending ship; P6 remains server-blocked. ## Summary @@ -13,7 +13,7 @@ Sau R1 (P0–P5) + R2 expansion (P0–P2), CLI đạt ~95% server coverage. R2 r |---|---|---|---|---| | 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 convenience, tools invoke `--args` alias) | ~250 | 🟡 | complete | -| P5 | Fillers residuals after sweep (team attachments download, evolution skill apply) | ~150 | 🟡 | not-started | +| 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 @@ -28,6 +28,7 @@ Sau R1 (P0–P5) + R2 expansion (P0–P2), CLI đạt ~95% server coverage. R2 r - 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 ./...`. 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 +```