From 75183d08e3d8d62c1d47730a4c6059e4b3256a26 Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Wed, 9 Sep 2026 16:44:31 +0300 Subject: [PATCH 01/11] feat(serenity): opt-in async provisioning for the 3 remaining skip-mode call sites (LLMO-7352/LLMO-7418 Phase 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the opt-in `async: true` contract (PR-C, #3246) to the last 3 ensureSubworkspace call sites that were still fully synchronous: createBrandForOrg's bare-create (no semrushMarket) branch, and activate's pending->active and bare-reactivation branches. These 3 sites use ensureSubworkspace's `createReadiness: 'skip'` mode (persist the workspace pointer immediately after a single create call, without polling for settle) rather than 'poll' mode, so they never had the in-request settle-poll latency/timeout problem PR-A/PR-B/PR-C fixed for the other 2 sites. This is deliberate hardening for consistency and defense against a slow/hanging single Semrush create call, not a fix for a still-open incident — the original LLMO-7352 symptom is already closed by PR-A alone, since any workspace pointer's first real use (e.g. adding a market) always runs the now-fail-fast pollUntilCreated first. - createBrandForOrg's bare-create branch mirrors its sibling hasSemrushMarket branch exactly: async:true persists the row first, then hands off to provision-workspace-job with no chained job (no project to create). - activate's two skip-mode branches each mint a provisioning attempt and chain to a new activate-brand-workspace-job, which does the one piece of business logic the generic worker doesn't: flipping (or re-affirming) the brand's status to active once the workspace is confirmed ready. One job handler covers both branches; `wasPending` in its metadata distinguishes a real pending->active transition (502 on save failure) from a no-op reactivation re-affirm (207). - Existing guardAgainstConcurrentProvisioning calls on the sync branches are unchanged (still needed to protect a sync caller against a concurrent async attempt from another endpoint). Stacked on PR-C (#3246). 30 new unit tests; 954 passing overall, 0 regressions. Co-Authored-By: Claude Sonnet 5 --- docs/openapi/brands-v2-api.yaml | 15 +-- docs/openapi/serenity-api.yaml | 42 ++++--- src/controllers/brands.js | 98 +++++++++++++++ src/controllers/serenity.js | 70 +++++++++++ src/serenity-prompt-classification/index.js | 9 +- .../handlers/activate-brand-workspace-job.js | 80 ++++++++++++ test/controllers/brands.test.js | 81 +++++++++++- test/controllers/serenity.test.js | 116 ++++++++++++++++++ .../activate-brand-workspace-job.test.js | 107 ++++++++++++++++ 9 files changed, 589 insertions(+), 29 deletions(-) create mode 100644 src/support/serenity/handlers/activate-brand-workspace-job.js create mode 100644 test/support/serenity/handlers/activate-brand-workspace-job.test.js diff --git a/docs/openapi/brands-v2-api.yaml b/docs/openapi/brands-v2-api.yaml index 35c0664555..6b214963fb 100644 --- a/docs/openapi/brands-v2-api.yaml +++ b/docs/openapi/brands-v2-api.yaml @@ -81,13 +81,14 @@ v2-brands-for-org: $ref: './schemas.yaml#/V2Brand' '202': description: | - PR-C (LLMO-7352/LLMO-7418): the request set `async: true` alongside - `semrushMarket`. The brand row is persisted immediately (visible, + The request set `async: true` (PR-C, LLMO-7352/LLMO-7418, alongside + `semrushMarket`; or Phase 4, same epic, on a bare create with no + market). The brand row is persisted immediately (visible, non-active — "Setting up", no workspace pointer yet); the - sub-workspace ready-check and the initial market's project create/ - publish are enqueued to a background job chain instead of running - inline. Poll the returned `jobId` at `GET .../serenity/jobs/{jobId}` - for status + result. + sub-workspace ready-check — and, when `semrushMarket` was supplied, + the initial market's project create/publish — are enqueued to a + background job chain instead of running inline. Poll the returned + `jobId` at `GET .../serenity/jobs/{jobId}` for status + result. content: application/json: schema: @@ -117,7 +118,7 @@ v2-brands-for-org: organization (`code: brand_site_org_mismatch`). - The brand's primary URL is already the primary URL (base site) of another brand in this organization. - - (PR-C, `async: true` only) a provisioning attempt is already in flight + - (`async: true` only) a provisioning attempt is already in flight for this brand (`error: semrushProvisioningInProgress`). headers: X-Error: diff --git a/docs/openapi/serenity-api.yaml b/docs/openapi/serenity-api.yaml index 4e32c47463..727334309a 100644 --- a/docs/openapi/serenity-api.yaml +++ b/docs/openapi/serenity-api.yaml @@ -1203,12 +1203,12 @@ v2-serenity-activate: intact; retry converges idempotently). An already-active brand re-supplying markets is never downgraded (a partial failure is reported as 207). - PR-C (LLMO-7352/LLMO-7418): when a primary URL/domain and markets are - present (the branch described above — NOT the sub-workspace-only pending - or bare-reactivation paths, which always stay synchronous), the caller may - set `async: true` to enqueue the whole batch to a background job chain - instead of running it inline; see the `202` response. Absent/false runs - synchronously as described above — this is opt-in only and not yet the + LLMO-7352/LLMO-7418: the caller may set `async: true` on ANY of the 3 + branches — project activation (PR-C), the sub-workspace-only + pending→active flip, and bare reactivation (both Phase 4) — to enqueue + that branch's work to a background job chain instead of running it + inline; see the `202` response. Absent/false runs synchronously as + described above on every branch — this is opt-in only and not yet the default. operationId: activateSerenityBrand security: @@ -1226,17 +1226,20 @@ v2-serenity-activate: schema: { $ref: './schemas.yaml#/SerenityActivateResponse' } '202': description: | - PR-C (LLMO-7352/LLMO-7418): the request set `async: true` on the - project-activation branch (primary URL + markets present). The whole - batch (sub-workspace ready-check, every market's project create/ - publish, the site link, and the active flip) is enqueued to a - background job chain instead of running inline. Poll the returned - `jobId` at `GET .../serenity/jobs/{jobId}` for status + result — the - polling endpoint transparently follows the job chain to its - EFFECTIVE terminal hop, so `status` only reports `COMPLETED` once - the actual batch has run, not merely once the sub-workspace became - ready; on COMPLETED, `result` is the same body a synchronous - `200`/`207` would have returned. + The request set `async: true`. On the project-activation branch + (primary URL + markets present — PR-C), the whole batch + (sub-workspace ready-check, every market's project create/publish, + the site link, and the active flip) is enqueued to a background job + chain. On the sub-workspace-only pending→active or bare-reactivation + branches (Phase 4), only the sub-workspace ready-check + the status + flip are enqueued — there is no project/market work on those + branches. Either way, poll the returned `jobId` at + `GET .../serenity/jobs/{jobId}` for status + result — the polling + endpoint transparently follows the job chain to its EFFECTIVE + terminal hop, so `status` only reports `COMPLETED` once the actual + batch (or status flip) has run, not merely once the sub-workspace + became ready; on COMPLETED, `result` is the same body the branch's + own synchronous `200`/`207`/`502` would have returned. content: application/json: schema: { $ref: './schemas.yaml#/SerenityPromptsJobAccepted' } @@ -1258,8 +1261,9 @@ v2-serenity-activate: outcomes — the operator must choose a different primary URL; (b) misconfiguration: the brand's sub-workspace equals the organization parent workspace (refused to avoid acting on the shared parent pool); or (c) - (PR-C, `async: true` only) a provisioning attempt is already in flight for - this brand (`error: semrush_provisioning_in_progress`) — retry shortly. + (`async: true` only, any branch) a provisioning attempt is already in + flight for this brand (`error: semrush_provisioning_in_progress`) — + retry shortly. content: application/json: schema: diff --git a/src/controllers/brands.js b/src/controllers/brands.js index ef5d68acee..3c39c0be48 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -1729,6 +1729,12 @@ function BrandsController(ctx, log, env) { // hoisted so the catch can mark that persisted-but-not-yet-provisioning row visibly failed if // starting the attempt itself throws (see the catch's compensation). let asyncMarketProvisioning = null; + // Phase 4 (LLMO-7352/LLMO-7418): the bare-create (no semrushMarket) sibling of + // asyncMarketProvisioning above — same "non-null after the row is persisted means start an + // attempt" contract, just with no chained job (a bare sub-workspace has no project to create + // afterward). Mutually exclusive with asyncMarketProvisioning (they're set in sibling + // if/else branches of the same hasSemrushMarket check). + let asyncBareProvisioning = null; try { if (!hasText(spaceCatId)) { @@ -1928,6 +1934,28 @@ function BrandsController(ctx, log, env) { languageCode: provisioned.languageCode, }; } + } else if (validateAsync(brandData)) { + // B (LLMO-6405): sub-workspace-only active create — no market supplied, so + // no project is provisioned. Markets are added afterwards from the Markets + // tab. The brand is anchored by its primary site (baseSiteId, persisted by + // upsertBrand below) AND by its Semrush sub-workspace. + // + // Phase 4 (LLMO-7352/LLMO-7418): opt-in only (mirrors the hasSemrushMarket branch's own + // `async` flag above). Absent/false runs the EXACT bespoke synchronous + // provisionBrandSubworkspaceBare call this branch has always run. `async: true` persists + // the brand row FIRST (visible, active, no workspace pointer yet), then hands the bare + // sub-workspace provisioning off to provision-workspace-job — no chained job, since a + // bare create has no project to create once the workspace is ready. + // + // Resolved and validated HERE, before any write — same rationale as the + // hasSemrushMarket branch: a missing org workspace config must never leave a + // persisted, permanently-inert brand row behind. + const parentWorkspaceId = await resolveWorkspaceId(context, spaceCatId); + if (!parentWorkspaceId || !hasText(parentWorkspaceId)) { + return badRequest('Organization has no Semrush workspace configured'); + } + provisionedBrandId = randomUUID(); + asyncBareProvisioning = { parentWorkspaceId }; } else { // B (LLMO-6405): sub-workspace-only active create — no market supplied, so // no project is provisioned. Markets are added afterwards from the Markets @@ -2126,6 +2154,76 @@ function BrandsController(ctx, log, env) { } } + // Phase 4 (LLMO-7352/LLMO-7418): bare-create's async sibling of the block above — same + // shape, no chained job (no project to create once the workspace is ready). + if (asyncBareProvisioning) { + const asyncBrandId = /** @type {string} */ (provisionedBrandId); + const { parentWorkspaceId } = asyncBareProvisioning; + const attemptId = randomUUID(); + let began; + try { + began = await beginProvisioningAttempt({ + brandId: asyncBrandId, + attemptId, + postgrestClient, + updatedBy, + }); + } catch (beginError) { + log.error('brands: failed to begin the provisioning attempt after brand row was persisted', { + brandId: asyncBrandId, error: beginError?.message, + }); + await recordFreshBrandProvisioningStartFailure({ + brandId: asyncBrandId, + error: 'Failed to start Semrush provisioning', + postgrestClient, + }).catch((failError) => { + log.error('brands: failed to record the provisioning-start failure itself', { + brandId: asyncBrandId, error: failError?.message, + }); + }); + throw beginError; + } + if (!began) { + return createResponse( + { + error: 'semrushProvisioningInProgress', + message: 'Unable to start Semrush provisioning for the new brand', + }, + 409, + ); + } + try { + const job = await createAndEnqueueJob(context, { + jobType: PROVISION_WORKSPACE_JOB_TYPE, + metadata: { + brandId: asyncBrandId, + attemptId, + parentWorkspaceId, + title: brandData.name, + }, + }); + return createResponse( + { ...withSerenityState(created, serenityScopes), status: 'pending', jobId: job.getId() }, + 202, + ); + } catch (enqueueError) { + log.error('brands: failed to start async Semrush provisioning after brand row was persisted', { + brandId: asyncBrandId, error: enqueueError?.message, + }); + await promoteProvisioningFailed({ + brandId: asyncBrandId, + attemptId, + error: 'Failed to start Semrush provisioning', + postgrestClient, + }).catch((failError) => { + log.error('brands: failed to record the provisioning-start failure itself', { + brandId: asyncBrandId, error: failError?.message, + }); + }); + throw enqueueError; + } + } + return createResponse(withSerenityState(created, serenityScopes), 201); } catch (error) { if (error.code === 'brand_status_demotion_not_allowed') { diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index d902fef896..babf3d574d 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -47,6 +47,7 @@ import { CLASSIFY_PROMPTS_JOB_TYPE } from '../support/serenity/handlers/classify import { PROVISION_WORKSPACE_JOB_TYPE } from '../support/serenity/handlers/provision-workspace-job.js'; import { CREATE_MARKET_JOB_TYPE } from '../support/serenity/handlers/create-market-job.js'; import { ACTIVATE_MARKETS_JOB_TYPE } from '../support/serenity/handlers/activate-markets-job.js'; +import { ACTIVATE_BRAND_WORKSPACE_JOB_TYPE } from '../support/serenity/handlers/activate-brand-workspace-job.js'; import { ORIGIN_VALUE } from '../support/serenity/prompt-tags.js'; import { BULK_TAGS_JOB_TYPE, @@ -1672,6 +1673,41 @@ function SerenityController(context, log, env) { if (!existingSiteId) { throw new ErrorWithStatusCode(`Brand has no onboarded primary site: ${brandUuid}`, 400); } + // Phase 4 (LLMO-7352/LLMO-7418): opt-in only (mirrors this endpoint's own + // project-activation branch below, and createMarket's/createBrandForOrg's `async` flag). + // Absent/false runs the EXACT synchronous pending->active flip this branch has always + // run. `async: true` hands the sub-workspace-ensure + status flip off to the + // `provision-workspace-job` -> + // `serenity-activate-brand-workspace` job chain instead. + if (validateAsync(body)) { + const attemptId = randomUUID(); + const began = await beginProvisioningAttempt({ + brandId: brandUuid, + attemptId, + postgrestClient: ctx.dataAccess.services.postgrestClient, + updatedBy: 'serenity-activate', + }); + if (!began) { + const err = new ErrorWithStatusCode( + 'A Semrush sub-workspace provisioning attempt is already in progress for this ' + + 'brand; please retry shortly.', + 409, + ); + err.code = 'semrush_provisioning_in_progress'; + throw err; + } + const job = await createAndEnqueueJob(ctx, { + jobType: PROVISION_WORKSPACE_JOB_TYPE, + metadata: { + brandId: brandUuid, + attemptId, + parentWorkspaceId: auth.parentWorkspaceId ?? '', + chainedJobType: ACTIVATE_BRAND_WORKSPACE_JOB_TYPE, + chainedJobMetadata: { brandId: brandUuid, wasPending: true }, + }, + }); + return accepted({ jobId: job.getId(), status: job.getStatus() }); + } // PR-C guard (LLMO-7352/LLMO-7418): this branch stays synchronous, but a market-creating // endpoint may have an async provisioning attempt in flight for this SAME brand — without // this check, ensureSubworkspace below could independently create a second workspace. @@ -1749,6 +1785,40 @@ function SerenityController(context, log, env) { if (generatePrompts) { throw new ErrorWithStatusCode('A primary URL is required to generate prompts', 400); } + // Phase 4 (LLMO-7352/LLMO-7418): opt-in only — see the wasPending branch above for the + // full rationale. `wasPending: false` in the chained metadata distinguishes this + // already-active no-op re-affirm from a real pending->active transition, so + // activate-brand-workspace-job.js's save-divergence handling matches this branch's own + // 207-not-502 contract. + if (validateAsync(body)) { + const attemptId = randomUUID(); + const began = await beginProvisioningAttempt({ + brandId: brandUuid, + attemptId, + postgrestClient: ctx.dataAccess.services.postgrestClient, + updatedBy: 'serenity-activate', + }); + if (!began) { + const err = new ErrorWithStatusCode( + 'A Semrush sub-workspace provisioning attempt is already in progress for this ' + + 'brand; please retry shortly.', + 409, + ); + err.code = 'semrush_provisioning_in_progress'; + throw err; + } + const job = await createAndEnqueueJob(ctx, { + jobType: PROVISION_WORKSPACE_JOB_TYPE, + metadata: { + brandId: brandUuid, + attemptId, + parentWorkspaceId: auth.parentWorkspaceId ?? '', + chainedJobType: ACTIVATE_BRAND_WORKSPACE_JOB_TYPE, + chainedJobMetadata: { brandId: brandUuid, wasPending: false }, + }, + }); + return accepted({ jobId: job.getId(), status: job.getStatus() }); + } // PR-C guard (LLMO-7352/LLMO-7418): see the wasPending branch above for rationale. await guardAgainstConcurrentProvisioning( brandUuid, diff --git a/src/serenity-prompt-classification/index.js b/src/serenity-prompt-classification/index.js index 6354dc65e9..40b069b6b1 100644 --- a/src/serenity-prompt-classification/index.js +++ b/src/serenity-prompt-classification/index.js @@ -47,6 +47,10 @@ import { activateMarketsJobHandler, ACTIVATE_MARKETS_JOB_TYPE, } from '../support/serenity/handlers/activate-markets-job.js'; +import { + activateBrandWorkspaceJobHandler, + ACTIVATE_BRAND_WORKSPACE_JOB_TYPE, +} from '../support/serenity/handlers/activate-brand-workspace-job.js'; import { isRateLimited, isSemrushTransportError, @@ -119,7 +123,9 @@ export const vaultOpts = { * sub-workspace provisioning lives in `../support/serenity/handlers/provision-workspace-job.js`; * and PR-C's two chained market-creation phases (workspace ready -> create the market/run the * activate batch against it) live in `../support/serenity/handlers/create-market-job.js` and - * `../support/serenity/handlers/activate-markets-job.js`. All five are registered below. + * `../support/serenity/handlers/activate-markets-job.js`; Phase 4's brand-status-flip half of + * `activate`'s two remaining "skip"-mode branches lives in + * `../support/serenity/handlers/activate-brand-workspace-job.js`. All six are registered below. * * @type {Record Promise>} @@ -130,6 +136,7 @@ const HANDLERS = { [PROVISION_WORKSPACE_JOB_TYPE]: provisionWorkspaceHandler, [CREATE_MARKET_JOB_TYPE]: createMarketJobHandler, [ACTIVATE_MARKETS_JOB_TYPE]: activateMarketsJobHandler, + [ACTIVATE_BRAND_WORKSPACE_JOB_TYPE]: activateBrandWorkspaceJobHandler, }; const TRANSIENT_NETWORK_ERROR_CODES = new Set([ diff --git a/src/support/serenity/handlers/activate-brand-workspace-job.js b/src/support/serenity/handlers/activate-brand-workspace-job.js new file mode 100644 index 0000000000..72c016a4ff --- /dev/null +++ b/src/support/serenity/handlers/activate-brand-workspace-job.js @@ -0,0 +1,80 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +// @ts-check + +/** + * Job type dispatched to {@link activateBrandWorkspaceJobHandler} by the runner + * (`src/serenity-prompt-classification/index.js`). Phase 4 (LLMO-7352/LLMO-7418): the second + * phase of `activate`'s two "skip"-mode branches' async flow (pending→active, and bare + * reactivation of an already-active brand) — enqueued by `provision-workspace-job.js` as its + * `chainedJobType` once the sub-workspace it depends on is confirmed `ready`, never enqueued + * directly by an HTTP controller. Unlike `serenity-create-market`/`serenity-activate-markets`, + * this job creates no Semrush project — its only job is the brand's own status flip, the one + * piece of business logic the generic provisioning worker doesn't do. + */ +export const ACTIVATE_BRAND_WORKSPACE_JOB_TYPE = 'serenity-activate-brand-workspace'; + +/** + * Flips (or re-affirms) the brand's status to `active` now that its sub-workspace is confirmed + * ready — the deferred half of what the synchronous `wasPending`/bare-reactivation branches in + * `serenity.js` do inline. Shared by both call sites: `wasPending` (a real pending→active + * transition) is distinguished from bare reactivation (already active, a no-op re-affirm) only + * by `wasPending` in this job's own metadata, so the result body mirrors each sync branch's own + * response shape exactly (a save failure is a retryable 502 for a real transition, but only a + * 207 for a reactivation that was already active regardless). + * + * @param {object} context - worker context (`dataAccess`, `log`). + * @param {object} job - the current `AsyncJob`. `job.getMetadata()` carries + * `{ brandId, wasPending? }` (the chain-enqueue's `chainedJobMetadata` — no `workspaceId` is + * needed here, unlike the market-creating chained jobs, since this job never talks to Semrush). + * @returns {Promise} the same `{ status, body }` shape the synchronous branches + * themselves return — surfaced verbatim as this AsyncJob's `result`. + */ +export async function activateBrandWorkspaceJobHandler(context, job) { + const { dataAccess, log } = context; + const metadata = job.getMetadata() ?? {}; + const { brandId, wasPending = false } = metadata; + + const brand = await dataAccess.Brand.findById(brandId); + let succeeded = true; + if (typeof brand.setStatus === 'function') { + brand.setStatus('active'); + } + try { + await brand.save(); + } catch (saveError) { + succeeded = false; + log?.error?.('serenity activate (async): SERENITY_ACTIVATE_SAVE_DIVERGENCE — sub-workspace ensured, but failed to persist active status', { + brandId, wasPending, error: saveError?.message, + }); + } + + if (succeeded) { + return { status: 200, body: { brandId, status: 'active', markets: [] } }; + } + // Mirrors the sync branches' own divergence handling: a genuine pending→active transition + // that failed to persist stays pending (retryable, 502); an already-active brand's no-op + // re-affirm failing is not a real regression (207, still `active`). + return wasPending + ? { + status: 502, + body: { + brandId, + status: 'pending', + error: 'serenityActivationIncomplete', + message: 'Sub-workspace provisioned but the active status could not be persisted.', + markets: [], + }, + } + : { status: 207, body: { brandId, status: 'active', markets: [] } }; +} diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index c7ac70bcd5..e0aa97ce42 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -5743,8 +5743,10 @@ describe('Brands Controller', () => { it('creates a bare sub-workspace (no project) when no market is supplied (serenity-active)', async () => { // LLMO-6405: market-scoped inputs moved to market creation, so a serenity-active - // create with no market provisions just the sub-workspace (no project), synchronously - // (unconverted, out of PR-C's scope) — the async job chain is never started. + // create with no market provisions just the sub-workspace (no project). Phase 4 + // (LLMO-7352/LLMO-7418) converted this branch too, but async is absent/false here, so + // this stays the EXACT synchronous provisionBrandSubworkspaceBare call — see the + // dedicated async tests below for the `async: true` job-chain path. const bareStub = sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }); const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); @@ -5771,6 +5773,81 @@ describe('Brands Controller', () => { expect(upsertArgs.brand.baseSiteId).to.equal('site-123'); }); + it('Phase 4: bare create mints a provisioning attempt and enqueues provision-workspace-job with NO chained job when async: true', async () => { + const bareStub = sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }); + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const controller = await buildController({ + provisionBrandSubworkspaceBare: bareStub, + upsertBrand: upsertStub, + createAndEnqueueJob: enqueueStub, + }); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(202); + const body = await response.json(); + expect(body.status).to.equal('pending'); + expect(body.jobId).to.equal('job-xyz'); + // The bare-sync provisioner never runs on the async path. + expect(bareStub.called).to.equal(false); + expect(enqueueStub.calledOnce).to.equal(true); + const [, enqueueArgs] = enqueueStub.firstCall.args; + expect(enqueueArgs.jobType).to.equal('serenity-provision-workspace'); + expect(enqueueArgs.metadata.parentWorkspaceId).to.equal('parent-ws-1'); + expect(enqueueArgs.metadata.chainedJobType).to.equal(undefined); + expect(enqueueArgs.metadata.chainedJobMetadata).to.equal(undefined); + // The row is persisted with no workspace pointer yet. + const upsertArgs = upsertStub.firstCall.args[0]; + expect(upsertArgs.semrushSubWorkspaceId).to.equal(null); + }); + + it('Phase 4: bare create returns 409 without enqueuing when a provisioning attempt is already in flight', async () => { + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const beginStub = sinon.stub().resolves(false); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const controller = await buildController({ + upsertBrand: upsertStub, + beginProvisioningAttempt: beginStub, + createAndEnqueueJob: enqueueStub, + }); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(409); + expect(enqueueStub.called).to.equal(false); + }); + + it('Phase 4: bare create returns 400 without persisting a row when the organization has no Semrush parent workspace configured', async () => { + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const controller = await buildController({ + upsertBrand: upsertStub, resolveWorkspaceId: sinon.stub().resolves(null), + }); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(400); + expect(upsertStub.called).to.equal(false); + }); + it('surfaces a bare sub-workspace provisioning failure and does not write the brand', async () => { // The bare sub-workspace is provisioned BEFORE the brand row is written; if // that upstream call throws, the create surfaces the error and never persists diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index cc3b921ada..cf501fa61c 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -421,6 +421,9 @@ describe('SerenityController', () => { '../../src/support/serenity/handlers/activate-markets-job.js': { ACTIVATE_MARKETS_JOB_TYPE: 'serenity-activate-markets', }, + '../../src/support/serenity/handlers/activate-brand-workspace-job.js': { + ACTIVATE_BRAND_WORKSPACE_JOB_TYPE: 'serenity-activate-brand-workspace', + }, })).default; }); @@ -2454,6 +2457,119 @@ describe('SerenityController', () => { expect(ensureSubworkspaceStub).to.not.have.been.called; }); + it('Phase 4: pending→active activation mints a provisioning attempt and enqueues the provision->activate-brand-workspace job chain when async: true', async () => { + getBrandBaseSiteIdStub.resolves('primary-site'); + const brand = makeBrandModel({}); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: true }, + })); + + expect(response.status).to.equal(202); + expect(ensureSubworkspaceStub).to.not.have.been.called; + expect(guardAgainstConcurrentProvisioningStub).to.not.have.been.called; + expect(brand.setStatus).to.not.have.been.called; + expect(beginProvisioningAttemptStub).to.have.been.calledOnce; + expect(beginProvisioningAttemptStub.firstCall.args[0]).to.include({ + brandId: BRAND, updatedBy: 'serenity-activate', + }); + expect(createAndEnqueueJobStub).to.have.been.calledOnce; + const [, enqueueArgs] = createAndEnqueueJobStub.firstCall.args; + expect(enqueueArgs.jobType).to.equal('serenity-provision-workspace'); + expect(enqueueArgs.metadata.chainedJobType).to.equal('serenity-activate-brand-workspace'); + expect(enqueueArgs.metadata.chainedJobMetadata) + .to.deep.equal({ brandId: BRAND, wasPending: true }); + }); + + it('Phase 4: pending→active activation answers 409 without enqueuing when async: true and a provisioning attempt is already in flight', async () => { + getBrandBaseSiteIdStub.resolves('primary-site'); + beginProvisioningAttemptStub.resolves(false); + const brand = makeBrandModel({}); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: true }, + })); + expect(response.status).to.equal(409); + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + + it('Phase 4: pending→active activation 400s when async is present but not a boolean', async () => { + getBrandBaseSiteIdStub.resolves('primary-site'); + const brand = makeBrandModel({}); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: 'yes' }, + })); + expect(response.status).to.equal(400); + expect(createAndEnqueueJobStub).to.not.have.been.called; + expect(ensureSubworkspaceStub).to.not.have.been.called; + }); + + it('Phase 4: pending→active activation runs the same synchronous flip as an absent flag when async is explicitly false', async () => { + getBrandBaseSiteIdStub.resolves('primary-site'); + handlers.handleCreateMarketSubworkspace.resolves({ status: 201, body: {} }); + const brand = makeBrandModel({}); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: false }, + })); + expect(response.status).to.equal(200); + expect(guardAgainstConcurrentProvisioningStub).to.have.been.calledOnce; + expect(ensureSubworkspaceStub).to.have.been.calledOnce; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + + it('Phase 4: bare reactivation mints a provisioning attempt and enqueues the provision->activate-brand-workspace job chain when async: true', async () => { + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: true }, + })); + + expect(response.status).to.equal(202); + expect(ensureSubworkspaceStub).to.not.have.been.called; + expect(guardAgainstConcurrentProvisioningStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.have.been.calledOnce; + const [, enqueueArgs] = createAndEnqueueJobStub.firstCall.args; + expect(enqueueArgs.metadata.chainedJobType).to.equal('serenity-activate-brand-workspace'); + expect(enqueueArgs.metadata.chainedJobMetadata) + .to.deep.equal({ brandId: BRAND, wasPending: false }); + }); + + it('Phase 4: bare reactivation answers 409 without enqueuing when async: true and a provisioning attempt is already in flight', async () => { + beginProvisioningAttemptStub.resolves(false); + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: true }, + })); + expect(response.status).to.equal(409); + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + + it('Phase 4: bare reactivation 400s when async is present but not a boolean', async () => { + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: 'yes' }, + })); + expect(response.status).to.equal(400); + expect(createAndEnqueueJobStub).to.not.have.been.called; + expect(ensureSubworkspaceStub).to.not.have.been.called; + }); + + it('Phase 4: bare reactivation runs the same synchronous flip as an absent flag when async is explicitly false', async () => { + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: false }, + })); + expect(response.status).to.equal(200); + expect(guardAgainstConcurrentProvisioningStub).to.have.been.calledOnce; + expect(ensureSubworkspaceStub).to.have.been.calledOnce; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('activate 400s when the markets array exceeds the cap (validated before either branch dispatches)', async () => { const markets = Array.from({ length: 51 }, (_, i) => ({ market: 'us', languageCode: `l${i}` })); const controller = SerenityController({ env: {} }, fakeLog(), {}); diff --git a/test/support/serenity/handlers/activate-brand-workspace-job.test.js b/test/support/serenity/handlers/activate-brand-workspace-job.test.js new file mode 100644 index 0000000000..14c11e642c --- /dev/null +++ b/test/support/serenity/handlers/activate-brand-workspace-job.test.js @@ -0,0 +1,107 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { expect } from 'chai'; +import sinon from 'sinon'; +import { activateBrandWorkspaceJobHandler, ACTIVATE_BRAND_WORKSPACE_JOB_TYPE } from '../../../../src/support/serenity/handlers/activate-brand-workspace-job.js'; + +const BRAND_ID = 'brand-1'; + +function fakeLog() { + return { + info: sinon.stub(), warn: sinon.stub(), error: sinon.stub(), debug: sinon.stub(), + }; +} + +function makeJob(metadata) { + return { getId: () => 'job-1', getMetadata: () => metadata }; +} + +function makeBrand({ saveError } = {}) { + return { + setStatus: sinon.stub(), + save: saveError ? sinon.stub().rejects(saveError) : sinon.stub().resolves(), + }; +} + +describe('handlers/activate-brand-workspace-job.js (Phase 4, LLMO-7352/LLMO-7418)', () => { + let context; + let brand; + let findByIdStub; + + beforeEach(() => { + brand = makeBrand(); + findByIdStub = sinon.stub().resolves(brand); + context = { + log: fakeLog(), + dataAccess: { Brand: { findById: findByIdStub } }, + }; + }); + + it('exports the expected job type constant', () => { + expect(ACTIVATE_BRAND_WORKSPACE_JOB_TYPE).to.equal('serenity-activate-brand-workspace'); + }); + + it('flips the brand to active and returns 200 on a successful save', async () => { + const job = makeJob({ brandId: BRAND_ID, wasPending: true }); + + const result = await activateBrandWorkspaceJobHandler(context, job); + + expect(findByIdStub).to.have.been.calledOnceWith(BRAND_ID); + expect(brand.setStatus).to.have.been.calledOnceWith('active'); + expect(brand.save).to.have.been.calledOnce; + expect(result).to.deep.equal({ status: 200, body: { brandId: BRAND_ID, status: 'active', markets: [] } }); + }); + + it('returns 502 with serenityActivationIncomplete when a real pending->active transition fails to save', async () => { + const saveError = new Error('write conflict'); + brand = makeBrand({ saveError }); + findByIdStub.resolves(brand); + const job = makeJob({ brandId: BRAND_ID, wasPending: true }); + + const result = await activateBrandWorkspaceJobHandler(context, job); + + expect(result).to.deep.equal({ + status: 502, + body: { + brandId: BRAND_ID, + status: 'pending', + error: 'serenityActivationIncomplete', + message: 'Sub-workspace provisioned but the active status could not be persisted.', + markets: [], + }, + }); + expect(context.log.error).to.have.been.calledOnce; + }); + + it('returns 207 (still active) when a bare-reactivation no-op re-affirm fails to save', async () => { + const saveError = new Error('transient write failure'); + brand = makeBrand({ saveError }); + findByIdStub.resolves(brand); + const job = makeJob({ brandId: BRAND_ID, wasPending: false }); + + const result = await activateBrandWorkspaceJobHandler(context, job); + + expect(result).to.deep.equal({ status: 207, body: { brandId: BRAND_ID, status: 'active', markets: [] } }); + }); + + it('defaults wasPending to false (207, not 502) when absent from metadata', async () => { + const saveError = new Error('transient write failure'); + brand = makeBrand({ saveError }); + findByIdStub.resolves(brand); + const job = makeJob({ brandId: BRAND_ID }); + + const result = await activateBrandWorkspaceJobHandler(context, job); + + expect(result.status).to.equal(207); + }); +}); From 837001d4f6ddadb7393e9bb777bc7b3cfd074285 Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Wed, 9 Sep 2026 17:46:01 +0300 Subject: [PATCH 02/11] fix(serenity): remove the unguarded, redundant status-write in activate-brand-workspace-job (LLMO-7352/LLMO-7418) Adversarial review (4-agent pass over Phase 4 + the read-field PR + the frontend UI) found a real race: promoteProvisioningReady already durably flips the brand to 'active' as part of the SAME atomic CAS write that promotes semrush_provisioning_status to 'ready' (provision-workspace-job.js only enqueues the chain once that write succeeds -- confirmed by reading the call site directly). This job's own brand.setStatus('active')+save() was therefore both redundant on success AND actively dangerous: if the chain runs minutes after enqueue (self-requeue backoff, plain SQS latency) and a legitimate concurrent POST /serenity/deactivate has since moved the brand back to pending in between, this job would blindly re-activate it -- exactly the "active brand bound to a workspace nobody meant it to have" failure class the whole epic exists to prevent, just reached through a new door. Fix: the handler is now READ-ONLY. It reports 200/active when the brand is (still) active (the common case), and otherwise reports the brand's ACTUAL current status via 207 rather than manufacturing a 502 "incomplete" -- the sub-workspace itself did provision successfully; only the brand's separate active/pending state changed underneath the chain, which is not an error. Also adds the missing async compensation-path tests for the bare-create branch (beginProvisioningAttempt/enqueue throwing) mirroring its sibling hasSemrushMarket branch's existing coverage, per the test-coverage review's finding. Co-Authored-By: Claude Sonnet 5 --- .../handlers/activate-brand-workspace-job.js | 69 +++++++-------- test/controllers/brands.test.js | 73 ++++++++++++++++ .../activate-brand-workspace-job.test.js | 83 +++++++++++-------- 3 files changed, 150 insertions(+), 75 deletions(-) diff --git a/src/support/serenity/handlers/activate-brand-workspace-job.js b/src/support/serenity/handlers/activate-brand-workspace-job.js index 72c016a4ff..c598bd757d 100644 --- a/src/support/serenity/handlers/activate-brand-workspace-job.js +++ b/src/support/serenity/handlers/activate-brand-workspace-job.js @@ -18,27 +18,37 @@ * phase of `activate`'s two "skip"-mode branches' async flow (pending→active, and bare * reactivation of an already-active brand) — enqueued by `provision-workspace-job.js` as its * `chainedJobType` once the sub-workspace it depends on is confirmed `ready`, never enqueued - * directly by an HTTP controller. Unlike `serenity-create-market`/`serenity-activate-markets`, - * this job creates no Semrush project — its only job is the brand's own status flip, the one - * piece of business logic the generic provisioning worker doesn't do. + * directly by an HTTP controller. */ export const ACTIVATE_BRAND_WORKSPACE_JOB_TYPE = 'serenity-activate-brand-workspace'; /** - * Flips (or re-affirms) the brand's status to `active` now that its sub-workspace is confirmed - * ready — the deferred half of what the synchronous `wasPending`/bare-reactivation branches in - * `serenity.js` do inline. Shared by both call sites: `wasPending` (a real pending→active - * transition) is distinguished from bare reactivation (already active, a no-op re-affirm) only - * by `wasPending` in this job's own metadata, so the result body mirrors each sync branch's own - * response shape exactly (a save failure is a retryable 502 for a real transition, but only a - * 207 for a reactivation that was already active regardless). + * Reports the brand's current status now that its sub-workspace is confirmed ready — READ-ONLY, + * never writes. + * + * `promoteProvisioningReady` (the hop immediately before this chain is enqueued — + * `provision-workspace-job.js`) already durably flips the brand to `active` as part of the SAME + * atomic compare-and-set write that promotes `semrush_provisioning_status` to `ready`; the chain + * is only ever enqueued once that write has succeeded. So by the time this job runs, the brand + * is ALREADY `active` in the common case — this handler's own job is not to repeat that write + * (an earlier version of this file did, via `brand.setStatus('active'); brand.save()`, which was + * both redundant on success AND unguarded: a chained job can run minutes after its enqueue + * (self-requeue backoff, plain SQS delivery latency), and blindly re-writing `active` with no + * CAS/re-check would silently RESURRECT a brand a legitimate concurrent `/serenity/deactivate` + * had already moved back to `pending` in the meantime — exactly the "active brand bound to a + * workspace nobody meant it to have" failure class this whole epic exists to prevent, just + * reached through a different door. Fixed in review before merge.) * * @param {object} context - worker context (`dataAccess`, `log`). * @param {object} job - the current `AsyncJob`. `job.getMetadata()` carries * `{ brandId, wasPending? }` (the chain-enqueue's `chainedJobMetadata` — no `workspaceId` is * needed here, unlike the market-creating chained jobs, since this job never talks to Semrush). - * @returns {Promise} the same `{ status, body }` shape the synchronous branches - * themselves return — surfaced verbatim as this AsyncJob's `result`. + * @returns {Promise} `{ status: 200, body: {...} }` when the brand is (still) active — + * the common case; `{ status: 207, body: {...} }` reporting whatever the brand's CURRENT + * status actually is when it is no longer active (a legitimate concurrent deactivate raced + * ahead of this chain) — never a manufactured 502 "incomplete", since the sub-workspace itself + * provisioned successfully; only the brand's separate, later-changed active/pending state + * diverged from what this chain expected to find. */ export async function activateBrandWorkspaceJobHandler(context, job) { const { dataAccess, log } = context; @@ -46,35 +56,14 @@ export async function activateBrandWorkspaceJobHandler(context, job) { const { brandId, wasPending = false } = metadata; const brand = await dataAccess.Brand.findById(brandId); - let succeeded = true; - if (typeof brand.setStatus === 'function') { - brand.setStatus('active'); - } - try { - await brand.save(); - } catch (saveError) { - succeeded = false; - log?.error?.('serenity activate (async): SERENITY_ACTIVATE_SAVE_DIVERGENCE — sub-workspace ensured, but failed to persist active status', { - brandId, wasPending, error: saveError?.message, - }); - } + const currentStatus = brand.getStatus?.(); - if (succeeded) { + if (currentStatus === 'active') { return { status: 200, body: { brandId, status: 'active', markets: [] } }; } - // Mirrors the sync branches' own divergence handling: a genuine pending→active transition - // that failed to persist stays pending (retryable, 502); an already-active brand's no-op - // re-affirm failing is not a real regression (207, still `active`). - return wasPending - ? { - status: 502, - body: { - brandId, - status: 'pending', - error: 'serenityActivationIncomplete', - message: 'Sub-workspace provisioned but the active status could not be persisted.', - markets: [], - }, - } - : { status: 207, body: { brandId, status: 'active', markets: [] } }; + + log?.info?.('activate-brand-workspace-job: brand is no longer active by the time this chain ran; reporting its current state instead of overwriting it', { + brandId, wasPending, currentStatus, + }); + return { status: 207, body: { brandId, status: currentStatus, markets: [] } }; } diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index e0aa97ce42..c6f44a3088 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -5848,6 +5848,79 @@ describe('Brands Controller', () => { expect(upsertStub.called).to.equal(false); }); + it('Phase 4: bare create records the provisioning-start failure and rethrows when the job enqueue itself fails', async () => { + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const enqueueStub = sinon.stub().rejects(new Error('SQS unavailable')); + const promoteFailedStub = sinon.stub().resolves(true); + const controller = await buildController({ + upsertBrand: upsertStub, + createAndEnqueueJob: enqueueStub, + promoteProvisioningFailed: promoteFailedStub, + }); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + // The row IS already persisted (visible, non-active) — marked failed rather than + // left silently inert — but the request itself surfaces the original error. + expect(response.status).to.equal(500); + expect(promoteFailedStub.calledOnce).to.equal(true); + expect(promoteFailedStub.firstCall.args[0].brandId) + .to.equal(upsertStub.firstCall.args[0].forceBrandId); + }); + + it('Phase 4: bare create records the provisioning-start failure via the fresh-brand path and rethrows when beginProvisioningAttempt itself fails', async () => { + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const beginStub = sinon.stub().rejects(new Error('DB unavailable')); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const promoteFailedStub = sinon.stub().resolves(true); + const recordFreshFailureStub = sinon.stub().resolves(true); + const Mocked = await esmock('../../src/controllers/brands.js', { + '../../src/support/serenity/brand-provisioning.js': { + provisionBrandSubworkspaceBare: sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }), + }, + '../../src/support/serenity/serenity-active.js': { isSerenityActiveForOrg: sinon.stub().resolves(true) }, + '../../src/support/serenity/workspace-resolver.js': { resolveWorkspaceId: sinon.stub().resolves('parent-ws-1') }, + '../../src/support/serenity/async-job-runner.js': { createAndEnqueueJob: enqueueStub }, + '../../src/support/serenity/handlers/provision-workspace-job.js': { + PROVISION_WORKSPACE_JOB_TYPE: 'serenity-provision-workspace', + }, + '../../src/support/serenity/handlers/create-market-job.js': { + CREATE_MARKET_JOB_TYPE: 'serenity-create-market', + }, + '../../src/support/brands-storage.js': { + upsertBrand: upsertStub, + beginProvisioningAttempt: beginStub, + promoteProvisioningFailed: promoteFailedStub, + recordFreshBrandProvisioningStartFailure: recordFreshFailureStub, + }, + }); + const controller = Mocked.default(context, loggerStub, mockEnv); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(500); + const body = await response.json(); + expect(body.message).to.equal('DB unavailable'); + expect(recordFreshFailureStub.calledOnce).to.equal(true); + expect(recordFreshFailureStub.firstCall.args[0].brandId) + .to.equal(upsertStub.firstCall.args[0].forceBrandId); + // The pre-existing established-brand compensation must NOT fire for a fresh brand. + expect(promoteFailedStub.called).to.equal(false); + expect(enqueueStub.called).to.equal(false); + }); + it('surfaces a bare sub-workspace provisioning failure and does not write the brand', async () => { // The bare sub-workspace is provisioned BEFORE the brand row is written; if // that upstream call throws, the create surfaces the error and never persists diff --git a/test/support/serenity/handlers/activate-brand-workspace-job.test.js b/test/support/serenity/handlers/activate-brand-workspace-job.test.js index 14c11e642c..de23a7da1b 100644 --- a/test/support/serenity/handlers/activate-brand-workspace-job.test.js +++ b/test/support/serenity/handlers/activate-brand-workspace-job.test.js @@ -26,21 +26,16 @@ function makeJob(metadata) { return { getId: () => 'job-1', getMetadata: () => metadata }; } -function makeBrand({ saveError } = {}) { - return { - setStatus: sinon.stub(), - save: saveError ? sinon.stub().rejects(saveError) : sinon.stub().resolves(), - }; +function makeBrand(status) { + return { getStatus: () => status }; } describe('handlers/activate-brand-workspace-job.js (Phase 4, LLMO-7352/LLMO-7418)', () => { let context; - let brand; let findByIdStub; beforeEach(() => { - brand = makeBrand(); - findByIdStub = sinon.stub().resolves(brand); + findByIdStub = sinon.stub(); context = { log: fakeLog(), dataAccess: { Brand: { findById: findByIdStub } }, @@ -51,57 +46,75 @@ describe('handlers/activate-brand-workspace-job.js (Phase 4, LLMO-7352/LLMO-7418 expect(ACTIVATE_BRAND_WORKSPACE_JOB_TYPE).to.equal('serenity-activate-brand-workspace'); }); - it('flips the brand to active and returns 200 on a successful save', async () => { + it('reports 200/active without writing anything when the brand is (still) active — the common case (wasPending: true)', async () => { + findByIdStub.resolves(makeBrand('active')); const job = makeJob({ brandId: BRAND_ID, wasPending: true }); const result = await activateBrandWorkspaceJobHandler(context, job); expect(findByIdStub).to.have.been.calledOnceWith(BRAND_ID); - expect(brand.setStatus).to.have.been.calledOnceWith('active'); - expect(brand.save).to.have.been.calledOnce; expect(result).to.deep.equal({ status: 200, body: { brandId: BRAND_ID, status: 'active', markets: [] } }); }); - it('returns 502 with serenityActivationIncomplete when a real pending->active transition fails to save', async () => { - const saveError = new Error('write conflict'); - brand = makeBrand({ saveError }); + it('reports 200/active without writing anything when the brand is (still) active (wasPending: false)', async () => { + findByIdStub.resolves(makeBrand('active')); + const job = makeJob({ brandId: BRAND_ID, wasPending: false }); + + const result = await activateBrandWorkspaceJobHandler(context, job); + + expect(result).to.deep.equal({ status: 200, body: { brandId: BRAND_ID, status: 'active', markets: [] } }); + }); + + it('never writes — this job has no save/setStatus to call, only a read', async () => { + const brand = makeBrand('active'); findByIdStub.resolves(brand); const job = makeJob({ brandId: BRAND_ID, wasPending: true }); + await activateBrandWorkspaceJobHandler(context, job); + + // The brand fake exposes only getStatus() — no setStatus/save were even offered, and the + // handler must not assume they exist or call them. If this test needed a setStatus/save + // stub to pass, the handler would be writing again. + expect(brand).to.not.have.property('setStatus'); + expect(brand).to.not.have.property('save'); + }); + + it('reports the brand\'s ACTUAL current status (not a manufactured failure) when a concurrent deactivate raced ahead of this chain (wasPending: true)', async () => { + // promoteProvisioningReady already durably flipped the brand active before this chain was + // enqueued; finding it 'pending' here means a legitimate /serenity/deactivate ran in + // between (see the handler's own doc). Must report that truthfully, not re-write 'active'. + findByIdStub.resolves(makeBrand('pending')); + const job = makeJob({ brandId: BRAND_ID, wasPending: true }); + const result = await activateBrandWorkspaceJobHandler(context, job); - expect(result).to.deep.equal({ - status: 502, - body: { - brandId: BRAND_ID, - status: 'pending', - error: 'serenityActivationIncomplete', - message: 'Sub-workspace provisioned but the active status could not be persisted.', - markets: [], - }, - }); - expect(context.log.error).to.have.been.calledOnce; + expect(result).to.deep.equal({ status: 207, body: { brandId: BRAND_ID, status: 'pending', markets: [] } }); }); - it('returns 207 (still active) when a bare-reactivation no-op re-affirm fails to save', async () => { - const saveError = new Error('transient write failure'); - brand = makeBrand({ saveError }); - findByIdStub.resolves(brand); + it('reports the brand\'s ACTUAL current status when a concurrent deactivate raced ahead of this chain (wasPending: false)', async () => { + findByIdStub.resolves(makeBrand('pending')); const job = makeJob({ brandId: BRAND_ID, wasPending: false }); const result = await activateBrandWorkspaceJobHandler(context, job); - expect(result).to.deep.equal({ status: 207, body: { brandId: BRAND_ID, status: 'active', markets: [] } }); + expect(result).to.deep.equal({ status: 207, body: { brandId: BRAND_ID, status: 'pending', markets: [] } }); }); - it('defaults wasPending to false (207, not 502) when absent from metadata', async () => { - const saveError = new Error('transient write failure'); - brand = makeBrand({ saveError }); - findByIdStub.resolves(brand); + it('logs when the brand is found not-active, for observability', async () => { + findByIdStub.resolves(makeBrand('pending')); + const job = makeJob({ brandId: BRAND_ID, wasPending: true }); + + await activateBrandWorkspaceJobHandler(context, job); + + expect(context.log.info).to.have.been.calledOnce; + }); + + it('defaults wasPending to false when absent from metadata, without affecting the (read-only) outcome', async () => { + findByIdStub.resolves(makeBrand('active')); const job = makeJob({ brandId: BRAND_ID }); const result = await activateBrandWorkspaceJobHandler(context, job); - expect(result.status).to.equal(207); + expect(result.status).to.equal(200); }); }); From fb236e6d6e0b06d9787963ee24548855892bbdcf Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Wed, 9 Sep 2026 20:09:49 +0300 Subject: [PATCH 03/11] fix(serenity): activate's async begin-sites reconcile stale attempts (LLMO-7418 external-review Finding 9) Completes Finding 9's fix (started on the async-provisioning-endpoints branch): the pending->active and bare-reactivation async branches also mint a fresh provisioning attempt via beginProvisioningAttempt, which has no staleness awareness of its own. Reconcile via guardAgainstConcurrentProvisioning first, same as createMarket and the batch-market activate branch. Co-Authored-By: Claude Sonnet 5 --- src/controllers/serenity.js | 16 ++++++++++++++++ test/controllers/serenity.test.js | 6 ++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index babf3d574d..4b2d99c4bf 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -1680,6 +1680,15 @@ function SerenityController(context, log, env) { // `provision-workspace-job` -> // `serenity-activate-brand-workspace` job chain instead. if (validateAsync(body)) { + // LLMO-7418 external-review Finding 9: see createMarket's async branch for the full + // rationale — reconcile a stale in-flight attempt (reusing the sync guard's own logic) + // before minting a new one, since beginProvisioningAttempt's own CAS has no staleness + // awareness on its own. + await guardAgainstConcurrentProvisioning( + brandUuid, + ctx.dataAccess.services.postgrestClient, + log, + ); const attemptId = randomUUID(); const began = await beginProvisioningAttempt({ brandId: brandUuid, @@ -1791,6 +1800,13 @@ function SerenityController(context, log, env) { // activate-brand-workspace-job.js's save-divergence handling matches this branch's own // 207-not-502 contract. if (validateAsync(body)) { + // LLMO-7418 external-review Finding 9: see createMarket's async branch (and the + // wasPending branch above) for the full rationale. + await guardAgainstConcurrentProvisioning( + brandUuid, + ctx.dataAccess.services.postgrestClient, + log, + ); const attemptId = randomUUID(); const began = await beginProvisioningAttempt({ brandId: brandUuid, diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index cf501fa61c..83626f7b2d 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -2467,7 +2467,8 @@ describe('SerenityController', () => { expect(response.status).to.equal(202); expect(ensureSubworkspaceStub).to.not.have.been.called; - expect(guardAgainstConcurrentProvisioningStub).to.not.have.been.called; + // LLMO-7418 external-review Finding 9: reconciles a stale in-flight attempt first. + expect(guardAgainstConcurrentProvisioningStub).to.have.been.calledOnceWith(BRAND); expect(brand.setStatus).to.not.have.been.called; expect(beginProvisioningAttemptStub).to.have.been.calledOnce; expect(beginProvisioningAttemptStub.firstCall.args[0]).to.include({ @@ -2528,7 +2529,8 @@ describe('SerenityController', () => { expect(response.status).to.equal(202); expect(ensureSubworkspaceStub).to.not.have.been.called; - expect(guardAgainstConcurrentProvisioningStub).to.not.have.been.called; + // LLMO-7418 external-review Finding 9: reconciles a stale in-flight attempt first. + expect(guardAgainstConcurrentProvisioningStub).to.have.been.calledOnceWith(BRAND); expect(createAndEnqueueJobStub).to.have.been.calledOnce; const [, enqueueArgs] = createAndEnqueueJobStub.firstCall.args; expect(enqueueArgs.metadata.chainedJobType).to.equal('serenity-activate-brand-workspace'); From 4ac1265fe3c2e10a2797e8aa410aff47584b9c6d Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Wed, 9 Sep 2026 21:30:56 +0300 Subject: [PATCH 04/11] fix(serenity): activate's 2 skip-mode async branches pass a real sub-workspace title (LLMO-7418 external-review Finding 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes Finding 4's fix (started on the worker branch, which added the worker's own fail-fast): neither of activate's pending->active or bare-reactivation async branches passed `title` in the provision-workspace- job metadata. The pending->active branch is a GUARANTEED bug (a pending brand never has a workspace pointer, so the worker always takes the create path there) — every async activation of a pending brand created an untitled Semrush sub-workspace, permanently unrecoverable by title-based adoption. The bare-reactivation branch mirrors its synchronous twin's own defensive handling of a possibly-pointer-less already-active brand, so it needed the same fix even though the common case is safe. createMarket's and the batch-market activate branch's own async sites are deliberately left untouched: both have a code-referenced, always-true invariant (subworkspace mode / an already-active brand) that the pointer already exists, so the worker's existing-pointer fast path is always taken and `title` is never read there. Co-Authored-By: Claude Sonnet 5 --- src/controllers/serenity.js | 13 +++++++++++++ test/controllers/serenity.test.js | 8 ++++++++ 2 files changed, 21 insertions(+) diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index 4b2d99c4bf..f7cf91bd9f 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -1711,6 +1711,12 @@ function SerenityController(context, log, env) { brandId: brandUuid, attemptId, parentWorkspaceId: auth.parentWorkspaceId ?? '', + // LLMO-7418 external-review Finding 4: this branch's brand is GUARANTEED + // pointer-less (a pending brand never has a workspace pointer), so the worker + // ALWAYS takes the create-or-adopt path here, never the existing-pointer fast + // path — omitting `title` would call Semrush with an untitled sub-workspace on + // every single pending->active async activation. + title: brand.getName?.() ?? '', chainedJobType: ACTIVATE_BRAND_WORKSPACE_JOB_TYPE, chainedJobMetadata: { brandId: brandUuid, wasPending: true }, }, @@ -1829,6 +1835,13 @@ function SerenityController(context, log, env) { brandId: brandUuid, attemptId, parentWorkspaceId: auth.parentWorkspaceId ?? '', + // LLMO-7418 external-review Finding 4: see the wasPending branch above — an + // already-active brand isn't as reliably pointer-less as a pending one, but the + // SYNCHRONOUS twin of this exact branch still defensively calls the general-purpose + // `ensureSubworkspace` (create-or-existing), so the async path must be able to + // create with a real title too, not assume the existing-pointer fast path always + // applies. + title: brand.getName?.() ?? '', chainedJobType: ACTIVATE_BRAND_WORKSPACE_JOB_TYPE, chainedJobMetadata: { brandId: brandUuid, wasPending: false }, }, diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index 83626f7b2d..e29aad7250 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -2480,6 +2480,10 @@ describe('SerenityController', () => { expect(enqueueArgs.metadata.chainedJobType).to.equal('serenity-activate-brand-workspace'); expect(enqueueArgs.metadata.chainedJobMetadata) .to.deep.equal({ brandId: BRAND, wasPending: true }); + // LLMO-7418 external-review Finding 4: a pending brand is GUARANTEED pointer-less, so the + // worker always takes the create path here — omitting title would create an untitled + // sub-workspace on every single async pending->active activation. + expect(enqueueArgs.metadata.title).to.equal('Test Brand'); }); it('Phase 4: pending→active activation answers 409 without enqueuing when async: true and a provisioning attempt is already in flight', async () => { @@ -2536,6 +2540,10 @@ describe('SerenityController', () => { expect(enqueueArgs.metadata.chainedJobType).to.equal('serenity-activate-brand-workspace'); expect(enqueueArgs.metadata.chainedJobMetadata) .to.deep.equal({ brandId: BRAND, wasPending: false }); + // LLMO-7418 external-review Finding 4: this branch's own synchronous twin defensively + // handles a pointer-less brand via ensureSubworkspace, so the async path needs a real + // title too rather than assuming a pointer always exists. + expect(enqueueArgs.metadata.title).to.equal('Test Brand'); }); it('Phase 4: bare reactivation answers 409 without enqueuing when async: true and a provisioning attempt is already in flight', async () => { From 5d681705bc864777102869e53ab8ae02733b1d3f Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 11:12:49 +0300 Subject: [PATCH 05/11] fix(serenity): record the first-hop job id at the remaining 3 async provisioning call sites (LLMO-7418 external-review Finding 17) - createBrandForOrg's hasSemrushMarket-async and bare-async branches (brands.js), and activate's wasPending/bare-reactivation async branches (serenity.js), now call updateProvisioningJobId (best-effort) right after their createAndEnqueueJob call. - Completes Finding 17 across all 5 async provisioning call sites; the other 2 (createMarket, activate batch-market) were already fixed on feat/LLMO-7418-async-provisioning-endpoints. Co-Authored-By: Claude Sonnet 5 --- src/controllers/brands.js | 25 +++++++++++++++++++++++++ src/controllers/serenity.js | 24 ++++++++++++++++++++++++ test/controllers/brands.test.js | 24 ++++++++++++++++++++++++ test/controllers/serenity.test.js | 16 ++++++++++++++++ 4 files changed, 89 insertions(+) diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 3c39c0be48..172ca4ddaf 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -67,6 +67,7 @@ import { readSerenityFlagScopes, withSerenityState, beginProvisioningAttempt, + updateProvisioningJobId, promoteProvisioningFailed, recordFreshBrandProvisioningStartFailure, } from '../support/brands-storage.js'; @@ -2128,6 +2129,18 @@ function BrandsController(ctx, log, env) { }, }, }); + // LLMO-7418 external-review Finding 17: records the first-hop job id, best-effort, + // so it isn't left permanently NULL until the worker's own self-requeue hop writes it. + await updateProvisioningJobId({ + brandId: asyncBrandId, + attemptId, + jobId: job.getId(), + postgrestClient, + }).catch((updateError) => { + log.error('brands: failed to record the first-hop job id (best-effort)', { + brandId: asyncBrandId, attemptId, jobId: job.getId(), error: updateError?.message, + }); + }); return createResponse( { ...withSerenityState(created, serenityScopes), status: 'pending', jobId: job.getId() }, 202, @@ -2202,6 +2215,18 @@ function BrandsController(ctx, log, env) { title: brandData.name, }, }); + // LLMO-7418 external-review Finding 17: records the first-hop job id, best-effort, + // so it isn't left permanently NULL until the worker's own self-requeue hop writes it. + await updateProvisioningJobId({ + brandId: asyncBrandId, + attemptId, + jobId: job.getId(), + postgrestClient, + }).catch((updateError) => { + log.error('brands: failed to record the first-hop job id (best-effort)', { + brandId: asyncBrandId, attemptId, jobId: job.getId(), error: updateError?.message, + }); + }); return createResponse( { ...withSerenityState(created, serenityScopes), status: 'pending', jobId: job.getId() }, 202, diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index f7cf91bd9f..cc94bfdd47 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -1721,6 +1721,18 @@ function SerenityController(context, log, env) { chainedJobMetadata: { brandId: brandUuid, wasPending: true }, }, }); + // LLMO-7418 external-review Finding 17: records the first-hop job id, best-effort, + // so it isn't left permanently NULL until the worker's own self-requeue hop writes it. + await updateProvisioningJobId({ + brandId: brandUuid, + attemptId, + jobId: job.getId(), + postgrestClient: ctx.dataAccess.services.postgrestClient, + }).catch((updateError) => { + log.error('activate: failed to record the first-hop job id (best-effort)', { + brandId: brandUuid, attemptId, jobId: job.getId(), error: updateError?.message, + }); + }); return accepted({ jobId: job.getId(), status: job.getStatus() }); } // PR-C guard (LLMO-7352/LLMO-7418): this branch stays synchronous, but a market-creating @@ -1846,6 +1858,18 @@ function SerenityController(context, log, env) { chainedJobMetadata: { brandId: brandUuid, wasPending: false }, }, }); + // LLMO-7418 external-review Finding 17: records the first-hop job id, best-effort, + // so it isn't left permanently NULL until the worker's own self-requeue hop writes it. + await updateProvisioningJobId({ + brandId: brandUuid, + attemptId, + jobId: job.getId(), + postgrestClient: ctx.dataAccess.services.postgrestClient, + }).catch((updateError) => { + log.error('activate: failed to record the first-hop job id (best-effort)', { + brandId: brandUuid, attemptId, jobId: job.getId(), error: updateError?.message, + }); + }); return accepted({ jobId: job.getId(), status: job.getStatus() }); } // PR-C guard (LLMO-7352/LLMO-7418): see the wasPending branch above for rationale. diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index c6f44a3088..fe48dbdb72 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -5318,6 +5318,7 @@ describe('Brands Controller', () => { beginProvisioningAttempt = sinon.stub().resolves(true), createAndEnqueueJob = sinon.stub().resolves({ getId: () => 'job-abc' }), promoteProvisioningFailed = sinon.stub().resolves(true), + updateProvisioningJobId = sinon.stub().resolves(true), } = {}) { const Mocked = await esmock('../../src/controllers/brands.js', { '../../src/support/serenity/brand-provisioning.js': { @@ -5340,6 +5341,7 @@ describe('Brands Controller', () => { ...(upsertBrand ? { upsertBrand } : {}), beginProvisioningAttempt, promoteProvisioningFailed, + updateProvisioningJobId, }, }); return Mocked.default(context, loggerStub, mockEnv); @@ -5383,10 +5385,12 @@ describe('Brands Controller', () => { const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); const beginStub = sinon.stub().resolves(true); const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const updateProvisioningJobIdStub = sinon.stub().resolves(true); const controller = await buildController({ upsertBrand: upsertStub, beginProvisioningAttempt: beginStub, createAndEnqueueJob: enqueueStub, + updateProvisioningJobId: updateProvisioningJobIdStub, }); const response = await controller.createBrandForOrg({ @@ -5414,6 +5418,14 @@ describe('Brands Controller', () => { expect(enqueueStub.calledOnce).to.equal(true); expect(enqueueStub.calledAfter(beginStub)).to.equal(true); + // LLMO-7418 external-review Finding 17: the first hop's job id is recorded, not left + // permanently NULL — only the worker's own self-requeue path used to write this. + expect(updateProvisioningJobIdStub).to.have.been.calledOnceWith({ + brandId: upsertArgs.forceBrandId, + attemptId: beginStub.firstCall.args[0].attemptId, + jobId: 'job-xyz', + postgrestClient: sinon.match.any, + }); const [, enqueueArgs] = enqueueStub.firstCall.args; expect(enqueueArgs.jobType).to.equal('serenity-provision-workspace'); expect(enqueueArgs.metadata.brandId).to.equal(upsertArgs.forceBrandId); @@ -5777,10 +5789,14 @@ describe('Brands Controller', () => { const bareStub = sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }); const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const beginStub = sinon.stub().resolves(true); + const updateProvisioningJobIdStub = sinon.stub().resolves(true); const controller = await buildController({ provisionBrandSubworkspaceBare: bareStub, upsertBrand: upsertStub, createAndEnqueueJob: enqueueStub, + beginProvisioningAttempt: beginStub, + updateProvisioningJobId: updateProvisioningJobIdStub, }); const response = await controller.createBrandForOrg({ @@ -5806,6 +5822,14 @@ describe('Brands Controller', () => { // The row is persisted with no workspace pointer yet. const upsertArgs = upsertStub.firstCall.args[0]; expect(upsertArgs.semrushSubWorkspaceId).to.equal(null); + // LLMO-7418 external-review Finding 17: the first hop's job id is recorded, not left + // permanently NULL — only the worker's own self-requeue path used to write this. + expect(updateProvisioningJobIdStub).to.have.been.calledOnceWith({ + brandId: upsertArgs.forceBrandId, + attemptId: beginStub.firstCall.args[0].attemptId, + jobId: 'job-xyz', + postgrestClient: sinon.match.any, + }); }); it('Phase 4: bare create returns 409 without enqueuing when a provisioning attempt is already in flight', async () => { diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index e29aad7250..1189e953fe 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -2484,6 +2484,14 @@ describe('SerenityController', () => { // worker always takes the create path here — omitting title would create an untitled // sub-workspace on every single async pending->active activation. expect(enqueueArgs.metadata.title).to.equal('Test Brand'); + // LLMO-7418 external-review Finding 17: the first hop's job id is recorded, not left + // permanently NULL — only the worker's own self-requeue path used to write this. + expect(updateProvisioningJobIdStub).to.have.been.calledOnceWith({ + brandId: BRAND, + attemptId: beginProvisioningAttemptStub.firstCall.args[0].attemptId, + jobId: 'job-abc', + postgrestClient: sinon.match.any, + }); }); it('Phase 4: pending→active activation answers 409 without enqueuing when async: true and a provisioning attempt is already in flight', async () => { @@ -2544,6 +2552,14 @@ describe('SerenityController', () => { // handles a pointer-less brand via ensureSubworkspace, so the async path needs a real // title too rather than assuming a pointer always exists. expect(enqueueArgs.metadata.title).to.equal('Test Brand'); + // LLMO-7418 external-review Finding 17: the first hop's job id is recorded, not left + // permanently NULL — only the worker's own self-requeue path used to write this. + expect(updateProvisioningJobIdStub).to.have.been.calledOnceWith({ + brandId: BRAND, + attemptId: beginProvisioningAttemptStub.firstCall.args[0].attemptId, + jobId: 'job-abc', + postgrestClient: sinon.match.any, + }); }); it('Phase 4: bare reactivation answers 409 without enqueuing when async: true and a provisioning attempt is already in flight', async () => { From 7ccf698c85f64fdf10ef93c81a2c5b5c966d4b92 Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 11:41:35 +0300 Subject: [PATCH 06/11] fix(serenity): fix 4 Medium external-review findings (async 202 shape, unguarded findById, stale OpenAPI docs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - brands.js: createBrandForOrg's two async 202 responses no longer clobber the brand's real persisted `status` with a hardcoded 'pending' literal via bad spread ordering — the response now matches the same shape the synchronous 201 response returns (plus jobId), so a caller anchoring the brand at create time (baseSiteId supplied) sees the real 'active' status instead of a misleading 'pending'. - activate-brand-workspace-job.js: guard the unguarded Brand.findById() — a concurrent hard-delete racing ahead of this chain no longer throws a TypeError on brand.getStatus(), it reports 207/deleted instead. - schemas.yaml: fix 3 stale `async` property descriptions that no longer matched current behavior — SerenityActivateRequest's said the pending/bare-reactivation branches "always stay synchronous" (Phase 4 made both async-capable), and V2BrandInput's/SerenityCreateMarketRequest's didn't document that `async` is silently ignored (no error) outside Semrush/subworkspace mode. Co-Authored-By: Claude Sonnet 5 --- docs/openapi/schemas.yaml | 50 +++++++++++-------- src/controllers/brands.js | 4 +- .../handlers/activate-brand-workspace-job.js | 8 +++ test/controllers/brands.test.js | 15 ++++-- .../activate-brand-workspace-job.test.js | 10 ++++ 5 files changed, 62 insertions(+), 25 deletions(-) diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index d47e40fdab..487773d23d 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -8101,17 +8101,21 @@ V2BrandInput: type: boolean default: false description: > - PR-C (LLMO-7352/LLMO-7418). Only meaningful when `semrushMarket` is - supplied. When true, the brand row is persisted first (visible, - non-active — "Setting up"), then the sub-workspace ready-check and the - initial market's project create/publish are enqueued to a background - job chain instead of running inline, and the endpoint returns 202 with - a job id to poll via `GET .../serenity/jobs/{jobId}`. Omitted/false - processes synchronously as today (still 201) — this is opt-in only and - not yet the default. Unlike `SerenityCreatePromptsRequest`'s `async` + PR-C (LLMO-7352/LLMO-7418), extended by Phase 4 (LLMO-6405 "B"). Meaningful on + either of this endpoint's two Semrush-provisioning creates: with `semrushMarket` + supplied (sub-workspace ready-check + initial market's project create/publish, + enqueued as a job chain), or without it, for a sub-workspace-only active create + anchored by `baseSiteId` (sub-workspace create/adopt only, no project). In both + cases, true persists the brand row first (visible, non-active — "Setting up") + and returns 202 with a job id to poll via `GET .../serenity/jobs/{jobId}`. + Ignored (no error, brand created synchronously) when the organization is not + serenity-active (a flat/brandalf org) or the brand is created `pending` (a + pending brand always defers ALL Semrush provisioning regardless of this flag). + Omitted/false processes synchronously as today (still 201) — this is opt-in + only and not yet the default. Unlike `SerenityCreatePromptsRequest`'s `async` flag, this one is not intended to be a permanent dual-mode feature: the - synchronous path is the LLMO-7352 bug pattern itself, and is expected - to be retired once known callers have migrated to `async: true`. + synchronous path is the LLMO-7352 bug pattern itself, and is expected to be + retired once known callers have migrated to `async: true`. V2BrandUpdateInput: type: object @@ -12676,12 +12680,15 @@ SerenityActivateRequest: type: boolean default: false description: > - PR-C (LLMO-7352/LLMO-7418). Only meaningful on the already-active-brand, - markets-supplied path — a pending or bare-reactivation activation always - stays synchronous regardless of this flag. When true, the sub-workspace - ready-check and the per-market project create/publish + site-link + active - flip are enqueued to a background job chain instead of running inline, and - the endpoint returns 202 with a job id to poll via + PR-C (LLMO-7352/LLMO-7418), extended by Phase 4. Meaningful on all three + activation shapes this endpoint supports: the already-active-brand, + markets-supplied path (sub-workspace ready-check + per-market project + create/publish + site-link + active flip, enqueued as a job chain); the + pending→active activation (sub-workspace create/adopt only, enqueued as + `provision-workspace-job` -> `activate-brand-workspace-job`); and bare + reactivation of an already-active, pointer-less brand (same chain). In + every case, true enqueues the work to a background job chain instead of + running inline, and the endpoint returns 202 with a job id to poll via `GET .../serenity/jobs/{jobId}`. Omitted/false processes synchronously as today — this is opt-in only and not yet the default. @@ -12818,12 +12825,15 @@ SerenityCreateMarketRequest: type: boolean default: false description: > - PR-C (LLMO-7352/LLMO-7418). When true (sub-workspace-mode brands only), - the sub-workspace-ready check and the project create/publish are + PR-C (LLMO-7352/LLMO-7418). When true, and only for a sub-workspace-mode + brand, the sub-workspace-ready check and the project create/publish are enqueued to a background job chain instead of running inline, and the endpoint returns 202 with a job id to poll via - `GET .../serenity/jobs/{jobId}`. Omitted/false processes synchronously - as today — this is opt-in only and not yet the default. Unlike + `GET .../serenity/jobs/{jobId}`. Ignored (no error, market created + synchronously) for a flat-mode brand — flat mode has no sub-workspace + readiness to await, so there is nothing to defer. Omitted/false + processes synchronously as today — this is opt-in only and not yet the + default. Unlike `SerenityCreatePromptsRequest`'s `async` flag, this one is not intended to be a permanent dual-mode feature: the synchronous path is the LLMO-7352 bug pattern itself, and is expected to be retired once known diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 172ca4ddaf..749361588e 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -2142,7 +2142,7 @@ function BrandsController(ctx, log, env) { }); }); return createResponse( - { ...withSerenityState(created, serenityScopes), status: 'pending', jobId: job.getId() }, + { ...withSerenityState(created, serenityScopes), jobId: job.getId() }, 202, ); } catch (enqueueError) { @@ -2228,7 +2228,7 @@ function BrandsController(ctx, log, env) { }); }); return createResponse( - { ...withSerenityState(created, serenityScopes), status: 'pending', jobId: job.getId() }, + { ...withSerenityState(created, serenityScopes), jobId: job.getId() }, 202, ); } catch (enqueueError) { diff --git a/src/support/serenity/handlers/activate-brand-workspace-job.js b/src/support/serenity/handlers/activate-brand-workspace-job.js index c598bd757d..33343adabb 100644 --- a/src/support/serenity/handlers/activate-brand-workspace-job.js +++ b/src/support/serenity/handlers/activate-brand-workspace-job.js @@ -56,6 +56,14 @@ export async function activateBrandWorkspaceJobHandler(context, job) { const { brandId, wasPending = false } = metadata; const brand = await dataAccess.Brand.findById(brandId); + if (!brand) { + // A concurrent hard-delete raced ahead of this chain — report it the same way a + // no-longer-active brand is reported above, rather than crashing on brand.getStatus(). + log?.info?.('activate-brand-workspace-job: brand no longer exists by the time this chain ran; reporting deleted instead of throwing', { + brandId, wasPending, + }); + return { status: 207, body: { brandId, status: 'deleted', markets: [] } }; + } const currentStatus = brand.getStatus?.(); if (currentStatus === 'active') { diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index fe48dbdb72..70140a18d3 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -5382,7 +5382,7 @@ describe('Brands Controller', () => { }); it('mints a provisioning attempt and enqueues the provision->create-market job chain, answering 202', async () => { - const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand', status: 'pending' }); const beginStub = sinon.stub().resolves(true); const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); const updateProvisioningJobIdStub = sinon.stub().resolves(true); @@ -5403,6 +5403,9 @@ describe('Brands Controller', () => { expect(response.status).to.equal(202); const body = await response.json(); + // LLMO-7418 external-review Medium finding: the 202 body's `status` must be the + // brand's REAL persisted status (same shape the sync 201 response returns), not a + // hardcoded literal that clobbers it via spread ordering. expect(body.status).to.equal('pending'); expect(body.jobId).to.equal('job-xyz'); @@ -5787,7 +5790,10 @@ describe('Brands Controller', () => { it('Phase 4: bare create mints a provisioning attempt and enqueues provision-workspace-job with NO chained job when async: true', async () => { const bareStub = sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }); - const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + // A baseSiteId is supplied below, so upsertBrand's own hasAnchor invariant persists + // this brand as 'active' immediately — the async path only defers Semrush + // sub-workspace provisioning, not the brand's own lifecycle status. + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand', status: 'active' }); const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); const beginStub = sinon.stub().resolves(true); const updateProvisioningJobIdStub = sinon.stub().resolves(true); @@ -5809,7 +5815,10 @@ describe('Brands Controller', () => { expect(response.status).to.equal(202); const body = await response.json(); - expect(body.status).to.equal('pending'); + // LLMO-7418 external-review Medium finding: the 202 body's `status` must be the + // brand's REAL persisted status (same shape the sync 201 response returns), not a + // hardcoded literal that clobbers it via spread ordering. + expect(body.status).to.equal('active'); expect(body.jobId).to.equal('job-xyz'); // The bare-sync provisioner never runs on the async path. expect(bareStub.called).to.equal(false); diff --git a/test/support/serenity/handlers/activate-brand-workspace-job.test.js b/test/support/serenity/handlers/activate-brand-workspace-job.test.js index de23a7da1b..a703124e50 100644 --- a/test/support/serenity/handlers/activate-brand-workspace-job.test.js +++ b/test/support/serenity/handlers/activate-brand-workspace-job.test.js @@ -109,6 +109,16 @@ describe('handlers/activate-brand-workspace-job.js (Phase 4, LLMO-7352/LLMO-7418 expect(context.log.info).to.have.been.calledOnce; }); + it('reports 207/deleted instead of throwing when a concurrent hard-delete raced ahead of this chain (LLMO-7418 external-review Medium finding)', async () => { + findByIdStub.resolves(null); + const job = makeJob({ brandId: BRAND_ID, wasPending: true }); + + const result = await activateBrandWorkspaceJobHandler(context, job); + + expect(result).to.deep.equal({ status: 207, body: { brandId: BRAND_ID, status: 'deleted', markets: [] } }); + expect(context.log.info).to.have.been.calledOnce; + }); + it('defaults wasPending to false when absent from metadata, without affecting the (read-only) outcome', async () => { findByIdStub.resolves(makeBrand('active')); const job = makeJob({ brandId: BRAND_ID }); From badbe6eebc04520041bffa858facd51c3a30c945 Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 11:41:57 +0300 Subject: [PATCH 07/11] fix(serenity): retry transient upstream/network errors during a workspace status poll (LLMO-7418 external-review Finding 14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously any error from transport.getWorkspaceStatus() — including a transient Semrush 503 or a network blip — landed in the generic outer catch and permanently failed the whole provisioning attempt with zero retries, even though the worker already has a bounded self-requeue mechanism built for exactly this "try again shortly" case. Classify by error.status: SerenityTransportError with 429/500/502/503/504, or any error with no status at all (a raw network-level failure), is now treated as transient — the poll result falls through as if the workspace were merely "not ready", routing through the EXISTING self-requeue ladder (same backoff/depth cap, no new mechanism). Everything else (a permanent 4xx like an expired IMS token) keeps today's fail-fast behavior. Co-Authored-By: Claude Sonnet 5 --- .../handlers/provision-workspace-job.js | 37 ++++++- .../handlers/provision-workspace-job.test.js | 97 +++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/src/support/serenity/handlers/provision-workspace-job.js b/src/support/serenity/handlers/provision-workspace-job.js index b9970eccaa..684635720c 100644 --- a/src/support/serenity/handlers/provision-workspace-job.js +++ b/src/support/serenity/handlers/provision-workspace-job.js @@ -38,6 +38,23 @@ import { */ export const PROVISION_WORKSPACE_JOB_TYPE = 'serenity-provision-workspace'; +// LLMO-7418 external-review Finding 14: retry classification for a status-poll failure. +// `SerenityTransportError` always carries a numeric `.status` (the upstream HTTP status); +// a raw network-level failure (fetch itself throwing — DNS, connection reset, timeout) has +// none. Only these are treated as transient and routed through the existing bounded +// self-requeue ladder below; everything else (a permanent 4xx like an expired/invalid IMS +// token, or an unexpected non-transport error) keeps today's fail-fast behavior via the +// outer catch. +const RETRYABLE_TRANSPORT_STATUSES = new Set([429, 500, 502, 503, 504]); +function isRetryableWorkspaceStatusError(error) { + const { status } = error ?? {}; + if (typeof status !== 'number') { + // No upstream status at all — a network-level failure, not an application error. + return true; + } + return RETRYABLE_TRANSPORT_STATUSES.has(status); +} + /** * Hard cap on self-requeue depth. Live-verified settle time for a SUCCESSFUL create is * seconds, not minutes (LLMO-7352 incident data: ~10s); this ladder exists for the @@ -403,7 +420,25 @@ export async function provisionWorkspaceHandler(context, job, accessToken) { } } - const statusResult = await transport.getWorkspaceStatus(candidate.workspaceId); + let statusResult; + try { + statusResult = await transport.getWorkspaceStatus(candidate.workspaceId); + } catch (error) { + if (!isRetryableWorkspaceStatusError(error)) { + throw error; + } + // Transient upstream/network failure — leave `statusResult` undefined so `status` + // below is `undefined`, which is neither ready nor terminal-failure, and this hop + // falls straight into the existing "still settling" self-requeue branch below (same + // bounded backoff/depth cap already used for an actual `not ready` poll result). + log?.warn?.('provision-workspace-job: transient error polling workspace status; treating as not-ready and self-requeuing', { + brandId, + attemptId, + semrushWorkspaceId: candidate.workspaceId, + error: error?.message, + status: error?.status, + }); + } const status = statusResult?.status; if (isWorkspaceReady(status)) { diff --git a/test/support/serenity/handlers/provision-workspace-job.test.js b/test/support/serenity/handlers/provision-workspace-job.test.js index 54015e57d0..20ec2052c8 100644 --- a/test/support/serenity/handlers/provision-workspace-job.test.js +++ b/test/support/serenity/handlers/provision-workspace-job.test.js @@ -763,6 +763,103 @@ describe('handlers/provision-workspace-job.js (LLMO-7352 / LLMO-7418)', () => { }); }); + describe('poll result: transient error retry classification (LLMO-7418 external-review Finding 14)', () => { + it('treats a 503 SerenityTransportError as transient and self-requeues instead of failing the attempt', async () => { + const err = new Error('Semrush GET .../status failed: 503'); + err.status = 503; + transport.getWorkspaceStatus.rejects(err); + const { provisionWorkspaceHandler } = await loadHandler(); + const job = makeJob(makeMetadata({ requeueDepth: 0 })); + + const result = await provisionWorkspaceHandler(context, job, 'token'); + + expect(createAndEnqueueJobStub).to.have.been.calledOnce; + expect(promoteProvisioningFailedStub).to.not.have.been.called; + expect(result).to.deep.equal({ requeuedJobId: 'job-followup' }); + }); + + it('retries 429/500/502/504 the same way', async () => { + for (const status of [429, 500, 502, 504]) { + const err = new Error(`upstream ${status}`); + err.status = status; + transport.getWorkspaceStatus.reset(); + transport.getWorkspaceStatus.rejects(err); + createAndEnqueueJobStub.resetHistory(); + promoteProvisioningFailedStub.resetHistory(); + // eslint-disable-next-line no-await-in-loop + const { provisionWorkspaceHandler } = await loadHandler(); + const job = makeJob(makeMetadata({ requeueDepth: 0 })); + + // eslint-disable-next-line no-await-in-loop + const result = await provisionWorkspaceHandler(context, job, 'token'); + + expect(createAndEnqueueJobStub, `status ${status}`).to.have.been.calledOnce; + expect(promoteProvisioningFailedStub, `status ${status}`).to.not.have.been.called; + expect(result, `status ${status}`).to.deep.equal({ requeuedJobId: 'job-followup' }); + } + }); + + it('treats a raw network failure (no .status) as transient and self-requeues', async () => { + transport.getWorkspaceStatus.rejects(new TypeError('fetch failed')); + const { provisionWorkspaceHandler } = await loadHandler(); + const job = makeJob(makeMetadata({ requeueDepth: 0 })); + + const result = await provisionWorkspaceHandler(context, job, 'token'); + + expect(createAndEnqueueJobStub).to.have.been.calledOnce; + expect(result).to.deep.equal({ requeuedJobId: 'job-followup' }); + }); + + it('still fails fast on a permanent 401 (expired/invalid IMS token), never self-requeuing it', async () => { + const err = new Error('Semrush GET .../status failed: 401'); + err.status = 401; + transport.getWorkspaceStatus.rejects(err); + const { provisionWorkspaceHandler } = await loadHandler(); + const job = makeJob(makeMetadata()); + + await expect(provisionWorkspaceHandler(context, job, 'token')).to.be.rejectedWith('401'); + + expect(createAndEnqueueJobStub).to.not.have.been.called; + expect(promoteProvisioningFailedStub).to.have.been.calledOnceWith({ + brandId: BRAND_ID, + attemptId: ATTEMPT_ID, + error: UNEXPECTED_ERROR_MESSAGE, + postgrestClient, + }); + }); + + it('still fails fast on a permanent 400, never self-requeuing it', async () => { + const err = new Error('Semrush GET .../status failed: 400'); + err.status = 400; + transport.getWorkspaceStatus.rejects(err); + const { provisionWorkspaceHandler } = await loadHandler(); + const job = makeJob(makeMetadata()); + + await expect(provisionWorkspaceHandler(context, job, 'token')).to.be.rejectedWith('400'); + + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + + it('still respects the requeue depth cap for a transient error at the cap (fails, does not loop forever)', async () => { + const err = new Error('upstream 503'); + err.status = 503; + transport.getWorkspaceStatus.rejects(err); + const { provisionWorkspaceHandler } = await loadHandler(); + const job = makeJob(makeMetadata({ requeueDepth: MAX_PROVISION_REQUEUE_DEPTH })); + + const result = await provisionWorkspaceHandler(context, job, 'token'); + + expect(createAndEnqueueJobStub).to.not.have.been.called; + expect(promoteProvisioningFailedStub).to.have.been.calledOnceWith({ + brandId: BRAND_ID, + attemptId: ATTEMPT_ID, + error: REQUEUE_EXHAUSTED_MESSAGE, + postgrestClient, + }); + expect(result).to.deep.equal({ provisioningStatus: 'failed' }); + }); + }); + describe('unexpected errors reaching the outer catch clean up an owned candidate (LLMO-7418 external-review Finding 16)', () => { it('cleans up a freshly-created candidate when an unexpected error is thrown BEFORE any self-requeue is enqueued', async () => { getBrandProvisioningStateStub.rejects(new Error('db read blip')); From ea68ae44f75c6b8540b0608796b86f35b8d5622a Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 12:05:26 +0300 Subject: [PATCH 08/11] fix(serenity): add a server-side kill switch for async provisioning (LLMO-7418 external-review Finding 15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously a caller passing `async: true` got the async provisioning path unconditionally, with no way to disable it org-wide short of a code change reverting every call site at once. Adds isAsyncProvisioningKillSwitched (serenity-active.js), reusing the same cached feature-flag machinery as every other predicate in that file, gated on a new opt-OUT flag (serenity_async_provisioning_disabled — off by default, so a transient read failure never silently disables async provisioning). Wired in immediately before validateAsync at all 6 async provisioning call sites (createMarket, createBrandForOrg's two branches, activate's three branches); a caller hits 503 and falls back to the synchronous path on retry when the switch is on. Co-Authored-By: Claude Sonnet 5 --- src/controllers/brands.js | 24 ++++++++++ src/controllers/serenity.js | 50 ++++++++++++++++++++- src/support/serenity/serenity-active.js | 42 ++++++++++++++++++ test/controllers/brands.test.js | 52 +++++++++++++++++++++- test/controllers/serenity.test.js | 59 +++++++++++++++++++++++++ 5 files changed, 225 insertions(+), 2 deletions(-) diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 749361588e..aee913226b 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -98,6 +98,7 @@ import { isSerenityActiveForBrand, isSerenityActiveForOrg, isSerenityUiActiveForOrg, + isAsyncProvisioningKillSwitched, } from '../support/serenity/serenity-active.js'; import { buildReservedIdentities, @@ -1870,6 +1871,18 @@ function BrandsController(ctx, log, env) { // is NOT permanent: the synchronous branch is the LLMO-7352 bug pattern itself, slated // for removal once every known caller has migrated to `async: true`. if (validateAsync(brandData)) { + // LLMO-7418 external-review Finding 15: server-side kill switch — lets ops disable + // the async path for this organization without a deploy if it misbehaves in + // production. The caller falls back to the synchronous path on its own retry. + if (await isAsyncProvisioningKillSwitched(context, spaceCatId, log)) { + return createResponse( + { + error: 'asyncProvisioningDisabled', + message: 'Async provisioning is temporarily disabled for this organization; retry without async: true', + }, + 503, + ); + } // brandAliases/urls/competitors are NOT read here (unlike the sync branch below): the // brand row this section persists below (upsertBrand) writes them to storage, and the // async chain's orchestration reads them back from there — the same DB-backed source @@ -1948,6 +1961,17 @@ function BrandsController(ctx, log, env) { // sub-workspace provisioning off to provision-workspace-job — no chained job, since a // bare create has no project to create once the workspace is ready. // + // LLMO-7418 external-review Finding 15: server-side kill switch — see the + // hasSemrushMarket branch above for rationale. + if (await isAsyncProvisioningKillSwitched(context, spaceCatId, log)) { + return createResponse( + { + error: 'asyncProvisioningDisabled', + message: 'Async provisioning is temporarily disabled for this organization; retry without async: true', + }, + 503, + ); + } // Resolved and validated HERE, before any write — same rationale as the // hasSemrushMarket branch: a missing org workspace config must never leave a // persisted, permanently-inert brand row behind. diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index cc94bfdd47..a13cd6adfd 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -92,7 +92,10 @@ import { handleTagImpactSubworkspace, } from '../support/serenity/handlers/tags.js'; import { ensureSubworkspace, decommissionBrandWorkspace } from '../support/serenity/workspace-lifecycle.js'; -import { isSerenityActiveForBrand } from '../support/serenity/serenity-active.js'; +import { + isSerenityActiveForBrand, + isAsyncProvisioningKillSwitched, +} from '../support/serenity/serenity-active.js'; import { marketForGeoTargetId } from '../support/serenity/locations.js'; import { brandNeedles, classifyBrandedTag } from '../support/serenity/branded-classifier.js'; import { computeWriteDeadline } from '../support/serenity/intent-classification.js'; @@ -1046,6 +1049,18 @@ function SerenityController(context, log, env) { // synchronous branch is the LLMO-7352 bug pattern itself, not a valid alternative, and // is slated for removal once every known caller has migrated to `async: true`. if (validateAsync(requestBody)) { + // LLMO-7418 external-review Finding 15: server-side kill switch — lets ops disable + // the async path for this organization without a deploy if it misbehaves in + // production. The caller falls back to the synchronous path on its own retry. + if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { + return createResponse( + { + error: 'asyncProvisioningDisabled', + message: 'Async provisioning is temporarily disabled for this organization; retry without async: true', + }, + 503, + ); + } // The worker's existing-pointer fast path (provision-workspace-job.js) polls THIS // brand's already-canonical workspace rather than provisioning a new one — every // brand reaching this branch already has one (`auth.mode === 'subworkspace'` IS that @@ -1680,6 +1695,17 @@ function SerenityController(context, log, env) { // `provision-workspace-job` -> // `serenity-activate-brand-workspace` job chain instead. if (validateAsync(body)) { + // LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's + // async branch for the full rationale. + if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { + return createResponse( + { + error: 'asyncProvisioningDisabled', + message: 'Async provisioning is temporarily disabled for this organization; retry without async: true', + }, + 503, + ); + } // LLMO-7418 external-review Finding 9: see createMarket's async branch for the full // rationale — reconcile a stale in-flight attempt (reusing the sync guard's own logic) // before minting a new one, since beginProvisioningAttempt's own CAS has no staleness @@ -1818,6 +1844,17 @@ function SerenityController(context, log, env) { // activate-brand-workspace-job.js's save-divergence handling matches this branch's own // 207-not-502 contract. if (validateAsync(body)) { + // LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's + // async branch for the full rationale. + if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { + return createResponse( + { + error: 'asyncProvisioningDisabled', + message: 'Async provisioning is temporarily disabled for this organization; retry without async: true', + }, + 503, + ); + } // LLMO-7418 external-review Finding 9: see createMarket's async branch (and the // wasPending branch above) for the full rationale. await guardAgainstConcurrentProvisioning( @@ -1944,6 +1981,17 @@ function SerenityController(context, log, env) { // in-request settle-poll + project-create/publish sequence), slated for removal once every // known caller has migrated to `async: true`. if (validateAsync(body)) { + // LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's + // async branch for the full rationale. + if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { + return createResponse( + { + error: 'asyncProvisioningDisabled', + message: 'Async provisioning is temporarily disabled for this organization; retry without async: true', + }, + 503, + ); + } // LLMO-7418 external-review Finding 9: see the createMarket async branch above for the // full rationale — reconcile a stale in-flight attempt before minting a new one, since // beginProvisioningAttempt's own CAS has no staleness awareness. diff --git a/src/support/serenity/serenity-active.js b/src/support/serenity/serenity-active.js index 39233e8b9c..2d8b161822 100644 --- a/src/support/serenity/serenity-active.js +++ b/src/support/serenity/serenity-active.js @@ -47,6 +47,17 @@ export const SERENITY_FEATURE_FLAG_NAME = 'serenity'; */ export const SERENITY_UI_FEATURE_FLAG_NAME = 'serenity_ui'; +/** + * Server-side kill switch for the opt-in `async: true` provisioning path (LLMO-7418 + * external-review Finding 15). Unlike `SERENITY_FEATURE_FLAG_NAME` above (an opt-IN + * rollout switch, off by default), this is an opt-OUT switch: the async path stays + * available by default, and an explicit `true` row disables it org-wide. Lets ops + * flip async provisioning off for one organization without a deploy, if it + * misbehaves in production — there was previously no way to do this short of a + * code change reverting every `async: true` call site at once. + */ +export const ASYNC_PROVISIONING_KILL_SWITCH_FLAG_NAME = 'serenity_async_provisioning_disabled'; + /** * Module-scoped TTL+size-bounded cache, mirroring the workspace-resolver cache * (warm Lambda containers reuse module state, so a Map here amortises the @@ -239,3 +250,34 @@ export async function isSerenityUiActiveForOrg(ctx, spaceCatId, log) { const scopes = await readCachedFlagScopes(ctx, spaceCatId, SERENITY_UI_FEATURE_FLAG_NAME, log); return scopes?.orgRow?.flag_value === true; } + +/** + * LLMO-7418 external-review Finding 15: server-side kill switch for the opt-in + * `async: true` provisioning path. Reads the org-wide + * `LLMO/serenity_async_provisioning_disabled` feature flag (cached, same + * machinery as every other flag in this file). + * + * Deliberately reuses the SAME "absent/unreadable resolves to `false`" + * fail-safe shape as every other predicate here — it happens to be the safe + * default in both directions: for a rollout flag, `false` means "stay off"; + * for this kill switch, `false` means "stay on" (async provisioning + * available). A transient PostgREST read failure must never silently disable + * async provisioning org-wide, so this is NOT inverted to fail closed. + * + * @param {object} ctx - Request context (uses + * `ctx.dataAccess.services.postgrestClient`). + * @param {string} spaceCatId - SpaceCat organization UUID. + * @param {object} [log] - Optional logger (used to surface a missing client / + * a read error without throwing on this hot path). + * @returns {Promise} `true` only when the flag is explicitly on + * (async provisioning is disabled for this organization). + */ +export async function isAsyncProvisioningKillSwitched(ctx, spaceCatId, log) { + const scopes = await readCachedFlagScopes( + ctx, + spaceCatId, + ASYNC_PROVISIONING_KILL_SWITCH_FLAG_NAME, + log, + ); + return scopes?.orgRow?.flag_value === true; +} diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index 70140a18d3..ebeceb9519 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -5319,6 +5319,9 @@ describe('Brands Controller', () => { createAndEnqueueJob = sinon.stub().resolves({ getId: () => 'job-abc' }), promoteProvisioningFailed = sinon.stub().resolves(true), updateProvisioningJobId = sinon.stub().resolves(true), + // LLMO-7418 external-review Finding 15: kill switch off by default (async available); + // specific tests override it to resolve(true) to exercise the 503 gate. + isAsyncProvisioningKillSwitched = sinon.stub().resolves(false), } = {}) { const Mocked = await esmock('../../src/controllers/brands.js', { '../../src/support/serenity/brand-provisioning.js': { @@ -5328,7 +5331,10 @@ describe('Brands Controller', () => { provisionBrandSubworkspaceBare: provisionBrandSubworkspaceBare || sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }), }, - '../../src/support/serenity/serenity-active.js': { isSerenityActiveForOrg }, + '../../src/support/serenity/serenity-active.js': { + isSerenityActiveForOrg, + isAsyncProvisioningKillSwitched, + }, '../../src/support/serenity/workspace-resolver.js': { resolveWorkspaceId }, '../../src/support/serenity/async-job-runner.js': { createAndEnqueueJob }, '../../src/support/serenity/handlers/provision-workspace-job.js': { @@ -5519,6 +5525,28 @@ describe('Brands Controller', () => { expect(enqueueStub).to.not.have.been.called; }); + it('returns 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => { + const beginStub = sinon.stub().resolves(true); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const controller = await buildController({ + beginProvisioningAttempt: beginStub, + createAndEnqueueJob: enqueueStub, + isAsyncProvisioningKillSwitched: sinon.stub().resolves(true), + }); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { ...semrushData }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(503); + expect(beginStub).to.not.have.been.called; + expect(enqueueStub).to.not.have.been.called; + }); + it('returns 400 without persisting a row when the organization has no Semrush parent workspace configured', async () => { const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); const controller = await buildController({ @@ -5863,6 +5891,28 @@ describe('Brands Controller', () => { expect(enqueueStub.called).to.equal(false); }); + it('Phase 4: bare create returns 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => { + const beginStub = sinon.stub().resolves(true); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const controller = await buildController({ + beginProvisioningAttempt: beginStub, + createAndEnqueueJob: enqueueStub, + isAsyncProvisioningKillSwitched: sinon.stub().resolves(true), + }); + + const response = await controller.createBrandForOrg({ + ...context, + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(503); + expect(beginStub.called).to.equal(false); + expect(enqueueStub.called).to.equal(false); + }); + it('Phase 4: bare create returns 400 without persisting a row when the organization has no Semrush parent workspace configured', async () => { const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); const controller = await buildController({ diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index 1189e953fe..9d1240e61e 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -199,6 +199,7 @@ describe('SerenityController', () => { let resolveWorkspaceIdStub; let resolveBrandWorkspaceStub; let isSerenityActiveStub; + let isAsyncProvisioningKillSwitchedStub; let createTransportStub; let resolveBrandUuidStub; let getBrandAliasesStub; @@ -238,6 +239,9 @@ describe('SerenityController', () => { // existing assertion that drives a brand-level route reaches its handler. // The "serenity inactive" describe overrides this to false. isSerenityActiveStub = sinon.stub().resolves(true); + // LLMO-7418 external-review Finding 15: kill switch off by default (async provisioning + // available); specific tests override it to resolve(true) to exercise the 503 gate. + isAsyncProvisioningKillSwitchedStub = sinon.stub().resolves(false); decommissionStub = sinon.stub().resolves(); ensureSubworkspaceStub = sinon.stub().resolves(SUBWS); clearBrandWorkspaceCacheStub = sinon.stub(); @@ -355,6 +359,7 @@ describe('SerenityController', () => { }, '../../src/support/serenity/serenity-active.js': { isSerenityActiveForBrand: isSerenityActiveStub, + isAsyncProvisioningKillSwitched: isAsyncProvisioningKillSwitchedStub, }, '../../src/support/access-control-util.js': MockAccessControlUtil, '../../src/support/prompts-storage.js': { @@ -1751,6 +1756,20 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('createMarket answers 503 without enqueuing anything when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => { + isAsyncProvisioningKillSwitchedStub.resolves(true); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.createMarket(fakeContext({ + data: { + market: 'us', languageCode: 'en', brandDomain: 'x.com', brandNames: ['X'], async: true, + }, + })); + expect(response.status).to.equal(503); + expect(guardAgainstConcurrentProvisioningStub).to.not.have.been.called; + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('createMarket runs the SAME synchronous orchestration it always has when async is absent (regression: default behavior unchanged)', async () => { orchestrateCreateMarketSubworkspaceStub.resolves({ status: 201, body: { brandId: BRAND, geoTargetId: 2840, languageCode: 'en' }, @@ -2306,6 +2325,21 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('activate answers 503 without enqueuing when async: true and the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => { + isAsyncProvisioningKillSwitchedStub.resolves(true); + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, + data: { + brandDomain: 'x.com', brandNames: ['X'], markets: [{ market: 'us', languageCode: 'en' }], async: true, + }, + })); + expect(response.status).to.equal(503); + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('activate 400s when async is present but not a boolean', async () => { const brand = makeBrandModel({ getStatus: () => 'active' }); const controller = SerenityController({ env: {} }, fakeLog(), {}); @@ -2506,6 +2540,19 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('Phase 4: pending→active activation answers 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => { + getBrandBaseSiteIdStub.resolves('primary-site'); + isAsyncProvisioningKillSwitchedStub.resolves(true); + const brand = makeBrandModel({}); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: true }, + })); + expect(response.status).to.equal(503); + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('Phase 4: pending→active activation 400s when async is present but not a boolean', async () => { getBrandBaseSiteIdStub.resolves('primary-site'); const brand = makeBrandModel({}); @@ -2573,6 +2620,18 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('Phase 4: bare reactivation answers 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => { + isAsyncProvisioningKillSwitchedStub.resolves(true); + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, data: { brandNames: ['X'], async: true }, + })); + expect(response.status).to.equal(503); + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('Phase 4: bare reactivation 400s when async is present but not a boolean', async () => { const brand = makeBrandModel({ getStatus: () => 'active' }); const controller = SerenityController({ env: {} }, fakeLog(), {}); From fe80d7dd304c050eac846af9cc96a54b6e775fec Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 19:03:53 +0300 Subject: [PATCH 09/11] feat(serenity): global async-provisioning master switch, default off (LLMO-7352/LLMO-7418) Separates "the code ships" from "the behaviour changes". Every Semrush brand-creating consumer now sends `async: true` (the dashboard's six flows, LLMO-7419), so without this gate merging the stack would by itself flip every Serenity organization onto the async path at once, with a merge revert as the only way back. With it, the stack merges inert, async is enabled deliberately, and it can be turned off again in seconds. `SERENITY_ASYNC_PROVISIONING_ENABLED`, a single global env boolean, DEFAULT OFF, wired at the 4 provisioning entry points in serenity.js and the 2 in brands.js -- alongside, not replacing, the existing per-org kill switch. When OFF an `async: true` request is NOT an error: it falls through to the synchronous branch the endpoint has always run, which is what makes merging safe by default (and is exercised by its own test). Deliberately global rather than a per-org opt-in: there are ~140 Serenity organizations, so a per-org ENABLE flag would mean creating and maintaining a row each -- real operational burden for no extra safety. The per-org kill switch already covers the cheap, exception-only direction (disable the one org that misbehaves). Same shape as this repo's other global serenity toggles (SERENITY_DEFER_PUBLISH, SERENITY_ALLOW_WORKSPACE_DELETE, SERENITY_ALLOW_NON_IMS_AUTH), so flipping it is a config change, not a code deploy. Note: createPrompts' own unrelated `async` flag is deliberately NOT gated -- it is the prompt classification feature, not provisioning. Tests: the off-case is pinned (async:true + switch off => synchronous, no attempt, no job) and mutation-verified -- removing the gate fails it. Existing async tests default the switch on. serenity + brands suites: 710 passing. eslint + type-check clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/controllers/brands.js | 5 ++-- src/controllers/serenity.js | 9 ++++--- src/support/serenity/serenity-active.js | 36 +++++++++++++++++++++++++ test/controllers/brands.test.js | 8 ++++++ test/controllers/serenity.test.js | 26 +++++++++++++++++- 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/controllers/brands.js b/src/controllers/brands.js index aee913226b..907850a0fc 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -99,6 +99,7 @@ import { isSerenityActiveForOrg, isSerenityUiActiveForOrg, isAsyncProvisioningKillSwitched, + isAsyncProvisioningEnabled, } from '../support/serenity/serenity-active.js'; import { buildReservedIdentities, @@ -1870,7 +1871,7 @@ function BrandsController(ctx, log, env) { // job chain, the same shared path Add Market uses. Unlike createPrompts's flag, this one // is NOT permanent: the synchronous branch is the LLMO-7352 bug pattern itself, slated // for removal once every known caller has migrated to `async: true`. - if (validateAsync(brandData)) { + if (validateAsync(brandData) && isAsyncProvisioningEnabled(context.env || env)) { // LLMO-7418 external-review Finding 15: server-side kill switch — lets ops disable // the async path for this organization without a deploy if it misbehaves in // production. The caller falls back to the synchronous path on its own retry. @@ -1948,7 +1949,7 @@ function BrandsController(ctx, log, env) { languageCode: provisioned.languageCode, }; } - } else if (validateAsync(brandData)) { + } else if (validateAsync(brandData) && isAsyncProvisioningEnabled(context.env || env)) { // B (LLMO-6405): sub-workspace-only active create — no market supplied, so // no project is provisioned. Markets are added afterwards from the Markets // tab. The brand is anchored by its primary site (baseSiteId, persisted by diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index a13cd6adfd..d06522931a 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -95,6 +95,7 @@ import { ensureSubworkspace, decommissionBrandWorkspace } from '../support/seren import { isSerenityActiveForBrand, isAsyncProvisioningKillSwitched, + isAsyncProvisioningEnabled, } from '../support/serenity/serenity-active.js'; import { marketForGeoTargetId } from '../support/serenity/locations.js'; import { brandNeedles, classifyBrandedTag } from '../support/serenity/branded-classifier.js'; @@ -1048,7 +1049,7 @@ function SerenityController(context, log, env) { // `createPrompts`'s flag, this one is NOT a permanent dual-mode feature: the // synchronous branch is the LLMO-7352 bug pattern itself, not a valid alternative, and // is slated for removal once every known caller has migrated to `async: true`. - if (validateAsync(requestBody)) { + if (validateAsync(requestBody) && isAsyncProvisioningEnabled(ctx.env || env)) { // LLMO-7418 external-review Finding 15: server-side kill switch — lets ops disable // the async path for this organization without a deploy if it misbehaves in // production. The caller falls back to the synchronous path on its own retry. @@ -1694,7 +1695,7 @@ function SerenityController(context, log, env) { // run. `async: true` hands the sub-workspace-ensure + status flip off to the // `provision-workspace-job` -> // `serenity-activate-brand-workspace` job chain instead. - if (validateAsync(body)) { + if (validateAsync(body) && isAsyncProvisioningEnabled(ctx.env || env)) { // LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's // async branch for the full rationale. if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { @@ -1843,7 +1844,7 @@ function SerenityController(context, log, env) { // already-active no-op re-affirm from a real pending->active transition, so // activate-brand-workspace-job.js's save-divergence handling matches this branch's own // 207-not-502 contract. - if (validateAsync(body)) { + if (validateAsync(body) && isAsyncProvisioningEnabled(ctx.env || env)) { // LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's // async branch for the full rationale. if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { @@ -1980,7 +1981,7 @@ function SerenityController(context, log, env) { // LLMO-7352 bug pattern itself (this is one of the 3 real conversion candidates — the // in-request settle-poll + project-create/publish sequence), slated for removal once every // known caller has migrated to `async: true`. - if (validateAsync(body)) { + if (validateAsync(body) && isAsyncProvisioningEnabled(ctx.env || env)) { // LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's // async branch for the full rationale. if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) { diff --git a/src/support/serenity/serenity-active.js b/src/support/serenity/serenity-active.js index 2d8b161822..58bdf00313 100644 --- a/src/support/serenity/serenity-active.js +++ b/src/support/serenity/serenity-active.js @@ -58,6 +58,42 @@ export const SERENITY_UI_FEATURE_FLAG_NAME = 'serenity_ui'; */ export const ASYNC_PROVISIONING_KILL_SWITCH_FLAG_NAME = 'serenity_async_provisioning_disabled'; +/** + * GLOBAL async-provisioning master switch (LLMO-7352/LLMO-7418), DEFAULT OFF. + * + * This exists to separate "the code ships" from "the behaviour changes". Every Semrush + * brand-creating consumer now sends `async: true` (the dashboard's six flows), so without this + * gate the very act of merging flips every Serenity organization onto the async path at once, + * and the only way back is reverting a merge under pressure. With it, the stack merges inert, + * async is turned on deliberately, and it can be turned off again in seconds. + * + * Deliberately a single global env boolean rather than a per-organization opt-in row: there are + * ~140 Serenity organizations, so a per-org ENABLE flag would mean creating and maintaining a row + * for each — real operational burden for no extra safety. The per-org + * {@link ASYNC_PROVISIONING_KILL_SWITCH_FLAG_NAME} covers the opposite, cheap case (disable the + * one organization that misbehaves), because that one is exception-only. + * + * Same shape as this codebase's other global serenity toggles (`SERENITY_DEFER_PUBLISH`, + * `SERENITY_ALLOW_WORKSPACE_DELETE`, `SERENITY_ALLOW_NON_IMS_AUTH`). Wired to Vault at + * `dx_mysticat//api-service`, so flipping it is a config change, not a code deploy. + * + * When OFF, an `async: true` request is NOT an error — it falls through to the synchronous branch + * the endpoint has always run. That is what makes merging safe by default: every consumer keeps + * working exactly as today until this is switched on. + */ +export const ASYNC_PROVISIONING_ENABLED_ENV_FLAG = 'SERENITY_ASYNC_PROVISIONING_ENABLED'; + +/** + * Reads the global async-provisioning master switch. `true` ONLY for the exact string `'true'` + * (env values are strings); anything else — unset, `'false'`, a typo — is OFF. Fail-safe by + * design: the default is the synchronous path this endpoint has always run. + * @param {object} [env] - the request env (`context.env`). + * @returns {boolean} + */ +export function isAsyncProvisioningEnabled(env) { + return env?.[ASYNC_PROVISIONING_ENABLED_ENV_FLAG] === 'true'; +} + /** * Module-scoped TTL+size-bounded cache, mirroring the workspace-resolver cache * (warm Lambda containers reuse module state, so a Map here amortises the diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index ebeceb9519..a2e7e07def 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -157,6 +157,11 @@ describe('Brands Controller', () => { BRAND_IMS_CLIENT_ID: 'client123', BRAND_IMS_CLIENT_CODE: 'code123', BRAND_IMS_CLIENT_SECRET: 'secret123', + // LLMO-7418: the global async-provisioning master switch is DEFAULT OFF in production, so an + // `async: true` request falls through to the synchronous branch unless it is enabled. Default + // it ON here so the existing async-path tests exercise the async branch they were written + // for; the OFF behaviour has its own dedicated test that omits this flag. + SERENITY_ASYNC_PROVISIONING_ENABLED: 'true', }; const authContextAdmin = { @@ -10334,6 +10339,9 @@ describe('Brands Controller — defensive branch coverage', () => { BRAND_IMS_CLIENT_ID: 'client', BRAND_IMS_CLIENT_CODE: 'code', BRAND_IMS_CLIENT_SECRET: 'secret', + // LLMO-7418: global async-provisioning master switch (DEFAULT OFF in production). Enabled + // here so this block's `async: true` cases exercise the async branch they assert on. + SERENITY_ASYNC_PROVISIONING_ENABLED: 'true', }; function buildContext() { diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index 9d1240e61e..1af5bebf28 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -115,7 +115,11 @@ function fakeContext({ data = undefined, brandId = BRAND, brand = makeBrandModel(), - env = {}, + // LLMO-7418: the global async-provisioning master switch is DEFAULT OFF in production, so an + // `async: true` request falls through to the synchronous branch unless it is enabled. Default it + // ON here so the existing async-path tests exercise the async branch they were written for; the + // OFF behaviour is covered by its own dedicated tests, which pass an env without this flag. + env = { SERENITY_ASYNC_PROVISIONING_ENABLED: 'true' }, promiseToken = undefined, headers = {}, } = {}) { @@ -1697,6 +1701,26 @@ describe('SerenityController', () => { expect(handlers.handleGetMarket).to.not.have.been.called; }); + it('createMarket runs the SYNCHRONOUS branch when the global async switch is off, even with async: true (LLMO-7418 master switch)', async () => { + // The master switch is DEFAULT OFF in production. An `async: true` request must then fall + // through to the synchronous branch — NOT error — so this whole stack can merge inert and + // async is turned on deliberately. This is what makes shipping safe by default. + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const ctx = fakeContext({ + env: {}, // switch absent => off + data: { + market: 'us', languageCode: 'en', brandDomain: 'x.com', brandNames: ['X'], async: true, + }, + }); + + const response = await controller.createMarket(ctx); + + // Synchronous path: not a 202, and no provisioning attempt or job was created. + expect(response.status).to.not.equal(202); + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('createMarket (PR-C) mints a provisioning attempt and enqueues the provision->create-market job chain, answering 202', async () => { const controller = SerenityController({ env: {} }, fakeLog(), {}); const ctx = fakeContext({ From 35d4e02898af4b45b1f4aa3ae39a0d45bab9d167 Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 21:24:37 +0300 Subject: [PATCH 10/11] test(serenity): pin the async master switch at all 6 gates, not just 1 The master-switch commit shipped an off-case test for createMarket only. The other five gates -- activate's three branches (pending->active, bare reactivation, project activation) and createBrandForOrg's two (initial-market create, bare create) -- relied on the shared helper without any test proving the gate is actually wired at that call site. That is the gap that matters for this switch specifically: its whole purpose is that the stack merges INERT, so an ungated site would flip real Serenity organizations onto the async path the moment the stack lands, silently, with a merge revert as the only way back. A gate is either present at every entry point or the "merges inert" claim is false. Each new test asserts the same contract the createMarket one does: with the switch off (its production default) an `async: true` request runs the exact SYNCHRONOUS branch the endpoint has always run -- it is not an error -- and mints no provisioning attempt and enqueues no job. Mutation-verified: stripping the gate expression from serenity.js fails all four serenity tests, and from brands.js fails both brands tests. serenity + brands suites: 717 passing. eslint clean. Co-Authored-By: Claude Opus 5 (1M context) --- test/controllers/brands.test.js | 60 +++++++++++++++++++++++++++++++ test/controllers/serenity.test.js | 55 ++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/test/controllers/brands.test.js b/test/controllers/brands.test.js index a2e7e07def..04668be930 100644 --- a/test/controllers/brands.test.js +++ b/test/controllers/brands.test.js @@ -5392,6 +5392,38 @@ describe('Brands Controller', () => { expect(enqueueStub).to.not.have.been.called; }); + it('runs the SYNCHRONOUS provisionBrandSubworkspace call when the global async switch is off, even with async: true (LLMO-7418 master switch)', async () => { + // The master switch is DEFAULT OFF in production. An `async: true` create must then fall + // through to the synchronous branch — NOT error — so this whole stack merges inert and + // async is turned on deliberately. `env: {}` on the context wins over mockEnv's `'true'`. + const provisionStub = sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-1' }); + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const beginStub = sinon.stub().resolves(true); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const controller = await buildController({ + provisionBrandSubworkspace: provisionStub, + upsertBrand: upsertStub, + beginProvisioningAttempt: beginStub, + createAndEnqueueJob: enqueueStub, + }); + + const response = await controller.createBrandForOrg({ + ...context, + env: {}, // switch absent => off + params: { spaceCatId: ORGANIZATION_ID }, + data: { ...semrushData }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + // Synchronous path: a 201 with the workspace already bound, not a 202 with a job id. + expect(response.status).to.equal(201); + expect(provisionStub.calledOnce).to.equal(true); + expect(upsertStub.firstCall.args[0].semrushSubWorkspaceId).to.equal('ws-1'); + expect(beginStub).to.not.have.been.called; + expect(enqueueStub).to.not.have.been.called; + }); + it('mints a provisioning attempt and enqueues the provision->create-market job chain, answering 202', async () => { const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand', status: 'pending' }); const beginStub = sinon.stub().resolves(true); @@ -5874,6 +5906,34 @@ describe('Brands Controller', () => { }); }); + it('Phase 4: bare create runs the SYNCHRONOUS provisionBrandSubworkspaceBare call when the global async switch is off, even with async: true (LLMO-7418 master switch)', async () => { + const bareStub = sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }); + const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); + const beginStub = sinon.stub().resolves(true); + const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' }); + const controller = await buildController({ + provisionBrandSubworkspaceBare: bareStub, + upsertBrand: upsertStub, + beginProvisioningAttempt: beginStub, + createAndEnqueueJob: enqueueStub, + }); + + const response = await controller.createBrandForOrg({ + ...context, + env: {}, // switch absent => off + params: { spaceCatId: ORGANIZATION_ID }, + data: { name: 'New Brand', baseSiteId: 'site-123', async: true }, + dataAccess: mockDataAccess, + attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } }, + }); + + expect(response.status).to.equal(201); + expect(bareStub.called).to.equal(true); + expect(upsertStub.firstCall.args[0].semrushSubWorkspaceId).to.equal('ws-bare'); + expect(beginStub).to.not.have.been.called; + expect(enqueueStub).to.not.have.been.called; + }); + it('Phase 4: bare create returns 409 without enqueuing when a provisioning attempt is already in flight', async () => { const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' }); const beginStub = sinon.stub().resolves(false); diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index 1af5bebf28..4eb6b6c0bc 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -2396,6 +2396,29 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('activate runs the SYNCHRONOUS market batch when the global async switch is off, even with async: true (LLMO-7418 master switch)', async () => { + // Mirrors createMarket's master-switch test, for activate's project-activation branch. + // The switch is DEFAULT OFF in production, so `async: true` must fall through to the + // synchronous orchestration rather than error — that is what lets the stack merge inert. + orchestrateActivateMarketsStub.resolves({ + status: 200, body: { brandId: BRAND, status: 'active', markets: [] }, + }); + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, + env: {}, // switch absent => off + data: { + brandDomain: 'x.com', brandNames: ['X'], markets: [{ market: 'us', languageCode: 'en' }], async: true, + }, + })); + + expect(response.status).to.equal(200); + expect(orchestrateActivateMarketsStub).to.have.been.calledOnce; + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('activate 400s when generatePrompts is true but there is no primary URL (nothing to generate into)', async () => { const brand = makeBrandModel({ getStatus: () => 'active' }); const controller = SerenityController({ env: {} }, fakeLog(), {}); @@ -2603,6 +2626,23 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('Phase 4: pending→active activation runs the SYNCHRONOUS flip when the global async switch is off, even with async: true (LLMO-7418 master switch)', async () => { + getBrandBaseSiteIdStub.resolves('primary-site'); + handlers.handleCreateMarketSubworkspace.resolves({ status: 201, body: {} }); + const brand = makeBrandModel({}); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, + env: {}, // switch absent => off + data: { brandNames: ['X'], async: true }, + })); + + expect(response.status).to.equal(200); + expect(ensureSubworkspaceStub).to.have.been.calledOnce; + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('Phase 4: bare reactivation mints a provisioning attempt and enqueues the provision->activate-brand-workspace job chain when async: true', async () => { const brand = makeBrandModel({ getStatus: () => 'active' }); const controller = SerenityController({ env: {} }, fakeLog(), {}); @@ -2679,6 +2719,21 @@ describe('SerenityController', () => { expect(createAndEnqueueJobStub).to.not.have.been.called; }); + it('Phase 4: bare reactivation runs the SYNCHRONOUS flip when the global async switch is off, even with async: true (LLMO-7418 master switch)', async () => { + const brand = makeBrandModel({ getStatus: () => 'active' }); + const controller = SerenityController({ env: {} }, fakeLog(), {}); + const response = await controller.activate(fakeContext({ + brand, + env: {}, // switch absent => off + data: { brandNames: ['X'], async: true }, + })); + + expect(response.status).to.equal(200); + expect(ensureSubworkspaceStub).to.have.been.calledOnce; + expect(beginProvisioningAttemptStub).to.not.have.been.called; + expect(createAndEnqueueJobStub).to.not.have.been.called; + }); + it('activate 400s when the markets array exceeds the cap (validated before either branch dispatches)', async () => { const markets = Array.from({ length: 51 }, (_, i) => ({ market: 'us', languageCode: `l${i}` })); const controller = SerenityController({ env: {} }, fakeLog(), {}); From b1c4843c9ec32f671d1a03b773928600d8332d8a Mon Sep 17 00:00:00 2001 From: Iris Alexandrescu Date: Thu, 10 Sep 2026 21:30:51 +0300 Subject: [PATCH 11/11] fix(serenity): one spelling per async-provisioning 409, registered in ERROR_CODES This stack introduced two new client-facing 409 conditions and spelled one of them two different ways. `serenity.js` raised `semrush_provisioning_in_progress` while `brands.js` raised `semrushProvisioningInProgress` for the exact same condition -- and each controller published its own spelling in its own OpenAPI document, so a client branching on the token handles one endpoint and silently falls through on the other. Neither spelling matched the repo's own convention: every other token is camelCase and lives in `ERROR_CODES`. Both are now registered there and referenced by constant at all 8 throw sites, so a third site cannot invent a third spelling. Snake_case is gone from the client-facing vocabulary. The two codes are documented as deliberately NOT interchangeable, because collapsing them is the mistake worth preventing: semrushProvisioningInProgress -- an attempt is live. Retryable; it converges. semrushProvisioningIncomplete -- provisioning is pending or FAILED, so the brand has no sub-workspace to write into. Retrying alone never converges. A client that treats the second as the first shows "please retry shortly" on a brand that will never come back. `semrush_workspace_id_conflict` stays snake_case deliberately: it is an internal control-flow code the provisioning worker catches, never an HTTP response token. Also documents createMarket's 409 properly. The B1 fix added the second, distinct conflict to that endpoint without touching its OpenAPI description, which still described only the pre-existing duplicate-market case. serenity + brands + brands-storage suites: 988 passing. eslint clean. `npm run docs:lint`: API description valid. Co-Authored-By: Claude Opus 5 (1M context) --- docs/openapi/serenity-api.yaml | 11 +++++++++-- src/controllers/brands.js | 8 +++++--- src/controllers/serenity.js | 10 +++++----- src/support/brands-storage.js | 9 +++++---- src/support/serenity/errors.js | 14 ++++++++++++++ test/controllers/serenity.test.js | 8 ++++---- test/support/brands-storage.test.js | 2 +- 7 files changed, 43 insertions(+), 19 deletions(-) diff --git a/docs/openapi/serenity-api.yaml b/docs/openapi/serenity-api.yaml index 727334309a..1f8eb3d57f 100644 --- a/docs/openapi/serenity-api.yaml +++ b/docs/openapi/serenity-api.yaml @@ -612,7 +612,14 @@ v2-serenity-markets: '404': description: Organization has no workspace, or serenity is not active for the organization. '409': - description: A market already exists for this (brandId, geoTargetId, languageCode) slice, or (async only) a provisioning attempt is already in flight for this brand. + description: >- + One of three conflicts, distinguished by the `error` token: (a) a market already exists + for this (brandId, geoTargetId, languageCode) slice; (b) `semrushProvisioningInProgress` + (`async: true` only) — a provisioning attempt is already in flight for this brand, so + retry shortly; or (c) `semrushProvisioningIncomplete` — the brand's Semrush sub-workspace + is pending or has failed to provision, so it has no workspace of its own to add a market + in. (b) converges on retry; (c) does not, and needs the brand's provisioning to complete + first. '502': description: Upstream returned a non-2xx response. content: @@ -1262,7 +1269,7 @@ v2-serenity-activate: misconfiguration: the brand's sub-workspace equals the organization parent workspace (refused to avoid acting on the shared parent pool); or (c) (`async: true` only, any branch) a provisioning attempt is already in - flight for this brand (`error: semrush_provisioning_in_progress`) — + flight for this brand (`error: semrushProvisioningInProgress`) — retry shortly. content: application/json: diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 907850a0fc..fc0687f4f7 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -88,7 +88,9 @@ import { } from '../support/serenity/mapping-rows.js'; import { propagateSiteUrlToSemrush } from '../support/serenity/site-url-propagation.js'; import { createSerenityTransport } from '../support/serenity/rest-transport.js'; -import { isSemrushTransportError, unwrapTransportCause } from '../support/serenity/errors.js'; +import { + ERROR_CODES, isSemrushTransportError, unwrapTransportCause, +} from '../support/serenity/errors.js'; import { logUpstreamError } from '../support/serenity/upstream-log.js'; import { buildBrandMarketsResponse } from '../support/serenity/brand-markets.js'; import { syncBrandUrlsAcrossMarkets } from '../support/serenity/brand-urls.js'; @@ -2121,7 +2123,7 @@ function BrandsController(ctx, log, env) { if (!began) { return createResponse( { - error: 'semrushProvisioningInProgress', + error: ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS, message: 'Unable to start Semrush provisioning for the new brand', }, 409, @@ -2224,7 +2226,7 @@ function BrandsController(ctx, log, env) { if (!began) { return createResponse( { - error: 'semrushProvisioningInProgress', + error: ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS, message: 'Unable to start Semrush provisioning for the new brand', }, 409, diff --git a/src/controllers/serenity.js b/src/controllers/serenity.js index d06522931a..8727205379 100644 --- a/src/controllers/serenity.js +++ b/src/controllers/serenity.js @@ -1092,7 +1092,7 @@ function SerenityController(context, log, env) { + 'brand; please retry shortly.', 409, ); - err.code = 'semrush_provisioning_in_progress'; + err.code = ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS; throw err; } // N2 note: no `title` is passed here deliberately. This branch runs only in @@ -1195,7 +1195,7 @@ function SerenityController(context, log, env) { + 'available until provisioning completes.', 409, ); - err.code = 'semrush_provisioning_incomplete'; + err.code = ERROR_CODES.SEMRUSH_PROVISIONING_INCOMPLETE; throw err; } // Flat handler self-derives brandDomain from siteId (it has Site access). @@ -1729,7 +1729,7 @@ function SerenityController(context, log, env) { + 'brand; please retry shortly.', 409, ); - err.code = 'semrush_provisioning_in_progress'; + err.code = ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS; throw err; } const job = await createAndEnqueueJob(ctx, { @@ -1876,7 +1876,7 @@ function SerenityController(context, log, env) { + 'brand; please retry shortly.', 409, ); - err.code = 'semrush_provisioning_in_progress'; + err.code = ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS; throw err; } const job = await createAndEnqueueJob(ctx, { @@ -2014,7 +2014,7 @@ function SerenityController(context, log, env) { + 'brand; please retry shortly.', 409, ); - err.code = 'semrush_provisioning_in_progress'; + err.code = ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS; throw err; } const job = await createAndEnqueueJob(ctx, { diff --git a/src/support/brands-storage.js b/src/support/brands-storage.js index b1cf2d861b..46a70ca971 100644 --- a/src/support/brands-storage.js +++ b/src/support/brands-storage.js @@ -20,6 +20,7 @@ import { sanitizeGuidanceText, } from './brand-guidance.js'; import { readFeatureFlagScopes, resolveFlagRowForBrand } from './feature-flags-storage.js'; +import { ERROR_CODES } from './serenity/errors.js'; import { SERENITY_FEATURE_FLAG_NAME, SERENITY_FEATURE_FLAG_PRODUCT, @@ -2338,9 +2339,9 @@ export const PROVISIONING_STALE_THRESHOLD_MS = 10 * 60 * 1000; * @param {object} postgrestClient * @param {object} [log] * @throws when a fresh attempt is genuinely in flight (`err.status = 409`, - * `err.code = 'semrush_provisioning_in_progress'`), or when the provisioning-state read itself - * fails for a reason OTHER than the columns not existing yet (see below) — an unreadable state - * must never be silently treated as an empty one. + * `err.code = ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS`), or when the provisioning-state + * read itself fails for a reason OTHER than the columns not existing yet (see below) — an + * unreadable state must never be silently treated as an empty one. */ export async function guardAgainstConcurrentProvisioning(brandId, postgrestClient, log) { // POSTGRES_UNDEFINED_COLUMN (LLMO-7418 external-review Finding 1): this guard defends against @@ -2383,7 +2384,7 @@ export async function guardAgainstConcurrentProvisioning(brandId, postgrestClien 'A Semrush sub-workspace provisioning attempt is already in progress for this brand; please retry shortly.', 409, ); - err.code = 'semrush_provisioning_in_progress'; + err.code = ERROR_CODES.SEMRUSH_PROVISIONING_IN_PROGRESS; throw err; } diff --git a/src/support/serenity/errors.js b/src/support/serenity/errors.js index ec366d646a..15f0e7028f 100644 --- a/src/support/serenity/errors.js +++ b/src/support/serenity/errors.js @@ -178,6 +178,20 @@ export const ERROR_CODES = Object.freeze({ // main_brand:true) could not be established. Surfaced via // MainBrandBenchmarkInvariantError, below. MAIN_BRAND_BENCHMARK_INVARIANT: 'mainBrandBenchmarkInvariant', + // Async sub-workspace provisioning (LLMO-7352/LLMO-7418). Both are 409s, and both are raised + // from BOTH serenity.js and brands.js, so they live here rather than as string literals at each + // throw site -- the two controllers previously spelled the in-progress one two different ways + // (`semrush_provisioning_in_progress` vs `semrushProvisioningInProgress`) and published both + // spellings in their respective OpenAPI documents, which no client could branch on reliably. + // + // They are NOT interchangeable, which is why there are two: + // IN_PROGRESS -- an attempt is live right now. RETRYABLE: wait and retry, it will converge. + // INCOMPLETE -- provisioning is pending or has FAILED, so the brand has no sub-workspace of + // its own to write into. NOT retryable on its own; the brand's provisioning + // has to complete (or be retried) first. + // A client that collapses them shows "please retry shortly" on a brand that will never converge. + SEMRUSH_PROVISIONING_IN_PROGRESS: 'semrushProvisioningInProgress', + SEMRUSH_PROVISIONING_INCOMPLETE: 'semrushProvisioningIncomplete', }); /** diff --git a/test/controllers/serenity.test.js b/test/controllers/serenity.test.js index 4eb6b6c0bc..a37cec5b23 100644 --- a/test/controllers/serenity.test.js +++ b/test/controllers/serenity.test.js @@ -1828,7 +1828,7 @@ describe('SerenityController', () => { 'A Semrush sub-workspace provisioning attempt is already in progress for this brand; please retry shortly.', 409, ); - conflictErr.code = 'semrush_provisioning_in_progress'; + conflictErr.code = 'semrushProvisioningInProgress'; guardAgainstConcurrentProvisioningStub.rejects(conflictErr); const controller = SerenityController({ env: {} }, fakeLog(), {}); const response = await controller.createMarket(fakeContext({ @@ -2511,7 +2511,7 @@ describe('SerenityController', () => { 'A Semrush sub-workspace provisioning attempt is already in progress for this brand; please retry shortly.', 409, ); - conflictErr.code = 'semrush_provisioning_in_progress'; + conflictErr.code = 'semrushProvisioningInProgress'; guardAgainstConcurrentProvisioningStub.rejects(conflictErr); getBrandBaseSiteIdStub.resolves('primary-site'); const brand = makeBrandModel({}); @@ -2519,7 +2519,7 @@ describe('SerenityController', () => { const response = await controller.activate(fakeContext({ brand, data: { brandNames: ['X'] } })); expect(response.status).to.equal(409); const { error } = await readBody(response); - expect(error).to.equal('semrush_provisioning_in_progress'); + expect(error).to.equal('semrushProvisioningInProgress'); expect(ensureSubworkspaceStub).to.not.have.been.called; expect(brand.setStatus).to.not.have.been.called; }); @@ -2529,7 +2529,7 @@ describe('SerenityController', () => { 'A Semrush sub-workspace provisioning attempt is already in progress for this brand; please retry shortly.', 409, ); - conflictErr.code = 'semrush_provisioning_in_progress'; + conflictErr.code = 'semrushProvisioningInProgress'; guardAgainstConcurrentProvisioningStub.rejects(conflictErr); const brand = makeBrandModel({ getStatus: () => 'active' }); const controller = SerenityController({ env: {} }, fakeLog(), {}); diff --git a/test/support/brands-storage.test.js b/test/support/brands-storage.test.js index 8d4687959a..b2f685db44 100644 --- a/test/support/brands-storage.test.js +++ b/test/support/brands-storage.test.js @@ -4949,7 +4949,7 @@ describe('brands-storage', () => { expect(caught).to.exist; expect(caught.status).to.equal(409); - expect(caught.code).to.equal('semrush_provisioning_in_progress'); + expect(caught.code).to.equal('semrushProvisioningInProgress'); }); it('reconciles a STALE attempt to failed and returns without throwing', async () => {