From d3c03b2f34a596a5530e1b8a2cd768bb3760c8e4 Mon Sep 17 00:00:00 2001 From: nullkey Date: Thu, 2 Jul 2026 15:53:51 +0800 Subject: [PATCH] feat(cli)!: v0.10 reliability, agent-UX, and command-surface hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hardening + finalization pass over the agent-first CLI: correctness fixes, richer machine-readable signals, flag/naming consistency, and a symmetric config surface. Pre-1.0, so it includes breaking renames. Correctness: - agent update: resolve/validate --model/--rerank-model (was storing a bogus name verbatim, corrupting config.model_id). - doctor: honor WEKNORA_HOST / WEKNORA_API_KEY (headless path no longer reports "no host configured"). - session ask / MCP session_ask: text answer was empty on non-TTY — the agent stream sets Done=true on an intermediate agent_query frame before the answer, and AgentAccumulator treated the first Done as terminal. Terminate on the `complete` event (new sdk.AgentResponseTypeComplete), not a per-frame Done. - batch exit codes: any per-item failure collapses to operation.failed (exit 1), including `doc upload --recursive` partial failures — a permanent per-file failure (e.g. a duplicate) no longer surfaces as a retryable exit 7 an agent would loop on; per-item typed errors stay in the envelope. Agent-first signals & discovery: - error.exit_code in the envelope (type + exit_code disambiguate the input.invalid_argument exit-2-vs-5 split in one JSON read). - meta.hint on empty content search and on draft doc create; doc wait fails fast on a never-parsing draft instead of hanging to --timeout. - retrieval-readiness is visible in the natural flow: kb status / kb check emit retrieval_ready, and kb create hints the fix when no embedding model is bound — an unconfigured KB no longer looks silently healthy. - schema contract completeness: every leaf declares output + >=1 example (drift-guarded); output strings match the meta actually emitted; chunk list and search docs now emit meta.total_count (both previously dropped it). - schema tolerates a quoted multi-word command label; zero-state auth — and `link` with no profile — point at profile setup / the headless WEKNORA_KB_ID path instead of looping on `auth login`. - id-addressed reads tolerate a redundant --kb (doc view/wait, chunk list/view accept and ignore it, declared in schema) so a carried-over --kb doesn't exit 2; streaming commands warn that --jq does not apply to an NDJSON stream. - keep JSON-always as the default; --jq hints spell out the .data path. Consistency & gating: - doc create: drop the deprecated --name alias (--title only; pre-1.0 break). - chunk list --limit aligned to 1..10000; model list --limit/-L with has_more/total_count; api write-gates -X PUT/PATCH (exit 10); skills install expands a leading ~. - docs corrected: search docs / doc list --keyword help is case-insensitive (server does LOWER LIKE); AGENTS.md risk-action list (no phantom kb.init; add model.update / kb.config.set) and batch example (failed item carries `error`); session resume --message id comes from `message list`, not the stream. - auth/profile ergonomics: env credentials are now first-class — `auth token` prints the active WEKNORA_API_KEY / WEKNORA_TOKEN, and auth login/logout/refresh give an env-aware message instead of looping on "run auth login". `auth logout` clears credentials but keeps the profile registered (host preserved for re-login); deleting a profile is `profile remove`'s job (clean logout/remove separation, matching gh / lark). Config surface (symmetric read/write, in-place model edits): - kb config now returns a secret-free KBModelConfigView (was {}); `kb config` reads, new `kb config set` writes; `kb init` removed (misnomer). - kb create --chat-model: retrieval-ready in one step. - model update: edit a model in place (id preserved, references survive) — rotate --api-key-stdin, change base-url / display-name / etc. - session continue-stream renamed to session resume. Docs: AGENTS.md is the single wire-contract source; CHANGELOG slimmed; stale kb-init / continue-stream references removed; skill wire-vocab guard extended. AGENTS.md / weknora-shared SKILL document retrieval_ready (a KB needs an embedding model to be searchable), that --jq does not apply to NDJSON streams, and the env-credential-first auth path; the KB quickstart example now creates a retrieval-ready KB. --- cli/AGENTS.md | 73 +++---- cli/CHANGELOG.md | 16 ++ cli/README.md | 29 ++- cli/cmd/agent/create.go | 2 +- cli/cmd/agent/delete.go | 1 + cli/cmd/agent/edit.go | 18 ++ cli/cmd/agent/edit_test.go | 36 ++++ cli/cmd/agenthelp_coverage_test.go | 64 ++++++ cli/cmd/api/api.go | 35 +++- cli/cmd/api/api_test.go | 52 +++++ cli/cmd/auth/login.go | 14 +- cli/cmd/auth/logout.go | 27 ++- cli/cmd/auth/logout_test.go | 21 +- cli/cmd/auth/refresh.go | 12 ++ cli/cmd/auth/token.go | 23 ++- cli/cmd/chunk/delete.go | 1 + cli/cmd/chunk/list.go | 20 +- cli/cmd/chunk/list_test.go | 28 ++- cli/cmd/chunk/view.go | 1 + cli/cmd/doc/create.go | 31 +-- cli/cmd/doc/create_test.go | 19 +- cli/cmd/doc/download.go | 5 + cli/cmd/doc/list.go | 4 +- cli/cmd/doc/reparse.go | 1 + cli/cmd/doc/update.go | 1 + cli/cmd/doc/upload_recursive.go | 17 +- cli/cmd/doc/upload_recursive_test.go | 11 +- cli/cmd/doc/view.go | 1 + cli/cmd/doc/wait.go | 9 + cli/cmd/doc/wait_test.go | 27 +++ cli/cmd/doctor/doctor.go | 29 ++- cli/cmd/doctor/doctor_test.go | 52 +++++ cli/cmd/dryrun_coverage_test.go | 28 ++- cli/cmd/kb/check.go | 24 ++- cli/cmd/kb/check_test.go | 5 +- cli/cmd/kb/config.go | 62 ++++-- cli/cmd/kb/{init.go => config_set.go} | 88 ++++---- .../kb/{init_test.go => config_set_test.go} | 52 ++--- cli/cmd/kb/config_test.go | 42 +++- cli/cmd/kb/create.go | 33 ++- cli/cmd/kb/create_test.go | 44 ++++ cli/cmd/kb/delete.go | 1 + cli/cmd/kb/edit.go | 1 + cli/cmd/kb/kb.go | 3 +- cli/cmd/kb/status.go | 30 ++- cli/cmd/kb/status_test.go | 27 ++- cli/cmd/link/link.go | 10 +- cli/cmd/link/unlink.go | 3 + cli/cmd/mcp/serve.go | 3 + cli/cmd/message/search.go | 2 +- cli/cmd/model/create.go | 4 +- cli/cmd/model/list.go | 41 +++- cli/cmd/model/list_test.go | 55 ++++- cli/cmd/model/model.go | 3 +- cli/cmd/model/update.go | 189 ++++++++++++++++++ cli/cmd/model/update_test.go | 81 ++++++++ cli/cmd/profile/add.go | 3 + cli/cmd/profile/remove.go | 1 + cli/cmd/root.go | 2 +- cli/cmd/schema.go | 10 + cli/cmd/schema_test.go | 19 ++ cli/cmd/search/chunks.go | 2 +- cli/cmd/search/docs.go | 16 +- cli/cmd/search/docs_test.go | 23 +++ cli/cmd/search/search.go | 13 ++ cli/cmd/session/ask.go | 2 +- cli/cmd/session/ask_test.go | 3 + cli/cmd/session/list.go | 2 +- .../session/{continue_stream.go => resume.go} | 98 ++++----- ...continue_stream_test.go => resume_test.go} | 70 ++++--- cli/cmd/session/session.go | 4 +- cli/cmd/session/stop.go | 8 +- cli/cmd/session/tool_approval.go | 4 +- cli/cmd/skills/skills.go | 27 ++- cli/cmd/skills/skills_test.go | 23 +++ cli/internal/cmdutil/batch.go | 14 +- cli/internal/cmdutil/batch_test.go | 31 ++- cli/internal/cmdutil/errors.go | 12 +- cli/internal/cmdutil/errors_retry_test.go | 30 +++ cli/internal/cmdutil/factory.go | 17 +- cli/internal/cmdutil/factory_test.go | 7 + cli/internal/cmdutil/format.go | 14 +- cli/internal/cmdutil/format_test.go | 5 +- cli/internal/cmdutil/profilename_test.go | 2 +- cli/internal/cmdutil/risk.go | 2 +- cli/internal/mcp/tools.go | 7 +- cli/internal/output/envelope.go | 15 +- cli/internal/output/ndjson_stream.go | 2 +- cli/internal/sse/accumulator.go | 10 - cli/scripts/check-secret-tokens.sh | 3 +- cli/scripts/check-skill-wire-vocab.sh | 4 +- cli/skills/weknora-rag-search/SKILL.md | 2 +- .../weknora-rag-search/references/chat.md | 4 +- cli/skills/weknora-shared/SKILL.md | 47 +++-- client/initialization.go | 78 +++++++- 95 files changed, 1711 insertions(+), 436 deletions(-) rename cli/cmd/kb/{init.go => config_set.go} (58%) rename cli/cmd/kb/{init_test.go => config_set_test.go} (58%) create mode 100644 cli/cmd/model/update.go create mode 100644 cli/cmd/model/update_test.go rename cli/cmd/session/{continue_stream.go => resume.go} (67%) rename cli/cmd/session/{continue_stream_test.go => resume_test.go} (72%) diff --git a/cli/AGENTS.md b/cli/AGENTS.md index aa5bddce2..8dee3ff16 100644 --- a/cli/AGENTS.md +++ b/cli/AGENTS.md @@ -36,12 +36,10 @@ fields are `omitempty` — they only appear when populated: ``` `data` is omitted on mutation-only success (no payload). `meta` carries list -counters (`count`, `has_more`) and batch successes/failures, and is omitted -when empty. `meta.next_cursor`, `meta.total_count`, and `meta.request_id` are +counters (`count`, `has_more`, `total_count`) and batch successes/failures, and is omitted +when empty. `meta.next_cursor` and `meta.request_id` are reserved — not currently populated; planned for v0.8 when the SDK exposes -pagination cursors and response headers. `_notice` is reserved — open-map -infrastructure is in place for deprecation / version_skew / security notices; -the field is omitted until a producer is wired in v0.8. `profile` echoes the +pagination cursors and response headers. `profile` echoes the resolved profile name and is omitted when no profile is configured. ### Stderr (error path) @@ -55,27 +53,29 @@ Errors emit an error envelope on stderr (`--format json`) or prose "error": { "type": "auth.unauthenticated", "message": "fetch current user: HTTP error 401", + "exit_code": 3, "hint": "run `weknora auth login`", - "retry_command": "weknora auth login", + "retry_argv": ["weknora", "auth", "login"], "retry_after_seconds": 0, "risk": {"level": "destructive", "action": "noun.verb"}, "detail": {} - }, - "_notice": {} + } } ``` `type` is the typed code (see [Error code reference](#error-code-reference) -below). `hint` is prose; `retry_command` is the suggested next argv (single -shell-escaped string). For non-destructive errors agents may execute it; on -exit-10 (`input.confirmation_required`) it is informational only — the human -must approve the destructive write explicitly. See "Exit-10 anti-patterns" for -details. Note: tokens in `retry_command` are built via `fmt.Sprintf` with -user-supplied IDs unquoted — callers that auto-execute must shell-quote each -token (emitting as a JSON array is planned for v0.8). -`retry_after_seconds` mirrors HTTP `Retry-After`. `risk` tags high-risk writes. -`detail` carries structured per-error context (e.g. `unknown_subcommand`'s -`available[]` list). +below). `hint` is prose; `retry_argv` is the suggested next command as a JSON +array of argv tokens — exec it directly (no shell-splitting or quoting needed). +For non-destructive errors agents may execute it; on exit-10 +(`input.confirmation_required`) it is informational only — the human must +approve the destructive write explicitly. See "Exit-10 anti-patterns" for +details. +`exit_code` embeds the process exit code (§2) so a single JSON read is +authoritative without observing `$?` — it also disambiguates the two +`input.invalid_argument` cases (a cobra parse error is exit 2, a typed-value +error is exit 5). `retry_after_seconds` mirrors HTTP `Retry-After`. `risk` tags +high-risk writes. `detail` carries structured per-error context (e.g. +`unknown_subcommand`'s `available[]` list). ### Buffered JSON and NDJSON streams (chat / session ask) @@ -88,7 +88,7 @@ text` renders the same projection live. `init` event at the head and passes all subsequent SDK events through verbatim: ``` -{"type":"init","session_id":"...","kb_id":"...","profile":"...","agent_id":"..."} +{"type":"init","session_id":"...","kb_id":"...","profile":"..."} // chat: kb_id ; session ask: agent_id instead {"response_type":"thinking","content":"..."} {"response_type":"answer","content":"Hello"} {"response_type":"tool_call","tool_calls":[...]} @@ -99,15 +99,6 @@ MCP `chat` / `session_ask` return the same `events` shape and accept `reference` / `verbose` booleans. NDJSON ignores both presentation flags and always stays raw. -### `_notice` evolution policy - -`_notice` is an open map. New keys are **additive non-breaking**; agents MUST -ignore unknown keys. v0.7 reserves three keys: `deprecation` / `version_skew` / -`security`. New keys follow snake_case convention. The `_notice` field is -currently always empty — producer wiring is planned for v0.8 when the SDK -exposes version metadata. The wire infrastructure is in place so adding a -producer in v0.8 will not change the envelope shape. - ### CLI vs server SDK contract boundary CLI 1.0 contract covers: @@ -165,12 +156,12 @@ is or isn't aligned with. | **WeKnora** | DELETE triggers exit-10 (`input.confirmation_required`); user bypasses with `-y/--yes` | | **Rationale** | DELETE is irreversible. Most raw-API CLI commands rely on restricted credentials for safety, but self-hosted deployments may not have restricted-credential infrastructure available. Defensive default because agents are common consumers. | -### 3. `retry_command` distinct from `hint` +### 3. `retry_argv` distinct from `hint` | | | |---|---| -| **WeKnora** | two separate fields: `retry_command` (suggested next argv, directly-executable for non-destructive errors; informational only on exit-10) + `hint` (prose) | -| **Rationale** | Agents don't regex-extract argv from prose — known fragility. Trade-off: one extra envelope field. On exit-10, the user must approve the destructive write; agents surface `retry_command` for human review, not auto-execution. | +| **WeKnora** | two separate fields: `retry_argv` (suggested next command as a JSON argv array, directly-executable for non-destructive errors; informational only on exit-10) + `hint` (prose) | +| **Rationale** | Agents don't regex-extract argv from prose — known fragility. An argv array is exec-ready with no shell-splitting or quoting. Trade-off: one extra envelope field. On exit-10, the user must approve the destructive write; agents surface `retry_argv` for human review, not auto-execution. | ### 4. NDJSON event stream has no envelope wrapping @@ -438,7 +429,7 @@ For common retry patterns, AI agents can hardcode: Exit code 10 (`input.confirmation_required`) marks a destructive write where the CLI refused to proceed without explicit user approval. The retry envelope includes -`retry_command` showing the exact argv that would proceed. AI agents must NEVER +`retry_argv` showing the exact argv that would proceed. AI agents must NEVER auto-retry this exit code — every exit 10 is a user-in-the-loop decision. **Don't do these:** @@ -446,7 +437,7 @@ auto-retry this exit code — every exit 10 is a user-in-the-loop decision. 1. **Auto-add `-y/--yes` and retry.** The flag exists for the user, not the agent. Surface the exit-10 envelope to the user verbatim and wait for explicit go-ahead. -2. **Parse the retry_command and run it.** The retry_command is *informational* -- +2. **Parse the retry_argv and run it.** The retry_argv is *informational* -- showing what *would* execute. Running it without user input collapses two steps the user is supposed to see. @@ -465,7 +456,7 @@ auto-retry this exit code — every exit 10 is a user-in-the-loop decision. ## Stream recovery -The `weknora session continue-stream --message ` command resumes an SSE event stream for an existing assistant message. Use cases: network-blip recovery, long-running agent invocation polling, completed-stream inspection. +The `weknora session resume --message ` command resumes an SSE event stream for an existing assistant message. Use cases: network-blip recovery, long-running agent invocation polling, completed-stream inspection. ### Server semantics: replay-from-0, not cursor-resume @@ -474,7 +465,7 @@ The server **replays all stored events from the start** of the assistant message ### Agent contract 1. **Dedupe by message_id** (or maintain a per-message event hash set). Naively processing all received events causes duplicate side effects (re-running tool calls, re-rendering answers). -2. **Capture message_id from the init event** of the original `chat` or `session ask` invocation — the CLI injects `{"event":"init", "session_id":"...", "message_id":"..."}` as the first NDJSON line. +2. **Get the assistant `message_id` from `weknora message list --session `** (a live stream's `assistant_message_id` is not resumable once the message persists). `session resume` then injects `{"type":"init", "session_id":"...", "message_id":"...", "profile":"..."}` as the first NDJSON line. 3. **Handle `local.sse_stream_aborted` typed error**: server-side buffer expired (TTL exceeded) or process restarted (memory mode). The message is no longer recoverable; restart the original query. ### Server-side buffer TTL @@ -484,11 +475,11 @@ The server **replays all stored events from the start** of the assistant message | `STREAM_MANAGER_TYPE=redis` | **1 hour** (server-side; not configurable from the CLI) | | `STREAM_MANAGER_TYPE=memory` (default) | **Process lifetime** (server restart = data loss; no explicit cleanup logic) | -After TTL, `weknora session continue-stream` returns the typed error `local.sse_stream_aborted`, which maps to exit code 1 per the Error code reference. +After TTL, `weknora session resume` returns the typed error `local.sse_stream_aborted`, which maps to exit code 1 per the Error code reference. ## Dry-run contract -The `--dry-run` flag is available on every mutation cobra command (`kb create/edit/delete`, `agent create/edit/delete`, `doc create/upload/fetch/delete`, `chunk delete`, `session delete`, `auth refresh/logout`, `link/unlink`, `profile add/remove`) and on `weknora api` (POST/PUT/PATCH/DELETE only; GET rejected with FlagError exit 2). +The `--dry-run` flag is available on every mutation cobra command (`kb create/update/delete`, `agent create/update/delete`, `doc create/upload/fetch/delete`, `chunk delete`, `session delete`, `auth refresh/logout`, `link/unlink`, `profile add/remove`) and on `weknora api` (POST/PUT/PATCH/DELETE only; GET rejected with FlagError exit 2). ### Envelope shape on dry-run @@ -516,8 +507,8 @@ The dry-run path is **offline** — no SDK calls, no Factory.Client() init, no R | `--dry-run` + `-y` | Equivalent to single `--dry-run`; `-y` is no-op (dry-run early-exits before ConfirmDestructive) | | `--dry-run` + `api -X GET` (or default GET) | FlagError exit 2: "--dry-run requires explicit -X POST/PUT/PATCH/DELETE; default GET is read-only with no side effect to preview" | | `--dry-run` + `--jq ` | jq applied to envelope output normally | -| `--dry-run` + `kb edit my-kb` | plan.args contains user raw input (NOT ResolveKB-resolved); agent verifies kb name correctness | -| `--dry-run` + fetch-then-update (`kb edit / agent edit`) | plan.args contains user-explicit fields ONLY; agent infers server-side fetch-then-update preserves unmentioned fields | +| `--dry-run` + `kb update my-kb` | plan.args contains user raw input (NOT ResolveKB-resolved); agent verifies kb name correctness | +| `--dry-run` + fetch-then-update (`kb update / agent update`) | plan.args contains user-explicit fields ONLY; agent infers server-side fetch-then-update preserves unmentioned fields | | `--dry-run` + body containing secrets (`--input` payload) | **plan.body echoes the full body to stdout** so the agent can verify what would be sent; avoid piping secret-bearing bodies through dry-run for inspection | ### Streaming commands explicitly excluded @@ -532,7 +523,7 @@ echo '{"query":"...","kb":"..."}' | weknora api -X POST /api/v1/sessions//ag Agents see the same `risk.action` string (in the form `noun.verb`) on three independent surfaces: -1. **Error envelope** — `envelope.error.risk.action` on an exit-10 confirmation-required error, so the agent can decide whether to escalate to the user. 11 unique values: `kb.delete`, `kb.edit`, `agent.delete`, `agent.edit`, `doc.delete`, `doc.delete_all`, `session.delete`, `chunk.delete`, `profile.remove`, `auth.logout`, `api.delete`. +1. **Error envelope** — `envelope.error.risk.action` on an exit-10 confirmation-required error, so the agent can decide whether to escalate to the user. 11 unique values: `kb.delete`, `kb.update`, `agent.delete`, `agent.update`, `doc.delete`, `doc.delete_all`, `session.delete`, `chunk.delete`, `profile.remove`, `auth.logout`, `api.delete`. 2. **Help text** — a `Risk: (destructive)` line prepended to the top of `--help` output on the 9 destructive cobra commands. `weknora api` is intentionally excluded: it is a generic HTTP passthrough whose risk depends on the method, so a static Risk: line would mislead for non-DELETE methods. @@ -582,7 +573,7 @@ Required-input idioms in this codebase: - Positional required: `cobra.ExactArgs(N)` or `cobra.MinimumNArgs(1)` - Flag required: `cmd.MarkFlagRequired("flag")` -- Custom required (e.g., `agent edit` needs at-least-one-edit-flag): RunE-level validation that returns `input.invalid_argument` +- Custom required (e.g., `agent update` needs at-least-one-edit-flag): RunE-level validation that returns `input.invalid_argument` - Mutex: `cmd.MarkFlagsMutuallyExclusive("a", "b")` Reasons hard-required-flags is the v0.5+ default: diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 997346053..6c7692f92 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -20,6 +20,10 @@ CLI history before v0.3 is recorded in the project root - JSON, text, and MCP chat/session output hide reasoning, tools, lifecycle frames, and references by default. `--reference` adds bounded `kb_id` / `chunk_id` / `parent_chunk_id` indexes; `--verbose` adds execution events. +- `session continue-stream` renamed to `session resume`. +- `kb init` renamed to `kb config set`. +- Error envelope: `retry_command` (a shell string) replaced by `retry_argv` + (a directly-executable argv array — no shell-splitting or quoting). ### Added - `chat` / `session ask --reference` includes indexed citations, while @@ -27,6 +31,18 @@ CLI history before v0.3 is recorded in the project root `session_ask` expose the same controls through `reference` / `verbose` inputs. - Buffered chat/session errors include the auto-created `session_id` in `error.detail` so interrupted sessions remain recoverable. +- `error.exit_code` embeds the process exit code in the JSON error envelope, + disambiguating the two `input.invalid_argument` cases (parse error → 2, + typed-value error → 5). +- `kb status` / `kb check` / `kb create` report `retrieval_ready` (whether an + embedding model is bound); `kb create` hints the fix when it is false. +- `model create` / `model update` / `model delete` (`update` rotates key / + base-url in place, preserving the id). +- Stateless env-credential auth: `WEKNORA_API_KEY` / `WEKNORA_TOKEN` + + `WEKNORA_HOST`, a zero-disk path for headless / agent use. `auth logout` now + keeps the profile registered (use `profile remove` to delete it entirely). +- `meta.total_count` on paginated list / search output (full result size before + client-side `--limit` truncation). ### Changed - JSON, text, and MCP now share one event projector and filtering policy. diff --git a/cli/README.md b/cli/README.md index a78c85b8f..247937a9a 100644 --- a/cli/README.md +++ b/cli/README.md @@ -17,16 +17,21 @@ Available Commands: chat Ask a streaming RAG question against a knowledge base chunk Manage document chunks (RAG retrieval debug) completion Generate the autocompletion script for the specified shell - profile Manage CLI profiles (named connection targets) + config Inspect the CLI's resolved configuration doc Manage documents in a knowledge base doctor Run 4 self-checks: base URL, auth, server version, credential storage + exit-codes Exit code matrix and the agent action for each help Help about any command kb Manage knowledge bases link Bind the current directory to a knowledge base mcp Run weknora as a Model Context Protocol server message Inspect and manage messages inside chat sessions + model Manage models (list / view / create / update / delete) + profile Manage CLI profiles (named connection targets) + schema Machine-readable contract for a command (or the whole surface) search Search across chunks, knowledge bases, documents, or sessions - session Manage chat sessions (incl. tool-approval resolve) + session Manage chat sessions + skills List and install the bundled Agent Skills unlink Remove the directory's knowledge-base binding version Show CLI build metadata ``` @@ -100,7 +105,7 @@ weknora message search "retry policy" # cross-session Q&A r # 11. Resolve a pending tool approval (agent run blocked on approval event) weknora session tool-approval resolve pend_xxx -y # approve (after user go-ahead) -weknora session continue-stream sess_abc --message msg_xyz # resume the blocked stream +weknora session resume sess_abc --message msg_xyz # resume the blocked stream # 12. Health & verification verbs weknora kb status kb_abc # fast snapshot: reachable / counts / processing flag (1 HTTP) @@ -198,7 +203,9 @@ weknora auth logout --all Designed to be AI-agent-first. Stable across minor releases; breaking changes announced in the changelog and the corresponding -`weknora --version` bump. +`weknora --version` bump. This section is the human overview; the complete, +authoritative contract (envelope field stability, error taxonomy, streaming, +confirmation and dry-run protocols) lives in **[AGENTS.md](AGENTS.md)**. ### Streams @@ -244,10 +251,10 @@ auth.unauthenticated: fetch current user: HTTP error 401: ... hint: run `weknora auth login` ``` -The full code registry is in `cli/internal/cmdutil/errors.go` -(`AllCodes()`). Code namespaces: `auth.*` / `resource.*` / `input.*` / -`server.*` / `network.*` / `local.*` / `mcp.*` / `operation.*` (CLI-level -wait/poll outcomes: `operation.timeout`, `operation.failed`, `operation.cancelled`). +Under `--format json` the same failure is the typed error envelope on stderr +(`{ok:false, error:{type, exit_code, hint?, retry_argv?, …}}`) — see +[AGENTS.md §1.4](AGENTS.md) for the field-by-field contract and the full code +taxonomy. ### Exit codes @@ -361,7 +368,7 @@ weknora api /api/v1/knowledge-bases --dry-run ## Resuming streams -The `weknora session continue-stream` command resumes an SSE event stream for an existing assistant message. Useful for network-blip recovery or polling long-running agent invocations: +The `weknora session resume` command resumes an SSE event stream for an existing assistant message. Useful for network-blip recovery or polling long-running agent invocations: ```bash # Original streaming call captures session_id + message_id from init event: @@ -371,7 +378,7 @@ weknora session ask "..." --agent ag_xxxx --format ndjson | tee /tmp/stream.ndjs # [network blip] # Resume the same stream: -weknora session continue-stream sess_abc --message msg_xyz +weknora session resume sess_abc --message msg_xyz # Server REPLAYS all stored events from the start, then tails new ones. # Agent must dedupe (by message_id or event hash) to avoid double-processing. ``` @@ -386,7 +393,7 @@ An agent run may pause the stream on a tool-approval event until a human approve weknora session tool-approval resolve pend_xxx -y # approve # weknora session tool-approval resolve pend_xxx --reject --reason "..." -y # reject # 3. Resume the stream — server replays + tails from where the run was blocked. -weknora session continue-stream sess_abc --message msg_xyz +weknora session resume sess_abc --message msg_xyz ``` Pass `--modified-args '{"key":"value"}'` to replace tool arguments on approve (must be a non-empty JSON object). Never auto-pass `-y` — the approval is the exit-10 human-in-the-loop gate. diff --git a/cli/cmd/agent/create.go b/cli/cmd/agent/create.go index 6c451fb5f..3536d1bb4 100644 --- a/cli/cmd/agent/create.go +++ b/cli/cmd/agent/create.go @@ -167,7 +167,7 @@ func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { return err } // Resolve --model / --rerank-model (id or name) and validate they - // exist, matching the id-or-name policy of `kb init`. A bogus name + // exist, matching the id-or-name policy of `kb config set`. A bogus name // fails fast here instead of creating an agent whose model never // resolves at run time. if opts.Model, err = cmdutil.ResolveModelRef(cmd.Context(), cli, opts.Model, "KnowledgeQA"); err != nil { diff --git a/cli/cmd/agent/delete.go b/cli/cmd/agent/delete.go index 361640bd6..6192df65f 100644 --- a/cli/cmd/agent/delete.go +++ b/cli/cmd/agent/delete.go @@ -93,6 +93,7 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "permanently delete a custom agent", RequiredFlags: []string{" (positional)"}, + Output: "envelope.data is {id, deleted:true}", Examples: []string{ "weknora agent delete ag_abc -y", "weknora agent delete ag_abc -y --format json", diff --git a/cli/cmd/agent/edit.go b/cli/cmd/agent/edit.go index 3620cbe1d..dcd1cc91b 100644 --- a/cli/cmd/agent/edit.go +++ b/cli/cmd/agent/edit.go @@ -21,6 +21,10 @@ import ( type EditService interface { GetAgent(ctx context.Context, id string) (*sdk.Agent, error) UpdateAgent(ctx context.Context, id string, req *sdk.UpdateAgentRequest) (*sdk.Agent, error) + // ListModels backs --model / --rerank-model id-or-name resolution so a + // bogus name fails fast instead of clobbering config.model_id with an + // unresolvable string (which never resolves at run time). + ListModels(ctx context.Context) ([]sdk.Model, error) } // EditOptions captures the surgical flag state. Both string fields and @@ -329,6 +333,20 @@ func runEdit(ctx context.Context, opts *EditOptions, fopts *cmdutil.FormatOption opts.SystemPrompt = strings.TrimSpace(string(body)) } + // Resolve --model / --rerank-model (id or name) and validate they exist, + // mirroring agent create. Without this a bogus name is stored verbatim as + // config.model_id and the agent silently never resolves at run time. + if opts.flags.modelSet { + if opts.Model, err = cmdutil.ResolveModelRef(ctx, svc, opts.Model, "KnowledgeQA"); err != nil { + return err + } + } + if opts.flags.rerankModelSet { + if opts.RerankModel, err = cmdutil.ResolveModelRef(ctx, svc, opts.RerankModel, "Rerank"); err != nil { + return err + } + } + // Compute KB list from current + add/remove with a stderr warning when // the same id appears in both flags (net no-op, still idempotent). kbs := computeKBList(base.KnowledgeBases, opts.AddKBs, opts.RemoveKBs) diff --git a/cli/cmd/agent/edit_test.go b/cli/cmd/agent/edit_test.go index 058c41373..b76881d66 100644 --- a/cli/cmd/agent/edit_test.go +++ b/cli/cmd/agent/edit_test.go @@ -28,6 +28,8 @@ type fakeEditSvc struct { updateResp *sdk.Agent updateErr error updateCalls int + models []sdk.Model + modelsErr error } func (f *fakeEditSvc) GetAgent(_ context.Context, _ string) (*sdk.Agent, error) { @@ -41,6 +43,40 @@ func (f *fakeEditSvc) UpdateAgent(_ context.Context, id string, req *sdk.UpdateA return f.updateResp, f.updateErr } +func (f *fakeEditSvc) ListModels(_ context.Context) ([]sdk.Model, error) { + return f.models, f.modelsErr +} + +func TestEdit_ModelName_ResolvedToID(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{ID: "ag_abc", Name: "A", Config: &sdk.AgentConfig{ModelID: "old-id"}}, + updateResp: &sdk.Agent{ID: "ag_abc"}, + models: []sdk.Model{{ID: "m-real", Name: "good-llm", Type: "KnowledgeQA"}}, + } + opts := &EditOptions{AgentID: "ag_abc", Model: "good-llm", flags: editFlagSet{modelSet: true}} + require.NoError(t, runEdit(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc)) + require.NotNil(t, svc.updateReq) + require.NotNil(t, svc.updateReq.Config) + assert.Equal(t, "m-real", svc.updateReq.Config.ModelID, "--model name must resolve to the model id") +} + +func TestEdit_BogusModelName_RejectedNoWrite(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeEditSvc{ + getResp: &sdk.Agent{ID: "ag_abc", Name: "A", Config: &sdk.AgentConfig{ModelID: "old-id"}}, + updateResp: &sdk.Agent{ID: "ag_abc"}, + models: []sdk.Model{{ID: "m-real", Name: "good-llm", Type: "KnowledgeQA"}}, + } + opts := &EditOptions{AgentID: "ag_abc", Model: "totally-bogus-model-xyz", flags: editFlagSet{modelSet: true}} + err := runEdit(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc) + require.Error(t, err, "a --model name matching no model must fail") + var e *cmdutil.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, cmdutil.CodeResourceNotFound, e.Code) + assert.Equal(t, 0, svc.updateCalls, "must not write an agent with an unresolvable model") +} + func TestEdit_FetchThenUpdate_PreservesUntouchedFields(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeEditSvc{ diff --git a/cli/cmd/agenthelp_coverage_test.go b/cli/cmd/agenthelp_coverage_test.go index 9c6210761..18eee5be8 100644 --- a/cli/cmd/agenthelp_coverage_test.go +++ b/cli/cmd/agenthelp_coverage_test.go @@ -43,3 +43,67 @@ func TestEveryLeafCommandHasAgentHelp(t *testing.T) { missing, len(missing)) } } + +// renderAgentHelp runs a leaf's help under WEKNORA_AGENT_HELP=1 and decodes the +// machine blob (used_for / output / examples) an agent would read. +func renderAgentHelp(t *testing.T, c *cobra.Command) struct { + UsedFor string `json:"used_for"` + Output string `json:"output"` + Examples []string `json:"examples"` +} { + t.Helper() + var buf bytes.Buffer + c.SetOut(&buf) + c.Help() + var ah struct { + UsedFor string `json:"used_for"` + Output string `json:"output"` + Examples []string `json:"examples"` + } + if err := json.Unmarshal(buf.Bytes(), &ah); err != nil { + t.Fatalf("%s: agent-help is not JSON: %v", c.CommandPath(), err) + } + return ah +} + +// TestEveryLeafCommandDeclaresOutput enforces that every leaf command tells an +// agent what its stdout carries. Even side-effect commands describe their +// envelope (e.g. deletes emit {id, deleted:true}); an empty Output is a contract +// gap, not a valid state. Sibling drift guard to the agent-help test above. +func TestEveryLeafCommandDeclaresOutput(t *testing.T) { + t.Setenv("WEKNORA_AGENT_HELP", "1") + root := NewRootCmd(cmdutil.New()) + + var missing []string + eachLeafCommand(root, func(c *cobra.Command) { + if renderAgentHelp(t, c).Output == "" { + missing = append(missing, c.CommandPath()) + } + }) + + sort.Strings(missing) + if len(missing) > 0 { + t.Errorf("leaf commands missing agent-help Output (declare AgentHelp.Output):\n %v\n(%d commands)", + missing, len(missing)) + } +} + +// TestEveryLeafCommandHasExample enforces that every leaf ships at least one +// runnable example — agents learn invocation shape from examples, not prose. +func TestEveryLeafCommandHasExample(t *testing.T) { + t.Setenv("WEKNORA_AGENT_HELP", "1") + root := NewRootCmd(cmdutil.New()) + + var missing []string + eachLeafCommand(root, func(c *cobra.Command) { + if len(renderAgentHelp(t, c).Examples) == 0 { + missing = append(missing, c.CommandPath()) + } + }) + + sort.Strings(missing) + if len(missing) > 0 { + t.Errorf("leaf commands missing agent-help Examples (declare AgentHelp.Examples):\n %v\n(%d commands)", + missing, len(missing)) + } +} diff --git a/cli/cmd/api/api.go b/cli/cmd/api/api.go index 630b20297..538e33f7e 100644 --- a/cli/cmd/api/api.go +++ b/cli/cmd/api/api.go @@ -121,11 +121,21 @@ Examples: } method := resolveMethod(opts) // Escape-hatch DELETE through `weknora api` is just as destructive - // as `weknora kb delete` - exit-10 protocol must apply (cli/README.md). - if method == http.MethodDelete { + // as `weknora kb delete` - exit-10 destructive protocol must apply + // (cli/README.md). PUT/PATCH mutate server state like a typed + // `kb/agent/doc update`, so they get the same exit-10 WRITE gate; + // without it the raw escape hatch bypassed the "an agent cannot + // silently mutate" guarantee. POST stays ungated to match typed + // `create` (also ungated). GET/HEAD are reads. + switch method { + case http.MethodDelete: if err := cmdutil.ConfirmDestructive(f.Prompter(), opts.Yes, fopts.WantsJSON(), "delete", "endpoint", args[0], "api.delete", []string{"weknora", "api", "-X", "DELETE", args[0], "-y"}); err != nil { return err } + case http.MethodPut, http.MethodPatch: + if err := cmdutil.ConfirmWrite(f.Prompter(), opts.Yes, fopts.WantsJSON(), "write", "endpoint", args[0], "api."+strings.ToLower(method), apiRetryArgv(opts, method, args[0])); err != nil { + return err + } } cli, err := f.Client() if err != nil { @@ -154,7 +164,8 @@ Examples: }, Output: "text mode (default): the raw server response body on stdout. json mode: the parsed server response is placed directly under envelope.data — project with --jq '.data...' at the server's own depth (e.g. '.data.data[]' for a list endpoint, '.data.data.id' for a created object). With --paginate, envelope.data is the merged {data, total}.", Warnings: []string{ - "Only -X DELETE is confirmation-gated (exit 10 / input.confirmation_required unless -y); -X GET/POST/PUT/PATCH and other methods are unguarded — you own the safety of writes made through this escape hatch.", + "-X DELETE is destructive-gated and -X PUT/PATCH are write-gated (exit 10 / input.confirmation_required unless -y), matching typed delete/update. -X POST (create-shaped) and GET are unguarded — you own the safety of creates made through this escape hatch.", + "Raw passthrough: the typed error envelope does NOT fully apply. The server's own response goes under envelope.data at its native depth; a non-2xx HTTP status surfaces via the exit code, not a typed error.type/retry_argv. Do not rely on error.type/retryable for `api` the way you do for typed subcommands.", "Raw HTTP passthrough; agents should prefer typed subcommands (kb/doc/session/...) when available.", }, }) @@ -293,6 +304,24 @@ func resolveMethod(opts *Options) string { return "GET" } +// apiRetryArgv reconstructs a directly-executable `weknora api` argv (with -y) +// for the write-confirmation gate, preserving the method, path and body flags +// the caller passed so an agent can re-run the exact mutation after approval. +func apiRetryArgv(opts *Options, method, path string) []string { + argv := []string{"weknora", "api", "-X", method, path} + switch { + case opts.Data != "": + argv = append(argv, "-d", opts.Data) + case opts.Input != "": + argv = append(argv, "--input", opts.Input) + default: + for _, f := range opts.Fields { + argv = append(argv, "-F", f) + } + } + return append(argv, "-y") +} + // runAPI is the testable core: validate inputs, dispatch via Service.Raw, // classify status, and emit either the raw body or a JSON object. The // caller is responsible for resolving the method (defaults / auto-POST) diff --git a/cli/cmd/api/api_test.go b/cli/cmd/api/api_test.go index c8402edc4..d93964106 100644 --- a/cli/cmd/api/api_test.go +++ b/cli/cmd/api/api_test.go @@ -551,3 +551,55 @@ func TestAPI_PaginateServerCapsPageSize(t *testing.T) { t.Errorf("got %d records, want 5 (server-capped page_size should not cause truncation)", len(got.Data)) } } + +// TestAPI_PUT_RequiresConfirmation pins the exit-10 write gate on the +// escape-hatch PUT path: `weknora api -X PUT /...` mutates server state the +// same way a typed `kb update` does, so it must require -y (exit 10) rather +// than silently writing. Regression: only DELETE was gated, letting an agent +// bypass the write-confirmation protocol via raw PUT/PATCH. +func TestAPI_PUT_RequiresConfirmation(t *testing.T) { + for _, method := range []string{"PUT", "PATCH"} { + t.Run(method, func(t *testing.T) { + iostreams.SetForTest(t) // non-TTY + f := &cmdutil.Factory{ + Client: func() (*sdk.Client, error) { return nil, nil }, + Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} }, + } + root := withRootHarness(NewCmd(f), "/api/v1/knowledge-bases/kb_xxx", "-X", method, "-F", "name=x") + err := root.Execute() + if err == nil { + t.Fatalf("expected confirmation_required for %s without -y", method) + } + var ce *cmdutil.Error + if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputConfirmationRequired { + t.Errorf("want input.confirmation_required, got %v", err) + } + if got := cmdutil.ExitCode(err); got != 10 { + t.Errorf("exit code = %d, want 10", got) + } + }) + } +} + +// TestAPI_POST_NotGated: POST is create-shaped and, like typed `kb create`, +// intentionally ungated — it must reach the SDK without a confirmation gate. +func TestAPI_POST_NotGated(t *testing.T) { + iostreams.SetForTest(t) + called := false + cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + }) + defer stop() + f := &cmdutil.Factory{ + Client: func() (*sdk.Client, error) { return cli, nil }, + Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} }, + } + root := withRootHarness(NewCmd(f), "/api/v1/knowledge-bases", "-X", "POST", "-F", "name=x") + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + if !called { + t.Error("POST handler not called - POST must not be gated") + } +} diff --git a/cli/cmd/auth/login.go b/cli/cmd/auth/login.go index 561048a4c..ffa556e7b 100644 --- a/cli/cmd/auth/login.go +++ b/cli/cmd/auth/login.go @@ -128,8 +128,12 @@ func resolveActiveProfile(f *cmdutil.Factory) (name, host string, err error) { } active := cfg.CurrentProfile if active == "" { - return "", "", cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, - "no active profile; run `weknora profile add --host --use` first") + msg := "no active profile; run `weknora profile add --host --use` first" + if envActive, kind := cmdutil.EnvCredential(); envActive { + msg = "no active profile to log in — you are already authenticated this session via " + kind + + "; `auth login` only persists a named profile, so run `weknora profile add --host --use` first if you want one" + } + return "", "", cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, msg) } prof, ok := cfg.Profiles[active] if !ok { @@ -310,9 +314,9 @@ func applyUser(prof *config.Profile, user *sdk.AuthUser) { // loginResult is the typed payload emitted by `--format json`. mode is derived from // whether the server returned a user (password flow) vs API-key flow. type loginResult struct { - Profile string `json:"profile"` - Host string `json:"host"` - Mode string `json:"mode"` // ModeBearer or ModeAPIKey + Profile string `json:"profile"` + Host string `json:"host"` + Mode string `json:"mode"` // ModeBearer or ModeAPIKey // Email is the authenticated principal's email. Named "email" (not // "user") so the identity field matches `auth status`, which also // exposes it as `email` — one key for one concept across both commands. diff --git a/cli/cmd/auth/logout.go b/cli/cmd/auth/logout.go index c35b25a68..3fc002107 100644 --- a/cli/cmd/auth/logout.go +++ b/cli/cmd/auth/logout.go @@ -61,6 +61,10 @@ accepted until it expires.`, return cfgErr } if len(cfg.Profiles) == 0 { + if active, kind := cmdutil.EnvCredential(); active { + return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, + "authenticated via "+kind+" (stateless env credential) — nothing is stored to log out; unset "+kind+" to drop it") + } return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, "no profiles configured; nothing to log out") } if _, err := pickLogoutTargets(opts, cfg); err != nil { @@ -86,7 +90,7 @@ accepted until it expires.`, cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetRisk(cmd, "auth.logout") cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "clear stored credentials for the active profile (or all) and remove the profile from config", + UsedFor: "clear stored credentials for the active profile (or --all); the profile itself stays registered (host preserved) so `auth login` can re-auth it — use `profile remove` to delete the profile entirely", Examples: []string{ "weknora auth logout", "weknora --profile staging auth logout", @@ -107,6 +111,10 @@ func runLogout(opts *LogoutOptions, fopts *cmdutil.FormatOptions, f *cmdutil.Fac return err } if len(cfg.Profiles) == 0 { + if active, kind := cmdutil.EnvCredential(); active { + return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, + "authenticated via "+kind+" (stateless env credential) — nothing is stored to log out; unset "+kind+" to drop it") + } return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, "no profiles configured; nothing to log out") } @@ -131,14 +139,15 @@ func runLogout(opts *LogoutOptions, fopts *cmdutil.FormatOptions, f *cmdutil.Fac } for _, name := range targets { clearProfileSecrets(store, cfg.Profiles[name], name) - delete(cfg.Profiles, name) - } - // If we removed the active profile, pick a remaining one (deterministic by - // map order would be flaky - leave CurrentProfile empty so the next - // invocation surfaces a clear "no current profile" error rather than - // silently switching). - if _, stillExists := cfg.Profiles[cfg.CurrentProfile]; !stillExists { - cfg.CurrentProfile = "" + // Keep the profile registered (its host stays) — only clear the + // credential refs so it reads as logged-out and can be re-authed with + // `auth login`. Deleting the profile entirely is `profile remove`'s job; + // that clean logout(who)/remove(what) split matches gh / lark. + p := cfg.Profiles[name] + p.APIKeyRef = "" + p.TokenRef = "" + p.RefreshRef = "" + cfg.Profiles[name] = p } if err := config.Save(cfg); err != nil { return cmdutil.Wrapf(cmdutil.CodeLocalFileIO, err, "save config") diff --git a/cli/cmd/auth/logout_test.go b/cli/cmd/auth/logout_test.go index 8f03b7e41..36885b2e1 100644 --- a/cli/cmd/auth/logout_test.go +++ b/cli/cmd/auth/logout_test.go @@ -48,8 +48,12 @@ func TestLogout_CurrentProfile(t *testing.T) { } require.NoError(t, runLogout(&LogoutOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, newLogoutFactory(t, cfg, store))) - assert.Empty(t, cfg.CurrentProfile, "current_profile should clear when removed") - assert.NotContains(t, cfg.Profiles, "prod") + assert.Equal(t, "prod", cfg.CurrentProfile, "active profile stays selected — logout clears creds, not the profile") + require.Contains(t, cfg.Profiles, "prod", "profile stays registered after logout") + assert.Empty(t, cfg.Profiles["prod"].APIKeyRef, "credential ref cleared") + assert.Empty(t, cfg.Profiles["prod"].TokenRef, "credential ref cleared") + assert.Empty(t, cfg.Profiles["prod"].RefreshRef, "credential ref cleared") + assert.Equal(t, "https://prod", cfg.Profiles["prod"].Host, "host preserved for re-login") assert.Contains(t, cfg.Profiles, "staging", "non-target profile untouched") // Secrets gone for the removed profile, kept for the survivor. @@ -81,7 +85,8 @@ func TestLogout_ActiveProfileViaOverride(t *testing.T) { } require.NoError(t, runLogout(&LogoutOptions{Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, newLogoutFactory(t, cfg, store))) - assert.NotContains(t, cfg.Profiles, "staging", "active profile (staging) is the target") + require.Contains(t, cfg.Profiles, "staging", "target profile stays registered (creds cleared, not removed)") + assert.Empty(t, cfg.Profiles["staging"].APIKeyRef, "target's credential ref cleared") assert.Contains(t, cfg.Profiles, "prod", "non-target profile untouched") } @@ -98,8 +103,14 @@ func TestLogout_All(t *testing.T) { } require.NoError(t, runLogout(&LogoutOptions{All: true, Yes: true}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, newLogoutFactory(t, cfg, store))) - assert.Empty(t, cfg.Profiles) - assert.Empty(t, cfg.CurrentProfile) + // --all clears every profile's credentials but keeps the profiles registered. + require.NotEmpty(t, cfg.Profiles, "profiles stay registered after logout --all") + for name, p := range cfg.Profiles { + assert.Empty(t, p.APIKeyRef, "%s api-key ref cleared", name) + assert.Empty(t, p.TokenRef, "%s token ref cleared", name) + assert.Empty(t, p.RefreshRef, "%s refresh ref cleared", name) + } + assert.Equal(t, "prod", cfg.CurrentProfile, "active selection is preserved; logout clears creds, not the profile") } func TestLogout_NoProfiles(t *testing.T) { diff --git a/cli/cmd/auth/refresh.go b/cli/cmd/auth/refresh.go index 1024b5b2e..2b9a7cab6 100644 --- a/cli/cmd/auth/refresh.go +++ b/cli/cmd/auth/refresh.go @@ -62,6 +62,10 @@ refresh semantic. Rotate the key in the server UI instead.`, } name := cfg.CurrentProfile if name == "" { + if active, kind := cmdutil.EnvCredential(); active { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + "authenticated via "+kind+" (stateless env credential): there is no stored JWT to refresh — env credentials are supplied fresh each call, so no refresh is needed") + } return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, "no active profile configured; run `weknora auth login` to set one up") } @@ -99,6 +103,10 @@ refresh semantic. Rotate the key in the server UI instead.`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "Renew the JWT access token for the active profile (override with the global --profile) using the stored refresh token. API-key profiles are rejected.", Output: "envelope.data has profile name that was refreshed", + Examples: []string{ + "weknora auth refresh", + "weknora --profile staging auth refresh", + }, }) return cmd } @@ -117,6 +125,10 @@ func runRefresh(ctx context.Context, opts *RefreshOptions, fopts *cmdutil.Format } name := cfg.CurrentProfile if name == "" { + if active, kind := cmdutil.EnvCredential(); active { + return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + "authenticated via "+kind+" (stateless env credential): there is no stored JWT to refresh — env credentials are supplied fresh each call, so no refresh is needed") + } return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, "no active profile configured; run `weknora auth login` to set one up") } diff --git a/cli/cmd/auth/token.go b/cli/cmd/auth/token.go index f438b2336..7ebd79427 100644 --- a/cli/cmd/auth/token.go +++ b/cli/cmd/auth/token.go @@ -2,6 +2,7 @@ package auth import ( "fmt" + "os" "github.com/spf13/cobra" @@ -90,6 +91,16 @@ to see which mode each profile uses, and construct the matching HTTP header: } func runToken(f *cmdutil.Factory, fopts *cmdutil.FormatOptions) error { + // Env credentials are the active credential on the headless path — `auth + // token` must surface them (they ARE the token / api key) instead of + // erroring on the absence of a stored profile. + if active, kind := cmdutil.EnvCredential(); active { + mode := ModeBearer + if kind == "WEKNORA_API_KEY" { + mode = ModeAPIKey + } + return emitToken(fopts, os.Getenv(kind), mode, "(env)") + } cfg, err := f.Config() if err != nil { return err @@ -140,8 +151,18 @@ func runToken(f *cmdutil.Factory, fopts *cmdutil.FormatOptions) error { fmt.Sprintf("profile %q credential is empty in keyring; run `weknora auth login`", profileName)) } + return emitToken(fopts, token, mode, profileName) +} + +// emitToken renders a resolved credential: the {token, mode, profile} envelope +// under --format json, else the raw token on stdout (no trailing newline, for +// clean $(weknora auth token) capture) with a TTY-only leak hint on stderr. +func emitToken(fopts *cmdutil.FormatOptions, token, mode, profile string) error { + if token == "" { + return cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, "active credential is empty") + } if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, tokenResult{Token: token, Mode: mode, Profile: profileName}, nil) + return fopts.Emit(iostreams.IO.Out, tokenResult{Token: token, Mode: mode, Profile: profile}, nil) } // No trailing newline - clean $(weknora auth token) substitution. diff --git a/cli/cmd/chunk/delete.go b/cli/cmd/chunk/delete.go index b3d8545b3..914b233e4 100644 --- a/cli/cmd/chunk/delete.go +++ b/cli/cmd/chunk/delete.go @@ -124,6 +124,7 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.DocID, "doc", "", "Parent document id (SDK knowledge_id) the chunks live under") _ = cmd.MarkFlagRequired("doc") + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.AddFormatFlag(cmd, chunkDeleteFields...) cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetRisk(cmd, "chunk.delete") diff --git a/cli/cmd/chunk/list.go b/cli/cmd/chunk/list.go index 24c2fbbda..0e72acaff 100644 --- a/cli/cmd/chunk/list.go +++ b/cli/cmd/chunk/list.go @@ -19,7 +19,7 @@ const ( defaultPageSize = 50 maxPageSize = 1000 defaultLimit = 50 - maxLimit = 1000 + maxLimit = 10000 previewWidth = 80 ) @@ -45,7 +45,7 @@ type ListOptions struct { DocID string // PageSize is the server batch size (1..1000, default 50). PageSize int - // Limit caps the client-side accumulated slice (1..1000, default 50). + // Limit caps the client-side accumulated slice (1..10000, default 50). // Default 50 chosen as domain-tuned for chunk enumeration (RAG debug). Limit int // AllPages walks server pages internally until total exhausted or @@ -65,7 +65,7 @@ For relevance-ranked retrieval (the RAG runtime surface), use vector + keyword scoring across all chunks of a knowledge base. Typed exit codes: - input.invalid_argument --limit / --page-size out of 1..1000 range (exit 5) + input.invalid_argument --limit out of 1..10000 or --page-size out of 1..1000 (exit 5) resource.not_found no document with the given id (exit 4) AI agents: prefer 'search chunks' for retrieval tasks. Use 'chunk list' @@ -106,15 +106,16 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.DocID, "doc", "", "Document id (SDK knowledge_id) to enumerate chunks for") _ = cmd.MarkFlagRequired("doc") - cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, "Maximum results to return — client-side cap; meta.has_more reports truncation (1..1000)") + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, "Maximum results to return — client-side cap; meta.has_more/total_count report truncation (1..10000)") cmd.Flags().IntVar(&opts.PageSize, "page-size", defaultPageSize, "Items per server batch (1..1000)") cmd.Flags().BoolVar(&opts.AllPages, "all-pages", false, "Walk all server pages until exhausted (or --limit hit)") cmdutil.AddFormatFlag(cmd, chunkListFields...) + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "List chunks of a specific document in stored order (admin/debug). Results come with meta.count; use --limit (1..1000) and --all-pages to paginate. Prefer 'search chunks' for RAG retrieval.", + UsedFor: "List chunks of a specific document in stored order (admin/debug). Results come with meta.count; use --limit (1..10000) and --all-pages to paginate. Prefer 'search chunks' for RAG retrieval.", RequiredFlags: []string{"--doc"}, Examples: []string{"weknora chunk list --doc doc_abc --format json", "weknora chunk list --doc doc_abc --all-pages --format json"}, - Output: "envelope.data is an array of Chunk objects with id, chunk_index, content, is_enabled; meta.count is the total returned", + Output: "envelope.data is an array of Chunk objects with id, chunk_index, content, is_enabled; meta.count is the returned count, meta.total_count the document's full chunk count, meta.has_more true when --limit truncated", }) return cmd } @@ -144,6 +145,7 @@ func runList(ctx context.Context, opts *ListOptions, fopts *cmdutil.FormatOption } var items []sdk.Chunk + var serverTotal int64 truncated := false if opts.AllPages { accum := make([]sdk.Chunk, 0) @@ -152,6 +154,7 @@ func runList(ctx context.Context, opts *ListOptions, fopts *cmdutil.FormatOption if err != nil { return cmdutil.WrapHTTP(err, "list chunks for doc %s", opts.DocID) } + serverTotal = total accum = append(accum, chunks...) if len(accum) >= opts.Limit { accum = accum[:opts.Limit] @@ -164,10 +167,11 @@ func runList(ctx context.Context, opts *ListOptions, fopts *cmdutil.FormatOption } items = accum } else { - chunks, _, err := svc.ListKnowledgeChunks(ctx, opts.DocID, 1, opts.PageSize) + chunks, total, err := svc.ListKnowledgeChunks(ctx, opts.DocID, 1, opts.PageSize) if err != nil { return cmdutil.WrapHTTP(err, "list chunks for doc %s", opts.DocID) } + serverTotal = total items = chunks } if items == nil { @@ -179,7 +183,7 @@ func runList(ctx context.Context, opts *ListOptions, fopts *cmdutil.FormatOption } if fopts.WantsJSON() { - meta := &output.Meta{Count: output.IntPtr(len(items)), HasMore: truncated} + meta := &output.Meta{Count: output.IntPtr(len(items)), TotalCount: output.IntPtr(int(serverTotal)), HasMore: truncated} return fopts.Emit(iostreams.IO.Out, items, meta) } diff --git a/cli/cmd/chunk/list_test.go b/cli/cmd/chunk/list_test.go index 110c62d5e..4279d38f9 100644 --- a/cli/cmd/chunk/list_test.go +++ b/cli/cmd/chunk/list_test.go @@ -128,9 +128,35 @@ func TestList_AllPages_LimitTruncatesAccumulated(t *testing.T) { assert.Equal(t, []string{"c1", "c2", "c3"}, []string{got[0].ID, got[1].ID, got[2].ID}) } +// TestList_JSON_EmitsTotalCount pins that chunk list surfaces the document's +// full chunk count as meta.total_count (like doc/session/kb/model list), not +// just the returned count — an agent must be able to tell truncation from +// completeness. +func TestList_JSON_EmitsTotalCount(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeListSvc{ + pages: [][]sdk.Chunk{{{ID: "c1"}, {ID: "c2"}}}, + totals: []int64{7}, + errs: []error{nil}, + } + opts := &ListOptions{DocID: "d1", Limit: 50, PageSize: 50} + require.NoError(t, runList(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc)) + var env struct { + Meta struct { + Count *int `json:"count"` + TotalCount *int `json:"total_count"` + } `json:"meta"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env)) + require.NotNil(t, env.Meta.TotalCount, "chunk list must emit meta.total_count") + assert.Equal(t, 7, *env.Meta.TotalCount) + require.NotNil(t, env.Meta.Count) + assert.Equal(t, 2, *env.Meta.Count) +} + func TestList_LimitInvalid(t *testing.T) { svc := &fakeListSvc{} - for _, lim := range []int{0, -1, 1001} { + for _, lim := range []int{0, -1, 10001} { err := runList(context.Background(), &ListOptions{DocID: "d", Limit: lim, PageSize: 50}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc) require.Error(t, err, "expect error for --limit %d", lim) assert.Contains(t, err.Error(), "input.invalid_argument") diff --git a/cli/cmd/chunk/view.go b/cli/cmd/chunk/view.go index e27212ef6..1d2aa69ba 100644 --- a/cli/cmd/chunk/view.go +++ b/cli/cmd/chunk/view.go @@ -81,6 +81,7 @@ func NewCmdView(f *cmdutil.Factory) *cobra.Command { }, } cmdutil.AddFormatFlag(cmd, chunkViewFields...) + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "fetch one chunk's fields and content by id (scope-less; no --doc needed)", RequiredFlags: []string{" (positional)"}, diff --git a/cli/cmd/doc/create.go b/cli/cmd/doc/create.go index 64f6f02cc..2f5196eb2 100644 --- a/cli/cmd/doc/create.go +++ b/cli/cmd/doc/create.go @@ -12,6 +12,7 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/output" sdk "github.com/Tencent/WeKnora/client" ) @@ -26,17 +27,12 @@ var docCreateFields = []string{ // CreateOptions holds CLI flag values for `doc create`. type CreateOptions struct { Text string // --text (required): document text content (Markdown) - Title string // --title: document title (preferred; matches `doc update --title`) - Name string // --name: deprecated alias for --title + Title string // --title: document title (matches `doc update --title`) TagID string // --tag-id: associate with a tag Channel string // --channel: ingestion-channel tag (default "api") DryRun bool } -// title returns the resolved document title, preferring --title over the -// deprecated --name alias. -func (o *CreateOptions) title() string { return cmp.Or(o.Title, o.Name) } - // CreateService is the narrow SDK surface for `doc create`. // *sdk.Client satisfies it. type CreateService interface { @@ -84,7 +80,7 @@ don't require a file upload or remote URL. KB resolution follows the standard Action: "doc.create", Args: map[string]any{ "text": opts.Text, - "title": opts.title(), + "title": opts.Title, "kb": kbID, }, }); handled { @@ -106,8 +102,6 @@ don't require a file upload or remote URL. KB resolution follows the standard cmdutil.AddKBFlag(cmd) cmd.Flags().StringVar(&opts.Text, "text", "", "Document text content in Markdown format (required)") cmd.Flags().StringVar(&opts.Title, "title", "", "Document title") - cmd.Flags().StringVar(&opts.Name, "name", "", "Document title (deprecated: use --title)") - _ = cmd.Flags().MarkDeprecated("name", "use --title instead") 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") @@ -134,7 +128,7 @@ func runCreate(ctx context.Context, opts *CreateOptions, fopts *cmdutil.FormatOp return cmdutil.NewFlagError(fmt.Errorf("--text is required")) } req := &sdk.CreateManualKnowledgeRequest{ - Title: opts.title(), + Title: opts.Title, Content: opts.Text, TagID: opts.TagID, Channel: cmp.Or(opts.Channel, uploadChannel), @@ -143,10 +137,18 @@ func runCreate(ctx context.Context, opts *CreateOptions, fopts *cmdutil.FormatOp if err != nil { return cmdutil.WrapHTTP(err, "create document") } - if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, k, nil) + // Inline-created docs land in parse_status=draft and are NOT auto-queued + // for parsing (unlike `doc upload`), so they aren't searchable until + // reparsed. Surface the next step so an agent doesn't `doc wait` into a + // timeout or `search` into empty results. + var meta *output.Meta + if k.ParseStatus == "draft" { + meta = &output.Meta{Hint: "document created in parse_status=draft (not yet indexed) — run `weknora doc reparse " + k.ID + "` to parse & make it searchable"} } - displayed := opts.title() + if fopts.WantsJSON() { + return fopts.Emit(iostreams.IO.Out, k, meta) + } + displayed := opts.Title if displayed == "" { displayed = k.Title } @@ -154,5 +156,8 @@ func runCreate(ctx context.Context, opts *CreateOptions, fopts *cmdutil.FormatOp displayed = k.ID } fmt.Fprintf(iostreams.IO.Out, "✓ Created %q (id: %s)\n", displayed, k.ID) + if meta != nil { + fmt.Fprintf(iostreams.IO.Out, " ⚠ %s\n", meta.Hint) + } return nil } diff --git a/cli/cmd/doc/create_test.go b/cli/cmd/doc/create_test.go index c8a4200d2..e4de983d3 100644 --- a/cli/cmd/doc/create_test.go +++ b/cli/cmd/doc/create_test.go @@ -35,28 +35,21 @@ func (f *fakeCreateSvc) CreateManualKnowledge( return f.resp, f.err } -// TestCreate_TitlePreferredOverName: --title is the canonical flag; the -// deprecated --name remains a working alias but --title wins when both are set. -func TestCreate_TitlePreferredOverName(t *testing.T) { +// TestCreate_TitleSetsRequestTitle: --title is the sole title flag and flows +// straight into the create request. +func TestCreate_TitleSetsRequestTitle(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "d1"}} require.NoError(t, runCreate(context.Background(), - &CreateOptions{Text: "x", Title: "FromTitle", Name: "FromName"}, + &CreateOptions{Text: "x", Title: "FromTitle"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb1")) assert.Equal(t, "FromTitle", svc.got.req.Title) - - // --name alone still works (back-compat for the deprecated alias). - svc2 := &fakeCreateSvc{resp: &sdk.Knowledge{ID: "d2"}} - require.NoError(t, runCreate(context.Background(), - &CreateOptions{Text: "x", Name: "OnlyName"}, - &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc2, "kb1")) - assert.Equal(t, "OnlyName", svc2.got.req.Title) } func TestCreate_Success_Text(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"} + opts := &CreateOptions{Text: "# Sprint Notes\n\nAction items: ...", Title: "Sprint Notes"} require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_xxx")) assert.Equal(t, "kb_xxx", svc.got.kbID) @@ -110,7 +103,7 @@ func TestCreate_Channel_DefaultIsAPI(t *testing.T) { 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"} + opts := &CreateOptions{Text: "# My Note", Title: "My Note"} fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} require.NoError(t, runCreate(context.Background(), opts, fopts, svc, "kb_xxx")) diff --git a/cli/cmd/doc/download.go b/cli/cmd/doc/download.go index ba627578a..078cb6870 100644 --- a/cli/cmd/doc/download.go +++ b/cli/cmd/doc/download.go @@ -79,11 +79,16 @@ stdout.`, } cmd.Flags().StringVarP(&opts.Output, "output", "O", "", `Output path; "-" for stdout. Defaults to the server-suggested filename.`) cmd.Flags().BoolVar(&opts.Clobber, "clobber", false, "Overwrite the output file if it already exists") + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.AddFormatFlag(cmd, downloadFields...) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "Download a document's bytes by id. Writes a file (or stdout with --output -).", RequiredFlags: []string{" (positional)"}, Output: "with --format json (file output): envelope.data has path, bytes, filename; suppressed with --output - (raw bytes to stdout)", + Examples: []string{ + "weknora doc download doc_abc --output ./manual.pdf", + "weknora doc download doc_abc --output - > manual.pdf", + }, }) return cmd } diff --git a/cli/cmd/doc/list.go b/cli/cmd/doc/list.go index 1aa992a82..4ac9d6163 100644 --- a/cli/cmd/doc/list.go +++ b/cli/cmd/doc/list.go @@ -106,7 +106,7 @@ backend storage order is not guaranteed and varies between deployments.`, cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum results to return — client-side cap; meta.has_more/total_count report the full size (1..10000)") cmd.Flags().BoolVar(&opts.AllPages, "all-pages", false, "Walk all server pages until exhausted (or --limit hit)") cmd.Flags().StringVar(&opts.Status, "status", "", "Filter by parse status: pending | processing | completed | failed") - cmd.Flags().StringVar(&opts.Keyword, "keyword", "", "Server-side substring match against title / file_name (case-sensitive)") + cmd.Flags().StringVar(&opts.Keyword, "keyword", "", "Server-side substring match against title / file_name (case-insensitive)") cmd.Flags().StringVar(&opts.FileType, "file-type", "", `Filter by file extension (e.g. "pdf", "md")`) cmd.Flags().StringVar(&opts.Source, "source", "", `Filter by ingestion source (e.g. "api", "web")`) cmd.Flags().StringVar(&opts.TagID, "tag-id", "", "Filter by tag association") @@ -116,7 +116,7 @@ backend storage order is not guaranteed and varies between deployments.`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "List documents in the resolved knowledge base. Results come with meta.count; use --limit to cap, --all-pages to walk every server page, --status/--keyword to filter server-side.", Examples: []string{"weknora doc list --format json", "weknora doc list --all-pages --limit 200 --format json"}, - Output: "envelope.data is an array of Knowledge objects with id, title, file_name, parse_status; meta.count is the total returned; meta.total_count is the server-side total before client-side --limit truncation", + Output: "envelope.data is an array of Knowledge objects with id, title, file_name, parse_status; meta.count is the returned count; meta.total_count is the server-side total before client-side --limit truncation; meta.has_more=true when --limit truncated", }) return cmd } diff --git a/cli/cmd/doc/reparse.go b/cli/cmd/doc/reparse.go index 58f822728..f33cfaf4b 100644 --- a/cli/cmd/doc/reparse.go +++ b/cli/cmd/doc/reparse.go @@ -57,6 +57,7 @@ keeps its id; parsing restarts asynchronously, so follow with return runReparse(c.Context(), opts, fopts, cli, args[0]) }, } + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.AddFormatFlag(cmd, docReparseFields...) cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ diff --git a/cli/cmd/doc/update.go b/cli/cmd/doc/update.go index a51266e5c..a5b38fe74 100644 --- a/cli/cmd/doc/update.go +++ b/cli/cmd/doc/update.go @@ -98,6 +98,7 @@ to the user first.`, } cmd.Flags().StringVar(&title, "title", "", "New title (omit to leave unchanged)") cmd.Flags().StringVar(&desc, "description", "", "New description (omit to leave unchanged)") + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.AddFormatFlag(cmd, docUpdateFields...) cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetWriteRisk(cmd, "doc.update") diff --git a/cli/cmd/doc/upload_recursive.go b/cli/cmd/doc/upload_recursive.go index 3c4cdbc4d..111ba29fe 100644 --- a/cli/cmd/doc/upload_recursive.go +++ b/cli/cmd/doc/upload_recursive.go @@ -81,16 +81,11 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, fopts *cmdutil // 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 := 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, nil) if err != nil { - code := cmdutil.ClassifyHTTPError(err) - if firstFailCode == "" { - firstFailCode = code - } // 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() { @@ -133,9 +128,15 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, fopts *cmdutil // 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()) + // Any per-file failure collapses to operation.failed (exit 1) — the + // per-file batch envelope carries each file's typed error, which is the + // authoritative signal (matches doc/session/chunk batch delete). A + // cancelled / timed-out batch keeps its context class (124 / 130) so a + // permanent partial failure (e.g. a duplicate) is never misreported as a + // retryable 5xx that an agent would loop on. + code := cmdutil.CodeOperationFailed + if ctxErr := ctx.Err(); ctxErr != nil { + code = cmdutil.ClassifyContextErr(ctxErr) } return &cmdutil.Error{ Code: code, diff --git a/cli/cmd/doc/upload_recursive_test.go b/cli/cmd/doc/upload_recursive_test.go index d4a6454d1..d8cb44a7b 100644 --- a/cli/cmd/doc/upload_recursive_test.go +++ b/cli/cmd/doc/upload_recursive_test.go @@ -109,9 +109,12 @@ func TestUploadRecursive_PartialFailure_Exits1(t *testing.T) { var typed *cmdutil.Error require.ErrorAs(t, err, &typed) - // CodeServerError preserves the 500 classification of the underlying - // SDK error - the recursive wrapper just aggregates. - assert.Equal(t, cmdutil.CodeServerError, typed.Code) + // Any per-file failure aggregates to operation.failed (exit 1), matching the + // batch-delete contract — a partial failure (even a permanent one like a + // duplicate, which classifies as server.error per-file) must not surface as + // a retryable exit 7 an agent would loop on. Per-file codes live in the + // batch envelope. + assert.Equal(t, cmdutil.CodeOperationFailed, typed.Code) got := out.String() assert.Contains(t, got, "OK") // ok.pdf still succeeded @@ -267,5 +270,5 @@ func TestUploadRecursive_JSON_BatchEnvelope(t *testing.T) { var typed *cmdutil.Error require.ErrorAs(t, err, &typed) assert.True(t, typed.Silent, "JSON-path partial failure must be Silent") - assert.Equal(t, cmdutil.CodeServerError, typed.Code) + assert.Equal(t, cmdutil.CodeOperationFailed, typed.Code) } diff --git a/cli/cmd/doc/view.go b/cli/cmd/doc/view.go index 9f3a9d325..4d94654de 100644 --- a/cli/cmd/doc/view.go +++ b/cli/cmd/doc/view.go @@ -52,6 +52,7 @@ func NewCmdView(f *cmdutil.Factory) *cobra.Command { }, } cmdutil.AddFormatFlag(cmd, docViewFields...) + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "fetch one document's metadata by id", RequiredFlags: []string{" (positional)"}, diff --git a/cli/cmd/doc/wait.go b/cli/cmd/doc/wait.go index 0933984d8..3e6c3c20c 100644 --- a/cli/cmd/doc/wait.go +++ b/cli/cmd/doc/wait.go @@ -94,6 +94,7 @@ For fail-fast semantics, use shell composition: cmd.Flags().DurationVar(&opts.Timeout, "timeout", 10*time.Minute, "Max wait time before exiting 124") cmd.Flags().DurationVar(&opts.Interval, "interval", 2*time.Second, "Initial poll interval; exponential backoff capped at 15s + jitter") cmdutil.AddFormatFlag(cmd) + cmdutil.AddIgnoredKBFlag(cmd) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "block until one or more documents reach a terminal parse state (completed or failed), or --timeout elapses", RequiredFlags: []string{"... (one or more positionals)"}, @@ -234,6 +235,14 @@ func waitForDocs(ctx context.Context, ids []string, svc WaitService, opts WaitOp case "failed": addFailed(FailedDoc{ID: id, Message: doc.ErrorMessage}) return + case "draft": + // draft = created but NOT queued for parsing (inline + // `doc create` leaves docs here; file `doc upload` + // auto-enqueues). It never progresses on its own, so + // waiting would hang to the --timeout (124). Fail fast with + // the exact unblock command instead of silently polling. + addFailed(FailedDoc{ID: id, Message: "parse_status=draft: not queued for parsing — run `weknora doc reparse " + id + "` to index it"}) + return } // Not yet terminal — sleep with jitter, then exp-backoff. diff --git a/cli/cmd/doc/wait_test.go b/cli/cmd/doc/wait_test.go index b0cf8b4d0..64d682ec7 100644 --- a/cli/cmd/doc/wait_test.go +++ b/cli/cmd/doc/wait_test.go @@ -311,3 +311,30 @@ func TestDocWait_FailureError_IsSilent(t *testing.T) { t.Errorf("exit code = %d, want 1", cmdutil.ExitCode(err)) } } + +// TestWaitForDocs_DraftFailsFast pins that a document stuck in parse_status +// "draft" (inline `doc create` leaves docs here; it never auto-progresses) is +// failed fast with a reparse hint instead of polling until the --timeout. +// Regression: `doc create` + `doc wait` used to hang to a 124 timeout. +func TestWaitForDocs_DraftFailsFast(t *testing.T) { + svc := newFakeKBSvc(map[string][]string{ + "doc_draft": {"draft", "draft", "draft"}, + }) + start := time.Now() + res, _ := waitForDocs(context.Background(), []string{"doc_draft"}, svc, WaitOptions{ + Timeout: 5 * time.Second, + Interval: 1 * time.Millisecond, + }) + if len(res.Timeout) != 0 { + t.Errorf("draft must NOT time out; got timeout=%v", res.Timeout) + } + if len(res.Failed) != 1 || res.Failed[0].ID != "doc_draft" { + t.Fatalf("draft doc must be failed-fast; got failed=%v", res.Failed) + } + if !strings.Contains(res.Failed[0].Message, "reparse") { + t.Errorf("draft failure message must point to reparse; got %q", res.Failed[0].Message) + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Errorf("draft must fail fast (well under timeout); took %v", elapsed) + } +} diff --git a/cli/cmd/doctor/doctor.go b/cli/cmd/doctor/doctor.go index ae267ac78..b699b5e17 100644 --- a/cli/cmd/doctor/doctor.go +++ b/cli/cmd/doctor/doctor.go @@ -24,6 +24,7 @@ import ( "fmt" "net/http" "os" + "strings" "time" "github.com/spf13/cobra" @@ -31,6 +32,7 @@ import ( "github.com/Tencent/WeKnora/cli/internal/build" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/compat" + "github.com/Tencent/WeKnora/cli/internal/config" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/secrets" sdk "github.com/Tencent/WeKnora/client" @@ -396,15 +398,38 @@ func buildServices(f *cmdutil.Factory) (Services, error) { if err != nil { return nil, err } + return &realServices{f: f, host: resolveDoctorHost(cfg)}, nil +} + +// resolveDoctorHost picks the host base_url_reachable probes. Tiers 2 and 3 +// mirror the client builder (buildClientFromEnv) so doctor probes the host the +// real commands actually connect to; tier 1 is a doctor-local test/dev knob: +// +// 1. WEKNORA_BASE_URL — doctor-only probe override (used by tests); NOT read +// by the client builder, so setting it points doctor at a host no real +// command uses. Kept for test/dev harnesses; leave unset in normal use. +// 2. WEKNORA_HOST — when stateless env credentials (WEKNORA_TOKEN / +// WEKNORA_API_KEY) are in effect, i.e. the headless agent path. Without +// this, `WEKNORA_API_KEY=… WEKNORA_HOST=… weknora doctor` falsely reported +// "no host configured" and exited 1 while every other command worked. +// 3. active profile host — the configured default. +func resolveDoctorHost(cfg *config.Config) string { host := "" if ctx, ok := cfg.Profiles[cfg.CurrentProfile]; ok { host = ctx.Host } - // WEKNORA_BASE_URL still wins as a test/dev override; production reads host. + // Env credentials authenticate via WEKNORA_HOST, bypassing the profile. + // Honor it only when such creds are actually set, matching the client + // builder (a bare WEKNORA_HOST without creds is ignored there too). + if envActive, _ := cmdutil.EnvCredential(); envActive { + if v := strings.TrimSpace(os.Getenv("WEKNORA_HOST")); v != "" { + host = v + } + } if v := os.Getenv("WEKNORA_BASE_URL"); v != "" { host = v } - return &realServices{f: f, host: host}, nil + return host } type realServices struct { diff --git a/cli/cmd/doctor/doctor_test.go b/cli/cmd/doctor/doctor_test.go index b1db3196f..3e56f1cca 100644 --- a/cli/cmd/doctor/doctor_test.go +++ b/cli/cmd/doctor/doctor_test.go @@ -10,6 +10,7 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/compat" + "github.com/Tencent/WeKnora/cli/internal/config" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/secrets" sdk "github.com/Tencent/WeKnora/client" @@ -507,3 +508,54 @@ func TestDoctor_RunE_WarnReturnsNil(t *testing.T) { t.Fatalf("setup error: expected Warned>=1, got %+v", r.Summary) } } + +// TestResolveDoctorHost_EnvCredentials - doctor's base_url probe must honor +// WEKNORA_HOST when stateless env credentials are in effect (the headless +// agent path), mirroring buildClientFromEnv. Regression: doctor previously +// read only the active profile host, so `WEKNORA_API_KEY=... WEKNORA_HOST=... +// weknora doctor` reported "no host configured" and exited 1 while every +// other command worked. +func TestResolveDoctorHost_EnvCredentials(t *testing.T) { + cfg := &config.Config{ + CurrentProfile: "prod", + Profiles: map[string]config.Profile{"prod": {Host: "https://profile-host"}}, + } + + t.Run("env creds + WEKNORA_HOST wins over profile", func(t *testing.T) { + t.Setenv("WEKNORA_TOKEN", "") + t.Setenv("WEKNORA_API_KEY", "sk-test") + t.Setenv("WEKNORA_HOST", "https://env-host:8080") + t.Setenv("WEKNORA_BASE_URL", "") + if got := resolveDoctorHost(cfg); got != "https://env-host:8080" { + t.Errorf("want env host, got %q", got) + } + }) + + t.Run("env creds without WEKNORA_HOST falls back to profile", func(t *testing.T) { + t.Setenv("WEKNORA_API_KEY", "sk-test") + t.Setenv("WEKNORA_HOST", "") + t.Setenv("WEKNORA_BASE_URL", "") + if got := resolveDoctorHost(cfg); got != "https://profile-host" { + t.Errorf("want profile host, got %q", got) + } + }) + + t.Run("no env creds ignores WEKNORA_HOST (matches client builder)", func(t *testing.T) { + t.Setenv("WEKNORA_TOKEN", "") + t.Setenv("WEKNORA_API_KEY", "") + t.Setenv("WEKNORA_HOST", "https://should-be-ignored") + t.Setenv("WEKNORA_BASE_URL", "") + if got := resolveDoctorHost(cfg); got != "https://profile-host" { + t.Errorf("want profile host, got %q", got) + } + }) + + t.Run("WEKNORA_BASE_URL test override always wins", func(t *testing.T) { + t.Setenv("WEKNORA_API_KEY", "sk-test") + t.Setenv("WEKNORA_HOST", "https://env-host") + t.Setenv("WEKNORA_BASE_URL", "https://base-url-override") + if got := resolveDoctorHost(cfg); got != "https://base-url-override" { + t.Errorf("want base-url override, got %q", got) + } + }) +} diff --git a/cli/cmd/dryrun_coverage_test.go b/cli/cmd/dryrun_coverage_test.go index 69b6d743f..78435b2d9 100644 --- a/cli/cmd/dryrun_coverage_test.go +++ b/cli/cmd/dryrun_coverage_test.go @@ -21,8 +21,8 @@ import ( var dryRunExpectation = map[string]bool{ // --- mutations: MUST have --dry-run --- "kb create": true, "kb update": true, "kb delete": true, "kb pin": true, "kb unpin": true, - "kb init": true, // binds models to a KB (state change) - "model create": true, "model delete": true, + "kb config set": true, // binds models to a KB (state change) + "model create": true, "model update": true, "model delete": true, "doc create": true, "doc upload": true, "doc fetch": true, "doc delete": true, "doc reparse": true, // re-triggers server-side parsing (a state change) "doc update": true, // edits title/description server-side @@ -55,21 +55,39 @@ var dryRunExpectation = map[string]bool{ "doctor": false, "version": false, // generate / stream ops — the session-creation side effect is incidental, // not a CRUD write; a no-SDK-call preview would be meaningless. - "chat": false, "session ask": false, "session continue-stream": false, + "chat": false, "session ask": false, "session resume": false, // auth login VALIDATES credentials against the server and stores them; its // whole purpose is the server round-trip, which a side-effect-free dry-run // cannot exercise — so previewing it would be misleading. Exempt by design. "auth login": false, // long-running stdio server, not a one-shot command. "mcp serve": false, - // offline read: enumerates tool metadata without any network call. - "mcp tools list": false, // offline help topic: prints the static exit-code matrix. "exit-codes": false, // offline introspection: prints command contracts from the in-binary tree. "schema": false, } +// TestIdAddressedCommandsTolerateKB pins that the id-addressed read/wait +// commands accept a (redundant, ignored) --kb flag, so an agent flowing from +// `doc upload --kb X` into `doc wait --kb X` doesn't hit exit 2. +func TestIdAddressedCommandsTolerateKB(t *testing.T) { + root := NewRootCmd(cmdutil.New()) + for _, path := range [][]string{ + {"doc", "view"}, {"doc", "wait"}, {"doc", "download"}, + {"doc", "reparse"}, {"doc", "update"}, + {"chunk", "list"}, {"chunk", "view"}, {"chunk", "delete"}, + } { + c, _, err := root.Find(path) + if err != nil { + t.Fatalf("find %v: %v", path, err) + } + if c.Flags().Lookup("kb") == nil { + t.Errorf("`%s` must accept a --kb flag (ignored) so a carried-over --kb doesn't error", strings.Join(path, " ")) + } + } +} + func TestDryRunCoverageMatchesExpectation(t *testing.T) { root := NewRootCmd(cmdutil.New()) diff --git a/cli/cmd/kb/check.go b/cli/cmd/kb/check.go index a88a8946f..df9551e3c 100644 --- a/cli/cmd/kb/check.go +++ b/cli/cmd/kb/check.go @@ -17,13 +17,17 @@ import ( // aggregated by paging the doc list. Verb split with `kb status`: // status reads existing state cheaply, check actively verifies. type CheckResult struct { - ID string `json:"id"` - Reachable bool `json:"reachable"` - KnowledgeCount int64 `json:"knowledge_count,omitempty"` - ChunkCount int64 `json:"chunk_count,omitempty"` - IsProcessing bool `json:"is_processing,omitempty"` - ProcessingCount int64 `json:"processing_count,omitempty"` - FailedCount int64 `json:"failed_count"` // always populated (no omitempty) + ID string `json:"id"` + Reachable bool `json:"reachable"` + // RetrievalReady is false when no embedding model is bound — the KB can never + // index/retrieve regardless of failed_count. Always emitted (no omitempty) so + // an unconfigured KB is not reported as silently healthy. + RetrievalReady bool `json:"retrieval_ready"` + KnowledgeCount int64 `json:"knowledge_count,omitempty"` + ChunkCount int64 `json:"chunk_count,omitempty"` + IsProcessing bool `json:"is_processing,omitempty"` + ProcessingCount int64 `json:"processing_count,omitempty"` + FailedCount int64 `json:"failed_count"` // always populated (no omitempty) } // CheckService is the narrow SDK surface needed for kb check. @@ -33,7 +37,7 @@ type CheckService interface { } var kbCheckFields = []string{ - "id", "reachable", "knowledge_count", "chunk_count", + "id", "reachable", "retrieval_ready", "knowledge_count", "chunk_count", "is_processing", "processing_count", "failed_count", } @@ -76,7 +80,7 @@ verification including failed-doc aggregation.`, UsedFor: "verify a knowledge base end-to-end: status plus failed-doc aggregation", RequiredFlags: []string{" (positional)"}, Examples: []string{"weknora kb check kb_abc"}, - Output: "envelope.data is {id, reachable, failed_count, ...}; deeper than `kb status`", + Output: "envelope.data is {id, reachable, retrieval_ready, failed_count, ...}; retrieval_ready=false means no embedding model is bound (run `kb config set`); deeper than `kb status`", }) return cmd } @@ -92,6 +96,7 @@ func runCheck(ctx context.Context, svc CheckService, id string) (*CheckResult, e res := &CheckResult{ ID: kb.ID, Reachable: true, + RetrievalReady: kb.EmbeddingModelID != "", KnowledgeCount: kb.KnowledgeCount, ChunkCount: kb.ChunkCount, IsProcessing: kb.IsProcessing, @@ -144,6 +149,7 @@ func writeCheckText(w io.Writer, res *CheckResult) error { if !res.Reachable { return nil } + fmt.Fprintf(w, "Retrieval: %v%s\n", res.RetrievalReady, retrievalHint(res.RetrievalReady)) fmt.Fprintf(w, "Knowledge: %d\n", res.KnowledgeCount) fmt.Fprintf(w, "Chunks: %d\n", res.ChunkCount) fmt.Fprintf(w, "Processing: %v (%d active)\n", res.IsProcessing, res.ProcessingCount) diff --git a/cli/cmd/kb/check_test.go b/cli/cmd/kb/check_test.go index 1450c28e0..73c17d7d8 100644 --- a/cli/cmd/kb/check_test.go +++ b/cli/cmd/kb/check_test.go @@ -39,7 +39,7 @@ func (f *fakeCheckSvc) ListKnowledgeWithFilter(_ context.Context, _ string, page func TestRunCheck_AggregatesFailed(t *testing.T) { svc := &fakeCheckSvc{ - kb: &sdk.KnowledgeBase{ID: "kb_x", KnowledgeCount: 5, ChunkCount: 20}, + kb: &sdk.KnowledgeBase{ID: "kb_x", KnowledgeCount: 5, ChunkCount: 20, EmbeddingModelID: "emb_1"}, failedDocs: []sdk.Knowledge{ {ID: "d1", ParseStatus: "failed"}, {ID: "d2", ParseStatus: "failed"}, @@ -55,6 +55,9 @@ func TestRunCheck_AggregatesFailed(t *testing.T) { if !res.Reachable { t.Error("Reachable=false, want true") } + if !res.RetrievalReady { + t.Error("RetrievalReady=false, want true when an embedding model is bound") + } if res.KnowledgeCount != 5 || res.ChunkCount != 20 { t.Errorf("got %+v", res) } diff --git a/cli/cmd/kb/config.go b/cli/cmd/kb/config.go index 5b1f3e6cb..ee9f2c1c0 100644 --- a/cli/cmd/kb/config.go +++ b/cli/cmd/kb/config.go @@ -12,25 +12,26 @@ import ( ) // kbConfigFields enumerates the fields surfaced for `--format json` discovery on -// `kb config`. Mirrors client.InitializationConfig. +// `kb config`. Mirrors client.KBModelConfigView (secret-free — no api keys). var kbConfigFields = []string{ - "chat_model_id", "embedding_model_id", "rerank_model_id", "multimodal_id", + "retrieval_ready", "embedding", "llm", "rerank", "multimodal", } // ConfigService is the narrow SDK surface this command depends on. type ConfigService interface { - GetInitializationConfig(ctx context.Context, kbID string) (*sdk.InitializationConfig, error) + GetInitializationConfig(ctx context.Context, kbID string) (*sdk.KBModelConfigView, error) } // NewCmdConfig builds `weknora kb config ` — read-only inspection of a -// knowledge base's model configuration (set it with `weknora kb init`). +// knowledge base's model configuration. Write it with `weknora kb config set`. func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "config ", - Short: "Show a knowledge base's model configuration", - Long: `Show the model configuration bound to a knowledge base: chat, embedding, -rerank, and multimodal model ids. An empty embedding_model_id means the KB is -not yet usable for retrieval — configure it with 'weknora kb init'.`, + Short: "Show a knowledge base's model configuration (set it with `config set`)", + Long: `Show the model configuration bound to a knowledge base: embedding, llm +(chat), rerank, and multimodal model names + source. retrieval_ready is false +until an embedding model is bound — configure it with 'weknora kb config set'. +Provider API keys are never shown.`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { fopts, err := cmdutil.CheckFormatFlag(c) @@ -45,12 +46,15 @@ not yet usable for retrieval — configure it with 'weknora kb init'.`, return runConfig(c.Context(), fopts, cli, args[0]) }, } + // `kb config` reads (this command's RunE); `kb config set` writes. Same + // read/write pairing as mainstream config surfaces. + cmd.AddCommand(newKBModelWriteCmd(f, "set ", []string{"weknora", "kb", "config", "set"})) cmdutil.AddFormatFlag(cmd, kbConfigFields...) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "show a KB's model config (chat/embedding/rerank/multimodal model ids). Empty embedding_model_id => not retrieval-ready; run `weknora kb init`.", + UsedFor: "show a KB's model config (embedding/llm/rerank/multimodal by name, secret-free). retrieval_ready=false => run `weknora kb config set`. Write config with `weknora kb config set`.", RequiredFlags: []string{" (positional)"}, Examples: []string{"weknora kb config kb_abc --jq .data.embedding_model_id"}, - Output: "envelope.data is {chat_model_id, embedding_model_id, rerank_model_id, multimodal_id}", + Output: "envelope.data is {retrieval_ready, embedding{configured,model_name,source,dimension}, llm{...}, rerank{enabled,model_name}, multimodal{enabled}} — secret-free (no provider api keys)", }) return cmd } @@ -61,27 +65,45 @@ func runConfig(ctx context.Context, fopts *cmdutil.FormatOptions, svc ConfigServ return cmdutil.WrapHTTP(err, "get config for knowledge base %q", kbID) } if cfg == nil { - cfg = &sdk.InitializationConfig{} + cfg = &sdk.KBModelConfigView{} } if fopts.WantsJSON() { return fopts.Emit(iostreams.IO.Out, cfg, nil) } w := iostreams.IO.Out - fmt.Fprintf(w, "%-11s %s\n", "EMBEDDING:", orNone(cfg.EmbeddingModelID)) - fmt.Fprintf(w, "%-11s %s\n", "CHAT:", orNone(cfg.ChatModelID)) - fmt.Fprintf(w, "%-11s %s\n", "RERANK:", orNone(cfg.RerankModelID)) - fmt.Fprintf(w, "%-11s %s\n", "MULTIMODAL:", orNone(cfg.MultimodalID)) - if cfg.EmbeddingModelID == "" { - fmt.Fprintln(w, "\n(no embedding model set — run `weknora kb init --embedding-model `)") + fmt.Fprintf(w, "%-13s %v\n", "RETRIEVAL:", readyLabel(cfg.RetrievalReady)) + fmt.Fprintf(w, "%-13s %s\n", "EMBEDDING:", slotLabel(cfg.Embedding)) + fmt.Fprintf(w, "%-13s %s\n", "CHAT (LLM):", slotLabel(cfg.LLM)) + fmt.Fprintf(w, "%-13s %s\n", "RERANK:", rerankLabel(cfg.Rerank)) + fmt.Fprintf(w, "%-13s %v\n", "MULTIMODAL:", cfg.Multimodal.Enabled) + if !cfg.RetrievalReady { + fmt.Fprintln(w, "\n(not retrieval-ready — no embedding model; run `weknora kb config set --chat-model --embedding-model `)") } return nil } -func orNone(s string) string { - if s == "" { +func readyLabel(ready bool) string { + if ready { + return "ready" + } + return "NOT ready (no embedding model)" +} + +func slotLabel(s sdk.ModelSlotView) string { + if !s.Configured { return "(unset)" } - return s + if s.Source != "" { + return fmt.Sprintf("%s (%s)", s.ModelName, s.Source) + } + return s.ModelName +} + +func rerankLabel(r sdk.RerankSlotView) string { + if !r.Enabled { + return "(disabled)" + } + return r.ModelName } // compile-time check: the production SDK client implements ConfigService. diff --git a/cli/cmd/kb/init.go b/cli/cmd/kb/config_set.go similarity index 58% rename from cli/cmd/kb/init.go rename to cli/cmd/kb/config_set.go index a6096ed19..624161e43 100644 --- a/cli/cmd/kb/init.go +++ b/cli/cmd/kb/config_set.go @@ -12,43 +12,40 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// kbInitFields enumerates the fields surfaced for `--format json` discovery on -// `kb init`. The result is the resulting InitializationConfig (read back). -var kbInitFields = []string{ - "chat_model_id", "embedding_model_id", "rerank_model_id", "multimodal_id", -} - -type InitOptions struct { +type ConfigSetOptions struct { ChatModel string EmbeddingModel string Yes bool DryRun bool } -// InitService is the narrow SDK surface this command depends on. SetKBModelConfig +// ConfigSetService is the narrow SDK surface this command depends on. SetKBModelConfig // points the KB at already-registered models; GetInitializationConfig re-reads // the server's resulting state so the success envelope reflects what stuck. -type InitService interface { +type ConfigSetService interface { SetKBModelConfig(ctx context.Context, kbID string, cfg *sdk.KBModelConfig) error - GetInitializationConfig(ctx context.Context, kbID string) (*sdk.InitializationConfig, error) + GetInitializationConfig(ctx context.Context, kbID string) (*sdk.KBModelConfigView, error) } -// NewCmdInit builds `weknora kb init ` — bind models to a knowledge base -// so it becomes usable for retrieval and generation. -func NewCmdInit(f *cmdutil.Factory) *cobra.Command { - opts := &InitOptions{} +// newKBModelWriteCmd builds the `kb config set` model-binding write command. +// head is the argv prefix used for the risk action and retry_argv (weknora kb +// config set). +func newKBModelWriteCmd(f *cmdutil.Factory, use string, head []string) *cobra.Command { + opts := &ConfigSetOptions{} + action := strings.Join(head[1:], ".") // e.g. "kb.config.set" cmd := &cobra.Command{ - Use: "init ", - Short: "Configure a knowledge base's models (make it usable)", + Use: use, + Short: "Bind embedding + chat models to a knowledge base (make it usable)", Long: `Bind already-registered models to a knowledge base so it can embed, retrieve, and generate. Both --chat-model (LLM, used for generation/summary) and --embedding-model (used for retrieval) are required; register models first with 'weknora model create' and discover ids with 'weknora model list'. High-risk write: changing a KB's embedding model affects how its content is -indexed and searched (and the server refuses once the KB has documents). -Without -y/--yes in a non-TTY / JSON context it exits 10 -(input.confirmation_required) without applying the change.`, +indexed and searched (the server refuses to CHANGE it once the KB has +documents; setting it on an unconfigured KB is allowed). Without -y/--yes in a +non-TTY / JSON context it exits 10 (input.confirmation_required) without +applying the change.`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { fopts, err := cmdutil.CheckFormatFlag(c) @@ -60,11 +57,11 @@ Without -y/--yes in a non-TTY / JSON context it exits 10 kbID := args[0] // Validate required flags before the dry-run gate so --dry-run rejects // identically to the live path. - if err := validateInitFlags(opts); err != nil { + if err := validateConfigSetFlags(opts); err != nil { return err } if handled, err := cmdutil.HandleDryRun(c, opts.DryRun, cmdutil.DryRunPlan{ - Action: "kb.init", + Action: action, Args: map[string]any{"kb": kbID, "chat_model": opts.ChatModel, "embedding_model": opts.EmbeddingModel}, }); handled { return err @@ -74,8 +71,8 @@ Without -y/--yes in a non-TTY / JSON context it exits 10 return err } if err := cmdutil.ConfirmDestructive(f.Prompter(), opts.Yes, fopts.WantsJSON(), - "configure", "knowledge base", kbID, "kb.init", - cmdutil.BuildRetryArgv(c, []string{"weknora", "kb", "init", kbID}, "chat-model", "embedding-model", "format")); err != nil { + "configure", "knowledge base", kbID, action, + cmdutil.BuildRetryArgv(c, append(append([]string{}, head...), kbID), "chat-model", "embedding-model", "format")); err != nil { return err } // Resolve name-or-id for the model flags (a UUID passes through; a @@ -87,30 +84,30 @@ Without -y/--yes in a non-TTY / JSON context it exits 10 if opts.EmbeddingModel, err = cmdutil.ResolveModelRef(c.Context(), cli, opts.EmbeddingModel, "Embedding"); err != nil { return err } - return runInit(c.Context(), opts, fopts, cli, kbID) + return runConfigSet(c.Context(), opts, fopts, cli, kbID) }, } cmd.Flags().StringVar(&opts.ChatModel, "chat-model", "", "Chat / LLM model id or name for generation & summary (required) — see `weknora model list`") cmd.Flags().StringVar(&opts.EmbeddingModel, "embedding-model", "", "Embedding model id or name for retrieval (required) — see `weknora model list`") - cmdutil.AddFormatFlag(cmd, kbInitFields...) + cmdutil.AddFormatFlag(cmd, kbConfigFields...) cmdutil.AddDryRunFlag(cmd, &opts.DryRun) - cmdutil.SetRisk(cmd, "kb.init") + cmdutil.SetRisk(cmd, action) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "bind models to a KB so it becomes usable. --chat-model and --embedding-model are required and accept a model id or name; discover them with `weknora model list`.", + UsedFor: "bind models to a KB so it becomes retrieval-ready. --chat-model and --embedding-model are required and accept a model id or name; discover them with `weknora model list`. Read the result back with `weknora kb config`.", RequiredFlags: []string{" (positional)", "--chat-model", "--embedding-model"}, Examples: []string{ - "weknora kb init kb_abc --chat-model model_llm --embedding-model model_emb -y", + "weknora kb config set kb_abc --chat-model model_llm --embedding-model model_emb -y", }, - Output: "envelope.data is the resulting {chat_model_id, embedding_model_id, rerank_model_id, multimodal_id}", + Output: "envelope.data is the resulting secret-free config view {retrieval_ready, embedding, llm, rerank, multimodal}", Warnings: []string{ "Requires explicit user approval (exit 10 / input.confirmation_required); never auto-add -y.", - "The server refuses to change the embedding model of a KB that already has documents.", + "The server refuses to CHANGE the embedding model of a KB that already has documents (setting it on an unconfigured KB is fine).", }, }) return cmd } -func validateInitFlags(opts *InitOptions) error { +func validateConfigSetFlags(opts *ConfigSetOptions) error { var missing []string if strings.TrimSpace(opts.ChatModel) == "" { missing = append(missing, "--chat-model") @@ -123,14 +120,13 @@ func validateInitFlags(opts *InitOptions) error { } return &cmdutil.Error{ Code: cmdutil.CodeInputMissingFlag, - Message: "kb init requires " + strings.Join(missing, " and "), + Message: "kb config set requires " + strings.Join(missing, " and "), Hint: "discover model ids with `weknora model list` (or register one with `weknora model create`), then pass --chat-model --embedding-model ", } } - -func runInit(ctx context.Context, opts *InitOptions, fopts *cmdutil.FormatOptions, svc InitService, kbID string) error { - if err := validateInitFlags(opts); err != nil { +func runConfigSet(ctx context.Context, opts *ConfigSetOptions, fopts *cmdutil.FormatOptions, svc ConfigSetService, kbID string) error { + if err := validateConfigSetFlags(opts); err != nil { return err } cfg := &sdk.KBModelConfig{ @@ -140,19 +136,31 @@ func runInit(ctx context.Context, opts *InitOptions, fopts *cmdutil.FormatOption if err := svc.SetKBModelConfig(ctx, kbID, cfg); err != nil { return cmdutil.WrapHTTP(err, "configure knowledge base %q", kbID) } - // Re-read the server's resulting state so the envelope reflects what stuck. + // Re-read the server's resulting state (secret-free view) so the envelope + // reflects what stuck — the same shape `kb config` returns. result, err := svc.GetInitializationConfig(ctx, kbID) if err != nil || result == nil { // The write succeeded; surface what we applied if the read-back failed. - result = &sdk.InitializationConfig{ChatModelID: opts.ChatModel, EmbeddingModelID: opts.EmbeddingModel} + result = &sdk.KBModelConfigView{ + RetrievalReady: opts.EmbeddingModel != "", + Embedding: sdk.ModelSlotView{Configured: opts.EmbeddingModel != "", ModelName: opts.EmbeddingModel}, + LLM: sdk.ModelSlotView{Configured: opts.ChatModel != "", ModelName: opts.ChatModel}, + } } if fopts.WantsJSON() { return fopts.Emit(iostreams.IO.Out, result, nil) } fmt.Fprintf(iostreams.IO.Out, "✓ Configured knowledge base %s (chat: %s, embedding: %s)\n", - kbID, result.ChatModelID, result.EmbeddingModelID) + kbID, orUnset(result.LLM.ModelName), orUnset(result.Embedding.ModelName)) return nil } -// compile-time check: the production SDK client implements InitService. -var _ InitService = (*sdk.Client)(nil) +func orUnset(s string) string { + if s == "" { + return "(unset)" + } + return s +} + +// compile-time check: the production SDK client implements ConfigSetService. +var _ ConfigSetService = (*sdk.Client)(nil) diff --git a/cli/cmd/kb/init_test.go b/cli/cmd/kb/config_set_test.go similarity index 58% rename from cli/cmd/kb/init_test.go rename to cli/cmd/kb/config_set_test.go index 8993720dd..4548823ee 100644 --- a/cli/cmd/kb/init_test.go +++ b/cli/cmd/kb/config_set_test.go @@ -19,7 +19,7 @@ import ( type fakeInitSvc struct { gotKB string gotCfg *sdk.KBModelConfig - result *sdk.InitializationConfig + result *sdk.KBModelConfigView setErr error } @@ -29,24 +29,25 @@ func (f *fakeInitSvc) SetKBModelConfig(_ context.Context, kbID string, cfg *sdk. return f.setErr } -func (f *fakeInitSvc) GetInitializationConfig(_ context.Context, _ string) (*sdk.InitializationConfig, error) { +func (f *fakeInitSvc) GetInitializationConfig(_ context.Context, _ string) (*sdk.KBModelConfigView, error) { if f.result != nil { return f.result, nil } if f.gotCfg == nil { - return &sdk.InitializationConfig{}, nil + return &sdk.KBModelConfigView{}, nil } - return &sdk.InitializationConfig{ - ChatModelID: f.gotCfg.LLMModelID, - EmbeddingModelID: f.gotCfg.EmbeddingModelID, + return &sdk.KBModelConfigView{ + RetrievalReady: f.gotCfg.EmbeddingModelID != "", + Embedding: sdk.ModelSlotView{Configured: f.gotCfg.EmbeddingModelID != "", ModelName: f.gotCfg.EmbeddingModelID}, + LLM: sdk.ModelSlotView{Configured: f.gotCfg.LLMModelID != "", ModelName: f.gotCfg.LLMModelID}, }, nil } -func TestKBInit_AppliesAndEmitsResult(t *testing.T) { +func TestKBConfigSet_AppliesAndEmitsResult(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeInitSvc{} - opts := &InitOptions{ChatModel: "model_llm", EmbeddingModel: "model_emb"} - require.NoError(t, runInit(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_abc")) + opts := &ConfigSetOptions{ChatModel: "model_llm", EmbeddingModel: "model_emb"} + require.NoError(t, runConfigSet(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_abc")) assert.Equal(t, "kb_abc", svc.gotKB) require.NotNil(t, svc.gotCfg) @@ -54,46 +55,47 @@ func TestKBInit_AppliesAndEmitsResult(t *testing.T) { assert.Equal(t, "model_emb", svc.gotCfg.EmbeddingModelID) var env struct { - OK bool `json:"ok"` - Data sdk.InitializationConfig `json:"data"` + OK bool `json:"ok"` + Data sdk.KBModelConfigView `json:"data"` } require.NoError(t, json.Unmarshal(out.Bytes(), &env)) assert.True(t, env.OK) - assert.Equal(t, "model_emb", env.Data.EmbeddingModelID) - assert.Equal(t, "model_llm", env.Data.ChatModelID) + assert.Equal(t, "model_emb", env.Data.Embedding.ModelName) + assert.Equal(t, "model_llm", env.Data.LLM.ModelName) + assert.True(t, env.Data.RetrievalReady) } -func TestKBInit_RequiresBothModels(t *testing.T) { +func TestKBConfigSet_RequiresBothModels(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeInitSvc{} // Missing both. - err := runInit(context.Background(), &InitOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_abc") + err := runConfigSet(context.Background(), &ConfigSetOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_abc") var ce *cmdutil.Error require.ErrorAs(t, err, &ce) assert.Equal(t, cmdutil.CodeInputMissingFlag, ce.Code) assert.Equal(t, "", svc.gotKB, "must not call SetKBModelConfig when flags are missing") // Missing just embedding. - err = runInit(context.Background(), &InitOptions{ChatModel: "m"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_abc") + err = runConfigSet(context.Background(), &ConfigSetOptions{ChatModel: "m"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc, "kb_abc") require.ErrorAs(t, err, &ce) assert.Contains(t, ce.Message, "--embedding-model") } -func TestKBInit_WriteSucceedsReadbackFails(t *testing.T) { +func TestKBConfigSet_WriteSucceedsReadbackFails(t *testing.T) { out, _ := iostreams.SetForTest(t) svc2 := &readbackErrSvc{fakeInitSvc: &fakeInitSvc{}} - opts := &InitOptions{ChatModel: "model_llm", EmbeddingModel: "model_emb"} - require.NoError(t, runInit(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc2, "kb_abc")) + opts := &ConfigSetOptions{ChatModel: "model_llm", EmbeddingModel: "model_emb"} + require.NoError(t, runConfigSet(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc2, "kb_abc")) var env struct { - Data sdk.InitializationConfig `json:"data"` + Data sdk.KBModelConfigView `json:"data"` } require.NoError(t, json.Unmarshal(out.Bytes(), &env)) - assert.Equal(t, "model_emb", env.Data.EmbeddingModelID, "falls back to applied config when read-back fails") + assert.Equal(t, "model_emb", env.Data.Embedding.ModelName, "falls back to applied config when read-back fails") } type readbackErrSvc struct{ *fakeInitSvc } -func (s *readbackErrSvc) GetInitializationConfig(_ context.Context, _ string) (*sdk.InitializationConfig, error) { +func (s *readbackErrSvc) GetInitializationConfig(_ context.Context, _ string) (*sdk.KBModelConfigView, error) { return nil, errors.New("read-back boom") } @@ -116,13 +118,14 @@ func withRootKB(sub *cobra.Command, args ...string) *cobra.Command { return root } -func TestKBInit_RequiresConfirmation(t *testing.T) { +func TestKBConfigSet_RequiresConfirmation(t *testing.T) { iostreams.SetForTest(t) f := &cmdutil.Factory{ Client: func() (*sdk.Client, error) { return nil, nil }, Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} }, } - root := withRootKB(NewCmdInit(f), "kb_abc", "--chat-model", "model_llm", "--embedding-model", "model_emb", "--format", "json") + // Drive `kb config set` (the config parent routes to its `set` subcommand). + root := withRootKB(NewCmdConfig(f), "set", "kb_abc", "--chat-model", "model_llm", "--embedding-model", "model_emb", "--format", "json") err := root.Execute() require.Error(t, err) var ce *cmdutil.Error @@ -131,4 +134,5 @@ func TestKBInit_RequiresConfirmation(t *testing.T) { assert.Equal(t, 10, cmdutil.ExitCode(err)) assert.Contains(t, ce.RetryArgv, "-y") assert.Contains(t, ce.RetryArgv, "model_emb") + assert.Contains(t, ce.RetryArgv, "set", "retry_argv should target `kb config set`") } diff --git a/cli/cmd/kb/config_test.go b/cli/cmd/kb/config_test.go index 98771271e..5e42adf78 100644 --- a/cli/cmd/kb/config_test.go +++ b/cli/cmd/kb/config_test.go @@ -14,37 +14,57 @@ import ( ) type fakeConfigSvc struct { - cfg *sdk.InitializationConfig + cfg *sdk.KBModelConfigView err error } -func (f *fakeConfigSvc) GetInitializationConfig(_ context.Context, _ string) (*sdk.InitializationConfig, error) { +func (f *fakeConfigSvc) GetInitializationConfig(_ context.Context, _ string) (*sdk.KBModelConfigView, error) { return f.cfg, f.err } func TestKBConfig_EmitsConfig(t *testing.T) { out, _ := iostreams.SetForTest(t) - svc := &fakeConfigSvc{cfg: &sdk.InitializationConfig{EmbeddingModelID: "model_emb", ChatModelID: "model_chat"}} + svc := &fakeConfigSvc{cfg: &sdk.KBModelConfigView{ + RetrievalReady: true, + Embedding: sdk.ModelSlotView{Configured: true, ModelName: "model_emb", Source: "remote"}, + LLM: sdk.ModelSlotView{Configured: true, ModelName: "model_chat"}, + }} require.NoError(t, runConfig(context.Background(), &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_abc")) var env struct { - OK bool `json:"ok"` - Data sdk.InitializationConfig `json:"data"` + OK bool `json:"ok"` + Data sdk.KBModelConfigView `json:"data"` } require.NoError(t, json.Unmarshal(out.Bytes(), &env)) assert.True(t, env.OK) - assert.Equal(t, "model_emb", env.Data.EmbeddingModelID) - assert.Equal(t, "model_chat", env.Data.ChatModelID) + assert.Equal(t, "model_emb", env.Data.Embedding.ModelName) + assert.Equal(t, "model_chat", env.Data.LLM.ModelName) + assert.True(t, env.Data.RetrievalReady) } -// TestKBConfig_NilConfig: a nil server config (KB not yet initialized) emits an -// empty object, not a crash. +// TestKBConfig_SecretFree: the view type has no apiKey/baseUrl field, so the +// JSON output can never carry provider credentials — the CLI never echoes the +// keys the server returns for the web config form. +func TestKBConfig_SecretFree(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeConfigSvc{cfg: &sdk.KBModelConfigView{ + Embedding: sdk.ModelSlotView{Configured: true, ModelName: "e", Source: "remote"}, + }} + require.NoError(t, runConfig(context.Background(), &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_abc")) + assert.NotContains(t, out.String(), "apiKey") + assert.NotContains(t, out.String(), "api_key") + assert.NotContains(t, out.String(), "baseUrl") +} + +// TestKBConfig_NilConfig: a nil server config (KB not yet initialized) emits +// retrieval_ready:false, not a crash. func TestKBConfig_NilConfig(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeConfigSvc{cfg: nil} require.NoError(t, runConfig(context.Background(), &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "kb_abc")) var env struct { - Data sdk.InitializationConfig `json:"data"` + Data sdk.KBModelConfigView `json:"data"` } require.NoError(t, json.Unmarshal(out.Bytes(), &env)) - assert.Empty(t, env.Data.EmbeddingModelID) + assert.False(t, env.Data.RetrievalReady) + assert.Empty(t, env.Data.Embedding.ModelName) } diff --git a/cli/cmd/kb/create.go b/cli/cmd/kb/create.go index 37e7fab7e..a2a595e3f 100644 --- a/cli/cmd/kb/create.go +++ b/cli/cmd/kb/create.go @@ -9,6 +9,7 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/output" sdk "github.com/Tencent/WeKnora/client" ) @@ -29,6 +30,7 @@ type CreateOptions struct { Name string Description string EmbeddingModel string + ChatModel string StorageProvider string DryRun bool } @@ -84,16 +86,23 @@ func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { } // --embedding-model accepts a model id or name (a UUID passes // through; a name resolves among Embedding models). Configuring a - // KB's models fully is `weknora kb init`; this just pre-sets the + // KB's models fully is `weknora kb config set`; this just pre-sets the // embedding model at creation. if opts.EmbeddingModel, err = cmdutil.ResolveModelRef(c.Context(), cli, opts.EmbeddingModel, "Embedding"); err != nil { return err } + // --chat-model (id or name) pre-sets the KB's LLM at creation, so a + // KB can be born retrieval-ready in one step. Full model config + // (rerank / multimodal) is still `weknora kb config set`. + if opts.ChatModel, err = cmdutil.ResolveModelRef(c.Context(), cli, opts.ChatModel, "KnowledgeQA"); err != nil { + return err + } return runCreate(c.Context(), opts, fopts, cli) }, } cmd.Flags().StringVar(&opts.Description, "description", "", "Knowledge base description (optional)") - cmd.Flags().StringVar(&opts.EmbeddingModel, "embedding-model", "", "Embedding model id or name (optional; configure models fully with `weknora kb init`)") + cmd.Flags().StringVar(&opts.EmbeddingModel, "embedding-model", "", "Embedding model id or name (optional; makes the KB retrieval-ready at creation)") + cmd.Flags().StringVar(&opts.ChatModel, "chat-model", "", "Chat/LLM model id or name (optional; pre-set the KB's answer model at creation)") cmd.Flags().StringVar(&opts.StorageProvider, "storage-provider", "", "Storage provider for documents in this KB: "+strings.Join(storageProviderValues, " | ")+" (optional; server default when unset)") cmdutil.AddFormatFlag(cmd, kbCreateFields...) @@ -103,10 +112,10 @@ func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { RequiredFlags: []string{" (positional)"}, Examples: []string{ `weknora kb create "Eng Docs"`, - `weknora kb create "Eng Docs" --description "engineering knowledge base"`, + `weknora kb create "Eng Docs" --embedding-model text-embedding-3-small --chat-model gpt-4o-mini # retrieval-ready in one step`, `weknora kb create "Eng Docs" --jq .data.id # capture id to chain into doc upload --kb`, }, - Output: "envelope.data is the created KnowledgeBase object with id, name, type, embedding_model_id", + Output: "envelope.data is the created KnowledgeBase object with id, name, type, embedding_model_id, summary_model_id", }) return cmd } @@ -125,6 +134,9 @@ func runCreate(ctx context.Context, opts *CreateOptions, fopts *cmdutil.FormatOp if opts.EmbeddingModel != "" { req.EmbeddingModelID = opts.EmbeddingModel } + if opts.ChatModel != "" { + req.SummaryModelID = opts.ChatModel + } if opts.StorageProvider != "" { canonSP, err := cmdutil.ValidateEnum("storage-provider", opts.StorageProvider, storageProviderValues) if err != nil { @@ -138,9 +150,20 @@ func runCreate(ctx context.Context, opts *CreateOptions, fopts *cmdutil.FormatOp return cmdutil.WrapHTTP(err, "create knowledge base") } + // A KB with no embedding model can hold documents but never index/retrieve + // them — surface the next step at the point of creation instead of leaving + // the agent to discover a silent-draft KB via a later empty search. + var meta *output.Meta + if created.EmbeddingModelID == "" { + meta = &output.Meta{Hint: "retrieval_ready=false: no embedding model bound. Uploaded docs will not be searchable until you run `weknora kb config set " + created.ID + " --embedding-model --chat-model ` (create the KB with --embedding-model/--chat-model to skip this step)."} + } + if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, created, nil) + return fopts.Emit(iostreams.IO.Out, created, meta) } fmt.Fprintf(iostreams.IO.Out, "✓ Created knowledge base %q (id: %s)\n", created.Name, created.ID) + if meta != nil { + fmt.Fprintf(iostreams.IO.Out, "⚠ %s\n", meta.Hint) + } return nil } diff --git a/cli/cmd/kb/create_test.go b/cli/cmd/kb/create_test.go index a92ca438b..b4e844740 100644 --- a/cli/cmd/kb/create_test.go +++ b/cli/cmd/kb/create_test.go @@ -66,6 +66,50 @@ func TestCreate_Success_OmitsEmbeddingModelWhenEmpty(t *testing.T) { assert.Equal(t, "", svc.got.EmbeddingModelID, "embedding-model unset ⇒ empty in request") } +// A KB created without an embedding model can never retrieve; the create result +// must hand the agent the fix (kb config set) instead of a silent unusable KB. +func TestCreate_HintsWhenNoEmbeddingModel(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.KnowledgeBase{ID: "kb_x", Name: "n"}} + require.NoError(t, runCreate(context.Background(), &CreateOptions{Name: "n"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc)) + var env struct { + Meta struct { + Hint string `json:"hint"` + } `json:"meta"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env)) + assert.Contains(t, env.Meta.Hint, "kb config set", "unconfigured KB must hint the retrieval-readiness fix") +} + +// A retrieval-ready KB (embedding model bound) carries no such hint — no noise. +func TestCreate_NoHintWhenEmbeddingModelBound(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.KnowledgeBase{ID: "kb_x", Name: "n", EmbeddingModelID: "emb_1"}} + require.NoError(t, runCreate(context.Background(), &CreateOptions{Name: "n", EmbeddingModel: "emb_1"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc)) + var env struct { + Meta *struct { + Hint string `json:"hint"` + } `json:"meta"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env)) + if env.Meta != nil { + assert.Empty(t, env.Meta.Hint, "retrieval-ready KB must not emit a readiness hint") + } +} + +// TestCreate_ChatModelSetsSummaryModelID: --chat-model rides the create request +// as summary_model_id, so a KB can be born retrieval-ready in one step. +func TestCreate_ChatModelSetsSummaryModelID(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeCreateSvc{resp: &sdk.KnowledgeBase{ID: "kb_x", Name: "n"}} + opts := &CreateOptions{Name: "n", EmbeddingModel: "emb_x", ChatModel: "chat_x"} + require.NoError(t, runCreate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc)) + + require.NotNil(t, svc.got) + assert.Equal(t, "emb_x", svc.got.EmbeddingModelID) + assert.Equal(t, "chat_x", svc.got.SummaryModelID, "--chat-model must set summary_model_id on the create request") +} + func TestCreate_NameRequired(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeCreateSvc{} diff --git a/cli/cmd/kb/delete.go b/cli/cmd/kb/delete.go index 02a5cd52d..4167504a2 100644 --- a/cli/cmd/kb/delete.go +++ b/cli/cmd/kb/delete.go @@ -81,6 +81,7 @@ exactly to guard against unintended deletes.`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "permanently delete a knowledge base and all its contents", RequiredFlags: []string{" (positional)"}, + Output: "envelope.data is {id, deleted:true}", Examples: []string{ "weknora kb delete kb_abc -y", "weknora kb delete kb_abc -y --format json", diff --git a/cli/cmd/kb/edit.go b/cli/cmd/kb/edit.go index 25ffed6c1..c77117583 100644 --- a/cli/cmd/kb/edit.go +++ b/cli/cmd/kb/edit.go @@ -120,6 +120,7 @@ to the user first.`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "update a knowledge base's name or description", RequiredFlags: []string{" (positional)", "--name or --description (at least one)"}, + Output: "envelope.data is the updated KnowledgeBase object (id, name, description)", Examples: []string{ "weknora kb update kb_abc --name \"New Name\" -y", "weknora kb update kb_abc --description \"Updated desc\" --format json -y", diff --git a/cli/cmd/kb/kb.go b/cli/cmd/kb/kb.go index 182f752e8..eb3f30d9b 100644 --- a/cli/cmd/kb/kb.go +++ b/cli/cmd/kb/kb.go @@ -25,7 +25,6 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(NewCmdUnpin(f)) cmd.AddCommand(NewCmdStatus(f)) cmd.AddCommand(NewCmdCheck(f)) - cmd.AddCommand(NewCmdConfig(f)) - cmd.AddCommand(NewCmdInit(f)) + cmd.AddCommand(NewCmdConfig(f)) // `config` also hosts the `config set` write subcommand return cmd } diff --git a/cli/cmd/kb/status.go b/cli/cmd/kb/status.go index 760e470de..fef06aa3e 100644 --- a/cli/cmd/kb/status.go +++ b/cli/cmd/kb/status.go @@ -16,12 +16,16 @@ import ( // Shallow read only: 1 HTTP call, no failed-doc aggregation. // For deep verification including failed_count, use `kb check `. type StatusResult struct { - ID string `json:"id"` - Reachable bool `json:"reachable"` - KnowledgeCount int64 `json:"knowledge_count,omitempty"` - ChunkCount int64 `json:"chunk_count,omitempty"` - IsProcessing bool `json:"is_processing,omitempty"` - ProcessingCount int64 `json:"processing_count,omitempty"` + ID string `json:"id"` + Reachable bool `json:"reachable"` + // RetrievalReady is false when the KB has no embedding model bound — it can + // never index or retrieve until `kb config set` runs. Always emitted (no + // omitempty) so a not-ready KB is visible, not silently green. + RetrievalReady bool `json:"retrieval_ready"` + KnowledgeCount int64 `json:"knowledge_count,omitempty"` + ChunkCount int64 `json:"chunk_count,omitempty"` + IsProcessing bool `json:"is_processing,omitempty"` + ProcessingCount int64 `json:"processing_count,omitempty"` } // StatusService is the narrow SDK surface needed for kb status. @@ -30,7 +34,7 @@ type StatusService interface { } var kbStatusFields = []string{ - "id", "reachable", "knowledge_count", "chunk_count", + "id", "reachable", "retrieval_ready", "knowledge_count", "chunk_count", "is_processing", "processing_count", } @@ -73,7 +77,7 @@ For full metadata (config / pinned / tenant), use 'weknora kb view '.`, UsedFor: "shallow health probe of a knowledge base (one HTTP call): reachability, no failed-doc aggregation", RequiredFlags: []string{" (positional)"}, Examples: []string{"weknora kb status kb_abc"}, - Output: "envelope.data is {id, reachable, ...}; use `kb check` for deep failed-doc aggregation", + Output: "envelope.data is {id, reachable, retrieval_ready, ...}; retrieval_ready=false means no embedding model is bound (run `kb config set`), so the KB cannot index/retrieve; use `kb check` for deep failed-doc aggregation", }) return cmd } @@ -89,6 +93,7 @@ func runStatus(ctx context.Context, svc StatusService, id string) (*StatusResult return &StatusResult{ ID: kb.ID, Reachable: true, + RetrievalReady: kb.EmbeddingModelID != "", KnowledgeCount: kb.KnowledgeCount, ChunkCount: kb.ChunkCount, IsProcessing: kb.IsProcessing, @@ -115,11 +120,20 @@ func writeStatusText(w io.Writer, res *StatusResult) error { if !res.Reachable { return nil } + fmt.Fprintf(w, "Retrieval: %v%s\n", res.RetrievalReady, retrievalHint(res.RetrievalReady)) fmt.Fprintf(w, "Knowledge: %d\n", res.KnowledgeCount) fmt.Fprintf(w, "Chunks: %d\n", res.ChunkCount) fmt.Fprintf(w, "Processing: %v (%d active)\n", res.IsProcessing, res.ProcessingCount) return nil } +// retrievalHint annotates a not-ready KB in text output with the fix. +func retrievalHint(ready bool) string { + if ready { + return "" + } + return " ← no embedding model bound; run `weknora kb config set `" +} + // compile-time check: SDK client satisfies StatusService. var _ StatusService = (*sdk.Client)(nil) diff --git a/cli/cmd/kb/status_test.go b/cli/cmd/kb/status_test.go index 7dc60d45a..853fab904 100644 --- a/cli/cmd/kb/status_test.go +++ b/cli/cmd/kb/status_test.go @@ -26,11 +26,12 @@ func (f *fakeStatusSvc) GetKnowledgeBase(_ context.Context, id string) (*sdk.Kno func TestRunStatus_ShallowFields(t *testing.T) { svc := &fakeStatusSvc{kb: &sdk.KnowledgeBase{ - ID: "kb_x", - KnowledgeCount: 42, - ChunkCount: 100, - IsProcessing: true, - ProcessingCount: 3, + ID: "kb_x", + KnowledgeCount: 42, + ChunkCount: 100, + IsProcessing: true, + ProcessingCount: 3, + EmbeddingModelID: "emb_1", }} res, err := runStatus(context.Background(), svc, "kb_x") if err != nil { @@ -42,6 +43,22 @@ func TestRunStatus_ShallowFields(t *testing.T) { if res.KnowledgeCount != 42 || res.ChunkCount != 100 || res.ProcessingCount != 3 || !res.IsProcessing { t.Errorf("got %+v", res) } + if !res.RetrievalReady { + t.Error("RetrievalReady=false, want true when an embedding model is bound") + } +} + +// A KB with no embedding model can never retrieve — the health probe must say +// so (retrieval_ready=false), not report a silent all-green status. +func TestRunStatus_RetrievalNotReadyWithoutEmbeddingModel(t *testing.T) { + svc := &fakeStatusSvc{kb: &sdk.KnowledgeBase{ID: "kb_x", KnowledgeCount: 1}} + res, err := runStatus(context.Background(), svc, "kb_x") + if err != nil { + t.Fatalf("runStatus: %v", err) + } + if res.RetrievalReady { + t.Error("RetrievalReady=true, want false when no embedding model is bound") + } } func TestRunStatus_Unreachable(t *testing.T) { diff --git a/cli/cmd/link/link.go b/cli/cmd/link/link.go index 53ffef93d..911a7d457 100644 --- a/cli/cmd/link/link.go +++ b/cli/cmd/link/link.go @@ -163,7 +163,15 @@ func resolveProfile(f *cmdutil.Factory) (string, error) { return "", err } if cfg.CurrentProfile == "" { - return "", cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, "no active profile; run `weknora auth login` first") + // `link` binds a directory to a profile+KB, so it needs a configured + // profile — env credentials (WEKNORA_API_KEY) alone have no profile to + // record. Point at profile setup (not `auth login`, which loops with no + // profile) and name the headless alternative so an env-cred agent isn't + // stranded on a misleading hint. + return "", cmdutil.NewError(cmdutil.CodeAuthUnauthenticated, + "`link` records an active profile, but none is configured"). + WithHint("register one with `weknora profile add --host --use`; for a headless (WEKNORA_API_KEY) workflow, skip `link` and pass --kb per command or set WEKNORA_KB_ID"). + WithRetryArgv([]string{"weknora", "profile", "add", "--help"}) } return cfg.CurrentProfile, nil } diff --git a/cli/cmd/link/unlink.go b/cli/cmd/link/unlink.go index 4c8f9d57b..162850472 100644 --- a/cli/cmd/link/unlink.go +++ b/cli/cmd/link/unlink.go @@ -79,6 +79,9 @@ is present anywhere in the parent chain.`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "Remove the .weknora/project.yaml KB binding from the current directory tree. No flags required; walks up from cwd to find the link.", Output: "envelope.data has project_link_path of the removed file", + Examples: []string{ + "weknora unlink", + }, }) return cmd } diff --git a/cli/cmd/mcp/serve.go b/cli/cmd/mcp/serve.go index 3c6246bfc..488c26f20 100644 --- a/cli/cmd/mcp/serve.go +++ b/cli/cmd/mcp/serve.go @@ -49,6 +49,9 @@ Consult your MCP client's documentation for the exact config-file location.`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "run weknora as a long-lived MCP (Model Context Protocol) server over stdio for an IDE/host agent", Output: "no stdout payload (JSON-RPC 2.0 protocol traffic); logs go to stderr", + Examples: []string{ + "weknora mcp serve", + }, Warnings: []string{ "this is a long-running stdio server, not a one-shot command — register it in your MCP client (command: weknora, args: [mcp, serve])", "exits with auth.unauthenticated at startup if no profile is configured", diff --git a/cli/cmd/message/search.go b/cli/cmd/message/search.go index 0df91abe0..cf3345ea0 100644 --- a/cli/cmd/message/search.go +++ b/cli/cmd/message/search.go @@ -76,7 +76,7 @@ func NewCmdSearch(f *cmdutil.Factory) *cobra.Command { `weknora message search "deploy steps"`, `weknora message search "deploy steps" --session sess_abc --limit 5`, }, - Output: "envelope.data is an array of grouped results (request_id, session_id, query_content, answer_content, score); meta.total_count is the server-side total. --mode accepts keyword | vector | hybrid (server default: hybrid)", + Output: "envelope.data is an array of grouped results (request_id, session_id, query_content, answer_content, score); meta.count is the returned count, meta.total_count is the server-side total. --mode accepts keyword | vector | hybrid (server default: hybrid)", }) return cmd } diff --git a/cli/cmd/model/create.go b/cli/cmd/model/create.go index e04795225..2d1de41b1 100644 --- a/cli/cmd/model/create.go +++ b/cli/cmd/model/create.go @@ -85,7 +85,7 @@ func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { Use: "create ", Short: "Register a model (embedding / rerank / chat / VLLM / ASR)", Long: `Register a model on the server so it can back a knowledge base's embedding / -summary config (see 'weknora kb init') or an agent (--model). +summary config (see 'weknora kb config set') or an agent (--model). is the model name as the provider knows it (e.g. "nomic-embed-text", "gpt-4o", "qwen2"). --type and --source are required. @@ -178,7 +178,7 @@ else goes through repeatable --param key=value.`, cmdutil.AddFormatFlag(cmd, modelCreateFields...) cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "register a model (embedding/rerank/chat/VLLM/ASR) so a KB or agent can use it; capture .data.id to pass to `weknora kb init` / `agent create --model`.", + UsedFor: "register a model (embedding/rerank/chat/VLLM/ASR) so a KB or agent can use it; capture .data.id to pass to `weknora kb config set` / `agent create --model`.", RequiredFlags: []string{" (positional)", "--type", "--source (local|remote)", "--provider (when --source remote)"}, Examples: []string{ `weknora model create nomic-embed-text --type Embedding --source local --dimension 768 # Ollama (server pulls it)`, diff --git a/cli/cmd/model/list.go b/cli/cmd/model/list.go index ecdcdfcf9..acd95cf92 100644 --- a/cli/cmd/model/list.go +++ b/cli/cmd/model/list.go @@ -40,6 +40,10 @@ type ListOptions struct { // …), matched case-insensitively. Empty shows everything. Type string Source string + // Limit caps the returned slice client-side (applied after --type/--source + // filtering and sort). The ListModels SDK is unpaginated, so the CLI holds + // the true total and reports meta.total_count/has_more when --limit drops any. + Limit int } // ListService is the narrow SDK surface this command depends on. @@ -62,6 +66,11 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { return err } fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) + // Validate static input before building the client so a bad --limit + // returns input.invalid_argument (exit 5), not an auth error (exit 3). + if err := validateListOpts(opts); err != nil { + return err + } cli, err := f.Client() if err != nil { return err @@ -71,6 +80,7 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.Type, "type", "", "Only show models of this type (Embedding, Rerank, KnowledgeQA, VLLM, ASR)") cmd.Flags().StringVar(&opts.Source, "source", "", "Only show models from this provider (local, remote, openai, aliyun, …)") + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum results to return — client-side cap; meta.has_more/total_count report the full size (1..10000)") cmdutil.AddFormatFlag(cmd, modelListFields...) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "discover model ids for `agent create --model` and a KB's embedding/summary model", @@ -79,12 +89,28 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { "weknora model list --type KnowledgeQA --format json", "weknora model list --source local", }, - Output: "envelope.data is an array of Model objects (id, name, display_name, type, source, is_default); narrow it with --type / --source", + Output: "envelope.data is an array of Model objects (id, name, display_name, type, source, is_default); narrow it with --type / --source; meta.count is the returned count, meta.total_count is the full set and meta.has_more=true means --limit truncated it", }) return cmd } +// validateListOpts checks --limit. Called from RunE before the client is built +// (so a bad value surfaces as exit 5, not an auth error) and at runList's top +// for direct callers; idempotent. +func validateListOpts(opts *ListOptions) error { + if opts.Limit < 1 || opts.Limit > 10000 { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: fmt.Sprintf("--limit must be in 1..10000, got %d", opts.Limit), + } + } + return nil +} + func runList(ctx context.Context, opts *ListOptions, fopts *cmdutil.FormatOptions, svc ListService) error { + if err := validateListOpts(opts); err != nil { + return err + } if _, err := cmdutil.ValidateEnum("type", opts.Type, modelTypeValues); err != nil { return err } @@ -121,8 +147,19 @@ func runList(ctx context.Context, opts *ListOptions, fopts *cmdutil.FormatOption return modelLabel(items[i]) < modelLabel(items[j]) }) + // Client-side --limit cap. The ListModels SDK is unpaginated, so the CLI + // holds the true total and can tell the caller whether --limit dropped any: + // total_count is the full (post-filter) count, has_more flags truncation. + total := len(items) + truncated := false + if opts.Limit > 0 && len(items) > opts.Limit { + items = items[:opts.Limit] + truncated = true + } + if fopts.WantsJSON() { - return fopts.Emit(iostreams.IO.Out, items, &output.Meta{Count: output.IntPtr(len(items))}) + meta := &output.Meta{Count: output.IntPtr(len(items)), HasMore: truncated, TotalCount: output.IntPtr(total)} + return fopts.Emit(iostreams.IO.Out, items, meta) } if len(items) == 0 { diff --git a/cli/cmd/model/list_test.go b/cli/cmd/model/list_test.go index 1dd857268..53f3b06dd 100644 --- a/cli/cmd/model/list_test.go +++ b/cli/cmd/model/list_test.go @@ -27,7 +27,7 @@ func TestModelList_Text(t *testing.T) { {ID: "m1", DisplayName: "GPT-X", Type: sdk.ModelTypeKnowledgeQA, Source: sdk.ModelSourceOpenAI, IsDefault: true}, {ID: "m2", Name: "bge", Type: sdk.ModelTypeEmbedding, Source: sdk.ModelSourceLocal}, }} - if err := runList(context.Background(), &ListOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc); err != nil { + if err := runList(context.Background(), &ListOptions{Limit: 30}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc); err != nil { t.Fatalf("runList: %v", err) } got := out.String() @@ -46,7 +46,7 @@ func TestModelList_TypeFilter(t *testing.T) { {ID: "m1", Type: sdk.ModelTypeKnowledgeQA}, {ID: "m2", Type: sdk.ModelTypeEmbedding}, }} - if err := runList(context.Background(), &ListOptions{Type: "embedding"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc); err != nil { + if err := runList(context.Background(), &ListOptions{Type: "embedding", Limit: 30}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc); err != nil { t.Fatalf("runList: %v", err) } var env struct { @@ -69,7 +69,7 @@ func TestModelList_TypeFilter(t *testing.T) { func TestModelList_Empty(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeListSvc{models: nil} - if err := runList(context.Background(), &ListOptions{}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc); err != nil { + if err := runList(context.Background(), &ListOptions{Limit: 30}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc); err != nil { t.Fatalf("runList: %v", err) } if !strings.Contains(out.String(), "(no models)") { @@ -80,7 +80,7 @@ func TestModelList_Empty(t *testing.T) { func TestModelList_EmptyAfterFilter(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeListSvc{models: []sdk.Model{{ID: "m1", Type: sdk.ModelTypeEmbedding}}} - if err := runList(context.Background(), &ListOptions{Type: "Rerank"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc); err != nil { + if err := runList(context.Background(), &ListOptions{Type: "Rerank", Limit: 30}, &cmdutil.FormatOptions{Mode: cmdutil.FormatText}, svc); err != nil { t.Fatalf("runList: %v", err) } if !strings.Contains(out.String(), "(no models match the filter)") { @@ -95,7 +95,7 @@ func TestModelList_SourceFilter(t *testing.T) { {ID: "m1", Type: sdk.ModelTypeEmbedding, Source: sdk.ModelSourceLocal}, {ID: "m2", Type: sdk.ModelTypeKnowledgeQA, Source: sdk.ModelSourceOpenAI}, }} - if err := runList(context.Background(), &ListOptions{Source: "OpenAI"}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc); err != nil { + if err := runList(context.Background(), &ListOptions{Source: "OpenAI", Limit: 30}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc); err != nil { t.Fatalf("runList: %v", err) } var env struct { @@ -113,8 +113,8 @@ func TestModelList_SourceFilter(t *testing.T) { // (input.invalid_argument) instead of silently returning an empty set. func TestModelList_InvalidEnum(t *testing.T) { for _, tc := range []struct{ name string; opts ListOptions }{ - {"type", ListOptions{Type: "bogus"}}, - {"source", ListOptions{Source: "bogus"}}, + {"type", ListOptions{Type: "bogus", Limit: 30}}, + {"source", ListOptions{Source: "bogus", Limit: 30}}, } { t.Run(tc.name, func(t *testing.T) { _, _ = iostreams.SetForTest(t) @@ -127,3 +127,44 @@ func TestModelList_InvalidEnum(t *testing.T) { }) } } + +// TestModelList_Limit_TruncatesAndSignals verifies --limit caps model list +// output and reports meta.total_count/has_more like every other list command. +// Regression: model list had no --limit, so an agent could not cap output or +// tell whether it saw the full set. +func TestModelList_Limit_TruncatesAndSignals(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeListSvc{models: []sdk.Model{ + {ID: "m1", Type: sdk.ModelTypeKnowledgeQA}, + {ID: "m2", Type: sdk.ModelTypeKnowledgeQA}, + {ID: "m3", Type: sdk.ModelTypeKnowledgeQA}, + }} + fopts := &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON} + if err := runList(context.Background(), &ListOptions{Limit: 2}, fopts, svc); err != nil { + t.Fatalf("runList: %v", err) + } + got := out.String() + if n := strings.Count(got, `"id":"m`); n != 2 { + t.Errorf("--limit 2 should slice to 2 models, got %d in:\n%s", n, got) + } + if !strings.Contains(got, `"has_more":true`) { + t.Errorf("truncated model list must set has_more:true; got:\n%s", got) + } + if !strings.Contains(got, `"total_count":3`) { + t.Errorf("truncated model list must report total_count:3; got:\n%s", got) + } +} + +// TestModelList_BadLimit_Rejected verifies an out-of-range --limit is exit-5 +// typed validation, consistent with kb/session list. +func TestModelList_BadLimit_Rejected(t *testing.T) { + _, _ = iostreams.SetForTest(t) + err := runList(context.Background(), &ListOptions{Limit: 99999}, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, &fakeListSvc{}) + if err == nil { + t.Fatal("--limit 99999 must be rejected") + } + var e *cmdutil.Error + if !errors.As(err, &e) || e.Code != cmdutil.CodeInputInvalidArgument { + t.Errorf("want input.invalid_argument, got %v", err) + } +} diff --git a/cli/cmd/model/model.go b/cli/cmd/model/model.go index f1ce15f14..43122f70f 100644 --- a/cli/cmd/model/model.go +++ b/cli/cmd/model/model.go @@ -21,7 +21,7 @@ import ( func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "model", - Short: "Manage models (list / view / create / delete)", + Short: "Manage models (list / view / create / update / delete)", Long: `List, inspect, register, and delete the models configured on the server. Use the model id to back a knowledge base's embedding / summary config ('weknora kb init') or an agent ('weknora agent create --model ').`, @@ -29,6 +29,7 @@ init') or an agent ('weknora agent create --model ').`, cmd.AddCommand(NewCmdList(f)) cmd.AddCommand(NewCmdView(f)) cmd.AddCommand(NewCmdCreate(f)) + cmd.AddCommand(NewCmdUpdate(f)) cmd.AddCommand(NewCmdDelete(f)) return cmd } diff --git a/cli/cmd/model/update.go b/cli/cmd/model/update.go new file mode 100644 index 000000000..a476bf590 --- /dev/null +++ b/cli/cmd/model/update.go @@ -0,0 +1,189 @@ +package modelcmd + +import ( + "context" + "fmt" + "io" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// UpdateOptions captures the surgical flag state for `model update`. Per-flag +// *Set bits distinguish "" (clear) from unset, matching agent/doc update. +type UpdateOptions struct { + DisplayName string + Description string + BaseURL string + APIKeyStdin bool + Params []string + Default bool + DryRun bool + StdinReader io.Reader + flags modelUpdateFlags +} + +type modelUpdateFlags struct{ displayName, description, baseURL, def bool } + +// UpdateService is the narrow SDK surface. UpdateModel is a full PUT, so the +// fetch (GetModel) is mandatory — without the baseline, any field not touched +// by a flag would clobber to its zero value. +type UpdateService interface { + GetModel(ctx context.Context, id string) (*sdk.Model, error) + UpdateModel(ctx context.Context, id string, req *sdk.UpdateModelRequest) (*sdk.Model, error) +} + +// NewCmdUpdate builds `weknora model update ` — update a registered +// model in place (id preserved), so KBs / agents referencing it keep working. +func NewCmdUpdate(f *cmdutil.Factory) *cobra.Command { + opts := &UpdateOptions{} + cmd := &cobra.Command{ + Use: "update ", + Short: "Update a model in place (rotate key, base URL, display name, default)", + Long: `Update a registered model WITHOUT changing its id, so KBs / agents that +reference it keep working (unlike delete + re-create, which orphans references). +Rotate the provider key with --api-key-stdin, or change --base-url, +--display-name, --description, extra --param entries, or --default. A model's +type and source are immutable — register a new model to change them. + +Reversible write: without -y/--yes in a non-TTY / JSON context it exits 10 +(input.confirmation_required) without applying the change.`, + 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()) + id := args[0] + opts.flags.displayName = c.Flags().Changed("display-name") + opts.flags.description = c.Flags().Changed("description") + opts.flags.baseURL = c.Flags().Changed("base-url") + opts.flags.def = c.Flags().Changed("default") + if !modelUpdateHasFlag(opts) { + return &cmdutil.Error{ + Code: cmdutil.CodeInputInvalidArgument, + Message: "model update requires at least one flag", + Hint: "pass e.g. --display-name, --base-url, --api-key-stdin, --param, or --default", + } + } + params, err := parseParams(opts.Params) + if err != nil { + return err + } + if handled, err := cmdutil.HandleDryRun(c, opts.DryRun, cmdutil.DryRunPlan{ + Action: "model.update", + Args: map[string]any{"model": id, "display_name": opts.DisplayName, "base_url": opts.BaseURL, "default": opts.Default, "rotate_api_key": opts.APIKeyStdin, "param_count": len(params)}, + }); handled { + return err + } + yes, _ := c.Flags().GetBool("yes") + // --api-key-stdin / --param excluded from retry_argv (stdin secret / + // repeatable), matching agent update's multi-value exclusions. + retry := cmdutil.BuildRetryArgv(c, []string{"weknora", "model", "update", id}, + "display-name", "description", "base-url", "default", "format") + if err := cmdutil.ConfirmWrite(f.Prompter(), yes, fopts.WantsJSON(), "update", "model", id, "model.update", retry); err != nil { + return err + } + if opts.StdinReader == nil { + opts.StdinReader = iostreams.IO.In + } + cli, err := f.Client() + if err != nil { + return err + } + return runUpdate(c.Context(), opts, fopts, cli, id, params) + }, + } + cmd.Flags().StringVar(&opts.DisplayName, "display-name", "", "New human-friendly name") + cmd.Flags().StringVar(&opts.Description, "description", "", "New description") + cmd.Flags().StringVar(&opts.BaseURL, "base-url", "", "New model API base URL") + cmd.Flags().BoolVar(&opts.APIKeyStdin, "api-key-stdin", false, "Rotate the provider API key, read from stdin (kept out of argv / history)") + cmd.Flags().StringArrayVar(&opts.Params, "param", nil, "Set an extra provider parameter as key=value, repeatable (value parsed as JSON)") + cmd.Flags().BoolVar(&opts.Default, "default", false, "Mark this the default model for its type (--default=false to unset)") + cmdutil.AddFormatFlag(cmd, modelListFields...) + cmdutil.AddDryRunFlag(cmd, &opts.DryRun) + cmdutil.SetWriteRisk(cmd, "model.update") + cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ + UsedFor: "update a registered model IN PLACE (id preserved, so KB/agent references keep working): rotate --api-key-stdin, change --base-url / --display-name / --description / --param, or set --default. Type and source are immutable.", + RequiredFlags: []string{" (positional)", "at least one update flag"}, + Examples: []string{ + `printf '%s' "$NEW_KEY" | weknora model update mdl_abc --api-key-stdin -y`, + `weknora model update mdl_abc --base-url https://api.example.com/v1 -y`, + `weknora model update mdl_abc --default -y`, + }, + Output: "envelope.data is the updated Model object (id preserved; provider api key never echoed)", + Warnings: []string{ + "Reversible write: requires explicit approval (exit 10 / input.confirmation_required) unless -y; never auto-add -y.", + "Server-side this is an admin operation; a non-admin credential gets auth.forbidden (exit 3).", + }, + }) + return cmd +} + +func modelUpdateHasFlag(o *UpdateOptions) bool { + return o.flags.displayName || o.flags.description || o.flags.baseURL || o.flags.def || + o.APIKeyStdin || len(o.Params) > 0 +} + +func runUpdate(ctx context.Context, opts *UpdateOptions, fopts *cmdutil.FormatOptions, svc UpdateService, id string, params map[string]any) error { + // Fetch-then-update: UpdateModel is a full PUT, so start from the server's + // current state and overlay only what the user changed. + cur, err := svc.GetModel(ctx, id) + if err != nil { + return cmdutil.WrapHTTP(err, "fetch model %s", id) + } + merged := sdk.ModelParameters{} + for k, v := range cur.Parameters { + merged[k] = v + } + for k, v := range params { + merged[k] = v + } + if opts.flags.baseURL { + merged["base_url"] = opts.BaseURL + } + if opts.APIKeyStdin { + key, err := readStdinTrimmed(opts.StdinReader) + if err != nil { + return cmdutil.Wrapf(cmdutil.CodeLocalFileIO, err, "read API key from stdin") + } + if key == "" { + return cmdutil.NewError(cmdutil.CodeInputMissingFlag, "--api-key-stdin requires the key piped to stdin") + } + merged["api_key"] = key + } + + req := &sdk.UpdateModelRequest{ + Name: cur.Name, + DisplayName: cur.DisplayName, + Description: cur.Description, + Parameters: merged, + IsDefault: cur.IsDefault, + } + if opts.flags.displayName { + req.DisplayName = opts.DisplayName + } + if opts.flags.description { + req.Description = opts.Description + } + if opts.flags.def { + req.IsDefault = opts.Default + } + + updated, err := svc.UpdateModel(ctx, id, req) + if err != nil { + return cmdutil.WrapHTTP(err, "update model %s", id) + } + if fopts.WantsJSON() { + return fopts.Emit(iostreams.IO.Out, updated, nil) + } + fmt.Fprintf(iostreams.IO.Out, "✓ Updated model %q (id: %s)\n", updated.Name, updated.ID) + return nil +} + +// compile-time check: the production SDK client implements UpdateService. +var _ UpdateService = (*sdk.Client)(nil) diff --git a/cli/cmd/model/update_test.go b/cli/cmd/model/update_test.go new file mode 100644 index 000000000..dd1a1ba18 --- /dev/null +++ b/cli/cmd/model/update_test.go @@ -0,0 +1,81 @@ +package modelcmd + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + sdk "github.com/Tencent/WeKnora/client" +) + +// fakeUpdateSvc scripts GetModel (fetch baseline) and captures the +// UpdateModelRequest so tests can assert the surgical overlay. +type fakeUpdateSvc struct { + cur *sdk.Model + gotReq *sdk.UpdateModelRequest + gotID string +} + +func (f *fakeUpdateSvc) GetModel(_ context.Context, _ string) (*sdk.Model, error) { + return f.cur, nil +} + +func (f *fakeUpdateSvc) UpdateModel(_ context.Context, id string, req *sdk.UpdateModelRequest) (*sdk.Model, error) { + f.gotID = id + f.gotReq = req + return &sdk.Model{ID: id, Name: req.Name, DisplayName: req.DisplayName, Parameters: req.Parameters, IsDefault: req.IsDefault}, nil +} + +func baseModel() *sdk.Model { + return &sdk.Model{ + ID: "mdl_x", Name: "keep-name", DisplayName: "old", Description: "olddesc", + Parameters: sdk.ModelParameters{"base_url": "http://old", "api_key": "SECRET-OLD", "provider": "generic"}, + IsDefault: false, + } +} + +// TestModelUpdate_SurgicalOverlay: only touched fields change; the rest (name, +// description, existing params) round-trip from the fetched baseline. +func TestModelUpdate_SurgicalOverlay(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUpdateSvc{cur: baseModel()} + opts := &UpdateOptions{DisplayName: "new-display", flags: modelUpdateFlags{displayName: true}} + require.NoError(t, runUpdate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "mdl_x", nil)) + + require.NotNil(t, svc.gotReq) + assert.Equal(t, "mdl_x", svc.gotID, "id preserved (in-place update)") + assert.Equal(t, "new-display", svc.gotReq.DisplayName, "display-name overlaid") + assert.Equal(t, "keep-name", svc.gotReq.Name, "untouched name round-trips") + assert.Equal(t, "olddesc", svc.gotReq.Description, "untouched description round-trips") + assert.Equal(t, "http://old", svc.gotReq.Parameters["base_url"], "untouched params round-trip") + assert.Equal(t, "SECRET-OLD", svc.gotReq.Parameters["api_key"], "existing key preserved when not rotating") +} + +// TestModelUpdate_RotateKeyAndBaseURL: --api-key-stdin + --base-url overlay the +// parameters map; the new key comes from stdin, never argv. +func TestModelUpdate_RotateKeyAndBaseURL(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &fakeUpdateSvc{cur: baseModel()} + opts := &UpdateOptions{ + BaseURL: "http://new", APIKeyStdin: true, + StdinReader: strings.NewReader("NEW-KEY\n"), + flags: modelUpdateFlags{baseURL: true}, + } + require.NoError(t, runUpdate(context.Background(), opts, &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc, "mdl_x", nil)) + assert.Equal(t, "http://new", svc.gotReq.Parameters["base_url"]) + assert.Equal(t, "NEW-KEY", svc.gotReq.Parameters["api_key"], "rotated key read from stdin") +} + +// TestModelUpdate_RequiresAtLeastOneFlag: a bare `model update ` is rejected +// before any network call, matching agent update. +func TestModelUpdate_RequiresAtLeastOneFlag(t *testing.T) { + assert.False(t, modelUpdateHasFlag(&UpdateOptions{})) + assert.True(t, modelUpdateHasFlag(&UpdateOptions{flags: modelUpdateFlags{displayName: true}})) + assert.True(t, modelUpdateHasFlag(&UpdateOptions{APIKeyStdin: true})) + assert.True(t, modelUpdateHasFlag(&UpdateOptions{Params: []string{"k=v"}})) +} diff --git a/cli/cmd/profile/add.go b/cli/cmd/profile/add.go index b29efe825..453393376 100644 --- a/cli/cmd/profile/add.go +++ b/cli/cmd/profile/add.go @@ -103,6 +103,9 @@ adds leave the current profile untouched unless --use is passed.`, UsedFor: "Register a new profile (connection target) with a name and host URL. Does not store credentials; make it active (--use, or `profile use `) and run `auth login` afterwards to authenticate.", RequiredFlags: []string{" (positional)", "--host"}, Output: "envelope.data has name, host, user, current", + Examples: []string{ + "weknora profile add prod --host https://kb.example.com --use", + }, }) return cmd } diff --git a/cli/cmd/profile/remove.go b/cli/cmd/profile/remove.go index 5e2b36030..165dd59e2 100644 --- a/cli/cmd/profile/remove.go +++ b/cli/cmd/profile/remove.go @@ -90,6 +90,7 @@ in scripted / --format json invocations (exit code 10; see cli/README.md).`, cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "remove a named profile and its stored credentials", RequiredFlags: []string{" (positional)"}, + Output: "envelope.data is {name, removed:true, was_current}", Examples: []string{ "weknora profile remove staging", "weknora profile remove production -y", diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 98a140246..b65b25793 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -238,7 +238,7 @@ func addGlobalFlags(cmd *cobra.Command) { // instead of being rejected as "unknown flag" exit 2 by cobra. Commands // that don't produce JSON output (e.g. `completion bash`) ignore the flag // rather than error — the unified agent contract is worth the trade. - pf.String("format", "", "Output format: text | json | ndjson (default: json)") + pf.String("format", "", "Output format: text | json | ndjson (default: json; env: WEKNORA_FORMAT)") pf.StringP("jq", "q", "", "Filter JSON output using a jq `expression` (requires --format json|ndjson)") } diff --git a/cli/cmd/schema.go b/cli/cmd/schema.go index 76970dfdc..604fc1311 100644 --- a/cli/cmd/schema.go +++ b/cli/cmd/schema.go @@ -105,6 +105,16 @@ human help prose.`, // Returns a typed input.unknown_subcommand error (with did-you-mean) when the // path does not resolve to a real command. func resolveSchemaTarget(root *cobra.Command, args []string) (*cobra.Command, error) { + // Tolerate the quoted multi-word form the no-arg `schema` index prints as a + // command label (e.g. `schema "agent create"`): re-split each arg on + // whitespace so an agent can paste a label verbatim and resolve it the same + // as `schema agent create`. + flat := make([]string, 0, len(args)) + for _, a := range args { + flat = append(flat, strings.Fields(a)...) + } + args = flat + target, rest, err := root.Find(args) // Find returns root (with the args unconsumed) when nothing matched; a // fully-resolved leaf returns itself with its positional args as rest. diff --git a/cli/cmd/schema_test.go b/cli/cmd/schema_test.go index bf0abdbf6..f7b21f516 100644 --- a/cli/cmd/schema_test.go +++ b/cli/cmd/schema_test.go @@ -68,6 +68,25 @@ func TestSchema_SingleCommand(t *testing.T) { assert.False(t, flagNames["profile"], "inherited global flags must be excluded") } +// TestSchema_QuotedMultiWordArg: the no-arg `schema` index prints command +// labels like "agent create"; an agent that pastes that label back as a single +// quoted arg (`schema "agent create"`) must resolve the same as two tokens, +// not fail with unknown_subcommand. +func TestSchema_QuotedMultiWordArg(t *testing.T) { + out, _ := iostreams.SetForTest(t) + root := NewRootCmd(cmdutil.New()) + root.SetArgs([]string{"schema", "agent create", "--format", "json"}) + require.NoError(t, root.Execute(), "got %q", out.String()) + + var env struct { + OK bool `json:"ok"` + Data commandSchema `json:"data"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env), "got %q", out.String()) + assert.True(t, env.OK) + assert.Equal(t, "agent create", env.Data.Command) +} + // TestSchema_SurfacesRisk: a destructive command exposes its risk annotation, // so an agent can discover confirmation-gating without running the command. func TestSchema_SurfacesRisk(t *testing.T) { diff --git a/cli/cmd/search/chunks.go b/cli/cmd/search/chunks.go index 806e49550..c862ab07d 100644 --- a/cli/cmd/search/chunks.go +++ b/cli/cmd/search/chunks.go @@ -158,7 +158,7 @@ func runChunks(ctx context.Context, opts *ChunksOptions, fopts *cmdutil.FormatOp if results == nil { results = []*sdk.SearchResult{} } - meta := &output.Meta{Count: output.IntPtr(len(results)), HasMore: truncated} + meta := &output.Meta{Count: output.IntPtr(len(results)), HasMore: truncated, Hint: emptyContentSearchHint(len(results))} return fopts.Emit(iostreams.IO.Out, results, meta) } return renderChunkResults(results, opts.KBID) diff --git a/cli/cmd/search/docs.go b/cli/cmd/search/docs.go index 65ce09fac..ba4606b1e 100644 --- a/cli/cmd/search/docs.go +++ b/cli/cmd/search/docs.go @@ -61,7 +61,7 @@ type DocsSearchService interface { // NewCmdDocs builds `weknora search docs "" --kb `. // Pages through the KB's documents and surfaces every entry whose title -// or file_name contains the query as a server-side case-sensitive LIKE +// or file_name contains the query as a server-side case-insensitive LIKE // match. Useful for finding a specific upload to download or delete. func NewCmdDocs(f *cmdutil.Factory) *cobra.Command { opts := &DocsSearchOptions{} @@ -72,11 +72,9 @@ func NewCmdDocs(f *cmdutil.Factory) *cobra.Command { keyword filter (matched against title / file_name). Useful for finding a specific upload to download or delete by id. -The query is a case-sensitive server-side LIKE filter (the server runs -` + "`LIKE %keyword%`" + ` against title and file_name). For case-insensitive -matching, lower-case the query yourself, e.g. -` + "`weknora search docs \"$(printf %s YOUR_QUERY | tr 'A-Z' 'a-z')\"`" + `, or -fall back to ` + "`weknora api`" + ` with a custom filter. +The query is a case-insensitive server-side LIKE filter (the server runs +` + "`LOWER(...) LIKE LOWER('%keyword%')`" + ` against title and file_name), so +` + "`FALCON`" + ` and ` + "`falcon`" + ` match the same documents. By default, --all-pages=true walks every server page until --limit is reached or the KB is exhausted. Pass --all-pages=false to stop after one page.`, @@ -123,7 +121,7 @@ reached or the KB is exhausted. Pass --all-pages=false to stop after one page.`, UsedFor: "Find documents in a knowledge base by keyword (server-side LIKE filter on title/file_name). The KB comes from --kb (id or name), else WEKNORA_KB_ID, else the linked directory. Results come with meta.count; use --limit to cap and --all-pages=false to stop after one page.", RequiredFlags: []string{" (positional)", "--kb (or WEKNORA_KB_ID / linked directory)"}, Examples: []string{`weknora search docs "spec" --kb engineering --format json`}, - Output: "envelope.data is an array of Knowledge objects with id, title, file_name, parse_status; meta.count is the returned count; meta.has_more=true if more matched than --limit", + Output: "envelope.data is an array of Knowledge objects with id, title, file_name, parse_status; meta.count is the returned count, meta.total_count the server's full match count, meta.has_more=true if more matched than --limit", }) return cmd } @@ -142,11 +140,13 @@ func runDocsSearch(ctx context.Context, opts *DocsSearchOptions, fopts *cmdutil. // --all-pages=true (default) walks every server page; --all-pages=false // stops after the first page. Termination counts records actually // received so server-capped page_size doesn't truncate. + var serverTotal int64 for page := 1; ; page++ { items, total, err := svc.ListKnowledgeWithFilter(ctx, opts.KBID, page, opts.PageSize, filter) if err != nil { return cmdutil.WrapHTTP(err, "list documents") } + serverTotal = total for _, k := range items { matches = append(matches, k) // Collect one past --limit so has_more is accurate; trimmed below. @@ -172,7 +172,7 @@ done: if matches == nil { matches = []sdk.Knowledge{} } - meta := &output.Meta{Count: output.IntPtr(len(matches)), HasMore: truncated} + meta := &output.Meta{Count: output.IntPtr(len(matches)), TotalCount: output.IntPtr(int(serverTotal)), HasMore: truncated, Hint: emptyContentSearchHint(len(matches))} return fopts.Emit(iostreams.IO.Out, matches, meta) } if len(matches) == 0 { diff --git a/cli/cmd/search/docs_test.go b/cli/cmd/search/docs_test.go index e0986629b..9b9ad4131 100644 --- a/cli/cmd/search/docs_test.go +++ b/cli/cmd/search/docs_test.go @@ -115,6 +115,29 @@ func TestDocsSearch_JSON(t *testing.T) { assert.Contains(t, got, `"id":"d1"`) } +// TestDocsSearch_JSON_EmitsTotalCount pins that search docs surfaces the +// server's full match total as meta.total_count (server-side keyword filter, so +// total is the real match count) — parity with doc/session/chunk list. +func TestDocsSearch_JSON_EmitsTotalCount(t *testing.T) { + out, _ := iostreams.SetForTest(t) + svc := &fakeDocsSearchSvc{ + pages: map[int][]sdk.Knowledge{1: {{ID: "d1", Title: "match"}, {ID: "d2", Title: "match2"}}}, + total: 9, // server reports 9 total matches; we display the first page + } + require.NoError(t, runDocsSearch(context.Background(), + &DocsSearchOptions{Query: "match", KBID: "kb1", Limit: 20, PageSize: docsPageSize, AllPages: false}, + &cmdutil.FormatOptions{Mode: cmdutil.FormatJSON}, svc)) + var env struct { + Meta struct { + Count *int `json:"count"` + TotalCount *int `json:"total_count"` + } `json:"meta"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &env)) + require.NotNil(t, env.Meta.TotalCount, "search docs must emit meta.total_count") + assert.Equal(t, 9, *env.Meta.TotalCount) +} + func TestDocsSearch_NetworkError(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDocsSearchSvc{err: errors.New("HTTP error 404: kb not found")} diff --git a/cli/cmd/search/search.go b/cli/cmd/search/search.go index 485a868c6..89b717d2b 100644 --- a/cli/cmd/search/search.go +++ b/cli/cmd/search/search.go @@ -32,3 +32,16 @@ func NewCmdSearch(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(NewCmdSessions(f)) return cmd } + +// emptyContentSearchHint returns an actionable note when a KB-scoped content +// search (chunks / docs) yields zero results, so an agent can distinguish +// "no match" from "the KB has no indexed content". Empty when n > 0 so it +// never adds noise to real results. +func emptyContentSearchHint(n int) string { + if n > 0 { + return "" + } + return "0 results: this may be no match, OR the KB has no indexed chunks. " + + "Check `weknora kb status ` (chunk_count) and `weknora doc list --kb ` (parse_status); " + + "documents in parse_status=draft are not indexed — run `weknora doc reparse `." +} diff --git a/cli/cmd/session/ask.go b/cli/cmd/session/ask.go index a7c0d9fa5..726cbf2ea 100644 --- a/cli/cmd/session/ask.go +++ b/cli/cmd/session/ask.go @@ -61,7 +61,7 @@ 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). +update / delete / status / check). Modes: --format json (default): one JSON envelope with answer events diff --git a/cli/cmd/session/ask_test.go b/cli/cmd/session/ask_test.go index 15735b811..58ad2258a 100644 --- a/cli/cmd/session/ask_test.go +++ b/cli/cmd/session/ask_test.go @@ -47,6 +47,9 @@ func (s *scriptedAskSvc) AgentQAStreamWithRequest(_ context.Context, sessionID s func answerEvent(content string) *sdk.AgentStreamResponse { return &sdk.AgentStreamResponse{ResponseType: sdk.AgentResponseTypeAnswer, Content: content} } +// doneEvent is the stream's terminal frame. The real server ends an agent +// stream with a `complete` event (it also sets Done=true on intermediate +// frames), so the terminal is modeled as complete, not a bare answer+done. func doneEvent() *sdk.AgentStreamResponse { return &sdk.AgentStreamResponse{ResponseType: sdk.AgentResponseTypeComplete, Done: true} } diff --git a/cli/cmd/session/list.go b/cli/cmd/session/list.go index 2e9700798..329deacd3 100644 --- a/cli/cmd/session/list.go +++ b/cli/cmd/session/list.go @@ -78,7 +78,7 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "List chat sessions for the active profile. Results come with meta.count; use --limit to cap, --all-pages to walk every server page, --since to filter by recency (e.g. 7d).", Examples: []string{"weknora session list --format json", "weknora session list --all-pages --since 7d --format json"}, - Output: "envelope.data is an array of Session objects with id, title, updated_at; meta.count is the total returned; meta.total_count is the server-side total before --since filtering", + Output: "envelope.data is an array of Session objects with id, title, updated_at; meta.count is the returned count; meta.total_count is the server-side total before --since filtering; meta.has_more=true when --limit truncated", }) return cmd } diff --git a/cli/cmd/session/continue_stream.go b/cli/cmd/session/resume.go similarity index 67% rename from cli/cmd/session/continue_stream.go rename to cli/cmd/session/resume.go index 5309e3c18..cb5362b02 100644 --- a/cli/cmd/session/continue_stream.go +++ b/cli/cmd/session/resume.go @@ -1,4 +1,4 @@ -// continue_stream.go implements `weknora session continue-stream` — +// resume.go implements `weknora session resume` — // re-attach to an SSE event buffer for an in-progress or already-completed // assistant message under a known session_id. // @@ -23,44 +23,42 @@ package sessioncmd import ( "context" - "errors" "github.com/spf13/cobra" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "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" ) -// continueStreamFields enumerates the NDJSON init-event + raw SDK event +// resumeFields enumerates the NDJSON init-event + raw SDK event // vocabulary surfaced for `--format json` / `--format ndjson` discovery. -var continueStreamFields = []string{ +var resumeFields = []string{ "session_id", "message_id", // SDK StreamResponse fields (pass-through): id, response_type, content, // done, knowledge_references, assistant_message_id, session_id, // tool_calls, data } -// ContinueStreamOptions captures `session continue-stream` flag/arg state. -type ContinueStreamOptions struct { +// ResumeOptions captures `session resume` flag/arg state. +type ResumeOptions struct { SessionID string MessageID string } -// ContinueStreamService is the narrow SDK surface this command depends on. +// ResumeService is the narrow SDK surface this command depends on. // *sdk.Client satisfies it; tests substitute a fake. Compile-time check // at the bottom of this file. -type ContinueStreamService interface { +type ResumeService interface { ContinueStream(ctx context.Context, sessionID, messageID string, cb func(*sdk.StreamResponse) error) error } -// NewCmdContinueStream builds `weknora session continue-stream --message `. -func NewCmdContinueStream(f *cmdutil.Factory) *cobra.Command { - opts := &ContinueStreamOptions{} +// NewCmdResume builds `weknora session resume --message `. +func NewCmdResume(f *cmdutil.Factory) *cobra.Command { + opts := &ResumeOptions{} cmd := &cobra.Command{ - Use: "continue-stream ", + Use: "resume ", Short: "Resume an SSE event stream for an in-progress or completed session message", Long: `Re-attach to the SSE event buffer for an assistant message under a known session. @@ -92,8 +90,8 @@ regardless of --format value. The operator use case (incident response, debugging) always wants the raw event log; there is no human-text rendering. --format json and --format ndjson behave identically here; --format text is silently treated as NDJSON.`, - Example: ` weknora session continue-stream sess_xyz --message msg_abc - weknora session continue-stream sess_xyz -m msg_abc --format ndjson`, + Example: ` weknora session resume sess_xyz --message msg_abc + weknora session resume sess_xyz -m msg_abc --format ndjson`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.SessionID = args[0] @@ -106,38 +104,39 @@ silently treated as NDJSON.`, if err != nil { return err } - return runContinueStream(c.Context(), opts, fopts, cli) + return runResume(c.Context(), opts, fopts, cli) }, } cmd.Flags().StringVarP(&opts.MessageID, "message", "m", "", "Assistant message ID to resume (from the init or agent_query event of the original stream)") _ = cmd.MarkFlagRequired("message") - cmdutil.AddFormatFlag(cmd, continueStreamFields...) + cmdutil.AddFormatFlag(cmd, resumeFields...) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ UsedFor: "Resume an SSE event stream for an in-progress or completed assistant message. Produces an NDJSON event stream: init line (session_id, message_id) then raw SDK StreamResponse events.", - RequiredFlags: []string{" (positional)", "--message (message_id from prior init / agent_query event)"}, + RequiredFlags: []string{" (positional)", "--message (persisted assistant message id — get it from `weknora message list --session `; a live stream's assistant_message_id is not resumable once the message persists)"}, Examples: []string{ - "weknora session continue-stream sess_xyz --message msg_abc --format json", - "# Network-blip recovery: replay with same session_id + message_id from the original 'session ask' init event", + "weknora session resume sess_xyz --message msg_abc --format json", + "# Get the message id from: weknora message list --session (the persisted assistant message)", }, Output: "NDJSON stream: {type:init, session_id, message_id, profile} then SDK StreamResponse events (response_type, content, done, knowledge_references, assistant_message_id, ...)", Warnings: []string{ "Server replays from event 0 (NOT cursor-from-disconnect). Agents that already consumed events on the original stream MUST dedupe by message_id + event hash to avoid double-processing.", "Buffer TTL: redis mode 1h hardcoded; memory mode = process lifetime. After expiry the CLI returns local.sse_stream_aborted.", + "Output is always NDJSON (an event stream, not an envelope): --jq does not apply and --format text/json/ndjson behave identically here — parse the event lines yourself.", }, }) return cmd } -// runContinueStream is the testable core: validate, dispatch the resume, and +// runResume is the testable core: validate, dispatch the resume, and // route the NDJSON stream. Returns a typed error. // // Always emits NDJSON: a buffered envelope makes no sense for a streaming -// command, and continue-stream has no human-text use case (operators reach +// command, and resume has no human-text use case (operators reach // for it during incident response / debugging, which always wants the raw // event log). --format text is therefore treated identically to --format // json/ndjson here. -func runContinueStream(ctx context.Context, opts *ContinueStreamOptions, _ *cmdutil.FormatOptions, svc ContinueStreamService) error { +func runResume(ctx context.Context, opts *ResumeOptions, _ *cmdutil.FormatOptions, svc ResumeService) error { if opts.SessionID == "" { return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "session-id argument cannot be empty") } @@ -145,7 +144,7 @@ func runContinueStream(ctx context.Context, opts *ContinueStreamOptions, _ *cmdu return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "--message cannot be empty") } if svc == nil { - return cmdutil.NewError(cmdutil.CodeServerError, "session continue-stream: no SDK client available") + return cmdutil.NewError(cmdutil.CodeServerError, "session resume: no SDK client available") } w := iostreams.IO.Out @@ -165,52 +164,27 @@ func runContinueStream(ctx context.Context, opts *ContinueStreamOptions, _ *cmdu // 2. Open the SDK replay stream and pass each event through as a bare // NDJSON line. The SDK's StreamResponse is the source of truth for // the event vocabulary; the CLI does not reshape it. - var streamErrMsg string + // The SDK invokes the callback for each event (including a terminal + // response_type=error frame) BEFORE returning, so raw passthrough still + // emits every event; on a terminal error frame the SDK then returns an + // *SSEStreamError. No CLI-level early-terminate is needed. cb := func(r *sdk.StreamResponse) error { - isErr := r != nil && r.ResponseType == sdk.ResponseTypeError - if isErr && streamErrMsg == "" { - if r.Content != "" { - streamErrMsg = r.Content - } else if r.Data != nil { - if e, ok := r.Data["error"].(string); ok { - streamErrMsg = e - } - } - } - if err := output.EmitSDKEvent(w, r); err != nil { - return err - } - // Stop once the error frame is emitted rather than blocking on the SDK - // read until the ~30s transport timeout (the server holds the stream - // open after the error). streamErrMsg carries the real reason. - if isErr { - return sse.ErrTerminate - } - return nil + return output.EmitSDKEvent(w, r) } err := svc.ContinueStream(ctx, opts.SessionID, opts.MessageID, cb) - if errors.Is(err, sse.ErrTerminate) { - err = nil - } - // A server-delivered error frame is the authoritative failure reason — - // surface it as operation.failed (exit 1) instead of the transport timeout - // the server's stream-close triggers afterwards. Mirrors chat / session ask. - if streamErrMsg != "" { - return cmdutil.NewError(cmdutil.CodeOperationFailed, "continue stream failed: "+streamErrMsg) - } if err != nil { // Ctrl-C / SIGTERM lineage (operator gave up on the resume). if cmdutil.IsCancelled(ctx, err) { - return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, err, "session continue-stream cancelled") + return cmdutil.Wrapf(cmdutil.CodeOperationCancelled, err, "session resume cancelled") } - // Pre-stream HTTP / transport failure (e.g. 404 if message_id is - // unknown, or buffer-expired body from the server). Route through - // the canonical classifier so codes survive — 404 still surfaces - // as resource.not_found etc. - return cmdutil.WrapHTTP(err, "continue stream") + // WrapStream routes through ClassifySDKError: a terminal SSE error + // frame classifies as server.error (matching chat / session ask); a + // pre-stream HTTP failure (e.g. 404 for an unknown message_id) still + // surfaces via ClassifyHTTPError as resource.not_found etc. + return cmdutil.WrapStream(err, "resume stream") } return nil } -// compile-time check: production SDK client satisfies ContinueStreamService. -var _ ContinueStreamService = (*sdk.Client)(nil) +// compile-time check: production SDK client satisfies ResumeService. +var _ ResumeService = (*sdk.Client)(nil) diff --git a/cli/cmd/session/continue_stream_test.go b/cli/cmd/session/resume_test.go similarity index 72% rename from cli/cmd/session/continue_stream_test.go rename to cli/cmd/session/resume_test.go index bbdc18df1..6f5f34ab9 100644 --- a/cli/cmd/session/continue_stream_test.go +++ b/cli/cmd/session/resume_test.go @@ -16,9 +16,9 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// scriptedContinueStreamSvc serves a canned stream of StreamResponse events -// to runContinueStream and records the (sessionID, messageID) passed in. -type scriptedContinueStreamSvc struct { +// scriptedResumeSvc serves a canned stream of StreamResponse events +// to runResume and records the (sessionID, messageID) passed in. +type scriptedResumeSvc struct { events []*sdk.StreamResponse streamErr error got struct { @@ -27,7 +27,7 @@ type scriptedContinueStreamSvc struct { } } -func (s *scriptedContinueStreamSvc) ContinueStream(_ context.Context, sessionID, messageID string, cb func(*sdk.StreamResponse) error) error { +func (s *scriptedResumeSvc) ContinueStream(_ context.Context, sessionID, messageID string, cb func(*sdk.StreamResponse) error) error { s.got.sessionID = sessionID s.got.messageID = messageID for _, e := range s.events { @@ -51,11 +51,11 @@ func contStreamComplete() *sdk.StreamResponse { // arrives. func TestContinueStream_NDJSON_FirstLineIsInitWithMessageID(t *testing.T) { out, _ := iostreams.SetForTest(t) - svc := &scriptedContinueStreamSvc{ + svc := &scriptedResumeSvc{ events: []*sdk.StreamResponse{contStreamAnswer("hello"), contStreamComplete()}, } - opts := &ContinueStreamOptions{SessionID: "sess_xyz", MessageID: "msg_abc"} - require.NoError(t, runContinueStream(context.Background(), opts, ndjsonOpts(), svc)) + opts := &ResumeOptions{SessionID: "sess_xyz", MessageID: "msg_abc"} + require.NoError(t, runResume(context.Background(), opts, ndjsonOpts(), svc)) lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") require.GreaterOrEqual(t, len(lines), 1, "expected at least the init line") @@ -75,15 +75,15 @@ func TestContinueStream_NDJSON_FirstLineIsInitWithMessageID(t *testing.T) { // events = N+1 total lines, all valid JSON. func TestContinueStream_NDJSON_PassthroughEvents(t *testing.T) { out, _ := iostreams.SetForTest(t) - svc := &scriptedContinueStreamSvc{ + svc := &scriptedResumeSvc{ events: []*sdk.StreamResponse{ contStreamAnswer("alpha"), contStreamAnswer("beta"), contStreamComplete(), }, } - opts := &ContinueStreamOptions{SessionID: "sess_x", MessageID: "msg_y"} - require.NoError(t, runContinueStream(context.Background(), opts, ndjsonOpts(), svc)) + opts := &ResumeOptions{SessionID: "sess_x", MessageID: "msg_y"} + require.NoError(t, runResume(context.Background(), opts, ndjsonOpts(), svc)) lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") // 1 init + 3 SDK events = 4 lines. @@ -98,9 +98,9 @@ func TestContinueStream_NDJSON_PassthroughEvents(t *testing.T) { // flow through to the SDK call. func TestContinueStream_PassesSessionAndMessageIDToSDK(t *testing.T) { _, _ = iostreams.SetForTest(t) - svc := &scriptedContinueStreamSvc{events: []*sdk.StreamResponse{contStreamComplete()}} - opts := &ContinueStreamOptions{SessionID: "sess_42", MessageID: "msg_99"} - require.NoError(t, runContinueStream(context.Background(), opts, ndjsonOpts(), svc)) + svc := &scriptedResumeSvc{events: []*sdk.StreamResponse{contStreamComplete()}} + opts := &ResumeOptions{SessionID: "sess_42", MessageID: "msg_99"} + require.NoError(t, runResume(context.Background(), opts, ndjsonOpts(), svc)) assert.Equal(t, "sess_42", svc.got.sessionID) assert.Equal(t, "msg_99", svc.got.messageID) } @@ -110,9 +110,9 @@ func TestContinueStream_PassesSessionAndMessageIDToSDK(t *testing.T) { // core must also refuse empty strings). func TestContinueStream_EmptySessionID_Rejected(t *testing.T) { _, _ = iostreams.SetForTest(t) - svc := &scriptedContinueStreamSvc{} - opts := &ContinueStreamOptions{SessionID: "", MessageID: "msg_x"} - err := runContinueStream(context.Background(), opts, ndjsonOpts(), svc) + svc := &scriptedResumeSvc{} + opts := &ResumeOptions{SessionID: "", MessageID: "msg_x"} + err := runResume(context.Background(), opts, ndjsonOpts(), svc) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -123,9 +123,9 @@ func TestContinueStream_EmptySessionID_Rejected(t *testing.T) { // point. func TestContinueStream_EmptyMessageID_Rejected(t *testing.T) { _, _ = iostreams.SetForTest(t) - svc := &scriptedContinueStreamSvc{} - opts := &ContinueStreamOptions{SessionID: "sess_x", MessageID: ""} - err := runContinueStream(context.Background(), opts, ndjsonOpts(), svc) + svc := &scriptedResumeSvc{} + opts := &ResumeOptions{SessionID: "sess_x", MessageID: ""} + err := runResume(context.Background(), opts, ndjsonOpts(), svc) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -138,9 +138,9 @@ func TestContinueStream_Cancellation_MapsToOperationCancelled(t *testing.T) { _, _ = iostreams.SetForTest(t) ctx, cancel := context.WithCancel(context.Background()) cancel() - svc := &scriptedContinueStreamSvc{streamErr: context.Canceled} - opts := &ContinueStreamOptions{SessionID: "sess_x", MessageID: "msg_x"} - err := runContinueStream(ctx, opts, ndjsonOpts(), svc) + svc := &scriptedResumeSvc{streamErr: context.Canceled} + opts := &ResumeOptions{SessionID: "sess_x", MessageID: "msg_x"} + err := runResume(ctx, opts, ndjsonOpts(), svc) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) @@ -152,20 +152,36 @@ func TestContinueStream_Cancellation_MapsToOperationCancelled(t *testing.T) { // the canonical HTTP classifier. func TestContinueStream_NotFound_MapsToResourceNotFound(t *testing.T) { _, _ = iostreams.SetForTest(t) - svc := &scriptedContinueStreamSvc{streamErr: errors.New("HTTP error 404: not found")} - opts := &ContinueStreamOptions{SessionID: "sess_x", MessageID: "msg_missing"} - err := runContinueStream(context.Background(), opts, ndjsonOpts(), svc) + svc := &scriptedResumeSvc{streamErr: errors.New("HTTP error 404: not found")} + opts := &ResumeOptions{SessionID: "sess_x", MessageID: "msg_missing"} + err := runResume(context.Background(), opts, ndjsonOpts(), svc) require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) assert.Equal(t, cmdutil.CodeResourceNotFound, typed.Code) } +// TestResume_TerminalStreamError_MapsToServerError pins that a terminal SSE +// error frame (surfaced by the SDK as *SSEStreamError) classifies as +// server.error (exit 7) — the SAME as chat / session ask. Guards against the +// prior inconsistency where resume reported the identical server condition as +// exit 1 while chat/ask reported exit 7. +func TestResume_TerminalStreamError_MapsToServerError(t *testing.T) { + _, _ = iostreams.SetForTest(t) + svc := &scriptedResumeSvc{streamErr: sdk.NewSSEStreamError("no chat model configured")} + opts := &ResumeOptions{SessionID: "sess_x", MessageID: "msg_x"} + err := runResume(context.Background(), opts, ndjsonOpts(), svc) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeServerError, typed.Code) +} + // TestContinueStream_RequiresMessageFlag verifies cobra refuses to run the // command without --message (the flag is marked required). func TestContinueStream_RequiresMessageFlag(t *testing.T) { f := &cmdutil.Factory{} - cmd := NewCmdContinueStream(f) + cmd := NewCmdResume(f) var buf bytes.Buffer cmd.SetOut(&buf) cmd.SetErr(&buf) @@ -185,7 +201,7 @@ func TestContinueStream_RequiresMessageFlag(t *testing.T) { // command without the positional . func TestContinueStream_RequiresSessionIDArg(t *testing.T) { f := &cmdutil.Factory{} - cmd := NewCmdContinueStream(f) + cmd := NewCmdResume(f) var buf bytes.Buffer cmd.SetOut(&buf) cmd.SetErr(&buf) diff --git a/cli/cmd/session/session.go b/cli/cmd/session/session.go index d30da35ef..b6c120536 100644 --- a/cli/cmd/session/session.go +++ b/cli/cmd/session/session.go @@ -1,5 +1,5 @@ // Package sessioncmd holds `weknora session` command tree (list / view / -// delete / ask / continue-stream / stop) for chat history and agent invocation. +// delete / ask / resume / stop) 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 @@ -22,7 +22,7 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(NewCmdView(f)) cmd.AddCommand(NewCmdDelete(f)) cmd.AddCommand(NewCmdAsk(f)) - cmd.AddCommand(NewCmdContinueStream(f)) + cmd.AddCommand(NewCmdResume(f)) cmd.AddCommand(NewCmdStop(f)) cmd.AddCommand(NewCmdToolApproval(f)) return cmd diff --git a/cli/cmd/session/stop.go b/cli/cmd/session/stop.go index db74ad11e..6e4de4469 100644 --- a/cli/cmd/session/stop.go +++ b/cli/cmd/session/stop.go @@ -4,9 +4,9 @@ // Unlike Ctrl-C (which only drops the local connection while the server keeps // generating and billing tokens), this tells the server to stop. // -// This is the symmetric counterpart to `session continue-stream`: both key on +// This is the symmetric counterpart to `session resume`: both key on // (session_id, message_id). The message_id comes from the init event of the -// original chat / session ask / continue-stream stream. +// original chat / session ask / resume stream. package sessioncmd import ( @@ -52,7 +52,7 @@ func NewCmdStop(f *cmdutil.Factory) *cobra.Command { session. Unlike Ctrl-C (which only drops the local connection while the server keeps generating and billing tokens), this tells the server to stop. -Symmetric with 'session continue-stream': both key on (session_id, message_id).`, +Symmetric with 'session resume': both key on (session_id, message_id).`, Example: ` weknora session stop sess_xyz --message msg_abc`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { @@ -81,7 +81,7 @@ Symmetric with 'session continue-stream': both key on (session_id, message_id).` cmdutil.AddFormatFlag(cmd, stopFields...) cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "Stop server-side generation for an in-flight assistant message (counterpart to continue-stream). The message_id comes from the init event of the chat / session ask / continue-stream stream you're stopping.", + UsedFor: "Stop server-side generation for an in-flight assistant message (counterpart to resume). The message_id comes from the init event of the chat / session ask / resume stream you're stopping.", RequiredFlags: []string{" (positional)", "--message (message_id from the init event of the stream you're stopping)"}, Examples: []string{"weknora session stop sess_xyz --message msg_abc"}, Output: "envelope {session_id, message_id, stopped:true}", diff --git a/cli/cmd/session/tool_approval.go b/cli/cmd/session/tool_approval.go index 44c18ca67..899b162c6 100644 --- a/cli/cmd/session/tool_approval.go +++ b/cli/cmd/session/tool_approval.go @@ -49,7 +49,7 @@ When a server-side agent run (weknora session ask) needs to call a tool that requires approval, the stream emits a tool-approval event carrying a pending id and the run blocks. This command unblocks it: approve (default) lets the tool call execute, --reject cancels it. After resolving, resume -the answer with weknora session continue-stream. +the answer with weknora session resume. --modified-args replaces the tool call arguments on approve (JSON object). It conflicts with --reject (rejected calls never execute). @@ -122,7 +122,7 @@ func newCmdResolve(f *cmdutil.Factory) *cobra.Command { cmdutil.AddDryRunFlag(cmd, &opts.DryRun) cmdutil.SetRisk(cmd, "session.tool_approval.resolve") cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "approve or reject a pending tool call from an agent run; then resume with session continue-stream", + UsedFor: "approve or reject a pending tool call from an agent run; then resume with session resume", RequiredFlags: []string{" (positional)"}, Examples: []string{ "weknora session tool-approval resolve pend_abc -y", diff --git a/cli/cmd/skills/skills.go b/cli/cmd/skills/skills.go index 883f7649c..4ec2a2bc5 100644 --- a/cli/cmd/skills/skills.go +++ b/cli/cmd/skills/skills.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "text/tabwriter" "github.com/spf13/cobra" @@ -148,10 +149,13 @@ Existing files are left untouched unless --force is passed.`, return cmd } -// resolveDir returns the explicit --dir or the default ~/.claude/skills. +// resolveDir returns the explicit --dir or the default ~/.claude/skills. A +// leading ~ in --dir is expanded to the home directory — otherwise a quoted +// `--dir '~/foo'` (which the shell leaves literal) would create a bogus "~" +// directory tree. func resolveDir(dir string) (string, error) { if dir != "" { - return dir, nil + return expandTilde(dir) } home, err := os.UserHomeDir() if err != nil { @@ -161,6 +165,25 @@ func resolveDir(dir string) (string, error) { return filepath.Join(home, ".claude", "skills"), nil } +// expandTilde resolves a leading "~" or "~/" path segment to the user's home +// directory. Other forms (including "~user" and a ~ that isn't the first +// segment) are returned unchanged — matching the common shell behavior a CLI +// is expected to reproduce when it receives an unexpanded literal tilde. +func expandTilde(path string) (string, error) { + if path != "~" && !strings.HasPrefix(path, "~/") { + return path, nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", cmdutil.NewError(cmdutil.CodeInputInvalidArgument, + "could not expand ~ (home directory unknown); pass an absolute --dir") + } + if path == "~" { + return home, nil + } + return filepath.Join(home, path[len("~/"):]), nil +} + // writeSkills writes every embedded skill file under target, creating parent // dirs. Without --force, an existing file is skipped (not overwritten). Returns // the paths actually written. diff --git a/cli/cmd/skills/skills_test.go b/cli/cmd/skills/skills_test.go index 79e86d3e0..f309dfcb9 100644 --- a/cli/cmd/skills/skills_test.go +++ b/cli/cmd/skills/skills_test.go @@ -73,3 +73,26 @@ func TestResolveDir(t *testing.T) { assert.True(t, filepath.IsAbs(def), "default dir must be absolute") assert.Contains(t, def, filepath.Join(".claude", "skills")) } + +// TestResolveDir_ExpandsTilde pins that a leading ~ in --dir is expanded to the +// home directory instead of creating a literal "~" directory. Regression: +// `skills install --dir '~/foo'` (quoted, so the shell doesn't expand it) used +// to create a bogus ./~ tree. +func TestResolveDir_ExpandsTilde(t *testing.T) { + home, err := os.UserHomeDir() + require.NoError(t, err) + + got, err := resolveDir("~/agents/skills") + require.NoError(t, err) + assert.Equal(t, filepath.Join(home, "agents", "skills"), got) + assert.NotContains(t, got, "~", "~ must be expanded, not left literal") + + bare, err := resolveDir("~") + require.NoError(t, err) + assert.Equal(t, home, bare) + + // A ~ that is NOT a leading path segment is left untouched (not a home ref). + lit, err := resolveDir("/tmp/a~b") + require.NoError(t, err) + assert.Equal(t, "/tmp/a~b", lit) +} diff --git a/cli/internal/cmdutil/batch.go b/cli/internal/cmdutil/batch.go index e97ecf49e..da589ad2b 100644 --- a/cli/internal/cmdutil/batch.go +++ b/cli/internal/cmdutil/batch.go @@ -40,7 +40,11 @@ func RunBatch(ctx context.Context, ids []string, op func(context.Context, string for _, id := range ids { select { case <-ctx.Done(): - outcomes = append(outcomes, BatchOutcome{ID: id, Err: ctx.Err()}) + // Classify the context signal so the per-item envelope reports + // operation.cancelled / operation.timeout (ClassifyContextErr) + // instead of the generic internal.error, and an all-aborted batch + // exits by that class. Cause preserved so errors.Is still matches. + outcomes = append(outcomes, BatchOutcome{ID: id, Err: Wrapf(ClassifyContextErr(ctx.Err()), ctx.Err(), "operation on %s aborted", id)}) failed++ continue default: @@ -52,12 +56,18 @@ func RunBatch(ctx context.Context, ids []string, op func(context.Context, string } } if failed > 0 { + // Any failure → operation.failed (exit 1). The aggregate code is + // deliberately coarse: the per-item batch envelope already carries each + // item's typed error (type + exit_code via ErrorToDetail), which is the + // authoritative per-item signal an agent should branch on. Collapsing + // "all failed with the same class" into that class was extra machinery + // for a convenience the per-item data already provides. return outcomes, &Error{ Code: CodeOperationFailed, Message: fmt.Sprintf("%d/%d operation(s) failed", failed, len(ids)), // Silent suppresses the stderr error envelope because the caller // already emitted the batch envelope to stdout. The exit code - // still propagates via Error.Code → ExitCode (falls through to 1). + // still propagates via Error.Code → ExitCode. Silent: true, } } diff --git a/cli/internal/cmdutil/batch_test.go b/cli/internal/cmdutil/batch_test.go index b832072d2..f0bc8eee4 100644 --- a/cli/internal/cmdutil/batch_test.go +++ b/cli/internal/cmdutil/batch_test.go @@ -72,8 +72,8 @@ func TestRunBatch_PartialFailure(t *testing.T) { // TestRunBatch_StatusExitTriState verifies the batch tri-state exit mapping: // all-success → exit 0 (nil summaryErr), partial → exit 1, all-fail → exit 1 -// (operation.failed fall-through). Pairs with the envelope-status tri-state in -// output.TestWriteBatchEnvelope_StatusTriState. +// (any failure collapses to operation.failed). Pairs with the envelope-status +// tri-state in output.TestWriteBatchEnvelope_StatusTriState. func TestRunBatch_StatusExitTriState(t *testing.T) { failIf := func(fails map[string]bool) func(context.Context, string) error { return func(_ context.Context, id string) error { @@ -224,3 +224,30 @@ func TestEmitBatch_Text_PerLine(t *testing.T) { t.Errorf("expected 'FAIL y: boom' line; got %q", got) } } + +// TestRunBatch_AllFailExit1 - any batch failure (partial OR all) collapses to +// operation.failed → exit 1. The authoritative per-item detail lives in the +// batch envelope (each item's typed error); the aggregate exit code is +// deliberately coarse. +func TestRunBatch_AllFailExit1(t *testing.T) { + notFound := func(_ context.Context, id string) error { + return NewError(CodeResourceNotFound, "no such thing "+id) + } + _, summaryErr := RunBatch(context.Background(), []string{"a", "b"}, notFound) + if got := ExitCode(summaryErr); got != 1 { + t.Errorf("all-fail batch ExitCode = %d, want 1; err=%v", got, summaryErr) + } +} + +// TestRunBatch_ContextErrorsClassifiedPerItem verifies per-item context errors +// are classified as operation.cancelled / operation.timeout in the batch +// envelope (not the generic internal.error), so an agent inspecting the +// per-item results sees why each item aborted. (The aggregate exit stays 1.) +func TestRunBatch_ContextErrorsClassifiedPerItem(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // pre-cancel so every item takes the ctx.Done branch + outcomes, _ := RunBatch(ctx, []string{"a"}, func(context.Context, string) error { return nil }) + if got := ErrorToDetail(outcomes[0].Err).Type; got != string(CodeOperationCancelled) { + t.Errorf("per-item type = %q, want %q", got, CodeOperationCancelled) + } +} diff --git a/cli/internal/cmdutil/errors.go b/cli/internal/cmdutil/errors.go index cf74db4e9..c4e96fa67 100644 --- a/cli/internal/cmdutil/errors.go +++ b/cli/internal/cmdutil/errors.go @@ -228,9 +228,11 @@ func ErrorToDetail(err error) *output.ErrDetail { detail := &output.ErrDetail{ Type: string(typed.Code), Message: msg, + ExitCode: ExitCode(err), Hint: hint, RetryArgv: retry, RetryAfterSeconds: typed.RetryAfterSeconds, + Retryable: retryableForCode(typed.Code), Detail: typed.Detail, } if typed.Risk != nil { @@ -245,12 +247,14 @@ func ErrorToDetail(err error) *output.ErrDetail { var fe *FlagError if errors.As(err, &fe) { return &output.ErrDetail{ - Type: string(CodeInputInvalidArgument), - Message: err.Error(), - Hint: defaultHint(CodeInputInvalidArgument), + Type: string(CodeInputInvalidArgument), + Message: err.Error(), + ExitCode: ExitCode(err), // FlagError → 2, distinguishing parse from typed-value (exit 5) + Hint: defaultHint(CodeInputInvalidArgument), + Retryable: retryableForCode(CodeInputInvalidArgument), } } - return &output.ErrDetail{Type: "internal.error", Message: err.Error()} + return &output.ErrDetail{Type: string(CodeInternalError), Message: err.Error(), ExitCode: ExitCode(err)} } // NewError constructs a typed error. diff --git a/cli/internal/cmdutil/errors_retry_test.go b/cli/internal/cmdutil/errors_retry_test.go index 5ad6fcdb3..59ec16490 100644 --- a/cli/internal/cmdutil/errors_retry_test.go +++ b/cli/internal/cmdutil/errors_retry_test.go @@ -1,6 +1,7 @@ package cmdutil import ( + "errors" "reflect" "testing" ) @@ -30,3 +31,32 @@ func TestError_RetryArgv_EmptyByDefault(t *testing.T) { t.Errorf("RetryArgv should default empty; got %v", err.RetryArgv) } } + +// TestErrorToDetail_CarriesExitCode verifies every error detail embeds the +// authoritative exit_code so an agent can branch on a single JSON read without +// observing $?. Regression: `input.invalid_argument` spans exit 2 (parse) and +// exit 5 (typed value), so `type` alone was insufficient — exit_code +// disambiguates them in the envelope. +func TestErrorToDetail_CarriesExitCode(t *testing.T) { + cases := []struct { + name string + err error + want int + }{ + {"not_found", NewError(CodeResourceNotFound, "x"), 4}, + {"typed_input_value", NewError(CodeInputInvalidArgument, "bad value"), 5}, + {"auth", NewError(CodeAuthUnauthenticated, "x"), 3}, + {"parse_flagerror", NewFlagError(errors.New("unknown flag: --nope")), 2}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + d := ErrorToDetail(tc.err) + if d == nil { + t.Fatal("nil detail") + } + if d.ExitCode != tc.want { + t.Errorf("exit_code = %d, want %d (type=%s)", d.ExitCode, tc.want, d.Type) + } + }) + } +} diff --git a/cli/internal/cmdutil/factory.go b/cli/internal/cmdutil/factory.go index e59ba12c0..6854a5423 100644 --- a/cli/internal/cmdutil/factory.go +++ b/cli/internal/cmdutil/factory.go @@ -127,7 +127,13 @@ func buildClient(f *Factory) (*sdk.Client, error) { } profileName := cfg.CurrentProfile if profileName == "" { - return nil, NewError(CodeAuthUnauthenticated, "no current profile configured; run `weknora auth login` to set one up") + // Zero-state: no profile exists at all. The generic auth.unauthenticated + // default retry_argv is `auth login`, but that ALSO fails here (it needs + // an active profile) — an agent that execs retry_argv would loop. Point + // hint + retry_argv at the real first step: create a profile. + return nil, NewError(CodeAuthUnauthenticated, "no profile configured"). + WithHint("add one first: `weknora profile add --host --use`, then `weknora auth login` (or set WEKNORA_API_KEY + WEKNORA_HOST for headless use)"). + WithRetryArgv([]string{"weknora", "profile", "add", "--help"}) } prof, ok := cfg.Profiles[profileName] if !ok { @@ -254,6 +260,15 @@ func AddKBFlag(cmd *cobra.Command) { cmd.Flags().String("kb", "", "Knowledge base UUID or name (overrides env / project link)") } +// AddIgnoredKBFlag registers a no-op --kb on id-addressed commands (doc view / +// wait, chunk list / view). Those resolve a globally-unique doc/chunk id, so +// --kb is redundant — but an agent flowing from `doc upload --kb X` naturally +// carries it, and rejecting it with exit 2 is pure friction. Accepted and +// ignored; declared here so schema still lists it truthfully. +func AddIgnoredKBFlag(cmd *cobra.Command) { + cmd.Flags().String("kb", "", "Ignored — the id argument is globally unique; accepted for symmetry with `doc list`/`doc upload` so a carried-over --kb doesn't error") +} + // ResolveKB returns the active KB id for the running command, applying the // 4-level fallback chain (highest to lowest): // 1. --kb flag (kb_<...> id passed through; anything else resolved via diff --git a/cli/internal/cmdutil/factory_test.go b/cli/internal/cmdutil/factory_test.go index 31a00e871..07b47a262 100644 --- a/cli/internal/cmdutil/factory_test.go +++ b/cli/internal/cmdutil/factory_test.go @@ -174,6 +174,13 @@ func TestBuildClient_NoCurrentProfile(t *testing.T) { var typed *Error require.ErrorAs(t, err, &typed) assert.Equal(t, CodeAuthUnauthenticated, typed.Code) + // Zero-state retry_argv must NOT be the generic `auth login` (which itself + // fails with no profile → an agent execing retry_argv would loop); it must + // point at profile creation instead. + detail := ErrorToDetail(err) + assert.NotEqual(t, []string{"weknora", "auth", "login"}, detail.RetryArgv, + "zero-state retry_argv must not loop back to auth login") + assert.Contains(t, detail.RetryArgv, "profile", "zero-state retry_argv should point at profile setup") } func TestBuildClient_UnknownContext(t *testing.T) { diff --git a/cli/internal/cmdutil/format.go b/cli/internal/cmdutil/format.go index 7e60398c1..6119f30f6 100644 --- a/cli/internal/cmdutil/format.go +++ b/cli/internal/cmdutil/format.go @@ -47,7 +47,12 @@ func AddFormatFlag(cmd *cobra.Command, fieldHints ...string) { if len(fieldHints) > 0 { sorted := append([]string(nil), fieldHints...) sort.Strings(sorted) - hdr := "\n\nJSON fields available (for --jq projection):\n " + + // Fields live under .data in the {ok,data,meta} envelope, so --jq must + // be rooted there: `--jq '.data.'` (object) or + // `--jq '.data[].'` (list). A bare `--jq '.'` matches the + // envelope top level and silently returns null — spell out the path so + // agents don't ship broken projections. + hdr := "\n\nJSON fields available under .data (project with --jq '.data.', or '.data[].' for lists):\n " + strings.Join(sorted, "\n ") if cmd.Long != "" { cmd.Long += hdr @@ -144,9 +149,14 @@ func mapJQError(err error) error { } // ResolveDefault fills in Mode when the caller has not explicitly set it: -// - Mode defaults to FormatJSON +// - Mode defaults to FormatJSON, regardless of TTY // - TTY only affects the indent decision (auto-indent in TTY; compact in pipe) // - For human-readable rendering, pass --format text explicitly +// +// JSON-always (not a TTY switch to text on a terminal) is deliberate: an +// agent-first CLI values output predictability over terminal ergonomics, so +// the default never depends on whether stdout is a TTY. Humans opt into +// human-readable output with `--format text`. func (o *FormatOptions) ResolveDefault(tty bool) { o.TTY = tty // Apply WEKNORA_FORMAT before the hard default so the documented diff --git a/cli/internal/cmdutil/format_test.go b/cli/internal/cmdutil/format_test.go index c0e4a84d3..c23175af4 100644 --- a/cli/internal/cmdutil/format_test.go +++ b/cli/internal/cmdutil/format_test.go @@ -96,6 +96,9 @@ func TestFormatOptions_TextModeReturnsError(t *testing.T) { // TestResolveDefault_AlwaysJSON verifies v0.7 semantics: default is FormatJSON // regardless of whether stdout is a TTY (BREAKING change from v0.6). +// TestResolveDefault_AlwaysJSON pins the deliberate JSON-always default (no +// TTY switch to text): the default output never depends on whether stdout is a +// TTY, so agents get predictable JSON. Humans opt into text with --format text. func TestResolveDefault_AlwaysJSON(t *testing.T) { for _, isTTY := range []bool{true, false} { o := &FormatOptions{} @@ -117,7 +120,7 @@ func TestResolveDefault(t *testing.T) { isTTY bool wantMode FormatMode }{ - // v0.7: empty Mode always resolves to FormatJSON regardless of TTY. + // JSON-always default regardless of TTY (predictable for agents). {"empty isTTY", "", "", true, FormatJSON}, {"empty no-tty", "", "", false, FormatJSON}, {"already set keeps value tty", FormatNDJSON, "", true, FormatNDJSON}, diff --git a/cli/internal/cmdutil/profilename_test.go b/cli/internal/cmdutil/profilename_test.go index 18576d188..075571ae2 100644 --- a/cli/internal/cmdutil/profilename_test.go +++ b/cli/internal/cmdutil/profilename_test.go @@ -69,7 +69,7 @@ func TestValidateProfileName_RejectsShellMetachars(t *testing.T) { for _, name := range cases { err := ValidateProfileName(name) if err == nil { - t.Errorf("ValidateProfileName(%q) should have rejected the name; an agent exec'ing retry_command would be injectable", name) + t.Errorf("ValidateProfileName(%q) should have rejected the name; a name echoed into retry_argv / prose would be injectable", name) continue } var ce *Error diff --git a/cli/internal/cmdutil/risk.go b/cli/internal/cmdutil/risk.go index 15a339b89..9217e0cf4 100644 --- a/cli/internal/cmdutil/risk.go +++ b/cli/internal/cmdutil/risk.go @@ -14,7 +14,7 @@ package cmdutil import "github.com/spf13/cobra" // Risk levels emitted in the annotation / envelope: -// - RiskDestructive: irreversible ops (delete, kb init clobber). +// - RiskDestructive: irreversible ops (delete). // - RiskWrite: reversible metadata edits (kb / agent / doc update). // // "read" remains reserved (read-only commands carry no risk annotation). diff --git a/cli/internal/mcp/tools.go b/cli/internal/mcp/tools.go index b1913eebb..e160c2c7d 100644 --- a/cli/internal/mcp/tools.go +++ b/cli/internal/mcp/tools.go @@ -659,14 +659,15 @@ func addChunkList(server *mcpsdk.Server, svc chunkListService) { } // `limit` is typed as int by chunkListInput, so the SDK rejects // non-numeric values at schema validation (e.g. "limit":"50") - // before this handler runs. Here we only default+clamp the - // already-decoded value. + // before this handler runs. Default when unset; reject over-max + // (rather than silently clamping) so the agent's request is never + // quietly changed — matching search_chunks in this same file. limit := in.Limit if limit < 1 { limit = chunkListDefaultLimit } if limit > chunkListMaxLimit { - limit = chunkListMaxLimit + return toolErrorResult(cmdutil.NewError(cmdutil.CodeInputInvalidArgument, fmt.Sprintf("limit must be in 1..%d", chunkListMaxLimit))), nil, nil } chunks, total, err := svc.ListKnowledgeChunks(ctx, in.DocID, 1, limit) if err != nil { diff --git a/cli/internal/output/envelope.go b/cli/internal/output/envelope.go index ce73033ca..149ab1ac0 100644 --- a/cli/internal/output/envelope.go +++ b/cli/internal/output/envelope.go @@ -45,6 +45,11 @@ type Meta struct { // Non-batch commands leave these nil so they are omitted from the envelope. Successes *int `json:"successes,omitempty"` // batch ops Failures *int `json:"failures,omitempty"` // batch ops + // Hint is an optional actionable note on a SUCCESS envelope — e.g. an + // empty search explaining the KB may be unindexed, or a freshly-created + // draft document pointing at `doc reparse`. Distinct from error.hint; + // omitted when empty so it never adds noise to normal results. + Hint string `json:"hint,omitempty"` // Dry-run preview fields. Populated by EmitDryRun (cmdutil/dryrun.go) // when --dry-run is set on a mutation command; omitted otherwise. DryRun bool `json:"dry_run,omitempty"` // true when --dry-run; omitted otherwise @@ -56,7 +61,12 @@ type Meta struct { type ErrDetail struct { Type string `json:"type"` Message string `json:"message"` - Hint string `json:"hint,omitempty"` + // ExitCode is the process exit code this error maps to, embedded so an + // agent can branch on a single JSON read without observing $?. Needed + // because one type (input.invalid_argument) spans exit 2 (parse) and + // exit 5 (typed value) — exit_code disambiguates them. + ExitCode int `json:"exit_code,omitempty"` + Hint string `json:"hint,omitempty"` // RetryArgv is a directly-executable argv array (e.g. // ["weknora","auth","login"]) so an agent can exec it without // shell-splitting or quote-handling. Distinct from the prose Hint. @@ -73,7 +83,8 @@ type ErrDetail struct { // RiskDetail tags high-risk writes for the agent protocol. Surfaces in // error.risk on confirmation_required errors. -// Level: only "destructive" is emitted; "read" / "write" slots reserved. +// Level: "write" (reversible mutations — update) or "destructive" (delete); +// the "read" slot is reserved. type RiskDetail struct { Level string `json:"level"` Action string `json:"action"` diff --git a/cli/internal/output/ndjson_stream.go b/cli/internal/output/ndjson_stream.go index f0a4fafc8..300f26f8a 100644 --- a/cli/internal/output/ndjson_stream.go +++ b/cli/internal/output/ndjson_stream.go @@ -13,7 +13,7 @@ import ( type InitEvent struct { Type string `json:"type"` SessionID string `json:"session_id"` - // MessageID anchors a resumed stream (`session continue-stream`) to the + // MessageID anchors a resumed stream (`session resume`) to the // specific assistant message whose event buffer is being replayed. Empty // for fresh streams (chat / session ask) where the message id is only // known after the SDK emits its first agent_query frame. diff --git a/cli/internal/sse/accumulator.go b/cli/internal/sse/accumulator.go index ba4f755c2..26546402b 100644 --- a/cli/internal/sse/accumulator.go +++ b/cli/internal/sse/accumulator.go @@ -9,21 +9,11 @@ package sse import ( - "errors" "strings" sdk "github.com/Tencent/WeKnora/client" ) -// ErrTerminate is the sentinel a stream callback returns to stop consuming the -// stream immediately after a terminal error frame (response_type=error). The -// server holds the SSE connection open after such a frame, so a callback that -// keeps returning nil blocks the SDK read until a ~30s transport timeout — -// turning a sub-second server error into a 30s hang for the (agent) caller. -// Callers derive the real result from the captured error message / accumulator -// and unwrap this sentinel (errors.Is) to a clean stop. -var ErrTerminate = errors.New("sse: stream terminated by terminal error event") - // Accumulator buffers a KnowledgeQAStream callback sequence. // // Zero value is ready to use. Not safe for concurrent Append calls - the SDK diff --git a/cli/scripts/check-secret-tokens.sh b/cli/scripts/check-secret-tokens.sh index 315898855..464bab7b3 100755 --- a/cli/scripts/check-secret-tokens.sh +++ b/cli/scripts/check-secret-tokens.sh @@ -2,8 +2,7 @@ # check-secret-tokens.sh — fail if a real-looking credential was committed to # the CLI's human-facing docs. An agent-first CLI's docs are where a live # API key or JWT most easily gets pasted by accident (copying a working -# session into a how-to). Mirrors the safety net lark CLI ships as -# .gitleaks.toml + check-doc-tokens.sh. +# session into a how-to). Same intent as a gitleaks/doc-token pre-commit scan. # # Heuristic, low false-positive: a real WeKnora API key is `sk-` followed by a # long high-entropy body that CONTAINS A DIGIT (e.g. diff --git a/cli/scripts/check-skill-wire-vocab.sh b/cli/scripts/check-skill-wire-vocab.sh index c6020cb58..068bd0f3c 100755 --- a/cli/scripts/check-skill-wire-vocab.sh +++ b/cli/scripts/check-skill-wire-vocab.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # check-skill-wire-vocab.sh — fail if cli/skills/ still references wire # vocabulary that the CLI has renamed or removed. Wire-shape changes must -# sweep skills in the same PR (pattern borrowed from larksuite/cli). +# sweep skills in the same PR. set -euo pipefail cd "$(dirname "$0")/.." @@ -16,6 +16,8 @@ BANNED=( "auth login --host:profile add --host (auth login dropped --host in v0.9)" "auth login --name:profile add (auth login dropped --name in v0.9)" "agent create --kb:agent create --attach-kb (renamed in v0.9)" + "kb init:kb config set (renamed — kb init removed)" + "continue-stream:session resume (renamed)" ) fail=0 diff --git a/cli/skills/weknora-rag-search/SKILL.md b/cli/skills/weknora-rag-search/SKILL.md index 25e166f42..166c85031 100644 --- a/cli/skills/weknora-rag-search/SKILL.md +++ b/cli/skills/weknora-rag-search/SKILL.md @@ -52,7 +52,7 @@ one wastes turns or returns the wrong shape. Use the decision table. connection; the server keeps generating + billing). Stop it server-side: `weknora session stop --message ` (session_id from `data.session_id`, or from `init` under `--format ndjson`). - Re-attach to a stream with `weknora session continue-stream + Re-attach to a stream with `weknora session resume --message `. - `search chunks --limit` defaults to **8** (tuned for an LLM context window); the `search docs/kb/sessions` lists default to 30. Tune retrieval with diff --git a/cli/skills/weknora-rag-search/references/chat.md b/cli/skills/weknora-rag-search/references/chat.md index 25bb118a2..efeca38ad 100644 --- a/cli/skills/weknora-rag-search/references/chat.md +++ b/cli/skills/weknora-rag-search/references/chat.md @@ -39,7 +39,7 @@ events through verbatim: - Accumulate `response_type:"answer"` `content` pieces for the final answer. - `knowledge_references` carry the grounding chunks (source attribution). - **Keep `init.session_id`** to continue the chat (`--session`). The - `assistant_message_id` needed for `session stop` / `session continue-stream` + `assistant_message_id` needed for `session stop` / `session resume` rides on the SDK's `agent_query` frame, not on `init` — scan for it. - On failure mid-stream you get `response_type:"error"`; a transport/HTTP error surfaces as the normal error envelope on stderr with a typed code. @@ -49,7 +49,7 @@ events through verbatim: - **Stop server-side generation:** `weknora session stop --message `. Ctrl-C only closes your local connection — the server keeps generating (and billing) until told to stop. -- **Re-attach after a dropped connection:** `weknora session continue-stream +- **Re-attach after a dropped connection:** `weknora session resume --message `. The server replays the event log from index 0 then tails new events, so **dedupe by message_id** if you already consumed some events. Buffer TTL is ~1h (redis) or process-lifetime (memory). diff --git a/cli/skills/weknora-shared/SKILL.md b/cli/skills/weknora-shared/SKILL.md index 81a6b46bc..89adfd1c5 100644 --- a/cli/skills/weknora-shared/SKILL.md +++ b/cli/skills/weknora-shared/SKILL.md @@ -13,6 +13,12 @@ output, not prose. Read this skill before any task-specific `weknora-*` skill. ## 1. Authenticate (do this first — order matters) +**Agents usually skip profiles entirely:** set `WEKNORA_API_KEY` (or +`WEKNORA_TOKEN`) + `WEKNORA_HOST` and every command authenticates statelessly and +zero-disk — no `profile add` / `auth login` needed. `auth token` echoes that env +credential; `auth status` / `doctor` confirm it. The steps below set up a +**persistent named profile** instead (interactive / multi-environment use). + Authentication is a **two-step sequence**. `weknora auth login` operates on the *active profile*, so the profile must exist first: @@ -31,7 +37,11 @@ weknora auth status # verify: who am I, which tenant (e.g. `weknora --profile staging auth refresh`). There is no per-command `--name`/`--host` on auth commands. - Get the raw token for scripting with `WEKNORA_TOKEN=$(weknora auth token)` - (raw token by default; `--format json` gives the `{token, mode, profile}` envelope). + (raw token by default; works with an env credential too; `--format json` gives + the `{token, mode, profile}` envelope). +- `weknora auth logout` clears a profile's stored credentials but **keeps the + profile registered** (re-auth later with `auth login`); use `profile remove` + to delete the profile entirely. - `weknora doctor` runs 4 health checks (reachability, credential, version, storage). ## 2. Selecting a knowledge base (`--kb`) @@ -42,6 +52,15 @@ the cwd) → error. Read/create commands that operate "inside a project" inherit the link; **`search *` and destructive `--all` operations always require an explicit `--kb`** (so an agent never silently hits the wrong corpus). +**A KB must have an embedding model bound to be searchable.** A freshly created +KB is `retrieval_ready:false` — uploaded docs stay unindexed and `search`/`chat` +return nothing until you bind models. Create it ready in one step +(`kb create --embedding-model --chat-model `, discover ids with +`weknora model list`) or bind after the fact +(`kb config set --embedding-model --chat-model `). `kb status` / +`kb check` report `retrieval_ready`, and `kb create` hints the fix when it is +false — so an unconfigured KB is never silently "healthy". + ## 3. Output contract — every command Default output is `--format json`: a single envelope. @@ -60,18 +79,22 @@ Default output is `--format json`: a single envelope. - `--format text` = a live human-readable projection. `chat` and `session ask` buffer a bounded answer-event projection into one JSON envelope by default; pass `--reference` for indexed citations, `--verbose` for execution detail, - or `--format ndjson` for raw event lines. `session continue-stream` remains + or `--format ndjson` for raw event lines. `session resume` remains an NDJSON streaming command. - `--jq ''` filters the envelope (e.g. `weknora kb list --jq '.data[].id'`). - Exception: `weknora auth token` emits the **raw token** by default (it's a scripting helper); pass `--format json` for the `{token, mode, profile}` envelope. -- **Batch / wait caveat:** for multi-item commands (`doc/chunk/session delete` - with several ids, `doc wait`), `ok:true` means *the command ran to completion*, - **not** that every item succeeded — per-item failures live in `data` and the - **exit code** carries the aggregate verdict (e.g. `doc wait` exits 1 if any doc - failed, 124 on timeout, while still printing the `{completed,failed,timeout}` - partition with `ok:true`). For these, branch on the **exit code**, then read - `data` for which items failed. +- **Batch / wait caveat:** multi-item commands come in two shapes, but for + **both you branch on the exit code, not `ok`**: + - `doc/chunk/session delete` with several ids → a **batch** envelope: + `status` is `success`/`partial`/`error`, `ok` is `true` *only* when every + item succeeded (**`ok:false` on any failure**), `data` is a per-item array + `[{id, ok, result|error}]`, and `meta.successes`/`failures` count the split. + Exit 1 if any item failed. + - `doc wait` → a normal `ok:true` envelope whose `data` partitions the ids + into `{completed, failed, timeout}`; `ok` stays `true` even with failures + (to avoid a contradictory envelope). Exit 1 if any doc failed, 124 on timeout. + Either way, read the **exit code** first, then `data` for which items failed. ## 4. Exit codes (branch on these) @@ -138,7 +161,7 @@ An agent run pauses mid-stream on a tool-approval event when the server requires 1. The stream emits a tool-approval event; capture the `pending_id`. 2. **Surface the pending tool call to the user** (show tool name + proposed args). Do not auto-approve. 3. After explicit user go-ahead: `weknora session tool-approval resolve -y` to approve, or add `--reject --reason "..."` to reject. -4. Resume the answer: `weknora session continue-stream --message `. +4. Resume the answer: `weknora session resume --message `. `--modified-args '{"key":"val"}'` replaces the tool arguments on approve (non-empty JSON object required). This is an exit-10 interaction — see §5. @@ -148,10 +171,10 @@ An agent run pauses mid-stream on a tool-approval event when the server requires kb knowledge bases list/view/create/update/delete/pin/unpin/status/check doc documents in a KB list/view/create/upload/fetch/download/reparse/update/delete/wait chunk retrieval units list/view/delete (RAG debug; not search) -session conversations list/view/delete/ask/stop/continue-stream/tool-approval resolve +session conversations list/view/delete/ask/stop/resume/tool-approval resolve message session messages list/search/delete agent custom agents list/view/create/update/delete/status/check -model configured models list/view (read-only; find the id for agent create --model) +model configured models list/view/create/update/delete (update rotates key / base-url in place, id preserved) search retrieval chunks / docs / kb / sessions chat one-shot KB RAG Q&A (streaming) api raw HTTP passthrough to any server endpoint (escape hatch) diff --git a/client/initialization.go b/client/initialization.go index cbbab475e..3e11578b3 100644 --- a/client/initialization.go +++ b/client/initialization.go @@ -8,7 +8,9 @@ import ( "time" ) -// InitializationConfig represents the initialization configuration for a knowledge base +// InitializationConfig is the WRITE payload for InitializeByKB / UpdateKBConfig +// (the server's write endpoint accepts these flat model ids). It is NOT the +// shape the read endpoint returns — see KBModelConfigView / GetInitializationConfig. type InitializationConfig struct { ChatModelID string `json:"chat_model_id,omitempty"` EmbeddingModelID string `json:"embedding_model_id,omitempty"` @@ -16,6 +18,40 @@ type InitializationConfig struct { MultimodalID string `json:"multimodal_id,omitempty"` } +// KBModelConfigView is the secret-free, read-only model configuration of a +// knowledge base, returned by GetInitializationConfig. The server's read +// response nests config under embedding/llm/rerank/multimodal and INCLUDES +// provider apiKey/baseUrl (for the web config form); this view intentionally +// parses only the non-secret fields, so credentials can never leak through the +// CLI. Field tags are snake_case (the CLI envelope convention), remapped from +// the server's camelCase. +type KBModelConfigView struct { + RetrievalReady bool `json:"retrieval_ready"` // embedding model bound → KB can embed/retrieve + Embedding ModelSlotView `json:"embedding"` + LLM ModelSlotView `json:"llm"` + Rerank RerankSlotView `json:"rerank"` + Multimodal MultimodalSlotView `json:"multimodal"` +} + +// ModelSlotView is one non-secret model slot (embedding / llm). +type ModelSlotView struct { + Configured bool `json:"configured"` + ModelName string `json:"model_name,omitempty"` + Source string `json:"source,omitempty"` + Dimension int `json:"dimension,omitempty"` +} + +// RerankSlotView is the rerank slot (may be disabled). +type RerankSlotView struct { + Enabled bool `json:"enabled"` + ModelName string `json:"model_name,omitempty"` +} + +// MultimodalSlotView reports whether multimodal processing is enabled. +type MultimodalSlotView struct { + Enabled bool `json:"enabled"` +} + // OllamaModelInfo represents info about an Ollama model type OllamaModelInfo struct { Name string `json:"name"` @@ -40,20 +76,50 @@ type ModelCheckResult struct { Message string `json:"message,omitempty"` } -// GetInitializationConfig gets the current initialization config for a knowledge base -func (c *Client) GetInitializationConfig(ctx context.Context, kbID string) (*InitializationConfig, error) { +// GetInitializationConfig returns a knowledge base's model configuration as a +// secret-free KBModelConfigView. The server response nests config under +// embedding/llm/rerank/multimodal and includes provider apiKey/baseUrl; this +// parses ONLY the non-secret fields (apiKey/baseUrl are never read into the +// struct, so they cannot leak through the CLI) and remaps to snake_case. +func (c *Client) GetInitializationConfig(ctx context.Context, kbID string) (*KBModelConfigView, error) { resp, err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/api/v1/initialization/config/%s", kbID), nil, nil) if err != nil { return nil, err } + // Deliberately model only non-secret fields; apiKey / baseUrl in the server + // payload are ignored by omission. var result struct { - Success bool `json:"success"` - Data *InitializationConfig `json:"data"` + Data struct { + Embedding struct { + Source string `json:"source"` + ModelName string `json:"modelName"` + Dimension int `json:"dimension"` + } `json:"embedding"` + LLM struct { + Source string `json:"source"` + ModelName string `json:"modelName"` + } `json:"llm"` + Rerank struct { + Enabled bool `json:"enabled"` + ModelName string `json:"modelName"` + } `json:"rerank"` + Multimodal struct { + Enabled bool `json:"enabled"` + } `json:"multimodal"` + } `json:"data"` } if err := parseResponse(resp, &result); err != nil { return nil, err } - return result.Data, nil + d := result.Data + view := &KBModelConfigView{ + RetrievalReady: d.Embedding.ModelName != "", + Embedding: ModelSlotView{Configured: d.Embedding.ModelName != "", ModelName: d.Embedding.ModelName, Source: d.Embedding.Source, Dimension: d.Embedding.Dimension}, + LLM: ModelSlotView{Configured: d.LLM.ModelName != "", ModelName: d.LLM.ModelName, Source: d.LLM.Source}, + Rerank: RerankSlotView{Enabled: d.Rerank.Enabled, ModelName: d.Rerank.ModelName}, + Multimodal: MultimodalSlotView{Enabled: d.Multimodal.Enabled}, + } + return view, nil } // InitializeByKB initializes a knowledge base with model configuration