github-actions[bot] commented on PR #61530:
URL: https://github.com/apache/doris/pull/61530#issuecomment-4091106670
## OpenCode Automated Code Review Summary
**PR:** [chore](ci) Add timeout and improve error handling for OpenCode
review workflow
**Scope:** `.github/workflows/opencode-review.yml` only (CI workflow, +9/-4
lines)
### Changes Analyzed
1. **Step-level timeout**: Adds `timeout-minutes: 55` to the review step,
correctly below the job-level 60-minute timeout, ensuring the review step
itself can be terminated if it hangs.
2. **Broader failure detection**: Changes the condition on failure-handling
steps from `steps.review.outcome == 'failure'` to `always() &&
steps.review.outcome != 'success'`, which correctly catches `cancelled`
(timeout with `continue-on-error`) and `skipped` outcomes in addition to
`failure`.
3. **Improved error messages**: Adds `REVIEW_OUTCOME` env var and provides a
meaningful fallback message when `FAILURE_REASON` is empty (as happens on
timeout since `GITHUB_OUTPUT` isn't written).
### Critical Checkpoint Conclusions
- **Goal and correctness**: The goal is to catch early errors (especially
timeouts) that were previously silently ignored. The code accomplishes this
correctly. The `always()` + `!= 'success'` pattern is the standard GitHub
Actions idiom for this.
- **Modification size and focus**: The change is minimal, clear, and focused
on a single concern.
- **Concurrency**: Not applicable (CI workflow YAML, no concurrent access
patterns).
- **Lifecycle management**: Not applicable.
- **Configuration items**: No new configs added.
- **Incompatible changes**: No incompatible changes; this is purely additive
error handling.
- **Parallel code paths**: The two failure-handling steps ("Comment PR on
review failure" and "Fail workflow if review failed") are updated consistently
with the same condition and error message logic.
- **Special conditional checks**: The `always() && steps.review.outcome !=
'success'` condition is well-understood and correctly handles all non-success
outcomes (failure, cancelled, skipped).
- **Test coverage**: Not applicable for CI workflow changes; the change is
best validated by triggering the workflow.
- **Observability**: The improved error messages enhance observability for
timeout and cancellation scenarios.
- **Performance**: Not applicable.
- **Other issues**: None identified. Minor note: if the review step is
`skipped` (due to a prior step failing), the fallback message says "possibly
timeout or cancelled" which is slightly imprecise, but this is acceptable since
the outcome value itself is included in the message and provides the accurate
information.
### Verdict
**No issues found.** The change is correct, focused, and improves the
robustness of the CI workflow by properly handling timeout and cancellation
scenarios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
