From 2491e0b7841b8e40844f9269ac436e3c16259164 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 03:38:10 +0000 Subject: [PATCH 1/2] fix(cli): flush stdio before exiting so piped output survives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent-relay cloud session --json` (and any other command) could emit nothing at all — empty stdout *and* empty stderr — when stdout was a pipe rather than a TTY. Node's stdio writes are synchronous for files and POSIX TTYs but asynchronous for pipes on macOS, so a `process.exit()` in the same tick as the write discards whatever is still buffered. Interactively the same command printed fine, which made the failure look TTY-conditional. Add `exitAfterFlush()`, which drains stdout and stderr (bounded by a timeout, so a stalled reader can't wedge the exit) before calling `process.exit`, and route the three hard-exit paths through it: the top-level failure handler, the `CliExit` path in `runCli`, and `runSignalHandler`. Fixes #1371 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0129AoahEiYiRdbPGQjNpjdf --- CHANGELOG.md | 1 + packages/cli/src/cli/bootstrap.ts | 7 +- packages/cli/src/cli/commands/cloud.test.ts | 44 ++++++++ packages/cli/src/cli/index.ts | 8 +- packages/cli/src/cli/lib/exit.ts | 5 +- packages/cli/src/cli/lib/flush-stdio.test.ts | 102 +++++++++++++++++++ packages/cli/src/cli/lib/flush-stdio.ts | 94 +++++++++++++++++ 7 files changed, 255 insertions(+), 6 deletions(-) create mode 100644 packages/cli/src/cli/lib/flush-stdio.test.ts create mode 100644 packages/cli/src/cli/lib/flush-stdio.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c4eef5630..4ab9688ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- CLI output no longer disappears when stdout or stderr is a pipe instead of a terminal. Node's stdio writes are asynchronous for pipes on macOS, so exiting in the same tick as the write discarded whatever was still buffered — `agent-relay cloud session --json | parser` and `$(agent-relay …)` could come back with empty stdout _and_ empty stderr, hiding the payload and the error that explained the failure. Every hard-exit path now drains stdio first. - A CLI command that fails with a non-`Error` value now reports it. A protocol-shaped rejection such as `{ status: 401 }` from a broker client printed `[object Object]`, and an `Error` with an empty message printed nothing at all; both now surface the message, status, and code, with credentials redacted. Applies to the top-level failure handler, `node agent attach`, and broker request failures. - `agent-relay integration webhook create` now works. It took a `` argument and sent `{ url, event }`, but `POST /v1/webhooks` accepts `{ channel, name? }` and returns the URL — so every invocation failed with `channel is required`. It now takes `` with an optional `--name`, matching `create-inbound`, which posts to the same endpoint. - `@agent-relay/sdk` `RelayCreateWebhookInput` declared a required `url` and an `event`, neither of which the endpoint accepts. It is now `{ channel, name? }`. Code passing `url`/`event` was already failing at runtime. diff --git a/packages/cli/src/cli/bootstrap.ts b/packages/cli/src/cli/bootstrap.ts index be34f9803..8d8db38c9 100644 --- a/packages/cli/src/cli/bootstrap.ts +++ b/packages/cli/src/cli/bootstrap.ts @@ -30,6 +30,7 @@ import { import { ensureWebSocketGlobal } from './lib/ensure-websocket.js'; import { assertSupportedNodeVersion } from './lib/node-version.js'; import { CliExit } from './lib/exit.js'; +import { exitAfterFlush } from './lib/flush-stdio.js'; import { errorClassName } from './lib/telemetry-helpers.js'; import { registerSetupCommands } from './commands/setup.js'; import { registerCoreCommands, registerCoreMaintenance } from './commands/core.js'; @@ -522,8 +523,10 @@ export async function runCli(argv: string[] = process.argv): Promise { } if (isCliExit) { - // Flush is done — now actually exit with the code the command asked for. - process.exit(err.code); + // Telemetry is flushed — now drain whatever the command printed (a piped + // stdout is asynchronous on macOS, so exiting in this tick would drop it) + // and exit with the code the command asked for. + await exitAfterFlush(err.code); } throw err; } diff --git a/packages/cli/src/cli/commands/cloud.test.ts b/packages/cli/src/cli/commands/cloud.test.ts index 06c1d8091..c1a2c9df9 100644 --- a/packages/cli/src/cli/commands/cloud.test.ts +++ b/packages/cli/src/cli/commands/cloud.test.ts @@ -404,6 +404,50 @@ describe('registerCloudCommands', () => { expect(sessionJson).not.toHaveProperty('refreshToken'); }); + it('writes the session JSON through the default stdout logger when stdout is not a TTY', async () => { + // Regression: scripted callers (`cloud session --json | parser`, `$(...)`) + // got empty stdout. The command must not gate its output on a TTY, and the + // production logger — not just an injected test double — must emit it. + vi.mocked(ensureCloudSession).mockResolvedValueOnce({ + auth: { + apiUrl: 'https://cloud.test', + accessToken: 'access-token', + refreshToken: 'refresh-token', + accessTokenExpiresAt: '2999-01-01T00:00:00.000Z', + }, + client: {} as never, + }); + + const program = new Command(); + program.exitOverride(); + // No dependency overrides: this exercises the production `console.log` path. + registerCloudCommands(program); + + const chunks: string[] = []; + const log = vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + chunks.push(args.map((arg) => String(arg)).join(' ')); + }); + const originalIsTty = Object.getOwnPropertyDescriptor(process.stdout, 'isTTY'); + Object.defineProperty(process.stdout, 'isTTY', { value: false, configurable: true }); + + try { + await program.parseAsync(['node', 'agent-relay', 'cloud', 'session', '--json']); + } finally { + log.mockRestore(); + if (originalIsTty) { + Object.defineProperty(process.stdout, 'isTTY', originalIsTty); + } else { + delete (process.stdout as { isTTY?: boolean }).isTTY; + } + } + + expect(JSON.parse(chunks.join(''))).toEqual({ + apiUrl: 'https://cloud.test', + accessToken: '…oken', + accessTokenExpiresAt: '2999-01-01T00:00:00.000Z', + }); + }); + it('includes the raw access token in JSON output only with --reveal-token', async () => { const { program, deps } = createHarness(); vi.mocked(ensureCloudSession).mockResolvedValueOnce({ diff --git a/packages/cli/src/cli/index.ts b/packages/cli/src/cli/index.ts index eefafbfa8..330f8f90e 100644 --- a/packages/cli/src/cli/index.ts +++ b/packages/cli/src/cli/index.ts @@ -8,6 +8,7 @@ import { runAiSdkSidecarMain } from '@agent-relay/harnesses'; import { runCli } from './bootstrap.js'; import { describeError } from './lib/describe-error.js'; +import { exitAfterFlush } from './lib/flush-stdio.js'; export * from './bootstrap.js'; @@ -23,12 +24,15 @@ function isEntrypoint(): boolean { if (isEntrypoint()) { const main = process.argv[2] === '__ai-sdk-sidecar' ? runAiSdkSidecarMain(process.argv.slice(3)) : runCli(); - main.catch((err) => { + main.catch(async (err) => { // Commander will have already printed a helpful message for parse errors. // For other top-level failures, surface them to stderr and exit non-zero. // `describeError` keeps a non-Error rejection (e.g. `{ status: 401 }` from // a broker client) readable instead of collapsing it to `[object Object]`. process.stderr.write(`${describeError(err)}\n`); - process.exit(1); + // Flush first: on macOS a piped stderr is asynchronous, so exiting in this + // tick would discard the message we just wrote and leave the caller with a + // silent non-zero exit. + await exitAfterFlush(1); }); } diff --git a/packages/cli/src/cli/lib/exit.ts b/packages/cli/src/cli/lib/exit.ts index d7e431b67..e68cbab8d 100644 --- a/packages/cli/src/cli/lib/exit.ts +++ b/packages/cli/src/cli/lib/exit.ts @@ -16,6 +16,7 @@ */ import { shutdown as shutdownTelemetry } from '../telemetry/index.js'; +import { exitAfterFlush } from './flush-stdio.js'; export class CliExit extends Error { /** Intended process exit code. */ @@ -67,10 +68,10 @@ export function runSignalHandler(handler: () => void | Promise): void { } catch { // Best-effort — never let flush errors mask the intended exit. } - process.exit(err.code); + await exitAfterFlush(err.code); } // eslint-disable-next-line no-console console.error(err); - process.exit(1); + await exitAfterFlush(1); }); } diff --git a/packages/cli/src/cli/lib/flush-stdio.test.ts b/packages/cli/src/cli/lib/flush-stdio.test.ts new file mode 100644 index 000000000..1840f4081 --- /dev/null +++ b/packages/cli/src/cli/lib/flush-stdio.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { exitAfterFlush, flushStdio, flushStream, type FlushableStream } from './flush-stdio.js'; + +/** + * A stream that behaves the way a piped stdout does on macOS: the write is + * accepted immediately, but only completes once the OS has taken the bytes. + */ +function deferredStream(): FlushableStream & { complete: () => void; writes: string[] } { + const pending: (() => void)[] = []; + const writes: string[] = []; + + return { + writes, + write(chunk: string, callback?: (error?: Error | null) => void): boolean { + writes.push(chunk); + if (callback) pending.push(() => callback(null)); + return true; + }, + complete(): void { + while (pending.length > 0) pending.shift()?.(); + }, + }; +} + +describe('flushStream', () => { + it('waits for the pending write to complete', async () => { + const stream = deferredStream(); + let drained = false; + const flushed = flushStream(stream).then(() => { + drained = true; + }); + + await Promise.resolve(); + expect(drained).toBe(false); + + stream.complete(); + await flushed; + expect(drained).toBe(true); + }); + + it('gives up after the timeout so a stalled reader cannot wedge the exit', async () => { + vi.useFakeTimers(); + try { + const stream = deferredStream(); + const flushed = flushStream(stream, 50); + await vi.advanceTimersByTimeAsync(50); + await expect(flushed).resolves.toBeUndefined(); + } finally { + vi.useRealTimers(); + } + }); + + it('resolves without writing when there is no usable stream', async () => { + await expect(flushStream(undefined)).resolves.toBeUndefined(); + + const destroyed = { ...deferredStream(), destroyed: true }; + await flushStream(destroyed); + expect(destroyed.writes).toEqual([]); + }); + + it('resolves when the write itself throws', async () => { + const stream: FlushableStream = { + write() { + throw new Error('EPIPE'); + }, + }; + + await expect(flushStream(stream)).resolves.toBeUndefined(); + }); +}); + +describe('flushStdio', () => { + it('drains every stream it is given', async () => { + const out = deferredStream(); + const err = deferredStream(); + const flushed = flushStdio({ streams: [out, err] }); + + out.complete(); + err.complete(); + + await expect(flushed).resolves.toBeUndefined(); + expect(out.writes).toEqual(['']); + expect(err.writes).toEqual(['']); + }); +}); + +describe('exitAfterFlush', () => { + it('exits only once stdio has drained, with the requested code', async () => { + const out = deferredStream(); + const exit = vi.fn((code: number) => code as never); + + const exited = exitAfterFlush(3, { streams: [out], exit }); + + await Promise.resolve(); + expect(exit).not.toHaveBeenCalled(); + + out.complete(); + await exited; + expect(exit).toHaveBeenCalledWith(3); + }); +}); diff --git a/packages/cli/src/cli/lib/flush-stdio.ts b/packages/cli/src/cli/lib/flush-stdio.ts new file mode 100644 index 000000000..8c761766b --- /dev/null +++ b/packages/cli/src/cli/lib/flush-stdio.ts @@ -0,0 +1,94 @@ +/** + * Flush `process.stdout` / `process.stderr` before a hard exit. + * + * Why this exists: Node's stdio writes are only synchronous for files and for + * POSIX TTYs. Pipes and sockets are **asynchronous on macOS**, so a + * `process.exit()` in the same tick as a `console.log()` throws away whatever + * is still buffered. The visible symptom is a command that prints fine in a + * terminal and prints *nothing* when piped or captured with `$(...)` — the + * `--json` payload disappears, and so does the error text that would have + * explained why the command failed. + * + * Every exit path that goes through a real `process.exit(code)` should await + * {@link exitAfterFlush} instead, which drains both streams first. The drain + * is bounded by a timeout so a stalled reader can never wedge the exit. + */ + +/** Minimal writable surface we need; keeps the helper testable. */ +export interface FlushableStream { + write(chunk: string, callback?: (error?: Error | null) => void): boolean; + readonly destroyed?: boolean; + readonly writableEnded?: boolean; +} + +/** How long to wait for a single stream to drain before giving up on it. */ +export const DEFAULT_FLUSH_TIMEOUT_MS = 2_000; + +export interface FlushOptions { + /** Streams to drain. Defaults to `[process.stdout, process.stderr]`. */ + streams?: (FlushableStream | undefined)[]; + /** Per-stream drain budget. Defaults to {@link DEFAULT_FLUSH_TIMEOUT_MS}. */ + timeoutMs?: number; +} + +/** + * Resolve once everything already written to `stream` has reached the OS. + * + * A zero-length write is queued behind the pending chunks, so its completion + * callback fires only after those chunks have been written — which is exactly + * the signal we lack on platforms where stdio is asynchronous. Errors (a + * closed pipe, for instance) resolve too: we are on our way out either way. + */ +export function flushStream( + stream: FlushableStream | undefined, + timeoutMs: number = DEFAULT_FLUSH_TIMEOUT_MS +): Promise { + return new Promise((resolve) => { + if (!stream || typeof stream.write !== 'function' || stream.destroyed || stream.writableEnded) { + resolve(); + return; + } + + let settled = false; + const settle = (): void => { + if (settled) return; + settled = true; + clearTimeout(timer); + resolve(); + }; + + const timer = setTimeout(settle, timeoutMs); + // Never let the flush watchdog be the reason the process stays alive. + timer.unref?.(); + + try { + stream.write('', () => settle()); + } catch { + settle(); + } + }); +} + +/** Drain stdout and stderr concurrently. Never rejects. */ +export async function flushStdio(options: FlushOptions = {}): Promise { + const streams = options.streams ?? [process.stdout, process.stderr]; + const timeoutMs = options.timeoutMs ?? DEFAULT_FLUSH_TIMEOUT_MS; + await Promise.all(streams.map((stream) => flushStream(stream, timeoutMs))); +} + +export interface ExitAfterFlushOptions extends FlushOptions { + /** Injectable for tests; defaults to the real `process.exit`. */ + exit?: (code: number) => never; +} + +/** + * Drain stdio, then exit with `code`. + * + * Returns `Promise` because the default `exit` does not return; the + * declared return type keeps call sites from needing an unreachable `return`. + */ +export async function exitAfterFlush(code: number, options: ExitAfterFlushOptions = {}): Promise { + await flushStdio(options); + const exit = options.exit ?? ((value: number) => process.exit(value)); + return exit(code); +} From 664c640e5412e04a7214e8fe8e81bbfcc69d2c83 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 03:12:47 +0000 Subject: [PATCH 2/2] docs(changelog): move the stdio-drain entry under Unreleased The 11.3.1 release was cut after this branch started, which renamed the pending heading the entry was written under, landing it inside a shipped release section. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0129AoahEiYiRdbPGQjNpjdf --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a479194b8..4525819da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,11 @@ All notable changes to Agent Relay will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Patch] + +### Fixed + +- CLI output no longer disappears when stdout or stderr is a pipe instead of a terminal. Node's stdio writes are asynchronous for pipes on macOS, so exiting in the same tick as the write discarded whatever was still buffered — `agent-relay cloud session --json | parser` and `$(agent-relay …)` could come back with empty stdout _and_ empty stderr, hiding the payload and the error that explained the failure. Every hard-exit path now drains stdio first. ## [11.4.0] - 2026-08-02 @@ -27,7 +31,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- CLI output no longer disappears when stdout or stderr is a pipe instead of a terminal. Node's stdio writes are asynchronous for pipes on macOS, so exiting in the same tick as the write discarded whatever was still buffered — `agent-relay cloud session --json | parser` and `$(agent-relay …)` could come back with empty stdout _and_ empty stderr, hiding the payload and the error that explained the failure. Every hard-exit path now drains stdio first. - A CLI command that fails with a non-`Error` value now reports it. A protocol-shaped rejection such as `{ status: 401 }` from a broker client printed `[object Object]`, and an `Error` with an empty message printed nothing at all; both now surface the message, status, and code, with credentials redacted. Applies to the top-level failure handler, `node agent attach`, and broker request failures. - `agent-relay integration webhook create` now works. It took a `` argument and sent `{ url, event }`, but `POST /v1/webhooks` accepts `{ channel, name? }` and returns the URL — so every invocation failed with `channel is required`. It now takes `` with an optional `--name`, matching `create-inbound`, which posts to the same endpoint. - `@agent-relay/sdk` `RelayCreateWebhookInput` declared a required `url` and an `event`, neither of which the endpoint accepts. It is now `{ channel, name? }`. Code passing `url`/`event` was already failing at runtime.