refactor(serenity): skip the brand's own primary domain in brand URLs

[rainer-friederich]

1. Abstract

Fixes the duplicate brand-domain entry a Serenity brand shows in Semrush by not adding the brand's own primary domain as a brand URL (it already lives on the project's own-brand benchmark). Replaces the earlier "resolve brand URLs to their scheme-less form before writing" approach, which a prod re-capture showed was invalid.

2. Reasoning

A brand shows its primary domain twice in Semrush (e.g. lovesac.com): once as the own-brand benchmark's own primary_url (which Semrush stores scheme-less), and once as an explicit website brand-URL row (https://lovesac.com) the write path added. A prod re-capture (2026-07-06) of the live contract established two facts that invalidate the original url/resolve approach this PR opened with:

• create_brand_urls REQUIRES a literal https:// value — it 400s on the scheme-less resolve primary_url ('url' tag) and on http:// ('startswith' tag). So the resolved scheme-less form cannot be written as a brand URL at all.
• create_benchmarks already normalizes the benchmark domain server-side, so url/resolve was redundant for the one field where a scheme-less value is valid.

Because a brand_urls row must keep its https:// scheme and the benchmark domain is scheme-less, the two can never collapse — the only way to remove the visible duplicate is to not emit the primary domain as a brand URL in the first place.

3. High-level overview of the changes

Before: every brand-URL entry was pushed to the own-brand benchmark, and (in the reverted revision) each was first run through url/resolve.

After: the brand's own primary domain is dropped from the website set; every other brand URL (secondary sites, socials, earned) is written verbatim as an https:// value.

• collectBrandUrlEntries takes the project's own-brand domain and skips any website URL whose host matches it. The match is host-wise via the existing normalizeBenchmarkDomain, so both the apex and www. forms of the primary are dropped.
• The market-create path passes the brand's domain; the brand-edit re-sync passes each project's own domain.
• Removes the resolveUrl transport method and the resolveBrandUrlEntries step — brand URLs are now written exactly as collected (the upstream stores them as-is and does not collapse www.-vs-apex, so no normalization is applied).

Net user-visible effect: a Serenity brand no longer double-lists its primary domain in Semrush; socials and any secondary sites are unchanged.

4. Required information

• Jira / issue: serenity-docs#25 — https://github.com/adobe/serenity-docs/issues/25
• Other: prod re-capture of the live brand-URL write contract (2026-07-06, throwaway benchmark in a test workspace)

5. Affected / used mysticat-workspace projects

• spacecat-shared (contract) — the vendored Project Engine mock is tightened to enforce the live https:// requirement so this repo's IT can't go green over a prod-400: fix(project-engine-client): enforce https:// on the brand_urls mock spacecat-shared#1778
• mysticat-data-service (related) — the serenity_migration CLI makes the same skip-primary-domain change on its write path: https://github.com/adobe/mysticat-data-service/pull/737

6. Additional information outside the code

The live contract behind this change was re-captured against prod adobe-hackathon.semrush.com on 2026-07-06: a read-only url/resolve edge matrix, plus a write-probe of create_brand_urls / create_benchmarks on a throwaway benchmark created and deleted in an authorized test workspace. That probe is what established the https://-only requirement and the server-side benchmark-domain normalization. The paid customer's live data was inspected read-only (no writes) and confirmed to carry the duplicate this PR prevents.

7. Test plan

• (a) Local: unit tests for collectBrandUrlEntries (primary-domain skip in apex + www forms; all-kept when no primary given) and both write paths (create attaches the non-primary set; edit diffs verbatim with no churn). The serenity IT exercises the create/edit write paths against the mock.
• (b) Per-env: after merge, verify on dev by creating (or editing) a Serenity brand whose website equals its benchmark domain and confirming the AIO benchmark shows the primary domain only once (no second https://<domain> brand-URL row), while socials/secondary sites still attach.

8. Deployment & merge order

• serenity-docs#25 redesign is a coordinated set. The spacecat-shared mock fix (fix(project-engine-client): enforce https:// on the brand_urls mock spacecat-shared#1778) hardens the contract this PR relies on; merging it first (and cutting a release) lets this PR's IT run against the tightened mock. The migration-side change (https://github.com/adobe/mysticat-data-service/pull/737) is independent but part of the same redesign. Suggested order: fix: remove NEW status restriction from edge-preview #1778 (+ release) → this PR / fix(deps): update adobe fixes #737.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MysticatBot]

Hey @rainer-friederich,

Verdict: Request changes - one convention fix needed before merge.
Complexity: HIGH - medium diff; dependency risk signal.
Changes: Adds URL canonicalization via Semrush's url/resolve before writing brand URLs, preventing www-vs-apex duplicates on both the create and edit paths (6 files).
Note: CI checks are currently pending.

Must fix before merge

1. [Important] Version specifier ^1.8.0 is inconsistent with the project's exact-pin convention - package.json:96 (details inline)

Non-blocking (4): minor issues and suggestions

• suggestion: Resolve brand-wide website URLs once before the per-market loop in syncBrandUrlsAcrossMarkets to avoid redundant HTTP calls (the resolve result is deterministic and region-less) - src/support/serenity/brand-urls.js:409
• nit: De-dup in resolveWebsiteEntries keys on url alone; a type-qualified key (${type}:${url}) would prevent a theoretical cross-type collision - src/support/serenity/brand-urls.js:155
• nit: The warn message says "url/resolve returned is_valid:false" even when the actual condition is is_valid:true with empty primary_url - src/support/serenity/brand-urls.js:131
• suggestion: Add a unit test for resolveUrl returning null/undefined to document the optional-chaining defense - test/support/serenity/brand-urls.test.js

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 23s | Cost: $5.53 | Commit: 7067b67edce8ac302d2291bc43c6f2ab4ea4ba1c
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[github-actions]

This PR will trigger a minor release when merged.

[rainer-friederich]

Addressed the review in b7ae9ad.

Must fix:

• [Important] ^1.8.0 inconsistent with the exact-pin convention — done. Pinned to exact 1.8.0 and regenerated the lockfile.

Non-blocking:

• suggestion: resolve website URLs once before the per-market loop — done. resolveWebsiteEntries now takes an optional per-sync resolve cache, and syncBrandUrlsAcrossMarkets shares one across markets; since website urls are region-less, each distinct one is resolved a single time (cache hit on markets 2..N). Added a helper test and a two-market sync test asserting one resolve call.
• nit: warn message inaccurate for the is_valid:true + empty primary_url case — done. Reworded to "url/resolve returned no usable primary_url — keeping raw url" (and it now warns only on a cache miss, so a bad url warns once, not per market).
• suggestion: test for resolveUrl returning null/undefined — done. Added a unit test pinning the optional-chaining defense.
• nit: type-qualified de-dup key (${type}:${url}) — declined. resolveWebsiteEntries intentionally mirrors collectBrandUrlEntries' url-only de-dup: a Semrush brand URL is unique by its url, so keying on ${type}:${url} would let the same url be written twice under different types (Semrush treats it as one entry). Post-resolution a canonicalized website (scheme-less apex, e.g. acme.com) also can't collide with a social/earned handle (a full https:// profile url), so the theoretical cross-type collision can't occur.

[MysticatBot]

Hey @rainer-friederich,

Verdict: Approve - all prior findings addressed, no new blocking issues.
Complexity: HIGH - medium diff; dependency risk signal.
Changes: Adds URL canonicalization via Semrush's url/resolve before writing brand URLs, preventing www-vs-apex duplicates on both the create and edit paths (6 files).
Note: CI checks are currently pending.

Previously flagged, now resolved

• Version pin now exact 1.8.0 (was ^1.8.0), consistent with all other shared dependencies
• Shared resolve cache across markets eliminates redundant HTTP calls for region-less website URLs
• Warn message reworded to accurately cover both is_valid:false and empty primary_url cases
• Unit test for resolveUrl returning null/undefined now present

Non-blocking (3): minor issues and suggestions

• nit: No test for cache-hit with a null (unresolvable) cached value - a test pre-seeding the cache with new Map([['https://bad.com', null]]) and asserting no re-resolve and no re-warn would pin the once-per-url contract explicitly - src/support/serenity/brand-urls.js:157
• nit: attachBrandUrlsToProject has no integration-level test verifying the de-dup effect of resolution (two raw URLs collapsing to one canonical), though the unit-level coverage in resolveWebsiteEntries tests is solid - test/support/serenity/brand-urls.test.js
• suggestion: Sequential resolution in resolveWebsiteEntries (one await per URL) is fine for current cardinality (1-3 website URLs per brand) but would serialize round-trips if a brand ever carries 20+ URLs - a bounded Promise.all would be a future optimization if that threshold is reached - src/support/serenity/brand-urls.js:130

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 6s | Cost: $6.54 | Commit: b7ae9ad37c0f04eab258412cb31e8361486b2346
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[rainer-friederich]

Follow-up in 2a5e049 (test-only): the new resolveUrl transport wrapper had no direct unit test (the brand-urls tests stub it), so codecov/patch flagged it. Added a rest-transport test asserting it GETs /v1/url/resolve?primary_url=… and returns the body — mirrors listGlobalAiModels. Re-requesting so the approval lands on HEAD.

[MysticatBot]

Hey @rainer-friederich,

Verdict: Approve - test-only commit, no new concerns.
Complexity: HIGH - medium diff; dependency risk signal.
Changes: Adds a unit test for the resolveUrl transport wrapper to close the Codecov patch coverage gap (7 files).
Note: CI checks are currently pending.

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 13s | Cost: $3.69 | Commit: 2a5e04923c1661a3cac2d9bb57b1ee7857c9b3b2
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[solaris007]

🎉 This PR is included in version 1.624.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀