Migrate to DefineConfigurationV2 / RunConnector (prerequisite for CE-1195) by laurenleach · Pull Request #49 · ConductorOne/baton-miro

laurenleach · 2026-08-14T23:54:38Z

Prerequisite for CE-1195. Plumbing only — no guard logic in this PR.

baton-miro was on the V1 entrypoint, so *cli.ConnectorOpts never reached connector.New — which is what blocks reading the sync resource-type filter.

What changed

cmd/: DefineConfiguration + getConnector + cmd.Execute() → config.RunConnector, with WithProvisioningEnabled() and WithDefaultCapabilitiesConnectorBuilderV2(&connector.Connector{}).
pkg/config: Config → Configuration, regenerated conf.gen.go.
connector.New: takes (*config.Miro, *cli.ConnectorOpts), returns (ConnectorBuilderV2, []connectorbuilder.Opt, error) instead of three positional strings.
ResourceSyncers → []connectorbuilder.ResourceSyncerV2; user, team and role builders move to the V2 sync signatures.

opts is accepted but deliberately unread. No field.Validate — the SDK validates via RunConnector.

Two forced renames

The V2 signatures name SDK packages that local identifiers were shadowing. Harmless under V1, compile errors under V2:

teams.go took a parameter literally named pagination — shadowing the pagination package. Now pToken.
roles_test.go had a local named resource — shadowing the SDK resource package. Now res.

A real panic the tests caught

roleBuilder.Grants returns nil, nil, nil for the empty case, so reading syncResults.NextPageToken unconditionally panicked — TestRoleBuilder_Grants_EmptyResult failed with a nil dereference. Test reads of *SyncOpResults now go through a nil check. Worth knowing since a nil results value is legitimate under V2.

Verification

Capabilities need no credentials: env -i ./connector capabilities and config regenerate byte-identical.
go build, go vet, go test ./... pass.
golangci-lint reports 0 issues, matching main.

🤖 Generated with Claude Code

Prerequisite plumbing only — no guard logic, no behaviour change. This connector was on the V1 entrypoint, so *cli.ConnectorOpts never reached connector.New. That is what blocks reading the sync resource-type filter (CE-1195). - cmd: DefineConfiguration + getConnector + cmd.Execute() -> config.RunConnector, with WithProvisioningEnabled and WithDefaultCapabilitiesConnectorBuilderV2 so `capabilities` needs no credentials. - pkg/config: `Config` -> `Configuration`; regenerated conf.gen.go. - connector.New: takes (*config.Miro, *cli.ConnectorOpts) and returns (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) instead of three positional strings. Accepted but unread — wiring it up is the follow-up. - ResourceSyncers returns []connectorbuilder.ResourceSyncerV2; the user, team and role builders move to the V2 sync signatures. Two renames were forced by the V2 signatures naming SDK packages that local identifiers shadowed: - teams.go took a parameter literally named `pagination`, which shadowed the pagination package; it is now pToken. - roles_test.go had a local named `resource` shadowing the SDK resource package; it is now res. Test reads of *SyncOpResults go through a nil check. roleBuilder.Grants returns `nil, nil, nil` for the empty case, so dereferencing the results unconditionally panicked — TestRoleBuilder_Grants_EmptyResult caught it. Verified with an empty environment: `env -i ./connector capabilities` and `config` regenerate byte-identical. golangci-lint reports 0 issues, same as main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

linear-code · 2026-08-14T23:54:41Z

CE-1195

github-actions · 2026-08-14T23:59:24Z

+		version,
+		cfg.Configuration,
+		connector.New,
+		connectorrunner.WithProvisioningEnabled(),


🟡 Suggestion: connectorrunner.WithProvisioningEnabled() is new — main passed no runner options, so runnerConfig.provisioningEnabled was only ever set from the --provisioning/BATON_PROVISIONING input. Hardcoding it means grant/revoke/account-create are now always registered on the wrapper in daemon mode, which is a real behaviour change despite the PR description saying "no behaviour change". Confidence: high on the diff, medium on impact (likely the intended fix for a provisioning-capable connector) — worth calling out in the PR body rather than changing.

github-actions · 2026-08-14T23:59:29Z

 // New returns a new instance of the connector.
-func New(ctx context.Context, accessToken string, scimAccessToken string, baseURL string) (*Connector, error) {
+// New returns a new instance of the connector.
+//
+// The *cli.ConnectorOpts parameter is part of the V2 entrypoint contract; it
+// carries runtime options such as the sync resource-type filter. It is accepted
+// but not yet read here.


🟡 Suggestion: the doc comment // New returns a new instance of the connector. is duplicated on consecutive lines. Drop the first one.

Suggested change

// New returns a new instance of the connector.

func New(ctx context.Context, accessToken string, scimAccessToken string, baseURL string) (*Connector, error) {

// New returns a new instance of the connector.

//

// The *cli.ConnectorOpts parameter is part of the V2 entrypoint contract; it

// carries runtime options such as the sync resource-type filter. It is accepted

// but not yet read here.

// New returns a new instance of the connector.

//

// The *cli.ConnectorOpts parameter is part of the V2 entrypoint contract; it

// carries runtime options such as the sync resource-type filter. It is accepted

// but not yet read here.

github-actions · 2026-08-14T23:59:34Z

 import (
 	"context"
+	"github.com/conductorone/baton-sdk/pkg/types/resource"
 	"testing"

 	v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
-	"github.com/conductorone/baton-sdk/pkg/pagination"
 )


🟡 Suggestion: the new resource import landed in the stdlib group. It's still gofmt-sorted so lint passes, but it breaks the stdlib/third-party grouping used everywhere else in this package.

Suggested change

import (

"context"

"github.com/conductorone/baton-sdk/pkg/types/resource"

"testing"

v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"

"github.com/conductorone/baton-sdk/pkg/pagination"

)

import (

"context"

"testing"

v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"

"github.com/conductorone/baton-sdk/pkg/types/resource"

)

github-actions · 2026-08-15T00:04:35Z

Connector PR Review: Migrate to DefineConfigurationV2 / RunConnector (prerequisite for CE-1195)

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 0b8093d1f023.
Review mode: incremental since d981a3e — reviewed head 06f0b354cc877ac198f082f152a7d238493cb14d, base 0b8093d1f023057962f9e549e1daa5a95c5bb004
View review run

Review Summary

The new commit removes the hardcoded connectorrunner.WithProvisioningEnabled() from cmd/baton-miro/main.go, which resolves the previous blocking finding: RunConnector forwards runner options into DefineConfigurationV2 exactly as DefineConfiguration did, and pkg/cli/commands.go re-adds WithProvisioningEnabled() based on v.GetBool("provisioning"), so provisioning is once again driven by --provisioning/BATON_PROVISIONING rather than being always-on. The full PR diff was scanned for security and correctness: the V2 signature migration is mechanically sound — the PageToken field on resource.SyncOpAttrs is a value type rather than a pointer, so pToken.PageToken.Token cannot nil-deref, and baton-sdk v0.24.1 normalises a nil *SyncOpResults to a zero value in ListResources/ListEntitlements/ListGrants, so the nil, nil, nil returns in roles.go:88 and users.go:100 are safe. go.mod/go.sum are unchanged, and the two cosmetic suggestions below carry over from the prior review and are still open.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

pkg/connector/connector.go:85-86 — the // New returns a new instance of the connector. doc comment is duplicated on consecutive lines (carried over from the previous review, still present).
pkg/connector/roles_test.go:5 — the github.com/conductorone/baton-sdk/pkg/types/resource import sits in the stdlib group between context and testing instead of the third-party group alongside the v2 import (carried over from the previous review, still present).

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/connector.go`:
- Around line 85-86: The doc comment line "// New returns a new instance of the connector." appears twice in a row directly above func New. Delete the duplicate line so the comment reads once, followed by the existing blank comment line and the explanation of the ConnectorOpts parameter.

In `pkg/connector/roles_test.go`:
- Around line 5: The import "github.com/conductorone/baton-sdk/pkg/types/resource" is placed inside the standard-library import group, between "context" and "testing". Move it down into the second (third-party) group alongside the v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" import, so the block is stdlib imports, a blank line, then third-party imports. Running gofumpt or goimports with the local prefix will do this.

github-actions

No blocking issues found.

Review feedback. The V1 DefineConfiguration call passed no connectorrunner options at all, so adding WithProvisioningEnabled() here would have flipped provisioning on unconditionally (it sets cfg.provisioningEnabled = true) rather than leaving it under whatever gates it before. That is a behaviour change, and this PR is meant to be pure entrypoint plumbing. Implementing Grant/Revoke/CreateAccount is not the same thing as the runner being told to enable provisioning, which is the mistake behind the original change. WithDefaultCapabilitiesConnectorBuilderV2 stays: that is what lets `capabilities` run without credentials. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions

No blocking issues found.

laurenleach self-assigned this Aug 14, 2026

github-actions Bot reviewed Aug 14, 2026

View reviewed changes

github-actions Bot reviewed Aug 15, 2026

View reviewed changes

laurenleach merged commit c89a856 into main Aug 15, 2026
10 checks passed

Navigation Menu

Uh oh!

Migrate to DefineConfigurationV2 / RunConnector (prerequisite for CE-1195) - #49

Migrate to DefineConfigurationV2 / RunConnector (prerequisite for CE-1195)#49
laurenleach merged 2 commits into
mainfrom
lauren/migrate-to-configuration-v2

laurenleach commented Aug 14, 2026

Uh oh!

linear-code Bot commented Aug 14, 2026

Uh oh!

github-actions Bot Aug 14, 2026

Uh oh!

github-actions Bot Aug 14, 2026

Uh oh!

github-actions Bot Aug 14, 2026

Uh oh!

github-actions Bot commented Aug 15, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

laurenleach commented Aug 14, 2026

What changed

Two forced renames

A real panic the tests caught

Verification

Uh oh!

linear-code Bot commented Aug 14, 2026

Uh oh!

github-actions Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Connector PR Review: Migrate to DefineConfigurationV2 / RunConnector (prerequisite for CE-1195)

Review Summary

Security Issues

Correctness Issues

Suggestions

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Aug 15, 2026 •

edited

Loading