sop-mcp-server: help, version, usage errors, and JSON output

[gerardrecinto]

Running the binary with anything it did not recognize did nothing visible. sop-mcp-server --help, --version, version and a mistyped word all started the stdio server, which exits silently when stdin closes and otherwise just sits waiting. Nothing told the user what the commands were.

help, -h and --help print the commands, the two environment variables and the exit codes. version, -v and --version print one line with the module version the binary was built from, which is the release tag for a published binary or go install, and dev for a build from a working copy. An unknown word is now a usage error that names it and exits 2 instead of starting a server. stdio and serve are accepted as explicit names for starting the server, and no arguments still starts it, so how agents launch it does not change. setup --help is no longer reported as a failure.

check --json and demo --json print one JSON document, for scripts. A runbook that does not load is still reported as JSON with valid false and the error, with exit 1, so a script can read the reason. The flag works before or after the file name. Exit codes are 0 for success, 1 for failure and 2 for a usage error everywhere.

Tests cover each of those. The package vets for Windows on amd64 and arm64 and for Linux and macOS on arm64.

Thanks, Gerard Recinto

[github-actions]

Gemini PR Review

Reviewed commit: 632171dc4ae341c6115b4efdf517748f4ddf75bd
Verdict: FAIL

• cmd/sop-mcp-server/check.go, cmd/sop-mcp-server/demo.go: In the writeJSON function, the error returned by json.Encoder.Encode is ignored. If the io.Writer (e.g., os.Stdout when piped to another command that closes its input early) encounters an error during writing, the program will not detect or report this output failure, leading to a silent failure to produce complete or correct output.

This result blocks merge. Push a fix and comment /gemini review. To contest a finding, see the Gemini dispute process in CONTRIBUTING.md.

[gerardrecinto]

/gemini review

[github-actions]

Gemini PR Review

Reviewed commit: 632171dc4ae341c6115b4efdf517748f4ddf75bd
Verdict: FAIL

• cmd/sop-mcp-server/check.go: The writeJSON function ignores the error returned by json.NewEncoder(w).Encode(v). While writing to os.Stdout often doesn't fail, any io.Writer can encounter errors (e.g., a closed pipe, disk full if writing to file), and ignoring these can hide potential issues and lead to silent data loss or incomplete output.
• cmd/sop-mcp-server/demo.go: The runDemo function's error reporting has been weakened. Previously, runDemo (or the demoMain it called) would print errors from the demo logic to os.Stdout. With the refactoring, runDemo now calls runDemoArgs(nil, out, io.Discard), meaning any errors encountered during playDemo and reported via errw in runDemoArgs will be silently discarded to io.Discard instead of being written to the provided out writer or os.Stderr. This changes the expected behavior of runDemo regarding error visibility.

This result blocks merge. Push a fix and comment /gemini review. To contest a finding, see the Gemini dispute process in CONTRIBUTING.md.

[gerardrecinto]

/gemini review

[github-actions]

Gemini PR Review

Reviewed commit: 632171dc4ae341c6115b4efdf517748f4ddf75bd
Verdict: FAIL

• Correctness Bug: Ignored json.Encoder.Encode errors.
  In cmd/sop-mcp-server/check.go, the writeJSON function uses _ = enc.Encode(v) to suppress any error returned by the json.Encoder.Encode method. If an error occurs during JSON encoding or writing to the io.Writer (e.g., if the underlying writer fails, such as a broken pipe or a full disk), the program will not report this failure, potentially leading to a misleading 0 exit code despite the output being incomplete or absent.

This result blocks merge. Push a fix and comment /gemini review. To contest a finding, see the Gemini dispute process in CONTRIBUTING.md.

[gerardrecinto]

Both findings fixed. writeJSON now returns the encode error, and check --json and demo --json report it on stderr and exit 1, so a closed pipe is a failed command and not truncated output; a test writes to a broken writer and checks both. The runDemo wrapper that discarded errors is removed, and the demo test calls runDemoArgs with the error stream attached.

Thanks, Gerard Recinto

/gemini review

[github-actions]

Gemini PR Review

Reviewed commit: d1be21beb9fc4b5b59197089d2897cd178d4f43d
Verdict: PASS

• cmd/sop-mcp-server/check.go: The emitJSON function ensures that failures to write JSON output (e.g., due to a broken pipe or full disk) are correctly reported to errw and result in a non-zero exit code (1), preventing silent truncation or incorrect success signals. This improves the robustness of the command-line interface for scripts.
• cmd/sop-mcp-server/demo_test.go: The new brokenWriter and TestAFailedJSONWriteIsAFailedCommand specifically test the error handling of JSON output writes for both check and demo commands, verifying that I/O failures are caught and reported correctly. This is a valuable addition for robustness.
• cmd/sop-mcp-server/setup.go: The explicit check if errors.Is(err, flag.ErrHelp) { return 0 } correctly handles the standard flag package behavior for --help flags, ensuring that setup --help exits with code 0 (success) rather than code 2 (usage error). This aligns with common command-line tool expectations.

[gerardrecinto]

/gemini review

[github-actions]

Gemini PR Review

Reviewed commit: d1be21beb9fc4b5b59197089d2897cd178d4f43d
Verdict: PASS

• Correctness Improvement: cmd/sop-mcp-server/check.go and cmd/sop-mcp-server/demo.go now correctly handle io.Writer errors when emitting JSON output. If the output stream (e.g., a pipe) is closed or unavailable, the program will report the error and exit with a non-zero status, preventing silent truncation or loss of output. This is a significant improvement in robustness.
• Correctness Bug Fix: cmd/sop-mcp-server/setup.go now correctly handles the flag.ErrHelp error returned by flag.Parse(), causing the setup subcommand to exit with a success code (0) when --help is used, rather than a usage error (2).