Skip to content

baton-github-enterprise: import the latest baton-github - #22

Merged
Bencheng21 merged 1 commit into
mainfrom
ben/update-enterprise
Jun 2, 2026
Merged

baton-github-enterprise: import the latest baton-github#22
Bencheng21 merged 1 commit into
mainfrom
ben/update-enterprise

Conversation

@Bencheng21

@Bencheng21 Bencheng21 commented May 28, 2026

Copy link
Copy Markdown
Contributor
    {
      "resourceType": {
        "id": "license",
        "displayName": "License",
        "traits": [
          "TRAIT_LICENSE_PROFILE"
        ],
        "annotations": [
          {
            "@type": "type.googleapis.com/c1.connector.v2.V1Identifier",
            "id": "license"
          },
          {
            "@type": "type.googleapis.com/c1.connector.v2.SkipEntitlements"
          }
        ]
      },
      "capabilities": [
        "CAPABILITY_SYNC"
      ],
      "permissions": {}

@Bencheng21
Bencheng21 requested a review from a team May 28, 2026 18:24
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Connector PR Review: baton-github-enterprise: import the latest baton-github

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Review mode: full
View review run

Review Summary

This PR bumps baton-github from v0.3.4 to v0.3.7, bringing in three new resource types (license, enterprise_role, api-key), exported builder functions, and a local DefaultCapabilitiesBuilder that replaces the old githubConnector.GitHub{} for capabilities discovery. The license resource type uses grant expansion through the enterprise "Member" role, with OptInRequired gating, and documentation is updated accordingly. No security or correctness issues found.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • .gitignore: (Carried from prior review) Add /baton-github-enterprise to .gitignore to prevent the extensionless Linux binary from being committed.

@github-actions github-actions 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.

No blocking issues found.

@github-actions github-actions 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.

No blocking issues found.

@github-actions github-actions 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.

Blocking issues found — see review comments.

@github-actions github-actions 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.

No blocking issues found.

Comment on lines +48 to +58
func (d *defaultCapabilitiesBuilder) ResourceSyncers(_ context.Context) []connectorbuilder.ResourceSyncerV2 {
return []connectorbuilder.ResourceSyncerV2{
connector.TeamBuilder(nil, nil, false),
connector.UserBuilder(nil, nil, nil, nil, nil, nil),
connector.RepositoryBuilder(nil, nil, false, false),
connector.OrgRoleBuilder(nil, nil),
connector.InvitationBuilder(connector.InvitationBuilderParams{}),
connector.APITokenBuilder(nil, nil),
connector.EnterpriseRoleBuilder(nil, nil, nil, nil),
connector.LicenseBuilder(nil, 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.

🟡 Suggestion: DefaultCapabilitiesBuilder omits OrgBuilder, which removes the org resource type (previously with CAPABILITY_PROVISION) from the advertised capabilities. The runtime GitHub.ResourceSyncers still includes OrgBuilder, so orgs are synced but no longer declared in baton_capabilities.json. Confirm this mismatch is intentional — if downstream systems use capabilities to determine expected resource types or provisioning support, removing org could be a breaking change (B5).

@github-actions github-actions 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.

No blocking issues found.

@Bencheng21
Bencheng21 dismissed github-actions[bot]’s stale review May 29, 2026 18:21

I've fixed the issue

Comment thread pkg/connector/connector.go Outdated

// DefaultCapabilitiesBuilder returns a connector builder that registers the
// resource syncers relevant to GitHub Enterprise so the generated capabilities
// are complete regardless of connector configuration. OrgBuilder is omitted

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.

why are orgs omitted for enterprise?

Comment thread baton_capabilities.json
@@ -1,6 +1,46 @@
{

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.

🟡 Suggestion: DefaultCapabilitiesBuilder now includes OrgBuilder (added in this commit), but baton_capabilities.json doesn't have an org resource type entry. The JSON should be regenerated so the checked-in capabilities match the builder.

@github-actions github-actions 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.

No blocking issues found.

@github-actions github-actions 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.

No blocking issues found.

@github-actions github-actions 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.

No blocking issues found.

@Bencheng21
Bencheng21 force-pushed the ben/update-enterprise branch from fb8e42d to 79a80bf Compare June 1, 2026 16:51

@github-actions github-actions 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.

No blocking issues found.

…erprise role/API key resources

Upgrades baton-sdk to v0.11.1 and baton-github to v0.3.7. Wires up a
DefaultCapabilitiesBuilder that exposes the new resource syncers (license,
enterprise role, API token) so capabilities advertise the full surface area.
Regenerates baton_capabilities.json and updates docs/connector.mdx to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Bencheng21
Bencheng21 force-pushed the ben/update-enterprise branch from 79a80bf to 7f9e4cb Compare June 2, 2026 22:26

@github-actions github-actions 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.

No blocking issues found.

@Bencheng21
Bencheng21 merged commit b1ff3a3 into main Jun 2, 2026
11 checks passed
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