-
-
Notifications
You must be signed in to change notification settings - Fork 826
fix: this improves the way we do opena api diffing #11208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this 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 improves the OpenAPI diffing workflow by comparing against the merge-base commit where the feature branch diverged from the base branch (typically main), rather than comparing against a specific version or the tip of main. The workflow now builds and runs the Unleash backend directly instead of using docker-compose.
Key changes:
- Introduces merge-base detection to find the exact commit where the branch diverged
- Replaces docker-compose-based setup with direct Node.js backend execution using GitHub Actions postgres service
- Updates from port 3000 to port 4242 for the backend service
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runs-on: ubuntu-latest | ||
| services: | ||
| postgres: | ||
| image: postgres |
Copilot
AI
Jan 9, 2026
•
edited by gastonfournier
Loading
edited by gastonfournier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The postgres service image should specify a version tag instead of using the implicit "latest" tag. Using "postgres" without a version tag can lead to unexpected behavior when the latest version changes. Consider pinning to a specific version like "postgres:15" or "postgres:16" to ensure consistent and reproducible builds.
| image: postgres | |
| image: postgres:16 |
| run: git checkout "${BASE_SHA}" | ||
| - name: Install node | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: 22.x | ||
| - name: Install dependencies | ||
| run: | | ||
| yarn install --immutable |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking out a commit SHA directly (line 48) puts the repository in a detached HEAD state. After this checkout, Node.js dependencies are installed (line 55), but the package.json and yarn.lock from the baseline commit might be incompatible with the later current branch state. This could cause the workflow to fail if there are dependency changes between the baseline and current branch.
| # end fake frontend build | ||
| # start unleash in background | ||
| NODE_ENV=openapi yarn dev:backend & |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow will fail if the backend doesn't start successfully. After starting the backend with "&", there's no check to ensure the process actually started. If the backend fails to start, the wait loop will timeout, but there's no cleanup or logging of the backend process output which would help debug failures.
| git fetch origin "${{ github.event.pull_request.base.ref }}" | ||
| BASE_SHA=$(git merge-base "origin/${{ github.event.pull_request.base.ref }}" "${{ github.sha }}") | ||
| else | ||
| git fetch origin main | ||
| BASE_SHA=$(git rev-parse "origin/main") | ||
| fi | ||
| echo "BASE_SHA=$BASE_SHA" >> $GITHUB_ENV |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Determine baseline commit" step will fail when triggered by workflow_dispatch event if the base branch doesn't exist on origin or if the merge-base cannot be determined. The script should handle potential git command failures and validate that BASE_SHA is not empty before proceeding.
| git fetch origin "${{ github.event.pull_request.base.ref }}" | |
| BASE_SHA=$(git merge-base "origin/${{ github.event.pull_request.base.ref }}" "${{ github.sha }}") | |
| else | |
| git fetch origin main | |
| BASE_SHA=$(git rev-parse "origin/main") | |
| fi | |
| echo "BASE_SHA=$BASE_SHA" >> $GITHUB_ENV | |
| if ! git fetch origin "${{ github.event.pull_request.base.ref }}"; then | |
| echo "Failed to fetch base branch 'origin/${{ github.event.pull_request.base.ref }}'." | |
| exit 1 | |
| fi | |
| BASE_SHA=$(git merge-base "origin/${{ github.event.pull_request.base.ref }}" "${{ github.sha }}") || { | |
| echo "Failed to determine merge-base with 'origin/${{ github.event.pull_request.base.ref }}'." | |
| exit 1 | |
| } | |
| else | |
| if ! git fetch origin main; then | |
| echo "Failed to fetch base branch 'origin/main'." | |
| exit 1 | |
| fi | |
| BASE_SHA=$(git rev-parse "origin/main") || { | |
| echo "Failed to resolve 'origin/main'." | |
| exit 1 | |
| } | |
| fi | |
| if [ -z "$BASE_SHA" ]; then | |
| echo "BASE_SHA is empty; unable to determine baseline commit." | |
| exit 1 | |
| fi | |
| echo "BASE_SHA=$BASE_SHA" >> "$GITHUB_ENV" |
| run: | | ||
| docker compose -f .github/docker-compose.test.yml up -d --wait -t 90 | ||
| # fake frontend build | ||
| mkdir frontend/build |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing the -p flag for mkdir. If the frontend/build directory already exists from a previous run or caching, the mkdir command will fail. Use "mkdir -p frontend/build" to ensure idempotent behavior.
| mkdir frontend/build | |
| mkdir -p frontend/build |
| - name: Determine baseline commit | ||
| run: | | ||
| if [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| git fetch origin "${{ github.event.pull_request.base.ref }}" | ||
| BASE_SHA=$(git merge-base "origin/${{ github.event.pull_request.base.ref }}" "${{ github.sha }}") | ||
| else | ||
| git fetch origin main | ||
| BASE_SHA=$(git rev-parse "origin/main") | ||
| fi | ||
| echo "BASE_SHA=$BASE_SHA" >> $GITHUB_ENV |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow still declares a "stable_version" input (line 14-17) but this input is no longer used anywhere in the workflow after the changes. The old docker-compose-based approach used UNLEASH_VERSION variable which referenced this input, but the new approach checks out the baseline commit directly. This unused input should be removed to avoid confusion.
This reverts commit 84b1277.
About the changes
This now checks out the commit from where your branch "branched" from main and makes the diff.