From e210fda0e5a844a01a8ca1ad5a4e4e1de62483a5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Mar 2026 21:40:25 +0000 Subject: [PATCH 1/4] Initial plan From cd3bd5b36ff296a53c7626396c8362193976a1e7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Mar 2026 21:47:13 +0000 Subject: [PATCH 2/4] feat: generic scope ID extraction and remote-scope API client - Add ScopeIDField and HasRepoScopes to ConnectionDef; set on github/gh-copilot entries - Replace ScopeListEntry typed struct with json.RawMessage + extractScopeID helper - Add RemoteScopeChild/RemoteScopeResponse types and ListRemoteScopes/SearchRemoteScopes client methods - Remove hardcoded if-plugin-github checks from listConnectionScopes, scope list, scope delete - Add table-driven tests for ExtractScopeID, ScopeListWrapper helpers, and new client methods Co-authored-by: ewega <26189114+ewega@users.noreply.github.com> --- cmd/configure_projects.go | 22 +-- cmd/configure_scope_delete.go | 13 +- cmd/configure_scope_list.go | 21 +-- cmd/connection_types.go | 5 + internal/devlake/client.go | 37 +++++ internal/devlake/client_test.go | 4 +- internal/devlake/types.go | 92 +++++++++-- internal/devlake/types_test.go | 277 ++++++++++++++++++++++++++++++++ 8 files changed, 433 insertions(+), 38 deletions(-) create mode 100644 internal/devlake/types_test.go diff --git a/cmd/configure_projects.go b/cmd/configure_projects.go index c791967..7a4ccac 100644 --- a/cmd/configure_projects.go +++ b/cmd/configure_projects.go @@ -233,23 +233,27 @@ func listConnectionScopes(client *devlake.Client, c connChoice) (*addedConnectio var bpScopes []devlake.BlueprintScope var repos []string + def := FindConnectionDef(c.plugin) for _, w := range resp.Scopes { - s := w.Scope - // Resolve scope ID: GitHub uses githubId (int), Copilot uses id (string) - scopeID := s.ID - if c.plugin == "github" && s.GithubID > 0 { - scopeID = fmt.Sprintf("%d", s.GithubID) + // Generic scope ID extraction using the plugin's configured ScopeIDField. + var scopeID string + if def != nil && def.ScopeIDField != "" { + scopeID = devlake.ExtractScopeID(w.RawScope, def.ScopeIDField) } - scopeName := s.FullName + scopeName := w.ScopeFullName() if scopeName == "" { - scopeName = s.Name + scopeName = w.ScopeName() + } + if scopeID == "" { + scopeID = scopeName } bpScopes = append(bpScopes, devlake.BlueprintScope{ ScopeID: scopeID, ScopeName: scopeName, }) - if c.plugin == "github" && s.FullName != "" { - repos = append(repos, s.FullName) + fullName := w.ScopeFullName() + if def != nil && def.HasRepoScopes && fullName != "" { + repos = append(repos, fullName) } fmt.Printf(" %s (ID: %s)\n", scopeName, scopeID) } diff --git a/cmd/configure_scope_delete.go b/cmd/configure_scope_delete.go index c3ae508..ee41a0f 100644 --- a/cmd/configure_scope_delete.go +++ b/cmd/configure_scope_delete.go @@ -2,11 +2,11 @@ package cmd import ( "fmt" - "strconv" "strings" "github.com/spf13/cobra" + "github.com/DevExpGBB/gh-devlake/internal/devlake" "github.com/DevExpGBB/gh-devlake/internal/prompt" ) @@ -101,14 +101,15 @@ func runScopeDelete(cmd *cobra.Command, args []string) error { } var entries []scopeEntry var labels []string + def := FindConnectionDef(selectedPlugin) for _, s := range resp.Scopes { - id := s.Scope.ID - if id == "" { - id = strconv.Itoa(s.Scope.GithubID) + var id string + if def != nil && def.ScopeIDField != "" { + id = devlake.ExtractScopeID(s.RawScope, def.ScopeIDField) } - name := s.Scope.FullName + name := s.ScopeFullName() if name == "" { - name = s.Scope.Name + name = s.ScopeName() } label := fmt.Sprintf("[%s] %s", id, name) entries = append(entries, scopeEntry{id: id, label: label}) diff --git a/cmd/configure_scope_list.go b/cmd/configure_scope_list.go index f3e92d6..745b73f 100644 --- a/cmd/configure_scope_list.go +++ b/cmd/configure_scope_list.go @@ -2,7 +2,6 @@ package cmd import ( "fmt" - "strconv" "strings" "text/tabwriter" @@ -110,15 +109,16 @@ func runScopeList(cmd *cobra.Command, args []string) error { // JSON output path if outputJSON { items := make([]scopeListItem, len(resp.Scopes)) + def := FindConnectionDef(selectedPlugin) for i, s := range resp.Scopes { - scopeID := s.Scope.ID - if scopeID == "" { - scopeID = strconv.Itoa(s.Scope.GithubID) + var scopeID string + if def != nil && def.ScopeIDField != "" { + scopeID = devlake.ExtractScopeID(s.RawScope, def.ScopeIDField) } items[i] = scopeListItem{ ID: scopeID, - Name: s.Scope.Name, - FullName: s.Scope.FullName, + Name: s.ScopeName(), + FullName: s.ScopeFullName(), } } return printJSON(items) @@ -132,12 +132,13 @@ func runScopeList(cmd *cobra.Command, args []string) error { w := tabwriter.NewWriter(cmd.OutOrStdout(), 0, 0, 2, ' ', 0) fmt.Fprintln(w, "Scope ID\tName\tFull Name") fmt.Fprintln(w, strings.Repeat("\u2500", 10)+"\t"+strings.Repeat("\u2500", 20)+"\t"+strings.Repeat("\u2500", 30)) + def := FindConnectionDef(selectedPlugin) for _, s := range resp.Scopes { - scopeID := s.Scope.ID - if scopeID == "" { - scopeID = strconv.Itoa(s.Scope.GithubID) + var scopeID string + if def != nil && def.ScopeIDField != "" { + scopeID = devlake.ExtractScopeID(s.RawScope, def.ScopeIDField) } - fmt.Fprintf(w, "%s\t%s\t%s\n", scopeID, s.Scope.Name, s.Scope.FullName) + fmt.Fprintf(w, "%s\t%s\t%s\n", scopeID, s.ScopeName(), s.ScopeFullName()) } w.Flush() fmt.Println() diff --git a/cmd/connection_types.go b/cmd/connection_types.go index 0413c1d..c68558a 100644 --- a/cmd/connection_types.go +++ b/cmd/connection_types.go @@ -35,6 +35,8 @@ type ConnectionDef struct { EnvVarNames []string // environment variable names for token resolution EnvFileKeys []string // .devlake.env keys for token resolution ScopeFunc ScopeHandler // nil = scope configuration not yet supported + ScopeIDField string // JSON field name for the scope ID (e.g. "githubId", "id") + HasRepoScopes bool // true = scopes carry a FullName that should be tracked as repos } // MenuLabel returns the label for interactive menus. @@ -144,6 +146,8 @@ var connectionRegistry = []*ConnectionDef{ EnvVarNames: []string{"GITHUB_PAT", "GITHUB_TOKEN", "GH_TOKEN"}, EnvFileKeys: []string{"GITHUB_PAT", "GITHUB_TOKEN", "GH_TOKEN"}, ScopeFunc: scopeGitHubHandler, + ScopeIDField: "githubId", + HasRepoScopes: true, }, { Plugin: "gh-copilot", @@ -161,6 +165,7 @@ var connectionRegistry = []*ConnectionDef{ EnvVarNames: []string{"GITHUB_PAT", "GITHUB_TOKEN", "GH_TOKEN"}, EnvFileKeys: []string{"GITHUB_PAT", "GITHUB_TOKEN", "GH_TOKEN"}, ScopeFunc: scopeCopilotHandler, + ScopeIDField: "id", }, { Plugin: "gitlab", diff --git a/internal/devlake/client.go b/internal/devlake/client.go index a9aa98a..d249d12 100644 --- a/internal/devlake/client.go +++ b/internal/devlake/client.go @@ -462,6 +462,43 @@ func (c *Client) GetPipeline(id int) (*Pipeline, error) { return doGet[Pipeline](c, fmt.Sprintf("/pipelines/%d", id)) } +// ListRemoteScopes queries the DevLake remote-scope API for a plugin connection. +// groupID and pageToken are optional (pass "" to omit). +func (c *Client) ListRemoteScopes(plugin string, connID int, groupID, pageToken string) (*RemoteScopeResponse, error) { + path := fmt.Sprintf("/plugins/%s/connections/%d/remote-scopes", plugin, connID) + q := url.Values{} + if groupID != "" { + q.Set("groupId", groupID) + } + if pageToken != "" { + q.Set("pageToken", pageToken) + } + if len(q) > 0 { + path += "?" + q.Encode() + } + return doGet[RemoteScopeResponse](c, path) +} + +// SearchRemoteScopes queries the DevLake search-remote-scopes API for a plugin connection. +// page and pageSize control pagination; pass 0 to use DevLake defaults. +func (c *Client) SearchRemoteScopes(plugin string, connID int, search string, page, pageSize int) (*RemoteScopeResponse, error) { + path := fmt.Sprintf("/plugins/%s/connections/%d/search-remote-scopes", plugin, connID) + q := url.Values{} + if search != "" { + q.Set("search", search) + } + if page > 0 { + q.Set("page", fmt.Sprintf("%d", page)) + } + if pageSize > 0 { + q.Set("pageSize", fmt.Sprintf("%d", pageSize)) + } + if len(q) > 0 { + path += "?" + q.Encode() + } + return doGet[RemoteScopeResponse](c, path) +} + // TriggerMigration triggers the DevLake database migration endpoint. func (c *Client) TriggerMigration() error { resp, err := c.HTTPClient.Get(c.BaseURL + "/proceed-db-migration") diff --git a/internal/devlake/client_test.go b/internal/devlake/client_test.go index 22c0db6..ea5efec 100644 --- a/internal/devlake/client_test.go +++ b/internal/devlake/client_test.go @@ -589,8 +589,8 @@ func TestListScopes(t *testing.T) { if len(result.Scopes) != 1 { t.Fatalf("len(Scopes) = %d, want 1", len(result.Scopes)) } - if result.Scopes[0].Scope.Name != "repo1" { - t.Errorf("Name = %q, want %q", result.Scopes[0].Scope.Name, "repo1") + if result.Scopes[0].ScopeName() != "repo1" { + t.Errorf("Name = %q, want %q", result.Scopes[0].ScopeName(), "repo1") } } diff --git a/internal/devlake/types.go b/internal/devlake/types.go index b866795..085eb88 100644 --- a/internal/devlake/types.go +++ b/internal/devlake/types.go @@ -1,5 +1,10 @@ package devlake +import ( + "encoding/json" + "strconv" +) + // ScopeConfig represents a DevLake scope configuration (e.g., DORA settings). type ScopeConfig struct { ID int `json:"id,omitempty"` @@ -46,18 +51,67 @@ type ScopeBatchRequest struct { // ScopeListWrapper wraps a scope object as returned by the DevLake GET scopes API. // The API nests each scope inside a "scope" key: { "scope": { ... } }. +// RawScope preserves the full plugin-specific payload for generic ID extraction. type ScopeListWrapper struct { - Scope ScopeListEntry `json:"scope"` -} - -// ScopeListEntry represents a scope object returned inside the wrapper. -// ID fields vary by plugin (githubId for GitHub, id for Copilot), so we -// capture both and resolve in the caller. -type ScopeListEntry struct { - GithubID int `json:"githubId,omitempty"` - ID string `json:"id,omitempty"` - Name string `json:"name"` - FullName string `json:"fullName,omitempty"` + RawScope json.RawMessage `json:"scope"` +} + +// ScopeName returns the display name from the raw scope JSON (checks "fullName" then "name"). +func (w *ScopeListWrapper) ScopeName() string { + var m map[string]json.RawMessage + if err := json.Unmarshal(w.RawScope, &m); err != nil { + return "" + } + for _, key := range []string{"fullName", "name"} { + if v, ok := m[key]; ok { + var s string + if err := json.Unmarshal(v, &s); err == nil { + return s + } + } + } + return "" +} + +// ScopeFullName returns the "fullName" field from the raw scope JSON, or "". +func (w *ScopeListWrapper) ScopeFullName() string { + var m map[string]json.RawMessage + if err := json.Unmarshal(w.RawScope, &m); err != nil { + return "" + } + if v, ok := m["fullName"]; ok { + var s string + if err := json.Unmarshal(v, &s); err == nil { + return s + } + } + return "" +} + +// ExtractScopeID extracts the scope ID from a raw JSON scope object using the +// given field name. It tries to decode the value as a string first, then as +// an integer (converted to its decimal string representation). +func ExtractScopeID(raw json.RawMessage, fieldName string) string { + if fieldName == "" { + return "" + } + var m map[string]json.RawMessage + if err := json.Unmarshal(raw, &m); err != nil { + return "" + } + v, ok := m[fieldName] + if !ok { + return "" + } + var s string + if err := json.Unmarshal(v, &s); err == nil && s != "" { + return s + } + var n int64 + if err := json.Unmarshal(v, &n); err == nil && n != 0 { + return strconv.FormatInt(n, 10) + } + return "" } // ScopeListResponse is the response from GET /plugins/{plugin}/connections/{id}/scopes. @@ -66,6 +120,22 @@ type ScopeListResponse struct { Count int `json:"count"` } +// RemoteScopeChild represents one item (group or scope) from the remote-scope API. +type RemoteScopeChild struct { + Type string `json:"type"` // "group" or "scope" + ID string `json:"id"` + ParentID string `json:"parentId"` + Name string `json:"name"` + FullName string `json:"fullName"` + Data json.RawMessage `json:"data"` +} + +// RemoteScopeResponse is the response from GET /plugins/{plugin}/connections/{id}/remote-scopes. +type RemoteScopeResponse struct { + Children []RemoteScopeChild `json:"children"` + NextPageToken string `json:"nextPageToken"` +} + // Project represents a DevLake project. type Project struct { Name string `json:"name"` diff --git a/internal/devlake/types_test.go b/internal/devlake/types_test.go new file mode 100644 index 0000000..33c3b15 --- /dev/null +++ b/internal/devlake/types_test.go @@ -0,0 +1,277 @@ +package devlake + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// TestExtractScopeID tests the generic scope ID extraction helper. +func TestExtractScopeID(t *testing.T) { + tests := []struct { + name string + raw string + fieldName string + want string + }{ + { + name: "string field", + raw: `{"id": "org/repo"}`, + fieldName: "id", + want: "org/repo", + }, + { + name: "integer field", + raw: `{"githubId": 12345678}`, + fieldName: "githubId", + want: "12345678", + }, + { + name: "field not present", + raw: `{"name": "repo1"}`, + fieldName: "githubId", + want: "", + }, + { + name: "empty field name", + raw: `{"id": "x"}`, + fieldName: "", + want: "", + }, + { + name: "zero integer is treated as missing", + raw: `{"githubId": 0}`, + fieldName: "githubId", + want: "", + }, + { + name: "empty string is treated as missing", + raw: `{"id": ""}`, + fieldName: "id", + want: "", + }, + { + name: "invalid JSON returns empty string", + raw: `not json`, + fieldName: "id", + want: "", + }, + { + name: "large integer field", + raw: `{"gitlabId": 9876543210}`, + fieldName: "gitlabId", + want: "9876543210", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ExtractScopeID(json.RawMessage(tt.raw), tt.fieldName) + if got != tt.want { + t.Errorf("ExtractScopeID(%q, %q) = %q, want %q", tt.raw, tt.fieldName, got, tt.want) + } + }) + } +} + +// TestScopeListWrapperHelpers tests the ScopeName and ScopeFullName helpers. +func TestScopeListWrapperHelpers(t *testing.T) { + tests := []struct { + name string + raw string + wantName string + wantFullName string + }{ + { + name: "fullName takes precedence in ScopeName", + raw: `{"name": "repo1", "fullName": "org/repo1"}`, + wantName: "org/repo1", + wantFullName: "org/repo1", + }, + { + name: "name used when fullName absent", + raw: `{"name": "repo1"}`, + wantName: "repo1", + wantFullName: "", + }, + { + name: "both empty", + raw: `{}`, + wantName: "", + wantFullName: "", + }, + { + name: "invalid JSON", + raw: `not json`, + wantName: "", + wantFullName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := ScopeListWrapper{RawScope: json.RawMessage(tt.raw)} + if got := w.ScopeName(); got != tt.wantName { + t.Errorf("ScopeName() = %q, want %q", got, tt.wantName) + } + if got := w.ScopeFullName(); got != tt.wantFullName { + t.Errorf("ScopeFullName() = %q, want %q", got, tt.wantFullName) + } + }) + } +} + +// TestListRemoteScopes tests the ListRemoteScopes client method. +func TestListRemoteScopes(t *testing.T) { + tests := []struct { + name string + groupID string + pageToken string + statusCode int + body string + wantErr bool + wantChildren int + wantNextToken string + wantPath string + }{ + { + name: "success without params", + statusCode: http.StatusOK, + body: `{"children": [{"type": "scope", "id": "123", "name": "repo1"}], "nextPageToken": ""}`, + wantChildren: 1, + wantNextToken: "", + wantPath: "/plugins/github/connections/1/remote-scopes", + }, + { + name: "success with groupID and pageToken", + groupID: "mygroup", + pageToken: "tok1", + statusCode: http.StatusOK, + body: `{"children": [{"type": "group", "id": "g1", "name": "Group 1"}, {"type": "scope", "id": "s1", "name": "Scope 1"}], "nextPageToken": "tok2"}`, + wantChildren: 2, + wantNextToken: "tok2", + wantPath: "/plugins/github/connections/1/remote-scopes", + }, + { + name: "server error", + statusCode: http.StatusInternalServerError, + body: `{"error": "server error"}`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("method = %s, want GET", r.Method) + } + if r.URL.Path != tt.wantPath && tt.wantPath != "" { + t.Errorf("path = %s, want %s", r.URL.Path, tt.wantPath) + } + if tt.groupID != "" && r.URL.Query().Get("groupId") != tt.groupID { + t.Errorf("groupId = %q, want %q", r.URL.Query().Get("groupId"), tt.groupID) + } + if tt.pageToken != "" && r.URL.Query().Get("pageToken") != tt.pageToken { + t.Errorf("pageToken = %q, want %q", r.URL.Query().Get("pageToken"), tt.pageToken) + } + w.WriteHeader(tt.statusCode) + _, _ = w.Write([]byte(tt.body)) + })) + defer srv.Close() + + client := NewClient(srv.URL) + result, err := client.ListRemoteScopes("github", 1, tt.groupID, tt.pageToken) + + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(result.Children) != tt.wantChildren { + t.Errorf("len(Children) = %d, want %d", len(result.Children), tt.wantChildren) + } + if result.NextPageToken != tt.wantNextToken { + t.Errorf("NextPageToken = %q, want %q", result.NextPageToken, tt.wantNextToken) + } + }) + } +} + +// TestSearchRemoteScopes tests the SearchRemoteScopes client method. +func TestSearchRemoteScopes(t *testing.T) { + tests := []struct { + name string + search string + page int + pageSize int + statusCode int + body string + wantErr bool + wantChildren int + }{ + { + name: "success with search term", + search: "my-repo", + page: 1, + pageSize: 20, + statusCode: http.StatusOK, + body: `{"children": [{"type": "scope", "id": "42", "name": "my-repo", "fullName": "org/my-repo"}]}`, + wantChildren: 1, + }, + { + name: "success no params", + statusCode: http.StatusOK, + body: `{"children": []}`, + wantChildren: 0, + }, + { + name: "not found", + search: "nothing", + statusCode: http.StatusNotFound, + body: `{"error": "not found"}`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("method = %s, want GET", r.Method) + } + expectedPath := "/plugins/gitlab/connections/2/search-remote-scopes" + if r.URL.Path != expectedPath { + t.Errorf("path = %s, want %s", r.URL.Path, expectedPath) + } + if tt.search != "" && r.URL.Query().Get("search") != tt.search { + t.Errorf("search = %q, want %q", r.URL.Query().Get("search"), tt.search) + } + w.WriteHeader(tt.statusCode) + _, _ = w.Write([]byte(tt.body)) + })) + defer srv.Close() + + client := NewClient(srv.URL) + result, err := client.SearchRemoteScopes("gitlab", 2, tt.search, tt.page, tt.pageSize) + + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(result.Children) != tt.wantChildren { + t.Errorf("len(Children) = %d, want %d", len(result.Children), tt.wantChildren) + } + }) + } +} From 5ec9da9012cae0ab49cd757a12d2f80b6693b06c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 4 Mar 2026 23:21:53 +0000 Subject: [PATCH 3/4] fix: ScopeName empty-string fallthrough and deduplicate ScopeFullName call - ScopeName() now skips empty strings (like ExtractScopeID does), so a JSON field of "" correctly falls through to the next candidate key - configure_projects.go: call ScopeFullName() once per scope and reuse the value for both the display name and repo-tracking check - Add "empty fullName falls through to name" test case Co-authored-by: ewega <26189114+ewega@users.noreply.github.com> --- cmd/configure_projects.go | 4 ++-- internal/devlake/types.go | 2 +- internal/devlake/types_test.go | 6 ++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/configure_projects.go b/cmd/configure_projects.go index 7a4ccac..df1945d 100644 --- a/cmd/configure_projects.go +++ b/cmd/configure_projects.go @@ -240,7 +240,8 @@ func listConnectionScopes(client *devlake.Client, c connChoice) (*addedConnectio if def != nil && def.ScopeIDField != "" { scopeID = devlake.ExtractScopeID(w.RawScope, def.ScopeIDField) } - scopeName := w.ScopeFullName() + fullName := w.ScopeFullName() + scopeName := fullName if scopeName == "" { scopeName = w.ScopeName() } @@ -251,7 +252,6 @@ func listConnectionScopes(client *devlake.Client, c connChoice) (*addedConnectio ScopeID: scopeID, ScopeName: scopeName, }) - fullName := w.ScopeFullName() if def != nil && def.HasRepoScopes && fullName != "" { repos = append(repos, fullName) } diff --git a/internal/devlake/types.go b/internal/devlake/types.go index 085eb88..e3e6359 100644 --- a/internal/devlake/types.go +++ b/internal/devlake/types.go @@ -65,7 +65,7 @@ func (w *ScopeListWrapper) ScopeName() string { for _, key := range []string{"fullName", "name"} { if v, ok := m[key]; ok { var s string - if err := json.Unmarshal(v, &s); err == nil { + if err := json.Unmarshal(v, &s); err == nil && s != "" { return s } } diff --git a/internal/devlake/types_test.go b/internal/devlake/types_test.go index 33c3b15..89c5c76 100644 --- a/internal/devlake/types_test.go +++ b/internal/devlake/types_test.go @@ -95,6 +95,12 @@ func TestScopeListWrapperHelpers(t *testing.T) { wantName: "repo1", wantFullName: "", }, + { + name: "empty fullName falls through to name", + raw: `{"name": "repo1", "fullName": ""}`, + wantName: "repo1", + wantFullName: "", + }, { name: "both empty", raw: `{}`, From 61e4270db06f466c5cd7c26c6325240e19566b3b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 5 Mar 2026 08:34:30 +0000 Subject: [PATCH 4/4] fix: cache parseScope, ScopeFullName empty-string, ID fallback, pagination test assertions - ScopeListWrapper gains a shared parseScope() helper with lazy map caching; ScopeName() and ScopeFullName() now unmarshal RawScope at most once per item - ScopeFullName() now treats "" as absent (consistent with ScopeName/ExtractScopeID) - configure_scope_list: deduplicate FindConnectionDef; add scopeIDFor() closure that falls back to ScopeName() when ExtractScopeID returns "" - configure_scope_delete: add id=="" fallback to name so interactive delete always has a usable identifier even when ScopeIDField payload is missing - TestSearchRemoteScopes: assert page, pageSize, and search query params are sent when non-zero and absent when zero Co-authored-by: ewega <26189114+ewega@users.noreply.github.com> --- cmd/configure_scope_delete.go | 9 ++++++--- cmd/configure_scope_list.go | 28 ++++++++++++++++------------ internal/devlake/types.go | 33 +++++++++++++++++++++++---------- internal/devlake/types_test.go | 30 ++++++++++++++++++++++++++++-- 4 files changed, 73 insertions(+), 27 deletions(-) diff --git a/cmd/configure_scope_delete.go b/cmd/configure_scope_delete.go index ee41a0f..d3f5f5e 100644 --- a/cmd/configure_scope_delete.go +++ b/cmd/configure_scope_delete.go @@ -103,13 +103,16 @@ func runScopeDelete(cmd *cobra.Command, args []string) error { var labels []string def := FindConnectionDef(selectedPlugin) for _, s := range resp.Scopes { + name := s.ScopeFullName() + if name == "" { + name = s.ScopeName() + } var id string if def != nil && def.ScopeIDField != "" { id = devlake.ExtractScopeID(s.RawScope, def.ScopeIDField) } - name := s.ScopeFullName() - if name == "" { - name = s.ScopeName() + if id == "" { + id = name } label := fmt.Sprintf("[%s] %s", id, name) entries = append(entries, scopeEntry{id: id, label: label}) diff --git a/cmd/configure_scope_list.go b/cmd/configure_scope_list.go index 745b73f..d20c6e4 100644 --- a/cmd/configure_scope_list.go +++ b/cmd/configure_scope_list.go @@ -106,17 +106,26 @@ func runScopeList(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to list scopes: %w", err) } + def := FindConnectionDef(selectedPlugin) + + // scopeIDFor extracts the scope ID using the plugin's ScopeIDField (def.ScopeIDField). + // When ExtractScopeID returns "" (field absent or plugin has no ScopeIDField configured), + // it falls back to the display name so the output always shows a usable identifier. + scopeIDFor := func(s *devlake.ScopeListWrapper) string { + if def != nil && def.ScopeIDField != "" { + if id := devlake.ExtractScopeID(s.RawScope, def.ScopeIDField); id != "" { + return id + } + } + return s.ScopeName() + } + // JSON output path if outputJSON { items := make([]scopeListItem, len(resp.Scopes)) - def := FindConnectionDef(selectedPlugin) for i, s := range resp.Scopes { - var scopeID string - if def != nil && def.ScopeIDField != "" { - scopeID = devlake.ExtractScopeID(s.RawScope, def.ScopeIDField) - } items[i] = scopeListItem{ - ID: scopeID, + ID: scopeIDFor(&s), Name: s.ScopeName(), FullName: s.ScopeFullName(), } @@ -132,13 +141,8 @@ func runScopeList(cmd *cobra.Command, args []string) error { w := tabwriter.NewWriter(cmd.OutOrStdout(), 0, 0, 2, ' ', 0) fmt.Fprintln(w, "Scope ID\tName\tFull Name") fmt.Fprintln(w, strings.Repeat("\u2500", 10)+"\t"+strings.Repeat("\u2500", 20)+"\t"+strings.Repeat("\u2500", 30)) - def := FindConnectionDef(selectedPlugin) for _, s := range resp.Scopes { - var scopeID string - if def != nil && def.ScopeIDField != "" { - scopeID = devlake.ExtractScopeID(s.RawScope, def.ScopeIDField) - } - fmt.Fprintf(w, "%s\t%s\t%s\n", scopeID, s.ScopeName(), s.ScopeFullName()) + fmt.Fprintf(w, "%s\t%s\t%s\n", scopeIDFor(&s), s.ScopeName(), s.ScopeFullName()) } w.Flush() fmt.Println() diff --git a/internal/devlake/types.go b/internal/devlake/types.go index e3e6359..35410f7 100644 --- a/internal/devlake/types.go +++ b/internal/devlake/types.go @@ -53,15 +53,30 @@ type ScopeBatchRequest struct { // The API nests each scope inside a "scope" key: { "scope": { ... } }. // RawScope preserves the full plugin-specific payload for generic ID extraction. type ScopeListWrapper struct { - RawScope json.RawMessage `json:"scope"` + RawScope json.RawMessage `json:"scope"` + parsed map[string]json.RawMessage // lazily populated by parseScope +} + +// parseScope unmarshals RawScope into a map exactly once per wrapper instance, +// caching the result so callers that invoke both ScopeName and ScopeFullName on +// the same item do not unmarshal the same JSON twice. +func (w *ScopeListWrapper) parseScope() map[string]json.RawMessage { + if w.parsed == nil { + var m map[string]json.RawMessage + if err := json.Unmarshal(w.RawScope, &m); err != nil || m == nil { + m = make(map[string]json.RawMessage) + } + w.parsed = m + } + return w.parsed } // ScopeName returns the display name from the raw scope JSON (checks "fullName" then "name"). +// Empty string values are skipped so the next candidate key is tried. +// Parsing is cached via parseScope() so calling ScopeName and ScopeFullName on the +// same instance only unmarshals the JSON once. func (w *ScopeListWrapper) ScopeName() string { - var m map[string]json.RawMessage - if err := json.Unmarshal(w.RawScope, &m); err != nil { - return "" - } + m := w.parseScope() for _, key := range []string{"fullName", "name"} { if v, ok := m[key]; ok { var s string @@ -74,14 +89,12 @@ func (w *ScopeListWrapper) ScopeName() string { } // ScopeFullName returns the "fullName" field from the raw scope JSON, or "". +// An empty string value is treated as absent (returns ""). func (w *ScopeListWrapper) ScopeFullName() string { - var m map[string]json.RawMessage - if err := json.Unmarshal(w.RawScope, &m); err != nil { - return "" - } + m := w.parseScope() if v, ok := m["fullName"]; ok { var s string - if err := json.Unmarshal(v, &s); err == nil { + if err := json.Unmarshal(v, &s); err == nil && s != "" { return s } } diff --git a/internal/devlake/types_test.go b/internal/devlake/types_test.go index 89c5c76..5019779 100644 --- a/internal/devlake/types_test.go +++ b/internal/devlake/types_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strconv" "testing" ) @@ -255,8 +256,33 @@ func TestSearchRemoteScopes(t *testing.T) { if r.URL.Path != expectedPath { t.Errorf("path = %s, want %s", r.URL.Path, expectedPath) } - if tt.search != "" && r.URL.Query().Get("search") != tt.search { - t.Errorf("search = %q, want %q", r.URL.Query().Get("search"), tt.search) + q := r.URL.Query() + if tt.search != "" { + if got := q.Get("search"); got != tt.search { + t.Errorf("search = %q, want %q", got, tt.search) + } + } else { + if q.Has("search") { + t.Errorf("unexpected search param %q", q.Get("search")) + } + } + if tt.page != 0 { + if got := q.Get("page"); got != strconv.Itoa(tt.page) { + t.Errorf("page = %q, want %d", got, tt.page) + } + } else { + if q.Has("page") { + t.Errorf("unexpected page param %q", q.Get("page")) + } + } + if tt.pageSize != 0 { + if got := q.Get("pageSize"); got != strconv.Itoa(tt.pageSize) { + t.Errorf("pageSize = %q, want %d", got, tt.pageSize) + } + } else { + if q.Has("pageSize") { + t.Errorf("unexpected pageSize param %q", q.Get("pageSize")) + } } w.WriteHeader(tt.statusCode) _, _ = w.Write([]byte(tt.body))