From d2496f8b0b409d3d92f631c36cff278e57ef2761 Mon Sep 17 00:00:00 2001 From: Robin Timan Date: Mon, 19 Oct 2026 15:30:03 +0200 Subject: [PATCH 1/2] refactor(cli): add shared Metrics HTTP command foundation Co-Authored-By: Xirp Agent (gpt-6.1-sol) --- packages/cli/__tests__/commands/data.test.ts | 51 ++++ .../__tests__/features/data/dry-run.test.ts | 284 ++++++++++++++++++ .../cli/__tests__/network/metrics.test.ts | 194 ++++++++++++ packages/cli/bin/cli.ts | 2 + packages/cli/src/commands/data.ts | 16 + packages/cli/src/commands/index.ts | 1 + packages/cli/src/features/data/index.ts | 2 + packages/cli/src/features/data/output.ts | 26 ++ packages/cli/src/features/data/request.ts | 34 +++ packages/cli/src/network/index.ts | 2 + packages/cli/src/network/metrics-types.ts | 28 ++ packages/cli/src/network/metrics.ts | 30 ++ packages/cli/src/utils/api-error.ts | 11 + packages/cli/src/utils/index.ts | 1 + packages/cli/src/utils/telemetry.ts | 13 +- packages/core/__tests__/api/client.test.ts | 176 +++++++++++ packages/core/src/api/client.ts | 66 +++- packages/core/src/api/types.ts | 6 + 18 files changed, 927 insertions(+), 16 deletions(-) create mode 100644 packages/cli/__tests__/commands/data.test.ts create mode 100644 packages/cli/__tests__/features/data/dry-run.test.ts create mode 100644 packages/cli/__tests__/network/metrics.test.ts create mode 100644 packages/cli/src/commands/data.ts create mode 100644 packages/cli/src/features/data/index.ts create mode 100644 packages/cli/src/features/data/output.ts create mode 100644 packages/cli/src/features/data/request.ts create mode 100644 packages/cli/src/network/metrics-types.ts create mode 100644 packages/cli/src/network/metrics.ts create mode 100644 packages/cli/src/utils/api-error.ts diff --git a/packages/cli/__tests__/commands/data.test.ts b/packages/cli/__tests__/commands/data.test.ts new file mode 100644 index 00000000..1bf9cd8e --- /dev/null +++ b/packages/cli/__tests__/commands/data.test.ts @@ -0,0 +1,51 @@ +import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import yargs from 'yargs'; +import { prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { dataCommand } from '@commands/index.js'; + +function run(args: string[]) { + let output = ''; + const sut = yargs(args).scriptName('confidence').command(dataCommand).help().exitProcess(false); + return sut + .parseAsync(args, (_error: Error | undefined, _argv: unknown, text: string) => { + output = text; + }) + .then(() => output); +} + +describe('data command group', () => { + it('is registered in the real root help alongside existing commands', () => { + using _auth = prepareAuthTokens('none'); + const sut = fileURLToPath(new URL('../../bin/cli.ts', import.meta.url)); + + const output = execFileSync(process.execPath, ['--import', 'tsx', sut, '--help'], { + encoding: 'utf8', + env: { ...process.env, CONFIDENCE_TELEMETRY: 'false' }, + }); + + expect(output).toContain('confidence data'); + expect(output).toContain('confidence warehouse'); + expect(output).toContain('confidence flags'); + }); + + it('shows help without authentication or placeholder API subcommands', async () => { + using _auth = prepareAuthTokens('none'); + const sut = run; + + const output = await sut(['data', '--help']); + + expect(output).toContain('confidence data '); + expect(output).not.toMatch(/onboard|discover|proposal|workflow/iu); + }); + + it('shows guidance when invoked without a subcommand', async () => { + const sut = run; + expect(await sut(['data'])).toContain('confidence data --help'); + }); + + it('rejects unimplemented subcommands', async () => { + const sut = run; + expect(await sut(['data', 'invented'])).toContain('Unknown argument: invented'); + }); +}); diff --git a/packages/cli/__tests__/features/data/dry-run.test.ts b/packages/cli/__tests__/features/data/dry-run.test.ts new file mode 100644 index 00000000..ad31223c --- /dev/null +++ b/packages/cli/__tests__/features/data/dry-run.test.ts @@ -0,0 +1,284 @@ +import { mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt, prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { resetTelemetry } from '@spotify-confidence/core'; +import { executeDataRequest, printData } from '@features/data/index.js'; +import { metricsRequest } from '@network/metrics.js'; +import { requireRecord, requireString } from '@network/metrics-types.js'; +import { flushTelemetry, telemetryMiddleware } from '@utils/telemetry.js'; +import { captureOutput } from '../../helpers/capture.js'; + +const TEST_URL = 'https://metrics.eu.confidence.dev/v1/testResources'; + +function inputFile(contents: string) { + const dir = mkdtempSync(join(tmpdir(), 'confidence-data-test-')); + const path = join(dir, 'input.json'); + writeFileSync(path, contents); + return { + path, + [Symbol.dispose]() { + rmSync(dir, { recursive: true, force: true }); + }, + }; +} + +// A feature-like caller validates real input before the shared execution helper. +async function runValidatedFile(argv: Record) { + const input = requireRecord( + JSON.parse(readFileSync(argv['from-file'] as string, 'utf8')), + 'Input', + ); + requireString(input.displayName, 'displayName'); + await executeDataRequest( + argv, + (token) => metricsRequest(token, { path: '/v1/testResources', method: 'POST', body: input }), + { method: 'POST', path: '/v1/testResources', body: input }, + ); +} + +afterEach(() => { + resetTelemetry(); + vi.unstubAllEnvs(); +}); + +describe('Data dry-runs', () => { + it('previews validated input before auth with no HTTP or file writes', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using file = inputFile('{"displayName":"Example"}'); + using output = captureOutput(); + const configDir = process.env.CONFIDENCE_CONFIG_DIR!; + const before = readdirSync(configDir, { recursive: true }); + let calls = 0; + server.use( + http.all('*', () => { + calls++; + return HttpResponse.json({}); + }), + ); + const sut = runValidatedFile; + + // Act + await sut({ 'dry-run': true, 'from-file': file.path, json: true, profile: '../invalid' }); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ + data: { method: 'POST', path: '/v1/testResources', body: { displayName: 'Example' } }, + }); + expect(output.stderr).toBe(''); + expect(calls).toBe(0); + expect(readdirSync(configDir, { recursive: true })).toEqual(before); + expect(readFileSync(file.path, 'utf8')).toBe('{"displayName":"Example"}'); + }); + + it.each(['invalid JSON', '[]', '{"displayName":42}'])( + 'still rejects invalid local input %s before preview or auth', + async (contents) => { + using _auth = prepareAuthTokens('none'); + using file = inputFile(contents); + using output = captureOutput(); + const sut = runValidatedFile; + + await expect(sut({ 'dry-run': true, 'from-file': file.path, json: true })).rejects.toThrow(); + + expect(output.stdout).toBe(''); + expect(output.stderr).toBe(''); + }, + ); + + it('has a useful local preview when none is supplied', async () => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + let calls = 0; + const sut = executeDataRequest; + + await sut({ 'dry-run': true, json: true }, async () => { + calls++; + return { ok: true, data: {} }; + }); + + expect(JSON.parse(output.stdout)).toEqual({ data: { dryRun: true } }); + expect(calls).toBe(0); + }); +}); + +describe('Data request execution', () => { + it.each(['none', 'expired'] as const)( + 'fails without usable %s credentials before sending', + async (auth) => { + using _auth = prepareAuthTokens(auth); + using output = captureOutput(); + let calls = 0; + const sut = executeDataRequest; + + await sut({}, async () => { + calls++; + return { ok: true, data: {} }; + }); + + expect(calls).toBe(0); + expect(output.stderr).toMatch(/Not logged in|Token expired/u); + expect(process.exitCode).toBe(1); + }, + ); + + it('sends once and preserves state, errors, and pagination in the JSON envelope', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using file = inputFile('{"displayName":"Example"}'); + using output = captureOutput(); + let calls = 0; + const data = { state: 'FAILED', error: { message: 'Resource failed' }, nextPageToken: 'next' }; + server.use( + http.post(TEST_URL, () => { + calls++; + return HttpResponse.json(data); + }), + ); + const previousExitCode = process.exitCode; + const sut = runValidatedFile; + + // Act + await sut({ 'from-file': file.path, json: true }); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data }); + expect(output.stderr).toBe(''); + expect(process.exitCode).toBe(previousExitCode); + expect(calls).toBe(1); + }); + + it('uses the requested auth profile', async () => { + // Arrange + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const profileDir = join(process.env.CONFIDENCE_CONFIG_DIR!, 'profiles', 'metrics'); + const token = buildTestJwt({ email: 'profile@example.com' }); + mkdirSync(profileDir, { recursive: true }); + writeFileSync(join(profileDir, 'credentials.json'), JSON.stringify({ accessToken: token })); + server.use( + http.get(TEST_URL, ({ request }) => + HttpResponse.json({ + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + const sut = executeDataRequest; + + // Act + await sut({ profile: 'metrics', json: true }, (value) => + metricsRequest(value, { path: '/v1/testResources' }), + ); + + // Assert + expect(JSON.parse(output.stdout)).toEqual({ data: { authorized: true } }); + }); + + it('prints a sanitized failure, omits arbitrary details, and exits nonzero without replay', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let calls = 0; + server.use( + http.get(TEST_URL, () => { + calls++; + return HttpResponse.json( + { + code: 7, + message: 'Denied\n\u001b[31m Bearer private-credential', + details: [{ secret: 'private' }], + }, + { status: 403 }, + ); + }), + ); + const sut = executeDataRequest; + + // Act + await sut({ json: true }, (token) => metricsRequest(token, { path: '/v1/testResources' })); + + // Assert + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('Denied'); + expect(output.stderr).not.toContain('private'); + expect(output.stderr).not.toContain('\u001b'); + expect(process.exitCode).toBe(1); + expect(calls).toBe(1); + }); + + it('reports thrown response validation errors as nonzero failures', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const sut = executeDataRequest; + + await sut({}, async () => { + throw new Error('Invalid resource response'); + }); + + expect(output.stderr).toContain('Invalid resource response'); + expect(process.exitCode).toBe(1); + }); + + it('renders nested resources as readable output when requested', () => { + using output = captureOutput(); + const sut = printData; + + sut({ name: 'testResources/a', status: { state: 'PENDING' } }, { output: 'table' }); + + expect(output.stdout).toContain('Field'); + expect(output.stdout).toContain('testResources/a'); + expect(output.stdout).toContain('PENDING'); + expect(output.stdout).not.toContain('[object Object]'); + }); +}); + +describe('dry-run telemetry suppression', () => { + it.each([ + ['data', 'entities', 'list'], + ['data', 'sql', 'preview'], + ['warehouse', 'list'], + ['warehouse', 'get'], + ])('sends no telemetry for %j even when telemetry is enabled', async (...command) => { + vi.stubEnv('CONFIDENCE_TELEMETRY', 'true'); + let calls = 0; + server.use( + http.all('*', () => { + calls++; + return HttpResponse.json({ clientSecret: 'test' }); + }), + ); + const sut = telemetryMiddleware; + + sut({ _: command, telemetry: true, 'dry-run': true }); + await flushTelemetry(); + + expect(calls).toBe(0); + }); + + it.each([ + { command: ['flags', 'list'], dryRun: true }, + { command: ['warehouse', 'create'], dryRun: true }, + { command: ['data', 'entities', 'list'], dryRun: false }, + ])('preserves existing telemetry behavior for %j', async ({ command, dryRun }) => { + vi.stubEnv('CONFIDENCE_TELEMETRY', 'true'); + const steps: string[] = []; + server.use( + http.post('https://onboarding.confidence.dev/v1/agentTelemetryKey:acquire', () => + HttpResponse.json({ clientSecret: 'test' }), + ), + http.post('https://events.eu.confidence.dev/v1/events:publish', async ({ request }) => { + const body = (await request.json()) as { events: { payload: { step: string } }[] }; + steps.push(body.events[0].payload.step); + return HttpResponse.json({}); + }), + ); + const sut = telemetryMiddleware; + + sut({ _: command, telemetry: true, 'dry-run': dryRun }); + await flushTelemetry(); + + expect(steps).toEqual([command.join('.')]); + }); +}); diff --git a/packages/cli/__tests__/network/metrics.test.ts b/packages/cli/__tests__/network/metrics.test.ts new file mode 100644 index 00000000..ea9cc088 --- /dev/null +++ b/packages/cli/__tests__/network/metrics.test.ts @@ -0,0 +1,194 @@ +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt } from '@spotify-confidence/testing/auth'; +import { metricsRequest } from '@network/metrics.js'; +import { isRecord, requireRecord, requireString, resourcePath } from '@network/metrics-types.js'; + +afterEach(() => vi.unstubAllEnvs()); + +describe('Metrics requests', () => { + it.each(['EU', 'US'])('routes to %s and authenticates the request', async (region) => { + const token = buildTestJwt({ 'https://confidence.dev/region': region }); + server.use( + http.get( + `https://metrics.${region.toLowerCase()}.confidence.dev/v1/testResources`, + ({ request }) => + HttpResponse.json({ + authorized: request.headers.get('Authorization') === `Bearer ${token}`, + }), + ), + ); + const sut = metricsRequest; + + const result = await sut(token, { path: '/v1/testResources' }); + + expect(result).toEqual({ ok: true, data: { authorized: true } }); + }); + + it('honors the API base override and sends body and explicit pagination parameters', async () => { + vi.stubEnv('CONFIDENCE_API_BASE_URL', 'http://localhost:9876'); + server.use( + http.post('http://localhost:9876/v1/testResources', async ({ request }) => + HttpResponse.json({ + body: await request.json(), + params: Object.fromEntries(new URL(request.url).searchParams), + }), + ), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + method: 'POST', + body: { displayName: 'Example' }, + params: { page_size: '10', page_token: 'opaque?token/#' }, + }); + + expect(result).toEqual({ + ok: true, + data: { + body: { displayName: 'Example' }, + params: { page_size: '10', page_token: 'opaque?token/#' }, + }, + }); + }); + + it('preserves structured failures without retrying writes', async () => { + let calls = 0; + const error = { code: 3, message: 'Invalid input', details: [{ field: 'name' }] }; + server.use( + http.post('https://metrics.eu.confidence.dev/v1/testResources', () => { + calls++; + return HttpResponse.json(error, { status: 400 }); + }), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { path: '/v1/testResources', method: 'POST' }); + + expect(result).toEqual({ ok: false, status: 400, error }); + expect(calls).toBe(1); + }); + + it('does not replay a redirected write or send credentials to its location', async () => { + let redirectedCalls = 0; + server.use( + http.post( + 'https://metrics.eu.confidence.dev/v1/testResources', + () => + new HttpResponse(null, { + status: 307, + headers: { Location: 'https://metrics.eu.confidence.dev/v1/redirected' }, + }), + ), + http.post('https://metrics.eu.confidence.dev/v1/redirected', () => { + redirectedCalls++; + return HttpResponse.json({}); + }), + ); + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + method: 'POST', + body: { displayName: 'Example' }, + }); + + expect(result).toMatchObject({ ok: false, status: 0 }); + expect(redirectedCalls).toBe(0); + }); + + it('passes cancellation through to the HTTP client', async () => { + const sut = metricsRequest; + + const result = await sut(buildTestJwt(), { + path: '/v1/testResources', + signal: AbortSignal.abort(), + timeoutMs: 100, + }); + + expect(result).toMatchObject({ ok: false, status: 0 }); + }); + + it.each([ + 'https://other.example/v1/testResources', + '//other.example/v1/testResources', + '/v1/../admin', + '/v1/%2e%2e/admin', + '/v1/testResources?secret=value', + '/v1/testResources#fragment', + '/v1/\\other.example', + ])('rejects unsafe API path %s before sending', async (path) => { + const sut = metricsRequest; + await expect(sut(buildTestJwt(), { path })).rejects.toThrow('local /v1/ API path'); + }); +}); + +describe('resource paths', () => { + it('builds a path from a qualified resource with exactly one ID', () => { + const sut = resourcePath; + expect(sut('testResources/abc-123_A', 'testResources')).toBe('/v1/testResources/abc-123_A'); + }); + + it.each([ + '', + 'abc', + '/testResources/abc', + 'https://evil.example/testResources/abc', + '//evil.example/testResources/abc', + 'other/abc', + 'testResources/', + 'testResources/.', + 'testResources/..', + 'testResources/../other', + 'testResources/abc/child', + 'testResources/%2e%2e', + 'testResources/%252fadmin', + 'testResources/a%2fb', + 'testResources/a?query=x', + 'testResources/a#fragment', + 'testResources/a\\b', + 'testResources/a b', + 'testResources/abc\n', + ])('rejects invalid resource name %j', (name) => { + const sut = resourcePath; + expect(() => sut(name, 'testResources')).toThrow('single valid ID'); + }); + + it.each(['../testResources', '/testResources', 'testResources/child', 'testResources?x=y'])( + 'rejects invalid collection %s', + (collection) => { + const sut = resourcePath; + expect(() => sut('testResources/abc', collection)).toThrow('Invalid resource collection'); + }, + ); +}); + +describe('response guards', () => { + it.each([null, [], 'text', 42, undefined].map((value) => ({ value })))( + 'rejects non-record value $value', + ({ value }) => { + const sut = requireRecord; + expect(isRecord(value)).toBe(false); + expect(() => sut(value, 'Response')).toThrow('Response must be an object'); + }, + ); + + it('preserves resource fields rather than discarding unknown ones', () => { + const resource = { name: 'testResources/a', state: 'FAILED', extra: { reason: 'example' } }; + const sut = requireRecord; + expect(sut(resource)).toBe(resource); + }); + + it.each(['', ' ', null, [], 42].map((value) => ({ value })))( + 'rejects empty or non-string value $value', + ({ value }) => { + const sut = requireString; + expect(() => sut(value, 'Name')).toThrow('Name must be a non-empty string'); + }, + ); + + it('does not silently rewrite strings', () => { + const sut = requireString; + expect(sut(' original ')).toBe(' original '); + }); +}); diff --git a/packages/cli/bin/cli.ts b/packages/cli/bin/cli.ts index 1e9f9157..f52a9689 100644 --- a/packages/cli/bin/cli.ts +++ b/packages/cli/bin/cli.ts @@ -20,6 +20,7 @@ import { pluginCommand, migrateCommand, warehouseCommand, + dataCommand, } from '../src/commands/index.js'; const cli = yargs(hideBin(process.argv)) @@ -88,6 +89,7 @@ const cli = yargs(hideBin(process.argv)) .command(pluginCommand) .command(migrateCommand) .command(warehouseCommand) + .command(dataCommand) .example('$0 login', 'Sign in to Confidence') .example('$0 flags setup', 'Set up feature flags in your project') .example('$0 docs search "feature flags"', 'Search the documentation') diff --git a/packages/cli/src/commands/data.ts b/packages/cli/src/commands/data.ts new file mode 100644 index 00000000..ec20479f --- /dev/null +++ b/packages/cli/src/commands/data.ts @@ -0,0 +1,16 @@ +import type { Argv } from 'yargs'; +import { noop } from '@spotify-confidence/shared-kernel'; + +/** Feature PRs register their subcommands in this group. */ +export const dataCommand = { + command: 'data', + describe: 'Work with Confidence data resources', + builder(yargs: Argv) { + return yargs + .usage('$0 data [options]') + .demandCommand(1, 'Run "confidence data --help" to see available commands.') + .strict() + .help(); + }, + handler: noop, +}; diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 84dd9b05..fbbee90a 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -13,4 +13,5 @@ export { pluginCommand } from './plugin.js'; export { updateCommand } from './update.js'; export { migrateCommand } from './migrate.js'; export { warehouseCommand } from './warehouse.js'; +export { dataCommand } from './data.js'; export type { GlobalFlags } from './types.js'; diff --git a/packages/cli/src/features/data/index.ts b/packages/cli/src/features/data/index.ts new file mode 100644 index 00000000..a4c9385e --- /dev/null +++ b/packages/cli/src/features/data/index.ts @@ -0,0 +1,2 @@ +export { printData } from './output.js'; +export { executeDataRequest } from './request.js'; diff --git a/packages/cli/src/features/data/output.ts b/packages/cli/src/features/data/output.ts new file mode 100644 index 00000000..06ba6ba0 --- /dev/null +++ b/packages/cli/src/features/data/output.ts @@ -0,0 +1,26 @@ +import { inspect } from 'node:util'; +import { isRecord } from '@network/metrics-types.js'; +import { extractFlags, formatJson, message, print, resolveFormat } from '@output/index.js'; + +/** Preserve the full response in JSON; render generic resources as key/value rows otherwise. */ +export function printData(data: unknown, argv: Record): void { + const flags = extractFlags(argv); + if (resolveFormat(flags) === 'json') { + message(formatJson(data)); + return; + } + + const fields = isRecord(data) ? data : { value: data }; + const rows = Object.entries(fields).map(([field, value]) => ({ + field, + value: typeof value === 'string' ? value : inspect(value, { depth: null, colors: false }), + })); + print({ + data: rows, + columns: [ + { key: 'field', header: 'Field' }, + { key: 'value', header: 'Value' }, + ], + flags, + }); +} diff --git a/packages/cli/src/features/data/request.ts b/packages/cli/src/features/data/request.ts new file mode 100644 index 00000000..24132bbd --- /dev/null +++ b/packages/cli/src/features/data/request.ts @@ -0,0 +1,34 @@ +import type { ApiResponse } from '@spotify-confidence/core'; +import { printApiError } from '@utils/api-error.js'; +import { requireAuth } from '@utils/auth.js'; +import { printData } from './output.js'; + +/** Callers must validate arguments and input before invoking this helper, including dry-runs. */ +export async function executeDataRequest( + argv: Record, + request: (token: string) => Promise>, + preview?: unknown, +): Promise { + if (argv['dry-run'] === true) { + printData(preview ?? { dryRun: true }, argv); + return; + } + + const token = requireAuth(argv.profile as string | undefined); + if (!token) return; + + try { + const result = await request(token); + if (!result.ok) { + printApiError(result); + return; + } + printData(result.data, argv); + } catch (err) { + printApiError({ + ok: false, + status: 0, + error: { code: 0, message: err instanceof Error ? err.message : 'Data request failed.' }, + }); + } +} diff --git a/packages/cli/src/network/index.ts b/packages/cli/src/network/index.ts index 609492b2..a63a9054 100644 --- a/packages/cli/src/network/index.ts +++ b/packages/cli/src/network/index.ts @@ -1,4 +1,6 @@ export { fetchLatestVersion } from './registry.js'; +export { metricsRequest } from './metrics.js'; +export { isRecord, requireRecord, requireString, resourcePath } from './metrics-types.js'; export { searchDocumentation, grepDocumentation, getFullSource } from './docs.js'; diff --git a/packages/cli/src/network/metrics-types.ts b/packages/cli/src/network/metrics-types.ts new file mode 100644 index 00000000..ac6f61dd --- /dev/null +++ b/packages/cli/src/network/metrics-types.ts @@ -0,0 +1,28 @@ +/** Small response/input guards; resource-specific validation belongs to each feature. */ +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +export function requireRecord(value: unknown, label = 'Value'): Record { + if (!isRecord(value)) throw new Error(`${label} must be an object.`); + return value; +} + +export function requireString(value: unknown, label = 'Value'): string { + if (typeof value !== 'string' || value.trim().length === 0) { + throw new Error(`${label} must be a non-empty string.`); + } + return value; +} + +/** A qualified resource name must contain exactly one unescaped resource ID. */ +export function resourcePath(name: string, collection: string): string { + if (!/^[A-Za-z][A-Za-z0-9]*$/u.test(collection)) { + throw new Error('Invalid resource collection.'); + } + const prefix = `${collection}/`; + if (!name.startsWith(prefix) || !/^[A-Za-z0-9][A-Za-z0-9_-]*$/u.test(name.slice(prefix.length))) { + throw new Error(`Resource name must be ${collection}/ with a single valid ID.`); + } + return `/v1/${name}`; +} diff --git a/packages/cli/src/network/metrics.ts b/packages/cli/src/network/metrics.ts new file mode 100644 index 00000000..30b32781 --- /dev/null +++ b/packages/cli/src/network/metrics.ts @@ -0,0 +1,30 @@ +import { + apiRequest, + extractRegion, + type ApiRequestOptions, + type ApiResponse, +} from '@spotify-confidence/core'; + +/** Send one authenticated request to the token's regional Metrics API. */ +export async function metricsRequest( + token: string, + options: Omit, +): Promise> { + // Accept API paths, never absolute URLs or response-supplied pagination URLs. + if ( + !options.path.startsWith('/v1/') || + /[\\%?#\s]/u.test(options.path) || + options.path.split('/').some((part) => part === '.' || part === '..') + ) { + throw new Error('Metrics requests require a local /v1/ API path.'); + } + + return apiRequest({ + ...options, + token, + region: extractRegion(token), + service: 'metrics', + // Redirects could replay writes or send credentials to an API-supplied URL. + redirect: 'error', + }); +} diff --git a/packages/cli/src/utils/api-error.ts b/packages/cli/src/utils/api-error.ts new file mode 100644 index 00000000..4022ce40 --- /dev/null +++ b/packages/cli/src/utils/api-error.ts @@ -0,0 +1,11 @@ +import type { ApiFailure } from '@spotify-confidence/core'; +import { fail } from '@output/index.js'; + +/** Render only the API message, not arbitrary response bodies or error details. */ +export function printApiError(failure: ApiFailure): void { + const message = failure.error.message + .replace(/Bearer\s+\S+/giu, 'Bearer [redacted]') + .replace(/[\p{Cc}\p{Cf}]/gu, ' ') + .trim(); + fail(message || `API request failed (HTTP ${failure.status}).`); +} diff --git a/packages/cli/src/utils/index.ts b/packages/cli/src/utils/index.ts index 248d517a..90f45a7e 100644 --- a/packages/cli/src/utils/index.ts +++ b/packages/cli/src/utils/index.ts @@ -1,4 +1,5 @@ export * from './auth.js'; +export * from './api-error.js'; export * from './mcp-error.js'; export * from './safely.js'; export * from './telemetry.js'; diff --git a/packages/cli/src/utils/telemetry.ts b/packages/cli/src/utils/telemetry.ts index 6733d36d..b0381671 100644 --- a/packages/cli/src/utils/telemetry.ts +++ b/packages/cli/src/utils/telemetry.ts @@ -2,7 +2,18 @@ import { initTelemetry, isTelemetryEnabled, getTelemetry, flush } from '@spotify export { flush as flushTelemetry }; -export function telemetryMiddleware(argv: { telemetry: boolean; _: (string | number)[] }) { +export function telemetryMiddleware(argv: { + telemetry: boolean; + _: (string | number)[]; + 'dry-run'?: boolean; +}) { + const [group, action] = argv._; + if ( + argv['dry-run'] === true && + (group === 'data' || (group === 'warehouse' && (action === 'list' || action === 'get'))) + ) { + return; + } if (!isTelemetryEnabled() || argv.telemetry === false) return; initTelemetry({ source: 'cli' }); diff --git a/packages/core/__tests__/api/client.test.ts b/packages/core/__tests__/api/client.test.ts index 981da382..77849826 100644 --- a/packages/core/__tests__/api/client.test.ts +++ b/packages/core/__tests__/api/client.test.ts @@ -1,7 +1,14 @@ import { http, HttpResponse, server } from '@spotify-confidence/testing'; import { resolveBaseUrl, apiRequest } from '@api/client.js'; +import type { ApiRequestOptions } from '@api/types.js'; const TEST_BASE = 'https://test-svc.eu.confidence.dev'; +const requestOptions: ApiRequestOptions = { + token: 'test-token', + region: 'EU', + service: 'test-svc', + path: '/v1/items', +}; beforeEach(() => { server.use( @@ -54,6 +61,175 @@ describe('resolveBaseUrl', () => { }); }); +describe('apiRequest response safety', () => { + it.each(['not JSON', '', 'private upstream error'])( + 'returns a controlled failure for malformed success body %j', + async (body) => { + server.use(http.get(`${TEST_BASE}/v1/items`, () => new HttpResponse(body))); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 200, + error: { code: 200, message: 'Invalid JSON response from API.' }, + }); + }, + ); + + it.each( + [null, [], 'error', { message: {} }, { code: 'bad', message: 'invalid' }].map((body) => ({ + body, + })), + )('normalizes malformed error response $body', async ({ body }) => { + server.use( + http.get(`${TEST_BASE}/v1/items`, () => + HttpResponse.json(body, { status: 503, statusText: 'Service Unavailable' }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 503, + error: { code: 503, message: 'Service Unavailable' }, + }); + }); + + it('does not expose non-JSON error bodies', async () => { + server.use( + http.get( + `${TEST_BASE}/v1/items`, + () => new HttpResponse('upstream private details', { status: 502 }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 502, + error: { code: 502, message: 'Bad Gateway' }, + }); + }); + + it('preserves structured API errors and details', async () => { + const error = { code: 3, message: 'Invalid input', details: [{ field: 'name' }] }; + server.use(http.get(`${TEST_BASE}/v1/items`, () => HttpResponse.json(error, { status: 400 }))); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ ok: false, status: 400, error }); + }); + + it('discards malformed error details', async () => { + server.use( + http.get(`${TEST_BASE}/v1/items`, () => + HttpResponse.json({ code: 3, message: 'Invalid input', details: 'bad' }, { status: 400 }), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toEqual({ + ok: false, + status: 400, + error: { code: 3, message: 'Invalid input' }, + }); + }); + + it('returns a controlled failure if reading the body fails', async () => { + server.use( + http.get( + `${TEST_BASE}/v1/items`, + () => + new HttpResponse( + new ReadableStream({ + start(controller) { + controller.error(new Error('Connection closed')); + }, + }), + ), + ), + ); + const sut = apiRequest; + + const result = await sut(requestOptions); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + if (!result.ok) expect(result.error.message).toContain('Network error'); + }); +}); + +describe('apiRequest cancellation', () => { + it('honors an already cancelled caller signal', async () => { + const sut = apiRequest; + + const result = await sut({ ...requestOptions, signal: AbortSignal.abort() }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); + + it.each(['timeout', 'caller'] as const)('cancels before headers via %s', async (mode) => { + const controller = new AbortController(); + server.use( + http.get(`${TEST_BASE}/v1/items`, async ({ request }) => { + if (mode === 'caller') controller.abort(); + await new Promise((resolve) => { + if (request.signal.aborted) resolve(); + else request.signal.addEventListener('abort', () => resolve(), { once: true }); + }); + return HttpResponse.json({}); + }), + ); + const sut = apiRequest; + + const result = await sut({ + ...requestOptions, + signal: controller.signal, + timeoutMs: mode === 'timeout' ? 50 : 1000, + }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); + + it.each(['timeout', 'caller'] as const)('cancels body consumption via %s', async (mode) => { + const controller = new AbortController(); + server.use( + http.get( + `${TEST_BASE}/v1/items`, + ({ request }) => + new HttpResponse( + new ReadableStream({ + start(body) { + body.enqueue(new TextEncoder().encode('{')); + request.signal.addEventListener('abort', () => body.error(request.signal.reason), { + once: true, + }); + if (mode === 'caller') queueMicrotask(() => controller.abort()); + }, + }), + ), + ), + ); + const sut = apiRequest; + + const result = await sut({ + ...requestOptions, + signal: controller.signal, + timeoutMs: mode === 'timeout' ? 50 : 1000, + }); + + expect(result).toMatchObject({ ok: false, status: 0, error: { code: 0 } }); + }); +}); + describe('apiRequest', () => { it('sends an authorized GET request', async () => { const result = await apiRequest<{ auth: string }>({ diff --git a/packages/core/src/api/client.ts b/packages/core/src/api/client.ts index a4501eee..71aecdbf 100644 --- a/packages/core/src/api/client.ts +++ b/packages/core/src/api/client.ts @@ -25,32 +25,68 @@ export async function apiRequest(opts: ApiRequestOptions): Promise; + /** Cancels both the request and response body consumption. */ + signal?: AbortSignal; + /** Redirect policy. Defaults to fetch's existing follow behavior. */ + redirect?: 'follow' | 'error'; + /** Overall deadline in milliseconds, including response body consumption. Defaults to 30 seconds. */ + timeoutMs?: number; }; From 4c3720d1e433453ee4d02c2bcbe48fd59f85d3e2 Mon Sep 17 00:00:00 2001 From: Robin Timan Date: Mon, 19 Oct 2026 15:41:19 +0200 Subject: [PATCH 2/2] feat(cli): add entity list and get commands Co-Authored-By: Xirp Agent (gpt-6.1-sol) --- packages/cli/README.md | 21 + packages/cli/__tests__/commands/data.test.ts | 11 + .../cli/__tests__/commands/entities.test.ts | 511 ++++++++++++++++++ .../network/metrics-entities.test.ts | 106 ++++ packages/cli/src/commands/data.ts | 4 +- packages/cli/src/commands/data/entities.ts | 46 ++ packages/cli/src/features/data/entities.ts | 43 ++ packages/cli/src/features/data/index.ts | 1 + packages/cli/src/network/metrics-entities.ts | 84 +++ 9 files changed, 825 insertions(+), 2 deletions(-) create mode 100644 packages/cli/__tests__/commands/entities.test.ts create mode 100644 packages/cli/__tests__/network/metrics-entities.test.ts create mode 100644 packages/cli/src/commands/data/entities.ts create mode 100644 packages/cli/src/features/data/entities.ts create mode 100644 packages/cli/src/network/metrics-entities.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index fcfcb34b..fa81f653 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -91,6 +91,27 @@ confidence quickstart # Launch the interactive setup wizard | `confidence warehouse assignment-table create` | Create an assignment table for experiment analysis | | `confidence warehouse crypto-key create` | Create a crypto key (Snowflake) | +### Data Entities + +Read entities directly from the regional Metrics API using your current credentials or `--profile`. +These commands do not install MCP servers or launch onboarding. + +```bash +confidence data entities list --page-size 100 --json +confidence data entities list --page-token '' --json +confidence data entities list --all --json +confidence data entities get --name entities/user_id --json +confidence data entities get --name entities/user_id --dry-run --json +``` + +- `list` fetches one page by default and returns `{ entities, nextPageToken, ... }` inside the JSON `data` envelope. Entity resources and unknown list-response fields retain all API fields. An empty or omitted server token becomes `""`. +- `--page-size` must be a positive integer no greater than 1000. `--page-token` is opaque and resumes a previous list response. +- `--all` collects all remaining pages, starting at `--page-token` if supplied. It returns an empty `nextPageToken` only when complete. Repeated cursors or more than 1000 pages fail without printing partial results. Unknown list-response fields are combined across pages; the latest value wins for repeated fields. +- `get` requires a qualified `--name entities/` (not a bare ID or URL) and rejects responses naming a different entity. +- `--dry-run` validates inputs and previews the GET request without authentication or network requests. For `--all`, the preview describes the initial request; later cursors are only known during execution. + +Only entity list/get are supported; these commands do not create, update, or delete entities. + ### SDK | Command | Description | diff --git a/packages/cli/__tests__/commands/data.test.ts b/packages/cli/__tests__/commands/data.test.ts index 1bf9cd8e..a6be19ea 100644 --- a/packages/cli/__tests__/commands/data.test.ts +++ b/packages/cli/__tests__/commands/data.test.ts @@ -39,6 +39,17 @@ describe('data command group', () => { expect(output).not.toMatch(/onboard|discover|proposal|workflow/iu); }); + it('advertises only list and get under entities without authentication', async () => { + using _auth = prepareAuthTokens('none'); + const sut = run; + + const output = await sut(['data', 'entities', '--help']); + + expect(output).toContain('entities list'); + expect(output).toContain('entities get'); + expect(output).not.toMatch(/create|update|delete|onboard|workflow/iu); + }); + it('shows guidance when invoked without a subcommand', async () => { const sut = run; expect(await sut(['data'])).toContain('confidence data --help'); diff --git a/packages/cli/__tests__/commands/entities.test.ts b/packages/cli/__tests__/commands/entities.test.ts new file mode 100644 index 00000000..67fbc764 --- /dev/null +++ b/packages/cli/__tests__/commands/entities.test.ts @@ -0,0 +1,511 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { server, http, HttpResponse } from '@spotify-confidence/testing'; +import { buildTestJwt, prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { dataCommand } from '@commands/data.js'; +import { captureOutput } from '../helpers/capture.js'; +import { createRunner } from '../helpers/run-command.js'; + +const EU_BASE = 'https://metrics.eu.confidence.dev'; +const US_BASE = 'https://metrics.us.confidence.dev'; +const sut = createRunner(dataCommand); +const entity = { + name: 'entities/user_id', + displayName: 'User', + primaryKeyType: 'COLUMN_TYPE_STRING', + createTime: '2026-01-01T00:00:00Z', + labels: { team: 'growth' }, + owner: 'identities/owner', + futureField: { retained: true }, +}; + +function writeTestCredentials(accessToken: string, profile?: string): void { + const base = process.env.CONFIDENCE_CONFIG_DIR!; + const dir = profile ? join(base, 'profiles', profile) : base; + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'credentials.json'), JSON.stringify({ accessToken }), { mode: 0o600 }); +} + +function recordRequests(): Request[] { + const requests: Request[] = []; + server.use( + http.all('*', ({ request }) => { + requests.push(request); + return HttpResponse.json({ message: 'Unexpected request' }, { status: 500 }); + }), + ); + return requests; +} + +describe('data entities list', () => { + it('returns one page with the cursor and complete entity resources by default', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const requests: URL[] = []; + server.use( + http.get(`${EU_BASE}/v1/entities`, ({ request }) => { + requests.push(new URL(request.url)); + return HttpResponse.json({ entities: [entity], nextPageToken: 'next-page' }); + }), + ); + + await sut(['data', 'entities', 'list', '--json']); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual({ + entities: [entity], + nextPageToken: 'next-page', + }); + expect(requests).toHaveLength(1); + expect(requests[0]!.search).toBe(''); + }); + + it.each([ + ['EU', EU_BASE, undefined], + ['US', US_BASE, undefined], + ['US', US_BASE, 'analytics'], + ])('uses %s regional credentials at %s with profile %s', async (region, base, profile) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const token = buildTestJwt({ 'https://confidence.dev/region': region }); + writeTestCredentials(token, profile); + server.use( + http.get(`${base}/v1/entities`, ({ request }) => { + if (request.headers.get('Authorization') !== `Bearer ${token}`) { + return HttpResponse.json({ message: 'Wrong credentials' }, { status: 401 }); + } + return HttpResponse.json({ entities: [entity] }); + }), + ); + + await sut(['data', 'entities', 'list', '--json', ...(profile ? ['--profile', profile] : [])]); + + expect(JSON.parse(output.stdout).data.entities).toEqual([entity]); + expect(output.stderr).toBe(''); + }); + + it('preserves unknown list response fields alongside complete entity resources', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const response = { entities: [entity], nextPageToken: 'later', futurePageField: { count: 4 } }; + server.use(http.get(`${EU_BASE}/v1/entities`, () => HttpResponse.json(response))); + + await sut(['data', 'entities', 'list', '--json']); + + expect(JSON.parse(output.stdout).data).toEqual(response); + }); + + it('passes page size and opaque page token to the actual GET request', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const cursors: string[] = []; + server.use( + http.get(`${EU_BASE}/v1/entities`, ({ request }) => { + const url = new URL(request.url); + cursors.push(`${url.searchParams.get('pageSize')}:${url.searchParams.get('pageToken')}`); + return HttpResponse.json({ entities: [entity], nextPageToken: 'later' }); + }), + ); + + await sut([ + 'data', + 'entities', + 'list', + '--page-size', + '1000', + '--page-token', + 'a+/=?& b', + '--json', + ]); + + expect(cursors).toEqual(['1000:a+/=?& b']); + expect(JSON.parse(output.stdout).data.nextPageToken).toBe('later'); + }); + + it.each([{ entities: [] }, {}])('normalizes empty proto lists: %j', async (response) => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(`${EU_BASE}/v1/entities`, () => HttpResponse.json(response))); + + await sut(['data', 'entities', 'list', '--json']); + + expect(JSON.parse(output.stdout).data).toEqual({ entities: [], nextPageToken: '' }); + }); + + it('renders entity resources and the next-page cursor in table output', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${EU_BASE}/v1/entities`, () => + HttpResponse.json({ entities: [entity], nextPageToken: 'next-page' }), + ), + ); + + await sut(['data', 'entities', 'list', '--output', 'table']); + + expect(output.stdout).toContain('entities/user_id'); + expect(output.stdout).toContain('next-page'); + }); + + it('collects all remaining pages while preserving page size and the starting token', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const requests: string[] = []; + const second = { ...entity, name: 'entities/account_id' }; + server.use( + http.get(`${EU_BASE}/v1/entities`, ({ request }) => { + const url = new URL(request.url); + const cursor = url.searchParams.get('pageToken'); + requests.push(`${url.searchParams.get('pageSize')}:${cursor}`); + if (cursor === 'start') { + return HttpResponse.json({ + entities: [entity], + nextPageToken: 'middle', + firstPageField: { retained: true }, + futurePageField: 'first', + }); + } + if (cursor === 'middle') { + return HttpResponse.json({ nextPageToken: 'end' }); + } + return HttpResponse.json({ + entities: [second], + finalPageField: { retained: true }, + futurePageField: 'last', + }); + }), + ); + + await sut([ + 'data', + 'entities', + 'list', + '--all', + '--page-size', + '2', + '--page-token', + 'start', + '--json', + ]); + + expect(requests).toEqual(['2:start', '2:middle', '2:end']); + expect(JSON.parse(output.stdout).data).toEqual({ + entities: [entity, second], + nextPageToken: '', + firstPageField: { retained: true }, + finalPageField: { retained: true }, + futurePageField: 'last', + }); + }); + + it.each([undefined, 'start'])( + 'stops repeated cursors from start %s without partial output', + async (start) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(`${EU_BASE}/v1/entities`, () => { + requests++; + return HttpResponse.json({ entities: [entity], nextPageToken: start ?? 'repeat' }); + }), + ); + + await sut([ + 'data', + 'entities', + 'list', + '--all', + '--json', + ...(start ? ['--page-token', start] : []), + ]); + + // Assert + expect(requests).toBe(start ? 1 : 2); + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('repeated page token'); + expect(process.exitCode).toBe(1); + }, + ); + + it('detects longer cursor cycles', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(`${EU_BASE}/v1/entities`, () => { + requests++; + return HttpResponse.json({ nextPageToken: requests === 2 ? 'second' : 'first' }); + }), + ); + + await sut(['data', 'entities', 'list', '--all', '--json']); + + expect(requests).toBe(3); + expect(output.stderr).toContain('repeated page token'); + expect(output.stdout).toBe(''); + }); + + it('reports errors on later pages without printing partial results', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${EU_BASE}/v1/entities`, ({ request }) => + new URL(request.url).searchParams.has('pageToken') + ? HttpResponse.json({ code: 7, message: 'Access denied' }, { status: 403 }) + : HttpResponse.json({ entities: [entity], nextPageToken: 'next' }), + ), + ); + + await sut(['data', 'entities', 'list', '--all', '--json']); + + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('Access denied'); + expect(process.exitCode).toBe(1); + }); + + it.each(['0', '-1', '1.5', '1001', 'NaN', 'Infinity'])( + 'rejects page size %s before authentication or requests', + async (size) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = recordRequests(); + + await sut(['data', 'entities', 'list', '--page-size', size, '--dry-run', '--json']); + + // Assert + expect(output.stderr).toContain('--page-size must be a positive integer'); + expect(output.stderr).not.toContain('Not logged in'); + expect(output.stdout).toBe(''); + expect(requests).toHaveLength(0); + expect(process.exitCode).toBe(1); + }, + ); + + it('rejects repeated page-token options instead of coercing them', async () => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = recordRequests(); + + await sut(['data', 'entities', 'list', '--page-token', 'a', '--page-token', 'b', '--dry-run']); + + expect(output.stderr).toContain('--page-token must be a string'); + expect(output.stdout).toBe(''); + expect(requests).toHaveLength(0); + }); + + it('previews list pagination without authentication or any requests', async () => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = recordRequests(); + + await sut([ + 'data', + 'entities', + 'list', + '--all', + '--page-size', + '1', + '--page-token', + 'opaque', + '--dry-run', + '--json', + ]); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual({ + dryRun: true, + method: 'GET', + path: '/v1/entities', + params: { pageSize: '1', pageToken: 'opaque' }, + all: true, + }); + expect(output.stderr).toBe(''); + expect(requests).toHaveLength(0); + }); + + it.each( + [ + null, + [], + { entities: null }, + { entities: {} }, + { entities: [null] }, + { entities: [{}] }, + { entities: [{ name: 42 }] }, + { entities: [{ name: 'flags/wrong' }] }, + { nextPageToken: 42 }, + { nextPageToken: null }, + ].map((response) => [response]), + )('reports malformed list response %j instead of success', async (response) => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(`${EU_BASE}/v1/entities`, () => HttpResponse.json(response))); + + await sut(['data', 'entities', 'list', '--json']); + + expect(output.stdout).toBe(''); + expect(output.stderr).not.toBe(''); + expect(process.exitCode).toBe(1); + }); +}); + +describe('data entities get', () => { + it('prints the complete resource returned by the Metrics API', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(`${EU_BASE}/v1/entities/user_id`, () => HttpResponse.json(entity))); + + await sut(['data', 'entities', 'get', '--name', 'entities/user_id', '--json']); + + expect(JSON.parse(output.stdout).data).toEqual(entity); + }); + + it('uses the selected US profile rather than the default EU credentials', async () => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + const token = buildTestJwt({ 'https://confidence.dev/region': 'US', email: 'us@example.com' }); + writeTestCredentials(token, 'us'); + server.use( + http.get(`${US_BASE}/v1/entities/user_id`, ({ request }) => + request.headers.get('Authorization') === `Bearer ${token}` + ? HttpResponse.json(entity) + : HttpResponse.json({ message: 'Wrong credentials' }, { status: 401 }), + ), + ); + + await sut([ + 'data', + 'entities', + 'get', + '--name', + 'entities/user_id', + '--profile', + 'us', + '--json', + ]); + + expect(JSON.parse(output.stdout).data).toEqual(entity); + expect(output.stderr).toBe(''); + }); + + it('rejects a different entity instead of reporting a successful get', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${EU_BASE}/v1/entities/user_id`, () => + HttpResponse.json({ ...entity, name: 'entities/account_id' }), + ), + ); + + await sut(['data', 'entities', 'get', '--name', 'entities/user_id', '--json']); + + // Assert + expect(output.stdout).toBe(''); + expect(output.stderr).toContain( + 'Entity response name does not match the requested resource name', + ); + expect(process.exitCode).toBe(1); + }); + + it('reports entity not found', async () => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use( + http.get(`${EU_BASE}/v1/entities/missing`, () => + HttpResponse.json({ code: 5, message: 'Entity not found' }, { status: 404 }), + ), + ); + + await sut(['data', 'entities', 'get', '--name', 'entities/missing', '--json']); + + expect(output.stderr).toContain('Entity not found'); + expect(output.stdout).toBe(''); + expect(process.exitCode).toBe(1); + }); + + it.each([ + 'user_id', + 'entities/', + 'flags/user_id', + 'entities/a/b', + 'entities/../user_id', + 'entities/a%2Fb', + 'entities/a?query', + 'entities/a#fragment', + 'entities/a\\b', + 'entities/a b', + 'https://metrics.eu.confidence.dev/v1/entities/user_id', + ])('rejects invalid qualified name %s even in dry-run', async (name) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = recordRequests(); + + await sut(['data', 'entities', 'get', '--name', name, '--dry-run', '--json']); + + // Assert + expect(output.stderr).toContain('Resource name must be entities/'); + expect(output.stderr).not.toContain('Not logged in'); + expect(output.stdout).toBe(''); + expect(requests).toHaveLength(0); + expect(process.exitCode).toBe(1); + }); + + it('previews the qualified GET path without authentication or requests', async () => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = recordRequests(); + + await sut(['data', 'entities', 'get', '--name', 'entities/user_id', '--dry-run', '--json']); + + // Assert + expect(JSON.parse(output.stdout).data).toEqual({ + dryRun: true, + method: 'GET', + path: '/v1/entities/user_id', + }); + expect(output.stderr).toBe(''); + expect(requests).toHaveLength(0); + }); + + it.each([null, [], {}, { name: 42 }, { name: 'entities/a/b' }].map((response) => [response]))( + 'rejects malformed entity response %j', + async (response) => { + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + server.use(http.get(`${EU_BASE}/v1/entities/user_id`, () => HttpResponse.json(response))); + + await sut(['data', 'entities', 'get', '--name', 'entities/user_id', '--json']); + + expect(output.stdout).toBe(''); + expect(output.stderr).not.toBe(''); + expect(process.exitCode).toBe(1); + }, + ); +}); + +describe('data entities authentication', () => { + it.each([['list'], ['get', '--name', 'entities/user_id']])( + 'requires authentication for %s without making requests', + async (...args) => { + using _auth = prepareAuthTokens('none'); + using output = captureOutput(); + const requests = recordRequests(); + + await sut(['data', 'entities', ...args, '--json']); + + expect(output.stderr).toContain('Not logged in'); + expect(output.stdout).toBe(''); + expect(requests).toHaveLength(0); + expect(process.exitCode).toBe(1); + }, + ); +}); diff --git a/packages/cli/__tests__/network/metrics-entities.test.ts b/packages/cli/__tests__/network/metrics-entities.test.ts new file mode 100644 index 00000000..a0f295a3 --- /dev/null +++ b/packages/cli/__tests__/network/metrics-entities.test.ts @@ -0,0 +1,106 @@ +import { http, HttpResponse, server } from '@spotify-confidence/testing'; +import { buildTestJwt, prepareAuthTokens } from '@spotify-confidence/testing/auth'; +import { executeDataRequest } from '@features/data/index.js'; +import { listEntities } from '@network/metrics-entities.js'; +import { captureOutput } from '../helpers/capture.js'; + +const ENTITIES_URL = 'https://metrics.eu.confidence.dev/v1/entities'; + +describe('entity pagination safety limit', () => { + it.each([{ entities: [] }, { entities: [{ name: 'entities/user_id' }] }])( + 'fails at the page limit for unique cursors without printing partial results: %j', + async (page) => { + // Arrange + using _auth = prepareAuthTokens('valid'); + using output = captureOutput(); + let requests = 0; + server.use( + http.get(ENTITIES_URL, () => { + requests++; + return HttpResponse.json({ + ...page, + nextPageToken: `cursor-${requests}`, + }); + }), + ); + const sut = () => + executeDataRequest({ json: true }, (token) => + listEntities(token, { all: true, maxPages: 3 }), + ); + + await sut(); + + // Assert + expect(requests).toBe(3); + expect(output.stdout).toBe(''); + expect(output.stderr).toContain('Entities pagination exceeded the 3-page limit.'); + expect(process.exitCode).toBe(1); + }, + ); + + it('accepts a complete traversal that finishes exactly at the page limit', async () => { + // Arrange + let requests = 0; + server.use( + http.get(ENTITIES_URL, () => { + requests++; + return HttpResponse.json({ + entities: [{ name: `entities/page_${requests}` }], + ...(requests < 3 ? { nextPageToken: `cursor-${requests}` } : {}), + }); + }), + ); + const sut = listEntities; + + const result = await sut(buildTestJwt(), { all: true, maxPages: 3 }); + + // Assert + expect(requests).toBe(3); + expect(result).toEqual({ + ok: true, + data: { + entities: [ + { name: 'entities/page_1' }, + { name: 'entities/page_2' }, + { name: 'entities/page_3' }, + ], + nextPageToken: '', + }, + }); + }); + + it('returns a single-page cursor without treating the page limit as completion', async () => { + server.use( + http.get(ENTITIES_URL, () => HttpResponse.json({ nextPageToken: 'remaining-pages' })), + ); + const sut = listEntities; + + const result = await sut(buildTestJwt(), { maxPages: 1 }); + + expect(result).toEqual({ + ok: true, + data: { entities: [], nextPageToken: 'remaining-pages' }, + }); + }); + + it.each([0, -1, 1.5, 1001, NaN, Infinity])( + 'rejects invalid or raised safety limit %s before making any request', + async (maxPages) => { + // Arrange + let requests = 0; + server.use( + http.get(ENTITIES_URL, () => { + requests++; + return HttpResponse.json({}); + }), + ); + const sut = listEntities; + + await expect(sut(buildTestJwt(), { all: true, maxPages })).rejects.toThrow( + 'Entities page limit must be an integer between 1 and 1000.', + ); + + expect(requests).toBe(0); + }, + ); +}); diff --git a/packages/cli/src/commands/data.ts b/packages/cli/src/commands/data.ts index ec20479f..c81dbe61 100644 --- a/packages/cli/src/commands/data.ts +++ b/packages/cli/src/commands/data.ts @@ -1,13 +1,13 @@ import type { Argv } from 'yargs'; import { noop } from '@spotify-confidence/shared-kernel'; - -/** Feature PRs register their subcommands in this group. */ +import { entitiesCommand } from './data/entities.js'; export const dataCommand = { command: 'data', describe: 'Work with Confidence data resources', builder(yargs: Argv) { return yargs .usage('$0 data [options]') + .command(entitiesCommand) .demandCommand(1, 'Run "confidence data --help" to see available commands.') .strict() .help(); diff --git a/packages/cli/src/commands/data/entities.ts b/packages/cli/src/commands/data/entities.ts new file mode 100644 index 00000000..fcb60a3e --- /dev/null +++ b/packages/cli/src/commands/data/entities.ts @@ -0,0 +1,46 @@ +import type { Argv } from 'yargs'; +import { noop } from '@spotify-confidence/shared-kernel'; +import { getEntityCmd, listEntitiesCmd } from '@features/data/index.js'; +import { safely } from '@utils/safely.js'; + +export const entitiesCommand = { + command: 'entities', + describe: 'Read Confidence entities', + builder(yargs: Argv) { + return yargs + .command( + 'list', + 'List entities', + (y: Argv) => + y + .option('page-size', { + type: 'number', + describe: 'Maximum entities per page (1–1000)', + }) + .option('page-token', { + type: 'string', + describe: 'Opaque token from a previous list response', + }) + .option('all', { + type: 'boolean', + default: false, + describe: 'Fetch all remaining pages', + }), + safely(listEntitiesCmd), + ) + .command( + 'get', + 'Get an entity by qualified resource name', + (y: Argv) => + y.option('name', { + type: 'string', + demandOption: true, + describe: 'Qualified entity name (entities/)', + }), + safely(getEntityCmd), + ) + .demandCommand(1, 'Available commands: list, get') + .strict(); + }, + handler: noop, +}; diff --git a/packages/cli/src/features/data/entities.ts b/packages/cli/src/features/data/entities.ts new file mode 100644 index 00000000..61c3a748 --- /dev/null +++ b/packages/cli/src/features/data/entities.ts @@ -0,0 +1,43 @@ +import { getEntity, listEntities } from '@network/metrics-entities.js'; +import { requireString, resourcePath } from '@network/metrics-types.js'; +import { executeDataRequest } from './request.js'; + +export async function listEntitiesCmd(argv: Record): Promise { + const pageSize = argv['page-size']; + if ( + pageSize !== undefined && + (typeof pageSize !== 'number' || !Number.isInteger(pageSize) || pageSize < 1 || pageSize > 1000) + ) { + throw new Error('--page-size must be a positive integer no greater than 1000.'); + } + const pageToken = argv['page-token']; + if (pageToken !== undefined && typeof pageToken !== 'string') { + throw new Error('--page-token must be a string.'); + } + const all = argv.all; + if (all !== undefined && typeof all !== 'boolean') { + throw new Error('--all must be a boolean.'); + } + const params: Record = {}; + if (pageSize !== undefined) params.pageSize = String(pageSize); + if (pageToken !== undefined) params.pageToken = pageToken; + + await executeDataRequest(argv, (token) => listEntities(token, { pageSize, pageToken, all }), { + dryRun: true, + method: 'GET', + path: '/v1/entities', + params, + all: all ?? false, + }); +} + +export async function getEntityCmd(argv: Record): Promise { + const name = requireString(argv.name, '--name'); + const path = resourcePath(name, 'entities'); + + await executeDataRequest(argv, (token) => getEntity(token, name), { + dryRun: true, + method: 'GET', + path, + }); +} diff --git a/packages/cli/src/features/data/index.ts b/packages/cli/src/features/data/index.ts index a4c9385e..b2fc32df 100644 --- a/packages/cli/src/features/data/index.ts +++ b/packages/cli/src/features/data/index.ts @@ -1,2 +1,3 @@ +export { getEntityCmd, listEntitiesCmd } from './entities.js'; export { printData } from './output.js'; export { executeDataRequest } from './request.js'; diff --git a/packages/cli/src/network/metrics-entities.ts b/packages/cli/src/network/metrics-entities.ts new file mode 100644 index 00000000..3fef0558 --- /dev/null +++ b/packages/cli/src/network/metrics-entities.ts @@ -0,0 +1,84 @@ +import type { ApiResponse } from '@spotify-confidence/core'; +import { metricsRequest } from './metrics.js'; +import { requireRecord, requireString, resourcePath } from './metrics-types.js'; + +type Entity = Record & { name: string }; +type EntityList = Record & { entities: Entity[]; nextPageToken: string }; +type ListOptions = { + pageSize?: number; + pageToken?: string; + all?: boolean; + /** Optional lower safety cap for programmatic callers; cannot raise the hard limit. */ + maxPages?: number; +}; + +const MAX_ENTITY_LIST_PAGES = 1000; + +function readEntity(value: unknown): Entity { + const entity = requireRecord(value, 'Entity response'); + const name = requireString(entity.name, 'Entity response name'); + resourcePath(name, 'entities'); + return { ...entity, name }; +} + +function readEntityList(value: unknown): EntityList { + const response = requireRecord(value, 'Entities response'); + // Proto JSON may omit empty repeated fields and default-valued strings. + const entities = response.entities === undefined ? [] : response.entities; + if (!Array.isArray(entities)) throw new Error('Entities response entities must be an array.'); + const nextPageToken = response.nextPageToken === undefined ? '' : response.nextPageToken; + if (typeof nextPageToken !== 'string') { + throw new Error('Entities response nextPageToken must be a string.'); + } + return { ...response, entities: entities.map(readEntity), nextPageToken }; +} + +export async function listEntities( + token: string, + options: ListOptions, +): Promise> { + const maxPages = options.maxPages ?? MAX_ENTITY_LIST_PAGES; + if (!Number.isInteger(maxPages) || maxPages < 1 || maxPages > MAX_ENTITY_LIST_PAGES) { + throw new Error( + `Entities page limit must be an integer between 1 and ${MAX_ENTITY_LIST_PAGES}.`, + ); + } + const entities: Entity[] = []; + let data: EntityList = { entities, nextPageToken: '' }; + const seen = new Set(); + let pageToken = options.pageToken; + if (pageToken) seen.add(pageToken); + + for (let pageIndex = 0; pageIndex < maxPages; pageIndex++) { + const params: Record = {}; + if (options.pageSize !== undefined) params.pageSize = String(options.pageSize); + if (pageToken !== undefined) params.pageToken = pageToken; + const result = await metricsRequest(token, { path: '/v1/entities', params }); + if (!result.ok) return result; + + const page = readEntityList(result.data); + if (!options.all) return { ok: true, data: page }; + entities.push(...page.entities); + // Retain unknown page fields; the latest value wins when pages repeat a field. + data = { ...data, ...page, entities }; + if (!page.nextPageToken) { + return { ok: true, data }; + } + if (seen.has(page.nextPageToken)) { + throw new Error('Entities pagination returned a repeated page token.'); + } + seen.add(page.nextPageToken); + pageToken = page.nextPageToken; + } + throw new Error(`Entities pagination exceeded the ${maxPages}-page limit.`); +} + +export async function getEntity(token: string, name: string): Promise> { + const result = await metricsRequest(token, { path: resourcePath(name, 'entities') }); + if (!result.ok) return result; + const entity = readEntity(result.data); + if (entity.name !== name) { + throw new Error('Entity response name does not match the requested resource name.'); + } + return { ok: true, data: entity }; +}