Skip to content

fix(medusa): API workflow subscription#13886

Open
v0eak wants to merge 11 commits intomedusajs:developfrom
v0eak:develop
Open

fix(medusa): API workflow subscription#13886
v0eak wants to merge 11 commits intomedusajs:developfrom
v0eak:develop

Conversation

@v0eak
Copy link
Copy Markdown
Contributor

@v0eak v0eak commented Oct 28, 2025

Summary

What
Change req.query to req.params in appropriate api routes

  • move /api/admin/workflows-executions/[workflow_id]/[transaction_id]/[step_id]/subscribe/route.ts
    to /api/admin/workflows-executions/[workflow_id]/[transaction_id]/subscribe/route.ts, because it is not possible to subscribe to individual steps, only to individual workflows.
  • modify API reference in documentation to mirror changes

Why — Current implementation relies on req.query to get the id of workflow/transaction, but it should get them from req.params instead

How — Change req.query to req.params

Testinghttps://docs.medusajs.com/api/admin#workflows-executions_getworkflowsexecutionsworkflow_idsubscribe

Examples

https://docs.medusajs.com/resources/js-sdk#stream-server-sent-events

// Example usage

Checklist

Please ensure the following before requesting a review:

  • I have added a changeset for this PR
    • Every non-breaking change should be marked as a patch
    • To add a changeset, run yarn changeset and follow the prompts
  • The changes are covered by relevant tests
  • I have verified the code works as intended locally
  • I have linked the related issue(s) if applicable

Additional Context

Add any additional context, related issues, or references that might help the reviewer understand this PR.


Note

Switch admin workflow subscription to path params and a single-execution endpoint; update JS SDK fetchStream to always yield a stream and throw on non-OK, with docs aligned.

  • API (Medusa)
    • Use req.params instead of req.query in admin/workflows-executions/[workflow_id]/[transaction_id]/subscribe and [workflow_id]/subscribe routes.
    • Remove step-level subscription; consolidate to .../[workflow_id]/[transaction_id]/subscribe.
  • JS SDK
    • Update client.fetchStream to return a non-null async generator; defer fetch, throw HttpError on non-OK, and stream via events with abort support.
    • Adjust FetchStreamResponse type: stream is non-null.
  • Docs
    • Revise API reference path/summary and cURL sample to match consolidated endpoint.
    • Update JS SDK examples: remove null checks for stream and handle HttpError by calling abort.
  • Changeset
    • @medusajs/medusa: minor; @medusajs/js-sdk: patch.

Written by Cursor Bugbot for commit c9137d9. This will update automatically on new commits. Configure here.

@v0eak v0eak requested review from a team as code owners October 28, 2025 16:13
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Oct 28, 2025

🦋 Changeset detected

Latest commit: c9137d9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 74 packages
Name Type
@medusajs/medusa Major
@medusajs/js-sdk Major
@medusajs/test-utils Major
@medusajs/medusa-oas-cli Major
integration-tests-http Patch
@medusajs/draft-order Major
@medusajs/dashboard Major
@medusajs/analytics Major
@medusajs/api-key Major
@medusajs/auth Major
@medusajs/caching Major
@medusajs/cart Major
@medusajs/currency Major
@medusajs/customer Major
@medusajs/file Major
@medusajs/fulfillment Major
@medusajs/index Major
@medusajs/inventory Major
@medusajs/link-modules Major
@medusajs/locking Major
@medusajs/notification Major
@medusajs/order Major
@medusajs/payment Major
@medusajs/pricing Major
@medusajs/product Major
@medusajs/promotion Major
@medusajs/region Major
@medusajs/sales-channel Major
@medusajs/settings Major
@medusajs/stock-location Major
@medusajs/store Major
@medusajs/tax Major
@medusajs/user Major
@medusajs/workflow-engine-inmemory Major
@medusajs/workflow-engine-redis Major
@medusajs/oas-github-ci Major
@medusajs/admin-bundler Major
@medusajs/cache-inmemory Major
@medusajs/cache-redis Major
@medusajs/event-bus-local Major
@medusajs/event-bus-redis Major
@medusajs/analytics-local Major
@medusajs/analytics-posthog Major
@medusajs/auth-emailpass Major
@medusajs/auth-github Major
@medusajs/auth-google Major
@medusajs/caching-redis Major
@medusajs/file-local Major
@medusajs/file-s3 Major
@medusajs/fulfillment-manual Major
@medusajs/locking-postgres Major
@medusajs/locking-redis Major
@medusajs/notification-local Major
@medusajs/notification-sendgrid Major
@medusajs/payment-stripe Major
@medusajs/core-flows Major
@medusajs/framework Major
@medusajs/modules-sdk Major
@medusajs/orchestration Major
@medusajs/types Major
@medusajs/utils Major
@medusajs/workflows-sdk Major
@medusajs/cli Major
@medusajs/deps Major
@medusajs/telemetry Major
@medusajs/admin-sdk Major
@medusajs/admin-shared Major
@medusajs/admin-vite-plugin Major
@medusajs/icons Major
@medusajs/toolbox Major
@medusajs/ui-preset Major
create-medusa-app Major
medusa-dev-cli Major
@medusajs/ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel bot commented Oct 28, 2025

@v0eak is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

Refactor fetch handling to use a promise and async generator for streaming response.
cursor[bot]

This comment was marked as outdated.

@v0eak
Copy link
Copy Markdown
Contributor Author

v0eak commented Oct 29, 2025

Also modified the js-sdk to return before it receives its first body response, because otherwise, when a stream is established, its not possible to abort it until data is sent. in the case of workflow subscriptions, this might take a while. If we then have a component that would like to unmount its stream, it is unable to, because it has not yet received its first data.
In the example provided here, we can only abort after first data received, because

    const { stream, abort } = await sdk.client.fetchStream("/admin/stream")
    setAbortStream(() => abort)

abortStream will not be set until the await fetchStream resolves.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 1, 2025

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Dec 1, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 7, 2025

This PR was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this Dec 7, 2025
@v0eak
Copy link
Copy Markdown
Contributor Author

v0eak commented Mar 6, 2026

would love to get an update on this

@NicolasGorga NicolasGorga reopened this Mar 23, 2026
@cursor
Copy link
Copy Markdown

cursor bot commented Mar 23, 2026

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@medusa-os-bot
Copy link
Copy Markdown

medusa-os-bot bot commented Apr 9, 2026

Thank you for your contribution!

After reviewing this PR, we need a few things addressed before we can move forward:

Required changes:

  • Integration tests missing — changes to API routes in packages/medusa/src/api/ must include corresponding integration tests in integration-tests/http/__tests__/. Please add a test that exercises the fixed subscribe endpoints (e.g., verifying that workflow_id and transaction_id are correctly read from path params).
  • Changeset bump type for @medusajs/js-sdk — the PR changes FetchStreamResponse.stream from AsyncGenerator | null to non-null, which removes an existing type union that callers may depend on. Per Medusa conventions, breaking type changes require minor, not patch. Please update the changeset accordingly.
  • No linked issue — this fix addresses a real bug but there's no GitHub issue linked via a closing keyword (closes #<number>). Please open an issue describing the bug and link it so the team can triage it before the merge.

Concerns:

The core req.queryreq.params fix is clearly correct — path parameters like workflow_id and transaction_id are never available on req.query, so both subscribe routes were silently receiving undefined IDs. The route rename from [step_id]/subscribe to [transaction_id]/subscribe is also well-justified since the step_id param was declared in the URL but never used in the handler.

One concern about the JS SDK change: making fetchStream return before the fetch completes moves error handling from the await site to inside the generator body. Callers that previously checked if (!stream) to detect non-OK responses will now need a try/catch around the async iteration instead. If you keep this change, please update the JSDoc on fetchStream and ensure the docs examples show the new error-handling pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants