feat(serenity): org-flag brand-create mode, sub-workspace-only pending activation, site_id anchoring (LLMO-6405)#2878
Conversation
…g activation, site_id anchoring (LLMO-6405) Backend for the brand & market management rework (serenity-docs #68). Market-scoped inputs move out of brand creation into market creation. - createBrandForOrg: Semrush mode is decided by the org serenity flag (isSerenityActiveForOrg), not the request body. A non-serenity org sending a market or generatePrompts:true is rejected (403); a bare generatePrompts:false stays a flat create. A serenity create WITHOUT a market provisions a bare sub-workspace (no project) via the new provisionBrandSubworkspaceBare; a supplied market keeps the existing project path. - upsertBrand: write brands.site_id from baseSiteId even on the Semrush-anchored path (a Semrush brand is anchored by BOTH its sub-workspace and its primary site). The DB CHECK tightening + NULL backfill remain a follow-up migration. - /serenity/activate: a pending brand activates sub-workspace-only (ensure sub-workspace + flip active + clear stash), never auto-creating a market. The body-driven market path runs only for an already-active brand (reactivation / onboarding API) and never downgrades it on a partial failure (207). Tests: serenity activate block recast to the new model + new pending sub-workspace-only cases; upsertBrand site_id tests updated; provisionBrandSubworkspaceBare covered. npm test + coverage green; type-check + lint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
This PR will trigger a minor release when merged. |
…t-management rework (LLMO-6405) - serenity.md: create mode is decided by the org serenity flag (not the body); a market-less serenity create provisions a bare sub-workspace; a pending brand activates sub-workspace-only (no market, no stash-driven provisioning); an already-active brand's reactivation is body-driven and never downgraded (207); brands.site_id anchors the brand from baseSiteId; site-mirror lifecycle updated. - openapi/schemas.yaml: add baseSiteId to V2BrandInput; rewrite the SerenityActivateRequest description (pending = sub-workspace-only, ignores markets/stash; active = body-driven) and the generatePrompts/modelIds field docs. docs:lint passes (API description valid). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Hey @igor-grubic,
Verdict: Request changes - three test coverage gaps for new code paths.
Complexity: HIGH - large diff; API surface.
Changes: Reworks brand-create mode detection to use the org-level serenity flag, makes pending activation sub-workspace-only, removes the Semrush site_id skip, and adds provisionBrandSubworkspaceBare (8 files).
Note: CI checks are currently failing - resolve before merge.
Must fix before merge
- [Important] No controller-level test for
provisionBrandSubworkspaceBarefailure during create -test/controllers/brands.test.js(details inline) - [Important] No assertion that
ensureMarketSiteis skipped on the bare sub-workspace create path -test/controllers/brands.test.js(details inline) - [Important] Missing test for
generatePrompts:true(no market) from a non-serenity org triggering 403 -test/controllers/brands.test.js(details inline)
Non-blocking (3): minor issues and suggestions
- nit:
deleteAllProjectsis called on a known-empty workspace in theprovisionBrandSubworkspaceBarecatch block -src/support/serenity/brand-provisioning.js:~460. The comment says it tolerates empty workspaces, but it is a wasted API call; consider removing it and calling onlyreleaseFullAllocation. - nit: Test title "STILL enters Semrush mode for a PENDING draft with generatePrompts:false and no market" is now misleading - it enters Semrush mode because the org is serenity-active, not because of the flag's presence -
test/controllers/brands.test.js:~5714. - suggestion: The double-guard
provisionedWorkspaceId && hasText(provisionedWorkspaceId) && provisionedBrandDomain && hasText(provisionedBrandDomain)is redundant sincehasTextalready handles falsy values - a singlehasText(x)per variable suffices -src/controllers/brands.js:1718.
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 53s | Cost: $7.75 | Commit: 8e40d4ee4c4683cb2e996228ac42b29e5f5e8434
If this code review was useful, please react with 👍. Otherwise, react with 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e-create coverage (LLMO-6405) - IT (test/it/shared/tests/brands.js): the LLMO-5870 unset+reuse test created bare brands and relied on the legacy no-anchor -> pending demotion. In a serenity-active org every create is now a Semrush create (a bare create provisions a sub-workspace and lands active), so create the two brands explicitly pending — the unset/reuse mechanic under test is unchanged. Fixes the it-postgres failure on #2878. - unit (test/controllers/brands.test.js): address the review coverage gaps — a bare sub-workspace provisioning failure surfaces and writes no brand row; ensureMarketSite is skipped on the bare path; a non-serenity org requesting generatePrompts:true (no market) is rejected 403. Retitle the pending-serenity-create test (it enters Semrush mode because the org is serenity-active, not because of the flag's presence). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @MysticatBot — addressed in d82c760, and the Must-fix (all done):
CI ( Nits:
|
aenascut
left a comment
There was a problem hiding this comment.
Hey! Checked the serenity migration skill in mysticat-data-service against this PR — no impact. Migration talks directly to Semrush APIs + Postgres, bypasses spacecat-api-service entirely, so the brand-create mode switch, provisionBrandSubworkspaceBare, and the pending-activation change don't touch it. The site_id reads in the migration use .get() so any existing NULLs are handled safely too.
Did notice the follow-up you called out — tightening active ⇒ site_id NOT NULL + backfill. We'll keep an eye on that when it lands to make sure the migration's persist step stays in sync. Thanks!
There was a problem hiding this comment.
Hey @igor-grubic,
Verdict: Approve - all prior findings addressed, tests well-structured, CI green.
Complexity: HIGH - large diff; API surface.
Changes: Reworks brand-create mode detection to use the org-level serenity flag, makes pending activation sub-workspace-only, removes the Semrush site_id skip, and adds provisionBrandSubworkspaceBare (11 files).
Note: Recommend a human read before merge - this change amends a shared contract (docs/openapi/schemas.yaml). The bot review is a complement to, not a replacement for, a human read here.
Previously flagged, now resolved
- Controller test for
provisionBrandSubworkspaceBarefailure now verifies 500 + no brand row written. - Bare-create test now asserts
ensureMarketSiteis not called (no market domain to mirror). - Non-serenity org requesting
generatePrompts:truenow covered with a 403 assertion.
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 2s | Cost: $3.37 | Commit: d82c760644408609296bcafae6c159f91b26843e
If this code review was useful, please react with 👍. Otherwise, react with 👎.
# [1.677.0](v1.676.0...v1.677.0) (2026-07-22) ### Features * **serenity:** org-flag brand-create mode, sub-workspace-only pending activation, site_id anchoring (LLMO-6405) ([#2878](#2878)) ([a9d7a4a](a9d7a4a))
|
🎉 This PR is included in version 1.677.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What
Backend for the brand & market management rework (serenity-docs #68, LLMO-6405). Market-scoped inputs move out of brand creation into market creation.
createBrandForOrgdecides Semrush mode from the org serenity flag (isSerenityActiveForOrg), not the request body. A non-serenity org that sends a market orgeneratePrompts:true→ 403; a baregeneratePrompts:falsestays a flat create.provisionBrandSubworkspaceBare; a supplied market keeps the existing project path./serenity/activateon a pending brand is sub-workspace-only (ensure sub-workspace + flip active + clear stash) — it never auto-creates a market. The body-driven market path runs only for an already-active brand (reactivation / onboarding API) and never downgrades it on a partial failure (207).upsertBrandwritesbrands.site_idfrombaseSiteIdeven on the Semrush path (anchored by both sub-workspace and primary site).Tests
Recast the
/serenity/activatesuite to the new model + added pending sub-workspace-only cases; updatedupsertBrandsite_id tests; new coverage forprovisionBrandSubworkspaceBare.npm test+ coverage green locally;type-check+eslintclean. The Postgres IT suite runs here on CI (it can't run in the dev environment).Related
Follow-ups (not in this PR)
mysticat-data-servicemigration: tightenactive ⇒ site_id NOT NULL+ backfill NULLsite_id(sequenced after this plumbing).brands-v2/serenity+docs/serenity.mdupdates.🤖 Generated with Claude Code