Claim sync tasks atomically

[hxu]

Why: parallel sync workers need to claim tasks without racing each other.

Example:

P0  Activities  #4855 Activities 200-250  waiting for rate limit 0s
P1  Performance #4236 2015-01-12          waiting for rate limit 1s
P2  GPX         #4839 2026-03-17 Running  waiting for rate limit 1s
P3  Health      #8 2014-12-07             waiting for rate limit 0s

With multiple workers polling the same SQLite queue, claiming a task should be one atomic operation.

Changes:

• Add an atomic pop-and-mark-in-progress queue path.
• Track in-progress task start times in sync storage.
• Expose active task details for status reporting.

[vicentereig]

@hxu I think the diagnosis here is real, but this PR does not complete the fix yet.

The current producer path still does “read a task” and “mark it in progress” as two separate operations:

• sync/mod.rs:1281 calls queue.pop_round_robin_with_pipeline(...) / queue.pop_round_robin()
• sync/mod.rs:1311 later calls queue.mark_in_progress(task_id)

That means the new claim helper added in task_queue.rs is not on the hot path.

A concrete failure trace still looks like this:

P0: pop_round_robin() -> task #4839, status is still pending
P1: pop_round_robin() -> task #4839, status is still pending
P0: mark_in_progress(#4839) -> attempts = 1
P1: mark_in_progress(#4839) -> attempts = 2
P0/P1 both fetch and send the same SyncData
C0/C1 both write/complete the same logical task

For activity page tasks this can fan out:

P0 and P1 both process Activities 200-250
P0 and P1 both enqueue GPX tasks discovered from that page
P0 and P1 both enqueue next Activities page 250-300

Because push_task has no uniqueness constraint, duplicate follow-up work can persist in the queue.

What “done” should guarantee from the outside:

Initial:
sync_tasks
  id=4839 task_type=download_gpx status=pending attempts=0

Expected with 4 producers:
P0 claim -> id=4839 status=in_progress attempts=1 in_progress_at=...
P1 claim -> some other eligible id, or None
P2 claim -> some other eligible id, or None
P3 claim -> some other eligible id, or None

Invariant:
No two producers receive the same sync_tasks.id from the claim call.
Each claimed task increments attempts exactly once.
A page task enqueues its GPX tasks and next-page task once per claimed page.

Minimum fix: wire producer_loop to call the new helper directly, for both pipeline-filtered and unfiltered cases, and remove the separate mark_in_progress call. That is the place where this PR currently stops short.

Better fix: move the claim all the way into SyncDb as one SQLite-level claim operation, not just a SharedTaskQueue mutex wrapper. Right now pop_round_robin_mark_in_progress still does SELECT then UPDATE internally, and mark_task_in_progress updates by WHERE id = ? without checking the previous status or affected row count. A robust DB-level claim should either use a transaction around select+update or an UPDATE ... WHERE id = (SELECT ...) RETURNING ... shape, so it remains correct even across multiple DB connections or CLI processes.

One more follow-through: the PR body says active task details are exposed for status reporting, but garmin sync status still only prints aggregate pending count. list_in_progress_tasks is added but not wired into the status command yet.

[hxu]

@vicentereig I've had my agent address the concern and then in a new context did a review too, which found another issue around multiple sync processes that I've also addressed as well.

Normally I would try to understand the code a bit more but unfortunately have not had the time 😅

[hxu]

On the status, I think that's staged in a follow up PR, so once this is merged I'll open the additional PRs.

[vicentereig]

I appreciate the time you are spending!

wrt: the "code" - I usually focus more on running scenarios through it and looking at the traces. I ask the model to do that for me, reads the code and shows me traces or stdout.

that helps understanding without relying on the code how the thing works or how it will work (including if necessary the unhappy paths).
Eventually use all that to get the agent generating the right piece of code using those scenarios to write tests.

[vicentereig]

Thanks for the follow-up here. This addresses the main thing I called out in the first review: the producer now uses the claim path, and the claim is pushed down into SQLite with UPDATE ... RETURNING, which is the right direction.

I did somewhat miss one scenario in my initial review: stale claims.

The trace I have in mind is:

P0 claims task #4839, attempts=1, in_progress_at=10:00
P0 stalls but is still alive
P1 starts later and recovers stale #4839 back to pending
P1 claims #4839, attempts=2
P0 wakes up and finishes
P0 mark_completed(#4839) succeeds, even though #4839 now belongs to P1's claim

So the remaining issue is not the first claim anymore. It is claim ownership after recovery. task_id alone is not enough once a stale task can be recovered and claimed again. The completion/failure path needs to prove it is completing the currently active claim, probably by carrying the claimed attempts value through and updating with something like WHERE id = ? AND attempts = ? AND status = 'in_progress'.

I’m going to merge this PR because it fixes the original race materially and moves the system forward. I’ll open a follow-up PR for the stale-claim ownership fix with a scenario test around the trace above.