Conversation
Update/trial membership byok
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f48e79a85c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| threadId, | ||
| copilotId | ||
| }) | ||
| if (!usesOrganizationCredentials(copilot, organizationId)) { |
There was a problem hiding this comment.
Use hydrated provider when skipping membership usage
When a tenant-global copilot is selected with an organization-visible provider, input.copilot carries that org provider, but this handler reloads the copilot by id through CopilotGetOneQuery, whose handler ignores organization visibility. In that case this new guard evaluates against the tenant provider instead, calls recordUsage, and org-funded calls that passed CopilotCheckLimitHandler can still consume or fail membership points. The guard needs to use the already-hydrated copilot/provider or reload through the same visible-provider path.
Useful? React with 👍 / 👎.
| : { ...baseWhere, organizationId: IsNull() }, | ||
| relations: resolvedRelations | ||
| }) | ||
| const hydratedItems = await this.hydrateVisibleModelProviders(items, tenantId, organizationId ?? null) |
There was a problem hiding this comment.
Avoid shadowing tenant providers with incomplete org overrides
Passing the request organization into hydration makes findVisibleByCopilotIds prefer any organization provider row over the tenant provider. For users whose membership access resolves to the tenant scope, an incomplete or invalid org override for a tenant-global copilot now shadows the valid tenant credentials; the later filter still keeps the copilot because its organizationId matches tenant access (null), so callers receive an unusable provider. Only select the org provider here when it actually satisfies usesOrganizationCredentials, otherwise keep the tenant-scoped provider selected by access.
Useful? React with 👍 / 👎.
PR
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Tip
Close issue syntax:
Fixes #<issue number>orResolves #<issue number>, see documentation for more details.Checklist
Please note: ✅ we will close your PR without comment if you do not check the boxes above and provide ALL requested information.