diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index a9bb54f8a..bc0bf2c02 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -1049,8 +1049,12 @@ jobs: # PR_REVIEW_MERGE_TOKEN does not cover it. The automation can never # merge those PRs regardless, so this is a skipped, non-fatal # "unavailable" repository, not a failure the sweep can act on. Any - # other non-zero exit is a genuine per-repository failure. - if printf '%s' "$sweep_output" | grep -qF "Resource not accessible by integration"; then + # other non-zero exit is a genuine per-repository failure. A + # versioned scheduler payload proves the repository was readable + # and the failure was an attempted per-PR action, even when that + # action's API error contains the same 403 wording. + if printf '%s' "$sweep_output" | grep -qF "Resource not accessible by integration" && + ! printf '%s' "$sweep_output" | grep -Eq '"schema_version"[[:space:]]*:[[:space:]]*"pr-review-merge-scheduler/v2"'; then echo "::warning::Skipping ${repo_full_name}: the sweep credential lacks access (HTTP 403 Resource not accessible by integration). Install the OpenCode app on this repository or grant PR_REVIEW_MERGE_TOKEN access to include it in the sweep." unavailable=$((unavailable + 1)) unavailable_repos+=("$repo_full_name") diff --git a/CHANGELOG.md b/CHANGELOG.md index 1630c32d4..c016e801f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,8 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Made pull-request scheduler mutation and dispatch failures fail the targeted workflow and organization sweep after the complete structured decision summary is emitted, while ordinary policy waits remain successful. + - Publish only the sanitized cumulative Strix report tree, avoiding a later copy of relative scanner output that could reintroduce known internal warning text into uploaded security evidence. diff --git a/docs/doctoring/pr-review-merge-scheduler.md b/docs/doctoring/pr-review-merge-scheduler.md new file mode 100644 index 000000000..4e355ec52 --- /dev/null +++ b/docs/doctoring/pr-review-merge-scheduler.md @@ -0,0 +1,40 @@ +# PR review and merge scheduler + +## Terminal result policy + +The scheduler isolates a failed mutation or dispatch to its pull request and +continues the bounded scan. It emits the human-readable lines, job summary, and +versioned JSON decision payload for every inspected pull request before choosing +the process result. + +An `action_error` is a material execution failure, so one or more such decisions +produce a non-zero terminal result after the summary is written. Policy outcomes +such as `wait`, `block`, `skip`, and deferred capacity do not make an otherwise +healthy scheduler invocation fail. + +A targeted single-pull-request run and the organization sweep use the same +terminal policy. The organization sweep preserves each repository's captured +summary, records that repository as failed, finishes its bounded repository +walk, and then fails the job. A repository is classified as unavailable only +when the scheduler fails before emitting its versioned structured payload and +the error proves that the sweep credential cannot read the repository. + +This separation keeps ordinary governance waits visible without reporting them +as incidents, while preventing a failed merge, update, auto-merge, or review +dispatch from producing a passing workflow result. GitHub Actions maps a +non-zero exit code to a failed check, and `GITHUB_STEP_SUMMARY` retains the +operator-facing Markdown evidence before that terminal result. + +## References (APA 7th) + +GitHub. (n.d.). *Setting exit codes for actions*. GitHub Docs. Retrieved August +24, 2026, from +https://docs.github.com/en/actions/how-tos/create-and-publish-actions/set-exit-codes + +GitHub. (n.d.). *Workflow commands for GitHub Actions*. GitHub Docs. Retrieved +August 24, 2026, from +https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands + +GitHub. (n.d.). *Workflow syntax for GitHub Actions*. GitHub Docs. Retrieved +August 24, 2026, from +https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 44620fcab..35ea31563 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2883,6 +2883,17 @@ def print_summary( ) +def scheduler_exit_code(decisions: list[Decision]) -> int: + """Return failure after a complete scan when a requested action failed. + + Ordinary policy outcomes remain successful scheduler executions. A caught + ``action_error`` is different: the scheduler attempted a mutation or + dispatch and could not complete it. The caller receives that failure only + after :func:`print_summary` preserves every per-PR decision. + """ + return 1 if any(decision.action == "action_error" for decision in decisions) else 0 + + def markdown_cell(value: object) -> str: """Escape a value for a compact GitHub Actions summary table cell.""" return str(value).replace("|", "\\|").replace("\n", "
") @@ -3960,7 +3971,7 @@ def main(argv: list[str]) -> int: base_branch=args.base_branch, project_flow=args.project_flow, ) - return 0 + return scheduler_exit_code(decisions) if __name__ == "__main__": # pragma: no cover diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 0e71bdbe2..24af4332d 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -4622,7 +4622,7 @@ def fake_inspect(repo, pr, **kwargs): monkeypatch.setattr(sched, "fetch_open_prs", lambda repo, max_prs: prs) monkeypatch.setattr(sched, "inspect_pr", fake_inspect) - assert sched.main(["--repo", "owner/repo", "--base-branch", "main", "--project-flow", "github"]) == 0 + assert sched.main(["--repo", "owner/repo", "--base-branch", "main", "--project-flow", "github"]) == 1 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 @@ -4714,7 +4714,7 @@ def fake_inspect(repo, pr, **kwargs): monkeypatch.setattr(sched, "fetch_open_prs", lambda repo, max_prs: prs) monkeypatch.setattr(sched, "inspect_pr", fake_inspect) - assert sched.main(["--repo", "owner/repo", "--base-branch", "main", "--project-flow", "github"]) == 0 + assert sched.main(["--repo", "owner/repo", "--base-branch", "main", "--project-flow", "github"]) == 1 assert seen == [1, 2, 3] output = capsys.readouterr().out assert "PR #1: action_error:" in output diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index e58f5e6c0..48f4baebf 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1219,6 +1219,19 @@ def test_org_queue_sweep_treats_inaccessible_repositories_as_non_fatal() -> None # The 403 signal is classified as a skipped, non-fatal "unavailable" repo. assert "ORG_SWEEP_MAX_UNAVAILABLE" in workflow assert 'grep -qF "Resource not accessible by integration"' in workflow + assert "grep -Eq '\"schema_version\"[[:space:]]*:[[:space:]]*\"pr-review-merge-scheduler/v2\"'" in workflow + schema_pattern = r'"schema_version"[[:space:]]*:[[:space:]]*"pr-review-merge-scheduler/v2"' + for payload in ( + '{"schema_version":"pr-review-merge-scheduler/v2"}\n', + '{\n "schema_version" : "pr-review-merge-scheduler/v2"\n}\n', + ): + result = subprocess.run( + ["grep", "-Eq", schema_pattern], + input=payload, + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr assert "unavailable=$((unavailable + 1))" in workflow assert 'unavailable_repos+=("$repo_full_name")' in workflow assert "the sweep credential lacks access (HTTP 403" in workflow @@ -1235,6 +1248,37 @@ def test_org_queue_sweep_treats_inaccessible_repositories_as_non_fatal() -> None assert "ORG_SWEEP_MAX_UNAVAILABLE must be a non-negative integer" in workflow +def test_scheduler_action_errors_propagate_after_structured_summary() -> None: + """Targeted and organization scans must fail after retaining their summary.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + + targeted = workflow.split(" - name: Inspect PR review and merge queue", 1)[1].split( + "\n org-queue-sweep:", 1 + )[0] + assert 'python3 scripts/ci/pr_review_merge_scheduler.py "${args[@]}"' in targeted + assert "continue-on-error: true" not in targeted + assert "|| true" not in targeted + + org_sweep = workflow.split(" org-queue-sweep:", 1)[1] + assert 'sweep_output="$(python3 scripts/ci/pr_review_merge_scheduler.py "${args[@]}" 2>&1)"' in org_sweep + assert "sweep_rc=$?" in org_sweep + assert 'if [ "$sweep_rc" -ne 0 ]; then' in org_sweep + assert "grep -Eq '\"schema_version\"[[:space:]]*:[[:space:]]*\"pr-review-merge-scheduler/v2\"'" in org_sweep + assert "failures=$((failures + 1))" in org_sweep + + +def test_scheduler_exit_policy_is_documented() -> None: + """Keep the doctoring record bound to the terminal action-error contract.""" + policy = (REPO_ROOT / "docs/doctoring/pr-review-merge-scheduler.md").read_text( + encoding="utf-8" + ) + + assert "action_error" in policy + assert "non-zero" in policy + assert "targeted" in policy + assert "organization sweep" in policy + + def test_fix_scheduler_cancels_superseded_cron_runs() -> None: """Cancel stale scheduled repair runs before they duplicate mutation work.""" workflow = workflow_text("pr-review-fix-scheduler.yml")