Avoid using short commit IDs

[camertron]

My team and I ran into a wild issue with changesets today that has prompted me to open this PR.

Setup

We're using GitHub Actions and saw the following output in one of our release builds:

The following error was encountered while generating changelog entries
We have escaped applying the changesets, and no files should have been affected
🦋  error TypeError: Cannot read properties of null (reading 'author')
🦋  error     at Object.getInfo (/home/runner/work/primitives/primitives/node_modules/@changesets/get-github-info/dist/get-github-info.cjs.dev.js:235:12)
🦋  error     at processTicksAndRejections (node:internal/process/task_queues:96:5)
🦋  error     at async /home/runner/work/primitives/primitives/node_modules/@changesets/changelog-github/dist/changelog-github.cjs.dev.js:122:13
🦋  error     at async Object.getReleaseLine (/home/runner/work/primitives/primitives/node_modules/@changesets/changelog-github/dist/changelog-github.cjs.dev.js:99:19)
🦋  error     at async Promise.all (index 0)
🦋  error     at async generateChangesForVersionTypeMarkdown (/home/runner/work/primitives/primitives/node_modules/@changesets/apply-release-plan/dist/apply-release-plan.cjs.dev.js:192:22)
🦋  error     at async getChangelogEntry (/home/runner/work/primitives/primitives/node_modules/@changesets/apply-release-plan/dist/apply-release-plan.cjs.dev.js:247:109)
🦋  error     at async /home/runner/work/primitives/primitives/node_modules/@changesets/apply-release-plan/dist/apply-release-plan.cjs.dev.js:420:[21](https://github.com/primer/primitives/actions/runs/6267066703/job/17044280307#step:8:22)
🦋  error     at async Promise.all (index 0)
🦋  error     at async Object.applyReleasePlan [as default] (/home/runner/work/primitives/primitives/node_modules/@changesets/apply-release-plan/dist/apply-release-plan.cjs.dev.js:[31](https://github.com/primer/primitives/actions/runs/6267066703/job/17044280307#step:8:32)1:31)

Looks like it's trying to access the author property of null. After looking at the changesets code, it appears the error happens when processing the result of executing a GraphQL query via the GitHub API. After sticking some console.log statements inside the changesets code, I was able to print out the query:

query {
    a0: repository(owner: "primer" name: "primitives") {
        a51dbba8: object(expression: "51dbba8") {
            ... on Commit {
                commitUrl
                associatedPullRequests(first: 50) {
                    nodes {
                        number
                        url
                        mergedAt
                        author {
                            login
                            url
                        }
                    }
                }
                author {
                    user {
                        login
                        url
                    }
                }
            }
        }
        a2565a2d: object(expression: "2565a2d") {
            ... on Commit {
                commitUrl
                associatedPullRequests(first: 50) {
                    nodes {
                        number
                        url
                        mergedAt
                        author {
                            login
                            url
                        }
                    }
                }
                author {
                    user {
                        login
                        url
                    }
                }
            }
        }
    }
}

You should be able to copy/paste this query into GitHub's GraphQL explorer and see the following response (modified for brevity):

{
  "data": {
    "a0": {
      "a51dbba8": null,
      "a2565a2d": {
        "commitUrl": "https://github.com/primer/primitives/commit/2565a2d30dbca13b73e21064036910e269385770",
        "associatedPullRequests": {
          "nodes": [
            { ... }
          ]
        },
        "author": {
          "user": { ... }
        }
      }
    }
  }
}

Notice the full and complete response for commit 2565a2d but a null value for commit 51dbba8.

Diagnosis

So what the heck? Running git rev-parse 51dbba8 returns the full commit ID of 51dbba881eea1cd1a2d0c601023c9d4b8357b64d, which definitely exists.

My only idea, although very unlikely, was a hash collision.

$> git cat-file -t 51dbba8
error: short object ID 51dbba8 is ambiguous
hint: The candidates are:
hint:   51dbba881 commit 2023-09-21 - update diffStat tokens (#728)
hint:   51dbba865 tree
fatal: Not a valid object name 51dbba8

Bingo! There is a hash collision! 51dbba8 is a prefix shared by both a commit and a tree object.

Using the full SHA (or even just one additional digit) in the GraphQL query returns non-null data for both the commits in question. Problem solved.

Solution

Unless there's a really good reason not to, I think changesets should use full commit IDs everywhere. Since it's a popular project with lots of users, it was only a matter of time before a collision issue like this occurred.

After code spelunking for a few hours, I wasn't able to identify precisely the right part of the code to modify, so I just removed all the instances of short: true being passed into the various git functions. Please let me know if a more nuanced change is necessary.

[changeset-bot]

🦋 Changeset detected

Latest commit: a907e78

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@changesets/apply-release-plan	Minor
@changesets/cli	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a907e78:

Sandbox	Source
Vanilla	Configuration

[Juice10]

Great find @camertron! We've been having exactly the same issue on https://github.com/rrweb-io/rrweb and (thanks to CodeSandbox) I've tested this PR and confirmed it fixes the issue.

[Andarist]

I think that the long hashes won't be as pleasant as the shorter ones in the CHANGELOGs so perhaps we could truncate them here, here, here, here and here

[camertron]

@Andarist ok makes sense 👍 Should I make those changes in this PR?

[Juice10]

@Andarist ok makes sense 👍 Should I make those changes in this PR?

I think that makes sense. Could you do it @camertron ?

[Andarist]

Yeah, it would be cool to do it as part of this PR.

[camertron]

Ok, done!

[Juice10]

Ok, done!

Awesome, thanks @camertron!

[Andarist]

Thanks for making the requested changes. I'll try to land this soon. It would be great to land #1085 first but I'm not sure if I will have time to finish this PR right now so I might decide just to validate this PR locally.