diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fa46d5..f00d1c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,21 +13,6 @@ and this project uses [Semantic Versioning](https://semver.org/). ## [0.22.1] - 2026-06-22 -### Fixed (remediation 2026-06-23) -- **`on_orchestrate_tick()` sync-mode double-drive** (P1 QA finding): added - `if ( ! self::$sync_mode )` guard around `wp_schedule_single_event(GENERATE_ACTION)` + - `fire_cron_now()` in `on_orchestrate_tick()`. Mirrors the existing guard in - `on_generate_tick()`. Prevents a background wp-cron process spawning in parallel - with the VPS `--sync` loop and prematurely finalizing the run lock. -- **Sync-mode unit tests**: added `test_orchestrate_tick_in_sync_mode_does_not_schedule_background_event` - and `test_generate_tick_async_mode_schedules_generate_action` to - `GenerationCheckpointRunnerTest.php`. -- **Root `CONTEXT.md`**: created domain glossary (run/run_stages/checkpoint/idea-queue/ - Generation_Lock) and execution mode comparison (async wp-cron vs. `--sync` VPS). -- **`ARCHITECTURE.md` §25**: added `WP-CLI (sync / VPS)` row to Entry Points table. - -## [0.22.1] - 2026-06-22 - ### Added - **`wp prautoblogger generate --sync`** (VPS orchestrator mode): runs the full checkpoint pipeline synchronously in the calling process — acquires lock, calls @@ -57,6 +42,17 @@ and this project uses [Semantic Versioning](https://semver.org/). scheduling as of v0.22.1. Note: wp-cron reliability on this Hostinger host is suspect (background PHP killed ~10 min); maintenance crons are low-stakes and acceptable on this transport; generation is not. +- **Sync-mode guard tests** (QA NF-1 remediation 2026-06-23): moved sync-mode + tests to `GenerationCheckpointRunnerSyncModeTest.php`; fixed + `test_orchestrate_tick_in_sync_mode_does_not_schedule_background_event` to + return >= 1 idea from `orchestrate_only()` so execution reaches the sync guard + at line ~181 (was vacuous -- previous stub returned `[]` causing early return); + added `test_generate_tick_sync_mode_suppresses_reschedule` for the + `on_generate_tick()` guard; async regression test preserved in same file. +- **Root `CONTEXT.md`**: created domain glossary (run/run_stages/checkpoint/ + idea-queue/Generation_Lock) and execution mode comparison (async wp-cron vs. + `--sync` VPS). +- **`ARCHITECTURE.md` SS25**: added `WP-CLI (sync / VPS)` row to Entry Points table. ## [0.22.0] - 2026-06-16 @@ -525,7 +521,6 @@ and this project uses [Semantic Versioning](https://semver.org/). the catalog + OpenRouter static list. Breaking change: callers that expect a specific static list should note the new dynamic Runware side. - ### Fixed - Runware image generation now correctly uses the admin-selected model instead of silently falling back to FLUX.1 schnell (runware:100@1) for all non-schnell @@ -625,7 +620,6 @@ and this project uses [Semantic Versioning](https://semver.org/). ### Fixed - Image prompt builder now properly delegates to static parser methods - ### Fixed - **Opik autoloader gap** — `class-autoloader.php` listed `services/` as a scan directory but not `services/opik/`, where all four Opik classes live (`Opik_Client`, `Opik_Trace_Context`, `Opik_Span_Queue`, `Opik_Dispatcher`). Composer's classmap (used in PHPUnit) covers `includes/` recursively so tests passed, but the WordPress runtime autoloader would fatal on first Opik class reference in production. Added `services/opik/` to the scan list. diff --git a/tests/unit/Core/GenerationCheckpointRunnerSyncModeTest.php b/tests/unit/Core/GenerationCheckpointRunnerSyncModeTest.php new file mode 100644 index 0000000..9a3e63a --- /dev/null +++ b/tests/unit/Core/GenerationCheckpointRunnerSyncModeTest.php @@ -0,0 +1,246 @@ + on_orchestrate_tick() must NOT schedule GENERATE_ACTION + * or call spawn_cron/fire_cron_now (VPS loop drives ticks directly). + * NON-VACUOUS: orchestrate_only() returns 1 idea so execution reaches the guard + * at line ~181, not the empty-ideas early return at ~148. + * set_sync_mode(true) -> on_generate_tick() with ideas remaining must NOT schedule + * GENERATE_ACTION or call spawn_cron/fire_cron_now. + * set_sync_mode(false) -> on_generate_tick() with ideas remaining DOES schedule + * GENERATE_ACTION (regression guard -- async path unaffected by sync guard). + * + * @package PRAutoBlogger\Tests\Core + */ + +namespace PRAutoBlogger\Tests\Core; + +use PRAutoBlogger\Tests\BaseTestCase; +use Brain\Monkey\Functions; + +class GenerationCheckpointRunnerSyncModeTest extends BaseTestCase { + + /** @var string[] Cron hooks captured from wp_schedule_single_event(). */ + private array $scheduled = array(); + + /** @var array Option writes captured from update_option(). */ + private array $options = array(); + + /** @var bool Tracks whether spawn_cron was called. */ + private bool $spawn_called = false; + + protected function setUp(): void { + parent::setUp(); + $this->scheduled = array(); + $this->options = array(); + $this->spawn_called = false; + + Functions\when( 'wp_schedule_single_event' )->alias( + function ( $ts, $hook, $args = array() ) { + $this->scheduled[] = $hook; + return true; + } + ); + Functions\when( 'wp_next_scheduled' )->justReturn( false ); + Functions\when( 'spawn_cron' )->alias( function () { + $this->spawn_called = true; + return true; + } ); + Functions\when( 'wp_remote_post' )->justReturn( array() ); + Functions\when( 'site_url' )->justReturn( 'https://example.com' ); + + Functions\when( 'set_transient' )->justReturn( true ); + Functions\when( 'update_option' )->alias( + function ( $name, $value, $autoload = null ) { + $this->options[ $name ] = $value; + return true; + } + ); + Functions\when( 'delete_option' )->alias( + function ( $name ) { + unset( $this->options[ $name ] ); + return true; + } + ); + Functions\when( 'get_option' )->alias( + function ( $name, $default = false ) { + return $this->options[ $name ] ?? $default; + } + ); + Functions\when( 'wp_generate_uuid4' )->justReturn( 'sync-test-uuid-001' ); + $this->options['prautoblogger_monthly_budget_usd'] = '0'; + Functions\when( 'wp_cache_get' )->justReturn( false ); + Functions\when( 'wp_cache_set' )->justReturn( true ); + Functions\when( 'wp_cache_delete' )->justReturn( true ); + \PRAutoBlogger_Run_State::flush_cache(); + } + + protected function tearDown(): void { + \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( false ); + \PRAutoBlogger_Run_State::flush_cache(); + unset( $GLOBALS['wpdb'] ); + parent::tearDown(); + } + + /** Stub the generation lock to always succeed. */ + private function wire_wpdb_for_lock(): void { + $wpdb = $this->create_mock_wpdb(); + $wpdb->options = 'wp_options'; + $GLOBALS['wpdb'] = $wpdb; + $wpdb->method( 'query' )->willReturn( 1 ); + $wpdb->method( 'get_var' )->willReturn( null ); + } + + /** Build a minimal Article_Idea-compatible array. */ + private function make_idea( string $topic = 'Sync Topic' ): array { + return array( + 'topic' => $topic, + 'article_type' => 'guide', + 'suggested_title' => 'Guide to ' . $topic, + 'summary' => '', + 'score' => 0.9, + 'analysis_id' => 1, + 'source_ids' => array(), + 'key_points' => array(), + 'target_keywords' => array(), + ); + } + + // __ sync-mode guard: on_orchestrate_tick() ______________________________________________ + + /** + * sync_mode=true + >=1 idea returned: on_orchestrate_tick() must not schedule + * GENERATE_ACTION or call spawn_cron (VPS loop drives generate ticks directly). + * + * NON-VACUOUS: orchestrate_only() returns 1 idea so execution reaches the sync + * guard at line ~181, not the empty-ideas early return at ~148. + */ + public function test_orchestrate_tick_in_sync_mode_does_not_schedule_background_event(): void { + $this->wire_wpdb_for_lock(); + + // Provide Pipeline_Runner stub that returns 1 idea -- forces execution past + // the empty-ideas early return so the sync guard at line ~181 is exercised. + if ( ! class_exists( 'PRAutoBlogger_Pipeline_Runner', false ) ) { + // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired + eval( 'class PRAutoBlogger_Pipeline_Runner { + public function set_skip_dedup($v){ return $this; } + public function orchestrate_only($ct){ + $idea = new PRAutoBlogger_Article_Idea([ + "topic"=>"Sync Topic","article_type"=>"guide", + "suggested_title"=>"Guide","summary"=>"","score"=>0.9, + "analysis_id"=>1,"source_ids"=>[],"key_points"=>[], + "target_keywords"=>[] + ]); + return [$idea]; + } + }' ); + } + + \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( true ); + \PRAutoBlogger_Generation_Checkpoint_Runner::on_orchestrate_tick(); + + $this->assertNotContains( + \PRAutoBlogger_Generation_Checkpoint_Runner::GENERATE_ACTION, + $this->scheduled, + 'sync mode: on_orchestrate_tick must not schedule GENERATE_ACTION' + ); + $this->assertFalse( + $this->spawn_called, + 'sync mode: on_orchestrate_tick must not call spawn_cron' + ); + } + + // __ sync-mode guard: on_generate_tick() _________________________________________________ + + /** + * sync_mode=true + 2 ideas in queue: on_generate_tick() must not reschedule + * GENERATE_ACTION or call spawn_cron (VPS loop is the sole driver of next tick). + */ + public function test_generate_tick_sync_mode_suppresses_reschedule(): void { + $run_id = 'sync-gen-tick-uuid'; + $this->options['prautoblogger_article_queue'] = array( + 'run_id' => $run_id, + 'ideas' => array( $this->make_idea( 'A' ), $this->make_idea( 'B' ) ), + 'results' => array( 'generated' => 0, 'published' => 0, 'rejected' => 0, 'cost' => 0.0 ), + ); + + $wpdb = $this->create_mock_wpdb(); + $wpdb->options = 'wp_options'; + $GLOBALS['wpdb'] = $wpdb; + $wpdb->method( 'get_row' )->willReturn( array( 'status' => 'running', 'ceiling_usd' => '0' ) ); + $wpdb->method( 'query' )->willReturn( 1 ); + $wpdb->method( 'get_var' )->willReturn( null ); + + if ( ! class_exists( 'PRAutoBlogger_Article_Worker', false ) ) { + // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired + eval( 'class PRAutoBlogger_Article_Worker { + public function __construct($ct){} + public function generate($idea){ return ["generated"=>1,"published"=>1,"rejected"=>0,"cost"=>0.01]; } + }' ); + } + if ( ! class_exists( 'PRAutoBlogger_Post_Assembler', false ) ) { + // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired + eval( 'class PRAutoBlogger_Post_Assembler { public static function amortize_research_costs($r){} }' ); + } + + \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( true ); + \PRAutoBlogger_Generation_Checkpoint_Runner::on_generate_tick(); + + $this->assertNotContains( + \PRAutoBlogger_Generation_Checkpoint_Runner::GENERATE_ACTION, + $this->scheduled, + 'sync mode: on_generate_tick must not schedule GENERATE_ACTION' + ); + $this->assertFalse( + $this->spawn_called, + 'sync mode: on_generate_tick must not call spawn_cron' + ); + } + + // __ async regression guard ______________________________________________________________ + + /** + * sync_mode=false: on_generate_tick() with ideas remaining DOES schedule + * GENERATE_ACTION (regression guard -- async path must be unaffected by sync guard). + */ + public function test_generate_tick_async_mode_schedules_generate_action(): void { + $run_id = 'async-regression-uuid'; + $this->options['prautoblogger_article_queue'] = array( + 'run_id' => $run_id, + 'ideas' => array( $this->make_idea( 'A' ), $this->make_idea( 'B' ) ), + 'results' => array( 'generated' => 0, 'published' => 0, 'rejected' => 0, 'cost' => 0.0 ), + ); + + $wpdb = $this->create_mock_wpdb(); + $wpdb->options = 'wp_options'; + $GLOBALS['wpdb'] = $wpdb; + $wpdb->method( 'get_row' )->willReturn( array( 'status' => 'running', 'ceiling_usd' => '0' ) ); + $wpdb->method( 'query' )->willReturn( 1 ); + $wpdb->method( 'get_var' )->willReturn( null ); + + if ( ! class_exists( 'PRAutoBlogger_Article_Worker', false ) ) { + // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired + eval( 'class PRAutoBlogger_Article_Worker { + public function __construct($ct){} + public function generate($idea){ return ["generated"=>1,"published"=>1,"rejected"=>0,"cost"=>0.01]; } + }' ); + } + if ( ! class_exists( 'PRAutoBlogger_Post_Assembler', false ) ) { + // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired + eval( 'class PRAutoBlogger_Post_Assembler { public static function amortize_research_costs($r){} }' ); + } + + \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( false ); + \PRAutoBlogger_Generation_Checkpoint_Runner::on_generate_tick(); + + $this->assertContains( + \PRAutoBlogger_Generation_Checkpoint_Runner::GENERATE_ACTION, + $this->scheduled, + 'async mode: on_generate_tick must schedule GENERATE_ACTION when ideas remain' + ); + } +} diff --git a/tests/unit/Core/GenerationCheckpointRunnerTest.php b/tests/unit/Core/GenerationCheckpointRunnerTest.php index c7e0a97..f9a7229 100644 --- a/tests/unit/Core/GenerationCheckpointRunnerTest.php +++ b/tests/unit/Core/GenerationCheckpointRunnerTest.php @@ -6,6 +6,9 @@ * kick_off() -- schedules cron + writes initial transient, no pipeline call. * on_generate_tick() -- pops ONE idea, reschedules or finalizes; halted run aborts early. * + * Sync-mode guard tests live in GenerationCheckpointRunnerSyncModeTest.php + * (split to comply with the 300-line rule). + * * @package PRAutoBlogger\Tests\Core */ @@ -104,7 +107,7 @@ class GenerationCheckpointRunnerTest extends BaseTestCase { ); } - // ── kick_off() ──────────────────────────────────────────────────────── + // __ kick_off() __________________________________________________________________________ /** * kick_off() schedules exactly the ORCHESTRATE cron action and writes @@ -129,7 +132,7 @@ class GenerationCheckpointRunnerTest extends BaseTestCase { $this->assertTrue( $found_running_transient, 'kick_off() must write a running status transient' ); } - // ── on_generate_tick() ──────────────────────────────────────────────── + // __ on_generate_tick() __________________________________________________________________ /** * An empty queue causes generate tick to finalize without scheduling @@ -227,71 +230,4 @@ class GenerationCheckpointRunnerTest extends BaseTestCase { $this->assertCount( 1, $remaining, 'Queue must have exactly 1 idea left after first tick' ); } } - - // ── sync mode guards ────────────────────────────────────────────────── - - /** - * sync_mode=true: on_orchestrate_tick() must not schedule GENERATE_ACTION - * or call spawn_cron (VPS loop drives generate ticks directly). - */ - public function test_orchestrate_tick_in_sync_mode_does_not_schedule_background_event(): void { - $this->wire_wpdb_for_lock(); - if ( ! class_exists( 'PRAutoBlogger_Pipeline_Runner', false ) ) { - // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired - eval( 'class PRAutoBlogger_Pipeline_Runner { - public function set_skip_dedup($v){ return $this; } - public function orchestrate_only($ct){ return []; } - }' ); - } - $spawn_called = false; - Functions\when( 'spawn_cron' )->alias( function() use ( &$spawn_called ) { - $spawn_called = true; - } ); - \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( true ); - \PRAutoBlogger_Generation_Checkpoint_Runner::on_orchestrate_tick(); - \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( false ); - $this->assertNotContains( - \PRAutoBlogger_Generation_Checkpoint_Runner::GENERATE_ACTION, - $this->scheduled, - 'sync mode: on_orchestrate_tick must not schedule GENERATE_ACTION' - ); - $this->assertFalse( $spawn_called, 'sync mode: must not call spawn_cron' ); - } - - /** - * sync_mode=false: on_generate_tick() with ideas remaining DOES schedule - * GENERATE_ACTION (regression guard — async path unaffected by sync guard). - */ - public function test_generate_tick_async_mode_schedules_generate_action(): void { - $run_id = 'async-regression-uuid'; - $this->options['prautoblogger_article_queue'] = array( - 'run_id' => $run_id, - 'ideas' => array( $this->make_idea( 'A' ), $this->make_idea( 'B' ) ), - 'results' => array( 'generated' => 0, 'published' => 0, 'rejected' => 0, 'cost' => 0.0 ), - ); - $wpdb = $this->create_mock_wpdb(); - $wpdb->options = 'wp_options'; - $GLOBALS['wpdb'] = $wpdb; - $wpdb->method( 'get_row' )->willReturn( array( 'status' => 'running', 'ceiling_usd' => '0' ) ); - $wpdb->method( 'query' )->willReturn( 1 ); - $wpdb->method( 'get_var' )->willReturn( null ); - if ( ! class_exists( 'PRAutoBlogger_Article_Worker', false ) ) { - // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired - eval( 'class PRAutoBlogger_Article_Worker { - public function __construct($ct){} - public function generate($idea){ return ["generated"=>1,"published"=>1,"rejected"=>0,"cost"=>0.01]; } - }' ); - } - if ( ! class_exists( 'PRAutoBlogger_Post_Assembler', false ) ) { - // phpcs:ignore Squiz.Strings.DoubleQuoteUsage.NotRequired - eval( 'class PRAutoBlogger_Post_Assembler { public static function amortize_research_costs($r){} }' ); - } - \PRAutoBlogger_Generation_Checkpoint_Runner::set_sync_mode( false ); - \PRAutoBlogger_Generation_Checkpoint_Runner::on_generate_tick(); - $this->assertContains( - \PRAutoBlogger_Generation_Checkpoint_Runner::GENERATE_ACTION, - $this->scheduled, - 'async mode: on_generate_tick must schedule GENERATE_ACTION when ideas remain' - ); - } }