Action size: Add a PR check that comments on significant repo size changes#3910
Action size: Add a PR check that comments on significant repo size changes#3910henrymercer wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a PR workflow and TypeScript helper to measure compressed repository archive size changes between a PR and its base, then post/update a sticky PR comment when the delta is significant.
Changes:
- Added a
Check repo sizeworkflow. - Added
pr-checks/check-repo-size.tswith archive measurement, formatting, and PR comment upsert logic. - Added unit tests and pr-check dependency updates for Sinon-based Octokit mocking.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/check-repo-size.yml |
Runs the repo size check on PR events. |
pr-checks/check-repo-size.ts |
Implements archive size measurement and sticky PR comment behavior. |
pr-checks/check-repo-size.test.ts |
Tests formatting, archive measurement, and comment upsert behavior. |
pr-checks/package.json |
Adds Sinon test dependencies for pr-checks. |
package-lock.json |
Updates lockfile metadata for pr-checks dependencies. |
Copilot's findings
- Files reviewed: 4/5 changed files
- Comments generated: 5
mbg
left a comment
There was a problem hiding this comment.
Thanks for looking at this! Two high-level design questions:
- Could we add this on to an existing workflow (pr-checks?) rather than adding a totally new one?
- Rather than comparing to the PR base ref, would it make sense to compare to the latest CodeQL Action release instead?
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
Could we add this on to an existing workflow (pr-checks?) rather than adding a totally new one?
| */ | ||
| export const SIGNIFICANT_DELTA_FRACTION = 0.1; | ||
|
|
||
| export type Octokit = ReturnType<typeof getOctokit>; |
There was a problem hiding this comment.
api-client.ts exports ApiClient (the return type of getOctokit). Why this roundabout way of getting hold of that type?
With that approach, if we merge a PR that significantly increases / decreases the repo size, this would create noise on all subsequent PRs until the next release. Since we compare against the base, we'll get the comparison against the latest release when we run the release process, so we have another opportunity to see significant changes at that point. |
7e6ce16 to
b34c6b4
Compare
b34c6b4 to
9b6438e
Compare
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
.github/workflows/pr-checks.yml:225
- Because
bodycomes from an artifact produced by PR-controlled code, passing it withgh api --fieldis unsafe:--field body=@pathmakes the CLI read a local file, so a malicious body could cause this privileged job to post files such as its environment. Use a raw field or file input only after constructing/validating the body in trusted code.
gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$comment_id" --field body="$body"
elif [[ "$significant" == "true" ]]; then
echo "Creating new repo size comment."
gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" --field body="$body"
- Files reviewed: 5/5 changed files
- Comments generated: 1
The compressed checkout of the repo is downloaded at the start of every CodeQL job. Significant jumps or drops are worth surfacing since they directly affect job startup time.
To that end, this PR adds a "Check repo size" PR check that streams
git archive --format=tar.gzfor both the PR base andHEAD, compares the compressed sizes, and posts a sticky comment when the difference is at least 10% in either direction.