feat(serenity): sub-workspace dual-mode provisioning (brand-create, activate/deactivate, subworkspace reads/writes)

[rainer-friederich]

What

Implements Phase 1 (synchronous dual-mode) of Serenity Semrush sub-workspace provisioning per adobe/serenity-docs PR 12. Every /serenity/* operation resolves, in one place, whether a brand runs in flat mode (shared org parent workspace) or subworkspace mode (its own Semrush sub-workspace), driven entirely by brands.semrush_workspace_id. The flat path is unchanged; the subworkspace path is added alongside. This PR also adds serenity-first brand creation (provision the sub-workspace before the brand row) and propagates a brand's full identity (URLs, competitors, aliases) onto its Semrush projects.

NULL brands.semrush_workspace_id means the brand is not connected to a Semrush sub-workspace - nothing more. It does not imply anything about flat mode or whether the brand uses Semrush at all.

How it works

resolveBrandWorkspace(ctx, spaceCatId, brandId) returns { mode, workspaceId }:

• semrush_workspace_id set -> { mode: 'subworkspace', workspaceId: <brand sub-workspace> }
• absent -> { mode: 'flat', workspaceId: <org parent ws> }

The controller dispatches on mode. The only thing that differs between modes is how a (geoTargetId, languageCode) slice resolves to an upstream project - flat reads the BrandSemrushProject mapping; subworkspace resolves it live from the brand's own workspace via listProjects. Everything downstream (the Semrush prompt/tag/model calls, publish-once, per-project tag-cache invalidation, bulk caps) is shared, project-keyed logic. Auth is identical in both modes: an IMS bearer is required and forwarded to the Semrush gateway; only the workspace id in the upstream URL differs.

Surface

• Transport: workspace/project methods (sub-workspace create + settle/poll, family listing, resources/transfer, project list ?type=ai, init-status, benchmark create/delete) + upstream error classification (allocation-failure, workspace-not-ready, workspace-drift). Sub-workspace deletion is fail-closed behind SERENITY_ALLOW_WORKSPACE_DELETE (never set in any deployed env), so it is test-cleanup only.
• Lifecycle: ensureSubworkspace (lazy-create -> poll created -> persist column; re-grant path; ambiguous-create adoption by family title) and decommissionBrandWorkspace (delete every project + release the allocation back to the parent pool - never deletes the workspace).
• Subworkspace handlers: markets (list/get/create/delete) + prompts/tags/models, reusing the flat cores via additive exports + behaviour-preserving extraction (flat handlers delegate to project-keyed cores; their tests are unchanged and green).
• Brand-create (serenity-first): when POST /v2/orgs/:id/brands carries a semrushMarket, provision the sub-workspace + one published AIO project before writing the brand row, attach the chosen models, generate+attach topics/prompts, and publish. On a post-provision DB failure the orphaned allocation is released back to the parent pool (best-effort) and logged, not leaked.
• Brand identity propagation: the brand's own URLs (sites + social + earned) are pushed onto the project's own-brand benchmark (created on demand); the brand's competitors are tracked as region-filtered project benchmarks (AIO projects have no settings.ci, so the legacy CI-competitor list was a no-op); brand aliases drive branded/non-branded prompt classification. Re-synced across markets on brand edit.
• New endpoints: POST /serenity/activate (ensure the sub-workspace once -> publish caller-supplied markets -> set brand active) and POST /serenity/deactivate (decommission -> clear the semrush_workspace_id pointer to disconnect the brand back to flat mode -> set brand pending). A future activate allocates a fresh sub-workspace; the emptied one is never deleted.
• Catalogue endpoint: GET /v2/orgs/:id/serenity/languages returns the Semrush-supported language catalogue so the UI only offers languages the AIO gateway accepts (sibling to the existing serenity/models). Both org-level catalogue routes are mapped organization:read.
• Republish fix: PUT /serenity/models publishes after a model-set change so it goes live (latent bug; fixed in the shared core for both modes).
• OpenAPI: documents activate/deactivate, the previously-undocumented PUT /serenity/models, and additive subworkspace-mode market fields (status, semrushProjectId, initialized) - all optional, so flat responses still validate.

Hardening (multi-agent review pass)

• Register both org-level catalogue routes in routeRequiredCapabilities (route-coverage gate).
• Brand-create releases the provisioned sub-workspace allocation + logs the orphan id when the row write fails after provisioning.
• Brand-create enforces the IMS-only contract (getType() === 'ims') before forwarding the bearer upstream, matching requireImsBearer on the rest of /serenity/*.
• Redact the gateway URL (internal host + workspace UUIDs) from upstream 401/403 responses.
• Drop an unused aliased data-access dependency.

Validation

• Serenity unit + controller-dispatch + OpenAPI-contract + route-coverage tests green; docs:lint valid; docs:build OK; lint clean.
• Live through-api e2e against the real dev Semrush gateway: flat markets -> activate (real sub-workspace created + persisted + market published) -> subworkspace markets/detail/models/prompts/tags -> deactivate (projects deleted, allocation released, pointer cleared). Net-zero cleanup verified. Brand-create provisioning (sub-workspace + URLs + competitor benchmarks + own-brand benchmark) verified live.

Cross-repo ordering

The runtime dataAccess.Brand entity and the brands.semrush_workspace_id column are merged and released:

• adobe/spacecat-shared Brand entity (feat(data-access): add minimal Brand entity for serenity sub-workspace spacecat-shared#1679) - merged + released as @adobe/spacecat-shared-data-access@3.75.0.

UI companion: adobe/project-elmo-ui 2078.

Known follow-ups (not blocking)

• First-time multi-market activate carves a flat CREATE_ALLOCATION (1 project); sizing it from marketCount is pending live Gate-A verification of the limits contract.
• Brand-edit Semrush re-sync runs serially across markets inside the synchronous PATCH; bound the fan-out (or add per-market failure observability) before high-market-count edits.

🤖 Generated with Claude Code

[github-actions]

This PR will trigger a minor release when merged.

[rainer-friederich]

Pushed a round of fixes from a local multi-perspective review (correctness, simplicity/DRY, conventions/security), plus the dependency bump and a description refresh.

Code fixes (commit c2f3cff):

• Route mapping: activate/deactivate are now in routeRequiredCapabilities as organization:write. This is the fix for the failing CI — the route-coverage drift test requires every route to be in routeRequiredCapabilities or INTERNAL_ROUTES, and these two were unmapped.
• Batch activate: ensureSubworkspace now runs once for the whole batch, sized to the real market count, and each market is created against the resolved workspace. Previously it re-granted + double-polled per market, so a multi-market activate risked the Lambda timeout and under-sized the allocation.
• Auth correctness: a subworkspace-mode brand is no longer 404'd when the org parent workspace pointer is missing — it resolves against its own sub-workspace. Only flat mode without a parent is a real 404.
• Cache correctness: deactivate invalidates the resolver brand cache before the save (the upstream is already emptied by decommission), so a failed save cannot keep routing to the emptied sub-workspace for the positive-TTL window.
• Readability/docs: clarified the ambiguous-create guard !(e instanceof ErrorWithStatusCode), corrected the createTaggedPrompts body-shape comment to match the prompt-text-keyed shape both modes send, and dropped a stray log arg passed to the 4-param flat handleListMarkets.

Tests: added/updated for batch-once activate, subworkspace-survives-missing-parent, cache-cleared-on-failed-save, and the pre-resolved-workspace create path. Serenity unit + dispatch + OpenAPI-contract + route-coverage suites green; lint and docs:lint clean.

Dependency: bumped @adobe/spacecat-shared-data-access to 3.75.0 (the released Brand entity), replacing the local npm-link used for e2e.

Deliberately deferred: the remaining flat/subworkspace prompts-orchestration duplication (list/create/bulk-delete) is left as-is and tracked in docs/specs/2026-06-15-serenity-flat-mode-removal-plan.md — extracting it now would couple the frozen flat handlers that are slated for deletion.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[rainer-friederich]

Second local review round (multi-perspective: correctness/architecture, security/auth, QA). No Critical or Important findings - both the security and architecture passes approved the new sub-workspace-vs-parent safety guard. Addressed the minor follow-ups:

• Self-defending decommission: decommissionBrandWorkspace now takes the parent workspace id and refuses to empty/release the org parent pool, so the most destructive primitive is protected even if a future caller ever bypasses the authorize() chokepoint. deactivate passes the resolved parent.
• OpenAPI: documented the new 409 workspaceMisconfigured response on activate and deactivate.
• Plan doc: noted the implementation deviations (the unwired error predicates and the two unused transport methods were dropped; the sub-workspace must-not-equal-parent invariant was added).
• Tests: covered the statusless-throw fallbacks (activate -> 502; prompts-subworkspace create/bulk-delete -> 500), flat-mode createMarket/bulkDeletePrompts dispatch, the decommission self-guard, and added a brand.save assertion on the partial-failure path.

Coverage: every changed serenity file now has zero uncovered lines; codecov/patch and ci build are green. Lint and docs:lint clean.

Context on the headline guard (answering an earlier question): if a brand's semrush_workspace_id ever equals the org's parent workspace id, the brand would resolve to sub-workspace mode pointing at the shared parent - and deactivate's decommission (delete all projects + release allocation) would wipe the parent pool for every brand in the org. That is now forbidden at three layers: the authorize() chokepoint (409, refuses all operations), ensureSubworkspace (re-grant and create paths), and decommissionBrandWorkspace itself.

[rainer-friederich]

Backwards compatibility check: flat mode

I reviewed whether existing brands (no semrush_workspace_id, i.e. flat mode running against the org's shared parent workspace) still behave exactly as before this change. Verdict: flat mode is preserved. No breakage to existing flat brands.

Routing is additive. The new routes (serenity/models, serenity/languages, serenity/activate, serenity/deactivate) are all new; no existing serenity route changed.

Mode resolution falls through to flat safely. resolveBrandWorkspace returns mode: 'flat' with workspaceId = the org parent whenever the brand has no semrush_workspace_id, resolved via the same resolveWorkspaceId lookup main used. A null brand subworkspace id is cached negatively, so a flat brand can never be routed into subworkspace code.

Dispatch preserves the original handlers. Every operation uses mode === 'subworkspace' ? <new handler> : <original flat handler>, and each flat branch calls the original handler with args equivalent to main (auth.workspaceId equals the old auth.semrushWorkspaceId). All eleven existing operations (prompts list/create/update/bulk-delete, markets list/get/create/delete, tags, models list/update) were enumerated and pass equivalent args.

Markets handlers keep identical flat behavior. handleListMarkets, handleGetMarket, handleCreateMarket, handleDeleteMarket, handleListTags, handleListModels produce the same upstream calls, response shapes, validation, and cache keys. The markets.js refactor only extracted behavior-preserving helpers (resolveLanguageId, listTagsForProject, syncModelsForProject, defaultMarketName) and moved resolveLocation to locations.js (verbatim, re-exported).

Brand create/edit gates the new behavior. Semrush provisioning is gated on semrushMarket being present; a create without it follows the existing path unchanged (forceBrandId/semrushWorkspaceId default to null, and the status computation collapses to the old hasBaseSite logic). The edit re-sync is gated on the brand having a semrush_workspace_id, so flat edits skip it entirely. mapDbBrandToV2 only adds semrushWorkspaceId (null for flat brands); no existing field is removed or renamed.

One intentional behavior change worth flagging: the flat PUT /models (handleUpdateModels) now publishes the project after a model-set change, via syncModelsForProject. On main the assignment was staged but never published, so the change never went live. This is a latent-bug fix rather than a regression: nothing that previously worked breaks; an endpoint that silently no-op'd now actually takes effect.

[calvarezg]

Multi-persona review (Architect · DBA · Security · Tester)

Strong, unusually well-defended PR — the destructive-safety design (3-layer parent-equality guard, never-delete decommission, brand-uuid adoption titles, fail-closed delete flag) and security posture (in-org authorize on every route, server-derived workspace ids, uniform 401/403 redaction, IMS-only bearer forwarding) all hold up. Security found nothing exploitable. One Major worth fixing before merge.

🟠 Major — activate status-save is unguarded (asymmetric with deactivate)

src/controllers/serenity.js:L2324-L2327

After markets go live, brand.setStatus('active'); await brand.save(); has no try/catch. If the save fails: markets are live upstream (pointer is correctly persisted by ensureSubworkspace), but brands.status stays pending, the throw hits mapError, and the caller gets a 5xx instead of the 207/200 body listing which markets went live. There's also no greppable alert token — unlike deactivate, which deliberately wraps the same seam with SERENITY_DEACTIVATE_SAVE_DIVERGENCE (L2397-L2416).

Fix: mirror deactivate — try/catch around save(), log a distinct token with {brandId, semrushWorkspaceId, marketsLive}, and return the 207 body (markets are live) rather than collapsing to a 5xx. Blast radius is bounded (re-activate converges), but the asymmetry reads as an oversight.

🟡 Minor / Questions

• Concurrent-activate "both-read-null" race isn't pinned by a test (workspace-lifecycle.js:L6196-L6234). 100% line coverage hides it (the null-winner test runs the same lines). Add a both-read-null characterization test before the tracked conditional-write fix lands.
• Brand-edit re-sync has no per-market failure observability (brands.js:L1606-L1619) — author-acknowledged. A mid-fan-out throw logs aggregate counts but not which market split. Log {workspaceId, projectId, market} before propagating.
• Deferred prompts-orchestration duplication (prompts-subworkspace.js vs prompts.js) — defensible given planned flat-mode removal; add a cross-reference comment in both files so a future fixer knows the twin exists.
• Q: flat PUT /serenity/models now publishes — disclosed in the description as a latent-bug fix, so not silent, but confirm no flat caller relied on staging model changes before a separate publish.
• Q: brand_name_display always uses brandNames[0], silently ignoring an explicit brandDisplayName (markets-subworkspace.js buildCreateProjectBody). Either honor it or drop it from the contract.
• Minor: deactivate save-failure stale-pointer window (documented, self-healing, has alert token — fine for Phase 1); unicode brand-name title round-trip untested.

Recommendation: Approve with reservations — address the Major before merge; the rest are discretionary or already tracked.

🤖 Multi-persona review (findings verified against the diff)

[rainer-friederich]

Thanks for the thorough multi-persona pass — addressed in ab640f85.

Major — activate status-save now guarded (symmetric with deactivate)

Wrapped the setStatus('active'); save() seam in a try/catch that mirrors deactivate's divergence guard:

• emits a distinct, greppable token SERENITY_ACTIVATE_SAVE_DIVERGENCE with {brandId, semrushWorkspaceId, marketsLive};
• forces the partial-failure path so the caller gets a 207 with the per-market results (markets ARE live) instead of a 5xx that discards them;
• falls through to return the multi-status body rather than throwing to mapError.

A re-activate converges (every live market returns 409), so the divergence self-heals. New test asserts 207 + status: active + the token on a save failure.

Minor / Questions

• Both-read-null race characterization — added a two-writer Promise.all characterization test in workspace-lifecycle.test.js: two activations that both re-read null both persist their own distinct workspace id and neither releases. Pins the documented residual race so the future conditional-write fix has a target to flip.
• Brand-edit re-sync per-market observability — addressed at the fan-out level (the better place — the controller can't know which market threw). Both syncBrandUrlsAcrossMarkets and syncCompetitorBenchmarksAcrossMarkets now log {workspaceId, projectId, market} (status only — the upstream text carries the gateway URL) before rethrowing. Tests cover the per-market throw path.
• Deferred prompts duplication — added explicit TWIN FILE notes to both prompts.js and prompts-subworkspace.js flagging the duplication as deferred pending flat-mode removal, so a future fixer knows the twin exists and to keep them in lockstep.
• Q: flat PUT /serenity/models now publishes — confirmed safe. There is a single PUT /serenity/models route and no separate publish-models step; on main, flat handleUpdateModels made zero publishProject calls, so model changes never propagated upstream at all. No flat caller could have relied on staging-without-publish — the change simply makes the PUT take effect.
• Q: brand_name_display ignoring brandDisplayName — now honored. buildCreateProjectBody uses brandDisplayName when present (falling back to brandNames[0]), making it consistent with the own-brand benchmark built from brandDisplayName (attachBrandUrlsToProject) and with the re-sync path that reads brand_name_display back as the benchmark name. Tests cover honored-vs-fallback.
• Unicode brand-name title round-trip / deactivate stale-pointer window — leaving as-is for Phase 1 (the deactivate window is documented, self-healing, and already has an alert token, as you noted).

All modified files at 100% line coverage; full serenity + brands suites green, lint clean.

[calvarezg]

Review follow-up — finding status after ab640f855

Re-checked every finding from the multi-persona review against the current head. ab640f855 ("guard activate status-save divergence + close review follow-ups") closes all actionable items. Status below.

Note: the earlier comment's serenity.js:L2324/L2397 cites were offsets into a concatenated diff, not source lines — the real activate seam is serenity.js:849. Apologies for the confusion.

#	Finding	Severity	Status
1	activate status-save unguarded (asymmetric with deactivate)	🟠 Major	✅ Fixed — SERENITY_ACTIVATE_SAVE_DIVERGENCE token + forces 207 (markets are live) instead of 5xx, with test (serenity.js:850)
2	Brand-edit re-sync had no per-market failure observability	🟡 Minor	✅ Fixed — fan-outs now log {workspaceId, projectId, market} before rethrow, + tests
3	brand_name_display ignored explicit brandDisplayName	❓ Question	✅ Fixed — now honors brandDisplayName, falls back to brandNames[0], + test
4	Concurrent-activate "both-read-null" race not pinned by a test	🟡 Minor	✅ Fixed — two-writer characterization test added
5	Deferred flat/subworkspace prompts duplication	🟡 Minor	✅ Fixed — explicit TWIN-FILE cross-reference notes in both files
6	Flat PUT /serenity/models now publishes	❓ Question	⚪ No change needed — disclosed in the PR description as a latent-bug fix; the open part is a product confirmation (did any flat caller rely on staging model changes before a separate publish?), not a code defect
7	deactivate save-failure stale-pointer window	🟡 Minor	⚪ Accepted for Phase 1 — already carries the SERENITY_DEACTIVATE_SAVE_DIVERGENCE alert token; documented and self-healing on re-activate
8	Unicode brand-name → workspace-title round-trip untested	🟡 Minor	⚪ Open (nit) — lowest priority; plain JS strings in JSON bodies, low risk

Net: 0 Critical, the 1 Major closed, all actionable minors addressed with tests. Items 6–8 are a disclosed product question and two explicitly-accepted Phase-1 residuals — none is a merge blocker. Code is in good shape to accept once ci / build goes green and an approving review lands.

🤖 Multi-persona review follow-up

[solaris007]

🎉 This PR is included in version 1.579.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀