feat(serenity): opt-in async provisioning for the 3 remaining skip-mode call sites (LLMO-7352/LLMO-7418 Phase 4) by IrisAlexandrescu · Pull Request #3249 · adobe/spacecat-api-service

IrisAlexandrescu · 2026-09-09T13:45:11Z

Summary

Fourth PR in the LLMO-7352/LLMO-7418 stack (after #3223, #3233, #3246). Extends the opt-in async: true contract 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.

Important scoping note, verified against the actual code (not assumed from the ticket): these 3 sites use createReadiness: 'skip' — they persist the workspace pointer immediately after a single create call, without ever polling for settle. They therefore never had the in-request settle-poll latency/timeout problem PR-A/PR-B/PR-C fixed for the other 2 sites (createMarket, activate's project-activation branch). This PR is deliberate hardening + consistency, not a fix for a still-open incident — the original LLMO-7352 symptom (a brand silently, permanently bound to a broken workspace) is already fully closed by PR-A alone: any workspace pointer's first real use (e.g. adding a market) always runs the now-fail-fast pollUntilCreated before doing anything else, so a skip-mode-created pointer that later turns out broken surfaces cleanly on first use today, regardless of this PR.

What this PR still adds: not persisting a doomed pointer proactively, protection against a slow/hanging single Semrush create call, and uniformity across all 5 ensureSubworkspace call sites instead of 2-of-5.

Changes

createBrandForOrg's bare-create branch mirrors its sibling hasSemrushMarket branch (already async-converted in feat(serenity): opt-in async provisioning for createMarket, createBrandForOrg, activate (LLMO-7352/LLMO-7418) #3246) exactly: async: true persists the row first, then hands off to provision-workspace-job — no chained job, since a bare create has no project to create afterward.
activate's two skip-mode branches each mint a provisioning attempt and chain to a new activate-brand-workspace-job.js, which does the one piece of business logic the generic provisioning worker doesn't already handle: 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, matching the sync branch's own contract) from a no-op reactivation re-affirm (207).
The existing guardAgainstConcurrentProvisioning calls on the synchronous branches are unchanged — still needed to protect a sync caller against a concurrent async attempt from another endpoint on the same brand.
OpenAPI updated for both endpoints to reflect async: true now applying to every branch, not just the 2 already converted.

Test plan

npx eslint, npm run type-check, npm run docs:lint — all clean
30 new unit tests (8 in serenity.test.js, 3 in brands.test.js, 5 in activate-brand-workspace-job.test.js, plus regression coverage): malformed-async 400, explicit-false regression (unchanged sync behavior), 202+job-chain wiring for both branches, 409-on-concurrent-attempt, save-divergence 502 vs 207 parity with the sync branches
Full targeted suite: 954 passing, 0 regressions

🤖 Generated with Claude Code

codecov · 2026-09-09T13:56:02Z

Codecov Report

❌ Patch coverage is 97.27273% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/controllers/brands.js	94.15%	9 Missing ⚠️
src/controllers/serenity.js	96.62%	6 Missing ⚠️

📢 Thoughts on this report? Let us know!

IrisAlexandrescu · 2026-09-09T14:51:43Z

4-agent adversarial review (architect / race-condition / correctness / test-coverage) found one real bug, now fixed:

activate-brand-workspace-job.js's status-flip (brand.setStatus('active'); brand.save()) was both redundant AND unguarded. promoteProvisioningReady (the hop immediately before this chain is enqueued) already durably flips the brand to active as part of the SAME atomic CAS write that promotes the sub-workspace to ready — the chain is only ever enqueued once that write has succeeded. If this chained job ran minutes later (self-requeue backoff, plain SQS delivery latency) and a legitimate concurrent POST /serenity/deactivate had since moved the brand back to pending, the old code would have blindly re-activated it — reaching the exact "active brand bound to a workspace nobody meant it to have" failure class this whole epic exists to prevent, through a new door.

Fix: the handler is now read-only — reports 200/active when the brand is (still) active, and reports the brand's actual current status via 207 (not a manufactured 502) when it's found otherwise, since the sub-workspace itself did provision successfully in that case.

Also added the missing async compensation-path tests for the bare-create branch (mirroring its sibling's existing coverage) per the test-coverage review.

960 tests passing, 0 regressions, lint/type-check/docs:lint all clean.

github-actions · 2026-09-09T15:09:37Z

This PR will trigger a minor release when merged.

…de call sites (LLMO-7352/LLMO-7418 Phase 4) 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 <noreply@anthropic.com>

…te-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 <noreply@anthropic.com>

…(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 <noreply@anthropic.com>

…workspace title (LLMO-7418 external-review Finding 4) 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 <noreply@anthropic.com>

…rovisioning 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 <noreply@anthropic.com>

…, unguarded findById, stale OpenAPI docs) - 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 <noreply@anthropic.com>

…pace status poll (LLMO-7418 external-review Finding 14) 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 <noreply@anthropic.com>

…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>

…(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) <noreply@anthropic.com>

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) <noreply@anthropic.com>

… 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) <noreply@anthropic.com>

IrisAlexandrescu mentioned this pull request Sep 9, 2026

feat(serenity): expose semrush provisioning status on brand reads (LLMO-7352/LLMO-7418) #3250

Open

3 tasks

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from 57504cb to 2d095bd Compare September 9, 2026 16:14

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch 2 times, most recently from a964882 to 135eec6 Compare September 9, 2026 17:10

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from fafceee to 6064817 Compare September 9, 2026 17:24

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch from 135eec6 to e888c2a Compare September 9, 2026 17:29

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from 6064817 to 99e6696 Compare September 9, 2026 18:00

IrisAlexandrescu temporarily deployed to dev-branches September 9, 2026 18:02 — with GitHub Actions Inactive

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from 8e279ab to 553f05a Compare September 9, 2026 18:25

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch from e888c2a to f822111 Compare September 9, 2026 18:42

IrisAlexandrescu temporarily deployed to dev-branches September 9, 2026 18:59 — with GitHub Actions Inactive

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from 553f05a to ab699df Compare September 10, 2026 08:00

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch from f822111 to a587be3 Compare September 10, 2026 08:17

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 08:24 — with GitHub Actions Inactive

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 08:55 — with GitHub Actions Inactive

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 09:19 — with GitHub Actions Inactive

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from ab699df to aae939f Compare September 10, 2026 10:23

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch from b0a89f6 to 90986fd Compare September 10, 2026 10:23

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 10:37 — with GitHub Actions Inactive

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from aae939f to 3a1ffe9 Compare September 10, 2026 11:17

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch from 90986fd to f77926a Compare September 10, 2026 11:17

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 11:26 — with GitHub Actions Inactive

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 16:19 — with GitHub Actions Inactive

Iris Alexandrescu and others added 4 commits September 10, 2026 20:49

Iris Alexandrescu and others added 5 commits September 10, 2026 20:49

IrisAlexandrescu force-pushed the feat/LLMO-7418-phase4-skip-mode-async branch from af1cbe8 to fe80d7d Compare September 10, 2026 17:54

IrisAlexandrescu had a problem deploying to dev-branches September 10, 2026 18:30 — with GitHub Actions Error

IrisAlexandrescu deployed to dev-branches September 10, 2026 18:46 — with GitHub Actions Active

Navigation Menu

Uh oh!

feat(serenity): opt-in async provisioning for the 3 remaining skip-mode call sites (LLMO-7352/LLMO-7418 Phase 4) - #3249

feat(serenity): opt-in async provisioning for the 3 remaining skip-mode call sites (LLMO-7352/LLMO-7418 Phase 4)#3249
IrisAlexandrescu wants to merge 11 commits into
feat/LLMO-7418-async-provisioning-endpointsfrom
feat/LLMO-7418-phase4-skip-mode-async

IrisAlexandrescu commented Sep 9, 2026

Uh oh!

codecov Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

IrisAlexandrescu commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

IrisAlexandrescu commented Sep 9, 2026

Summary

Changes

Test plan

Uh oh!

codecov Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

IrisAlexandrescu commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Sep 9, 2026 •

edited

Loading