d3c03b2f34
A hardening + finalization pass over the agent-first CLI: correctness fixes,
richer machine-readable signals, flag/naming consistency, and a symmetric
config surface. Pre-1.0, so it includes breaking renames.
Correctness:
- agent update: resolve/validate --model/--rerank-model (was storing a bogus
name verbatim, corrupting config.model_id).
- doctor: honor WEKNORA_HOST / WEKNORA_API_KEY (headless path no longer reports
"no host configured").
- session ask / MCP session_ask: text answer was empty on non-TTY — the agent
stream sets Done=true on an intermediate agent_query frame before the answer,
and AgentAccumulator treated the first Done as terminal. Terminate on the
`complete` event (new sdk.AgentResponseTypeComplete), not a per-frame Done.
- batch exit codes: any per-item failure collapses to operation.failed (exit 1),
including `doc upload --recursive` partial failures — a permanent per-file
failure (e.g. a duplicate) no longer surfaces as a retryable exit 7 an agent
would loop on; per-item typed errors stay in the envelope.
Agent-first signals & discovery:
- error.exit_code in the envelope (type + exit_code disambiguate the
input.invalid_argument exit-2-vs-5 split in one JSON read).
- meta.hint on empty content search and on draft doc create; doc wait fails fast
on a never-parsing draft instead of hanging to --timeout.
- retrieval-readiness is visible in the natural flow: kb status / kb check emit
retrieval_ready, and kb create hints the fix when no embedding model is bound
— an unconfigured KB no longer looks silently healthy.
- schema contract completeness: every leaf declares output + >=1 example
(drift-guarded); output strings match the meta actually emitted; chunk list
and search docs now emit meta.total_count (both previously dropped it).
- schema tolerates a quoted multi-word command label; zero-state auth — and
`link` with no profile — point at profile setup / the headless WEKNORA_KB_ID
path instead of looping on `auth login`.
- id-addressed reads tolerate a redundant --kb (doc view/wait, chunk list/view
accept and ignore it, declared in schema) so a carried-over --kb doesn't
exit 2; streaming commands warn that --jq does not apply to an NDJSON stream.
- keep JSON-always as the default; --jq hints spell out the .data path.
Consistency & gating:
- doc create: drop the deprecated --name alias (--title only; pre-1.0 break).
- chunk list --limit aligned to 1..10000; model list --limit/-L with
has_more/total_count; api write-gates -X PUT/PATCH (exit 10); skills install
expands a leading ~.
- docs corrected: search docs / doc list --keyword help is case-insensitive
(server does LOWER LIKE); AGENTS.md risk-action list (no phantom kb.init; add
model.update / kb.config.set) and batch example (failed item carries `error`);
session resume --message id comes from `message list`, not the stream.
- auth/profile ergonomics: env credentials are now first-class — `auth token`
prints the active WEKNORA_API_KEY / WEKNORA_TOKEN, and auth login/logout/refresh
give an env-aware message instead of looping on "run auth login". `auth logout`
clears credentials but keeps the profile registered (host preserved for
re-login); deleting a profile is `profile remove`'s job (clean logout/remove
separation, matching gh / lark).
Config surface (symmetric read/write, in-place model edits):
- kb config now returns a secret-free KBModelConfigView (was {}); `kb config`
reads, new `kb config set` writes; `kb init` removed (misnomer).
- kb create --chat-model: retrieval-ready in one step.
- model update: edit a model in place (id preserved, references survive) —
rotate --api-key-stdin, change base-url / display-name / etc.
- session continue-stream renamed to session resume.
Docs: AGENTS.md is the single wire-contract source; CHANGELOG slimmed; stale
kb-init / continue-stream references removed; skill wire-vocab guard extended.
AGENTS.md / weknora-shared SKILL document retrieval_ready (a KB needs an
embedding model to be searchable), that --jq does not apply to NDJSON
streams, and the env-credential-first auth path; the KB quickstart example
now creates a retrieval-ready KB.
327 lines
12 KiB
Go
327 lines
12 KiB
Go
package agentcmd
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"fmt"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/spf13/cobra"
|
|
"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/prompt"
|
|
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
|
|
models []sdk.Model
|
|
modelsErr error
|
|
}
|
|
|
|
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 (f *fakeEditSvc) ListModels(_ context.Context) ([]sdk.Model, error) {
|
|
return f.models, f.modelsErr
|
|
}
|
|
|
|
func TestEdit_ModelName_ResolvedToID(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
svc := &fakeEditSvc{
|
|
getResp: &sdk.Agent{ID: "ag_abc", Name: "A", Config: &sdk.AgentConfig{ModelID: "old-id"}},
|
|
updateResp: &sdk.Agent{ID: "ag_abc"},
|
|
models: []sdk.Model{{ID: "m-real", Name: "good-llm", Type: "KnowledgeQA"}},
|
|
}
|
|
opts := &EditOptions{AgentID: "ag_abc", Model: "good-llm", flags: editFlagSet{modelSet: true}}
|
|
require.NoError(t, runEdit(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc))
|
|
require.NotNil(t, svc.updateReq)
|
|
require.NotNil(t, svc.updateReq.Config)
|
|
assert.Equal(t, "m-real", svc.updateReq.Config.ModelID, "--model name must resolve to the model id")
|
|
}
|
|
|
|
func TestEdit_BogusModelName_RejectedNoWrite(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
svc := &fakeEditSvc{
|
|
getResp: &sdk.Agent{ID: "ag_abc", Name: "A", Config: &sdk.AgentConfig{ModelID: "old-id"}},
|
|
updateResp: &sdk.Agent{ID: "ag_abc"},
|
|
models: []sdk.Model{{ID: "m-real", Name: "good-llm", Type: "KnowledgeQA"}},
|
|
}
|
|
opts := &EditOptions{AgentID: "ag_abc", Model: "totally-bogus-model-xyz", flags: editFlagSet{modelSet: true}}
|
|
err := runEdit(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc)
|
|
require.Error(t, err, "a --model name matching no model must fail")
|
|
var e *cmdutil.Error
|
|
require.ErrorAs(t, err, &e)
|
|
assert.Equal(t, cmdutil.CodeResourceNotFound, e.Code)
|
|
assert.Equal(t, 0, svc.updateCalls, "must not write an agent with an unresolvable model")
|
|
}
|
|
|
|
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: "model-x", 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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, "model-x", 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, 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.FormatOptions{Mode: cmdutil.FormatJSON}, svc))
|
|
assert.Equal(t, "new prompt", svc.updateReq.Config.SystemPrompt)
|
|
}
|
|
|
|
// withRootHarnessAgent wraps `weknora agent update ...` under a synthetic root
|
|
// cmd that registers the global persistent flags (mirrors addGlobalFlags in
|
|
// cmd/root.go).
|
|
func withRootHarnessAgent(edit *cobra.Command, args ...string) *cobra.Command {
|
|
root := &cobra.Command{Use: "weknora"}
|
|
pf := root.PersistentFlags()
|
|
pf.BoolP("yes", "y", false, "")
|
|
pf.String("format", "", "Output format: text | json | ndjson")
|
|
pf.StringP("jq", "q", "", "")
|
|
ag := &cobra.Command{Use: "agent"}
|
|
ag.AddCommand(edit)
|
|
root.AddCommand(ag)
|
|
root.SetArgs(append([]string{"agent", "update"}, args...))
|
|
root.SetContext(context.Background())
|
|
root.SilenceErrors = true
|
|
root.SilenceUsage = true
|
|
return root
|
|
}
|
|
|
|
// TestAgentEdit_RequiresConfirmation asserts that without -y (non-TTY / JSON
|
|
// mode), agent update returns input.confirmation_required (exit 10).
|
|
func TestAgentEdit_RequiresConfirmation(t *testing.T) {
|
|
iostreams.SetForTest(t) // non-TTY
|
|
f := &cmdutil.Factory{
|
|
Client: func() (*sdk.Client, error) { return nil, nil },
|
|
Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} },
|
|
}
|
|
root := withRootHarnessAgent(NewCmdEdit(f), "ag_abc", "--name", "Renamed", "--format", "json")
|
|
err := root.Execute()
|
|
require.Error(t, err)
|
|
var ce *cmdutil.Error
|
|
require.ErrorAs(t, err, &ce)
|
|
assert.Equal(t, cmdutil.CodeInputConfirmationRequired, ce.Code)
|
|
assert.Equal(t, 10, cmdutil.ExitCode(err), "exit code 10 per destructive-write protocol")
|
|
// retry argv must include -y and the agent id
|
|
assert.Contains(t, ce.RetryArgv, "-y")
|
|
assert.Contains(t, ce.RetryArgv, "ag_abc")
|
|
}
|