Skip to content

fix(serenity): forward browser promise token to async jobs - #3204

Merged
byteclimber merged 2 commits into
mainfrom
fix/serenity-async-promise-token
Sep 11, 2026
Merged

fix(serenity): forward browser promise token to async jobs#3204
byteclimber merged 2 commits into
mainfrom
fix/serenity-async-promise-token

Conversation

@byteclimber

Copy link
Copy Markdown
Contributor

Summary

Makes browser-initiated async Serenity prompt classification preserve the caller-supplied Semrush promise token across the SQS boundary. The request Lambda no longer exchanges that token before enqueueing; it stores the token and matching promise pair on the job so the worker performs the exchange immediately before its Semrush work.

Companion backend fix for adobe/project-elmo-ui#3002.

Problem

The UI authenticates to SpaceCat with its session JWT and separately sends x-promise-token plus x-promise-audience: semrush. The async controller previously exchanged the supplied token in the request Lambda, then called createAndEnqueueJob without it. The runner consequently tried to mint a replacement from the session JWT in Authorization, which IMS rejects.

Changes

  • Skip synchronous Semrush token exchange for async: true prompt creates.
  • Decode and forward x-promise-token as the job promise token.
  • Resolve and persist the matching Semrush promise pair.
  • Preserve the existing runner minting fallback when no promise-token header is supplied.
  • Add controller coverage for a non-IMS SpaceCat session JWT plus browser promise token, including the no-pre-enqueue-exchange invariant.

Validation

  • npx mocha --timeout 10000 test/controllers/serenity.test.js test/support/serenity/async-job-runner.test.js — 254 passing
  • npx eslint src/controllers/serenity.js test/controllers/serenity.test.js
  • npm run type-check
  • npm run build and npm run build:worker

Change Management

cm-assessment: v1
changeType: standard
impact: unnoticeable
risk: minor
changeApprovedBy: ["Dominique Jäggi"]
rationale: "Targeted credential-forwarding fix for a feature that remains gated per organization in the UI."

Carry the browser-supplied Semrush promise token and matching pair into the classification job without exchanging it in the request Lambda. Preserve the existing minting fallback when callers do not provide a token.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@MysticatBot MysticatBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @byteclimber,

Verdict: Approve - clean, well-scoped fix with good test coverage.
Complexity: MEDIUM - small diff; API surface signal.
Changes: Defers the IMS promise-token exchange from the request Lambda to the async worker by forwarding the browser-supplied x-promise-token and its audience pair on the SQS job payload (2 files).

Non-blocking (3): minor issues and suggestions
  • nit: The decodeURIComponent try/catch block duplicates the identical pattern from resolveSemrushImsToken in utils.js (same comment and all) - consider a shared helper if more decode sites appear - src/controllers/serenity.js:608
  • suggestion: No test exercises x-promise-token present with an unknown x-promise-audience value - a case that should propagate a 400 from resolvePromisePair. Pinning that contract in the controller test suite would prevent a silent regression - test/controllers/serenity.test.js
  • suggestion: The promiseToken object passed to createAndEnqueueJob carries only { promise_token } while auto-minted tokens also include expires_in and token_type. The worker only reads .promise_token for the exchange so this is functionally harmless, but a one-line comment noting the intentional shape difference would save a future reader from investigating - src/controllers/serenity.js:614

Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 12m 37s | Cost: $5.79 | Commit: f827d267ac9be004a2e3dc050529c82f5f26514b
If this code review was useful, please react with 👍. Otherwise, react with 👎.

@MysticatBot MysticatBot added ai-reviewed Reviewed by AI complexity:medium AI-assessed PR complexity: MEDIUM labels Sep 4, 2026
Resolve Serenity async promise-token conflicts while preserving mainline validation and no pre-enqueue exchange coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

This PR will trigger a patch release when merged.

@byteclimber
byteclimber merged commit e551703 into main Sep 11, 2026
24 checks passed
@byteclimber
byteclimber deleted the fix/serenity-async-promise-token branch September 11, 2026 13:02
solaris007 pushed a commit that referenced this pull request Sep 11, 2026
## [1.807.1](v1.807.0...v1.807.1) (2026-09-11)

### Bug Fixes

* **serenity:** forward browser promise token to async jobs ([#3204](#3204)) ([e551703](e551703))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed Reviewed by AI complexity:medium AI-assessed PR complexity: MEDIUM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants