cd65665894ac2fd0b72e4d6a4cdf7dd05ea7db5e
12 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
fc98f000d8 |
feat(upload): add process configuration support to CreateKnowledgeFromFile
- Updated the CreateKnowledgeFromFile function to accept an optional processConfig parameter, enhancing flexibility in knowledge file uploads. - Modified related service and test files to accommodate the new parameter, ensuring consistent handling across the upload process. |
||
|
|
2ee9741fa1 |
refactor(cli): finish context→profile cascade + post-review hardening (BREAKING)
Post-review polish on the v0.7 wire / surface contract. Bundles five
follow-ups that landed after the main BREAKING feat commit:
1. Complete context→profile cascade (internal API + YAML schema)
The prior commit renamed only the user-visible surface (commands /
flags / env / project link / envelope field). The internal Go API
and on-disk config schema were still half-renamed — an L-25
self-consistency violation flagged by post-merge review. Closed here:
Internal Go API:
- config.Context → config.Profile
- config.Config.CurrentContext → CurrentProfile
- config.Config.Contexts → Profiles
- LoginOptions.Context → LoginOptions.Profile
- clearContextSecrets() → clearProfileSecrets()
- saveContextRef() → saveProfileRef()
- secrets.Store: param name `context` → `profile` (interface +
FileStore + KeyringStore + MemStore)
- cmdutil.LoadSecret(store, context, key) → LoadSecret(store, profile, key)
- cmdutil.RefreshAndPersist's ctxName → profileName
- Local var `ctx := &config.Profile{...}` → `prof := &config.Profile{...}`
in auth/login.go to eliminate the visual collision with Go stdlib
context.Context that motivated the whole rename in the first place.
On-disk config.yaml schema:
- current_context: → current_profile:
- contexts: → profiles:
- Pre-1.0 break, no compat alias. Users on v0.6 dogfooded configs
must delete ~/.config/weknora/config.yaml or hand-rename the two
keys (CHANGELOG migration note added).
Tests / fixtures / golden files:
- factory_test.go YAML fixture + assertion updated.
- acceptance/e2e/e2e_test.go writeContextYAML → writeProfileYAML,
fixture YAML keys updated.
- acceptance/testdata/wire/doctor.error_network.json golden updated
("active context" → "active profile" in hint string).
User-visible prose sweep:
- cmd/mcp/serve.go --help Long: "active context (or --context)" →
"active profile (or --profile)" — most-visible miss.
- cmd/{kb/list, search/kb, session/list, api/api} Short/Long help.
- cmd/auth/login.go stdout: `(context=%s)` → `(profile=%s)`.
- cmd/auth/logout.go error: `"no current context"` → `"no current profile"`.
- cmd/doctor/doctor.go hint string (also the wire golden above).
- cmd/auth/refresh.go error: `"refresh token missing for context"` →
`"refresh token missing for profile"`.
- README.md: `## Multi-context` H2 → `## Multi-profile`; code-block
comment `# current context` → `# current profile`.
Code-comment / docstring sweep across cli/cmd/auth/ and
cli/internal/cmdutil/. Comments referencing Go stdlib context.Context,
the RAG / LLM "context window" concept, and historical CHANGELOG
entries for v0.4 / v0.5 were left alone.
CHANGELOG v0.7 BREAKING entry gains the on-disk-schema bullet under
the existing "context → profile" item.
2. Profile name validation (shell-injection guard)
`envelope.error.retry_command` is a single shell-string field. An
AI agent that exec()s it via `sh -c <retry_command>` was injectable
through a maliciously-named profile:
weknora auth logout --name 'x; rm -rf ~'
# would produce: retry_command = "weknora auth logout --name x; rm -rf ~ -y"
`cmd/profile/add.go` already enforced an alphanumeric + `-_.`
allowlist via `validateName`. The `auth login` and `auth logout`
paths bypassed it.
- Moved validation from `cmd/profile/add.go` to
`cli/internal/cmdutil/profilename.go` as exported
`ValidateProfileName` (cmdutil is the import-cycle-safe home;
internal/config can't depend on cmdutil).
- `auth login` runs the validator before any persist call.
- `auth logout` runs the validator on `opts.Name` before
constructing `retry_command`.
- Unit tests (`profilename_test.go`) cover the allowlist, empty
rejection, path-traversal, shell metacharacters (`;`, `&`, `|`,
`$()`, backticks, quotes, whitespace, glob, redirects), and the
user-facing hint text. The shell-metachar test exists as a
regression guard.
Wire shape (`retry_command` string → `retry_command_argv []string`)
remains a v0.8 additive change per ROADMAP — this fix removes the
practical exploit path without touching the wire contract.
3. AI-agent terminology disambiguation
"agent" has three referents in this codebase: (a) WeKnora's
server-side Custom Agent resource, (b) the removed `agent invoke`
verb, (c) external LLM/automation consumers. Per project memory
feedback_no_meta_disambiguation_in_docs, the fix is full-term
naming, not "X has N meanings" prose. Surgical changes at section
headers + ambiguous prose:
- AGENTS.md: "Agent decision shortcuts" → "AI agent decision
shortcuts"; "agent-callable surface" → "AI-agent-callable
surface".
- README.md: "Designed to be agent-first" → "AI-agent-first";
"Other agent ergonomics" → "Other AI-agent ergonomics"; "in
agent contexts" → "in AI-agent contexts"; "for CI / agents" →
"for CI / AI agents".
Anaphoric "agents" inside paragraphs that already established
"AI agents" was left alone — full substitution everywhere would
have been prose noise without clarity gain.
4. Wire-contract review follow-ups
Real findings from a second-pass review of the v0.7 envelope /
streaming / surface design. Per project memory
feedback_check_in_domain_anchor_first, candidate findings were
first verified against the in-domain peer CLI explicitly cited as
the envelope anchor; two earlier-flagged issues turned out to be
in-pattern and were withdrawn.
Surviving fixes:
- AGENTS.md success-envelope example rewritten. The prior example
showed `has_more: false` / `_notice: {}` as if they were always
present, but both fields are `omitempty` and never serialize
when zero / nil. Replaced with three realistic shapes (list /
single resource / mutation with no payload) and added a note
that optional fields are omitted when empty.
- cmd/chat/chat.go Args: MinimumNArgs(1) → ExactArgs(1).
v0.6 silently joined `weknora chat hello world` into
`"hello world"`. v0.7 now rejects multi-arg with exit 2,
matching `weknora session ask`. BREAKING; CHANGELOG entry
added under v0.7 BREAKING.
- internal/output/envelope.go extracts NewEnvelope(data, meta,
profile) constructor. The jq-filter path in
cmdutil.FormatOptions.Emit was manually rebuilding the
envelope literal alongside the canonical WriteEnvelope path —
drift risk when fields are added. Single construction point now.
- internal/cmdutil/factory.go adds AddKBFlag(cmd) helper.
Five files (chat, doc/list, doc/upload, doc/create, doc/fetch)
had verbatim-identical `cmd.Flags().String("kb", ...)`
declarations. Centralised so flag name + help text stay
in sync with Factory.ResolveKB. Docstring reordering + gofmt
fixup landed in the same edit to keep ResolveKB's own godoc
attached to its function.
5. OSS-readiness comment / doc sweep
Pre-publication scrub of code, comments, and shipped Markdown to
remove references that only make sense in the development repo:
- AGENTS.md "Deliberate deviations + mainstream alignments"
section: removed peer-project name-drops from the comparison
table; rewrote as five flagged design decisions with rationale
but no specific competitor named. The four rows that previously
contrasted against a named peer CLI now state WeKnora's choice
+ rationale directly. Section header renamed to "Design
decisions worth flagging" since it is no longer a
deviation/alignment matrix.
- CHANGELOG v0.7 BREAKING rationales: three references to a
named peer CLI removed; the context→profile rationale now
cites only mainstream multi-credential CLIs by category (AWS /
Stripe / OpenAI / Anthropic), and the `api -d/--data` removal
rationale cites only `gh api` / `curl`. `chat` BREAKING entry
rationale similarly simplified.
- 35 cross-references to design-spec section numbers (§4.1 /
§4.5 / §5.3 etc.) removed from Go doc comments and test
comments across 13 files. The referenced spec lives outside
the shipped tree; readers of the public repo cannot resolve
them. Each reference replaced with a self-contained semantic
description (e.g. "the batch envelope" / "AGENTS.md section
on the success path").
- Mixed-language strings translated to English:
- Four Go comments: internal/cmdutil/exit.go:213,215,
internal/cmdutil/errors.go:156,
internal/output/batch_test.go:90,
internal/output/envelope_test.go:27.
- One CHANGELOG section title:
`v0.7 — Agent-first wire contract + 命令面集中清理` →
`... + command-surface cleanup`.
- CJK test fixtures (internal/text/truncate_test.go CJK
truncation cases, cmd/session/list_test.go Chinese session
title, acceptance/e2e/e2e_test.go Chinese RAG corpus)
retained — they are intentional test inputs, not stray prose.
- Makefile help comment: `golangci-lint added in PR-9` →
`golangci-lint planned`. Internal PR numbering should not
surface in shipped Makefile prose.
Build green, 28/28 packages, +5 new ValidateProfileName tests.
go vet / gofmt / go mod verify / go mod tidy all clean.
Rationale for the cascade: pre-1.0 is the cheapest moment to close
L-25 self-consistency (L-26). The half-finished internal rename
would have perpetuated the very `context` vs `context.Context`
ambiguity that motivated v0.7's user-visible rename in the first
place.
|
||
|
|
2ce348d020 |
feat(cli): --format json default + NDJSON event stream + context→profile cascade + help calibration + docs (BREAKING)
D1 — --format default flipped to json regardless of TTY: - v0.6: smart default (text on TTY, json on pipe). - v0.7: always json; TTY only affects indent (compact in pipe). Enum values unchanged (text | json | ndjson). - Typed FormatMode enum replaces untyped string consts. - --format / --jq promoted to persistent root flags so unknown- subcommand paths still reach the typed-envelope guard (per-command registration in v0.6 would have rejected --format on unknown commands as cobra-prose exit 2). - WEKNORA_FORMAT env var added; precedence --format > env > default. Invalid env values silently ignored. D2 — chat / session ask default to NDJSON event-stream: - New cli/internal/output/ndjson_stream.go: InitEvent struct + EmitInit / EmitSDKEvent / WriteNDJSONLine helpers. EmitInit doc encodes the must-be-first-line invariant agents key on. - chat / session ask: --format json AND --format ndjson both emit one JSON event per line (no envelope wrapping). CLI injects exactly one `init` event at stream head carrying session_id + optional kb_id / agent_id / profile. Subsequent events pass through verbatim from the SDK (passthrough discipline per spec §5.1). - --format text keeps the SSE-style live renderer. context → profile full cascade: - Command group: cli/cmd/context/ → cli/cmd/profile/ (git mv; package contextcmd → profilecmd). - Global flag --context → --profile. Factory.ContextOverride → ProfileOverride. WEKNORA_PROFILE env var honored (--profile flag > env > config.CurrentContext). When --profile or WEKNORA_PROFILE references a missing profile, the error is input.invalid_argument with hint "weknora profile list" — not the destructive local.config_corrupt path (which would have told users to delete their config file). - Binding file .weknora/project.yaml field context: → profile: (no backwards-compat alias; re-run weknora link). - profile use JSON fields current_context / previous_context → current_profile / previous_profile. - weknora link JSON field context → profile. - CodeLocalContextNotFound → CodeLocalProfileNotFound (typed code rename). - Envelope top-level profile field populated via globalProfile (set by root PersistentPreRunE from Factory.ActiveProfile). chat / session ask NDJSON init event carries the same profile. - Rationale: "context" collided with LLM context window / RAG context / Go context.Context; mainstream multi-credential CLIs (AWS / Stripe / OpenAI / Anthropic / lark) all use "profile". H2/C1' help calibration: - AgentHelp gains Warnings []string; single SetAgentHelp helper routes on WEKNORA_AGENT_HELP=1 (emits JSON blob including warnings) vs human help (appends "AI agents:" block from same source). Warnings surface as both a structured JSON field and visible help-text addendum without drift. - 9 destructive commands carry warnings: kb / doc / agent / session / chunk delete; profile remove; kb / agent edit; auth logout. - weknora doc wait dedups ids at entry; SIGINT mid-wait returns silently (root signal handler maps to exit 130) instead of being miscategorised as operation.timeout / operation.failed. A4 — docs: - cli/AGENTS.md gains four agent-facing sections: Wire contract for AI agents (stdout / stderr / NDJSON / _notice evolution / SDK contract boundary); Deliberate deviations + mainstream alignments; Pre-1.0 breaking policy; Exit-10 anti-patterns. ERROR_REFERENCE table extended. - cli/README.md adds Agent quick start under Wire contract. - cli/CHANGELOG.md v0.7 section: BREAKING entries with migration notes, Added (WEKNORA_FORMAT / WEKNORA_PROFILE / retry_command / retry_after_seconds / risk / _notice reserved infra / meta.count / meta.has_more / doc fetch / doc create / session ask / doc delete --all / NDJSON init), Changed (docs additions), Deprecated (none — pre-release one-shot breaking). Spec: docs/superpowers/specs/2026-05-20-weknora-cli-v0.7-design.md §3 / §4 / §5 / §6 / §11 |
||
|
|
a2e368b1e8 |
refactor(cli): command-surface rename — session ask / doc fetch / doc create / doc delete --all
Three command renames consolidate the v0.7 verb table:
- `weknora agent invoke` → `weknora session ask --agent <id>`.
Server route POST /sessions/{session_id}/agent-qa is session-
anchored, so the verb moves with it. `weknora agent` subtree keeps
CRUD only (list / view / create / edit / delete / status / check).
SDK call (AgentQAStreamWithRequest) preserved verbatim; only the
command surface + flag layout move.
- `weknora doc upload` split into three commands:
- `weknora doc upload <file>` — local file (only).
- `weknora doc fetch <url>` — server-side remote fetch (was
`upload --from-url`). URL-only flags (--title / --file-type /
--tag-id) move with the verb.
- `weknora doc create --text` — direct text knowledge entry via
server CreateManualKnowledge.
- `weknora kb empty <id>` → `weknora doc delete --all --kb=<id>`.
Atomic server ClearKnowledgeBaseContents (no list-then-delete
race). Same exit-10 -y/--yes guard as other destructive verbs;
unified through the extended ConfirmDestructive helper.
Parent commands (agent, kb, doc, chunk, session, auth, profile,
search) lose their explicit Args:NoArgs + Run:cmd.Help so the
unknown-subcommand guard fires correctly — `weknora agent invoke
ag_x q` now emits the typed input.unknown_subcommand envelope with
detail.available[] instead of cobra's free-form exit-2 prose.
Spec: docs/superpowers/specs/2026-05-20-weknora-cli-v0.7-design.md §3.4 / §10.7
|
||
|
|
34bb0b5096 |
feat(cli): doc delete multi-id + api --paginate + paginate fixes + batch deletes
Batch-write surface and pagination consistency:
* weknora doc delete <doc-id> [<doc-id>...] — positional multi-id, default
keep-going on failure. Single -y confirms entire batch.
* weknora session delete <session-id> [<session-id>...] — same shape.
* weknora chunk delete <chunk-id> [<chunk-id>...] --doc <doc-id> — multi-id
with shared --doc parent.
* Multi-id partial-failure rolls up as operation.failed (exit 1), not
server.error (exit 7) — failures are operation outcomes, not transient
transport issues, and the retry-with-backoff hint for server.* would
mislead callers.
* weknora api <path> --paginate — auto-walks offset pagination and merges
pages into a single {data, total} JSON response.
* Paginate truncation fix across 6 list/follower call sites.
* All doc / search / chunk / session / kb list commands migrated to
FormatOptions API.
Multi-id RunE only emits the {ok, failed} envelope when the operation
actually ran — pre-flight failures (e.g. confirmation_required) leave
stdout empty per the wire contract.
doc upload's missing-positional-or-flag check is wrapped as FlagError so
the exit code (2) matches the convention used by other commands that
require a positional argument directly.
|
||
|
|
f89d54362d |
feat(cli): doc/kb resource expansion — upload flags + list filters + view fields
Closes deep-tuning gaps in v0.4-shipped doc / kb / session commands. Each command had multiple SDK fields the CLI silently hardcoded or omitted; this commit threads them through. doc upload: - --enable-multimodel (tri-state via NoOptDefVal): toggle multimodal extraction (PDF/DOCX image-to-text) per upload. Empty explicit value (e.g. --enable-multimodel="" from uninterpolated shell var) rejected as input.invalid_argument rather than silently coerced. - --metadata key=value (repeatable): attach arbitrary metadata - --channel <name> (default api, override for browser / wechat ingests) - URL mode gains --title / --file-type / --tag-id; URL-only flags rejected with input.invalid_argument when used without --from-url doc list filter flags: - --keyword (server-side LIKE — case-sensitive per PG) - --file-type / --source / --tag-id - --start-time / --end-time (RFC3339) search docs switches from client-side substring to server-side Keyword via ListKnowledgeWithFilter — smaller wire payload, but case-sensitivity shifts (documented in CHANGELOG + help text + Long). MCP doc_list schema gains the same 6 filter fields (parity with CLI). session view --full + --limit: loads chat history via LoadMessages SDK method. kb view human KV expanded: type / pinned / temporary / processing state + count / summary model / created timestamp. All omit-empty. doc view human KV expanded: title (when distinct from filename) / description / source / channel / summary_status / enable_status / tag_id / storage (human bytes) / file_hash (12-char prefix). |
||
|
|
69fbbfc252 |
chore(cli): gofmt sweep
`gofmt -w cli/` — trailing newlines, minor whitespace alignment. Caught during PR-readiness review (`gofmt -l .` had been non-clean since the em-dash → ASCII bulk replace earlier in the v0.4 series). |
||
|
|
f2e8e3f56c |
refactor(cli): drop aiclient package; align AGENTS.md with mainstream
Survey of 10 mainstream CLIs (gh, lark, stripe, vercel, supabase, aws, azure, gcloud, openai/codex, github-copilot-cli) showed env-gated per-command --help blurbs are a Stripe-only pattern; gh uses env detect for telemetry only, and lark relies on installed agent Skills + MCP. Our cmd/mcp/serve already covers the dominant 2025/26 path, so internal/aiclient/ (136 LOC + 38 callsites) is net maintenance burden without precedent. - Drop internal/aiclient/ entirely (annotations + detect + tests) - Remove 38 SetAgentHelp callsites + agentAwareHelpFunc / SetHelpFunc wiring in cmd/root.go - Migrate 4 command-level rules to standard Long help (visible to all, not env-gated): doc upload mode mutex, kb edit at-least-one, kb pin idempotent, search chunks channel mutex - Rewrite AGENTS.md as a developer guide (gh-style 6 H2 / 167 lines): audience preamble + Build / Architecture / Command Structure / Testing / Code Style / Error Handling. Drops sections absent in surveyed projects (Commit & PR Conventions, Who Uses This CLI) - Clean 14 internal doc refs (ADR-N, spec §X, v0.X) in source comments and docs that pointed at docs/superpowers/ — that directory is local-only / uncommitted, so refs are dead for outside readers - Drop forward-looking "once v0.2 ships" from README |
||
|
|
cc8254f862 |
refactor(cli): drop --dry-run + introduce bare-JSON output path
Two intertwined mainstream-alignment moves bundled because they share
the migration target (every command's --json path):
1. Drop --dry-run entirely. Survey of comparable API-wrapper CLIs
(gh, aws, stripe, lark): none expose --dry-run. The mainstream that
does (kubectl/git/helm/ansible) operates on declarative manifests
or local state where the preview is materially different from the
executed action. WeKnora's CLI just echoed the same parameters
that would have gone on the wire — the preview added no real
signal over `--help` + reading the call site. Removes:
- root --dry-run persistent flag + cmdutil/dryrun.go
- DryRun fields + EmitDryRun calls in 12 write commands
- format.Envelope.DryRun field
- 8 corresponding *_test.go cases
- --dry-run mention from README.md and CHANGELOG.md
- "dry_run":false from 16 golden envelopes
2. Migrate every --json output to bare data:
- New format.WriteJSON / WriteJSONFiltered helpers
(cli/internal/format/bare.go) share filterArrayItems /
filterObjectKeys / writeJQ with the (still-live for now) envelope
filter helpers.
- Read commands (kb/doc/session list+view, search chunks/docs/
sessions/kb, auth list/status, agent list/view, context list,
doctor) emit bare arrays / objects on stdout.
- Write commands (kb create/edit/delete/pin/empty, doc upload/
upload_recursive/delete, session delete, auth login/logout/
refresh/token, link/unlink, context add/use/remove, agent
invoke, chat, api, version) emit bare result objects. Risk
classification dropped — the resource + exit code already
convey the action.
Per-command shape changes:
list / search → []T (was {ok, data:{items:[…]}})
view → T (was {ok, data:T, _meta:…})
create / edit → T
delete / pin / etc. → {id, …action result…}
doctor → {summary, checks}
api → {status, headers, body}
_meta dropped on the read path:
pagination (page/page_size/total/has_more) — agents iterate with
--all-pages or accept --limit (gh CLI parity);
kb_id / context echo — caller already knows what it asked for.
Acceptance contract goldens regenerated for the new bare shape.
Error envelope on stdout (PrintErrorEnvelope) stays live for now —
the envelope-infra deletion lands in the next commit.
|
||
|
|
1b20b06f5e |
feat(cli): --json field-select, --jq, auth token, doc --from-url
Output ergonomics:
- `--json` accepts a comma-separated field list (gh-parity); selects
named keys from the per-command payload. Bare `--json` keeps the
full shape.
- `--jq <expr>` evaluates a gojq expression over the JSON; pairs with
`--json field-list` so projection runs before jq.
- `--version` is a global cobra flag in addition to the `version`
subcommand; both render the same line.
- Per-command `--help` now renders the available JSON field list under
"JSON fields available via `--json id,name,...`" (field-discovery
parity with gh / kubectl `-o jsonpath`).
New commands:
- `auth token` — print the active context's credential to stdout for
shell command substitution (`WEKNORA_TOKEN=$(weknora auth token)`).
Default: raw secret, no trailing newline. `--json` emits
`{token, mode, context}`.
- `doc upload --from-url <URL>` — ingest a remote URL via the SDK
`CreateKnowledgeFromURL`. `--name` forwarded as `FileName` so the
server's known-extension heuristic upgrades crawl-mode to
file-download-mode where appropriate.
Includes the simplify post-review polish pass (field-filter unit
tests, --json/--jq compose check, agent_help copy fixes).
|
||
|
|
4a5449233d |
fix(cli): plug v0.3 final review findings (json + auth + path + bounds + kb)
Seven bugs surfaced via two audit rounds — parallel reviewer agents
plus a real-server end-to-end demo. Each fix arrives with a
regression test.
1. doc upload --recursive --json corrupted the envelope stream.
Per-file FAIL/OK plain lines printed unconditionally to stdout,
then a Success envelope, then on partial failure a typed error
that the root handler turned into a SECOND Failure envelope —
three outputs where one was expected. Fix: gate the plain lines
behind !opts.JSONOut, and add cmdutil.Error.Silent so the JSON-
path partial-failure preserves its typed exit code without
triggering PrintErrorEnvelope's default Failure-envelope write.
2. auth refresh / AuthRetryTransport misclassified HTTP failures as
network.error. RefreshAndPersist wrapped every refresher error
with CodeNetworkError, but the SDK emits "HTTP error 401: ..."
for a rejected refresh token — which should surface as
auth.token_expired. Switched to WrapHTTP for proper status-
derived classification. Affects both `auth refresh` and the
transport's refresh closure.
3. doc download accepted ".." as a server-suggested filename. The
rejection list covered "" / "." / filepath.Separator but not
bare ".." — filepath.Base("..") is "..", which slipped through
to os.Create and produced a confusing local.file_io wrap. Added
to the rejection set.
4. search chunks / docs / kb / sessions had no lower bound on
--limit. `-L 0` / `-L -1` was forwarded to the server with
undefined behavior. Added a 1..1000 bound at the RunE boundary
across all four (matching doc list / session list page-size
bounds). Internal callers in tests can still pass Limit==0 for
the "no client-side cap" runChunks path — the bound only applies
at the user-input layer.
5. cli/AGENTS.md ADR-3 verb-canon summary listed only v0.2 verbs as
"gh-canonical" and missed v0.3 additions (edit, pin, unpin,
download — all gh-canonical) plus locally-introduced ones
(empty, refresh, add, remove, link). Rewritten as an explicit
gh-canonical / locally-introduced split.
6. kb pin returned 404. Server registers /knowledge-bases/{id}/pin
as PUT (router.go:292); SDK was using POST. gin's router silently
404s on method-mismatch (treats it as path-not-found, not 405),
so the CLI classified the response as resource.not_found and
masked the real failure mode. Switched the SDK to http.MethodPut.
The asymmetry that hid this past round 1: kb unpin on a freshly-
created KB hits the no-op branch in cmd/kb/pin.go that skips the
SDK call entirely, so unpin "worked" without ever exercising the
broken path. Only the real-server demo, where kb pin actually
fires, surfaced it.
7. kb edit clobbered current Name when only --description was
passed. EditOptions used *string to distinguish "unset" from
"set to empty", but sdk.UpdateKnowledgeBaseRequest declares both
fields as plain string (no omitempty), so the JSON body always
carried `"name": ""`. Server requires Name → 400. Fix: runEdit
does fetch-then-update — GetKnowledgeBase first, build the PUT
body with current values, then overlay user-set fields. Same
TOCTOU window as kb pin / unpin.
Audit-flagged items intentionally NOT changed:
- kb pin / unpin check-then-toggle TOCTOU: documented; the clean
fix would be a server-side setter and belongs in a separate API
change.
- AuthRetryTransport singleflight test gap for one concurrency
scenario; v0.4 polish.
- cli/README.md:50 "once v0.2 ships" and CHANGELOG.md:8
"10 top-level commands": v0.2-PR artifacts, not v0.3-introduced.
- kb edit / kb pin are v0.3-new commands, so neither bug needs a
cli/CHANGELOG.md Fixed entry — the v0.3 release ships them
working as the Added bullets advertise.
|
||
|
|
78f3994112 |
feat(cli): doc download + upload --recursive
Roadmap items 3-9 (download) and 3-10 (recursive upload).
SDK addition (additive, non-breaking):
- OpenKnowledgeFile(ctx, id) (filename, body io.ReadCloser, err) —
the new primitive that returns the body as a stream plus the
server-suggested Content-Disposition filename. The existing path-
form DownloadKnowledgeFile is now a thin wrapper (also gained
partial-file-on-error cleanup, a pre-existing bug exposed by the
reshape).
doc download <id>:
Borrows shape from `gh release download` (positional id, output flag,
`-` sentinel for stdout). Flag names match gh canon verified against
the gh manual: `-O, --output <file>` for destination; `--clobber` for
overwrite control.
- Default: writes to cwd under the server-suggested filename. If the
server didn't send one, errors with input.missing_flag.
- --output FILE / -O FILE: writes to FILE. Refuses overwrite without
--clobber.
- --output -: stream to stdout (binary-safe).
- Partial writes on error are cleaned up.
doc upload --recursive <dir> --glob '*.pdf':
NOTE on upstream parity: `gh release upload` does NOT support
--recursive (verified — it takes individual file args only). `aws s3
cp --recursive` does, but uses `--include`/`--exclude` glob pattern
pairs rather than a single `--glob`. weknora's single positive `--glob`
is a deliberate simplification, not a direct mirror of either tool.
- Walks the tree, filters by base-name glob, uploads each match
sequentially. Per-file line output: OK / FAIL with the underlying
error. Exit 0 only on full success; on partial failure returns the
first failure's typed code so callers can branch. Rejects --name
with --recursive.
- --dry-run lists matches without uploading.
- --json emits {kb_id, uploaded[], failed[]} envelope at completion.
Bugs caught in the post-commit reviewer round:
- SECURITY: server-supplied filename was used in os.Rename without
sanitization. A malicious / buggy server returning
"../../etc/shadow" could escape cwd. Now filepath.Base'd; "." / "/"
/ "" rejected. Regression test added.
- Wasted-bytes path eliminated via the SDK reshape: the CLI now
inspects filename and applies refuseIfExists BEFORE streaming.
Two-phase temp+rename gone.
- refuseIfExists(path, clobber) helper extracted.
- --json honored in --recursive (uploadOutcome was JSON-tagged but
the envelope was never emitted).
7 + 7 unit tests for download (+ path-traversal regression) and
recursive upload (+ JSON envelope regression).
Roadmap: 3-9, 3-10.
|