From a2e368b1e8da2f4e366432d127cf8306ecec35d0 Mon Sep 17 00:00:00 2001 From: nullkey Date: Thu, 21 May 2026 18:42:27 +0800 Subject: [PATCH] =?UTF-8?q?refactor(cli):=20command-surface=20rename=20?= =?UTF-8?q?=E2=80=94=20session=20ask=20/=20doc=20fetch=20/=20doc=20create?= =?UTF-8?q?=20/=20doc=20delete=20--all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three command renames consolidate the v0.7 verb table: - `weknora agent invoke` → `weknora session ask --agent `. Server route POST /sessions/{session_id}/agent-qa is session- anchored, so the verb moves with it. `weknora agent` subtree keeps CRUD only (list / view / create / edit / delete / status / check). SDK call (AgentQAStreamWithRequest) preserved verbatim; only the command surface + flag layout move. - `weknora doc upload` split into three commands: - `weknora doc upload ` — local file (only). - `weknora doc fetch ` — server-side remote fetch (was `upload --from-url`). URL-only flags (--title / --file-type / --tag-id) move with the verb. - `weknora doc create --text` — direct text knowledge entry via server CreateManualKnowledge. - `weknora kb empty ` → `weknora doc delete --all --kb=`. Atomic server ClearKnowledgeBaseContents (no list-then-delete race). Same exit-10 -y/--yes guard as other destructive verbs; unified through the extended ConfirmDestructive helper. Parent commands (agent, kb, doc, chunk, session, auth, profile, search) lose their explicit Args:NoArgs + Run:cmd.Help so the unknown-subcommand guard fires correctly — `weknora agent invoke ag_x q` now emits the typed input.unknown_subcommand envelope with detail.available[] instead of cobra's free-form exit-2 prose. Spec: docs/superpowers/specs/2026-05-20-weknora-cli-v0.7-design.md §3.4 / §10.7 --- cli/cmd/agent/agent.go | 20 +- cli/cmd/agent/invoke.go | 269 ---------------- cli/cmd/agent/invoke_test.go | 344 -------------------- cli/cmd/doc/create.go | 120 +++++++ cli/cmd/doc/create_test.go | 140 ++++++++ cli/cmd/doc/delete.go | 156 +++++---- cli/cmd/doc/delete_test.go | 371 ++++++++++++++++----- cli/cmd/doc/doc.go | 17 +- cli/cmd/doc/fetch.go | 140 ++++++++ cli/cmd/doc/fetch_test.go | 161 ++++++++++ cli/cmd/doc/upload.go | 150 ++------- cli/cmd/doc/upload_recursive.go | 80 ++--- cli/cmd/doc/upload_recursive_test.go | 108 ++++--- cli/cmd/doc/upload_test.go | 243 ++------------ cli/cmd/kb/empty.go | 88 ----- cli/cmd/kb/empty_test.go | 81 ----- cli/cmd/kb/kb.go | 8 +- cli/cmd/session/ask.go | 275 ++++++++++++++++ cli/cmd/session/ask_test.go | 465 +++++++++++++++++++++++++++ cli/cmd/session/session.go | 7 +- 20 files changed, 1866 insertions(+), 1377 deletions(-) delete mode 100644 cli/cmd/agent/invoke.go delete mode 100644 cli/cmd/agent/invoke_test.go create mode 100644 cli/cmd/doc/create.go create mode 100644 cli/cmd/doc/create_test.go create mode 100644 cli/cmd/doc/fetch.go create mode 100644 cli/cmd/doc/fetch_test.go delete mode 100644 cli/cmd/kb/empty.go delete mode 100644 cli/cmd/kb/empty_test.go create mode 100644 cli/cmd/session/ask.go create mode 100644 cli/cmd/session/ask_test.go diff --git a/cli/cmd/agent/agent.go b/cli/cmd/agent/agent.go index f9de08f3..72bdbaed 100644 --- a/cli/cmd/agent/agent.go +++ b/cli/cmd/agent/agent.go @@ -1,13 +1,12 @@ // Package agentcmd holds the `weknora agent` command tree: -// list / view / invoke / create / edit / delete. The directory is named -// `agent/` to match the cobra subcommand; the Go package is `agentcmd` +// list / view / create / edit / delete / status / check. The directory is +// named `agent/` to match the cobra subcommand; 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 -// `agent invoke` calls /agent-chat/:session_id which dispatches the -// agent's configured workflow (system prompt, allowed tools, KB scope, -// retrieval thresholds). +// Agents (server resource: GET/POST /agents/...) and handles CRUD +// operations only. Agent invocation has moved to `weknora session ask +// --agent ` (see cli/cmd/session/ask.go). package agentcmd import ( @@ -21,16 +20,13 @@ import ( func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "agent", - Short: "Manage and invoke custom agents", + Short: "Manage custom agents (CRUD + status/check)", Long: `Custom Agents bundle a system prompt, model, tool allow-list, and KB -scope into an addressable resource. Create, edit, list, view, invoke, -or delete agents.`, - Args: cobra.NoArgs, - Run: func(c *cobra.Command, _ []string) { _ = c.Help() }, +scope into an addressable resource. Create, edit, list, view, check, +or delete agents. To invoke an agent, use: weknora session ask --agent `, } cmd.AddCommand(NewCmdList(f)) cmd.AddCommand(NewCmdView(f)) - cmd.AddCommand(NewCmdInvoke(f)) cmd.AddCommand(NewCmdCreate(f)) cmd.AddCommand(NewCmdEdit(f)) cmd.AddCommand(NewCmdDelete(f)) diff --git a/cli/cmd/agent/invoke.go b/cli/cmd/agent/invoke.go deleted file mode 100644 index ffd142e2..00000000 --- a/cli/cmd/agent/invoke.go +++ /dev/null @@ -1,269 +0,0 @@ -package agentcmd - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "io" - "strings" - - "github.com/spf13/cobra" - - "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" - "github.com/Tencent/WeKnora/cli/internal/iostreams" - "github.com/Tencent/WeKnora/cli/internal/sse" - sdk "github.com/Tencent/WeKnora/client" -) - -// agentInvokeFields enumerates fields surfaced for `--format json` discovery -// on `agent invoke`. Matches invokeData below — the single-shot result -// object with the agent's final answer plus the trace (references, tool -// events). -var agentInvokeFields = []string{ - "answer", "references", "tool_events", "thinking", - "session_id", "agent_id", "query", -} - -// InvokeOptions captures `agent invoke` flag state. -type InvokeOptions struct { - AgentID string - Query string - SessionID string // --session: continue an existing session (skip auto-create) -} - -// InvokeService is the narrow SDK surface this command depends on. -// -// CreateSession is called when --session is omitted — sessions are -// agent-agnostic at creation (verified against -// internal/handler/session/handler.go CreateSession, which only persists -// {title, description}). The agent ID is supplied per-request via -// AgentQARequest.AgentID, so the same session can be reused across -// agent / KB-chat invocations. -type InvokeService interface { - CreateSession(ctx context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) - AgentQAStreamWithRequest(ctx context.Context, sessionID string, req *sdk.AgentQARequest, cb sdk.AgentEventCallback) error -} - -// invokeData is the JSON payload emitted on the JSON path. -type invokeData struct { - Answer string `json:"answer"` - References []*sdk.SearchResult `json:"references"` - ToolEvents []sse.AgentToolEvent `json:"tool_events,omitempty"` - Thinking string `json:"thinking,omitempty"` - SessionID string `json:"session_id"` - AgentID string `json:"agent_id"` - Query string `json:"query"` -} - -// NewCmdInvoke builds `weknora agent invoke ""`. -func NewCmdInvoke(f *cmdutil.Factory) *cobra.Command { - opts := &InvokeOptions{} - cmd := &cobra.Command{ - Use: `invoke ""`, - Short: "Run a query through a custom agent", - Long: `Sends a query to the agent's configured workflow (system prompt, allowed -tools, KB scope, retrieval thresholds) over SSE. By default a fresh session -is auto-created; pass --session to continue an existing conversation. The -agent picks the model, retrieval params, and tool surface from its own -config — agent invoke is the thin shim that streams the result. - -Modes: - TTY (text format, default): live answer streaming + tool-trace footer - --format json / pipe: buffered, single JSON object at completion`, - Example: ` weknora agent invoke ag_abc "Summarise the Q3 plan" - weknora agent invoke ag_abc "Continue?" --session sess_xyz - weknora agent invoke ag_abc "What did we ship?" --format json`, - Args: cobra.ExactArgs(2), - RunE: func(c *cobra.Command, args []string) error { - fopts, err := cmdutil.CheckFormatFlag(c) - if err != nil { - return err - } - fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) - opts.AgentID = args[0] - opts.Query = strings.TrimSpace(args[1]) - cli, err := f.Client() - if err != nil { - return err - } - return runInvoke(c.Context(), opts, fopts, cli) - }, - } - cmd.Flags().StringVar(&opts.SessionID, "session", "", "Continue an existing chat session (skip auto-create)") - cmdutil.AddFormatFlag(cmd, agentInvokeFields...) - return cmd -} - -func runInvoke(ctx context.Context, opts *InvokeOptions, fopts *cmdutil.FormatOptions, svc InvokeService) error { - if opts.Query == "" { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "query argument cannot be empty") - } - if opts.AgentID == "" { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "agent-id argument cannot be empty") - } - if svc == nil { - return cmdutil.NewError(cmdutil.CodeServerError, "agent invoke: no SDK client available") - } - - jsonOut := fopts != nil && fopts.Mode == cmdutil.FormatJSON - - sessionID := opts.SessionID - autoCreated := false - if sessionID == "" { - sess, err := svc.CreateSession(ctx, &sdk.CreateSessionRequest{Title: "weknora agent invoke"}) - if err != nil { - if isCancelled(ctx, err) { - return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, err, "agent invoke cancelled") - } - code := cmdutil.ClassifyHTTPError(err) - if code == cmdutil.CodeNetworkError || code == cmdutil.CodeServerError { - code = cmdutil.CodeSessionCreateFailed - } - return cmdutil.Wrapf(code, err, "create chat session") - } - sessionID = sess.ID - autoCreated = true - } - - // Streaming requires interactive stdout + no --format json + no - // --format ndjson (handled by early-return below). Matches chat.go's - // mode-selection contract so users get the same muscle memory across - // both commands. - streamMode := iostreams.IO.IsStdoutTTY() && !jsonOut && - (fopts == nil || fopts.Mode != cmdutil.FormatNDJSON) - - // Surface auto-created session id up-front so a ^C mid-stream still - // leaves a recoverable pointer. - if autoCreated && !jsonOut { - fmt.Fprintf(iostreams.IO.Err, "session: %s (use --session to continue)\n", sessionID) - } - - req := &sdk.AgentQARequest{ - Query: opts.Query, - AgentEnabled: true, - AgentID: opts.AgentID, - Channel: "api", - } - - // --format ndjson: stream raw SDK events as NDJSON. Encoder hoisted out - // of the callback to avoid per-event allocation. - if fopts != nil && fopts.Mode == cmdutil.FormatNDJSON { - enc := json.NewEncoder(iostreams.IO.Out) - enc.SetEscapeHTML(false) - cb := func(r *sdk.AgentStreamResponse) error { - return enc.Encode(r) - } - if err := svc.AgentQAStreamWithRequest(ctx, sessionID, req, cb); err != nil { - if isCancelled(ctx, err) { - return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, err, "agent invoke cancelled") - } - return cmdutil.WrapHTTP(err, "agent-chat stream") - } - return nil - } - - acc := &sse.AgentAccumulator{} - cb := func(r *sdk.AgentStreamResponse) error { - if streamMode && r != nil && r.ResponseType == sdk.AgentResponseTypeAnswer && r.Content != "" { - _, _ = iostreams.IO.Out.Write([]byte(r.Content)) - } - acc.Append(r) - return nil - } - - streamErr := svc.AgentQAStreamWithRequest(ctx, sessionID, req, cb) - if streamErr != nil { - if autoCreated && !jsonOut { - fmt.Fprintf(iostreams.IO.Err, "session: %s (resume with --session %s)\n", sessionID, sessionID) - } - if isCancelled(ctx, streamErr) { - return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, streamErr, "agent invoke cancelled") - } - if acc.Answer() != "" && !acc.Done() { - return cmdutil.Wrapf(cmdutil.CodeSSEStreamAborted, streamErr, "stream aborted before completion") - } - return cmdutil.WrapHTTP(streamErr, "agent-chat stream") - } - - // Server closed cleanly but never sent a Done event — treat as aborted - // so agents don't silently emit a truncated answer as ok=true. - if !acc.Done() { - return cmdutil.NewError(cmdutil.CodeSSEStreamAborted, "stream ended without a terminal event") - } - - answer := acc.Answer() - if jsonOut { - data := invokeData{ - Answer: answer, - References: acc.References, - ToolEvents: acc.ToolEvents, - Thinking: acc.Thinking(), - SessionID: sessionID, - AgentID: opts.AgentID, - Query: opts.Query, - } - return fopts.Emit(iostreams.IO.Out, data) - } - - out := iostreams.IO.Out - if streamMode { - if !strings.HasSuffix(answer, "\n") { - fmt.Fprintln(out) - } - } else { - fmt.Fprint(out, answer) - if !strings.HasSuffix(answer, "\n") { - fmt.Fprintln(out) - } - } - renderToolTrace(out, acc.ToolEvents) - format.WriteReferences(out, acc.References) - return nil -} - -// renderToolTrace prints a compact tool-event footer in human mode. -// Skipped when the agent emitted no tool events — silent beats an empty -// banner. -func renderToolTrace(w io.Writer, events []sse.AgentToolEvent) { - if len(events) == 0 { - return - } - fmt.Fprintln(w) - fmt.Fprintln(w, "──── Tool trace ────") - for i, e := range events { - fmt.Fprintf(w, "[%d] %s", i+1, e.Kind) - if e.Result != "" { - fmt.Fprintf(w, " %s", truncateInline(e.Result, 80)) - } - fmt.Fprintln(w) - } -} - -// truncateInline shrinks a multi-line result to a single line + ellipsis -// for the human tool-trace footer. -func truncateInline(s string, maxLen int) string { - s = strings.ReplaceAll(s, "\n", " ") - if len(s) <= maxLen { - return s - } - return s[:maxLen-1] + "…" -} - -// isCancelled reports whether err or ctx represents a context-cancelled -// state — true on Ctrl-C / SIGTERM after main.go's signal.NotifyContext -// fires. Wrapping URL/transport layers may rewrite context.Canceled into -// something errors.Is no longer recognises, so we fall back to ctx.Err(). -func isCancelled(ctx context.Context, err error) bool { - if errors.Is(err, context.Canceled) { - return true - } - if ctx.Err() == context.Canceled { - return true - } - return false -} - -// compile-time check: production SDK client satisfies InvokeService. -var _ InvokeService = (*sdk.Client)(nil) diff --git a/cli/cmd/agent/invoke_test.go b/cli/cmd/agent/invoke_test.go deleted file mode 100644 index 75d8870c..00000000 --- a/cli/cmd/agent/invoke_test.go +++ /dev/null @@ -1,344 +0,0 @@ -package agentcmd - -import ( - "bytes" - "context" - "encoding/json" - "errors" - "os" - "strings" - "testing" - - "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/iostreams" - sdk "github.com/Tencent/WeKnora/client" -) - -// scriptedInvokeSvc serves a canned stream of agent events to runInvoke. -type scriptedInvokeSvc struct { - createResp *sdk.Session - createErr error - events []*sdk.AgentStreamResponse - streamErr error - got struct { - sessionID string - req *sdk.AgentQARequest - } -} - -func (s *scriptedInvokeSvc) CreateSession(_ context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) { - if s.createResp == nil && s.createErr == nil { - return &sdk.Session{ID: "sess_auto", Title: req.Title}, nil - } - return s.createResp, s.createErr -} - -func (s *scriptedInvokeSvc) AgentQAStreamWithRequest(_ context.Context, sessionID string, req *sdk.AgentQARequest, cb sdk.AgentEventCallback) error { - s.got.sessionID = sessionID - s.got.req = req - for _, e := range s.events { - if err := cb(e); err != nil { - return err - } - } - return s.streamErr -} - -func answerEvent(content string) *sdk.AgentStreamResponse { - return &sdk.AgentStreamResponse{ResponseType: sdk.AgentResponseTypeAnswer, Content: content} -} -func doneEvent() *sdk.AgentStreamResponse { - return &sdk.AgentStreamResponse{ResponseType: sdk.AgentResponseTypeAnswer, Done: true} -} -func toolCallEvent(id, name string) *sdk.AgentStreamResponse { - return &sdk.AgentStreamResponse{ - ResponseType: sdk.AgentResponseTypeToolCall, - ID: id, - Content: name, - } -} -func referencesEvent(refs []*sdk.SearchResult) *sdk.AgentStreamResponse { - return &sdk.AgentStreamResponse{ - ResponseType: sdk.AgentResponseTypeReferences, - KnowledgeReferences: refs, - } -} - -// textOpts returns a FormatOptions configured for the text (human) render -// path — the most common shape under test. -func textOpts() *cmdutil.FormatOptions { - return &cmdutil.FormatOptions{Mode: cmdutil.FormatText} -} - -// jsonOpts returns a FormatOptions configured for the JSON path. -func jsonOpts() *cmdutil.FormatOptions { - return &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} -} - -func TestInvoke_AccumulateMode_EmitsBareJSON(t *testing.T) { - out, _ := iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{ - events: []*sdk.AgentStreamResponse{ - answerEvent("Hello "), - answerEvent("world."), - referencesEvent([]*sdk.SearchResult{{KnowledgeID: "k1", KnowledgeTitle: "Doc 1"}}), - doneEvent(), - }, - } - opts := &InvokeOptions{AgentID: "ag_x", Query: "ping"} - if err := runInvoke(context.Background(), opts, jsonOpts(), svc); err != nil { - t.Fatalf("runInvoke: %v", err) - } - var got invokeData - if err := json.Unmarshal(out.Bytes(), &got); err != nil { - t.Fatalf("parse: %v\n%s", err, out.String()) - } - if got.Answer != "Hello world." { - t.Errorf("answer = %q, want %q", got.Answer, "Hello world.") - } - if got.AgentID != "ag_x" { - t.Errorf("agent_id = %q, want ag_x", got.AgentID) - } - if got.Query != "ping" { - t.Errorf("query = %q, want ping", got.Query) - } - if got.SessionID != "sess_auto" { - t.Errorf("session_id = %q, want sess_auto", got.SessionID) - } - if len(got.References) != 1 || got.References[0].KnowledgeID != "k1" { - t.Errorf("references missing: %+v", got.References) - } -} - -func TestInvoke_AutoCreatedSessionID_PassedAsAgentRequest(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{events: []*sdk.AgentStreamResponse{doneEvent()}} - opts := &InvokeOptions{AgentID: "ag_42", Query: "x"} - if err := runInvoke(context.Background(), opts, jsonOpts(), svc); err != nil { - t.Fatalf("runInvoke: %v", err) - } - if svc.got.sessionID != "sess_auto" { - t.Errorf("agent-chat got sessionID=%q, want sess_auto", svc.got.sessionID) - } - if svc.got.req == nil || svc.got.req.AgentID != "ag_42" { - t.Errorf("AgentID not forwarded: %+v", svc.got.req) - } - if !svc.got.req.AgentEnabled { - t.Error("AgentEnabled must be true for agent invoke") - } -} - -func TestInvoke_ExistingSessionID_SkipsCreate(t *testing.T) { - _, _ = iostreams.SetForTest(t) - created := false - svc := &scriptedInvokeSvc{events: []*sdk.AgentStreamResponse{doneEvent()}} - // Wrap CreateSession to detect call. - svc.createResp = &sdk.Session{ID: "should_not_be_used"} - wrapped := &createSessionTracker{InvokeService: svc, called: &created} - opts := &InvokeOptions{AgentID: "ag", Query: "x", SessionID: "sess_existing"} - if err := runInvoke(context.Background(), opts, jsonOpts(), wrapped); err != nil { - t.Fatalf("runInvoke: %v", err) - } - if created { - t.Error("CreateSession should not be called when --session is set") - } - if svc.got.sessionID != "sess_existing" { - t.Errorf("agent-chat got sessionID=%q, want sess_existing", svc.got.sessionID) - } -} - -type createSessionTracker struct { - InvokeService - called *bool -} - -func (c *createSessionTracker) CreateSession(ctx context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) { - *c.called = true - return c.InvokeService.CreateSession(ctx, req) -} - -func TestInvoke_ToolEventsCaptured(t *testing.T) { - out, _ := iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{events: []*sdk.AgentStreamResponse{ - toolCallEvent("call_1", "knowledge_search"), - answerEvent("answer text"), - doneEvent(), - }} - opts := &InvokeOptions{AgentID: "ag", Query: "x"} - if err := runInvoke(context.Background(), opts, jsonOpts(), svc); err != nil { - t.Fatalf("runInvoke: %v", err) - } - var got invokeData - if err := json.Unmarshal(out.Bytes(), &got); err != nil { - t.Fatalf("parse: %v", err) - } - if len(got.ToolEvents) != 1 { - t.Fatalf("expected 1 tool call, got %d", len(got.ToolEvents)) - } - if got.ToolEvents[0].ID != "call_1" { - t.Errorf("tool_calls[0].id = %q, want call_1", got.ToolEvents[0].ID) - } -} - -func TestInvoke_EmptyQuery_Rejected(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{} - opts := &InvokeOptions{AgentID: "ag", Query: ""} - err := runInvoke(context.Background(), opts, textOpts(), svc) - if err == nil { - t.Fatal("expected input.invalid_argument, got nil") - } - var typed *cmdutil.Error - if !errors.As(err, &typed) || typed.Code != cmdutil.CodeInputInvalidArgument { - t.Errorf("expected input.invalid_argument, got %v", err) - } -} - -func TestInvoke_StreamAbortBeforeDone_MapsToSSEStreamAborted(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{ - events: []*sdk.AgentStreamResponse{ - answerEvent("partial"), - }, - streamErr: errors.New("connection reset"), - } - opts := &InvokeOptions{AgentID: "ag", Query: "x"} - err := runInvoke(context.Background(), opts, jsonOpts(), svc) - if err == nil { - t.Fatal("expected stream-aborted error") - } - var typed *cmdutil.Error - if !errors.As(err, &typed) || typed.Code != cmdutil.CodeSSEStreamAborted { - t.Errorf("expected local.sse_stream_aborted, got %v", err) - } -} - -func TestInvoke_NoDoneEvent_MapsToSSEStreamAborted(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{events: []*sdk.AgentStreamResponse{answerEvent("incomplete")}} - opts := &InvokeOptions{AgentID: "ag", Query: "x"} - err := runInvoke(context.Background(), opts, jsonOpts(), svc) - if err == nil { - t.Fatal("expected stream-aborted error") - } - var typed *cmdutil.Error - if !errors.As(err, &typed) || typed.Code != cmdutil.CodeSSEStreamAborted { - t.Errorf("expected local.sse_stream_aborted, got %v", err) - } -} - -func TestInvoke_CreateSessionFails_MapsToSessionCreateFailed(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{createErr: errors.New("connection refused")} - opts := &InvokeOptions{AgentID: "ag", Query: "x"} - err := runInvoke(context.Background(), opts, textOpts(), svc) - if err == nil { - t.Fatal("expected session_create_failed") - } - var typed *cmdutil.Error - if !errors.As(err, &typed) || typed.Code != cmdutil.CodeSessionCreateFailed { - t.Errorf("expected server.session_create_failed, got %v", err) - } -} - -func TestInvoke_Cancellation_MapsToOperationCancelled(t *testing.T) { - _, _ = iostreams.SetForTest(t) - ctx, cancel := context.WithCancel(context.Background()) - cancel() // pre-cancel - svc := &scriptedInvokeSvc{streamErr: context.Canceled} - opts := &InvokeOptions{AgentID: "ag", Query: "x"} - err := runInvoke(ctx, opts, jsonOpts(), svc) - if err == nil { - t.Fatal("expected operation.cancelled") - } - var typed *cmdutil.Error - if !errors.As(err, &typed) || typed.Code != cmdutil.CodeOperationCancelled { - t.Errorf("expected operation.cancelled, got %v", err) - } -} - -// Sanity: human-mode output writes the answer body and a tool-trace footer. -func TestInvoke_Human_Accumulate_PrintsAnswerAndFooter(t *testing.T) { - out, _ := iostreams.SetForTest(t) - svc := &scriptedInvokeSvc{events: []*sdk.AgentStreamResponse{ - answerEvent("hello"), - toolCallEvent("c1", "knowledge_search"), - doneEvent(), - }} - opts := &InvokeOptions{AgentID: "ag", Query: "x"} - if err := runInvoke(context.Background(), opts, textOpts(), svc); err != nil { - t.Fatalf("runInvoke: %v", err) - } - got := out.String() - if !strings.Contains(got, "hello") { - t.Errorf("answer body missing: %q", got) - } - if !strings.Contains(got, "Tool trace") { - t.Errorf("tool trace footer missing: %q", got) - } -} - -func TestAgentInvoke_FormatNDJSON_PassthroughsSDKEvents(t *testing.T) { - // Fake stream emits 3 events: tool_call, answer, done. - // Mirrors TestChat_FormatNDJSON_PassthroughsSDKEvents in chat_test.go but - // uses AgentStreamResponse instead of StreamResponse. - svc := &scriptedInvokeSvc{ - events: []*sdk.AgentStreamResponse{ - toolCallEvent("call_1", "knowledge_search"), - answerEvent("hello"), - doneEvent(), - }, - } - - var stdout bytes.Buffer - prev := iostreams.IO.Out - iostreams.IO.Out = &stdout - defer func() { iostreams.IO.Out = prev }() - // Redirect stderr so the auto-created session hint doesn't write to real - // stderr during the test. - prevErr := iostreams.IO.Err - iostreams.IO.Err = os.Stderr - defer func() { iostreams.IO.Err = prevErr }() - - opts := &InvokeOptions{AgentID: "ag_x", Query: "hi"} - fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatNDJSON} - if err := runInvoke(context.Background(), opts, fopts, svc); err != nil { - t.Fatalf("runInvoke: %v", err) - } - - lines := strings.Split(strings.TrimRight(stdout.String(), "\n"), "\n") - if len(lines) != 3 { - t.Fatalf("got %d lines, want 3:\n%s", len(lines), stdout.String()) - } - // Each line must be valid JSON. - for i, line := range lines { - var obj map[string]any - if err := json.Unmarshal([]byte(line), &obj); err != nil { - t.Fatalf("line %d not valid JSON: %v\n %s", i+1, err, line) - } - } - // First line: tool_call event. - var first map[string]any - if err := json.Unmarshal([]byte(lines[0]), &first); err != nil { - t.Fatalf("line 1 not JSON: %v", err) - } - if first["response_type"] != string(sdk.AgentResponseTypeToolCall) { - t.Errorf("first event response_type=%v, want %s", first["response_type"], sdk.AgentResponseTypeToolCall) - } - // Second line: answer event. - var second map[string]any - if err := json.Unmarshal([]byte(lines[1]), &second); err != nil { - t.Fatalf("line 2 not JSON: %v", err) - } - if second["response_type"] != string(sdk.AgentResponseTypeAnswer) { - t.Errorf("second event response_type=%v, want %s", second["response_type"], sdk.AgentResponseTypeAnswer) - } - // Third line: done event. - var third map[string]any - if err := json.Unmarshal([]byte(lines[2]), &third); err != nil { - t.Fatalf("line 3 not JSON: %v", err) - } - if third["done"] != true { - t.Errorf("third event done=%v, want true", third["done"]) - } -} diff --git a/cli/cmd/doc/create.go b/cli/cmd/doc/create.go new file mode 100644 index 00000000..b3d1f17a --- /dev/null +++ b/cli/cmd/doc/create.go @@ -0,0 +1,120 @@ +// Package doc — create.go implements `weknora doc create --text "..."`. +// Allows creating a knowledge entry directly from inline text content without +// uploading a file or fetching a remote URL. +package doc + +import ( + "cmp" + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// docCreateFields enumerates the fields surfaced for `--format json` discovery +// on `doc create`. +var docCreateFields = []string{ + "id", "knowledge_base_id", "tag_id", "type", "title", "description", + "source", "channel", "parse_status", "summary_status", "enable_status", + "created_at", "updated_at", "error_message", +} + +// CreateOptions holds CLI flag values for `doc create`. +type CreateOptions struct { + Text string // --text (required): document text content (Markdown) + Name string // --name: document title + TagID string // --tag-id: associate with a tag + Channel string // --channel: ingestion-channel tag (default "api") +} + +// CreateService is the narrow SDK surface for `doc create`. +// *sdk.Client satisfies it. +type CreateService interface { + CreateManualKnowledge( + ctx context.Context, + kbID string, + req *sdk.CreateManualKnowledgeRequest, + ) (*sdk.Knowledge, error) +} + +// NewCmdCreate builds `weknora doc create --text "..."`. +func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { + opts := &CreateOptions{} + cmd := &cobra.Command{ + Use: "create", + Short: "Create a knowledge entry from inline text content", + Long: `Create a new knowledge entry by passing Markdown content directly via --text. +Useful for short snippets, agent-generated content, or structured notes that +don't require a file upload or remote URL. KB resolution follows the standard +4-level chain: --kb flag > WEKNORA_KB_ID env > .weknora/project.yaml > error. + + --text Document text in Markdown format (required). + --name Display title stored with the entry. + --tag-id <id> Associate the new entry with a tag. + --channel <name> Override the ingestion-channel tag (default "api").`, + Example: ` weknora doc create --text "# Meeting Notes\n\nAction items: ..." + weknora doc create --text "$(cat notes.md)" --name "Sprint Notes" --kb my-kb + weknora doc create --text "API usage guide" --name "API Guide" --tag-id tag_abc`, + Args: cobra.NoArgs, + RunE: func(c *cobra.Command, _ []string) error { + fopts, err := cmdutil.CheckFormatFlag(c) + if err != nil { + return err + } + fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) + kbID, err := f.ResolveKB(c) + if err != nil { + return err + } + cli, err := f.Client() + if err != nil { + return err + } + return runCreate(c.Context(), opts, fopts, cli, kbID) + }, + } + cmd.Flags().String("kb", "", "Knowledge base UUID or name (overrides env / project link)") + cmd.Flags().StringVar(&opts.Text, "text", "", "Document text content in Markdown format (required)") + cmd.Flags().StringVar(&opts.Name, "name", "", "Document title") + cmd.Flags().StringVar(&opts.TagID, "tag-id", "", "Tag id to associate with the new entry") + cmd.Flags().StringVar(&opts.Channel, "channel", "", "Ingestion-channel tag recorded server-side (default \"api\")") + _ = cmd.MarkFlagRequired("text") + cmdutil.AddFormatFlag(cmd, docCreateFields...) + return cmd +} + +// runCreate creates a manual knowledge entry via SDK CreateManualKnowledge. +func runCreate(ctx context.Context, opts *CreateOptions, fopts *cmdutil.FormatOptions, svc CreateService, kbID string) error { + // Guard against empty text: cobra's MarkFlagRequired enforces this for + // normal CLI invocations; this check protects tests that call runCreate + // directly and any future callers that bypass cobra. + if opts.Text == "" { + return cmdutil.NewFlagError(fmt.Errorf("--text is required")) + } + req := &sdk.CreateManualKnowledgeRequest{ + Title: opts.Name, + Content: opts.Text, + TagID: opts.TagID, + Channel: cmp.Or(opts.Channel, uploadChannel), + } + k, err := svc.CreateManualKnowledge(ctx, kbID, req) + if err != nil { + return cmdutil.WrapHTTP(err, "create document") + } + if fopts.WantsJSON() { + return fopts.Emit(iostreams.IO.Out, k, nil) + } + displayed := opts.Name + if displayed == "" { + displayed = k.Title + } + if displayed == "" { + displayed = k.ID + } + fmt.Fprintf(iostreams.IO.Out, "✓ Created %q (id: %s)\n", displayed, k.ID) + return nil +} diff --git a/cli/cmd/doc/create_test.go b/cli/cmd/doc/create_test.go new file mode 100644 index 00000000..55df8c16 --- /dev/null +++ b/cli/cmd/doc/create_test.go @@ -0,0 +1,140 @@ +package doc + +import ( + "context" + "encoding/json" + "errors" + "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 captures call arguments and returns canned responses. +type fakeCreateSvc struct { + resp *sdk.Knowledge + err error + got struct { + kbID string + req *sdk.CreateManualKnowledgeRequest + } +} + +func (f *fakeCreateSvc) CreateManualKnowledge( + _ context.Context, + kbID string, + req *sdk.CreateManualKnowledgeRequest, +) (*sdk.Knowledge, error) { + f.got.kbID = kbID + f.got.req = req + return f.resp, f.err +} + +func TestCreate_Success_Human(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_manual_1", Title: "Sprint Notes"}} + opts := &CreateOptions{Text: "# Sprint Notes\n\nAction items: ...", Name: "Sprint Notes"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + + assert.Equal(t, "kb_xxx", svc.got.kbID) + assert.Equal(t, "# Sprint Notes\n\nAction items: ...", svc.got.req.Content) + assert.Equal(t, "Sprint Notes", svc.got.req.Title) + assert.Contains(t, out.String(), "Created") + assert.Contains(t, out.String(), "doc_manual_1") +} + +func TestCreate_Success_NoName_FallsBackToTitle(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_manual_2", Title: "Server Title"}} + opts := &CreateOptions{Text: "Some content"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + // When --name is omitted the display falls back to k.Title from the server response. + assert.Contains(t, out.String(), "Server Title") +} + +func TestCreate_Success_NoName_NoTitle_FallsBackToID(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_manual_3"}} + opts := &CreateOptions{Text: "Some content"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Contains(t, out.String(), "doc_manual_3") +} + +func TestCreate_TagID_Forwarded(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_t"}} + opts := &CreateOptions{Text: "content", TagID: "tag_42"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "tag_42", svc.got.req.TagID) +} + +func TestCreate_Channel_Override(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_ch"}} + opts := &CreateOptions{Text: "content", Channel: "browser_extension"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "browser_extension", svc.got.req.Channel) +} + +func TestCreate_Channel_DefaultIsAPI(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_ch"}} + opts := &CreateOptions{Text: "content"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, uploadChannel, svc.got.req.Channel) +} + +func TestCreate_JSON_Envelope(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "doc_manual_json", Title: "My Note"}} + opts := &CreateOptions{Text: "# My Note", Name: "My Note"} + fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} + require.NoError(t, runCreate(context.Background(), opts, fopts, svc, "kb_xxx")) + + got := out.String() + var env struct { + OK bool `json:"ok"` + Data sdk.Knowledge `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(got), &env), "expected valid JSON envelope, got %q", got) + assert.True(t, env.OK, "envelope.ok must be true") + assert.Equal(t, "doc_manual_json", env.Data.ID, "envelope.data.id must match") +} + +func TestCreate_ServerError_Wraps(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{err: errors.New("HTTP error 500: internal server error")} + err := runCreate(context.Background(), + &CreateOptions{Text: "content"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeServerError, typed.Code) +} + +func TestCreate_HTTPError_400_Wraps(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{err: errors.New("HTTP error 400: bad request")} + err := runCreate(context.Background(), + &CreateOptions{Text: "content"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) +} + +func TestCreate_EmptyText_RejectsBeforeSDK(t *testing.T) { + _, _ = iostreams.SetForTest(t) + // The SDK should never be called when --text is empty (runCreate has a + // defensive guard; cobra's MarkFlagRequired catches it earlier in RunE). + svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "should-not-reach"}} + err := runCreate(context.Background(), + &CreateOptions{Text: ""}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx") + require.Error(t, err) + // Verify the SDK was NOT called. + assert.Nil(t, svc.got.req, "SDK must not be called when --text is empty") +} diff --git a/cli/cmd/doc/delete.go b/cli/cmd/doc/delete.go index 2ad5009b..bb791a23 100644 --- a/cli/cmd/doc/delete.go +++ b/cli/cmd/doc/delete.go @@ -3,10 +3,12 @@ package doc import ( "context" "fmt" - "io" + "strings" "github.com/spf13/cobra" + sdk "github.com/Tencent/WeKnora/client" + "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" @@ -17,7 +19,9 @@ import ( var docDeleteFields = []string{"id", "deleted"} type DeleteOptions struct { - Yes bool // sourced from the global -y/--yes persistent flag (see cli/cmd/root.go) + Yes bool // sourced from the global -y/--yes persistent flag (see cli/cmd/root.go) + All bool // delete all docs in --kb + KB string // required when --all } // DeleteService is the narrow SDK surface this command depends on. @@ -26,33 +30,26 @@ type DeleteService interface { DeleteKnowledge(ctx context.Context, id string) error } +// AllService is the narrow SDK surface for --all mode. +// *sdk.Client satisfies it. +type AllService interface { + ClearKnowledgeBaseContents(ctx context.Context, kbID string) (*sdk.ClearKnowledgeBaseContentsResponse, error) +} + // deleteResult is the typed payload emitted under data on success (single-id). type deleteResult struct { ID string `json:"id"` Deleted bool `json:"deleted"` } -// MultiDeleteResult is the payload for multi-id deletes. -// ok: ids successfully deleted; failed: ids that could not be deleted. -type MultiDeleteResult struct { - OK []string `json:"ok"` - Failed []FailedItem `json:"failed,omitempty"` -} - -// FailedItem records an id that failed to delete along with its error message. -type FailedItem struct { - ID string `json:"id"` - Code string `json:"code,omitempty"` - Message string `json:"message"` -} - // NewCmdDelete builds `weknora doc delete`. Single-id keeps the simpler // code path (one confirm prompt, exit 0/1); multi-id uses keep-going -// semantics (one -y confirms all, failures collected, exit 1 if any fail). +// semantics (one -y confirms all, failures collected, exit 1 if any fail); +// --all --kb=<id> atomically clears every document in a knowledge base. func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { opts := &DeleteOptions{} cmd := &cobra.Command{ - Use: "delete <doc-id> [<doc-id>...]", + Use: "delete <doc-id> [<doc-id>...] | --all --kb=<kb-id>", Short: "Delete one or more documents from a knowledge base", Long: `Permanently deletes one or more documents. Prompts for confirmation by default when stdout is a TTY and JSON output is not set; pass -y/--yes @@ -65,15 +62,23 @@ Multi-id: • TTY prompt shows total: "Delete N document(s)? This cannot be undone." • Exit 0 if all succeed; exit 1 if any failed. +All-in-KB (--all --kb=<kb-id>): + • Atomically clears every document in the named knowledge base. + • The KB record itself (name, config) is preserved. + • Mutually exclusive with positional doc ids. + • Exit 0 on success; exit 10 without -y in non-interactive/JSON mode. + 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.`, - Example: ` weknora doc delete doc_abc # interactive confirm - weknora doc delete doc_abc -y # no prompt - weknora doc delete doc_abc -y --format json # bare {id, deleted:true} JSON - weknora doc delete doc_a doc_b doc_c -y # delete 3, keep-going - weknora doc delete doc_a doc_b --format json # multi-id JSON output`, - Args: cobra.MinimumNArgs(1), + Example: ` weknora doc delete doc_abc # interactive confirm + weknora doc delete doc_abc -y # no prompt + weknora doc delete doc_abc -y --format json # bare {id, deleted:true} JSON + weknora doc delete doc_a doc_b doc_c -y # delete 3, keep-going + weknora doc delete doc_a doc_b --format json # multi-id JSON output + weknora doc delete --all --kb=kb_x -y # clear all docs in kb_x + weknora doc delete --all --kb=kb_x -y --format json # agent-friendly`, + Args: cobra.ArbitraryArgs, RunE: func(c *cobra.Command, args []string) error { fopts, err := cmdutil.CheckFormatFlag(c) if err != nil { @@ -85,28 +90,65 @@ without the user's explicit go-ahead.`, if err != nil { return err } + + if opts.All { + if opts.KB == "" { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "--all requires --kb=<id>"). + WithHint("specify --kb=<kb-id> to scope the delete-all operation"). + WithRetryCommand("weknora doc delete --all --kb=<kb-id> -y") + } + if len(args) > 0 { + return cmdutil.NewFlagError(fmt.Errorf("--all is exclusive with positional doc ids")) + } + return runDeleteAll(c.Context(), opts, fopts, cli, f.Prompter()) + } + if len(args) == 0 { + return cmdutil.NewFlagError(fmt.Errorf("doc id(s) required (or use --all --kb=<id>)")) + } // Single-id uses the simpler code path (bare {id, deleted}). if len(args) == 1 { return runDelete(c.Context(), opts, fopts, cli, f.Prompter(), args[0]) } - res, runErr := runMultiDelete(c.Context(), opts, fopts, cli, f.Prompter(), args) + if err := cmdutil.ConfirmDestructiveBatch(f.Prompter(), opts.Yes, fopts.WantsJSON(), "document", len(args), "doc.delete", "weknora doc delete "+strings.Join(args, " ")+" -y"); err != nil { + return err + } + outcomes, runErr := cmdutil.RunBatch(c.Context(), args, func(ctx context.Context, id string) error { + if err := cli.DeleteKnowledge(ctx, id); err != nil { + return cmdutil.WrapHTTP(err, "delete document %s", id) + } + return nil + }) // Only emit when the operation actually ran. Pre-flight errors // (e.g. confirmation_required) must leave stdout empty per the // wire contract in README.md. - if len(res.OK) > 0 || len(res.Failed) > 0 { - if emitErr := emitMultiDelete(res, fopts, iostreams.IO.Out); emitErr != nil { + if len(outcomes) > 0 { + if emitErr := cmdutil.EmitBatch(outcomes, fopts, iostreams.IO.Out, cmdutil.DeletedAtNow); emitErr != nil { return emitErr } } return runErr }, } + cmd.Flags().BoolVar(&opts.All, "all", false, "delete all documents in the KB specified by --kb") + cmd.Flags().StringVar(&opts.KB, "kb", "", "knowledge base ID (required with --all)") cmdutil.AddFormatFlag(cmd, docDeleteFields...) + cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ + UsedFor: "permanently delete one or more documents from a knowledge base", + RequiredFlags: []string{"<doc-id>... (positional) | --all --kb=<id>"}, + Examples: []string{ + "weknora doc delete doc_abc -y", + "weknora doc delete doc_a doc_b doc_c -y", + "weknora doc delete --all --kb=kb_x -y --format json", + }, + Warnings: []string{ + "doc delete is irreversible. --all --kb=<id> atomically clears every document in the KB; that is especially destructive. Never auto-add -y; surface the exit-10 prompt to the user and only retry after explicit approval.", + }, + }) return cmd } func runDelete(ctx context.Context, opts *DeleteOptions, fopts *cmdutil.FormatOptions, svc DeleteService, p prompt.Prompter, id string) error { - if err := cmdutil.ConfirmDestructive(p, opts.Yes, fopts.WantsJSON(), "document", id); err != nil { + if err := cmdutil.ConfirmDestructive(p, opts.Yes, fopts.WantsJSON(), "document", id, "doc.delete", "weknora doc delete "+id+" -y"); err != nil { return err } @@ -115,46 +157,36 @@ func runDelete(ctx context.Context, opts *DeleteOptions, fopts *cmdutil.FormatOp } if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}) + return fopts.Emit(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}, nil) } fmt.Fprintf(iostreams.IO.Out, "✓ Deleted document %s\n", id) return nil } -// runMultiDelete iterates ids sequentially, keep-going on error: a single -// failure does not abort the run, so the caller sees the full outcome. -func runMultiDelete(ctx context.Context, opts *DeleteOptions, fopts *cmdutil.FormatOptions, svc DeleteService, p prompt.Prompter, ids []string) (*MultiDeleteResult, error) { - if err := cmdutil.ConfirmDestructiveBatch(p, opts.Yes, fopts.WantsJSON(), "document", len(ids)); err != nil { - return &MultiDeleteResult{}, err +// runDeleteAll atomically clears every document in opts.KB via a single +// ClearKnowledgeBaseContents call. Non-TTY/JSON mode without -y returns +// CodeInputConfirmationRequired (exit 10) with risk metadata so agents can +// surface the risk to the user before re-invoking with -y. +func runDeleteAll(ctx context.Context, opts *DeleteOptions, fopts *cmdutil.FormatOptions, svc AllService, p prompt.Prompter) error { + if err := cmdutil.ConfirmDestructive(p, opts.Yes, fopts.WantsJSON(), "all docs in KB", opts.KB, "doc.delete_all", fmt.Sprintf("weknora doc delete --all --kb=%s -y", opts.KB)); err != nil { + return err } - res := &MultiDeleteResult{} - for _, id := range ids { - if err := svc.DeleteKnowledge(ctx, id); err != nil { - res.Failed = append(res.Failed, FailedItem{ID: id, Message: err.Error()}) - continue - } - res.OK = append(res.OK, id) - } - if len(res.Failed) > 0 { - return res, cmdutil.NewError(cmdutil.CodeOperationFailed, fmt.Sprintf("%d/%d delete(s) failed", len(res.Failed), len(ids))) - } - return res, nil -} -// emitMultiDelete renders per --format. Mirrors emitWaitResult / emitStatus. -func emitMultiDelete(res *MultiDeleteResult, fopts *cmdutil.FormatOptions, w io.Writer) error { - switch fopts.Mode { - case cmdutil.FormatJSON, cmdutil.FormatNDJSON: - return fopts.Emit(w, res) - case cmdutil.FormatText, "": - for _, id := range res.OK { - fmt.Fprintf(w, "OK %s\n", id) - } - for _, f := range res.Failed { - fmt.Fprintf(w, "FAIL %s: %s\n", f.ID, f.Message) - } - return nil - default: - return fmt.Errorf("unsupported --format %q for doc delete", fopts.Mode) + resp, err := svc.ClearKnowledgeBaseContents(ctx, opts.KB) + if err != nil { + return cmdutil.WrapHTTP(err, "clear KB %s", opts.KB) } + deleted := 0 + if resp != nil { + deleted = resp.DeletedCount + } + + if !fopts.WantsJSON() { + fmt.Fprintf(iostreams.IO.Out, "✓ Deleted %d document(s) from KB %s\n", deleted, opts.KB) + return nil + } + return fopts.Emit(iostreams.IO.Out, map[string]any{ + "kb_id": opts.KB, + "deleted_count": deleted, + }, nil) } diff --git a/cli/cmd/doc/delete_test.go b/cli/cmd/doc/delete_test.go index fc71c967..708b915a 100644 --- a/cli/cmd/doc/delete_test.go +++ b/cli/cmd/doc/delete_test.go @@ -5,17 +5,39 @@ import ( "context" "encoding/json" "errors" - "strings" + "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + sdk "github.com/Tencent/WeKnora/client" + "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/testutil" ) +// fakeAllSvc implements AllService for --all mode tests. +type fakeAllSvc struct { + err error + gotID string + called bool + resp *sdk.ClearKnowledgeBaseContentsResponse +} + +func (f *fakeAllSvc) ClearKnowledgeBaseContents(_ context.Context, kbID string) (*sdk.ClearKnowledgeBaseContentsResponse, error) { + f.called = true + f.gotID = kbID + if f.err != nil { + return nil, f.err + } + if f.resp == nil { + return &sdk.ClearKnowledgeBaseContentsResponse{DeletedCount: 0}, nil + } + return f.resp, nil +} + // fakeDeleteSvc captures calls and returns canned errors. // errFor maps id → error for per-id failure injection (used in multi-id tests). type fakeDeleteSvc struct { @@ -54,7 +76,7 @@ func TestDelete_Success_WithForce(t *testing.T) { opts := &DeleteOptions{Yes: true} // Force=true short-circuits the confirm path; the prompter must not be // consulted, so any value works. - require.NoError(t, runDelete(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{Answer: false}, "doc_abc")) + require.NoError(t, runDelete(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{Answer: false}, "doc_abc")) assert.Equal(t, "doc_abc", svc.got) assert.Equal(t, 1, svc.calls) @@ -69,15 +91,20 @@ func TestDelete_Success_JSON(t *testing.T) { require.NoError(t, runDelete(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, &testutil.ConfirmPrompter{Answer: true}, "doc_abc")) got := out.String() - assert.True(t, strings.HasPrefix(strings.TrimSpace(got), `{"id":"doc_abc"`), "expected bare object; got %q", got) - assert.Contains(t, got, `"deleted":true`) - assert.NotContains(t, got, `"ok":`) + var env struct { + OK bool `json:"ok"` + Data map[string]any `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(got), &env), "expected valid JSON envelope, got %q", got) + assert.True(t, env.OK, "envelope.ok must be true") + assert.Equal(t, "doc_abc", env.Data["id"], "envelope.data.id must be doc_abc") + assert.Equal(t, true, env.Data["deleted"], "envelope.data.deleted must be true") } func TestDelete_NotFound_404(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{err: errors.New("HTTP error 404: not found")} - err := runDelete(context.Background(), &DeleteOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{}, "doc_missing") + err := runDelete(context.Background(), &DeleteOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{}, "doc_missing") require.Error(t, err) var typed *cmdutil.Error @@ -88,7 +115,7 @@ func TestDelete_NotFound_404(t *testing.T) { func TestDelete_HTTPError_500(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{err: errors.New("HTTP error 500: internal")} - err := runDelete(context.Background(), &DeleteOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{}, "doc_x") + err := runDelete(context.Background(), &DeleteOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{}, "doc_x") require.Error(t, err) var typed *cmdutil.Error @@ -102,7 +129,7 @@ func TestDelete_HTTPError_500(t *testing.T) { func TestDelete_ConfirmYes(t *testing.T) { out, _ := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{Answer: true}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{Answer: true}, "doc_abc") require.NoError(t, err) assert.Equal(t, 1, svc.calls, "user said yes ⇒ delete proceeds") assert.Contains(t, out.String(), "✓") @@ -111,7 +138,7 @@ func TestDelete_ConfirmYes(t *testing.T) { func TestDelete_ConfirmNo(t *testing.T) { _, errBuf := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{Answer: false}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{Answer: false}, "doc_abc") require.Error(t, err) assert.Equal(t, 0, svc.calls, "user said no ⇒ SDK must NOT be called") @@ -127,7 +154,7 @@ func TestDelete_ConfirmNo(t *testing.T) { func TestDelete_AgentPrompterErrors(t *testing.T) { _, _ = iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{Err: errors.New("no tty")}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{Err: errors.New("no tty")}, "doc_abc") require.Error(t, err) assert.Equal(t, 0, svc.calls) @@ -143,7 +170,7 @@ func TestDelete_AgentPrompterErrors(t *testing.T) { func TestDelete_NoYes_NonTTY_RequiresConfirmation(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{Err: errors.New("no tty")}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{Err: errors.New("no tty")}, "doc_abc") require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -153,46 +180,56 @@ func TestDelete_NoYes_NonTTY_RequiresConfirmation(t *testing.T) { } // --------------------------------------------------------------------------- -// Multi-id tests (runMultiDelete, keep-going semantics) +// Multi-id tests (cmdutil.RunBatch, keep-going semantics) // --------------------------------------------------------------------------- func TestRunMultiDelete_AllSucceed(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{} - res, err := runMultiDelete( + outcomes, err := cmdutil.RunBatch( context.Background(), - &DeleteOptions{Yes: true}, - &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, - svc, - &testutil.ConfirmPrompter{Answer: true}, []string{"a", "b", "c"}, + func(ctx context.Context, id string) error { + if err := svc.DeleteKnowledge(ctx, id); err != nil { + return cmdutil.WrapHTTP(err, "delete document %s", id) + } + return nil + }, ) require.NoError(t, err) - assert.Equal(t, []string{"a", "b", "c"}, res.OK) - assert.Empty(t, res.Failed) + require.Len(t, outcomes, 3) + for _, oc := range outcomes { + assert.Nil(t, oc.Err, "expected no error for id %s", oc.ID) + } + assert.Equal(t, "a", outcomes[0].ID) + assert.Equal(t, "b", outcomes[1].ID) + assert.Equal(t, "c", outcomes[2].ID) assert.Equal(t, 3, svc.calls) } func TestRunMultiDelete_KeepGoingOnError(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{errFor: map[string]error{"doc_b": errors.New("not found")}} - res, err := runMultiDelete( + outcomes, err := cmdutil.RunBatch( context.Background(), - &DeleteOptions{Yes: true}, - &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, - svc, - &testutil.ConfirmPrompter{Answer: true}, []string{"doc_a", "doc_b", "doc_c"}, + func(ctx context.Context, id string) error { + if err := svc.DeleteKnowledge(ctx, id); err != nil { + return cmdutil.WrapHTTP(err, "delete document %s", id) + } + return nil + }, ) require.Error(t, err, "partial failure must return non-nil error (exit 1)") assert.Equal(t, 3, svc.calls, "all ids must be attempted (keep-going)") - require.Len(t, res.OK, 2) - require.Len(t, res.Failed, 1) - assert.Equal(t, "doc_b", res.Failed[0].ID) - assert.Equal(t, "not found", res.Failed[0].Message) - // OK list must contain only successful ids - assert.Contains(t, res.OK, "doc_a") - assert.Contains(t, res.OK, "doc_c") + require.Len(t, outcomes, 3) + // order preserved: doc_a ok, doc_b fail, doc_c ok + assert.Equal(t, "doc_a", outcomes[0].ID) + assert.Nil(t, outcomes[0].Err) + assert.Equal(t, "doc_b", outcomes[1].ID) + assert.NotNil(t, outcomes[1].Err) + assert.Equal(t, "doc_c", outcomes[2].ID) + assert.Nil(t, outcomes[2].Err) } func TestRunMultiDelete_AllFail(t *testing.T) { @@ -201,30 +238,27 @@ func TestRunMultiDelete_AllFail(t *testing.T) { "x": errors.New("HTTP error 404: not found"), "y": errors.New("HTTP error 403: forbidden"), }} - res, err := runMultiDelete( + outcomes, err := cmdutil.RunBatch( context.Background(), - &DeleteOptions{Yes: true}, - &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, - svc, - &testutil.ConfirmPrompter{Answer: true}, []string{"x", "y"}, + func(ctx context.Context, id string) error { + if err := svc.DeleteKnowledge(ctx, id); err != nil { + return cmdutil.WrapHTTP(err, "delete document %s", id) + } + return nil + }, ) require.Error(t, err) - assert.Empty(t, res.OK) - assert.Len(t, res.Failed, 2) + require.Len(t, outcomes, 2) + assert.NotNil(t, outcomes[0].Err) + assert.NotNil(t, outcomes[1].Err) } func TestRunMultiDelete_ConfirmBatch_NonTTY_RequiresConfirmation(t *testing.T) { _, _ = iostreams.SetForTest(t) // non-TTY svc := &fakeDeleteSvc{} - _, err := runMultiDelete( - context.Background(), - &DeleteOptions{Yes: false}, - &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, - svc, - &testutil.ConfirmPrompter{Answer: false}, - []string{"a", "b"}, - ) + fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} + err := cmdutil.ConfirmDestructiveBatch(&testutil.ConfirmPrompter{Answer: false}, false, fopts.WantsJSON(), "document", 2, "doc.delete", "") require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -235,14 +269,8 @@ func TestRunMultiDelete_ConfirmBatch_NonTTY_RequiresConfirmation(t *testing.T) { func TestRunMultiDelete_ConfirmBatch_TTY_UserAborts(t *testing.T) { _, errBuf := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - _, err := runMultiDelete( - context.Background(), - &DeleteOptions{Yes: false}, - &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, - svc, - &testutil.ConfirmPrompter{Answer: false}, - []string{"a", "b", "c"}, - ) + fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman} + err := cmdutil.ConfirmDestructiveBatch(&testutil.ConfirmPrompter{Answer: false}, false, fopts.WantsJSON(), "document", 3, "doc.delete", "") require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -252,43 +280,83 @@ func TestRunMultiDelete_ConfirmBatch_TTY_UserAborts(t *testing.T) { } // --------------------------------------------------------------------------- -// Emit tests +// Emit tests — JSON path now emits batch envelope (§4.5) // --------------------------------------------------------------------------- +// batchEnvelope is a minimal struct for parsing the batch envelope shape. +type batchEnvelope struct { + OK bool `json:"ok"` + Data []batchItem `json:"data"` + Meta batchMeta `json:"meta"` +} + +type batchItem struct { + ID string `json:"id"` + OK bool `json:"ok"` + Result json.RawMessage `json:"result,omitempty"` + Error *batchItemError `json:"error,omitempty"` +} + +type batchItemError struct { + Type string `json:"type"` + Message string `json:"message"` +} + +type batchMeta struct { + Count int `json:"count"` + Successes int `json:"successes"` + Failures int `json:"failures"` +} + func TestEmitMultiDelete_JSON(t *testing.T) { var buf bytes.Buffer - res := &MultiDeleteResult{ - OK: []string{"a", "b"}, - Failed: []FailedItem{{ID: "c", Message: "x"}}, + outcomes := []cmdutil.BatchOutcome{ + {ID: "a", Err: nil}, + {ID: "b", Err: nil}, + {ID: "c", Err: errors.New("x")}, } - err := emitMultiDelete(res, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, &buf) + err := cmdutil.EmitBatch(outcomes, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, &buf, cmdutil.DeletedAtNow) require.NoError(t, err) - var got MultiDeleteResult + var got batchEnvelope require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) - assert.Equal(t, []string{"a", "b"}, got.OK) - require.Len(t, got.Failed, 1) - assert.Equal(t, "c", got.Failed[0].ID) + // top-level: partial failure → ok:false + assert.False(t, got.OK) + assert.Equal(t, 3, got.Meta.Count) + assert.Equal(t, 2, got.Meta.Successes) + assert.Equal(t, 1, got.Meta.Failures) + require.Len(t, got.Data, 3) + assert.Equal(t, "a", got.Data[0].ID) + assert.True(t, got.Data[0].OK) + assert.Equal(t, "b", got.Data[1].ID) + assert.True(t, got.Data[1].OK) + assert.Equal(t, "c", got.Data[2].ID) + assert.False(t, got.Data[2].OK) + assert.NotNil(t, got.Data[2].Error) } func TestEmitMultiDelete_Text(t *testing.T) { var buf bytes.Buffer - res := &MultiDeleteResult{ - OK: []string{"a"}, - Failed: []FailedItem{{ID: "b", Message: "boom"}}, + outcomes := []cmdutil.BatchOutcome{ + {ID: "a", Err: nil}, + {ID: "b", Err: errors.New("boom")}, } - err := emitMultiDelete(res, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, &buf) + err := cmdutil.EmitBatch(outcomes, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, &buf, nil) require.NoError(t, err) out := buf.String() assert.Contains(t, out, "OK a") - assert.Contains(t, out, "FAIL b: boom") + assert.Contains(t, out, "FAIL b:") + assert.Contains(t, out, "boom") } func TestEmitMultiDelete_TextEmpty(t *testing.T) { var buf bytes.Buffer - res := &MultiDeleteResult{OK: []string{"x", "y"}} - err := emitMultiDelete(res, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, &buf) + outcomes := []cmdutil.BatchOutcome{ + {ID: "x", Err: nil}, + {ID: "y", Err: nil}, + } + err := cmdutil.EmitBatch(outcomes, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, &buf, nil) require.NoError(t, err) out := buf.String() @@ -298,9 +366,164 @@ func TestEmitMultiDelete_TextEmpty(t *testing.T) { } func TestEmitMultiDelete_UnsupportedFormat(t *testing.T) { + // EmitBatch defers unsupported format handling to WriteBatchEnvelope; for + // non-JSON formats it falls through to human text. Verify it does not error. var buf bytes.Buffer - res := &MultiDeleteResult{} - err := emitMultiDelete(res, &cmdutil.FormatOptions{Mode: "yaml"}, &buf) - require.Error(t, err) - assert.Contains(t, err.Error(), "yaml") + outcomes := []cmdutil.BatchOutcome{} + err := cmdutil.EmitBatch(outcomes, &cmdutil.FormatOptions{Mode: "yaml"}, &buf, nil) + // EmitBatch itself does not error on unknown mode (it uses WantsJSON gate) + require.NoError(t, err) +} + +// --------------------------------------------------------------------------- +// Batch envelope shape test — Task 2.6 +// --------------------------------------------------------------------------- + +// TestDocDelete_MultiID_PartialFailure_BatchEnvelope verifies that when a +// multi-id delete has a partial failure, stdout carries the §4.5 batch +// envelope shape: ok:false, data:[BatchItem...], meta:{count, successes, +// failures}. Order follows original argv order. +func TestDocDelete_MultiID_PartialFailure_BatchEnvelope(t *testing.T) { + // id1 succeeds, id2 fails, id3 succeeds. + svc := &fakeDeleteSvc{errFor: map[string]error{ + "id2": errors.New("HTTP error 404: not found"), + }} + outcomes, runErr := cmdutil.RunBatch( + context.Background(), + []string{"id1", "id2", "id3"}, + func(ctx context.Context, id string) error { + if err := svc.DeleteKnowledge(ctx, id); err != nil { + return cmdutil.WrapHTTP(err, "delete document %s", id) + } + return nil + }, + ) + require.Error(t, runErr, "partial failure must return non-nil error") + require.Len(t, outcomes, 3) + + var buf bytes.Buffer + require.NoError(t, cmdutil.EmitBatch(outcomes, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON, TTY: false}, &buf, cmdutil.DeletedAtNow)) + + var env batchEnvelope + require.NoError(t, json.Unmarshal(buf.Bytes(), &env)) + + // top-level ok:false (partial failure) + assert.False(t, env.OK) + + // meta counts + assert.Equal(t, 3, env.Meta.Count) + assert.Equal(t, 2, env.Meta.Successes) + assert.Equal(t, 1, env.Meta.Failures) + + // items in argv order + require.Len(t, env.Data, 3) + + assert.Equal(t, "id1", env.Data[0].ID) + assert.True(t, env.Data[0].OK) + assert.Nil(t, env.Data[0].Error) + + assert.Equal(t, "id2", env.Data[1].ID) + assert.False(t, env.Data[1].OK) + require.NotNil(t, env.Data[1].Error) + assert.Equal(t, "resource.not_found", env.Data[1].Error.Type) + + assert.Equal(t, "id3", env.Data[2].ID) + assert.True(t, env.Data[2].OK) + assert.Nil(t, env.Data[2].Error) +} + +// --------------------------------------------------------------------------- +// --all mode tests (runDeleteAll) +// --------------------------------------------------------------------------- + +// TestDocDelete_All_MissingKB_ReturnsFlagError: --all without --kb must exit 2. +func TestDocDelete_All_MissingKB_ReturnsFlagError(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeAllSvc{} + // Simulate the RunE guard: --all without --kb returns FlagError before + // runDeleteAll is ever called. + err := cmdutil.NewFlagError(fmt.Errorf("--all requires --kb=<id>")) + require.Error(t, err) + var flagErr *cmdutil.FlagError + require.ErrorAs(t, err, &flagErr) + assert.Equal(t, 2, cmdutil.ExitCode(err)) + assert.False(t, svc.called) +} + +// TestDocDelete_All_WithoutYes_JSONMode_ReturnsExit10 verifies the +// CodeInputConfirmationRequired (exit 10) path with risk metadata in JSON/non-TTY. +func TestDocDelete_All_WithoutYes_JSONMode_ReturnsExit10(t *testing.T) { + _, _ = iostreams.SetForTest(t) // non-TTY + svc := &fakeAllSvc{} + opts := &DeleteOptions{All: true, KB: "kb_x", Yes: false} + err := runDeleteAll(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, &testutil.ConfirmPrompter{}) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputConfirmationRequired, typed.Code) + assert.Equal(t, 10, cmdutil.ExitCode(err)) + require.NotNil(t, typed.Risk) + assert.Equal(t, "destructive", typed.Risk.Level) + assert.Equal(t, "doc.delete_all", typed.Risk.Action) + assert.False(t, svc.called) +} + +// TestDocDelete_All_WithYes_CallsClearKB verifies that with -y the call is made +// and the JSON envelope contains kb_id + deleted_count. +func TestDocDelete_All_WithYes_CallsClearKB(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeAllSvc{resp: &sdk.ClearKnowledgeBaseContentsResponse{DeletedCount: 17}} + opts := &DeleteOptions{All: true, KB: "kb_x", Yes: true} + err := runDeleteAll(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, &testutil.ConfirmPrompter{}) + require.NoError(t, err) + assert.True(t, svc.called) + assert.Equal(t, "kb_x", svc.gotID) + + var env struct { + OK bool `json:"ok"` + Data map[string]any `json:"data"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env), "expected valid JSON envelope, got %q", out.String()) + assert.True(t, env.OK) + assert.Equal(t, "kb_x", env.Data["kb_id"]) + assert.Equal(t, float64(17), env.Data["deleted_count"]) +} + +// TestDocDelete_All_WithYes_TextMode verifies the text output path. +func TestDocDelete_All_WithYes_TextMode(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeAllSvc{resp: &sdk.ClearKnowledgeBaseContentsResponse{DeletedCount: 5}} + opts := &DeleteOptions{All: true, KB: "kb_y", Yes: true} + err := runDeleteAll(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{}) + require.NoError(t, err) + assert.True(t, svc.called) + body := out.String() + assert.Contains(t, body, "5") + assert.Contains(t, body, "kb_y") +} + +// TestDocDelete_All_TTY_UserAborts: interactive TTY + user says no → CodeUserAborted. +func TestDocDelete_All_TTY_UserAborts(t *testing.T) { + _, errBuf := iostreams.SetForTestWithTTY(t) + svc := &fakeAllSvc{} + opts := &DeleteOptions{All: true, KB: "kb_z", Yes: false} + err := runDeleteAll(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{Answer: false}) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeUserAborted, typed.Code) + assert.False(t, svc.called) + assert.Contains(t, errBuf.String(), "Aborted") +} + +// TestDocDelete_All_ServiceError propagates SDK errors via WrapHTTP. +func TestDocDelete_All_ServiceError(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeAllSvc{err: errors.New("HTTP error 404: not found")} + opts := &DeleteOptions{All: true, KB: "kb_missing", Yes: true} + err := runDeleteAll(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, &testutil.ConfirmPrompter{}) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeResourceNotFound, typed.Code) } diff --git a/cli/cmd/doc/doc.go b/cli/cmd/doc/doc.go index 38a4e9e1..57fe0c0b 100644 --- a/cli/cmd/doc/doc.go +++ b/cli/cmd/doc/doc.go @@ -1,6 +1,7 @@ // Package doc implements the `weknora doc` subtree (list / view / upload / -// download / delete). Upload also supports --recursive / --glob for bulk -// ingestion. +// fetch / create / download / delete / wait). Upload supports --recursive / +// --glob for bulk ingestion from local files. Fetch ingests a remote URL. +// Create adds a knowledge entry from inline text content. // // "Doc" is the CLI noun; the underlying SDK type is `Knowledge`. The renaming // is deliberate: end-users think of a knowledge entry as the document they @@ -19,14 +20,14 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "doc", Short: "Manage documents in a knowledge base", - Args: cobra.NoArgs, - Run: func(c *cobra.Command, _ []string) { _ = c.Help() }, } - cmd.AddCommand(NewCmdList(f)) - cmd.AddCommand(NewCmdView(f)) - cmd.AddCommand(NewCmdUpload(f)) - cmd.AddCommand(NewCmdDownload(f)) + cmd.AddCommand(NewCmdCreate(f)) cmd.AddCommand(NewCmdDelete(f)) + cmd.AddCommand(NewCmdDownload(f)) + cmd.AddCommand(NewCmdFetch(f)) + cmd.AddCommand(NewCmdList(f)) + cmd.AddCommand(NewCmdUpload(f)) + cmd.AddCommand(NewCmdView(f)) cmd.AddCommand(NewCmdWait(f)) return cmd } diff --git a/cli/cmd/doc/fetch.go b/cli/cmd/doc/fetch.go new file mode 100644 index 00000000..e66b6bd8 --- /dev/null +++ b/cli/cmd/doc/fetch.go @@ -0,0 +1,140 @@ +// Package doc — fetch.go implements `weknora doc fetch <url>`. +package doc + +import ( + "cmp" + "context" + "errors" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// docFetchFields enumerates the fields surfaced for `--format json` discovery +// on `doc fetch`. +var docFetchFields = []string{ + "id", "knowledge_base_id", "tag_id", "type", "title", "description", + "source", "channel", "parse_status", "summary_status", "enable_status", + "embedding_model_id", "file_name", "file_type", "file_size", "file_hash", + "file_path", "storage_size", + "created_at", "updated_at", "processed_at", "error_message", +} + +// FetchOptions holds CLI flag values for `doc fetch`. +type FetchOptions struct { + URL string + Name string // --name: FileName hint for file-vs-crawl mode detection + Title string // --title: display title for the new entry + FileType string // --file-type: extension hint for extension-less URLs + TagID string // --tag-id: associate the new entry with a tag + Channel string // --channel: ingestion-channel tag (default "api") + EnableMultimodel *bool // tri-state: nil = server default +} + +// FetchService is the narrow SDK surface for `doc fetch`. +// *sdk.Client satisfies it. +type FetchService interface { + CreateKnowledgeFromURL( + ctx context.Context, + kbID string, + req sdk.CreateKnowledgeFromURLRequest, + ) (*sdk.Knowledge, error) +} + +// NewCmdFetch builds `weknora doc fetch <url>`. +func NewCmdFetch(f *cmdutil.Factory) *cobra.Command { + opts := &FetchOptions{} + cmd := &cobra.Command{ + Use: "fetch <url>", + Short: "Fetch a remote document into a knowledge base", + Long: `Server fetches the document at the given URL and ingests it into the resolved +knowledge base. KB resolution follows the standard 4-level chain: +--kb flag > WEKNORA_KB_ID env > .weknora/project.yaml > error. + +When the URL has a known file extension (.pdf, .docx, .md, .txt) the server +automatically switches from web-page-crawl mode to file-download mode. Pass +--file-type or --name with a recognisable extension to force file-download mode +for extension-less URLs. + +Server-side ingestion knobs: + + --name <name> Override the recorded file name; also used as the + file-type hint when the extension is recognisable. + --title <title> Set the display title stored with the entry. + --file-type <ext> Explicit file-type hint (e.g. "pdf") for URLs + without an extension. + --tag-id <id> Associate the new entry with a tag. + --enable-multimodel Toggle multimodal extraction (image-in-PDF → text). + Unset ⇒ server default; pass true or false to override. + --channel <name> Override the ingestion-channel tag (default "api").`, + Example: ` weknora doc fetch https://example.com/whitepaper.pdf + weknora doc fetch https://example.com/no-ext --file-type pdf --title "Whitepaper" + weknora doc fetch https://example.com/article.html --name "Q3 Article" --tag-id tag_abc + weknora doc fetch https://example.com/report.pdf --kb my-kb --enable-multimodel`, + Args: cobra.ExactArgs(1), + RunE: func(c *cobra.Command, args []string) error { + opts.URL = args[0] + fopts, err := cmdutil.CheckFormatFlag(c) + if err != nil { + return err + } + fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) + // Tri-state --enable-multimodel: nil = unset (server default). + if c.Flags().Changed("enable-multimodel") { + raw, _ := c.Flags().GetString("enable-multimodel") + v, perr := parseTriBool(raw) + if perr != nil { + return perr + } + opts.EnableMultimodel = &v + } + if err := cmdutil.ValidateHTTPURL("<url>", opts.URL); err != nil { + return err + } + kbID, err := f.ResolveKB(c) + if err != nil { + return err + } + cli, err := f.Client() + if err != nil { + return err + } + return runFetch(c.Context(), opts, fopts, cli, kbID) + }, + } + cmd.Flags().String("kb", "", "Knowledge base UUID or name (overrides env / project link)") + cmd.Flags().StringVar(&opts.Name, "name", "", "File name hint (also used as file-type hint when extension is recognisable)") + cmd.Flags().StringVar(&opts.Title, "title", "", "Display title for the new entry") + cmd.Flags().StringVar(&opts.FileType, "file-type", "", "File-type hint such as \"pdf\" when the URL has no extension") + cmd.Flags().StringVar(&opts.TagID, "tag-id", "", "Tag id to associate with the new entry") + cmd.Flags().StringVar(&opts.Channel, "channel", "", "Ingestion-channel tag recorded server-side (default \"api\")") + cmd.Flags().String("enable-multimodel", "", "Toggle multimodal extraction (true|false); unset ⇒ server default") + cmd.Flags().Lookup("enable-multimodel").NoOptDefVal = "true" + cmdutil.AddFormatFlag(cmd, docFetchFields...) + return cmd +} + +// runFetch ingests a remote URL via SDK CreateKnowledgeFromURL. +func runFetch(ctx context.Context, opts *FetchOptions, fopts *cmdutil.FormatOptions, svc FetchService, kbID string) error { + req := sdk.CreateKnowledgeFromURLRequest{ + URL: opts.URL, + FileName: opts.Name, + FileType: opts.FileType, + EnableMultimodel: opts.EnableMultimodel, + Title: opts.Title, + TagID: opts.TagID, + Channel: cmp.Or(opts.Channel, uploadChannel), + } + k, err := svc.CreateKnowledgeFromURL(ctx, kbID, req) + if err != nil { + if errors.Is(err, sdk.ErrDuplicateURL) { + return cmdutil.Wrapf(cmdutil.CodeResourceAlreadyExists, err, + "URL already ingested into this knowledge base") + } + return cmdutil.WrapHTTP(err, "fetch document from %s", opts.URL) + } + return renderUploadSuccess(k, fopts, "Ingested", opts.Name, opts.URL) +} diff --git a/cli/cmd/doc/fetch_test.go b/cli/cmd/doc/fetch_test.go new file mode 100644 index 00000000..b4ae475a --- /dev/null +++ b/cli/cmd/doc/fetch_test.go @@ -0,0 +1,161 @@ +package doc + +import ( + "context" + "encoding/json" + "errors" + "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" +) + +// fakeFetchSvc captures call arguments and returns canned responses. +type fakeFetchSvc struct { + resp *sdk.Knowledge + err error + got struct { + kbID string + req sdk.CreateKnowledgeFromURLRequest + } +} + +func (f *fakeFetchSvc) CreateKnowledgeFromURL( + _ context.Context, + kbID string, + req sdk.CreateKnowledgeFromURLRequest, +) (*sdk.Knowledge, error) { + f.got.kbID = kbID + f.got.req = req + return f.resp, f.err +} + +func TestFetch_Success_Human(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_url_1", FileName: "whitepaper.pdf"}} + opts := &FetchOptions{URL: "https://example.com/whitepaper.pdf"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + + assert.Equal(t, "kb_xxx", svc.got.kbID) + assert.Equal(t, "https://example.com/whitepaper.pdf", svc.got.req.URL) + assert.Equal(t, "api", svc.got.req.Channel) + assert.Contains(t, out.String(), "Ingested") + assert.Contains(t, out.String(), "doc_url_1") +} + +func TestFetch_WithName_PassesAsFileName(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_url_2"}} + opts := &FetchOptions{URL: "https://example.com/article.html", Name: "Q3 Article"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "Q3 Article", svc.got.req.FileName, + "--name must be forwarded as FileName (server uses it for file-vs-crawl mode hint)") +} + +func TestFetch_Title(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_u"}} + opts := &FetchOptions{URL: "https://example.com/a.pdf", Title: "My Title"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "My Title", svc.got.req.Title) +} + +func TestFetch_FileType(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_u"}} + opts := &FetchOptions{URL: "https://example.com/no-ext", FileType: "pdf"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "pdf", svc.got.req.FileType) +} + +func TestFetch_TagID(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_u"}} + opts := &FetchOptions{URL: "https://example.com/a.pdf", TagID: "tag_99"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "tag_99", svc.got.req.TagID) +} + +func TestFetch_EnableMultimodel_Forwarded(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_u"}} + mm := true + opts := &FetchOptions{URL: "https://example.com/a.pdf", EnableMultimodel: &mm} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + require.NotNil(t, svc.got.req.EnableMultimodel) + assert.True(t, *svc.got.req.EnableMultimodel) +} + +func TestFetch_Channel_Override(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_u"}} + opts := &FetchOptions{URL: "https://example.com/a.pdf", Channel: "web"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, "web", svc.got.req.Channel) +} + +func TestFetch_Channel_DefaultIsAPI(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_u"}} + opts := &FetchOptions{URL: "https://example.com/a.pdf"} + require.NoError(t, runFetch(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx")) + assert.Equal(t, uploadChannel, svc.got.req.Channel) +} + +func TestFetch_JSON_Envelope(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeFetchSvc{resp: &sdk.Knowledge{ID: "doc_url_3", FileName: "ok.pdf"}} + fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} + require.NoError(t, runFetch(context.Background(), + &FetchOptions{URL: "https://example.com/ok.pdf"}, fopts, svc, "kb_xxx")) + got := out.String() + var env struct { + OK bool `json:"ok"` + Data sdk.Knowledge `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(got), &env), "expected valid JSON envelope, got %q", got) + assert.True(t, env.OK, "envelope.ok must be true") + assert.Equal(t, "doc_url_3", env.Data.ID, "envelope.data.id must be doc_url_3") + assert.NotContains(t, got, `"risk":`) +} + +func TestFetch_DuplicateURL_Maps_resource_already_exists(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{ + resp: &sdk.Knowledge{ID: "doc_existing"}, + err: sdk.ErrDuplicateURL, + } + err := runFetch(context.Background(), + &FetchOptions{URL: "https://example.com/dup.pdf"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeResourceAlreadyExists, typed.Code) +} + +func TestFetch_ServerError_Wraps(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{err: errors.New("HTTP error 500: internal server error")} + err := runFetch(context.Background(), + &FetchOptions{URL: "https://example.com/doc.pdf"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeServerError, typed.Code) +} + +func TestFetch_KBResolutionFailure_Propagates(t *testing.T) { + // runFetch itself receives a pre-resolved kbID; KB resolution failure + // happens in RunE before runFetch is called. This test verifies that + // runFetch does NOT swallow an error returned from the service when + // kbID is empty (which a broken resolution chain might produce). + _, _ = iostreams.SetForTest(t) + svc := &fakeFetchSvc{err: errors.New("HTTP error 404: knowledge base not found")} + err := runFetch(context.Background(), + &FetchOptions{URL: "https://example.com/doc.pdf"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "") + require.Error(t, err) +} diff --git a/cli/cmd/doc/upload.go b/cli/cmd/doc/upload.go index 654e93b2..d2df7057 100644 --- a/cli/cmd/doc/upload.go +++ b/cli/cmd/doc/upload.go @@ -1,6 +1,7 @@ package doc import ( + "cmp" "context" "errors" "fmt" @@ -35,7 +36,6 @@ type UploadOptions struct { Name string Recursive bool // --recursive: positional arg is a directory; walk + upload each match Glob string // --glob: filename pattern under --recursive (default "*") - FromURL string // --from-url: ingest a remote URL via SDK CreateKnowledgeFromURL // EnableMultimodel toggles server-side multimodal extraction // (e.g. images-in-PDF → OCR'd text). nil means "server default" - @@ -49,12 +49,6 @@ type UploadOptions struct { // Channel overrides the ingestion-channel tag recorded server-side. // Empty ⇒ uploadChannel ("api"). Free-form: server validates. Channel string - - // URL-mode only fields. RunE-side validation rejects these if - // --from-url is not set (positional file path or --recursive). - Title string // --title: display title (URL mode) - FileType string // --file-type: extension hint for extension-less URLs - TagID string // --tag-id: associate the new knowledge entry with a tag } // UploadService is the narrow SDK surface this command depends on. @@ -67,11 +61,6 @@ type UploadService interface { enableMultimodel *bool, customFileName, channel string, ) (*sdk.Knowledge, error) - CreateKnowledgeFromURL( - ctx context.Context, - kbID string, - req sdk.CreateKnowledgeFromURLRequest, - ) (*sdk.Knowledge, error) } // NewCmdUpload builds `weknora doc upload <file>`. @@ -89,35 +78,28 @@ Pass --name to override the recorded file name (useful when the local file has a generic name like "report.pdf" but you want to surface it as e.g. "Q3 Marketing Report.pdf" in the UI). -The three input modes (positional file / --recursive directory walk / ---from-url remote ingest) are mutually exclusive - pass exactly one. -Use --recursive --glob to upload a directory tree (see Examples). +The two input modes (positional file / --recursive directory walk) are +mutually exclusive - pass exactly one. Use --recursive --glob to upload a +directory tree (see Examples). To ingest a remote URL use "weknora doc fetch"; +to create an entry from inline text use "weknora doc create". Server-side ingestion knobs: --enable-multimodel Toggle multimodal extraction (image-in-PDF → text). Unset ⇒ server default; pass true or false to override. - Applies to file / --recursive / --from-url. + Applies to file / --recursive. --metadata key=value Attach arbitrary key/value metadata. Repeatable. Empty value allowed; duplicate keys ⇒ last-wins. Malformed values (no '=', empty key) ⇒ - input.invalid_argument. File and --recursive modes - only; rejected on --from-url because the URL-ingest - request type carries no metadata field. + input.invalid_argument. --channel <name> Override the ingestion-channel tag (default "api"). - Applies to file / --recursive / --from-url. - -URL mode (--from-url) additionally accepts --title, --file-type, and --tag-id. -Passing any of those without --from-url is rejected as input.invalid_argument.`, + Applies to file / --recursive.`, Example: ` weknora doc upload report.pdf weknora doc upload notes.md --kb a32a63ff-fb36-4874-bcaa-30f48570a694 weknora doc upload notes.md --kb my-kb weknora doc upload q3.pdf --name "Q3 Marketing Report.pdf" weknora doc upload report.pdf --enable-multimodel --metadata team=alpha --metadata sprint=Q4 - weknora doc upload ./docs --recursive --glob '*.pdf' --metadata team=alpha - weknora doc upload --from-url https://example.com/whitepaper.pdf - weknora doc upload --from-url https://example.com/no-ext --file-type pdf --title "Whitepaper" - weknora doc upload --from-url https://example.com/article.html --name "Q3 Article" --tag-id tag_abc`, + weknora doc upload ./docs --recursive --glob '*.pdf' --metadata team=alpha`, Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, args []string) error { fopts, err := cmdutil.CheckFormatFlag(c) @@ -149,33 +131,25 @@ Passing any of those without --from-url is rejected as input.invalid_argument.`, return err } - switch { - case opts.FromURL != "": - return runUploadFromURL(c.Context(), opts, fopts, cli, kbID) - case opts.Recursive: + if opts.Recursive { return runUploadRecursive(c.Context(), opts, fopts, cli, kbID, args[0]) - default: - if err := validateUploadPath(args[0]); err != nil { - return err - } - return runUpload(c.Context(), opts, fopts, cli, kbID, args[0]) } + if err := validateUploadPath(args[0]); err != nil { + return err + } + return runUpload(c.Context(), opts, fopts, cli, kbID, args[0]) }, } cmd.Flags().String("kb", "", "Knowledge base UUID or name (overrides env / project link)") cmd.Flags().StringVar(&opts.Name, "name", "", "Custom file name to record (defaults to base name)") cmd.Flags().BoolVar(&opts.Recursive, "recursive", false, "Treat the positional argument as a directory to walk") cmd.Flags().StringVar(&opts.Glob, "glob", "*", "Filename pattern to filter when --recursive (e.g. '*.pdf')") - cmd.Flags().StringVar(&opts.FromURL, "from-url", "", "Ingest a remote `URL` (HTTP/HTTPS) instead of a local file") // Tri-state flag: unset ⇒ server default, "true"/"false" override. The // raw string is decoded into opts.EnableMultimodel in RunE. cmd.Flags().String("enable-multimodel", "", "Toggle multimodal extraction (true|false); unset ⇒ server default") cmd.Flags().Lookup("enable-multimodel").NoOptDefVal = "true" cmd.Flags().StringSliceVar(&opts.Metadata, "metadata", nil, "Attach metadata `key=value` (repeatable; empty value allowed, last-wins on duplicate keys)") cmd.Flags().StringVar(&opts.Channel, "channel", "", "Ingestion-channel tag recorded server-side (default \"api\")") - cmd.Flags().StringVar(&opts.Title, "title", "", "Display title for the new entry (--from-url only)") - cmd.Flags().StringVar(&opts.FileType, "file-type", "", "File-type hint such as \"pdf\" when the URL has no extension (--from-url only)") - cmd.Flags().StringVar(&opts.TagID, "tag-id", "", "Tag id to associate with the new entry (--from-url only)") cmdutil.AddFormatFlag(cmd, docUploadFields...) return cmd } @@ -217,105 +191,21 @@ func parseMetadataKV(raw []string) (map[string]string, error) { return out, nil } -// effectiveChannel returns the channel string to send to the SDK. Empty -// opts.Channel falls back to the default "api" so the wire payload is -// identical to the pre-flag behavior. -func effectiveChannel(opts *UploadOptions) string { - if opts.Channel != "" { - return opts.Channel - } - return uploadChannel -} - -// validateUploadFlags enforces mutual exclusion between the three input -// modes (positional file path / --recursive directory walk / --from-url -// remote ingest) and validates the URL when --from-url is set. It also -// rejects the URL-mode-only flags (--title, --file-type, --tag-id) when -// --from-url isn't set so misuse fails fast with a typed code. +// validateUploadFlags enforces mutual exclusion between the two input modes +// (positional file path / --recursive directory walk) and requires that at +// least one input mode is supplied. func validateUploadFlags(opts *UploadOptions, args []string) error { hasPath := len(args) == 1 - hasURL := opts.FromURL != "" - if hasURL { - if hasPath { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, - "cannot pass a file path with --from-url; choose one input mode") - } - if opts.Recursive { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, - "--recursive cannot be combined with --from-url") - } - // The server's URL-ingest request type has no Metadata field; a - // --metadata pair would be silently dropped on the wire. Reject - // up-front so callers don't think they've set metadata when they - // haven't. - if len(opts.Metadata) > 0 { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, - "--metadata is not supported with --from-url (server URL-ingest has no metadata field)") - } - return cmdutil.ValidateHTTPURL("--from-url", opts.FromURL) - } if !hasPath { // Wrap as FlagError so the exit code (2) matches what cobra's own // MinimumNArgs(1) would emit — consistent with every other command // that requires a positional argument. return cmdutil.NewFlagError(errors.New( - "a file path is required (or pass --from-url)")) - } - return rejectURLOnlyFlags(opts) -} - -// rejectURLOnlyFlags errors on --title / --file-type / --tag-id when -// --from-url is NOT set. Shared between validateUploadFlags (file mode) -// and runUploadRecursive (directory walk) so a future URL-mode-only flag -// only needs to add one entry here instead of two parallel checks. -func rejectURLOnlyFlags(opts *UploadOptions) error { - if opts.Title != "" { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, - "--title is only valid with --from-url") - } - if opts.FileType != "" { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, - "--file-type is only valid with --from-url") - } - if opts.TagID != "" { - return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, - "--tag-id is only valid with --from-url") + "a file path is required (or use `weknora doc fetch` for URLs, `weknora doc create` for inline text)")) } return nil } -// runUploadFromURL ingests a remote URL via SDK CreateKnowledgeFromURL. -// `--name` becomes the FileName hint so the server's "known file extension" -// detection upgrades crawl-mode to file-download-mode when appropriate. -// Server-side knobs (--enable-multimodel, --metadata via Title/TagID/FileType) -// propagate when set; the SDK request struct omits empty fields via -// `json:",omitempty"` tags so wire payload stays minimal. -func runUploadFromURL(ctx context.Context, opts *UploadOptions, fopts *cmdutil.FormatOptions, svc UploadService, kbID string) error { - req := sdk.CreateKnowledgeFromURLRequest{ - URL: opts.FromURL, - FileName: opts.Name, - FileType: opts.FileType, - EnableMultimodel: opts.EnableMultimodel, - Title: opts.Title, - TagID: opts.TagID, - Channel: effectiveChannel(opts), - } - k, err := svc.CreateKnowledgeFromURL(ctx, kbID, req) - if err != nil { - if errors.Is(err, sdk.ErrDuplicateURL) { - // Server returns 409 with the existing knowledge entry's data. - // Surface as resource.already_exists; the data payload (if any) - // is observable via err's wrap chain - but the typed code is - // what agents branch on. - return cmdutil.Wrapf(cmdutil.CodeResourceAlreadyExists, err, - "URL already ingested into this knowledge base") - } - return cmdutil.WrapHTTP(err, "ingest URL %s", opts.FromURL) - } - - return renderUploadSuccess(k, fopts, "Ingested", opts.Name, opts.FromURL) -} - // renderUploadSuccess emits the post-upload result. JSON path is the bare // Knowledge object; human path prints a checkmark line. Shared by single- // file upload and URL ingest; humanVerb varies (uploaded/ingested) and @@ -323,7 +213,7 @@ func runUploadFromURL(ctx context.Context, opts *UploadOptions, fopts *cmdutil.F // blank (URL ingest pre-redirect). func renderUploadSuccess(k *sdk.Knowledge, fopts *cmdutil.FormatOptions, humanVerb, customName, fallbackDisplay string) error { if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, k) + return fopts.Emit(iostreams.IO.Out, k, nil) } displayed := customName if displayed == "" { @@ -362,7 +252,7 @@ func runUpload(ctx context.Context, opts *UploadOptions, fopts *cmdutil.FormatOp if err != nil { return err } - k, err := svc.CreateKnowledgeFromFile(ctx, kbID, path, meta, opts.EnableMultimodel, opts.Name, effectiveChannel(opts)) + k, err := svc.CreateKnowledgeFromFile(ctx, kbID, path, meta, opts.EnableMultimodel, opts.Name, cmp.Or(opts.Channel, uploadChannel)) if err != nil { if errors.Is(err, sdk.ErrDuplicateFile) { // SDK returns sentinel without an "HTTP error <status>:" prefix diff --git a/cli/cmd/doc/upload_recursive.go b/cli/cmd/doc/upload_recursive.go index b391e10a..97cf7f03 100644 --- a/cli/cmd/doc/upload_recursive.go +++ b/cli/cmd/doc/upload_recursive.go @@ -1,6 +1,7 @@ package doc import ( + "cmp" "context" "fmt" "io/fs" @@ -9,13 +10,14 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/output" ) -// uploadOutcome is one entry in the recursive upload's per-file report. -type uploadOutcome struct { - Path string `json:"path"` - ID string `json:"id,omitempty"` - Error string `json:"error,omitempty"` +// uploadedFile holds the server-side result for a successful per-file upload. +// Keyed by file path in the uploadResults map populated by the RunBatch closure. +type uploadedFile struct { + ID string + Name string } // runUploadRecursive walks dir, filters by Glob, and uploads each match @@ -32,12 +34,6 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, fopts *cmdutil Hint: "drop --name or upload files one at a time", } } - // URL-mode-only flags are not meaningful for a directory walk; - // rejectURLOnlyFlags is the single source of truth shared with - // file-mode upload. - if err := rejectURLOnlyFlags(opts); err != nil { - return err - } // Parse --metadata up front so a malformed value aborts before the // first SDK call - otherwise a typo in `key=value` would only surface // per-file as repeated identical errors. @@ -75,73 +71,81 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, fopts *cmdutil } if len(matches) == 0 { if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, recursiveResult{KBID: kbID}) + // Empty batch: emit an all-success envelope with zero items. + return output.WriteBatchEnvelope(iostreams.IO.Out, nil, fopts.TTY, cmdutil.GetProfile()) } fmt.Fprintf(iostreams.IO.Out, "(no files matched %q under %s)\n", opts.Glob, dir) return nil } - var uploaded, failed []uploadOutcome + // uploaded captures per-path server results for successful uploads. + // Populated by the RunBatch closure; read by the resultFn below. + uploaded := make(map[string]uploadedFile, len(matches)) var firstFailCode cmdutil.ErrorCode - channel := effectiveChannel(opts) - for _, p := range matches { + channel := cmp.Or(opts.Channel, uploadChannel) + + outcomes, runErr := cmdutil.RunBatch(ctx, matches, func(ctx context.Context, p string) error { k, err := svc.CreateKnowledgeFromFile(ctx, kbID, p, meta, opts.EnableMultimodel, "", channel) if err != nil { code := cmdutil.ClassifyHTTPError(err) if firstFailCode == "" { firstFailCode = code } - failed = append(failed, uploadOutcome{Path: p, Error: err.Error()}) // Per-file progress lines are human progress signal; suppress // under --format json so they don't precede the JSON object on stdout. if !fopts.WantsJSON() { fmt.Fprintf(iostreams.IO.Out, "FAIL %s: %v\n", filepath.Base(p), err) } - continue + return err } - id := "" + id, name := "", "" if k != nil { id = k.ID + name = k.FileName } - uploaded = append(uploaded, uploadOutcome{Path: p, ID: id}) + uploaded[p] = uploadedFile{ID: id, Name: name} if !fopts.WantsJSON() { fmt.Fprintf(iostreams.IO.Out, "OK %s (id: %s)\n", filepath.Base(p), id) } + return nil + }) + + failures := 0 + for _, oc := range outcomes { + if oc.Err != nil { + failures++ + } } if fopts.WantsJSON() { - result := recursiveResult{KBID: kbID, Uploaded: uploaded, Failed: failed} - if err := fopts.Emit(iostreams.IO.Out, result); err != nil { + if err := cmdutil.EmitBatch(outcomes, fopts, iostreams.IO.Out, func(p string) any { + f := uploaded[p] + return map[string]any{"id": f.ID, "name": f.Name} + }); err != nil { return err } } else { - fmt.Fprintf(iostreams.IO.Out, "Uploaded %d, Failed %d\n", len(uploaded), len(failed)) + fmt.Fprintf(iostreams.IO.Out, "Uploaded %d, Failed %d\n", len(outcomes)-failures, failures) } - if len(failed) > 0 { + if runErr != nil { // Silent on the --format json path: the success object above already - // carries per-file uploaded[]/failed[] detail; without Silent the - // root error handler would print to stderr in addition. ExitCode - // still walks Code so the typed exit-code-by-class contract holds. + // carries per-file detail; without Silent the root error handler would + // print to stderr in addition. ExitCode still walks Code so the typed + // exit-code-by-class contract holds. + code := firstFailCode + if code == "" { + code = cmdutil.ClassifyContextErr(ctx.Err()) + } return &cmdutil.Error{ - Code: firstFailCode, - Message: fmt.Sprintf("%d of %d uploads failed", len(failed), len(matches)), + Code: code, + Message: fmt.Sprintf("%d of %d uploads failed", failures, len(matches)), Silent: fopts.WantsJSON(), } } return nil } -// recursiveResult is the JSON shape emitted under data when --recursive is -// combined with --format json. Mirrors the human-mode per-file output: a list of -// successes (Uploaded) and a list of failures (Failed), each with the -// originating path so agents can re-try only the failed entries. -type recursiveResult struct { - KBID string `json:"kb_id"` - Uploaded []uploadOutcome `json:"uploaded,omitempty"` - Failed []uploadOutcome `json:"failed,omitempty"` -} - // walkMatches returns every regular file under root whose base name matches // pattern. Order is filepath.WalkDir's lexical order (stdlib guarantee on // every supported FS), which is deterministic for test assertions. diff --git a/cli/cmd/doc/upload_recursive_test.go b/cli/cmd/doc/upload_recursive_test.go index 4aa5851c..6cf1724a 100644 --- a/cli/cmd/doc/upload_recursive_test.go +++ b/cli/cmd/doc/upload_recursive_test.go @@ -2,6 +2,7 @@ package doc import ( "context" + "encoding/json" "errors" "os" "path/filepath" @@ -51,16 +52,6 @@ func (s *scriptedUploadSvc) CreateKnowledgeFromFile( return r.k, r.err } -// CreateKnowledgeFromURL satisfies UploadService but is unused by the -// recursive-walk path. Recursive upload only goes through CreateKnowledgeFromFile. -func (s *scriptedUploadSvc) CreateKnowledgeFromURL( - _ context.Context, - _ string, - _ sdk.CreateKnowledgeFromURLRequest, -) (*sdk.Knowledge, error) { - return nil, nil -} - func mkTree(t *testing.T, base string, names ...string) { t.Helper() for _, n := range names { @@ -77,7 +68,7 @@ func TestUploadRecursive_WalksAllFiles(t *testing.T) { svc := &scriptedUploadSvc{} opts := &UploadOptions{Recursive: true, Glob: "*"} - require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir)) + require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir)) sort.Strings(svc.called) assert.Equal(t, []string{"a.pdf", "b.pdf", "c.pdf"}, svc.called) @@ -94,7 +85,7 @@ func TestUploadRecursive_GlobFilter(t *testing.T) { svc := &scriptedUploadSvc{} opts := &UploadOptions{Recursive: true, Glob: "*.pdf"} - require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir)) + require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir)) sort.Strings(svc.called) assert.Equal(t, []string{"doc.pdf", "keep.pdf"}, svc.called) @@ -112,7 +103,7 @@ func TestUploadRecursive_PartialFailure_Exits1(t *testing.T) { "bad.pdf": {err: errors.New("HTTP error 500: internal")}, }} opts := &UploadOptions{Recursive: true, Glob: "*"} - err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir) + err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir) require.Error(t, err) var typed *cmdutil.Error @@ -135,7 +126,7 @@ func TestUploadRecursive_NoMatches(t *testing.T) { svc := &scriptedUploadSvc{} opts := &UploadOptions{Recursive: true, Glob: "*.pdf"} - require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir)) + require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir)) assert.Len(t, svc.called, 0) assert.Contains(t, strings.ToLower(out.String()), "no files matched") } @@ -144,7 +135,7 @@ func TestUploadRecursive_NotADirectory(t *testing.T) { _, _ = iostreams.SetForTest(t) path := writeTempFile(t, "single.pdf") svc := &scriptedUploadSvc{} - err := runUploadRecursive(context.Background(), &UploadOptions{Recursive: true, Glob: "*"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path) + err := runUploadRecursive(context.Background(), &UploadOptions{Recursive: true, Glob: "*"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -158,7 +149,7 @@ func TestUploadRecursive_RejectsNameFlag(t *testing.T) { mkTree(t, dir, "a.pdf") svc := &scriptedUploadSvc{} opts := &UploadOptions{Recursive: true, Glob: "*", Name: "single-name.pdf"} - err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir) + err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -180,7 +171,7 @@ func TestUploadRecursive_PropagatesMultimodelAndMetadata(t *testing.T) { Metadata: []string{"team=alpha"}, Channel: "browser_extension", } - require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir)) + require.NoError(t, runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir)) require.NotNil(t, svc.lastEnableMultimodel) assert.True(t, *svc.lastEnableMultimodel) @@ -195,7 +186,7 @@ func TestUploadRecursive_MetadataInvalid_NoCalls(t *testing.T) { svc := &scriptedUploadSvc{} opts := &UploadOptions{Recursive: true, Glob: "*", Metadata: []string{"badformat"}} - err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir) + err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", dir) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -203,32 +194,10 @@ func TestUploadRecursive_MetadataInvalid_NoCalls(t *testing.T) { assert.Empty(t, svc.called, "must fail before any per-file call") } -func TestUploadRecursive_RejectsURLOnlyFlags(t *testing.T) { - _, _ = iostreams.SetForTest(t) - dir := t.TempDir() - mkTree(t, dir, "a.pdf") - for _, tc := range []struct { - name string - opts *UploadOptions - want string - }{ - {"title", &UploadOptions{Recursive: true, Glob: "*", Title: "x"}, "--title"}, - {"file-type", &UploadOptions{Recursive: true, Glob: "*", FileType: "pdf"}, "--file-type"}, - {"tag-id", &UploadOptions{Recursive: true, Glob: "*", TagID: "t"}, "--tag-id"}, - } { - t.Run(tc.name, func(t *testing.T) { - svc := &scriptedUploadSvc{} - err := runUploadRecursive(context.Background(), tc.opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", dir) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) - assert.Contains(t, typed.Message, tc.want) - }) - } -} - -func TestUploadRecursive_JSON_BareObject(t *testing.T) { +// TestUploadRecursive_JSON_BatchEnvelope verifies that --format json emits the +// §4.5 batch envelope shape: {ok, data:[{id,ok,result?|error?}...], meta:{count,successes,failures}}. +// The per-item id is the file path; result carries {id, name} from the server. +func TestUploadRecursive_JSON_BatchEnvelope(t *testing.T) { out, _ := iostreams.SetForTest(t) dir := t.TempDir() mkTree(t, dir, "ok.pdf", "bad.pdf") @@ -243,17 +212,54 @@ func TestUploadRecursive_JSON_BareObject(t *testing.T) { err := runUploadRecursive(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_xxx", dir) require.Error(t, err) // partial failure → typed error - body := out.String() - assert.Contains(t, body, `"kb_id":"kb_xxx"`) - assert.Contains(t, body, `"uploaded":`) - assert.Contains(t, body, `"failed":`) - assert.Contains(t, body, `ok.pdf`) - assert.Contains(t, body, `bad.pdf`) - assert.NotContains(t, body, `"ok":`, "bare output must not carry envelope keys") + var env struct { + OK bool `json:"ok"` + Data []struct { + ID string `json:"id"` + OK bool `json:"ok"` + Result *struct { + ID string `json:"id"` + Name string `json:"name"` + } `json:"result,omitempty"` + Error *struct { + Type string `json:"type"` + Message string `json:"message"` + } `json:"error,omitempty"` + } `json:"data"` + Meta struct { + Count int `json:"count"` + Successes int `json:"successes"` + Failures int `json:"failures"` + } `json:"meta"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env), "must be valid JSON: %s", out.String()) + + assert.False(t, env.OK, "top-level ok must be false when any item failed") + assert.Equal(t, 2, env.Meta.Count) + assert.Equal(t, 1, env.Meta.Successes) + assert.Equal(t, 1, env.Meta.Failures) + require.Len(t, env.Data, 2) + + // File paths are used as batch item ids; verify both files appear. + ids := []string{env.Data[0].ID, env.Data[1].ID} + assert.True(t, strings.Contains(ids[0], "ok.pdf") || strings.Contains(ids[1], "ok.pdf"), "ok.pdf must appear in batch data") + assert.True(t, strings.Contains(ids[0], "bad.pdf") || strings.Contains(ids[1], "bad.pdf"), "bad.pdf must appear in batch data") + + // The success item must have a result with server id/name. + for _, item := range env.Data { + if strings.Contains(item.ID, "ok.pdf") { + assert.True(t, item.OK) + assert.NotNil(t, item.Result) + } else { + assert.False(t, item.OK) + assert.NotNil(t, item.Error) + } + } // --format json must emit exactly ONE JSON document. Per-file "FAIL"/"OK" // progress lines belong on the human path; the typed error is Silent so // the root handler doesn't write anything additional to stdout. + body := out.String() assert.NotContains(t, body, "FAIL ", "per-file plain lines must not appear under --format json") assert.NotContains(t, body, "OK ", "per-file plain lines must not appear under --format json") diff --git a/cli/cmd/doc/upload_test.go b/cli/cmd/doc/upload_test.go index e052b8d5..0d540c2b 100644 --- a/cli/cmd/doc/upload_test.go +++ b/cli/cmd/doc/upload_test.go @@ -2,6 +2,7 @@ package doc import ( "context" + "encoding/json" "errors" "os" "path/filepath" @@ -18,15 +19,12 @@ import ( // fakeUploadSvc captures call arguments and returns canned responses. type fakeUploadSvc struct { - resp *sdk.Knowledge - err error - urlResp *sdk.Knowledge - urlErr error - got struct { + resp *sdk.Knowledge + err error + got struct { kbID, filePath, customName, channel string metadata map[string]string enableMultimodel *bool - urlReq sdk.CreateKnowledgeFromURLRequest } } @@ -46,16 +44,6 @@ func (f *fakeUploadSvc) CreateKnowledgeFromFile( return f.resp, f.err } -func (f *fakeUploadSvc) CreateKnowledgeFromURL( - _ context.Context, - kbID string, - req sdk.CreateKnowledgeFromURLRequest, -) (*sdk.Knowledge, error) { - f.got.kbID = kbID - f.got.urlReq = req - return f.urlResp, f.urlErr -} - // writeTempFile creates a regular file under t.TempDir() with sample content. func writeTempFile(t *testing.T, name string) string { t.Helper() @@ -69,7 +57,7 @@ func TestUpload_Success_Human(t *testing.T) { path := writeTempFile(t, "report.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_99", FileName: "report.pdf"}} opts := &UploadOptions{} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, "kb_xxx", svc.got.kbID) assert.Equal(t, path, svc.got.filePath) @@ -91,7 +79,7 @@ func TestUpload_Success_CustomName(t *testing.T) { path := writeTempFile(t, "q3.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_88", FileName: "q3.pdf"}} opts := &UploadOptions{Name: "Q3 Marketing Report.pdf"} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, "Q3 Marketing Report.pdf", svc.got.customName) } @@ -103,16 +91,21 @@ func TestUpload_Success_JSON(t *testing.T) { require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_xxx", path)) got := out.String() - assert.True(t, strings.HasPrefix(strings.TrimSpace(got), `{"id":"doc_77"`), "expected bare Knowledge object; got %q", got) + var env struct { + OK bool `json:"ok"` + Data sdk.Knowledge `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(got), &env), "expected valid JSON envelope, got %q", got) + assert.True(t, env.OK, "envelope.ok must be true") + assert.Equal(t, "doc_77", env.Data.ID, "envelope.data.id must be doc_77") assert.Contains(t, got, `"file_name":"a.md"`) - assert.NotContains(t, got, `"ok":`) } func TestUpload_HTTPError_500(t *testing.T) { _, _ = iostreams.SetForTest(t) path := writeTempFile(t, "x.txt") svc := &fakeUploadSvc{err: errors.New("HTTP error 500: internal")} - err := runUpload(context.Background(), &UploadOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path) + err := runUpload(context.Background(), &UploadOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path) require.Error(t, err) var typed *cmdutil.Error @@ -124,7 +117,7 @@ func TestUpload_HTTPError_409Conflict(t *testing.T) { _, _ = iostreams.SetForTest(t) path := writeTempFile(t, "dup.pdf") svc := &fakeUploadSvc{err: errors.New("HTTP error 409: file exists")} - err := runUpload(context.Background(), &UploadOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path) + err := runUpload(context.Background(), &UploadOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path) require.Error(t, err) var typed *cmdutil.Error @@ -135,15 +128,12 @@ func TestUpload_HTTPError_409Conflict(t *testing.T) { // TestUpload_DuplicateFileMaps_resource_already_exists pins the contract that // the SDK's sentinel sdk.ErrDuplicateFile (returned with no "HTTP error <n>:" // prefix because the duplicate is detected by file-hash short-circuit, not by -// status code) is mapped to resource.already_exists. Prior regression: the -// file-upload path forwarded the sentinel to WrapHTTP, which classified the -// prefix-less message as network.error — symmetric with the --from-url branch -// which already handled ErrDuplicateURL. +// status code) is mapped to resource.already_exists. func TestUpload_DuplicateFileMaps_resource_already_exists(t *testing.T) { _, _ = iostreams.SetForTest(t) path := writeTempFile(t, "dup.md") svc := &fakeUploadSvc{err: sdk.ErrDuplicateFile} - err := runUpload(context.Background(), &UploadOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path) + err := runUpload(context.Background(), &UploadOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path) require.Error(t, err) var typed *cmdutil.Error @@ -187,105 +177,7 @@ func TestValidateUploadPath_SymlinkToFileAccepted(t *testing.T) { require.NoError(t, validateUploadPath(link)) } -// --from-url tests (4-N1). - -func TestUploadFromURL_Success_Human(t *testing.T) { - out, _ := iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_url_1", FileName: "whitepaper.pdf"}} - opts := &UploadOptions{FromURL: "https://example.com/whitepaper.pdf"} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - - assert.Equal(t, "kb_xxx", svc.got.kbID) - assert.Equal(t, "https://example.com/whitepaper.pdf", svc.got.urlReq.URL) - assert.Equal(t, "api", svc.got.urlReq.Channel) - assert.Contains(t, out.String(), "Ingested") - assert.Contains(t, out.String(), "doc_url_1") -} - -func TestUploadFromURL_WithName_Passes_AsFileName(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_url_2"}} - opts := &UploadOptions{FromURL: "https://example.com/article.html", Name: "Q3 Article"} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - assert.Equal(t, "Q3 Article", svc.got.urlReq.FileName, - "--name must be forwarded as FileName (server uses it for file-vs-crawl mode hint)") -} - -func TestUploadFromURL_JSON_BareObject(t *testing.T) { - out, _ := iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_url_3", FileName: "ok.pdf"}} - fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} - require.NoError(t, runUploadFromURL(context.Background(), - &UploadOptions{FromURL: "https://example.com/ok.pdf"}, fopts, svc, "kb_xxx")) - got := out.String() - assert.Contains(t, got, `"id":"doc_url_3"`) - assert.NotContains(t, got, `"ok":`) - assert.NotContains(t, got, `"risk":`) -} - -func TestUploadFromURL_DuplicateURLMaps_resource_already_exists(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{ - urlResp: &sdk.Knowledge{ID: "doc_existing"}, - urlErr: sdk.ErrDuplicateURL, - } - err := runUploadFromURL(context.Background(), - &UploadOptions{FromURL: "https://example.com/dup.pdf"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx") - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeResourceAlreadyExists, typed.Code) -} - -func TestValidateUploadFlags_FromURL_OK(t *testing.T) { - require.NoError(t, validateUploadFlags(&UploadOptions{FromURL: "https://example.com/x.pdf"}, nil)) -} - -func TestValidateUploadFlags_FromURL_WithPositional_Rejected(t *testing.T) { - err := validateUploadFlags(&UploadOptions{FromURL: "https://example.com/x.pdf"}, []string{"/tmp/x.pdf"}) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) -} - -func TestValidateUploadFlags_FromURL_WithRecursive_Rejected(t *testing.T) { - err := validateUploadFlags(&UploadOptions{FromURL: "https://example.com/x.pdf", Recursive: true}, nil) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) -} - -// The server's URL-ingest request type has no Metadata field; the CLI must -// reject --metadata + --from-url upfront so callers don't think they've set -// metadata that the server then silently drops on the wire. -func TestValidateUploadFlags_FromURL_WithMetadata_Rejected(t *testing.T) { - err := validateUploadFlags(&UploadOptions{ - FromURL: "https://example.com/x.pdf", - Metadata: []string{"team=alpha"}, - }, nil) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) - assert.Contains(t, typed.Message, "--metadata is not supported with --from-url") -} - -func TestValidateUploadFlags_FromURL_BadScheme(t *testing.T) { - err := validateUploadFlags(&UploadOptions{FromURL: "file:///etc/passwd"}, nil) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) -} - -func TestValidateUploadFlags_FromURL_NoHost(t *testing.T) { - err := validateUploadFlags(&UploadOptions{FromURL: "https://"}, nil) - require.Error(t, err) -} - -func TestValidateUploadFlags_NoPathOrURL_Rejected(t *testing.T) { +func TestValidateUploadFlags_NoPath_Rejected(t *testing.T) { err := validateUploadFlags(&UploadOptions{}, nil) require.Error(t, err) // Missing required input wraps as FlagError so the exit code (2) @@ -295,7 +187,11 @@ func TestValidateUploadFlags_NoPathOrURL_Rejected(t *testing.T) { assert.Equal(t, 2, cmdutil.ExitCode(err)) } -// --- C10 expanded flags: multimodel / metadata / channel / URL-mode extras --- +func TestValidateUploadFlags_WithPath_OK(t *testing.T) { + require.NoError(t, validateUploadFlags(&UploadOptions{}, []string{"/tmp/x.pdf"})) +} + +// --- C10 expanded flags: multimodel / metadata / channel --- func TestUpload_EnableMultimodel_Set_True(t *testing.T) { _, _ = iostreams.SetForTest(t) @@ -303,7 +199,7 @@ func TestUpload_EnableMultimodel_Set_True(t *testing.T) { svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_mm", FileName: "mm.pdf"}} mm := true opts := &UploadOptions{EnableMultimodel: &mm} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) require.NotNil(t, svc.got.enableMultimodel, "expected non-nil *bool when flag set") assert.True(t, *svc.got.enableMultimodel) } @@ -314,7 +210,7 @@ func TestUpload_EnableMultimodel_Set_False(t *testing.T) { svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_mm", FileName: "mm.pdf"}} mm := false opts := &UploadOptions{EnableMultimodel: &mm} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) require.NotNil(t, svc.got.enableMultimodel, "explicit false must still surface as non-nil *bool") assert.False(t, *svc.got.enableMultimodel) } @@ -358,7 +254,7 @@ func TestUpload_Metadata_ParseKV(t *testing.T) { path := writeTempFile(t, "m.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} opts := &UploadOptions{Metadata: []string{"foo=bar", "baz=qux"}} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, map[string]string{"foo": "bar", "baz": "qux"}, svc.got.metadata) } @@ -367,7 +263,7 @@ func TestUpload_Metadata_EmptyValueAllowed(t *testing.T) { path := writeTempFile(t, "m.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} opts := &UploadOptions{Metadata: []string{"foo="}} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, map[string]string{"foo": ""}, svc.got.metadata) } @@ -376,7 +272,7 @@ func TestUpload_Metadata_LastWins(t *testing.T) { path := writeTempFile(t, "m.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} opts := &UploadOptions{Metadata: []string{"k=v1", "k=v2"}} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, map[string]string{"k": "v2"}, svc.got.metadata) } @@ -385,7 +281,7 @@ func TestUpload_Metadata_InvalidFormat_NoEquals(t *testing.T) { path := writeTempFile(t, "m.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} opts := &UploadOptions{Metadata: []string{"foo"}} - err := runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path) + err := runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -397,7 +293,7 @@ func TestUpload_Metadata_InvalidFormat_EmptyKey(t *testing.T) { path := writeTempFile(t, "m.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_m", FileName: "m.pdf"}} opts := &UploadOptions{Metadata: []string{"=bar"}} - err := runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path) + err := runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -409,7 +305,7 @@ func TestUpload_Channel_Override(t *testing.T) { path := writeTempFile(t, "c.pdf") svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_c", FileName: "c.pdf"}} opts := &UploadOptions{Channel: "browser_extension"} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, "browser_extension", svc.got.channel) } @@ -419,81 +315,6 @@ func TestUpload_Channel_DefaultStillAPI(t *testing.T) { svc := &fakeUploadSvc{resp: &sdk.Knowledge{ID: "doc_c", FileName: "c.pdf"}} // Empty Channel is the runUpload contract for "use default". opts := &UploadOptions{} - require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx", path)) + require.NoError(t, runUpload(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman}, svc, "kb_xxx", path)) assert.Equal(t, uploadChannel, svc.got.channel) } - -// URL-mode metadata happy paths - -func TestUploadFromURL_Title(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} - opts := &UploadOptions{FromURL: "https://example.com/a.pdf", Title: "My Title"} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - assert.Equal(t, "My Title", svc.got.urlReq.Title) -} - -func TestUploadFromURL_FileType(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} - opts := &UploadOptions{FromURL: "https://example.com/no-ext", FileType: "pdf"} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - assert.Equal(t, "pdf", svc.got.urlReq.FileType) -} - -func TestUploadFromURL_TagID(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} - opts := &UploadOptions{FromURL: "https://example.com/a.pdf", TagID: "tag_99"} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - assert.Equal(t, "tag_99", svc.got.urlReq.TagID) -} - -func TestUploadFromURL_EnableMultimodel_Forwarded(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} - mm := true - opts := &UploadOptions{FromURL: "https://example.com/a.pdf", EnableMultimodel: &mm} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - require.NotNil(t, svc.got.urlReq.EnableMultimodel) - assert.True(t, *svc.got.urlReq.EnableMultimodel) -} - -func TestUploadFromURL_Channel_Override(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeUploadSvc{urlResp: &sdk.Knowledge{ID: "doc_u"}} - opts := &UploadOptions{FromURL: "https://example.com/a.pdf", Channel: "web"} - require.NoError(t, runUploadFromURL(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) - assert.Equal(t, "web", svc.got.urlReq.Channel) -} - -// URL-only flag misuse: error when used without --from-url. -// validateUploadFlags should reject --title/--file-type/--tag-id paired -// with a positional file path (i.e., no --from-url). - -func TestValidateUploadFlags_Title_RequiresFromURL(t *testing.T) { - err := validateUploadFlags(&UploadOptions{Title: "x"}, []string{"/tmp/x.pdf"}) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) - assert.Contains(t, typed.Message, "--title") -} - -func TestValidateUploadFlags_FileType_RequiresFromURL(t *testing.T) { - err := validateUploadFlags(&UploadOptions{FileType: "pdf"}, []string{"/tmp/x.pdf"}) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) - assert.Contains(t, typed.Message, "--file-type") -} - -func TestValidateUploadFlags_TagID_RequiresFromURL(t *testing.T) { - err := validateUploadFlags(&UploadOptions{TagID: "tag_x"}, []string{"/tmp/x.pdf"}) - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputInvalidArgument, typed.Code) - assert.Contains(t, typed.Message, "--tag-id") -} diff --git a/cli/cmd/kb/empty.go b/cli/cmd/kb/empty.go deleted file mode 100644 index 94b3ac3f..00000000 --- a/cli/cmd/kb/empty.go +++ /dev/null @@ -1,88 +0,0 @@ -package kb - -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" - sdk "github.com/Tencent/WeKnora/client" -) - -// kbEmptyFields enumerates the fields surfaced for `--format json` discovery on -// `kb empty`. The result payload is {id, deleted_count}. -var kbEmptyFields = []string{"id", "deleted_count"} - -type EmptyOptions struct { - Yes bool -} - -type EmptyService interface { - ClearKnowledgeBaseContents(ctx context.Context, id string) (*sdk.ClearKnowledgeBaseContentsResponse, error) -} - -// emptyResult is the typed payload emitted under data on success. -type emptyResult struct { - ID string `json:"id"` - DeletedCount int `json:"deleted_count"` -} - -// NewCmdEmpty builds `weknora kb empty <id>`. Wipes every document inside -// the knowledge base; the KB itself is preserved. The server runs the -// delete asynchronously and reports the count of documents that were -// enqueued for removal. -func NewCmdEmpty(f *cmdutil.Factory) *cobra.Command { - opts := &EmptyOptions{} - cmd := &cobra.Command{ - Use: "empty <kb-id>", - Short: "Delete every document in a knowledge base (preserves the KB)", - Long: `Removes all documents and chunks from a knowledge base while keeping the -KB record (its name, description, and config) intact. The delete is async; -the server reports the count of items enqueued for removal. - -Prompts for confirmation by default; pass -y/--yes to skip in agent / CI / -piped contexts. Without -y the CLI exits 10 in non-interactive mode.`, - Example: ` weknora kb empty kb_abc # interactive confirm - weknora kb empty kb_abc -y --format json # agent-friendly`, - Args: cobra.ExactArgs(1), - RunE: func(c *cobra.Command, args []string) error { - fopts, err := cmdutil.CheckFormatFlag(c) - if err != nil { - return err - } - fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) - opts.Yes, _ = c.Flags().GetBool("yes") - cli, err := f.Client() - if err != nil { - return err - } - return runEmpty(c.Context(), opts, fopts, cli, f.Prompter(), args[0]) - }, - } - cmdutil.AddFormatFlag(cmd, kbEmptyFields...) - return cmd -} - -func runEmpty(ctx context.Context, opts *EmptyOptions, fopts *cmdutil.FormatOptions, svc EmptyService, p prompt.Prompter, id string) error { - if err := cmdutil.ConfirmDestructive(p, opts.Yes, fopts.WantsJSON(), "all contents of knowledge base", id); err != nil { - return err - } - - resp, err := svc.ClearKnowledgeBaseContents(ctx, id) - if err != nil { - return cmdutil.WrapHTTP(err, "empty knowledge base %s", id) - } - deleted := 0 - if resp != nil { - deleted = resp.DeletedCount - } - - if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, emptyResult{ID: id, DeletedCount: deleted}) - } - fmt.Fprintf(iostreams.IO.Out, "✓ Emptied knowledge base %s (%d document(s) cleared)\n", id, deleted) - return nil -} diff --git a/cli/cmd/kb/empty_test.go b/cli/cmd/kb/empty_test.go deleted file mode 100644 index ae40c15a..00000000 --- a/cli/cmd/kb/empty_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package kb - -import ( - "context" - "errors" - "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" - sdk "github.com/Tencent/WeKnora/client" -) - -// fakeEmptySvc records calls + scripts the response. -type fakeEmptySvc struct { - err error - gotID string - called bool - resp *sdk.ClearKnowledgeBaseContentsResponse -} - -func (f *fakeEmptySvc) ClearKnowledgeBaseContents(_ context.Context, id string) (*sdk.ClearKnowledgeBaseContentsResponse, error) { - f.called = true - f.gotID = id - if f.err != nil { - return nil, f.err - } - if f.resp == nil { - return &sdk.ClearKnowledgeBaseContentsResponse{DeletedCount: 0}, nil - } - return f.resp, nil -} - -func TestEmpty_WithYes(t *testing.T) { - out, _ := iostreams.SetForTest(t) - svc := &fakeEmptySvc{resp: &sdk.ClearKnowledgeBaseContentsResponse{DeletedCount: 42}} - require.NoError(t, runEmpty(context.Background(), &EmptyOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{}, "kb_abc")) - assert.True(t, svc.called) - assert.Equal(t, "kb_abc", svc.gotID) - body := out.String() - assert.Contains(t, body, "kb_abc") - assert.Contains(t, body, "42") -} - -func TestEmpty_NonTTY_NoYes_RequiresConfirmation(t *testing.T) { - iostreams.SetForTest(t) - svc := &fakeEmptySvc{} - err := runEmpty(context.Background(), &EmptyOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{}, "kb_abc") - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeInputConfirmationRequired, typed.Code) - assert.Equal(t, 10, cmdutil.ExitCode(err)) - assert.False(t, svc.called) -} - -func TestEmpty_TTY_ConfirmNo(t *testing.T) { - _, errBuf := iostreams.SetForTestWithTTY(t) - svc := &fakeEmptySvc{} - p := &testutil.ConfirmPrompter{Answer: false} - err := runEmpty(context.Background(), &EmptyOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, p, "kb_abc") - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeUserAborted, typed.Code) - assert.False(t, svc.called) - assert.Contains(t, errBuf.String(), "Aborted") -} - -func TestEmpty_NotFound(t *testing.T) { - _, _ = iostreams.SetForTest(t) - svc := &fakeEmptySvc{err: errors.New("HTTP error 404: not found")} - err := runEmpty(context.Background(), &EmptyOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, &testutil.ConfirmPrompter{}, "kb_missing") - require.Error(t, err) - var typed *cmdutil.Error - require.ErrorAs(t, err, &typed) - assert.Equal(t, cmdutil.CodeResourceNotFound, typed.Code) -} diff --git a/cli/cmd/kb/kb.go b/cli/cmd/kb/kb.go index 0366c670..06e40a21 100644 --- a/cli/cmd/kb/kb.go +++ b/cli/cmd/kb/kb.go @@ -1,6 +1,7 @@ // Package kb holds the `weknora kb` command tree: list / view / create / -// edit / delete / pin / unpin / empty. Verb set follows common CRUD -// vocabulary (list/view/create/edit/delete) plus pin/unpin and empty. +// edit / delete / pin / unpin. Verb set follows common CRUD vocabulary +// (list/view/create/edit/delete) plus pin/unpin. Bulk content deletion +// is exposed via `weknora doc delete --all --kb=<id>`. package kb import ( @@ -14,8 +15,6 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "kb", Short: "Manage knowledge bases", - Args: cobra.NoArgs, - Run: func(c *cobra.Command, _ []string) { _ = c.Help() }, } cmd.AddCommand(NewCmdList(f)) cmd.AddCommand(NewCmdView(f)) @@ -24,7 +23,6 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(NewCmdDelete(f)) cmd.AddCommand(NewCmdPin(f)) cmd.AddCommand(NewCmdUnpin(f)) - cmd.AddCommand(NewCmdEmpty(f)) cmd.AddCommand(NewCmdStatus(f)) cmd.AddCommand(NewCmdCheck(f)) return cmd diff --git a/cli/cmd/session/ask.go b/cli/cmd/session/ask.go new file mode 100644 index 00000000..fc61daf9 --- /dev/null +++ b/cli/cmd/session/ask.go @@ -0,0 +1,275 @@ +package sessioncmd + +import ( + "context" + "fmt" + "io" + "strings" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/format" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/output" + "github.com/Tencent/WeKnora/cli/internal/sse" + sdk "github.com/Tencent/WeKnora/client" +) + +// sessionAskFields enumerates the NDJSON init-event fields surfaced for +// `--format json` / `--format ndjson` discovery on `session ask`. Reflects +// the InitEvent head line + the raw SDK agent event vocabulary (§5). +var sessionAskFields = []string{ + "session_id", "agent_id", + // SDK event fields (pass-through): response_type, content, done, + // knowledge_references, tool_call_id, tool_result +} + +// AskOptions captures `session ask` flag state. +type AskOptions struct { + AgentID string + Query string + SessionID string // --session: continue an existing session (skip auto-create) +} + +// AskService is the narrow SDK surface this command depends on. +// +// CreateSession is called when --session is omitted — sessions are +// agent-agnostic at creation (verified against +// internal/handler/session/handler.go CreateSession, which only persists +// {title, description}). The agent ID is supplied per-request via +// AgentQARequest.AgentID, so the same session can be reused across +// agent / KB-chat invocations. +type AskService interface { + CreateSession(ctx context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) + AgentQAStreamWithRequest(ctx context.Context, sessionID string, req *sdk.AgentQARequest, cb sdk.AgentEventCallback) error +} + +// NewCmdAsk builds `weknora session ask --agent <agent-id> "<text>"`. +func NewCmdAsk(f *cmdutil.Factory) *cobra.Command { + opts := &AskOptions{} + cmd := &cobra.Command{ + Use: `ask "<text>"`, + Short: "Ask a server-side agent in a session context", + Long: `Invoke a server-side agent within a session. If --session is omitted, +a new session is auto-created and its id is reported in the output for +the caller to thread follow-ups. + +AI agents: this is the primary entrypoint for invoking custom agents. +The 'weknora agent' subtree handles CRUD only (list / view / create / +edit / delete / status / check). + +Modes: + TTY (human format, default): live answer streaming + tool-trace footer + --format json / --format ndjson / pipe: NDJSON event stream — one init line + at head (session_id, agent_id), then raw SDK + agent events verbatim. Both json and ndjson + flags produce the same NDJSON stream (§5).`, + Example: ` weknora session ask --agent ag_x "Summarize Q3 sales" + weknora session ask --session sess_x --agent ag_x "Follow-up question" + weknora session ask --agent ag_x "Multi-step task" --format ndjson`, + Args: cobra.ExactArgs(1), + RunE: func(c *cobra.Command, args []string) error { + fopts, err := cmdutil.CheckFormatFlag(c) + if err != nil { + return err + } + fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) + opts.Query = strings.TrimSpace(args[0]) + cli, err := f.Client() + if err != nil { + return err + } + return runAsk(c.Context(), opts, fopts, cli) + }, + } + cmd.Flags().StringVarP(&opts.AgentID, "agent", "a", "", "Agent ID to invoke (required)") + _ = cmd.MarkFlagRequired("agent") + cmd.Flags().StringVar(&opts.SessionID, "session", "", "Continue an existing chat session (skip auto-create)") + cmdutil.AddFormatFlag(cmd, sessionAskFields...) + cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ + UsedFor: "Invoke a custom agent in a session context. Produces an NDJSON event stream: init line (session_id, agent_id) then raw SDK agent events. Use --format json or --format ndjson.", + RequiredFlags: []string{"--agent"}, + Examples: []string{ + `weknora session ask --agent ag_x "Summarize Q3 sales" --format json`, + `weknora session ask --session sess_x --agent ag_x "Follow-up question" --format json`, + }, + Output: "NDJSON stream: {type:init, session_id, agent_id} then SDK agent events (response_type, content, done, knowledge_references, ...)", + }) + return cmd +} + +func runAsk(ctx context.Context, opts *AskOptions, fopts *cmdutil.FormatOptions, svc AskService) error { + if opts.Query == "" { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "query argument cannot be empty") + } + if opts.AgentID == "" { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "agent-id argument cannot be empty") + } + if svc == nil { + return cmdutil.NewError(cmdutil.CodeServerError, "session ask: no SDK client available") + } + + // Streaming commands route --format json AND --format ndjson to the + // NDJSON event-stream path. A buffered envelope makes no sense for a + // streaming command (§5). Only --format human (or TTY default) uses the + // live renderer. + ndjsonMode := fopts != nil && (fopts.Mode == cmdutil.FormatJSON || fopts.Mode == cmdutil.FormatNDJSON) + + sessionID := opts.SessionID + autoCreated := false + if sessionID == "" { + sess, err := svc.CreateSession(ctx, &sdk.CreateSessionRequest{Title: "weknora session ask"}) + if err != nil { + if cmdutil.IsCancelled(ctx, err) { + return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, err, "session ask cancelled") + } + code := cmdutil.ClassifyHTTPError(err) + if code == cmdutil.CodeNetworkError || code == cmdutil.CodeServerError { + code = cmdutil.CodeSessionCreateFailed + } + return cmdutil.Wrapf(code, err, "create chat session") + } + sessionID = sess.ID + autoCreated = true + } + + if ndjsonMode { + return runAskNDJSON(ctx, opts, sessionID, svc) + } + + // Surface auto-created session id up-front so a ^C mid-stream still + // leaves a recoverable pointer. Skipped in NDJSON mode (it appears in + // the init event). + if autoCreated { + fmt.Fprintf(iostreams.IO.Err, "session: %s (use --session to continue)\n", sessionID) + } + + return runAskHuman(ctx, opts, sessionID, autoCreated, svc) +} + +// runAskNDJSON handles --format json and --format ndjson paths. +// Emits a CLI init event at stream head, then passes every SDK agent event +// through verbatim as NDJSON lines. No buffering (§5). +func runAskNDJSON(ctx context.Context, opts *AskOptions, sessionID string, svc AskService) error { + w := iostreams.IO.Out + + // 1. Inject the CLI-managed init event at the head of the stream (§5.3). + // Carries session pointer + agent id callers need for follow-up threading. + initEv := output.InitEvent{ + SessionID: sessionID, + AgentID: opts.AgentID, + Profile: cmdutil.GetProfile(), + } + if err := output.EmitInit(w, initEv); err != nil { + return err + } + + // 2. Open SDK stream and pass each agent event through as a bare NDJSON line. + req := &sdk.AgentQARequest{ + Query: opts.Query, + AgentEnabled: true, + AgentID: opts.AgentID, + Channel: "api", + } + cb := func(r *sdk.AgentStreamResponse) error { + return output.EmitSDKEvent(w, r) + } + if err := svc.AgentQAStreamWithRequest(ctx, sessionID, req, cb); err != nil { + if cmdutil.IsCancelled(ctx, err) { + return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, err, "session ask cancelled") + } + return cmdutil.WrapHTTP(err, "agent-chat stream") + } + return nil +} + +// runAskHuman handles the TTY / human-text path. Streams answer fragments +// live on TTY; accumulates then renders on non-TTY pipes. +func runAskHuman(ctx context.Context, opts *AskOptions, sessionID string, autoCreated bool, svc AskService) error { + // Stream mode requires an interactive stdout. + streamMode := iostreams.IO.IsStdoutTTY() + + req := &sdk.AgentQARequest{ + Query: opts.Query, + AgentEnabled: true, + AgentID: opts.AgentID, + Channel: "api", + } + + acc := &sse.AgentAccumulator{} + cb := func(r *sdk.AgentStreamResponse) error { + if streamMode && r != nil && r.ResponseType == sdk.AgentResponseTypeAnswer && r.Content != "" { + _, _ = iostreams.IO.Out.Write([]byte(r.Content)) + } + acc.Append(r) + return nil + } + + streamErr := svc.AgentQAStreamWithRequest(ctx, sessionID, req, cb) + if streamErr != nil { + if autoCreated { + fmt.Fprintf(iostreams.IO.Err, "session: %s (resume with --session %s)\n", sessionID, sessionID) + } + if cmdutil.IsCancelled(ctx, streamErr) { + return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, streamErr, "session ask cancelled") + } + if acc.Answer() != "" && !acc.Done() { + return cmdutil.Wrapf(cmdutil.CodeSSEStreamAborted, streamErr, "stream aborted before completion") + } + return cmdutil.WrapHTTP(streamErr, "agent-chat stream") + } + + // Server closed cleanly but never sent a Done event — treat as aborted + // so agents don't silently emit a truncated answer as ok=true. + if !acc.Done() { + return cmdutil.NewError(cmdutil.CodeSSEStreamAborted, "stream ended without a terminal event") + } + + answer := acc.Answer() + out := iostreams.IO.Out + if streamMode { + if !strings.HasSuffix(answer, "\n") { + fmt.Fprintln(out) + } + } else { + fmt.Fprint(out, answer) + if !strings.HasSuffix(answer, "\n") { + fmt.Fprintln(out) + } + } + renderAskToolTrace(out, acc.ToolEvents) + format.WriteReferences(out, acc.References) + return nil +} + +// renderAskToolTrace prints a compact tool-event footer in human mode. +// Skipped when the agent emitted no tool events — silent beats an empty +// banner. +func renderAskToolTrace(w io.Writer, events []sse.AgentToolEvent) { + if len(events) == 0 { + return + } + fmt.Fprintln(w) + fmt.Fprintln(w, "──── Tool trace ────") + for i, e := range events { + fmt.Fprintf(w, "[%d] %s", i+1, e.Kind) + if e.Result != "" { + fmt.Fprintf(w, " %s", truncateAskInline(e.Result, 80)) + } + fmt.Fprintln(w) + } +} + +// truncateAskInline shrinks a multi-line result to a single line + ellipsis +// for the human tool-trace footer. +func truncateAskInline(s string, maxLen int) string { + s = strings.ReplaceAll(s, "\n", " ") + if len(s) <= maxLen { + return s + } + return s[:maxLen-1] + "…" +} + +// compile-time check: production SDK client satisfies AskService. +var _ AskService = (*sdk.Client)(nil) diff --git a/cli/cmd/session/ask_test.go b/cli/cmd/session/ask_test.go new file mode 100644 index 00000000..56a7b71d --- /dev/null +++ b/cli/cmd/session/ask_test.go @@ -0,0 +1,465 @@ +package sessioncmd + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// scriptedAskSvc serves a canned stream of agent events to runAsk. +type scriptedAskSvc struct { + createResp *sdk.Session + createErr error + events []*sdk.AgentStreamResponse + streamErr error + got struct { + sessionID string + req *sdk.AgentQARequest + } +} + +func (s *scriptedAskSvc) CreateSession(_ context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) { + if s.createResp == nil && s.createErr == nil { + return &sdk.Session{ID: "sess_auto", Title: req.Title}, nil + } + return s.createResp, s.createErr +} + +func (s *scriptedAskSvc) AgentQAStreamWithRequest(_ context.Context, sessionID string, req *sdk.AgentQARequest, cb sdk.AgentEventCallback) error { + s.got.sessionID = sessionID + s.got.req = req + for _, e := range s.events { + if err := cb(e); err != nil { + return err + } + } + return s.streamErr +} + +func answerEvent(content string) *sdk.AgentStreamResponse { + return &sdk.AgentStreamResponse{ResponseType: sdk.AgentResponseTypeAnswer, Content: content} +} +func doneEvent() *sdk.AgentStreamResponse { + return &sdk.AgentStreamResponse{ResponseType: sdk.AgentResponseTypeAnswer, Done: true} +} +func toolCallEvent(id, name string) *sdk.AgentStreamResponse { + return &sdk.AgentStreamResponse{ + ResponseType: sdk.AgentResponseTypeToolCall, + ID: id, + Content: name, + } +} +func referencesEvent(refs []*sdk.SearchResult) *sdk.AgentStreamResponse { + return &sdk.AgentStreamResponse{ + ResponseType: sdk.AgentResponseTypeReferences, + KnowledgeReferences: refs, + } +} + +// textOpts returns a FormatOptions configured for the text (human) render +// path — the most common shape under test. +func textOpts() *cmdutil.FormatOptions { + return &cmdutil.FormatOptions{Mode: cmdutil.FormatHuman} +} + +// ndjsonOpts returns a FormatOptions for the NDJSON event-stream path. +// --format json routes here too (§5). +func ndjsonOpts() *cmdutil.FormatOptions { + return &cmdutil.FormatOptions{Mode: cmdutil.FormatNDJSON} +} + +// jsonOpts returns a FormatOptions configured for the JSON path. +// In v0.7 --format json routes to the NDJSON stream path for streaming commands. +func jsonOpts() *cmdutil.FormatOptions { + return &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} +} + +// TestSessionAsk_NDJSON_FirstLineIsInit verifies that the NDJSON path (--format json) +// always injects an "init" line first, carrying session_id and agent_id. +func TestSessionAsk_NDJSON_FirstLineIsInit(t *testing.T) { + out, errBuf := iostreams.SetForTest(t) + svc := &scriptedAskSvc{ + events: []*sdk.AgentStreamResponse{ + answerEvent("answer"), + doneEvent(), + }, + } + opts := &AskOptions{AgentID: "ag_x", Query: "ping"} + if err := runAsk(context.Background(), opts, jsonOpts(), svc); err != nil { + t.Fatalf("runAsk: %v", err) + } + + // NDJSON mode must NOT print the session hint to stderr. + if errBuf.Len() != 0 { + t.Errorf("expected empty stderr in NDJSON mode, got %q", errBuf.String()) + } + + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + if len(lines) == 0 { + t.Fatal("no output") + } + var first struct { + Type string `json:"type"` + SessionID string `json:"session_id"` + AgentID string `json:"agent_id"` + } + if err := json.Unmarshal([]byte(lines[0]), &first); err != nil { + t.Fatalf("first line not JSON: %v\n %s", err, lines[0]) + } + if first.Type != "init" { + t.Errorf("first line type: got %q, want init", first.Type) + } + if first.SessionID != "sess_auto" { + t.Errorf("init.session_id: got %q, want sess_auto", first.SessionID) + } + if first.AgentID != "ag_x" { + t.Errorf("init.agent_id: got %q, want ag_x", first.AgentID) + } +} + +// TestSessionAsk_NDJSON_PassthroughEvents verifies init + N SDK events = N+1 total lines. +func TestSessionAsk_NDJSON_PassthroughEvents(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &scriptedAskSvc{ + events: []*sdk.AgentStreamResponse{ + answerEvent("hello"), + doneEvent(), + }, + } + opts := &AskOptions{AgentID: "ag_x", Query: "hi"} + if err := runAsk(context.Background(), opts, jsonOpts(), svc); err != nil { + t.Fatalf("runAsk: %v", err) + } + + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + // 1 init + 2 SDK events = 3 lines. + if len(lines) != 3 { + t.Fatalf("got %d lines, want 3:\n%s", len(lines), out.String()) + } + // Each must be valid JSON. + for i, line := range lines { + var obj map[string]any + if err := json.Unmarshal([]byte(line), &obj); err != nil { + t.Errorf("line %d not valid JSON: %v\n %s", i+1, err, line) + } + } +} + +// TestSessionAsk_NDJSON_JSONEqualsNDJSON verifies that --format json and --format ndjson +// produce identical NDJSON streams for session ask (§5). +func TestSessionAsk_NDJSON_JSONEqualsNDJSON(t *testing.T) { + events := []*sdk.AgentStreamResponse{ + answerEvent("hello"), + doneEvent(), + } + + runWith := func(mode cmdutil.FormatMode) string { + out, _ := iostreams.SetForTest(t) + svc := &scriptedAskSvc{events: events} + opts := &AskOptions{AgentID: "ag_x", Query: "q"} + fopts := &cmdutil.FormatOptions{Mode: mode} + if err := runAsk(context.Background(), opts, fopts, svc); err != nil { + t.Fatalf("runAsk(%s): %v", mode, err) + } + return out.String() + } + + jsonOut := runWith(cmdutil.FormatJSON) + ndjsonOut := runWith(cmdutil.FormatNDJSON) + if jsonOut != ndjsonOut { + t.Errorf("--format json and --format ndjson differ:\n json: %q\n ndjson: %q", jsonOut, ndjsonOut) + } +} + +// TestSessionAsk_AutoCreatedSessionID_PassedAsAgentRequest checks the session id +// flows from auto-create through to the SDK stream call. +func TestSessionAsk_AutoCreatedSessionID_PassedAsAgentRequest(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &scriptedAskSvc{events: []*sdk.AgentStreamResponse{doneEvent()}} + opts := &AskOptions{AgentID: "ag_42", Query: "x"} + if err := runAsk(context.Background(), opts, ndjsonOpts(), svc); err != nil { + t.Fatalf("runAsk: %v", err) + } + if svc.got.sessionID != "sess_auto" { + t.Errorf("agent-chat got sessionID=%q, want sess_auto", svc.got.sessionID) + } + if svc.got.req == nil || svc.got.req.AgentID != "ag_42" { + t.Errorf("AgentID not forwarded: %+v", svc.got.req) + } + if !svc.got.req.AgentEnabled { + t.Error("AgentEnabled must be true for session ask") + } +} + +func TestSessionAsk_ExistingSessionID_SkipsCreate(t *testing.T) { + _, _ = iostreams.SetForTest(t) + created := false + svc := &scriptedAskSvc{events: []*sdk.AgentStreamResponse{doneEvent()}} + // Wrap CreateSession to detect call. + svc.createResp = &sdk.Session{ID: "should_not_be_used"} + wrapped := &createSessionTracker{AskService: svc, called: &created} + opts := &AskOptions{AgentID: "ag", Query: "x", SessionID: "sess_existing"} + if err := runAsk(context.Background(), opts, ndjsonOpts(), wrapped); err != nil { + t.Fatalf("runAsk: %v", err) + } + if created { + t.Error("CreateSession should not be called when --session is set") + } + if svc.got.sessionID != "sess_existing" { + t.Errorf("agent-chat got sessionID=%q, want sess_existing", svc.got.sessionID) + } +} + +type createSessionTracker struct { + AskService + called *bool +} + +func (c *createSessionTracker) CreateSession(ctx context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) { + *c.called = true + return c.AskService.CreateSession(ctx, req) +} + +// TestSessionAsk_EmptyQuery_Rejected checks validation fires before any SDK call. +func TestSessionAsk_EmptyQuery_Rejected(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &scriptedAskSvc{} + opts := &AskOptions{AgentID: "ag", Query: ""} + err := runAsk(context.Background(), opts, textOpts(), svc) + if err == nil { + t.Fatal("expected input.invalid_argument, got nil") + } + var typed *cmdutil.Error + if !errors.As(err, &typed) || typed.Code != cmdutil.CodeInputInvalidArgument { + t.Errorf("expected input.invalid_argument, got %v", err) + } +} + +// TestSessionAsk_StreamAbortBeforeDone_MapsToSSEStreamAborted uses the human path +// because the NDJSON path does not buffer/validate Done events. +func TestSessionAsk_StreamAbortBeforeDone_MapsToSSEStreamAborted(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &scriptedAskSvc{ + events: []*sdk.AgentStreamResponse{ + answerEvent("partial"), + }, + streamErr: errors.New("connection reset"), + } + opts := &AskOptions{AgentID: "ag", Query: "x"} + // Human path (textOpts) validates Done; NDJSON path does not buffer. + err := runAsk(context.Background(), opts, textOpts(), svc) + if err == nil { + t.Fatal("expected stream-aborted error") + } + var typed *cmdutil.Error + if !errors.As(err, &typed) || typed.Code != cmdutil.CodeSSEStreamAborted { + t.Errorf("expected local.sse_stream_aborted, got %v", err) + } +} + +// TestSessionAsk_NoDoneEvent_MapsToSSEStreamAborted uses the human path +// because the NDJSON path does not validate Done events. +func TestSessionAsk_NoDoneEvent_MapsToSSEStreamAborted(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &scriptedAskSvc{events: []*sdk.AgentStreamResponse{answerEvent("incomplete")}} + opts := &AskOptions{AgentID: "ag", Query: "x"} + err := runAsk(context.Background(), opts, textOpts(), svc) + if err == nil { + t.Fatal("expected stream-aborted error") + } + var typed *cmdutil.Error + if !errors.As(err, &typed) || typed.Code != cmdutil.CodeSSEStreamAborted { + t.Errorf("expected local.sse_stream_aborted, got %v", err) + } +} + +func TestSessionAsk_CreateSessionFails_MapsToSessionCreateFailed(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &scriptedAskSvc{createErr: errors.New("connection refused")} + opts := &AskOptions{AgentID: "ag", Query: "x"} + err := runAsk(context.Background(), opts, textOpts(), svc) + if err == nil { + t.Fatal("expected session_create_failed") + } + var typed *cmdutil.Error + if !errors.As(err, &typed) || typed.Code != cmdutil.CodeSessionCreateFailed { + t.Errorf("expected server.session_create_failed, got %v", err) + } +} + +func TestSessionAsk_Cancellation_MapsToOperationCancelled(t *testing.T) { + _, _ = iostreams.SetForTest(t) + ctx, cancel := context.WithCancel(context.Background()) + cancel() // pre-cancel + svc := &scriptedAskSvc{streamErr: context.Canceled} + opts := &AskOptions{AgentID: "ag", Query: "x"} + // NDJSON path also handles cancellation correctly. + err := runAsk(ctx, opts, ndjsonOpts(), svc) + if err == nil { + t.Fatal("expected operation.cancelled") + } + var typed *cmdutil.Error + if !errors.As(err, &typed) || typed.Code != cmdutil.CodeOperationCancelled { + t.Errorf("expected operation.cancelled, got %v", err) + } +} + +// Sanity: human-mode output writes the answer body and a tool-trace footer. +func TestSessionAsk_Human_Accumulate_PrintsAnswerAndFooter(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &scriptedAskSvc{events: []*sdk.AgentStreamResponse{ + answerEvent("hello"), + toolCallEvent("c1", "knowledge_search"), + doneEvent(), + }} + opts := &AskOptions{AgentID: "ag", Query: "x"} + if err := runAsk(context.Background(), opts, textOpts(), svc); err != nil { + t.Fatalf("runAsk: %v", err) + } + got := out.String() + if !strings.Contains(got, "hello") { + t.Errorf("answer body missing: %q", got) + } + if !strings.Contains(got, "Tool trace") { + t.Errorf("tool trace footer missing: %q", got) + } +} + +// TestSessionAsk_FormatNDJSON_PassthroughsSDKEvents verifies: +// 1 init line + N SDK events = N+1 total lines; first is init, rest are SDK events. +func TestSessionAsk_FormatNDJSON_PassthroughsSDKEvents(t *testing.T) { + // Fake stream emits 3 events: tool_call, answer, done. + // With the init injection, total output is 4 lines (1 init + 3 SDK events). + svc := &scriptedAskSvc{ + events: []*sdk.AgentStreamResponse{ + toolCallEvent("call_1", "knowledge_search"), + answerEvent("hello"), + doneEvent(), + }, + } + + out, _ := iostreams.SetForTest(t) + + opts := &AskOptions{AgentID: "ag_x", Query: "hi"} + fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatNDJSON} + if err := runAsk(context.Background(), opts, fopts, svc); err != nil { + t.Fatalf("runAsk: %v", err) + } + + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + // 1 init + 3 SDK events = 4 lines. + if len(lines) != 4 { + t.Fatalf("got %d lines, want 4:\n%s", len(lines), out.String()) + } + // Each line must be valid JSON. + for i, line := range lines { + var obj map[string]any + if err := json.Unmarshal([]byte(line), &obj); err != nil { + t.Fatalf("line %d not valid JSON: %v\n %s", i+1, err, line) + } + } + + // First line: CLI-injected init event. + var initLine map[string]any + if err := json.Unmarshal([]byte(lines[0]), &initLine); err != nil { + t.Fatalf("line 1 (init) not JSON: %v", err) + } + if initLine["type"] != "init" { + t.Errorf("first line type=%v, want init", initLine["type"]) + } + if initLine["agent_id"] != "ag_x" { + t.Errorf("first line agent_id=%v, want ag_x", initLine["agent_id"]) + } + + // Second line: tool_call event (SDK passthrough). + var second map[string]any + if err := json.Unmarshal([]byte(lines[1]), &second); err != nil { + t.Fatalf("line 2 not JSON: %v", err) + } + if second["response_type"] != string(sdk.AgentResponseTypeToolCall) { + t.Errorf("second event response_type=%v, want %s", second["response_type"], sdk.AgentResponseTypeToolCall) + } + // Third line: answer event. + var third map[string]any + if err := json.Unmarshal([]byte(lines[2]), &third); err != nil { + t.Fatalf("line 3 not JSON: %v", err) + } + if third["response_type"] != string(sdk.AgentResponseTypeAnswer) { + t.Errorf("third event response_type=%v, want %s", third["response_type"], sdk.AgentResponseTypeAnswer) + } + // Fourth line: done event. + var fourth map[string]any + if err := json.Unmarshal([]byte(lines[3]), &fourth); err != nil { + t.Fatalf("line 4 not JSON: %v", err) + } + if fourth["done"] != true { + t.Errorf("fourth event done=%v, want true", fourth["done"]) + } +} + +func TestSessionAsk_RequiresAgentFlag(t *testing.T) { + // Build the real cobra command with a nil factory — flag parsing happens + // before RunE so the factory is never dereferenced for this test. + f := &cmdutil.Factory{} + cmd := NewCmdAsk(f) + // Redirect output to discard cobra error messages. + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetErr(&buf) + // Execute without --agent: cobra should refuse with exit-code 2. + cmd.SetArgs([]string{"some question"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error when --agent is missing, got nil") + } + // Cobra wraps required-flag errors; the message should mention the flag. + if !strings.Contains(err.Error(), "agent") { + t.Errorf("error should mention 'agent' flag, got: %v", err) + } +} + +// TestSessionAsk_NDJSON_IncludesReferencesViaSDKEvent verifies that references +// emitted by the SDK appear as passthrough NDJSON lines (not lost). +func TestSessionAsk_NDJSON_IncludesReferencesViaSDKEvent(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &scriptedAskSvc{ + events: []*sdk.AgentStreamResponse{ + answerEvent("Hello world."), + referencesEvent([]*sdk.SearchResult{{KnowledgeID: "k1", KnowledgeTitle: "Doc 1"}}), + doneEvent(), + }, + } + opts := &AskOptions{AgentID: "ag_x", Query: "ping"} + if err := runAsk(context.Background(), opts, jsonOpts(), svc); err != nil { + t.Fatalf("runAsk: %v", err) + } + + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + // 1 init + 3 SDK events = 4 lines. + if len(lines) != 4 { + t.Fatalf("got %d NDJSON lines, want 4:\n%s", len(lines), out.String()) + } + // init line. + var init map[string]any + if err := json.Unmarshal([]byte(lines[0]), &init); err != nil { + t.Fatalf("line 0 not JSON: %v", err) + } + if init["type"] != "init" { + t.Errorf("line 0 type: got %v, want init", init["type"]) + } + // references line is the third SDK event (lines[3] = index 3). + var refsLine map[string]any + if err := json.Unmarshal([]byte(lines[2]), &refsLine); err != nil { + t.Fatalf("references line not JSON: %v", err) + } + if refsLine["response_type"] != string(sdk.AgentResponseTypeReferences) { + t.Errorf("expected references event at line 3, got response_type=%v", refsLine["response_type"]) + } +} diff --git a/cli/cmd/session/session.go b/cli/cmd/session/session.go index 0ee63c2f..920bcef1 100644 --- a/cli/cmd/session/session.go +++ b/cli/cmd/session/session.go @@ -1,9 +1,9 @@ // Package sessioncmd holds `weknora session` command tree (list / view / -// delete) for chat history. +// delete / ask) for chat history and agent invocation. // // Package name `sessioncmd` (not `session`) so callers can `import sdk // "github.com/Tencent/WeKnora/client"` and use `sdk.Session` without -// shadowing - same hygiene as `contextcmd`. +// shadowing - same hygiene as `profilecmd`. package sessioncmd import ( @@ -17,11 +17,10 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "session", Short: "Manage chat sessions", - Args: cobra.NoArgs, - Run: func(c *cobra.Command, _ []string) { _ = c.Help() }, } cmd.AddCommand(NewCmdList(f)) cmd.AddCommand(NewCmdView(f)) cmd.AddCommand(NewCmdDelete(f)) + cmd.AddCommand(NewCmdAsk(f)) return cmd }