fix(config): store one key per normalized slug, closing a fifth sanitize hole

.planning/STATE.md

@@ -62,9 +62,10 @@ settled. The gate earned its place; the hold was correct.
 
 ### Caveats that survive into the milestone record
 
-- The #128 fixes are themselves unreviewed — the ultrareview ran against
-  `1a32f08`. Accepted deliberately: the collapse removed a class of seam rather
-  than adding another guard.
+- ~~The #128 fixes are themselves unreviewed — the ultrareview ran against
+  `1a32f08`.~~ **CLOSED 2026-08-10.** Reviewed via #149 (review-only PR, base
+  pinned at `1a32f08`, so the diff was everything no adversarial pass had seen).
+  **It found a fifth hole in the same function** — see the entry below.
 - No human screen-reader pass on the person picker. axe is clean across empty and
   populated states, which is not the same thing. **Consolidated 2026-08-10 into
   `todos/pending/2026-08-10-person-picker-screen-reader-pass.md`** — see the
@@ -199,10 +200,10 @@ not automatic and required a manual `workflow_dispatch`. Plan the step in.
   (`todos/pending/2026-08-02-cloned-role-hiding-profiles.md`)
 - Phase 22 (Playground demo) remains open. ~~Phase 25 (toolbar polish)~~ —
   ✅ COMPLETE 2026-08-09, SHIPPED in v1.5.1
-- The #128 fixes shipped unreviewed (the ultrareview ran against the prior commit).
-  **Still true as of v1.5.1** — the mechanism is intact in `main`, touched since
-  only by #138's 20-line entity-collision fix. Closing it needs `/code-review ultra`
-  over `1a32f08..707d9b6`, which is user-triggered.
+- ~~The #128 fixes shipped unreviewed (the ultrareview ran against the prior
+  commit).~~ **CLOSED 2026-08-10** via #149 — which found a fifth hole in the same
+  sanitize path (two normalizing-equivalent payload keys). Fixed; see the
+  reassessment near the end of this file.
 - No human screen-reader pass on the person picker → consolidated into
   `todos/pending/2026-08-10-person-picker-screen-reader-pass.md`
 - ~~21-05 Task 5 (human browser verification) was never performed~~ — **STRUCK
@@ -525,13 +526,26 @@ re-opening either question:**
   **withdrawn**: `160px` is just hardcoded in three places.
 
 **Reviewed 2026-08-10 — three carried items are now TWO, and both are actionable:**
-1. **The #128 fixes shipped unreviewed.** Verified still live: `707d9b6` changed
-   192 lines of `class-config.php` plus the logic modules, and that mechanism is
-   intact in `main` (touched since only by #138's 20 lines). It is the same
-   sanitize path in which the ultrareview found four consecutive holes, and it is
-   the one round no adversarial pass has ever seen. **Close with `/code-review
-   ultra` over `1a32f08..707d9b6`** — user-triggered and billed, so it needs Dan.
-   Highest value of the three, and the cheapest.
+1. ~~**The #128 fixes shipped unreviewed.**~~ **CLOSED 2026-08-10 — and it was
+   worth doing.** The ultrareview ran against #149 (`1a32f08..main`) and returned
+   **one finding, confirmed real: a FIFTH hole in `Config::sanitize()`.**
+
+   Two payload keys that normalize alike (`upload.php` + `upload.php?ver=1`) both
+   got stored: the first claimed and `unset()` the protected map entry, so the
+   second found no match and wrote itself as a fresh entry. Two stored keys
+   normalizing to one is exactly what Replay's Axis-1 guard resolves to "apply
+   nothing" — so a saver WITHOUT `list_users` could neutralise an administrator's
+   per-user rule with one POST, leaving it visibly present in storage and applying
+   nowhere.
+
+   **The pattern held for a fifth time**: the seam was the `unset()` after the
+   first match, and the code comment directly above it claimed the merge existed
+   to prevent precisely this ambiguity — true only of the FIRST equivalent
+   spelling. Fixed by deduping at the point of write, unconditionally, so the
+   collision is unconstructible rather than blocked on one path. Reproduced with a
+   failing test first; three replay-side tests had to be re-seeded because
+   sanitize can no longer create the state they assert on (they still can, and
+   must, via legacy configs and slug drift — all three re-verified falsifiable).
 2. **The screen-reader pass**, now a real todo rather than prose:
    `todos/pending/2026-08-10-person-picker-screen-reader-pass.md`. It absorbed
    item 3 and slightly widened — Phase 25's M2 change made a previously-skipped

.planning/compat/PRIOR-ART-admin-menu-editor.md

@@ -99,6 +99,44 @@ solved — or paid for the sharp corners of — most of Maestro's hard problems.
   drag-between-levels (reparenting), and per-role deny are all either Pro-gated or fragile in AME.
   Maestro's own backlog (`config-presets-export-import`, reparenting v2) lines up to undercut these
   for free.
+- **D4 — Zero front-end footprint: one sparse, NON-AUTOLOADED row (added 2026-08-11).** The
+  storage-row consequence of V2, and a **key differentiator / USP** — the one Maestro claim that is
+  measurable by a third party (`wp option list --autoload=on`) without installing either plugin.
+  - **AME's row is autoloaded**, so its bytes ride in the `alloptions` bundle on every request that
+    boots WP — front end and logged-out included, where the data is never read. Its size grows with
+    the *whole* admin menu (every plugin installed adds to the stored full tree) and with every
+    role/user rule, not with what the user edited.
+  - **Where that cost actually lands** (state it precisely; the loose version invites a correction):
+    `wp_load_alloptions()` fetches every autoloaded row in one query and holds them in memory for
+    the request. The option's own `maybe_unserialize()` runs only when something calls
+    `get_option()` for it — so the front-end tax is the **bundle**, not that call: DB→PHP transfer,
+    memory, and — with a persistent object cache, where `alloptions` is a single cache object — a
+    per-request fetch and unserialize of the *whole* bundle, which is precisely where a fat row
+    hurts most. A full-page cache spares requests that never boot WP; a logged-in or otherwise
+    uncacheable front-end request pays in full.
+  - **This is acknowledged upstream, not inferred.** A wordpress.org thread reports `ws_menu_editor`
+    as the site's largest autoloaded row; Elsts confirms the autoload behavior and declines to flip
+    it because non-autoloaded "would mean an additional SQL query on every admin page", pointing to
+    compression instead ([support thread](https://wordpress.org/support/topic/wp_option-table-ws_menu_editor-auotload-yes/)).
+    Three separate size mitigations over eight years ([Pro changelog](https://adminmenueditor.com/documentation/changelog/)):
+    **2.5** (2017) compress-menu-config, **2.11** (2020) + zlib ("greatly decreases the amount of
+    data stored… but increases decompression overhead"), **2.27** (2025) "Optimize menu
+    configuration size" for the `ws_menu_editor_pro` entry. Three rounds ⇒ structural, not a bug.
+  - **Maestro's position:** one `maestro_config` row written `update_option( …, false )`
+    (`includes/class-config.php`), never created until first save; sparse delta so size tracks
+    *edits*, not installed-plugin count; hard 1 MB aggregate cap; every hook admin-gated (`Replay`
+    on `admin_menu`, `Admin_Bar::node()` bails on `! is_admin()`) so a front-end request reads
+    nothing at all. Measured: **0 extra front-end queries**; ~0.1 ms added per admin page at a
+    realistic 5–15 KB config, ~1 ms at the 1 MB ceiling
+    (`docs/performance/config-size-and-page-load.md`).
+  - **The trade, stated honestly:** non-autoloaded costs **1 extra admin-page query** — exactly
+    Elsts' objection. Maestro takes it because that query is admin-only, memoized once per request
+    by `Config::get()`, and zero with a persistent object cache, while public traffic — the bulk of
+    a live site's requests — pays nothing. AME's arrangement inverts the tax.
+  - **Evidence caveat:** the autoload flag comes from the author's own reply and the changelog, not
+    from the 1.15.1 source read (that spike recorded the storage *format*, not the autoload
+    argument). Confirm the `update_option` call when the free zip is next opened — the feature
+    sweep todo now carries that as a check.
 
 ---
 

.planning/todos/pending/2026-08-10-ame-feature-surface-research.md

@@ -86,6 +86,20 @@ The sweep is not done until each of these is enumerated and every item lands in
 4. **The settings/UI surface** — what AME exposes where. Relevant because
    `DECISION-settings-surface.md` locked Maestro's answer and AME is the
    counter-example (a dedicated editor screen).
+5. **The database-footprint features** — AME's Settings tab ships "Compress menu
+   configuration data" (2.5, zlib in 2.11) and "Optimize menu configuration size"
+   (2.27). These are *user-facing toggles*, so they are feature rows despite the
+   storage carve-out in Out of scope below, and they are the rows where Maestro's
+   **D4 differentiator** (single sparse non-autoloaded row — see
+   [`PRIOR-ART-admin-menu-editor.md`](../../compat/PRIOR-ART-admin-menu-editor.md)
+   § Differentiate) states itself: status `deliberately-not`, because a config that
+   is sparse and out of `alloptions` never needs a compression toggle. Expected to
+   be among the strongest `have`/`deliberately-not` rows in the matrix for
+   competitive-positioning purposes.
+   **Also settle the open evidence question while the zip is open:** confirm
+   whether `ws_menu_editor` is written with an autoload argument at all. D4
+   currently rests on the author's forum reply plus the changelog, not on a source
+   read — grep the `update_option` call and record the answer as `source-read`.
 
 **Completion check** (mirroring SCHEMA.md's): every file in AME's `modules/`
 accounted for in at least one row; every bullet on the Pro pricing page accounted
@@ -117,6 +131,12 @@ The architecture comparison — apply model, hook ordering, storage format, menu
 identity, submenu targeting — all answered 2026-08-01. If a feature row needs an
 architectural answer, note the dependency in its Note cell and move on.
 
+**One carve-out: the options-table footprint is in scope** (Coverage item 5). The
+storage *format* is architecture and stays out, but its consequences — an
+autoloaded row that every front-end request pays for, and the two Settings-tab
+toggles AME ships to manage it — are user-visible and are Maestro's D4 selling
+point. They earn rows.
+
 Other prior art is also out: "Wider Admin Menu" and "Hide Admin Menu" belong to
 `SPEC.md` items 9 and 10. AME-only keeps this bounded.
 

includes/class-config.php

@@ -133,8 +133,11 @@ class Config {
 	 * under this — five 128 KB icons plus titles/roles is ~0.65 MB. The ceiling
 	 * exists only to refuse the pathological multi-MB payload (e.g. 200 max
 	 * items each carrying a 128 KB icon ≈ 25 MB) that would bloat the option
-	 * and every autoloaded read of it. An over-ceiling save is rejected whole
-	 * rather than truncated — a partial config is worse than the prior one.
+	 * and every admin-request read of it. (The option is stored non-autoloaded —
+	 * see save() — so it is never part of the `alloptions` bundle; the cost the
+	 * ceiling bounds is the per-admin-request get_option + unserialize, not a
+	 * front-end tax.) An over-ceiling save is rejected whole rather than
+	 * truncated — a partial config is worse than the prior one.
 	 *
 	 * @var int
 	 */
@@ -405,6 +408,11 @@ public function sanitize( array $raw ) {
 		// than an administrator's rules.
 		$item_budget = self::MAX_ITEMS - count( $protected );
 
+		// Normalized keys already written to $out['items'] this pass. See the
+		// dedupe guard in the loop — this is what makes the sanitized output
+		// collision-free BY CONSTRUCTION rather than collision-detected later.
+		$emitted_norm = array();
+
 		if ( ! empty( $raw['items'] ) && is_array( $raw['items'] ) ) {
 			foreach ( $raw['items'] as $slug => $item ) {
 				// Qualified `parent>child` submenu keys: clean each half
@@ -442,6 +450,31 @@ public function sanitize( array $raw ) {
 					$slug = $this->clean_slug( $slug );
 				}
 
+				// ONE stored key per NORMALIZED identity.
+				//
+				// Two payload keys that normalize alike (`upload.php` and
+				// `upload.php?ver=1`) would otherwise BOTH be stored, and the
+				// Axis-1 guard in Replay::normalized_items() resolves that
+				// ambiguity to "apply nothing" — dropping every override on that
+				// item, including a protected per-user rule merged in below. The
+				// config still looks healthy; the rule simply stops applying.
+				//
+				// That was reachable by a saver WITHOUT `list_users`: the protected
+				// map is claimed and unset on the first matching spelling, so a
+				// second equivalent spelling in the same payload found no match,
+				// stored itself as a fresh entry, and neutralised an administrator's
+				// rule with one POST. Deduping at the point of write closes it for
+				// every caller at once, rather than special-casing that path — the
+				// seam only existed because the collision was constructible at all.
+				//
+				// First spelling in incoming object order wins, matching the
+				// MAX_ITEMS break below. Recorded only when an entry is actually
+				// stored, so an empty first entry does not shadow a real later one.
+				$nk = Slug::normalize_qualified( (string) $slug, $base );
+				if ( '' !== $nk && isset( $emitted_norm[ $nk ] ) ) {
+					continue;
+				}
+
 				$entry = array();
 
 				if ( isset( $item['title'] ) && '' !== trim( (string) $item['title'] ) ) {
@@ -501,7 +534,14 @@ public function sanitize( array $raw ) {
 					// item are ambiguous, and the Axis-1 guard resolves ambiguity
 					// to "apply nothing", which would leave the rule stored but
 					// inert while the config still looked healthy.
-					$nk = Slug::normalize_qualified( (string) $slug, $base );
+					//
+					// That reasoning only ever covered the FIRST equivalent spelling:
+					// this branch claims and unsets the protected entry, so a second
+					// equivalent key in the same payload found no match and stored
+					// itself anyway — recreating the exact ambiguity the comment above
+					// says it prevents. The dedupe guard earlier in the loop is what
+					// actually closes it; $nk is computed once up there and reused
+					// here, since a second local copy is how the two would drift.
 					if ( '' !== $nk && isset( $protected[ $nk ] ) ) {
 						$entry = array_merge( $entry, $protected[ $nk ]['axes'] );
 						unset( $protected[ $nk ] ); // Claimed; do not re-add below.
@@ -527,6 +567,9 @@ public function sanitize( array $raw ) {
 
 				if ( $entry ) {
 					$out['items'][ $slug ] = $entry;
+					if ( '' !== $nk ) {
+						$emitted_norm[ $nk ] = true;
+					}
 					if ( count( $out['items'] ) >= $item_budget ) {
 						break; // Deterministic: first N slugs in incoming object order win.
 					}