Skip to content

Conversation

@zarko-a
Copy link

@zarko-a zarko-a commented Oct 24, 2025

This PR fixes the issue reported in 265.

As described in that issue output of find wasn't consistent when running with various container images which was causing The hash of the directory does not match the expected value to be emitted until the maximum of 15 was reached and then it workflow steps would continue.

Emitting of that message happened twice for each step in the workflow which caused delays of about 30s [for each step], making it more than just a nuisance.

PR introduces sorting of the find output before hash is computed and compared. Making resulting hash consistent which fixes the comparison. After the fix, runner doesn't emit any unexpected hash values messages.

Initially I tested with just piping the output to Linux's sort command - that worked, but it's hard to predict if sorting would be consistent across different versions and platforms. Sorting in TS seems most reliable since runner uses the same Node binary between runner and workflow (at least I assume as much).

I tested in our environment by running a custom image with these changes. Seems to work fine.
Screenshots of before and after below.

@zarko-a zarko-a requested a review from nikola-jokic as a code owner October 24, 2025 21:50
Copilot AI review requested due to automatic review settings October 24, 2025 21:50
@zarko-a zarko-a requested a review from a team as a code owner October 24, 2025 21:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses inconsistent hash calculations caused by non-deterministic find command output across different container images. The solution sorts file listings before hashing to ensure consistency, eliminating spurious "hash mismatch" errors and reducing step delays.

Key Changes:

  • Modified hash calculation to sort output lines before hashing instead of streaming directly to hash
  • Renamed functions to execCalculateOutputHashSorted and localCalculateOutputHashSorted to reflect new behavior
  • Updated all callers to use the renamed functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +325 to +330
const sortedOutput =
output
.split('\n')
.filter(line => line.length > 0)
.sort()
.join('\n') + '\n'
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The filter removes empty lines but unconditionally adds a trailing newline. If the original output had no trailing newline, this changes the hash value. Consider preserving the original trailing newline behavior: const hasTrailingNewline = output.endsWith('\n'); const sortedOutput = output.split('\n').filter(line => line.length > 0).sort().join('\n') + (hasTrailingNewline ? '\n' : '');

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Trailing new line is added consistently so the resulting hash would be consistent as well.

Comment on lines +353 to +358
const sortedOutput =
output
.split('\n')
.filter(line => line.length > 0)
.sort()
.join('\n') + '\n'
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The filter removes empty lines but unconditionally adds a trailing newline. If the original output had no trailing newline, this changes the hash value. Consider preserving the original trailing newline behavior: const hasTrailingNewline = output.endsWith('\n'); const sortedOutput = output.split('\n').filter(line => line.length > 0).sort().join('\n') + (hasTrailingNewline ? '\n' : '');

Suggested change
const sortedOutput =
output
.split('\n')
.filter(line => line.length > 0)
.sort()
.join('\n') + '\n'
const hasTrailingNewline = output.endsWith('\n')
const sortedOutput =
output
.split('\n')
.filter(line => line.length > 0)
.sort()
.join('\n') + (hasTrailingNewline ? '\n' : '')

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

Same as above, trailing new line is added consistently so the resulting hash would be consistent as well.

@zarko-a
Copy link
Author

zarko-a commented Oct 24, 2025

Before:

image image

After:

image image

@nikola-jokic
Copy link
Collaborator

Hey @zarko-a,

Thank you for the PR. The PR looks good overall. My only worry is retrieving everything in memory and then sorting it. For the temp directory, it shouldn't cause any issues, but when copying the work directory, the output can potentially be large.

But since essentially, these are only with their sizes, I might be worrying for no reason. Let's move forward with this change.

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.

2 participants