-
Notifications
You must be signed in to change notification settings - Fork 6
fix(purchases): updatePlanProgress non-atomic read-modify-write (05-L4) #1071
Copy link
Copy link
Closed
Labels
effort/sHoursHoursimpact/fewLimited audienceLimited audiencepr-createdA PR has been opened for this issue (dedup guard for the auto-PR loop)A PR has been opened for this issue (dedup guard for the auto-PR loop)pr-mergedThe PR for this issue has been mergedThe PR for this issue has been mergedpriority/p2Backlog-worthyBacklog-worthyseverity/mediumModerate harmModerate harmtriagedItem has been triagedItem has been triagedtype/bugDefectDefecturgency/this-sprintWithin the current sprintWithin the current sprint
Description
Activity
Metadata
Metadata
Assignees
Labels
effort/sHoursHoursimpact/fewLimited audienceLimited audiencepr-createdA PR has been opened for this issue (dedup guard for the auto-PR loop)A PR has been opened for this issue (dedup guard for the auto-PR loop)pr-mergedThe PR for this issue has been mergedThe PR for this issue has been mergedpriority/p2Backlog-worthyBacklog-worthyseverity/mediumModerate harmModerate harmtriagedItem has been triagedItem has been triagedtype/bugDefectDefecturgency/this-sprintWithin the current sprintWithin the current sprint
Problem
updatePlanProgressperforms a read-modify-write on thePurchasePlan.CurrentStepfield without locking or a database-level atomic increment.In a multi-tick cron scenario (or if two Lambda invocations overlap on the same plan), both invocations can read the same
CurrentStepvalue, both increment it, and both writeCurrentStep+1— skipping a step or landing the plan at the wrong step index.Evidence
Report 05, finding 05-L4 (
updatePlanProgressnon-atomic RMW). Already noted as tracked by issues I-02/I-03 (#1013/#1014) in the fold-1037 plan; filed here as a standalone so it has its own issue for tracking and triage.Fix
Use
FOR UPDATEadvisory locking or a database-levelSET current_step = current_step + 1 WHERE ... RETURNING *(atomic increment) so concurrent callers converge on the correct step regardless of ordering.Files
internal/config/store_postgres.go(or whereverUpdatePurchasePlan/updatePlanProgresslive)Intentionally NOT part of #1037: the CAS-per-execution-row approach chosen in #1037 is sound and is not affected by this plan-level RMW. This is a separate concern at the plan layer.