Commit Graph

4 Commits

Author SHA1 Message Date
nullkey 6d8c8650cd feat(cli): --dry-run + risk metadata + validation parity on 19 mutations
Two intertwined agent safety nets that share the same files:

1. --dry-run flag for offline preview of mutation commands
2. Risk: metadata + SetRisk helper for destructive command surfaces

Coverage (19 mutation commands with --dry-run):
  kb.create/edit/delete           agent.create/edit/delete
  doc.create/upload/fetch/delete  doc.delete_all (special variant)
  session.delete  chunk.delete    profile.add/remove
  auth.refresh/logout             link  unlink
  api.{post,put,patch,delete}     (api.get + --dry-run rejected, exit 2)

Envelope additions (omitempty in non-dry-run paths):
  meta.dry_run: bool        true when --dry-run was used
  meta.plan:    map         {action, args} per the per-command taxonomy

Risk: metadata
--------------
cmdutil.SetRisk(cmd, action) stamps cobra.Annotations with
risk.level=destructive + risk.action=<action> on the 9 destructive
commands. The SetAgentHelp wrapper prepends a "Risk: <action>
(destructive)" line in the default text help path so agents see a clear
warning before parsing Usage. WEKNORA_AGENT_HELP=1 JSON path stays
unchanged — structured agent-help already carries warnings[].

Validation parity with the live path
------------------------------------
Every pure-local validation (flag presence, mutual exclusion, enum
bounds, URL/regex format, ResolveKBLocal for KB resolution that does not
require an SDK call) runs BEFORE the dry-run gate. This matches the
industry-standard "preview shows what live would do" contract:
--dry-run accepts exactly the same invocations the live path accepts and
rejects exactly the same invocations the live path rejects, modulo the
side-effecting work itself.

The side-effecting work (SDK calls, file writes, keyring writes, server-
side name → id resolution) is what --dry-run actually gates. Each
mutation file pairs its RunE validation block with a regression test
under *_dry_run_test.go / dryrun_validation_test.go so future refactors
don't reintroduce the gap.

Helper surface
--------------
- HandleDryRun(cmd, dryRun, plan) extracts the early-return so the
  19 RunE call sites stay 3 lines each.
- EmitDryRun routes through FormatOptions.Emit, inheriting _notice /
  TTY indent / --jq filtering for free.
- ResolveKBLocal mirrors ResolveKB but never calls the SDK; dry-run
  paths use it so the plan reports the raw --kb value (UUID or name)
  without a name → id lookup.

Streaming commands (chat, session ask, session continue-stream) are
deliberately excluded: a buffered plan makes no sense for an event
stream.

Lock semantics in the dry-run path:
  - destructive + --dry-run: exit 0, no exit-10 confirmation prompt
  - --dry-run + -y: byte-identical envelope to --dry-run alone
  - --dry-run + --jq: filter applies to the preview envelope normally
2026-05-28 19:29:50 +08:00
nullkey 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.
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