[evoveotech]

Summary

Ref #1503 (deliberately not auto-closed; the issue can be closed deliberately on merge).

Every identity NOOP wrote into Apple Health embedded the active strap id (noop:<deviceId>:<kind>:<identity>). That id is not durable — it changes on a re-pair or serial-based identification — and when it changes, the delete-then-write reconciliation can no longer find the old records (its predicate is built from the id active now). The stranded records stay in Apple Health permanently as duplicates, with no way to clean them up from inside NOOP.

The fix

Drop the id segment entirely, matching the Android twin (HealthConnectWriter.clientRecordId carries no device id — noop-workout-<startTs>). The key becomes a natural key: noop:<kind>:<identity>, stable across strap lifecycle changes.

Three write paths affected:

• vitals: noop:<metricId>:<day>
• sleep: noop:sleep:<startTs>
• workouts: noop:workout:<startTs>

Heart-rate is immune (it already deletes by date range, not external UUID) and is unchanged.

One-off migration (per-type completion tracking)

A one-off sweep (migrateStrandedHealthRecords) clears records written under the OLD id-keyed scheme. It runs before the new-key writes, using HKSource.default() + date range — the same pattern the HR path uses — to reach records the new key-based delete predicate can never find. The normal writes then re-add everything under the stable keys.

Completion is tracked per-type (not a single boolean), addressing the review on the first push: a type is marked swept only when its deleteObjects returned a result (succeeded, even if it deleted zero records). A type that was declined (partial authorization) or whose delete threw is left un-swept, so a later authorization grant or a successful retry finishes the migration instead of abandoning the stranded records. The decision helpers (strandedSweepPending / strandedSweepResult) are extracted into HealthWriteback so they are unit-tested without HealthKit.

Pure, testable key helpers

The key generation is extracted into pure, testable helpers in HealthWriteback (Packages/StrandImport) so the natural-key format is pinned by swift test, not just by reading the app-target code that can't be unit-tested (HealthKit itself can't be).

Tests

HealthWritebackTests:

• testVitalKeyHasNoDeviceIdSegment — pins noop:restingHeartRate:2026-08-27, asserts no my-whoop
• testSleepKeyHasNoDeviceIdSegment — pins noop:sleep:1700000000
• testWorkoutKeyHasNoDeviceIdSegment — pins noop:workout:1700000000
• testKeysAreDeterministic — same inputs → same key
• testVitalKeysDifferByMetric — no collision across metrics on the same day
• testStrandedSweepPendingForAuthorizedUnsweptType — authorized + unswept → pending
• testStrandedSweepSkipsAlreadySweptType — already swept → not retried
• testStrandedSweepNeverAttemptsDeclinedType — declined type never pending (partial-auth)
• testStrandedSweepResultRecordsSuccessfulDelete — success → marked swept
• testStrandedSweepResultDoesNotRecordFailedDelete — failure → NOT marked (retried)
• testStrandedSweepCompletesAcrossRunsAfterLateAuthorizationGrant — late grant still swept
• testStrandedSweepRetriesFailedDeleteOnNextRun — failed delete retried next run

Test plan

• swift test over Packages/ (StrandImport tests — the pure key + sweep helpers)
• app-build.yml on-demand (the iOS app target compiles the new HealthWriteback.* call sites in HealthKitBridge)
• doc_comment_lint (no new detached doc comments)

Generated with Devin

[ryanbr]

Thanks @evoveotech — the key change is right and the risky parts are handled. I checked each of these rather than reading the summary, because this one deletes from Apple Health:

• The delete is scoped to NOOP's own records. HKQuery.predicateForObjects(from: HKSource.default()), the same pattern the existing HR path uses — it cannot reach another app's data.
• Sweep and re-write share fromTs, and the sweep runs before all four writers. There is no window where the range deleted is wider than the range restored, which is the way this shape usually loses data.
• Heart rate really is untouched. .heartRate is in highResQuantityWriteIds; the sweep iterates quantityWriteIds only.
• Dropping the device id cannot collide. This was my main concern on a two-strap install, but writeVitals merges computed and imported into byDay before writing, so noop:<metric>:<day> is unique by construction — the write-back was already one value per day rather than one per device. That is what makes the id safe to drop, and it is worth stating, because the safety comes from the merge rather than from the key.

The one-shot flag is set even when the sweep did nothing

Each delete is guarded by authorizationStatus(for:) == .sharingAuthorized, and strandedRecordsMigratedKey is then set unconditionally at the end of the function.

But partial authorization is a supported, documented state — the resume check is writeTypes.contains { ... == .sharingAuthorized }, with the comment:

Resume when at least one write type is granted so a person who intentionally declined (for example) workouts still gets sleep/vitals exported after a relaunch.

So this sequence loses the migration permanently:

1. A user has granted sleep but declined workouts.
2. The write-back resumes (one type is enough), the sweep deletes sleep records, skips workouts as unauthorized, and sets the flag.
3. The user later grants workouts.
4. The sweep never runs again. The stranded workout duplicates — the entire reason for this PR — stay in Apple Health forever, with no in-app way to clear them.

The same ending arrives if a deleteObjects call simply fails: every one is _ = try? await ..., so a failure is indistinguishable from a successful no-op, and the flag is set regardless.

This is the "fails to NOTHING rather than to something wrong, which is why it kept reading as already done" pattern from the #1919 revert. It is worse here than there: a one-shot migration has no second chance, and the records it was meant to remove cannot be reached by anything else afterwards.

Smallest fix that closes it: set the flag only when the sweep actually ran for the types that were authorized — or record it per type, so a later grant still gets swept. Either keeps the "once per install" intent while letting an install that could not complete the sweep finish it later.

Minor

Closes #1503 in the description will auto-close the issue on merge; the convention here is to avoid the closing verb next to the number so the issue can be closed deliberately.

[evoveotech]

Thanks for the review — both points addressed in 38debae4f.

The one-shot flag is set even when the sweep did nothing

Switched from a single boolean to per-type completion tracking. Each write type is marked swept only when its deleteObjects returned a result (succeeded, even if it deleted zero records). A type that was declined (partial authorization) or whose delete threw (try? returned nil) is left un-swept, so a later authorization grant or a successful retry finishes the migration instead of abandoning the records.

The partial-authorization sequence you described now completes across runs:

1. Sleep granted, workouts declined → only sleep is swept and marked; workouts stay pending.
2. User later grants workouts → the next write-back run sweeps workouts and marks them.
3. The migration completes, instead of being lost forever.

The decision helpers (strandedSweepPending / strandedSweepResult) are extracted into HealthWriteback (StrandImport) so they are unit-tested without HealthKit. New tests cover: authorized+unswept → pending, already-swept → skipped, declined → never attempted, success → marked, failure → NOT marked (retried), late-grant → still swept, failed-delete → retried next run.

Minor: Closes #1503

Edited the PR description to Ref #1503 so the issue isn't auto-closed on merge and can be closed deliberately.