Skip to content

Fix stale sync claim ownership - #23

Merged
vicentereig merged 1 commit into
mainfrom
codex/fix-stale-sync-claim-ownership
Jul 5, 2026
Merged

vicentereig merged 1 commit into
mainfrom
codex/fix-stale-sync-claim-ownership

Conversation

@vicentereig

@vicentereig vicentereig commented Jul 5, 2026 •

Copy link
Copy Markdown
Owner

Fixes the stale-claim ownership scenario that came out of the review on #20.

@hxu this is the follow-up I mentioned: once an in_progress task can be recovered as stale and claimed again, the old worker must not be able to complete or fail the newer claim by task id alone.

The first version of this PR fenced the final completed / failed transition with (task_id, claim_attempt). The review tightened the invariant: the claim token has to cover the whole compound action that creates durable queue effects.

Scenario covered:

P0 claims activity page #4839, attempts=1, in_progress_at=10:00
P0 fetches activities and then stalls
P1 starts later and recovers stale #4839 back to pending
P1 claims #4839, attempts=2
P0 wakes up with the old page result
P0 mark_completed_with_followups(#4839, attempts=1, gpx_tasks, next_page) must fail
No GPX tasks or next page are inserted for the stale owner
P1 remains the only claim allowed to complete #4839 and enqueue follow-up work

Changes:

  • Require the active claim attempt when marking sync tasks completed or failed.
  • Carry the claimed attempt from producer to consumer through SyncData.
  • Add a claim-guarded SQLite transaction for complete source task + enqueue follow-up tasks.
  • Move activity GPX / next-page queue insertion behind that guarded completion path.
  • Add regression coverage for both stale completion and stale follow-up enqueue attempts.

Validation:

  • cargo fmt --all -- --check
  • cargo test -p garmin-cli stale_claim
  • cargo test -p garmin-cli
  • cargo clippy --all-targets --all-features -- -D warnings

@vicentereig
vicentereig force-pushed the codex/fix-stale-sync-claim-ownership branch from ec9802d to fb27193 Compare July 5, 2026 15:17
@vicentereig
vicentereig merged commit 2ebd450 into main Jul 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant