[JavierCarnelli-ConductorOne]

Promotes staging to main.

Included

• CXP-1274 Fix sync for projects with invalid chars in name #58 — fix: skip inherited workbook grants for locked projects with unfilterable names

Pending

• docs: correct the locked-project restriction and document the undocumented sync limits #59 (docs: correct the locked-project restriction and document the undocumented sync limits) will be included automatically once it merges into staging.

🤖 Generated with Claude Code

[github-actions]

Superseded — see the current review report for commit 2c96eeff51d3

Connector PR Review: Release: merge staging into main

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

Review Summary

This release PR brings #58 from staging into main. The change adds client.IsFilterSafe, which rejects empty names and names containing , or &. In projectBuilder.Grants, a locked project whose name fails that check no longer starts the projectName:eq: workbook phase that Tableau rejects. Instead, the connector logs a sampled Warn (sampledWarn, sampled at occurrences 1/10/100/every 1000, with total_occurrences). The PR also wraps the workbook-listing error with %w and the project ID, removes the unused GetProject client method and its imports, and adds README and docs/connector.mdx notes plus table-driven tests. I scanned the full PR diff for security and correctness.

I applied the trusted repo-local criteria:

• Log level (L1/L7): the condition is per-resource and the customer can fix it by renaming the project, so a sampled Warn is the right choice.
• Error wrapping (R4): the new error wrap uses %w.
• Data loss on empty results (S1–S6): the skip happens before any API call, so no error is swallowed. It does knowingly leave inherited grants incomplete; see Suggestions.
• Docs (D1–D4): docs were updated.
• Other: no changes to endpoints, scopes, resource IDs, or provisioning code. B10, provisioning, and breaking-change rules don't apply.

There were no prior findings. go.mod/go.sum are unchanged.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (medium confidence): pkg/connector/project.go:182-192. Locked projects with , or & in their names sync without their inherited workbook grants. If those grants were emitted before (for example, before a rename), they are dropped. Phase 2 already filters by workbook.Project.ID, so an unfiltered, paginated GetWorkbooks for these projects would keep the data complete instead of skipping it.

Prompt for AI agents

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

## Suggestions

In `pkg/connector/project.go`:
- Around line 182-192 and 201-205: Locked projects whose DisplayName fails client.IsFilterSafe skip the workbook phase, so their inherited workbook grants are never emitted. Instead of skipping, push the workbook phase anyway. In phase 2, apply client.WithFilter("projectName:eq:"+name) only when IsFilterSafe(name) is true; otherwise call GetWorkbooks with no filter. Keep the existing `workbook.Project.ID != projectID` check so only this project's workbooks produce grants. Update the README/docs note and the tests to match.

[github-actions]

🟡 Suggestion (confidence: medium): For locked projects whose names contain , or &, this branch skips the workbook phase, so the sync finishes with that project's inherited workbook access missing. If those grants were emitted earlier (for example, before a rename), C1 will drop them. Phase 2 already re-checks workbook.Project.ID == projectID itself, so for these projects you could page through GetWorkbooks without the name filter and keep the same ID match. That would still return complete data instead of skipping.

[github-actions]

Connector PR Review: Release: merge staging into main

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

Review Summary

This release PR brings #58 from staging into main. The change adds client.IsFilterSafe, which rejects empty names and names containing , or &. In projectBuilder.Grants, a locked project whose name fails that check no longer starts the projectName:eq: workbook phase that Tableau rejects. Instead, the connector logs a sampled Warn (sampledWarn, sampled at occurrences 1/10/100/every 1000, with total_occurrences). The PR also wraps the workbook-listing error with %w and the project ID, removes the unused GetProject client method and its imports, and adds README and docs/connector.mdx notes plus table-driven tests. I scanned the full PR diff for security and correctness.

I applied the trusted repo-local criteria:

• Log level (L1/L7): the condition is per-resource and the customer can fix it by renaming the project, so a sampled Warn is the right choice.
• Error wrapping (R4): the new error wrap uses %w.
• Data loss on empty results (S1–S6): the skip happens before any API call, so no error is swallowed. It does knowingly leave inherited grants incomplete; see Suggestions.
• Docs (D1–D4): docs were updated.
• Other: no changes to endpoints, scopes, resource IDs, or provisioning code. B10, provisioning, and breaking-change rules don't apply.

There were no prior findings. go.mod/go.sum are unchanged.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• New (medium confidence): pkg/connector/project.go:182-192. Locked projects with , or & in their names sync without their inherited workbook grants. If those grants were emitted before (for example, before a rename), they are dropped. Phase 2 already filters by workbook.Project.ID, so an unfiltered, paginated GetWorkbooks for these projects would keep the data complete instead of skipping it.

Prompt for AI agents

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

## Suggestions

In `pkg/connector/project.go`:
- Around line 182-192 and 201-205: Locked projects whose DisplayName fails client.IsFilterSafe skip the workbook phase, so their inherited workbook grants are never emitted. Instead of skipping, push the workbook phase anyway. In phase 2, apply client.WithFilter("projectName:eq:"+name) only when IsFilterSafe(name) is true; otherwise call GetWorkbooks with no filter. Keep the existing `workbook.Project.ID != projectID` check so only this project's workbooks produce grants. Update the README/docs note and the tests to match.

---

Reviewed commit: 2c96eeff51d3

[github-actions]

No blocking issues found — see the full review report