From 9199ed0b831470d2b558c782dbc2c06e010f6db9 Mon Sep 17 00:00:00 2001 From: Rajkumar TS Date: Thu, 9 Jul 2026 17:48:31 -0400 Subject: [PATCH 1/2] fix: write error JSON envelope to stdout, not stderr The CLI contract states stdout is always the JSON envelope (success or error); stderr is reserved for diagnostics. Agents parsing stdout saw empty output on failures because the error envelope was written to stderr. Always write the rendered output to stdout regardless of exit code. The exit code still communicates success/failure to the shell, and the envelope's error field communicates it to JSON consumers. Fixes: DEVEX-697 Co-authored-by: Cursor --- src/cli.rs | 12 ++++++------ tests/foundation.rs | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 8017b78..814f39c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -981,7 +981,7 @@ impl Cli { &self, args: I, stdout: &mut O, - stderr: &mut E, + _stderr: &mut E, shutdown: Shutdown, ) -> std::io::Result where @@ -999,11 +999,11 @@ impl Cli { { on_shutdown(); } - if output.exit_code == 0 { - stdout.write_all(output.rendered.as_bytes())?; - } else { - stderr.write_all(output.rendered.as_bytes())?; - } + // The JSON envelope always goes to stdout — success and error alike. + // The exit code communicates success/failure to the shell; the envelope's + // `error` field communicates it to JSON consumers. stderr is reserved for + // diagnostics only (tracing, update notices, signal messages). + stdout.write_all(output.rendered.as_bytes())?; Ok(process_exit_code(output.exit_code)) } diff --git a/tests/foundation.rs b/tests/foundation.rs index ea8e948..365880d 100644 --- a/tests/foundation.rs +++ b/tests/foundation.rs @@ -458,7 +458,7 @@ async fn cli_runtime_root_help_includes_find_commands_without_modules() { } #[tokio::test] -async fn cli_execute_from_writes_success_to_stdout_and_errors_to_stderr() { +async fn cli_execute_from_writes_all_output_to_stdout() { // execute_from publishes the config-derived User-Agent process-wide, so this // test shares the lock with the user-agent tests and restores the default // on exit (including panic) via the RAII guard, while the lock is held. @@ -502,13 +502,13 @@ async fn cli_execute_from_writes_success_to_stdout_and_errors_to_stderr() { .await .expect("execute should write"); assert_eq!(code, std::process::ExitCode::from(1)); - assert!(stdout.is_empty()); - let rendered = String::from_utf8(stderr).expect("utf8"); + assert!(stderr.is_empty()); + let rendered = String::from_utf8(stdout).expect("utf8"); assert!(rendered.contains("missing")); } #[tokio::test] -async fn cli_execute_from_shutdown_signal_writes_interrupt_to_stderr() { +async fn cli_execute_from_shutdown_signal_writes_interrupt_to_stdout() { // execute_from_until_signal publishes the config-derived User-Agent // process-wide; share the lock and restore the default (panic-safe) like above. let _ua_guard = USER_AGENT_TEST_LOCK.lock().await; @@ -540,9 +540,9 @@ async fn cli_execute_from_shutdown_signal_writes_interrupt_to_stderr() { .expect("execute should write interrupt output"); assert_eq!(code, std::process::ExitCode::from(130)); - assert!(stdout.is_empty()); + assert!(stderr.is_empty()); assert_eq!( - String::from_utf8(stderr).expect("stderr should be utf8"), + String::from_utf8(stdout).expect("stdout should be utf8"), "command interrupted\n" ); assert_eq!(shutdown_count.load(Ordering::SeqCst), 1); From 1f0ca86b08b5653469ef4c0b5ac406af26f9e695 Mon Sep 17 00:00:00 2001 From: Rajkumar TS Date: Thu, 9 Jul 2026 18:22:29 -0400 Subject: [PATCH 2/2] chore: add pull request template Standardizes PR descriptions with a Summary, Test plan, and Manual verification section that guides reviewers through testing with and without the change using a local cli-engine path override. Co-authored-by: Cursor --- .github/pull_request_template.md | 49 ++++++++++++++++++++++++++++++++ src/cli.rs | 4 --- 2 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..774e5ef --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,49 @@ +## Summary + +* + +## Test plan + +* `cargo fmt --all --check` +* `cargo clippy --all-targets -- -D warnings` +* `cargo test --all-targets` + +### Manual verification + + + +**Setup:** + +```bash +# In the cli repo, temporarily override the engine dependency: +cd cli/rust +# Edit Cargo.toml → cli-engine = { features = ["pkce-auth"], path = "../../cli-engine" } +cargo build --release && cp target/release/gddy ~/.local/bin/gddy +``` + +**Test WITHOUT the fix (baseline):** + +```bash +cd cli-engine && git checkout main +cd cli/rust && cargo build --release && cp target/release/gddy ~/.local/bin/gddy +# +# Expected: +``` + +**Test WITH the fix:** + +```bash +cd cli-engine && git checkout +cd cli/rust && cargo build --release && cp target/release/gddy ~/.local/bin/gddy +# +# Expected: +``` + +**Cleanup:** + +```bash +# Revert cli/rust/Cargo.toml back to: +# cli-engine = { features = ["pkce-auth"], version = "" } +``` diff --git a/src/cli.rs b/src/cli.rs index 814f39c..a556010 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -999,10 +999,6 @@ impl Cli { { on_shutdown(); } - // The JSON envelope always goes to stdout — success and error alike. - // The exit code communicates success/failure to the shell; the envelope's - // `error` field communicates it to JSON consumers. stderr is reserved for - // diagnostics only (tracing, update notices, signal messages). stdout.write_all(output.rendered.as_bytes())?; Ok(process_exit_code(output.exit_code)) }