[CXH-2100] hard failure on getOrgBasePermission failure

[kans]

now, with more faulting.

[github-actions]

Connector PR Review: hard failure on getOrgBasePermission failure

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

Review Summary

Scanned the full PR diff for security and correctness. This change converts the previous silent fail-closed behavior (treating an absent default_repository_permission as none) into a hard PermissionDenied failure whenever direct-collaborators-only is enabled, plus an up-front Validate() probe (validateBasePermissionVisibility) on one confirmed-admin org in both the PAT and GitHub App credential paths. The behavior change is intentional, gated behind the opt-in direct-collaborators-only flag, and well covered by new and updated tests (connector_test.go, repository_test.go) that assert both the missing-field PermissionDenied error (with empty grants) and the base-permission none no-expansion path. The adminFound bool to firstAdminOrg refactor is correct, and the installation-token client (gh.client) is the right one to probe in App mode. No new issues found.

The prior suggestion about documenting the new credential requirement is partly addressed: the optimizeForLargeOrgs config description now states admin:org / Organization Administration read is required, but docs/connector.mdx:455-462 still does not mention it (D3), so that earlier finding stands and is not re-posted here.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

[github-actions]

No blocking issues found.

[github-actions]

No blocking issues found.

[linear-code]

CXH-2100

[mateoHernandez123]

You updated this config-field description to require the credential to read the org's default_repository_permission (admin:org scope / Organization Administration read), and validation + syncs now hard-fail without it when direct-collaborators-only is on — that's the right call. The gap is that the customer-facing docs weren't updated to match: docs/connector.mdx (credential/scopes section, ~L455-462) still doesn't list this new requirement, so an operator reading the docs won't know they need org-owner visibility before enabling this mode. Worth syncing that requirement into the docs in this PR.