Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,34 @@ Commands that charge money or request user consent should follow the examples of
- Return `Ok(CommandResult::new(json!({...})))` for success.
- Prefer `crate::error::GddyError::{not_found,validation,auth,config,security,network,鈥` (and `GddyError::from` for module client errors) so agents get stable `error.code` + top-level `fix`. Use `Err(cli_engine::CliCoreError::message("..."))` only for one-off cases that do not yet have a shared mapping.
- Streaming commands use `RuntimeCommandSpec::new_streaming` and emit events via `StreamSender`.
- Commands with external effects are marked mutating so the engine's dry-run safeguard applies. Dry-run paths validate and read every prerequisite the real call needs (so they fail where it would) and return `CommandResult::with_dry_run()`.
- Next actions (suggested follow-up commands) use a command template plus structured params, not a `format!`-built command line like `--query '{query}'` (a quote in the value breaks it; metacharacters can inject commands). Param names must match the target command's args and the template's `<placeholder>`s. Emit next actions only when executable and appropriate to the returned state, and never include consent-bypass flags (e.g. `--agree`) in them.
- Encode dynamic path segments with `api::http::encode_path_segment` when assembling URLs by hand. Generated (Progenitor) clients already percent-encode path parameters; pass them raw to avoid double encoding.
- Do not call `--debug transport` logging helpers for payloads that may hold customer, payment or order data.
- Constrain flags to the API's documented values at argument parsing, so bad input fails locally with clear help.
- Correctable input or config failures use a stable validation error with an actionable `fix`; never turn malformed config into an empty payload.
- If an API returns an error payload inside a 2xx response, treat it as an error: don't let a typed client turn it into an empty success, and don't cache it. Keep an empty 202/204 response distinct (null/none) rather than substituting a default object that looks like a real, empty resource.
- Polling/retry wrappers map only the exhausted expected status (e.g. 404) to `not_found`; keep 429/5xx/network errors as-is.
- Resolve the API base URL from the selected environment; do not add per-service URL overrides or `--env` flags on follow-up commands.

## Reuse Before You Build (Required)

Search the codebase and `cli-engine` before writing a helper; reviewers reject duplication.

- Typed clients: generate with Progenitor from the OpenAPI spec (as existing generated clients in the workspace do). Do not hand-write `reqwest` clients that traverse `serde_json::Value`.
- Rendering: prefer `cli-engine` rendering (`HumanViewDef`/`TableColumn`, structured next actions and its standard footer) over hand-formatted tables or local display logic. Only write custom rendering when `cli-engine` cannot express it.
- Shared formatting (money, etc.): reuse existing helpers rather than adding per-module copies.

## User-Facing Text (Required)

- Write help, guides and output for customers: no internal system or API names, scopes, environments or implementation jargon.
- Where an AI assistant must act differently from a human (e.g. consent before a charge), address it directly in a clearly marked `AI assistants:` note; never mix that into customer-facing prose.
- Command descriptions are short imperatives from the user's point of view; give flags concrete examples and discoverable values; show where prerequisite values (IDs) come from.
- Don't expose internals (retry mechanics, generated keys, etc.) in normal help or output; when something fails, put the suggested next step in the error `fix`.
- Avoid raw JSON inputs in the main flow. Guides should use soft line breaks (hard breaks only in shell examples).
- PR descriptions must match implemented behavior.

Full checklist: [Command authoring](./docs/command-authoring.md).

## Code File Structure (Required)

Expand Down
57 changes: 57 additions & 0 deletions docs/command-authoring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Command authoring checklist

Read this before adding a command or command group. The short, enforceable version lives in [`AGENTS.md`](../AGENTS.md); this doc adds the reasoning.

## 1. Reuse before you build

Search the codebase and `cli-engine` before writing a helper. Duplication is the most common review finding.

| Need | Use | Not |
| --- | --- | --- |
| Typed API client | Progenitor-generated client from the OpenAPI spec (as existing generated clients in the workspace do) | A hand-written `reqwest` client traversing `serde_json::Value` |
| Human output (tables, lists, footers) | `cli-engine` rendering: `HumanViewDef` / `TableColumn` | Hand-formatted strings |
| Follow-up suggestions | `cli-engine` structured next actions and its standard footer | Project-local display or placeholder-substitution code |
| Shared formatting (currency, etc.) | The existing shared helper | A per-module copy |
| URL path segments (hand-built URLs) | `api::http::encode_path_segment` | String interpolation into paths |

Prefer `cli-engine` rendering whenever it can express the output. Custom rendering should be the exception, with a reason stated in the PR. If duplication is truly unavoidable, say why.

## 2. Correctness and security

- **Encode dynamic path segments.** IDs containing `/`, `?`, `#` or `%` must not change URL structure. Use `encode_path_segment` for hand-built URLs and test those characters. Generated (Progenitor) clients already encode path params, so pass raw values to them; pre-encoding double-encodes `%`.
- **Build next actions from a command template plus structured params, never a formatted command line.** A quote in a value breaks `format!("... --query '{query}'")`, and shell metacharacters can inject commands. Param names must match the target command's args and the template's `<placeholder>`s.
- **Keep next actions honest.** Emit them only when executable and appropriate to the returned state. Never include consent-bypass flags (e.g. `--agree`), and re-present approval guidance after material changes so stale approval isn't reused.
- **Make dry runs faithful.** Mark commands with external effects as mutating so the engine's dry-run safeguard applies. Dry-run validates and reads every prerequisite the real call needs, so it fails where the real call would, and returns `CommandResult::with_dry_run()` so consumers can tell a preview from an executed mutation.
- **Keep output consistent across modes.** Preview and real runs return the same fields in camelCase, and the preview says what the real run would do (e.g. items that would fail are reported separately). Output schemas and default-field projections must include every field users should see, or default rendering drops it.
- **Test what users see.** Assert on rendered output with default fields, not just the helper that builds it. A test comment must state what the test actually exercises; rendering a hand-written JSON literal doesn't prove the handler produces it.
- **Validate early, fail actionably.** Restrict flags to the API's documented values at argument parsing. Every user-correctable input or config failure gets a stable validation error with a `fix`; never turn malformed config into an empty payload.
- **Don't turn failures or "no content" into fake successes.** APIs may report failure inside a 2xx response (an error object, or error-severity `messages`), which generated clients can deserialize as a valid empty result. Return an error and never cache it, or a transient failure is served as truth until the cache expires. A 202/204 with no body stays null/none, not `Default::default()`, which prints as a real, empty resource.
- **Follow HTTP and API conventions.** Header names are case-insensitive (`idempotency-key` matches a spec header `Idempotency-Key`). Pagination links may be relative, so read the token from the query string rather than requiring an absolute URL. Strip a user-supplied query string (`/v1/items?limit=10`) when matching the catalog, but keep it on the request.
- **Don't log sensitive payloads.** Avoid the `--debug transport` helpers for requests or responses that may contain customer, payment or order data.
- **Map only the error you mean to map.** When polling for eventual consistency, only the exhausted expected status (e.g. 404) becomes `not_found`; network errors, 429s and 5xxs keep their real mapping.
- **Use the selected environment.** No private per-service URL overrides or `--env` flags on follow-up commands.

## 3. Write for the customer

Users don't know our system names, API names or environments.

- Help text, guides and output must not leak internal terms (service names, scopes, environment plumbing, implementation jargon).
- Command descriptions are short imperatives from the user's point of view.
- Text may address AI assistants directly when they must behave differently from a human (e.g. obtaining explicit user consent before a charge). Put it in a clearly marked `AI assistants:` note, and keep it separate from the customer-facing prose.
- Prefer common terms users already know. If an API resource name differs, define it once in the guide.
- Every flag gets concrete examples and discoverable values. Don't ask for things the system can infer, and don't assume users know standards by name.
- If a command needs a value produced by an earlier command (an ID, a selection), that command's output and the guide must show where to get it.
- Don't over-communicate internals (retry mechanics, generated keys, etc.) in normal help or output. When something fails, put the suggested next step in the error's `fix` text.
- Avoid raw JSON inputs (`--body`, `--file`) in the main flow. Prefer simple flags. If JSON is unavoidable, point agents at where to get the schema instead of embedding samples.

## 4. Guides (`guides/*.md`)

- Structure: introduction (what you'll learn), concepts (what the CLI exposes, including entity relationships, opaque identifiers and consistent terminology), then task-oriented command sequences with any non-obvious step explained.
- Update discovery metadata and any related proposal docs when command availability changes, so agent matching and documentation don't contradict shipped behavior.
- Use soft line breaks in prose. The renderer wraps to the terminal width, and hard breaks look wrong in narrow terminals. Hard breaks are fine inside shell examples.
- Describe what customers can do, not which APIs are being called.

## 5. Pull requests

- Make sure the PR description matches the implemented behavior.
- Reply on each review thread with the fixing commit. Leave human reviewers' threads open for them to resolve.
1 change: 1 addition & 0 deletions docs/maintainers.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ If you are a new developer on this project, read the following to get started:

- [`cli-engine` concepts](https://github.com/godaddy/cli-engine/blob/main/cli-engine/docs/concepts.md) goes over the components making up GoDaddy CLIs.
- [This repo's docs](../docs/)
- [Command authoring checklist](./command-authoring.md) covers conventions for adding commands and command groups.
- [`CONTRIBUTING.md`](../CONTRIBUTING.md) gives some general tips for setting up your workspace for development.

If you are doing agentic coding and see your agent struggling with following standards correctly, contributions to [`AGENTS.md`](../AGENTS.md) are greatly appreciated.
Expand Down
Loading