diff --git a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php index 98ce19d44..11b516167 100644 --- a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php +++ b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php @@ -179,7 +179,7 @@ public function executeTask( int $jobId, array $params ): void { $write = $memory->replace_all( $content ); if ( empty( $write['success'] ) ) { - $this->failJob( $jobId, $write['message'] ?? 'Failed to write wake briefing.' ); + $this->failJob( $jobId, $write['message'] ); return; } @@ -241,10 +241,16 @@ private function resolveScope( array $params, int $agent_id ): string { /** * Gather threshold-crossing signal lines for the current blog only. * - * @param string $since Window start (UTC). + * @param string $since Window start (UTC). + * @param bool $scan_fatals Whether to include the debug.log PHP-fatals + * scan in this call. `false` when the caller + * (gatherNetworkSignals()) is about to run that + * scan exactly once at network scope instead of + * once per site — see gatherNetworkSignals() + * docblock for why. * @return string[] Terse signal lines (may be empty). */ - private function gatherSiteSignals( string $since ): array { + private function gatherSiteSignals( string $since, bool $scan_fatals = true ): array { $signals = array(); $failing = $this->getRepeatedJobFailures( $since ); @@ -262,9 +268,11 @@ private function gatherSiteSignals( string $since ): array { $signals[] = $errors; } - $fatals = $this->getPhpFatals( $since ); - if ( ! empty( $fatals ) ) { - $signals[] = $fatals; + if ( $scan_fatals ) { + $fatals = $this->getPhpFatals( $since ); + if ( ! empty( $fatals ) ) { + $signals[] = $fatals; + } } // Disk is host-global; emit it once per run, not once per blog. @@ -294,8 +302,33 @@ private function gatherSiteSignals( string $since ): array { } /** - * Gather signals across every site in the network, one labeled line per - * site that has anything to report. + * Gather signals across every site in the network: one hoisted + * network-wide line for host-global facts, plus one labeled line per + * site that has anything genuinely site-specific to report. + * + * ## Why PHP fatals are scanned once here, not once per site + * + * `wp-content/debug.log` is a single file shared by every site on a + * multisite network — WP_CONTENT_DIR is network-wide and + * resolveDebugLogPath() never reads a per-site option. So calling + * getPhpFatals() under the per-blog switch_to_blog() loop below does not + * just risk duplicate *lines*; it re-reads and re-parses the same up-to- + * 5MB tail N times for an identical result every time (see + * https://github.com/Extra-Chill/data-machine/issues/3522). Rather than + * scanning N times and then deduping N identical strings back down to + * one, this scans once, at network scope, before the per-site loop, and + * excludes it from each per-site gatherSiteSignals() call via + * `$scan_fatals = false`. That is the root fix: no per-site fact is lost + * (fatals were never a per-site fact to begin with — the same signature + * and count would surface under any site chosen), and the I/O cost drops + * from O(sites) to O(1) per run. + * + * A true "identical on N/N sites" heuristic was considered and rejected: + * it would still pay the cost of scanning the file N times just to + * discover the signatures are identical, and — because the file is + * structurally shared, not coincidentally identical — there is no + * meaningful "different" case for a same-vs-different threshold to guard + * against. * * Runs the same per-site pulse queries under switch_to_blog() and collapses * each site's signals into a single site-tagged line. Sites with nothing to @@ -303,12 +336,19 @@ private function gatherSiteSignals( string $since ): array { * would defeat the 3-second-glance bar on a large network). * * @param string $since Window start (UTC). - * @return string[] Per-site signal lines (may be empty when the whole - * network is quiet). + * @return string[] Network-wide line (if any) followed by per-site signal + * lines (may be empty when the whole network is quiet). */ private function gatherNetworkSignals( string $since ): array { $lines = array(); + // Host-global: the shared debug.log is scanned exactly once here, + // ahead of the per-site loop — see docblock above. + $fatals = $this->getPhpFatals( $since ); + if ( ! empty( $fatals ) ) { + $lines[] = sprintf( '**network-wide** — %s', $fatals ); + } + $sites = get_sites( array( 'number' => 0, @@ -323,7 +363,7 @@ private function gatherNetworkSignals( string $since ): array { $blog_id = (int) $blog_id; switch_to_blog( $blog_id ); try { - $site_signals = $this->gatherSiteSignals( $since ); + $site_signals = $this->gatherSiteSignals( $since, false ); $label = $this->siteLabel( $blog_id ); } finally { restore_current_blog(); @@ -536,24 +576,54 @@ private function getPhpFatals( string $since ): string { fgets( $handle ); // Discard the partial first line after the seek. } - $groups = array(); // signature => [ 'count' => int, 'sample' => string ]. - $total = 0; + // First pass: split the tail into discrete (possibly multi-line) log + // entries via a plain collect-as-you-go array, not a by-reference + // `use (&$current)` closure flushed mid-loop. PHPStan cannot soundly + // narrow the type of a variable that is both read/reset inside a + // closure literal and mutated by the surrounding loop after that + // literal — it infers the variable as permanently stuck at its + // value from the closure-definition site, which made every branch + // below look like dead code reachable only through an + // always-null/always-zero state. Collecting entries first, then + // grouping them in a second, ordinary loop keeps every state + // transition inside straight-line control flow that is both + // correct and statically checkable. + $entries = array(); $current = null; - $flush = function () use ( &$current, &$groups, &$total, $since_ts ) { - if ( null === $current ) { - return; + for ( $line = fgets( $handle ); false !== $line; $line = fgets( $handle ) ) { + $ts = $this->parseLogTimestamp( $line ); + if ( null !== $ts || ( '' !== $line && '[' === $line[0] ) ) { + if ( null !== $current ) { + $entries[] = $current; + } + $current = array( + 'ts' => $ts, + 'raw' => rtrim( $line, "\r\n" ), + ); + continue; + } + if ( null !== $current ) { + $current['raw'] .= "\n" . rtrim( $line, "\r\n" ); } - $entry = $current; - $current = null; + } + if ( null !== $current ) { + $entries[] = $current; + } + fclose( $handle ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose + + // Second pass: filter to the rolling window, normalize, and group. + $groups = array(); // signature => [ 'count' => int, 'sample' => string ]. + $total = 0; + foreach ( $entries as $entry ) { if ( null !== $entry['ts'] && $entry['ts'] < $since_ts ) { - return; + continue; } $norm = $this->normalizeFatal( $entry['raw'] ); if ( null === $norm ) { - return; + continue; } ++$total; @@ -564,26 +634,9 @@ private function getPhpFatals( string $since ): string { ); } ++$groups[ $norm['signature'] ]['count']; - }; - - for ( $line = fgets( $handle ); false !== $line; $line = fgets( $handle ) ) { - $ts = $this->parseLogTimestamp( $line ); - if ( null !== $ts || ( '' !== $line && '[' === $line[0] ) ) { - $flush(); - $current = array( - 'ts' => $ts, - 'raw' => rtrim( $line, "\r\n" ), - ); - continue; - } - if ( null !== $current ) { - $current['raw'] .= "\n" . rtrim( $line, "\r\n" ); - } } - $flush(); - fclose( $handle ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_fclose - if ( 0 === $total || empty( $groups ) ) { + if ( empty( $groups ) ) { return ''; } @@ -611,8 +664,17 @@ private function getPhpFatals( string $since ): string { * @return string Absolute path (may not exist), or '' when undeterminable. */ private function resolveDebugLogPath(): string { - if ( defined( 'WP_DEBUG_LOG' ) && is_string( WP_DEBUG_LOG ) && '' !== WP_DEBUG_LOG ) { - return WP_DEBUG_LOG; + // WordPress stubs type WP_DEBUG_LOG as bool, but the documented runtime + // contract is bool OR an explicit log path string — handling the string + // case is the entire reason this method exists. A direct reference (or + // constant() with a literal name) lets static analysis constant-fold to + // the stub type and declare the string branch dead. Resolving the name + // through a variable keeps the real, wider contract checkable without + // suppressing the rule or weakening the stub for everyone else. + $debug_log_constant = 'WP_DEBUG_LOG'; + $debug_log = defined( $debug_log_constant ) ? constant( $debug_log_constant ) : null; + if ( is_string( $debug_log ) && '' !== $debug_log ) { + return $debug_log; } $ini_path = ini_get( 'error_log' ); @@ -620,7 +682,9 @@ private function resolveDebugLogPath(): string { return $ini_path; } - if ( defined( 'WP_CONTENT_DIR' ) && is_string( WP_CONTENT_DIR ) && '' !== WP_CONTENT_DIR ) { + // WP_CONTENT_DIR is already typed as string by the stubs, so an + // is_string() guard here is provably redundant rather than defensive. + if ( defined( 'WP_CONTENT_DIR' ) && '' !== WP_CONTENT_DIR ) { return rtrim( WP_CONTENT_DIR, '/\\' ) . '/debug.log'; } diff --git a/tests/wake-briefing-network-fatals-dedupe-smoke.php b/tests/wake-briefing-network-fatals-dedupe-smoke.php new file mode 100644 index 000000000..21c7d21bc --- /dev/null +++ b/tests/wake-briefing-network-fatals-dedupe-smoke.php @@ -0,0 +1,314 @@ + 'https://extrachill.com', + 2 => 'https://community.extrachill.com', + 7 => 'https://events.extrachill.com', + ); + + function get_current_blog_id(): int { + return end( $GLOBALS['__wake_blog_stack'] ); + } + + function switch_to_blog( int $blog_id ): bool { + $GLOBALS['__wake_blog_stack'][] = $blog_id; + return true; + } + + function restore_current_blog(): bool { + if ( count( $GLOBALS['__wake_blog_stack'] ) > 1 ) { + array_pop( $GLOBALS['__wake_blog_stack'] ); + } + return true; + } + + function get_sites( array $args = array() ): array { + unset( $args ); + return array_keys( $GLOBALS['__wake_site_urls'] ); + } + + function home_url( string $path = '' ): string { + return $GLOBALS['__wake_site_urls'][ get_current_blog_id() ] . $path; + } + + function wp_parse_url( string $url, int $component = -1 ) { + return parse_url( $url, $component ); + } + + function is_multisite(): bool { + return true; + } + + // In-memory filter registry so apply_filters() returns overrides. + $GLOBALS['__wake_filters'] = array(); + + function apply_filters( string $hook, $value, ...$rest ) { + if ( array_key_exists( $hook, $GLOBALS['__wake_filters'] ) ) { + return $GLOBALS['__wake_filters'][ $hook ]; + } + return $value; + } + function do_action( ...$args ) {} + + function wake_set_filter( string $hook, $value ): void { + $GLOBALS['__wake_filters'][ $hook ] = $value; + } + + // Force disk pressure permanently quiet — this fixture is about fatals. + wake_set_filter( 'datamachine_wake_briefing_disk_min_free_pct', 0.0 ); + wake_set_filter( 'datamachine_wake_briefing_disk_min_free_bytes', 0.0 ); + + $failed = 0; + $total = 0; + + function wake_assert( string $name, bool $condition, string $detail = '' ): void { + global $failed, $total; + ++$total; + if ( $condition ) { + echo " [PASS] {$name}\n"; + return; + } + ++$failed; + echo " [FAIL] {$name}" . ( $detail ? " — {$detail}" : '' ) . "\n"; + } + + echo "=== wake-briefing-network-fatals-dedupe-smoke ===\n"; + + require_once $root . '/inc/Engine/AI/System/Tasks/SystemTask.php'; + require_once $root . '/inc/Engine/AI/System/Tasks/WakeBriefingTask.php'; + + // ----------------------------------------------------------------------- + // Fake $wpdb: only blog 7 (events.extrachill.com) has genuinely + // per-site facts — stuck jobs, a repeatedly-failing task type, and + // grouped errors — mirroring the shape reported in issue #3522. + // Blogs 1 and 2 are otherwise quiet aside from the shared fatal. + // ----------------------------------------------------------------------- + + $GLOBALS['wpdb'] = new class() { + public string $prefix = 'wp_'; + + public function prepare( string $query, ...$args ): array { + if ( 1 === count( $args ) && is_array( $args[0] ) ) { + $args = $args[0]; + } + return array( + 'sql' => $query, + 'args' => $args, + ); + } + + public function get_results( $prepared, $output = ARRAY_A ) { + if ( 7 !== get_current_blog_id() ) { + return array(); + } + if ( str_contains( $prepared['sql'], 'GROUP BY task_type' ) ) { + return array( + array( + 'task_type' => 'unknown', + 'n' => 128, + ), + ); + } + if ( str_contains( $prepared['sql'], 'GROUP BY message' ) ) { + return array( + array( + 'message' => 'Job marked as failed', + 'n' => 134, + ), + ); + } + return array(); + } + + public function get_var( $prepared = null ) { + if ( 7 === get_current_blog_id() ) { + return '15'; + } + return '0'; + } + + public function get_row( $prepared, $output = ARRAY_A ) { + return null; + } + }; + + // ----------------------------------------------------------------------- + // Shared debug.log: one PHP fatal signature, structurally identical no + // matter which blog is "current" when it is read. + // ----------------------------------------------------------------------- + + $log = tempnam( sys_get_temp_dir(), 'wake-network-debug-' ); + $now = time(); + $recent = gmdate( 'd-M-Y H:i:s', $now - 600 ) . ' UTC'; + $since = gmdate( 'Y-m-d H:i:s', $now - ( 24 * 3600 ) ); + + $fatal_body = array_fill( + 0, + 8, + "[{$recent}] PHP Fatal error: Cannot redeclare function ec_link_page_owner_compatibility() in /var/www/wp-content/plugins/extrachill-link-pages/owner-reference.php on line 12" + ); + file_put_contents( $log, implode( "\n", $fatal_body ) . "\n" ); + + $prev_error_log = ini_get( 'error_log' ); + ini_set( 'error_log', $log ); + + $ref = new ReflectionClass( WakeBriefingTask::class ); + $task = $ref->newInstanceWithoutConstructor(); + + $invoke = function ( string $method, array $args = array() ) use ( $ref, $task ) { + $m = $ref->getMethod( $method ); + return $m->invoke( $task, ...$args ); + }; + + // ----------------------------------------------------------------------- + // 1-4: gatherNetworkSignals() across the 3-site fixture. + // ----------------------------------------------------------------------- + + $lines = $invoke( 'gatherNetworkSignals', array( $since ) ); + + $network_wide_lines = array_values( + array_filter( $lines, static fn( $l ) => str_starts_with( (string) $l, '**network-wide**' ) ) + ); + wake_assert( + 'network-wide: exactly one hoisted line for the shared fatal', + 1 === count( $network_wide_lines ), + 'got: ' . json_encode( $lines ) + ); + + $network_line = (string) ( $network_wide_lines[0] ?? '' ); + wake_assert( + 'network-wide: names the fatal count and signature', + str_contains( $network_line, '8 PHP fatal(s)' ) + && str_contains( $network_line, 'ec_link_page_owner_compatibility' ), + "got: {$network_line}" + ); + + wake_assert( + 'network-wide: hoisted line renders before per-site lines', + ! empty( $lines ) && '**network-wide**' === substr( (string) $lines[0], 0, 16 ), + 'got: ' . json_encode( $lines ) + ); + + $site_lines = array_values( + array_filter( $lines, static fn( $l ) => ! str_starts_with( (string) $l, '**network-wide**' ) ) + ); + wake_assert( + 'per-site: no site line repeats the PHP-fatal text', + 0 === count( array_filter( $site_lines, static fn( $l ) => str_contains( (string) $l, 'PHP fatal' ) ) ), + 'got: ' . json_encode( $site_lines ) + ); + + wake_assert( + 'per-site: exactly one site line (only events.extrachill.com has facts)', + 1 === count( $site_lines ), + 'got: ' . json_encode( $site_lines ) + ); + + $events_line = (string) ( $site_lines[0] ?? '' ); + wake_assert( + 'per-site: the genuinely site-specific facts still render on their own site line', + str_starts_with( $events_line, '**events.extrachill.com**' ) + && str_contains( $events_line, '15 job(s) stuck in processing' ) + && str_contains( $events_line, 'unknown' ) + && str_contains( $events_line, '128' ) + && str_contains( $events_line, '134 error(s) logged' ), + "got: {$events_line}" + ); + + // ----------------------------------------------------------------------- + // 5. Single-site scope is unaffected: gatherSiteSignals() with its + // default $scan_fatals=true still includes the fatal directly (no + // network-wide hoist applies outside the multisite loop). + // ----------------------------------------------------------------------- + + $GLOBALS['__wake_blog_stack'] = array( 1 ); + $site_scope_signals = $invoke( 'gatherSiteSignals', array( $since ) ); + wake_assert( + 'single-site scope: fatals still render directly (regression guard)', + 1 === count( array_filter( $site_scope_signals, static fn( $l ) => str_contains( (string) $l, '8 PHP fatal(s)' ) ) ), + 'got: ' . json_encode( $site_scope_signals ) + ); + + // Explicit opt-out still works standalone (what gatherNetworkSignals() + // relies on internally). + $scan_disabled = $invoke( 'gatherSiteSignals', array( $since, false ) ); + wake_assert( + 'gatherSiteSignals($since, false): suppresses the fatals scan entirely', + 0 === count( array_filter( $scan_disabled, static fn( $l ) => str_contains( (string) $l, 'PHP fatal' ) ) ), + 'got: ' . json_encode( $scan_disabled ) + ); + + ini_set( 'error_log', false === $prev_error_log ? '' : $prev_error_log ); + @unlink( $log ); + + // ----------------------------------------------------------------------- + + if ( $failed > 0 ) { + echo "\nwake-briefing-network-fatals-dedupe-smoke failed: {$failed}/{$total} assertions failed.\n"; + exit( 1 ); + } + + echo "\nwake-briefing-network-fatals-dedupe-smoke passed: {$total} assertions.\n"; +}