diff --git a/cli/AGENTS.md b/cli/AGENTS.md index d524d45d..c6f7651d 100644 --- a/cli/AGENTS.md +++ b/cli/AGENTS.md @@ -11,9 +11,9 @@ > > **Naming note.** "Agent" appears in two distinct WeKnora contexts: > -> - **This file (`AGENTS.md`) + the `agent` annotation on each command's -> `--help`**: documents the contract for AI coding agents (you, the -> LLM-driven CLI consumer). +> - **This file (`AGENTS.md`) + the `agent_help` annotation on each +> command's `--help`**: documents the contract for AI coding agents +> (you, the LLM-driven CLI consumer). > - **The `weknora agent` subtree** (`agent list / view / invoke`): > manages WeKnora's first-class *Custom Agent* resources — server-side > records (system prompt + model + allowed tools + KB scope) that the @@ -22,11 +22,11 @@ > coding agent, drive WeKnora — that's `kb` / `doc` / `search` / `chat` > / `mcp serve`. -`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. +`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 "Output contract" and "Behavioral rules" sections below are the self-contained specification of the wire format; everything an integrator @@ -38,52 +38,102 @@ needs is in this document. ### 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. +- **stdout** is the data channel: bare JSON (with `--json`) or + human-formatted output (without `--json`). Never carries error text. +- **stderr** is logs / progress / warnings / errors / agent guidance + footnotes. On failure, the error message + actionable hint go here. A non-empty stderr does **not** mean failure — read the exit code instead. -### JSON envelope +### JSON output (bare data) -When `--json` is set, stdout contains exactly one envelope: +When `--json` is set on a successful command, stdout contains exactly +one JSON value matching the resource the command produces: -```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 -} +| Command shape | stdout JSON | +|---|---| +| `list` / `search` | `[ { …resource… }, … ]` (bare array; `[]` when empty) | +| `view` / `create` / `edit` | `{ …resource… }` (bare object) | +| `delete` / `pin` / write ops | `{ id, …action-result fields… }` (bare object) | +| `doctor` | `{ summary: { all_passed, passed, warned, failed, skipped }, checks: [ … ] }` | + +There is no `ok` / `data` / `error` wrapper. Agents read the resource +shape directly. Successful runs always return exit 0; failures never +emit data JSON on stdout. + +#### Field selection: `--json=field,field,…` + +`--json` accepts a comma-separated field list (passed with `=`) to restrict +each top-level object (or each element of a top-level array) to the named +keys: + +```bash +weknora kb list --json=id,name # [{ "id": "kb_x", "name": "Eng" }, …] +weknora kb view kb_x --json=id # { "id": "kb_x" } ``` -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`. +Note the `=` form: pflag's optional-value parser treats space-separated +arguments after a bare `--json` as positionals, so `--json id,name` would +be interpreted as bare `--json` plus the positional `id,name`. Always use +`--json=field,...` for projection. -### Error codes (closed registry) +Unknown field names are silently dropped so you can pass an aspirational +field set across heterogenous outputs. -`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. +#### jq pipeline: `--jq ` -Categories: `auth.*` / `resource.*` / `input.*` / `server.*` / `network.*` / -`local.*` / `mcp.*`. +`--jq` applies a jq expression to the JSON before printing. The +expression sees the same bare shape the command produces (no envelope +indirection). String results render without quotes for shell-friendly +substitution; non-string results render as JSON. -`error.hint` provides a deterministic next-step hint agents can follow -without natural-language parsing. +```bash +weknora kb list --jq '.[].id' # one id per line +weknora kb view kb_x --jq .name # bare name +weknora search chunks "x" --kb e --jq '.[].score' # scores per line +``` + +`--jq` requires `--json`. Combining with `--json=id,name` is fine — the +filter runs after the field projection. + +### Errors (stderr, exit code carries the class) + +On failure, stdout is empty (or holds the partial-success output the +command already wrote before the failure). The error message goes to +stderr in this format: + +``` +: [: ] +hint: +``` + +- `` 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. +- `` is the human-readable description. +- `` is the deterministic next-step hint (omitted when no hint + applies). + +Code namespaces: `auth.*` / `resource.*` / `input.*` / `server.*` / +`network.*` / `local.*` / `mcp.*`. + +To pattern-match programmatically: split on the first `:` to extract +the code, or branch on the exit code (see below). ### Exit codes | Code | Meaning | Agent action | |---|---|---| | `0` | Success | Continue | -| `1` | Typed error (see envelope.error.code) | Read code, decide retry/abort | +| `1` | Typed `local.*` error or unclassified | Read stderr, 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 | +| `3` | `auth.*` (token missing / expired / forbidden) | Re-auth then retry | +| `4` | `resource.not_found` | Verify the resource id | +| `5` | `input.*` (other than confirmation_required) | Adjust args and retry | +| `6` | `server.rate_limited` | Back off, then retry | +| `7` | `server.*` / `network.*` | Transient; retry with backoff | +| `10` | **`input.confirmation_required`** — high-risk write needs `-y` | Ask the human, retry with `-y` only after explicit approval | | `130` | Cancelled (SIGINT / Ctrl-C) | Stop, do not retry | Exit 10 is the wire-level signal for "high-risk write needs explicit @@ -123,11 +173,11 @@ The command tree follows ` `. Verbs are: bundle host + tenant + credentials, so they need a richer abstraction than a single per-host token slot). `auth refresh` exchanges the stored refresh token for a new access + refresh pair (OAuth refresh-token -grant); it -errors with `input.invalid_argument` on API-key contexts which have no -refresh semantic. Transparent 401 → refresh → retry is wired into the -SDK transport (`cli/internal/cmdutil/authretry.go`) with singleflight -de-dup, so most callers never need to invoke `auth refresh` explicitly. +grant); it errors with `input.invalid_argument` on API-key contexts +which have no refresh semantic. Transparent 401 → refresh → retry is +wired into the SDK transport (`cli/internal/cmdutil/authretry.go`) +with singleflight de-dup, so most callers never need to invoke `auth +refresh` explicitly. `search` subtree: `search chunks "" --kb X` for hybrid retrieval; `search kb ""` / `search docs "" --kb X` / `search sessions ""` @@ -141,29 +191,31 @@ Top-level RAG / connectivity verbs: `chat`, `search`, `api`, `link`, `doctor` is a deliberate WeKnora addition: RAG deployments routinely break on misconfigured embeddings, storage backends, and credentials, -and a structured 4-status envelope (ok/warn/fail/skip) is the cleanest -agent-readable surface for that. +and the structured `{summary: {all_passed, passed, warned, failed, +skipped}, checks: [...]}` JSON shape is the cleanest agent-readable +surface for that. --- ## Behavioral rules Per-command guidance also appears in each command's `--help` output -(under "AI Agent guidance:"). +(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. +1. **Pass `-y/--yes`** on destructive writes (`kb delete` / `kb empty` / + `doc delete` / `session delete` / `context remove` when targeting + the current context) 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. **`link` writes to the user's working directory** — only run it when - the user invoked it, not as a side effect of unrelated automation. + 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 parsing impossible. +4. **`link` 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`.) @@ -185,11 +237,9 @@ 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 in favor of per-command `--json` + TTY auto-detect, -which covers the same ground without an extra global switch. Agent -detection (`CLAUDECODE` / `CURSOR_AGENT` env) only tags the User-Agent -header for server-side telemetry — it never changes CLI behavior. +Agent detection (`CLAUDECODE` / `CURSOR_AGENT` env) also tags the +User-Agent header for server-side telemetry — it never changes CLI +behavior. --- @@ -198,11 +248,17 @@ header for server-side telemetry — it never changes CLI behavior. A handful of decisions are referenced inline in the source as `ADR-N`. They live here, alongside the contract they shape. -**ADR-3 — opinionated noun-verb tree with stable JSON envelope.** The -v0.0/v0.1 surface was audited against several mainstream CLIs; the -"opinionated noun-verb tool with stable JSON envelope + agent-aware -error model" shape was the closest fit for the agent-friendly contract -this document promises. WeKnora-specific shape choices: +**ADR-3 — bare-data JSON on stdout, errors on stderr.** Successful +commands emit the raw resource shape (`[]Item` for lists, `T` for views, +`{id, deleted: true}` for deletes) — no `ok` / `data` / `error` +wrapper. Errors are not data; they go to stderr in `code: message\nhint: +…` form, and the typed exit code carries the failure class for +programmatic branching. This separates "what the command produces" from +"how the run went", so `--json | jq` pipelines never have to filter +error shapes out of the success stream, and matches the contract of +gh / aws / stripe. + +WeKnora-specific shape choices: - `link` (project-binding) — `/.weknora/project.yaml` walk-up matches how RAG users scope work to a specific knowledge base. There @@ -213,10 +269,12 @@ this document promises. WeKnora-specific shape choices: - `context use` switches the active credential set; contexts bundle host + tenant + credential, so a richer abstraction than a single per-host token slot is required. -- `doctor` (4-status: ok / warn / fail / skip) is the agent-readable - surface for RAG-deployment misconfiguration (embeddings, storage, - credentials) — failure modes that the underlying SDK can't classify - on its own. +- `doctor` (4-status: ok / warn / fail / skip per check, plus a + summary object) is the agent-readable surface for RAG-deployment + misconfiguration (embeddings, storage, credentials) — failure modes + that the underlying SDK can't classify on its own. Agents short- + circuit on `summary.all_passed`; exit code is non-zero iff any + check is `fail`. Verb canon: `list / view / create / edit / delete / upload / download / pin / unpin / use`. WeKnora-specific verbs for resource semantics @@ -238,19 +296,20 @@ SDK, and the dependency graph of any one command is visible in one file. ## Known limitations -The following classes of failure currently surface as `error.code = "network.error"` -with `context deadline exceeded` rather than a precise typed code. A future -release will introduce a `precondition.*` namespace (server returns HTTP 412 -with a typed remediation body before opening the SSE / streaming response): +The following classes of failure currently surface as `error.code = +"network.error"` with `context deadline exceeded` rather than a precise +typed code. A future release will introduce a `precondition.*` +namespace (server returns HTTP 412 with a typed remediation body before +opening the SSE / streaming response): - `weknora chat` when no chat model is configured for the active tenant - `weknora search chunks` when no retriever / vector store is configured - `weknora doc upload` when no storage engine is selected for the KB -Workaround until then: if a chat / search / upload call times out without -producing a first-byte response, check the server's tenant configuration -(LLM / vector store / storage engine) before retrying. A planned -`weknora doctor --server-config` will probe these directly. +Workaround until then: if a chat / search / upload call times out +without producing a first-byte response, check the server's tenant +configuration (LLM / vector store / storage engine) before retrying. A +planned `weknora doctor --server-config` will probe these directly. --- @@ -261,4 +320,5 @@ https://github.com/Tencent/WeKnora/issues with: - The exact command line - `weknora --version` output -- The envelope you got vs the envelope this document promises +- The output (stdout + stderr) you got vs the output this document + promises diff --git a/cli/README.md b/cli/README.md index de212216..ad516b47 100644 --- a/cli/README.md +++ b/cli/README.md @@ -25,8 +25,9 @@ Available Commands: The command surface mirrors `gh` CLI's ` ` convention. See [AGENTS.md](AGENTS.md) for the operational contract that AI agents -(Claude Code, Cursor, Aider, …) can rely on: envelope schema, exit-code -protocol, error-code registry, and per-command guidance. +(Claude Code, Cursor, Aider, …) can rely on: bare-JSON output shape, +stderr error format, exit-code protocol, error-code registry, and +per-command guidance. --- @@ -105,33 +106,30 @@ weknora auth logout --all --- -## JSON envelope output +## JSON output -Every command supports `--json`, returning a stable envelope shape: +Every command supports `--json`, emitting bare JSON for the resource it +produces — an array for `list` / `search`, a single object for `view` +and write outcomes: -```json -{ - "ok": true, - "data": { /* command-specific payload */ }, - "_meta": { "context": "prod", "kb_id": "a32a63ff-fb36-4874-bcaa-30f48570a694" } -} +```bash +weknora kb list --json # [{ "id": "kb_x", "name": "Eng" }, …] +weknora kb view kb_x --json # { "id": "kb_x", "name": "Eng", … } +weknora kb list --json=id,name # project to listed fields +weknora kb list --json --jq '.[].id' # jq over the bare data ``` -On error: +On failure, stdout stays empty and the typed error goes to stderr in +`code: message\nhint: ...` form: -```json -{ - "ok": false, - "error": { - "code": "auth.unauthenticated", - "message": "...", - "hint": "run `weknora auth login`" - } -} +``` +auth.unauthenticated: fetch current user: HTTP error 401: ... +hint: run `weknora auth login` ``` -The full schema, error-code registry, and exit-code protocol (0 / 1 / 2 / 10 -/ 130) are documented in [AGENTS.md](AGENTS.md). +The typed exit code (3 / 4 / 5 / 6 / 7 / 10) carries the failure +class for agents that branch on it. The full error-code registry and +exit-code protocol are documented in [AGENTS.md](AGENTS.md). --- diff --git a/cli/acceptance/contract/helpers_test.go b/cli/acceptance/contract/helpers_test.go index 72878802..95c37a3e 100644 --- a/cli/acceptance/contract/helpers_test.go +++ b/cli/acceptance/contract/helpers_test.go @@ -21,7 +21,7 @@ import ( // TestMain pins the doctor credential-storage outcome for the whole suite. // Otherwise the check probes the real OS keyring, which differs between // macOS dev machines (Keychain present → ok) and Linux CI runners without -// libsecret (file fallback → warn), making golden envelopes host-dependent. +// libsecret (file fallback → warn), making golden outputs host-dependent. // MemStore is neither *FileStore nor a real keyring, so the doctor's // type-switch hits the StatusOK branch. func TestMain(m *testing.M) { @@ -62,13 +62,9 @@ func newTestFactory(t *testing.T, mockServer *httptest.Server, mockClient *sdk.C // runCmd executes the root command in-process and returns captured stdout/stderr. // Replaces iostreams.IO singleton via SetForTest (auto-restored in t.Cleanup). // -// Mirrors cmd.Execute() carefully: callers expect the same envelope-printing -// behavior the real entrypoint provides. The helper (a) wires the cobra Out / -// Err sinks to the same buffers it returns (the `version` leaf and any future -// command using c.OutOrStdout would otherwise leak to os.Stdout), and (b) -// re-runs the error-envelope path so failure cases produce the JSON envelope -// the contract test compares against. Without (b), every error scenario's -// golden would be empty. +// Mirrors cmd.Execute(): wires the cobra Out / Err sinks to the same buffers +// it returns, and re-runs cmdutil.PrintError on stderr for failure cases so +// the contract assertion sees the typed `code: message\nhint: ...` line. func runCmd(t *testing.T, f *cmdutil.Factory, args ...string) (stdout, stderr string, exitCode int) { t.Helper() out, errBuf := iostreams.SetForTest(t) @@ -77,31 +73,25 @@ func runCmd(t *testing.T, f *cmdutil.Factory, args ...string) (stdout, stderr st root.SetContext(context.Background()) root.SetOut(out) root.SetErr(errBuf) - leaf, err := root.ExecuteC() + _, err := root.ExecuteC() if err != nil { err = cmd.MapCobraError(err) - if cmd.WantsJSONOutput(leaf) { - cmdutil.PrintErrorEnvelope(iostreams.IO.Out, err) - } else { - cmdutil.PrintError(iostreams.IO.Err, err) - } + cmdutil.PrintError(iostreams.IO.Err, err) } return out.String(), errBuf.String(), cmdutil.ExitCode(err) } // assertGolden compares got against the JSON golden file at path. -// With -update, writes got to path. Normalizes _meta.request_id to "" -// before compare (only field known unstable in v0.0). +// With -update, writes got to path. // // CRLF normalization: Windows checkouts with the default core.autocrlf=true // turn LF in tracked text files into CRLF on disk. The command output is -// always LF, so byte-equal would fail despite identical content. .gitattributes -// is the primary defense (forcing LF on testdata/**/*.json), but we also -// strip CR here so a misconfigured contributor checkout doesn't break the -// suite locally before they push. +// always LF, so byte-equal would fail despite identical content. +// .gitattributes is the primary defense (forcing LF on testdata/**/*.json), +// but we also strip CR here so a misconfigured contributor checkout doesn't +// break the suite locally before they push. func assertGolden(t *testing.T, got []byte, path string) { t.Helper() - got = normalizeEnvelope(got) if *update { if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { t.Fatalf("mkdir testdata: %v", err) @@ -118,7 +108,7 @@ func assertGolden(t *testing.T, got []byte, path string) { want = stripCR(want) got = stripCR(got) if !bytes.Equal(want, got) { - t.Errorf("envelope mismatch for %s\nwant:\n%s\ngot:\n%s", path, want, got) + t.Errorf("stdout mismatch for %s\nwant:\n%s\ngot:\n%s", path, want, got) } } @@ -127,10 +117,3 @@ func assertGolden(t *testing.T, got []byte, path string) { func stripCR(b []byte) []byte { return bytes.ReplaceAll(b, []byte{'\r'}, nil) } - -// normalizeEnvelope replaces unstable fields with placeholders for stable diff. -// Currently no-op (v0.0 commands don't set _meta.request_id, so output is stable). -// Hook for future fields. -func normalizeEnvelope(b []byte) []byte { - return b -} diff --git a/cli/acceptance/contract/envelope_test.go b/cli/acceptance/contract/wire_test.go similarity index 63% rename from cli/acceptance/contract/envelope_test.go rename to cli/acceptance/contract/wire_test.go index 9e659129..6b2332db 100644 --- a/cli/acceptance/contract/envelope_test.go +++ b/cli/acceptance/contract/wire_test.go @@ -1,39 +1,34 @@ -// cli/acceptance/contract/envelope_test.go +// cli/acceptance/contract/wire_test.go // -// Envelope contract test (PR-8 Task 18). Drives root cobra in-process for -// each scenario, captures stdout, and compares against a JSON golden file -// in cli/acceptance/testdata/envelopes/. +// Wire contract test. Drives root cobra in-process for each scenario, +// captures stdout + stderr, and asserts: // -// Spec §4.1 lists 19 envelope scenarios. Implemented count: 16. +// - stdout matches a JSON golden in cli/acceptance/testdata/wire/ +// - on wantErr cases, stderr contains the expected typed error code // -// Cases dropped (with reason): -// - doctor.success — non-offline path emits unstable -// timing ("reachable in 2ms"). -// Unit tests in cli/cmd/doctor -// cover the all-pass shape; -// doctor.success_offline is the -// deterministic sibling kept here. -// - auth_login.success — requires stdin pipe -// (--with-token) + keyring-aware -// Secrets store; helpers_test -// (PR-6) does not yet expose a -// stdin hook. Deferred to the e2e harness. -// - auth_login.error_auth_unauthenticated — same setup as above; deferred -// together. -// - context_use.error_local_context_not_found — `context use` has no --json -// flag in v0.1, so error path -// renders plain stderr. Pinning -// its envelope shape needs either -// a --json flag added to the leaf -// or a global --json. Deferred -// until that lands; the success -// case is golden-pinned (writes -// envelope unconditionally). +// Successful cases produce bare JSON on stdout (no envelope wrapper); +// failure cases produce empty stdout (or, for `doctor`, the data object +// the command writes before returning SilentError) and a `code: msg` +// line on stderr. // -// All cases use leaf-positioned --json (e.g. `version --json`) instead of the -// `--json version` form sketched in the spec. v0.0–v0.1 implements --json as a -// per-leaf flag, not a global persistent flag — root-level --json is detected -// only as an error-envelope fallback (see argsRequestJSON in cmd/root.go). +// Cases intentionally omitted (with reason): +// - doctor.success — non-offline path emits +// unstable timing +// ("reachable in 2ms"). +// Unit tests in cli/cmd/doctor +// cover the all-pass shape; +// doctor.success_offline is +// the deterministic sibling +// kept here. +// - auth_login.success — requires stdin pipe +// (--with-token) + keyring- +// aware Secrets store; the +// helper does not yet expose +// a stdin hook. +// - auth_login.error_auth_unauthenticated — same setup as above. +// +// All cases use leaf-positioned --json (e.g. `version --json`). --json is a +// per-leaf flag, not a global persistent flag. package contract_test import ( @@ -49,23 +44,28 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// envelopeCase declares one row in the contract matrix. Optional fields: -// server — mock /api/v1/* endpoints; nil means no network needed. -// preConfig — seed config.yaml under the per-test XDG_CONFIG_HOME (set by -// newTestFactory); use for cases like context use that read -// local state without an SDK round-trip. -// wantErr — true means the run is expected to exit non-zero. -type envelopeCase struct { - name string - args []string - server http.HandlerFunc - preConfig func(t *testing.T) - wantErr bool +// wireCase declares one row in the contract matrix. Optional fields: +// server — mock /api/v1/* endpoints; nil means no network. +// preConfig — seed config.yaml under the per-test XDG_CONFIG_HOME +// (set by newTestFactory); use for cases like +// `context use` that read local state without an +// SDK round-trip. +// wantErr — non-zero exit expected. +// wantStderrSubstring — stderr must contain this substring (typically the +// typed error code, e.g. "auth.unauthenticated"). +// Only meaningful when wantErr=true. +type wireCase struct { + name string + args []string + server http.HandlerFunc + preConfig func(t *testing.T) + wantErr bool + wantStderrSubstring string } -// envelopeCases enumerates every contract scenario whose envelope is golden- -// pinned. Order is illustrative (matches spec §4.1 mostly), not load-bearing. -var envelopeCases = []envelopeCase{ +// wireCases enumerates every contract scenario whose stdout is golden-pinned. +// Order is illustrative, not load-bearing. +var wireCases = []wireCase{ // 1. version.success — pure local; no client touched. { name: "version.success", @@ -82,9 +82,9 @@ var envelopeCases = []envelopeCase{ // 3. doctor.error_network — base_url returns 500 → ping fail → cascade // skip on auth_credential + server_version. credential_storage still - // runs (independent). v0.2 contract: any check=fail flips envelope.ok - // to false and exits 1 (RunE returns SilentError so the data envelope - // written by emit() is preserved as the only stdout content). + // runs (independent). Contract: any check=fail bumps summary.failed + // and RunE returns SilentError → exit 1 with the data object + // written by emit() as the only stdout content. { name: "doctor.error_network", args: []string{"doctor", "--json"}, @@ -104,10 +104,11 @@ var envelopeCases = []envelopeCase{ server: kbListEmpty, }, { - name: "kb_list.error_auth_forbidden", - args: []string{"kb", "list", "--json"}, - server: always403, - wantErr: true, + name: "kb_list.error_auth_forbidden", + args: []string{"kb", "list", "--json"}, + server: always403, + wantErr: true, + wantStderrSubstring: "auth.forbidden", }, { name: "kb_view.success", @@ -115,16 +116,17 @@ var envelopeCases = []envelopeCase{ server: kbGetOne, }, { - name: "kb_view.error_resource_not_found", - args: []string{"kb", "view", "missing", "--json"}, - server: always404, - wantErr: true, + name: "kb_view.error_resource_not_found", + args: []string{"kb", "view", "missing", "--json"}, + server: always404, + wantErr: true, + wantStderrSubstring: "resource.not_found", }, // 8. context use — pure local I/O against config.yaml. { name: "context_use.success", - args: []string{"context", "use", "production"}, + args: []string{"context", "use", "production", "--json"}, preConfig: func(t *testing.T) { cfg := &config.Config{ CurrentContext: "staging", @@ -147,10 +149,11 @@ var envelopeCases = []envelopeCase{ server: whoamiOK, }, { - name: "auth_status.error_auth_unauthenticated", - args: []string{"auth", "status", "--json"}, - server: always401, - wantErr: true, + name: "auth_status.error_auth_unauthenticated", + args: []string{"auth", "status", "--json"}, + server: always401, + wantErr: true, + wantStderrSubstring: "auth.unauthenticated", }, // 11-13. search chunks — verb-noun shape (gh search parity), positional query, --kb required. @@ -163,26 +166,28 @@ var envelopeCases = []envelopeCase{ server: searchTwoResults, }, { - name: "search.error_resource_not_found", - args: []string{"search", "chunks", "query", "--kb=eeeeeeee-eeee-4eee-8eee-eeeeeeeeeeee", "--json"}, - server: always404, - wantErr: true, + name: "search.error_resource_not_found", + args: []string{"search", "chunks", "query", "--kb=eeeeeeee-eeee-4eee-8eee-eeeeeeeeeeee", "--json"}, + server: always404, + wantErr: true, + wantStderrSubstring: "resource.not_found", }, { // --no-vector + --no-keyword is the input.invalid case; the KB UUID // is just there to satisfy MarkFlagRequired so validation runs deep // enough to hit the mutex-channel check. - name: "search.error_input_invalid", - args: []string{"search", "chunks", "query", "--kb=11111111-1111-4111-8111-111111111111", "--no-vector", "--no-keyword", "--json"}, - wantErr: true, + name: "search.error_input_invalid", + args: []string{"search", "chunks", "query", "--kb=11111111-1111-4111-8111-111111111111", "--no-vector", "--no-keyword", "--json"}, + wantErr: true, + wantStderrSubstring: "input.invalid_argument", }, } -// TestEnvelopeGolden is the matrix-runner. Cases are sequential (the -// iostreams singleton swap inside helpers.runCmd is package-global; t.Parallel -// is contractually forbidden — see helpers_test.go SetForTest comment). -func TestEnvelopeGolden(t *testing.T) { - for _, tc := range envelopeCases { +// TestWireGolden is the matrix-runner. Cases are sequential (the +// iostreams singleton swap inside helpers.runCmd is package-global; +// t.Parallel is contractually forbidden — see helpers_test.go). +func TestWireGolden(t *testing.T) { + for _, tc := range wireCases { t.Run(tc.name, func(t *testing.T) { var ts *httptest.Server var mockClient *sdk.Client @@ -202,7 +207,10 @@ func TestEnvelopeGolden(t *testing.T) { if !tc.wantErr && exit != 0 { t.Errorf("unexpected non-zero exit %d; stdout=%q stderr=%q", exit, stdout, stderr) } - path := filepath.Join("..", "testdata", "envelopes", tc.name+".json") + if tc.wantStderrSubstring != "" && !strings.Contains(stderr, tc.wantStderrSubstring) { + t.Errorf("stderr missing %q; got %q", tc.wantStderrSubstring, stderr) + } + path := filepath.Join("..", "testdata", "wire", tc.name+".json") assertGolden(t, []byte(stdout), path) }) } diff --git a/cli/acceptance/e2e/e2e_test.go b/cli/acceptance/e2e/e2e_test.go index 3b1b751f..9a94ccbe 100644 --- a/cli/acceptance/e2e/e2e_test.go +++ b/cli/acceptance/e2e/e2e_test.go @@ -31,7 +31,7 @@ import ( // TestRAGFullLoop walks the demo MVP path: link a context, create a KB, // upload a doc, wait for indexing, search it, then chat against it. Each -// step parses the CLI's JSON envelope to extract IDs for the next step — +// step parses the CLI's bare JSON to extract IDs for the next step — // validating both functional behavior and wire-contract stability. func TestRAGFullLoop(t *testing.T) { host := mustEnv(t, "WEKNORA_E2E_HOST") @@ -45,75 +45,69 @@ func TestRAGFullLoop(t *testing.T) { env := append(os.Environ(), "XDG_CONFIG_HOME="+xdg, "XDG_CACHE_HOME="+filepath.Join(xdg, "cache"), - // SDK debug off — explicit so the CI run isn't noisy. C1 SDK silence - // makes this redundant in practice but the explicit flag documents - // intent. + // SDK debug off — explicit so the CI run isn't noisy. "WEKNORA_SDK_DEBUG=", ) - // 1. kb create + // 1. kb create → bare KnowledgeBase object kbName := prefix + fmt.Sprintf("%d", time.Now().UnixNano()) - createOut := runJSON(t, bin, env, "kb", "create", "--name", kbName, "--json") - kbData, ok := createOut["data"].(map[string]any) - if !ok { - t.Fatalf("kb create envelope: data not an object: %v", createOut) + var created struct { + ID string `json:"id"` + Name string `json:"name"` } - kbID, _ := kbData["id"].(string) - if kbID == "" { - t.Fatalf("kb create returned no id: %v", createOut) + runJSONInto(t, bin, env, &created, "kb", "create", "--name", kbName, "--json") + if created.ID == "" { + t.Fatalf("kb create returned no id") } - t.Logf("created KB: %s (%s)", kbID, kbName) + t.Logf("created KB: %s (%s)", created.ID, kbName) t.Cleanup(func() { // Best-effort cleanup; a 404 means the KB was already gone. - out, err := run(bin, env, "kb", "delete", kbID, "-y", "--json") + out, err := run(bin, env, "kb", "delete", created.ID, "-y", "--json") if err != nil { t.Logf("cleanup kb delete: %v\n%s", err, out) } }) - // 2. doc upload + // 2. doc upload → bare Knowledge object docPath := writeSampleDoc(t) - uploadOut := runJSON(t, bin, env, "doc", "upload", docPath, "--kb", kbID, "--json") - docData, _ := uploadOut["data"].(map[string]any) - docID, _ := docData["id"].(string) - if docID == "" { - t.Fatalf("doc upload returned no id: %v", uploadOut) + var uploaded struct { + ID string `json:"id"` } - t.Logf("uploaded doc: %s", docID) + runJSONInto(t, bin, env, &uploaded, "doc", "upload", docPath, "--kb", created.ID, "--json") + if uploaded.ID == "" { + t.Fatalf("doc upload returned no id") + } + t.Logf("uploaded doc: %s", uploaded.ID) // 3. poll until indexing finishes (status changes from "pending" / "processing" to "ready" / similar) - waitDocReady(t, bin, env, kbID, docID, 90*time.Second) + waitDocReady(t, bin, env, created.ID, uploaded.ID, 90*time.Second) - // 4. search — verify retrieval returns chunks - searchOut := runJSON(t, bin, env, "search", "chunks", "sample", "--kb", kbID, "--limit", "5", "--json") - searchData, _ := searchOut["data"].(map[string]any) - results, _ := searchData["results"].([]any) + // 4. search chunks → bare []SearchResult + var results []map[string]any + runJSONInto(t, bin, env, &results, "search", "chunks", "sample", "--kb", created.ID, "--limit", "5", "--json") if len(results) == 0 { - t.Fatalf("search returned no results: %v", searchOut) + t.Fatalf("search returned no results") } t.Logf("search returned %d results", len(results)) - // 5. chat — verify LLM answer + references in --json + --no-stream mode - // (--no-stream forces accumulator path; --json gates envelope output) - chatOut := runJSON(t, bin, env, "chat", "summarize the document briefly", "--kb", kbID, "--no-stream", "--json") - chatData, _ := chatOut["data"].(map[string]any) - answer, _ := chatData["answer"].(string) - if strings.TrimSpace(answer) == "" { - t.Fatalf("chat returned empty answer: %v", chatOut) + // 5. chat --no-stream --json → bare {answer, references, ...} object + var chat struct { + Answer string `json:"answer"` + References []map[string]any `json:"references"` } - refs, _ := chatData["references"].([]any) - t.Logf("chat answer (%d chars), %d references", len(answer), len(refs)) - if len(refs) == 0 { + runJSONInto(t, bin, env, &chat, "chat", "summarize the document briefly", "--kb", created.ID, "--no-stream", "--json") + if strings.TrimSpace(chat.Answer) == "" { + t.Fatalf("chat returned empty answer") + } + t.Logf("chat answer (%d chars), %d references", len(chat.Answer), len(chat.References)) + if len(chat.References) == 0 { // Soft warning — some servers may not surface references for every // question, but the demo flow is supposed to. t.Logf("warning: chat returned 0 references (server may have a different config)") } } -// mustEnv reads an env var and skips the test if missing — keeps the -// suite friendly to community contributors who clone the repo without -// access to the maintainer's E2E secrets. func mustEnv(t *testing.T, key string) string { t.Helper() v := os.Getenv(key) @@ -203,27 +197,23 @@ func waitDocReady(t *testing.T, bin string, env []string, kbID, docID string, ti deadline := time.Now().Add(timeout) tick := 2 * time.Second for time.Now().Before(deadline) { - out := runJSON(t, bin, env, "doc", "list", "--kb", kbID, "--page-size", "100", "--json") - data, _ := out["data"].(map[string]any) - items, _ := data["items"].([]any) - for _, it := range items { - m, ok := it.(map[string]any) - if !ok { + var docs []struct { + ID string `json:"id"` + ParseStatus string `json:"parse_status"` + } + runJSONInto(t, bin, env, &docs, "doc", "list", "--kb", kbID, "--page-size", "100", "--json") + for _, d := range docs { + if d.ID != docID { continue } - id, _ := m["id"].(string) - if id != docID { - continue - } - status, _ := m["status"].(string) - low := strings.ToLower(status) + low := strings.ToLower(d.ParseStatus) switch { case low == "failed", low == "error": - t.Fatalf("doc %s indexing failed: status=%q", docID, status) + t.Fatalf("doc %s indexing failed: status=%q", docID, d.ParseStatus) case low == "pending", low == "processing", low == "": // keep waiting default: - t.Logf("doc %s ready (status=%q)", docID, status) + t.Logf("doc %s ready (status=%q)", docID, d.ParseStatus) return } } @@ -246,20 +236,16 @@ func run(bin string, env []string, args ...string) ([]byte, error) { return stdout.Bytes(), nil } -// runJSON runs the CLI expecting --json output and parses the envelope. -// Test fails immediately on non-zero exit or unparseable JSON. -func runJSON(t *testing.T, bin string, env []string, args ...string) map[string]any { +// runJSONInto runs the CLI expecting bare JSON output and decodes stdout +// into out (a struct, slice, or map pointer). Test fails on non-zero exit +// or unparseable JSON. +func runJSONInto(t *testing.T, bin string, env []string, out any, args ...string) { t.Helper() - out, err := run(bin, env, args...) + stdout, err := run(bin, env, args...) if err != nil { t.Fatalf("%v", err) } - var env_ map[string]any - if err := json.Unmarshal(out, &env_); err != nil { - t.Fatalf("parse envelope from %v: %v\nstdout:\n%s", args, err, string(out)) + if err := json.Unmarshal(stdout, out); err != nil { + t.Fatalf("parse JSON from %v: %v\nstdout:\n%s", args, err, string(stdout)) } - if ok, _ := env_["ok"].(bool); !ok { - t.Fatalf("envelope ok=false from %v: %s", args, string(out)) - } - return env_ } diff --git a/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json b/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json deleted file mode 100644 index 7cc15d93..00000000 --- a/cli/acceptance/testdata/envelopes/auth_status.error_auth_unauthenticated.json +++ /dev/null @@ -1 +0,0 @@ -{"ok":false,"error":{"code":"auth.unauthenticated","message":"fetch current user: HTTP error 401: {\"error\":\"unauthenticated\"}","hint":"run `weknora auth login`"}} diff --git a/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json b/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json deleted file mode 100644 index 0cb60cc7..00000000 --- a/cli/acceptance/testdata/envelopes/kb_list.error_auth_forbidden.json +++ /dev/null @@ -1 +0,0 @@ -{"ok":false,"error":{"code":"auth.forbidden","message":"list knowledge bases: HTTP error 403: {\"error\":\"forbidden\"}","hint":"active context lacks permission for this resource"}} diff --git a/cli/acceptance/testdata/envelopes/kb_view.error_resource_not_found.json b/cli/acceptance/testdata/envelopes/kb_view.error_resource_not_found.json deleted file mode 100644 index 9ad8b4e2..00000000 --- a/cli/acceptance/testdata/envelopes/kb_view.error_resource_not_found.json +++ /dev/null @@ -1 +0,0 @@ -{"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`"}} diff --git a/cli/acceptance/testdata/envelopes/search.error_input_invalid.json b/cli/acceptance/testdata/envelopes/search.error_input_invalid.json deleted file mode 100644 index 40c1a907..00000000 --- a/cli/acceptance/testdata/envelopes/search.error_input_invalid.json +++ /dev/null @@ -1 +0,0 @@ -{"ok":false,"error":{"code":"input.invalid_argument","message":"--no-vector and --no-keyword cannot both be set","hint":"see `weknora --help` for valid usage"}} diff --git a/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json b/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json deleted file mode 100644 index 7c4b2b6b..00000000 --- a/cli/acceptance/testdata/envelopes/search.error_resource_not_found.json +++ /dev/null @@ -1 +0,0 @@ -{"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`"}} diff --git a/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json b/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json deleted file mode 100644 index 7cc15d93..00000000 --- a/cli/acceptance/testdata/envelopes/whoami.error_auth_unauthenticated.json +++ /dev/null @@ -1 +0,0 @@ -{"ok":false,"error":{"code":"auth.unauthenticated","message":"fetch current user: HTTP error 401: {\"error\":\"unauthenticated\"}","hint":"run `weknora auth login`"}} diff --git a/cli/acceptance/testdata/envelopes/whoami.success.json b/cli/acceptance/testdata/envelopes/whoami.success.json deleted file mode 100644 index 38a284ae..00000000 --- a/cli/acceptance/testdata/envelopes/whoami.success.json +++ /dev/null @@ -1 +0,0 @@ -{"ok":true,"data":{"user_id":"usr_abc","tenant_id":42}} diff --git a/cli/acceptance/testdata/wire/auth_status.error_auth_unauthenticated.json b/cli/acceptance/testdata/wire/auth_status.error_auth_unauthenticated.json new file mode 100644 index 00000000..e69de29b diff --git a/cli/acceptance/testdata/envelopes/auth_status.success.json b/cli/acceptance/testdata/wire/auth_status.success.json similarity index 100% rename from cli/acceptance/testdata/envelopes/auth_status.success.json rename to cli/acceptance/testdata/wire/auth_status.success.json diff --git a/cli/acceptance/testdata/envelopes/context_use.success.json b/cli/acceptance/testdata/wire/context_use.success.json similarity index 100% rename from cli/acceptance/testdata/envelopes/context_use.success.json rename to cli/acceptance/testdata/wire/context_use.success.json diff --git a/cli/acceptance/testdata/envelopes/doctor.error_network.json b/cli/acceptance/testdata/wire/doctor.error_network.json similarity index 100% rename from cli/acceptance/testdata/envelopes/doctor.error_network.json rename to cli/acceptance/testdata/wire/doctor.error_network.json diff --git a/cli/acceptance/testdata/envelopes/doctor.success_offline.json b/cli/acceptance/testdata/wire/doctor.success_offline.json similarity index 100% rename from cli/acceptance/testdata/envelopes/doctor.success_offline.json rename to cli/acceptance/testdata/wire/doctor.success_offline.json diff --git a/cli/acceptance/testdata/wire/kb_list.error_auth_forbidden.json b/cli/acceptance/testdata/wire/kb_list.error_auth_forbidden.json new file mode 100644 index 00000000..e69de29b diff --git a/cli/acceptance/testdata/envelopes/kb_list.success.json b/cli/acceptance/testdata/wire/kb_list.success.json similarity index 100% rename from cli/acceptance/testdata/envelopes/kb_list.success.json rename to cli/acceptance/testdata/wire/kb_list.success.json diff --git a/cli/acceptance/testdata/envelopes/kb_list.success_empty.json b/cli/acceptance/testdata/wire/kb_list.success_empty.json similarity index 100% rename from cli/acceptance/testdata/envelopes/kb_list.success_empty.json rename to cli/acceptance/testdata/wire/kb_list.success_empty.json diff --git a/cli/acceptance/testdata/wire/kb_view.error_resource_not_found.json b/cli/acceptance/testdata/wire/kb_view.error_resource_not_found.json new file mode 100644 index 00000000..e69de29b diff --git a/cli/acceptance/testdata/envelopes/kb_view.success.json b/cli/acceptance/testdata/wire/kb_view.success.json similarity index 100% rename from cli/acceptance/testdata/envelopes/kb_view.success.json rename to cli/acceptance/testdata/wire/kb_view.success.json diff --git a/cli/acceptance/testdata/wire/search.error_input_invalid.json b/cli/acceptance/testdata/wire/search.error_input_invalid.json new file mode 100644 index 00000000..e69de29b diff --git a/cli/acceptance/testdata/wire/search.error_resource_not_found.json b/cli/acceptance/testdata/wire/search.error_resource_not_found.json new file mode 100644 index 00000000..e69de29b diff --git a/cli/acceptance/testdata/envelopes/search.success.json b/cli/acceptance/testdata/wire/search.success.json similarity index 100% rename from cli/acceptance/testdata/envelopes/search.success.json rename to cli/acceptance/testdata/wire/search.success.json diff --git a/cli/acceptance/testdata/envelopes/version.success.json b/cli/acceptance/testdata/wire/version.success.json similarity index 100% rename from cli/acceptance/testdata/envelopes/version.success.json rename to cli/acceptance/testdata/wire/version.success.json diff --git a/cli/cmd/agent/invoke.go b/cli/cmd/agent/invoke.go index 51ba9e48..17cf61c5 100644 --- a/cli/cmd/agent/invoke.go +++ b/cli/cmd/agent/invoke.go @@ -18,8 +18,9 @@ import ( ) // agentInvokeFields enumerates fields surfaced for `--json` discovery on -// `agent invoke`. Matches invokeData below — single-shot result envelope -// with the agent's final answer plus the trace (references, tool events). +// `agent invoke`. Matches invokeData below — the single-shot result +// object with the agent's final answer plus the trace (references, +// tool events). var agentInvokeFields = []string{ "answer", "references", "tool_events", "thinking", "session_id", "agent_id", "query", @@ -46,7 +47,7 @@ type InvokeService interface { AgentQAStreamWithRequest(ctx context.Context, sessionID string, req *sdk.AgentQARequest, cb sdk.AgentEventCallback) error } -// invokeData is the JSON envelope payload. +// invokeData is the JSON payload emitted on the --json path. type invokeData struct { Answer string `json:"answer"` References []*sdk.SearchResult `json:"references"` @@ -71,7 +72,7 @@ config — agent invoke is the thin shim that streams the result. Modes: TTY (default): live answer streaming + tool-trace footer - Pipe / --no-stream / --json: buffered, single envelope at completion`, + Pipe / --no-stream / --json: buffered, single JSON object at completion`, Example: ` weknora agent invoke ag_abc "Summarise the Q3 plan" weknora agent invoke ag_abc "Continue?" --session sess_xyz weknora agent invoke ag_abc "What did we ship?" --json`, @@ -93,7 +94,7 @@ Modes: cmd.Flags().StringVar(&opts.SessionID, "session", "", "Continue an existing chat session (skip auto-create)") cmd.Flags().BoolVar(&opts.NoStream, "no-stream", false, "Buffer the full answer before printing (forces accumulate mode)") cmdutil.AddJSONFlags(cmd, agentInvokeFields) - aiclient.SetAgentHelp(cmd, "Streams an agent's answer over SSE. Pass --json (or run non-TTY) to receive a single completed envelope with answer + references + tool_events instead of partial chunks. Errors: resource.not_found (unknown agent_id) / server.session_create_failed (auto-create) / local.sse_stream_aborted (mid-stream).") + aiclient.SetAgentHelp(cmd, "Streams an agent's answer over SSE. Pass --json (or run non-TTY) to receive a single completed {answer, references, tool_events, …} JSON object instead of partial chunks. Errors: resource.not_found (unknown agent_id) / server.session_create_failed (auto-create) / local.sse_stream_aborted (mid-stream).") return cmd } @@ -183,7 +184,7 @@ func runInvoke(ctx context.Context, opts *InvokeOptions, jopts *cmdutil.JSONOpti AgentID: opts.AgentID, Query: opts.Query, } - return format.WriteJSONFiltered(iostreams.IO.Out, data, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, data) } out := iostreams.IO.Out diff --git a/cli/cmd/agent/invoke_test.go b/cli/cmd/agent/invoke_test.go index 0e550154..0f5ffa60 100644 --- a/cli/cmd/agent/invoke_test.go +++ b/cli/cmd/agent/invoke_test.go @@ -62,7 +62,7 @@ func referencesEvent(refs []*sdk.SearchResult) *sdk.AgentStreamResponse { } } -func TestInvoke_AccumulateMode_EmitsJSONEnvelope(t *testing.T) { +func TestInvoke_AccumulateMode_EmitsBareJSON(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &scriptedInvokeSvc{ events: []*sdk.AgentStreamResponse{ diff --git a/cli/cmd/agent/list.go b/cli/cmd/agent/list.go index 7858d383..aeab1cc3 100644 --- a/cli/cmd/agent/list.go +++ b/cli/cmd/agent/list.go @@ -11,7 +11,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -60,7 +59,7 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum results to return (0 = no cap, 1..10000 = explicit)") cmdutil.AddJSONFlags(cmd, agentListFields) - aiclient.SetAgentHelp(cmd, "Lists tenant-visible agents (built-in + custom) as a bare JSON array of Agent objects (empty `[]` when none). --limit caps the returned slice. Use `--json id,name` to project fields, `--jq` for arbitrary reshape.") + aiclient.SetAgentHelp(cmd, "Lists tenant-visible agents (built-in + custom) as a bare JSON array of Agent objects (empty `[]` when none). --limit caps the returned slice. Use `--json=id,name` to project fields, `--jq` for arbitrary reshape.") return cmd } @@ -92,7 +91,7 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, items, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, items) } if len(items) == 0 { diff --git a/cli/cmd/agent/view.go b/cli/cmd/agent/view.go index 8928244d..3c4d5214 100644 --- a/cli/cmd/agent/view.go +++ b/cli/cmd/agent/view.go @@ -9,7 +9,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -64,7 +63,7 @@ func runView(ctx context.Context, jopts *cmdutil.JSONOptions, svc ViewService, a return cmdutil.WrapHTTP(err, "fetch agent %s", agentID) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, a, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, a) } renderAgent(iostreams.IO.Out, a) return nil diff --git a/cli/cmd/api/api.go b/cli/cmd/api/api.go index 63107094..6f519fe7 100644 --- a/cli/cmd/api/api.go +++ b/cli/cmd/api/api.go @@ -3,7 +3,7 @@ // Shape: one positional (path) + `-X/--method` flag, default GET (auto- // promoted to POST when a body is supplied via --data or --input). The two // body-source flags are mutually exclusive. Default raw response body to -// stdout; --json wraps in CLI envelope. Reuses sdk.Client.Raw which already +// stdout; --json emits a {status, headers, body} object. Reuses sdk.Client.Raw which already // applies tenant + auth headers. package api @@ -26,7 +26,7 @@ import ( ) // apiFields is intentionally a marker — api wraps arbitrary HTTP responses -// whose schema the CLI doesn't know, so the `--json id,name` field-filter +// whose schema the CLI doesn't know, so the `--json=id,name` field-filter // is a no-op here. The marker shows up in --help so users can tell. var apiFields = []string{""} @@ -59,7 +59,7 @@ POST. Use -X/--method to override (DELETE / PUT / PATCH / HEAD). Auth, tenant, and request-id headers are applied automatically from the active context. The response body is written to stdout by default; use ---json to wrap it in the CLI envelope (status / headers / body). +--json to emit a {status, headers, body} JSON object. Examples: weknora api /api/v1/knowledge-bases # GET @@ -131,7 +131,7 @@ func resolveMethod(opts *Options) string { } // runAPI is the testable core: validate inputs, dispatch via Service.Raw, -// classify status, and emit either the raw body or a JSON envelope. The +// classify status, and emit either the raw body or a JSON object. The // caller is responsible for resolving the method (defaults / auto-POST) // and uppercasing it; runAPI guards against unsupported values like // `-X PATCH-INVALID` reaching the wire. @@ -181,7 +181,7 @@ func runAPI(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc out := iostreams.IO.Out if jopts.Enabled() { // Best-effort decode: if response body is valid JSON, surface the - // parsed structure under .data.body so envelope consumers can drill + // parsed structure under .body so JSON consumers can drill // in; otherwise fall back to the raw string. var bodyAny any if len(respBody) > 0 { @@ -195,11 +195,13 @@ func runAPI(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc hdrs[k] = v[0] } } - return format.WriteJSON(out, map[string]any{ + // --json field-filter is ignored (response shape unknown to the + // CLI); --jq runs over the full {status, headers, body} object. + return format.WriteJSONFiltered(out, map[string]any{ "status": resp.StatusCode, "headers": hdrs, "body": bodyAny, - }) + }, nil, jopts.JQ) } if _, err := out.Write(respBody); err != nil { diff --git a/cli/cmd/auth/auth.go b/cli/cmd/auth/auth.go index 4b2e149e..1b7e7166 100644 --- a/cli/cmd/auth/auth.go +++ b/cli/cmd/auth/auth.go @@ -8,7 +8,7 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" ) -// Credential-mode tokens used in the JSON envelope of auth list / login / +// Credential-mode tokens used in the JSON output of auth list / login / // status / token. The string names describe the HTTP credential type rather // than the login flow (e.g. JWT → bearer regardless of whether it was // obtained via password or refresh) so an agent can branch directly on the diff --git a/cli/cmd/auth/list.go b/cli/cmd/auth/list.go index 506a28fb..9ece8674 100644 --- a/cli/cmd/auth/list.go +++ b/cli/cmd/auth/list.go @@ -68,7 +68,7 @@ func runList(jopts *cmdutil.JSONOptions, f *cmdutil.Factory) error { sort.Slice(entries, func(i, j int) bool { return entries[i].Name < entries[j].Name }) if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, entries, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, entries) } if len(entries) == 0 { fmt.Fprintln(iostreams.IO.Out, "No contexts configured. Run `weknora auth login` to create one.") diff --git a/cli/cmd/auth/login.go b/cli/cmd/auth/login.go index 8687cba5..e88df816 100644 --- a/cli/cmd/auth/login.go +++ b/cli/cmd/auth/login.go @@ -11,7 +11,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/config" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -75,7 +74,7 @@ the current_context in ~/.config/weknora/config.yaml.`, cmd.Flags().StringVar(&opts.Context, "name", "default", "Context name to register in config.yaml") cmd.Flags().BoolVar(&opts.WithToken, "with-token", false, "Read an API key from stdin instead of prompting for password") cmdutil.AddJSONFlags(cmd, authLoginFields) - cmdutil.MustRequireFlag(cmd, "host") + _ = cmd.MarkFlagRequired("host") aiclient.SetAgentHelp(cmd, "Authenticates and stores credentials. --with-token reads an API key from stdin (no password prompt, agent-safe). Otherwise email/password prompts fire — non-TTY callers must pipe `--with-token` or pre-set --name. Errors: auth.bad_credential on wrong password; input.invalid_argument on bad --host; input.missing_flag when --with-token has empty stdin.") return cmd } @@ -211,7 +210,7 @@ func saveContextRef(opts *LoginOptions, jopts *cmdutil.JSONOptions, f *cmdutil.F result.User = user.Email result.TenantID = user.TenantID } - return format.WriteJSONFiltered(iostreams.IO.Out, result, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, result) } who := opts.Context if user != nil { diff --git a/cli/cmd/auth/logout.go b/cli/cmd/auth/logout.go index 6df2b3eb..7f6fc669 100644 --- a/cli/cmd/auth/logout.go +++ b/cli/cmd/auth/logout.go @@ -9,7 +9,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/config" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/secrets" ) @@ -97,7 +96,7 @@ func runLogout(opts *LogoutOptions, jopts *cmdutil.JSONOptions, f *cmdutil.Facto } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, logoutResult{Removed: targets}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, logoutResult{Removed: targets}) } fmt.Fprintf(iostreams.IO.Out, "✓ Logged out of %d context(s): %s\n", len(targets), strings.Join(targets, ", ")) return nil diff --git a/cli/cmd/auth/refresh.go b/cli/cmd/auth/refresh.go index f8a6f071..f1fcbc21 100644 --- a/cli/cmd/auth/refresh.go +++ b/cli/cmd/auth/refresh.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -113,7 +112,7 @@ func runRefresh(ctx context.Context, opts *RefreshOptions, jopts *cmdutil.JSONOp } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, refreshResult{Context: name}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, refreshResult{Context: name}) } fmt.Fprintf(iostreams.IO.Out, "✓ Refreshed access token for context %s\n", name) return nil diff --git a/cli/cmd/auth/status.go b/cli/cmd/auth/status.go index 24382390..476e1b71 100644 --- a/cli/cmd/auth/status.go +++ b/cli/cmd/auth/status.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -82,7 +81,7 @@ func runStatus(ctx context.Context, jopts *cmdutil.JSONOptions, f *cmdutil.Facto if tenant != nil { result.TenantName = tenant.Name } - return format.WriteJSONFiltered(iostreams.IO.Out, result, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, result) } host := "" diff --git a/cli/cmd/auth/token.go b/cli/cmd/auth/token.go index cf47e839..fd1ff082 100644 --- a/cli/cmd/auth/token.go +++ b/cli/cmd/auth/token.go @@ -7,7 +7,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" ) @@ -32,7 +31,7 @@ type tokenResult struct { // `auth list` shows which mode each context uses. // // Default output: raw token on stdout, no trailing newline (clean $(...)). -// `--json[=fields]` wraps in envelope {token, mode, context}. +// `--json[=fields]` emits a bare {token, mode, context} object. func NewCmdToken(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "token", @@ -61,7 +60,7 @@ to see which mode each context uses, and construct the matching HTTP header: }, } cmdutil.AddJSONFlags(cmd, authTokenFields) - aiclient.SetAgentHelp(cmd, "Prints the active context's credential to stdout for scripting. Default: raw secret, no trailing newline. With --json: envelope {token, mode, context}. Errors: auth.unauthenticated when no active context or no stored credential (run `auth login`); local.keychain_denied when the keyring rejects the read.") + aiclient.SetAgentHelp(cmd, "Prints the active context's credential to stdout for scripting. Default: raw secret, no trailing newline. With --json: bare {token, mode, context} object. Errors: auth.unauthenticated when no active context or no stored credential (run `auth login`); local.keychain_denied when the keyring rejects the read.") return cmd } @@ -117,9 +116,7 @@ func runToken(f *cmdutil.Factory, jopts *cmdutil.JSONOptions) error { } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, - tokenResult{Token: token, Mode: mode, Context: ctxName}, - jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, tokenResult{Token: token, Mode: mode, Context: ctxName}) } // No trailing newline — clean $(weknora auth token) substitution. diff --git a/cli/cmd/chat/chat.go b/cli/cmd/chat/chat.go index b23fa2f4..9dea3baf 100644 --- a/cli/cmd/chat/chat.go +++ b/cli/cmd/chat/chat.go @@ -9,13 +9,13 @@ // "feels alive" UX a human typing in a terminal expects. // // - Accumulate mode (non-TTY, --no-stream, or --json): buffer every -// fragment via sse.Accumulator and emit a single envelope (or a single +// fragment via sse.Accumulator and emit a single JSON object (or a single // plain-text answer + references block) once Done. Agents and pipes // get a deterministic single record to parse. // // The SDK's KnowledgeQAStream callback contract is invoked sequentially on // one goroutine, so neither mode needs locking. The runChat core takes a -// chatService interface so tests inject a fake without standing up a real +// ChatService interface so tests inject a fake without standing up a real // SSE server. package chat @@ -49,15 +49,15 @@ type Options struct { NoStream bool } -// chatService is the narrow SDK surface this command depends on. *sdk.Client +// ChatService is the narrow SDK surface this command depends on. *sdk.Client // satisfies it; tests substitute a fake. Compile-time check is at the bottom // of this file. -type chatService interface { +type ChatService interface { CreateSession(ctx context.Context, req *sdk.CreateSessionRequest) (*sdk.Session, error) KnowledgeQAStream(ctx context.Context, sessionID string, req *sdk.KnowledgeQARequest, cb func(*sdk.StreamResponse) error) error } -// chatData is the success-envelope payload. Mirrors what an agent needs to +// chatData is the JSON payload emitted on the --json path. Mirrors what an agent needs to // continue a conversation: the answer text, retrieval references, and the // session pointer to thread follow-ups. type chatData struct { @@ -116,13 +116,13 @@ Modes: cmd.Flags().StringVar(&opts.SessionID, "session", "", "Continue an existing chat session (skip auto-create)") cmd.Flags().BoolVar(&opts.NoStream, "no-stream", false, "Buffer the full answer before printing (forces accumulate mode)") cmdutil.AddJSONFlags(cmd, chatFields) - aiclient.SetAgentHelp(cmd, "Streams an LLM answer over SSE. Agent / non-TTY callers should pass --json so the full {answer, references, session_id, assistant_message_id} envelope is emitted at completion (no partial chunks). Pass --session to thread follow-ups. Errors: server.session_create_failed when auto-create fails; local.sse_stream_aborted on mid-stream disconnect.") + aiclient.SetAgentHelp(cmd, "Streams an LLM answer over SSE. Agent / non-TTY callers should pass --json so the full {answer, references, session_id, assistant_message_id} object is emitted once at completion (no partial chunks). Pass --session to thread follow-ups. Errors: server.session_create_failed when auto-create fails; local.sse_stream_aborted on mid-stream disconnect.") return cmd } // runChat is the testable core: validate, ensure a session, dispatch the -// stream, and route output. Returns a typed error suitable for the envelope. -func runChat(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc chatService) error { +// stream, and route output. Returns a typed error. +func runChat(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc ChatService) error { if opts.Query == "" { return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "query argument cannot be empty") } @@ -157,12 +157,12 @@ func runChat(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc // Decide output mode. Stream mode requires: // 1. an interactive stdout (tty) // 2. no --no-stream - // 3. no --json (envelope is single-record by definition) + // 3. no --json (JSON output is single-record by definition) streamMode := iostreams.IO.IsStdoutTTY() && !opts.NoStream && !jsonOut // Surface the auto-created session ID up-front so a user who hits ^C // mid-stream still has the pointer to resume — no need to scroll back - // past tokens. Skipped in JSON mode (it ends up in the envelope) and + // past tokens. Skipped in JSON mode (it ends up in the data object) and // when the caller already supplied --session. if autoCreated && !jsonOut { fmt.Fprintf(iostreams.IO.Err, "session: %s (use --session to continue)\n", sessionID) @@ -193,7 +193,7 @@ func runChat(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc // Re-surface the auto-created session id on failure so a user who // missed the start-of-stream notice (it scrolls past mid-stream // tokens, especially on ^C) can still recover with --session. - // Skipped in JSON mode — the envelope carries it in .data.session_id. + // Skipped in JSON mode — the data object carries it in .session_id. if autoCreated && !jsonOut { fmt.Fprintf(iostreams.IO.Err, "session: %s (resume with --session %s)\n", sessionID, sessionID) } @@ -241,7 +241,7 @@ func runChat(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc KBID: opts.KBID, Query: opts.Query, } - return format.WriteJSONFiltered(iostreams.IO.Out, data, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, data) } // Human / non-JSON paths: streaming mode already wrote the answer body @@ -264,5 +264,5 @@ func runChat(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, svc return nil } -// compile-time check: the production SDK client implements chatService. -var _ chatService = (*sdk.Client)(nil) +// compile-time check: the production SDK client implements ChatService. +var _ ChatService = (*sdk.Client)(nil) diff --git a/cli/cmd/chat/chat_test.go b/cli/cmd/chat/chat_test.go index 1e2e000b..380c7b21 100644 --- a/cli/cmd/chat/chat_test.go +++ b/cli/cmd/chat/chat_test.go @@ -12,7 +12,7 @@ import ( sdk "github.com/Tencent/WeKnora/client" ) -// fakeChatService implements chatService for unit tests. Tests configure the +// fakeChatService implements ChatService for unit tests. Tests configure the // callback driver via streamEvents (delivered in order) and observe captured // inputs through the exported fields. type fakeChatService struct { @@ -37,7 +37,7 @@ func (f *fakeChatService) CreateSession(_ context.Context, req *sdk.CreateSessio return f.createSessionResp, nil } // Default: return a deterministic session id derived from the title so - // envelope assertions don't depend on uuid generation. + // JSON assertions don't depend on uuid generation. return &sdk.Session{ID: "sess_auto", Title: req.Title}, nil } @@ -56,9 +56,9 @@ func (f *fakeChatService) KnowledgeQAStream(ctx context.Context, sessionID strin return f.streamErr } -// Sanity: fakeChatService must satisfy chatService. Mirrors the production -// var _ chatService = (*sdk.Client)(nil) check at the bottom of chat.go. -var _ chatService = (*fakeChatService)(nil) +// Sanity: fakeChatService must satisfy ChatService. Mirrors the production +// var _ ChatService = (*sdk.Client)(nil) check at the bottom of chat.go. +var _ ChatService = (*fakeChatService)(nil) func TestChat_StreamMode(t *testing.T) { out, errBuf := iostreams.SetForTestWithTTY(t) @@ -117,7 +117,7 @@ func TestChat_JSONMode(t *testing.T) { } // JSON mode must NOT print the human session-hint on stderr; the session - // id is carried inside the envelope instead. + // id is carried inside the JSON object instead. if errBuf.Len() != 0 { t.Errorf("expected empty stderr in JSON mode, got %q", errBuf.String()) } diff --git a/cli/cmd/context/add.go b/cli/cmd/context/add.go index a87b2011..e7938651 100644 --- a/cli/cmd/context/add.go +++ b/cli/cmd/context/add.go @@ -9,7 +9,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/config" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" ) @@ -102,9 +101,7 @@ func runAdd(opts *AddOptions, jopts *cmdutil.JSONOptions, name string) error { } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, - addResult{Name: name, Host: host, User: opts.User, Current: wasFirst}, - jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, addResult{Name: name, Host: host, User: opts.User, Current: wasFirst}) } if wasFirst { fmt.Fprintf(iostreams.IO.Out, "✓ Added context %s (now current). Run `weknora auth login --name %s` to attach credentials.\n", name, name) diff --git a/cli/cmd/context/list.go b/cli/cmd/context/list.go index 8edbc73f..f05827ea 100644 --- a/cli/cmd/context/list.go +++ b/cli/cmd/context/list.go @@ -73,7 +73,7 @@ func runList(jopts *cmdutil.JSONOptions) error { sort.Slice(entries, func(i, j int) bool { return entries[i].Name < entries[j].Name }) if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, entries, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, entries) } if len(entries) == 0 { fmt.Fprintln(iostreams.IO.Out, "No contexts configured. Run `weknora auth login` (or `weknora context add`) to create one.") diff --git a/cli/cmd/context/remove.go b/cli/cmd/context/remove.go index 3ec5f9bc..7bc2f12c 100644 --- a/cli/cmd/context/remove.go +++ b/cli/cmd/context/remove.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/config" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" "github.com/Tencent/WeKnora/cli/internal/secrets" @@ -80,7 +79,6 @@ func runRemove(opts *RemoveOptions, jopts *cmdutil.JSONOptions, name string, sto return notFoundError(name, cfg) } wasCurrent := name == cfg.CurrentContext - risk := riskForRemove(name, wasCurrent) jsonOut := jopts.Enabled() // Confirmation only fires for removing the current context — non-current @@ -102,10 +100,9 @@ func runRemove(opts *RemoveOptions, jopts *cmdutil.JSONOptions, name string, sto } clearContextSecrets(store, ctx, name) - _ = risk // risk classification dropped in v0.4; exit code already signals result := removeResult{Name: name, Removed: true, WasCurrent: wasCurrent} if jsonOut { - return format.WriteJSONFiltered(iostreams.IO.Out, result, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, result) } if wasCurrent { fmt.Fprintf(iostreams.IO.Out, "✓ Removed context %s (current context cleared — run `weknora context use ` to pick another)\n", name) @@ -115,19 +112,6 @@ func runRemove(opts *RemoveOptions, jopts *cmdutil.JSONOptions, name string, sto return nil } -// riskForRemove returns the operation risk: high-risk-write only when the -// target is the currently-active context (subsequent commands will have no -// default --context until the user picks one). -func riskForRemove(name string, wasCurrent bool) *format.Risk { - if wasCurrent { - return &format.Risk{ - Level: format.RiskHighRiskWrite, - Action: fmt.Sprintf("remove context %s (the current context — subsequent commands will need a new --context)", name), - } - } - return &format.Risk{Level: format.RiskWrite, Action: fmt.Sprintf("remove context %s", name)} -} - // clearContextSecrets mirrors auth/logout.go: best-effort delete every secret // slot the context references. Errors are swallowed so a missing keyring // entry doesn't block remove (logout has had the same policy since v0.2). diff --git a/cli/cmd/context/use.go b/cli/cmd/context/use.go index 1eb1b224..c3ada836 100644 --- a/cli/cmd/context/use.go +++ b/cli/cmd/context/use.go @@ -9,10 +9,13 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/config" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" ) +// contextUseFields enumerates fields surfaced for `--json` discovery on +// `context use`. +var contextUseFields = []string{"current_context", "previous_context"} + // NewCmdUse builds the `weknora context use ` command. func NewCmdUse(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ @@ -29,13 +32,18 @@ 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`, + weknora context use staging --json # {current_context, previous_context}`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { - return runUse(args[0]) + jopts, err := cmdutil.CheckJSONFlags(c) + if err != nil { + return err + } + return runUse(args[0], jopts) }, } - aiclient.SetAgentHelp(cmd, "Switches default CLI context. Returns previous_context + current_context. Errors with hint when name unknown.") + cmdutil.AddJSONFlags(cmd, contextUseFields) + aiclient.SetAgentHelp(cmd, "Switches default CLI context. With --json: returns {current_context, previous_context}. Errors with local.context_not_found and a did-you-mean hint when name unknown.") return cmd } @@ -44,7 +52,7 @@ type useResult struct { PreviousContext string `json:"previous_context,omitempty"` } -func runUse(name string) error { +func runUse(name string, jopts *cmdutil.JSONOptions) error { cfg, err := config.Load() if err != nil { return err @@ -57,10 +65,16 @@ func runUse(name string) error { if err := config.Save(cfg); err != nil { return err } - return format.WriteJSON(iostreams.IO.Out, useResult{ - CurrentContext: name, - PreviousContext: prev, - }) + result := useResult{CurrentContext: name, PreviousContext: prev} + if jopts.Enabled() { + return jopts.Emit(iostreams.IO.Out, result) + } + if prev != "" && prev != name { + fmt.Fprintf(iostreams.IO.Out, "✓ Switched context to %s (was %s)\n", name, prev) + } else { + fmt.Fprintf(iostreams.IO.Out, "✓ Active context: %s\n", name) + } + return nil } func notFoundError(name string, cfg *config.Config) error { diff --git a/cli/cmd/context/use_test.go b/cli/cmd/context/use_test.go index c56bdda3..e1159463 100644 --- a/cli/cmd/context/use_test.go +++ b/cli/cmd/context/use_test.go @@ -24,7 +24,7 @@ func TestUse_OK(t *testing.T) { t.Fatalf("Save initial config: %v", err) } - if err := runUse("production"); err != nil { + if err := runUse("production", nil); err != nil { t.Fatalf("runUse: %v", err) } @@ -52,7 +52,7 @@ func TestUse_NotFound_WithDidYouMean(t *testing.T) { t.Fatalf("Save: %v", err) } - err := runUse("prodution") // typo: missing 'c' + err := runUse("prodution", nil) // typo: missing 'c' if err == nil { t.Fatal("expected error") } @@ -84,7 +84,7 @@ func TestUse_NotFound_DeterministicTieBreak(t *testing.T) { } // "prox" is distance 1 from prod / prom (both win); lex tie-break → prod. for i := 0; i < 5; i++ { - err := runUse("prox") + err := runUse("prox", nil) if err == nil { t.Fatalf("iter %d: expected error", i) } @@ -99,7 +99,7 @@ func TestUse_NotFound_EmptyContexts(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) _, _ = iostreams.SetForTest(t) - err := runUse("anything") + err := runUse("anything", nil) if err == nil { t.Fatal("expected error") } @@ -118,7 +118,7 @@ func TestUse_CaseSensitive(t *testing.T) { }} _ = config.Save(cfg) - err := runUse("production") // lowercase — must NOT match "Production" + err := runUse("production", nil) // lowercase — must NOT match "Production" if err == nil { t.Fatal("expected case-sensitive miss") } diff --git a/cli/cmd/doc/delete.go b/cli/cmd/doc/delete.go index b68427da..4500d7f3 100644 --- a/cli/cmd/doc/delete.go +++ b/cli/cmd/doc/delete.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" ) @@ -44,12 +43,12 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { 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). -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 +AI agents: This is a high-risk write. Without -y/--yes the CLI exits 10 +and writes input.confirmation_required to stderr. NEVER auto-pass -y without the user's explicit go-ahead.`, Example: ` weknora doc delete doc_abc # interactive confirm weknora doc delete doc_abc -y # no prompt - weknora doc delete doc_abc -y --json # envelope output`, + weknora doc delete doc_abc -y --json # bare {id, deleted:true} JSON`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { jopts, err := cmdutil.CheckJSONFlags(c) @@ -79,7 +78,7 @@ func runDelete(ctx context.Context, opts *DeleteOptions, jopts *cmdutil.JSONOpti } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}) } 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 addb1ee6..a864e9d1 100644 --- a/cli/cmd/doc/delete_test.go +++ b/cli/cmd/doc/delete_test.go @@ -11,6 +11,7 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" + "github.com/Tencent/WeKnora/cli/internal/testutil" ) // fakeDeleteSvc captures the id passed and returns a canned error. @@ -26,30 +27,13 @@ func (f *fakeDeleteSvc) DeleteKnowledge(_ context.Context, id string) error { return f.err } -// scriptedConfirm satisfies prompt.Prompter and returns predetermined answers. -type scriptedConfirm struct{ confirmReturn bool } - -func (s scriptedConfirm) Input(string, string) (string, error) { return "", nil } -func (s scriptedConfirm) Password(string) (string, error) { return "", nil } -func (s scriptedConfirm) Confirm(string, bool) (bool, error) { return s.confirmReturn, nil } - -// errPrompter returns an error from Confirm — simulates a non-TTY agent -// prompter. -type errPrompter struct{} - -func (errPrompter) Input(string, string) (string, error) { return "", nil } -func (errPrompter) Password(string) (string, error) { return "", nil } -func (errPrompter) Confirm(string, bool) (bool, error) { - return false, errors.New("no tty") -} - func TestDelete_Success_WithForce(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeDeleteSvc{} opts := &DeleteOptions{Yes: true} // Force=true short-circuits the confirm path; the prompter must not be // consulted, so any value works. - require.NoError(t, runDelete(context.Background(), opts, nil, svc, scriptedConfirm{confirmReturn: false}, "doc_abc")) + require.NoError(t, runDelete(context.Background(), opts, nil, svc, &testutil.ConfirmPrompter{Answer: false}, "doc_abc")) assert.Equal(t, "doc_abc", svc.got) assert.Equal(t, 1, svc.calls) @@ -61,7 +45,7 @@ func TestDelete_Success_JSON(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeDeleteSvc{} opts := &DeleteOptions{Yes: true} - require.NoError(t, runDelete(context.Background(), opts, &cmdutil.JSONOptions{}, svc, scriptedConfirm{confirmReturn: true}, "doc_abc")) + require.NoError(t, runDelete(context.Background(), opts, &cmdutil.JSONOptions{}, svc, &testutil.ConfirmPrompter{Answer: true}, "doc_abc")) got := out.String() assert.True(t, strings.HasPrefix(strings.TrimSpace(got), `{"id":"doc_abc"`), "expected bare object; got %q", got) @@ -72,7 +56,7 @@ func TestDelete_Success_JSON(t *testing.T) { func TestDelete_NotFound_404(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{err: errors.New("HTTP error 404: not found")} - err := runDelete(context.Background(), &DeleteOptions{Yes: true}, nil, svc, scriptedConfirm{}, "doc_missing") + err := runDelete(context.Background(), &DeleteOptions{Yes: true}, nil, svc, &testutil.ConfirmPrompter{}, "doc_missing") require.Error(t, err) var typed *cmdutil.Error @@ -83,7 +67,7 @@ func TestDelete_NotFound_404(t *testing.T) { func TestDelete_HTTPError_500(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{err: errors.New("HTTP error 500: internal")} - err := runDelete(context.Background(), &DeleteOptions{Yes: true}, nil, svc, scriptedConfirm{}, "doc_x") + err := runDelete(context.Background(), &DeleteOptions{Yes: true}, nil, svc, &testutil.ConfirmPrompter{}, "doc_x") require.Error(t, err) var typed *cmdutil.Error @@ -94,7 +78,7 @@ func TestDelete_HTTPError_500(t *testing.T) { func TestDelete_ConfirmYes(t *testing.T) { out, _ := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, scriptedConfirm{confirmReturn: true}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, &testutil.ConfirmPrompter{Answer: true}, "doc_abc") require.NoError(t, err) assert.Equal(t, 1, svc.calls, "user said yes ⇒ delete proceeds") assert.Contains(t, out.String(), "✓") @@ -103,7 +87,7 @@ func TestDelete_ConfirmYes(t *testing.T) { func TestDelete_ConfirmNo(t *testing.T) { _, errBuf := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, scriptedConfirm{confirmReturn: false}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, &testutil.ConfirmPrompter{Answer: false}, "doc_abc") require.Error(t, err) assert.Equal(t, 0, svc.calls, "user said no ⇒ SDK must NOT be called") @@ -119,7 +103,7 @@ func TestDelete_ConfirmNo(t *testing.T) { func TestDelete_AgentPrompterErrors(t *testing.T) { _, _ = iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, errPrompter{}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, &testutil.ConfirmPrompter{Err: errors.New("no tty")}, "doc_abc") require.Error(t, err) assert.Equal(t, 0, svc.calls) @@ -135,7 +119,7 @@ func TestDelete_AgentPrompterErrors(t *testing.T) { func TestDelete_NoYes_NonTTY_RequiresConfirmation(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{} - err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, errPrompter{}, "doc_abc") + err := runDelete(context.Background(), &DeleteOptions{Yes: false}, nil, svc, &testutil.ConfirmPrompter{Err: errors.New("no tty")}, "doc_abc") require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) diff --git a/cli/cmd/doc/list.go b/cli/cmd/doc/list.go index 7e6c78a5..17b076e7 100644 --- a/cli/cmd/doc/list.go +++ b/cli/cmd/doc/list.go @@ -13,7 +13,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -123,19 +122,14 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, // Pagination is always 1-indexed internally. --all-pages walks; the // non-walking path returns the first page only. - var ( - items []sdk.Knowledge - total int64 - ) + var items []sdk.Knowledge if opts.AllPages { accum := make([]sdk.Knowledge, 0) - page := 1 - for { - chunk, t, err := svc.ListKnowledgeWithFilter(ctx, kbID, page, opts.PageSize, filter) + for page := 1; ; page++ { + chunk, total, err := svc.ListKnowledgeWithFilter(ctx, kbID, page, opts.PageSize, filter) if err != nil { return cmdutil.WrapHTTP(err, "list documents") } - total = t accum = append(accum, chunk...) if opts.Limit > 0 && len(accum) >= opts.Limit { accum = accum[:opts.Limit] @@ -144,16 +138,14 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, if int64(page*opts.PageSize) >= total || len(chunk) == 0 { break } - page++ } items = accum } else { - chunk, t, err := svc.ListKnowledgeWithFilter(ctx, kbID, 1, opts.PageSize, filter) + chunk, _, err := svc.ListKnowledgeWithFilter(ctx, kbID, 1, opts.PageSize, filter) if err != nil { return cmdutil.WrapHTTP(err, "list documents") } items = chunk - total = t } if items == nil { items = []sdk.Knowledge{} // ensure JSON [] not null @@ -169,10 +161,9 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, if opts.Limit > 0 && len(items) > opts.Limit { items = items[:opts.Limit] } - _ = total // pagination metadata is no longer surfaced; --all-pages drains for callers who need everything if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, items, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, items) } if len(items) == 0 { diff --git a/cli/cmd/doc/upload.go b/cli/cmd/doc/upload.go index 2d8595c8..56749f09 100644 --- a/cli/cmd/doc/upload.go +++ b/cli/cmd/doc/upload.go @@ -10,7 +10,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -165,17 +164,17 @@ func runUploadFromURL(ctx context.Context, opts *UploadOptions, jopts *cmdutil.J return cmdutil.WrapHTTP(err, "ingest URL %s", opts.FromURL) } - return printUploadSuccess(k, jopts, "Ingested", opts.Name, opts.FromURL) + return renderUploadSuccess(k, jopts, "Ingested", opts.Name, opts.FromURL) } -// printUploadSuccess emits the post-upload result. JSON path is the bare +// renderUploadSuccess emits the post-upload result. JSON path is the bare // Knowledge object; human path prints a checkmark line. Shared by single- // file upload and URL ingest; humanVerb varies (uploaded/ingested) and // fallbackDisplay covers the case when the server-recorded file_name is // blank (URL ingest pre-redirect). -func printUploadSuccess(k *sdk.Knowledge, jopts *cmdutil.JSONOptions, humanVerb, customName, fallbackDisplay string) error { +func renderUploadSuccess(k *sdk.Knowledge, jopts *cmdutil.JSONOptions, humanVerb, customName, fallbackDisplay string) error { if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, k, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, k) } displayed := customName if displayed == "" { @@ -214,5 +213,5 @@ func runUpload(ctx context.Context, opts *UploadOptions, jopts *cmdutil.JSONOpti if err != nil { return cmdutil.WrapHTTP(err, "upload %s", path) } - return printUploadSuccess(k, jopts, "Uploaded", opts.Name, path) + return renderUploadSuccess(k, jopts, "Uploaded", opts.Name, path) } diff --git a/cli/cmd/doc/upload_recursive.go b/cli/cmd/doc/upload_recursive.go index 5e3d280d..325db3fc 100644 --- a/cli/cmd/doc/upload_recursive.go +++ b/cli/cmd/doc/upload_recursive.go @@ -8,7 +8,6 @@ import ( "path/filepath" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" ) @@ -63,7 +62,7 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, jopts *cmdutil } if len(matches) == 0 { if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, recursiveResult{KBID: kbID}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, recursiveResult{KBID: kbID}) } fmt.Fprintf(iostreams.IO.Out, "(no files matched %q under %s)\n", opts.Glob, dir) return nil @@ -80,7 +79,7 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, jopts *cmdutil } failed = append(failed, uploadOutcome{Path: p, Error: err.Error()}) // Per-file progress lines are human progress signal; suppress - // under --json so they don't precede the envelope on stdout. + // under --json so they don't precede the JSON object on stdout. if !jopts.Enabled() { fmt.Fprintf(iostreams.IO.Out, "FAIL %s: %v\n", filepath.Base(p), err) } @@ -98,7 +97,7 @@ func runUploadRecursive(ctx context.Context, opts *UploadOptions, jopts *cmdutil if jopts.Enabled() { result := recursiveResult{KBID: kbID, Uploaded: uploaded, Failed: failed} - if err := format.WriteJSONFiltered(iostreams.IO.Out, result, jopts.Fields, jopts.JQ); err != nil { + if err := jopts.Emit(iostreams.IO.Out, result); err != nil { return err } } else { diff --git a/cli/cmd/doc/view.go b/cli/cmd/doc/view.go index 5338f953..bd78a212 100644 --- a/cli/cmd/doc/view.go +++ b/cli/cmd/doc/view.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -63,7 +62,7 @@ func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, return cmdutil.WrapHTTP(err, "get document %q", id) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, doc, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, doc) } w := iostreams.IO.Out fmt.Fprintf(w, "ID: %s\n", doc.ID) diff --git a/cli/cmd/doctor/doctor.go b/cli/cmd/doctor/doctor.go index 1aeb7526..e4b2a6fe 100644 --- a/cli/cmd/doctor/doctor.go +++ b/cli/cmd/doctor/doctor.go @@ -32,7 +32,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/build" "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/compat" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/secrets" sdk "github.com/Tencent/WeKnora/client" @@ -207,7 +206,7 @@ func runChecks(ctx context.Context, opts *Options, svc Services, cliVer string) // the loader knows authoritatively which branch it took, time-based // derivation from ProbedAt is unreliable since SaveCache uses time.Now(). // -// v0.2 mapping: +// Mapping: // // compat.OK → StatusOK // compat.SoftWarn → StatusWarn (server older but in-range; soft skew) @@ -336,7 +335,7 @@ func summarize(cs []Check) Summary { // code, set by the caller). func emit(jopts *cmdutil.JSONOptions, r Result) { if jopts.Enabled() { - _ = format.WriteJSONFiltered(iostreams.IO.Out, r, jopts.Fields, jopts.JQ) + _ = jopts.Emit(iostreams.IO.Out, r) return } for _, c := range r.Checks { diff --git a/cli/cmd/doctor/doctor_test.go b/cli/cmd/doctor/doctor_test.go index 002f7ef4..bc59ad78 100644 --- a/cli/cmd/doctor/doctor_test.go +++ b/cli/cmd/doctor/doctor_test.go @@ -236,11 +236,12 @@ func TestDoctor_NoCache_BypassesCache(t *testing.T) { } } -// TestDoctor_VersionSkewWarns covers the v0.2 soft-skew path: server is older +// TestDoctor_VersionSkewWarns covers the soft-skew path: server is older // than CLI by ≥ 1 minor (same major, in compat range) → server_version=warn, -// envelope.ok stays true, all_passed=false (so agents reading just the -// boolean still notice). The compat decision lives in cli/internal/compat; -// this test pins the doctor-side mapping (compat.SoftWarn → StatusWarn). +// summary.failed=0 (exit 0), summary.all_passed=false (so agents reading +// just the boolean still notice). The compat decision lives in +// cli/internal/compat; this test pins the doctor-side mapping +// (compat.SoftWarn → StatusWarn). func TestDoctor_VersionSkewWarns(t *testing.T) { t.Setenv("XDG_CACHE_HOME", t.TempDir()) _, _ = iostreams.SetForTest(t) @@ -315,7 +316,7 @@ func TestDoctor_HardErrorStillFails(t *testing.T) { } } -// TestDoctor_KeychainFallbackWarns covers credential_storage's v0.2 third +// TestDoctor_KeychainFallbackWarns covers credential_storage's third // state: keyring unavailable, fell back to FileStore (agent containers, // headless CI, WSL without DBus). The check should warn — secrets still // persist (0600 file perms) but the OS-backed path was unreachable. @@ -439,7 +440,7 @@ func TestDoctor_HumanMarker_Warn(t *testing.T) { // TestDoctor_WarnedField_OmittedAtZero protects the JSON wire compactness: // `warned` carries omitempty, so a clean run has no warned key. Existing -// agents inspecting older envelopes shouldn't see a sudden new field unless +// agents inspecting older outputs shouldn't see a sudden new field unless // it actually fired. func TestDoctor_WarnedField_OmittedAtZero(t *testing.T) { out, _ := iostreams.SetForTest(t) @@ -455,14 +456,14 @@ func TestDoctor_WarnedField_OmittedAtZero(t *testing.T) { emit(&cmdutil.JSONOptions{}, r) got := out.String() if strings.Contains(got, `"warned"`) { - t.Errorf("envelope should omit `warned` field when zero, got %q", got) + t.Errorf("output should omit `warned` field when zero, got %q", got) } } -// TestDoctor_RunE_FailReturnsSilentError is a behavior test on NewCmd: when -// any check is fail, RunE must return cmdutil.SilentError so the framework -// exit-1 path runs WITHOUT overwriting the data envelope emit() already -// wrote. This is the v0.2 contract change from v0.1's "always nil". +// TestDoctor_RunE_FailReturnsSilentError is a behavior test on NewCmd: +// when any check is fail, RunE must return cmdutil.SilentError so the +// framework exit-1 path runs without writing a second error line on top +// of the data object emit() already wrote. // // SilenceErrors/SilenceUsage on the leaf cobra.Command suppress cobra's own // "Error: ..." + usage dump that would otherwise leak to stderr when running diff --git a/cli/cmd/kb/create.go b/cli/cmd/kb/create.go index 5c629a5f..b7d4cc15 100644 --- a/cli/cmd/kb/create.go +++ b/cli/cmd/kb/create.go @@ -9,7 +9,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -62,13 +61,15 @@ func NewCmdCreate(f *cmdutil.Factory) *cobra.Command { cmd.Flags().StringVar(&opts.Description, "description", "", "Knowledge base description (optional)") cmd.Flags().StringVar(&opts.EmbeddingModel, "embedding-model", "", "Embedding model ID (optional; server picks default when unset)") cmdutil.AddJSONFlags(cmd, kbCreateFields) - aiclient.SetAgentHelp(cmd, "Creates a knowledge base under the active context. --name is required; --description and --embedding-model are optional. Returns data: full KnowledgeBase object including the new id.") + _ = cmd.MarkFlagRequired("name") + aiclient.SetAgentHelp(cmd, "Creates a knowledge base under the active context. --name is required; --description and --embedding-model are optional. Returns the full KnowledgeBase object including the new id.") return cmd } func runCreate(ctx context.Context, opts *CreateOptions, jopts *cmdutil.JSONOptions, svc CreateService) error { - // Validate locally before any HTTP — keeps `input.invalid_argument` - // distinct from a server-side 400. + // Trim defensively in case a caller invokes runCreate directly with + // whitespace; the cobra layer marks --name required so the empty-string + // case is unreachable from the CLI. if strings.TrimSpace(opts.Name) == "" { return cmdutil.NewError(cmdutil.CodeInputInvalidArgument, "--name is required") } @@ -87,7 +88,7 @@ func runCreate(ctx context.Context, opts *CreateOptions, jopts *cmdutil.JSONOpti } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, created, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, created) } 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 baf3e88c..3035b6ed 100644 --- a/cli/cmd/kb/delete.go +++ b/cli/cmd/kb/delete.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" ) @@ -46,13 +45,13 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { 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). -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.`, +AI agents: This is a high-risk write. Without -y/--yes the CLI exits 10 +and writes input.confirmation_required to stderr. NEVER auto-pass -y +without the user's explicit go-ahead — 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`, + weknora kb delete kb_abc -y --json # bare {id, deleted:true} JSON`, Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { jopts, err := cmdutil.CheckJSONFlags(c) @@ -82,7 +81,7 @@ func runDelete(ctx context.Context, opts *DeleteOptions, jopts *cmdutil.JSONOpti } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}) } 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 d9a9a4a0..aa08b1e8 100644 --- a/cli/cmd/kb/delete_test.go +++ b/cli/cmd/kb/delete_test.go @@ -12,13 +12,14 @@ import ( "github.com/Tencent/WeKnora/cli/internal/cmdutil" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" + "github.com/Tencent/WeKnora/cli/internal/testutil" ) // fakeDeleteSvc records what id was deleted. type fakeDeleteSvc struct { - err error - gotID string - called bool + err error + gotID string + called bool } func (f *fakeDeleteSvc) DeleteKnowledgeBase(_ context.Context, id string) error { @@ -27,30 +28,16 @@ func (f *fakeDeleteSvc) DeleteKnowledgeBase(_ context.Context, id string) error return f.err } -// confirmPrompter scripts a Confirm answer; Input/Password are unused here. -type confirmPrompter struct { - answer bool - err error - asked bool -} - -func (c *confirmPrompter) Input(string, string) (string, error) { return "", prompt.ErrAgentNoPrompt } -func (c *confirmPrompter) Password(string) (string, error) { return "", prompt.ErrAgentNoPrompt } -func (c *confirmPrompter) Confirm(string, bool) (bool, error) { - c.asked = true - return c.answer, c.err -} - func TestDelete_Success_WithForce(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{} + p := &testutil.ConfirmPrompter{} opts := &DeleteOptions{Yes: true} require.NoError(t, runDelete(context.Background(), opts, nil, svc, p, "kb_force")) assert.True(t, svc.called) assert.Equal(t, "kb_force", svc.gotID) - assert.False(t, p.asked, "--force must skip the confirm prompt") + assert.False(t, p.Asked, "--force must skip the confirm prompt") assert.Contains(t, out.String(), "✓ Deleted") assert.Contains(t, out.String(), "kb_force") } @@ -58,7 +45,7 @@ func TestDelete_Success_WithForce(t *testing.T) { func TestDelete_NotFound(t *testing.T) { _, _ = iostreams.SetForTest(t) svc := &fakeDeleteSvc{err: errors.New("HTTP error 404: not found")} - p := &confirmPrompter{} + p := &testutil.ConfirmPrompter{} err := runDelete(context.Background(), &DeleteOptions{Yes: true}, nil, svc, p, "kb_missing") require.Error(t, err) @@ -73,7 +60,7 @@ func TestDelete_NonTTY_NoYes_RequiresConfirmation(t *testing.T) { // silently proceed in scripted contexts. iostreams.SetForTest(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{} + p := &testutil.ConfirmPrompter{} err := runDelete(context.Background(), &DeleteOptions{}, nil, svc, p, "kb_nontty") require.Error(t, err) @@ -81,14 +68,14 @@ func TestDelete_NonTTY_NoYes_RequiresConfirmation(t *testing.T) { 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.False(t, p.Asked, "non-TTY ⇒ Confirm is never invoked") assert.Equal(t, 10, cmdutil.ExitCode(err), "exit code 10 per destructive-write protocol") } func TestDelete_JSONOutput(t *testing.T) { out, _ := iostreams.SetForTest(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{} + p := &testutil.ConfirmPrompter{} opts := &DeleteOptions{Yes: true} require.NoError(t, runDelete(context.Background(), opts, &cmdutil.JSONOptions{}, svc, p, "kb_json")) @@ -104,10 +91,10 @@ func TestDelete_JSONOutput(t *testing.T) { func TestDelete_ConfirmYes(t *testing.T) { _, _ = iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{answer: true} + p := &testutil.ConfirmPrompter{Answer: true} require.NoError(t, runDelete(context.Background(), &DeleteOptions{}, nil, svc, p, "kb_yes")) - assert.True(t, p.asked, "confirm prompt should fire on TTY without --force") + assert.True(t, p.Asked, "confirm prompt should fire on TTY without --force") assert.True(t, svc.called, "answer=yes ⇒ delete proceeds") assert.Equal(t, "kb_yes", svc.gotID) } @@ -115,14 +102,14 @@ func TestDelete_ConfirmYes(t *testing.T) { func TestDelete_ConfirmNo(t *testing.T) { _, errBuf := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{answer: false} + p := &testutil.ConfirmPrompter{Answer: false} err := runDelete(context.Background(), &DeleteOptions{}, nil, svc, p, "kb_no") require.Error(t, err) var typed *cmdutil.Error require.ErrorAs(t, err, &typed) assert.Equal(t, cmdutil.CodeUserAborted, typed.Code) - assert.True(t, p.asked) + assert.True(t, p.Asked) assert.False(t, svc.called, "answer=no ⇒ delete must NOT run") assert.Contains(t, errBuf.String(), "Aborted") } @@ -130,7 +117,7 @@ func TestDelete_ConfirmNo(t *testing.T) { func TestDelete_ConfirmPrompterError(t *testing.T) { _, _ = iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{err: prompt.ErrAgentNoPrompt} + p := &testutil.ConfirmPrompter{Err: prompt.ErrAgentNoPrompt} err := runDelete(context.Background(), &DeleteOptions{}, nil, svc, p, "kb_err") require.Error(t, err) @@ -146,7 +133,7 @@ func TestDelete_JSONOut_NoYes_RequiresConfirmation(t *testing.T) { // Exit-10 protocol must fire when -y is absent. iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{} + p := &testutil.ConfirmPrompter{} opts := &DeleteOptions{} err := runDelete(context.Background(), opts, &cmdutil.JSONOptions{}, svc, p, "kb_jtty") @@ -154,21 +141,21 @@ func TestDelete_JSONOut_NoYes_RequiresConfirmation(t *testing.T) { 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, 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. + // approval. Must call SDK and emit the bare result object. out, _ := iostreams.SetForTestWithTTY(t) svc := &fakeDeleteSvc{} - p := &confirmPrompter{} + p := &testutil.ConfirmPrompter{} opts := &DeleteOptions{Yes: true} require.NoError(t, runDelete(context.Background(), opts, &cmdutil.JSONOptions{}, svc, p, "kb_jtty")) - assert.False(t, p.asked, "-y must skip the prompt") + 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/kb/edit.go b/cli/cmd/kb/edit.go index 0be33936..70bc58bf 100644 --- a/cli/cmd/kb/edit.go +++ b/cli/cmd/kb/edit.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -111,7 +110,7 @@ func runEdit(ctx context.Context, opts *EditOptions, jopts *cmdutil.JSONOptions, return cmdutil.WrapHTTP(err, "edit knowledge base %s", id) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, updated, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, updated) } fmt.Fprintf(iostreams.IO.Out, "✓ Updated knowledge base %s\n", id) return nil diff --git a/cli/cmd/kb/empty.go b/cli/cmd/kb/empty.go index 09d6b53a..34d508d1 100644 --- a/cli/cmd/kb/empty.go +++ b/cli/cmd/kb/empty.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" sdk "github.com/Tencent/WeKnora/client" @@ -83,8 +82,7 @@ func runEmpty(ctx context.Context, opts *EmptyOptions, jopts *cmdutil.JSONOption } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, - emptyResult{ID: id, DeletedCount: deleted}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, emptyResult{ID: id, DeletedCount: deleted}) } fmt.Fprintf(iostreams.IO.Out, "✓ Emptied knowledge base %s (%d document(s) cleared)\n", id, deleted) return nil diff --git a/cli/cmd/kb/list.go b/cli/cmd/kb/list.go index cba332b6..b07167ee 100644 --- a/cli/cmd/kb/list.go +++ b/cli/cmd/kb/list.go @@ -11,7 +11,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -20,7 +19,7 @@ import ( // kbListFields enumerates the fields surfaced for `--json` discovery on // `kb list`. Nested config structs (chunking / image / FAQ / VLM / storage // / extract) are intentionally omitted — users wanting those can use `--jq` -// against the full envelope. +// against the full object. var kbListFields = []string{ "id", "name", "type", "description", "is_temporary", "is_pinned", @@ -66,7 +65,7 @@ func NewCmdList(f *cmdutil.Factory) *cobra.Command { cmd.Flags().BoolVar(&opts.Pinned, "pinned", false, "Only show pinned knowledge bases") cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 30, "Maximum results to return (0 = no cap, 1..10000 = explicit)") cmdutil.AddJSONFlags(cmd, kbListFields) - aiclient.SetAgentHelp(cmd, "Lists all knowledge bases as a bare JSON array of {id, name, ...} objects (empty `[]` when none). --pinned restricts to pinned KBs (client-side filter). --limit caps the returned slice. Use `--json` (bare) for full objects, `--json id,name` to project fields, or `--jq` for arbitrary reshape.") + aiclient.SetAgentHelp(cmd, "Lists all knowledge bases as a bare JSON array of {id, name, ...} objects (empty `[]` when none). --pinned restricts to pinned KBs (client-side filter). --limit caps the returned slice. Use `--json` (bare) for full objects, `--json=id,name` to project fields, or `--jq` for arbitrary reshape.") return cmd } @@ -105,7 +104,7 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, items, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, items) } if len(items) == 0 { diff --git a/cli/cmd/kb/pin.go b/cli/cmd/kb/pin.go index b84e6ba1..2fc8be07 100644 --- a/cli/cmd/kb/pin.go +++ b/cli/cmd/kb/pin.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -80,7 +79,7 @@ func runPin(ctx context.Context, opts *PinOptions, jopts *cmdutil.JSONOptions, s // both fresh-toggle and no-op paths. Human path prints a confirming // line; agents observe via the unchanged is_pinned field. if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, current, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, current) } fmt.Fprintf(iostreams.IO.Out, "✓ %s is already %s\n", id, state) return nil @@ -91,7 +90,7 @@ func runPin(ctx context.Context, opts *PinOptions, jopts *cmdutil.JSONOptions, s return cmdutil.WrapHTTP(err, "%s knowledge base %s", verb, id) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, updated, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, updated) } state := "pinned" if !updated.IsPinned { diff --git a/cli/cmd/kb/view.go b/cli/cmd/kb/view.go index 381ed0a1..f29e37c1 100644 --- a/cli/cmd/kb/view.go +++ b/cli/cmd/kb/view.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -63,7 +62,7 @@ func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, return cmdutil.WrapHTTP(err, "get knowledge base %q", id) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, kb, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, kb) } // Human: KEY: VALUE w := iostreams.IO.Out diff --git a/cli/cmd/link/link.go b/cli/cmd/link/link.go index 4ed944af..1aa64e8c 100644 --- a/cli/cmd/link/link.go +++ b/cli/cmd/link/link.go @@ -16,7 +16,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/projectlink" ) @@ -105,7 +104,7 @@ func runLink(ctx context.Context, opts *Options, jopts *cmdutil.JSONOptions, f * ProjectLinkPath: linkPath, } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, r, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, r) } if kbName != "" { fmt.Fprintf(iostreams.IO.Out, "✓ Linked %s to %s (kb=%s, id=%s)\n", linkPath, ctxName, kbName, kbID) diff --git a/cli/cmd/link/unlink.go b/cli/cmd/link/unlink.go index 26416166..d6927295 100644 --- a/cli/cmd/link/unlink.go +++ b/cli/cmd/link/unlink.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/projectlink" ) @@ -39,7 +38,7 @@ discovery that ` + "`--kb`" + ` resolution uses; you do not need to cd to the project root to unlink. Errors with input.invalid_argument when no link is present anywhere in the parent chain.`, Example: ` weknora unlink # remove the binding for this project - weknora unlink --json # envelope output (CI / agents)`, + weknora unlink --json # bare JSON (CI / agents)`, Args: cobra.NoArgs, RunE: func(c *cobra.Command, _ []string) error { jopts, err := cmdutil.CheckJSONFlags(c) @@ -74,8 +73,7 @@ func runUnlink(opts *UnlinkOptions, jopts *cmdutil.JSONOptions) error { return cmdutil.Wrapf(cmdutil.CodeLocalFileIO, err, "remove %s", linkPath) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, - unlinkResult{ProjectLinkPath: linkPath}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, unlinkResult{ProjectLinkPath: linkPath}) } fmt.Fprintf(iostreams.IO.Out, "✓ Unlinked %s\n", linkPath) return nil diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 38f707da..7c67b6e6 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -3,7 +3,6 @@ package cmd import ( "fmt" - "os" "strings" "github.com/spf13/cobra" @@ -30,62 +29,16 @@ import ( // Execute is the entry point invoked by main(). Returns the process exit code. func Execute() int { root := NewRootCmd(cmdutil.New()) - // ExecuteC returns the actually-invoked leaf (or root when invocation - // failed before dispatch); we use it to honor the leaf's --json and - // inherited --format without walking the tree ourselves. - cmd, err := root.ExecuteC() - if err == nil { - return 0 + if err := root.Execute(); err != nil { + // Errors go to stderr (matches gh/aws/stripe). Stdout stays + // empty (or holds partial success the command produced) so + // downstream `--json | jq` pipelines never filter error shapes + // out of the success stream. The typed exit code (3/4/5/6/7/10) + // carries the error class. + cmdutil.PrintError(iostreams.IO.Err, MapCobraError(err)) + return cmdutil.ExitCode(MapCobraError(err)) } - err = MapCobraError(err) - if WantsJSONOutput(cmd) { - cmdutil.PrintErrorEnvelope(iostreams.IO.Out, err) - } else { - cmdutil.PrintError(iostreams.IO.Err, err) - } - return cmdutil.ExitCode(err) -} - -// WantsJSONOutput reports whether cmd was invoked with --json, so error -// output matches the success format. Persistent flags inherit automatically -// via cmd.Flags(). -// -// Falls back to scanning os.Args when cobra never reached the leaf — e.g. -// unknown subcommand or unknown flag at root level. Without this, `weknora -// bogus --json` would emit a human stderr line instead of the envelope the -// agent asked for. -// -// Exported so the acceptance/contract test helper can replicate Execute()'s -// envelope-printing path without having to call os.Exit-bound Execute() itself. -func WantsJSONOutput(cmd *cobra.Command) bool { - // --json is a StringSlice in v0.4 (optional field filter). A - // Changed=true flag indicates the user requested JSON output. - if f := cmd.Flags().Lookup("json"); f != nil && f.Changed { - return true - } - return argsRequestJSON(os.Args[1:]) -} - -// argsRequestJSON scans a flag-only slice for --json in the forms pflag -// accepts. Used as a fallback when cobra short-circuits before flag parsing -// (unknown command / unknown flag at root). Recognizes `--json` bare, -// `--json id,name`, and `--json=id,name` forms. -func argsRequestJSON(args []string) bool { - for i, a := range args { - switch { - case a == "--json": - return true - case strings.HasPrefix(a, "--json="): - return true - default: - // `--json id,name` — split into two args; we don't try to - // distinguish "next arg is a value" vs "next arg is a flag" - // here, since false positives just mean we emit JSON for an - // error that would otherwise be human (still parseable). - _ = i - } - } - return false + return 0 } // MapCobraError tags the textually-emitted cobra errors as cmdutil.FlagError @@ -97,8 +50,8 @@ func argsRequestJSON(args []string) bool { // cobra/command.go: required-flag / unknown-command). TestMapCobraError_PinnedPrefixes // guards against a silent break on cobra bumps. // -// Exported so the acceptance/contract test helper can reuse the mapping when -// replicating Execute()'s error-envelope path in-process. +// Exported so the acceptance/contract test helper can reuse the mapping +// when replicating Execute()'s stderr error-path in-process. func MapCobraError(err error) error { if err == nil { return nil @@ -143,8 +96,8 @@ hybrid searches against a WeKnora server from your shell or an AI agent.`, SilenceErrors: true, // Version makes cobra auto-register a `--version` global flag that // prints this string. We accept both `--version` and a `version` - // subcommand; the subcommand still owns the richer `--json` envelope - // output. + // subcommand; the subcommand still owns the richer `--json` output + // (build commit + date). Version: fmt.Sprintf("%s (commit %s, built %s)", v, commit, date), PersistentPreRun: func(c *cobra.Command, args []string) { // Propagate the global --context flag into the Factory for this @@ -191,10 +144,10 @@ func addGlobalFlags(cmd *cobra.Command) { pf.String("context", "", "Override the active context for this invocation (no disk write)") } -// agentAwareHelpFunc wraps cobra's default help to append the AI agent guidance -// (Annotations[aiclient.AIAgentHelpKey]) only when an AI coding agent env var is -// detected (CLAUDECODE / CURSOR_AGENT). Help-only render — no behavior switch -// (v0.2 ADR-3). +// agentAwareHelpFunc wraps cobra's default help to append the AI agent +// guidance (Annotations[aiclient.AIAgentHelpKey]) only when an AI coding +// agent env var is detected (CLAUDECODE / CURSOR_AGENT). Help-only +// render — no behavior switch. func agentAwareHelpFunc(orig func(*cobra.Command, []string)) func(*cobra.Command, []string) { return func(c *cobra.Command, args []string) { orig(c, args) @@ -213,7 +166,7 @@ func agentAwareHelpFunc(orig func(*cobra.Command, []string)) func(*cobra.Command } // versionFields enumerates the fields surfaced for `--json` discovery on -// `version`. Mirrors the version envelope payload. +// `version`. Mirrors the version object payload. var versionFields = []string{"version", "commit", "date"} // newVersionCmd is the only leaf command shipped in the foundation PR. It diff --git a/cli/cmd/root_test.go b/cli/cmd/root_test.go index 6f444bc0..cb58582e 100644 --- a/cli/cmd/root_test.go +++ b/cli/cmd/root_test.go @@ -140,36 +140,3 @@ func TestRoot_ContextFlagPropagation(t *testing.T) { } } -func TestArgsRequestJSON(t *testing.T) { - // v0.4 R-1: --json is now StringSlice (gh-style field filter). Any - // `--json` token in args means the user wants JSON output, regardless - // of value (boolean string parsing dropped). - cases := []struct { - name string - args []string - want bool - }{ - {"empty", nil, false}, - {"--json bare", []string{"version", "--json"}, true}, - {"--json=id,name", []string{"kb", "list", "--json=id,name"}, true}, - {"--json=anything", []string{"version", "--json=foo"}, true}, - {"unrelated", []string{"bogus", "--kb", "x"}, false}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.want, argsRequestJSON(tc.args)) - }) - } -} - -func TestWantsJSONOutput(t *testing.T) { - // Build a minimal *cobra.Command with the json flag directly so we test - // the helper without going through cobra's parse pipeline. WantsJSONOutput - // reads cmd.Flags() which on a fresh command equals LocalFlags(). - c := &cobra.Command{Use: "x"} - c.Flags().Bool("json", false, "") - assert.False(t, WantsJSONOutput(c), "default: --json unset") - - require.NoError(t, c.Flags().Set("json", "true")) - assert.True(t, WantsJSONOutput(c), "--json=true honored") -} diff --git a/cli/cmd/search/chunks.go b/cli/cmd/search/chunks.go index b379b9df..9b465fa0 100644 --- a/cli/cmd/search/chunks.go +++ b/cli/cmd/search/chunks.go @@ -9,7 +9,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -144,7 +143,7 @@ func runChunks(ctx context.Context, opts *ChunksOptions, jopts *cmdutil.JSONOpti if results == nil { results = []*sdk.SearchResult{} } - return format.WriteJSONFiltered(iostreams.IO.Out, results, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, results) } return renderChunkResults(results, opts.KBID) } diff --git a/cli/cmd/search/chunks_test.go b/cli/cmd/search/chunks_test.go index b2542a3a..026a1db7 100644 --- a/cli/cmd/search/chunks_test.go +++ b/cli/cmd/search/chunks_test.go @@ -44,7 +44,7 @@ func TestRunSearch_HumanOutput(t *testing.T) { assert.Contains(t, got, "doc-1") } -// JSON envelope must surface match_type so machine consumers / agents can +// JSON output must surface match_type so machine consumers / agents can // reason about retrieval channels without re-implementing the wire format. // (Human renderer keeps default minimal — diagnostic info opt-in via --json.) func TestRunSearch_JSONIncludesMatchType(t *testing.T) { diff --git a/cli/cmd/search/docs.go b/cli/cmd/search/docs.go index 3f34fc49..fbcbe50a 100644 --- a/cli/cmd/search/docs.go +++ b/cli/cmd/search/docs.go @@ -11,7 +11,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -120,7 +119,7 @@ done: if matches == nil { matches = []sdk.Knowledge{} } - return format.WriteJSONFiltered(iostreams.IO.Out, matches, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, matches) } if len(matches) == 0 { fmt.Fprintln(iostreams.IO.Out, "(no matches)") diff --git a/cli/cmd/search/kb.go b/cli/cmd/search/kb.go index 2f0dbcb3..3a327db4 100644 --- a/cli/cmd/search/kb.go +++ b/cli/cmd/search/kb.go @@ -11,7 +11,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -91,7 +90,7 @@ func runKBSearch(ctx context.Context, opts *KBSearchOptions, jopts *cmdutil.JSON if matches == nil { matches = []sdk.KnowledgeBase{} } - return format.WriteJSONFiltered(iostreams.IO.Out, matches, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, matches) } if len(matches) == 0 { fmt.Fprintln(iostreams.IO.Out, "(no matches)") diff --git a/cli/cmd/search/sessions.go b/cli/cmd/search/sessions.go index 613090dd..ca5accf0 100644 --- a/cli/cmd/search/sessions.go +++ b/cli/cmd/search/sessions.go @@ -11,7 +11,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" @@ -100,7 +99,7 @@ done: if matches == nil { matches = []sdk.Session{} } - return format.WriteJSONFiltered(iostreams.IO.Out, matches, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, matches) } if len(matches) == 0 { fmt.Fprintln(iostreams.IO.Out, "(no matches)") diff --git a/cli/cmd/session/delete.go b/cli/cmd/session/delete.go index 06c2bc40..cef8b34b 100644 --- a/cli/cmd/session/delete.go +++ b/cli/cmd/session/delete.go @@ -8,7 +8,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/prompt" ) @@ -44,8 +43,8 @@ func NewCmdDelete(f *cmdutil.Factory) *cobra.Command { 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). -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 +AI agents: This is a high-risk write. Without -y/--yes the CLI exits 10 +and writes input.confirmation_required to stderr. NEVER auto-pass -y without the user's explicit go-ahead.`, Example: ` weknora session delete s_abc # interactive confirm weknora session delete s_abc -y # no prompt @@ -79,7 +78,7 @@ func runDelete(ctx context.Context, opts *DeleteOptions, jopts *cmdutil.JSONOpti } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, deleteResult{ID: id, Deleted: true}) } fmt.Fprintf(iostreams.IO.Out, "✓ Deleted session %s\n", id) return nil diff --git a/cli/cmd/session/list.go b/cli/cmd/session/list.go index ce5420a8..75dba700 100644 --- a/cli/cmd/session/list.go +++ b/cli/cmd/session/list.go @@ -12,14 +12,12 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" "github.com/Tencent/WeKnora/cli/internal/text" sdk "github.com/Tencent/WeKnora/client" ) const ( - defaultPage = 1 defaultPageSize = 30 maxPageSize = 1000 ) @@ -96,19 +94,14 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, since = d } - var ( - items []sdk.Session - total int - ) + var items []sdk.Session if opts.AllPages { accum := make([]sdk.Session, 0) - page := 1 - for { - chunk, t, err := svc.GetSessionsByTenant(ctx, page, opts.PageSize) + for page := 1; ; page++ { + chunk, total, err := svc.GetSessionsByTenant(ctx, page, opts.PageSize) if err != nil { return cmdutil.WrapHTTP(err, "list sessions") } - total = t accum = append(accum, chunk...) if opts.Limit > 0 && len(accum) >= opts.Limit { accum = accum[:opts.Limit] @@ -117,16 +110,14 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, if page*opts.PageSize >= total || len(chunk) == 0 { break } - page++ } items = accum } else { - chunk, t, err := svc.GetSessionsByTenant(ctx, 1, opts.PageSize) + chunk, _, err := svc.GetSessionsByTenant(ctx, 1, opts.PageSize) if err != nil { return cmdutil.WrapHTTP(err, "list sessions") } items = chunk - total = t } if items == nil { items = []sdk.Session{} // JSON [] not null @@ -149,10 +140,9 @@ func runList(ctx context.Context, opts *ListOptions, jopts *cmdutil.JSONOptions, if opts.Limit > 0 && len(items) > opts.Limit { items = items[:opts.Limit] } - _ = total // pagination metadata no longer surfaced; --all-pages drains for callers who need everything if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, items, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, items) } if len(items) == 0 { diff --git a/cli/cmd/session/view.go b/cli/cmd/session/view.go index 55201581..e4dbec07 100644 --- a/cli/cmd/session/view.go +++ b/cli/cmd/session/view.go @@ -9,7 +9,6 @@ import ( "github.com/Tencent/WeKnora/cli/internal/aiclient" "github.com/Tencent/WeKnora/cli/internal/cmdutil" - "github.com/Tencent/WeKnora/cli/internal/format" "github.com/Tencent/WeKnora/cli/internal/iostreams" sdk "github.com/Tencent/WeKnora/client" ) @@ -60,7 +59,7 @@ func runView(ctx context.Context, opts *ViewOptions, jopts *cmdutil.JSONOptions, return cmdutil.WrapHTTP(err, "get session %q", id) } if jopts.Enabled() { - return format.WriteJSONFiltered(iostreams.IO.Out, s, jopts.Fields, jopts.JQ) + return jopts.Emit(iostreams.IO.Out, s) } w := iostreams.IO.Out fmt.Fprintf(w, "ID: %s\n", s.ID) diff --git a/cli/internal/aiclient/detect.go b/cli/internal/aiclient/detect.go index bb4cc6af..ee990584 100644 --- a/cli/internal/aiclient/detect.go +++ b/cli/internal/aiclient/detect.go @@ -1,12 +1,6 @@ -// Package agent handles AI agent integration: env-based detection (used to -// trigger AGENT-targeted help text) and per-command help annotations. -// -// v0.2 ADR-3: removed the omnibus `--agent` flag + ApplyAgentSugar -// mode-switch as over-design. Per-command --json + TTY auto-detect cover -// 95% of cases. 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). +// Package aiclient handles AI agent integration: env-based detection +// (used to trigger AGENT-targeted help text) and per-command help +// annotations. See cli/AGENTS.md for the full agent contract. package aiclient import "os" diff --git a/cli/internal/aiclient/detect_test.go b/cli/internal/aiclient/detect_test.go index 301a4b77..3d4079e6 100644 --- a/cli/internal/aiclient/detect_test.go +++ b/cli/internal/aiclient/detect_test.go @@ -29,7 +29,7 @@ func TestDetectAIAgent(t *testing.T) { {name: "claude code", set: map[string]string{"CLAUDECODE": "1"}, want: "claude-code"}, {name: "cursor", set: map[string]string{"CURSOR_AGENT": "yes"}, want: "cursor"}, // Other entries (codex / aider / continue / opencode / gemini-coder) - // were dropped in v0.2 ADR-3 — env names had no official agent docs. + // were dropped — those env names had no official agent docs. // New entries should arrive with a documented source URL. { name: "first-match precedence", diff --git a/cli/internal/cmdutil/confirm.go b/cli/internal/cmdutil/confirm.go index ee167229..3915599e 100644 --- a/cli/internal/cmdutil/confirm.go +++ b/cli/internal/cmdutil/confirm.go @@ -1,7 +1,6 @@ package cmdutil import ( - "errors" "fmt" "github.com/Tencent/WeKnora/cli/internal/iostreams" @@ -25,24 +24,15 @@ import ( // proceed. See cli/AGENTS.md "Exit codes". // // `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 { return nil } - risk := &OperationRisk{Level: "high-risk-write", Action: fmt.Sprintf("delete %s %s", what, id)} if !iostreams.IO.IsStdoutTTY() || jsonOut { - e := NewError( + return 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 { diff --git a/cli/internal/cmdutil/errors.go b/cli/internal/cmdutil/errors.go index 9d1fe98e..6170a205 100644 --- a/cli/internal/cmdutil/errors.go +++ b/cli/internal/cmdutil/errors.go @@ -9,9 +9,9 @@ import ( "strings" ) -// ErrorCode is a namespaced stable identifier carried in the failure envelope. -// SemVer governance: v0.x maintains the registry below; new codes are noted -// in release notes. v0.9 introduces a CI compat test (see ADR-6b). +// ErrorCode is a namespaced stable identifier emitted on stderr in the +// `code: message` failure line. SemVer governance: v0.x maintains the +// registry below; new codes are noted in release notes. type ErrorCode string const ( @@ -33,9 +33,9 @@ const ( 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 (see cli/AGENTS.md). - // Agents must surface the envelope to the user and only retry with -y - // after explicit human approval; never auto-retry. + // Mapped to exit code 10 (see cli/AGENTS.md). Agents must surface the + // error to the user and only retry with -y after explicit human + // approval; never auto-retry. CodeInputConfirmationRequired ErrorCode = "input.confirmation_required" // server.* / network.* @@ -60,9 +60,9 @@ const ( CodeKBNotFound ErrorCode = "local.kb_not_found" CodeProjectLinkCorrupt ErrorCode = "local.project_link_corrupt" // CodeUserAborted marks a user-cancelled destructive operation (declined a - // confirm prompt). Distinct from SilentError so envelopes still carry a - // stable code; distinct from input.* because the user supplied valid args - // and simply chose not to proceed. + // confirm prompt). Distinct from SilentError so the stderr line still + // carries a stable code; distinct from input.* because the user supplied + // valid args and simply chose not to proceed. CodeUserAborted ErrorCode = "local.user_aborted" // CodeUploadFileNotFound marks a `weknora doc upload` invocation pointing at // a path that does not exist. Distinct from CodeLocalFileIO (permission / @@ -81,7 +81,9 @@ const ( ) // Error is the typed error implementations carry through the call stack. -// RunE returns a *Error and the root command formats it into the envelope. +// RunE returns a *Error and the root command renders it on stderr in +// `code: message[: cause]\nhint: ...` form. Exit code is derived by +// ExitCode(). type Error struct { Code ErrorCode Message string @@ -89,27 +91,13 @@ 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 - // Silent suppresses the default Failure envelope written by - // PrintErrorEnvelope while preserving the typed Code for ExitCode. - // Set by commands that already wrote their own envelope (e.g. bulk - // operations reporting partial-success data) but still need to surface - // a non-zero exit code matched to the failure class. + // Silent suppresses PrintError's stderr output while preserving the + // typed Code for ExitCode. Set by commands that already wrote their + // own output (e.g. bulk operations reporting partial-success data on + // stdout) but still need to surface a non-zero exit code. Silent bool } -// 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 { if e == nil { return "" @@ -202,7 +190,7 @@ func matchPrefix(err error, prefix string) bool { } // ClassifyHTTPStatus maps an HTTP status code to the canonical ErrorCode. -// Single source of truth so envelope codes stay aligned whether the failure +// Single source of truth so error codes stay aligned whether the failure // was detected by the SDK (string-formatted error) or by the CLI directly // (e.g. raw passthrough reading resp.StatusCode). func ClassifyHTTPStatus(status int) ErrorCode { @@ -229,7 +217,7 @@ func ClassifyHTTPStatus(status int) ErrorCode { // parsing the "HTTP error : ..." message format the SDK currently // emits (client.parseResponse). Until the SDK exposes a typed APIError this // is the lowest-friction way to surface 401/404/429/etc. as the right -// envelope code instead of every server-side problem collapsing to +// typed code instead of every server-side problem collapsing to // server.error. // // Returns CodeNetworkError when err is not an HTTP error (transport / DNS), diff --git a/cli/internal/cmdutil/exit.go b/cli/internal/cmdutil/exit.go index 835af3b7..b0ecfce7 100644 --- a/cli/internal/cmdutil/exit.go +++ b/cli/internal/cmdutil/exit.go @@ -4,8 +4,6 @@ import ( "errors" "fmt" "io" - - "github.com/Tencent/WeKnora/cli/internal/format" ) // ExitCode maps an error to the documented CLI exit code (spec §2.4 + ADR-3). @@ -56,13 +54,10 @@ func ExitCode(err error) int { return 1 } -// PrintError writes err to w in human-readable form. The envelope-aware -// JSON formatter is in `internal/format`; this helper is the human path used -// when no command produced its own output. -// -// Typed *Error values surface their Hint as a second line so users see the -// actionable next-step (matches envelope.error.hint visibility in --json). -// Falls through to defaultHint when caller didn't set one. +// PrintError writes err to w (typically stderr) as `code: message\nhint: +// ...`. Typed *Error values surface their Hint as a second line so users +// see the actionable next-step. Falls through to defaultHint when the +// caller didn't set one. func PrintError(w io.Writer, err error) { if err == nil || errors.Is(err, SilentError) { return @@ -80,76 +75,10 @@ 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. 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 - } - var typed *Error - if errors.As(err, &typed) && typed.Silent { - return - } - 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 -// other emit paths (planned: MCP) reuse the same projection rather than -// reimplementing the typed-error → wire-shape mapping. -func ToErrorBody(err error) *format.ErrorBody { - if err == nil { - return nil - } - body := &format.ErrorBody{Message: err.Error()} - var typed *Error - if errors.As(err, &typed) { - body.Code = string(typed.Code) - body.Message = typed.Message - body.Hint = typed.Hint - if body.Hint == "" { - body.Hint = defaultHint(typed.Code) - } - body.Retryable = typed.Retryable - // Surface the wrapped cause so agents see the actual server / SDK - // error string, not just the wrap message ("hybrid search"). The - // human's printed line and the JSON envelope both end with the - // underlying problem. - if typed.Cause != nil { - body.Message = typed.Message + ": " + typed.Cause.Error() - } - return body - } - var fe *FlagError - if errors.As(err, &fe) { - body.Code = string(CodeInputInvalidArgument) - return body - } - // Unclassified error; consumers see the message but no stable code. - body.Code = string(CodeServerError) - return body -} - -// defaultHint returns a canonical actionable hint for known error codes when -// the call site didn't set one. Spec §1.4 zero-config matrix mandates -// `auth.unauthenticated` envelopes carry "run weknora auth login" — this -// fallback covers the broad surface (auth status / kb list / kb get / search) -// without per-command hint plumbing. +// defaultHint returns a canonical actionable hint for known error codes +// when the call site didn't set one. `auth.unauthenticated` always points +// at `weknora auth login` — covers the broad surface (auth status / kb +// list / kb view / search) without per-command hint plumbing. // // Empty string for codes without a stable canonical hint. func defaultHint(code ErrorCode) string { @@ -171,7 +100,7 @@ func defaultHint(code ErrorCode) string { case CodeServerTimeout: return "request timed out; retry, or run `weknora doctor` to check connectivity" case CodeResourceNotFound: - return "verify the resource ID; list available with `weknora kb list`" + return "verify the resource ID and try again" case CodeInputInvalidArgument, CodeInputMissingFlag: return "see `weknora --help` for valid usage" case CodeInputConfirmationRequired: diff --git a/cli/internal/cmdutil/exit_test.go b/cli/internal/cmdutil/exit_test.go index 32fea773..d0698cd6 100644 --- a/cli/internal/cmdutil/exit_test.go +++ b/cli/internal/cmdutil/exit_test.go @@ -36,34 +36,6 @@ func TestExitCode(t *testing.T) { } } -func TestToErrorBody(t *testing.T) { - t.Run("nil returns nil", func(t *testing.T) { - assert.Nil(t, ToErrorBody(nil)) - }) - t.Run("typed without cause", func(t *testing.T) { - body := ToErrorBody(NewError(CodeResourceNotFound, "kb missing")) - assert.Equal(t, "resource.not_found", body.Code) - assert.Equal(t, "kb missing", body.Message) - }) - t.Run("typed with cause surfaces inner", func(t *testing.T) { - inner := errors.New("HTTP error 500: server exploded") - body := ToErrorBody(Wrapf(CodeServerError, inner, "hybrid search")) - assert.Equal(t, "server.error", body.Code) - // Both wrap context AND server cause must appear so agents see what - // actually broke, not just our wrap label. - assert.Equal(t, "hybrid search: HTTP error 500: server exploded", body.Message) - }) - t.Run("flag error", func(t *testing.T) { - body := ToErrorBody(NewFlagError(errors.New("bad flag"))) - assert.Equal(t, "input.invalid_argument", body.Code) - }) - t.Run("unclassified", func(t *testing.T) { - body := ToErrorBody(errors.New("anything")) - assert.Equal(t, "server.error", body.Code) - assert.Equal(t, "anything", body.Message) - }) -} - func TestPrintError(t *testing.T) { t.Run("nil is silent", func(t *testing.T) { var buf bytes.Buffer diff --git a/cli/internal/cmdutil/exporter.go b/cli/internal/cmdutil/exporter.go deleted file mode 100644 index 23c8a162..00000000 --- a/cli/internal/cmdutil/exporter.go +++ /dev/null @@ -1,25 +0,0 @@ -package cmdutil - -import ( - "io" - - "github.com/Tencent/WeKnora/cli/internal/format" -) - -// Exporter renders an envelope to a writer. Currently the only -// implementation is the JSON exporter; the interface stays in case a future -// renderer (templated text, table) needs to plug in without changing call -// sites that already write through Exporter.Write. -type Exporter interface { - Write(w io.Writer, env format.Envelope) error -} - -// NewJSONExporter returns an Exporter that emits envelope JSON via -// format.WriteEnvelope (single-source encoder config: no HTML escape). -func NewJSONExporter() Exporter { return &jsonExporter{} } - -type jsonExporter struct{} - -func (jsonExporter) Write(w io.Writer, env format.Envelope) error { - return format.WriteEnvelope(w, env) -} diff --git a/cli/internal/cmdutil/exporter_test.go b/cli/internal/cmdutil/exporter_test.go deleted file mode 100644 index 21bc8b24..00000000 --- a/cli/internal/cmdutil/exporter_test.go +++ /dev/null @@ -1,27 +0,0 @@ -package cmdutil - -import ( - "bytes" - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/Tencent/WeKnora/cli/internal/format" -) - -func TestJSONExporter_WritesEnvelope(t *testing.T) { - var buf bytes.Buffer - exp := NewJSONExporter() - require.NoError(t, exp.Write(&buf, format.Success(map[string]string{"k": "v"}, nil))) - var got map[string]any - require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) - assert.Equal(t, true, got["ok"]) -} - -func TestFlagError_IsSentinel(t *testing.T) { - err := NewFlagError(assert.AnError) - _, ok := err.(*FlagError) - assert.True(t, ok) -} diff --git a/cli/internal/cmdutil/factory.go b/cli/internal/cmdutil/factory.go index feb5ff0b..c9b1fd43 100644 --- a/cli/internal/cmdutil/factory.go +++ b/cli/internal/cmdutil/factory.go @@ -76,9 +76,9 @@ func New() *Factory { f.Config = func() (*config.Config, error) { cfg, err := config.Load() if err != nil { - // Map raw fs / parse errors to typed codes so envelopes don't - // surface bare `server.error` for what's actually a local IO / - // corrupt-config problem. + // Map raw fs / parse errors to typed codes so the stderr line + // doesn't surface bare `server.error` for what's actually a + // local IO / corrupt-config problem. if errors.Is(err, config.ErrCorrupt) { return nil, Wrapf(CodeLocalConfigCorrupt, err, "config malformed") } diff --git a/cli/internal/cmdutil/jsonflags.go b/cli/internal/cmdutil/jsonflags.go index 814e0b7c..c1928222 100644 --- a/cli/internal/cmdutil/jsonflags.go +++ b/cli/internal/cmdutil/jsonflags.go @@ -2,17 +2,20 @@ package cmdutil import ( "errors" + "io" "sort" "strings" "github.com/spf13/cobra" "github.com/spf13/pflag" + + "github.com/Tencent/WeKnora/cli/internal/format" ) // JSONOptions captures the resolved --json + --jq state after CheckJSONFlags. // A non-nil value means the user requested JSON output; Fields restricts -// data.items[*] when len(Fields) > 0; JQ is a jq expression applied to the -// final envelope JSON. +// each top-level object (or each element of a top-level array) to the +// listed keys; JQ is a jq expression applied to the final JSON. type JSONOptions struct { Fields []string JQ string @@ -22,24 +25,32 @@ type JSONOptions struct { // shorthand for `opts != nil`. func (o *JSONOptions) Enabled() bool { return o != nil } -// jsonNoOptSentinel marks a bare `--json` (no comma-separated values -// after it). pflag's NoOptDefVal mechanism stores this sentinel into the -// slice; CheckJSONFlags then maps it to "no field filter" (full envelope). +// Emit serializes v as bare JSON to w, honoring the resolved field-filter +// and jq expression. Equivalent to calling format.WriteJSONFiltered with +// o.Fields / o.JQ, but lets call sites stay free of the format import. +// Safe to call on a nil receiver in case the caller composes it with +// Enabled(). +func (o *JSONOptions) Emit(w io.Writer, v any) error { + if o == nil { + return format.WriteJSON(w, v) + } + return format.WriteJSONFiltered(w, v, o.Fields, o.JQ) +} + +// jsonNoOptSentinel marks a bare `--json` (no comma-separated values after +// it). pflag's NoOptDefVal mechanism stores this sentinel into the slice; +// CheckJSONFlags then maps it to "no field filter" (full payload). // -// Rationale: WeKnora's envelope is itself the machine-readable contract -// (carries typed error.code / _meta / risk), so a bare `--json` always -// producing the full envelope keeps the standard `weknora kb list --json | -// jq` pattern working. Field discovery moves to per-command `--help` -// "JSON fields available" sections rendered by AddJSONFlags. +// Field discovery moves to per-command `--help` "JSON fields available" +// sections rendered by AddJSONFlags. const jsonNoOptSentinel = "\x00json-no-value" // AddJSONFlags registers --json and --jq on cmd. // -// - `--json` → full envelope (no field filter) -// - `--json id,name` → envelope with data.items[*] / data restricted -// to listed fields -// - `--jq ` → applies a jq expression after marshaling; -// requires --json to be set explicitly +// - `--json` → bare JSON payload, no field filter +// - `--json=id,name` → each object restricted to the listed fields +// - `--jq ` → apply a jq expression to the JSON; requires +// --json to be set explicitly // // `fields` is the set of available fields the user may pass; rendered in // the command's help. Pass nil to skip the help annotation (uncommon). @@ -48,7 +59,7 @@ func AddJSONFlags(cmd *cobra.Command, fields []string) { // Backticks reserved for pflag's UnquoteUsage to extract the varname; // avoid them in the description so the help doesn't render the flag // name twice. - f.StringSlice("json", nil, "Output JSON envelope (bare for full; --json=id,name for `fields`)") + f.StringSlice("json", nil, "Output bare JSON (--json=id,name to project `fields`)") f.Lookup("json").NoOptDefVal = jsonNoOptSentinel f.StringP("jq", "q", "", "Filter JSON output using a jq `expression` (requires --json)") @@ -56,7 +67,7 @@ func AddJSONFlags(cmd *cobra.Command, fields []string) { sorted := append([]string(nil), fields...) sort.Strings(sorted) // Append to Long without overwriting per-command prose. - hdr := "\n\nJSON fields available via `--json id,name,...`:\n " + + hdr := "\n\nJSON fields available via `--json=id,name,...`:\n " + strings.Join(sorted, "\n ") if cmd.Long != "" { cmd.Long += hdr @@ -71,8 +82,8 @@ func AddJSONFlags(cmd *cobra.Command, fields []string) { // - (*JSONOptions, nil) --json set (possibly with --jq) // - (nil, error) --jq without --json (plain error, exit 1) // -// Bare `--json` yields Fields == nil (full envelope). Explicit field list -// yields Fields == []string{"id", "name", ...} (filter applied). +// Bare `--json` yields Fields == nil (no field filter). Explicit field +// list yields Fields == []string{"id", "name", ...} (filter applied). func CheckJSONFlags(cmd *cobra.Command) (*JSONOptions, error) { f := cmd.Flags() jsonFlag := f.Lookup("json") diff --git a/cli/internal/cmdutil/jsonflags_test.go b/cli/internal/cmdutil/jsonflags_test.go index ef618b58..6149a3c6 100644 --- a/cli/internal/cmdutil/jsonflags_test.go +++ b/cli/internal/cmdutil/jsonflags_test.go @@ -32,8 +32,8 @@ func newTestCmd(t *testing.T, captured **cmdutil.JSONOptions) *cobra.Command { return cmd } -func TestAddJSONFlags_BareYieldsFullEnvelopeOpts(t *testing.T) { - // `--json` bare → Enabled() with empty Fields → caller emits full envelope. +func TestAddJSONFlags_BareYieldsEnabledOptsWithNoFields(t *testing.T) { + // `--json` bare → Enabled() with empty Fields → caller emits full payload. var captured *cmdutil.JSONOptions cmd := newTestCmd(t, &captured) cmd.SetArgs([]string{"--json"}) @@ -53,8 +53,9 @@ func TestAddJSONFlags_BareYieldsFullEnvelopeOpts(t *testing.T) { func TestAddJSONFlags_FieldsFlagParsing(t *testing.T) { // NoOptDefVal sentinel means the `=` form is required for value passing. - // Space form `--json id,name` parses as bare + positional, which is - // documented divergence from gh CLI to keep bare-envelope semantics. + // Space form `--json id,name` parses as bare + positional, which is a + // documented divergence from gh CLI: weknora keeps bare `--json` as a + // shortcut for "full payload". cases := []struct { args []string want []string diff --git a/cli/internal/cmdutil/options.go b/cli/internal/cmdutil/options.go deleted file mode 100644 index ccd48d4f..00000000 --- a/cli/internal/cmdutil/options.go +++ /dev/null @@ -1,17 +0,0 @@ -package cmdutil - -import ( - "fmt" - - "github.com/spf13/cobra" -) - -// MustRequireFlag panics on programmer error (typo in flag name). cobra's -// MarkFlagRequired only returns an error when the named flag does not exist -// on the command, which means the caller has a typo at registration time — -// non-recoverable. Wrap so command builders stay one-line. -func MustRequireFlag(cmd *cobra.Command, name string) { - if err := cmd.MarkFlagRequired(name); err != nil { - panic(fmt.Sprintf("MarkFlagRequired %q: %v", name, err)) - } -} diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index f5bb85fd..bec2e511 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -1,9 +1,8 @@ // Package config reads and writes the user-level config at -// $XDG_CONFIG_HOME/weknora/config.yaml. yaml.v3 directly; viper is intentionally -// not used (see ADR-2). -// -// v0.0 supports only Load/Save with multi-host context map; project link -// (.weknora/project.toml) is wired in v0.2 (ADR-16). +// $XDG_CONFIG_HOME/weknora/config.yaml. yaml.v3 directly; viper is +// intentionally not used (see ADR-2). Multi-host context map lives here; +// the per-project link (.weknora/project.yaml) is handled by the +// projectlink package (see ADR-16). package config import ( diff --git a/cli/internal/format/bare.go b/cli/internal/format/bare.go index ceb188d0..bc3a8cb6 100644 --- a/cli/internal/format/bare.go +++ b/cli/internal/format/bare.go @@ -6,10 +6,9 @@ import ( "io" ) -// WriteJSON serializes v as one-line JSON to w. Bare-data contract: no -// envelope wrapper. Each list command emits its array directly, each -// single-resource command emits its object directly. The shape is whatever -// the producing command marshals. +// WriteJSON serializes v as one-line JSON to w. Bare-data contract: list +// commands emit their array directly, single-resource commands emit their +// object directly. The shape is whatever the producing command marshals. func WriteJSON(w io.Writer, v any) error { enc := json.NewEncoder(w) enc.SetEscapeHTML(false) @@ -31,9 +30,11 @@ func WriteJSON(w io.Writer, v any) error { // - v marshals to a scalar → unchanged // // Unknown field names are silently dropped so users can pass an aspirational -// field set across heterogenous list outputs without per-command tailoring -// (same policy as the old envelope filter). +// field set across heterogenous list outputs. func WriteJSONFiltered(w io.Writer, v any, fields []string, jqExpr string) error { + if len(fields) == 0 && jqExpr == "" { + return WriteJSON(w, v) + } raw, err := marshalJSON(v) if err != nil { return err diff --git a/cli/internal/format/envelope.go b/cli/internal/format/envelope.go deleted file mode 100644 index 38762218..00000000 --- a/cli/internal/format/envelope.go +++ /dev/null @@ -1,128 +0,0 @@ -// Package format renders command output: JSON envelope, jq, template, table. -// -// Non-TTY default = JSON envelope (ADR-5). Envelope schema is the contract -// shared with `weknora mcp serve` tools and external agents. -package format - -import ( - "encoding/json" - "io" -) - -// Envelope is the canonical success/failure shape returned by every command. -// -// v0.2 ADR-3 added Notice / Risk. Both 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"` - Notice *Notice `json:"_notice,omitempty"` - Risk *Risk `json:"risk,omitempty"` -} - -// 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". -type Risk struct { - Level RiskLevel `json:"level"` - Action string `json:"action,omitempty"` -} - -// Meta carries non-payload context fields useful to agents and observability. -// -// Pagination metadata (Page / PageSize / Total) lives here rather than in -// data.{...} so every list command's `data` field has the same shape — -// always `{items: [...]}` — and agents can branch on the resource type -// without per-list parser variants. -type Meta struct { - Context string `json:"context,omitempty"` - TenantID uint64 `json:"tenant_id,omitempty"` - KBID string `json:"kb_id,omitempty"` - RequestID string `json:"request_id,omitempty"` - NextCursor string `json:"next_cursor,omitempty"` - HasMore bool `json:"has_more,omitempty"` - Page int `json:"page,omitempty"` - PageSize int `json:"page_size,omitempty"` - Total int64 `json:"total,omitempty"` - Warnings []string `json:"warnings,omitempty"` - AppliedFilters []string `json:"applied_filters,omitempty"` -} - -// RiskLevel classifies an operation. Agents use this to decide whether to -// retry, require explicit user approval, or stop. -type RiskLevel string - -const ( - 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. 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"` - Retryable bool `json:"retryable,omitempty"` - ConsoleURL string `json:"console_url,omitempty"` - Details map[string]any `json:"details,omitempty"` -} - -// 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 cli/AGENTS.md for the agent contract). -func WriteEnvelope(w io.Writer, env Envelope) error { - enc := json.NewEncoder(w) - enc.SetEscapeHTML(false) - return enc.Encode(env) -} - -// Success constructs a success envelope. -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 deleted file mode 100644 index fd358efc..00000000 --- a/cli/internal/format/envelope_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package format - -import ( - "bytes" - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestSuccessEnvelope(t *testing.T) { - env := Success(map[string]string{"id": "kb_abc"}, &Meta{Context: "prod", RequestID: "cli-01"}) - var buf bytes.Buffer - require.NoError(t, WriteEnvelope(&buf, env)) - var got map[string]any - require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) - assert.Equal(t, true, got["ok"]) - assert.NotNil(t, got["data"]) - assert.NotNil(t, got["_meta"]) - assert.Nil(t, got["error"]) -} - -func TestFailureEnvelope(t *testing.T) { - env := Failure(&ErrorBody{ - Code: "auth.unauthenticated", - Message: "no creds", - Hint: "run weknora auth login", - Retryable: false, - }) - var buf bytes.Buffer - require.NoError(t, WriteEnvelope(&buf, env)) - var got map[string]any - require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) - assert.Equal(t, false, got["ok"]) - errBody := got["error"].(map[string]any) - assert.Equal(t, "auth.unauthenticated", errBody["code"]) - assert.Equal(t, "run weknora auth login", errBody["hint"]) -} - -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) { - // Hints / messages may include & or <; ensure we don't HTML-escape them - // (would break agent jq pipelines). - env := Failure(&ErrorBody{Code: "x", Message: "a & b < c"}) - var buf bytes.Buffer - require.NoError(t, WriteEnvelope(&buf, env)) - assert.Contains(t, buf.String(), "a & b < c") -} diff --git a/cli/internal/format/filter.go b/cli/internal/format/filter.go index dcc9adb5..7bde5150 100644 --- a/cli/internal/format/filter.go +++ b/cli/internal/format/filter.go @@ -9,127 +9,12 @@ import ( "github.com/itchyny/gojq" ) -// WriteEnvelopeFiltered serializes env to w, optionally restricting -// data.items[*] (for list envelopes) or data (for single-resource envelopes) -// to the named fields, then applying a jq expression on the result. -// -// - len(fields) == 0 → no field filter (full envelope) -// - jqExpr == "" → no jq filter (just write the envelope) -// -// The envelope structure (ok / data / error / _meta / risk / _notice) -// is preserved across field filtering — only Data is rewritten. jq operates -// on the entire envelope JSON so users can `--jq '.data.items[].id'`. -func WriteEnvelopeFiltered(w io.Writer, env Envelope, fields []string, jqExpr string) error { - raw, err := marshalEnvelope(env) - if err != nil { - return err - } - if len(fields) > 0 { - raw, err = applyFieldFilter(raw, fields) - if err != nil { - return err - } - } - if jqExpr != "" { - return writeJQ(w, raw, jqExpr) - } - _, err = w.Write(raw) - return err -} - -// marshalEnvelope returns the canonical newline-terminated envelope JSON used -// by the non-filtered path. Identical to WriteEnvelope but as bytes. -func marshalEnvelope(env Envelope) ([]byte, error) { - var buf bytes.Buffer - enc := json.NewEncoder(&buf) - enc.SetEscapeHTML(false) - if err := enc.Encode(env); err != nil { - return nil, err - } - return buf.Bytes(), nil -} - -// applyFieldFilter rewrites envelope.data so that nested objects keep only -// the named keys. The shape rules: -// -// - data is an object with `items` (the standard list envelope shape): -// filter each items[*] to the named fields. -// - data is an object without `items` (single-resource envelope): -// filter data itself. -// - data is an array (uncommon — currently no command produces this, but -// defensive): filter each [*]. -// - data is nil / scalar: unchanged. -// -// Unknown field names are silently ignored so a user may pass an -// aspirational field set across heterogenous list outputs without per- -// command tailoring. -func applyFieldFilter(envelopeJSON []byte, fields []string) ([]byte, error) { - var raw map[string]json.RawMessage - if err := json.Unmarshal(envelopeJSON, &raw); err != nil { - return nil, fmt.Errorf("field filter: parse envelope: %w", err) - } - dataRaw, ok := raw["data"] - if !ok || len(dataRaw) == 0 || string(dataRaw) == "null" { - return envelopeJSON, nil - } - - filtered, err := filterDataPayload(dataRaw, fields) - if err != nil { - return nil, err - } - raw["data"] = filtered - - var buf bytes.Buffer - enc := json.NewEncoder(&buf) - enc.SetEscapeHTML(false) - if err := enc.Encode(raw); err != nil { - return nil, err - } - return buf.Bytes(), nil -} - -// filterDataPayload dispatches on the shape of the data JSON value. -func filterDataPayload(dataRaw json.RawMessage, fields []string) (json.RawMessage, error) { - trimmed := bytes.TrimSpace(dataRaw) - if len(trimmed) == 0 { - return dataRaw, nil - } - switch trimmed[0] { - case '{': - return filterObjectData(dataRaw, fields) - case '[': - return filterArrayItems(dataRaw, fields) - default: - // scalar (number / string / bool / null) — nothing to filter - return dataRaw, nil - } -} - -// filterObjectData filters either data.items[*] (list shape) or data (single -// resource). -func filterObjectData(dataRaw json.RawMessage, fields []string) (json.RawMessage, error) { - var obj map[string]json.RawMessage - if err := json.Unmarshal(dataRaw, &obj); err != nil { - return nil, fmt.Errorf("field filter: parse data object: %w", err) - } - if items, ok := obj["items"]; ok { - filtered, err := filterArrayItems(items, fields) - if err != nil { - return nil, err - } - obj["items"] = filtered - return json.Marshal(obj) - } - // Single-resource envelope: filter the data object itself. - return filterObjectKeys(dataRaw, fields) -} - // filterArrayItems applies filterObjectKeys to each element of an array. // Non-object elements (e.g. an array of strings) are passed through. func filterArrayItems(arrayRaw json.RawMessage, fields []string) (json.RawMessage, error) { var items []json.RawMessage if err := json.Unmarshal(arrayRaw, &items); err != nil { - return nil, fmt.Errorf("field filter: parse data items: %w", err) + return nil, fmt.Errorf("field filter: parse array: %w", err) } for i, item := range items { trimmed := bytes.TrimSpace(item) @@ -161,21 +46,20 @@ func filterObjectKeys(objRaw json.RawMessage, fields []string) (json.RawMessage, return json.Marshal(dst) } -// writeJQ evaluates expr against envelopeJSON and writes each result line -// by line to w. String results render without quotes (so `--jq '.x.name'` -// yields shell-friendly bare strings); non-string results use -// encoding/json. +// 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 envelope code. -func writeJQ(w io.Writer, envelopeJSON []byte, expr string) error { +// the typed code. +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) } var input any - if err := json.Unmarshal(envelopeJSON, &input); err != nil { + if err := json.Unmarshal(raw, &input); err != nil { return fmt.Errorf("jq input parse: %w", err) } iter := query.Run(input) diff --git a/cli/internal/format/filter_test.go b/cli/internal/format/filter_test.go deleted file mode 100644 index 67749f6c..00000000 --- a/cli/internal/format/filter_test.go +++ /dev/null @@ -1,195 +0,0 @@ -package format_test - -import ( - "bytes" - "encoding/json" - "strings" - "testing" - - "github.com/Tencent/WeKnora/cli/internal/format" -) - -func TestWriteEnvelopeFiltered_NoFilter(t *testing.T) { - env := format.Success(map[string]any{"items": []any{ - map[string]any{"id": "1", "name": "alpha"}, - }}, &format.Meta{KBID: "kb_x"}) - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, nil, ""); err != nil { - t.Fatalf("err = %v", err) - } - var got map[string]any - if err := json.Unmarshal(buf.Bytes(), &got); err != nil { - t.Fatalf("parse: %v", err) - } - if got["ok"] != true { - t.Errorf("ok = %v, want true", got["ok"]) - } -} - -func TestWriteEnvelopeFiltered_FieldsOnListItems(t *testing.T) { - env := format.Success(map[string]any{"items": []any{ - map[string]any{"id": "1", "name": "alpha", "kb_id": "kb_x", "updated_at": "2026-01-01"}, - map[string]any{"id": "2", "name": "beta", "kb_id": "kb_x", "updated_at": "2026-01-02"}, - }}, nil) - - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, []string{"id", "name"}, ""); err != nil { - t.Fatalf("err = %v", err) - } - - var got struct { - OK bool `json:"ok"` - Data struct { - Items []map[string]string `json:"items"` - } `json:"data"` - } - if err := json.Unmarshal(buf.Bytes(), &got); err != nil { - t.Fatalf("parse: %v\n%s", err, buf.String()) - } - if len(got.Data.Items) != 2 { - t.Fatalf("items len = %d, want 2", len(got.Data.Items)) - } - for i, item := range got.Data.Items { - if _, has := item["kb_id"]; has { - t.Errorf("item[%d] should not have kb_id: %v", i, item) - } - if _, has := item["updated_at"]; has { - t.Errorf("item[%d] should not have updated_at: %v", i, item) - } - if item["id"] == "" || item["name"] == "" { - t.Errorf("item[%d] missing required keys: %v", i, item) - } - } -} - -func TestWriteEnvelopeFiltered_FieldsOnSingleObject(t *testing.T) { - env := format.Success(map[string]any{ - "id": "kb_x", - "name": "Engineering", - "owner": "alice", - }, nil) - - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, []string{"id", "name"}, ""); err != nil { - t.Fatalf("err = %v", err) - } - - var got struct { - Data map[string]string `json:"data"` - } - if err := json.Unmarshal(buf.Bytes(), &got); err != nil { - t.Fatalf("parse: %v\n%s", err, buf.String()) - } - if _, has := got.Data["owner"]; has { - t.Errorf("should not have owner: %v", got.Data) - } - if got.Data["id"] != "kb_x" || got.Data["name"] != "Engineering" { - t.Errorf("missing kept fields: %v", got.Data) - } -} - -func TestWriteEnvelopeFiltered_UnknownFieldSilent(t *testing.T) { - env := format.Success(map[string]any{"id": "1"}, nil) - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, []string{"id", "nonexistent"}, ""); err != nil { - t.Fatalf("err = %v", err) - } - var got struct { - Data map[string]string `json:"data"` - } - if err := json.Unmarshal(buf.Bytes(), &got); err != nil { - t.Fatalf("parse: %v", err) - } - if got.Data["id"] != "1" { - t.Errorf("id missing: %v", got.Data) - } - if _, has := got.Data["nonexistent"]; has { - t.Errorf("nonexistent should be silently dropped: %v", got.Data) - } -} - -func TestWriteEnvelopeFiltered_PreservesEnvelopeFields(t *testing.T) { - // Even with field filter, meta/risk/error must be preserved. - env := format.Success(map[string]any{"items": []any{ - map[string]any{"id": "1", "name": "x", "kb_id": "kb"}, - }}, &format.Meta{KBID: "kb_x", RequestID: "req_123"}) - - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, []string{"id"}, ""); err != nil { - t.Fatalf("err = %v", err) - } - var got struct { - OK bool `json:"ok"` - Meta *format.Meta `json:"_meta"` - Data struct { - Items []map[string]any `json:"items"` - } `json:"data"` - } - if err := json.Unmarshal(buf.Bytes(), &got); err != nil { - t.Fatalf("parse: %v\n%s", err, buf.String()) - } - if got.Meta == nil || got.Meta.RequestID != "req_123" { - t.Errorf("_meta lost or mangled: %+v", got.Meta) - } -} - -func TestWriteEnvelopeFiltered_JQOnly(t *testing.T) { - env := format.Success(map[string]any{"items": []any{ - map[string]any{"id": "1", "name": "alpha"}, - map[string]any{"id": "2", "name": "beta"}, - }}, nil) - - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, nil, ".data.items[].id"); err != nil { - t.Fatalf("err = %v", err) - } - // gh CLI parity: string results render without JSON quotes - // (see https://github.com/cli/cli/blob/trunk/pkg/export/filter.go). - lines := strings.Split(strings.TrimRight(buf.String(), "\n"), "\n") - if len(lines) != 2 { - t.Fatalf("expected 2 lines, got %d: %q", len(lines), buf.String()) - } - if lines[0] != "1" || lines[1] != "2" { - t.Errorf("wrong output: %q", buf.String()) - } -} - -func TestWriteEnvelopeFiltered_FieldsAndJQ(t *testing.T) { - env := format.Success(map[string]any{"items": []any{ - map[string]any{"id": "1", "name": "alpha", "kb_id": "kb"}, - map[string]any{"id": "2", "name": "beta", "kb_id": "kb"}, - }}, nil) - - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, []string{"id", "name"}, ".data.items | length"); err != nil { - t.Fatalf("err = %v", err) - } - out := strings.TrimSpace(buf.String()) - if out != "2" { - t.Errorf("expected '2', got %q", out) - } -} - -func TestWriteEnvelopeFiltered_JQParseError(t *testing.T) { - env := format.Success(map[string]any{"x": 1}, nil) - buf := &bytes.Buffer{} - err := format.WriteEnvelopeFiltered(buf, env, nil, ".[invalid jq") - if err == nil { - t.Fatal("expected parse error, got nil") - } - if !strings.Contains(err.Error(), "jq parse") { - t.Errorf("expected 'jq parse' in error, got %q", err.Error()) - } -} - -func TestWriteEnvelopeFiltered_ScalarDataPassThrough(t *testing.T) { - // Defensive: scalar data should not crash field filter. - env := format.Success("just a string", nil) - buf := &bytes.Buffer{} - if err := format.WriteEnvelopeFiltered(buf, env, []string{"id"}, ""); err != nil { - t.Fatalf("err = %v", err) - } - if !strings.Contains(buf.String(), `"just a string"`) { - t.Errorf("scalar data lost: %s", buf.String()) - } -} diff --git a/cli/internal/secrets/secrets.go b/cli/internal/secrets/secrets.go index 1dac96bd..6e6b3627 100644 --- a/cli/internal/secrets/secrets.go +++ b/cli/internal/secrets/secrets.go @@ -34,8 +34,8 @@ type Store interface { } // FileStore writes 0600 plain-text files under $XDG_CONFIG_HOME/weknora/secrets/. -// It is the headless / CI default and the keychain fallback. Real keychain support -// is wired in v0.1 (see ADR-17 for namespace strategy). +// It is the headless / CI default and the keychain fallback (see ADR-17 for +// namespace strategy). type FileStore struct { root string } diff --git a/cli/internal/sse/accumulator.go b/cli/internal/sse/accumulator.go index 8940f47d..5fb702fc 100644 --- a/cli/internal/sse/accumulator.go +++ b/cli/internal/sse/accumulator.go @@ -3,9 +3,9 @@ // // Accumulator is the canonical sink: every callback event appends to a // buffered Content string and updates terminal-state fields like References -// and SessionID. The non-streaming JSON envelope mode reads .Result() once -// .Done() is true; streaming mode writes Content tokens directly to stdout -// and only consults the accumulator for the final References footer. +// and SessionID. The non-streaming JSON mode reads .Result() once .Done() +// is true; streaming mode writes Content tokens directly to stdout and +// only consults the accumulator for the final References footer. package sse import ( diff --git a/cli/internal/xdg/xdg.go b/cli/internal/xdg/xdg.go index 7f66dc15..feb38868 100644 --- a/cli/internal/xdg/xdg.go +++ b/cli/internal/xdg/xdg.go @@ -1,11 +1,7 @@ // Package xdg consolidates the XDG-rooted file path lookup and atomic-write -// idioms used by config / compat / secrets / future stores. -// -// Before extraction these patterns were copy-pasted across cli/internal/{config, -// compat, secrets}; reuse review surfaced 3× duplication of Path and 2× of the -// tmp+rename atomic-write recipe. Centralizing here means a single place to fix -// behavior (mode bits, fallback dirs, mkdir order, error wrapping) and one less -// thing to copy when v0.2 adds project-link / state stores. +// idioms used by config / compat / secrets / projectlink. Centralizing means +// a single place to fix behavior (mode bits, fallback dirs, mkdir order, +// error wrapping) instead of copy-pasting across stores. package xdg import (