[otavio]

Summary

Both device cleanups lowered each namespace's device count by the number of devices they listed. Two overlapping runs list the same devices, but the second run's DELETE blocks on the first run's row locks and then deletes nothing. It still subtracted its whole list, so the count dropped twice and stayed wrong until the 03:00 count sync.

 deviceCleanup / ephemeralCleanup
   devices := list expired
-  count per tenant (and status) from devices
-  DeviceDeleteMany(uids)                 -> int64
+  deleted := DeviceDeleteMany(uids)      -> []store.DeletedDevice{UID, TenantID, Status}
+  count per tenant (and status) from deleted
   NamespaceIncrementDeviceCount(tenant, status, -count)

DeviceDeleteMany now runs DELETE ... RETURNING id, namespace_id, status, so it reports only rows this call removed. DeviceDelete keeps its ErrNoDocuments contract on top of it.

Scheduled runs don't overlap, because both jobs are registered asynq.Unique(), which holds a Redis lock across replicas. Overlap takes a run queued outside the scheduler (the e2e suite's RunCron, an asynq CLI enqueue), or SHELLHUB_ASYNQ_UNIQUENESS_TIMEOUT=0. The automated review of #7600 raised it, since that PR's cleanup tests trigger the crons by hand while the scheduler is still running.

Evidence

• Before: the new unit cases list three devices and have the store delete fewer, as a concurrent run would leave it. On the old code both cleanups subtracted all three:

  --- FAIL: TestService_DeviceCleanup/decrements_the_counters_only_by_the_devices_it_deleted
  --- FAIL: TestService_EphemeralCleanup/decrements_the_counters_only_by_the_devices_it_deleted
          mock: Unexpected Method Call
  

  After: go test ./api/services/ passes, and so does TestPgStore/DeviceStore, including two new store cases:

  • returns_the_namespace_and_status_of_each_device_it_deleted
  • leaves_out_a_device_an_earlier_call_already_deleted

  golangci-lint reports 0 issues and go mod tidy -diff is clean.

Merge Danger

Door: two-way

No migration, wire format or API change. DeviceDeleteMany is a server-internal store method; cloud doesn't call it.

Blast Radius: narrow

Only the two cleanup crons and DeviceDelete go through the changed method.

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 1m 56s —— View job

---

Code Review Complete

Reviewed 6 files on two axes: Standards (repo rules, Go conventions, tests, code smells) and Correctness (security and runtime defects). No new issues found, and the code looks good as it stands. The Spec axis did not run because the PR closes no issue and its commit trailers cite none, so there is nothing to check it against.