feat(serenity): opt-in async provisioning for createMarket, createBrandForOrg, activate (LLMO-7352/LLMO-7418) by IrisAlexandrescu · Pull Request #3246 · adobe/spacecat-api-service

IrisAlexandrescu · 2026-09-09T11:12:52Z

Summary

Third in the stack (base: #3233). Adds the producers for the worker #3233 introduced: createMarket, createBrandForOrg and activate gain an opt-in async: true flag that hands provisioning off to a provision-workspace-job → chained-job pipeline instead of running it inline.

Opt-in only. Absent or false runs the exact synchronous path each endpoint has always run. No shipped caller sends it in this PR.

⚠️ Reviewers: read the "Integration with #3252" section below. This PR changes behaviour inside the prompt-generation feature's own code path, and that is the part most worth a careful look.

Changes

async: true on the three endpoints; a 202 carries a job id to poll at GET .../serenity/jobs/{jobId}.
Job-status polling follows the chain to its effective terminal hop, so COMPLETED means the market/activation actually ran, not merely that the sub-workspace became ready.
The synchronous branches gain a concurrency guard: a sync createMarket can no longer race an async activate on the same brand.
Add Market is refused on a mid-provisioning brand (409). Previously such a brand looked "flat" and the market would have been created in the shared organization workspace — consuming the org's allocation, bound in the DB to this brand, and invisible the moment the real pointer landed.
Deactivate re-reads the workspace pointer after cancelling, so a promote that lands mid-flight is still decommissioned.

Integration with #3252 (async Semrush-market AI-prompt provisioning)

Both features convert the same endpoints to async for different reasons — #3252 moves prompt generation off the request path to fix prompt quality; this stack moves sub-workspace provisioning off it to stop brands binding to dead workspaces. They meet in createMarket and activate.

Without the last three commits here, turning SERENITY_ASYNC_PROMPT_GEN on would have split the product in two: #3252's enqueue lives in the synchronous create path, and this stack routes the same create through a job chain that bypasses it. Chain-created markets would have kept generating prompts inline — exactly the verbatim catalogue strings #3252 exists to eliminate — while synchronously-created markets got the DRS-generated ones. Same product, two prompt qualities, decided by a code path the user never chose, reported by nothing.

It is not hypothetical: the dashboard sends generatePrompts and async: true in the same request object, on both Add Market and onboarding.

How it is resolved:

The flag is read in create-market-orchestration.js / activate-markets-orchestration.js, because those modules own the market-create and activate bodies for both the controller and the worker. Reading it anywhere else recreates the split.
The enqueue itself cannot live there — feat(serenity): async Semrush-market AI-prompt provisioning (#3194) #3252's producer needs a full request/worker context (sqs, authInfo, postgrestClient) that these modules deliberately do not take, since a flat param bag is what lets one function serve both callers. So the modules surface the inputs and each caller enqueues with its own context.
enqueueSemrushMarketGeneration now accepts a pre-minted promise token: it minted from the request, and a worker has none. The chain forwards the token it already holds, the same mechanism its own hops use. Additive — a request-driven enqueue mints as before.
generationInputs is stripped before the chain's result is stored on the AsyncJob. That result is served verbatim to any client polling the job, and the field carries the brand's Semrush workspace id and alias set.

All four flag combinations are pinned, plus the cases that must not hand off (a 409 slice that already exists, a 201 naming no project, and activate's site-conflict early return). Mutation-verified: removing the inline-generation gate fails the matrix, and handing off on a 409 fails the mixed-batch test.

Open question for review

main routes token-bearing Semrush-write jobs onto a dedicated queue whose DLQ is deliberately not auto-redriven. These jobs now carry #3252's fail-closed promise-pair binding (requirePair: PROMISE_PAIR_SEMRUSH) at all five enqueues — the three controller ones and the worker's own self-requeue and chained-job enqueues — but they are not on that queue, which changes their operational recovery story and is an infrastructure decision rather than a merge resolution. Worth a view from whoever owns that queue.

Correction. An earlier version of this description said the pair binding had already been adopted. It had not: when that was written, requirePair was passed by exactly one enqueue in the repo, the market-generation job, and all five provisioning enqueues went out unbound. That is fixed in this PR rather than walked back — see fix(serenity): bind the provisioning enqueues to the Semrush promise pair, which also records why the binding is safe at each call site. Apologies to anyone who reviewed the earlier text.

Test plan

npm run lint, npm run type-check, npm run docs:lint — clean
1486 passing across serenity, brands and the handlers
Flag-combination matrices mutation-verified on both orchestration modules
Rebased onto current main (this stack was 33 commits behind; feat(serenity): async Semrush-market AI-prompt provisioning (#3194) #3252, fix(serenity): forward browser promise token to async jobs #3204/fix(serenity): forward caller tokens for browser async jobs #3214 and LLMO-7284 all landed in these files)

Related Issues

LLMO-7352 / LLMO-7418
Integrates with [Serenity][Prompt Strategy] Async Semrush-market AI prompt provisioning — AsyncJob producer, token lifecycle, Semrush writes, polling & reauth #3194 / feat(serenity): async Semrush-market AI-prompt provisioning (#3194) #3252 (serenity-docs#443)
Token contracts from fix(serenity): forward browser promise token to async jobs #3204 / fix(serenity): forward caller tokens for browser async jobs #3214; duplicate-active-name from LLMO-7284

🤖 Generated with Claude Code

codecov · 2026-09-09T11:16:59Z

Codecov Report

❌ Patch coverage is 94.76861% with 130 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/support/serenity/async-prompt-gen.js	52.74%	43 Missing ⚠️
src/controllers/brands.js	86.54%	37 Missing ⚠️
src/controllers/serenity.js	93.75%	26 Missing ⚠️
src/support/brands-storage.js	91.85%	18 Missing ⚠️
src/support/serenity/handlers/create-market-job.js	96.00%	6 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

github-actions · 2026-09-09T16:38:45Z

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>

… path The activate half of the #3194/#3252 integration, same reasoning as the createMarket half in the previous commit: this module owns the activate batch for both the synchronous controller and the chained worker, so the flag is read there or the two paths generate prompts differently. Activate hands off PER MARKET rather than once, which makes two things different from createMarket: - A mixed batch is possible, and only markets this call actually CREATED may be handed off. A 409 means the slice was already live, and generating into a project this request did not create is somebody else's data. That guard is status-based, and the test now proves it: the 409 fixture deliberately names an existing project, because with a project-less body the projectId check alone carried the test and the status check could have been deleted unnoticed. - Both callers need the identical loop, so it lives in async-prompt-gen.js as `enqueueMarketGenerations` rather than being written twice. It started in the controller and moved when eslint caught the import cycle that created (the controller already imports the job module for its job types). The worker forwards its own promise token, as in createMarket, and strips `generationInputs` before the result is stored on the AsyncJob -- that result is served verbatim to any client polling the job. Four flag combinations plus the mixed-batch case, all mutation-verified: stripping the inline-generation gate fails the matrix, and handing off on a 409 fails the mixed-batch test. 1490 passing across serenity, brands, the handlers and async-prompt-gen. eslint clean, no dependency cycles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Type-check failures from the previous two commits, caught by CI. I ran eslint and the suites locally but not `npm run type-check`, which is the gate that actually reads the JSDoc contracts. Mostly annotation: both orchestration modules now return `generationInputs` alongside `{ status, body }`, and `enqueueSemrushMarketGeneration` accepts an optional pre-minted `promiseToken`. Declared rather than cast away, so the callers keep real types instead of `any`. One was NOT annotation. The activate orchestration's site-conflict early return never carried the field at all -- a caller reading it unconditionally would have got `undefined` rather than an empty list. It hands off nothing, correctly: the brand stays pending on a site conflict, so no market this call created should have prompts generated against it. Only the type checker found that path. `MarketCreateSuccessBody` also gains `projectId`, which a 201 genuinely carries and the handoff condition depends on. type-check, eslint and the suites all clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ixtures Six failures CI found that my local runs did not, because I ran targeted suites (serenity, brands, handlers) and never the full one. Two distinct causes, both mine. FIXTURES I DROPPED. Resolving the rebase, I took this branch's version of test/openapi-contract/serenity-api.test.js wholesale, which silently discarded main's fixtures for finalizeSerenityPrompts, getSerenityMarketGenerationJobStatus and reauthSerenityMarketGenerationJob along with the harness they need. Rebuilt the other way round: main's file, with this branch's two deltas re-applied -- activate's response shape now comes from activate-markets-orchestration.js (a DIRECT import of serenity.js, so esmock cannot reach the underlying handler transitively), and its synchronous branch needs the concurrency guard stubbed rather than the postgrest fake widened. A GAP THE REBASE EXPOSED. `getSerenityJobStatus` had no contract fixture at all. The endpoint was routed and specced, but its path was never registered in api.yaml -- and this test enumerates the BUNDLED spec, so nothing checked it. Registering the path during the rebase brought it under the test for the first time. It now has a fixture. WORKER DISPATCH. Two more instances of the guard main added that DROPS a message whose type disagrees with the job's stored jobType. Jobs built with the default classify type never reached their handler. Same fix as the provision-workspace dispatch test earlier in this rebase: give the fixture a matching stored type. Full suite run locally this time: 18246 passing. The one remaining failure is a stale local node_modules (tokowaka-client 1.23.3 installed, 1.24.0 required) in an unrelated controller. eslint and type-check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Luis's review on #3246. The job-status handler checks `metadata.brandId` against the caller's brand on the ORIGINAL job, then follows `chainedJobId`/`requeuedJobId` pointers without re-checking. Chain pointers are written by the worker and are never caller-supplied, so this is defence in depth rather than a live IDOR — but this handler is now a generic "serve any job in this brand's chain" surface, and a corrupt pointer resolving to another brand's job would have returned that job's result to this caller. One line, mutation-verified. Two other findings on this PR needed no change, and I am replying to him separately with the evidence: - The missing `updateProvisioningJobId` in `createBrandForOrg`'s async path is real as of THIS PR, but #3249 closes it at all three remaining call sites, and async cannot run at all until #3249's master switch ships. No exposure. - He suggested mirroring the `failBestEffort` no-op rationale onto the call site itself. That comment is already there, and the reality is stronger than the suggestion assumed: `failBestEffort` is SKIPPED outright when the candidate was promoted, not merely CAS-no-oped. 1493 passing, eslint and type-check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

Found by a four-perspective review pass; three of the four reviewers reached it independently, and it violates an acceptance criterion quoted verbatim from the epic: "Retry creates or safely adopts at most one healthy candidate for the current attempt. It does not resume a terminally failed shell." The worker did resume it. When a brand already has a canonical pointer the existing-pointer fast path polls THAT workspace, and on a terminal status the handler recorded `failed` and stopped, leaving the dead pointer in place. So every retry re-polled the same corpse. The only code that clears a pointer is `deactivate`, which also decommissions the brand's projects. The practical effect is that the feature could not fix the brands it was written for. The five known production brands are already bound to dead workspaces, and the Retry button this stack adds to the UI sits on exactly that state — it could never have succeeded for any of them. Now, when the workspace that polled terminally failed IS the brand's canonical pointer, the attempt abandons it and self-requeues with `abandonedCanonicalPointer`, dropping `candidateWorkspaceId` so the next hop create-or-adopts a healthy workspace. `promoteProvisioningReady` then overwrites the dead pointer when that candidate settles — its CAS never required the pointer to be null — so no separate pointer-clear is introduced, and the active-brand anchor constraint (site_id only since 20260818141343) is untouched. Bounded and self-limiting: the flag is sticky for the chain so no later hop re-adopts the dead pointer, the replacement candidate is freshly created and so can never re-enter this branch as a canonical pointer, and the ordinary requeue-depth cap still applies. When there is no title to create a replacement with, the original record-and-stop behaviour is kept rather than throwing. Three tests, both halves mutation-verified: never abandoning fails the first, and ignoring the flag on the next hop fails the second. 1043 passing across serenity and the handlers. eslint and type-check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The provisioning worker enqueues its chained job AFTER promoting the workspace to canonical, and the chained handlers then act on Semrush — creating and PUBLISHING real projects. A `deactivate` landing between those two moments decommissions the workspace, clears the brand's canonical pointer and tombstones its mapping rows. The chained job would then create a live project inside the workspace that was just torn down, and write a fresh mapping row for a brand pointing nowhere — violating the epic's "a late worker cannot recreate, repoint, or reactivate" criterion. The orchestration cannot catch this for us: both chained handlers pass `preResolvedWorkspaceId`, which makes `ensureSubworkspace` skip its own pointer read entirely. `assertChainedJobStillApplies` re-reads the brand at the START of each chained job and stands down (207 `superseded`, no transport built) when the brand is gone or no longer bound to the workspace the job was handed. It compares the CANONICAL pointer, not the provisioning status: by the time the chain runs the attempt is legitimately no longer `pending`, so status says nothing — the pointer is what deactivate clears and what a re-provision replaces. It fails OPEN on an unreadable state, deliberately: the guard exists to stop a job whose brand demonstrably moved on, and a transient PostgREST error is not that evidence. Failing closed would abandon legitimate market creation on a blip. Tested: guard logic directly (the handler tests mock it away, so its own branches need their own file), plus stand-down and call-wiring tests in both handlers. All four mutations — dropped early-return, wrong workspace id passed, fail-closed, inverted pointer comparison — are caught. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

All three new `async: true` 202 responses pointed at `SerenityPromptsJobAccepted`, which they do not satisfy. That schema requires `[jobId, jobType, status]` with `jobType` fixed to the enum `[classifyPrompts]`: - `POST .../serenity/markets` returned `{ jobId, status }` — no `jobType`. - `POST .../serenity/activate` returned `{ jobId, status }` — no `jobType`. - `POST /v2/orgs/{id}/brands` returned the whole brand plus `{ status: 'pending', jobId }` — no `jobType`, and `status` there is the BRAND's status, which is not even the same field the job schema describes. So the published contract told clients to expect a required discriminator none of the three sent, and described a brand-shaped body as a job envelope. Two new schemas describe what is actually returned: - `SerenityProvisioningJobAccepted` — the markets/activate 202s. `jobType` is the FIRST hop of the chain, `serenity-provision-workspace`, not the market or activation work that follows; the poll endpoint follows the chain to its effective terminal hop. - `V2BrandProvisioningAccepted` — the brand-create 202, an allOf over `V2Brand`, documenting that `status: pending` is the brand's state (rendered "Setting up") and not the job's, since the row is already persisted when the 202 is sent. `jobType` is now sent at all three sites, so the bodies are self-describing and consistent with the pre-existing `classifyPrompts` 202. The CSV-import 202 keeps `SerenityPromptsJobAccepted`, which it does satisfy. Tested: the markets and activate 202 bodies are pinned whole with deep.equal rather than `include` (which cannot notice a required field going missing), and the brand 202 asserts jobType and id. Removing jobType from any of the three bodies fails a test. Redocly validates the spec with zero new warnings (221 before and after). Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… too `LEASE_REQUIRED_JOB_TYPES` held only the Semrush market-generation job. The three provisioning job types added by this stack were never added to it, so they ran with no replay protection at all. They meet both criteria the lease exists for. Each carries a `promiseToken` / `promisePair` in its metadata that the runner exchanges up front, so a replayed delivery replays a live token. And each writes to Semrush: sub-workspace create for the provisioning worker, project create and project publish for the two chained phases. Their compare-and-set discipline is not a substitute. CAS makes a duplicate delivery's DB write lose, but only after the upstream Semrush call has already happened — so a concurrent redelivery still creates a real sub-workspace or a real project that then has to be cleaned up. The lease stops the second delivery before it reaches Semrush at all. Safe for the self-requeue and chain hops: the lease is per-job and every hop is a new AsyncJob with its own id, so a hop never contends with its own successor. The runner already releases the lease on a retryable failure and scrubs it on every terminal path, so a redelivery can always re-claim. Also closes two test gaps found while covering this: `activate-markets-job.js` was never mocked in the runner test (so the real module was pulled in and its registration was never exercised), and it had no dispatch test, unlike the other two provisioning handlers. Tested: per job type, that the lease is claimed, that a delivery losing the lease reaches neither the handler nor the token exchange, and that a claim-query error fails closed. Reverting the set to generation-only fails 9 tests. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…th too `createBrandForOrg`'s async branch went straight to `beginProvisioningAttempt`, which is a bare compare-and-set with no staleness awareness. The six equivalent call sites in serenity.js all call `guardAgainstConcurrentProvisioning` first (external-review Finding 9); this one never did. The consequence is on the upsert path. `upsertBrand` updates an existing brand when one already has that name in the org, so this endpoint can land on a brand that is already stuck at `pending` — a worker that died mid-attempt, or a rollback that left its enqueued job unhandled. The CAS then returns false on every subsequent call and the endpoint 409s that brand permanently, with no path back: nothing else on this route ever reconciles the stale attempt. The guard reconciles an attempt older than the stale threshold (10 minutes, matching the window the dashboard uses to call a brand "delayed") and 409s only a genuinely live one. On a brand-new row it reads no state and returns immediately, so the common case is unchanged. Also corrects a false claim in the guard's own doc comment: it stated the guard was already called from `brands.js`'s `createBrandForOrg`, which it was not. The rest of that comment — why `ErrorWithStatusCode` satisfies both error mappers — is accurate and kept. Tested: that the guard runs BEFORE the CAS (reconciling afterwards would unstick nothing), and that a live attempt 409s with its code while minting no attempt and enqueuing no job. Removing the call, or moving it after the CAS, each fail 2 tests. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Repo-wide `eslint .` is clean again (the previous commit's check read the exit status of a pipe, not of eslint itself, so a max-len error slipped past it). Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The same condition was reported three different ways. The stale/concurrent guard throws `semrush_provisioning_in_progress`; serenity.js's mapError surfaces that token under `error`; brands.js's CAS-loss branch invented a third spelling, `semrushProvisioningInProgress`, under `error`, while brands.js's own error mapper surfaces the guard's token under `code` (the convention its sibling conflicts already use, e.g. `brand_duplicate_active_name`). A client cannot branch reliably on that. It matters because the frontend has to tell this 409 apart from the market-slice 409, which means the opposite: the slice conflict IS idempotent success, while this one means nothing was started and the caller should retry shortly. Treating them alike reports a market as created when no work happened. The CAS-loss branch now returns the same `semrush_provisioning_in_progress` token under `code`, with the same message the guard throws, and the spec says so — including the warning that this 409 is not the idempotent one. Tested: the previously untested CAS-loss branch now pins the token, alongside the guard's own 409. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…pair PR #3246's description told reviewers this stack had "adopted #3252's fail-closed promise-pair binding for these jobs". It had not: `requirePair` was passed by exactly one enqueue in the repo, the market-generation job. All five provisioning enqueues — createMarket, activate, brand create, and the worker's own self-requeue and chained-job enqueues — went out unbound. They are the same kind of job the guard was written for: each mints a promise token and writes to Semrush. Without the binding, an enqueue whose context resolved a different pair would mint on the wrong delegated credential and write to Semrush with it, instead of refusing before the token is ever minted. Safe at every one of these call sites, verified rather than assumed: - The three controller enqueues pass no explicit pair, so it is resolved from the request context. The async branch only runs for a Serenity-mode request, and the dashboard attaches the Semrush audience header to exactly those, so the resolved pair is already the Semrush one. - The two worker enqueues forward `metadata.promisePair`. `createAndEnqueueJob` persists the resolved pair into every job's metadata, so what the worker forwards is the pair the originating request resolved. Tested: the createMarket enqueue is asserted to carry the Semrush pair, with the constant pinned in the mock rather than relying on esmock passthrough. Removing the binding fails that test. The PR description is corrected separately; this commit makes the claim true. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

luis6156

Review — feat(serenity): async producers + #3252 integration

This is the complex one and it's well-structured. The orchestration extraction into create-market-orchestration.js (verbatim lift from createMarket's subworkspace branch) is the right approach — the async job caller gets identical behavior without hand-reimplementing the site-linking and model-id resolution that are easy to silently drop. The chained-job-guard.js deactivate-race defense, the fail-open rationale (assertChainedJobStillApplies), and the bounded retry on enqueueChainedJobIfConfigured (swallowed after exhaustion, since the workspace is already promoted — a chained-job enqueue failure must not re-fail the brand) are all correct.

The 3 commits wiring the #3252 integration make this safe to land even when SERENITY_ASYNC_PROMPT_GEN is already on: the sequencing is preserved for both the sync and async paths.

Finding — `MAX_TOPICS_ON_CREATE` shadow copy in `create-market-orchestration.js`

// Mirrors brand-provisioning.js's own MAX_TOPICS_ON_CREATE (same value) — not imported from
// there to avoid a circular import
const MAX_TOPICS_ON_CREATE = 5;

The circular-import constraint is real and the value matches today. The risk is that a future change to brand-provisioning.js's constant silently diverges from this shadow. Not a blocker — the value is unlikely to change — but it would be worth adding a test assertion (or at minimum a // MUST MATCH brand-provisioning.js MAX_TOPICS_ON_CREATE comment with a grep-findable token) so a change to one fails a visible check rather than silently drifting. The re-export path (brand-provisioning.js → exported constant, create-market-orchestration.js imports from a shared constants module instead) would be cleaner if the import cycle can be broken.

Confirmed: `updateProvisioningJobId` gap in `createBrandForOrg`'s async path

serenity.js's createMarket and activate async branches both call updateProvisioningJobId after each self-requeue (per Luis's review). brands.js's createBrandForOrg bare-create async path doesn't. Luis caught this too, and confirmed it's addressed in #3249 — verified: updateProvisioningJobId appears in #3249's brands.js import additions. Since the master switch defaults OFF this doesn't affect what merges here, but it is a pre-flight item before the switch is flipped.

Question on dual-flag test coverage

The PR body correctly describes what the last 3 commits ensure: turning SERENITY_ASYNC_PROMPT_GEN on while this stack is live won't split the async path. Is there a test that exercises both flags simultaneously — specifically a createMarket path where async: true triggers workspace provisioning AND maybeEnqueueMarketGeneration fires inside the chained job? The individual paths appear tested in isolation; the interleaved scenario (workspace self-requeuing while the chained job eventually fires prompt gen) is the one most likely to surface sequencing bugs before prod does.

The requirePair: PROMISE_PAIR_SEMRUSH fail-closed binding on the chained-job enqueue is correct and closes Gap 1 from #3252 cleanly.

Approving.

…and too The chained-job guard compared only the canonical workspace pointer. That misses a delete entirely, because `deleteBrand` is a SOFT write: it sets `status = 'deleted'` and renames the row, and deliberately leaves `semrush_sub_workspace_id` in place. The state read applies no status filter, so a deleted brand is still returned AND still matches the pointer — the guard waved the job straight through. The consequence is the one the guard exists to prevent, reached by a different route: delete a brand between the provisioning worker promoting its workspace and the chained job running, and that job creates and PUBLISHES a live, billable Semrush project, writes a `brand_to_semrush_projects` row and links a SpaceCat site, all for a brand the customer just deleted. Offboarding (`status = 'ignored'`) has the same shape. `promoteProvisioningReady` already refuses exactly this on its own write, with `status IN ('pending','active')`. The promotion path was protected and this one was not, even though the state read already returns `status`. The guard now applies the same predicate. `pending` stays allowed, and is tested: it is the normal state of a brand whose first market is still being provisioned, so excluding it would stand down on every healthy first-market chain. Tested: delete and offboard each stand the job down while the pointer still matches, and `pending` still proceeds. Removing the check fails 2 tests. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…writes The attempt-id-scoped compare-and-set is the invariant this whole feature rests on: it is what stops a superseded attempt, or a late SQS redelivery, from clobbering a newer winner's canonical workspace pointer. Neither terminal write asserted it. Both tests for these functions checked only the row being written, or fed a mock that ignores filters entirely and so proved only the `Boolean(data)` mapping. The result: `promoteProvisioningReady` could be reduced to "UPDATE this brand, unconditionally" — no attempt scoping, no pending check — and the entire suite stayed green. Verified by mutation before writing these, with the applied diff confirmed each time rather than assumed. The assertions follow the pattern this file already uses for `persistProvisioningCandidate`, which pins its `eq` and `is` filters; the two terminal writes were the only provisioning writes that skipped it. Now caught by mutation: - dropping attempt-id + `pending` from the ready promotion: 1 failing - dropping attempt-id from the failed promotion: 1 failing - dropping the `status IN ('pending','active')` resurrection guard: 2 failing Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…tion jobs Both chained market handlers forward THIS job's promise token to the prompt-generation jobs they enqueue, because there is no request on the worker path to mint a fresh one. Neither then returned `requeuedJobId` or `chainedJobId`, so the runner computed no ownership transfer and invalidated the token on terminal state. Invalidation is BY IDENTITY and kills every token in the exchange chain, so the copy those generation jobs hold was dead before any of them exchanged it — and the generation job exchanges late, after its downstream call returns. The result: every market created through the async chain gets no generated prompts, and the generation job the UI polls ends FAILED needing reauth. This is deterministic, not a race. It is also not behind the prompt-generation feature flag on this path: the chained handler passes `enabled: true` as a literal, so it is gated only by the async switch, and onboarding always requests generation — exactly the "same product, two prompt qualities, decided by a code path the user never sees" outcome the handoff was written to prevent. The runner already reasoned about this hazard for self-requeue and chained jobs. Generation jobs are neither, so they need their own signal: a handler that hands its token to jobs it enqueued itself now returns `tokenHandedOff`, and the runner treats that as ownership transfer. The flag is stripped BEFORE the result is stored, so it never reaches a client polling the job. `enqueueMarketGenerations` now returns how many jobs it enqueued, so the activate handler only claims a hand-off when one actually happened. Tested: the handoff block had zero coverage (c8 reported lines 112-138 unhit). Now covered for token forwarding, the signal, the no-generation case, and that `generationInputs` never leaks into the stored result — it carries the brand's workspace id and alias set. Two mutations verified with the applied diff confirmed: the runner ignoring the signal, and the handler not reporting it, each fail a test. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…queues The controller side pins `requirePair`; the worker's own two enqueues did not. Deleting the binding from either the self-requeue hop or the chained-job enqueue left the entire suite green, so the security control that stops a token being minted on the wrong delegated credential was unguarded at exactly the two call sites that run without a request context. The fixture also never set `promisePair`, so the forwarded value read `undefined` in every test. `createAndEnqueueJob` persists the resolved pair onto every job's metadata, so a real job always carries one; the fixture now does too, which is what makes asserting the forwarded pair meaningful. Verified by mutation with the applied diff confirmed: dropping the binding from either enqueue now fails a test. Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…body `SerenityJobStatus.result` was documented as "the same body the equivalent synchronous endpoint would have returned". The chained jobs return `{ status, body }`, so a client following the spec reads `result.markets` and gets `undefined` — silently, for every market on every async activate. This is not hypothetical. The dashboard shipped exactly that bug and fixed it, and left the autopsy in place: "the stored job result is the handler's `{ status, body }` ENVELOPE, not the activate response itself — `result.markets` was always undefined, so every market after the first was silently dropped." The spec is the half that was never corrected, so the next client would walk into it. The description now names all three real shapes, including the provisioning job's `{ provisioningStatus }`, and says which field to read. It also states the other trap the same clients hit: COMPLETED means the job RAN, not that it worked. A chained job returns a 4xx/5xx status inside the envelope and a provisioning job returns `provisioningStatus: 'failed'` rather than throwing, so both reach COMPLETED with `error: null`. Redocly still validates with no new warnings (221 before and after). Introduced by: N/A Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

…lready does The chained-job enqueue gets three attempts with backoff; the self-requeue got a bare call. The asymmetry ran in the dangerous direction. A throw from the self-requeue reaches the outer catch with `requeueEnqueued` still false, so one transient SQS failure does not merely lose a hop. It runs `cleanupIfOwned` on the sub-workspace this attempt just created, emptying it, and records the attempt failed with "must be re-created" — on an attempt that was perfectly healthy a moment earlier. Nothing rescues it: this handler never throws a retryable job error, so the runner marks the job FAILED and stops rather than letting SQS redeliver. The retry uses the same attempt count and delays as the chained enqueue, and deliberately rethrows on exhaustion rather than swallowing. The chained enqueue can swallow because its workspace is already promoted and a lost chain must not re-fail the brand; a lost self-requeue means this attempt has no future hop, and the outer catch is what records that. So the terminal behaviour is unchanged — only the single-blip case is removed. Tested: one transient failure now recovers AND leaves the workspace untouched, and a persistent failure still fails the attempt after three attempts. Verified by mutation with the applied diff confirmed: reverting to a bare enqueue fails 2 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…and row Staleness was computed from `brands.updated_at`. A row-level BEFORE UPDATE trigger bumps that column on ANY edit to the brand — verified against production — so a rename, a site link or an alias sync on a brand whose attempt died an hour ago makes the row look edited a moment ago. The guard then calls that dead attempt fresh, answers 409, and does it again on every retry. There is no sweep to rescue it. A stuck attempt only ever reconciles when a later request for that same brand arrives and finds it stale, so an attempt whose clock keeps getting reset is stuck for good. That is why I previously described recovery as bounded at ten minutes and it was not bounded at all. `semrush_provisioning_started_at` records when the current attempt began, is set by `beginProvisioningAttempt`, and is cleared by every terminal write so it cannot outlive its attempt. It is immune to unrelated edits, so the age the guard computes is the age of the attempt. Falls back to `updated_at` when the column is null, which covers rows written before the migration lands. That is no worse than the previous behaviour and no better — the fallback is a compatibility shim, not a second mechanism. Tested: a stale attempt is reconciled even when an unrelated edit just bumped `updated_at` (the exact case that was broken), and a genuinely fresh attempt still 409s without writing. Verified by mutation with the applied diff confirmed: reading `updated_at` again, or not writing the start time, each fail a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

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

This was referenced Sep 9, 2026

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

Open

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

Open

IrisAlexandrescu force-pushed the feat/LLMO-7418-provisioning-worker branch from 70fe12d to ae4b0a9 Compare September 9, 2026 16:00

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-async-provisioning-endpoints branch from fafceee to 6064817 Compare September 9, 2026 17:24

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

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

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

IrisAlexandrescu force-pushed the feat/LLMO-7418-provisioning-worker branch from fce9e83 to 09c741f Compare September 10, 2026 07:42

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

IrisAlexandrescu temporarily deployed to dev-branches September 10, 2026 08:12 — 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 temporarily deployed to dev-branches September 10, 2026 10:36 — 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 temporarily deployed to dev-branches September 10, 2026 11:26 — with GitHub Actions Inactive

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

IrisAlexandrescu force-pushed the feat/LLMO-7418-provisioning-worker branch from 6ff60d2 to 146216b Compare September 14, 2026 13:43

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from 26ce2db to b563c4b Compare September 14, 2026 13:43

Iris Alexandrescu and others added 4 commits September 16, 2026 12:12

IrisAlexandrescu force-pushed the feat/LLMO-7418-provisioning-worker branch from 146216b to 4c23566 Compare September 16, 2026 09:37

IrisAlexandrescu force-pushed the feat/LLMO-7418-async-provisioning-endpoints branch from f16863a to e8c2c71 Compare September 16, 2026 09:37

IrisAlexandrescu deployed to dev-branches September 16, 2026 09:53 — with GitHub Actions Active

Iris Alexandrescu and others added 8 commits September 16, 2026 13:25

IrisAlexandrescu deployed to dev-branches September 16, 2026 11:57 — with GitHub Actions Active

luis6156 approved these changes Sep 16, 2026

View reviewed changes

Iris Alexandrescu and others added 5 commits September 16, 2026 15:32

IrisAlexandrescu deployed to dev-branches September 16, 2026 13:11 — with GitHub Actions Active

Iris Alexandrescu and others added 2 commits September 16, 2026 20:40

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

Navigation Menu

feat(serenity): opt-in async provisioning for createMarket, createBrandForOrg, activate (LLMO-7352/LLMO-7418) - #3246

feat(serenity): opt-in async provisioning for createMarket, createBrandForOrg, activate (LLMO-7352/LLMO-7418)#3246
IrisAlexandrescu wants to merge 29 commits into
feat/LLMO-7418-provisioning-workerfrom
feat/LLMO-7418-async-provisioning-endpoints

IrisAlexandrescu commented Sep 9, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

luis6156 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Summary

Changes

Integration with #3252 (async Semrush-market AI-prompt provisioning)

Open question for review

Test plan

Related Issues

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!

github-actions Bot commented Sep 9, 2026

Uh oh!

luis6156 left a comment

Choose a reason for hiding this comment

Review — feat(serenity): async producers + #3252 integration

Finding — MAX_TOPICS_ON_CREATE shadow copy in create-market-orchestration.js

Confirmed: updateProvisioningJobId gap in createBrandForOrg's async path

Question on dual-flag test coverage

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IrisAlexandrescu commented Sep 9, 2026 •

edited

Loading

codecov Bot commented Sep 9, 2026 •

edited

Loading

Finding — `MAX_TOPICS_ON_CREATE` shadow copy in `create-market-orchestration.js`

Confirmed: `updateProvisioningJobId` gap in `createBrandForOrg`'s async path