diff --git a/docs/superpowers/plans/2026-09-18-skill-materialization.md b/docs/superpowers/plans/2026-09-18-skill-materialization.md new file mode 100644 index 0000000..0052eee --- /dev/null +++ b/docs/superpowers/plans/2026-09-18-skill-materialization.md @@ -0,0 +1,170 @@ +# Skill Materialization Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add opt-in `--save` delivery that materializes a fetched skill into an immutable temporary file and returns a compact instruction, without changing existing callers. + +**Architecture:** Add a focused materializer that owns validation, canonicalization, hashing, collision resolution, and atomic directory publication. Keep registry transport and default output unchanged; make `skills get --save` opt into materialization and its reader-aware handoff output. + +**Tech Stack:** TypeScript, Node.js standard library, Zod, Vitest + +**Spec:** `docs/superpowers/specs/2026-09-18-skill-materialization.md` + +## Global Constraints + +- Do not use TypeScript type assertions or coercions. +- Return public CLI failures as `CliError` values with actionable messages. +- Do not edit generated artifacts. +- Use test-driven development and preserve the compact/nondecorated piped output contract. +- Without `--save`, preserve the current raw Markdown and `data.document` JSON contracts byte-for-byte. +- Treat `--save=false` as an explicit opt-out. +- Never overwrite a published skill artifact; reuse an exact match or extend the hash prefix for a conflicting path. +- Do not create a commit unless the user explicitly asks for one. + +--- + +### Task 1: Immutable skill materializer + +**Files:** +- Create: `src/skill-materializer.ts` +- Create: `src/skill-materializer.test.ts` + +**Interfaces:** +- Consumes: a requested skill name, fetched Markdown string, and optional `{ rootDirectory, now }` dependencies. +- Produces: `materializeSkill(name, document, deps): MaterializedSkill`, where `MaterializedSkill` contains `name`, `path`, `sha256`, `bytes`, `lines`, and `instruction`. + +- [ ] **Step 1: Write failing tests for canonical materialization** + + Cover the literal 12-character-prefix directory shape, canonical bytes, full + SHA-256 metadata, byte/line counts, file permissions, instruction string, and + absence of staging directories after success using a temporary test root and + fixed clock. + +- [ ] **Step 2: Run the focused test and verify the expected missing-module failure** + + Run: `pnpm vitest run src/skill-materializer.test.ts` + +- [ ] **Step 3: Implement minimal validation and materialization** + + Use `node:crypto`, `node:fs`, `node:os`, and `node:path`. Validate the name, + frontmatter name, and EOF sentinel with a Zod string schema. Canonicalize CRLF + to LF plus one trailing newline, create private parent directories, write + `SKILL.md` completely inside a uniquely named private sibling staging + directory, then atomically rename the staging directory to the final version + directory. Map filesystem failures to actionable `CliError` values and clean + the staging directory on every unsuccessful publication. + +- [ ] **Step 4: Run the focused test and verify it passes** + + Run: `pnpm vitest run src/skill-materializer.test.ts` + +- [ ] **Step 5: Add failing validation tests, then implement actionable failures** + + Cover invalid requested names, mismatched frontmatter names, missing + sentinels, and mismatched sentinels. Each failure must be an `upstream_error` + or `usage_error` `CliError` and must leave no final skill file. + +- [ ] **Step 6: Add failing collision tests** + + Prepublish an exact canonical document at the fixed timestamp and 12-character + prefix, then assert a second materialization returns that path without + changing the existing file metadata. Prepublish different bytes at the same + candidate path, then assert materialization leaves that artifact untouched + and publishes under the same timestamp with a 16-character prefix. Also + assert no staging directories remain after either outcome. + +- [ ] **Step 7: Implement deterministic collision resolution** + + Before publication, compare an existing candidate's exact bytes with the + canonical document and reuse an exact match. For different bytes, try digest + prefix lengths `12, 16, 20, ..., 64`. During a publication race, inspect the + candidate after a failed directory rename: reuse it if the bytes match or + continue to the next prefix if they differ. If the 64-character candidate is + also occupied by different bytes, return an actionable `CliError` without + modifying any existing artifact. + +- [ ] **Step 8: Run the focused materializer tests and verify they pass** + + Run: `pnpm vitest run src/skill-materializer.test.ts` + +### Task 2: Command and help contract + +**Files:** +- Modify: `src/args.ts` +- Modify: `src/args.test.ts` +- Modify: `src/skills-commands.ts` +- Modify: `src/skills-commands.test.ts` +- Modify: `src/catalog.ts` +- Modify: `src/catalog.test.ts` +- Modify: `src/help.ts` +- Modify: `src/help.test.ts` + +**Interfaces:** +- Consumes: `materializeSkill` from Task 1. +- Produces: the existing raw/JSON document output without `--save`; one-line text output with `--save`; and `{ data: MaterializedSkill }` with `--save --json`. + +- [ ] **Step 1: Write failing parsing and flag-validation tests for `--save`** + + Add `save` as an optional boolean global option that is accepted only by + `skills get`. Assert that bare `--save` and `--save=true` opt in, + `--save=false` opts out, and other commands reject it. + +- [ ] **Step 2: Write failing command tests for the opt-in output matrix** + + Preserve the existing assertions for raw Markdown and + `{ data: { name, document } }` without `--save`. Add assertions that + `--save` emits only the materializer instruction at both a TTY and through a + pipe, invokes the materializer exactly once, and that `--save --json` emits + the same materialization fields without `document`. + +- [ ] **Step 3: Run command, argument, catalog, and help tests and verify contract failures** + + Run: `pnpm vitest run src/args.test.ts src/skills-commands.test.ts src/catalog.test.ts src/help.test.ts` + +- [ ] **Step 4: Wire opt-in materialization into `runSkillsGet`** + + Add `save` to the parseable global options and the `skills get` catalog flag + set. Inject the materializer through command dependencies for focused tests. + Always fetch once; without `--save`, retain the existing output branches. + With `--save`, materialize once, emit the instruction plus one newline, and + use the existing JSON formatter when `--json` is also enabled. + +- [ ] **Step 5: Update public catalog and help copy** + + Preserve the description of the default raw and `data.document` contracts. + Explain that `--save` stores a complete skill and prints the instruction + needed to use it, and that combining it with `--json` returns structured + materialization metadata. + +- [ ] **Step 6: Run focused tests and verify they pass** + + Run: `pnpm vitest run src/skill-materializer.test.ts src/args.test.ts src/skills-commands.test.ts src/catalog.test.ts src/help.test.ts` + +### Task 3: Full verification + +**Files:** +- Verify all modified and created files. + +**Interfaces:** +- Consumes: completed Tasks 1 and 2. +- Produces: evidence that the public CLI remains type-safe and its full test suite passes. + +- [ ] **Step 1: Run the TypeScript check** + + Run: `pnpm test:typescript` + +- [ ] **Step 2: Run the complete package test suite** + + Run: `pnpm test` + +- [ ] **Step 3: Run the package build** + + Run: `pnpm build` + +- [ ] **Step 4: Review the diff against the design** + + Confirm the final output is minimal, the file is immutable and complete, no + generated files changed, `--save` always fetches before materializing, and + omitting `--save` retains every existing output and filesystem behavior. + Confirm exact collisions reuse without rewriting, different-byte collisions + extend the digest prefix, and no code path overwrites a published artifact. diff --git a/docs/superpowers/specs/2026-09-18-skill-materialization.md b/docs/superpowers/specs/2026-09-18-skill-materialization.md new file mode 100644 index 0000000..95fc71b --- /dev/null +++ b/docs/superpowers/specs/2026-09-18-skill-materialization.md @@ -0,0 +1,78 @@ +# Skill Materialization Design + +## Goal + +Make `amp skills get ` reliable for agents even when a skill is larger +than a shell tool's inline-output limit, while preserving the existing command +behavior during an opt-in evaluation period. + +## Command contract + +- Without `--save`, `amp skills get ` preserves its current behavior: + raw Markdown goes to stdout, while `--json` returns the document in + `data.document`. +- With `--save`, every invocation fetches the current skill, validates and + canonicalizes it, and publishes or reuses an immutable artifact beneath the + operating system temporary directory: + `/amp/skills//-/SKILL.md`. +- `--save` prints one instruction: + `Read and follow the complete \`\` skill at \`\` for the current task.` +- `--save --json` returns structured materialization metadata instead of + embedding the skill document. +- `--save=false` is equivalent to omitting `--save`. + +## Validation and identity + +- Skill names use lowercase letters, digits, and internal hyphens. +- The Markdown begins with frontmatter whose `name` equals the requested name. +- The final nonblank line is ``. +- Documents are canonicalized to LF line endings and exactly one final newline. +- The directory uses a UTC second-resolution timestamp and at least the first + 12 hexadecimal characters of the canonical document's SHA-256 digest. +- The full canonical bytes, not only the shortened hash in the path, determine + whether an existing artifact is identical and reusable. +- JSON exposes the full digest, byte count, and line count for diagnostics. + +These checks apply to the `--save` path during the evaluation period. The EOF +sentinel is an internal delivery check. The normal instruction does not ask +agents to verify metadata or perform a second consumption workflow. + +## Filesystem behavior + +- Without `--save`, the command performs no filesystem writes. +- Parent directories and the final saved skill file are private to the current + user. +- The CLI writes `SKILL.md` completely inside a uniquely named sibling staging + directory, then atomically renames that directory to the final version path. + The final path is never used as a staging location and a published artifact + is never overwritten. +- If another invocation has already published the same timestamp, hash prefix, + and exact canonical bytes, the CLI removes its staging directory and reuses + the existing path without rewriting `SKILL.md`. +- If a candidate path exists with different bytes, the CLI extends the digest + prefix four hexadecimal characters at a time until it finds an unused path. + If all 64 characters are exhausted, it fails without changing any published + artifact. +- A failed publication removes its private staging directory. It determines a + collision by inspecting the final path after the failed rename rather than + relying on a platform-specific filesystem error code. +- No mutable `latest` pointer is created. Running the command again always + fetches and materializes the current registry response, so an earlier path + cannot silently stand in for a later retrieval. +- Lifecycle cleanup is delegated to the operating system temporary directory. + +## Rollout + +- The website prompt opts into the experiment with + `amp skills get integrating-amplitude --region us --save`. +- Existing users, scripts, redirects, and JSON consumers remain unchanged until + they add `--save`. +- Making saved delivery the default is a separate decision informed by agent + completion quality, unnecessary tool-call count, and compatibility results. + +## Skill authoring + +Entry-point `SKILL.md` files should remain focused on routing, invariants, and +the workflow. Large SDK-specific material should be progressively disclosed +through supporting files or child skills. A practical internal target is at +most 20 KB or 300 lines for the entry file. diff --git a/openapi/bundled/openapi.bundled.json b/openapi/bundled/openapi.bundled.json index 49c2a5a..172e574 100644 --- a/openapi/bundled/openapi.bundled.json +++ b/openapi/bundled/openapi.bundled.json @@ -27,6 +27,10 @@ } ], "tags": [ + { + "name": "Agent Analytics", + "description": "Saved findings about agent cost, efficiency, and quality." + }, { "name": "Projects", "description": "Project discovery and project-scoped navigation." @@ -65,6 +69,137 @@ } ], "paths": { + "/v1/projects/{project_id}/agent-analytics/insights": { + "get": { + "tags": ["Agent Analytics"], + "operationId": "listAgentAnalyticsInsights", + "summary": "List saved Agent Analytics insights", + "description": "Returns saved cost, efficiency, and quality findings, ranked by magnitude\ntimes confidence. These are periodically evaluated findings, not live\nqueries. Healthy and closed findings can remain visible. Thumbs-downed\nfindings are excluded. Use each finding's project_id for individual reads.\n", + "x-required-scopes": ["analytics:read"], + "parameters": [ + { + "$ref": "#/components/parameters/ProjectId" + }, + { + "$ref": "#/components/parameters/Limit" + }, + { + "$ref": "#/components/parameters/Cursor" + }, + { + "name": "agent_name", + "in": "query", + "allowEmptyValue": true, + "description": "Exact agent name. Omit for all scopes; an empty string selects project-level findings only.", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Saved findings and an opaque next-page cursor. Keep the same project and agent filter when paging.", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["data", "pagination"], + "properties": { + "data": { + "type": "array", + "items": { + "$ref": "#/components/schemas/AgentInsight" + } + }, + "pagination": { + "$ref": "#/components/schemas/Pagination" + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationProblem" + }, + "401": { + "$ref": "#/components/responses/Problem" + }, + "403": { + "$ref": "#/components/responses/Problem" + }, + "404": { + "$ref": "#/components/responses/Problem" + }, + "429": { + "$ref": "#/components/responses/RateLimitProblem" + }, + "502": { + "$ref": "#/components/responses/Problem" + } + } + } + }, + "/v1/projects/{project_id}/agent-analytics/insights/{insight_id}": { + "get": { + "tags": ["Agent Analytics"], + "operationId": "getAgentAnalyticsInsight", + "summary": "Get a saved Agent Analytics insight", + "description": "Returns one finding by its stable UUID and own project_id. Missing, cross-project, and thumbs-downed findings return 404.", + "x-required-scopes": ["analytics:read"], + "parameters": [ + { + "$ref": "#/components/parameters/ProjectId" + }, + { + "name": "insight_id", + "in": "path", + "required": true, + "description": "Stable insight UUID returned by the list operation, not a finding key such as tool-failures.", + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "Saved finding.", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["data"], + "properties": { + "data": { + "$ref": "#/components/schemas/AgentInsight" + } + } + } + } + } + }, + "400": { + "$ref": "#/components/responses/ValidationProblem" + }, + "401": { + "$ref": "#/components/responses/Problem" + }, + "403": { + "$ref": "#/components/responses/Problem" + }, + "404": { + "$ref": "#/components/responses/Problem" + }, + "429": { + "$ref": "#/components/responses/RateLimitProblem" + }, + "502": { + "$ref": "#/components/responses/Problem" + } + } + } + }, "/v1/auth/device-authorization": { "post": { "tags": ["Auth"], @@ -4113,6 +4248,134 @@ } } }, + "AgentInsight": { + "type": "object", + "required": [ + "id", + "object", + "project_id", + "agent_name", + "finding_key", + "category", + "title", + "summary", + "recommendation", + "measurements", + "evidence", + "magnitude", + "confidence", + "score", + "status", + "window_start", + "window_end", + "evaluated_at" + ], + "properties": { + "id": { + "type": "string", + "format": "uuid" + }, + "object": { + "type": "string", + "const": "agent_analytics_insight" + }, + "project_id": { + "type": "string", + "description": "The finding's own project, including when listed through a portfolio." + }, + "agent_name": { + "type": "string", + "description": "Empty for project-level findings." + }, + "finding_key": { + "type": "string", + "description": "Rule key, such as tool-failures. Not a unique finding identifier." + }, + "category": { + "type": "string", + "enum": ["cost", "efficiency", "quality"] + }, + "title": { + "type": "string" + }, + "summary": { + "type": "string" + }, + "recommendation": { + "type": "string" + }, + "measurements": { + "type": "array", + "items": { + "type": "object", + "required": ["key", "label", "value", "unit"], + "properties": { + "key": { + "type": "string" + }, + "label": { + "type": "string" + }, + "value": { + "type": "number" + }, + "unit": { + "type": ["string", "null"] + } + } + } + }, + "evidence": { + "type": "object", + "required": ["session_ids", "chart_definition"], + "properties": { + "session_ids": { + "type": "array", + "items": { + "type": "string" + } + }, + "chart_definition": { + "type": ["object", "null"], + "additionalProperties": true, + "description": "Opaque, read-only chart definition. Its nested keys are preserved; the shape is not intended for authoring." + } + } + }, + "magnitude": { + "type": "number", + "minimum": 0, + "maximum": 1 + }, + "confidence": { + "type": "number", + "minimum": 0, + "maximum": 1 + }, + "score": { + "type": "number", + "description": "100 times magnitude times confidence; higher ranks first.", + "minimum": 0, + "maximum": 100 + }, + "status": { + "type": "string", + "enum": ["opened", "in_progress", "closed"] + }, + "window_start": { + "type": "string", + "format": "date-time" + }, + "window_end": { + "type": "string", + "format": "date-time" + }, + "evaluated_at": { + "type": "string", + "format": "date-time" + } + } + }, "DeviceCodeTokenRequest": { "type": "object", "required": ["grant_type", "device_code"], diff --git a/openapi/bundled/openapi.bundled.yaml b/openapi/bundled/openapi.bundled.yaml index 5f300b9..5928c73 100644 --- a/openapi/bundled/openapi.bundled.yaml +++ b/openapi/bundled/openapi.bundled.yaml @@ -19,6 +19,8 @@ servers: security: - bearerAuth: [] tags: + - name: Agent Analytics + description: Saved findings about agent cost, efficiency, and quality. - name: Projects description: Project discovery and project-scoped navigation. - name: Taxonomy Events @@ -38,6 +40,100 @@ tags: - name: Auth description: OAuth device-flow and token endpoints (RFC 8628 / RFC 6749). paths: + /v1/projects/{project_id}/agent-analytics/insights: + get: + tags: + - Agent Analytics + operationId: listAgentAnalyticsInsights + summary: List saved Agent Analytics insights + description: | + Returns saved cost, efficiency, and quality findings, ranked by magnitude + times confidence. These are periodically evaluated findings, not live + queries. Healthy and closed findings can remain visible. Thumbs-downed + findings are excluded. Use each finding's project_id for individual reads. + x-required-scopes: + - analytics:read + parameters: + - $ref: '#/components/parameters/ProjectId' + - $ref: '#/components/parameters/Limit' + - $ref: '#/components/parameters/Cursor' + - name: agent_name + in: query + allowEmptyValue: true + description: Exact agent name. Omit for all scopes; an empty string selects project-level findings only. + schema: + type: string + responses: + '200': + description: Saved findings and an opaque next-page cursor. Keep the same project and agent filter when paging. + content: + application/json: + schema: + type: object + required: + - data + - pagination + properties: + data: + type: array + items: + $ref: '#/components/schemas/AgentInsight' + pagination: + $ref: '#/components/schemas/Pagination' + '400': + $ref: '#/components/responses/ValidationProblem' + '401': + $ref: '#/components/responses/Problem' + '403': + $ref: '#/components/responses/Problem' + '404': + $ref: '#/components/responses/Problem' + '429': + $ref: '#/components/responses/RateLimitProblem' + '502': + $ref: '#/components/responses/Problem' + /v1/projects/{project_id}/agent-analytics/insights/{insight_id}: + get: + tags: + - Agent Analytics + operationId: getAgentAnalyticsInsight + summary: Get a saved Agent Analytics insight + description: Returns one finding by its stable UUID and own project_id. Missing, cross-project, and thumbs-downed findings return 404. + x-required-scopes: + - analytics:read + parameters: + - $ref: '#/components/parameters/ProjectId' + - name: insight_id + in: path + required: true + description: Stable insight UUID returned by the list operation, not a finding key such as tool-failures. + schema: + type: string + format: uuid + responses: + '200': + description: Saved finding. + content: + application/json: + schema: + type: object + required: + - data + properties: + data: + $ref: '#/components/schemas/AgentInsight' + '400': + $ref: '#/components/responses/ValidationProblem' + '401': + $ref: '#/components/responses/Problem' + '403': + $ref: '#/components/responses/Problem' + '404': + $ref: '#/components/responses/Problem' + '429': + $ref: '#/components/responses/RateLimitProblem' + '502': + $ref: '#/components/responses/Problem' /v1/auth/device-authorization: post: tags: @@ -3201,6 +3297,119 @@ components: type: string id_token: type: string + AgentInsight: + type: object + required: + - id + - object + - project_id + - agent_name + - finding_key + - category + - title + - summary + - recommendation + - measurements + - evidence + - magnitude + - confidence + - score + - status + - window_start + - window_end + - evaluated_at + properties: + id: + type: string + format: uuid + object: + type: string + const: agent_analytics_insight + project_id: + type: string + description: The finding's own project, including when listed through a portfolio. + agent_name: + type: string + description: Empty for project-level findings. + finding_key: + type: string + description: Rule key, such as tool-failures. Not a unique finding identifier. + category: + type: string + enum: + - cost + - efficiency + - quality + title: + type: string + summary: + type: string + recommendation: + type: string + measurements: + type: array + items: + type: object + required: + - key + - label + - value + - unit + properties: + key: + type: string + label: + type: string + value: + type: number + unit: + type: + - string + - 'null' + evidence: + type: object + required: + - session_ids + - chart_definition + properties: + session_ids: + type: array + items: + type: string + chart_definition: + type: + - object + - 'null' + additionalProperties: true + description: Opaque, read-only chart definition. Its nested keys are preserved; the shape is not intended for authoring. + magnitude: + type: number + minimum: 0 + maximum: 1 + confidence: + type: number + minimum: 0 + maximum: 1 + score: + type: number + description: 100 times magnitude times confidence; higher ranks first. + minimum: 0 + maximum: 100 + status: + type: string + enum: + - opened + - in_progress + - closed + window_start: + type: string + format: date-time + window_end: + type: string + format: date-time + evaluated_at: + type: string + format: date-time DeviceCodeTokenRequest: type: object required: diff --git a/package.json b/package.json index 3cddb23..30f3800 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "vitest": "^4.1.4" }, "engines": { - "node": "^26.8.2" + "node": "^22.13.0 || >=23.5.0" }, "packageManager": "pnpm@10.33.0+sha512.10568bb4a6afb58c9eb3630da90cc9516417abebd3fabbe6739f0ae795728da1491e9db5a544c76ad8eb7570f5c4bb3d6c637b2cb41bfdcdb47fa823c8649319" } diff --git a/src/auth-commands.ts b/src/auth-commands.ts index 78080c3..e2bc33a 100644 --- a/src/auth-commands.ts +++ b/src/auth-commands.ts @@ -16,10 +16,9 @@ import { } from './authToken'; import { authError, CliError, usageError } from './cli-error'; import { - assertRegionAndEnvNotBothSet, DEFAULT_POLL_TIMEOUT_SECONDS, ENV_BASE_URLS, - resolveNamedBaseUrl, + resolveExplicitBaseUrl, } from './config'; import { resolveAuthFromFlags, @@ -108,16 +107,13 @@ export function loginBaseUrl(args: { regionFlag?: string; existing?: Profile; }): string { - assertRegionAndEnvNotBothSet(args); - if (args.baseUrlFlag) { - return args.baseUrlFlag.replace(/\/$/, ''); - } - const named = resolveNamedBaseUrl({ + const explicitBaseUrl = resolveExplicitBaseUrl({ + baseUrlFlag: args.baseUrlFlag, envFlag: args.envFlag, regionFlag: args.regionFlag, }); - if (named) { - return named; + if (explicitBaseUrl) { + return explicitBaseUrl; } if (args.existing) { return args.existing.base_url; diff --git a/src/catalog.test.ts b/src/catalog.test.ts index 5d15917..646547a 100644 --- a/src/catalog.test.ts +++ b/src/catalog.test.ts @@ -58,7 +58,9 @@ describe('buildCatalog — API operations', () => { ); expect(check).toMatchObject({ - example: 'amp events check-ingestion-by-api-key --api-key ', + description: expect.stringContaining('requires --region '), + example: + 'amp events check-ingestion-by-api-key --api-key --region ', requiredScopes: [], summary: 'Check recent event ingestion with an ingestion API key', }); diff --git a/src/catalog.ts b/src/catalog.ts index 5b14f53..6d92dd2 100644 --- a/src/catalog.ts +++ b/src/catalog.ts @@ -21,6 +21,12 @@ export interface CatalogPositional { required: boolean; } +/** A visible global selector that must appear in a command's usage line. */ +export interface RequiredUsageGlobalFlag { + alias: GlobalOptionAlias; + valueName: string; +} + export interface CatalogCommand { command: string[]; summary: string; @@ -36,6 +42,11 @@ export interface CatalogCommand { * flag the command rejects. */ globalFlags: GlobalOptionAlias[]; + /** + * Curated usage-only selectors. Kept out of JSON detail because global flags + * are validation metadata rather than operation parameters. + */ + requiredUsageGlobalFlags?: RequiredUsageGlobalFlag[]; example?: string; requiredScopes: string[]; } @@ -112,7 +123,7 @@ const EXAMPLES: Partial> = { 'events check-ingestion': 'amp events check-ingestion --project --event-type ', 'events check-ingestion-by-api-key': - 'amp events check-ingestion-by-api-key --api-key ', + 'amp events check-ingestion-by-api-key --api-key --region ', 'flags list': 'amp flags list --project --limit 5', 'flags create': 'amp flags create --project --key my-flag --name "My Flag"', @@ -121,6 +132,19 @@ const EXAMPLES: Partial> = { 'amp flags archive --project --flag --dry-run', }; +const API_COMMAND_DESCRIPTIONS: Partial> = { + 'events check-ingestion-by-api-key': + 'Checks recent event ingestion using an ingestion API key. This command requires --region and never infers an endpoint from credentials or configuration.', +}; + +const REQUIRED_USAGE_GLOBAL_FLAGS: Partial< + Record +> = { + 'events check-ingestion-by-api-key': [ + { alias: 'region', valueName: 'us|eu' }, + ], +}; + // 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; @@ -155,7 +179,9 @@ function apiCommands(): CatalogCommand[] { order: GROUP_ORDER[group], flags: flagsForOperation(operation), globalFlags: apiGlobalOptionAliases(operation.authentication), + requiredUsageGlobalFlags: REQUIRED_USAGE_GLOBAL_FLAGS[key], example: EXAMPLES[key], + description: API_COMMAND_DESCRIPTIONS[key], requiredScopes: operation.requiredScopes, }; }); diff --git a/src/config.test.ts b/src/config.test.ts index c2c830a..f46ce5b 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it } from 'vitest'; import { resolveBaseUrl, + resolveExplicitBaseUrl, resolveNamedBaseUrl, resolveRegionBaseUrl, } from './config'; @@ -82,3 +83,24 @@ describe('resolveNamedBaseUrl', () => { ).toThrow('Pass either --region or --env, not both.'); }); }); + +describe('resolveExplicitBaseUrl', () => { + it('uses a trailing-slash-trimmed base URL over a selected region', () => { + expect( + resolveExplicitBaseUrl({ + baseUrlFlag: 'https://preview.example.com/', + regionFlag: 'eu', + }), + ).toBe('https://preview.example.com'); + }); + + it('returns undefined when no endpoint selector is supplied', () => { + expect(resolveExplicitBaseUrl({})).toBeUndefined(); + }); + + it('rejects region and environment selectors together', () => { + expect(() => + resolveExplicitBaseUrl({ regionFlag: 'us', envFlag: 'staging' }), + ).toThrow('Pass either --region or --env, not both.'); + }); +}); diff --git a/src/config.ts b/src/config.ts index 52353d5..af75082 100644 --- a/src/config.ts +++ b/src/config.ts @@ -68,10 +68,9 @@ export function assertRegionAndEnvNotBothSet(options: { } } -// 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. +// Centralizes the "--region xor --env" precedence shared by every caller that +// accepts the named endpoint selectors. This deliberately does not consider +// defaults such as AMP_API_BASE_URL or a saved profile. export function resolveNamedBaseUrl(options: { envFlag?: string; regionFlag?: string; @@ -86,14 +85,31 @@ export function resolveNamedBaseUrl(options: { return undefined; } +/** + * Resolves only an endpoint that the caller explicitly selected. Commands can + * apply their own fallback policy when this returns undefined (for example, + * login may reuse a saved profile while an API-key ingestion check must fail). + */ +export function resolveExplicitBaseUrl(options: { + baseUrlFlag?: string; + envFlag?: string; + regionFlag?: string; +}): string | undefined { + assertRegionAndEnvNotBothSet(options); + if (options.baseUrlFlag) { + return options.baseUrlFlag.replace(/\/$/, ''); + } + return resolveNamedBaseUrl(options); +} + export function resolveBaseUrl(flags: Record): string { - const namedBaseUrl = resolveNamedBaseUrl({ + const explicitBaseUrl = resolveExplicitBaseUrl({ + baseUrlFlag: stringFlag(flags, ['base-url']), envFlag: stringFlag(flags, ['env']), regionFlag: stringFlag(flags, ['region']), }); return ( - stringFlag(flags, ['base-url']) ?? - namedBaseUrl ?? + explicitBaseUrl ?? apiBaseUrlFromEnv() ?? DEFAULT_API_BASE_URL ).replace(/\/$/, ''); diff --git a/src/credential-resolver.ts b/src/credential-resolver.ts index 75d294d..2bebe9a 100644 --- a/src/credential-resolver.ts +++ b/src/credential-resolver.ts @@ -1,10 +1,6 @@ import { type FlagValue, stringFlag } from './args'; import { authError } from './cli-error'; -import { - assertRegionAndEnvNotBothSet, - DEFAULT_API_BASE_URL, - resolveNamedBaseUrl, -} from './config'; +import { DEFAULT_API_BASE_URL, resolveExplicitBaseUrl } from './config'; import { type CredentialStore, type Profile, @@ -225,21 +221,17 @@ export function resolveAuth(input: ResolveInput = {}): ResolvedAuth { /** * The base-url override carried by the request flags: explicit `--base-url` - * 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. + * wins, else `--region`/`--env` is mapped through resolveExplicitBaseUrl. + * 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; - } - return resolveNamedBaseUrl({ + return resolveExplicitBaseUrl({ + baseUrlFlag: stringFlag(flags, ['base-url']), envFlag, regionFlag, }); diff --git a/src/generated/cli-manifest.ts b/src/generated/cli-manifest.ts index ad1a6e1..3d537d8 100644 --- a/src/generated/cli-manifest.ts +++ b/src/generated/cli-manifest.ts @@ -12,6 +12,7 @@ export interface CliParameter { required: boolean; aliases: string[]; type: string; + allowEmptyValue?: boolean; } export interface CliBodyProperty { @@ -37,6 +38,73 @@ export interface CliOperation { } export const CLI_OPERATIONS = [ + { + command: ['agent-analytics', 'insights', 'list'], + method: 'GET', + operationId: 'listAgentAnalyticsInsights', + path: '/v1/projects/{project_id}/agent-analytics/insights', + requiredScopes: ['analytics:read'], + summary: 'List saved Agent Analytics insights', + successStatus: 200, + parameters: [ + { + name: 'project_id', + in: 'path', + required: true, + aliases: ['project', 'project-id'], + type: 'string', + }, + { + name: 'limit', + in: 'query', + required: false, + aliases: ['limit'], + type: 'integer', + }, + { + name: 'cursor', + in: 'query', + required: false, + aliases: ['cursor'], + type: 'string', + }, + { + name: 'agent_name', + in: 'query', + required: false, + allowEmptyValue: true, + aliases: ['agent-name'], + type: 'string', + }, + ], + body: [], + }, + { + command: ['agent-analytics', 'insights', 'get'], + method: 'GET', + operationId: 'getAgentAnalyticsInsight', + path: '/v1/projects/{project_id}/agent-analytics/insights/{insight_id}', + requiredScopes: ['analytics:read'], + summary: 'Get a saved Agent Analytics insight', + successStatus: 200, + parameters: [ + { + name: 'project_id', + in: 'path', + required: true, + aliases: ['project', 'project-id'], + type: 'string', + }, + { + name: 'insight_id', + in: 'path', + required: true, + aliases: ['insight-id'], + type: 'string', + }, + ], + body: [], + }, { command: ['context'], method: 'GET', diff --git a/src/help.test.ts b/src/help.test.ts index 68880f7..63440a1 100644 --- a/src/help.test.ts +++ b/src/help.test.ts @@ -44,13 +44,13 @@ describe('help', () => { .map((call) => String(call[0])) .join('\n'); expect(apiKeyCommandHelp).toContain( - 'amp events check-ingestion-by-api-key --api-key [--timeout-seconds ]', - ); - expect(apiKeyCommandHelp).toContain( - 'amp events check-ingestion-by-api-key --api-key ', + 'amp events check-ingestion-by-api-key --region --api-key [--timeout-seconds ]', ); + expect(apiKeyCommandHelp).toContain('requires --region '); expect(apiKeyCommandHelp).not.toContain('--project'); expect(apiKeyCommandHelp).not.toContain('--token'); + expect(apiKeyCommandHelp).not.toContain('--base-url'); + expect(apiKeyCommandHelp).not.toContain('--env'); } finally { log.mockRestore(); } @@ -239,6 +239,7 @@ describe('help', () => { 'destination-types', 'destinations', 'skills', + 'agent-analytics', ]); expect(groups.indexOf('auth')).toBeGreaterThan(groups.indexOf('charts')); }); @@ -387,6 +388,21 @@ describe('help JSON', () => { expect(parsed).not.toHaveProperty('order'); }); + it('describes the required visible endpoint selector in API-key ingestion JSON help', () => { + const parsed = JSON.parse( + capture(() => + printCommandHelp(['events', 'check-ingestion-by-api-key'], { + json: true, + isTTY: false, + }), + ), + ); + + expect(parsed.description).toContain('requires --region '); + expect(JSON.stringify(parsed)).not.toContain('--base-url'); + expect(JSON.stringify(parsed)).not.toContain('--env'); + }); + it('surface + json → compact index entries for that group only', () => { const parsed = JSON.parse( capture(() => printCommandHelp(['flags'], { json: true, isTTY: false })), diff --git a/src/help.ts b/src/help.ts index addd409..b925a73 100644 --- a/src/help.ts +++ b/src/help.ts @@ -41,7 +41,7 @@ export interface CatalogIndexEntry { // See catalog-shape.test.ts. export type CatalogDetail = Omit< CatalogCommand, - 'order' | 'command' | 'globalFlags' + 'order' | 'command' | 'globalFlags' | 'requiredUsageGlobalFlags' > & { command: string; }; @@ -78,7 +78,7 @@ function toIndexEntry(c: CatalogCommand): CatalogIndexEntry { } function toDetail(c: CatalogCommand): CatalogDetail { - const { order, globalFlags, ...rest } = c; + const { order, globalFlags, requiredUsageGlobalFlags, ...rest } = c; return { ...rest, command: c.command.join(' ') }; } @@ -311,7 +311,14 @@ function printCatalogCommandHelp(entry: CatalogCommand): void { ? `--${f.aliases[0]} <${f.name}>` : `[--${f.aliases[0]} <${f.name}>]`, ); - const usageArgs = [...usagePositional(entry.positional), ...usageFlags]; + const requiredUsageFlags = (entry.requiredUsageGlobalFlags ?? []).map( + (flag) => `--${flag.alias} <${flag.valueName}>`, + ); + const usageArgs = [ + ...usagePositional(entry.positional), + ...requiredUsageFlags, + ...usageFlags, + ]; lines.push( ` amp ${entry.command.join(' ')} ${usageArgs.join(' ')}`.trimEnd(), ); diff --git a/src/package-engines.test.ts b/src/package-engines.test.ts new file mode 100644 index 0000000..c085517 --- /dev/null +++ b/src/package-engines.test.ts @@ -0,0 +1,23 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; + +const packageManifestSchema = z.object({ + engines: z.object({ + node: z.string(), + }), +}); + +function packageManifest() { + return packageManifestSchema.parse( + JSON.parse(readFileSync(join(__dirname, '..', 'package.json'), 'utf8')), + ); +} + +describe('published package engine contract', () => { + it('matches the supported runtime dependency ranges', () => { + expect(packageManifest().engines.node).toBe('^22.13.0 || >=23.5.0'); + }); +}); diff --git a/src/request.test.ts b/src/request.test.ts index 0a37183..3673a88 100644 --- a/src/request.test.ts +++ b/src/request.test.ts @@ -26,6 +26,35 @@ function request(command: string[], argv: string[]) { } describe('manifest invariants', () => { + it('builds saved insight list/get requests and preserves project-level filtering', () => { + const list = ['agent-analytics', 'insights', 'list']; + expect(operation(list).requiredScopes).toEqual(['analytics:read']); + expect( + request(list, [ + '--project', + '12345', + '--agent-name', + '', + '--limit', + '7', + '--cursor', + '', + ]).path, + ).toBe('/v1/projects/12345/agent-analytics/insights?limit=7&agent_name='); + expect(request(list, ['--project', '12345']).path).toBe( + '/v1/projects/12345/agent-analytics/insights', + ); + expect(request(['projects', 'list'], ['--cursor', '']).path).toBe( + '/v1/projects', + ); + const id = 'a37cf684-96ca-48d8-86ca-9c6213b0ade5'; + expect( + request( + ['agent-analytics', 'insights', 'get'], + ['--project', '12345', '--insight-id', id], + ).path, + ).toBe(`/v1/projects/12345/agent-analytics/insights/${id}`); + }); it('does not generate duplicate aliases within a command', () => { for (const cliOperation of CLI_OPERATIONS) { const aliases = new Map(); diff --git a/src/request.ts b/src/request.ts index b20d84c..50a489e 100644 --- a/src/request.ts +++ b/src/request.ts @@ -145,11 +145,22 @@ function parseBodyJson( return result.data; } -function withQuery(path: string, query: Record): string { +function withQuery( + path: string, + query: Record, + operation: CliOperation, +): string { const params = new URLSearchParams(); for (const [key, value] of Object.entries(query)) { - if (value !== undefined && value !== null && value !== '') { + if ( + value !== undefined && + value !== null && + (value !== '' || + operation.parameters.some( + (parameter) => parameter.name === key && parameter.allowEmptyValue, + )) + ) { params.set(key, String(value)); } } @@ -297,6 +308,6 @@ export function buildRequest( ? body : undefined, headers, - path: withQuery(path, query), + path: withQuery(path, query, operation), }; } diff --git a/src/run.test.ts b/src/run.test.ts index e7c59d0..e2b27a7 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 type { FlagValue } from './args'; import { CliError } from './cli-error'; import * as credentialResolver from './credential-resolver'; import type { OAuthCredential } from './credential-store'; @@ -201,6 +202,7 @@ describe('runOperation', () => { afterEach(() => { vi.useRealTimers(); + vi.unstubAllEnvs(); vi.unstubAllGlobals(); vi.restoreAllMocks(); logSpy.mockRestore(); @@ -280,6 +282,94 @@ describe('runOperation', () => { expect(logSpy).toHaveBeenCalledWith(JSON.stringify(envelope)); }); + it('suggests the authenticated ingestion check after an API-key rate limit', async () => { + fetchMock.mockResolvedValue(jsonResponse(429, { title: 'Rate limited' })); + + const error: unknown = await runOperation( + operation(['events', 'check-ingestion-by-api-key']), + { + 'api-key': 'project-api-key', + 'base-url': 'https://developer-api.example.com', + }, + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(CliError); + if (!(error instanceof CliError)) { + throw new Error('Expected a CliError.'); + } + expect(error.httpStatus).toBe(429); + expect(error.hint).toBe( + 'If you continue to see this error, retry with `amp events check-ingestion`.', + ); + }); + + it('requires an explicit endpoint for an API-key ingestion check', async () => { + vi.stubEnv('AMP_API_BASE_URL', 'https://configured.example.com'); + + await expect( + runOperation(operation(['events', 'check-ingestion-by-api-key']), { + 'api-key': 'project-api-key', + }), + ).rejects.toMatchObject({ + errorCode: 'usage_error', + message: 'Checking ingestion by API key requires --region .', + }); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('keys the API-key endpoint requirement to its generated operation ID', async () => { + const apiKeyIngestionOperation: CliOperation = { + ...operation(['events', 'check-ingestion-by-api-key']), + command: ['events', 'renamed-ingestion-check'], + }; + + await expect( + runOperation(apiKeyIngestionOperation, { 'api-key': 'project-api-key' }), + ).rejects.toMatchObject({ + errorCode: 'usage_error', + message: 'Checking ingestion by API key requires --region .', + }); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('does not disclose an invalid hidden environment selector for an API-key ingestion check', async () => { + await expect( + runOperation(operation(['events', 'check-ingestion-by-api-key']), { + 'api-key': 'project-api-key', + env: 'not-a-real-environment', + }), + ).rejects.toMatchObject({ + errorCode: 'usage_error', + message: 'Checking ingestion by API key requires --region .', + }); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + const hiddenEndpointFlagSets: Array> = [ + { 'base-url': true }, + { env: 'staging', region: 'us' }, + ]; + + it.each(hiddenEndpointFlagSets)( + 'does not disclose hidden endpoint selectors in API-key ingestion errors', + async (hiddenFlags) => { + await expect( + runOperation(operation(['events', 'check-ingestion-by-api-key']), { + 'api-key': 'project-api-key', + ...hiddenFlags, + }), + ).rejects.toMatchObject({ + errorCode: 'usage_error', + message: 'Checking ingestion by API key requires --region .', + }); + + expect(fetchMock).not.toHaveBeenCalled(); + }, + ); + it('targets the selected region for an API-key ingestion check', async () => { fetchMock.mockResolvedValue( jsonResponse(200, { @@ -305,6 +395,31 @@ describe('runOperation', () => { ); }); + it('targets an explicit environment for an API-key ingestion check', async () => { + fetchMock.mockResolvedValue( + jsonResponse(200, { + data: { + status: 'observed', + window: { + lookback_hours: 8, + start: '2026-09-03T09:00:00Z', + end: '2026-09-03T17:00:00Z', + basis: 'server_upload_time', + }, + }, + }), + ); + + await runOperation(operation(['events', 'check-ingestion-by-api-key']), { + 'api-key': 'project-api-key', + env: 'staging', + }); + + expect(fetchMock.mock.calls[0]?.[0]).toBe( + 'https://developer-api.stag2.amplitude.com/v1/events/check-recent-ingestion', + ); + }); + it('polls API-key ingestion checks using server guidance', async () => { vi.useFakeTimers(); const window = { diff --git a/src/run.ts b/src/run.ts index 82890bc..e953dd7 100644 --- a/src/run.ts +++ b/src/run.ts @@ -3,10 +3,15 @@ import { setTimeout as sleep } from 'node:timers/promises'; import { z } from 'zod'; -import { type FlagValue, isFlagEnabled } from './args'; -import { cliErrorFromResponse, transportError, usageError } from './cli-error'; +import { type FlagValue, isFlagEnabled, stringFlag } from './args'; +import { + CliError, + cliErrorFromResponse, + transportError, + usageError, +} from './cli-error'; import { deviceIdHeader } from './client-identity'; -import { resolveBaseUrl } from './config'; +import { resolveBaseUrl, resolveExplicitBaseUrl } from './config'; import { authorizationHeaderForToken, resolveAuthWithRefresh, @@ -39,6 +44,59 @@ const ingestionCheckResponseSchema = z.object({ }), }); +const API_KEY_INGESTION_RATE_LIMIT_HINT = + 'If you continue to see this error, retry with `amp events check-ingestion`.'; + +function errorForOperationResponse(options: { + operation: CliOperation; + response: Response; + responseBody: unknown; +}): CliError { + const error = cliErrorFromResponse( + options.response.status, + options.response.statusText, + options.responseBody, + ); + if ( + options.operation.operationId === 'checkRecentEventIngestionByApiKey' && + options.response.status === 429 + ) { + error.hint = API_KEY_INGESTION_RATE_LIMIT_HINT; + } + return error; +} + +function resolveUnauthenticatedOperationBaseUrl( + operation: CliOperation, + flags: Record, +): string { + if (operation.operationId !== 'checkRecentEventIngestionByApiKey') { + return resolveBaseUrl(flags); + } + + const hasHiddenEndpointSelector = + flags['base-url'] !== undefined || flags.env !== undefined; + let explicitBaseUrl: string | undefined; + try { + explicitBaseUrl = resolveExplicitBaseUrl({ + baseUrlFlag: stringFlag(flags, ['base-url']), + envFlag: stringFlag(flags, ['env']), + regionFlag: stringFlag(flags, ['region']), + }); + } catch (error) { + if (hasHiddenEndpointSelector && error instanceof CliError) { + throw usageError( + 'Checking ingestion by API key requires --region .', + ); + } + throw error; + } + if (explicitBaseUrl) { + return explicitBaseUrl; + } + throw usageError('Checking ingestion by API key requires --region .'); +} + function resolveRequestPollingDurationSeconds( operation: CliOperation, body: Record | undefined, @@ -245,7 +303,7 @@ export async function runOperation( signal?: AbortSignal, ): Promise => sendHttpRequest({ - baseUrl: resolveBaseUrl(flags), + baseUrl: resolveUnauthenticatedOperationBaseUrl(operation, flags), headers, signal, }); @@ -307,11 +365,7 @@ export async function runOperation( } if (!response.ok) { - throw cliErrorFromResponse( - response.status, - response.statusText, - responseBody, - ); + throw errorForOperationResponse({ operation, response, responseBody }); } const isTTY = Boolean(process.stdout.isTTY);