P1: Keep damaged Tag source state out of hash recovery writes #938

Open
opened 2026-10-02 17:41:44 +00:00 by kayg · 1 comment
Owner

Read-only review of #664 at 884cc8ba6e1d275d14844187ed16e65f4b20a354. This is a static code finding; no builds or tests ran.

Evidence: crates/calternal-tags/src/index.rs:285 now uses the skip reader. At :306–314, an empty Tag set still enters hash-based recovery if one missing old path has the same content hash and nonempty Tags. This calls source::set_file_tags.

crates/calternal-tags/src/source.rs:522–555 returns no Tags for damaged XMP. The writer at :683–687 parses the same bytes with Adobe's parser but does not call validate_xmp_xml. The validator comment at :609–610 states that Adobe accepts some mismatched XML. The writer then serializes and replaces that packet at :721–730. Thus a skipped damaged Sidecar can still be replaced through hash recovery. Packets that Adobe rejects, and damaged folder metadata, instead cause strict writer errors and stop the rebuild.

DESIGN §2.1 and §31 K4 keep User metadata in files. #664 requires the rebuild to keep damaged bytes and rebuild valid sources. Data loss blocks a merge under CLAUDE.md.

Expected behavior: keep damaged-source state separate from absent Tags. Do not perform hash recovery writes for damaged sources. Apply XML validation at all XMP write entries.

Focused test idea: extend the damaged-source fixtures with a missing old path and a current path that share one content hash. Cover folder metadata, rejected XMP, and XMP accepted by Adobe but rejected by the XML validator. Require exact damaged-byte preservation and successful rebuild of valid sources.

Searches before filing: damaged Sidecar, Tag reconcile, and Sidecar; no separate matching hash-recovery issue found. Keep the recovery and writer validation fix under one owner.

Read-only review of #664 at `884cc8ba6e1d275d14844187ed16e65f4b20a354`. This is a static code finding; no builds or tests ran. Evidence: `crates/calternal-tags/src/index.rs:285` now uses the skip reader. At `:306–314`, an empty Tag set still enters hash-based recovery if one missing old path has the same content hash and nonempty Tags. This calls `source::set_file_tags`. `crates/calternal-tags/src/source.rs:522–555` returns no Tags for damaged XMP. The writer at `:683–687` parses the same bytes with Adobe's parser but does not call `validate_xmp_xml`. The validator comment at `:609–610` states that Adobe accepts some mismatched XML. The writer then serializes and replaces that packet at `:721–730`. Thus a skipped damaged Sidecar can still be replaced through hash recovery. Packets that Adobe rejects, and damaged folder metadata, instead cause strict writer errors and stop the rebuild. DESIGN §2.1 and §31 K4 keep User metadata in files. #664 requires the rebuild to keep damaged bytes and rebuild valid sources. Data loss blocks a merge under CLAUDE.md. Expected behavior: keep damaged-source state separate from absent Tags. Do not perform hash recovery writes for damaged sources. Apply XML validation at all XMP write entries. Focused test idea: extend the damaged-source fixtures with a missing old path and a current path that share one content hash. Cover folder metadata, rejected XMP, and XMP accepted by Adobe but rejected by the XML validator. Require exact damaged-byte preservation and successful rebuild of valid sources. Searches before filing: `damaged Sidecar`, `Tag reconcile`, and `Sidecar`; no separate matching hash-recovery issue found. Keep the recovery and writer validation fix under one owner.
Author
Owner

Fixed in commit b45febe5f: reconciliation now keeps damaged-source state separate from an empty Tag set and skips hash recovery for damaged folder metadata or XMP Sidecars. XMP writes also validate packet XML before Adobe's permissive parser can serialize it. Regression coverage uses a vanished Tag path with a matching current content hash, checks exact source-byte preservation, and proves other valid sources still rebuild.

Focused verification:

running 2 tests
test tests::hash_recovery_skips_damaged_folder_metadata ... ok
test tests::hash_recovery_skips_damaged_xmp_sidecars ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 13 filtered out; finished in 24.86s
Fixed in commit `b45febe5f`: reconciliation now keeps damaged-source state separate from an empty Tag set and skips hash recovery for damaged folder metadata or XMP Sidecars. XMP writes also validate packet XML before Adobe's permissive parser can serialize it. Regression coverage uses a vanished Tag path with a matching current content hash, checks exact source-byte preservation, and proves other valid sources still rebuild. Focused verification: ```text running 2 tests test tests::hash_recovery_skips_damaged_folder_metadata ... ok test tests::hash_recovery_skips_damaged_xmp_sidecars ... ok test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 13 filtered out; finished in 24.86s ```
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#938
No description provided.