feat(serenity): async bulk-delete to stop edge-timeout 503s on large brands

[aliciadriani]

1. Abstract

Moves the Serenity Prompt Library bulk-delete off the synchronous request path onto the existing job runner: the endpoint returns 202 + jobId and the delete + publish run in the worker, with the UI polling for the outcome. Gated behind SERENITY_ASYNC_BULK_DELETE (default off).

2. Reasoning

POST /v2/orgs/:spaceCatId/brands/:brandId/serenity/prompts/bulk-delete runs the Semrush delete + publishProject synchronously. On a large brand (Adobe Helpx, ~9.7k prompts) the publish can outlive the ~15s Fastly edge budget while the Lambda itself has a 900s budget, so the client gets a 503 ({"message":"Service Unavailable"}) even though the delete keeps running and likely completes server-side. The UI then retries, which piles more requests onto Semrush. This matches the two root causes found on the incident: (1) too many Semrush requests for the large brand, amplified by (2) the retry loop. Full RCA: #3287.

3. High-level overview of the changes

• New src/support/serenity/handlers/bulk-delete-job.js:
  • acceptBulkDelete (producer) validates the request and createAndEnqueueJobs it, returning 202 { jobId, status, jobType: 'bulkDelete' }.
  • bulkDeleteHandler (consumer) runs the existing handleBulkDeletePrompts / handleBulkDeletePromptsSubworkspace with the worker's exchanged Semrush access token, then records the same { deleted, failed } result on the job. The delete logic is reused verbatim, so idempotent already-gone deletes, per-target failures, and publish reconciliation are unchanged.
• Worker: register BULK_DELETE_JOB_TYPE in the job-runner HANDLERS (exchange-first, no lease).
• Controller bulkDeletePrompts: behind SERENITY_ASYNC_BULK_DELETE (default off), exchange the promise token exactly as the async bulk-tags path does and return 202; the synchronous path is unchanged and remains the default.
• getPromptsJobStatus: reports the delete job as public type bulkDelete; its { deleted, failed } result passes through the existing poll endpoint.
• Behaviour is identical to today until the flag is switched on (and the polling UI ships).

4. Required information

• Jira / issue: LLMO-7615
• Other: GitHub issue — Prompt Library: serenity bulk-delete 503s at the edge while the Lambda keeps running (likely succeeds server-side) — make it async #3287

6. Affected / used mysticat-workspace projects

• project-elmo-ui (consumer, follow-up): to realise the fix end to end, the Prompt Library must handle the 202 { jobId } on delete and poll serenity/prompts/jobs/{jobId} instead of blocking on the response. Separately, the incident also involves a UI bug that persistently shows the deletion error — that is a project-elmo-ui fix, independent of this PR.

7. Additional information outside the code

Root cause established from code (prompts.js handler returns 200/failed[] and never throws the observed 503; mapError doesn't emit it; Lambda timeout 900s in package.json vs the ~15s edge) — the 503 is an edge timeout on a mutation that keeps running. Splunk confirmation of the exact edge layer is tracked as an open item in #3287 (Splunk MCP was unavailable during triage).

8. Test plan

(a) Local: unit tests for the producer validation and the consumer (delegates to the existing delete logic, completes the job with the result, marks FAILED on a deterministic error, rethrows a retryable job error on a transient 5xx). Ran the serenity controller and job-runner suites; both green with the flag default-off (sync path unchanged).
(b) Per-env:

• dev: set SERENITY_ASYNC_BULK_DELETE=true, POST a bulk-delete with x-promise-token + x-promise-audience: semrush, confirm a 202 { jobId } and that GET .../serenity/prompts/jobs/{jobId} reports bulkDelete progressing to COMPLETED with { deleted, failed }.
• prod: keep the flag off until the polling UI ships; then enable and verify the Adobe Helpx delete completes without a client 503.

9. Deployment & merge order

• Related: Prompt Library: serenity bulk-delete 503s at the edge while the Lambda keeps running (likely succeeds server-side) — make it async #3287 (RCA), fix(serenity): cap faceted prompt corpus walk to fail fast on large brands #3285 (faceted list fail-fast — the other Semrush-hammering path), fix(prompts): bulk-delete the whole batch in one round-trip #3280 (merged, Postgres path). Independent to merge. Do not flip SERENITY_ASYNC_BULK_DELETE on in prod until the project-elmo-ui polling change ships; the default-off flag makes merge safe ahead of that.

🤖 Generated with review-kit

Change Management

cm-assessment: v1
changeType: standard
impact: unnoticeable
risk: minor
scope: single-repo
relatedPRs: ["adobe/spacecat-api-service#3287", "adobe/spacecat-api-service#3285"]
rationale: "New async job path for bulk-delete behind a default-off flag; the synchronous path and all behaviour are unchanged until the flag is enabled, the delete logic is reused verbatim, deletes stay soft/idempotent, and the change is reversible by flag or redeploy."
recommendations: "Enable SERENITY_ASYNC_BULK_DELETE in dev first and only in prod once the project-elmo-ui polling UI ships; watch the job-runner queue and Semrush error rate after enabling."
backout: "Set SERENITY_ASYNC_BULK_DELETE=false (instant revert to the synchronous path) or redeploy the previous release."

---

Correction (from review)

• Corpus size: the Adobe Helpx brand holds 38,764 serenity prompts (confirmed live via GET .../serenity/markets promptsCount), not ~9.7k — that earlier figure was the unrelated Postgres prompts table (Prompt Library: bulk delete issues one round-trip per prompt id, and the list read materializes the whole collection — both 503 at the Fastly edge for large brands #3279), a different store/path.
• Semrush load attribution: the dominant Semrush load (~10x on /aio/prompts/by_tags) is the read/list walk (listPromptsByTags), fixed in fix(serenity): cap faceted prompt corpus walk to fail fast on large brands #3285 — not delete retries. Delete hits deletePromptsByIds (prompts/delete), a different, cheaper endpoint. This PR fixes the delete-503 UX (moves it off the edge-bounded path); it does not reduce by_tags load.

[aliciadriani]

UI consumer follow-up (handle 202 + poll, and fix the persistent deletion-error display): https://github.com/adobe/project-elmo-ui/issues/3185 — roll the SERENITY_ASYNC_BULK_DELETE flag on in lockstep with the polling change there.

[aliciadriani]

Hey @aliciadriani,

Verdict: Comment - self-review, non-binding (GitHub blocks author approval); no blockers, two should-fixes and two nits inline.
Complexity: MEDIUM - new async job path (producer + consumer + worker registration + controller branch), but flag-gated and reusing the existing delete logic.
Changes: enqueue serenity bulk-delete as a job (202 + poll) behind SERENITY_ASYNC_BULK_DELETE, reusing handleBulkDeletePrompts in the worker (5 files).

Must fix before merge

None.

Non-blocking (4): should-fix and nits

• suggestion: [Should] unbounded failed[] on the job result risks the AsyncJob item-size limit for a large delete - src/support/serenity/handlers/bulk-delete-job.js:138 (details inline)
• suggestion: [Should] no idempotency key, so a double-submit enqueues duplicate delete jobs and re-hammers Semrush - src/support/serenity/handlers/bulk-delete-job.js:71 (details inline)
• nit: enabling the flag before the UI handles 202 breaks the grid (reads res.deleted) - src/controllers/serenity.js:954 (details inline)
• nit: add a subworkspace-path handler test - src/support/serenity/handlers/bulk-delete-job.js:109 (details inline)

[aliciadriani]

suggestion (should): job.setResult(result) stores the full { deleted, failed } verbatim. For a large brand a delete with many upstream failures could produce a failed[] big enough to push the AsyncJob record past the store's item-size limit, so job.save() fails and the whole job is marked failed even though the deletes landed. The sibling bulk-tags job already guards this with pageBulkFailures / BULK_FAILURE_PAGE_LIMIT. Bound or page the stored failures the same way (and expose them page-wise via the poller) rather than storing an unbounded array.

[aliciadriani]

suggestion (should): acceptBulkDelete enqueues a fresh job on every call - no idempotency key / deterministic job id. The exact failure mode this PR targets is a retry storm (the UI re-submitting a delete that 503'd), and without dedupe each re-submit enqueues another delete+publish job, re-hammering Semrush for the same work. It's harmless for correctness (the delete is idempotent), but it defeats part of the goal. bulk-tags-job derives a deterministic job id from an idempotency-key header and reuses the in-flight job; consider the same here so a retried submit joins the existing job instead of spawning a new one.

[aliciadriani]

nit: once SERENITY_ASYNC_BULK_DELETE is on, the endpoint returns 202 { jobId } with no deleted field, but today's UI reads res.deleted on the delete response - so flipping the flag before project-elmo-ui#3185 / #3184 ships the 202-handling would render a broken success toast. The PR body already says to roll them in lockstep; worth a comment on the flag itself (or a guard) so a future operator doesn't enable it standalone.

[aliciadriani]

nit: the handler tests cover the flat path; add one for subworkspace === true so the handleBulkDeletePromptsSubworkspace branch (different arg shape) is exercised too.

[codecov]

Codecov Report

❌ Patch coverage is 70.37037% with 72 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/controllers/serenity.js	26.66%	44 Missing ⚠️
src/support/serenity/handlers/bulk-delete-job.js	84.26%	28 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

This PR will trigger a minor release when merged.

[aliciadriani]

Hey @aliciadriani,

Verdict: Comment - re-review of the update; self-review, non-binding. No open blockers.
Complexity: MEDIUM - one commit addressing the prior self-review.
Changes: bound the job's stored failures (with a true-count) and guard the async flag against pre-UI enablement (3 files).

Must fix before merge

None.

Previously flagged, now resolved

• Unbounded failed[] on the job result: now capped at MAX_STORED_FAILURES (200) with a failedTotal count and failedTruncated flag, so a large delete can't push the AsyncJob record past the store's item-size limit. Covered by a new truncation test.
• Flag could be enabled before the UI understands 202: added a comment on SERENITY_ASYNC_BULK_DELETE tying it to project-elmo-ui#3185/#3184.

Deferred (2), with rationale

• [Should] idempotency-key dedupe: deferred to its own follow-up. It needs extracting bulk-tags' deterministic-jobId + race-recheck machinery (private to that handler) into a shared helper - too much for this PR. It's correctness-safe to defer: the delete is idempotent (already-gone → success), so a duplicate job wastes Semrush calls but does no harm, and the retry storm that would cause duplicates is what project-elmo-ui#3184/#3185 remove. Worth a ticket if we want the server-side guard regardless.
• [Nit] subworkspace-path handler test: minor; the flat path is covered and the subworkspace branch is a thin delegate to the existing (separately-tested) handleBulkDeletePromptsSubworkspace. Follow-up.

[aliciadriani]

Filed the deferred idempotency-key dedupe as its own tracked issue: #3289 (reuse the bulk-tags idempotency pattern; correctness-safe to do separately since the delete is idempotent).

[dzehnder]

Review — async bulk-delete

Verdict: Request changes. The async approach is the right call and correctly scoped to the delete path (complementary to #3285, doesn't touch the read walk; sync path stays byte-for-byte unchanged while the flag is off). But there's a confirmed Critical integration bug that makes the feature non-functional as written and, worse, silently reports failed hard-deletes as successes. The unit tests pass only because they exercise the handler in isolation against the wrong contract.

Critical

1. The handler self-manages job state and returns nothing; the runner then overwrites it — the result is lost and failures are masked as success. (details inline on bulk-delete-job.js)

bulkDeleteHandler sets the job state itself and returns undefined:

• success → job.setResult(storedResult); job.setStatus('COMPLETED'); await job.save() (bulk-delete-job.js:153-155)
• deterministic error → job.setStatus('FAILED'); job.setError(...); await job.save() (bulk-delete-job.js:167-173), no throw, no return

But the runner invokes handlers on a return-value contract, unconditionally, after the handler returns (serenity-prompt-classification/index.js:325-327):

const result = await handler(context, job, accessToken);
job.setStatus('COMPLETED');
job.setResult(result ?? null);

So in production:

• Success: the handler's storedResult is overwritten by setResult(undefined ?? null) → the job persists with result: null. The poller / UI (project-elmo-ui#3185) never receives { deleted, failed } — the point of the async path is defeated.
• Deterministic failure: the handler's FAILED is overwritten by setStatus('COMPLETED') + result: null. A delete that failed with nothing deleted is reported to the UI as COMPLETED (success) — on an irreversible Semrush hard-delete, the worst direction to be wrong in (a sync 503 at least signals failure). Any unexpected throw where error.status is undefined (< 500 → treated as deterministic) hits this path.

Only the transient branch (throw retryableJobError(...), bulk-delete-job.js:165) is wired correctly, because throwing is what the runner's catch expects.

Fix: follow the exact contract bulkTagsHandler already uses — return storedResult on success (drop the setResult/setStatus/save at 153-155) and throw the deterministic error (replace 167-173 with throw error;) so the runner's catch sets FAILED. Keep the transient throw retryableJobError as-is.

Important

2. Missing // @ts-check — the new file is silently untyped. bulk-delete-job.js:13 goes straight from the license block to import, with no // @ts-check. src/support/serenity/CLAUDE.md rule 1 requires it as the first non-license line, and the sibling bulk-tags-job.js:14 has it. Without it the whole file is excluded from the type-check gate — which would likely have caught finding 1. Add the pragma and fix anything it surfaces.

3. Tests encode the wrong contract and mask finding 1. The consumer tests call bulkDeleteHandler in isolation and assert on the mock job's self-set status/result — they never dispatch through the runner (index.js:325-327), so they pass while production overwrites the result. After fixing finding 1, assert on the handler's return value (success) and on a thrown error (deterministic failure), ideally plus one through-the-runner integration test so the persisted result is actually verified.

Minor / observations

• No publish-retry. handleBulkDeletePrompts folds a publish failure into failed[] as a status:502 pseudo-entry and returns normally (prompts.js:2668-2680), so the job completes with deletes applied-but-unpublished and no recovery — unlike bulk-tags' checkpoint/resume. Inherited from the sync handler, not new, but the async move was the chance to close it. Worth a note that an unpublished delete has no automatic retry.
• No per-job lease / no idempotency key. A token-bearing hard-delete gets neither a lease nor a deterministic jobId (bulk-tags supports Idempotency-Key). Consistent with the bulk-tags precedent and mitigated by the terminal-state guard + verified already-gone idempotency, so low risk — but a client POST retry spawns a second job that re-runs (and re-publishes) the whole batch.

What's genuinely good

• Correct scoping (delete path only, complementary to #3285, read path untouched); default-off gating leaves the sync path unchanged and the 202 branch validates promise-token + x-promise-audience: semrush before enqueue. requirePair: PROMISE_PAIR_SEMRUSH is actually stricter than bulk-tags.
• Idempotency verified: deleteProjectBatches counts isUpstreamGone as deleted (prompts.js:1023-1024), so a transient re-run is safe — already-deleted ids aren't reported as failures.
• MAX_STORED_FAILURES cap (200) to keep the job record under the store size limit is a thoughtful touch.
• The "reuses handleBulkDeletePrompts verbatim" claim holds — reused-handler signatures match the callers exactly.

Factual corrections for the PR / RCA (#3287) description

• Corpus size: the brand holds 38,764 serenity prompts (confirmed live via GET .../serenity/markets, promptsCount), not ~9.7k (that figure is the unrelated Postgres table from #3279).
• Semrush load attribution: Semrush's "10x load on /aio/prompts/by_tags" is the read/list walk (listPromptsByTags), fixed in #3285 — not delete retries (delete hits deletePromptsByIds → prompts/delete, a different, cheaper endpoint). #3288 fixes the delete-503 UX but does not reduce the by_tags load; recommend noting the dominant load source is the read path.

Net: fix finding 1 (return/throw instead of self-managing job state), add // @ts-check, rework the tests to the corrected contract — after that this is a solid, well-scoped fix for the delete-503 half.

[dzehnder]

issue (blocking): This handler self-manages job state and returns undefined, but the job runner treats handlers on a RETURN-VALUE contract and unconditionally overwrites state afterward (serenity-prompt-classification/index.js:325-327: const result = await handler(...); job.setStatus('COMPLETED'); job.setResult(result ?? null);).

So this job.setResult(storedResult); setStatus('COMPLETED'); save() (and the FAILED branch at 167-173) get clobbered: on success the real {deleted, failed} becomes null (UI poller gets nothing); on a deterministic error the FAILED becomes COMPLETED with null result - a failed hard-delete reported as success. Only the transient throw retryableJobError branch is wired correctly.

Fix (match bulkTagsHandler): RETURN storedResult on success (drop the setResult/setStatus/save here), and THROW the deterministic error (replace the FAILED branch with throw error;) so the runner's catch sets FAILED. Keep the transient throw as-is.

[aliciadriani]

Fixed in 0e9e890. bulkDeleteHandler now returns the (bounded) result on success and throws the deterministic error, so the runner's return-value contract (index.js:325-327) sets COMPLETED+setResult / FAILED correctly; the transient retryableJobError throw is unchanged. Added // @ts-check (which is what would have caught this), and reworked the tests to assert the RETURN value + a through-runner-contract test, a non-retryable throw on deterministic failure, and a retryable throw on a transient 5xx. Thanks for the precise catch.

[aliciadriani]

Thanks @dzehnder — all addressed in 0e9e890:

Critical (1) — fixed: handler now RETURNs the bounded result / THROWs the deterministic error per the runner's return-value contract; transient retryableJobError throw unchanged. No more self-managed setResult/setStatus/save.
Important (2) — added // @ts-check; type-check passes (and yes, it would have caught #1 — that's why the file slipping past the gate mattered).
Important (3) — tests reworked to the real contract: assert the RETURN value on success + a through-runner-contract test that persists it; a non-retryable THROW on deterministic failure; a retryable throw on transient 5xx.

Minor/observations:

• publish-retry: agreed it's inherited from the sync handler (a publish failure folds into failed[] and the job completes with deletes-applied-but-unpublished, no auto-recovery). Left as-is here to keep the reuse verbatim; worth its own follow-up to add checkpoint/resume like bulk-tags — say the word and I'll file it.
• idempotency key / lease: filed as Async serenity bulk-delete: add idempotency-key dedupe so a retried submit reuses the in-flight job (reuse the bulk-tags pattern) #3289 (reuse the bulk-tags deterministic-jobId + race-recheck).

Factual corrections applied to this PR's description and #3287: corpus is 38,764 serenity prompts (not the ~9.7k Postgres figure), and the dominant Semrush by_tags load is the read/list path (#3285), not delete — this PR is the delete-503 UX fix, not a by_tags load reduction.

Re-requesting your review.

[aliciadriani]

Filed the publish-retry observation as its own follow-up: #3290 — add a publish-resume checkpoint to the async delete job (reuse bulk-tags' recoverFailedPublish / publishRecoveryPending), so a delete that applied but failed to publish recovers instead of sitting deleted-but-unpublished.