3f9b09e3063741d2fcf6699ff2b2a77ca0083d53
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
e236be1ced |
fix(cli): correct KB id detection, SSE terminal-frame, and CI test isolation
Three defects surfaced during end-to-end RAG verification — the first two
block real chat usage, the third makes Linux CI flaky:
1. KB id detection — `IsKBID` was checking
`strings.HasPrefix(s, "kb_")`, but WeKnora generates KB ids as bare
UUIDs (internal/types/knowledge_base.go: `uuid.New().String()` stored
in a `varchar(36)` column). Real ids therefore fell through to the
name-resolution path:
$ weknora chat ... --kb a32a63ff-fb36-4874-bcaa-30f48570a694
Error: knowledge base not found: a32a63ff-...
Switched the discriminator to a UUID regex
(`^[0-9a-fA-F]{8}-…-[0-9a-fA-F]{12}$`). KB names are arbitrary
user-supplied strings, so the canonical 8-4-4-4-12 form is an
unambiguous signal. Mirrors gcloud `--project`'s id-vs-name detection.
2. SSE terminal-frame — the accumulator's `Append` was gating
finalization on `r.Done`, but the server's KnowledgeQAStream protocol
emits a leading `agent_query` frame with `done=true` to deliver
session + message metadata *before* the answer fragments arrive:
event: message
data: {"response_type":"agent_query","content":"","done":true,…}
event: message
data: {"response_type":"answer","content":"你好","done":false}
…
event: message
data: {"response_type":"complete","content":"","done":true}
The accumulator therefore flipped to `finished=true` on frame #1 and
discarded every subsequent answer fragment — `weknora chat … --json`
returned `answer: ""` even though the LLM reported completion_tokens
> 0. Fixed: terminate only on `response_type == complete`.
References still captured opportunistically (they may arrive on a
dedicated `references` event before the terminator).
3. doctor credential_storage CI isolation — the check probes the real
OS keyring via `secrets.NewBestEffortStore()`: present on macOS dev
machines → StatusOK; absent on Linux CI runners without libsecret /
Gnome-Keyring → StatusWarn ("falling back to file store"). That
host-dependence was leaking into two test classes that assumed
StatusOK:
* cmd/doctor/doctor_test.go: TestDoctor_AllOK and
TestDoctor_NoConfig_StillRunsCredentialStorage already had a
withCredStoreFactory seam but didn't use it. Added the pin.
* acceptance/contract/envelope_test.go: doctor.success_offline
and doctor.error_network golden cases. The contract test runs
through the cobra tree in-process and shares cmd/doctor's
package-level credStoreFactory var — but couldn't reach it
because the existing seam was unexported.
Fix: export `doctor.SetCredStoreFactoryForTest(fn) (restore func())`
for out-of-package tests; acceptance/contract/helpers_test.go adds
a TestMain that pins the factory to a MemStore-returning closure
for the whole suite (MemStore is neither *FileStore nor a real
keyring, so doctor's type-switch hits StatusOK). Production stays
at secrets.NewBestEffortStore — only the test hook is now reachable
from across packages.
Test fixtures and goldens that used the old `kb_xxx` literals or
`Done: true` terminators were rewritten to use real UUIDs and
`ResponseType: ResponseTypeComplete` respectively. Per-command --help
text and Long descriptions / Examples now show a UUID rather than
`kb_…` so users see the correct shape from the start. New
TestAccumulator_IgnoresAgentQueryDone pins the SSE terminator bug so
it can't regress.
Tests: 24 cli packages green on macOS dev + Linux/macOS/Windows CI
matrix. Verified end-to-end against a live WeKnora server: `weknora
chat "..." --kb <UUID> --no-stream --json` returns the full LLM answer
in the envelope, live token streaming in TTY mode works, and the
credential_storage check renders deterministic envelopes across hosts.
|
||
|
|
8bcbf5a154 |
refactor(cli): align command surface with mainstream conventions
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.
|
||
|
|
f7d7c8054d |
chore(cli): remove unused v0.0 scaffolding
Foundation PR-1 reserved several internal packages and helpers as
scaffolding for follow-up PRs that ended up taking different routes.
Audit confirms zero production references; this commit removes them so
the cli/ tree reflects what's actually shipped.
Removed (148 LOC):
cli/internal/safepaths/ — `Validate` / `WithinRoot` /
three sentinel errors. Reserved
for `weknora doc upload`'s path
scrubbing; that command landed
in PR-10 using its own
`validateUploadPath` (os.Stat +
regular-file check) — sufficient
for the actual threat model
(local CLI invocations).
cli/internal/cmdutil/json_flags.go — `AddJSONFlags` helper +
unused --jq / --template flag
registration. Reserved for PR-3
"lipgloss tables / jq evaluator"
which never materialized; every
command directly registers
BoolVar(&JSONOut, "json", ...)
since v0.0 ship time.
cmdutil.NewTableExporter — empty alias for jsonExporter,
reserved for the same PR-3
renderer. Removed; jsonExporter
stays under NewJSONExporter.
cmdutil.Options marker interface — empty interface{} reserved as a
convention; no command embeds
or asserts against it.
Stale comments fixed:
- cmd/root.go: package comment updated kb (list+get) → kb
(list+view+create+delete) and noted the `get` cobra alias.
- cmd/root.go: dropped --no-version-check forward-reference (no such flag).
- cmd/root.go: removed "(PR-7)" attribution from NewRootCmd doc comment.
- cmd/kb/kb.go: same package-comment update.
- cmd/chat/chat.go: replaced "PR-7" mention in --help example with a
generic placeholder so cobra-rendered help is review-clean.
- cmd/search/search.go: removed "Lipgloss tables arrive in PR-3"
forward-reference; the inline indent helper is the shipped form.
- internal/agent/annotations.go: ShouldUseAgentMode → DetectAIAgent
(removed in PR-12).
AGENTS.md "Known limitations" section added:
Documents that chat / search / doc upload currently surface server-side
precondition misses (LLM / vector store / storage engine not configured)
as `network.error` with `context deadline exceeded`. A planned future
release will introduce a `precondition.*` typed error namespace
(server returns HTTP 412 before opening the SSE / streaming response).
This documents the limitation honestly for reviewers and integrators
rather than claiming a behavior we don't yet have.
Tests: 27 cli packages pass (safepaths_test was the 28th — gone with the
package). go vet clean.
|
||
|
|
3fb3583a92 |
feat(cli): add api passthrough, chat streaming, doctor warn status
Close the v0.2 RAG demo loop and ship the validation infrastructure:
- weknora api <method> <path> [--data X | --data-file F]
Raw passthrough wrapping client.Raw, gh-style. JSON envelope mode
surfaces status / headers / parsed body. Non-2xx routes through
cmdutil.ClassifyHTTPStatus (factored out of ClassifyHTTPError so
both SDK-error and direct-status paths stay aligned — reuse review).
- weknora chat <text> [--session-id S] [--no-stream]
KnowledgeQAStream consumer with two output modes:
- TTY default: token streaming + references footer
- --json / --no-stream / non-TTY: buffered single envelope
Auto-creates a session when --session-id is omitted; the id prints
to stderr at start AND on stream failure (^C scrolls past the
first announcement, so the recovery hint is re-surfaced when the
user is most likely to need it).
- cli/internal/sse/Accumulator
buffers Content / References / SessionID across SDK callbacks.
Idempotent post-Done so misbehaving servers don't corrupt state.
- doctor: ok → ok / warn / fail / skip
warn marks soft issues that don't block: server within compat range
but >=1 minor behind CLI; credential storage falling back to file
because keyring is unavailable. Envelope.ok stays true on warn,
flips false on fail (exit 1). doctor.error_network golden updated.
- cli/acceptance/e2e/ real-server RAG full loop
Build-tagged //go:build acceptance_e2e — kept out of the default
`go test ./...`. Exercises kb create → doc upload → poll ready →
search → chat → cleanup against a server pointed at by
WEKNORA_E2E_HOST / _TOKEN.
- .github/workflows/cli-e2e.yml
manual workflow_dispatch + label-gated PR trigger
("acceptance-e2e"). No-ops gracefully when the secrets aren't set
so cross-fork PRs can't accidentally fail the suite.
|