feat(cli)!: v0.10 reliability, agent-UX, and command-surface hardening
A hardening + finalization pass over the agent-first CLI: correctness fixes,
richer machine-readable signals, flag/naming consistency, and a symmetric
config surface. Pre-1.0, so it includes breaking renames.
Correctness:
- agent update: resolve/validate --model/--rerank-model (was storing a bogus
name verbatim, corrupting config.model_id).
- doctor: honor WEKNORA_HOST / WEKNORA_API_KEY (headless path no longer reports
"no host configured").
- session ask / MCP session_ask: text answer was empty on non-TTY — the agent
stream sets Done=true on an intermediate agent_query frame before the answer,
and AgentAccumulator treated the first Done as terminal. Terminate on the
`complete` event (new sdk.AgentResponseTypeComplete), not a per-frame Done.
- batch exit codes: any per-item failure collapses to operation.failed (exit 1),
including `doc upload --recursive` partial failures — a permanent per-file
failure (e.g. a duplicate) no longer surfaces as a retryable exit 7 an agent
would loop on; per-item typed errors stay in the envelope.
Agent-first signals & discovery:
- error.exit_code in the envelope (type + exit_code disambiguate the
input.invalid_argument exit-2-vs-5 split in one JSON read).
- meta.hint on empty content search and on draft doc create; doc wait fails fast
on a never-parsing draft instead of hanging to --timeout.
- retrieval-readiness is visible in the natural flow: kb status / kb check emit
retrieval_ready, and kb create hints the fix when no embedding model is bound
— an unconfigured KB no longer looks silently healthy.
- schema contract completeness: every leaf declares output + >=1 example
(drift-guarded); output strings match the meta actually emitted; chunk list
and search docs now emit meta.total_count (both previously dropped it).
- schema tolerates a quoted multi-word command label; zero-state auth — and
`link` with no profile — point at profile setup / the headless WEKNORA_KB_ID
path instead of looping on `auth login`.
- id-addressed reads tolerate a redundant --kb (doc view/wait, chunk list/view
accept and ignore it, declared in schema) so a carried-over --kb doesn't
exit 2; streaming commands warn that --jq does not apply to an NDJSON stream.
- keep JSON-always as the default; --jq hints spell out the .data path.
Consistency & gating:
- doc create: drop the deprecated --name alias (--title only; pre-1.0 break).
- chunk list --limit aligned to 1..10000; model list --limit/-L with
has_more/total_count; api write-gates -X PUT/PATCH (exit 10); skills install
expands a leading ~.
- docs corrected: search docs / doc list --keyword help is case-insensitive
(server does LOWER LIKE); AGENTS.md risk-action list (no phantom kb.init; add
model.update / kb.config.set) and batch example (failed item carries `error`);
session resume --message id comes from `message list`, not the stream.
- auth/profile ergonomics: env credentials are now first-class — `auth token`
prints the active WEKNORA_API_KEY / WEKNORA_TOKEN, and auth login/logout/refresh
give an env-aware message instead of looping on "run auth login". `auth logout`
clears credentials but keeps the profile registered (host preserved for
re-login); deleting a profile is `profile remove`'s job (clean logout/remove
separation, matching gh / lark).
Config surface (symmetric read/write, in-place model edits):
- kb config now returns a secret-free KBModelConfigView (was {}); `kb config`
reads, new `kb config set` writes; `kb init` removed (misnomer).
- kb create --chat-model: retrieval-ready in one step.
- model update: edit a model in place (id preserved, references survive) —
rotate --api-key-stdin, change base-url / display-name / etc.
- session continue-stream renamed to session resume.
Docs: AGENTS.md is the single wire-contract source; CHANGELOG slimmed; stale
kb-init / continue-stream references removed; skill wire-vocab guard extended.
AGENTS.md / weknora-shared SKILL document retrieval_ready (a KB needs an
embedding model to be searchable), that --jq does not apply to NDJSON
streams, and the env-credential-first auth path; the KB quickstart example
now creates a retrieval-ready KB.
This commit is contained in:
@@ -40,7 +40,11 @@ func RunBatch(ctx context.Context, ids []string, op func(context.Context, string
|
||||
for _, id := range ids {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
outcomes = append(outcomes, BatchOutcome{ID: id, Err: ctx.Err()})
|
||||
// Classify the context signal so the per-item envelope reports
|
||||
// operation.cancelled / operation.timeout (ClassifyContextErr)
|
||||
// instead of the generic internal.error, and an all-aborted batch
|
||||
// exits by that class. Cause preserved so errors.Is still matches.
|
||||
outcomes = append(outcomes, BatchOutcome{ID: id, Err: Wrapf(ClassifyContextErr(ctx.Err()), ctx.Err(), "operation on %s aborted", id)})
|
||||
failed++
|
||||
continue
|
||||
default:
|
||||
@@ -52,12 +56,18 @@ func RunBatch(ctx context.Context, ids []string, op func(context.Context, string
|
||||
}
|
||||
}
|
||||
if failed > 0 {
|
||||
// Any failure → operation.failed (exit 1). The aggregate code is
|
||||
// deliberately coarse: the per-item batch envelope already carries each
|
||||
// item's typed error (type + exit_code via ErrorToDetail), which is the
|
||||
// authoritative per-item signal an agent should branch on. Collapsing
|
||||
// "all failed with the same class" into that class was extra machinery
|
||||
// for a convenience the per-item data already provides.
|
||||
return outcomes, &Error{
|
||||
Code: CodeOperationFailed,
|
||||
Message: fmt.Sprintf("%d/%d operation(s) failed", failed, len(ids)),
|
||||
// Silent suppresses the stderr error envelope because the caller
|
||||
// already emitted the batch envelope to stdout. The exit code
|
||||
// still propagates via Error.Code → ExitCode (falls through to 1).
|
||||
// still propagates via Error.Code → ExitCode.
|
||||
Silent: true,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -72,8 +72,8 @@ func TestRunBatch_PartialFailure(t *testing.T) {
|
||||
|
||||
// TestRunBatch_StatusExitTriState verifies the batch tri-state exit mapping:
|
||||
// all-success → exit 0 (nil summaryErr), partial → exit 1, all-fail → exit 1
|
||||
// (operation.failed fall-through). Pairs with the envelope-status tri-state in
|
||||
// output.TestWriteBatchEnvelope_StatusTriState.
|
||||
// (any failure collapses to operation.failed). Pairs with the envelope-status
|
||||
// tri-state in output.TestWriteBatchEnvelope_StatusTriState.
|
||||
func TestRunBatch_StatusExitTriState(t *testing.T) {
|
||||
failIf := func(fails map[string]bool) func(context.Context, string) error {
|
||||
return func(_ context.Context, id string) error {
|
||||
@@ -224,3 +224,30 @@ func TestEmitBatch_Text_PerLine(t *testing.T) {
|
||||
t.Errorf("expected 'FAIL y: boom' line; got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunBatch_AllFailExit1 - any batch failure (partial OR all) collapses to
|
||||
// operation.failed → exit 1. The authoritative per-item detail lives in the
|
||||
// batch envelope (each item's typed error); the aggregate exit code is
|
||||
// deliberately coarse.
|
||||
func TestRunBatch_AllFailExit1(t *testing.T) {
|
||||
notFound := func(_ context.Context, id string) error {
|
||||
return NewError(CodeResourceNotFound, "no such thing "+id)
|
||||
}
|
||||
_, summaryErr := RunBatch(context.Background(), []string{"a", "b"}, notFound)
|
||||
if got := ExitCode(summaryErr); got != 1 {
|
||||
t.Errorf("all-fail batch ExitCode = %d, want 1; err=%v", got, summaryErr)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunBatch_ContextErrorsClassifiedPerItem verifies per-item context errors
|
||||
// are classified as operation.cancelled / operation.timeout in the batch
|
||||
// envelope (not the generic internal.error), so an agent inspecting the
|
||||
// per-item results sees why each item aborted. (The aggregate exit stays 1.)
|
||||
func TestRunBatch_ContextErrorsClassifiedPerItem(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel() // pre-cancel so every item takes the ctx.Done branch
|
||||
outcomes, _ := RunBatch(ctx, []string{"a"}, func(context.Context, string) error { return nil })
|
||||
if got := ErrorToDetail(outcomes[0].Err).Type; got != string(CodeOperationCancelled) {
|
||||
t.Errorf("per-item type = %q, want %q", got, CodeOperationCancelled)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -228,9 +228,11 @@ func ErrorToDetail(err error) *output.ErrDetail {
|
||||
detail := &output.ErrDetail{
|
||||
Type: string(typed.Code),
|
||||
Message: msg,
|
||||
ExitCode: ExitCode(err),
|
||||
Hint: hint,
|
||||
RetryArgv: retry,
|
||||
RetryAfterSeconds: typed.RetryAfterSeconds,
|
||||
Retryable: retryableForCode(typed.Code),
|
||||
Detail: typed.Detail,
|
||||
}
|
||||
if typed.Risk != nil {
|
||||
@@ -245,12 +247,14 @@ func ErrorToDetail(err error) *output.ErrDetail {
|
||||
var fe *FlagError
|
||||
if errors.As(err, &fe) {
|
||||
return &output.ErrDetail{
|
||||
Type: string(CodeInputInvalidArgument),
|
||||
Message: err.Error(),
|
||||
Hint: defaultHint(CodeInputInvalidArgument),
|
||||
Type: string(CodeInputInvalidArgument),
|
||||
Message: err.Error(),
|
||||
ExitCode: ExitCode(err), // FlagError → 2, distinguishing parse from typed-value (exit 5)
|
||||
Hint: defaultHint(CodeInputInvalidArgument),
|
||||
Retryable: retryableForCode(CodeInputInvalidArgument),
|
||||
}
|
||||
}
|
||||
return &output.ErrDetail{Type: "internal.error", Message: err.Error()}
|
||||
return &output.ErrDetail{Type: string(CodeInternalError), Message: err.Error(), ExitCode: ExitCode(err)}
|
||||
}
|
||||
|
||||
// NewError constructs a typed error.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package cmdutil
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"reflect"
|
||||
"testing"
|
||||
)
|
||||
@@ -30,3 +31,32 @@ func TestError_RetryArgv_EmptyByDefault(t *testing.T) {
|
||||
t.Errorf("RetryArgv should default empty; got %v", err.RetryArgv)
|
||||
}
|
||||
}
|
||||
|
||||
// TestErrorToDetail_CarriesExitCode verifies every error detail embeds the
|
||||
// authoritative exit_code so an agent can branch on a single JSON read without
|
||||
// observing $?. Regression: `input.invalid_argument` spans exit 2 (parse) and
|
||||
// exit 5 (typed value), so `type` alone was insufficient — exit_code
|
||||
// disambiguates them in the envelope.
|
||||
func TestErrorToDetail_CarriesExitCode(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
want int
|
||||
}{
|
||||
{"not_found", NewError(CodeResourceNotFound, "x"), 4},
|
||||
{"typed_input_value", NewError(CodeInputInvalidArgument, "bad value"), 5},
|
||||
{"auth", NewError(CodeAuthUnauthenticated, "x"), 3},
|
||||
{"parse_flagerror", NewFlagError(errors.New("unknown flag: --nope")), 2},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
d := ErrorToDetail(tc.err)
|
||||
if d == nil {
|
||||
t.Fatal("nil detail")
|
||||
}
|
||||
if d.ExitCode != tc.want {
|
||||
t.Errorf("exit_code = %d, want %d (type=%s)", d.ExitCode, tc.want, d.Type)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -127,7 +127,13 @@ func buildClient(f *Factory) (*sdk.Client, error) {
|
||||
}
|
||||
profileName := cfg.CurrentProfile
|
||||
if profileName == "" {
|
||||
return nil, NewError(CodeAuthUnauthenticated, "no current profile configured; run `weknora auth login` to set one up")
|
||||
// Zero-state: no profile exists at all. The generic auth.unauthenticated
|
||||
// default retry_argv is `auth login`, but that ALSO fails here (it needs
|
||||
// an active profile) — an agent that execs retry_argv would loop. Point
|
||||
// hint + retry_argv at the real first step: create a profile.
|
||||
return nil, NewError(CodeAuthUnauthenticated, "no profile configured").
|
||||
WithHint("add one first: `weknora profile add <name> --host <url> --use`, then `weknora auth login` (or set WEKNORA_API_KEY + WEKNORA_HOST for headless use)").
|
||||
WithRetryArgv([]string{"weknora", "profile", "add", "--help"})
|
||||
}
|
||||
prof, ok := cfg.Profiles[profileName]
|
||||
if !ok {
|
||||
@@ -254,6 +260,15 @@ func AddKBFlag(cmd *cobra.Command) {
|
||||
cmd.Flags().String("kb", "", "Knowledge base UUID or name (overrides env / project link)")
|
||||
}
|
||||
|
||||
// AddIgnoredKBFlag registers a no-op --kb on id-addressed commands (doc view /
|
||||
// wait, chunk list / view). Those resolve a globally-unique doc/chunk id, so
|
||||
// --kb is redundant — but an agent flowing from `doc upload --kb X` naturally
|
||||
// carries it, and rejecting it with exit 2 is pure friction. Accepted and
|
||||
// ignored; declared here so schema still lists it truthfully.
|
||||
func AddIgnoredKBFlag(cmd *cobra.Command) {
|
||||
cmd.Flags().String("kb", "", "Ignored — the id argument is globally unique; accepted for symmetry with `doc list`/`doc upload` so a carried-over --kb doesn't error")
|
||||
}
|
||||
|
||||
// ResolveKB returns the active KB id for the running command, applying the
|
||||
// 4-level fallback chain (highest to lowest):
|
||||
// 1. --kb flag (kb_<...> id passed through; anything else resolved via
|
||||
|
||||
@@ -174,6 +174,13 @@ func TestBuildClient_NoCurrentProfile(t *testing.T) {
|
||||
var typed *Error
|
||||
require.ErrorAs(t, err, &typed)
|
||||
assert.Equal(t, CodeAuthUnauthenticated, typed.Code)
|
||||
// Zero-state retry_argv must NOT be the generic `auth login` (which itself
|
||||
// fails with no profile → an agent execing retry_argv would loop); it must
|
||||
// point at profile creation instead.
|
||||
detail := ErrorToDetail(err)
|
||||
assert.NotEqual(t, []string{"weknora", "auth", "login"}, detail.RetryArgv,
|
||||
"zero-state retry_argv must not loop back to auth login")
|
||||
assert.Contains(t, detail.RetryArgv, "profile", "zero-state retry_argv should point at profile setup")
|
||||
}
|
||||
|
||||
func TestBuildClient_UnknownContext(t *testing.T) {
|
||||
|
||||
@@ -47,7 +47,12 @@ func AddFormatFlag(cmd *cobra.Command, fieldHints ...string) {
|
||||
if len(fieldHints) > 0 {
|
||||
sorted := append([]string(nil), fieldHints...)
|
||||
sort.Strings(sorted)
|
||||
hdr := "\n\nJSON fields available (for --jq projection):\n " +
|
||||
// Fields live under .data in the {ok,data,meta} envelope, so --jq must
|
||||
// be rooted there: `--jq '.data.<field>'` (object) or
|
||||
// `--jq '.data[].<field>'` (list). A bare `--jq '.<field>'` matches the
|
||||
// envelope top level and silently returns null — spell out the path so
|
||||
// agents don't ship broken projections.
|
||||
hdr := "\n\nJSON fields available under .data (project with --jq '.data.<field>', or '.data[].<field>' for lists):\n " +
|
||||
strings.Join(sorted, "\n ")
|
||||
if cmd.Long != "" {
|
||||
cmd.Long += hdr
|
||||
@@ -144,9 +149,14 @@ func mapJQError(err error) error {
|
||||
}
|
||||
|
||||
// ResolveDefault fills in Mode when the caller has not explicitly set it:
|
||||
// - Mode defaults to FormatJSON
|
||||
// - Mode defaults to FormatJSON, regardless of TTY
|
||||
// - TTY only affects the indent decision (auto-indent in TTY; compact in pipe)
|
||||
// - For human-readable rendering, pass --format text explicitly
|
||||
//
|
||||
// JSON-always (not a TTY switch to text on a terminal) is deliberate: an
|
||||
// agent-first CLI values output predictability over terminal ergonomics, so
|
||||
// the default never depends on whether stdout is a TTY. Humans opt into
|
||||
// human-readable output with `--format text`.
|
||||
func (o *FormatOptions) ResolveDefault(tty bool) {
|
||||
o.TTY = tty
|
||||
// Apply WEKNORA_FORMAT before the hard default so the documented
|
||||
|
||||
@@ -96,6 +96,9 @@ func TestFormatOptions_TextModeReturnsError(t *testing.T) {
|
||||
|
||||
// TestResolveDefault_AlwaysJSON verifies v0.7 semantics: default is FormatJSON
|
||||
// regardless of whether stdout is a TTY (BREAKING change from v0.6).
|
||||
// TestResolveDefault_AlwaysJSON pins the deliberate JSON-always default (no
|
||||
// TTY switch to text): the default output never depends on whether stdout is a
|
||||
// TTY, so agents get predictable JSON. Humans opt into text with --format text.
|
||||
func TestResolveDefault_AlwaysJSON(t *testing.T) {
|
||||
for _, isTTY := range []bool{true, false} {
|
||||
o := &FormatOptions{}
|
||||
@@ -117,7 +120,7 @@ func TestResolveDefault(t *testing.T) {
|
||||
isTTY bool
|
||||
wantMode FormatMode
|
||||
}{
|
||||
// v0.7: empty Mode always resolves to FormatJSON regardless of TTY.
|
||||
// JSON-always default regardless of TTY (predictable for agents).
|
||||
{"empty isTTY", "", "", true, FormatJSON},
|
||||
{"empty no-tty", "", "", false, FormatJSON},
|
||||
{"already set keeps value tty", FormatNDJSON, "", true, FormatNDJSON},
|
||||
|
||||
@@ -69,7 +69,7 @@ func TestValidateProfileName_RejectsShellMetachars(t *testing.T) {
|
||||
for _, name := range cases {
|
||||
err := ValidateProfileName(name)
|
||||
if err == nil {
|
||||
t.Errorf("ValidateProfileName(%q) should have rejected the name; an agent exec'ing retry_command would be injectable", name)
|
||||
t.Errorf("ValidateProfileName(%q) should have rejected the name; a name echoed into retry_argv / prose would be injectable", name)
|
||||
continue
|
||||
}
|
||||
var ce *Error
|
||||
|
||||
@@ -14,7 +14,7 @@ package cmdutil
|
||||
import "github.com/spf13/cobra"
|
||||
|
||||
// Risk levels emitted in the annotation / envelope:
|
||||
// - RiskDestructive: irreversible ops (delete, kb init clobber).
|
||||
// - RiskDestructive: irreversible ops (delete).
|
||||
// - RiskWrite: reversible metadata edits (kb / agent / doc update).
|
||||
//
|
||||
// "read" remains reserved (read-only commands carry no risk annotation).
|
||||
|
||||
Reference in New Issue
Block a user