doc/changes/client-robustness-fixes.md

@@ -0,0 +1,30 @@
+# Client Robustness Fixes
+
+## Summary
+
+This change improves shared client/context behavior used by all shortcuts and raw API calls:
+
+- send `Accept: application/json` on every request
+- send `Content-Type: application/json` when a JSON body is present
+- decode GitLink responses whose `data` field is itself a JSON string into structured data
+- avoid mutating caller-provided pagination query parameters in `PaginateAll`
+- normalize remote URL path segments more defensively when resolving `owner/repo`
+
+## Why
+
+These fixes make CLI behavior more predictable for both human users and Agents:
+
+- API gateways can correctly classify request and response formats.
+- JSON-string `data` payloads become usable structured output instead of opaque strings.
+- Pagination helpers no longer leak `page`/default `limit` mutations back to callers.
+- Repository auto-detection works better with remote URLs that include redundant slashes or trailing path separators.
+
+## Validation
+
+```bash
+git diff --check
+GOPROXY=https://goproxy.cn,direct go test ./internal/client ./internal/context
+go vet ./internal/client ./internal/context
+GOPROXY=https://goproxy.cn,direct go test ./...
+go vet ./...
+```

internal/client/client.go

@@ -78,6 +78,10 @@ func (c *Client) Do(method, path string, body interface{}, query url.Values) (*o
 	if err != nil {
 		return nil, err
 	}
+	req.Header.Set("Accept", "application/json")
+	if bodyReader != nil {
+		req.Header.Set("Content-Type", "application/json")
+	}
 
 	if c.Debug {
 		fmt.Printf("→ %s %s\n", method, fullURL)
@@ -151,7 +155,7 @@ func (c *Client) Do(method, path string, body interface{}, query url.Values) (*o
 	if dataStr, ok := raw["data"].(string); ok {
 		var parsedData interface{}
 		if err := json.Unmarshal([]byte(dataStr), &parsedData); err == nil {
-			raw["data"] = json.RawMessage(dataStr)
+			raw["data"] = parsedData
 		}
 	}
 

internal/client/client_test.go

@@ -226,6 +226,35 @@ func TestClientDoStatusZero(t *testing.T) {
 	}
 }
 
+func TestClientDoParsesJSONStringData(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"status":0,"data":"[{\"id\":1,\"name\":\"demo\"}]"}`))
+	}))
+	defer server.Close()
+
+	c := &Client{HTTP: server.Client(), BaseURL: server.URL}
+	env, err := c.Do("GET", "/api/test", nil, nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	raw, ok := env.Data.(map[string]interface{})
+	if !ok {
+		t.Fatalf("Data = %T, want map[string]interface{}", env.Data)
+	}
+	items, ok := raw["data"].([]interface{})
+	if !ok {
+		t.Fatalf("nested data = %T, want []interface{}", raw["data"])
+	}
+	if len(items) != 1 {
+		t.Fatalf("expected one parsed item, got %d", len(items))
+	}
+	item, ok := items[0].(map[string]interface{})
+	if !ok || item["name"] != "demo" {
+		t.Fatalf("unexpected parsed item: %#v", items[0])
+	}
+}
+
 func TestClientDoPaginationMeta(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
@@ -297,6 +326,12 @@ func TestClientDoWithBody(t *testing.T) {
 		if r.Method != "POST" {
 			t.Fatalf("expected POST, got %s", r.Method)
 		}
+		if got := r.Header.Get("Accept"); got != "application/json" {
+			t.Fatalf("Accept = %q, want application/json", got)
+		}
+		if got := r.Header.Get("Content-Type"); got != "application/json" {
+			t.Fatalf("Content-Type = %q, want application/json", got)
+		}
 		w.Header().Set("Content-Type", "application/json")
 		w.Write([]byte(`{"id":123}`))
 	}))
@@ -312,6 +347,25 @@ func TestClientDoWithBody(t *testing.T) {
 	}
 }
 
+func TestClientDoWithoutBodySetsAcceptOnly(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if got := r.Header.Get("Accept"); got != "application/json" {
+			t.Fatalf("Accept = %q, want application/json", got)
+		}
+		if got := r.Header.Get("Content-Type"); got != "" {
+			t.Fatalf("Content-Type = %q, want empty", got)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{}`))
+	}))
+	defer server.Close()
+
+	c := &Client{HTTP: server.Client(), BaseURL: server.URL}
+	if _, err := c.Do("GET", "/api/test", nil, nil); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
 func TestClientGet(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != "GET" {
@@ -498,6 +552,30 @@ func TestPaginateAllWrappedData(t *testing.T) {
 	}
 }
 
+func TestPaginateAllDoesNotMutateParams(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"data":[{"id":1}]}`))
+	}))
+	defer server.Close()
+
+	params := url.Values{}
+	params.Set("state", "open")
+	c := &Client{HTTP: server.Client(), BaseURL: server.URL}
+	if _, err := c.PaginateAll("/test", params); err != nil {
+		t.Fatalf("PaginateAll error: %v", err)
+	}
+	if got := params.Get("state"); got != "open" {
+		t.Fatalf("state param mutated to %q", got)
+	}
+	if got := params.Get("page"); got != "" {
+		t.Fatalf("page param leaked into caller params: %q", got)
+	}
+	if got := params.Get("limit"); got != "" {
+		t.Fatalf("default limit leaked into caller params: %q", got)
+	}
+}
+
 func TestPaginateAllSingleObject(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")

internal/client/pagination.go

@@ -9,19 +9,20 @@ import (
 
 // PaginateAll fetches all pages and returns combined results.
 func (c *Client) PaginateAll(path string, params url.Values) ([]json.RawMessage, error) {
-	if params == nil {
-		params = url.Values{}
+	pageParams := cloneValues(params)
+	if pageParams == nil {
+		pageParams = url.Values{}
 	}
-	if params.Get("limit") == "" {
-		params.Set("limit", "50")
+	if pageParams.Get("limit") == "" {
+		pageParams.Set("limit", "50")
 	}
 
 	var all []json.RawMessage
 	page := 1
 
 	for {
-		params.Set("page", strconv.Itoa(page))
-		env, err := c.Get(path, params)
+		pageParams.Set("page", strconv.Itoa(page))
+		env, err := c.Get(path, pageParams)
 		if err != nil {
 			return nil, err
 		}
@@ -61,7 +62,7 @@ func (c *Client) PaginateAll(path string, params url.Values) ([]json.RawMessage,
 		all = append(all, items...)
 
 		// Check if we got fewer items than limit
-		limit, _ := strconv.Atoi(params.Get("limit"))
+		limit, _ := strconv.Atoi(pageParams.Get("limit"))
 		if len(items) < limit {
 			break
 		}
@@ -71,3 +72,14 @@ func (c *Client) PaginateAll(path string, params url.Values) ([]json.RawMessage,
 
 	return all, nil
 }
+
+func cloneValues(values url.Values) url.Values {
+	if values == nil {
+		return nil
+	}
+	cloned := make(url.Values, len(values))
+	for key, vals := range values {
+		cloned[key] = append([]string(nil), vals...)
+	}
+	return cloned
+}

internal/context/repo.go

@@ -59,10 +59,10 @@ func parseRemoteURL(remote string) (string, string, error) {
 }
 
 func parsePathSegments(path string) (string, string, error) {
-	path = strings.TrimPrefix(path, "/")
+	path = strings.Trim(path, "/")
 	path = strings.TrimSuffix(path, ".git")
 	parts := strings.SplitN(path, "/", 3)
-	if len(parts) < 2 {
+	if len(parts) < 2 || parts[0] == "" || parts[1] == "" {
 		return "", "", fmt.Errorf("cannot extract owner/repo from path: %s", path)
 	}
 	return parts[0], parts[1], nil

internal/context/repo_test.go

@@ -76,8 +76,10 @@ func TestParsePathSegments(t *testing.T) {
 		{"with git", "owner/repo.git", "owner", "repo", false},
 		{"leading slash", "/owner/repo", "owner", "repo", false},
 		{"both", "/owner/repo.git", "owner", "repo", false},
+		{"trailing slash", "/owner/repo.git/", "owner", "repo", false},
 		{"with subpath", "owner/repo/sub", "owner", "repo", false},
 		{"single segment", "onlyowner", "", "", true},
+		{"missing repo", "onlyowner/", "", "", true},
 		{"empty", "", "", "", true},
 	}
 	for _, tt := range tests {