[codex] Fix workflow queue dedupe - #444
Conversation
|
Review changes with Changed Files |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR refactors the
Confidence Score: 3/5Not safe to merge until the migration handles existing databases correctly. The consolidated migration uses apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617111954.ts needs Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant ReviewPOST as POST /store/reviews
participant DB as Database
Client->>ReviewPOST: "POST {product_id, review_token?}"
ReviewPOST->>DB: retrieveReviewToken (if token provided)
ReviewPOST->>DB: ensureProductExists
alt No review token
ReviewPOST->>DB: "query.graph(order, filters={customer_id, items.product_id, payment_status})"
DB-->>ReviewPOST: matching orders
ReviewPOST-->>Client: 403 NOT_ALLOWED (if no orders found)
end
ReviewPOST->>DB: ensureReviewDoesNotExist
ReviewPOST->>DB: createReviewWorkflow.run()
ReviewPOST-->>Client: "200 {review}"
Note over ReviewPOST,DB: Queue side-path (separate flow)
participant Scheduler
participant QueueService as WorkflowQueueService
Scheduler->>QueueService: "listWorkflowQueueItems({dedupe_key, workflow})"
QueueService-->>Scheduler: existing items
alt Not already queued/sent
Scheduler->>QueueService: "createWorkflowQueueItems({workflow, dedupe_key, run_at, arguments:{order_id}})"
Note right of QueueService: Unique index on (workflow, dedupe_key) enforces DB-level deduplication
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant ReviewPOST as POST /store/reviews
participant DB as Database
Client->>ReviewPOST: "POST {product_id, review_token?}"
ReviewPOST->>DB: retrieveReviewToken (if token provided)
ReviewPOST->>DB: ensureProductExists
alt No review token
ReviewPOST->>DB: "query.graph(order, filters={customer_id, items.product_id, payment_status})"
DB-->>ReviewPOST: matching orders
ReviewPOST-->>Client: 403 NOT_ALLOWED (if no orders found)
end
ReviewPOST->>DB: ensureReviewDoesNotExist
ReviewPOST->>DB: createReviewWorkflow.run()
ReviewPOST-->>Client: "200 {review}"
Note over ReviewPOST,DB: Queue side-path (separate flow)
participant Scheduler
participant QueueService as WorkflowQueueService
Scheduler->>QueueService: "listWorkflowQueueItems({dedupe_key, workflow})"
QueueService-->>Scheduler: existing items
alt Not already queued/sent
Scheduler->>QueueService: "createWorkflowQueueItems({workflow, dedupe_key, run_at, arguments:{order_id}})"
Note right of QueueService: Unique index on (workflow, dedupe_key) enforces DB-level deduplication
end
Reviews (7): Last reviewed commit: "fix(queue-fix): Workflow queue migration" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (6 files)
Previous Review Summaries (5 snapshots, latest commit 9a77a8f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9a77a8f)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (7 files)
Previous review (commit c8a6609)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (7 files)
Previous review (commit 50ce286)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Resolved Issues (click to expand)CRITICAL
Existing Inline Comments (carried forward)
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous review (commit 45135c4)Status: No New Issues Found | Recommendation: Previous critical issues appear resolved; existing inline comments should be reviewed before merge Overview
Resolved Issues (click to expand)CRITICAL
Existing Inline Comments (carried forward)
Files Reviewed (2 files)
Previous review (commit da1c74a)Status: 2 Critical Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Other Observations (not in diff)No additional issues found outside the diff. Files Reviewed (4 files)
Reviewed by laguna-m.1-20260312:free · 2,352,302 tokens |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![kilo-code-bot[bot]](https://avatars.github.com/in/2193792?s=60&v=4){kind=small}
|
nemas tam ani snapshot DB ani aktualni schemu vuci modelu ... soude dle tech migration namingach tak to bylo generated ne skrze MODULE=workflow_queue mise run dev:medusa:migration:generate, pls fix |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/medusa-be/src/api/store/reviews/helpers.ts (1)
214-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten purchase-eligibility filtering in the guard query.
Line 222 only filters by
payment_status, so cancelled/archived/draft orders can still satisfy this authorisation check. Please align this guard with the existing paid-order eligibility contract and bound the existence query to a single match.Also applies to: 226-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/medusa-be/src/api/store/reviews/helpers.ts` around lines 214 - 223, The query.graph call in the purchase-eligibility check is filtering only by payment_status but allows cancelled, archived, or draft orders to pass the authorization check. Extend the filters object in the query.graph method call to explicitly exclude unwanted order statuses (cancelled, archived, draft) or restrict to only allowed statuses that align with the existing paid-order eligibility contract. Additionally, add a limit parameter to the query.graph call to bound the existence check to a single match, ensuring the query only returns one order result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/medusa-be/src/api/store/reviews/helpers.ts`:
- Around line 214-223: The query.graph call in the purchase-eligibility check is
filtering only by payment_status but allows cancelled, archived, or draft orders
to pass the authorization check. Extend the filters object in the query.graph
method call to explicitly exclude unwanted order statuses (cancelled, archived,
draft) or restrict to only allowed statuses that align with the existing
paid-order eligibility contract. Additionally, add a limit parameter to the
query.graph call to bound the existence check to a single match, ensuring the
query only returns one order result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a90b1a1f-dbf4-4847-b72b-55cdd4a97819
📒 Files selected for processing (8)
apps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/modules/workflow-queue/migrations/.snapshot-workflow-queue.jsonapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.tsapps/medusa-be/src/utils/product-review-request-queue.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Greptile Review
- GitHub Check: main
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (14)
apps/medusa-be/src/modules/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place custom Medusa modules under apps/medusa-be/src/modules
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
apps/medusa-be/**/*.{ts,tsx}: Use TypeScript for type checking - runnpx tsc --noEmitfor validation
Forbidden: Non-null assertions (!) - always use type guards and validation instead
Annotate generic field access with explicitunknowntype before type guards -const v: unknown = result[field]
Don't useas Typewithout validation - always validate before casting
UseModules.*andContainerRegistrationKeys.*constants instead of hardcoding strings
Batch operations with CHUNK_SIZE to avoid unbounded operations
Extract pure functions to separate files for testability without runtime dependencies
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/utils/product-review-request-queue.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
apps/medusa-be/**/*.{ts,tsx,js,jsx}: Use Biome linter with ultracite preset - runbunx biome check --write .and always use braces in conditionals
Comments should explain 'why', never 'what' - use self-documenting code via clear naming
Always use const per declaration -const a = 1; const b = 2is correct, one variable per line
Use nullish coalescing (??) operator instead of logical OR (||) for default values
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/utils/product-review-request-queue.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/src/modules/**/*.ts
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
apps/medusa-be/src/modules/**/*.ts: Module directories use hyphens (my-module/), module keys use underscores (my_module), and export module key as constant
Loaders cannot resolve cross-module dependencies - use__hooks.onApplicationStartfor deferred initialization instead
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/src/{api,modules}/**/*.ts
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
Use MedusaError with proper error types - INVALID_DATA(400), NOT_FOUND(404), UNAUTHORIZED(401), NOT_ALLOWED(400), DUPLICATE_ERROR(422), CONFLICT(409)
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/src/{modules,api}/**/*.ts
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
Provider ID format in DB:
{identifier}_{id}(e.g.,my_shipping_default), in container:fp_{identifier}_{id}
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Import UI components using the
@techsio/ui-kitnamespace, not@libs/ui, for runtime apps
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/utils/product-review-request-queue.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome linting and formatting only on changed files using 'bunx biome check --write path/to/file'
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/utils/product-review-request-queue.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Medusa backend custom logic should be organized in api/, modules/, workflows/, admin/, subscribers/, and jobs/ directories under apps/medusa-be/src/
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/utils/product-review-request-queue.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/**/!(medusa-be)/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use modern React patterns and React 19 for frontend applications in the monorepo
Files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.tsapps/medusa-be/src/utils/product-review-request-queue.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.tsapps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
apps/medusa-be/src/api/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place custom Medusa backend API endpoints under apps/medusa-be/src/api
Files:
apps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.ts
apps/medusa-be/src/{api,jobs}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
Use Query service for data retrieval - access via
container.resolve<Query>(ContainerRegistrationKeys.QUERY)and usequery.graph()
Files:
apps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.ts
apps/medusa-be/src/api/**/*.ts
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
apps/medusa-be/src/api/**/*.ts: Colocate validators.ts, middlewares.ts, and route.ts together - export route middlewares asMiddlewareRoute[]array
Route handlers should access validated request data viareq.validatedBody- it is type-safe and pre-validated
Files:
apps/medusa-be/src/api/store/reviews/helpers.tsapps/medusa-be/src/api/store/reviews/route.ts
apps/medusa-be/src/modules/**/models/*.ts
📄 CodeRabbit inference engine (apps/medusa-be/CLAUDE.md)
apps/medusa-be/src/modules/**/models/*.ts: Data layer indexes must be soft-delete safe - usewhere: { deleted_at: null }for unique constraints
Model definitions: Usemodel.text()for strings,model.float()for decimals,model.bigNumber()for high-precision (money)
Soft-delete index format:{ on: ["handle"], unique: true, where: { deleted_at: null } }(DML format)
Module checks: Usechecks()method with named SQL expressions for column constraints
Files:
apps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts
🧠 Learnings (1)
📚 Learning: 2026-05-18T16:32:35.366Z
Learnt from: redeyecz
Repo: TechsioCZ/new-engine PR: 409
File: apps/medusa-be/src/modules/producer/migrations/Migration20260518122326.ts:9-35
Timestamp: 2026-05-18T16:32:35.366Z
Learning: In `apps/medusa-be` MikroORM migration files under `src/modules/**/migrations/`, keep SQL schema-agnostic: do not explicitly qualify tables/indexes with a schema prefix (e.g., avoid `
Applied to files:
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.tsapps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.ts
🔇 Additional comments (10)
apps/medusa-be/src/api/store/reviews/route.ts (1)
4-5: LGTM!Also applies to: 29-31
apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610120000.ts (2)
11-13: Previous reviewer noted: hard-deleting duplicates removes queue history.Soft-deleting would satisfy the unique index whilst preserving auditability. This concern was raised in an earlier review.
14-16: Previous reviewer noted: non-concurrent index creation may lock the table.
CREATE UNIQUE INDEXwithoutCONCURRENTLYcan hold an exclusive lock on a populated table during deployment. Consider a concurrent index strategy or a scheduled deployment window. This concern was raised in an earlier review.apps/medusa-be/src/utils/product-review-request-queue.ts (3)
185-203: Previous reviewer noted: race window between pre-check and insert.The unique index prevents duplicates, but a concurrent caller passing
hasQueuedReviewRequest()can fail the insert with an unhandled unique-constraint error. Consider catching and handling the constraint violation as "already queued". This concern was raised in an earlier review.
17-67: LGTM!
116-134: LGTM!apps/medusa-be/src/modules/workflow-queue/models/workflow-queue-item.ts (1)
1-30: LGTM!apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260610110000.ts (1)
1-25: LGTM!apps/medusa-be/src/modules/workflow-queue/migrations/.snapshot-workflow-queue.json (1)
1-190: LGTM!apps/medusa-be/src/modules/workflow-queue/migrations/Migration20260617104357.ts (1)
3-20: Unable to complete verification due to technical limitations. The review comment's critical assertions about the migration chain (specifically thatMigration20260610110000dropsorder_id,Migration20260610120000addsdedupe_key, andMigration20260617104357attempts to rename a non-existent column) cannot be independently verified. Access to the migration files is required to confirm or refute these claims.
|
🎉 This PR is included in version 0.14.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Summary
This PR cleans up the workflow queue schema so the generic queue table no longer carries an order-specific
order_idcolumn. Product review reminder jobs now keep the order id in workflow arguments and use a genericdedupe_keyfor duplicate prevention.Problem
workflow_queue_itemis intended to be generic infrastructure for delayed workflow execution, but the table contained a nullableorder_idcolumn and an index on(workflow, order_id). That leaked order-domain details into the queue schema and made future workflow types less clean.Changes
order_idfrom the workflow queue model.dedupe_keytoworkflow_queue_item.(workflow, dedupe_key)for active rows wherededupe_keyis present.workflow = send-product-review-requestdedupe_key = send-review-reminder-{orderId}arguments = { order_id }dedupe_keyinstead of queryingarguments->>'order_id'.order_idcolumn/index and add/backfilldedupe_key.Validation
Migration20260610110000andMigration20260610120000applied.bunx nx run medusa-be:build; backend compilation completed, but the full build failed during admin/frontend bundling because Rolldown could not resolve@medusajs/admin-sharedfrommedusa-order-dashboard-plugin. This appears unrelated to the workflow queue changes.Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores