From 1a0ea022fd3cc90cc6823df8b72c44a91df83ed8 Mon Sep 17 00:00:00 2001 From: "John R. D'Orazio" Date: Mon, 8 Jun 2026 07:17:29 -0400 Subject: [PATCH] fix(term-propagation): two corruption bugs uncovered by ConfessIt backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both observed on production 2026-06-08 when backfilling project_tag terms on the ConfessIt community_project translations (referral predated PR #201, so its 5 non-EN siblings had zero tags). Toggling status to fire the propagation hook on each non-EN sibling produced: - DE post 1501: 4 correctly-localized tags (Beichte/Examen/Prüfung/ Versöhnung) — German distinguishes "examen" from "examination", no collision, all clean. - IT/PT posts (1497/1500): only 3 tags each — lost "examination" because OpenAI translates both EN "examen" and EN "examination" to the same target word (esame/exame). - ES/FR posts (1498/1499): 4 tags listed but one entry is the raw EN term 171 ("examen"), and querying term 171 directly now shows language=fr — the EN source term itself got language-flipped. - EN source 1485 still references term 171 in its term_relationships, so its `projectTags.nodes[1].language.slug` is now "fr". Cross-class corruption: an EN post serving a French-classified tag. Two distinct bugs in cdcf_get_or_create_translated_term, both fixed here: BUG 1 — pll_get_term self-fallback treated as "found a sibling". Some Polylang versions return the INPUT term_id from pll_get_term when no target-language sibling exists (vs returning 0/false). The handler's `if ($existing) return (int) $existing;` then returns the EN term_id, which propagates up to wp_set_object_terms($post, [..., EN_TERM_ID, ...]) — assigning an English term to a non-English post. Subsequent runs with the same EN term then call pll_set_term_language($new_id, $target_lang) on the EN term id (because `pll_get_term` would return the just-promoted-with-incorrect-language sibling), flipping its language out from under the EN post that still references it. Fix: guard with `if ($existing && (int) $existing !== (int) $en_term->term_id)`. BUG 2 — slug-collision adoption rewrites the colliding term's polylang group. When wp_insert_term hits term_exists (two distinct EN terms translating to the same target word), the old code adopted the existing term_id AND called both pll_set_term_language AND pll_save_term_translations on it. The save_term_translations call REASSIGNS the adopted term from its original EN sibling to ours — orphaning the original link. The set_term_language re-asserts language on a term that already had one, which in pathological cases (interacting with bug 1) flipped the language outright. Fix: on term_exists, look up the EN term's current polylang group: - If the colliding term IS already our sibling in this language, return its id (idempotent re-runs are safe). - Otherwise, log a warning and return null. Don't adopt; don't rewrite. The post ends up with fewer tags than the source had, which is information loss but NOT corruption. The alternative (rewriting the group) was a silent data-integrity violation. Test churn: - REMOVED test_get_or_create_adopts_existing_term_on_slug_collision — its assertion encoded the buggy adoption + rewrite behavior. - ADDED test_get_or_create_treats_pll_get_term_self_fallback_as_no_sibling (regression guard for bug 1; stubs pll_get_term to return the input term_id and asserts the handler creates a NEW term instead of reusing the EN id). - ADDED test_get_or_create_reuses_collision_term_when_already_my_polylang_sibling (idempotent re-run path on collision). - ADDED test_get_or_create_returns_null_when_collision_term_belongs_to_different_sibling (the corruption case — returns null, never calls pll_set_term_language or pll_save_term_translations). Production cleanup of the ConfessIt term state (restoring term 171's language to 'en', un-leaking it from ES/FR posts) is deferred to a follow-up manual step once this fix deploys — re-toggling status while the buggy code is live would just re-corrupt. All 9 existing tests for the function (the ones unrelated to the buggy adoption path) continue to pass unchanged. 546/546 theme suite green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../includes/admin/term-propagation.php | 74 ++++++++++---- .../tests/TermPropagationTest.php | 98 +++++++++++++++---- 2 files changed, 132 insertions(+), 40 deletions(-) diff --git a/wordpress/themes/cdcf-headless/includes/admin/term-propagation.php b/wordpress/themes/cdcf-headless/includes/admin/term-propagation.php index 5a009a9..59e0ce9 100644 --- a/wordpress/themes/cdcf-headless/includes/admin/term-propagation.php +++ b/wordpress/themes/cdcf-headless/includes/admin/term-propagation.php @@ -103,12 +103,21 @@ function cdcf_propagate_project_tags_on_publish($new_status, $old_status, $post) * ID. * * @return int|null The target-language term ID, or null on failure - * (OpenAI error, wp_insert_term error other than - * term_exists, Polylang missing). + * (OpenAI error, wp_insert_term error other than a + * term_exists already-my-sibling case, Polylang missing, + * or a slug collision against a term that belongs to a + * different EN sibling — see below). */ function cdcf_get_or_create_translated_term($en_term, string $target_lang): ?int { $existing = pll_get_term((int) $en_term->term_id, $target_lang); - if ($existing) { + // pll_get_term can return the input term_id itself as a fallback in + // some Polylang versions when no target-language sibling exists. + // Without this guard the EN term gets assigned to a non-EN post + // and pll_set_term_language below silently flips its language, + // corrupting the source — observed on production 2026-06-08 with + // ConfessIt: EN term 171 ("examen") ended up language=fr after + // propagation runs against ES/FR posts. + if ($existing && (int) $existing !== (int) $en_term->term_id) { return (int) $existing; } @@ -121,33 +130,58 @@ function cdcf_get_or_create_translated_term($en_term, string $target_lang): ?int 'slug' => sanitize_title($translated_name . '-' . $target_lang), ]); - $new_term_id = null; if (is_wp_error($result)) { - // Slug collision: WP returns term_exists with the existing - // term's ID in error_data. Adopt and Polylang-link it anyway. if ($result->get_error_code() === 'term_exists') { + // A target-language term with this slug already exists. + // Typical cause: two distinct EN terms whose OpenAI + // translations land on the same target word (e.g. "examen" + // + "examination" both translate to "examen" in Romance + // languages). Resolution policy: + // - If the colliding term IS already a Polylang sibling + // of our EN term, return its id (idempotent — safe to + // reuse on a re-run). + // - Otherwise, DO NOT adopt + rewrite the colliding term's + // Polylang group. That would reassign it from its + // original EN sibling to ours, orphaning the original + // link AND (because pll_set_term_language re-asserts + // language) sometimes flipping a term's language to + // match the adopter — the bug that corrupted ConfessIt. + // Skip with a logged warning and let the caller render + // fewer tags than the source had. $existing_id = (int) $result->get_error_data(); if ($existing_id) { - $new_term_id = $existing_id; + $en_translations = pll_get_term_translations((int) $en_term->term_id); + $is_already_my_sibling = is_array($en_translations) + && isset($en_translations[$target_lang]) + && (int) $en_translations[$target_lang] === $existing_id; + if ($is_already_my_sibling) { + return $existing_id; + } + error_log(sprintf( + 'cdcf_get_or_create_translated_term: slug collision on "%s" (%s); existing term %d is linked to a different EN sibling — skipping to avoid corrupting Polylang group. EN term %d will not be propagated to %s.', + $translated_name, + $target_lang, + $existing_id, + (int) $en_term->term_id, + $target_lang + )); + return null; } } - if (!$new_term_id) { - error_log(sprintf( - 'cdcf_get_or_create_translated_term: wp_insert_term failed for "%s" (%s): %s', - $translated_name, - $target_lang, - $result->get_error_message() - )); - return null; - } - } else { - $new_term_id = (int) $result['term_id']; + error_log(sprintf( + 'cdcf_get_or_create_translated_term: wp_insert_term failed for "%s" (%s): %s', + $translated_name, + $target_lang, + $result->get_error_message() + )); + return null; } + $new_term_id = (int) $result['term_id']; pll_set_term_language($new_term_id, $target_lang); - // Merge into the EN term's existing Polylang group so all 6 - // language siblings know about each other. + // Merge into the EN term's existing Polylang group so all language + // siblings know about each other. $translations = pll_get_term_translations((int) $en_term->term_id); if (!is_array($translations)) { $translations = []; diff --git a/wordpress/themes/cdcf-headless/tests/TermPropagationTest.php b/wordpress/themes/cdcf-headless/tests/TermPropagationTest.php index 486ccdd..f329a82 100644 --- a/wordpress/themes/cdcf-headless/tests/TermPropagationTest.php +++ b/wordpress/themes/cdcf-headless/tests/TermPropagationTest.php @@ -405,40 +405,98 @@ public function test_get_or_create_returns_null_when_openai_translation_fails(): $this->assertNull(cdcf_get_or_create_translated_term($this->fakeTerm(169, 'confession'), 'it')); } - public function test_get_or_create_adopts_existing_term_on_slug_collision(): void + public function test_get_or_create_treats_pll_get_term_self_fallback_as_no_sibling(): void { - // wp_insert_term returns a term_exists WP_Error with the colliding - // term's ID in data. The handler should adopt that ID and still - // Polylang-link it. + // Regression guard for the production-2026-06-08 corruption of + // ConfessIt EN term 171 ("examen"): some Polylang versions return + // the input term_id itself from pll_get_term when no + // target-language sibling exists. Without a self-equality guard, + // the handler treats this fallback as "found a sibling", assigns + // the EN term to a non-EN post, and pll_set_term_language flips + // the EN term's language to match the non-EN target. Here we + // stub pll_get_term to return the input id and assert the + // handler falls through to OpenAI + wp_insert_term instead of + // returning the EN id. $this->stubCommonFunctions(); - Functions\when('pll_get_term')->justReturn(0); - Functions\when('cdcf_openai_translate')->justReturn(['term' => 'confessione']); - Functions\when('wp_insert_term')->justReturn( - new WP_Error('term_exists', 'A term with this slug exists', 880) + Functions\when('pll_get_term')->alias( + static fn(int $term_id, string $lang): int => $term_id // self-fallback ); + Functions\when('cdcf_openai_translate')->justReturn(['term' => 'confessione']); + Functions\when('wp_insert_term')->justReturn(['term_id' => 802]); - $polyCall = new stdClass(); - $polyCall->set_lang = null; - $polyCall->save_translations = null; + $captured = new stdClass(); + $captured->set_lang = null; Functions\when('pll_set_term_language')->alias( - function (int $term_id, string $lang) use ($polyCall): bool { - $polyCall->set_lang = [$term_id, $lang]; + function (int $term_id, string $lang) use ($captured): bool { + $captured->set_lang = [$term_id, $lang]; return true; } ); - Functions\when('pll_save_term_translations')->alias( - function (array $translations) use ($polyCall): bool { - $polyCall->save_translations = $translations; - return true; - } + $this->allowAllFunctionsToExist(); + + $result = cdcf_get_or_create_translated_term($this->fakeTerm(169, 'confession'), 'it'); + + $this->assertSame(802, $result, 'must create a new IT term, not reuse the EN id 169'); + // pll_set_term_language must operate on the NEW term (802), not on + // the EN term (169) — that was the bug that flipped EN 171 to fr. + $this->assertSame([802, 'it'], $captured->set_lang); + } + + public function test_get_or_create_reuses_collision_term_when_already_my_polylang_sibling(): void + { + // Idempotent path on slug collision: the colliding term IS + // already a sibling of our EN term in this language (e.g. the + // hook fired twice for the same post). Return its id; do NOT + // re-run pll_set_term_language or pll_save_term_translations. + $this->stubCommonFunctions(); + Functions\when('pll_get_term')->justReturn(0); // no sibling via direct lookup + Functions\when('cdcf_openai_translate')->justReturn(['term' => 'confessione']); + Functions\when('wp_insert_term')->justReturn( + new WP_Error('term_exists', 'A term with this slug exists', 880) ); + // The collision-resolution path queries the EN term's polylang + // group; here term 880 IS already its IT sibling. + Functions\when('pll_get_term_translations')->justReturn([ + 'en' => 169, 'it' => 880, + ]); + Functions\expect('pll_set_term_language')->never(); + Functions\expect('pll_save_term_translations')->never(); $this->allowAllFunctionsToExist(); $result = cdcf_get_or_create_translated_term($this->fakeTerm(169, 'confession'), 'it'); $this->assertSame(880, $result); - $this->assertSame([880, 'it'], $polyCall->set_lang); - $this->assertSame(['en' => 169, 'it' => 880], $polyCall->save_translations); + } + + public function test_get_or_create_returns_null_when_collision_term_belongs_to_different_sibling(): void + { + // The corruption case: two distinct EN terms whose OpenAI + // translations land on the same target word (e.g. EN "examen" + // + EN "examination" both translate to "examen" in Romance + // languages). The colliding term is already linked to a + // DIFFERENT EN sibling; adopting + rewriting its polylang group + // would orphan that other link AND can flip term languages. + // Skip with a logged warning, return null. + $this->stubCommonFunctions(); + Functions\when('pll_get_term')->justReturn(0); + Functions\when('cdcf_openai_translate')->justReturn(['term' => 'examen']); + Functions\when('wp_insert_term')->justReturn( + new WP_Error('term_exists', 'A term with this slug exists', 880) + ); + // EN term 173 ("examination")'s polylang group does NOT have + // term 880 as its IT sibling — 880 belongs to a different EN + // sibling (e.g. EN 171 "examen", which translated to the same + // word in Italian). + Functions\when('pll_get_term_translations')->justReturn([ + 'en' => 173, // (we're asking about term 173, but 880 is not in this map) + ]); + Functions\expect('pll_set_term_language')->never(); + Functions\expect('pll_save_term_translations')->never(); + $this->allowAllFunctionsToExist(); + + $result = cdcf_get_or_create_translated_term($this->fakeTerm(173, 'examination'), 'it'); + + $this->assertNull($result); } public function test_get_or_create_links_new_term_into_polylang_group(): void