From ae32876b6bd0943be8e2720d855738859a8a960b Mon Sep 17 00:00:00 2001 From: peptiderepo Date: Fri, 12 Jun 2026 11:18:15 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20dossier=20page=20403=20=E2=80=94=20optio?= =?UTF-8?q?ns.php-parent=20pattern=20(v0.19.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dossier page dead on prod (403) since v0.19.2 deploy. Root cause: on_register_menu() registered under 'prautoblogger-settings' parent then unset the slug from $submenu to hide the nav entry. At request time get_admin_page_parent() scanned $submenu, found nothing, fell back to orphan hookname 'admin_page_prautoblogger-dossier'. Render callback was registered under 'prautoblogger_page_prautoblogger-dossier'. Mismatch -> wp_die(403). Same hookname-mismatch class as the board 404 (v0.19.1). Fix (seq 21 §Required fix items 1-6): 1. options.php-parent: add_submenu_page('options.php', ...) -> hookname is 'admin_page_prautoblogger-dossier' at BOTH registration and request time. Hidden by construction; no $submenu mutation needed or permitted. Add PARENT_SLUG = 'options.php' constant. 2. Asset enqueue gate: updated from 'prautoblogger_page_*' to 'admin_page_*' prefix to match options.php-parent hookname. 3. Deep-link URLs (admin.php?page=prautoblogger-dossier&post_id=N): parent-agnostic; unchanged. Asserted in test_url_for_post_contains_slug_and_post_id + test_url_for_post_exact_shape. 4. Regression test upgraded to request-time faithful: test_request_time_hookname_matches_registration_hookname simulates get_admin_page_parent() against post-registration $submenu state. FAILS on v0.19.2 hide-by-unset (4 failures, captured in evidence): - test_request_time_hookname_matches_registration_hookname FAIL - test_dossier_parent_slug_is_options_php FAIL - test_no_submenu_mutation_after_registration FAIL - test_asset_enqueue_gate_uses_admin_page_prefix FAIL PASSES on v0.19.3 options.php-parent (9/9 OK, captured in evidence). test_hide_by_unset_would_fail_request_time_check documents broken pattern inline. 5. ARCHITECTURE.md §22b: hidden-page convention cross-reference + v0.19.3 incident record added. §23: page registration description updated to options.php-parent. File tree entry updated. CONVENTIONS.md §Hidden Admin Pages: full rule + both incident history (board 404 v0.19.1, dossier 403 v0.19.2) + test methodology. 6. Version 0.19.2 -> 0.19.3, CHANGELOG entry. tests/bootstrap.php: PRAUTOBLOGGER_PLUGIN_URL + PRAUTOBLOGGER_VERSION constants added (resolves 'Undefined constant' in enqueue tests). Deletion audit: removed lines are the broken 'prautoblogger-settings' parent string, the 12-line $submenu mutation block, the wrong hook prefix 'prautoblogger_page_*'. No other logic removed. Agent-Session: dossier-403-hotfix-20260612 --- ARCHITECTURE.md | 31 +- CHANGELOG.md | 31 + CONVENTIONS.md | 70 +++ includes/admin/class-dossier-page.php | 67 ++- prautoblogger.php | 4 +- tests/bootstrap.php | 13 + .../Admin/DossierMenuRegistrationTest.php | 544 +++++++++++++++--- 7 files changed, 654 insertions(+), 106 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index dbf480d..323a92f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -145,7 +145,7 @@ prautoblogger/ │ │ ├── class-board-page.php # Kanban board -- primary landing screen (v0.19.0) │ │ ├── class-board-data-provider.php # Board orchestrator: 4-column snapshot + dossier deep-links (v0.19.0, v0.19.2) │ │ ├── class-board-gen-log-query.php # Board gen_log queries: Generating + Failed column raw DB reads (v0.19.0) -│ │ ├── class-dossier-page.php # Article dossier admin page: hidden-submenu, priority 12, enqueue, nonce (v0.19.2) +│ │ ├── class-dossier-page.php # Article dossier admin page: options.php-parent hidden-page, priority 12, enqueue, nonce (v0.19.3) │ │ ├── class-dossier-data-assembler.php # 5-query view-model builder: runs+stages+gen_log+decisions+meta (v0.19.2) │ │ ├── class-dossier-gen-log-query.php # Dossier gen_log queries: per-stage cost receipt + raw trace (v0.19.2) │ │ ├── class-admin-page.php # Main settings page (tabbed SaaS-style UI) @@ -871,6 +871,8 @@ Tests use Brain\Monkey for WordPress function mocking (no database required). Th ### #22b: Kanban board as primary admin landing screen (v0.19.0 / v0.19.1) +**Hidden admin page convention (v0.19.3):** See CONVENTIONS.md §Hidden Admin Pages for the full rule. Summary: hidden admin pages (link-accessed only, not visible in nav) MUST use `options.php` as the `add_submenu_page()` parent. NEVER unset/remove registered pages from `$submenu` after registration -- doing so causes `get_admin_page_parent()` to fail at request time, which makes WP recompute the hookname in the `admin_page_*` orphan namespace, find no registered handler, and call `wp_die(403)`. This is the dossier 403 incident (v0.19.2→v0.19.3). Same hookname-mismatch class as the board 404 (v0.19.0→v0.19.1); the only difference is that the board bug was a priority-ordering issue at registration time while the dossier bug was a post-registration $submenu mutation. + The primary admin screen is now a kanban board (Direction C "Editorial Record") with four columns: Generating | In Review | Published | Failed. A separate `class-board-page.php` registers the `Board` submenu and a `$submenu` reorder makes it the first (primary) entry. @@ -904,14 +906,25 @@ a single post: generation run, stage I/O, cost receipts, raw-trace toggles, edit decisions, and image A/B pairs. It is link-accessed only (hidden submenu) — no navigation entry appears in the WP sidebar. -**Page registration** (`class-dossier-page.php`): registered as a hidden submenu under -`prautoblogger-settings` at `admin_menu` priority **12**, one step after the board (priority -11), which is itself after the parent menu (priority 10). The page is immediately removed -from `$submenu` so it does not appear in the WP admin sidebar, but remains reachable by -direct URL. URL shape: `admin.php?page=prautoblogger-dossier&post_id=`. Secured by -`manage_options` capability + nonce (`prautoblogger_dossier`). Static `url_for_post(int -$post_id): string` provides the canonical deep-link. Regression test: -`tests/unit/Admin/DossierMenuRegistrationTest.php`. +**Page registration** (`class-dossier-page.php`): uses the canonical hidden-admin-page +pattern -- registered under parent **`options.php`** at `admin_menu` priority **12**. The +hookname is `admin_page_prautoblogger-dossier` at BOTH registration time and request time, +so there is no hookname mismatch and no need to manipulate `$submenu`. The page does not +appear in the WP admin sidebar (options.php-parent pages are hidden by construction). +URL shape: `admin.php?page=prautoblogger-dossier&post_id=` (unchanged from v0.19.2 -- +deep-link URLs are parent-agnostic). Secured by `manage_options` capability. Static +`url_for_post(int $post_id): string` provides the canonical deep-link. + +v0.19.2 used `prautoblogger-settings` as parent with post-registration `unset($submenu[...])` to +hide the page. This caused `get_admin_page_parent()` to fail at request time -> orphan +`admin_page_*` hookname -> no handler registered -> `wp_die(403)`. Fixed in v0.19.3 by switching +to `options.php` parent and removing all `$submenu` mutation. See CONVENTIONS.md §Hidden Admin +Pages for the full rule and ARCHITECTURE.md §22b for both incident records. + +Regression test: `tests/unit/Admin/DossierMenuRegistrationTest.php` -- the +`test_request_time_hookname_matches_registration_hookname` test replicates the request-time +resolution algorithm (simulate `get_admin_page_parent()`) so it FAILS on hide-by-unset and +PASSES on options.php-parent. **5-query data assembly** (`class-dossier-data-assembler.php`): diff --git a/CHANGELOG.md b/CHANGELOG.md index 772e01c..2728809 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,37 @@ All notable changes to PRAutoBlogger will be documented in this file. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project uses [Semantic Versioning](https://semver.org/). +## [0.19.3] - 2026-06-12 + +### Fixed +- **Dossier page 403 (P0 hotfix).** `PRAutoBlogger_Dossier_Page::on_register_menu` registered the + dossier as a hidden submenu under `prautoblogger-settings` then called `unset($submenu[...])` to + hide it from the nav. This is the **hide-by-unset anti-pattern**: at registration time WP records + the render callback under hookname `prautoblogger_page_prautoblogger-dossier`; at request time + `get_admin_page_parent()` scans `$submenu`, finds the slug absent (it was unset), returns false, + and WP recomputes the hookname under the orphan `admin_page_*` namespace -- no handler registered + there -- `wp_die(403)`. Same hookname-mismatch class as the board 404 (v0.19.1), different vector. + **Fix:** register under parent `options.php` (canonical hidden-page pattern). Hookname is + `admin_page_prautoblogger-dossier` at both registration and request time. No `$submenu` mutation + required or permitted. Asset enqueue gate updated to match (`admin_page_*` prefix). + Deep-link URLs (`admin.php?page=prautoblogger-dossier&post_id=N`) are parent-agnostic and + unchanged. Board cards and the post metabox link continue to work without modification. + +### Changed +- **Regression test upgraded to request-time faithful** (`DossierMenuRegistrationTest`). + New `test_request_time_hookname_matches_registration_hookname` simulates + `get_admin_page_parent()` against the post-registration `$submenu` state and asserts it + equals the registration-time hookname. FAILS on hide-by-unset (4 failures on v0.19.2 code, + captured in test evidence). PASSES on options.php-parent. New + `test_hide_by_unset_would_fail_request_time_check` documents the broken pattern inline. +- **CONVENTIONS.md §Hidden Admin Pages** added: options.php-parent is the only permitted + pattern for hidden admin pages; `$submenu` mutation after registration is explicitly + prohibited; full incident history (board 404 + dossier 403) and test methodology documented. +- **ARCHITECTURE.md §22b** updated with hidden-page convention cross-reference. + **§23** updated to reflect options.php-parent registration and the v0.19.2→v0.19.3 fix. +- **tests/bootstrap.php**: added `PRAUTOBLOGGER_PLUGIN_URL` and `PRAUTOBLOGGER_VERSION` + constants so admin enqueue tests resolve without "Undefined constant" errors. + ## [0.19.2] - 2026-06-12 ### Added diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 4382ecf..f1ae518 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -502,3 +502,73 @@ public function my_operation( $input ) { ``` That's it. No config, no feature-flag checks, no try/catch needed. + +--- + +## Hidden Admin Pages (v0.19.3 convention, codified after two incidents) + +### Rule: use options.php as parent; NEVER unset/remove registered pages from $submenu + +When a WordPress admin page must be accessible by URL but hidden from the nav menu, use the +**options.php-parent pattern** exclusively: + +```php +add_submenu_page( + 'options.php', // parent: built-in WP page; pages hidden by construction + __( 'My Page', 'prautoblogger' ), // page title + __( 'My Page', 'prautoblogger' ), // menu title (never shown; options.php is hidden) + 'manage_options', // capability + 'my-page-slug', // page slug + array( $this, 'render_page' ), // callback +); +// No $submenu manipulation. options.php-parent pages are hidden from nav by construction. +``` + +The resulting hookname is `admin_page_my-page-slug` at BOTH registration time and request time. + +### Why the hide-by-unset pattern causes 403 (the incident class) + +WordPress computes the page hookname at two distinct moments: + +**Registration time** (inside `add_submenu_page`): +WP resolves the parent slug against `$admin_page_hooks`. If found, hookname = `{parent_key}_page_{slug}`. WP registers the render callback and `$_registered_pages` entry under that name. + +**Request time** (inside `wp-admin/admin.php`): +WP calls `get_admin_page_parent()` which scans `$submenu` for the page slug. If found, it derives the hookname the same way as (1). If **not found** (e.g., because the entry was unset), it falls back to `admin_page_{slug}`. + +When these two hooknames diverge, WP dispatches the request to a hookname with no registered callback → `wp_die(403)`. + +**Why options.php avoids this:** `options.php` is a built-in page that WordPress always keeps intact in `$admin_page_hooks` and `$submenu`. For its children, `get_admin_page_parent()` returns `options.php` consistently at both moments, so both produce `admin_page_{slug}`. No mutation required. + +### Incident history + +| Version | Symptom | Root cause | Fix | +|---------|---------|------------|-----| +| v0.19.1 | Board page 404 | Board submenu registered at same priority as parent (both 10) — add_submenu_page fired before add_menu_page → wrong hookname at registration time | Board moved to priority 11, after parent at 10 | +| v0.19.3 | Dossier page 403 | Dossier registered under `prautoblogger-settings` parent (priority 12, after parent ✓) but post-registration `unset($submenu[...])` removed the slug → get_admin_page_parent() returned false at request time → orphan `admin_page_*` hookname → no handler → 403 | Switched to `options.php` parent; removed all $submenu mutation | + +Both incidents are in the same **hookname-mismatch class**: registration-time hookname ≠ request-time hookname. + +### Test pattern + +The `DossierMenuRegistrationTest::test_request_time_hookname_matches_registration_hookname` test +replicates the request-time resolution algorithm (scan `$submenu` for the slug; if found derive +hookname; if absent return `admin_page_*`) and asserts it equals the registration-time hookname. +This test FAILS on hide-by-unset implementations and passes on the options.php-parent pattern. +Apply the same test strategy to any new hidden admin page. + +### Asset enqueue gate + +For `options.php`-parent pages, the hook suffix passed to `admin_enqueue_scripts` is +`admin_page_{slug}`. For plugin-parent pages it would be `{parent_key}_page_{slug}`. Always +match the asset enqueue gate to the actual hookname for the chosen parent pattern: + +```php +// options.php parent → 'admin_page_{slug}' +public function on_enqueue_assets( string $hook_suffix ): void { + if ( $hook_suffix !== 'admin_page_' . self::PAGE_SLUG ) { + return; + } + // ... enqueue +} +``` diff --git a/includes/admin/class-dossier-page.php b/includes/admin/class-dossier-page.php index f2ae1e2..9d7bc8c 100644 --- a/includes/admin/class-dossier-page.php +++ b/includes/admin/class-dossier-page.php @@ -6,10 +6,25 @@ declare(strict_types=1); * * Registers and renders the Article Dossier admin page (M2). * - * The dossier is link-accessed -- it is NOT a visible submenu item. It registers - * as a hidden submenu under 'prautoblogger-settings' at admin_menu priority 12 - * (after board at 11) so WordPress resolves the hookname correctly. Board cards and - * post list columns deep-link here via ?page=prautoblogger-dossier&post_id=X. + * The dossier is link-accessed -- it is NOT a visible submenu item. It uses + * the canonical hidden-admin-page pattern: register under parent 'options.php' + * so WordPress computes the hookname as `admin_page_prautoblogger-dossier` at + * BOTH registration time AND request time. The page is hidden from nav by + * construction (options.php is a built-in WP page with no visible submenu block) + * so no $submenu mutation is needed or permitted. + * + * Why NOT the hide-by-unset pattern (the v0.19.2 bug): + * add_submenu_page('prautoblogger-settings', ...) → hookname 'prautoblogger_page_prautoblogger-dossier' + * post-registration unset($submenu['prautoblogger-settings'][$idx]) + * → at request time get_admin_page_parent() no longer finds the slug in $submenu + * → WP recomputes hookname in the 'admin_page_*' orphan namespace + * → no callback registered there → wp_die(403). + * See ARCHITECTURE.md §22c + CONVENTIONS.md §Hidden Admin Pages for the full incident + * history (board 404 v0.19.1, dossier 403 v0.19.2). + * + * Board cards and post-metabox deep-link here via + * admin.php?page=prautoblogger-dossier&post_id=X. Deep-link URLs are parent-agnostic + * (they reference only the page slug) so they are unchanged by this fix. * * Design contract: Proposal C "Editorial Record" -- warm editorial dossier, * verdict pills, receipt-style cost sidebar, per-stage I/O with raw-trace toggle. @@ -19,46 +34,44 @@ declare(strict_types=1); * * @see admin/class-dossier-data-assembler.php -- Builds the view model. * @see templates/admin/dossier-page.php -- HTML template. - * @see ARCHITECTURE.md -- §Dossier (M2). + * @see ARCHITECTURE.md -- §22c (hidden admin page convention). + * @see CONVENTIONS.md -- §Hidden Admin Pages. */ class PRAutoBlogger_Dossier_Page { /** Admin page slug. */ public const PAGE_SLUG = 'prautoblogger-dossier'; + /** + * Parent slug used at registration. + * 'options.php' is the canonical WP hidden-page parent: the hookname resolves + * to `admin_page_{slug}` at both registration time and request time, so there is + * no hookname mismatch and no need to manipulate $submenu. + */ + public const PARENT_SLUG = 'options.php'; + /** Nonce action for any future stateful actions. */ public const NONCE_ACTION = 'prautoblogger_dossier'; /** - * Register the dossier as a hidden submenu page. + * Register the dossier as a hidden admin page. * - * Hidden = not visible in the menu, but reachable by URL. - * Priority 12 ensures add_menu_page() (priority 10) has populated - * $admin_page_hooks before this add_submenu_page() call. See ARCHITECTURE.md §Board. + * Uses the canonical options.php-parent pattern so the hookname is stable at + * both registration time and request time. No $submenu manipulation required + * or permitted (see CONVENTIONS.md §Hidden Admin Pages). * * @return void */ public function on_register_menu(): void { add_submenu_page( - 'prautoblogger-settings', + self::PARENT_SLUG, __( 'Article Dossier', 'prautoblogger' ), __( 'Dossier', 'prautoblogger' ), 'manage_options', self::PAGE_SLUG, array( $this, 'render_page' ) ); - - // Remove from visible submenu so it doesn't clutter the nav. - // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- intentional hidden-submenu pattern. - global $submenu; - if ( isset( $submenu['prautoblogger-settings'] ) ) { - foreach ( $submenu['prautoblogger-settings'] as $idx => $item ) { - if ( isset( $item[2] ) && self::PAGE_SLUG === $item[2] ) { - unset( $submenu['prautoblogger-settings'][ $idx ] ); - break; - } - } - } + // No $submenu mutation. options.php-parent pages are hidden by construction. } /** @@ -71,7 +84,7 @@ class PRAutoBlogger_Dossier_Page { return; } // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only dossier; no state changes. - $post_id = isset( $_GET['post_id'] ) ? absint( $_GET['post_id'] ) : 0; + $post_id = isset( $_GET['post_id'] ) ? absint( $_GET['post_id'] ) : 0; $assembler = new PRAutoBlogger_Dossier_Data_Assembler(); $dossier = $assembler->assemble( $post_id ); include PRAUTOBLOGGER_PLUGIN_DIR . 'templates/admin/dossier-page.php'; @@ -80,11 +93,14 @@ class PRAutoBlogger_Dossier_Page { /** * Enqueue dossier-specific CSS + JS. * + * The hook suffix for an options.php-parent page is `admin_page_{slug}`. + * * @param string $hook_suffix Current admin page hook suffix. * @return void */ public function on_enqueue_assets( string $hook_suffix ): void { - $dossier_hook = 'prautoblogger_page_' . self::PAGE_SLUG; + // options.php-parent pages resolve to 'admin_page_{slug}' (not 'prautoblogger_page_*'). + $dossier_hook = 'admin_page_' . self::PAGE_SLUG; if ( $hook_suffix !== $dossier_hook ) { return; } @@ -108,6 +124,9 @@ class PRAutoBlogger_Dossier_Page { /** * Build the URL to the dossier for a given post. * + * Deep-link URLs use admin.php?page= which is parent-agnostic. + * This method is unchanged from v0.19.2 -- only the registration parent changed. + * * @param int $post_id WordPress post ID. * @return string Admin URL to the dossier page. */ diff --git a/prautoblogger.php b/prautoblogger.php index 912d2e0..5e26004 100644 --- a/prautoblogger.php +++ b/prautoblogger.php @@ -9,7 +9,7 @@ * Plugin Name: PRAutoBlogger * Plugin URI: https://peptiderepo.com/prautoblogger * Description: Monitors social media for trending topics, generates SEO-friendly blog posts using AI, and publishes them on a daily schedule with full cost tracking and self-improvement metrics. - * Version: 0.19.2 + * Version: 0.19.3 * Requires at least: 6.0 * Requires PHP: 7.4 * Author: PeptideRepo @@ -35,7 +35,7 @@ if ( ! defined( 'ABSPATH' ) ) { | Defined here so every file in the plugin can reference paths, versions, | and limits without magic strings. */ -define( 'PRAUTOBLOGGER_VERSION', '0.19.2' ); +define( 'PRAUTOBLOGGER_VERSION', '0.19.3' ); define( 'PRAUTOBLOGGER_DB_VERSION', '1.1.0' ); define( 'PRAUTOBLOGGER_PLUGIN_FILE', __FILE__ ); define( 'PRAUTOBLOGGER_PLUGIN_DIR', plugin_dir_path( __FILE__ ) ); diff --git a/tests/bootstrap.php b/tests/bootstrap.php index c4d1b67..c12c42f 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -205,3 +205,16 @@ if ( ! class_exists( 'WP_Post' ) ) { } } } + +// Plugin URL and version constants used by admin enqueue methods. +// Defined here so all tests that load admin classes can stub WP enqueue calls +// without hitting "Undefined constant" errors. +if ( ! defined( 'PRAUTOBLOGGER_PLUGIN_URL' ) ) { + define( 'PRAUTOBLOGGER_PLUGIN_URL', 'https://test.example.com/wp-content/plugins/prautoblogger/' ); +} +if ( ! defined( 'PRAUTOBLOGGER_VERSION' ) ) { + define( 'PRAUTOBLOGGER_VERSION', '0.19.3-test' ); +} +if ( ! defined( 'PRAUTOBLOGGER_PLUGIN_DIR' ) ) { + define( 'PRAUTOBLOGGER_PLUGIN_DIR', dirname( __DIR__ ) . '/' ); +} diff --git a/tests/unit/Admin/DossierMenuRegistrationTest.php b/tests/unit/Admin/DossierMenuRegistrationTest.php index fceb9c8..ef62cba 100644 --- a/tests/unit/Admin/DossierMenuRegistrationTest.php +++ b/tests/unit/Admin/DossierMenuRegistrationTest.php @@ -2,13 +2,64 @@ /** * Tests for Article Dossier admin page menu registration. * - * Regression guard for the menu-ordering rule (v0.19.1 lesson): - * any submenu registered BEFORE add_menu_page fires will resolve under the - * fallback hookname `admin_page_*` instead of `prautoblogger_page_*`, causing - * a wp_die 404 at request time. + * ## Why the v0.19.2 hide-by-unset pattern caused a 403 * - * The dossier page uses a hidden-submenu pattern (removed from $submenu after - * registration) at priority 12 -- after board (11) and parent (10). + * WordPress computes a page's hook suffix at TWO separate moments: + * + * 1. Registration time (inside add_submenu_page): + * WP resolves the parent slug against $admin_page_hooks. If found, the hookname + * becomes "{parent_key}_page_{slug}" (e.g. "prautoblogger_page_prautoblogger-dossier"). + * WP registers the render callback + $_registered_pages entry under that name. + * + * 2. Request time (inside wp-admin/admin.php): + * WP calls get_admin_page_parent() which scans $submenu for the slug. If it finds + * a match, it derives the hookname the same way as (1). If not found (because it + * was unset post-registration), it falls back to "admin_page_{slug}". + * + * When these two hooknames diverge, the render callback lives under name-A but WP + * dispatches to name-B -- no handler found -- wp_die(403). + * + * The hide-by-unset pattern creates exactly this divergence: + * Registration: parent found in $admin_page_hooks → "prautoblogger_page_prautoblogger-dossier" + * Post-unset: slug absent from $submenu → "admin_page_prautoblogger-dossier" at request time + * Mismatch → 403. + * + * ## The fix: options.php-parent pattern + * + * options.php is a built-in WordPress admin page in $admin_page_hooks. Its key is + * "settings" so a child page resolves to "settings_page_{slug}" at registration time -- + * BUT WordPress special-cases options.php children: get_admin_page_parent() returns + * "options.php" directly (the page is always in the admin page hooks map and never + * removed from $submenu by WP itself), so the hookname is "admin_page_{slug}" consistently + * at both registration and request time. No $submenu mutation needed or permitted. + * + * ## Test strategy + * + * Brain\Monkey stubs WordPress functions. We cannot run a real WP request cycle in unit + * tests, but we can replicate the request-time resolution algorithm precisely: + * + * simulate_request_time_hookname($submenu_state, $slug): + * scan $submenu for $slug + * if found: return "{parent_key}_page_{slug}" (same logic as registration-time) + * if not found: return "admin_page_{slug}" (WP orphan fallback) + * + * This function is the discriminator. Against the v0.19.2 hide-by-unset implementation: + * - post-registration $submenu does NOT contain the slug (it was unset) + * - simulate_request_time_hookname returns "admin_page_prautoblogger-dossier" + * - but registration returned "prautoblogger_page_prautoblogger-dossier" + * - MISMATCH → test_request_time_hookname_matches_registration_hookname FAILS ✓ + * + * Against the v0.19.3 options.php-parent fix: + * - $submenu is not mutated; the slug appears under options.php + * - simulate_request_time_hookname sees options.php as parent + * - registration also used options.php → both produce "admin_page_prautoblogger-dossier" + * - MATCH → test passes ✓ + * + * Additional tests: + * - Parent slug must be PARENT_SLUG constant ('options.php') + * - No $submenu mutation after registration + * - Deep-link URLs are parent-agnostic (admin.php?page=...&post_id=N unchanged) + * - Asset enqueue gate uses 'admin_page_' prefix (not 'prautoblogger_page_') * * @package PRAutoBlogger\Tests\Admin */ @@ -24,53 +75,445 @@ class DossierMenuRegistrationTest extends BaseTestCase { /** @var array Call sequence spy. */ private array $call_sequence = []; + /** + * Captured parent slug from the most recent add_submenu_page call. + * + * @var string|null + */ + private ?string $captured_parent_slug = null; + + /** + * Captured menu slug from the most recent add_submenu_page call. + * + * @var string|null + */ + private ?string $captured_menu_slug = null; + + /** + * Hookname returned by the most recent add_submenu_page call. + * + * In real WordPress this is `{parent_key}_page_{slug}` when parent is known, + * or `admin_page_{slug}` when parent is not in $admin_page_hooks. For options.php + * children WordPress always emits `admin_page_{slug}`. + * + * @var string|null + */ + private ?string $registration_hookname = null; + protected function setUp(): void { parent::setUp(); - $this->call_sequence = []; + $this->call_sequence = []; + $this->captured_parent_slug = null; + $this->captured_menu_slug = null; + $this->registration_hookname = null; + // Spy stub: tracks call order, captures slugs, returns a hookname that + // reflects how WordPress actually resolves it for known parents. Functions\when( 'add_menu_page' )->alias( function () { $this->call_sequence[] = 'add_menu_page'; return 'prautoblogger-settings'; } ); + + // Simulate WordPress hookname computation: + // - options.php parent → 'admin_page_{slug}' (WP built-in; options.php children always resolve here) + // - known plugin parent → '{parent_key}_page_{slug}' + // - unknown parent → 'admin_page_{slug}' (orphan fallback) Functions\when( 'add_submenu_page' )->alias( - function ( string $parent_slug, string $page_title, string $menu_title, string $capability, string $menu_slug ) { - $this->call_sequence[] = 'add_submenu_page:' . $menu_slug; - return 'prautoblogger_page_' . $menu_slug; + function ( + string $parent_slug, + string $page_title, + string $menu_title, + string $capability, + string $menu_slug, + $callback = '' + ) { + $this->call_sequence[] = 'add_submenu_page:' . $parent_slug . ':' . $menu_slug; + $this->captured_parent_slug = $parent_slug; + $this->captured_menu_slug = $menu_slug; + + // Replicate WordPress hookname logic: + // options.php-parent pages always resolve to admin_page_{slug}. + // Plugin-parent pages (like prautoblogger-settings) resolve to + // {parent_key}_page_{slug} at registration time. + if ( 'options.php' === $parent_slug ) { + $hookname = 'admin_page_' . $menu_slug; + } else { + // Derive parent key: strip .php extension if present, then + // replace - with _ for the namespace prefix. + $parent_key = str_replace( '.php', '', $parent_slug ); + $hookname = $parent_key . '_page_' . $menu_slug; + } + + $this->registration_hookname = $hookname; + + // Populate $submenu to simulate what WordPress does at registration time. + // This is the state that get_admin_page_parent() scans at request time. + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- test simulation. + global $submenu; + $submenu[ $parent_slug ] = $submenu[ $parent_slug ] ?? array(); + $submenu[ $parent_slug ][] = array( $page_title, $capability, $menu_slug, $menu_title ); + + return $hookname; } ); Functions\when( 'current_user_can' )->justReturn( true ); Functions\when( 'get_option' )->justReturn( false ); - Functions\when( 'admin_url' )->alias( function ( $path = '' ) { - return 'https://example.com/wp-admin/' . ltrim( $path, '/' ); - } ); + Functions\when( 'admin_url' )->alias( + function ( string $path = '' ) { + return 'https://example.com/wp-admin/' . ltrim( $path, '/' ); + } + ); + Functions\when( '__' )->returnArg( 1 ); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- test isolation. $GLOBALS['submenu'] = array(); } protected function tearDown(): void { - $this->call_sequence = []; + $this->call_sequence = []; + $this->captured_parent_slug = null; + $this->captured_menu_slug = null; + $this->registration_hookname = null; unset( $GLOBALS['submenu'] ); parent::tearDown(); } + // ========================================================================= + // § REQUEST-TIME HOOKNAME FAITHFULNESS + // The primary regression guard for the 403 incident class. + // ========================================================================= + /** - * Dossier on_register_menu must be hooked at priority > board (11). - * Mirrors the bindings in register_admin_hooks(): - * parent at 10, board at 11, dossier at 12. + * Simulate the request-time hookname resolution algorithm. * - * FAILS if dossier drops to priority ≤ 11 (same as or before board). - * FAILS if dossier drops to priority ≤ 10 (same as or before parent). + * Replicates get_admin_page_parent() + the hookname derivation in + * wp-admin/admin.php::wp_admin_canonical_url() / do_action("admin_{$page}"): + * + * scan $submenu for the slug + * if found at parent P → return hookname derived from P + * if not found → return "admin_page_{slug}" + * + * This is the discriminator function: it WILL return a different hookname than + * add_submenu_page returned when the slug has been unset from $submenu. + * + * @param array>> $submenu_state $submenu after all registration + mutations. + * @param string $slug Page slug to resolve. + * @return string Request-time hookname. */ - public function test_dossier_hook_registered_after_board(): void { + private function simulate_request_time_hookname( array $submenu_state, string $slug ): string { + foreach ( $submenu_state as $parent => $items ) { + foreach ( $items as $item ) { + if ( isset( $item[2] ) && $slug === $item[2] ) { + // Found the slug; derive hookname the same way as registration. + if ( 'options.php' === $parent ) { + return 'admin_page_' . $slug; + } + $parent_key = str_replace( '.php', '', $parent ); + return $parent_key . '_page_' . $slug; + } + } + } + // Slug absent from $submenu -- WP falls back to the orphan namespace. + return 'admin_page_' . $slug; + } + + /** + * REQUEST-TIME FAITHFULNESS: registration hookname must equal request-time hookname. + * + * This test directly discriminates the two patterns: + * + * HIDE-BY-UNSET (v0.19.2, BROKEN): + * - add_submenu_page('prautoblogger-settings', ...) → 'prautoblogger_page_prautoblogger-dossier' + * - unset($submenu['prautoblogger-settings'][$idx]) + * - post-registration $submenu does NOT contain the slug + * - simulate_request_time_hookname → 'admin_page_prautoblogger-dossier' + * - 'prautoblogger_page_...' ≠ 'admin_page_...' → FAIL ✓ (test correctly detects broken state) + * + * OPTIONS.PHP-PARENT (v0.19.3, FIXED): + * - add_submenu_page('options.php', ...) → 'admin_page_prautoblogger-dossier' + * - no $submenu mutation + * - simulate_request_time_hookname → 'admin_page_prautoblogger-dossier' + * - MATCH → PASS ✓ + * + * This test ALSO fails if any future change reintroduces $submenu mutation, + * or switches back to a plugin-owned parent without preserving the submenu entry. + */ + public function test_request_time_hookname_matches_registration_hookname(): void { + $dossier_page = new \PRAutoBlogger_Dossier_Page(); + $dossier_page->on_register_menu(); + + $this->assertNotNull( + $this->registration_hookname, + 'add_submenu_page must be called during on_register_menu()' + ); + + // Capture $submenu state AFTER all mutations that on_register_menu() performs. + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- test read. + global $submenu; + $submenu_after = $submenu; + + $request_time_hookname = $this->simulate_request_time_hookname( + $submenu_after, + \PRAutoBlogger_Dossier_Page::PAGE_SLUG + ); + + $this->assertSame( + $this->registration_hookname, + $request_time_hookname, + sprintf( + 'Hookname mismatch: registration="%s", request-time="%s". ' + . 'This is the 403 failure class. ' + . 'The hide-by-unset pattern unsets the slug from $submenu after registration, ' + . 'so get_admin_page_parent() fails at request time, WP recomputes under ' + . 'the orphan "admin_page_*" namespace, finds no registered handler, ' + . 'and calls wp_die(403). Fix: use options.php as parent -- hookname is ' + . '"admin_page_{slug}" at both registration and request time, no mutation needed.', + $this->registration_hookname, + $request_time_hookname + ) + ); + } + + /** + * REGRESSION PROOF: documents that v0.19.2 hide-by-unset would have failed the + * request-time test above. + * + * Directly simulates the broken implementation, applies simulate_request_time_hookname, + * and asserts the MISMATCH -- proving the discriminator is effective. + * + * This test documents the failure state and serves as evidence that the test methodology + * is correct. It passes by verifying the broken pattern produces the wrong hookname. + */ + public function test_hide_by_unset_would_fail_request_time_check(): void { + // --- SIMULATE V0.19.2 hide-by-unset IMPLEMENTATION --- + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- test simulation. + global $submenu; + + // Step 1: Registration (add_submenu_page with plugin parent). + // Real add_submenu_page stub returns 'prautoblogger_page_prautoblogger-dossier' + // when parent is 'prautoblogger-settings'. Populate $submenu as WP does. + $broken_parent_slug = 'prautoblogger-settings'; + $slug = 'prautoblogger-dossier'; + + $submenu[ $broken_parent_slug ] = $submenu[ $broken_parent_slug ] ?? array(); + $submenu[ $broken_parent_slug ][] = array( 'Article Dossier', 'manage_options', $slug, 'Dossier' ); + + // The hookname WP computed at registration time with plugin-owned parent: + $broken_registration_hookname = 'prautoblogger_page_' . $slug; + + // Step 2: The unset mutation (the bug). + // This is what v0.19.2's on_register_menu() did after add_submenu_page. + foreach ( $submenu[ $broken_parent_slug ] as $idx => $item ) { + if ( isset( $item[2] ) && $slug === $item[2] ) { + unset( $submenu[ $broken_parent_slug ][ $idx ] ); + break; + } + } + + // Step 3: Request time -- slug is now ABSENT from $submenu. + $request_time_hookname = $this->simulate_request_time_hookname( $submenu, $slug ); + + // THE MISMATCH that caused the 403: + $this->assertNotSame( + $broken_registration_hookname, + $request_time_hookname, + 'Test methodology error: hide-by-unset must produce a hookname mismatch. ' + . 'If this assertion fails the discriminator is broken.' + ); + + // Specifically: request-time falls back to admin_page_* orphan namespace. + $this->assertSame( + 'admin_page_' . $slug, + $request_time_hookname, + 'Request-time fallback must be admin_page_{slug} when slug absent from $submenu.' + ); + $this->assertSame( + 'prautoblogger_page_' . $slug, + $broken_registration_hookname, + 'Registration-time hookname with prautoblogger-settings parent must be prautoblogger_page_{slug}.' + ); + } + + // ========================================================================= + // § STRUCTURAL INVARIANTS + // ========================================================================= + + /** + * Parent slug must be PARENT_SLUG constant ('options.php'). + * + * FAILS on v0.19.2 (parent was 'prautoblogger-settings'). + * PASSES after v0.19.3 fix (parent is 'options.php'). + */ + public function test_dossier_parent_slug_is_options_php(): void { + $dossier_page = new \PRAutoBlogger_Dossier_Page(); + $dossier_page->on_register_menu(); + + $this->assertSame( + 'options.php', + $this->captured_parent_slug, + 'Dossier must use options.php as parent (canonical hidden-page pattern). ' + . 'Never use a plugin-owned parent with post-registration $submenu unset -- ' + . 'that is the v0.19.2 403 bug class. See CONVENTIONS.md §Hidden Admin Pages.' + ); + + $this->assertSame( + \PRAutoBlogger_Dossier_Page::PARENT_SLUG, + $this->captured_parent_slug, + 'Captured parent slug must match PARENT_SLUG constant.' + ); + } + + /** + * No $submenu mutation after registration. + * + * options.php-parent pages are hidden by construction; the slug must remain + * in $submenu so get_admin_page_parent() can resolve the parent at request time. + * + * FAILS on v0.19.2 (slug was unset from $submenu after registration). + * PASSES after v0.19.3 fix (no mutation). + */ + public function test_no_submenu_mutation_after_registration(): void { + $dossier_page = new \PRAutoBlogger_Dossier_Page(); + $dossier_page->on_register_menu(); + + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- test read. + global $submenu; + + // The dossier slug must still be present somewhere in $submenu. + $slug_found = false; + foreach ( $submenu as $parent => $items ) { + foreach ( $items as $item ) { + if ( isset( $item[2] ) && \PRAutoBlogger_Dossier_Page::PAGE_SLUG === $item[2] ) { + $slug_found = true; + break 2; + } + } + } + + $this->assertTrue( + $slug_found, + 'Dossier slug must remain in $submenu after on_register_menu() completes. ' + . 'Unsetting it causes get_admin_page_parent() to fail at request time and ' + . 'produces the admin_page_* orphan hookname -- the v0.19.2 403 bug class. ' + . 'options.php-parent pages are hidden by construction; no mutation needed.' + ); + } + + /** + * Page slug must be PAGE_SLUG constant ('prautoblogger-dossier'). + */ + public function test_dossier_uses_correct_slug(): void { + $dossier_page = new \PRAutoBlogger_Dossier_Page(); + $dossier_page->on_register_menu(); + + $this->assertSame( + \PRAutoBlogger_Dossier_Page::PAGE_SLUG, + $this->captured_menu_slug, + 'Dossier must register under PAGE_SLUG.' + ); + } + + /** + * Asset enqueue gate must use 'admin_page_' prefix (not 'prautoblogger_page_'). + * + * options.php-parent pages always resolve to 'admin_page_{slug}'. + * FAILS on v0.19.2 (gate checked 'prautoblogger_page_prautoblogger-dossier'). + * PASSES after v0.19.3 fix (gate checks 'admin_page_prautoblogger-dossier'). + */ + public function test_asset_enqueue_gate_uses_admin_page_prefix(): void { + Functions\when( 'wp_enqueue_style' )->justReturn( null ); + Functions\when( 'wp_enqueue_script' )->justReturn( null ); + + $dossier_page = new \PRAutoBlogger_Dossier_Page(); + + $expected_hook = 'admin_page_' . \PRAutoBlogger_Dossier_Page::PAGE_SLUG; + + // Wrong hook -- assets must NOT enqueue. + $enqueued = false; + Functions\when( 'wp_enqueue_style' )->alias( function () use ( &$enqueued ) { + $enqueued = true; + } ); + + $dossier_page->on_enqueue_assets( 'wrong_hook_prautoblogger_page_' . \PRAutoBlogger_Dossier_Page::PAGE_SLUG ); + $this->assertFalse( + $enqueued, + 'Assets must NOT enqueue when hook_suffix is the old prautoblogger_page_* hook.' + ); + + // Correct hook -- assets must enqueue. + $dossier_page->on_enqueue_assets( $expected_hook ); + $this->assertTrue( + $enqueued, + sprintf( + 'Assets must enqueue when hook_suffix is "%s" (admin_page_* from options.php parent).', + $expected_hook + ) + ); + } + + // ========================================================================= + // § DEEP-LINK URL INVARIANCE + // ========================================================================= + + /** + * url_for_post() must produce admin.php?page=prautoblogger-dossier&post_id=N. + * + * Deep-link URLs are parent-agnostic -- they reference only the page slug. + * The fix changes only the registration parent; the URL shape is unchanged. + */ + public function test_url_for_post_contains_slug_and_post_id(): void { + $url = \PRAutoBlogger_Dossier_Page::url_for_post( 42 ); + + $this->assertStringContainsString( + \PRAutoBlogger_Dossier_Page::PAGE_SLUG, + $url, + 'url_for_post() must contain the dossier page slug.' + ); + $this->assertStringContainsString( + '42', + $url, + 'url_for_post() must contain the post_id.' + ); + $this->assertStringContainsString( + 'admin.php', + $url, + 'url_for_post() must use admin.php (parent-agnostic deep-link).' + ); + } + + /** + * url_for_post() URL shape is unchanged from v0.19.2. + * + * Confirms the exact URL format that board cards and the metabox link depend on. + */ + public function test_url_for_post_exact_shape(): void { + $url = \PRAutoBlogger_Dossier_Page::url_for_post( 930 ); + + // Must contain admin.php?page=prautoblogger-dossier&post_id=930 + // (or equivalent query string order -- admin_url may vary order). + $this->assertStringContainsString( 'page=prautoblogger-dossier', $url ); + $this->assertStringContainsString( 'post_id=930', $url ); + } + + // ========================================================================= + // § HOOK REGISTRATION ORDERING (priority guard) + // ========================================================================= + + /** + * Dossier on_register_menu must remain at priority 12 (after board 11, parent 10). + * + * The options.php-parent fix removes the dependency on ordering relative to the + * plugin's own parent menu (since we no longer add a submenu under it), but we + * keep the priority assertion to prevent accidental priority regression. + */ + public function test_dossier_hook_registered_at_priority_12(): void { $admin_page = new \PRAutoBlogger_Admin_Page(); $board_page = new \PRAutoBlogger_Board_Page(); $dossier_page = new \PRAutoBlogger_Dossier_Page(); - // Mirror class-prautoblogger.php register_admin_hooks() priorities. add_action( 'admin_menu', array( $admin_page, 'on_register_menu' ), 10 ); add_action( 'admin_menu', array( $board_page, 'on_register_menu' ), 11 ); add_action( 'admin_menu', array( $dossier_page, 'on_register_menu' ), 12 ); @@ -94,56 +537,15 @@ class DossierMenuRegistrationTest extends BaseTestCase { if ( false !== strpos( $cb, 'PRAutoBlogger_Dossier_Page' ) ) { $dossier_priority = $priority; } } - $this->assertGreaterThan( $parent_priority, $board_priority, - 'Board priority must exceed parent priority.' ); - $this->assertGreaterThan( $board_priority, $dossier_priority, - 'Dossier priority must exceed board priority to ensure hookname resolves correctly.' ); - } - - /** - * Dossier submenu slug must be 'prautoblogger-dossier'. - */ - public function test_dossier_uses_correct_slug(): void { - $dossier_page = new \PRAutoBlogger_Dossier_Page(); - $dossier_page->on_register_menu(); - - $found_slug = null; - foreach ( $this->call_sequence as $entry ) { - if ( str_starts_with( $entry, 'add_submenu_page:' ) ) { - $found_slug = substr( $entry, strlen( 'add_submenu_page:' ) ); - break; - } - } - - $this->assertSame( \PRAutoBlogger_Dossier_Page::PAGE_SLUG, $found_slug, - 'Dossier must register under PAGE_SLUG.' ); - } - - /** - * Dossier submenu parent must be 'prautoblogger-settings'. - */ - public function test_dossier_parent_slug_is_settings(): void { - $captured_parent = null; - Functions\when( 'add_submenu_page' )->alias( - function ( string $parent_slug ) use ( &$captured_parent ) { - $captured_parent = $parent_slug; - return null; - } + $this->assertGreaterThan( + $parent_priority, + $board_priority, + 'Board priority must exceed parent priority.' + ); + $this->assertGreaterThan( + $board_priority, + $dossier_priority, + 'Dossier priority must exceed board priority.' ); - - $dossier_page = new \PRAutoBlogger_Dossier_Page(); - $dossier_page->on_register_menu(); - - $this->assertSame( 'prautoblogger-settings', $captured_parent, - 'Dossier must use prautoblogger-settings as parent.' ); - } - - /** - * url_for_post() returns a URL containing the dossier slug and post_id. - */ - public function test_url_for_post_contains_slug_and_post_id(): void { - $url = \PRAutoBlogger_Dossier_Page::url_for_post( 42 ); - $this->assertStringContainsString( \PRAutoBlogger_Dossier_Page::PAGE_SLUG, $url ); - $this->assertStringContainsString( '42', $url ); } }