Repository navigation
Connectors: Show filesystem credentials modal on plugin install - #78367
westonruter wants to merge 13 commits into
Conversation
When `FS_METHOD` is not `direct` and FTP credentials have not been stored, the REST `/wp/v2/plugins` endpoint rejects an install attempt up front, surfacing only a generic "Failed to install plugin" snackbar on the Connectors screen. Plugins > Add Plugin handles the same case by displaying the standard "Connection Information" modal. Print that modal on the Connectors screen, expose `filesystemCredentialsRequired` via the script module data, and route through `wp.updates.installPlugin()` when credentials are required so the legacy AJAX install path supplies them. After the install succeeds, activate via REST and refresh the plugin entity.
|
Size Change: 0 B Total Size: 8.21 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Pull request overview
This PR fixes the Connectors plugin-install flow when the WordPress filesystem method is not direct (e.g. FTP), by mirroring Core’s standard “Connection Information” modal behavior so users have a clear path to provide credentials and complete installation.
Changes:
- Enqueues Core’s updates installer script and prints the filesystem credentials modal footer markup on the Connectors admin screen.
- Exposes a
filesystemCredentialsRequiredflag to the Connectors script module so the client can choose the correct install mechanism. - Updates the Connectors install hook to route through
wp.updates.installPlugin()when credentials are required, then activates via REST.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/experimental/connectors/load.php |
Adds Connectors-screen setup for updates script + filesystem credentials modal, and exposes a “credentials required” flag to script module data. |
routes/connectors-home/use-connector-plugin.ts |
Adds a wp.updates-based install path when filesystem credentials are needed, and suppresses snackbar on user-cancel. |
routes/connectors-home/default-connectors.tsx |
Reads the new script module flag and passes it into the connector install hook. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| jq( document ).one( 'wp-plugin-install-success', onSuccess ); | ||
| jq( document ).one( 'wp-plugin-install-error', onError ); | ||
| jq( document ).one( 'credential-modal-cancel', onCancel ); | ||
|
|
||
| updates.installPlugin( { slug } ); |
There was a problem hiding this comment.
Per Claude:
Fixed in cfeeea4. installPluginViaWpUpdates() now registers the three handlers with jQuery(document).on(...) (not .one(...)) and tears them down via an explicit shared cleanup() that only runs once the event whose response.slug matches the requested slug arrives. A wp-plugin-install-success/-error for a different plugin no longer consumes a one-shot handler and strands this promise — non-matching events are ignored and the handlers stay registered until the matching one resolves/rejects (or credential-modal-cancel fires). Verified end-to-end under FS_METHOD=ftpsockets (install, cancel, and re-install paths).
| await installPluginViaWpUpdates( pluginSlug ); | ||
| if ( pluginBasename ) { | ||
| await saveEntityRecord( | ||
| 'root', | ||
| 'plugin', | ||
| { plugin: pluginBasename, status: 'active' }, | ||
| { throwOnError: true } | ||
| ); |
There was a problem hiding this comment.
Per Claude:
Thanks — but I believe this concern is based on two incorrect premises, so I'm leaving the code as-is:
-
The REST endpoint expects the basename without
.php.WP_REST_Plugins_Controller::sanitize_plugin_param()isreturn plugin_basename( sanitize_text_field( $file . '.php' ) );— it appends.phpitself. So passingai-provider-for-openai/plugin(the strippedpluginBasename) is exactly what the endpoint wants; passing the full…/plugin.phpwould result in…/plugin.php.php. -
The
wp-plugin-install-successpayload has no top-levelresponse.plugin. Perwp_ajax_install_plugin()inwp-admin/includes/ajax-actions.php, the success$statusobject carriesslugand (conditionally)activateUrl; the plugin file is only embedded inside theactivateUrlquery string, not exposed as a clean field. So there's nothing reliable to "return frominstallPluginViaWpUpdates" without fragile URL parsing.
Additionally, the non-FS activatePlugin() path in this same file (pre-existing, not introduced here) already uses plugin: pluginBasename and works, and the FS path was verified end-to-end under FS_METHOD=ftpsockets (install via FTP → REST activate → entity refresh → UI expands), so this isn't a regression.
Happy to revisit if there's a connector whose configured plugin.file doesn't match its actual installed main file, but that gap (if it exists) would predate this PR and affect the existing activate path equally.
There was a problem hiding this comment.
I agree with your analysis: keeping pluginBasename is the correct REST value for this flow, and wp-plugin-install-success doesn’t provide a reliable top-level plugin file to switch to. So no additional code change is needed here beyond cfeeea4.
| } catch ( error ) { | ||
| // A user-initiated cancel of the filesystem credentials dialog | ||
| // should silently abort the install without a snackbar. | ||
| const message = error instanceof Error ? error.message : ''; | ||
| if ( message === 'Filesystem credentials request canceled.' ) { | ||
| return; | ||
| } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Per Claude:
Fixed in cfeeea4. The hard-coded error-message string match is gone. installPluginViaWpUpdates() now resolves with a discriminated 'installed' | 'canceled' value: closing the credentials modal (credential-modal-cancel) resolves 'canceled', a genuine failure still reject()s with an Error. The caller branches on outcome === 'canceled' to silently abort, so cancellation is a normal control-flow outcome rather than an exception identified by message text. (And re: the message-string note — you were right, that string was an invention with no basis in wordpress-develop; it no longer exists anywhere now.) Verified the cancel path leaves no error snackbar and the button returns to "Install".
|
Flaky tests detected in 530755e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26016663464
|
- Register the install event handlers with on()/off() instead of one() and filter by slug. With one(), a concurrent install of a different plugin firing first would consume the one-shot handler and leave the promise pending. on() + explicit cleanup() only tears down once the matching slug's event arrives. - Replace the brittle "cancel via hard-coded error message string" detection. installPluginViaWpUpdates() now resolves with 'installed' | 'canceled' so cancel is a normal control-flow outcome the caller branches on, while genuine install failures still reject. Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
I tested this PR locally with The provider connector cards now open the standard “Connection Information” filesystem credentials modal as expected. I tested this with the OpenAI connector, and canceling the modal returned the button back to I noticed one related case that still seems to use the old REST install path: the top “Install the AI plugin” callout. When clicking that button on the same Connectors screen, it shows “Failed to install the AI plugin.” instead of opening the filesystem credentials modal. Steps:
Expected: the standard filesystem credentials modal opens, matching the provider connector install flow. Actual: the install fails with the generic AI plugin install error. |
The "Install the AI plugin" button in AiPluginCallout had the same silent-failure bug as the connector install: it POSTed directly to the REST plugin endpoint, which rejects the install up front when FS_METHOD is not 'direct' and credentials have not been stored. Reuse the now-exported installPluginViaWpUpdates() helper so the AI plugin install routes through wp.updates.installPlugin() (surfacing the standard "Connection Information" modal) when credentials are required, then activates via REST. The non-FS path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-fs-credentials Resolved a modify/delete conflict from trunk #78228, which relocated the Connectors loader out of lib/experimental/connectors/load.php and consolidated it into lib/compat/wordpress-7.0/connectors.php. The filesystem-credentials additions (the load-{hook} screen setup, the credentials-modal printer, and the script-module-data filter) were ported into the new connectors.php location; the obsolete experimental/connectors/load.php was removed (its default-connectors.php require is now handled directly by lib/load.php). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ekamran Great catch! This is fixed now in 8d3b1c2. The merge conflicts are also fixed in 9ae49aa after #78228, although this may cause problems if this is cherry-picked to |
|
Here's a screen recording showing the installation of the AI plugin via FTP: AI.plugin.install.via.FTP.mov(Ignore Chrome's warning about an insecure FTP password! |
If we intend to backport this PR to WP 7.0, we can backport #78228 to the |
request_filesystem_credentials() returns true (no credentials needed), false (needed but not provided), or an array (needed and provided). Only the explicit false sentinel should set filesystemCredentialsRequired, so compare with `false === $stored` instead of `! $stored`. This avoids coercing a (theoretical) empty-array return to "required" and matches the strict-boolean expectation of PHPStan, mirroring the Performance Lab counterpart which already uses the strict comparison. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the follow-up fix here. I pulled the latest PR head ( After rebuilding the Gutenberg assets, I can confirm the UI behavior looks fixed for both paths I checked:
One thing I noticed while checking the latest branch: the targeted unit test for the AI plugin callout is still failing locally: npm run test:unit -- routes/connectors-home/test/ai-plugin-callout.tsx --runInBandFailure: From a quick look, |
|
I was also able to get the failing The patch updates the After that change, the focused test passes locally: npm run test:unit -- routes/connectors-home/test/ai-plugin-callout.tsx --runInBandHappy to share the patch or open a small follow-up PR if that would be helpful. |
…rage The AiPluginCallout test mocked ../default-connectors with only getConnectorData(), so once the component started importing getRequiresFilesystemCredentials() that call was undefined and the "installs the AI plugin using the plugin slug" test failed (the install threw before reaching saveEntityRecord). Add getRequiresFilesystemCredentials() to the mock (defaulting to false) and mock installPluginViaWpUpdates from ../use-connector-plugin, then add companion tests covering the filesystem-credentials path: install routed through wp.updates then activating ai/ai, and a silent abort when the credentials modal is canceled. Co-authored-by: ekamran <ekamran@git.wordpress.org> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The wp.updates fallback in use-connector-plugin.ts declared its own
minimal `jQuery?: ( target: Document ) => { on, off }` shape. Depend on
@types/jquery (pinned to 3.5.34, matching wordpress-develop core) and
reuse the canonical JQueryStatic / JQuery.TriggeredEvent types instead
of reinventing them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| "@inquirer/prompts": "7.2.0", | ||
| "@octokit/rest": "16.26.0", | ||
| "@testing-library/jest-dom": "6.9.1", | ||
| "@types/jquery": "3.5.34", |
There was a problem hiding this comment.
This matches what is used in core:
Thanks! I added The latest changes look good to me, and I see the checks are now in much better shape. |
t-hamano
left a comment
There was a problem hiding this comment.
Instead of defining filesystemCredentialsRequired on the server-side, could we check if the REST API response's error code is fs_unavailable on the client-side? This might significantly reduce the amount of code.
Although I haven't tested it, the AI suggested the following diff.
Details
diff --git a/lib/compat/wordpress-7.0/connectors.php b/lib/compat/wordpress-7.0/connectors.php
index d8d3a03b769..f3cf682353c 100644
--- a/lib/compat/wordpress-7.0/connectors.php
+++ b/lib/compat/wordpress-7.0/connectors.php
@@ -270,36 +270,3 @@ function _gutenberg_connectors_print_filesystem_credentials_modal(): void {
wp_print_request_filesystem_credentials_modal();
wp_print_admin_notice_templates();
}
-
-/**
- * Exposes whether the current request can install plugins directly to the
- * Connectors script module.
- *
- * When the filesystem method is anything other than `direct` and stored
- * credentials are not yet available, the REST plugin endpoint will reject
- * an install attempt up front. Surfacing this state to the client lets the
- * React layer pre-emptively route through `wp.updates.installPlugin()`,
- * which displays the standard filesystem credentials modal.
- *
- * @access private
- *
- * @param array<string, mixed> $data Existing script module data.
- * @return array<string, mixed> Script module data with the filesystem flag added.
- */
-function _gutenberg_connectors_add_filesystem_data_to_script_module_data( array $data ): array {
- require_once ABSPATH . 'wp-admin/includes/file.php';
-
- $method = get_filesystem_method();
- if ( 'direct' === $method ) {
- $data['filesystemCredentialsRequired'] = false;
- return $data;
- }
-
- ob_start();
- $stored = request_filesystem_credentials( self_admin_url() );
- ob_end_clean();
-
- $data['filesystemCredentialsRequired'] = false === $stored;
- return $data;
-}
-add_filter( 'script_module_data_options-connectors-wp-admin', '_gutenberg_connectors_add_filesystem_data_to_script_module_data' );
diff --git a/routes/connectors-home/ai-plugin-callout.tsx b/routes/connectors-home/ai-plugin-callout.tsx
index 1b9a2d70d59..2b9e4192cb3 100644
--- a/routes/connectors-home/ai-plugin-callout.tsx
+++ b/routes/connectors-home/ai-plugin-callout.tsx
@@ -17,10 +17,7 @@ import { addQueryArgs } from '@wordpress/url';
/**
* Internal dependencies
*/
-import {
- getConnectorData,
- getRequiresFilesystemCredentials,
-} from './default-connectors';
+import { getConnectorData } from './default-connectors';
import { WpLogoDecoration } from './wp-logo-decoration';
import { installPluginViaWpUpdates } from './use-connector-plugin';
@@ -134,12 +131,27 @@ export function AiPluginCallout() {
const installPlugin = async () => {
setIsBusy( true );
try {
- if ( getRequiresFilesystemCredentials() ) {
- // The REST plugin endpoint rejects installs up front when
- // FS_METHOD is not 'direct' and credentials have not been
- // stored. Route through wp.updates.installPlugin() so the
- // standard "Connection Information" modal handles credential
- // entry, then activate via REST.
+ try {
+ // Standard install via the REST plugin endpoint.
+ await saveEntityRecord(
+ 'root',
+ 'plugin',
+ { slug: AI_PLUGIN_SLUG, status: 'active' },
+ { throwOnError: true }
+ );
+ } catch ( error ) {
+ // The REST plugin endpoint rejects installs up front with
+ // the `fs_unavailable` error code when FS_METHOD is not
+ // 'direct' and credentials have not been stored. Route
+ // through wp.updates.installPlugin() so the standard
+ // "Connection Information" modal handles credential entry,
+ // then activate via REST.
+ console.log( error );
+ if (
+ ( error as { code?: string } )?.code !== 'fs_unavailable'
+ ) {
+ throw error;
+ }
const outcome =
await installPluginViaWpUpdates( AI_PLUGIN_SLUG );
if ( outcome === 'canceled' ) {
@@ -152,13 +164,6 @@ export function AiPluginCallout() {
{ plugin: AI_PLUGIN_ID, status: 'active' },
{ throwOnError: true }
);
- } else {
- await saveEntityRecord(
- 'root',
- 'plugin',
- { slug: AI_PLUGIN_SLUG, status: 'active' },
- { throwOnError: true }
- );
}
setJustActivated( true );
createSuccessNotice(
diff --git a/routes/connectors-home/default-connectors.tsx b/routes/connectors-home/default-connectors.tsx
index 00824f0bce0..2f2dec2db0e 100644
--- a/routes/connectors-home/default-connectors.tsx
+++ b/routes/connectors-home/default-connectors.tsx
@@ -46,7 +46,6 @@ interface ConnectorData {
interface ConnectorScriptModuleData {
connectors?: Record< string, ConnectorData >;
isFileModDisabled?: boolean;
- filesystemCredentialsRequired?: boolean;
}
function getConnectorScriptModuleData(): ConnectorScriptModuleData {
@@ -72,10 +71,6 @@ export function getIsFileModDisabled(): boolean {
return !! getConnectorScriptModuleData().isFileModDisabled;
}
-export function getRequiresFilesystemCredentials(): boolean {
- return !! getConnectorScriptModuleData().filesystemCredentialsRequired;
-}
-
const CONNECTOR_LOGOS: Record< string, React.ComponentType > = {
google: GeminiLogo,
openai: OpenAILogo,
@@ -175,7 +170,6 @@ function ApiKeyConnector( {
isActivated: plugin?.isActivated,
keySource: auth?.keySource,
initialIsConnected: auth?.isConnected,
- requiresFilesystemCredentials: getRequiresFilesystemCredentials(),
} );
const isExternallyConfigured =
keySource === 'env' || keySource === 'constant';
diff --git a/routes/connectors-home/test/ai-plugin-callout.tsx b/routes/connectors-home/test/ai-plugin-callout.tsx
index 50af7dd98fb..7223e04df9f 100644
--- a/routes/connectors-home/test/ai-plugin-callout.tsx
+++ b/routes/connectors-home/test/ai-plugin-callout.tsx
@@ -13,7 +13,6 @@ import { useDispatch, useSelect } from '@wordpress/data';
* Internal dependencies
*/
import { AiPluginCallout } from '../ai-plugin-callout';
-import { getRequiresFilesystemCredentials } from '../default-connectors';
import { installPluginViaWpUpdates } from '../use-connector-plugin';
jest.mock( '@wordpress/a11y', () => ( {
@@ -95,7 +94,6 @@ jest.mock( '../default-connectors', () => ( {
},
},
} ) ),
- getRequiresFilesystemCredentials: jest.fn( () => false ),
} ) );
jest.mock( '../use-connector-plugin', () => ( {
@@ -165,10 +163,6 @@ describe( 'AiPluginCallout', () => {
mapSelect( () => selectorStore )
);
- ( getRequiresFilesystemCredentials as jest.Mock ).mockReset();
- ( getRequiresFilesystemCredentials as jest.Mock ).mockReturnValue(
- false
- );
( installPluginViaWpUpdates as jest.Mock ).mockReset();
( installPluginViaWpUpdates as jest.Mock ).mockResolvedValue(
'installed'
@@ -218,12 +212,21 @@ describe( 'AiPluginCallout', () => {
expect( installPluginViaWpUpdates ).not.toHaveBeenCalled();
} );
- it( 'installs via wp.updates then activates when filesystem credentials are required', async () => {
+ it( 'falls back to wp.updates then activates when the REST install reports fs_unavailable', async () => {
const user = userEvent.setup();
storeState.plugin = undefined;
- ( getRequiresFilesystemCredentials as jest.Mock ).mockReturnValue(
- true
+ // The REST install (using `slug`) is rejected up front; the
+ // subsequent activate (using `plugin`) succeeds.
+ mockSaveEntityRecord.mockImplementation(
+ (
+ _kind: string,
+ _name: string,
+ record: Record< string, unknown >
+ ) =>
+ 'slug' in record
+ ? Promise.reject( { code: 'fs_unavailable' } )
+ : Promise.resolve( undefined )
);
( installPluginViaWpUpdates as jest.Mock ).mockResolvedValue(
'installed'
@@ -247,15 +250,15 @@ describe( 'AiPluginCallout', () => {
{ throwOnError: true }
);
} );
+
+ expect( mockCreateErrorNotice ).not.toHaveBeenCalled();
} );
it( 'aborts silently when the filesystem credentials modal is canceled', async () => {
const user = userEvent.setup();
storeState.plugin = undefined;
- ( getRequiresFilesystemCredentials as jest.Mock ).mockReturnValue(
- true
- );
+ mockSaveEntityRecord.mockRejectedValue( { code: 'fs_unavailable' } );
( installPluginViaWpUpdates as jest.Mock ).mockResolvedValue(
'canceled'
);
@@ -270,7 +273,31 @@ describe( 'AiPluginCallout', () => {
expect( installPluginViaWpUpdates ).toHaveBeenCalledWith( 'ai' );
} );
- expect( mockSaveEntityRecord ).not.toHaveBeenCalled();
+ // Only the initial REST install attempt ran — no activate follow-up,
+ // no error notice.
+ expect( mockSaveEntityRecord ).toHaveBeenCalledTimes( 1 );
expect( mockCreateErrorNotice ).not.toHaveBeenCalled();
} );
+
+ it( 'shows an error notice when the REST install fails for another reason', async () => {
+ const user = userEvent.setup();
+
+ storeState.plugin = undefined;
+ mockSaveEntityRecord.mockRejectedValue( {
+ code: 'rest_cannot_install',
+ } );
+
+ render( <AiPluginCallout /> );
+
+ await user.click(
+ screen.getByRole( 'button', { name: 'Install the AI plugin' } )
+ );
+
+ await waitFor( () => {
+ expect( mockCreateErrorNotice ).toHaveBeenCalled();
+ } );
+
+ // A non-filesystem error must not trigger the wp.updates fallback.
+ expect( installPluginViaWpUpdates ).not.toHaveBeenCalled();
+ } );
} );
diff --git a/routes/connectors-home/use-connector-plugin.ts b/routes/connectors-home/use-connector-plugin.ts
index 0cd16228f31..7d4a9cdb431 100644
--- a/routes/connectors-home/use-connector-plugin.ts
+++ b/routes/connectors-home/use-connector-plugin.ts
@@ -19,7 +19,6 @@ interface UseConnectorPluginOptions {
isActivated?: boolean;
keySource?: ApiKeySource;
initialIsConnected?: boolean;
- requiresFilesystemCredentials?: boolean;
}
interface WpUpdatesInstallPluginResponse {
@@ -38,7 +37,7 @@ interface WpUpdates {
}
type JQueryHandler = (
- event: unknown,
+ event: JQuery.TriggeredEvent,
response:
| WpUpdatesInstallPluginResponse
| WpUpdatesInstallPluginErrorResponse
@@ -48,10 +47,7 @@ interface WpUpdatesGlobal {
wp?: {
updates?: WpUpdates;
};
- jQuery?: ( target: Document ) => {
- on: ( event: string, handler: JQueryHandler ) => unknown;
- off: ( event: string, handler: JQueryHandler ) => unknown;
- };
+ jQuery?: JQueryStatic;
}
/**
@@ -145,7 +141,6 @@ export function useConnectorPlugin( {
isActivated,
keySource = 'none',
initialIsConnected = false,
- requiresFilesystemCredentials = false,
}: UseConnectorPluginOptions ): UseConnectorPluginReturn {
const [ isExpanded, setIsExpanded ] = useState( false );
const [ isBusy, setIsBusy ] = useState( false );
@@ -269,12 +264,26 @@ export function useConnectorPlugin( {
}
setIsBusy( true );
try {
- if ( requiresFilesystemCredentials ) {
- // The REST plugin endpoint rejects installs up front when
- // FS_METHOD is not 'direct' and credentials have not been
- // stored. Route through wp.updates.installPlugin() so the
- // standard "Connection Information" modal handles credential
- // entry, then activate via REST.
+ try {
+ // Standard install via the REST plugin endpoint.
+ await saveEntityRecord(
+ 'root',
+ 'plugin',
+ { slug: pluginSlug, status: 'active' },
+ { throwOnError: true }
+ );
+ } catch ( error ) {
+ // The REST plugin endpoint rejects installs up front with
+ // the `fs_unavailable` error code when FS_METHOD is not
+ // 'direct' and credentials have not been stored. Route
+ // through wp.updates.installPlugin() so the standard
+ // "Connection Information" modal handles credential entry,
+ // then activate via REST.
+ if (
+ ( error as { code?: string } )?.code !== 'fs_unavailable'
+ ) {
+ throw error;
+ }
const outcome = await installPluginViaWpUpdates( pluginSlug );
if ( outcome === 'canceled' ) {
// User closed the credentials modal; abort silently.
@@ -294,13 +303,6 @@ export function useConnectorPlugin( {
'plugin',
pluginBasename,
] );
- } else {
- await saveEntityRecord(
- 'root',
- 'plugin',
- { slug: pluginSlug, status: 'active' },
- { throwOnError: true }
- );
}
setPluginStatusOverride( 'active' );
// Re-fetch settings since the new plugin may register new settings.| } | ||
|
|
||
| type JQueryHandler = ( | ||
| event: JQuery.TriggeredEvent, |
There was a problem hiding this comment.
Personally, I would like to avoid installing @types/jquery just for this. In the future, we will rarely use jQuery in TypeScript. Could we perhaps emulate a minimal custom type as follows?
type JQueryHandler = (
event: unknown,
response:
| WpUpdatesInstallPluginResponse
| WpUpdatesInstallPluginErrorResponse
) => void;
interface WpUpdatesGlobal {
wp?: {
updates?: {
installPlugin: ( args: { slug: string } ) => void;
};
};
jQuery?: ( target: Document ) => {
on: ( events: string, handler: JQueryHandler ) => void;
off: ( events: string, handler: JQueryHandler ) => void;
};
}There was a problem hiding this comment.
Note that core uses this same
@types/jquerypackage.
Oh, I didn't realize that. I just felt uneasy adding jQuery types to the root package.json solely to fix one issue, but I don't have a strong opinion.
|
I tested the latest PR head again locally. Current implementationThe focused
Result: 4 tests passed. Alternative
|
|
@westonruter would it help if I build t-hamano's |
🤖 PR meta 🤖🎉 PropsUnlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @claude. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: 0 B Total Size: 8.64 MB
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. default to a list of pages if there are no menus in
|
Resolve conflicts after trunk bumped the minimum WordPress version to 7.0 and moved to Vitest: - Move the Connectors filesystem credentials setup from the removed lib/compat/wordpress-7.0/connectors.php into lib/compat/plugin/connectors.php. - Drop the root @types/jquery devDependency, since root devDependencies were moved into workspaces, and use minimal local jQuery types instead. - Port the AiPluginCallout filesystem credentials tests to Vitest. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVht4racQmKuYmELHoLXdx
…le error Instead of computing `filesystemCredentialsRequired` on the server and exposing it through script module data, attempt the standard REST plugin install first and fall back to `wp.updates.installPlugin()` only when the endpoint rejects with the `fs_unavailable` error code. The REST response becomes the single source of truth, and the PHP flag is removed. The Connectors screen still enqueues the `updates` script and prints the filesystem credentials modal so the fallback can show it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVht4racQmKuYmELHoLXdx
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: t-hamano <54422211+t-hamano@users.noreply.github.com> Claude-Session: https://claude.ai/code/session_01FVht4racQmKuYmELHoLXdx
|
@ekamran Thanks for the offer! I've resolved the conflicts with @westonruter, could you take a look when you have a chance? |
|
Thanks for picking this up. I tested the latest head ( The main flows work:
I found three issues:
For 2 and 3, I tried calling |
Important
The corresponding wordpress-develop backport PR has not been created yet. Once the approach is validated here and approved, the core PR will be created.Edited by @t-hamano: Core backport PR: WordPress/wordpress-develop#13953
Summary
Fixes the Connectors install flow when
FS_METHODis notdirect— for example,ftpext/ftpsockets. The current behavior is a generic "Failed to install plugin for X." snackbar with no path forward; Plugins > Add Plugin already handles the same case by showing the standard "Connection Information" modal.This PR mirrors that flow on the Connectors screen:
lib/experimental/connectors/load.php— on the Connectors page only, enqueuewp.updates, printwp_print_request_filesystem_credentials_modal()in the footer, and exposefilesystemCredentialsRequiredvia thescript_module_data_options-connectors-wp-adminfilter.routes/connectors-home/use-connector-plugin.ts— whenfilesystemCredentialsRequiredis true, route throughwp.updates.installPlugin()(which knows how to surface the FTP credentials modal, collect creds, and POST them with the install). After success, activate via the existing REST flow and refresh the plugin entity. Cancelling the modal silently aborts.routes/connectors-home/default-connectors.tsx— read the new flag and forward touseConnectorPlugin.Edited by @t-hamano:
lib/compat/plugin/connectors.php— on the Connectors page only, enqueuewp.updatesand printwp_print_request_filesystem_credentials_modal()in the footer.routes/connectors-home/use-connector-plugin.ts/ai-plugin-callout.tsx— install via the REST plugin endpoint first. If it rejects withfs_unavailable, fall back towp.updates.installPlugin(), which shows the FTP credentials modal, collects credentials, and POSTs them with the install. After success, activate via REST and refresh the plugin entity. Cancelling the modal silently aborts. Any other REST error shows the existing error notice.Fixes Core-65223.
Screenshots
Open AI plugin is not installed yet:
Clicking Install causes the FTP credentials modal to appear:
Providing credentials results in the plugin being active and ready for setup:
Test plan
Reproduces against an environment where the filesystem method requires credentials:
ai-provider-for-openai) is not installed.directfilesystem environment — the install proceeds via REST as before, no modal.Use of AI Tools
Claude Opus 4.7 was used to create the PR and validate the fix.
Claude prompt and conversation summary
Prompt
Investigation
FS_METHOD≠direct, while Plugins > Add Plugin handles it correctly.request_filesystem_credentials()+wp_print_request_filesystem_credentials_modal()inwp-admin/includes/file.phprender the "Connection Information" modal.wp.updates.installPlugin()(inwp-admin/js/updates.js) wraps the AJAX call, includes_fs_nonceand FTP credentials in the POST body, shows the modal whenshouldRequestFilesystemCredentialsis true, and fireswp-plugin-install-success/wp-plugin-install-errorjQuery events.useConnectorPluginhook callssaveEntityRecord( 'root', 'plugin', … ), which hitsWP_REST_Plugins_Controller::create_item. That controller callsis_filesystem_available(), which returns afs_unavailableWP_Errorup front whenever FS_METHOD isn'tdirectand no credentials are stored — so the REST endpoint can never accept FTP creds in its JSON body.lib/experimental/connectors/load.phpand the per-page enqueue/render is auto-generated frompackages/wp-build/templates/page-wp-admin.php.template(not safe to edit directly), so additions must hook off the connectors menu page specifically.Implementation (commit 171d758)
Branch:
fix/65223-connectors-fs-credentialsin WordPress/gutenberg.lib/experimental/connectors/load.php: capture the$hook_suffixfromadd_submenu_page()and register aload-{hook_suffix}action thatwp.updates, andadmin_footerto printwp_print_request_filesystem_credentials_modal()+wp_print_admin_notice_templates().Also added a
script_module_data_options-connectors-wp-adminfilter that exposesfilesystemCredentialsRequired: boolbased onget_filesystem_method()and stored credentials.routes/connectors-home/use-connector-plugin.ts: added a newrequiresFilesystemCredentialsoption to the hook. When set, the install path routes through a newinstallPluginViaWpUpdates(slug)helper that callswp.updates.installPlugin({ slug })and resolves on thewp-plugin-install-successjQuery event (rejects onwp-plugin-install-error, silently aborts oncredential-modal-cancel). After install succeeds, it activates via the existing REST flow and invalidates the plugin entity resolution.routes/connectors-home/default-connectors.tsx: added agetRequiresFilesystemCredentials()reader on the script module data and forwarded it touseConnectorPlugin.Verification
Tested in Chrome at
http://localhost:8000/wp-admin/options-general.php?page=options-connectors-wp-adminagainst an environment withFS_METHOD=ftpsockets:ftp/ftpuser/ftppass), submitted, andwp plugin status ai-provider-for-openaireportedStatus: Active. The React UI flipped the button to Cancel and auto-expanded the API KEY entry form, matching the direct-FS path.filesystemCredentialsRequired: true, the modal<div>present,wp.updates.installPluginloaded, andwp.updates.shouldRequestFilesystemCredentials: true.After verification: deleted the installed
ai-provider-for-openaiplugin and theftp_credentialsoption to leave the env in a re-testable state.