Skip to content

Addressing PR comments#136

Merged
ewega merged 5 commits intomainfrom
codex/add-circleci-plugin-support
Mar 11, 2026
Merged

Addressing PR comments#136
ewega merged 5 commits intomainfrom
codex/add-circleci-plugin-support

Conversation

@Codex
Copy link
Contributor

@Codex Codex AI commented Mar 11, 2026

No description provided.

@Codex Codex AI changed the title [WIP] Add CircleCI plugin support in CLI Add CircleCI plugin support with remote-scope picker Mar 11, 2026
@ewega ewega marked this pull request as ready for review March 11, 2026 21:58
Copilot AI review requested due to automatic review settings March 11, 2026 21:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class CircleCI plugin support to the gh-devlake GitHub CLI extension, enabling users to create/test CircleCI connections and pick CircleCI projects as scopes via the DevLake remote-scope API.

Changes:

  • Added CircleCI to the connection registry (default endpoint, token env keys, supports test, scope handler, scope ID field).
  • Implemented a CircleCI remote-scope picker and scope upsert flow (PUT scopes + blueprint scopes).
  • Introduced CircleCI scope type plus unit tests; updated docs to include CircleCI usage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/devlake/types.go Adds CircleCIProjectScope type for PUT /scopes payloads.
cmd/connection_types.go Registers CircleCI plugin metadata and wires in the scope handler.
cmd/configure_scopes.go Implements CircleCI remote-scope listing, selection, and scope upsert.
cmd/connection_types_test.go Adds registry coverage for CircleCI and token-expiry matrix update.
cmd/configure_scopes_test.go Adds tests for CircleCI project parsing and label formatting helpers.
docs/configure-connection.md Documents CircleCI connection creation/token requirements and plugin list.
docs/configure-scope.md Documents CircleCI scope selection and expands supported plugin list.

Comment on lines 35 to 38
| `--plugin` | *(interactive or required)* | Plugin to configure (`github`, `gh-copilot`, `gitlab`, `bitbucket`, `azuredevops_go`, `jenkins`, `jira`, `sonarqube`, `circleci`) |
| `--connection-id` | *(auto-detected)* | Override the connection ID to scope |
| `--org` | *(required)* | GitHub organization slug |
| `--enterprise` | | Enterprise slug (enables enterprise-level Copilot metrics) |
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flags table marks --org as "(required)", but scope add only requires an org for plugins whose ConnectionDef has NeedsOrg set (e.g., CircleCI/Jenkins/SonarQube don’t require it). This is especially confusing now that circleci is listed as a supported plugin. Update the description to indicate --org is plugin-dependent (or list which plugins require it).

Copilot uses AI. Check for mistakes.
Comment on lines +1255 to +1261
projectMap := make(map[string]devlake.CircleCIProjectScope)
for _, child := range children {
if child.Type != "scope" || child.ID == "" {
continue
}
project := circleCIProjectFromChild(child, connID)
label := circleCIProjectLabel(project)
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

projectMap is keyed by the human-readable label, which can collide (e.g., multiple projects with the same name, or name-only labels when slug is missing). Collisions will overwrite entries in the map and can cause the wrong project to be scoped when a user selects an option. Use a stable unique key (e.g., project ID/slug) for the selection mapping, or make the displayed labels unique by appending the ID when duplicates are detected.

Suggested change
projectMap := make(map[string]devlake.CircleCIProjectScope)
for _, child := range children {
if child.Type != "scope" || child.ID == "" {
continue
}
project := circleCIProjectFromChild(child, connID)
label := circleCIProjectLabel(project)
projectMap := make(map[string]devlake.CircleCIProjectScope)
// First pass: count how many times each base label appears
labelCounts := make(map[string]int, len(children))
for _, child := range children {
if child.Type != "scope" || child.ID == "" {
continue
}
project := circleCIProjectFromChild(child, connID)
label := circleCIProjectLabel(project)
labelCounts[label]++
}
// Second pass: build unique labels and project map
for _, child := range children {
if child.Type != "scope" || child.ID == "" {
continue
}
project := circleCIProjectFromChild(child, connID)
baseLabel := circleCIProjectLabel(project)
label := baseLabel
if labelCounts[baseLabel] > 1 {
// Disambiguate duplicate labels by appending the project ID
label = fmt.Sprintf("%s (ID: %s)", baseLabel, project.ID)
}

Copilot uses AI. Check for mistakes.
@ewega
Copy link
Contributor

ewega commented Mar 11, 2026

@codex[agent] Please address the review findings on this PR: 1) update docs/configure-scope.md so --org is described as plugin-dependent rather than universally required; 2) make CircleCI selection mapping collision-safe so duplicate human-readable labels cannot overwrite the wrong project. After updating, rerun build/test/vet and let the review agent re-check.

@Codex
Copy link
Contributor Author

Codex AI commented Mar 11, 2026

@codex[agent] Please address the review findings on this PR: 1) update docs/configure-scope.md so --org is described as plugin-dependent rather than universally required; 2) make CircleCI selection ma...

Updated: --org is now described as plugin-dependent in docs/configure-scope.md, and CircleCI scope selection now disambiguates duplicate labels by appending the project ID to avoid collisions. Commit a2d7436.

@ewega
Copy link
Contributor

ewega commented Mar 11, 2026

@codex[agent] One review item still appears unresolved on this PR: update the configure-scope docs so --org is explicitly described as plugin-dependent rather than universally required. After updating, rerun build/test/vet and let the review agent re-check.

@Codex
Copy link
Contributor Author

Codex AI commented Mar 11, 2026

@codex[agent] One review item still appears unresolved on this PR: update the configure-scope docs so --org is explicitly described as plugin-dependent rather than universally required. After updati...

Updated configure-scope docs to state --org is required only for plugins that scope by org/workspace and explicitly note the plugins that don’t need it. Commit 951cb22.

@Codex Codex AI changed the title Add CircleCI plugin support with remote-scope picker Addressing PR comments Mar 11, 2026
@ewega
Copy link
Contributor

ewega commented Mar 11, 2026

@codex[agent] This PR now needs a latest-main refresh before merge. Please rebase/update onto current main, preserve the CircleCI fixes, ensure the --org docs wording is plugin-dependent, rerun checks, and push an updated head.

@Codex
Copy link
Contributor Author

Codex AI commented Mar 11, 2026

@codex[agent] This PR now needs a latest-main refresh before merge. Please rebase/update onto current main, preserve the CircleCI fixes, ensure the --org docs wording is plugin-dependent, rerun chec...

Rebased onto latest main, resolved conflicts (including org docs and CircleCI scopes), restored upstream types, and reran go build/test/vet. Head commit ce3001e.

@ewega
Copy link
Contributor

ewega commented Mar 11, 2026

@codex[agent] thanks — CI is green, but this PR is still DIRTY against current main. Please do a pure merge-refresh/rebase against the latest main and resolve conflicts carefully in the shared hotspots: cmd/connection_types.go, cmd/configure_scopes.go, internal/devlake/types.go, docs/configure-connection.md, and docs/configure-scope.md. Preserve already-merged plugin work on main (ArgoCD, Bitbucket, SonarQube, start command updates). Goal: same CircleCI behavior, just conflict-free on latest base.

@ewega ewega merged commit 2bc1d40 into main Mar 11, 2026
7 checks passed
@ewega ewega deleted the codex/add-circleci-plugin-support branch March 11, 2026 23:08
Copilot stopped work on behalf of ewega due to an error March 11, 2026 23:09
@ewega ewega mentioned this pull request Mar 12, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants