fix(generated): No changes detected in API specification#371
fix(generated): No changes detected in API specification#371workos-sdk-automation[bot] wants to merge 1 commit into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
| elseif ($password instanceof PasswordHashed) { | ||
| $body['password_hash'] = $password->hash; | ||
| $body['password_hash_type'] = $password->hashType; |
There was a problem hiding this comment.
🔴 Enum object passed to JSON body instead of its string value in createUser
In createUser, $body['password_hash_type'] = $password->hashType assigns a CreateUserPasswordHashType backed enum object directly into the request body array. The old code correctly used $passwordHashType?->value to extract the string. PHP backed enums are not directly JSON-serializable — Guzzle's json option calls json_encode with JSON_THROW_ON_ERROR (lib/HttpClient.php:227), which will throw a \JsonException whenever a PasswordHashed object is passed. This completely breaks creating users with hashed passwords.
| $body['password_hash_type'] = $password->hashType; | |
| $body['password_hash_type'] = $password->hashType->value; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
| elseif ($password instanceof PasswordHashed) { | ||
| $body['password_hash'] = $password->hash; | ||
| $body['password_hash_type'] = $password->hashType; |
There was a problem hiding this comment.
🔴 Enum object passed to JSON body instead of its string value in updateUser
Same issue as in createUser: $body['password_hash_type'] = $password->hashType at line 780 assigns the raw CreateUserPasswordHashType enum object instead of calling ->value. This will cause a \JsonException when Guzzle tries to json_encode the request body (lib/HttpClient.php:227), breaking all updateUser calls that use PasswordHashed.
| $body['password_hash_type'] = $password->hashType; | |
| $body['password_hash_type'] = $password->hashType->value; |
Was this helpful? React with 👍 or 👎 to provide feedback.
Greptile SummaryThis PR is generated by
Confidence Score: 3/5Not safe to merge without acknowledging multiple breaking API changes that are mislabeled as non-breaking in the PR metadata. Two P1 findings: 11 public method renames in Authorization and a parameter rename in AdminPortal, both breaking changes for existing SDK consumers. The PR automation metadata ("Breaking: 0") is incorrect, raising concern about whether this was intentional or a tooling defect. New features (Groups service, WaitlistUser resources) look correct and well-tested. Score is pulled below P1 ceiling due to the compounded nature of the breaking changes across multiple services. lib/Service/Authorization.php (11 renamed methods), lib/Service/AdminPortal.php (renamed parameter + JSON key) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[WorkOS Client] --> B[groups]
A --> C[userManagementOrgMembershipGroups]
A --> D[authorization]
A --> E[userManagement]
A --> F[adminPortal]
B --> B1[listOrganizationGroups]
B --> B2[createOrganizationGroup]
B --> B3[getOrganizationGroup]
B --> B4[updateOrganizationGroup]
B --> B5[deleteOrganizationGroup]
B --> B6[listGroupOrganizationMemberships]
B --> B7[createGroupOrganizationMembership]
B --> B8[deleteGroupOrganizationMembership]
C --> C1[listOrganizationMembershipGroups]
D --> D1[checkPermission]
D --> D2[listResourcesForMembership - renamed]
D --> D3[listEffectivePermissions - renamed]
D --> D4[listRoleAssignments - renamed]
D --> D5[removeRoleAssignment - renamed]
E --> E1[createUser with PasswordPlaintext or PasswordHashed]
E --> E2[createOrganizationMembership with RoleSingle or RoleMultiple]
F --> F1[generateLink - itContactEmails renamed from adminEmails]
Reviews (1): Last reviewed commit: "fix(generated): No changes detected in A..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -75,7 +73,7 @@ public function check( | |||
| * @return \WorkOS\PaginatedResponse<\WorkOS\Resource\AuthorizationResource> | |||
There was a problem hiding this comment.
Eleven public methods are renamed in this file (listOrganizationMembershipResources → listResourcesForMembership, listResourcePermissions → listEffectivePermissions, listOrganizationMembershipRoleAssignments → listRoleAssignments, deleteOrganizationMembershipRoleAssignment → removeRoleAssignment, createRolePermission → addOrganizationRolePermission, updateRolePermissions → setOrganizationRolePermissions, deleteRolePermission → removeOrganizationRolePermission, getOrganizationResource → getResourceByExternalId, updateOrganizationResource → updateResourceByExternalId, deleteOrganizationResource → deleteResourceByExternalId, listResourceOrganizationMemberships → listMembershipsForResourceByExternalId). Any existing consumer calling the old method names will get a fatal "call to undefined method" PHP error at runtime. The PR metadata ("Breaking: 0") does not reflect this reality — a semver major bump and changelog entry are likely warranted.
| @@ -51,7 +51,7 @@ public function generateLink( | |||
| 'organization' => $organization, | |||
| 'intent' => $intent?->value, | |||
| 'intent_options' => $intentOptions, | |||
There was a problem hiding this comment.
$adminEmails → $itContactEmails
The parameter was renamed and the corresponding JSON key changed from admin_emails to it_contact_emails. Any caller passing adminEmails: as a named argument (or relying on positional ordering) will silently stop sending the field or receive a PHP named-argument mismatch error. This is a breaking change for existing integrations that is not reflected in the PR's "Breaking: 0" claim.
| return $this->auditLogs ??= new Service\AuditLogs($this->httpClient); | ||
| } | ||
|
|
||
| // @oagen-ignore-start — non-spec service properties (hand-maintained) | ||
| private ?Passwordless $passwordless = null; | ||
| private ?Vault $vault = null; | ||
| private ?WebhookVerification $webhookVerification = null; |
There was a problem hiding this comment.
The @oagen-ignore-start block containing six private property declarations ($passwordless, $vault, $webhookVerification, $actions, $sessionManager, $pkce) was moved from the top property-declaration section to a position after auditLogs() and before passwordless(). While this is valid PHP, some static-analysis tools and IDEs assume properties precede methods and may report false-positive "undefined property" warnings. Consider keeping property declarations grouped at the top of the class to maintain conventional PHP class structure.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
No changes detected in API specification
Spec Diff
Triggered by openapi-spec commit: 608a735533a6afbb21e58bd1f1b05b94e8b1c162