From da9faa9e07ac818cdaec130e7dca70901aeead8b Mon Sep 17 00:00:00 2001 From: nullkey Date: Mon, 11 May 2026 02:44:34 +0800 Subject: [PATCH] =?UTF-8?q?feat(cli):=20add=20agent-first=20affordance=20?= =?UTF-8?q?=E2=80=94=20envelope,=20exit-10,=20--dry-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Borrows the lark-cli agent-affordance model (https://github.com/larksuite/cli/blob/main/AGENTS.md + skills/lark-shared/SKILL.md) so weknora is designed to be agent-friendly: error messages, output format, and flag design follow conventions agents can rely on. cli/AGENTS.md (operational reference for LLM agents invoking weknora): Public document covering envelope schema, exit-code protocol (0/1/2/10/130), stdout/stderr separation, and behavioral rules. Sensitive commands (\`context use\`, \`kb delete\`, \`doc delete\`, \`init\`) gain "AI agents:" paragraphs in their cobra Long descriptions so guidance shows in --help. format.Envelope schema additions: Risk per-operation classification (read / write / high-risk-write + action description), populated by write commands on both success and failure paths. Notice system advisories (CLI update available, server-CLI version skew); type defined, emit sites land in v0.3. DryRun marker for envelopes returned from --dry-run preview paths. RiskLevel constants realigned to lark's taxonomy: read / write / high-risk-write (was: read / mutating / destructive — not yet wired by any command). cmdutil.Error gains OperationRisk; PrintErrorEnvelope auto-attaches it to envelope.Risk so destructive failure paths surface uniformly. Exit-10 confirmation protocol: New ErrorCode \`input.confirmation_required\` mapped to exit code 10 in cmdutil.ExitCode. ConfirmDestructive now returns this code (with OperationRisk attached) when stdout is non-TTY or --json was set, with -y/--yes absent. Previous behavior — silent proceed in non-TTY — was unsafe: scripts and agents could delete resources with no explicit approval. Three test cases re-pinned around the new contract. This is a wire-contract change for any caller who relied on silent proceed; v0.0/v0.1 had no destructive commands, so the blast radius is contained to v0.2 itself. --dry-run global flag: cmd write paths (kb create/delete, doc upload/delete, api POST/PUT/PATCH/ DELETE) check cmdutil.IsDryRun(cmd) and skip the SDK call, emitting an envelope with dry_run=true plus a Risk classification. Read commands ignore --dry-run by design (no side effect to preview). Human-mode prints \`[dry-run] would \` to stdout. Command discovery: agents introspect via the existing \`--help\` surface (consistent with gh / kubectl / aws / gcloud / terraform — none of them ship a CLI-tree self-description command). An earlier draft added a \`weknora schema\` reflection command; dropped after a mainstream survey found it has no stable analog (lark-cli's schema describes Lark API methods, not its own CLI tree). Tests: 27 cli packages pass at this commit. Added two new tests covering envelope.risk and envelope._notice serialization. --- cli/AGENTS.md | 165 ++++++++++++++++++ cli/acceptance/e2e/e2e_test.go | 2 +- ...uth_status.error_auth_unauthenticated.json | 2 +- .../envelopes/auth_status.success.json | 2 +- .../envelopes/context_use.success.json | 2 +- .../envelopes/doctor.error_network.json | 2 +- .../envelopes/doctor.success_offline.json | 2 +- .../kb_get.error_resource_not_found.json | 2 +- .../testdata/envelopes/kb_get.success.json | 2 +- .../kb_list.error_auth_forbidden.json | 2 +- .../testdata/envelopes/kb_list.success.json | 2 +- .../envelopes/kb_list.success_empty.json | 2 +- .../envelopes/search.error_input_invalid.json | 2 +- .../search.error_resource_not_found.json | 2 +- .../testdata/envelopes/search.success.json | 2 +- .../testdata/envelopes/version.success.json | 2 +- .../whoami.error_auth_unauthenticated.json | 2 +- .../testdata/envelopes/whoami.success.json | 2 +- cli/cmd/api/api.go | 31 ++++ cli/cmd/api/api_test.go | 70 ++++++++ cli/cmd/context/use.go | 6 +- cli/cmd/doc/delete.go | 20 ++- cli/cmd/doc/delete_test.go | 19 +- cli/cmd/doc/upload.go | 15 +- cli/cmd/init/init.go | 6 +- cli/cmd/kb/create.go | 15 +- cli/cmd/kb/delete.go | 23 ++- cli/cmd/kb/delete_test.go | 46 +++-- cli/cmd/root.go | 7 +- cli/go.mod | 2 +- cli/internal/agent/detect.go | 10 +- cli/internal/cmdutil/confirm.go | 47 +++-- cli/internal/cmdutil/dryrun.go | 45 +++++ cli/internal/cmdutil/errors.go | 25 ++- cli/internal/cmdutil/exit.go | 50 ++++-- cli/internal/cmdutil/factory.go | 2 +- cli/internal/format/envelope.go | 81 +++++++-- cli/internal/format/envelope_test.go | 19 +- 38 files changed, 634 insertions(+), 104 deletions(-) create mode 100644 cli/AGENTS.md create mode 100644 cli/internal/cmdutil/dryrun.go diff --git a/cli/AGENTS.md b/cli/AGENTS.md new file mode 100644 index 00000000..24c5c356 --- /dev/null +++ b/cli/AGENTS.md @@ -0,0 +1,165 @@ +# Agent Integration Guide for `weknora` CLI + +> **Scope.** This file is an **operational reference** for LLM agents +> (Claude Code, Cursor, Codex, Aider, Gemini Coder, etc.) that **invoke +> `weknora` on a user's behalf**. It documents the wire shape, exit code, +> and behavioral conventions an agent integration relies on. +> +> This is **not** a contributor guide. If you are an AI coding agent +> editing weknora's source, see the repo root `README.md` (and, if added +> later, a separate contributor `AGENTS.md` at the repo root). + +`weknora` is designed to be agent-friendly: error messages, output format, +and flag design follow conventions agents can rely on. Wire-contract +breaking changes are flagged in their PR description and the corresponding +`weknora --version` bump — agents should pin a known-good version and +re-validate against `--help` output on upgrade. + +The model: **gh CLI** as the human-side north star, **lark-cli (larksuite)** +as the agent-affordance reference. The "Output contract" and "Behavioral +rules" sections below are the self-contained specification of that +decision; everything an integrator needs is in this document. + +--- + +## Output contract + +### Streams + +- **stdout** is the data channel: JSON envelope (with `--json`) or + human-formatted output. +- **stderr** is logs / progress / warnings / agent guidance footnotes. + Never parse stderr for data. + +A non-empty stderr does **not** mean failure — read the exit code instead. + +### JSON envelope + +When `--json` is set, stdout contains exactly one envelope: + +```jsonc +{ + "ok": true, // false on failure; check this first + "data": { /* command-specific payload */ }, + "error": { "code": "...", "message": "...", "hint": "..." }, // iff ok=false + "_meta": { "request_id": "...", "kb_id": "..." }, // optional + "risk": { "level": "high-risk-write", "action": "..." }, // write commands + "dry_run": false // true on --dry-run +} +``` + +This snippet is illustrative. Fields are added (never renamed or repurposed) +within a minor version, and agents must not error on unknown keys. The +authoritative envelope shape lives in `cli/internal/format/envelope.go`. + +### Error codes (closed registry) + +`error.code` is a `namespace.snake_case` string from a closed registry in +`cli/internal/cmdutil/errors.go` `AllCodes()`. An acceptance test enforces +that every code referenced in `cli/cmd/` is registered. + +Categories: `auth.*` / `resource.*` / `input.*` / `server.*` / `network.*` / +`local.*` / `mcp.*`. + +`error.hint` provides a deterministic next-step hint agents can follow +without natural-language parsing. + +### Exit codes + +| Code | Meaning | Agent action | +|---|---|---| +| `0` | Success | Continue | +| `1` | Typed error (see envelope.error.code) | Read code, decide retry/abort | +| `2` | Flag/argument validation error | Re-check `weknora --help` | +| `10` | **Confirmation required** for high-risk write | Ask the human, retry with `-y` only after explicit approval | +| `130` | Cancelled (SIGINT / Ctrl-C) | Stop, do not retry | + +The exit-10 protocol mirrors `lark-cli`'s +([source](https://github.com/larksuite/cli/blob/main/skills/lark-shared/SKILL.md)) +"high-risk write requires confirmation" model. **Never bypass exit 10 by +auto-passing `-y` without explicit user permission.** + +--- + +## Command surface + +Discover the command tree the same way human users do: + +```bash +weknora --help # top-level +weknora kb --help # subtree +weknora kb delete --help # single command flags +``` + +The command tree follows ` ` (gh style). Verbs are: + +| Verb | Semantics | Example | +|---|---|---| +| `list` | Multi-resource read | `kb list` | +| `view` | Single-resource read (alias `get` for v0.0/v0.1 callers) | `kb view ` | +| `create` | Create resource | `kb create --name X` | +| `delete` | Destructive remove | `kb delete -y` | +| `upload` | Bulk write content | `doc upload ` | +| `use` | Switch active selection | `context use ` | + +Top-level RAG / connectivity verbs: `chat`, `search`, `api`, `init`, `link`, +`auth`, `whoami`, `doctor`, `version`. + +--- + +## Behavioral rules + +These mirror lark-cli's per-command `Tips`. Per-command guidance also +appears in each command's `--help` output (under "AI agents:"). + +1. **Pass `-y/--yes`** on `kb delete` / `doc delete` / `auth logout` when + running headless. Without it, you will get exit 10. **Never auto-add + `-y`** without the user's explicit go-ahead — the exit-10 protocol is + the one explicit guard against unintended writes. +2. **Prefer typed commands over `weknora api`** for known endpoints. + Fallback to `weknora api` only when no typed command covers the call. +3. **For chat, prefer `--no-stream --json`** in agent contexts. Streaming + tokens to stdout makes JSON envelope parsing impossible. +4. **Honor `--dry-run`** — when the user passes it, don't follow up with + the real command unless explicitly asked. The dry-run envelope is the + answer. +5. **`init` writes to the user's working directory** — only run it when + the user invoked it, not as a side effect of unrelated automation. + +(Additional safety guidance — e.g. "do not switch context unless the +user asked" — is documented in the affected command's own `--help`.) + +--- + +## Auto-detection of agent environments + +`weknora` checks these environment variables (case-sensitive): + +| Env var | Detected agent name | +|---|---| +| `CLAUDECODE` | `claude-code` | +| `CURSOR_AGENT` | `cursor` | + +When any is set, `weknora --help` appends the command's `agent_help` +annotation. **No behavior change** — this is help-text rendering only. + +To suppress detection (e.g. running `weknora` interactively from inside +Claude Code without the agent footer): `WEKNORA_NO_AGENT_AUTODETECT=1`. + +The omnibus `--agent` mode-switch flag that briefly existed in early v0.2 +was removed: gh / kubectl / aws / docker / flyctl all decline this kind +of flag, since per-command `--json` + TTY auto-detect cover the same +ground without an extra global switch. Stripe's `DetectAIAgent` (the +inspiration) only tags User-Agent for telemetry, never flips behavior; +`weknora` now follows that narrower scope. + +--- + +## Reporting issues + +If the CLI's behavior contradicts this document, that is a bug. File at +https://github.com/Tencent/WeKnora/issues with: + +- The exact command line +- `weknora --version` output +- The envelope you got vs the envelope this document promises diff --git a/cli/acceptance/e2e/e2e_test.go b/cli/acceptance/e2e/e2e_test.go index 2d89afdb..f83d75dc 100644 --- a/cli/acceptance/e2e/e2e_test.go +++ b/cli/acceptance/e2e/e2e_test.go @@ -66,7 +66,7 @@ func TestRAGFullLoop(t *testing.T) { t.Cleanup(func() { // Best-effort cleanup; a 404 means the KB was already gone. - out, err := run(bin, env, "kb", "delete", kbID, "--force", "--json") + out, err := run(bin, env, "kb", "delete", kbID, "-y", "--json") if err != nil { t.Logf("cleanup kb delete: %v\n%s", err, out) } diff --git a/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json b/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json index 7cc15d93..161fb4f3 100644 --- a/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json +++ b/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json @@ -1 +1 @@ -{"ok":false,"error":{"code":"auth.unauthenticated","message":"fetch current user: HTTP error 401: {\"error\":\"unauthenticated\"}","hint":"run `weknora auth login`"}} +{"ok":false,"error":{"code":"auth.unauthenticated","message":"fetch current user: HTTP error 401: {\"error\":\"unauthenticated\"}","hint":"run `weknora auth login`"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/auth_status.success.json b/cli/acceptance/testdata/envelopes/auth_status.success.json index ccf07885..420547b1 100644 --- a/cli/acceptance/testdata/envelopes/auth_status.success.json +++ b/cli/acceptance/testdata/envelopes/auth_status.success.json @@ -1 +1 @@ -{"ok":true,"data":{"context":"","user_id":"usr_abc","email":"user@example.com","tenant_id":42,"tenant_name":"Acme"},"_meta":{"tenant_id":42}} +{"ok":true,"data":{"context":"","user_id":"usr_abc","email":"user@example.com","tenant_id":42,"tenant_name":"Acme"},"_meta":{"tenant_id":42},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/context_use.success.json b/cli/acceptance/testdata/envelopes/context_use.success.json index cb8c2529..19a0de1a 100644 --- a/cli/acceptance/testdata/envelopes/context_use.success.json +++ b/cli/acceptance/testdata/envelopes/context_use.success.json @@ -1 +1 @@ -{"ok":true,"data":{"current_context":"production","previous_context":"staging"}} +{"ok":true,"data":{"current_context":"production","previous_context":"staging"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/doctor.error_network.json b/cli/acceptance/testdata/envelopes/doctor.error_network.json index 99d75213..843c9f9a 100644 --- a/cli/acceptance/testdata/envelopes/doctor.error_network.json +++ b/cli/acceptance/testdata/envelopes/doctor.error_network.json @@ -1 +1 @@ -{"ok":false,"data":{"summary":{"all_passed":false,"passed":1,"failed":1,"skipped":2},"checks":[{"name":"base_url_reachable","status":"fail","details":"server returned 500","hint":"verify the host configured for the active context (run `weknora auth login --host=...`) and network reachability"},{"name":"auth_credential","status":"skip","details":"prereq failed: base_url_reachable"},{"name":"server_version","status":"skip","details":"prereq failed: auth_credential"},{"name":"credential_storage","status":"ok","details":"keyring or file storage available"}]}} +{"ok":false,"data":{"summary":{"all_passed":false,"passed":1,"failed":1,"skipped":2},"checks":[{"name":"base_url_reachable","status":"fail","details":"server returned 500","hint":"verify the host configured for the active context (run `weknora auth login --host=...`) and network reachability"},{"name":"auth_credential","status":"skip","details":"prereq failed: base_url_reachable"},{"name":"server_version","status":"skip","details":"prereq failed: auth_credential"},{"name":"credential_storage","status":"ok","details":"keyring or file storage available"}]},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/doctor.success_offline.json b/cli/acceptance/testdata/envelopes/doctor.success_offline.json index d5fbfbd5..2fabfaaa 100644 --- a/cli/acceptance/testdata/envelopes/doctor.success_offline.json +++ b/cli/acceptance/testdata/envelopes/doctor.success_offline.json @@ -1 +1 @@ -{"ok":true,"data":{"summary":{"all_passed":false,"passed":1,"failed":0,"skipped":3},"checks":[{"name":"base_url_reachable","status":"skip","details":"offline mode"},{"name":"auth_credential","status":"skip","details":"offline mode"},{"name":"server_version","status":"skip","details":"offline mode"},{"name":"credential_storage","status":"ok","details":"keyring or file storage available"}]}} +{"ok":true,"data":{"summary":{"all_passed":false,"passed":1,"failed":0,"skipped":3},"checks":[{"name":"base_url_reachable","status":"skip","details":"offline mode"},{"name":"auth_credential","status":"skip","details":"offline mode"},{"name":"server_version","status":"skip","details":"offline mode"},{"name":"credential_storage","status":"ok","details":"keyring or file storage available"}]},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/kb_get.error_resource_not_found.json b/cli/acceptance/testdata/envelopes/kb_get.error_resource_not_found.json index 9ad8b4e2..8f683d51 100644 --- a/cli/acceptance/testdata/envelopes/kb_get.error_resource_not_found.json +++ b/cli/acceptance/testdata/envelopes/kb_get.error_resource_not_found.json @@ -1 +1 @@ -{"ok":false,"error":{"code":"resource.not_found","message":"get knowledge base \"missing\": HTTP error 404: {\"error\":\"not found\"}","hint":"verify the resource ID; list available with `weknora kb list`"}} +{"ok":false,"error":{"code":"resource.not_found","message":"get knowledge base \"missing\": HTTP error 404: {\"error\":\"not found\"}","hint":"verify the resource ID; list available with `weknora kb list`"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/kb_get.success.json b/cli/acceptance/testdata/envelopes/kb_get.success.json index cc86fc7c..2a3d47aa 100644 --- a/cli/acceptance/testdata/envelopes/kb_get.success.json +++ b/cli/acceptance/testdata/envelopes/kb_get.success.json @@ -1 +1 @@ -{"ok":true,"data":{"id":"kb1","name":"Onboarding Docs","type":"","is_temporary":false,"description":"Internal onboarding handbook","tenant_id":42,"chunking_config":{"chunk_size":0,"chunk_overlap":0,"separators":null},"image_processing_config":{"model_id":""},"faq_config":null,"embedding_model_id":"text-embedding-3","summary_model_id":"","vlm_config":{"enabled":false,"model_id":""},"storage_provider_config":null,"storage_config":{"secret_id":"","secret_key":"","region":"","bucket_name":"","app_id":"","path_prefix":"","provider":""},"extract_config":null,"created_at":"2025-01-01T12:00:00Z","updated_at":"2025-01-01T12:00:00Z","knowledge_count":5,"chunk_count":128,"is_processing":false,"processing_count":0}} +{"ok":true,"data":{"id":"kb1","name":"Onboarding Docs","type":"","is_temporary":false,"description":"Internal onboarding handbook","tenant_id":42,"chunking_config":{"chunk_size":0,"chunk_overlap":0,"separators":null},"image_processing_config":{"model_id":""},"faq_config":null,"embedding_model_id":"text-embedding-3","summary_model_id":"","vlm_config":{"enabled":false,"model_id":""},"storage_provider_config":null,"storage_config":{"secret_id":"","secret_key":"","region":"","bucket_name":"","app_id":"","path_prefix":"","provider":""},"extract_config":null,"created_at":"2025-01-01T12:00:00Z","updated_at":"2025-01-01T12:00:00Z","knowledge_count":5,"chunk_count":128,"is_processing":false,"processing_count":0},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json b/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json index 0cb60cc7..591faf33 100644 --- a/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json +++ b/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json @@ -1 +1 @@ -{"ok":false,"error":{"code":"auth.forbidden","message":"list knowledge bases: HTTP error 403: {\"error\":\"forbidden\"}","hint":"active context lacks permission for this resource"}} +{"ok":false,"error":{"code":"auth.forbidden","message":"list knowledge bases: HTTP error 403: {\"error\":\"forbidden\"}","hint":"active context lacks permission for this resource"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/kb_list.success.json b/cli/acceptance/testdata/envelopes/kb_list.success.json index 0bb564a1..96e9481c 100644 --- a/cli/acceptance/testdata/envelopes/kb_list.success.json +++ b/cli/acceptance/testdata/envelopes/kb_list.success.json @@ -1 +1 @@ -{"ok":true,"data":{"items":[{"id":"kb1","name":"Onboarding Docs","type":"","is_temporary":false,"description":"","tenant_id":42,"chunking_config":{"chunk_size":0,"chunk_overlap":0,"separators":null},"image_processing_config":{"model_id":""},"faq_config":null,"embedding_model_id":"text-embedding-3","summary_model_id":"","vlm_config":{"enabled":false,"model_id":""},"storage_provider_config":null,"storage_config":{"secret_id":"","secret_key":"","region":"","bucket_name":"","app_id":"","path_prefix":"","provider":""},"extract_config":null,"created_at":"2025-01-01T12:00:00Z","updated_at":"2025-01-01T12:00:00Z","knowledge_count":5,"chunk_count":128,"is_processing":false,"processing_count":0},{"id":"kb2","name":"API Reference","type":"","is_temporary":false,"description":"","tenant_id":42,"chunking_config":{"chunk_size":0,"chunk_overlap":0,"separators":null},"image_processing_config":{"model_id":""},"faq_config":null,"embedding_model_id":"text-embedding-3","summary_model_id":"","vlm_config":{"enabled":false,"model_id":""},"storage_provider_config":null,"storage_config":{"secret_id":"","secret_key":"","region":"","bucket_name":"","app_id":"","path_prefix":"","provider":""},"extract_config":null,"created_at":"2025-01-01T12:00:00Z","updated_at":"2025-01-01T12:00:00Z","knowledge_count":12,"chunk_count":340,"is_processing":false,"processing_count":0}]}} +{"ok":true,"data":{"items":[{"id":"kb1","name":"Onboarding Docs","type":"","is_temporary":false,"description":"","tenant_id":42,"chunking_config":{"chunk_size":0,"chunk_overlap":0,"separators":null},"image_processing_config":{"model_id":""},"faq_config":null,"embedding_model_id":"text-embedding-3","summary_model_id":"","vlm_config":{"enabled":false,"model_id":""},"storage_provider_config":null,"storage_config":{"secret_id":"","secret_key":"","region":"","bucket_name":"","app_id":"","path_prefix":"","provider":""},"extract_config":null,"created_at":"2025-01-01T12:00:00Z","updated_at":"2025-01-01T12:00:00Z","knowledge_count":5,"chunk_count":128,"is_processing":false,"processing_count":0},{"id":"kb2","name":"API Reference","type":"","is_temporary":false,"description":"","tenant_id":42,"chunking_config":{"chunk_size":0,"chunk_overlap":0,"separators":null},"image_processing_config":{"model_id":""},"faq_config":null,"embedding_model_id":"text-embedding-3","summary_model_id":"","vlm_config":{"enabled":false,"model_id":""},"storage_provider_config":null,"storage_config":{"secret_id":"","secret_key":"","region":"","bucket_name":"","app_id":"","path_prefix":"","provider":""},"extract_config":null,"created_at":"2025-01-01T12:00:00Z","updated_at":"2025-01-01T12:00:00Z","knowledge_count":12,"chunk_count":340,"is_processing":false,"processing_count":0}]},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/kb_list.success_empty.json b/cli/acceptance/testdata/envelopes/kb_list.success_empty.json index 74e94f48..c5c66c94 100644 --- a/cli/acceptance/testdata/envelopes/kb_list.success_empty.json +++ b/cli/acceptance/testdata/envelopes/kb_list.success_empty.json @@ -1 +1 @@ -{"ok":true,"data":{"items":[]}} +{"ok":true,"data":{"items":[]},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/search.error_input_invalid.json b/cli/acceptance/testdata/envelopes/search.error_input_invalid.json index 40c1a907..d14b64ee 100644 --- a/cli/acceptance/testdata/envelopes/search.error_input_invalid.json +++ b/cli/acceptance/testdata/envelopes/search.error_input_invalid.json @@ -1 +1 @@ -{"ok":false,"error":{"code":"input.invalid_argument","message":"--no-vector and --no-keyword cannot both be set","hint":"see `weknora --help` for valid usage"}} +{"ok":false,"error":{"code":"input.invalid_argument","message":"--no-vector and --no-keyword cannot both be set","hint":"see `weknora --help` for valid usage"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json b/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json index 7c4b2b6b..55bd0d43 100644 --- a/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json +++ b/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json @@ -1 +1 @@ -{"ok":false,"error":{"code":"resource.not_found","message":"hybrid search: HTTP error 404: {\"error\":\"not found\"}","hint":"verify the resource ID; list available with `weknora kb list`"}} +{"ok":false,"error":{"code":"resource.not_found","message":"hybrid search: HTTP error 404: {\"error\":\"not found\"}","hint":"verify the resource ID; list available with `weknora kb list`"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/search.success.json b/cli/acceptance/testdata/envelopes/search.success.json index 3af7e9d6..2251f444 100644 --- a/cli/acceptance/testdata/envelopes/search.success.json +++ b/cli/acceptance/testdata/envelopes/search.success.json @@ -1 +1 @@ -{"ok":true,"data":[{"id":"chunk-1","content":"first chunk content","knowledge_id":"doc-1","chunk_index":0,"knowledge_title":"Doc 1","start_at":0,"end_at":0,"seq":0,"score":0.92,"match_type":0,"chunk_type":"","image_info":"","metadata":null,"knowledge_filename":"","knowledge_source":"","knowledge_channel":""},{"id":"chunk-2","content":"second chunk content","knowledge_id":"doc-2","chunk_index":1,"knowledge_title":"Doc 2","start_at":0,"end_at":0,"seq":0,"score":0.81,"match_type":1,"chunk_type":"","image_info":"","metadata":null,"knowledge_filename":"","knowledge_source":"","knowledge_channel":""}],"_meta":{"kb_id":"kb1"}} +{"ok":true,"data":[{"id":"chunk-1","content":"first chunk content","knowledge_id":"doc-1","chunk_index":0,"knowledge_title":"Doc 1","start_at":0,"end_at":0,"seq":0,"score":0.92,"match_type":0,"chunk_type":"","image_info":"","metadata":null,"knowledge_filename":"","knowledge_source":"","knowledge_channel":""},{"id":"chunk-2","content":"second chunk content","knowledge_id":"doc-2","chunk_index":1,"knowledge_title":"Doc 2","start_at":0,"end_at":0,"seq":0,"score":0.81,"match_type":1,"chunk_type":"","image_info":"","metadata":null,"knowledge_filename":"","knowledge_source":"","knowledge_channel":""}],"_meta":{"kb_id":"kb1"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/version.success.json b/cli/acceptance/testdata/envelopes/version.success.json index b49e35ed..288d34e4 100644 --- a/cli/acceptance/testdata/envelopes/version.success.json +++ b/cli/acceptance/testdata/envelopes/version.success.json @@ -1 +1 @@ -{"ok":true,"data":{"commit":"none","date":"unknown","version":"dev"}} +{"ok":true,"data":{"commit":"none","date":"unknown","version":"dev"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json b/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json index 7cc15d93..161fb4f3 100644 --- a/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json +++ b/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json @@ -1 +1 @@ -{"ok":false,"error":{"code":"auth.unauthenticated","message":"fetch current user: HTTP error 401: {\"error\":\"unauthenticated\"}","hint":"run `weknora auth login`"}} +{"ok":false,"error":{"code":"auth.unauthenticated","message":"fetch current user: HTTP error 401: {\"error\":\"unauthenticated\"}","hint":"run `weknora auth login`"},"dry_run":false} diff --git a/cli/acceptance/testdata/envelopes/whoami.success.json b/cli/acceptance/testdata/envelopes/whoami.success.json index 38a284ae..665bb8c5 100644 --- a/cli/acceptance/testdata/envelopes/whoami.success.json +++ b/cli/acceptance/testdata/envelopes/whoami.success.json @@ -1 +1 @@ -{"ok":true,"data":{"user_id":"usr_abc","tenant_id":42}} +{"ok":true,"data":{"user_id":"usr_abc","tenant_id":42},"dry_run":false} diff --git a/cli/cmd/api/api.go b/cli/cmd/api/api.go index 6ea10067..98a033f7 100644 --- a/cli/cmd/api/api.go +++ b/cli/cmd/api/api.go @@ -30,6 +30,8 @@ type Options struct { Data string DataFile string JSONOut bool + DryRun bool + Yes bool } // Service is the narrow SDK surface this command depends on. The production @@ -58,6 +60,20 @@ Examples: weknora api DELETE /api/v1/knowledge-bases/kb_xxx`, Args: cobra.ExactArgs(2), RunE: func(c *cobra.Command, args []string) error { + opts.DryRun = cmdutil.IsDryRun(c) + opts.Yes, _ = c.Flags().GetBool("yes") + method := strings.ToUpper(args[0]) + // Escape-hatch DELETE through `weknora api` is just as destructive + // as `weknora kb delete` — exit-10 protocol must apply (AGENTS.md). + // Dry-run is read-only preview, so it skips confirmation. + if !opts.DryRun && method == "DELETE" { + if err := cmdutil.ConfirmDestructive(f.Prompter(), opts.Yes, opts.JSONOut, "endpoint", args[1]); err != nil { + return err + } + } + if opts.DryRun { + return runAPI(c.Context(), opts, nil, args[0], args[1]) + } cli, err := f.Client() if err != nil { return err @@ -100,6 +116,21 @@ func runAPI(ctx context.Context, opts *Options, svc Service, methodArg, path str body = json.RawMessage(contents) } + // --dry-run only meaningful for write methods; GET/HEAD have no side + // effect to preview, so we proceed normally even with --dry-run. + if opts.DryRun && method != "GET" && method != "HEAD" { + level := format.RiskWrite + if method == "DELETE" { + level = format.RiskHighRiskWrite + } + preview := map[string]any{"method": method, "path": path} + if body != nil { + preview["body"] = body + } + return cmdutil.EmitDryRun(opts.JSONOut, preview, nil, + &format.Risk{Level: level, Action: fmt.Sprintf("%s %s", method, path)}) + } + resp, err := svc.Raw(ctx, method, path, body) if err != nil { // Transport / DNS failure (Raw never returns a typed HTTP error of its diff --git a/cli/cmd/api/api_test.go b/cli/cmd/api/api_test.go index 739767e2..873340e8 100644 --- a/cli/cmd/api/api_test.go +++ b/cli/cmd/api/api_test.go @@ -11,8 +11,11 @@ import ( "strings" "testing" + "github.com/spf13/cobra" + "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/prompt" sdk "github.com/Tencent/WeKnora/client" ) @@ -179,6 +182,73 @@ func TestAPI_PathWithoutSlash(t *testing.T) { } } +// withRootHarness wraps `weknora api ...` under a synthetic root cmd that +// registers the global `-y/--yes` persistent flag (mirrors addGlobalFlags in +// cmd/root.go). Required because api's NewCmd doesn't register --yes itself +// — it inherits from root in production. +func withRootHarness(api *cobra.Command, args ...string) *cobra.Command { + root := &cobra.Command{Use: "weknora"} + root.PersistentFlags().BoolP("yes", "y", false, "") + root.PersistentFlags().Bool("dry-run", false, "") + root.AddCommand(api) + root.SetArgs(append([]string{"api"}, args...)) + root.SetContext(context.Background()) + root.SilenceErrors = true + root.SilenceUsage = true + return root +} + +// TestAPI_DELETE_RequiresConfirmation pins the exit-10 protocol on the +// escape-hatch DELETE path: agent invokes `weknora api DELETE /...` without +// -y/--yes, must get input.confirmation_required + exit 10. Confirmation is +// enforced in NewCmd.RunE (not runAPI), so the test drives the cobra cmd. +func TestAPI_DELETE_RequiresConfirmation(t *testing.T) { + iostreams.SetForTest(t) // non-TTY + f := &cmdutil.Factory{ + Client: func() (*sdk.Client, error) { return nil, nil }, + Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} }, + } + root := withRootHarness(NewCmd(f), "DELETE", "/api/v1/knowledge-bases/kb_xxx") + err := root.Execute() + if err == nil { + t.Fatal("expected confirmation_required error for DELETE without -y") + } + 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_DELETE_WithYes_Proceeds: -y/--yes opt-in skips confirmation and +// dispatches to the SDK. Server returns 200 to verify the happy-path lands +// on the response body emit. +func TestAPI_DELETE_WithYes_Proceeds(t *testing.T) { + iostreams.SetForTest(t) + called := false + cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + t.Errorf("expected DELETE, got %s", r.Method) + } + 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), "DELETE", "/api/v1/knowledge-bases/kb_xxx", "-y") + if err := root.Execute(); err != nil { + t.Fatalf("execute: %v", err) + } + if !called { + t.Error("DELETE handler not called — confirmation may have blocked") + } +} + // asTypedError is a tiny wrapper around errors.As that keeps the call sites // concise. Returns true on success, populating dst. func asTypedError(err error, dst **cmdutil.Error) bool { diff --git a/cli/cmd/context/use.go b/cli/cmd/context/use.go index 408787f5..eeaf5ccf 100644 --- a/cli/cmd/context/use.go +++ b/cli/cmd/context/use.go @@ -22,7 +22,11 @@ func NewCmdUse(f *cmdutil.Factory) *cobra.Command { The active context is what every subsequent command uses for auth + host. The global --context flag (e.g. weknora --context staging kb list) overrides for -one command without writing to disk.`, +one command without writing to disk. + +AI agents: Do NOT switch the active context unless the user explicitly asked +you to. Context selection is a user preference; one-shot overrides should use +the global --context flag instead, which writes nothing to disk.`, Example: ` weknora context use staging # persist switch weknora --context staging kb list # one-shot override (no disk write) weknora context use --help # this help`, diff --git a/cli/cmd/doc/delete.go b/cli/cmd/doc/delete.go index 08a0d51a..d9b25e13 100644 --- a/cli/cmd/doc/delete.go +++ b/cli/cmd/doc/delete.go @@ -18,6 +18,7 @@ import ( type DeleteOptions struct { Yes bool JSONOut bool + DryRun bool } // DeleteService is the narrow SDK surface this command depends on. @@ -41,13 +42,21 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { Short: "Delete a document from a knowledge base", Long: `Permanently deletes one document. Prompts for confirmation by default when stdout is a TTY and --json is not set; pass -y/--yes (global flag) to skip -the prompt (required in agent / CI / piped contexts).`, +the prompt (required in agent / CI / piped contexts). + +AI agents: This is a high-risk write. Without -y/--yes the CLI exits 10 and +returns an envelope describing the missing confirmation. NEVER auto-pass -y +without the user's explicit go-ahead.`, Example: ` weknora doc delete doc_abc # interactive confirm weknora doc delete doc_abc -y # no prompt weknora doc delete doc_abc -y --json # envelope output`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.Yes, _ = c.Flags().GetBool("yes") + opts.DryRun = cmdutil.IsDryRun(c) + if opts.DryRun { + return runDelete(c.Context(), opts, nil, f.Prompter(), args[0]) + } cli, err := f.Client() if err != nil { return err @@ -61,6 +70,12 @@ the prompt (required in agent / CI / piped contexts).`, } func runDelete(ctx context.Context, opts *DeleteOptions, svc DeleteService, p prompt.Prompter, id string) error { + if opts.DryRun { + return cmdutil.EmitDryRun(opts.JSONOut, + deleteResult{ID: id, Deleted: false}, nil, + &format.Risk{Level: format.RiskHighRiskWrite, Action: fmt.Sprintf("delete document %s", id)}) + } + if err := cmdutil.ConfirmDestructive(p, opts.Yes, opts.JSONOut, "document", id); err != nil { return err } @@ -70,7 +85,8 @@ func runDelete(ctx context.Context, opts *DeleteOptions, svc DeleteService, p pr } if opts.JSONOut { - return format.WriteEnvelope(iostreams.IO.Out, format.Success(deleteResult{ID: id, Deleted: true}, nil)) + risk := &format.Risk{Level: format.RiskHighRiskWrite, Action: fmt.Sprintf("deleted document %s", id)} + return format.WriteEnvelope(iostreams.IO.Out, format.SuccessWithRisk(deleteResult{ID: id, Deleted: true}, nil, risk)) } fmt.Fprintf(iostreams.IO.Out, "✓ Deleted document %s\n", id) return nil diff --git a/cli/cmd/doc/delete_test.go b/cli/cmd/doc/delete_test.go index f3da2a59..140fdfac 100644 --- a/cli/cmd/doc/delete_test.go +++ b/cli/cmd/doc/delete_test.go @@ -128,15 +128,18 @@ func TestDelete_AgentPrompterErrors(t *testing.T) { assert.Equal(t, cmdutil.CodeInputMissingFlag, typed.Code) } -// TestDelete_NoForce_NonTTY_Proceeds: when stdout isn't a TTY (typical agent -// pipe / CI), the confirm guard is skipped. This documents the existing -// contract — destructive ops in a pipe rely on the caller having chosen to -// pipe (intent expressed by the redirection) and on agents passing --force -// explicitly. Mirrors `weknora kb delete`. -func TestDelete_NoForce_NonTTY_Proceeds(t *testing.T) { +// TestDelete_NoYes_NonTTY_RequiresConfirmation: when stdout isn't a TTY +// (typical agent pipe / CI), the lark-cli skill protocol requires explicit +// -y/--yes. The CLI exits 10 with input.confirmation_required, never +// silently proceeds. See cli/AGENTS.md "Exit codes". +func TestDelete_NoYes_NonTTY_RequiresConfirmation(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{} err := runDelete(context.Background(), &DeleteOptions{Yes: false}, svc, errPrompter{}, "doc_abc") - require.NoError(t, err, "non-TTY non-json invocation should proceed without prompting") - assert.Equal(t, 1, svc.calls) + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputConfirmationRequired, typed.Code) + assert.Equal(t, 0, svc.calls, "non-TTY without -y must not call DeleteKnowledge") + assert.Equal(t, 10, cmdutil.ExitCode(err)) } diff --git a/cli/cmd/doc/upload.go b/cli/cmd/doc/upload.go index 82e98537..02fd087e 100644 --- a/cli/cmd/doc/upload.go +++ b/cli/cmd/doc/upload.go @@ -23,6 +23,7 @@ const uploadChannel = "api" type UploadOptions struct { Name string JSONOut bool + DryRun bool } // UploadService is the narrow SDK surface this command depends on. @@ -62,10 +63,14 @@ planned for v0.3.`, if err := validateUploadPath(path); err != nil { return err } + opts.DryRun = cmdutil.IsDryRun(c) kbID, err := f.ResolveKB(c) if err != nil { return err } + if opts.DryRun { + return runUpload(c.Context(), opts, nil, kbID, path) + } cli, err := f.Client() if err != nil { return err @@ -104,13 +109,21 @@ func validateUploadPath(path string) error { } func runUpload(ctx context.Context, opts *UploadOptions, svc UploadService, kbID, path string) error { + if opts.DryRun { + return cmdutil.EmitDryRun(opts.JSONOut, + map[string]string{"file": path, "kb_id": kbID, "name": opts.Name}, + &format.Meta{KBID: kbID}, + &format.Risk{Level: format.RiskWrite, Action: fmt.Sprintf("upload %s to kb %s", path, kbID)}) + } + k, err := svc.CreateKnowledgeFromFile(ctx, kbID, path, nil /*metadata*/, nil /*enableMultimodel*/, opts.Name, uploadChannel) if err != nil { return cmdutil.Wrapf(cmdutil.ClassifyHTTPError(err), err, "upload %s", path) } if opts.JSONOut { - return format.WriteEnvelope(iostreams.IO.Out, format.Success(k, &format.Meta{KBID: kbID})) + risk := &format.Risk{Level: format.RiskWrite, Action: fmt.Sprintf("uploaded %s", path)} + return format.WriteEnvelope(iostreams.IO.Out, format.SuccessWithRisk(k, &format.Meta{KBID: kbID}, risk)) } displayed := opts.Name if displayed == "" { diff --git a/cli/cmd/init/init.go b/cli/cmd/init/init.go index 1b50cbe0..491553c9 100644 --- a/cli/cmd/init/init.go +++ b/cli/cmd/init/init.go @@ -52,7 +52,11 @@ directory (or any subdirectory) automatically resolve --kb-id from the link unless overridden by the --kb-id / --kb flags or WEKNORA_KB_ID env var. Mirrors the npm init / cargo init / git init UX pattern: one-time setup that -removes the need to re-pass --kb-id on every command.`, +removes the need to re-pass --kb-id on every command. + +AI agents: ` + "`init`" + ` writes to the user's working directory. Only run +it when the user explicitly asked to link this directory — don't run it as a +side effect of unrelated automation.`, Example: ` weknora init --kb-id kb_abc # explicit id weknora init --kb engineering --yes # name → id, no prompt weknora init # interactive (TTY) diff --git a/cli/cmd/kb/create.go b/cli/cmd/kb/create.go index 3a428001..2374c5ce 100644 --- a/cli/cmd/kb/create.go +++ b/cli/cmd/kb/create.go @@ -20,6 +20,7 @@ type CreateOptions struct { Description string EmbeddingModel string JSONOut bool + DryRun bool } // CreateService is the narrow SDK surface this command depends on. @@ -36,6 +37,10 @@ func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { Short: "Create a new knowledge base", Args: cobra.NoArgs, RunE: func(c *cobra.Command, _ []string) error { + opts.DryRun = cmdutil.IsDryRun(c) + if opts.DryRun { + return runCreate(c.Context(), opts, nil) // service unused on dry-run + } cli, err := f.Client() if err != nil { return err @@ -66,15 +71,19 @@ func runCreate(ctx context.Context, opts *CreateOptions, svc CreateService) erro req.EmbeddingModelID = opts.EmbeddingModel } + if opts.DryRun { + return cmdutil.EmitDryRun(opts.JSONOut, req, nil, + &format.Risk{Level: format.RiskWrite, Action: fmt.Sprintf("create knowledge base %q", opts.Name)}) + } + created, err := svc.CreateKnowledgeBase(ctx, req) if err != nil { return cmdutil.Wrapf(cmdutil.ClassifyHTTPError(err), err, "create knowledge base") } if opts.JSONOut { - return format.WriteEnvelope(iostreams.IO.Out, format.Success(created, &format.Meta{ - KBID: created.ID, - })) + risk := &format.Risk{Level: format.RiskWrite, Action: fmt.Sprintf("created knowledge base %s", created.ID)} + return format.WriteEnvelope(iostreams.IO.Out, format.SuccessWithRisk(created, &format.Meta{KBID: created.ID}, risk)) } fmt.Fprintf(iostreams.IO.Out, "✓ Created knowledge base %q (id: %s)\n", created.Name, created.ID) return nil diff --git a/cli/cmd/kb/delete.go b/cli/cmd/kb/delete.go index 782fda27..117f9bc5 100644 --- a/cli/cmd/kb/delete.go +++ b/cli/cmd/kb/delete.go @@ -18,6 +18,7 @@ import ( type DeleteOptions struct { Yes bool JSONOut bool + DryRun bool } // DeleteService is the narrow SDK surface this command depends on. @@ -44,13 +45,22 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { Long: `Permanently deletes a knowledge base and all its contents. Prompts for confirmation by default when stdout is a TTY and --json is not set. -Pass -y/--yes (global flag) to skip the prompt (required in agent / CI / piped contexts).`, +Pass -y/--yes (global flag) to skip the prompt (required in agent / CI / piped contexts). + +AI agents: This is a high-risk write. Without -y/--yes the CLI exits 10 and +returns an envelope describing the missing confirmation. NEVER auto-pass -y +without the user's explicit go-ahead — the exit-10 protocol exists exactly to +guard against unintended deletes.`, Example: ` weknora kb delete kb_abc # interactive confirm weknora kb delete kb_abc -y # no prompt weknora kb delete kb_abc -y --json # envelope output`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.Yes, _ = c.Flags().GetBool("yes") + opts.DryRun = cmdutil.IsDryRun(c) + if opts.DryRun { + return runDelete(c.Context(), opts, nil, f.Prompter(), args[0]) + } cli, err := f.Client() if err != nil { return err @@ -64,6 +74,12 @@ Pass -y/--yes (global flag) to skip the prompt (required in agent / CI / piped c } func runDelete(ctx context.Context, opts *DeleteOptions, svc DeleteService, p prompt.Prompter, id string) error { + if opts.DryRun { + return cmdutil.EmitDryRun(opts.JSONOut, + deleteResult{ID: id, Deleted: false}, &format.Meta{KBID: id}, + &format.Risk{Level: format.RiskHighRiskWrite, Action: fmt.Sprintf("delete knowledge base %s", id)}) + } + if err := cmdutil.ConfirmDestructive(p, opts.Yes, opts.JSONOut, "knowledge base", id); err != nil { return err } @@ -73,9 +89,8 @@ func runDelete(ctx context.Context, opts *DeleteOptions, svc DeleteService, p pr } if opts.JSONOut { - return format.WriteEnvelope(iostreams.IO.Out, format.Success(deleteResult{ID: id, Deleted: true}, &format.Meta{ - KBID: id, - })) + risk := &format.Risk{Level: format.RiskHighRiskWrite, Action: fmt.Sprintf("deleted knowledge base %s", id)} + return format.WriteEnvelope(iostreams.IO.Out, format.SuccessWithRisk(deleteResult{ID: id, Deleted: true}, &format.Meta{KBID: id}, risk)) } fmt.Fprintf(iostreams.IO.Out, "✓ Deleted knowledge base %s\n", id) return nil diff --git a/cli/cmd/kb/delete_test.go b/cli/cmd/kb/delete_test.go index 624e1f3d..fde0b91d 100644 --- a/cli/cmd/kb/delete_test.go +++ b/cli/cmd/kb/delete_test.go @@ -67,17 +67,22 @@ func TestDelete_NotFound(t *testing.T) { assert.Equal(t, cmdutil.CodeResourceNotFound, typed.Code) } -func TestDelete_NonTTY_NoPrompt_NoForce(t *testing.T) { - // SetForTest uses bytes.Buffer for Out — IsStdoutTTY() = false. Confirm - // path must be skipped entirely, so DeleteKnowledgeBase should run. - out, _ := iostreams.SetForTest(t) +func TestDelete_NonTTY_NoYes_RequiresConfirmation(t *testing.T) { + // SetForTest uses bytes.Buffer for Out — IsStdoutTTY() = false. Without + // -y/--yes, exit-10 protocol fires (lark-cli skill protocol; AGENTS.md): + // the CLI must NOT silently proceed in scripted contexts. + iostreams.SetForTest(t) svc := &fakeDeleteSvc{} p := &confirmPrompter{} - require.NoError(t, runDelete(context.Background(), &DeleteOptions{}, svc, p, "kb_nontty")) + err := runDelete(context.Background(), &DeleteOptions{}, svc, p, "kb_nontty") - assert.True(t, svc.called, "non-TTY must skip prompt and proceed") + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputConfirmationRequired, typed.Code) + assert.False(t, svc.called, "non-TTY without -y must not call DeleteKnowledgeBase") assert.False(t, p.asked, "non-TTY ⇒ Confirm is never invoked") - assert.Contains(t, out.String(), "kb_nontty") + assert.Equal(t, 10, cmdutil.ExitCode(err), "exit code 10 per lark-cli skill protocol") } func TestDelete_JSONOutput(t *testing.T) { @@ -137,15 +142,34 @@ func TestDelete_ConfirmPrompterError(t *testing.T) { assert.False(t, svc.called) } -func TestDelete_JSONOut_SkipsPrompt(t *testing.T) { - // Even on a TTY, --json indicates a scripted caller; don't prompt. - out, _ := iostreams.SetForTestWithTTY(t) +func TestDelete_JSONOut_NoYes_RequiresConfirmation(t *testing.T) { + // Even on a TTY, --json indicates a scripted caller; cannot prompt. + // Exit-10 protocol must fire when -y is absent. + iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} p := &confirmPrompter{} opts := &DeleteOptions{JSONOut: true} + err := runDelete(context.Background(), opts, svc, p, "kb_jtty") + + require.Error(t, err) + var typed *cmdutil.Error + require.ErrorAs(t, err, &typed) + assert.Equal(t, cmdutil.CodeInputConfirmationRequired, typed.Code) + assert.False(t, p.asked, "--json must skip the prompt even on TTY") + assert.False(t, svc.called, "--json without -y must not call DeleteKnowledgeBase") + assert.Equal(t, 10, cmdutil.ExitCode(err)) +} + +func TestDelete_JSONOut_WithYes_Proceeds(t *testing.T) { + // --json + -y is the agent happy-path: scripted caller with explicit + // approval. Must call SDK and emit envelope. + out, _ := iostreams.SetForTestWithTTY(t) + svc := &fakeDeleteSvc{} + p := &confirmPrompter{} + opts := &DeleteOptions{Yes: true, JSONOut: true} require.NoError(t, runDelete(context.Background(), opts, svc, p, "kb_jtty")) - assert.False(t, p.asked, "--json must skip the prompt even on TTY") + assert.False(t, p.asked, "-y must skip the prompt") assert.True(t, svc.called) assert.Contains(t, out.String(), `"deleted":true`) } diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 4d10c5c8..4c952f60 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -1,8 +1,10 @@ // Package cmd holds the cobra command tree. main.go calls Execute(). // // v0.0 shipped: version / auth / search. -// v0.1 adds: whoami / doctor / kb (list + get) / context (use). -// v0.2 adds: init / link / kb (create + delete) / doc (list + upload + delete) / api / chat. +// v0.1 adds: whoami / doctor / kb (list + view) / context (use). +// v0.2 adds: init / link / kb (create + delete) / doc (list + upload + delete) +// / api / chat. The kb view command is the primary; "get" +// is preserved as a cobra alias for v0.0/v0.1 callers. package cmd import ( @@ -197,6 +199,7 @@ func addGlobalFlags(cmd *cobra.Command) { pf := cmd.PersistentFlags() pf.BoolP("yes", "y", false, "Skip confirmation prompts on destructive operations") pf.String("context", "", "Override the active context for this invocation (no disk write)") + pf.Bool("dry-run", false, "Preview the operation without executing (write commands only; read commands ignore)") } // agentAwareHelpFunc wraps cobra's default help to append the AI agent guidance diff --git a/cli/go.mod b/cli/go.mod index 91e0fe25..08a993c1 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -10,6 +10,7 @@ require ( github.com/mattn/go-isatty v0.0.22 github.com/mattn/go-runewidth v0.0.23 github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.9 github.com/stretchr/testify v1.11.1 github.com/zalando/go-keyring v0.2.8 gopkg.in/yaml.v3 v3.0.1 @@ -42,7 +43,6 @@ require ( github.com/muesli/termenv v0.16.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect - github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/sync v0.15.0 // indirect golang.org/x/sys v0.33.0 // indirect diff --git a/cli/internal/agent/detect.go b/cli/internal/agent/detect.go index 392f85f1..d36c697a 100644 --- a/cli/internal/agent/detect.go +++ b/cli/internal/agent/detect.go @@ -1,11 +1,11 @@ // Package agent handles AI agent integration: env-based detection (used to // trigger AGENT-targeted help text) and per-command help annotations. // -// v0.2: removed the omnibus `--agent` flag + ApplyAgentSugar mode-switch -// (audited as over-design — see docs/superpowers/specs/2026-05-09 ADR-3). -// Mainstream CLIs (gh / kubectl / aws / docker / flyctl) deliberately don't -// have a single mode-switch flag; per-command --json + TTY auto-detect cover -// 95% of cases. WeKnora now follows that convention. +// v0.2 ADR-3: removed the omnibus `--agent` flag + ApplyAgentSugar +// mode-switch as over-design. Mainstream CLIs (gh / kubectl / aws / docker / +// flyctl) deliberately don't have a single mode-switch flag; per-command +// --json + TTY auto-detect cover 95% of cases. WeKnora now follows that +// convention. See cli/AGENTS.md for the full agent contract. // // What remains here: a small env-detect for known coding agents, used purely // to render the AGENT-targeted help section (no behavior change). Patterned diff --git a/cli/internal/cmdutil/confirm.go b/cli/internal/cmdutil/confirm.go index 414e8a05..317ea5e0 100644 --- a/cli/internal/cmdutil/confirm.go +++ b/cli/internal/cmdutil/confirm.go @@ -1,35 +1,50 @@ package cmdutil import ( + "errors" "fmt" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" ) -// ConfirmDestructive prompts the user to confirm a destructive operation -// (e.g. delete) when the call is interactive, and proceeds without -// prompting otherwise. Behavior matrix: +// ConfirmDestructive guards a destructive operation (delete, force-overwrite) +// behind explicit user approval. Behavior matrix: // -// yes=true → proceed (skip prompt; explicit user opt-in via -y) -// non-TTY stdout → proceed (no UI to ask; safer to follow the call -// than to break scripts/CI that don't pass -// -y every time) -// jsonOut=true → proceed (envelope mode is by definition scripted, -// a prompt would deadlock the consumer) +// yes=true → proceed (explicit user opt-in via -y/--yes) +// non-TTY OR jsonOut → return CodeInputConfirmationRequired (exit 10); +// no UI to prompt, agent/CI must re-invoke with -y +// after the human explicitly approves // TTY + interactive → prompt; user-yes proceeds, user-no returns // CodeUserAborted ("Aborted." to stderr) -// prompter error → returns CodeInputMissingFlag (rare; AgentPrompter -// path or stdin closed mid-prompt) +// prompter error → returns CodeInputMissingFlag (rare; stdin closed +// mid-prompt) // -// `yes` should be sourced from the persistent global -y/--yes flag (see -// addGlobalFlags in cli/cmd/root.go). Mirrors gh's `--yes` semantics on -// destructive commands: gh repo delete --yes -// (https://cli.github.com/manual/gh_repo_delete). +// The non-TTY branch is the lark-cli skill protocol: high-risk writes +// always require explicit confirmation in scripted contexts, never silent +// proceed. See cli/AGENTS.md "Exit codes" and +// https://github.com/larksuite/cli/blob/main/skills/lark-shared/SKILL.md. +// +// `yes` should be sourced from the persistent global -y/--yes flag. +// +// On exit-10 path, the returned *Error carries OperationRisk so the envelope +// printer attaches `risk: {level: "high-risk-write", action: ...}`. func ConfirmDestructive(p prompt.Prompter, yes, jsonOut bool, what, id string) error { - if yes || !iostreams.IO.IsStdoutTTY() || jsonOut { + if yes { return nil } + risk := &OperationRisk{Level: "high-risk-write", Action: fmt.Sprintf("delete %s %s", what, id)} + if !iostreams.IO.IsStdoutTTY() || jsonOut { + e := NewError( + CodeInputConfirmationRequired, + fmt.Sprintf("delete %s %s requires explicit confirmation: re-run with -y/--yes", what, id), + ) + var typed *Error + if errors.As(e, &typed) { + typed.OperationRisk = risk + } + return e + } ok, err := p.Confirm(fmt.Sprintf("Delete %s %s? This cannot be undone.", what, id), false) if err != nil { return Wrapf(CodeInputMissingFlag, err, "confirm delete") diff --git a/cli/internal/cmdutil/dryrun.go b/cli/internal/cmdutil/dryrun.go new file mode 100644 index 00000000..ae29b9db --- /dev/null +++ b/cli/internal/cmdutil/dryrun.go @@ -0,0 +1,45 @@ +package cmdutil + +import ( + "fmt" + + "github.com/spf13/cobra" + + "github.com/Tencent/WeKnora/cli/internal/format" + "github.com/Tencent/WeKnora/cli/internal/iostreams" +) + +// IsDryRun reports whether the global --dry-run flag was set on cmd or any +// of its parents. Write commands check this and skip the SDK call, returning +// an envelope with dry_run=true that describes the action that would have +// run. Read commands ignore --dry-run (no side effect to preview). +// +// Pattern from lark-cli's `--dry-run` (cmd/api/api.go DryRun field). +func IsDryRun(cmd *cobra.Command) bool { + if cmd == nil { + return false + } + v, _ := cmd.Flags().GetBool("dry-run") + return v +} + +// EmitDryRun writes the canonical preview envelope for a write command. JSON +// mode emits SuccessWithRisk + DryRun=true; human mode prints the +// `[dry-run] would ` line to stdout, with " (high-risk)" +// appended automatically when risk.Level == RiskHighRiskWrite. Centralized +// so wire shape stays identical across kb create/delete, doc upload/delete, +// api, and any future write command. +func EmitDryRun(jsonOut bool, data any, meta *format.Meta, risk *format.Risk) error { + out := iostreams.IO.Out + if jsonOut { + env := format.SuccessWithRisk(data, meta, risk) + env.DryRun = true + return format.WriteEnvelope(out, env) + } + line := risk.Action + if risk.Level == format.RiskHighRiskWrite { + line += " (high-risk)" + } + _, err := fmt.Fprintf(out, "[dry-run] would %s\n", line) + return err +} diff --git a/cli/internal/cmdutil/errors.go b/cli/internal/cmdutil/errors.go index 924a8cd5..c10dadc9 100644 --- a/cli/internal/cmdutil/errors.go +++ b/cli/internal/cmdutil/errors.go @@ -29,8 +29,14 @@ const ( CodeResourceLocked ErrorCode = "resource.locked" // input.* — flag and argument validation - CodeInputInvalidArgument ErrorCode = "input.invalid_argument" - CodeInputMissingFlag ErrorCode = "input.missing_flag" + CodeInputInvalidArgument ErrorCode = "input.invalid_argument" + CodeInputMissingFlag ErrorCode = "input.missing_flag" + // CodeInputConfirmationRequired marks a high-risk write that has no + // interactive UI (non-TTY or --json) and was invoked without -y/--yes. + // Mapped to exit code 10 (lark-cli skill protocol — see cli/AGENTS.md). + // Agents must surface the envelope to the user and only retry with -y + // after explicit human approval; never auto-retry. + CodeInputConfirmationRequired ErrorCode = "input.confirmation_required" // server.* / network.* CodeServerError ErrorCode = "server.error" @@ -84,6 +90,19 @@ type Error struct { Cause error Retryable bool HTTPStatus int + // Risk classifies the operation that produced this error. Set by callers + // invoking destructive write paths so envelope.risk surfaces to agents. + // Stored as the format.Risk JSON shape via OperationRisk to avoid an + // import cycle with internal/format. + OperationRisk *OperationRisk +} + +// OperationRisk mirrors format.Risk in the cmdutil layer (avoiding a circular +// import). cmdutil → format is OK; the inverse is not, so cmdutil owns its +// own type and ToErrorBody / PrintErrorEnvelope translate. +type OperationRisk struct { + Level string // "read" | "write" | "high-risk-write" + Action string } func (e *Error) Error() string { @@ -229,7 +248,7 @@ func AllCodes() []ErrorCode { // resource CodeResourceNotFound, CodeResourceAlreadyExists, CodeResourceLocked, // input - CodeInputInvalidArgument, CodeInputMissingFlag, + CodeInputInvalidArgument, CodeInputMissingFlag, CodeInputConfirmationRequired, // server / network CodeServerError, CodeServerTimeout, CodeServerRateLimited, CodeServerIncompatibleVersion, CodeNetworkError, diff --git a/cli/internal/cmdutil/exit.go b/cli/internal/cmdutil/exit.go index 37f51e57..4016b619 100644 --- a/cli/internal/cmdutil/exit.go +++ b/cli/internal/cmdutil/exit.go @@ -8,16 +8,19 @@ import ( "github.com/Tencent/WeKnora/cli/internal/format" ) -// ExitCode maps an error to the documented CLI exit code (spec §2.4). -// Mirrors gh / Stripe convention: -// - 0 success -// - 1 generic / unknown -// - 2 flag / argument problem -// - 3 auth.* -// - 4 resource.not_found -// - 5 input.* -// - 6 server.rate_limited -// - 7 server.* (other) / network.* +// ExitCode maps an error to the documented CLI exit code (spec §2.4 + ADR-3). +// Mirrors gh / Stripe / lark-cli convention: +// - 0 success +// - 1 generic / unknown typed error +// - 2 flag / argument problem +// - 3 auth.* +// - 4 resource.not_found +// - 5 input.* (other than confirmation_required) +// - 6 server.rate_limited +// - 7 server.* (other) / network.* +// - 10 input.confirmation_required — high-risk write needs explicit -y +// (lark-cli skill protocol; see cli/AGENTS.md) +// - 130 SIGINT (handled by Go runtime, not this function) func ExitCode(err error) int { if err == nil { return 0 @@ -29,6 +32,9 @@ func ExitCode(err error) int { if errors.Is(err, SilentError) { return 1 } + if matchCode(err, CodeInputConfirmationRequired) { + return 10 + } if IsAuthError(err) { return 3 } @@ -72,12 +78,28 @@ func PrintError(w io.Writer, err error) { } // PrintErrorEnvelope writes err as a JSON envelope on w. Used in agent mode / -// --json / --format=json output so failures stay machine-parseable. +// --json / --format=json output so failures stay machine-parseable. When the +// error carries an OperationRisk (destructive write paths), it's surfaced as +// the envelope-level Risk field so agents can decide whether to surface the +// failure differently to the user. func PrintErrorEnvelope(w io.Writer, err error) { if err == nil || errors.Is(err, SilentError) { return } - _ = format.WriteEnvelope(w, format.Failure(ToErrorBody(err))) + env := format.Failure(ToErrorBody(err)) + if r := operationRiskOf(err); r != nil { + env.Risk = &format.Risk{Level: format.RiskLevel(r.Level), Action: r.Action} + } + _ = format.WriteEnvelope(w, env) +} + +// operationRiskOf extracts an OperationRisk from a typed *Error chain, or nil. +func operationRiskOf(err error) *OperationRisk { + var typed *Error + if errors.As(err, &typed) { + return typed.OperationRisk + } + return nil } // ToErrorBody projects err into the canonical envelope ErrorBody. Exposed so @@ -145,6 +167,8 @@ func defaultHint(code ErrorCode) string { return "verify the resource ID; list available with `weknora kb list`" case CodeInputInvalidArgument, CodeInputMissingFlag: return "see `weknora --help` for valid usage" + case CodeInputConfirmationRequired: + return "high-risk write — re-run with -y/--yes after the user explicitly approves" case CodeLocalKeychainDenied: return "verify keyring access; falls back to file storage" case CodeLocalConfigCorrupt: @@ -160,7 +184,7 @@ func defaultHint(code ErrorCode) string { case CodeProjectLinkCorrupt: return "remove .weknora/project.yaml and run `weknora init` again" case CodeUserAborted: - return "no action taken; pass --force to skip the confirmation prompt" + return "no action taken; pass -y/--yes to skip the confirmation prompt" case CodeUploadFileNotFound: return "verify the path is correct and readable" case CodeSSEStreamAborted: diff --git a/cli/internal/cmdutil/factory.go b/cli/internal/cmdutil/factory.go index 0d098e52..cc28bc29 100644 --- a/cli/internal/cmdutil/factory.go +++ b/cli/internal/cmdutil/factory.go @@ -17,7 +17,7 @@ import ( ) // Factory is the dependency container injected at command construction. Each -// closure is lazy: --help / completion / `weknora schema` must NOT trigger +// closure is lazy: --help / completion / `weknora version` must NOT trigger // HTTP, keyring access, or filesystem I/O beyond the bare minimum. // // Four closures (ADR-4): diff --git a/cli/internal/format/envelope.go b/cli/internal/format/envelope.go index 5f5049a8..d0c60ed2 100644 --- a/cli/internal/format/envelope.go +++ b/cli/internal/format/envelope.go @@ -10,11 +10,57 @@ import ( ) // Envelope is the canonical success/failure shape returned by every command. +// +// v0.2 ADR-3 added Notice / Risk / DryRun, borrowed from lark-cli's envelope +// (https://github.com/larksuite/cli/blob/main/internal/output/envelope.go). +// All three are absent on read-only commands; write commands populate Risk +// even on success so agents can record what action ran. type Envelope struct { - OK bool `json:"ok"` - Data any `json:"data,omitempty"` - Error *ErrorBody `json:"error,omitempty"` - Meta *Meta `json:"_meta,omitempty"` + OK bool `json:"ok"` + Data any `json:"data,omitempty"` + Error *ErrorBody `json:"error,omitempty"` + Meta *Meta `json:"_meta,omitempty"` + Notice *Notice `json:"_notice,omitempty"` + Risk *Risk `json:"risk,omitempty"` + // DryRun is intentionally NOT omitempty: AGENTS.md documents + // `dry_run: false` as the literal default in the schema example, so + // agents that pin the field's presence aren't surprised by it + // disappearing on non-dry-run envelopes. + DryRun bool `json:"dry_run"` +} + +// Notice carries system-level advisories independent of the command outcome: +// CLI update available, server-CLI version skew, etc. Agents read these to +// surface upgrade prompts to users without polluting `data`. +type Notice struct { + Update *UpdateNotice `json:"update,omitempty"` + VersionSkew *VersionSkewNotice `json:"version_skew,omitempty"` +} + +// UpdateNotice indicates a newer CLI version is available. +type UpdateNotice struct { + Available bool `json:"available"` + Current string `json:"current"` + Latest string `json:"latest,omitempty"` +} + +// VersionSkewNotice indicates the server is behind the CLI within the compat +// window. `Level` mirrors doctor's status semantics: "warn" (degraded but +// functional) or "error" (out of compat). +type VersionSkewNotice struct { + Client string `json:"client"` + Server string `json:"server"` + Level string `json:"level"` +} + +// Risk classifies the operation the user is performing — not the error. +// Agents inspect this on every envelope. When Level == RiskHighRiskWrite and +// the operation requires confirmation (no -y), the CLI exits 10. See +// cli/AGENTS.md "Exit codes" and lark-cli's +// skills/lark-shared/SKILL.md. +type Risk struct { + Level RiskLevel `json:"level"` + Action string `json:"action,omitempty"` } // Meta carries non-payload context fields useful to agents and observability. @@ -29,26 +75,27 @@ type Meta struct { AppliedFilters []string `json:"applied_filters,omitempty"` } -// RiskLevel classifies an error by the kind of operation that produced it. -// Agents use this to decide whether to retry, escalate, or stop. +// RiskLevel classifies an operation. Agents use this to decide whether to +// retry, require explicit user approval, or stop. Values are aligned with +// lark-cli's risk taxonomy +// (https://github.com/larksuite/cli/blob/main/internal/output/envelope.go). type RiskLevel string const ( - RiskRead RiskLevel = "read" - RiskMutating RiskLevel = "mutating" - RiskDestructive RiskLevel = "destructive" + RiskRead RiskLevel = "read" + RiskWrite RiskLevel = "write" + RiskHighRiskWrite RiskLevel = "high-risk-write" ) // ErrorBody is the failure shape. `code` is a stable namespaced ID (e.g. -// "auth.unauthenticated"); `hint` is an actionable next step; `risk` flags -// destructive intent for agent self-governance. +// "auth.unauthenticated"); `hint` is an actionable next step. Operation-level +// risk lives at the envelope level (Envelope.Risk), not here. type ErrorBody struct { Code string `json:"code"` Message string `json:"message"` Hint string `json:"hint,omitempty"` RequestID string `json:"request_id,omitempty"` Context string `json:"context,omitempty"` - Risk RiskLevel `json:"risk,omitempty"` Retryable bool `json:"retryable,omitempty"` ConsoleURL string `json:"console_url,omitempty"` Details map[string]any `json:"details,omitempty"` @@ -56,7 +103,7 @@ type ErrorBody struct { // WriteEnvelope serializes env as one-line JSON to w. Used for non-TTY output // and for `--json` per-command mode (the omnibus `--agent` mode-switch was -// removed in v0.2 ADR-3; see docs/superpowers/specs/2026-05-09). +// removed in v0.2 ADR-3; see cli/AGENTS.md for the agent contract). func WriteEnvelope(w io.Writer, env Envelope) error { enc := json.NewEncoder(w) enc.SetEscapeHTML(false) @@ -68,6 +115,14 @@ func Success(data any, meta *Meta) Envelope { return Envelope{OK: true, Data: data, Meta: meta} } +// SuccessWithRisk is Success + a per-operation Risk classification. Used by +// every write command (kb create/delete, doc upload/delete, api POST/PUT/ +// DELETE, ...) so an agent reading any envelope can tell what kind of +// operation produced it without parsing the data shape. +func SuccessWithRisk(data any, meta *Meta, risk *Risk) Envelope { + return Envelope{OK: true, Data: data, Meta: meta, Risk: risk} +} + // Failure constructs a failure envelope. func Failure(err *ErrorBody) Envelope { return Envelope{OK: false, Error: err} diff --git a/cli/internal/format/envelope_test.go b/cli/internal/format/envelope_test.go index 97a77e27..fd358efc 100644 --- a/cli/internal/format/envelope_test.go +++ b/cli/internal/format/envelope_test.go @@ -26,7 +26,6 @@ func TestFailureEnvelope(t *testing.T) { Code: "auth.unauthenticated", Message: "no creds", Hint: "run weknora auth login", - Risk: RiskRead, Retryable: false, }) var buf bytes.Buffer @@ -37,7 +36,23 @@ func TestFailureEnvelope(t *testing.T) { errBody := got["error"].(map[string]any) assert.Equal(t, "auth.unauthenticated", errBody["code"]) assert.Equal(t, "run weknora auth login", errBody["hint"]) - assert.Equal(t, "read", errBody["risk"]) +} + +func TestEnvelope_RiskAndNotice(t *testing.T) { + env := Success(map[string]string{"id": "kb_x"}, nil) + env.Risk = &Risk{Level: RiskHighRiskWrite, Action: "delete kb_x"} + env.Notice = &Notice{Update: &UpdateNotice{Available: true, Current: "0.2.0", Latest: "0.3.0"}} + var buf bytes.Buffer + require.NoError(t, WriteEnvelope(&buf, env)) + var got map[string]any + require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) + risk := got["risk"].(map[string]any) + assert.Equal(t, "high-risk-write", risk["level"]) + assert.Equal(t, "delete kb_x", risk["action"]) + notice := got["_notice"].(map[string]any) + upd := notice["update"].(map[string]any) + assert.Equal(t, true, upd["available"]) + assert.Equal(t, "0.3.0", upd["latest"]) } func TestEnvelope_NoEscapeHTML(t *testing.T) {