Skip to content

Commit b0a89f6

Browse files
Iris Alexandrescuclaude
andcommitted
fix(serenity): add a server-side kill switch for async provisioning (LLMO-7418 external-review Finding 15)
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 <noreply@anthropic.com>
1 parent fbb8def commit b0a89f6

5 files changed

Lines changed: 225 additions & 2 deletions

File tree

src/controllers/brands.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ import {
9898
isSerenityActiveForBrand,
9999
isSerenityActiveForOrg,
100100
isSerenityUiActiveForOrg,
101+
isAsyncProvisioningKillSwitched,
101102
} from '../support/serenity/serenity-active.js';
102103
import {
103104
buildReservedIdentities,
@@ -1870,6 +1871,18 @@ function BrandsController(ctx, log, env) {
18701871
// is NOT permanent: the synchronous branch is the LLMO-7352 bug pattern itself, slated
18711872
// for removal once every known caller has migrated to `async: true`.
18721873
if (validateAsync(brandData)) {
1874+
// LLMO-7418 external-review Finding 15: server-side kill switch — lets ops disable
1875+
// the async path for this organization without a deploy if it misbehaves in
1876+
// production. The caller falls back to the synchronous path on its own retry.
1877+
if (await isAsyncProvisioningKillSwitched(context, spaceCatId, log)) {
1878+
return createResponse(
1879+
{
1880+
error: 'asyncProvisioningDisabled',
1881+
message: 'Async provisioning is temporarily disabled for this organization; retry without async: true',
1882+
},
1883+
503,
1884+
);
1885+
}
18731886
// brandAliases/urls/competitors are NOT read here (unlike the sync branch below): the
18741887
// brand row this section persists below (upsertBrand) writes them to storage, and the
18751888
// async chain's orchestration reads them back from there — the same DB-backed source
@@ -1948,6 +1961,17 @@ function BrandsController(ctx, log, env) {
19481961
// sub-workspace provisioning off to provision-workspace-job — no chained job, since a
19491962
// bare create has no project to create once the workspace is ready.
19501963
//
1964+
// LLMO-7418 external-review Finding 15: server-side kill switch — see the
1965+
// hasSemrushMarket branch above for rationale.
1966+
if (await isAsyncProvisioningKillSwitched(context, spaceCatId, log)) {
1967+
return createResponse(
1968+
{
1969+
error: 'asyncProvisioningDisabled',
1970+
message: 'Async provisioning is temporarily disabled for this organization; retry without async: true',
1971+
},
1972+
503,
1973+
);
1974+
}
19511975
// Resolved and validated HERE, before any write — same rationale as the
19521976
// hasSemrushMarket branch: a missing org workspace config must never leave a
19531977
// persisted, permanently-inert brand row behind.

src/controllers/serenity.js

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ import {
9292
handleTagImpactSubworkspace,
9393
} from '../support/serenity/handlers/tags.js';
9494
import { ensureSubworkspace, decommissionBrandWorkspace } from '../support/serenity/workspace-lifecycle.js';
95-
import { isSerenityActiveForBrand } from '../support/serenity/serenity-active.js';
95+
import {
96+
isSerenityActiveForBrand,
97+
isAsyncProvisioningKillSwitched,
98+
} from '../support/serenity/serenity-active.js';
9699
import { marketForGeoTargetId } from '../support/serenity/locations.js';
97100
import { brandNeedles, classifyBrandedTag } from '../support/serenity/branded-classifier.js';
98101
import { computeWriteDeadline } from '../support/serenity/intent-classification.js';
@@ -1045,6 +1048,18 @@ function SerenityController(context, log, env) {
10451048
// synchronous branch is the LLMO-7352 bug pattern itself, not a valid alternative, and
10461049
// is slated for removal once every known caller has migrated to `async: true`.
10471050
if (validateAsync(requestBody)) {
1051+
// LLMO-7418 external-review Finding 15: server-side kill switch — lets ops disable
1052+
// the async path for this organization without a deploy if it misbehaves in
1053+
// production. The caller falls back to the synchronous path on its own retry.
1054+
if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) {
1055+
return createResponse(
1056+
{
1057+
error: 'asyncProvisioningDisabled',
1058+
message: 'Async provisioning is temporarily disabled for this organization; retry without async: true',
1059+
},
1060+
503,
1061+
);
1062+
}
10481063
// The worker's existing-pointer fast path (provision-workspace-job.js) polls THIS
10491064
// brand's already-canonical workspace rather than provisioning a new one — every
10501065
// brand reaching this branch already has one (`auth.mode === 'subworkspace'` IS that
@@ -1641,6 +1656,17 @@ function SerenityController(context, log, env) {
16411656
// `provision-workspace-job` ->
16421657
// `serenity-activate-brand-workspace` job chain instead.
16431658
if (validateAsync(body)) {
1659+
// LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's
1660+
// async branch for the full rationale.
1661+
if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) {
1662+
return createResponse(
1663+
{
1664+
error: 'asyncProvisioningDisabled',
1665+
message: 'Async provisioning is temporarily disabled for this organization; retry without async: true',
1666+
},
1667+
503,
1668+
);
1669+
}
16441670
// LLMO-7418 external-review Finding 9: see createMarket's async branch for the full
16451671
// rationale — reconcile a stale in-flight attempt (reusing the sync guard's own logic)
16461672
// before minting a new one, since beginProvisioningAttempt's own CAS has no staleness
@@ -1779,6 +1805,17 @@ function SerenityController(context, log, env) {
17791805
// activate-brand-workspace-job.js's save-divergence handling matches this branch's own
17801806
// 207-not-502 contract.
17811807
if (validateAsync(body)) {
1808+
// LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's
1809+
// async branch for the full rationale.
1810+
if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) {
1811+
return createResponse(
1812+
{
1813+
error: 'asyncProvisioningDisabled',
1814+
message: 'Async provisioning is temporarily disabled for this organization; retry without async: true',
1815+
},
1816+
503,
1817+
);
1818+
}
17821819
// LLMO-7418 external-review Finding 9: see createMarket's async branch (and the
17831820
// wasPending branch above) for the full rationale.
17841821
await guardAgainstConcurrentProvisioning(
@@ -1905,6 +1942,17 @@ function SerenityController(context, log, env) {
19051942
// in-request settle-poll + project-create/publish sequence), slated for removal once every
19061943
// known caller has migrated to `async: true`.
19071944
if (validateAsync(body)) {
1945+
// LLMO-7418 external-review Finding 15: server-side kill switch — see createMarket's
1946+
// async branch for the full rationale.
1947+
if (await isAsyncProvisioningKillSwitched(ctx, ctx?.params?.spaceCatId, log)) {
1948+
return createResponse(
1949+
{
1950+
error: 'asyncProvisioningDisabled',
1951+
message: 'Async provisioning is temporarily disabled for this organization; retry without async: true',
1952+
},
1953+
503,
1954+
);
1955+
}
19081956
// LLMO-7418 external-review Finding 9: see the createMarket async branch above for the
19091957
// full rationale — reconcile a stale in-flight attempt before minting a new one, since
19101958
// beginProvisioningAttempt's own CAS has no staleness awareness.

src/support/serenity/serenity-active.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,17 @@ export const SERENITY_FEATURE_FLAG_NAME = 'serenity';
4747
*/
4848
export const SERENITY_UI_FEATURE_FLAG_NAME = 'serenity_ui';
4949

50+
/**
51+
* Server-side kill switch for the opt-in `async: true` provisioning path (LLMO-7418
52+
* external-review Finding 15). Unlike `SERENITY_FEATURE_FLAG_NAME` above (an opt-IN
53+
* rollout switch, off by default), this is an opt-OUT switch: the async path stays
54+
* available by default, and an explicit `true` row disables it org-wide. Lets ops
55+
* flip async provisioning off for one organization without a deploy, if it
56+
* misbehaves in production — there was previously no way to do this short of a
57+
* code change reverting every `async: true` call site at once.
58+
*/
59+
export const ASYNC_PROVISIONING_KILL_SWITCH_FLAG_NAME = 'serenity_async_provisioning_disabled';
60+
5061
/**
5162
* Module-scoped TTL+size-bounded cache, mirroring the workspace-resolver cache
5263
* (warm Lambda containers reuse module state, so a Map here amortises the
@@ -239,3 +250,34 @@ export async function isSerenityUiActiveForOrg(ctx, spaceCatId, log) {
239250
const scopes = await readCachedFlagScopes(ctx, spaceCatId, SERENITY_UI_FEATURE_FLAG_NAME, log);
240251
return scopes?.orgRow?.flag_value === true;
241252
}
253+
254+
/**
255+
* LLMO-7418 external-review Finding 15: server-side kill switch for the opt-in
256+
* `async: true` provisioning path. Reads the org-wide
257+
* `LLMO/serenity_async_provisioning_disabled` feature flag (cached, same
258+
* machinery as every other flag in this file).
259+
*
260+
* Deliberately reuses the SAME "absent/unreadable resolves to `false`"
261+
* fail-safe shape as every other predicate here — it happens to be the safe
262+
* default in both directions: for a rollout flag, `false` means "stay off";
263+
* for this kill switch, `false` means "stay on" (async provisioning
264+
* available). A transient PostgREST read failure must never silently disable
265+
* async provisioning org-wide, so this is NOT inverted to fail closed.
266+
*
267+
* @param {object} ctx - Request context (uses
268+
* `ctx.dataAccess.services.postgrestClient`).
269+
* @param {string} spaceCatId - SpaceCat organization UUID.
270+
* @param {object} [log] - Optional logger (used to surface a missing client /
271+
* a read error without throwing on this hot path).
272+
* @returns {Promise<boolean>} `true` only when the flag is explicitly on
273+
* (async provisioning is disabled for this organization).
274+
*/
275+
export async function isAsyncProvisioningKillSwitched(ctx, spaceCatId, log) {
276+
const scopes = await readCachedFlagScopes(
277+
ctx,
278+
spaceCatId,
279+
ASYNC_PROVISIONING_KILL_SWITCH_FLAG_NAME,
280+
log,
281+
);
282+
return scopes?.orgRow?.flag_value === true;
283+
}

test/controllers/brands.test.js

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5319,6 +5319,9 @@ describe('Brands Controller', () => {
53195319
createAndEnqueueJob = sinon.stub().resolves({ getId: () => 'job-abc' }),
53205320
promoteProvisioningFailed = sinon.stub().resolves(true),
53215321
updateProvisioningJobId = sinon.stub().resolves(true),
5322+
// LLMO-7418 external-review Finding 15: kill switch off by default (async available);
5323+
// specific tests override it to resolve(true) to exercise the 503 gate.
5324+
isAsyncProvisioningKillSwitched = sinon.stub().resolves(false),
53225325
} = {}) {
53235326
const Mocked = await esmock('../../src/controllers/brands.js', {
53245327
'../../src/support/serenity/brand-provisioning.js': {
@@ -5328,7 +5331,10 @@ describe('Brands Controller', () => {
53285331
provisionBrandSubworkspaceBare:
53295332
provisionBrandSubworkspaceBare || sinon.stub().resolves({ semrushSubWorkspaceId: 'ws-bare' }),
53305333
},
5331-
'../../src/support/serenity/serenity-active.js': { isSerenityActiveForOrg },
5334+
'../../src/support/serenity/serenity-active.js': {
5335+
isSerenityActiveForOrg,
5336+
isAsyncProvisioningKillSwitched,
5337+
},
53325338
'../../src/support/serenity/workspace-resolver.js': { resolveWorkspaceId },
53335339
'../../src/support/serenity/async-job-runner.js': { createAndEnqueueJob },
53345340
'../../src/support/serenity/handlers/provision-workspace-job.js': {
@@ -5519,6 +5525,28 @@ describe('Brands Controller', () => {
55195525
expect(enqueueStub).to.not.have.been.called;
55205526
});
55215527

5528+
it('returns 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => {
5529+
const beginStub = sinon.stub().resolves(true);
5530+
const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' });
5531+
const controller = await buildController({
5532+
beginProvisioningAttempt: beginStub,
5533+
createAndEnqueueJob: enqueueStub,
5534+
isAsyncProvisioningKillSwitched: sinon.stub().resolves(true),
5535+
});
5536+
5537+
const response = await controller.createBrandForOrg({
5538+
...context,
5539+
params: { spaceCatId: ORGANIZATION_ID },
5540+
data: { ...semrushData },
5541+
dataAccess: mockDataAccess,
5542+
attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } },
5543+
});
5544+
5545+
expect(response.status).to.equal(503);
5546+
expect(beginStub).to.not.have.been.called;
5547+
expect(enqueueStub).to.not.have.been.called;
5548+
});
5549+
55225550
it('returns 400 without persisting a row when the organization has no Semrush parent workspace configured', async () => {
55235551
const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' });
55245552
const controller = await buildController({
@@ -5863,6 +5891,28 @@ describe('Brands Controller', () => {
58635891
expect(enqueueStub.called).to.equal(false);
58645892
});
58655893

5894+
it('Phase 4: bare create returns 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => {
5895+
const beginStub = sinon.stub().resolves(true);
5896+
const enqueueStub = sinon.stub().resolves({ getId: () => 'job-xyz' });
5897+
const controller = await buildController({
5898+
beginProvisioningAttempt: beginStub,
5899+
createAndEnqueueJob: enqueueStub,
5900+
isAsyncProvisioningKillSwitched: sinon.stub().resolves(true),
5901+
});
5902+
5903+
const response = await controller.createBrandForOrg({
5904+
...context,
5905+
params: { spaceCatId: ORGANIZATION_ID },
5906+
data: { name: 'New Brand', baseSiteId: 'site-123', async: true },
5907+
dataAccess: mockDataAccess,
5908+
attributes: { authInfo: { getType: () => 'ims', profile: { email: 'user@test.com' } } },
5909+
});
5910+
5911+
expect(response.status).to.equal(503);
5912+
expect(beginStub.called).to.equal(false);
5913+
expect(enqueueStub.called).to.equal(false);
5914+
});
5915+
58665916
it('Phase 4: bare create returns 400 without persisting a row when the organization has no Semrush parent workspace configured', async () => {
58675917
const upsertStub = sinon.stub().resolves({ id: 'forced-id', name: 'New Brand' });
58685918
const controller = await buildController({

test/controllers/serenity.test.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ describe('SerenityController', () => {
199199
let resolveWorkspaceIdStub;
200200
let resolveBrandWorkspaceStub;
201201
let isSerenityActiveStub;
202+
let isAsyncProvisioningKillSwitchedStub;
202203
let createTransportStub;
203204
let resolveBrandUuidStub;
204205
let getBrandAliasesStub;
@@ -237,6 +238,9 @@ describe('SerenityController', () => {
237238
// existing assertion that drives a brand-level route reaches its handler.
238239
// The "serenity inactive" describe overrides this to false.
239240
isSerenityActiveStub = sinon.stub().resolves(true);
241+
// LLMO-7418 external-review Finding 15: kill switch off by default (async provisioning
242+
// available); specific tests override it to resolve(true) to exercise the 503 gate.
243+
isAsyncProvisioningKillSwitchedStub = sinon.stub().resolves(false);
240244
decommissionStub = sinon.stub().resolves();
241245
ensureSubworkspaceStub = sinon.stub().resolves(SUBWS);
242246
clearBrandWorkspaceCacheStub = sinon.stub();
@@ -352,6 +356,7 @@ describe('SerenityController', () => {
352356
},
353357
'../../src/support/serenity/serenity-active.js': {
354358
isSerenityActiveForBrand: isSerenityActiveStub,
359+
isAsyncProvisioningKillSwitched: isAsyncProvisioningKillSwitchedStub,
355360
},
356361
'../../src/support/access-control-util.js': MockAccessControlUtil,
357362
'../../src/support/prompts-storage.js': {
@@ -1711,6 +1716,20 @@ describe('SerenityController', () => {
17111716
expect(createAndEnqueueJobStub).to.not.have.been.called;
17121717
});
17131718

1719+
it('createMarket answers 503 without enqueuing anything when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => {
1720+
isAsyncProvisioningKillSwitchedStub.resolves(true);
1721+
const controller = SerenityController({ env: {} }, fakeLog(), {});
1722+
const response = await controller.createMarket(fakeContext({
1723+
data: {
1724+
market: 'us', languageCode: 'en', brandDomain: 'x.com', brandNames: ['X'], async: true,
1725+
},
1726+
}));
1727+
expect(response.status).to.equal(503);
1728+
expect(guardAgainstConcurrentProvisioningStub).to.not.have.been.called;
1729+
expect(beginProvisioningAttemptStub).to.not.have.been.called;
1730+
expect(createAndEnqueueJobStub).to.not.have.been.called;
1731+
});
1732+
17141733
it('createMarket runs the SAME synchronous orchestration it always has when async is absent (regression: default behavior unchanged)', async () => {
17151734
orchestrateCreateMarketSubworkspaceStub.resolves({
17161735
status: 201, body: { brandId: BRAND, geoTargetId: 2840, languageCode: 'en' },
@@ -2262,6 +2281,21 @@ describe('SerenityController', () => {
22622281
expect(createAndEnqueueJobStub).to.not.have.been.called;
22632282
});
22642283

2284+
it('activate answers 503 without enqueuing when async: true and the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => {
2285+
isAsyncProvisioningKillSwitchedStub.resolves(true);
2286+
const brand = makeBrandModel({ getStatus: () => 'active' });
2287+
const controller = SerenityController({ env: {} }, fakeLog(), {});
2288+
const response = await controller.activate(fakeContext({
2289+
brand,
2290+
data: {
2291+
brandDomain: 'x.com', brandNames: ['X'], markets: [{ market: 'us', languageCode: 'en' }], async: true,
2292+
},
2293+
}));
2294+
expect(response.status).to.equal(503);
2295+
expect(beginProvisioningAttemptStub).to.not.have.been.called;
2296+
expect(createAndEnqueueJobStub).to.not.have.been.called;
2297+
});
2298+
22652299
it('activate 400s when async is present but not a boolean', async () => {
22662300
const brand = makeBrandModel({ getStatus: () => 'active' });
22672301
const controller = SerenityController({ env: {} }, fakeLog(), {});
@@ -2462,6 +2496,19 @@ describe('SerenityController', () => {
24622496
expect(createAndEnqueueJobStub).to.not.have.been.called;
24632497
});
24642498

2499+
it('Phase 4: pending→active activation answers 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => {
2500+
getBrandBaseSiteIdStub.resolves('primary-site');
2501+
isAsyncProvisioningKillSwitchedStub.resolves(true);
2502+
const brand = makeBrandModel({});
2503+
const controller = SerenityController({ env: {} }, fakeLog(), {});
2504+
const response = await controller.activate(fakeContext({
2505+
brand, data: { brandNames: ['X'], async: true },
2506+
}));
2507+
expect(response.status).to.equal(503);
2508+
expect(beginProvisioningAttemptStub).to.not.have.been.called;
2509+
expect(createAndEnqueueJobStub).to.not.have.been.called;
2510+
});
2511+
24652512
it('Phase 4: pending→active activation 400s when async is present but not a boolean', async () => {
24662513
getBrandBaseSiteIdStub.resolves('primary-site');
24672514
const brand = makeBrandModel({});
@@ -2529,6 +2576,18 @@ describe('SerenityController', () => {
25292576
expect(createAndEnqueueJobStub).to.not.have.been.called;
25302577
});
25312578

2579+
it('Phase 4: bare reactivation answers 503 without enqueuing when the async kill switch is on (LLMO-7418 external-review Finding 15)', async () => {
2580+
isAsyncProvisioningKillSwitchedStub.resolves(true);
2581+
const brand = makeBrandModel({ getStatus: () => 'active' });
2582+
const controller = SerenityController({ env: {} }, fakeLog(), {});
2583+
const response = await controller.activate(fakeContext({
2584+
brand, data: { brandNames: ['X'], async: true },
2585+
}));
2586+
expect(response.status).to.equal(503);
2587+
expect(beginProvisioningAttemptStub).to.not.have.been.called;
2588+
expect(createAndEnqueueJobStub).to.not.have.been.called;
2589+
});
2590+
25322591
it('Phase 4: bare reactivation 400s when async is present but not a boolean', async () => {
25332592
const brand = makeBrandModel({ getStatus: () => 'active' });
25342593
const controller = SerenityController({ env: {} }, fakeLog(), {});

0 commit comments

Comments
 (0)