diff --git a/.agents/rules/development/coding-conventions.md b/.agents/rules/development/coding-conventions.md index 52ed376ee..1f79c449d 100644 --- a/.agents/rules/development/coding-conventions.md +++ b/.agents/rules/development/coding-conventions.md @@ -18,3 +18,8 @@ description: Core coding conventions for the abapify monorepo. TypeScript strict - Cross-package imports: `@abapify/` - Internal file imports: extensionless relative paths — see [bundler-imports](bundler-imports.md) for details - `workspace:*` protocol for local workspace deps — see `$link-workspace-packages` skill for setup +- **CLI stream contract** — stdout contains only the command payload (source, + JSON, XML, or other data intended for piping); progress, diagnostics, and + errors go to stderr. Source-producing commands must be safe to use as the + input of a corresponding write command. Enforce this via a regression test + at the shared output/progress abstraction, not in an individual command. diff --git a/packages/adt-cli/src/lib/shared/adt-client.ts b/packages/adt-cli/src/lib/shared/adt-client.ts index f3fffbd62..62815d8da 100644 --- a/packages/adt-cli/src/lib/shared/adt-client.ts +++ b/packages/adt-cli/src/lib/shared/adt-client.ts @@ -70,10 +70,10 @@ export const silentLogger: Logger = { * Console logger - outputs to console (used when enableLogging is true) */ export const consoleLogger: Logger = { - trace: (msg: string) => console.debug(msg), - debug: (msg: string) => console.debug(msg), - info: (msg: string) => console.log(msg), - warn: (msg: string) => console.warn(msg), + trace: (msg: string) => console.error(msg), + debug: (msg: string) => console.error(msg), + info: (msg: string) => console.error(msg), + warn: (msg: string) => console.error(msg), error: (msg: string) => console.error(msg), fatal: (msg: string) => console.error(msg), child: () => consoleLogger, diff --git a/packages/adt-cli/src/lib/utils/logger-config.ts b/packages/adt-cli/src/lib/utils/logger-config.ts index f8c11f25b..ec340bf08 100644 --- a/packages/adt-cli/src/lib/utils/logger-config.ts +++ b/packages/adt-cli/src/lib/utils/logger-config.ts @@ -45,6 +45,7 @@ export function createCliLogger(options: LoggerOptions = {}): Logger { ignore: 'pid,hostname,time', messageFormat: '[{component}] {msg}', hideObject: true, + destination: 2, }, } : undefined, diff --git a/packages/adt-cli/src/lib/utils/progress-reporter.test.ts b/packages/adt-cli/src/lib/utils/progress-reporter.test.ts new file mode 100644 index 000000000..c22ee3208 --- /dev/null +++ b/packages/adt-cli/src/lib/utils/progress-reporter.test.ts @@ -0,0 +1,55 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createProgressReporter } from './progress-reporter'; + +describe('createProgressReporter', () => { + const stdout = vi + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + + afterEach(() => { + stdout.mockClear(); + stderr.mockClear(); + }); + + it('keeps stdout clean and writes compact step/done to stderr', () => { + const reporter = createProgressReporter({ compact: true }); + + reporter.step('Reading source...'); + reporter.done(); + + expect(stdout).not.toHaveBeenCalled(); + expect(stderr).toHaveBeenCalledTimes(3); + expect(stderr).toHaveBeenNthCalledWith(1, 'Reading source...'); + expect(stderr).toHaveBeenNthCalledWith(2, '\r\x1b[K'); + expect(stderr).toHaveBeenNthCalledWith(3, 'Reading source...\n'); + }); + + it('writes compact persist messages to stderr without touching stdout', () => { + const reporter = createProgressReporter({ compact: true }); + + reporter.step('Reading source...'); + reporter.persist('Persisted source'); + + expect(stdout).not.toHaveBeenCalled(); + expect(stderr).toHaveBeenCalledTimes(3); + expect(stderr).toHaveBeenNthCalledWith(1, 'Reading source...'); + expect(stderr).toHaveBeenNthCalledWith(2, '\r\x1b[K'); + expect(stderr).toHaveBeenNthCalledWith(3, 'Persisted source\n'); + }); + + it('writes compact done(finalMessage) to stderr without touching stdout', () => { + const reporter = createProgressReporter({ compact: true }); + + reporter.step('Reading source...'); + reporter.done('Finished'); + + expect(stdout).not.toHaveBeenCalled(); + expect(stderr).toHaveBeenCalledTimes(3); + expect(stderr).toHaveBeenNthCalledWith(1, 'Reading source...'); + expect(stderr).toHaveBeenNthCalledWith(2, '\r\x1b[K'); + expect(stderr).toHaveBeenNthCalledWith(3, 'Finished\n'); + }); +}); diff --git a/packages/adt-cli/src/lib/utils/progress-reporter.ts b/packages/adt-cli/src/lib/utils/progress-reporter.ts index 7e7cc1835..5f6427959 100644 --- a/packages/adt-cli/src/lib/utils/progress-reporter.ts +++ b/packages/adt-cli/src/lib/utils/progress-reporter.ts @@ -1,7 +1,9 @@ /** * Lightweight progress reporter for CLI output. * - Compact mode keeps updates on a single line (overwriting previous text). - * - Non-compact mode logs through the provided logger (or console). + * - Non-compact mode logs through the provided logger (CLI loggers target stderr). + * - Progress is written to stderr so stdout stays safe for piping source, + * JSON, XML, and other command payloads. * * When a logger is provided, progress messages are tagged with { progress: true }. */ @@ -54,7 +56,7 @@ export function createProgressReporter( const clearLine = () => { if (open) { - process.stdout.write('\r\x1b[K'); + process.stderr.write('\r\x1b[K'); } }; @@ -65,7 +67,7 @@ export function createProgressReporter( return; } clearLine(); - process.stdout.write(clean); + process.stderr.write(clean); lastMessage = clean; open = true; logWithFlag(clean, { transient: true }); @@ -75,7 +77,7 @@ export function createProgressReporter( const clean = sanitize(message); clearLine(); if (compact || !logger) { - process.stdout.write(`${clean}\n`); + process.stderr.write(`${clean}\n`); } logWithFlag(clean); open = false; @@ -91,10 +93,10 @@ export function createProgressReporter( if (compact) { if (finalMessage) { const clean = sanitize(finalMessage); - process.stdout.write(`${clean}\n`); + process.stderr.write(`${clean}\n`); logWithFlag(clean); } else if (open && lastMessage) { - process.stdout.write(`${lastMessage}\n`); + process.stderr.write(`${lastMessage}\n`); logWithFlag(lastMessage); } } else {