Prevent review failures from failing the calling workflow#52
Prevent review failures from failing the calling workflow#52derekmisler merged 1 commit intodocker:mainfrom
Conversation
|
/describe |
|
✅ PR description has been generated and updated! |
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
✅ APPROVED - No issues found.
This PR appropriately adds continue-on-error: true to both PR review steps in the workflow. The change is straightforward, safe, and accomplishes its stated goal of preventing review failures from blocking the calling workflow. The implementation is consistent across both the member and non-member review paths.
derekmisler
force-pushed
the
never-fail-the-workflow
branch
from
a5c8b42 to
a1230a7
Compare
derekmisler
force-pushed
the
never-fail-the-workflow
branch
from
a1230a7 to
a432aa3
Compare
There was a problem hiding this comment.
Review Summary
✅ This PR looks good! The changes correctly add continue-on-error: true to prevent review failures from blocking the workflow.
Minor Observation
The jq command refactoring (replacing --arg with direct variable substitution) technically deviates from jq best practices, but poses minimal practical risk since PRE_REVIEW_TS is a controlled timestamp variable. The code will work correctly under normal conditions.
Signed-off-by: Derek Misler <derek.misler@docker.com>
derekmisler
force-pushed
the
never-fail-the-workflow
branch
from
a432aa3 to
3cbf6c7
Compare
There was a problem hiding this comment.
Review Summary
✅ This PR successfully simplifies the review workflow by removing non-functional verification logic and adding continue-on-error to prevent review failures from blocking calling workflows.
Analysis
I reviewed the changes and found that the removed code (review-posted output, verify-review step, timestamp recording) was either non-functional or redundant:
- The
review-postedoutput referenced a non-existentverify-reviewstep output - The timestamp recording was never actually used
- The review verification logic relied on steps that don't exist in the current architecture
The PR correctly delegates review posting verification to the cagent agent itself, which handles it internally. The continue-on-error: true addition is appropriate for ensuring workflow resilience.
Note: One minor breaking change: The review-posted output was removed from the action's public interface. While this output was non-functional, any external workflows referencing it will need to be updated to use the exit-code output instead.
There was a problem hiding this comment.
Review Summary
This PR introduces changes that improve workflow resilience but also creates potential issues around observability and backward compatibility.
Critical Concern: The removal of the review-posted output is a breaking API change that could impact existing workflows consuming this action.
Notable Concern: The combination of continue-on-error: true and removal of verification logic means workflows will succeed even when reviews fail to post, reducing observability.
|
/describe |
|
✅ PR description has been generated and updated! |
There was a problem hiding this comment.
Pull request overview
This PR modifies the PR review workflow to make it more resilient by preventing review failures from blocking calling workflows. It adds continue-on-error: true to review steps and removes complex verification logic that attempted to detect token expiry issues.
Changes:
- Added
continue-on-error: trueto PR review steps in both auto-review and manual-review jobs - Removed review verification logic that tracked timestamps and verified review posting
- Simplified post-review reactions to only distinguish between failure (confused) and success (thumbs up)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
.github/workflows/review-pr.yml |
Added continue-on-error: true to both review step invocations to prevent workflow failures |
review-pr/action.yml |
Removed review-posted output, pre-review timestamp tracking, review verification step, and simplified reaction logic; changed token usage in post-review steps from github.token to resolved token |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Adds
continue-on-error: trueto the PR review steps in the workflow to prevent review failures from blocking the calling workflow. Also removes complex review verification logic that was attempting to detect token expiry issues.Changes
.github/workflows/review-pr.yml: Addedcontinue-on-error: trueto both PR review steps (member and slash-command paths)review-pr/action.yml: Removed 77 lines of review verification logic including timestamp tracking, review posting verification, and conditional reaction handling based on review statereview-pr/action.yml: Simplified post-summary step to show "Review completed" instead of "Review posted successfully" and removed token expiry fallback commentsreview-pr/action.yml: Removedreview-postedoutput that was tracking verification statusBreaking Changes
review-postedoutput is no longer available from thereview-practionHow to Test