diff --git a/cli/AGENTS.md b/cli/AGENTS.md index 3e3c9345..5925a8fd 100644 --- a/cli/AGENTS.md +++ b/cli/AGENTS.md @@ -56,7 +56,7 @@ Errors emit an error envelope on stderr (`--format json`) or prose "type": "auth.unauthenticated", "message": "fetch current user: HTTP error 401", "hint": "run `weknora auth login`", - "retry_command": "weknora auth login --host https://kb.example.com", + "retry_command": "weknora auth login", "retry_after_seconds": 0, "risk": {"level": "destructive", "action": "noun.verb"}, "detail": {} @@ -413,9 +413,6 @@ Agents parse the first colon to extract the typed code. The exit code class (see | `local.unimplemented` | 1 | no | (planned in a future release) | | `local.upload_file_not_found` | 1 | no | verify the path is correct and readable | | `local.user_aborted` | 1 | no (user said no) | no action taken; pass `-y/--yes` to skip the confirmation prompt | -| `mcp.readonly_mode` | 1 | no | MCP tool surface is read-only; mutations not exposed in this mode | -| `mcp.schema_unknown_command` | 1 | no | (no canonical hint) | -| `mcp.tool_not_allowed` | 1 | no | MCP tool not in the curated allowlist | @@ -538,7 +535,7 @@ The three surfaces do not auto-sync: each is wired separately so agents that onl ## MCP Tool Surface -WeKnora's MCP server exposes a curated read-only tool surface. Many MCP servers in the wild ship write / mutation operations on by default and rely on credential-scope or sandbox restrictions for safety. WeKnora opts for curation instead: the server side doesn't yet enforce per-token scope, so an agent holding a user's token has full write access. Until server-side scope ships, the CLI keeps mutation tools out of the MCP surface as a belt-and-braces second line of defense. When server scope arrives this stance can loosen. +WeKnora's MCP server exposes a curated 10-tool surface where most tools are read-only but `chat` and `session_ask` create conversation/message records. Many MCP servers in the wild ship write / mutation operations on by default and rely on credential-scope or sandbox restrictions for safety. WeKnora opts for curation instead: the server side doesn't yet enforce per-token scope, so an agent holding a user's token has full write access. Until server-side scope ships, the CLI keeps mutation tools out of the MCP surface as a belt-and-braces second line of defense. When server scope arrives this stance can loosen. The curated 10 tools (`cli/internal/mcp/tools.go`): @@ -553,7 +550,7 @@ The curated 10 tools (`cli/internal/mcp/tools.go`): | `search_chunks` | hybrid (vector + keyword) retrieval | | `chat` | stream a RAG answer; auto-creates a session if absent | | `agent_list` | list custom agents | -| `agent_invoke` | run a query through a custom agent | +| `session_ask` | run a query through a custom agent (`session ask --agent`) | Adding a tool is a deliberate API expansion — the AI-agent-callable surface is the reason this CLI ships an MCP server, not its CLI command list, so the registration list in `registerTools` is maintained by hand. diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index fe95ae3c..b6a82d5f 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -12,6 +12,30 @@ CLI history before v0.3 is recorded in the project root ## [Unreleased] +### v0.9 — auth/profile model harmonization + flag cleanup + +#### Added +- `weknora session stop ` command to abort an in-flight agent run. +- `profile add --use`: switch to the newly-added profile immediately (instead of only auto-selecting the first profile added). +- `-L` shorthand on `session view` (alias for `--limit`). +- `doc download --format json` now emits a success envelope (was bare). +- `SetAgentHelp` coverage extended across create / list / search commands. + +#### Changed +- `auth login` now authenticates the **active profile** (resolved from config / global `--profile`) instead of creating a profile. Re-login MERGES credential refs into the existing record — host and an existing user are preserved, never clobbered. + +#### Breaking +- **`--kb` now accepts a knowledge-base name *or* id** on `doc delete --all` and `search chunks` / `search docs`; it stays required (no silent project-link fallback on these commands). +- **`agent create --kb` renamed to `--attach-kb`** to disambiguate from the global `--kb` scope flag. +- **MCP tool `agent_invoke` renamed to `session_ask`** (clean rename — external MCP clients must update cached tool schemas). +- **`auth login` drops `--host` and `--name`.** It authenticates the active profile; create it first with `weknora profile add --host --use`. Target a non-active profile with the global `weknora --profile auth login`. +- **`auth logout` and `auth refresh` drop `--name`.** They act on the active profile; target another with the global `--profile `. + +#### Removed +- Dead MCP error codes `mcp.readonly_mode`, `mcp.tool_not_allowed`, and `mcp.schema_unknown_command` (never emitted by the current tool surface). + +--- + ### v0.8 — Agent safety nets + MCP annotations #### Added diff --git a/cli/README.md b/cli/README.md index 4b13f06b..213ae83f 100644 --- a/cli/README.md +++ b/cli/README.md @@ -58,11 +58,14 @@ release. Grab the latest from the [Releases page](https://github.com/Tencent/WeK ## 5-minute quickstart ```bash -# 1. Log in to your WeKnora server (interactive password prompt) -weknora auth login --host https://kb.example.com +# 1. Register your WeKnora server as a profile and make it active +weknora profile add prod --host https://kb.example.com --use -# 2. Or pipe an API key from stdin (for CI / AI agents) -echo "sk-..." | weknora auth login --host https://kb.example.com --with-token +# 2. Authenticate the active profile (interactive password prompt) +weknora auth login + +# 2b. Or pipe an API key from stdin (for CI / AI agents) +echo "sk-..." | weknora auth login --with-token # 3. List knowledge bases weknora kb list @@ -101,9 +104,10 @@ weknora agent check ag_abc # deep: probes every KB in the agent's scope For AI agents (Claude Code, Cursor, Gemini CLI, etc.) integrating WeKnora: 1. Install: `brew install weknora` or `go install github.com/Tencent/WeKnora/cli@latest` -2. Authenticate (background; extract login URL for the user): +2. Register a profile, then authenticate it (background; extract login URL for the user): ```bash - weknora auth login --host + weknora profile add prod --host --use + weknora auth login ``` 3. Register MCP in the host's MCP config: ```json @@ -114,18 +118,44 @@ For AI agents (Claude Code, Cursor, Gemini CLI, etc.) integrating WeKnora: 5. Read the [exit-10 anti-patterns](AGENTS.md#exit-10-anti-patterns) before any destructive call. +**Bundled Agent Skills.** This CLI ships [Agent Skills](https://agentskills.io/specification) +under [`skills/`](skills/) that teach an agent to drive WeKnora without trial and error: + +- [`weknora-shared`](skills/weknora-shared/SKILL.md) — **read first**: auth/profile + sequence, `--kb` resolution, the JSON-envelope + exit-code contract, the exit-10 + protocol, `--dry-run`, and CLI-vs-MCP selection. +- [`weknora-rag-search`](skills/weknora-rag-search/SKILL.md) — when to use `chat` + vs `session ask` vs `search chunks`, plus retrieval gotchas. + +MVP install: symlink them into your agent's skills directory (from a source checkout): + +```bash +ln -s "$PWD/skills/weknora-shared" ~/.claude/skills/weknora-shared +ln -s "$PWD/skills/weknora-rag-search" ~/.claude/skills/weknora-rag-search +``` + +Each skill's frontmatter records the CLI version it was `tested_against`; a CI +parity test (`internal/skillparity`) fails if a skill ever references a command, +flag, or MCP tool the CLI no longer has. (A `weknora skills install` command is +planned; for now, symlink or copy.) + --- ## Multi-profile -Switch between several WeKnora servers (or several tenants on the same server) -without re-logging in: +`profile.*` manages profile *records* (positional ``); `auth.*` operates +on the *active* profile (override per-invocation with the global `--profile` +flag). Create a profile first, then authenticate it: ```bash -weknora auth login --host https://prod.example.com --name prod -weknora auth login --host https://staging.example.com --name staging --with-token < .staging-key +weknora profile add prod --host https://prod.example.com --use # add + switch +weknora auth login # authenticate active (prod) + +weknora profile add staging --host https://staging.example.com # add (stays inactive) +echo "sk-..." | weknora --profile staging auth login --with-token # authenticate staging + weknora auth list -weknora profile use prod +weknora profile use prod # switch back ``` Credentials are persisted to your OS keyring (Keychain on macOS, libsecret on @@ -136,8 +166,8 @@ under `$XDG_CONFIG_HOME/weknora/secrets/`. The active profile lives in To remove a profile's stored credentials: ```bash -weknora auth logout # current profile -weknora auth logout --name staging # specific +weknora auth logout # active profile +weknora --profile staging auth logout # specific profile weknora auth logout --all ``` @@ -303,8 +333,8 @@ The `weknora session continue-stream` command resumes an SSE event stream for an ```bash # Original streaming call captures session_id + message_id from init event: -weknora session ask "..." --kb kb_xxxx --format ndjson | tee /tmp/stream.ndjson -# {"event":"init","session_id":"sess_abc","message_id":"msg_xyz"} +weknora session ask "..." --agent ag_xxxx --format ndjson | tee /tmp/stream.ndjson +# {"type":"init","session_id":"sess_abc","message_id":"msg_xyz"} # ... events flow ... # [network blip] diff --git a/cli/acceptance/contract/errorcodes_test.go b/cli/acceptance/contract/errorcodes_test.go index b80ab640..26ae553a 100644 --- a/cli/acceptance/contract/errorcodes_test.go +++ b/cli/acceptance/contract/errorcodes_test.go @@ -203,12 +203,6 @@ func identToErrorCode(name string) (cmdutil.ErrorCode, bool) { return cmdutil.CodeSSEStreamAborted, true case "CodeSessionCreateFailed": return cmdutil.CodeSessionCreateFailed, true - case "CodeMCPReadonlyMode": - return cmdutil.CodeMCPReadonlyMode, true - case "CodeMCPToolNotAllowed": - return cmdutil.CodeMCPToolNotAllowed, true - case "CodeMCPSchemaUnknown": - return cmdutil.CodeMCPSchemaUnknown, true case "CodeOperationTimeout": return cmdutil.CodeOperationTimeout, true case "CodeOperationFailed": diff --git a/cli/cmd/agenthelp_coverage_test.go b/cli/cmd/agenthelp_coverage_test.go new file mode 100644 index 00000000..9c621076 --- /dev/null +++ b/cli/cmd/agenthelp_coverage_test.go @@ -0,0 +1,45 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "sort" + "testing" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" +) + +// TestEveryLeafCommandHasAgentHelp enforces the agent-first contract: every +// leaf (runnable, no subcommands) command must emit a structured AgentHelp JSON +// blob under WEKNORA_AGENT_HELP=1, so an agent never has to scrape human prose. +// Drift guard in the spirit of the K6/K7 skill-parity tests: a new leaf command +// added without SetAgentHelp fails CI here. +// +// Exemptions: cobra's generated `completion`/`help` subtrees carry no +// domain semantics worth a machine blob. +func TestEveryLeafCommandHasAgentHelp(t *testing.T) { + t.Setenv("WEKNORA_AGENT_HELP", "1") + root := NewRootCmd(cmdutil.New()) + + var missing []string + eachLeafCommand(root, func(c *cobra.Command) { + // Invoke the leaf's help func with the agent env set and require JSON. + var buf bytes.Buffer + c.SetOut(&buf) + c.Help() + var ah struct { + UsedFor string `json:"used_for"` + } + if err := json.Unmarshal(buf.Bytes(), &ah); err != nil || ah.UsedFor == "" { + missing = append(missing, c.CommandPath()) + } + }) + + sort.Strings(missing) + if len(missing) > 0 { + t.Errorf("leaf commands missing agent-help JSON (register cmdutil.SetAgentHelp):\n %v\n(%d commands)", + missing, len(missing)) + } +} diff --git a/cli/cmd/coverage_helpers_test.go b/cli/cmd/coverage_helpers_test.go new file mode 100644 index 00000000..105d3758 --- /dev/null +++ b/cli/cmd/coverage_helpers_test.go @@ -0,0 +1,28 @@ +package cmd + +import "github.com/spf13/cobra" + +// eachLeafCommand invokes fn for every runnable leaf command (no real +// subcommands) in the tree rooted at root, skipping cobra's generated +// `completion`/`help` subtrees and hidden commands. Shared by the agent-help +// and dry-run coverage drift guards so they agree, byte-for-byte, on what +// counts as a leaf. +func eachLeafCommand(root *cobra.Command, fn func(*cobra.Command)) { + exempt := map[string]bool{"completion": true, "help": true} + var walk func(c *cobra.Command) + walk = func(c *cobra.Command) { + realKids := 0 + for _, k := range c.Commands() { + if exempt[k.Name()] || k.Hidden { + continue + } + realKids++ + walk(k) + } + if c == root || realKids > 0 || exempt[c.Name()] { + return // group command or exempt — not a leaf + } + fn(c) + } + walk(root) +} diff --git a/cli/cmd/dryrun_coverage_test.go b/cli/cmd/dryrun_coverage_test.go new file mode 100644 index 00000000..29b53616 --- /dev/null +++ b/cli/cmd/dryrun_coverage_test.go @@ -0,0 +1,81 @@ +package cmd + +import ( + "sort" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" +) + +// dryRunExpectation declares, for every leaf command, whether it must register +// --dry-run. The rule (per internal/cmdutil/dryrun.go): commands that change +// server/local state get a preview path. Reads, generate/stream ops, the +// interactive credential flow, and the long-running MCP server are exempt. +// +// This is a drift guard (sibling of TestEveryLeafCommandHasAgentHelp): every +// leaf must appear here, so a newly-added command forces an explicit dry-run +// decision rather than silently inheriting "no preview". +var dryRunExpectation = map[string]bool{ + // --- mutations: MUST have --dry-run --- + "kb create": true, "kb edit": true, "kb delete": true, "kb pin": true, "kb unpin": true, + "doc create": true, "doc upload": true, "doc fetch": true, "doc delete": true, + "chunk delete": true, + "session delete": true, "session stop": true, + "agent create": true, "agent edit": true, "agent delete": true, + "profile add": true, "profile use": true, "profile remove": true, + "auth logout": true, "auth refresh": true, + "link": true, "unlink": true, + "api": true, // passthrough: dry-run previews write methods, rejected on GET + + // --- exempt: no state change to preview --- + // reads + "kb list": false, "kb view": false, "kb status": false, "kb check": false, + "doc list": false, "doc view": false, "doc download": false, + "doc wait": false, // polling read, no mutation + "chunk list": false, "chunk view": false, + "session list": false, "session view": false, + "agent list": false, "agent view": false, "agent status": false, "agent check": false, + "search chunks": false, "search docs": false, "search kb": false, "search sessions": false, + "auth list": false, "auth status": false, "auth token": false, + "profile list": false, + "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, + // 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, +} + +func TestDryRunCoverageMatchesExpectation(t *testing.T) { + root := NewRootCmd(cmdutil.New()) + + var unlisted, wrong []string + eachLeafCommand(root, func(c *cobra.Command) { + key := strings.TrimPrefix(c.CommandPath(), "weknora ") + want, declared := dryRunExpectation[key] + if !declared { + unlisted = append(unlisted, key) + return + } + has := c.Flags().Lookup("dry-run") != nil + if has != want { + wrong = append(wrong, key) + } + }) + + sort.Strings(unlisted) + sort.Strings(wrong) + if len(unlisted) > 0 { + t.Errorf("leaf commands missing from dryRunExpectation (declare must-have-dry-run true/false):\n %v", unlisted) + } + if len(wrong) > 0 { + t.Errorf("dry-run flag presence disagrees with dryRunExpectation:\n %v", wrong) + } +} diff --git a/cli/cmd/mcp/mcp.go b/cli/cmd/mcp/mcp.go index a2f3992b..26eefac4 100644 --- a/cli/cmd/mcp/mcp.go +++ b/cli/cmd/mcp/mcp.go @@ -2,10 +2,11 @@ // // MCP (Model Context Protocol; https://spec.modelcontextprotocol.io/) is // the JSON-RPC 2.0 wire protocol agentic IDEs use to call external tools. -// `weknora mcp serve` exposes a curated subset of the CLI's read surface -// as MCP tools so an IDE-side agent can list / view / search / chat against -// the user's active WeKnora profile without shelling out to the CLI per -// call. +// `weknora mcp serve` exposes a curated subset of the CLI as MCP tools so +// an IDE-side agent can list / view / search / chat against the user's +// active WeKnora profile without shelling out to the CLI per call. Most +// tools are read-only; chat and session_ask create conversation/message +// records. // // Package name is `mcpcmd` to avoid shadowing `cli/internal/mcp` (the // transport-and-handlers implementation). Same naming hygiene as @@ -23,14 +24,15 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "mcp", Short: "Run weknora as a Model Context Protocol server", - Long: `Exposes weknora's read surface as MCP tools so any + Long: `Exposes weknora's tool surface as MCP tools so any MCP-compatible client can call them over JSON-RPC. -Initial tool surface is read-only and curated: kb_list / kb_view / -doc_list / doc_view / doc_download / search_chunks / chat / agent_list / -agent_invoke. Destructive verbs (create / delete / upload) are deliberately -excluded - the agent should ask the user before mutating; the CLI's -exit-10 protocol covers that path.`, +Curated 10-tool surface: kb_list / kb_view / doc_list / doc_view / +doc_download / search_chunks / chunk_list / agent_list are read-only; +chat and session_ask create conversation/message records. Destructive +verbs (create / delete / upload) are deliberately excluded - the agent +should ask the user before mutating; the CLI's exit-10 protocol covers +that path.`, Args: cobra.NoArgs, Run: func(c *cobra.Command, _ []string) { _ = c.Help() }, } diff --git a/cli/cmd/mcp/serve.go b/cli/cmd/mcp/serve.go index e507806d..3c6246bf 100644 --- a/cli/cmd/mcp/serve.go +++ b/cli/cmd/mcp/serve.go @@ -46,5 +46,13 @@ Consult your MCP client's documentation for the exact config-file location.`, return mcpserver.RunStdio(c.Context(), cli) }, } + 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", + 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", + }, + }) return cmd } diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 0242c7ce..1b22c872 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -53,15 +53,15 @@ func resolveFormatEarly(args []string) { } } switch mode { - case "json", "ndjson", "text": + case "ndjson", "text": cmdutil.SetFormatMode(mode) - case "": - // nothing to set; leave globalFormatMode at its zero value default: - // Invalid format value: promote to json so the subsequent - // rejection error (from CheckFormatFlag) still emits as an envelope - // rather than prose. The real validation error will still fire. - cmdutil.SetFormatMode("json") + // "json", "" (no flag/env), or an invalid value all route the cobra-side + // error through the JSON envelope. Cobra parse errors fire before + // PersistentPreRunE runs ResolveDefault, so we apply the same default + // here (DefaultFormatMode) — otherwise the error path would emit prose + // while the success path emits the envelope. + cmdutil.SetFormatMode(string(cmdutil.DefaultFormatMode)) } } @@ -135,7 +135,8 @@ func NewRootCmd(f *cmdutil.Factory) *cobra.Command { Long: `Command-line client for the WeKnora RAG server. Manage knowledge bases and documents, run hybrid search, chat with grounded answers, or expose a curated read-only MCP tool surface for AI agents.`, - Example: ` weknora auth login --host=https://kb.example.com + Example: ` weknora profile add prod --host=https://kb.example.com --use + weknora auth login weknora kb list weknora chat "summarise the design doc" weknora doctor --format json`, @@ -253,6 +254,11 @@ func newVersionCmd(f *cmdutil.Factory) *cobra.Command { }, } cmdutil.AddFormatFlag(cmd, versionFields...) + cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ + UsedFor: "show CLI build metadata (version, commit, build date)", + Examples: []string{"weknora version --format json"}, + Output: "envelope.data is {version, commit, date}", + }) return cmd } diff --git a/cli/cmd/root_test.go b/cli/cmd/root_test.go index fe55c66a..bb525315 100644 --- a/cli/cmd/root_test.go +++ b/cli/cmd/root_test.go @@ -161,3 +161,31 @@ func TestRoot_ProfileFlagPropagation(t *testing.T) { }) } } + +// resolveFormatEarly must default to the JSON envelope when no --format/env is +// given, so cobra-side errors (unknown flag, arg-count) — which fire before +// PersistentPreRunE runs ResolveDefault — emit a machine-readable envelope on +// stderr, not bare prose. Regression for the success-path-defaults-to-json / +// error-path-defaults-to-prose asymmetry. +func TestResolveFormatEarly_DefaultsToEnvelope(t *testing.T) { + cases := []struct { + name string + args []string + want bool // true ⇒ expect JSON envelope + }{ + {"no format flag (default)", []string{"kb", "view"}, true}, + {"explicit --format json", []string{"kb", "view", "--format", "json"}, true}, + {"explicit --format text", []string{"kb", "view", "--format", "text"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resolveFormatEarly(tc.args) + err := MapCobraError(errors.New("accepts 1 arg(s), received 0")) + var buf bytes.Buffer + cmdutil.PrintError(&buf, err) + isEnvelope := strings.HasPrefix(strings.TrimSpace(buf.String()), "{") + assert.Equal(t, tc.want, isEnvelope, + "args=%v: got %q", tc.args, buf.String()) + }) + } +} diff --git a/cli/cmd/search/chunks.go b/cli/cmd/search/chunks.go index 22a42f43..2636b4ac 100644 --- a/cli/cmd/search/chunks.go +++ b/cli/cmd/search/chunks.go @@ -70,23 +70,27 @@ func NewCmdChunks(f *cmdutil.Factory) *cobra.Command { return err } fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) - cli, err := f.Client() - if err != nil { - return err - } - kbID, err := cmdutil.ResolveKBFlag(c.Context(), cli, opts.KB) + // Resolve KB via the shared flag→env→project-link chain (same as + // `doc list` / `chat`), so a linked directory or WEKNORA_KB_ID + // works without an explicit --kb. Resolve before building the + // client so an unresolved KB short-circuits to local.kb_id_required + // without a client round-trip. + kbID, err := f.ResolveKB(c) if err != nil { return err } opts.KBID = kbID + cli, err := f.Client() + if err != nil { + return err + } return runChunks(c.Context(), opts, fopts, cli) }, } bindChunksFlags(cmd, opts) - _ = cmd.MarkFlagRequired("kb") cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "Hybrid (vector + keyword) chunk retrieval against a knowledge base. Results come with meta.count; use --limit to cap (default 8, tuned for RAG context). Pass --no-vector or --no-keyword to disable one channel.", - RequiredFlags: []string{" (positional)", "--kb"}, + UsedFor: "Hybrid (vector + keyword) chunk retrieval against a knowledge base. 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 (default 8, tuned for RAG context). Pass --no-vector or --no-keyword to disable one channel.", + RequiredFlags: []string{" (positional)", "--kb (or WEKNORA_KB_ID / linked directory)"}, Examples: []string{`weknora search chunks "what is RAG?" --kb engineering --format json`}, Output: "envelope.data is an array of SearchResult objects with id, content, score, knowledge_id; meta.count is the returned count; meta.has_more=true if more matched than --limit", }) @@ -94,9 +98,10 @@ func NewCmdChunks(f *cmdutil.Factory) *cobra.Command { } // bindChunksFlags registers the chunks flag surface in one place to keep -// the constructor readable; --kb is marked required by the caller. +// the constructor readable. --kb is optional: when omitted it falls back to +// WEKNORA_KB_ID / the linked directory via Factory.ResolveKB. func bindChunksFlags(cmd *cobra.Command, opts *ChunksOptions) { - cmd.Flags().StringVar(&opts.KB, "kb", "", "Knowledge base UUID or name") + cmd.Flags().StringVar(&opts.KB, "kb", "", "Knowledge base UUID or name (overrides env / project link)") cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 8, "Maximum results to return (default 8 - tuned for RAG context window; list commands default to 30)") cmd.Flags().Float64Var(&opts.VectorThreshold, "vector-threshold", 0, "Vector retrieval similarity floor (per-channel, pre-fusion); 0 = no filter") cmd.Flags().Float64Var(&opts.KeywordThreshold, "keyword-threshold", 0, "Keyword retrieval score floor (per-channel, pre-fusion); 0 = no filter") diff --git a/cli/cmd/search/chunks_test.go b/cli/cmd/search/chunks_test.go index cf5c01ed..5b708e40 100644 --- a/cli/cmd/search/chunks_test.go +++ b/cli/cmd/search/chunks_test.go @@ -137,21 +137,53 @@ func TestRunSearch_NilService(t *testing.T) { assert.Contains(t, err.Error(), "server.error") } -// TestNewCmdChunks_RequiresKB asserts that running `search chunks ""` -// without --kb returns an error containing `required flag(s) "kb"`. -// This locks the behavior so a later refactor cannot silently introduce a -// project-link fallback for this destructive-free read path. -func TestNewCmdChunks_RequiresKB(t *testing.T) { +// TestNewCmdChunks_NoKBUsesResolver asserts that `search chunks ""` +// without --kb no longer fails with cobra's `required flag(s) "kb"`. The +// command now resolves the KB through the shared flag→env→project-link chain +// (cmdutil.Factory.ResolveKB), matching `doc list` / `chat`; when nothing +// resolves it reports the typed local.kb_id_required (exit 1), not a cobra +// usage error. This is the inverse of the old lock test, which deliberately +// blocked the link fallback for this read path — the asymmetry with `doc +// list` was not worth keeping (a non-destructive search is the same risk +// profile as a list). The destructive `doc delete --all` keeps its explicit +// --kb rule; that safety guard is unrelated to this path. +func TestNewCmdChunks_NoKBUsesResolver(t *testing.T) { iostreams.SetForTest(t) + t.Setenv("WEKNORA_KB_ID", "") // no ambient KB from env + t.Chdir(t.TempDir()) // no .weknora project link discoverable cmd := NewCmdChunks(&cmdutil.Factory{ - Client: func() (*sdk.Client, error) { return nil, nil }, + // Resolution reaches local.kb_id_required before any client is built, + // so this must never be invoked; make it loud if it is. + Client: func() (*sdk.Client, error) { return nil, errors.New("client should not be built") }, }) cmd.SetArgs([]string{"some query"}) // query but no --kb cmd.SilenceErrors = true cmd.SilenceUsage = true err := cmd.Execute() require.Error(t, err) - assert.Contains(t, err.Error(), `required flag(s) "kb"`) + assert.NotContains(t, err.Error(), `required flag(s) "kb"`) + typed := cmdutil.AsError(err) + require.NotNil(t, typed) + assert.Equal(t, cmdutil.CodeKBIDRequired, typed.Code) +} + +// TestNewCmdChunks_HonorsKBEnv proves the env fallback is wired: with +// WEKNORA_KB_ID set and no --kb, KB resolution succeeds (no kb-required +// error) and the command proceeds to the client step — which here errors, +// confirming we got past resolution using the env value alone. +func TestNewCmdChunks_HonorsKBEnv(t *testing.T) { + iostreams.SetForTest(t) + t.Setenv("WEKNORA_KB_ID", "kb_from_env") + cmd := NewCmdChunks(&cmdutil.Factory{ + Client: func() (*sdk.Client, error) { return nil, errors.New("client boom") }, + }) + cmd.SetArgs([]string{"some query"}) // no --kb; env supplies it + cmd.SilenceErrors = true + cmd.SilenceUsage = true + err := cmd.Execute() + require.Error(t, err) + assert.NotContains(t, err.Error(), "kb is required") + assert.Contains(t, err.Error(), "client boom") } func TestNewCmdChunks_RequiresQuery(t *testing.T) { diff --git a/cli/cmd/search/docs.go b/cli/cmd/search/docs.go index f7b49948..26065308 100644 --- a/cli/cmd/search/docs.go +++ b/cli/cmd/search/docs.go @@ -97,27 +97,31 @@ reached or the KB is exhausted. Pass --all-pages=false to stop after one page.`, return err } fopts.ResolveDefault(iostreams.IO.IsStdoutTTY()) - cli, err := f.Client() - if err != nil { - return err - } - kbID, err := cmdutil.ResolveKBFlag(c.Context(), cli, opts.KB) + // Resolve KB via the shared flag→env→project-link chain (same as + // `doc list` / `chat`), so a linked directory or WEKNORA_KB_ID + // works without an explicit --kb. Resolve before building the + // client so an unresolved KB short-circuits to local.kb_id_required + // without a client round-trip. + kbID, err := f.ResolveKB(c) if err != nil { return err } opts.KBID = kbID + cli, err := f.Client() + if err != nil { + return err + } return runDocsSearch(c.Context(), opts, fopts, cli) }, } - cmd.Flags().StringVar(&opts.KB, "kb", "", "Knowledge base UUID or name (required)") + cmd.Flags().StringVar(&opts.KB, "kb", "", "Knowledge base UUID or name (overrides env / project link)") cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum results to return") cmd.Flags().IntVar(&opts.PageSize, "page-size", docsPageSize, "Items per server batch (1..1000)") cmd.Flags().BoolVar(&opts.AllPages, "all-pages", true, "Walk every server page until exhausted or --limit hit") cmdutil.AddFormatFlag(cmd, docsFields...) - _ = cmd.MarkFlagRequired("kb") cmdutil.SetAgentHelp(cmd, cmdutil.AgentHelp{ - UsedFor: "Find documents in a knowledge base by keyword (server-side LIKE filter on title/file_name). Results come with meta.count; use --limit to cap and --all-pages=false to stop after one page.", - RequiredFlags: []string{" (positional)", "--kb"}, + 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", }) diff --git a/cli/cmd/search/docs_test.go b/cli/cmd/search/docs_test.go index 78e49f5b..e0986629 100644 --- a/cli/cmd/search/docs_test.go +++ b/cli/cmd/search/docs_test.go @@ -237,3 +237,41 @@ func TestDocsSearch_HasMore(t *testing.T) { assert.False(t, m.HasMore, "has_more must be false/absent when results fit under --limit") }) } + +// TestNewCmdDocs_NoKBUsesResolver mirrors the chunks guard: `search docs` +// without --kb resolves the KB through the shared flag→env→project-link +// chain (Factory.ResolveKB), not cobra's required-flag check. With nothing +// to resolve it reports the typed local.kb_id_required, not a usage error. +func TestNewCmdDocs_NoKBUsesResolver(t *testing.T) { + iostreams.SetForTest(t) + t.Setenv("WEKNORA_KB_ID", "") + t.Chdir(t.TempDir()) + cmd := NewCmdDocs(&cmdutil.Factory{ + Client: func() (*sdk.Client, error) { return nil, errors.New("client should not be built") }, + }) + cmd.SetArgs([]string{"some query"}) // query but no --kb + cmd.SilenceErrors = true + cmd.SilenceUsage = true + err := cmd.Execute() + require.Error(t, err) + assert.NotContains(t, err.Error(), `required flag(s) "kb"`) + typed := cmdutil.AsError(err) + require.NotNil(t, typed) + assert.Equal(t, cmdutil.CodeKBIDRequired, typed.Code) +} + +// TestNewCmdDocs_HonorsKBEnv proves the env fallback is wired for search docs. +func TestNewCmdDocs_HonorsKBEnv(t *testing.T) { + iostreams.SetForTest(t) + t.Setenv("WEKNORA_KB_ID", "kb_from_env") + cmd := NewCmdDocs(&cmdutil.Factory{ + Client: func() (*sdk.Client, error) { return nil, errors.New("client boom") }, + }) + cmd.SetArgs([]string{"some query"}) // no --kb; env supplies it + cmd.SilenceErrors = true + cmd.SilenceUsage = true + err := cmd.Execute() + require.Error(t, err) + assert.NotContains(t, err.Error(), "kb is required") + assert.Contains(t, err.Error(), "client boom") +} diff --git a/cli/internal/cmdutil/confirm.go b/cli/internal/cmdutil/confirm.go index 8372ad2a..6e8a1d53 100644 --- a/cli/internal/cmdutil/confirm.go +++ b/cli/internal/cmdutil/confirm.go @@ -2,11 +2,35 @@ package cmdutil import ( "fmt" + "unicode" + "unicode/utf8" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" ) +// confirmCaveat returns the trailing safety note shown after the interactive +// prompt, tailored to the verb. Deletes/removals are irreversible; edits +// overwrite prior values but are recoverable if you know the old value. +func confirmCaveat(verb string) string { + switch verb { + case "edit": + return "This overwrites the current values." + default: // delete / remove — irreversible + return "This cannot be undone." + } +} + +// titleFirst upper-cases the first rune so the interactive prompt reads as a +// sentence ("Delete …?" / "Edit …?") without pulling in golang.org/x/text. +func titleFirst(s string) string { + if s == "" { + return s + } + r, size := utf8.DecodeRuneInString(s) + return string(unicode.ToUpper(r)) + s[size:] +} + // ConfirmDestructiveBatch is the multi-id flavor of ConfirmDestructive: same // behavior matrix (yes / non-TTY / TTY-prompt / user-no) but the prompt text // reflects the count, not a single id. Used by `doc delete [...]` @@ -16,19 +40,19 @@ import ( // action is the namespaced action verb (e.g. "doc.delete") for the risk envelope. // retryCmd is the directly-executable retry argv (e.g. "weknora doc delete a b -y"); // pass "" when no clean retry argv is available. -func ConfirmDestructiveBatch(p prompt.Prompter, yes, jsonOut bool, what string, n int, action, retryCmd string) error { +func ConfirmDestructiveBatch(p prompt.Prompter, yes, jsonOut bool, verb, what string, n int, action, retryCmd string) error { if yes { return nil } if !iostreams.IO.IsStdoutTTY() || jsonOut { return NewError( CodeInputConfirmationRequired, - fmt.Sprintf("delete %d %s(s) requires explicit confirmation: re-run with -y/--yes", n, what), + fmt.Sprintf("%s %d %s(s) requires explicit confirmation: re-run with -y/--yes", verb, n, what), ). WithRetryCommand(retryCmd). WithRisk("destructive", action) } - ok, err := p.Confirm(fmt.Sprintf("Delete %d %s(s)? This cannot be undone.", n, what), false) + ok, err := p.Confirm(fmt.Sprintf("%s %d %s(s)? %s", titleFirst(verb), n, what, confirmCaveat(verb)), false) if err != nil { return Wrapf(CodeInputMissingFlag, err, "confirm batch delete") } @@ -59,19 +83,19 @@ func ConfirmDestructiveBatch(p prompt.Prompter, yes, jsonOut bool, what string, // action is the namespaced action verb (e.g. "kb.delete") for the risk envelope. // retryCmd is the directly-executable retry argv (e.g. "weknora kb delete kb_x -y"); // pass "" when no clean retry argv is available. -func ConfirmDestructive(p prompt.Prompter, yes, jsonOut bool, what, id, action, retryCmd string) error { +func ConfirmDestructive(p prompt.Prompter, yes, jsonOut bool, verb, what, id, action, retryCmd string) error { if yes { return nil } if !iostreams.IO.IsStdoutTTY() || jsonOut { return NewError( CodeInputConfirmationRequired, - fmt.Sprintf("delete %s %s requires explicit confirmation: re-run with -y/--yes", what, id), + fmt.Sprintf("%s %s %s requires explicit confirmation: re-run with -y/--yes", verb, what, id), ). WithRetryCommand(retryCmd). WithRisk("destructive", action) } - ok, err := p.Confirm(fmt.Sprintf("Delete %s %s? This cannot be undone.", what, id), false) + ok, err := p.Confirm(fmt.Sprintf("%s %s %s? %s", titleFirst(verb), what, id, confirmCaveat(verb)), false) if err != nil { return Wrapf(CodeInputMissingFlag, err, "confirm delete") } diff --git a/cli/internal/cmdutil/confirm_test.go b/cli/internal/cmdutil/confirm_test.go new file mode 100644 index 00000000..9ff66110 --- /dev/null +++ b/cli/internal/cmdutil/confirm_test.go @@ -0,0 +1,55 @@ +package cmdutil_test + +import ( + "strings" + "testing" + + "github.com/Tencent/WeKnora/cli/internal/cmdutil" + "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/testutil" +) + +// The confirmation message verb must match the actual operation: an `edit` +// must not be described as `delete`. Regression for the hardcoded-"delete" +// confirmation message that mislabeled kb/agent edits. +func TestConfirmDestructive_VerbMatchesOperation(t *testing.T) { + iostreams.SetForTest(t) // non-TTY buffers ⇒ the jsonOut/non-TTY branch + + cases := []struct { + verb, what, id string + wantPrefix string + wantNotContains string + }{ + {"edit", "knowledge base", "kb_x", "edit knowledge base kb_x requires", "delete"}, + {"delete", "knowledge base", "kb_x", "delete knowledge base kb_x requires", ""}, + {"remove", "current profile", "prod", "remove current profile prod requires", "delete"}, + } + for _, tc := range cases { + err := cmdutil.ConfirmDestructive(&testutil.ConfirmPrompter{}, false, true, tc.verb, tc.what, tc.id, tc.what+"."+tc.verb, "") + if err == nil { + t.Fatalf("verb %q: expected confirmation_required error", tc.verb) + } + msg := err.Error() + if !strings.Contains(msg, tc.wantPrefix) { + t.Errorf("verb %q: message %q does not contain %q", tc.verb, msg, tc.wantPrefix) + } + if tc.wantNotContains != "" && strings.Contains(msg, tc.wantNotContains) { + t.Errorf("verb %q: message %q must not contain %q", tc.verb, msg, tc.wantNotContains) + } + if typed := cmdutil.AsError(err); typed == nil || typed.Code != cmdutil.CodeInputConfirmationRequired { + t.Errorf("verb %q: expected CodeInputConfirmationRequired, got %v", tc.verb, err) + } + } +} + +// The batch flavor must likewise honor the verb. +func TestConfirmDestructiveBatch_VerbMatchesOperation(t *testing.T) { + iostreams.SetForTest(t) + err := cmdutil.ConfirmDestructiveBatch(&testutil.ConfirmPrompter{}, false, true, "delete", "document", 3, "doc.delete", "") + if err == nil { + t.Fatal("expected confirmation_required error") + } + if !strings.Contains(err.Error(), "delete 3 document(s) requires") { + t.Errorf("unexpected batch message: %q", err.Error()) + } +} diff --git a/cli/internal/cmdutil/errors.go b/cli/internal/cmdutil/errors.go index ce06bb78..8384132e 100644 --- a/cli/internal/cmdutil/errors.go +++ b/cli/internal/cmdutil/errors.go @@ -100,11 +100,6 @@ const ( // from network.error (pre-stream transport failure) so users see the // stream specifically aborted, not a connection that never opened. CodeSSEStreamAborted ErrorCode = "local.sse_stream_aborted" - - // mcp.* - CodeMCPReadonlyMode ErrorCode = "mcp.readonly_mode" - CodeMCPToolNotAllowed ErrorCode = "mcp.tool_not_allowed" - CodeMCPSchemaUnknown ErrorCode = "mcp.schema_unknown_command" ) // Error is the typed error implementations carry through the call stack. @@ -178,6 +173,15 @@ func (e *Error) WithRisk(level, action string) *Error { return e } +// WithSilent suppresses PrintError's stderr envelope while preserving the Code +// for ExitCode. Use when the command already emitted a structured outcome +// envelope to stdout (e.g. batch / wait partitions) and a second error envelope +// on stderr would contradict it. Mirrors RunBatch's Silent behavior. +func (e *Error) WithSilent() *Error { + e.Silent = true + return e +} + // AsError unwraps to *Error if the chain contains one. Returns nil if not found. func AsError(err error) *Error { var typed *Error @@ -419,8 +423,6 @@ func AllCodes() []ErrorCode { CodeSSEStreamAborted, CodeSessionCreateFailed, // operation CodeOperationTimeout, CodeOperationFailed, CodeOperationCancelled, - // mcp - CodeMCPReadonlyMode, CodeMCPToolNotAllowed, CodeMCPSchemaUnknown, } } diff --git a/cli/internal/cmdutil/errors_retry_test.go b/cli/internal/cmdutil/errors_retry_test.go index f6bc773f..40b34e22 100644 --- a/cli/internal/cmdutil/errors_retry_test.go +++ b/cli/internal/cmdutil/errors_retry_test.go @@ -13,9 +13,9 @@ func TestErrorToDetail_NilSafe(t *testing.T) { func TestError_WithRetryCommand(t *testing.T) { err := NewError(CodeAuthUnauthenticated, "session expired"). WithHint("run `weknora auth login`"). - WithRetryCommand("weknora auth login --host https://kb.example.com") + WithRetryCommand("weknora auth login") - if err.RetryCommand != "weknora auth login --host https://kb.example.com" { + if err.RetryCommand != "weknora auth login" { t.Errorf("RetryCommand not set; got %q", err.RetryCommand) } if err.Hint != "run `weknora auth login`" { diff --git a/cli/internal/cmdutil/format.go b/cli/internal/cmdutil/format.go index 5129e0c2..b97ff577 100644 --- a/cli/internal/cmdutil/format.go +++ b/cli/internal/cmdutil/format.go @@ -23,6 +23,12 @@ const ( FormatNDJSON FormatMode = "ndjson" ) +// DefaultFormatMode is the mode used when neither --format nor WEKNORA_FORMAT +// is set. Single source of truth shared by FormatOptions.ResolveDefault and +// cmd.resolveFormatEarly (the early cobra-parse-error path) so the two cannot +// drift on what "no flag" defaults to. +const DefaultFormatMode = FormatJSON + // FormatOptions captures the resolved --format + --jq state for a command. // Mode is one of FormatText / FormatJSON / FormatNDJSON, or "" before // ResolveDefault has been called. @@ -106,12 +112,12 @@ func (o *FormatOptions) Emit(w io.Writer, data any, meta *output.Meta) error { switch o.Mode { case FormatJSON: if o.JQ != "" { - return format.WriteJSONFiltered(w, output.NewEnvelope(data, meta, globalProfile), nil, o.JQ) + return mapJQError(format.WriteJSONFiltered(w, output.NewEnvelope(data, meta, globalProfile), nil, o.JQ)) } return output.WriteEnvelope(w, data, meta, o.TTY, globalProfile) case FormatNDJSON: if o.JQ != "" { - return format.WriteJSONFiltered(w, data, nil, o.JQ) + return mapJQError(format.WriteJSONFiltered(w, data, nil, o.JQ)) } return format.WriteNDJSON(w, data) case FormatText: @@ -121,6 +127,22 @@ func (o *FormatOptions) Emit(w io.Writer, data any, meta *output.Meta) error { } } +// mapJQError converts a failure rooted in the user-supplied --jq expression +// (format.JQError) into a typed input.invalid_argument error (exit 5) so agents +// fix the expression instead of treating a bad --jq as a CLI bug. Non-jq errors +// (e.g. an internal serialization fault) pass through unchanged. +func mapJQError(err error) error { + if err == nil { + return nil + } + var jqe *format.JQError + if errors.As(err, &jqe) { + return NewError(CodeInputInvalidArgument, jqe.Error()). + WithHint("invalid --jq expression; see https://jqlang.github.io/jq/manual/") + } + return err +} + // ResolveDefault fills in Mode when the caller has not explicitly set it: // - Mode defaults to FormatJSON // - TTY only affects the indent decision (auto-indent in TTY; compact in pipe) @@ -128,7 +150,7 @@ func (o *FormatOptions) Emit(w io.Writer, data any, meta *output.Meta) error { func (o *FormatOptions) ResolveDefault(tty bool) { o.TTY = tty if o.Mode == "" { - o.Mode = FormatJSON + o.Mode = DefaultFormatMode } } diff --git a/cli/internal/format/filter.go b/cli/internal/format/filter.go index 7bde5150..1a70b7e7 100644 --- a/cli/internal/format/filter.go +++ b/cli/internal/format/filter.go @@ -46,17 +46,27 @@ func filterObjectKeys(objRaw json.RawMessage, fields []string) (json.RawMessage, return json.Marshal(dst) } +// JQError marks a failure attributable to the user-supplied --jq expression +// (parse or evaluation), as opposed to an internal serialization fault. The +// cmdutil layer (which can't be imported here without a cycle) maps it to +// input.invalid_argument so agents fix the expression rather than treating a +// bad --jq as a CLI bug (internal.error / exit 1). +type JQError struct{ Err error } + +func (e *JQError) Error() string { return e.Err.Error() } +func (e *JQError) Unwrap() error { return e.Err } + // writeJQ evaluates expr against raw and writes each result line by line to w. // String results render without quotes (so `--jq '.name'` yields shell-friendly // bare strings); non-string results use encoding/json. // -// Returns input.invalid_argument-shaped errors via plain errors.New + fmt; -// the caller is responsible for wrapping with cmdutil.NewError if it wants -// the typed code. +// Expression-attributable failures (parse / eval) are wrapped in *JQError so +// the caller can map them to a typed input.invalid_argument code. A failure to +// parse `raw` itself is internal (our own envelope JSON) and stays plain. func writeJQ(w io.Writer, raw []byte, expr string) error { query, err := gojq.Parse(expr) if err != nil { - return fmt.Errorf("jq parse: %w", err) + return &JQError{Err: fmt.Errorf("jq parse: %w", err)} } var input any if err := json.Unmarshal(raw, &input); err != nil { @@ -69,7 +79,7 @@ func writeJQ(w io.Writer, raw []byte, expr string) error { return nil } if e, ok := v.(error); ok { - return fmt.Errorf("jq eval: %w", e) + return &JQError{Err: fmt.Errorf("jq eval: %w", e)} } if s, ok := v.(string); ok { if _, err := fmt.Fprintln(w, s); err != nil { diff --git a/cli/internal/format/filter_test.go b/cli/internal/format/filter_test.go new file mode 100644 index 00000000..6b58406c --- /dev/null +++ b/cli/internal/format/filter_test.go @@ -0,0 +1,50 @@ +package format_test + +import ( + "bytes" + "errors" + "testing" + + "github.com/Tencent/WeKnora/cli/internal/format" +) + +// A malformed --jq expression must surface as *format.JQError so the cmdutil +// layer can map it to a typed input.invalid_argument (exit 5) instead of the +// unclassified internal.error bucket (exit 1). Regression for the bad-jq +// misclassification. +func TestWriteJSONFiltered_BadJQExpr_IsJQError(t *testing.T) { + for _, expr := range []string{".data[", "this is not jq"} { + buf := &bytes.Buffer{} + err := format.WriteJSONFiltered(buf, map[string]any{"data": []any{}}, nil, expr) + if err == nil { + t.Fatalf("expr %q: expected error, got nil", expr) + } + var jqe *format.JQError + if !errors.As(err, &jqe) { + t.Errorf("expr %q: expected *format.JQError, got %T: %v", expr, err, err) + } + } +} + +// A jq eval failure (valid parse, runtime error) is also expression-attributable +// and must be a *JQError. +func TestWriteJSONFiltered_JQEvalError_IsJQError(t *testing.T) { + buf := &bytes.Buffer{} + // ".foo.bar" against a non-object .foo raises an eval error. + err := format.WriteJSONFiltered(buf, map[string]any{"foo": 5}, nil, ".foo.bar") + if err == nil { + t.Fatal("expected eval error, got nil") + } + var jqe *format.JQError + if !errors.As(err, &jqe) { + t.Errorf("expected *format.JQError, got %T: %v", err, err) + } +} + +// A valid expression must NOT error. +func TestWriteJSONFiltered_ValidJQ_NoError(t *testing.T) { + buf := &bytes.Buffer{} + if err := format.WriteJSONFiltered(buf, map[string]any{"data": []any{"a", "b"}}, nil, ".data[]"); err != nil { + t.Fatalf("valid jq errored: %v", err) + } +} diff --git a/cli/internal/skillparity/parity_test.go b/cli/internal/skillparity/parity_test.go index 4688f363..2404368f 100644 --- a/cli/internal/skillparity/parity_test.go +++ b/cli/internal/skillparity/parity_test.go @@ -83,8 +83,13 @@ func TestSkillsReferenceLiveCommandsAndFlags(t *testing.T) { if err != nil { t.Fatalf("read %s: %v", path, err) } + // Normalize CRLF → LF before parsing: Windows CI checks out *.md with + // CRLF (git autocrlf), and the \n-anchored codeFence regex would never + // match ```weknora\r\n, resolving zero commands. Keep the tokenizer + // OS-independent rather than depend on checkout line endings. + content := strings.ReplaceAll(string(raw), "\r\n", "\n") rel, _ := filepath.Rel(skillsRoot(t), path) - for _, block := range codeFence.FindAllStringSubmatch(string(raw), -1) { + for _, block := range codeFence.FindAllStringSubmatch(content, -1) { for _, line := range strings.Split(block[1], "\n") { idx := strings.Index(line, "weknora ") if idx < 0 { diff --git a/cli/skills/weknora-rag-search/SKILL.md b/cli/skills/weknora-rag-search/SKILL.md index d094ac31..5f83745b 100644 --- a/cli/skills/weknora-rag-search/SKILL.md +++ b/cli/skills/weknora-rag-search/SKILL.md @@ -39,9 +39,11 @@ one wastes turns or returns the wrong shape. Use the decision table. ## Safety / Gotchas -- `--kb` is **required** for `chat`, `search chunks`, `search docs` (no implicit - project-link fallback) — an unset `--kb` is exit 2, a bad name is exit 1 +- `chat`, `search chunks`, `search docs` need a KB: pass `--kb `, or + set `WEKNORA_KB_ID`, or `weknora link` the directory (resolved in that order). + If none resolves it's exit 1 (`local.kb_id_required`); a bad name is exit 1 (`local.kb_not_found`). Resolve names with `weknora kb list` / `search kb`. + (`search kb` / `search sessions` are tenant-wide and take no `--kb`.) - `chat` / `session ask` stream **NDJSON** by default. Parse line-by-line; keep the `init` event's `session_id` **and** `message_id`. Use `--format text` only for a human transcript. diff --git a/cli/skills/weknora-shared/SKILL.md b/cli/skills/weknora-shared/SKILL.md index c09edfd9..80b9da28 100644 --- a/cli/skills/weknora-shared/SKILL.md +++ b/cli/skills/weknora-shared/SKILL.md @@ -166,4 +166,4 @@ that to learn a command without scraping the human help table. | Expecting `auth login --host/--name` flags | Gone — host comes from the profile; use global `--profile` | | Auto-adding `-y` to clear an exit-10 | Never; get user approval first (§5) | | `weknora chat "..."` prints "garbled" stream | NDJSON by default; use `--format text` for a transcript | -| `search chunks "q"` → exit 2 | `--kb` is required on search; pass `--kb ` | +| `search chunks "q"` → exit 1 `local.kb_id_required` | Pass `--kb `, set `WEKNORA_KB_ID`, or `weknora link` the dir |