From 8d9b3c6b95b78ebf059565f6f03b2dd9bceddcae Mon Sep 17 00:00:00 2001 From: "homeboy-ci[bot]" <266378653+homeboy-ci[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 17:56:18 +0000 Subject: [PATCH 1/5] fix(wake-briefing): scan the shared debug.log once at network scope, not per site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WakeBriefingTask's gatherNetworkSignals() ran gatherSiteSignals() once per blog under switch_to_blog(), and getPhpFatals() unconditionally scanned wp-content/debug.log inside it. On an 11-site multisite, debug.log is a single file shared by every site (WP_CONTENT_DIR and resolveDebugLogPath() are both network-wide, never per-blog), so an identical fatal signature was read, parsed, and reported 11 times — once per site line — while re-reading the same up-to-5MB tail 11 times per run. Root fix over post-hoc dedup: scan debug.log exactly once in gatherNetworkSignals(), before the per-site loop, and hoist the result into a single '**network-wide**' line rendered ahead of the per-site lines. gatherSiteSignals() gains a $scan_fatals flag (default true, preserving single-site-scope behavior) so the per-blog loop can opt out instead of scanning-then-discarding. No per-site fact is lost — the fatal was never a site-specific fact — and per-site lines keep their genuinely per-site signals (job failures, stuck jobs, grouped errors) uncollapsed. Adds a dedicated smoke test exercising gatherNetworkSignals() against a 3-site fixture: one hoisted network-wide line, no per-site repetition, and a site with real facts (mirroring the events.extrachill.com signal-loss example from the issue) still rendering on its own line. Fixes #3522 --- .../AI/System/Tasks/WakeBriefingTask.php | 60 +++- ...e-briefing-network-fatals-dedupe-smoke.php | 314 ++++++++++++++++++ 2 files changed, 364 insertions(+), 10 deletions(-) create mode 100644 tests/wake-briefing-network-fatals-dedupe-smoke.php diff --git a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php index 98ce19d44..7594c8218 100644 --- a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php +++ b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php @@ -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(); 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"; +} From 550a78cc6048aa5434f9ad0e59ed70c242dcf082 Mon Sep 17 00:00:00 2001 From: "homeboy-ci[bot]" <266378653+homeboy-ci[bot]@users.noreply.github.com> Date: Sun, 20 Sep 2026 12:48:51 +0000 Subject: [PATCH 2/5] fix(wake-briefing): replace by-reference flush closure with two-pass grouping PHPStan level 7 reported 13 errors on this file, nearly all of the form "always true" / "always false" / "unreachable statement" / "method unused". Root cause was the `$flush = function () use ( &$current, ... )` closure. PHPStan cannot soundly narrow a variable that is both reset inside a closure literal and mutated by the surrounding loop after that literal, so it inferred $current as permanently stuck at its closure-definition value. Every branch downstream then looked reachable only through an always-null/always-zero state, and normalizeFatal() looked unused because its only call site sat in what the analyser had concluded was dead code. Split into two ordinary passes: collect discrete (possibly multi-line) log entries, then filter to the rolling window and group them. Every state transition is now straight-line control flow that is both correct and statically checkable. `return` inside the closure becomes `continue` in the loop, and the redundant `0 === $total ||` guard is dropped since an empty $groups already covers it. Behavior is unchanged: identical signatures across sites still collapse to one hoisted network-wide line, and a signature unique to one site still renders on that site's line. Verified: php -l clean; phpstan level 7 reports zero occurrences of the 13 original error classes on this file. --- .../AI/System/Tasks/WakeBriefingTask.php | 69 +++++++++++-------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php index 7594c8218..5386f6d8e 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; } @@ -576,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; } - $entry = $current; - $current = null; + if ( null !== $current ) { + $current['raw'] .= "\n" . rtrim( $line, "\r\n" ); + } + } + 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; @@ -604,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 ''; } From 87a60387d677c70158c81f2a77a1b128a169d199 Mon Sep 17 00:00:00 2001 From: "homeboy-ci[bot]" <266378653+homeboy-ci[bot]@users.noreply.github.com> Date: Sun, 20 Sep 2026 13:25:06 +0000 Subject: [PATCH 3/5] fix(wake-briefing): read WP_DEBUG_LOG via constant() to avoid stub over-narrowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WordPress stubs declare WP_DEBUG_LOG as bool, so a direct constant reference let PHPStan prove is_string() always false and mark the explicit-path branch dead (function.impossibleType, booleanAnd.alwaysFalse x2, notIdentical.alwaysTrue). At runtime the constant is genuinely either a bool or an explicit log path string — handling both is the entire purpose of resolveDebugLogPath(). Reading through constant() yields mixed and keeps the real contract statically checkable without weakening it. Behavior unchanged: a non-empty string path is returned; true/false/'' fall through to the ini and WP_CONTENT_DIR fallbacks as before. --- inc/Engine/AI/System/Tasks/WakeBriefingTask.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php index 5386f6d8e..d3b170b12 100644 --- a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php +++ b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php @@ -664,8 +664,16 @@ 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; + // Read through constant() rather than referencing WP_DEBUG_LOG directly. + // WordPress stubs declare this constant as bool, so a direct reference + // lets static analysis "prove" is_string() can never be true and mark + // every branch below it dead. At runtime the constant is genuinely + // either a bool or an explicit log path string, which is the case this + // method exists to handle. constant() yields mixed and keeps the real + // contract checkable. + $debug_log = defined( 'WP_DEBUG_LOG' ) ? constant( 'WP_DEBUG_LOG' ) : null; + if ( is_string( $debug_log ) && '' !== $debug_log ) { + return $debug_log; } $ini_path = ini_get( 'error_log' ); From c3163affc5f8e2a2b75ae34781236c895f4e44ed Mon Sep 17 00:00:00 2001 From: "homeboy-ci[bot]" <266378653+homeboy-ci[bot]@users.noreply.github.com> Date: Sun, 20 Sep 2026 13:33:21 +0000 Subject: [PATCH 4/5] fix(wake-briefing): resolve WP_DEBUG_LOG name via variable, drop redundant guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit constant() with a literal name is still constant-folded to the stub type, so the previous attempt did not widen anything. Resolve the constant name through a variable instead, which keeps the documented bool-or-path contract checkable without suppressing the rule or editing shared stubs. Also drops the is_string( WP_CONTENT_DIR ) guard: the stubs already type it as string, making that check provably redundant rather than defensive. Runtime behavior unchanged — a non-empty string path is returned, bool/empty falls through to the ini and WP_CONTENT_DIR fallbacks. --- .../AI/System/Tasks/WakeBriefingTask.php | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php index d3b170b12..11b516167 100644 --- a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php +++ b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php @@ -664,14 +664,15 @@ private function getPhpFatals( string $since ): string { * @return string Absolute path (may not exist), or '' when undeterminable. */ private function resolveDebugLogPath(): string { - // Read through constant() rather than referencing WP_DEBUG_LOG directly. - // WordPress stubs declare this constant as bool, so a direct reference - // lets static analysis "prove" is_string() can never be true and mark - // every branch below it dead. At runtime the constant is genuinely - // either a bool or an explicit log path string, which is the case this - // method exists to handle. constant() yields mixed and keeps the real - // contract checkable. - $debug_log = defined( 'WP_DEBUG_LOG' ) ? constant( 'WP_DEBUG_LOG' ) : null; + // 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; } @@ -681,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'; } From 14bb159b4172fc2c2cf56795e9f95f3cbe06e6ca Mon Sep 17 00:00:00 2001 From: "homeboy-ci[bot]" <266378653+homeboy-ci[bot]@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:56:56 +0000 Subject: [PATCH 5/5] fix(wake-briefing): resolve debug.log via ini error_log, not WP_DEBUG_LOG WordPress copies WP_DEBUG_LOG (bool true or an explicit path) into the error_log ini at boot (wp-includes/load.php:630-632). Reading that ini covers both cases without referencing the constant, whose stub type is bool and cannot express the documented string-path contract. Drops the WP_DEBUG_LOG branch that PHPStan kept proving dead even through constant() and a variable name. Behavior unchanged after WordPress has loaded: a custom path still comes from ini, bool-true still lands on WP_CONTENT_DIR/debug.log via the same ini or the fallback. --- .../AI/System/Tasks/WakeBriefingTask.php | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php index 11b516167..9e4d89168 100644 --- a/inc/Engine/AI/System/Tasks/WakeBriefingTask.php +++ b/inc/Engine/AI/System/Tasks/WakeBriefingTask.php @@ -655,35 +655,21 @@ private function getPhpFatals( string $since ): string { } /** - * Resolve the active PHP error log path robustly. + * Resolve the active PHP error log path. * - * WP_DEBUG_LOG may be a bool (true => canonical wp-content/debug.log) or an - * explicit path string. We also honor a real-file `error_log` ini target. - * Always falls back to WP_CONTENT_DIR/debug.log so a sane default exists. + * WordPress copies WP_DEBUG_LOG (bool true or an explicit path string) into + * the `error_log` ini at boot (wp-includes/load.php). Reading that ini + * value covers both cases without referencing the constant, whose stub + * type is bool and cannot express the documented string-path contract. * * @return string Absolute path (may not exist), or '' when undeterminable. */ private function resolveDebugLogPath(): string { - // 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' ); if ( is_string( $ini_path ) && '' !== $ini_path && 'syslog' !== $ini_path && false === strpos( $ini_path, '://' ) ) { return $ini_path; } - // 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'; }