-
Notifications
You must be signed in to change notification settings - Fork 6
Check result of branch-deploy on every subsequent step #112
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
base: main
Are you sure you want to change the base?
Conversation
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 refactors the smoketest workflow by consolidating it from a two-job structure into a single job that checks the branch-deploy output on each subsequent step. This approach eliminates the need for job dependencies and output passing between jobs.
Key Changes:
- Merged
permission-checkandrun-testsjobs into a singleLinuxjob - Added conditional checks on each step to verify
branch-deploycontinue output - Removed job-level
needsandoutputsconfiguration
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/smoketest.yaml
Outdated
| python -m pip install hatch | ||
| - name: Run tests | ||
| - if steps.branch-deploy.outputs.continue == "true" |
Copilot
AI
Dec 5, 2025
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 if conditional syntax is incorrect. In GitHub Actions, the if keyword should be followed by a colon : and the condition expression should reference the step output using ${{ }} syntax.
The correct syntax should be:
- if: steps.branch-deploy.outputs.continue == 'true'
name: Run testsNote the colon after if and the proper expression syntax with ${{ }}.
| - if: steps.branch-deploy.outputs.continue == "true" | ||
| name: Setup Python venv | ||
| run: | | ||
| python -m venv .venv | ||
| source .venv/bin/activate | ||
| python -m pip install hatch | ||
| - name: Run tests | ||
| - if: steps.branch-deploy.outputs.continue == "true" |
Check failure
Code scanning / CodeQL
Untrusted Checkout TOCTOU Critical
issue_comment
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
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - if: steps.branch-deploy.outputs.continue == "true" | ||
| name: Run tests |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The if conditional should be placed after the name field. Consider:
- name: Run tests
if: steps.branch-deploy.outputs.continue == 'true'
env:
AI_API_TOKEN: ${{ secrets.AI_API_TOKEN }}
GITHUB_AUTH_HEADER: "Bearer ${{ secrets.GITHUB_TOKEN }}"
run: |
...See below for a potential fix:
- name: Setup Python
if: steps.branch-deploy.outputs.continue == "true"
uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Checkout the repo
if: steps.branch-deploy.outputs.continue == "true"
uses: actions/checkout@v6
with:
ref: ${{ steps.branch-deploy.outputs.sha }}
- name: Setup Python venv
if: steps.branch-deploy.outputs.continue == "true"
run: |
python -m venv .venv
source .venv/bin/activate
python -m pip install hatch
- name: Run tests
if: steps.branch-deploy.outputs.continue == "true"
| - if: steps.branch-deploy.outputs.continue == "true" | ||
| name: Setup Python |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The if conditional should be placed after the name field in the step definition, not before it. While GitHub Actions is sometimes forgiving about field order, the standard convention is:
- name: Setup Python
if: steps.branch-deploy.outputs.continue == 'true'
uses: actions/setup-python@v5
with:
python-version: '3.11'This ensures better readability and follows GitHub Actions best practices.
See below for a potential fix:
- name: Setup Python
if: steps.branch-deploy.outputs.continue == "true"
uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Checkout the repo
if: steps.branch-deploy.outputs.continue == "true"
uses: actions/checkout@v6
with:
ref: ${{ steps.branch-deploy.outputs.sha }}
- name: Setup Python venv
if: steps.branch-deploy.outputs.continue == "true"
run: |
python -m venv .venv
source .venv/bin/activate
python -m pip install hatch
- name: Run tests
if: steps.branch-deploy.outputs.continue == "true"
| - if: steps.branch-deploy.outputs.continue == "true" | ||
| name: Checkout the repo |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The if conditional should be placed after the name field. Consider:
- name: Checkout the repo
if: steps.branch-deploy.outputs.continue == 'true'
uses: actions/checkout@v6
with:
ref: ${{ steps.branch-deploy.outputs.sha }}See below for a potential fix:
- name: Setup Python
if: steps.branch-deploy.outputs.continue == "true"
uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Checkout the repo
if: steps.branch-deploy.outputs.continue == "true"
uses: actions/checkout@v6
with:
ref: ${{ steps.branch-deploy.outputs.sha }}
- name: Setup Python venv
if: steps.branch-deploy.outputs.continue == "true"
run: |
python -m venv .venv
source .venv/bin/activate
python -m pip install hatch
- name: Run tests
if: steps.branch-deploy.outputs.continue == "true"
| - if: steps.branch-deploy.outputs.continue == "true" | ||
| name: Setup Python venv |
Copilot
AI
Dec 5, 2025
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.
[nitpick] The if conditional should be placed after the name field. Consider:
- name: Setup Python venv
if: steps.branch-deploy.outputs.continue == 'true'
run: |
python -m venv .venv
source .venv/bin/activate
python -m pip install hatch| - if: steps.branch-deploy.outputs.continue == "true" | |
| name: Setup Python venv | |
| - name: Setup Python venv | |
| if: steps.branch-deploy.outputs.continue == "true" |
Unfortunately, this seems to be the only solution. See #111 for my previous attempts.