From 3462da632c6b17f8d10dfebda699f3ef2411df7f Mon Sep 17 00:00:00 2001 From: Eric Kim Date: Mon, 27 Jul 2026 11:18:24 -0400 Subject: [PATCH] feat: sync developer-cli from amplitude/javascript@083b16f59710 Mirror monorepo developer-cli/ at 083b16f59710c3d1f9c6d27e6452b157fb102936. Includes: - Charts commands (list/get/query) and read:analytics in default login scopes (MCP-472) - OAuth device-flow login (start/poll), agent/JSON output mode, structured errors - External sync guards and public-hygiene cleanup (MCP-431 #138033, #138056) - Add fastest-levenshtein dependency (monorepo parity) Verified: pnpm check:developer-cli-external-ready, sync --verify (423 tests) Co-authored-by: Cursor --- AGENTS.md | 38 ++ README.md | 41 +- docs/cli.md | 58 +- openapi/bundled/openapi.bundled.json | 659 +++++++++++++++++- openapi/bundled/openapi.bundled.yaml | 579 +++++++++++++++- package.json | 1 + pnpm-lock.yaml | 9 + src/args.ts | 132 +++- src/auth-commands.test.ts | 967 +++++++++++++++++++++++++++ src/auth-commands.ts | 902 ++++++++++++++++++++++--- src/auth-flag-coverage.test.ts | 101 +++ src/auth-guidance.test.ts | 4 +- src/auth-guidance.ts | 7 +- src/auth-login.test.ts | 174 ++++- src/auth-pat.test.ts | 160 ++++- src/auth-profiles.test.ts | 495 +++++++++++++- src/authToken.test.ts | 175 +++++ src/authToken.ts | 128 +++- src/catalog-shape.test.ts | 76 +++ src/catalog.test.ts | 78 +++ src/catalog.ts | 334 +++++++++ src/cli-error-usage.test.ts | 77 +++ src/cli-error.test.ts | 150 +++++ src/cli-error.ts | 160 +++++ src/cli-main-e2e.test.ts | 47 ++ src/cli-state.test.ts | 107 +++ src/cli-state.ts | 108 +++ src/cli.test.ts | 525 ++++++++++++++- src/cli.ts | 216 ++++-- src/config.test.ts | 47 +- src/config.ts | 59 +- src/credential-resolver.test.ts | 110 +++ src/credential-resolver.ts | 34 +- src/credential-store.test.ts | 4 +- src/credential-store.ts | 15 +- src/error-contract.test.ts | 127 ++++ src/errors.test.ts | 60 +- src/errors.ts | 59 +- src/generated/cli-manifest.ts | 149 +++++ src/generated/scopes.ts | 6 + src/help-drift.test.ts | 33 + src/help-json-drift.test.ts | 23 + src/help.test.ts | 248 ++++++- src/help.ts | 305 +++++---- src/output.ts | 6 +- src/pending-store.test.ts | 88 +++ src/pending-store.ts | 122 ++++ src/request.test.ts | 160 ++++- src/request.ts | 109 ++- src/run.test.ts | 59 ++ src/run.ts | 38 +- src/scopes.test.ts | 6 +- src/scopes.ts | 13 +- src/semantic-copy.test.ts | 105 +++ 54 files changed, 7896 insertions(+), 597 deletions(-) create mode 100644 src/auth-commands.test.ts create mode 100644 src/auth-flag-coverage.test.ts create mode 100644 src/catalog-shape.test.ts create mode 100644 src/catalog.test.ts create mode 100644 src/catalog.ts create mode 100644 src/cli-error-usage.test.ts create mode 100644 src/cli-error.test.ts create mode 100644 src/cli-error.ts create mode 100644 src/cli-main-e2e.test.ts create mode 100644 src/cli-state.test.ts create mode 100644 src/cli-state.ts create mode 100644 src/error-contract.test.ts create mode 100644 src/generated/scopes.ts create mode 100644 src/help-drift.test.ts create mode 100644 src/help-json-drift.test.ts create mode 100644 src/pending-store.test.ts create mode 100644 src/pending-store.ts create mode 100644 src/semantic-copy.test.ts diff --git a/AGENTS.md b/AGENTS.md index a1a6b55..6f5c442 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,3 +95,41 @@ readability; agents want minimal characters. never cost correctness). - Human-only adornments (spinners, prompts, ANSI codes) leaking into non-interactive output. + +## Conventions for changing commands (and how they're enforced) + +The rubric: the CLI optimizes for the agent characteristics that underpin +MCP — self-describing, semantic-not-prescriptive, structured/typed, uniform, +progressive, actionable-failure, stable-identity, explicit — expressed +CLI-natively rather than as a bolt-on protocol. + +**Edit / add / remove a command checklist** — each item names the guard that +enforces it: + +- API commands come from the OpenAPI spec → regenerate (`pnpm generate:cli`); + the manifest is the source of truth, never hand-edit `src/generated/**`. + (`verify:generated`) +- Auth/meta commands are hand-authored in `catalog.ts`'s `AUTH_COMMANDS`, kept + in sync with the routing in `cli.ts`. (`catalog.test.ts` parity; + `auth-flag-coverage.test.ts` proves handlers only read declared/global + flags, so the misplaced-flag reject can't false-positive) +- Every catalog command appears in help + JSON. + (`help-drift.test.ts`/`help-json-drift.test.ts`) +- Only authoritative/sourced fields belong in the catalog — no invented state + (e.g. no method-derived `destructive`). Adding a serialized field trips + `catalog-shape.test.ts`. Scopes come from the OpenAPI `x-required-scopes` + extension. +- Describe behavior semantically; no audience-targeting or scripted recipes in + summaries, descriptions, hints, or help. (`semantic-copy.test.ts`) +- Reader-aware output: prose at a TTY, JSON when piped or with `--json` + (`shouldUseJsonOutput`/`formatJsonOutput`). +- Failures: a structured JSON error on stderr plus a differentiated exit code + via `CliError` — never a bare `Error` for a user-facing failure. Sanctioned + exceptions (e.g. a TTY-only cancellation) carry a `// plain-error-ok: ` + marker on the throw. (exit-code map in `error-contract.test.ts`; every + `throw new Error(` without that marker fails `cli-error-usage.test.ts`) + - Exception: the device-flow verbs `auth login start`/`poll` always emit + their `{status,message,…}` envelope on **stdout** (it is the command's + primary output, relayed to the user), signalling failure via the exit code; + `poll` uses exit 75 for a still-`pending` authorization. A thrown `CliError` + on these paths keeps its own `error_code`/exit code. diff --git a/README.md b/README.md index 44ec4fe..675b6e2 100644 --- a/README.md +++ b/README.md @@ -41,36 +41,37 @@ npx @amplitude/developer-cli help ## Authentication -Run `amp auth login` to authenticate via the OAuth device flow. Creating a -profile is force-explicit — name it and pick its environment: +Run `amp auth login` to authenticate via the OAuth device flow. Omit +`--profile` and the CLI targets `default` — the implicit profile used when you +don't name one. Picking a region stays explicit: ```bash -amp auth login --profile prod --env prod # device flow → save + activate +amp auth login --region us # device flow → "default" profile amp auth status # active credential, type, expiry amp context ``` -Profiles bind a credential to an environment (its `base_url`), so a staging -token can never be sent to prod. Add more and switch between them: +Profiles bind a credential to a region (its `base_url`), so an EU token can +never be sent to prod. Name one explicitly when you want more than one: ```bash -amp auth login --profile staging --env staging # activates staging +amp auth login --profile eu --region eu # activates the EU profile amp auth use prod # switch back (no re-auth) amp auth list # * marks the active profile -amp logout --profile staging # remove one +amp logout --profile eu # remove one amp logout --all # remove every profile ``` A bare `amp auth login` re-authenticates the active profile in place. Profiles are saved to `~/.amplitude/amp/credentials.json` (0600). -Prefer a Personal Access Token? `amp auth pat --with-token --profile --env ` -reads a PAT from stdin (or a masked prompt at a terminal) and saves it as a -profile — same force-explicit create rule, same store, just a different -credential type. `--with-token` is required. +Prefer a Personal Access Token? `amp auth pat --with-token --region ` reads a +PAT from stdin (or a masked prompt at a terminal) and saves it as a profile — +`--profile` optional (defaults to `default`), same force-explicit create rule, +same store. `--with-token` is required. ```bash -echo "$PAT" | amp auth pat --with-token --profile ci --env prod +echo "$PAT" | amp auth pat --with-token --region us ``` For CI or a one-off shell, set a raw token — it overrides stored profiles: @@ -82,22 +83,6 @@ export AMP_TOKEN=amp_... # amp_… is treated as a PAT, anything else a b `AMP_PROFILE` selects a stored profile by name; `amp auth token` prints the active access token to stdout (`TOKEN=$(amp auth token)`). -## Base URL - -Defaults to production: - -```text -https://developer-api.amplitude.com -``` - -Override for staging or a local server: - -```bash -export AMP_API_BASE_URL=http://localhost:3036 -# or -amp --base-url http://localhost:3036 context -``` - ## Examples ```bash diff --git a/docs/cli.md b/docs/cli.md index ef95c89..80c0a57 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -5,42 +5,42 @@ OpenAPI spec. A handwritten runtime in `src/cli.ts` maps flags to HTTP requests. ## Authentication -Authenticate via the OAuth device flow and save the result as a named profile. -Creating a profile is force-explicit — name it and pick its environment: +Authenticate via the OAuth device flow and save the result as a profile. Omit +`--profile` and the CLI targets `default` — the implicit profile used when you +don't name one. Picking a region stays explicit: ```bash -amp auth login --profile prod --env prod +amp auth login --region us ``` -That runs the device flow against the chosen environment's Developer API, saves +That runs the device flow against the chosen region's Developer API, saves the token to `~/.amplitude/amp/credentials.json` (0600), and activates the -profile. A -profile binds a credential to an environment (`base_url`), so a staging token -can never be sent to prod. +profile. A profile binds a credential to a region (`base_url`), so an EU +token can never be sent to prod. ```bash -amp auth login --profile staging --env staging # activates staging +amp auth login --profile eu --region eu # activates the EU profile amp auth use prod # switch (no re-auth) amp auth list # * marks the active profile amp auth status # type, base URL, expiry -amp logout --profile staging # remove one +amp logout --profile eu # remove one amp logout --all # remove every profile ``` -A bare `amp auth login` re-authenticates the active profile in place (env from -its record). `--env` maps `local|dev|staging|prod|prod-eu` to a base URL; pass -`--base-url ` instead to target an arbitrary host. +A bare `amp auth login` re-authenticates the active profile in place (region +from its record). `--region` maps `us|eu` to a base URL. To use a Personal Access Token instead of the device flow: ```bash -echo "$PAT" | amp auth pat --with-token --profile ci --env prod # stdin -amp auth pat --with-token --profile ci --env prod # masked prompt at a TTY +echo "$PAT" | amp auth pat --with-token --region us # stdin → "default" profile +amp auth pat --with-token --profile ci --region us # name it; masked prompt at a TTY ``` -`--with-token` reads the PAT from stdin when piped (the agent / CI path) or a -masked prompt at a terminal (the human path). Same force-explicit create rule -and store as `login`; the credential is saved as `{ "type": "pat" }`. +`--with-token` reads the PAT from stdin when piped, or from a masked prompt at a +terminal. Same rules as `login`: `--profile` +is optional (defaults to `default`) and `--region` is required to create a +profile; the credential is saved as `{ "type": "pat" }`. `--with-token` is mandatory: it makes the supply-an-existing-PAT path explicit and keeps the bare `amp auth pat` verb reserved. @@ -54,7 +54,7 @@ profile; `amp auth status` announces when it is in effect. ```bash export AMP_TOKEN=amp_... # one-off / CI; overrides stored profiles -export AMP_PROFILE=staging # select a stored profile by name +export AMP_PROFILE=eu # select a stored profile by name ``` Accepted forms for `--token` / `AMP_TOKEN`: @@ -78,15 +78,20 @@ scopes below document what each command needs. | `context`, `projects list` | `read:projects` | | `events *` | `read:taxonomy`, `write:taxonomy` | | `flags *` | `read:flags`, `write:flags` | +| `charts *` | `read:analytics` | Route-level scopes are defined on each OpenAPI operation (`x-required-scopes`). +When adding new CLI scopes, ensure the OAuth client used for `amp auth login` +allows those scopes before default login requests them. This requires an +out-of-band update on Amplitude's side — contact Amplitude if your integration +needs additional scopes. + ## Global flags | Flag / env | Purpose | | --------------------------------------- | ------------------------------------------ | -| `--base-url` / `AMP_API_BASE_URL` | API host (default: prod) | -| `--env ` | Friendly env name → base URL | +| `--region ` | Data region name → base URL | | `--profile ` / `AMP_PROFILE` | Select a stored profile | | `--token` / `AMP_TOKEN` / saved profile | Auth token | | `--json` | Force raw JSON output (default when piped) | @@ -97,7 +102,7 @@ Route-level scopes are defined on each OpenAPI operation (`x-required-scopes`). ## Help and output ```bash -amp help # product surfaces (context, projects, events, flags, …) +amp help # product surfaces (context, projects, events, flags, charts, …) amp help flags # commands within a surface amp flags list --help # flags and examples for one command amp version @@ -125,19 +130,12 @@ Manual checklist: 3. **Projects** — `amp projects list` 4. **Flags read** — `amp flags list --project --limit 5` 5. **Flags write** — create → get → update description → archive dry-run → archive -6. **Events** — list → create → update → (optional delete with `--yes`) +6. **Charts read** — `amp charts list --project --limit 5` +7. **Events** — list → create → update → (optional delete with `--yes`) Known issue: `flags update --enabled false` fails for deployment-less flags. Avoid that path in smoke tests until it is fixed. -## Local server - -Point at a running local server: - -```bash -amp --base-url http://localhost:3036 context -``` - ## Generated files Do not edit by hand: diff --git a/openapi/bundled/openapi.bundled.json b/openapi/bundled/openapi.bundled.json index 9de5081..7cf9ffe 100644 --- a/openapi/bundled/openapi.bundled.json +++ b/openapi/bundled/openapi.bundled.json @@ -43,6 +43,10 @@ "name": "Taxonomy User Properties", "description": "User-property taxonomy operations (scoped to a project)." }, + { + "name": "Analytics", + "description": "Saved chart discovery and query operations." + }, { "name": "Feature Flags", "description": "Feature flag configuration and rollout operations." @@ -1277,6 +1281,212 @@ } } }, + "/v1/projects/{project_id}/charts": { + "get": { + "tags": ["Analytics"], + "operationId": "listCharts", + "summary": "List charts", + "description": "Returns saved charts in the project. Results are paginated with cursor-based\npagination. Chart definitions are omitted; use GET chart with\n`include_definition=true` to retrieve a read-only definition.\n\nPagination uses an opaque numeric cursor (offset into the current sorted\nsearch result set). Each list query can access at most **10,000** matching\ncharts from the search index; pages beyond that window return fewer items\nand `next_cursor: null` even when more charts may exist in the project.\nUse filters (`q`, `chart_type`) to narrow large projects.\n\nSortable fields are `updated_at` and `name`; prefix with `-` for descending.\nDefaults to `-updated_at` (most recently updated first). `created_at` is not\nsupported in this API version.\n", + "x-required-scopes": ["read:analytics"], + "parameters": [ + { + "$ref": "#/components/parameters/ProjectId" + }, + { + "$ref": "#/components/parameters/Cursor" + }, + { + "$ref": "#/components/parameters/Limit" + }, + { + "$ref": "#/components/parameters/Query" + }, + { + "$ref": "#/components/parameters/ChartType" + }, + { + "$ref": "#/components/parameters/Sort" + } + ], + "responses": { + "200": { + "description": "Successful response.", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["data", "pagination"], + "properties": { + "data": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Chart" + } + }, + "pagination": { + "$ref": "#/components/schemas/Pagination" + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationProblem" + }, + "401": { + "$ref": "#/components/responses/Problem" + }, + "403": { + "$ref": "#/components/responses/Problem" + }, + "500": { + "$ref": "#/components/responses/Problem" + } + } + } + }, + "/v1/projects/{project_id}/charts/{chart_id}": { + "parameters": [ + { + "$ref": "#/components/parameters/ProjectId" + }, + { + "$ref": "#/components/parameters/ChartId" + } + ], + "get": { + "tags": ["Analytics"], + "operationId": "getChart", + "summary": "Get chart", + "description": "Returns chart metadata. The chart definition is omitted by default. Set\n`include_definition=true` to include a read-only definition object. The\ndefinition shape is unstable and not intended for authoring.\n", + "x-required-scopes": ["read:analytics"], + "parameters": [ + { + "$ref": "#/components/parameters/IncludeDefinition" + } + ], + "responses": { + "200": { + "description": "Successful response.", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["data"], + "properties": { + "data": { + "$ref": "#/components/schemas/Chart" + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationProblem" + }, + "401": { + "$ref": "#/components/responses/Problem" + }, + "403": { + "$ref": "#/components/responses/Problem" + }, + "404": { + "$ref": "#/components/responses/Problem" + }, + "500": { + "$ref": "#/components/responses/Problem" + } + } + } + }, + "/v1/projects/{project_id}/charts/{chart_id}/query": { + "parameters": [ + { + "$ref": "#/components/parameters/ProjectId" + }, + { + "$ref": "#/components/parameters/ChartId" + } + ], + "post": { + "tags": ["Analytics"], + "operationId": "queryChart", + "summary": "Query chart", + "description": "Computes and returns normalized results for a saved chart. v1 supports\n`event_segmentation`, `sessions`, `funnels`, and `retention` chart types;\nother chart types return `422` with `error_code: unsupported_chart_type`.\nResults reflect the authenticated caller's project access and chart\npermissions.\n\nThis POST computes a result and does not mutate state. It is therefore a\nread operation and does not require an `Idempotency-Key`.\n\nOmitting `time_range` uses the chart's saved range; if the chart has no\nsaved range, the server defaults to the last 30 days.\n\nQuery is synchronous with bounded defaults. Expensive queries may return\n`504` when exceeding the server timeout; async query jobs are planned for\na follow-up slice. Retryable responses (`429`, `502`, `504`) populate\n`retry_after_seconds` in the problem body when a delay is advised.\n", + "x-required-scopes": ["read:analytics"], + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ChartQueryRequest" + } + } + } + }, + "responses": { + "200": { + "description": "Query completed successfully.", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["data"], + "properties": { + "data": { + "$ref": "#/components/schemas/AnalyticsResult" + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationProblem" + }, + "401": { + "$ref": "#/components/responses/Problem" + }, + "403": { + "$ref": "#/components/responses/Problem" + }, + "404": { + "$ref": "#/components/responses/Problem" + }, + "422": { + "description": "Chart type is not supported for public query in this version. The\nrequest was understood but cannot be processed. Returned with\n`error_code: unsupported_chart_type`; `detail` names the chart type\nand the currently supported set.\n", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "429": { + "$ref": "#/components/responses/Problem" + }, + "500": { + "$ref": "#/components/responses/Problem" + }, + "502": { + "$ref": "#/components/responses/Problem" + }, + "504": { + "description": "Query exceeded the synchronous timeout.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/v1/projects/{project_id}/flags": { "get": { "tags": ["Feature Flags"], @@ -1637,7 +1847,7 @@ "name": "cursor", "in": "query", "required": false, - "description": "Cursor token from previous list response.", + "description": "Opaque cursor from `pagination.next_cursor` on the previous response.\nNumeric offsets are capped by the underlying search index (10,000 results\nper query). Well-formed pagination stops before that window; requests whose\ncursor would exceed it return 400 `pagination_not_supported`.\n", "schema": { "type": ["string", "null"] } @@ -1658,10 +1868,10 @@ "name": "sort", "in": "query", "required": false, - "description": "Comma-separated sort fields. Prefix with `-` for descending.", + "description": "Sort field. Prefix with `-` for descending. Supported values are `updated_at` and `name`.", "schema": { "type": "string", - "example": "-created_at,name" + "example": "-updated_at" } }, "Query": { @@ -1734,6 +1944,35 @@ "minLength": 1 } }, + "ChartId": { + "name": "chart_id", + "in": "path", + "required": true, + "description": "Saved chart identifier.", + "schema": { + "type": "string", + "minLength": 1 + } + }, + "ChartType": { + "name": "chart_type", + "in": "query", + "required": false, + "description": "Filter list results to a specific chart type. Use `unknown` to return charts\nwhose type is not represented by a public API value.\n", + "schema": { + "$ref": "#/components/schemas/ChartType" + } + }, + "IncludeDefinition": { + "name": "include_definition", + "in": "query", + "required": false, + "description": "When true, include the read-only chart definition on GET chart. The definition\nshape is unstable and not intended for authoring.\n", + "schema": { + "type": "boolean", + "default": false + } + }, "ExperimentId": { "name": "experiment_id", "in": "path", @@ -1794,7 +2033,8 @@ "required": ["next_cursor", "has_more"], "properties": { "next_cursor": { - "type": ["string", "null"] + "type": ["string", "null"], + "description": "Cursor for the next page, or `null` on the last page. Chart list\nresponses may stop before all project charts are enumerated because\nthe search backend caps each query at 10,000 sorted hits.\n" }, "has_more": { "type": "boolean" @@ -2068,6 +2308,204 @@ "type": ["string", "null"], "enum": ["string", "number", "boolean", "object", "enum", "any", null] }, + "Chart": { + "type": "object", + "required": [ + "id", + "object", + "project_id", + "name", + "chart_type", + "created_at", + "updated_at", + "url", + "archived" + ], + "properties": { + "id": { + "type": "string", + "description": "Stable chart identifier." + }, + "object": { + "type": "string", + "const": "chart", + "description": "Resource-type discriminator. Always `chart`." + }, + "project_id": { + "type": "string", + "pattern": "^[0-9]+$" + }, + "name": { + "type": "string" + }, + "description": { + "type": ["string", "null"] + }, + "chart_type": { + "$ref": "#/components/schemas/ChartType" + }, + "created_at": { + "type": ["string", "null"], + "format": "date-time", + "description": "ISO-8601 creation time, or `null` when unavailable." + }, + "updated_at": { + "type": ["string", "null"], + "format": "date-time", + "description": "ISO-8601 last-modified time, or `null` when unavailable." + }, + "created_by": { + "type": ["string", "null"], + "description": "Chart creator. The creator's email when available, otherwise a login\nidentifier; `null` when the creator cannot be resolved.\n" + }, + "url": { + "type": "string", + "format": "uri", + "description": "Deep link to the chart in the Amplitude UI." + }, + "archived": { + "type": "boolean", + "description": "Whether the chart is archived." + }, + "definition": { + "type": "object", + "additionalProperties": true, + "description": "Read-only chart definition. Returned only when `include_definition=true`\non GET chart. Shape is unstable and may change between API versions;\nnot intended for authoring.\n" + } + } + }, + "ChartType": { + "type": "string", + "description": "Public chart type discriminator (snake_case). All values may appear on\nlist/get; query returns `422` (`unsupported_chart_type`) for types outside\nthe v1 supported matrix.\n\nUnsupported or unrecognized chart types are surfaced as `unknown`.\n\nNew chart types may be added over time. Clients should treat `unknown` as\n\"a chart type this API version does not model yet\".\n", + "enum": [ + "event_segmentation", + "sessions", + "funnels", + "retention", + "composition", + "revenue_ltv", + "stickiness", + "data_table", + "engagement_matrix", + "metric_explorer", + "growth_accounting", + "impact", + "users", + "unknown" + ] + }, + "ChartQueryRequest": { + "type": "object", + "description": "v1 query parameters. Filter and group-by overrides are not supported in v1.\n", + "properties": { + "time_range": { + "$ref": "#/components/schemas/TimeRange" + }, + "timezone": { + "type": "string", + "description": "IANA timezone identifier (e.g. `America/New_York`).", + "example": "America/New_York" + }, + "exclude_incomplete_datapoints": { + "type": "boolean", + "default": false, + "description": "When true, excludes the current incomplete interval from results.\n" + }, + "group_by_limit": { + "type": "integer", + "minimum": 1, + "maximum": 1000, + "default": 10, + "description": "Maximum number of breakdown groups returned per series." + }, + "time_series_limit": { + "type": "integer", + "minimum": 0, + "maximum": 1000, + "default": 100, + "description": "Maximum number of time buckets per series. `0` collapses each series to\na single scalar aggregate.\n" + } + }, + "additionalProperties": false + }, + "AnalyticsResult": { + "type": "object", + "required": [ + "id", + "object", + "source_type", + "source_id", + "project_id", + "computed_at", + "timezone", + "result_kind", + "metric_semantics", + "data", + "metadata", + "warnings" + ], + "properties": { + "id": { + "type": "string", + "description": "Identifier for this query execution." + }, + "object": { + "type": "string", + "const": "analytics_result", + "description": "Resource-type discriminator. Always `analytics_result`." + }, + "source_type": { + "type": "string", + "enum": ["chart"], + "description": "Source artifact type. v1 supports `chart` only. `dashboard` and `query`\nare reserved for future slices.\n" + }, + "source_id": { + "type": "string", + "description": "Identifier of the source chart." + }, + "project_id": { + "type": "string", + "pattern": "^[0-9]+$" + }, + "computed_at": { + "type": "string", + "format": "date-time" + }, + "timezone": { + "type": "string", + "description": "IANA timezone used for computation." + }, + "result_kind": { + "$ref": "#/components/schemas/ResultKind" + }, + "metric_semantics": { + "$ref": "#/components/schemas/MetricSemantics" + }, + "data": { + "$ref": "#/components/schemas/AnalyticsResultData" + }, + "metadata": { + "$ref": "#/components/schemas/AnalyticsResultMetadata" + }, + "warnings": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Non-fatal issues encountered during query execution." + }, + "truncated": { + "oneOf": [ + { + "$ref": "#/components/schemas/TruncationInfo" + }, + { + "type": "null" + } + ] + } + } + }, "FeatureFlag": { "allOf": [ { @@ -2271,6 +2709,219 @@ } } }, + "TimeRange": { + "type": "object", + "required": ["start", "end"], + "properties": { + "start": { + "type": "string", + "format": "date", + "description": "Inclusive start date (project timezone unless `timezone` is set on query)." + }, + "end": { + "type": "string", + "format": "date", + "description": "Inclusive end date." + } + }, + "additionalProperties": false + }, + "ResultKind": { + "type": "string", + "description": "High-level shape of the normalized result payload. Adapters set this\nexplicitly per supported chart type; `unknown` means the server could not\nclassify the result shape safely.\n", + "enum": [ + "timeseries", + "scalar", + "funnel", + "retention", + "table", + "unknown" + ] + }, + "RecommendedAggregate": { + "type": "string", + "description": "Recommended aggregation method for downstream consumers. Non-additive metrics\n(e.g. unique users) must not be summed across intervals. `unknown` is used\nwhen the adapter cannot determine a safe aggregate; treat values as\nnon-additive in that case.\n", + "enum": ["sum", "last", "mean", "deduped_total", "none", "unknown"] + }, + "MetricSemantics": { + "type": "object", + "required": ["additive", "recommended_aggregate"], + "properties": { + "additive": { + "type": "boolean", + "description": "When false, values must not be summed across time intervals or groups\nwithout understanding deduplication semantics.\n" + }, + "recommended_aggregate": { + "$ref": "#/components/schemas/RecommendedAggregate" + }, + "notes": { + "type": ["string", "null"], + "description": "Human-readable guidance for interpreting values." + } + } + }, + "ResultDimension": { + "type": "object", + "required": ["id", "label", "role"], + "properties": { + "id": { + "type": "string" + }, + "label": { + "type": "string" + }, + "role": { + "type": "string", + "enum": ["time", "segment", "breakdown", "step"], + "description": "Role of this dimension in the result." + } + } + }, + "ResultPoint": { + "type": "object", + "required": ["x", "y"], + "properties": { + "x": { + "description": "Dimension value (typically an ISO date or category label).", + "oneOf": [ + { + "type": "string" + }, + { + "type": "number" + } + ] + }, + "y": { + "type": ["number", "null"], + "description": "Metric value at this point." + }, + "complete": { + "type": "boolean", + "description": "When false, the interval is incomplete (current bucket). Only present\nwhen `exclude_incomplete_datapoints` is false.\n" + } + } + }, + "ResultSeries": { + "type": "object", + "required": ["id", "label", "points"], + "properties": { + "id": { + "type": "string" + }, + "label": { + "type": "string" + }, + "points": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ResultPoint" + } + }, + "aggregate": { + "type": "object", + "description": "Optional pre-computed aggregate for the series.", + "properties": { + "value": { + "type": ["number", "null"] + }, + "method": { + "type": "string" + } + }, + "required": ["value", "method"] + } + } + }, + "AnalyticsResultData": { + "type": "object", + "description": "Normalized result payload. Populated fields depend on `result_kind`.\nTimeseries and funnel results use `dimensions` and `series`. Table results\nuse `columns` and `rows`.\n", + "properties": { + "dimensions": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ResultDimension" + } + }, + "series": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ResultSeries" + } + }, + "columns": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Column headers for table-shaped results." + }, + "rows": { + "type": "array", + "items": { + "type": "array", + "items": { + "oneOf": [ + { + "type": "string" + }, + { + "type": "number" + }, + { + "type": "boolean" + }, + { + "type": "null" + } + ] + } + } + } + } + }, + "AnalyticsResultMetadata": { + "type": "object", + "description": "Echo of effective query parameters and chart context.", + "properties": { + "chart_type": { + "$ref": "#/components/schemas/ChartType" + }, + "chart_name": { + "type": "string" + }, + "time_range": { + "$ref": "#/components/schemas/TimeRange" + }, + "exclude_incomplete_datapoints": { + "type": "boolean" + }, + "group_by_limit": { + "type": "integer" + }, + "time_series_limit": { + "type": "integer" + } + }, + "additionalProperties": true + }, + "TruncationInfo": { + "type": "object", + "properties": { + "group_by_limit": { + "type": "integer", + "description": "Applied group-by limit when breakdown was truncated." + }, + "time_series_limit": { + "type": "integer", + "description": "Applied time-series limit when buckets were truncated." + }, + "reason": { + "type": "string", + "description": "Human-readable explanation of truncation." + } + } + }, "RolloutWeights": { "type": "object", "additionalProperties": { diff --git a/openapi/bundled/openapi.bundled.yaml b/openapi/bundled/openapi.bundled.yaml index f54e382..4adf2c9 100644 --- a/openapi/bundled/openapi.bundled.yaml +++ b/openapi/bundled/openapi.bundled.yaml @@ -27,6 +27,8 @@ tags: description: Event-property taxonomy operations (scoped to an event). - name: Taxonomy User Properties description: User-property taxonomy operations (scoped to a project). + - name: Analytics + description: Saved chart discovery and query operations. - name: Feature Flags description: Feature flag configuration and rollout operations. - name: Auth @@ -920,6 +922,175 @@ paths: $ref: '#/components/responses/Problem' '500': $ref: '#/components/responses/Problem' + /v1/projects/{project_id}/charts: + get: + tags: + - Analytics + operationId: listCharts + summary: List charts + description: | + Returns saved charts in the project. Results are paginated with cursor-based + pagination. Chart definitions are omitted; use GET chart with + `include_definition=true` to retrieve a read-only definition. + + Pagination uses an opaque numeric cursor (offset into the current sorted + search result set). Each list query can access at most **10,000** matching + charts from the search index; pages beyond that window return fewer items + and `next_cursor: null` even when more charts may exist in the project. + Use filters (`q`, `chart_type`) to narrow large projects. + + Sortable fields are `updated_at` and `name`; prefix with `-` for descending. + Defaults to `-updated_at` (most recently updated first). `created_at` is not + supported in this API version. + x-required-scopes: + - read:analytics + parameters: + - $ref: '#/components/parameters/ProjectId' + - $ref: '#/components/parameters/Cursor' + - $ref: '#/components/parameters/Limit' + - $ref: '#/components/parameters/Query' + - $ref: '#/components/parameters/ChartType' + - $ref: '#/components/parameters/Sort' + responses: + '200': + description: Successful response. + content: + application/json: + schema: + type: object + required: + - data + - pagination + properties: + data: + type: array + items: + $ref: '#/components/schemas/Chart' + pagination: + $ref: '#/components/schemas/Pagination' + '400': + $ref: '#/components/responses/ValidationProblem' + '401': + $ref: '#/components/responses/Problem' + '403': + $ref: '#/components/responses/Problem' + '500': + $ref: '#/components/responses/Problem' + /v1/projects/{project_id}/charts/{chart_id}: + parameters: + - $ref: '#/components/parameters/ProjectId' + - $ref: '#/components/parameters/ChartId' + get: + tags: + - Analytics + operationId: getChart + summary: Get chart + description: | + Returns chart metadata. The chart definition is omitted by default. Set + `include_definition=true` to include a read-only definition object. The + definition shape is unstable and not intended for authoring. + x-required-scopes: + - read:analytics + parameters: + - $ref: '#/components/parameters/IncludeDefinition' + responses: + '200': + description: Successful response. + content: + application/json: + schema: + type: object + required: + - data + properties: + data: + $ref: '#/components/schemas/Chart' + '400': + $ref: '#/components/responses/ValidationProblem' + '401': + $ref: '#/components/responses/Problem' + '403': + $ref: '#/components/responses/Problem' + '404': + $ref: '#/components/responses/Problem' + '500': + $ref: '#/components/responses/Problem' + /v1/projects/{project_id}/charts/{chart_id}/query: + parameters: + - $ref: '#/components/parameters/ProjectId' + - $ref: '#/components/parameters/ChartId' + post: + tags: + - Analytics + operationId: queryChart + summary: Query chart + description: | + Computes and returns normalized results for a saved chart. v1 supports + `event_segmentation`, `sessions`, `funnels`, and `retention` chart types; + other chart types return `422` with `error_code: unsupported_chart_type`. + Results reflect the authenticated caller's project access and chart + permissions. + + This POST computes a result and does not mutate state. It is therefore a + read operation and does not require an `Idempotency-Key`. + + Omitting `time_range` uses the chart's saved range; if the chart has no + saved range, the server defaults to the last 30 days. + + Query is synchronous with bounded defaults. Expensive queries may return + `504` when exceeding the server timeout; async query jobs are planned for + a follow-up slice. Retryable responses (`429`, `502`, `504`) populate + `retry_after_seconds` in the problem body when a delay is advised. + x-required-scopes: + - read:analytics + requestBody: + required: false + content: + application/json: + schema: + $ref: '#/components/schemas/ChartQueryRequest' + responses: + '200': + description: Query completed successfully. + content: + application/json: + schema: + type: object + required: + - data + properties: + data: + $ref: '#/components/schemas/AnalyticsResult' + '400': + $ref: '#/components/responses/ValidationProblem' + '401': + $ref: '#/components/responses/Problem' + '403': + $ref: '#/components/responses/Problem' + '404': + $ref: '#/components/responses/Problem' + '422': + description: | + Chart type is not supported for public query in this version. The + request was understood but cannot be processed. Returned with + `error_code: unsupported_chart_type`; `detail` names the chart type + and the currently supported set. + content: + application/problem+json: + schema: + $ref: '#/components/schemas/ProblemDetails' + '429': + $ref: '#/components/responses/Problem' + '500': + $ref: '#/components/responses/Problem' + '502': + $ref: '#/components/responses/Problem' + '504': + description: Query exceeded the synchronous timeout. + content: + application/problem+json: + schema: + $ref: '#/components/schemas/ProblemDetails' /v1/projects/{project_id}/flags: get: tags: @@ -1179,7 +1350,11 @@ components: name: cursor in: query required: false - description: Cursor token from previous list response. + description: | + Opaque cursor from `pagination.next_cursor` on the previous response. + Numeric offsets are capped by the underlying search index (10,000 results + per query). Well-formed pagination stops before that window; requests whose + cursor would exceed it return 400 `pagination_not_supported`. schema: type: - string @@ -1198,10 +1373,10 @@ components: name: sort in: query required: false - description: Comma-separated sort fields. Prefix with `-` for descending. + description: Sort field. Prefix with `-` for descending. Supported values are `updated_at` and `name`. schema: type: string - example: '-created_at,name' + example: '-updated_at' Query: name: q in: query @@ -1262,6 +1437,33 @@ components: schema: type: string minLength: 1 + ChartId: + name: chart_id + in: path + required: true + description: Saved chart identifier. + schema: + type: string + minLength: 1 + ChartType: + name: chart_type + in: query + required: false + description: | + Filter list results to a specific chart type. Use `unknown` to return charts + whose type is not represented by a public API value. + schema: + $ref: '#/components/schemas/ChartType' + IncludeDefinition: + name: include_definition + in: query + required: false + description: | + When true, include the read-only chart definition on GET chart. The definition + shape is unstable and not intended for authoring. + schema: + type: boolean + default: false ExperimentId: name: experiment_id in: path @@ -1316,6 +1518,10 @@ components: type: - string - 'null' + description: | + Cursor for the next page, or `null` on the last page. Chart list + responses may stop before all project charts are enumerated because + the search backend caps each query at 10,000 sorted hits. has_more: type: boolean ProblemDetails: @@ -1566,6 +1772,186 @@ components: - enum - any - null + Chart: + type: object + required: + - id + - object + - project_id + - name + - chart_type + - created_at + - updated_at + - url + - archived + properties: + id: + type: string + description: Stable chart identifier. + object: + type: string + const: chart + description: Resource-type discriminator. Always `chart`. + project_id: + type: string + pattern: ^[0-9]+$ + name: + type: string + description: + type: + - string + - 'null' + chart_type: + $ref: '#/components/schemas/ChartType' + created_at: + type: + - string + - 'null' + format: date-time + description: ISO-8601 creation time, or `null` when unavailable. + updated_at: + type: + - string + - 'null' + format: date-time + description: ISO-8601 last-modified time, or `null` when unavailable. + created_by: + type: + - string + - 'null' + description: | + Chart creator. The creator's email when available, otherwise a login + identifier; `null` when the creator cannot be resolved. + url: + type: string + format: uri + description: Deep link to the chart in the Amplitude UI. + archived: + type: boolean + description: Whether the chart is archived. + definition: + type: object + additionalProperties: true + description: | + Read-only chart definition. Returned only when `include_definition=true` + on GET chart. Shape is unstable and may change between API versions; + not intended for authoring. + ChartType: + type: string + description: | + Public chart type discriminator (snake_case). All values may appear on + list/get; query returns `422` (`unsupported_chart_type`) for types outside + the v1 supported matrix. + + Unsupported or unrecognized chart types are surfaced as `unknown`. + + New chart types may be added over time. Clients should treat `unknown` as + "a chart type this API version does not model yet". + enum: + - event_segmentation + - sessions + - funnels + - retention + - composition + - revenue_ltv + - stickiness + - data_table + - engagement_matrix + - metric_explorer + - growth_accounting + - impact + - users + - unknown + ChartQueryRequest: + type: object + description: | + v1 query parameters. Filter and group-by overrides are not supported in v1. + properties: + time_range: + $ref: '#/components/schemas/TimeRange' + timezone: + type: string + description: IANA timezone identifier (e.g. `America/New_York`). + example: America/New_York + exclude_incomplete_datapoints: + type: boolean + default: false + description: | + When true, excludes the current incomplete interval from results. + group_by_limit: + type: integer + minimum: 1 + maximum: 1000 + default: 10 + description: Maximum number of breakdown groups returned per series. + time_series_limit: + type: integer + minimum: 0 + maximum: 1000 + default: 100 + description: | + Maximum number of time buckets per series. `0` collapses each series to + a single scalar aggregate. + additionalProperties: false + AnalyticsResult: + type: object + required: + - id + - object + - source_type + - source_id + - project_id + - computed_at + - timezone + - result_kind + - metric_semantics + - data + - metadata + - warnings + properties: + id: + type: string + description: Identifier for this query execution. + object: + type: string + const: analytics_result + description: Resource-type discriminator. Always `analytics_result`. + source_type: + type: string + enum: + - chart + description: | + Source artifact type. v1 supports `chart` only. `dashboard` and `query` + are reserved for future slices. + source_id: + type: string + description: Identifier of the source chart. + project_id: + type: string + pattern: ^[0-9]+$ + computed_at: + type: string + format: date-time + timezone: + type: string + description: IANA timezone used for computation. + result_kind: + $ref: '#/components/schemas/ResultKind' + metric_semantics: + $ref: '#/components/schemas/MetricSemantics' + data: + $ref: '#/components/schemas/AnalyticsResultData' + metadata: + $ref: '#/components/schemas/AnalyticsResultMetadata' + warnings: + type: array + items: + type: string + description: Non-fatal issues encountered during query execution. + truncated: + oneOf: + - $ref: '#/components/schemas/TruncationInfo' + - type: 'null' FeatureFlag: allOf: - $ref: '#/components/schemas/FeatureFlagBase' @@ -1738,6 +2124,193 @@ components: type: string id_token: type: string + TimeRange: + type: object + required: + - start + - end + properties: + start: + type: string + format: date + description: Inclusive start date (project timezone unless `timezone` is set on query). + end: + type: string + format: date + description: Inclusive end date. + additionalProperties: false + ResultKind: + type: string + description: | + High-level shape of the normalized result payload. Adapters set this + explicitly per supported chart type; `unknown` means the server could not + classify the result shape safely. + enum: + - timeseries + - scalar + - funnel + - retention + - table + - unknown + RecommendedAggregate: + type: string + description: | + Recommended aggregation method for downstream consumers. Non-additive metrics + (e.g. unique users) must not be summed across intervals. `unknown` is used + when the adapter cannot determine a safe aggregate; treat values as + non-additive in that case. + enum: + - sum + - last + - mean + - deduped_total + - none + - unknown + MetricSemantics: + type: object + required: + - additive + - recommended_aggregate + properties: + additive: + type: boolean + description: | + When false, values must not be summed across time intervals or groups + without understanding deduplication semantics. + recommended_aggregate: + $ref: '#/components/schemas/RecommendedAggregate' + notes: + type: + - string + - 'null' + description: Human-readable guidance for interpreting values. + ResultDimension: + type: object + required: + - id + - label + - role + properties: + id: + type: string + label: + type: string + role: + type: string + enum: + - time + - segment + - breakdown + - step + description: Role of this dimension in the result. + ResultPoint: + type: object + required: + - x + - 'y' + properties: + x: + description: Dimension value (typically an ISO date or category label). + oneOf: + - type: string + - type: number + 'y': + type: + - number + - 'null' + description: Metric value at this point. + complete: + type: boolean + description: | + When false, the interval is incomplete (current bucket). Only present + when `exclude_incomplete_datapoints` is false. + ResultSeries: + type: object + required: + - id + - label + - points + properties: + id: + type: string + label: + type: string + points: + type: array + items: + $ref: '#/components/schemas/ResultPoint' + aggregate: + type: object + description: Optional pre-computed aggregate for the series. + properties: + value: + type: + - number + - 'null' + method: + type: string + required: + - value + - method + AnalyticsResultData: + type: object + description: | + Normalized result payload. Populated fields depend on `result_kind`. + Timeseries and funnel results use `dimensions` and `series`. Table results + use `columns` and `rows`. + properties: + dimensions: + type: array + items: + $ref: '#/components/schemas/ResultDimension' + series: + type: array + items: + $ref: '#/components/schemas/ResultSeries' + columns: + type: array + items: + type: string + description: Column headers for table-shaped results. + rows: + type: array + items: + type: array + items: + oneOf: + - type: string + - type: number + - type: boolean + - type: 'null' + AnalyticsResultMetadata: + type: object + description: Echo of effective query parameters and chart context. + properties: + chart_type: + $ref: '#/components/schemas/ChartType' + chart_name: + type: string + time_range: + $ref: '#/components/schemas/TimeRange' + exclude_incomplete_datapoints: + type: boolean + group_by_limit: + type: integer + time_series_limit: + type: integer + additionalProperties: true + TruncationInfo: + type: object + properties: + group_by_limit: + type: integer + description: Applied group-by limit when breakdown was truncated. + time_series_limit: + type: integer + description: Applied time-series limit when buckets were truncated. + reason: + type: string + description: Human-readable explanation of truncation. RolloutWeights: type: object additionalProperties: diff --git a/package.json b/package.json index e056159..4306de9 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "@inquirer/prompts": "^8.5.2", "chalk": "^4.1.2", "commander": "^15.0.0", + "fastest-levenshtein": "^1.0.16", "open": "^11.0.0", "zod": "^4.3.6" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1d47951..a8c830c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -17,6 +17,9 @@ importers: commander: specifier: ^15.0.0 version: 15.0.0 + fastest-levenshtein: + specifier: ^1.0.16 + version: 1.0.16 open: specifier: ^11.0.0 version: 11.0.0 @@ -620,6 +623,10 @@ packages: fast-wrap-ansi@0.2.2: resolution: {integrity: sha512-7F2Fl+TjRSenLqlU3UjSH0iyqopqoZIu7eZVpEirP2g1GtWa2G/ecEmBdgz31+Mxr+ELclgg6sokpSFIQiZ02Q==} + fastest-levenshtein@1.0.16: + resolution: {integrity: sha512-eRnCtTTtGZFpQCwhJiUOuxPQWRXVKYDn0b2PeHfXL6/Zi53SLAzAHfVhVWK2AryC/WH05kGfxhFIPvTF0SXQzg==} + engines: {node: '>= 4.9.1'} + fdir@6.5.0: resolution: {integrity: sha512-tIbYtZbucOs0BRGqPJkshJUYdL+SDH7dVM8gjy+ERp3WAUjLEFJE+02kanyHtwjWOnwrKYBiwAmM0p4kLJAnXg==} engines: {node: '>=12.0.0'} @@ -1425,6 +1432,8 @@ snapshots: dependencies: fast-string-width: 3.0.2 + fastest-levenshtein@1.0.16: {} + fdir@6.5.0(picomatch@4.0.4): optionalDependencies: picomatch: 4.0.4 diff --git a/src/args.ts b/src/args.ts index 3fd56b9..d176fa0 100644 --- a/src/args.ts +++ b/src/args.ts @@ -1,5 +1,7 @@ import { Command, CommanderError, Option } from 'commander'; +import { distance } from 'fastest-levenshtein'; +import { usageError } from './cli-error'; import { CLI_OPERATIONS } from './generated/cli-manifest'; export type FlagValue = boolean | string; @@ -11,28 +13,76 @@ export interface ParsedArgs { type ValueRequirement = 'optional' | 'required'; -interface CliOptionDefinition { +interface ParseableOption { aliases: string[]; valueRequirement: ValueRequirement; } +interface CliOptionDefinition extends ParseableOption { + // Whether this global is meaningful on generated API commands. Auth-flow-only + // flags (--flow, --scope, --with-token, --timeout, --all, --force, --open) + // must stay globally *parseable* so the bespoke auth/logout handlers can read + // them, but they are not honored by the API request path — accepting them + // there would silently drop the flag and mislead the caller. See + // apiGlobalOptionAliases(). + onApiCommands: boolean; +} + const GLOBAL_OPTIONS: CliOptionDefinition[] = [ - { aliases: ['token'], valueRequirement: 'required' }, - { aliases: ['base-url'], valueRequirement: 'required' }, - { aliases: ['body-json'], valueRequirement: 'required' }, - { aliases: ['json'], valueRequirement: 'optional' }, - { aliases: ['yes'], valueRequirement: 'optional' }, - { aliases: ['open'], valueRequirement: 'optional' }, - { aliases: ['flow'], valueRequirement: 'required' }, - { aliases: ['scope'], valueRequirement: 'required' }, - { aliases: ['profile'], valueRequirement: 'required' }, - { aliases: ['env'], valueRequirement: 'required' }, - { aliases: ['with-token'], valueRequirement: 'optional' }, - { aliases: ['all'], valueRequirement: 'optional' }, - { aliases: ['help', 'h'], valueRequirement: 'optional' }, - { aliases: ['version', 'v'], valueRequirement: 'optional' }, + { aliases: ['token'], valueRequirement: 'required', onApiCommands: true }, + { aliases: ['base-url'], valueRequirement: 'required', onApiCommands: true }, + { aliases: ['body-json'], valueRequirement: 'required', onApiCommands: true }, + { aliases: ['json'], valueRequirement: 'optional', onApiCommands: true }, + { aliases: ['yes'], valueRequirement: 'optional', onApiCommands: true }, + { aliases: ['open'], valueRequirement: 'optional', onApiCommands: false }, + { aliases: ['flow'], valueRequirement: 'required', onApiCommands: false }, + { aliases: ['scope'], valueRequirement: 'required', onApiCommands: false }, + { aliases: ['profile'], valueRequirement: 'required', onApiCommands: true }, + { aliases: ['env'], valueRequirement: 'required', onApiCommands: true }, + { aliases: ['region'], valueRequirement: 'required', onApiCommands: true }, + { aliases: ['timeout'], valueRequirement: 'required', onApiCommands: false }, + { + aliases: ['with-token'], + valueRequirement: 'optional', + onApiCommands: false, + }, + { aliases: ['all'], valueRequirement: 'optional', onApiCommands: false }, + { aliases: ['force'], valueRequirement: 'optional', onApiCommands: false }, + { aliases: ['dry-run'], valueRequirement: 'optional', onApiCommands: true }, + { aliases: ['help', 'h'], valueRequirement: 'optional', onApiCommands: true }, + { + aliases: ['version', 'v'], + valueRequirement: 'optional', + onApiCommands: true, + }, ]; +// `--dry-run` is defined per-operation in the manifest (only DELETE +// operations declare a `dry_run` parameter), but `run.ts`'s delete gate reads +// it unconditionally on every DELETE command — including ones that don't +// support it, to produce a specific "does not support --dry-run" error. It +// must stay globally parseable so that check can run before flag validation. + +/** Every flag alias accepted on any command, regardless of the resolved operation. */ +export function globalOptionAliases(): string[] { + return [...new Set(GLOBAL_OPTIONS.flatMap((option) => option.aliases))]; +} + +/** + * Global aliases meaningful on generated API commands. Excludes the + * auth-flow-only globals, so `amp projects list --timeout 5` is rejected as an + * unknown flag instead of silently accepted-and-dropped. + */ +export function apiGlobalOptionAliases(): string[] { + return [ + ...new Set( + GLOBAL_OPTIONS.filter((option) => option.onApiCommands).flatMap( + (option) => option.aliases, + ), + ), + ]; +} + function attributeName(alias: string): string { return alias.replace(/-([a-z])/g, (_, letter: string) => letter.toUpperCase(), @@ -54,7 +104,7 @@ function defineOption( program.addOption(new Option(flags).hideHelp()); } -function optionDefinitions(): CliOptionDefinition[] { +function optionDefinitions(): ParseableOption[] { const byAlias = new Map(); for (const option of GLOBAL_OPTIONS) { @@ -105,6 +155,52 @@ function createParser(): Command { return program; } +// Relative similarity (not a raw edit-distance cutoff) so short aliases like +// --yes or --all don't become coincidental "did you mean" matches for an +// unrelated short flag — mirrors commander's own suggestSimilar heuristic. +const MIN_SUGGESTION_SIMILARITY = 0.4; + +/** Closest candidate to `flag` by relative similarity, for "did you mean" hints. */ +export function nearestAlias( + flag: string, + candidates: Iterable, +): string | undefined { + let best: string | undefined; + let bestSimilarity = MIN_SUGGESTION_SIMILARITY; + + for (const candidate of candidates) { + if (candidate.length <= 1) { + continue; + } + + const dist = distance(flag, candidate); + const length = Math.max(flag.length, candidate.length); + const similarity = (length - dist) / length; + if (similarity > bestSimilarity) { + bestSimilarity = similarity; + best = candidate; + } + } + + return best; +} + +function withUnknownOptionSuggestion(error: CommanderError): string { + const message = error.message.replace(/^error: /, ''); + const match = + error.code === 'commander.unknownOption' + ? /^unknown option '(-[^']+)'/.exec(message) + : null; + + if (!match) { + return message; + } + + const knownAliases = optionDefinitions().map((option) => option.aliases[0]); + const suggestion = nearestAlias(match[1].replace(/^--?/, ''), knownAliases); + return suggestion ? `${message} Did you mean --${suggestion}?` : message; +} + export function parseArgs(argv: string[]): ParsedArgs { const flags: Record = {}; const program = createParser(); @@ -114,7 +210,7 @@ export function parseArgs(argv: string[]): ParsedArgs { program.parse(normalizedArgv, { from: 'user' }); } catch (error) { if (error instanceof CommanderError) { - throw new Error(error.message.replace(/^error: /, '')); + throw usageError(withUnknownOptionSuggestion(error)); } throw error; } @@ -152,7 +248,7 @@ export function stringFlag( ): string | undefined { const value = flagValue(flags, aliases); if (value === true) { - throw new Error(`Expected --${aliases[0]} to have a value.`); + throw usageError(`Expected --${aliases[0]} to have a value.`); } return typeof value === 'string' ? value : undefined; } diff --git a/src/auth-commands.test.ts b/src/auth-commands.test.ts new file mode 100644 index 0000000..545ea7b --- /dev/null +++ b/src/auth-commands.test.ts @@ -0,0 +1,967 @@ +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { + resolvePollProfile, + runAuthLoginPoll, + runAuthLoginStart, + targetProfileName, +} from './auth-commands'; +import { + CURRENT_VERSION, + type CredentialStore, + getProfile, + loadStore, + saveStore, + setProfile, +} from './credential-store'; +import { + emptyPending, + getPending, + loadPending, + savePending, + setPending, +} from './pending-store'; + +const profile = { + base_url: 'https://developer-api.amplitude.com', + credential: { type: 'pat' as const, pat: 'amp_x' }, + saved_at: '2026-06-23T12:00:00.000Z', +}; + +function storeWith(over: Partial = {}): CredentialStore { + return { version: CURRENT_VERSION, profiles: {}, ...over }; +} + +describe('targetProfileName', () => { + it('returns an explicit --profile after validating it', () => { + expect(targetProfileName(storeWith(), 'prod')).toBe('prod'); + }); + + it('rejects an invalid explicit --profile', () => { + expect(() => targetProfileName(storeWith(), 'bad/name')).toThrow( + /Invalid profile name/, + ); + }); + + it('falls back to the active pointer when --profile is omitted', () => { + const store = storeWith({ default: 'prod', profiles: { prod: profile } }); + expect(targetProfileName(store, undefined)).toBe('prod'); + }); + + it('falls back to "default" on a cold store (unset pointer)', () => { + expect(targetProfileName(storeWith(), undefined)).toBe('default'); + }); + + it('throws for an orphaned pointer (set but missing profile)', () => { + const store = storeWith({ default: 'ghost' }); + expect(() => targetProfileName(store, undefined)).toThrow( + /No such profile: ghost/, + ); + }); +}); + +function paths() { + const dir = mkdtempSync(join(tmpdir(), 'amp-start-')); + return { + path: join(dir, 'credentials.json'), + pendingPath: join(dir, 'pending.json'), + }; +} + +describe('runAuthLoginStart', () => { + it('persists a pending entry and emits verification_required without secrets', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async (method, p) => { + expect(method).toBe('POST'); + expect(p).toBe('/v1/auth/device-authorization'); + return { + status: 200, + body: { + device_code: 'DC', + user_code: 'HZNK-QLIB', + verification_uri: 'https://app.amplitude.com/device', + verification_uri_complete: + 'https://app.amplitude.com/device?user_code=HZNK-QLIB', + expires_in: 600, + interval: 5, + }, + }; + }, + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('verification_required'); + expect(out.data.user_code).toBe('HZNK-QLIB'); + expect(out.data.profile).toBe('default'); + expect(out.data.region).toBe('us'); + // Proactively tells the agent the poll blocks (default) and is tunable. + expect(out.message).toMatch(/blocks up to ~25s by default/); + expect(out.message).toMatch(/--timeout/); + expect(lines.join('\n')).not.toContain('DC'); // device_code withheld + expect(getPending(loadPending(pendingPath), 'default')?.device_code).toBe( + 'DC', + ); + }); + + it('surfaces the server OAuth error on a non-2xx device-authorization response', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async () => ({ + status: 400, + body: { + error: 'invalid_request', + error_description: 'Unknown scope requested.', + }, + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('invalid_request'); + expect(out.error.detail).toBe('Unknown scope requested.'); + expect(out.message).toContain('Unknown scope requested.'); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + }); + + it('garbage-collects expired pending entries on start', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending(emptyPending(), 'old', pendingEntry('2000-01-01T00:00:00Z')), + pendingPath, + ); + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => Date.parse('2026-07-15T00:00:00.000Z'), + deviceId: () => 'dev', + stdout: () => {}, + request: async () => ({ + status: 200, + body: { + device_code: 'DC', + user_code: 'UC', + verification_uri: 'https://app.amplitude.com/device', + verification_uri_complete: + 'https://app.amplitude.com/device?user_code=UC', + expires_in: 600, + interval: 5, + }, + }), + }, + ); + expect(Object.keys(loadPending(pendingPath).pending)).toEqual(['default']); + }); + + it('mints a fresh code and notes it supersedes a still-live prior one', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async () => ({ + status: 200, + body: { + device_code: 'DC2', + user_code: 'NEW-CODE', + verification_uri: 'https://app.amplitude.com/device', + verification_uri_complete: + 'https://app.amplitude.com/device?user_code=NEW-CODE', + expires_in: 600, + interval: 5, + }, + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('verification_required'); + expect(out.message).toMatch(/replaces an earlier in-progress code/); + expect(getPending(loadPending(pendingPath), 'default')?.device_code).toBe( + 'DC2', + ); + }); + + it('refuses to silently retarget an existing profile to a different region', async () => { + const { path, pendingPath } = paths(); + const store: CredentialStore = storeWith({ + default: 'default', + profiles: { + default: { + base_url: 'https://developer-api.eu.amplitude.com', + credential: { type: 'pat', pat: 'amp_x' }, + saved_at: '2026-06-23T12:00:00.000Z', + }, + }, + }); + saveStore(store, path); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async () => { + throw new Error('request should not be called'); + }, + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + // Same usage-error contract as `auth pat`'s retarget refusal and start's + // other invocation errors: usage_error / exit 2. + expect(out.error.error_code).toBe('usage_error'); + expect(out.message).toContain('https://developer-api.eu.amplitude.com'); + expect(process.exitCode).toBe(2); + process.exitCode = 0; + expect(Object.keys(loadPending(pendingPath).pending)).toEqual([]); + }); + + it('proceeds with a cross-region retarget when --force is set', async () => { + const { path, pendingPath } = paths(); + const store: CredentialStore = storeWith({ + default: 'default', + profiles: { + default: { + base_url: 'https://developer-api.eu.amplitude.com', + credential: { type: 'pat', pat: 'amp_x' }, + saved_at: '2026-06-23T12:00:00.000Z', + }, + }, + }); + saveStore(store, path); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us', force: true }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async () => ({ + status: 200, + body: { + device_code: 'DC', + user_code: 'UC', + verification_uri: 'https://app.amplitude.com/device', + verification_uri_complete: + 'https://app.amplitude.com/device?user_code=UC', + expires_in: 600, + interval: 5, + }, + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('verification_required'); + }); + + it('pretty-prints the JSON envelope at a TTY', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + isTTY: true, + stdout: (l) => lines.push(l), + request: async () => ({ + status: 200, + body: { + device_code: 'DC', + user_code: 'UC', + verification_uri: 'https://app.amplitude.com/device', + verification_uri_complete: + 'https://app.amplitude.com/device?user_code=UC', + expires_in: 600, + interval: 5, + }, + }), + }, + ); + const out = lines.join('\n'); + expect(out).toContain('\n "status"'); + }); + + it('emits a clean unexpected_response envelope on a malformed 2xx device-authorization body', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + await runAuthLoginStart( + { region: 'us' }, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async () => ({ + status: 200, + body: {}, // missing device_code/user_code/verification_uri/expires_in + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('unexpected_response'); + expect(out.message).toBe( + 'The authorization server returned an unexpected device-authorization response.', + ); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + expect(getPending(loadPending(pendingPath), 'default')).toBeUndefined(); + }); + + it('emits a JSON error envelope (not a throw) on invalid input', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + // No --region on a cold store → loginBaseUrl throws a usage error; it must + // surface as a JSON envelope that keeps the usage_error/exit-2 + // classification, not get flattened to start_failed/exit-1. + await runAuthLoginStart( + {}, + { + path, + pendingPath, + now: () => 0, + deviceId: () => 'dev', + stdout: (l) => lines.push(l), + request: async () => ({ status: 200, body: {} }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('usage_error'); + expect(process.exitCode).toBe(2); + process.exitCode = 0; + }); +}); + +const pendingEntry = (expiresAt: string) => ({ + device_code: 'DC', + code_verifier: 'CV', + base_url: 'https://developer-api.amplitude.com', + expires_at: expiresAt, + interval: 5, + started_at: '2026-07-15T00:00:00.000Z', +}); + +describe('resolvePollProfile', () => { + it('auto-selects the sole pending login when --profile is omitted', () => { + const pending = setPending( + emptyPending(), + 'work', + pendingEntry('2999-01-01T00:00:00Z'), + ); + expect( + resolvePollProfile(pending, undefined, undefined, Date.now()), + ).toEqual({ + name: 'work', + }); + }); + + it('falls back to the store default when multiple logins are pending', () => { + const pending = setPending( + setPending(emptyPending(), 'work', pendingEntry('2999-01-01T00:00:00Z')), + 'personal', + pendingEntry('2999-01-01T00:00:00Z'), + ); + expect( + resolvePollProfile(pending, undefined, 'personal', Date.now()), + ).toEqual({ + name: 'personal', + }); + }); + + it('errors listing profiles when multiple logins are pending and none disambiguates', () => { + const pending = setPending( + setPending(emptyPending(), 'work', pendingEntry('2999-01-01T00:00:00Z')), + 'personal', + pendingEntry('2999-01-01T00:00:00Z'), + ); + const result = resolvePollProfile( + pending, + undefined, + undefined, + Date.now(), + ); + expect('error' in result && result.error).toMatch(/work/); + expect('error' in result && result.error).toMatch(/personal/); + expect('error' in result && result.code).toBe('ambiguous_pending_login'); + }); + + it('does not guess "default" over another live login when the active pointer is elsewhere', () => { + const pending = setPending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + 'work', + pendingEntry('2999-01-01T00:00:00Z'), + ); + const result = resolvePollProfile(pending, undefined, 'prod', Date.now()); + expect('error' in result && result.code).toBe('ambiguous_pending_login'); + }); + + it('ignores expired entries so a stale sibling does not shadow the live login', () => { + const pending = setPending( + setPending(emptyPending(), 'stale', pendingEntry('2000-01-01T00:00:00Z')), + 'live', + pendingEntry('2999-01-01T00:00:00Z'), + ); + expect( + resolvePollProfile(pending, undefined, undefined, Date.now()), + ).toEqual({ + name: 'live', + }); + }); + + it('reports no login in progress when every pending entry is expired', () => { + const pending = setPending( + setPending(emptyPending(), 'a', pendingEntry('2000-01-01T00:00:00Z')), + 'b', + pendingEntry('2000-01-01T00:00:00Z'), + ); + const result = resolvePollProfile( + pending, + undefined, + undefined, + Date.now(), + ); + expect('error' in result && result.error).toMatch(/No login in progress/); + expect('error' in result && result.code).toBe('no_pending_login'); + }); +}); + +describe('runAuthLoginPoll', () => { + it('authorizes: writes+activates the profile and clears pending', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => 0, + stdout: (l) => lines.push(l), + request: async () => ({ + status: 200, + body: { access_token: 'AT', token_type: 'bearer', expires_in: 3600 }, + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('authorized'); + expect(out.data.profile).toBe('default'); + expect(out.data.region).toBe('us'); + const store = loadStore(path); + expect(store.default).toBe('default'); + expect(getProfile(store, 'default')?.credential.type).toBe('oauth'); + expect(getPending(loadPending(pendingPath), 'default')).toBeUndefined(); + }); + + it('authorizes with region derived from an EU pending base_url', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending(emptyPending(), 'default', { + ...pendingEntry('2999-01-01T00:00:00Z'), + base_url: 'https://developer-api.eu.amplitude.com', + }), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => 0, + stdout: (l) => lines.push(l), + request: async () => ({ + status: 200, + body: { access_token: 'AT', token_type: 'bearer', expires_in: 3600 }, + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('authorized'); + expect(out.data.region).toBe('eu'); + }); + + it('reports pending (exit 75) when not yet confirmed', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + process.exitCode = 0; + await runAuthLoginPoll( + { profile: 'default', timeout: '0' }, + { + path, + pendingPath, + now: () => 0, + sleep: async () => {}, + stdout: (l) => lines.push(l), + request: async () => ({ + status: 400, + body: { error: 'authorization_pending' }, + }), + }, + ); + expect(JSON.parse(lines.join('\n')).status).toBe('pending'); + expect(process.exitCode).toBe(75); + process.exitCode = 0; + }); + + it('errors when there is no pending login', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default' }, + { path, pendingPath, now: () => 0, stdout: (l) => lines.push(l) }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('no_pending_login'); + process.exitCode = 0; + }); + + it('surfaces ambiguous_pending_login when multiple logins are pending and none disambiguates', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + setPending( + emptyPending(), + 'work', + pendingEntry('2999-01-01T00:00:00Z'), + ), + 'personal', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + {}, + { path, pendingPath, now: () => 0, stdout: (l) => lines.push(l) }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('ambiguous_pending_login'); + process.exitCode = 0; + }); + + it('names --region eu in the restart hint for an EU pending entry', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending(emptyPending(), 'default', { + ...pendingEntry('2000-01-01T00:00:00Z'), // already expired + base_url: 'https://developer-api.eu.amplitude.com', + }), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => Date.now(), + stdout: (l) => lines.push(l), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('expired'); + expect(out.message).toContain('--region eu'); + expect(out.message).not.toContain('--region us'); + process.exitCode = 0; + }); + + it('surfaces the server error_code on an OAuth-error outcome (access_denied)', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => 0, + stdout: (l) => lines.push(l), + request: async () => ({ + status: 400, + body: { + error: 'access_denied', + error_description: 'The user denied the authorization request.', + }, + }), + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('access_denied'); + expect(out.error.detail).toBe('The user denied the authorization request.'); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + }); + + it('rejects a non-numeric --timeout with an invalid_timeout envelope', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default', timeout: 'abc' }, + { path, pendingPath, now: () => 0, stdout: (l) => lines.push(l) }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('invalid_timeout'); + expect(process.exitCode).toBe(2); + process.exitCode = 0; + }); + + it('preserves a thrown usage error (bare --profile) as usage_error/exit 2, not poll_failed', async () => { + const { path, pendingPath } = paths(); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: true }, + { path, pendingPath, now: () => 0, stdout: (l) => lines.push(l) }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('usage_error'); + expect(process.exitCode).toBe(2); + process.exitCode = 0; + }); + + it('keeps the pending entry on a transient server_error (retryable)', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + let t = 0; + const lines: string[] = []; + process.exitCode = 0; + await runAuthLoginPoll( + { profile: 'default', timeout: '2' }, + { + path, + pendingPath, + now: () => (t += 1000), + sleep: async () => {}, + stdout: (l) => lines.push(l), + request: async () => ({ status: 500, body: { error: 'server_error' } }), + }, + ); + expect(JSON.parse(lines.join('\n')).status).toBe('pending'); + expect(getPending(loadPending(pendingPath), 'default')).toBeDefined(); + expect(process.exitCode).toBe(75); + process.exitCode = 0; + }); + + it('notes the pending login is preserved and hints a retry on poll_failed', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + const lines: string[] = []; + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => 0, + stdout: (l) => lines.push(l), + request: async () => { + throw new Error('Could not reach the API'); + }, + }, + ); + const out = JSON.parse(lines.join('\n')); + expect(out.status).toBe('error'); + expect(out.error.error_code).toBe('poll_failed'); + expect(out.message).toMatch(/re-run the poll command to retry/); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + expect(getPending(loadPending(pendingPath), 'default')).toBeDefined(); + }); + + it('does not apply a slow_down interval to a concurrently-started fresh code', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + let t = 0; + process.exitCode = 0; + await runAuthLoginPoll( + { profile: 'default', timeout: '5' }, + { + path, + pendingPath, + now: () => (t += 1000), + sleep: async () => {}, + stdout: () => {}, + request: async () => { + // A concurrent `start` overwrites the entry with a fresh device_code + // (interval 5) while this poll's request is in flight; the slow_down + // below was raised against the old code and must not touch the fresh row. + savePending( + setPending(loadPending(pendingPath), 'default', { + ...pendingEntry('2999-01-01T00:00:00Z'), + device_code: 'FRESH', + }), + pendingPath, + ); + return { status: 400, body: { error: 'slow_down' } }; + }, + }, + ); + const after = getPending(loadPending(pendingPath), 'default'); + expect(after?.device_code).toBe('FRESH'); + expect(after?.interval).toBe(5); + process.exitCode = 0; + }); + + it('does not clear a pending entry a concurrent start superseded when erroring', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + process.exitCode = 0; + await runAuthLoginPoll( + { profile: 'default', timeout: '0' }, + { + path, + pendingPath, + now: () => 0, + sleep: async () => {}, + stdout: () => {}, + request: async () => { + // A concurrent `start` mints a fresh code mid-flight; this slower + // poll then errors and must not wipe the newer in-progress login. + savePending( + setPending(loadPending(pendingPath), 'default', { + ...pendingEntry('2999-01-01T00:00:00Z'), + device_code: 'FRESH', + }), + pendingPath, + ); + return { status: 400, body: { error: 'access_denied' } }; + }, + }, + ); + expect(getPending(loadPending(pendingPath), 'default')?.device_code).toBe( + 'FRESH', + ); + process.exitCode = 0; + }); + + it('clears the pending entry on authorize even if a concurrent start superseded it', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => 0, + stdout: () => {}, + request: async () => { + // A concurrent `start` mints a fresh code mid-flight; this poll then + // authorizes. The profile is now authenticated, so the leftover code + // must be cleared, not left to make later polls report `pending`. + savePending( + setPending(loadPending(pendingPath), 'default', { + ...pendingEntry('2999-01-01T00:00:00Z'), + device_code: 'FRESH', + }), + pendingPath, + ); + return { + status: 200, + body: { + access_token: 'AT', + token_type: 'bearer', + expires_in: 3600, + }, + }; + }, + }, + ); + expect(getPending(loadPending(pendingPath), 'default')).toBeUndefined(); + }); + + it('does not clobber a profile a concurrent auth wrote to the store mid-poll', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + await runAuthLoginPoll( + { profile: 'default' }, + { + path, + pendingPath, + now: () => 0, + stdout: () => {}, + request: async () => { + // A concurrent auth (e.g. another profile's poll, or an interactive + // login) writes to the SAME store file while this poll is in + // flight. The authorized save below must layer onto this, not a + // stale pre-poll snapshot. + saveStore( + setProfile(loadStore(path), 'other', { + base_url: 'https://concurrent.example.com', + credential: { type: 'pat', pat: 'amp_other' }, + saved_at: '2026-06-23T12:00:00.000Z', + store: 'file', + }), + path, + ); + return { + status: 200, + body: { + access_token: 'AT', + token_type: 'bearer', + expires_in: 3600, + }, + }; + }, + }, + ); + expect(getProfile(loadStore(path), 'default')).toBeDefined(); + const other = getProfile(loadStore(path), 'other'); + expect(other).toBeDefined(); + expect(other?.base_url).toBe('https://concurrent.example.com'); + }); + + it('persists a slow_down-raised interval for the next poll', async () => { + const { path, pendingPath } = paths(); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('2999-01-01T00:00:00Z'), + ), + pendingPath, + ); + let t = 0; + process.exitCode = 0; + await runAuthLoginPoll( + { profile: 'default', timeout: '2' }, + { + path, + pendingPath, + now: () => (t += 1000), + sleep: async () => {}, + stdout: () => {}, + request: async () => ({ status: 400, body: { error: 'slow_down' } }), + }, + ); + expect( + getPending(loadPending(pendingPath), 'default')?.interval, + ).toBeGreaterThan(5); + process.exitCode = 0; + }); +}); diff --git a/src/auth-commands.ts b/src/auth-commands.ts index 225141b..26be76d 100644 --- a/src/auth-commands.ts +++ b/src/auth-commands.ts @@ -2,13 +2,27 @@ import { text } from 'node:stream/consumers'; import { type FlagValue, isFlagEnabled, stringFlag } from './args'; -import { personalAccessTokenSetupUrl, resolveOrgUrl } from './auth-guidance'; import { + personalAccessTokenSetupUrl, + resolveAppOrigin, + resolveOrgUrl, +} from './auth-guidance'; +import { + type AnonymousRequest, type DeviceFlowOptions, createAnonymousRequest, + generatePkcePair, + pollDeviceTokenBounded, requestDeviceToken, } from './authToken'; -import { ENV_BASE_URLS, resolveEnvBaseUrl } from './config'; +import { CliError, usageError } from './cli-error'; +import { getOrCreateDeviceId } from './cli-state'; +import { + assertRegionAndEnvNotBothSet, + DEFAULT_POLL_TIMEOUT_SECONDS, + ENV_BASE_URLS, + resolveNamedBaseUrl, +} from './config'; import { resolveAuthFromFlags, selectProfileFromFlags, @@ -29,7 +43,25 @@ import { setDefault, setProfile, } from './credential-store'; -import type { TokenResponse } from './oauthResponseSchemas'; +import { toOAuthError } from './oauthError'; +import { + deviceAuthorizationResponseSchema, + type TokenResponse, +} from './oauthResponseSchemas'; +import { formatJsonOutput, shouldUseJsonOutput } from './output'; +import { + emptyPending, + gcExpiredPending, + getPending, + isPendingExpired, + loadPending, + type PendingEntry, + type PendingStore, + pendingPath as defaultPendingPath, + removePending, + savePending, + setPending, +} from './pending-store'; import { askSecret, confirm } from './prompt'; import { DEFAULT_SCOPES } from './scopes'; import { terminal, terminalForStdout } from './terminal'; @@ -73,26 +105,583 @@ export function oauthCredentialFromToken( export function loginBaseUrl(args: { baseUrlFlag?: string; envFlag?: string; + regionFlag?: string; existing?: Profile; }): string { + assertRegionAndEnvNotBothSet(args); if (args.baseUrlFlag) { return args.baseUrlFlag.replace(/\/$/, ''); } - if (args.envFlag) { - return resolveEnvBaseUrl(args.envFlag); + const named = resolveNamedBaseUrl({ + envFlag: args.envFlag, + regionFlag: args.regionFlag, + }); + if (named) { + return named; } if (args.existing) { return args.existing.base_url; } - throw new Error( - 'Creating a profile requires --env or --base-url .', + throw usageError('Creating a profile requires --region .'); +} + +/** + * The profile name a create-or-reauth verb targets: an explicit `--profile`, + * else the active pointer (`store.default`), else the implicit `default` + * (materialized on first login). Throws when relying on a *set* pointer whose + * profile is gone (an orphaned hand-edited default) so the caller matches the + * resolver's "No such profile" wording rather than falling into loginBaseUrl's + * create-time "requires --region". A cold store (unset pointer) falls through to + * `default` and create-mode. + */ +export function targetProfileName( + store: CredentialStore, + requestedName: string | undefined, +): string { + if (requestedName !== undefined) { + assertValidProfileName(requestedName); + return requestedName; + } + if (store.default) { + if (!getProfile(store, store.default)) { + throw usageError( + `No such profile: ${store.default}. Run \`amp auth list\`.`, + ); + } + return store.default; + } + return 'default'; +} + +/** + * Wraps a phase response in the `{ status, message, data | error }` envelope + * every agent-driven auth verb emits. `status` is always a string enum; + * secrets (`device_code`, `code_verifier`) must never be passed in `data`. + */ +export function authFlowJson( + payload: { + status: + | 'verification_required' + | 'pending' + | 'authorized' + | 'expired' + | 'error'; + message: string; + data?: Record; + error?: Record; + }, + isTTY: boolean, +): string { + return isTTY ? JSON.stringify(payload, null, 2) : JSON.stringify(payload); +} + +export interface AuthStartDeps { + path?: string; + pendingPath?: string; + now?: () => number; + deviceId?: () => string; + stdout?: (line: string) => void; + request?: AnonymousRequest; + isTTY?: boolean; +} + +/** + * `amp auth login start` — device-flow phase 1. Requests a device + * authorization, stashes the secret `device_code`/`code_verifier` in the + * pending-logins store (never emitted), and prints the JSON envelope an agent + * relays to the human: the `user_code` to confirm and the poll command to run + * next. + */ +export async function runAuthLoginStart( + flags: Record, + deps: AuthStartDeps = {}, +): Promise { + const now = deps.now ?? (() => Date.now()); + const emit = deps.stdout ?? ((line) => console.log(line)); + const pPath = deps.pendingPath ?? defaultPendingPath(); + const isTTY = deps.isTTY ?? Boolean(process.stdout.isTTY); + const toJson = (payload: Parameters[0]): string => + authFlowJson(payload, isTTY); + + // Machine verb: any validation/parse failure must still be a JSON envelope, + // never a thrown prose error to stderr. + try { + const store = loadStore(deps.path); + const profileName = targetProfileName( + store, + stringFlag(flags, ['profile']), + ); + const existing = getProfile(store, profileName); + const baseUrl = loginBaseUrl({ + baseUrlFlag: stringFlag(flags, ['base-url']), + envFlag: stringFlag(flags, ['env']), + regionFlag: stringFlag(flags, ['region']), + existing, + }); + + if ( + existing && + existing.base_url !== baseUrl && + !isFlagEnabled(flags.force) + ) { + // A retarget refusal is a usage error, like a missing --region — throw so + // the catch emits usage_error/exit 2, matching `auth pat` instead of a + // bespoke profile_target_conflict/exit 1. + throw usageError( + `Profile "${profileName}" targets ${existing.base_url}; refusing to silently retarget it to ${baseUrl}. Use a different --profile, or pass --force to overwrite.`, + ); + } + + const request = + deps.request ?? + createAnonymousRequest(baseUrl, (deps.deviceId ?? getOrCreateDeviceId)()); + const { codeVerifier, codeChallenge } = generatePkcePair(); + const response = await request('POST', '/v1/auth/device-authorization', { + code_challenge: codeChallenge, + code_challenge_method: 'S256', + scope: DEFAULT_SCOPES, + }); + if (response.status < 200 || response.status >= 300) { + const oauthError = toOAuthError(response.body); + const detail = oauthError.error_description ?? oauthError.error_hint; + emit( + toJson({ + status: 'error', + message: `Could not start device authorization: ${detail ?? oauthError.error}.`, + error: { + error_code: oauthError.error, + detail: detail ?? undefined, + status: response.status, + }, + }), + ); + process.exitCode = 1; + return; + } + const parsed = deviceAuthorizationResponseSchema.safeParse(response.body); + if (!parsed.success) { + emit( + toJson({ + status: 'error', + message: + 'The authorization server returned an unexpected device-authorization response.', + error: { error_code: 'unexpected_response' }, + }), + ); + process.exitCode = 1; + return; + } + const device = parsed.data; + const expiresAt = new Date(now() + device.expires_in * 1000).toISOString(); + const entry: PendingEntry = { + device_code: device.device_code, + code_verifier: codeVerifier, + base_url: baseUrl, + expires_at: expiresAt, + interval: device.interval ?? 5, + started_at: new Date(now()).toISOString(), + }; + // Every start mints a fresh code; we never resume (a stale or errored code + // must never trap the user). Opportunistically GC expired entries on the way + // in, so abandoned logins don't accumulate. After GC, any remaining entry for + // this profile is a still-live code being overwritten — flag it so the agent + // steers the human to the new code, not one they may already have open. + const pending = gcExpiredPending(loadPending(pPath), now()); + const supersededPriorLogin = pending.pending[profileName] !== undefined; + savePending(setPending(pending, profileName, entry), pPath); + + const verificationUrl = + device.verification_uri_complete ?? device.verification_uri; + const pollCmd = `amp auth login poll --profile ${profileName} --json`; + const supersedeNote = supersededPriorLogin + ? ' This replaces an earlier in-progress code for this profile — have the user use this one.' + : ''; + emit( + toJson({ + status: 'verification_required', + message: `Ask the user to open ${verificationUrl} and confirm code ${device.user_code}, then run \`${pollCmd}\` until it reports authorized. Each poll blocks up to ~${DEFAULT_POLL_TIMEOUT_SECONDS}s by default (pass \`--timeout \`, or \`--timeout 0\` for a single check).${supersedeNote}`, + data: { + user_code: device.user_code, + verification_uri: device.verification_uri, + verification_uri_complete: verificationUrl, + expires_at: expiresAt, + expires_in_seconds: device.expires_in, + region: regionLabelForBaseUrl(baseUrl)?.toLowerCase(), + profile: profileName, + scopes: DEFAULT_SCOPES.split(' '), + }, + }), + ); + } catch (error) { + // A thrown CliError (e.g. loginBaseUrl's "requires --region", a usage + // error) already classifies itself — carry its code and exit code into + // the envelope instead of flattening every failure to start_failed/1, so + // the same mistake exits 2 here as it does on `auth pat`. + if (error instanceof CliError) { + emit( + toJson({ + status: 'error', + message: error.message, + error: { error_code: error.errorCode, detail: error.detail }, + }), + ); + process.exitCode = error.exitCode; + return; + } + emit( + toJson({ + status: 'error', + message: error instanceof Error ? error.message : String(error), + error: { + error_code: 'start_failed', + detail: error instanceof Error ? error.message : undefined, + }, + }), + ); + process.exitCode = 1; + } +} + +/** + * Resolves which profile `amp auth login poll` targets: an explicit + * `--profile`, else the sole in-flight pending login, else the store's active + * default (when it has a pending entry). Returns an error when there's nothing + * to poll, or when more than one pending login exists and none of the above + * disambiguates it (we never guess a profile out of several live logins). + */ +export function resolvePollProfile( + pending: PendingStore, + explicit: string | undefined, + storeDefault: string | undefined, + now: number, +): + | { name: string } + | { + error: string; + code: 'no_pending_login' | 'ambiguous_pending_login'; + } { + if (explicit) { + return { name: explicit }; + } + // Only non-expired entries are in-flight. Expired ones are removed lazily + // (when poll targets them), so a leftover stale entry must not shadow the live + // login or manufacture false ambiguity when --profile is omitted. + const live = Object.keys(pending.pending).filter( + (name) => !isPendingExpired(pending.pending[name], now), ); + if (live.length === 1) { + return { name: live[0] }; + } + if (storeDefault && live.includes(storeDefault)) { + return { name: storeDefault }; + } + if (live.length === 0) { + return { + error: `No login in progress. Start one with \`${loginStartRestartHint()}\`.`, + code: 'no_pending_login', + }; + } + return { + error: `Ambiguous: pending logins for ${live.join(', ')}. Pass --profile .`, + code: 'ambiguous_pending_login', + }; +} + +/** + * The `amp auth login start` command to suggest in a restart hint. Derives + * `--region ` from a pending entry's `base_url` when known (via + * `regionLabelForBaseUrl`); falls back to the generic `` placeholder + * when there's no entry to derive from, or its base_url isn't a recognized + * region (e.g. an internal/dev host). + */ +function loginStartRestartHint(baseUrl?: string): string { + const region = baseUrl && regionLabelForBaseUrl(baseUrl)?.toLowerCase(); + return `amp auth login start --region ${region ?? ''} --json`; +} + +export interface AuthPollDeps extends AuthStartDeps { + sleep?: (seconds: number) => Promise; +} + +/** + * Parses `--timeout` into whole seconds: omitted falls back to the default, + * an explicit value must be a finite non-negative integer (0 means single-shot). + * Returns `undefined` on invalid input rather than throwing, so the caller can + * emit an error envelope instead of crashing the process. + */ +function parseTimeoutSeconds(raw: string | undefined): number | undefined { + if (raw === undefined) { + return DEFAULT_POLL_TIMEOUT_SECONDS; + } + const parsed = Number(raw); + if (!Number.isFinite(parsed) || !Number.isInteger(parsed) || parsed < 0) { + return undefined; + } + return parsed; +} + +/** + * `amp auth login poll` — device-flow phase 2. Resolves the in-flight + * profile (see `resolvePollProfile`), then makes one bounded poll attempt + * against `/v1/auth/token`: `authorized` saves + activates the profile and + * clears the pending entry; `pending` reports back (exit 75) for the agent to + * retry; `expired`/`error` clear the pending entry and exit non-zero. Never + * emits the pending entry's secret `device_code`/`code_verifier`. + */ +export async function runAuthLoginPoll( + flags: Record, + deps: AuthPollDeps = {}, +): Promise { + const now = deps.now ?? (() => Date.now()); + const emit = deps.stdout ?? ((line) => console.log(line)); + const sleep = + deps.sleep ?? + ((seconds: number) => + new Promise((resolve) => setTimeout(resolve, seconds * 1000))); + const pPath = deps.pendingPath ?? defaultPendingPath(); + const isTTY = deps.isTTY ?? Boolean(process.stdout.isTTY); + const toJson = (payload: Parameters[0]): string => + authFlowJson(payload, isTTY); + + // Machine verb: any unexpected failure must still be a JSON envelope. + try { + const store = loadStore(deps.path); + const pending = loadPending(pPath); + const resolved = resolvePollProfile( + pending, + stringFlag(flags, ['profile']), + store.default, + now(), + ); + if ('error' in resolved) { + emit( + toJson({ + status: 'error', + message: resolved.error, + error: { error_code: resolved.code }, + }), + ); + process.exitCode = 1; + return; + } + + const name = resolved.name; + const entry = getPending(pending, name); + if (!entry) { + emit( + toJson({ + status: 'error', + message: `No pending login for "${name}". Start one with \`${loginStartRestartHint()}\`.`, + error: { error_code: 'no_pending_login' }, + }), + ); + process.exitCode = 1; + return; + } + + const clearPending = (): void => { + const current = loadPending(pPath); + // A concurrent `start` may have superseded our entry with a fresh + // device_code; only clear the row we were actually polling, never a + // newer in-progress login. + if (getPending(current, name)?.device_code !== entry.device_code) { + return; + } + savePending(removePending(current, name), pPath); + }; + + if (isPendingExpired(entry, now())) { + clearPending(); + emit( + toJson({ + status: 'expired', + message: `The device code expired before confirmation. Start over with \`${loginStartRestartHint(entry.base_url)}\`.`, + }), + ); + process.exitCode = 1; + return; + } + + const timeoutRaw = stringFlag(flags, ['timeout']); + const timeoutSeconds = parseTimeoutSeconds(timeoutRaw); + if (timeoutSeconds === undefined) { + emit( + toJson({ + status: 'error', + message: `Invalid --timeout "${timeoutRaw}". Must be a non-negative integer number of seconds.`, + error: { error_code: 'invalid_timeout' }, + }), + ); + // A bad flag value is a usage error — exit 2, matching every other + // invalid invocation, not the generic 1. + process.exitCode = 2; + return; + } + const request = + deps.request ?? + createAnonymousRequest( + entry.base_url, + (deps.deviceId ?? getOrCreateDeviceId)(), + ); + + const result = await pollDeviceTokenBounded({ + request, + deviceCode: entry.device_code, + codeVerifier: entry.code_verifier, + intervalSeconds: entry.interval, + codeExpiresAtMs: Date.parse(entry.expires_at), + timeoutSeconds, + now, + sleep, + }); + + if (result.status === 'authorized') { + const profile: Profile = { + base_url: entry.base_url, + credential: oauthCredentialFromToken(result.token, now()), + saved_at: new Date(now()).toISOString(), + store: 'file', + }; + // Re-load right before the write: a concurrent auth may have saved to + // the store during the poll's long wait; the stale `store` would clobber it. + saveStore( + setDefault(setProfile(loadStore(deps.path), name, profile), name), + deps.path, + ); + // Profile is now authenticated, so any pending code for it is moot — + // clear unconditionally (unlike the error/expired paths, which guard on + // device_code to avoid nuking a fresh start). Otherwise a code a + // concurrent `start` minted mid-poll would linger and make later polls + // misreport `pending` for an already-authenticated profile. + savePending(removePending(loadPending(pPath), name), pPath); + emit( + toJson({ + status: 'authorized', + message: `Logged in; profile "${name}" activated. Confirm with \`amp context --json\`.`, + data: { + profile: name, + base_url: entry.base_url, + region: regionLabelForBaseUrl(entry.base_url)?.toLowerCase(), + token_expires_at: + profile.credential.type === 'oauth' + ? profile.credential.expires_at + : undefined, + scopes: result.token.scope + ? result.token.scope.split(' ') + : undefined, + }, + }), + ); + return; + } + + if (result.status === 'pending') { + // Persist a slow_down-raised interval so the next poll subprocess honors it + // (RFC 8628 §3.5 — the raised interval applies to all subsequent requests). + // Only onto the row we actually polled: a concurrent `start` may have + // superseded it, and this slow_down was raised against the old code, so it + // must not inflate the fresh code's interval (nor clobber its device_code). + const fresh = loadPending(pPath); + const current = getPending(fresh, name); + if ( + current && + current.device_code === entry.device_code && + current.interval !== result.interval + ) { + savePending( + setPending(fresh, name, { ...current, interval: result.interval }), + pPath, + ); + } + emit( + toJson({ + status: 'pending', + message: `Not confirmed yet. Re-run \`amp auth login poll --profile ${name} --json\` immediately — it blocks up to ~${timeoutSeconds}s internally; no sleep needed.`, + data: { + expires_at: entry.expires_at, + poll_waits_seconds: timeoutSeconds, + }, + }), + ); + process.exitCode = 75; + return; + } + + clearPending(); + if (result.status === 'expired') { + emit( + toJson({ + status: 'expired', + message: `The device code expired before confirmation. Start over with \`${loginStartRestartHint(entry.base_url)}\`.`, + }), + ); + } else { + const detail = result.error.description ?? result.error.hint; + emit( + toJson({ + status: 'error', + message: `Authorization failed: ${detail ?? result.error.code}. Start over with \`${loginStartRestartHint(entry.base_url)}\`.`, + error: { + error_code: result.error.code, + detail: detail ?? undefined, + }, + }), + ); + } + process.exitCode = 1; + } catch (error) { + // A thrown CliError (e.g. a bare `--profile` with no value) is already a + // classified usage error — surface its code and exit code rather than + // wrapping it as a retryable poll_failed/1. + if (error instanceof CliError) { + emit( + toJson({ + status: 'error', + message: error.message, + error: { error_code: error.errorCode, detail: error.detail }, + }), + ); + process.exitCode = error.exitCode; + return; + } + const detail = error instanceof Error ? error.message : String(error); + emit( + toJson({ + status: 'error', + message: `${detail} The pending login is preserved — re-run the poll command to retry.`, + error: { + error_code: 'poll_failed', + detail: error instanceof Error ? error.message : undefined, + }, + }), + ); + process.exitCode = 1; + } +} + +/** + * Drops any in-flight `login start` entry for `name`. Completing auth + * out-of-band (interactive `login`, `pat`) strands the device-flow code the + * agent started; without this a later `login poll` would keep hammering the + * abandoned code and reporting `pending` until it expired. No write when + * there's nothing pending for the profile. + */ +function clearPendingLogin(pendingPath: string, name: string): void { + const pending = loadPending(pendingPath); + if (!getPending(pending, name)) { + return; + } + savePending(removePending(pending, name), pendingPath); } export interface AuthLoginDeps { requestToken?: (options: DeviceFlowOptions) => Promise; now?: () => number; path?: string; + pendingPath?: string; + // Resolves the stable install device_id. Injected so tests don't touch the + // real state file; defaults to reading/minting from ~/.amplitude/amp/state.json. + deviceId?: () => string; stdout?: (line: string) => void; stderr?: (line: string) => void; confirm?: (message: string) => Promise; @@ -100,8 +689,9 @@ export interface AuthLoginDeps { /** * `amp auth login` — runs the device flow, saves the token as an OAuth profile, - * and activates it (announcing the switch). Requires an explicit `--profile`; - * creating a profile also requires `--env`/`--base-url`. + * and activates it (announcing the switch). `--profile` is optional: omitting + * it targets the active pointer, else the implicit `default` profile (see + * `targetProfileName`). Creating a profile also requires `--env`/`--base-url`. */ export async function runAuthLogin( flags: Record, @@ -112,57 +702,47 @@ export async function runAuthLogin( const emitStderr = deps.stderr ?? ((line) => console.error(line)); const confirmOverwrite = deps.confirm ?? confirm; const requestToken = deps.requestToken ?? requestDeviceToken; + const resolveDeviceId = deps.deviceId ?? getOrCreateDeviceId; const store = loadStore(deps.path); - // No --profile re-auths the active profile in place (force-explicit applies to - // *creating* a profile, not refreshing one — the default's env+name are already - // recorded). With no default there's nothing to refresh, so require both flags. - // Validate the user-supplied name only; a stored default was already validated - // when it was created. - const requestedName = stringFlag(flags, ['profile']); - if (requestedName !== undefined) { - assertValidProfileName(requestedName); - } - const profileName = requestedName ?? store.default; - if (!profileName) { - throw new Error( - 'No default profile to re-authenticate. Run `amp auth login --profile --env `.', - ); - } - + const profileName = targetProfileName(store, stringFlag(flags, ['profile'])); const existing = getProfile(store, profileName); - // Orphan default: the name came from the store's `default` but that profile is - // gone (e.g. a hand-edited file). The user meant to refresh, not create, so - // match the resolver's "No such profile" instead of falling into - // loginBaseUrl's misleading "creating a profile requires --env" error. A - // user-supplied --profile with no match is the legitimate create path. - if (requestedName === undefined && !existing) { - throw new Error(`No such profile: ${profileName}. Run \`amp auth list\`.`); - } - + const regionFlag = stringFlag(flags, ['region']); + const baseUrlFlag = stringFlag(flags, ['base-url']); const baseUrl = loginBaseUrl({ - baseUrlFlag: stringFlag(flags, ['base-url']), + baseUrlFlag, envFlag: stringFlag(flags, ['env']), + regionFlag, existing, }); + if (regionFlag && !baseUrlFlag) { + emitStdout( + `Authenticating to ${resolveAppOrigin({ apiBaseUrl: baseUrl })}/`, + ); + } // Reusing a name for a different target is almost always a mistake — confirm - // before clobbering. Same target is a silent refresh. - if (existing && existing.base_url !== baseUrl) { + // before clobbering, unless --force opts in (mirrors auth pat / login start). + // Same target is a silent refresh. + if ( + existing && + existing.base_url !== baseUrl && + !isFlagEnabled(flags.force) + ) { const approved = await confirmOverwrite( `Profile "${profileName}" currently targets ${existing.base_url}. Overwrite to ${baseUrl}?`, ); if (!approved) { - throw new Error('Aborted.'); + throw new Error('Aborted.'); // plain-error-ok: TTY-only user cancellation; unreachable non-interactively. } } const token = await requestToken({ flow: stringFlag(flags, ['flow']) ?? 'device', scope: stringFlag(flags, ['scope']) ?? DEFAULT_SCOPES, - request: createAnonymousRequest(baseUrl), + request: createAnonymousRequest(baseUrl, resolveDeviceId()), stderr: emitStderr, }); @@ -178,6 +758,7 @@ export async function runAuthLogin( setDefault(setProfile(store, profileName, profile), profileName), deps.path, ); + clearPendingLogin(deps.pendingPath ?? defaultPendingPath(), profileName); const verb = existing ? 'updated' : 'created'; const wasNote = @@ -225,9 +806,9 @@ async function readWithToken( export interface AuthPatDeps { path?: string; + pendingPath?: string; now?: () => number; stdout?: (line: string) => void; - confirm?: (message: string) => Promise; // Test seam: supplies the raw token in place of stdin / the masked prompt. readToken?: () => Promise; } @@ -235,8 +816,10 @@ export interface AuthPatDeps { /** * `amp auth pat --with-token` — save a supplied Personal Access Token as a * profile and activate it. The token is read from stdin when piped, or a masked - * prompt at a TTY. Force-explicit like login: a new profile needs --profile and - * --env/--base-url; re-auth of an existing profile reuses its recorded env. + * prompt at a TTY. `--profile` is optional: omitting it targets the active + * pointer, else the implicit `default` profile (see `targetProfileName`). + * Creating a profile still requires `--env`/`--base-url`; re-auth of an + * existing profile reuses its recorded env. * * `--with-token` is mandatory: it makes the supply-an-existing-PAT path * explicit and keeps the bare `amp auth pat` verb reserved. @@ -246,45 +829,50 @@ export async function runAuthPat( deps: AuthPatDeps = {}, ): Promise { if (!isFlagEnabled(flags['with-token'])) { - throw new Error( + throw usageError( '`amp auth pat` requires --with-token to supply an existing PAT (piped on stdin, or pasted at a prompt).', ); } - const profileName = stringFlag(flags, ['profile']); - if (!profileName) { - throw new Error('`amp auth pat` requires --profile .'); - } - assertValidProfileName(profileName); - const now = deps.now ?? (() => Date.now()); const emitStdout = deps.stdout ?? ((line) => console.log(line)); - const confirmOverwrite = deps.confirm ?? confirm; const store = loadStore(deps.path); + const profileName = targetProfileName(store, stringFlag(flags, ['profile'])); const existing = getProfile(store, profileName); + const regionFlag = stringFlag(flags, ['region']); + const baseUrlFlag = stringFlag(flags, ['base-url']); const baseUrl = loginBaseUrl({ - baseUrlFlag: stringFlag(flags, ['base-url']), + baseUrlFlag, envFlag: stringFlag(flags, ['env']), + regionFlag, existing, }); + if (regionFlag && !baseUrlFlag) { + emitStdout( + `Authenticating to ${resolveAppOrigin({ apiBaseUrl: baseUrl })}/`, + ); + } - // Reusing a name for a different target is almost always a mistake — confirm - // before clobbering. Same target is a silent refresh. - if (existing && existing.base_url !== baseUrl) { - const approved = await confirmOverwrite( - `Profile "${profileName}" currently targets ${existing.base_url}. Overwrite to ${baseUrl}?`, + // Reusing a name for a different target is almost always a mistake — block + // unless the caller explicitly opts in with --force (mirrors + // `auth login --start`'s gate; no interactive confirm here since this path + // must also work non-interactively/piped). + if ( + existing && + existing.base_url !== baseUrl && + !isFlagEnabled(flags.force) + ) { + throw usageError( + `Profile "${profileName}" targets ${existing.base_url}; refusing to silently retarget it to ${baseUrl}. Use a different --profile, or pass --force to overwrite.`, ); - if (!approved) { - throw new Error('Aborted.'); - } } const readToken = deps.readToken ?? (() => readWithToken(baseUrl, emitStdout)); const pat = normalizePat(await readToken()); if (!pat) { - throw new Error('PAT cannot be empty.'); + throw usageError('PAT cannot be empty.'); } const credential: PatCredential = { type: 'pat', pat }; @@ -300,6 +888,7 @@ export async function runAuthPat( setDefault(setProfile(store, profileName, profile), profileName), deps.path, ); + clearPendingLogin(deps.pendingPath ?? defaultPendingPath(), profileName); const verb = existing ? 'updated' : 'created'; const wasNote = @@ -315,6 +904,7 @@ export async function runAuthPat( interface ProfileCommandDeps { path?: string; + pendingPath?: string; now?: () => number; stdout?: (line: string) => void; confirm?: (message: string) => Promise; @@ -331,6 +921,17 @@ export function envLabel(baseUrl: string): string { return baseUrl; } +/** US/EU label for `auth status`, shown only for prod/prod-eu profiles. */ +export function regionLabelForBaseUrl(baseUrl: string): string | undefined { + if (baseUrl === ENV_BASE_URLS.prod) { + return 'US'; + } + if (baseUrl === ENV_BASE_URLS['prod-eu']) { + return 'EU'; + } + return undefined; +} + function humanizeDuration(ms: number): string { const totalMinutes = Math.round(ms / 60_000); const hours = Math.floor(totalMinutes / 60); @@ -393,11 +994,41 @@ export function formatProfileList(store: CredentialStore, now: number): string { ); } -/** `amp auth list` — show profiles, marking the default with `*`. */ -export function runAuthList(deps: ProfileCommandDeps = {}): void { +/** + * `amp auth list` — show profiles, marking the default with `*`. Piped or + * `--json` emits `{ profiles: [...], default }` on stdout instead of the + * table; an empty store is still exit 0 (listing succeeded, there's just + * nothing to show). + */ +export function runAuthList( + flags: Record, + deps: ProfileCommandDeps = {}, +): void { const emitStdout = deps.stdout ?? ((line) => console.log(line)); + const isTTY = deps.isTTY ?? Boolean(process.stdout.isTTY); const now = deps.now?.() ?? Date.now(); - emitStdout(formatProfileList(loadStore(deps.path), now)); + const store = loadStore(deps.path); + + if (shouldUseJsonOutput({ jsonFlag: isFlagEnabled(flags.json), isTTY })) { + const profiles = Object.keys(store.profiles).map((name) => { + const profile = store.profiles[name]; + const oauth = oauthCredentialSchema.safeParse(profile.credential); + return { + name, + type: profile.credential.type, + base_url: profile.base_url, + region: regionLabelForBaseUrl(profile.base_url)?.toLowerCase(), + expires_at: oauth.success ? oauth.data.expires_at : undefined, + is_default: store.default === name, + }; + }); + emitStdout( + formatJsonOutput({ profiles, default: store.default ?? null }, isTTY), + ); + return; + } + + emitStdout(formatProfileList(store, now)); } /** `amp auth use ` — repoint the default with no re-auth. */ @@ -406,7 +1037,7 @@ export function runAuthUse( deps: ProfileCommandDeps = {}, ): void { if (!name) { - throw new Error( + throw usageError( '`amp auth use` requires a profile name: amp auth use .', ); } @@ -414,7 +1045,7 @@ export function runAuthUse( const store = loadStore(deps.path); if (!getProfile(store, name)) { const known = Object.keys(store.profiles); - throw new Error( + throw usageError( `No such profile: ${name}.${known.length ? ` Known: ${known.join(', ')}.` : ' Run `amp auth login`.'}`, ); } @@ -429,7 +1060,8 @@ export function runAuthUse( /** * `amp logout [--profile | --all]` — remove a profile, or wipe the whole - * store with `--all`. Target resolution is `--profile` > default > error. Clears + * store with `--all`. Target resolution is `--profile` > default > a solitary + * in-progress login > error. Clears * `default` if it pointed at the removed profile and never auto-promotes a * survivor (the active identity only changes on an explicit command), so * logging out the default leaves no default set. @@ -440,61 +1072,108 @@ export async function runLogout( ): Promise { const emitStdout = deps.stdout ?? ((line) => console.log(line)); const confirmLogout = deps.confirm ?? confirm; + const now = deps.now ?? (() => Date.now()); + const pPath = deps.pendingPath ?? defaultPendingPath(); const store = loadStore(deps.path); if (isFlagEnabled(flags.all)) { if (stringFlag(flags, ['profile'])) { - throw new Error('Pass either --profile or --all, not both.'); + throw usageError('Pass either --profile or --all, not both.'); } const count = Object.keys(store.profiles).length; - if (count === 0) { + const pendingCount = Object.keys(loadPending(pPath).pending).length; + if (count === 0 && pendingCount === 0) { emitStdout('No profiles to remove.'); return; } - const decision = logoutAllGateDecision({ - isTTY: deps.isTTY ?? Boolean(process.stdin.isTTY && process.stdout.isTTY), - yes: isFlagEnabled(flags.yes), - }); - if (decision === 'block') { - throw new Error( - 'Pass --yes to remove every stored profile with `amp logout --all`.', - ); - } - if (decision === 'confirm') { - const approved = await confirmLogout( - `Remove all ${count} profile${count === 1 ? '' : 's'}? Stored credentials cannot be recovered.`, - ); - if (!approved) { - throw new Error('Aborted.'); + // Gate protects stored credentials; only apply it when there are profiles + // to remove. An in-flight pending login is transient, so clearing it alone + // needs no confirmation. + if (count > 0) { + const decision = logoutAllGateDecision({ + isTTY: + deps.isTTY ?? Boolean(process.stdin.isTTY && process.stdout.isTTY), + yes: isFlagEnabled(flags.yes), + }); + if (decision === 'block') { + throw usageError( + 'Pass --yes to remove every stored profile with `amp logout --all`.', + ); + } + if (decision === 'confirm') { + const approved = await confirmLogout( + `Remove all ${count} profile${count === 1 ? '' : 's'}? Stored credentials cannot be recovered.`, + ); + if (!approved) { + throw new Error('Aborted.'); // plain-error-ok: TTY-only user cancellation; unreachable non-interactively. + } } } saveStore(emptyStore(), deps.path); + savePending(emptyPending(), pPath); emitStdout( terminal.success( - `Removed all ${count} profile${count === 1 ? '' : 's'}. No credentials remain — run \`amp auth login\`.`, + count > 0 + ? `Removed all ${count} profile${count === 1 ? '' : 's'}. No credentials remain — run \`amp auth login\`.` + : `Cleared ${pendingCount} in-progress login${pendingCount === 1 ? '' : 's'}.`, ), ); return; } - const target = stringFlag(flags, ['profile']) ?? store.default; + const pending = loadPending(pPath); + // A cold `login start` leaves a pending entry but no store default yet, so + // fall back to a solitary in-progress login — otherwise bare `logout` can't + // cancel it, though `logout --profile ` can. Only live entries count + // (expired ones are GC'd lazily), matching how `login poll` disambiguates. + const livePending = Object.keys(pending.pending).filter( + (name) => !isPendingExpired(pending.pending[name], now()), + ); + const target = + stringFlag(flags, ['profile']) ?? + store.default ?? + (livePending.length === 1 ? livePending[0] : undefined); if (!target) { - throw new Error( + throw usageError( 'No profile to log out of. Pass --profile or set a default with `amp auth use`.', ); } - if (!getProfile(store, target)) { + const hasProfile = getProfile(store, target) !== undefined; + const hasPending = getPending(pending, target) !== undefined; + // A `start`ed-but-never-completed login has a pending entry with no stored + // profile yet (e.g. first-time `default`). `logout --profile ` must be + // able to cancel it, so only error when there's neither a profile nor a + // pending login to remove. + if (!hasProfile && !hasPending) { const known = Object.keys(store.profiles); - throw new Error( + throw usageError( `No such profile: ${target}.${known.length ? ` Known: ${known.join(', ')}.` : ''}`, ); } - const wasDefault = store.default === target; - const next = removeProfile(store, target); + // Drop any in-flight login for this profile — a logout shouldn't leave its + // device-flow secrets behind. Re-read immediately before the write so a + // `login start` for another profile that landed since the snapshot above + // isn't clobbered (same reload-before-mutate rule the poll paths follow). + if (hasPending) { + savePending(removePending(loadPending(pPath), target), pPath); + } + if (!hasProfile) { + emitStdout( + terminal.success(`Canceled the in-progress login for "${target}".`), + ); + return; + } + + // Re-read before the write for the same reason as the pending clear above: + // another auth for a different profile may have landed since the snapshot at + // the top, and removing `target` from a stale store would clobber it. + const current = loadStore(deps.path); + const wasDefault = current.default === target; + const next = removeProfile(current, target); saveStore(next, deps.path); if (wasDefault) { @@ -535,16 +1214,46 @@ interface AuthStatusDeps { * when a usable credential resolves, non-zero otherwise — scriptable. When * `AMP_TOKEN` is in effect it is announced (the gh "exported token silently * shadows my login" mitigation). No network call. + * + * Piped or `--json`: emits a single JSON object on stdout and exits 0 when + * authenticated. When resolution fails, stdout stays empty — the resolver's + * `CliError` (`authentication_required`/`invalid_token`) is left to propagate + * to `main()`'s standard error envelope on stderr, rather than being caught + * and rendered as the TTY diagnostic below. */ export function runAuthStatus( flags: Record, deps: AuthStatusDeps = {}, ): void { const emitStdout = deps.stdout ?? ((line) => console.log(line)); - const styled = terminalForStdout(deps.isTTY); + const isTTY = deps.isTTY ?? Boolean(process.stdout.isTTY); const now = deps.now?.() ?? Date.now(); const store = deps.store ?? loadStore(); + if (shouldUseJsonOutput({ jsonFlag: isFlagEnabled(flags.json), isTTY })) { + const auth = resolveAuthFromFlags(flags, { store, now, env: deps.env }); + const profile = auth.profile ? getProfile(store, auth.profile) : undefined; + const oauth = + profile && oauthCredentialSchema.safeParse(profile.credential); + emitStdout( + formatJsonOutput( + { + authenticated: true, + source: auth.source, + profile: auth.profile, + type: profile?.credential.type ?? 'token', + base_url: auth.baseUrl, + expires_at: + oauth && oauth.success ? oauth.data.expires_at : undefined, + token: maskToken(auth.token), + }, + isTTY, + ), + ); + return; + } + + const styled = terminalForStdout(isTTY); emitStdout(`${styled.heading('Auth status')}\n`); const emitProfileRows = (name: string, profile: Profile): void => { @@ -568,7 +1277,10 @@ export function runAuthStatus( emitStdout( styled.warning(error instanceof Error ? error.message : String(error)), ); - process.exitCode = 1; + // Same failure must exit the same regardless of output mode: carry the + // resolver's CliError exit code (invalid_token/authentication_required → + // 3) instead of flattening the TTY path to 1, matching the JSON path. + process.exitCode = error instanceof CliError ? error.exitCode : 1; return; } @@ -585,6 +1297,12 @@ export function runAuthStatus( emitProfileRows(auth.profile, profile); } + const region = regionLabelForBaseUrl(auth.baseUrl); + if (region) { + emitStdout( + `Region: ${region} (${resolveAppOrigin({ apiBaseUrl: auth.baseUrl })}/)`, + ); + } emitStdout(`Base URL: ${styled.dim(auth.baseUrl)}`); emitStdout(`Token: ${maskToken(auth.token)}`); } diff --git a/src/auth-flag-coverage.test.ts b/src/auth-flag-coverage.test.ts new file mode 100644 index 0000000..9d45eb7 --- /dev/null +++ b/src/auth-flag-coverage.test.ts @@ -0,0 +1,101 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { globalOptionAliases } from './args'; +import { buildCatalog } from './catalog'; + +/** + * `auth-commands.ts`'s handlers dispatch outside `buildRequest`, so the only + * thing standing between a typo'd flag and a silent no-op is + * `assertKnownAuthFlags` in cli.ts rejecting anything not in + * globals ∪ the catalog's hand-authored `AUTH_COMMANDS` flags. That reject is + * only safe if every flag alias a handler actually reads is declared + * somewhere in that allowed set — otherwise a legitimate flag a handler reads + * could be falsely rejected before the handler ever sees it. `auth-commands.ts` + * itself delegates its `env`/`region`/`base-url`/`token`/`profile` reads to + * `resolveAuthFromFlags`/`selectProfileFromFlags` in `credential-resolver.ts`, + * so this test scans BOTH files with the same extraction and asserts every + * alias referenced in either is covered. The boundary this test enforces: it + * covers exactly the flag reads reachable from an auth/logout handler today + * (`auth-commands.ts` + `credential-resolver.ts`). If a handler is ever + * changed to delegate flag-reading to some OTHER module, that module must be + * added to `SCANNED_FILES` below, or its reads become invisible to this + * guard. + */ + +const SCANNED_FILES = ['auth-commands.ts', 'credential-resolver.ts']; + +const SCANNED_SOURCE = SCANNED_FILES.map((file) => + readFileSync(join(__dirname, file), 'utf8'), +).join('\n'); + +/** + * Every flag alias a scanned file reads off `flags`, via any of the + * recognized read patterns: + * - `stringFlag(flags, [...])` / `flagValue(flags, [...])` / `hasFlag(flags, [...])` + * - `isFlagEnabled(flags['x'])` / `isFlagEnabled(flags.x)` + * - direct `flags['x']` / `flags.x` reads + * Intentionally over-broad rather than under: a pattern matching more call + * sites than strictly necessary only means more aliases must be covered, + * which is the safe direction for a guard against false-positive rejects. + */ +function extractReferencedAliases(source: string): Set { + const aliases = new Set(); + + // stringFlag(flags, ['a', 'b']) / flagValue(flags, [...]) / hasFlag(flags, [...]) + const arrayCallPattern = + /\b(?:stringFlag|flagValue|hasFlag)\(\s*flags\s*,\s*\[([^\]]*)\]/g; + for (const match of source.matchAll(arrayCallPattern)) { + for (const literal of match[1].matchAll(/['"]([^'"]+)['"]/g)) { + aliases.add(literal[1]); + } + } + + // flags['with-token'] / flags["with-token"] + const bracketAccessPattern = /\bflags\[\s*['"]([^'"]+)['"]\s*\]/g; + for (const match of source.matchAll(bracketAccessPattern)) { + aliases.add(match[1]); + } + + // flags.force / flags.json / flags.all / flags.yes + const dotAccessPattern = /\bflags\.([A-Za-z_][A-Za-z0-9_]*)/g; + for (const match of source.matchAll(dotAccessPattern)) { + aliases.add(match[1]); + } + + return aliases; +} + +function allowedAuthFlagAliases(): Set { + const catalogAuthAliases = buildCatalog() + .filter((entry) => entry.group === 'auth') + .flatMap((entry) => entry.flags.flatMap((flag) => flag.aliases)); + return new Set([...globalOptionAliases(), ...catalogAuthAliases]); +} + +describe('auth handler flag-read coverage', () => { + it('extracts a non-empty, sane set of referenced aliases (sanity check on the extractor itself)', () => { + const referenced = extractReferencedAliases(SCANNED_SOURCE); + // Known reads as of writing — asserting a floor guards against the + // extractor silently regressing to matching nothing. + for (const expected of ['profile', 'region', 'with-token', 'all']) { + expect(referenced.has(expected)).toBe(true); + } + }); + + it('covers every flag alias the scanned files read with globals ∪ AUTH_COMMANDS catalog flags', () => { + const referenced = extractReferencedAliases(SCANNED_SOURCE); + const allowed = allowedAuthFlagAliases(); + + const uncovered = [...referenced].filter((alias) => !allowed.has(alias)); + + expect( + uncovered, + `${SCANNED_FILES.join(', ')} read flag alias(es) not covered by GLOBAL_OPTIONS or any AUTH_COMMANDS catalog entry: ${uncovered.join(', ')}. ` + + 'Declare the flag on the relevant AUTH_COMMANDS entry in catalog.ts (or, if it should apply to every command, add it to GLOBAL_OPTIONS in args.ts) — ' + + 'otherwise the misplaced-flag reject can falsely reject a legitimate auth flag.', + ).toEqual([]); + }); +}); diff --git a/src/auth-guidance.test.ts b/src/auth-guidance.test.ts index 02aee8d..06c0edc 100644 --- a/src/auth-guidance.test.ts +++ b/src/auth-guidance.test.ts @@ -36,7 +36,7 @@ describe('auth guidance', () => { apiBaseUrl: 'https://developer-api.eu.amplitude.com', }), ).toBe( - 'https://eu.amplitude.com/analytics/amplitude/settings/profile/personal-access-tokens', + 'https://app.eu.amplitude.com/analytics/amplitude/settings/profile/personal-access-tokens', ); }); @@ -69,7 +69,7 @@ describe('auth guidance', () => { apiBaseUrl: 'https://developer-api.eu.amplitude.com', }), ).toContain( - 'https://eu.amplitude.com/analytics/amplitude/settings/profile/personal-access-tokens', + 'https://app.eu.amplitude.com/analytics/amplitude/settings/profile/personal-access-tokens', ); }); diff --git a/src/auth-guidance.ts b/src/auth-guidance.ts index ae88f05..31ee39c 100644 --- a/src/auth-guidance.ts +++ b/src/auth-guidance.ts @@ -1,7 +1,7 @@ import { apiBaseUrlFromEnv } from './env'; export const DEFAULT_APP_ORIGIN = 'https://app.amplitude.com'; -export const EU_APP_ORIGIN = 'https://eu.amplitude.com'; +export const EU_APP_ORIGIN = 'https://app.eu.amplitude.com'; export const DEFAULT_ORG_URL = 'amplitude'; const PAT_SETTINGS_SUFFIX = '/settings/profile/personal-access-tokens'; @@ -68,10 +68,11 @@ export function authSetupInstructions(options?: { 'Authenticate amp with a Personal Access Token (PAT).', '', 'Recommended:', - ' amp auth pat --with-token --profile --env ', + ' amp auth pat --with-token --region ', '', 'That prints the PAT settings page and reads the token from stdin (or a', - 'masked prompt at a terminal), saving it as a profile for future commands.', + 'masked prompt at a terminal), saving it as the "default" profile (pass', + '--profile to use a different one) for future commands.', '', 'Manual setup:', ` ${setupUrl}`, diff --git a/src/auth-login.test.ts b/src/auth-login.test.ts index 39ba876..69cf42c 100644 --- a/src/auth-login.test.ts +++ b/src/auth-login.test.ts @@ -1,6 +1,6 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -16,6 +16,12 @@ import { saveStore, } from './credential-store'; import type { TokenResponse } from './oauthResponseSchemas'; +import { + getPending, + loadPending, + savePending, + setPending, +} from './pending-store'; const NOW = Date.parse('2026-06-23T12:00:00.000Z'); @@ -53,6 +59,28 @@ describe('loginBaseUrl', () => { ); }); + it('maps --region', () => { + expect(loginBaseUrl({ regionFlag: 'us' })).toBe( + 'https://developer-api.amplitude.com', + ); + }); + + it('throws when both --region and --env are given', () => { + expect(() => + loginBaseUrl({ regionFlag: 'us', envFlag: 'staging' }), + ).toThrow('Pass either --region or --env, not both.'); + }); + + it('throws when both --region and --env are given even though --base-url would win', () => { + expect(() => + loginBaseUrl({ + regionFlag: 'us', + envFlag: 'staging', + baseUrlFlag: 'http://localhost:3036', + }), + ).toThrow('Pass either --region or --env, not both.'); + }); + it('reuses an existing profile base_url on re-auth', () => { expect( loginBaseUrl({ @@ -65,8 +93,15 @@ describe('loginBaseUrl', () => { ).toBe('https://prod'); }); - it('errors creating a profile without --env/--base-url (force-explicit)', () => { - expect(() => loginBaseUrl({})).toThrow(/requires --env|--base-url/); + it('errors creating a profile without --region (hidden --env/--base-url stay out of the message)', () => { + expect(() => loginBaseUrl({})).toThrow(/requires --region /); + try { + loginBaseUrl({}); + } catch (error) { + const message = String(error); + expect(message).not.toContain('--env'); + expect(message).not.toContain('--base-url'); + } }); it('errors on an unknown --env', () => { @@ -90,10 +125,16 @@ describe('runAuthLogin', () => { return join(dir, 'credentials.json'); } + function pendingPathFor(path: string): string { + return join(dirname(path), 'pending.json'); + } + function deps(path: string, out: string[]) { return { path, + pendingPath: pendingPathFor(path), now: () => NOW, + deviceId: () => 'test-device-id', stdout: (line: string) => out.push(line), stderr: () => {}, requestToken: () => Promise.resolve(TOKEN), @@ -143,10 +184,23 @@ describe('runAuthLogin', () => { expect(out.join('\n')).toMatch(/updated and set as default\./); }); - it('errors when neither --profile nor a default profile exists', async () => { - await expect( - runAuthLogin({ env: 'prod' }, deps(tempPath(), [])), - ).rejects.toThrow(/No default profile/); + it('creates the implicit "default" profile when --profile is omitted', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthLogin({ env: 'prod' }, deps(path, out)); + + const store = loadStore(path); + expect(store.default).toBe('default'); + expect(getProfile(store, 'default')?.base_url).toBe( + 'https://developer-api.amplitude.com', + ); + expect(out.join('\n')).toMatch(/created and set as default/); + }); + + it('errors asking for --region on a cold bare login', async () => { + await expect(runAuthLogin({}, deps(tempPath(), []))).rejects.toThrow( + /requires --region/, + ); }); it('errors with "No such profile" when the stored default is orphaned', async () => { @@ -164,10 +218,12 @@ describe('runAuthLogin', () => { ); }); - it('rejects the reserved name "default"', async () => { - await expect( - runAuthLogin({ profile: 'default', env: 'prod' }, deps(tempPath(), [])), - ).rejects.toThrow(/reserved/); + it('accepts an explicit --profile default', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthLogin({ profile: 'default', env: 'prod' }, deps(path, out)); + expect(loadStore(path).default).toBe('default'); + expect(out.join('\n')).toMatch(/created and set as default/); }); it('rejects an invalid profile name', async () => { @@ -176,6 +232,39 @@ describe('runAuthLogin', () => { ).rejects.toThrow(/Invalid profile name/); }); + it('announces the region when --region is used', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthLogin({ profile: 'amplitude', region: 'us' }, deps(path, out)); + expect(out.join('\n')).toContain( + 'Authenticating to https://app.amplitude.com/', + ); + }); + + it('does not announce a region when --env is used', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthLogin({ profile: 'amplitude', env: 'prod' }, deps(path, out)); + expect(out.join('\n')).not.toContain('Authenticating to'); + }); + + it('does not announce a region when --base-url wins over --region', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthLogin( + { + profile: 'amplitude', + region: 'us', + 'base-url': 'http://localhost:3036', + }, + deps(path, out), + ); + expect(out.join('\n')).not.toContain('Authenticating to'); + expect(getProfile(loadStore(path), 'amplitude')?.base_url).toBe( + 'http://localhost:3036', + ); + }); + it('aborts a re-login to a different target when not confirmed', async () => { const path = tempPath(); await runAuthLogin({ profile: 'p', env: 'prod' }, deps(path, [])); @@ -192,6 +281,25 @@ describe('runAuthLogin', () => { ); }); + it('skips the overwrite confirm and retargets when --force is passed', async () => { + const path = tempPath(); + await runAuthLogin({ profile: 'p', env: 'prod' }, deps(path, [])); + + await runAuthLogin( + { profile: 'p', env: 'staging', force: true }, + { + ...deps(path, []), + confirm: () => { + throw new Error('confirm must not be called when --force is passed'); + }, + }, + ); + + expect(loadStore(path).profiles.p?.base_url).toBe( + 'https://developer-api.stag2.amplitude.com', + ); + }); + it('requests the full default scope set when --scope is absent', async () => { const path = tempPath(); const scopes: Array = []; @@ -205,7 +313,7 @@ describe('runAuthLogin', () => { await runAuthLogin({ profile: 'amplitude', env: 'prod' }, capturing); expect(scopes[0]).toBe( - 'mcp:read mcp:write read:flags read:projects read:taxonomy write:flags write:taxonomy', + 'mcp:read mcp:write read:analytics read:flags read:projects read:taxonomy write:flags write:taxonomy', ); }); @@ -226,4 +334,46 @@ describe('runAuthLogin', () => { ); expect(scopes[0]).toBe('read:projects'); }); + + it('clears a stranded login-start entry for the profile it authenticates', async () => { + const path = tempPath(); + const pendingPath = pendingPathFor(path); + savePending( + setPending(loadPending(pendingPath), 'amplitude', { + device_code: 'abandoned', + code_verifier: 'cv', + base_url: 'https://developer-api.amplitude.com', + expires_at: new Date(NOW + 600_000).toISOString(), + interval: 5, + started_at: new Date(NOW).toISOString(), + }), + pendingPath, + ); + + await runAuthLogin({ profile: 'amplitude', env: 'prod' }, deps(path, [])); + + expect(getPending(loadPending(pendingPath), 'amplitude')).toBeUndefined(); + }); + + it('leaves another profile’s pending login untouched', async () => { + const path = tempPath(); + const pendingPath = pendingPathFor(path); + savePending( + setPending(loadPending(pendingPath), 'other', { + device_code: 'other-code', + code_verifier: 'cv', + base_url: 'https://developer-api.amplitude.com', + expires_at: new Date(NOW + 600_000).toISOString(), + interval: 5, + started_at: new Date(NOW).toISOString(), + }), + pendingPath, + ); + + await runAuthLogin({ profile: 'amplitude', env: 'prod' }, deps(path, [])); + + expect(getPending(loadPending(pendingPath), 'other')?.device_code).toBe( + 'other-code', + ); + }); }); diff --git a/src/auth-pat.test.ts b/src/auth-pat.test.ts index bff46de..3efe70e 100644 --- a/src/auth-pat.test.ts +++ b/src/auth-pat.test.ts @@ -1,11 +1,18 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { type AuthPatDeps, normalizePat, runAuthPat } from './auth-commands'; +import { CliError } from './cli-error'; import { getProfile, loadStore } from './credential-store'; +import { + getPending, + loadPending, + savePending, + setPending, +} from './pending-store'; const NOW = Date.parse('2026-06-23T12:00:00.000Z'); @@ -33,12 +40,16 @@ describe('runAuthPat', () => { return join(dir, 'credentials.json'); } + function pendingPathFor(path: string): string { + return join(dirname(path), 'pending.json'); + } + function deps(path: string, out: string[], over: Partial = {}) { return { path, + pendingPath: pendingPathFor(path), now: () => NOW, stdout: (line: string) => out.push(line), - confirm: () => Promise.resolve(true), readToken: () => Promise.resolve('amp_pasted'), ...over, }; @@ -85,19 +96,27 @@ describe('runAuthPat', () => { expect(loadStore(path).profiles.ci).toBeUndefined(); }); - it('requires --profile', async () => { - await expect( - runAuthPat({ env: 'prod', 'with-token': true }, deps(tempPath(), [])), - ).rejects.toThrow(/--profile/); + it('creates the implicit "default" profile when --profile is omitted', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthPat({ env: 'prod', 'with-token': true }, deps(path, out)); + + const store = loadStore(path); + expect(store.default).toBe('default'); + expect(getProfile(store, 'default')?.credential).toEqual({ + type: 'pat', + pat: 'amp_pasted', + }); + expect(out.join('\n')).toMatch(/created and set as default/); }); - it('rejects the reserved name "default"', async () => { - await expect( - runAuthPat( - { profile: 'default', env: 'prod', 'with-token': true }, - deps(tempPath(), []), - ), - ).rejects.toThrow(/reserved/); + it('accepts an explicit --profile default', async () => { + const path = tempPath(); + await runAuthPat( + { profile: 'default', env: 'prod', 'with-token': true }, + deps(path, []), + ); + expect(loadStore(path).default).toBe('default'); }); it('rejects an invalid profile name', async () => { @@ -109,10 +128,57 @@ describe('runAuthPat', () => { ).rejects.toThrow(/Invalid profile name/); }); - it('is force-explicit: creating without --env/--base-url errors', async () => { + it('is force-explicit: creating without --region errors', async () => { await expect( runAuthPat({ profile: 'ci', 'with-token': true }, deps(tempPath(), [])), - ).rejects.toThrow(/requires --env|--base-url/); + ).rejects.toThrow(/requires --region/); + }); + + it('maps --region for a new PAT profile', async () => { + const path = tempPath(); + await runAuthPat( + { profile: 'ci', region: 'eu', 'with-token': true }, + deps(path, []), + ); + expect(getProfile(loadStore(path), 'ci')?.base_url).toBe( + 'https://developer-api.eu.amplitude.com', + ); + }); + + it('announces the region when --region is used', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthPat( + { profile: 'ci', region: 'eu', 'with-token': true }, + deps(path, out), + ); + expect(out.join('\n')).toContain( + 'Authenticating to https://app.eu.amplitude.com/', + ); + }); + + it('does not announce a region when --base-url wins over --region', async () => { + const path = tempPath(); + const out: string[] = []; + await runAuthPat( + { + profile: 'ci', + region: 'eu', + 'base-url': 'http://localhost:3036', + 'with-token': true, + }, + deps(path, out), + ); + expect(out.join('\n')).not.toContain('Authenticating to'); + expect(getProfile(loadStore(path), 'ci')?.base_url).toBe( + 'http://localhost:3036', + ); + }); + + it('errors asking for --region on a cold bare pat', async () => { + await expect( + runAuthPat({ 'with-token': true }, deps(tempPath(), [])), + ).rejects.toThrow(/requires --region/); }); it('rejects an empty supplied PAT', async () => { @@ -124,21 +190,73 @@ describe('runAuthPat', () => { ).rejects.toThrow(/cannot be empty/); }); - it('aborts a re-pat to a different target when not confirmed', async () => { + it('refuses to silently retarget a profile without --force (non-interactive, no confirm)', async () => { const path = tempPath(); await runAuthPat( { profile: 'ci', env: 'prod', 'with-token': true }, deps(path, []), ); - await expect( - runAuthPat( + let caught: unknown; + try { + await runAuthPat( { profile: 'ci', env: 'staging', 'with-token': true }, - deps(path, [], { confirm: () => Promise.resolve(false) }), - ), - ).rejects.toThrow(/Aborted/); + deps(path, []), + ); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(CliError); + if (caught instanceof CliError) { + expect(caught.errorCode).toBe('usage_error'); + expect(caught.exitCode).toBe(2); + expect(caught.message).toMatch(/refusing to silently retarget/); + } expect(loadStore(path).profiles.ci?.base_url).toBe( 'https://developer-api.amplitude.com', ); }); + + it('retargets a profile with --force (no confirm needed)', async () => { + const path = tempPath(); + await runAuthPat( + { profile: 'ci', env: 'prod', 'with-token': true }, + deps(path, []), + ); + + const out: string[] = []; + await runAuthPat( + { profile: 'ci', env: 'staging', 'with-token': true, force: true }, + deps(path, out), + ); + + expect(loadStore(path).profiles.ci?.base_url).not.toBe( + 'https://developer-api.amplitude.com', + ); + expect(out.join('\n')).toMatch(/updated and set as default/); + }); + + it('clears a stranded login-start entry for the profile it authenticates', async () => { + const path = tempPath(); + const pendingPath = pendingPathFor(path); + savePending( + setPending(loadPending(pendingPath), 'ci', { + device_code: 'abandoned', + code_verifier: 'cv', + base_url: 'https://developer-api.amplitude.com', + expires_at: new Date(NOW + 600_000).toISOString(), + interval: 5, + started_at: new Date(NOW).toISOString(), + }), + pendingPath, + ); + + await runAuthPat( + { profile: 'ci', env: 'prod', 'with-token': true }, + deps(path, []), + ); + + expect(getPending(loadPending(pendingPath), 'ci')).toBeUndefined(); + }); }); diff --git a/src/auth-profiles.test.ts b/src/auth-profiles.test.ts index 14cf553..8f8604e 100644 --- a/src/auth-profiles.test.ts +++ b/src/auth-profiles.test.ts @@ -1,6 +1,6 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -10,11 +10,14 @@ import { formatProfileList, logoutAllGateDecision, maskToken, + regionLabelForBaseUrl, + runAuthList, runAuthStatus, runAuthToken, runAuthUse, runLogout, } from './auth-commands'; +import { CliError } from './cli-error'; import { type CredentialStore, CURRENT_VERSION, @@ -24,9 +27,25 @@ import { setDefault, setProfile, } from './credential-store'; +import { + emptyPending, + getPending, + loadPending, + savePending, + setPending, +} from './pending-store'; const NOW = Date.parse('2026-06-23T12:00:00.000Z'); +const pendingEntry = (baseUrl: string) => ({ + device_code: 'DC', + code_verifier: 'CV', + base_url: baseUrl, + expires_at: '2999-01-01T00:00:00Z', + interval: 5, + started_at: '2026-07-15T00:00:00.000Z', +}); + function oauthProfile(baseUrl: string, expiresAt: string) { return { base_url: baseUrl, @@ -54,6 +73,24 @@ describe('envLabel', () => { }); }); +describe('regionLabelForBaseUrl', () => { + it('labels prod as US and prod-eu as EU', () => { + expect(regionLabelForBaseUrl('https://developer-api.amplitude.com')).toBe( + 'US', + ); + expect( + regionLabelForBaseUrl('https://developer-api.eu.amplitude.com'), + ).toBe('EU'); + }); + + it('returns undefined for internal envs', () => { + expect(regionLabelForBaseUrl('http://localhost:3036')).toBeUndefined(); + expect( + regionLabelForBaseUrl('https://developer-api.stag2.amplitude.com'), + ).toBeUndefined(); + }); +}); + describe('expiryLabel', () => { it('reports a relative time for a valid oauth token', () => { expect( @@ -118,6 +155,105 @@ describe('formatProfileList', () => { }); }); +describe('runAuthList', () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs) { + rmSync(dir, { force: true, recursive: true }); + } + dirs.length = 0; + }); + + function seeded(): string { + const dir = mkdtempSync(join(tmpdir(), 'amp-list-')); + dirs.push(dir); + const path = join(dir, 'credentials.json'); + let store: CredentialStore = setProfile( + emptyStore(), + 'amplitude', + oauthProfile( + 'https://developer-api.amplitude.com', + '2026-06-23T14:05:00.000Z', + ), + ); + store = setProfile(store, 'ci', { + base_url: 'https://developer-api.stag2.amplitude.com', + credential: { type: 'pat', pat: 'amp_ci' }, + saved_at: '2026-06-23T00:00:00.000Z', + store: 'file', + }); + store = setDefault(store, 'amplitude'); + saveStore(store, path); + return path; + } + + it('renders the table at a TTY, unchanged', () => { + const path = seeded(); + const out: string[] = []; + runAuthList( + {}, + { path, now: () => NOW, isTTY: true, stdout: (l) => out.push(l) }, + ); + expect(out).toEqual([formatProfileList(loadStore(path), NOW)]); + }); + + it('emits the profiles envelope on stdout, non-TTY', () => { + const path = seeded(); + const out: string[] = []; + runAuthList( + {}, + { path, now: () => NOW, isTTY: false, stdout: (l) => out.push(l) }, + ); + + expect(out).toHaveLength(1); + const payload = JSON.parse(out[0]); + expect(payload.default).toBe('amplitude'); + expect(payload.profiles).toEqual([ + { + name: 'amplitude', + type: 'oauth', + base_url: 'https://developer-api.amplitude.com', + region: 'us', + expires_at: '2026-06-23T14:05:00.000Z', + is_default: true, + }, + { + name: 'ci', + type: 'pat', + base_url: 'https://developer-api.stag2.amplitude.com', + is_default: false, + }, + ]); + }); + + it('emits the JSON envelope at a TTY when --json is passed', () => { + const path = seeded(); + const out: string[] = []; + runAuthList( + { json: true }, + { path, now: () => NOW, isTTY: true, stdout: (l) => out.push(l) }, + ); + const payload = JSON.parse(out.join('')); + expect(Array.isArray(payload.profiles)).toBe(true); + expect(payload.default).toBe('amplitude'); + }); + + it('reports an empty store as {profiles:[],default:null}, exit 0', () => { + const dir = mkdtempSync(join(tmpdir(), 'amp-list-empty-')); + dirs.push(dir); + const path = join(dir, 'credentials.json'); + saveStore(emptyStore(), path); + const out: string[] = []; + const original = process.exitCode; + runAuthList( + {}, + { path, now: () => NOW, isTTY: false, stdout: (l) => out.push(l) }, + ); + expect(JSON.parse(out.join(''))).toEqual({ profiles: [], default: null }); + expect(process.exitCode).toBe(original); + }); +}); + describe('runAuthUse', () => { const dirs: string[] = []; afterEach(() => { @@ -342,6 +478,149 @@ describe('runLogout', () => { runLogout({ all: true, profile: 'staging' }, { path: seeded() }), ).rejects.toThrow(/not both/); }); + + it('clears the pending login for the profile being logged out', async () => { + const path = seeded(); + const pendingPath = join(dirname(path), 'pending.json'); + savePending( + setPending( + setPending( + emptyPending(), + 'staging', + pendingEntry('https://developer-api.stag2.amplitude.com'), + ), + 'amplitude', + pendingEntry('https://developer-api.amplitude.com'), + ), + pendingPath, + ); + await runLogout( + { profile: 'staging' }, + { path, pendingPath, stdout: () => {} }, + ); + const after = loadPending(pendingPath); + expect(after.pending.staging).toBeUndefined(); + expect(after.pending.amplitude).toBeDefined(); + }); + + it('--all clears every pending login alongside the profiles', async () => { + const path = seeded(); + const pendingPath = join(dirname(path), 'pending.json'); + savePending( + setPending( + emptyPending(), + 'amplitude', + pendingEntry('https://developer-api.amplitude.com'), + ), + pendingPath, + ); + await runLogout( + { all: true, yes: true }, + { path, pendingPath, isTTY: false, stdout: () => {} }, + ); + expect(Object.keys(loadPending(pendingPath).pending)).toEqual([]); + }); + + it('cancels a pending login for a profile with no stored credential yet', async () => { + const dir = mkdtempSync(join(tmpdir(), 'amp-logout-pending-cancel-')); + dirs.push(dir); + const path = join(dir, 'credentials.json'); + const pendingPath = join(dir, 'pending.json'); + saveStore(emptyStore(), path); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('https://developer-api.amplitude.com'), + ), + pendingPath, + ); + const out: string[] = []; + await runLogout( + { profile: 'default' }, + { path, pendingPath, stdout: (l) => out.push(l) }, + ); + expect(getPending(loadPending(pendingPath), 'default')).toBeUndefined(); + expect(out.join('\n')).toMatch(/Canceled the in-progress login/); + }); + + it('bare logout cancels a solitary in-progress login with no default set', async () => { + const dir = mkdtempSync(join(tmpdir(), 'amp-logout-bare-pending-')); + dirs.push(dir); + const path = join(dir, 'credentials.json'); + const pendingPath = join(dir, 'pending.json'); + // Cold `login start`: a pending entry exists but store.default is unset. + saveStore(emptyStore(), path); + savePending( + setPending( + emptyPending(), + 'default', + pendingEntry('https://developer-api.amplitude.com'), + ), + pendingPath, + ); + const out: string[] = []; + await runLogout({}, { path, pendingPath, stdout: (l) => out.push(l) }); + expect(getPending(loadPending(pendingPath), 'default')).toBeUndefined(); + expect(out.join('\n')).toMatch(/Canceled the in-progress login/); + }); + + it('bare logout selects the sole live login, ignoring an expired sibling', async () => { + const dir = mkdtempSync(join(tmpdir(), 'amp-logout-live-plus-expired-')); + dirs.push(dir); + const path = join(dir, 'credentials.json'); + const pendingPath = join(dir, 'pending.json'); + saveStore(emptyStore(), path); + let store = emptyPending(); + store = setPending( + store, + 'live', + pendingEntry('https://developer-api.amplitude.com'), + ); + store = setPending(store, 'stale', { + ...pendingEntry('https://developer-api.amplitude.com'), + device_code: 'STALE', + expires_at: '2000-01-01T00:00:00Z', + }); + savePending(store, pendingPath); + const out: string[] = []; + await runLogout( + {}, + { + path, + pendingPath, + now: () => Date.parse('2026-07-16T00:00:00Z'), + stdout: (l) => out.push(l), + }, + ); + const after = loadPending(pendingPath); + expect(getPending(after, 'live')).toBeUndefined(); + expect(getPending(after, 'stale')?.device_code).toBe('STALE'); + expect(out.join('\n')).toMatch(/Canceled the in-progress login/); + }); + + it('--all clears pending even when there are no profiles', async () => { + const dir = mkdtempSync(join(tmpdir(), 'amp-logout-pending-only-')); + dirs.push(dir); + const path = join(dir, 'credentials.json'); + const pendingPath = join(dir, 'pending.json'); + saveStore(emptyStore(), path); + savePending( + setPending( + emptyPending(), + 'work', + pendingEntry('https://developer-api.amplitude.com'), + ), + pendingPath, + ); + const out: string[] = []; + await runLogout( + { all: true }, + { path, pendingPath, stdout: (l) => out.push(l) }, + ); + expect(Object.keys(loadPending(pendingPath).pending)).toEqual([]); + expect(out.join('\n')).toMatch(/Cleared 1 in-progress login/); + }); }); describe('maskToken', () => { @@ -380,6 +659,7 @@ describe('runAuthStatus', () => { store: statusStore(), now: () => NOW, env: {}, + isTTY: true, stdout: (l) => out.push(l), }, ); @@ -393,7 +673,7 @@ describe('runAuthStatus', () => { expect(process.exitCode).not.toBe(1); }); - it('emits plain text without ANSI when stdout is not a TTY', () => { + it('shows the Region line for a prod profile', () => { const out: string[] = []; runAuthStatus( {}, @@ -401,18 +681,42 @@ describe('runAuthStatus', () => { store: statusStore(), now: () => NOW, env: {}, - isTTY: false, + isTTY: true, stdout: (l) => out.push(l), }, ); - const text = out.join('\n'); + expect(out.join('\n')).toContain( + 'Region: US (https://app.amplitude.com/)', + ); + }); - expect(text).toMatch(/Auth status/); - expect(text).toMatch(/Source: {3}default profile amplitude/); - expect(text).not.toContain(`${String.fromCharCode(0x1b)}[`); + it('omits the Region line for an internal-env profile', () => { + const store = setDefault( + setProfile( + emptyStore(), + 'staging', + oauthProfile( + 'https://developer-api.stag2.amplitude.com', + '2026-06-23T14:00:00.000Z', + ), + ), + 'staging', + ); + const out: string[] = []; + runAuthStatus( + {}, + { + store, + now: () => NOW, + env: {}, + isTTY: true, + stdout: (l) => out.push(l), + }, + ); + expect(out.join('\n')).not.toContain('Region:'); }); - it('exits non-zero with guidance when nothing resolves', () => { + it('exits non-zero with guidance when nothing resolves, at a TTY', () => { const out: string[] = []; runAuthStatus( {}, @@ -420,12 +724,14 @@ describe('runAuthStatus', () => { store: emptyStore(), now: () => NOW, env: {}, + isTTY: true, stdout: (l) => out.push(l), }, ); expect(out.join('\n')).toMatch(/amp auth login/); - expect(process.exitCode).toBe(1); + // Same exit code as the JSON path for the same failure (authentication_required → 3). + expect(process.exitCode).toBe(3); }); it('announces AMP_TOKEN when it is in effect', () => { @@ -436,6 +742,7 @@ describe('runAuthStatus', () => { store: statusStore(), now: () => NOW, env: { AMP_TOKEN: 'env-token' }, + isTTY: true, stdout: (l) => out.push(l), }, ); @@ -446,7 +753,7 @@ describe('runAuthStatus', () => { expect(process.exitCode).not.toBe(1); }); - it('still shows the profile row (Expires: expired) and exits non-zero for an expired credential', () => { + it('still shows the profile row (Expires: expired) and exits non-zero for an expired credential, at a TTY', () => { const store = setDefault( setProfile( emptyStore(), @@ -461,7 +768,13 @@ describe('runAuthStatus', () => { const out: string[] = []; runAuthStatus( {}, - { store, now: () => NOW, env: {}, stdout: (l) => out.push(l) }, + { + store, + now: () => NOW, + env: {}, + isTTY: true, + stdout: (l) => out.push(l), + }, ); const text = out.join('\n'); @@ -470,7 +783,165 @@ describe('runAuthStatus', () => { expect(text).toMatch(/Expires: {2}expired/); expect(text).toMatch(/expired/i); expect(text).not.toContain('eyJ.jwt'); - expect(process.exitCode).toBe(1); + // An expired token resolves to invalid_token → exit 3, same as the JSON path. + expect(process.exitCode).toBe(3); + }); + + describe('JSON output (non-TTY or --json)', () => { + it('emits the authenticated envelope on stdout for the default profile, non-TTY', () => { + const out: string[] = []; + runAuthStatus( + {}, + { + store: statusStore(), + now: () => NOW, + env: {}, + isTTY: false, + stdout: (l) => out.push(l), + }, + ); + + expect(out).toHaveLength(1); + const payload = JSON.parse(out[0]); + expect(payload).toEqual({ + authenticated: true, + source: 'default profile amplitude', + profile: 'amplitude', + type: 'oauth', + base_url: 'https://developer-api.amplitude.com', + expires_at: '2026-06-23T14:00:00.000Z', + token: maskToken('eyJ.jwt'), + }); + expect(out.join('\n')).not.toContain('eyJ.jwt'); + expect(process.exitCode).not.toBe(1); + }); + + it('emits the JSON envelope at a TTY when --json is passed', () => { + const out: string[] = []; + runAuthStatus( + { json: true }, + { + store: statusStore(), + now: () => NOW, + env: {}, + isTTY: true, + stdout: (l) => out.push(l), + }, + ); + + const payload = JSON.parse(out.join('')); + expect(payload.authenticated).toBe(true); + expect(payload.type).toBe('oauth'); + }); + + it('omits expires_at for a pat profile and labels its type', () => { + const store = setDefault( + setProfile(emptyStore(), 'ci', { + base_url: 'https://developer-api.amplitude.com', + credential: { type: 'pat', pat: 'amp_x' }, + saved_at: '2026-06-23T00:00:00.000Z', + store: 'file', + }), + 'ci', + ); + const out: string[] = []; + runAuthStatus( + {}, + { + store, + now: () => NOW, + env: {}, + isTTY: false, + stdout: (l) => out.push(l), + }, + ); + + const payload = JSON.parse(out.join('')); + expect(payload.type).toBe('pat'); + expect(payload).not.toHaveProperty('expires_at'); + }); + + it('reports type "token" and no profile when AMP_TOKEN shadows the store', () => { + const out: string[] = []; + runAuthStatus( + {}, + { + store: statusStore(), + now: () => NOW, + env: { AMP_TOKEN: 'env-token-value' }, + isTTY: false, + stdout: (l) => out.push(l), + }, + ); + + const payload = JSON.parse(out.join('')); + expect(payload.authenticated).toBe(true); + expect(payload.source).toBe('AMP_TOKEN env var'); + expect(payload).not.toHaveProperty('profile'); + expect(payload.type).toBe('token'); + expect(payload.token).toBe(maskToken('env-token-value')); + expect(payload).not.toHaveProperty('expires_at'); + }); + + it('does not print to stdout and propagates a CliError when nothing resolves, non-TTY', () => { + const out: string[] = []; + let thrown: unknown; + try { + runAuthStatus( + {}, + { + store: emptyStore(), + now: () => NOW, + env: {}, + isTTY: false, + stdout: (l) => out.push(l), + }, + ); + } catch (error) { + thrown = error; + } + + expect(out).toHaveLength(0); + expect(thrown).toBeInstanceOf(CliError); + const cliError = thrown as CliError; + expect(cliError.errorCode).toBe('authentication_required'); + expect(cliError.exitCode).toBe(3); + }); + + it('propagates an invalid_token CliError (not prose) for an expired stored credential, non-TTY', () => { + const store = setDefault( + setProfile( + emptyStore(), + 'amplitude', + oauthProfile( + 'https://developer-api.amplitude.com', + '2026-06-23T10:00:00.000Z', + ), + ), + 'amplitude', + ); + const out: string[] = []; + let thrown: unknown; + try { + runAuthStatus( + {}, + { + store, + now: () => NOW, + env: {}, + isTTY: false, + stdout: (l) => out.push(l), + }, + ); + } catch (error) { + thrown = error; + } + + expect(out).toHaveLength(0); + expect(thrown).toBeInstanceOf(CliError); + expect((thrown as CliError).errorCode).toBe('invalid_token'); + expect((thrown as CliError).exitCode).toBe(3); + }); }); }); diff --git a/src/authToken.test.ts b/src/authToken.test.ts index 3a33bdd..dfaed41 100644 --- a/src/authToken.test.ts +++ b/src/authToken.test.ts @@ -7,6 +7,7 @@ import { createAnonymousRequest, formatVerificationPrompt, generatePkcePair, + pollDeviceTokenBounded, pollForToken, requestDeviceToken, runAuthTokenCommand, @@ -413,6 +414,25 @@ describe('createAnonymousRequest', () => { const [url, init] = fetchMock.mock.calls[0]; expect(url).toBe('https://api.test/v1/auth/token'); expect(init?.headers).not.toHaveProperty('Authorization'); + // Identifies the API client to the server for analytics. + expect(new Headers(init?.headers).get('user-agent')).toMatch( + /^amp-cli\/\S+$/, + ); + // No device id was supplied, so no header is sent. + expect(new Headers(init?.headers).get('amp-device-id')).toBeNull(); + }); + + it('sends the persistent device_id as the Amp-Device-Id header', async () => { + const fetchMock = vi.fn(() => + Promise.resolve(new Response('{}', { status: 200 })), + ); + vi.stubGlobal('fetch', fetchMock); + + const request = createAnonymousRequest('https://api.test', 'install-xyz'); + await request('POST', '/v1/auth/device-authorization', { x: 1 }); + + const [, init] = fetchMock.mock.calls[0]; + expect(new Headers(init?.headers).get('amp-device-id')).toBe('install-xyz'); }); it('throws a readable error when the API is unreachable', async () => { @@ -447,3 +467,158 @@ describe('createAnonymousRequest', () => { }); }); }); + +const noSleep = async () => {}; +const token = { access_token: 'at', token_type: 'bearer', expires_in: 3600 }; + +describe('pollDeviceTokenBounded', () => { + it('returns authorized when the exchange succeeds', async () => { + const res = await pollDeviceTokenBounded({ + request: async () => ({ status: 200, body: token }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 10_000, + timeoutSeconds: 5, + now: () => 0, + sleep: noSleep, + }); + expect(res).toEqual({ status: 'authorized', token }); + }); + + it('returns pending when the bounded timeout elapses but the code is still valid', async () => { + let t = 0; + const res = await pollDeviceTokenBounded({ + request: async () => ({ + status: 400, + body: { error: 'authorization_pending' }, + }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 1_000_000, + timeoutSeconds: 2, + now: () => (t += 1000), + sleep: noSleep, + }); + expect(res).toEqual({ status: 'pending', interval: 1 }); + }); + + it('stops polling once a full interval no longer fits the timeout budget', async () => { + // Clock advances only by what we sleep, so wall-clock == sum(sleeps). + const sleeps: number[] = []; + let calls = 0; + const res = await pollDeviceTokenBounded({ + request: async () => { + calls += 1; + return { status: 400, body: { error: 'authorization_pending' } }; + }, + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 5, + codeExpiresAtMs: 1_000_000, + timeoutSeconds: 12, + now: () => sleeps.reduce((sum, s) => sum + s, 0) * 1000, + sleep: (s) => { + sleeps.push(s); + return Promise.resolve(); + }, + }); + // Polls at t=0 and t=5 (each +5s still fits the 12s budget), then at t=10 a + // further 5s sleep would overshoot to 15s — so it returns pending instead of + // sleeping past --timeout. Total slept 10s, never exceeding the budget. + expect(res).toEqual({ status: 'pending', interval: 5 }); + expect(sleeps).toEqual([5, 5]); + expect(calls).toBe(3); + }); + + it('retries transient server_error within the window (pending, not error)', async () => { + let t = 0; + const res = await pollDeviceTokenBounded({ + request: async () => ({ status: 500, body: { error: 'server_error' } }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 1_000_000, + timeoutSeconds: 2, + now: () => (t += 1000), + sleep: noSleep, + }); + expect(res).toEqual({ status: 'pending', interval: 1 }); + }); + + it('raises the interval on slow_down and reports it on pending', async () => { + let t = 0; + const res = await pollDeviceTokenBounded({ + request: async () => ({ status: 400, body: { error: 'slow_down' } }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 1_000_000, + timeoutSeconds: 2, + now: () => (t += 1000), + sleep: noSleep, + }); + expect(res).toEqual({ status: 'pending', interval: 6 }); + }); + + it('returns expired when the device code lifetime passes', async () => { + let t = 0; + const res = await pollDeviceTokenBounded({ + request: async () => ({ + status: 400, + body: { error: 'authorization_pending' }, + }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 1500, + timeoutSeconds: 999, + now: () => (t += 1000), + sleep: noSleep, + }); + expect(res).toEqual({ status: 'expired' }); + }); + + it('returns error on access_denied', async () => { + const res = await pollDeviceTokenBounded({ + request: async () => ({ status: 400, body: { error: 'access_denied' } }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 10_000, + timeoutSeconds: 5, + now: () => 0, + sleep: noSleep, + }); + expect(res.status).toBe('error'); + }); + + it('preserves the structured OAuth error on a non-expired_token error', async () => { + const res = await pollDeviceTokenBounded({ + request: async () => ({ + status: 400, + body: { + error: 'access_denied', + error_description: 'The user denied the request.', + error_hint: 'Ask the user to approve the code.', + }, + }), + deviceCode: 'dc', + codeVerifier: 'cv', + intervalSeconds: 1, + codeExpiresAtMs: 10_000, + timeoutSeconds: 5, + now: () => 0, + sleep: noSleep, + }); + expect(res).toEqual({ + status: 'error', + error: { + code: 'access_denied', + description: 'The user denied the request.', + hint: 'Ask the user to approve the code.', + }, + }); + }); +}); diff --git a/src/authToken.ts b/src/authToken.ts index bed4279..9c7bf1e 100644 --- a/src/authToken.ts +++ b/src/authToken.ts @@ -5,6 +5,8 @@ import { setTimeout as delay } from 'node:timers/promises'; import open from 'open'; import { z } from 'zod'; +import { usageError } from './cli-error'; +import { CLI_VERSION } from './help'; import { toOAuthError } from './oauthError'; import { type DeviceAuthorizationResponse, @@ -36,7 +38,7 @@ function parseResponse( return result.data; } - throw new Error( + throw new Error( // plain-error-ok: device-flow errors are caught and re-emitted as a JSON envelope by `auth login start`/`poll`; only the interactive TTY `auth login` surfaces them as prose. `The authorization server returned an unexpected ${description} response.`, ); } @@ -73,13 +75,13 @@ export function formatVerificationPrompt( device: DeviceAuthorizationResponse, ): string { return [ - 'To authorize, confirm this code in your browser:', + 'To authorize, confirm this code:', '', ` ${device.user_code}`, '', 'At the following url:', '', - ` ${verificationUrl(device)}`, + verificationUrl(device), ].join('\n'); } @@ -265,7 +267,7 @@ export async function pollForToken({ // RFC 8628 §3.5: slow_down permanently raises the interval by 5s. interval += 5; } else if (code !== 'authorization_pending') { - throw new Error(oauthErrorMessage(body)); + throw new Error(oauthErrorMessage(body)); // plain-error-ok: device-flow errors are caught and re-emitted as a JSON envelope by `auth login start`/`poll`; only the interactive TTY `auth login` surfaces them as prose. } // Give up only after a poll has just come back pending/slow_down and the @@ -273,13 +275,103 @@ export async function pollForToken({ // attempt that lands on the deadline still runs and can return a token the // user approved during the preceding sleep. if (deadline && deadlineMs !== undefined && deadline.now() >= deadlineMs) { - throw new Error('The device code has expired before approval.'); + throw new Error('The device code has expired before approval.'); // plain-error-ok: device-flow errors are caught and re-emitted as a JSON envelope by `auth login start`/`poll`; only the interactive TTY `auth login` surfaces them as prose. } await sleep(interval); } } +export type PollDeviceResult = + | { status: 'authorized'; token: TokenResponse } + // `interval` is the current poll interval (raised by any `slow_down`), so the + // caller can persist it — RFC 8628 §3.5 requires the raised interval to apply + // to all subsequent requests, including later poll invocations. + | { status: 'pending'; interval: number } + | { status: 'expired' } + | { + status: 'error'; + error: { code: string; description?: string; hint?: string }; + }; + +// RFC 6749 §5.2 transient errors — retry within the bounded window rather than +// giving up. `server_error` is also the fallback for a non-RFC/non-JSON body +// (see toOAuthError), so a transient token-endpoint blip never destroys a valid +// device code. +const TRANSIENT_OAUTH_ERRORS = new Set([ + 'server_error', + 'temporarily_unavailable', +]); + +interface PollDeviceBoundedOptions { + request: AnonymousRequest; + deviceCode: string; + codeVerifier: string; + intervalSeconds: number; + codeExpiresAtMs: number; + timeoutSeconds: number; + now: () => number; + sleep: (seconds: number) => Promise; +} + +// Bounded variant of pollForToken: instead of throwing on deadline, returns a +// result union so the caller can distinguish "my bounded timeout elapsed but +// the code is still valid" (pending — safe to resume polling later) from "the +// device code itself expired" (expired — the user must restart the flow). +export async function pollDeviceTokenBounded( + options: PollDeviceBoundedOptions, +): Promise { + let interval = options.intervalSeconds; + const timeoutDeadlineMs = options.now() + options.timeoutSeconds * 1000; + + for (;;) { + const { status, body } = await options.request('POST', '/v1/auth/token', { + grant_type: DEVICE_CODE_GRANT_TYPE, + device_code: options.deviceCode, + code_verifier: options.codeVerifier, + }); + + if (isOk(status)) { + return { + status: 'authorized', + token: parseResponse(tokenResponseSchema, body, 'token'), + }; + } + + const oauthError = toOAuthError(body); + if (oauthError.error === 'slow_down') { + // RFC 8628 §3.5: slow_down permanently raises the interval by 5s. + interval += 5; + } else if ( + oauthError.error !== 'authorization_pending' && + !TRANSIENT_OAUTH_ERRORS.has(oauthError.error) + ) { + return oauthError.error === 'expired_token' + ? { status: 'expired' } + : { + status: 'error', + error: { + code: oauthError.error, + description: oauthError.error_description ?? undefined, + hint: oauthError.error_hint ?? undefined, + }, + }; + } + + if (options.now() >= options.codeExpiresAtMs) { + return { status: 'expired' }; + } + // Only sleep + re-poll when a full RFC interval still fits before the + // deadline. Otherwise return pending now: sleeping the interval would + // overshoot the caller's --timeout budget, and a shorter sleep would poll + // faster than the interval RFC 8628 §3.5 mandates. + if (options.now() + interval * 1000 >= timeoutDeadlineMs) { + return { status: 'pending', interval }; + } + await options.sleep(interval); + } +} + export type AnonymousRequest = ( method: string, path: string, @@ -292,9 +384,21 @@ export type AnonymousRequest = ( // getJson/requestJson, never throws on a non-2xx — it returns the status and // parsed body for the poll loop to interpret. A non-JSON body (e.g. an HTML // error page) degrades to the raw text rather than throwing a parse error. -export function createAnonymousRequest(baseUrl: string): AnonymousRequest { +export function createAnonymousRequest( + baseUrl: string, + deviceId?: string, +): AnonymousRequest { return async (method, path, body) => { - const headers: Record = { Accept: 'application/json' }; + // Client-context headers, stamped on analytics server-side and sent on every + // request: `User-Agent` identifies the API client + version; `Amp-Device-Id` + // (when known) carries the persistent install id so the anonymous identity + // is stable across invocations. Headers, not the body, so they can't affect + // request validation. + const headers: Record = { + Accept: 'application/json', + 'User-Agent': `amp-cli/${CLI_VERSION}`, + ...(deviceId ? { 'Amp-Device-Id': deviceId } : {}), + }; if (body !== undefined) { headers['Content-Type'] = 'application/json'; } @@ -308,7 +412,7 @@ export function createAnonymousRequest(baseUrl: string): AnonymousRequest { try { response = await fetch(`${baseUrl}${path}`, init); } catch { - throw new Error(`Could not reach the API at ${baseUrl}.`); + throw new Error(`Could not reach the API at ${baseUrl}.`); // plain-error-ok: device-flow errors are caught and re-emitted as a JSON envelope by `auth login start`/`poll`; only the interactive TTY `auth login` surfaces them as prose. } const text = await response.text(); @@ -327,6 +431,8 @@ export interface DeviceFlowOptions { flow: string | undefined; scope?: string; // Token-optional, non-throwing HTTP call against the Developer API endpoints. + // The persistent device_id (when known) is carried as a header baked into this + // request by createAnonymousRequest — not a device-flow option. request: AnonymousRequest; sleep?: (seconds: number) => Promise; now?: () => number; @@ -352,11 +458,11 @@ export async function requestDeviceToken( options: DeviceFlowOptions, ): Promise { if (!options.flow) { - throw new Error('Missing --flow. The only supported value is "device".'); + throw usageError('Missing --flow. The only supported value is "device".'); } if (options.flow !== 'device') { - throw new Error( + throw usageError( `Unsupported --flow "${options.flow}". The only supported value is "device".`, ); } @@ -380,7 +486,7 @@ export async function requestDeviceToken( ); if (!isOk(deviceResponse.status)) { - throw new Error(oauthErrorMessage(deviceResponse.body)); + throw new Error(oauthErrorMessage(deviceResponse.body)); // plain-error-ok: device-flow errors are caught and re-emitted as a JSON envelope by `auth login start`/`poll`; only the interactive TTY `auth login` surfaces them as prose. } const deviceAuthorization = parseResponse( diff --git a/src/catalog-shape.test.ts b/src/catalog-shape.test.ts new file mode 100644 index 0000000..6629a15 --- /dev/null +++ b/src/catalog-shape.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { buildCatalog } from './catalog'; +import { printCommandHelp, serializeCatalog } from './help'; + +const INDEX_ALLOWLIST = [ + 'command', + 'group', + 'requiredScopes', + 'summary', +].sort(); + +// Tripwire: the fields below are the only ones a per-command detail object is +// allowed to serialize. `order` and `destructive` must never appear — the +// latter would be method-derived, not authoritative/sourced from the OpenAPI +// spec. Adding a new serialized field (the next `destructive`) fails this test, +// forcing a deliberate "is this field authoritative/sourced, not invented?" +// decision in review before it ships on the catalog's public JSON surface. +const DETAIL_ALLOWLIST = [ + 'command', + 'description', + 'example', + 'flags', + 'group', + 'requiredScopes', + 'summary', +]; + +// Fields the catalog once carried and deliberately removed; they must never +// reappear on the serialized surface. `agentNote` was audience-targeting copy; +// `destructive`/`order` are non-authoritative/presentation-only. +const FORBIDDEN_DETAIL_FIELDS = ['agentNote', 'destructive', 'order']; + +function captureJson(fn: () => void) { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + fn(); + return JSON.parse(log.mock.calls.map((call) => String(call[0])).join('')); + } finally { + log.mockRestore(); + } +} + +describe('catalog shape — serialized field allowlist', () => { + it('every index entry exposes exactly command/group/requiredScopes/summary', () => { + const dump = serializeCatalog(); + expect(dump.commands.length).toBeGreaterThan(0); + + for (const entry of dump.commands) { + expect( + Object.keys(entry).sort(), + `index entry \`${entry.command}\` has unexpected keys`, + ).toEqual(INDEX_ALLOWLIST); + } + }); + + it('every command detail object carries only allowlisted fields', () => { + for (const command of buildCatalog()) { + const detail = captureJson(() => + printCommandHelp(command.command, { json: true, isTTY: false }), + ); + const keys = Object.keys(detail); + const label = command.command.join(' '); + const unknownKeys = keys.filter((key) => !DETAIL_ALLOWLIST.includes(key)); + expect(unknownKeys, `\`${label}\` serializes unknown field(s)`).toEqual( + [], + ); + for (const forbidden of FORBIDDEN_DETAIL_FIELDS) { + expect( + keys, + `\`${label}\` must not serialize ${forbidden}`, + ).not.toContain(forbidden); + } + } + }); +}); diff --git a/src/catalog.test.ts b/src/catalog.test.ts new file mode 100644 index 0000000..82e6189 --- /dev/null +++ b/src/catalog.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from 'vitest'; + +import { buildCatalog } from './catalog'; + +describe('buildCatalog — API operations', () => { + it('includes the charts surface (regression: charts was hidden from help)', () => { + const groups = new Set(buildCatalog().map((c) => c.group)); + expect(groups.has('charts')).toBe(true); + }); + + it('maps flags with type and required but no description (API commands are undescribed in this PR)', () => { + const list = buildCatalog().find( + (c) => c.command.join(' ') === 'projects list', + ); + const limit = list?.flags.find((f) => f.name === 'limit'); + expect(limit?.type).toBe('integer'); + expect(limit?.required).toBe(false); + expect(limit?.description).toBeUndefined(); + }); + + it('carries an enum through from a body property to the catalog flag', () => { + const create = buildCatalog().find( + (c) => c.command.join(' ') === 'event-properties create', + ); + const dataType = create?.flags.find((f) => f.name === 'data_type'); + expect(dataType?.enum).toEqual([ + 'string', + 'number', + 'boolean', + 'object', + 'enum', + 'any', + ]); + }); +}); + +describe('buildCatalog — auth/meta commands', () => { + it('describes auth login start behaviorally, with no agentNote field', () => { + const start = buildCatalog().find( + (c) => c.command.join(' ') === 'auth login start', + ); + expect(start?.group).toBe('auth'); + expect(start?.description).toMatch(/device authorization/); + expect(start?.description).toMatch(/poll/); + for (const entry of buildCatalog()) { + expect(entry).not.toHaveProperty('agentNote'); + } + }); + + it('carries the hand-authored description for an auth flag', () => { + const start = buildCatalog().find( + (c) => c.command.join(' ') === 'auth login start', + ); + const region = start?.flags.find((f) => f.name === 'region'); + expect(region?.description).toBeDefined(); + expect(region?.description).toMatch(/region/); + }); + + it('covers every auth/meta command the CLI routes', () => { + // `version` and `help` are also routed by cli.ts but are intentionally NOT + // catalog entries — they're documented in the global-help header instead, + // so this parity list is deliberately auth/meta-command-scoped. + const commands = new Set(buildCatalog().map((c) => c.command.join(' '))); + for (const routed of [ + 'auth login', + 'auth login start', + 'auth login poll', + 'auth pat', + 'auth use', + 'auth list', + 'auth status', + 'auth token', + 'logout', + ]) { + expect(commands.has(routed)).toBe(true); + } + }); +}); diff --git a/src/catalog.ts b/src/catalog.ts new file mode 100644 index 0000000..b0ef1a0 --- /dev/null +++ b/src/catalog.ts @@ -0,0 +1,334 @@ +import { DEFAULT_POLL_TIMEOUT_SECONDS } from './config'; +import type { CliOperation } from './generated/cli-manifest'; +import { CLI_OPERATIONS } from './generated/cli-manifest'; + +export interface CatalogFlag { + name: string; + aliases: string[]; + type: string; + required: boolean; + enum?: string[]; + description?: string; +} + +export interface CatalogCommand { + command: string[]; + summary: string; + description?: string; + group: string; + order?: number; + flags: CatalogFlag[]; + example?: string; + requiredScopes: string[]; +} + +// Curated presentation overlay. Generation fills the rest; curation wins where +// present (design rule: generation augments curation, never replaces it). +const GROUP_ORDER: Partial> = { + context: 0, + projects: 1, + events: 2, + 'event-properties': 3, + 'user-properties': 4, + flags: 5, + charts: 6, + auth: 20, +}; + +const GROUP_DESCRIPTIONS: Partial> = { + context: 'Authenticated user and org context', + projects: 'Projects in your organization', + events: 'Event taxonomy', + 'event-properties': 'Properties on events', + 'user-properties': 'User properties', + flags: 'Feature flags', + charts: 'Saved and ad-hoc charts', + auth: 'Authentication and credential profiles', +}; + +const EXAMPLES: Partial> = { + context: 'amp context', + 'projects list': 'amp projects list --limit 10', + 'events list': 'amp events list --project --limit 5', + 'events create': + 'amp events create --project --event-type my_event', + 'events get': 'amp events get --project --event ', + 'flags list': 'amp flags list --project --limit 5', + 'flags create': + 'amp flags create --project --key my-flag --name "My Flag"', + 'flags get': 'amp flags get --project --flag ', + 'flags archive': + 'amp flags archive --project --flag --dry-run', +}; + +// Uncurated groups get a stable order *after* curated ones (alphabetical), +// so a new API surface still appears in help — just without a hand-picked slot. +const UNCURATED_ORDER_BASE = 100; + +function flagsForOperation(operation: CliOperation): CatalogFlag[] { + const fromParams = operation.parameters + .filter((parameter) => parameter.in !== 'header') + .map((parameter) => ({ + name: parameter.name, + aliases: parameter.aliases, + type: parameter.type, + required: parameter.required, + })); + const fromBody = operation.body.map((property) => ({ + name: property.name, + aliases: property.aliases, + type: property.type, + required: property.required, + enum: property.enum, + })); + return [...fromParams, ...fromBody]; +} + +function apiCommands(): CatalogCommand[] { + return CLI_OPERATIONS.map((operation) => { + const group = operation.command[0]; + const key = operation.command.join(' '); + return { + command: operation.command, + summary: operation.summary ?? 'Call the Developer API.', + group, + order: GROUP_ORDER[group], + flags: flagsForOperation(operation), + example: EXAMPLES[key], + requiredScopes: operation.requiredScopes, + }; + }); +} + +// Auth/meta commands are bespoke routing (excluded from the OpenAPI manifest by +// the generator's `Auth` tag filter), so they are hand-authored here in the same +// shape. Keep this in sync with the routing in cli.ts — catalog.test.ts's +// "covers every auth/meta command the CLI routes" test checks parity against a +// manually-maintained list, so update that list too. There is no automated +// routing-derived parity check. +// +// auth-flag-coverage.test.ts enforces that handlers only read declared/global +// flags, so the misplaced-flag reject can't false-positive. +const AUTH_COMMANDS: CatalogCommand[] = [ + { + command: ['auth', 'login'], + summary: 'Authenticate and save a profile (interactive device flow)', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [ + { + name: 'region', + aliases: ['region'], + type: 'string', + required: true, + enum: ['us', 'eu'], + description: 'Target region (us|eu); sets the base URL.', + }, + { + name: 'profile', + aliases: ['profile'], + type: 'string', + required: false, + description: 'Profile name; omit to use the implicit default profile.', + }, + { + name: 'force', + aliases: ['force'], + type: 'boolean', + required: false, + description: + 'Overwrite the profile when it already targets a different region/base URL, skipping the confirm prompt.', + }, + ], + example: 'amp auth login --region us', + requiredScopes: [], + }, + { + command: ['auth', 'login', 'start'], + summary: 'Begin the device flow (emits a JSON envelope)', + group: 'auth', + order: GROUP_ORDER.auth, + description: + 'Begins OAuth device authorization: returns a verification URL, user code, and expiry, and records a pending authorization. The authorization is completed out of band by confirming the code; `amp auth login poll` returns the resulting credential.', + flags: [ + { + name: 'region', + aliases: ['region'], + type: 'string', + required: true, + enum: ['us', 'eu'], + description: 'Target region (us|eu).', + }, + { + name: 'profile', + aliases: ['profile'], + type: 'string', + required: false, + description: 'Profile name; defaults to the implicit default profile.', + }, + { + name: 'force', + aliases: ['force'], + type: 'boolean', + required: false, + description: + 'Overwrite the profile when it already targets a different region/base URL (the retarget is refused without it).', + }, + ], + example: 'amp auth login start --region us --json', + requiredScopes: [], + }, + { + command: ['auth', 'login', 'poll'], + summary: + 'Complete the device flow started by `login start` (emits a JSON envelope)', + group: 'auth', + order: GROUP_ORDER.auth, + description: `Completes the device authorization started by \`amp auth login start\`: returns the credential once the code is confirmed, or reports that authorization is still pending or the code has expired. Each call blocks up to ~${DEFAULT_POLL_TIMEOUT_SECONDS}s (\`--timeout \`, or \`--timeout 0\` for a single check).`, + flags: [ + { + name: 'profile', + aliases: ['profile'], + type: 'string', + required: false, + description: + 'Profile being authorized; defaults to the implicit default.', + }, + { + name: 'timeout', + aliases: ['timeout'], + type: 'integer', + required: false, + description: + 'Max seconds to block this call. 0 = one check and return.', + }, + ], + example: 'amp auth login poll --json', + requiredScopes: [], + }, + { + command: ['auth', 'pat'], + summary: 'Save a supplied Personal Access Token as a profile', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [ + { + name: 'with-token', + aliases: ['with-token'], + type: 'boolean', + required: true, + description: + 'Read the PAT from stdin (piped) or a masked prompt (TTY).', + }, + { + name: 'region', + aliases: ['region'], + type: 'string', + required: true, + enum: ['us', 'eu'], + description: 'Target region (us|eu).', + }, + { + name: 'profile', + aliases: ['profile'], + type: 'string', + required: false, + description: 'Profile name; defaults to the implicit default profile.', + }, + { + name: 'force', + aliases: ['force'], + type: 'boolean', + required: false, + description: + 'Overwrite the profile when it already targets a different region/base URL (the retarget is refused without it).', + }, + ], + example: 'echo "$PAT" | amp auth pat --with-token --region us', + requiredScopes: [], + }, + { + command: ['auth', 'use'], + summary: 'Switch the active profile (no re-auth)', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [], + example: 'amp auth use eu', + requiredScopes: [], + }, + { + command: ['auth', 'list'], + summary: 'List profiles (* marks the active one)', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [], + example: 'amp auth list', + requiredScopes: [], + }, + { + command: ['auth', 'status'], + summary: 'Show the active credential, base URL, and expiry', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [], + example: 'amp auth status', + requiredScopes: [], + }, + { + command: ['auth', 'token'], + summary: 'Print the active access token to stdout', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [], + example: 'TOKEN=$(amp auth token)', + requiredScopes: [], + }, + { + command: ['logout'], + summary: 'Remove a profile (or all with --all)', + group: 'auth', + order: GROUP_ORDER.auth, + flags: [ + { + name: 'profile', + aliases: ['profile'], + type: 'string', + required: false, + description: 'Profile to remove; defaults to the active one.', + }, + { + name: 'all', + aliases: ['all'], + type: 'boolean', + required: false, + description: 'Remove every stored profile.', + }, + ], + example: 'amp logout --profile eu', + requiredScopes: [], + }, +]; + +export function buildCatalog(): CatalogCommand[] { + return [...apiCommands(), ...AUTH_COMMANDS]; +} + +export function catalogGroups(): { + group: string; + description: string; + order: number; +}[] { + const seen = new Map(); + for (const entry of buildCatalog()) { + if (!seen.has(entry.group)) { + seen.set(entry.group, entry.order ?? UNCURATED_ORDER_BASE); + } + } + return [...seen.entries()] + .map(([group, order]) => ({ + group, + order, + description: GROUP_DESCRIPTIONS[group] ?? 'Developer API commands', + })) + .sort((a, b) => a.order - b.order || a.group.localeCompare(b.group)); +} diff --git a/src/cli-error-usage.test.ts b/src/cli-error-usage.test.ts new file mode 100644 index 0000000..b554263 --- /dev/null +++ b/src/cli-error-usage.test.ts @@ -0,0 +1,77 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import { extname, join, relative } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +/** + * Tripwire: user-facing failures in this CLI must throw a structured + * `CliError` (`usageError`/`authError`/`transportError`), never a bare + * `Error` — a plain `Error` reaching `main()`'s catch renders as the + * generic `error_code:"error"` / exit 1, which breaks the differentiated + * exit-code contract (`error-contract.test.ts`). + * + * This scans every non-test source file for `throw new Error(` and requires + * a `// plain-error-ok: ` marker on the same line for the rare + * genuine exception (e.g. a TTY-only cancellation that can't reach an agent + * non-interactively). Anything unannotated fails, naming file:line, so a new + * bare `throw new Error(...)` can't sneak back in unnoticed. + * + * Limitation: this is a textual match on the literal `throw new Error(`. + * Aliasing it away (`const e = new Error('x'); throw e;`, or a factory that + * returns/throws a plain `Error`) evades the scan. The convention is to + * annotate the flagged line, not to alias around the guard. + */ + +const SRC_DIR = join(__dirname); + +function listSourceFiles(dir: string): string[] { + const entries = readdirSync(dir, { withFileTypes: true }); + return entries.flatMap((entry) => { + const fullPath = join(dir, entry.name); + if (entry.isDirectory()) { + return listSourceFiles(fullPath); + } + if ( + extname(entry.name) === '.ts' && + !entry.name.endsWith('.test.ts') && + !entry.name.endsWith('.d.ts') + ) { + return [fullPath]; + } + return []; + }); +} + +function unannotatedThrowSites(): string[] { + const offenders: string[] = []; + for (const file of listSourceFiles(SRC_DIR)) { + const lines = readFileSync(file, 'utf8').split('\n'); + lines.forEach((line, index) => { + if (!/\bthrow new Error\(/.test(line)) { + return; + } + if (/\/\/\s*plain-error-ok:/.test(line)) { + return; + } + offenders.push(`${relative(SRC_DIR, file)}:${index + 1}`); + }); + } + return offenders; +} + +describe('CliError usage guard', () => { + it('every `throw new Error(` is either converted to CliError or explicitly sanctioned', () => { + const offenders = unannotatedThrowSites(); + expect( + offenders, + offenders.length + ? [ + 'user-facing failures must throw CliError (usageError/authError/transportError);', + 'if a bare Error is genuinely correct, annotate with `// plain-error-ok: `.', + 'Offending site(s):', + ...offenders.map((site) => ` - ${site}`), + ].join('\n') + : undefined, + ).toEqual([]); + }); +}); diff --git a/src/cli-error.test.ts b/src/cli-error.test.ts new file mode 100644 index 0000000..6c07bd0 --- /dev/null +++ b/src/cli-error.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, it } from 'vitest'; + +import { + CliError, + cliErrorFromResponse, + exitCodeForErrorCode, + formatErrorEnvelope, + formatErrorText, + usageError, +} from './cli-error'; + +describe('CliError', () => { + it('maps error codes to exit codes', () => { + expect(exitCodeForErrorCode('insufficient_scope', 403)).toBe(3); + expect(exitCodeForErrorCode('validation_error', 422)).toBe(2); + expect(exitCodeForErrorCode('not_found', 404)).toBe(4); + expect(exitCodeForErrorCode('upstream_error', 502)).toBe(5); + expect(exitCodeForErrorCode('something_new', 418)).toBe(1); + }); + + it('builds a CliError from an API problem+json body', () => { + const err = cliErrorFromResponse(403, 'Forbidden', { + error_code: 'insufficient_scope', + title: 'Forbidden', + detail: 'Missing scope write:flags', + }); + expect(err.errorCode).toBe('insufficient_scope'); + expect(err.exitCode).toBe(3); + expect(err.httpStatus).toBe(403); + expect(err.hint).toMatch(/amp context/); + }); + + it('serializes the agreed envelope shape', () => { + const env = JSON.parse( + formatErrorEnvelope(usageError('Missing --project .'), false), + ); + expect(env.status).toBe('error'); + expect(env.error.error_code).toBe('usage_error'); + expect(typeof env.message).toBe('string'); + }); + + it('captures a non-problem string body into detail (raw-body fallback)', () => { + const err = cliErrorFromResponse( + 502, + 'Bad Gateway', + 'gateway unavailable', + ); + expect(err.detail).toContain('gateway unavailable'); + }); + + it('captures a non-problem object body into detail as JSON', () => { + const err = cliErrorFromResponse(500, 'Internal Server Error', { + message: 'boom', + }); + expect(err.detail).toContain('"boom"'); + }); + + it('leaves detail undefined for a null body', () => { + const err = cliErrorFromResponse(500, 'Internal Server Error', null); + expect(err.detail).toBeUndefined(); + }); +}); + +describe('formatErrorText', () => { + it('formats RFC 7807 problem responses with hints', () => { + const err = cliErrorFromResponse(403, 'Forbidden', { + title: 'Insufficient scope', + detail: 'Token missing required scopes.', + error_code: 'insufficient_scope', + }); + + const text = formatErrorText(err); + expect(text).toContain( + 'Insufficient scope: Token missing required scopes.', + ); + expect(text).toContain('amp context'); + }); + + it('includes validation errors when present', () => { + const err = cliErrorFromResponse(400, 'Bad Request', { + title: 'Validation failed', + error_code: 'validation_error', + validation_errors: [ + { field: 'event_type', message: 'Required', code: 'required' }, + ], + }); + + const text = formatErrorText(err); + expect(text).toContain('Validation errors:'); + expect(text).toContain(' - event_type: Required'); + expect(text).toContain('amp --help'); + }); + + it('falls back to showing a non-problem body', () => { + const err = cliErrorFromResponse(500, 'Internal Server Error', { + message: 'boom', + }); + + const text = formatErrorText(err); + expect(text).toContain('500'); + expect(text).toContain('"boom"'); + }); + + it('prints non-JSON error bodies directly', () => { + const err = cliErrorFromResponse( + 502, + 'Bad Gateway', + 'gateway unavailable', + ); + + const text = formatErrorText(err); + expect(text).toContain('502'); + expect(text).toContain('gateway unavailable'); + }); + + it('renders message, validation errors, and hint in order', () => { + const err = new CliError({ + message: 'Validation failed', + errorCode: 'validation_error', + exitCode: 2, + validationErrors: [ + { field: 'a', message: 'A is required', code: 'required' }, + { field: 'b', message: 'B is invalid', code: 'invalid' }, + ], + hint: 'Check required flags with `amp --help`.', + }); + + expect(formatErrorText(err)).toBe( + [ + 'Validation failed', + '', + 'Validation errors:', + ' - a: A is required', + ' - b: B is invalid', + '', + 'Check required flags with `amp --help`.', + ].join('\n'), + ); + }); + + it('omits the validation/hint sections when absent', () => { + const err = new CliError({ + message: 'Something broke', + errorCode: 'error', + exitCode: 1, + }); + + expect(formatErrorText(err)).toBe('Something broke'); + }); +}); diff --git a/src/cli-error.ts b/src/cli-error.ts new file mode 100644 index 0000000..e3df762 --- /dev/null +++ b/src/cli-error.ts @@ -0,0 +1,160 @@ +import { asProblem, hintForErrorCode } from './errors'; +import { formatJsonOutput } from './output'; + +export interface CliErrorFields { + errorCode: string; + exitCode: number; + httpStatus?: number; + detail?: string; + validationErrors?: Array<{ field: string; message: string; code: string }>; + hint?: string; +} + +const AUTH_CODES = new Set([ + 'authentication_required', + 'invalid_token', + 'insufficient_scope', +]); +const TRANSPORT_CODES = new Set(['auth_unavailable', 'upstream_error']); + +export function exitCodeForErrorCode( + errorCode: string, + httpStatus?: number, +): number { + if ( + errorCode === 'validation_error' || + errorCode === 'usage_error' || + httpStatus === 400 || + httpStatus === 422 + ) { + return 2; + } + if (AUTH_CODES.has(errorCode) || httpStatus === 401 || httpStatus === 403) { + return 3; + } + if (errorCode === 'not_found' || httpStatus === 404) { + return 4; + } + if ( + TRANSPORT_CODES.has(errorCode) || + (httpStatus !== undefined && httpStatus >= 500) + ) { + return 5; + } + return 1; +} + +export class CliError extends Error implements CliErrorFields { + errorCode: string; + exitCode: number; + httpStatus?: number; + detail?: string; + validationErrors?: Array<{ field: string; message: string; code: string }>; + hint?: string; + + constructor(fields: CliErrorFields & { message: string }) { + super(fields.message); + this.name = 'CliError'; + this.errorCode = fields.errorCode; + this.exitCode = fields.exitCode; + this.httpStatus = fields.httpStatus; + this.detail = fields.detail; + this.validationErrors = fields.validationErrors; + this.hint = fields.hint; + } +} + +function rawBodyDetail(body: unknown): string | undefined { + if (typeof body === 'string') { + const trimmed = body.trim(); + return trimmed ? trimmed : undefined; + } + if (body !== null && typeof body === 'object') { + return JSON.stringify(body); + } + return undefined; +} + +export function cliErrorFromResponse( + status: number, + statusText: string, + body: unknown, +): CliError { + const problem = asProblem(body); + const errorCode = problem?.error_code ?? 'request_failed'; + const title = problem?.title ?? `HTTP ${status}`; + const detail = problem ? (problem.detail ?? undefined) : rawBodyDetail(body); + return new CliError({ + message: detail + ? `${title}: ${detail}` + : `${title} (${status} ${statusText})`, + errorCode, + exitCode: exitCodeForErrorCode(errorCode, status), + httpStatus: status, + detail, + validationErrors: problem?.validation_errors ?? undefined, + hint: hintForErrorCode(errorCode), + }); +} + +export function usageError(message: string): CliError { + return new CliError({ message, errorCode: 'usage_error', exitCode: 2 }); +} + +export function authError( + message: string, + errorCode: + | 'authentication_required' + | 'invalid_token' = 'authentication_required', +): CliError { + return new CliError({ + message, + errorCode, + exitCode: exitCodeForErrorCode(errorCode), + hint: hintForErrorCode(errorCode), + }); +} + +export function transportError(message: string): CliError { + return new CliError({ + message, + errorCode: 'transport_error', + exitCode: 5, + }); +} + +export function formatErrorText(error: CliError): string { + const lines: string[] = [error.message]; + + if (error.validationErrors && error.validationErrors.length > 0) { + lines.push(''); + lines.push('Validation errors:'); + for (const validationError of error.validationErrors) { + lines.push(` - ${validationError.field}: ${validationError.message}`); + } + } + + if (error.hint) { + lines.push(''); + lines.push(error.hint); + } + + return lines.join('\n'); +} + +export function formatErrorEnvelope(error: CliError, isTTY: boolean): string { + return formatJsonOutput( + { + status: 'error', + message: error.message, + error: { + error_code: error.errorCode, + http_status: error.httpStatus, + detail: error.detail, + validation_errors: error.validationErrors, + hint: error.hint, + }, + }, + isTTY, + ); +} diff --git a/src/cli-main-e2e.test.ts b/src/cli-main-e2e.test.ts new file mode 100644 index 0000000..79eb37e --- /dev/null +++ b/src/cli-main-e2e.test.ts @@ -0,0 +1,47 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { main } from './cli'; + +/** + * Unlike `cli.test.ts`, this file does NOT mock `./auth-commands` — it drives + * `main()` end to end so a converted `usageError` really reaches the CliError + * catch in `cli.ts`, not a mocked stand-in for the handler. + */ +describe('main() end to end — CliError conversions', () => { + const originalArgv = process.argv; + let errSpy: ReturnType; + let stdoutIsTTY: PropertyDescriptor | undefined; + + function runWith(argv: string[]): Promise { + process.argv = ['node', 'amp', ...argv]; + return main(); + } + + beforeEach(() => { + vi.spyOn(console, 'log').mockImplementation(() => undefined); + errSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + stdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdout, 'isTTY', { + value: false, + configurable: true, + }); + }); + + afterEach(() => { + process.argv = originalArgv; + vi.restoreAllMocks(); + if (stdoutIsTTY) { + Object.defineProperty(process.stdout, 'isTTY', stdoutIsTTY); + } + process.exitCode = undefined; + }); + + it('renders a usage_error (exit 2) for `auth pat` without --with-token', async () => { + await runWith(['auth', 'pat']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/requires --with-token/); + }); +}); diff --git a/src/cli-state.test.ts b/src/cli-state.test.ts new file mode 100644 index 0000000..e134ec4 --- /dev/null +++ b/src/cli-state.test.ts @@ -0,0 +1,107 @@ +import { + chmodSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { getOrCreateDeviceId } from './cli-state'; + +const UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +describe('getOrCreateDeviceId', () => { + const dirs: string[] = []; + + afterEach(() => { + vi.restoreAllMocks(); + for (const dir of dirs) { + rmSync(dir, { force: true, recursive: true }); + } + dirs.length = 0; + }); + + function tempDir(): string { + const dir = mkdtempSync(join(tmpdir(), 'amp-state-')); + dirs.push(dir); + return dir; + } + + function tempPath(): string { + return join(tempDir(), 'state.json'); + } + + it('mints and persists a device_id with a created_at on first use', () => { + const path = tempPath(); + const id = getOrCreateDeviceId(path); + + expect(id).toMatch(UUID_RE); + const written = JSON.parse(readFileSync(path, 'utf8')); + expect(written.device_id).toBe(id); + expect(Number.isNaN(Date.parse(written.created_at))).toBe(false); + }); + + it('returns the same device_id on subsequent calls (stable)', () => { + const path = tempPath(); + expect(getOrCreateDeviceId(path)).toBe(getOrCreateDeviceId(path)); + }); + + it('returns an existing device_id from a valid file', () => { + const path = tempPath(); + writeFileSync(path, JSON.stringify({ version: 1, device_id: 'kept-id' })); + expect(getOrCreateDeviceId(path)).toBe('kept-id'); + }); + + it('mints into a valid file that has no device_id, preserving other keys', () => { + const path = tempPath(); + writeFileSync(path, JSON.stringify({ version: 1, future_flag: true })); + const id = getOrCreateDeviceId(path); + const written = JSON.parse(readFileSync(path, 'utf8')); + expect(written.device_id).toBe(id); + expect(written.future_flag).toBe(true); + }); + + it('recreates a corrupt file (self-heal) without any stderr output', () => { + const path = tempPath(); + writeFileSync(path, '{ not json'); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + + const id = getOrCreateDeviceId(path); + expect(id).toMatch(UUID_RE); + // Corrupt content held no recoverable id, so it's replaced with a valid file. + expect(JSON.parse(readFileSync(path, 'utf8')).device_id).toBe(id); + // Telemetry is silent — no user-facing output. + expect(stderr).not.toHaveBeenCalled(); + }); + + it('never throws when the state cannot be written', () => { + // Read-only dir: the file is absent (read → mint path), but the write fails. + // The failure must be swallowed and the (ephemeral) id still returned. + const dir = tempDir(); + chmodSync(dir, 0o500); + let id: string | undefined; + try { + id = getOrCreateDeviceId(join(dir, 'state.json')); + } finally { + chmodSync(dir, 0o700); + } + expect(id).toMatch(UUID_RE); + }); + + it('leaves no temp file behind after a write', () => { + const path = tempPath(); + getOrCreateDeviceId(path); + const leftovers = readdirSync(dirname(path)).filter((f) => + f.includes('.tmp'), + ); + expect(leftovers).toEqual([]); + }); +}); diff --git a/src/cli-state.ts b/src/cli-state.ts new file mode 100644 index 0000000..0ac0530 --- /dev/null +++ b/src/cli-state.ts @@ -0,0 +1,108 @@ +import { randomUUID } from 'node:crypto'; +import { + mkdirSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { homedir } from 'node:os'; +import { dirname, join } from 'node:path'; + +import { z } from 'zod'; + +/** + * On-disk install state for the `amp` CLI: a versioned, non-secret file holding + * a stable per-installation device_id. Separate from credentials.json (the + * secrets store): this is write-once and must never churn, and would have + * nowhere to live once secrets move to an OS keychain. + * + * It mirrors the credential store's write mechanics (atomic temp-file + rename, + * 0600/0700, forward-compat `.catchall`) but follows two rules the credential + * store does not, because this is telemetry: it is entirely silent (never writes + * to stderr) and it never throws — a read or write failure degrades to "no id + * this run", never an error the user sees or that could break a command. + */ + +const CURRENT_VERSION = 1; + +const stateSchema = z + .object({ + version: z.number().int(), + device_id: z.string().min(1).optional(), + created_at: z.string().min(1).optional(), + }) + .catchall(z.unknown()); + +type CliState = z.infer; + +function statePath(): string { + return join(homedir(), '.amplitude', 'amp', 'state.json'); +} + +// The parsed state, or undefined when there is nothing usable to build on — +// missing, unreadable, corrupt, or schema-invalid all collapse to undefined, +// and the caller recreates the file. An unreadable/corrupt file holds no +// recoverable id, so recreating self-heals rather than getting stuck. +function readState(path: string): CliState | undefined { + try { + const parsed = stateSchema.safeParse( + JSON.parse(readFileSync(path, 'utf8')), + ); + return parsed.success ? parsed.data : undefined; + } catch { + return undefined; + } +} + +function writeState(state: CliState, path: string): void { + const dir = dirname(path); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + + const tempPath = join( + dir, + `.state.${process.pid}.${Date.now()}.${Math.random() + .toString(36) + .slice(2)}.tmp`, + ); + + try { + writeFileSync(tempPath, `${JSON.stringify(state, null, 2)}\n`, { + mode: 0o600, + }); + renameSync(tempPath, path); + } catch (error) { + rmSync(tempPath, { force: true }); + throw error; + } +} + +/** + * The installation's stable device_id, minting and persisting one on first use + * and returning it unchanged thereafter. A missing, corrupt, or unreadable file + * is recreated (it holds no recoverable id, so this self-heals); a valid file + * without an id is filled in, preserving its other keys. Never throws and never + * writes to stderr: a telemetry helper must not break or bother the CLI, so a + * failed persist just yields an ephemeral id for this run. + */ +export function getOrCreateDeviceId(path: string = statePath()): string { + const existing = readState(path); + if (existing?.device_id) { + return existing.device_id; + } + + const deviceId = randomUUID(); + try { + writeState( + { + ...(existing ?? { version: CURRENT_VERSION }), + device_id: deviceId, + created_at: new Date().toISOString(), + }, + path, + ); + } catch { + // Best-effort persist. + } + return deviceId; +} diff --git a/src/cli.test.ts b/src/cli.test.ts index dabace1..2732605 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1,8 +1,37 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { parseArgs } from './args'; +import * as authCommands from './auth-commands'; import { isHelpRequested, isVersionRequested, main } from './cli'; +import * as credentialStore from './credential-store'; import { formatVersion } from './help'; +vi.mock('./auth-commands', async () => { + const actual = + await vi.importActual('./auth-commands'); + return { + ...actual, + runAuthLoginStart: vi.fn(), + runAuthLoginPoll: vi.fn(), + runAuthLogin: vi.fn(), + runAuthPat: vi.fn(), + }; +}); + +// `loadStore` reads real `~/.amplitude/amp/credentials.json` by default, +// which main-routing tests must not depend on. Give it a safe empty-store +// default here; the expired-token test below overrides it for that case only. +vi.mock('./credential-store', async () => { + const actual = + await vi.importActual( + './credential-store', + ); + return { + ...actual, + loadStore: vi.fn(), + }; +}); + describe('isHelpRequested', () => { it('recognizes the help command and --help/-h in every form', () => { expect(isHelpRequested(['help', 'flags'], {})).toBe(true); @@ -33,6 +62,8 @@ describe('isVersionRequested', () => { describe('main routing', () => { const originalArgv = process.argv; let logSpy: ReturnType; + let errSpy: ReturnType; + let stdoutIsTTY: PropertyDescriptor | undefined; function runWith(argv: string[]): Promise { process.argv = ['node', 'amp', ...argv]; @@ -41,17 +72,35 @@ describe('main routing', () => { beforeEach(() => { logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + errSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + stdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdout, 'isTTY', { + value: false, + configurable: true, + }); vi.stubGlobal( 'fetch', vi.fn(() => { throw new Error('fetch should not be called for these routes'); }), ); + vi.mocked(authCommands.runAuthLoginStart).mockClear(); + vi.mocked(authCommands.runAuthLoginPoll).mockClear(); + vi.mocked(authCommands.runAuthLogin).mockClear(); + vi.mocked(authCommands.runAuthPat).mockClear(); + vi.mocked(credentialStore.loadStore).mockReturnValue( + credentialStore.emptyStore(), + ); }); afterEach(() => { process.argv = originalArgv; logSpy.mockRestore(); + errSpy.mockRestore(); + if (stdoutIsTTY) { + Object.defineProperty(process.stdout, 'isTTY', stdoutIsTTY); + } + process.exitCode = undefined; vi.unstubAllGlobals(); }); @@ -62,23 +111,477 @@ describe('main routing', () => { expect(logSpy).toHaveBeenCalledTimes(2); }); - it('prints global help when no command is given', async () => { - await runWith([]); - const output = logSpy.mock.calls - .map((call: unknown[]) => String(call[0])) - .join('\n'); - expect(output).toContain('amp'); + it('renders a helpful error for an unknown command as a JSON usage error on stderr', async () => { + await runWith(['frobnicate']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Unknown command: frobnicate/); }); - it('throws a helpful error for an unknown command', async () => { - await expect(runWith(['frobnicate'])).rejects.toThrow( - /Unknown command: frobnicate/, + it('renders a usage_error (exit 2) for an unknown --region on an API command', async () => { + await runWith(['projects', 'list', '--region', 'bogus']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Unknown --region "bogus"/); + }); + + it('routes `amp help ` through the error envelope (stderr, exit 2), not stdout', async () => { + await runWith(['help', 'frobnicate']); + + expect(process.exitCode).toBe(2); + expect(logSpy).not.toHaveBeenCalled(); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Unknown command: frobnicate/); + }); + + it('renders a structured auth error (exit 3) for an unknown --profile on an API command', async () => { + await runWith(['projects', 'list', '--profile', '__nope__']); + + expect(process.exitCode).toBe(3); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('authentication_required'); + expect(parsed.error.hint).toBeTruthy(); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a structured invalid_token error (exit 3) for an expired stored oauth credential', async () => { + const expiredStore = credentialStore.setDefault( + credentialStore.setProfile(credentialStore.emptyStore(), 'default', { + base_url: 'https://prod', + credential: { + type: 'oauth', + access_token: 'expired-access-token', + token_type: 'Bearer', + expires_at: '2000-01-01T00:00:00.000Z', + }, + saved_at: '2000-01-01T00:00:00.000Z', + store: 'file', + }), + 'default', ); + vi.mocked(credentialStore.loadStore).mockReturnValue(expiredStore); + + await runWith(['projects', 'list']); + + expect(process.exitCode).toBe(3); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('invalid_token'); + expect(parsed.error.hint).toBeTruthy(); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a deterministic usage_error (exit 2) for a malformed command even with an unresolvable --profile, not an auth error', async () => { + await runWith(['events', 'get', '--profile', '__nope__']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Missing --project \./); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a usage_error (exit 2) for a flag valid on another command but not this one', async () => { + await runWith(['flags', 'list', '--key', 'foo']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toBe('Unknown flag --key for `amp flags list`.'); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('validates flags before the delete-confirmation gate: an unknown flag on a DELETE with no --yes surfaces usage_error, not the "Pass --yes" gate error', async () => { + await runWith([ + 'events', + 'delete', + '--project', + '187520', + '--totally-bogus', + 'x', + ]); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/--totally-bogus/); + expect(parsed.message).not.toMatch(/Pass --yes/); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a truly-unknown flag (no alias anywhere) as usage_error (exit 2) with a "Did you mean" suggestion', async () => { + await runWith(['flags', 'list', '--porject', '1']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Did you mean --project/); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a structured authentication_required error (exit 3) for `auth status` piped with no credential, stdout empty', async () => { + await runWith(['auth', 'status']); + + expect(process.exitCode).toBe(3); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.status).toBe('error'); + expect(parsed.error.error_code).toBe('authentication_required'); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a structured invalid_token error (exit 3) for `auth status` piped with an expired stored credential, stdout empty', async () => { + const expiredStore = credentialStore.setDefault( + credentialStore.setProfile(credentialStore.emptyStore(), 'default', { + base_url: 'https://prod', + credential: { + type: 'oauth', + access_token: 'expired-access-token', + token_type: 'Bearer', + expires_at: '2000-01-01T00:00:00.000Z', + }, + saved_at: '2000-01-01T00:00:00.000Z', + store: 'file', + }), + 'default', + ); + vi.mocked(credentialStore.loadStore).mockReturnValue(expiredStore); + + await runWith(['auth', 'status']); + + expect(process.exitCode).toBe(3); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('invalid_token'); + expect(logSpy).not.toHaveBeenCalled(); }); it('rejects `auth use` with an extra argument instead of ignoring it', async () => { - await expect(runWith(['auth', 'use', 'foo', 'bar'])).rejects.toThrow( - /Unknown auth command: auth use foo bar/, + await runWith(['auth', 'use', 'foo', 'bar']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Unknown auth command: auth use foo bar/); + }); + + it('dispatches `auth login start` to runAuthLoginStart, not runAuthLoginPoll', async () => { + await runWith(['auth', 'login', 'start', '--region', 'us']); + + expect(authCommands.runAuthLoginStart).toHaveBeenCalledWith( + expect.objectContaining({ region: 'us' }), + ); + expect(authCommands.runAuthLoginPoll).not.toHaveBeenCalled(); + }); + + it('dispatches `auth login poll` to runAuthLoginPoll, not runAuthLoginStart', async () => { + await runWith(['auth', 'login', 'poll', '--profile', 'default']); + + expect(authCommands.runAuthLoginPoll).toHaveBeenCalledWith( + expect.objectContaining({ profile: 'default' }), + ); + expect(authCommands.runAuthLoginStart).not.toHaveBeenCalled(); + }); + + it('rejects `auth login start` with a trailing argument instead of ignoring it', async () => { + await runWith(['auth', 'login', 'start', 'default']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.message).toMatch( + /Unknown auth command: auth login start default/, ); + expect(authCommands.runAuthLoginStart).not.toHaveBeenCalled(); + }); + + it('rejects `auth login poll` with a trailing argument instead of ignoring it', async () => { + await runWith(['auth', 'login', 'poll', 'default']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.message).toMatch( + /Unknown auth command: auth login poll default/, + ); + expect(authCommands.runAuthLoginPoll).not.toHaveBeenCalled(); + }); + + describe('auth/logout input safety', () => { + it('accepts legit flags on `auth login` without an "Unknown flag" error', async () => { + await runWith([ + 'auth', + 'login', + '--region', + 'us', + '--profile', + 'p', + '--flow', + 'device', + ]); + + expect(errSpy).not.toHaveBeenCalled(); + expect(authCommands.runAuthLoginStart).toHaveBeenCalledWith( + expect.objectContaining({ + region: 'us', + profile: 'p', + flow: 'device', + }), + ); + }); + + it('accepts legit flags on `auth login start` without an "Unknown flag" error', async () => { + await runWith(['auth', 'login', 'start', '--region', 'us', '--json']); + + expect(errSpy).not.toHaveBeenCalled(); + expect(authCommands.runAuthLoginStart).toHaveBeenCalledWith( + expect.objectContaining({ region: 'us', json: true }), + ); + }); + + it('accepts legit flags on `auth login poll` without an "Unknown flag" error', async () => { + await runWith(['auth', 'login', 'poll', '--timeout', '0', '--json']); + + expect(errSpy).not.toHaveBeenCalled(); + expect(authCommands.runAuthLoginPoll).toHaveBeenCalledWith( + expect.objectContaining({ timeout: '0', json: true }), + ); + }); + + it('accepts legit flags on `auth pat` without an "Unknown flag" error', async () => { + await runWith(['auth', 'pat', '--with-token', '--region', 'us']); + + expect(errSpy).not.toHaveBeenCalled(); + expect(authCommands.runAuthPat).toHaveBeenCalledWith( + expect.objectContaining({ 'with-token': true, region: 'us' }), + ); + }); + + it('rejects an unknown flag on `auth pat` (exit 2, usage_error, names the flag)', async () => { + await runWith(['auth', 'pat', '--key', 'foo']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toBe('Unknown flag --key for `amp auth pat`.'); + expect(authCommands.runAuthPat).not.toHaveBeenCalled(); + }); + + it('suggests the nearest known flag for a typo on `auth pat`', async () => { + await runWith(['auth', 'pat', '--toekn', 'x']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toMatch(/Did you mean --token\?/); + expect(authCommands.runAuthPat).not.toHaveBeenCalled(); + }); + + it('rejects an unknown flag on `logout` (exit 2, usage_error, names the flag)', async () => { + await runWith(['logout', '--key', 'foo']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.error.error_code).toBe('usage_error'); + expect(parsed.message).toBe('Unknown flag --key for `amp logout`.'); + }); + }); + + describe('bare `auth login` TTY dispatch', () => { + let stdinIsTTY: PropertyDescriptor | undefined; + let stdoutIsTTY: PropertyDescriptor | undefined; + + beforeEach(() => { + stdinIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + stdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + }); + + afterEach(() => { + if (stdinIsTTY) { + Object.defineProperty(process.stdin, 'isTTY', stdinIsTTY); + } + if (stdoutIsTTY) { + Object.defineProperty(process.stdout, 'isTTY', stdoutIsTTY); + } + }); + + it('dispatches to runAuthLoginStart when not an interactive terminal', async () => { + Object.defineProperty(process.stdin, 'isTTY', { + value: false, + configurable: true, + }); + Object.defineProperty(process.stdout, 'isTTY', { + value: false, + configurable: true, + }); + + await runWith(['auth', 'login']); + + expect(authCommands.runAuthLoginStart).toHaveBeenCalled(); + expect(authCommands.runAuthLogin).not.toHaveBeenCalled(); + }); + + it('dispatches to runAuthLogin when at an interactive terminal', async () => { + Object.defineProperty(process.stdin, 'isTTY', { + value: true, + configurable: true, + }); + Object.defineProperty(process.stdout, 'isTTY', { + value: true, + configurable: true, + }); + + await runWith(['auth', 'login']); + + expect(authCommands.runAuthLogin).toHaveBeenCalled(); + expect(authCommands.runAuthLoginStart).not.toHaveBeenCalled(); + }); + }); + + describe('help JSON routing', () => { + let stdoutIsTTY: PropertyDescriptor | undefined; + + beforeEach(() => { + stdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + }); + + afterEach(() => { + if (stdoutIsTTY) { + Object.defineProperty(process.stdout, 'isTTY', stdoutIsTTY); + } + }); + + function setTTY(value: boolean): void { + Object.defineProperty(process.stdout, 'isTTY', { + value, + configurable: true, + }); + } + + it('prints prose global help at a TTY with no command', async () => { + setTTY(true); + await runWith([]); + + const output = logSpy.mock.calls + .map((call: unknown[]) => String(call[0])) + .join('\n'); + expect(() => JSON.parse(output)).toThrow(); + expect(output).toContain('Usage:'); + }); + + it('prints JSON for a bare command when not a TTY', async () => { + setTTY(false); + await runWith([]); + + const parsed = JSON.parse(String(logSpy.mock.calls[0]?.[0])); + expect(Array.isArray(parsed.commands)).toBe(true); + }); + + it('prints prose global help for bare `amp help` at a TTY, not an unknown-command error', async () => { + setTTY(true); + await runWith(['help']); + + const output = logSpy.mock.calls + .map((call: unknown[]) => String(call[0])) + .join('\n'); + expect(output).toContain('Usage:'); + expect(output).toContain('Product surfaces:'); + expect(errSpy).not.toHaveBeenCalled(); + expect(process.exitCode).not.toBeTruthy(); + }); + + it('prints JSON for a bare command at a TTY when --json is passed', async () => { + setTTY(true); + await runWith(['--json']); + + expect(() => JSON.parse(String(logSpy.mock.calls[0]?.[0]))).not.toThrow(); + }); + + it('prints JSON for ` --help` when not a TTY, including the command topic', async () => { + setTTY(false); + await runWith(['flags', 'list', '--help']); + + const parsed = JSON.parse(String(logSpy.mock.calls[0]?.[0])); + expect(parsed.command).toBe('flags list'); + }); + + it('prints prose for ` --help` at a TTY', async () => { + setTTY(true); + await runWith(['flags', 'list', '--help']); + + const output = logSpy.mock.calls + .map((call: unknown[]) => String(call[0])) + .join('\n'); + expect(() => JSON.parse(output)).toThrow(); + }); + }); + + describe('error rendering', () => { + let stdoutIsTTY: PropertyDescriptor | undefined; + + beforeEach(() => { + stdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + }); + + afterEach(() => { + if (stdoutIsTTY) { + Object.defineProperty(process.stdout, 'isTTY', stdoutIsTTY); + } + }); + + function setTTY(value: boolean): void { + Object.defineProperty(process.stdout, 'isTTY', { + value, + configurable: true, + }); + } + + it('renders a usage error as a JSON envelope on stderr when not a TTY', async () => { + setTTY(false); + await runWith(['bogus']); + + expect(process.exitCode).toBe(2); + const parsed = JSON.parse(String(errSpy.mock.calls[0]?.[0])); + expect(parsed.status).toBe('error'); + expect(parsed.error.error_code).toBe('usage_error'); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('renders a usage error as prose with remediation guidance at a TTY', async () => { + setTTY(true); + await runWith(['bogus']); + + expect(process.exitCode).toBe(2); + const output = String(errSpy.mock.calls[0]?.[0]); + expect(() => JSON.parse(output)).toThrow(); + expect(output).toContain('Unknown command: bogus'); + expect(output).toContain('Run `amp help` to list commands.'); + expect(logSpy).not.toHaveBeenCalled(); + }); + + it('honors --json false on the error path at a TTY (prose, not a JSON envelope)', async () => { + setTTY(true); + // argv carries a bare `--json` token, but the parsed flag is `false`; the + // error envelope must follow the parsed flag like the success path does. + await runWith(['bogus', '--json', 'false']); + + expect(process.exitCode).toBe(2); + const output = String(errSpy.mock.calls[0]?.[0]); + expect(() => JSON.parse(output)).toThrow(); + expect(output).toContain('Unknown command: bogus'); + }); + }); +}); + +describe('cli routing', () => { + it('keeps --timeout as a value flag', () => { + expect( + parseArgs(['auth', 'login', 'poll', '--timeout', '25']).flags.timeout, + ).toBe('25'); + }); + it('parses the login start/poll command path', () => { + expect( + parseArgs(['auth', 'login', 'start', '--region', 'us']).command, + ).toEqual(['auth', 'login', 'start']); }); }); diff --git a/src/cli.ts b/src/cli.ts index 76dfc1e..1bbb731 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,25 +1,66 @@ #!/usr/bin/env node /* eslint-disable no-console */ -import { type FlagValue, isFlagEnabled, parseArgs } from './args'; +import { + type FlagValue, + globalOptionAliases, + isFlagEnabled, + parseArgs, +} from './args'; import { runAuthList, runAuthLogin, + runAuthLoginPoll, + runAuthLoginStart, runAuthPat, runAuthStatus, runAuthToken, runAuthUse, runLogout, } from './auth-commands'; -import { DEFAULT_API_BASE_URL } from './config'; +import { buildCatalog } from './catalog'; +import { + CliError, + formatErrorEnvelope, + formatErrorText, + usageError, +} from './cli-error'; import { findOperation, formatVersion, printCommandHelp, printGlobalHelp, } from './help'; +import { shouldUseJsonOutput } from './output'; +import { assertFlagsAllowed } from './request'; import { runOperation } from './run'; import { terminal } from './terminal'; +/** + * Validates flags for a bespoke auth/logout command against globals ∪ that + * command's catalog-declared flags. These commands dispatch directly in + * `main()` rather than through `buildRequest`, so without this they'd skip + * the same misplaced/unknown-flag check API operations get from + * `assertKnownFlags` — a typo'd flag (e.g. `--toekn`) would silently drop + * instead of erroring. + * + * `command` must be the catalog path (e.g. `['auth', 'use']`), not the raw + * parsed command tokens — those may carry a trailing positional argument + * (`auth use `) that isn't part of the catalog key. + */ +function assertKnownAuthFlags( + command: string[], + flags: Record, +): void { + const entry = buildCatalog().find( + (candidate) => + candidate.command.length === command.length && + candidate.command.every((part, index) => part === command[index]), + ); + const catalogAliases = entry?.flags.flatMap((flag) => flag.aliases) ?? []; + const allowed = new Set([...globalOptionAliases(), ...catalogAliases]); + assertFlagsAllowed(allowed, `amp ${command.join(' ')}`, flags); +} + export function isHelpRequested( command: string[], flags: Record, @@ -41,74 +82,143 @@ export function isVersionRequested( } export async function main(): Promise { - const { command, flags } = parseArgs(process.argv.slice(2)); - - if (isVersionRequested(command, flags)) { - console.log(formatVersion()); - return; - } - - if (command.length === 0) { - printGlobalHelp(DEFAULT_API_BASE_URL); - return; - } - - if (isHelpRequested(command, flags)) { - const topic = command[0] === 'help' ? command.slice(1) : command; - printCommandHelp(topic, DEFAULT_API_BASE_URL); - return; - } + const isTTY = Boolean(process.stdout.isTTY); + let flags: Record | undefined; + try { + const parsed = parseArgs(process.argv.slice(2)); + flags = parsed.flags; + const { command } = parsed; - if (command[0] === 'auth') { - if (command.length === 2 && command[1] === 'login') { - await runAuthLogin(flags); - return; - } - if (command.length === 2 && command[1] === 'pat') { - await runAuthPat(flags); + if (isVersionRequested(command, flags)) { + console.log(formatVersion()); return; } - if (command.length === 2 && command[1] === 'list') { - runAuthList(); + + const wantsJson = shouldUseJsonOutput({ + jsonFlag: isFlagEnabled(flags.json), + isTTY, + }); + + if (command.length === 0) { + if (wantsJson) { + printCommandHelp([], { json: true, isTTY }); + } else { + printGlobalHelp(); + } return; } - if (command[1] === 'use' && command.length <= 3) { - runAuthUse(command[2]); + + if (isHelpRequested(command, flags)) { + const topic = command[0] === 'help' ? command.slice(1) : command; + printCommandHelp(topic, { json: wantsJson, isTTY }); return; } - if (command.length === 2 && command[1] === 'status') { - runAuthStatus(flags); - return; + + if (command[0] === 'auth') { + if ( + command.length === 3 && + command[1] === 'login' && + command[2] === 'start' + ) { + assertKnownAuthFlags(['auth', 'login', 'start'], flags); + await runAuthLoginStart(flags); + return; + } + if ( + command.length === 3 && + command[1] === 'login' && + command[2] === 'poll' + ) { + assertKnownAuthFlags(['auth', 'login', 'poll'], flags); + await runAuthLoginPoll(flags); + return; + } + if (command.length === 2 && command[1] === 'login') { + assertKnownAuthFlags(['auth', 'login'], flags); + const isInteractive = Boolean( + process.stdin.isTTY && process.stdout.isTTY, + ); + if (!isInteractive) { + await runAuthLoginStart(flags); + } else { + await runAuthLogin(flags); + } + return; + } + if (command.length === 2 && command[1] === 'pat') { + assertKnownAuthFlags(['auth', 'pat'], flags); + await runAuthPat(flags); + return; + } + if (command.length === 2 && command[1] === 'list') { + assertKnownAuthFlags(['auth', 'list'], flags); + runAuthList(flags); + return; + } + if (command[1] === 'use' && command.length <= 3) { + assertKnownAuthFlags(['auth', 'use'], flags); + runAuthUse(command[2]); + return; + } + if (command.length === 2 && command[1] === 'status') { + assertKnownAuthFlags(['auth', 'status'], flags); + runAuthStatus(flags); + return; + } + if (command.length === 2 && command[1] === 'token') { + assertKnownAuthFlags(['auth', 'token'], flags); + runAuthToken(flags); + return; + } + + throw usageError( + `Unknown auth command: ${command.join(' ')}. Try: amp help auth`, + ); } - if (command.length === 2 && command[1] === 'token') { - runAuthToken(flags); + + if (command[0] === 'logout' && command.length === 1) { + assertKnownAuthFlags(['logout'], flags); + await runLogout(flags); return; } - throw new Error( - `Unknown auth command: ${command.join(' ')}. Try: amp help auth`, - ); - } - - if (command[0] === 'logout' && command.length === 1) { - await runLogout(flags); - return; - } + const operation = findOperation(command); + if (!operation) { + throw usageError( + `Unknown command: ${command.join(' ')}. Run \`amp help\` to list commands.`, + ); + } - const operation = findOperation(command); - if (!operation) { - throw new Error( - `Unknown command: ${command.join(' ')}. Run \`amp help ${command[0]}\` for related commands.`, - ); + await runOperation(operation, flags); + } catch (error) { + // Prefer the parsed flag so the error envelope honors `--json false` the + // same way the success path does; fall back to an argv scan only when + // parseArgs itself threw and `flags` was never assigned. + const wantsJson = flags + ? shouldUseJsonOutput({ jsonFlag: isFlagEnabled(flags.json), isTTY }) + : process.argv.includes('--json') || !isTTY; + const cliError = + error instanceof CliError + ? error + : new CliError({ + message: error instanceof Error ? error.message : String(error), + errorCode: 'error', + exitCode: 1, + }); + if (wantsJson) { + console.error(formatErrorEnvelope(cliError, isTTY)); + } else { + console.error(terminal.error(formatErrorText(cliError))); + } + process.exitCode = cliError.exitCode; } - - await runOperation(operation, flags); } if (require.main === module) { main().catch((error: unknown) => { - const message = error instanceof Error ? error.message : String(error); - console.error(terminal.error(message)); + console.error( + terminal.error(error instanceof Error ? error.message : String(error)), + ); process.exitCode = 1; }); } diff --git a/src/config.test.ts b/src/config.test.ts index 62a8bfa..c2c830a 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -1,6 +1,10 @@ import { afterEach, describe, expect, it } from 'vitest'; -import { resolveBaseUrl } from './config'; +import { + resolveBaseUrl, + resolveNamedBaseUrl, + resolveRegionBaseUrl, +} from './config'; function restoreEnvValue(key: string, value: string | undefined): void { if (value === undefined) { @@ -37,3 +41,44 @@ describe('resolveBaseUrl', () => { ); }); }); + +describe('resolveRegionBaseUrl', () => { + it('maps us and eu to the prod / prod-eu hosts', () => { + expect(resolveRegionBaseUrl('us')).toBe( + 'https://developer-api.amplitude.com', + ); + expect(resolveRegionBaseUrl('eu')).toBe( + 'https://developer-api.eu.amplitude.com', + ); + }); + + it('throws on an unknown region', () => { + expect(() => resolveRegionBaseUrl('apac')).toThrow( + 'Unknown --region "apac". Known: us, eu.', + ); + }); +}); + +describe('resolveNamedBaseUrl', () => { + it('resolves --region', () => { + expect(resolveNamedBaseUrl({ regionFlag: 'us' })).toBe( + 'https://developer-api.amplitude.com', + ); + }); + + it('resolves --env', () => { + expect(resolveNamedBaseUrl({ envFlag: 'staging' })).toBe( + 'https://developer-api.stag2.amplitude.com', + ); + }); + + it('returns undefined when neither is given', () => { + expect(resolveNamedBaseUrl({})).toBeUndefined(); + }); + + it('throws when both --region and --env are given', () => { + expect(() => + resolveNamedBaseUrl({ regionFlag: 'us', envFlag: 'staging' }), + ).toThrow('Pass either --region or --env, not both.'); + }); +}); diff --git a/src/config.ts b/src/config.ts index 91000e2..219dfb3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,8 +1,17 @@ import { type FlagValue, stringFlag } from './args'; +import { usageError } from './cli-error'; import { apiBaseUrlFromEnv } from './env'; export const DEFAULT_API_BASE_URL = 'https://developer-api.amplitude.com'; +// Seconds a single `amp auth login poll` blocks before returning `pending`. +// Lives here (not in auth-commands.ts) so the poll command's help copy in +// catalog.ts can interpolate the real value without importing the auth handlers +// — that import would cycle (catalog → auth-commands → authToken → help → +// catalog). Sourcing both the runtime default and the help text from this one +// constant keeps the documented block time from drifting. +export const DEFAULT_POLL_TIMEOUT_SECONDS = 25; + // Friendly `--env` names → Developer API base URLs. `auth login` requires an // explicit env (or --base-url) when creating a profile — no implicit default — // so this map is the ergonomic primitive, not optional sugar. @@ -22,13 +31,61 @@ export const ENV_BASE_URLS: Record = { export function resolveEnvBaseUrl(name: string): string { const url = ENV_BASE_URLS[name]; if (!url) { - throw new Error( + throw usageError( `Unknown --env "${name}". Known: ${Object.keys(ENV_BASE_URLS).join(', ')}.`, ); } return url; } +// Friendly `--region` names → the same Developer API base URLs as the +// internal prod/prod-eu envs. References ENV_BASE_URLS rather than +// hardcoding hosts a second time, so the two maps can't drift apart. +export const REGION_BASE_URLS: Record = { + us: ENV_BASE_URLS.prod, + eu: ENV_BASE_URLS['prod-eu'], +}; + +export function resolveRegionBaseUrl(name: string): string { + const url = REGION_BASE_URLS[name]; + if (!url) { + throw usageError( + `Unknown --region "${name}". Known: ${Object.keys(REGION_BASE_URLS).join(', ')}.`, + ); + } + return url; +} + +// Callers must run this before any --base-url short-circuit, not just inside +// resolveNamedBaseUrl — otherwise a conflicting --region/--env pair silently +// passes through whenever --base-url also happens to be set. +export function assertRegionAndEnvNotBothSet(options: { + envFlag?: string; + regionFlag?: string; +}): void { + if (options.regionFlag && options.envFlag) { + throw usageError('Pass either --region or --env, not both.'); + } +} + +// Centralizes the "--region xor --env" precedence shared by loginBaseUrl +// (auth-commands.ts) and baseUrlOverrideFromFlags (credential-resolver.ts) so +// both call sites agree on what each flag means and on the same +// mutual-exclusivity error. +export function resolveNamedBaseUrl(options: { + envFlag?: string; + regionFlag?: string; +}): string | undefined { + assertRegionAndEnvNotBothSet(options); + if (options.regionFlag) { + return resolveRegionBaseUrl(options.regionFlag); + } + if (options.envFlag) { + return resolveEnvBaseUrl(options.envFlag); + } + return undefined; +} + export function resolveBaseUrl(flags: Record): string { return ( stringFlag(flags, ['base-url']) ?? diff --git a/src/credential-resolver.test.ts b/src/credential-resolver.test.ts index 8863742..cfb56ea 100644 --- a/src/credential-resolver.test.ts +++ b/src/credential-resolver.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it } from 'vitest'; +import { CliError } from './cli-error'; import { authorizationHeaderForToken, resolveAuth, + resolveAuthFromFlags, } from './credential-resolver'; import { type CredentialStore, @@ -110,6 +112,23 @@ describe('resolveAuth', () => { ).toThrow(/nope/); }); + it('throws a structured CliError with an auth hint on an unknown profile', () => { + try { + resolveAuth({ + profileFlag: 'nope', + env: {}, + store: emptyStore(), + now: NOW, + }); + expect.unreachable('resolveAuth should have thrown'); + } catch (error) { + if (!(error instanceof CliError)) throw error; + expect(error.errorCode).toBe('authentication_required'); + expect(error.exitCode).toBe(3); + expect(error.hint).toBeTruthy(); + } + }); + it('throws when the selected oauth token has expired', () => { expect(() => resolveAuth({ @@ -120,6 +139,22 @@ describe('resolveAuth', () => { ).toThrow(/expired/i); }); + it('throws a structured CliError with an invalid_token code when the stored token has expired', () => { + try { + resolveAuth({ + env: {}, + store: oauthStore('p', 'https://prod', PAST), + now: NOW, + }); + expect.unreachable('resolveAuth should have thrown'); + } catch (error) { + if (!(error instanceof CliError)) throw error; + expect(error.errorCode).toBe('invalid_token'); + expect(error.exitCode).toBe(3); + expect(error.hint).toBeTruthy(); + } + }); + it('returns a PAT credential as its raw token', () => { let store = setProfile(emptyStore(), 'ci', { base_url: 'https://prod', @@ -142,12 +177,42 @@ describe('resolveAuth', () => { ); }); + it('throws a structured CliError with an invalid_token code for an unsupported credential type', () => { + let store = setProfile(emptyStore(), 'sa', { + base_url: 'https://prod', + credential: { type: 'service_account', client_id: 'c' }, + saved_at: '2026-06-23T00:00:00.000Z', + }); + store = setDefault(store, 'sa'); + try { + resolveAuth({ env: {}, store, now: NOW }); + expect.unreachable('resolveAuth should have thrown'); + } catch (error) { + if (!(error instanceof CliError)) throw error; + expect(error.errorCode).toBe('invalid_token'); + expect(error.exitCode).toBe(3); + expect(error.hint).toBeTruthy(); + } + }); + it('errors with login guidance when the store is empty', () => { expect(() => resolveAuth({ env: {}, store: emptyStore(), now: NOW }), ).toThrow(/No credentials\. Run `amp auth login`/); }); + it('throws a structured CliError with an authentication_required code when the store is empty', () => { + try { + resolveAuth({ env: {}, store: emptyStore(), now: NOW }); + expect.unreachable('resolveAuth should have thrown'); + } catch (error) { + if (!(error instanceof CliError)) throw error; + expect(error.errorCode).toBe('authentication_required'); + expect(error.exitCode).toBe(3); + expect(error.hint).toBeTruthy(); + } + }); + it('points to `auth use` when profiles exist but none is active', () => { // A profile exists, but no default (e.g. just after logging out the default). const store = setProfile(emptyStore(), 'p', { @@ -160,6 +225,23 @@ describe('resolveAuth', () => { ); }); + it('throws a structured CliError with an authentication_required code when no profile is active', () => { + const store = setProfile(emptyStore(), 'p', { + base_url: 'https://prod', + credential: { type: 'pat', pat: 'amp_x' }, + saved_at: '2026-06-23T00:00:00.000Z', + }); + try { + resolveAuth({ env: {}, store, now: NOW }); + expect.unreachable('resolveAuth should have thrown'); + } catch (error) { + if (!(error instanceof CliError)) throw error; + expect(error.errorCode).toBe('authentication_required'); + expect(error.exitCode).toBe(3); + expect(error.hint).toBeTruthy(); + } + }); + it('raw-token path defaults base_url and honors AMP_API_BASE_URL', () => { expect( resolveAuth({ tokenFlag: 't', env: {}, store: emptyStore(), now: NOW }) @@ -176,6 +258,34 @@ describe('resolveAuth', () => { }); }); +describe('resolveAuthFromFlags', () => { + it('resolves --region into the base URL', () => { + const r = resolveAuthFromFlags( + { region: 'eu' }, + { store: oauthStore('p', 'https://prod', FUTURE), now: NOW }, + ); + expect(r.baseUrl).toBe('https://developer-api.eu.amplitude.com'); + }); + + it('throws when both --region and --env are given', () => { + expect(() => + resolveAuthFromFlags( + { region: 'eu', env: 'staging' }, + { store: oauthStore('p', 'https://prod', FUTURE), now: NOW }, + ), + ).toThrow('Pass either --region or --env, not both.'); + }); + + it('throws when both --region and --env are given even though --base-url would win', () => { + expect(() => + resolveAuthFromFlags( + { region: 'us', env: 'staging', 'base-url': 'http://localhost:3036' }, + { store: oauthStore('p', 'https://prod', FUTURE), now: NOW }, + ), + ).toThrow('Pass either --region or --env, not both.'); + }); +}); + describe('authorizationHeaderForToken', () => { it('formats by token shape', () => { expect(authorizationHeaderForToken('amp_x')).toBe('Bearer PAT=amp_x'); diff --git a/src/credential-resolver.ts b/src/credential-resolver.ts index 474d6bd..be093c0 100644 --- a/src/credential-resolver.ts +++ b/src/credential-resolver.ts @@ -1,5 +1,10 @@ import { type FlagValue, stringFlag } from './args'; -import { DEFAULT_API_BASE_URL, resolveEnvBaseUrl } from './config'; +import { authError } from './cli-error'; +import { + assertRegionAndEnvNotBothSet, + DEFAULT_API_BASE_URL, + resolveNamedBaseUrl, +} from './config'; import { type CredentialStore, type Profile, @@ -83,8 +88,9 @@ export function tokenFromProfile( const oauth = oauthCredentialSchema.safeParse(profile.credential); if (oauth.success) { if (isExpired(oauth.data.expires_at, now)) { - throw new Error( + throw authError( `Stored token for profile "${name}" has expired. Run \`amp auth login\`.`, + 'invalid_token', ); } return oauth.data.access_token; @@ -95,8 +101,9 @@ export function tokenFromProfile( return pat.data.pat; } - throw new Error( + throw authError( `Profile "${name}" uses an unsupported credential type "${profile.credential.type}". Update the CLI.`, + 'invalid_token', ); } @@ -182,33 +189,38 @@ export function resolveAuth(input: ResolveInput = {}): ResolvedAuth { const name = trimmed(input.profileFlag) ?? trimmed(env.AMP_PROFILE) ?? store.default; if (name) { - throw new Error(`No such profile: ${name}. Run \`amp auth list\`.`); + throw authError(`No such profile: ${name}. Run \`amp auth list\`.`); } // "no profiles at all" vs "profiles exist but none is active" (e.g. just // after logging out the default) — the suggested fix differs. if (Object.keys(store.profiles).length > 0) { - throw new Error( + throw authError( 'No active profile. Run `amp auth use ` to pick one (`amp auth list`), or `amp auth login`.', ); } - throw new Error('No credentials. Run `amp auth login`.'); + throw authError('No credentials. Run `amp auth login`.'); } /** * The base-url override carried by the request flags: explicit `--base-url` - * wins, else `--env` is mapped through the friendly-name table. Mirrors - * `loginBaseUrl`'s precedence so a command and a login agree on what an env - * name means. + * wins, else `--region`/`--env` is mapped through resolveNamedBaseUrl. Mirrors + * `loginBaseUrl`'s precedence so a command and a login agree on what a region + * or env name means. */ function baseUrlOverrideFromFlags( flags: Record, ): string | undefined { + const envFlag = stringFlag(flags, ['env']); + const regionFlag = stringFlag(flags, ['region']); + assertRegionAndEnvNotBothSet({ envFlag, regionFlag }); const baseUrl = stringFlag(flags, ['base-url']); if (baseUrl) { return baseUrl; } - const env = stringFlag(flags, ['env']); - return env ? resolveEnvBaseUrl(env) : undefined; + return resolveNamedBaseUrl({ + envFlag, + regionFlag, + }); } /** diff --git a/src/credential-store.test.ts b/src/credential-store.test.ts index 22f6b05..c4fbf17 100644 --- a/src/credential-store.test.ts +++ b/src/credential-store.test.ts @@ -290,8 +290,8 @@ describe('credential-store', () => { } }); - it('rejects the reserved name "default"', () => { - expect(() => assertValidProfileName('default')).toThrow(/reserved/); + it('accepts the name "default" (used as the implicit profile)', () => { + expect(() => assertValidProfileName('default')).not.toThrow(); }); it('rejects whitespace, control chars, slashes, empty, and over-length', () => { diff --git a/src/credential-store.ts b/src/credential-store.ts index 60434d5..cefc786 100644 --- a/src/credential-store.ts +++ b/src/credential-store.ts @@ -10,6 +10,8 @@ import { dirname, join } from 'node:path'; import { z } from 'zod'; +import { usageError } from './cli-error'; + /** * On-disk credential store for the `amp` CLI: a versioned file holding named * profiles (one credential + the backend it targets) plus a pointer to the @@ -28,19 +30,22 @@ export const CURRENT_VERSION = 1; // lenient so a name written by a newer CLI (or before this rule existed) // round-trips instead of dropping the profile. Strict identifier charset keeps // names safe to interpolate into `auth list` / `status` output (no newlines or -// control chars to spoof a row) and predictable across tools. +// control chars to spoof a row) and predictable across tools. `default` is a +// valid name: it is the profile the CLI targets when --profile is omitted. export const profileNameSchema = z .string() .min(1, 'must not be empty') .max(64, 'must be at most 64 characters') - .regex(/^[A-Za-z0-9._-]+$/, 'use letters, digits, dot, underscore, or hyphen') - .refine((name) => name !== 'default', 'the name "default" is reserved'); + .regex( + /^[A-Za-z0-9._-]+$/, + 'use letters, digits, dot, underscore, or hyphen', + ); /** Throws a friendly error if `name` is not a valid profile name. */ export function assertValidProfileName(name: string): void { const result = profileNameSchema.safeParse(name); if (!result.success) { - throw new Error( + throw usageError( `Invalid profile name "${name}": ${result.error.issues[0]?.message ?? 'invalid'}.`, ); } @@ -230,7 +235,7 @@ export function setDefault( name: string, ): CredentialStore { if (!store.profiles[name]) { - throw new Error(`No such profile: ${name}`); + throw new Error(`No such profile: ${name}`); // plain-error-ok: internal invariant guarded by callers, which always create or verify the profile first — unreachable in practice. } return { ...store, default: name }; } diff --git a/src/error-contract.test.ts b/src/error-contract.test.ts new file mode 100644 index 0000000..510ed04 --- /dev/null +++ b/src/error-contract.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest'; + +import { + exitCodeForErrorCode, + formatErrorEnvelope, + usageError, +} from './cli-error'; + +describe('error contract: exit-code map', () => { + const cases: Array<{ + label: string; + errorCode: string; + httpStatus?: number; + exitCode: number; + }> = [ + // usage / validation → 2 + { + label: 'validation_error code, no status', + errorCode: 'validation_error', + exitCode: 2, + }, + { + label: 'usage_error code, no status', + errorCode: 'usage_error', + exitCode: 2, + }, + { + label: '400 status fallback', + errorCode: 'something_else', + httpStatus: 400, + exitCode: 2, + }, + { + label: '422 status fallback', + errorCode: 'something_else', + httpStatus: 422, + exitCode: 2, + }, + // auth → 3 + { + label: 'authentication_required code', + errorCode: 'authentication_required', + exitCode: 3, + }, + { label: 'invalid_token code', errorCode: 'invalid_token', exitCode: 3 }, + { + label: 'insufficient_scope code', + errorCode: 'insufficient_scope', + exitCode: 3, + }, + { + label: '401 status fallback', + errorCode: 'something_else', + httpStatus: 401, + exitCode: 3, + }, + { + label: '403 status fallback', + errorCode: 'something_else', + httpStatus: 403, + exitCode: 3, + }, + // not-found → 4 + { label: 'not_found code, no status', errorCode: 'not_found', exitCode: 4 }, + { + label: '404 status fallback', + errorCode: 'something_else', + httpStatus: 404, + exitCode: 4, + }, + // transport → 5 + { + label: 'auth_unavailable code', + errorCode: 'auth_unavailable', + exitCode: 5, + }, + { label: 'upstream_error code', errorCode: 'upstream_error', exitCode: 5 }, + { + label: '500 status fallback', + errorCode: 'something_else', + httpStatus: 500, + exitCode: 5, + }, + { + label: '502 status fallback', + errorCode: 'something_else', + httpStatus: 502, + exitCode: 5, + }, + { + label: '599 status fallback (>=500 catch-all)', + errorCode: 'something_else', + httpStatus: 599, + exitCode: 5, + }, + // generic → 1 + { + label: 'unknown code, no status', + errorCode: 'something_new', + exitCode: 1, + }, + { + label: 'unknown code, unmapped status', + errorCode: 'something_new', + httpStatus: 418, + exitCode: 1, + }, + ]; + + it.each(cases)( + '$label → exit $exitCode', + ({ errorCode, httpStatus, exitCode }) => { + expect(exitCodeForErrorCode(errorCode, httpStatus)).toBe(exitCode); + }, + ); +}); + +describe('error contract: envelope shape', () => { + it('always has status:"error" and error.error_code', () => { + const envelope = JSON.parse( + formatErrorEnvelope(usageError('Missing --project .'), false), + ); + expect(envelope.status).toBe('error'); + expect(typeof envelope.error.error_code).toBe('string'); + expect(envelope.error.error_code).toBe('usage_error'); + }); +}); diff --git a/src/errors.test.ts b/src/errors.test.ts index d8c3bb8..e705ab4 100644 --- a/src/errors.test.ts +++ b/src/errors.test.ts @@ -1,50 +1,40 @@ import { describe, expect, it } from 'vitest'; -import { formatApiError } from './errors'; +import { asProblem, hintForErrorCode } from './errors'; -describe('formatApiError', () => { - it('formats RFC 7807 problem responses with hints', () => { - const message = formatApiError(403, 'Forbidden', { +describe('asProblem', () => { + it('recognizes an RFC 7807 problem body', () => { + const problem = asProblem({ title: 'Insufficient scope', - detail: 'Token missing required scopes.', error_code: 'insufficient_scope', }); - - expect(message).toContain('Insufficient scope (403 Forbidden)'); - expect(message).toContain('Token missing required scopes.'); - expect(message).toContain('amp context'); + expect(problem?.title).toBe('Insufficient scope'); }); - it('includes validation errors when present', () => { - const message = formatApiError(400, 'Bad Request', { - title: 'Validation failed', - error_code: 'validation_error', - validation_errors: [ - { field: 'event_type', message: 'Required', code: 'required' }, - ], - }); - - expect(message).toContain('event_type: Required'); - expect(message).toContain('amp help'); + it('returns undefined for a non-problem body', () => { + expect(asProblem({ message: 'boom' })).toBeUndefined(); + expect(asProblem('gateway unavailable')).toBeUndefined(); + expect(asProblem(null)).toBeUndefined(); }); +}); - it('falls back to JSON for unknown error bodies', () => { - const message = formatApiError(500, 'Internal Server Error', { - message: 'boom', - }); - - expect(message).toContain('500 Internal Server Error'); - expect(message).toContain('"boom"'); +describe('hintForErrorCode', () => { + it('returns a remediation hint for known error codes', () => { + expect(hintForErrorCode('insufficient_scope')).toMatch(/amp context/); + expect(hintForErrorCode('validation_error')).toMatch( + /amp --help/, + ); }); - it('prints non-JSON error bodies directly', () => { - const message = formatApiError( - 502, - 'Bad Gateway', - 'gateway unavailable', - ); + it('keeps the auth hint semantic — no audience-targeting or scripted recipe', () => { + const hint = hintForErrorCode('authentication_required'); + expect(hint).toMatch(/amp auth login/); + expect(hint).not.toMatch(/Agents\/CI|Interactive:|login start` then/); + expect(hintForErrorCode('invalid_token')).toBe(hint); + }); - expect(message).toContain('502 Bad Gateway'); - expect(message).toContain('gateway unavailable'); + it('returns undefined for unknown error codes', () => { + expect(hintForErrorCode('something_new')).toBeUndefined(); + expect(hintForErrorCode(undefined)).toBeUndefined(); }); }); diff --git a/src/errors.ts b/src/errors.ts index 312a581..d9adc5c 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -10,7 +10,7 @@ interface ProblemDetails { }> | null; } -function asProblem(body: unknown): ProblemDetails | undefined { +export function asProblem(body: unknown): ProblemDetails | undefined { if (!body || typeof body !== 'object') { return undefined; } @@ -26,68 +26,23 @@ function asProblem(body: unknown): ProblemDetails | undefined { return undefined; } -function hintForErrorCode(errorCode: string | undefined): string | undefined { +export function hintForErrorCode( + errorCode: string | undefined, +): string | undefined { switch (errorCode) { case 'authentication_required': case 'invalid_token': - return 'Run `amp auth login`, or set AMP_TOKEN for a single shell.'; + return 'Authenticate with `amp auth login`, or set AMP_TOKEN. Run `amp auth --help` for all authentication commands.'; case 'insufficient_scope': return 'Run `amp context` to inspect your token, then re-authenticate with the required access (`amp auth login`).'; case 'validation_error': - return 'Check required flags with `amp help `.'; + return 'Check required flags with `amp --help`.'; case 'not_found': return 'Verify identifiers such as --project, --flag, and --event.'; case 'auth_unavailable': case 'upstream_error': - return 'Retry the request. If it persists, check API status or use --base-url for a different host.'; + return 'Retry the request. If it persists, check API status or try a different region with --region.'; default: return undefined; } } - -export function formatApiError( - status: number, - statusText: string, - body: unknown, -): string { - const problem = asProblem(body); - const lines: string[] = []; - - if (problem) { - const title = problem.title ?? `HTTP ${status}`; - lines.push(`${title} (${status} ${statusText})`); - - if (problem.detail) { - lines.push(problem.detail); - } - - if (problem.validation_errors && problem.validation_errors.length > 0) { - lines.push(''); - lines.push('Validation errors:'); - for (const error of problem.validation_errors) { - lines.push(` - ${error.field}: ${error.message}`); - } - } - - const hint = hintForErrorCode(problem.error_code); - if (hint) { - lines.push(''); - lines.push(hint); - } - - return lines.join('\n'); - } - - if (typeof body === 'string') { - const trimmed = body.trim(); - if (trimmed) { - return `Request failed (${status} ${statusText}):\n${trimmed}`; - } - } - - if (body !== null && body !== undefined) { - return `Request failed (${status} ${statusText}):\n${JSON.stringify(body, null, 2)}`; - } - - return `Request failed (${status} ${statusText}).`; -} diff --git a/src/generated/cli-manifest.ts b/src/generated/cli-manifest.ts index fcb0df1..9b3133c 100644 --- a/src/generated/cli-manifest.ts +++ b/src/generated/cli-manifest.ts @@ -850,6 +850,155 @@ export const CLI_OPERATIONS = [ }, ], }, + { + command: ['charts', 'list'], + method: 'GET', + operationId: 'listCharts', + path: '/v1/projects/{project_id}/charts', + requiredScopes: ['read:analytics'], + summary: 'List charts', + successStatus: 200, + parameters: [ + { + name: 'project_id', + in: 'path', + required: true, + aliases: ['project', 'project-id'], + type: 'string', + }, + { + name: 'cursor', + in: 'query', + required: false, + aliases: ['cursor'], + type: 'string', + }, + { + name: 'limit', + in: 'query', + required: false, + aliases: ['limit'], + type: 'integer', + }, + { + name: 'q', + in: 'query', + required: false, + aliases: ['q', 'query'], + type: 'string', + }, + { + name: 'chart_type', + in: 'query', + required: false, + aliases: ['chart-type', 'type'], + type: 'string', + }, + { + name: 'sort', + in: 'query', + required: false, + aliases: ['sort'], + type: 'string', + }, + ], + body: [], + }, + { + command: ['charts', 'get'], + method: 'GET', + operationId: 'getChart', + path: '/v1/projects/{project_id}/charts/{chart_id}', + requiredScopes: ['read:analytics'], + summary: 'Get chart', + successStatus: 200, + parameters: [ + { + name: 'project_id', + in: 'path', + required: true, + aliases: ['project', 'project-id'], + type: 'string', + }, + { + name: 'chart_id', + in: 'path', + required: true, + aliases: ['chart', 'chart-id'], + type: 'string', + }, + { + name: 'include_definition', + in: 'query', + required: false, + aliases: ['include-definition'], + type: 'boolean', + }, + ], + body: [], + }, + { + command: ['charts', 'query'], + method: 'POST', + operationId: 'queryChart', + path: '/v1/projects/{project_id}/charts/{chart_id}/query', + requiredScopes: ['read:analytics'], + summary: 'Query chart', + successStatus: 200, + parameters: [ + { + name: 'project_id', + in: 'path', + required: true, + aliases: ['project', 'project-id'], + type: 'string', + }, + { + name: 'chart_id', + in: 'path', + required: true, + aliases: ['chart', 'chart-id'], + type: 'string', + }, + ], + body: [ + { + name: 'time_range', + required: false, + aliases: ['time-range'], + type: 'object', + nullable: false, + }, + { + name: 'timezone', + required: false, + aliases: ['timezone'], + type: 'string', + nullable: false, + }, + { + name: 'exclude_incomplete_datapoints', + required: false, + aliases: ['exclude-incomplete-datapoints'], + type: 'boolean', + nullable: false, + }, + { + name: 'group_by_limit', + required: false, + aliases: ['group-by-limit'], + type: 'integer', + nullable: false, + }, + { + name: 'time_series_limit', + required: false, + aliases: ['time-series-limit'], + type: 'integer', + nullable: false, + }, + ], + }, { command: ['flags', 'list'], method: 'GET', diff --git a/src/generated/scopes.ts b/src/generated/scopes.ts new file mode 100644 index 0000000..42f3a97 --- /dev/null +++ b/src/generated/scopes.ts @@ -0,0 +1,6 @@ +/* eslint-disable */ +// This file is generated by scripts/generate-cli-manifest.ts. Do not edit by hand. + +// Base MCP scopes the CLI requests at login, sourced from the centralized +// OAuth scope catalog (amplitude/auth/oauth-scopes). +export const MCP_BASE_SCOPES = ['mcp:read', 'mcp:write'] as const; diff --git a/src/help-drift.test.ts b/src/help-drift.test.ts new file mode 100644 index 0000000..4de5b95 --- /dev/null +++ b/src/help-drift.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { buildCatalog } from './catalog'; +import { listProductSurfaces, printCommandHelp } from './help'; + +function capture(fn: () => void): string { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + fn(); + return log.mock.calls.map((c) => String(c[0])).join('\n'); + } finally { + log.mockRestore(); + } +} + +describe('help drift guard', () => { + it('surfaces every non-auth catalog group as a product-surface row', () => { + const rowLabels = new Set(listProductSurfaces().map((s) => s.label)); + const groups = new Set(buildCatalog().map((c) => c.group)); + for (const group of groups) { + // auth has its own dedicated Usage section, not a product-surface row + if (group === 'auth') continue; + expect(rowLabels.has(group)).toBe(true); + } + }); + + it('renders per-command help for every catalog command', () => { + for (const entry of buildCatalog()) { + const out = capture(() => printCommandHelp(entry.command)); + expect(out).toContain(entry.command.join(' ')); + } + }); +}); diff --git a/src/help-json-drift.test.ts b/src/help-json-drift.test.ts new file mode 100644 index 0000000..43b3c51 --- /dev/null +++ b/src/help-json-drift.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { buildCatalog } from './catalog'; +import { printCommandHelp } from './help'; + +describe('help JSON drift', () => { + it('the rendered JSON help contains every catalog command', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + let out = ''; + try { + printCommandHelp([], { json: true, isTTY: false }); + out = log.mock.calls.map((c) => String(c[0])).join(''); + } finally { + log.mockRestore(); + } + const dumped = new Set( + JSON.parse(out).commands.map((c: { command: string }) => c.command), + ); + for (const entry of buildCatalog()) { + expect(dumped.has(entry.command.join(' '))).toBe(true); + } + }); +}); diff --git a/src/help.test.ts b/src/help.test.ts index e339b66..2948ea3 100644 --- a/src/help.test.ts +++ b/src/help.test.ts @@ -1,17 +1,23 @@ import { describe, expect, it, vi } from 'vitest'; +import { catalogGroups } from './catalog'; +import { CliError } from './cli-error'; import { authHelpText, findOperation, listProductSurfaces, operationsMatchingPrefix, + printCommandHelp, printGlobalHelp, + serializeCatalog, } from './help'; +import { normalizeWhitespace } from './output'; describe('help', () => { - it('lists product surfaces for top-level help', () => { + it('lists all catalog groups for top-level help, including charts', () => { const surfaces = listProductSurfaces().map((surface) => surface.label); - expect(surfaces).toEqual([ + expect(surfaces).toContain('charts'); + expect(surfaces.slice(0, 6)).toEqual([ 'context', 'projects', 'events', @@ -21,6 +27,35 @@ describe('help', () => { ]); }); + it('excludes auth from product surfaces since it has its own help section', () => { + const surfaces = listProductSurfaces().map((surface) => surface.label); + expect(surfaces).not.toContain('auth'); + expect(surfaces).toContain('charts'); + }); + + it('shows per-flag descriptions in per-command help for auth commands', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + printCommandHelp(['auth', 'login', 'start']); + const output = log.mock.calls.map((c) => String(c[0])).join('\n'); + expect(output).toMatch(/Target region/); + } finally { + log.mockRestore(); + } + }); + + it('renders the semantic description as prose, with no "Agents:" heading', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + printCommandHelp(['auth', 'login', 'start']); + const output = log.mock.calls.map((c) => String(c[0])).join('\n'); + expect(output).toMatch(/device authorization/); + expect(output).not.toMatch(/Agents:/); + } finally { + log.mockRestore(); + } + }); + it('finds an exact operation', () => { expect(findOperation(['flags', 'list'])?.operationId).toBe( 'listFeatureFlags', @@ -40,12 +75,27 @@ describe('help', () => { expect(help).toContain('amp auth login'); expect(help).toContain('amp auth status'); expect(help).toContain('--profile'); + expect(help).toContain('--region'); + expect(help).not.toContain('--env'); + expect(help).not.toContain('--base-url'); + }); + + it('mentions the scripting-friendly login start/poll verbs', () => { + const help = authHelpText(); + expect(help).toMatch(/login start/); + expect(help).toMatch(/login poll/); + }); + + it('describes --profile as optional, defaulting to the implicit "default" profile', () => { + const help = authHelpText(); + expect(help).toMatch(/implicit `default` profile/); + expect(help).not.toMatch(/needs both --profile/); }); it('describes DELETE safety without overstating --dry-run support', () => { const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); try { - printGlobalHelp('https://developer-api.amplitude.com'); + printGlobalHelp(); const output = log.mock.calls.map((call) => String(call[0])).join('\n'); expect(output).toContain('Skip interactive confirmation for DELETE'); expect(output).toContain('Preview supported DELETE commands'); @@ -54,4 +104,196 @@ describe('help', () => { log.mockRestore(); } }); + + it('documents --region and not --env/--base-url in global help', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + printGlobalHelp(); + const output = log.mock.calls.map((call) => String(call[0])).join('\n'); + expect(output).toContain('--region '); + expect(output).not.toContain('--env'); + expect(output).not.toContain('--base-url'); + } finally { + log.mockRestore(); + } + }); + + it('collapses embedded newlines and extra whitespace', () => { + expect(normalizeWhitespace('a\n\n b')).toBe('a b'); + }); + + it('orders curated groups before auth', () => { + const groups = catalogGroups().map((group) => group.group); + const curated = groups.filter((group) => group !== 'auth'); + expect(curated).toEqual([ + 'context', + 'projects', + 'events', + 'event-properties', + 'user-properties', + 'flags', + 'charts', + ]); + expect(groups.indexOf('auth')).toBeGreaterThan(groups.indexOf('charts')); + }); + + it('describes the JSON output surface behaviorally in prose global help', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + printGlobalHelp(); + const out = log.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toMatch(/--json/); + expect(out).toMatch(/JSON/); + expect(out).not.toMatch(/agents/i); + } finally { + log.mockRestore(); + } + }); + + it('documents the exit-code / JSON-on-stderr error contract in global help', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + printGlobalHelp(); + const out = log.mock.calls.map((c) => String(c[0])).join('\n'); + expect(out).toMatch(/exit/); + expect(out).toMatch(/JSON/); + expect(out).toMatch(/stderr/); + } finally { + log.mockRestore(); + } + }); +}); + +describe('help JSON', () => { + const capture = (fn: () => void): string => { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + fn(); + return log.mock.calls.map((c) => String(c[0])).join(''); + } finally { + log.mockRestore(); + } + }; + + it('serializeCatalog dumps cli, spec, a drilldown hint, and a compact index of every catalog command', () => { + const dump = serializeCatalog(); + expect(typeof dump.cli).toBe('string'); + expect(typeof dump.spec).toBe('string'); + expect(typeof dump.detail).toBe('string'); + const paths = dump.commands.map((c) => c.command); + expect(paths).toContain('charts list'); + expect(paths).toContain('auth login start'); + }); + + it('empty topic + json → compact index, not full detail', () => { + const parsed = JSON.parse( + capture(() => printCommandHelp([], { json: true, isTTY: false })), + ); + expect(Array.isArray(parsed.commands)).toBe(true); + expect(typeof parsed.detail).toBe('string'); + for (const entry of parsed.commands) { + expect(Object.keys(entry).sort()).toEqual( + ['command', 'group', 'requiredScopes', 'summary'].sort(), + ); + expect(entry).not.toHaveProperty('flags'); + expect(entry).not.toHaveProperty('agentNote'); + expect(entry).not.toHaveProperty('order'); + expect(entry).not.toHaveProperty('destructive'); + } + }); + + it('the compact index is far smaller than a full per-command dump would be', () => { + const out = capture(() => + printCommandHelp([], { json: true, isTTY: false }), + ); + // Old behavior dumped every flag/enum/description for all ~34 commands (~19KB). + // The compact index should stay well under half that. + expect(out.length).toBeLessThan(8000); + }); + + it('exact command + json → full detail, with flags, and no order', () => { + const parsed = JSON.parse( + capture(() => + printCommandHelp(['auth', 'login', 'start'], { + json: true, + isTTY: false, + }), + ), + ); + expect(parsed.command).toBe('auth login start'); + expect(parsed.description).toMatch(/device authorization/); + expect(parsed.description).toMatch(/poll/); + expect(parsed).not.toHaveProperty('agentNote'); + expect(parsed).not.toHaveProperty('order'); + expect(parsed).not.toHaveProperty('destructive'); + }); + + it('exact command + json → flags array with a required flag, no order', () => { + const parsed = JSON.parse( + capture(() => + printCommandHelp(['flags', 'create'], { json: true, isTTY: false }), + ), + ); + expect(parsed.command).toBe('flags create'); + expect(Array.isArray(parsed.flags)).toBe(true); + expect(parsed.flags.some((f: { required: boolean }) => f.required)).toBe( + true, + ); + expect(parsed).not.toHaveProperty('order'); + }); + + it('surface + json → compact index entries for that group only', () => { + const parsed = JSON.parse( + capture(() => printCommandHelp(['flags'], { json: true, isTTY: false })), + ); + expect(parsed.command).toBe('flags'); + expect(typeof parsed.detail).toBe('string'); + expect( + parsed.commands.every((c: { group: string }) => c.group === 'flags'), + ).toBe(true); + for (const entry of parsed.commands) { + expect(entry).not.toHaveProperty('flags'); + expect(entry).not.toHaveProperty('order'); + } + }); + + it('exact command wins over group filter for `context` (both a command and a group)', () => { + const parsed = JSON.parse( + capture(() => + printCommandHelp(['context'], { json: true, isTTY: false }), + ), + ); + expect(parsed.command).toBe('context'); + expect(parsed.commands).toBeUndefined(); + }); + + it('unknown topic + json → throws a usage error instead of returning a bespoke error shape', () => { + expect(() => + printCommandHelp(['nope', 'x'], { json: true, isTTY: false }), + ).toThrow(/Unknown command: nope x/); + }); + + it('unknown topic + prose → throws a usage error instead of printing global help', () => { + expect(() => printCommandHelp(['nope', 'x'])).toThrow( + /Unknown command: nope x/, + ); + }); + + it('unknown topic throws a structured CliError (usage_error, exit 2)', () => { + try { + printCommandHelp(['nope', 'x'], { json: true, isTTY: false }); + expect.unreachable('printCommandHelp should have thrown'); + } catch (error) { + if (!(error instanceof CliError)) throw error; + expect(error.errorCode).toBe('usage_error'); + expect(error.exitCode).toBe(2); + } + }); + + it('json:false keeps prose (contains Usage:)', () => { + const out = capture(() => + printCommandHelp(['flags', 'list'], { json: false }), + ); + expect(out).toContain('Usage:'); + }); }); diff --git a/src/help.ts b/src/help.ts index 8b17e67..fc6b4d8 100644 --- a/src/help.ts +++ b/src/help.ts @@ -1,6 +1,10 @@ import packageJson from '../package.json'; +import { buildCatalog, catalogGroups } from './catalog'; +import type { CatalogCommand } from './catalog'; +import { usageError } from './cli-error'; import type { CliOperation } from './generated/cli-manifest'; import { API_SPEC_VERSION, CLI_OPERATIONS } from './generated/cli-manifest'; +import { formatJsonOutput, normalizeWhitespace } from './output'; export const CLI_VERSION = packageJson.version; @@ -15,23 +19,81 @@ export function formatVersion(): string { return `amp/${CLI_VERSION} (Developer API spec ${API_SPEC_VERSION})`; } -const COMMAND_EXAMPLES: Record = { - context: 'amp context', - 'projects list': 'amp projects list --limit 10', - 'events list': 'amp events list --project --limit 5', - 'events create': - 'amp events create --project --event-type my_event', - 'events get': 'amp events get --project --event ', - 'flags list': 'amp flags list --project --limit 5', - 'flags create': - 'amp flags create --project --key my-flag --name "My Flag"', - 'flags get': 'amp flags get --project --flag ', - 'flags archive': - 'amp flags archive --project --flag --dry-run', +// Compact per-command entry for the index tiers (global + surface). Deliberately +// omits flags/description/example/order — drilling into a single +// command via `--help --json` gets the full CatalogCommand instead. +// +// `command` is serialized as a single invocation string (the tokens typed +// after `amp`), not an array — that's the stable agent-facing identity for a +// command. The resource namespace is carried separately by `group`. +export interface CatalogIndexEntry { + command: string; + summary: string; + group: string; + requiredScopes: string[]; +} + +export type CatalogDetail = Omit & { + command: string; }; -function commandKey(command: string[]): string { - return command.join(' '); +const JSON_DRILLDOWN_HINT = + "Run `amp --help --json` for a command's flags and parameters."; + +export interface CatalogDump { + cli: string; + spec: string; + detail: string; + commands: CatalogIndexEntry[]; +} + +function toIndexEntry(c: CatalogCommand): CatalogIndexEntry { + return { + command: c.command.join(' '), + summary: c.summary, + group: c.group, + requiredScopes: c.requiredScopes, + }; +} + +function toDetail(c: CatalogCommand): CatalogDetail { + const { order, ...rest } = c; + return { ...rest, command: c.command.join(' ') }; +} + +export function serializeCatalog(): CatalogDump { + return { + cli: CLI_VERSION, + spec: API_SPEC_VERSION, + detail: JSON_DRILLDOWN_HINT, + commands: buildCatalog().map(toIndexEntry), + }; +} + +function helpAsJson(command: string[], isTTY: boolean): string { + if (command.length === 0) { + return formatJsonOutput(serializeCatalog(), isTTY); + } + const entry = findCatalogCommand(command); + if (entry) { + return formatJsonOutput(toDetail(entry), isTTY); + } + if (command.length === 1) { + const commands = buildCatalog().filter((c) => c.group === command[0]); + if (commands.length > 0) { + return formatJsonOutput( + { + command: command.join(' '), + detail: JSON_DRILLDOWN_HINT, + commands: commands.map(toIndexEntry), + }, + isTTY, + ); + } + } + throw usageError( + `Unknown command: ${command.join(' ')}. Run \`amp help\` to list commands.`, + ); } export function operationsMatchingPrefix(command: string[]): CliOperation[] { @@ -50,47 +112,6 @@ export function findOperation(command: string[]): CliOperation | undefined { ); } -function operationUsage(operation: CliOperation): string { - const flags = [ - ...operation.parameters - .filter((parameter) => parameter.in !== 'header') - .map((parameter) => - parameter.required - ? `--${parameter.aliases[0]} <${parameter.name}>` - : `[--${parameter.aliases[0]} <${parameter.name}>]`, - ), - ...operation.body.map((property) => - property.required - ? `--${property.aliases[0]} <${property.name}>` - : `[--${property.aliases[0]} <${property.name}>]`, - ), - ]; - - return ` amp ${operation.command.join(' ')} ${flags.join(' ')}`.trimEnd(); -} - -function exampleFor(operation: CliOperation): string | undefined { - return COMMAND_EXAMPLES[commandKey(operation.command)]; -} - -const SURFACE_DESCRIPTIONS: Record = { - context: 'Authenticated user and org context', - projects: 'Projects in your organization', - events: 'Event taxonomy', - 'event-properties': 'Properties on events', - 'user-properties': 'User properties', - flags: 'Feature flags', -}; - -const SURFACE_ORDER = [ - 'context', - 'projects', - 'events', - 'event-properties', - 'user-properties', - 'flags', -] as const; - export interface ProductSurface { command: string[]; description: string; @@ -98,30 +119,13 @@ export interface ProductSurface { } export function listProductSurfaces(): ProductSurface[] { - const byLabel = new Map(); - - for (const operation of CLI_OPERATIONS) { - const label = operation.command[0]; - if (byLabel.has(label)) { - continue; - } - - byLabel.set(label, { - label, - command: operation.command.length === 1 ? operation.command : [label], - description: - SURFACE_DESCRIPTIONS[label] ?? - operation.summary ?? - 'Developer API commands', - }); - } - - // A SURFACE_ORDER label may legitimately be absent when the manifest does - // not include that surface, so skip missing entries instead of asserting. - return SURFACE_ORDER.flatMap((label) => { - const surface = byLabel.get(label); - return surface ? [surface] : []; - }); + return catalogGroups() + .filter((group) => group.group !== 'auth') + .map((group) => ({ + label: group.group, + command: [group.group], + description: group.description, + })); } function printSurfaceOverview(): string { @@ -135,15 +139,15 @@ function printSurfaceOverview(): string { return lines.join('\n'); } -export function printGlobalHelp(defaultApiBaseUrl: string): void { +export function printGlobalHelp(): void { console.log(`amp ${CLI_VERSION} — Amplitude Developer API CLI Usage: - amp auth login --profile --env Authenticate and save a profile - amp auth Inspect, switch, or print credentials - amp logout [--profile |--all] Remove a profile (or all) - amp version Print CLI version - amp help [surface...] Explore commands for a product surface + amp auth login --region Authenticate and save a profile + amp auth Inspect, switch, or print credentials + amp logout [--profile |--all] Remove a profile (or all) + amp version Print CLI version + amp help [surface...] Explore commands for a product surface ${printSurfaceOverview()} @@ -151,9 +155,14 @@ Explore commands: amp help List commands for a surface (e.g. amp help flags) amp --help Flags and examples for one command +Output: + Human-readable at a terminal; JSON when piped or with --json. + amp help --json Full command catalog (compact index) + amp --help --json One command's parameters + Errors: non-zero exit + JSON {"status":"error","error":{"error_code",…}} on stderr + Global flags: - --base-url API base URL, defaults to ${defaultApiBaseUrl} - --env Target env (local|dev|staging|prod|prod-eu); sets the base URL + --region Target region (us|eu); sets the base URL --profile Use a stored profile for this command --token Raw PAT, PAT=, or bearer-compatible token --json Print raw JSON (default when piped) @@ -164,7 +173,6 @@ Global flags: Environment: AMP_TOKEN Raw token (amp_... → PAT, else bearer); overrides stored profiles AMP_PROFILE Stored profile to select by name - AMP_API_BASE_URL API base URL ~/.amplitude/amp/credentials.json Saved profiles from \`amp auth login\``); } @@ -175,49 +183,65 @@ export function authHelpText(): string { 'Authenticate and manage saved credential profiles.', '', 'Usage:', - ' amp auth login --profile --env Device flow → save + activate a profile', - ' amp auth login Re-authenticate the active profile', - ' amp auth pat --with-token --profile --env Save a supplied PAT (stdin/prompt)', - ' amp auth use Switch the active profile (no re-auth)', - ' amp auth list List profiles (* marks the active one)', - ' amp auth status Show the active credential and expiry', - ' amp auth token Print the active access token to stdout', - ' amp logout [--profile |--all] Remove a profile (or all)', + ' amp auth login --region Device flow → save + activate a profile', + ' amp auth login Re-authenticate the active profile', + ' amp auth pat --with-token --region Save a supplied PAT (stdin/prompt)', + ' amp auth use Switch the active profile (no re-auth)', + ' amp auth list List profiles (* marks the active one)', + ' amp auth status Show the active credential and expiry', + ' amp auth token Print the active access token to stdout', + ' amp logout [--profile |--all] Remove a profile (or all)', '', 'Examples:', - ' amp auth login --profile prod --env prod', - ' amp auth login --profile staging --env staging', + ' amp auth login --region us', + ' amp auth login --profile eu --region eu', ' amp auth use prod', ' TOKEN=$(amp auth token)', '', - 'Creating a profile is force-explicit: a new profile needs both --profile', - ' and --env (or --base-url ). Re-authenticating an existing', - 'profile reuses the env recorded on it, so a bare `amp auth login` refreshes', - 'the active profile in place. Login activates the profile it mints and prints', - 'the switch; the active identity never changes without a command.', + 'Creating a profile is force-explicit: --region is required;', + '--profile is optional — omit it and the CLI targets the', + 'implicit `default` profile, created on first use.', + 'Re-authenticating an existing profile reuses the target recorded on it, so a', + 'bare `amp auth login` refreshes the active profile in place. Login activates', + 'the profile it mints and prints the switch; the active identity never changes', + 'without a command.', '', 'A login requests every scope the CLI can use by default, so all commands', 'work immediately.', '', + 'JSON output:', + ' amp auth login start --region --json Begin the device flow', + ' amp auth login poll --json Complete it (add --timeout )', + ' Both always emit a JSON envelope, never prose.', + '', 'Environment:', ' AMP_TOKEN Raw token (amp_... → PAT, else bearer); overrides stored profiles', ' AMP_PROFILE Stored profile to select by name', - ' AMP_API_BASE_URL API base URL', ].join('\n'); } export function printCommandHelp( command: string[], - defaultApiBaseUrl: string, + options: { json?: boolean; isTTY?: boolean } = {}, ): void { + if (options.json) { + console.log( + helpAsJson(command, options.isTTY ?? Boolean(process.stdout.isTTY)), + ); + return; + } + if (command.length === 0) { + printGlobalHelp(); + return; + } if (command.length === 1 && command[0] === 'auth') { console.log(authHelpText()); return; } - const exact = findOperation(command); - if (exact) { - printOperationHelp(exact); + const entry = findCatalogCommand(command); + if (entry) { + printCatalogCommandHelp(entry); return; } @@ -227,38 +251,55 @@ export function printCommandHelp( return; } - printGlobalHelp(defaultApiBaseUrl); + throw usageError( + `Unknown command: ${command.join(' ')}. Run \`amp help\` to list commands.`, + ); } -function printOperationHelp(operation: CliOperation): void { - const lines = [ - `amp ${operation.command.join(' ')}`, - '', - operation.summary ?? 'Call the Developer API.', - '', - 'Usage:', - operationUsage(operation), - ]; +function findCatalogCommand(command: string[]): CatalogCommand | undefined { + return buildCatalog().find( + (c) => + c.command.length === command.length && + c.command.every((part, i) => part === command[i]), + ); +} - const example = exampleFor(operation); - if (example) { - lines.push('', 'Example:', ` ${example}`); +function printCatalogCommandHelp(entry: CatalogCommand): void { + const lines = [`amp ${entry.command.join(' ')}`, '', entry.summary]; + if (entry.description) { + lines.push('', normalizeWhitespace(entry.description)); } + lines.push('', 'Usage:'); + const usageFlags = entry.flags.map((f) => + f.required + ? `--${f.aliases[0]} <${f.name}>` + : `[--${f.aliases[0]} <${f.name}>]`, + ); + lines.push( + ` amp ${entry.command.join(' ')} ${usageFlags.join(' ')}`.trimEnd(), + ); - if (operation.requiredScopes.length > 0) { - lines.push( - '', - 'Required scopes:', - ` ${operation.requiredScopes.join(', ')}`, - ); + const described = entry.flags.filter((f) => f.description); + if (described.length > 0) { + lines.push('', 'Flags:'); + for (const f of described) { + const description = f.description + ? normalizeWhitespace(f.description) + : ''; + lines.push(` --${f.aliases[0]} ${description}`.trimEnd()); + } + } + if (entry.example) { + lines.push('', 'Example:', ` ${entry.example}`); + } + if (entry.requiredScopes.length > 0) { + lines.push('', 'Required scopes:', ` ${entry.requiredScopes.join(', ')}`); } - lines.push( '', - 'Global flags: --base-url, --token, --json, --yes, --body-json', + 'Global flags: --token, --json, --yes, --body-json', 'Run `amp help` for product surfaces.', ); - console.log(lines.join('\n')); } @@ -271,7 +312,7 @@ function printGroupHelp(command: string[], operations: CliOperation[]): void { if (summary) { console.log(` ${summary}`); } - const example = exampleFor(operation); + const example = findCatalogCommand(operation.command)?.example; if (example) { console.log(` e.g. ${example}`); } diff --git a/src/output.ts b/src/output.ts index 090f85c..fbfedca 100644 --- a/src/output.ts +++ b/src/output.ts @@ -34,10 +34,14 @@ function cellValue(value: unknown): string { return String(value); } -function normalizeCell(value: string): string { +export function normalizeWhitespace(value: string): string { return value.replace(/\s+/g, ' ').trim(); } +function normalizeCell(value: string): string { + return normalizeWhitespace(value); +} + function maxWidth(column: string): number { return COLUMN_WIDTHS[column] ?? DEFAULT_CELL_WIDTH; } diff --git a/src/pending-store.test.ts b/src/pending-store.test.ts new file mode 100644 index 0000000..51158cc --- /dev/null +++ b/src/pending-store.test.ts @@ -0,0 +1,88 @@ +import { mkdtempSync, statSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { + emptyPending, + gcExpiredPending, + getPending, + isPendingExpired, + loadPending, + removePending, + savePending, + setPending, +} from './pending-store'; + +const entry = { + device_code: 'dc', + code_verifier: 'cv', + base_url: 'https://developer-api.amplitude.com', + expires_at: '2026-07-15T00:10:00.000Z', + interval: 5, + started_at: '2026-07-15T00:00:00.000Z', +}; + +function tmpFile() { + return join( + mkdtempSync(join(tmpdir(), 'amp-pending-')), + 'pending-logins.json', + ); +} + +function writeRaw(path: string, contents: string) { + writeFileSync(path, contents); +} + +describe('pending-store', () => { + it('round-trips an entry and writes 0600', () => { + const path = tmpFile(); + savePending(setPending(emptyPending(), 'default', entry), path); + expect(getPending(loadPending(path), 'default')).toEqual(entry); + // eslint-disable-next-line no-bitwise -- masking permission bits off st_mode + expect(statSync(path).mode & 0o777).toBe(0o600); + }); + + it('removePending drops the entry', () => { + const path = tmpFile(); + savePending(setPending(emptyPending(), 'default', entry), path); + savePending(removePending(loadPending(path), 'default'), path); + expect(getPending(loadPending(path), 'default')).toBeUndefined(); + }); + + it('treats missing/corrupt/mismatched files as empty', () => { + const path = tmpFile(); + expect(loadPending(path)).toEqual(emptyPending()); // ENOENT + savePending(emptyPending(), path); + writeRaw(path, '{ not json'); + expect(loadPending(path)).toEqual(emptyPending()); + writeRaw( + path, + JSON.stringify({ version: 1, pending: { x: { device_code: 'd' } } }), + ); + expect(loadPending(path)).toEqual(emptyPending()); // bad entry schema + }); + + it('isPendingExpired compares against expires_at', () => { + const at = Date.parse(entry.expires_at); + expect(isPendingExpired(entry, at - 1)).toBe(false); + expect(isPendingExpired(entry, at)).toBe(true); + }); + + it('gcExpiredPending drops expired entries and keeps live ones', () => { + const store = setPending( + setPending(emptyPending(), 'live', { + ...entry, + expires_at: '2026-07-15T00:10:00.000Z', + }), + 'dead', + { ...entry, expires_at: '2026-07-15T00:00:00.000Z' }, + ); + const gced = gcExpiredPending( + store, + Date.parse('2026-07-15T00:05:00.000Z'), + ); + expect(Object.keys(gced.pending)).toEqual(['live']); + }); +}); diff --git a/src/pending-store.ts b/src/pending-store.ts new file mode 100644 index 0000000..77b9a6a --- /dev/null +++ b/src/pending-store.ts @@ -0,0 +1,122 @@ +import { + mkdirSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { homedir } from 'node:os'; +import { dirname, join } from 'node:path'; + +import { z } from 'zod'; + +export const CURRENT_PENDING_VERSION = 1; + +const pendingEntrySchema = z + .object({ + device_code: z.string().min(1), + code_verifier: z.string().min(1), + base_url: z.string().min(1), + expires_at: z.string().min(1), + interval: z.number().int().positive(), + started_at: z.string().min(1), + }) + .catchall(z.unknown()); + +const pendingStoreSchema = z + .object({ + version: z.number().int(), + pending: z.record(z.string(), pendingEntrySchema), + }) + .catchall(z.unknown()); + +export type PendingEntry = z.infer; +export type PendingStore = z.infer; + +export function pendingPath(override?: string): string { + return ( + override ?? join(homedir(), '.amplitude', 'amp', 'pending-logins.json') + ); +} + +export function emptyPending(): PendingStore { + return { version: CURRENT_PENDING_VERSION, pending: {} }; +} + +// Pending entries are disposable: any unreadable / unparseable / schema-mismatched +// file is treated as "nothing in flight" rather than surfaced as an error. +export function loadPending(path: string = pendingPath()): PendingStore { + let raw: string; + try { + raw = readFileSync(path, 'utf8'); + } catch { + return emptyPending(); + } + try { + const parsed = pendingStoreSchema.safeParse(JSON.parse(raw)); + return parsed.success ? parsed.data : emptyPending(); + } catch { + return emptyPending(); + } +} + +export function savePending( + store: PendingStore, + path: string = pendingPath(), +): void { + const dir = dirname(path); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + const tempPath = join( + dir, + `.pending-logins.${process.pid}.${Date.now()}.tmp`, + ); + try { + writeFileSync(tempPath, `${JSON.stringify(store, null, 2)}\n`, { + mode: 0o600, + }); + renameSync(tempPath, path); + } catch (error) { + rmSync(tempPath, { force: true }); + throw error; + } +} + +export function getPending( + store: PendingStore, + name: string, +): PendingEntry | undefined { + return store.pending[name]; +} + +export function setPending( + store: PendingStore, + name: string, + entry: PendingEntry, +): PendingStore { + return { ...store, pending: { ...store.pending, [name]: entry } }; +} + +export function removePending(store: PendingStore, name: string): PendingStore { + const pending = { ...store.pending }; + delete pending[name]; + return { ...store, pending }; +} + +export function isPendingExpired(entry: PendingEntry, now: number): boolean { + const at = Date.parse(entry.expires_at); + return Number.isNaN(at) ? true : at <= now; +} + +/** Returns a copy of the store with expired entries dropped (opportunistic GC). */ +export function gcExpiredPending( + store: PendingStore, + now: number, +): PendingStore { + const pending: Record = {}; + for (const [name, entry] of Object.entries(store.pending)) { + if (!isPendingExpired(entry, now)) { + pending[name] = entry; + } + } + return { ...store, pending }; +} diff --git a/src/request.test.ts b/src/request.test.ts index 6703c1b..315e4a1 100644 --- a/src/request.test.ts +++ b/src/request.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { parseArgs } from './args'; +import { CliError } from './cli-error'; import { CLI_OPERATIONS, type CliOperation } from './generated/cli-manifest'; import { buildRequest, @@ -8,8 +9,6 @@ import { parseResponseBody, } from './request'; -const AUTH = 'Bearer test'; - function operation(command: string[]): CliOperation { const found = CLI_OPERATIONS.find( (candidate) => @@ -26,7 +25,7 @@ function operation(command: string[]): CliOperation { function request(command: string[], argv: string[]) { const parsed = parseArgs([...command, ...argv]); - return buildRequest(operation(command), parsed.flags, AUTH); + return buildRequest(operation(command), parsed.flags); } describe('manifest invariants', () => { @@ -54,6 +53,16 @@ describe('manifest invariants', () => { }); describe('buildRequest', () => { + it('builds headers without an Authorization value — credentials are attached later, at fetch time', () => { + const built = request( + ['events', 'get'], + ['--project', '187520', '--event', 'signup'], + ); + + expect(built.headers.Authorization).toBeUndefined(); + expect(built.headers.Accept).toBe('application/json'); + }); + it('sends bare dry-run flags as query parameters', () => { const built = request( ['events', 'delete'], @@ -118,14 +127,10 @@ describe('buildRequest', () => { it('rejects string-valued flags passed without a value', () => { expect(() => - buildRequest( - operation(['events', 'create']), - { - project: '187520', - 'body-json': true, - }, - AUTH, - ), + buildRequest(operation(['events', 'create']), { + project: '187520', + 'body-json': true, + }), ).toThrowError('Expected --body-json to have a value.'); }); @@ -168,6 +173,59 @@ describe('buildRequest', () => { ).toThrowError('--body-json must be a JSON object.'); }); + it('rejects malformed JSON as a usage error naming the flag, not a raw SyntaxError', () => { + let caught: unknown; + try { + request( + ['events', 'create'], + ['--project', '187520', '--body-json={bad'], + ); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(CliError); + if (!(caught instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(caught.errorCode).toBe('usage_error'); + expect(caught.exitCode).toBe(2); + expect(caught.message).toBe('--body-json must be valid JSON.'); + }); + + it('rejects malformed JSON in an object/array-typed body flag as a usage error', () => { + expect(() => + request( + ['flags', 'create'], + [ + '--project', + '187520', + '--key', + 'k', + '--name', + 'n', + '--rollout-weights', + '{bad', + ], + ), + ).toThrowError('--rollout-weights must be valid JSON.'); + + expect(() => + request( + ['flags', 'create'], + [ + '--project', + '187520', + '--key', + 'k', + '--name', + 'n', + '--target-segments', + '[bad', + ], + ), + ).toThrowError('--target-segments must be valid JSON.'); + }); + it('rejects empty required path and body flags', () => { expect(() => request(['events', 'list'], ['--project='])).toThrowError( 'Missing --project .', @@ -180,6 +238,86 @@ describe('buildRequest', () => { ), ).toThrowError('Missing --event-type .'); }); + + it('throws a CliError with errorCode usage_error for a missing required flag', () => { + let caught: unknown; + try { + request(['events', 'list'], ['--project=']); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(CliError); + if (!(caught instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(caught.errorCode).toBe('usage_error'); + expect(caught.exitCode).toBe(2); + expect(caught.message).toBe('Missing --project .'); + }); +}); + +describe('assertKnownFlags (via buildRequest)', () => { + it('rejects a flag that is valid for another command but not this one', () => { + expect(() => request(['flags', 'list'], ['--key', 'foo'])).toThrowError( + 'Unknown flag --key for `amp flags list`.', + ); + }); + + it('throws a CliError with errorCode usage_error for a misplaced flag', () => { + let caught: unknown; + try { + request(['flags', 'list'], ['--key', 'foo']); + } catch (error) { + caught = error; + } + + expect(caught).toBeInstanceOf(CliError); + if (!(caught instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(caught.errorCode).toBe('usage_error'); + expect(caught.exitCode).toBe(2); + }); + + it('suggests the nearest valid alias for a near-miss flag on this command', () => { + expect(() => request(['flags', 'list'], ['--porject', '1'])).toThrowError( + /Did you mean --project\?/, + ); + }); + + it('does not throw for flags valid on this command, including global flags', () => { + expect(() => + request( + ['flags', 'list'], + ['--project', '187520', '--limit', '5', '--json'], + ), + ).not.toThrow(); + }); + + it('allows a global flag even on a command with no operation-specific parameters', () => { + expect(() => request(['context'], ['--json'])).not.toThrow(); + }); + + it('allows --dry-run globally even on operations that do not declare dry_run', () => { + expect(() => request(['context'], ['--dry-run'])).not.toThrow(); + }); + + it('rejects auth-flow-only globals on an API command instead of silently dropping them', () => { + const cases: Array<[string, string[]]> = [ + ['timeout', ['--timeout', '5']], + ['with-token', ['--with-token']], + ['flow', ['--flow', 'device']], + ['scope', ['--scope', 'read']], + ['force', ['--force']], + ]; + for (const [flag, argv] of cases) { + expect( + () => request(['events', 'list'], ['--project', '187520', ...argv]), + `--${flag} should be rejected on an API command`, + ).toThrowError(`Unknown flag --${flag} for \`amp events list\`.`); + } + }); }); describe('operationSupportsDryRun', () => { diff --git a/src/request.ts b/src/request.ts index b766849..b211b46 100644 --- a/src/request.ts +++ b/src/request.ts @@ -2,11 +2,14 @@ import { randomUUID } from 'node:crypto'; import { type FlagValue, + apiGlobalOptionAliases, flagValue, hasFlag, isMissingRequiredValue, + nearestAlias, stringFlag, } from './args'; +import { usageError } from './cli-error'; import type { CliBodyProperty, CliOperation } from './generated/cli-manifest'; import { jsonRecordSchema } from './schemas'; @@ -53,11 +56,11 @@ function parseScalar( if (value === 'false') { return false; } - throw new Error(`Expected boolean value, got ${value}.`); + throw usageError(`Expected boolean value, got ${value}.`); } if (typeof value === 'boolean') { - throw new Error(`Expected ${type} value, got ${value}.`); + throw usageError(`Expected ${type} value, got ${value}.`); } if (type === 'integer' || type === 'number') { @@ -66,7 +69,7 @@ function parseScalar( !Number.isFinite(parsed) || (type === 'integer' && !Number.isInteger(parsed)) ) { - throw new Error(`Expected ${type} value, got ${value}.`); + throw usageError(`Expected ${type} value, got ${value}.`); } return parsed; } @@ -74,6 +77,17 @@ function parseScalar( return value; } +// A raw JSON.parse throws a SyntaxError, which escapes as a generic error/exit +// 1; a malformed inline JSON value is a local input mistake, so classify it as +// a usage error (exit 2) that names the flag it came from. +function parseJsonFlag(raw: string, label: string): unknown { + try { + return JSON.parse(raw); + } catch { + throw usageError(`${label} must be valid JSON.`); + } +} + function parseBodyValue( property: CliBodyProperty, value: FlagValue, @@ -87,14 +101,14 @@ function parseBodyValue( } if (property.enum && !property.enum.includes(value)) { - throw new Error( + throw usageError( `Invalid --${property.aliases[0]} ${value}. Allowed: ${property.enum.join(', ')}.`, ); } if (property.type === 'array') { if (value.startsWith('[')) { - return JSON.parse(value) as QueryValue[]; + return parseJsonFlag(value, `--${property.aliases[0]}`) as QueryValue[]; } return value .split(',') @@ -103,7 +117,10 @@ function parseBodyValue( } if (property.type === 'object') { - return JSON.parse(value) as Record; + return parseJsonFlag(value, `--${property.aliases[0]}`) as Record< + string, + unknown + >; } return parseScalar(value, property.type, property.nullable); @@ -117,12 +134,12 @@ function parseBodyJson( return {}; } if (raw.trim() === '') { - throw new Error('--body-json must be a JSON object.'); + throw usageError('--body-json must be a JSON object.'); } - const result = jsonRecordSchema.safeParse(JSON.parse(raw)); + const result = jsonRecordSchema.safeParse(parseJsonFlag(raw, '--body-json')); if (!result.success) { - throw new Error('--body-json must be a JSON object.'); + throw usageError('--body-json must be a JSON object.'); } return result.data; @@ -142,14 +159,70 @@ function withQuery(path: string, query: Record): string { return queryString ? `${path}?${queryString}` : path; } +function allowedAliases(operation: CliOperation): Set { + const aliases = new Set(apiGlobalOptionAliases()); + for (const parameter of operation.parameters) { + for (const alias of parameter.aliases) { + aliases.add(alias); + } + } + for (const property of operation.body) { + for (const alias of property.aliases) { + aliases.add(alias); + } + } + return aliases; +} + +/** + * Flags parse successfully against the global commander option set even when + * they belong to a different command (all flags are defined globally, see + * args.ts). Left unchecked, a flag meant for another command — e.g. --key on + * `flags list` — parses fine and is then silently dropped by the loop below, + * which only reads this operation's own parameters/body. Reject anything not + * valid for the resolved command before that can happen. + * + * Shared by both dispatch paths: API operations (via {@link assertKnownFlags}, + * allowed = globals ∪ the operation's manifest-declared flags) and the + * bespoke auth/logout commands in cli.ts (allowed = globals ∪ that command's + * catalog-declared flags), so both reject a misplaced/unknown flag the same + * way instead of the auth/logout path silently dropping it. + */ +export function assertFlagsAllowed( + allowed: Set, + commandLabel: string, + flags: Record, +): void { + for (const key of Object.keys(flags)) { + if (allowed.has(key)) { + continue; + } + + const suggestion = nearestAlias(key, allowed); + const hint = suggestion ? ` Did you mean --${suggestion}?` : ''; + throw usageError(`Unknown flag --${key} for \`${commandLabel}\`.${hint}`); + } +} + +export function assertKnownFlags( + operation: CliOperation, + flags: Record, +): void { + assertFlagsAllowed( + allowedAliases(operation), + `amp ${operation.command.join(' ')}`, + flags, + ); +} + export function buildRequest( operation: CliOperation, flags: Record, - authorizationHeader: string, ): BuiltRequest { + assertKnownFlags(operation, flags); + const headers: Record = { Accept: 'application/json', - Authorization: authorizationHeader, }; const query: Record = {}; let path = operation.path; @@ -161,12 +234,14 @@ export function buildRequest( isMissingRequiredValue(raw) && parameter.in !== 'header' ) { - throw new Error(`Missing --${parameter.aliases[0]} <${parameter.name}>.`); + throw usageError( + `Missing --${parameter.aliases[0]} <${parameter.name}>.`, + ); } if (parameter.in === 'path') { if (typeof raw === 'boolean') { - throw new Error(`Expected --${parameter.aliases[0]} to have a value.`); + throw usageError(`Expected --${parameter.aliases[0]} to have a value.`); } path = path.replace(`{${parameter.name}}`, encodeURIComponent(raw ?? '')); } else if (parameter.in === 'query' && raw !== undefined) { @@ -180,14 +255,14 @@ export function buildRequest( } else if (typeof raw === 'string') { headers[parameter.name] = raw; } else if (raw !== undefined) { - throw new Error(`Expected --${parameter.aliases[0]} to have a value.`); + throw usageError(`Expected --${parameter.aliases[0]} to have a value.`); } } } const body = parseBodyJson(flags); if (operation.body.length === 0 && Object.keys(body).length > 0) { - throw new Error( + throw usageError( `\`amp ${operation.command.join(' ')}\` does not accept a request body.`, ); } @@ -195,14 +270,14 @@ export function buildRequest( for (const property of operation.body) { const raw = flagValue(flags, property.aliases); if (property.required && raw === '') { - throw new Error(`Missing --${property.aliases[0]} <${property.name}>.`); + throw usageError(`Missing --${property.aliases[0]} <${property.name}>.`); } if ( property.required && raw === undefined && !hasFlag(flags, ['body-json']) ) { - throw new Error(`Missing --${property.aliases[0]} <${property.name}>.`); + throw usageError(`Missing --${property.aliases[0]} <${property.name}>.`); } if (raw !== undefined) { diff --git a/src/run.test.ts b/src/run.test.ts index c4e539e..0094007 100644 --- a/src/run.test.ts +++ b/src/run.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { CliError } from './cli-error'; import { CLI_OPERATIONS, type CliOperation } from './generated/cli-manifest'; import { deleteGateDecision, runOperation } from './run'; @@ -172,6 +173,22 @@ describe('runOperation', () => { expect(init.headers.Authorization).toBe('Bearer PAT=amp_secret'); }); + it('validates flags before resolving credentials: a missing required flag surfaces usage_error even with an unresolvable profile', async () => { + const error: unknown = await runOperation(operation(['events', 'get']), { + profile: '__nope__', + event: 'signup', + // --project omitted: should fail validation before touching auth. + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(CliError); + if (!(error instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(error.errorCode).toBe('usage_error'); + expect(error.exitCode).toBe(2); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it('throws a formatted error on non-2xx responses', async () => { fetchMock.mockResolvedValue( jsonResponse(403, { detail: 'insufficient scope' }), @@ -186,6 +203,48 @@ describe('runOperation', () => { ).rejects.toThrow(/403/); }); + it('throws a CliError with exitCode 3 for a 403 insufficient_scope response', async () => { + fetchMock.mockResolvedValue( + jsonResponse(403, { + error_code: 'insufficient_scope', + title: 'Forbidden', + detail: 'Missing scope write:flags', + }), + ); + + const error: unknown = await runOperation(operation(['events', 'get']), { + token: 'amp_test', + project: '187520', + event: 'signup', + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(CliError); + if (!(error instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(error.errorCode).toBe('insufficient_scope'); + expect(error.exitCode).toBe(3); + expect(error.httpStatus).toBe(403); + }); + + it('throws a transport CliError when the fetch call rejects', async () => { + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + + const error: unknown = await runOperation(operation(['events', 'get']), { + token: 'amp_test', + project: '187520', + event: 'signup', + }).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(CliError); + if (!(error instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(error.errorCode).toBe('transport_error'); + expect(error.exitCode).toBe(5); + expect(error.message).toMatch(/Could not reach the API/); + }); + it('blocks a DELETE without --yes in a non-interactive shell', async () => { await expect( runOperation(operation(['events', 'delete']), { diff --git a/src/run.ts b/src/run.ts index a7e560d..bbebe51 100644 --- a/src/run.ts +++ b/src/run.ts @@ -1,10 +1,10 @@ /* eslint-disable no-console */ import { type FlagValue, isFlagEnabled } from './args'; +import { cliErrorFromResponse, transportError, usageError } from './cli-error'; import { authorizationHeaderForToken, resolveAuthFromFlags, } from './credential-resolver'; -import { formatApiError } from './errors'; import type { CliOperation } from './generated/cli-manifest'; import { formatJsonOutput, @@ -72,11 +72,11 @@ async function ensureDeleteAllowed( if (decision === 'block') { if (dryRunRequested && !dryRunSupported) { - throw new Error( + throw usageError( `\`amp ${operation.command.join(' ')}\` does not support --dry-run. Drop --dry-run and pass --yes to confirm a real delete, or run it in an interactive terminal.`, ); } - throw new Error( + throw usageError( 'Pass --yes to run a DELETE command, or use --dry-run if the command supports it.', ); } @@ -85,7 +85,7 @@ async function ensureDeleteAllowed( `This runs DELETE \`amp ${operation.command.join(' ')}\`. Continue?`, ); if (!approved) { - throw new Error('Aborted.'); + throw new Error('Aborted.'); // plain-error-ok: TTY-only user cancellation; unreachable non-interactively. } } @@ -93,25 +93,27 @@ export async function runOperation( operation: CliOperation, flags: Record, ): Promise { + const request = buildRequest(operation, flags); await ensureDeleteAllowed(operation, flags); - const auth = resolveAuthFromFlags(flags); - const request = buildRequest( - operation, - flags, - authorizationHeaderForToken(auth.token), - ); - const response = await fetch(`${auth.baseUrl}${request.path}`, { - method: operation.method, - headers: request.headers, - body: request.body === undefined ? undefined : JSON.stringify(request.body), - }); + let response: Response; + try { + response = await fetch(`${auth.baseUrl}${request.path}`, { + method: operation.method, + headers: { + ...request.headers, + Authorization: authorizationHeaderForToken(auth.token), + }, + body: + request.body === undefined ? undefined : JSON.stringify(request.body), + }); + } catch { + throw transportError(`Could not reach the API at ${auth.baseUrl}.`); + } const parsed = parseResponseBody(await response.text()); if (!response.ok) { - throw new Error( - formatApiError(response.status, response.statusText, parsed), - ); + throw cliErrorFromResponse(response.status, response.statusText, parsed); } const isTTY = Boolean(process.stdout.isTTY); diff --git a/src/scopes.test.ts b/src/scopes.test.ts index e591247..1fae7c6 100644 --- a/src/scopes.test.ts +++ b/src/scopes.test.ts @@ -5,7 +5,7 @@ import { DEFAULT_SCOPES } from './scopes'; describe('DEFAULT_SCOPES', () => { it('is the manifest scope union plus the legacy mcp scopes, sorted', () => { expect(DEFAULT_SCOPES).toBe( - 'mcp:read mcp:write read:flags read:projects read:taxonomy write:flags write:taxonomy', + 'mcp:read mcp:write read:analytics read:flags read:projects read:taxonomy write:flags write:taxonomy', ); }); @@ -14,4 +14,8 @@ describe('DEFAULT_SCOPES', () => { expect(scopes).toContain('mcp:read'); expect(scopes).toContain('mcp:write'); }); + + it('includes read:analytics for charts commands', () => { + expect(DEFAULT_SCOPES.split(' ')).toContain('read:analytics'); + }); }); diff --git a/src/scopes.ts b/src/scopes.ts index 19b9a82..d7ee666 100644 --- a/src/scopes.ts +++ b/src/scopes.ts @@ -1,10 +1,9 @@ import { CLI_OPERATIONS } from './generated/cli-manifest'; - -// Legacy org-level scopes. They map to the granular API read/write scopes -// server-side, but the device-flow client can grant them directly, so include -// them in the default request for full coverage. Added explicitly because no -// CLI command declares them as required. -const MCP_SCOPES = ['mcp:read', 'mcp:write']; +// Legacy org-level scopes, generated from the central OAuth scope catalog. +// They map to the granular API read/write scopes server-side, but the +// device-flow client can grant them directly, so include them in the default +// request for full coverage. No CLI command declares them as required. +import { MCP_BASE_SCOPES } from './generated/scopes'; /** * The scope set a login requests when no `--scope` is given. The granular @@ -15,7 +14,7 @@ const MCP_SCOPES = ['mcp:read', 'mcp:write']; export const DEFAULT_SCOPES: string = [ ...new Set([ ...CLI_OPERATIONS.flatMap((operation) => operation.requiredScopes), - ...MCP_SCOPES, + ...MCP_BASE_SCOPES, ]), ] .sort() diff --git a/src/semantic-copy.test.ts b/src/semantic-copy.test.ts new file mode 100644 index 0000000..59308c5 --- /dev/null +++ b/src/semantic-copy.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { buildCatalog } from './catalog'; +import { hintForErrorCode } from './errors'; +import { authHelpText, printGlobalHelp } from './help'; + +// Generalizes the single audience-targeting/prescriptive-copy guard that used +// to live only on the auth hint (errors.test.ts) to every piece of +// user-facing copy the CLI emits about commands. The principle: describe +// behavior semantically for both readers (humans and agents); prescriptive +// agent recipes don't belong in summaries/descriptions/hints/help. +const BANNED_PATTERNS: RegExp[] = [ + /\bAgents?\s*\/?\s*CI\b/i, // audience segmentation ("Agents/CI: ...") + /\bInteractive:/, // audience segmentation ("Interactive: ...") + /\bfor agents\b/i, // audience targeting + /\bfor scripted\b/i, // audience targeting + /agent-friendly path/i, // audience targeting + /\(agent path\)/i, // audience targeting + /then run .*(until|then)/i, // scripted recipe ("then run X until/then Y") +]; + +const KNOWN_ERROR_CODES = [ + 'authentication_required', + 'invalid_token', + 'insufficient_scope', + 'validation_error', + 'not_found', + 'auth_unavailable', + 'upstream_error', +] as const; + +interface CopySample { + source: string; + text: string; +} + +function capture(fn: () => void): string { + const log = vi.spyOn(console, 'log').mockImplementation(() => undefined); + try { + fn(); + return log.mock.calls.map((call) => String(call[0])).join('\n'); + } finally { + log.mockRestore(); + } +} + +function collectCopy(): CopySample[] { + const samples: CopySample[] = []; + + for (const command of buildCatalog()) { + const label = command.command.join(' '); + samples.push({ + source: `catalog[${label}].summary`, + text: command.summary, + }); + if (command.description) { + samples.push({ + source: `catalog[${label}].description`, + text: command.description, + }); + } + for (const flag of command.flags) { + if (flag.description) { + samples.push({ + source: `catalog[${label}].flags[${flag.name}].description`, + text: flag.description, + }); + } + } + } + + for (const code of KNOWN_ERROR_CODES) { + const hint = hintForErrorCode(code); + if (hint) { + samples.push({ source: `hintForErrorCode(${code})`, text: hint }); + } + } + + samples.push({ + source: 'printGlobalHelp()', + text: capture(() => printGlobalHelp()), + }); + samples.push({ source: 'authHelpText()', text: authHelpText() }); + + return samples; +} + +describe('semantic copy — no audience-targeting or prescriptive language', () => { + const samples = collectCopy(); + + it('collected copy from every catalog command, error hint, and help surface', () => { + // Sanity floor so a refactor that empties `samples` can't silently make + // every pattern check below vacuously pass. + expect(samples.length).toBeGreaterThan(20); + }); + + for (const pattern of BANNED_PATTERNS) { + it(`no collected copy matches ${pattern}`, () => { + const offenders = samples + .filter((sample) => pattern.test(sample.text)) + .map((sample) => `${sample.source}: ${JSON.stringify(sample.text)}`); + expect(offenders).toEqual([]); + }); + } +});