Skip to content

ci(pr-coments): handle partial successes in pr-comments workflow #15294

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.

Sign up for GitHub

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

Closed
slonka wants to merge 3 commits into from
Closed

ci(pr-coments): handle partial successes in pr-comments workflow #15294

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4a0c5d
fix(ci): handle partial successes in pr-comments workflow
slonka Dec 16, 2025
f72c032
ci(pr-comments): address comments
slonka Dec 16, 2025
e85e819
ci(pr-comments): address comments 2
slonka Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 87 additions & 4 deletions .github/workflows/pr-comments.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,31 +50,114 @@ jobs:
# Automatically update code formatting if /format is in the comment
- name: "Auto-format code"
if: contains(github.event.comment.body, '/format')
id: format
continue-on-error: true
run: make clean/generated check

- name: "Commit and push format changes"
Comment thread
slonka marked this conversation as resolved.
Outdated
if: contains(github.event.comment.body, '/format') && steps.format.outcome == 'success'
continue-on-error: true
env:
GITHUB_TOKEN: ${{ steps.github-app-token.outputs.token }}
run: |
if git diff --exit-code --stat; then
echo "No format changes detected"
else
git config user.name "${GH_USER}"
git config user.email "${GH_EMAIL}"
git commit -s -m "fix(ci): format files" .
git push
fi

# Update all golden files except transparent proxy tests if /golden_files is in the comment
- name: "Update golden files (excluding transparent proxy)"
if: contains(github.event.comment.body, '/golden_files')
id: golden-files
continue-on-error: true
run: make test UPDATE_GOLDEN_FILES=true

- name: "Commit and push golden files changes"
if: contains(github.event.comment.body, '/golden_files') && steps.golden-files.outcome == 'success'
continue-on-error: true
env:
GITHUB_TOKEN: ${{ steps.github-app-token.outputs.token }}
run: |
if git diff --exit-code --stat; then
echo "No golden files changes detected"
else
git config user.name "${GH_USER}"
git config user.email "${GH_EMAIL}"
git commit -s -m "fix(ci): update golden files" .
git push
fi

# Update only transparent proxy golden files if /golden_files_tproxy is in the comment
- name: "Update transparent proxy golden files"
if: contains(github.event.comment.body, '/golden_files_tproxy')
id: golden-files-tproxy
continue-on-error: true
run: make test/transparentproxy UPDATE_GOLDEN_FILES=true

- name: commit and push fixes
- name: "Commit and push transparent proxy golden files changes"
if: contains(github.event.comment.body, '/golden_files_tproxy') && steps.golden-files-tproxy.outcome == 'success'
continue-on-error: true
env:
GITHUB_TOKEN: ${{ steps.github-app-token.outputs.token }}
run: |
if git diff --exit-code --stat; then
echo "No change detected, skipping git push"
echo "No transparent proxy golden files changes detected"
else
git config user.name "${GH_USER}"
git config user.email "${GH_EMAIL}"
git commit -s -m "fix(ci): format files" .
git commit -s -m "fix(ci): update transparent proxy golden files" .
git push
fi
- run: gh api --method POST -f content='hooray' ${{ github.event.comment.url }}/reactions

- name: "Post summary comment"
if: always()
env:
GITHUB_TOKEN: ${{ steps.github-app-token.outputs.token }}
run: |
SUMMARY="## Workflow Results\n\n"

if [ "${{ steps.format.outcome }}" != "" ]; then
if [ "${{ steps.format.outcome }}" == "success" ]; then
SUMMARY="${SUMMARY}✅ **Format**: Succeeded and pushed\n"
elif [ "${{ steps.format.outcome }}" == "failure" ]; then
SUMMARY="${SUMMARY}❌ **Format**: Failed\n"
fi
fi

if [ "${{ steps.golden-files.outcome }}" != "" ]; then
if [ "${{ steps.golden-files.outcome }}" == "success" ]; then
SUMMARY="${SUMMARY}✅ **Golden files**: Succeeded and pushed\n"
elif [ "${{ steps.golden-files.outcome }}" == "failure" ]; then
SUMMARY="${SUMMARY}❌ **Golden files**: Failed\n"
fi
fi

if [ "${{ steps.golden-files-tproxy.outcome }}" != "" ]; then
if [ "${{ steps.golden-files-tproxy.outcome }}" == "success" ]; then
SUMMARY="${SUMMARY}✅ **Transparent proxy golden files**: Succeeded and pushed\n"
elif [ "${{ steps.golden-files-tproxy.outcome }}" == "failure" ]; then
SUMMARY="${SUMMARY}❌ **Transparent proxy golden files**: Failed\n"
Comment on lines +134 to +165
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The summary message incorrectly states "Succeeded and pushed" for all successful outcomes, but the commit-and-push steps have continue-on-error: true, meaning they could fail silently after the main step succeeds. If the git push fails (e.g., due to permissions or conflicts), the summary will still show "Succeeded and pushed" even though nothing was actually pushed.

Consider checking the outcome of the commit-and-push steps as well, or remove continue-on-error from those steps so failures are properly reported. Alternatively, update the message to say "Succeeded" without implying the push completed successfully.

Copilot uses AI. Check for mistakes.
fi
fi
Comment on lines +133 to +167
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition checking for empty outcome (!= "") doesn't distinguish between a step that was skipped versus one that ran. When a step is skipped (because its if condition is false), the outcome will be an empty string, but this check will treat it the same as if it never existed. This means if someone comments /format only, the summary will incorrectly show results for golden files steps that were never triggered.

Consider checking if the step was actually triggered by verifying the corresponding comment body contains the command, or use the conclusion field which differentiates between skipped and not-run steps.

Copilot uses AI. Check for mistakes.

gh pr comment ${{ github.event.issue.number }} --repo ${{ github.repository }} --body "$SUMMARY"

- name: "Add success reaction"
Comment thread
slonka marked this conversation as resolved.
if: |
always() &&
steps.format.outcome != 'failure' &&
steps.golden-files.outcome != 'failure' &&
steps.golden-files-tproxy.outcome != 'failure'
Comment on lines +174 to +176
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition uses string inequality checks that could be simplified and made more explicit. Currently, checking steps.format.outcome != 'failure' for each step means that skipped steps (with empty outcomes) are treated as successes.

Consider using explicit positive checks like (steps.format.outcome == 'success' || steps.golden-files.outcome == 'success' || steps.golden-files-tproxy.outcome == 'success') to only add the success reaction when at least one operation actually succeeded.

Suggested change
steps.format.outcome != 'failure' &&
steps.golden-files.outcome != 'failure' &&
steps.golden-files-tproxy.outcome != 'failure'
(
steps.format.outcome == 'success' ||
steps.golden-files.outcome == 'success' ||
steps.golden-files-tproxy.outcome == 'success'
)

Copilot uses AI. Check for mistakes.
run: gh api --method POST -f content='hooray' ${{ github.event.comment.url }}/reactions
env:
GITHUB_TOKEN: ${{ steps.github-app-token.outputs.token }}

- name: "Add failure reaction"
if: always() && (steps.format.outcome == 'failure' || steps.golden-files.outcome == 'failure' || steps.golden-files-tproxy.outcome == 'failure')
run: gh api --method POST -f content='-1' ${{ github.event.comment.url }}/reactions
env:
GITHUB_TOKEN: ${{ steps.github-app-token.outputs.token }}
Loading