-
Notifications
You must be signed in to change notification settings - Fork 0
fix(opencode-review): bound verdict-polling loop by wall clock #1707
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -423,7 +423,25 @@ jobs: | |
| review_poll_failures=0 | ||
| max_poll_transport_failures=3 | ||
| poll_interval_seconds=60 | ||
| # Wall-clock backstop, distinct from max_poll_transport_failures above: | ||
| # that counter only bounds *consecutive transport failures*, so a | ||
| # review dispatch that never produces a verdict -- while every | ||
| # individual `gh api` call keeps succeeding -- previously polled | ||
| # forever, holding a live runner for up to GitHub's 360-minute | ||
| # platform default job timeout. 10800s (3h) is chosen to stay | ||
| # comfortably above this org's own documented "accommodate over 2 | ||
| # hours per model" allowance (docs/product-goal-directive.md §8) | ||
| # while still releasing the runner well before the platform | ||
| # default. This bounds how long the CI job waits for a verdict; it | ||
| # does not cap the model's own reasoning/streaming time, which | ||
| # remains governed entirely upstream by the dispatched review run | ||
| # itself. | ||
| poll_deadline_epoch=$(( $(date -u +%s) + 10800 )) | ||
| while :; do | ||
| if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then | ||
| echo "::error::No current-head OpenCode verdict after 180 minutes of polling; failing closed and releasing the runner." | ||
| exit 1 | ||
|
Comment on lines
+439
to
+443
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Long reviews lose required approval When a verdict needs over three hours, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+441
to
+443
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Final-minute verdicts are discarded When a verdict arrives during the final sleep, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+439
to
+443
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| fi | ||
| if ! live_poll_pr="$(timeout 30s gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")"; then | ||
| live_poll_failures=$((live_poll_failures + 1)) | ||
| if [ "$live_poll_failures" -ge "$max_poll_transport_failures" ]; then | ||
|
|
||
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.
📝 Info: Epoch arithmetic is mechanically sound
On the pinned Ubuntu runner,
date -u +%sand Bash arithmetic support this range. A date failure also stops the step closed.Was this helpful? React with 👍 or 👎 to provide feedback.