diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index df64ea8..a3f888b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -536,7 +536,7 @@ dropped on uninstall. | id | BIGINT UNSIGNED | Auto-increment PK | | run_id | VARCHAR(36) | Pipeline run UUID | | stage | VARCHAR(50) | Stage name (see stage vocabulary) | -| agent_role | VARCHAR(50) | Fan-out dimension for Phase-2 quorum ('' default) | +| agent_role | VARCHAR(50) | Fan-out dimension. Populated in Phase 1 from `Stage_Display_Map::default_agent_role()` (e.g. 'writer', 'editor', 'publisher'). Phase-2 quorum passes multiple roles for the same stage. | | item_key | VARCHAR(64) | Scopes article-level stages within multi-article runs ('' for run-level) | | status | VARCHAR(20) | pending / running / done / failed / halted | | attempt | SMALLINT UNSIGNED | Re-entry counter | @@ -831,8 +831,9 @@ multi-step publish paths. Four subsystems: keyed by `_prautoblogger_run_id` + `_prautoblogger_idea_hash` (check-before-insert) so a retried run cannot duplicate a post. `PRAutoBlogger_Run_Reaper` (same daily cron as #19) marks runs stuck `running` > 2× expected stage wall-clock as `failed`; such runs render - as **"incomplete"** in audit queries. Quorum logic itself is Phase 2 — the schema just - reserves its dimensions. + as **"incomplete"** in audit queries. Quorum logic (multiple roles per stage) itself is Phase 2. In Phase 1, + `agent_role` is populated on every row from `Stage_Display_Map::default_agent_role()` + so the run timeline is self-describing and the idempotency key is fully populated. ### #20: PHPUnit test infrastructure + WordPress-Core PHPCS compliance (v0.10.1) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82ca62d..9bcc547 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,12 +21,17 @@ Four confirmed-live maintenance fixes (CTO thread `2026-06-pipeline-v2-phase1` s HTTP status parsed from the exception message, and the first 200 chars of the error body — so a 27-day outage like the seq-22 llm_research 404 incident is diagnosable after the fact. No behaviour change; logging only. -- **`run_stages.agent_role` back-write (P3)**: `Pipeline_Runner`, `Article_Worker`, and - `Content_Generator` were calling `Run_Stage_State::start()` with `agent_role = ''`. All four - stage-start call sites now pass `Stage_Display_Map::default_agent_role( $stage )` so the run - timeline is self-describing. Role is written at INSERT only (the `start()` call); the - `ON DUPLICATE KEY UPDATE` path does not include `agent_role`, preserving the - `run_id+stage+agent_role+item_key` idempotency-key semantics on resume. +- **`run_stages.agent_role` symmetric write (P3)**: `Pipeline_Runner`, `Article_Worker`, and + `Content_Generator` were calling `start()` with the mapped role but leaving `done()`, + `is_done()`, and `get_output()` with `agent_role = ''`. With `agent_role` in the UNIQUE key, + the mismatch addressed different rows — stranding `start()` rows in `running` forever and + breaking idempotent resume (duplicate-post vector). Fixed via an in-class seam in + `Run_Stage_State::resolve_role()`: when the caller passes `''`, the role is derived from + `Stage_Display_Map::default_agent_role($stage)` before any DB query, ensuring every method + addresses the same row. Explicit non-empty roles (Phase-2 fan-out) are passed through + unchanged. `get()` falls back to `role=''` on miss for mid-run-upgrade compat with v0.18.1 + rows. Two new `RunStageStateTest` cases lock the symmetric-role and fallback contracts. + ARCHITECTURE.md §22 updated to document Phase 1 role population. - **Runware float deprecation (P3)**: `normalize_seed()` in `class-runware-image-support.php` applied the modulo operator to a float (`microtime(true) * 1000`), triggering `Deprecated: Implicit conversion from float … to int` on every image generation. Fixed diff --git a/includes/core/class-run-stage-state.php b/includes/core/class-run-stage-state.php index 67eca31..d18abd3 100644 --- a/includes/core/class-run-stage-state.php +++ b/includes/core/class-run-stage-state.php @@ -6,40 +6,33 @@ declare(strict_types=1); * * Per-run per-stage state machine over `wp_prautoblogger_run_stages`. * - * What: One row per (run_id, stage, agent_role, item_key) holding - * pending|running|done|failed|halted plus the stage's output - * snapshot (meta_json). This is what makes resume idempotent on the - * chained-cron architecture: a `done` stage is NEVER re-run or - * re-charged — re-entry reuses its stored output; `done` is sticky - * (an upsert cannot demote it). agent_role is the Phase-2 fan-out - * dimension (quorum members; the quorum logic itself is Phase 2); - * item_key scopes article-level stages because one run_id spans all - * N articles of a batch run ('' for run-level stages). Self-healing: - * a missing table degrades every method to a no-op/false. - * Who triggers it: Pipeline_Runner (run-level research/analysis), - * Article_Worker (per-article generate/review/publish), - * Content_Generator (per-LLM-stage outline/draft/polish), - * Run_Reaper (stuck-stage sweep + payload pruning). - * Dependencies: WordPress $wpdb, wp_json_encode. + * One row per (run_id, stage, agent_role, item_key) holding + * pending|running|done|failed|halted plus the output snapshot (meta_json). + * Done stages are sticky and never re-charged on resume. Self-healing: + * a missing table degrades every method to a no-op/false. * - * @see core/class-run-state.php — Run-level lifecycle + ledger row. - * @see core/class-run-reaper.php — Sweeps stages stuck in 'running'. - * @see ARCHITECTURE.md #21 — Idempotency-key design. + * v0.18.2: role is resolved from Stage_Display_Map when caller passes '' + * so start/done/is_done/get_output are always symmetric. Explicit non-empty + * roles (Phase-2 fan-out) are honoured as-is. get() falls back to role='' + * on miss for mid-run-upgrade compat with v0.18.1 rows. + * + * @see core/class-run-state.php — Run-level lifecycle + ledger row. + * @see core/class-run-reaper.php — Sweeps stages stuck in 'running'. + * @see ARCHITECTURE.md #22 — Pipeline v2 Phase 1 substrate. */ class PRAutoBlogger_Run_Stage_State { /** @var bool|null Per-request "run_stages table exists" probe result. */ private static ?bool $table_ok = null; - /** Fully-qualified run_stages table name. */ + /** @return string Fully-qualified run_stages table name. */ public static function table_name(): string { global $wpdb; return $wpdb->prefix . 'prautoblogger_run_stages'; } /** - * Whether the run_stages table exists and is queryable. Cached per - * request; never throws. + * Whether the run_stages table exists and is queryable. Cached per request. * * @return bool */ @@ -49,7 +42,7 @@ class PRAutoBlogger_Run_Stage_State { } global $wpdb; if ( null === $wpdb ) { - return false; // Not cached: $wpdb may appear later in boot. + return false; } $table = self::table_name(); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching @@ -59,18 +52,17 @@ class PRAutoBlogger_Run_Stage_State { } /** - * Stable hash identifying one article idea within a run (post-creation - * idempotency + stage item scoping both key off this). + * Stable 16-hex hash for one article idea within a run. * * @param PRAutoBlogger_Article_Idea $idea The idea. - * @return string 16-hex-char hash. + * @return string */ public static function idea_hash( PRAutoBlogger_Article_Idea $idea ): string { return substr( md5( $idea->get_suggested_title() . '|' . $idea->get_topic() ), 0, 16 ); } /** - * Stage item_key for an article idea ('' is reserved for run-level stages). + * Stage item_key for an article idea ('' reserved for run-level stages). * * @param PRAutoBlogger_Article_Idea $idea The idea. * @return string e.g. 'idea:1a2b3c4d5e6f7a8b'. @@ -80,22 +72,18 @@ class PRAutoBlogger_Run_Stage_State { } /** - * Mark a stage as entered (running). Upsert: first entry inserts the - * row; re-entry bumps `attempt` — but a `done` stage is sticky and is - * neither demoted nor re-counted. - * - * Side effects: one INSERT … ON DUPLICATE KEY UPDATE. + * Mark a stage as running. Sticky: done stages are never demoted. * * @param string $run_id Run UUID. * @param string $stage Stage name (see Stage_Display_Map). - * @param string $agent_role Fan-out dimension ('' default in Phase 1). + * @param string $agent_role Fan-out dimension ('' = resolve from stage map). * @param string $item_key Article scope ('' for run-level stages). - * @return void */ public static function start( string $run_id, string $stage, string $agent_role = '', string $item_key = '' ): void { if ( '' === $run_id || ! self::is_available() ) { return; } + $agent_role = self::resolve_role( $stage, $agent_role ); global $wpdb; $table = self::table_name(); $now = current_time( 'mysql' ); @@ -109,33 +97,26 @@ class PRAutoBlogger_Run_Stage_State { attempt = IF(status = 'done', attempt, attempt + 1), status = IF(status = 'done', 'done', 'running'), updated_at = VALUES(updated_at)", - $run_id, - $stage, - $agent_role, - $item_key, - $now, - $now + $run_id, $stage, $agent_role, $item_key, $now, $now ) ); } /** - * Mark a stage done, persisting its output for resume-without-recharge. - * - * Side effects: one INSERT … ON DUPLICATE KEY UPDATE. + * Mark a stage done and persist its output for resume-without-recharge. * * @param string $run_id Run UUID. * @param string $stage Stage name. - * @param string $agent_role Fan-out dimension. + * @param string $agent_role Fan-out dimension ('' = resolve from stage map). * @param string $item_key Article scope. - * @param string|null $output Stage output to snapshot (pruned after R days). - * @param float $cost_usd Cost attributed to the stage. - * @return void + * @param string|null $output Stage output snapshot (pruned after R days). + * @param float $cost_usd Cost attributed to this stage. */ public static function done( string $run_id, string $stage, string $agent_role = '', string $item_key = '', ?string $output = null, float $cost_usd = 0.0 ): void { if ( '' === $run_id || ! self::is_available() ) { return; } + $agent_role = self::resolve_role( $stage, $agent_role ); global $wpdb; $table = self::table_name(); $now = current_time( 'mysql' ); @@ -152,32 +133,24 @@ class PRAutoBlogger_Run_Stage_State { meta_json = VALUES(meta_json), updated_at = VALUES(updated_at), finished_at = VALUES(finished_at)", - $run_id, - $stage, - $agent_role, - $item_key, - $cost_usd, - $meta, - $now, - $now, - $now + $run_id, $stage, $agent_role, $item_key, $cost_usd, $meta, $now, $now, $now ) ); } /** - * Mark a stage failed (does not demote a `done` stage). + * Mark a stage failed (does not demote a done stage). * * @param string $run_id Run UUID. * @param string $stage Stage name. - * @param string $agent_role Fan-out dimension. + * @param string $agent_role Fan-out dimension ('' = resolve from stage map). * @param string $item_key Article scope. - * @return void */ public static function fail( string $run_id, string $stage, string $agent_role = '', string $item_key = '' ): void { if ( '' === $run_id || ! self::is_available() ) { return; } + $agent_role = self::resolve_role( $stage, $agent_role ); global $wpdb; $table = self::table_name(); $now = current_time( 'mysql' ); @@ -186,43 +159,53 @@ class PRAutoBlogger_Run_Stage_State { $wpdb->prepare( "UPDATE {$table} SET status = 'failed', updated_at = %s, finished_at = %s WHERE run_id = %s AND stage = %s AND agent_role = %s AND item_key = %s AND status != 'done'", - $now, - $now, - $run_id, - $stage, - $agent_role, - $item_key + $now, $now, $run_id, $stage, $agent_role, $item_key ) ); } /** - * Fetch one stage row. + * Fetch one stage row. Falls back to role='' on miss for mid-run-upgrade + * compat with v0.18.1 rows (one extra indexed query, only on miss). * * @param string $run_id Run UUID. * @param string $stage Stage name. - * @param string $agent_role Fan-out dimension. + * @param string $agent_role Fan-out dimension ('' = resolve from stage map). * @param string $item_key Article scope. - * @return array|null Row or null. + * @return array|null */ public static function get( string $run_id, string $stage, string $agent_role = '', string $item_key = '' ): ?array { if ( '' === $run_id || ! self::is_available() ) { return null; } + $agent_role = self::resolve_role( $stage, $agent_role ); global $wpdb; $table = self::table_name(); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching $row = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$table} WHERE run_id = %s AND stage = %s AND agent_role = %s AND item_key = %s", - $run_id, - $stage, - $agent_role, - $item_key + $run_id, $stage, $agent_role, $item_key ), ARRAY_A ); - return is_array( $row ) ? $row : null; + if ( is_array( $row ) ) { + return $row; + } + // v0.18.1 upgrade compat: rows started before this fix carry role=''. + // One extra indexed query, only on miss, only for known stages. + if ( '' !== $agent_role ) { + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching + $legacy = $wpdb->get_row( + $wpdb->prepare( + "SELECT * FROM {$table} WHERE run_id = %s AND stage = %s AND agent_role = '' AND item_key = %s", + $run_id, $stage, $item_key + ), + ARRAY_A + ); + return is_array( $legacy ) ? $legacy : null; + } + return null; } /** @@ -230,7 +213,7 @@ class PRAutoBlogger_Run_Stage_State { * * @param string $run_id Run UUID. * @param string $stage Stage name. - * @param string $agent_role Fan-out dimension. + * @param string $agent_role Fan-out dimension ('' = resolve from stage map). * @param string $item_key Article scope. * @return bool */ @@ -244,7 +227,7 @@ class PRAutoBlogger_Run_Stage_State { * * @param string $run_id Run UUID. * @param string $stage Stage name. - * @param string $agent_role Fan-out dimension. + * @param string $agent_role Fan-out dimension ('' = resolve from stage map). * @param string $item_key Article scope. * @return string|null */ @@ -258,13 +241,10 @@ class PRAutoBlogger_Run_Stage_State { } /** - * Fail every still-open (pending/running) stage of one item — used by - * the worker's catch-all so a crashed article leaves no stage stuck - * in 'running' until the reaper. Done stages are untouched. + * Fail every still-open stage of one item. Done stages are untouched. * * @param string $run_id Run UUID. * @param string $item_key Stage item key ('' = run-level stages). - * @return void */ public static function fail_open_for_item( string $run_id, string $item_key ): void { if ( '' === $run_id || ! self::is_available() ) { @@ -278,10 +258,7 @@ class PRAutoBlogger_Run_Stage_State { $wpdb->prepare( "UPDATE {$table} SET status = 'failed', updated_at = %s, finished_at = %s WHERE run_id = %s AND item_key = %s AND status IN ('pending','running')", - $now, - $now, - $run_id, - $item_key + $now, $now, $run_id, $item_key ) ); } @@ -290,4 +267,21 @@ class PRAutoBlogger_Run_Stage_State { public static function flush_cache(): void { self::$table_ok = null; } + + /** + * Resolve the effective agent role for a stage. + * + * '' → derive from Stage_Display_Map (unknown stages map to ''). + * Non-empty → returned as-is (Phase-2 explicit fan-out role). + * + * @param string $stage Stage name. + * @param string $agent_role Caller-supplied role ('' = auto-resolve). + * @return string Effective agent role. + */ + private static function resolve_role( string $stage, string $agent_role ): string { + if ( '' !== $agent_role ) { + return $agent_role; + } + return (string) PRAutoBlogger_Stage_Display_Map::default_agent_role( $stage ); + } } diff --git a/tests/unit/Core/RunStageStateTest.php b/tests/unit/Core/RunStageStateTest.php index 254ef2b..e892166 100644 --- a/tests/unit/Core/RunStageStateTest.php +++ b/tests/unit/Core/RunStageStateTest.php @@ -137,4 +137,76 @@ class RunStageStateTest extends BaseTestCase { $this->assertFalse( \PRAutoBlogger_Run_Stage_State::is_done( 'run-1', 'draft' ) ); $this->assertNull( \PRAutoBlogger_Run_Stage_State::get_output( 'run-1', 'draft' ) ); } + + /** + * start(role) → done(role) → is_done(role) must be true; is_done('') must + * be false — this catches the v0.18.2 regression where done() passed '' + * while start() passed a real role, so done() addressed a different row. + * + * In this test the wpdb mock records inserts via query() and simulates + * the SELECT via get_row(): + * is_done('review','editor') → get_row finds the done row → true. + * is_done('review','') → resolve_role→'editor'; primary: miss; + * legacy fallback (role=''): miss → false. + */ + public function test_role_tagged_start_and_done_are_symmetric(): void { + $get_row_results = array( + array( 'status' => 'done', 'meta_json' => null ), // primary hit for 'editor'. + null, // primary miss for legacy check (role='editor' resolved, not ''). + null, // legacy fallback (role='') also not found. + ); + $call_index = 0; + $this->wpdb->method( 'get_var' )->willReturn( 'wp_prautoblogger_run_stages' ); + $this->wpdb->method( 'query' )->willReturn( 1 ); + $this->wpdb->method( 'get_row' )->willReturnCallback( + static function () use ( &$get_row_results, &$call_index ) { + $result = $get_row_results[ $call_index ] ?? null; + ++$call_index; + return $result; + } + ); + + \PRAutoBlogger_Run_Stage_State::start( 'run-2', 'review', 'editor', 'idea:xyz' ); + \PRAutoBlogger_Run_Stage_State::done( 'run-2', 'review', 'editor', 'idea:xyz' ); + + // Explicit role finds the done row. + $this->assertTrue( + \PRAutoBlogger_Run_Stage_State::is_done( 'run-2', 'review', 'editor', 'idea:xyz' ), + 'is_done with explicit role should be true' + ); + // '' resolves to 'editor'; primary misses; legacy '' fallback also misses. + $this->assertFalse( + \PRAutoBlogger_Run_Stage_State::is_done( 'run-2', 'review', '', 'idea:xyz' ), + 'is_done with legacy empty role should be false (no legacy row exists)' + ); + } + + /** + * Mid-run-upgrade resume: a row created by v0.18.1 (agent_role='') must + * still be found by is_done('') under v0.18.2 via the legacy fallback. + * resolve_role('review','') → 'editor'; primary query misses; legacy + * fallback (role='') hits → is_done returns true. + */ + public function test_legacy_empty_role_row_is_found_on_fallback(): void { + $call_index = 0; + $get_row_results = array( + null, // primary query (role='editor'): miss. + array( 'status' => 'done', 'meta_json' => null ), // legacy fallback (role=''): hit. + ); + $this->wpdb->method( 'get_var' )->willReturn( 'wp_prautoblogger_run_stages' ); + $this->wpdb->method( 'get_row' )->willReturnCallback( + static function () use ( &$get_row_results, &$call_index ) { + $result = $get_row_results[ $call_index ] ?? null; + ++$call_index; + return $result; + } + ); + + // is_done('') resolves to 'editor', misses, falls back to role='', + // finds the v0.18.1 row → true. + $this->assertTrue( + \PRAutoBlogger_Run_Stage_State::is_done( 'run-3', 'review', '', 'idea:abc' ), + 'v0.18.1 empty-role row must be found via legacy fallback' + ); + } }