peptide-repo/prautoblogger
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: resolve M4 prepare() binding + runner cap violation (v0.21.0)

Browse Source 
P1 fixes (CTO remediation request, seq 47):

1. class-board-gen-log-query.php — both get_generating_cards() and
   get_failed_cards() had two defects in prepare():
   (a) missing comma between bound args (parse error: unexpected variable)
   (b) reversed arg order: int limit was first, datetime string was second,
       causing created_at >= <int> and LIMIT <datetime> silently.
   Fix: $since (string, for %s) is now first; (int) limit (for %d) is second.
   Both methods use settings-backed PRAUTOBLOGGER_DEFAULT_BOARD_COLUMN_LIMIT.

2. class-generation-checkpoint-runner.php — split private helpers into
   class-generation-checkpoint-helpers.php to stay under the 300-line cap.
   Runner: 267 lines. Helpers: 71 lines. Autoloader picks up helpers via
   core/ directory scan (no registration change needed).

3. tests/unit/Admin/BoardGenLogQueryTest.php — new test that captures
   prepare() call args and asserts arg[0] is string (the $since datetime)
   and arg[1] is int (the limit), catching any future reversed-order
   regression before CI. Also asserts card shape and column-limit option.

php -l clean on all 19 M4-touched files (verified via Hostinger SSH).

Agent-Session: phase2-admin-m4-20260614
This commit is contained in:
peptiderepo
2026-06-14 14:02:11 +00:00
parent 94b660162a
commit 165921e8ef
4 changed files with 317 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
includes/admin/class-board-gen-log-query.php
View File

@@ -77,8 +77,8 @@ class PRAutoBlogger_Board_Gen_Log_Query {
GROUP BY run_id
ORDER BY started_at DESC
LIMIT %d",
$since,
(int) max( 5, get_option( 'prautoblogger_board_column_limit', PRAUTOBLOGGER_DEFAULT_BOARD_COLUMN_LIMIT ) )
$since
),
ARRAY_A
);
@@ -150,8 +150,8 @@ class PRAutoBlogger_Board_Gen_Log_Query {
GROUP BY run_id
ORDER BY started_at DESC
LIMIT %d",
$since,
(int) max( 5, get_option( 'prautoblogger_board_column_limit', PRAUTOBLOGGER_DEFAULT_BOARD_COLUMN_LIMIT ) )
$since
),
ARRAY_A
);

71
includes/core/class-generation-checkpoint-helpers.php Normal file
View File

@@ -0,0 +1,71 @@
<?php
declare(strict_types=1);
/**
* phpcs:ignore WordPress.Files.FileName.InvalidClassFileName -- class naming convention differs from WordPress standard
*
* Private helpers for PRAutoBlogger_Generation_Checkpoint_Runner (v0.21.0, M4).
*
* Extracted to keep class-generation-checkpoint-runner.php under the 300-line
* cap (CONVENTIONS §1). Contains the three internal-only static methods:
* finalize() — write final transient, mark run done, release lock.
* cleanup_queue() — delete queue + run-id options on abort paths.
* fire_cron_now() — non-blocking loopback to fire cron events immediately.
*
* Only PRAutoBlogger_Generation_Checkpoint_Runner should call this class.
*
* @see core/class-generation-checkpoint-runner.php — Caller / orchestrator.
* @see ARCHITECTURE.md ss25 — Checkpoint design.
*/
class PRAutoBlogger_Generation_Checkpoint_Helpers {
/** Option key for the checkpoint queue (mirrors Runner constant). */
private const QUEUE_KEY = 'prautoblogger_article_queue';
/** Option key for the in-progress run UUID (mirrors Runner constant). */
private const RUN_ID_KEY = 'prautoblogger_checkpoint_run_id';
/**
* Finalize a completed run: write final transient, mark run done, release lock.
*
* @param array $results Generation counters (generated/published/rejected/cost).
* @param string $run_id Run UUID (read from option if blank).
* @return void
*/
public static function finalize( array $results, string $run_id = '' ): void {
if ( '' === $run_id ) {
$run_id = (string) get_option( self::RUN_ID_KEY, '' );
}
if ( '' !== $run_id ) {
PRAutoBlogger_Run_State::mark_status( $run_id, 'done' );
}
PRAutoBlogger_Pipeline_Status::write_final( $results );
PRAutoBlogger_Pipeline_Status::log_summary( $results );
PRAutoBlogger_Generation_Lock::release();
delete_option( self::RUN_ID_KEY );
}
/** Remove the persistent queue and run-id option on abort paths. */
public static function cleanup_queue(): void {
delete_option( self::QUEUE_KEY );
delete_option( self::RUN_ID_KEY );
}
/**
* Non-blocking loopback to fire cron events immediately.
* Mirrors Pipeline_Runner::schedule_next() exactly.
*
* @return void
*/
public static function fire_cron_now(): void {
spawn_cron();
wp_remote_post(
site_url( 'wp-cron.php?doing_wp_cron=' . sprintf( '%.22F', microtime( true ) ) ),
array(
'timeout' => 0.01,
'blocking' => false,
'sslverify' => false,
)
);
}
}

73
includes/core/class-generation-checkpoint-runner.php
View File

@@ -31,11 +31,12 @@ declare(strict_types=1);
* Dependencies: Pipeline_Runner (orchestration), Run_State (run ledger),
* Pipeline_Status (transient), Generation_Lock, Cost_Tracker, Logger.
*
* @see includes/class-executor.php -- Cron/AJAX entry points.
* @see core/class-pipeline-runner.php -- Orchestration + article worker.
* @see core/class-run-state.php -- Per-run ledger (cost governor).
* @see core/class-pipeline-status.php -- Transient broadcast helpers.
* @see ARCHITECTURE.md ss25 -- Checkpoint generation path design.
* @see includes/class-executor.php -- Cron/AJAX entry points.
* @see core/class-pipeline-runner.php -- Orchestration + article worker.
* @see core/class-run-state.php -- Per-run ledger (cost governor).
* @see core/class-pipeline-status.php -- Transient broadcast helpers.
* @see core/class-generation-checkpoint-helpers.php -- finalize/cleanup/fire helpers.
* @see ARCHITECTURE.md ss25 -- Checkpoint generation path design.
*/
class PRAutoBlogger_Generation_Checkpoint_Runner {
@@ -66,7 +67,7 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
}
wp_schedule_single_event( time(), self::ORCHESTRATE_ACTION );
PRAutoBlogger_Pipeline_Status::write_initial();
self::fire_cron_now();
PRAutoBlogger_Generation_Checkpoint_Helpers::fire_cron_now();
}
/**
@@ -157,7 +158,7 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
if ( ! wp_next_scheduled( self::GENERATE_ACTION ) ) {
wp_schedule_single_event( time(), self::GENERATE_ACTION );
}
self::fire_cron_now();
PRAutoBlogger_Generation_Checkpoint_Helpers::fire_cron_now();
} catch ( \Throwable $e ) {
PRAutoBlogger_Logger::instance()->error(
@@ -187,7 +188,7 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
$queue = get_option( self::QUEUE_KEY );
if ( ! is_array( $queue ) || empty( $queue['ideas'] ) ) {
self::finalize(
PRAutoBlogger_Generation_Checkpoint_Helpers::finalize(
array(
'generated' => 0,
'published' => 0,
@@ -205,7 +206,7 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
sprintf( 'Generate tick: run %s is %s -- aborting.', $run_id, $run_status ),
'pipeline'
);
self::cleanup_queue();
PRAutoBlogger_Generation_Checkpoint_Helpers::cleanup_queue();
PRAutoBlogger_Generation_Lock::release();
return;
}
@@ -217,7 +218,7 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
if ( $cost_tracker->is_budget_exceeded() ) {
PRAutoBlogger_Run_State::mark_status( $run_id, 'failed' );
self::cleanup_queue();
PRAutoBlogger_Generation_Checkpoint_Helpers::cleanup_queue();
PRAutoBlogger_Generation_Lock::release();
PRAutoBlogger_Pipeline_Status::write_error(
__( 'Monthly API budget exceeded. Generation halted.', 'prautoblogger' )
@@ -247,10 +248,10 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
if ( ! wp_next_scheduled( self::GENERATE_ACTION ) ) {
wp_schedule_single_event( time(), self::GENERATE_ACTION );
}
self::fire_cron_now();
PRAutoBlogger_Generation_Checkpoint_Helpers::fire_cron_now();
} else {
PRAutoBlogger_Post_Assembler::amortize_research_costs( $run_id );
self::finalize( $queue['results'], $run_id );
PRAutoBlogger_Generation_Checkpoint_Helpers::finalize( $queue['results'], $run_id );
}
} catch ( \Throwable $e ) {
PRAutoBlogger_Logger::instance()->error(
@@ -258,55 +259,9 @@ class PRAutoBlogger_Generation_Checkpoint_Runner {
'pipeline'
);
PRAutoBlogger_Run_State::mark_status( $run_id, 'failed' );
self::cleanup_queue();
PRAutoBlogger_Generation_Checkpoint_Helpers::cleanup_queue();
PRAutoBlogger_Generation_Lock::release();
PRAutoBlogger_Pipeline_Status::write_error( $e->getMessage() );
}
}
// -- Private helpers --------------------------------------------------
/**
* Finalize a completed run: write final transient, mark run done, release lock.
*
* @param array $results Generation counters (generated/published/rejected/cost).
* @param string $run_id Run UUID (read from option if blank).
* @return void
*/
private static function finalize( array $results, string $run_id = '' ): void {
if ( '' === $run_id ) {
$run_id = (string) get_option( self::RUN_ID_KEY, '' );
}
if ( '' !== $run_id ) {
PRAutoBlogger_Run_State::mark_status( $run_id, 'done' );
}
PRAutoBlogger_Pipeline_Status::write_final( $results );
PRAutoBlogger_Pipeline_Status::log_summary( $results );
PRAutoBlogger_Generation_Lock::release();
delete_option( self::RUN_ID_KEY );
}
/** Remove the persistent queue and run-id option on abort paths. */
private static function cleanup_queue(): void {
delete_option( self::QUEUE_KEY );
delete_option( self::RUN_ID_KEY );
}
/**
* Non-blocking loopback to fire cron events immediately.
* Mirrors Pipeline_Runner::schedule_next() exactly.
*
* @return void
*/
private static function fire_cron_now(): void {
spawn_cron();
wp_remote_post(
site_url( 'wp-cron.php?doing_wp_cron=' . sprintf( '%.22F', microtime( true ) ) ),
array(
'timeout' => 0.01,
'blocking' => false,
'sslverify' => false,
)
);
}
}

230
tests/unit/Admin/BoardGenLogQueryTest.php Normal file
View File

@@ -0,0 +1,230 @@
<?php
/**
* Tests for PRAutoBlogger_Board_Gen_Log_Query — binding order and result shape.
*
* The key regression: both get_generating_cards() and get_failed_cards() had
* (a) a missing comma between bound args and (b) reversed argument order — the
* integer limit was passed first, the datetime string second, which bound
* created_at >= <int> and LIMIT <datetime>. These tests assert the correct
* binding order (string first, int second) so any future regression is caught
* before CI.
*
* @package PRAutoBlogger\Tests\Admin
*/
namespace PRAutoBlogger\Tests\Admin;
use PRAutoBlogger\Tests\BaseTestCase;
use Brain\Monkey\Functions;
class BoardGenLogQueryTest extends BaseTestCase {
/** @var \PHPUnit\Framework\MockObject\MockObject Mock $wpdb. */
private $wpdb;
/**
* Captured prepare() calls: each entry is ['sql' => string, 'args' => array].
*
* @var array<int, array{sql: string, args: array}>
*/
private array $prepare_calls = array();
protected function setUp(): void {
parent::setUp();
$this->prepare_calls = array();
$this->wpdb = $this->create_mock_wpdb();
$GLOBALS['wpdb'] = $this->wpdb;
$this->wpdb->prefix = 'wp_';
// Capture prepare() calls so we can assert arg order.
$self = $this;
$this->wpdb->method( 'prepare' )->willReturnCallback(
static function ( string $sql, ...$args ) use ( $self ) {
// Flatten a single-array spread (Brain\Monkey unpacking).
if ( 1 === count( $args ) && is_array( $args[0] ) ) {
$args = $args[0];
}
$self->prepare_calls[] = array( 'sql' => $sql, 'args' => $args );
return 'PREPARED_SQL';
}
);
$this->wpdb->method( 'get_results' )->willReturn( array() );
Functions\when( 'get_transient' )->justReturn( false );
Functions\when( 'admin_url' )->alias(
function ( string $path = '' ) {
return 'https://test.example.com/wp-admin/' . ltrim( $path, '/' );
}
);
Functions\when( 'get_option' )->alias(
function ( string $name, $default = false ) {
if ( 'prautoblogger_board_column_limit' === $name ) {
return 10;
}
return $default;
}
);
if ( ! defined( 'DAY_IN_SECONDS' ) ) {
define( 'DAY_IN_SECONDS', 86400 );
}
if ( ! defined( 'PRAUTOBLOGGER_DEFAULT_BOARD_COLUMN_LIMIT' ) ) {
define( 'PRAUTOBLOGGER_DEFAULT_BOARD_COLUMN_LIMIT', 10 );
}
}
protected function tearDown(): void {
unset( $GLOBALS['wpdb'] );
parent::tearDown();
}
// ── get_generating_cards() binding order ────────────────────────────────
/**
* get_generating_cards() must pass $since (string) as the FIRST bound arg
* and the integer limit as the SECOND — matching placeholder order %s … %d.
*
* This would have caught the reversed-order bug (limit int passed first,
* datetime string second) that silently bound created_at >= <int>.
*/
public function test_generating_cards_prepare_arg_order(): void {
$query = new \PRAutoBlogger_Board_Gen_Log_Query();
$query->get_generating_cards();
// Find the prepare() call containing LIMIT %d (the gen-log secondary query).
$gen_call = null;
foreach ( $this->prepare_calls as $call ) {
if ( str_contains( $call['sql'], 'LIMIT %d' ) && str_contains( $call['sql'], 'post_id IS NULL' ) && ! str_contains( $call['sql'], "response_status = 'error'" ) ) {
$gen_call = $call;
break;
}
}
$this->assertNotNull( $gen_call, 'get_generating_cards() must call $wpdb->prepare() with LIMIT %d' );
$this->assertCount( 2, $gen_call['args'], 'prepare() must receive exactly 2 bound args' );
// First arg must be a string (the $since datetime).
$this->assertIsString( $gen_call['args'][0], 'First arg to prepare() must be the $since string (for %s)' );
// Second arg must be an integer (the column limit).
$this->assertIsInt( $gen_call['args'][1], 'Second arg to prepare() must be the integer limit (for %d)' );
}
/**
* get_generating_cards() must return an empty array (not error) when DB returns nothing.
*/
public function test_generating_cards_returns_empty_array_on_no_results(): void {
$query = new \PRAutoBlogger_Board_Gen_Log_Query();
$cards = $query->get_generating_cards();
$this->assertIsArray( $cards );
}
/**
* When the status transient signals a running job, get_generating_cards()
* returns a card from the transient (no DB call needed for that card).
*/
public function test_generating_cards_returns_transient_card_when_running(): void {
Functions\when( 'get_transient' )->alias(
function ( $key ) {
if ( 'prautoblogger_generation_status' === $key ) {
return array(
'status' => 'running',
'stage' => 'Drafting…',
'started' => time() - 30,
);
}
return false;
}
);
$query = new \PRAutoBlogger_Board_Gen_Log_Query();
$cards = $query->get_generating_cards();
$this->assertNotEmpty( $cards, 'Transient running state must produce at least one card' );
$this->assertSame( '', $cards[0]['run_id'], 'Transient card has empty run_id' );
$this->assertSame( 'logs', $cards[0]['click_action'] );
}
// ── get_failed_cards() binding order ────────────────────────────────────
/**
* get_failed_cards() must pass $since (string) as the FIRST bound arg
* and the integer limit as the SECOND — matching placeholder order %s … %d.
*/
public function test_failed_cards_prepare_arg_order(): void {
$query = new \PRAutoBlogger_Board_Gen_Log_Query();
$query->get_failed_cards();
// Find the prepare() call for the failed query.
$fail_call = null;
foreach ( $this->prepare_calls as $call ) {
if ( str_contains( $call['sql'], "response_status = 'error'" ) ) {
$fail_call = $call;
break;
}
}
$this->assertNotNull( $fail_call, 'get_failed_cards() must call $wpdb->prepare() with response_status filter' );
$this->assertCount( 2, $fail_call['args'], 'prepare() must receive exactly 2 bound args' );
// First arg must be a string (the $since datetime).
$this->assertIsString( $fail_call['args'][0], 'First arg to prepare() must be the $since string (for %s)' );
// Second arg must be an integer (the column limit).
$this->assertIsInt( $fail_call['args'][1], 'Second arg to prepare() must be the integer limit (for %d)' );
}
/**
* get_failed_cards() returns the correct card shape from a seeded result row.
*/
public function test_failed_cards_returns_correct_shape(): void {
$this->wpdb->method( 'get_results' )->willReturn(
array(
array(
'run_id' => 'fail-run-abc',
'started_at' => gmdate( 'Y-m-d H:i:s', time() - 3600 ),
'last_error' => 'Rate limit exceeded',
'last_stage' => 'draft',
'call_count' => '3',
),
)
);
$query = new \PRAutoBlogger_Board_Gen_Log_Query();
$cards = $query->get_failed_cards();
$this->assertCount( 1, $cards );
$this->assertSame( 'fail-run-abc', $cards[0]['run_id'] );
$this->assertStringContainsString( 'Rate limit', $cards[0]['error_message'] );
$this->assertSame( 'logs', $cards[0]['click_action'] );
$this->assertArrayHasKey( 'started_at', $cards[0] );
}
/**
* Column-limit option is honoured: limit 7 → second arg to prepare() is 7.
*/
public function test_column_limit_option_is_respected(): void {
Functions\when( 'get_option' )->alias(
function ( string $name, $default = false ) {
if ( 'prautoblogger_board_column_limit' === $name ) {
return 7;
}
return $default;
}
);
$query = new \PRAutoBlogger_Board_Gen_Log_Query();
$query->get_generating_cards();
$gen_call = null;
foreach ( $this->prepare_calls as $call ) {
if ( str_contains( $call['sql'], 'LIMIT %d' ) && ! str_contains( $call['sql'], "response_status = 'error'" ) ) {
$gen_call = $call;
break;
}
}
$this->assertNotNull( $gen_call );
$this->assertSame( 7, $gen_call['args'][1], 'Limit arg must match the option value' );
}
}