Skip to content

Connectors: Keep AI provider API keys that cannot be validated - #13870

Closed
jorgefilipecosta wants to merge 2 commits into
WordPress:trunkfrom
jorgefilipecosta:fix/connectors-api-key-provider-outage
Closed

jorgefilipecosta wants to merge 2 commits into
WordPress:trunkfrom
jorgefilipecosta:fix/connectors-api-key-provider-outage

Conversation

@jorgefilipecosta

@jorgefilipecosta jorgefilipecosta commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

What

Alternative to #12345. Which on its own doesn't fix the outage: the official provider plugins check availability by listing models, and ListModelsApiBasedProviderAvailability::isConfigured() turns every failure into false, so an unreachable provider still wiped the key.

Doesn't depend on any php-ai-client change: it works with the bundled php-ai-client 1.3.1, which is also the version in the 7.0 and 7.1 branches.

  • Only discard a submitted AI provider key when the provider rejects it (a 4xx other than 408 or 429). Network errors, server errors, timeouts and rate limits keep the key.
  • Validate against a fresh model list. The cached one made any key pass for up to a day on sites with a persistent object cache.

The Connectors screen pre-filled the masked key when a provider showed as not connected, which is what it shows during an outage, so saving it overwrote the key before validation ran. WordPress/gutenberg#83889 fixes that on the screen, which now shows a stored key read-only until it is removed, and should ship along with this change.

Testing

Verify unit tests are passing:

vendor/bin/phpunit --group connectors

Fix and tests drafted with AI assistance (Claude Code) and reviewed by me.

Trac ticket: https://core.trac.wordpress.org/ticket/65551

Only discard a submitted key when the provider rejects it. A network
error, server error, or rate limit during validation no longer wipes the
key, a cached model list no longer validates a different key, and a
masked key submitted back to the settings endpoint keeps the stored key.

See #65551.
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props jorgefilipecosta, muneebashraf.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

The Connectors screen no longer submits a masked API key, as it shows a
stored key read-only until it is removed, so the setting does not need
to handle one: WordPress/gutenberg#83889

See #65551.
@muneeb-ashraf

Copy link
Copy Markdown

Patch Testing Report

Patch tested: #13870 (head 62d9119)

Environment

  • WordPress: trunk dc829be for the reproduction, this PR's head 62d9119 for the patch
  • PHP: 8.4.26 (CLI, Windows)
  • Database: SQLite (SQLite Database Integration 3.0.2)
  • Plugins: AI Provider for OpenAI 1.2.0 from WordPress.org
  • Test method: a PHPUnit test on the core test suite. It saves connectors_ai_openai_api_key through POST /wp/v2/settings and applies rest_post_dispatch the way WP_REST_Server::serve_request() does. A pre_http_request filter answers the provider's GET https://api.openai.com/v1/models. Each case saves the key once with a 200 response, then saves the same key again while the provider fails. "Fresh cache" flushes the object cache between the two saves (separate requests, no persistent object cache). "Persistent cache" keeps it.

Actual results

Stored key after the second save:

Provider response on re-save trunk, fresh cache trunk, persistent cache PR, fresh cache PR, persistent cache
Network error (cURL 28) wiped kept, provider not called kept kept
503 wiped kept, provider not called kept kept
500 wiped kept, provider not called kept kept
429 wiped kept, provider not called kept kept
408 wiped kept, provider not called kept kept
401 wiped kept, provider not called wiped wiped
  • ✅ Reproduced on trunk: any failure during a re-save wipes a valid key. With a persistent cache the key is never re-validated, so a key the provider rejects (401) is kept.
  • ✅ With the patch: outages, timeouts and rate limits keep the key, a 401 discards it, and the provider is called in both cache setups.
  • ✅ vendor/bin/phpunit --group connectors on the PR head: OK (124 tests, 305 assertions).

Additional notes

  • When the key is kept, _wp_connectors_is_ai_api_key_valid() reports the failure through wp_trigger_error(), for example: Network error occurred while sending GET request to https://api.openai.com/v1/models: cURL error 28: Connection timed out. With WP_DEBUG on this shows as a notice, which looks intended.
  • Not tested: the Connectors screen itself (the masked-key change that Connectors: Keep a stored API key read-only when it can't be verified gutenberg#83889 covers), or a live OpenAI endpoint.
  • Tested on SQLite rather than MySQL. The code under test only reads and writes options.
Reproduction test

Run with the core PHPUnit config and a bootstrap that loads the AI Provider for OpenAI plugin on muplugins_loaded.

<?php
/**
 * Reproduction for #65551 with the real AI Provider for OpenAI plugin.
 *
 * Saves the OpenAI key through POST /wp/v2/settings (what the Connectors screen does)
 * while pre_http_request simulates the provider's answer to the list models request.
 */
class Repro65551Test extends WP_UnitTestCase {
	const OPTION = 'connectors_ai_openai_api_key';
	const KEY    = 'sk-repro-65551-0123456789abcdef';

	private $mode     = 'ok';
	private $requests = 0;
	private $notices  = 0;

	public function set_up() {
		parent::set_up();
		wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) );
		add_filter( 'pre_http_request', array( $this, 'fake_openai' ), 10, 3 );
		// Count wp_trigger_error() notices instead of letting PHPUnit turn them into exceptions.
		add_filter(
			'wp_trigger_error_trigger_error',
			function () {
				++$this->notices;
				return false;
			}
		);
	}

	public function fake_openai( $pre, $args, $url ) {
		if ( false === strpos( $url, 'api.openai.com' ) ) {
			return $pre;
		}
		++$this->requests;
		if ( 'network' === $this->mode ) {
			return new WP_Error( 'http_request_failed', 'cURL error 28: Connection timed out' );
		}
		$code = 'ok' === $this->mode ? 200 : (int) $this->mode;
		$body = 200 === $code
			? wp_json_encode( array( 'object' => 'list', 'data' => array( array( 'id' => 'gpt-4o', 'object' => 'model', 'created' => 1715367049, 'owned_by' => 'system' ) ) ) )
			: wp_json_encode( array( 'error' => array( 'message' => "Simulated $code", 'type' => 'test' ) ) );
		return array(
			'headers'  => array( 'content-type' => 'application/json' ),
			'body'     => $body,
			'response' => array( 'code' => $code, 'message' => 'Simulated' ),
			'cookies'  => array(),
			'filename' => null,
		);
	}

	private function save_key( string $mode ): string {
		$this->mode = $mode;
		$request    = new WP_REST_Request( 'POST', '/wp/v2/settings' );
		$request->set_param( self::OPTION, self::KEY );
		// rest_do_request() skips rest_post_dispatch; apply it as WP_REST_Server::serve_request() does.
		$response = rest_do_request( $request );
		$response = apply_filters( 'rest_post_dispatch', rest_ensure_response( $response ), rest_get_server(), $request );
		$this->assertSame( 200, $response->get_status(), 'Settings save should succeed.' );
		return (string) get_option( self::OPTION, '' );
	}

	/** @dataProvider data_modes */
	public function test_resave_during_provider_response( string $mode, bool $fresh_cache ) {
		$this->assertTrue( wp_is_connector_registered( 'openai' ), 'The OpenAI connector should be registered.' );

		// A valid key is stored first.
		$this->assertSame( self::KEY, $this->save_key( 'ok' ), 'Baseline: a valid key is stored.' );

		// Separate requests on a site without a persistent object cache start with an empty cache.
		if ( $fresh_cache ) {
			wp_cache_flush();
		}
		$before = $this->requests;

		// Re-save the same key while the provider answers with $mode.
		$stored = $this->save_key( $mode );

		fwrite(
			STDERR,
			sprintf(
				"\n[65551] cache=%-10s provider=%-7s provider_called=%-3s notice=%-3s result=%s",
				$fresh_cache ? 'fresh' : 'persistent',
				$mode,
				$this->requests > $before ? 'yes' : 'no',
				$this->notices > 0 ? 'yes' : 'no',
				'' === $stored ? 'KEY WIPED' : 'key kept'
			)
		);
	}

	public function data_modes() {
		$data = array();
		foreach ( array( true, false ) as $fresh_cache ) {
			foreach ( array( 'network', '503', '500', '429', '408', '401' ) as $mode ) {
				$data[ ( $fresh_cache ? 'fresh ' : 'persistent ' ) . $mode ] = array( $mode, $fresh_cache );
			}
		}
		return $data;
	}
}

AI assistance: Claude Code (Claude Opus 5.5) helped write the reproduction test. The results above come from running it.

@jorgefilipecosta

Copy link
Copy Markdown
Member Author

I kept digging into this issue and this solution still has a problem, when there is an outage we show connected:
image

While we don't know if there was a connection. I think there the root cause of the problem of storing the same key during an outage, is the UI allowing during an outage the same key to be stored instead of the normal flow of removing and then typing a new key I'm fixing that issue at WordPress/gutenberg#83889.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants