Share content-hash across storage backends; drop unused Storage.stat() - #118
Merged
Conversation
in-jun
commented
Jul 18, 2026
in-jun
left a comment
Owner
Author
There was a problem hiding this comment.
Reviewed the full diff against the surrounding code. Both cleanups land cleanly.
#19 — shared content hash
ContentHash.sha256Hexcentralizes the algorithm correctly:HashingSource.sha256streamed intoblackholeSink(), digest read viaByteString.hex(). The source is consumed and closed inside the helper, so both call sites just hand it a fresh source.- The output encoding is byte-identical to the previous hand-rolled version —
hex()and"%02x".formatboth emit lowercase hex of the same SHA-256 — so hashes already persisted in the ancestor store stay comparable. No migration hazard, and converged files won't suddenly read as conflicts. - The SMB handle/stream ordering is right: hashing consumes the whole file, so releasing the
InputStream(via the okioSource) before the outer.usecloses the handle is safe, and the comment captures why it differs fromread(). okio is already a dependency of both core-sync and app. hashFile/hashRemotestay live throughscan(), so nothing goes dead underallWarningsAsErrors.
#21 — drop stat()
- No production caller remained; the only three call sites were tests, all migrated to
probe(). The assertions that moved (mtimeMillisequality, null-on-absent) are semantically preserved, andprobe()is the cheaper check since it skips hashing. - Keeping
move()is justified — it is used in production byConflictManagerfor keep-both resolution.
Contract docs on Storage.scan and FileMeta.contentHash make the cross-backend invariant explicit, which is the real point of the change. No regressions or scope creep. Looks good to merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related cleanups to the
Storageport contract.MessageDigest+ hex-join. Added a single sharedContentHash.sha256Hexincore-sync(streaming, via okio'sHashingSource) and pointed bothDirectFileLocalStorageandSmbRemoteStorageat it, so a new backend can't silently diverge on algorithm or encoding. Documented the invariant onStorage.scanandFileMeta.contentHash.Storage.stat(), which had no production caller (only tests, which now useprobe()). Keptmove(): it is now used in production byConflictManagerfor "keep both" resolution, so it is a live method rather than port bloat.Fixes #19
Fixes #21