Skip to content

Commit bc7e436

Browse files
committed
feat: add new dependencies for testing and enhance error reporting in package command
1 parent f380081 commit bc7e436

6 files changed

Lines changed: 136 additions & 16 deletions

File tree

Cargo.lock

Lines changed: 75 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ tauri-plugin-updater = "2"
6969
# JSON5 (for GUI log parsing compat)
7070
json5 = "1.3.1"
7171

72+
# Tests
73+
assert_cmd = "2.1.1"
74+
predicates = "3.1.3"
75+
7276
[profile.release]
7377
strip = true
7478
lto = true

mcp/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,7 @@ path = "src/main.rs"
1616
tnmsd = { workspace = true }
1717
clap = { workspace = true }
1818
serde_json = { workspace = true }
19+
20+
[dev-dependencies]
21+
assert_cmd = { workspace = true }
22+
predicates = { workspace = true }

mcp/src/commands/package.rs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::fs;
2+
use std::io::{self, Write};
23
use std::path::{Path, PathBuf};
34
use std::process::ExitCode;
45

@@ -39,19 +40,22 @@ const PACKAGE_TARGETS: &[PackageTarget] = &[
3940
];
4041

4142
pub fn execute(args: &AssembleNpmArgs) -> ExitCode {
43+
let mut stderr = io::stderr().lock();
44+
execute_with_stderr(args, &mut stderr)
45+
}
46+
47+
fn execute_with_stderr(args: &AssembleNpmArgs, stderr: &mut impl Write) -> ExitCode {
4248
match assemble_packages(args) {
4349
Ok(copied) => {
44-
// Use stderr: this binary's primary mode is the MCP stdio server,
45-
// and stdout is reserved for JSON-RPC framing. Routing all
46-
// assemble-npm chatter to stderr keeps stdout protocol-safe even
47-
// if the subcommand is ever invoked from a wrapped context.
50+
// Fixes #225: stdout is reserved for MCP JSON-RPC framing, so even
51+
// package-hydration status output must stay off stdout.
4852
for path in copied {
49-
eprintln!("Hydrated {}", path.display());
53+
let _ = writeln!(stderr, "Hydrated {}", path.display());
5054
}
5155
ExitCode::SUCCESS
5256
}
5357
Err(error) => {
54-
eprintln!("Error: {error}");
58+
let _ = writeln!(stderr, "Error: {error}");
5559
ExitCode::FAILURE
5660
}
5761
}
@@ -187,3 +191,26 @@ fn set_executable_permissions(path: &Path) -> Result<(), String> {
187191
fn set_executable_permissions(_path: &Path) -> Result<(), String> {
188192
Ok(())
189193
}
194+
195+
#[cfg(test)]
196+
mod tests {
197+
use super::*;
198+
199+
#[test]
200+
fn assemble_npm_reports_errors_to_stderr_only() {
201+
let mut stderr = Vec::new();
202+
let args = AssembleNpmArgs {
203+
artifacts_dir: None,
204+
profile: "missing-profile-for-issue-225".to_string(),
205+
};
206+
207+
let exit = execute_with_stderr(&args, &mut stderr);
208+
209+
assert_eq!(exit, ExitCode::FAILURE);
210+
let stderr = String::from_utf8(stderr).expect("stderr output should be UTF-8");
211+
assert!(
212+
stderr.contains("Error: Missing local host binary"),
213+
"expected assemble-npm errors to be written to stderr, got: {stderr}"
214+
);
215+
}
216+
}

mcp/src/main.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -391,23 +391,17 @@ fn main() -> ExitCode {
391391
);
392392

393393
let cli = Cli::parse();
394-
let logger = tnmsd::infra::logger::create_logger("tnmsm", None);
395394

396395
match resolve_command(&cli) {
397396
ResolvedCommand::Serve => {
398-
let _span = logger.span("server.serve").enter();
399-
logger.info(
400-
"MCP server started",
401-
Some(json!({
402-
"serverName": SERVER_NAME,
403-
"protocolVersion": PROTOCOL_VERSION,
404-
})),
405-
);
397+
// Fixes #225: in stdio mode stdout is the MCP JSON-RPC transport, so
398+
// server startup must not emit logger messages or spans there.
406399
run_stdio_server();
407400
ExitCode::SUCCESS
408401
}
409402
ResolvedCommand::AssembleNpm(args) => {
410-
let _span = logger.span("command.assemble_npm").enter();
403+
// Fixes #225: keep hidden packaging output off stdout as well; the
404+
// package command writes human-readable status to stderr internally.
411405
commands::package::execute(&args)
412406
}
413407
}

mcp/tests/protocol_stdout.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
use assert_cmd::Command;
2+
3+
#[test]
4+
fn assemble_npm_does_not_write_human_output_to_stdout() {
5+
// Fixes #225: stdout belongs to MCP JSON-RPC framing, so human-readable
6+
// package command failures must stay on stderr.
7+
Command::cargo_bin("tnmsm")
8+
.expect("tnmsm binary should be available")
9+
.args(["assemble-npm", "--profile", "missing-profile-for-issue-225"])
10+
.assert()
11+
.failure()
12+
.stdout("")
13+
.stderr(predicates::str::contains(
14+
"Error: Missing local host binary",
15+
));
16+
}

0 commit comments

Comments
 (0)