From 826c6eeec7aa5a72ce69521e23c3f6a1b998fe52 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 8 Aug 2026 23:24:43 +0900 Subject: [PATCH 1/6] fix(actions): bound agent mention dispatch envelope --- .../agent-mention-opencode-dispatch.yml | 199 +++--- .../agent-mention-router-quality-ci.yml | 18 + .../workflows/opencode-review-dispatch.yml | 11 +- .../workflows/pr-review-merge-scheduler.yml | 173 ++++- .github/workflows/strix.yml | 7 +- CHANGELOG.md | 1 + .../review-agent-comment-invocation.md | 14 +- scripts/ci/agent_mention_router.py | 31 +- scripts/ci/pr_review_merge_scheduler.py | 364 ++++++++++- scripts/ci/test_strix_quick_gate.sh | 18 +- ..._agent_mention_complete_payload_binding.py | 60 +- ...st_agent_mention_downstream_idempotency.py | 27 +- tests/test_agent_mention_idempotency.py | 18 +- ...nt_mention_repository_dispatch_envelope.py | 343 ++++++++++ tests/test_agent_mention_router.py | 12 +- tests/test_opencode_agent_contract.py | 17 +- tests/test_opencode_workflow_shell_syntax.py | 96 +++ tests/test_pr_review_merge_scheduler.py | 609 ++++++++++++++++++ .../test_required_workflow_queue_contract.py | 34 +- 19 files changed, 1835 insertions(+), 217 deletions(-) create mode 100644 tests/test_agent_mention_repository_dispatch_envelope.py diff --git a/.github/workflows/agent-mention-opencode-dispatch.yml b/.github/workflows/agent-mention-opencode-dispatch.yml index 160b4723d..af8446286 100644 --- a/.github/workflows/agent-mention-opencode-dispatch.yml +++ b/.github/workflows/agent-mention-opencode-dispatch.yml @@ -1,7 +1,7 @@ name: Agent Mention OpenCode Dispatch run-name: >- - Agent Mention OpenCode ${{ github.event.client_payload.target_repository }}#${{ - github.event.client_payload.pr_number }} [cwl-agent-invocation:${{ + Agent Mention OpenCode ${{ github.event.client_payload.claim.repository }}#${{ + github.event.client_payload.claim.pr_number }} [cwl-agent-invocation:${{ github.event.client_payload.agent_invocation_key }}] on: @@ -26,40 +26,15 @@ jobs: contents: write env: GH_TOKEN: ${{ github.token }} - REQUESTED_AGENT: "opencode-agent" - PAYLOAD_AGENT: ${{ github.event.client_payload.requested_agent || '' }} + CLIENT_PAYLOAD_JSON: ${{ toJSON(github.event.client_payload) }} + PAYLOAD_SCHEMA: ${{ github.event.client_payload.schema || '' }} INVOCATION_KEY: ${{ github.event.client_payload.agent_invocation_key || '' }} - TARGET_REPOSITORY: ${{ github.event.client_payload.target_repository || '' }} - PR_NUMBER: ${{ github.event.client_payload.pr_number || '' }} - PR_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha || '' }} - PR_BASE_SHA: ${{ github.event.client_payload.pr_base_sha || '' }} - BASE_BRANCH: ${{ github.event.client_payload.base_branch || '' }} - REQUESTED_BY: ${{ github.event.client_payload.requested_by || '' }} - SOURCE_COMMENT_ID: ${{ github.event.client_payload.source_comment_id || '' }} - TRIGGER_REVIEWS: ${{ github.event.client_payload.trigger_reviews }} - REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || '' }} - ENABLE_AUTO_MERGE: ${{ github.event.client_payload.enable_auto_merge }} - UPDATE_BRANCHES: ${{ github.event.client_payload.update_branches }} - MERGE_MODE: ${{ github.event.client_payload.merge_mode || '' }} steps: - - name: Validate exact invocation payload + - name: Validate exact invocation payload and prepare scheduler request run: | set -euo pipefail - if [ "$PAYLOAD_AGENT" != "$REQUESTED_AGENT" ] || - ! [[ "$INVOCATION_KEY" =~ ^[0-9a-f]{64}$ ]] || - ! [[ "$TARGET_REPOSITORY" =~ ^ContextualWisdomLab/[A-Za-z0-9_.-]+$ ]] || - ! [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]] || - ! [[ "$PR_HEAD_SHA" =~ ^[0-9a-f]{40}$ ]] || - ! [[ "$PR_BASE_SHA" =~ ^[0-9a-f]{40}$ ]] || - ! [[ "$BASE_BRANCH" =~ ^[A-Za-z0-9._/-]+$ ]] || - [[ "$BASE_BRANCH" == -* ]] || - ! [[ "$SOURCE_COMMENT_ID" =~ ^[1-9][0-9]*$ ]] || - [ "$TRIGGER_REVIEWS" != "true" ] || - [ "$REVIEW_DISPATCH_LIMIT" != "1" ] || - [ "$ENABLE_AUTO_MERGE" != "false" ] || - [ "$UPDATE_BRANCHES" != "false" ] || - [ "$MERGE_MODE" != "disabled" ] || - ! [[ "$REQUESTED_BY" =~ ^[A-Za-z0-9-]+$ ]]; then + if [ "$PAYLOAD_SCHEMA" != "cwl.agent-invocation/v2" ] || + ! [[ "$INVOCATION_KEY" =~ ^[0-9a-f]{64}$ ]]; then echo "::error::Rejected malformed or mismatched OpenCode agent invocation payload." exit 1 fi @@ -69,23 +44,85 @@ jobs: import hmac import json import os + from pathlib import Path + import re + + envelope = json.loads(os.environ["CLIENT_PAYLOAD_JSON"]) + envelope_keys = {"schema", "claim", "agent_invocation_key"} + claim_keys = { + "actor", + "agent", + "base_branch", + "base_sha", + "comment_id", + "enable_auto_merge", + "head_sha", + "merge_mode", + "pr_number", + "repository", + "review_dispatch_limit", + "trigger_reviews", + "update_branches", + } + if not isinstance(envelope, dict) or set(envelope) != envelope_keys: + raise SystemExit("invalid OpenCode invocation envelope") + claim = envelope["claim"] + if not isinstance(claim, dict): + raise SystemExit("OpenCode invocation claim must be an object") + if envelope["schema"] != "cwl.agent-invocation/v2": + raise SystemExit("unsupported OpenCode invocation schema") + if envelope["schema"] != os.environ["PAYLOAD_SCHEMA"]: + raise SystemExit("OpenCode invocation schema context mismatch") + if envelope["agent_invocation_key"] != os.environ["INVOCATION_KEY"]: + raise SystemExit("OpenCode invocation key context mismatch") + if set(claim) != claim_keys: + raise SystemExit("invalid OpenCode invocation claim fields") + + text_fields = { + "actor", + "agent", + "base_branch", + "base_sha", + "head_sha", + "merge_mode", + "repository", + "review_dispatch_limit", + } + if any(not isinstance(claim[field], str) for field in text_fields): + raise SystemExit("OpenCode invocation claim has a non-string text field") + if type(claim["comment_id"]) is not int or claim["comment_id"] < 1: + raise SystemExit("OpenCode invocation claim has an invalid comment id") + if type(claim["pr_number"]) is not int or claim["pr_number"] < 1: + raise SystemExit("OpenCode invocation claim has an invalid pull request number") + for field in ("enable_auto_merge", "trigger_reviews", "update_branches"): + if type(claim[field]) is not bool: + raise SystemExit(f"OpenCode invocation claim has an invalid {field} flag") + if not re.fullmatch(r"ContextualWisdomLab/[A-Za-z0-9_.-]+", claim["repository"]): + raise SystemExit("OpenCode invocation claim has an invalid repository") + if not re.fullmatch(r"[0-9a-f]{40}", claim["head_sha"]): + raise SystemExit("OpenCode invocation claim has an invalid head SHA") + if not re.fullmatch(r"[0-9a-f]{40}", claim["base_sha"]): + raise SystemExit("OpenCode invocation claim has an invalid base SHA") + if ( + not re.fullmatch(r"[A-Za-z0-9._/-]+", claim["base_branch"]) + or claim["base_branch"].startswith("-") + ): + raise SystemExit("OpenCode invocation claim has an invalid base branch") + if not re.fullmatch(r"[A-Za-z0-9-]+", claim["actor"]): + raise SystemExit("OpenCode invocation claim has an invalid actor") + expected_policy = { + "agent": "opencode-agent", + "enable_auto_merge": False, + "merge_mode": "disabled", + "review_dispatch_limit": "1", + "trigger_reviews": True, + "update_branches": False, + } + if any(claim[field] != value for field, value in expected_policy.items()): + raise SystemExit("OpenCode invocation claim violates review-only policy") canonical = json.dumps( - { - "actor": os.environ["REQUESTED_BY"], - "agent": os.environ["REQUESTED_AGENT"], - "base_branch": os.environ["BASE_BRANCH"], - "base_sha": os.environ["PR_BASE_SHA"], - "comment_id": int(os.environ["SOURCE_COMMENT_ID"]), - "enable_auto_merge": os.environ["ENABLE_AUTO_MERGE"] == "true", - "head_sha": os.environ["PR_HEAD_SHA"], - "merge_mode": os.environ["MERGE_MODE"], - "pr_number": int(os.environ["PR_NUMBER"]), - "repository": os.environ["TARGET_REPOSITORY"], - "review_dispatch_limit": os.environ["REVIEW_DISPATCH_LIMIT"], - "trigger_reviews": os.environ["TRIGGER_REVIEWS"] == "true", - "update_branches": os.environ["UPDATE_BRANCHES"] == "true", - }, + claim, ensure_ascii=True, separators=(",", ":"), sort_keys=True, @@ -93,6 +130,41 @@ jobs: expected = hashlib.sha256(canonical).hexdigest() if not hmac.compare_digest(expected, os.environ["INVOCATION_KEY"]): raise SystemExit("invocation key does not match canonical payload") + + scheduler_request = { + "event_type": "merge-scheduler-agent-review-v2", + "client_payload": envelope, + } + encoded_request = json.dumps( + scheduler_request, + ensure_ascii=True, + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + if len(envelope) > 10 or len(encoded_request) > 65_535: + raise SystemExit("scheduler repository dispatch exceeds GitHub limits") + + request_path = Path(os.environ["RUNNER_TEMP"]) / "agent-review-scheduler-request.json" + request_path.write_bytes(encoded_request) + exports = { + "BASE_BRANCH": claim["base_branch"], + "ENABLE_AUTO_MERGE": str(claim["enable_auto_merge"]).lower(), + "MERGE_MODE": claim["merge_mode"], + "PR_BASE_SHA": claim["base_sha"], + "PR_HEAD_SHA": claim["head_sha"], + "PR_NUMBER": str(claim["pr_number"]), + "REQUESTED_AGENT": claim["agent"], + "REQUESTED_BY": claim["actor"], + "REVIEW_DISPATCH_LIMIT": claim["review_dispatch_limit"], + "SCHEDULER_REQUEST_FILE": str(request_path), + "SOURCE_COMMENT_ID": str(claim["comment_id"]), + "TARGET_REPOSITORY": claim["repository"], + "TRIGGER_REVIEWS": str(claim["trigger_reviews"]).lower(), + "UPDATE_BRANCHES": str(claim["update_branches"]).lower(), + } + with Path(os.environ["GITHUB_ENV"]).open("a", encoding="utf-8") as handle: + for name, value in exports.items(): + handle.write(f"{name}={value}\n") PYTHON - name: Inspect exact-name Actions artifact ledger @@ -189,33 +261,6 @@ jobs: if: steps.ledger.outputs.claim == 'true' run: | set -euo pipefail - jq -n \ - --arg target_repository "$TARGET_REPOSITORY" \ - --argjson pr_number "$PR_NUMBER" \ - --arg pr_head_sha "$PR_HEAD_SHA" \ - --arg pr_base_sha "$PR_BASE_SHA" \ - --arg base_branch "$BASE_BRANCH" \ - --arg requested_agent "$REQUESTED_AGENT" \ - --arg agent_invocation_key "$INVOCATION_KEY" \ - --arg requested_by "$REQUESTED_BY" \ - --argjson source_comment_id "$SOURCE_COMMENT_ID" \ - '{ - event_type: "merge-scheduler", - client_payload: { - target_repository: $target_repository, - pr_number: $pr_number, - pr_head_sha: $pr_head_sha, - pr_base_sha: $pr_base_sha, - base_branch: $base_branch, - trigger_reviews: true, - review_dispatch_limit: "1", - enable_auto_merge: false, - update_branches: false, - merge_mode: "disabled", - requested_agent: $requested_agent, - agent_invocation_key: $agent_invocation_key, - requested_by: $requested_by, - source_comment_id: $source_comment_id - } - }' \ - | gh api "repos/${GITHUB_REPOSITORY}/dispatches" -X POST --input - + gh api "repos/${GITHUB_REPOSITORY}/dispatches" \ + -X POST \ + --input "$SCHEDULER_REQUEST_FILE" diff --git a/.github/workflows/agent-mention-router-quality-ci.yml b/.github/workflows/agent-mention-router-quality-ci.yml index f69cdce10..66634aa7c 100644 --- a/.github/workflows/agent-mention-router-quality-ci.yml +++ b/.github/workflows/agent-mention-router-quality-ci.yml @@ -8,11 +8,20 @@ on: - ".github/workflows/agent-mention-router-quality-ci.yml" - ".github/workflows/agent-mention-noema-dispatch.yml" - ".github/workflows/agent-mention-opencode-dispatch.yml" + - ".github/workflows/opencode-review-dispatch.yml" + - ".github/workflows/pr-review-merge-scheduler.yml" + - ".github/workflows/strix.yml" - "docs/automation/review-agent-comment-invocation.md" - "scripts/ci/agent_mention_router.py" - "scripts/ci/agent_mention_sweep.py" + - "scripts/ci/pr_review_merge_scheduler.py" + - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_agent_mention_*.py" + - "tests/test_opencode_agent_contract.py" + - "tests/test_opencode_workflow_shell_syntax.py" + - "tests/test_pr_review_merge_scheduler.py" - "tests/test_pr_review_fix_scheduler_coverage.py" + - "tests/test_required_workflow_queue_contract.py" - "requirements-opencode-review-ci-hashes.txt" push: branches: [main] @@ -21,11 +30,20 @@ on: - ".github/workflows/agent-mention-router-quality-ci.yml" - ".github/workflows/agent-mention-noema-dispatch.yml" - ".github/workflows/agent-mention-opencode-dispatch.yml" + - ".github/workflows/opencode-review-dispatch.yml" + - ".github/workflows/pr-review-merge-scheduler.yml" + - ".github/workflows/strix.yml" - "docs/automation/review-agent-comment-invocation.md" - "scripts/ci/agent_mention_router.py" - "scripts/ci/agent_mention_sweep.py" + - "scripts/ci/pr_review_merge_scheduler.py" + - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_agent_mention_*.py" + - "tests/test_opencode_agent_contract.py" + - "tests/test_opencode_workflow_shell_syntax.py" + - "tests/test_pr_review_merge_scheduler.py" - "tests/test_pr_review_fix_scheduler_coverage.py" + - "tests/test_required_workflow_queue_contract.py" - "requirements-opencode-review-ci-hashes.txt" concurrency: diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 83f6830d5..e532041ca 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -12,13 +12,10 @@ on: types: [opencode-review] concurrency: - # PR-number scope keeps stale dispatches replaced for the current head. - group: >- - opencode-review-repository-dispatch-${{ - github.event.client_payload.target_repository || github.repository }}-${{ - github.event.client_payload.pr_number && format('pr-{0}', github.event.client_payload.pr_number) || - github.run_id }} - cancel-in-progress: true + # A repository_dispatch is validated only after queue admission. Run-id scope + # prevents an out-of-order stale dispatch from cancelling newer valid work. + group: opencode-review-repository-dispatch-${{ github.run_id }} + cancel-in-progress: false permissions: contents: read diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 8e1157060..0520a555b 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -84,7 +84,7 @@ on: # within ~15 minutes instead of sitting idle for up to an hour. - cron: "*/15 * * * *" repository_dispatch: - types: [merge-scheduler] + types: [merge-scheduler, merge-scheduler-agent-review-v2] concurrency: group: >- @@ -95,8 +95,10 @@ concurrency: github.event_name == 'workflow_call' && inputs.pr_number != '' && format('pr-{0}', inputs.pr_number) || github.event_name == 'workflow_call' && inputs.base_branch != '' && format('call-{0}', inputs.base_branch) || github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule) || - github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != '' && format('target-{0}-pr-{1}', github.event.client_payload.target_repository, github.event.client_payload.pr_number) || - github.event_name == 'repository_dispatch' && github.event.client_payload.pr_number != '' && format('pr-{0}', github.event.client_payload.pr_number) || + github.event_name == 'repository_dispatch' && github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.agent_invocation_key != '' && format('agent-review-{0}', github.event.client_payload.agent_invocation_key) || + github.event_name == 'repository_dispatch' && github.event.action == 'merge-scheduler-agent-review-v2' && github.run_id || + github.event_name == 'repository_dispatch' && github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != '' && format('target-{0}-pr-{1}', github.event.client_payload.target_repository, github.event.client_payload.pr_number) || + github.event_name == 'repository_dispatch' && github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.pr_number != '' && format('pr-{0}', github.event.client_payload.pr_number) || github.event_name == 'repository_dispatch' && github.run_id || github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || github.event_name == 'repository_dispatch' }} @@ -150,17 +152,17 @@ jobs: env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true GH_TOKEN: ${{ github.token }} - DEFAULT_BRANCH: ${{ github.event.client_payload.base_branch || inputs.base_branch || github.event.repository.default_branch }} + DEFAULT_BRANCH: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.base_branch || github.event.client_payload.base_branch || inputs.base_branch || github.event.repository.default_branch }} DRY_RUN: ${{ github.event.client_payload.dry_run == true || inputs.dry_run == true }} MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || '100' }} PROJECT_FLOW_INPUT: ${{ github.event.client_payload.project_flow || inputs.project_flow || vars.PROJECT_FLOW || '' }} - PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || github.event.client_payload.pr_number || inputs.pr_number || '' }} - TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_run' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false) || inputs.trigger_reviews == true }} - REVIEW_DISPATCH_LIMIT_INPUT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '1' }} + PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.pr_number || github.event.client_payload.pr_number || inputs.pr_number || '' }} + TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_run' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.action == 'merge-scheduler-agent-review-v2') || (github.event_name == 'repository_dispatch' && github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.trigger_reviews != false) || inputs.trigger_reviews == true }} + REVIEW_DISPATCH_LIMIT_INPUT: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && '1' || github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '1' }} BRANCH_UPDATE_LIMIT_INPUT: ${{ github.event.client_payload.branch_update_limit || inputs.branch_update_limit || vars.BRANCH_UPDATE_LIMIT || '1' }} - ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false) || inputs.enable_auto_merge == true }} - MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || vars.PR_MERGE_MODE || 'direct_or_auto' }} - UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false) || inputs.update_branches == true }} + ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.enable_auto_merge != false) || inputs.enable_auto_merge == true }} + MERGE_MODE: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && 'disabled' || github.event.client_payload.merge_mode || inputs.merge_mode || vars.PR_MERGE_MODE || 'direct_or_auto' }} + UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.update_branches != false) || inputs.update_branches == true }} STALE_OPENCODE_MINUTES: ${{ github.event.client_payload.stale_opencode_minutes || inputs.stale_opencode_minutes || vars.STALE_OPENCODE_MINUTES || '90' }} steps: - name: Exchange OpenCode app token for scheduler mutations @@ -233,13 +235,124 @@ jobs: id: targeted_dispatch env: GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token || github.token }} - TARGET_REPOSITORY_INPUT: ${{ github.event.client_payload.target_repository || '' }} - TARGET_PR_NUMBER: ${{ github.event.client_payload.pr_number || '' }} - TARGET_BASE_BRANCH_INPUT: ${{ github.event.client_payload.base_branch || '' }} + DISPATCH_CLIENT_PAYLOAD_JSON: ${{ toJSON(github.event.client_payload) }} + GITHUB_EVENT_ACTION: ${{ github.event.action || '' }} + TARGET_REPOSITORY_INPUT: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.repository || github.event.client_payload.target_repository || '' }} + TARGET_PR_NUMBER: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.pr_number || github.event.client_payload.pr_number || '' }} + TARGET_BASE_BRANCH_INPUT: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.base_branch || github.event.client_payload.base_branch || '' }} + TARGET_EXPECTED_BASE_BRANCH_INPUT: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.base_branch || '' }} + TARGET_HEAD_SHA_INPUT: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.head_sha || '' }} + TARGET_BASE_SHA_INPUT: ${{ github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.base_sha || '' }} ALLOWED_TARGET_REPOSITORIES: ${{ vars.OPENCODE_REPOSITORY_DISPATCH_TARGETS }} run: | set -euo pipefail + EXPECTED_SCHEMA="cwl.agent-invocation/v2" + export EXPECTED_SCHEMA + python3 - <<'PYTHON' + import hashlib + import hmac + import json + import os + import re + + envelope = json.loads(os.environ["DISPATCH_CLIENT_PAYLOAD_JSON"] or "null") + if envelope is None: + envelope = {} + if not isinstance(envelope, dict): + raise SystemExit("repository dispatch client_payload must be an object") + is_agent_review = ( + os.environ["GITHUB_EVENT_ACTION"] == "merge-scheduler-agent-review-v2" + ) + if not is_agent_review: + if "schema" in envelope or "claim" in envelope: + raise SystemExit( + "versioned scheduler payload cannot use the legacy merge-scheduler event" + ) + raise SystemExit(0) + + envelope_keys = {"schema", "claim", "agent_invocation_key"} + claim_keys = { + "actor", + "agent", + "base_branch", + "base_sha", + "comment_id", + "enable_auto_merge", + "head_sha", + "merge_mode", + "pr_number", + "repository", + "review_dispatch_limit", + "trigger_reviews", + "update_branches", + } + if set(envelope) != envelope_keys: + raise SystemExit("invalid agent-review scheduler envelope fields") + if envelope["schema"] != os.environ["EXPECTED_SCHEMA"]: + raise SystemExit("unsupported agent-review scheduler schema") + invocation_key = envelope["agent_invocation_key"] + if not isinstance(invocation_key, str) or not re.fullmatch( + r"[0-9a-f]{64}", invocation_key + ): + raise SystemExit("invalid agent-review scheduler invocation key") + claim = envelope["claim"] + if not isinstance(claim, dict) or set(claim) != claim_keys: + raise SystemExit("invalid agent-review scheduler claim fields") + + text_fields = { + "actor", + "agent", + "base_branch", + "base_sha", + "head_sha", + "merge_mode", + "repository", + "review_dispatch_limit", + } + if any(not isinstance(claim[field], str) for field in text_fields): + raise SystemExit("agent-review scheduler claim has a non-string text field") + if type(claim["comment_id"]) is not int or claim["comment_id"] < 1: + raise SystemExit("agent-review scheduler claim has an invalid comment id") + if type(claim["pr_number"]) is not int or claim["pr_number"] < 1: + raise SystemExit("agent-review scheduler claim has an invalid PR number") + for field in ("enable_auto_merge", "trigger_reviews", "update_branches"): + if type(claim[field]) is not bool: + raise SystemExit(f"agent-review scheduler claim has an invalid {field} flag") + if not re.fullmatch(r"ContextualWisdomLab/[A-Za-z0-9_.-]+", claim["repository"]): + raise SystemExit("agent-review scheduler claim has an invalid repository") + if not re.fullmatch(r"[0-9a-f]{40}", claim["head_sha"]): + raise SystemExit("agent-review scheduler claim has an invalid head SHA") + if not re.fullmatch(r"[0-9a-f]{40}", claim["base_sha"]): + raise SystemExit("agent-review scheduler claim has an invalid base SHA") + if ( + not re.fullmatch(r"[A-Za-z0-9._/-]+", claim["base_branch"]) + or claim["base_branch"].startswith("-") + ): + raise SystemExit("agent-review scheduler claim has an invalid base branch") + if not re.fullmatch(r"[A-Za-z0-9-]+", claim["actor"]): + raise SystemExit("agent-review scheduler claim has an invalid actor") + expected_policy = { + "agent": "opencode-agent", + "enable_auto_merge": False, + "merge_mode": "disabled", + "review_dispatch_limit": "1", + "trigger_reviews": True, + "update_branches": False, + } + if any(claim[field] != value for field, value in expected_policy.items()): + raise SystemExit("agent-review scheduler claim violates review-only policy") + canonical = json.dumps( + claim, + ensure_ascii=True, + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + expected_key = hashlib.sha256(canonical).hexdigest() + if not hmac.compare_digest(expected_key, invocation_key): + raise SystemExit("agent-review scheduler invocation key mismatch") + PYTHON + if [ -z "$TARGET_REPOSITORY_INPUT" ]; then { printf 'repository=%s\n' "$GITHUB_REPOSITORY" @@ -280,14 +393,16 @@ jobs: live_base_repository="$(jq -r '.base.repo.full_name // empty' <<<"$pull_json")" live_head_repository="$(jq -r '.head.repo.full_name // empty' <<<"$pull_json")" live_base_branch="$(jq -r '.base.ref // empty' <<<"$pull_json")" + live_base_sha="$(jq -r '.base.sha // empty' <<<"$pull_json")" live_head_sha="$(jq -r '.head.sha // empty' <<<"$pull_json")" if [ "$live_number" != "$TARGET_PR_NUMBER" ] || [ "$live_state" != "open" ] || [ "$live_base_repository" != "$TARGET_REPOSITORY_INPUT" ] || [ "$live_head_repository" != "$TARGET_REPOSITORY_INPUT" ] || [ -z "$live_base_branch" ] || + ! [[ "$live_base_sha" =~ ^[0-9a-fA-F]{40}$ ]] || ! [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then - printf '::error::Targeted scheduler dispatch rejected closed, cross-repository, or malformed live PR metadata. target=%s pr=%s state=%s base_repository=%s head_repository=%s base_branch=%s head_sha=%s\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "${live_state:-}" "${live_base_repository:-}" "${live_head_repository:-}" "${live_base_branch:-}" "${live_head_sha:-}" + printf '::error::Targeted scheduler dispatch rejected closed, cross-repository, or malformed live PR metadata. target=%s pr=%s state=%s base_repository=%s head_repository=%s base_branch=%s base_sha=%s head_sha=%s\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "${live_state:-}" "${live_base_repository:-}" "${live_head_repository:-}" "${live_base_branch:-}" "${live_base_sha:-}" "${live_head_sha:-}" exit 1 fi if [ -n "$TARGET_BASE_BRANCH_INPUT" ] && @@ -295,13 +410,27 @@ jobs: printf '::error::Targeted scheduler dispatch base branch does not match the live PR. supplied=%s live=%s\n' "$TARGET_BASE_BRANCH_INPUT" "$live_base_branch" exit 1 fi + if [ "$GITHUB_EVENT_ACTION" = "merge-scheduler-agent-review-v2" ] && + [ "$TARGET_HEAD_SHA_INPUT" != "$live_head_sha" ]; then + printf '::error::Agent-review scheduler dispatch head SHA changed before execution. supplied=%s live=%s\n' "$TARGET_HEAD_SHA_INPUT" "$live_head_sha" + exit 1 + fi + if [ "$GITHUB_EVENT_ACTION" = "merge-scheduler-agent-review-v2" ] && + [ "$TARGET_BASE_SHA_INPUT" != "$live_base_sha" ]; then + printf '::error::Agent-review scheduler dispatch base SHA changed before execution. supplied=%s live=%s\n' "$TARGET_BASE_SHA_INPUT" "$live_base_sha" + exit 1 + fi { printf 'repository=%s\n' "$TARGET_REPOSITORY_INPUT" printf 'base_branch=%s\n' "$live_base_branch" + printf 'base_sha=%s\n' "$live_base_sha" printf 'head_sha=%s\n' "$live_head_sha" + printf 'expected_base_sha=%s\n' "$TARGET_BASE_SHA_INPUT" + printf 'expected_head_sha=%s\n' "$TARGET_HEAD_SHA_INPUT" + printf 'expected_base_branch=%s\n' "$TARGET_EXPECTED_BASE_BRANCH_INPUT" } >>"$GITHUB_OUTPUT" - printf 'Validated exact targeted scheduler dispatch for %s#%s at %s on base %s.\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "$live_head_sha" "$live_base_branch" + printf 'Validated exact targeted scheduler dispatch for %s#%s at %s on base %s@%s.\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "$live_head_sha" "$live_base_branch" "$live_base_sha" - name: Resolve trusted scheduler source ref id: trusted_source @@ -483,14 +612,17 @@ jobs: GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token || github.token }} TARGET_REPOSITORY: ${{ steps.targeted_dispatch.outputs.repository }} TARGET_DEFAULT_BRANCH: ${{ steps.targeted_dispatch.outputs.base_branch }} - SCHEDULER_ACTIONS_TOKEN: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && (secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token) || github.token }} + EXPECTED_HEAD_SHA: ${{ steps.targeted_dispatch.outputs.expected_head_sha || '' }} + EXPECTED_BASE_SHA: ${{ steps.targeted_dispatch.outputs.expected_base_sha || '' }} + EXPECTED_BASE_BRANCH: ${{ steps.targeted_dispatch.outputs.expected_base_branch || '' }} + SCHEDULER_ACTIONS_TOKEN: ${{ github.event_name == 'repository_dispatch' && (github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.repository != '' || github.event.client_payload.target_repository != '') && (secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token) || github.token }} # Same-repository dispatch credential: when this scheduler runs inside # ContextualWisdomLab/.github (the repository the required workflows are # dispatched on), the runner token can dispatch them without any # cross-repository PAT. The scheduler only uses it when # GITHUB_REPOSITORY equals the dispatch repository. SCHEDULER_DISPATCH_TOKEN: ${{ github.token }} - SCHEDULER_READ_TOKEN: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && (secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token) || github.token }} + SCHEDULER_READ_TOKEN: ${{ github.event_name == 'repository_dispatch' && (github.event.action == 'merge-scheduler-agent-review-v2' && github.event.client_payload.claim.repository != '' || github.event.client_payload.target_repository != '') && (secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.scheduler_app_token.outputs.token) || github.token }} SCHEDULER_MUTATION_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || steps.scheduler_app_token.outputs.available == 'true' && 'opencode-app' || 'github-token' }} SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY: ContextualWisdomLab/.github SCHEDULER_ALLOW_CROSS_REPO_REPOSITORY_DISPATCH: ${{ (secrets.PR_REVIEW_MERGE_TOKEN != '' || secrets.OPENCODE_APPROVE_TOKEN != '') && 'true' || 'false' }} @@ -525,6 +657,13 @@ jobs: if [ -n "$PULL_REQUEST_NUMBER" ]; then args+=(--pr-number "$PULL_REQUEST_NUMBER") fi + if [ -n "$EXPECTED_HEAD_SHA" ] || [ -n "$EXPECTED_BASE_SHA" ] || [ -n "$EXPECTED_BASE_BRANCH" ]; then + args+=( + --expected-head-sha "$EXPECTED_HEAD_SHA" + --expected-base-sha "$EXPECTED_BASE_SHA" + --expected-base-branch "$EXPECTED_BASE_BRANCH" + ) + fi if [ "$DRY_RUN" = "true" ]; then args+=(--dry-run) fi diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 03ec23257..456d99fd2 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -69,12 +69,13 @@ on: concurrency: # Include the event name so default-branch repository_dispatch evidence cannot cancel # the required pull_request_target Strix context that branch protection reads. - # PR-number scope keeps the queue on the current HEAD within each event class. + # Repository-dispatch run-id scope prevents an out-of-order stale dispatch from + # cancelling newer valid evidence before live metadata validation can reject it. group: >- strix-${{ github.event_name }}-${{ github.event.client_payload.target_repository || github.event.pull_request.base.repo.full_name || github.repository }}-${{ github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || - github.event_name == 'repository_dispatch' && github.event.client_payload.pr_number != '' && format('pr-{0}', github.event.client_payload.pr_number) || github.ref }} - cancel-in-progress: true + github.event_name == 'repository_dispatch' && github.run_id || github.ref }} + cancel-in-progress: ${{ github.event_name != 'repository_dispatch' }} # Scorecard Token-Permissions (alert #43): keep the workflow-level token # read-only and scope same-repo status publication to the Strix scan job. diff --git a/CHANGELOG.md b/CHANGELOG.md index bf30091dd..8c06d80c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Replaced both oversized 14-property OpenCode mention dispatches with one three-property versioned claim envelope, preserved invocation/ledger identities, and made the review-only scheduler reject malformed policy, closed PRs, or live head/base/target-branch drift; stale snapshot work can neither enter general queue/merge mutations nor cancel newer scheduler, OpenCode, or Strix runs, while the explicit legacy scheduler path remains intact. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/docs/automation/review-agent-comment-invocation.md b/docs/automation/review-agent-comment-invocation.md index 51c84dcde..642445a48 100644 --- a/docs/automation/review-agent-comment-invocation.md +++ b/docs/automation/review-agent-comment-invocation.md @@ -1,6 +1,6 @@ # Review-agent comment invocation -Updated: 2026-08-06 +Updated: 2026-08-08 ## Purpose @@ -9,7 +9,7 @@ Trusted ContextualWisdomLab maintainers can invoke the existing review planes fr - `@cwl-noema-review` requests the independent Noema review. - `@opencode-agent` requests a bounded current-head OpenCode review only; the invocation itself disables branch updates, automatic merge, and direct merge. -The router never checks out or executes pull-request-controlled code. It reads live PR metadata, binds the request to the current head SHA and base branch, and dispatches the already deployed central workflows in `ContextualWisdomLab/.github`. +The router never checks out or executes pull-request-controlled code. It reads live PR metadata, binds the request to the current head SHA, base SHA, and base branch, and dispatches the already deployed central workflows in `ContextualWisdomLab/.github`. ## Architecture @@ -20,7 +20,9 @@ The implementation uses two bounded paths: 1. **Local fast path.** Comments on `ContextualWisdomLab/.github` trigger `issue_comment` immediately. 2. **Organization sweep.** Every five minutes, the central workflow enumerates repositories visible to its cross-repository credential, finds recently updated open PRs and recent comments, validates trusted exact mentions, and consults the central exact-name Actions artifact ledger before queuing work. -Each requested agent receives a deterministic invocation key containing the target repository, PR number, exact head SHA, base branch, requested agent, source comment ID, and requesting actor. Each agent-specific wrapper reconstructs the same canonical JSON from its validated payload, hashes it with SHA-256, and compares the result in constant time with the supplied key. Altering any bound field while retaining a syntactically valid key therefore fails closed. +Each requested agent receives a deterministic invocation key containing the target repository, PR number, exact head SHA, exact base SHA, base branch, requested agent, source comment ID, and requesting actor. Each agent-specific wrapper reconstructs the same canonical JSON from its validated payload, hashes it with SHA-256, and compares the result in constant time with the supplied key. Altering any bound field while retaining a syntactically valid key therefore fails closed. + +OpenCode uses the versioned `cwl.agent-invocation/v2` envelope at both central dispatch hops. Its `client_payload` has exactly three top-level properties: `schema`, the complete canonical `claim`, and `agent_invocation_key`. The wrapper strictly rejects missing, extra, mistyped, policy-changing, or key-mismatched fields, materializes the complete downstream `merge-scheduler-agent-review-v2` request, and verifies GitHub's ten-property and 65,535-character limits before claiming the immutable ledger artifact. The downstream scheduler revalidates the same envelope and compares the live open PR repository, number, head SHA, base SHA, and base branch with the claim. Its CLI then performs two fresh exact-snapshot reads before entering a dedicated review-only path that cannot cancel workflow runs, clean up reviews, update branches, enable or disable auto-merge, or merge. A different-head active run suppresses the stale dispatch, v2 scheduler concurrency is invocation-scoped, and the downstream OpenCode/Strix receivers isolate pre-validation dispatches by run ID, so an out-of-order stale event cannot cancel newer valid work. Schema-free legacy `merge-scheduler` requests remain a separate explicit compatibility path and cannot accept a versioned claim. The exact-name Actions artifact ledger uses `cwl-agent-invocation-` as the artifact name. The router queries GitHub's repository artifact endpoint with the server-side exact `name` filter, validates the complete response, and treats any live exact-name artifact as durable dispatch evidence. This avoids depending on filtered workflow-run enumeration, which GitHub caps at 1,000 results even when pagination is requested. @@ -45,8 +47,8 @@ This preserves the central MSA boundary without copying privileged workflow code - `contents: write` is intentionally retained only on jobs that call GitHub's create-repository-dispatch endpoint. GitHub documents that endpoint as requiring Contents repository permission at write level. Removing it would disable the bounded central dispatch path; broad workflow-default write access is not granted. - The organization sweep uses the established cross-repository credential chain for reading target comments, while the central repository's own short-lived job token dispatches the central workflows. - OpenCode dispatch is restricted to the exact `OPENCODE_REPOSITORY_DISPATCH_TARGETS` allowlist. -- An invocation cannot merge: `enable_auto_merge=false`, `update_branches=false`, and `merge_mode=disabled` are explicit in the dispatch payload. -- Every dispatch is bound to live PR number, current head SHA, base branch, source comment, requested agent, and requesting actor metadata fetched or validated immediately before dispatch. +- An invocation cannot merge: `enable_auto_merge=false`, `update_branches=false`, and `merge_mode=disabled` are explicit in the canonical claim and are forced again by the dedicated scheduler event. +- Every dispatch is bound to live open PR number, current head SHA, current base SHA, base branch, source comment, requested agent, and requesting actor metadata fetched or validated immediately before dispatch. The scheduler CLI re-fetches and repeats all snapshot comparisons immediately before its mutation-free review-only decision path. If another-head run appears after that read, the scheduler suppresses the POST; if drift occurs after the active-run lookup, run-isolated receiver concurrency lets live-metadata validation reject the stale work without cancelling the newer run. - Router jobs use the fixed `ubuntu-24.04` runner and an immutable `actions/checkout` v7.0.1 commit pin; checkout credentials are not persisted. - A branch-selectable `workflow_dispatch` trigger is intentionally absent. This prevents a repository writer from choosing an unreviewed branch version of the central router while the job holds dispatch permissions. @@ -67,7 +69,7 @@ The permanent quality workflow runs the deterministic router, sweep, exact-name The router is inactive until its workflows and helper code are merged into the protected default branch. A materialization, predecessor, cancelled, queued, or stale-head run is not activation evidence. Production activation requires the exact final head to pass the permanent quality workflow, security and supply-chain checks, current-head automated review, an independent approval, unresolved-thread policy, and branch protection without bypass. -Rollback is deletion of the four mention-router workflows, the two Python helpers, and their focused tests. Existing Noema and OpenCode review workflows remain independently invocable and authoritative; the router does not own reviewer identity, credentials, verdict acceptance, approval, merge, or release. +Rollback of the OpenCode v2 transport removes the dedicated `merge-scheduler-agent-review-v2` trigger and its expected-ref CLI guards together with the OpenCode mention wrapper/router path; the schema-free legacy scheduler path remains unchanged. A full router rollback also deletes the mention-router workflows, the two mention Python helpers, and their focused tests. Existing Noema and OpenCode review workflows remain independently invocable and authoritative; the router does not own reviewer identity, credentials, verdict acceptance, approval, merge, or release. ## References diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index bdb8ac3db..81edbf421 100644 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -28,6 +28,7 @@ f"repos/{CENTRAL_AUTOMATION_REPOSITORY}/actions/artifacts" ) LEDGER_ARTIFACT_PREFIX = "cwl-agent-invocation-" +OPENCODE_INVOCATION_SCHEMA = "cwl.agent-invocation/v2" REPOSITORY_RE = re.compile(r"^ContextualWisdomLab/[A-Za-z0-9_.-]+$") HEAD_SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") BASE_BRANCH_RE = re.compile(r"^(?!-)[A-Za-z0-9._/-]+$") @@ -394,24 +395,22 @@ def opencode_payload(request: MentionRequest) -> dict[str, Any]: agent = "opencode-agent" claim = agent_invocation_claim(request, agent) + client_payload = { + "schema": OPENCODE_INVOCATION_SCHEMA, + "claim": claim, + "agent_invocation_key": agent_invocation_key(request, agent), + } + encoded = json.dumps( + client_payload, + ensure_ascii=True, + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + if len(client_payload) > 10 or len(encoded) > 65_535: + raise ValueError("OpenCode repository dispatch exceeds GitHub limits") return { "event_type": "agent-mention-opencode", - "client_payload": { - "target_repository": request.repository, - "pr_number": request.pull_request_number, - "pr_head_sha": request.pull_request_head_sha, - "pr_base_sha": request.pull_request_base_sha, - "base_branch": request.pull_request_base_branch, - "trigger_reviews": claim["trigger_reviews"], - "review_dispatch_limit": claim["review_dispatch_limit"], - "enable_auto_merge": claim["enable_auto_merge"], - "update_branches": claim["update_branches"], - "merge_mode": claim["merge_mode"], - "requested_agent": agent, - "agent_invocation_key": agent_invocation_key(request, agent), - "requested_by": request.actor, - "source_comment_id": request.comment_id, - }, + "client_payload": client_payload, } diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 75e18c860..53b2c5a90 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -22,6 +22,7 @@ PULL_REQUEST_FIELDS_FRAGMENT = """\ fragment SchedulerPullRequestFields on PullRequest { number + state title isDraft mergeable @@ -722,6 +723,7 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: ) return { "number": number, + "state": str(pr.get("state") or "").upper(), "title": pr.get("title"), "isDraft": bool(pr.get("draft")), "mergeable": pr.get("mergeable"), @@ -2069,7 +2071,14 @@ def cancel_stale_opencode_runs(repo: str, workflow: str, pr: dict[str, Any], *, return [run_id for _, run_id in stale_refs] -def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> str: +def dispatch_opencode_review( + repo: str, + workflow: str, + pr: dict[str, Any], + *, + dry_run: bool, + snapshot_guarded: bool = False, +) -> str: """Dispatch trusted OpenCode for the PR head, or report an active run. The review job is intentionally restricted to ``repository_dispatch``. A @@ -2081,7 +2090,14 @@ def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dr if not dry_run: require_github_actions_control_actor("inspect-active-opencode-review") current_run_refs, stale_run_refs = active_opencode_run_refs(repo, workflow, pr) - force_cancel_workflow_run_refs(stale_run_refs) + if snapshot_guarded and stale_run_refs: + print( + "OpenCode review dispatch skipped: a different-head workflow run became active " + "after snapshot validation" + ) + return "snapshot_changed" + if not snapshot_guarded: + force_cancel_workflow_run_refs(stale_run_refs) if current_run_refs: print( "OpenCode review dispatch skipped: active same-head workflow run(s) " @@ -2123,12 +2139,20 @@ def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dr return "dispatched" -def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> str: +def dispatch_strix_evidence( + repo: str, + workflow: str, + pr: dict[str, Any], + *, + dry_run: bool, + snapshot_guarded: bool = False, +) -> str: """Dispatch same-head Strix workflow evidence before OpenCode reviews.""" - job_id = matching_actions_job_id(pr, is_strix_context) - if job_id: - rerun_actions_job(repo, job_id, dry_run=dry_run, action="rerun-strix-evidence") - return "rerun" if not dry_run else "dry_run" + if not snapshot_guarded: + job_id = matching_actions_job_id(pr, is_strix_context) + if job_id: + rerun_actions_job(repo, job_id, dry_run=dry_run, action="rerun-strix-evidence") + return "rerun" if not dry_run else "dry_run" if dry_run: return "dry_run" require_github_actions_control_actor("inspect-active-strix-evidence") @@ -2139,7 +2163,14 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry run_title="Strix Security Scan", workflow_aliases=frozenset({"Strix Security Scan"}), ) - force_cancel_workflow_run_refs(stale_run_refs) + if snapshot_guarded and stale_run_refs: + print( + "Strix evidence dispatch skipped: a different-head workflow run became active " + "after snapshot validation" + ) + return "snapshot_changed" + if not snapshot_guarded: + force_cancel_workflow_run_refs(stale_run_refs) if current_run_refs: print( "Strix evidence dispatch skipped: active same-head workflow run(s) " @@ -2247,6 +2278,214 @@ def current_head_can_attempt_merge(pr: dict[str, Any], merge_state: str) -> bool return False +def inspect_snapshot_bound_review( + repo: str, + pr: dict[str, Any], + *, + dry_run: bool, + trigger_reviews: bool, + review_dispatch_allowed: bool, + workflow: str, + security_workflow: str, + base_branch: str, + stale_opencode_minutes: int, +) -> Decision: + """Dispatch exact-snapshot review work without general scheduler mutations.""" + + number = pr["number"] + base_ref = pr.get("baseRefName") + if str(pr.get("state") or "").upper() != "OPEN": + return Decision(number, "wait", "snapshot-bound target PR is no longer open") + if pr.get("isDraft"): + return Decision(number, "skip", "draft PR") + if base_ref != base_branch: + return Decision( + number, + "wait", + f"snapshot-bound target base branch changed from {base_branch} to {base_ref}", + ) + unresolved = unresolved_thread_count(pr) + if unresolved: + return Decision(number, "block", f"{unresolved} unresolved review thread(s)") + if has_current_head_changes_requested(pr): + return Decision(number, "block", "current-head OpenCode review requested changes") + + merge_state = effective_merge_state(pr) + current_head_approved = has_current_head_approval(pr) + if merge_state in {"DIRTY", "CONFLICTING"}: + return Decision(number, "block", merge_conflict_guidance(pr, merge_state)) + 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])}") + workflow_action_required = action_required_checks(pr) + if workflow_action_required: + return Decision( + number, + "wait", + workflow_action_required_reason(workflow_action_required), + ) + + behind_by = branch_outdated_by_base(pr, merge_state) + if behind_by and trigger_reviews: + return Decision( + number, + "wait", + "current head has no OpenCode approval; snapshot-bound review cannot update an outdated branch", + ) + if merge_state == "UNKNOWN": + return Decision( + number, + "wait", + "mergeability is still being calculated and no branch freshness evidence is available", + ) + if current_head_approved: + return Decision( + number, + "wait", + "current head is approved; snapshot-bound invocation is review-only", + ) + + opencode_state = opencode_progress_state( + pr, + stale_after_minutes=stale_opencode_minutes, + ) + if opencode_state == "running": + return Decision(number, "wait", "OpenCode review is already in progress") + if ( + os.environ.get("GITHUB_EVENT_NAME") == "workflow_run" + and has_current_head_deterministic_fallback_approval(pr) + ): + return Decision( + number, + "wait", + "current-head deterministic fallback is not merge evidence; defer real-model retry to the next scheduler heartbeat", + ) + if opencode_state == "stale" and not trigger_reviews: + return Decision( + number, + "wait", + f"OpenCode review exceeded {stale_opencode_minutes} minute retry threshold; review dispatch disabled", + ) + if opencode_state == "stale": + if not review_dispatch_allowed: + return Decision( + number, + "wait", + f"OpenCode review exceeded {stale_opencode_minutes} minute retry threshold; review dispatch limit reached", + ) + dispatch_result = dispatch_opencode_review( + repo, + workflow, + pr, + dry_run=dry_run, + snapshot_guarded=True, + ) + if dispatch_result == "snapshot_changed": + return Decision( + number, + "wait", + "OpenCode review retry skipped because a different-head review run is active", + ) + if dispatch_result == "already_running": + return Decision( + number, + "wait", + "OpenCode review exceeded the status-check retry threshold, but a same-head workflow run is already active", + ) + return Decision( + number, + "review_dispatch", + f"OpenCode review exceeded {stale_opencode_minutes} minute retry threshold; same-head OpenCode re-dispatched", + ) + + if trigger_reviews: + strix_state = strix_evidence_state(pr) + if strix_state == "missing": + if not review_dispatch_allowed: + return Decision( + number, + "wait", + "current head has no completed Strix evidence; review dispatch limit reached", + ) + wait_reason = repository_dispatch_wait_reason(repo, security_workflow) + if wait_reason: + return Decision( + number, + "wait", + f"current head has no completed Strix evidence; {wait_reason}", + ) + dispatch_result = dispatch_strix_evidence( + repo, + security_workflow, + pr, + dry_run=dry_run, + snapshot_guarded=True, + ) + if dispatch_result == "snapshot_changed": + return Decision( + number, + "wait", + "Strix dispatch skipped because a different-head Strix run is active", + ) + if dispatch_result == "already_running": + return Decision( + number, + "wait", + "same-head Strix evidence workflow run is already active", + ) + return Decision( + number, + "security_dispatch", + "current head has no completed Strix evidence; same-head Strix dispatched", + ) + if strix_state == "running": + return Decision(number, "wait", "same-head Strix evidence is still running") + if not review_dispatch_allowed: + return Decision( + number, + "wait", + "current head has completed Strix evidence; review dispatch limit reached", + ) + wait_reason = repository_dispatch_wait_reason(repo, workflow) + if wait_reason: + return Decision( + number, + "wait", + f"current head has completed Strix evidence; {wait_reason}", + ) + dispatch_result = dispatch_opencode_review( + repo, + workflow, + pr, + dry_run=dry_run, + snapshot_guarded=True, + ) + if dispatch_result == "snapshot_changed": + return Decision( + number, + "wait", + "OpenCode dispatch skipped because a different-head review run is active", + ) + if dispatch_result == "already_running": + return Decision( + number, + "wait", + "current head has completed Strix evidence; same-head OpenCode workflow run is already active", + ) + return Decision( + number, + "review_dispatch", + "current head has completed Strix evidence; same-head OpenCode dispatched", + ) + + return Decision( + number, + "block", + "current head has no OpenCode approval; review dispatch disabled", + ) + + def inspect_pr( repo: str, pr: dict[str, Any], @@ -2263,8 +2502,21 @@ def inspect_pr( base_branch: str, merge_mode: str = "direct_or_auto", stale_opencode_minutes: int = DEFAULT_STALE_OPENCODE_MINUTES, + snapshot_guarded: bool = False, ) -> Decision: """Decide and optionally act on one pull request's merge-readiness state.""" + if snapshot_guarded: + return inspect_snapshot_bound_review( + repo, + pr, + dry_run=dry_run, + trigger_reviews=trigger_reviews, + review_dispatch_allowed=review_dispatch_allowed, + workflow=workflow, + security_workflow=security_workflow, + base_branch=base_branch, + stale_opencode_minutes=stale_opencode_minutes, + ) number = pr["number"] base_ref = pr.get("baseRefName") @@ -3691,6 +3943,68 @@ def self_test() -> None: print("self-test passed") +def validate_expected_pr_snapshot( + prs: Sequence[dict[str, Any]], + *, + pr_number: int, + expected_head_sha: str, + expected_base_sha: str, + expected_base_branch: str, +) -> None: + """Fail closed when a targeted PR no longer matches its validated refs.""" + + expected_values = ( + expected_head_sha, + expected_base_sha, + expected_base_branch, + ) + if not any(expected_values): + return + if not all(expected_values): + raise SystemExit( + "--expected-head-sha, --expected-base-sha, and --expected-base-branch " + "must be supplied together" + ) + if pr_number < 1: + raise SystemExit("expected PR snapshot guards require --pr-number") + try: + expected_head = validate_git_sha(expected_head_sha).lower() + expected_base = validate_git_sha(expected_base_sha).lower() + expected_branch = validate_git_ref(expected_base_branch) + except ValueError as exc: + raise SystemExit(str(exc)) from exc + if len(prs) != 1 or int(prs[0].get("number") or 0) != pr_number: + raise SystemExit( + f"target PR #{pr_number} snapshot is unavailable or ambiguous" + ) + try: + observed_head = validate_git_sha(str(prs[0].get("headRefOid") or "")).lower() + observed_base = validate_git_sha(str(prs[0].get("baseRefOid") or "")).lower() + observed_branch = validate_git_ref(str(prs[0].get("baseRefName") or "")) + except ValueError as exc: + raise SystemExit(f"target PR #{pr_number} returned malformed refs: {exc}") from exc + observed_state = str(prs[0].get("state") or "").upper() + if observed_state != "OPEN": + raise SystemExit( + f"target PR #{pr_number} is no longer open: observed {observed_state or ''}" + ) + if observed_head != expected_head: + raise SystemExit( + f"target PR #{pr_number} head SHA changed: " + f"expected {expected_head}, observed {observed_head}" + ) + if observed_base != expected_base: + raise SystemExit( + f"target PR #{pr_number} base SHA changed: " + f"expected {expected_base}, observed {observed_base}" + ) + if observed_branch != expected_branch: + raise SystemExit( + f"target PR #{pr_number} base branch changed: " + f"expected {expected_branch}, observed {observed_branch}" + ) + + def parse_args(argv: list[str]) -> argparse.Namespace: """Parse scheduler CLI arguments.""" parser = argparse.ArgumentParser() @@ -3699,6 +4013,18 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser.add_argument("--project-flow", default=os.environ.get("PROJECT_FLOW", "")) parser.add_argument("--max-prs", type=int, default=100) parser.add_argument("--pr-number", type=int, default=0) + parser.add_argument( + "--expected-head-sha", + default=os.environ.get("EXPECTED_HEAD_SHA", ""), + ) + parser.add_argument( + "--expected-base-sha", + default=os.environ.get("EXPECTED_BASE_SHA", ""), + ) + parser.add_argument( + "--expected-base-branch", + default=os.environ.get("EXPECTED_BASE_BRANCH", ""), + ) parser.add_argument("--dry-run", action="store_true") parser.add_argument("--trigger-reviews", action=argparse.BooleanOptionalAction, default=True) parser.add_argument( @@ -3750,6 +4076,27 @@ def main(argv: list[str]) -> int: if args.branch_update_limit < -1: raise SystemExit("--branch-update-limit must be -1 or greater") prs = fetch_pr(args.repo, args.pr_number) if args.pr_number else fetch_open_prs(args.repo, args.max_prs) + validate_expected_pr_snapshot( + prs, + pr_number=args.pr_number, + expected_head_sha=args.expected_head_sha, + expected_base_sha=args.expected_base_sha, + expected_base_branch=args.expected_base_branch, + ) + snapshot_guarded = bool( + args.expected_head_sha + or args.expected_base_sha + or args.expected_base_branch + ) + if snapshot_guarded: + prs = fetch_pr(args.repo, args.pr_number) + validate_expected_pr_snapshot( + prs, + pr_number=args.pr_number, + expected_head_sha=args.expected_head_sha, + expected_base_sha=args.expected_base_sha, + expected_base_branch=args.expected_base_branch, + ) decisions = [] review_dispatches_used = 0 branch_updates_used = 0 @@ -3774,6 +4121,7 @@ def main(argv: list[str]) -> int: security_workflow=args.security_workflow, base_branch=args.base_branch, stale_opencode_minutes=args.stale_opencode_minutes, + snapshot_guarded=snapshot_guarded, ) except RuntimeError as exc: decision = Decision( diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 7343c06ac..544edfa5b 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -181,12 +181,12 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" 'strix-${{ github.event_name }}-' "strix workflow isolates manual evidence runs from required PR contexts" assert_file_contains "$workflow_file" "format('pr-{0}', github.event.pull_request.number)" "strix workflow scopes pull_request_target concurrency to the active pull request" assert_file_contains "$workflow_file" "github.event.client_payload.target_repository ||" "strix manual dispatch concurrency scopes to the target repository when provided" - assert_file_contains "$workflow_file" "github.event.client_payload.pr_number != '' && format('pr-{0}', github.event.client_payload.pr_number)" "strix workflow retains a manual PR fallback group when no head SHA is provided" + assert_file_contains "$workflow_file" "github.event_name == 'repository_dispatch' && github.run_id" "strix gives every pre-validation repository dispatch an isolated queue group" assert_file_contains "$workflow_file" "github.ref }}" "strix workflow scopes non-PR concurrency to the current ref" assert_file_not_contains "$workflow_file" "format('pr-{0}-{1}'" "strix workflow does not keep stale head-specific concurrency groups" - assert_file_contains "$workflow_file" "cancel-in-progress: true" "strix workflow cancels stale PR evidence runs when a newer PR event arrives" + assert_file_contains "$workflow_file" "cancel-in-progress: \${{ github.event_name != 'repository_dispatch' }}" "strix never lets an unvalidated stale repository dispatch cancel newer evidence" assert_file_contains "$workflow_file" "default-branch repository_dispatch evidence cannot cancel" "strix workflow documents manual evidence isolation from branch protection contexts" - assert_file_contains "$workflow_file" "PR-number scope keeps the queue on the current HEAD" "strix workflow documents current-head queue management" + assert_file_contains "$workflow_file" "run-id scope prevents an out-of-order stale dispatch" "strix workflow documents stale dispatch isolation" assert_file_contains "$workflow_file" "refs/pull//head has already advanced before this queued run starts" "strix workflow documents stale scan queue avoidance" status_token_count="$(grep -c '^[[:space:]]*GITHUB_STATUS_TOKEN:' "$workflow_file")" assert_equals "1" "$status_token_count" "strix workflow defines GITHUB_STATUS_TOKEN once so GitHub can parse repository_dispatch" @@ -513,11 +513,9 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { if awk '/^ required-workflow-bootstrap:$/,/^[^ ]/' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then record_failure "opencode required workflow bootstrap must not depend on required-workflow event payload fields" fi - assert_file_contains "$workflow_file" 'github.event.client_payload.target_repository || github.repository' "opencode review scopes concurrency by target repository" - assert_file_contains "$workflow_file" "format('pr-{0}', github.event.client_payload.pr_number)" "opencode review scopes repository_dispatch concurrency by current PR" - assert_file_not_contains "$workflow_file" "format('pr-{0}-{1}'" "opencode review does not keep stale head-specific concurrency groups" - assert_file_contains "$workflow_file" "github.event.client_payload.pr_number && format('pr-{0}', github.event.client_payload.pr_number)" "opencode review retains a manual PR fallback group when no head SHA is provided" - assert_file_contains "$workflow_file" 'cancel-in-progress: true' "opencode review cancels stale in-progress review attempts when a newer PR event arrives" + assert_file_contains "$workflow_file" 'group: opencode-review-repository-dispatch-${{ github.run_id }}' "opencode isolates every dispatch until live snapshot validation" + assert_file_contains "$workflow_file" 'cancel-in-progress: false' "opencode prevents stale pre-validation dispatches from cancelling newer valid work" + assert_file_contains "$workflow_file" "Run-id scope" "opencode documents stale dispatch isolation" assert_file_contains "$workflow_file" "Materialize pull request merge tree for coverage measurement" "opencode pull_request coverage execution materializes the exact base/head merge tree" assert_file_contains "$workflow_file" "stale OpenCode run: event head=" "opencode review side effects are skipped for stale heads" assert_file_not_contains "$workflow_file" "github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name" "opencode never treats a same-repository pull_request_target head as authorization to execute PR-controlled code" @@ -1505,6 +1503,8 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" "github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number)" "scheduler scopes pull_request_target concurrency to the active PR" assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number && format('pr-{0}', github.event.workflow_run.pull_requests[0].number)" "scheduler scopes workflow_run concurrency to the completed review PR" assert_file_contains "$workflow_file" "github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule)" "scheduler isolates the 15-minute organization sweep from the separate 30-minute scheduled scan" + assert_file_contains "$workflow_file" "github.event.client_payload.agent_invocation_key != '' && format('agent-review-{0}', github.event.client_payload.agent_invocation_key)" "scheduler isolates each validated agent-review invocation before live snapshot validation" + assert_file_contains "$workflow_file" "github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.target_repository" "scheduler retains target-PR cancellation only for legacy dispatches" assert_file_contains "$workflow_file" "github.event_name == 'repository_dispatch' && github.run_id" "scheduler keeps manual queue scans isolated per run" assert_file_contains "$workflow_file" "cancel-in-progress: \${{ github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || github.event_name == 'repository_dispatch' }}" "scheduler cancels stale PR/review/manual queue scans instead of accumulating merge/update attempts" assert_file_contains "$workflow_file" "timeout-minutes: 60" "organization sweep has enough headroom to finish the complete repository walk" @@ -1516,7 +1516,7 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' || github.event_name == 'push'" "scheduler can dispatch a bounded follow-up OpenCode review after review workflow completion" assert_file_contains "$workflow_file" "github.event_name == 'push' || github.event_name == 'pull_request_target'" "scheduler treats base-branch pushes as queue-maintenance events" assert_file_contains "$workflow_file" "github.event.client_payload.enable_auto_merge != false" "scheduler enables auto-merge by default for default-branch dispatch events" - assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false) || inputs.update_branches == true" "scheduler enables branch updates after review completion or an explicit default-branch dispatch" + assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' || (github.event_name == 'repository_dispatch' && github.event.action != 'merge-scheduler-agent-review-v2' && github.event.client_payload.update_branches != false) || inputs.update_branches == true" "scheduler enables branch updates after review completion or an explicit legacy default-branch dispatch while keeping agent review v2 disabled" assert_file_contains "$workflow_file" "review_dispatch_limit:" "scheduler exposes a bounded review dispatch budget" assert_file_contains "$workflow_file" "REVIEW_DISPATCH_LIMIT_INPUT" "scheduler forwards the review dispatch budget to the canonical script" assert_file_contains "$workflow_file" 'review_dispatch_limit="-1"' "scheduler dispatches every eligible same-head review or Strix evidence job immediately unless an explicit budget overrides it" diff --git a/tests/test_agent_mention_complete_payload_binding.py b/tests/test_agent_mention_complete_payload_binding.py index 04562e93f..bfb0ff4f3 100644 --- a/tests/test_agent_mention_complete_payload_binding.py +++ b/tests/test_agent_mention_complete_payload_binding.py @@ -74,12 +74,13 @@ def test_event_and_payloads_bind_exact_base_identity() -> None: assert request.pull_request_base_branch == "main" assert request.pull_request_base_sha == "b" * 40 - for payload in ( - router.noema_payload(request)["client_payload"], - router.opencode_payload(request)["client_payload"], - ): - assert payload["base_branch"] == "main" - assert payload["pr_base_sha"] == "b" * 40 + noema_payload = router.noema_payload(request)["client_payload"] + assert noema_payload["base_branch"] == "main" + assert noema_payload["pr_base_sha"] == "b" * 40 + opencode_envelope = router.opencode_payload(request)["client_payload"] + assert opencode_envelope["schema"] == "cwl.agent-invocation/v2" + assert opencode_envelope["claim"]["base_branch"] == "main" + assert opencode_envelope["claim"]["base_sha"] == "b" * 40 malformed = _event() malformed["pull_request"]["base"]["sha"] = "not-a-sha" @@ -150,36 +151,37 @@ def test_wrappers_recompute_complete_claim_before_ledger_access() -> None: noema = NOEMA_WORKFLOW.read_text(encoding="utf-8") opencode = OPENCODE_WORKFLOW.read_text(encoding="utf-8") + assert "PR_BASE_SHA:" in noema + assert "github.event.client_payload.pr_base_sha" in noema + assert '! [[ "$PR_BASE_SHA" =~ ^[0-9a-f]{40}$ ]]' in noema + assert '"base_sha": os.environ["PR_BASE_SHA"]' in noema + assert "--arg pr_base_sha \"$PR_BASE_SHA\"" in noema + assert "pr_base_sha: $pr_base_sha" in noema + assert noema.count('"base_sha": os.environ["PR_BASE_SHA"]') >= 2 + + assert "CLIENT_PAYLOAD_JSON:" in opencode + assert "github.event.client_payload.claim" in opencode + assert '"base_sha"' in opencode + assert 'r"[0-9a-f]{40}", claim["base_sha"]' in opencode + assert "set(envelope)" in opencode + assert "set(claim)" in opencode + assert '"client_payload": envelope' in opencode + assert '--input "$SCHEDULER_REQUEST_FILE"' in opencode for workflow in (noema, opencode): - assert "PR_BASE_SHA:" in workflow - assert "github.event.client_payload.pr_base_sha" in workflow - assert '! [[ "$PR_BASE_SHA" =~ ^[0-9a-f]{40}$ ]]' in workflow - assert '"base_sha": os.environ["PR_BASE_SHA"]' in workflow assert "hmac.compare_digest" in workflow assert workflow.index("Validate exact invocation payload") < workflow.index( "Inspect exact-name Actions artifact ledger" ) - assert "--arg pr_base_sha \"$PR_BASE_SHA\"" in workflow - assert "pr_base_sha: $pr_base_sha" in workflow - - for field in ( - '"trigger_reviews": os.environ["TRIGGER_REVIEWS"] == "true"', - '"review_dispatch_limit": os.environ["REVIEW_DISPATCH_LIMIT"]', - '"enable_auto_merge": os.environ["ENABLE_AUTO_MERGE"] == "true"', - '"update_branches": os.environ["UPDATE_BRANCHES"] == "true"', - '"merge_mode": os.environ["MERGE_MODE"]', - ): - assert field in opencode - assert noema.count('"base_sha": os.environ["PR_BASE_SHA"]') >= 2 - for field in ( - '"trigger_reviews": os.environ["TRIGGER_REVIEWS"] == "true"', - '"review_dispatch_limit": os.environ["REVIEW_DISPATCH_LIMIT"]', - '"enable_auto_merge": os.environ["ENABLE_AUTO_MERGE"] == "true"', - '"update_branches": os.environ["UPDATE_BRANCHES"] == "true"', - '"merge_mode": os.environ["MERGE_MODE"]', + for field, value in ( + ('"agent"', '"opencode-agent"'), + ('"trigger_reviews"', "True"), + ('"review_dispatch_limit"', '"1"'), + ('"enable_auto_merge"', "False"), + ('"update_branches"', "False"), + ('"merge_mode"', '"disabled"'), ): - assert opencode.count(field) >= 2 + assert f"{field}: {value}" in opencode def test_no_pr_specific_writer_workflow_remains() -> None: diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index 4fc40a782..9be43ebc3 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -30,12 +30,9 @@ def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None: for text in (noema, opencode): assert "github.event.client_payload.agent_invocation_key" in text assert "cwl-agent-invocation:" in text - assert "source_comment_id" in text - assert "requested_agent" in text assert "cancel-in-progress: false" in text assert "queue: max" in text assert "^[0-9a-f]{64}$" in text - assert "^[1-9][0-9]*$" in text assert "actions/artifacts" in text assert "name=${LEDGER_ARTIFACT_NAME}" in text assert f"actions/upload-artifact@{UPLOAD_ARTIFACT_SHA}" in text @@ -49,11 +46,17 @@ def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None: assert "types: [agent-mention-noema]" in noema assert 'event_type: "noema-review"' in noema assert 'REQUESTED_AGENT: "cwl-noema-review"' in noema + assert "^[1-9][0-9]*$" in noema + assert "source_comment_id" in noema + assert "requested_agent" in noema assert "types: [agent-mention-opencode]" in opencode - assert 'event_type: "merge-scheduler"' in opencode - assert 'REQUESTED_AGENT: "opencode-agent"' in opencode - assert '[[ "$BASE_BRANCH" =~ ^[A-Za-z0-9._/-]+$ ]]' in opencode - assert '[[ "$BASE_BRANCH" == -* ]]' in opencode + assert '"event_type": "merge-scheduler-agent-review-v2"' in opencode + assert '"agent": "opencode-agent"' in opencode + assert '"comment_id"' in opencode + assert 'type(claim["comment_id"]) is not int' in opencode + assert 'type(claim["pr_number"]) is not int' in opencode + assert 're.fullmatch(r"[A-Za-z0-9._/-]+", claim["base_branch"])' in opencode + assert 'claim["base_branch"].startswith("-")' in opencode def test_wrappers_recompute_the_router_canonical_payload_digest() -> None: @@ -74,11 +77,11 @@ def test_wrappers_recompute_the_router_canonical_payload_digest() -> None: '"pr_number"', '"repository"', ) - for text in ( - NOEMA_WORKFLOW.read_text(encoding="utf-8"), - OPENCODE_WORKFLOW.read_text(encoding="utf-8"), - ): - assert "BASE_BRANCH:" in text + noema = NOEMA_WORKFLOW.read_text(encoding="utf-8") + opencode = OPENCODE_WORKFLOW.read_text(encoding="utf-8") + assert "BASE_BRANCH:" in noema + assert '"BASE_BRANCH": claim["base_branch"]' in opencode + for text in (noema, opencode): assert "import hashlib" in text assert "import hmac" in text assert "json.dumps(" in text diff --git a/tests/test_agent_mention_idempotency.py b/tests/test_agent_mention_idempotency.py index 499730a22..1b7959203 100644 --- a/tests/test_agent_mention_idempotency.py +++ b/tests/test_agent_mention_idempotency.py @@ -37,6 +37,7 @@ def request(module: ModuleType): 12345, "maintainer", ("cwl-noema-review", "opencode-agent"), + "b" * 40, ) @@ -194,15 +195,20 @@ def test_payloads_carry_exact_agent_invocation_identity() -> None: assert noema["agent_invocation_key"] == module.agent_invocation_key( mention_request, "cwl-noema-review" ) - assert opencode["requested_agent"] == "opencode-agent" + assert opencode["schema"] == "cwl.agent-invocation/v2" + assert opencode["claim"]["agent"] == "opencode-agent" assert opencode["agent_invocation_key"] == module.agent_invocation_key( mention_request, "opencode-agent" ) - for payload in (noema, opencode): - assert payload["target_repository"] == mention_request.repository - assert payload["pr_number"] == mention_request.pull_request_number - assert payload["pr_head_sha"] == mention_request.pull_request_head_sha - assert payload["source_comment_id"] == mention_request.comment_id + assert noema["target_repository"] == mention_request.repository + assert noema["pr_number"] == mention_request.pull_request_number + assert noema["pr_head_sha"] == mention_request.pull_request_head_sha + assert noema["source_comment_id"] == mention_request.comment_id + assert opencode["claim"]["repository"] == mention_request.repository + assert opencode["claim"]["pr_number"] == mention_request.pull_request_number + assert opencode["claim"]["head_sha"] == mention_request.pull_request_head_sha + assert opencode["claim"]["base_sha"] == mention_request.pull_request_base_sha + assert opencode["claim"]["comment_id"] == mention_request.comment_id def test_existing_artifacts_are_per_agent_durable_evidence() -> None: diff --git a/tests/test_agent_mention_repository_dispatch_envelope.py b/tests/test_agent_mention_repository_dispatch_envelope.py new file mode 100644 index 000000000..8b5f2b3d4 --- /dev/null +++ b/tests/test_agent_mention_repository_dispatch_envelope.py @@ -0,0 +1,343 @@ +"""Contracts for the bounded OpenCode repository-dispatch envelope.""" + +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import sys +import textwrap +from copy import deepcopy +from dataclasses import replace +from pathlib import Path +from types import ModuleType + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +ROUTER_PATH = ROOT / "scripts" / "ci" / "agent_mention_router.py" +WRAPPER_WORKFLOW = ROOT / ".github" / "workflows" / "agent-mention-opencode-dispatch.yml" +SCHEDULER_WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" +QUALITY_WORKFLOW = ROOT / ".github" / "workflows" / "agent-mention-router-quality-ci.yml" +SCHEMA = "cwl.agent-invocation/v2" +ENVELOPE_KEYS = {"schema", "claim", "agent_invocation_key"} + + +def _load_router() -> ModuleType: + """Load the mention router from the repository under test.""" + + module_name = "agent_mention_repository_dispatch_envelope" + spec = importlib.util.spec_from_file_location(module_name, ROUTER_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + sys.modules[module_name] = module + spec.loader.exec_module(module) + return module + + +def _request(router: ModuleType): + """Return one complete OpenCode mention request.""" + + return router.MentionRequest( + repository="ContextualWisdomLab/example", + pull_request_number=17, + pull_request_head_sha="a" * 40, + pull_request_base_branch="main", + comment_id=91, + actor="maintainer", + agents=("opencode-agent",), + pull_request_base_sha="b" * 40, + ) + + +def _named_step(workflow: str, name: str) -> str: + """Return one exact named workflow step.""" + + marker = f" - name: {name}\n" + start = workflow.index(marker) + try: + end = workflow.index("\n - name:", start + len(marker)) + except ValueError: + end = len(workflow) + return workflow[start:end] + + +def _python_heredoc(step: str) -> str: + """Return executable Python from one workflow heredoc.""" + + marker = "python3 - <<'PYTHON'\n" + start = step.index(marker) + len(marker) + end = step.index("\n PYTHON", start) + return textwrap.dedent(step[start:end]) + + +def _run_python_contract(code: str, environment: dict[str, str]) -> subprocess.CompletedProcess: + """Execute one extracted workflow validator with an isolated environment.""" + + return subprocess.run( + [sys.executable, "-c", code], + text=True, + capture_output=True, + check=False, + env={**os.environ, **environment}, + ) + + +def test_router_emits_three_key_versioned_envelope_without_changing_claim_key() -> None: + """Keep the transport bounded while preserving existing ledger identities.""" + + router = _load_router() + request = _request(router) + body = router.opencode_payload(request) + payload = body["client_payload"] + + assert body["event_type"] == "agent-mention-opencode" + assert set(payload) == ENVELOPE_KEYS + assert len(payload) == 3 + assert payload["schema"] == SCHEMA + assert payload["claim"] == router.agent_invocation_claim( + request, "opencode-agent" + ) + assert payload["agent_invocation_key"] == ( + "8c73b6aa8ca5ef7b610b997e6913b71bfad29e74330836263087617fb3d0b9ff" + ) + assert router.agent_ledger_artifact_name(request, "opencode-agent") == ( + "cwl-agent-invocation-" + "8c73b6aa8ca5ef7b610b997e6913b71bfad29e74330836263087617fb3d0b9ff" + ) + assert len(json.dumps(payload, separators=(",", ":"))) <= 65_535 + + +def test_router_rejects_an_oversized_first_hop_before_calling_github() -> None: + """The producer enforces GitHub's size contract before any API request.""" + + router = _load_router() + oversized = replace(_request(router), actor="a" * 70_000) + + with pytest.raises(ValueError, match="repository dispatch exceeds GitHub limits"): + router.opencode_payload(oversized) + + +def test_wrapper_validates_and_reuses_the_same_bounded_envelope_before_ledger() -> None: + """Validate the complete second hop before claiming an immutable artifact.""" + + workflow = WRAPPER_WORKFLOW.read_text(encoding="utf-8") + validate = _named_step( + workflow, + "Validate exact invocation payload and prepare scheduler request", + ) + forward = _named_step( + workflow, + "Forward once to the authoritative review-only scheduler", + ) + + assert "github.event.client_payload.claim.repository" in workflow + assert "github.event.client_payload.claim.pr_number" in workflow + assert f'PAYLOAD_SCHEMA: ${{{{ github.event.client_payload.schema || \'\' }}}}' in workflow + assert "set(envelope)" in validate + for key in sorted(ENVELOPE_KEYS): + assert f'"{key}"' in validate + assert "set(claim)" in validate + assert "hmac.compare_digest" in validate + assert "65_535" in validate + assert '"event_type": "merge-scheduler-agent-review-v2"' in validate + assert '"client_payload": envelope' in validate + assert workflow.index( + "Validate exact invocation payload and prepare scheduler request" + ) < workflow.index("Inspect exact-name Actions artifact ledger") + assert '--input "$SCHEDULER_REQUEST_FILE"' in forward + assert "client_payload:" not in forward + + +def test_wrapper_executes_the_validated_envelope_as_the_exact_second_hop( + tmp_path: Path, +) -> None: + """The materialized scheduler request reuses the exact three-key payload.""" + + router = _load_router() + payload = router.opencode_payload(_request(router))["client_payload"] + workflow = WRAPPER_WORKFLOW.read_text(encoding="utf-8") + code = _python_heredoc( + _named_step( + workflow, + "Validate exact invocation payload and prepare scheduler request", + ) + ) + environment_file = tmp_path / "github-env" + completed = _run_python_contract( + code, + { + "CLIENT_PAYLOAD_JSON": json.dumps(payload), + "GITHUB_ENV": str(environment_file), + "INVOCATION_KEY": payload["agent_invocation_key"], + "PAYLOAD_SCHEMA": payload["schema"], + "RUNNER_TEMP": str(tmp_path), + }, + ) + + assert completed.returncode == 0, completed.stderr + request_path = tmp_path / "agent-review-scheduler-request.json" + request = json.loads(request_path.read_text(encoding="utf-8")) + assert request == { + "event_type": "merge-scheduler-agent-review-v2", + "client_payload": payload, + } + assert len(request["client_payload"]) == 3 + assert request_path.stat().st_size <= 65_535 + assert f"SCHEDULER_REQUEST_FILE={request_path}" in environment_file.read_text( + encoding="utf-8" + ) + + +@pytest.mark.parametrize( + "mutation", + [ + "extra-envelope-field", + "missing-claim-field", + "wrong-boolean-type", + "altered-bound-field", + "unsupported-schema", + ], +) +def test_wrapper_rejects_malformed_or_unbound_envelopes_before_materialization( + tmp_path: Path, + mutation: str, +) -> None: + """Unknown, malformed, or key-mismatched claims fail before ledger access.""" + + router = _load_router() + payload = deepcopy(router.opencode_payload(_request(router))["client_payload"]) + if mutation == "extra-envelope-field": + payload["extra"] = "rejected" + elif mutation == "missing-claim-field": + del payload["claim"]["base_sha"] + elif mutation == "wrong-boolean-type": + payload["claim"]["enable_auto_merge"] = "false" + elif mutation == "altered-bound-field": + payload["claim"]["head_sha"] = "c" * 40 + elif mutation == "unsupported-schema": + payload["schema"] = "cwl.agent-invocation/v3" + else: # pragma: no cover - the parameter list is exhaustive + raise AssertionError(mutation) + + workflow = WRAPPER_WORKFLOW.read_text(encoding="utf-8") + code = _python_heredoc( + _named_step( + workflow, + "Validate exact invocation payload and prepare scheduler request", + ) + ) + completed = _run_python_contract( + code, + { + "CLIENT_PAYLOAD_JSON": json.dumps(payload), + "GITHUB_ENV": str(tmp_path / "github-env"), + "INVOCATION_KEY": payload["agent_invocation_key"], + "PAYLOAD_SCHEMA": payload["schema"], + "RUNNER_TEMP": str(tmp_path), + }, + ) + + assert completed.returncode != 0 + assert not (tmp_path / "agent-review-scheduler-request.json").exists() + + +def test_scheduler_has_strict_v2_and_explicit_legacy_dispatch_paths() -> None: + """The dedicated review event must not fall through generic flat defaults.""" + + workflow = SCHEDULER_WORKFLOW.read_text(encoding="utf-8") + targeted = _named_step(workflow, "Validate targeted repository dispatch") + inspect = _named_step(workflow, "Inspect PR review and merge queue") + + assert "types: [merge-scheduler, merge-scheduler-agent-review-v2]" in workflow + assert "github.event.client_payload.claim.repository" in workflow + assert "github.event.client_payload.claim.pr_number" in workflow + assert ( + 'os.environ["GITHUB_EVENT_ACTION"] == "merge-scheduler-agent-review-v2"' + in targeted + ) + assert f'EXPECTED_SCHEMA="{SCHEMA}"' in targeted + assert "set(envelope)" in targeted + assert "set(claim)" in targeted + assert "hmac.compare_digest" in targeted + assert 'live_base_sha="$(jq -r \'.base.sha // empty\'' in targeted + assert '"$TARGET_HEAD_SHA_INPUT" != "$live_head_sha"' in targeted + assert '"$TARGET_BASE_SHA_INPUT" != "$live_base_sha"' in targeted + assert "--expected-head-sha" in inspect + assert "--expected-base-sha" in inspect + assert "--expected-base-branch" in inspect + concurrency = workflow.split("concurrency:", 1)[1].split("jobs:", 1)[0] + assert "github.event.client_payload.agent_invocation_key" in concurrency + assert "github.event.action != 'merge-scheduler-agent-review-v2'" in concurrency + assert "github.run_id" in concurrency + + +def test_scheduler_executes_strict_v2_validation_and_keeps_legacy_explicit() -> None: + """Only a valid v2 envelope or a schema-free legacy payload is accepted.""" + + router = _load_router() + payload = router.opencode_payload(_request(router))["client_payload"] + workflow = SCHEDULER_WORKFLOW.read_text(encoding="utf-8") + code = _python_heredoc( + _named_step(workflow, "Validate targeted repository dispatch") + ) + common = {"EXPECTED_SCHEMA": SCHEMA} + + valid = _run_python_contract( + code, + { + **common, + "DISPATCH_CLIENT_PAYLOAD_JSON": json.dumps(payload), + "GITHUB_EVENT_ACTION": "merge-scheduler-agent-review-v2", + }, + ) + assert valid.returncode == 0, valid.stderr + + legacy = _run_python_contract( + code, + { + **common, + "DISPATCH_CLIENT_PAYLOAD_JSON": json.dumps( + {"target_repository": "ContextualWisdomLab/example", "pr_number": 17} + ), + "GITHUB_EVENT_ACTION": "merge-scheduler", + }, + ) + assert legacy.returncode == 0, legacy.stderr + + wrong_event = _run_python_contract( + code, + { + **common, + "DISPATCH_CLIENT_PAYLOAD_JSON": json.dumps(payload), + "GITHUB_EVENT_ACTION": "merge-scheduler", + }, + ) + assert wrong_event.returncode != 0 + + malformed = deepcopy(payload) + malformed["claim"]["update_branches"] = True + invalid_policy = _run_python_contract( + code, + { + **common, + "DISPATCH_CLIENT_PAYLOAD_JSON": json.dumps(malformed), + "GITHUB_EVENT_ACTION": "merge-scheduler-agent-review-v2", + }, + ) + assert invalid_policy.returncode != 0 + + +def test_agent_mention_quality_gate_covers_the_downstream_scheduler_contract() -> None: + """Every production and regression path in this transport runs its quality gate.""" + + workflow = QUALITY_WORKFLOW.read_text(encoding="utf-8") + + for path in ( + '.github/workflows/pr-review-merge-scheduler.yml', + 'scripts/ci/pr_review_merge_scheduler.py', + 'tests/test_pr_review_merge_scheduler.py', + ): + assert workflow.count(f' - "{path}"') == 2 diff --git a/tests/test_agent_mention_router.py b/tests/test_agent_mention_router.py index 4509d43f0..1520a204d 100644 --- a/tests/test_agent_mention_router.py +++ b/tests/test_agent_mention_router.py @@ -220,11 +220,13 @@ def test_eligible_agents_and_payloads() -> None: assert noema["client_payload"]["pr_base_sha"] == "b" * 40 opencode = module.opencode_payload(request) assert opencode["event_type"] == "agent-mention-opencode" - assert opencode["client_payload"]["base_branch"] == "develop" - assert opencode["client_payload"]["pr_base_sha"] == "b" * 40 - assert opencode["client_payload"]["merge_mode"] == "disabled" - assert opencode["client_payload"]["enable_auto_merge"] is False - assert opencode["client_payload"]["update_branches"] is False + assert opencode["client_payload"]["schema"] == "cwl.agent-invocation/v2" + claim = opencode["client_payload"]["claim"] + assert claim["base_branch"] == "develop" + assert claim["base_sha"] == "b" * 40 + assert claim["merge_mode"] == "disabled" + assert claim["enable_auto_merge"] is False + assert claim["update_branches"] is False def test_dispatch_uses_central_events_and_acknowledges() -> None: diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index daeaa37a2..f578a2fed 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -1270,18 +1270,11 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): concurrency_contract = workflow.split("concurrency:", 1)[1].split( "permissions:", 1 )[0] - assert ( - "format('pr-{0}', github.event.client_payload.pr_number)" - in concurrency_contract - ) - assert "format('pr-{0}-{1}'" not in concurrency_contract - assert "github.event.client_payload.pr_head_sha" not in concurrency_contract assert "opencode-review-repository-dispatch-" in concurrency_contract + assert "github.run_id" in concurrency_contract + assert "github.event.client_payload.pr_number" not in concurrency_contract + assert "cancel-in-progress: false" in concurrency_contract assert "github.event.pull_request" not in concurrency_contract - assert ( - "github.event.client_payload.pr_number && format('pr-{0}', github.event.client_payload.pr_number)" - in workflow - ) assert "OPENCODE_MODEL_CANDIDATES" in workflow model_pool_runner = Path("scripts/ci/run_opencode_review_model_pool.sh").read_text( encoding="utf-8" @@ -1954,7 +1947,9 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): assert "steps.scheduler_app_token.outputs.token" in workflow assert ( "SCHEDULER_READ_TOKEN: ${{ github.event_name == 'repository_dispatch' " - "&& github.event.client_payload.target_repository != '' && " + "&& (github.event.action == 'merge-scheduler-agent-review-v2' && " + "github.event.client_payload.claim.repository != '' || " + "github.event.client_payload.target_repository != '') && " "(secrets.PR_REVIEW_MERGE_TOKEN || " "secrets.OPENCODE_APPROVE_TOKEN || " "steps.scheduler_app_token.outputs.token) || github.token }}" diff --git a/tests/test_opencode_workflow_shell_syntax.py b/tests/test_opencode_workflow_shell_syntax.py index ec6edca40..42edb6f89 100644 --- a/tests/test_opencode_workflow_shell_syntax.py +++ b/tests/test_opencode_workflow_shell_syntax.py @@ -1,3 +1,4 @@ +import hashlib import json import os import shutil @@ -181,6 +182,7 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): "state": "open", "base": { "ref": "develop", + "sha": "b" * 40, "repo": {"full_name": "ContextualWisdomLab/naruon"}, }, "head": { @@ -193,6 +195,14 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): **os.environ, "PATH": f"{fake_bin}:{os.environ['PATH']}", "FAKE_PULL_JSON": json.dumps(pull), + "DISPATCH_CLIENT_PAYLOAD_JSON": json.dumps( + { + "target_repository": "ContextualWisdomLab/naruon", + "pr_number": 1179, + "base_branch": "develop", + } + ), + "GITHUB_EVENT_ACTION": "merge-scheduler", "GITHUB_EVENT_NAME": "repository_dispatch", "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", "GITHUB_OUTPUT": str(output), @@ -200,6 +210,9 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): "TARGET_REPOSITORY_INPUT": "ContextualWisdomLab/naruon", "TARGET_PR_NUMBER": "1179", "TARGET_BASE_BRANCH_INPUT": "develop", + "TARGET_EXPECTED_BASE_BRANCH_INPUT": "", + "TARGET_HEAD_SHA_INPUT": "", + "TARGET_BASE_SHA_INPUT": "", "ALLOWED_TARGET_REPOSITORIES": ( "ContextualWisdomLab/.github, ContextualWisdomLab/naruon" ), @@ -218,7 +231,11 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): assert output.read_text(encoding="utf-8").splitlines() == [ "repository=ContextualWisdomLab/naruon", "base_branch=develop", + f"base_sha={'b' * 40}", "head_sha=4afd4af7ad343660356791873d940aa2846f40c2", + "expected_base_sha=", + "expected_head_sha=", + "expected_base_branch=", ] output.unlink() @@ -263,3 +280,82 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): assert cross_repo.returncode == 1 assert "cross-repository" in cross_repo.stdout assert not output.exists() + + claim = { + "actor": "maintainer", + "agent": "opencode-agent", + "base_branch": "develop", + "base_sha": "b" * 40, + "comment_id": 91, + "enable_auto_merge": False, + "head_sha": "4afd4af7ad343660356791873d940aa2846f40c2", + "merge_mode": "disabled", + "pr_number": 1179, + "repository": "ContextualWisdomLab/naruon", + "review_dispatch_limit": "1", + "trigger_reviews": True, + "update_branches": False, + } + canonical = json.dumps( + claim, + ensure_ascii=True, + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + envelope = { + "schema": "cwl.agent-invocation/v2", + "claim": claim, + "agent_invocation_key": hashlib.sha256(canonical).hexdigest(), + } + v2_env = { + **env, + "DISPATCH_CLIENT_PAYLOAD_JSON": json.dumps(envelope), + "GITHUB_EVENT_ACTION": "merge-scheduler-agent-review-v2", + "TARGET_EXPECTED_BASE_BRANCH_INPUT": "develop", + "TARGET_HEAD_SHA_INPUT": claim["head_sha"], + "TARGET_BASE_SHA_INPUT": claim["base_sha"], + } + accepted_v2 = subprocess.run( + [bash], + input=script, + text=True, + capture_output=True, + check=False, + env=v2_env, + ) + + assert accepted_v2.returncode == 0, accepted_v2.stderr + assert output.read_text(encoding="utf-8").splitlines()[-3:] == [ + f"expected_base_sha={'b' * 40}", + "expected_head_sha=4afd4af7ad343660356791873d940aa2846f40c2", + "expected_base_branch=develop", + ] + + drift_cases = [ + ({**pull, "state": "closed"}, "closed"), + ( + {**pull, "base": {**pull["base"], "ref": "release"}}, + "base branch does not match", + ), + ( + {**pull, "base": {**pull["base"], "sha": "c" * 40}}, + "base SHA changed", + ), + ( + {**pull, "head": {**pull["head"], "sha": "d" * 40}}, + "head SHA changed", + ), + ] + for drifted_pull, message in drift_cases: + output.unlink(missing_ok=True) + rejected_v2 = subprocess.run( + [bash], + input=script, + text=True, + capture_output=True, + check=False, + env={**v2_env, "FAKE_PULL_JSON": json.dumps(drifted_pull)}, + ) + assert rejected_v2.returncode == 1 + assert message in rejected_v2.stdout + assert not output.exists() diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3e421e903..236612028 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -33,6 +33,7 @@ def fake_fine_grained_github_token(body): def make_pr(**overrides): value = { "number": 1, + "state": "OPEN", "title": "Central review", "isDraft": False, "mergeable": "MERGEABLE", @@ -3399,6 +3400,392 @@ def test_inspect_pr_cancels_stale_queued_runs_before_decision(monkeypatch): assert cancelled == [("owner/repo", 1, True)] +def test_snapshot_bound_inspection_performs_review_dispatch_without_general_mutations( + monkeypatch, +): + """A digest-bound mention cannot clean up, merge, or update unrelated state.""" + + def forbidden(*args, **kwargs): + raise AssertionError(f"unexpected general scheduler mutation: {args!r} {kwargs!r}") + + for name in ( + "cancel_stale_pr_runs", + "resolve_outdated_review_threads", + "dismiss_stale_opencode_approvals", + "dismiss_stale_opencode_change_requests", + "disable_auto_merge", + "enable_auto_merge", + "merge_pr", + "restamp_pr_head_for_last_push_approval", + "update_branch", + ): + monkeypatch.setattr(sched, name, forbidden) + monkeypatch.setattr(sched, "repository_dispatch_wait_reason", lambda repo, workflow: None) + dispatched = [] + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda repo, workflow, pr, dry_run, snapshot_guarded: dispatched.append( + (repo, workflow, pr["headRefOid"], dry_run, snapshot_guarded) + ) + or "dispatched", + ) + + decision = inspect( + make_pr(), + dry_run=False, + snapshot_guarded=True, + enable_auto_merge_flag=False, + merge_mode="disabled", + update_branches=False, + ) + + assert decision.action == "security_dispatch" + assert dispatched == [ + ("owner/repo", "Strix Security Scan", "head", False, True) + ] + + +def test_snapshot_bound_review_covers_every_fail_closed_decision_boundary(monkeypatch): + """The review-only path remains explicit for every terminal evidence state.""" + + monkeypatch.setattr( + sched, + "unresolved_thread_count", + lambda pr: pr.get("test_unresolved", 0), + ) + monkeypatch.setattr( + sched, + "has_current_head_changes_requested", + lambda pr: pr.get("test_changes_requested", False), + ) + monkeypatch.setattr( + sched, + "effective_merge_state", + lambda pr: pr.get("test_merge_state", "CLEAN"), + ) + monkeypatch.setattr( + sched, + "has_current_head_approval", + lambda pr: pr.get("test_approved", False), + ) + monkeypatch.setattr( + sched, + "failed_status_checks", + lambda pr: pr.get("test_failed", []), + ) + monkeypatch.setattr( + sched, + "action_required_checks", + lambda pr: pr.get("test_action_required", []), + ) + monkeypatch.setattr( + sched, + "branch_outdated_by_base", + lambda pr, merge_state: pr.get("test_behind", 0), + ) + monkeypatch.setattr( + sched, + "opencode_progress_state", + lambda pr, stale_after_minutes: pr.get("test_opencode", "absent"), + ) + monkeypatch.setattr( + sched, + "has_current_head_deterministic_fallback_approval", + lambda pr: pr.get("test_fallback", False), + ) + monkeypatch.setattr( + sched, + "strix_evidence_state", + lambda pr: pr.get("test_strix", "missing"), + ) + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: "dispatch unavailable" if workflow.startswith("wait-") else None, + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run, snapshot_guarded: pr.get( + "test_dispatch_result", "dispatched" + ), + ) + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda repo, workflow, pr, dry_run, snapshot_guarded: pr.get( + "test_dispatch_result", "dispatched" + ), + ) + + def decide( + pr, + *, + trigger_reviews=True, + review_dispatch_allowed=True, + workflow="review", + security_workflow="security", + ): + return sched.inspect_snapshot_bound_review( + "owner/repo", + pr, + dry_run=False, + trigger_reviews=trigger_reviews, + review_dispatch_allowed=review_dispatch_allowed, + workflow=workflow, + security_workflow=security_workflow, + base_branch="main", + stale_opencode_minutes=60, + ) + + cases = [ + (make_pr(state="CLOSED"), {}, "wait", "no longer open"), + (make_pr(isDraft=True), {}, "skip", "draft"), + (make_pr(baseRefName="release"), {}, "wait", "base branch changed"), + (make_pr(test_unresolved=2), {}, "block", "2 unresolved"), + (make_pr(test_changes_requested=True), {}, "block", "requested changes"), + (make_pr(test_merge_state="DIRTY"), {}, "block", "merge conflict"), + ( + make_pr(test_approved=True, test_failed=["quality"]), + {}, + "block", + "failed check(s)", + ), + ( + make_pr(test_action_required=["deploy"]), + {}, + "wait", + "workflow action required", + ), + (make_pr(test_behind=2), {}, "wait", "cannot update"), + (make_pr(test_merge_state="UNKNOWN"), {}, "wait", "still being calculated"), + (make_pr(test_approved=True), {}, "wait", "review-only"), + (make_pr(test_opencode="running"), {}, "wait", "already in progress"), + ( + make_pr(test_opencode="stale"), + {"trigger_reviews": False}, + "wait", + "dispatch disabled", + ), + ( + make_pr(test_opencode="stale"), + {"review_dispatch_allowed": False}, + "wait", + "limit reached", + ), + ( + make_pr(test_opencode="stale", test_dispatch_result="already_running"), + {}, + "wait", + "already active", + ), + ( + make_pr(test_opencode="stale", test_dispatch_result="snapshot_changed"), + {}, + "wait", + "different-head review run is active", + ), + (make_pr(test_opencode="stale"), {}, "review_dispatch", "re-dispatched"), + ( + make_pr(test_strix="missing"), + {"review_dispatch_allowed": False}, + "wait", + "limit reached", + ), + ( + make_pr(test_strix="missing"), + {"security_workflow": "wait-security"}, + "wait", + "dispatch unavailable", + ), + ( + make_pr(test_strix="missing", test_dispatch_result="snapshot_changed"), + {}, + "wait", + "different-head Strix run is active", + ), + ( + make_pr(test_strix="missing", test_dispatch_result="already_running"), + {}, + "wait", + "same-head Strix evidence", + ), + (make_pr(test_strix="running"), {}, "wait", "still running"), + ( + make_pr(test_strix="complete"), + {"review_dispatch_allowed": False}, + "wait", + "limit reached", + ), + ( + make_pr(test_strix="complete"), + {"workflow": "wait-review"}, + "wait", + "dispatch unavailable", + ), + ( + make_pr(test_strix="complete", test_dispatch_result="already_running"), + {}, + "wait", + "already active", + ), + ( + make_pr(test_strix="complete", test_dispatch_result="snapshot_changed"), + {}, + "wait", + "different-head review run is active", + ), + ( + make_pr(test_strix="complete"), + {}, + "review_dispatch", + "OpenCode dispatched", + ), + ( + make_pr(test_behind=1), + {"trigger_reviews": False}, + "block", + "dispatch disabled", + ), + ] + for pr, kwargs, action, reason in cases: + decision = decide(pr, **kwargs) + assert decision.action == action + assert reason in decision.reason + + monkeypatch.setenv("GITHUB_EVENT_NAME", "workflow_run") + fallback = decide(make_pr(test_fallback=True)) + assert fallback.action == "wait" + assert "deterministic fallback" in fallback.reason + no_fallback = decide( + make_pr(test_fallback=False), + trigger_reviews=False, + ) + assert no_fallback.action == "block" + + +def test_snapshot_guarded_review_dispatch_does_not_post_when_another_head_is_active(monkeypatch): + """A post-validation push prevents stale dispatch and receiver-side cancellation.""" + + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda action: None) + monkeypatch.setattr( + sched, + "active_opencode_run_refs", + lambda repo, workflow, pr: ([], [(repo, "new-head-run")]), + ) + monkeypatch.setattr( + sched, + "force_cancel_workflow_run_refs", + lambda refs: (_ for _ in ()).throw(AssertionError(f"unexpected cancellation: {refs}")), + ) + payloads = [] + monkeypatch.setattr( + sched, + "run_github_dispatch", + lambda args, stdin=None: payloads.append(json.loads(stdin)), + ) + pr = make_pr(baseRefOid="b" * 40, headRefOid="a" * 40) + + assert ( + sched.dispatch_opencode_review( + "owner/repo", + "OpenCode Review", + pr, + dry_run=False, + snapshot_guarded=True, + ) + == "snapshot_changed" + ) + assert payloads == [] + + +def test_snapshot_guarded_strix_dispatch_does_not_post_or_cancel_newer_runs( + monkeypatch, +): + """Snapshot mode leaves later-head Strix runs alone without queuing stale work.""" + + monkeypatch.setattr( + sched, + "matching_actions_job_id", + lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("unexpected rerun lookup")), + ) + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda action: None) + monkeypatch.setattr( + sched, + "active_review_run_refs", + lambda *args, **kwargs: ([], [("owner/repo", "new-head-run")]), + ) + monkeypatch.setattr( + sched, + "force_cancel_workflow_run_refs", + lambda refs: (_ for _ in ()).throw(AssertionError(f"unexpected cancellation: {refs}")), + ) + payloads = [] + monkeypatch.setattr( + sched, + "run_github_dispatch", + lambda args, stdin=None: payloads.append(json.loads(stdin)), + ) + pr = make_pr(baseRefOid="b" * 40, headRefOid="a" * 40) + + assert ( + sched.dispatch_strix_evidence( + "owner/repo", + "Strix Security Scan", + pr, + dry_run=False, + snapshot_guarded=True, + ) + == "snapshot_changed" + ) + assert payloads == [] + + +def test_snapshot_guarded_dispatch_deduplicates_same_head_without_cancellation( + monkeypatch, +): + """An exact-head active run suppresses duplicate OpenCode and Strix POSTs.""" + + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda action: None) + monkeypatch.setattr( + sched, + "active_opencode_run_refs", + lambda repo, workflow, pr: ([(repo, "same-head")], []), + ) + monkeypatch.setattr( + sched, + "active_review_run_refs", + lambda *args, **kwargs: ([("owner/repo", "same-head")], []), + ) + monkeypatch.setattr( + sched, + "force_cancel_workflow_run_refs", + lambda refs: (_ for _ in ()).throw(AssertionError(f"unexpected cancellation: {refs}")), + ) + monkeypatch.setattr( + sched, + "run_github_dispatch", + lambda *args, **kwargs: (_ for _ in ()).throw(AssertionError("unexpected POST")), + ) + pr = make_pr(baseRefOid="b" * 40, headRefOid="a" * 40) + + assert sched.dispatch_opencode_review( + "owner/repo", + "OpenCode Review", + pr, + dry_run=False, + snapshot_guarded=True, + ) == "already_running" + assert sched.dispatch_strix_evidence( + "owner/repo", + "Strix Security Scan", + pr, + dry_run=False, + snapshot_guarded=True, + ) == "already_running" + + def test_inspect_pr_blocks_auto_merge_for_approved_conflicts(monkeypatch): auto_merges = [] disables = [] @@ -4365,6 +4752,228 @@ def test_main_rejects_invalid_branch_update_limit(): ) +@pytest.mark.parametrize( + ("observed_head", "observed_base", "observed_branch", "observed_state", "message"), + [ + ("c" * 40, "b" * 40, "main", "OPEN", "head SHA changed"), + ("a" * 40, "d" * 40, "main", "OPEN", "base SHA changed"), + ("a" * 40, "b" * 40, "release", "OPEN", "base branch changed"), + ("a" * 40, "b" * 40, "main", "CLOSED", "no longer open"), + ], +) +def test_main_rejects_target_snapshot_drift_before_inspection( + monkeypatch, + observed_head, + observed_base, + observed_branch, + observed_state, + message, +): + """A validated mention cannot act after the PR head or base advances.""" + + inspected = [] + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: [ + make_pr( + number=number, + headRefOid=observed_head, + baseRefOid=observed_base, + baseRefName=observed_branch, + state=observed_state, + ) + ], + ) + monkeypatch.setattr( + sched, + "inspect_pr", + lambda *args, **kwargs: inspected.append((args, kwargs)), + ) + + with pytest.raises(SystemExit, match=message): + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--pr-number", + "7", + "--expected-head-sha", + "a" * 40, + "--expected-base-sha", + "b" * 40, + "--expected-base-branch", + "main", + ] + ) + + assert inspected == [] + + +def test_main_accepts_the_exact_expected_target_snapshot(monkeypatch, capsys): + """Matching immutable refs reach inspection with the review-only policy.""" + + inspected = [] + monkeypatch.setattr( + sched, + "fetch_pr", + lambda repo, number: [ + make_pr(number=number, headRefOid="a" * 40, baseRefOid="b" * 40) + ], + ) + + def fake_inspect(repo, pr, **kwargs): + inspected.append((repo, pr, kwargs)) + return sched.Decision(pr["number"], "skip", "exact snapshot") + + monkeypatch.setattr(sched, "inspect_pr", fake_inspect) + + assert ( + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--pr-number", + "7", + "--expected-head-sha", + "a" * 40, + "--expected-base-sha", + "b" * 40, + "--expected-base-branch", + "main", + "--no-enable-auto-merge", + "--merge-mode", + "disabled", + "--no-update-branches", + ] + ) + == 0 + ) + assert len(inspected) == 1 + assert inspected[0][1]["headRefOid"] == "a" * 40 + assert inspected[0][1]["baseRefOid"] == "b" * 40 + assert inspected[0][2]["enable_auto_merge_flag"] is False + assert inspected[0][2]["merge_mode"] == "disabled" + assert inspected[0][2]["update_branches"] is False + assert inspected[0][2]["snapshot_guarded"] is True + assert "exact snapshot" in capsys.readouterr().out + + +@pytest.mark.parametrize( + ("prs", "pr_number", "head", "base", "branch", "message"), + [ + ([], 7, "a" * 40, "", "main", "must be supplied together"), + ([], 7, "", "b" * 40, "main", "must be supplied together"), + ([], 7, "a" * 40, "b" * 40, "", "must be supplied together"), + ([], 0, "a" * 40, "b" * 40, "main", "require --pr-number"), + ([], 7, "invalid", "b" * 40, "main", "invalid git sha"), + ([], 7, "a" * 40, "b" * 40, "bad..branch", "invalid git ref"), + ([], 7, "a" * 40, "b" * 40, "main", "unavailable or ambiguous"), + ( + [make_pr(number=8, headRefOid="a" * 40, baseRefOid="b" * 40)], + 7, + "a" * 40, + "b" * 40, + "main", + "unavailable or ambiguous", + ), + ( + [make_pr(number=7, headRefOid="invalid", baseRefOid="b" * 40)], + 7, + "a" * 40, + "b" * 40, + "main", + "returned malformed refs", + ), + ( + [make_pr(number=7, headRefOid="a" * 40, baseRefOid="invalid")], + 7, + "a" * 40, + "b" * 40, + "main", + "returned malformed refs", + ), + ], +) +def test_expected_snapshot_guard_rejects_incomplete_or_malformed_inputs( + prs, + pr_number, + head, + base, + branch, + message, +): + """Target snapshot guards reject incomplete identities before inspection.""" + + with pytest.raises(SystemExit, match=message): + sched.validate_expected_pr_snapshot( + prs, + pr_number=pr_number, + expected_head_sha=head, + expected_base_sha=base, + expected_base_branch=branch, + ) + + +def test_main_refetches_a_guarded_snapshot_before_any_general_scheduler_action( + monkeypatch, +): + """A push after initial validation aborts before cleanup or dispatch.""" + + snapshots = [ + make_pr(number=7, headRefOid="a" * 40, baseRefOid="b" * 40), + make_pr(number=7, headRefOid="c" * 40, baseRefOid="b" * 40), + ] + fetches = [] + general_inspections = [] + + def fake_fetch(repo, number): + fetches.append((repo, number)) + return [snapshots.pop(0)] + + monkeypatch.setattr(sched, "fetch_pr", fake_fetch) + monkeypatch.setattr( + sched, + "inspect_pr", + lambda *args, **kwargs: general_inspections.append((args, kwargs)), + ) + + with pytest.raises(SystemExit, match="head SHA changed"): + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--pr-number", + "7", + "--expected-head-sha", + "a" * 40, + "--expected-base-sha", + "b" * 40, + "--expected-base-branch", + "main", + "--no-enable-auto-merge", + "--merge-mode", + "disabled", + "--no-update-branches", + ] + ) + + assert fetches == [("owner/repo", 7), ("owner/repo", 7)] + assert general_inspections == [] + + def test_print_summary_self_test_parse_args_and_main(monkeypatch, capsys): sched.print_summary( [sched.Decision(1, "wait", "ready"), sched.Decision(2, "wait", "queued")], diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 233c08584..54b0cfd7e 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -77,6 +77,11 @@ def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> Non assert '[ "$live_base_repository" != "$TARGET_REPOSITORY_INPUT" ]' in validation assert '[ "$live_head_repository" != "$TARGET_REPOSITORY_INPUT" ]' in validation assert "Targeted scheduler dispatch base branch does not match the live PR" in validation + assert "TARGET_HEAD_SHA_INPUT:" in validation + assert "TARGET_BASE_SHA_INPUT:" in validation + assert "live_base_sha=" in validation + assert '"$TARGET_HEAD_SHA_INPUT" != "$live_head_sha"' in validation + assert '"$TARGET_BASE_SHA_INPUT" != "$live_base_sha"' in validation assert "TARGET_REPOSITORY: ${{ steps.targeted_dispatch.outputs.repository }}" in inspect assert ( "TARGET_DEFAULT_BRANCH: ${{ steps.targeted_dispatch.outputs.base_branch }}" @@ -85,9 +90,13 @@ def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> Non assert '--repo "$TARGET_REPOSITORY"' in inspect assert '--base-branch "$TARGET_DEFAULT_BRANCH"' in inspect assert 'args+=(--pr-number "$PULL_REQUEST_NUMBER")' in inspect + assert '--expected-head-sha "$EXPECTED_HEAD_SHA"' in inspect + assert '--expected-base-sha "$EXPECTED_BASE_SHA"' in inspect assert ( "github.event_name == 'repository_dispatch' && " - "github.event.client_payload.target_repository != '' && " + "(github.event.action == 'merge-scheduler-agent-review-v2' && " + "github.event.client_payload.claim.repository != '' || " + "github.event.client_payload.target_repository != '') && " "(secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || " "steps.scheduler_app_token.outputs.token) || github.token" ) in inspect @@ -111,7 +120,13 @@ def test_privileged_review_retries_use_default_branch_repository_dispatch() -> N trigger_contract = workflow.split("concurrency:", 1)[0] assert "repository_dispatch:" in trigger_contract - assert f"types: [{event_type}]" in trigger_contract + if filename == "pr-review-merge-scheduler.yml": + assert ( + "types: [merge-scheduler, merge-scheduler-agent-review-v2]" + in trigger_contract + ) + else: + assert f"types: [{event_type}]" in trigger_contract assert "workflow_dispatch:" not in trigger_contract assert "github.event.inputs" not in workflow assert "github.event.client_payload" in workflow @@ -206,7 +221,7 @@ def test_central_semgrep_logs_every_finding_and_distinguishes_engine_failure() - assert "Semgrep engine/configuration failed with rc=${SEMGREP_RC}" in workflow -def test_strix_cancels_superseded_pr_head_security_evidence() -> None: +def test_strix_isolates_repository_dispatch_runs_from_stale_event_cancellation() -> None: workflow = workflow_text("strix.yml") concurrency_contract = workflow.split("concurrency:", 1)[1].split( "permissions:", 1 @@ -221,13 +236,10 @@ def test_strix_cancels_superseded_pr_head_security_evidence() -> None: "github.event.pull_request.base.repo.full_name || github.repository }}" ) in concurrency_contract assert "format('pr-{0}', github.event.pull_request.number)" in concurrency_contract - assert "github.event.client_payload.pr_number != '' && format('pr-{0}'," in workflow - assert "format('pr-{0}-{1}'" not in concurrency_contract - assert "github.event.pull_request.head.sha" not in concurrency_contract - assert "github.event.client_payload.pr_head_sha" not in concurrency_contract - assert "cancel-in-progress: true" in workflow + assert "github.event_name == 'repository_dispatch' && github.run_id" in concurrency_contract + assert "cancel-in-progress: ${{ github.event_name != 'repository_dispatch' }}" in workflow assert "default-branch repository_dispatch evidence cannot cancel" in workflow - assert "PR-number scope keeps the queue on the current HEAD" in workflow + assert "run-id scope prevents an out-of-order stale dispatch" in workflow assert ( "refs/pull//head has already advanced before this queued run starts" in workflow @@ -281,8 +293,8 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - assert "${{ secrets." not in opencode_bootstrap strix_workflow = workflow_text("strix.yml") - assert "cancel-in-progress: true" in strix_workflow - assert "PR-number scope keeps the queue on the current HEAD" in strix_workflow + assert "cancel-in-progress: ${{ github.event_name != 'repository_dispatch' }}" in strix_workflow + assert "run-id scope prevents an out-of-order stale dispatch" in strix_workflow def test_close_empty_pr_metadata_lookup_retries_and_fails_open() -> None: From 2fe27c077dc96a408686493697f98c250c57be17 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 00:04:50 +0900 Subject: [PATCH 2/6] test(agent-mention): require complete quality gate scope --- tests/test_agent_mention_workflow_contract.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/test_agent_mention_workflow_contract.py b/tests/test_agent_mention_workflow_contract.py index c5fc4cae5..6958a5a05 100644 --- a/tests/test_agent_mention_workflow_contract.py +++ b/tests/test_agent_mention_workflow_contract.py @@ -52,11 +52,22 @@ def test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes() -> def test_quality_workflow_measures_exact_files_without_module_name_warnings() -> None: - """Coverage includes the two script paths instead of treating paths as modules.""" + """Coverage and docstring gates include every changed production helper.""" text = QUALITY_WORKFLOW.read_text(encoding="utf-8") + trigger_block = text.split("\nconcurrency:\n", 1)[0] coverage_config = text.split("[run]\n", 1)[1].split("[report]\n", 1)[0] + interrogate_block = text.split( + "python -m interrogate --fail-under=100 \\\n", 1 + )[1].split("\n python -m compileall", 1)[0] + assert "include =" in coverage_config assert "source =" not in coverage_config - assert "scripts/ci/agent_mention_router.py" in coverage_config - assert "scripts/ci/agent_mention_sweep.py" in coverage_config + for production_path in ( + "scripts/ci/agent_mention_router.py", + "scripts/ci/agent_mention_sweep.py", + "scripts/ci/pr_review_merge_scheduler.py", + ): + assert production_path in coverage_config + assert production_path in interrogate_block + assert trigger_block.count('"scripts/ci/pr_review_fix_scheduler.py"') == 2 From 7bffba7f7c4f5e19389a402752f9db1eebe25367 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 00:06:18 +0900 Subject: [PATCH 3/6] fix(agent-mention): cover scheduler quality surfaces --- .github/workflows/agent-mention-router-quality-ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/agent-mention-router-quality-ci.yml b/.github/workflows/agent-mention-router-quality-ci.yml index 66634aa7c..2bb5f0b28 100644 --- a/.github/workflows/agent-mention-router-quality-ci.yml +++ b/.github/workflows/agent-mention-router-quality-ci.yml @@ -15,6 +15,7 @@ on: - "scripts/ci/agent_mention_router.py" - "scripts/ci/agent_mention_sweep.py" - "scripts/ci/pr_review_merge_scheduler.py" + - "scripts/ci/pr_review_fix_scheduler.py" - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_agent_mention_*.py" - "tests/test_opencode_agent_contract.py" @@ -37,6 +38,7 @@ on: - "scripts/ci/agent_mention_router.py" - "scripts/ci/agent_mention_sweep.py" - "scripts/ci/pr_review_merge_scheduler.py" + - "scripts/ci/pr_review_fix_scheduler.py" - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_agent_mention_*.py" - "tests/test_opencode_agent_contract.py" @@ -117,6 +119,7 @@ jobs: include = scripts/ci/agent_mention_router.py scripts/ci/agent_mention_sweep.py + scripts/ci/pr_review_merge_scheduler.py [report] fail_under = 100 show_missing = True @@ -127,6 +130,7 @@ jobs: python -m coverage report --fail-under=100 python -m interrogate --fail-under=100 \ scripts/ci/agent_mention_router.py \ - scripts/ci/agent_mention_sweep.py + scripts/ci/agent_mention_sweep.py \ + scripts/ci/pr_review_merge_scheduler.py python -m compileall -q scripts/ci tests git diff --check "$CHANGE_DIFF_RANGE" From e408117ee7bd11c858f1c9a80956985e552162ec Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 00:07:42 +0900 Subject: [PATCH 4/6] test(agent-mention): cover wrapper validation branches --- ...nt_mention_repository_dispatch_envelope.py | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/test_agent_mention_repository_dispatch_envelope.py b/tests/test_agent_mention_repository_dispatch_envelope.py index 8b5f2b3d4..92e94b2fe 100644 --- a/tests/test_agent_mention_repository_dispatch_envelope.py +++ b/tests/test_agent_mention_repository_dispatch_envelope.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import importlib.util import json import os @@ -73,7 +74,9 @@ def _python_heredoc(step: str) -> str: return textwrap.dedent(step[start:end]) -def _run_python_contract(code: str, environment: dict[str, str]) -> subprocess.CompletedProcess: +def _run_python_contract( + code: str, environment: dict[str, str] +) -> subprocess.CompletedProcess: """Execute one extracted workflow validator with an isolated environment.""" return subprocess.run( @@ -85,6 +88,18 @@ def _run_python_contract(code: str, environment: dict[str, str]) -> subprocess.C ) +def _rebind_invocation_key(payload: dict[str, object]) -> None: + """Recompute the canonical claim digest after an intentional claim mutation.""" + + canonical = json.dumps( + payload["claim"], + ensure_ascii=True, + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + payload["agent_invocation_key"] = hashlib.sha256(canonical).hexdigest() + + def test_router_emits_three_key_versioned_envelope_without_changing_claim_key() -> None: """Keep the transport bounded while preserving existing ledger identities.""" @@ -135,7 +150,7 @@ def test_wrapper_validates_and_reuses_the_same_bounded_envelope_before_ledger() assert "github.event.client_payload.claim.repository" in workflow assert "github.event.client_payload.claim.pr_number" in workflow - assert f'PAYLOAD_SCHEMA: ${{{{ github.event.client_payload.schema || \'\' }}}}' in workflow + assert "PAYLOAD_SCHEMA: ${{ github.event.client_payload.schema || '' }}" in workflow assert "set(envelope)" in validate for key in sorted(ENVELOPE_KEYS): assert f'"{key}"' in validate @@ -199,6 +214,11 @@ def test_wrapper_executes_the_validated_envelope_as_the_exact_second_hop( "wrong-boolean-type", "altered-bound-field", "unsupported-schema", + "policy-violating-claim", + "invalid-repository", + "invalid-head-sha", + "invalid-base-branch", + "invalid-actor", ], ) def test_wrapper_rejects_malformed_or_unbound_envelopes_before_materialization( @@ -219,6 +239,21 @@ def test_wrapper_rejects_malformed_or_unbound_envelopes_before_materialization( payload["claim"]["head_sha"] = "c" * 40 elif mutation == "unsupported-schema": payload["schema"] = "cwl.agent-invocation/v3" + elif mutation == "policy-violating-claim": + payload["claim"]["update_branches"] = True + _rebind_invocation_key(payload) + elif mutation == "invalid-repository": + payload["claim"]["repository"] = "OtherOrg/example" + _rebind_invocation_key(payload) + elif mutation == "invalid-head-sha": + payload["claim"]["head_sha"] = "not-a-sha" + _rebind_invocation_key(payload) + elif mutation == "invalid-base-branch": + payload["claim"]["base_branch"] = "-main" + _rebind_invocation_key(payload) + elif mutation == "invalid-actor": + payload["claim"]["actor"] = "invalid actor" + _rebind_invocation_key(payload) else: # pragma: no cover - the parameter list is exhaustive raise AssertionError(mutation) From ba93f537302086ae54a919ea45c3ca96f2458415 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 00:38:04 +0900 Subject: [PATCH 5/6] fix(actions): address remaining exact-head review findings --- .../agent-mention-router-quality-ci.yml | 4 +- .../workflows/opencode-review-dispatch.yml | 17 +++- .github/workflows/strix.yml | 4 + scripts/ci/pr_review_merge_scheduler.py | 7 +- scripts/ci/test_strix_quick_gate.sh | 10 +- ...st_agent_mention_downstream_idempotency.py | 1 + ...nt_mention_repository_dispatch_envelope.py | 54 +++++++---- tests/test_opencode_agent_contract.py | 91 ++++++++++++++++++- tests/test_pr_review_merge_scheduler.py | 15 ++- .../test_required_workflow_queue_contract.py | 4 + 10 files changed, 178 insertions(+), 29 deletions(-) diff --git a/.github/workflows/agent-mention-router-quality-ci.yml b/.github/workflows/agent-mention-router-quality-ci.yml index 2bb5f0b28..d18ed53b2 100644 --- a/.github/workflows/agent-mention-router-quality-ci.yml +++ b/.github/workflows/agent-mention-router-quality-ci.yml @@ -14,8 +14,8 @@ on: - "docs/automation/review-agent-comment-invocation.md" - "scripts/ci/agent_mention_router.py" - "scripts/ci/agent_mention_sweep.py" - - "scripts/ci/pr_review_merge_scheduler.py" - "scripts/ci/pr_review_fix_scheduler.py" + - "scripts/ci/pr_review_merge_scheduler.py" - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_agent_mention_*.py" - "tests/test_opencode_agent_contract.py" @@ -37,8 +37,8 @@ on: - "docs/automation/review-agent-comment-invocation.md" - "scripts/ci/agent_mention_router.py" - "scripts/ci/agent_mention_sweep.py" - - "scripts/ci/pr_review_merge_scheduler.py" - "scripts/ci/pr_review_fix_scheduler.py" + - "scripts/ci/pr_review_merge_scheduler.py" - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_agent_mention_*.py" - "tests/test_opencode_agent_contract.py" diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index e532041ca..c2f0bf868 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -12,10 +12,21 @@ on: types: [opencode-review] concurrency: - # A repository_dispatch is validated only after queue admission. Run-id scope - # prevents an out-of-order stale dispatch from cancelling newer valid work. - group: opencode-review-repository-dispatch-${{ github.run_id }} + # Serialize valid dispatches per sender and target PR without dropping an + # already-pending newer head when a delayed stale event arrives. queue:max + # preserves pending runs; payloads missing queue-key fields get isolated by run + # id, and cancel:false lets live metadata validation reject stale snapshots. + group: >- + opencode-review-repository-dispatch-${{ + github.event.sender.id && + github.event.client_payload.target_repository && + github.event.client_payload.pr_number && + format('sender-{0}-{1}-pr-{2}', github.event.sender.id, + github.event.client_payload.target_repository, + github.event.client_payload.pr_number) || + format('invalid-{0}', github.run_id) }} cancel-in-progress: false + queue: max permissions: contents: read diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 456d99fd2..d962ad518 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -71,6 +71,10 @@ concurrency: # the required pull_request_target Strix context that branch protection reads. # Repository-dispatch run-id scope prevents an out-of-order stale dispatch from # cancelling newer valid evidence before live metadata validation can reject it. + # A shared PR-only dispatch group is unsafe here. + # default pending-run replacement could discard newer valid evidence. + # queue: max cannot be combined with the pull-request cancellation policy used + # by this mixed-event workflow. group: >- strix-${{ github.event_name }}-${{ github.event.client_payload.target_repository || github.event.pull_request.base.repo.full_name || github.repository }}-${{ github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 53b2c5a90..6b8331d2c 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -2328,10 +2328,15 @@ def inspect_snapshot_bound_review( behind_by = branch_outdated_by_base(pr, merge_state) if behind_by and trigger_reviews: + approval_state = ( + "current head is approved" + if current_head_approved + else "current head has no OpenCode approval" + ) return Decision( number, "wait", - "current head has no OpenCode approval; snapshot-bound review cannot update an outdated branch", + f"{approval_state}; snapshot-bound review cannot update an outdated branch", ) if merge_state == "UNKNOWN": return Decision( diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 544edfa5b..2dc613844 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -513,9 +513,15 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { if awk '/^ required-workflow-bootstrap:$/,/^[^ ]/' "$bootstrap_file" | grep -q '^[[:space:]]*if:'; then record_failure "opencode required workflow bootstrap must not depend on required-workflow event payload fields" fi - assert_file_contains "$workflow_file" 'group: opencode-review-repository-dispatch-${{ github.run_id }}' "opencode isolates every dispatch until live snapshot validation" + assert_file_contains "$workflow_file" 'opencode-review-repository-dispatch-${{' "opencode defines a bounded repository-dispatch concurrency group" + assert_file_contains "$workflow_file" 'github.event.sender.id' "opencode isolates pre-validation queues by immutable dispatch sender" + assert_file_contains "$workflow_file" 'github.event.client_payload.target_repository' "opencode serializes valid dispatches by target repository" + assert_file_contains "$workflow_file" 'github.event.client_payload.pr_number' "opencode serializes valid dispatches by pull request" + assert_file_contains "$workflow_file" "format('sender-{0}-{1}-pr-{2}'" "opencode derives the validated sender/repository/PR queue key" + assert_file_contains "$workflow_file" "format('invalid-{0}', github.run_id)" "opencode isolates payloads missing pre-validation queue keys" assert_file_contains "$workflow_file" 'cancel-in-progress: false' "opencode prevents stale pre-validation dispatches from cancelling newer valid work" - assert_file_contains "$workflow_file" "Run-id scope" "opencode documents stale dispatch isolation" + assert_file_contains "$workflow_file" 'queue: max' "opencode preserves valid pending dispatches instead of replacing them" + assert_file_contains "$workflow_file" "preserves pending runs" "opencode documents stale dispatch queue preservation" assert_file_contains "$workflow_file" "Materialize pull request merge tree for coverage measurement" "opencode pull_request coverage execution materializes the exact base/head merge tree" assert_file_contains "$workflow_file" "stale OpenCode run: event head=" "opencode review side effects are skipped for stale heads" assert_file_not_contains "$workflow_file" "github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name" "opencode never treats a same-repository pull_request_target head as authorization to execute PR-controlled code" diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index 9be43ebc3..a3940924b 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -107,3 +107,4 @@ def test_quality_gate_runs_full_suite_for_docs_and_exact_diff() -> None: coverage_config = text.split("[run]\n", 1)[1].split("[report]\n", 1)[0] assert "scripts/ci/agent_mention_router.py" in coverage_config assert "scripts/ci/agent_mention_sweep.py" in coverage_config + assert "scripts/ci/pr_review_merge_scheduler.py" in coverage_config diff --git a/tests/test_agent_mention_repository_dispatch_envelope.py b/tests/test_agent_mention_repository_dispatch_envelope.py index 92e94b2fe..786b4e56b 100644 --- a/tests/test_agent_mention_repository_dispatch_envelope.py +++ b/tests/test_agent_mention_repository_dispatch_envelope.py @@ -207,23 +207,25 @@ def test_wrapper_executes_the_validated_envelope_as_the_exact_second_hop( @pytest.mark.parametrize( - "mutation", + ("mutation", "error_fragment"), [ - "extra-envelope-field", - "missing-claim-field", - "wrong-boolean-type", - "altered-bound-field", - "unsupported-schema", - "policy-violating-claim", - "invalid-repository", - "invalid-head-sha", - "invalid-base-branch", - "invalid-actor", + ("extra-envelope-field", "invalid OpenCode invocation envelope"), + ("missing-claim-field", "invalid OpenCode invocation claim fields"), + ("wrong-boolean-type", "invalid enable_auto_merge flag"), + ("altered-bound-field", "invocation key does not match canonical payload"), + ("unsupported-schema", "unsupported OpenCode invocation schema"), + ("policy-violating-claim", "violates review-only policy"), + ("invalid-repository", "invalid repository"), + ("invalid-head-sha", "invalid head SHA"), + ("invalid-base-sha", "invalid base SHA"), + ("invalid-base-branch", "invalid base branch"), + ("invalid-actor", "invalid actor"), ], ) def test_wrapper_rejects_malformed_or_unbound_envelopes_before_materialization( tmp_path: Path, mutation: str, + error_fragment: str, ) -> None: """Unknown, malformed, or key-mismatched claims fail before ledger access.""" @@ -241,22 +243,29 @@ def test_wrapper_rejects_malformed_or_unbound_envelopes_before_materialization( payload["schema"] = "cwl.agent-invocation/v3" elif mutation == "policy-violating-claim": payload["claim"]["update_branches"] = True - _rebind_invocation_key(payload) elif mutation == "invalid-repository": payload["claim"]["repository"] = "OtherOrg/example" - _rebind_invocation_key(payload) elif mutation == "invalid-head-sha": - payload["claim"]["head_sha"] = "not-a-sha" - _rebind_invocation_key(payload) + payload["claim"]["head_sha"] = "z" * 40 + elif mutation == "invalid-base-sha": + payload["claim"]["base_sha"] = "z" * 40 elif mutation == "invalid-base-branch": payload["claim"]["base_branch"] = "-main" - _rebind_invocation_key(payload) elif mutation == "invalid-actor": - payload["claim"]["actor"] = "invalid actor" - _rebind_invocation_key(payload) + payload["claim"]["actor"] = "invalid_actor" else: # pragma: no cover - the parameter list is exhaustive raise AssertionError(mutation) + if mutation in { + "policy-violating-claim", + "invalid-repository", + "invalid-head-sha", + "invalid-base-sha", + "invalid-base-branch", + "invalid-actor", + }: + _rebind_invocation_key(payload) + workflow = WRAPPER_WORKFLOW.read_text(encoding="utf-8") code = _python_heredoc( _named_step( @@ -276,6 +285,7 @@ def test_wrapper_rejects_malformed_or_unbound_envelopes_before_materialization( ) assert completed.returncode != 0 + assert error_fragment in completed.stderr assert not (tmp_path / "agent-review-scheduler-request.json").exists() @@ -373,6 +383,14 @@ def test_agent_mention_quality_gate_covers_the_downstream_scheduler_contract() - for path in ( '.github/workflows/pr-review-merge-scheduler.yml', 'scripts/ci/pr_review_merge_scheduler.py', + 'scripts/ci/pr_review_fix_scheduler.py', 'tests/test_pr_review_merge_scheduler.py', ): assert workflow.count(f' - "{path}"') == 2 + + coverage_config = workflow.split("[run]\n", 1)[1].split("[report]\n", 1)[0] + assert "scripts/ci/pr_review_merge_scheduler.py" in coverage_config + interrogate = workflow.split("python -m interrogate --fail-under=100", 1)[1].split( + "python -m compileall", 1 + )[0] + assert "scripts/ci/pr_review_merge_scheduler.py" in interrogate diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index f578a2fed..094a1e87f 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -850,6 +850,89 @@ def test_opencode_repository_dispatch_authorization_is_fail_closed(): assert expected_reason in rejected.stdout +def test_opencode_repository_dispatch_rejects_stale_snapshot_before_downstream( + tmp_path: Path, +): + """A delayed old-head dispatch cannot pass live metadata binding ahead of B.""" + + workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( + encoding="utf-8" + ) + validate_step = workflow.split( + " - name: Bind workflow inputs to live organization pull request metadata\n", + 1, + )[1].split("\n\n coverage-source-tree:", 1)[0] + shell = textwrap.dedent(validate_step.split(" run: |\n", 1)[1]) + + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_gh = fake_bin / "gh" + fake_gh.write_text( + "#!/bin/sh\nprintf '%s\\n' \"$LIVE_PR_JSON\"\n", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + + live_head = "b" * 40 + live_base = "c" * 40 + live_pr = { + "state": "open", + "base": { + "ref": "main", + "sha": live_base, + "repo": { + "full_name": "ContextualWisdomLab/example", + "private": False, + }, + }, + "head": { + "ref": "feature", + "sha": live_head, + "repo": {"full_name": "ContextualWisdomLab/example"}, + }, + } + output_file = tmp_path / "github-output" + base_env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "LIVE_PR_JSON": json.dumps(live_pr), + "GITHUB_OUTPUT": str(output_file), + "EVENT_NAME": "repository_dispatch", + "DISPATCH_ACTOR": "github-actions[bot]", + "DISPATCH_SENDER": "github-actions[bot]", + "ALLOWED_DISPATCH_ACTOR": "github-actions[bot]", + "ALLOWED_DISPATCH_TARGETS": "ContextualWisdomLab/example", + "TARGET_REPOSITORY": "ContextualWisdomLab/example", + "PR_NUMBER": "17", + "SUPPLIED_BASE_REF": "main", + "SUPPLIED_BASE_SHA": live_base, + "SUPPLIED_HEAD_REF": "feature", + } + + stale = subprocess.run( + ["bash", "-c", shell], + env={**base_env, "SUPPLIED_HEAD_SHA": "a" * 40}, + text=True, + capture_output=True, + check=False, + ) + assert stale.returncode == 1 + assert "metadata does not match the live pull request: head_sha" in stale.stdout + assert not output_file.exists() + + current = subprocess.run( + ["bash", "-c", shell], + env={**base_env, "SUPPLIED_HEAD_SHA": live_head}, + text=True, + capture_output=True, + check=False, + ) + assert current.returncode == 0, current.stderr + outputs = output_file.read_text(encoding="utf-8") + assert f"base_sha={live_base}" in outputs + assert f"head_sha={live_head}" in outputs + + def test_opencode_model_exhaustion_retry_stays_owned_by_central_scheduler(): """Do not broaden workflow permissions for a recursive review dispatch.""" workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text(encoding="utf-8") @@ -1271,9 +1354,13 @@ def test_workflow_provisions_sandbox_tool_and_reviewer_agent(): "permissions:", 1 )[0] assert "opencode-review-repository-dispatch-" in concurrency_contract - assert "github.run_id" in concurrency_contract - assert "github.event.client_payload.pr_number" not in concurrency_contract + assert "github.event.sender.id" in concurrency_contract + assert "github.event.client_payload.target_repository" in concurrency_contract + assert "github.event.client_payload.pr_number" in concurrency_contract + assert "format('sender-{0}-{1}-pr-{2}'" in concurrency_contract + assert "format('invalid-{0}', github.run_id)" in concurrency_contract assert "cancel-in-progress: false" in concurrency_contract + assert "queue: max" in concurrency_contract assert "github.event.pull_request" not in concurrency_contract assert "OPENCODE_MODEL_CANDIDATES" in workflow model_pool_runner = Path("scripts/ci/run_opencode_review_model_pool.sh").read_text( diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 236612028..ce9434377 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -3558,7 +3558,12 @@ def decide( "wait", "workflow action required", ), - (make_pr(test_behind=2), {}, "wait", "cannot update"), + ( + make_pr(test_behind=2), + {}, + "wait", + "current head has no OpenCode approval; snapshot-bound review cannot update", + ), (make_pr(test_merge_state="UNKNOWN"), {}, "wait", "still being calculated"), (make_pr(test_approved=True), {}, "wait", "review-only"), (make_pr(test_opencode="running"), {}, "wait", "already in progress"), @@ -3654,6 +3659,14 @@ def decide( assert decision.action == action assert reason in decision.reason + approved_behind = decide(make_pr(test_behind=2, test_approved=True)) + assert approved_behind == sched.Decision( + 1, + "wait", + "current head is approved; snapshot-bound review cannot update an outdated branch", + ) + assert "no OpenCode approval" not in approved_behind.reason + monkeypatch.setenv("GITHUB_EVENT_NAME", "workflow_run") fallback = decide(make_pr(test_fallback=True)) assert fallback.action == "wait" diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 54b0cfd7e..cf901fc23 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -240,6 +240,9 @@ def test_strix_isolates_repository_dispatch_runs_from_stale_event_cancellation() assert "cancel-in-progress: ${{ github.event_name != 'repository_dispatch' }}" in workflow assert "default-branch repository_dispatch evidence cannot cancel" in workflow assert "run-id scope prevents an out-of-order stale dispatch" in workflow + assert "default pending-run replacement could discard newer valid evidence" in workflow + assert "queue: max cannot be combined with the pull-request cancellation policy" in workflow + assert "\n queue: max\n" not in concurrency_contract assert ( "refs/pull//head has already advanced before this queued run starts" in workflow @@ -295,6 +298,7 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - strix_workflow = workflow_text("strix.yml") assert "cancel-in-progress: ${{ github.event_name != 'repository_dispatch' }}" in strix_workflow assert "run-id scope prevents an out-of-order stale dispatch" in strix_workflow + assert "default pending-run replacement could discard newer valid evidence" in strix_workflow def test_close_empty_pr_metadata_lookup_retries_and_fails_open() -> None: From 04c68a997b658019a1df9958a6bfe88ec72f792c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 9 Aug 2026 15:06:20 +0900 Subject: [PATCH 6/6] test(agent-mention): cover fix scheduler quality surface --- .github/workflows/agent-mention-router-quality-ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/agent-mention-router-quality-ci.yml b/.github/workflows/agent-mention-router-quality-ci.yml index d18ed53b2..7492b7671 100644 --- a/.github/workflows/agent-mention-router-quality-ci.yml +++ b/.github/workflows/agent-mention-router-quality-ci.yml @@ -119,6 +119,7 @@ jobs: include = scripts/ci/agent_mention_router.py scripts/ci/agent_mention_sweep.py + scripts/ci/pr_review_fix_scheduler.py scripts/ci/pr_review_merge_scheduler.py [report] fail_under = 100 @@ -131,6 +132,7 @@ jobs: python -m interrogate --fail-under=100 \ scripts/ci/agent_mention_router.py \ scripts/ci/agent_mention_sweep.py \ + scripts/ci/pr_review_fix_scheduler.py \ scripts/ci/pr_review_merge_scheduler.py python -m compileall -q scripts/ci tests git diff --check "$CHANGE_DIFF_RANGE"