fix(cli): preserve MCP call results and structured errors

cmd/mcpproxy/call_cmd.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -288,6 +289,17 @@ func outputCallResultPretty(result interface{}) {
 	fmt.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n\n")
 }
 
+func outputPrettyCallResult(result interface{}, isError bool) error {
+	if isError {
+		fmt.Fprintln(os.Stderr, "⚠️  Tool call returned an error.")
+		outputCallResultPretty(result)
+		return callResultError{}
+	}
+	fmt.Fprintf(os.Stderr, "✅ Tool call completed successfully!\n\n")
+	outputCallResultPretty(result)
+	return nil
+}
+
 // createLogger creates a zap logger with the specified level
 func createLogger(level string) (*zap.Logger, error) {
 	var zapLevel zap.AtomicLevel
@@ -317,22 +329,33 @@ func createLogger(level string) (*zap.Logger, error) {
 }
 
 // runCallToolRead handles the tool-read command (Spec 018)
-func runCallToolRead(_ *cobra.Command, _ []string) error {
-	return runCallToolVariant("call_tool_read", "read")
+func runCallToolRead(cmd *cobra.Command, _ []string) error {
+	return runCallToolVariant(cmd, "call_tool_read", "read")
 }
 
 // runCallToolWrite handles the tool-write command (Spec 018)
-func runCallToolWrite(_ *cobra.Command, _ []string) error {
-	return runCallToolVariant("call_tool_write", "write")
+func runCallToolWrite(cmd *cobra.Command, _ []string) error {
+	return runCallToolVariant(cmd, "call_tool_write", "write")
 }
 
 // runCallToolDestructive handles the tool-destructive command (Spec 018)
-func runCallToolDestructive(_ *cobra.Command, _ []string) error {
-	return runCallToolVariant("call_tool_destructive", "destructive")
+func runCallToolDestructive(cmd *cobra.Command, _ []string) error {
+	return runCallToolVariant(cmd, "call_tool_destructive", "destructive")
 }
 
 // runCallToolVariant is the common implementation for intent-based tool calls
-func runCallToolVariant(toolVariant, operationType string) error {
+func runCallToolVariant(cmd *cobra.Command, toolVariant, operationType string) (callErr error) {
+	// Cobra validates required flags before RunE. Runtime failures in JSON mode
+	// get one machine-readable document on stdout and retain a nonzero exit.
+	if callOutputFormat == outputFormatJSON {
+		cmd.SilenceUsage = true
+		defer func() {
+			var resultErr callResultError
+			if callErr != nil && !errors.As(callErr, &resultErr) {
+				writeCallJSONError(callErr)
+			}
+		}()
+	}
 	ctx, cancel := context.WithTimeout(context.Background(), callTimeout)
 	defer cancel()
 
@@ -391,6 +414,29 @@ func runCallToolVariant(toolVariant, operationType string) error {
 	return runCallToolVariantStandalone(ctx, toolVariant, variantArgs, globalConfig)
 }
 
+// callResultError retains the nonzero process status after a complete MCP
+// isError result has already been written to stdout.
+type callResultError struct{}
+
+func (callResultError) Error() string { return "tool returned an error result" }
+
+type callDaemonError struct{ cause error }
+
+func (e callDaemonError) Error() string {
+	return "failed to call tool via daemon: " + formatErrorWithRequestID(e.cause)
+}
+
+func (e callDaemonError) Unwrap() error { return e.cause }
+
+func writeCallJSONError(err error) {
+	structured := output.NewStructuredError(output.ErrCodeOperationFailed, err.Error())
+	var apiErr *cliclient.APIError
+	if errors.As(err, &apiErr) && apiErr.RequestID != "" {
+		structured = structured.WithRequestID(apiErr.RequestID)
+	}
+	_ = outputCallResultAsJSON(structured)
+}
+
 // runCallToolVariantClientMode calls tool variant via the daemon HTTP API
 func runCallToolVariantClientMode(client *cliclient.Client, toolVariant string, args map[string]interface{}, logger *zap.Logger) error {
 	// Ping daemon to verify connectivity
@@ -416,22 +462,25 @@ func runCallToolVariantClientMode(client *cliclient.Client, toolVariant string,
 	// For tool variants, we call the variant tool directly
 	result, err := client.CallTool(callCtx, toolVariant, args)
 	if err != nil {
-		// T028: Use cliError to include request_id in error output
-		return cliError("failed to call tool via daemon", err)
+		// Keep the API error wrapped so JSON output can retain request_id.
+		return callDaemonError{cause: err}
 	}
 
 	// Output results based on format
 	switch callOutputFormat {
 	case outputFormatJSON:
-		return outputCallResultAsJSON(result)
+		if err := outputCallResultAsJSON(result); err != nil {
+			return err
+		}
+		if result.IsError {
+			return callResultError{}
+		}
+		return nil
 	case outputFormatPretty, "":
 		fallthrough
 	default:
-		fmt.Fprintf(os.Stderr, "✅ Tool call completed successfully!\n\n")
-		outputCallResultPretty(result)
+		return outputPrettyCallResult(result, result.IsError)
 	}
-
-	return nil
 }
 
 // runCallToolVariantStandalone calls tool variant directly without daemon
@@ -499,13 +548,16 @@ func runCallToolVariantStandalone(ctx context.Context, toolVariant string, args
 	// Output results based on format
 	switch callOutputFormat {
 	case outputFormatJSON:
-		return outputCallResultAsJSON(result)
+		if err := outputCallResultAsJSON(result); err != nil {
+			return err
+		}
+		if result.IsError {
+			return callResultError{}
+		}
+		return nil
 	case outputFormatPretty, "":
 		fallthrough
 	default:
-		fmt.Fprintf(os.Stderr, "✅ Tool call completed successfully!\n\n")
-		outputCallResultPretty(result)
+		return outputPrettyCallResult(result, result.IsError)
 	}
-
-	return nil
 }

cmd/mcpproxy/call_result_json_test.go

@@ -0,0 +1,143 @@
+package main
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/mark3labs/mcp-go/mcp"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/cli/output"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func TestCallToolJSONFailureIsParseableAndNonzero(t *testing.T) {
+	oldFormat, oldArgs := callOutputFormat, callJSONArgs
+	t.Cleanup(func() { callOutputFormat, callJSONArgs = oldFormat, oldArgs })
+	callOutputFormat, callJSONArgs = "json", "{invalid"
+	reader, writer, err := os.Pipe()
+	require.NoError(t, err)
+	oldStdout := os.Stdout
+	os.Stdout = writer
+	t.Cleanup(func() { os.Stdout = oldStdout })
+
+	cmd := &cobra.Command{}
+	callErr := runCallToolVariant(cmd, "call_tool_read", "read")
+	require.Error(t, callErr)
+	require.True(t, cmd.SilenceUsage)
+	require.NoError(t, writer.Close())
+	bytes, err := io.ReadAll(reader)
+	require.NoError(t, err)
+	var got output.StructuredError
+	require.NoError(t, json.Unmarshal(bytes, &got))
+	require.Equal(t, output.ErrCodeOperationFailed, got.Code)
+	require.Contains(t, got.Message, "invalid JSON arguments")
+}
+
+func TestCallToolDaemonJSONRetainsResultAndErrorStatus(t *testing.T) {
+	oldFormat := callOutputFormat
+	callOutputFormat = "json"
+	t.Cleanup(func() { callOutputFormat = oldFormat })
+	for _, tc := range []struct {
+		name, data string
+		wantError  bool
+	}{
+		{"success", `{"content":[{"type":"text","text":"{\"ok\":true}"},{"type":"image","data":"abc","mimeType":"image/png"}],"structuredContent":{"ok":true},"_meta":{"trace":"test"}}`, false},
+		{"tool error", `{"content":[{"type":"text","text":"denied"}],"isError":true}`, true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path == "/api/v1/status" {
+					_, _ = w.Write([]byte(`{"status":"ok"}`))
+					return
+				}
+				require.Equal(t, "/api/v1/tools/call", r.URL.Path)
+				_, _ = w.Write([]byte(`{"success":true,"data":` + tc.data + `}`))
+			}))
+			defer srv.Close()
+			reader, writer, err := os.Pipe()
+			require.NoError(t, err)
+			oldStdout := os.Stdout
+			os.Stdout = writer
+			defer func() { os.Stdout = oldStdout }()
+			err = runCallToolVariantClientMode(cliclient.NewClient(srv.URL, nil), "call_tool_read", map[string]interface{}{"name": "fixture:read"}, zap.NewNop())
+			if tc.wantError {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+			require.NoError(t, writer.Close())
+			actual, err := io.ReadAll(reader)
+			require.NoError(t, err)
+			var got map[string]interface{}
+			require.NoError(t, json.Unmarshal(actual, &got))
+			var want map[string]interface{}
+			require.NoError(t, json.Unmarshal([]byte(tc.data), &want))
+			require.Equal(t, want["content"], got["content"])
+			require.Equal(t, want["structuredContent"], got["structuredContent"])
+			require.Equal(t, want["_meta"], got["_meta"])
+			require.Equal(t, tc.wantError, got["isError"])
+		})
+	}
+}
+
+func TestCallToolJSONErrorRetainsRequestID(t *testing.T) {
+	reader, writer, err := os.Pipe()
+	require.NoError(t, err)
+	oldStdout := os.Stdout
+	os.Stdout = writer
+	defer func() { os.Stdout = oldStdout }()
+	writeCallJSONError(callDaemonError{cause: &cliclient.APIError{Message: "missing server", RequestID: "req-123"}})
+	require.NoError(t, writer.Close())
+	bytes, err := io.ReadAll(reader)
+	require.NoError(t, err)
+	var got output.StructuredError
+	require.NoError(t, json.Unmarshal(bytes, &got))
+	require.Equal(t, "req-123", got.RequestID)
+	require.Equal(t, output.ErrCodeOperationFailed, got.Code)
+}
+
+func TestCallToolDaemonPrettyErrorIsNonzeroWithoutSuccessMessage(t *testing.T) {
+	oldFormat := callOutputFormat
+	callOutputFormat = "pretty"
+	defer func() { callOutputFormat = oldFormat }()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/api/v1/status" {
+			_, _ = w.Write([]byte(`{"status":"ok"}`))
+			return
+		}
+		_, _ = w.Write([]byte(`{"success":true,"data":{"content":[{"type":"text","text":"fixture denied"}],"isError":true}}`))
+	}))
+	defer srv.Close()
+	stdoutReader, stdoutWriter, err := os.Pipe()
+	require.NoError(t, err)
+	stderrReader, stderrWriter, err := os.Pipe()
+	require.NoError(t, err)
+	oldStdout, oldStderr := os.Stdout, os.Stderr
+	os.Stdout, os.Stderr = stdoutWriter, stderrWriter
+	defer func() { os.Stdout, os.Stderr = oldStdout, oldStderr }()
+
+	err = runCallToolVariantClientMode(cliclient.NewClient(srv.URL, nil), "call_tool_read", map[string]interface{}{"name": "fixture:fail"}, zap.NewNop())
+	require.ErrorAs(t, err, new(callResultError))
+	require.NoError(t, stdoutWriter.Close())
+	require.NoError(t, stderrWriter.Close())
+	stdout, err := io.ReadAll(stdoutReader)
+	require.NoError(t, err)
+	stderr, err := io.ReadAll(stderrReader)
+	require.NoError(t, err)
+	require.True(t, strings.Contains(string(stdout), "fixture denied"))
+	require.NotContains(t, string(stderr), "completed successfully")
+	require.Contains(t, string(stderr), "returned an error")
+}
+
+func TestStandalonePrettyResultErrorIsNonzero(t *testing.T) {
+	result := &mcp.CallToolResult{Content: []mcp.Content{mcp.NewTextContent("standalone denied")}, IsError: true}
+	err := outputPrettyCallResult(result, result.IsError)
+	require.ErrorAs(t, err, new(callResultError))
+}

docs/cli-output-formatting.md

@@ -144,6 +144,20 @@ This enables AI agents to discover available commands and their options programm
 
 ## Structured Error Output
 
+For `mcpproxy call tool-read`, `tool-write`, and `tool-destructive -o json`,
+stdout contains the MCP result envelope. The CLI sends
+`X-MCPProxy-Result-Envelope: 1` to opt into this REST response shape; calls
+without that header retain their content-array response. `content` retains its ordered text,
+image, audio, or resource blocks; `structuredContent` and `_meta` are included
+when supplied by the daemon. Older daemons that return a content array are
+normalized to `{ "content": [...], "isError": false }`. An `isError` result
+keeps that envelope on stdout and exits nonzero.
+
+Runtime tool-call failures emit one structured error object on stdout with
+`code: "OPERATION_FAILED"`, a `message`, and `request_id` when available, and
+exit nonzero. Diagnostics may also appear on stderr. Required-flag validation
+still follows Cobra's usual usage output.
+
 When using JSON output format, errors include structured information:
 
 ```json

internal/cliclient/call_tool_test.go

@@ -0,0 +1,38 @@
+package cliclient_test
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCallToolPreservesResultShapes(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		data string
+		want string
+	}{
+		{"legacy content array", `[{"type":"text","text":"{\"ok\":true}"},{"type":"image","data":"abc","mimeType":"image/png"}]`, `{"content":[{"type":"text","text":"{\"ok\":true}"},{"type":"image","data":"abc","mimeType":"image/png"}],"isError":false}`},
+		{"full result", `{"content":[{"type":"text","text":"hello"}],"structuredContent":{"count":2},"isError":true,"_meta":{"source":"test"}}`, `{"content":[{"type":"text","text":"hello"}],"structuredContent":{"count":2},"isError":true,"_meta":{"source":"test"}}`},
+		{"structured only", `{"structuredContent":{"count":2}}`, `{"content":[],"structuredContent":{"count":2},"isError":false}`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				require.Equal(t, "/api/v1/tools/call", r.URL.Path)
+				require.Equal(t, "1", r.Header.Get("X-MCPProxy-Result-Envelope"))
+				_, _ = w.Write([]byte(`{"success":true,"data":` + tc.data + `}`))
+			}))
+			defer srv.Close()
+			got, err := cliclient.NewClient(srv.URL, nil).CallTool(context.Background(), "call_tool_read", nil)
+			require.NoError(t, err)
+			actual, err := json.Marshal(got)
+			require.NoError(t, err)
+			require.JSONEq(t, tc.want, string(actual))
+		})
+	}
+}