Commit Graph

8 Commits

Author SHA1 Message Date
nullkey 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).
2026-05-15 12:03:56 +08:00
nullkey 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
2026-05-15 12:03:56 +08:00
nullkey e623e8208f refactor(cli): delete envelope infrastructure, errors to stderr
Removes the entire envelope machinery now that every success path
emits bare JSON:

- cli/internal/format/envelope.go (Envelope, Success, Failure,
  SuccessWithRisk, WriteEnvelope, Meta, Notice, UpdateNotice,
  VersionSkewNotice, Risk, RiskLevel, ErrorBody) + tests.
- cli/internal/format/filter.go envelope-specific helpers
  (WriteEnvelopeFiltered, marshalEnvelope, applyFieldFilter,
  filterDataPayload, filterObjectData); the reusable
  filterArrayItems / filterObjectKeys / writeJQ stay for bare.go.
- cli/internal/cmdutil/exporter.go + tests (envelope-only).
- cli/internal/cmdutil/PrintErrorEnvelope + ToErrorBody +
  operationRiskOf + Error.OperationRisk field + OperationRisk struct.

Error path: all errors now go to stderr via cmdutil.PrintError in
`code: message\nhint: ...` form, regardless of --json. Stdout stays
empty (or holds the partial-success the command already wrote) so
downstream `--json | jq` pipelines never have to filter error shapes
out of the success stream. Typed exit codes (3 auth.* / 4
resource.not_found / 5 input.* / 6 server.rate_limited / 7 server.*
+ network.* / 10 input.confirmation_required) carry the failure
class for agents that branch on it.

Acceptance contract:
- envelope_test.go → wire_test.go (TestEnvelopeGolden → TestWireGolden).
- testdata/envelopes/ → testdata/wire/.
- Error-path cases assert the typed code substring on stderr.
- Orphan whoami.*.json goldens deleted.

AGENTS.md + README.md rewritten for the bare-data contract:
- Drop envelope schema section + dry-run rule.
- Document bare JSON on stdout + `code: msg\nhint: …` on stderr.
- ADR-3 reframed around bare data and why error separation matters
  for `--json | jq` pipelines.

WriteJSONFiltered short-circuits to WriteJSON when both filters are
empty (skip the marshal-buffer round-trip for the common case).

Final review pass:
- Fix wire-contract bug: `--json id,name` (space form) is broken by
  pflag's NoOptDefVal; AGENTS.md / README.md / SetAgentHelp + the
  field-discovery help text all switched to `--json=id,name`.
- Fix `weknora api --jq` silently ignored: api.go now routes through
  WriteJSONFiltered with jopts.JQ.
- AGENTS.md: drop the false claim that `auth logout` honors `-y`
  (logout is local-only with no ConfirmDestructive guard); list the
  actual destructive commands instead.
- Rewrite cli/acceptance/e2e/e2e_test.go for the bare-data wire shape
  (was still parsing `out["data"]` / `env["ok"]`).
- Add `JSONOptions.Emit(w, v)` helper; collapse ~33 repeated
  `format.WriteJSONFiltered(iostreams.IO.Out, X, jopts.Fields,
  jopts.JQ)` sites to `jopts.Emit(iostreams.IO.Out, X)` — drops the
  format import from 22 cmd/* files.
- Delete single-caller `cmdutil.MustRequireFlag`; inline as
  `_ = cmd.MarkFlagRequired(...)` everywhere.
- Add `_ = cmd.MarkFlagRequired("name")` to `kb create`; it was the
  only write command relying on runtime --name validation while
  `context add` already used the cobra-level mark.
- `context use`: register `--json` / `--jq` (was always emitting JSON
  unconditionally with no human path and no flag — diverged from
  every other write command); human mode now prints
  `✓ Switched context to X (was Y)`.
- Replace per-package `confirmPrompter` / `scriptedConfirm` /
  `errPrompter` test doubles with `testutil.ConfirmPrompter`.
- Rename `chatService` → `ChatService` (export to match siblings
  `ListService` / `ViewService`); rename `printUploadSuccess` →
  `renderUploadSuccess` (siblings use `render*`).
- `defaultHint(CodeResourceNotFound)`: drop the hardcoded
  "list available with `weknora kb list`" — misleading on agent /
  doc / session 404. Replaced with "verify the resource ID and try
  again".
- Strip stale `v0.2/v0.3` / "envelope" / "v0.0/v0.1 supports only"
  historical tags from production comments and a few test
  descriptions.
2026-05-15 12:03:56 +08:00
nullkey 5adcedf170 refactor(cli): v0.3 cross-cutting cleanup
Cross-cutting findings surfaced by the branch-completion review.

Perf bug:
- Factory.Client closure was not memoized. Factory.ResolveKB internally
  calls f.Client() to resolve --kb name → id, then the command's RunE
  calls f.Client() again. Two SDK clients, two keyring reads, two
  AuthRetryTransport allocations per name-resolved invocation, with
  *independent* token state (a refresh in one was invisible to the
  other). Switched to sync.Once like Secrets already does.

Silent bug bait:
- cmdutil.NormalizeHost docstring claimed CodeInputMissingFlag for the
  empty case; code returned CodeInputInvalidArgument. Aligned doc to
  code (present-but-empty is a bad value, not a missing flag).

Agent contract gaps:
- Five user-facing subcommands lacked SetAgentHelp: auth login /
  logout / list / status and chat. Added concise strings with error-
  code call-outs so agents can branch without parsing human strings.

Helper extraction (≥3 callers):
- text.KnowledgeDisplayName(fileName, title, id) — byte-identical
  formatter that was in both cmd/doc/list.go and cmd/search/docs.go.
  Takes fields directly so internal/text stays SDK-free.
- cmdutil.WrapHTTP(cause, fmt, args...) *Error — replaces the
  `Wrapf(ClassifyHTTPError(err), err, ...)` pattern across 24 SDK
  call sites. Sed-driven migration; off-pattern shapes in chat.go
  (used streamErr) and cmdutil/kb.go (in-package) hand-edited.
  Contract test gains a comment update: post-migration the dominant
  pattern is WrapHTTP which the AST scanner skips entirely (only
  NewError/Wrapf selectors inspected); ClassifyHTTPErrorOutputs()
  bridge still covers the dynamic codes those paths can yield.

UX consistency:
- cmd/doc/list.go --page-size help now reads "Items per page
  (1..1000)" matching cmd/session/list.go. The bounds validation
  already enforced 1..1000; the help text was the last drift.

Comment-discipline sweep:
- Deleted the WHAT-only "*Options captures `weknora ...` flag state"
  docstring across 23 files (context, kb, auth, doc, session, search,
  chat, doctor, link). Where the line carried a real WHY clause
  (kb/delete, doc/delete, session/delete, kb/edit), kept the WHY and
  dropped only the leading WHAT phrase.
- Stripped third-party project-name attribution from inline comments
  and one user-visible flag-help string across ~40 files in cli/cmd
  and cli/internal (plus 4 test-file comments). Removed phrases like
  "Mirrors `gh X`", "borrowed from lark-cli", "kubectl-style",
  "gcloud `--project`", "Stripe pattern", and the embedded GitHub
  URLs pointing at those projects. Behavioral descriptions and the
  WHY behind each comment are preserved; only the upstream-name
  attribution is gone. Inspiration / north-star references belong in
  cli/AGENTS.md (the design doc) and commit messages, not scattered
  through every file.

  Triggered by an audit round that surfaced several false / fragile
  parity claims (e.g. "Mirrors `gh repo edit`" — gh repo edit has no
  --name flag; "matches gcloud `--project` id-or-name" — gcloud's
  --project accepts ID only). Rather than fix them one by one, the
  whole category of in-comment external-project references was
  stripped uniformly.
2026-05-14 10:57:17 +08:00
nullkey da9faa9e07 feat(cli): add agent-first affordance — envelope, exit-10, --dry-run
Borrows the lark-cli agent-affordance model
(https://github.com/larksuite/cli/blob/main/AGENTS.md +
skills/lark-shared/SKILL.md) so weknora is designed to be agent-friendly:
error messages, output format, and flag design follow conventions agents
can rely on.

cli/AGENTS.md (operational reference for LLM agents invoking weknora):
  Public document covering envelope schema, exit-code protocol
  (0/1/2/10/130), stdout/stderr separation, and behavioral rules.
  Sensitive commands (\`context use\`, \`kb delete\`, \`doc delete\`, \`init\`)
  gain "AI agents:" paragraphs in their cobra Long descriptions so
  guidance shows in --help.

format.Envelope schema additions:
  Risk    per-operation classification (read / write / high-risk-write +
          action description), populated by write commands on both success
          and failure paths.
  Notice  system advisories (CLI update available, server-CLI version
          skew); type defined, emit sites land in v0.3.
  DryRun  marker for envelopes returned from --dry-run preview paths.

  RiskLevel constants realigned to lark's taxonomy: read / write /
  high-risk-write (was: read / mutating / destructive — not yet wired by
  any command).

  cmdutil.Error gains OperationRisk; PrintErrorEnvelope auto-attaches it
  to envelope.Risk so destructive failure paths surface uniformly.

Exit-10 confirmation protocol:
  New ErrorCode \`input.confirmation_required\` mapped to exit code 10 in
  cmdutil.ExitCode. ConfirmDestructive now returns this code (with
  OperationRisk attached) when stdout is non-TTY or --json was set, with
  -y/--yes absent. Previous behavior — silent proceed in non-TTY — was
  unsafe: scripts and agents could delete resources with no explicit
  approval. Three test cases re-pinned around the new contract.

  This is a wire-contract change for any caller who relied on silent
  proceed; v0.0/v0.1 had no destructive commands, so the blast radius is
  contained to v0.2 itself.

--dry-run global flag:
  cmd write paths (kb create/delete, doc upload/delete, api POST/PUT/PATCH/
  DELETE) check cmdutil.IsDryRun(cmd) and skip the SDK call, emitting an
  envelope with dry_run=true plus a Risk classification. Read commands
  ignore --dry-run by design (no side effect to preview). Human-mode
  prints \`[dry-run] would <action>\` to stdout.

Command discovery: agents introspect via the existing \`--help\` surface
(consistent with gh / kubectl / aws / gcloud / terraform — none of them
ship a CLI-tree self-description command). An earlier draft added a
\`weknora schema\` reflection command; dropped after a mainstream survey
found it has no stable analog (lark-cli's schema describes Lark API
methods, not its own CLI tree).

Tests: 27 cli packages pass at this commit. Added two new tests covering
envelope.risk and envelope._notice serialization.
2026-05-12 13:20:42 +08:00
nullkey 9d2e740753 refactor(cli): align command surface with gh CLI conventions (ADR-3)
Audited the v0.0~v0.2 21-command surface against gh / kubectl / cargo /
npm / git / docker / flyctl / vercel / supabase / brew. WeKnora was
cherry-picking from multiple heritages, producing an inconsistent feel:
the kb subtree mixed gh verbs (create / delete / list) with a kubectl
verb (get); confirmation flag duplicated --force (docker/kubectl) with
global -y/--yes (gh/vercel/npm); the --agent flag stretched Stripe's
telemetry-tag pattern into a behavior-mode switch that no mainstream
CLI does.

ADR-3 picks gh as the primary north star. Documented deviations remain
for project-link (vercel/cargo), chat (openai-cli), context (kubectl-
light), and doctor (brew/flutter). The decision and its deviations are
documented self-contained in cli/AGENTS.md.

Surface changes:

  - kb get → kb view (gh repo view convention); "get" kept as cobra Alias
    for v0.0/v0.1 callers — see https://cli.github.com/manual/gh_repo_view.

  - kb delete --force / doc delete --force removed in favor of the global
    -y/--yes persistent flag (gh repo delete --yes convention). One
    mechanism skips destructive prompts; ConfirmDestructive's parameter
    renamed `force` → `yes` to match.

  - --agent omnibus mode-switch removed. Stripe's DetectAIAgent (the
    cited inspiration) only tags User-Agent for telemetry, never flips
    behavior; gh / kubectl / aws / docker / flyctl all decline this kind
    of flag. The 7-env auto-detect list is reduced to the two entries
    Stripe also recognizes (CLAUDECODE, CURSOR_AGENT) — the other five
    had no agent-documented source. ApplyAgentSugar / ShouldUseAgentMode
    and the dead --no-interactive / --no-progress globals are deleted
    entirely.

  - DetectAIAgent and SetAgentHelp annotations are kept: env detection
    now only triggers AGENT-targeted help text rendering (no behavior
    change), matching Stripe's narrower scope.

Tests: 27 cli packages green (acceptance/contract still pins kb get
golden; the alias keeps it valid).
2026-05-12 13:20:42 +08:00
nullkey 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.
2026-05-12 13:20:42 +08:00
nullkey 8a0674186e feat(cli): add kb create/delete and doc list/upload/delete commands
Add the resource-management surface to the v0.2 CLI:

  - weknora kb create --name X [--description Y] [--embedding-model Z]
  - weknora kb delete <id> [--force]
  - weknora doc list [--kb-id X | --kb NAME] [--page N] [--page-size M]
  - weknora doc upload <file> [--kb-id X | --kb NAME] [--name custom]
  - weknora doc delete <id> [--force]

doc/* uses Factory.ResolveKB so the cwd's project link is honored when
--kb-id is omitted. doc upload validates path existence with os.Stat
(rejects directories; follows symlinks to mirror SDK os.Open behavior).
doc list sorts by updated_at desc so newer items surface first.

Both delete commands route through cmdutil.ConfirmDestructive — the
"destructive op needs explicit user opt-in" pattern was about to be
copy-pasted across the new subtree, so it was extracted with the delete
commands as their first consumer. Saves the same dedup pass when v0.3
adds session/agent delete. (PR-12 later renames the flag from --force
to the global -y/--yes for gh-style consistency.)

iostreams.SetForTestWithTTY pairs with the existing SetForTest helper:
the latter never reports stdout as a TTY (singleton replacement uses an
in-memory buffer), so the confirm-yes / confirm-no test branches need a
TTY-on variant.
2026-05-12 13:20:42 +08:00