Skip to content

[FEAT]: Add team command group#727

Open
mcncl wants to merge 3 commits into
mainfrom
SUP-6704/team_commands
Open

[FEAT]: Add team command group#727
mcncl wants to merge 3 commits into
mainfrom
SUP-6704/team_commands

Conversation

@mcncl

@mcncl mcncl commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Description

Adds support for group of team commands, as available in go-buildkite

Changes

  • adds team {create,delete,list,update,view}
  • adds tests for the new commands
  • updates main.go to support
❯ go run . team create Testers
{
  "id": "89cea5f7-a7c0-4183-b48c",
  "name": "Testers",
  "slug": "testers",
  "privacy": "visible",
  "default": false,
  "created_at": "2026-03-29T23:20:04.042Z",
  "created_by": {
    "id": "01831b25-7d66-431e-8dcf",
    "name": "Ben McNicholl",
    "email": "ben.mcnicholl@buildkite.com",
    "created_at": "2022-09-08T03:29:45.318Z"
  }
}

❯ go run . team list
[
  {
    "id": "3c911fd5-cf8f-4a45-814d",
    "name": "Everybody",
    "slug": "everybody",
    "description": "The default team",
    "privacy": "visible",
    "default": true,
    "created_at": "2026-03-16T02:08:21.763Z",
    "created_by": {
      "id": "01831b25-7d66-431e-8dcf",
      "name": "Ben McNicholl",
      "email": "ben.mcnicholl@buildkite.com",
      "created_at": "2022-09-08T03:29:45.318Z"
    }
  },
  {
    "id": "89cea5f7-a7c0-4183-b48c",
    "name": "Testers",
    "slug": "testers",
    "privacy": "visible",
    "default": false,
    "created_at": "2026-03-29T23:20:04.042Z",
    "created_by": {
      "id": "01831b25-7d66-431e-8dcf",
      "name": "Ben McNicholl",
      "email": "ben.mcnicholl@buildkite.com",
      "created_at": "2022-09-08T03:29:45.318Z"
    }
  }
]

❯ go run . team delete 89cea5f7-a7c0-4183-b48c --yes
Team deleted successfully.

Testing

  • Tests have run locally (with go test ./...)
  • Code is formatted (with go fmt ./...)

@mcncl
mcncl requested review from a team as code owners March 29, 2026 23:22
Comment thread cmd/team/list.go Outdated
var err error

spinErr := bkIO.SpinWhile(f, "Loading teams information", func() {
teams, _, err = f.RestAPIClient.Teams.List(ctx, f.Config.OrganizationSlug(), nil)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any concerns about not paginating here? I'm not sure how many teams we return per page, but I'm guessing sooner or later we will come across someone who has hundreds of them.

Comment thread cmd/team/update.go
Description: current.Description,
Privacy: current.Privacy,
IsDefaultTeam: current.Default,
MembersCanCreatePipelines: false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude flagged the fact that every time someone updates a team, it's going to set members_can_create_pipelines: false even if the current setting for that team is true. "Balderdash," said I. "Prove it!" So we looked at go_buildkite, where it turns out that the Team struct doesn't include all the fields that the API returns, including MembersCanCreatePipelines. And then looking upstream at the API in Team::Updater and how it would handle this, it looks like it would in fact apply the false flag and change the setting unexpectedly. We'd need to fix this in go_buildkite and then here do MembersCanCreatePipelines: current.MembersCanCreatePipelines,

If this makes sense to you, I can throw the PR at go_buildkite and then add a commit to this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think false is a sensible default? It's definitely safer than true and would still allow for creation of the team but with safeguards in place.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Discussed in Slack - we still need to fix go_buildkite to avoid us forcibly changing this setting when we update teams. That'll be a separate task.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The latest merge from main left the branch unbuildable. I left three blockers for the parse, module, and callback-signature failures reported by CI, plus one non-blocking pagination edge case.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 8503, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

About buildsworth

Model: gpt-5.6-sol with xhigh thinking.

How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.

Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. To let it approve, grant L2 approval by mentioning @buildsworth-bk (see L2 approval grant):

  • L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
  • L2 — Standard risk (new UI, additive API fields, refactors). Approved only with an L2 grant; otherwise comment-only.
  • L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.

Comment thread internal/team/view.go
"time"

"github.com/buildkite/cli/v3/pkg/output"
buildkite "github.com/buildkite/go-buildkite/v4"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking: The repository now requires go-buildkite/v5, and Factory.RestAPIClient is a v5 client, so this /v4 import (and the other new team imports) has no module provider; CI fails before compiling these packages. Please migrate the team code and tests to /v5. The v5 update endpoint takes buildkite.UpdateTeam with buildkite.Some(...), rather than CreateTeam.

Comment thread cmd/team/create.go
}

var t buildkite.Team
spinErr := bkIO.SpinWhile(f, "Creating team", func() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking: SpinWhile now accepts func() error, so this callback—and the five other new team callbacks—does not type-check. Please return the REST call’s error from each callback, following the current cluster and queue commands.

Comment thread main.go
// Kong CLI structure, with base commands defined as additional commands are defined in their respective files
type CLI struct {
// Global flags
Yes bool `help:"Skip all confirmation prompts" short:"y"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking: This inserted block duplicates the current CLI fields immediately below it, and the added UserCmd near TeamCmd is also left unclosed. As a result, main.go does not parse (CI reports the redeclarations and missing }). Please reapply only the Team field and TeamCmd definition to the current main.go.

Comment thread cmd/team/list.go

all = append(all, pageTeams...)

if len(pageTeams) < perPage {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: If a partial final page crosses the limit—for example, 90 collected plus a 15-item page with --limit 100—this breaks before hasMore is set. The result is then truncated to 100, but text output says Showing 100 teams even though five were omitted. Could we set hasMore when len(all) > limit before this break?

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.

2 participants