fix(files): physical-generation snapshots reject database-loaded observation timestamps

[kevinheneveld]

The bug

AudiobookFile.ApplyPhysicalObjectIdentity guards that its observedAtUtc argument has DateTimeKind.Utc — but SQLite/EF materializes the stored PhysicalIdentityObservedAtUtc column as Unspecified. Two paths re-apply that database-loaded value verbatim and throw:

• AudiobookFileService.ClonePhysicalGeneration — hit live on my instance: any rescan of a book that has an existing physical identity fails its scan job with System.ArgumentException: Physical identity observation time must be UTC. (stack: RefreshPhysicalGenerationCoreAsync → ClonePhysicalGeneration). This bites more often after upgrading across the v1.3.0→v1.3.1 identity-scheme change, since stale identities now commonly trigger generation refreshes.
• EfAudiobookFileRepository.ApplyPhysicalGeneration — same exposure when its source row came from the database.

The fix

Normalize with DateTime.SpecifyKind(value, DateTimeKind.Utc) at both re-apply sites — the same treatment RootFolderRelocationService.MetadataPlanning already applies to this exact column, so this just closes the two spots that were missed.

🤖 Generated with Claude Code

[therobbiedavis]

I think we're repairing this one layer too late. PhysicalIdentityObservedAtUtc is UTC-by-contract — AudiobookFile.ApplyPhysicalObjectIdentity rejects anything else — but SQLite materialization currently allows an AudiobookFile to leave persistence with this property set to DateTimeKind.Unspecified.

Rather than requiring each consumer to know that persisted AudiobookFile timestamps need repairing, can we normalize this when EF materializes PhysicalIdentityObservedAtUtc, ideally in AudiobookFileConfiguration with a value conversion?

That would restore the entity invariant at the persistence boundary and keep ClonePhysicalGeneration persistence-agnostic. Otherwise the next consumer that reuses this property can reproduce the same failure.

[therobbiedavis]

Can you clarify the production path this is intended to fix?

ApplyPhysicalGeneration appears to be used only by the non-relational fallback. SQLite takes the IsRelational() / ExecuteUpdateAsync path, and the source passed here is the caller-provided replacement rather than the database-loaded existing row.

So I don't think this method represents the SQLite failure described in the PR body. If we normalize PhysicalIdentityObservedAtUtc at EF materialization instead, I think this workaround becomes unnecessary as well.

If there is a path where a database-materialized source reaches this method, could we add a regression test demonstrating it?

[therobbiedavis]

The underlying bug looks valid, but I'd like regression coverage before we merge this.

Our normal BaseTests configuration uses EF InMemory, which preserves the DateTime.Kind and therefore cannot reproduce the SQLite behavior that caused this failure. The regression test needs to cross an actual SQLite persistence boundary:

Create an AudiobookFile with a UTC physical-identity observation timestamp.
Save it through SQLite.
Dispose that context.
Load the row through a fresh context.
Verify the loaded entity still satisfies the UTC contract.

I'd also like a test that drives the reported failure through RefreshPhysicalGenerationAsync using a persisted/reloaded file, since that proves the original ClonePhysicalGeneration exception cannot recur.

The fresh context is important here: an existing SQLite repository test saves and reloads through the same context, so EF can return the already-tracked UTC object instead of materializing the value back from SQLite. That's why the existing test suite doesn't catch this.

[kevinheneveld]

Thanks for the review — all three points addressed in the follow-up commit:

• Persistence-boundary fix: the repair now lives in AudiobookFileConfiguration as a materialization value conversion on PhysicalIdentityObservedAtUtc, so a loaded AudiobookFile always satisfies its own UTC contract and ClonePhysicalGeneration stays persistence-agnostic. Both consumer-side SpecifyKind patches are reverted.
• EfAudiobookFileRepository.ApplyPhysicalGeneration: you're right — that's the non-relational fallback and its source is the caller-provided replacement, not a database-loaded row. Reverted; no production path there matches the reported failure.
• Regression coverage (both crossing a real SQLite boundary):
  • AudiobookFilePhysicalIdentityTimestampTests saves through one context, disposes it, reloads through a fresh context on the same connection, and asserts the loaded timestamp keeps Utc kind and re-applies through the guarded domain method.
  • RefreshPhysicalGenerationAsync_SqlitePersistedFile_SnapshotsPredecessorWithoutUtcKindViolation drives the reported failure end-to-end against a SQLite-backed provider, with registration and refresh in separate resolutions so the refresh materializes the persisted row rather than reusing a tracked entity. Verified red without the conversion (it throws the exact original ArgumentException) and green with it.

Test infra note: ServiceCollectionBuilder gained an opt-in WithSqliteDatabase(connection) so service-level tests can run over real SQLite materialization when a regression depends on it — default remains InMemory.