Split out of LeanerCloud/cloud-commitments-cli#1516, where it was documented (commit c47179567) rather than fixed, deliberately.
Problem
releaseSkippedCollectionMarker scopes its release by a token that identifies the marker, not the invocation.
Under Lambda's at-least-once async delivery, two deliveries of the same event carry the same token. One wins the advisory lock and proceeds to collect; the other is lock-skipped and, seeing a token it considers its own, clears the marker while the first is still collecting.
Effect
- The collection-in-progress banner drops early.
- A refresh during the remaining collection window returns 202 without actually collecting.
Bounded and self-healing, on no money or data-integrity path. Releasing on lock-skip is still strictly better than not releasing, because cron overlap is routine while duplicate async delivery is rare. That is why LeanerCloud/cloud-commitments-cli#1516 shipped as-is.
Proper fix
Invocation-scoped identity rather than marker-scoped: the lock winner re-stamps the owner column with a fresh token once it has the lock, so a lock-skipped duplicate no longer matches and declines to clear.
This is a design change, not a patch. LeanerCloud/cloud-commitments-cli#1516 deliberately avoided bolting on a half-measure.
Related, separate
internal/scheduler/scheduler_test.go:857 carries the same vacuous AssertNotCalled shape that LeanerCloud/cloud-commitments-cli#1516 fixed elsewhere (MockConfigStore short-circuits unregistered methods before mock.Called, so the assertion walks an empty call list and can never fail). Its comment wrongly claims testify would panic. It was outside LeanerCloud/cloud-commitments-cli#1516's diff and left alone rather than fixed drive-by. The pattern likely recurs across the repo's ~147 AssertNotCalled call sites and deserves its own audit.
Split out of LeanerCloud/cloud-commitments-cli#1516, where it was documented (commit
c47179567) rather than fixed, deliberately.Problem
releaseSkippedCollectionMarkerscopes its release by a token that identifies the marker, not the invocation.Under Lambda's at-least-once async delivery, two deliveries of the same event carry the same token. One wins the advisory lock and proceeds to collect; the other is lock-skipped and, seeing a token it considers its own, clears the marker while the first is still collecting.
Effect
Bounded and self-healing, on no money or data-integrity path. Releasing on lock-skip is still strictly better than not releasing, because cron overlap is routine while duplicate async delivery is rare. That is why LeanerCloud/cloud-commitments-cli#1516 shipped as-is.
Proper fix
Invocation-scoped identity rather than marker-scoped: the lock winner re-stamps the owner column with a fresh token once it has the lock, so a lock-skipped duplicate no longer matches and declines to clear.
This is a design change, not a patch. LeanerCloud/cloud-commitments-cli#1516 deliberately avoided bolting on a half-measure.
Related, separate
internal/scheduler/scheduler_test.go:857carries the same vacuousAssertNotCalledshape that LeanerCloud/cloud-commitments-cli#1516 fixed elsewhere (MockConfigStoreshort-circuits unregistered methods beforemock.Called, so the assertion walks an empty call list and can never fail). Its comment wrongly claims testify would panic. It was outside LeanerCloud/cloud-commitments-cli#1516's diff and left alone rather than fixed drive-by. The pattern likely recurs across the repo's ~147AssertNotCalledcall sites and deserves its own audit.