diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 2a207bc50..3c055710e 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -476,7 +476,7 @@ jobs: "- mergeStateStatus: `" + $state + "`", "- mergeable: `" + ((.mergeable // "unknown") | tostring) + "`", if ($state == "DIRTY" or $state == "CONFLICTING") then - "- Review direction: PR has merge conflicts. OpenCode must explain how to merge or rebase the latest base branch into the PR branch, resolve conflict markers, rerun focused checks, and push the same branch." + "- Review direction: PR has merge conflicts. OpenCode must explain how to merge or rebase the latest base branch into the PR branch, resolve conflict markers, rerun focused checks, and push the same branch, including a compact command block with gh pr checkout, git fetch, merge or rebase, git status --short, and the normal or --force-with-lease push path." elif $state == "BLOCKED" then "- Review direction: `BLOCKED` is a branch policy, review, or check state, not merge conflict evidence. Do not request conflict repair unless mergeStateStatus is `DIRTY` or `CONFLICTING`." else @@ -787,7 +787,9 @@ jobs: Only mergeStateStatus DIRTY or CONFLICTING means a merge conflict. mergeStateStatus BLOCKED is a branch policy, review, or check state, not conflict guidance. When the PR mergeability evidence reports mergeStateStatus DIRTY or CONFLICTING, include a merge-conflict repair direction that names the base/head branch relationship, instructs the author to merge or rebase the latest base branch into the PR branch, resolve conflict markers in changed files, rerun focused checks, - and push the same branch. + and push the same branch. Include a compact repair command block with gh pr checkout, git fetch, + merge or rebase, git status --short, the resolved-file step, the normal push path, and the + --force-with-lease path only for rebased branches. For Greptile-style specificity, include a P1/P2/P3 priority in each actionable finding, cite the evidence type behind the claim (nearby implementation, matching existing example, cross-file counterpart, current official docs, or failed check/log evidence), flag unrelated PR @@ -850,7 +852,9 @@ jobs: Only mergeStateStatus DIRTY or CONFLICTING means a merge conflict. mergeStateStatus BLOCKED is a branch policy, review, or check state, not conflict guidance. When the PR mergeability evidence reports mergeStateStatus DIRTY or CONFLICTING, include a merge-conflict repair direction that names the base/head branch relationship, instructs the author to merge or rebase the latest base branch into the PR branch, resolve conflict markers in changed files, rerun focused checks, - and push the same branch. + and push the same branch. Include a compact repair command block with gh pr checkout, git fetch, + merge or rebase, git status --short, the resolved-file step, the normal push path, and the + --force-with-lease path only for rebased branches. For Greptile-style specificity, include a P1/P2/P3 priority in each actionable finding, cite the evidence type behind the claim (nearby implementation, matching existing example, cross-file counterpart, current official docs, or failed check/log evidence), flag unrelated PR @@ -1779,7 +1783,7 @@ jobs: } append_merge_conflict_guidance() { - local pr_json merge_state base_ref head_ref + local pr_json merge_state base_ref head_ref base_fetch_ref base_origin_ref head_push_ref pr_json="$(gh pr view "$PR_NUMBER" --repo "$GH_REPOSITORY" --json baseRefName,headRefName,mergeStateStatus 2>/dev/null || true)" if [ -z "$pr_json" ]; then return 0 @@ -1790,11 +1794,26 @@ jobs: fi base_ref="$(printf '%s' "$pr_json" | jq -r '.baseRefName // "base"')" head_ref="$(printf '%s' "$pr_json" | jq -r '.headRefName // "head"')" + printf -v base_fetch_ref '%q' "$base_ref" + printf -v base_origin_ref '%q' "origin/${base_ref}" + printf -v head_push_ref '%q' "HEAD:${head_ref}" printf '\n## Merge Conflict Guidance\n\n' printf '%s\n' "- Current merge state: \`${merge_state}\`" printf '%s\n' "- Base branch: \`${base_ref}\`" printf '%s\n' "- Head branch: \`${head_ref}\`" printf '%s\n' "- Fix direction: merge or rebase \`origin/${base_ref}\` into \`${head_ref}\`, resolve conflict markers in the changed files, rerun the focused checks, then push the same branch." + printf '%s\n' "- Repair commands:" + printf '%s\n' '```bash' + printf 'gh pr checkout %s --repo %s\n' "$PR_NUMBER" "$GH_REPOSITORY" + printf 'git fetch origin %s\n' "$base_fetch_ref" + printf 'git merge --no-ff %s # or: git rebase %s\n' "$base_origin_ref" "$base_origin_ref" + printf 'git status --short\n' + printf '# resolve files, then git add \n' + printf '# merge path: git commit\n' + printf '# rebase path: git rebase --continue\n' + printf 'git push origin %s\n' "$head_push_ref" + printf '# rebase path only: git push --force-with-lease origin %s\n' "$head_push_ref" + printf '%s\n' '```' } perl -pe 's/\x1b\[[0-9;?]*[A-Za-z]//g' "$review_output_file" >"$clean_output" @@ -2028,7 +2047,7 @@ jobs: } append_merge_conflict_guidance() { - local pr_json merge_state base_ref head_ref + local pr_json merge_state base_ref head_ref base_fetch_ref base_origin_ref head_push_ref pr_json="$(gh pr view "$PR_NUMBER" --repo "$GH_REPOSITORY" --json baseRefName,headRefName,mergeStateStatus 2>/dev/null || true)" if [ -z "$pr_json" ]; then return 0 @@ -2039,11 +2058,26 @@ jobs: fi base_ref="$(printf '%s' "$pr_json" | jq -r '.baseRefName // "base"')" head_ref="$(printf '%s' "$pr_json" | jq -r '.headRefName // "head"')" + printf -v base_fetch_ref '%q' "$base_ref" + printf -v base_origin_ref '%q' "origin/${base_ref}" + printf -v head_push_ref '%q' "HEAD:${head_ref}" printf '\n## Merge Conflict Guidance\n\n' printf '%s\n' "- Current merge state: \`${merge_state}\`" printf '%s\n' "- Base branch: \`${base_ref}\`" printf '%s\n' "- Head branch: \`${head_ref}\`" printf '%s\n' "- Fix direction: merge or rebase \`origin/${base_ref}\` into \`${head_ref}\`, resolve conflict markers in the changed files, rerun the focused checks, then push the same branch." + printf '%s\n' "- Repair commands:" + printf '%s\n' '```bash' + printf 'gh pr checkout %s --repo %s\n' "$PR_NUMBER" "$GH_REPOSITORY" + printf 'git fetch origin %s\n' "$base_fetch_ref" + printf 'git merge --no-ff %s # or: git rebase %s\n' "$base_origin_ref" "$base_origin_ref" + printf 'git status --short\n' + printf '# resolve files, then git add \n' + printf '# merge path: git commit\n' + printf '# rebase path: git rebase --continue\n' + printf 'git push origin %s\n' "$head_push_ref" + printf '# rebase path only: git push --force-with-lease origin %s\n' "$head_push_ref" + printf '%s\n' '```' } update_review_overview() { @@ -2115,6 +2149,25 @@ jobs: update_review_overview "$event" "$body" } + stop_approval_without_review() { + local result="$1" + local body="$2" + + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + { + printf '## OpenCode review state unchanged\n\n' + printf -- "- Result: \`%s\`\n" "$result" + printf -- "- Head SHA: \`%s\`\n" "$HEAD_SHA" + printf -- '- Workflow run: %s\n' "$RUN_ID" + printf -- '- Workflow attempt: %s\n\n' "$RUN_ATTEMPT" + printf '%s\n' "$body" + } >>"$GITHUB_STEP_SUMMARY" + fi + printf '::error::%s: OpenCode did not change the pull request review state. %s\n' "$result" "$(printf '%s' "$body" | head -n 1)" + echo "::endgroup::" + exit 1 + } + collect_unresolved_human_review_threads() { local output_file="$1" local owner="${GH_REPOSITORY%%/*}" @@ -2447,9 +2500,9 @@ jobs: local helper_findings_file helper_findings_file="$(mktemp)" if "${repo_root%/}/scripts/ci/emit_opencode_failed_check_fallback_findings.sh" "$evidence_file" "$repo_root" >"$helper_findings_file"; then - if grep -Fq "No deterministic missing-string markers" "$helper_findings_file" || + if grep -Eiq 'deterministic[ -]?missing[- ]string markers|strix report locations|map each failed check' "$helper_findings_file" || ! grep -Eq '^### [0-9]+\. ' "$helper_findings_file"; then - printf 'OpenCode failed-check fallback helper returned non-source-backed generic output; leaving the PR review unchanged for rerun.\n' >&2 + printf 'OpenCode failed-check fallback helper returned non-source-backed output. No PR review was posted; retry after current-head failed-check logs or annotations are available, or rerun the failed check to collect them.\n' >&2 rm -f "$helper_findings_file" return 1 fi @@ -2458,7 +2511,7 @@ jobs: return 0 fi rm -f "$helper_findings_file" - printf 'OpenCode failed-check fallback helper did not produce source-backed findings; leaving the PR review unchanged for rerun.\n' >&2 + printf 'OpenCode failed-check fallback helper did not produce source-backed findings. No PR review was posted; retry after current-head failed-check logs or annotations are available, or rerun the failed check to collect them.\n' >&2 return 1 fi @@ -2627,7 +2680,7 @@ jobs: rm -f "$strix_evidence_file" if [ "$finding_index" -eq 0 ]; then - printf 'No automated source-backed fallback pattern matched this failed check; leaving the PR review unchanged for rerun.\n' >&2 + printf 'No automated source-backed fallback pattern matched this failed check. No PR review was posted; retry after current-head failed-check logs or annotations are available, or rerun the failed check to collect them.\n' >&2 return 1 fi } @@ -2668,6 +2721,23 @@ jobs: rm -f "$findings_file" } + stop_failed_check_fallback_unavailable() { + local body + + body="$(printf '%s\n' \ + "OpenCode could not derive source-backed line-specific findings after retries." \ + "" \ + "- Result: FAILED_CHECK_DIAGNOSIS_UNAVAILABLE" \ + "- Reason: current-head failed checks were present, but neither model diagnosis nor deterministic fallback mapped them to concrete source-backed findings." \ + "- Required next evidence: failed-check logs or annotations that identify an exact local file line and a concrete fix." \ + "- Head SHA: \`${HEAD_SHA}\`" \ + "- Workflow run: ${RUN_ID}" \ + "- Workflow attempt: ${RUN_ATTEMPT}" \ + "" \ + "No PR review was posted because an evidence-mapping failure is a review-tool state, not a source finding.")" + stop_approval_without_review "FAILED_CHECK_DIAGNOSIS_UNAVAILABLE" "$body" + } + is_github_billing_lock_evidence() { local evidence_file="$1" @@ -2825,13 +2895,13 @@ jobs: { printf '## Pull request overview\n\n' printf 'OpenCode reviewed the current-head bounded evidence but could not approve while peer GitHub Checks were still pending.\n\n' - printf '## Findings\n\n' - printf '### 1. HIGH .github/workflows/opencode-review.yml:1 - Peer GitHub Checks were still pending before approval\n' + printf '## Approval hold\n\n' + printf '### Peer GitHub Checks were still pending before approval\n' printf -- '- Problem: Current-head GitHub Checks did not all complete before the bounded approval wait ended.\n' printf -- '- Root cause: OpenCode cannot safely approve until security and build checks have finished for the same head SHA.\n' printf -- '- Fix: Re-run OpenCode after the pending checks finish, or wait for this approval step to observe completed peer checks.\n' - printf -- '- Regression test: Keep the approval gate waiting for peer checks and emitting REQUEST_CHANGES instead of approving stale evidence.\n\n' - printf -- '- Result: REQUEST_CHANGES\n' + printf -- '- Regression test: Keep the approval gate waiting for peer checks and stopping without approval instead of approving stale evidence.\n\n' + printf -- '- Result: WAITING_FOR_CHECKS\n' printf -- "- Reason: current-head GitHub Checks did not all complete before the bounded approval wait ended for \`%s\`.\n" "$HEAD_SHA" printf -- "- Head SHA: \`%s\`\n" "$HEAD_SHA" printf -- '- Workflow run: %s\n' "$RUN_ID" @@ -2883,7 +2953,7 @@ jobs: { printf 'GitHub Checks failed after the initial OpenCode review. Diagnose the failed checks and return a line-specific REQUEST_CHANGES review for PR #%s in %s.\n' "$PR_NUMBER" "$GITHUB_WORKSPACE" - printf 'Use the failed log excerpt and annotations below as evidence, follow the Review language evidence from bounded-review-evidence.md for the final review language, then inspect local source files and focused hunks to identify the exact line to edit. For each actionable Strix or GitHub Check failure, provide one finding with path,line,severity,title,problem,root_cause,fix_direction,regression_test_direction,suggested_diff. If PR mergeability evidence reports mergeStateStatus DIRTY, include merge-conflict repair direction that names base/head branches, tells the author to merge or rebase the latest base branch into the PR branch, resolve conflict markers, rerun focused checks, and push the same branch. Use Greptile-style specificity: preserve a P1/P2/P3 priority, cite the evidence type behind the claim (nearby implementation, matching existing example, cross-file counterpart, current official docs, or failed check/log evidence), flag unrelated PR scope drift, make suggested diffs GitHub suggestion-ready minimal diffs when possible, and include one compact Mermaid DAG that names the changed file or surface and maps it to the affected execution path, main risk, and verification path; do not use generic placeholder nodes like Changed surface or Main risk. The line must be a positive line number from an actual changed or relevant local file; never use line 0. Include the failed check label and exact failed log phrase in problem or root_cause; unrelated speculative findings are invalid. Prefer deletion, stdlib/native platform features, and already-installed dependencies before proposing new code or packages, but do not simplify away trust-boundary validation, data-loss handling, security, accessibility, or required tests. The fix_direction must state the concrete from/to change, not only the workflow URL. The suggested_diff must be source-backed and GitHub suggestion-ready when possible: every removed line in the diff must exist in the cited current local file, so do not request changes for code you did not verify in the current source. If Strix evidence contains multiple model vulnerability reports, include every model-reported vulnerability as a separate evidence-backed finding and preserve each report'\''s model name, title, severity, endpoint, and Code Locations/path:line evidence in problem or root_cause when present. When evidence supports it, name the concrete CWE/KISA-style class such as injection, auth/authz, secrets, crypto, path traversal/file upload, XSS/CSRF/SSRF, error disclosure, or debug/deployment config; do not invent a category without evidence. One Strix model vulnerability report requires one distinct finding; do not combine duplicate titles or matching locations from different models into one finding. If a failure is external infrastructure with no source fix, the finding must identify the exact external blocker, supporting log line, and why no repository line can fix it.\n\n' + printf 'Use the failed log excerpt and annotations below as evidence, follow the Review language evidence from bounded-review-evidence.md for the final review language, then inspect local source files and focused hunks to identify the exact line to edit. For each actionable Strix or GitHub Check failure, provide one finding with path,line,severity,title,problem,root_cause,fix_direction,regression_test_direction,suggested_diff. If PR mergeability evidence reports mergeStateStatus DIRTY, include merge-conflict repair direction that names base/head branches, tells the author to merge or rebase the latest base branch into the PR branch, resolve conflict markers, rerun focused checks, and push the same branch, including a compact command block with gh pr checkout, git fetch, merge or rebase, git status --short, and the normal or --force-with-lease push path. Use Greptile-style specificity: preserve a P1/P2/P3 priority, cite the evidence type behind the claim (nearby implementation, matching existing example, cross-file counterpart, current official docs, or failed check/log evidence), flag unrelated PR scope drift, make suggested diffs GitHub suggestion-ready minimal diffs when possible, and include one compact Mermaid DAG that names the changed file or surface and maps it to the affected execution path, main risk, and verification path; do not use generic placeholder nodes like Changed surface or Main risk. The line must be a positive line number from an actual changed or relevant local file; never use line 0. Include the failed check label and exact failed log phrase in problem or root_cause; unrelated speculative findings are invalid. Prefer deletion, stdlib/native platform features, and already-installed dependencies before proposing new code or packages, but do not simplify away trust-boundary validation, data-loss handling, security, accessibility, or required tests. The fix_direction must state the concrete from/to change, not only the workflow URL. The suggested_diff must be source-backed and GitHub suggestion-ready when possible: every removed line in the diff must exist in the cited current local file, so do not request changes for code you did not verify in the current source. If Strix evidence contains multiple model vulnerability reports, include every model-reported vulnerability as a separate evidence-backed finding and preserve each report'\''s model name, title, severity, endpoint, and Code Locations/path:line evidence in problem or root_cause when present. When evidence supports it, name the concrete CWE/KISA-style class such as injection, auth/authz, secrets, crypto, path traversal/file upload, XSS/CSRF/SSRF, error disclosure, or debug/deployment config; do not invent a category without evidence. One Strix model vulnerability report requires one distinct finding; do not combine duplicate titles or matching locations from different models into one finding. If a failure is external infrastructure with no source fix, the finding must identify the exact external blocker, supporting log line, and why no repository line can fix it.\n\n' printf 'Format the human-readable review with OpenCode-owned sections compatible with Copilot Review and CodeRabbitAI: start with a concise pull request overview, then list severity-ordered actionable findings without raw tool logs. Do not depend on those agents or a human reviewer being present.\n\n' printf 'Failed checks:\n' cat "$failed_checks_file" @@ -3338,6 +3408,18 @@ jobs: "- Problem: GitHub reports mergeStateStatus \`${merge_state}\` for this pull request." \ "- Root cause: Branch \`${head_ref}\` cannot be merged cleanly into \`${base_ref}\`; the changed-file flow below shows which review/runtime path is blocked by the conflict." \ "- Fix: Merge or rebase the latest \`${base_ref}\` into \`${head_ref}\`, resolve conflict markers in the PR branch, rerun the focused checks, and push the same branch." \ + "- Repair commands:" \ + '```bash' \ + "gh pr checkout ${PR_NUMBER} --repo ${GH_REPOSITORY}" \ + "git fetch origin ${base_ref}" \ + "git merge --no-ff origin/${base_ref} # or: git rebase origin/${base_ref}" \ + "git status --short" \ + "# resolve files, then git add " \ + "# merge path: git commit" \ + "# rebase path: git rebase --continue" \ + "git push origin HEAD:${head_ref}" \ + "# rebase path only: git push --force-with-lease origin HEAD:${head_ref}" \ + '```' \ "- Regression test: Keep OpenCode approval gated on mergeability so model-output failures cannot approve a conflicted PR." \ "" \ "## Change Flow DAG" \ @@ -3424,9 +3506,7 @@ jobs: elif build_failed_check_fallback_body "$failed_checks_file" "$failed_check_evidence_file" "$failed_check_review_body_file"; then create_pull_review "REQUEST_CHANGES" "$(cat "$failed_check_review_body_file")" else - echo "::error::Failed GitHub Checks were present, but OpenCode could not derive source-backed line-specific findings after retries. Leaving the PR review unchanged so the current-head review can be rerun with better evidence." - echo "::endgroup::" - exit 1 + stop_failed_check_fallback_unavailable fi echo "::endgroup::" exit 0 @@ -3435,9 +3515,18 @@ jobs: if request_changes_for_merge_conflict_if_present; then : else - echo "::error::OpenCode action outcomes were primary=${OPENCODE_PRIMARY_OUTCOME:-unknown}, fallback=${OPENCODE_FALLBACK_OUTCOME:-unknown}, second_fallback=${OPENCODE_SECOND_FALLBACK_OUTCOME:-unknown}, o_series_fallback=${OPENCODE_O_SERIES_FALLBACK_OUTCOME:-unknown}; all configured OpenCode model attempts failed to produce a usable current-head control block for head ${HEAD_SHA}. no valid source-backed review output was available after retries; it will not approve without source-backed current-head review evidence. Leaving the PR review unchanged because this is review tooling instability, not a source-code finding." - echo "::endgroup::" - exit 1 + body="$(printf '%s\n' \ + "all configured OpenCode model attempts failed to produce a usable current-head control block." \ + "" \ + "- Result: OPENCODE_REVIEW_UNAVAILABLE" \ + "- Reason: OpenCode action outcomes were primary=${OPENCODE_PRIMARY_OUTCOME:-unknown}, fallback=${OPENCODE_FALLBACK_OUTCOME:-unknown}, second_fallback=${OPENCODE_SECOND_FALLBACK_OUTCOME:-unknown}, o_series_fallback=${OPENCODE_O_SERIES_FALLBACK_OUTCOME:-unknown}." \ + "- Required next evidence: rerun OpenCode with a model/tooling attempt that emits a valid source-backed control block for this head." \ + "- Head SHA: \`${HEAD_SHA}\`" \ + "- Workflow run: ${RUN_ID}" \ + "- Workflow attempt: ${RUN_ATTEMPT}" \ + "" \ + "Leaving the PR review unchanged because this is review tooling instability, not a source-code finding.")" + stop_approval_without_review "OPENCODE_REVIEW_UNAVAILABLE" "$body" fi echo "::endgroup::" exit 0 @@ -3543,29 +3632,25 @@ jobs: "" \ "OpenCode reviewed the current-head evidence but could not verify peer GitHub Checks before approval." \ "" \ - "## Findings" \ + "## Approval hold" \ "" \ - "### 1. HIGH .github/workflows/opencode-review.yml:1 - GitHub Checks statusCheckRollup could not be read before approval" \ + "### GitHub Checks statusCheckRollup could not be read before approval" \ "- Problem: GitHub Checks statusCheckRollup could not be read for the current head." \ "- Root cause: OpenCode cannot safely approve without verifying the same-head check rollup." \ "- Fix: Re-run OpenCode after GitHub statusCheckRollup is readable." \ "- Regression test: Keep the approval gate failing closed when check rollup lookup fails." \ "" \ - "- Result: REQUEST_CHANGES" \ + "- Result: CHECKS_LOOKUP_FAILED" \ "- Reason: GitHub Checks statusCheckRollup could not be read for current head \`${HEAD_SHA}\`." \ "- Head SHA: \`${HEAD_SHA}\`" \ "- Workflow run: ${RUN_ID}" \ "- Workflow attempt: ${RUN_ATTEMPT}")" - create_pull_review "REQUEST_CHANGES" "$body" - echo "::endgroup::" - exit 0 + stop_approval_without_review "CHECKS_LOOKUP_FAILED" "$body" fi if [ "$pending_wait_status" -ne 0 ]; then failed_check_review_body_file="$(mktemp)" build_pending_check_body "$pending_checks_file" "$failed_check_review_body_file" - create_pull_review "REQUEST_CHANGES" "$(cat "$failed_check_review_body_file")" - echo "::endgroup::" - exit 0 + stop_approval_without_review "WAITING_FOR_CHECKS" "$(cat "$failed_check_review_body_file")" fi failed_checks_file="$(mktemp)" if ! collect_github_checks_with_retry collect_failed_github_checks "$failed_checks_file"; then @@ -3574,22 +3659,20 @@ jobs: "" \ "OpenCode reviewed the current-head evidence but could not verify peer GitHub Checks before approval." \ "" \ - "## Findings" \ + "## Approval hold" \ "" \ - "### 1. HIGH .github/workflows/opencode-review.yml:1 - GitHub Checks statusCheckRollup could not be read before approval" \ + "### GitHub Checks statusCheckRollup could not be read before approval" \ "- Problem: GitHub Checks statusCheckRollup could not be read for the current head." \ "- Root cause: OpenCode cannot safely approve without verifying the same-head check rollup." \ "- Fix: Re-run OpenCode after GitHub statusCheckRollup is readable." \ "- Regression test: Keep the approval gate failing closed when check rollup lookup fails." \ "" \ - "- Result: REQUEST_CHANGES" \ + "- Result: CHECKS_LOOKUP_FAILED" \ "- Reason: GitHub Checks statusCheckRollup could not be read for current head \`${HEAD_SHA}\`." \ "- Head SHA: \`${HEAD_SHA}\`" \ "- Workflow run: ${RUN_ID}" \ "- Workflow attempt: ${RUN_ATTEMPT}")" - create_pull_review "REQUEST_CHANGES" "$body" - echo "::endgroup::" - exit 0 + stop_approval_without_review "CHECKS_LOOKUP_FAILED" "$body" fi if [ -s "$failed_checks_file" ]; then failed_check_evidence_file="$(mktemp)" @@ -3616,9 +3699,7 @@ jobs: echo "::endgroup::" exit 0 else - echo "::error::Failed GitHub Checks were present, but OpenCode could not derive source-backed line-specific findings after retries. Leaving the PR review unchanged so the current-head review can be rerun with better evidence." - echo "::endgroup::" - exit 1 + stop_failed_check_fallback_unavailable fi fi unresolved_human_threads_file="$(mktemp)" @@ -3663,9 +3744,18 @@ jobs: failed_check_inline_failure_body_file="$(mktemp)" failed_checks_file="$(mktemp)" if ! collect_github_checks_with_retry collect_failed_github_checks "$failed_checks_file"; then - request_changes_for_gate_failure "GitHub Checks statusCheckRollup could not be read before validating OpenCode REQUEST_CHANGES against current-head failed checks." - echo "::endgroup::" - exit 0 + body="$(printf '%s\n' \ + "OpenCode could not validate REQUEST_CHANGES against current-head failed checks." \ + "" \ + "- Result: CHECKS_LOOKUP_FAILED" \ + "- Reason: GitHub Checks statusCheckRollup could not be read before validating OpenCode REQUEST_CHANGES." \ + "- Required next evidence: readable current-head statusCheckRollup plus failed-check logs or annotations." \ + "- Head SHA: \`${HEAD_SHA}\`" \ + "- Workflow run: ${RUN_ID}" \ + "- Workflow attempt: ${RUN_ATTEMPT}" \ + "" \ + "No PR review was posted because check lookup failure is a review-tool state, not a source finding.")" + stop_approval_without_review "CHECKS_LOOKUP_FAILED" "$body" fi if [ -s "$failed_checks_file" ]; then @@ -3688,9 +3778,7 @@ jobs: elif build_failed_check_fallback_body "$failed_checks_file" "$failed_check_evidence_file" "$failed_check_review_body_file"; then create_pull_review "REQUEST_CHANGES" "$(cat "$failed_check_review_body_file")" else - echo "::error::Failed GitHub Checks were present, but OpenCode could not derive source-backed line-specific findings after retries. Leaving the PR review unchanged so the current-head review can be rerun with better evidence." - echo "::endgroup::" - exit 1 + stop_failed_check_fallback_unavailable fi else publish_request_changes_from_control "$control_json" @@ -3702,9 +3790,18 @@ jobs: failed_check_inline_failure_body_file="$(mktemp)" failed_checks_file="$(mktemp)" if ! collect_github_checks_with_retry collect_failed_github_checks "$failed_checks_file"; then - echo "::error::GitHub Checks statusCheckRollup could not be read after OpenCode gate result ${gate_result:-empty}. Leaving the PR review unchanged." - echo "::endgroup::" - exit 1 + body="$(printf '%s\n' \ + "OpenCode could not interpret the model gate result because current-head checks were unavailable." \ + "" \ + "- Result: CHECKS_LOOKUP_FAILED" \ + "- Reason: GitHub Checks statusCheckRollup could not be read after OpenCode gate result ${gate_result:-empty}." \ + "- Required next evidence: readable current-head statusCheckRollup." \ + "- Head SHA: \`${HEAD_SHA}\`" \ + "- Workflow run: ${RUN_ID}" \ + "- Workflow attempt: ${RUN_ATTEMPT}" \ + "" \ + "No PR review was posted because check lookup failure is a review-tool state, not a source finding.")" + stop_approval_without_review "CHECKS_LOOKUP_FAILED" "$body" fi if [ -s "$failed_checks_file" ]; then @@ -3725,17 +3822,24 @@ jobs: elif build_failed_check_fallback_body "$failed_checks_file" "$failed_check_evidence_file" "$failed_check_review_body_file"; then create_pull_review "REQUEST_CHANGES" "$(cat "$failed_check_review_body_file")" else - echo "::error::Failed GitHub Checks were present, but OpenCode could not derive source-backed line-specific findings after retries. Leaving the PR review unchanged so the current-head review can be rerun with better evidence." - echo "::endgroup::" - exit 1 + stop_failed_check_fallback_unavailable fi else if request_changes_for_merge_conflict_if_present; then : else - echo "::error::OpenCode gate result ${gate_result:-empty} was not publishable for head ${HEAD_SHA}. Leaving the PR review unchanged because this is review tooling instability, not a source-code finding." - echo "::endgroup::" - exit 1 + body="$(printf '%s\n' \ + "OpenCode gate result was not publishable for the current head." \ + "" \ + "- Result: OPENCODE_REVIEW_UNAVAILABLE" \ + "- Reason: OpenCode gate result ${gate_result:-empty} was not publishable for head ${HEAD_SHA}." \ + "- Required next evidence: rerun OpenCode with a valid source-backed control block, or obtain a source-backed failed-check diagnosis." \ + "- Head SHA: \`${HEAD_SHA}\`" \ + "- Workflow run: ${RUN_ID}" \ + "- Workflow attempt: ${RUN_ATTEMPT}" \ + "" \ + "Leaving the PR review unchanged because this is review tooling instability, not a source-code finding.")" + stop_approval_without_review "OPENCODE_REVIEW_UNAVAILABLE" "$body" fi fi ;; diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 47b00d375..db016dcf1 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -30,10 +30,13 @@ on: concurrency: group: >- strix-${{ github.repository }}-${{ github.event_name == 'pull_request_target' && - format('pr-{0}', github.event.pull_request.number) || github.event.inputs.pr_number != '' && - format('pr-{0}', github.event.inputs.pr_number) || github.ref }} + format('pr-{0}-{1}', github.event.pull_request.number, github.event.pull_request.head.sha) || + github.event.inputs.pr_number != '' && github.event.inputs.pr_head_sha != '' && + format('pr-{0}-{1}', github.event.inputs.pr_number, github.event.inputs.pr_head_sha) || + github.event.inputs.pr_number != '' && format('pr-{0}', github.event.inputs.pr_number) || github.ref }} # cancel-in-progress deliberately disabled: an attacker could force-push - # a benign commit to cancel an in-progress scan of a malicious commit. + # a benign commit to cancel an in-progress scan of a malicious commit. The + # head SHA in PR groups prevents stale scans from serializing newer evidence. cancel-in-progress: false permissions: @@ -377,9 +380,9 @@ jobs: STRIX_SOURCE_DIRS: ". backend frontend" STRIX_REASONING_EFFORT: low STRIX_LLM_MAX_RETRIES: 1 - STRIX_TRANSIENT_RETRY_PER_MODEL: 5 + STRIX_TRANSIENT_RETRY_PER_MODEL: 2 STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS: 60 - STRIX_FALLBACK_MODELS: ${{ steps.gate.outputs.provider_mode == 'github_models' && 'github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324' || '' }} + STRIX_FALLBACK_MODELS: ${{ steps.gate.outputs.provider_mode == 'github_models' && 'github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528' || '' }} STRIX_FAIL_ON_PROVIDER_SIGNAL: "1" STRIX_VERTEX_FALLBACK_MODELS: "" NPM_CONFIG_IGNORE_SCRIPTS: "true" @@ -440,7 +443,7 @@ jobs: publish-manual-pr-evidence-status: name: publish-manual-pr-evidence-status needs: strix - if: ${{ always() && github.event_name == 'workflow_dispatch' && github.event.inputs.pr_head_sha != '' }} + if: ${{ always() && !cancelled() && github.event_name == 'workflow_dispatch' && github.event.inputs.pr_head_sha != '' }} runs-on: ubuntu-latest permissions: statuses: write diff --git a/PR_GOVERNANCE_AUDIT.md b/PR_GOVERNANCE_AUDIT.md index 8a5dc3b18..171f88b6a 100644 --- a/PR_GOVERNANCE_AUDIT.md +++ b/PR_GOVERNANCE_AUDIT.md @@ -1,33 +1,34 @@ # PR Governance Audit -Live check: 2026-06-23 KST, GitHub API via `gh` as `seonghobae`. +Live check: 2026-06-25 10:49 KST, GitHub API via `gh` as `seonghobae`. ## Canonical Policy OpenCode decides; GitHub Actions mutates. - OpenCode may return only a decision: `UPDATE_BRANCH`, `WAIT`, `REQUEST_CHANGES`, or `NO_ACTION`. -- GitHub Actions updates same-repository PR heads with `expected_head_sha`. +- GitHub Actions updates same-repository PR heads with `expected_head_sha` + only after current-head failed checks have been ruled out. +- The GitHub REST permission surfaces are split: `update-branch` uses Pull + requests write permission, while merge uses Contents write permission + (GitHub REST pull request endpoint docs: + https://docs.github.com/en/rest/pulls/pulls#update-a-pull-request-branch and + https://docs.github.com/en/rest/pulls/pulls#merge-a-pull-request, + 2026-06-25 check). Do not widen `contents` just to support `update-branch`. - Old approvals and old checks are not merge evidence after a head SHA changes. - Merge uses one path: current-head OpenCode approval, no unresolved review threads, required checks green or native auto-merge waiting on them, mergeable head, and no policy blocker. - Prefer `gh pr merge --auto --merge --match-head-commit ` when native auto-merge is enabled. - Use direct `gh pr merge --merge --match-head-commit ` only when the repo policy already allows immediate merge. - OpenCode app-token merges are deprecated; keep app tokens for review publication, not mechanical branch mutation. - OpenCode approval publication must be bounded. Peer GitHub Checks can be awaited, but the approval step itself must time out instead of running for hours; the current central limit is a 45 minute approval step with 81 peer-check probes at 30 seconds. -- Tool failures are not source findings. Model failure, API transient, update-branch `422/403`, fork/write-permission failure, conflict, failed checks, and stale review state must be reported as distinct scheduler outcomes. +- Tool failures are not source findings. Model failure, API transient, update-branch `422/403`, fork/write-permission failure, conflict, failed checks, and stale review state must be reported as distinct scheduler outcomes. A failed current-head check blocks `UPDATE_BRANCH`; the scheduler must not use a branch update as a way to hide or bypass failed evidence. - Developer experience and user experience are separate review surfaces. Reviews must adopt helpful sibling-repo automation, review, setup, documentation, and product-flow patterns when they reduce friction, and flag noisy automation, false failures, misleading status, repeated waiting, or URL-only diagnostics as experience defects instead of treating them as neutral implementation detail. ## Non-Actionable Findings Ban The review surface must not publish a `Findings` block that merely says the -reviewer failed to map evidence. In particular, this text is banned from PR -reviews: - -```text -No deterministic missing-string markers or Strix report locations were recognized. Use the failed-check evidence below to map each failed check to exact local source lines before approving. -``` - -That sentence is an internal diagnosis failure, not a code-review finding. If +reviewer failed to map evidence. Generic "I could not map the failed check" +phrasing is an internal diagnosis failure, not a code-review finding. If OpenCode or the deterministic fallback helper cannot map an active failed check to a concrete local file, positive line number, failed log phrase, observable impact, fix direction, regression command, and source-backed suggested diff, @@ -46,35 +47,37 @@ them to a local defect. ## Live Repository Inventory -Live generated: 2026-06-23 04:18 KST. PR #28 post-merge refresh: 2026-06-23 16:05 KST. PR #37 post-merge refresh: 2026-06-23 21:50 KST. clearfolio PR #13 post-merge refresh: 2026-06-24 04:48 KST. Non-actionable Findings refresh: 2026-06-25 KST. +Live generated: 2026-06-25 13:46 KST via GitHub REST/GraphQL APIs. PR #28 post-merge refresh: 2026-06-23 16:05 KST. PR #37 post-merge refresh: 2026-06-23 21:50 KST. clearfolio PR #13 post-merge refresh: 2026-06-24 04:48 KST. Non-actionable Findings refresh: 2026-06-25 KST. -| Repo | Flow | Default | Auto | Rulesets | Required checks | Stale dismissal | Merge queue | Workflows | Recent merged actor | +| Repo | Flow | Default | Auto | Rulesets | Required checks | Stale dismissal | Open PRs | Workflows | Recent merged actor | |---|---:|---:|---:|---|---|---:|---:|---|---| -| `ContextualWisdomLab/.github` | GitHub Flow | `main` | on | `Lock default branch` | none | true | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #49 `seonghobae`; #41 `seonghobae`; #38 `seonghobae` | -| `ContextualWisdomLab/bandscope` | Git Flow | `develop` | on | `Lock default branch` | `ci / build-and-test`, `dependency-review`, `security-audit`, `CodeQL`, `sbom`, `release-preflight`, `gate / build / windows`, `gate / build / macos`, `trivy-fs-scan` | false | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #427 `github-actions`; #408 `seonghobae`; #405 `seonghobae` | -| `ContextualWisdomLab/clearfolio` | GitHub Flow | `main` | off | `PR` | none | false | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #13 `seonghobae` merge `4bc17c6`; #9 `seonghobae`; #8 `seonghobae` | -| `ContextualWisdomLab/codec-carver` | GitHub Flow | `main` | on | `Lock default branch` | none | true | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #98 `seonghobae`; #94 `opencode-agent`; #93 `seonghobae` | -| `ContextualWisdomLab/contextual-orchestrator` | GitHub Flow | `main` | off | none | none | unknown | unknown | none matched | none | -| `ContextualWisdomLab/ContextualWisdomLab.github.io` | GitHub Flow | `main` | on | `Lock default branch` | none | true | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #15 `seonghobae`; #14 `seonghobae`; #13 `github-actions` auto by `github-actions` | -| `ContextualWisdomLab/naruon` | Git Flow | `develop` | on | `Lock default branch`, `PR` | `opencode-review`, `strix` | true | no | OpenCode Review; PR Governance; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | #756 `seonghobae`; #747 `seonghobae`; #715 `seonghobae` | -| `ContextualWisdomLab/newsdom-api` | Git Flow | `develop` | on | `Lock default branch`, `mirror-classic-protection-main-develop` | `codeql (python, actions)`, `dependency-review`, `pytest`, `quality-gate`, `scorecard` | true | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #197 `seonghobae`; #163 `seonghobae`; #105 `seonghobae` | -| `ContextualWisdomLab/pg-erd-cloud` | GitHub Flow | `main` | on | `Lock default branch` | none | true | no | OpenCode Review; PR Review Autofix; PR Review Fix Scheduler; PR Review Merge Scheduler; Strix Security Scan | #247 `github-actions`; #239 `github-actions`; #238 `seonghobae` | -| `ContextualWisdomLab/scopeweave` | Git Flow | `develop` | on | `Lock default branch` | none | true | no | OpenCode Review; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | #117 `seonghobae`; #106 `seonghobae`; #102 `seonghobae` | -| `ContextualWisdomLab/VibeSec` | Git Flow | `develop` | on | `Lock default branch`, `PR` | none | false/true | no | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #122 `seonghobae`; #109 `seonghobae`; #67 `github-actions` | +| `ContextualWisdomLab/.github` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 26 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #50 `seonghobae`; #51 `seonghobae`; #53 `github-actions` | +| `ContextualWisdomLab/bandscope` | Git Flow | `develop` | on | `Lock default branch` | `ci / build-and-test`, `dependency-review`, `security-audit`, `CodeQL`, `sbom`, `release-preflight`, `gate / build / windows`, `gate / build / macos`, `trivy-fs-scan` | ruleset true; classic false | 81 | OpenCode Review; PR Review Merge Scheduler | #374 `github-actions`; #388 `github-actions`; #379 `github-actions` | +| `ContextualWisdomLab/clearfolio` | GitHub Flow | `main` | off | `PR` | none | false | 13 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #8 `seonghobae`; #9 `seonghobae`; #13 `seonghobae` | +| `ContextualWisdomLab/codec-carver` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 8 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #103 `github-actions`; #98 `seonghobae`; #97 `opencode-agent` | +| `ContextualWisdomLab/contextual-orchestrator` | GitHub Flow | `main` | off | none | none | none | 0 | none matched | none | +| `ContextualWisdomLab/ContextualWisdomLab.github.io` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 6 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #15 `seonghobae`; #14 `seonghobae`; #13 `github-actions` | +| `ContextualWisdomLab/hyosung-itx-slogan-brief` | GitHub Flow | `main` | off | `Do not delete any branches` | none | none | 0 | OpenCode Review; PR Review Merge Scheduler | #1 `seonghobae` | +| `ContextualWisdomLab/naruon` | Git Flow | `develop` | on | `Lock default branch`, `PR` | `strix`, `opencode-review` | true | 2 | OpenCode Review; PR Governance; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | #758 `seonghobae`; #757 `seonghobae`; #749 `seonghobae` | +| `ContextualWisdomLab/newsdom-api` | Git Flow | `develop` | on | `Lock default branch`, `mirror-classic-protection-main-develop` | `pytest`, `scorecard`, `codeql (python, actions)`, `dependency-review`, `quality-gate` | ruleset true | 7 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #188 `seonghobae`; #204 `seonghobae`; #173 `seonghobae` | +| `ContextualWisdomLab/pg-erd-cloud` | GitHub Flow | `main` | on | `Lock default branch` | none | ruleset true | 10 | OpenCode Review; PR Review Autofix; PR Review Fix Scheduler; PR Review Merge Scheduler; Strix Security Scan | #247 `github-actions`; #246 `github-actions`; #239 `github-actions` | +| `ContextualWisdomLab/scopeweave` | Git Flow | `develop` | on | `Lock default branch` | none | ruleset true | 7 | OpenCode Review; PR Review Merge Scheduler; Strix Gate Self-Test; Strix Security Scan | #124 `seonghobae`; #118 `seonghobae`; #116 `seonghobae` | +| `ContextualWisdomLab/VibeSec` | Git Flow | `develop` | on | `Lock default branch`, `PR` | none | mixed true/false | 4 | OpenCode Review; PR Review Merge Scheduler; Strix Security Scan | #122 `seonghobae`; #121 `seonghobae`; #126 `github-actions` | ## Current Gaps By Repo | Repo | Gap | |---|---| -| `.github` | PR #37, #38, #41, #42, and #49 are merged. PR #49 is the current central proof that generic failed-check deflections such as `No deterministic missing-string markers...` are rejected before publication. Remaining open PRs still need current-head review/check evaluation rather than stale review reuse. | -| `bandscope` | Required checks are repo-specific and broad; keep GitHub native auto-merge as the check interpreter. | +| `.github` | PR #37, #38, #41, #42, and #49 are merged. PR #49 is the central proof that generic failed-check deflections are rejected before publication. PR #58 extends that contract so pending checks, check-rollup lookup failures, failed-check diagnosis gaps, and scheduler decisions stay tool states or Actions Summary output instead of becoming user-facing Findings. Remaining open PRs still need current-head review/check evaluation rather than stale review reuse. | +| `bandscope` | Required checks are repo-specific and broad; keep GitHub native auto-merge as the check interpreter. Live dry-run `28134181171` proved the repo-local scheduler still waited on an already enabled auto-merge request instead of updating a `BEHIND` approved PR, so PR #450 syncs the scheduler script, adds explicit update-branch workflow control, writes scheduler decisions to Actions Summary, and prevents failed-check mapping failures from being published as Findings. The default branch currently has OpenCode Review and PR Review Merge Scheduler, but no Strix workflow. | | `clearfolio` | PR #13 is merged at `4bc17c6` after same-head manual Strix run `28051319530`, same-head manual OpenCode run `28051665082`, unresolved review threads `0`, and guarded merge against head `5fe1791`. Auto-merge remains off, so direct guarded merge is the repo path. | | `codec-carver` | PR #98 replaced the legacy scheduler with the central GitHub Actions path. Keep #94 as the historical negative sample because it used `opencode-agent` as a merge actor. | | `contextual-orchestrator` | No matching rulesets or review workflows; either opt in deliberately or mark unmanaged. | +| `hyosung-itx-slogan-brief` | Public non-fork repo discovered in the 2026-06-25 13:46 KST refresh. It has OpenCode Review and PR Review Merge Scheduler but auto-merge is off and the only ruleset prevents branch deletion, so it should either stay as a lightweight GitHub Flow repo or explicitly opt into the default-branch lock contract. | | `naruon` | Canonical strict check source. PR #756 synced the central scheduler into `naruon`; its first head proved that widening `GITHUB_TOKEN` permissions to solve DX creates Scorecard and governance failures, so the merged rollout keeps minimal token permissions and defaults risky review-dispatch/auto-merge paths off. PR #721 remains the useful historical fixture for `BEHIND` handling: central dry-run selected `update_branch`, while the older repo-local workflow treated it as `wait`. | | `newsdom-api` | Ruleset-required checks must stay GitHub-interpreted; open queue is mostly review/check blocked. | | `pg-erd-cloud` | Good GitHub Actions merge samples; keep autofix workflows repo-local. | -| `scopeweave` | Has central scheduler and Strix self-test, but no current representative update/merge trace captured. | +| `scopeweave` | PR #127 is the current representative trace. Dry-run `28147098767` selected `auto_merge`, but live run `28147157319` failed with `GraphQL: Resource not accessible by integration (mergePullRequest)` because merge through GitHub Actions requires a contents-write mutation surface. Commit `6601953` proved the tempting fix, but Scorecard immediately opened a Token-Permissions review thread against job-level `contents: write`; follow-up commit `c5c5530` restores `contents: read` and keeps update-branch on the lower-privilege PR-write path. Actions-based merge remains an explicit repo policy exception, not the default rollout. | | `VibeSec` | Actor history is mixed; central scheduler should make GitHub Actions or native auto-merge the only mechanical path. | ## Representative Evidence @@ -82,7 +85,8 @@ Live generated: 2026-06-23 04:18 KST. PR #28 post-merge refresh: 2026-06-23 16:0 | Repo | Live evidence | Adopt | Reject | |---|---|---|---| | `naruon` | `develop`, strict required checks `opencode-review` and `strix`, stale review dismissal enabled. Open PRs show `BEHIND`, `DIRTY`, and `CHANGES_REQUESTED` cases. | Strict current-head evidence and stale-dismissal awareness. | Treating `BEHIND` as merge-ready. | -| `.github` | PR #28 head `811446d` reached current-head approval after manual Strix run `28007326148` published a successful `strix` status and manual OpenCode run `28008174977` approved the same head; it was merged by `seonghobae` with merge commit `a025be1`. PR #49 then merged the explicit ban on generic failed-check deflections such as `No deterministic missing-string markers...`. | Same-head manual evidence for self-modifying trusted workflow changes, current-head OpenCode approval, unresolved thread check, `--match-head-commit` guarded merge, and non-actionable Findings rejection. | Treating stale PR-target failure logs as merge blockers after newer same-head evidence exists, or posting an evidence-mapping failure as a user-facing Finding. | +| `bandscope` | Workflow dry-run `28134181171` used the repo-local scheduler and reported `PR #367: wait: current head is approved; auto-merge already enabled`, while the central scheduler dry-run selected `update_branch` for the same `BEHIND` + current-head-approved class. PR #450 is the corrective rollout. Its first head also reproduced the bad generic failed-check Findings text; the second head patches that path and the stale review was dismissed. | Keep broad repo-specific required checks delegated to GitHub, but update outdated same-repo PR heads before relying on native auto-merge. Treat failed-check mapping failure as a review-tool state unless a source-backed finding exists. | Assuming an enabled auto-merge request means the PR branch is current enough to merge, or converting missing evidence into a PR Finding. | +| `.github` | PR #28 head `811446d` reached current-head approval after manual Strix run `28007326148` published a successful `strix` status and manual OpenCode run `28008174977` approved the same head; it was merged by `seonghobae` with merge commit `a025be1`. PR #49 then merged the explicit ban on generic failed-check deflections, and PR #58 removes remaining fallback/pending/check-lookup paths that could turn review-tool states into PR review Findings. | Same-head manual evidence for self-modifying trusted workflow changes, current-head OpenCode approval, unresolved thread check, `--match-head-commit` guarded merge, and non-actionable Findings rejection. | Treating stale PR-target failure logs as merge blockers after newer same-head evidence exists, or posting an evidence-mapping failure as a user-facing Finding. | | `pg-erd-cloud` | Recent PRs #236, #237, #239 were merged by `app/github-actions`. | GitHub Actions as mechanical merge actor with head guard. | Human-only queue draining. | | `codec-carver` | Recent PR #94 was merged by `app/opencode-agent`, and the repo still has legacy `Scheduled PR Review Merge`. | Native auto-merge path for current-head approved PRs. | OpenCode app as merge actor. | | `VibeSec` | PR #108 had native auto-merge enabled; #106 merged by `app/github-actions`; #109 merged by human. | Keep native auto-merge as preferred waiting path. | Repo-by-repo actor inconsistency. | @@ -102,10 +106,11 @@ both separately; a change can improve one while harming the other. | `VibeSec` | Native auto-merge examples show a lower-friction waiting path after current-head approval. | Mixed human, OpenCode, and GitHub Actions merge actors make audit trails harder to interpret. | Prefer native auto-merge or GitHub Actions mutation; do not let OpenCode merge directly. | | `bandscope` | Broad required checks encode repo-specific release, build, SBOM, and security expectations. | A central script would be noisy if it tried to reinterpret every required check itself. | Let GitHub native auto-merge and rulesets interpret required checks. | | `newsdom-api` | Required quality gates and security checks give API changes stronger release evidence. | Central review comments that only point at failing check URLs do not help an API maintainer fix the failure. | Require failed-check root cause, source location when available, fix direction, and rerun command. | -| `scopeweave` | Strix self-test and the central scheduler are useful rollout fixtures. | Claiming the rollout complete without a representative current-head trace would be premature. | Keep it on the central path and require a live trace before declaring update/merge behavior proven. | +| `scopeweave` | Strix self-test and the central scheduler are useful rollout fixtures. Live scheduler run `28147157319` proved `action_error` is reported per PR instead of aborting the queue, and follow-up `c5c5530` shows the safer rollback when Scorecard rejects a broad token. | The scheduler could identify #127 as merge-ready, but enabling GitHub Actions merge by adding job-level `contents: write` triggered a Scorecard Token-Permissions thread. | Keep update-branch on `pull-requests: write` with `contents: read`; keep OpenCode read-only; require an explicit repo-level exception before letting the scheduler perform merge or auto-merge with `contents: write`. | | `clearfolio` | Direct guarded merge works while auto-merge is intentionally off. | Treating it like an auto-merge repo would create confusing expectations. | Use immediate guarded merge only after same-head evidence, unresolved-thread check, and head guard pass. | | `codec-carver` | Existing native merge behavior can be retained once current-head evidence is clean. | Legacy OpenCode app-token merge creates a second mechanical actor and weakens audit consistency. | Replace legacy scheduled merge with the central GitHub Actions scheduler. | | `ContextualWisdomLab.github.io` | Site and documentation changes make reader-facing UX review concrete. | Review comments that say only that a check failed do not help the site reader or maintainer understand the issue. | Treat documentation clarity, homepage behavior, and status-check explanations as UX surfaces. | +| `hyosung-itx-slogan-brief` | The repo has the central review/merge workflow names without heavier required checks, which makes it a lightweight GitHub Flow fixture. | It lacks the default-branch lock/stale-dismissal policy used by most organization repos. | Leave unmanaged only if that is intentional; otherwise add the standard default-branch lock before relying on autonomous merge. | | `contextual-orchestrator` | No central pattern is present yet, so it can be onboarded deliberately instead of accidentally. | Silent unmanaged status is easy to miss in organization-level governance. | Either opt it into the central workflows or explicitly mark it unmanaged. | ## Current Scheduler Contract @@ -113,16 +118,19 @@ both separately; a change can improve one while harming the other. The checked-in scheduler already does the minimal central path: - skips draft, wrong-base, and fork/external-head PRs; -- blocks `DIRTY` or `CONFLICTING`; +- blocks `DIRTY` or `CONFLICTING` with repair guidance that names the base branch, head branch, merge/rebase direction, conflict-marker cleanup, focused checks, same-branch push, and a compact `gh pr checkout` / `git fetch` / merge-or-rebase / `git status --short` command path; - blocks unresolved review threads; - blocks current-head OpenCode `CHANGES_REQUESTED`; - blocks current-head failed check runs or status contexts before enabling auto-merge; -- updates `BEHIND` only when OpenCode approved the exact current head, using `expected_head_sha`; +- updates `BEHIND` only when OpenCode approved the exact current head and no current-head failed check is present, using `expected_head_sha` from the scheduler workflow `GITHUB_TOKEN` so the mechanical branch update is performed by `github-actions[bot]` instead of an OpenCode or personal credential; this path needs `pull-requests: write`, not `contents: write`; - enables native auto-merge only for current-head OpenCode approval; - dispatches same-head Strix evidence first when the current head has no completed Strix evidence; - waits while same-head Strix evidence is still running, so OpenCode is not started just to poll a peer check; +- keeps old Strix evidence running instead of cancelling it, but scopes PR Strix concurrency by head SHA so an obsolete scan does not serialize newer current-head evidence; - dispatches OpenCode only after same-head Strix evidence is complete, including failed Strix evidence that OpenCode must explain from logs. - records mutation failures as `action_error` for the affected PR and continues scanning later PRs, so a permission failure on one merge/update action does not hide the rest of the queue. +- writes the same per-PR decisions to the GitHub Actions step summary, so conflict repair and update-branch decisions are visible without opening raw logs. +- caps each GraphQL PR page at 25 nodes, so large queues can be scanned without hitting GitHub's query resource limit. Small proof run: @@ -149,6 +157,17 @@ PR #34: block: current-head OpenCode review requested changes PR #35: block: current-head OpenCode review requested changes PR #36: block: merge conflict: DIRTY {"base_branch": "main", "counts": {"block": 17}, "dry_run": true, "inspected": 17, "project_flow": "github-flow"} + +$ python3 scripts/ci/pr_review_merge_scheduler.py --repo ContextualWisdomLab/bandscope --base-branch develop --project-flow git-flow --dry-run --max-prs 20 --no-trigger-reviews --no-enable-auto-merge +PR #367: update_branch: current-head OpenCode review approved; branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions) +PR #368: update_branch: current-head OpenCode review approved; branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions) +... +{"base_branch": "develop", "counts": {"block": 7, "update_branch": 13}, "dry_run": true, "inspected": 20, "project_flow": "git-flow"} + +$ gh run view 28134181171 --repo ContextualWisdomLab/bandscope --log +scheduler token source=github-token +PR #364: block: 2 unresolved review thread(s) +PR #367: wait: current head is approved; auto-merge already enabled ``` ## Rollout List @@ -161,12 +180,20 @@ PR #36: block: merge conflict: DIRTY ## Remaining Proof Gaps +- 2026-06-25 13:46 KST continuation snapshot: `.github` PR #58 is at head `0aa8b06cbd8653fa1b10dd4d017490810e3ecc5a` with manual Strix run `28147017855` in progress; `bandscope` PR #450 is still `BEHIND` with OpenCode in progress and macOS/rust gates queued; `newsdom-api` PR #207 is at head `ac3635eb4ad45722a1dcad90e58935be4607e73b` with Strix/fuzz in progress; `scopeweave` PR #127 is at head `66019533aa8343262879c027f40b831e78bfe9b4` after the scheduler-token fix, with checks queued and the previous OpenCode approval dismissed as stale; `naruon` PR #760 is at head `57a2f8e4fcdfd0c23380c4a5c80a12901e4fe606` with Strix/OpenCode in progress. All checked representative PRs had zero unresolved review threads in the live GraphQL checks. +- `newsdom-api` PR #207 is not update-branch proof. Its newer heads contain human-authored commits, including `ac3635eb4ad45722a1dcad90e58935be4607e73b` by `seonghobae`; the GitHub Actions bot update-branch proof must use a head commit authored by `github-actions[bot]` or an Actions run log showing the `update-branch` API call. - A live current-head review -> same-head manual Strix status bridge -> OpenCode approval -> guarded merge trace has been completed on `.github` PR #28. -- No live outdated -> update-branch -> new-head review -> merge/auto-merge trace has been completed yet. +- No live outdated -> update-branch -> new-head review -> merge/auto-merge trace has been completed yet. `bandscope` PR #450 is the first corrective rollout after live evidence showed the stale scheduler was waiting instead of updating. The update-branch leg now has a live partial proof: `bandscope` scheduler run `28139266598` selected PR #378 for `update_branch`, and the resulting PR head commit `68d5153ac9d5667c13b8e5e6a231c9fbb2a68f9f` was authored by `github-actions[bot]`. +- `bandscope` PR #378 still needs the new-head review/check/merge leg before the full outdated -> update-branch -> new-head review -> merge/auto-merge trace can be closed. +- `scopeweave` PR #127 now proves the current-head approval/check -> scheduler decision -> action-error leg: dry-run `28147098767` selected `auto_merge`, and live run `28147157319` reported `action_error` for `mergePullRequest` instead of posting a false code finding or aborting earlier PR decisions. Commit `6601953` showed why blindly adding `contents: write` is not an acceptable universal fix: Scorecard raised an unresolved Token-Permissions thread on the new head. Commit `c5c5530` restores the safer pattern: lower-privilege update-branch by GitHub Actions, with merge through Actions only where the repo deliberately accepts the contents-write exception. +- `bandscope` also proved the large-queue scan risk: `max_prs=120` initially failed with `Resource limits for this query exceeded` while reading 80 open PRs. After reducing the GraphQL page size to 25, the same dry-run scanned all 80 open PRs and returned `{"block": 67, "update_branch": 1, "wait": 12}`, including PR #378 as `update_branch` and PR #404 as a conflict block with repair guidance. +- `newsdom-api` PR #200 is the smaller current live proof candidate: head `c87140d3aa877106e26bcee705d988efe0384d23` is `BEHIND`, has current-head OpenCode approval, zero unresolved review threads, and green required checks on that head. Update-only scheduler run `28140376261` was dispatched with `update_branches=true`, `trigger_reviews=false`, and `enable_auto_merge=false`, but it remains queued. +- `.github` PR #58 exposed that a cancelled manual Strix run can keep its manual status publisher queued and delay the next same-PR Strix run. PR #58 now skips that publisher when the workflow is cancelled, scopes Strix PR concurrency by head SHA so obsolete scans do not serialize newer evidence, and requires conflict reviews to include a concrete `gh pr checkout` / `git fetch` / merge-or-rebase / `git status --short` repair path. - PR #721 in `naruon` remains the historical fixture for this proof: head `b683deaf8b4761399321799279f58d884db57141`, current-head OpenCode approval `4558310923`, unresolved review threads `0`, and `mergeStateStatus=BEHIND`. Central `.github` dry-run selected `update_branch`, but `naruon` workflow run `28073586594` used the then-stale repo-local scheduler and did not update it. PR #756 has since rolled the central scheduler into `naruon`, so the next proof must use a fresh current-head outdated PR instead of reusing stale evidence from #721. - `naruon` workflow run `28073490721` failed at `gh pr merge 694 --auto --merge --match-head-commit 76416321742af4c8dcd0f96927f64b7548d66fd8` with `GraphQL: Resource not accessible by integration (enablePullRequestAutoMerge)`. This is a DX/governance action failure, not a source-code finding, and the scheduler now records it per PR instead of aborting the scan. - `naruon` PR #756 completed the repo-local rollout for the scheduler contract. Its initial head failed backend governance and Scorecard because `actions: write`/`contents: write` were broader than the repo policy allows; the amended and merged head restores minimal `GITHUB_TOKEN` permissions, keeps `trigger_reviews` and `enable_auto_merge` defaulted off, keeps `update_branches` defaulted on, and still dry-runs PR #694/#721 as `update_branch`. - `update-branch` `422/403` behavior still needs a safe fixture or a real blocked case before claiming standardized handling. +- Public repo drift is real, not hypothetical: only `.github` matched the central scheduler/workflow byte-for-byte in the 2026-06-25 scan. Some drift is policy-specific and should not be overwritten blindly, but `bandscope` had behaviorally unsafe drift and now has PR #450. - Required-check interpretation should stay delegated to GitHub native auto-merge until a repo needs immediate merge. - PR #28 proves the self-modifying trusted workflow bootstrap path after newer same-head evidence exists, but it does not prove update-branch behavior, stale approval dismissal after a head change, or cross-repository rollout. - PR #37 adds a bounded OpenCode approval publication timeout after manual current-head OpenCode run `28011338113` reached the approval step and was observed waiting on peer checks instead of finishing promptly. diff --git a/README.md b/README.md index 141594205..c9ace239a 100644 --- a/README.md +++ b/README.md @@ -15,17 +15,37 @@ because it adds noise or misleading review experience. OpenCode judges PRs; GitHub Actions performs mechanical updates and merges. The scheduler updates a same-repository PR branch only when the latest OpenCode -review is approved and GitHub reports the PR as behind. After that update, the -new head must pass OpenCode, Strix, required checks, and review-thread gates -again before auto-merge or `--match-head-commit` merge can proceed. -Branch updates and merges run through the workflow `GITHUB_TOKEN`, so GitHub -records those mechanical mutations as `github-actions[bot]` rather than an -OpenCode app token or a personal token. +review is approved, no current-head failed check is present, and GitHub reports +the PR as behind. After that update, the new head must pass OpenCode, Strix, +required checks, and review-thread gates again before auto-merge or +`--match-head-commit` merge can proceed. +Branch updates run through the workflow `GITHUB_TOKEN`, so GitHub records those +mechanical updates as `github-actions[bot]` rather than an OpenCode app token or +a personal token. That path uses the pull-request branch update API and should +only need `pull-requests: write`; it does not justify widening repository +`contents` permission. Merge or auto-merge is a separate mutation. When a repo +wants GitHub Actions to perform the merge itself, that repo needs an explicit +scheduler-job `contents: write` policy exception and should expect Scorecard or +token-permission policy review to notice it. +That `update_branch` path is deliberately not used for `DIRTY` or +`CONFLICTING` PRs: GitHub cannot synthesize a safe conflict resolution for the +author, so the review must give the author a repair path instead of pretending +the bot can fix it. +When GitHub reports `DIRTY` or `CONFLICTING`, the scheduler does not pretend to +fix the branch. It blocks the PR with repair guidance: merge or rebase the +latest base branch into the PR branch, resolve conflict markers in that PR +branch, rerun focused checks, and push the same branch. OpenCode comments must +include a compact command block covering `gh pr checkout`, `git fetch`, merge or +rebase, `git status --short`, resolved-file staging, normal push, and +`--force-with-lease` only for rebased branches. OpenCode review execution is `workflow_dispatch`-only. The scheduler dispatches same-head Strix evidence first, then dispatches OpenCode for the same PR head. This avoids running PR-head review, CodeGraph, coverage, or PoC code from a privileged `pull_request_target` OpenCode workflow. +Strix keeps `cancel-in-progress: false` so old evidence is not cancelled by a +force-push, but PR-scoped concurrency includes the head SHA so an obsolete scan +does not serialize newer current-head evidence. OpenCode approval is evidence-gated. Before approval, the review summary must name changed files, CodeGraph or structural MCP evidence, a Change Flow DAG, diff --git a/scripts/ci/emit_opencode_failed_check_fallback_findings.sh b/scripts/ci/emit_opencode_failed_check_fallback_findings.sh index f8d13e8c8..940267a40 100755 --- a/scripts/ci/emit_opencode_failed_check_fallback_findings.sh +++ b/scripts/ci/emit_opencode_failed_check_fallback_findings.sh @@ -668,8 +668,8 @@ emit_strix_provider_failure_finding() { if grep -Eq "api\\.deepseek\\.com|401 Unauthorized|Authentication Fails|DeepseekException" "$strix_evidence_file"; then printf -- '- Problem: Strix failed before producing vulnerability reports. The failed log reported `RateLimitError` / `Too many requests` for the primary `openai/gpt-5` attempt, then fallback attempts reached direct DeepSeek (`api.deepseek.com`) and failed with `401 Unauthorized` or `Authentication Fails`, ending with `Configured model and fallback models were unavailable`.\n' printf -- '- Root cause: The fallback model names were not routed through the GitHub Models endpoint for this failed PR check, so a GitHub Models token was used against direct DeepSeek instead of `https://models.github.ai/inference`; no Strix Vulnerability Report window was produced.\n' - printf -- '- Fix: Do not approve from this failed scan. Keep %s:%s using the GitHub Models-qualified fallback list (`github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324`) and keep the Strix gate mapping those values to `openai/deepseek/...` for the GitHub Models API base, then rerun the failed PR Strix check.\n' "$path" "$line" - printf -- '- Suggested edit: `%s:%s` must use `STRIX_FALLBACK_MODELS: ${{ steps.gate.outputs.provider_mode == '\''github_models'\'' && '\''github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324'\'' || '\'''\'' }}` instead of unqualified `deepseek/...` values that route to `api.deepseek.com`.\n' "$path" "$line" + printf -- '- Fix: Do not approve from this failed scan. Keep %s:%s using the GitHub Models-qualified fallback list (`github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528`) and keep the Strix gate mapping those values to `openai/deepseek/...` for the GitHub Models API base, then rerun the failed PR Strix check.\n' "$path" "$line" + printf -- '- Suggested edit: `%s:%s` must use `STRIX_FALLBACK_MODELS: ${{ steps.gate.outputs.provider_mode == '\''github_models'\'' && '\''github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528'\'' || '\'''\'' }}` instead of unqualified `deepseek/...` values that route to `api.deepseek.com`.\n' "$path" "$line" else printf -- '- Problem: Strix failed before producing vulnerability reports. The failed log reported LLM CONNECTION FAILED, RateLimitError or Too many requests for the primary model, provider/budget output for fallback models, and Configured model and fallback models were unavailable.\n' printf -- '- Root cause: The configured GitHub Models primary/fallback provider capacity or provider route failed for this run; no Strix Vulnerability Report window was produced, so there is no application source line to patch from this evidence.\n' @@ -745,6 +745,6 @@ emit_strix_provider_failure_finding "$strix_evidence_file" emit_strix_cancelled_without_log_finding "$strix_evidence_file" if [ "$finding_index" -eq 0 ]; then - printf 'No source-backed failed-check fallback finding matched the available evidence; leaving the PR review unchanged so the current-head review can be rerun with better evidence.\n' >&2 + printf 'No source-backed failed-check fallback finding matched the available evidence. No PR review was posted; retry after current-head failed-check logs or annotations are available, or rerun the failed check to collect them.\n' >&2 exit 1 fi diff --git a/scripts/ci/opencode_review_normalize_output.py b/scripts/ci/opencode_review_normalize_output.py index 8c9cf928b..b865a566f 100755 --- a/scripts/ci/opencode_review_normalize_output.py +++ b/scripts/ci/opencode_review_normalize_output.py @@ -72,11 +72,11 @@ ) NON_ACTIONABLE_FAILED_CHECK_REVIEW_PHRASES = ( - "no deterministic missing-string markers", - "no deterministic missing string markers", - "strix report locations were recognized", - "use the failed-check evidence below to map", - "map each failed check to exact local source lines before approving", + "deterministic missing-string markers", + "deterministic missing string markers", + "strix report locations", + "failed-check evidence below", + "map each failed check to exact local source lines", ) CHANGED_FILE_EVIDENCE_PATTERN = re.compile( diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 8ad8d7074..df56b3dfb 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -6,6 +6,7 @@ import argparse import json import os +import shlex import subprocess import sys from collections.abc import Sequence @@ -70,6 +71,8 @@ } """ +OPEN_PRS_PAGE_SIZE = 25 + @dataclass class Decision: @@ -120,7 +123,7 @@ def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: cursor: str | None = None while len(prs) < max_prs: - page_size = min(100, max_prs - len(prs)) + page_size = min(OPEN_PRS_PAGE_SIZE, max_prs - len(prs)) fields: dict[str, str | int] = { "owner": owner, "name": name, @@ -338,6 +341,20 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry ) +def merge_conflict_guidance(pr: dict[str, Any], merge_state: str) -> str: + """Return actionable conflict repair guidance for a conflicting PR.""" + base_ref = pr.get("baseRefName") or "base" + head_ref = pr.get("headRefName") or "head" + return ( + f"merge conflict: {merge_state}; base={base_ref}, head={head_ref}; " + f"run `gh pr checkout {pr.get('number', '')}`, `git fetch origin {base_ref}`, then " + f"`git merge --no-ff origin/{base_ref}` or `git rebase origin/{base_ref}`; " + "use `git status --short` to find conflicted files, resolve conflict markers in the PR branch, " + f"rerun focused checks, and push the same {head_ref} branch " + "(use `git push --force-with-lease` only if rebased)" + ) + + def inspect_pr( repo: str, pr: dict[str, Any], @@ -364,7 +381,7 @@ def inspect_pr( merge_state = (pr.get("mergeStateStatus") or "").upper() if merge_state in {"DIRTY", "CONFLICTING"}: - return Decision(number, "block", f"merge conflict: {merge_state}") + return Decision(number, "block", merge_conflict_guidance(pr, merge_state)) unresolved = unresolved_thread_count(pr) if unresolved: @@ -373,16 +390,23 @@ def inspect_pr( if has_current_head_changes_requested(pr): return Decision(number, "block", "current-head OpenCode review requested changes") - if merge_state == "BEHIND" and has_current_head_approval(pr): + current_head_approved = has_current_head_approval(pr) + if current_head_approved: + failed_checks = failed_status_checks(pr) + if failed_checks: + return Decision(number, "block", f"failed check(s): {', '.join(failed_checks[:5])}") + + if merge_state == "BEHIND" and current_head_approved: if not update_branches: return Decision(number, "wait", "current-head OpenCode review approved; branch update disabled") update_branch(repo, pr, dry_run=dry_run) - return Decision(number, "update_branch", "current-head OpenCode review approved; branch update requested") + return Decision( + number, + "update_branch", + "current-head OpenCode review approved; branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions)", + ) - if has_current_head_approval(pr): - failed_checks = failed_status_checks(pr) - if failed_checks: - return Decision(number, "block", f"failed check(s): {', '.join(failed_checks[:5])}") + if current_head_approved: if pr.get("autoMergeRequest"): return Decision(number, "wait", "current head is approved; auto-merge already enabled") if not enable_auto_merge_flag: @@ -428,6 +452,13 @@ def print_summary( for decision in decisions: counts[decision.action] = counts.get(decision.action, 0) + 1 print(f"PR #{decision.pr}: {decision.action}: {decision.reason}") + write_actions_summary( + decisions, + counts=counts, + dry_run=dry_run, + base_branch=base_branch, + project_flow=project_flow, + ) print( json.dumps( { @@ -442,12 +473,178 @@ def print_summary( ) +def markdown_cell(value: object) -> str: + """Escape a value for a compact GitHub Actions summary table cell.""" + return str(value).replace("|", "\\|").replace("\n", "
") + + +def write_actions_summary( + decisions: list[Decision], + *, + counts: dict[str, int], + dry_run: bool, + base_branch: str, + project_flow: str, +) -> None: + """Append scheduler decisions to the GitHub Actions step summary.""" + summary_path = os.environ.get("GITHUB_STEP_SUMMARY") + if not summary_path: + return + + lines = [ + "## PR review merge scheduler", + "", + f"- Base branch: `{base_branch}`", + f"- Project flow: `{project_flow}`", + f"- Dry run: `{str(dry_run).lower()}`", + f"- Inspected PRs: `{len(decisions)}`", + f"- Actions: `{json.dumps(counts, sort_keys=True)}`", + "", + "| PR | Action | Reason |", + "| ---: | --- | --- |", + ] + lines.extend( + f"| #{decision.pr} | {markdown_cell(decision.action)} | {markdown_cell(decision.reason)} |" + for decision in decisions + ) + lines.extend(conflict_repair_summary(decisions)) + lines.extend(update_branch_summary(decisions)) + lines.extend(action_error_summary(decisions)) + + with open(summary_path, "a", encoding="utf-8") as handle: + handle.write("\n".join(lines)) + handle.write("\n") + + +def parse_conflict_reason(reason: str) -> tuple[str, str, str] | None: + """Extract merge state, base branch, and head branch from conflict guidance.""" + prefix = "merge conflict: " + if not reason.startswith(prefix): + return None + state = reason[len(prefix) :].split(";", 1)[0].strip() or "UNKNOWN" + base_ref = "base" + head_ref = "head" + for segment in reason.split(";"): + segment = segment.strip() + if not segment.startswith("base="): + continue + branch_bits = segment.split(",") + for branch_bit in branch_bits: + key, _, value = branch_bit.strip().partition("=") + if key == "base" and value: + base_ref = value + if key == "head" and value: + head_ref = value + break + return state, base_ref, head_ref + + +def conflict_repair_summary(decisions: list[Decision]) -> list[str]: + """Return a GitHub Actions Summary section with concrete conflict repair steps.""" + conflicted = [(decision, parse_conflict_reason(decision.reason)) for decision in decisions] + conflicted = [(decision, parsed) for decision, parsed in conflicted if parsed is not None] + if not conflicted: + return [] + + lines = [ + "", + "### Conflict repair", + "", + "GitHub cannot safely update `DIRTY` or `CONFLICTING` PR branches. Repair the PR branch, then push the same branch so OpenCode and required checks can run on the new head.", + ] + for decision, parsed in conflicted: + assert parsed is not None + state, base_ref, head_ref = parsed + base_remote = f"origin/{base_ref}" + lines.extend( + [ + "", + f"PR #{decision.pr} is `{state}` against `{base_ref}` from `{head_ref}`:", + "", + "```bash", + f"gh pr checkout {decision.pr}", + f"git fetch origin {shlex.quote(base_ref)}", + "# choose merge or rebase", + f"git merge --no-ff {shlex.quote(base_remote)}", + f"# git rebase {shlex.quote(base_remote)}", + "git status --short", + "# resolve conflict markers in the PR branch", + "git add ", + "# run the focused checks for the changed area", + "git push", + "# if you chose rebase: git push --force-with-lease", + "```", + ] + ) + return lines + + +def update_branch_summary(decisions: list[Decision]) -> list[str]: + """Return a GitHub Actions Summary section explaining branch update mutations.""" + updates = [decision for decision in decisions if decision.action == "update_branch"] + if not updates: + return [] + pr_list = ", ".join(f"#{decision.pr}" for decision in updates) + return [ + "", + "### Branch update requests", + "", + f"Requested `update-branch` for PR {pr_list} with the workflow `GITHUB_TOKEN`, guarded by the observed `expected_head_sha`.", + "This branch-update API path needs `pull-requests: write`; it does not require the scheduler job to widen repository `contents` to write.", + "When repository permissions allow the mutation, GitHub records the resulting branch update as `github-actions[bot]`.", + "The updated head is not merge evidence by itself. Wait for the new head to receive OpenCode approval, Strix evidence, required checks, and unresolved-thread checks before merge or auto-merge.", + ] + + +def action_error_summary(decisions: list[Decision]) -> list[str]: + """Return a GitHub Actions Summary section for mutation failures.""" + errors = [decision for decision in decisions if decision.action == "action_error"] + if not errors: + return [] + lines = [ + "", + "### Action errors", + "", + "These are scheduler or GitHub permission/runtime failures, not source-code review findings.", + ] + for decision in errors: + lines.append(f"- PR #{decision.pr}: {decision.reason}") + return lines + + +def bounded_error_summary(text: str, *, limit: int = 500) -> str: + """Cap an action-error message without dropping the actionable prefix.""" + return text if len(text) <= limit else text[: limit - 1].rstrip() + "..." + + def summarize_action_error(exc: RuntimeError) -> str: """Return a compact, log-safe scheduler action error summary.""" lines = [line.strip() for line in str(exc).splitlines() if line.strip()] if not lines: return "scheduler action failed without stderr" - return "; ".join(lines[:2])[:500] + summary = "; ".join(lines[:2]) + lower_summary = summary.lower() + if "resource not accessible by integration" in lower_summary: + if "mergepullrequest" in lower_summary or "enablepullrequestautomerge" in lower_summary or "gh pr merge" in lower_summary: + summary = ( + f"{summary}; scheduler GitHub token could not perform merge or auto-merge. " + "Merging through GitHub Actions needs an explicit repo policy exception for scheduler-job `contents: write`; otherwise leave auto-merge disabled and keep update-branch on the lower-privilege PR-write path." + ) + elif "update-branch" in lower_summary: + summary = ( + f"{summary}; scheduler GitHub token could not update the PR branch. " + "Give the scheduler job `pull-requests: write`, then rerun with the same expected-head guard; do not widen `contents` just for update-branch." + ) + else: + summary = ( + f"{summary}; scheduler GitHub token lacks a required repository mutation permission. " + "Fix the scheduler job permissions instead of posting a code-review finding." + ) + if "expected_head_sha" in lower_summary and ("422" in lower_summary or "head" in lower_summary): + summary = ( + f"{summary}; the PR head likely changed after inspection. Rerun the scheduler so it reads the new head before mutating." + ) + return bounded_error_summary(summary) def self_test() -> None: @@ -584,6 +781,42 @@ def self_test() -> None: base_branch="main", ) assert decision.action == "update_branch" + sample["statusCheckRollup"]["contexts"]["nodes"] = [ + {"__typename": "CheckRun", "name": "strix", "status": "COMPLETED", "conclusion": "FAILURE"} + ] + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "block" + assert decision.reason == "failed check(s): strix" + sample["statusCheckRollup"]["contexts"]["nodes"] = [] + sample["mergeStateStatus"] = "DIRTY" + decision = inspect_pr( + "owner/repo", + sample, + dry_run=True, + trigger_reviews=True, + enable_auto_merge_flag=True, + update_branches=True, + workflow="OpenCode Review", + security_workflow="Strix Security Scan", + base_branch="main", + ) + assert decision.action == "block" + assert "gh pr checkout 1" in decision.reason + assert "git fetch origin main" in decision.reason + assert "git merge --no-ff origin/main" in decision.reason + assert "git rebase origin/main" in decision.reason + assert "git status --short" in decision.reason + assert "resolve conflict markers" in decision.reason print("self-test passed") diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 69ebdb0a8..e0258779c 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -96,10 +96,12 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" "branches: [main, develop, master]" "strix workflow scans GitHub Flow and Git Flow protected branches" assert_file_contains "$workflow_file" "pull_request_target:" "strix workflow uses trusted PR trigger" - assert_file_contains "$workflow_file" "format('pr-{0}', github.event.pull_request.number)" "strix workflow scopes concurrency to the active pull request" - assert_file_contains "$workflow_file" "format('pr-{0}', github.event.inputs.pr_number)" "strix workflow scopes manual PR evidence concurrency to the requested pull request" + assert_file_contains "$workflow_file" "format('pr-{0}-{1}', github.event.pull_request.number, github.event.pull_request.head.sha)" "strix workflow scopes pull_request_target concurrency to the active pull request head" + assert_file_contains "$workflow_file" "format('pr-{0}-{1}', github.event.inputs.pr_number, github.event.inputs.pr_head_sha)" "strix workflow scopes manual PR evidence concurrency to the requested pull request head" + assert_file_contains "$workflow_file" "github.event.inputs.pr_number != '' && format('pr-{0}', github.event.inputs.pr_number)" "strix workflow retains a manual PR fallback group when no head SHA is provided" assert_file_contains "$workflow_file" "|| github.ref" "strix workflow scopes non-PR concurrency to the current ref" assert_file_contains "$workflow_file" "cancel-in-progress: false" "strix workflow never cancels in-progress security evidence" + assert_file_contains "$workflow_file" "head SHA in PR groups prevents stale scans from serializing newer evidence" "strix workflow documents stale scan queue avoidance" assert_file_contains "$workflow_file" "models: read" "strix workflow grants only the GitHub Models read permission needed for Strix" assert_file_contains "$workflow_file" "actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6" "strix workflow pins actions/setup-python" assert_file_contains "$workflow_file" 'python-version: "3.13"' "strix workflow runs Python steps on Python 3.13" @@ -213,7 +215,7 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" "https://models.github.ai/inference" "strix workflow routes GitHub Models scans to the inference endpoint" assert_file_contains "$workflow_file" "LLM_API_BASE_FILE" "strix workflow passes the GitHub Models API base through a trusted input file" assert_file_not_contains "$workflow_file" '${{ secrets.STRIX_OPENAI_API_KEY || github.token }}' "strix workflow must not use fallback-secret syntax for LLM API keys" - assert_file_contains "$workflow_file" "github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324" "strix workflow configures reachable stronger-than-GPT-4.1 GitHub Models fallback models" + assert_file_contains "$workflow_file" "github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528" "strix workflow configures reachable stronger-than-GPT-4.1 GitHub Models fallback models" assert_file_not_contains "$workflow_file" 'github_models/deepseek/deepseek-r1-0528 | github_models/deepseek/deepseek-v3-0324)' "strix workflow keeps DeepSeek GitHub Models restricted to fallback-only routing" assert_file_contains "$workflow_file" '${strix_model#github_models/}' "strix workflow strips manual github_models routing prefix for OpenAI GPT model names before passing model names to LiteLLM" assert_file_contains "$workflow_file" "openai_direct/%s" "strix workflow keeps manual direct OpenAI scans distinct from GitHub Models openai/gpt-* routing" @@ -423,6 +425,10 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" 'git -C "$OPENCODE_SOURCE_WORKDIR" ls-tree -r --name-only "$PR_HEAD_SHA" -- "$docs_dir"' "opencode review evidence lists current-head docs assets from the PR head worktree before judging docs claims" assert_file_contains "$workflow_file" "Do not claim repository docs, images, or reference assets are unavailable, missing, or absent unless the changed docs repository tree evidence proves it." "opencode review prompt forbids unsupported docs asset absence claims" assert_file_contains "$workflow_file" "Merge Conflict Guidance" "opencode review overview includes conflict repair guidance" + assert_file_contains "$workflow_file" "gh pr checkout" "opencode merge-conflict guidance starts from checking out the PR branch" + assert_file_contains "$workflow_file" "git fetch origin" "opencode merge-conflict guidance fetches the latest base branch" + assert_file_contains "$workflow_file" "git status --short" "opencode merge-conflict guidance tells the author how to find unresolved conflict files" + assert_file_contains "$workflow_file" "git push --force-with-lease" "opencode merge-conflict guidance limits force pushes to the rebase path" assert_file_contains "$workflow_file" "mergeStateStatus DIRTY or CONFLICTING" "opencode review prompt handles merge conflicts" assert_file_contains "$workflow_file" "mergeStateStatus BLOCKED is a branch policy, review, or check state, not conflict guidance" "opencode review prompt does not misclassify branch-policy blockers as merge conflicts" if [ -e "$REPO_ROOT/.github/workflows/opencode-merge-conflict-guidance.yml" ]; then @@ -726,8 +732,8 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" '"lsp": "allow"' "opencode generated config enables LSP" assert_file_contains "$workflow_file" '"lsp": true' "opencode generated config starts built-in LSP servers when available" assert_file_contains "$workflow_file" "OpenCode runtime tools are enabled: bash, task, webfetch, websearch, and lsp." "opencode review prompt names the enabled runtime tools" - assert_file_contains "$workflow_file" "OpenCode failed-check fallback helper did not produce source-backed findings; leaving the PR review unchanged for rerun." "opencode failed-check fallback avoids generic review comments when helper output is not source-backed" - assert_file_contains "$workflow_file" "OpenCode failed-check fallback helper returned non-source-backed generic output; leaving the PR review unchanged for rerun." "opencode failed-check fallback rejects stale helper scripts that exit zero with generic no-evidence text" + assert_file_contains "$workflow_file" "OpenCode failed-check fallback helper did not produce source-backed findings. No PR review was posted; retry after current-head failed-check logs or annotations are available" "opencode failed-check fallback avoids generic review comments when helper output is not source-backed" + assert_file_contains "$workflow_file" "OpenCode failed-check fallback helper returned non-source-backed output. No PR review was posted; retry after current-head failed-check logs or annotations are available" "opencode failed-check fallback rejects stale helper scripts that exit zero with generic no-evidence text" assert_file_contains "$workflow_file" "could not derive source-backed line-specific findings after retries" "opencode failed-check fallback fails the check instead of posting URL-only request-changes reviews" assert_file_not_contains "$workflow_file" "OpenCode failed-check fallback helper exited non-zero; using inline fallback." "opencode failed-check fallback must not silently downgrade helper failures to generic inline fallback reviews" assert_file_contains "$workflow_file" "Do not depend on Copilot Review, CodeRabbitAI, or any human reviewer" "opencode review format is independent of other review agents" @@ -1729,7 +1735,7 @@ EOF assert_file_contains "$output_file" "Strix provider failure blocked current-head security evidence" "fallback treats no-report summary as provider blocker" assert_file_contains "$output_file" "api.deepseek.com" "fallback preserves direct DeepSeek endpoint failure evidence" assert_file_contains "$output_file" "Authentication Fails" "fallback preserves direct DeepSeek authentication failure evidence" - assert_file_contains "$output_file" "github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324" "fallback gives exact GitHub Models fallback list" + assert_file_contains "$output_file" "github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528" "fallback gives exact GitHub Models fallback list" assert_file_contains "$output_file" "Suggested edit: \`.github/workflows/strix.yml" "fallback gives a line-specific suggested edit for provider routing" assert_file_not_contains "$output_file" "Strix provider signal left current-head security evidence incomplete" "fallback does not invent vulnerability report windows from a no-report summary" assert_file_not_contains "$output_file" "after vulnerability reports" "fallback does not contradict no-report evidence" @@ -1925,7 +1931,7 @@ jobs: steps: - name: Run Strix env: - STRIX_FALLBACK_MODELS: github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324 + STRIX_FALLBACK_MODELS: github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528 EOF cat >"$evidence_file" <<'EOF' @@ -9362,11 +9368,11 @@ run_gate_case "github-models-fallback-requires-api-base" \ run_gate_case "github-models-fallback-success" \ "vertex_ai/missing-primary" \ - "github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324" \ + "github_models/deepseek/deepseek-v3-0324 github_models/deepseek/deepseek-r1-0528" \ "0" \ - "REGEX:Strix quick scan succeeded with fallback model 'github_models/deepseek/deepseek-r1-0528' in [0-9]+s\\." \ + "REGEX:Strix quick scan succeeded with fallback model 'github_models/deepseek/deepseek-v3-0324' in [0-9]+s\\." \ "2" \ - "vertex_ai/missing-primary|openai/deepseek/deepseek-r1-0528" \ + "vertex_ai/missing-primary|openai/deepseek/deepseek-v3-0324" \ "|https://models.github.ai/inference" \ "vertex_ai" \ "https://models.github.ai/inference" \ diff --git a/tests/test_opencode_review_normalize_output.py b/tests/test_opencode_review_normalize_output.py index eac03297c..1590fd786 100644 --- a/tests/test_opencode_review_normalize_output.py +++ b/tests/test_opencode_review_normalize_output.py @@ -155,14 +155,13 @@ def test_check_structural_approval_rejects_invalid_or_unsafe_approvals(tmp_path) control( result="REQUEST_CHANGES", summary=( - "No deterministic missing-string markers or Strix report locations were " - "recognized. Use the failed-check evidence below to map each failed check " - "to exact local source lines before approving." + "The review could not map each failed check to exact local source lines " + "from the available logs, so it needs better failed-check evidence." ), findings=[ finding( title="Generic failed-check deflection", - problem="No deterministic missing-string markers were recognized.", + problem="The failed-check diagnosis did not produce source-backed findings.", ) ], ) @@ -206,9 +205,8 @@ def test_valid_control_filters_shape_head_and_review_contract(): dict( request, summary=( - "No deterministic missing-string markers or Strix report locations were " - "recognized. Use the failed-check evidence below to map each failed check " - "to exact local source lines before approving." + "The review could not map each failed check to exact local source lines " + "from the available logs, so it needs better failed-check evidence." ), ), **kwargs, diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index f1a006ea7..aa64d4bf7 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -136,6 +136,28 @@ def fake_graphql(query, **fields): assert seen[1]["cursor"] == "cursor" +def test_fetch_open_prs_caps_page_size_to_avoid_graphql_resource_limits(monkeypatch): + seen = [] + + def fake_graphql(query, **fields): + seen.append(fields) + return { + "data": { + "repository": { + "pullRequests": { + "nodes": [{"number": fields["pageSize"]}], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + } + } + } + } + + monkeypatch.setattr(sched, "gh_graphql", fake_graphql) + + assert sched.fetch_open_prs("owner/repo", 120) == [{"number": sched.OPEN_PRS_PAGE_SIZE}] + assert seen[0]["pageSize"] == sched.OPEN_PRS_PAGE_SIZE + + def test_context_review_and_check_helpers(): assert sched.context_nodes({}) == [] assert sched.context_nodes(make_pr()) == [] @@ -248,15 +270,99 @@ def test_actions_call_gh_with_expected_arguments(monkeypatch): sched.dispatch_opencode_review("owner/repo", "OpenCode Review", pr, dry_run=False) assert calls[0][:4] == ["gh", "pr", "merge", "1"] assert calls[1][:4] == ["gh", "api", "-X", "PUT"] + assert calls[1][-1] == "expected_head_sha=head" assert calls[2][:5] == ["gh", "workflow", "run", "Strix Security Scan", "--repo"] assert calls[3][:5] == ["gh", "workflow", "run", "OpenCode Review", "--repo"] +def test_print_summary_writes_github_step_summary(monkeypatch, tmp_path, capsys): + summary_path = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary_path)) + decisions = [ + sched.Decision(7, "block", "merge conflict: DIRTY; base=main, head=feature|x"), + sched.Decision( + 8, + "update_branch", + "current-head OpenCode review approved; branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions)", + ), + ] + + sched.print_summary(decisions, dry_run=True, base_branch="main", project_flow="github-flow") + + output = capsys.readouterr().out + assert "PR #7: block: merge conflict: DIRTY" in output + assert json.loads(output.splitlines()[-1]) == { + "base_branch": "main", + "counts": {"block": 1, "update_branch": 1}, + "dry_run": True, + "inspected": 2, + "project_flow": "github-flow", + } + summary = summary_path.read_text(encoding="utf-8") + assert "## PR review merge scheduler" in summary + assert "| #7 | block | merge conflict: DIRTY; base=main, head=feature\\|x |" in summary + assert ( + "| #8 | update_branch | current-head OpenCode review approved; " + "branch update requested with workflow GH_TOKEN (github-actions[bot] in GitHub Actions) |" + ) in summary + assert "### Conflict repair" in summary + assert "PR #7 is `DIRTY` against `main` from `feature\\|x`:" not in summary + assert "PR #7 is `DIRTY` against `main` from `feature|x`:" in summary + assert "gh pr checkout 7" in summary + assert "git fetch origin main" in summary + assert "git merge --no-ff origin/main" in summary + assert "git push --force-with-lease" in summary + assert "### Branch update requests" in summary + assert "Requested `update-branch` for PR #8 with the workflow `GITHUB_TOKEN`" in summary + assert "needs `pull-requests: write`" in summary + assert "does not require the scheduler job to widen repository `contents` to write" in summary + assert "github-actions[bot]" in summary + + +def test_write_actions_summary_is_noop_without_summary_path(monkeypatch): + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + + sched.write_actions_summary( + [sched.Decision(1, "block", "merge conflict: DIRTY; base=main, head=feature")], + counts={"block": 1}, + dry_run=True, + base_branch="main", + project_flow="github-flow", + ) + + +def test_summary_section_helpers_handle_empty_and_action_error_cases(): + wait_decisions = [sched.Decision(1, "wait", "nothing to do")] + assert sched.conflict_repair_summary(wait_decisions) == [] + assert sched.update_branch_summary(wait_decisions) == [] + assert sched.action_error_summary(wait_decisions) == [] + + lines = sched.action_error_summary([sched.Decision(2, "action_error", "permission failed")]) + assert "### Action errors" in lines + assert "not source-code review findings" in "\n".join(lines) + assert "- PR #2: permission failed" in lines + + def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert inspect(make_pr(isDraft=True)).action == "skip" assert inspect(make_pr(baseRefName="develop")).reason == "base branch is develop; expected main" assert inspect(make_pr(headRepository={"nameWithOwner": "fork/repo"})).action == "skip" - assert inspect(make_pr(mergeStateStatus="DIRTY")).reason == "merge conflict: DIRTY" + conflict = inspect(make_pr(mergeStateStatus="DIRTY")) + assert conflict.action == "block" + assert "merge conflict: DIRTY" in conflict.reason + assert "base=main, head=feature" in conflict.reason + assert "gh pr checkout 1" in conflict.reason + assert "git fetch origin main" in conflict.reason + assert "git merge --no-ff origin/main" in conflict.reason + assert "git rebase origin/main" in conflict.reason + assert "git status --short" in conflict.reason + assert "resolve conflict markers in the PR branch" in conflict.reason + assert "rerun focused checks" in conflict.reason + assert "git push --force-with-lease" in conflict.reason + assert "push the same feature branch" in conflict.reason + conflicting = inspect(make_pr(mergeStateStatus="CONFLICTING")) + assert conflicting.action == "block" + assert "merge conflict: CONFLICTING" in conflicting.reason assert inspect(make_pr(reviewThreads={"nodes": [{"isResolved": False}]})).reason == "1 unresolved review thread(s)" assert inspect(make_pr(reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "head")]})).reason == ( "current-head OpenCode review requested changes" @@ -273,7 +379,27 @@ def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): assert inspect(behind, update_branches=False).reason == "current-head OpenCode review approved; branch update disabled" called = [] monkeypatch.setattr(sched, "update_branch", lambda repo, pr, dry_run: called.append((repo, pr["number"], dry_run))) - assert inspect(behind).action == "update_branch" + decision = inspect(behind) + assert decision.action == "update_branch" + assert "workflow GH_TOKEN" in decision.reason + assert "github-actions[bot]" in decision.reason + assert called == [("owner/repo", 1, True)] + called.clear() + behind_failed = make_pr( + mergeStateStatus="BEHIND", + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + statusCheckRollup={"contexts": {"nodes": [{"__typename": "CheckRun", "name": "strix", "conclusion": "FAILURE"}]}}, + ) + failed_decision = inspect(behind_failed) + assert failed_decision.action == "block" + assert failed_decision.reason == "failed check(s): strix" + assert called == [] + behind_auto_merge_enabled = make_pr( + mergeStateStatus="BEHIND", + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + autoMergeRequest={"enabledAt": "now"}, + ) + assert inspect(behind_auto_merge_enabled).action == "update_branch" assert called == [("owner/repo", 1, True)] @@ -372,5 +498,44 @@ def fake_inspect(repo, pr, **kwargs): assert seen == [1, 2] output = capsys.readouterr().out assert "PR #1: action_error: Command failed (1): gh pr merge 1; GraphQL: Resource not accessible by integration" in output + assert "scheduler GitHub token could not perform merge or auto-merge" in output assert "PR #2: wait: next PR still inspected" in output assert json.loads(output.strip().splitlines()[-1])["counts"] == {"action_error": 1, "wait": 1} + + +def test_action_error_guidance_distinguishes_update_branch_from_merge(): + update_error = sched.summarize_action_error( + RuntimeError( + "Command failed (1): gh api -X PUT repos/owner/repo/pulls/7/update-branch\n" + "HTTP 403: Resource not accessible by integration" + ) + ) + assert "pull-requests: write" in update_error + assert "do not widen `contents` just for update-branch" in update_error + + merge_error = sched.summarize_action_error( + RuntimeError( + "Command failed (1): gh pr merge 7 --auto --merge\n" + "GraphQL: Resource not accessible by integration (mergePullRequest)" + ) + ) + assert "explicit repo policy exception" in merge_error + assert "contents: write" in merge_error + + unknown_mutation_error = sched.summarize_action_error( + RuntimeError( + "Command failed (1): gh api graphql -f mutation=unknown\n" + "GraphQL: Resource not accessible by integration (unknownMutation)" + ) + ) + assert "lacks a required repository mutation permission" in unknown_mutation_error + assert "instead of posting a code-review finding" in unknown_mutation_error + + stale_head_error = sched.summarize_action_error( + RuntimeError( + "Command failed (1): gh api -X PUT repos/owner/repo/pulls/7/update-branch\n" + "HTTP 422: expected_head_sha does not match current head" + ) + ) + assert "PR head likely changed after inspection" in stale_head_error + assert "reads the new head before mutating" in stale_head_error