feat(serenity): defer Semrush provisioning for pending (draft) brands

[rainer-friederich]

What

Lets the add-brand wizard's "Save as pending" store a Semrush brand with no sub-workspace, no project, and no primary URL — deferring all Semrush provisioning to activation.

Previously a pending save still 400'd with A primary URL is required to provision a Semrush brand, because createBrandForOrg branched only on whether semrushMarket was present, never on status. The elmo client already sent { status: 'pending', semrushMarket } correctly — the fix is server-side.

Changes

• createBrandForOrg: when status is pending, skip provisioning entirely and stash { primaryUrl: first url || null, markets:[{market,languageCode}] } on the brand instead of requiring a primary URL + AI models. The row lands as pending (no anchor — no site_id, no semrush_workspace_id). The active path is unchanged (still provisions).
• brands-storage: persist / clear / surface pending_semrush_provisioning (upsertBrand, updateBrand, mapDbBrandToV2).
• activate: fall back to the stash when the request omits markets / brandDomain, then remove each market that provisions (status 201 or 409) from the stash — nulling the column once none remain, keeping failed markets (with the primary URL) for retry. The brand still flips active on any live market.
• OpenAPI Brand schema documents the read-only pendingSemrushProvisioning field.

Storage shape

One JSONB column brands.pending_semrush_provisioning = { primaryUrl?, markets:[{market, languageCode}] }. It folds both the deferred market and the primary URL (which otherwise lives only on the Semrush side) into a single draft-only staging area.

Out of scope

AI models for a pending brand are not stored, and activate does not attach models (pre-existing — models go via PUT /serenity/models). Models chosen during a pending save are dropped.

Tests

581 passing in the touched suites (+12 new: pending create paths, storage persist/clear, activate fallback + per-market removal). Lint, OpenAPI validation, and elmo tsc/build all clean.

Dependency ordering

Requires both to ship first:

• mysticat-data-service migration for brands.pending_semrush_provisioning: https://github.com/adobe/mysticat-data-service/pull/710
• spacecat-shared brand pendingSemrushProvisioning attribute: feat(data-access): add brand pendingSemrushProvisioning attribute spacecat-shared#1692 (bump the dependency once published)

Reads are forward-safe before then (base collection selects *; save() only patches dirty attributes), but the pending-create write needs the column to exist.

UI companion (already updated): https://github.com/adobe/project-elmo-ui/pull/2078

🤖 Generated with Claude Code

[github-actions]

This PR will trigger a minor release when merged.

[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 authorization gap and two code-quality concerns need addressing before merge.
Changes: Defers Semrush provisioning for pending (draft) brands by stashing market + primaryUrl at create time and replaying at activation (7 files).

Must fix before merge

1. [Critical] pendingSemrushProvisioning writable via PATCH without runtime enforcement - src/support/brands-storage.js:878 (details inline)
2. [Important] Null brandDomain silently passed to handleCreateMarketSubworkspace - src/controllers/serenity.js:802 (details inline)
3. [Important] domainFromUrl duplicates brandDomainFromPayload creating divergence risk - src/controllers/serenity.js:85 (details inline)
4. [Important] Stale docs/serenity.md no longer reflects activate endpoint's fallback behavior - the doc describes markets as required in the activate body, which is no longer true for pending brands. Update the "activate / deactivate" section to document the new fallback path and stash-cleanup lifecycle.

Non-blocking (3): minor issues and suggestions

• nit: OpenAPI schema pendingSemrushProvisioning.markets items lack required: [market, languageCode] constraint - docs/openapi/schemas.yaml:3844
• suggestion: Stash cleanup is coupled to the anyLive && setStatus guard. Self-heals via 409 idempotency on retry, but a comment explaining why the coupling is acceptable would help future readers - src/controllers/serenity.js:904
• suggestion: Create path hardcodes single-market stash (markets: [{ market, languageCode }]) while activate handles N markets. If the wizard evolves to multi-market draft creation, this asymmetry becomes a bug. A TODO comment would flag the constraint - src/controllers/brands.js:1410

Note: CI checks are passing.

---

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

[MysticatBot]

issue (blocking): pendingSemrushProvisioning is writable via the PATCH /brands/:brandId endpoint. The OpenAPI schema marks this field readOnly: true, but that is a documentation hint only - there is no runtime enforcement. The updateBrandForOrg controller passes context.data into updateBrand as updates, and the new code here persists updates.pendingSemrushProvisioning if present.

Why it matters: This field is system-controlled (set during "Save as pending", consumed and cleared during activation). An authenticated user can inject arbitrary primaryUrl and markets into any brand they can PATCH. At activation without explicit body parameters, the stashed values are trusted - primaryUrl flows into brandDomain for upstream Semrush project creation, and markets drive which markets get provisioned.

Fix: Strip the field in updateBrandForOrg before passing to updateBrand:

delete updates.pendingSemrushProvisioning;

[MysticatBot]

issue (blocking): brandDomain can resolve to null here (stash has no primaryUrl, or URL is unparseable) and that null propagates into handleCreateMarketSubworkspace. The non-pending create path in brands.js explicitly guards this with if (!hasText(brandDomain)) return badRequest(...). Activation should fail fast with the same discipline.

Why it matters: A silent null can produce an opaque upstream error or an orphaned sub-workspace rather than a clear 400 to the caller.

Fix: After line 802, add:

if (!hasText(brandDomain)) {
  throw new ErrorWithStatusCode('brandDomain is required to provision a Semrush market', 400);
}

[MysticatBot]

issue (blocking): domainFromUrl reimplements the hostname-extraction logic from brandDomainFromPayload in src/controllers/brands.js. The JSDoc acknowledges "Mirrors brandDomainFromPayload" - but code duplication IS the divergence mechanism. When someone adds www. stripping or punycode normalization to one site and not the other, a draft brand will resolve to a different domain at activation than at direct create.

Why it matters: Two implementations of the same domain derivation that must agree by contract but share no code.

Fix: Extract the URL-string-to-hostname kernel into a shared utility (e.g., src/support/url-utils.js) and import from both controllers.

[rainer-friederich]

Addressed all 7 items in 15658257:

Must-fix

• [Critical] PATCH injection — updateBrandForOrg now delete updates.pendingSemrushProvisioning before calling updateBrand (mirrors the existing delete updates.baseUrl). Added a test asserting an attacker-supplied stash is stripped.
• [Important] Null brandDomain at activate — fail fast with a 400 (brandDomain is required to provision a Semrush market), matching the create path's guard, before anything reaches handleCreateMarketSubworkspace. Rewrote the two null-brandDomain activate tests to assert the 400.
• [Important] domainFromUrl duplication — extracted the URL→hostname kernel into src/support/url-utils.js (hostnameFromUrlString); both brandDomainFromPayload (brands) and the serenity activate fallback now call it. Added a unit test for the util.
• [Important] Stale docs/serenity.md — activate section now documents brandDomain/markets as optional with the stash fallback and the per-market cleanup lifecycle.

Non-blocking

• OpenAPI pendingSemrushProvisioning.markets items now carry required: [market, languageCode].
• Added a comment explaining why stash cleanup is coupled to the anyLive && setStatus guard (idempotent 409 retry self-heals).
• Added a TODO flagging the single-market draft stash vs N-market activate asymmetry.

459 tests pass; serenity/brands/url-utils at 100% line coverage; docs:lint valid.

Note: the @adobe/spacecat-shared-data-access bump to the version carrying the pendingSemrushProvisioning schema attribute is a separate follow-up commit, pending the spacecat-shared release (1692 just merged and is publishing).

[MysticatBot]

Hey @rainer-friederich,

Verdict: Approve - all 7 prior findings addressed correctly; no new issues.
Changes: Defers Semrush provisioning for pending (draft) brands by stashing market + primaryUrl at create time and replaying at activation (10 files).

Previously flagged, now resolved

• PATCH injection of pendingSemrushProvisioning - stripped via delete before reaching storage, with regression test
• Null brandDomain at activate - fail-fast 400 guard added, matching the create path's discipline
• domainFromUrl duplication - extracted to shared hostnameFromUrlString in src/support/url-utils.js
• Stale docs/serenity.md - activate section now documents the fallback path and stash lifecycle
• OpenAPI pendingSemrushProvisioning.markets items now carry required: [market, languageCode]
• Stash cleanup coupling comment added explaining idempotent-retry safety
• Single-market stash TODO added flagging the wizard limitation

Note: CI build is pending at review time.

---

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

[solaris007]

🎉 This PR is included in version 1.584.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀