Add external cron service integration #324
Conversation
Introduces external cron service support for offloading WordPress cron jobs to a dedicated service. This improves reliability and performance by eliminating dependency on site traffic for scheduled tasks. New components: - External_Cron_Manager: Core manager for registration and configuration - External_Cron_Registration: Handles site registration with the service - External_Cron_Schedule_Reporter: Reports WP cron schedules to the service - External_Cron_Service_Client: API client for service communication - External_Cron_Admin_Page: Admin dashboard for monitoring and management Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a complete External Cron Service integration for WP Ultimo, enabling sites to offload WordPress cron execution to an external service. The implementation spans a service client, registration manager, schedule reporter, central coordinator, admin interface, and dashboard UI. Changes
Sequence DiagramssequenceDiagram
participant Admin as Admin User
participant Page as External Cron Admin Page
participant Manager as External Cron Manager
participant Client as Service Client
participant API as External Service API
participant Options as Site Options
Admin->>Page: Clicks Connect Network
Page->>Manager: register_network()
Manager->>Client: register_site(site_data)
Client->>API: POST /register (OAuth)
API-->>Client: {site_id, api_key, api_secret}
Client-->>Manager: Success response
Manager->>Options: Save credentials & enable
Manager-->>Page: Success
Page-->>Admin: Reload page / Display connected status
sequenceDiagram
participant Admin as Admin User
participant Page as External Cron Admin Page
participant Manager as External Cron Manager
participant Reporter as Schedule Reporter
participant Client as Service Client
participant API as External Service API
Admin->>Page: Clicks Sync Schedules
Page->>Manager: sync_schedules()
Manager->>Reporter: report_all_schedules()
Reporter->>Reporter: get_site_schedules()
Reporter->>Reporter: get_action_scheduler_jobs()
Reporter->>Client: update_schedules(site_id, schedules)
Client->>API: POST /sites/{site_id}/schedules
API-->>Client: Success
Client-->>Reporter: Success
Reporter-->>Manager: Complete
Manager->>Options: Update last_sync
Manager-->>Page: Success
Page-->>Admin: Reload / Show sync confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@inc/admin-pages/class-external-cron-admin-page.php`:
- Around line 394-415: The ajax_toggle method currently casts
wu_request('enabled', false) with (bool) which treats the string "false" as
true; update ajax_toggle to parse the request value with
filter_var(wu_request('enabled', false), FILTER_VALIDATE_BOOLEAN) (keeping the
same default) and use that boolean for wu_save_setting('external_cron_enabled',
$enabled) and in the payload sent via wp_send_json_success so literal strings
like "false"/"0"/"off" are handled correctly.
- Around line 276-294: The get_connect_url() method is double-encoding the
redirect parameters by wrapping network_site_url() and $return_url in
rawurlencode() before passing them to add_query_arg(); remove the rawurlencode()
calls and pass the raw $return_url and network_site_url() values to
add_query_arg() so add_query_arg() can perform proper encoding for the
external_cron_connect, site_url and return_url parameters when building the URI
with self::SERVICE_URL.
In `@inc/external-cron/class-external-cron-manager.php`:
- Around line 106-201: The code currently only checks is_service_active() before
disabling WP-Cron and running schedule/sync/heartbeat; update
maybe_disable_wp_cron(), schedule_sync(), schedule_heartbeat(),
sync_schedules(), and send_heartbeat() to also require registration by gating
them on is_registered() (or create a shared readiness helper like is_ready()
that returns is_service_active() && is_registered() and call that) so WP-Cron
isn’t disabled and actions aren’t scheduled or sent until the site has a site
ID.
In `@inc/external-cron/class-external-cron-registration.php`:
- Around line 48-85: The register_network method currently assumes $result
contains 'site_id', 'api_key', and 'api_secret' and writes them directly; add
defensive validation after $result is returned from
$this->client->register_site: confirm $result is an array (or not a WP_Error)
and that isset($result['site_id'], $result['api_key'], $result['api_secret'])
(and optionally validate they are non-empty strings), and if any are missing
return a WP_Error indicating a malformed API response; only call
wu_save_setting(...) and $this->client->set_credentials(...) after these checks
pass to avoid PHP notices and invalid state.
In `@inc/external-cron/class-external-cron-service-client.php`:
- Around line 267-270: The get_site_hash() method currently concatenates
AUTH_KEY directly and can silently fail if AUTH_KEY is undefined or empty;
update get_site_hash() to first determine an $auth_key by using AUTH_KEY when
defined and non-empty and otherwise falling back to a deterministic alternative
(for example wp_salt('auth') or another stable site-specific value), then return
hash('sha256', network_site_url() . $auth_key); ensure you reference
get_site_hash(), AUTH_KEY, and network_site_url() when making the change.
In `@views/external-cron/dashboard.php`:
- Around line 163-181: The row rendering calls
human_time_diff(strtotime($log['execution_time']), time()) without checking
strtotime() result; if strtotime() returns false you must avoid passing false to
human_time_diff. Update the recent_logs loop (the block using
$log['execution_time'], strtotime, and human_time_diff) to first cast or
validate $timestamp = strtotime($log['execution_time']); if $timestamp === false
then render '-' (or the raw value) instead of calling human_time_diff, otherwise
call human_time_diff($timestamp, time()) and escape the output via
esc_html/esc_attr as currently done.
🧹 Nitpick comments (6)
inc/external-cron/class-external-cron-registration.php (2)
136-160: Consider batching or pagination for large multisite networks.Using
'number' => 0retrieves all sites at once, which could exhaust memory on networks with thousands of subsites. Consider processing in batches or adding a limit with multiple iterations.Proposed batch approach
public function register_all_subsites(): array { - $sites = get_sites( - [ - 'number' => 0, - 'fields' => 'ids', - 'archived' => 0, - 'deleted' => 0, - ] - ); $results = []; + $offset = 0; + $batch = 100; + + do { + $sites = get_sites( + [ + 'number' => $batch, + 'offset' => $offset, + 'fields' => 'ids', + 'archived' => 0, + 'deleted' => 0, + ] + ); - foreach ($sites as $blog_id) { - // Skip if already registered. - $existing = get_blog_option($blog_id, 'external_cron_site_id'); - if (! empty($existing)) { - $results[ $blog_id ] = ['skipped' => true]; - continue; + foreach ($sites as $blog_id) { + $existing = get_blog_option($blog_id, 'external_cron_site_id'); + if (! empty($existing)) { + $results[ $blog_id ] = ['skipped' => true]; + continue; + } + $results[ $blog_id ] = $this->register_subsite($blog_id); } - $results[ $blog_id ] = $this->register_subsite($blog_id); - } + $offset += $batch; + } while (count($sites) === $batch); return $results; }
203-212: Duplicate site hash generation logic exists in both this class and the service client.
generate_site_hash()here duplicatesget_site_hash()inExternal_Cron_Service_Client. Consider consolidating to avoid drift.inc/external-cron/class-external-cron-schedule-reporter.php (3)
63-80: Error fromupdate_schedules()is silently ignored.If the API call fails, there's no logging or feedback. Consider handling the error or logging for debugging purposes.
Proposed fix
// Send to service. - $this->client->update_schedules((int) $site_id, $schedules); + $result = $this->client->update_schedules((int) $site_id, $schedules); + + if (is_wp_error($result)) { + wu_log_add('external-cron', sprintf('Failed to update schedules: %s', $result->get_error_message())); + } }
145-164: Remove unused loop variable$key.Static analysis flagged this unused variable.
Proposed fix
- foreach ($events as $key => $event) { + foreach ($events as $event) {
177-211: Action Scheduler query is limited to 500 jobs without indication of truncation.If there are more than 500 pending actions, the sync will be incomplete. Consider either paginating through all results or logging when the limit is reached.
views/external-cron/dashboard.php (1)
198-292: Consider using WordPress admin notices instead ofalert()for better UX.Using JavaScript
alert()blocks the UI and isn't consistent with WordPress admin patterns. Consider using WordPress admin notices or inline feedback for a more polished experience.
| public function get_connect_url(): string { | ||
|
|
||
| $return_url = add_query_arg( | ||
| [ | ||
| 'page' => $this->id, | ||
| 'action' => 'connect_callback', | ||
| ], | ||
| network_admin_url('admin.php') | ||
| ); | ||
|
|
||
| return add_query_arg( | ||
| [ | ||
| 'action' => 'external_cron_connect', | ||
| 'site_url' => rawurlencode(network_site_url()), | ||
| 'return_url' => rawurlencode($return_url), | ||
| ], | ||
| self::SERVICE_URL . '/wp-json/cron-service/v1/oauth/authorize' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
WordPress add_query_arg() encoding behavior parameter values
💡 Result:
Short answer: add_query_arg() does not automatically encode parameter values — it expects values to be pre-encoded (urlencode() or rawurlencode()). If you pass unencoded values that contain & or =, they will be treated as additional query separators (possible parameter hijack). Use rawurlencode() (or array_map('rawurlencode', $args)) for values and late-escape the returned URL with esc_url(). [1][2]
Example:
- Wrong: add_query_arg( 'my_arg', 'a&b=1', $url ); // produces ...&my_arg=a&b=1 (broken)
- Right: add_query_arg( 'my_arg', rawurlencode('a&b=1'), $url ); // produces ...&my_arg=a%26b%3D1
References:
- WordPress add_query_arg() docs. [1]
- WPVIP guidance on encoding values passed to add_query_arg(). [2]
🌐 Web query:
WordPress add_query_arg() double encoding rawurlencode issue
💡 Result:
Short answer: double-encoding happens when you feed add_query_arg() an already-encoded string (or an encoded URL) and then encode it again. Fixes: pass raw (unencoded) values to add_query_arg(), or if you have an encoded value decode it first and then encode exactly once. Sources and examples below.
Why it happens
- add_query_arg()/related functions expect query values to be URL-encoded in a single, consistent way; feeding already-encoded values (or an encoded URL) will produce %25 sequences (double-encoding) or incorrectly treat encoded ampersands as anchors. (developer.wordpress.org)
Recommended fixes (pick one)
- Pass unencoded values and let WordPress handle encoding
- Preferred and simplest:
$url = add_query_arg( 'foo', $value /* unencoded */, $url );
- If you must handle encoding yourself, avoid double-encode:
$value = rawurldecode( $possibly_encoded_value ); // remove prior encoding
$url = add_query_arg( 'foo', rawurlencode( $value ), $url ); - Build the query yourself when you need full control:
$url = $base . '?' . http_build_query( $params ); // http_build_query encodes once
Notes/warnings
- Look for %25 in parameters to detect double-encoding. (utmguard.com)
- There are open/related Trac tickets and docs discussing behavior when input URLs are already encoded; be careful when passing encoded URLs into add_query_arg(). (core.trac.wordpress.org)
References
- add_query_arg() documentation (expects proper encoding). [WordPress Developer Reference]. (developer.wordpress.org)
- WP VIP guidance: encode values (rawurlencode) or map array values; shows examples and pitfalls. (docs.wpvip.com)
- Trac ticket and historical notes about encoded input URL handling / double-encoding issues. (core.trac.wordpress.org)
If you want, paste the exact code and URL you're getting and I’ll show the minimal change to avoid double-encoding.
Citations:
- 1: https://developer.wordpress.org/reference/functions/add_query_arg/?utm_source=openai
- 2: https://www.utmguard.com/docs/syntax/double-url-encoding?utm_source=openai
- 3: https://core.trac.wordpress.org/ticket/63462?utm_source=openai
- 4: https://developer.wordpress.org/reference/functions/add_query_arg/?utm_source=openai
- 5: https://docs.wpvip.com/security/encode-values-add-query-arg/?utm_source=openai
- 6: https://core.trac.wordpress.org/ticket/63462?utm_source=openai
🏁 Script executed:
# Check if there's WordPress documentation or comments in the codebase about add_query_arg behavior
cd / && find . -name "*.php" -type f 2>/dev/null | head -1 | xargs -I {} bash -c "grep -r 'add_query_arg' {} 2>/dev/null | head -5" || echo "Searching in repo for add_query_arg usage patterns"
rg -i "add_query_arg" --type php -A 3 -B 3 | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 4297
Remove rawurlencode() calls to prevent double-encoding in the OAuth redirect URL.
add_query_arg() applies URL encoding internally when processing query parameters. Pre-encoding $return_url and network_site_url() with rawurlencode() causes double-encoding (e.g., % becomes %25), which breaks the OAuth callback redirect. Pass these values unencoded to add_query_arg().
✅ Suggested fix
@@
return add_query_arg(
[
'action' => 'external_cron_connect',
- 'site_url' => rawurlencode(network_site_url()),
- 'return_url' => rawurlencode($return_url),
+ 'site_url' => network_site_url(),
+ 'return_url' => $return_url,
],
self::SERVICE_URL . '/wp-json/cron-service/v1/oauth/authorize'
);📝 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.
| public function get_connect_url(): string { | |
| $return_url = add_query_arg( | |
| [ | |
| 'page' => $this->id, | |
| 'action' => 'connect_callback', | |
| ], | |
| network_admin_url('admin.php') | |
| ); | |
| return add_query_arg( | |
| [ | |
| 'action' => 'external_cron_connect', | |
| 'site_url' => rawurlencode(network_site_url()), | |
| 'return_url' => rawurlencode($return_url), | |
| ], | |
| self::SERVICE_URL . '/wp-json/cron-service/v1/oauth/authorize' | |
| ); | |
| } | |
| public function get_connect_url(): string { | |
| $return_url = add_query_arg( | |
| [ | |
| 'page' => $this->id, | |
| 'action' => 'connect_callback', | |
| ], | |
| network_admin_url('admin.php') | |
| ); | |
| return add_query_arg( | |
| [ | |
| 'action' => 'external_cron_connect', | |
| 'site_url' => network_site_url(), | |
| 'return_url' => $return_url, | |
| ], | |
| self::SERVICE_URL . '/wp-json/cron-service/v1/oauth/authorize' | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@inc/admin-pages/class-external-cron-admin-page.php` around lines 276 - 294,
The get_connect_url() method is double-encoding the redirect parameters by
wrapping network_site_url() and $return_url in rawurlencode() before passing
them to add_query_arg(); remove the rawurlencode() calls and pass the raw
$return_url and network_site_url() values to add_query_arg() so add_query_arg()
can perform proper encoding for the external_cron_connect, site_url and
return_url parameters when building the URI with self::SERVICE_URL.
| public function ajax_toggle(): void { | ||
|
|
||
| check_ajax_referer('wu_external_cron_nonce', 'nonce'); | ||
|
|
||
| if ( ! current_user_can('manage_network')) { | ||
| wp_send_json_error(['message' => __('Permission denied.', 'ultimate-multisite')]); | ||
| } | ||
|
|
||
| $enabled = (bool) wu_request('enabled', false); | ||
|
|
||
| wu_save_setting('external_cron_enabled', $enabled); | ||
|
|
||
| $message = $enabled | ||
| ? __('External Cron Service enabled.', 'ultimate-multisite') | ||
| : __('External Cron Service disabled.', 'ultimate-multisite'); | ||
|
|
||
| wp_send_json_success( | ||
| [ | ||
| 'message' => $message, | ||
| 'enabled' => $enabled, | ||
| ] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PHP boolean cast string "false" behavior FILTER_VALIDATE_BOOLEAN
💡 Result:
Short answer:
-
Casting the string "false" to boolean with (bool) yields true — because any non-empty string is truthy in PHP (the only string treated as false is "0"). [1][2]
-
filter_var($str, FILTER_VALIDATE_BOOLEAN) treats the literal strings "false", "off", "no", "0" (case‑insensitive) as boolean false and "true","on","yes","1" as true. Use the FILTER_NULL_ON_FAILURE flag to get NULL for unrecognized values instead of FALSE. [3]
Notes:
- filter_var always casts the input to string before validating, so behavior is string-based. [3]
Sources:
- PHP types / boolean conversion. [1]
- PHP language: converting to boolean (non-empty strings are true; "0" is false). [2]
- PHP filter docs: FILTER_VALIDATE_BOOLEAN / FILTER_VALIDATE_BOOL and FILTER_NULL_ON_FAILURE. [3]
🌐 Web query:
PHP how to parse boolean values from request strings best practice
💡 Result:
Short answer — validate and cast explicitly at the boundary (don’t rely on implicit truthiness). Use filter_var(..., FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) (or framework helpers) so common strings map correctly and invalid values are detectable.
Why
- PHP request data are strings; implicit (bool) casts are surprising (e.g. "0" is false but "false" is truthy if you rely on non-empty-string rules). Explicit parsing avoids bugs. [5][2]
Recommended approaches
- Plain PHP (recommended)
- Accept these canonical truthy/falsy strings and detect invalid inputs:
- Use FILTER_VALIDATE_BOOLEAN to convert "1","true","on","yes" → true and "0","false","off","no","" → false.
- Use FILTER_NULL_ON_FAILURE to return null when the input is not recognized (so you can validate/return error).
Example:
$raw = $_GET['active'] ?? null;
$bool = filter_var($raw, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
if ($bool === null) { /* invalid value: reject / default */ }
// $bool is true/false otherwise
Reference: PHP filter_var / FILTER_VALIDATE_BOOLEAN docs. [1]
- Framework helper (if using Laravel)
- Use $request->boolean('flag') which implements the same mapping (1, "1", "true", "on", "yes" → true; else false). Use validation rules for required/nullable when you need strictness. [3]
- Additional best practices
- Cast early (in controller/request DTO) and use typed properties/return types for downstream code. Validate presence vs. value separately (isset / request method checks). [4][5]
- Avoid $_REQUEST (ambiguous source) and avoid relying on (bool)$string or (int)$string for boolean semantics. [5][2]
Other utilities
- boolval() simply casts truthiness (not suitable for parsing textual booleans reliably). Use only when you explicitly want PHP’s truthiness rules. [2]
References
- PHP filter_var / FILTER_VALIDATE_BOOLEAN. [1]
- PHP boolval / casting behavior. [2]
- Laravel Request::boolean helper. [3]
- Practical casting / validate-and-cast-early guidance. [4]
- Notes on request superglobals and string input behavior. [5]
🏁 Script executed:
# Check if the file exists and verify the code snippet
find . -name "class-external-cron-admin-page.php" -type fRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 133
🏁 Script executed:
# Search for wu_request function to understand its behavior
rg "function wu_request" -A 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 570
🏁 Script executed:
# Verify the exact code at lines 394-415 in the file
sed -n '394,415p' ./inc/admin-pages/class-external-cron-admin-page.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 668
Boolean parsing mis-handles "false" strings from request data.
In PHP, (bool) 'false' evaluates to true because any non-empty string is truthy. When AJAX sends enabled=false, wu_request() returns the string "false", which then converts to true, inverting the intended logic.
Use filter_var(wu_request('enabled', false), FILTER_VALIDATE_BOOLEAN) instead, which correctly parses the literal strings "false", "0", "off", "no" as boolean false and "true", "1", "on", "yes" as true.
✅ Suggested fix
- $enabled = (bool) wu_request('enabled', false);
+ $enabled = filter_var(wu_request('enabled', false), FILTER_VALIDATE_BOOLEAN);🤖 Prompt for AI Agents
In `@inc/admin-pages/class-external-cron-admin-page.php` around lines 394 - 415,
The ajax_toggle method currently casts wu_request('enabled', false) with (bool)
which treats the string "false" as true; update ajax_toggle to parse the request
value with filter_var(wu_request('enabled', false), FILTER_VALIDATE_BOOLEAN)
(keeping the same default) and use that boolean for
wu_save_setting('external_cron_enabled', $enabled) and in the payload sent via
wp_send_json_success so literal strings like "false"/"0"/"off" are handled
correctly.
| public function maybe_disable_wp_cron(): void { | ||
|
|
||
| if ( ! $this->is_service_active()) { | ||
| return; | ||
| } | ||
|
|
||
| if ( ! defined('DISABLE_WP_CRON')) { | ||
| define('DISABLE_WP_CRON', true); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check if external cron service is active. | ||
| * | ||
| * @since 2.3.0 | ||
| * @return bool | ||
| */ | ||
| public function is_service_active(): bool { | ||
|
|
||
| return (bool) wu_get_setting('external_cron_enabled', false); | ||
| } | ||
|
|
||
| /** | ||
| * Check if site is registered with the service. | ||
| * | ||
| * @since 2.3.0 | ||
| * @return bool | ||
| */ | ||
| public function is_registered(): bool { | ||
|
|
||
| $site_id = wu_get_setting('external_cron_site_id'); | ||
|
|
||
| return ! empty($site_id); | ||
| } | ||
|
|
||
| /** | ||
| * Schedule the sync action. | ||
| * | ||
| * @since 2.3.0 | ||
| */ | ||
| private function schedule_sync(): void { | ||
|
|
||
| if ( ! $this->is_service_active()) { | ||
| return; | ||
| } | ||
|
|
||
| if ( ! wu_next_scheduled_action('wu_external_cron_sync_schedules')) { | ||
| wu_schedule_recurring_action(time() + HOUR_IN_SECONDS, HOUR_IN_SECONDS, 'wu_external_cron_sync_schedules'); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Schedule the heartbeat action. | ||
| * | ||
| * @since 2.3.0 | ||
| */ | ||
| private function schedule_heartbeat(): void { | ||
|
|
||
| if ( ! $this->is_service_active()) { | ||
| return; | ||
| } | ||
|
|
||
| if ( ! wu_next_scheduled_action('wu_external_cron_heartbeat')) { | ||
| wu_schedule_recurring_action(time() + 300, 300, 'wu_external_cron_heartbeat'); // Every 5 minutes. | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sync schedules with the service. | ||
| * | ||
| * @since 2.3.0 | ||
| */ | ||
| public function sync_schedules(): void { | ||
|
|
||
| if ( ! $this->is_service_active()) { | ||
| return; | ||
| } | ||
|
|
||
| $this->reporter->report_all_schedules(); | ||
|
|
||
| update_site_option('wu_external_cron_last_sync', time()); | ||
| } | ||
|
|
||
| /** | ||
| * Send heartbeat to the service. | ||
| * | ||
| * @since 2.3.0 | ||
| */ | ||
| public function send_heartbeat(): void { | ||
|
|
||
| if ( ! $this->is_service_active()) { | ||
| return; | ||
| } | ||
|
|
||
| $this->client->heartbeat(); | ||
| } |
There was a problem hiding this comment.
Guard disable/sync/heartbeat on registration to avoid stopping cron.
If the feature is enabled before registration, WP cron is disabled and scheduled tasks stop, while sync/heartbeat may hit the service without a site ID. Gate these paths on is_registered() (or a shared helper).
✅ Suggested fix (shared readiness check)
@@
public function is_registered(): bool {
@@
return ! empty($site_id);
}
+
+ /**
+ * Check if service is enabled and registered.
+ *
+ * `@since` 2.3.0
+ * `@return` bool
+ */
+ private function is_service_ready(): bool {
+ return $this->is_service_active() && $this->is_registered();
+ }
@@
- if ( ! $this->is_service_active()) {
+ if ( ! $this->is_service_ready()) {
return;
}
@@
- if ( ! $this->is_service_active()) {
+ if ( ! $this->is_service_ready()) {
return;
}
@@
- if ( ! $this->is_service_active()) {
+ if ( ! $this->is_service_ready()) {
return;
}
@@
- if ( ! $this->is_service_active()) {
+ if ( ! $this->is_service_ready()) {
return;
}
@@
- if ( ! $this->is_service_active()) {
+ if ( ! $this->is_service_ready()) {
return;
}🤖 Prompt for AI Agents
In `@inc/external-cron/class-external-cron-manager.php` around lines 106 - 201,
The code currently only checks is_service_active() before disabling WP-Cron and
running schedule/sync/heartbeat; update maybe_disable_wp_cron(),
schedule_sync(), schedule_heartbeat(), sync_schedules(), and send_heartbeat() to
also require registration by gating them on is_registered() (or create a shared
readiness helper like is_ready() that returns is_service_active() &&
is_registered() and call that) so WP-Cron isn’t disabled and actions aren’t
scheduled or sent until the site has a site ID.
| public function register_network() { | ||
|
|
||
| $granularity = wu_get_setting('external_cron_granularity', 'network'); | ||
|
|
||
| $data = [ | ||
| 'site_url' => network_site_url(), | ||
| 'site_hash' => $this->client->get_site_hash(), | ||
| 'is_network_registration' => true, | ||
| 'granularity' => $granularity, | ||
| 'timezone' => wp_timezone_string(), | ||
| 'cron_url' => site_url('wp-cron.php'), | ||
| ]; | ||
|
|
||
| $result = $this->client->register_site($data); | ||
|
|
||
| if (is_wp_error($result)) { | ||
| return $result; | ||
| } | ||
|
|
||
| // Save credentials. | ||
| wu_save_setting('external_cron_site_id', $result['site_id']); | ||
| wu_save_setting('external_cron_api_key', $result['api_key']); | ||
| wu_save_setting('external_cron_api_secret', $result['api_secret']); | ||
| wu_save_setting('external_cron_enabled', true); | ||
|
|
||
| // Update client with new credentials. | ||
| $this->client->set_credentials($result['api_key'], $result['api_secret']); | ||
|
|
||
| /** | ||
| * Fires after network is registered with external cron service. | ||
| * | ||
| * @since 2.3.0 | ||
| * @param array $result Registration result. | ||
| */ | ||
| do_action('wu_external_cron_network_registered', $result); | ||
|
|
||
| return $result; | ||
| } |
There was a problem hiding this comment.
Add defensive checks before accessing array keys from API response.
If the API response is malformed or missing expected keys (site_id, api_key, api_secret), this will cause PHP notices/warnings. Add validation before persisting credentials.
Proposed fix
if (is_wp_error($result)) {
return $result;
}
+ // Validate required fields in response.
+ if (empty($result['site_id']) || empty($result['api_key']) || empty($result['api_secret'])) {
+ return new \WP_Error('invalid_response', __('Invalid registration response from service.', 'ultimate-multisite'));
+ }
+
// Save credentials.
wu_save_setting('external_cron_site_id', $result['site_id']);🤖 Prompt for AI Agents
In `@inc/external-cron/class-external-cron-registration.php` around lines 48 - 85,
The register_network method currently assumes $result contains 'site_id',
'api_key', and 'api_secret' and writes them directly; add defensive validation
after $result is returned from $this->client->register_site: confirm $result is
an array (or not a WP_Error) and that isset($result['site_id'],
$result['api_key'], $result['api_secret']) (and optionally validate they are
non-empty strings), and if any are missing return a WP_Error indicating a
malformed API response; only call wu_save_setting(...) and
$this->client->set_credentials(...) after these checks pass to avoid PHP notices
and invalid state.
| public function get_site_hash(): string { | ||
|
|
||
| return hash('sha256', network_site_url() . AUTH_KEY); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'AUTH_KEY' --type=php -C2Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 839
🏁 Script executed:
rg -n "defined\s*\(" --type=php -C1 | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1829
🏁 Script executed:
# Check if there are any try-catch blocks or error handling in the external-cron files
rg -n "try|catch|defined|empty|isset" inc/external-cron/ --type=php -C1Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 4156
🏁 Script executed:
# Check WordPress documentation/comments about AUTH_KEY in the files
rg -n "AUTH_KEY|wp-config" inc/external-cron/ --type=php -B3 -A3Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1060
🏁 Script executed:
# Check if AUTH_KEY is initialized/checked anywhere in the codebase
rg -n "AUTH_KEY|SECURE_AUTH_KEY|LOGGED_IN_KEY" --type=php -B2 -A2Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 839
🏁 Script executed:
# Check the main plugin file or initialization code
git ls-files | grep -E "(^[^/]*\.php$|main\.php|plugin\.php|index\.php)" | head -10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 316
🏁 Script executed:
# Check constants.php and the main plugin file for AUTH_KEY handling
cat -n constants.php | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 849
🏁 Script executed:
# Check if AUTH_KEY is used/validated in the main plugin file
rg -n "AUTH_KEY" ultimate-multisite.php -B3 -A3Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
Add fallback for AUTH_KEY constant to handle edge cases.
Although WordPress defines AUTH_KEY in wp-config.php, defensive programming patterns in this codebase (e.g., checks for DISABLE_WP_CRON, WP_CLI) suggest awareness of edge cases. The get_site_hash() method should use a fallback when AUTH_KEY is empty or undefined to prevent silent failures in misconfigured environments.
🤖 Prompt for AI Agents
In `@inc/external-cron/class-external-cron-service-client.php` around lines 267 -
270, The get_site_hash() method currently concatenates AUTH_KEY directly and can
silently fail if AUTH_KEY is undefined or empty; update get_site_hash() to first
determine an $auth_key by using AUTH_KEY when defined and non-empty and
otherwise falling back to a deterministic alternative (for example
wp_salt('auth') or another stable site-specific value), then return
hash('sha256', network_site_url() . $auth_key); ensure you reference
get_site_hash(), AUTH_KEY, and network_site_url() when making the change.
| <?php foreach ($recent_logs as $log) : ?> | ||
| <tr> | ||
| <td><code><?php echo esc_html($log['hook_name'] ?? ''); ?></code></td> | ||
| <td> | ||
| <?php | ||
| $status = $log['status'] ?? 'unknown'; | ||
| $status_class = 'wu-text-gray-600'; | ||
| if ('success' === $status) { | ||
| $status_class = 'wu-text-green-600'; | ||
| } elseif (in_array($status, ['failed', 'timeout'], true)) { | ||
| $status_class = 'wu-text-red-600'; | ||
| } | ||
| ?> | ||
| <span class="<?php echo esc_attr($status_class); ?>"><?php echo esc_html(ucfirst($status)); ?></span> | ||
| </td> | ||
| <td><?php echo isset($log['duration_ms']) ? esc_html($log['duration_ms'] . 'ms') : '-'; ?></td> | ||
| <td><?php echo isset($log['execution_time']) ? esc_html(human_time_diff(strtotime($log['execution_time']), time()) . ' ' . __('ago', 'ultimate-multisite')) : '-'; ?></td> | ||
| </tr> | ||
| <?php endforeach; ?> |
There was a problem hiding this comment.
Add defensive check for strtotime() returning false.
If $log['execution_time'] is malformed, strtotime() returns false, which would cause incorrect output from human_time_diff().
Proposed fix
- <td><?php echo isset($log['execution_time']) ? esc_html(human_time_diff(strtotime($log['execution_time']), time()) . ' ' . __('ago', 'ultimate-multisite')) : '-'; ?></td>
+ <td><?php
+ $exec_time = isset($log['execution_time']) ? strtotime($log['execution_time']) : false;
+ echo $exec_time ? esc_html(human_time_diff($exec_time, time()) . ' ' . __('ago', 'ultimate-multisite')) : '-';
+ ?></td>📝 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.
| <?php foreach ($recent_logs as $log) : ?> | |
| <tr> | |
| <td><code><?php echo esc_html($log['hook_name'] ?? ''); ?></code></td> | |
| <td> | |
| <?php | |
| $status = $log['status'] ?? 'unknown'; | |
| $status_class = 'wu-text-gray-600'; | |
| if ('success' === $status) { | |
| $status_class = 'wu-text-green-600'; | |
| } elseif (in_array($status, ['failed', 'timeout'], true)) { | |
| $status_class = 'wu-text-red-600'; | |
| } | |
| ?> | |
| <span class="<?php echo esc_attr($status_class); ?>"><?php echo esc_html(ucfirst($status)); ?></span> | |
| </td> | |
| <td><?php echo isset($log['duration_ms']) ? esc_html($log['duration_ms'] . 'ms') : '-'; ?></td> | |
| <td><?php echo isset($log['execution_time']) ? esc_html(human_time_diff(strtotime($log['execution_time']), time()) . ' ' . __('ago', 'ultimate-multisite')) : '-'; ?></td> | |
| </tr> | |
| <?php endforeach; ?> | |
| <?php foreach ($recent_logs as $log) : ?> | |
| <tr> | |
| <td><code><?php echo esc_html($log['hook_name'] ?? ''); ?></code></td> | |
| <td> | |
| <?php | |
| $status = $log['status'] ?? 'unknown'; | |
| $status_class = 'wu-text-gray-600'; | |
| if ('success' === $status) { | |
| $status_class = 'wu-text-green-600'; | |
| } elseif (in_array($status, ['failed', 'timeout'], true)) { | |
| $status_class = 'wu-text-red-600'; | |
| } | |
| ?> | |
| <span class="<?php echo esc_attr($status_class); ?>"><?php echo esc_html(ucfirst($status)); ?></span> | |
| </td> | |
| <td><?php echo isset($log['duration_ms']) ? esc_html($log['duration_ms'] . 'ms') : '-'; ?></td> | |
| <td><?php | |
| $exec_time = isset($log['execution_time']) ? strtotime($log['execution_time']) : false; | |
| echo $exec_time ? esc_html(human_time_diff($exec_time, time()) . ' ' . __('ago', 'ultimate-multisite')) : '-'; | |
| ?></td> | |
| </tr> | |
| <?php endforeach; ?> |
🤖 Prompt for AI Agents
In `@views/external-cron/dashboard.php` around lines 163 - 181, The row rendering
calls human_time_diff(strtotime($log['execution_time']), time()) without
checking strtotime() result; if strtotime() returns false you must avoid passing
false to human_time_diff. Update the recent_logs loop (the block using
$log['execution_time'], strtotime, and human_time_diff) to first cast or
validate $timestamp = strtotime($log['execution_time']); if $timestamp === false
then render '-' (or the raw value) instead of calling human_time_diff, otherwise
call human_time_diff($timestamp, time()) and escape the output via
esc_html/esc_attr as currently done.
Summary
New Components
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.