From 3c63145fa4b1bfb4833acecc8785d7441834e353 Mon Sep 17 00:00:00 2001 From: Harsh-2002 Date: Sun, 14 Jun 2026 09:19:46 +0000 Subject: [PATCH] feat(cli): UX & consistency polish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier-2/3 polish across the CLI surface: - Auth-aware errors: 401/403 now say "not authenticated — run `orva login`…" instead of a bare "API error (401)" (the #1 first-run stumble). - Destructive-op guards: `dns set` (replaces the whole DNS config) and `system vacuum` (write-blocking lock) now confirm() — refuse on a non-TTY without --yes, like every other destructive command. - Aliases (additive): fn/fns, secret, route, key for the noun groups, and ls / rm,del on every list / delete leaf. - `deploy --watch` → `--follow`/`-f` to match logs/activity/deployments-logs; --watch kept as a hidden, deprecated alias. - JSON output holes: `webhooks inbound test` honors -o json (status/headers/ body envelope); fixtures/cron/webhooks/channels `delete` emit {"deleted":true,...} in JSON mode. - stdout discipline: `firewall resolve` empty-result notes and `fixtures get` body label now go to stderr, keeping stdout pipe-clean. - Help: every `webhooks` and `channels` leaf gains a Long + Example (were Short-only). - Docs: cli/CLAUDE.md refreshed — clientFactories → RegisterClient, and the layout lists backup.go / executions.go and the kv incr/cas + deploy --follow surfaces. Deferred as lower-value follow-ups: list-timestamp formatting unification and positional-placeholder standardization. --- cli/CLAUDE.md | 22 ++++++---- cli/commands/channels.go | 69 ++++++++++++++++++++++---------- cli/commands/commands_test.go | 1 + cli/commands/cron.go | 3 ++ cli/commands/deploy.go | 13 ++++-- cli/commands/dns.go | 7 ++++ cli/commands/firewall.go | 4 +- cli/commands/fixtures.go | 5 ++- cli/commands/helpers.go | 9 +++++ cli/commands/root.go | 50 +++++++++++++++++++++++ cli/commands/system.go | 7 ++++ cli/commands/webhooks.go | 75 ++++++++++++++++++++++++++++------- 12 files changed, 217 insertions(+), 48 deletions(-) diff --git a/cli/CLAUDE.md b/cli/CLAUDE.md index 4fabbb4..841ad6a 100644 --- a/cli/CLAUDE.md +++ b/cli/CLAUDE.md @@ -15,23 +15,25 @@ cli/ ├── helpers.go # getClient(cmd), checkResponse, etc. ├── output.go # shared output framework (stdout/stderr split, table|json, color, confirm) ├── activity.go # `orva activity` + ├── backup.go # `orva backup download/restore` ├── channels.go # `orva channels …` ├── chat.go # `orva chat` (interactive AI REPL + one-shot -p, SSE) ├── completion.go # `orva completion {bash|zsh|fish|powershell}` ├── completions.go # dynamic shell completions (fn names, runtimes, models) ├── cron.go # `orva cron …` - ├── deploy.go # `orva deploy [--watch]` + ├── deploy.go # `orva deploy [--follow]` (--watch = deprecated alias) ├── deployments.go # `orva deployments list/get/logs` ├── diff.go # `orva diff ` (unified diff between deployments) ├── dns.go # `orva dns get/set` ├── docs.go # `orva docs` (renders embedded docs/reference.md) + ├── executions.go # `orva executions list/get/logs/delete/prune/replay` ├── firewall.go # `orva firewall list/add/enable/disable/delete/resolve` ├── fixtures.go # `orva fixtures list/get/save/delete/test` ├── functions.go # `orva functions …` ├── invoke.go # `orva invoke ` (--body/--stream/--route/-H/-X) ├── jobs.go # `orva jobs …` ├── keys.go # `orva keys …` - ├── kv.go # `orva kv …` + ├── kv.go # `orva kv list/get/put/delete/incr/cas` ├── login.go # `orva login --endpoint --api-key [--test]` ├── logs.go # `orva logs [--follow]` (SSE) ├── pool.go # `orva pool get/set` (per-fn warm-pool autoscaler) @@ -129,9 +131,10 @@ is built by `make build` from `./backend/cmd/orva`. - **No `os.Exit` inside subcommand bodies.** Use `RunE` and return errors so tests can observe failures. The existing `Run`-style commands are pre-refactor; new commands should use `RunE`. -- **Add new subcommands to `clientFactories` in `root.go`.** Otherwise - they won't show up — and `cli/commands/commands_test.go::TestCommandTree` - will fail in CI. +- **Register new top-level commands in `RegisterClient` (`root.go`).** Add the + `*Cmd` var to the `root.AddCommand(...)` list and give it a group in + `commandGroups()`. Otherwise it won't show up — and + `cli/commands/commands_test.go::TestCommandTree` will fail in CI. ## Testing @@ -160,11 +163,14 @@ weekly schedule to catch GH-API / release-asset drift. 1. Create `cli/commands/.go` with `package commands`. 2. Define `var Cmd = &cobra.Command{…}` + an `init()` for flags. -3. Add `Cmd` to the `clientFactories` slice in `root.go`. +3. Add `Cmd` to the `root.AddCommand(...)` list in `RegisterClient` (`root.go`) + and assign it a group in `commandGroups()`. 4. Add the leaf path to `commands_test.go::TestCommandTree`'s `paths` list. 5. Add any required flags to `TestRequiredFlagsPresent`. -6. Run `go test ./cli/commands/` — should pass. -7. Run `bash test/cli/command-tree.sh` — golden diff should remain zero. +6. (If a subcommand takes a function name) wire fn-name completion in + `wireCompletions` (`completions.go`). +7. Run `go test ./cli/commands/` — should pass. +8. Run `bash test/cli/command-tree.sh` — golden diff should remain zero. ## Self-update (`orva upgrade`) diff --git a/cli/commands/channels.go b/cli/commands/channels.go index d482274..2661237 100644 --- a/cli/commands/channels.go +++ b/cli/commands/channels.go @@ -26,51 +26,77 @@ an agentic workflow without giving it Orva management. var channelsListCmd = &cobra.Command{ Use: "list", Short: "List agent channels", - RunE: runChannelsList, + Long: `List agent channels (function bundles exposed at /mcp under a static token). + + orva channels list + orva channels list -o json | jq '.[].name'`, + RunE: runChannelsList, } var channelsCreateCmd = &cobra.Command{ - Use: "create [name]", + Use: "create ", Short: "Create a new channel", - Long: "Create a channel. The token plaintext is printed once and never shown again.", - Args: cobra.ExactArgs(1), - RunE: runChannelsCreate, + Long: `Create a channel bundling one or more functions under a static bearer token. +The token plaintext is printed once and never shown again — store it. + + orva channels create prod --functions greeter,echo + orva channels create ci --functions deploy-hook --expires-in-days 90`, + Args: cobra.ExactArgs(1), + RunE: runChannelsCreate, } var channelsShowCmd = &cobra.Command{ - Use: "show [id|name]", + Use: "show ", Short: "Show a channel + its function set", - Args: cobra.ExactArgs(1), - RunE: runChannelsShow, + Long: `Show a channel's metadata and the functions it exposes. + + orva channels show prod`, + Args: cobra.ExactArgs(1), + RunE: runChannelsShow, } var channelsAddFunctionsCmd = &cobra.Command{ - Use: "add-functions [id|name] [fn1] [fn2] ...", + Use: "add-functions ...", Short: "Add functions to a channel", - Args: cobra.MinimumNArgs(2), - RunE: runChannelsAddFunctions, + Long: `Add one or more functions (by id or name) to an existing channel. + + orva channels add-functions prod echo summarize`, + Args: cobra.MinimumNArgs(2), + RunE: runChannelsAddFunctions, } var channelsRemoveFunctionsCmd = &cobra.Command{ - Use: "remove-functions [id|name] [fn1] [fn2] ...", + Use: "remove-functions ...", Short: "Remove functions from a channel", - Args: cobra.MinimumNArgs(2), - RunE: runChannelsRemoveFunctions, + Long: `Remove one or more functions (by id or name) from a channel. + + orva channels remove-functions prod echo`, + Args: cobra.MinimumNArgs(2), + RunE: runChannelsRemoveFunctions, } var channelsRotateCmd = &cobra.Command{ - Use: "rotate [id|name]", + Use: "rotate ", Short: "Rotate the channel's token (invalidates the old one)", - Args: cobra.ExactArgs(1), - RunE: runChannelsRotate, + Long: `Issue a fresh token for the channel and invalidate the old one. The new +token plaintext is printed once. + + orva channels rotate prod`, + Args: cobra.ExactArgs(1), + RunE: runChannelsRotate, } var channelsDeleteCmd = &cobra.Command{ - Use: "delete [id|name]", + Use: "delete ", Aliases: []string{"rm"}, Short: "Delete a channel", - Args: cobra.ExactArgs(1), - RunE: runChannelsDelete, + Long: `Delete a channel and invalidate its token. Prompts for confirmation unless +--yes is passed. + + orva channels delete prod + orva channels delete prod --yes`, + Args: cobra.ExactArgs(1), + RunE: runChannelsDelete, } func init() { @@ -317,6 +343,9 @@ func runChannelsDelete(cmd *cobra.Command, args []string) error { return err } resp.Body.Close() + if outputJSON(cmd) { + return emitJSON(map[string]any{"deleted": true, "id": id, "name": args[0]}) + } okf(cmd, "Channel %s deleted.", args[0]) return nil } diff --git a/cli/commands/commands_test.go b/cli/commands/commands_test.go index 352c8cd..3b44ae7 100644 --- a/cli/commands/commands_test.go +++ b/cli/commands/commands_test.go @@ -70,6 +70,7 @@ func TestRequiredFlagsPresent(t *testing.T) { {[]string{"deploy"}, "runtime"}, {[]string{"deploy"}, "entrypoint"}, {[]string{"deploy"}, "watch"}, + {[]string{"deploy"}, "follow"}, {[]string{"invoke"}, "body"}, {[]string{"invoke"}, "method"}, {[]string{"invoke"}, "header"}, diff --git a/cli/commands/cron.go b/cli/commands/cron.go index affb450..9f62299 100644 --- a/cli/commands/cron.go +++ b/cli/commands/cron.go @@ -321,6 +321,9 @@ func runCronDelete(cmd *cobra.Command, args []string) error { } resp.Body.Close() + if outputJSON(cmd) { + return emitJSON(map[string]any{"deleted": true, "id": id}) + } okf(cmd, "Deleted cron schedule %s", id) return nil } diff --git a/cli/commands/deploy.go b/cli/commands/deploy.go index 9bcedb3..2500f4b 100644 --- a/cli/commands/deploy.go +++ b/cli/commands/deploy.go @@ -30,7 +30,7 @@ auto-detects the entrypoint: Examples: orva deploy ./src --name greeter --runtime node - orva deploy ./src --name greeter --runtime node --watch # stream build logs`, + orva deploy ./src --name greeter --runtime node --follow # stream build logs`, Args: cobra.ExactArgs(1), RunE: runDeploy, } @@ -39,7 +39,11 @@ func init() { deployCmd.Flags().String("name", "", "function name (required)") deployCmd.Flags().String("runtime", "", "runtime: node or python (required)") deployCmd.Flags().String("entrypoint", "", "entrypoint file (optional; auto-detects handler.ts when tsconfig.json + handler.ts present)") - deployCmd.Flags().Bool("watch", false, "stream build logs and wait for the deploy to finish (non-zero exit on build failure)") + // --follow/-f matches logs/activity/deployments-logs; --watch is the + // original name, kept as a hidden alias for back-compat. + deployCmd.Flags().BoolP("follow", "f", false, "stream build logs and wait for the deploy to finish (non-zero exit on build failure)") + deployCmd.Flags().Bool("watch", false, "deprecated alias for --follow") + _ = deployCmd.Flags().MarkHidden("watch") deployCmd.MarkFlagRequired("name") deployCmd.MarkFlagRequired("runtime") } @@ -54,7 +58,10 @@ func runDeploy(cmd *cobra.Command, args []string) error { name, _ := cmd.Flags().GetString("name") runtime, _ := cmd.Flags().GetString("runtime") entrypoint, _ := cmd.Flags().GetString("entrypoint") - watch, _ := cmd.Flags().GetBool("watch") + watch, _ := cmd.Flags().GetBool("follow") + if w, _ := cmd.Flags().GetBool("watch"); w { + watch = true // honor the deprecated --watch alias + } // Verify the source path exists. info, err := os.Stat(srcPath) diff --git a/cli/commands/dns.go b/cli/commands/dns.go index ac9ad0b..901960e 100644 --- a/cli/commands/dns.go +++ b/cli/commands/dns.go @@ -140,6 +140,13 @@ func runDNSSet(cmd *cobra.Command, _ []string) error { records = append(records, dnsRecord{Host: name, IP: ip}) } + // This replaces the entire DNS config — omitted flags clear their section + // (e.g. passing no --server wipes all servers). Confirm so it can't silently + // nuke the config; --yes skips the prompt and is required on a non-TTY. + if err := confirm(cmd, "Replace the sandbox DNS config? (omitted flags clear their section)"); err != nil { + return err + } + body := map[string]any{ "servers": cleanServers, "search": strings.TrimSpace(search), diff --git a/cli/commands/firewall.go b/cli/commands/firewall.go index 180d52e..3c78af4 100644 --- a/cli/commands/firewall.go +++ b/cli/commands/firewall.go @@ -332,7 +332,7 @@ func runFirewallResolve(cmd *cobra.Command, args []string) error { // applied set when the host isn't a configured hostname rule. if ips, ok := result.Status.HostnameMap[hostname]; ok { if len(ips) == 0 { - fmt.Printf("(%s resolved to no addresses)\n", hostname) + infof(cmd, "%s resolved to no addresses", hostname) return nil } for _, ip := range ips { @@ -344,7 +344,7 @@ func runFirewallResolve(cmd *cobra.Command, args []string) error { infof(cmd, "%q is not a configured hostname rule; showing the full applied set:", hostname) all := append(append([]string{}, result.Status.IPv4...), result.Status.IPv6...) if len(all) == 0 { - fmt.Println("(no resolved IPs in the applied set)") + infof(cmd, "no resolved IPs in the applied set") return nil } for _, ip := range all { diff --git a/cli/commands/fixtures.go b/cli/commands/fixtures.go index d57a4be..1f19342 100644 --- a/cli/commands/fixtures.go +++ b/cli/commands/fixtures.go @@ -234,7 +234,7 @@ func runFixturesGet(cmd *cobra.Command, args []string) error { ht.flush() } if f.Body != "" { - fmt.Println("\nbody:") + infof(cmd, "\nbody:") // label → stderr; the body itself stays on stdout for piping fmt.Println(f.Body) } return nil @@ -334,6 +334,9 @@ func runFixturesDelete(cmd *cobra.Command, args []string) error { } resp.Body.Close() + if outputJSON(cmd) { + return emitJSON(map[string]any{"deleted": true, "name": name}) + } okf(cmd, "deleted fixture %q", name) return nil } diff --git a/cli/commands/helpers.go b/cli/commands/helpers.go index f2be4ed..5858f2c 100644 --- a/cli/commands/helpers.go +++ b/cli/commands/helpers.go @@ -43,6 +43,15 @@ func checkResponse(resp *http.Response) error { defer resp.Body.Close() body, _ := io.ReadAll(resp.Body) + // Auth failures are the most common first-run stumble — point at the fix + // instead of a bare "API error (401)". + switch resp.StatusCode { + case http.StatusUnauthorized: + return fmt.Errorf("not authenticated (HTTP 401) — run `orva login`, or pass --endpoint/--api-key (or set ORVA_ENDPOINT / ORVA_API_KEY)") + case http.StatusForbidden: + return fmt.Errorf("not authorized (HTTP 403) — the API key is valid but lacks the required permission for this command") + } + var errResp struct { Error struct { Code string `json:"code"` diff --git a/cli/commands/root.go b/cli/commands/root.go index bd068ef..fa07619 100644 --- a/cli/commands/root.go +++ b/cli/commands/root.go @@ -146,5 +146,55 @@ func RegisterClient(root *cobra.Command) { for cmd, group := range commandGroups() { cmd.GroupID = group } + addConvenienceAliases(root) wireCompletions(root) } + +// addConvenienceAliases adds the reflexive aliases developers reach for: +// short/singular names for the noun groups (fn, secret, route, key) and +// `ls`/`rm`/`del` on every list/delete leaf. Purely additive — primary names +// are unchanged, so scripts and the command-tree golden test are unaffected. +func addConvenienceAliases(root *cobra.Command) { + top := map[string][]string{ + "functions": {"fn", "fns"}, + "secrets": {"secret"}, + "routes": {"route"}, + "keys": {"key"}, + } + for _, c := range root.Commands() { + if a, ok := top[c.Name()]; ok { + c.Aliases = appendMissing(c.Aliases, a...) + } + } + var walk func(c *cobra.Command) + walk = func(c *cobra.Command) { + for _, sub := range c.Commands() { + switch sub.Name() { + case "list": + sub.Aliases = appendMissing(sub.Aliases, "ls") + case "delete": + sub.Aliases = appendMissing(sub.Aliases, "rm", "del") + } + walk(sub) + } + } + walk(root) +} + +// appendMissing appends each value not already present (alias de-dup so we +// never hand cobra a duplicate, which it rejects). +func appendMissing(have []string, add ...string) []string { + for _, a := range add { + found := false + for _, h := range have { + if h == a { + found = true + break + } + } + if !found { + have = append(have, a) + } + } + return have +} diff --git a/cli/commands/system.go b/cli/commands/system.go index a46fefa..8daecbd 100644 --- a/cli/commands/system.go +++ b/cli/commands/system.go @@ -213,6 +213,13 @@ func runSystemVacuum(cmd *cobra.Command, args []string) error { return err } + // VACUUM holds an exclusive lock — writes block until it finishes. Confirm + // so it isn't run by accident; --yes skips the prompt and is required on a + // non-TTY. + if err := confirm(cmd, "Run VACUUM now? Writes to the database will block until it completes."); err != nil { + return err + } + infof(cmd, "Running VACUUM (writes will block briefly)...") resp, err := client.Post("/api/v1/system/vacuum", nil) if err != nil { diff --git a/cli/commands/webhooks.go b/cli/commands/webhooks.go index f5ede5d..a77ce38 100644 --- a/cli/commands/webhooks.go +++ b/cli/commands/webhooks.go @@ -30,20 +30,34 @@ history, and manage inbound webhook triggers. var webhooksListCmd = &cobra.Command{ Use: "list", Short: "List webhook subscriptions", - RunE: runWebhooksList, + Long: `List outbound webhook subscriptions with their URL, subscribed events, and +last delivery status. + + orva webhooks list + orva webhooks list -o json | jq '.subscriptions[].url'`, + RunE: runWebhooksList, } var webhooksCreateCmd = &cobra.Command{ Use: "create", Short: "Create a webhook subscription", - RunE: runWebhooksCreate, + Long: `Create an outbound webhook subscription. The signing secret is returned ONCE +on create — store it. Events default to '*' (all). + + orva webhooks create --name ci --url https://example.com/hook --events deployment.failed + orva webhooks create --name all --url https://example.com/hook`, + RunE: runWebhooksCreate, } var webhooksTestCmd = &cobra.Command{ - Use: "test [id]", + Use: "test ", Short: "Send a synthetic test event to a webhook", - Args: cobra.ExactArgs(1), - RunE: runWebhooksTest, + Long: `Deliver a synthetic test event to a subscription so you can verify the +endpoint receives and accepts it. + + orva webhooks test `, + Args: cobra.ExactArgs(1), + RunE: runWebhooksTest, } var webhooksGetCmd = &cobra.Command{ @@ -72,24 +86,38 @@ The secret cannot be rotated here; delete and recreate to rotate it. } var webhooksDeleteCmd = &cobra.Command{ - Use: "delete [id]", + Use: "delete ", Short: "Delete a webhook subscription", - Args: cobra.ExactArgs(1), - RunE: runWebhooksDelete, + Long: `Delete an outbound webhook subscription. Prompts for confirmation unless +--yes is passed. + + orva webhooks delete + orva webhooks delete --yes`, + Args: cobra.ExactArgs(1), + RunE: runWebhooksDelete, } var webhooksDeliveriesCmd = &cobra.Command{ - Use: "deliveries [id]", + Use: "deliveries ", Short: "List recent delivery attempts for a subscription", - Args: cobra.ExactArgs(1), - RunE: runWebhooksDeliveries, + Long: `Show recent delivery attempts (status, response code, error) for a +subscription — the first stop when a webhook isn't arriving. + + orva webhooks deliveries + orva webhooks deliveries -o json | jq .`, + Args: cobra.ExactArgs(1), + RunE: runWebhooksDeliveries, } var webhooksRetryCmd = &cobra.Command{ - Use: "retry [delivery-id]", + Use: "retry ", Short: "Retry a failed delivery", - Args: cobra.ExactArgs(1), - RunE: runWebhooksRetry, + Long: `Re-attempt a single failed delivery by its delivery id (from +'orva webhooks deliveries '). + + orva webhooks retry `, + Args: cobra.ExactArgs(1), + RunE: runWebhooksRetry, } func init() { @@ -323,6 +351,9 @@ func runWebhooksDelete(cmd *cobra.Command, args []string) error { return err } resp.Body.Close() + if outputJSON(cmd) { + return emitJSON(map[string]any{"deleted": true, "id": args[0]}) + } okf(cmd, "Webhook %s deleted", args[0]) return nil } @@ -632,6 +663,22 @@ func runInboundTest(cmd *cobra.Command, args []string) error { } defer resp.Body.Close() respBody, _ := io.ReadAll(resp.Body) + if outputJSON(cmd) { + out := map[string]any{ + "status": resp.StatusCode, + "headers": flattenHeaders(resp.Header), + } + var parsed any + if jsonUnmarshal(respBody, &parsed) == nil { + out["body"] = parsed + } else { + out["body"] = string(respBody) + } + if err := emitJSON(out); err != nil { + return err + } + return exitForStatus(resp.StatusCode) + } infof(cmd, "HTTP %d", resp.StatusCode) fmt.Println(string(respBody)) return exitForStatus(resp.StatusCode)