Skip to content

fix(core): normalize line endings in template hash calculation#826

Merged
mttrbrts merged 1 commit intoaccordproject:mainfrom
Raakshass:fix/windows-hash-mismatch
Feb 24, 2026
Merged

fix(core): normalize line endings in template hash calculation#826
mttrbrts merged 1 commit intoaccordproject:mainfrom
Raakshass:fix/windows-hash-mismatch

Conversation

@Raakshass
Copy link
Contributor

This PR fixes a cross-platform compatibility issue where template hash calculations would fail on Windows due to line-ending differences (CRLF vs LF).

The Issue

The getHash() method in cicero-core calculates a SHA-256 fingerprint based on the raw string contents of the template (Grammar, Models, Scripts, README, etc.).

  • On Linux/CI, files typically use \n (LF).
  • On Windows, files often use \r\n (CRLF).

Because the hash function did not normalize these strings, the same template would generate different hashes on different operating systems, causing unit tests (should roundtrip a template, should return a SHA-256 hash) to fail locally for Windows developers.

The Fix

I added a private _normalize(str) helper method to the Template class.

  • This method aggressively strips Carriage Return (\r) characters from strings before they are added to the content object used for hashing.
  • It is applied to:
    • Template Grammar
    • Model definitions
    • Script contents
    • Metadata README
    • Metadata Samples

Verification

Ran npm test locally on a Windows environment.

  • Before: 4 failures in Template (hash mismatch).
  • After: 157 passing tests. All hash-related tests now pass.

Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests (Existing tests now pass).
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to main from fork:branchname

Signed-off-by: siddhant jain <siddhantjainofficial26@gmail.com>
@mttrbrts mttrbrts enabled auto-merge (squash) February 14, 2026 13:15
@mttrbrts mttrbrts closed this Feb 24, 2026
auto-merge was automatically disabled February 24, 2026 09:34

Pull request was closed

@mttrbrts mttrbrts reopened this Feb 24, 2026
@mttrbrts mttrbrts enabled auto-merge (squash) February 24, 2026 09:34
@mttrbrts mttrbrts merged commit 08f3b58 into accordproject:main Feb 24, 2026
4 checks passed
@coveralls
Copy link

Pull Request Test Coverage Report for Build 22344927585

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 12 of 15 (80.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 82.401%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/cicero-core/src/template.js 12 15 80.0%
Totals Coverage Status
Change from base Build 22003091420: -0.2%
Covered Lines: 647
Relevant Lines: 772

💛 - Coveralls

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.

3 participants