Skip to content

Commit aedfc21

Browse files
authored
fix(cli): exit non-zero when feedback submission is rejected (#1766)
## Bug `mcpproxy feedback` exits 0 when the submission was not accepted. `SubmitFeedback` returns `Success: false` without an error in two cases: the backend answers 2xx with `{"success": false, "error": "..."}`, or it answers a non-JSON 3xx. `runFeedback` prints "Feedback submission failed: ..." (in text mode) and then returns `nil`. In `-o json` mode it prints the JSON and also returns `nil`. Scripts and CI wrappers therefore treat a rejected submission as sent. ## Trigger With the telemetry endpoint returning `200 {"success":false,"error":"rejected by backend"}`: ``` $ mcpproxy feedback "the search results could be more relevant" --category bug Feedback submission failed: rejected by backend $ echo $? 0 ``` ## Fix After rendering the output, return `feedback submission failed: <error>` when `resp.Success` is false, so the exit code is non-zero in both text and JSON modes. The text branch no longer prints its own failure line, because cobra prints the returned error. The success path is unchanged. ## Test `TestRunFeedback_RejectedSubmissionReturnsError` points the telemetry endpoint at an `httptest` server that returns `{"success":false}`, and asserts that `runFeedback` returns an error. ## Commands run - `go vet ./cmd/mcpproxy`: clean - `go test -count=1 ./cmd/mcpproxy`: ok (needs cgo; run with `SDKROOT` set to the macOS 26.5 SDK locally) - Without the fix, the new test fails with `runFeedback returned nil for a submission the backend rejected`. - `gofmt -l` on the edited files: no output
1 parent 3b86b9d commit aedfc21

2 files changed

Lines changed: 38 additions & 2 deletions

File tree

‎cmd/mcpproxy/feedback_cmd.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,12 @@ func runFeedback(cmd *cobra.Command, args []string) error {
9191
if resp.IssueURL != "" {
9292
fmt.Printf("Track your feedback: %s\n", resp.IssueURL)
9393
}
94-
} else {
95-
fmt.Printf("Feedback submission failed: %s\n", resp.Error)
9694
}
9795
}
9896

97+
if !resp.Success {
98+
return fmt.Errorf("feedback submission failed: %s", resp.Error)
99+
}
99100
return nil
100101
}
101102

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/http/httptest"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
)
11+
12+
// A backend that answers 2xx with {"success":false} rejected the feedback;
13+
// the command must exit non-zero so scripts don't treat it as submitted.
14+
func TestRunFeedback_RejectedSubmissionReturnsError(t *testing.T) {
15+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
16+
w.Header().Set("Content-Type", "application/json")
17+
_, _ = w.Write([]byte(`{"success":false,"error":"rejected by backend"}`))
18+
}))
19+
defer srv.Close()
20+
21+
dir := t.TempDir()
22+
cfgPath := filepath.Join(dir, "mcp_config.json")
23+
cfgJSON := fmt.Sprintf(`{"data_dir": %q, "telemetry": {"endpoint": %q}}`, dir, srv.URL)
24+
if err := os.WriteFile(cfgPath, []byte(cfgJSON), 0o600); err != nil {
25+
t.Fatal(err)
26+
}
27+
28+
origConfig, origCategory := configFile, feedbackCategory
29+
t.Cleanup(func() { configFile, feedbackCategory = origConfig, origCategory })
30+
configFile, feedbackCategory = cfgPath, "bug"
31+
32+
if err := runFeedback(nil, []string{"the search results could be more relevant"}); err == nil {
33+
t.Fatal("runFeedback returned nil for a submission the backend rejected")
34+
}
35+
}

0 commit comments

Comments
 (0)