Skip to content

fix(review): preserve full-hour sessions in .github/workflows/opencode-review.yml #566

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

Open
seonghobae wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(review): preserve full-hour sessions in .github/workflows/opencode-review.yml #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
44ac19e
fix(review): preserve full-hour model sessions
seonghobae Jul 15, 2026
3ba7d35
fix(review): bind verified probe locations
seonghobae Jul 15, 2026
0517060
fix(review): require current-head R CMD evidence
seonghobae Jul 15, 2026
df40893
fix(review): recover timed-out model sessions
seonghobae Jul 15, 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
174 changes: 152 additions & 22 deletions .github/workflows/opencode-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,13 @@ jobs:
)
runs-on: ubuntu-latest
permissions:
actions: read
contents: read
id-token: write
outputs:
r_package_peer_result: ${{ steps.r_package_peer.outputs.result }}
r_package_peer_conclusion: ${{ steps.r_package_peer.outputs.conclusion }}
r_package_peer_url: ${{ steps.r_package_peer.outputs.run_url }}
env:
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
steps:
Expand Down Expand Up @@ -272,6 +277,99 @@ jobs:
git -C "$COVERAGE_SOURCE_WORKDIR" status --short
tar -cf "$COVERAGE_SOURCE_ARCHIVE" -C "$COVERAGE_SOURCE_WORKDIR" .

- name: Await current-head R CMD check package evidence
id: r_package_peer
env:
GH_TOKEN: ${{ steps.coverage_read_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }}
TARGET_REPOSITORY: ${{ needs.validate-pr-metadata.outputs.target_repository }}
PR_BASE_SHA: ${{ needs.validate-pr-metadata.outputs.base_sha }}
PR_HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }}
COVERAGE_SOURCE_WORKDIR: ${{ runner.temp }}/opencode-coverage-source
run: |
set -euo pipefail

write_result() {
local result="$1"
local conclusion="${2:-}"
local run_url="${3:-}"
{
printf 'result=%s\n' "$result"
printf 'conclusion=%s\n' "$conclusion"
printf 'run_url=%s\n' "$run_url"
} >>"$GITHUB_OUTPUT"
}

if [ ! -f "${COVERAGE_SOURCE_WORKDIR}/DESCRIPTION" ]; then
echo "No root R package DESCRIPTION exists; peer R CMD check evidence is not applicable."
write_result "not_applicable"
exit 0
fi

changed_paths="$(git -C "$COVERAGE_SOURCE_WORKDIR" diff --name-only "$PR_BASE_SHA" "$PR_HEAD_SHA")"
if ! rg -q '(^DESCRIPTION$|^NAMESPACE$|^R/.*[.]R$|^tests/testthat/.*[.]R$)' <<<"$changed_paths"; then
echo "The current head does not change the root R package or its testthat suite; peer R CMD check evidence is not applicable."
write_result "not_applicable"
exit 0
fi

mapfile -t workflow_paths < <(
rg -l \
--glob '*.yml' \
--glob '*.yaml' \
'r-lib/actions/check-r-package@' \
"${COVERAGE_SOURCE_WORKDIR}/.github/workflows" 2>/dev/null \
| while IFS= read -r path; do
printf '%s\n' "${path#${COVERAGE_SOURCE_WORKDIR}/}"
done
)
if [ "${#workflow_paths[@]}" -eq 0 ]; then
echo "::error::Root R package changes require a repository R CMD check workflow using the pinned r-lib check action."
write_result "missing"
exit 0
fi

workflow_paths_json="$(printf '%s\n' "${workflow_paths[@]}" | jq -Rsc 'split("\n") | map(select(length > 0))')"
deadline=$((SECONDS + 5400))
while [ "$SECONDS" -lt "$deadline" ]; do
runs_json="$(
gh api --method GET "repos/${TARGET_REPOSITORY}/actions/runs" \
-f "head_sha=${PR_HEAD_SHA}" \
-f event=pull_request \
-F per_page=100
)"
current_run="$(
jq -c \
--arg head_sha "$PR_HEAD_SHA" \
--argjson workflow_paths "$workflow_paths_json" \
'[.workflow_runs[]
| select(.head_sha == $head_sha)
| select(.path as $path | $workflow_paths | index($path))]
| sort_by(.run_number, .run_attempt)
| last // empty' <<<"$runs_json"
)"
if [ -n "$current_run" ]; then
status="$(jq -r '.status // empty' <<<"$current_run")"
conclusion="$(jq -r '.conclusion // empty' <<<"$current_run")"
run_url="$(jq -r '.html_url // empty' <<<"$current_run")"
run_id="$(jq -r '.id // empty' <<<"$current_run")"
printf 'Current-head R CMD check run %s is %s/%s for %s.\n' "$run_id" "${status:-unknown}" "${conclusion:-pending}" "$PR_HEAD_SHA"
if [ "$status" = "completed" ]; then
if [ "$conclusion" = "success" ]; then
write_result "success" "$conclusion" "$run_url"
else
write_result "failure" "${conclusion:-unknown}" "$run_url"
fi
exit 0
fi
else
printf 'Waiting for a current-head R CMD check run for %s.\n' "$PR_HEAD_SHA"
fi
sleep 30
done

echo "::error::Current-head R CMD check did not complete within 90 minutes. The package check may still be running; its result was not discarded or mislabeled as a code failure."
write_result "timed_out"

- name: Upload materialized pull request merge tree
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
Expand Down Expand Up @@ -385,6 +483,9 @@ jobs:
PR_BASE_SHA: ${{ needs.validate-pr-metadata.outputs.base_sha }}
PR_HEAD_SHA: ${{ needs.validate-pr-metadata.outputs.head_sha }}
COVERAGE_SOURCE_WORKDIR: ${{ github.workspace }}/pr-head
R_PACKAGE_PEER_RESULT: ${{ needs.coverage-source-tree.outputs.r_package_peer_result }}
R_PACKAGE_PEER_CONCLUSION: ${{ needs.coverage-source-tree.outputs.r_package_peer_conclusion }}
R_PACKAGE_PEER_URL: ${{ needs.coverage-source-tree.outputs.r_package_peer_url }}
# Dependency resolution may consume wheels/packages, but PR-defined
# install/build hooks are never executed implicitly.
UV_NO_BUILD: "1"
Expand Down Expand Up @@ -996,8 +1097,34 @@ jobs:
Rscript -e 'required <- c("covr", "testthat"); missing <- required[!vapply(required, requireNamespace, logical(1), quietly = TRUE)]; if (length(missing)) stop("signed distribution coverage packages unavailable: ", paste(missing, collapse = ", "))'
if [ -f DESCRIPTION ]; then
if [ -d tests/testthat ]; then
run_and_capture "R package testthat suite" \
Rscript -e 'lib <- Sys.getenv("R_LIBS_USER"); .libPaths(c(lib, .libPaths())); if (!requireNamespace("testthat", quietly = TRUE)) { message("testthat unavailable in coverage runner; deferring to required peer R CMD check evidence."); quit(status = 0) }; testthat::test_dir("tests/testthat")'
case "${R_PACKAGE_PEER_RESULT:-not_applicable}" in
success)
append "### R package testthat suite"
append ""
append "- Result: PASS"
append "- Reason: the repository's current-head R CMD check completed successfully and provides package-aware installation plus testthat evidence."
if [ -n "${R_PACKAGE_PEER_URL:-}" ]; then
append "- Peer run: ${R_PACKAGE_PEER_URL}"
fi
append ""
;;
missing|failure|timed_out)
append "### R package testthat suite"
append ""
append "- Result: FAIL"
append "- Reason: current-head package-aware R CMD check evidence is ${R_PACKAGE_PEER_RESULT} (conclusion: ${R_PACKAGE_PEER_CONCLUSION:-unavailable})."
if [ -n "${R_PACKAGE_PEER_URL:-}" ]; then
append "- Peer run: ${R_PACKAGE_PEER_URL}"
fi
append "- Fix: keep the pinned repository R CMD check workflow enabled and fix its exact installation or test failure before review approval."
append ""
failures=$((failures + 1))
;;
*)
run_and_capture "R package testthat suite" \
Rscript -e 'lib <- Sys.getenv("R_LIBS_USER"); .libPaths(c(lib, .libPaths())); if (!requireNamespace("testthat", quietly = TRUE)) { message("testthat unavailable in coverage runner; no package-aware peer evidence exists."); quit(status = 1) }; testthat::test_dir("tests/testthat")'
;;
esac
else
append "### R package testthat suite"
append ""
Expand Down Expand Up @@ -3368,7 +3495,7 @@ jobs:
- name: Run OpenCode PR Review model pool
id: opencode_review_model_pool
if: needs.coverage-evidence.result == 'success'
timeout-minutes: 65
timeout-minutes: 205
continue-on-error: true
env:
STRIX_GITHUB_MODELS_TOKEN: ${{ secrets.STRIX_GITHUB_MODELS_TOKEN || github.token }}
Expand All @@ -3395,13 +3522,16 @@ jobs:
# the SAME model 5x let a rate-limited/hung leader consume the whole
# step, so the pool never reached a healthy fallback model.
OPENCODE_MODEL_ATTEMPTS: "1"
# Keep stale providers from pinning required review jobs for hours.
# Adversarial validation needs enough room to read the evidence, but
# dynamic cadence and the outer watchdog still bound each current-head run.
OPENCODE_RUN_TIMEOUT_SECONDS: "600"
OPENCODE_EXPORT_TIMEOUT_SECONDS: "120"
OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "3600"
OPENCODE_POOL_STEP_TIMEOUT_SECONDS: "2100"
# Preserve reviews that legitimately need tens of minutes to inspect a
# large repository. Changed-file count is not a reliable proxy for
# repository complexity, so every cadence class gets 90 minutes per
# model and a full-hour review is never discarded at the boundary.
# The 195-minute retry budget leaves room for a second substantial
# candidate while the outer 200-minute watchdog remains bounded.
OPENCODE_RUN_TIMEOUT_SECONDS: "5400"
OPENCODE_EXPORT_TIMEOUT_SECONDS: "180"
OPENCODE_TOTAL_RETRY_BUDGET_SECONDS: "11700"
OPENCODE_POOL_STEP_TIMEOUT_SECONDS: "12000"
# Keep cycling through the high-sensitivity candidate catalog until
# the retry budget or step timeout is exhausted; a single invalid
# cycle can be all provider formatting noise rather than review
Expand All @@ -3410,16 +3540,16 @@ jobs:
OPENCODE_DYNAMIC_REVIEW_CADENCE: "true"
OPENCODE_SMALL_CHANGE_FILE_THRESHOLD: "3"
OPENCODE_MEDIUM_CHANGE_FILE_THRESHOLD: "20"
OPENCODE_SMALL_CHANGE_RUN_TIMEOUT_SECONDS: "600"
OPENCODE_SMALL_CHANGE_TOTAL_BUDGET_SECONDS: "3600"
OPENCODE_MEDIUM_CHANGE_RUN_TIMEOUT_SECONDS: "600"
OPENCODE_MEDIUM_CHANGE_TOTAL_BUDGET_SECONDS: "3600"
OPENCODE_LARGE_CHANGE_RUN_TIMEOUT_SECONDS: "600"
OPENCODE_LARGE_CHANGE_TOTAL_BUDGET_SECONDS: "3600"
OPENCODE_UNKNOWN_CHANGE_RUN_TIMEOUT_SECONDS: "600"
OPENCODE_UNKNOWN_CHANGE_TOTAL_BUDGET_SECONDS: "3600"
OPENCODE_DYNAMIC_RUN_TIMEOUT_CAP_SECONDS: "600"
OPENCODE_DYNAMIC_TOTAL_BUDGET_CAP_SECONDS: "1800"
OPENCODE_SMALL_CHANGE_RUN_TIMEOUT_SECONDS: "5400"
OPENCODE_SMALL_CHANGE_TOTAL_BUDGET_SECONDS: "11700"
OPENCODE_MEDIUM_CHANGE_RUN_TIMEOUT_SECONDS: "5400"
OPENCODE_MEDIUM_CHANGE_TOTAL_BUDGET_SECONDS: "11700"
OPENCODE_LARGE_CHANGE_RUN_TIMEOUT_SECONDS: "5400"
OPENCODE_LARGE_CHANGE_TOTAL_BUDGET_SECONDS: "11700"
OPENCODE_UNKNOWN_CHANGE_RUN_TIMEOUT_SECONDS: "5400"
OPENCODE_UNKNOWN_CHANGE_TOTAL_BUDGET_SECONDS: "11700"
OPENCODE_DYNAMIC_RUN_TIMEOUT_CAP_SECONDS: "5400"
OPENCODE_DYNAMIC_TOTAL_BUDGET_CAP_SECONDS: "11700"
OPENCODE_DYNAMIC_MAX_CYCLES_CAP: "0"
# This installation currently reports a 4k request-body limit for
# GitHub Models GPT-5 endpoints even though the public catalog is
Expand All @@ -3429,8 +3559,8 @@ jobs:
OPENCODE_DYNAMIC_MAX_CYCLES: "0"
CENTRAL_REVIEW_PROCESS_FALLBACK_ELIGIBLE: ${{ steps.central_review_process_fallback_scope.outputs.eligible || 'false' }}
CENTRAL_REVIEW_PROCESS_FALLBACK_SCOPE_LABEL: ${{ steps.central_review_process_fallback_scope.outputs.scope_label || 'unsupported' }}
OPENCODE_CENTRAL_REVIEW_PROCESS_FALLBACK_RUN_TIMEOUT_SECONDS: "600"
OPENCODE_CENTRAL_REVIEW_PROCESS_FALLBACK_TOTAL_BUDGET_SECONDS: "3600"
OPENCODE_CENTRAL_REVIEW_PROCESS_FALLBACK_RUN_TIMEOUT_SECONDS: "5400"
OPENCODE_CENTRAL_REVIEW_PROCESS_FALLBACK_TOTAL_BUDGET_SECONDS: "11700"
OPENCODE_CENTRAL_REVIEW_PROCESS_FALLBACK_MAX_CYCLES: "1"
OPENCODE_BACKOFF_INITIAL_SECONDS: "30"
OPENCODE_BACKOFF_MAX_SECONDS: "30"
Expand Down
70 changes: 70 additions & 0 deletions scripts/ci/opencode_review_normalize_output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,75 @@ def adversarial_probe_source_receipt_error(
return ""


def repair_adversarial_probe_evidence_bindings(value: Any) -> dict[str, Any] | Any:
"""Bind a verified structured probe location into its evidence text.

Models can put the exact changed-file path and positive line in the
structured ``path``/``line`` fields while omitting the redundant
``path:line`` citation from ``evidence``. Repair only that duplication when
the single receipt already matches the sealed current-head source line.
Every other validation failure remains untouched and therefore fails closed.
"""
if not isinstance(value, dict):
return value
validation = value.get("adversarial_validation")
if not isinstance(validation, dict):
return value
probes = validation.get("probes")
if not isinstance(probes, list):
return value

changed_files = current_changed_files()
repaired_probes: list[Any] = []
changed = False
for probe in probes:
if not isinstance(probe, dict):
repaired_probes.append(probe)
continue
path = probe.get("path")
line = probe.get("line")
evidence = probe.get("evidence")
if (
not isinstance(path, str)
or path not in changed_files
or isinstance(line, bool)
or not isinstance(line, int)
or line <= 0
or not isinstance(evidence, str)
or not evidence.strip()
or adversarial_probe_location_error(path, line)
):
repaired_probes.append(probe)
continue

receipts = SOURCE_LINE_RECEIPT_RE.findall(evidence)
if len(receipts) != 1 or adversarial_probe_source_receipt_error(
evidence, path, line
):
repaired_probes.append(probe)
continue
rejection = adversarial_evidence_rejection_reason(evidence, path, line)
if rejection != "must cite the exact probe path and positive line":
repaired_probes.append(probe)
continue

repaired_evidence = f"{path}:{line} {evidence}"
if adversarial_probe_source_receipt_error(
repaired_evidence, path, line
) or adversarial_evidence_rejection_reason(repaired_evidence, path, line):
repaired_probes.append(probe)
continue
repaired_probes.append({**probe, "evidence": repaired_evidence})
changed = True

if not changed:
return value
return {
**value,
"adversarial_validation": {**validation, "probes": repaired_probes},
}


def adversarial_validation_error(
value: Any,
*,
Expand Down Expand Up @@ -1267,6 +1336,7 @@ def reject(reason: str) -> None:
return reject("APPROVE cannot contain findings")
if result == "REQUEST_CHANGES" and not findings:
return reject("REQUEST_CHANGES requires at least one finding")
value = repair_adversarial_probe_evidence_bindings(value)
adversarial_error = adversarial_validation_error(
value.get("adversarial_validation"),
result=result,
Expand Down
Loading
Loading