8bcbf5a154
Empirical mainstream-CLI surveys (gh / kubectl / aws / gcloud / stripe /
flyctl / terraform / vercel / netlify / lark) drove five alignment
fixes — each replaces a weknora-only design choice that mainstream CLIs
do not share. No backwards-compat shims; the CLI has no v0.1 users yet.
1. Single --kb flag (was --kb-id + --kb mutually exclusive)
Survey: 0/7 mainstream CLIs use two parallel flags for "by id" vs
"by name". Single flag (gh -R, gcloud --project) or positional
(kubectl, stripe, terraform). Closest analog — gcloud --project —
collapses identifier types onto one flag.
Now: every command exposes one --kb flag; client-side prefix
detection (cmdutil.IsKBID looks for "kb_") routes id-form values
through directly and name-form values through ListKnowledgeBases.
Mirrors gcloud --project's id-or-name auto-detection.
Touched: search, chat, doc list / upload / delete, link.
Factory.ResolveKB chain trimmed from 5 levels to 4.
2. link supersedes init
Survey: only vercel and netlify ship both `init` AND `link` as
siblings, and they keep them semantically distinct. weknora's pair
wrote the same .weknora/project.yaml file with the same meaning,
differentiated only by interactivity — that's a flag concern, not
a command concern.
Now: cmd/init/ deleted. cmd/link absorbs the interactive flow:
- link --kb <id-or-name> → non-interactive write
- link on a TTY → interactive prompt (lists KBs)
- link non-TTY without --kb → CodeKBIDRequired
Always overwrites silently (matches vercel link / netlify link /
kubectl apply rather than git init's refuse-if-exists).
Dead code purged: --force flag, CodeProjectAlreadyLinked error code.
3. whoami dropped
Survey: 7/7 mainstream CLIs ship exactly one identity command —
never both a status and a whoami. gh / gcloud / stripe pick status
(config + live API); aws / kubectl / flyctl pick whoami (live API).
weknora's auth status was already a superset of whoami (host +
context + user + email + tenant_id + tenant_name vs user_id +
tenant_id), so dropping whoami preserves all functionality and
aligns with the gh / gcloud / stripe form.
4. kb get alias dropped
`view` was already primary (gh repo view / gh pr view convention);
`get` was kept as a cobra alias for v0.0/v0.1 callers. With no
v0.0/v0.1 users to break, the alias is just noise on the command
surface. Acceptance contract envelope cases renamed kb_get.* →
kb_view.*; goldens renamed in lockstep.
5. api refactored to gh shape (-X/--method, default GET, auto-POST)
gh CLI's signature is `gh api <endpoint> [--method M]` — single
positional path, method as a flag, default GET, auto-promoted to
POST when a body is supplied. weknora's previous `api <method>
<path>` inverted this and forced the method to be passed even for
GET — a needless deviation from our declared north star.
Now: `api <path> [-X METHOD] [--data ...]`. Exit-10 protocol
on the DELETE escape-hatch is preserved; -X DELETE still hits
ConfirmDestructive when -y absent.
Plus: AGENTS.md gains an explicit note that `doctor` is a deliberate
divergence from gh / lark — borrowed from `flutter doctor` / `brew
doctor` because RAG deployments routinely break on misconfigured
embeddings / storage / credentials and a 4-status structured envelope
is the cleanest surface for it.
Tests: 24 cli packages green (was 26 in PR-14; init + whoami packages
removed). Acceptance contract envelope cases for whoami removed,
kb_get → kb_view renamed, search args / mock path updated for the
kb_<id> form. e2e harness flag args updated. Factory.ResolveKB tests
rewritten for the single-flag shape. api_test driver updated for the
positional-path / -X-method shape.
269 lines
8.6 KiB
Go
269 lines
8.6 KiB
Go
package api
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/spf13/cobra"
|
|
|
|
"github.com/Tencent/WeKnora/cli/internal/cmdutil"
|
|
"github.com/Tencent/WeKnora/cli/internal/iostreams"
|
|
"github.com/Tencent/WeKnora/cli/internal/prompt"
|
|
sdk "github.com/Tencent/WeKnora/client"
|
|
)
|
|
|
|
// newTestClient stands up an httptest server with the supplied handler and
|
|
// returns an *sdk.Client targeting it plus a teardown closure. The real SDK is
|
|
// used so we exercise the same Raw() code path as production (header
|
|
// injection, JSON marshalling, etc.).
|
|
func newTestClient(t *testing.T, h http.HandlerFunc) (*sdk.Client, func()) {
|
|
t.Helper()
|
|
srv := httptest.NewServer(h)
|
|
return sdk.NewClient(srv.URL), srv.Close
|
|
}
|
|
|
|
func TestAPI_GetSuccess(t *testing.T) {
|
|
out, _ := iostreams.SetForTest(t)
|
|
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodGet || r.URL.Path != "/api/v1/foo" {
|
|
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
|
|
}
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_, _ = w.Write([]byte(`{"hello":"world"}`))
|
|
})
|
|
defer stop()
|
|
|
|
if err := runAPI(context.Background(), &Options{}, cli, "GET", "/api/v1/foo"); err != nil {
|
|
t.Fatalf("runAPI: %v", err)
|
|
}
|
|
got := out.String()
|
|
if !strings.Contains(got, `"hello":"world"`) {
|
|
t.Errorf("expected raw JSON body in stdout, got %q", got)
|
|
}
|
|
if !strings.HasSuffix(got, "\n") {
|
|
t.Errorf("expected trailing newline appended, got %q", got)
|
|
}
|
|
}
|
|
|
|
func TestAPI_GetSuccess_JSON(t *testing.T) {
|
|
out, _ := iostreams.SetForTest(t)
|
|
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
w.Header().Set("X-Request-Id", "req-123")
|
|
w.WriteHeader(http.StatusOK)
|
|
_, _ = w.Write([]byte(`{"value":42}`))
|
|
})
|
|
defer stop()
|
|
|
|
if err := runAPI(context.Background(), &Options{JSONOut: true}, cli, "GET", "/api/v1/foo"); err != nil {
|
|
t.Fatalf("runAPI: %v", err)
|
|
}
|
|
var env struct {
|
|
OK bool `json:"ok"`
|
|
Data struct {
|
|
Status int `json:"status"`
|
|
Headers map[string]string `json:"headers"`
|
|
Body map[string]any `json:"body"`
|
|
} `json:"data"`
|
|
}
|
|
if err := json.Unmarshal(out.Bytes(), &env); err != nil {
|
|
t.Fatalf("decode envelope: %v\n%s", err, out.String())
|
|
}
|
|
if !env.OK {
|
|
t.Errorf("expected ok:true, got %s", out.String())
|
|
}
|
|
if env.Data.Status != 200 {
|
|
t.Errorf("status: want 200, got %d", env.Data.Status)
|
|
}
|
|
if env.Data.Headers["Content-Type"] != "application/json" {
|
|
t.Errorf("Content-Type header missing: %v", env.Data.Headers)
|
|
}
|
|
if got, ok := env.Data.Body["value"]; !ok || got.(float64) != 42 {
|
|
t.Errorf("body.value: want 42, got %v", env.Data.Body)
|
|
}
|
|
}
|
|
|
|
func TestAPI_PostWithData(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
var seenBody []byte
|
|
var seenMethod, seenPath string
|
|
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
|
seenMethod = r.Method
|
|
seenPath = r.URL.Path
|
|
seenBody, _ = io.ReadAll(r.Body)
|
|
w.Header().Set("Content-Type", "application/json")
|
|
_, _ = w.Write([]byte(`{"id":"new"}`))
|
|
})
|
|
defer stop()
|
|
|
|
opts := &Options{Data: `{"name":"foo"}`}
|
|
if err := runAPI(context.Background(), opts, cli, "POST", "/api/v1/things"); err != nil {
|
|
t.Fatalf("runAPI: %v", err)
|
|
}
|
|
if seenMethod != http.MethodPost || seenPath != "/api/v1/things" {
|
|
t.Errorf("server saw %s %s, want POST /api/v1/things", seenMethod, seenPath)
|
|
}
|
|
// SDK marshals body via json.Marshal; json.RawMessage round-trips
|
|
// verbatim so the bytes server-side equal the --data argument.
|
|
if string(seenBody) != `{"name":"foo"}` {
|
|
t.Errorf("server received body %q, want %q", seenBody, `{"name":"foo"}`)
|
|
}
|
|
}
|
|
|
|
func TestAPI_DataFile(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
tmp := filepath.Join(t.TempDir(), "body.json")
|
|
payload := `{"k":"from-file"}`
|
|
if err := os.WriteFile(tmp, []byte(payload), 0o600); err != nil {
|
|
t.Fatalf("write temp: %v", err)
|
|
}
|
|
var seenBody []byte
|
|
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
|
seenBody, _ = io.ReadAll(r.Body)
|
|
_, _ = w.Write([]byte(`{}`))
|
|
})
|
|
defer stop()
|
|
|
|
opts := &Options{DataFile: tmp}
|
|
if err := runAPI(context.Background(), opts, cli, "POST", "/api/v1/x"); err != nil {
|
|
t.Fatalf("runAPI: %v", err)
|
|
}
|
|
if string(seenBody) != payload {
|
|
t.Errorf("body from --data-file: got %q, want %q", seenBody, payload)
|
|
}
|
|
}
|
|
|
|
func TestAPI_NotFound(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusNotFound)
|
|
_, _ = w.Write([]byte(`{"error":"missing"}`))
|
|
})
|
|
defer stop()
|
|
|
|
err := runAPI(context.Background(), &Options{}, cli, "GET", "/api/v1/missing")
|
|
if err == nil {
|
|
t.Fatal("expected error for 404")
|
|
}
|
|
if !cmdutil.IsNotFound(err) {
|
|
t.Errorf("expected resource.not_found, got %v", err)
|
|
}
|
|
}
|
|
|
|
func TestAPI_InvalidMethod(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
// No server needed: validation should fail before dispatch.
|
|
err := runAPI(context.Background(), &Options{}, nil, "FOO", "/api/v1/things")
|
|
if err == nil {
|
|
t.Fatal("expected error for unsupported method")
|
|
}
|
|
var ce *cmdutil.Error
|
|
if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputInvalidArgument {
|
|
t.Errorf("expected input.invalid_argument, got %v", err)
|
|
}
|
|
}
|
|
|
|
func TestAPI_PathWithoutSlash(t *testing.T) {
|
|
_, _ = iostreams.SetForTest(t)
|
|
err := runAPI(context.Background(), &Options{}, nil, "GET", "api/v1/things")
|
|
if err == nil {
|
|
t.Fatal("expected error for missing leading slash")
|
|
}
|
|
var ce *cmdutil.Error
|
|
if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputInvalidArgument {
|
|
t.Errorf("expected input.invalid_argument, got %v", err)
|
|
}
|
|
}
|
|
|
|
// withRootHarness wraps `weknora api ...` under a synthetic root cmd that
|
|
// registers the global `-y/--yes` persistent flag (mirrors addGlobalFlags in
|
|
// cmd/root.go). Required because api's NewCmd doesn't register --yes itself
|
|
// — it inherits from root in production.
|
|
func withRootHarness(api *cobra.Command, args ...string) *cobra.Command {
|
|
root := &cobra.Command{Use: "weknora"}
|
|
root.PersistentFlags().BoolP("yes", "y", false, "")
|
|
root.PersistentFlags().Bool("dry-run", false, "")
|
|
root.AddCommand(api)
|
|
root.SetArgs(append([]string{"api"}, args...))
|
|
root.SetContext(context.Background())
|
|
root.SilenceErrors = true
|
|
root.SilenceUsage = true
|
|
return root
|
|
}
|
|
|
|
// TestAPI_DELETE_RequiresConfirmation pins the exit-10 protocol on the
|
|
// escape-hatch DELETE path: agent invokes `weknora api DELETE /...` without
|
|
// -y/--yes, must get input.confirmation_required + exit 10. Confirmation is
|
|
// enforced in NewCmd.RunE (not runAPI), so the test drives the cobra cmd.
|
|
func TestAPI_DELETE_RequiresConfirmation(t *testing.T) {
|
|
iostreams.SetForTest(t) // non-TTY
|
|
f := &cmdutil.Factory{
|
|
Client: func() (*sdk.Client, error) { return nil, nil },
|
|
Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} },
|
|
}
|
|
root := withRootHarness(NewCmd(f), "/api/v1/knowledge-bases/kb_xxx", "-X", "DELETE")
|
|
err := root.Execute()
|
|
if err == nil {
|
|
t.Fatal("expected confirmation_required error for DELETE without -y")
|
|
}
|
|
var ce *cmdutil.Error
|
|
if !asTypedError(err, &ce) || ce.Code != cmdutil.CodeInputConfirmationRequired {
|
|
t.Errorf("want input.confirmation_required, got %v", err)
|
|
}
|
|
if got := cmdutil.ExitCode(err); got != 10 {
|
|
t.Errorf("exit code = %d, want 10", got)
|
|
}
|
|
}
|
|
|
|
// TestAPI_DELETE_WithYes_Proceeds: -y/--yes opt-in skips confirmation and
|
|
// dispatches to the SDK. Server returns 200 to verify the happy-path lands
|
|
// on the response body emit.
|
|
func TestAPI_DELETE_WithYes_Proceeds(t *testing.T) {
|
|
iostreams.SetForTest(t)
|
|
called := false
|
|
cli, stop := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodDelete {
|
|
t.Errorf("expected DELETE, got %s", r.Method)
|
|
}
|
|
called = true
|
|
w.WriteHeader(http.StatusOK)
|
|
})
|
|
defer stop()
|
|
f := &cmdutil.Factory{
|
|
Client: func() (*sdk.Client, error) { return cli, nil },
|
|
Prompter: func() prompt.Prompter { return prompt.AgentPrompter{} },
|
|
}
|
|
root := withRootHarness(NewCmd(f), "/api/v1/knowledge-bases/kb_xxx", "-X", "DELETE", "-y")
|
|
if err := root.Execute(); err != nil {
|
|
t.Fatalf("execute: %v", err)
|
|
}
|
|
if !called {
|
|
t.Error("DELETE handler not called — confirmation may have blocked")
|
|
}
|
|
}
|
|
|
|
// asTypedError is a tiny wrapper around errors.As that keeps the call sites
|
|
// concise. Returns true on success, populating dst.
|
|
func asTypedError(err error, dst **cmdutil.Error) bool {
|
|
for e := err; e != nil; {
|
|
if t, ok := e.(*cmdutil.Error); ok {
|
|
*dst = t
|
|
return true
|
|
}
|
|
type unwrapper interface{ Unwrap() error }
|
|
u, ok := e.(unwrapper)
|
|
if !ok {
|
|
return false
|
|
}
|
|
e = u.Unwrap()
|
|
}
|
|
return false
|
|
}
|