diff --git a/experiments/issue-46-redirection-parity.mjs b/experiments/issue-46-redirection-parity.mjs index 007267d3..2566d359 100644 --- a/experiments/issue-46-redirection-parity.mjs +++ b/experiments/issue-46-redirection-parity.mjs @@ -6,7 +6,8 @@ // (virtual) command is dispatched to that built-in with the shell operators // left in place as literal arguments, so the redirection silently does // nothing. This script diffs command-stream against /bin/sh so any divergence -// in exit code, stdout, or files written is visible. +// in exit code, stdout, stderr, or files written is visible. Issue #47 exposed +// the stderr part of that contract with a CLI success URL. import { execSync, spawnSync } from 'child_process'; import { mkdtempSync, rmSync, writeFileSync } from 'fs'; import { tmpdir } from 'os'; @@ -28,6 +29,8 @@ const CASES = [ 'seq 1 3 > out.txt', 'basename /a/b > out.txt', // stderr redirection on a built-in. + 'echo https://github.com/octo/example/pull/123 >&2', + "sh -c 'echo https://github.com/octo/example/pull/123 >&2' 2>&1", 'echo hello 2> err.txt', 'ls /definitely/missing/path 2>/dev/null', 'ls /definitely/missing/path 2>&1', @@ -57,14 +60,18 @@ for (const cmd of CASES) { cwd: dirSh, encoding: 'utf8', }); - const expected = { code: sh.status, stdout: sh.stdout }; + const expected = { + code: sh.status, + stdout: sh.stdout, + stderr: sh.stderr, + }; let actual; try { const r = await $({ cwd: dirCs, mirror: false })`${{ raw: cmd }}`; - actual = { code: r.code, stdout: r.stdout }; + actual = { code: r.code, stdout: r.stdout, stderr: r.stderr }; } catch (e) { - actual = { code: e.code, stdout: e.stdout }; + actual = { code: e.code, stdout: e.stdout, stderr: e.stderr }; } const shFiles = execSync('ls -1', { cwd: dirSh, encoding: 'utf8' }).trim(); @@ -78,6 +85,8 @@ for (const cmd of CASES) { expected.code === actual.code && expected.stdout.replaceAll(dirSh, '') === actual.stdout.replaceAll(dirCs, '') && + expected.stderr.replaceAll(dirSh, '') === + actual.stderr.replaceAll(dirCs, '') && shFiles === csFiles && shOut === csOut; if (!same) { @@ -86,10 +95,10 @@ for (const cmd of CASES) { console.log(`${same ? 'OK ' : 'DIFF'} ${JSON.stringify(cmd)}`); if (!same) { console.log( - ` sh: code=${expected.code} stdout=${JSON.stringify(expected.stdout)} files=${JSON.stringify(shFiles)} contents=${JSON.stringify(shOut)}` + ` sh: code=${expected.code} stdout=${JSON.stringify(expected.stdout)} stderr=${JSON.stringify(expected.stderr)} files=${JSON.stringify(shFiles)} contents=${JSON.stringify(shOut)}` ); console.log( - ` cs: code=${actual.code} stdout=${JSON.stringify(actual.stdout)} files=${JSON.stringify(csFiles)} contents=${JSON.stringify(csOut)}` + ` cs: code=${actual.code} stdout=${JSON.stringify(actual.stdout)} stderr=${JSON.stringify(actual.stderr)} files=${JSON.stringify(csFiles)} contents=${JSON.stringify(csOut)}` ); } rmSync(dirSh, { recursive: true, force: true }); diff --git a/js/.changeset/issue-47-stderr-only-output.md b/js/.changeset/issue-47-stderr-only-output.md new file mode 100644 index 00000000..553cfd9e --- /dev/null +++ b/js/.changeset/issue-47-stderr-only-output.md @@ -0,0 +1,6 @@ +--- +'command-stream': patch +--- + +Guarantee that successful stderr-only CLI output remains separately captured, +including pull request URLs, while `2>&1` retains normal shell merge behavior. diff --git a/js/README.md b/js/README.md index 10cf5426..5bfdd6e6 100644 --- a/js/README.md +++ b/js/README.md @@ -1416,6 +1416,29 @@ console.log('Captured stderr:', result.stderr); // "Error!\n" console.log('Exit code:', result.code); // 0 ``` +### Successful CLI output on stderr + +Command-stream preserves the file descriptor chosen by the child process. A +successful command can therefore have an empty `stdout` and useful `stderr`. +For example, some CLI versions have printed a newly created pull request URL to +stderr: + +```javascript +const result = await $({ mirror: false })`gh pr create --fill`; +const prUrl = `${result.stdout}\n${result.stderr}`.match( + /^https:\/\/github\.com\/.*\/pull\/\d+$/m +)?.[0]; +``` + +When one combined stream is more convenient, use normal shell redirection. It +is opt-in because the shell-like default keeps stdout and stderr distinct: + +```javascript +const result = await $({ mirror: false })`gh pr create --fill 2>&1`; +console.log(result.stdout); // includes anything the command wrote to stderr +console.log(result.stderr); // empty after the redirection +``` + **Key Default Options:** - `mirror: true` - Live output to terminal (like shell) diff --git a/js/tests/competitor-compatibility.test.mjs b/js/tests/competitor-compatibility.test.mjs index 069543de..4755a3de 100644 --- a/js/tests/competitor-compatibility.test.mjs +++ b/js/tests/competitor-compatibility.test.mjs @@ -451,13 +451,29 @@ describe('ported public process behavior', () => { port( 'stdout-stderr-separation', - 'captures stdout and stderr independently', + 'captures stdout-only, stderr-only, and mixed output independently', async () => { - const result = await runFixture('stdio', ['out\n', 'err\n']); + const cases = [ + { stdout: 'out\n', stderr: 'err\n' }, + { stdout: 'out-only\n', stderr: '' }, + // A successful CLI may use stderr for machine-readable output. gh pr + // create was reported to do this for its URL in issue #47. + { + stdout: '', + stderr: 'https://github.com/octo/example/pull/123\n', + }, + ]; - expect(result.code).toBe(0); - expect(result.stdout).toBe('out\n'); - expect(result.stderr).toBe('err\n'); + for (const expected of cases) { + const result = await runFixture('stdio', [ + expected.stdout, + expected.stderr, + ]); + + expect(result.code).toBe(0); + expect(result.stdout).toBe(expected.stdout); + expect(result.stderr).toBe(expected.stderr); + } } ); diff --git a/js/tests/redirection-silent-failure.test.mjs b/js/tests/redirection-silent-failure.test.mjs index 4113a51d..49f3ac89 100644 --- a/js/tests/redirection-silent-failure.test.mjs +++ b/js/tests/redirection-silent-failure.test.mjs @@ -74,6 +74,11 @@ describe('Redirection is never handed to a built-in as an argument (issue #46)', 'cat /definitely/missing/path 2>/dev/null', 'ls /definitely/missing/path 2>/dev/null', 'ls /definitely/missing/path 2>&1', + // gh pr create was reported to emit its success URL on stderr (issue #47). + // Preserve the stream selected by the child/shell instead of losing or + // silently relabelling it. + 'echo https://github.com/octo/example/pull/123 >&2', + "sh -c 'echo https://github.com/octo/example/pull/123 >&2' 2>&1", 'exit 3 2>&1', 'echo a > out.txt && echo b >> out.txt', 'false > out.txt || echo fallback > out.txt', @@ -93,12 +98,21 @@ describe('Redirection is never handed to a built-in as an argument (issue #46)', cwd: csDir, mirror: false, })`${{ raw: command }}`; - actual = { code: result.code, stdout: result.stdout }; + actual = { + code: result.code, + stdout: result.stdout, + stderr: result.stderr, + }; } catch (error) { - actual = { code: error.code, stdout: error.stdout }; + actual = { + code: error.code, + stdout: error.stdout, + stderr: error.stderr, + }; } expect(actual.stdout).toBe(expected.stdout); + expect(actual.stderr).toBe(expected.stderr); expect(actual.code).toBe(expected.code); expect(await snapshot(csDir)).toEqual(await snapshot(shDir)); }); diff --git a/rust/README.md b/rust/README.md index bf93e912..3fa16823 100644 --- a/rust/README.md +++ b/rust/README.md @@ -44,6 +44,31 @@ async fn main() { } ``` +### Successful CLI output on stderr + +Command-stream preserves the file descriptor chosen by the child process. A +zero exit code can therefore accompany an empty `stdout` and useful `stderr`. +Check both when a CLI version may print a machine-readable result, such as a +new pull request URL, to stderr: + +```rust,no_run +use command_stream::run; + +# async fn example() -> Result<(), command_stream::Error> { +let result = run("gh pr create --fill").await?; +let pull_request_url = result + .stdout + .lines() + .chain(result.stderr.lines()) + .find(|line| line.starts_with("https://github.com/")); +# let _ = pull_request_url; +# Ok(()) +# } +``` + +Append `2>&1` to the command when normal shell stream merging is preferred. The +merged output is captured in `stdout`, while `stderr` is empty. + ## Streaming `StreamingRunner` streams output as it arrives and mirrors the JavaScript diff --git a/rust/changelog.d/20260915_124000_stderr_only_output.md b/rust/changelog.d/20260915_124000_stderr_only_output.md new file mode 100644 index 00000000..a4afaaac --- /dev/null +++ b/rust/changelog.d/20260915_124000_stderr_only_output.md @@ -0,0 +1,8 @@ +--- +bump: patch +--- + +### Fixed + +- Guarantee that successful stderr-only CLI output remains separately captured, + including pull request URLs, while `2>&1` retains normal shell merge behavior. diff --git a/rust/tests/competitor_compatibility/behavior.rs b/rust/tests/competitor_compatibility/behavior.rs index c2c78d80..389ab2b5 100644 --- a/rust/tests/competitor_compatibility/behavior.rs +++ b/rust/tests/competitor_compatibility/behavior.rs @@ -133,11 +133,19 @@ async fn environment_passes_explicit_values_to_the_child() { #[tokio::test] async fn stdout_and_stderr_are_captured_separately() { - let result = run_fixture("output", &["out-value", "err-value"]).await; + for (stdout, stderr) in [ + ("out-value", "err-value"), + ("out-only", ""), + // A successful CLI may use stderr for machine-readable output. gh pr + // create was reported to do this for its URL in issue #47. + ("", "https://github.com/octo/example/pull/123\n"), + ] { + let result = run_fixture("output", &[stdout, stderr]).await; - assert_eq!(result.code, 0); - assert_eq!(result.stdout, "out-value"); - assert_eq!(result.stderr, "err-value"); + assert_eq!(result.code, 0); + assert_eq!(result.stdout, stdout); + assert_eq!(result.stderr, stderr); + } } #[tokio::test] diff --git a/rust/tests/redirection_silent_failure.rs b/rust/tests/redirection_silent_failure.rs index 102e168b..7ca93994 100644 --- a/rust/tests/redirection_silent_failure.rs +++ b/rust/tests/redirection_silent_failure.rs @@ -14,8 +14,8 @@ use std::path::Path; use std::process::Command; use tempfile::TempDir; -/// Run `command` in `dir` through /bin/sh and return (exit code, stdout). -fn run_in_sh(command: &str, dir: &Path) -> (i32, String) { +/// Run `command` in `dir` through /bin/sh and return its captured result. +fn run_in_sh(command: &str, dir: &Path) -> (i32, String, String) { let output = Command::new("/bin/sh") .arg("-c") .arg(command) @@ -25,6 +25,7 @@ fn run_in_sh(command: &str, dir: &Path) -> (i32, String) { ( output.status.code().unwrap_or(-1), String::from_utf8_lossy(&output.stdout).into_owned(), + String::from_utf8_lossy(&output.stderr).into_owned(), ) } @@ -54,7 +55,7 @@ async fn assert_matches_sh(command: &str) { let sh_dir = scratch(); let cs_dir = scratch(); - let (expected_code, expected_stdout) = run_in_sh(command, sh_dir.path()); + let (expected_code, expected_stdout, expected_stderr) = run_in_sh(command, sh_dir.path()); let mut runner = ProcessRunner::new( command, @@ -71,6 +72,11 @@ async fn assert_matches_sh(command: &str) { "stdout mismatch for {:?}", command ); + assert_eq!( + result.stderr, expected_stderr, + "stderr mismatch for {:?}", + command + ); assert_eq!( result.code, expected_code, "exit code mismatch for {:?}", @@ -101,6 +107,10 @@ async fn redirection_on_virtual_commands_matches_sh() { "cat 0< seed.txt", "cat /definitely/missing/path 2>/dev/null", "ls /definitely/missing/path 2>&1", + // gh pr create was reported to emit its success URL on stderr (issue + // #47). Preserve that stream instead of dropping or relabelling it. + "echo https://github.com/octo/example/pull/123 >&2", + "sh -c 'echo https://github.com/octo/example/pull/123 >&2' 2>&1", "echo a > out.txt && echo b >> out.txt", "false > out.txt || echo fallback > out.txt", // Quoted redirection characters are literal in sh, so they must stay