Skip to content

CI: Fail build when compile leaves uncommitted changes#35123

Merged
JReinhold merged 2 commits into
nextfrom
ci/check-git-clean-after-build
Jun 10, 2026
Merged

CI: Fail build when compile leaves uncommitted changes#35123
JReinhold merged 2 commits into
nextfrom
ci/check-git-clean-after-build

Conversation

@JReinhold

@JReinhold JReinhold commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Closes #

What I did

The CircleCI build job already had a git.check() step, but it ran before compile — so it never caught generated file changes that compile produces (e.g. updated package.json exports).

This PR:

  • Moves the git cleanliness check to after compile (and Verdaccio publish) in the Linux build job
  • Adds the same check to the Windows build job
  • Improves the check to use git status --porcelain (catches untracked files too) and prints a helpful message with the diff on failure

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

No manual testing needed — this is a CI configuration change. CircleCI will verify the check works on this PR itself.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Made with Cursor

Summary by CodeRabbit

  • Chores
    • Improved build workflow diagnostics for repository state validation, with enhanced error reporting during the CI pipeline.

Move the git cleanliness check to after compile so generated files must be committed before merge.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JReinhold JReinhold added build Internal-facing build tooling & test updates ci:normal Run our default set of CI jobs (choose this for most PRs). labels Jun 10, 2026
@JReinhold JReinhold marked this pull request as ready for review June 10, 2026 09:08
@JReinhold JReinhold requested review from a team June 10, 2026 09:08
@JReinhold JReinhold self-assigned this Jun 10, 2026
@JReinhold JReinhold added the qa:skip Pull Requests that do not need any QA. label Jun 10, 2026
@JReinhold JReinhold enabled auto-merge June 10, 2026 09:11
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d1f20f3-1a3d-4fc6-afc4-b94e2bc28d06

📥 Commits

Reviewing files that changed from the base of the PR and between 01b3bc0 and 1e41430.

📒 Files selected for processing (2)
  • scripts/ci/common-jobs.ts
  • scripts/ci/utils/helpers.ts

📝 Walkthrough

Walkthrough

This PR enhances git working-tree validation and repositions the git.check() step in Linux and Windows CI workflows. The git.check() helper implementation now performs explicit cleanliness checks with diagnostic output, and execution is moved to later stages after compilation and publishing steps.

Changes

Git state validation repositioning and enhancement

Layer / File(s) Summary
Enhanced git.check() implementation
scripts/ci/utils/helpers.ts
git.check() now checks git status --porcelain explicitly, prints diagnostic output (git status and git diff) when uncommitted changes are detected, and fails with exit code 1.
Repositioned git.check() calls in build workflows
scripts/ci/common-jobs.ts
In build_linux, git.check() is removed from early position before npm.check() and added after Verdaccio publish. In build_windows, git.check() is newly inserted after the compile step.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands and usage tips.

@Sidnioulz Sidnioulz moved this to In Progress in Core Team Projects Jun 10, 2026
@JReinhold JReinhold merged commit 514a19b into next Jun 10, 2026
136 of 142 checks passed
@JReinhold JReinhold deleted the ci/check-git-clean-after-build branch June 10, 2026 09:37
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Core Team Projects Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants