From 6229601df4f92fc0a642082e3a8ae8967c022b1e Mon Sep 17 00:00:00 2001 From: peptiderepo Date: Mon, 22 Jun 2026 17:14:31 +0000 Subject: [PATCH] fix: QA remediation P1-A/P1-B/P1-C/P2 for v0.22.1 sync-mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1-A: wrap wp_schedule_single_event(GENERATE_ACTION) + fire_cron_now() in on_orchestrate_tick() with 'if ( ! self::$sync_mode )' guard. 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 before Article_Worker completes. P1-B: add two unit tests to GenerationCheckpointRunnerTest.php: - test_orchestrate_tick_in_sync_mode_does_not_schedule_background_event: asserts sync_mode=true suppresses GENERATE_ACTION schedule + spawn_cron. - test_generate_tick_async_mode_schedules_generate_action: asserts sync_mode=false (async path) still reschedules GENERATE_ACTION. P1-C: create root CONTEXT.md with domain glossary (run, run_stages, checkpoint, idea queue, Generation_Lock) and execution mode reference (async wp-cron vs --sync VPS-driven). P2: update ARCHITECTURE.md §25 Entry Points table with the 'wp prautoblogger generate --sync' VPS path row. Agent-Session: prab-reliability-remediation-20260622 --- ARCHITECTURE.md | 3 +- CHANGELOG.md | 15 ++++ CONTEXT.md | 77 +++++++++++++++++++ .../class-generation-checkpoint-runner.php | 11 ++- .../Core/GenerationCheckpointRunnerTest.php | 67 ++++++++++++++++ 5 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 CONTEXT.md diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d6208a1..e026951 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1117,7 +1117,8 @@ If the host kills any tick: | Caller | Path | |--------|------| | Board "New Article" button | AJAX `prautoblogger_generate_now` → `Generation_Status_Poller::on_ajax_generate_now()` → schedules `prautoblogger_manual_generation` → `Executor::on_manual_generation()` → `kick_off()` | -| WP-CLI | `wp prautoblogger generate` → `WP_CLI_Commands::generate_command()` → `kick_off()` | +| WP-CLI (async) | `wp prautoblogger generate` → `WP_CLI_Commands::generate_command()` → `kick_off()` | +| WP-CLI (sync / VPS) | `wp prautoblogger generate --sync` → `WP_CLI_Commands::run_sync()` → `set_sync_mode(true)` → `on_orchestrate_tick()` → tick loop `on_generate_tick()` (×N) → `set_sync_mode(false)` | | Old cron hook (retained) | `prautoblogger_manual_generation` → `Executor::on_manual_generation()` → `kick_off()` | ### SSH Workaround Retirement diff --git a/CHANGELOG.md b/CHANGELOG.md index b838b46..3fa46d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,21 @@ 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 diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..3298e2a --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,77 @@ +# PRAutoBlogger — Domain Glossary + +Quick reference for the generation pipeline domain model. For full design rationale +see `ARCHITECTURE.md` (§21–§25) and `CONVENTIONS.md`. + +--- + +## Core Domain Terms + +**run** +A single end-to-end article generation cycle: orchestrate → generate idea(s) → +finalize. Represented as a row in `wp_prautoblogger_runs` (status, cost ceiling, +timestamps). One run may produce multiple articles if the daily idea-count > 1. + +**run\_stages** (`wp_prautoblogger_run_stages`) +Sub-steps within a run (research, analysis, generation, publish). Each stage has +its own status and can be replayed independently via `Rerun_Executor`. Generation +checkpoint runs populate stages as Article_Worker progresses. + +**checkpoint** +The persistence boundary between pipeline ticks. After each tick the idea queue +and run state are written to `wp_options` so a process kill cannot lose work. The +next tick reads the queue option and continues. See §25 of ARCHITECTURE.md. + +**idea queue** (`prautoblogger_article_queue` option) +Serialized array of `Article_Idea` objects written by `on_orchestrate_tick()` and +consumed one-at-a-time by successive `on_generate_tick()` calls. Deleted on +finalize or cleanup. + +**Generation\_Lock** (`wp_prautoblogger_generation_lock` or `wp_options`) +DB mutex that prevents two concurrent runs. Acquired at the start of +`on_orchestrate_tick()`, released by `Helpers::finalize()` or on error/halt. + +--- + +## Execution Modes + +### Async (wp-cron driven) + +Default path for the Board "New Article" button and the plain +`wp prautoblogger generate` WP-CLI command. + +1. `kick_off()` schedules `prautoblogger_gen_orchestrate` and calls `fire_cron_now()`. +2. `on_orchestrate_tick()` runs in a background PHP process, schedules + `prautoblogger_gen_tick`, and calls `fire_cron_now()` to spawn the next process. +3. Each `on_generate_tick()` spawns the next until the queue drains. + +Limitation: Hostinger kills background PHP processes at ~10 min. Orchestrate tick +(LLM-free) and individual generate ticks (one LLM call each) are both well within +this limit; the checkpoint design is the mitigation. + +### Sync (VPS-driven / `--sync`) + +Used by the Coolify KVM8 VPS systemd timer via `infra/vps/orchestrate-generation.sh` +in `peptide-e2e`. Introduced v0.22.1. + +1. `run_sync()` sets `Generation_Checkpoint_Runner::$sync_mode = true`. +2. Calls `on_orchestrate_tick()` **in-process** (no background spawn). +3. Loops calling `on_generate_tick()` directly until queue empty or terminal state. +4. `$sync_mode = true` suppresses all `wp_schedule_single_event` + + `fire_cron_now` calls inside both tick methods, preventing a competing + background cron chain from forming in parallel. + +The VPS SSH session is held open; the process is not subject to Hostinger's +background-kill constraint. + +--- + +## Table Shorthand + +Throughout the codebase `wp_prautoblogger_*` tables are referenced by their +`$wpdb->` property names (e.g. `$wpdb->prab_runs`, `$wpdb->prab_run_stages`). +Full table names are defined in `class-activator.php`. + +--- + +*See also: `ARCHITECTURE.md` §25 (checkpoint design), `CONVENTIONS.md` (coding rules).* diff --git a/includes/core/class-generation-checkpoint-runner.php b/includes/core/class-generation-checkpoint-runner.php index f625e19..c48c979 100644 --- a/includes/core/class-generation-checkpoint-runner.php +++ b/includes/core/class-generation-checkpoint-runner.php @@ -175,10 +175,15 @@ class PRAutoBlogger_Generation_Checkpoint_Runner { ) ); - if ( ! wp_next_scheduled( self::GENERATE_ACTION ) ) { - wp_schedule_single_event( time(), self::GENERATE_ACTION ); + // In sync mode the VPS tick loop is the external driver; skip + // wp-cron reschedule to prevent a competing background tick. + // on_generate_tick() has the same guard for subsequent reschedules. + if ( ! self::$sync_mode ) { + if ( ! wp_next_scheduled( self::GENERATE_ACTION ) ) { + wp_schedule_single_event( time(), self::GENERATE_ACTION ); + } + PRAutoBlogger_Generation_Checkpoint_Helpers::fire_cron_now(); } - PRAutoBlogger_Generation_Checkpoint_Helpers::fire_cron_now(); } catch ( \Throwable $e ) { PRAutoBlogger_Logger::instance()->error( diff --git a/tests/unit/Core/GenerationCheckpointRunnerTest.php b/tests/unit/Core/GenerationCheckpointRunnerTest.php index 2a8e104..c7e0a97 100644 --- a/tests/unit/Core/GenerationCheckpointRunnerTest.php +++ b/tests/unit/Core/GenerationCheckpointRunnerTest.php @@ -227,4 +227,71 @@ 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' + ); + } }