From 8d79f4f906b3e2a55a038142c933d9d8673809d7 Mon Sep 17 00:00:00 2001 From: ZhenghuaBao Date: Wed, 22 Jul 2026 14:41:05 +0800 Subject: [PATCH 1/2] Add wall-clock timeout for a single engine pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New input timeout-minutes (default 20). Wraps the `ocr review` call in run_pass with GNU `timeout --kill-after=10s ${N}m`: SIGTERM at N minutes, SIGKILL 10s later if the engine ignored the term. This caps how long a single review pass can run — previously there was no wall-clock guard, so a large-PR + slow-model combo could burn 40+ minutes before failing closed on subtask timeouts (observed at 43min on a 71-file PR). On timeout (rc 124 or 137), run_pass drops a marker in $RUNNER_TEMP; run_review reads it and prints a wall-clock-specific error, distinct from the existing 'no usable result' error so log readers can tell which mode failed and whether to bump timeout-minutes or investigate the engine. Accepts decimals so tests can force a timeout quickly (e.g. "0.5" = 30s). Co-Authored-By: Claude Opus 4 --- action.yml | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/action.yml b/action.yml index dbc5fa7..e784dd2 100644 --- a/action.yml +++ b/action.yml @@ -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" runs: using: "composite" @@ -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 @@ -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. @@ -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=$? 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" + fi echo "::endgroup::" node "$CHECK" "$1" "$rc" } @@ -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 @@ -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" From 9ee16f8955812d547265ad269e9e19dd74678eae Mon Sep 17 00:00:00 2001 From: ZhenghuaBao Date: Wed, 22 Jul 2026 15:46:36 +0800 Subject: [PATCH 2/2] Document timeout-minutes input in README + example workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README inputs table gets a row explaining the wall-clock guard, the distinct fail-close error, decimals for testing, and the exhaustive-mode caveat (per-pass budget → worst case timeout-minutes × 3). Example workflow gets a commented `# timeout-minutes: "20"` line so consumers see it as a tunable when they copy the file. --- README.md | 1 + workflows/orca-code-review.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/README.md b/README.md index d16750b..9699ca1 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/workflows/orca-code-review.yml b/workflows/orca-code-review.yml index 40c3302..0a0f0ae 100644 --- a/workflows/orca-code-review.yml +++ b/workflows/orca-code-review.yml @@ -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