From 52b9895dd7b38ca538d6c6e8c0f7122196b0d5bb Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Wed, 13 May 2026 15:39:06 -0700 Subject: [PATCH 1/3] Don't print response in `-o hex --json` --- crates/icp-cli/src/commands/canister/call.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index 97fc4c0d..b3c49f2a 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -277,7 +277,9 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E } } CallOutputMode::Hex => { - writeln!(term, "{}", hex::encode(&res))?; + if !args.json { + writeln!(term, "{}", hex::encode(&res))?; + } } }; anyhow::Ok(()) From eecb012ce4cdfd5a81b52423edb6ffca4edffd58 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Wed, 13 May 2026 15:43:37 -0700 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a199d9ea..da294325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +* fix: `icp canister call` with both `--json` and `-o hex` no longer prints both kinds of output at once. * fix: `icp` no longer picks up a stale inherited `$PWD` when launched as a subprocess via `chdir(2)` + `execve` (e.g. from a test harness). The logical `$PWD` path is now validated against `getcwd()` by inode before use, preserving symlink-aware project root discovery while ignoring stale values. # v0.2.6 From 965644cd6a91bd49d624a181f26f126fc0087376 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 14 May 2026 09:43:27 -0400 Subject: [PATCH 3/3] refactor(canister call): separate response decoding from emission (#554) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each output-mode arm previously forked on `--json`, duplicating the "decode and stash vs. decode and print" logic. The recent `-o hex --json` double-printing bug was a direct consequence — easy to forget one of the `if !args.json` guards. Split the flow into a `decode_response` step that returns a `Decoded` enum (Candid / Text / Bytes) and a single emission step that forks on `--json` exactly once. Semantics are unchanged: `-o` still controls decoding strictness, `--json` still controls envelope vs. human output, and the two remain orthogonal. The write-vs-decode error reconciliation in JSON mode is preserved. Co-authored-by: Claude Opus 4.7 (1M context) --- crates/icp-cli/src/commands/canister/call.rs | 130 ++++++++++--------- 1 file changed, 70 insertions(+), 60 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index b3c49f2a..d810344e 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -228,80 +228,74 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E }; let mut term = Term::buffered_stdout(); - let res_hex = || format!("response (hex): {}", hex::encode(&res)); - let mut json_response = JsonCallResponse { - response_bytes: hex::encode(&res), - response_text: None, - response_candid: None, - }; + let decoded = decode_response(&res, args.output, declared_method.as_ref()); - // catch errors, because the json result should be printed regardless of errors - let res: Result<(), anyhow::Error> = (|| { - match args.output { - CallOutputMode::Auto => { - if let Ok(ret) = try_decode_candid(&res, declared_method.as_ref()) { - if args.json { - json_response.response_candid = Some(format!("{ret}")); - } else { - print_candid_for_term(&mut term, &ret) - .context("failed to print candid return value")?; - } - } else if let Ok(s) = std::str::from_utf8(&res) { - if args.json { - json_response.response_text = Some(s.to_string()); - } else { - writeln!(term, "{s}")?; - } - } else if !args.json { - writeln!(term, "{}", hex::encode(&res))?; - } - } - CallOutputMode::Candid => { - let ret = - try_decode_candid(&res, declared_method.as_ref()).with_context(res_hex)?; - if args.json { - json_response.response_candid = Some(format!("{ret}")); - } else { - print_candid_for_term(&mut term, &ret) - .context("failed to print candid return value")?; - } - } - CallOutputMode::Text => { - let s = std::str::from_utf8(&res) - .with_context(res_hex) - .context("response is not valid UTF-8")?; - if args.json { - json_response.response_text = Some(s.to_string()); - } else { - writeln!(term, "{s}")?; - } - } - CallOutputMode::Hex => { - if !args.json { - writeln!(term, "{}", hex::encode(&res))?; - } - } - }; - anyhow::Ok(()) - })(); if args.json { - let write_result = serde_json::to_writer(&term, &json_response); - if let Err(write_err) = write_result { - if let Err(decode_err) = res { + let envelope = JsonCallResponse::build(&res, decoded.as_ref().ok()); + let write_result = serde_json::to_writer(&term, &envelope); + match (write_result, decoded) { + (Ok(()), decode_result) => { + decode_result?; + } + (Err(write_err), Err(decode_err)) => { + // Prefer the decode error; the write failure is incidental. error!("failed to write JSON response: {write_err}"); return Err(decode_err); - } else { + } + (Err(write_err), Ok(_)) => { return Err(write_err).context("failed to write JSON response"); } } + } else { + match decoded? { + Decoded::Candid(ret) => print_candid_for_term(&mut term, &ret) + .context("failed to print candid return value")?, + Decoded::Text(s) => writeln!(term, "{s}")?, + Decoded::Bytes => writeln!(term, "{}", hex::encode(&res))?, + } } - res?; + // term is buffered; this single flush covers all output paths (json and non-json). term.flush()?; Ok(()) } +/// A response decoded according to the requested `CallOutputMode`. +enum Decoded { + Candid(IDLArgs), + Text(String), + /// No decoding was attempted or all attempts failed; emit raw bytes as hex. + Bytes, +} + +fn decode_response( + res: &[u8], + mode: CallOutputMode, + method: Option<&(TypeEnv, Function)>, +) -> Result { + let res_hex = || format!("response (hex): {}", hex::encode(res)); + match mode { + CallOutputMode::Auto => { + if let Ok(args) = try_decode_candid(res, method) { + Ok(Decoded::Candid(args)) + } else if let Ok(s) = std::str::from_utf8(res) { + Ok(Decoded::Text(s.to_string())) + } else { + Ok(Decoded::Bytes) + } + } + CallOutputMode::Candid => try_decode_candid(res, method) + .map(Decoded::Candid) + .with_context(res_hex), + CallOutputMode::Text => std::str::from_utf8(res) + .map(|s| Decoded::Text(s.to_string())) + .with_context(res_hex) + .context("response is not valid UTF-8"), + CallOutputMode::Hex => Ok(Decoded::Bytes), + } +} + #[derive(Serialize)] struct JsonCallResponse { response_bytes: String, @@ -309,6 +303,22 @@ struct JsonCallResponse { response_candid: Option, } +impl JsonCallResponse { + fn build(res: &[u8], decoded: Option<&Decoded>) -> Self { + Self { + response_bytes: hex::encode(res), + response_text: match decoded { + Some(Decoded::Text(s)) => Some(s.clone()), + _ => None, + }, + response_candid: match decoded { + Some(Decoded::Candid(args)) => Some(format!("{args}")), + _ => None, + }, + } + } +} + /// Tries to decode the response as Candid. Returns `None` if decoding fails. fn try_decode_candid( res: &[u8],