[fix](ci) Add error handling and PR notification for OpenCode review failures

[zclllyybb]

before it may silently fail. now try it best to return the status if failed.

[Thearas]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[zclllyybb]

/review

[zclllyybb]

run buildall

[github-actions]

Code Review Summary

This PR adds error handling and PR notification for OpenCode review failures in the CI workflow. The intent is sound — previously failures were silent, and now the PR gets notified. However, there is one functional bug and one cosmetic issue.

Critical Checkpoints

• Goal: Add error handling to the OpenCode review CI step so failures are surfaced as PR comments. The intent is achieved but has a bug that undermines it in the most important scenario (opencode failure).
• Modification scope: Small, focused, single-file CI change. Appropriate scope.
• Concurrency: N/A — no concurrency concerns in a CI workflow.
• Lifecycle / static init: N/A.
• Configuration items: N/A.
• Incompatible changes: N/A.
• Parallel code paths: N/A.
• Special conditional checks: The failure_reason detection logic has an edge case; see inline comment.
• Test coverage: CI workflows are not unit-tested; manual testing is expected.
• Observability: The PR comment on failure is the observability mechanism here. It will be partially broken due to the bug below.
• Performance: N/A.

Issues Found

1. Bug (High): set -e (GitHub Actions default shell) causes immediate script exit when the opencode | tee pipeline fails, so PIPESTATUS and failure_reason logic never executes. The PR comment will have an empty error message in the most common failure case. See inline comment for details and suggested fix.

2. Minor (Cosmetic): The heredoc in the "Comment PR on review failure" step uses leading spaces that will appear in the PR comment body, making it look oddly indented.

[zclllyybb]

/review

[github-actions]

Code Review Summary

This PR adds error handling and PR notification for OpenCode review failures in the CI workflow, and adds a new "Review Holistically" principle plus an example to the "Reuse First" principle in the code-review skill.

Critical Checkpoint Conclusions

• Goal: Add resilient error handling so that when the OpenCode review process fails, the failure is captured, reported to the PR, and the workflow correctly fails. The SKILL.md changes add review guidance. The goal is largely accomplished, with issues noted below.
• Modification size/focus: The change is small and focused on two related concerns (CI resilience + review skill docs). Appropriate scope.
• Concurrency: Not applicable - this is a sequential CI workflow.
• Lifecycle management: Not applicable.
• Configuration items: No new config items.
• Incompatible changes: No incompatible changes.
• Parallel code paths: No other workflows need this same change.
• Special conditional checks: The rg pattern and the conditional logic have a correctness issue (see inline comment) where a successful run (exit 0) with an incidental Error: in the log would be falsely treated as a failure.
• Test coverage: CI workflow changes are not typically unit-tested; manual testing of failure scenarios is recommended.
• Observability: The failure reason is posted to the PR and also echoed in the workflow logs - good.
• Transaction/persistence: Not applicable.
• Data writes: Not applicable.
• FE-BE variables: Not applicable.
• Performance: Not applicable for CI workflow.
• Other issues: (1) The heredoc in the PR comment step preserves leading whitespace, resulting in poorly formatted comments. (2) The rg pattern can cause false positives when exit status is 0 but log contains matching text.

Issues Found

1. [Bug] False-positive failure detection: When opencode exits with status 0 (success), but the log happens to contain a line matching ^Error: or SSE read timed out (e.g., from the review content itself discussing errors), the failure_reason variable will be non-empty, causing the step to exit 1 and falsely report failure.
2. [Cosmetic] Heredoc indentation: The cat <<EOF heredoc preserves leading whitespace, so the PR comment will have each line indented with extra spaces.

[zclllyybb]

/review

[github-actions]

Code Review Summary

PR: fix Add error handling and PR notification for OpenCode review failures

This PR adds error handling to the OpenCode automated review workflow and a new "Review Holistically" principle to the code-review skill. The changes are small, focused, and accomplish their stated goals.

Critical Checkpoint Conclusions

• Goal & correctness: The goal is to capture OpenCode review failures (exit status and known error patterns) and notify the PR with a comment. The implementation accomplishes this through continue-on-error, PIPESTATUS capture, log pattern matching, and a follow-up notification step. The logic flow is sound.
• Modification scope: The change is minimal and focused — one file for the workflow error handling, one file for the skill documentation. Appropriate scope.
• Concurrency: Not applicable. This is a CI workflow, single-threaded shell script execution.
• Lifecycle management: Not applicable. No static initialization or complex lifecycle concerns.
• Configuration items: No new configuration items added.
• Incompatible changes: No incompatible changes. The workflow behavior is strictly additive — it now handles a failure path that was previously silent.
• Parallel code paths: No parallel code paths affected.
• Special conditional checks: The rg regex pattern ^Error:|SSE read timed out correctly matches lines starting with Error: OR containing SSE read timed out (case-insensitive). This is appropriate for known OpenCode failure patterns.
• Test coverage: This is CI infrastructure code; testing is done by the workflow itself in real failure scenarios. No additional automated tests are expected.
• Observability: The PR improves observability by posting failure information to the PR comment and including a workflow run URL for further investigation.
• Transaction/persistence: Not applicable.
• FE-BE variables: Not applicable.
• Performance: Not applicable for CI workflow code.
• Other issues:
  • The GITHUB_OUTPUT heredoc uses EOF as a delimiter; if failure_reason ever contained a bare EOF line it could corrupt the output. Extremely unlikely given the expected content, but a more unique delimiter would be marginally safer.
  • The use of rg (ripgrep) assumes it is pre-installed on ubuntu-latest, which is the case for GitHub-hosted runners, so this is fine.
  • The YAML heredoc indentation is correctly handled by YAML block scalar stripping.
  • The SKILL.md changes are purely additive documentation improvements (expanding "Reuse First" with examples and adding "Review Holistically" principle). No issues.

Verdict: No blocking issues found. The implementation is clean and accomplishes its goal.

[zclllyybb]

run buildall

[github-actions]

PR approved by at least one committer and no changes requested.

[github-actions]

PR approved by anyone and no changes requested.