[c1-squire-dev]

CXH-3517: workspace discovery depends on which token class lands in --token, and nothing guarded or documented that

What was asked

CXH-3517 reports that a customer with four Slack workspaces installed on the app synced only one: workspace discovery ran auth.teams.list against a user OAuth token, which only returns workspaces the authorizing user is a member of. The same call with the Bot User OAuth Token returns every workspace where the app is installed. The ticket asks for discovery to use the bot token, or — if the user-token behavior stays — for the docs to say the authorizing user must be a member of every workspace they want synced.

The code has moved on since the ticket's citation. The method it names (GetAuthTeamsList) stopped being called when enterprise-specific discovery was removed in December 2025 (4e9c5ab5); today all discovery goes through the slack-go client built from config.Token in NewSlack. So with a bot token configured as documented, discovery already returns every installed workspace. What had not changed: nothing stopped a deployment from putting a user token in --token — the setup docs even make that easy — and when that happens the symptom is silent: fewer workspaces, no error, no log line. This PR makes that misconfiguration impossible to miss and removes the dead code that still carried the user-token default the ticket quoted.

What changed

1. pkg/connector/client/slack.go and pkg/connector/client/path.go

Where: GetAuthTeamsList was at slack.go L207-243; UrlPathAuthTeamsList was path.go L13. Both deleted.

What changed

• Removed the uncalled GetAuthTeamsList method and the now-unused UrlPathAuthTeamsList constant.
• post's useBotToken switch was removed entirely in review: post now always authenticates with the bot token (the switch had become vestigial — every remaining caller passed true — and a lingering false branch would silently pick the SCIM user token for any future Web API call), while the SCIM methods keep calling doRequest directly with the user token.

Why

• The method had zero callers since the December 2025 refactor, and its useBotToken=false default is exactly what the ticket quotes. Keeping a boolean switch that can silently re-introduce the bug the ticket documents is worse than removing the switch. A grep over the repo confirms no references remain.

2. pkg/connector/connector.go

Where: NewSlack ~L112-113, right after the logger is set up.

What changed

• If the token passed as --token starts with xoxp (a user OAuth token), NewSlack logs one warning naming the symptom and the fix: discovery will only list the workspaces the authorizing user belongs to, and the Bot User OAuth Token is what discovers every installed workspace. Warn only — startup and sync behavior are unchanged, and unknown token shapes are not warned about.

Why

• This is the one runtime signal a misconfigured deployment will actually see. The condition is customer-actionable (a wrong credential), which the repo's own logging rules classify as a warning, and it fires once at init rather than per page.

3. pkg/connector/workspace_test.go (new)

Where: whole file; TestWorkspaceList_MultiTeamDiscovery L114, TestWorkspaceList_ErrorPropagation L157, newStubbedTeamsBuilder L93, memSessionStore L24.

What changed

• Two tests drive the real discovery path (workspaceResourceType.List) against a local stub server that answers auth.teams.list — and answers nothing else; a non-POST or wrong-path request gets a 500, so the test fails if discovery ever drifts off that endpoint.
• The discovery test seeds two workspaces and asserts one workspace resource per team, IDs derived from the Slack team ID, correct display names, and an empty next-page token. The error test makes the stub return ok: false and asserts the connector returns a typed Unauthenticated error wrapped with the connector's own error message instead of an empty result.
• A small in-memory session store stub backs the session call List makes; the vendored SDK's session helper panics on a nil store with non-empty results, so the stub is required.

Why

• Discovery is the surface the ticket is about, and it had no direct test. These pin both halves of the contract: all teams returned become resources keyed by their stable Slack IDs, and an API failure is returned rather than swallowed (an empty result here reads as "all workspaces deleted" downstream).

4. README.md and docs/connector.mdx

Where: README L23; connector.mdx L121-124 (outside the auto-generated capabilities block).

What changed

• Both docs now state the token contract in one place: the Bot User OAuth Token goes in the Access token field and is what workspace discovery uses; a user OAuth token there narrows discovery to the authorizing user's memberships; the user OAuth token belongs in the Business plus token field.

Why

• The docs previously named only "your user OAuth token" in the setup flow and never said what belongs in the Access token field — the plausible origin of the customer's misconfiguration. A customer with the reported symptom can now match their setup against the docs.

How far this is verified

• The changed packages were built, vetted and tested with the repo's targeted validation (scripts/validate-changes.sh): exit 0, both new tests green alongside the existing package tests. Repo-wide builds were not run. CI ran and is green on the final head, including verify / lint, verify / test and connector-docs / validate.
• An editing adversarial review ran on the branch and ended SHIP after two hardening fixes (the stub now rejects wrong endpoints; the error test pins a typed gRPC status). The connector rulebook's mechanical checks: 0 violations.
• The repository's own review bot judged the PR at every pushed head and passes at the current head with no unresolved threads. It took three fix passes to get here: the judge asked for the error test to pin codes.Unauthenticated exactly rather than "some gRPC status" (done), CI lint flagged a 233-character warn line (fixed by splitting the string literal, message unchanged), and the independent review below asked for two more changes, all applied.
• connector-eval (advisory): clean — a mock-based full sync of this branch was compared against the same sync on main and lost 0 / gained 0 resources, entitlements and grants, with 0 identifier changes. That is the expected result: this PR intentionally changes no sync output, only guardrails, tests and docs. (Comparison ran against a mock of the Slack API authored from its documentation, since no vendor credentials were available.)
• Smoke: cmd/baton-slack builds and starts (SMOKE PASS, partial). This repo declares no mock for the smoke harness, so no sync was exercised by the smoke itself — the mock-based sync comparison above and the tester's own mock-backed sync (which discovered every stubbed workspace through 2-page pagination) are what exercised a real sync.
• VERA (the batch's tester, in a fresh clone of the PR branch) verified all six acceptance-criteria claims WORKS with captured evidence: both new tests pass, no references to the deleted method remain, the xoxp warning fires at startup while an xoxb token stays silent, the docs name both token classes, a real mock-backed sync discovers all installed workspaces, and build + vet are clean.
• Independent deep review: CLEAR after 2 rounds — a reviewer that had seen none of the batch raised 3 nits (the PR body's token rationale was factually wrong about the surviving code — corrected; the warning prefix broke the repo's baton-slack: log convention — fixed; the vestigial useBotToken switch on post was itself the wrong-token trap this PR removes elsewhere — removed entirely, so post now always authenticates with the bot token). All three threads were answered and resolved; the judge re-passed the PR after each fix.

What to look at hardest

• The warn only fires on the xoxp prefix. A user-token shape Slack introduces without that prefix would stay silent (docs are the fallback). The check is deliberately narrow to avoid false warnings on GovSlack or future shapes.
• The deleted method is exported on the internal pkg/connector/client package. Zero in-repo callers since 4e9c5ab5; anything importing that package from outside this repo (none is known) would need to adjust.
• The discovery test's stub session store is hand-rolled. It implements the seven-method interface the SDK requires; if the interface grows, the stub needs updating too.

Linear requirements

CXH-3517 — this PR meets the ticket's acceptance criteria via its documented alternative, with the primary ask satisfied at the architectural level:

Requirement	Status	Evidence
Discovery returns every installed workspace, not only the authorizing user's memberships	✅ Met	Discovery runs on the bot token when --token is configured as documented (connector.go L122); the new tests pin that path (workspace_test.go L114). The ticket's premise predated the December 2025 refactor (4e9c5ab5) that moved discovery off the user token.
GetAuthTeamsList no longer defaults to the user token	✅ Met	Method and its URL constant deleted; zero references remain. The ticket asked for a useBotToken=true switch on that method; the method has had no callers since that refactor, so removal is the honest form of the same change.
Docs flag the authorizing-user-membership limitation (the ticket's alternative ask)	✅ Met	README L23 and connector.mdx L121-124 name both token classes and the narrowing symptom.
Misconfigurations are diagnosable, not silent	✅ Met	One-shot xoxp warning at init (connector.go L112-113).
No breaking change to synced data	✅ Met	Workspace resources still derive IDs from Slack team IDs; mock-based sync comparison shows 0 lost / 0 gained objects.

Release Notes

baton-slack now warns at startup when the access token is a user OAuth token instead of the Bot User OAuth Token, because workspace discovery only lists the workspaces the authorizing user is a member of when a user token is used — pass the Bot User OAuth Token as --token to sync every workspace where the Slack app is installed. The README and connector setup docs now state which token belongs in which field, and a long-dead code path that still carried the old user-token discovery default was removed. Synced data, resource IDs and connector behavior are unchanged.

Scope

Exactly six files: pkg/connector/client/slack.go, pkg/connector/client/path.go, pkg/connector/connector.go, pkg/connector/workspace_test.go (new), README.md, docs/connector.mdx. No config schema, vendor, resource-type, entitlement or provisioning changes.

[linear-code]

CXH-3517

[github-actions]

Superseded — see the current review report for commit 5a51c35258f0

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: full
View review run

Review Summary

This PR changes no sync output. It deletes the uncalled GetAuthTeamsList and UrlPathAuthTeamsList; a grep finds no remaining references, and the slack import is still used. NewSlack now logs a single Warn at startup when --token starts with xoxp. The PR also adds stub-server tests for workspaceResourceType.List (ListTeamsContext → auth.teams.list) and clarifies in the README and docs/connector.mdx which token goes in which field. I scanned the full diff for security and correctness and read workspace.go List and client.WrapError.

How the repo-local criteria applied:

• A / L1, L7: the new warning is customer-actionable (a wrong credential) and fires once at init, so Warn with no sampling is correct. It logs nothing secret: no token value, only the class name.
• S: discovery List still returns the wrapped error instead of an empty result, and the new error test pins that.
• B / E, BP, D: no breaking change (resource IDs still come from the Slack team ID), and the docs were updated to match.
• C, D (JSON), F: no spans, response structs or provisioning code changed, so these did not apply.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (confidence: high) pkg/connector/workspace_test.go:182: TestWorkspaceList_ErrorPropagation only checks that status.FromError(err) succeeds. The PR description says it pins the Unauthenticated code, but if invalid_auth were remapped to a different gRPC code, the test would still pass. Assert status.Code(err) == codes.Unauthenticated.

Prompt for AI agents

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

## Suggestions

In `pkg/connector/workspace_test.go`:
- Around line 180-183: The test only checks that the error is some gRPC status (`status.FromError(err)` returns ok), so it would pass even if `invalid_auth` stopped mapping to `codes.Unauthenticated`. Import `google.golang.org/grpc/codes` and assert `status.Code(err) == codes.Unauthenticated`, so the test pins the classification the PR description claims.

[github-actions]

Superseded — see the current review report for commit c00dfbcc38e2

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: full
View review run

Review Summary

This PR changes no sync output. It deletes the uncalled GetAuthTeamsList and UrlPathAuthTeamsList; a grep finds no remaining references, and the slack import is still used. NewSlack now logs a single Warn at startup when --token starts with xoxp. The PR also adds stub-server tests for workspaceResourceType.List (ListTeamsContext → auth.teams.list) and clarifies in the README and docs/connector.mdx which token goes in which field. I scanned the full diff for security and correctness and read workspace.go List and client.WrapError.

How the repo-local criteria applied:

• A / L1, L7: the new warning is customer-actionable (a wrong credential) and fires once at init, so Warn with no sampling is correct. It logs nothing secret: no token value, only the class name.
• S: discovery List still returns the wrapped error instead of an empty result, and the new error test pins that.
• B / E, BP, D: no breaking change (resource IDs still come from the Slack team ID), and the docs were updated to match.
• C, D (JSON), F: no spans, response structs or provisioning code changed, so these did not apply.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (confidence: high) pkg/connector/workspace_test.go:182: TestWorkspaceList_ErrorPropagation only checks that status.FromError(err) succeeds. The PR description says it pins the Unauthenticated code, but if invalid_auth were remapped to a different gRPC code, the test would still pass. Assert status.Code(err) == codes.Unauthenticated.

Prompt for AI agents

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

## Suggestions

In `pkg/connector/workspace_test.go`:
- Around line 180-183: The test only checks that the error is some gRPC status (`status.FromError(err)` returns ok), so it would pass even if `invalid_auth` stopped mapping to `codes.Unauthenticated`. Import `google.golang.org/grpc/codes` and assert `status.Code(err) == codes.Unauthenticated`, so the test pins the classification the PR description claims.

---

Reviewed commit: 5a51c35258f0

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit c00dfbcc38e2

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since 5a51c35
View review run

Review Summary

The PR deletes the uncalled GetAuthTeamsList client method and its UrlPathAuthTeamsList constant. It also adds a one-time startup Warn in NewSlack when --token is an xoxp user token, writes down which token goes in which field in the README and docs/connector.mdx, and adds stub-server tests for workspaceResourceType.List. The only new commit (c00dfbc) makes TestWorkspaceList_ErrorPropagation check that the error code is codes.Unauthenticated, not just that it is some gRPC status. That matches the invalid_auth → Unauthenticated mapping in pkg/connector/client/helpers.go:141-142.

I scanned the full PR diff for security and correctness issues. Nothing else in the repo references the removed method or constant (vendor excluded). The incremental artifact was complete (not partial). The repo-local criteria were applied:

• A / L1, L7: the warning is about a credential the customer can fix and fires once at init, so Warn without sampling is correct. It logs only the token class, not the token value.
• S / S1: discovery List still returns the wrapped error rather than an empty result (pkg/connector/workspace.go:93-95), and the new test checks this.
• B / D: no resource types, entitlements, scopes or endpoints changed. Sections C/E/F/G don't apply.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

• Fixed: the earlier suggestion said the error-propagation test only checked status.FromError. pkg/connector/workspace_test.go:181-183 now checks status.Code(err) == codes.Unauthenticated.

[github-actions]

Superseded — see the current review report for commit de1d015a7458

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since 5a51c35
View review run

Review Summary

The PR deletes the uncalled GetAuthTeamsList client method and its UrlPathAuthTeamsList constant. It also adds a one-time startup Warn in NewSlack when --token is an xoxp user token, writes down which token goes in which field in the README and docs/connector.mdx, and adds stub-server tests for workspaceResourceType.List. The only new commit (c00dfbc) makes TestWorkspaceList_ErrorPropagation check that the error code is codes.Unauthenticated, not just that it is some gRPC status. That matches the invalid_auth → Unauthenticated mapping in pkg/connector/client/helpers.go:141-142.

I scanned the full PR diff for security and correctness issues. Nothing else in the repo references the removed method or constant (vendor excluded). The incremental artifact was complete (not partial). The repo-local criteria were applied:

• A / L1, L7: the warning is about a credential the customer can fix and fires once at init, so Warn without sampling is correct. It logs only the token class, not the token value.
• S / S1: discovery List still returns the wrapped error rather than an empty result (pkg/connector/workspace.go:93-95), and the new test checks this.
• B / D: no resource types, entitlements, scopes or endpoints changed. Sections C/E/F/G don't apply.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

• Fixed: the earlier suggestion said the error-propagation test only checked status.FromError. pkg/connector/workspace_test.go:181-183 now checks status.Code(err) == codes.Unauthenticated.

---

Reviewed commit: c00dfbcc38e2

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit de1d015a7458

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since c00dfbc
View review run

Review Summary

The new commit only splits the xoxp startup warning in pkg/connector/connector.go:113-114 into two concatenated string literals to satisfy revive's line-length lint. The logged text and behavior are unchanged. I scanned the full PR diff again for security and correctness. In full, the PR:

• deletes the uncalled GetAuthTeamsList method and the UrlPathAuthTeamsList constant (no references remain in the repo),
• adds a one-time Warn when --token is a user OAuth token,
• adds discovery tests for workspaceResourceType.List,
• updates the token docs in the README and docs/connector.mdx.

How the repo-local criteria apply to this change:

• L1/L7: the warning is about a credential the customer can fix and fires once at init, so Warn without sampling is correct. It logs only the token class, never the token value.
• S1: discovery List still returns the wrapped error instead of an empty result (pkg/connector/workspace.go:93-95), and TestWorkspaceList_ErrorPropagation checks this, including codes.Unauthenticated.
• B/BP: resource IDs, entitlements and resource types are unchanged, so there is no breaking change.
• Sections C, D, F and G do not apply. The PR adds no spans, response structs or provisioning code, and does not change ID derivation.

The incremental diff was complete (no paths dropped, not truncated). There are no prior inline findings to audit.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

[github-actions]

Superseded — see the current review report for commit 53bf15717bfd

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since c00dfbc
View review run

Review Summary

The new commit only splits the xoxp startup warning in pkg/connector/connector.go:113-114 into two concatenated string literals to satisfy revive's line-length lint. The logged text and behavior are unchanged. I scanned the full PR diff again for security and correctness. In full, the PR:

• deletes the uncalled GetAuthTeamsList method and the UrlPathAuthTeamsList constant (no references remain in the repo),
• adds a one-time Warn when --token is a user OAuth token,
• adds discovery tests for workspaceResourceType.List,
• updates the token docs in the README and docs/connector.mdx.

How the repo-local criteria apply to this change:

• L1/L7: the warning is about a credential the customer can fix and fires once at init, so Warn without sampling is correct. It logs only the token class, never the token value.
• S1: discovery List still returns the wrapped error instead of an empty result (pkg/connector/workspace.go:93-95), and TestWorkspaceList_ErrorPropagation checks this, including codes.Unauthenticated.
• B/BP: resource IDs, entitlements and resource types are unchanged, so there is no breaking change.
• Sections C, D, F and G do not apply. The PR adds no spans, response structs or provisioning code, and does not change ID derivation.

The incremental diff was complete (no paths dropped, not truncated). There are no prior inline findings to audit.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

---

Reviewed commit: de1d015a7458

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit 53bf15717bfd

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since de1d015
View review run

Review Summary

The new commit (53bf157) changes one thing: the xoxp user-token warning in NewSlack (pkg/connector/connector.go:113) now starts with baton-slack: instead of slack:. That matches the prefix used elsewhere in the repo (group.go:218, user_group.go:151). The message is otherwise the same, and nothing else changed in behavior.

I re-scanned the full PR diff for security and correctness. It covers:

• deleting GetAuthTeamsList and UrlPathAuthTeamsList
• the one-time init warning
• the new workspace_test.go
• the README and docs/connector.mdx token guidance

Nothing in it is a problem.

I applied the repo-local criteria:

• L1/L7: the warning is about a credential the customer can fix, and it fires once at startup. So Warn is the right level and it doesn't need sampling. It logs only the token type, never the token value.
• S1: discovery List still returns the wrapped error rather than an empty result (pkg/connector/workspace.go:93-95). TestWorkspaceList_ErrorPropagation checks this.
• Sections B–G and E: none apply. The PR changes no endpoints, IDs, provisioning code, spans or response structs.

The incremental diff was complete: nothing was dropped or truncated. There were no prior findings to recheck, and I found no new issues.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

[github-actions]

Superseded — see the current review report for commit 65f2203dda38

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since de1d015
View review run

Review Summary

The new commit (53bf157) changes one thing: the xoxp user-token warning in NewSlack (pkg/connector/connector.go:113) now starts with baton-slack: instead of slack:. That matches the prefix used elsewhere in the repo (group.go:218, user_group.go:151). The message is otherwise the same, and nothing else changed in behavior.

I re-scanned the full PR diff for security and correctness. It covers:

• deleting GetAuthTeamsList and UrlPathAuthTeamsList
• the one-time init warning
• the new workspace_test.go
• the README and docs/connector.mdx token guidance

Nothing in it is a problem.

I applied the repo-local criteria:

• L1/L7: the warning is about a credential the customer can fix, and it fires once at startup. So Warn is the right level and it doesn't need sampling. It logs only the token type, never the token value.
• S1: discovery List still returns the wrapped error rather than an empty result (pkg/connector/workspace.go:93-95). TestWorkspaceList_ErrorPropagation checks this.
• Sections B–G and E: none apply. The PR changes no endpoints, IDs, provisioning code, spans or response structs.

The incremental diff was complete: nothing was dropped or truncated. There were no prior findings to recheck, and I found no new issues.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

---

Reviewed commit: 53bf15717bfd

[github-actions]

No blocking issues found — see the full review report

[github-actions]

Superseded — see the current review report for commit 65f2203dda38

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since 53bf157
View review run

Review Summary

The new commit (65f2203) removes the useBotToken parameter from Client.post (pkg/connector/client/request.go:61-78), so post now always sends c.botToken. It also drops the trailing true argument from the four callers: GetUserInfo, GetUserGroupMembers, GetUsers and GetUserGroups. Every caller already passed true, so the token each request uses is the same as before. The SCIM methods still call doRequest directly with c.token, and a repo-wide grep (excluding vendor) finds no leftover useBotToken or 5-argument post call sites. Overall, this PR deletes the dead GetAuthTeamsList/UrlPathAuthTeamsList and adds a one-time startup Warn when --token starts with xoxp. It also adds tests for workspace discovery, both the success path and error propagation, plus README/docs/connector.mdx guidance on which token goes where. I scanned the full PR diff for security and correctness. I applied the repo-local criteria as follows:

• L1/L7: the warning is about a credential the customer can fix and fires once, so Warn without sampling is correct. It never logs the token value.
• S1: workspace List still returns the error instead of an empty result, and TestWorkspaceList_ErrorPropagation checks this.
• E1/E2: no hand-wrapped errors were added.
• B1–B10: resource IDs, entitlements and endpoints are unchanged.
• D1–D4: the docs were updated.

There is no provisioning, config schema, go.mod or vendor change. The incremental diff was complete (nothing dropped or truncated).

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (confidence: high) — the PR description is now out of date. Section 1 says post's useBotToken parameter "is left in place to keep this diff minimal", and the Scope section says "Exactly six files". This commit removes the parameter and touches a seventh file, pkg/connector/client/request.go:61-78. The code is fine; the description should be updated so reviewers and release notes match what actually merges.

Prompt for AI agents

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

## Suggestions

In the PR description (not a source file):
- Section "1. pkg/connector/client/slack.go and pkg/connector/client/path.go" says the `useBotToken` parameter on `post` is left in place, and the "Scope" section lists exactly six files. Commit 65f2203 removed `useBotToken` from `Client.post` in `pkg/connector/client/request.go` (lines ~61-78), so `post` always uses the bot token. Update the description to say the parameter was removed (no behavior change, since every caller passed true) and add `pkg/connector/client/request.go` to the Scope file list.

[github-actions]

Connector PR Review: CXH-3517: discover Slack workspaces with the bot token

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 66cb9e24fdac.
Review mode: incremental since 53bf157
View review run

Review Summary

The new commit (65f2203) removes the useBotToken parameter from Client.post (pkg/connector/client/request.go:61-78), so post now always sends c.botToken. It also drops the trailing true argument from the four callers: GetUserInfo, GetUserGroupMembers, GetUsers and GetUserGroups. Every caller already passed true, so the token each request uses is the same as before. The SCIM methods still call doRequest directly with c.token, and a repo-wide grep (excluding vendor) finds no leftover useBotToken or 5-argument post call sites. Overall, this PR deletes the dead GetAuthTeamsList/UrlPathAuthTeamsList and adds a one-time startup Warn when --token starts with xoxp. It also adds tests for workspace discovery, both the success path and error propagation, plus README/docs/connector.mdx guidance on which token goes where. I scanned the full PR diff for security and correctness. I applied the repo-local criteria as follows:

• L1/L7: the warning is about a credential the customer can fix and fires once, so Warn without sampling is correct. It never logs the token value.
• S1: workspace List still returns the error instead of an empty result, and TestWorkspaceList_ErrorPropagation checks this.
• E1/E2: no hand-wrapped errors were added.
• B1–B10: resource IDs, entitlements and endpoints are unchanged.
• D1–D4: the docs were updated.

There is no provisioning, config schema, go.mod or vendor change. The incremental diff was complete (nothing dropped or truncated).

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (confidence: high) — the PR description is now out of date. Section 1 says post's useBotToken parameter "is left in place to keep this diff minimal", and the Scope section says "Exactly six files". This commit removes the parameter and touches a seventh file, pkg/connector/client/request.go:61-78. The code is fine; the description should be updated so reviewers and release notes match what actually merges.

Prompt for AI agents

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

## Suggestions

In the PR description (not a source file):
- Section "1. pkg/connector/client/slack.go and pkg/connector/client/path.go" says the `useBotToken` parameter on `post` is left in place, and the "Scope" section lists exactly six files. Commit 65f2203 removed `useBotToken` from `Client.post` in `pkg/connector/client/request.go` (lines ~61-78), so `post` always uses the bot token. Update the description to say the parameter was removed (no behavior change, since every caller passed true) and add `pkg/connector/client/request.go` to the Scope file list.

---

Reviewed commit: 65f2203dda38

[github-actions]

No blocking issues found — see the full review report