A single digit modification is not detected by isomorphic-git's status call

[scottmmorris]

Describe the bug

When making a change in a file using await vault.commit(...) and changing only a single number on that file will sometimes cause isomorphic-git to not recognise that the file has been modified and therefore if no other changes were made a commit will not be made even though a file was changed.

To Reproduce

1. Add a file using await vault.commit

      const secretVer1 = {
        name: secretName,
        content: 'Secret-1-content-ver1',
      };

2. Modify the file using await vault.commit

      const secretVer2 = {
        name: secretName,
        content: 'Secret-1-content-ver2',
      };

3. Check the log using await vault.log to see if the most recent modification shows up.

NOTE: This won't happen ALL the time. It seems to be most common when running large groups of tests together?

Expected behavior

We expect any file change, even a single digit change to be detected by isomorphic-git and therefore create a new commit when vault.commit is used.

Additional context

• Most likely related to this issue on isomorphic-git
• This issue has given a possible workaround by using resetIndex on each file in the vault
• Discussion about the problem is here
• This issue created for EFS has the potential to resolve this issue

Resolved

This was addressed by doing the following.

1. All contents of the vault are added using git.add before processing the statusMatrix. Making sure everything is staged beforehand essentially solves the issue.
2. Simplified the logic for handling the statusMatrix to a much simpler switch statement.

[scottmmorris]

The three possible solutions for that I see at the moment:

1. Make every call to vault.commit cause a git commit, even if no changes are made. The issue with this is that sometimes a commit message would be wrong. Acutally, this might not work I've just realised. You would need to do git.add on the 'unmodified file. So I think this would be the worst option.
2. Recursively scan over the vault and for each file call resetIndex. Preliminary testing shows this works although the drawback of this is that for large vaults performance will be significantly decreased. I think this should be the short term solution
3. Implement our event watching API for efs so we dont have to rely on isomorphic git and statusMatrix. We coudl then add and remove files based on the event watching information. I think this is the best long term solution.

[scottmmorris]

To clarify what I am currently doing (point 2 on the above comment:

1. Get the status of all files using statusMatrix provided by isomorphic git
2. Iterate over the status of each file
3. If the file is marked as unmodified, reset the index for this file
4. Check the status of the file again using statusMatrix
5. If still marked as unmodified then pass over this file
6. Otherwise assign the new status and go through the automatic commit message check as normal

I had to change this slightly to specifically check for unmodified files rather than resetting the index for all files because isomorphic git handles directories in an odd way and so resetting the index for them removed some changes that needed to be kept. This way is more robust I think because we are only touching the files that would be affected by this bug.

However, this just goes to show that there could be more bugs arising from this hack. Best to switch over to using the event watching API for EFS asap.

[joshuakarp]

• Structured Change Detection for Mutation Events and Schema Compliance js-encryptedfs#55 would resolve this issue.

[CMCDragonkai]

@tegefaulkes you might want to test this too in #709.

[tegefaulkes]

Digging into this, It seems calling git.add() on all the secrets before processing the message and committing effectively solves the issue. I'll need to re-jigger some logic to work this way however.

[tegefaulkes]

I've simplified the logic some. All the same tests are passing so it's looking good.

The solution was to just add all the files using git.add. We can then construct the message the same way using the status matrix. I haven't seen the bug since I've made the changes.

[CMCDragonkai]

Appears to be a timing problem. Suggest doing a await sleep(1) to mitigate if mtime is millisecond resolution. Verify this @tegefaulkes for a quickfix.

[tegefaulkes]

Added a commit to staging 62945f0 to wait the 1 ms. We already have tests for regressions for this behaviour.

I'm pretty sure it's fixed. I could re-create the issue with the original code but haven't seen it happen at all with the current changes.