Skip to content

fix(targethasher): frame rule hashes for BUG-006 - #295

Merged
sbalabanov merged 1 commit into
mainfrom
sbalabanov/bug-006
Aug 27, 2026
Merged

fix(targethasher): frame rule hashes for BUG-006#295
sbalabanov merged 1 commit into
mainfrom
sbalabanov/bug-006

Conversation

@sbalabanov

@sbalabanov sbalabanov commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix BUG-006 by type- and length-framing rule data before hashing.

Example: changing a string-list attribute from ["a", "bc"] to ["ab", "c"] previously contributed the same raw bytes (abc) to the hash, so Tango could miss a changed target. Canonical serialization makes these values distinct while retaining deterministic Bazel attribute and collection ordering.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@sbalabanov
sbalabanov marked this pull request as ready for review August 26, 2026 18:32
@sbalabanov
sbalabanov requested review from a team as code owners August 26, 2026 18:32
@sbalabanov
sbalabanov marked this pull request as draft August 26, 2026 19:17
Comment thread core/targethasher/sourcehasher.go Outdated

@sbalabanov sbalabanov left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorten diff description and list edge cases in serialization it addresses

Comment thread core/targethasher/sourcehasher.go Outdated
Comment thread core/targethasher/sourcehasher.go Outdated
@sbalabanov

Copy link
Copy Markdown
Contributor Author

Shortened the PR description and listed the BUG-006 serialization edge cases in ba296a4. The concise body now covers boundary, type/presence, ordering, nested-message, and nil-message cases.

[addressed by agent]

Comment thread core/targethasher/sourcehasher.go Outdated
Comment thread core/targethasher/sourcehasher.go Outdated
@sbalabanov
sbalabanov force-pushed the sbalabanov/bug-006 branch 2 times, most recently from 3f6e092 to d5efe0c Compare August 26, 2026 23:53

@sbalabanov sbalabanov left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update PR description with the example of the use case of the bug the PR is fixing

Comment thread core/targethasher/sourcehasher.go Outdated
Comment thread core/targethasher/sourcehasher.go Outdated
Summary:
Intent:
- Fix BUG-006 by eliminating structural collisions in rule hashing.
- Preserve deterministic, order-independent hashing for unordered rule data.

Changes:
- Add canonical encoding with field, type, presence, and length framing without an encoding version marker.
- Frame collection element types, counts, values, and nested messages before hashing.
- Add regression coverage for prior collision shapes and collection-order determinism.

---

<sub>Generated by the pr-create skill in devexp-agent-marketplace</sub>
@sbalabanov

Copy link
Copy Markdown
Contributor Author

Updated and simplified the PR description with a concrete BUG-006 failure example in baaabf9. It now shows how ["a", "bc"] and ["ab", "c"] previously serialized to the same bytes and could cause changed-target detection to miss a real update.

[addressed by agent]

@sbalabanov
sbalabanov marked this pull request as ready for review August 27, 2026 00:36
@sbalabanov
sbalabanov merged commit b93caf5 into main Aug 27, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants