Repository navigation
feat: multi-provider OAuth pool — OpenAI, Cursor Pro, Google AI Pro (#22) - #24
Conversation
📝 WalkthroughWalkthroughThis PR extends the OAuth account pool from single-provider (Anthropic Max) to multi-provider (Anthropic, OpenAI, Cursor, Google) via new ProviderConfig/ProviderPool/PoolRegistry architecture, refactors PoolManager as a legacy facade, implements provider-scoped REST routes with backwards-compatible legacy endpoints, generalizes the connector UI with provider-specific forms (OAuth, manual token, device code), adds ChatGPT Codex as an initial SDK provider, and updates plugin metadata to version 1.2.0. ChangesMulti-Provider OAuth Pool Architecture
REST API: Routes & Handlers
Frontend: Multi-Provider Connector Cards
ChatGPT Codex Provider Integration
Provider Registration, Metadata & Documentation
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.54)PHPStan was skipped because the config uses disallowed 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1ffc4be to
8397cf7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/OAuthPool/ProviderConfig.php (2)
23-33: 💤 Low valueClass documented as "immutable" but properties are mutable.
The docblock describes this as an "immutable value-object," but the public properties can be modified after construction since PHP 7.4 doesn't support
readonly. If PHP 8.1+ is acceptable (the plugin requires PHP 7.4 per plugin.php), consider addingreadonlyto enforce immutability. Otherwise, the docblock could be adjusted to note this is a convention rather than enforcement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OAuthPool/ProviderConfig.php` around lines 23 - 33, The class ProviderConfig is documented as an immutable value-object but exposes public mutable properties (e.g., $id); to enforce immutability on PHP 7.4, make those properties private, add a constructor that sets all fields, and expose read-only access via getter methods (e.g., getId()) so values can only be set at construction time; alternatively, if you upgrade to PHP 8.1+, mark the properties as readonly instead (change public string $id to public readonly string $id) and keep or remove getters accordingly.
127-146: 💤 Low valueConstructor allows empty
idwhich may cause silent issues.If
$args['id']is missing or empty,$this->idbecomes'', and$this->optionKeydefaults to'anthropic_max_oauth_pool_'(with trailing underscore). WhileforId()prevents lookup of unknown providers, a directly constructed config with empty id could cause subtle bugs.🛡️ Proposed validation in constructor
public function __construct(array $args) { $this->id = (string) ($args['id'] ?? ''); + if ($this->id === '') { + throw new \InvalidArgumentException('ProviderConfig requires a non-empty id.'); + } $this->label = (string) ($args['label'] ?? $this->id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OAuthPool/ProviderConfig.php` around lines 127 - 146, The constructor (__construct) currently allows an empty $args['id'] which leads to a blank $this->id and an incorrect $this->optionKey; update the __construct in ProviderConfig to validate that the computed $this->id is non-empty (e.g., trim and check), and if it's empty throw a clear InvalidArgumentException (or similar) instead of silently accepting it so callers constructing ProviderConfig directly cannot create configs with an empty id; ensure the optionKey generation (currently ('anthropic_max_oauth_pool_' . $this->id)) only runs after the id is validated so it cannot produce a trailing underscore for an empty id and mention the relationship to forId() in the error message if helpful.src/OAuthPool/ProviderPool.php (2)
509-526: 💤 Low valueComplex URL/code parsing logic handles multiple provider formats.
The code extraction handles three formats: URL with query params (OpenAI localhost callback), Anthropic's
code#stateformat, and bare codes. The conditional at line 521 prevents misinterpreting URL fragments. Consider adding a brief inline comment summarizing the three cases for future maintainers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OAuthPool/ProviderPool.php` around lines 509 - 526, The block that mutates $code and $state handles three different provider callback formats but lacks a summary comment—add a concise inline comment above the block that lists the three cases handled (1) full URL with query params where state/code are in query (e.g. OpenAI localhost callback), (2) fragment-style "code#state" (Anthropic) where the code must be cut before the #, and (3) bare code strings; also note the intent of the conditional that checks for '#' and avoids mistaking URL fragments for query parameters (the checks around preg_match('#[?&]state=...', $code, $sm), strpos($code, '#'), and preg_match('#[?&]code=...', $code, $m)).
660-662: 💤 Low valueFallback to 'unknown' email could cause account collisions.
If multiple tokens lack email/preferred_username/sub claims, they'd all map to 'unknown', causing the second token to overwrite the first via the upsert logic in
addAccount(). For Cursor tokens,subtypically contains a user identifier, so this may be acceptable in practice. Consider logging a warning when falling back to 'unknown'.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OAuthPool/ProviderPool.php` around lines 660 - 662, The current fallback that sets $email to 'unknown' when $jwt lacks email/preferred_username/sub can cause account collisions via addAccount() upserts; update the code in ProviderPool.php where $email is set to detect this all-missing case and emit a warning via the class logger (e.g. $this->logger->warning(...)) including context from $jwt (such as the raw sub or token id) before assigning 'unknown', so operators can investigate collisions; keep the existing fallback value but ensure a clear warning is logged referencing the method/variable ($email, $jwt, addAccount) so incidents are traceable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inc/rest-api.php`:
- Around line 666-675: rest_legacy_start_oauth currently calls
PoolManager::getInstance()->startOAuthFlow(...) directly and lets failures
propagate; wrap the call in the same try/catch used by rest_start_oauth so any
WP_Error returned/ thrown by startOAuthFlow is caught and returned via
rest_ensure_response($error) (or converted to a WP_REST_Response) instead of
bubbling out; update the function to catch \WP_Error (and generic Exception if
present in the new handler) and return the error through rest_ensure_response
while preserving the successful response building that uses 'authorize_url' and
'state'.
- Around line 123-131: The OAuth opaque values (parameters 'code' and 'state' in
the /exchange route and 'access_token' and 'refresh_token' in the /manual route)
must not be mutated by sanitize_text_field; replace each 'sanitize_callback' =>
'sanitize_text_field' with a 'validate_callback' that only checks presence/type
(e.g., ensures the value is a non-empty string) and returns true or a WP_Error
if invalid, so the raw token string is preserved for the provider; update the
argument definitions for the 'code', 'state', 'access_token', and
'refresh_token' keys accordingly and remove the sanitization that strips
percent-encoded characters.
In `@js/connector.jsx`:
- Around line 225-234: The Refresh button is hidden for manual-token providers
because the UI checks providerSupportsRefresh (previously derived from mode ===
'oauth-paste') instead of whether the account actually has a refresh token;
update the condition to show the button when either the provider supports
refresh or the account already has a refresh token (e.g. change instances of
providerSupportsRefresh to providerSupportsRefresh || account.hasRefresh in the
JSX that renders the Button and similar places), keep the existing disabled
logic (disabled={ isBusy || ! account.hasRefresh }) and ensure the same change
is applied to the other occurrences that render refresh controls (the other
Button render sites and any uses of supportsRefresh).
- Around line 305-310: When opening the auth popup in the connector (inside the
block that calls window.open(data.authorize_url, '_blank', 'noopener')), check
the returned window handle and whether it was blocked (null or closed) before
deciding UI; if window.open returns a falsy value or immediately closed, call
setShowManualLink(true) and avoid advancing as if the popup opened; otherwise
setShowManualLink(false). Keep existing sandbox branch using
isSandboxedEnvironment() unchanged, but replace the unconditional window.open
path with a conditional based on the popup handle to decide between manual-link
guidance and normal flow.
- Line 356: Replace the custom sprintf__() usage with WordPress i18n-safe
sprintf by importing sprintf from wp.i18n and wrapping each format string with
__() before calling sprintf; specifically, change calls like sprintf__( '%s
Account Email', label ) to sprintf( __( '%s Account Email' ), label ) for the
occurrences of sprintf__() in connector.jsx (notably the uses around the label
variable and the other two locations flagged), and ensure you add the import for
sprintf at the top so the new calls use the wp.i18n sprintf function.
In `@readme.txt`:
- Around line 39-44: Update the Installation section wording that currently
mentions “Claude Max account(s)” to reference all supported providers or use a
provider-agnostic phrase so it matches the multi-provider flow; specifically,
replace the “Claude Max account(s)” phrase in the Installation step with either
a list of supported providers (Anthropic Max, OpenAI ChatGPT/Codex, Cursor Pro,
Google AI Pro) or a neutral term like “provider account(s)” to avoid confusion
and align with the Connectors setup instructions.
In `@wp-multi-provider-test.mjs`:
- Line 8: Replace the hardcoded SHOTS path with a portable, platform-independent
directory (e.g., const SHOTS = path.join(os.tmpdir(), 'aidevops', 'screenshots')
or path.join(process.env.HOME || os.tmpdir(), '.aidevops', 'screenshots')) and
ensure the directory exists before any screenshot calls by creating it with
fs.mkdirSync(SHOTS, { recursive: true }); update every use of the SHOTS constant
(including where it's referenced later) so screenshots write to this new path
and will not fail if the directory is missing.
- Around line 61-91: The test currently silently logs and continues when it
cannot open a provider's setup UI or when probes don't match expectations, and
the locator using 'tr, li, div' plus .first() is too broad; update the code to
(1) tighten the provider card locator to target the provider row more precisely
(use a unique attribute or include p.id/p.label in a scoped selector instead of
'tr, li, div').filter({ hasText: ... }).first() so the correct card is selected,
(2) replace log(...) + continue with fail(...) when the setup button is missing
(replace the setupBtn.count() branch) and also call fail(...) inside the catch
when click/probe fails, and (3) before asserting hasOAuth/hasManual, consult or
add a mapping of expected auth type per provider (e.g., a field on PROVIDERS
like expectedAuth: 'oauth'|'manual') and assert against that mapping rather than
unconditional checks.
---
Nitpick comments:
In `@src/OAuthPool/ProviderConfig.php`:
- Around line 23-33: The class ProviderConfig is documented as an immutable
value-object but exposes public mutable properties (e.g., $id); to enforce
immutability on PHP 7.4, make those properties private, add a constructor that
sets all fields, and expose read-only access via getter methods (e.g., getId())
so values can only be set at construction time; alternatively, if you upgrade to
PHP 8.1+, mark the properties as readonly instead (change public string $id to
public readonly string $id) and keep or remove getters accordingly.
- Around line 127-146: The constructor (__construct) currently allows an empty
$args['id'] which leads to a blank $this->id and an incorrect $this->optionKey;
update the __construct in ProviderConfig to validate that the computed $this->id
is non-empty (e.g., trim and check), and if it's empty throw a clear
InvalidArgumentException (or similar) instead of silently accepting it so
callers constructing ProviderConfig directly cannot create configs with an empty
id; ensure the optionKey generation (currently ('anthropic_max_oauth_pool_' .
$this->id)) only runs after the id is validated so it cannot produce a trailing
underscore for an empty id and mention the relationship to forId() in the error
message if helpful.
In `@src/OAuthPool/ProviderPool.php`:
- Around line 509-526: The block that mutates $code and $state handles three
different provider callback formats but lacks a summary comment—add a concise
inline comment above the block that lists the three cases handled (1) full URL
with query params where state/code are in query (e.g. OpenAI localhost
callback), (2) fragment-style "code#state" (Anthropic) where the code must be
cut before the #, and (3) bare code strings; also note the intent of the
conditional that checks for '#' and avoids mistaking URL fragments for query
parameters (the checks around preg_match('#[?&]state=...', $code, $sm),
strpos($code, '#'), and preg_match('#[?&]code=...', $code, $m)).
- Around line 660-662: The current fallback that sets $email to 'unknown' when
$jwt lacks email/preferred_username/sub can cause account collisions via
addAccount() upserts; update the code in ProviderPool.php where $email is set to
detect this all-missing case and emit a warning via the class logger (e.g.
$this->logger->warning(...)) including context from $jwt (such as the raw sub or
token id) before assigning 'unknown', so operators can investigate collisions;
keep the existing fallback value but ensure a clear warning is logged
referencing the method/variable ($email, $jwt, addAccount) so incidents are
traceable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e877998-2dd7-4c9c-a7fc-57cd7aad8698
📒 Files selected for processing (10)
build/connector.jsinc/rest-api.phpjs/connector.jsxplugin.phpreadme.txtsrc/OAuthPool/PoolManager.phpsrc/OAuthPool/PoolRegistry.phpsrc/OAuthPool/ProviderConfig.phpsrc/OAuthPool/ProviderPool.phpwp-multi-provider-test.mjs
| 'code' => [ | ||
| 'required' => true, | ||
| 'type' => 'string', | ||
| 'sanitize_callback' => 'sanitize_text_field', | ||
| ], | ||
| 'state' => [ | ||
| 'required' => true, | ||
| 'type' => 'string', | ||
| 'sanitize_callback' => 'sanitize_text_field', |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What does WordPress sanitize_text_field() do to percent-encoded input, and is it appropriate for OAuth authorization codes, callback URLs, access tokens, or refresh tokens?
💡 Result:
The WordPress function sanitize_text_field is designed to clean free-form text input by stripping tags, removing line breaks, tabs, and extra whitespace, and converting single less-than characters (<) to entities [1][2][3]. Crucially, it also strips percent-encoded characters [1][3][4]. This means that if an input string contains percent-encoded data (e.g., %20, %3A), the function will remove or alter those sequences, which can corrupt the data [4]. Because of this behavior, sanitize_text_field is not appropriate for OAuth authorization codes, access tokens, refresh tokens, or callback URLs [4]. These values often rely on specific character sequences, including percent-encoded characters, to maintain their integrity and functionality. Modifying them via sanitization will likely render the tokens invalid or break the URL structure [4]. For these types of data, you should use more specific validation or sanitization methods: 1. OAuth Tokens and Authorization Codes: These should generally be treated as opaque strings. Instead of sanitizing them with text-focused functions, ensure they are handled securely (e.g., using prepared statements for database storage) and validated against expected formats (e.g., length or character set constraints) if necessary, without stripping characters [5]. 2. URLs: Use esc_url for outputting URLs or specific validation functions if you need to verify the structure of a URL provided by a user. Do not use sanitize_text_field on a full URL, as it will strip characters essential to the URL's validity [1][3]. In summary, sanitize_text_field is intended for general text fields (like titles or names) and should be avoided for any data where character-level precision is required for protocol compliance or security tokens [2][3][4].
Citations:
- 1: https://codex.wordpress.org/Function_Reference/sanitize_text_field
- 2: https://developer.wordpress.org/apis/security/sanitizing/
- 3: https://developer.wordpress.org/reference/functions/sanitize_text_field/
- 4: https://wordpress.stackexchange.com/questions/428476/why-is-sanitize-text-field-selectively-trimming-data
- 5: https://medium.com/@roman_fedyskyi/github-app-tokens-should-be-opaque-6b854484c65f
🏁 Script executed:
# Check the structure and locate the exact lines mentioned in the review
head -c 5000 inc/rest-api.php | tail -c 3000 || cat inc/rest-api.php | head -180 | tail -60Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 3092
🏁 Script executed:
# Get lines 123-131 and surrounding context
sed -n '115,140p' inc/rest-api.phpRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 834
🏁 Script executed:
# Get lines 153-161 and surrounding context
sed -n '145,170p' inc/rest-api.phpRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 946
Replace sanitize_text_field() with validation callbacks for OAuth tokens and codes.
The /exchange endpoint (code and state parameters) and /manual endpoint (access_token and refresh_token parameters) use sanitize_text_field(), which strips percent-encoded characters. This corrupts opaque OAuth values before they reach the provider, breaking token exchange and manual token injection. Use a validation callback instead to check presence without mutating the raw string.
Suggested fix
'code' => [
'required' => true,
'type' => 'string',
- 'sanitize_callback' => 'sanitize_text_field',
+ 'validate_callback' => static function ( $value ) {
+ return is_string( $value ) && trim( $value ) !== '';
+ },
],
'state' => [
'required' => true,
'type' => 'string',
- 'sanitize_callback' => 'sanitize_text_field',
+ 'validate_callback' => static function ( $value ) {
+ return is_string( $value ) && trim( $value ) !== '';
+ },
],Apply the same pattern to access_token and refresh_token in the /manual route.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'code' => [ | |
| 'required' => true, | |
| 'type' => 'string', | |
| 'sanitize_callback' => 'sanitize_text_field', | |
| ], | |
| 'state' => [ | |
| 'required' => true, | |
| 'type' => 'string', | |
| 'sanitize_callback' => 'sanitize_text_field', | |
| 'code' => [ | |
| 'required' => true, | |
| 'type' => 'string', | |
| 'validate_callback' => static function ( $value ) { | |
| return is_string( $value ) && trim( $value ) !== ''; | |
| }, | |
| ], | |
| 'state' => [ | |
| 'required' => true, | |
| 'type' => 'string', | |
| 'validate_callback' => static function ( $value ) { | |
| return is_string( $value ) && trim( $value ) !== ''; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inc/rest-api.php` around lines 123 - 131, The OAuth opaque values (parameters
'code' and 'state' in the /exchange route and 'access_token' and 'refresh_token'
in the /manual route) must not be mutated by sanitize_text_field; replace each
'sanitize_callback' => 'sanitize_text_field' with a 'validate_callback' that
only checks presence/type (e.g., ensures the value is a non-empty string) and
returns true or a WP_Error if invalid, so the raw token string is preserved for
the provider; update the argument definitions for the 'code', 'state',
'access_token', and 'refresh_token' keys accordingly and remove the sanitization
that strips percent-encoded characters.
| function rest_legacy_start_oauth( \WP_REST_Request $request ): \WP_REST_Response { | ||
| $data = PoolManager::getInstance()->startOAuthFlow( | ||
| $request->get_param( 'login_hint' ) ?: null, | ||
| $request->get_param( 'login_method' ) ?: null, | ||
| $request->get_param( 'org_uuid' ) ?: null | ||
| ); | ||
|
|
||
| // Only return the authorize URL and state (never the verifier). | ||
| return rest_ensure_response( [ | ||
| 'authorize_url' => $data['authorize_url'], | ||
| 'state' => $data['state'], | ||
| ] ); |
There was a problem hiding this comment.
Wrap the legacy /authorize handler in the same try/catch as the new path.
rest_start_oauth() now converts startOAuthFlow() failures into a structured WP_Error, but rest_legacy_start_oauth() still lets the same failure mode bubble out. That weakens the backwards-compatibility guarantee for existing Anthropic clients.
Suggested fix
-function rest_legacy_start_oauth( \WP_REST_Request $request ): \WP_REST_Response {
- $data = PoolManager::getInstance()->startOAuthFlow(
- $request->get_param( 'login_hint' ) ?: null,
- $request->get_param( 'login_method' ) ?: null,
- $request->get_param( 'org_uuid' ) ?: null
- );
+function rest_legacy_start_oauth( \WP_REST_Request $request ) {
+ try {
+ $data = PoolManager::getInstance()->startOAuthFlow(
+ $request->get_param( 'login_hint' ) ?: null,
+ $request->get_param( 'login_method' ) ?: null,
+ $request->get_param( 'org_uuid' ) ?: null
+ );
+ } catch ( \Throwable $e ) {
+ return new \WP_Error( 'oauth_start_failed', $e->getMessage(), [ 'status' => 500 ] );
+ }
return rest_ensure_response( [
'authorize_url' => $data['authorize_url'],
'state' => $data['state'],
] );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function rest_legacy_start_oauth( \WP_REST_Request $request ): \WP_REST_Response { | |
| $data = PoolManager::getInstance()->startOAuthFlow( | |
| $request->get_param( 'login_hint' ) ?: null, | |
| $request->get_param( 'login_method' ) ?: null, | |
| $request->get_param( 'org_uuid' ) ?: null | |
| ); | |
| // Only return the authorize URL and state (never the verifier). | |
| return rest_ensure_response( [ | |
| 'authorize_url' => $data['authorize_url'], | |
| 'state' => $data['state'], | |
| ] ); | |
| function rest_legacy_start_oauth( \WP_REST_Request $request ) { | |
| try { | |
| $data = PoolManager::getInstance()->startOAuthFlow( | |
| $request->get_param( 'login_hint' ) ?: null, | |
| $request->get_param( 'login_method' ) ?: null, | |
| $request->get_param( 'org_uuid' ) ?: null | |
| ); | |
| } catch ( \Throwable $e ) { | |
| return new \WP_Error( 'oauth_start_failed', $e->getMessage(), [ 'status' => 500 ] ); | |
| } | |
| return rest_ensure_response( [ | |
| 'authorize_url' => $data['authorize_url'], | |
| 'state' => $data['state'], | |
| ] ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inc/rest-api.php` around lines 666 - 675, rest_legacy_start_oauth currently
calls PoolManager::getInstance()->startOAuthFlow(...) directly and lets failures
propagate; wrap the call in the same try/catch used by rest_start_oauth so any
WP_Error returned/ thrown by startOAuthFlow is caught and returned via
rest_ensure_response($error) (or converted to a WP_REST_Response) instead of
bubbling out; update the function to catch \WP_Error (and generic Exception if
present in the new handler) and return the error through rest_ensure_response
while preserving the successful response building that uses 'authorize_url' and
'state'.
| __nextHasNoMarginBottom | ||
| __next40pxDefaultSize | ||
| label={ __( 'Claude Max Account Email' ) } | ||
| label={ sprintf__( '%s Account Email', label ) } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does WordPress wp.i18nexposesprintf, and is sprintf( __( '…%s…' ), value ) the recommended pattern for localizable placeholder strings in browser code?
💡 Result:
Yes, the WordPress @wordpress/i18n package explicitly exposes the sprintf function [1][2][3]. The pattern sprintf( ( '…%s…'), value) is the standard, recommended approach for localizing strings with dynamic placeholders in WordPress JavaScript code [3][4]. This pattern is recommended because it ensures that the entire phrase is translatable as a single unit [3]. Translators need the full context of a sentence to provide accurate translations, as word order and grammatical structures vary significantly across languages [3]. By using sprintf to inject dynamic values into a fully translated string, you avoid the common pitfall of concatenating translated fragments, which often results in broken grammar or incorrect word order in non-English languages [3]. When using this pattern: - The translation function (e.g.,, _x, _n) should wrap the entire format string containing the placeholders [3][5]. - The sprintf function then replaces the placeholders (e.g., %s, %d) with the provided dynamic values [3][6]. - You should use numbered placeholders (e.g., %1$s, %2$s) if the string contains multiple dynamic values, as this allows translators to reorder the placeholders to match the grammatical requirements of their specific language [3][6]. Official WordPress development tools, such as the @wordpress/eslint-plugin, even include rules (e.g., valid-sprintf) to enforce the correct usage of this pattern, ensuring that placeholders are properly handled and that translation functions are used correctly [5].
Citations:
- 1: https://www.npmjs.com/package/%40wordpress/i18n
- 2: https://developer.wordpress.org/block-editor/reference-guides/packages/packages-i18n/
- 3: https://mintlify.com/wordpress/gutenberg/api/packages/i18n
- 4: https://core.trac.wordpress.org/ticket/38147
- 5: https://github.com/WordPress/gutenberg/tree/HEAD/packages/eslint-plugin/docs/rules/valid-sprintf.md
- 6: https://wordpress.stackexchange.com/questions/294327/what-does-the-token-1s-in-wordpress-represent
🏁 Script executed:
# First, find the connector file
find . -name "connector.jsx" -type fRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 111
🏁 Script executed:
# Read the connector.jsx file to verify the custom sprintf__ function and its usage
cat -n js/connector.jsx | head -n 800Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 27014
Replace sprintf__() with sprintf( __( '...' ), value ) for translatable placeholder strings.
The custom sprintf__() function at line 543 prevents translation of format strings. The strings at lines 356, 533, and 708 bypass the translation function entirely, keeping them in English and preventing translators from reordering placeholders for their language's grammar.
Import sprintf from wp.i18n and wrap each format string with __() before passing it to sprintf():
Suggested fix
-const { __ } = wp.i18n;
+const { __, sprintf } = wp.i18n;
…
- label={ sprintf__( '%s Account Email', label ) }
+ label={ sprintf( __( '%s Account Email' ), label ) }
…
- { sprintf__( 'Add %s Account', label ) }
+ { sprintf( __( 'Add %s Account' ), label ) }
…
- { sprintf__( 'No %s accounts configured.', label ) }
+ { sprintf( __( 'No %s accounts configured.' ), label ) }
…
-// Tiny sprintf shim — avoids pulling in `@wordpress/i18n.sprintf`.
-function sprintf__( fmt, value ) {
- return fmt.replace( '%s', value );
-}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@js/connector.jsx` at line 356, Replace the custom sprintf__() usage with
WordPress i18n-safe sprintf by importing sprintf from wp.i18n and wrapping each
format string with __() before calling sprintf; specifically, change calls like
sprintf__( '%s Account Email', label ) to sprintf( __( '%s Account Email' ),
label ) for the occurrences of sprintf__() in connector.jsx (notably the uses
around the label variable and the other two locations flagged), and ensure you
add the import for sprintf at the top so the new calls use the wp.i18n sprintf
function.
| const BASE = process.env.WP_BASE || 'http://wordpress.local:8080'; | ||
| const ADMIN_USER = process.env.WP_USER || 'admin'; | ||
| const ADMIN_PASS = process.env.WP_PASS || 'testpass123'; | ||
| const SHOTS = '/home/dave/.aidevops/.agent-workspace/tmp'; |
There was a problem hiding this comment.
Make screenshot output path portable and ensure it exists.
Hardcoding SHOTS to a user-specific absolute path makes this smoke test fragile outside your machine; screenshot calls can fail if the directory is missing.
Proposed fix
import { chromium } from 'playwright';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
@@
-const SHOTS = '/home/dave/.aidevops/.agent-workspace/tmp';
+const SHOTS = process.env.WP_SHOTS_DIR || path.join(os.tmpdir(), 'wp-multi-provider-shots');
+fs.mkdirSync(SHOTS, { recursive: true });Also applies to: 47-48, 73-74, 109-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wp-multi-provider-test.mjs` at line 8, Replace the hardcoded SHOTS path with
a portable, platform-independent directory (e.g., const SHOTS =
path.join(os.tmpdir(), 'aidevops', 'screenshots') or path.join(process.env.HOME
|| os.tmpdir(), '.aidevops', 'screenshots')) and ensure the directory exists
before any screenshot calls by creating it with fs.mkdirSync(SHOTS, { recursive:
true }); update every use of the SHOTS constant (including where it's referenced
later) so screenshots write to this new path and will not fail if the directory
is missing.
| for (const p of PROVIDERS) { | ||
| // Find the row containing the provider label, then the "Set up" button at row-level. | ||
| const row = page.locator('tr, li, div').filter({ hasText: p.label }).filter({ has: page.locator('button, a').filter({ hasText: /Set up|Manage/i }) }).first(); | ||
| const setupBtn = row.locator('button, a').filter({ hasText: /Set up|Manage/i }).first(); | ||
| try { | ||
| const c = await setupBtn.count(); | ||
| if (!c) { | ||
| log(` ${p.id}: no setup button found`); | ||
| continue; | ||
| } | ||
| await setupBtn.click({ timeout: 5000 }); | ||
| await page.waitForTimeout(1200); | ||
| await page.screenshot({ path: `${SHOTS}/multi-card-${p.id}.png` }); | ||
|
|
||
| // Probe for expected fields | ||
| const txt = await page.locator('body').innerText(); | ||
| const hasManual = txt.includes('access_token') || txt.includes('Access token') || txt.includes('Paste'); | ||
| const hasOAuth = txt.includes('Authorize') || txt.includes('authorize') || txt.includes('OAuth') || txt.includes('paste'); | ||
| log(` ${p.id}: form opened, oauth=${hasOAuth} manual=${hasManual}`); | ||
|
|
||
| // Try to close via cancel/back button or escape | ||
| const cancelBtn = page.locator('button').filter({ hasText: /Cancel|Back|Close/i }).first(); | ||
| if (await cancelBtn.count()) { | ||
| await cancelBtn.click({ timeout: 1500 }).catch(() => {}); | ||
| } else { | ||
| await page.keyboard.press('Escape').catch(() => {}); | ||
| } | ||
| await page.waitForTimeout(500); | ||
| } catch (e) { | ||
| log(` ${p.id}: click/probe failed (${String(e).split('\n')[0]})`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the test file and explore context
find . -name "wp-multi-provider-test.mjs" -type fRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 121
🏁 Script executed:
# Get file size to decide how to read it
wc -l wp-multi-provider-test.mjs 2>/dev/null || find . -name "wp-multi-provider-test.mjs" -exec wc -l {} \;Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 123
🏁 Script executed:
# Read the beginning of the file to understand structure and imports
head -60 wp-multi-provider-test.mjs 2>/dev/null || find . -name "wp-multi-provider-test.mjs" -exec head -60 {} \;Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 2722
🏁 Script executed:
# Check for PROVIDERS definition
grep -n "PROVIDERS" wp-multi-provider-test.mjs 2>/dev/null || find . -name "wp-multi-provider-test.mjs" -exec grep -n "PROVIDERS" {} \;Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 189
🏁 Script executed:
# Search for 'fail' function usage or definition in test framework
grep -rn "fail\(" wp-multi-provider-test.mjs 2>/dev/null || rg "fail\(" --type mjs --max-count 20Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 120
🏁 Script executed:
# Read lines around 61-91 where the issue is
sed -n '55,95p' wp-multi-provider-test.mjsRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 2100
🏁 Script executed:
# Read the full file to understand all context
cat wp-multi-provider-test.mjsRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 5125
Fail the test when provider setup UI cannot be opened or expected fields are missing.
This block logs failures but continues without asserting, allowing regressions to slip through. Additionally, the tr, li, div locator is too broad and with only .first() and .hasText filtering could target the wrong card.
Replace log() + continue with the existing fail() function when the setup button is not found or when form probes fail. However, before adding assertions for hasOAuth and hasManual, the expected authentication type for each provider (OAuth vs. manual token) must be documented or verified in the connector implementations, as these are not defined in the test file.
Suggested improvements
- const row = page.locator('tr, li, div').filter({ hasText: p.label }).filter({ has: page.locator('button, a').filter({ hasText: /Set up|Manage/i }) }).first();
+ const row = page
+ .locator('tr, li, section, article, div')
+ .filter({ has: page.getByText(p.label, { exact: true }) })
+ .filter({ has: page.locator('button, a').filter({ hasText: /Set up|Manage/i }) })
+ .first();
const setupBtn = row.locator('button, a').filter({ hasText: /Set up|Manage/i }).first();
try {
const c = await setupBtn.count();
if (!c) {
- log(` ${p.id}: no setup button found`);
- continue;
+ fail(`${p.id}: setup/manage button not found`);
+ continue;
}
@@
log(` ${p.id}: form opened, oauth=${hasOAuth} manual=${hasManual}`);
+ // Add assertions only after documenting expected auth mode per provider
@@
} catch (e) {
- log(` ${p.id}: click/probe failed (${String(e).split('\n')[0]})`);
+ fail(`${p.id}: click/probe failed (${String(e).split('\n')[0]})`);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wp-multi-provider-test.mjs` around lines 61 - 91, The test currently silently
logs and continues when it cannot open a provider's setup UI or when probes
don't match expectations, and the locator using 'tr, li, div' plus .first() is
too broad; update the code to (1) tighten the provider card locator to target
the provider row more precisely (use a unique attribute or include p.id/p.label
in a scoped selector instead of 'tr, li, div').filter({ hasText: ... }).first()
so the correct card is selected, (2) replace log(...) + continue with fail(...)
when the setup button is missing (replace the setupBtn.count() branch) and also
call fail(...) inside the catch when click/probe fails, and (3) before asserting
hasOAuth/hasManual, consult or add a mapping of expected auth type per provider
(e.g., a field on PROVIDERS like expectedAuth: 'oauth'|'manual') and assert
against that mapping rather than unconditional checks.
8397cf7 to
bddf237
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@readme.txt`:
- Line 20: Update the README text that currently says SDK integration is
deferred/out-of-scope to reflect that the PR ships the ChatGPT/Codex provider
integration: find the bullet mentioning "OpenAI ChatGPT / Codex" and replace the
phrase about "SDK integration ships in a follow-up" (and the similar wording
referenced later) with a short statement that the OpenAI SDK/ChatGPT-Codex
provider is included and registered in this release (1.2.0), and ensure any
phrasing about needing a follow-up or being out-of-scope is removed or changed
to indicate it is implemented and available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6268484f-18b7-4dbe-9f40-8d08927f6e7c
📒 Files selected for processing (15)
build/connector.jsinc/provider-registration.phpinc/rest-api.phpjs/connector.jsxplugin.phpreadme.txtsrc/Authentication/ChatGptCodexOAuthRequestAuthentication.phpsrc/Metadata/ChatGptCodexModelMetadataDirectory.phpsrc/Models/ChatGptCodexTextGenerationModel.phpsrc/OAuthPool/PoolManager.phpsrc/OAuthPool/PoolRegistry.phpsrc/OAuthPool/ProviderConfig.phpsrc/OAuthPool/ProviderPool.phpsrc/Provider/ChatGptCodexProvider.phpwp-multi-provider-test.mjs
✅ Files skipped from review due to trivial changes (1)
- src/OAuthPool/PoolRegistry.php
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin.php
- src/OAuthPool/ProviderConfig.php
- src/OAuthPool/PoolManager.php
- inc/rest-api.php
- js/connector.jsx
- src/OAuthPool/ProviderPool.php
| * **Rate limit handling** -- Rate-limited accounts are automatically rotated out with cooldowns. | ||
| * **Connectors page integration** -- Manage accounts from the familiar Settings > Connectors UI. | ||
| * **Anthropic Max** -- OAuth (PKCE) flow with `claude.ai`. Pool with rotation, refresh, and cooldowns. Wired into the AI Client SDK as `ultimate-ai-connector-anthropic-max` (full text/image/tool support). | ||
| * **OpenAI ChatGPT / Codex** -- OAuth flow with `auth.openai.com`. Paste the auth code from the localhost callback. Pool storage and rotation are implemented; SDK integration ships in a follow-up. |
There was a problem hiding this comment.
Update OpenAI SDK-integration status text to match shipped behavior.
Lines 20 and 82 say SDK integration is deferred/out-of-scope, but this PR now includes and registers ChatGPT Codex provider integration. This doc mismatch will confuse users about what works in 1.2.0.
Also applies to: 82-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.txt` at line 20, Update the README text that currently says SDK
integration is deferred/out-of-scope to reflect that the PR ships the
ChatGPT/Codex provider integration: find the bullet mentioning "OpenAI ChatGPT /
Codex" and replace the phrase about "SDK integration ships in a follow-up" (and
the similar wording referenced later) with a short statement that the OpenAI
SDK/ChatGPT-Codex provider is included and registered in this release (1.2.0),
and ensure any phrasing about needing a follow-up or being out-of-scope is
removed or changed to indicate it is implemented and available.
bddf237 to
c2c14f9
Compare
Adds per-provider OAuth account pool support alongside the existing
Anthropic Max pool. Supported providers:
- Anthropic Max — OAuth PKCE, paste code. Wired into AI Client SDK.
- OpenAI ChatGPT (Codex) — OAuth device-code flow (no localhost
callback). Wired into AI Client SDK via a dedicated provider.
- Cursor Pro — Manual token paste; email derived from JWT sub.
- Google AI Pro — OAuth OOB, paste code shown by Google.
## Architecture
New classes in src/OAuthPool/:
- ProviderConfig — per-provider OAuth constants (client id, endpoints,
scopes, redirect URI, user-agent, health-check URL, plus new
device-code fields: supportsDeviceCode, deviceCodeUrl,
deviceTokenUrl, deviceCallbackUri).
- ProviderPool — generic pool: load/save, list, addAccount,
addAccountManual, removeAccount, markRateLimited, refreshAccount,
healthCheck, startOAuthFlow (PKCE), exchangeCode, refreshTokens,
getActiveTokenWithMeta (returns token+email+accountId for Codex
header injection), startDeviceCode/pollDeviceCode (transient-backed
session keys, JWT-derived chatgpt_account_id extraction).
- PoolRegistry — memoised pool factory + summary() for /providers index.
- PoolManager — thin facade over PoolRegistry::pool('anthropic').
Preserves every 1.0/1.1 public method and constant for back-compat.
ChatGPT Codex provider analogue (mirrors AnthropicMax):
- src/Authentication/ChatGptCodexOAuthRequestAuthentication.php —
injects Authorization: Bearer, chatgpt-account-id, Originator headers
(Originator filterable via anthropic_max_chatgpt_codex_originator).
- src/Provider/ChatGptCodexProvider.php — id
ultimate-ai-connector-chatgpt-codex, baseUrl
https://chatgpt.com/backend-api/codex.
- src/Metadata/ChatGptCodexModelMetadataDirectory.php — hardcoded
model list (gpt-5.4-mini, gpt-5.4, gpt-5.5, gpt-5.2), filter
anthropic_max_chatgpt_codex_models.
- src/Models/ChatGptCodexTextGenerationModel.php — Responses API
request, SSE consumer aggregating response.completed with
response.output_text.delta fallback. Drops max_output_tokens /
max_tokens defensively. Direct wp_remote_post (timeout 120s).
- inc/provider-registration.php registers both AnthropicMax and
ChatGptCodex on init priority 5 when their pools have ≥1 account.
REST API (inc/rest-api.php):
- New per-provider routes under /anthropic-max-pool/v1/{provider}/:
accounts, authorize, exchange, manual, accounts/remove,
accounts/refresh, health, device-start, device-poll.
- New /anthropic-max-pool/v1/providers index endpoint.
- All legacy Anthropic-only routes (/accounts, /authorize, /exchange,
/accounts/remove, /accounts/refresh, /health) preserved unchanged.
UI (js/connector.jsx):
- Four registerConnector() cards driven by a PROVIDERS map.
- Shared ProviderConnectorCard, OAuthAddForm (oauth-paste mode),
ManualTokenAddForm (manual-token mode), DeviceCodeForm (device-code
mode) with large monospace code, Copy button, M:SS countdown,
polling at interval_ms, success/expired/error phases with Try Again.
- apiFetchErrorMessage + sanitizeErrorMessage harden the error path:
any HTML response body (e.g. Cloudflare "Just a moment…" challenge
pages) is replaced with a friendly operator-facing notice instead
of rendering raw markup.
Error handling (src/OAuthPool/ProviderPool.php):
- stringifyTokenError() detects HTML bodies via looksLikeHtml() and
returns a sanitised message while logging the full body to PHP
error log. RFC 6749 JSON paths (error_description, error, message)
unchanged; plain-text fallback unchanged.
## Backward compatibility
- anthropic_max_oauth_pool option key unchanged.
- New providers use anthropic_max_oauth_pool_{openai,cursor,google}.
- PoolManager public API unchanged.
- Legacy REST routes unchanged.
## Version
Bumps plugin to 1.2.0.
Resolves #22.
c2c14f9 to
7824442
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@readme.txt`:
- Around line 42-43: Update the README bullets to separate OpenAI's device-code
flow from browser-based OAuth/paste-code instructions: change the combined
"Anthropic / OpenAI / Google: authorize in a browser, paste the returned code."
into two bullets—one that states "OpenAI: use the device-code flow (follow
instructions to complete authorization on another device)" and another for
browser OAuth providers like "Anthropic / Google: authorize in a browser and
paste the returned code." Keep the Cursor line as-is ("Cursor: paste your access
token (and optional refresh token) from the Cursor IDE.") so each provider's
auth method is clearly and accurately described.
In `@src/OAuthPool/ProviderConfig.php`:
- Line 312: Replace the deprecated OOB value for the redirect URI in the
ProviderConfig (the 'redirectUri' entry currently set to
'urn:ietf:wg:oauth:2.0:oob') with a loopback callback URL (e.g.
'http://127.0.0.1:PORT/callback' or 'http://localhost:PORT/callback'), update
any configuration or constant that builds that URI, and ensure the application
starts a local HTTP listener to receive the OAuth callback and exchange the
code; adjust any code that expects the OOB flow (token handling in
ProviderConfig or OAuthPool classes) to handle the HTTP redirect/callback flow
instead.
- Line 314: The Google OAuth token exchange is currently sending JSON; update
the provider configuration entry that sets 'tokenContentType' for the Google AI
Pro provider to use 'application/x-www-form-urlencoded' instead of
'application/json' so the token endpoint receives RFC 6749-compliant
form-encoded data. Locate the provider config array/object that includes the
'tokenContentType' key (the Google/Google AI Pro provider entry in
ProviderConfig.php), change its value to 'application/x-www-form-urlencoded',
and ensure the token request builder (the code that reads 'tokenContentType'
when posting to the token endpoint) will send form-encoded body parameters
accordingly.
In `@src/OAuthPool/ProviderPool.php`:
- Around line 154-205: The pool read-modify-write sequence (using loadPool,
clearExpiredCooldowns, selecting a best account, updating
status/lastUsed/expires/refresh and calling savePool) is not synchronized,
causing races under concurrent requests; protect these critical sections by
adding a per-provider lock or CAS around the entire selection-and-update flow
(wrap loadPool → account selection → potential refreshTokens call → pool
mutation → savePool) so only one worker can modify the option row at a time, or
implement optimistic CAS compare-and-swap using a version/timestamp field on the
pool to detect conflicts and retry the operation (apply the same protection to
the other similar blocks referenced around lines 355-370 and 391-418).
- Around line 833-838: The current error_log call is leaking sensitive device
login tokens and account identifiers ($user_code and later email/accountId);
update the logging in ProviderPool (the sprintf that uses $this->config->id,
$session_key, $user_code) to remove or redact $user_code and any account
identifiers—log only non-sensitive context such as $this->config->id and
$session_key (or a one-way hash/short fingerprint of $user_code if you must
trace it) and ensure the same change is applied to the other occurrences
mentioned (around lines 989-995) so no raw device codes or PII are written to
the PHP error log.
- Around line 165-207: The selection loop currently picks the LRU account into
$best without considering expiry, so an expired access token can be chosen and
later returned or cause an early null; update the foreach over $pool['accounts']
to skip accounts whose access token is expired (expires > 0 && expires <=
$now_ms) unless they carry a refresh token (i.e. only consider expired accounts
as candidates if they have a refresh), and when attempting to refresh via
refreshTokens($best['refresh']) if refresh fails do not immediately set status
'refresh-failed' and return null — instead mark that account appropriately and
continue scanning the remaining accounts in $pool to find another valid
candidate, using the same $best/$best_index logic and still persisting changes
with savePool($pool).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7376c735-81e1-469b-8a32-1cf3b8334edc
📒 Files selected for processing (15)
.gitignorebuild/connector.jsinc/provider-registration.phpinc/rest-api.phpjs/connector.jsxplugin.phpreadme.txtsrc/Authentication/ChatGptCodexOAuthRequestAuthentication.phpsrc/Metadata/ChatGptCodexModelMetadataDirectory.phpsrc/Models/ChatGptCodexTextGenerationModel.phpsrc/OAuthPool/PoolManager.phpsrc/OAuthPool/PoolRegistry.phpsrc/OAuthPool/ProviderConfig.phpsrc/OAuthPool/ProviderPool.phpsrc/Provider/ChatGptCodexProvider.php
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (9)
- plugin.php
- src/Metadata/ChatGptCodexModelMetadataDirectory.php
- src/OAuthPool/PoolRegistry.php
- inc/provider-registration.php
- src/OAuthPool/PoolManager.php
- src/Authentication/ChatGptCodexOAuthRequestAuthentication.php
- src/Provider/ChatGptCodexProvider.php
- src/Models/ChatGptCodexTextGenerationModel.php
- js/connector.jsx
| * Anthropic / OpenAI / Google: authorize in a browser, paste the returned code. | ||
| * Cursor: paste your access token (and optional refresh token) from the Cursor IDE. |
There was a problem hiding this comment.
Split OpenAI setup instructions from OAuth paste-code providers.
OpenAI now uses the device-code flow, so grouping it with “authorize + paste returned code” is misleading.
Suggested wording
- * Anthropic / OpenAI / Google: authorize in a browser, paste the returned code.
+ * Anthropic / Google: authorize in a browser, then paste the returned code.
+ * OpenAI: open the device link, enter the device code, and wait for automatic completion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.txt` around lines 42 - 43, Update the README bullets to separate
OpenAI's device-code flow from browser-based OAuth/paste-code instructions:
change the combined "Anthropic / OpenAI / Google: authorize in a browser, paste
the returned code." into two bullets—one that states "OpenAI: use the
device-code flow (follow instructions to complete authorization on another
device)" and another for browser OAuth providers like "Anthropic / Google:
authorize in a browser and paste the returned code." Keep the Cursor line as-is
("Cursor: paste your access token (and optional refresh token) from the Cursor
IDE.") so each provider's auth method is clearly and accurately described.
| 'clientId' => '681255809395-oo8ft6t5t0rnmhfqgpnkqtev5b9a2i5j.apps.googleusercontent.com', | ||
| 'authorizeUrl' => 'https://accounts.google.com/o/oauth2/v2/auth', | ||
| 'tokenEndpoint' => 'https://oauth2.googleapis.com/token', | ||
| 'redirectUri' => 'urn:ietf:wg:oauth:2.0:oob', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the ProviderConfig.php file
find . -name "ProviderConfig.php" -type fRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 127
🏁 Script executed:
# Check the context around line 312 in the Google provider config
sed -n '300,330p' src/OAuthPool/ProviderConfig.phpRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 1668
🌐 Web query:
Google OAuth 2.0 OOB out-of-band redirect URI deprecation 2024 2025
💡 Result:
Google deprecated the OAuth 2.0 Out-of-Band (OOB) flow to mitigate risks associated with phishing and app impersonation [1][2]. This legacy flow, which allowed native clients without a redirect URI to manually copy and paste authorization codes, is no longer supported [1][2]. The deprecation process followed a strict timeline: - February 28, 2022: New OAuth usage of the OOB flow was blocked [1][2]. - September 5, 2022: A user-facing warning message began appearing for non-compliant requests [1][2]. - October 3, 2022: The OOB flow was deprecated for existing clients [1][2]. - January 31, 2023: All remaining OOB usage, including for clients that had received temporary extensions, was fully blocked [1][3]. As of May 2026, the OOB flow is completely non-functional [1]. Developers who previously relied on OOB for native or desktop applications must migrate to more secure alternatives, such as using loopback IP addresses (e.g., http://127.0.0.1) or platform-specific SDKs (e.g., Google Sign-In for Android, iOS, or Chrome Identity API) [2][4]. Applications attempting to use OOB redirect URIs (such as urn:ietf:wg:oauth:2.0:oob) will fail [1].
Citations:
- 1: https://developers.google.com/identity/protocols/oauth2/resources/oob-migration
- 2: https://developers.googleblog.com/en/making-google-oauth-interactions-safer-by-using-more-secure-oauth-flows
- 3: Out-of-bound flow is being deprecated r-lib/gargle#202
- 4: https://stackoverflow.com/questions/72141009/how-to-use-google-oauth2-0-without-out-of-band-oob
🌐 Web query:
Google OAuth installed app loopback redirect URI recommended alternative
💡 Result:
For desktop applications, the loopback IP address flow (using http://127.0.0.1 or http://localhost) remains the recommended mechanism for obtaining authorization codes [1]. While Google has restricted or disallowed this flow for specific client types like Android, iOS, and Chrome apps due to security concerns [2], it is still supported and encouraged for standard desktop applications (macOS, Linux, and Windows) [1]. If you are developing a desktop application and need to move away from or optimize your use of the loopback flow, consider the following alternatives and best practices: 1. Use Platform-Specific SDKs: The most secure and recommended approach is to use official Google SDKs (e.g., Google Sign-In for Android/iOS) where available [3][2]. These libraries abstract the complexity of the OAuth flow and handle security requirements automatically [3]. 2. Implement PKCE (Proof Key for Code Exchange): Regardless of the redirect method, you should always use PKCE for native desktop applications [4]. PKCE adds a layer of security by ensuring that the authorization code cannot be intercepted and used by a malicious actor, even if the redirect URI is compromised [4]. 3. Avoid Out-of-Band (OOB) Flow: The OOB flow (urn:ietf:wg:oauth:2.0:oob) is deprecated and has been blocked by Google [2][5]. Do not attempt to use this as an alternative to the loopback flow [2]. 4. Ensure Proper Configuration: If you continue to use the loopback flow, ensure your application is correctly registered in the Google Cloud Console as a Desktop app [1]. The redirect URI must exactly match the value configured in your client's credentials [1]. While some developers have attempted to use custom redirect URIs or web-based client types to bypass limitations, these methods are generally discouraged as they may violate security policies or lead to redirect_uri_mismatch errors [6]. If you are encountering issues with the loopback flow, verify that your application is listening on the local interface correctly and that no firewalls are blocking the connection [1]. For further guidance on secure implementation, refer to the official Google Identity documentation for native applications [1][4].
Citations:
- 1: https://developers.google.com/identity/protocols/oauth2/native-app
- 2: https://developers.googleblog.com/en/making-google-oauth-interactions-safer-by-using-more-secure-oauth-flows
- 3: https://developers.google.com/identity/protocols/oauth2/resources/loopback-migration
- 4: https://developers.google.com/identity/protocols/oauth2/resources/best-practices
- 5: Google depricates "OAuth out-of-band (OOB) flow" vitalif/grive2#362
- 6: https://stackoverflow.com/questions/60743080/using-an-external-redirect-uri-with-googles-oauth2-for-desktop-apps
Replace the blocked Google OOB redirect with a loopback flow.
Google completely blocked the urn:ietf:wg:oauth:2.0:oob redirect URI as of January 31, 2023. This provider is currently non-functional in production. Migrate to the loopback IP address flow (http://127.0.0.1 or http://localhost) as documented by Google for installed applications. Note that this requires the application to run a local HTTP server to handle the OAuth callback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/OAuthPool/ProviderConfig.php` at line 312, Replace the deprecated OOB
value for the redirect URI in the ProviderConfig (the 'redirectUri' entry
currently set to 'urn:ietf:wg:oauth:2.0:oob') with a loopback callback URL (e.g.
'http://127.0.0.1:PORT/callback' or 'http://localhost:PORT/callback'), update
any configuration or constant that builds that URI, and ensure the application
starts a local HTTP listener to receive the OAuth callback and exchange the
code; adjust any code that expects the OOB flow (token handling in
ProviderConfig or OAuthPool classes) to handle the HTTP redirect/callback flow
instead.
| 'tokenEndpoint' => 'https://oauth2.googleapis.com/token', | ||
| 'redirectUri' => 'urn:ietf:wg:oauth:2.0:oob', | ||
| 'scopes' => 'https://www.googleapis.com/auth/generative-language https://www.googleapis.com/auth/cloud-platform openid email profile', | ||
| 'tokenContentType' => 'application/json', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the ProviderConfig.php file
fd -t f "ProviderConfig.php" | head -5Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 125
🏁 Script executed:
# Get the file structure
git ls-files | head -20Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 399
🏁 Script executed:
# Examine the ProviderConfig.php file - check its size first
wc -l src/OAuthPool/ProviderConfig.phpRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 129
🏁 Script executed:
# Read around line 314 to see the context
sed -n '300,330p' src/OAuthPool/ProviderConfig.phpRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 1668
🏁 Script executed:
# Find the buildTokenRequestBody method
rg -n "buildTokenRequestBody" --type phpRepository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 469
🏁 Script executed:
# Also search for where tokenContentType is used
rg -n "tokenContentType" --type php -A 5 -B 2Repository: Ultimate-Multisite/ai-provider-for-anthropic-max
Length of output: 7291
🌐 Web query:
Google OAuth2 token endpoint content-type application/x-www-form-urlencoded vs application/json
💡 Result:
When interacting with the Google OAuth 2.0 token endpoint, you must use the application/x-www-form-urlencoded content type for your POST requests [1]. The OAuth 2.0 specification (RFC 6749) explicitly mandates that requests to the token endpoint use the application/x-www-form-urlencoded format [2][3]. Google's implementation strictly adheres to this standard [1]. If you attempt to send a request with application/json, the server will fail to parse the parameters (such as grant_type, client_id, or code), resulting in an error [4][5][6]. In contrast, the response from the Google OAuth 2.0 token endpoint will be in application/json format [1][2]. Summary of requirements: - Request Content-Type: application/x-www-form-urlencoded [1][2] - Response Content-Type: application/json [1][2] If you are using tools like curl or standard HTTP libraries, ensure you explicitly set the header Content-Type: application/x-www-form-urlencoded and format your request body as a query string (e.g., param1=value1¶m2=value2) rather than a JSON object [4][5][1].
Citations:
- 1: https://developers.google.com/identity/protocols/oauth2/web-server
- 2: https://www.rfc-editor.org/rfc/rfc6749.txt
- 3: https://datatracker.ietf.org/doc/html/rfc6749
- 4: https://stackoverflow.com/questions/37107847/how-to-get-google-oauth-2-0-access-token-directly-using-curl-without-using-goo?noredirect=1
- 5: https://stackoverflow.com/questions/25030312/google-oauth2-required-parameter-is-missing-grant-type/25042391
- 6: https://stackoverflow.com/questions/32107881/unsupported-content-with-type-multipart-form-data-in-google-plus-api-oauth2
Change Google OAuth token requests to form-urlencoded format.
Google's OAuth2 token endpoint requires application/x-www-form-urlencoded per RFC 6749 and Google's official specification. The current application/json setting causes token exchange requests to fail because the parameters cannot be parsed by Google's server. This breaks OAuth authentication for the Google AI Pro provider.
Suggested fix
- 'tokenContentType' => 'application/json',
+ 'tokenContentType' => 'application/x-www-form-urlencoded',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'tokenContentType' => 'application/json', | |
| 'tokenContentType' => 'application/x-www-form-urlencoded', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/OAuthPool/ProviderConfig.php` at line 314, The Google OAuth token
exchange is currently sending JSON; update the provider configuration entry that
sets 'tokenContentType' for the Google AI Pro provider to use
'application/x-www-form-urlencoded' instead of 'application/json' so the token
endpoint receives RFC 6749-compliant form-encoded data. Locate the provider
config array/object that includes the 'tokenContentType' key (the Google/Google
AI Pro provider entry in ProviderConfig.php), change its value to
'application/x-www-form-urlencoded', and ensure the token request builder (the
code that reads 'tokenContentType' when posting to the token endpoint) will send
form-encoded body parameters accordingly.
| $pool = $this->loadPool(); | ||
| $now_ms = $this->nowMs(); | ||
|
|
||
| $changed = $this->clearExpiredCooldowns($pool, $now_ms); | ||
| if ($changed) { | ||
| $this->savePool($pool); | ||
| } | ||
|
|
||
| $best = null; | ||
| $best_index = -1; | ||
|
|
||
| foreach ($pool['accounts'] as $index => $account) { | ||
| $status = $account['status'] ?? 'idle'; | ||
| if ($status === 'rate-limited') { | ||
| continue; | ||
| } | ||
| if (empty($account['access'])) { | ||
| continue; | ||
| } | ||
| if ($best === null || ($account['lastUsed'] ?? '') < ($best['lastUsed'] ?? '')) { | ||
| $best = $account; | ||
| $best_index = $index; | ||
| } | ||
| } | ||
|
|
||
| if ($best === null) { | ||
| return null; | ||
| } | ||
|
|
||
| $expires = $best['expires'] ?? 0; | ||
| if ($expires > 0 && $expires <= $now_ms && !empty($best['refresh'])) { | ||
| $refreshed = $this->refreshTokens($best['refresh']); | ||
| if ($refreshed !== null) { | ||
| $pool['accounts'][$best_index]['access'] = $refreshed['access_token']; | ||
| $pool['accounts'][$best_index]['expires'] = $now_ms + ($refreshed['expires_in'] * 1000); | ||
| $pool['accounts'][$best_index]['status'] = 'active'; | ||
| $pool['accounts'][$best_index]['lastUsed'] = gmdate('Y-m-d\TH:i:s\Z'); | ||
| if (!empty($refreshed['refresh_token'])) { | ||
| $pool['accounts'][$best_index]['refresh'] = $refreshed['refresh_token']; | ||
| } | ||
| $this->savePool($pool); | ||
| return ['token' => $refreshed['access_token'], 'email' => $best['email'] ?? '']; | ||
| } | ||
|
|
||
| $pool['accounts'][$best_index]['status'] = 'refresh-failed'; | ||
| $this->savePool($pool); | ||
| return null; | ||
| } | ||
|
|
||
| $pool['accounts'][$best_index]['status'] = 'active'; | ||
| $pool['accounts'][$best_index]['lastUsed'] = gmdate('Y-m-d\TH:i:s\Z'); | ||
| $this->savePool($pool); |
There was a problem hiding this comment.
Protect pool mutations with a per-provider lock or CAS.
These paths all do a read-modify-write against the same option row with no synchronization. Concurrent requests can select the same account, then overwrite each other's lastUsed, cooldown, or refreshed token state, which breaks the rotation and rate-limit semantics once traffic is parallel.
Also applies to: 355-370, 391-418
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/OAuthPool/ProviderPool.php` around lines 154 - 205, The pool
read-modify-write sequence (using loadPool, clearExpiredCooldowns, selecting a
best account, updating status/lastUsed/expires/refresh and calling savePool) is
not synchronized, causing races under concurrent requests; protect these
critical sections by adding a per-provider lock or CAS around the entire
selection-and-update flow (wrap loadPool → account selection → potential
refreshTokens call → pool mutation → savePool) so only one worker can modify the
option row at a time, or implement optimistic CAS compare-and-swap using a
version/timestamp field on the pool to detect conflicts and retry the operation
(apply the same protection to the other similar blocks referenced around lines
355-370 and 391-418).
| foreach ($pool['accounts'] as $index => $account) { | ||
| $status = $account['status'] ?? 'idle'; | ||
| if ($status === 'rate-limited') { | ||
| continue; | ||
| } | ||
| if (empty($account['access'])) { | ||
| continue; | ||
| } | ||
| if ($best === null || ($account['lastUsed'] ?? '') < ($best['lastUsed'] ?? '')) { | ||
| $best = $account; | ||
| $best_index = $index; | ||
| } | ||
| } | ||
|
|
||
| if ($best === null) { | ||
| return null; | ||
| } | ||
|
|
||
| $expires = $best['expires'] ?? 0; | ||
| if ($expires > 0 && $expires <= $now_ms && !empty($best['refresh'])) { | ||
| $refreshed = $this->refreshTokens($best['refresh']); | ||
| if ($refreshed !== null) { | ||
| $pool['accounts'][$best_index]['access'] = $refreshed['access_token']; | ||
| $pool['accounts'][$best_index]['expires'] = $now_ms + ($refreshed['expires_in'] * 1000); | ||
| $pool['accounts'][$best_index]['status'] = 'active'; | ||
| $pool['accounts'][$best_index]['lastUsed'] = gmdate('Y-m-d\TH:i:s\Z'); | ||
| if (!empty($refreshed['refresh_token'])) { | ||
| $pool['accounts'][$best_index]['refresh'] = $refreshed['refresh_token']; | ||
| } | ||
| $this->savePool($pool); | ||
| return ['token' => $refreshed['access_token'], 'email' => $best['email'] ?? '']; | ||
| } | ||
|
|
||
| $pool['accounts'][$best_index]['status'] = 'refresh-failed'; | ||
| $this->savePool($pool); | ||
| return null; | ||
| } | ||
|
|
||
| $pool['accounts'][$best_index]['status'] = 'active'; | ||
| $pool['accounts'][$best_index]['lastUsed'] = gmdate('Y-m-d\TH:i:s\Z'); | ||
| $this->savePool($pool); | ||
|
|
||
| return ['token' => $best['access'], 'email' => $best['email'] ?? '']; |
There was a problem hiding this comment.
Keep scanning when the LRU account is already expired.
This selection path ignores expiry when choosing $best, so the least-recently-used account can win even when its access token is already dead. The method then either returns that expired token as active when no refresh token exists, or returns null after a failed refresh instead of falling through to another usable account later in the pool.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/OAuthPool/ProviderPool.php` around lines 165 - 207, The selection loop
currently picks the LRU account into $best without considering expiry, so an
expired access token can be chosen and later returned or cause an early null;
update the foreach over $pool['accounts'] to skip accounts whose access token is
expired (expires > 0 && expires <= $now_ms) unless they carry a refresh token
(i.e. only consider expired accounts as candidates if they have a refresh), and
when attempting to refresh via refreshTokens($best['refresh']) if refresh fails
do not immediately set status 'refresh-failed' and return null — instead mark
that account appropriately and continue scanning the remaining accounts in $pool
to find another valid candidate, using the same $best/$best_index logic and
still persisting changes with savePool($pool).
| error_log(sprintf( | ||
| '[anthropic-max:%s] Device code started session=%s user_code=%s', | ||
| $this->config->id, | ||
| $session_key, | ||
| $user_code | ||
| )); |
There was a problem hiding this comment.
Redact device codes and account identifiers from logs.
user_code is enough to finish the device login during its validity window, and the completion log writes email/accountId as well. Shipping those values to the PHP error log creates an unnecessary credential and PII leak outside the pool storage.
Suggested fix
- error_log(sprintf(
- '[anthropic-max:%s] Device code started session=%s user_code=%s',
- $this->config->id,
- $session_key,
- $user_code
- ));
+ error_log(sprintf(
+ '[anthropic-max:%s] Device code started',
+ $this->config->id
+ ));
…
- error_log(sprintf(
- '[anthropic-max:%s] Device code completed session=%s email=%s accountId=%s',
- $this->config->id,
- $session_key,
- $email,
- $account_id ?? 'none'
- ));
+ error_log(sprintf(
+ '[anthropic-max:%s] Device code completed',
+ $this->config->id
+ ));Also applies to: 989-995
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/OAuthPool/ProviderPool.php` around lines 833 - 838, The current error_log
call is leaking sensitive device login tokens and account identifiers
($user_code and later email/accountId); update the logging in ProviderPool (the
sprintf that uses $this->config->id, $session_key, $user_code) to remove or
redact $user_code and any account identifiers—log only non-sensitive context
such as $this->config->id and $session_key (or a one-way hash/short fingerprint
of $user_code if you must trace it) and ensure the same change is applied to the
other occurrences mentioned (around lines 989-995) so no raw device codes or PII
are written to the PHP error log.
superdav42
added
the
review-feedback-scanned
1 participant
Summary
Adds multi-provider OAuth account pool support — OpenAI ChatGPT/Codex, Cursor Pro, and Google AI Pro alongside the existing Anthropic Max pool.
Resolves #22.
What this enables
Users with paid AI subscriptions (ChatGPT Plus/Pro, Codex, Cursor Pro, Google AI Pro/Ultra) can use those plans from WordPress without per-token API billing, with the same pool rotation and auto-refresh as the existing Anthropic Max flow.
anthropic_max_oauth_pool(unchanged)localhost:1455/auth/callbackanthropic_max_oauth_pool_openaianthropic_max_oauth_pool_cursoranthropic_max_oauth_pool_googleArchitecture
src/OAuthPool/ProviderConfig.phpsrc/OAuthPool/ProviderPool.phpsrc/OAuthPool/PoolRegistry.phpsrc/OAuthPool/PoolManager.phpinc/rest-api.phpjs/connector.jsxBackward compatibility
anthropic_max_oauth_pooloption key unchanged./anthropic-max-pool/v1/routes preserved with identical shapes.Out of scope
AI Client SDK provider classes for OpenAI/Cursor/Google are out of scope for this PR — pool storage and rotation are fully implemented, SDK integration ships in a follow-up. The
localhost:1455OAuth callback server is also out of scope; users paste the code from the browser URL, matching theoauth-pool-add.shheadless flow.Verification
REST smoke tests
All endpoints tested against a live WordPress install with cookie + nonce auth:
GET /providersGET /openai/authorizehttps://auth.openai.com/oauth/authorize?…&redirect_uri=http://localhost:1455/auth/callbackGET /google/authorizehttps://accounts.google.com/o/oauth2/v2/auth?…&redirect_uri=urn:ietf:wg:oauth:2.0:oobGET /anthropic/authorizehttps://claude.ai/oauth/authorize?…GET /cursor/authorizeoauth_unsupportedPOST /cursor/manual(JWT)sub, storedPOST /cursor/accounts/removeGET /accounts(legacy)GET /madeup/accountsunknown_providerBrowser test
Playwright smoke test confirms all four provider cards render on the Connectors admin page with correct badges (MAX/GPT/CUR/GAI), descriptions, and Set up buttons. No plugin-related JS console errors.
Lint
php -lclean on all 4 new + 2 modified PHP files.npm run buildsucceeds;build/connector.js15.4 KiB minified (was 12.5 KiB).aidevops.sh v3.15.55 plugin for OpenCode v1.15.1 with claude-sonnet-4-6 spent 1d 23h and 383 tokens on this as a headless worker.
Summary by CodeRabbit
Release Notes
New Features
Updates