diff --git a/README.md b/README.md index dbb11bd..9e4fbd9 100644 --- a/README.md +++ b/README.md @@ -1192,7 +1192,8 @@ abs events list --from 7d --to now abs events list --event-name my-experiment --event-type exposure --from 5h abs events list --app 1 --unit-type 2 --event-type exposure --items 50 abs events list --unit-uid user123 --env-type production -abs events list --effective-exposures --from 1d # only assignment-changing exposures +abs events list --valid-exposures --from 1d # only assignment-changing exposures +abs events list --invalid-exposures --from 1d # only ineffective exposures (filtered out) abs events list --event-name exp1 --event-name exp2 # multiple names (all filters repeatable) abs events history --from 7d --period 1d abs events history --event-name my-experiment --env-type production diff --git a/package.json b/package.json index 3ea1c3c..9e769b7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@absmartly/cli", - "version": "1.5.0", + "version": "1.6.0", "description": "ABSmartly CLI - A/B Testing and Feature Flags command-line tool for AI agents and humans", "type": "module", "main": "./dist/index.js", diff --git a/src/commands/events/events.test.ts b/src/commands/events/events.test.ts index da17fd1..f0d41c3 100644 --- a/src/commands/events/events.test.ts +++ b/src/commands/events/events.test.ts @@ -96,6 +96,30 @@ describe('events command', () => { expect(printFormatted).toHaveBeenCalled(); }); + it('should send effective_exposures=true with --valid-exposures', async () => { + await eventsCommand.parseAsync(['node', 'test', 'list', '--valid-exposures']); + expect(mockClient.listEvents).toHaveBeenCalledWith({ + filters: { effective_exposures: true }, + }); + }); + + it('should send effective_exposures=false with --invalid-exposures', async () => { + await eventsCommand.parseAsync(['node', 'test', 'list', '--invalid-exposures']); + expect(mockClient.listEvents).toHaveBeenCalledWith({ + filters: { effective_exposures: false }, + }); + }); + + it('should reject combining --valid-exposures and --invalid-exposures', async () => { + await expect( + eventsCommand.parseAsync(['node', 'test', 'list', '--valid-exposures', '--invalid-exposures']) + ).rejects.toThrow('process.exit'); + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.anything(), + expect.stringContaining('mutually exclusive') + ); + }); + it('should list events history with filters', async () => { await eventsCommand.parseAsync([ 'node', diff --git a/src/commands/events/index.ts b/src/commands/events/index.ts index 28bc25a..e939972 100644 --- a/src/commands/events/index.ts +++ b/src/commands/events/index.ts @@ -51,7 +51,8 @@ const listCommand = new Command('list') parseStringArray, [] ) - .option('--effective-exposures', 'only effective exposures') + .option('--valid-exposures', 'only valid (effective) exposures') + .option('--invalid-exposures', 'only invalid (ineffective) exposures') .option('--items ', 'number of items to take', Number) .option('--skip ', 'number of items to skip', Number) .action( @@ -59,6 +60,15 @@ const listCommand = new Command('list') const globalOptions = getGlobalOptions(listCommand); const client = await getAPIClientFromOptions(globalOptions); + if (options.validExposures && options.invalidExposures) { + throw new Error('--valid-exposures and --invalid-exposures are mutually exclusive'); + } + const validExposures = options.validExposures + ? true + : options.invalidExposures + ? false + : undefined; + const fromTs = parseDateFlagOrUndefined(options.from); const toTs = parseDateFlagOrUndefined(options.to); @@ -71,7 +81,7 @@ const listCommand = new Command('list') eventNames: options.eventName.length > 0 ? options.eventName : undefined, unitUids: options.unitUid.length > 0 ? options.unitUid : undefined, environmentTypes: options.envType.length > 0 ? options.envType : undefined, - effectiveExposures: options.effectiveExposures || undefined, + validExposures, take: options.items, skip: options.skip, }); diff --git a/src/commands/experiments/list.ts b/src/commands/experiments/list.ts index d7e5022..4ea811e 100644 --- a/src/commands/experiments/list.ts +++ b/src/commands/experiments/list.ts @@ -3,11 +3,11 @@ import { getAPIClientFromOptions, getGlobalOptions, printFormatted, + shouldOutputIdsOnly, withErrorHandling, } from '../../lib/utils/api-helper.js'; import { printPaginationFooter } from '../../lib/utils/pagination.js'; import { getDefaultType } from './default-type.js'; -import { isStdoutPiped } from '../../lib/utils/stdin.js'; import { listExperiments } from '../../core/experiments/list.js'; export const listCommand = new Command('list') @@ -73,7 +73,7 @@ export const listCommand = new Command('list') raw: globalOptions.raw, }); - if (globalOptions.output === 'ids' || (isStdoutPiped() && !options.output)) { + if (shouldOutputIdsOnly(globalOptions)) { for (const exp of result.data) console.log((exp as Record).id); return; } diff --git a/src/core/events/events.test.ts b/src/core/events/events.test.ts index dea538c..d6499f9 100644 --- a/src/core/events/events.test.ts +++ b/src/core/events/events.test.ts @@ -109,6 +109,28 @@ describe('events', () => { await listEvents(mockClient as any, {}); expect(mockClient.listEvents).toHaveBeenCalledWith({}); }); + + it('should send effective_exposures=true when validExposures is true', async () => { + mockClient.listEvents.mockResolvedValue([]); + await listEvents(mockClient as any, { validExposures: true }); + expect(mockClient.listEvents).toHaveBeenCalledWith({ + filters: { effective_exposures: true }, + }); + }); + + it('should send effective_exposures=false when validExposures is false', async () => { + mockClient.listEvents.mockResolvedValue([]); + await listEvents(mockClient as any, { validExposures: false }); + expect(mockClient.listEvents).toHaveBeenCalledWith({ + filters: { effective_exposures: false }, + }); + }); + + it('should omit effective_exposures when validExposures is undefined', async () => { + mockClient.listEvents.mockResolvedValue([]); + await listEvents(mockClient as any, { from: 1 }); + expect(mockClient.listEvents).toHaveBeenCalledWith({ filters: { from: 1 } }); + }); }); describe('listEventsHistory', () => { diff --git a/src/core/events/events.ts b/src/core/events/events.ts index 248f9b0..8ef1b35 100644 --- a/src/core/events/events.ts +++ b/src/core/events/events.ts @@ -44,7 +44,7 @@ export interface ListEventsParams { eventNames?: string[] | undefined; unitUids?: string[] | undefined; environmentTypes?: string[] | undefined; - effectiveExposures?: boolean | undefined; + validExposures?: boolean | undefined; take?: number | undefined; skip?: number | undefined; } @@ -64,7 +64,7 @@ export async function listEvents( if (params.unitUids && params.unitUids.length > 0) filters.unit_uids = params.unitUids; if (params.environmentTypes && params.environmentTypes.length > 0) filters.environment_types = params.environmentTypes; - if (params.effectiveExposures) filters.effective_exposures = true; + if (params.validExposures !== undefined) filters.effective_exposures = params.validExposures; const body: Record = {}; if (Object.keys(filters).length > 0) body.filters = filters; diff --git a/src/lib/utils/api-helper.test.ts b/src/lib/utils/api-helper.test.ts index b458983..d38aeeb 100644 --- a/src/lib/utils/api-helper.test.ts +++ b/src/lib/utils/api-helper.test.ts @@ -23,6 +23,7 @@ import { getAPIClientFromOptions, getGlobalOptions, printFormatted, + shouldOutputIdsOnly, withErrorHandling, resolveAuth, } from './api-helper.js'; @@ -30,6 +31,7 @@ import { loadConfig, getProfile } from '../config/config.js'; import { getAPIKey, getOAuthToken } from '../config/keyring.js'; import { createAPIClient } from '../api/client.js'; import { formatOutput } from '../output/formatter.js'; +import { setTTYOverride } from './stdin.js'; describe('API Helper', () => { const savedEnv: Record = {}; @@ -307,6 +309,86 @@ describe('API Helper', () => { }) ); }); + + describe('outputExplicit', () => { + it('is false when --output is not passed on the CLI', () => { + // Mock command at line 192 registers --output with a default of 'table'. + // Defaults must NOT count as "explicit" — that's the whole point. + mockCommand.parse(['node', 'test']); + + const options = getGlobalOptions(mockCommand); + expect(options.output).toBe('table'); + expect(options.outputExplicit).toBe(false); + }); + + it('is true when --output is passed on the CLI', () => { + mockCommand.parse(['node', 'test', '--output', 'json']); + + const options = getGlobalOptions(mockCommand); + expect(options.output).toBe('json'); + expect(options.outputExplicit).toBe(true); + }); + + it('is true even when the user explicitly chooses table', () => { + // Regression: prior implementation could not distinguish "no flag" + // from "explicit -o table", which broke piping into table format. + mockCommand.parse(['node', 'test', '--output', 'table']); + + const options = getGlobalOptions(mockCommand); + expect(options.output).toBe('table'); + expect(options.outputExplicit).toBe(true); + }); + + it('is detected on the leaf subcommand when --output is set as a global option', () => { + const root = new Command(); + root.option('-o, --output ', 'output format'); + const sub = new Command('list'); + root.addCommand(sub); + + root.parse(['node', 'test', '--output', 'json', 'list']); + + const options = getGlobalOptions(sub); + expect(options.output).toBe('json'); + expect(options.outputExplicit).toBe(true); + }); + }); + }); + + describe('shouldOutputIdsOnly', () => { + afterEach(() => { + setTTYOverride({ stdin: true, stdout: true }); + }); + + it('returns true when output is explicitly "ids"', () => { + setTTYOverride({ stdout: true }); + expect(shouldOutputIdsOnly({ output: 'ids', outputExplicit: true })).toBe(true); + }); + + it('returns true when stdout is piped and no explicit --output was given', () => { + setTTYOverride({ stdout: false }); + expect(shouldOutputIdsOnly({ output: 'table', outputExplicit: false })).toBe(true); + }); + + it('returns false when stdout is piped but the user explicitly chose --output json', () => { + // Regression: this is the original bug — piped + -o json was forcing ids. + setTTYOverride({ stdout: false }); + expect(shouldOutputIdsOnly({ output: 'json', outputExplicit: true })).toBe(false); + }); + + it('returns false when stdout is piped but the user explicitly chose --output table', () => { + setTTYOverride({ stdout: false }); + expect(shouldOutputIdsOnly({ output: 'table', outputExplicit: true })).toBe(false); + }); + + it('returns false on a TTY when no explicit output was given', () => { + setTTYOverride({ stdout: true }); + expect(shouldOutputIdsOnly({ output: 'table', outputExplicit: false })).toBe(false); + }); + + it('returns false on a TTY when explicit --output json was given', () => { + setTTYOverride({ stdout: true }); + expect(shouldOutputIdsOnly({ output: 'json', outputExplicit: true })).toBe(false); + }); }); describe('withErrorHandling', () => { diff --git a/src/lib/utils/api-helper.ts b/src/lib/utils/api-helper.ts index 9e09fe8..e5514be 100644 --- a/src/lib/utils/api-helper.ts +++ b/src/lib/utils/api-helper.ts @@ -4,6 +4,7 @@ import { getAPIKey, getOAuthToken } from '../config/keyring.js'; import type { AuthConfig } from '../api/axios-adapter.js'; import { Command } from 'commander'; import { formatOutput, type OutputFormat } from '../output/formatter.js'; +import { isStdoutPiped } from './stdin.js'; export async function resolveAPIKey(options: GlobalOptions): Promise { const config = loadConfig(); @@ -101,6 +102,10 @@ export interface GlobalOptions { app?: string; env?: string; output?: OutputFormat; + // True when the user explicitly set --output (via CLI or env), false when + // the value was defaulted. Used to decide whether to auto-switch to ids + // mode when stdout is piped. + outputExplicit?: boolean; noColor?: boolean; // True when the user explicitly disabled color (--no-color or NO_COLOR env). // noColor above additionally disables when stdout isn't a TTY; this field @@ -135,6 +140,8 @@ const VALID_FORMATS: OutputFormat[] = [ export function getGlobalOptions(cmd: Command): GlobalOptions { const opts = cmd.optsWithGlobals(); const output = (opts.output || 'table') as OutputFormat; + const outputSource = cmd.getOptionValueSourceWithGlobals('output'); + const outputExplicit = outputSource === 'cli' || outputSource === 'env'; if (!VALID_FORMATS.includes(output)) { throw new Error( @@ -155,6 +162,7 @@ export function getGlobalOptions(cmd: Command): GlobalOptions { app: opts.app, env: opts.env, output, + outputExplicit, noColor: colorDisabled || !process.stdout.isTTY, colorDisabled, verbose: opts.verbose || false, @@ -172,6 +180,13 @@ export function getGlobalOptions(cmd: Command): GlobalOptions { }; } +// When stdout is piped to another process, list commands fall back to +// emitting ids only — unless the user explicitly chose an output format. +// Centralizes the rule so every list-style command behaves the same way. +export function shouldOutputIdsOnly(globalOptions: GlobalOptions): boolean { + return globalOptions.output === 'ids' || (isStdoutPiped() && !globalOptions.outputExplicit); +} + export function printFormatted(data: unknown, globalOptions: GlobalOptions): void { const output = formatOutput(data, globalOptions.output, { noColor: globalOptions.noColor ?? false, diff --git a/src/lib/utils/list-command.test.ts b/src/lib/utils/list-command.test.ts index 6541ede..7526e16 100644 --- a/src/lib/utils/list-command.test.ts +++ b/src/lib/utils/list-command.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { createListCommand } from './list-command.js'; import { getAPIClientFromOptions, getGlobalOptions, printFormatted } from './api-helper.js'; import { resetCommand } from '../../test/helpers/command-reset.js'; +import { setTTYOverride } from './stdin.js'; vi.mock('./api-helper.js', async (importOriginal) => { const actual = await importOriginal(); @@ -68,4 +69,66 @@ describe('createListCommand', () => { expect(mockFetch).toHaveBeenCalledWith(mockClient, expect.any(Object)); expect(printFormatted).toHaveBeenCalled(); }); + + describe('output behavior under piping', () => { + afterEach(() => { + setTTYOverride({ stdin: true, stdout: true }); + }); + + it('prints only ids when stdout is piped and no explicit output is set', async () => { + setTTYOverride({ stdout: false }); + vi.mocked(getGlobalOptions).mockReturnValue({ + output: 'table', + outputExplicit: false, + } as any); + const fetch = vi.fn().mockResolvedValue([{ id: 1 }, { id: 2 }, { id: 3 }]); + const cmd = createListCommand({ description: 'List items', fetch }); + resetCommand(cmd); + + await cmd.parseAsync(['node', 'test']); + + expect(printFormatted).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(1); + expect(consoleSpy).toHaveBeenCalledWith(2); + expect(consoleSpy).toHaveBeenCalledWith(3); + }); + + it('renders the requested format when stdout is piped and -o json is explicit', async () => { + // Regression: previously `--items 200 -o json` over a pipe collapsed + // to id-only output because the command read its local `options.output` + // (always undefined — the flag lives on the root program), so the + // "is explicit?" check always said no. + setTTYOverride({ stdout: false }); + vi.mocked(getGlobalOptions).mockReturnValue({ + output: 'json', + outputExplicit: true, + } as any); + const fetch = vi.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]); + const cmd = createListCommand({ description: 'List items', fetch }); + resetCommand(cmd); + + await cmd.parseAsync(['node', 'test']); + + expect(printFormatted).toHaveBeenCalledOnce(); + // Items must not have been spilled as bare ids on stdout. + expect(consoleSpy).not.toHaveBeenCalledWith(1); + expect(consoleSpy).not.toHaveBeenCalledWith(2); + }); + + it('prints only ids when output is explicitly "ids", even on a TTY', async () => { + setTTYOverride({ stdout: true }); + vi.mocked(getGlobalOptions).mockReturnValue({ + output: 'ids', + outputExplicit: true, + } as any); + const fetch = vi.fn().mockResolvedValue([{ id: 42 }]); + const cmd = createListCommand({ description: 'List items', fetch }); + resetCommand(cmd); + + await cmd.parseAsync(['node', 'test']); + + expect(printFormatted).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalledWith(42); + }); + }); }); diff --git a/src/lib/utils/list-command.ts b/src/lib/utils/list-command.ts index 4bfa452..ba72c6f 100644 --- a/src/lib/utils/list-command.ts +++ b/src/lib/utils/list-command.ts @@ -3,9 +3,9 @@ import { getAPIClientFromOptions, getGlobalOptions, printFormatted, + shouldOutputIdsOnly, withErrorHandling, } from './api-helper.js'; -import { isStdoutPiped } from './stdin.js'; import { addPaginationOptions, printPaginationFooter } from './pagination.js'; import { applyShowExclude } from '../../api-client/entity-summary.js'; import type { APIClient } from '../../api-client/api-client.js'; @@ -42,7 +42,7 @@ export function createListCommand(opts: ListCommandOptions): Command { const items = await opts.fetch(client, options); - if (globalOptions.output === 'ids' || (isStdoutPiped() && !options.output)) { + if (shouldOutputIdsOnly(globalOptions)) { for (const item of items) console.log((item as Record).id); return; } diff --git a/vitest.config.ts b/vitest.config.ts index 6e89984..6cc345b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,6 +1,6 @@ import { config } from 'dotenv'; config({ path: ['.env.test.local', '.env.local', '.env'] }); -import { defineConfig } from 'vitest/config'; +import { configDefaults, defineConfig } from 'vitest/config'; const isLiveMode = process.env.USE_LIVE_API === '1'; @@ -12,6 +12,9 @@ export default defineConfig({ hookTimeout: isLiveMode ? 15_000 : 10_000, setupFiles: ['./src/test/setup.ts'], fileParallelism: false, + // Sibling git worktrees live under .worktrees/ and would otherwise be + // discovered as duplicate test files against stale source. + exclude: [...configDefaults.exclude, '.worktrees/**'], server: { deps: { inline: ['absmartly-api-mocks'],