diff --git a/cli/cmd/agent/agent.go b/cli/cmd/agent/agent.go index 100d3b793..ce5a39c01 100644 --- a/cli/cmd/agent/agent.go +++ b/cli/cmd/agent/agent.go @@ -1,7 +1,7 @@ // Package agentcmd holds the `weknora agent` command tree: -// list / view / invoke. The directory is named `agent/` (matches cobra -// noun-verb convention) but the Go package is `agentcmd` to avoid -// colliding with cobra's *cobra.Command identifier. +// list / view / invoke / create / edit / delete. The directory is named +// `agent/` (matches cobra noun-verb convention) but the Go package is +// `agentcmd` to avoid colliding with cobra's *cobra.Command identifier. // // "agent" in this subtree refers to WeKnora's user-defined Custom // Agents (server resource: GET/POST /agents/...). The CLI's @@ -23,13 +23,16 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { Use: "agent", Short: "Manage and invoke custom agents", Long: `Custom Agents bundle a system prompt, model, tool allow-list, and KB -scope into an addressable resource. List visible agents, view a single -agent's configuration, or invoke an agent against a query.`, +scope into an addressable resource. Create, edit, list, view, invoke, +or delete agents.`, Args: cobra.NoArgs, Run: func(c *cobra.Command, _ []string) { _ = c.Help() }, } cmd.AddCommand(NewCmdList(f)) cmd.AddCommand(NewCmdView(f)) cmd.AddCommand(NewCmdInvoke(f)) + cmd.AddCommand(NewCmdCreate(f)) + cmd.AddCommand(NewCmdEdit(f)) + cmd.AddCommand(NewCmdDelete(f)) return cmd } diff --git a/cli/cmd/agent/create.go b/cli/cmd/agent/create.go new file mode 100644 index 000000000..c51f6a650 --- /dev/null +++ b/cli/cmd/agent/create.go @@ -0,0 +1,312 @@ +package agentcmd + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// CreateService is the narrow SDK surface this command depends on. +// *sdk.Client satisfies it via duck typing. +type CreateService interface { + CreateAgent(ctx context.Context, req *sdk.CreateAgentRequest) (*sdk.Agent, error) + CopyAgent(ctx context.Context, id string) (*sdk.Agent, error) + UpdateAgent(ctx context.Context, id string, req *sdk.UpdateAgentRequest) (*sdk.Agent, error) +} + +// CreateOptions captures flag state. SystemPromptReader and ConfigFileBody +// are populated from --system-prompt-file and --config-file respectively +// (or stdin when the value is "-"). The embedded flags struct tracks "was +// set" bits so MergeAgentConfig can distinguish "user passed --foo zero" +// from "user did not pass --foo". +type CreateOptions struct { + Name string + Model string + Description string + SystemPrompt string + SystemPromptReader io.Reader + AgentMode string + KBs []string + KBSelectionMode string + RerankModel string + Temperature float64 + From string + ConfigFileBody io.Reader + ConfigFileKind string // "yaml" or "json" + GenerateSkeleton bool + + flags createFlagSet // populated in PreRunE for *Set bits +} + +// createFlagSet records which hot-path flags the user explicitly passed so +// MergeAgentConfig knows which fields to overlay onto the base config. +type createFlagSet struct { + agentModeSet bool + systemPromptSet bool + rerankModelSet bool + temperatureSet bool + kbSelectionModeSet bool + kbsSet bool +} + +const agentCreateExample = ` weknora agent create "Support Bot" --model gpt-4 + weknora agent create "Code Reviewer" --model gpt-4 --system-prompt-file ./prompt.md --kb kb_eng --kb kb_arch + weknora agent create "From Template" --model gpt-4 --from ag_existing + weknora agent create --generate-skeleton > my-agent.yaml + weknora agent create "Tuned" --model gpt-4 --config-file ./my-agent.yaml` + +const agentCreateLong = `Create a new custom agent. + +--model is required (an agent without a model cannot invoke). The 7 +optional hot-path flags cover the most frequently set AgentConfig fields; +for the remaining 27 use --config-file with a YAML or JSON document +matching the AgentConfig schema (run --generate-skeleton to get a +ready-to-edit template). + +Precedence when both a config file and hot-path flags are supplied: + hot-path flag > config-file value > server default + +--from copies an existing agent (SDK CopyAgent) then applies any +hot-path overrides via UpdateAgent. --kb on --from REPLACES the copied +agent's KB list (not merge) — matches surgical-flag semantics. + +AI agents: writes a new resource server-side. Failure surfaces as a +typed code on stderr: input.invalid_argument (bad flags, bad file, or +bad model), resource.not_found (--from ), auth.unauthenticated.` + +// NewCmdCreate builds `weknora agent create --model `. +func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { + opts := &CreateOptions{} + var systemPromptFile, configFile string + + cmd := &cobra.Command{ + Use: "create ", + Short: "Create a new custom agent", + Long: agentCreateLong, + Example: agentCreateExample, + // Allow 0 args for --generate-skeleton; PreRunE enforces the real + // arity rule once it knows whether skeleton mode is active. + Args: cobra.MaximumNArgs(1), + PreRunE: func(cmd *cobra.Command, args []string) error { + if opts.GenerateSkeleton { + return nil + } + if len(args) != 1 { + return cmdutil.NewFlagError(fmt.Errorf("accepts 1 arg, received %d", len(args))) + } + opts.Name = args[0] + if opts.Model == "" { + return cmdutil.NewFlagError(fmt.Errorf(`required flag(s) "model" not set`)) + } + opts.flags.agentModeSet = cmd.Flags().Changed("agent-mode") + opts.flags.systemPromptSet = cmd.Flags().Changed("system-prompt") || cmd.Flags().Changed("system-prompt-file") + opts.flags.rerankModelSet = cmd.Flags().Changed("rerank-model") + opts.flags.temperatureSet = cmd.Flags().Changed("temperature") + opts.flags.kbSelectionModeSet = cmd.Flags().Changed("kb-selection-mode") + opts.flags.kbsSet = cmd.Flags().Changed("kb") + + // L-5 / §1.5.1: --temperature is bounded 0.0..2.0. Reject + // out-of-range early with a typed input.invalid_argument so + // users don't burn a roundtrip on a value the server would + // also reject. + if opts.flags.temperatureSet && (opts.Temperature < 0.0 || opts.Temperature > 2.0) { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + fmt.Sprintf("--temperature must be in 0.0..2.0, got %g", opts.Temperature)) + } + + if systemPromptFile != "" { + r, err := openInput(systemPromptFile) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--system-prompt-file: %v", err)) + } + opts.SystemPromptReader = r + } + if configFile != "" { + r, kind, err := openConfigFile(configFile) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--config-file: %v", err)) + } + opts.ConfigFileBody = r + opts.ConfigFileKind = kind + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + jopts, err := cmdutil.CheckJSONFlags(cmd) + if err != nil { + return err + } + cli, err := f.Client() + if err != nil { + return err + } + return runCreate(cmd.Context(), opts, jopts, cli) + }, + } + + // Required (enforced in PreRunE rather than cmd.MarkFlagRequired so + // --generate-skeleton can bypass it — cobra's MarkFlagRequired runs + // before PreRunE and would otherwise block the skeleton path). + cmd.Flags().StringVar(&opts.Model, "model", "", "LLM model id (required, except with --generate-skeleton)") + + // Hot-path (8 flag names, 7 distinct config fields) + cmd.Flags().StringVar(&opts.Description, "description", "", "Agent description") + cmd.Flags().StringVar(&opts.SystemPrompt, "system-prompt", "", "System prompt text (mutex with --system-prompt-file)") + cmd.Flags().StringVar(&systemPromptFile, "system-prompt-file", "", "Read system prompt from FILE, or '-' for stdin") + cmd.MarkFlagsMutuallyExclusive("system-prompt", "system-prompt-file") + cmd.Flags().StringVar(&opts.AgentMode, "agent-mode", "", "Agent operating mode: quick-answer | smart-reasoning") + cmd.Flags().StringSliceVar(&opts.KBs, "kb", nil, "Attach knowledge base id (repeatable)") + cmd.Flags().StringVar(&opts.KBSelectionMode, "kb-selection-mode", "", "KB selection mode: all | selected | none") + cmd.Flags().StringVar(&opts.RerankModel, "rerank-model", "", "Rerank model id") + cmd.Flags().Float64Var(&opts.Temperature, "temperature", 0.0, "Generation temperature (0.0..2.0)") + + // Power-user / utility + cmd.Flags().StringVar(&opts.From, "from", "", "Copy from existing agent id (then apply other flags)") + cmd.Flags().StringVar(&configFile, "config-file", "", "Full AgentConfig YAML or JSON (use '-' for stdin)") + cmd.Flags().BoolVar(&opts.GenerateSkeleton, "generate-skeleton", false, "Emit blank AgentConfig YAML to stdout and exit") + + cmdutil.AddJSONFlags(cmd, agentViewFields) + return cmd +} + +func runCreate(ctx context.Context, opts *CreateOptions, jopts *cmdutil.JSONOptions, svc CreateService) error { + if opts.GenerateSkeleton { + return cmdutil.GenerateAgentSkeleton(iostreams.IO.Out) + } + + // 1. Build base AgentConfig from --config-file (if any), else zero. + // (For --from, the copied agent's existing config becomes the base + // instead — set after CopyAgent below, before MergeAgentConfig.) + var base sdk.AgentConfig + if opts.ConfigFileBody != nil { + parsed, err := cmdutil.LoadAgentConfig(opts.ConfigFileBody, opts.ConfigFileKind) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, err.Error()) + } + base = *parsed + } + + // 2. Resolve system prompt (file/stdin > flag string) + if opts.SystemPromptReader != nil { + body, err := io.ReadAll(opts.SystemPromptReader) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--system-prompt-file read: %v", err)) + } + opts.SystemPrompt = strings.TrimSpace(string(body)) + } + + // 3. Either copy-then-update, or create from scratch. For --from we + // must seed `base` from the copied agent's existing config FIRST so + // MergeAgentConfig preserves source fields the user did not override + // (e.g. SystemPrompt, AgentMode, KB list). Without this seeding the + // surgical overrides would ship UpdateAgent with the other 33 fields + // zeroed, clobbering source state. + if opts.From != "" { + copied, err := svc.CopyAgent(ctx, opts.From) + if err != nil { + return cmdutil.WrapHTTP(err, "copy agent %s", opts.From) + } + if copied.Config != nil { + base = *copied.Config + } + + // --kb on --from REPLACES the copied KB list (per spec); when --kb + // is not set, KnowledgeBasesSet stays false inside applyCreateOverrides + // and the copy's KB list passes through unchanged. + cfg := applyCreateOverrides(&base, opts) + + // Apply overrides on top of copied state via UpdateAgent (no + // server-side template-parameters route, so we do two roundtrips). + updateReq := &sdk.UpdateAgentRequest{ + Name: opts.Name, + Description: opts.Description, + Config: cfg, + } + updated, err := svc.UpdateAgent(ctx, copied.ID, updateReq) + if err != nil { + return cmdutil.WrapHTTP(err, "update copied agent %s", copied.ID) + } + return emitAgent(jopts, updated) + } + + // 4. Plain create path: apply hot-path flag overrides onto base + // (zero-valued unless --config-file supplied content). + cfg := applyCreateOverrides(&base, opts) + + req := &sdk.CreateAgentRequest{ + Name: opts.Name, + Description: opts.Description, + Config: cfg, + } + created, err := svc.CreateAgent(ctx, req) + if err != nil { + return cmdutil.WrapHTTP(err, "create agent") + } + return emitAgent(jopts, created) +} + +// applyCreateOverrides merges hot-path flag overrides into the base config, +// then applies the "--kb implies --kb-selection-mode=selected" fallback. +// Shared by the --from path (base=copied agent's config) and the plain +// create path (base=zero or --config-file). Keeping both paths on one +// helper prevents silent divergence when future flags are added. +func applyCreateOverrides(base *sdk.AgentConfig, opts *CreateOptions) *sdk.AgentConfig { + overrides := cmdutil.AgentConfigFlags{ + AgentMode: opts.AgentMode, AgentModeSet: opts.flags.agentModeSet, + SystemPrompt: opts.SystemPrompt, SystemPromptSet: opts.flags.systemPromptSet, + ModelID: opts.Model, ModelIDSet: true, // --model is required + RerankModelID: opts.RerankModel, RerankModelIDSet: opts.flags.rerankModelSet, + Temperature: opts.Temperature, TemperatureSet: opts.flags.temperatureSet, + KBSelectionMode: opts.KBSelectionMode, KBSelectionModeSet: opts.flags.kbSelectionModeSet, + KnowledgeBases: opts.KBs, KnowledgeBasesSet: opts.flags.kbsSet, + } + cfg := cmdutil.MergeAgentConfig(base, overrides) + + // --kb without explicit --kb-selection-mode implies "selected". + if opts.flags.kbsSet && !opts.flags.kbSelectionModeSet && cfg.KBSelectionMode == "" { + cfg.KBSelectionMode = "selected" + } + return cfg +} + +// openInput returns a Reader for path; "-" means the global stdin. +func openInput(path string) (io.Reader, error) { + if path == "-" { + return iostreams.IO.In, nil + } + return os.Open(path) +} + +// openConfigFile returns a Reader, the detected kind ("yaml"/"json"), or +// an error. Format is inferred from file extension; "-" defaults to YAML. +func openConfigFile(path string) (io.Reader, string, error) { + r, err := openInput(path) + if err != nil { + return nil, "", err + } + kind := "yaml" + if strings.EqualFold(filepath.Ext(path), ".json") { + kind = "json" + } + return r, kind, nil +} + +// emitAgent writes the Agent to stdout per the v0.4 wire contract (bare +// SDK shape for --json, human KV otherwise). Shared by create and edit; +// defined here for proximity to the create flow. +func emitAgent(jopts *cmdutil.JSONOptions, ag *sdk.Agent) error { + if jopts.Enabled() { + return jopts.Emit(iostreams.IO.Out, ag) + } + renderAgent(iostreams.IO.Out, ag) + return nil +} diff --git a/cli/cmd/agent/create_test.go b/cli/cmd/agent/create_test.go new file mode 100644 index 000000000..afaacde07 --- /dev/null +++ b/cli/cmd/agent/create_test.go @@ -0,0 +1,234 @@ +package agentcmd + +import ( + "bytes" + "context" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// fakeCreateSvc records all three SDK methods this command may invoke. +type fakeCreateSvc struct { + createReq *sdk.CreateAgentRequest + createResp *sdk.Agent + createErr error + copySrcID string + copyResp *sdk.Agent + copyErr error + updateID string + updateReq *sdk.UpdateAgentRequest + updateResp *sdk.Agent + updateErr error + updateCalled bool +} + +func (f *fakeCreateSvc) CreateAgent(_ context.Context, req *sdk.CreateAgentRequest) (*sdk.Agent, error) { + f.createReq = req + return f.createResp, f.createErr +} +func (f *fakeCreateSvc) CopyAgent(_ context.Context, id string) (*sdk.Agent, error) { + f.copySrcID = id + return f.copyResp, f.copyErr +} +func (f *fakeCreateSvc) UpdateAgent(_ context.Context, id string, req *sdk.UpdateAgentRequest) (*sdk.Agent, error) { + f.updateCalled = true + f.updateID = id + f.updateReq = req + return f.updateResp, f.updateErr +} + +func TestCreate_HappyPath_MinimalRequired(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{createResp: &sdk.Agent{ID: "ag_new", Name: "Test"}} + opts := &CreateOptions{Name: "Test", Model: "gpt-4"} + err := runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc) + require.NoError(t, err) + require.NotNil(t, svc.createReq) + assert.Equal(t, "Test", svc.createReq.Name) + require.NotNil(t, svc.createReq.Config) + assert.Equal(t, "gpt-4", svc.createReq.Config.ModelID) +} + +func TestCreate_MissingName_FlagError(t *testing.T) { + cmd := NewCmdCreate(nil) + cmd.SetArgs([]string{"--model", "gpt-4"}) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + require.Error(t, err) + // PreRunE rejects "0 args" with our flag-error sentinel; the message + // always carries the canonical "accepts 1 arg" phrase. + assert.Contains(t, err.Error(), "accepts 1 arg") +} + +func TestCreate_MissingModel_FlagError(t *testing.T) { + cmd := NewCmdCreate(nil) + cmd.SetArgs([]string{"Test"}) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), `required flag(s) "model" not set`) +} + +func TestCreate_ConfigFile_FlagsOverrideFile(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{createResp: &sdk.Agent{ID: "ag_new"}} + opts := &CreateOptions{ + Name: "Test", + Model: "gpt-4", // override file + ConfigFileBody: bytes.NewBufferString(`{"agent_mode":"smart-reasoning","model_id":"gpt-3.5","temperature":0.5}`), + ConfigFileKind: "json", + } + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + require.NotNil(t, svc.createReq.Config) + assert.Equal(t, "smart-reasoning", svc.createReq.Config.AgentMode, "file value preserved when no flag override") + assert.Equal(t, "gpt-4", svc.createReq.Config.ModelID, "flag overrides file") + assert.InDelta(t, 0.5, svc.createReq.Config.Temperature, 0.001) +} + +func TestCreate_From_CopiesThenUpdates(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{ + copyResp: &sdk.Agent{ID: "ag_clone", Name: "Source", Config: &sdk.AgentConfig{ModelID: "gpt-3.5"}}, + updateResp: &sdk.Agent{ID: "ag_clone", Name: "Renamed"}, + } + opts := &CreateOptions{Name: "Renamed", Model: "gpt-4", From: "ag_source"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, "ag_source", svc.copySrcID) + require.True(t, svc.updateCalled, "must Update after Copy when overrides present") + assert.Equal(t, "ag_clone", svc.updateID) + assert.Equal(t, "Renamed", svc.updateReq.Name) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, "gpt-4", svc.updateReq.Config.ModelID) +} + +func TestCreate_GenerateSkeleton_NoAPICall(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{} + opts := &CreateOptions{GenerateSkeleton: true} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Nil(t, svc.createReq, "must not call CreateAgent") + assert.Equal(t, "", svc.copySrcID, "must not call CopyAgent") + assert.Contains(t, out.String(), "agent_mode:", "skeleton emitted to stdout") +} + +func TestCreate_RepeatedKB_ImpliesSelectedMode(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{createResp: &sdk.Agent{ID: "ag_new"}} + opts := &CreateOptions{ + Name: "Test", + Model: "gpt-4", + KBs: []string{"kb_a", "kb_b"}, + flags: createFlagSet{kbsSet: true}, + } + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, []string{"kb_a", "kb_b"}, svc.createReq.Config.KnowledgeBases) + assert.Equal(t, "selected", svc.createReq.Config.KBSelectionMode, "passing --kb implies selected mode") +} + +func TestCreate_SystemPromptFile_ReaderRead(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{createResp: &sdk.Agent{ID: "ag_new"}} + opts := &CreateOptions{ + Name: "Test", + Model: "gpt-4", + SystemPromptReader: strings.NewReader("You are a helpful assistant.\n"), + flags: createFlagSet{systemPromptSet: true}, + } + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, "You are a helpful assistant.", svc.createReq.Config.SystemPrompt, "TrimSpace removes trailing newline") +} + +func TestCreate_From_PreservesSourceFieldsNotOverridden(t *testing.T) { + // Regression: with --from X and only --temperature overridden, the + // other 33 AgentConfig fields must round-trip from the copied agent. + // Pre-fix, runCreate built `cfg` from a zero AgentConfig{} baseline, + // so UpdateAgent shipped temperature=0.9 plus every other field + // zeroed — clobbering source SystemPrompt / AgentMode / KBs. + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{ + copyResp: &sdk.Agent{ID: "ag_clone", Config: &sdk.AgentConfig{ + ModelID: "gpt-3.5", + SystemPrompt: "Source prompt", + AgentMode: "smart-reasoning", + Temperature: 0.5, + KnowledgeBases: []string{"kb_src_a", "kb_src_b"}, + }}, + updateResp: &sdk.Agent{ID: "ag_clone"}, + } + // Only --temperature overridden; other fields should round-trip. + opts := &CreateOptions{ + Name: "Renamed", Model: "gpt-3.5", From: "ag_source", + Temperature: 0.9, + flags: createFlagSet{temperatureSet: true}, + } + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + require.NotNil(t, svc.updateReq) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, "Source prompt", svc.updateReq.Config.SystemPrompt, "source SystemPrompt must round-trip") + assert.Equal(t, "smart-reasoning", svc.updateReq.Config.AgentMode, "source AgentMode must round-trip") + assert.Equal(t, []string{"kb_src_a", "kb_src_b"}, svc.updateReq.Config.KnowledgeBases, "source KB list must round-trip when --kb not passed") + assert.InDelta(t, 0.9, svc.updateReq.Config.Temperature, 0.001, "Temperature overridden") +} + +func TestCreate_From_KBReplacesSourceList(t *testing.T) { + // --kb on --from REPLACES the copied agent's KB list (spec §2.1). + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{ + copyResp: &sdk.Agent{ID: "ag_clone", Config: &sdk.AgentConfig{ + ModelID: "gpt-3.5", + KnowledgeBases: []string{"kb_src_a", "kb_src_b"}, + }}, + updateResp: &sdk.Agent{ID: "ag_clone"}, + } + opts := &CreateOptions{ + Name: "X", Model: "gpt-3.5", From: "ag_source", + KBs: []string{"kb_new"}, + flags: createFlagSet{kbsSet: true}, + } + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, []string{"kb_new"}, svc.updateReq.Config.KnowledgeBases, "--kb replaces source KB list") + assert.Equal(t, "selected", svc.updateReq.Config.KBSelectionMode, "--kb on --from implies selected mode") +} + +func TestCreate_Temperature_Bounds(t *testing.T) { + for _, badT := range []float64{-0.1, 2.1, 100.0} { + t.Run(fmt.Sprintf("t=%g", badT), func(t *testing.T) { + cmd := NewCmdCreate(nil) + cmd.SetArgs([]string{"Test", "--model", "gpt-4", "--temperature", fmt.Sprintf("%f", badT)}) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + require.Error(t, err, "expected error for --temperature %g", badT) + assert.Contains(t, err.Error(), "0.0..2.0") + }) + } +} + +func TestCreate_CopyAgent_NotFound(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{copyErr: errBadHTTP404} + opts := &CreateOptions{Name: "X", Model: "gpt-4", From: "ag_missing"} + err := runCreate(context.Background(), opts, &cmdutil.JSONOptions{}, svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "resource.not_found") +} + +// errBadHTTP404 simulates the SDK's "HTTP error 404: not found" format that +// ClassifyHTTPError parses. Defined here so create_test and edit_test/delete_test +// can share it via package scope without spinning up an HTTP server. +var errBadHTTP404 = &simpleErr{msg: "HTTP error 404: not found"} + +type simpleErr struct{ msg string } + +func (e *simpleErr) Error() string { return e.msg } diff --git a/cli/cmd/agent/delete.go b/cli/cmd/agent/delete.go new file mode 100644 index 000000000..0343c7459 --- /dev/null +++ b/cli/cmd/agent/delete.go @@ -0,0 +1,97 @@ +package agentcmd + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/prompt" +) + +// agentDeleteFields enumerates the JSON discovery fields for `agent delete`. +// Result payload is a tiny {id, deleted} object — mirrors `kb delete`. +var agentDeleteFields = []string{"id", "deleted"} + +// DeleteOptions captures `agent delete` flag state. +type DeleteOptions struct { + AgentID string + Yes bool // sourced from the global -y/--yes persistent flag +} + +// DeleteService is the narrow SDK surface this command depends on. +type DeleteService interface { + DeleteAgent(ctx context.Context, id string) error +} + +// deleteResult is the typed payload emitted on success in JSON mode. +type deleteResult struct { + ID string `json:"id"` + Deleted bool `json:"deleted"` +} + +// Delete is NOT idempotent on a missing id — it surfaces resource.not_found +// rather than silently exiting 0. Idempotent-already-true semantics are +// reserved for unlink-style local cleanups, not server-side resource removal. +const agentDeleteLong = `Permanently delete a custom agent. + +Prompts for confirmation by default when stdout is a TTY and --json is +not set. Pass -y/--yes (the global flag) to skip the prompt (required in +agent / CI / piped contexts). + +Typed exit codes: + resource.not_found no agent with the given id (exit 4) + auth.forbidden caller lacks delete permission on the agent (exit 3) + input.confirmation_required destructive op without -y on a TTY (exit 10) + +AI agents: This is a high-risk write. Without -y/--yes the CLI exits 10 +and writes input.confirmation_required to stderr. NEVER auto-pass -y +without the user's explicit go-ahead — the exit-10 protocol exists +exactly to guard against unintended deletes.` + +const agentDeleteExample = ` weknora agent delete ag_abc # interactive confirm + weknora agent delete ag_abc -y # no prompt + weknora agent delete ag_abc -y --json # bare {id, deleted:true} JSON` + +// NewCmdDelete builds `weknora agent delete `. +func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { + opts := &DeleteOptions{} + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete a custom agent", + Long: agentDeleteLong, + Example: agentDeleteExample, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + jopts, err := cmdutil.CheckJSONFlags(cmd) + if err != nil { + return err + } + opts.AgentID = args[0] + opts.Yes, _ = cmd.Flags().GetBool("yes") + cli, err := f.Client() + if err != nil { + return err + } + return runDelete(cmd.Context(), opts, jopts, cli, f.Prompter()) + }, + } + cmdutil.AddJSONFlags(cmd, agentDeleteFields) + return cmd +} + +func runDelete(ctx context.Context, opts *DeleteOptions, jopts *cmdutil.JSONOptions, svc DeleteService, p prompt.Prompter) error { + if err := cmdutil.ConfirmDestructive(p, opts.Yes, jopts.Enabled(), "agent", opts.AgentID); err != nil { + return err + } + if err := svc.DeleteAgent(ctx, opts.AgentID); err != nil { + return cmdutil.WrapHTTP(err, "delete agent %s", opts.AgentID) + } + if jopts.Enabled() { + return jopts.Emit(iostreams.IO.Out, deleteResult{ID: opts.AgentID, Deleted: true}) + } + fmt.Fprintf(iostreams.IO.Out, "✓ Deleted agent %s\n", opts.AgentID) + return nil +} diff --git a/cli/cmd/agent/delete_test.go b/cli/cmd/agent/delete_test.go new file mode 100644 index 000000000..b85785fe9 --- /dev/null +++ b/cli/cmd/agent/delete_test.go @@ -0,0 +1,107 @@ +package agentcmd + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/testutil" +) + +type fakeDeleteSvc struct { + gotID string + err error +} + +func (f *fakeDeleteSvc) DeleteAgent(_ context.Context, id string) error { + f.gotID = id + return f.err +} + +func TestDelete_NonTTY_NoYes_ExitTen(t *testing.T) { + _, _ = iostreams.SetForTest(t) // non-TTY + svc := &fakeDeleteSvc{} + err := runDelete( + context.Background(), + &DeleteOptions{AgentID: "ag_abc", Yes: false}, + &cmdutil.JSONOptions{}, svc, &testutil.ConfirmPrompter{}, + ) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputConfirmationRequired, typed.Code) + assert.Empty(t, svc.gotID, "must not call DeleteAgent without confirm") + assert.Equal(t, 10, cmdutil.ExitCode(err), "exit code 10 per destructive-write protocol") +} + +func TestDelete_NonTTY_WithYes_Direct(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeDeleteSvc{} + require.NoError(t, runDelete( + context.Background(), + &DeleteOptions{AgentID: "ag_abc", Yes: true}, + nil, svc, &testutil.ConfirmPrompter{}, + )) + assert.Equal(t, "ag_abc", svc.gotID) + assert.Contains(t, out.String(), "ag_abc") +} + +func TestDelete_404_PropagatesNotFound(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeDeleteSvc{err: errBadHTTP404} + err := runDelete( + context.Background(), + &DeleteOptions{AgentID: "ag_missing", Yes: true}, + nil, svc, &testutil.ConfirmPrompter{}, + ) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeResourceNotFound, typed.Code) +} + +func TestDelete_TTY_ConfirmYes(t *testing.T) { + _, _ = iostreams.SetForTestWithTTY(t) + svc := &fakeDeleteSvc{} + p := &testutil.ConfirmPrompter{Answer: true} + require.NoError(t, runDelete( + context.Background(), + &DeleteOptions{AgentID: "ag_abc"}, + nil, svc, p, + )) + assert.True(t, p.Asked) + assert.Equal(t, "ag_abc", svc.gotID) +} + +func TestDelete_TTY_ConfirmNo(t *testing.T) { + _, errBuf := iostreams.SetForTestWithTTY(t) + svc := &fakeDeleteSvc{} + p := &testutil.ConfirmPrompter{Answer: false} + err := runDelete( + context.Background(), + &DeleteOptions{AgentID: "ag_abc"}, + nil, svc, p, + ) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeUserAborted, typed.Code) + assert.Empty(t, svc.gotID, "answer=no must not call DeleteAgent") + assert.Contains(t, errBuf.String(), "Aborted") +} + +func TestDelete_JSON_BareObject(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeDeleteSvc{} + require.NoError(t, runDelete( + context.Background(), + &DeleteOptions{AgentID: "ag_abc", Yes: true}, + &cmdutil.JSONOptions{}, svc, &testutil.ConfirmPrompter{}, + )) + assert.Contains(t, out.String(), `"id":"ag_abc"`) + assert.Contains(t, out.String(), `"deleted":true`) +} diff --git a/cli/cmd/agent/edit.go b/cli/cmd/agent/edit.go new file mode 100644 index 000000000..bb7c4a663 --- /dev/null +++ b/cli/cmd/agent/edit.go @@ -0,0 +1,329 @@ +package agentcmd + +import ( + "context" + "fmt" + "io" + "sort" + "strings" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// EditService is the narrow SDK surface this command depends on. The fetch +// half (GetAgent) is mandatory because UpdateAgent is a full PUT — without +// the pre-fetch baseline, any field not passed as a flag would silently +// clobber to the zero value. +type EditService interface { + GetAgent(ctx context.Context, id string) (*sdk.Agent, error) + UpdateAgent(ctx context.Context, id string, req *sdk.UpdateAgentRequest) (*sdk.Agent, error) +} + +// EditOptions captures the surgical flag state. Both string fields and +// reader-based file inputs are tracked alongside per-flag *Set bits in +// editFlagSet so empty strings are distinguishable from "unset". +type EditOptions struct { + AgentID string + Name string + Description string + Model string + SystemPrompt string + SystemPromptReader io.Reader + AgentMode string + RerankModel string + Temperature float64 + AddKBs []string + RemoveKBs []string + KBSelectionMode string + ConfigFileBody io.Reader + ConfigFileKind string // "yaml" or "json" + + flags editFlagSet +} + +// editFlagSet tracks which surgical flags the user passed. Empty-string +// values are valid (clear semantics) so cmd.Flags().Changed() is the only +// reliable signal of "user supplied this flag." +type editFlagSet struct { + nameSet bool + descriptionSet bool + modelSet bool + systemPromptSet bool + agentModeSet bool + rerankModelSet bool + temperatureSet bool + addKBsSet bool + removeKBsSet bool + kbSelectionModeSet bool + configFileSet bool +} + +const agentEditLong = `Edit a custom agent's fields surgically. + +At least one update flag is required; flags you omit preserve the current +server-side value via fetch-then-update. Pass --description "" to clear +the description (empty string is a valid value, not "unset"). + +KB list operations are list-shaped: --add-kb and --remove-kb are +idempotent (re-adding an already-attached KB is silent success; removing +an unattached KB is silent success). Passing the same id to both flags +nets out to no-op and prints a stderr warning. + +--config-file fully replaces the AgentConfig baseline (the same shape +GenerateAgentSkeleton emits). Surgical flags then apply on top of that +replaced baseline. To partially update one or two fields without +touching the rest, use surgical flags alone — that path L-2 fetches +current state and only mutates what's set. Precedence within a single +invocation: + + surgical flag > config-file value > zero value + +AI agents: writes to a server resource. Failure surfaces as a typed +code on stderr: resource.not_found (agent id or KB id), auth.forbidden, +input.invalid_argument (no flags passed, bad file).` + +const agentEditExample = ` weknora agent edit ag_abc --name "Renamed" + weknora agent edit ag_abc --description "" # clear description + weknora agent edit ag_abc --add-kb kb_new --remove-kb kb_old + weknora agent edit ag_abc --system-prompt-file ./prompt.md + weknora agent edit ag_abc --config-file ./tuned.yaml --temperature 0.9` + +// NewCmdEdit builds `weknora agent edit `. +func NewCmdEdit(f *cmdutil.Factory) *cobra.Command { + opts := &EditOptions{} + var systemPromptFile, configFile string + + cmd := &cobra.Command{ + Use: "edit ", + Short: "Edit a custom agent's configuration", + Long: agentEditLong, + Example: agentEditExample, + Args: cobra.ExactArgs(1), + PreRunE: func(cmd *cobra.Command, args []string) error { + opts.AgentID = args[0] + opts.flags.nameSet = cmd.Flags().Changed("name") + opts.flags.descriptionSet = cmd.Flags().Changed("description") + opts.flags.modelSet = cmd.Flags().Changed("model") + opts.flags.systemPromptSet = cmd.Flags().Changed("system-prompt") || cmd.Flags().Changed("system-prompt-file") + opts.flags.agentModeSet = cmd.Flags().Changed("agent-mode") + opts.flags.rerankModelSet = cmd.Flags().Changed("rerank-model") + opts.flags.temperatureSet = cmd.Flags().Changed("temperature") + opts.flags.addKBsSet = cmd.Flags().Changed("add-kb") + opts.flags.removeKBsSet = cmd.Flags().Changed("remove-kb") + opts.flags.kbSelectionModeSet = cmd.Flags().Changed("kb-selection-mode") + opts.flags.configFileSet = cmd.Flags().Changed("config-file") + + // L-5 / §1.5.1: --temperature is bounded 0.0..2.0. Reject + // out-of-range early with a typed input.invalid_argument. + if opts.flags.temperatureSet && (opts.Temperature < 0.0 || opts.Temperature > 2.0) { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + fmt.Sprintf("--temperature must be in 0.0..2.0, got %g", opts.Temperature)) + } + + if systemPromptFile != "" { + r, err := openInput(systemPromptFile) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--system-prompt-file: %v", err)) + } + opts.SystemPromptReader = r + } + if configFile != "" { + r, kind, err := openConfigFile(configFile) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--config-file: %v", err)) + } + opts.ConfigFileBody = r + opts.ConfigFileKind = kind + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + jopts, err := cmdutil.CheckJSONFlags(cmd) + if err != nil { + return err + } + cli, err := f.Client() + if err != nil { + return err + } + return runEdit(cmd.Context(), opts, jopts, cli) + }, + } + + // Surgical flags + cmd.Flags().StringVar(&opts.Name, "name", "", "New agent name") + cmd.Flags().StringVar(&opts.Description, "description", "", `New description (use "" to clear)`) + cmd.Flags().StringVar(&opts.Model, "model", "", "LLM model id") + cmd.Flags().StringVar(&opts.SystemPrompt, "system-prompt", "", "System prompt text (mutex with --system-prompt-file)") + cmd.Flags().StringVar(&systemPromptFile, "system-prompt-file", "", "Read system prompt from FILE, or '-' for stdin") + cmd.MarkFlagsMutuallyExclusive("system-prompt", "system-prompt-file") + cmd.Flags().StringVar(&opts.AgentMode, "agent-mode", "", "Agent operating mode: quick-answer | smart-reasoning") + cmd.Flags().StringVar(&opts.RerankModel, "rerank-model", "", "Rerank model id") + cmd.Flags().Float64Var(&opts.Temperature, "temperature", 0.0, "Generation temperature (0.0..2.0)") + cmd.Flags().StringSliceVar(&opts.AddKBs, "add-kb", nil, "Attach knowledge base id (repeatable, idempotent)") + cmd.Flags().StringSliceVar(&opts.RemoveKBs, "remove-kb", nil, "Detach knowledge base id (repeatable, idempotent)") + cmd.Flags().StringVar(&opts.KBSelectionMode, "kb-selection-mode", "", "KB selection mode: all | selected | none") + + // Full-replace + cmd.Flags().StringVar(&configFile, "config-file", "", "Full AgentConfig YAML or JSON (REPLACES current config baseline; surgical flags then apply on top)") + + cmdutil.AddJSONFlags(cmd, agentViewFields) + return cmd +} + +// editHasAnyFlag reports whether opts carries at least one surgical update +// signal. Required-flag validation lives in runEdit (not PreRunE) so unit +// tests can invoke runEdit with a hand-built EditOptions directly. +func editHasAnyFlag(opts *EditOptions) bool { + fl := opts.flags + return fl.nameSet || fl.descriptionSet || fl.modelSet || fl.systemPromptSet || + fl.agentModeSet || fl.rerankModelSet || fl.temperatureSet || + fl.addKBsSet || fl.removeKBsSet || fl.kbSelectionModeSet || fl.configFileSet +} + +func runEdit(ctx context.Context, opts *EditOptions, jopts *cmdutil.JSONOptions, svc EditService) error { + if !editHasAnyFlag(opts) { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: "agent edit requires at least one flag", + Hint: "pass at least one update flag (e.g., --name, --add-kb, --description) or --config-file", + } + } + + // L-2 fetch-then-update so omitted fields round-trip unchanged through + // the full PUT body. + current, err := svc.GetAgent(ctx, opts.AgentID) + if err != nil { + return cmdutil.WrapHTTP(err, "fetch agent %s", opts.AgentID) + } + + // Build base config: server state, then overlay --config-file (if any). + base := sdk.AgentConfig{} + if current.Config != nil { + base = *current.Config + } + if opts.ConfigFileBody != nil { + parsed, err := cmdutil.LoadAgentConfig(opts.ConfigFileBody, opts.ConfigFileKind) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, err.Error()) + } + base = *parsed + } + + // Resolve --system-prompt-file before flag overlay. + if opts.SystemPromptReader != nil { + body, err := io.ReadAll(opts.SystemPromptReader) + if err != nil { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("--system-prompt-file read: %v", err)) + } + opts.SystemPrompt = strings.TrimSpace(string(body)) + } + + // Compute KB list from current + add/remove with a stderr warning when + // the same id appears in both flags (net no-op, still idempotent). + kbs := computeKBList(base.KnowledgeBases, opts.AddKBs, opts.RemoveKBs) + + overrides := cmdutil.AgentConfigFlags{ + AgentMode: opts.AgentMode, AgentModeSet: opts.flags.agentModeSet, + SystemPrompt: opts.SystemPrompt, SystemPromptSet: opts.flags.systemPromptSet, + ModelID: opts.Model, ModelIDSet: opts.flags.modelSet, + RerankModelID: opts.RerankModel, RerankModelIDSet: opts.flags.rerankModelSet, + Temperature: opts.Temperature, TemperatureSet: opts.flags.temperatureSet, + KBSelectionMode: opts.KBSelectionMode, KBSelectionModeSet: opts.flags.kbSelectionModeSet, + // KB list is always replaced (its add/remove was already merged + // into kbs); we only signal "set" when the user actually touched + // the list so a plain --name edit doesn't churn the field. + KnowledgeBases: kbs, + KnowledgeBasesSet: opts.flags.addKBsSet || opts.flags.removeKBsSet, + } + cfg := cmdutil.MergeAgentConfig(&base, overrides) + + // Build the full PUT body. Name/Description default to the current + // server values so the surgical-flag-only path preserves them. + req := &sdk.UpdateAgentRequest{ + Name: current.Name, + Description: current.Description, + Config: cfg, + } + if opts.flags.nameSet { + req.Name = opts.Name + } + if opts.flags.descriptionSet { + req.Description = opts.Description + } + + updated, err := svc.UpdateAgent(ctx, opts.AgentID, req) + if err != nil { + return cmdutil.WrapHTTP(err, "edit agent %s", opts.AgentID) + } + return emitAgent(jopts, updated) +} + +// computeKBList applies --add-kb / --remove-kb to current with idempotent +// semantics. Ids present in both add and remove cancel out and surface a +// stderr warning so users notice the conflict but don't see a hard error. +// Stderr is the right channel here (not stdout) because callers piping +// --json | jq would otherwise see corrupted JSON. +func computeKBList(current, add, remove []string) []string { + // Detect ids in both add and remove; they net out to no-op and are + // excluded from both operations. + canceledSet := map[string]struct{}{} + addSeen := map[string]struct{}{} + for _, id := range add { + addSeen[id] = struct{}{} + } + for _, id := range remove { + if _, both := addSeen[id]; both { + canceledSet[id] = struct{}{} + } + } + if len(canceledSet) > 0 { + canceled := make([]string, 0, len(canceledSet)) + for id := range canceledSet { + canceled = append(canceled, id) + } + // Sort for deterministic test output; map iteration is random. + sort.Strings(canceled) + fmt.Fprintf(iostreams.IO.Err, "warning: --add-kb and --remove-kb cancel out for: %s\n", strings.Join(canceled, ", ")) + } + + // Compute effective remove set (excluding canceled). + removeEff := map[string]struct{}{} + for _, id := range remove { + if _, c := canceledSet[id]; c { + continue + } + removeEff[id] = struct{}{} + } + + // Filter removals out of the current list (idempotent: unattached id + // simply isn't in current). + out := make([]string, 0, len(current)) + for _, id := range current { + if _, drop := removeEff[id]; drop { + continue + } + out = append(out, id) + } + // Append any add ids not already present (idempotent: already-attached + // id silently de-dupes). + present := map[string]struct{}{} + for _, id := range out { + present[id] = struct{}{} + } + for _, id := range add { + if _, c := canceledSet[id]; c { + continue + } + if _, dup := present[id]; dup { + continue + } + out = append(out, id) + present[id] = struct{}{} + } + return out +} diff --git a/cli/cmd/agent/edit_test.go b/cli/cmd/agent/edit_test.go new file mode 100644 index 000000000..f96a72741 --- /dev/null +++ b/cli/cmd/agent/edit_test.go @@ -0,0 +1,249 @@ +package agentcmd + +import ( + "bytes" + "context" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// fakeEditSvc scripts GetAgent (fetch baseline) + UpdateAgent (apply +// surgical overlays). updateCalls lets tests verify that no-flag invocations +// don't reach the wire. +type fakeEditSvc struct { + getResp *sdk.Agent + getErr error + updateReq *sdk.UpdateAgentRequest + updateID string + updateResp *sdk.Agent + updateErr error + updateCalls int +} + +func (f *fakeEditSvc) GetAgent(_ context.Context, _ string) (*sdk.Agent, error) { + return f.getResp, f.getErr +} + +func (f *fakeEditSvc) UpdateAgent(_ context.Context, id string, req *sdk.UpdateAgentRequest) (*sdk.Agent, error) { + f.updateReq = req + f.updateID = id + f.updateCalls++ + return f.updateResp, f.updateErr +} + +func TestEdit_FetchThenUpdate_PreservesUntouchedFields(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{ + ID: "ag_abc", Name: "Original", Description: "Keep me", + Config: &sdk.AgentConfig{ModelID: "gpt-4", Temperature: 0.7, KnowledgeBases: []string{"kb_a"}}, + }, + updateResp: &sdk.Agent{ID: "ag_abc"}, + } + // Only --description passed; everything else should round-trip. + opts := &EditOptions{ + AgentID: "ag_abc", + Description: "Updated", + flags: editFlagSet{descriptionSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + require.NotNil(t, svc.updateReq) + assert.Equal(t, "Original", svc.updateReq.Name, "Name must round-trip unchanged") + assert.Equal(t, "Updated", svc.updateReq.Description) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, "gpt-4", svc.updateReq.Config.ModelID, "ModelID must round-trip") + assert.Equal(t, []string{"kb_a"}, svc.updateReq.Config.KnowledgeBases, "KBs must round-trip") + assert.InDelta(t, 0.7, svc.updateReq.Config.Temperature, 0.001) +} + +func TestEdit_AddRemoveKB_SameID_NetNoOpWithWarning(t *testing.T) { + _, errBuf := iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{Config: &sdk.AgentConfig{KnowledgeBases: []string{"kb_a"}}}, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + AddKBs: []string{"kb_b"}, + RemoveKBs: []string{"kb_b"}, + flags: editFlagSet{addKBsSet: true, removeKBsSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, []string{"kb_a"}, svc.updateReq.Config.KnowledgeBases, "net no-op preserves original list") + assert.Contains(t, errBuf.String(), "cancel out", "warning emitted to stderr") +} + +func TestEdit_NoFlags_InvalidArgument(t *testing.T) { + svc := &fakeEditSvc{} + err := runEdit(context.Background(), &EditOptions{AgentID: "ag_abc"}, &cmdutil.JSONOptions{}, svc) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) + assert.Equal(t, 0, svc.updateCalls, "must not call UpdateAgent") +} + +func TestEdit_AddKB_AlreadyAttached_Silent(t *testing.T) { + _, errBuf := iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{Config: &sdk.AgentConfig{KnowledgeBases: []string{"kb_a", "kb_b"}}}, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + AddKBs: []string{"kb_a"}, // already attached + flags: editFlagSet{addKBsSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, []string{"kb_a", "kb_b"}, svc.updateReq.Config.KnowledgeBases, "no duplicate") + assert.NotContains(t, errBuf.String(), "warning", "already-attached is silent success") +} + +func TestEdit_RemoveKB_Unattached_Silent(t *testing.T) { + _, errBuf := iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{Config: &sdk.AgentConfig{KnowledgeBases: []string{"kb_a"}}}, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + RemoveKBs: []string{"kb_zzz"}, + flags: editFlagSet{removeKBsSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, []string{"kb_a"}, svc.updateReq.Config.KnowledgeBases) + assert.NotContains(t, errBuf.String(), "warning") +} + +func TestEdit_ClearDescription_EmptyString(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{Name: "X", Description: "old", Config: &sdk.AgentConfig{ModelID: "m"}}, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + Description: "", + flags: editFlagSet{descriptionSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, "", svc.updateReq.Description, "explicit empty must clear server-side") + assert.Equal(t, "X", svc.updateReq.Name, "Name round-trip unchanged") +} + +func TestEdit_ConfigFile_OverridesByFlag(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{ + Name: "X", + Config: &sdk.AgentConfig{ModelID: "old-model", Temperature: 0.1}, + }, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + Temperature: 0.9, + ConfigFileBody: bytes.NewBufferString(`{"temperature":0.5,"model_id":"file-model"}`), + ConfigFileKind: "json", + flags: editFlagSet{temperatureSet: true, configFileSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, "file-model", svc.updateReq.Config.ModelID, "file overrides current state") + assert.InDelta(t, 0.9, svc.updateReq.Config.Temperature, 0.001, "flag overrides file") +} + +// TestEdit_ConfigFile_FullReplacesBaseline pins the documented behavior: +// --config-file fully replaces the AgentConfig baseline; current-server +// fields not mentioned in the file are zeroed. The Long help directs +// users to surgical flags when they want a partial update. +func TestEdit_ConfigFile_FullReplacesBaseline(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{ + Name: "X", + Config: &sdk.AgentConfig{ + SystemPrompt: "Existing prompt", + ModelID: "old-model", + Temperature: 0.1, + AgentMode: "smart-reasoning", + KnowledgeBases: []string{"kb_existing"}, + }, + }, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + ConfigFileBody: bytes.NewBufferString(`{"model_id":"file-only"}`), + ConfigFileKind: "json", + flags: editFlagSet{configFileSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, "file-only", svc.updateReq.Config.ModelID, "file's model_id applied") + assert.Equal(t, "", svc.updateReq.Config.SystemPrompt, "file fully replaces baseline; unset fields are zeroed") + assert.InDelta(t, 0.0, svc.updateReq.Config.Temperature, 0.001, "unset fields zeroed") + assert.Equal(t, "", svc.updateReq.Config.AgentMode, "unset fields zeroed") + assert.Empty(t, svc.updateReq.Config.KnowledgeBases, "unset fields zeroed") +} + +func TestEdit_NotFound(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{getErr: errBadHTTP404} + opts := &EditOptions{AgentID: "ag_missing", Name: "x", flags: editFlagSet{nameSet: true}} + err := runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "resource.not_found") +} + +func TestEdit_AddKB_AppendsToExisting(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{Config: &sdk.AgentConfig{KnowledgeBases: []string{"kb_a"}}}, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + AddKBs: []string{"kb_b", "kb_c"}, + flags: editFlagSet{addKBsSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, []string{"kb_a", "kb_b", "kb_c"}, svc.updateReq.Config.KnowledgeBases) +} + +func TestEdit_Temperature_Bounds(t *testing.T) { + for _, badT := range []float64{-0.1, 2.1, 100.0} { + t.Run(fmt.Sprintf("t=%g", badT), func(t *testing.T) { + cmd := NewCmdEdit(nil) + cmd.SetArgs([]string{"ag_abc", "--temperature", fmt.Sprintf("%f", badT)}) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + require.Error(t, err, "expected error for --temperature %g", badT) + assert.Contains(t, err.Error(), "0.0..2.0") + }) + } +} + +func TestEdit_SystemPromptFile(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{Config: &sdk.AgentConfig{ModelID: "m"}}, + updateResp: &sdk.Agent{}, + } + opts := &EditOptions{ + AgentID: "ag_abc", + SystemPromptReader: strings.NewReader("new prompt\n"), + flags: editFlagSet{systemPromptSet: true}, + } + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.JSONOptions{}, svc)) + assert.Equal(t, "new prompt", svc.updateReq.Config.SystemPrompt) +} diff --git a/cli/cmd/agent/view.go b/cli/cmd/agent/view.go index 9d225d457..80f2344b4 100644 --- a/cli/cmd/agent/view.go +++ b/cli/cmd/agent/view.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "strings" "github.com/spf13/cobra" @@ -12,13 +13,16 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// agentViewFields enumerates fields surfaced for `--json` discovery on -// `agent view`. Filter applies to the bare Agent object. Config sub-fields -// are intentionally omitted - too granular for naked projection; use -// `--jq '.config'` to reach them. +// agentViewFields enumerates fields surfaced for `--json=` field discovery +// on `agent view`. Only top-level Agent keys are listed because the +// `--json=foo,bar` field-projection filter matches flat top-level keys +// (see internal/format/filter.go). Nested AgentConfig fields are reachable +// via `--jq '.config.system_prompt'` or by selecting `config` whole and +// post-processing — listing them here would misleadingly advertise a +// projection path that does not actually filter to them. var agentViewFields = []string{ "id", "name", "description", "avatar", - "is_builtin", "tenant_id", "created_by", + "is_builtin", "tenant_id", "created_by", "config", "created_at", "updated_at", } @@ -32,12 +36,17 @@ func NewCmdView(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "view ", Short: "Show a custom agent's configuration", - Long: `Renders the agent's metadata (id / name / description / created-by / -timestamps) plus a compact config summary (mode, model, allowed tools, KB -scope). Pass --json for the full Agent object including the nested config -struct - or --jq '.config' to extract just the config.`, + Long: `Renders the agent's metadata and full AgentConfig as grouped KV +sections (Identity / LLM / KB attachment / Retrieval / Query rewrite / +Tools / FAQ / Web search / Multi-turn / Fallback / Templates). Zero-value +fields are omitted; sections with no set fields are suppressed entirely. + +Pass --json for the bare SDK Agent object (config nested, not flattened). +Field projection (--json=id,name) works on top-level keys only; reach +nested config fields via --jq.`, Example: ` weknora agent view ag_abc - weknora agent view ag_abc --json | jq '.config.allowed_tools'`, + weknora agent view ag_abc --json=id,name,config # top-level projection + weknora agent view ag_abc --json --jq '.config.system_prompt'`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { jopts, err := cmdutil.CheckJSONFlags(c) @@ -67,47 +76,217 @@ func runView(ctx context.Context, jopts *cmdutil.JSONOptions, svc ViewService, a return nil } -// renderAgent prints a single agent in human-readable KV form. Empty -// fields are omitted (mirrors doc view / kb view); the Config block is -// compacted to its agent-mode-defining keys. +// renderAgent prints a single agent in human-readable form, grouped into +// 10 presentation sections (spec §2.9). Zero-value fields are omitted; +// a section header prints only when at least one of its fields is set. +// Group labels and order match the spec exactly so spec drift surfaces +// as test failure rather than silent divergence. func renderAgent(w io.Writer, a *sdk.Agent) { - fmt.Fprintf(w, "ID: %s\n", a.ID) - fmt.Fprintf(w, "Name: %s\n", a.Name) + // Identity is always rendered — id/name/created_at/updated_at are + // never meaningfully empty for a fetched Agent. + fmt.Fprintln(w, "Identity:") + fmt.Fprintf(w, " ID: %s\n", a.ID) + fmt.Fprintf(w, " Name: %s\n", a.Name) if a.Description != "" { - fmt.Fprintf(w, "Description: %s\n", a.Description) + fmt.Fprintf(w, " Description: %s\n", a.Description) } if a.IsBuiltin { - fmt.Fprintln(w, "Builtin: yes") + fmt.Fprintln(w, " Builtin: yes") } if a.CreatedBy != "" { - fmt.Fprintf(w, "Created by: %s\n", a.CreatedBy) + fmt.Fprintf(w, " Created by: %s\n", a.CreatedBy) } - fmt.Fprintf(w, "Created at: %s\n", a.CreatedAt.Format("2006-01-02 15:04:05")) - fmt.Fprintf(w, "Updated at: %s\n", a.UpdatedAt.Format("2006-01-02 15:04:05")) + if a.TenantID != 0 { + fmt.Fprintf(w, " Tenant ID: %d\n", a.TenantID) + } + fmt.Fprintf(w, " Created at: %s\n", a.CreatedAt.Format("2006-01-02 15:04:05")) + fmt.Fprintf(w, " Updated at: %s\n", a.UpdatedAt.Format("2006-01-02 15:04:05")) + if a.Config == nil { return } - fmt.Fprintln(w) - fmt.Fprintln(w, "Config:") - if a.Config.AgentMode != "" { - fmt.Fprintf(w, " Mode: %s\n", a.Config.AgentMode) + c := a.Config + + // Each group is rendered via a tiny helper: collect its set rows + // upfront, suppress the whole section if empty. Avoids the + // "header printed but no body" failure mode that plagues naive + // conditional rendering. + type row struct{ k, v string } + emit := func(label string, rows []row) { + if len(rows) == 0 { + return + } + fmt.Fprintln(w) + fmt.Fprintf(w, "%s:\n", label) + for _, r := range rows { + fmt.Fprintf(w, " %-32s %s\n", r.k+":", r.v) + } } - if a.Config.ModelID != "" { - fmt.Fprintf(w, " Model ID: %s\n", a.Config.ModelID) + + // LLM + llm := []row{} + if c.ModelID != "" { + llm = append(llm, row{"Model ID", c.ModelID}) } - if a.Config.RerankModelID != "" { - fmt.Fprintf(w, " Rerank model ID: %s\n", a.Config.RerankModelID) + if c.RerankModelID != "" { + llm = append(llm, row{"Rerank model ID", c.RerankModelID}) } - if a.Config.KBSelectionMode != "" { - fmt.Fprintf(w, " KB selection mode: %s\n", a.Config.KBSelectionMode) + if c.Temperature != 0 { + llm = append(llm, row{"Temperature", fmt.Sprintf("%g", c.Temperature)}) } - if len(a.Config.KnowledgeBases) > 0 { - fmt.Fprintf(w, " Knowledge bases: %v\n", a.Config.KnowledgeBases) + if c.MaxCompletionTokens != 0 { + llm = append(llm, row{"Max completion tokens", fmt.Sprintf("%d", c.MaxCompletionTokens)}) } - if len(a.Config.AllowedTools) > 0 { - fmt.Fprintf(w, " Allowed tools: %v\n", a.Config.AllowedTools) + if c.MaxIterations != 0 { + llm = append(llm, row{"Max iterations", fmt.Sprintf("%d", c.MaxIterations)}) } - if a.Config.WebSearchEnabled { - fmt.Fprintln(w, " Web search: enabled") + if c.AgentMode != "" { + llm = append(llm, row{"Mode", c.AgentMode}) + } + emit("LLM", llm) + + // KB attachment + kb := []row{} + if c.KBSelectionMode != "" { + kb = append(kb, row{"KB selection mode", c.KBSelectionMode}) + } + if len(c.KnowledgeBases) > 0 { + kb = append(kb, row{"Knowledge bases", strings.Join(c.KnowledgeBases, ", ")}) + } + emit("KB attachment", kb) + + // Retrieval + retr := []row{} + if c.EmbeddingTopK != 0 { + retr = append(retr, row{"Embedding top K", fmt.Sprintf("%d", c.EmbeddingTopK)}) + } + if c.KeywordThreshold != 0 { + retr = append(retr, row{"Keyword threshold", fmt.Sprintf("%g", c.KeywordThreshold)}) + } + if c.VectorThreshold != 0 { + retr = append(retr, row{"Vector threshold", fmt.Sprintf("%g", c.VectorThreshold)}) + } + if c.RerankTopK != 0 { + retr = append(retr, row{"Rerank top K", fmt.Sprintf("%d", c.RerankTopK)}) + } + if c.RerankThreshold != 0 { + retr = append(retr, row{"Rerank threshold", fmt.Sprintf("%g", c.RerankThreshold)}) + } + emit("Retrieval", retr) + + // Query rewrite + qr := []row{} + if c.EnableQueryExpansion { + qr = append(qr, row{"Query expansion", "enabled"}) + } + if c.EnableRewrite { + qr = append(qr, row{"Rewrite", "enabled"}) + } + if c.QueryUnderstandModelID != "" { + qr = append(qr, row{"Query understand model ID", c.QueryUnderstandModelID}) + } + if c.RewritePromptSystem != "" { + qr = append(qr, row{"Rewrite prompt (system)", truncate1Line(c.RewritePromptSystem)}) + } + if c.RewritePromptUser != "" { + qr = append(qr, row{"Rewrite prompt (user)", truncate1Line(c.RewritePromptUser)}) + } + emit("Query rewrite", qr) + + // Tools + tools := []row{} + if len(c.AllowedTools) > 0 { + tools = append(tools, row{"Allowed tools", strings.Join(c.AllowedTools, ", ")}) + } + if c.MCPSelectionMode != "" { + tools = append(tools, row{"MCP selection mode", c.MCPSelectionMode}) + } + if len(c.MCPServices) > 0 { + tools = append(tools, row{"MCP services", strings.Join(c.MCPServices, ", ")}) + } + if len(c.SupportedFileTypes) > 0 { + tools = append(tools, row{"Supported file types", strings.Join(c.SupportedFileTypes, ", ")}) + } + emit("Tools", tools) + + // FAQ + faq := []row{} + if c.FAQPriorityEnabled { + faq = append(faq, row{"FAQ priority", "enabled"}) + } + if c.FAQDirectAnswerThreshold != 0 { + faq = append(faq, row{"FAQ direct-answer threshold", fmt.Sprintf("%g", c.FAQDirectAnswerThreshold)}) + } + if c.FAQScoreBoost != 0 { + faq = append(faq, row{"FAQ score boost", fmt.Sprintf("%g", c.FAQScoreBoost)}) + } + emit("FAQ", faq) + + // Web search + web := []row{} + if c.WebSearchEnabled { + web = append(web, row{"Web search", "enabled"}) + } + if c.WebSearchMaxResults != 0 { + web = append(web, row{"Web search max results", fmt.Sprintf("%d", c.WebSearchMaxResults)}) + } + emit("Web search", web) + + // Multi-turn + mt := []row{} + if c.MultiTurnEnabled { + mt = append(mt, row{"Multi-turn", "enabled"}) + } + if c.HistoryTurns != 0 { + mt = append(mt, row{"History turns", fmt.Sprintf("%d", c.HistoryTurns)}) + } + emit("Multi-turn", mt) + + // Fallback + fb := []row{} + if c.FallbackStrategy != "" { + fb = append(fb, row{"Strategy", c.FallbackStrategy}) + } + if c.FallbackResponse != "" { + fb = append(fb, row{"Response", c.FallbackResponse}) + } + if c.FallbackPrompt != "" { + fb = append(fb, row{"Prompt", truncate1Line(c.FallbackPrompt)}) + } + emit("Fallback", fb) + + // Templates — system_prompt and context_template can be multi-line; + // render headed blocks rather than KV rows for readability. + if c.SystemPrompt != "" || c.ContextTemplate != "" { + fmt.Fprintln(w) + fmt.Fprintln(w, "Templates:") + if c.SystemPrompt != "" { + fmt.Fprintln(w, " System prompt:") + writeIndented(w, c.SystemPrompt, " ") + } + if c.ContextTemplate != "" { + fmt.Fprintln(w, " Context template:") + writeIndented(w, c.ContextTemplate, " ") + } + } +} + +// truncate1Line collapses newlines and clips long values for inline KV +// rows. Templates section gets full multi-line treatment instead. +func truncate1Line(s string) string { + s = strings.ReplaceAll(s, "\n", " ") + s = strings.ReplaceAll(s, "\r", " ") + const max = 80 + if len(s) > max { + return s[:max-3] + "..." + } + return s +} + +// writeIndented prints s with the given prefix on every line. Trailing +// newline always added so the next section starts on its own line. +func writeIndented(w io.Writer, s, prefix string) { + for _, line := range strings.Split(strings.TrimRight(s, "\n"), "\n") { + fmt.Fprintf(w, "%s%s\n", prefix, line) } } diff --git a/cli/cmd/agent/view_test.go b/cli/cmd/agent/view_test.go index 80139e3da..4266f165b 100644 --- a/cli/cmd/agent/view_test.go +++ b/cli/cmd/agent/view_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "slices" "strings" "testing" "time" @@ -44,13 +45,82 @@ func TestView_Human_RendersMetadataAndConfig(t *testing.T) { t.Fatalf("runView: %v", err) } got := out.String() - for _, want := range []string{"ag_abc", "Research", "deep-dive helper", "Builtin:", "Config:", "smart-reasoning", "model_42", "selected", "kb_x", "knowledge_search", "Web search:"} { + for _, want := range []string{"ag_abc", "Research", "deep-dive helper", "Builtin:", "Identity", "LLM", "KB attachment", "Tools", "smart-reasoning", "model_42", "selected", "kb_x", "knowledge_search", "Web search"} { if !strings.Contains(got, want) { t.Errorf("missing %q in:\n%s", want, got) } } } +// TestAgentViewFields_TopLevelOnly pins the contract that `--json=` field +// discovery on `agent view` only lists top-level Agent keys (including +// `config` as a whole). Nested AgentConfig fields are NOT in the list +// because the filter at internal/format/filter.go matches flat top-level +// keys only — listing `config.system_prompt` etc. would advertise a +// projection path that silently returns nothing. +func TestAgentViewFields_TopLevelOnly(t *testing.T) { + for _, want := range []string{"id", "name", "config", "created_at"} { + if !slices.Contains(agentViewFields, want) { + t.Errorf("agentViewFields missing top-level key %q", want) + } + } + for _, dotted := range []string{"config.system_prompt", "config.model_id", "config.fallback_strategy"} { + if slices.Contains(agentViewFields, dotted) { + t.Errorf("agentViewFields must not list dotted nested key %q (filter does not support nested projection)", dotted) + } + } +} + +// TestRenderAgent_RendersAllGroupsWithOmitEmpty validates the grouped +// human rendering: present groups print, zero-value fields omit, and an +// entire section is suppressed when all of its fields are zero. +func TestRenderAgent_RendersAllGroupsWithOmitEmpty(t *testing.T) { + out, _ := iostreams.SetForTest(t) + ag := &sdk.Agent{ + ID: "ag_abc", + Name: "Test", + Config: &sdk.AgentConfig{ + AgentMode: "smart-reasoning", + SystemPrompt: "You help users.", + ModelID: "gpt-4", + Temperature: 0.7, + KBSelectionMode: "selected", + KnowledgeBases: []string{"kb_a"}, + FAQPriorityEnabled: true, + WebSearchEnabled: false, // zero — omitted in human + FallbackStrategy: "fixed", + FallbackResponse: "I don't know.", + }, + } + renderAgent(iostreams.IO.Out, ag) + body := out.String() + // Group labels appear: + for _, label := range []string{"Identity", "LLM", "KB attachment", "FAQ", "Fallback", "Templates"} { + if !strings.Contains(body, label) { + t.Errorf("missing group label %q in:\n%s", label, body) + } + } + // Set fields rendered: + for _, want := range []string{"smart-reasoning", "gpt-4", "You help users."} { + if !strings.Contains(body, want) { + t.Errorf("missing value %q in:\n%s", want, body) + } + } + // Zero-value fields omitted (web search disabled → max_results not shown): + if strings.Contains(body, "web_search_max_results") { + t.Errorf("zero-valued web_search_max_results leaked into:\n%s", body) + } + // Section with all zero values must be suppressed entirely (no Retrieval + // fields were set in this fixture). + if strings.Contains(body, "Retrieval") { + t.Errorf("Retrieval section rendered with all-zero fields in:\n%s", body) + } + // Multi-turn section is all-zero too — must be suppressed. + if strings.Contains(body, "Multi-turn") { + t.Errorf("Multi-turn section rendered with all-zero fields in:\n%s", body) + } +} + func TestView_Human_OmitsEmptyFields(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeViewSvc{resp: &sdk.Agent{ @@ -69,8 +139,11 @@ func TestView_Human_OmitsEmptyFields(t *testing.T) { if strings.Contains(got, "Builtin:") { t.Errorf("non-builtin should not render Builtin: line, got:\n%s", got) } - if strings.Contains(got, "Config:") { - t.Errorf("nil Config should not render Config: section, got:\n%s", got) + // With Config==nil none of the grouped config sections should appear. + for _, label := range []string{"LLM:", "KB attachment:", "Retrieval:", "Query rewrite:", "Tools:", "FAQ:", "Web search:", "Multi-turn:", "Fallback:", "Templates:"} { + if strings.Contains(got, label) { + t.Errorf("nil Config should not render %q, got:\n%s", label, got) + } } } diff --git a/cli/internal/cmdutil/agentconfig.go b/cli/internal/cmdutil/agentconfig.go new file mode 100644 index 000000000..8f4cdd302 --- /dev/null +++ b/cli/internal/cmdutil/agentconfig.go @@ -0,0 +1,183 @@ +package cmdutil + +import ( + "encoding/json" + "fmt" + "io" + "strings" + + "gopkg.in/yaml.v3" + + sdk "github.com/Tencent/WeKnora/client" +) + +// AgentConfigFlags carries hot-path flag values plus per-flag "was set" +// bits so the merge step can distinguish "user did not pass --foo" from +// "user passed --foo zero-value". Mirrors the canonical CLI pattern of +// keeping presence-tracking out-of-band from the value itself, which +// pflag's Changed() facility supplies at the cobra layer. +type AgentConfigFlags struct { + AgentMode string + AgentModeSet bool + SystemPrompt string + SystemPromptSet bool + ModelID string + ModelIDSet bool + RerankModelID string + RerankModelIDSet bool + Temperature float64 + TemperatureSet bool + KBSelectionMode string + KBSelectionModeSet bool + KnowledgeBases []string + KnowledgeBasesSet bool +} + +// LoadAgentConfig parses YAML or JSON from r into an AgentConfig. kind is +// "yaml" or "json" (typically inferred from file extension by the caller). +// Returned errors carry no typed code — callers should wrap with +// CodeInputInvalidArgument as the user-facing context. +// +// YAML route round-trips through JSON so the SDK's existing `json:` field +// tags (snake_case) are the single source of truth for key names. yaml.v3 +// alone would lowercase field names (KnowledgeBases → knowledgebases), +// which doesn't match the JSON schema users see elsewhere. +func LoadAgentConfig(r io.Reader, kind string) (*sdk.AgentConfig, error) { + body, err := io.ReadAll(r) + if err != nil { + return nil, fmt.Errorf("read config: %w", err) + } + var cfg sdk.AgentConfig + switch strings.ToLower(kind) { + case "yaml", "yml": + var raw map[string]any + if err := yaml.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("parse YAML config: %w", err) + } + jsBody, err := json.Marshal(raw) + if err != nil { + return nil, fmt.Errorf("re-encode YAML as JSON: %w", err) + } + if err := json.Unmarshal(jsBody, &cfg); err != nil { + return nil, fmt.Errorf("parse YAML config (via JSON): %w", err) + } + case "json": + if err := json.Unmarshal(body, &cfg); err != nil { + return nil, fmt.Errorf("parse JSON config: %w", err) + } + default: + return nil, fmt.Errorf("unknown config format %q (want yaml or json)", kind) + } + return &cfg, nil +} + +// MergeAgentConfig returns base with hot-path flag overrides applied. Only +// fields whose corresponding *Set bit is true are overridden; the rest of +// base passes through unchanged. The returned pointer is a fresh copy so +// callers can mutate it without aliasing base. +func MergeAgentConfig(base *sdk.AgentConfig, ov AgentConfigFlags) *sdk.AgentConfig { + out := *base // shallow copy + if ov.AgentModeSet { + out.AgentMode = ov.AgentMode + } + if ov.SystemPromptSet { + out.SystemPrompt = ov.SystemPrompt + } + if ov.ModelIDSet { + out.ModelID = ov.ModelID + } + if ov.RerankModelIDSet { + out.RerankModelID = ov.RerankModelID + } + if ov.TemperatureSet { + out.Temperature = ov.Temperature + } + if ov.KBSelectionModeSet { + out.KBSelectionMode = ov.KBSelectionMode + } + if ov.KnowledgeBasesSet { + out.KnowledgeBases = ov.KnowledgeBases + } + return &out +} + +// skeletonCommentVersion is embedded in the skeleton header so users +// regenerating an outdated template after a schema bump can spot the +// version mismatch. +const skeletonCommentVersion = "v0.5 / 34 fields" + +// GenerateAgentSkeleton writes a commented YAML template with every +// AgentConfig field at its zero value to w. Used by +// `agent create --generate-skeleton` so users get a ready-to-edit +// starting point without authoring the 34-field schema from memory. +func GenerateAgentSkeleton(w io.Writer) error { + const skeleton = `# WeKnora AgentConfig skeleton (` + skeletonCommentVersion + `) +# Edit this file and pass it to: +# weknora agent create "My Agent" --model --config-file +# Hot-path flags on the create command override values set here. + +# Operating mode: "quick-answer" or "smart-reasoning" +agent_mode: "" + +# System prompt for the agent (also settable via --system-prompt[-file]) +system_prompt: "" + +# Optional template applied to retrieved context before model input +context_template: "" + +# REQUIRED: LLM model id (server-side managed); also settable via --model +model_id: "" + +# Optional rerank model id (server-side managed); also settable via --rerank-model +rerank_model_id: "" + +# Generation tuning +temperature: 0.0 +max_completion_tokens: 0 +max_iterations: 0 + +# Tools / MCP integration +allowed_tools: [] +mcp_selection_mode: "" # "all" / "selected" / "none" +mcp_services: [] + +# Knowledge base attachment +kb_selection_mode: "" # "all" / "selected" / "none"; also settable via --kb-selection-mode +knowledge_bases: [] # KB ids; also settable via repeated --kb +supported_file_types: [] + +# FAQ +faq_priority_enabled: false +faq_direct_answer_threshold: 0.0 +faq_score_boost: 0.0 + +# Web search +web_search_enabled: false +web_search_max_results: 0 + +# Multi-turn +multi_turn_enabled: false +history_turns: 0 + +# Retrieval thresholds +embedding_top_k: 0 +keyword_threshold: 0.0 +vector_threshold: 0.0 +rerank_top_k: 0 +rerank_threshold: 0.0 + +# Query understanding / rewrite +enable_query_expansion: false +enable_rewrite: false +rewrite_prompt_system: "" +rewrite_prompt_user: "" +query_understand_model_id: "" + +# Fallback when retrieval / generation fails +fallback_strategy: "" # "fixed" or "model" +fallback_response: "" +fallback_prompt: "" +` + _, err := io.WriteString(w, skeleton) + return err +} diff --git a/cli/internal/cmdutil/agentconfig_test.go b/cli/internal/cmdutil/agentconfig_test.go new file mode 100644 index 000000000..1d167e170 --- /dev/null +++ b/cli/internal/cmdutil/agentconfig_test.go @@ -0,0 +1,114 @@ +package cmdutil + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + sdk "github.com/Tencent/WeKnora/client" +) + +func TestLoadConfigFile_YAML(t *testing.T) { + yaml := strings.NewReader(` +agent_mode: smart-reasoning +model_id: gpt-4 +temperature: 0.7 +knowledge_bases: + - kb_abc + - kb_def +`) + cfg, err := LoadAgentConfig(yaml, "yaml") + require.NoError(t, err) + assert.Equal(t, "smart-reasoning", cfg.AgentMode) + assert.Equal(t, "gpt-4", cfg.ModelID) + assert.InDelta(t, 0.7, cfg.Temperature, 0.001) + assert.Equal(t, []string{"kb_abc", "kb_def"}, cfg.KnowledgeBases) +} + +func TestLoadConfigFile_JSON(t *testing.T) { + js := strings.NewReader(`{"agent_mode":"quick-answer","model_id":"gpt-3.5"}`) + cfg, err := LoadAgentConfig(js, "json") + require.NoError(t, err) + assert.Equal(t, "quick-answer", cfg.AgentMode) + assert.Equal(t, "gpt-3.5", cfg.ModelID) +} + +func TestLoadConfigFile_UnknownKind(t *testing.T) { + _, err := LoadAgentConfig(strings.NewReader(""), "xml") + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown config format") +} + +func TestLoadConfigFile_BadYAML(t *testing.T) { + _, err := LoadAgentConfig(strings.NewReader("not: : yaml"), "yaml") + require.Error(t, err) +} + +func TestLoadConfigFile_BadJSON(t *testing.T) { + _, err := LoadAgentConfig(strings.NewReader("{not json"), "json") + require.Error(t, err) +} + +func TestMergeAgentConfig_FlagsOverrideFile(t *testing.T) { + base := &sdk.AgentConfig{ModelID: "gpt-3.5"} + overrides := AgentConfigFlags{ModelIDSet: true, ModelID: "gpt-4"} + merged := MergeAgentConfig(base, overrides) + assert.Equal(t, "gpt-4", merged.ModelID) +} + +func TestMergeAgentConfig_UnsetFlagsPreserveBase(t *testing.T) { + base := &sdk.AgentConfig{ModelID: "gpt-3.5", Temperature: 0.5} + overrides := AgentConfigFlags{} // nothing set + merged := MergeAgentConfig(base, overrides) + assert.Equal(t, "gpt-3.5", merged.ModelID) + assert.InDelta(t, 0.5, merged.Temperature, 0.001) +} + +func TestMergeAgentConfig_EveryFieldOverlay(t *testing.T) { + base := &sdk.AgentConfig{ + AgentMode: "quick-answer", + SystemPrompt: "old", + ModelID: "gpt-3.5", + RerankModelID: "rerank-old", + Temperature: 0.1, + KBSelectionMode: "all", + KnowledgeBases: []string{"kb_old"}, + } + overrides := AgentConfigFlags{ + AgentMode: "smart-reasoning", AgentModeSet: true, + SystemPrompt: "new", SystemPromptSet: true, + ModelID: "gpt-4", ModelIDSet: true, + RerankModelID: "rerank-new", RerankModelIDSet: true, + Temperature: 0.9, TemperatureSet: true, + KBSelectionMode: "selected", KBSelectionModeSet: true, + KnowledgeBases: []string{"kb_new"}, KnowledgeBasesSet: true, + } + merged := MergeAgentConfig(base, overrides) + assert.Equal(t, "smart-reasoning", merged.AgentMode) + assert.Equal(t, "new", merged.SystemPrompt) + assert.Equal(t, "gpt-4", merged.ModelID) + assert.Equal(t, "rerank-new", merged.RerankModelID) + assert.InDelta(t, 0.9, merged.Temperature, 0.001) + assert.Equal(t, "selected", merged.KBSelectionMode) + assert.Equal(t, []string{"kb_new"}, merged.KnowledgeBases) +} + +func TestGenerateSkeleton_EmitsAllFields(t *testing.T) { + var out strings.Builder + require.NoError(t, GenerateAgentSkeleton(&out)) + body := out.String() + for _, field := range []string{ + "agent_mode:", + "model_id:", + "system_prompt:", + "knowledge_bases:", + "fallback_strategy:", + "context_template:", + "max_completion_tokens:", + "embedding_top_k:", + } { + assert.Contains(t, body, field, "skeleton missing field %s", field) + } +}