Skip to content

v1.3.0: wall-clock timeout for the engine (default 20 min, tunable) #8

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

Merged
ZhenghuaBao merged 2 commits into from
Jul 22, 2026
Merged

v1.3.0: wall-clock timeout for the engine (default 20 min, tunable) #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d79f4f
Add wall-clock timeout for a single engine pass
ZhenghuaBao Jul 22, 2026
9ee16f8
Document timeout-minutes input in README + example workflow
ZhenghuaBao Jul 22, 2026
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ All optional — pass as `with:` inputs on the action:
| `report` | `true` | Send a per-run summary (severity counts only — never code) to the OrcaRouter control plane; set `"false"` to disable — see [Run reporting](#run-reporting) |
| `github-token` | `${{ github.token }}` | Token used to fetch the PR head, post review comments, and manage the tier label; override only if the default `GITHUB_TOKEN` lacks the needed scopes |
| `engine-version` | `1.3.13` | Pinned `@alibaba-group/open-code-review` version (the review engine); bump deliberately after testing — later steps parse its JSON output shape |
| `timeout-minutes` | `20` | Wall-clock ceiling (in minutes) for one engine review pass. If the engine hasn't produced a result within this window it is killed and the check fails closed with a distinct `wall-clock timeout` error (separate from the "no usable result" mode, so the log tells you which one tripped). Accepts decimals (e.g. `"0.5"` = 30s) for testing. Bump for very large diffs or slow-per-call models where per-file review takes longer. In `exhaustive` mode each engine pass has its own budget, so the worst-case whole-review wall time is `timeout-minutes × 3`. |

`fix-first` and `block-on` can also be set per-repo from the OrcaRouter
dashboard — see the precedence rule under
Expand Down
45 changes: 39 additions & 6 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ inputs:
description: "Pinned @alibaba-group/open-code-review version (the review engine). Bump deliberately after testing — the later steps parse its JSON output shape."
required: false
default: "1.3.13"
timeout-minutes:
description: >-
Wall-clock ceiling (in minutes) for ONE engine review pass. If the engine
hasn't produced a result within this window it is killed and the run
fails closed with a distinct "wall-clock timeout" error (separate from
"no usable result", so the log makes clear which mode failed). Accepts
decimals (e.g. "0.5" = 30 seconds) for testing. Default 20 covers most
PRs on shipped models; bump for very large diffs or slow-per-call
models where per-file review takes longer.
required: false
default: "20"

@github-actions github-actions Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ P3 Ensure consistency in spelling and naming conventions for YAML keys, like timeout-minutes, to maintain readability and maintainability.


runs:
using: "composite"
Expand All @@ -149,7 +160,8 @@ runs:
"$RUNNER_TEMP/cr-settings.json" "$RUNNER_TEMP/cr-rubric.md" \
"$RUNNER_TEMP/cr-facts.json" "$RUNNER_TEMP/proxy.out" "$RUNNER_TEMP/proxy.err" \
"$RUNNER_TEMP/policy-block.json" \
"$RUNNER_TEMP/pr.diff" "$RUNNER_TEMP/diff-guard.json" "$RUNNER_TEMP/prev-summary.md"
"$RUNNER_TEMP/pr.diff" "$RUNNER_TEMP/diff-guard.json" "$RUNNER_TEMP/prev-summary.md" \
"$RUNNER_TEMP/wallclock-timeout"

- name: Resolve PR refs
id: pr
Expand Down Expand Up @@ -580,6 +592,10 @@ runs:
env:
TIER_STATE: ${{ steps.pr.outputs.tier }}
ROUTER: ${{ inputs.router }}
# Wall-clock ceiling for ONE engine pass; wrapped around `ocr review`
# via GNU `timeout` inside run_pass. Passed as-is (units are minutes;
# decimals accepted, e.g. "0.5" = 30s). See the input docstring.
TIMEOUT_MIN: ${{ inputs.timeout-minutes }}
# Effective value after the settings/input precedence rule.
FIX_FIRST: ${{ steps.settings.outputs.fix_first }}
# Non-empty only when the dashboard supplies a replacement rubric.
Expand Down Expand Up @@ -732,10 +748,22 @@ runs:
set_facts "$3" "$4"
echo "::group::Review — $2"
rc=0
ocr review --from "$BASE" --to "$HEAD" \
--background "$(cat "$BACKGROUND")" --format json \
> "$1" 2> "$REVIEW_LOG" || rc=$?
# Wall-clock guard: SIGTERM at TIMEOUT_MIN; SIGKILL 10s later if the
# engine ignored the term. Redirections apply to whatever `timeout`
# runs — i.e. ocr's own stdout/stderr — so the JSON result and log
# end up in the same files as before.
timeout --kill-after=10s "${TIMEOUT_MIN}m" \
ocr review --from "$BASE" --to "$HEAD" \
--background "$(cat "$BACKGROUND")" --format json \
> "$1" 2> "$REVIEW_LOG" || rc=$?

@github-actions github-actions Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 P1 The code updates introduce a timeout command to control execution duration, but there may still be a potential race condition between the termination signal and the timeout. This could lead to unpredictable states if other processes or instructions are not correctly managing concurrent operations. Review external/environment interactions to ensure proper sequence and completion.

cat "$REVIEW_LOG"
# 124 = GNU timeout tripped (SIGTERM); 137 = 128+9, i.e. --kill-after
# escalated to SIGKILL. Leave a marker so run_review can print a
# wall-clock-specific error (distinct from "no usable result").
if [ "$rc" = "124" ] || [ "$rc" = "137" ]; then
echo "::warning::Engine wall-clock timeout after ${TIMEOUT_MIN} minute(s) (rc=$rc)."
printf 'wall-clock timeout after %sm\n' "$TIMEOUT_MIN" > "$RUNNER_TEMP/wallclock-timeout"
Comment on lines +763 to +765

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Fail closed when exhaustive passes time out

When exhaustive is enabled, a timeout from pass 2 or 3 writes the wall-clock marker here, but the caller at the extra-pass branch treats any non-policy run_pass failure as best-effort and only warns before continuing with the earlier result. That means a run can hit the new wall-clock ceiling and still publish/pass using an incomplete exhaustive review, despite the new input documenting that timeouts fail closed and that each exhaustive pass has its own budget; timeout exits 124 on expiry per timeout --help, so this is exactly the path those passes take.

Useful? React with 👍 / 👎.

fi
echo "::endgroup::"
node "$CHECK" "$1" "$rc"
}
Expand All @@ -754,7 +782,11 @@ runs:
# $4 = extra passes allowed (true only on the strong tier)
run_review() {
if ! run_pass "$RESULT" "$1" "$2" "$3"; then
echo "::error::$BRAND: review engine produced no usable result — failing closed."
if [ -f "$RUNNER_TEMP/wallclock-timeout" ]; then
echo "::error::$BRAND: $(cat "$RUNNER_TEMP/wallclock-timeout") — engine killed, failing closed. Bump the 'timeout-minutes' input for very large PRs or slow-per-call models."
else
echo "::error::$BRAND: review engine produced no usable result — failing closed."
fi
exit 1
fi
PASSES=1
Expand Down Expand Up @@ -1271,4 +1303,5 @@ runs:
"$RUNNER_TEMP/cr-settings.json" "$RUNNER_TEMP/cr-rubric.md" \
"$RUNNER_TEMP/cr-facts.json" "$RUNNER_TEMP/proxy.out" "$RUNNER_TEMP/proxy.err" \
"$RUNNER_TEMP/policy-block.json" \
"$RUNNER_TEMP/pr.diff" "$RUNNER_TEMP/diff-guard.json" "$RUNNER_TEMP/prev-summary.md"
"$RUNNER_TEMP/pr.diff" "$RUNNER_TEMP/diff-guard.json" "$RUNNER_TEMP/prev-summary.md" \
"$RUNNER_TEMP/wallclock-timeout"
1 change: 1 addition & 0 deletions workflows/orca-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,4 @@ jobs:
# on-oversized-diff: "fail" # oversized skip fails the check; "pass" makes it advisory
# settings: "true" # "false" skips the dashboard fetch — this file is authoritative
# report: "true" # per-run severity counts (never code) to your control plane
# timeout-minutes: "20" # wall-clock ceiling per engine pass; bump for very large PRs
Loading